Fix MgmtRtgRsp.Routes not being marked as optional

[fdelacou]

Closes #278

Summary

ZDO.MgmtRtgRsp's response schema (zigpy_znp/commands/zdo.py) declares its Routes parameter as required. Per the Zigbee spec, a device that returns Status: NOT_SUPPORTED for a routing table request legitimately omits the routes list — the frame is genuinely shorter, not corrupted or malformed.

Because Routes isn't marked optional, CommandBase.from_frame() (zigpy_znp/types/commands.py) runs out of data while trying to deserialize it, hits the "out of data but required" branch, and raises:

```
ValueError: Frame data is truncated (parsed {...}), required parameter remains: Param(name='Routes', ...)
```

This exception is unhandled at the point frames are processed, which halts processing of whatever comes next on the wire — meaning a single NOT_SUPPORTED response from any router on the mesh can silently drop check-ins from unrelated devices queued behind it. In my case this manifested as sleepy Zigbee end-devices (battery sensors) going unavailable with zero errors logged under their own address — the actual failure was upstream, triggered by a completely different device's routing response. Full details and log excerpts are in #278.

Fix

One-line change: mark Routes optional on MgmtRtgRsp, exactly as Neighbors is already marked optional two command definitions earlier in the same file:

```python

Before

t.Param("Routes", zigpy.zdo.types.Routes, "Routes"),

After

t.Param("Routes", zigpy.zdo.types.Routes, "Routes", optional=True),
```

from_frame()'s existing optional-handling logic already does the right thing once this flag is set — it cleanly stops parsing and returns Routes=None instead of raising.

This also brings zigpy-znp's TI Z-Stack-specific schema in line with upstream zigpy's own generic ZDO schema, which already treats this field as optional:

```python

zigpy/zdo/types.py

ZDOCmd.Mgmt_Rtg_rsp: (STATUS, ("Routes", t.Optional(Routes))),
```

Testing

Reproduced the crash against production logs (Status.NOT_SUPPORTED responses from two different routers on my mesh, both triggering the exact ValueError above — see #278). Applied this exact change as a runtime patch against a live Home Assistant / zigpy-znp instance and confirmed the schema now correctly reports Routes as optional=True with no regressions to normal (non-NOT_SUPPORTED) MgmtRtgRsp parsing.

Related

Sleepy end-devices affected by the resulting dropped check-ins do not self-heal and require a manual re-pair — this fix should eliminate that class of failure at the source rather than requiring the workaround.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.49%. Comparing base (614f74e) to head (5ae3f9d).

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #279   +/-   ##
=======================================
  Coverage   98.49%   98.49%           
=======================================
  Files          43       43           
  Lines        3583     3583           
=======================================
  Hits         3529     3529           
  Misses         54       54           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[zigpy-review-bot]

Reviewed at 5ae3f9d. The one-line change is correct and I'd merge it — but the impact described in the PR body (and in #278) doesn't survive a look at uart.py, and I'd like the regression test that the precedent PR shipped with.

The fix itself is right

• Matches upstream. zigpy/zdo/types.py:723 already has ZDOCmd.Mgmt_Rtg_rsp: (STATUS, ("Routes", t.Optional(Routes))), and that follows the spec: Mgmt_Rtg_rsp only carries RoutingTableEntries / StartIndex / RoutingTableList when Status == SUCCESS, so a NOT_SUPPORTED response legitimately ends after the status byte.
• Matches in-repo precedent. MgmtLqiRsp.Neighbors (zigpy_znp/commands/zdo.py:1184, thirteen lines up) was marked optional for the byte-for-byte identical NOT_SUPPORTED crash in #228 (406b21f). This is that same fix for the sibling command.
• Verified against the real frame. Replaying the exact payload from #278's log on this branch:

>>> c.ZDO.MgmtRtgRsp.Callback.from_frame(GeneralFrame(header, b"\x85\xcc\x84"))
ZDO.MgmtRtgRsp.Callback(Src=0xCC85, Status=<Status.NOT_SUPPORTED: 132>, Routes=None)
round-trip back to b"\x85\xcc\x84": True

• No consumer regresses. Nothing in the codebase reads MgmtRtgRsp.Callback.Routes, so Routes=None can't propagate anywhere. Full suite green here (390 passed).

Please add the regression test

#228 landed the same one-line change together with a test, and copying it is nearly free. Suggested addition to tests/test_commands.py, right after test_neighbors_missing_payload, using the real frame from #278:

def test_routes_missing_payload():
    frame = frames.GeneralFrame(
        header=t.CommandHeader(
            id=0xB2,
            subsystem=t.Subsystem.ZDO,
            type=t.CommandType.AREQ,
        ),
        data=b"\x85\xCC\x84",
    )

    assert c.ZDO.MgmtRtgRsp.Callback.from_frame(frame) == c.ZDO.MgmtRtgRsp.Callback(
        Src=0xCC85,
        Status=t.ZDOStatus.NOT_SUPPORTED,
    )

I ran that test against this branch and it passes (and it fails on dev, as it should).

Re: the stated impact — the exception does not halt frame processing

This doesn't change my verdict on the diff, but it matters for how #278 gets closed, so it's worth being explicit about.

The PR body says the exception "halts processing of whatever comes next on the wire — meaning a single NOT_SUPPORTED response from any router on the mesh can silently drop check-ins from unrelated devices queued behind it." That isn't what happens. In zigpy_znp/uart.py:51-62 the try/except Exception sits inside the for frame in self._extract_frames() loop body:

for frame in self._extract_frames():
    try:
        self._api.frame_received(frame.payload)
    except Exception as e:
        LOGGER.error("Received an exception while passing frame to API: %s", frame, exc_info=e)

The failure is contained to the single offending frame and the loop continues to the next one. The Received an exception while passing frame to API line quoted in #278 is that handler firing — so the log is evidence that the frame was isolated, not that the pipeline stopped.

And the one frame that does get dropped carries nothing zigpy was waiting on. zigpy-znp registers ZDO.MsgCallbackRegister.Req(ClusterId=0xFFFF) (application.py:163) and relays real ZDO responses to zigpy via ZDO.MsgCbIncoming → on_zdo_message (application.py:376). No listener is ever registered for ZDO.MgmtRtgRsp.Callback, so with this patch applied the frame parses and then falls straight through to _unhandled_command() → LOGGER.debug("Command was not handled").

So the concrete benefit of this PR is removing a spurious ERROR-with-traceback from the log for a frame that is then discarded anyway — genuinely worth fixing (that log line sends people chasing ghosts, as it did here), but it is not a data-loss path. I'd expect the sleepy-device drops described in #278 to have a different root cause, so I'd suggest not auto-closing #278 on merge, or at least re-checking whether the Aqara/Third Reality devices still go unavailable afterwards. Happy to be shown wrong if there's a log where the error is followed by an actually-missing device message.

Not blocking — the same latent bug elsewhere

MgmtNwkDiscRsp (0xB0) and MgmtBindRsp (0xB3) have the identical shape: trailing fields the spec omits on a non-SUCCESS status, but declared required here. Upstream zigpy leaves those required too (with an explicit # ... TODO optional stuff ... next to them in zdo/types.py), so this is pre-existing and clearly out of scope for a targeted fix — just noting it in case a maintainer wants a follow-up.

Nice, well-evidenced bug report and a minimal fix. Just the test, please.

[zigpy-review-bot]

Please pair this with a regression test, the way the identical Neighbors fix in #228 (406b21f) did — that PR added test_neighbors_missing_payload to tests/test_commands.py alongside the one-line schema change.

Dropping this in right after test_neighbors_missing_payload, using the real frame from #278, is enough:

def test_routes_missing_payload():
    frame = frames.GeneralFrame(
        header=t.CommandHeader(
            id=0xB2,
            subsystem=t.Subsystem.ZDO,
            type=t.CommandType.AREQ,
        ),
        data=b"\x85\xCC\x84",
    )

    assert c.ZDO.MgmtRtgRsp.Callback.from_frame(frame) == c.ZDO.MgmtRtgRsp.Callback(
        Src=0xCC85,
        Status=t.ZDOStatus.NOT_SUPPORTED,
    )

I ran it both ways: it passes on this branch, and on dev it fails with the exact ValueError from #278's traceback (zigpy_znp/types/commands.py:424).