Set up reporting on AC attribute divisor and multiplier #348
Conversation
| divisor = self._divisor | ||
|
|
||
| value = float(value * multiplier) / divisor | ||
| if value < 100 and divisor > 1: | ||
| return round(value, self._decimals) |
There was a problem hiding this comment.
Should we remove rounding and let HA format this?
There was a problem hiding this comment.
Yeah, but this might be required to avoid getting some rly weird numbers like x.99999978 because of using float in the line above and dividing the number? I didn't check the definition for self._decimals, just briefly having a look at your PR here, so also just giving a random opinion on this.
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.ac_power_divisor.name, | ||
| config=REPORT_CONFIG_IMMEDIATE, | ||
| ), |
There was a problem hiding this comment.
I put these at the very beginning just in case the device's attribute reports are sent in the order of the reporting config. If this is the case, you may need to re-join the device to get this to work.
It looks like the original code shadowed `attrs`
puddly
force-pushed
the
puddly/ac-divisor-properties
branch
from
6d53bb8 to
a29cfdf
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #348 +/- ##
=======================================
Coverage 96.50% 96.50%
=======================================
Files 61 61
Lines 9439 9446 +7
=======================================
+ Hits 9109 9116 +7
Misses 330 330 ☔ View full report in Codecov by Sentry. |
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.ac_voltage_multiplier.name, | ||
| config=REPORT_CONFIG_IMMEDIATE, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.ac_voltage_divisor.name, | ||
| config=REPORT_CONFIG_IMMEDIATE, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.ac_current_multiplier.name, | ||
| config=REPORT_CONFIG_IMMEDIATE, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.ac_current_divisor.name, | ||
| config=REPORT_CONFIG_IMMEDIATE, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.ac_power_multiplier.name, | ||
| config=REPORT_CONFIG_IMMEDIATE, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.ac_power_divisor.name, | ||
| config=REPORT_CONFIG_IMMEDIATE, | ||
| ), |
There was a problem hiding this comment.
ZCL_INIT_ATTRS with True will populate cached. REPORT_CONFIG will populate uncached. So, the attributes will be read twice. They should be removed from ZCL_INIT_ATTRS at least. Relevant code:
zha/zha/zigbee/cluster_handlers/__init__.py
Lines 458 to 460 in fd6cf2f
Do note that, by default, we will always read six more attributes per smart plug on network startup now, as the attributes are in uncached. Is that what we want?
(Also somewhat related is a change in this PR: https://github.com/zigpy/zha/pull/254/files#diff-90a1515ba92d644591ac0c972b7ed0d9e53112a3cc9bc00d27c9804273b5d2d8R458-R461)
Should resolve zigpy/zha-device-handlers#3374
I'll test it out with a real plug when I get one.