From d49a4365735fbb6af09a6f0a02fe9f7655985484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ab=C3=ADlio=20Costa?= Date: Tue, 16 Mar 2021 21:38:16 +0000 Subject: [PATCH] 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 --- homeassistant/components/zha/entity.py | 7 ++++++- tests/components/zha/common.py | 9 +++++++++ tests/components/zha/test_fan.py | 22 ++++++++++++++++------ tests/components/zha/test_light.py | 25 +++++++++++++++---------- tests/components/zha/test_switch.py | 17 +++++++++++------ 5 files changed, 57 insertions(+), 23 deletions(-) diff --git a/homeassistant/components/zha/entity.py b/homeassistant/components/zha/entity.py index c19bad21455..9a573e92aa4 100644 --- a/homeassistant/components/zha/entity.py +++ b/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.""" diff --git a/tests/components/zha/common.py b/tests/components/zha/common.py index 234ca0c9ba5..eeffa3fb911 100644 --- a/tests/components/zha/common.py +++ b/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() diff --git a/tests/components/zha/test_fan.py b/tests/components/zha/test_fan.py index 81a441a4101..eed0e0b691e 100644 --- a/tests/components/zha/test_fan.py +++ b/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 diff --git a/tests/components/zha/test_light.py b/tests/components/zha/test_light.py index 021f4f09dd9..fe367a3969b 100644 --- a/tests/components/zha/test_light.py +++ b/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 diff --git a/tests/components/zha/test_switch.py b/tests/components/zha/test_switch.py index da3037f720d..4cec0753c68 100644 --- a/tests/components/zha/test_switch.py +++ b/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