From f774ddf781c13559bae1550e5a3e7aa0c1509018 Mon Sep 17 00:00:00 2001 From: zigpy-review-bot <286747149+zigpy-review-bot@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:04:28 +0200 Subject: [PATCH] Fix spurious "Unknown status" warning from XNCP replies on Simplicity SDK firmware The XNCP frame's one status byte is typed by the firmware as the SDK's native status type, so it is an `EmberStatus` on Gecko SDK 4.x builds but the low octet of an `sl_status_t` on Simplicity SDK builds. bellows decoded it as an `EmberStatus` unconditionally, so a normal "this token has no override" reply (0x2D, `SL_STATUS_NOT_FOUND`) became `undefined_0x2d` and tripped the "Unknown status" warning on every startup and every periodic backup. 0x21 (`SL_STATUS_INVALID_PARAMETER`) was worse: being a valid `EmberStatus`, it silently mis-decoded as the unrelated `SERIAL_INVALID_PORT`. Give the byte its own `XncpStatus` enum and compare against it directly instead of routing it through `sl_Status.from_ember_status()`, which translates EZSP command statuses. For all 256 status byte values the old and new success tests agree on whether to raise `InvalidCommandError`, so only the log line changes. --- bellows/ezsp/__init__.py | 2 +- bellows/ezsp/xncp.py | 35 +++++++++++++++++++++++++---------- tests/test_xncp.py | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/bellows/ezsp/__init__.py b/bellows/ezsp/__init__.py index 734dc080..a158018e 100644 --- a/bellows/ezsp/__init__.py +++ b/bellows/ezsp/__init__.py @@ -770,7 +770,7 @@ async def send_xncp_frame( LOGGER.debug("Received XNCP frame: %s", rsp_frame) - if t.sl_Status.from_ember_status(rsp_frame.status) != t.sl_Status.OK: + if rsp_frame.status != xncp.XncpStatus.OK: raise InvalidCommandError(f"XNCP response error: {rsp_frame.status}") return rsp_frame.payload diff --git a/bellows/ezsp/xncp.py b/bellows/ezsp/xncp.py index aa1eb2b4..1048e967 100644 --- a/bellows/ezsp/xncp.py +++ b/bellows/ezsp/xncp.py @@ -7,13 +7,7 @@ import zigpy.types as t -from bellows.types import ( - EmberApsFrame, - EmberStatus, - EzspMfgTokenId, - RouteRecordStatus, - sl_Status, -) +from bellows.types import EmberApsFrame, EzspMfgTokenId, RouteRecordStatus, sl_Status _LOGGER = logging.getLogger(__name__) @@ -68,24 +62,45 @@ class XncpCommandId(t.enum16): UNKNOWN = 0xFFFF +class XncpStatus(t.enum8): + """Status byte of an XNCP frame. + + The firmware types this byte as the SDK's native status type: an `EmberStatus` in + Gecko SDK 4.x builds, the low octet of an `sl_status_t` in Simplicity SDK builds. + The values current firmware emits happen not to overlap, so both encodings fit + into one enum, but only `OK` (zero under either) is relied upon: every other value + is treated as an opaque failure. + """ + + OK = 0x00 + + # Gecko SDK 4.x: `EmberStatus` + EMBER_BAD_ARGUMENT = 0x02 + EMBER_NOT_FOUND = 0x03 + + # Simplicity SDK: low octet of an `sl_status_t` + SL_STATUS_INVALID_PARAMETER = 0x21 + SL_STATUS_NOT_FOUND = 0x2D + + @dataclasses.dataclass class XncpCommand: command_id: XncpCommandId - status: EmberStatus + status: XncpStatus payload: XncpCommandPayload @classmethod def from_payload(cls, payload: XncpCommandPayload) -> XncpCommand: return cls( command_id=REV_COMMANDS[type(payload)], - status=EmberStatus.SUCCESS, + status=XncpStatus.OK, payload=payload, ) @classmethod def from_bytes(cls, data: bytes) -> XncpCommand: command_id, data = XncpCommandId.deserialize(data) - status, data = EmberStatus.deserialize(data) + status, data = XncpStatus.deserialize(data) if command_id not in COMMANDS: raise ValueError( diff --git a/tests/test_xncp.py b/tests/test_xncp.py index 74a099d3..451f5e8e 100644 --- a/tests/test_xncp.py +++ b/tests/test_xncp.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging from unittest.mock import AsyncMock, call, patch import pytest @@ -17,7 +18,7 @@ async def test_xncp_failure(ezsp_f: EZSP) -> None: command = xncp.XncpCommand.from_payload( xncp.GetSupportedFeaturesRsp(features=xncp.FirmwareFeatures.MANUAL_SOURCE_ROUTE) ) - command.status = t.EmberStatus.ERR_FATAL + command.status = xncp.XncpStatus.EMBER_BAD_ARGUMENT ezsp_f._mock_commands["customFrame"] = customFrame = AsyncMock( return_value=[ @@ -34,6 +35,39 @@ async def test_xncp_failure(ezsp_f: EZSP) -> None: ] +@pytest.mark.parametrize( + "rsp", + [ + # Gecko SDK 4.x firmware replies with an `EmberStatus`: `EMBER_NOT_FOUND` + b"\x02\x80\x03", + # Simplicity SDK firmware replies with the low octet of an + # `sl_status_t`: `SL_STATUS_NOT_FOUND` + b"\x02\x80\x2D", + ], +) +async def test_xncp_missing_mfg_token_override( + ezsp_f: EZSP, rsp: bytes, caplog +) -> None: + """Test that a token without an override fails quietly, regardless of the SDK.""" + ezsp_f._mock_commands["customFrame"] = AsyncMock( + return_value=[t.EmberStatus.SUCCESS, rsp] + ) + ezsp_f._mock_commands["getMfgToken"] = AsyncMock(return_value=[b"\xFF" * 8]) + ezsp_f._xncp_features |= xncp.FirmwareFeatures.MFG_TOKEN_OVERRIDES + + with caplog.at_level(logging.WARNING, logger="bellows"): + assert ( + await ezsp_f.get_mfg_token(t.EzspMfgTokenId.MFG_CUSTOM_EUI_64) + ) == b"\xFF" * 8 + + # An unsupported override is an expected condition, not a warning + assert [ + r.getMessage() + for r in caplog.records + if r.name.startswith("bellows.") and r.levelno >= logging.WARNING + ] == [] + + async def test_xncp_failure_multiprotocol(ezsp_f: EZSP) -> None: """Test XNCP failure with multiprotocol firmware.""" ezsp_f._mock_commands["customFrame"] = customFrame = AsyncMock(