Debounce group entity update when member state changes (#74)

* Debounce group entity update when member state changes

* handle timing difference when assume group state is True for lights

* cleanup

* wrap with callback

Author: dmulcahey

tests/conftest.py

@@ -295,7 +295,7 @@ def zha_data() -> ZHAData:
             ),
             light_options=LightOptions(
                 enable_enhanced_light_transition=True,
-                group_members_assume_state=False,
+                group_members_assume_state=True,
             ),
             alarm_control_panel_options=AlarmControlPanelOptions(
                 arm_requires_code=False,

tests/test_fan.py

@@ -2,6 +2,7 @@
 
 # pylint: disable=redefined-outer-name
 
+import asyncio
 from collections.abc import Awaitable, Callable
 import logging
 from typing import Optional
@@ -255,6 +256,7 @@ async def async_set_preset_mode(
     "zigpy.zcl.clusters.hvac.Fan.write_attributes",
     new=AsyncMock(return_value=zcl_f.WriteAttributesResponse.deserialize(b"\x00")[0]),
 )
+@pytest.mark.looptime
 async def test_zha_group_fan_entity(
     device_fan_1: Device, device_fan_2: Device, zha_gateway: Gateway
 ):
@@ -338,12 +340,25 @@ async def test_zha_group_fan_entity(
     await send_attributes_report(zha_gateway, dev2_fan_cluster, {0: 2})
     await zha_gateway.async_block_till_done()
 
-    # test that group fan is speed medium
+    # no update yet because of debouncing
+    assert entity.state["is_on"] is False
+
+    # member updates are debounced for .5s
+    await asyncio.sleep(1)
+    await zha_gateway.async_block_till_done()
+
     assert entity.state["is_on"] is True
 
     await send_attributes_report(zha_gateway, dev2_fan_cluster, {0: 0})
     await zha_gateway.async_block_till_done()
 
+    # no update yet because of debouncing
+    assert entity.state["is_on"] is True
+
+    # member updates are debounced for .5s
+    await asyncio.sleep(1)
+    await zha_gateway.async_block_till_done()
+
     # test that group fan is now off
     assert entity.state["is_on"] is False
 

tests/test_light.py

@@ -515,11 +515,25 @@ async def async_test_on_off_from_light(
     # turn on at light
     await send_attributes_report(zha_gateway, cluster, {1: 0, 0: 1, 2: 3})
     await zha_gateway.async_block_till_done()
+
+    # group member updates are debounced
+    if isinstance(entity, GroupEntity):
+        assert bool(entity.state["on"]) is False
+        await asyncio.sleep(0.1)
+        await zha_gateway.async_block_till_done()
+
     assert bool(entity.state["on"]) is True
 
     # turn off at light
     await send_attributes_report(zha_gateway, cluster, {1: 1, 0: 0, 2: 3})
     await zha_gateway.async_block_till_done()
+
+    # group member updates are debounced
+    if isinstance(entity, GroupEntity):
+        assert bool(entity.state["on"]) is True
+        await asyncio.sleep(0.1)
+        await zha_gateway.async_block_till_done()
+
     assert bool(entity.state["on"]) is False
 
 
@@ -534,6 +548,13 @@ async def async_test_on_from_light(
         zha_gateway, cluster, {general.OnOff.AttributeDefs.on_off.id: 1}
     )
     await zha_gateway.async_block_till_done()
+
+    # group member updates are debounced
+    if isinstance(entity, GroupEntity):
+        assert bool(entity.state["on"]) is False
+        await asyncio.sleep(0.1)
+        await zha_gateway.async_block_till_done()
+
     assert bool(entity.state["on"]) is True
 
 
@@ -695,6 +716,10 @@ async def async_test_dimmer_from_light(
     if level == 0:
         assert entity.state["brightness"] is None
     else:
+        # group member updates are debounced
+        if isinstance(entity, GroupEntity):
+            await asyncio.sleep(0.1)
+            await zha_gateway.async_block_till_done()
         assert entity.state["brightness"] == level
 
 
@@ -873,6 +898,11 @@ async def test_zha_group_light_entity(
     # test that group light is now off
     assert device_1_light_entity.state["on"] is False
     assert device_2_light_entity.state["on"] is False
+
+    # group member updates are debounced
+    assert bool(entity.state["on"]) is True
+    await asyncio.sleep(0.1)
+    await zha_gateway.async_block_till_done()
     assert bool(entity.state["on"]) is False
 
     await send_attributes_report(zha_gateway, dev1_cluster_on_off, {0: 1})
@@ -881,13 +911,21 @@ async def test_zha_group_light_entity(
     # test that group light is now back on
     assert device_1_light_entity.state["on"] is True
     assert device_2_light_entity.state["on"] is False
+    # group member updates are debounced
+    assert bool(entity.state["on"]) is False
+    await asyncio.sleep(0.1)
+    await zha_gateway.async_block_till_done()
     assert bool(entity.state["on"]) is True
 
     # turn it off to test a new member add being tracked
     await send_attributes_report(zha_gateway, dev1_cluster_on_off, {0: 0})
     await zha_gateway.async_block_till_done()
     assert device_1_light_entity.state["on"] is False
     assert device_2_light_entity.state["on"] is False
+    # group member updates are debounced
+    assert bool(entity.state["on"]) is True
+    await asyncio.sleep(0.1)
+    await zha_gateway.async_block_till_done()
     assert bool(entity.state["on"]) is False
 
     # add a new member and test that his state is also tracked
@@ -905,6 +943,10 @@ async def test_zha_group_light_entity(
     assert device_1_light_entity.state["on"] is False
     assert device_2_light_entity.state["on"] is False
     assert device_3_light_entity.state["on"] is True
+    # group member updates are debounced
+    assert bool(entity.state["on"]) is False
+    await asyncio.sleep(0.1)
+    await zha_gateway.async_block_till_done()
     assert bool(entity.state["on"]) is True
 
     # make the group have only 1 member and now there should be no entity
@@ -941,6 +983,10 @@ async def test_zha_group_light_entity(
     await send_attributes_report(zha_gateway, dev1_cluster_on_off, {0: 0})
     await send_attributes_report(zha_gateway, dev3_cluster_on_off, {0: 0})
     await zha_gateway.async_block_till_done()
+    # group member updates are debounced
+    assert bool(entity.state["on"]) is True
+    await asyncio.sleep(0.1)
+    await zha_gateway.async_block_till_done()
     assert bool(entity.state["on"]) is False
 
     # this will test that _reprobe_group is used correctly
@@ -956,6 +1002,10 @@ async def test_zha_group_light_entity(
     assert entity is not None
     await send_attributes_report(zha_gateway, dev2_cluster_on_off, {0: 1})
     await zha_gateway.async_block_till_done()
+    # group member updates are debounced
+    assert bool(entity.state["on"]) is False
+    await asyncio.sleep(0.1)
+    await zha_gateway.async_block_till_done()
     assert bool(entity.state["on"]) is True
 
     await zha_group.async_remove_members(

tests/test_switch.py

@@ -1,5 +1,6 @@
 """Test zha switch."""
 
+import asyncio
 from collections.abc import Awaitable, Callable
 import logging
 from unittest.mock import call, patch
@@ -235,6 +236,7 @@ async def test_switch(
     assert bool(entity.state["state"]) is True
 
 
+@pytest.mark.looptime
 async def test_zha_group_switch_entity(
     device_switch_1: Device,  # pylint: disable=redefined-outer-name
     device_switch_2: Device,  # pylint: disable=redefined-outer-name
@@ -312,6 +314,11 @@ async def test_zha_group_switch_entity(
     await send_attributes_report(zha_gateway, dev2_cluster_on_off, {0: 1})
     await zha_gateway.async_block_till_done()
 
+    # group member updates are debounced
+    assert bool(entity.state["state"]) is False
+    await asyncio.sleep(1)
+    await zha_gateway.async_block_till_done()
+
     # test that group light is on
     assert bool(entity.state["state"]) is True
 
@@ -324,12 +331,22 @@ async def test_zha_group_switch_entity(
     await send_attributes_report(zha_gateway, dev2_cluster_on_off, {0: 0})
     await zha_gateway.async_block_till_done()
 
+    # group member updates are debounced
+    assert bool(entity.state["state"]) is True
+    await asyncio.sleep(1)
+    await zha_gateway.async_block_till_done()
+
     # test that group light is now off
     assert bool(entity.state["state"]) is False
 
     await send_attributes_report(zha_gateway, dev1_cluster_on_off, {0: 1})
     await zha_gateway.async_block_till_done()
 
+    # group member updates are debounced
+    assert bool(entity.state["state"]) is False
+    await asyncio.sleep(1)
+    await zha_gateway.async_block_till_done()
+
     # test that group light is now back on
     assert bool(entity.state["state"]) is True
 

zha/application/platforms/__init__.py

@@ -16,6 +16,8 @@
 
 from zha.application import Platform
 from zha.const import STATE_CHANGED
+from zha.debounce import Debouncer
+from zha.decorators import callback
 from zha.event import EventBase
 from zha.mixins import LogMixin
 from zha.zigbee.cluster_handlers import ClusterHandlerInfo
@@ -29,6 +31,8 @@
 
 _LOGGER = logging.getLogger(__name__)
 
+DEFAULT_UPDATE_GROUP_FROM_CHILD_DELAY: float = 0.5
+
 
 class EntityCategory(StrEnum):
     """Category of an entity."""
@@ -404,11 +408,22 @@ async def async_update(self) -> None:
 class GroupEntity(BaseEntity):
     """A base class for group entities."""
 
-    def __init__(self, group: Group) -> None:
+    def __init__(
+        self,
+        group: Group,
+        update_group_from_member_delay: float = DEFAULT_UPDATE_GROUP_FROM_CHILD_DELAY,
+    ) -> None:
         """Initialize a group."""
         super().__init__(unique_id=f"{self.PLATFORM}_zha_group_0x{group.group_id:04x}")
         self._attr_fallback_name: str = group.name
         self._group: Group = group
+        self._change_listener_debouncer = Debouncer(
+            group.gateway,
+            _LOGGER,
+            cooldown=update_group_from_member_delay,
+            immediate=False,
+            function=self.update,
+        )
         self._group.register_group_entity(self)
 
     @cached_property
@@ -438,6 +453,19 @@ def group(self) -> Group:
         """Return the group."""
         return self._group
 
+    @callback
+    def debounced_update(self, _: Any | None = None) -> None:
+        """Debounce updating group entity from member entity updates."""
+        # Delay to ensure that we get updates from all members before updating the group entity
+        assert self._change_listener_debouncer
+        self.group.gateway.create_task(self._change_listener_debouncer.async_call())
+
+    async def on_remove(self) -> None:
+        """Cancel tasks this entity owns."""
+        await super().on_remove()
+        if self._change_listener_debouncer:
+            self._change_listener_debouncer.async_cancel()
+
     @abstractmethod
     def update(self, _: Any | None = None) -> None:
         """Update the state of this group entity."""

zha/application/platforms/fan/__init__.py

@@ -40,6 +40,7 @@
     ranged_value_to_percentage,
 )
 from zha.application.registries import PLATFORM_ENTITIES
+from zha.decorators import callback
 from zha.zigbee.cluster_handlers import (
     ClusterAttributeUpdatedEvent,
     wrap_zigpy_exceptions,
@@ -345,6 +346,7 @@ async def _async_set_fan_mode(self, fan_mode: int) -> None:
 
         self.maybe_emit_state_changed_event()
 
+    @callback
     def update(self, _: Any = None) -> None:
         """Attempt to retrieve on off state from the fan."""
         self.debug("Updating fan group entity state")

zha/application/platforms/light/__init__.py

@@ -65,7 +65,7 @@
 )
 from zha.application.registries import PLATFORM_ENTITIES
 from zha.debounce import Debouncer
-from zha.decorators import periodic
+from zha.decorators import callback, periodic
 from zha.zigbee.cluster_handlers import ClusterAttributeUpdatedEvent
 from zha.zigbee.cluster_handlers.const import (
     CLUSTER_HANDLER_ATTRIBUTE_UPDATED,
@@ -1128,7 +1128,27 @@ class LightGroup(GroupEntity, BaseLight):
 
     def __init__(self, group: Group):
         """Initialize a light group."""
-        super().__init__(group)
+        # light groups change the update_group_from_child_delay so we need to do this
+        # before calling super
+        light_options = group.gateway.config.config.light_options
+        self._zha_config_transition = light_options.default_light_transition
+        self._zha_config_enable_light_transitioning_flag = (
+            light_options.enable_light_transitioning_flag
+        )
+        self._zha_config_always_prefer_xy_color_mode = (
+            light_options.always_prefer_xy_color_mode
+        )
+        self._zha_config_group_members_assume_state = (
+            light_options.group_members_assume_state
+        )
+        kwargs = {}
+        if self._zha_config_group_members_assume_state:
+            kwargs["update_group_from_member_delay"] = (
+                ASSUME_UPDATE_GROUP_FROM_CHILD_DELAY
+            )
+        self._zha_config_enhanced_light_transition = False
+
+        super().__init__(group, **kwargs)
         self._GROUP_SUPPORTS_EXECUTE_IF_OFF: bool = True
 
         for member in group.members:
@@ -1163,33 +1183,18 @@ def __init__(self, group: Group):
         self._identify_cluster_handler: None | (
             ClusterHandler
         ) = group.zigpy_group.endpoint[Identify.cluster_id]
-        self._debounced_member_refresh: Debouncer | None = None
-        light_options = group.gateway.config.config.light_options
-        self._zha_config_transition = light_options.default_light_transition
-        self._zha_config_enable_light_transitioning_flag = (
-            light_options.enable_light_transitioning_flag
-        )
-        self._zha_config_always_prefer_xy_color_mode = (
-            light_options.always_prefer_xy_color_mode
-        )
-        self._zha_config_group_members_assume_state = (
-            light_options.group_members_assume_state
-        )
-        if self._zha_config_group_members_assume_state:
-            self._update_group_from_child_delay = ASSUME_UPDATE_GROUP_FROM_CHILD_DELAY
-        self._zha_config_enhanced_light_transition = False
 
         self._color_mode = ColorMode.UNKNOWN
         self._supported_color_modes = {ColorMode.ONOFF}
 
-        force_refresh_debouncer = Debouncer(
+        self._debounced_member_refresh: Debouncer | None = Debouncer(
             self.group.gateway,
             _LOGGER,
             cooldown=3,
             immediate=True,
             function=self._force_member_updates,
         )
-        self._debounced_member_refresh = force_refresh_debouncer
+
         if hasattr(self, "info_object"):
             delattr(self, "info_object")
         self.update()
@@ -1241,6 +1246,7 @@ async def async_turn_off(self, **kwargs: Any) -> None:
         if self._debounced_member_refresh:
             await self._debounced_member_refresh.async_call()
 
+    @callback
     def update(self, _: Any = None) -> None:
         """Query all members and determine the light group state."""
         self.debug("Updating light group entity state")

zha/application/platforms/switch.py

@@ -24,6 +24,7 @@
     PlatformEntity,
 )
 from zha.application.registries import PLATFORM_ENTITIES
+from zha.decorators import callback
 from zha.zigbee.cluster_handlers import ClusterAttributeUpdatedEvent
 from zha.zigbee.cluster_handlers.const import (
     CLUSTER_HANDLER_ATTRIBUTE_UPDATED,
@@ -168,6 +169,7 @@ async def async_turn_off(self, **kwargs: Any) -> None:  # pylint: disable=unused
         self._state = False
         self.maybe_emit_state_changed_event()
 
+    @callback
     def update(self, _: Any | None = None) -> None:
         """Query all members and determine the light group state."""
         self.debug("Updating switch group entity state")