-
Notifications
You must be signed in to change notification settings - Fork 738
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 Tuya RCBO quirk power factor calculation for home-assistant/core#107641 #2896
Update Tuya RCBO quirk power factor calculation for home-assistant/core#107641 #2896
Conversation
04c1ec9
to
6c7cfe2
Compare
The Tuya quirks tests probably need to be corrected to expect the new value then. See:
(and ideally, the HA PR needs to have tests for the added code) |
Amend power_factor calculation to respect Zigbee cluster specification - required for release of zha fix: home-assistant/core#107641
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2896 +/- ##
==========================================
+ Coverage 87.58% 87.60% +0.02%
==========================================
Files 294 295 +1
Lines 9010 9030 +20
==========================================
+ Hits 7891 7911 +20
Misses 1119 1119 ☔ View full report in Codecov by Sentry. |
2180278
to
3c44240
Compare
@TheJulianJES once you are good with this you can merge this and the corresponding HA PR: home-assistant/core#107641 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
…re#107641 (zigpy#2896) Amend power_factor calculation to respect Zigbee cluster specification - required for release of ZHA fix: home-assistant/core#107641
Proposed change
Amend power_factor calculation to respect Zigbee cluster specification - required for release of zha fix:
home-assistant/core#107641
Additional information
This updates the power factor calculation to multiply by 100 rather than 1000, this is because the ElectricalMeasurement attribute is supposed to expose values between -100 and 100 (see the pre-requisite PR 107641 for details).
The current code exposes values between 0 and 1000, which is subsequently divided by 10 due to the behaviour of the zha cluster sensor corrected in home-assistant/core#107641
If released without home-assistant/core#107641 the value reported will be an order of magnitude too low.
Checklist
pre-commit
checks pass / the code has been formatted using Black