Delay ZHA group updates to ensure all members are updated first (#46861)

* Delay ZHA group updates to ensure all members are updated first

After turning off a group, when the first device reports "off", the
other devices may still be "on". If HA processes the group state update
quickly enough, the group will see that some devices are on, so the
state of the group will revert back to "on", and then "off" when the
remaining devices all report "off". That would cause the UI toggle to go
back and forward quickly, and automations that trigger with "state: on"
to fire when the user turns the group off.

This PR fixes that by delaying the group state update, giving time for
all the devices to report their states first.

* Fix zha group tests

* Reorder sleeping.

* Update tests/components/zha/common.py

Co-authored-by: Alexei Chetroi <lexoid@gmail.com>

homeassistant/components/zha/entity.py

@@ -32,6 +32,7 @@ from .core.typing import CALLABLE_T, ChannelType, ZhaDeviceType
 _LOGGER = logging.getLogger(__name__)
 
 ENTITY_SUFFIX = "entity_suffix"
+UPDATE_GROUP_FROM_CHILD_DELAY = 0.2
 
 
 class BaseZhaEntity(LogMixin, entity.Entity):
@@ -267,7 +268,11 @@ class ZhaGroupEntity(BaseZhaEntity):
     @callback
     def async_state_changed_listener(self, event: Event):
         """Handle child updates."""
-        self.async_schedule_update_ha_state(True)
+        # Delay to ensure that we get updates from all members before updating the group
+        self.hass.loop.call_later(
+            UPDATE_GROUP_FROM_CHILD_DELAY,
+            lambda: self.async_schedule_update_ha_state(True),
+        )
 
     async def async_will_remove_from_hass(self) -> None:
         """Handle removal from Home Assistant."""

tests/components/zha/common.py

@@ -1,4 +1,5 @@
 """Common test objects."""
+import asyncio
 import time
 from unittest.mock import AsyncMock, Mock
 
@@ -237,3 +238,11 @@ async def async_test_rejoin(hass, zigpy_device, clusters, report_counts, ep_id=1
         assert cluster.bind.await_count == 1
         assert cluster.configure_reporting.call_count == reports
         assert cluster.configure_reporting.await_count == reports
+
+
+async def async_wait_for_updates(hass):
+    """Wait until all scheduled updates are executed."""
+    await hass.async_block_till_done()
+    await asyncio.sleep(0)
+    await asyncio.sleep(0)
+    await hass.async_block_till_done()

tests/components/zha/test_fan.py

@@ -50,6 +50,8 @@ from .common import (
     send_attributes_report,
 )
 
+from tests.components.zha.common import async_wait_for_updates
+
 IEEE_GROUPABLE_DEVICE = "01:2d:6f:00:0a:90:69:e8"
 IEEE_GROUPABLE_DEVICE2 = "02:2d:6f:00:0a:90:69:e8"
 
@@ -246,6 +248,10 @@ async def async_set_preset_mode(hass, entity_id, preset_mode=None):
     "zigpy.zcl.clusters.hvac.Fan.write_attributes",
     new=AsyncMock(return_value=zcl_f.WriteAttributesResponse.deserialize(b"\x00")[0]),
 )
+@patch(
+    "homeassistant.components.zha.entity.UPDATE_GROUP_FROM_CHILD_DELAY",
+    new=0,
+)
 async def test_zha_group_fan_entity(hass, device_fan_1, device_fan_2, coordinator):
     """Test the fan entity for a ZHA group."""
     zha_gateway = get_zha_gateway(hass)
@@ -283,13 +289,13 @@ async def test_zha_group_fan_entity(hass, device_fan_1, device_fan_2, coordinato
     dev2_fan_cluster = device_fan_2.device.endpoints[1].fan
 
     await async_enable_traffic(hass, [device_fan_1, device_fan_2], enabled=False)
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     # test that the fans were created and that they are unavailable
     assert hass.states.get(entity_id).state == STATE_UNAVAILABLE
 
     # allow traffic to flow through the gateway and device
     await async_enable_traffic(hass, [device_fan_1, device_fan_2])
-
+    await async_wait_for_updates(hass)
     # test that the fan group entity was created and is off
     assert hass.states.get(entity_id).state == STATE_OFF
 
@@ -338,13 +344,13 @@ async def test_zha_group_fan_entity(hass, device_fan_1, device_fan_2, coordinato
     assert hass.states.get(entity_id).state == STATE_OFF
 
     await send_attributes_report(hass, dev2_fan_cluster, {0: 2})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group fan is speed medium
     assert hass.states.get(entity_id).state == STATE_ON
 
     await send_attributes_report(hass, dev2_fan_cluster, {0: 0})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group fan is now off
     assert hass.states.get(entity_id).state == STATE_OFF
@@ -354,6 +360,10 @@ async def test_zha_group_fan_entity(hass, device_fan_1, device_fan_2, coordinato
     "zigpy.zcl.clusters.hvac.Fan.write_attributes",
     new=AsyncMock(side_effect=ZigbeeException),
 )
+@patch(
+    "homeassistant.components.zha.entity.UPDATE_GROUP_FROM_CHILD_DELAY",
+    new=0,
+)
 async def test_zha_group_fan_entity_failure_state(
     hass, device_fan_1, device_fan_2, coordinator, caplog
 ):
@@ -390,13 +400,13 @@ async def test_zha_group_fan_entity_failure_state(
     group_fan_cluster = zha_group.endpoint[hvac.Fan.cluster_id]
 
     await async_enable_traffic(hass, [device_fan_1, device_fan_2], enabled=False)
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     # test that the fans were created and that they are unavailable
     assert hass.states.get(entity_id).state == STATE_UNAVAILABLE
 
     # allow traffic to flow through the gateway and device
     await async_enable_traffic(hass, [device_fan_1, device_fan_2])
-
+    await async_wait_for_updates(hass)
     # test that the fan group entity was created and is off
     assert hass.states.get(entity_id).state == STATE_OFF
 

tests/components/zha/test_light.py

@@ -25,6 +25,7 @@ from .common import (
 )
 
 from tests.common import async_fire_time_changed
+from tests.components.zha.common import async_wait_for_updates
 
 ON = 1
 OFF = 0
@@ -309,7 +310,7 @@ async def async_test_on_from_light(hass, cluster, entity_id):
     """Test on off functionality from the light."""
     # turn on at light
     await send_attributes_report(hass, cluster, {1: -1, 0: 1, 2: 2})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     assert hass.states.get(entity_id).state == STATE_ON
 
 
@@ -466,6 +467,10 @@ async def async_test_flash_from_hass(hass, cluster, entity_id, flash):
     "zigpy.zcl.clusters.general.OnOff.request",
     new=AsyncMock(return_value=[sentinel.data, zcl_f.Status.SUCCESS]),
 )
+@patch(
+    "homeassistant.components.zha.entity.UPDATE_GROUP_FROM_CHILD_DELAY",
+    new=0,
+)
 async def test_zha_group_light_entity(
     hass, device_light_1, device_light_2, device_light_3, coordinator
 ):
@@ -522,13 +527,13 @@ async def test_zha_group_light_entity(
     await async_enable_traffic(
         hass, [device_light_1, device_light_2, device_light_3], enabled=False
     )
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     # test that the lights were created and that they are unavailable
     assert hass.states.get(group_entity_id).state == STATE_UNAVAILABLE
 
     # allow traffic to flow through the gateway and device
     await async_enable_traffic(hass, [device_light_1, device_light_2, device_light_3])
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that the lights were created and are off
     assert hass.states.get(group_entity_id).state == STATE_OFF
@@ -580,7 +585,7 @@ async def test_zha_group_light_entity(
     assert hass.states.get(group_entity_id).state == STATE_ON
 
     await send_attributes_report(hass, dev2_cluster_on_off, {0: 0})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group light is now off
     assert hass.states.get(device_1_entity_id).state == STATE_OFF
@@ -588,7 +593,7 @@ async def test_zha_group_light_entity(
     assert hass.states.get(group_entity_id).state == STATE_OFF
 
     await send_attributes_report(hass, dev1_cluster_on_off, {0: 1})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group light is now back on
     assert hass.states.get(device_1_entity_id).state == STATE_ON
@@ -597,7 +602,7 @@ async def test_zha_group_light_entity(
 
     # turn it off to test a new member add being tracked
     await send_attributes_report(hass, dev1_cluster_on_off, {0: 0})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     assert hass.states.get(device_1_entity_id).state == STATE_OFF
     assert hass.states.get(device_2_entity_id).state == STATE_OFF
     assert hass.states.get(group_entity_id).state == STATE_OFF
@@ -605,7 +610,7 @@ async def test_zha_group_light_entity(
     # add a new member and test that his state is also tracked
     await zha_group.async_add_members([GroupMember(device_light_3.ieee, 1)])
     await send_attributes_report(hass, dev3_cluster_on_off, {0: 1})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     assert device_3_entity_id in zha_group.member_entity_ids
     assert len(zha_group.members) == 3
 
@@ -629,14 +634,14 @@ async def test_zha_group_light_entity(
     # add a member back and ensure that the group entity was created again
     await zha_group.async_add_members([GroupMember(device_light_3.ieee, 1)])
     await send_attributes_report(hass, dev3_cluster_on_off, {0: 1})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     assert len(zha_group.members) == 2
     assert hass.states.get(group_entity_id).state == STATE_ON
 
     # add a 3rd member and ensure we still have an entity and we track the new one
     await send_attributes_report(hass, dev1_cluster_on_off, {0: 0})
     await send_attributes_report(hass, dev3_cluster_on_off, {0: 0})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     assert hass.states.get(group_entity_id).state == STATE_OFF
 
     # this will test that _reprobe_group is used correctly
@@ -644,7 +649,7 @@ async def test_zha_group_light_entity(
         [GroupMember(device_light_2.ieee, 1), GroupMember(coordinator.ieee, 1)]
     )
     await send_attributes_report(hass, dev2_cluster_on_off, {0: 1})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
     assert len(zha_group.members) == 4
     assert hass.states.get(group_entity_id).state == STATE_ON
 

tests/components/zha/test_switch.py

@@ -20,6 +20,7 @@ from .common import (
 )
 
 from tests.common import mock_coro
+from tests.components.zha.common import async_wait_for_updates
 
 ON = 1
 OFF = 0
@@ -160,6 +161,10 @@ async def test_switch(hass, zha_device_joined_restored, zigpy_device):
     await async_test_rejoin(hass, zigpy_device, [cluster], (1,))
 
 
+@patch(
+    "homeassistant.components.zha.entity.UPDATE_GROUP_FROM_CHILD_DELAY",
+    new=0,
+)
 async def test_zha_group_switch_entity(
     hass, device_switch_1, device_switch_2, coordinator
 ):
@@ -195,14 +200,14 @@ async def test_zha_group_switch_entity(
     dev2_cluster_on_off = device_switch_2.device.endpoints[1].on_off
 
     await async_enable_traffic(hass, [device_switch_1, device_switch_2], enabled=False)
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that the lights were created and that they are off
     assert hass.states.get(entity_id).state == STATE_UNAVAILABLE
 
     # allow traffic to flow through the gateway and device
     await async_enable_traffic(hass, [device_switch_1, device_switch_2])
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that the lights were created and are off
     assert hass.states.get(entity_id).state == STATE_OFF
@@ -240,25 +245,25 @@ async def test_zha_group_switch_entity(
     # test some of the group logic to make sure we key off states correctly
     await send_attributes_report(hass, dev1_cluster_on_off, {0: 1})
     await send_attributes_report(hass, dev2_cluster_on_off, {0: 1})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group light is on
     assert hass.states.get(entity_id).state == STATE_ON
 
     await send_attributes_report(hass, dev1_cluster_on_off, {0: 0})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group light is still on
     assert hass.states.get(entity_id).state == STATE_ON
 
     await send_attributes_report(hass, dev2_cluster_on_off, {0: 0})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group light is now off
     assert hass.states.get(entity_id).state == STATE_OFF
 
     await send_attributes_report(hass, dev1_cluster_on_off, {0: 1})
-    await hass.async_block_till_done()
+    await async_wait_for_updates(hass)
 
     # test that group light is now back on
     assert hass.states.get(entity_id).state == STATE_ON