fix(lf): flush stale input before bulk download (desync -> garbage/DEADBEEF)
A real download came back as BigBuf memory (0xDEADBEEF poison) with a PM3 response magic (0x62334d50) embedded — the classic signature of a desynced read: the stream had stale bytes (a prior partial transaction, or the C client having touched the port), and the bulk reader consumes fixed 544-byte OLD frames that carry no magic to resync on, so once misaligned it stays misaligned and reads memory as "samples". transport.download_bulk now flushes the input first (reset_input_buffer + drain the asyncio StreamReader until an 80ms quiet gap), so the transfer always begins byte- aligned. Drains no-op when disconnected. Test asserts the flush runs before the request. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -278,16 +278,42 @@ class PM3Transport:
|
|||||||
frame = encode_old_frame(cmd, arg0, arg1, arg2, data)
|
frame = encode_old_frame(cmd, arg0, arg1, arg2, data)
|
||||||
await self.send_frame(frame)
|
await self.send_frame(frame)
|
||||||
|
|
||||||
|
async def _drain_input(self, quiet: float = 0.08) -> int:
|
||||||
|
"""Discard buffered/stale bytes (a prior partial transaction, or another process that
|
||||||
|
touched the port) so the next request/response starts byte-aligned. OLD-format frames
|
||||||
|
carry no magic to resync on, so a bulk download MUST begin on a clean stream. Returns the
|
||||||
|
number of bytes dropped."""
|
||||||
|
if not self._reader:
|
||||||
|
return 0
|
||||||
|
try: # clear the OS/pyserial buffer too
|
||||||
|
tr = self._writer.transport
|
||||||
|
if hasattr(tr, "serial"):
|
||||||
|
tr.serial.reset_input_buffer()
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
dropped = 0
|
||||||
|
while True:
|
||||||
|
try:
|
||||||
|
chunk = await asyncio.wait_for(self._reader.read(4096), timeout=quiet)
|
||||||
|
except (asyncio.TimeoutError, Exception):
|
||||||
|
break
|
||||||
|
if not chunk:
|
||||||
|
break
|
||||||
|
dropped += len(chunk)
|
||||||
|
return dropped
|
||||||
|
|
||||||
async def download_bulk(self, cmd: int, start: int, count: int, data_cmd: int,
|
async def download_bulk(self, cmd: int, start: int, count: int, data_cmd: int,
|
||||||
ack_cmd: int, timeout: float = 5.0, on_bytes=None) -> bytes:
|
ack_cmd: int, timeout: float = 5.0, on_bytes=None) -> bytes:
|
||||||
"""Run a bulk BigBuf/EML/SPIFFS download and return the concatenated bytes.
|
"""Run a bulk BigBuf/EML/SPIFFS download and return the concatenated bytes.
|
||||||
|
|
||||||
The firmware answers one download request (a MIX frame) by streaming ``count`` bytes as
|
The firmware answers one download request (a MIX frame) by streaming ``count`` bytes as
|
||||||
a run of *OLD*-format ``data_cmd`` frames (no magic — that's why the plain NG reader
|
a run of *OLD*-format ``data_cmd`` frames (no magic — that's why the plain NG reader
|
||||||
can't see them and the naive download hangs), then a final ``ack_cmd`` frame. We read
|
can't see them and the naive download hangs), then a final ``ack_cmd`` frame. We flush
|
||||||
with the NG/OLD-agnostic reader until the ACK, or until we've collected ``count`` bytes
|
any stale input first (OLD frames can't resync mid-stream), then read with the NG/OLD-
|
||||||
(draining the trailing ACK best-effort so it can't poison the next command)."""
|
agnostic reader until the ACK, or until we've collected ``count`` bytes (draining the
|
||||||
|
trailing ACK best-effort so it can't poison the next command)."""
|
||||||
async with self._lock:
|
async with self._lock:
|
||||||
|
await self._drain_input() # start byte-aligned — no stale frames
|
||||||
await self.send_frame(encode_mix_frame(cmd, start, count, 0))
|
await self.send_frame(encode_mix_frame(cmd, start, count, 0))
|
||||||
buf = bytearray()
|
buf = bytearray()
|
||||||
while True:
|
while True:
|
||||||
|
|||||||
@@ -133,3 +133,21 @@ def test_download_bulk_uses_oldarg_length_not_padding():
|
|||||||
t = _transport_with([_bigbuf_frame(0, b"\xAA\xBB\xCC"), _ack()])
|
t = _transport_with([_bigbuf_frame(0, b"\xAA\xBB\xCC"), _ack()])
|
||||||
got = _run(t.download_bulk(Cmd.DOWNLOAD_BIGBUF, 0, 3, Cmd.DOWNLOADED_BIGBUF, Cmd.ACK))
|
got = _run(t.download_bulk(Cmd.DOWNLOAD_BIGBUF, 0, 3, Cmd.DOWNLOADED_BIGBUF, Cmd.ACK))
|
||||||
assert got == b"\xAA\xBB\xCC" # trimmed to oldarg length, not 512 of padding
|
assert got == b"\xAA\xBB\xCC" # trimmed to oldarg length, not 512 of padding
|
||||||
|
|
||||||
|
|
||||||
|
def test_download_bulk_drains_stale_input_first():
|
||||||
|
# a bulk download must flush stale bytes before sending, so OLD frames start byte-aligned
|
||||||
|
payload = bytes(range(100))
|
||||||
|
t = _transport_with([_bigbuf_frame(0, payload), _ack()])
|
||||||
|
calls = []
|
||||||
|
async def fake_drain(quiet=0.08):
|
||||||
|
calls.append("drain")
|
||||||
|
return 7
|
||||||
|
orig_send = t.send_frame
|
||||||
|
async def tracking_send(frame):
|
||||||
|
calls.append("send")
|
||||||
|
return await orig_send(frame)
|
||||||
|
t._drain_input = fake_drain
|
||||||
|
t.send_frame = tracking_send
|
||||||
|
_run(t.download_bulk(Cmd.DOWNLOAD_BIGBUF, 0, 100, Cmd.DOWNLOADED_BIGBUF, Cmd.ACK))
|
||||||
|
assert calls[0] == "drain" and calls[1] == "send" # drained BEFORE the request went out
|
||||||
|
|||||||
Reference in New Issue
Block a user