Implement the combined XNCP unicast API

[puddly]

Feature detect XNCP_FEATURE_COMBINED_SEND to reduce round trips when sending unicast commands. This is especially useful for networked coordinators.

Firmware PR: NabuCasa/silabs-firmware-builder#209

[Copilot]

Pull request overview

Adds support for the combined XNCP unicast send API (COMBINED_SEND) so the stack can apply extended timeout and (manual) source routing and send a unicast in a single custom EZSP frame, reducing round trips (notably beneficial for networked coordinators).

Changes:

• Add new XNCP command/feature definitions for SEND_UNICAST_{REQ,RSP} and COMBINED_SEND.
• Route ControllerApplication.send_packet() down the combined-send path when supported and payload size permits, with fallbacks for oversized payloads and native source-route cases.
• Add/extend tests to validate request shaping, combined-send selection, and fallback behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/test_ezsp.py	Adds unit coverage for xncp_send_unicast() request encoding (flags + optional fields).
tests/test_application.py	Adds integration-style tests ensuring send_packet() uses combined send when available and falls back when it can’t.
bellows/zigbee/application.py	Introduces combined-send selection logic and a payload-size threshold constant.
bellows/ezsp/xncp.py	Defines the new XNCP command IDs, feature bit, request/response structs, and flags.
bellows/ezsp/init.py	Implements xncp_send_unicast() on the EZSP layer to send the new combined command.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[puddly]

The newly-added APS encryption option is encoded within the APS frame itself so it doesn't need to be included in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.55%. Comparing base (cd25d97) to head (8fad743).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #732   +/-   ##
=======================================
  Coverage   99.54%   99.55%           
=======================================
  Files          64       64           
  Lines        4205     4263   +58     
=======================================
+ Hits         4186     4244   +58     
  Misses         19       19           

☔ 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 together with the firmware side (NabuCasa/silabs-firmware-builder#209): the wire format agrees byte-for-byte between the two (APS frame layout, conditional ieee+Bool and LV source-route fields keyed off the same flag bits, u32 status + u8 sequence reply), and the feature bit / command id match. The firmware's apply_extended_timeout also faithfully mirrors the host-side set_extended_timeout logic, including the random address-table-entry replacement, so behavior is preserved when the round trips move into the NCP. Fallback gating looks right and is well tested: native (non-manual) source routes, oversized payloads, and firmwares without COMBINED_SEND all take the existing multi-command path, and the ACK ⇒ no-extended-timeout rule carries over. 437 tests pass locally.

Edit (2026-07-28): the size-guard comment referenced below turned out to be partially incorrect (wrong transport mechanism, overstated impact) — the verified analysis is in the follow-up comment: #732 (comment)

Original closing line (superseded)

One substantive comment on the size guard below.

[zigpy-review-bot]

Follow-up from a second, independent review pass (a duplicate /review-request session that correctly stood down when the review above posted first). Two minor points from it worth recording — neither blocking:

• No byte-level test for SendUnicastReq. test_xncp_send_unicast patches send_xncp_frame and asserts on the request struct (mock_send.mock_calls == [call(xncp.SendUnicastReq(...))]), and the application.py tests mock xncp_send_unicast itself — so nothing serializes the new command to bytes. A field-order or width regression against the firmware's hand-rolled parser would stay green. The other XNCP commands have byte-level round-trips in tests/test_xncp.py, so SendUnicastReq/SendUnicastRsp could get the same treatment cheaply.
• The size-guard comment cites EZSP_MAX_FRAME_LENGTH, a name that doesn't exist in bellows (it's an SDK-side constant). When the guard is adjusted per the review above, the comment could name the limit that actually binds (ASH data field / customFrame payload cap) instead.

[zigpy-review-bot]

Follow-up on the (now-resolved) MAX_COMBINED_SEND_DATA_LENGTH thread — the complete, verified picture, with corrections to my original comment in both directions.

TL;DR: the 128-byte cap cannot break any packet that is actually transmittable over the air, so my original comment overstated the impact. The binding limit is also not the ASH/EZSP transport (which carries 220-byte frames in current stacks, contrary to the stale UG100/UG101 figures I cited) — it is a hard 119-byte cap on the customFrame payload inside the closed-source XNCP plugin, exactly as puddly measured on hardware, and now also confirmed by disassembling the prebuilt SDK library. What remains is a failure-mode nit for payloads that could never be sent anyway (93–128 B): on the combined path they end in a 10-second TimeoutError while holding the request lock, instead of the old path's immediate MESSAGE_TOO_LONG → DeliveryError. Suggested tweak at the end.

Transport limits — what my original comment got wrong

The "ASH data field 3–128 bytes" (UG101) and "customFrame payload max 119 bytes" (UG100) figures describe the legacy frame format and are stale as transport limits: in current stacks the ASH data field is bounded by the EZSP frame size, not by 128.

• EZSP_MAX_FRAME_LENGTH = (218 + 1 + 1) = 220 — protocol/zigbee/app/util/ezsp/ezsp-protocol.h; bounds the whole EZSP frame including the 5-byte v8+ header.
• ASH_MAX_DATA_FIELD_LEN = EZSP_MAX_FRAME_LENGTH — platform/service/legacy_common_ash/inc/ash-protocol.h, shared by the host and NCP ASH implementations.
• zigbee-herdsman's ember driver (a direct port of the Silabs host library) mirrors both constants.

So the worst-case combined-send frame (128 B of data plus every optional field ≈ 184 B on the wire) is comfortably transportable: there is no ASH-level rejection, and EZSP_MAX_FRAME_LENGTH is 220, not the 200 I quoted earlier.

The real limit: a hard 119-byte customFrame payload cap in the closed-source XNCP plugin (disassembly)

The cap measured on hardware is enforced in libzigbee-xncp.a (prebuilt, no public source). Disassembly of sli_zigbee_xncp_handle_incoming_custom_ezsp_message — byte-identical logic in EmberZNet 9.0.2 (SiSDK 2025.12.3) and 9.1.0 (SiSDK 2026.6.0, the current firmware-builder target):

bl   fetchInt8u                                    ; read customFrame payload length
cmp  r0, #119                                      ; 0x77
bhi.n <function epilogue>                          ; > 119: return immediately
...
bl   sl_zigbee_af_xncp_incoming_custom_frame_cb    ; only reached when length <= 119
bl   appendInt32u                                  ; status + reply appended only on this path

Semantics: an over-119 payload is dropped before the XNCP dispatcher runs — the extension callback is never invoked and no status/reply bytes are appended to the response. That matches the observed "parsing is halted early", rules out any partial parse or truncated send on the firmware side, and corresponds to the otherwise-unreferenced EMBER_MAX_CUSTOM_EZSP_MESSAGE_PAYLOAD 119 in protocol/zigbee/app/xncp/xncp.h (the legacy <parameters:0-120> window minus the length byte).

Byte budget: what can and cannot hit the cap

The serialized SendUnicastReq occupies the customFrame payload as: 3 B XNCP header (command id + status) + 15 B fixed fields (flags 1, destination 2, APS frame 11, tag 1) + 9 B extended-timeout fields (ieee + Bool, included whenever the device is known — i.e. virtually always) + 1 + 2·R for a manual source route with R relays + len(data). Against the 119-byte cap:

variant	budget for data
extended timeout, no route	92 B
no extended timeout, no route	101 B
extended timeout + R-relay route	91 − 2·R B

The unfragmented over-the-air APS ceiling is ~82 B, and it shrinks by ~2 + 2·R once a source-route subframe is in the NWK header — always below the corresponding row above. So no packet that could actually be transmitted can exceed the 119-byte cap, and the 128 guard cannot break real traffic.

The residue: payloads of 93–128 B — producible by callers but unsendable regardless (the stack rejects them) — pass len(data) <= MAX_COMBINED_SEND_DATA_LENGTH and hit the NCP's silent drop. Because nothing is appended to the reply, bellows receives a customFrame response it cannot parse, the pending future never resolves, and the command waits out EZSP_CMD_TIMEOUT (10 s, bellows/ezsp/protocol.py) while _req_lock is held, then surfaces as a TimeoutError. The same packet on the fallback path fails instantly and cleanly with MESSAGE_TOO_LONG → DeliveryError.

Suggestion (robustness nit, not a blocker): size the check against what is actually sent — build the request first and require len(XncpCommand.from_payload(req).serialize()) <= 119 before taking the combined path (falling back otherwise), or simply lower MAX_COMBINED_SEND_DATA_LENGTH to 92. Either variant also makes the constant's comment accurate: EZSP_MAX_FRAME_LENGTH (220) isn't the binding limit — the XNCP plugin's 119-byte gate is.

Process note: these reviews now have local access to the full Simplicity SDK — including the NCP sources and the prebuilt stack libraries used for the disassembly above — for source-level comparison. Previously only silabs-firmware-builder reviews had that, which is why the first version of this comment leaned on stale public documentation.

[TheJulianJES]

Nice! Looks good