From 81d552a0478b216df8e62e8565c7247a69af1251 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Thu, 3 Apr 2025 15:14:54 +0200 Subject: [PATCH 1/2] Improve mDNS browsing to find operational nodes Use the fabric specific DNS-SD service subtype instead of the generic "_matter._tcp" service type. This avoids querying for operational nodes of other fabrics. This is closer to the recommendations in "4.3.2.6. Performance Recommendations" of the Matter spec. While at it, remove the commissionable node browsing as it is not used currently. --- matter_server/server/device_controller.py | 43 ++++++----------------- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/matter_server/server/device_controller.py b/matter_server/server/device_controller.py index da791cc1..4da55221 100644 --- a/matter_server/server/device_controller.py +++ b/matter_server/server/device_controller.py @@ -24,7 +24,12 @@ from chip.discovery import DiscoveryType from chip.exceptions import ChipStackError from chip.native import PyChipError -from zeroconf import BadTypeInNameException, IPVersion, ServiceStateChange, Zeroconf +from zeroconf import ( + DNSQuestionType, + IPVersion, + ServiceStateChange, + Zeroconf, +) from zeroconf.asyncio import AsyncServiceBrowser, AsyncServiceInfo, AsyncZeroconf from matter_server.common.const import VERBOSE_LOG_LEVEL @@ -215,11 +220,14 @@ async def start(self) -> None: LOGGER.info("Loaded %s nodes from stored configuration", len(self._nodes)) # set-up mdns browser self._aiozc = AsyncZeroconf(ip_version=IPVersion.All) - services = [MDNS_TYPE_OPERATIONAL_NODE, MDNS_TYPE_COMMISSIONABLE_NODE] + services = [ + f"_I{self.compressed_fabric_id:0{16}X}._sub.{MDNS_TYPE_OPERATIONAL_NODE}" + ] self._aiobrowser = AsyncServiceBrowser( self._aiozc.zeroconf, services, handlers=[self._on_mdns_service_state_change], + question_type=DNSQuestionType.QM, ) async def stop(self) -> None: @@ -1510,13 +1518,6 @@ def _on_mdns_service_state_change( # We already have a timer to resolve this service, so ignore this callback. return - if service_type == MDNS_TYPE_COMMISSIONABLE_NODE: - # process the event with a debounce timer - self._mdns_event_timer[name] = self._loop.call_later( - 0.5, self._on_mdns_commissionable_node_state, name, state_change - ) - return - if service_type == MDNS_TYPE_OPERATIONAL_NODE: if self._fabric_id_hex is None or self._fabric_id_hex not in name.lower(): # filter out messages that are not for our fabric @@ -1566,30 +1567,6 @@ def _on_mdns_operational_node_state( ) ) - def _on_mdns_commissionable_node_state( - self, name: str, state_change: ServiceStateChange - ) -> None: - """Handle a (commissionable) Matter node MDNS state change.""" - self._mdns_event_timer.pop(name, None) - logger = LOGGER.getChild("mdns") - - try: - info = AsyncServiceInfo(MDNS_TYPE_COMMISSIONABLE_NODE, name) - except BadTypeInNameException as ex: - logger.debug("Ignoring record with bad type in name: %s: %s", name, ex) - return - - async def handle_commissionable_node_added() -> None: - if TYPE_CHECKING: - assert self._aiozc is not None - await info.async_request(self._aiozc.zeroconf, 3000) - logger.debug("Discovered commissionable Matter node: %s", info) - - if state_change == ServiceStateChange.Added: - asyncio.create_task(handle_commissionable_node_added()) - elif state_change == ServiceStateChange.Removed: - logger.debug("Commissionable Matter node disappeared: %s", info) - def _write_node_state(self, node_id: int, force: bool = False) -> None: """Schedule the write of the current node state to persistent storage.""" if node_id not in self._nodes: From df569148e8bd300c7bd9f24b1feaf6c5ff948a04 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Thu, 3 Apr 2025 15:46:12 +0200 Subject: [PATCH 2/2] Mock ChipDeviceControllerWrapper The wrapper is how we call the CHIP SDK. We don't actually want to call the SDK, so let's start mocking on the wrapper level. --- tests/server/test_server.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/server/test_server.py b/tests/server/test_server.py index 1afedd86..06f56865 100644 --- a/tests/server/test_server.py +++ b/tests/server/test_server.py @@ -3,12 +3,13 @@ from __future__ import annotations from typing import TYPE_CHECKING -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, create_autospec, patch import pytest from matter_server.common.helpers.api import parse_arguments from matter_server.common.models import APICommand +from matter_server.server.sdk import ChipDeviceControllerWrapper from matter_server.server.server import MatterServer if TYPE_CHECKING: @@ -21,6 +22,7 @@ "chip_native", "chip_logging", "chip_stack", + "chip_device_controller_wrapper", "certificate_authority_manager", "storage_controller", ) @@ -74,6 +76,25 @@ def chip_stack_fixture() -> Generator[MagicMock, None, None]: yield chip_stack +@pytest.fixture(name="chip_device_controller_wrapper") +def chip_device_controller_wrapper_fixture() -> Generator[MagicMock, None, None]: + """Return a mocked chip native.""" + with patch( + "matter_server.server.device_controller.ChipDeviceControllerWrapper", + autospec=True, + ) as chip_device_controller_wrapper_class: + chip_device_controller_wrapper = create_autospec( + ChipDeviceControllerWrapper, instance=True + ) + chip_device_controller_wrapper.get_compressed_fabric_id = AsyncMock( + return_value=1234 + ) + chip_device_controller_wrapper_class.return_value = ( + chip_device_controller_wrapper + ) + yield chip_device_controller_wrapper + + @pytest.fixture(name="certificate_authority_manager") def certificate_authority_manager_fixture() -> Generator[MagicMock, None, None]: """Return a mocked certificate authority manager."""