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

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

Closed
nickbroon opened this issue Dec 5, 2018 · 7 comments
Closed
Labels
enhancement New feature or request

Comments

@nickbroon
Copy link

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

@marcelbuesing marcelbuesing added the enhancement New feature or request label Dec 5, 2018
@nickbroon
Copy link
Author

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?

6b6e19b#diff-b4aea3e418ccdb71239b96952d9cddb6L229

@marcelbuesing
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@marcelbuesing
Copy link
Owner

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 added a commit that referenced this issue Dec 21, 2018
@marcelbuesing
Copy link
Owner

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.

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

No branches or pull requests

2 participants