From e3cfab1cef961a97f380285526f461c000fed0e4 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:12:29 -0400 Subject: [PATCH 1/4] Implement the combined XNCP unicast API --- bellows/ezsp/__init__.py | 31 ++++++++ bellows/ezsp/xncp.py | 51 +++++++++++- bellows/zigbee/application.py | 91 ++++++++++++++------- tests/test_application.py | 146 +++++++++++++++++++++++++++++++++- tests/test_ezsp.py | 70 ++++++++++++++++ 5 files changed, 359 insertions(+), 30 deletions(-) diff --git a/bellows/ezsp/__init__.py b/bellows/ezsp/__init__.py index 1ef25f56..6491cf1f 100644 --- a/bellows/ezsp/__init__.py +++ b/bellows/ezsp/__init__.py @@ -785,6 +785,37 @@ async def xncp_set_manual_source_route( ) ) + async def xncp_send_unicast( + self, + destination: t.NWK, + aps_frame: t.EmberApsFrame, + message_tag: t.uint8_t, + data: bytes, + *, + source_route: list[t.NWK] | None = None, + extended_timeout: tuple[t.EUI64, bool] | None = None, + ) -> tuple[t.sl_Status, t.uint8_t]: + """Send a combined unicast command.""" + flags = xncp.SendUnicastFlags.NONE + req = xncp.SendUnicastReq( + flags=flags, + destination=destination, + aps_frame=aps_frame, + message_tag=message_tag, + data=xncp.Bytes(data), + ) + + if extended_timeout is not None: + req.flags |= xncp.SendUnicastFlags.EXTENDED_TIMEOUT + req.ieee, req.extended_timeout = extended_timeout + + if source_route is not None: + req.flags |= xncp.SendUnicastFlags.SOURCE_ROUTE + req.source_route = source_route + + rsp = await self.send_xncp_frame(req) + return rsp.status, rsp.sequence + async def xncp_get_mfg_token_override(self, token: t.EzspMfgTokenId) -> bytes: """Get manufacturing token override.""" rsp = await self.send_xncp_frame(xncp.GetMfgTokenOverrideReq(token=token)) diff --git a/bellows/ezsp/xncp.py b/bellows/ezsp/xncp.py index 2cfc50d4..2512b59b 100644 --- a/bellows/ezsp/xncp.py +++ b/bellows/ezsp/xncp.py @@ -7,7 +7,13 @@ import zigpy.types as t -from bellows.types import EmberStatus, EzspMfgTokenId, RouteRecordStatus +from bellows.types import ( + EmberApsFrame, + EmberStatus, + EzspMfgTokenId, + RouteRecordStatus, + sl_Status, +) _LOGGER = logging.getLogger(__name__) @@ -43,6 +49,7 @@ class XncpCommandId(t.enum16): SET_ROUTE_TABLE_ENTRY_REQ = 0x0006 GET_ROUTE_TABLE_ENTRY_REQ = 0x0007 GET_TX_POWER_INFO_REQ = 0x0008 + SEND_UNICAST_REQ = 0x0009 GET_SUPPORTED_FEATURES_RSP = GET_SUPPORTED_FEATURES_REQ | 0x8000 SET_SOURCE_ROUTE_RSP = SET_SOURCE_ROUTE_REQ | 0x8000 @@ -53,6 +60,7 @@ class XncpCommandId(t.enum16): SET_ROUTE_TABLE_ENTRY_RSP = SET_ROUTE_TABLE_ENTRY_REQ | 0x8000 GET_ROUTE_TABLE_ENTRY_RSP = GET_ROUTE_TABLE_ENTRY_REQ | 0x8000 GET_TX_POWER_INFO_RSP = GET_TX_POWER_INFO_REQ | 0x8000 + SEND_UNICAST_RSP = SEND_UNICAST_REQ | 0x8000 UNKNOWN = 0xFFFF @@ -123,6 +131,10 @@ class FirmwareFeatures(t.bitmap32): # Recommended and maximum TX power can be queried by country code TX_POWER_INFO = 1 << 7 + # A unicast can be sent, its source route and extended timeout applied, in a + # single command + COMBINED_SEND = 1 << 8 + class XncpCommandPayload(t.Struct): pass @@ -233,6 +245,43 @@ class GetTxPowerInfoRsp(XncpCommandPayload): max_power_dbm: t.int8s +class SendUnicastFlags(t.bitmap8): + NONE = 0 + + # The extended timeout for the destination should be set before sending + EXTENDED_TIMEOUT = 1 << 0 + + # A manual source route should be installed before sending + SOURCE_ROUTE = 1 << 1 + + +@register_command(XncpCommandId.SEND_UNICAST_REQ) +class SendUnicastReq(XncpCommandPayload): + flags: SendUnicastFlags + destination: t.NWK + aps_frame: EmberApsFrame + message_tag: t.uint8_t + + ieee: t.EUI64 = t.StructField( + requires=lambda s: SendUnicastFlags.EXTENDED_TIMEOUT in s.flags + ) + extended_timeout: t.Bool = t.StructField( + requires=lambda s: SendUnicastFlags.EXTENDED_TIMEOUT in s.flags + ) + + source_route: t.LVList[t.NWK, t.uint8_t] = t.StructField( + requires=lambda s: SendUnicastFlags.SOURCE_ROUTE in s.flags + ) + + data: Bytes + + +@register_command(XncpCommandId.SEND_UNICAST_RSP) +class SendUnicastRsp(XncpCommandPayload): + status: sl_Status + sequence: t.uint8_t + + @register_command(XncpCommandId.UNKNOWN) class Unknown(XncpCommandPayload): pass diff --git a/bellows/zigbee/application.py b/bellows/zigbee/application.py index bee1ca54..93629e02 100644 --- a/bellows/zigbee/application.py +++ b/bellows/zigbee/application.py @@ -49,6 +49,11 @@ MESSAGE_SEND_TIMEOUT_MAINS = 3 MESSAGE_SEND_TIMEOUT_BATTERY = 8 +# Largest unicast payload that fits in a single combined XNCP send command. The +# `customFrame` payload is bounded (EZSP_MAX_FRAME_LENGTH), so oversized packets +# fall back to the multi-command send path. +MAX_COMBINED_SEND_DATA_LENGTH = 128 + COUNTER_EZSP_BUFFERS = "EZSP_FREE_BUFFERS" COUNTER_NWK_CONFLICTS = "nwk_conflicts" COUNTER_RESET_REQ = "reset_requests" @@ -1029,37 +1034,69 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: try: async with self._req_lock: if packet.dst.addr_mode == zigpy.types.AddrMode.NWK: - if device is not None: - await self._ezsp.set_extended_timeout( - nwk=device.nwk, - ieee=device.ieee, - extended_timeout=extended_timeout, - ) - - if packet.source_route is not None: - if ( + # Manual source routes are installed through XNCP; native + # source routes cannot be folded into the combined command + use_manual_source_route = ( + packet.source_route is not None + and ( FirmwareFeatures.MANUAL_SOURCE_ROUTE in self._ezsp._xncp_features - and self.config[CONF_BELLOWS_CONFIG][ - CONF_MANUAL_SOURCE_ROUTING - ] - ): - await self._ezsp.xncp_set_manual_source_route( - destination=packet.dst.address, - route=packet.source_route, - ) - else: - await self._ezsp.set_source_route( - nwk=packet.dst.address, - relays=packet.source_route, + ) + and self.config[CONF_BELLOWS_CONFIG][ + CONF_MANUAL_SOURCE_ROUTING + ] + ) + + if ( + FirmwareFeatures.COMBINED_SEND in self._ezsp._xncp_features + and (packet.source_route is None or use_manual_source_route) + and ( + len(packet.data.serialize()) + <= MAX_COMBINED_SEND_DATA_LENGTH + ) + ): + status, _ = await self._ezsp.xncp_send_unicast( + destination=packet.dst.address, + aps_frame=aps_frame, + message_tag=message_tag, + data=packet.data.serialize(), + source_route=( + packet.source_route + if use_manual_source_route + else None + ), + extended_timeout=( + (device.ieee, extended_timeout) + if device is not None + else None + ), + ) + else: + if device is not None: + await self._ezsp.set_extended_timeout( + nwk=device.nwk, + ieee=device.ieee, + extended_timeout=extended_timeout, ) - status, _ = await self._ezsp.send_unicast( - nwk=packet.dst.address, - aps_frame=aps_frame, - message_tag=message_tag, - data=packet.data.serialize(), - ) + if packet.source_route is not None: + if use_manual_source_route: + await self._ezsp.xncp_set_manual_source_route( + destination=packet.dst.address, + route=packet.source_route, + ) + else: + await self._ezsp.set_source_route( + nwk=packet.dst.address, + relays=packet.source_route, + ) + + status, _ = await self._ezsp.send_unicast( + nwk=packet.dst.address, + aps_frame=aps_frame, + message_tag=message_tag, + data=packet.data.serialize(), + ) elif packet.dst.addr_mode == zigpy.types.AddrMode.Group: status, _ = await self._ezsp.send_multicast( aps_frame=aps_frame, diff --git a/tests/test_application.py b/tests/test_application.py index c5355d2e..295ba96e 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -29,7 +29,11 @@ import bellows.types.struct import bellows.uart as uart import bellows.zigbee.application -from bellows.zigbee.application import DEFAULT_TX_POWER, ControllerApplication +from bellows.zigbee.application import ( + DEFAULT_TX_POWER, + MAX_COMBINED_SEND_DATA_LENGTH, + ControllerApplication, +) import bellows.zigbee.device from bellows.zigbee.device import EZSPEndpoint, EZSPGroupEndpoint from bellows.zigbee.util import map_rssi_to_energy @@ -827,7 +831,7 @@ def send_unicast(*args, **kwargs): sequence=packet.tsn, ), message_tag=sentinel.msg_tag, - data=b"some data", + data=packet.data.serialize(), ) ] @@ -918,6 +922,144 @@ async def test_send_packet_unicast_manual_source_route(make_app, packet): ) +async def _test_send_packet_unicast_combined( + app, + packet, + *, + status=bellows.types.sl_Status.OK, + source_route=None, + extended_timeout=None, + options=( + t.EmberApsOption.APS_OPTION_ENABLE_ROUTE_DISCOVERY + | t.EmberApsOption.APS_OPTION_RETRY + ), +): + def xncp_send_unicast(*args, **kwargs): + asyncio.get_running_loop().call_later( + 0.01, + app.ezsp_callback_handler, + "messageSentHandler", + list( + dict( + type=t.EmberOutgoingMessageType.OUTGOING_DIRECT, + indexOrDestination=0x1234, + apsFrame=sentinel.aps, + messageTag=sentinel.msg_tag, + status=bellows.types.sl_Status.OK, + message=b"", + ).values() + ), + ) + + return [status, 0x12] + + app._ezsp.xncp_send_unicast = AsyncMock( + side_effect=xncp_send_unicast, spec=app._ezsp.xncp_send_unicast + ) + app.get_sequence = MagicMock(return_value=sentinel.msg_tag) + + await app.send_packet(packet) + + assert app._ezsp.xncp_send_unicast.mock_calls == [ + call( + destination=t.EmberNodeId(0x1234), + aps_frame=t.EmberApsFrame( + profileId=packet.profile_id, + clusterId=packet.cluster_id, + sourceEndpoint=packet.src_ep, + destinationEndpoint=packet.dst_ep, + options=options, + groupId=0x0000, + sequence=packet.tsn, + ), + message_tag=sentinel.msg_tag, + data=packet.data.serialize(), + source_route=source_route, + extended_timeout=extended_timeout, + ) + ] + + assert len(app._pending_requests) == 0 + + +async def test_send_packet_unicast_combined(app, packet): + app._ezsp._xncp_features |= FirmwareFeatures.COMBINED_SEND + await _test_send_packet_unicast_combined(app, packet) + + +async def test_send_packet_unicast_combined_extended_timeout(app, ieee, packet): + app._ezsp._xncp_features |= FirmwareFeatures.COMBINED_SEND + app.add_device(nwk=packet.dst.address, ieee=ieee) + + # The APS ACK in the packet's tx_options disables the extended timeout + await _test_send_packet_unicast_combined( + app, packet, extended_timeout=(ieee, False) + ) + + +async def test_send_packet_unicast_combined_manual_source_route(make_app, packet): + app = make_app( + { + zigpy.config.CONF_SOURCE_ROUTING: True, + config.CONF_BELLOWS_CONFIG: {config.CONF_MANUAL_SOURCE_ROUTING: True}, + } + ) + app._ezsp._xncp_features |= ( + FirmwareFeatures.COMBINED_SEND | FirmwareFeatures.MANUAL_SOURCE_ROUTE + ) + + packet.source_route = [0x0001, 0x0002] + await _test_send_packet_unicast_combined( + app, + packet, + source_route=[0x0001, 0x0002], + options=( + t.EmberApsOption.APS_OPTION_RETRY + | t.EmberApsOption.APS_OPTION_ENABLE_ADDRESS_DISCOVERY + ), + ) + + +async def test_send_packet_unicast_combined_native_source_route_fallback( + make_app, packet +): + # A native (non-manual) source route cannot be folded into the combined command + app = make_app({zigpy.config.CONF_SOURCE_ROUTING: True}) + app._ezsp._xncp_features |= FirmwareFeatures.COMBINED_SEND + app._ezsp._protocol.set_source_route = AsyncMock( + return_value=t.sl_Status.OK, spec=app._ezsp._protocol.set_source_route + ) + app._ezsp.xncp_send_unicast = AsyncMock(spec=app._ezsp.xncp_send_unicast) + + packet.source_route = [0x0001, 0x0002] + await _test_send_packet_unicast( + app, + packet, + options=( + t.EmberApsOption.APS_OPTION_RETRY + | t.EmberApsOption.APS_OPTION_ENABLE_ADDRESS_DISCOVERY + ), + ) + + assert len(app._ezsp.xncp_send_unicast.mock_calls) == 0 + assert app._ezsp._protocol.set_source_route.mock_calls == [ + call(nwk=packet.dst.address, relays=[0x0001, 0x0002]) + ] + + +async def test_send_packet_unicast_combined_oversized_fallback(app, packet): + # Payloads that do not fit in a custom frame fall back to the multi-command path + app._ezsp._xncp_features |= FirmwareFeatures.COMBINED_SEND + app._ezsp.xncp_send_unicast = AsyncMock(spec=app._ezsp.xncp_send_unicast) + + packet = packet.replace( + data=zigpy_t.SerializableBytes(b"a" * (MAX_COMBINED_SEND_DATA_LENGTH + 1)) + ) + await _test_send_packet_unicast(app, packet) + + assert len(app._ezsp.xncp_send_unicast.mock_calls) == 0 + + async def test_send_packet_unicast_extended_timeout_with_acks(app, ieee, packet): app.add_device(nwk=packet.dst.address, ieee=ieee) diff --git a/tests/test_ezsp.py b/tests/test_ezsp.py index d548309a..699427d8 100644 --- a/tests/test_ezsp.py +++ b/tests/test_ezsp.py @@ -1014,6 +1014,76 @@ def test_frame_parsing_error_doesnt_disconnect(ezsp_f, caplog): assert "Failed to parse frame" in caplog.text +@pytest.mark.parametrize( + "source_route, extended_timeout, expected_flags, expected_extra", + [ + (None, None, xncp.SendUnicastFlags.NONE, {}), + ( + [t.NWK(0x0001), t.NWK(0x0002)], + None, + xncp.SendUnicastFlags.SOURCE_ROUTE, + {"source_route": [t.NWK(0x0001), t.NWK(0x0002)]}, + ), + ( + None, + (t.EUI64.convert("00:11:22:33:44:55:66:77"), True), + xncp.SendUnicastFlags.EXTENDED_TIMEOUT, + { + "ieee": t.EUI64.convert("00:11:22:33:44:55:66:77"), + "extended_timeout": t.Bool.true, + }, + ), + ], +) +async def test_xncp_send_unicast( + ezsp_f, source_route, extended_timeout, expected_flags, expected_extra +): + """Test sending a unicast via the combined XNCP command.""" + ezsp_f._xncp_features = xncp.FirmwareFeatures.COMBINED_SEND + + aps_frame = t.EmberApsFrame( + profileId=0x0104, + clusterId=0x0006, + sourceEndpoint=1, + destinationEndpoint=1, + options=t.EmberApsOption.APS_OPTION_RETRY, + groupId=0x0000, + sequence=0x42, + ) + + with patch.object( + ezsp_f, + "send_xncp_frame", + new=AsyncMock( + return_value=xncp.SendUnicastRsp(status=t.sl_Status.OK, sequence=0x12) + ), + ) as mock_send: + status, sequence = await ezsp_f.xncp_send_unicast( + destination=t.NWK(0x1234), + aps_frame=aps_frame, + message_tag=0x07, + data=b"some data", + source_route=source_route, + extended_timeout=extended_timeout, + ) + + assert status == t.sl_Status.OK + assert sequence == 0x12 + + assert mock_send.mock_calls == [ + call( + xncp.SendUnicastReq( + flags=expected_flags, + destination=t.NWK(0x1234), + aps_frame=aps_frame, + message_tag=0x07, + data=xncp.Bytes(b"some data"), + **expected_extra, + ) + ) + ] + + async def test_xncp_get_chip_info(ezsp_f): """Test getting chip info via XNCP.""" ezsp_f._xncp_features = xncp.FirmwareFeatures.CHIP_INFO From 0227c1228e4f840ccdccbecb3bcbb1e109267b95 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:37:57 -0400 Subject: [PATCH 2/4] Serialize early --- bellows/zigbee/application.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/bellows/zigbee/application.py b/bellows/zigbee/application.py index 93629e02..16deaa71 100644 --- a/bellows/zigbee/application.py +++ b/bellows/zigbee/application.py @@ -1033,6 +1033,8 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: try: async with self._req_lock: + data = packet.data.serialize() + if packet.dst.addr_mode == zigpy.types.AddrMode.NWK: # Manual source routes are installed through XNCP; native # source routes cannot be folded into the combined command @@ -1050,16 +1052,13 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: if ( FirmwareFeatures.COMBINED_SEND in self._ezsp._xncp_features and (packet.source_route is None or use_manual_source_route) - and ( - len(packet.data.serialize()) - <= MAX_COMBINED_SEND_DATA_LENGTH - ) + and (len(data) <= MAX_COMBINED_SEND_DATA_LENGTH) ): status, _ = await self._ezsp.xncp_send_unicast( destination=packet.dst.address, aps_frame=aps_frame, message_tag=message_tag, - data=packet.data.serialize(), + data=data, source_route=( packet.source_route if use_manual_source_route @@ -1095,7 +1094,7 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: nwk=packet.dst.address, aps_frame=aps_frame, message_tag=message_tag, - data=packet.data.serialize(), + data=data, ) elif packet.dst.addr_mode == zigpy.types.AddrMode.Group: status, _ = await self._ezsp.send_multicast( @@ -1103,7 +1102,7 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: radius=packet.radius, non_member_radius=packet.non_member_radius, message_tag=message_tag, - data=packet.data.serialize(), + data=data, ) elif packet.dst.addr_mode == zigpy.types.AddrMode.Broadcast: status, _ = await self._ezsp.send_broadcast( @@ -1112,7 +1111,7 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: radius=packet.radius, message_tag=message_tag, aps_sequence=packet.tsn, - data=packet.data.serialize(), + data=data, ) if status != t.sl_Status.OK: From 523af2ba6209c0b90977610303d18a687f9a03ea Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:27:06 -0400 Subject: [PATCH 3/4] Compute the XNCP unicast payload size in advance to fallback gracefully --- bellows/exception.py | 4 ++ bellows/ezsp/__init__.py | 25 ++++++++--- bellows/ezsp/xncp.py | 3 ++ bellows/zigbee/application.py | 49 +++++++++++---------- tests/test_application.py | 83 +++++++++++++++++++++-------------- tests/test_ezsp.py | 14 +++--- 6 files changed, 111 insertions(+), 67 deletions(-) diff --git a/bellows/exception.py b/bellows/exception.py index 21f2e6d8..2def7a3c 100644 --- a/bellows/exception.py +++ b/bellows/exception.py @@ -9,6 +9,10 @@ class InvalidCommandError(EzspError): pass +class PayloadTooLongError(InvalidCommandError): + pass + + class InvalidCommandPayload(InvalidCommandError): def __init__(self, msg: str, raw_bytes: bytes) -> None: super().__init__(msg) diff --git a/bellows/ezsp/__init__.py b/bellows/ezsp/__init__.py index 6491cf1f..3aaf0c13 100644 --- a/bellows/ezsp/__init__.py +++ b/bellows/ezsp/__init__.py @@ -17,10 +17,16 @@ from bellows.ash import NcpFailure import bellows.config as conf -from bellows.exception import EzspError, InvalidCommandError, InvalidCommandPayload +from bellows.exception import ( + EzspError, + InvalidCommandError, + InvalidCommandPayload, + PayloadTooLongError, +) from bellows.ezsp import xncp from bellows.ezsp.config import DEFAULT_CONFIG, RuntimeConfig, ValueConfig from bellows.ezsp.xncp import ( + MAX_XNCP_PAYLOAD_LENGTH, FirmwareFeatures, FlowControlType, GetRouteTableEntryRsp, @@ -785,7 +791,7 @@ async def xncp_set_manual_source_route( ) ) - async def xncp_send_unicast( + def xncp_prepare_unicast( self, destination: t.NWK, aps_frame: t.EmberApsFrame, @@ -794,8 +800,8 @@ async def xncp_send_unicast( *, source_route: list[t.NWK] | None = None, extended_timeout: tuple[t.EUI64, bool] | None = None, - ) -> tuple[t.sl_Status, t.uint8_t]: - """Send a combined unicast command.""" + ) -> xncp.SendUnicastReq: + """Prepare a combined unicast command.""" flags = xncp.SendUnicastFlags.NONE req = xncp.SendUnicastReq( flags=flags, @@ -813,7 +819,16 @@ async def xncp_send_unicast( req.flags |= xncp.SendUnicastFlags.SOURCE_ROUTE req.source_route = source_route - rsp = await self.send_xncp_frame(req) + if len(req.serialize()) > MAX_XNCP_PAYLOAD_LENGTH: + raise PayloadTooLongError() + + return req + + async def xncp_send_unicast( + self, request: xncp.SendUnicastReq + ) -> tuple[t.sl_Status, t.uint8_t]: + """Send a combined unicast command.""" + rsp = await self.send_xncp_frame(request) return rsp.status, rsp.sequence async def xncp_get_mfg_token_override(self, token: t.EzspMfgTokenId) -> bytes: diff --git a/bellows/ezsp/xncp.py b/bellows/ezsp/xncp.py index 2512b59b..aa1eb2b4 100644 --- a/bellows/ezsp/xncp.py +++ b/bellows/ezsp/xncp.py @@ -17,6 +17,9 @@ _LOGGER = logging.getLogger(__name__) +MAX_XNCP_FRAME_LENGTH = 119 +MAX_XNCP_PAYLOAD_LENGTH = MAX_XNCP_FRAME_LENGTH - 3 # u16 + u8 + COMMANDS: dict[XncpCommandId, type[XncpCommandPayload]] = {} REV_COMMANDS: dict[type[XncpCommandPayload], XncpCommandId] = {} diff --git a/bellows/zigbee/application.py b/bellows/zigbee/application.py index 16deaa71..d1bba5c0 100644 --- a/bellows/zigbee/application.py +++ b/bellows/zigbee/application.py @@ -36,6 +36,7 @@ ControllerError, EzspError, InvalidCommandError, + PayloadTooLongError, StackAlreadyRunning, ) import bellows.ezsp @@ -49,11 +50,6 @@ MESSAGE_SEND_TIMEOUT_MAINS = 3 MESSAGE_SEND_TIMEOUT_BATTERY = 8 -# Largest unicast payload that fits in a single combined XNCP send command. The -# `customFrame` payload is bounded (EZSP_MAX_FRAME_LENGTH), so oversized packets -# fall back to the multi-command send path. -MAX_COMBINED_SEND_DATA_LENGTH = 128 - COUNTER_EZSP_BUFFERS = "EZSP_FREE_BUFFERS" COUNTER_NWK_CONFLICTS = "nwk_conflicts" COUNTER_RESET_REQ = "reset_requests" @@ -1049,27 +1045,36 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None: ] ) + # XNCP combined unicasts can fail in a few ways so it's simpler + # to prepare the request in advance + xncp_unicast = None + if ( FirmwareFeatures.COMBINED_SEND in self._ezsp._xncp_features and (packet.source_route is None or use_manual_source_route) - and (len(data) <= MAX_COMBINED_SEND_DATA_LENGTH) ): - status, _ = await self._ezsp.xncp_send_unicast( - destination=packet.dst.address, - aps_frame=aps_frame, - message_tag=message_tag, - data=data, - source_route=( - packet.source_route - if use_manual_source_route - else None - ), - extended_timeout=( - (device.ieee, extended_timeout) - if device is not None - else None - ), - ) + try: + xncp_unicast = self._ezsp.xncp_prepare_unicast( + destination=packet.dst.address, + aps_frame=aps_frame, + message_tag=message_tag, + data=data, + source_route=( + packet.source_route + if use_manual_source_route + else None + ), + extended_timeout=( + (device.ieee, extended_timeout) + if device is not None + else None + ), + ) + except PayloadTooLongError: + xncp_unicast = None + + if xncp_unicast is not None: + status, _ = await self._ezsp.xncp_send_unicast(xncp_unicast) else: if device is not None: await self._ezsp.set_extended_timeout( diff --git a/tests/test_application.py b/tests/test_application.py index 295ba96e..e6d47308 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -18,22 +18,21 @@ import bellows.ezsp as ezsp from bellows.ezsp.v9.commands import GetTokenDataRsp from bellows.ezsp.xncp import ( + MAX_XNCP_PAYLOAD_LENGTH, FirmwareFeatures, FlowControlType, GetChipInfoRsp, GetRouteTableEntryRsp, GetTxPowerInfoRsp, + SendUnicastFlags, + SendUnicastReq, ) import bellows.types import bellows.types as t import bellows.types.struct import bellows.uart as uart import bellows.zigbee.application -from bellows.zigbee.application import ( - DEFAULT_TX_POWER, - MAX_COMBINED_SEND_DATA_LENGTH, - ControllerApplication, -) +from bellows.zigbee.application import DEFAULT_TX_POWER, ControllerApplication import bellows.zigbee.device from bellows.zigbee.device import EZSPEndpoint, EZSPGroupEndpoint from bellows.zigbee.util import map_rssi_to_energy @@ -49,6 +48,8 @@ zigpy.config.CONF_STARTUP_ENERGY_SCAN: False, } +MSG_TAG = t.uint8_t(0x42) + @pytest.fixture def ieee(init=0): @@ -802,7 +803,7 @@ def send_unicast(*args, **kwargs): type=t.EmberOutgoingMessageType.OUTGOING_DIRECT, indexOrDestination=0x1234, apsFrame=sentinel.aps, - messageTag=sentinel.msg_tag, + messageTag=MSG_TAG, status=sent_handler_status, message=b"", ).values() @@ -814,7 +815,7 @@ def send_unicast(*args, **kwargs): app._ezsp.send_unicast = AsyncMock( side_effect=send_unicast, spec=app._ezsp.send_unicast ) - app.get_sequence = MagicMock(return_value=sentinel.msg_tag) + app.get_sequence = MagicMock(return_value=MSG_TAG) await app.send_packet(packet) @@ -830,7 +831,7 @@ def send_unicast(*args, **kwargs): groupId=0x0000, sequence=packet.tsn, ), - message_tag=sentinel.msg_tag, + message_tag=MSG_TAG, data=packet.data.serialize(), ) ] @@ -944,7 +945,7 @@ def xncp_send_unicast(*args, **kwargs): type=t.EmberOutgoingMessageType.OUTGOING_DIRECT, indexOrDestination=0x1234, apsFrame=sentinel.aps, - messageTag=sentinel.msg_tag, + messageTag=MSG_TAG, status=bellows.types.sl_Status.OK, message=b"", ).values() @@ -956,26 +957,40 @@ def xncp_send_unicast(*args, **kwargs): app._ezsp.xncp_send_unicast = AsyncMock( side_effect=xncp_send_unicast, spec=app._ezsp.xncp_send_unicast ) - app.get_sequence = MagicMock(return_value=sentinel.msg_tag) + app.get_sequence = MagicMock(return_value=MSG_TAG) await app.send_packet(packet) + flags = SendUnicastFlags.NONE + + if extended_timeout is not None: + flags |= SendUnicastFlags.EXTENDED_TIMEOUT + + if source_route is not None: + flags |= SendUnicastFlags.SOURCE_ROUTE + assert app._ezsp.xncp_send_unicast.mock_calls == [ call( - destination=t.EmberNodeId(0x1234), - aps_frame=t.EmberApsFrame( - profileId=packet.profile_id, - clusterId=packet.cluster_id, - sourceEndpoint=packet.src_ep, - destinationEndpoint=packet.dst_ep, - options=options, - groupId=0x0000, - sequence=packet.tsn, - ), - message_tag=sentinel.msg_tag, - data=packet.data.serialize(), - source_route=source_route, - extended_timeout=extended_timeout, + SendUnicastReq( + flags=flags, + destination=t.EmberNodeId(0x1234), + aps_frame=t.EmberApsFrame( + profileId=packet.profile_id, + clusterId=packet.cluster_id, + sourceEndpoint=packet.src_ep, + destinationEndpoint=packet.dst_ep, + options=options, + groupId=0x0000, + sequence=packet.tsn, + ), + message_tag=MSG_TAG, + ieee=extended_timeout[0] if extended_timeout is not None else None, + extended_timeout=( + extended_timeout[1] if extended_timeout is not None else None + ), + source_route=source_route, + data=packet.data.serialize(), + ) ) ] @@ -1053,7 +1068,7 @@ async def test_send_packet_unicast_combined_oversized_fallback(app, packet): app._ezsp.xncp_send_unicast = AsyncMock(spec=app._ezsp.xncp_send_unicast) packet = packet.replace( - data=zigpy_t.SerializableBytes(b"a" * (MAX_COMBINED_SEND_DATA_LENGTH + 1)) + data=zigpy_t.SerializableBytes(b"a" * (MAX_XNCP_PAYLOAD_LENGTH + 1)) ) await _test_send_packet_unicast(app, packet) @@ -1258,7 +1273,7 @@ async def test_send_packet_broadcast(app, packet): app._ezsp.send_broadcast = AsyncMock( return_value=(bellows.types.named.sl_Status.OK, 0x12) ) - app.get_sequence = MagicMock(return_value=sentinel.msg_tag) + app.get_sequence = MagicMock(return_value=MSG_TAG) asyncio.get_running_loop().call_soon( app.ezsp_callback_handler, @@ -1268,7 +1283,7 @@ async def test_send_packet_broadcast(app, packet): type=t.EmberOutgoingMessageType.OUTGOING_BROADCAST, indexOrDestination=0xFFFE, apsFrame=sentinel.aps, - messageTag=sentinel.msg_tag, + messageTag=MSG_TAG, status=t.EmberStatus.SUCCESS, message=b"", ).values() @@ -1289,7 +1304,7 @@ async def test_send_packet_broadcast(app, packet): sequence=packet.tsn, ), radius=packet.radius, - message_tag=sentinel.msg_tag, + message_tag=MSG_TAG, aps_sequence=packet.tsn, data=b"some data", ) @@ -1304,7 +1319,7 @@ async def test_send_packet_broadcast_ignored_delivery_failure(app, packet): ) packet.radius = 30 - app.get_sequence = MagicMock(return_value=sentinel.msg_tag) + app.get_sequence = MagicMock(return_value=MSG_TAG) asyncio.get_running_loop().call_soon( app.ezsp_callback_handler, @@ -1314,7 +1329,7 @@ async def test_send_packet_broadcast_ignored_delivery_failure(app, packet): type=t.EmberOutgoingMessageType.OUTGOING_BROADCAST, indexOrDestination=0xFFFE, apsFrame=sentinel.aps, - messageTag=sentinel.msg_tag, + messageTag=MSG_TAG, status=t.EmberStatus.DELIVERY_FAILED, message=b"", ).values() @@ -1337,7 +1352,7 @@ async def test_send_packet_broadcast_ignored_delivery_failure(app, packet): sequence=packet.tsn, ), radius=packet.radius, - message_tag=sentinel.msg_tag, + message_tag=MSG_TAG, aps_sequence=packet.tsn, data=b"some data", ) @@ -1357,7 +1372,7 @@ async def test_send_packet_multicast(app, packet): return_value=(bellows.types.sl_Status.OK, 0x12), spec=app._ezsp._protocol.send_multicast, ) - app.get_sequence = MagicMock(return_value=sentinel.msg_tag) + app.get_sequence = MagicMock(return_value=MSG_TAG) asyncio.get_running_loop().call_soon( app.ezsp_callback_handler, @@ -1367,7 +1382,7 @@ async def test_send_packet_multicast(app, packet): type=t.EmberOutgoingMessageType.OUTGOING_MULTICAST, indexOrDestination=0x1234, apsFrame=sentinel.aps, - messageTag=sentinel.msg_tag, + messageTag=MSG_TAG, status=t.EmberStatus.SUCCESS, message=b"", ).values() @@ -1388,7 +1403,7 @@ async def test_send_packet_multicast(app, packet): ), radius=packet.radius, non_member_radius=packet.non_member_radius, - message_tag=sentinel.msg_tag, + message_tag=MSG_TAG, data=b"some data", ) ] diff --git a/tests/test_ezsp.py b/tests/test_ezsp.py index 699427d8..e2e26e1d 100644 --- a/tests/test_ezsp.py +++ b/tests/test_ezsp.py @@ -1059,12 +1059,14 @@ async def test_xncp_send_unicast( ), ) as mock_send: status, sequence = await ezsp_f.xncp_send_unicast( - destination=t.NWK(0x1234), - aps_frame=aps_frame, - message_tag=0x07, - data=b"some data", - source_route=source_route, - extended_timeout=extended_timeout, + ezsp_f.xncp_prepare_unicast( + destination=t.NWK(0x1234), + aps_frame=aps_frame, + message_tag=0x07, + data=b"some data", + source_route=source_route, + extended_timeout=extended_timeout, + ) ) assert status == t.sl_Status.OK From 8fad7431e3eb61b128fe976c753e9717a7185ea1 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:31:18 -0400 Subject: [PATCH 4/4] Test boundary more explicitly --- tests/test_application.py | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/test_application.py b/tests/test_application.py index e6d47308..3e643b06 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -50,6 +50,26 @@ MSG_TAG = t.uint8_t(0x42) +# Every `SendUnicastReq` field but `data` is fixed-size, so the largest unicast payload +# that still fits in a single combined send command is a simple subtraction +MAX_COMBINED_SEND_DATA_LENGTH = MAX_XNCP_PAYLOAD_LENGTH - len( + SendUnicastReq( + flags=SendUnicastFlags.NONE, + destination=t.EmberNodeId(0x0000), + aps_frame=t.EmberApsFrame( + profileId=0x0000, + clusterId=0x0000, + sourceEndpoint=0x00, + destinationEndpoint=0x00, + options=t.EmberApsOption(0), + groupId=0x0000, + sequence=0x00, + ), + message_tag=MSG_TAG, + data=b"", + ).serialize() +) + @pytest.fixture def ieee(init=0): @@ -1062,13 +1082,23 @@ async def test_send_packet_unicast_combined_native_source_route_fallback( ] +async def test_send_packet_unicast_combined_max_size(app, packet): + # The largest payload that fits in a custom frame still uses the combined command + app._ezsp._xncp_features |= FirmwareFeatures.COMBINED_SEND + + packet = packet.replace( + data=zigpy_t.SerializableBytes(b"a" * MAX_COMBINED_SEND_DATA_LENGTH) + ) + await _test_send_packet_unicast_combined(app, packet) + + async def test_send_packet_unicast_combined_oversized_fallback(app, packet): - # Payloads that do not fit in a custom frame fall back to the multi-command path + # One byte more does not fit and falls back to the multi-command path app._ezsp._xncp_features |= FirmwareFeatures.COMBINED_SEND app._ezsp.xncp_send_unicast = AsyncMock(spec=app._ezsp.xncp_send_unicast) packet = packet.replace( - data=zigpy_t.SerializableBytes(b"a" * (MAX_XNCP_PAYLOAD_LENGTH + 1)) + data=zigpy_t.SerializableBytes(b"a" * (MAX_COMBINED_SEND_DATA_LENGTH + 1)) ) await _test_send_packet_unicast(app, packet)