From 27583ff5a6ec89e8f5f1244f287b61240376203a Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Mon, 27 Jul 2026 12:56:15 -0400 Subject: [PATCH 1/8] API to subscribe to multicast groups --- bellows/multicast.py | 2 +- bellows/zigbee/application.py | 14 ++++++++++++ tests/test_application.py | 40 +++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/bellows/multicast.py b/bellows/multicast.py index 1e7955d0..0b0e2348 100644 --- a/bellows/multicast.py +++ b/bellows/multicast.py @@ -80,7 +80,7 @@ async def unsubscribe(self, group_id) -> t.sl_Status: try: entry, idx = self._multicast[group_id] except KeyError: - LOGGER.error( + LOGGER.debug( "Couldn't find MulticastTableEntry for %s multicast_id", group_id ) return t.sl_Status.INVALID_INDEX diff --git a/bellows/zigbee/application.py b/bellows/zigbee/application.py index f64254ac..4153a836 100644 --- a/bellows/zigbee/application.py +++ b/bellows/zigbee/application.py @@ -1131,6 +1131,20 @@ async def permit_with_link_key( return await super().permit(time_s) + async def _subscribe_to_multicast_group(self, group_id: t.Group) -> None: + """Ask the coordinator firmware to subscribe to a group, if needed.""" + if self._multicast is None: + return None + + await self._multicast.subscribe(group_id) + + async def _unsubscribe_from_multicast_group(self, group_id: t.Group) -> None: + """Ask the coordinator firmware to unsubscribe from a group, if needed.""" + if self._multicast is None: + return None + + await self._multicast.unsubscribe(group_id) + def _handle_id_conflict(self, nwk: t.EmberNodeId) -> None: LOGGER.warning("NWK conflict is reported for 0x%04x", nwk) self.state.counters[COUNTERS_CTRL][COUNTER_NWK_CONFLICTS].increment() diff --git a/tests/test_application.py b/tests/test_application.py index 9170747b..934a6af5 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -24,6 +24,7 @@ GetRouteTableEntryRsp, GetTxPowerInfoRsp, ) +from bellows.multicast import Multicast import bellows.types import bellows.types as t import bellows.types.struct @@ -2683,3 +2684,42 @@ async def test_set_tx_power(app: ControllerApplication) -> None: assert result == 12.0 assert app._ezsp.setRadioPower.mock_calls == [call(power=12)] assert mock_update.mock_calls == [call(app._ezsp, tx_power=12)] + + +async def test_multicast_group_subscription(app: ControllerApplication) -> None: + """Test multicast group subscription APIs when there are no XNCP extensions.""" + app._ezsp._xncp_features = FirmwareFeatures.NONE + + app._multicast = Multicast(app._ezsp) + await app._multicast._initialize() + + # Subscribe to a group + await app.subscribe_to_multicast_group(0x1234) + assert app._ezsp._protocol.setMulticastTableEntry.mock_calls == [ + call( + 0, + t.EmberMulticastTableEntry(multicastId=0x1234, endpoint=1, networkIndex=0), + ) + ] + + app._ezsp._protocol.setMulticastTableEntry.reset_mock() + + # Unsubscribe from a group + await app.unsubscribe_from_multicast_group(0x1234) + assert app._ezsp._protocol.setMulticastTableEntry.mock_calls == [ + call( + 0, + t.EmberMulticastTableEntry(multicastId=0x1234, endpoint=0, networkIndex=0), + ) + ] + + +async def test_multicast_group_subscription_xncp(app: ControllerApplication) -> None: + """Test multicast group subscription APIs when XNCP extensions are available.""" + app._ezsp._xncp_features |= FirmwareFeatures.MEMBER_OF_ALL_GROUPS + + # Subscribe to a group (no-op) + await app.subscribe_to_multicast_group(0x1234) + + # Unsubscribe from a group (no-op) + await app.unsubscribe_from_multicast_group(0x1234) From daaf9cf1fc45aa0ce46b42e3763a2bc7bfc18c60 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Mon, 27 Jul 2026 12:56:46 -0400 Subject: [PATCH 2/8] Bump minimum zigpy version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 98db341c..84074acd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,7 @@ dependencies = [ "click", "click-log>=0.2.1", "voluptuous", - "zigpy>=0.87.0", + "zigpy>=2.1.0", ] [tool.setuptools.packages.find] From c0281d2b5bb123ce0a7d47dc94be8a77568f2e18 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:03:25 -0400 Subject: [PATCH 3/8] Ensure XNCP never touches the multicast table --- tests/test_application.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_application.py b/tests/test_application.py index 934a6af5..c035fb58 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -2723,3 +2723,6 @@ async def test_multicast_group_subscription_xncp(app: ControllerApplication) -> # Unsubscribe from a group (no-op) await app.unsubscribe_from_multicast_group(0x1234) + + # The multicast table was never touched + assert len(app._ezsp._protocol.setMulticastTableEntry.mock_calls) == 0 From 38f2043d3a88509d92e6a35f46f570201a70877d Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:58:00 -0400 Subject: [PATCH 4/8] Allow specifying `endpoint_id` --- bellows/multicast.py | 88 ++++++++++++++++++----------------- bellows/zigbee/application.py | 12 +++-- 2 files changed, 53 insertions(+), 47 deletions(-) diff --git a/bellows/multicast.py b/bellows/multicast.py index 0b0e2348..8b5e21eb 100644 --- a/bellows/multicast.py +++ b/bellows/multicast.py @@ -10,7 +10,7 @@ class Multicast: def __init__(self, ezsp): self._ezsp = ezsp - self._multicast = {} + self._multicast: dict[int, int] = {} self._available = set() async def _initialize(self) -> None: @@ -30,7 +30,7 @@ async def _initialize(self) -> None: continue LOGGER.debug("MulticastTableEntry[%s] = %s", i, entry) if entry.endpoint != 0: - self._multicast[entry.multicastId] = (entry, i) + self._multicast[entry.multicastId, entry.endpoint] = (entry, i) else: self._available.add(i) @@ -42,21 +42,15 @@ async def startup(self, coordinator) -> None: for group_id in ep.member_of: await self.subscribe(group_id) - async def subscribe(self, group_id) -> t.sl_Status: - if group_id in self._multicast: - LOGGER.debug("%s is already subscribed", t.EmberMulticastId(group_id)) - return t.sl_Status.OK - - try: - idx = self._available.pop() - except KeyError: - LOGGER.error("No more available slots MulticastId subscription") - return t.sl_Status.INVALID_INDEX + async def _set_multicast_entry( + self, idx: int, group_id: int, endpoint_id: int + ) -> tuple[t.sl_Status, t.EmberMulticastTableEntry]: entry = t.EmberMulticastTableEntry() - entry.endpoint = t.uint8_t(1) + entry.endpoint = t.uint8_t(endpoint_id) entry.multicastId = t.EmberMulticastId(group_id) entry.networkIndex = t.uint8_t(0) status = await self._ezsp.setMulticastTableEntry(idx, entry) + if t.sl_Status.from_ember_status(status[0]) != t.sl_Status.OK: LOGGER.warning( "Set MulticastTableEntry #%s for %s multicast id: %s", @@ -64,44 +58,52 @@ async def subscribe(self, group_id) -> t.sl_Status: entry.multicastId, status, ) - self._available.add(idx) - return status[0] - - self._multicast[entry.multicastId] = (entry, idx) - LOGGER.debug( - "Set MulticastTableEntry #%s for %s multicast id: %s", - idx, - entry.multicastId, - status, + else: + LOGGER.debug( + "Set MulticastTableEntry #%s for %s multicast id %s for endpoint %d: %s", + idx, + entry.multicastId, + entry.endpoint, + status, + ) + + return status[0], entry + + async def subscribe(self, group_id: int, endpoint_id: int = 1) -> t.sl_Status: + if (group_id, endpoint_id) in self._multicast: + LOGGER.debug("%s is already subscribed", t.EmberMulticastId(group_id)) + return t.sl_Status.OK + + try: + idx = self._available.pop() + except KeyError: + LOGGER.error("No more available slots MulticastId subscription") + return t.sl_Status.INVALID_INDEX + + status, entry = await self._set_multicast_entry( + idx=idx, group_id=group_id, endpoint_id=endpoint_id ) - return status[0] - async def unsubscribe(self, group_id) -> t.sl_Status: + self._multicast[entry.multicastId, entry.endpoint] = (entry, idx) + + return status + + async def unsubscribe(self, group_id: int, endpoint_id: int = 1) -> t.sl_Status: try: - entry, idx = self._multicast[group_id] + _entry, idx = self._multicast[group_id, endpoint_id] except KeyError: LOGGER.debug( "Couldn't find MulticastTableEntry for %s multicast_id", group_id ) return t.sl_Status.INVALID_INDEX - entry.endpoint = t.uint8_t(0) - status = await self._ezsp.setMulticastTableEntry(idx, entry) - if t.sl_Status.from_ember_status(status[0]) != t.sl_Status.OK: - LOGGER.warning( - "Set MulticastTableEntry #%s for %s multicast id: %s", - idx, - entry.multicastId, - status, - ) - return status[0] + status, _entry = await self._set_multicast_entry( + idx=idx, + group_id=group_id, + endpoint_id=0, + ) - self._multicast.pop(group_id) + self._multicast.pop((group_id, endpoint_id)) self._available.add(idx) - LOGGER.debug( - "Set MulticastTableEntry #%s for %s multicast id: %s", - idx, - entry.multicastId, - status, - ) - return status[0] + + return status diff --git a/bellows/zigbee/application.py b/bellows/zigbee/application.py index 4153a836..92506f8f 100644 --- a/bellows/zigbee/application.py +++ b/bellows/zigbee/application.py @@ -1131,19 +1131,23 @@ async def permit_with_link_key( return await super().permit(time_s) - async def _subscribe_to_multicast_group(self, group_id: t.Group) -> None: + async def _subscribe_to_multicast_group( + self, group_id: t.Group, endpoint_id: int + ) -> None: """Ask the coordinator firmware to subscribe to a group, if needed.""" if self._multicast is None: return None - await self._multicast.subscribe(group_id) + await self._multicast.subscribe(group_id=group_id, endpoint_id=endpoint_id) - async def _unsubscribe_from_multicast_group(self, group_id: t.Group) -> None: + async def _unsubscribe_from_multicast_group( + self, group_id: t.Group, endpoint_id: int + ) -> None: """Ask the coordinator firmware to unsubscribe from a group, if needed.""" if self._multicast is None: return None - await self._multicast.unsubscribe(group_id) + await self._multicast.unsubscribe(group_id=group_id, endpoint_id=endpoint_id) def _handle_id_conflict(self, nwk: t.EmberNodeId) -> None: LOGGER.warning("NWK conflict is reported for 0x%04x", nwk) From 75f4667293bcf9f81189a98b242e432fedd321be Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:28:52 -0400 Subject: [PATCH 5/8] Fix tests --- bellows/multicast.py | 18 ++++++++++++------ tests/test_multicast.py | 12 ++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/bellows/multicast.py b/bellows/multicast.py index 8b5e21eb..4705ee1f 100644 --- a/bellows/multicast.py +++ b/bellows/multicast.py @@ -49,9 +49,11 @@ async def _set_multicast_entry( entry.endpoint = t.uint8_t(endpoint_id) entry.multicastId = t.EmberMulticastId(group_id) entry.networkIndex = t.uint8_t(0) - status = await self._ezsp.setMulticastTableEntry(idx, entry) - if t.sl_Status.from_ember_status(status[0]) != t.sl_Status.OK: + (status,) = await self._ezsp.setMulticastTableEntry(idx, entry) + status = t.sl_Status.from_ember_status(status) + + if status != t.sl_Status.OK: LOGGER.warning( "Set MulticastTableEntry #%s for %s multicast id: %s", idx, @@ -67,7 +69,7 @@ async def _set_multicast_entry( status, ) - return status[0], entry + return status, entry async def subscribe(self, group_id: int, endpoint_id: int = 1) -> t.sl_Status: if (group_id, endpoint_id) in self._multicast: @@ -84,7 +86,10 @@ async def subscribe(self, group_id: int, endpoint_id: int = 1) -> t.sl_Status: idx=idx, group_id=group_id, endpoint_id=endpoint_id ) - self._multicast[entry.multicastId, entry.endpoint] = (entry, idx) + if status is t.sl_Status.OK: + self._multicast[entry.multicastId, entry.endpoint] = (entry, idx) + else: + self._available.add(idx) return status @@ -103,7 +108,8 @@ async def unsubscribe(self, group_id: int, endpoint_id: int = 1) -> t.sl_Status: endpoint_id=0, ) - self._multicast.pop((group_id, endpoint_id)) - self._available.add(idx) + if status is t.sl_Status.OK: + self._multicast.pop((group_id, endpoint_id)) + self._available.add(idx) return status diff --git a/tests/test_multicast.py b/tests/test_multicast.py index c08e0685..ef44bf6d 100644 --- a/tests/test_multicast.py +++ b/tests/test_multicast.py @@ -115,14 +115,14 @@ async def test_subscribe(multicast): set_entry = multicast._ezsp.setMulticastTableEntry assert set_entry.call_count == 1 assert set_entry.call_args[0][1].multicastId == grp_id - assert grp_id in multicast._multicast + assert (grp_id, 1) in multicast._multicast set_entry.reset_mock() ret = await _subscribe(multicast, grp_id, success=True) assert ret == t.EmberStatus.SUCCESS set_entry = multicast._ezsp.setMulticastTableEntry assert set_entry.call_count == 0 - assert grp_id in multicast._multicast + assert (grp_id, 1) in multicast._multicast async def test_subscribe_fail(multicast): @@ -134,7 +134,7 @@ async def test_subscribe_fail(multicast): set_entry = multicast._ezsp.setMulticastTableEntry assert set_entry.call_count == 1 assert set_entry.call_args[0][1].multicastId == grp_id - assert grp_id not in multicast._multicast + assert (grp_id, 1) not in multicast._multicast assert len(multicast._available) == 1 @@ -167,7 +167,7 @@ async def test_unsubscribe(multicast): assert ret == t.EmberStatus.SUCCESS set_entry = multicast._ezsp.setMulticastTableEntry assert set_entry.call_count == 1 - assert grp_id not in multicast._multicast + assert (grp_id, 1) not in multicast._multicast assert len(multicast._available) == 1 multicast._ezsp.setMulticastTableEntry.reset_mock() @@ -175,7 +175,7 @@ async def test_unsubscribe(multicast): assert ret != t.EmberStatus.SUCCESS set_entry = multicast._ezsp.setMulticastTableEntry assert set_entry.call_count == 0 - assert grp_id not in multicast._multicast + assert (grp_id, 1) not in multicast._multicast assert len(multicast._available) == 1 @@ -190,5 +190,5 @@ async def test_unsubscribe_fail(multicast): assert ret != t.EmberStatus.SUCCESS set_entry = multicast._ezsp.setMulticastTableEntry assert set_entry.call_count == 1 - assert grp_id in multicast._multicast + assert (grp_id, 1) in multicast._multicast assert len(multicast._available) == 0 From f97a8473ebfc8d9581138f37833a83b671dd6fe0 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:44:00 -0400 Subject: [PATCH 6/8] Fail on bad log formatting --- tests/conftest.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..7f08caf6 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,29 @@ +"""Common pytest fixtures for all tests.""" + +import logging + +import pytest + + +class FailOnBadFormattingHandler(logging.Handler): + def emit(self, record): + try: + record.msg % record.args + except Exception as e: # noqa: BLE001 + pytest.fail( + f"Failed to format log message {record.msg!r} with {record.args!r}: {e}" + ) + + +@pytest.fixture(autouse=True) +def raise_on_bad_log_formatting(): + handler = FailOnBadFormattingHandler() + + root = logging.getLogger() + root.addHandler(handler) + root.setLevel(logging.DEBUG) + + try: + yield + finally: + root.removeHandler(handler) From c9bc64df2f387d7b9087c91e2980dbf4dab43a7a Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:45:17 -0400 Subject: [PATCH 7/8] Fix bad log formatting --- bellows/multicast.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/bellows/multicast.py b/bellows/multicast.py index 4705ee1f..73c36c3f 100644 --- a/bellows/multicast.py +++ b/bellows/multicast.py @@ -53,17 +53,20 @@ async def _set_multicast_entry( (status,) = await self._ezsp.setMulticastTableEntry(idx, entry) status = t.sl_Status.from_ember_status(status) - if status != t.sl_Status.OK: - LOGGER.warning( - "Set MulticastTableEntry #%s for %s multicast id: %s", + if status is t.sl_Status.OK: + LOGGER.debug( + "Set MulticastTableEntry #%s for %s multicast id %s for endpoint %d: %s", idx, + group_id, entry.multicastId, + entry.endpoint, status, ) else: - LOGGER.debug( - "Set MulticastTableEntry #%s for %s multicast id %s for endpoint %d: %s", + LOGGER.warning( + "Failed to set MulticastTableEntry #%s for %s multicast id %s for endpoint %d: %s", idx, + group_id, entry.multicastId, entry.endpoint, status, From f7a8520e090b80b831e81bc3131c53ab398d467b Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:47:32 -0400 Subject: [PATCH 8/8] Address review comments --- bellows/multicast.py | 4 +++- bellows/zigbee/application.py | 4 ++-- tests/test_application.py | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/bellows/multicast.py b/bellows/multicast.py index 73c36c3f..a1fe3634 100644 --- a/bellows/multicast.py +++ b/bellows/multicast.py @@ -10,7 +10,9 @@ class Multicast: def __init__(self, ezsp): self._ezsp = ezsp - self._multicast: dict[int, int] = {} + self._multicast: dict[ + tuple[int, int], tuple[t.EmberMulticastTableEntry, int] + ] = {} self._available = set() async def _initialize(self) -> None: diff --git a/bellows/zigbee/application.py b/bellows/zigbee/application.py index 92506f8f..c8870b86 100644 --- a/bellows/zigbee/application.py +++ b/bellows/zigbee/application.py @@ -1132,7 +1132,7 @@ async def permit_with_link_key( return await super().permit(time_s) async def _subscribe_to_multicast_group( - self, group_id: t.Group, endpoint_id: int + self, group_id: zigpy.types.Group, endpoint_id: int ) -> None: """Ask the coordinator firmware to subscribe to a group, if needed.""" if self._multicast is None: @@ -1141,7 +1141,7 @@ async def _subscribe_to_multicast_group( await self._multicast.subscribe(group_id=group_id, endpoint_id=endpoint_id) async def _unsubscribe_from_multicast_group( - self, group_id: t.Group, endpoint_id: int + self, group_id: zigpy.types.Group, endpoint_id: int ) -> None: """Ask the coordinator firmware to unsubscribe from a group, if needed.""" if self._multicast is None: diff --git a/tests/test_application.py b/tests/test_application.py index c035fb58..890ab2ab 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -2717,6 +2717,7 @@ async def test_multicast_group_subscription(app: ControllerApplication) -> None: async def test_multicast_group_subscription_xncp(app: ControllerApplication) -> None: """Test multicast group subscription APIs when XNCP extensions are available.""" app._ezsp._xncp_features |= FirmwareFeatures.MEMBER_OF_ALL_GROUPS + assert app._multicast is None # Subscribe to a group (no-op) await app.subscribe_to_multicast_group(0x1234)