Fix spurious "Unknown status" warning from XNCP replies on Simplicity SDK firmware - #744
Fix spurious "Unknown status" warning from XNCP replies on Simplicity SDK firmware#744zigpy-review-bot wants to merge 1 commit into
Conversation
|
One adjacent thing I noticed while tracing this. It's a firmware-side observation, not something to fix here, but flagging it in case it's useful upstream. The XNCP status slot is a single byte ( That's harmless today, because only three values are ever returned and all of them survive truncation (
The bounded part: no error value in Nothing to do in Separately, a documentation nit in the same header, since it caused a real mistake: the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #744 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 64 64
Lines 4263 4269 +6
=======================================
+ Hits 4244 4250 +6
Misses 19 19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… 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.
9d0cba4 to
f774ddf
Compare
The problem
On firmware built against Simplicity SDK, every startup and every periodic network backup logs a
WARNINGthat reads like a real failure but is a normal, expected condition:Seen on a Home Assistant Connect ZBT-2 running
9.1.1.0 build 0 (20260805160939)— a builder artifact from Simplicity SDK 2026.6.1, not a released image.Worth being explicit that no shipped firmware hits this yet: silabs-firmware-builder's latest release (
v2026.02.23) predates its Simplicity SDK migration, so released images still take the Gecko SDK path and decode fine. This is a fix ahead of the planned Simplicity SDK beta rather than a response to breakage in the field.Root cause
An XNCP frame is a
uint16command ID, then one status byte, then the payload. The firmware types that byte as the SDK's native status type, so its meaning depends on which SDK the image was built against (xncp_types.h):EmberStatus0x000x020x03sl_status_t, truncated to one byte0x000x210x2DThe split is on
#ifdef STACK_TYPES_HEADER, so it tracks the SDK family rather than any particular release.bellowsdecodes that byte as anEmberStatusunconditionally and then runs it throughsl_Status.from_ember_status(). On Simplicity SDK firmware this goes wrong two ways:0x2D(SL_STATUS_NOT_FOUND) is not a validEmberStatus, so it becomesundefined_0x2dand trips the "Unknown status" warning.0x21(SL_STATUS_INVALID_PARAMETER) is worse, because it is a validEmberStatus— it silently mis-decodes as the unrelatedEmberStatus.SERIAL_INVALID_PORT, with no warning at all.The path that hits this on every startup is
load_network_info()→can_burn_userdata_custom_eui64()→get_mfg_token(MFG_CUSTOM_EUI_64). The firmware only overridesMFG_STRINGandMFG_BOARD_NAME, soNOT_FOUNDfor any other token is the correct, expected reply, andget_mfg_token()already handles it by falling back to the raw token value. Only the log line is wrong.The fix
Give the XNCP status byte its own
XncpStatusenum instead of borrowingEmberStatus, and compare against it directly rather than routing it throughsl_Status.from_ember_status()— that helper translates EZSP command statuses, and its warning is a genuinely useful signal that this was drowning out.The values the two encodings actually use don't collide, so both fit in one enum. But
bellowsonly ever needs OK vs. not-OK here, and0x00is success under either interpretation, so nothing depends on that staying true if firmware adds status values later.No behaviour change beyond the log line
I checked this exhaustively rather than by inspection. For all 256 possible status byte values, the old and new success tests agree on whether to raise
InvalidCommandError:That follows from the only
SL_STATUS_MAPentries producingsl_Status.OKbeingEmberStatus.SUCCESSandEzspStatus.SUCCESS, both0x00. Request frames also serialize to the same bytes as before, sinceXncpStatus.OKandEmberStatus.SUCCESSare both0x00in one byte.Why not just extend
SL_STATUS_MAPAdding
0x2Dthere would silence the warning, but unlike #742/#743 — whereNETWORK_BUSYreally is anEmberStatusthat was mapped to the wrongsl_Status— this byte isn't anEmberStatusvalue at all, so there's nothing correct to map it to. It would also leave the0x21→SERIAL_INVALID_PORTmis-decode in place.Tests
Added a regression test covering both encodings of "this token has no override", asserting that the call succeeds and logs no warning. It fails on
devfor the0x2Dcase and passes for both with this change. Full suite: 443 passed.