CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

values not calculating correctly - CLOSED

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
Gabriel



Joined: 03 Aug 2009
Posts: 1067
Location: Panama

View user's profile Send private message

values not calculating correctly - CLOSED
PostPosted: Mon Nov 23, 2020 4:55 pm     Reply with quote

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
PCM programmer



Joined: 06 Sep 2003
Posts: 21708

View user's profile Send private message

PostPosted: Mon Nov 23, 2020 6:43 pm     Reply with quote

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.
Ttelmah



Joined: 11 Mar 2010
Posts: 19215

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 2:15 am     Reply with quote

'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.
temtronic



Joined: 01 Jul 2010
Posts: 9097
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 6:13 am     Reply with quote

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
Ttelmah



Joined: 11 Mar 2010
Posts: 19215

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 8:22 am     Reply with quote

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....
Gabriel



Joined: 03 Aug 2009
Posts: 1067
Location: Panama

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 10:26 am     Reply with quote

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
Gabriel



Joined: 03 Aug 2009
Posts: 1067
Location: Panama

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 10:35 am     Reply with quote

Indeed negative numbers are not computing correctly.
_________________
CCS PCM 5.078 & CCS PCH 5.093
temtronic



Joined: 01 Jul 2010
Posts: 9097
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 10:37 am     Reply with quote

hmmm...
I wonder if the compiler defaults int16 to be UNSIGNED int16 ? If so does that change 'things'...??
Gabriel



Joined: 03 Aug 2009
Posts: 1067
Location: Panama

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 10:52 am     Reply with quote

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
Ttelmah



Joined: 11 Mar 2010
Posts: 19215

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 11:32 am     Reply with quote

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!...
Gabriel



Joined: 03 Aug 2009
Posts: 1067
Location: Panama

View user's profile Send private message

PostPosted: Tue Nov 24, 2020 3:15 pm     Reply with quote

Thank you all for your help!
_________________
CCS PCM 5.078 & CCS PCH 5.093
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group