feat(transport): COBS+CRC16 serial framing for MCUboot raw serial recovery

src/smpclient/transport/serial/__init__.py

@@ -8,4 +8,6 @@
 from smpclient.transport.serial.encoded import BufferSize as BufferSize
 from smpclient.transport.serial.encoded import FragmentationStrategy as FragmentationStrategy
 from smpclient.transport.serial.encoded import SMPSerialTransport as SMPSerialTransport
+from smpclient.transport.serial.framing import SerialFraming as SerialFraming
+from smpclient.transport.serial.framing.cobs import Cobs as Cobs
 from smpclient.transport.serial.unencoded import SMPSerialRawTransport as SMPSerialRawTransport

src/smpclient/transport/serial/framing/__init__.py

@@ -0,0 +1,28 @@
+"""Pluggable wire framing for the raw serial SMP transport.
+
+`SMPSerialRawTransport` sends bare bytes when its `framing` is `None`; a `SerialFraming`
+wraps each SMP message instead (e.g. `cobs.Cobs`).  A framing owns its connection's
+reassembly buffer, so it is stateful: one instance per transport.
+"""
+
+from typing import Iterator, Protocol
+
+
+class SerialFraming(Protocol):
+    """Wire framing plus a stateful frame decoder for one serial connection."""
+
+    def encode(self, data: bytes) -> Iterator[bytes]:  # pragma: no cover
+        """Yield the wire bytes framing the SMP message `data`."""
+
+    def feed(self, data: bytes) -> None:  # pragma: no cover
+        """Buffer received bytes for decoding."""
+
+    def take(self) -> bytes | None:  # pragma: no cover
+        """Return the next decoded SMP message, or `None` if no complete frame is buffered.
+
+        Unconsumed bytes persist for the next call (a read may span frame boundaries), and a
+        framing that can detect corruption drops the damaged frame and resynchronises.
+        """
+
+    def reset(self) -> None:  # pragma: no cover
+        """Discard buffered bytes so a new connection starts clean."""

src/smpclient/transport/serial/framing/cobs.py

@@ -0,0 +1,150 @@
+"""COBS+CRC16 framing for MCUboot raw serial recovery (intercreate/mcuboot#5).
+
+The COBS codec matches MCUboot's ``boot/boot_serial/src/cobs.c``; the CRC is
+reused from `smp.packet`.
+"""
+
+import logging
+from dataclasses import dataclass, field
+from typing import Final, Iterator
+
+from smp.packet import CRC16_STRUCT, crc16_func
+
+logger = logging.getLogger(__name__)
+
+_DELIMITER: Final = 0
+_CODE_FULL: Final = 0xFF
+
+_MAX_BUFFER_BYTES: Final = 1 << 16
+"""Resync ceiling: bytes with no `0x00` delimiter past this are noise, not a frame, so the
+buffer is dropped.  64 KiB is far above any SMP-over-serial frame (recovery's
+`BOOT_SERIAL_MAX_RECEIVE_SIZE` is a few KiB), and bounds memory on a delimiter-free stream
+(e.g. a wrong-baud or wrong-protocol peer) the way the length-prefixed transport already does."""
+
+
+def cobs_encode(data: bytes) -> bytes:
+    r"""COBS-encode `data`, with no frame delimiter appended.
+
+    >>> cobs_encode(b"").hex()
+    '01'
+    >>> cobs_encode(b"\x01\x02\x03").hex()
+    '04010203'
+    >>> cobs_encode(b"\x00").hex()
+    '0101'
+    >>> cobs_encode(b"\x11\x00\x22").hex()
+    '02110222'
+    >>> cobs_decode(cobs_encode(bytes(range(256)))) == bytes(range(256))
+    True
+    """
+    out = bytearray([_DELIMITER])
+    code_index = 0
+    code = 1
+    for byte in data:
+        if byte != _DELIMITER:
+            out.append(byte)
+            code += 1
+            if code != _CODE_FULL:
+                continue
+        out[code_index] = code
+        code_index = len(out)
+        out.append(_DELIMITER)
+        code = 1
+    out[code_index] = code
+    return bytes(out)
+
+
+def cobs_decode(encoded: bytes) -> bytes:
+    r"""Decode a COBS frame whose delimiter has been stripped; inverse of `cobs_encode`.
+
+    >>> cobs_decode(bytes.fromhex("04010203")).hex()
+    '010203'
+    >>> cobs_decode(bytes.fromhex("0101"))
+    b'\x00'
+    >>> cobs_decode(bytes.fromhex("00"))
+    Traceback (most recent call last):
+        ...
+    ValueError: 0x00 code byte in COBS frame
+    >>> cobs_decode(bytes.fromhex("0511"))
+    Traceback (most recent call last):
+        ...
+    ValueError: truncated COBS frame
+    """
+    out = bytearray()
+    src = 0
+    while src < len(encoded):
+        code = encoded[src]
+        src += 1
+        if code == _DELIMITER:
+            raise ValueError("0x00 code byte in COBS frame")
+        for _ in range(1, code):
+            if src >= len(encoded):
+                raise ValueError("truncated COBS frame")
+            out.append(encoded[src])
+            src += 1
+        if code != _CODE_FULL and src < len(encoded):
+            out.append(_DELIMITER)
+    return bytes(out)
+
+
+def _decode_frame(frame: bytes) -> bytes | None:
+    r"""Decode and CRC-verify one COBS frame; return its SMP message, or `None` if damaged.
+
+    >>> message = b"\x0a\x00\x00\x01\x00\x01\x00\x05"
+    >>> _decode_frame(cobs_encode(message + CRC16_STRUCT.pack(crc16_func(message)))) == message
+    True
+    >>> _decode_frame(cobs_encode(message + b"\x00\x00")) is None  # wrong CRC
+    True
+    >>> _decode_frame(b"") is None  # empty frame (a stray delimiter)
+    True
+    >>> _decode_frame(cobs_encode(b"\x00\x00")) is None  # decodes to an empty message + CRC
+    True
+    """
+    try:
+        decoded = cobs_decode(frame)
+    except ValueError:
+        return None
+    if len(decoded) <= CRC16_STRUCT.size:  # nothing but (at most) a CRC: no message to carry
+        return None
+    message, crc = decoded[: -CRC16_STRUCT.size], decoded[-CRC16_STRUCT.size :]
+    if crc16_func(message) != CRC16_STRUCT.unpack(crc)[0]:
+        return None
+    return message
+
+
+@dataclass(frozen=True, slots=True)
+class Cobs:
+    """`SerialFraming` for MCUboot COBS+CRC16 raw serial recovery.
+
+    A value that also owns its connection's reassembly buffer, mutated in place (frozen
+    forbids rebinding the field, not mutating the `bytearray`); `reset` clears it.  Self-
+    synchronising: a corrupt or truncated frame is dropped and decoding resumes at the next
+    `0x00` delimiter.  Stateful, so use one instance per transport.
+    """
+
+    _buffer: bytearray = field(default_factory=bytearray, repr=False, compare=False)
+
+    def encode(self, data: bytes) -> Iterator[bytes]:
+        """Yield the one COBS frame for the SMP message `data`."""
+        yield cobs_encode(data + CRC16_STRUCT.pack(crc16_func(data))) + bytes([_DELIMITER])
+
+    def feed(self, data: bytes) -> None:
+        """Buffer received bytes for decoding."""
+        self._buffer.extend(data)
+
+    def take(self) -> bytes | None:
+        """Pop the next CRC-valid SMP message from the buffer, resyncing past bad frames."""
+        while (end := self._buffer.find(_DELIMITER)) != -1:
+            frame = bytes(self._buffer[:end])
+            del self._buffer[: end + 1]
+            if (message := _decode_frame(frame)) is not None:
+                return message
+            if frame:  # a stray delimiter gives an empty frame; only log real drops
+                logger.warning(f"COBS: dropped a {len(frame)} B frame, resyncing")
+        if len(self._buffer) > _MAX_BUFFER_BYTES:  # no delimiter in this much: noise, not a frame
+            logger.warning(f"COBS: discarding {len(self._buffer)} delimiter-less bytes, resyncing")
+            self._buffer.clear()
+        return None
+
+    def reset(self) -> None:
+        """Discard buffered bytes so a new connection starts clean."""
+        self._buffer.clear()

src/smpclient/transport/serial/unencoded.py

@@ -20,6 +20,7 @@
 
 from smpclient.exceptions import SMPClientException
 from smpclient.transport.serial.common import _SerialTransportBase
+from smpclient.transport.serial.framing import SerialFraming
 
 logger = logging.getLogger(__name__)
 
@@ -28,6 +29,8 @@ class SMPSerialRawTransport(_SerialTransportBase):
     def __init__(
         self,
         mtu: int = 384,
+        *,
+        framing: SerialFraming | None = None,
         baudrate: int = 115200,
         bytesize: int = 8,
         parity: str = "N",
@@ -47,6 +50,8 @@ def __init__(
                 bytes.  A serial link has no MTU of its own, but the SMP
                 server's receive buffer does -- this should match the server's
                 `CONFIG_MCUMGR_TRANSPORT_NETBUF_SIZE` (Zephyr default 384).
+            framing: optional wire framing for each SMP message (e.g. `Cobs()`);
+                `None` sends the bare `[header][payload]`.
             baudrate: The baudrate of the serial connection.  OK to ignore for
                 USB CDC ACM.
             bytesize: The number of data bits.
@@ -76,9 +81,16 @@ def __init__(
             exclusive=exclusive,
         )
         self._mtu: Final = mtu
+        self._framing: Final = framing
 
         logger.debug(f"Initialized {self.__class__.__name__}")
 
+    @override
+    def _reset_state(self) -> None:
+        """Clear the framing's reassembly buffer so a (re)connection starts clean."""
+        if self._framing is not None:
+            self._framing.reset()
+
     @override
     async def send(self, data: bytes) -> None:
         if len(data) > self.max_unencoded_size:
@@ -87,12 +99,41 @@ async def send(self, data: bytes) -> None:
             )
         logger.debug(f"Sending {len(data)} bytes")
         with self._serial_exception_to_disconnected():
-            self._conn.write(data)
+            if self._framing is None:
+                self._conn.write(data)
+            else:
+                for frame in self._framing.encode(data):
+                    self._conn.write(frame)
             await self._drain_tx()
         logger.debug(f"Sent {len(data)} bytes")
 
     @override
     async def receive(self) -> bytes:
+        if self._framing is None:
+            return await self._receive_length_prefixed()
+        return await self._receive_framed(self._framing)
+
+    async def _receive_framed(self, framing: SerialFraming) -> bytes:
+        """Return the next message from `framing`, reading more bytes as needed.
+
+        `framing` owns the buffer, so a read that spanned into the next frame -- or that
+        delivered several frames at once -- is drained from it before any further read.
+        """
+        logger.debug("Waiting for framed response")
+        while (message := framing.take()) is None:
+            data = await self._read_all()
+            if data:
+                framing.feed(data)
+                # `_read_all` does not await (pyserial is synchronous), so yield each
+                # iteration -- otherwise a non-stop non-framing stream would spin without
+                # ever suspending, and an outer request timeout could never fire.
+                await asyncio.sleep(0)
+            else:
+                await asyncio.sleep(self._POLLING_INTERVAL_S)
+        logger.debug(f"Finished receiving framed {len(message)} B response")
+        return bytes(message)
+
+    async def _receive_length_prefixed(self) -> bytes:
         logger.debug("Waiting for response")
         message = bytearray()
 

tests/fixtures/smp-server/SHA256SUMS

@@ -43,3 +43,5 @@ bf5b803bc5b3a108e7c5bfba025a6b923bf39603525b9e980638940dd24dd482  zephyr_4.4.0_s
 5cb70b1a4dee7ec367dd298813e40e01f05f62f408025c607880a3f1f8e96fa5  zephyr_4.4.0_smp_server_0eae053d_qemu_cortex_m0_serial_buf512.hex
 afd094784f98011121b292e2325afc30fd0d342378d9a091f05f0a7b95859f54  zephyr_f33aa2bc4a43_smp_server_27c54835_mps2_an385_serial_recovery_raw.hex
 f73694b0eb9feb6e53311a0143be81b0cb5c163a7af29ff7cb6a44fe5c618798  zephyr_f33aa2bc4a43_smp_server_27c54835_mps2_an385_serial_recovery_raw.signed.bin
+d38b208c0259d772e60eaa534c1bee7a1522cd6b4043cbba4ed52bbd9ac5eed2  zephyr_f33aa2bc4a43_smp_server_fa39f8c2_mps2_an385_serial_recovery_raw_cobs.hex
+ebe37ddd46f94b06b34a4ac081f32b482b32c6f83e29114c27b13fc56a4e729a  zephyr_f33aa2bc4a43_smp_server_fa39f8c2_mps2_an385_serial_recovery_raw_cobs.signed.bin

tests/fixtures/smp-server/VERSION

@@ -33,5 +33,14 @@
 # next includes #2755 and #2746, re-vendor the whole set from a single release and drop
 # this note.
 #
+# Additional pin — the COBS raw serial-recovery fixture `serial_recovery_raw_cobs` (mps2):
+#   tag:      fa39f8c2            https://github.com/intercreate/smp-server-fixtures/releases/tag/fa39f8c2
+#   commit:   fa39f8c2ce1bacf9907af8df4b04c397be2763cb (smp-server-fixtures)
+#   zephyr:   main f33aa2bc (post-4.4.0)
+#
+# COBS+CRC16 framing (CONFIG_BOOT_SERIAL_RAW_PROTOCOL_COBS) is not upstream MCUboot
+# (intercreate/mcuboot#5); the fixtures repo's build-patched job applies it to this one
+# fixture, whose artifacts carry the fa39f8c2 hash and so do not collide with the others.
+#
 # To bump: re-download the artifacts + SHA256SUMS + manifest.json from a newer
 # per-commit release and update the `tag` above.

tests/fixtures/smp-server/manifest.json

@@ -745,5 +745,32 @@
     "target": "mps2_an385",
     "transport": "serial_raw",
     "udp_port": null
+  },
+  {
+    "artifact": "zephyr_f33aa2bc4a43_smp_server_fa39f8c2_mps2_an385_serial_recovery_raw_cobs.hex",
+    "buf_count": 4,
+    "buf_size": 384,
+    "config": "serial_recovery_raw_cobs",
+    "groups": [
+      "os",
+      "img",
+      "stat",
+      "settings",
+      "fs",
+      "shell",
+      "enum",
+      "zbasic"
+    ],
+    "ip_family": null,
+    "line_length_max": 128,
+    "mcuboot": true,
+    "qemu_cmd": "qemu-system-arm -cpu cortex-m3 -machine mps2-an385 -nographic -chardev socket,id=con,host=127.0.0.1,port=<PORT>,server=on,wait=off -serial chardev:con -serial null -monitor none -device loader,file=zephyr_f33aa2bc4a43_smp_server_fa39f8c2_mps2_an385_serial_recovery_raw_cobs.hex -device loader,file=zephyr_f33aa2bc4a43_smp_server_fa39f8c2_mps2_an385_serial_recovery_raw_cobs.signed.bin,addr=0x20050000",
+    "recovery_buf_count": 1,
+    "recovery_buf_size": 1024,
+    "run": null,
+    "serial_recovery": true,
+    "target": "mps2_an385",
+    "transport": "serial_raw",
+    "udp_port": null
   }
 ]