From 1875caafbab59ad8c3109ceb2217853667ced6df Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:40:20 -0500 Subject: [PATCH 1/3] Give connect an overall deadline so a board that floods output ends with a RESET-button message (#193) --- cli/src/mpftp/sidecar.py | 74 +++++++++++++++++++++++++- cli/tests/test_connect_deadline.py | 85 ++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 cli/tests/test_connect_deadline.py diff --git a/cli/src/mpftp/sidecar.py b/cli/src/mpftp/sidecar.py index 87f269e..1163dc7 100644 --- a/cli/src/mpftp/sidecar.py +++ b/cli/src/mpftp/sidecar.py @@ -203,6 +203,60 @@ def _bound_write_timeout(transport: Any, secs: float = WRITE_TIMEOUT_SECS) -> No pass +#: Longest one connect attempt may spend taking control of a serial board, in +#: seconds. mpremote's timeouts are between bytes, so a board that prints +#: forever never trips them. ``MPFTP_CONNECT_DEADLINE`` overrides it. +CONNECT_DEADLINE_SECS = 45.0 + + +def connect_deadline_secs() -> float: + try: + return float(os.environ.get("MPFTP_CONNECT_DEADLINE") or CONNECT_DEADLINE_SECS) + except ValueError: + return CONNECT_DEADLINE_SECS + + +class ConnectDeadlineError(RuntimeError): + """The connect deadline passed while the board was still sending bytes.""" + + +class _DeadlineSerial: + """A serial port that refuses everything once a deadline has passed. + + Wraps the port only while connect takes control, so a flood of output that + keeps mpremote's per-byte timeout from ever firing still ends the attempt. + After the deadline even a line-control write (the reset path) is refused. + """ + + def __init__(self, inner: Any, deadline: float) -> None: + object.__setattr__(self, "_inner", inner) + object.__setattr__(self, "_deadline", deadline) + + def _check(self) -> None: + import time + + if time.monotonic() >= self._deadline: + raise ConnectDeadlineError("connect deadline passed") + + def __getattr__(self, name: str) -> Any: + self._check() + return getattr(self._inner, name) + + def __setattr__(self, name: str, value: Any) -> None: + self._check() + setattr(self._inner, name, value) + + +def connect_deadline_message(device: str, secs: float) -> str: + return ( + f"{device} kept sending output for {secs:g} s and never answered the REPL, " + f"so mpftp gave up. A program that prints continuously (a timer callback " + f"that never stops, say) keeps the board from listening. A board with only " + f"its native USB for serial can't be reset from the host: press its RESET " + f"button, then try again." + ) + + def is_eof_timeout_error(exc: BaseException) -> bool: """True when mpremote follow-exec timed out waiting for raw-REPL EOF.""" msg = str(exc).lower() @@ -1001,9 +1055,27 @@ def connect( raise RuntimeError(self._friendly_port_open_error(device, e)) from e # Opening the COM port succeeds in UF2/bootloader mode too — require a # MicroPython raw-REPL handshake before we report connected. + bounded = None try: - rtc = self._probe_micropython(self.transport) + secs = connect_deadline_secs() + raw_serial = getattr(self.transport, "serial", None) + if raw_serial is not None and not network: + bounded = _DeadlineSerial(raw_serial, time.monotonic() + secs) + self.transport.serial = bounded + try: + rtc = self._probe_micropython(self.transport) + finally: + if bounded is not None: + self.transport.serial = raw_serial except Exception as e: + if bounded is not None and time.monotonic() >= bounded._deadline: + try: + self.transport.close() + except Exception: + pass + self.transport = None + self.device = None + raise RuntimeError(connect_deadline_message(device, secs)) from e trace("attempt %d: probe failed: %s" % (attempt, str(e).splitlines()[0][:80])) try: self.transport.close() diff --git a/cli/tests/test_connect_deadline.py b/cli/tests/test_connect_deadline.py new file mode 100644 index 0000000..80f9227 --- /dev/null +++ b/cli/tests/test_connect_deadline.py @@ -0,0 +1,85 @@ +"""A board that floods output forever must not hold connect indefinitely (mpftp#193). + +mpremote's timeouts run between bytes, so continuous output defeats them. +No board required: a fake transport streams forever. +""" + +from __future__ import annotations + +import os +import time +import unittest +from unittest import mock + + +class FloodingSerial: + """Always has a byte to give, like a Timer printing every few ms.""" + + def __init__(self): + self.writes = [] + self.dtr = False + + def inWaiting(self): + return 64 + + def read(self, n=1): + time.sleep(0.001) + return b"tick tick tick\r\n"[:n] if n < 16 else b"tick tick tick\r\n" + + def write(self, data): + self.writes.append(data) + return len(data) + + def close(self): + pass + + +class FloodingTransport: + def __init__(self): + self.serial = FloodingSerial() + self.in_raw_repl = False + self.closed = False + + def enter_raw_repl(self, soft_reset=True, timeout_overall=10.0): + # mpremote's read_until: restarts its per-byte timer on every byte. + while True: + self.serial.read(1) + + def close(self): + self.closed = True + + +class ConnectDeadlineTests(unittest.TestCase): + def test_endless_flood_ends_with_a_reset_button_message(self): + from mpftp import sidecar + + s = sidecar.Session() + t = FloodingTransport() + started = time.monotonic() + with mock.patch.dict(os.environ, {"MPFTP_CONNECT_DEADLINE": "1.5"}), \ + mock.patch.object(s, "_open_transport", return_value=t), \ + mock.patch.object(s, "_serial_peek", return_value=b""): + with self.assertRaises(RuntimeError) as cm: + s.connect("COM99", attempts=3) + took = time.monotonic() - started + msg = str(cm.exception) + self.assertIn("RESET button", msg) + self.assertIn("kept sending output", msg) + self.assertLess(took, 6) + self.assertTrue(t.closed) + self.assertIsNone(s.transport) + + def test_the_port_is_unwrapped_after_a_good_connect(self): + from mpftp import sidecar + + s = sidecar.Session() + t = FloodingTransport() + raw = t.serial + with mock.patch.object(s, "_open_transport", return_value=t), \ + mock.patch.object(s, "_probe_micropython", return_value=None): + s.connect("COM99", attempts=1) + self.assertIs(t.serial, raw) + + +if __name__ == "__main__": + unittest.main() From 765c4ad5bf703e678fb68f492a8981482bc544e6 Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:46:49 -0500 Subject: [PATCH 2/3] Combine the with statements in the deadline test --- cli/tests/test_connect_deadline.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/tests/test_connect_deadline.py b/cli/tests/test_connect_deadline.py index 80f9227..8d976d6 100644 --- a/cli/tests/test_connect_deadline.py +++ b/cli/tests/test_connect_deadline.py @@ -58,9 +58,9 @@ def test_endless_flood_ends_with_a_reset_button_message(self): started = time.monotonic() with mock.patch.dict(os.environ, {"MPFTP_CONNECT_DEADLINE": "1.5"}), \ mock.patch.object(s, "_open_transport", return_value=t), \ - mock.patch.object(s, "_serial_peek", return_value=b""): - with self.assertRaises(RuntimeError) as cm: - s.connect("COM99", attempts=3) + mock.patch.object(s, "_serial_peek", return_value=b""), \ + self.assertRaises(RuntimeError) as cm: + s.connect("COM99", attempts=3) took = time.monotonic() - started msg = str(cm.exception) self.assertIn("RESET button", msg) From 3c5bd3553ace7e9d20f238d9cd3aff667db747a5 Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:52:18 -0500 Subject: [PATCH 3/3] Bound the port under the read-ahead wrapper so a flood inside read_until is cut off --- cli/src/mpftp/sidecar.py | 25 +++++++++++++++++++++---- cli/tests/test_connect_deadline.py | 24 ++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/cli/src/mpftp/sidecar.py b/cli/src/mpftp/sidecar.py index 1163dc7..c50dfb3 100644 --- a/cli/src/mpftp/sidecar.py +++ b/cli/src/mpftp/sidecar.py @@ -247,6 +247,22 @@ def __setattr__(self, name: str, value: Any) -> None: setattr(self._inner, name, value) +def _serial_holder(transport: Any) -> tuple[Any, str]: + """Where the raw pyserial port sits, so it can be wrapped in place. + + Our transport reads through a ReadAheadSerial that keeps buffered bytes, so + the port inside it is wrapped, not the transport's ``serial`` (setting that + would build a new, empty ReadAheadSerial). Any other transport holds the + port in ``serial``. + """ + ser = getattr(transport, "serial", None) + if ser is None: + return None, "" + if "_port" in getattr(ser, "__dict__", {}): + return ser, "_port" + return transport, "serial" + + def connect_deadline_message(device: str, secs: float) -> str: return ( f"{device} kept sending output for {secs:g} s and never answered the REPL, " @@ -1058,15 +1074,16 @@ def connect( bounded = None try: secs = connect_deadline_secs() - raw_serial = getattr(self.transport, "serial", None) - if raw_serial is not None and not network: + holder, attr = _serial_holder(self.transport) + if holder is not None and not network: + raw_serial = object.__getattribute__(holder, attr) bounded = _DeadlineSerial(raw_serial, time.monotonic() + secs) - self.transport.serial = bounded + object.__setattr__(holder, attr, bounded) try: rtc = self._probe_micropython(self.transport) finally: if bounded is not None: - self.transport.serial = raw_serial + object.__setattr__(holder, attr, raw_serial) except Exception as e: if bounded is not None and time.monotonic() >= bounded._deadline: try: diff --git a/cli/tests/test_connect_deadline.py b/cli/tests/test_connect_deadline.py index 8d976d6..29dc872 100644 --- a/cli/tests/test_connect_deadline.py +++ b/cli/tests/test_connect_deadline.py @@ -69,6 +69,30 @@ def test_endless_flood_ends_with_a_reset_button_message(self): self.assertTrue(t.closed) self.assertIsNone(s.transport) + def test_flood_inside_our_read_ahead_transport_is_cut_off(self): + # The real transport reads through ReadAheadSerial.read_until, which + # checks timeout_overall only while idle, so a flood stays inside it. + from mpftp import rawpaste, sidecar + + s = sidecar.Session() + cls = rawpaste._transport_class() + t = cls.__new__(cls) + t.in_raw_repl = False + t.use_raw_paste = True + t.device_name = "COM99" + t.mounted = False + t.is_pty = False + t.paced = None + t.serial = FloodingSerial() + started = time.monotonic() + with mock.patch.dict(os.environ, {"MPFTP_CONNECT_DEADLINE": "1.5"}), \ + mock.patch.object(s, "_open_transport", return_value=t), \ + mock.patch.object(s, "_serial_peek", return_value=b""), \ + self.assertRaises(RuntimeError) as cm: + s.connect("COM99", attempts=3) + self.assertIn("RESET button", str(cm.exception)) + self.assertLess(time.monotonic() - started, 8) + def test_the_port_is_unwrapped_after_a_good_connect(self): from mpftp import sidecar