Add remaining 3 phase electrical attributes #339
Conversation
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.rms_current_ph_b.name, | ||
| config=REPORT_CONFIG_OP, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.rms_current_ph_c.name, | ||
| config=REPORT_CONFIG_OP, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.rms_current_max.name, | ||
| config=REPORT_CONFIG_DEFAULT, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.rms_current_max_ph_b.name, | ||
| config=REPORT_CONFIG_DEFAULT, | ||
| ), | ||
| AttrReportConfig( | ||
| attr=ElectricalMeasurement.AttributeDefs.rms_current_max_ph_c.name, | ||
| config=REPORT_CONFIG_DEFAULT, | ||
| ), |
There was a problem hiding this comment.
The not so great thing about this is that we'll add another four attributes to poll per device on each startup.
With #324, that makes it eight attributes that are polled (per device on startup).
That isn't ideal, so we might want to look at only polling the attributes here if they're not marked as unsupported already. Unsupported attributes might change with firmware updates, but otherwise shouldn't (hopefully).
There was a problem hiding this comment.
Ah never mind, this PR includes the changes from the other one. So it's only four additional attributes (added in the previous PR).
There was a problem hiding this comment.
But we'd also need to set up attribute reporting for power, power factor, and voltage here, right?
That would make a lot of attributes that are read on each startup.
Like mentioned above, we might want to consider only polling the attributes if they're not marked as unsupported already OR allow setting up attribute reports without also polling them on each startup?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #339 +/- ##
==========================================
- Coverage 96.50% 96.46% -0.04%
==========================================
Files 61 61
Lines 9431 9504 +73
==========================================
+ Hits 9101 9168 +67
- Misses 330 336 +6 ☔ View full report in Codecov by Sentry. |
Follow up to #324
Related HA PR: home-assistant/core#133969