View previous topic :: View next topic |
Author |
Message |
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
values not calculating correctly - CLOSED |
Posted: Mon Nov 23, 2020 4:55 pm |
|
|
Hi
I'm working on a DS18B20 and for some reason
Code: | //DS_Raw_Temperatures[DS_Reading_Counter]=(float)(signed int16)(make16(OW_Buffer[1],OW_Buffer[0]))/16.0;
Temp_Raw=make16(OW_Buffer[1],OW_Buffer[0]);
DS_Raw_Temperatures[DS_Reading_Counter]=(float)(signed int16)Temp_Raw/16.0; |
I've had to change the first line into 2 lines as shown above.
The previous line worked fine on the same hardware and printing the values before the original line, showed the right values, CRC checks out etc.
Why?
Thanks. _________________ CCS PCM 5.078 & CCS PCH 5.093
Last edited by Gabriel on Tue Nov 24, 2020 3:15 pm; edited 1 time in total |
|
![](templates/subSilver/images/spacer.gif) |
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Mon Nov 23, 2020 6:43 pm |
|
|
Post the variable declarations for all the variables in your code above.
Give us some sample values for OW_Buffer[0] and OW_Buffer[1].
Also post your PIC and compiler version. |
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19358
|
|
Posted: Tue Nov 24, 2020 2:15 am |
|
|
'How things are declared', is vital for this. There are also things that 'worry'.
The poster is using a double cast. Now assuming 'Temp_Raw' is possibly
a unsigned int16, casting it to signed, will _not_ 'sign extend' this into a
signed type. I'm not sure double casting is actually legal, and in fact the
default order of precedence for operators, is left to right, suggesting this
will convert the value to float, and then to signed int16!...
No, checking, the associativity or casts is right to left, so it should do the
conversion to signed int16 first, but still has the problem that this
will not sign extend the value received from the sensor. It also is a
complete waste of time (suspect the optimiser will actually get rid of
it and just convert the int16 directly to a float.
Now the point about signed conversion, is that if you have a value in a
unsigned int16, like perhaps 0xC040, and cast this to a signed int16, this
will not give you a -ve result. Instead the value will result in an overflow.
In fact if the optimiser does remove the double cast, the result of casting
this into the float will be 49216, not the negative number wanted....
A quick test shows this is what happens.
To convert a binary signed value into a signed int16, move it with a union,
not a cast.
So:
Code: |
union {
unsigned int16 uword;
signed int16 sword;
} converter;
converter.uword=make16(OW_Buffer[1],OW_Buffer[0]);
Result=converter.sword/16.0;
|
Which will then put into 'result', the result of converting the binary
16bit value from the make16, into a signed int16, and dividing this by
16. |
|
![](templates/subSilver/images/spacer.gif) |
temtronic
Joined: 01 Jul 2010 Posts: 9168 Location: Greensville,Ontario
|
|
Posted: Tue Nov 24, 2020 6:13 am |
|
|
I'm thinking 'something' happened in the newer compiler version as it worked before 'sometime'.
I have to admit that with any 'complicated' math, I break it down into several lines of code,making it easier for my old eyes to see and follow.
I use 'scaled integers' for that sensor instead of floating point. It was a 'mind exercise' and even though it looks 'ugly',it actually produces faster,smaller code. I also convert *C to *F in 3 lines while real programmers would do it in one.
As I've said before I'm not a trained C programmer, learned it by 'osmosis' while playing with PICs.
Jay |
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19358
|
|
Posted: Tue Nov 24, 2020 8:22 am |
|
|
Yes, they changed how sign extension was handled a few versions ago.
Whether a unsigned value, that would actually represent a -ve number
gets turned into a -ve when cast into a signed value is 'implementation
defined' in C, so it is always safer to not rely on doing this.... |
|
![](templates/subSilver/images/spacer.gif) |
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Tue Nov 24, 2020 10:26 am |
|
|
These are the original declarations, which I'm still using..
Code: | int DS_Reading_Counter=0;
float DS_Raw_Temperatures[9];
int OW_Buffer[OW_BUFFER_SIZE];
int OW_Buffer_Index=0;
int16 Temp_Raw=0; |
Based on the comments, I've updated code to this.
Code: | Temp_Raw=make16(OW_Buffer[1],OW_Buffer[0]);
DS_Raw_Temperatures[DS_Reading_Counter]=(float)Temp_Raw/16.0; |
For sample values:
https://datasheets.maximintegrated.com/en/ds/DS18B20.pdf
See TABLE 1
Just for reference, the line originally posted has been in use for about 5 years, no issues, multiple ds18b20s, even in 1-wire networks, in temps ranging from -25C to +15C... different compiler and chip.
I've "Recently" updated compiler to 5.093 to allow use of the 18F47Q43.
Thanks _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
![](templates/subSilver/images/spacer.gif) |
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Tue Nov 24, 2020 10:35 am |
|
|
Indeed negative numbers are not computing correctly. _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
![](templates/subSilver/images/spacer.gif) |
temtronic
Joined: 01 Jul 2010 Posts: 9168 Location: Greensville,Ontario
|
|
Posted: Tue Nov 24, 2020 10:37 am |
|
|
hmmm...
I wonder if the compiler defaults int16 to be UNSIGNED int16 ? If so does that change 'things'...?? |
|
![](templates/subSilver/images/spacer.gif) |
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Tue Nov 24, 2020 10:52 am |
|
|
I've implemented the solution with the union suggested by Ttelmah:
Code: | converter.uword=make16(OW_Buffer[1],OW_Buffer[0]);
DS_Raw_Temperatures[DS_Reading_Counter]=(float)converter.sword/16.0; |
All good now.
Thank you! _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
![](templates/subSilver/images/spacer.gif) |
Ttelmah
Joined: 11 Mar 2010 Posts: 19358
|
|
Posted: Tue Nov 24, 2020 11:32 am |
|
|
It may be worth saying, you don't actually need to cast sword to float. That
the division is by 16.0 (with the decimal) forces the maths to use
float and do an implicit cast. Generally I've found this to be slightly
more efficient in many cases than explicitly casting!... |
|
![](templates/subSilver/images/spacer.gif) |
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Tue Nov 24, 2020 3:15 pm |
|
|
Thank you all for your help! _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
![](templates/subSilver/images/spacer.gif) |
|