Fix EmberStatus.NETWORK_BUSY not mapping to sl_Status.BUSY - #743
Conversation
`NETWORK_BUSY` means the network is overloaded, not that the APS unicast in-flight message limit was hit. Current EmberZNet firmware reports the same condition as `SL_STATUS_BUSY`, so map the legacy status to match. Fixes zigpy#742
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #743 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 64 64
Lines 4263 4263
=======================================
Hits 4244 4244
Misses 19 19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR corrects the legacy-to-modern status normalization in bellows so that EmberStatus.NETWORK_BUSY maps to the modern sl_Status.BUSY, aligning older EmberZNet firmware behavior with current Simplicity SDK firmware and avoiding misleading “unicast table” error reporting.
Changes:
- Remap
EmberStatus.NETWORK_BUSYfromsl_Status.ZIGBEE_MAX_MESSAGE_LIMIT_REACHEDtosl_Status.BUSY. - Preserve the existing mapping of
EmberStatus.MAX_MESSAGE_LIMIT_REACHEDtosl_Status.ZIGBEE_MAX_MESSAGE_LIMIT_REACHED(no collapse of distinct conditions).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zigpy-review-bot
left a comment
There was a problem hiding this comment.
Reviewed at d495498. The change is correct, and I audited the rest of the table for the same class of error as requested.
The change itself
EmberStatus.NETWORK_BUSY (0xA1) and EmberStatus.MAX_MESSAGE_LIMIT_REACHED (0x72) are two distinct conditions, and mapping both onto sl_Status.ZIGBEE_MAX_MESSAGE_LIMIT_REACHED (0x0C03) collapsed them. Silabs' own EZSP command docs describe them separately under the new API: SL_STATUS_BUSY is "insufficient resources available in Network or MAC layers to send message" (the network-overload / send-threshold case), while SL_STATUS_ZIGBEE_MAX_MESSAGE_LIMIT_REACHED is the APS unicast in-flight limit. MAX_MESSAGE_LIMIT_REACHED keeps its own entry on the line above, so no sl_Status value is orphaned by the change.
Independent confirmation: zigbee-herdsman keeps an equivalent EmberStatus → SLStatus table for pre-v14 NCPs (src/adapter/ember/ezsp/buffalo.ts), and its entry is [0xa1 /*NETWORK_BUSY*/, SLStatus.BUSY].
The user-visible effect matches your Impact section: on EZSP v4–v13 the raw status is converted inside the per-version send_multicast / send_broadcast wrappers, and ControllerApplication.send_packet raises DeliveryError(f"Failed to enqueue message: {status!r}") with the already-converted value — so group-command spam surfaced the wrong constant name. Nothing in bellows, zigpy or zha branches on either constant, so the change is limited to the reported status, with no control-flow effect.
Full test suite green (441 passed). Nothing under tests/ references SL_STATUS_MAP or asserts individual mappings, so there was no test pinning the old value — adding one for this pair would be reasonable, but it would be the first of its kind here, so I don't consider it blocking.
Other mappings — audit
I diffed the whole SL_STATUS_MAP against herdsman's table by numeric value, and separately diffed EmberStatus against sl_Status for unmapped members.
Wrong mappings: with this PR applied there are zero disagreements across the 37 entries the two tables share. Before it, NETWORK_BUSY was the only one — so this was the last incorrect mapping among the codes for which an independent reference exists.
One missing entry with a direct reference: herdsman maps 0xBF TRUST_CENTER_SWAPPED_OUT_EUI_HAS_NOT_CHANGED → ZIGBEE_TRUST_CENTER_SWAP_EUI_HAS_NOT_CHANGED. bellows doesn't define 0xBF in EmberStatus at all, even though sl_Status.ZIGBEE_TRUST_CENTER_SWAP_EUI_HAS_NOT_CHANGED (0x0C11) already exists and the 0xBC "has changed" counterpart is mapped. That looks like a genuine gap worth closing.
Unmapped members with unambiguous counterparts: 77 of 154 EmberStatus members are unmapped. Most are legitimately host-irrelevant (bootloader, SIM EEPROM, ADC, serial framing, APPLICATION_ERROR_*), but the 0x0030–0x0060 block of sl_status.h is the old PHY/MAC/flash/NVM3 block renumbered 1:1, so a set of exact pairs is simply absent. Two families are already partially mapped, which is what makes the gaps look accidental rather than deliberate: PHY_TX_BLOCKED, PHY_TX_SCHED_FAIL, PHY_TX_CCA_FAIL and PHY_INVALID_CHANNEL are mapped while five other PHY_* are not; six MAC_* are mapped while four are not.
The 22 pairs I verified exist on both sides and are currently unmapped
EmberStatus |
sl_Status |
|
|---|---|---|
PHY_TX_UNDERFLOW |
0x88 |
TRANSMIT_UNDERFLOW (0x0032) |
PHY_TX_INCOMPLETE |
0x89 |
TRANSMIT_INCOMPLETE (0x0033) |
PHY_INVALID_POWER |
0x8B |
TRANSMIT_INVALID_POWER (0x0056) |
PHY_TX_BUSY |
0x8C |
TRANSMIT_BUSY (0x0034) |
PHY_ACK_RECEIVED |
0x8F |
TRANSMIT_ACK_RECEIVED (0x0057) |
MAC_BAD_SCAN_DURATION |
0x33 |
BAD_SCAN_DURATION (0x0050) |
MAC_INCORRECT_SCAN_TYPE |
0x34 |
MAC_INCORRECT_SCAN_TYPE (0x004E) |
MAC_COMMAND_TRANSMIT_FAILURE |
0x36 |
MAC_COMMAND_TRANSMIT_FAILURE (0x0044) |
MAC_UNKNOWN_HEADER_TYPE |
0x3A |
MAC_UNKNOWN_HEADER_TYPE (0x0042) |
MAC_ACK_HEADER_TYPE |
0x3B |
MAC_ACK_HEADER_TYPE (0x0043) |
EEPROM_MFG_VERSION_MISMATCH |
0x06 |
EEPROM_MFG_VERSION_MISMATCH (0x0039) |
EEPROM_STACK_VERSION_MISMATCH |
0x07 |
EEPROM_STACK_VERSION_MISMATCH (0x003A) |
ERR_FLASH_WRITE_INHIBITED |
0x46 |
FLASH_WRITE_INHIBITED (0x003B) |
ERR_FLASH_VERIFY_FAILED |
0x47 |
FLASH_VERIFY_FAILED (0x003C) |
ERR_FLASH_PROG_FAIL |
0x4B |
FLASH_PROGRAM_FAILED (0x003D) |
ERR_FLASH_ERASE_FAIL |
0x4C |
FLASH_ERASE_FAILED (0x003E) |
NVM3_TOKEN_NO_VALID_PAGES |
0xC0 |
NVM3_NO_VALID_PAGES (0x005E) |
NVM3_ERR_OPENED_WITH_OTHER_PARAMETERS |
0xC1 |
NVM3_OPENED_WITH_OTHER_PARAMETERS (0x005D) |
NVM3_ERR_ALIGNMENT_INVALID |
0xC2 |
NVM3_ALIGNMENT_INVALID (0x0059) |
NVM3_ERR_SIZE_TOO_SMALL |
0xC3 |
NVM3_SIZE_TOO_SMALL (0x005A) |
NVM3_ERR_PAGE_SIZE_NOT_SUPPORTED |
0xC4 |
NVM3_PAGE_SIZE_NOT_SUPPORTED (0x005B) |
NVM3_ERR_TOKEN_INIT |
0xC5 |
NVM3_TOKEN_INIT_FAILED (0x005C) |
Caveat on that list: I verified the name/semantic correspondence, not that each code actually reaches the host over EZSP — I didn't trace a specific command returning e.g. PHY_ACK_RECEIVED. The NVM3 ones are the most plausibly reachable, since bellows does drive the NV3 token APIs for backup/restore and repairs. Practically, an unmapped status means from_ember_status logs Unknown status …, converting to generic FAIL and returns FAIL, so each gap costs a warning plus a lost distinction — the same class of problem this PR fixes, just less visible.
None of that blocks this PR; it's follow-up material if you want it.
This PR fixes an issue where the Gecko SDK's
EmberStatus.NETWORK_BUSYwas remapped tosl_Status.ZIGBEE_MAX_MESSAGE_LIMIT_REACHED, instead ofsl_Status.BUSY.Current EmberZNet firmware built with the newer Simplicity SDK reports hitting the broadcast send threshold as
sl_Status.BUSY, so this PR changes the old status to map to that one.EmberStatus.NETWORK_BUSYis mapped toZIGBEE_MAX_MESSAGE_LIMIT_REACHED, reporting broadcast congestion as a unicast-table error #742Impact in HA
Without this PR, there's an issue where spamming group commands in Home Assistant showed the incorrect status
ZIGBEE_MAX_MESSAGE_LIMIT_REACHEDin an error message, which is for the unicast message limit.@zigpy-review-bot Review this PR and also check if other mappings need to be changed (or added).