From 9efb88b2ced286acead723ecea3f7c86d2c43e23 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Fri, 26 Jul 2024 16:41:16 +0200 Subject: [PATCH 1/8] Let cluster config/init bubble up exception groups Only skip errors when re-joining --- tests/test_cluster_handlers.py | 4 ++-- zha/application/gateway.py | 7 ++++++- zha/zigbee/endpoint.py | 22 +++++++++++++--------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/tests/test_cluster_handlers.py b/tests/test_cluster_handlers.py index a4c25cf33..2d6d42573 100644 --- a/tests/test_cluster_handlers.py +++ b/tests/test_cluster_handlers.py @@ -847,8 +847,8 @@ async def test_ep_cluster_handlers_configure(cluster_handler) -> None: assert ch.async_configure.call_count == 1 assert ch.async_configure.await_count == 1 - assert ch_3.debug.call_count == 2 - assert ch_5.debug.call_count == 2 + assert ch_3.warning.call_count == 2 + assert ch_5.warning.call_count == 2 async def test_poll_control_configure( diff --git a/zha/application/gateway.py b/zha/application/gateway.py index 87a2f0ba2..2ee705ef8 100644 --- a/zha/application/gateway.py +++ b/zha/application/gateway.py @@ -23,6 +23,7 @@ ) import zigpy.device import zigpy.endpoint +from zigpy.exceptions import DeliveryError import zigpy.group from zigpy.state import State from zigpy.types.named import EUI64 @@ -618,7 +619,11 @@ async def _async_device_rejoined(self, zha_device: Device) -> None: ) # we don't have to do this on a nwk swap # but we don't have a way to tell currently - await zha_device.async_configure() + try: + await zha_device.async_configure() + except* (TimeoutError, DeliveryError): + zha_device.debug("ignoring error %s during rejoin", exc_info=True) + device_info = ExtendedDeviceInfoWithPairingStatus( pairing_status=DevicePairingStatus.CONFIGURED, **zha_device.extended_device_info.__dict__, diff --git a/zha/zigbee/endpoint.py b/zha/zigbee/endpoint.py index e222606cc..930c21ae8 100644 --- a/zha/zigbee/endpoint.py +++ b/zha/zigbee/endpoint.py @@ -192,7 +192,16 @@ async def _execute_handler_tasks( *self.claimed_cluster_handlers.values(), *self.client_cluster_handlers.values(), ] - tasks = [getattr(ch, func_name)(*args) for ch in cluster_handlers] + + faults: list[Exception] = [] + + async def caller(ch: ClusterHandler): + try: + await getattr(ch, func_name)(*args) + except Exception as outcome: + faults.append(outcome) + + tasks = [caller(ch) for ch in cluster_handlers] gather: Callable[..., Awaitable] @@ -201,14 +210,9 @@ async def _execute_handler_tasks( else: gather = functools.partial(gather_with_limited_concurrency, max_concurrency) - results = await gather(*tasks, return_exceptions=True) - for cluster_handler, outcome in zip(cluster_handlers, results): - if isinstance(outcome, Exception): - cluster_handler.debug( - "'%s' stage failed: %s", func_name, str(outcome), exc_info=outcome - ) - else: - cluster_handler.debug("'%s' stage succeeded", func_name) + await gather(*tasks) + if faults: + raise ExceptionGroup(f"{func_name}: some clusters failed", faults) def async_new_entity( self, From 5d5c614eb65a35a2c9a7f1afb38185ca00df6940 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 1 Aug 2024 02:57:32 +0200 Subject: [PATCH 2/8] Hack for default response in tests --- zha/zigbee/cluster_handlers/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zha/zigbee/cluster_handlers/__init__.py b/zha/zigbee/cluster_handlers/__init__.py index 3ff76ee1a..77fd97004 100644 --- a/zha/zigbee/cluster_handlers/__init__.py +++ b/zha/zigbee/cluster_handlers/__init__.py @@ -374,7 +374,7 @@ def _configure_reporting_status( event_data: dict[str, dict[str, Any]], ) -> None: """Parse configure reporting result.""" - if isinstance(res, (Exception, ConfigureReportingResponseRecord)): + if isinstance(res, (Exception, ConfigureReportingResponseRecord, int)): # assume default response self.debug( "attr reporting for '%s' on '%s': %s", From 7b47e7f761174c5eb9a76946d8d9a45cb02691e0 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 1 Aug 2024 15:58:03 +0200 Subject: [PATCH 3/8] Group names are limited to 16 characters --- zha/zigbee/cluster_handlers/lightlink.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zha/zigbee/cluster_handlers/lightlink.py b/zha/zigbee/cluster_handlers/lightlink.py index 4322b7899..ae63174cb 100644 --- a/zha/zigbee/cluster_handlers/lightlink.py +++ b/zha/zigbee/cluster_handlers/lightlink.py @@ -44,4 +44,4 @@ async def async_configure(self) -> None: self.debug("Adding coordinator to 0x%04x group id", group.group_id) await coordinator.add_to_group(group.group_id) else: - await coordinator.add_to_group(0x0000, name="Default Lightlink Group") + await coordinator.add_to_group(0x0000, name="Lightlink Group") From 19848662f10d9d577f85b6b2aef4c63715a591f4 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 1 Aug 2024 17:22:48 +0200 Subject: [PATCH 4/8] Handle errors on configure/init --- tests/test_cluster_handlers.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/test_cluster_handlers.py b/tests/test_cluster_handlers.py index 2d6d42573..95886f5e5 100644 --- a/tests/test_cluster_handlers.py +++ b/tests/test_cluster_handlers.py @@ -837,8 +837,10 @@ async def test_ep_cluster_handlers_configure(cluster_handler) -> None: mock.patch.dict(endpoint.claimed_cluster_handlers, claimed, clear=True), mock.patch.dict(endpoint.client_cluster_handlers, client_handlers, clear=True), ): - await endpoint.async_configure() - await endpoint.async_initialize(mock.sentinel.from_cache) + with pytest.raises(ExceptionGroup): + await endpoint.async_configure() + with pytest.raises(ExceptionGroup): + await endpoint.async_initialize(mock.sentinel.from_cache) for ch in [*claimed.values(), *client_handlers.values()]: assert ch.async_initialize.call_count == 1 @@ -847,9 +849,6 @@ async def test_ep_cluster_handlers_configure(cluster_handler) -> None: assert ch.async_configure.call_count == 1 assert ch.async_configure.await_count == 1 - assert ch_3.warning.call_count == 2 - assert ch_5.warning.call_count == 2 - async def test_poll_control_configure( poll_control_ch: PollControlClusterHandler, From 5071e2d3e4ba46de38e383db944368b550d62326 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 1 Aug 2024 17:37:19 +0200 Subject: [PATCH 5/8] Check for actual schema instead of negative --- zha/zigbee/cluster_handlers/lightlink.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/zha/zigbee/cluster_handlers/lightlink.py b/zha/zigbee/cluster_handlers/lightlink.py index ae63174cb..53b736d42 100644 --- a/zha/zigbee/cluster_handlers/lightlink.py +++ b/zha/zigbee/cluster_handlers/lightlink.py @@ -34,10 +34,12 @@ async def async_configure(self) -> None: self.warning("Couldn't get list of groups: %s", str(exc)) return - if isinstance(rsp, GENERAL_COMMANDS[GeneralCommand.Default_Response].schema): - groups = [] - else: + if isinstance( + rsp, LightLink.ClientCommandDefs.get_group_identifiers_rsp.schema + ): groups = rsp.group_info_records + else: + groups = [] if groups: for group in groups: From 57789254f7c9364bda70f947ee9a38c479ac9a2b Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 1 Aug 2024 16:27:08 +0200 Subject: [PATCH 6/8] Make sure we respond with a general command as default mock We must also make sure that CustomEndpoints get the mocked request. --- tests/conftest.py | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4e4c2c22f..fe8d36275 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,7 +25,7 @@ from zigpy.quirks import get_device import zigpy.types from zigpy.zcl.clusters.general import Basic, Groups -from zigpy.zcl.foundation import Status +from zigpy.zcl.foundation import GENERAL_COMMANDS, GeneralCommand, Status import zigpy.zdo.types as zdo_t from tests import common @@ -450,7 +450,6 @@ def _mock_dev( endpoint = device.add_endpoint(epid) endpoint.device_type = ep[SIG_EP_TYPE] endpoint.profile_id = ep.get(SIG_EP_PROFILE) - endpoint.request = AsyncMock(return_value=[0]) for cluster_id in ep.get(SIG_EP_INPUT, []): endpoint.add_input_cluster(cluster_id) @@ -463,9 +462,38 @@ def _mock_dev( else: device = get_device(device) + async def mock_request( + cluster: zigpy.types.ClusterId, + sequence: zigpy.types.uint8_t, + data: bytes, + expect_reply: bool = True, + command_id: GeneralCommand | zigpy.types.uint8_t = 0x00, + ): + # if isinstance(command_id, (int, GeneralCommand)): + # Some commands can't handle default response, and will + # fail with a non list element as first element. + if command_id in ( + GeneralCommand.Read_Reporting_Configuration, + GeneralCommand.Write_Attributes, + ): + return [[]] + + if command_id in GeneralCommand.__members__: + return GENERAL_COMMANDS[GeneralCommand.Default_Response].schema( + command_id=command_id, status=Status.UNSUP_GENERAL_COMMAND + ) + + return [0] + + # add request mock after device creation since quirks may have added endpoints + for epid, endpoint in device.endpoints.items(): + if epid: + endpoint.request = AsyncMock(side_effect=mock_request) + else: + endpoint.request = AsyncMock(return_value=[0]) + if patch_cluster: for endpoint in (ep for epid, ep in device.endpoints.items() if epid): - endpoint.request = AsyncMock(return_value=[0]) for cluster in itertools.chain( endpoint.in_clusters.values(), endpoint.out_clusters.values() ): From 5cb93fb21170b3f4b4758354b0841e61d5be3ffc Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 1 Aug 2024 17:40:21 +0200 Subject: [PATCH 7/8] Drop caplog checks --- tests/test_device.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/test_device.py b/tests/test_device.py index 4e97bd28f..cff04ba71 100644 --- a/tests/test_device.py +++ b/tests/test_device.py @@ -616,7 +616,6 @@ async def test_async_add_to_group_remove_from_group( async def test_async_bind_to_group( device_joined: Callable[[ZigpyDevice], Awaitable[Device]], zigpy_device: Callable[..., ZigpyDevice], # pylint: disable=redefined-outer-name - caplog: pytest.LogCaptureFixture, ) -> None: """Test async_bind_to_group method.""" zigpy_dev = zigpy_device(with_basic_cluster_handler=True) @@ -640,19 +639,12 @@ async def test_async_bind_to_group( group.group_id, [ClusterBinding(name="on_off", type=CLUSTER_TYPE_OUT, id=6, endpoint_id=3)], ) - assert ( - "0xb79c: Bind_req 00:0d:7f:00:0a:90:69:e8, ep: 3, cluster: 6 to group: 0x1001 completed: []" - in caplog.text - ) await zha_device_remote.async_unbind_from_group( group.group_id, [ClusterBinding(name="on_off", type=CLUSTER_TYPE_OUT, id=6, endpoint_id=3)], ) - m1 = "0xb79c: Unbind_req 00:0d:7f:00:0a:90:69:e8, ep: 3, cluster: 6" - assert f"{m1} to group: 0x1001 completed: []" in caplog.text - async def test_device_automation_triggers( device_joined: Callable[[ZigpyDevice], Awaitable[Device]], From ebc708cfc591b19211c144c14d0a2af174eb81e4 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 1 Aug 2024 17:42:57 +0200 Subject: [PATCH 8/8] Drop unused in light link --- zha/zigbee/cluster_handlers/lightlink.py | 1 - 1 file changed, 1 deletion(-) diff --git a/zha/zigbee/cluster_handlers/lightlink.py b/zha/zigbee/cluster_handlers/lightlink.py index 53b736d42..83edd58f6 100644 --- a/zha/zigbee/cluster_handlers/lightlink.py +++ b/zha/zigbee/cluster_handlers/lightlink.py @@ -2,7 +2,6 @@ import zigpy.exceptions from zigpy.zcl.clusters.lightlink import LightLink -from zigpy.zcl.foundation import GENERAL_COMMANDS, GeneralCommand from zha.zigbee.cluster_handlers import ClusterHandler, ClusterHandlerStatus, registries