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

Exposing the raw temperature and humidity? #17

Closed
jgillula opened this issue Aug 4, 2021 · 6 comments
Closed

Exposing the raw temperature and humidity? #17

jgillula opened this issue Aug 4, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@jgillula
Copy link
Contributor

jgillula commented Aug 4, 2021

Hi there! I'm using your awesome library for a low-power wireless temperature sensor I'm building.

To save power on my wireless node, I want to transmit the raw two-byte values for temperature and humidity instead of the four-byte floats. Would you be open to a PR that:

  • Saves those raw values and exposes them
  • Moves the conversion to actual floating point values to the getTemperature() and getHumidity() functions?
  • Optionally: create a pair of boolean flags _needToRecomputeTemp/_needToRecomputeHumidity that are set to True when the raw values are read in readData(bool). Then, every time getTemperature() or getHumidity() check those flags and if they're true, then recompute the value based off the raw two-byte values, store it in the appropriate variable, and set the flag to false, so that if getTemperature() or getHumidity() is called again, we skip redundant recomputation and just return the cached value.

WDYT?

If this is all too complicated for you and you'd rather not accept a PR like this, no worries--I can always fork the library for my own purposes. I just thought since I was going to do the work anyway, I'd offer to share it back upstream.

@RobTillaart
Copy link
Owner

RobTillaart commented Aug 5, 2021

Hi,

Thanks for your proposal, I will go through your points with my (first) thoughts.

Saves those raw values and exposes them

Given your rationale to transmit these - saving 50% - I think it is a good idea to do that. Names could be

  • rawHumidity
  • rawTemperature
    I prefer accessor functions e.g. getRawHumidity() getRawTemperature() around to prevent writing to them.
    These will be optimized by the compiler anyway but makes the variables effectively read-only preventing errors.
    The extra storage is 4 bytes, which is no problem imho.

Moves the conversion to actual floating point values to the getTemperature() and getHumidity() functions?

The rationale to have the math direct after read, is to do it only once per read. It allows one to access temperature / humidity more than once without redoing the math. So there is little need to change this sec. However in combination with the "raw" change it definitely makes sense (as the 2 floats to hold the temp/hum are replaced by 2 ints to hold their raw value).

Optionally: create a pair of boolean flags _needToRecomputeTemp/_needToRecomputeHumidity.

Think this is fixing a problem "created by design changes above", yes I understand 100% why one could do this.
However it is far easier to tell the user that if he wants to speed things up, he should cache the value himself.
That would also keep the code and memory footprint smaller, so in short a note in the readme.md file should be sufficient.

Please update the version number to 0.3.2 in your PR.

Any questions, let me know / or discuss in PR
Rob

Note: https://github.com/RobTillaart/SHT85 will be updated later - I try to keep these "cousins" a bit in sync.

@jgillula jgillula mentioned this issue Aug 5, 2021
@RobTillaart
Copy link
Owner

0.3.2 released,

@RobTillaart RobTillaart added the enhancement New feature or request label Aug 7, 2021
@RobTillaart
Copy link
Owner

@jgillula
FYI, added the changes to https://github.com/RobTillaart/SHT85 library
thanks

@jgillula
Copy link
Contributor Author

jgillula commented Aug 7, 2021

Thank you so much!

@jgillula
Copy link
Contributor Author

jgillula commented Aug 7, 2021

BTW do you have a way I can toss some cash your way for making the most useful SHT-3X library I've found? :-D

@RobTillaart
Copy link
Owner

Thanks for the compliment, you helped to improve it.
If there are other issues or you need a "better" library for some other device let me know.

I do not have a sponsor program in place (yet) so please donate to a charity of your choice.

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