From 89a3c304c2b9216510600ed66e2a064fb161e76d Mon Sep 17 00:00:00 2001 From: TheJulianJES Date: Tue, 28 Mar 2023 18:39:10 +0200 Subject: [PATCH] Refactor ZHA binary sensors to read from zigpy cache (#89481) * Construct binary sensor state from zigpy cache (WIP) * Workaround zha-quirks issue where "MotionWithReset" quirks don't update attribute cache (WIP) zha-quirks currently has an issue where the ZONE_STATE attribute is updated (when the zone_STATUS changes). https://github.com/zigpy/zha-device-handlers/pull/2231 is a proper fix for this. For now, we just update the attribute cache when we get the "zone status update notification" command. This wasn't noticed before, as the "attribute report signal" was sent from the `cluster_command()` method and the used the provided attribute (in the signal) to update the `_state` value in the binary sensor class. As we just tell HA to write state again when we get an attribute report now, the ZONE_STATUS attribute is read now (and needs to be correct). * Use parse() method of main class for IasZone entity (with stripped bits) * Change wording in comment, remove explicitly sending attr signal (This comment should be removed/changed later anyway) * Remove note * Get zone_status attribute id with zigpy * Remove `security.` prefix for `IasZone` import `AceCluster` was already directly imported and `IasZone` is too now for getting the attribute id * Store full zone status attribute in cache * Check that non-alarm bits are ignored in IasZone sensor test * Re-enable occupancy binary sensor test This test seems to work fine and I don't see any reason why it was commented out for a while * Fix cached read mix-up for `zone_status`/`zone_state` This allows cached reads for `zone_state` (enrolled or not), but forces a new read for `zone_status` (alarm or not). --- homeassistant/components/zha/binary_sensor.py | 37 ++++++------------- .../components/zha/core/channels/security.py | 20 +++++----- tests/components/zha/test_binary_sensor.py | 7 +++- 3 files changed, 28 insertions(+), 36 deletions(-) diff --git a/homeassistant/components/zha/binary_sensor.py b/homeassistant/components/zha/binary_sensor.py index b6a0af8e459..9c2fb49de61 100644 --- a/homeassistant/components/zha/binary_sensor.py +++ b/homeassistant/components/zha/binary_sensor.py @@ -8,7 +8,7 @@ from homeassistant.components.binary_sensor import ( BinarySensorEntity, ) from homeassistant.config_entries import ConfigEntry -from homeassistant.const import STATE_ON, Platform +from homeassistant.const import Platform from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddEntitiesCallback @@ -76,34 +76,23 @@ class BinarySensor(ZhaEntity, BinarySensorEntity): self._channel, SIGNAL_ATTR_UPDATED, self.async_set_state ) - @callback - def async_restore_last_state(self, last_state): - """Restore previous state.""" - super().async_restore_last_state(last_state) - self._state = last_state.state == STATE_ON - @property def is_on(self) -> bool: """Return True if the switch is on based on the state machine.""" - if self._state is None: + raw_state = self._channel.cluster.get(self.SENSOR_ATTR) + if raw_state is None: return False - return self._state + return self.parse(raw_state) @callback def async_set_state(self, attr_id, attr_name, value): """Set the state.""" - if self.SENSOR_ATTR is None or attr_name != self.SENSOR_ATTR: - return - self._state = bool(value) self.async_write_ha_state() - async def async_update(self) -> None: - """Attempt to retrieve on off state from the binary sensor.""" - await super().async_update() - attribute = getattr(self._channel, "value_attribute", "on_off") - attr_value = await self._channel.get_attribute_value(attribute) - if attr_value is not None: - self._state = attr_value + @staticmethod + def parse(value: bool | int) -> bool: + """Parse the raw attribute into a bool state.""" + return bool(value) @MULTI_MATCH(channel_names=CHANNEL_ACCELEROMETER) @@ -167,12 +156,10 @@ class IASZone(BinarySensor): """Return device class from component DEVICE_CLASSES.""" return CLASS_MAPPING.get(self._channel.cluster.get("zone_type")) - async def async_update(self) -> None: - """Attempt to retrieve on off state from the binary sensor.""" - await super().async_update() - value = await self._channel.get_attribute_value("zone_status") - if value is not None: - self._state = value & 3 + @staticmethod + def parse(value: bool | int) -> bool: + """Parse the raw attribute into a bool state.""" + return BinarySensor.parse(value & 3) # use only bit 0 and 1 for alarm state @MULTI_MATCH( diff --git a/homeassistant/components/zha/core/channels/security.py b/homeassistant/components/zha/core/channels/security.py index b5a8d5d8cf5..404e4a8d258 100644 --- a/homeassistant/components/zha/core/channels/security.py +++ b/homeassistant/components/zha/core/channels/security.py @@ -11,7 +11,7 @@ from typing import TYPE_CHECKING, Any from zigpy.exceptions import ZigbeeException import zigpy.zcl from zigpy.zcl.clusters import security -from zigpy.zcl.clusters.security import IasAce as AceCluster +from zigpy.zcl.clusters.security import IasAce as AceCluster, IasZone from homeassistant.core import callback @@ -332,21 +332,22 @@ class IasWd(ZigbeeChannel): ) -@registries.ZIGBEE_CHANNEL_REGISTRY.register(security.IasZone.cluster_id) +@registries.ZIGBEE_CHANNEL_REGISTRY.register(IasZone.cluster_id) class IASZoneChannel(ZigbeeChannel): """Channel for the IASZone Zigbee cluster.""" - ZCL_INIT_ATTRS = {"zone_status": True, "zone_state": False, "zone_type": True} + ZCL_INIT_ATTRS = {"zone_status": False, "zone_state": True, "zone_type": True} @callback def cluster_command(self, tsn, command_id, args): """Handle commands received to this cluster.""" if command_id == 0: - state = args[0] & 3 - self.async_send_signal( - f"{self.unique_id}_{SIGNAL_ATTR_UPDATED}", 2, "zone_status", state + zone_status = args[0] + # update attribute cache with new zone status + self.cluster.update_attribute( + IasZone.attributes_by_name["zone_status"].id, zone_status ) - self.debug("Updated alarm state: %s", state) + self.debug("Updated alarm state: %s", zone_status) elif command_id == 1: self.debug("Enroll requested") res = self._cluster.enroll_response(0, 0) @@ -389,11 +390,10 @@ class IASZoneChannel(ZigbeeChannel): @callback def attribute_updated(self, attrid, value): """Handle attribute updates on this cluster.""" - if attrid == 2: - value = value & 3 + if attrid == IasZone.attributes_by_name["zone_status"].id: self.async_send_signal( f"{self.unique_id}_{SIGNAL_ATTR_UPDATED}", attrid, - self.cluster.attributes.get(attrid, [attrid])[0], + "zone_status", value, ) diff --git a/tests/components/zha/test_binary_sensor.py b/tests/components/zha/test_binary_sensor.py index 58264bf6664..d633e9173e7 100644 --- a/tests/components/zha/test_binary_sensor.py +++ b/tests/components/zha/test_binary_sensor.py @@ -75,12 +75,17 @@ async def async_test_iaszone_on_off(hass, cluster, entity_id): await hass.async_block_till_done() assert hass.states.get(entity_id).state == STATE_OFF + # check that binary sensor remains off when non-alarm bits change + cluster.listener_event("cluster_command", 1, 0, [0b1111111100]) + await hass.async_block_till_done() + assert hass.states.get(entity_id).state == STATE_OFF + @pytest.mark.parametrize( ("device", "on_off_test", "cluster_name", "reporting"), [ (DEVICE_IAS, async_test_iaszone_on_off, "ias_zone", (0,)), - # (DEVICE_OCCUPANCY, async_test_binary_sensor_on_off, "occupancy", (1,)), + (DEVICE_OCCUPANCY, async_test_binary_sensor_on_off, "occupancy", (1,)), ], ) async def test_binary_sensor(