diff --git a/matter_server/server/device_controller.py b/matter_server/server/device_controller.py index c4cb34a8..bdb06fec 100644 --- a/matter_server/server/device_controller.py +++ b/matter_server/server/device_controller.py @@ -62,8 +62,6 @@ from collections.abc import Iterable from pathlib import Path - from chip.native import PyChipError - from .server import MatterServer DATA_KEY_NODES = "nodes" @@ -73,7 +71,6 @@ NODE_SUBSCRIPTION_CEILING_WIFI = 60 NODE_SUBSCRIPTION_CEILING_THREAD = 60 NODE_SUBSCRIPTION_CEILING_BATTERY_POWERED = 600 -MAX_COMMISSION_RETRIES = 3 NODE_RESUBSCRIBE_ATTEMPTS_UNAVAILABLE = 3 NODE_RESUBSCRIBE_TIMEOUT_OFFLINE = 30 * 60 * 1000 NODE_PING_TIMEOUT = 10 @@ -262,34 +259,29 @@ async def commission_with_code( """ node_id = self._get_next_node_id() - attempts = 0 - # we retry commissioning a few times as we've seen devices in the wild - # that are a bit unstable. - # by retrying, we increase the chances of a successful commission - while attempts <= MAX_COMMISSION_RETRIES: - attempts += 1 - LOGGER.info( - "Starting Matter commissioning with code using Node ID %s (attempt %s/%s).", - node_id, - attempts, - MAX_COMMISSION_RETRIES, - ) - result: ( - PyChipError | None - ) = await self._chip_device_controller.commission_with_code( - node_id, - code, - DiscoveryType.DISCOVERY_NETWORK_ONLY - if network_only - else DiscoveryType.DISCOVERY_ALL, - ) - if result and result.is_success: - break - if attempts >= MAX_COMMISSION_RETRIES: - raise NodeCommissionFailed( - f"Commission with code failed for node {node_id}." + LOGGER.info( + "Starting Matter commissioning with code using Node ID %s.", + node_id, + ) + try: + commissioned_node_id: int = ( + await self._chip_device_controller.commission_with_code( + node_id, + code, + DiscoveryType.DISCOVERY_NETWORK_ONLY + if network_only + else DiscoveryType.DISCOVERY_ALL, ) - await asyncio.sleep(5) + ) + # We use SDK default behavior which always uses the commissioning Node ID in the + # generated NOC. So this should be the same really. + LOGGER.info("Commissioned Node ID: %s vs %s", commissioned_node_id, node_id) + if commissioned_node_id != node_id: + raise RuntimeError("Returned Node ID must match requested Node ID") + except ChipStackError as err: + raise NodeCommissionFailed( + f"Commission with code failed for node {node_id}." + ) from err LOGGER.info("Matter commissioning of Node ID %s successful.", node_id) @@ -340,40 +332,35 @@ async def commission_on_network( if ip_addr is not None: ip_addr = self.server.scope_ipv6_lla(ip_addr) - attempts = 0 - # we retry commissioning a few times as we've seen devices in the wild - # that are a bit unstable. - # by retrying, we increase the chances of a successful commission - while attempts <= MAX_COMMISSION_RETRIES: - attempts += 1 - result: PyChipError | None + try: if ip_addr is None: # regular CommissionOnNetwork if no IP address provided LOGGER.info( - "Starting Matter commissioning on network using Node ID %s (attempt %s/%s).", + "Starting Matter commissioning on network using Node ID %s.", node_id, - attempts, - MAX_COMMISSION_RETRIES, ) - result = await self._chip_device_controller.commission_on_network( - node_id, setup_pin_code, filter_type, filter + commissioned_node_id = ( + await self._chip_device_controller.commission_on_network( + node_id, setup_pin_code, filter_type, filter + ) ) else: LOGGER.info( - "Starting Matter commissioning using Node ID %s and IP %s (attempt %s/%s).", + "Starting Matter commissioning using Node ID %s and IP %s.", node_id, ip_addr, - attempts, - MAX_COMMISSION_RETRIES, ) - result = await self._chip_device_controller.commission_ip( + commissioned_node_id = await self._chip_device_controller.commission_ip( node_id, setup_pin_code, ip_addr ) - if result and result.is_success: - break - if attempts >= MAX_COMMISSION_RETRIES: - raise NodeCommissionFailed(f"Commissioning failed for node {node_id}.") - await asyncio.sleep(5) + # We use SDK default behavior which always uses the commissioning Node ID in the + # generated NOC. So this should be the same really. + if commissioned_node_id != node_id: + raise RuntimeError("Returned Node ID must match requested Node ID") + except ChipStackError as err: + raise NodeCommissionFailed( + f"Commissioning failed for node {node_id}." + ) from err LOGGER.info("Matter commissioning of Node ID %s successful.", node_id) diff --git a/matter_server/server/sdk.py b/matter_server/server/sdk.py index 6fcca266..b55a8335 100644 --- a/matter_server/server/sdk.py +++ b/matter_server/server/sdk.py @@ -8,7 +8,6 @@ from __future__ import annotations import asyncio -from concurrent.futures import ThreadPoolExecutor from functools import partial import logging import time @@ -25,6 +24,7 @@ if TYPE_CHECKING: from collections.abc import Callable + from concurrent.futures import ThreadPoolExecutor from pathlib import Path from chip.ChipDeviceCtrl import ( @@ -59,7 +59,6 @@ def __init__(self, server: MatterServer, paa_root_cert_dir: Path): self._node_lock: dict[int, asyncio.Lock] = {} self._subscriptions: dict[int, Attribute.SubscriptionTransaction] = {} - self._sdk_non_entrant_executor = ThreadPoolExecutor(max_workers=1) # Instantiate the underlying ChipDeviceController instance on the Fabric self._chip_controller = self.server.stack.fabric_admin.NewController( @@ -100,16 +99,6 @@ async def _call_sdk( ) -> _T: return await self._call_sdk_executor(None, target, *args, **kwargs) - async def _call_sdk_non_reentrant( - self, - target: Callable[..., _T], - *args: Any, - **kwargs: Any, - ) -> _T: - return await self._call_sdk_executor( - self._sdk_non_entrant_executor, target, *args, **kwargs - ) - async def get_compressed_fabric_id(self) -> int: """Get the compressed fabric id.""" return await self._call_sdk(self._chip_controller.GetCompressedFabricId) @@ -128,13 +117,15 @@ async def commission_with_code( node_id: int, setup_payload: str, discovery_type: DiscoveryType, - ) -> PyChipError: + ) -> int: """Commission a device using a QR Code or Manual Pairing Code.""" - return await self._call_sdk_non_reentrant( - self._chip_controller.CommissionWithCode, - setupPayload=setup_payload, - nodeid=node_id, - discoveryType=discovery_type, + return cast( + int, + await self._chip_controller.CommissionWithCode( + setupPayload=setup_payload, + nodeid=node_id, + discoveryType=discovery_type, + ), ) async def commission_on_network( @@ -143,25 +134,29 @@ async def commission_on_network( setup_pin_code: int, disc_filter_type: FilterType = FilterType.NONE, disc_filter: Any = None, - ) -> PyChipError: + ) -> int: """Commission a device on the network.""" - return await self._call_sdk_non_reentrant( - self._chip_controller.CommissionOnNetwork, - nodeId=node_id, - setupPinCode=setup_pin_code, - filterType=disc_filter_type, - filter=disc_filter, + return cast( + int, + await self._chip_controller.CommissionOnNetwork( + nodeId=node_id, + setupPinCode=setup_pin_code, + filterType=disc_filter_type, + filter=disc_filter, + ), ) async def commission_ip( self, node_id: int, setup_pin_code: int, ip_addr: str - ) -> PyChipError: + ) -> int: """Commission a device using an IP address.""" - return await self._call_sdk_non_reentrant( - self._chip_controller.CommissionIP, - nodeid=node_id, - setupPinCode=setup_pin_code, - ipaddr=ip_addr, + return cast( + int, + await self._chip_controller.CommissionIP( + nodeid=node_id, + setupPinCode=setup_pin_code, + ipaddr=ip_addr, + ), ) async def set_wifi_credentials(self, ssid: str, credentials: str) -> None: @@ -185,9 +180,7 @@ async def unpair_device(self, node_id: int) -> PyChipError: Tries to look up the device attached to our controller with the given remote node id and ask it to remove Fabric. """ - return await self._call_sdk_non_reentrant( - self._chip_controller.UnpairDevice, nodeid=node_id - ) + return await self._chip_controller.UnpairDevice(nodeid=node_id) async def open_commissioning_window( self, @@ -199,8 +192,7 @@ async def open_commissioning_window( ) -> CommissioningParameters: """Open a commissioning window to commission a device present on this controller to another.""" async with self._get_node_lock(node_id): - return await self._call_sdk_non_reentrant( - self._chip_controller.OpenCommissioningWindow, + return await self._chip_controller.OpenCommissioningWindow( nodeid=node_id, timeout=timeout, iteration=iteration, diff --git a/pyproject.toml b/pyproject.toml index 3327ba7a..e59752e1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ dependencies = [ "async-timeout", "coloredlogs", "orjson", - "home-assistant-chip-clusters==2024.6.1", + "home-assistant-chip-clusters==2024.6.2", ] description = "Python Matter WebSocket Server" license = {text = "Apache-2.0"} @@ -39,7 +39,7 @@ server = [ "cryptography==42.0.8", "orjson==3.10.5", "zeroconf==0.132.2", - "home-assistant-chip-core==2024.6.1", + "home-assistant-chip-core==2024.6.2", ] test = [ "codespell==2.3.0",