From 1a35829c9f9b1d76ac4cd88e0a25a8c495b7cef0 Mon Sep 17 00:00:00 2001 From: Fabian <6667968+fabcoded@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:42:54 +0200 Subject: [PATCH 1/8] Validate association lookups before dropping or restoring a child `send_packet` treated every `UTIL.AssocGetWithAddress` answer except `NodeRelation.NOTUSED` as a direct child. When the device is not in the association table, Z-Stack returns `INVALID_NODE_ADDR` (0xFFFE) followed by uninitialized memory, or by zeroes (`NodeRelation.PARENT`) on Koenkk's builds. zigpy-znp then removed nothing and, once the retry failed too, sent `UTIL.AssocAdd(NWK=0xFFFE, NodeRelation=PARENT)`. Only remove and restore an entry with a valid address and one of the four child relations. For any other answer, skip `UTIL.AssocRemove` and the restore but keep the route discovery and the one retry that followed the removal; `NOTUSED` still re-raises at once. zigbee-herdsman added the same address check in Koenkk/zigbee-herdsman#685 and likewise keeps its route discovery and retries. The restore after a failed retry wrote back the address from the lookup made before the recovery, even if the device had rejoined under a new address meanwhile. `UTIL.AssocAdd` calls `AssocAddNew`, which TI documents as "create a new or update a previous association": for an IEEE that is already in the table it overwrites that entry in place (measured on Koenkk's build 20250321). Restore only if a second lookup does not find the IEEE and the host still knows the removed address once that lookup has returned, so a join handled while it is awaited also skips the restore. A join the firmware handles after the lookup's snapshot is not excluded; serializing table changes per IEEE is left out. --- tests/application/test_requests.py | 520 ++++++++++++++++++++++++++++- zigpy_znp/zigbee/application.py | 68 +++- 2 files changed, 581 insertions(+), 7 deletions(-) diff --git a/tests/application/test_requests.py b/tests/application/test_requests.py index f073cf6..58905ec 100644 --- a/tests/application/test_requests.py +++ b/tests/application/test_requests.py @@ -9,6 +9,7 @@ import zigpy_znp.commands as c import zigpy_znp.config as conf +from zigpy_znp.exceptions import CommandNotRecognized import zigpy_znp.types as t from ..conftest import ( @@ -633,9 +634,16 @@ def data_confirm_replier(req): ], ) + def assoc_get_with_addr(req): + # Once removed, the device is no longer found and the restore goes ahead + if did_assoc_remove.call_count: + return assoc_lookup_rsp(b"\x00", 0xFFFE, c.util.NodeRelation.PARENT) + + return c.UTIL.AssocGetWithAddress.Rsp(Device=assoc_device) + znp_server.reply_to( c.UTIL.AssocGetWithAddress.Req(IEEE=device.ieee, partial=True), - responses=[c.UTIL.AssocGetWithAddress.Rsp(Device=assoc_device)], + responses=[assoc_get_with_addr], ) did_assoc_remove = znp_server.reply_to( @@ -805,6 +813,516 @@ def assoc_remove(req): await app.shutdown() +def assoc_lookup_rsp(fill: bytes, nwk: int, relation: c.util.NodeRelation): + """Build a `UTIL.AssocGetWithAddress` answer, with `fill` in the other fields.""" + assoc_device, _ = c.util.Device.deserialize(fill * 100) + assoc_device.shortAddr = t.NWK(nwk) + assoc_device.nodeRelation = relation + + return c.UTIL.AssocGetWithAddress.Rsp(Device=assoc_device) + + +ASSOC_NOT_FOUND_ZEROED = assoc_lookup_rsp(b"\x00", 0xFFFE, c.util.NodeRelation.PARENT) +ASSOC_NOT_FOUND_NOTUSED = assoc_lookup_rsp(b"\xff", 0xFFFF, c.util.NodeRelation.NOTUSED) + + +class AssocTable: + """ + Association table of a simulated coordinator. It applies `UTIL.AssocRemove` and + `UTIL.AssocAdd`, answers lookups for absent devices with `not_found` (Koenkk's + builds: `0xFFFE`, then zeroes) and logs every request in order. `on_remove` runs + right after a removal, `on_lookup` once a lookup's answer is built but before + the host receives it. + """ + + def __init__(self, znp_server, entries, *, not_found=None, assoc_add=True): + self.entries = dict(entries) + self.not_found = not_found or ASSOC_NOT_FOUND_ZEROED + self.assoc_add = assoc_add + self.on_remove = None + self.on_lookup = None + self.log = [] + + znp_server.reply_to( + c.UTIL.AssocGetWithAddress.Req(partial=True), responses=[self.get] + ) + znp_server.reply_to( + c.UTIL.AssocRemove.Req(partial=True), responses=[self.remove] + ) + znp_server.reply_to(c.UTIL.AssocAdd.Req(partial=True), responses=[self.add]) + + def get(self, req): + self.log.append(("AssocGetWithAddress", req.IEEE)) + + if req.IEEE not in self.entries: + rsp = self.not_found + else: + rsp = assoc_lookup_rsp(b"\x00", *self.entries[req.IEEE]) + + if self.on_lookup is not None: + self.on_lookup() + + return rsp + + def remove(self, req): + self.log.append(("AssocRemove", req.IEEE)) + self.entries.pop(req.IEEE, None) + + if self.on_remove is not None: + self.on_remove() + + return c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS) + + def add(self, req): + self.log.append(("AssocAdd", req.IEEE, req.NWK, req.NodeRelation)) + + # Koenkk's builds 20200805 and 20200925 have `AssocRemove`, not `AssocAdd` + if not self.assoc_add: + return c.RPCError.CommandNotRecognized.Rsp( + ErrorCode=c.rpc_error.ErrorCode.InvalidCommandId, + RequestHeader=req.to_frame().header, + ) + + self.entries[req.IEEE] = (req.NWK, req.NodeRelation) + + return c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS) + + +async def request_with_failed_recovery( + app, + znp_server, + device, + during_retry=None, + statuses=(t.Status.MAC_TRANSACTION_EXPIRED, t.Status.NWK_NO_ROUTE), +): + """ + Send a request whose two attempts fail with `statuses`. By default the first + one expires, starting the association recovery, and the retry finds no route. + `during_retry` runs while the second attempt is in flight. + """ + + attempts = 0 + + async def data_request(req): + nonlocal attempts + attempts += 1 + + yield c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS) + + if attempts == 2 and during_retry is not None: + during_retry() + + yield c.AF.DataConfirm.Callback( + Status=statuses[attempts - 1], Endpoint=1, TSN=1 + ) + + znp_server.reply_to(c.AF.DataRequestExt.Req(partial=True), responses=[data_request]) + znp_server.reply_to( + c.ZDO.ExtRouteDisc.Req(partial=True), + responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], + ) + + await app.request( + device=device, + profile=260, + cluster=1, + src_ep=1, + dst_ep=1, + sequence=1, + data=b"\x00", + ) + + +@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) +@pytest.mark.parametrize( + ("lookup_rsp", "retried"), + [ + # Not found: Z-Stack returns `INVALID_NODE_ADDR`. Koenkk's firmware zeroes + # the remainder, which reads as `PARENT`. Stock firmware leaves it + # uninitialized, so it may even contain a child relation. + pytest.param( + assoc_lookup_rsp(b"\x00", 0xFFFE, c.util.NodeRelation.PARENT), + True, + id="not_found_zeroed", + ), + pytest.param( + assoc_lookup_rsp(b"\xa5", 0xFFFE, c.util.NodeRelation.CHILD_RFD), + True, + id="not_found_garbage", + ), + # Found, but not a direct child of the coordinator + pytest.param( + assoc_lookup_rsp(b"\x00", 0xABCD, c.util.NodeRelation.PARENT), + True, + id="parent", + ), + pytest.param( + assoc_lookup_rsp(b"\x00", 0xABCD, c.util.NodeRelation.NEIGHBOR), + True, + id="neighbor", + ), + pytest.param( + assoc_lookup_rsp(b"\x00", 0xABCD, c.util.NodeRelation.OTHER), + True, + id="other", + ), + # Unchanged: `NOTUSED` ends the send at once + pytest.param( + assoc_lookup_rsp(b"\xff", 0xABCD, c.util.NodeRelation.NOTUSED), + False, + id="notused", + ), + ], +) +async def test_request_recovery_assoc_not_a_child( + device_cls, lookup_rsp, retried, make_application, mocker +): + app, znp_server = make_application(server_cls=device_cls) + + await app.startup(auto_form=False) + + mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) + app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + + device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + + data_req = znp_server.reply_to( + c.AF.DataRequestExt.Req(partial=True), + responses=[ + c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS), + c.AF.DataConfirm.Callback( + Status=t.Status.MAC_TRANSACTION_EXPIRED, Endpoint=1, TSN=1 + ), + ], + ) + + did_assoc_get = znp_server.reply_to( + c.UTIL.AssocGetWithAddress.Req(IEEE=device.ieee, partial=True), + responses=[lookup_rsp], + ) + + # Match any arguments: neither command may be sent at all + did_assoc_remove = znp_server.reply_to( + c.UTIL.AssocRemove.Req(partial=True), + responses=[c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS)], + ) + + did_assoc_add = znp_server.reply_to( + c.UTIL.AssocAdd.Req(partial=True), + responses=[c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS)], + ) + + was_route_discovered = znp_server.reply_to( + c.ZDO.ExtRouteDisc.Req(Dst=device.nwk, partial=True), + responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], + ) + + with pytest.raises(DeliveryError) as exc_info: + await app.request( + device=device, + profile=260, + cluster=1, + src_ep=1, + dst_ep=1, + sequence=1, + data=b"\x00", + ) + + # The association table is left alone. Other than `NOTUSED`, the route is + # rediscovered and the send retried once, as before, then the error propagates. + assert exc_info.value.status == t.Status.MAC_TRANSACTION_EXPIRED + assert data_req.call_count == (2 if retried else 1) + assert did_assoc_get.call_count == 1 + assert did_assoc_remove.call_count == 0 + assert did_assoc_add.call_count == 0 + assert was_route_discovered.call_count == (1 if retried else 0) + + await app.shutdown() + + +@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) +@pytest.mark.parametrize( + "relation", + [ + c.util.NodeRelation.CHILD_RFD, + c.util.NodeRelation.CHILD_RFD_RX_IDLE, + c.util.NodeRelation.CHILD_FFD, + c.util.NodeRelation.CHILD_FFD_RX_IDLE, + ], +) +async def test_request_recovery_assoc_remove_child_relations( + device_cls, relation, make_application, mocker +): + app, znp_server = make_application(server_cls=device_cls) + + await app.startup(auto_form=False) + + mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) + app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + + device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + + data_req = znp_server.reply_to( + c.AF.DataRequestExt.Req(partial=True), + responses=[ + c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS), + c.AF.DataConfirm.Callback( + Status=t.Status.MAC_TRANSACTION_EXPIRED, Endpoint=1, TSN=1 + ), + ], + ) + + table = AssocTable(znp_server, {device.ieee: (device.nwk, relation)}) + + was_route_discovered = znp_server.reply_to( + c.ZDO.ExtRouteDisc.Req(Dst=device.nwk, partial=True), + responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], + ) + + with pytest.raises(DeliveryError): + await app.request( + device=device, + profile=260, + cluster=1, + src_ep=1, + dst_ep=1, + sequence=1, + data=b"\x00", + ) + + # Every child relation is still removed, retried once and restored as it was + assert data_req.call_count == 2 + assert was_route_discovered.call_count == 1 + assert table.log == [ + ("AssocGetWithAddress", device.ieee), + ("AssocRemove", device.ieee), + ("AssocGetWithAddress", device.ieee), + ("AssocAdd", device.ieee, device.nwk, relation), + ] + + await app.shutdown() + + +@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) +@pytest.mark.parametrize("rejoin", ["never", "during_retry", "during_restore_lookup"]) +async def test_request_recovery_assoc_restore_after_rejoin( + device_cls, rejoin, make_application, mocker +): + app, znp_server = make_application(server_cls=device_cls) + + await app.startup(auto_form=False) + + mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) + app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + + device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + + # A rejoin only updates the device here, it does not start a new interview + mocker.patch.object(device, "schedule_initialize") + + table = AssocTable( + znp_server, {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)} + ) + + def rejoin_if(when): + if rejoin == when: + # The device rejoins through a router under a new address. The host learns + # it from `ZDO.TCDevInd` or the announcement, both end in `handle_join`. + # The coordinator's table does not list it. + app.handle_join(nwk=0x1234, ieee=device.ieee, parent_nwk=0x5678) + + def on_lookup(): + # Only the restore looks the device up after its removal + if ("AssocRemove", device.ieee) in table.log: + rejoin_if("during_restore_lookup") + + table.on_lookup = on_lookup + + with pytest.raises(DeliveryError) as exc_info: + await request_with_failed_recovery( + app, znp_server, device, lambda: rejoin_if("during_retry") + ) + + assert exc_info.value.status == t.Status.NWK_NO_ROUTE + + removal = [ + ("AssocGetWithAddress", device.ieee), + ("AssocRemove", device.ieee), + ] + restore_lookup = ("AssocGetWithAddress", device.ieee) + + if rejoin == "never": + assert device.nwk == 0xABCD + assert table.log == removal + [ + restore_lookup, + ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD), + ] + elif rejoin == "during_retry": + # The old address is not written back over the one the host now knows + assert device.nwk == 0x1234 + assert table.log == removal + else: + # The address is checked again once the restore's lookup has returned + assert device.nwk == 0x1234 + assert table.log == removal + [restore_lookup] + + await app.shutdown() + + +@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) +async def test_request_recovery_assoc_restore_trace_1730( + device_cls, make_application, mocker +): + # Order of a production log (2026-10-20 17:30, Koenkk's build 20250321): the + # recovery runs on the final attempt, the device announces itself under its old + # address and leaves while the route is rediscovered, the restore follows, and + # the device rejoins under a new address five seconds later. + app, znp_server = make_application(server_cls=device_cls) + + await app.startup(auto_form=False) + + mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) + app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + + device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + mocker.patch.object(device, "schedule_initialize") + mocker.patch.object(device, "schedule_group_membership_scan") + + table = AssocTable( + znp_server, {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)} + ) + + def announce_and_leave(): + # Whether Z-Stack re-adds the entry at the announcement or drops it at the + # leave is not measured; the table keeps it removed + app.handle_join(nwk=0xABCD, ieee=device.ieee, parent_nwk=None) + app.handle_leave(nwk=0xABCD, ieee=device.ieee) + + table.on_remove = announce_and_leave + + with pytest.raises(DeliveryError) as exc_info: + await request_with_failed_recovery( + app, + znp_server, + device, + statuses=(t.Status.NWK_NO_ROUTE, t.Status.MAC_TRANSACTION_EXPIRED), + ) + + assert exc_info.value.status == t.Status.MAC_TRANSACTION_EXPIRED + + app.handle_join(nwk=0x1234, ieee=device.ieee, parent_nwk=0x0000) + assert device.nwk == 0x1234 + + # The new address arrives after the restore, so neither guard can apply + assert table.log == [ + ("AssocGetWithAddress", device.ieee), + ("AssocRemove", device.ieee), + ("AssocGetWithAddress", device.ieee), + ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD), + ] + + await app.shutdown() + + +@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) +@pytest.mark.parametrize( + ("entry_at_restore", "not_found", "restored"), + [ + # Absent: the device is still removed, so it is restored as it was + pytest.param(None, ASSOC_NOT_FOUND_ZEROED, True, id="absent_zeroed"), + pytest.param(None, ASSOC_NOT_FOUND_NOTUSED, True, id="absent_notused"), + # Present: Z-Stack re-associated the device before the host heard of it. + # `UTIL.AssocAdd` would overwrite this entry in place. + pytest.param( + (0xABCD, c.util.NodeRelation.CHILD_RFD), None, False, id="present" + ), + pytest.param( + (0x1234, c.util.NodeRelation.CHILD_RFD), None, False, id="present_new_nwk" + ), + pytest.param( + (0xABCD, c.util.NodeRelation.NEIGHBOR), None, False, id="present_neighbor" + ), + ], +) +async def test_request_recovery_assoc_restore_lookup( + device_cls, entry_at_restore, not_found, restored, make_application, mocker +): + app, znp_server = make_application(server_cls=device_cls) + + await app.startup(auto_form=False) + + mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) + app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + + device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + + table = AssocTable( + znp_server, + {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)}, + not_found=not_found, + ) + + def during_retry(): + if entry_at_restore is not None: + table.entries[device.ieee] = entry_at_restore + + with pytest.raises(DeliveryError): + await request_with_failed_recovery(app, znp_server, device, during_retry) + + expected = [ + ("AssocGetWithAddress", device.ieee), + ("AssocRemove", device.ieee), + ("AssocGetWithAddress", device.ieee), + ] + + if restored: + expected.append( + ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD) + ) + + assert table.log == expected + + await app.shutdown() + + +@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) +async def test_request_recovery_assoc_add_unsupported( + device_cls, make_application, mocker +): + app, znp_server = make_application(server_cls=device_cls) + + await app.startup(auto_form=False) + + mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) + app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + + device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + + table = AssocTable( + znp_server, + {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)}, + assoc_add=False, + ) + + # Unchanged behaviour: the failed restore replaces the `DeliveryError` + with pytest.raises(CommandNotRecognized) as exc_info: + await request_with_failed_recovery(app, znp_server, device) + + assert isinstance(exc_info.value.__context__, DeliveryError) + assert exc_info.value.__context__.status == t.Status.NWK_NO_ROUTE + assert table.log == [ + ("AssocGetWithAddress", device.ieee), + ("AssocRemove", device.ieee), + ("AssocGetWithAddress", device.ieee), + ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD), + ] + + # The device stays removed + assert device.ieee not in table.entries + + await app.shutdown() + + @pytest.mark.parametrize("device", [FormedLaunchpadCC26X2R1]) async def test_route_discovery_concurrency(device, make_application): app, znp_server = make_application(server_cls=device) diff --git a/zigpy_znp/zigbee/application.py b/zigpy_znp/zigbee/application.py index be95a9a..0f2c4da 100644 --- a/zigpy_znp/zigbee/application.py +++ b/zigpy_znp/zigbee/application.py @@ -37,6 +37,16 @@ EXTENDED_DATA_CONFIRM_TIMEOUT = 30 DEVICE_JOIN_MAX_DELAY = 5 +# Association table entries that the `send_packet` recovery may remove and restore +CHILD_NODE_RELATIONS = frozenset( + { + c.util.NodeRelation.CHILD_RFD, + c.util.NodeRelation.CHILD_RFD_RX_IDLE, + c.util.NodeRelation.CHILD_FFD, + c.util.NodeRelation.CHILD_FFD_RX_IDLE, + } +) + LOGGER = logging.getLogger(__name__) @@ -959,6 +969,20 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: ): raise + # Only a direct child can be dropped and restored. A failed + # lookup returns `INVALID_NODE_ADDR` (0xFFFE) followed by + # uninitialized memory on stock Z-Stack or by zeroes + # (`NodeRelation.PARENT`) on Koenkk's builds, and would be + # restored as a bogus entry (Koenkk/zigbee-herdsman#685). + # Rediscover the route and retry without touching the table. + if ( + child_association.Device.shortAddr == 0xFFFE + or child_association.Device.nodeRelation + not in CHILD_NODE_RELATIONS + ): + await self._discover_route(device.nwk) + continue + try: await self._znp.request( c.UTIL.AssocRemove.Req(IEEE=device.ieee) @@ -1005,10 +1029,42 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: # not be able to find it again. if not succeeded and tried_assoc_remove: assert child_association is not None - await self._znp.request( - c.UTIL.AssocAdd.Req( - NWK=child_association.Device.shortAddr, - IEEE=device.ieee, - NodeRelation=child_association.Device.nodeRelation, + removed = child_association.Device + + # Restore only the entry we removed, and only while nothing newer is + # known. `UTIL.AssocAdd` calls `AssocAddNew`, which TI documents as + # "create a new or update a previous association": for an IEEE that is + # already in the table it overwrites that entry in place, short address + # included (measured on Koenkk's build 20250321; the implementation is + # in TI's library). A device that rejoined under a new address during + # the recovery would be listed under its old one again. + # + # The host's address is compared again once the lookup has returned, + # which covers every address change the host has seen until the + # `AssocAdd` is handed to `ZNP.request`. A join that the firmware + # handles after the lookup's snapshot, including while `ZNP.request` + # waits for `_sync_request_lock`, cannot be excluded by the lookup + # alone; the create-or-update above then decides. Serializing table + # changes per IEEE is out of scope. + if device.nwk == removed.shortAddr: + current = await self._znp.request( + c.UTIL.AssocGetWithAddress.Req( + IEEE=device.ieee, + NWK=0x0000, # IEEE takes priority + ) ) - ) + + # The host may have handled a join while the lookup was awaited. + # The firmware must still answer "absent", as for the failed + # lookups above. + if device.nwk == removed.shortAddr and ( + current.Device.shortAddr == 0xFFFE + or current.Device.nodeRelation == c.util.NodeRelation.NOTUSED + ): + await self._znp.request( + c.UTIL.AssocAdd.Req( + NWK=removed.shortAddr, + IEEE=device.ieee, + NodeRelation=removed.nodeRelation, + ) + ) From b2314b479ff3b26be38f887f148dbbfcd2e03e0d Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:13:23 +0000 Subject: [PATCH 2/8] Model Z-Stack 3.x device association table behavior (cherry picked from commit de4f5a1860545e50f18eab07179fc6e288b4349e) --- tests/application/test_requests.py | 114 ++++++++--------------------- tests/conftest.py | 105 ++++++++++++++++++++++++++ zigpy_znp/commands/util.py | 15 +++- 3 files changed, 151 insertions(+), 83 deletions(-) diff --git a/tests/application/test_requests.py b/tests/application/test_requests.py index 58905ec..b2a246b 100644 --- a/tests/application/test_requests.py +++ b/tests/application/test_requests.py @@ -14,8 +14,10 @@ from ..conftest import ( FORMED_DEVICES, + BaseLaunchpadCC26X2R1, CoroutineMock, FormedLaunchpadCC26X2R1, + FormedLaunchpadCC26X2R1WithoutAssocCommands, serialize_zdo_command, zdo_request_matcher, ) @@ -609,9 +611,11 @@ async def test_request_recovery_route_rediscovery_then_assoc_failure( device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) - assoc_device, _ = c.util.Device.deserialize(b"\xff" * 100) - assoc_device.shortAddr = device.nwk - assoc_device.nodeRelation = c.util.NodeRelation.CHILD_FFD_RX_IDLE + znp_server.assoc_add_new( + nwk=device.nwk, + ieee=device.ieee, + node_relation=c.util.NodeRelation.CHILD_FFD_RX_IDLE, + ) # First attempt fails with NWK_NO_ROUTE, the retried attempt fails with a # first-time MAC_TRANSACTION_EXPIRED association failure @@ -634,31 +638,9 @@ def data_confirm_replier(req): ], ) - def assoc_get_with_addr(req): - # Once removed, the device is no longer found and the restore goes ahead - if did_assoc_remove.call_count: - return assoc_lookup_rsp(b"\x00", 0xFFFE, c.util.NodeRelation.PARENT) - - return c.UTIL.AssocGetWithAddress.Rsp(Device=assoc_device) - - znp_server.reply_to( - c.UTIL.AssocGetWithAddress.Req(IEEE=device.ieee, partial=True), - responses=[assoc_get_with_addr], - ) - - did_assoc_remove = znp_server.reply_to( - c.UTIL.AssocRemove.Req(IEEE=device.ieee), - responses=[c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS)], - ) - - did_assoc_add = znp_server.reply_to( - c.UTIL.AssocAdd.Req( - NWK=device.nwk, - IEEE=device.ieee, - NodeRelation=c.util.NodeRelation.CHILD_FFD_RX_IDLE, - ), - responses=[c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS)], - ) + # The association table responds on its own; just watch the mutations + did_assoc_remove = mocker.spy(znp_server, "assoc_remove") + did_assoc_add = mocker.spy(znp_server, "assoc_add_new") was_route_discovered = znp_server.reply_to( c.ZDO.ExtRouteDisc.Req( @@ -689,12 +671,18 @@ def assoc_get_with_addr(req): await app.shutdown() -@pytest.mark.parametrize("device_cls", FORMED_DEVICES) -@pytest.mark.parametrize("fw_assoc_remove", [True, False]) +@pytest.mark.parametrize( + "device_cls", FORMED_DEVICES + [FormedLaunchpadCC26X2R1WithoutAssocCommands] +) @pytest.mark.parametrize("final_status", [t.Status.SUCCESS, t.Status.APS_NO_ACK]) async def test_request_recovery_assoc_remove( - device_cls, fw_assoc_remove, final_status, make_application, mocker + device_cls, final_status, make_application, mocker ): + # Only the newer Z-Stack 3 firmware builds expose the custom AssocAdd/AssocRemove + # commands that back the child-association recovery flow + is_launchpad = issubclass(device_cls, BaseLaunchpadCC26X2R1) + fw_assoc_remove = issubclass(device_cls, FormedLaunchpadCC26X2R1) + app, znp_server = make_application(server_cls=device_cls) await app.startup(auto_form=False) @@ -705,12 +693,20 @@ async def test_request_recovery_assoc_remove( device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) - assoc_device, _ = c.util.Device.deserialize(b"\xff" * 100) - assoc_device.shortAddr = device.nwk - assoc_device.nodeRelation = c.util.NodeRelation.CHILD_FFD_RX_IDLE + znp_server.assoc_add_new( + nwk=device.nwk, + ieee=device.ieee, + node_relation=c.util.NodeRelation.CHILD_FFD_RX_IDLE, + ) + + # The association table responds on its own; just watch the mutations + did_assoc_get = mocker.spy(znp_server, "assoc_get") + did_assoc_remove = mocker.spy(znp_server, "assoc_remove") + did_assoc_add = mocker.spy(znp_server, "assoc_add_new") def data_confirm_replier(req): - bad_assoc = assoc_device + # The child stops expiring transactions once its association is dropped + bad_assoc = len(did_assoc_remove.mock_calls) == 0 return c.AF.DataConfirm.Callback( Status=t.Status.MAC_TRANSACTION_EXPIRED if bad_assoc else final_status, @@ -726,52 +722,6 @@ def data_confirm_replier(req): ], ) - def assoc_get_with_addr(req): - nonlocal assoc_device - - if assoc_device is None: - dev, _ = c.util.Device.deserialize(b"\xff" * 100) - return c.UTIL.AssocGetWithAddress.Rsp(Device=dev) - - return c.UTIL.AssocGetWithAddress.Rsp(Device=assoc_device) - - did_assoc_get = znp_server.reply_to( - c.UTIL.AssocGetWithAddress.Req(IEEE=device.ieee, partial=True), - responses=[assoc_get_with_addr], - ) - - if not issubclass(device_cls, FormedLaunchpadCC26X2R1): - fw_assoc_remove = False - - # Not all firmwares support Add/Remove - if fw_assoc_remove: - - def assoc_remove(req): - nonlocal assoc_device - - if assoc_device is None: - return c.UTIL.AssocRemove.Rsp(Status=t.Status.FAILURE) - - assoc_device = None - return c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS) - - did_assoc_remove = znp_server.reply_to( - c.UTIL.AssocRemove.Req(IEEE=device.ieee), - responses=[assoc_remove], - ) - - did_assoc_add = znp_server.reply_to( - c.UTIL.AssocAdd.Req( - NWK=device.nwk, - IEEE=device.ieee, - NodeRelation=c.util.NodeRelation.CHILD_FFD_RX_IDLE, - ), - responses=[c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS)], - ) - else: - did_assoc_remove = None - did_assoc_add = None - was_route_discovered = znp_server.reply_to( c.ZDO.ExtRouteDisc.Req( Dst=device.nwk, Options=c.zdo.RouteDiscoveryOptions.UNICAST, partial=True @@ -803,7 +753,7 @@ def assoc_remove(req): assert len(did_assoc_add.mock_calls) >= 1 else: assert len(did_assoc_add.mock_calls) == 0 - elif issubclass(device_cls, FormedLaunchpadCC26X2R1): + elif is_launchpad: assert len(did_assoc_get.mock_calls) >= 1 else: # Don't even attempt this with older firmwares diff --git a/tests/conftest.py b/tests/conftest.py index 58b2690..f1591c6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -431,6 +431,9 @@ def __init__(self, *args, **kwargs): self.device_state = t.DeviceState.InitializedNotStarted self.zdo_callbacks = set() + self._assoc_table = {} + self._assoc_next_addr_idx = 0 + # Handle the decorators for name in dir(self): func = getattr(self, name) @@ -916,6 +919,85 @@ def register_zdo_callback(self, request): def assoc_find_dev_responder(self, req): return req.Rsp(Device=t.Bytes(b"\xff" * (36 if self.align_structs else 28))) + # Simulated association table, modeled on the Z-Stack `AssocAddNew`/`AssocRemove`/ + # `AssocGetWithAddress` functions. Keyed by IEEE, like the address manager lookup. + def assoc_get(self, *, ieee=None, nwk=None): + # The IEEE takes priority; all-zeroes means "use the NWK address instead" + if ieee is not None and any(ieee): + return self._assoc_table.get(ieee) + + for device in self._assoc_table.values(): + if device.shortAddr == nwk: + return device + + return None + + def assoc_add_new(self, *, nwk, ieee, node_relation): + # `AssocAddNew` updates an existing entry in place (keeping its addrIdx) or + # allocates a new one when the IEEE is not already known. + node_relation = c.util.NodeRelation(node_relation) + existing = self._assoc_table.get(ieee) + + if existing is not None: + addr_idx = existing.addrIdx + else: + addr_idx = self._assoc_next_addr_idx + self._assoc_next_addr_idx += 1 + + is_child = ( + c.util.NodeRelation.CHILD_RFD + <= node_relation + <= c.util.NodeRelation.CHILD_FFD_RX_IDLE + ) + # Only RFD children age out and track keepalives; routers don't + is_aging_child = node_relation in ( + c.util.NodeRelation.CHILD_RFD, + c.util.NodeRelation.CHILD_RFD_RX_IDLE, + ) + + device = c.util.Device( + shortAddr=t.NWK(nwk), + addrIdx=addr_idx, + nodeRelation=node_relation, + devStatus=( + c.util.DeviceStatus.SEC_INIT if is_child else c.util.DeviceStatus.NONE + ), + assocCnt=0, + age=0, + linkInfo=c.util.LinkInfo( + txCounter=0, + txCost=1, + rxLqi=0xAA, + inKeySeqNum=0, + inFrmCntr=0, + txFailure=0, + ), + endDev=c.util.AgingEndDevice( + endDevCfg=0, + deviceTimeout=0 if is_aging_child else 0xFFFFFFFE, + ), + timeoutCounter=0 if is_aging_child else 0xFFFFFFFE, + keepaliveRcv=1 if is_aging_child else 0, + ctrl=0x80, # ASSOC_LIST_CTRL_SAVE + ) + self._assoc_table[ieee] = device + + return device + + def assoc_remove(self, ieee): + # `AssocRemove` frees the slot and returns whether it was present + return self._assoc_table.pop(ieee, None) is not None + + @reply_to(c.UTIL.AssocGetWithAddress.Req(partial=True)) + def on_util_assoc_get_with_address(self, req): + device = self.assoc_get(ieee=req.IEEE, nwk=req.NWK) + + if device is None: + # The firmware packs INVALID_NODE_ADDR / NOTUSED for an unknown device + device, _ = c.util.Device.deserialize(b"\xff" * 100) + + return c.UTIL.AssocGetWithAddress.Rsp(Device=device) + class BaseZStack1CC2531(BaseZStackDevice): align_structs = False @@ -1361,6 +1443,29 @@ def __init__(self, *args, **kwargs): self._nvram = load_nvram_json("CC2652R-ZStack4.formed.json") self._orig_nvram = simple_deepcopy(self._nvram) + # Koenkk Z-Stack 3.x firmware (post-20201017) adds the custom AssocAdd/AssocRemove + # MT commands. Both proxy into the association table but always reply with a single + # SUCCESS status byte, ignoring the proxied call's result. + @reply_to(c.UTIL.AssocRemove.Req(partial=True)) + def on_util_assoc_remove(self, req): + self.assoc_remove(req.IEEE) + + return c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS) + + @reply_to(c.UTIL.AssocAdd.Req(partial=True)) + def on_util_assoc_add(self, req): + self.assoc_add_new(nwk=req.NWK, ieee=req.IEEE, node_relation=req.NodeRelation) + + return c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS) + + +class FormedLaunchpadCC26X2R1WithoutAssocCommands(BaseLaunchpadCC26X2R1): + # Models an older firmware build that lacks the custom AssocAdd/AssocRemove commands + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._nvram = load_nvram_json("CC2652R-ZStack4.formed.json") + self._orig_nvram = simple_deepcopy(self._nvram) + class ResetLaunchpadCC26X2R1(BaseLaunchpadCC26X2R1): def __init__(self, *args, **kwargs): diff --git a/zigpy_znp/commands/util.py b/zigpy_znp/commands/util.py index 8f5f0a0..a43779b 100644 --- a/zigpy_znp/commands/util.py +++ b/zigpy_znp/commands/util.py @@ -16,6 +16,19 @@ class NodeRelation(t.enum8): NOTUSED = 0xFF +class DeviceStatus(t.bitmap8): + # Bitmap of an associated device's status fields (AssocList.h) + NONE = 0x00 + LINK_STATUS = 0x01 # link is in-active + LINK_REPAIR = 0x02 # link repair in progress + SEC_INIT = 0x04 # security init + SEC_AUTH = 0x08 # security authenticated + SEC_AUTH_TC_REJOIN = 0x10 # assumed-secure device that performs a TC rejoin + SECURED_JOIN = 0x20 # device joined secure + REJOIN = 0x40 # device rejoined + HIGH_SEC = 0x80 # device joined as High Security + + class BindEntry(t.Struct): srcEP: t.uint8_t dstGroupMode: t.uint8_t # 0 - Normal address index, 1 - Group address @@ -44,7 +57,7 @@ class Device(t.CStruct): shortAddr: t.NWK addrIdx: t.uint16_t nodeRelation: NodeRelation - devStatus: t.uint8_t + devStatus: DeviceStatus assocCnt: t.uint8_t age: t.uint8_t linkInfo: LinkInfo From b96eaa2e8b99f9e6656ffcaa030f719e0d08b765 Mon Sep 17 00:00:00 2001 From: Fabian <6667968+fabcoded@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:30:24 +0200 Subject: [PATCH 3/8] Rebuild the recovery tests on the association-table model, trim comments --- tests/application/test_requests.py | 519 ++++++----------------------- zigpy_znp/zigbee/application.py | 27 +- 2 files changed, 110 insertions(+), 436 deletions(-) diff --git a/tests/application/test_requests.py b/tests/application/test_requests.py index b2a246b..d3a300c 100644 --- a/tests/application/test_requests.py +++ b/tests/application/test_requests.py @@ -9,7 +9,6 @@ import zigpy_znp.commands as c import zigpy_znp.config as conf -from zigpy_znp.exceptions import CommandNotRecognized import zigpy_znp.types as t from ..conftest import ( @@ -763,169 +762,37 @@ def data_confirm_replier(req): await app.shutdown() -def assoc_lookup_rsp(fill: bytes, nwk: int, relation: c.util.NodeRelation): - """Build a `UTIL.AssocGetWithAddress` answer, with `fill` in the other fields.""" - assoc_device, _ = c.util.Device.deserialize(fill * 100) - assoc_device.shortAddr = t.NWK(nwk) - assoc_device.nodeRelation = relation +def koenkk_absent_lookup(znp_server): + # Koenkk's builds answer a lookup miss with 0xFFFE and zeroes (`PARENT`) + def lookup(req): + device = znp_server.assoc_get(ieee=req.IEEE, nwk=req.NWK) - return c.UTIL.AssocGetWithAddress.Rsp(Device=assoc_device) + if device is None: + device, _ = c.util.Device.deserialize(b"\x00" * 100) + device.shortAddr = t.NWK(0xFFFE) + return c.UTIL.AssocGetWithAddress.Rsp(Device=device) -ASSOC_NOT_FOUND_ZEROED = assoc_lookup_rsp(b"\x00", 0xFFFE, c.util.NodeRelation.PARENT) -ASSOC_NOT_FOUND_NOTUSED = assoc_lookup_rsp(b"\xff", 0xFFFF, c.util.NodeRelation.NOTUSED) - - -class AssocTable: - """ - Association table of a simulated coordinator. It applies `UTIL.AssocRemove` and - `UTIL.AssocAdd`, answers lookups for absent devices with `not_found` (Koenkk's - builds: `0xFFFE`, then zeroes) and logs every request in order. `on_remove` runs - right after a removal, `on_lookup` once a lookup's answer is built but before - the host receives it. - """ - - def __init__(self, znp_server, entries, *, not_found=None, assoc_add=True): - self.entries = dict(entries) - self.not_found = not_found or ASSOC_NOT_FOUND_ZEROED - self.assoc_add = assoc_add - self.on_remove = None - self.on_lookup = None - self.log = [] - - znp_server.reply_to( - c.UTIL.AssocGetWithAddress.Req(partial=True), responses=[self.get] - ) - znp_server.reply_to( - c.UTIL.AssocRemove.Req(partial=True), responses=[self.remove] - ) - znp_server.reply_to(c.UTIL.AssocAdd.Req(partial=True), responses=[self.add]) - - def get(self, req): - self.log.append(("AssocGetWithAddress", req.IEEE)) - - if req.IEEE not in self.entries: - rsp = self.not_found - else: - rsp = assoc_lookup_rsp(b"\x00", *self.entries[req.IEEE]) - - if self.on_lookup is not None: - self.on_lookup() - - return rsp - - def remove(self, req): - self.log.append(("AssocRemove", req.IEEE)) - self.entries.pop(req.IEEE, None) - - if self.on_remove is not None: - self.on_remove() - - return c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS) - - def add(self, req): - self.log.append(("AssocAdd", req.IEEE, req.NWK, req.NodeRelation)) - - # Koenkk's builds 20200805 and 20200925 have `AssocRemove`, not `AssocAdd` - if not self.assoc_add: - return c.RPCError.CommandNotRecognized.Rsp( - ErrorCode=c.rpc_error.ErrorCode.InvalidCommandId, - RequestHeader=req.to_frame().header, - ) - - self.entries[req.IEEE] = (req.NWK, req.NodeRelation) - - return c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS) - - -async def request_with_failed_recovery( - app, - znp_server, - device, - during_retry=None, - statuses=(t.Status.MAC_TRANSACTION_EXPIRED, t.Status.NWK_NO_ROUTE), -): - """ - Send a request whose two attempts fail with `statuses`. By default the first - one expires, starting the association recovery, and the retry finds no route. - `during_retry` runs while the second attempt is in flight. - """ - - attempts = 0 - - async def data_request(req): - nonlocal attempts - attempts += 1 - - yield c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS) - - if attempts == 2 and during_retry is not None: - during_retry() - - yield c.AF.DataConfirm.Callback( - Status=statuses[attempts - 1], Endpoint=1, TSN=1 - ) - - znp_server.reply_to(c.AF.DataRequestExt.Req(partial=True), responses=[data_request]) znp_server.reply_to( - c.ZDO.ExtRouteDisc.Req(partial=True), - responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], - ) - - await app.request( - device=device, - profile=260, - cluster=1, - src_ep=1, - dst_ep=1, - sequence=1, - data=b"\x00", + c.UTIL.AssocGetWithAddress.Req(partial=True), responses=[lookup], override=True ) @pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) @pytest.mark.parametrize( - ("lookup_rsp", "retried"), + ("relation", "koenkk_lookup", "retried"), [ - # Not found: Z-Stack returns `INVALID_NODE_ADDR`. Koenkk's firmware zeroes - # the remainder, which reads as `PARENT`. Stock firmware leaves it - # uninitialized, so it may even contain a child relation. - pytest.param( - assoc_lookup_rsp(b"\x00", 0xFFFE, c.util.NodeRelation.PARENT), - True, - id="not_found_zeroed", - ), - pytest.param( - assoc_lookup_rsp(b"\xa5", 0xFFFE, c.util.NodeRelation.CHILD_RFD), - True, - id="not_found_garbage", - ), + # Absent: stock firmware reports `NOTUSED`, which ends the send at once + (None, False, False), + (None, True, True), # Found, but not a direct child of the coordinator - pytest.param( - assoc_lookup_rsp(b"\x00", 0xABCD, c.util.NodeRelation.PARENT), - True, - id="parent", - ), - pytest.param( - assoc_lookup_rsp(b"\x00", 0xABCD, c.util.NodeRelation.NEIGHBOR), - True, - id="neighbor", - ), - pytest.param( - assoc_lookup_rsp(b"\x00", 0xABCD, c.util.NodeRelation.OTHER), - True, - id="other", - ), - # Unchanged: `NOTUSED` ends the send at once - pytest.param( - assoc_lookup_rsp(b"\xff", 0xABCD, c.util.NodeRelation.NOTUSED), - False, - id="notused", - ), + (c.util.NodeRelation.PARENT, False, True), + (c.util.NodeRelation.NEIGHBOR, False, True), + (c.util.NodeRelation.OTHER, False, True), ], ) async def test_request_recovery_assoc_not_a_child( - device_cls, lookup_rsp, retried, make_application, mocker + device_cls, relation, koenkk_lookup, retried, make_application, mocker ): app, znp_server = make_application(server_cls=device_cls) @@ -936,6 +803,17 @@ async def test_request_recovery_assoc_not_a_child( device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + if relation is not None: + znp_server.assoc_add_new( + nwk=device.nwk, ieee=device.ieee, node_relation=relation + ) + + if koenkk_lookup: + koenkk_absent_lookup(znp_server) + + did_assoc_remove = mocker.spy(znp_server, "assoc_remove") + did_assoc_add = mocker.spy(znp_server, "assoc_add_new") + data_req = znp_server.reply_to( c.AF.DataRequestExt.Req(partial=True), responses=[ @@ -946,28 +824,12 @@ async def test_request_recovery_assoc_not_a_child( ], ) - did_assoc_get = znp_server.reply_to( - c.UTIL.AssocGetWithAddress.Req(IEEE=device.ieee, partial=True), - responses=[lookup_rsp], - ) - - # Match any arguments: neither command may be sent at all - did_assoc_remove = znp_server.reply_to( - c.UTIL.AssocRemove.Req(partial=True), - responses=[c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS)], - ) - - did_assoc_add = znp_server.reply_to( - c.UTIL.AssocAdd.Req(partial=True), - responses=[c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS)], - ) - was_route_discovered = znp_server.reply_to( c.ZDO.ExtRouteDisc.Req(Dst=device.nwk, partial=True), responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], ) - with pytest.raises(DeliveryError) as exc_info: + with pytest.raises(DeliveryError): await app.request( device=device, profile=260, @@ -978,85 +840,38 @@ async def test_request_recovery_assoc_not_a_child( data=b"\x00", ) - # The association table is left alone. Other than `NOTUSED`, the route is - # rediscovered and the send retried once, as before, then the error propagates. - assert exc_info.value.status == t.Status.MAC_TRANSACTION_EXPIRED + # The table is left alone, the route is rediscovered and the send retried assert data_req.call_count == (2 if retried else 1) - assert did_assoc_get.call_count == 1 + assert was_route_discovered.call_count == (1 if retried else 0) assert did_assoc_remove.call_count == 0 assert did_assoc_add.call_count == 0 - assert was_route_discovered.call_count == (1 if retried else 0) await app.shutdown() +RFD = c.util.NodeRelation.CHILD_RFD + + @pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) @pytest.mark.parametrize( - "relation", + ("scenario", "relation", "restored"), [ - c.util.NodeRelation.CHILD_RFD, - c.util.NodeRelation.CHILD_RFD_RX_IDLE, - c.util.NodeRelation.CHILD_FFD, - c.util.NodeRelation.CHILD_FFD_RX_IDLE, + # Every direct child relation is removed and restored as it was + ("unchanged", c.util.NodeRelation.CHILD_RFD, True), + ("unchanged", c.util.NodeRelation.CHILD_RFD_RX_IDLE, True), + ("unchanged", c.util.NodeRelation.CHILD_FFD, True), + ("unchanged", c.util.NodeRelation.CHILD_FFD_RX_IDLE, True), + ("koenkk_lookup", RFD, True), + # The host learned a new address, so the old one must not be written back + ("rejoin_during_retry", RFD, False), + ("rejoin_during_restore_lookup", RFD, False), + # The firmware has the device again, `AssocAdd` would overwrite the entry + ("re_associated", RFD, False), + ("re_associated_new_nwk", RFD, False), ], ) -async def test_request_recovery_assoc_remove_child_relations( - device_cls, relation, make_application, mocker -): - app, znp_server = make_application(server_cls=device_cls) - - await app.startup(auto_form=False) - - mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) - app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 - - device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) - - data_req = znp_server.reply_to( - c.AF.DataRequestExt.Req(partial=True), - responses=[ - c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS), - c.AF.DataConfirm.Callback( - Status=t.Status.MAC_TRANSACTION_EXPIRED, Endpoint=1, TSN=1 - ), - ], - ) - - table = AssocTable(znp_server, {device.ieee: (device.nwk, relation)}) - - was_route_discovered = znp_server.reply_to( - c.ZDO.ExtRouteDisc.Req(Dst=device.nwk, partial=True), - responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], - ) - - with pytest.raises(DeliveryError): - await app.request( - device=device, - profile=260, - cluster=1, - src_ep=1, - dst_ep=1, - sequence=1, - data=b"\x00", - ) - - # Every child relation is still removed, retried once and restored as it was - assert data_req.call_count == 2 - assert was_route_discovered.call_count == 1 - assert table.log == [ - ("AssocGetWithAddress", device.ieee), - ("AssocRemove", device.ieee), - ("AssocGetWithAddress", device.ieee), - ("AssocAdd", device.ieee, device.nwk, relation), - ] - - await app.shutdown() - - -@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) -@pytest.mark.parametrize("rejoin", ["never", "during_retry", "during_restore_lookup"]) -async def test_request_recovery_assoc_restore_after_rejoin( - device_cls, rejoin, make_application, mocker +async def test_request_recovery_assoc_restore( + device_cls, scenario, relation, restored, make_application, mocker ): app, znp_server = make_application(server_cls=device_cls) @@ -1070,205 +885,83 @@ async def test_request_recovery_assoc_restore_after_rejoin( # A rejoin only updates the device here, it does not start a new interview mocker.patch.object(device, "schedule_initialize") - table = AssocTable( - znp_server, {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)} - ) - - def rejoin_if(when): - if rejoin == when: - # The device rejoins through a router under a new address. The host learns - # it from `ZDO.TCDevInd` or the announcement, both end in `handle_join`. - # The coordinator's table does not list it. - app.handle_join(nwk=0x1234, ieee=device.ieee, parent_nwk=0x5678) - - def on_lookup(): - # Only the restore looks the device up after its removal - if ("AssocRemove", device.ieee) in table.log: - rejoin_if("during_restore_lookup") - - table.on_lookup = on_lookup - - with pytest.raises(DeliveryError) as exc_info: - await request_with_failed_recovery( - app, znp_server, device, lambda: rejoin_if("during_retry") - ) - - assert exc_info.value.status == t.Status.NWK_NO_ROUTE + assoc_add_new = znp_server.assoc_add_new + assoc_add_new(nwk=device.nwk, ieee=device.ieee, node_relation=relation) - removal = [ - ("AssocGetWithAddress", device.ieee), - ("AssocRemove", device.ieee), - ] - restore_lookup = ("AssocGetWithAddress", device.ieee) - - if rejoin == "never": - assert device.nwk == 0xABCD - assert table.log == removal + [ - restore_lookup, - ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD), - ] - elif rejoin == "during_retry": - # The old address is not written back over the one the host now knows - assert device.nwk == 0x1234 - assert table.log == removal - else: - # The address is checked again once the restore's lookup has returned - assert device.nwk == 0x1234 - assert table.log == removal + [restore_lookup] - - await app.shutdown() + if scenario == "koenkk_lookup": + koenkk_absent_lookup(znp_server) + assoc_get = znp_server.assoc_get -@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) -async def test_request_recovery_assoc_restore_trace_1730( - device_cls, make_application, mocker -): - # Order of a production log (2026-10-20 17:30, Koenkk's build 20250321): the - # recovery runs on the final attempt, the device announces itself under its old - # address and leaves while the route is rediscovered, the restore follows, and - # the device rejoins under a new address five seconds later. - app, znp_server = make_application(server_cls=device_cls) + def rejoin_on_restore_lookup(**kwargs): + # Only the restore looks the device up after its removal + entry = assoc_get(**kwargs) - await app.startup(auto_form=False) + if entry is None and scenario == "rejoin_during_restore_lookup": + app.handle_join(nwk=0x1234, ieee=device.ieee, parent_nwk=0x5678) - mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) - app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + return entry - device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) - mocker.patch.object(device, "schedule_initialize") - mocker.patch.object(device, "schedule_group_membership_scan") + mocker.patch.object(znp_server, "assoc_get", side_effect=rejoin_on_restore_lookup) + did_assoc_remove = mocker.spy(znp_server, "assoc_remove") + did_assoc_add = mocker.spy(znp_server, "assoc_add_new") - table = AssocTable( - znp_server, {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)} - ) + attempts = 0 - def announce_and_leave(): - # Whether Z-Stack re-adds the entry at the announcement or drops it at the - # leave is not measured; the table keeps it removed - app.handle_join(nwk=0xABCD, ieee=device.ieee, parent_nwk=None) - app.handle_leave(nwk=0xABCD, ieee=device.ieee) + def data_confirm_replier(req): + nonlocal attempts + attempts += 1 - table.on_remove = announce_and_leave + # The first attempt expires and starts the recovery, the retry finds no route + if attempts == 2: + if scenario == "rejoin_during_retry": + app.handle_join(nwk=0x1234, ieee=device.ieee, parent_nwk=0x5678) + elif scenario.startswith("re_associated"): + nwk = 0x1234 if scenario.endswith("new_nwk") else device.nwk + assoc_add_new(nwk=nwk, ieee=device.ieee, node_relation=relation) - with pytest.raises(DeliveryError) as exc_info: - await request_with_failed_recovery( - app, - znp_server, - device, - statuses=(t.Status.NWK_NO_ROUTE, t.Status.MAC_TRANSACTION_EXPIRED), + return c.AF.DataConfirm.Callback( + Status=( + t.Status.MAC_TRANSACTION_EXPIRED + if attempts == 1 + else t.Status.NWK_NO_ROUTE + ), + Endpoint=1, + TSN=1, ) - assert exc_info.value.status == t.Status.MAC_TRANSACTION_EXPIRED - - app.handle_join(nwk=0x1234, ieee=device.ieee, parent_nwk=0x0000) - assert device.nwk == 0x1234 - - # The new address arrives after the restore, so neither guard can apply - assert table.log == [ - ("AssocGetWithAddress", device.ieee), - ("AssocRemove", device.ieee), - ("AssocGetWithAddress", device.ieee), - ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD), - ] - - await app.shutdown() - - -@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) -@pytest.mark.parametrize( - ("entry_at_restore", "not_found", "restored"), - [ - # Absent: the device is still removed, so it is restored as it was - pytest.param(None, ASSOC_NOT_FOUND_ZEROED, True, id="absent_zeroed"), - pytest.param(None, ASSOC_NOT_FOUND_NOTUSED, True, id="absent_notused"), - # Present: Z-Stack re-associated the device before the host heard of it. - # `UTIL.AssocAdd` would overwrite this entry in place. - pytest.param( - (0xABCD, c.util.NodeRelation.CHILD_RFD), None, False, id="present" - ), - pytest.param( - (0x1234, c.util.NodeRelation.CHILD_RFD), None, False, id="present_new_nwk" - ), - pytest.param( - (0xABCD, c.util.NodeRelation.NEIGHBOR), None, False, id="present_neighbor" - ), - ], -) -async def test_request_recovery_assoc_restore_lookup( - device_cls, entry_at_restore, not_found, restored, make_application, mocker -): - app, znp_server = make_application(server_cls=device_cls) - - await app.startup(auto_form=False) - - mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) - app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 - - device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) - - table = AssocTable( - znp_server, - {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)}, - not_found=not_found, + znp_server.reply_to( + c.AF.DataRequestExt.Req(partial=True), + responses=[ + c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS), + data_confirm_replier, + ], ) - def during_retry(): - if entry_at_restore is not None: - table.entries[device.ieee] = entry_at_restore + znp_server.reply_to( + c.ZDO.ExtRouteDisc.Req(partial=True), + responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], + ) with pytest.raises(DeliveryError): - await request_with_failed_recovery(app, znp_server, device, during_retry) + await app.request( + device=device, + profile=260, + cluster=1, + src_ep=1, + dst_ep=1, + sequence=1, + data=b"\x00", + ) - expected = [ - ("AssocGetWithAddress", device.ieee), - ("AssocRemove", device.ieee), - ("AssocGetWithAddress", device.ieee), - ] + assert did_assoc_remove.call_count == 1 if restored: - expected.append( - ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD) + did_assoc_add.assert_called_once_with( + nwk=0xABCD, ieee=device.ieee, node_relation=relation ) - - assert table.log == expected - - await app.shutdown() - - -@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) -async def test_request_recovery_assoc_add_unsupported( - device_cls, make_application, mocker -): - app, znp_server = make_application(server_cls=device_cls) - - await app.startup(auto_form=False) - - mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) - app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 - - device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) - - table = AssocTable( - znp_server, - {device.ieee: (0xABCD, c.util.NodeRelation.CHILD_RFD)}, - assoc_add=False, - ) - - # Unchanged behaviour: the failed restore replaces the `DeliveryError` - with pytest.raises(CommandNotRecognized) as exc_info: - await request_with_failed_recovery(app, znp_server, device) - - assert isinstance(exc_info.value.__context__, DeliveryError) - assert exc_info.value.__context__.status == t.Status.NWK_NO_ROUTE - assert table.log == [ - ("AssocGetWithAddress", device.ieee), - ("AssocRemove", device.ieee), - ("AssocGetWithAddress", device.ieee), - ("AssocAdd", device.ieee, 0xABCD, c.util.NodeRelation.CHILD_RFD), - ] - - # The device stays removed - assert device.ieee not in table.entries + else: + did_assoc_add.assert_not_called() await app.shutdown() diff --git a/zigpy_znp/zigbee/application.py b/zigpy_znp/zigbee/application.py index 0f2c4da..e6aed9d 100644 --- a/zigpy_znp/zigbee/application.py +++ b/zigpy_znp/zigbee/application.py @@ -970,11 +970,8 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: raise # Only a direct child can be dropped and restored. A failed - # lookup returns `INVALID_NODE_ADDR` (0xFFFE) followed by - # uninitialized memory on stock Z-Stack or by zeroes - # (`NodeRelation.PARENT`) on Koenkk's builds, and would be - # restored as a bogus entry (Koenkk/zigbee-herdsman#685). - # Rediscover the route and retry without touching the table. + # lookup returns 0xFFFE and a non-child relation on some + # builds (Koenkk/zigbee-herdsman#685). if ( child_association.Device.shortAddr == 0xFFFE or child_association.Device.nodeRelation @@ -1031,21 +1028,8 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: assert child_association is not None removed = child_association.Device - # Restore only the entry we removed, and only while nothing newer is - # known. `UTIL.AssocAdd` calls `AssocAddNew`, which TI documents as - # "create a new or update a previous association": for an IEEE that is - # already in the table it overwrites that entry in place, short address - # included (measured on Koenkk's build 20250321; the implementation is - # in TI's library). A device that rejoined under a new address during - # the recovery would be listed under its old one again. - # - # The host's address is compared again once the lookup has returned, - # which covers every address change the host has seen until the - # `AssocAdd` is handed to `ZNP.request`. A join that the firmware - # handles after the lookup's snapshot, including while `ZNP.request` - # waits for `_sync_request_lock`, cannot be excluded by the lookup - # alone; the create-or-update above then decides. Serializing table - # changes per IEEE is out of scope. + # `AssocAdd` updates an existing entry in place, so skip it if the + # device rejoined, possibly under a new address, since the removal if device.nwk == removed.shortAddr: current = await self._znp.request( c.UTIL.AssocGetWithAddress.Req( @@ -1054,9 +1038,6 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: ) ) - # The host may have handled a join while the lookup was awaited. - # The firmware must still answer "absent", as for the failed - # lookups above. if device.nwk == removed.shortAddr and ( current.Device.shortAddr == 0xFFFE or current.Device.nodeRelation == c.util.NodeRelation.NOTUSED From 940e6077a66a1a0cb3caee5c7a1b648ab7d24fbd Mon Sep 17 00:00:00 2001 From: Fabian <6667968+fabcoded@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:34:27 +0200 Subject: [PATCH 4/8] Test a lookup miss that reads as a child relation --- tests/application/test_requests.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/application/test_requests.py b/tests/application/test_requests.py index d3a300c..3d06ad6 100644 --- a/tests/application/test_requests.py +++ b/tests/application/test_requests.py @@ -762,14 +762,16 @@ def data_confirm_replier(req): await app.shutdown() -def koenkk_absent_lookup(znp_server): - # Koenkk's builds answer a lookup miss with 0xFFFE and zeroes (`PARENT`) +def absent_lookup_with_address_0xfffe(znp_server, relation=c.util.NodeRelation.PARENT): + # A lookup miss answers 0xFFFE: Koenkk's builds zero the rest (`PARENT`), stock + # Z-Stack leaves uninitialized memory, which may read as a child relation def lookup(req): device = znp_server.assoc_get(ieee=req.IEEE, nwk=req.NWK) if device is None: device, _ = c.util.Device.deserialize(b"\x00" * 100) device.shortAddr = t.NWK(0xFFFE) + device.nodeRelation = relation return c.UTIL.AssocGetWithAddress.Rsp(Device=device) @@ -780,19 +782,20 @@ def lookup(req): @pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) @pytest.mark.parametrize( - ("relation", "koenkk_lookup", "retried"), + ("relation", "absent_relation", "retried"), [ - # Absent: stock firmware reports `NOTUSED`, which ends the send at once - (None, False, False), - (None, True, True), + # Absent: the default model reports `NOTUSED`, which ends the send at once + (None, None, False), + (None, c.util.NodeRelation.PARENT, True), + (None, c.util.NodeRelation.CHILD_RFD, True), # Found, but not a direct child of the coordinator - (c.util.NodeRelation.PARENT, False, True), - (c.util.NodeRelation.NEIGHBOR, False, True), - (c.util.NodeRelation.OTHER, False, True), + (c.util.NodeRelation.PARENT, None, True), + (c.util.NodeRelation.NEIGHBOR, None, True), + (c.util.NodeRelation.OTHER, None, True), ], ) async def test_request_recovery_assoc_not_a_child( - device_cls, relation, koenkk_lookup, retried, make_application, mocker + device_cls, relation, absent_relation, retried, make_application, mocker ): app, znp_server = make_application(server_cls=device_cls) @@ -808,8 +811,8 @@ async def test_request_recovery_assoc_not_a_child( nwk=device.nwk, ieee=device.ieee, node_relation=relation ) - if koenkk_lookup: - koenkk_absent_lookup(znp_server) + if absent_relation is not None: + absent_lookup_with_address_0xfffe(znp_server, absent_relation) did_assoc_remove = mocker.spy(znp_server, "assoc_remove") did_assoc_add = mocker.spy(znp_server, "assoc_add_new") @@ -889,7 +892,7 @@ async def test_request_recovery_assoc_restore( assoc_add_new(nwk=device.nwk, ieee=device.ieee, node_relation=relation) if scenario == "koenkk_lookup": - koenkk_absent_lookup(znp_server) + absent_lookup_with_address_0xfffe(znp_server) assoc_get = znp_server.assoc_get From 1d608cb8393624ef24757c16cd1ae4618c6f16f7 Mon Sep 17 00:00:00 2001 From: Fabian <6667968+fabcoded@users.noreply.github.com> Date: Thu, 22 Oct 2026 00:40:15 +0200 Subject: [PATCH 5/8] Shorten comments --- tests/application/test_requests.py | 22 +++++++++++----------- zigpy_znp/zigbee/application.py | 11 +++++------ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/tests/application/test_requests.py b/tests/application/test_requests.py index 3d06ad6..250627e 100644 --- a/tests/application/test_requests.py +++ b/tests/application/test_requests.py @@ -763,8 +763,8 @@ def data_confirm_replier(req): def absent_lookup_with_address_0xfffe(znp_server, relation=c.util.NodeRelation.PARENT): - # A lookup miss answers 0xFFFE: Koenkk's builds zero the rest (`PARENT`), stock - # Z-Stack leaves uninitialized memory, which may read as a child relation + # Real miss: 0xFFFE, then zeros on Koenkk's builds (`PARENT`) or garbage on stock + # Z-Stack, which may read as a child def lookup(req): device = znp_server.assoc_get(ieee=req.IEEE, nwk=req.NWK) @@ -784,11 +784,11 @@ def lookup(req): @pytest.mark.parametrize( ("relation", "absent_relation", "retried"), [ - # Absent: the default model reports `NOTUSED`, which ends the send at once + # Miss as `NOTUSED` (model default): send ends at once (None, None, False), (None, c.util.NodeRelation.PARENT, True), (None, c.util.NodeRelation.CHILD_RFD, True), - # Found, but not a direct child of the coordinator + # Found, not a direct child (c.util.NodeRelation.PARENT, None, True), (c.util.NodeRelation.NEIGHBOR, None, True), (c.util.NodeRelation.OTHER, None, True), @@ -843,7 +843,7 @@ async def test_request_recovery_assoc_not_a_child( data=b"\x00", ) - # The table is left alone, the route is rediscovered and the send retried + # Table untouched, route rediscovered, send retried assert data_req.call_count == (2 if retried else 1) assert was_route_discovered.call_count == (1 if retried else 0) assert did_assoc_remove.call_count == 0 @@ -859,16 +859,16 @@ async def test_request_recovery_assoc_not_a_child( @pytest.mark.parametrize( ("scenario", "relation", "restored"), [ - # Every direct child relation is removed and restored as it was + # Each child relation: removed, restored as it was ("unchanged", c.util.NodeRelation.CHILD_RFD, True), ("unchanged", c.util.NodeRelation.CHILD_RFD_RX_IDLE, True), ("unchanged", c.util.NodeRelation.CHILD_FFD, True), ("unchanged", c.util.NodeRelation.CHILD_FFD_RX_IDLE, True), ("koenkk_lookup", RFD, True), - # The host learned a new address, so the old one must not be written back + # Host knows a new address: no restore ("rejoin_during_retry", RFD, False), ("rejoin_during_restore_lookup", RFD, False), - # The firmware has the device again, `AssocAdd` would overwrite the entry + # Firmware has the device again: no overwrite ("re_associated", RFD, False), ("re_associated_new_nwk", RFD, False), ], @@ -885,7 +885,7 @@ async def test_request_recovery_assoc_restore( device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) - # A rejoin only updates the device here, it does not start a new interview + # Rejoin only updates the device, no interview mocker.patch.object(device, "schedule_initialize") assoc_add_new = znp_server.assoc_add_new @@ -897,7 +897,7 @@ async def test_request_recovery_assoc_restore( assoc_get = znp_server.assoc_get def rejoin_on_restore_lookup(**kwargs): - # Only the restore looks the device up after its removal + # Only the restore looks up after the removal entry = assoc_get(**kwargs) if entry is None and scenario == "rejoin_during_restore_lookup": @@ -915,7 +915,7 @@ def data_confirm_replier(req): nonlocal attempts attempts += 1 - # The first attempt expires and starts the recovery, the retry finds no route + # Attempt 1 expires (recovery), retry: no route if attempts == 2: if scenario == "rejoin_during_retry": app.handle_join(nwk=0x1234, ieee=device.ieee, parent_nwk=0x5678) diff --git a/zigpy_znp/zigbee/application.py b/zigpy_znp/zigbee/application.py index e6aed9d..e3b3d4b 100644 --- a/zigpy_znp/zigbee/application.py +++ b/zigpy_znp/zigbee/application.py @@ -37,7 +37,7 @@ EXTENDED_DATA_CONFIRM_TIMEOUT = 30 DEVICE_JOIN_MAX_DELAY = 5 -# Association table entries that the `send_packet` recovery may remove and restore +# Child relations the recovery may drop and restore CHILD_NODE_RELATIONS = frozenset( { c.util.NodeRelation.CHILD_RFD, @@ -969,9 +969,8 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: ): raise - # Only a direct child can be dropped and restored. A failed - # lookup returns 0xFFFE and a non-child relation on some - # builds (Koenkk/zigbee-herdsman#685). + # Not a direct child, or a miss (0xFFFE, see + # Koenkk/zigbee-herdsman#685): retry only if ( child_association.Device.shortAddr == 0xFFFE or child_association.Device.nodeRelation @@ -1028,8 +1027,7 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: assert child_association is not None removed = child_association.Device - # `AssocAdd` updates an existing entry in place, so skip it if the - # device rejoined, possibly under a new address, since the removal + # `AssocAdd` overwrites in place: skip if the device rejoined if device.nwk == removed.shortAddr: current = await self._znp.request( c.UTIL.AssocGetWithAddress.Req( @@ -1038,6 +1036,7 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: ) ) + # Again: a join may have arrived during the lookup if device.nwk == removed.shortAddr and ( current.Device.shortAddr == 0xFFFE or current.Device.nodeRelation == c.util.NodeRelation.NOTUSED From 152fa2f814a493d9a01cc36e99468bd505b4c2bd Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:34:19 +0000 Subject: [PATCH 6/8] Fix ZGP startup --- tests/application/test_joining.py | 2 +- tests/application/test_startup.py | 5 +++-- tests/conftest.py | 11 +++++++++++ zigpy_znp/zigbee/application.py | 3 ++- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/application/test_joining.py b/tests/application/test_joining.py index 886a92d..4565e9d 100644 --- a/tests/application/test_joining.py +++ b/tests/application/test_joining.py @@ -39,7 +39,7 @@ async def test_permit_join(device, mocker, make_application): request=zdo_request_matcher( dst_addr=t.AddrModeAddress(t.AddrMode.Broadcast, 0xFFFC), command_id=zdo_t.ZDOCmd.Mgmt_Permit_Joining_req, - TSN=2, + TSN=4, zdo_PermitDuration=10, zdo_TC_Significant=0, ), diff --git a/tests/application/test_startup.py b/tests/application/test_startup.py index 8394a86..34ed927 100644 --- a/tests/application/test_startup.py +++ b/tests/application/test_startup.py @@ -1,6 +1,7 @@ import pytest import voluptuous as vol from zigpy.exceptions import NetworkNotFormed +import zigpy.zgp from zigpy_znp.api import ZNP import zigpy_znp.commands as c @@ -103,10 +104,10 @@ async def test_endpoints(device, make_application): await app.startup(auto_form=False) - # We currently just register two endpoints - assert len(endpoints) == 2 + assert len(endpoints) == 3 assert 1 in app._device.endpoints assert 2 in app._device.endpoints + assert zigpy.zgp.GP_ENDPOINT in app._device.endpoints await app.shutdown() diff --git a/tests/conftest.py b/tests/conftest.py index f1591c6..7aea6e8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ import zigpy.endpoint import zigpy.zdo.types as zdo_t +import zigpy.zgp from zigpy_znp.api import ZNP import zigpy_znp.commands as c @@ -597,6 +598,16 @@ def permit_join(self, request): c.ZDO.MgmtPermitJoinRsp.Callback(Src=0x0000, Status=t.ZDOStatus.SUCCESS), ] + @reply_to( + c.AF.DataRequestExt.Req( + partial=True, + DstEndpoint=zigpy.zgp.GP_ENDPOINT, + SrcEndpoint=zigpy.zgp.GP_ENDPOINT, + ) + ) + def on_gp_broadcast(self, req): + return c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS) + @reply_to(c.AF.DataRequestExt.Req(partial=True, DstEndpoint=0)) def on_zdo_request(self, req): kwargs = deserialize_zdo_command(req.ClusterId, req.Data[1:]) diff --git a/zigpy_znp/zigbee/application.py b/zigpy_znp/zigbee/application.py index e3b3d4b..6c493f6 100644 --- a/zigpy_znp/zigbee/application.py +++ b/zigpy_znp/zigbee/application.py @@ -16,6 +16,7 @@ import zigpy.zcl import zigpy.zdo import zigpy.zdo.types as zdo_t +import zigpy.zgp from zigpy_znp.api import ZNP import zigpy_znp.commands as c @@ -669,7 +670,7 @@ def _find_endpoint(self, dst_ep: int, profile: int, cluster: int) -> int: return ZDO_ENDPOINT if profile == zigpy.profiles.zgp.PROFILE_ID: - return zigpy.profiles.zgp.GREENPOWER_ENDPOINT_ID + return zigpy.zgp.GP_ENDPOINT # Newer Z-Stack releases ignore profiles and will work properly with endpoint 1 if ( From 8d1447a40e804828b477365859726e236a12184d Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:41:43 +0000 Subject: [PATCH 7/8] Bump minimum zigpy to 2.4.0 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d3153d2..3e4067d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ readme = "README.md" license = {text = "GPL-3.0"} requires-python = ">=3.11" dependencies = [ - "zigpy>=1.5.0", + "zigpy>=2.4.0", "voluptuous", "coloredlogs", "jsonschema", From dc4df658947da4daf325fcc742b17792ec10107d Mon Sep 17 00:00:00 2001 From: Fabian <6667968+fabcoded@users.noreply.github.com> Date: Sun, 25 Oct 2026 12:57:47 +0100 Subject: [PATCH 8/8] Keep a child the firmware holds at another address than the host's The lookup can return the device as a direct child at an address other than `device.nwk`: the host missed an address update, or the firmware's entry is stale. Dropping that entry cannot help a send to the host's address, and the restore check then skipped the restore because the two addresses differed, so a valid entry was lost where `dev` restored it (review of 2026-10-23). Treat such a lookup like a miss: route discovery and one retry, table untouched. The `0xFFFE` miss is covered by the same comparison, so the restore check now fires only on an address change during the recovery. --- tests/application/test_requests.py | 54 ++++++++++++++++++++++++++++++ zigpy_znp/zigbee/application.py | 6 ++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/tests/application/test_requests.py b/tests/application/test_requests.py index 250627e..b0072aa 100644 --- a/tests/application/test_requests.py +++ b/tests/application/test_requests.py @@ -855,6 +855,60 @@ async def test_request_recovery_assoc_not_a_child( RFD = c.util.NodeRelation.CHILD_RFD +@pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) +async def test_request_recovery_assoc_other_address( + device_cls, make_application, mocker +): + # The firmware holds the device as a child at an address other than device.nwk: + # dropping it cannot help, and the entry must survive + app, znp_server = make_application(server_cls=device_cls) + + await app.startup(auto_form=False) + + mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1) + app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1 + + device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD) + entry = znp_server.assoc_add_new(nwk=0x1234, ieee=device.ieee, node_relation=RFD) + + did_assoc_remove = mocker.spy(znp_server, "assoc_remove") + did_assoc_add = mocker.spy(znp_server, "assoc_add_new") + + data_req = znp_server.reply_to( + c.AF.DataRequestExt.Req(partial=True), + responses=[ + c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS), + c.AF.DataConfirm.Callback( + Status=t.Status.MAC_TRANSACTION_EXPIRED, Endpoint=1, TSN=1 + ), + ], + ) + + was_route_discovered = znp_server.reply_to( + c.ZDO.ExtRouteDisc.Req(Dst=device.nwk, partial=True), + responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)], + ) + + with pytest.raises(DeliveryError): + await app.request( + device=device, + profile=260, + cluster=1, + src_ep=1, + dst_ep=1, + sequence=1, + data=b"\x00", + ) + + assert data_req.call_count == 2 + assert was_route_discovered.call_count == 1 + assert did_assoc_remove.call_count == 0 + assert did_assoc_add.call_count == 0 + assert znp_server.assoc_get(ieee=device.ieee) is entry + + await app.shutdown() + + @pytest.mark.parametrize("device_cls", [FormedLaunchpadCC26X2R1]) @pytest.mark.parametrize( ("scenario", "relation", "restored"), diff --git a/zigpy_znp/zigbee/application.py b/zigpy_znp/zigbee/application.py index 6c493f6..83504b3 100644 --- a/zigpy_znp/zigbee/application.py +++ b/zigpy_znp/zigbee/application.py @@ -970,10 +970,10 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: ): raise - # Not a direct child, or a miss (0xFFFE, see - # Koenkk/zigbee-herdsman#685): retry only + # Not a direct child, or not at device.nwk (a miss reads + # 0xFFFE, see Koenkk/zigbee-herdsman#685): retry only if ( - child_association.Device.shortAddr == 0xFFFE + child_association.Device.shortAddr != device.nwk or child_association.Device.nodeRelation not in CHILD_NODE_RELATIONS ):