search

marcelbuesing / bme680

Pure Rust BME680 implementation
MIT License
29 stars 27 forks source link

Feature Request: Add setting to offset calibrate the temperature #11

Closed nickbroon closed 5 years ago

nickbroon commented 5 years ago

The python library provides this (https://github.com/pimoroni/bme680-python/pull/13) based on details gleamed from the Bosch drivers (https://github.com/pimoroni/bme680-python/issues/11) which also allows to set a temperature offset as the device is not factory calibrated.

nickbroon commented 5 years ago

I thought about adding a t_fine member to the CalibData struct, and modifying that with an offset (basically just doing the same as python code does in the previous mentioned links), but I've noticed in the commit below, that such a member previously existed but was removed and instead t_fine was explicitly passed to temp/pressure/humidity function instead. Was there reason for removing t_fine from the CalibData struct? Would be acceptable to add it back, and then modify it with new adding temp_offset setting?

https://github.com/marcelbuesing/bme680/commit/6b6e19b9752c9f95350c8fdcc191771ccd41d1fd#diff-b4aea3e418ccdb71239b96952d9cddb6L229

marcelbuesing commented 5 years ago

Hi nick, sorry for the slow the response. I'll try to check this in the next days, thanks for digging in the commits. It does sound like a good suggestion though.

marcelbuesing commented 5 years ago

Ok I just added it to the SettingsBuilder as with_temperature_offset(i32). After looking at the code again the reason why I initially moved out t_fine was that I felt that the this enforces the order of function execution.

Temperature measurement can be enabled or skipped. Skipping the measurement is typically not recommended since temperature information is used to compensate temperature influences in the other parameters. Datasheet

When looking at the calculation of the pressure and humidity it actually looks to me more like the temperature measurement must not be skipped.

E.g. in the C library the order of execution affect the results of L887/L903

What do you think about this and would you mind giving the code a try ? It's located in the offset branch.

nickbroon commented 5 years ago

I've had a look over https://github.com/marcelbuesing/bme680/compare/offset and this should work fine. I agree that it appears that order appears to be import in both the C and Python implementations with t_fine being an output from temperature calculation and an input for pressure/humidity. Storing it in calibration data can make appear as if pressure/humidity can be calculated independently from temperature, but your approach makes them explicitly dependant which I think is better.

marcelbuesing commented 5 years ago

Thank for looking at this! I've just released 0.4.0 containing the change.

marcelbuesing commented 5 years ago

Would you mind looking at the fix again? Sorry for this. It actually also fixes the else instead of else if case (offset > 0) that was corrected later in the python merge request.

marcelbuesing commented 5 years ago

Well I had to get creative, since there does not seem to be a copysign fn for i32, but I think this should do the job.