feat(transport): COBS+CRC16 serial framing for MCUboot raw serial recovery#122
Open
JPHutchins wants to merge 1 commit into
Open
feat(transport): COBS+CRC16 serial framing for MCUboot raw serial recovery#122JPHutchins wants to merge 1 commit into
JPHutchins wants to merge 1 commit into
Conversation
Add a pluggable `SerialFraming` to `SMPSerialRawTransport` plus a `Cobs` framing that wraps each raw SMP packet as COBS(header || payload || CRC16) || 0x00 -- MCUboot's `BOOT_SERIAL_RAW_PROTOCOL_COBS` (intercreate/mcuboot#5, layered on mcu-tools/mcuboot#2755). The trailing 0x00 makes the byte stream self-synchronising and the CRC-16/XMODEM rejects damaged frames, so the decoder drops a corrupt or truncated frame and resyncs at the next delimiter. `Cobs` is a frozen, slotted dataclass that owns its connection's reassembly buffer (the transport feeds/takes/resets it); `framing=None` leaves the bare length-prefixed raw path unchanged. Vendor the params-advertising `serial_recovery_raw_cobs` fixture from smp-server-fixtures release fa39f8c2 (intercreate/smp-server-fixtures#12), and unify the MCUboot serial-recovery integration test over every framing (console Auto/BufferSize, raw, raw+COBS). Also strengthen the DFU upload test to match the image's SHA256 TLV against the reported slot hash. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KKXy7L2Udjw929nie2c363
JPHutchins
requested review from
aldenhaase,
davedesro,
elcalzado,
intercreate-gab,
klam20,
kylermconnelly,
robhelvestine and
swoisz
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds COBS+CRC16 framing to
SMPSerialRawTransport, so smpclient can drive MCUboot serial recovery built with the new self-synchronising raw protocol.Upstream references
CONFIG_BOOT_SERIAL_RAW_PROTOCOL_COBSthis transport speaks; draft, layered on the raw protocol below).BOOT_SERIAL_RAW_PROTOCOL) this builds on.west patchof mcuboot#5) and released asfa39f8c2.On the wire
Each SMP message is framed as
COBS(header ‖ payload ‖ CRC16) ‖ 0x00. The CRC is CRC-16/XMODEM over the message (reused fromsmp.packet); COBS byte-stuffing removes every0x00, so the single trailing0x00is an unambiguous delimiter. The COBS codec matches MCUboot'sboot/boot_serial/src/cobs.cbyte-for-byte. Because the delimiter is a resync point and the CRC rejects damage, the decoder drops a corrupt or truncated frame and resyncs at the next delimiter rather than failing the stream.Design
SerialFramingProtocol (transport/serial/framing/):encode/feed/take/reset— a wire framing plus a stateful per-connection decoder.Cobs— a@dataclass(frozen=True, slots=True)that owns its reassembly buffer (field(default_factory=bytearray)→ fresh per instance, mutated in place;frozenblocks rebinding, not mutation). One instance per transport.SMPSerialRawTransport(framing=...)—None(default) keeps the bare[header][payload]length-prefixed path unchanged;framing=Cobs()opts into COBS.receive()just pumps bytes throughfeed/take;reset()clears the buffer on (re)connect.max_unencoded_sizeis unchanged (COBS keeps the CRC in extra server-buffer space, so the cap is the advertisedbuf_size, same as raw).COBS isn't auto-negotiated — there's no params bit for it (mcuboot#5: "the SMP client must be configured to use a matching COBS+CRC16 transport"), so it's an explicit opt-in.
Tests
tests/test_cobs.py,tests/test_smp_serial_raw_transport.py): COBS codec round-trip + run-length-boundary + malformed + deterministic fuzz; framing leftover-across-feeds, multi-frame-per-feed, byte-at-a-time, randomly-chunked stream; resync past bad-CRC / leading garbage / empty frames; lone-corrupt →None; empty-message frame dropped; delimiter-less buffer capped (64 KiB) + resync; per-instance buffer independence +reset(); and the transport's framed send/receive incl. a regression test that a non-stop non-framing stream still lets an outer timeout fire.tests/integration/test_serial_recovery.py): the MCUboot recovery test is now one parameterized test (sum type,match/assert_never) over every framing — consoleAuto, consoleBufferSize, raw, and raw+COBS — each asserting the negotiated cap/wire size, uploading against real QEMU (mps2/an385), and confirming the recovery img group is coherent afterward.test_serial_recovery_raw.pyis folded in.test_image_management.pynow asserts a completed upload's image (ImageInfoSHA256 TLV) actually appears as a slothash, not merely that something landed.ruff/pydoclint/mypy --strict/ unit all green; recovery + DFU integration pass against QEMU.The change was adversarially stress-tested by independent reviewers (crafted byte streams against the decoder + a diff review); confirmed findings — a busy-loop hang on a non-framing stream, an empty-message-frame acceptance, an unbounded buffer, and a vacuous resync test — were all fixed and have regression tests.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KKXy7L2Udjw929nie2c363