-
Notifications
You must be signed in to change notification settings - Fork 902
Fix energy and voltage for Aqara T2 relay #4458
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
base: dev
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #4458 +/- ##
=======================================
Coverage 92.24% 92.24%
=======================================
Files 369 369
Lines 12088 12090 +2
=======================================
+ Hits 11150 11152 +2
Misses 938 938 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| class T2MeteringCluster(MeteringCluster): | ||
| """T2 Metering cluster to fix divisor.""" | ||
|
|
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.
Would this work? It should avoid requiring any runtime-testable code:
_CONSTANT_ATTRIBUTES = {
MeteringCluster.AttributeDefs.divisor.id: 1000,
}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.
Yes! But its 1000000 instead since 1000 is already what we have by default for xiaomi: https://github.com/zigpy/zha-device-handlers/blob/dev/zhaquirks/xiaomi/__init__.py#L712
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.
Ah. In that case you'd need to do _CONSTANT_ATTRIBUTES = {**MeteringCluster._CONSTANT_ATTRIBUTES, MeteringCluster.AttributeDefs.divisor.id: 1000000,}.
Is the device really sending reports scaled this much? Or are we hiding a * 1000 somewhere?
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.
Ah. In that case you'd need to do
_CONSTANT_ATTRIBUTES = {**MeteringCluster._CONSTANT_ATTRIBUTES, MeteringCluster.AttributeDefs.divisor.id: 1000000,}.
Oh, duh! This is a sign that I should probably go to sleep... 😆
Is the device really sending reports scaled this much? Or are we hiding a
* 1000somewhere?
We scale it here: https://github.com/abmantis/zha-device-handlers/blob/xiaomi_t2_fix_energy/zhaquirks/xiaomi/__init__.py#L
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.
Z2M also has a special case for this device:
https://github.com/Koenkk/zigbee-herdsman-converters/blob/ea780c92873ad254818f9bb7060f9417879893d6/src/lib/lumi.ts#L503-L507
The standard 1000 divisor is because of us needing to do the trip through zigpy cache/ZCL datatypes: https://github.com/abmantis/zha-device-handlers/blob/3916d9b9efa74e35a745c1c3d88cccb1645e86f2/zhaquirks/xiaomi/__init__.py#L329-L333
Z2M doesn't need to do this, but we'd just cut off all decimal points otherwise.
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!
|
Energy and voltage just didn't display for this device before at all, right? |
Power was already displayed and updated more frequently than the custom xiaomi attribute updates. |
Proposed change
SSIA.
Additional information
Device diagnostics
Checklist
pre-commitchecks pass / the code has been formatted using Black