Zigpy 0.91.0: attribute events, manufacturer codes, and signature fixes#605
Zigpy 0.91.0: attribute events, manufacturer codes, and signature fixes#605
Conversation
puddly
force-pushed
the
puddly/zigpy-attribute-rewrite
branch
from
dad8c6f to
61aaf60
Compare
TheJulianJES
left a comment
TheJulianJES
left a comment
There was a problem hiding this comment.
This looks good. Only the comment about emitting ZHA events for LevelControl changes should be addressed IMO, as it'll get spammy otherwise. That part needs to be reworked a bit in the future anyway.
Otherwise, why do the diagnostics change?
| | AttributeUpdatedEvent | ||
| | AttributeWrittenEvent, | ||
| ) -> None: | ||
| """Handle an attribute updated on this cluster.""" |
There was a problem hiding this comment.
Side note: Could be nice to add some sort of EMIT_EVENTS attribute for ClusterHandlers in general, so we don't need this empty override, just to block emitting ZHA events.
Could just set EMIT_EVENTS = False here.
| async def _get_attributes( | ||
| self, | ||
| raise_exceptions: bool, | ||
| attributes: list[str], | ||
| from_cache: bool = True, | ||
| only_cache: bool = True, | ||
| ) -> dict[int | str, Any]: | ||
| """Get the values for a list of attributes.""" | ||
| manufacturer = None | ||
| manufacturer_code = self._endpoint.device.manufacturer_code | ||
| if self.cluster.cluster_id >= 0xFC00 and manufacturer_code: | ||
| manufacturer = manufacturer_code | ||
| chunk = attributes[:CLUSTER_READS_PER_REQ] | ||
| rest = attributes[CLUSTER_READS_PER_REQ:] | ||
| result = {} | ||
| while chunk: | ||
| try: | ||
| self.debug("Reading attributes in chunks: %s", chunk) | ||
| read, _ = await RETRYABLE_REQUEST_DECORATOR( | ||
| self.cluster.read_attributes | ||
| )( | ||
| chunk, | ||
| allow_cache=from_cache, | ||
| only_cache=only_cache, | ||
| manufacturer=manufacturer, | ||
| ) | ||
| self.debug("Got attributes: %s", read) | ||
| result.update(read) | ||
| except (TimeoutError, zigpy.exceptions.ZigbeeException) as ex: | ||
| self.debug( | ||
| "failed to get attributes '%s' on '%s' cluster: %s", | ||
| chunk, | ||
| self.cluster.ep_attribute, | ||
| str(ex), | ||
| ) | ||
| if raise_exceptions: | ||
| raise | ||
| chunk = rest[:CLUSTER_READS_PER_REQ] | ||
| rest = rest[CLUSTER_READS_PER_REQ:] | ||
| return result |
There was a problem hiding this comment.
Side note: We should also update the batched get_attributes when initializing a cluster handler to properly handle manufacturer codes based on attribute definitions. The code is really flawed right now.
configure_reporting should likely be adjusted as well (and likely everything where we do the self.cluster.cluster_id >= 0xFC00 and manufacturer_code check).
We can also do that in a separate PR.
puddly
force-pushed
the
puddly/zigpy-attribute-rewrite
branch
from
4ade322 to
b8d89f1
Compare
puddly
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #605 +/- ##
==========================================
- Coverage 97.17% 97.13% -0.04%
==========================================
Files 62 62
Lines 10720 10710 -10
==========================================
- Hits 10417 10403 -14
- Misses 303 307 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I tracked down why the new entities were being created for some of these Tuya devices and it's a diagnostics loading bug triggered by zigpy changing With that, I think there's no unexpected diagnostics JSON changes with this PR. |
2 participants
ZHA fixes for zigpy/zigpy#1719.
A few tests needed tweaking because of attribute "collisions" but otherwise nothing is breaking as far as I can tell and a few new entities are showing up for existing test devices.
The most visible change is switching from
attribute_updatedto explicit events for every possible attribute change type.