test(integration): pace raw socket-chardev writes to stop UART RX overrun

[JPHutchins]

Warning

LLM Disclosure

This PR was authored by claude-opus-5[1m] on behalf of @JPHutchins, who saw the integration job fail on #127, asked whether the flake could be fixed, directed that the fix land as its own PR, and — after the first version of it caused a regression — pushed back that a structural fix was wanted over monkey-patching with side effects. The pacing is confined to the test harness (src/ untouched) at their direction.

Caution

Force-pushed. The first version of this PR paced every socket chardev and regressed qemu_cortex_m0.serial_buf256. It has been amended into a single commit that scopes the pacing to the raw transport. See The regression, and the scoping fix below.

The flake

test_upload_to_mcuboot_recovery[mps2_an385.serial_recovery_raw-raw] failed the integration job on #127 — a change touching none of this — and reproduces locally at ~1 in 30.

What was actually happening

The raw transport writes a 1024 B SMP chunk as a single burst. While the server is still busy flashing the previous chunk, its UART RX pool overruns and the overflow is silently dropped. Nothing recovers: the raw protocol is [8-byte header][header.length bytes] with no delimiter and no CRC, so the server waits forever for a message whose tail never arrived, and the client burns its entire 15 s request timeout.

That predicts a bimodal latency distribution — and measuring one confirms it. Worst single request per run:

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

Nothing in between. A 50× gap with no tail means this is not "qemu is slow" — raising the timeout would only make the test hang longer before failing identically.

ServerFixture.bursty_fragment_drop already documents this mechanism for native_sim PTY serial:

native_sim PTY serial has no baud pacing, so a >2-fragment message written all at once is dropped unless the build enlarged its UART RX pool (bigrx). Emulated (socket) and UDP fixtures are unaffected.

That last sentence is what's wrong. mps2_an385 is affected too — just rarely, because it also needs the server to be mid-flash-write.

The fix

A real UART paces the client: bytes leave at the baud rate, so the server's RX pool drains about as fast as it fills. A socket chardev has no pacing at all, so _PacedSocketChardev supplies it.

The pause has to be wall-clock. Measured, 40 runs each:

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

An event-loop yield does nothing: the guest needs real time on a real CPU, not a turn of the event loop. The chunk size is incidental; the interleaved delay is the whole fix.

The regression, and the scoping fix

The first version put the pacing in the shared _connect_socket_chardev, so it also hit SMPSerialTransport. That traded one flake for another:

Test	Paced everything	Unpaced
test_max_payload_roundtrip[qemu_cortex_m0.serial_buf256]	2/15 fail	0/15 fail

max_reliable_line_packets already flags that 16 KB target as fragile once a transaction stays open too long, and the encoded transport writes one small base64 line packet at a time — it paces itself well enough.

So the chardev class became a parameter of _connect_socket_chardev, and the caller names it. The encoded transport is now unaffected by construction, not by exclusion:

# QemuSocketSerialTransport  (encoded) — default, unpaced
await _connect_socket_chardev(self, self._url, timeout_s)

# QemuSocketSerialRawTransport (raw)  — opts in
await _connect_socket_chardev(self, self._url, timeout_s, _PacedSocketChardev)

Splitting the chardev into _SocketChardev / _PacedSocketChardev also retires a monkeypatch — out_waiting becomes a plain, type-checked class attribute:

-        conn = pyserial.serial_for_url(url, timeout=0, write_timeout=0)
+        conn = chardev(url, timeout=0, write_timeout=_WRITE_TIMEOUT_S)
         # `_conn` is `Final` on the base class; replace it for the socket backend.
         object.__setattr__(transport, "_conn", conn)
-        # A socket chardev has no host-side TX buffer; pyserial omits `out_waiting` for it.
-        # Supply 0 so the inherited `send`'s `_drain_tx` poll is a no-op (nothing to drain).
-        object.__setattr__(conn, "out_waiting", 0)

A latent bug found on the way, deliberately not claimed as the cause

pyserial silently short-writes when write_timeout=0

I first blamed this, and I was wrong — fixing it alone still failed 2/40.

pyserial reads a zero write_timeout as "non-blocking". Its socket write() then issues one socket.send() and returns that count without looping:

n = self._socket.send(d)
if timeout.is_non_blocking:
    # Zero timeout indicates non-blocking - simply return the
    # number of bytes of data actually written
    return n

Any remainder is silently discarded, and _SerialTransportBase.send() discards write()'s return value — so a short write truncates an SMP message with no error anywhere. A genuine latent hazard worth its own issue (it belongs with #56), but not this flake. Here a non-zero write_timeout is simply required for super().write() to put the whole chunk out.

Verification

• The original flake: 25/25, from 1-in-30.
• The regression the first version caused: 25/25, from 2-in-15.
• Full integration suite: 229 passed, 101 skipped, 0 failures.
• mypy and pyright both clean on the changed file.

Scope

src/ is untouched. A real USB CDC raw UART has no baud pacing either, so the production transport arguably wants an explicit pacing option — an API question for the screaming-goblin window, filed separately rather than smuggled into a flake fix.

[JPHutchins]

LGTM