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

Raw values #18

Merged
merged 5 commits into from
Aug 5, 2021
Merged

Raw values #18

merged 5 commits into from
Aug 5, 2021

Conversation

jgillula
Copy link
Contributor

@jgillula jgillula commented Aug 5, 2021

Per the comments in issue #17, here's a PR for exposing the raw two-byte humidity and temperature values.

I think I've followed all your suggestions, but if I messed up or misinterpreted, just let me know! I'm happy to fix it so it matches what you want. :-)

The only question I have is: since this is making a significant-yet-backwards-compatible change, maybe the new version should be 0.4.0 instead of 0.3.2? I'll defer to whatever you prefer--just thought I'd ask.

…tions of the temperature and humidity. Computation of the actual values now happens in `getHumidity()` and `getTemperature()`. Additionally, `getRawHumidity()` and `getRawTemperature()` will return the raw two-byte representations.

Bumped the version to 0.3.2.
@RobTillaart
Copy link
Owner

Did a quick review and it all looks pretty straightforward.
I started the continuous integration workflows, they do some additional checks on the code.
Will merge a.s.a.p.

Thanks

@jgillula
Copy link
Contributor Author

jgillula commented Aug 5, 2021

Looks like the tests failed since now the initial value for getTemperature() is -45, not 0. FWIW I couldn't make it return 0, since the two-byte representation can't actually represent 0 exactly (just -0.0023 and 0.0004).

BTW, do you want me to add some tests for the raw values? It didn't occur to me before, but I'd be happy to do it.

@RobTillaart
Copy link
Owner

RobTillaart commented Aug 5, 2021

You broke the unit tests - no big problem.
I will merge your code and fix the unit test.


update: fix is under test

@RobTillaart RobTillaart merged commit 4fa6b94 into RobTillaart:master Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants