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

Update hvacThermostat attributes + invalid value handling #5892

Merged
merged 4 commits into from
Jun 20, 2023

Conversation

tomasbedrich
Copy link
Sponsor Contributor

Please review per-commit.

  • a5be5d6 likely won't raise any significant discussion.
  • 32f3888 can potentially cause some backwards compatibility issues, if someone has an automation based on "expected wrong" value reported by their thermostat. On the other hand – localTemperature is so common (ZCL mandatory) that hopefully no/very-little devices returns invalid values.
  • ad5ceb7 should be fixed because 0 can easily be a valid value for these attributes and by converting invalid values to 0 we loose the information. The subject of discussion is if null is the right value to return in this case.

Your thoughts are welcome.

The most important change is NOT to return 0 in case invalid value is returned.

Second change is to detect invalid values differently: according to ZCL spec rev8, the valid range for most of the hvacThermostat attributes is 0x954d – 0x7fff. The lower bound translates to -27315 (-273.15 respectivelly). Invalid value for sint16 is 0x800. The change updates the threshold for invalid value detection from -250 to -273.15.
@tomasbedrich tomasbedrich changed the title Update hvac thermostat Update hvacThermostat attributes + invalid value handling Jun 18, 2023
@tomasbedrich tomasbedrich requested a review from Koenkk June 20, 2023 06:32
@tomasbedrich
Copy link
Sponsor Contributor Author

As part of some future refactor, I'd propose creating an abstraction layer on top of Zigbee type parsing – to handle the invalid cases on a single place.

@Koenkk Koenkk merged commit 13d7016 into Koenkk:master Jun 20, 2023
1 check passed
@Koenkk
Copy link
Owner

Koenkk commented Jun 20, 2023

Thanks!

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