From a0dd0fbac85b649457f09690542f1fe4eb26c02d Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Mon, 19 Jun 2023 15:36:09 +0200 Subject: [PATCH] Some small bugfixes and stability improvements (#324) Co-authored-by: Martin Hjelmare --- .pre-commit-config.yaml | 4 +- matter_server/client/models/device_types.py | 2 +- matter_server/client/models/node.py | 2 +- matter_server/server/device_controller.py | 44 ++++++++++++++++----- pyproject.toml | 5 +++ scripts/beautify_diagnostics.py | 2 +- scripts/generate_devices.py | 2 +- 7 files changed, 47 insertions(+), 14 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index da7b3b74..b4f55b13 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,12 +9,14 @@ repos: - --quiet files: ^(matter_server|scripts)/.+\.py$ - repo: https://github.com/codespell-project/codespell - rev: v2.2.2 + rev: v2.2.4 hooks: - id: codespell args: [] exclude_types: [csv, json] exclude: ^tests/fixtures/ + additional_dependencies: + - tomli - repo: https://github.com/PyCQA/flake8 rev: 6.0.0 hooks: diff --git a/matter_server/client/models/device_types.py b/matter_server/client/models/device_types.py index a5dff676..ddd2f3c3 100644 --- a/matter_server/client/models/device_types.py +++ b/matter_server/client/models/device_types.py @@ -27,7 +27,7 @@ def __init_subclass__(cls, *, device_type: int, **kwargs: typing.Any) -> None: def __hash__(self) -> int: """Return unique hash for this object.""" - return hash(self.device_type) + return self.device_type class OrphanClusters(DeviceType, device_type=0xF001): diff --git a/matter_server/client/models/node.py b/matter_server/client/models/node.py index bd58b94e..38b939a7 100644 --- a/matter_server/client/models/node.py +++ b/matter_server/client/models/node.py @@ -65,7 +65,7 @@ def __init__( self.node = node self.endpoint_id = endpoint_id self.clusters: dict[int, Clusters.Cluster] = {} - self.device_types: set[DeviceType] = set() + self.device_types: set[type[DeviceType]] = set() self.update(attributes_data) @property diff --git a/matter_server/server/device_controller.py b/matter_server/server/device_controller.py index 0ade369e..7c877fa4 100644 --- a/matter_server/server/device_controller.py +++ b/matter_server/server/device_controller.py @@ -285,13 +285,13 @@ async def interview_node(self, node_id: int) -> None: LOGGER.debug("Interviewing node: %s", node_id) try: - await self._call_sdk(self.chip_controller.ResolveNode, nodeid=node_id) + await self._resolve_node(node_id=node_id) read_response: Attribute.AsyncReadTransaction.ReadResponse = ( await self.chip_controller.Read( nodeid=node_id, attributes="*", events="*", fabricFiltered=False ) ) - except ChipStackError as err: + except (ChipStackError, NodeNotResolving) as err: raise NodeInterviewFailed(f"Failed to interview node {node_id}") from err is_new_node = node_id not in self._nodes @@ -406,12 +406,9 @@ async def subscribe_node(self, node_id: int) -> None: node_logger.debug("Setting up subscriptions...") node = cast(MatterNodeData, self._nodes[node_id]) - - try: - await self._call_sdk(self.chip_controller.ResolveNode, nodeid=node_id) - except ChipStackError as err: - node.available = False - raise NodeNotResolving(f"Failed to resolve node {node_id}") from err + node.available = False + await self._resolve_node(node_id=node_id) + node.available = True # we follow the pattern of apple and google here and # just do a wildcard subscription for all clusters and properties @@ -421,7 +418,7 @@ async def subscribe_node(self, node_id: int) -> None: sub: Attribute.SubscriptionTransaction = await self.chip_controller.Read( nodeid=node_id, attributes="*", - events=[("*", 0)], + events=[("*", 1)], reportInterval=(0, 120), fabricFiltered=False, ) @@ -632,3 +629,32 @@ def _parse_attributes_from_read_result( ) result[attribute_path] = attr_value return result + + async def _resolve_node( + self, node_id: int, retries: int = 3, allow_pase: bool = False + ) -> None: + """Resolve a Node on the network.""" + if self.chip_controller is None: + raise RuntimeError("Device Controller not initialized.") + try: + if allow_pase: + # last attempt allows PASE connection (last resort) + LOGGER.debug( + "Attempting to resolve node %s (with PASE connection)", node_id + ) + await self._call_sdk( + self.chip_controller.GetConnectedDeviceSync, + nodeid=node_id, + allowPASE=True, + ) + return + LOGGER.debug("Resolving node %s", node_id) + await self._call_sdk(self.chip_controller.ResolveNode, nodeid=node_id) + except (ChipStackError, TimeoutError) as err: + if not retries: + # when we're out of retries, raise NodeNotResolving + raise NodeNotResolving(f"Unable to resolve Node {node_id}") from err + await self._resolve_node( + node_id=node_id, retries=retries - 1, allow_pase=retries - 1 == 0 + ) + await asyncio.sleep(2) diff --git a/pyproject.toml b/pyproject.toml index b87486fc..0e10d853 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -49,6 +49,9 @@ test = [ [project.scripts] matter-server = "matter_server.server.__main__:main" +[tool.codespell] +ignore-words-list = "pase," + [tool.mypy] python_version = "3.10" check_untyped_defs = true @@ -134,3 +137,5 @@ forced_separate = [ "tests", ] combine_as_imports = true + + diff --git a/scripts/beautify_diagnostics.py b/scripts/beautify_diagnostics.py index 6e295b7f..a0b2faee 100644 --- a/scripts/beautify_diagnostics.py +++ b/scripts/beautify_diagnostics.py @@ -75,7 +75,7 @@ def process_node(node): continue for device_type in device_types: - device_type_id = device_type["type"] + device_type_id = device_type["deviceType"] if device_type_id in ALL_TYPES: device_type_name = ALL_TYPES[device_type_id].__name__ else: diff --git a/scripts/generate_devices.py b/scripts/generate_devices.py index 5df59266..a2f6daec 100644 --- a/scripts/generate_devices.py +++ b/scripts/generate_devices.py @@ -9,7 +9,7 @@ CHIP_ROOT = REPO_ROOT / "../../project-chip/connectedhomeip" DEVICE_XML = CHIP_ROOT / "src/app/zap-templates/zcl/data-model/chip/matter-devices.xml" -OUTPUT_PYTHON = REPO_ROOT / "matter_server/common/models/device_types.py" +OUTPUT_PYTHON = REPO_ROOT / "matter_server/client/models/device_types.py" def gen_cls_name(name: str):