diff --git a/cli/src/mpftp/rawpaste.py b/cli/src/mpftp/rawpaste.py index 535a1ac..d9ef917 100644 --- a/cli/src/mpftp/rawpaste.py +++ b/cli/src/mpftp/rawpaste.py @@ -79,6 +79,133 @@ def paced_raw_paste_write(transport: Any, command_bytes: bytes) -> None: raise TransportError("could not complete raw paste: {!r}".format(data)) +def _repr_line(out: bytes) -> bytes: + """The printed ``repr`` in a board's reply: its first line, with the zeros + some boards append to it taken off. A ``repr`` never holds a raw zero, CR + or LF, so whatever follows its closing quote is not content.""" + line = out.split(b"\r", 1)[0].split(b"\n", 1)[0] + return line.rstrip(b"\x00 ") + + +def read_file_checked( + transport: Any, + src: str, + chunk_size: int = 256, + progress_callback: Optional[Callable[[int, int], None]] = None, + attempts: int = 8, +) -> bytearray: + """mpremote's ``fs_readfile``, except that a chunk that arrives wrong is + asked for again. + + mpremote reads a file 256 bytes at a time as ``print(repr(r(256)))`` and + parses the line it gets back. The P4 DEV-KIT's native USB sometimes sends + more than it was given: after the closing quote come the zero padding of + the string's memory and, now and then, the 16-byte header of the string + object next to it, a byte of which can be the ``\\x04`` that ends the + reply. A 2.8 MB model failed that way twice in two attempts ("source code + string cannot contain null bytes"), and a third run lost the framing too. + The bytes of the file inside the quotes were right every time, so each + chunk is read with an explicit ``seek``, parsed from the first line only, + and checked against the length it should have; a chunk that fails either + is read again, up to ``attempts`` times. + """ + import ast + + size = None + for _ in range(attempts): + try: + size = transport.fs_stat(src).st_size + break + except OSError: + raise + except Exception as e: # noqa: BLE001 - a stat reply with noise in it + if _is_board_error(e): + raise _convert_fs_error(transport, e, src) from None + _resync(transport) + if size is None: + raise OSError("could not stat %s" % src) + contents = bytearray() + try: + transport.exec("f=open('%s','rb')\nr=f.read\ns=f.seek" % src) + except Exception as e: # noqa: BLE001 - same conversion mpremote does + raise _convert_fs_error(transport, e, src) from None + try: + while len(contents) < size: + want = min(chunk_size, size - len(contents)) + chunk = None + last: Optional[BaseException] = None + for _ in range(attempts): + try: + out = transport.exec( + "s(%d)\nprint(repr(r(%d)))" % (len(contents), want) + ) + value = ast.literal_eval(_repr_line(out).decode("ascii")) + except (ValueError, SyntaxError, UnicodeDecodeError, MemoryError) as e: + last = e + _resync(transport) + continue + except Exception as e: # noqa: BLE001 + # A TransportExecError whose "error" is the spurious bytes + # (its text is decoded from them) or a framing timeout + # that the next command's wait for the prompt recovers. + if _is_board_error(e): + raise _convert_fs_error(transport, e, src) from None + last = e + _resync(transport) + continue + if isinstance(value, (bytes, bytearray)) and len(value) == want: + chunk = bytes(value) + break + last = ValueError( + "read %d bytes where %d were asked for" + % (len(value) if hasattr(value, "__len__") else -1, want) + ) + _resync(transport) + if chunk is None: + raise OSError( + "reading %s at byte %d failed %d times: %s" + % (src, len(contents), attempts, last) + ) + contents.extend(chunk) + if progress_callback: + progress_callback(len(contents), size) + finally: + try: + transport.exec("f.close()") + except Exception: # noqa: BLE001 - the read's own error is the one to show + pass + return contents + + +def _resync(transport: Any) -> None: + """Back to a clean raw-REPL prompt after a reply that did not parse. The + spurious bytes can include a ``\\x04``, which ends the reply early and leaves + the real end marker and prompt unread. Entering raw REPL again (no soft + reset, so the open file stays open) throws all of that away.""" + enter = getattr(transport, "enter_raw_repl", None) + if enter is not None: + try: + enter(soft_reset=False) + except Exception: # noqa: BLE001 - the retry will say if it did not work + pass + + +def _is_board_error(exc: BaseException) -> bool: + """True for an error the board reported about the file (a Python + traceback), not for noise in the reply.""" + text = str(getattr(exc, "error_output", "") or "") + return "Traceback" in text + + +def _convert_fs_error(transport: Any, exc: BaseException, src: str) -> BaseException: + try: + from mpremote.transport import _convert_filesystem_error + + return _convert_filesystem_error(exc, src) or exc + except Exception: # noqa: BLE001 + return exc + + def _transport_class() -> Any: global _TRANSPORT_CLASS if _TRANSPORT_CLASS is not None: @@ -155,6 +282,9 @@ def raw_paste_write(self, command_bytes: bytes) -> None: else: super().raw_paste_write(command_bytes) + def fs_readfile(self, src, chunk_size=256, progress_callback=None): + return read_file_checked(self, src, chunk_size, progress_callback) + _TRANSPORT_CLASS = PacedSerialTransport return _TRANSPORT_CLASS diff --git a/cli/tests/test_read_file_checked.py b/cli/tests/test_read_file_checked.py new file mode 100644 index 0000000..97a98a5 --- /dev/null +++ b/cli/tests/test_read_file_checked.py @@ -0,0 +1,152 @@ +"""``get`` survives a board that sends more than it was asked to print. + +The P4 DEV-KIT, on its native USB, sometimes sends bytes after the closing +quote of a printed ``repr``: the zero padding of the string's memory, and now +and then the 16-byte header of the string object beside it. A byte of that +header can be ``\\x04``, which ends the reply early. mpremote parsed the line +with ``ast.literal_eval`` and failed on the zeros ("source code string cannot +contain null bytes"), once in a few hundred thousand bytes. These tests give +``read_file_checked`` a stand-in board that does the same and check that the +file comes out byte for byte. +""" + +from __future__ import annotations + +import contextlib +import io +import random +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace + +from mpftp import rawpaste + +try: + from mpremote.transport import TransportExecError + + HAVE_MPREMOTE = True +except ImportError: + HAVE_MPREMOTE = False + + class TransportExecError(Exception): # type: ignore[no-redef] + def __init__(self, status_code, error_output): + self.status_code = status_code + self.error_output = error_output + super().__init__(error_output) + + +def blob() -> bytes: + rng = random.Random(7) + return bytes(range(256)) + bytes(700) + bytes(rng.randrange(256) for _ in range(4000)) + b"\x04\x00" + + +class FaultyBoard: + """Runs the commands under CPython; ``fault(n, out)`` may change reply n.""" + + def __init__(self, path: str, fault=None) -> None: + self.path = path + self.ns: dict = {} + self.fault = fault + self.calls = 0 + self.resyncs = 0 + + def fs_stat(self, src): + return SimpleNamespace(st_size=Path(self.path).stat().st_size) + + def enter_raw_repl(self, soft_reset=True): + assert soft_reset is False, "a soft reset would close the open file" + self.resyncs += 1 + + def exec(self, command, data_consumer=None): + command = command.replace(repr(self.path)[1:-1], self.path) + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + exec(compile(command, "", "exec"), self.ns) + out = buf.getvalue().replace("\n", "\r\n").encode() + if "print(repr(r(" in command: + self.calls += 1 + if self.fault is not None: + return self.fault(self.calls, out) + return out + + +@unittest.skipUnless(HAVE_MPREMOTE, "mpremote not installed") +class ReadFileCheckedTests(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.path = str(Path(self.tmp.name) / "net.espdl") + self.data = blob() + Path(self.path).write_bytes(self.data) + + def read(self, fault=None, **kw): + board = FaultyBoard(self.path, fault) + return board, rawpaste.read_file_checked(board, self.path, **kw) + + def test_a_clean_board_gives_the_file(self): + board, got = self.read() + self.assertEqual(bytes(got), self.data) + self.assertEqual(board.resyncs, 0) + + def test_zeros_after_the_closing_quote(self): + # What the board sent: the repr, 16 zeros, then the newline. + def fault(n, out): + return out[:-2] + bytes(16) + b"\r\n" if n % 5 == 0 else out + + board, got = self.read(fault) + self.assertEqual(bytes(got), self.data) + + def test_zeros_and_the_next_objects_header_with_its_04(self): + # type pointer 0x406b3704, hash, length, data pointer: the stream then + # carries \x04 early, mpremote ends the reply there and reads the rest + # as the error text. + header = b"\x047k@=|\x00\x00\xc4\x03\x00\x00\xe0\x93\x00H" + + def fault(n, out): + if n == 3: + raise TransportExecError(out[:-2] + bytes(12), header[1:].decode("latin-1") + "\r\n") + return out + + board, got = self.read(fault) + self.assertEqual(bytes(got), self.data) + self.assertEqual(board.resyncs, 1) + + def test_a_reply_cut_short_is_read_again(self): + def fault(n, out): + return out[:40] + b"'\r\n" if n == 4 else out + + board, got = self.read(fault) + self.assertEqual(bytes(got), self.data) + + def test_a_reply_that_never_parses_gives_up_and_says_where(self): + def fault(n, out): + return b"b'\\x00" + bytes(8) if n >= 4 else out + + with self.assertRaises(OSError) as cm: + self.read(fault, attempts=3) + self.assertIn("at byte 768", str(cm.exception)) + + def test_a_board_error_is_not_retried(self): + board = FaultyBoard(self.path) + Path(self.path).unlink() + board.fs_stat = lambda src: SimpleNamespace(st_size=10) + with self.assertRaises(Exception) as cm: + rawpaste.read_file_checked(board, self.path) + self.assertEqual(board.calls, 0) + self.assertIsInstance(cm.exception, (OSError, FileNotFoundError)) + + def test_the_file_is_not_a_multiple_of_the_chunk(self): + Path(self.path).write_bytes(self.data[:1000]) + _, got = self.read() + self.assertEqual(bytes(got), self.data[:1000]) + Path(self.path).write_bytes(b"") + _, got = self.read() + self.assertEqual(bytes(got), b"") + + def test_repr_line_drops_the_padding_only(self): + self.assertEqual(rawpaste._repr_line(b"b'a\\x00'\x00\x00\x00\r\n"), b"b'a\\x00'") + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/agent-guide.md b/docs/agent-guide.md index 912e75b..bd60dce 100644 --- a/docs/agent-guide.md +++ b/docs/agent-guide.md @@ -956,6 +956,7 @@ where it hung. | Two editors on the *same* workspace folder — CLI reaches an unexpected session | One-entry-per-path registry; most recent connect wins — was tracked in [PyDevices/mpftp#21](https://github.com/PyDevices/mpftp/issues/21), fixed: no longer silent. `mpftp status` → check `editor`/`pid`/`updatedAt` on the resolved entry | | `Access is denied` / `transport_dead` after hung `exec`/`run` | Sidecar releases the COM handle automatically (was tracked in [PyDevices/mpftp#3](https://github.com/PyDevices/mpftp/issues/3), fixed via a bounded serial write-timeout); `disconnect` then `resume`/`connect`. If still busy: reload extension window, then replug USB only as last resort | | `busy or locked` on a board's native USB port (`303A`, `239A`, `2E8A`) with no tool holding it | The board's program is stuck in a callback that never returns, so its USB stops answering and Windows refuses the open ([mpftp#79](https://github.com/PyDevices/mpftp/issues/79)). A killed mpftp command lets go of the port within about 10 s; past that, reset the board through its UART port (`hard-reset -d `) or its RESET button | +| `get` failed with "source code string cannot contain null bytes" (older mpftp) | The board's native USB sent a few extra bytes after a printed chunk: zero padding, sometimes the next object's header, which can hold a `\x04` that ends the reply early. The file's own bytes were right. A serial `get`/`cp` now reads each chunk with an explicit `seek`, checks its length and asks again when it is wrong. Seen on the P4 DEV-KIT (ESP32-P4, TinyUSB CDC); a failed chunk that never comes right fails with the byte offset | | `timeout waiting for first EOF` | Board still running (UI loop). Use `run` without `--follow` / `exec --no-follow`, then `interrupt` or `soft-reset` | | Soft-reset left UI dead after deploy | Expected: soft-reset skips `main.py`. Use `soft-reboot` or `hard-reset` to run startup | | Dual USB (UART + native CDC) | `mpftp ports` shows `role` (`repl` vs `cdc_debug`); control on UART, `monitor`/`debug-tee` on CDC |