-
Notifications
You must be signed in to change notification settings - Fork 688
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
Add Owon PC321 3-phase energy meter support #3274
base: dev
Are you sure you want to change the base?
Conversation
Another attempt to add Owon PC321 3-phase meter support to ZHA. The meter reports couple of standart and a list of non-standard attributes to Metering cluster. This quirk maps all of them to OwonManufacturerSpecific cluster, and also only supported ones to ElectricalMeasurement cluster. Since not all sensors are available in Metering and ElectricalMeasurement clusters for reported attributes (e.g. phase-specific consumption figures), the missing ones will be supported later on in HACS plugin.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #3274 +/- ##
==========================================
- Coverage 88.18% 87.60% -0.59%
==========================================
Files 301 303 +2
Lines 9412 9680 +268
==========================================
+ Hits 8300 8480 +180
- Misses 1112 1200 +88 ☔ View full report in Codecov by Sentry. |
def __init__(self, *args, **kwargs): | ||
"""Init.""" | ||
self._current_state = {} | ||
super().__init__(*args, **kwargs) |
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.
_current_state
doesn't seem to be used. If so, this can be removed completely.
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.
The Bus
stuff isn't needed anymore. You can directly use something like this
self.endpoint.device.endpoints[1].smartenergy_metering.update_attribute(...)
to update attribute cace on another endpoint.
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.
Also, thanks for the PR, of course.
I think the solution in #2602 might be preferable.
However, it should be converted to a v2 quirk and add custom sensors for the custom attributes (using quirks v2 "exposes").
The entities would also have proper default names in HA then.
(Outdated docs draft PR, but still mostly valid for v2 quirks: #3019)
@TheJulianJES Could you please clarify if v2 quirks are already supported? I mean shall I update the current PR with both v1 and v2 quirks or v2 only? |
Another attempt to add Owon PC321 3-phase meter support to ZHA. The meter reports a couple of the standard and a list of non-standard attributes to the Metering cluster.
Proposed change
The proposed quirk maps all reported attributes to OwonManufacturerSpecific custom cluster, and also only supported ones to Metering and ElectricalMeasurement clusters (a separate set of clusters per each of 3 phase-specific endpoints + totals in the main Metering endpoint).
Additional information
Since not all sensors are available in Metering and ElectricalMeasurement clusters for reported attributes, the missing ones will be supported later on in HACS plugin, or alternatively - if corresponding clusters eventually get a full set of sensors natively. But as for now, most users will probably be fine as it is (see screenshot attached)
Fixes #977 #1657
Also, see #2602 for the same device but with different approach (never merged)
Checklist
pre-commit
checks pass / the code has been formatted using Black