Fix EZSP v14+ leaveNetwork and setManufacturerCode schemas - #749
Conversation
Since EZSP v14, `leaveNetwork` takes a one-byte `options` argument and `setManufacturerCode` returns an `sl_status_t`. The v14 table inherited both v4-era layouts, so bellows sent `leaveNetwork` without the byte the NCP reads and left the `setManufacturerCode` status as trailing data.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #749 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 64 64
Lines 4269 4281 +12
=======================================
+ Hits 4250 4262 +12
Misses 19 19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| with self.wait_for_stack_status(t.sl_Status.NETWORK_DOWN) as stack_status: | ||
| (status,) = await self._command("leaveNetwork") | ||
| if self._ezsp_version >= 14: |
There was a problem hiding this comment.
@zigpy-review-bot Turn leave_network into a version-specific protocol command accepting options. We generally do not want version branching like this. I believe this same options enum is used by ZDO so use the zigpy one directly in the shared interface to avoid EZSP internals from leaking out. For the V4 implementation, throw an error if anything other than "no option" is passed.
There was a problem hiding this comment.
Pushed dd592dd. ProtocolHandler now has an abstract leave_network(options) that takes zigpy's ZDO.LeaveOptions, and EZSP.leaveNetwork() calls it (passing options through) instead of checking the version. EZSPv4 raises ValueError for anything other than LeaveOptions.NONE.
One correction: the ZDO and EZSP enums don't share values. zigpy's LeaveOptions uses the Mgmt_Leave_req layout (RemoveChildren = 0x40, Rejoin = 0x80). EZSP's sl_zigbee_leave_network_option_t uses the NWK leave command layout: WITH_OPTION_REJOIN = 0x20, IS_REQUESTED = 0x40, and 0x80 is SL_ZIGBEE_LEAVE_NWK_SILENTLY, a testing-only "leave without broadcasting a Network Leave" bit that used to be remove-children. Passing zigpy's value straight through would turn Rejoin into a silent leave. So EZSPv14 translates NONE → WITH_NO_OPTION and Rejoin → WITH_OPTION_REJOIN, and raises ValueError for anything that includes RemoveChildren, since EZSP has no equivalent. SlZigbeeLeaveNetworkOption is now only used in the v14 command schema.
The PR description's leaveNetwork bullet still describes the old version check. Re-mention me if you want it updated, or if you'd rather handle RemoveChildren differently.
Verification
- Enum values:
stack/include/sl_zigbee_types.h(SiSDK) forsl_zigbee_leave_network_option_t;zigpy/zdo/__init__.pyZDO.LeaveOptionsfor the ZDO side. - New tests: v4
leave_network()with no options, andValueErrorforRejoin/RemoveChildren; the v14 option mapping and theRemoveChildrenrejection; theEZSP.leaveNetwork()pass-through, error, and timeout paths now mock the protocol'sleave_network. - Full suite: 466 passed. The changed lines in
v4/__init__.pyandv14/__init__.pyare fully covered. pre-commit passes under Python 3.12 (the pinned pyupgrade/mypy hooks crash under 3.14, which is an environment issue).
There was a problem hiding this comment.
@zigpy-review-bot Hmm, t.SlZigbeeLeaveNetworkOption indeed looks cleaner. Let's avoid the ZDO remapping and just use the SiLabs enum directly.
There was a problem hiding this comment.
Pushed 8c8fbba. The ZDO remapping is gone: ProtocolHandler.leave_network() and EZSP.leaveNetwork() now take t.SlZigbeeLeaveNetworkOption (default WITH_NO_OPTION). EZSPv14 passes it straight to the NCP. EZSPv4 still raises ValueError for anything other than WITH_NO_OPTION. Nothing in bellows imports zigpy.zdo for this anymore.
The PR description's leaveNetwork bullet still describes the original version check. I haven't edited it. Re-mention me if you want it updated.
Verification
- Tests updated: v14 now checks the default and a
WITH_OPTION_REJOINpass-through. v4 checks thatWITH_OPTION_REJOIN/IS_REQUESTEDraise. TheEZSP.leaveNetwork()pass-through test uses the SiLabs enum. The ZDO-mapping tests are removed. - Full suite: 464 passed. The changed lines in
v4/__init__.pyandv14/__init__.pyare fully covered. pre-commit passes under Python 3.12.
The shared protocol interface takes zigpy's ZDO `LeaveOptions` instead of branching on the EZSP version. EZSPv4 rejects any option, since its `leaveNetwork` has no options argument. EZSPv14 translates `Rejoin` into `SlZigbeeLeaveNetworkOption.WITH_OPTION_REJOIN` and rejects `RemoveChildren`: EZSP uses the NWK leave command's bit layout, where 0x40 means the leave was requested and 0x80 is a testing-only "leave silently" bit, so the ZDO values can't be passed through.
Drop the ZDO `LeaveOptions` translation: `ProtocolHandler.leave_network` and `EZSP.leaveNetwork` now take the SiLabs enum. v14+ passes it through to the NCP unchanged; v4 raises `ValueError` for anything other than `WITH_NO_OPTION`.
2 participants
Since EZSP v14,
leaveNetworktakes a one-byteoptionsargument andsetManufacturerCodereturns ansl_status_t. bellows' v14 command table inherited both v4-era layouts:_REPLACEMENTSonly swapsEmberStatus/EzspStatusforsl_Status, and v16–v19 spread the v14 table. Found in the audit on #747.leaveNetwork(0x0020): on v14+, bellows sent the command with no argument, but the NCP readsoptions = fetchInt8u();regardless, so it read one byte past the end of the frame.EZSP.leaveNetwork()now sendsoptions=SlZigbeeLeaveNetworkOption.WITH_NO_OPTION(0x00) on v14+, the way the SDK host and zigbee-herdsman do. Pre-v14 behaviour is unchanged.setManufacturerCode(0x0015): on v14+, the 4-byte status was left over as trailing data. The v14 table now parses it. The call site inapplication.pyignores the result, as before.Both are overridden in
bellows/ezsp/v14/commands.py, so v16–v19 pick them up through inheritance. New tests check theleaveNetworkframe bytes and both response layouts through every v14+ table, and the version gate inEZSP.leaveNetwork(). The full suite passes.Related: #747. The other v14+ drift from that audit is covered by #748 (
getExtendedTimeout/lookupNodeIdByEui64) and #750 (the definitions bellows doesn't call).Verification
command-functions.h):ezspLeaveNetwork(void)sends no argument and reads a 1-byte status.ezspSetManufacturerCodereads nothing back.sl_zigbee_ezsp_leave_network(options)doesappendInt8u(options)thenfetchInt32u().sl_zigbee_ezsp_set_manufacturer_codedoesfetchInt32u(). The NCP handlers inapp/em260/command-handlers-networking-generated.cmatch. Neither command changed between v14 and v19.sl_zigbee_types.h.🤖 Generated with Claude Code