From 52f4409a1465823a46d896b59d75b5d3d676f586 Mon Sep 17 00:00:00 2001 From: zigpy-review-bot <286747149+zigpy-review-bot@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:42:14 +0100 Subject: [PATCH 1/2] Fit the Green Power endpoint into two-slot firmware zigpy 2.4.0 registers the Green Power endpoint 242 next to the HA (1) and ZLL (2) endpoints, but ConBee II and RaspBee II firmware only has two endpoint slots. When both slots already held zigpy's endpoints 1 and 2, `add_endpoint` raised `No available endpoint slots exist` and startup failed on every attempt. Otherwise, a written slot was never marked as used, so endpoint 242 overwrote endpoint 2 and slot 1 was rewritten twice on every start. Register all endpoints at once instead: when they don't fit, drop the ZLL endpoint (nothing is sent from it), the same layout deCONZ and Z2M use, keep endpoints that are already configured and only write the rest. --- tests/test_application.py | 111 ++++++++++++++++++++++++++++- zigpy_deconz/zigbee/application.py | 105 +++++++++++++++++++-------- 2 files changed, 183 insertions(+), 33 deletions(-) diff --git a/tests/test_application.py b/tests/test_application.py index 90ea531..3f3fff5 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -514,8 +514,6 @@ async def read_param(param_id, index): app._api.read_parameter = AsyncMock(side_effect=read_param) app._api.write_parameter = AsyncMock() - app._written_endpoints.add(0x00) - app._written_endpoints.add(0x01) with pytest.raises(ValueError): await app.add_endpoint(ENDPOINT.replace(endpoint=1)) @@ -552,6 +550,115 @@ async def read_param(param_id, index): ) +def mock_endpoint_slots(app, slots): + """Mock the firmware's endpoint slots, keeping track of writes.""" + + async def read_param(param_id, index): + assert param_id == deconz_api.NetworkParameter.configure_endpoint + + if index not in slots: + raise zigpy_deconz.exception.CommandError( + "Unsupported", + status=deconz_api.Status.UNSUPPORTED, + command=None, + ) + + return deconz_api.IndexedEndpoint(index=index, descriptor=slots[index]) + + async def write_param(param_id, value): + assert param_id == deconz_api.NetworkParameter.configure_endpoint + assert value.index in slots + slots[value.index] = value.descriptor + + app._api.read_parameter = AsyncMock(side_effect=read_param) + app._api.write_parameter = AsyncMock(side_effect=write_param) + + +async def zigpy_endpoints(app): + """Return the endpoints zigpy registers: HA, ZLL and Green Power.""" + with patch.object(app, "add_endpoint", AsyncMock()) as add_endpoint: + await zigpy.application.ControllerApplication.register_endpoints(app) + + return [call.args[0] for call in add_endpoint.await_args_list] + + +@pytest.mark.parametrize( + "initial, expected, num_writes", + [ + # Two slots last configured by zigpy 2.1-2.3: the ZLL endpoint makes room + (["ha", "zll"], ["ha", "gp"], 1), + # Two slots already holding the HA and GP endpoints are left alone + (["ha", "gp"], ["ha", "gp"], 0), + # Two slots configured by something else entirely + (["other1", "other2"], ["ha", "gp"], 2), + # Firmware with more slots keeps the ZLL endpoint + (["ha", "zll", "other1", "other2"], ["ha", "zll", "gp", "other2"], 1), + (["ha", "zll", "gp"], ["ha", "zll", "gp"], 0), + ], +) +async def test_register_endpoints(app, initial, expected, num_writes): + ha, zll, gp = await zigpy_endpoints(app) + named = { + "ha": ha, + "zll": zll, + "gp": gp, + "other1": ENDPOINT.replace(endpoint=10), + "other2": ENDPOINT.replace(endpoint=11), + } + + slots = {index: named[name] for index, name in enumerate(initial)} + mock_endpoint_slots(app, slots) + + await app.register_endpoints() + + assert slots == {index: named[name] for index, name in enumerate(expected)} + assert app._api.write_parameter.await_count == num_writes + + # Registering again, e.g. when `start_network` is retried, writes nothing + app._api.write_parameter.reset_mock() + await app.register_endpoints() + assert app._api.write_parameter.await_count == 0 + + +async def test_register_endpoints_too_many(app): + ha, zll, gp = await zigpy_endpoints(app) + slots = {0: ha, 1: zll} + mock_endpoint_slots(app, slots) + + with patch.dict( + app._config, + {zigpy.config.CONF_ADDITIONAL_ENDPOINTS: [ENDPOINT.replace(endpoint=10)]}, + ): + with pytest.raises(ValueError): + await app.register_endpoints() + + assert slots == {0: ha, 1: zll} + app._api.write_parameter.assert_not_called() + + +async def test_add_endpoint_after_register(app): + ha, zll, gp = await zigpy_endpoints(app) + slots = {0: ha, 1: zll, 2: gp, 3: ENDPOINT.replace(endpoint=10)} + mock_endpoint_slots(app, slots) + + await app.register_endpoints() + app._api.write_parameter.assert_not_called() + + # A new endpoint goes into the free slot, not one of the registered ones + await app.add_endpoint(ENDPOINT.replace(endpoint=20)) + assert slots == {0: ha, 1: zll, 2: gp, 3: ENDPOINT.replace(endpoint=20)} + + # Replacing an endpoint with the same ID reuses its slot + await app.add_endpoint(ENDPOINT.replace(endpoint=20, profile=1234)) + assert slots == { + 0: ha, + 1: zll, + 2: gp, + 3: ENDPOINT.replace(endpoint=20, profile=1234), + } + assert app._api.write_parameter.await_count == 2 + + async def test_reset_network_info(app): app.write_network_info = AsyncMock() await app.reset_network_info() diff --git a/zigpy_deconz/zigbee/application.py b/zigpy_deconz/zigbee/application.py index 45d6e23..e9bed3a 100644 --- a/zigpy_deconz/zigbee/application.py +++ b/zigpy_deconz/zigbee/application.py @@ -19,6 +19,7 @@ import zigpy.endpoint import zigpy.exceptions from zigpy.exceptions import FormationFailure, NetworkNotFormed +import zigpy.profiles.zll import zigpy.state import zigpy.types import zigpy.util @@ -79,7 +80,8 @@ def __init__(self, config: dict[str, Any]): self._delayed_neighbor_scan_task = None self._reconnect_task = None - self._written_endpoints = set() + self._endpoints: list[zdo_t.SimpleDescriptor] = [] + self._pending_endpoints: list[zdo_t.SimpleDescriptor] | None = None async def _watchdog_feed(self): if self._api.protocol_version >= PROTO_VER_WATCHDOG and not ( @@ -102,7 +104,7 @@ async def connect(self): raise self._api = api - self._written_endpoints.clear() + self._endpoints = [] async def disconnect(self): if self._delayed_neighbor_scan_task is not None: @@ -492,56 +494,97 @@ async def _move_network_to_channel( await asyncio.sleep(CHANGE_NETWORK_STATE_DELAY) await self._change_network_state(NetworkState.CONNECTED) + async def register_endpoints(self) -> None: + """Register all endpoints at once, so they can be fit into the available slots.""" + self._pending_endpoints = [] + + try: + await super().register_endpoints() + descriptors = self._pending_endpoints + finally: + self._pending_endpoints = None + + await self._configure_endpoints(descriptors) + async def add_endpoint(self, descriptor: zdo_t.SimpleDescriptor) -> None: """Register an endpoint on the device, replacing any with conflicting IDs.""" + if self._pending_endpoints is not None: + self._pending_endpoints.append(descriptor) + return - endpoints = {} + await self._configure_endpoints( + [ep for ep in self._endpoints if ep.endpoint != descriptor.endpoint] + + [descriptor] + ) - # Read and count the current endpoints. Some firmwares have three, others four. + async def _read_endpoint_slots(self) -> dict[int, zdo_t.SimpleDescriptor]: + """Read the endpoint slots of the firmware, they can't be queried otherwise.""" + slots = {} + + # Most firmwares have two slots, a ConBee II test firmware had four for index in range(255 + 1): try: - current_descriptor = await self._api.read_parameter( + current = await self._api.read_parameter( NetworkParameter.configure_endpoint, index ) except zigpy_deconz.exception.CommandError as ex: assert ex.status == Status.UNSUPPORTED break else: - endpoints[index] = current_descriptor.descriptor + slots[index] = current.descriptor - LOGGER.debug("Got endpoint slots: %r", endpoints) + LOGGER.debug("Got endpoint slots: %r", slots) - # Don't write endpoints unnecessarily - if descriptor in endpoints.values(): - LOGGER.debug("Endpoint already registered, skipping") + return slots - # Pretend we wrote it - self._written_endpoints.add(list(endpoints.values()).index(descriptor)) - return - - # Keep track of the best endpoint descriptor to replace - target_index = None + async def _configure_endpoints( + self, descriptors: list[zdo_t.SimpleDescriptor] + ) -> None: + """Fit the endpoints into the slots, writing only the ones that changed.""" + slots = await self._read_endpoint_slots() + + # Two-slot firmware cannot host the ZLL endpoint next to the HA and Green Power + # ones. Nothing is sent from it, so drop it like deCONZ and Z2M do. + if len(descriptors) > len(slots): + LOGGER.debug("Not enough endpoint slots, skipping the ZLL endpoint") + descriptors = [ + ep for ep in descriptors if ep.profile != zigpy.profiles.zll.PROFILE_ID + ] + + if len(descriptors) > len(slots): + raise ValueError( + f"Cannot fit {len(descriptors)} endpoints into {len(slots)} endpoint" + f" slots: {descriptors!r}" + ) - for index, current_descriptor in endpoints.items(): - # Ignore ones we've already written - if index in self._written_endpoints: - continue + assigned: dict[int, zdo_t.SimpleDescriptor] = {} + missing: list[zdo_t.SimpleDescriptor] = [] - target_index = index + # Don't write endpoints unnecessarily + for descriptor in descriptors: + for index, current in slots.items(): + if index not in assigned and current == descriptor: + assigned[index] = descriptor + break + else: + missing.append(descriptor) - if current_descriptor.endpoint == descriptor.endpoint: - # Prefer to replace the endpoint with the same ID - break + free = [index for index in slots if index not in assigned] - if target_index is None: - raise ValueError(f"No available endpoint slots exist: {endpoints!r}") + for descriptor in missing: + # Prefer to replace the endpoint with the same ID + index = next( + (i for i in free if slots[i].endpoint == descriptor.endpoint), free[0] + ) + free.remove(index) - LOGGER.debug("Writing %s to slot %r", descriptor, target_index) + LOGGER.debug("Writing %s to slot %r", descriptor, index) + await self._api.write_parameter( + NetworkParameter.configure_endpoint, + IndexedEndpoint(index=index, descriptor=descriptor), + ) - await self._api.write_parameter( - NetworkParameter.configure_endpoint, - IndexedEndpoint(index=target_index, descriptor=descriptor), - ) + self._endpoints = list(descriptors) async def send_packet(self, packet): LOGGER.debug("Sending packet: %r", packet) From 7d9b369e6b8ef8025cd59c4f8e6c2e7b8e591ee1 Mon Sep 17 00:00:00 2001 From: zigpy-review-bot <286747149+zigpy-review-bot@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:53:47 +0100 Subject: [PATCH 2/2] Only drop zigpy's ZLL endpoint and merge endpoints with the same ID A configured endpoint with the ID of one of zigpy's replaces it, as a standalone `add_endpoint` call already did, instead of taking a slot of its own. Only zigpy's ZLL endpoint 2 makes room on two-slot firmware, a configured ZLL endpoint is kept. --- tests/test_application.py | 30 ++++++++++++++++++++++++++++++ zigpy_deconz/zigbee/application.py | 29 ++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/tests/test_application.py b/tests/test_application.py index 3f3fff5..4f29dac 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -8,6 +8,7 @@ import zigpy.application import zigpy.config import zigpy.device +import zigpy.profiles.zll from zigpy.types import EUI64, Channels, KeyData import zigpy.zdo.types as zdo_t @@ -636,6 +637,35 @@ async def test_register_endpoints_too_many(app): app._api.write_parameter.assert_not_called() +async def test_register_endpoints_additional_zll(app): + """Only zigpy's own ZLL endpoint makes room, a configured one is kept.""" + ha, zll, gp = await zigpy_endpoints(app) + slots = {0: ha, 1: zll} + mock_endpoint_slots(app, slots) + + extra = ENDPOINT.replace(endpoint=3, profile=zigpy.profiles.zll.PROFILE_ID) + + with patch.dict(app._config, {zigpy.config.CONF_ADDITIONAL_ENDPOINTS: [extra]}): + with pytest.raises(ValueError): + await app.register_endpoints() + + assert slots == {0: ha, 1: zll} + + +async def test_register_endpoints_override(app): + """A configured endpoint replaces zigpy's endpoint with the same ID.""" + ha, zll, gp = await zigpy_endpoints(app) + slots = {0: ha, 1: zll, 2: gp, 3: ENDPOINT.replace(endpoint=10)} + mock_endpoint_slots(app, slots) + + override = ha.replace(device_type=0x1234) + + with patch.dict(app._config, {zigpy.config.CONF_ADDITIONAL_ENDPOINTS: [override]}): + await app.register_endpoints() + + assert slots == {0: override, 1: zll, 2: gp, 3: ENDPOINT.replace(endpoint=10)} + + async def test_add_endpoint_after_register(app): ha, zll, gp = await zigpy_endpoints(app) slots = {0: ha, 1: zll, 2: gp, 3: ENDPOINT.replace(endpoint=10)} diff --git a/zigpy_deconz/zigbee/application.py b/zigpy_deconz/zigbee/application.py index e9bed3a..5ee5a40 100644 --- a/zigpy_deconz/zigbee/application.py +++ b/zigpy_deconz/zigbee/application.py @@ -58,6 +58,9 @@ CONBEE_III_ENERGY_SCAN_ATTEMPTS = 5 +# zigpy registers its ZLL endpoint here, see `ControllerApplication.register_endpoints` +ZLL_ENDPOINT_ID = 2 + class ControllerApplication(zigpy.application.ControllerApplication): SCHEMA = CONFIG_SCHEMA @@ -512,10 +515,7 @@ async def add_endpoint(self, descriptor: zdo_t.SimpleDescriptor) -> None: self._pending_endpoints.append(descriptor) return - await self._configure_endpoints( - [ep for ep in self._endpoints if ep.endpoint != descriptor.endpoint] - + [descriptor] - ) + await self._configure_endpoints([*self._endpoints, descriptor]) async def _read_endpoint_slots(self) -> dict[int, zdo_t.SimpleDescriptor]: """Read the endpoint slots of the firmware, they can't be queried otherwise.""" @@ -543,14 +543,25 @@ async def _configure_endpoints( """Fit the endpoints into the slots, writing only the ones that changed.""" slots = await self._read_endpoint_slots() - # Two-slot firmware cannot host the ZLL endpoint next to the HA and Green Power - # ones. Nothing is sent from it, so drop it like deCONZ and Z2M do. + # A later endpoint replaces an earlier one with the same ID + descriptors = list({ep.endpoint: ep for ep in descriptors}.values()) + + # Two-slot firmware cannot host zigpy's ZLL endpoint next to the HA and Green + # Power ones. Nothing is sent from it, so drop it like deCONZ and Z2M do. if len(descriptors) > len(slots): - LOGGER.debug("Not enough endpoint slots, skipping the ZLL endpoint") - descriptors = [ - ep for ep in descriptors if ep.profile != zigpy.profiles.zll.PROFILE_ID + fitting = [ + ep + for ep in descriptors + if not ( + ep.endpoint == ZLL_ENDPOINT_ID + and ep.profile == zigpy.profiles.zll.PROFILE_ID + ) ] + if len(fitting) < len(descriptors): + LOGGER.debug("Not enough endpoint slots, skipping the ZLL endpoint") + descriptors = fitting + if len(descriptors) > len(slots): raise ValueError( f"Cannot fit {len(descriptors)} endpoints into {len(slots)} endpoint"