Skip to content

Raw serial transport has no write pacing; an unpaced burst overruns the server UART #131

Description

@JPHutchins

Warning

LLM Disclosure

This issue was filed by claude-opus-5[1m] on behalf of @JPHutchins, who asked that the findings from the smpclient screaming-goblin kickoff session be recorded as issues for durable context. #128 fixed the test-harness half of this and deliberately left the production question open for the breaking window.

SMPSerialRawTransport.send() writes an entire SMP message in one burst. If the client can put bytes on the link faster than the server drains its UART RX pool, the overflow is silently dropped — and in the unframed raw protocol nothing detects or recovers from it: the server waits forever for the tail of a header.length it will never receive.

A real UART at a real baud rate paces the client for free, which is what hides this. Two common cases do not:

  • USB CDC ACM, where baudrate is ignored entirely and the host writes at bus speed.
  • A socket chardev (an emulator's serial), which has no pacing at all.

#128 demonstrated the mechanism concretely on the second one.

Evidence from #128

test_upload_to_mcuboot_recovery[mps2_an385.serial_recovery_raw-raw] failed ~1 in 30 with a perfectly bimodal signature — the burst either landed intact or was truncated and ate the whole request timeout, with nothing in between:

Run Result Worst request
1–7 PASS 0.276 – 0.309 s
9 FAIL 15.001 s

Interleaving a wall-clock pause between sub-chunks of the write fixed it outright; an asyncio.sleep(0) yield did not, because the server needs real time to drain, not a turn of the event loop:

Variant Failures (40 runs)
unpaced 1/30, then 2/40
64 B chunks + asyncio.sleep(0) 3/40 — no better
64 B chunks + 1 ms pause 0/40

ServerFixture.bursty_fragment_drop already documented this class of failure for native_sim PTY serial ("no baud pacing, so a >2-fragment message written all at once is dropped"); #128 corrected its claim that socket fixtures are unaffected.

The production question

Should SMPSerialRawTransport offer explicit write pacing? #128 kept the fix in the test harness precisely so this stays a deliberate API decision rather than a side effect of a flake fix.

Sketch, if yes

A sum type, mirroring the existing FragmentationStrategy (Auto / BufferSize) precedent in encoded.py, defaulting to unpaced so nothing gets slower by accident:

WritePacing = Unpaced | Paced

class Unpaced(NamedTuple): ...

class Paced(NamedTuple):
    chunk_bytes: int = 64
    pause_s: float = 0.001

Expressing it as a baud rate instead (Paced(baudrate=115200)sleep(len(chunk) * 10 / baudrate)) is more physically meaningful and self-documenting, but measured ~5x more delay than was actually needed to fix #128, so it should not be the default.

Important scoping lesson from #128: pacing must not be applied to SMPSerialTransport. It already writes one small base64 line packet at a time, and pacing it further measurably destabilised qemu_cortex_m0 (2/15 failures against 0/15 unpaced).

Note this is a lossy-link problem, so it also interacts with #56 (retransmit), #129 (the transport cannot resync after a truncated message), and #130 (a short write truncates one too).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions