Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Negative temperature values calculated incorrectly #2

Closed
cklam2 opened this issue Jan 20, 2015 · 6 comments
Closed

Negative temperature values calculated incorrectly #2

cklam2 opened this issue Jan 20, 2015 · 6 comments

Comments

@cklam2
Copy link

cklam2 commented Jan 20, 2015

Hi,

First of all thank you very much for this library.

I think the library shows incorrect temperature when the temp is negative. According the LM75 datasheets the most significant bit (D15) contains 1 when value is negative.

In your case the temp value should be subtracted by 0x100 to get the right (negative) value.

float LM75::regdata2floattemp (word regdata)
{
  float temp = ((float)(int)regdata / 32) / 8;
  if (regdata >> 15) temp -= 0x100;
  return temp;
}

Also think that D0..D6 (don't care bits) should be masked. Not sure though...

@thefekete
Copy link
Owner

Thanks! Unfortunately, I'm unable to test this for a while... Have you tested it? If so I'll patch..

@cklam2
Copy link
Author

cklam2 commented Jan 20, 2015

Don't know what to say.. but I tested your code with my Arduino and your library does calculate negative values correctly. I only have no idea why.

This is your code:

float LM75::regdata2float (word regdata)
{
  return ((float)(int)regdata / 32) / 8;
}

When passing regdata = 0xC920 then it outputs -54.88 C
That's correct but how can that be???

Anyway, no changes required. Sorry :(

@cklam2 cklam2 closed this as completed Jan 20, 2015
@thefekete
Copy link
Owner

Hey no worries.. It's been 4 years since I wrote this and 2 since I last looked at it.. And all I can say is this belongs on a slide in a presentation on how NOT to write code! It took me 10 minutes of staring at it before I figured it out too! I'll try to explain it for both of us ;)

First of all, let me re-write regdata2float so it shows the intention a little better. It also helps to look at
float2regdata, since it's just doing the inverse of that function (although maybe not, that one's pretty bad too :)

word LM75::float2regdata (float temp)
{
  // First multiply by 8 and coerce to integer to get +/- whole numbers
  // Then coerce to word and bitshift 5 to fill out MSB
  return (word)((int)(temp * 8) << 5);
}

float LM75::regdata2float (uint16_t regdata)
{
    /* regdata comes in as a 16bit word that has a bunch of 'don't care bits' in the
     * 7 LSBs. It's basically a signed integer representing +/- degrees in the upper
     * byte and 256ths in the lower byte. So first it casts regdata as an (signed)
     * integer, converting it to +/-. Then to a float, allowing us to divide and get the
     * decimals. After that, it's just a matter of "shifting" the lower byte to the other
     * side of the decimal point by dividing by 256 (not sure why I divided by 32 then
     * 8, but same result).
     */ 
    return ( (float)( (int16_t)regdata)) / 256;
}

Of course this is super convoluted and very hard to follow code... But I think the important parts are casting to an int gives you the +/- and then dividing by 256 "shifts" it down so that the units column is whole degrees. It might be possible to right shift by 8 instead, but I'm not sure how that works for floats..

Anyways, not sure if you wanted an explaination, but I figured I'd write one for you as much as myself if I ever need to use this again ;)

Hope that helps and thanks for asking the question, I know I learned something!

@cklam2
Copy link
Author

cklam2 commented Jan 21, 2015

Thanks for the explanation.

I also tried to figure out how it works. What I didn't know was that on Arduino an int is a 16-bit value. I've been working on 32-bit systems where int was always 32-bit.
By converting a word to an int would mark the value negative if the MSB is 1.

Reason who you divide by 32 and then by 8 is because you first eliminate the 5 don't care LSB's and then divide by 8 as the lowest temp resolution is 1/8th = 0.125C

@vanbwodonk
Copy link

Hi, i used this library with STM32duino. This library works perfectly, except can't show negative value. After some research i know it because casts to (int) variable in AVR/Arduino use 16 bit. But int the ARM it use 32 bit. I need to changed into int16_t and this patch works under ARM and AVR/Arduino. Need some little patch this code:

word LM75::float2regdata (float temp)
{
  // First multiply by 8 and coerce to integer to get +/- whole numbers
  // Then coerce to word and bitshift 5 to fill out MSB
  return (word)((int)(temp * 8) << 5);
}

float LM75::regdata2float (word regdata)
{
  return ((float)(int)regdata / 32) / 8;
}

to this,

uint16_t LM75::float2regdata (float temp)
{
  // First multiply by 8 and coerce to integer to get +/- whole numbers
  // Then coerce to word and bitshift 5 to fill out MSB
  return (uint16_t)((int16_t)(temp * 8) << 5);
}

float LM75::regdata2float (word regdata)
{
  return ((float)(int16_t)regdata / 32) / 8;
}

@geresy
Copy link

geresy commented Dec 2, 2020

Indeed on SAMD the values rollover when the temperature is negative. Changing to uint16_t as exemplified above has fixed the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants