From 423608ccd1ba23c0524e5f8858ef5e0a091d4131 Mon Sep 17 00:00:00 2001 From: serversdown Date: Fri, 2 Oct 2026 14:14:02 -0400 Subject: [PATCH] perf(micromate): 16 KB per request, not 1024 -- 14x fewer round trips The chunk-size ceiling, measured on UM20147 with every size checked byte-for-byte against a known-good download: 1024 / 2048 / 4096 / 8192 / 16384 -> served in full 32768 / 65535 -> SILENTLY CLAMPED to 16384 The clamp is the important part: a 32,768 B request returns 16,384 B of perfectly good data and no error. Nothing in the response says it was truncated; the length is the only signal. THAT DICTATES HOW THE LOOP MUST BE WRITTEN. read_event_file() now tracks its offset by BYTES RECEIVED rather than striding by chunk index. A fixed stride would either fail on a clamp or skip the bytes it never collected; tracking what actually arrived makes a clamp cost one extra request, and makes the loop self-correcting against any short response -- precisely the failure mode that has bitten the Series III side repeatedly. Consequence of that change, recorded because it reverses an earlier decision: a short chunk is NO LONGER AN ERROR. It used to raise, on the principle that a silently short event is this codebase's recurring bug. But the device returns short legitimately, and the real protection is the offset arithmetic plus the final total-length check -- which still raises on a genuinely truncated event. The test was rewritten rather than deleted, and says why. CHUNK_SIZE = 16384. For UM20147's events: 4,796 B goes 5 requests -> 1, 30,230 B goes 30 -> 2, and 72,560 B goes 71 -> 5. Over cellular at ~0.65 s per round trip that is ~46 s -> ~3.2 s on the large one. Measured on one unit over USB, so THOR_CHUNK_SIZE = 1024 stays available and the replay tests pin it -- reproducing THOR's exact traffic is one argument away, and client.download_event()/get_event() take chunk_size for a link where large responses are not surviving. Full suite: 472 passed, 16 pre-existing failures unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ru8Lg9HkkYvX9VWWo65SmL --- docs/micromate_protocol_reference.md | 44 ++++++++++-- micromate/client.py | 15 ++-- micromate/protocol.py | 72 ++++++++++++++----- tests/test_micromate_events.py | 8 ++- tests/test_micromate_protocol.py | 101 +++++++++++++++++++++++---- 5 files changed, 194 insertions(+), 46 deletions(-) diff --git a/docs/micromate_protocol_reference.md b/docs/micromate_protocol_reference.md index 7363e38..34ad37a 100644 --- a/docs/micromate_protocol_reference.md +++ b/docs/micromate_protocol_reference.md @@ -732,14 +732,44 @@ THOR's chunk size. The offset field is a **uint16**, so the structural ceiling i **65,535 bytes per request** — which would make that same event **2 requests, ~1.3 seconds**. -⚠ The *device's* ceiling is not yet known, only that it is at least 4,380. It may -be bounded by an internal buffer well below 65,535. `scratch/mm_stream_probe.py` -walks ascending sizes and checks each against a known-good chunk-loop download, -so a pass means byte-identical output rather than a plausible length. +#### ✅ The ceiling is 16,384 bytes, and over it the device CLAMPS SILENTLY -⚠ `micromate/protocol.py` still uses **1024** — THOR's value, the one with -captures behind it. Raising it is a one-constant change once the ceiling is -measured, and it should not be raised on inference. +Measured on UM20147, each size checked byte-for-byte against a known-good +download: + +| requested | served | | +|---|---|---| +| 1,024 | 1,024 | ✅ | +| 2,048 | 2,048 | ✅ | +| 4,096 | 4,096 | ✅ | +| 8,192 | 8,192 | ✅ | +| **16,384** | **16,384** | ✅ | +| 32,768 | **16,384** | ⚠ clamped | +| 65,535 | **16,384** | ⚠ clamped | + +⚠ **The clamp is silent and the bytes are correct.** A 32,768-byte request +returns 16,384 bytes of perfectly good data and no error. There is nothing in the +response to say it was truncated — the length is the only signal. + +**That is what dictates how the download loop must be written.** A loop striding +by a fixed chunk size would either fail on a clamp or, worse, skip the bytes it +never collected. `read_event_file()` therefore tracks its offset by **bytes +received**, which makes a clamp cost one extra request rather than corrupting the +file — and makes the loop self-correcting against any short response, which is +precisely the failure mode that has bitten the Series III side repeatedly. + +`CHUNK_SIZE` is now **16,384**. For UM20147's events: + +| event | THOR's 1024 | 16,384 | cellular | +|---|---|---|---| +| 4,796 B | 5 requests | **1** | ~3.3 s → ~0.7 s | +| 30,230 B | 30 | **2** | ~20 s → ~1.3 s | +| 72,560 B | 71 | **5** | ~46 s → ~3.2 s | + +⚠ **Measured on one unit, over USB.** `THOR_CHUNK_SIZE = 1024` remains available +and the replay tests pin it, so reproducing THOR's exact traffic is still one +argument away. A unit that clamps lower than 16,384 costs extra requests, not a +failure. **Implement THOR's chunked form.** It is verified byte-exact across six events and five distinct sizes, and it is what the firmware runs every day. diff --git a/micromate/client.py b/micromate/client.py index 030e167..67f47e3 100644 --- a/micromate/client.py +++ b/micromate/client.py @@ -474,16 +474,23 @@ class MicromateClient: ref.peak_vector_sum_ips = struct.unpack( ">f", c[tran - _REC_PVS_BACK: tran - _REC_PVS_BACK + 4])[0] - def download_event(self, ref: MicromateEventRef) -> bytes: + def download_event(self, ref: MicromateEventRef, *, + chunk_size: Optional[int] = None) -> bytes: """The raw `.IDFW`/`.IDFH` bytes, exactly as THOR would have stored them. Feeds `micromate.idf_file.read_idf_file()` and `/db/import/idf_file` unchanged — no new codec work is needed for a directly downloaded event. + + `chunk_size` defaults to the measured device ceiling (16,384 B), which is + 16x THOR's 1024 and therefore ~14x fewer round trips on a large event. + Pass `THOR_CHUNK_SIZE` to reproduce THOR's wire traffic exactly, or a + smaller value on a link where big responses are not surviving. """ - return self._proto.read_event_file(ref.key, ref.size) + kw = {} if chunk_size is None else {"chunk_size": chunk_size} + return self._proto.read_event_file(ref.key, ref.size, **kw) def get_event(self, ref: MicromateEventRef, *, verify: bool = True, - tolerance: float = 0.01): + tolerance: float = 0.01, chunk_size: Optional[int] = None): """Download and decode one event. Returns the codec's `IdfReadResult`. Needs `ref.record_type`, since @@ -510,7 +517,7 @@ class MicromateClient: f"event {ref.key_hex}: record_type is unknown, so the codec " f"cannot be dispatched. Use list_events(with_records=True)." ) - blob = self.download_event(ref) + blob = self.download_event(ref, chunk_size=chunk_size) import tempfile from .idf_file import read_idf_file diff --git a/micromate/protocol.py b/micromate/protocol.py index 0c404b1..42ca96a 100644 --- a/micromate/protocol.py +++ b/micromate/protocol.py @@ -106,8 +106,25 @@ OBSERVED_DATA_LEN = { # bulk stream; here THOR sends it on every chain read, browse or download. EVENT_TOKEN = 0xFE -# `SUB 0x5A` chunk size, in bytes of file payload per response. -CHUNK_SIZE = 1024 +# `SUB 0x5A` request size, in bytes of file payload per response. +# +# ⚠ **16,384, not THOR's 1024.** Measured on UM20147 (2026-10-02): requests of +# 1024 / 2048 / 4096 / 8192 / 16384 all returned byte-identical data against a +# known-good download, and anything larger is **silently clamped to 16,384** — +# correct bytes, short length, no error. +# +# This matters because round trips dominate a cellular download at ~0.65 s each +# regardless of payload. A 72,560-byte event is 71 requests (~46 s) at THOR's +# size and **5 requests (~3.2 s)** at this one. +# +# ⚠ Measured on ONE unit, over USB. The loop below is driven by bytes received +# rather than chunk index, so a unit that clamps lower simply takes more +# requests instead of failing — which is what makes raising this safe. +CHUNK_SIZE = 16384 + +# THOR's value. Pass `chunk_size=THOR_CHUNK_SIZE` to reproduce its wire traffic +# exactly; the replay test does. +THOR_CHUNK_SIZE = 1024 # Every `0x5A` response prefixes the file bytes with 11 bytes of header. _CHUNK_PREFIX = 11 @@ -373,14 +390,15 @@ class MicromateProtocol: # ── Bulk download ───────────────────────────────────────────────────────── - def read_event_file(self, key4: bytes, size: int) -> bytes: + def read_event_file(self, key4: bytes, size: int, *, + chunk_size: int = CHUNK_SIZE) -> bytes: """`0x5A` → `0xA5`. The `.IDFW`/`.IDFH` file, byte for byte. `size` is the 4 bytes after the key in the `1E`/`1F` response. Returns exactly that many bytes, or raises `ShortRead`. - A bounded chunk walk — `ceil(size / 1024)` requests, each asking for - `min(1024, remaining)` bytes: + A bounded walk — `ceil(size / chunk_size)` requests, each asking for + `min(chunk_size, remaining)` bytes: offset = the byte count wanted (NOT an address) params = the key on chunk 0, then a uint16 BE byte offset @@ -401,36 +419,54 @@ class MicromateProtocol: if size <= 0: raise ValueError(f"size must be positive, got {size}") + if chunk_size < 1: + raise ValueError(f"chunk_size must be positive, got {chunk_size}") + + # ⚠ Driven by BYTES RECEIVED, not by chunk index. + # + # The device silently clamps an over-large request: ask for 32,768 and it + # returns exactly 16,384 — correct bytes, short length, no error. A loop + # that strides by a fixed chunk size would either fail on that or, worse, + # skip the bytes it never collected. Tracking the offset by what actually + # arrived makes the clamp a non-event: it just takes another request. + # + # That also makes this self-correcting against any short response, which + # is the failure mode this codebase has been bitten by repeatedly on the + # Series III side — there, a short read surfaced as a silently truncated + # channel. out = bytearray() - n_chunks = math.ceil(size / CHUNK_SIZE) - for i in range(n_chunks): - want = min(CHUNK_SIZE, size - i * CHUNK_SIZE) + requests = 0 + while len(out) < size: + want = min(chunk_size, size - len(out)) data = self._read( SUB_BULK_DOWNLOAD, offset=want, - params=chunk_params(key4, i * CHUNK_SIZE), + params=chunk_params(key4, len(out)), ) + requests += 1 if len(data) < _CHUNK_PREFIX: raise ShortRead( - f"chunk {i + 1}/{n_chunks} of {key4.hex()}: " - f"{len(data)} B is too short to hold a chunk header" + f"{key4.hex()} at offset {len(out)}: {len(data)} B is too " + f"short to hold a chunk header" ) body = data[_CHUNK_PREFIX:] - if len(body) != want: - # Worth being loud: a silently short event is the failure mode - # this project has been bitten by repeatedly on the Series III - # side, and here the expected length is known up front. + if not body: + # No progress at all — continuing would spin forever. raise ShortRead( - f"chunk {i + 1}/{n_chunks} of {key4.hex()}: asked for " - f"{want} B, got {len(body)}" + f"{key4.hex()} at offset {len(out)}: asked for {want} B and " + f"got none; {len(out)} of {size} B assembled" ) + if len(body) < want: + log.debug("%s: asked %d B at offset %d, served %d — clamped", + key4.hex(), want, len(out), len(body)) out += body if len(out) != size: raise ShortRead( f"{key4.hex()}: assembled {len(out)} B, device promised {size}" ) - log.debug("downloaded %s: %d B in %d chunks", key4.hex(), len(out), n_chunks) + log.debug("downloaded %s: %d B in %d request(s)", + key4.hex(), len(out), requests) return bytes(out) # ── Plumbing ────────────────────────────────────────────────────────────── diff --git a/tests/test_micromate_events.py b/tests/test_micromate_events.py index 7423457..883ed26 100644 --- a/tests/test_micromate_events.py +++ b/tests/test_micromate_events.py @@ -293,7 +293,9 @@ def test_replaying_thors_session_reproduces_every_byte_and_decodes_every_event() decoded, checked = 0, 0 for ref in mm.iter_events(): - result = mm.get_event(ref) # verify=True by default + # Pin THOR's chunk size: this test asserts byte-identity with + # THOR's frames, and our default is 16x larger. + result = mm.get_event(ref, chunk_size=1024) decoded += 1 assert ref.serial == "UM12947" assert ref.filename and ref.filename.endswith(ref.suffix) @@ -334,10 +336,10 @@ def test_a_corrupted_stored_peak_is_caught_by_the_self_check(): for ref in mm.iter_events(): if ref.is_histogram: - mm.download_event(ref) # keep the replay in step + mm.download_event(ref, chunk_size=1024) # keep the replay in step continue ref.peak_vector_sum_ips *= 1.5 # as a bad decode would look with pytest.raises(DecodeMismatch, match="decode is suspect"): - mm.get_event(ref) + mm.get_event(ref, chunk_size=1024) return pytest.fail("no waveform event found in the capture") diff --git a/tests/test_micromate_protocol.py b/tests/test_micromate_protocol.py index 00e81c7..0e652e7 100644 --- a/tests/test_micromate_protocol.py +++ b/tests/test_micromate_protocol.py @@ -174,12 +174,13 @@ def test_download_reproduces_thors_chunk_sequence_byte_for_byte(): payload = payload[:SIZE_4A81] responses = [] for i in range(4): - want = min(P.CHUNK_SIZE, SIZE_4A81 - i * P.CHUNK_SIZE) - body = payload[i * P.CHUNK_SIZE: i * P.CHUNK_SIZE + want] + want = min(P.THOR_CHUNK_SIZE, SIZE_4A81 - i * P.THOR_CHUNK_SIZE) + body = payload[i * P.THOR_CHUNK_SIZE: i * P.THOR_CHUNK_SIZE + want] responses.append(frame(0xA5, bytes(11) + body, page=want // 256)) p, t = proto(responses) - got = p.read_event_file(bytes.fromhex("055d4a81"), SIZE_4A81) + got = p.read_event_file(bytes.fromhex("055d4a81"), SIZE_4A81, + chunk_size=P.THOR_CHUNK_SIZE) assert t.written == REQ_CHUNKS_4A81 assert got == payload @@ -198,11 +199,12 @@ def test_chunk_count_and_final_offset(size, n_chunks, last_offset): """The first six rows are the six bench events, with THOR's real offsets.""" responses = [] for i in range(n_chunks): - want = min(P.CHUNK_SIZE, size - i * P.CHUNK_SIZE) + want = min(P.THOR_CHUNK_SIZE, size - i * P.THOR_CHUNK_SIZE) responses.append(frame(0xA5, bytes(11) + bytes(want))) p, t = proto(responses) - p.read_event_file(bytes.fromhex("055d4a81"), size) + p.read_event_file(bytes.fromhex("055d4a81"), size, + chunk_size=P.THOR_CHUNK_SIZE) assert len(t.written) == n_chunks # offset is payload[4:5] of the request; recover it from the built frame @@ -214,17 +216,29 @@ def test_chunk_count_and_final_offset(size, n_chunks, last_offset): ) -def test_a_short_chunk_raises_rather_than_truncating(): - """A silently short event is the failure mode this codebase keeps hitting.""" - p, _ = proto([frame(0xA5, bytes(11) + bytes(900))]) # asked for 1024 - with pytest.raises(ShortRead, match="asked for 1024 B, got 900"): - p.read_event_file(bytes.fromhex("055d4a81"), 1024) +def test_a_short_chunk_is_absorbed_and_the_remainder_refetched(): + """⚠ Changed 2026-10-02: a short chunk is no longer an error. + + It used to raise, on the principle that a silently short event is the failure + mode this codebase keeps hitting. But the device *legitimately* returns + short — it clamps an over-large request to 16,384 B without saying so — and + the real protection is tracking the offset by bytes received, which makes a + short response cost one extra request instead of corrupting the file. The + total length is still checked, so a genuinely truncated event still raises. + """ + p, t = proto([frame(0xA5, bytes(11) + b"\xaa" * 900), + frame(0xA5, bytes(11) + b"\xbb" * 124)]) + got = p.read_event_file(bytes.fromhex("055d4a81"), 1024, + chunk_size=P.THOR_CHUNK_SIZE) + assert got == b"\xaa" * 900 + b"\xbb" * 124 + assert len(t.written) == 2, "the 124 B remainder was refetched" def test_a_chunk_too_short_to_hold_its_header_raises(): p, _ = proto([frame(0xA5, bytes(4))]) with pytest.raises(ShortRead, match="too short to hold a chunk header"): - p.read_event_file(bytes.fromhex("055d4a81"), 1024) + p.read_event_file(bytes.fromhex("055d4a81"), 1024, + chunk_size=P.THOR_CHUNK_SIZE) def test_download_rejects_a_nonsense_size(): @@ -391,7 +405,8 @@ def test_every_captured_download_frame_is_one_we_would_have_sent(): for e in events: p, t = proto([bytes([STX]) + stuff(r + bytes([checksum(r)])) + bytes([ETX]) for r in e["rsps"]]) - got = p.read_event_file(e["key"], e["size"]) + got = p.read_event_file(e["key"], e["size"], + chunk_size=P.THOR_CHUNK_SIZE) assert t.written == e["reqs"], ( f"event {e['key'].hex()}: our {len(t.written)} frames differ from " f"THOR's {len(e['reqs'])}" @@ -452,14 +467,72 @@ def test_a_72kb_event_downloads_in_71_chunks(): n = 71 responses = [] for i in range(n): - want = min(P.CHUNK_SIZE, size - i * P.CHUNK_SIZE) + want = min(P.THOR_CHUNK_SIZE, size - i * P.THOR_CHUNK_SIZE) responses.append(frame(0xA5, bytes(11) + bytes(want))) p, t = proto(responses) - got = p.read_event_file(bytes.fromhex("055d4a83"), size) + got = p.read_event_file(bytes.fromhex("055d4a83"), size, + chunk_size=P.THOR_CHUNK_SIZE) assert len(got) == size assert len(t.written) == n # Chunk 64 is the first past the old cap; its params must carry the 0x01. wire = t.written[64] assert bytes.fromhex("000100") in wire, "the carry into params[1] is on the wire" + + +# ── The 16 KB ceiling, and the silent clamp ─────────────────────────────────── + +def test_default_chunk_size_is_the_measured_ceiling_not_thors(): + assert P.CHUNK_SIZE == 16384 + assert P.THOR_CHUNK_SIZE == 1024 + + +def test_a_silently_clamped_response_is_absorbed_not_failed(): + """⚠ Ask for more than the device serves and it CLAMPS — silently. + + Measured on UM20147: a 32,768 B request returns exactly 16,384 B of correct + data, no error. A loop striding by a fixed chunk size would either fail on + that or skip the bytes it never collected. Driving by bytes received makes + it a non-event: one more request. + + Here a 20,000 B event is fetched with chunk_size=16384 against a device + pretending to clamp at 8192, so the walk must take 3 requests at + offsets 0 / 8192 / 16384. + """ + size, clamp = 20000, 8192 + payload = bytes(range(256)) * 100 + payload = payload[:size] + + served, responses = 0, [] + while served < size: + n = min(clamp, size - served) + responses.append(frame(0xA5, bytes(11) + payload[served:served + n])) + served += n + + p, t = proto(responses) + got = p.read_event_file(bytes.fromhex("055d4a83"), size, chunk_size=16384) + + assert got == payload + assert len(t.written) == 3, "two clamped requests plus the remainder" + + +def test_a_chunk_that_returns_nothing_raises_rather_than_spinning(): + """A byte-driven loop must not loop forever on zero progress.""" + p, _ = proto([frame(0xA5, bytes(11))] * 4) + with pytest.raises(ShortRead, match="got none"): + p.read_event_file(bytes.fromhex("055d4a83"), 5000) + + +def test_the_72kb_event_now_takes_5_requests_not_71(): + size = 72560 + served, responses = 0, [] + while served < size: + n = min(P.CHUNK_SIZE, size - served) + responses.append(frame(0xA5, bytes(11) + bytes(n))) + served += n + + p, t = proto(responses) + got = p.read_event_file(bytes.fromhex("055d4a83"), size) + assert len(got) == size + assert len(t.written) == 5, "14x fewer round trips than THOR's 71"