JChristensen / MCP9808

Arduino Library for Microchip MCP9808 Maximum Accuracy Digital Temperature Sensor
Other
6 stars 1 forks source link

Register Conversion Routines #1

Closed jecottrell closed 3 years ago

jecottrell commented 8 years ago

It appears these lines don't do what is required to receive the correct values. Specifically the tAmbient and the tLower. Both are returning values that are obviously incorrect.

    tUpper    = ( ( regs[2] << 11 ) + ( regs[3] << 3 ) ) >> 3;
    tLower    = ( ( regs[4] << 11 ) + ( regs[5] << 3 ) ) >> 3;
    tCritical = ( ( regs[6] << 11 ) + ( regs[7] << 3 ) ) >> 3;
    tAmbient  = ( ( regs[8] << 11 ) + ( regs[9] << 3 ) ) >> 3;

As I was debugging, I tried Adafruit's snippet on the values return in regs[8] and regs[9] and received the correct value. So, I would assume the raw bytes returned from the MCP9808 are correct.

This is what I used to validate the raw bytes:

  uint16_t t = (regs[8] << 8) + (regs[9]);
  float temp = t & 0x0FFF;
  temp /=  16.0;
  if (t & 0x1000) temp -= 256;

Actually, it looks like the shifting left and then right to mask the 3 leftmost bits isn't working for some reason.

JChristensen commented 8 years ago

You are aware that this library deals with the ambient temperature in units of °C16, and the limit temperatures as °C4? See the first three paragraphs in the Introduction section in the ReadMe file.

jecottrell commented 8 years ago

I am.

But it appears that 1.6.8 isn't behaving the way it ought to?

uint8_t test_regs[2]; 
uint16_t test_tAmbient;

test_regs[0] = 0xFF;
test_regs[1] = 0xFF;

test_tAmbient  = ( ( test_regs[0] << 11 ) + ( test_regs[1] << 3 ) ) >> 3;
Serial.println(test_tAmbient, HEX);

test_tAmbient  = (uint16_t)( ( test_regs[0] << 11 ) + ( test_regs[1] << 3 ) ) >> 3;
Serial.println(test_tAmbient, HEX);

test_tAmbient  = ( ( (uint16_t)test_regs[0] << 11 ) + ( (uint16_t)test_regs[1] << 3 ) ) >> 3;
Serial.println(test_tAmbient, HEX);

Output:

FFFF
1FFF
FFFF

The library and example sketch, is returning a C temp of ~1052 for ~28C. The tUpper flag isn't being stripped by the shifting.

JChristensen commented 8 years ago

I'll have to download 1.6.8, the latest I have is 1.6.7. Which microcontroller are you using? Did it work for you under any earlier releases? I'll try it next chance I get.

jecottrell commented 8 years ago

I've got multiple versions of the IDE. The machine I'm testing on has 1.6.4, 1.6.5 and 1.6.8.

The very first time I tried the example, I believe it worked. But, the amount of fiddling I've done over the last two days, my memory could be off.

I'm using a ATSAMD21G18 with the "Adafruit Feather M0" board setting. I will try the shifting tests on an AVR part and report back as well.

I've got a thread at arduino.cc - programming going where there's some interesting discussion and I'm learning some of the underlying details.

BTW, thanks for writing and sharing the variety of comprehensive libraries. I tend to grab yours first when I need something.

jecottrell commented 8 years ago

AVR part behaves as you had intended.

SAMD21 core is the culprit?

JChristensen commented 8 years ago

Probably not the core per se but how the hardware or the ARM compiler handles left shifts. I don't have a SAMD board so I can't test it. I'd need to research it, but I believe the propagation of the sign bit for right-shift is "implementation specific" according to the C++ standard. In your test code above, the second of the three results is 0x1FFF because the sign bit is not propagated for unsigned integers (in the library, tAmbient is a signed 16 bit integer). Even if the sign were not being propagated, unless the temperature is negative, it should work. So something else must be going on. The four assignment statements in the library may need some casts to work properly on an ARM. For the AVR, the default type is int16_t but for the ARM, it's probably int32_t. So on the ARM, the intermediate results in those assignment statements may not be the data type I intended. Glad you like my libraries. They're pretty much just stuff I've written for myself but I publish them in case they might help someone else. Unfortunately, I've heard of a few situations like this where they don't work properly with the ARM chips. I haven't had time (or a reason, really) to get into the ARM chips so I haven't much been able to discover and address these sorts of issues. If it were me, I'd fiddle with some test code on the AVR and on the ARM; I imagine the issue would become apparent fairly quickly.

jecottrell commented 8 years ago

That did it! A simple cast of the correct type cleaned it up. It must be the default int32_t of the ARM hardware.

The unsigned int here was the wrong approach... but it kept me occupied for a while.

test_tAmbient  = (uint16_t)( ( test_regs[0] << 11 ) + ( test_regs[1] << 3 ) ) >> 3;
Serial.println(test_tAmbient, HEX);

This appears to have solved the issue.

test_tAmbient  = (int16_t)( ( test_regs[0] << 11 ) + ( test_regs[1] << 3 ) ) >> 3;
Serial.println(test_tAmbient, HEX);

If you've got a minute, can you explain how you handle the datasheet condition of, if the value is less than zero, subtract it from 256?

I fiddled with the hex values and the C*16. I can see it would actually be 0x1000 - value, prior to the divide by 16. Not having worked with these types of numbers enough, I can't see how all of the conditions of the datasheet calculation are getting covered.

Thanks again,

John

JChristensen commented 8 years ago

Hi John,

Great! I was thinking both halves needed to be cast, e.g. test_tAmbient = ( ( (int16_t)test_regs[0] << 11 ) + ( (int16_t)test_regs[1] << 3 ) ) >> 3; but maybe not. I might try both and compare the results to ensure they're identical.

The way they wrote the datasheet is confusing. It's really just a simple two's-complement number, which they do say. I guess what they are trying to do in the sample code is to end up with a positive number either way, but then I would want to track the sign bit separately, which they do not do. And there is another error, they subtract from 256 (28, see the Wikipedia article), but the number is not an 8-bit value at that point, it must be larger to hold all 12 bits of the temperature data. So assuming a 16-bit data type, the right value to subtract from is 216 or 65535. I don't know who writes these datasheets. I have to think that code was never tested.

Bottom line, since it is a two's-complement number, all that is needed is to combine the two bytes and shift things to align properly. For my money, an ordinary signed int is just fine!

HTH ... Jack