Zanduino / BME680

Arduino Library to access the Bosch BME680 - temperature, pressure, humidity and gas sensor
GNU General Public License v3.0
37 stars 10 forks source link

Where are the Gas calibration constants used? #26

Closed stefanrueger closed 3 years ago

stefanrueger commented 3 years ago

I've browsed through the source code and didn't see where the Gas calibration constants are used. This must be a bug. I recommend reviewing the code and comparing against the original published library from Bosch Sensortek.

Stefan

stefanrueger commented 3 years ago

OK, I know where the problem is: lines 481 and 482 of the .cpp file wongly use the humidity constants _H1 ... _H3 instead of the _G1 ... _G3 gas constants (this function sets the parameters for the gas measurement). The gas readings between different sensors will be inconsistent, and the temperature settings for the gas sensor hot plate quite wrong.

stefanrueger commented 3 years ago

There is another problem in the section of lines 410 to 414: (int32_t) 1 << 31 yields the smallest 32 bit signed integer; the comparison on line 411is therefore always true. The code looks like it wants to avoid computational 32-bit overflow. As a consequence the code always loses one bit resolution (rather than only in those cases where the numbers would overflow). Not a biggie. It looks like the code was originally written for 32 bit arithmetics, but that is at loggerheads with the variables actually being 64 bit here?

Again, I recommend comparing against the published reference implementation from Bosch. Perhaps a good idea to go carefully go through the full code?

stefanrueger commented 3 years ago

OK - I've just checked the reference implementation at https://github.com/BoschSensortec/BME680_driver

It's 1 << 30 (not 1 << 31); and yes, in that section of the code it's sufficient to use 32 bit integer variables.

stefanrueger commented 3 years ago

The variable status in readSensors() is computed, but not used.

Maybe it's a good idea to return status in readSensors(), so the caller can evaluate that? Status could also usefully contain a copy of the bit "new_data" from the meas_status register (which resides in buff[0]). Either that, or if status is ignored (and its computation code removed) one could then read only 13 bytes into buff+2 at a position BME680_STATUS_REGISTER+2 (not 15 bytes at position BME680_STATUS_REGISTER). Note that buff[0] and buff[1] are never used so the function readSensors() currently reads these two bytes from the sensor but never looks at them.

SV-Zanshin commented 3 years ago

Corrected the errors that you found in the source code regarding gas computations and pressure overflow calculations. Good catch.

SV-Zanshin commented 3 years ago

I've added an optional return value for [getSensorData()](https://github.com/SV-Zanshin/BME680/wiki/getSensorData())