From c5e4d5451297b940bf5ed15704134de371e32878 Mon Sep 17 00:00:00 2001 From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:53:17 -0500 Subject: [PATCH] Read files from a board in 2 KB base64 chunks (#133) --- CHANGELOG.md | 1 + cli/src/mpftp/rawpaste.py | 39 +++++++++++++++++++---- cli/tests/test_read_file_checked.py | 48 ++++++++++++++++++++++++----- 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2482f6..3e3ec47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Unreleased - `run --follow --timeout SECONDS` waits longer for a script that works quietly, and `--timeout 0` waits until it ends. The default is still 10 s, and the timeout error now names the option (#172) +- Downloads from a board are several times faster: a file is read in larger chunks, sent as base64 when the board has `binascii` (#133) ## v0.1.5rc1 (2026-10-28) diff --git a/cli/src/mpftp/rawpaste.py b/cli/src/mpftp/rawpaste.py index 0558153..fa2a984 100644 --- a/cli/src/mpftp/rawpaste.py +++ b/cli/src/mpftp/rawpaste.py @@ -27,6 +27,7 @@ from __future__ import annotations +import re import struct from typing import Any, Callable, Optional @@ -169,6 +170,9 @@ def read_until( return data +_B64_LINE = re.compile(r"[A-Za-z0-9+/]*={0,2}") + + 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 @@ -180,7 +184,7 @@ def _repr_line(out: bytes) -> bytes: def read_file_checked( transport: Any, src: str, - chunk_size: int = 256, + chunk_size: int = 2048, progress_callback: Optional[Callable[[int, int], None]] = None, attempts: int = 8, ) -> bytearray: @@ -200,6 +204,7 @@ def read_file_checked( is read again, up to ``attempts`` times. """ import ast + import binascii size = None for _ in range(attempts): @@ -217,6 +222,14 @@ def read_file_checked( contents = bytearray() try: transport.exec("f=open('%s','rb')\nr=f.read\ns=f.seek" % src) + try: + transport.exec("from binascii import b2a_base64 as e") + b64 = True + except Exception: # noqa: BLE001 - no binascii: repr, as before + b64 = False + # Without base64 every byte can come back as four characters of + # repr, so keep the old small chunk. + chunk_size = min(chunk_size, 256) except Exception as e: # noqa: BLE001 - same conversion mpremote does raise _convert_fs_error(transport, e, src) from None try: @@ -226,10 +239,19 @@ def read_file_checked( 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")) + if b64: + out = transport.exec( + "s(%d)\nprint(e(r(%d)).decode())" % (len(contents), want) + ) + text = _repr_line(out).decode("ascii") + if not _B64_LINE.fullmatch(text): + raise ValueError("not base64: %r" % text[:40]) + value = binascii.a2b_base64(text) + else: + 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) @@ -239,6 +261,11 @@ def read_file_checked( # (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): + if "MemoryError" in str(getattr(e, "error_output", "")) and want > 256: + # A small heap: ask for less per round trip. + chunk_size = max(256, want // 2) + want = chunk_size + continue raise _convert_fs_error(transport, e, src) from None last = e _resync(transport) @@ -411,7 +438,7 @@ def read_until( assert data_consumer is None or len(ending) == 1 return self._serial.read_until(ending, timeout, timeout_overall, data_consumer) - def fs_readfile(self, src, chunk_size=256, progress_callback=None): + def fs_readfile(self, src, chunk_size=2048, progress_callback=None): return read_file_checked(self, src, chunk_size, progress_callback) _TRANSPORT_CLASS = PacedSerialTransport diff --git a/cli/tests/test_read_file_checked.py b/cli/tests/test_read_file_checked.py index 97a98a5..eb65494 100644 --- a/cli/tests/test_read_file_checked.py +++ b/cli/tests/test_read_file_checked.py @@ -44,8 +44,11 @@ def blob() -> bytes: class FaultyBoard: """Runs the commands under CPython; ``fault(n, out)`` may change reply n.""" - def __init__(self, path: str, fault=None) -> None: + def __init__(self, path: str, fault=None, no_b64=False, max_read=None) -> None: self.path = path + self.no_b64 = no_b64 + self.max_read = max_read + self.wants: list = [] self.ns: dict = {} self.fault = fault self.calls = 0 @@ -60,11 +63,18 @@ def enter_raw_repl(self, soft_reset=True): def exec(self, command, data_consumer=None): command = command.replace(repr(self.path)[1:-1], self.path) + if self.no_b64 and "binascii" in command: + raise TransportExecError(1, "Traceback\r\nImportError: no module named 'binascii'\r\n") + if "print(" in command and "r(" in command: + want = int(command.rsplit("r(", 1)[1].split(")", 1)[0]) + self.wants.append(want) + if self.max_read is not None and want > self.max_read: + raise TransportExecError(1, "Traceback\r\nMemoryError: memory allocation failed\r\n") 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: + if "print(repr(r(" in command or "print(e(r(" in command: self.calls += 1 if self.fault is not None: return self.fault(self.calls, out) @@ -80,8 +90,8 @@ def setUp(self) -> None: self.data = blob() Path(self.path).write_bytes(self.data) - def read(self, fault=None, **kw): - board = FaultyBoard(self.path, fault) + def read(self, fault=None, board_kw=None, **kw): + board = FaultyBoard(self.path, fault, **(board_kw or {})) return board, rawpaste.read_file_checked(board, self.path, **kw) def test_a_clean_board_gives_the_file(self): @@ -94,7 +104,7 @@ def test_zeros_after_the_closing_quote(self): def fault(n, out): return out[:-2] + bytes(16) + b"\r\n" if n % 5 == 0 else out - board, got = self.read(fault) + board, got = self.read(fault, chunk_size=256) self.assertEqual(bytes(got), self.data) def test_zeros_and_the_next_objects_header_with_its_04(self): @@ -108,7 +118,7 @@ def fault(n, out): raise TransportExecError(out[:-2] + bytes(12), header[1:].decode("latin-1") + "\r\n") return out - board, got = self.read(fault) + board, got = self.read(fault, chunk_size=256) self.assertEqual(bytes(got), self.data) self.assertEqual(board.resyncs, 1) @@ -116,7 +126,7 @@ 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) + board, got = self.read(fault, chunk_size=256) self.assertEqual(bytes(got), self.data) def test_a_reply_that_never_parses_gives_up_and_says_where(self): @@ -124,7 +134,7 @@ 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.read(fault, attempts=3, chunk_size=256) self.assertIn("at byte 768", str(cm.exception)) def test_a_board_error_is_not_retried(self): @@ -144,6 +154,28 @@ def test_the_file_is_not_a_multiple_of_the_chunk(self): _, got = self.read() self.assertEqual(bytes(got), b"") + def test_base64_in_big_chunks_is_few_round_trips(self): + board, got = self.read() + self.assertEqual(bytes(got), self.data) + self.assertEqual(len(board.wants), 3) # 4958 bytes at 2048 a time + + def test_a_board_without_binascii_reads_repr_in_small_chunks(self): + board, got = self.read(board_kw={"no_b64": True}) + self.assertEqual(bytes(got), self.data) + self.assertEqual(max(board.wants), 256) + + def test_a_small_heap_halves_the_chunk(self): + board, got = self.read(board_kw={"max_read": 600}) + self.assertEqual(bytes(got), self.data) + self.assertLessEqual(max(board.wants[2:]), 512) + + def test_base64_noise_is_read_again(self): + def fault(n, out): + return b"!!notbase64!!\r\n" if n == 2 else out + + board, got = self.read(fault) + self.assertEqual(bytes(got), self.data) + 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'")