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

Initial support for GVH5101/5012 and enum GVH5072 dev type #19

Merged
merged 3 commits into from
Jan 15, 2022

Conversation

zrhutto
Copy link
Contributor

@zrhutto zrhutto commented Jan 14, 2022

Apologies for the separate pull request - GitHub SNAFU. This is a revised commit based on #13

@zrhutto
Copy link
Contributor Author

zrhutto commented Jan 14, 2022

I’m a little wary of forcing users to specify their device type (e.g., GVH5101 v. 5102 v. 5075 v. 5072), given that we have the logic to distinguish the model and parse the advertisement packet appropriately (feels like an unnecessary extra step), but perhaps that’s an improvement to make with a future refactoring.

@tony-fav
Copy link
Owner

That change doesn't force them to specify that model, just gives them the option if they want their user_config to be very specific. It's exposing user options that have no real effect essentially.

@tony-fav
Copy link
Owner

tony-fav commented Jan 14, 2022

What's up with changing the humidity calculation? By my memory, it was a requirement to do that kind of shift when dealing with negative temperature values. output_map['Humidity'] = ((basenum - 0x800000) % 1000)/10.0

An alternate could be to calculate humidity as output_map['Humidity'] = ((basenum & 0x7FFFFF) % 1000)/10.0 This should work as a one liner for negative and positive values of temp.

@tony-fav
Copy link
Owner

Last but probably not last, do you have an example you could add to the spoof_devices.be?

@zrhutto
Copy link
Contributor Author

zrhutto commented Jan 14, 2022

Good catch on the humidity calculation. Chalk that up to hastiness (and the fact that the sensor seemed to still be reporting sane values…).

Sample packet also added to spoof_devices.be. Let me know if anything else stands out.

@tony-fav tony-fav merged commit 692567f into tony-fav:main Jan 15, 2022
@tony-fav
Copy link
Owner

Looks good! Thanks for the contribution!

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.

None yet

2 participants