finitespace / BME280

Provides an Arduino library for reading and interpreting Bosch BME280 data over I2C, SPI or Sw SPI.
GNU General Public License v3.0
212 stars 105 forks source link

H4/H5 are 12 bits signed integers #103

Closed AmedeeBulle closed 3 years ago

AmedeeBulle commented 5 years ago

Related issue # and issue behavior

12 bits signed integer are not handles properly, see #102

Description of changes/fixes

Propagate sign to 16 bit signed integer

@mention a person to review

Steps to test

Compare the following snippets of code:

    uint8_t byte = 0xf0;           // This is a negative value in an unsigned
    int16_t word = (byte << 4);    // Wrong way of propagating to 16bits signed

    Serial.print("Byte: ");
    Serial.println(byte, HEX);
    Serial.print("Word: ");
    Serial.println(word, HEX);

    word = ((int8_t)byte * 16);    // Better way of propagating to 16bits signed
    Serial.print("Word: ");
    Serial.println(word, HEX);

Any outstanding TODOs or known issues

finitespace commented 3 years ago

These calculations are given directly from the manufacture. I am hesitant to change them. They do some unconventional things. There is a possibility the behavior you are describing is expected.

AmedeeBulle commented 3 years ago

This is exactly my point -- the manufacturer reference code linked in #102 is handling these values as 12 bits signed integers; which mean this library differs from the manufacturer reference implementation...

finitespace commented 3 years ago

They were taken from the data sheet (I think, 5 years ago is a long time), but I can't seem to find the humidity equation now in the ds. I will take your word for it. Tbh, I don't have time to look into it properly right now.

Thanks for sticking with this. You have a pull request, right. Let me take a look.

finitespace commented 3 years ago

Oh gosh, it is too early and I haven't had enough coffee. THIS IS THE PULL REQUEST #facepalm

finitespace commented 3 years ago

Changes look fine to me. You have tested them, correct?

AmedeeBulle commented 3 years ago

I am running 10+ sensors with this change for more than 2 years and the measurements look good to me...