From a8bc14577697a80174d8be634e24acfe3c51667b Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Thu, 20 Aug 2026 03:36:00 +0000 Subject: [PATCH 01/11] WIP --- bellows/ash.py | 15 ++++++++--- bellows/ezsp/__init__.py | 12 ++++++--- bellows/uart.py | 16 +++++++++-- tests/test_ash.py | 48 +++++++++++++++++++++++++++++++++ tests/test_ezsp.py | 24 +++++++++++++++++ tests/test_uart.py | 57 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 163 insertions(+), 9 deletions(-) diff --git a/bellows/ash.py b/bellows/ash.py index 48b492caa..2ba3e32b4 100644 --- a/bellows/ash.py +++ b/bellows/ash.py @@ -353,7 +353,7 @@ def parse_frame( class AshProtocol(asyncio.Protocol): - def __init__(self, ezsp_protocol) -> None: + def __init__(self, ezsp_protocol, *, suppress_acks: bool = False) -> None: self._ezsp_protocol = ezsp_protocol self._transport = None self._buffer = bytearray() @@ -363,6 +363,7 @@ def __init__(self, ezsp_protocol) -> None: self._tx_seq: int = 0 self._rx_seq: int = 0 self._t_rx_ack = T_RX_ACK_INIT + self._suppress_acks = suppress_acks self._ncp_reset_code: t.NcpResetCode | None = None self._ncp_state: NcpState = NcpState.CONNECTED @@ -555,12 +556,20 @@ def frame_received(self, frame: AshFrame) -> None: else: raise TypeError(f"Unknown frame received: {frame}") # pragma: no cover + def _send_ack(self) -> None: + # Something upstream may be ACKing on our behalf, in which case our own ACKs are + # pure overhead. NAKs are still sent: see `_reject_frame`. + if self._suppress_acks: + return + + self._write_frame(AckFrame(res=0, ncp_ready=0, ack_num=self._rx_seq)) + def data_frame_received(self, frame: DataFrame) -> None: # The Host may not piggyback acknowledgments and should promptly send an ACK # frame when it receives a DATA frame. if frame.frm_num == self._rx_seq: self._rx_seq = (frame.frm_num + 1) % 8 - self._write_frame(AckFrame(res=0, ncp_ready=0, ack_num=self._rx_seq)) + self._send_ack() # Clear reject condition on valid in-sequence DATA frame self._clear_reject_condition() @@ -569,7 +578,7 @@ def data_frame_received(self, frame: DataFrame) -> None: elif frame.re_tx: # Retransmitted frames must be immediately ACKed even if they are out of # sequence - self._write_frame(AckFrame(res=0, ncp_ready=0, ack_num=self._rx_seq)) + self._send_ack() else: # Out-of-sequence non-retransmitted frame: reject it _LOGGER.debug("Received out-of-sequence frame: %r", frame) diff --git a/bellows/ezsp/__init__.py b/bellows/ezsp/__init__.py index a21138292..c77153333 100644 --- a/bellows/ezsp/__init__.py +++ b/bellows/ezsp/__init__.py @@ -119,14 +119,18 @@ def maybe_remove(_): listeners.remove(future) @property - def is_tcp_serial_port(self) -> bool: + def resets_on_connect(self) -> bool: + """Whether the NCP may reset by itself as we connect, before we ask it to.""" parsed_path = urllib.parse.urlparse(self._config[conf.CONF_DEVICE_PATH]) - return parsed_path.scheme == "socket" + return parsed_path.scheme in ("socket", "esphome") async def _startup_reset(self) -> None: """Start EZSP and reset the stack.""" - # `zigbeed` resets on startup - if self.is_tcp_serial_port: + # `zigbeed` resets on startup, and an ESPHome proxy resets the NCP when it takes + # over the shared UART. Absorbing that RSTACK here matters: outside this window an + # unsolicited reset reaches `Gateway.reset_received` with no future waiting for it, + # which fails the connection and cancels whatever command is in flight. + if self.resets_on_connect: try: async with asyncio_timeout(NETWORK_COORDINATOR_STARTUP_RESET_WAIT): await self._gw.wait_for_startup_reset() diff --git a/bellows/uart.py b/bellows/uart.py index 9849e0a6a..4f685766c 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -1,6 +1,7 @@ import asyncio from asyncio import timeout as asyncio_timeout import logging +import urllib.parse import zigpy.config import zigpy.serial @@ -12,6 +13,15 @@ LOGGER = logging.getLogger(__name__) RESET_TIMEOUT = 2.5 +# An ESPHome port in this mode ACKs ASH frames on our behalf +ACKLESS_URL_MODE = "ezsp_ash" + + +def url_suppresses_acks(path: str) -> bool: + """Whether something upstream ACKs our ASH frames, making our own ACKs overhead.""" + query = urllib.parse.parse_qs(urllib.parse.urlparse(path).query) + return "mode" in query and ACKLESS_URL_MODE in query["mode"] + class Gateway(zigpy.serial.SerialProtocol): def __init__(self, api, connection_done_future=None): @@ -110,15 +120,17 @@ async def _connect(config, api): connection_done_future = loop.create_future() + path = config[zigpy.config.CONF_DEVICE_PATH] + gateway = Gateway(api, connection_done_future) - protocol = AshProtocol(gateway) + protocol = AshProtocol(gateway, suppress_acks=url_suppresses_acks(path)) flow_control = config[zigpy.config.CONF_DEVICE_FLOW_CONTROL] transport, _ = await zigpy.serial.create_serial_connection( loop, lambda: protocol, - url=config[zigpy.config.CONF_DEVICE_PATH], + url=path, baudrate=config[zigpy.config.CONF_DEVICE_BAUDRATE], xonxoff=flow_control in (None, "software"), rtscts=flow_control == "hardware", diff --git a/tests/test_ash.py b/tests/test_ash.py index 8b1169260..503606bb7 100644 --- a/tests/test_ash.py +++ b/tests/test_ash.py @@ -834,6 +834,54 @@ async def test_reject_condition_firmware_amplification_bug() -> None: assert len(transport.write.mock_calls) == 0 +@pytest.mark.parametrize("suppress_acks", [False, True]) +async def test_suppress_acks_still_naks(suppress_acks: bool) -> None: + """ACKs can be suppressed but NAKs must always be sent.""" + ezsp = MagicMock() + protocol = ash.AshProtocol(ezsp, suppress_acks=suppress_acks) + transport = MagicMock() + transport.is_closing.return_value = False + protocol.connection_made(transport) + protocol._write_frame = MagicMock(wraps=protocol._write_frame) + + # An in-sequence DATA frame is passed up but only ACKed if ACKs are not suppressed + good_frame = ash.DataFrame(frm_num=0, re_tx=0, ack_num=0, ezsp_frame=b"good") + protocol.data_received(good_frame.to_bytes() + bytes([ash.Reserved.FLAG])) + + assert ezsp.data_received.mock_calls == [call(b"good")] + + if suppress_acks: + assert protocol._write_frame.mock_calls == [] + else: + assert protocol._write_frame.mock_calls == [ + call(ash.AckFrame(res=0, ncp_ready=0, ack_num=1)) + ] + + protocol._write_frame.reset_mock() + + # A retransmitted out-of-sequence frame is ACKed under the same rules + retx_frame = ash.DataFrame(frm_num=4, re_tx=1, ack_num=0, ezsp_frame=b"retx") + protocol.data_received(retx_frame.to_bytes() + bytes([ash.Reserved.FLAG])) + + if suppress_acks: + assert protocol._write_frame.mock_calls == [] + else: + assert protocol._write_frame.mock_calls == [ + call(ash.AckFrame(res=0, ncp_ready=0, ack_num=1)) + ] + + protocol._write_frame.reset_mock() + + # But a NAK is sent for an out-of-sequence frame, even with ACKs suppressed + bad_frame = ash.DataFrame(frm_num=4, re_tx=0, ack_num=0, ezsp_frame=b"bad") + protocol.data_received(bad_frame.to_bytes() + bytes([ash.Reserved.FLAG])) + + assert protocol._in_reject_condition is True + assert protocol._write_frame.mock_calls == [ + call(ash.NakFrame(res=0, ncp_ready=0, ack_num=1)) + ] + + def test_ncp_failure_comparison() -> None: exc1 = ash.NcpFailure(code=t.NcpResetCode.ERROR_EXCEEDED_MAXIMUM_ACK_TIMEOUT_COUNT) exc2 = ash.NcpFailure(code=t.NcpResetCode.RESET_POWER_ON) diff --git a/tests/test_ezsp.py b/tests/test_ezsp.py index bccf17b39..8254e9373 100644 --- a/tests/test_ezsp.py +++ b/tests/test_ezsp.py @@ -768,6 +768,30 @@ async def test_write_nwk_update_id_failure(ezsp_f): ) +@pytest.mark.parametrize( + ("path", "resets"), + [ + ("socket://localhost:1234", True), + ( + "esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee&mode=ezsp_ash", + True, + ), + ("esphome://example.com:6053/", True), + ("/dev/serial", False), + ("/dev/null", False), + ], +) +def test_resets_on_connect(path: str, resets: bool) -> None: + ezsp = make_ezsp( + config={ + **DEVICE_CONFIG, + zigpy.config.CONF_DEVICE_PATH: path, + } + ) + + assert ezsp.resets_on_connect is resets + + @patch.object(EZSP, "version", new_callable=AsyncMock) @patch.object(EZSP, "reset", new_callable=AsyncMock) @patch.object(EZSP, "get_xncp_features", new_callable=AsyncMock) diff --git a/tests/test_uart.py b/tests/test_uart.py index 1bfda7c20..2ae283f1b 100644 --- a/tests/test_uart.py +++ b/tests/test_uart.py @@ -150,6 +150,63 @@ async def mock_connect(loop, protocol_factory, *args, **kwargs): assert len(threads) == 0 +@pytest.mark.parametrize( + ("path", "suppressed"), + [ + ( + "esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee&mode=ezsp_ash", + True, + ), + ("esphome://example.com:6053/?mode=ezsp_ash", True), + ("esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee", False), + ("esphome://example.com:6053/", False), + ("socket://example.com:1234", False), + ("/dev/serial", False), + # The literal text in the wrong position does not count + ("esphome://example.com:6053/?port_name=mode=ezsp_ash", False), + ("esphome://example.com:6053/?port_name=mode%3Dezsp_ash", False), + ("esphome://example.com:6053/?noise_psk=mode=ezsp_ash", False), + ("/dev/serial/by-id/mode=ezsp_ash", False), + ], +) +def test_url_suppresses_acks(path: str, suppressed: bool) -> None: + assert uart.url_suppresses_acks(path) is suppressed + + +@pytest.mark.parametrize( + ("path", "suppressed"), + [ + ("esphome://example.com:6053/?port_name=Zigbee&mode=ezsp_ash", True), + ("esphome://example.com:6053/?port_name=Zigbee", False), + ("/dev/serial", False), + ], +) +async def test_connect_suppress_acks(path, suppressed, monkeypatch): + appmock = MagicMock() + transport = MagicMock() + + async def mockconnect(loop, protocol_factory, **kwargs): + protocol = protocol_factory() + loop.call_soon(protocol.connection_made, transport) + return None, protocol + + monkeypatch.setattr(zigpy.serial, "create_serial_connection", mockconnect) + + with patch("bellows.uart.AshProtocol", wraps=uart.AshProtocol) as ash_mock: + gw = await uart.connect( + conf.SCHEMA_DEVICE( + {conf.CONF_DEVICE_PATH: path, conf.CONF_DEVICE_BAUDRATE: 115200} + ), + appmock, + use_thread=False, + ) + + assert len(ash_mock.mock_calls) == 1 + assert ash_mock.mock_calls[0].kwargs == {"suppress_acks": suppressed} + + gw.close() + + @pytest.fixture async def gw(): gw = uart.Gateway(MagicMock()) From 0063a20f1ea549bcea7656e116b89e86598159d8 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:56:05 +0000 Subject: [PATCH 02/11] Test: suppress ACKs based on URI and the presence of `tap_mode` --- bellows/ash.py | 11 +++++++++++ bellows/ezsp/__init__.py | 6 ++++-- bellows/uart.py | 41 ++++++++++++++++++++++++++++++++++------ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/bellows/ash.py b/bellows/ash.py index 2ba3e32b4..165830a35 100644 --- a/bellows/ash.py +++ b/bellows/ash.py @@ -369,6 +369,17 @@ def __init__(self, ezsp_protocol, *, suppress_acks: bool = False) -> None: self._ncp_state: NcpState = NcpState.CONNECTED self._in_reject_condition: bool = False + @property + def suppress_acks(self) -> bool: + """Whether something upstream acknowledges the NCP's frames on our behalf.""" + return self._suppress_acks + + @suppress_acks.setter + def suppress_acks(self, value: bool) -> None: + # Settable after construction because only the open transport can say whether the + # port will do it: the answer comes from the device, not from the path. + self._suppress_acks = value + def connection_made(self, transport): self._transport = transport self._ezsp_protocol.connection_made(self) diff --git a/bellows/ezsp/__init__.py b/bellows/ezsp/__init__.py index c77153333..215a37512 100644 --- a/bellows/ezsp/__init__.py +++ b/bellows/ezsp/__init__.py @@ -121,8 +121,10 @@ def maybe_remove(_): @property def resets_on_connect(self) -> bool: """Whether the NCP may reset by itself as we connect, before we ask it to.""" - parsed_path = urllib.parse.urlparse(self._config[conf.CONF_DEVICE_PATH]) - return parsed_path.scheme in ("socket", "esphome") + path = self._config[conf.CONF_DEVICE_PATH] + if bellows.uart.is_esphome_url(path): + return True + return urllib.parse.urlparse(path).scheme == "socket" async def _startup_reset(self) -> None: """Start EZSP and reset the stack.""" diff --git a/bellows/uart.py b/bellows/uart.py index 4f685766c..6174db973 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -14,13 +14,28 @@ RESET_TIMEOUT = 2.5 # An ESPHome port in this mode ACKs ASH frames on our behalf -ACKLESS_URL_MODE = "ezsp_ash" +ACKLESS_MODE = "ezsp_ash" +# URL schemes served by an ESPHome serial proxy, which is the only transport that can take +# the acknowledging off our hands +ESPHOME_SCHEMES = frozenset({"esphome", "esphome-hass"}) -def url_suppresses_acks(path: str) -> bool: - """Whether something upstream ACKs our ASH frames, making our own ACKs overhead.""" - query = urllib.parse.parse_qs(urllib.parse.urlparse(path).query) - return "mode" in query and ACKLESS_URL_MODE in query["mode"] + +def is_esphome_url(path: str) -> bool: + """Whether the path is served by an ESPHome serial proxy.""" + return urllib.parse.urlparse(path).scheme in ESPHOME_SCHEMES + + +def offloads_acks(transport) -> bool: + """Whether the port will acknowledge the NCP's frames on our behalf. + + Asked of the port rather than assumed from the URL: if we stop acknowledging and nothing + else does, the NCP retransmits until it drops the link, whereas both of us acknowledging + is merely redundant. So this has to be something the device stated, and it is per port -- + a tap belongs to one port, and a device-wide answer would be wrong for every other. + """ + serial = getattr(transport, "serial", None) + return getattr(serial, "tap_mode", None) == ACKLESS_MODE class Gateway(zigpy.serial.SerialProtocol): @@ -123,10 +138,19 @@ async def _connect(config, api): path = config[zigpy.config.CONF_DEVICE_PATH] gateway = Gateway(api, connection_done_future) - protocol = AshProtocol(gateway, suppress_acks=url_suppresses_acks(path)) + protocol = AshProtocol(gateway) flow_control = config[zigpy.config.CONF_DEVICE_FLOW_CONTROL] + extra_kwargs = {} + + if is_esphome_url(path): + # We are the ASH endpoint, so we are the one who knows the framing: ask the proxy for + # it rather than expecting whoever stored the path to have said so. Leaving it out of + # the path also keeps the mode from following that path into other tools -- a firmware + # flasher opening the same port needs a plain byte pipe. + extra_kwargs["mode"] = ACKLESS_MODE + transport, _ = await zigpy.serial.create_serial_connection( loop, lambda: protocol, @@ -134,8 +158,13 @@ async def _connect(config, api): baudrate=config[zigpy.config.CONF_DEVICE_BAUDRATE], xonxoff=flow_control in (None, "software"), rtscts=flow_control == "hardware", + **extra_kwargs, ) + if offloads_acks(transport): + LOGGER.debug("Port acknowledges NCP frames on our behalf, suppressing our own ACKs") + protocol.suppress_acks = True + await gateway.wait_until_connected() thread_safe_protocol = ThreadsafeProxy(gateway, loop) From b6545d4330da1c1ed714980e142a2222394a124c Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:32:44 +0000 Subject: [PATCH 03/11] WIP --- bellows/uart.py | 21 +++++-- tests/test_ezsp.py | 2 +- tests/test_uart.py | 133 +++++++++++++++++++++++++++++++++++---------- 3 files changed, 121 insertions(+), 35 deletions(-) diff --git a/bellows/uart.py b/bellows/uart.py index 6174db973..0c8fd6100 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -4,6 +4,7 @@ import urllib.parse import zigpy.config +import zigpy.exceptions import zigpy.serial from bellows.ash import AshProtocol @@ -13,8 +14,9 @@ LOGGER = logging.getLogger(__name__) RESET_TIMEOUT = 2.5 -# An ESPHome port in this mode ACKs ASH frames on our behalf -ACKLESS_MODE = "ezsp_ash" +# An ESPHome port in this mode engages its tap, which ACKs frames on our behalf. The +# mode does not name a protocol: which one the tap speaks is the device's configuration. +ACKLESS_MODE = "protocol" # URL schemes served by an ESPHome serial proxy, which is the only transport that can take # the acknowledging off our hands @@ -161,8 +163,19 @@ async def _connect(config, api): **extra_kwargs, ) - if offloads_acks(transport): - LOGGER.debug("Port acknowledges NCP frames on our behalf, suppressing our own ACKs") + if is_esphome_url(path): + # The proxy answers our mode request, so a port with no tap is known here rather + # than discovered later as a link that keeps dropping + if not offloads_acks(transport): + transport.close() + raise zigpy.exceptions.ControllerException( + f"Serial proxy port {path!r} has no tap to acknowledge NCP frames." + f" Configure zigbee_proxy_tap on that port." + ) + + LOGGER.debug( + "Port acknowledges NCP frames on our behalf, suppressing our own ACKs" + ) protocol.suppress_acks = True await gateway.wait_until_connected() diff --git a/tests/test_ezsp.py b/tests/test_ezsp.py index 8254e9373..849930570 100644 --- a/tests/test_ezsp.py +++ b/tests/test_ezsp.py @@ -773,7 +773,7 @@ async def test_write_nwk_update_id_failure(ezsp_f): [ ("socket://localhost:1234", True), ( - "esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee&mode=ezsp_ash", + "esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee&mode=protocol", True, ), ("esphome://example.com:6053/", True), diff --git a/tests/test_uart.py b/tests/test_uart.py index 2ae283f1b..8db9b2292 100644 --- a/tests/test_uart.py +++ b/tests/test_uart.py @@ -4,6 +4,7 @@ import pytest import zigpy.config as conf +import zigpy.exceptions import zigpy.serial from bellows import uart @@ -151,58 +152,102 @@ async def mock_connect(loop, protocol_factory, *args, **kwargs): @pytest.mark.parametrize( - ("path", "suppressed"), + ("tap_mode", "offloads"), + [ + (uart.ACKLESS_MODE, True), + ("raw", False), + (None, False), + # A port that has never been resolved has nothing to say yet + (MagicMock(), False), + ], +) +def test_offloads_acks(tap_mode, offloads: bool) -> None: + """Whether the port acknowledges for us is asked of the port, not of the URL.""" + transport = MagicMock() + transport.serial.tap_mode = tap_mode + assert uart.offloads_acks(transport) is offloads + + +def test_offloads_acks_without_a_serial_port() -> None: + """A transport with no underlying serial object never offloads.""" + transport = MagicMock() + transport.serial = None + assert uart.offloads_acks(transport) is False + + +@pytest.mark.parametrize( + ("path", "tap_mode", "suppressed"), [ ( - "esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee&mode=ezsp_ash", + "esphome://example.com:6053/?port_name=Zigbee", + uart.ACKLESS_MODE, True, ), - ("esphome://example.com:6053/?mode=ezsp_ash", True), - ("esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee", False), - ("esphome://example.com:6053/", False), - ("socket://example.com:1234", False), - ("/dev/serial", False), - # The literal text in the wrong position does not count - ("esphome://example.com:6053/?port_name=mode=ezsp_ash", False), - ("esphome://example.com:6053/?port_name=mode%3Dezsp_ash", False), - ("esphome://example.com:6053/?noise_psk=mode=ezsp_ash", False), - ("/dev/serial/by-id/mode=ezsp_ash", False), + ("/dev/serial", None, False), + # A non-proxy transport is never asked to acknowledge for us + ("socket://example.com:1234", None, False), ], ) -def test_url_suppresses_acks(path: str, suppressed: bool) -> None: - assert uart.url_suppresses_acks(path) is suppressed +async def test_connect_suppress_acks(path, tap_mode, suppressed, monkeypatch): + appmock = MagicMock() + transport = MagicMock() + transport.serial.tap_mode = tap_mode + protocols = [] + + async def mockconnect(loop, protocol_factory, **kwargs): + protocol = protocol_factory() + protocols.append(protocol) + loop.call_soon(protocol.connection_made, transport) + return transport, protocol + + monkeypatch.setattr(zigpy.serial, "create_serial_connection", mockconnect) + + gw = await uart.connect( + conf.SCHEMA_DEVICE( + {conf.CONF_DEVICE_PATH: path, conf.CONF_DEVICE_BAUDRATE: 115200} + ), + appmock, + use_thread=False, + ) + + assert len(protocols) == 1 + assert protocols[0].suppress_acks is suppressed + + gw.close() @pytest.mark.parametrize( - ("path", "suppressed"), + ("path", "requests_mode"), [ - ("esphome://example.com:6053/?port_name=Zigbee&mode=ezsp_ash", True), - ("esphome://example.com:6053/?port_name=Zigbee", False), + ("esphome://example.com:6053/?port_name=Zigbee", True), + ("esphome-hass://example.com:6053/?port_name=Zigbee", True), + ("socket://example.com:1234", False), ("/dev/serial", False), ], ) -async def test_connect_suppress_acks(path, suppressed, monkeypatch): - appmock = MagicMock() +async def test_connect_requests_ackless_mode(path, requests_mode, monkeypatch): + """The mode is asked for by us, not carried in the stored path.""" + kwargs_seen = {} transport = MagicMock() + transport.serial.tap_mode = uart.ACKLESS_MODE async def mockconnect(loop, protocol_factory, **kwargs): + kwargs_seen.update(kwargs) protocol = protocol_factory() loop.call_soon(protocol.connection_made, transport) - return None, protocol + return transport, protocol monkeypatch.setattr(zigpy.serial, "create_serial_connection", mockconnect) - with patch("bellows.uart.AshProtocol", wraps=uart.AshProtocol) as ash_mock: - gw = await uart.connect( - conf.SCHEMA_DEVICE( - {conf.CONF_DEVICE_PATH: path, conf.CONF_DEVICE_BAUDRATE: 115200} - ), - appmock, - use_thread=False, - ) + gw = await uart.connect( + conf.SCHEMA_DEVICE( + {conf.CONF_DEVICE_PATH: path, conf.CONF_DEVICE_BAUDRATE: 115200} + ), + MagicMock(), + use_thread=False, + ) - assert len(ash_mock.mock_calls) == 1 - assert ash_mock.mock_calls[0].kwargs == {"suppress_acks": suppressed} + assert kwargs_seen.get("mode") == (uart.ACKLESS_MODE if requests_mode else None) gw.close() @@ -339,3 +384,31 @@ def test_unexpected_reset_triggers_failed_state(gw): assert gw._api.enter_failed_state.mock_calls == [ call(t.NcpResetCode.RESET_SOFTWARE) ] + + +@pytest.mark.parametrize("tap_mode", ["raw", None]) +async def test_connect_without_a_tap_is_refused(tap_mode, monkeypatch): + """A proxy port that will not acknowledge for us is an error, not a fallback.""" + transport = MagicMock() + transport.serial.tap_mode = tap_mode + + async def mockconnect(loop, protocol_factory, **kwargs): + protocol = protocol_factory() + loop.call_soon(protocol.connection_made, transport) + return transport, protocol + + monkeypatch.setattr(zigpy.serial, "create_serial_connection", mockconnect) + + with pytest.raises(zigpy.exceptions.ControllerException, match="no tap"): + await uart.connect( + conf.SCHEMA_DEVICE( + { + conf.CONF_DEVICE_PATH: "esphome://example.com:6053/?port_name=Zigbee", + conf.CONF_DEVICE_BAUDRATE: 115200, + } + ), + MagicMock(), + use_thread=False, + ) + + assert len(transport.close.mock_calls) == 1 From b23f3e84c91633b7b82ed6437928ba2f8750ea8b Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 13 Oct 2026 16:25:59 +0000 Subject: [PATCH 04/11] Clean up --- bellows/ezsp/__init__.py | 8 ++++---- bellows/uart.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bellows/ezsp/__init__.py b/bellows/ezsp/__init__.py index 215a37512..876e2cbe0 100644 --- a/bellows/ezsp/__init__.py +++ b/bellows/ezsp/__init__.py @@ -128,10 +128,10 @@ def resets_on_connect(self) -> bool: async def _startup_reset(self) -> None: """Start EZSP and reset the stack.""" - # `zigbeed` resets on startup, and an ESPHome proxy resets the NCP when it takes - # over the shared UART. Absorbing that RSTACK here matters: outside this window an - # unsolicited reset reaches `Gateway.reset_received` with no future waiting for it, - # which fails the connection and cancels whatever command is in flight. + # `zigbeed` resets on startup. Absorbing that RSTACK here matters: outside this + # window an unsolicited reset reaches `Gateway.reset_received` with no future + # waiting for it, which fails the connection and cancels whatever command is in + # flight. if self.resets_on_connect: try: async with asyncio_timeout(NETWORK_COORDINATOR_STARTUP_RESET_WAIT): diff --git a/bellows/uart.py b/bellows/uart.py index 0c8fd6100..9cecb98a2 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -170,7 +170,7 @@ async def _connect(config, api): transport.close() raise zigpy.exceptions.ControllerException( f"Serial proxy port {path!r} has no tap to acknowledge NCP frames." - f" Configure zigbee_proxy_tap on that port." + f" Configure `ezsp_proxy_tap` on that port." ) LOGGER.debug( From 0a88c190fc3293992c0040b7b89bb07b96aef266 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 13 Oct 2026 16:26:56 +0000 Subject: [PATCH 05/11] Add a minimum serialx version for serial proxy tap --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 84074acd2..db331f570 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,6 +18,7 @@ dependencies = [ "click-log>=0.2.1", "voluptuous", "zigpy>=2.1.0", + "serialx>=1.11.0" ] [tool.setuptools.packages.find] From ca6e66f54b236978e4a71b8436dddd2d00a519ff Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 13 Oct 2026 16:29:14 +0000 Subject: [PATCH 06/11] Clean up comments --- bellows/uart.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/bellows/uart.py b/bellows/uart.py index 9cecb98a2..cd34762b9 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -29,13 +29,7 @@ def is_esphome_url(path: str) -> bool: def offloads_acks(transport) -> bool: - """Whether the port will acknowledge the NCP's frames on our behalf. - - Asked of the port rather than assumed from the URL: if we stop acknowledging and nothing - else does, the NCP retransmits until it drops the link, whereas both of us acknowledging - is merely redundant. So this has to be something the device stated, and it is per port -- - a tap belongs to one port, and a device-wide answer would be wrong for every other. - """ + """Whether the port will acknowledge the NCP's frames on our behalf.""" serial = getattr(transport, "serial", None) return getattr(serial, "tap_mode", None) == ACKLESS_MODE From 714ab5a972890242f0cd9aeed5d004f70978e0fe Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 13 Oct 2026 16:31:12 +0000 Subject: [PATCH 07/11] Remove old code --- bellows/ezsp/__init__.py | 16 +++++----------- bellows/uart.py | 4 ---- tests/test_ezsp.py | 24 ------------------------ 3 files changed, 5 insertions(+), 39 deletions(-) diff --git a/bellows/ezsp/__init__.py b/bellows/ezsp/__init__.py index 876e2cbe0..a21138292 100644 --- a/bellows/ezsp/__init__.py +++ b/bellows/ezsp/__init__.py @@ -119,20 +119,14 @@ def maybe_remove(_): listeners.remove(future) @property - def resets_on_connect(self) -> bool: - """Whether the NCP may reset by itself as we connect, before we ask it to.""" - path = self._config[conf.CONF_DEVICE_PATH] - if bellows.uart.is_esphome_url(path): - return True - return urllib.parse.urlparse(path).scheme == "socket" + def is_tcp_serial_port(self) -> bool: + parsed_path = urllib.parse.urlparse(self._config[conf.CONF_DEVICE_PATH]) + return parsed_path.scheme == "socket" async def _startup_reset(self) -> None: """Start EZSP and reset the stack.""" - # `zigbeed` resets on startup. Absorbing that RSTACK here matters: outside this - # window an unsolicited reset reaches `Gateway.reset_received` with no future - # waiting for it, which fails the connection and cancels whatever command is in - # flight. - if self.resets_on_connect: + # `zigbeed` resets on startup + if self.is_tcp_serial_port: try: async with asyncio_timeout(NETWORK_COORDINATOR_STARTUP_RESET_WAIT): await self._gw.wait_for_startup_reset() diff --git a/bellows/uart.py b/bellows/uart.py index cd34762b9..2a14ef92c 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -141,10 +141,6 @@ async def _connect(config, api): extra_kwargs = {} if is_esphome_url(path): - # We are the ASH endpoint, so we are the one who knows the framing: ask the proxy for - # it rather than expecting whoever stored the path to have said so. Leaving it out of - # the path also keeps the mode from following that path into other tools -- a firmware - # flasher opening the same port needs a plain byte pipe. extra_kwargs["mode"] = ACKLESS_MODE transport, _ = await zigpy.serial.create_serial_connection( diff --git a/tests/test_ezsp.py b/tests/test_ezsp.py index 849930570..bccf17b39 100644 --- a/tests/test_ezsp.py +++ b/tests/test_ezsp.py @@ -768,30 +768,6 @@ async def test_write_nwk_update_id_failure(ezsp_f): ) -@pytest.mark.parametrize( - ("path", "resets"), - [ - ("socket://localhost:1234", True), - ( - "esphome://example.com:6053/?noise_psk=KEY&port_name=Zigbee&mode=protocol", - True, - ), - ("esphome://example.com:6053/", True), - ("/dev/serial", False), - ("/dev/null", False), - ], -) -def test_resets_on_connect(path: str, resets: bool) -> None: - ezsp = make_ezsp( - config={ - **DEVICE_CONFIG, - zigpy.config.CONF_DEVICE_PATH: path, - } - ) - - assert ezsp.resets_on_connect is resets - - @patch.object(EZSP, "version", new_callable=AsyncMock) @patch.object(EZSP, "reset", new_callable=AsyncMock) @patch.object(EZSP, "get_xncp_features", new_callable=AsyncMock) From 3ff6d0d9bcb22ebf18011672c68a0a3ad767cfc9 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:40:13 +0000 Subject: [PATCH 08/11] Offload tap mode validation to serialx --- bellows/uart.py | 18 ++---------- tests/test_uart.py | 69 ++++------------------------------------------ 2 files changed, 7 insertions(+), 80 deletions(-) diff --git a/bellows/uart.py b/bellows/uart.py index 2a14ef92c..fe4f1444f 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -4,7 +4,6 @@ import urllib.parse import zigpy.config -import zigpy.exceptions import zigpy.serial from bellows.ash import AshProtocol @@ -28,12 +27,6 @@ def is_esphome_url(path: str) -> bool: return urllib.parse.urlparse(path).scheme in ESPHOME_SCHEMES -def offloads_acks(transport) -> bool: - """Whether the port will acknowledge the NCP's frames on our behalf.""" - serial = getattr(transport, "serial", None) - return getattr(serial, "tap_mode", None) == ACKLESS_MODE - - class Gateway(zigpy.serial.SerialProtocol): def __init__(self, api, connection_done_future=None): super().__init__() @@ -154,15 +147,8 @@ async def _connect(config, api): ) if is_esphome_url(path): - # The proxy answers our mode request, so a port with no tap is known here rather - # than discovered later as a link that keeps dropping - if not offloads_acks(transport): - transport.close() - raise zigpy.exceptions.ControllerException( - f"Serial proxy port {path!r} has no tap to acknowledge NCP frames." - f" Configure `ezsp_proxy_tap` on that port." - ) - + # serialx refuses to open a port that cannot deliver the mode, so reaching here + # means the tap is acknowledging for us LOGGER.debug( "Port acknowledges NCP frames on our behalf, suppressing our own ACKs" ) diff --git a/tests/test_uart.py b/tests/test_uart.py index 8db9b2292..2eb684a73 100644 --- a/tests/test_uart.py +++ b/tests/test_uart.py @@ -4,7 +4,6 @@ import pytest import zigpy.config as conf -import zigpy.exceptions import zigpy.serial from bellows import uart @@ -152,46 +151,17 @@ async def mock_connect(loop, protocol_factory, *args, **kwargs): @pytest.mark.parametrize( - ("tap_mode", "offloads"), + ("path", "suppressed"), [ - (uart.ACKLESS_MODE, True), - ("raw", False), - (None, False), - # A port that has never been resolved has nothing to say yet - (MagicMock(), False), - ], -) -def test_offloads_acks(tap_mode, offloads: bool) -> None: - """Whether the port acknowledges for us is asked of the port, not of the URL.""" - transport = MagicMock() - transport.serial.tap_mode = tap_mode - assert uart.offloads_acks(transport) is offloads - - -def test_offloads_acks_without_a_serial_port() -> None: - """A transport with no underlying serial object never offloads.""" - transport = MagicMock() - transport.serial = None - assert uart.offloads_acks(transport) is False - - -@pytest.mark.parametrize( - ("path", "tap_mode", "suppressed"), - [ - ( - "esphome://example.com:6053/?port_name=Zigbee", - uart.ACKLESS_MODE, - True, - ), - ("/dev/serial", None, False), + ("esphome://example.com:6053/?port_name=Zigbee", True), + ("/dev/serial", False), # A non-proxy transport is never asked to acknowledge for us - ("socket://example.com:1234", None, False), + ("socket://example.com:1234", False), ], ) -async def test_connect_suppress_acks(path, tap_mode, suppressed, monkeypatch): +async def test_connect_suppress_acks(path, suppressed, monkeypatch): appmock = MagicMock() transport = MagicMock() - transport.serial.tap_mode = tap_mode protocols = [] async def mockconnect(loop, protocol_factory, **kwargs): @@ -229,7 +199,6 @@ async def test_connect_requests_ackless_mode(path, requests_mode, monkeypatch): """The mode is asked for by us, not carried in the stored path.""" kwargs_seen = {} transport = MagicMock() - transport.serial.tap_mode = uart.ACKLESS_MODE async def mockconnect(loop, protocol_factory, **kwargs): kwargs_seen.update(kwargs) @@ -384,31 +353,3 @@ def test_unexpected_reset_triggers_failed_state(gw): assert gw._api.enter_failed_state.mock_calls == [ call(t.NcpResetCode.RESET_SOFTWARE) ] - - -@pytest.mark.parametrize("tap_mode", ["raw", None]) -async def test_connect_without_a_tap_is_refused(tap_mode, monkeypatch): - """A proxy port that will not acknowledge for us is an error, not a fallback.""" - transport = MagicMock() - transport.serial.tap_mode = tap_mode - - async def mockconnect(loop, protocol_factory, **kwargs): - protocol = protocol_factory() - loop.call_soon(protocol.connection_made, transport) - return transport, protocol - - monkeypatch.setattr(zigpy.serial, "create_serial_connection", mockconnect) - - with pytest.raises(zigpy.exceptions.ControllerException, match="no tap"): - await uart.connect( - conf.SCHEMA_DEVICE( - { - conf.CONF_DEVICE_PATH: "esphome://example.com:6053/?port_name=Zigbee", - conf.CONF_DEVICE_BAUDRATE: 115200, - } - ), - MagicMock(), - use_thread=False, - ) - - assert len(transport.close.mock_calls) == 1 From 5e897327ff6573a317c0964d4e55e45133448d69 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:48:34 +0000 Subject: [PATCH 09/11] Suppress ACKs early --- bellows/ash.py | 6 ------ bellows/uart.py | 7 +++---- tests/test_uart.py | 2 ++ 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/bellows/ash.py b/bellows/ash.py index 165830a35..fe5376b01 100644 --- a/bellows/ash.py +++ b/bellows/ash.py @@ -374,12 +374,6 @@ def suppress_acks(self) -> bool: """Whether something upstream acknowledges the NCP's frames on our behalf.""" return self._suppress_acks - @suppress_acks.setter - def suppress_acks(self, value: bool) -> None: - # Settable after construction because only the open transport can say whether the - # port will do it: the answer comes from the device, not from the path. - self._suppress_acks = value - def connection_made(self, transport): self._transport = transport self._ezsp_protocol.connection_made(self) diff --git a/bellows/uart.py b/bellows/uart.py index fe4f1444f..f17558dc8 100644 --- a/bellows/uart.py +++ b/bellows/uart.py @@ -127,7 +127,9 @@ async def _connect(config, api): path = config[zigpy.config.CONF_DEVICE_PATH] gateway = Gateway(api, connection_done_future) - protocol = AshProtocol(gateway) + # serialx opens an ESPHome port in the mode we ask for or raises, so whether the tap + # acknowledges for us is known before the first frame arrives + protocol = AshProtocol(gateway, suppress_acks=is_esphome_url(path)) flow_control = config[zigpy.config.CONF_DEVICE_FLOW_CONTROL] @@ -147,12 +149,9 @@ async def _connect(config, api): ) if is_esphome_url(path): - # serialx refuses to open a port that cannot deliver the mode, so reaching here - # means the tap is acknowledging for us LOGGER.debug( "Port acknowledges NCP frames on our behalf, suppressing our own ACKs" ) - protocol.suppress_acks = True await gateway.wait_until_connected() diff --git a/tests/test_uart.py b/tests/test_uart.py index 2eb684a73..a7cf752c3 100644 --- a/tests/test_uart.py +++ b/tests/test_uart.py @@ -160,6 +160,7 @@ async def mock_connect(loop, protocol_factory, *args, **kwargs): ], ) async def test_connect_suppress_acks(path, suppressed, monkeypatch): + """Suppression is decided before connecting: a frame can arrive mid-handshake.""" appmock = MagicMock() transport = MagicMock() protocols = [] @@ -167,6 +168,7 @@ async def test_connect_suppress_acks(path, suppressed, monkeypatch): async def mockconnect(loop, protocol_factory, **kwargs): protocol = protocol_factory() protocols.append(protocol) + assert protocol.suppress_acks is suppressed loop.call_soon(protocol.connection_made, transport) return transport, protocol From ababfe51acfd14a79a5022338e16104554ec37b2 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:49:01 +0000 Subject: [PATCH 10/11] Bump minimum serialx to 1.12.0 (unreleased) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index db331f570..8935ecf6b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ "click-log>=0.2.1", "voluptuous", "zigpy>=2.1.0", - "serialx>=1.11.0" + "serialx>=1.12.0" ] [tool.setuptools.packages.find] From 17e79aa7c35db690692ba4deb274b6ee09839b8a Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Wed, 21 Oct 2026 19:59:22 +0000 Subject: [PATCH 11/11] Bump minimum serialx to 1.13.0 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 8935ecf6b..c7c907386 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ "click-log>=0.2.1", "voluptuous", "zigpy>=2.1.0", - "serialx>=1.12.0" + "serialx>=1.13.0" ] [tool.setuptools.packages.find]