wip: use mcumgr params for serial transports

.github/workflows/test.yaml

@@ -14,11 +14,11 @@ jobs:
         python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"]
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
         with:
           fetch-depth: 0
 
-      - uses: astral-sh/setup-uv@v6
+      - uses: astral-sh/setup-uv@v8.1.0
 
       - run: uv run --python ${{ matrix.python-version }} task lint
 
@@ -28,7 +28,7 @@ jobs:
 
       - run: uv build
 
-      - uses: actions/upload-artifact@v4
+      - uses: actions/upload-artifact@v7
         with:
           name: dist-${{ matrix.os }}-${{ matrix.python-version }}
           path: |
@@ -80,11 +80,11 @@ jobs:
             expect-hci-firmware: pass
     name: transport-extras (${{ matrix.name }})
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
         with:
           fetch-depth: 0
 
-      - uses: astral-sh/setup-uv@v6
+      - uses: astral-sh/setup-uv@v8.1.0
 
       - run: uv venv && uv pip install .${{ matrix.extras }}
 
@@ -126,8 +126,33 @@ jobs:
   coverage:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
 
-      - uses: astral-sh/setup-uv@v6
+      - uses: astral-sh/setup-uv@v8.1.0
 
       - run: uv run task coverage
+
+  integration:
+    name: integration (native_sim + qemu)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          fetch-depth: 0
+
+      - uses: astral-sh/setup-uv@v8.1.0
+
+      - run: |
+          sudo dpkg --add-architecture i386
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends qemu-system-arm libc6:i386
+
+      - run: uv run task test-integration
+
+      - name: Upload integration test logs
+        if: always()
+        uses: actions/upload-artifact@v7
+        with:
+          path: integration-tests.log
+          archive: false
+          if-no-files-found: warn

.gitignore

@@ -6,3 +6,6 @@ __pycache__
 dist
 site
 coverage.xml
+integration-tests.log
+.claude/
+flash.bin

README.md

@@ -43,6 +43,98 @@ The UDP transport has no additional dependencies and is always available.
 Documentation is in the source code so that it is available to your editor.
 An online version is generated and available [here](https://intercreate.github.io/smpclient/).
 
+## Server Buffers & SMP Serial Fragmentation
+
+Choosing the right fragmentation for the **serial** transport (UART/USB/CAN/etc.) requires
+understanding the buffers in the firmware on the other end. The two SMP servers we
+target name the same concepts differently, and the docs/Kconfig on each side are
+easy to misread, so this section unifies the terminology. (BLE and UDP negotiate
+their MTU directly and need none of this.)
+
+### The SMP serial frame
+
+A serial SMP transaction is one **decoded frame**: a 2-byte length and a 2-byte
+CRC16 wrap the SMP message — and that wrapped message is exactly what the BLE and
+UDP transports send bare.
+
+```mermaid
+flowchart LR
+  subgraph FRAME["decoded frame"]
+    direction LR
+    LEN["length<br/>(2 B)"]
+    MSG["decoded SMP message<br/>(SMP header + CBOR)"]
+    CRC["CRC16<br/>(2 B)"]
+    LEN --- MSG --- CRC
+  end
+```
+
+For the **serial** transport that whole frame is base64-encoded and split into
+≤ 128-byte (configurable as "line length", but it should always be left at 128) lines on the wire. The server base64-**decodes each line as it arrives**
+and appends it to a single reassembly buffer — it never holds the whole encoded frame
+at once (mcuboot
+[`boot_serial_in_dec`](https://github.com/mcu-tools/mcuboot/blob/4ae65047c628365c30ec9a48eb0433169dc4f7ef/boot/boot_serial/src/boot_serial.c#L1460-L1490);
+Zephyr
+[`mcumgr_serial_decode_frag`](https://github.com/zephyrproject-rtos/zephyr/blob/a65b87f2c6d5961b105193bd94058e523d6df54f/subsys/mgmt/mcumgr/transport/src/serial_util.c#L48-L64)):
+
+```mermaid
+sequenceDiagram
+  participant C as smpclient
+  participant S as SMP server
+  loop each of N+1 base64 lines (≤ 128 B)
+    C->>S: start / cont. marker · base64(slice) · newline
+    S->>S: decode, append to reassembly buffer
+  end
+  S-->>C: response (length + CRC ok → dispatch)
+```
+
+So the buffer that actually limits a transaction holds the **decoded** frame, and:
+
+> **max SMP message = (reassembly buffer size) − 4**  (the 2-byte length + 2-byte CRC16).
+
+That message becomes **~1.37×** as many bytes once base64-encoded and split into
+≤ 128-byte lines (base64's 4/3 expansion plus per-line framing). A server that
+advertises a 2048-byte buffer therefore accepts a 2044-byte message that arrives as a
+**2801-byte, 23-line** encoded frame. The advertised/configured number is the
+*decoded* size — **not** the size of the encoded frame on the wire.
+
+### Terminology (mcuboot vs. Zephyr)
+
+| Role | Bounds | Enc/Dec | mcuboot symbol (default) | Zephyr symbol (default) |
+| --- | --- | --- | --- | --- |
+| **Line buffer** | one base64 line on the wire | encoded | [`BOOT_MAX_LINE_INPUT_LEN`](https://github.com/mcu-tools/mcuboot/blob/4ae65047c628365c30ec9a48eb0433169dc4f7ef/boot/zephyr/Kconfig.serial_recovery#L76) (128) | [`UART_MCUMGR_RX_BUF_SIZE`](https://github.com/zephyrproject-rtos/zephyr/blob/a65b87f2c6d5961b105193bd94058e523d6df54f/drivers/console/Kconfig#L226) (128) |
+| **Fragment pool** | queued unprocessed lines (throughput only — *not* a frame-size cap) | — | [`BOOT_LINE_BUFS`](https://github.com/mcu-tools/mcuboot/blob/4ae65047c628365c30ec9a48eb0433169dc4f7ef/boot/zephyr/Kconfig.serial_recovery#L84) (8) | [`UART_MCUMGR_RX_BUF_COUNT`](https://github.com/zephyrproject-rtos/zephyr/blob/a65b87f2c6d5961b105193bd94058e523d6df54f/drivers/console/Kconfig#L233) (2) |
+| **Reassembly buffer** | the whole message + 4 B framing — *the real ceiling* | decoded | [`BOOT_SERIAL_MAX_RECEIVE_SIZE`](https://github.com/mcu-tools/mcuboot/blob/4ae65047c628365c30ec9a48eb0433169dc4f7ef/boot/zephyr/Kconfig.serial_recovery#L91) (1024) → [`dec_buf`](https://github.com/mcu-tools/mcuboot/blob/4ae65047c628365c30ec9a48eb0433169dc4f7ef/boot/boot_serial/src/boot_serial.c#L164) | [`MCUMGR_TRANSPORT_NETBUF_SIZE`](https://github.com/zephyrproject-rtos/zephyr/blob/a65b87f2c6d5961b105193bd94058e523d6df54f/subsys/mgmt/mcumgr/transport/Kconfig#L40) (384) |
+| **Advertised `buf_size`** (MCUmgr params, OS group cmd 6) | what a client should target | decoded | *not advertised yet* ([mcuboot#2746](https://github.com/mcu-tools/mcuboot/pull/2746)) | = `NETBUF_SIZE` |
+| **Transport "MTU"** | mostly response framing / advisory | — | [`BOOT_SERIAL_FRAME_MTU`](https://github.com/mcu-tools/mcuboot/blob/4ae65047c628365c30ec9a48eb0433169dc4f7ef/boot/boot_serial/src/boot_serial.c#L131) (124, TX only) | [`MCUMGR_TRANSPORT_UART_MTU`](https://github.com/zephyrproject-rtos/zephyr/blob/a65b87f2c6d5961b105193bd94058e523d6df54f/subsys/mgmt/mcumgr/transport/Kconfig.uart#L27) (256, *registered but unused on UART*) |
+
+Two traps the table is meant to defuse:
+
+- The **fragment pool count** (`BOOT_LINE_BUFS` / `UART_MCUMGR_RX_BUF_COUNT`) is a
+  throughput queue — buffers are recycled as lines are decoded — and does **not**
+  bound the frame size. The decoded **reassembly buffer** does. (mcuboot's Kconfig
+  help suggests `MAX_RECEIVE_SIZE = line_len × line_bufs`, conflating the two; the
+  code sizes `dec_buf` to `MAX_RECEIVE_SIZE` *decoded* regardless.)
+- Zephyr's `MCUMGR_TRANSPORT_UART_MTU` reads like the send/receive limit, but its
+  `get_mtu` is never called on the UART path; the real incoming limit is `NETBUF_SIZE`.
+
+The **128-byte line** is a convention, not negotiated: both servers cap a line and
+discard the overflow ([mcuboot](https://github.com/mcu-tools/mcuboot/blob/4ae65047c628365c30ec9a48eb0433169dc4f7ef/boot/zephyr/serial_adapter.c#L160-L165),
+[Zephyr](https://github.com/zephyrproject-rtos/zephyr/blob/a65b87f2c6d5961b105193bd94058e523d6df54f/drivers/console/uart_mcumgr.c#L110-L118)),
+so clients must fragment at ≤ 128 (smpclient's default). Don't change it.
+
+### How smpclient targets these
+
+`SMPSerialTransport` fills the decoded reassembly buffer for best throughput. The
+`fragmentation_strategy` chooses how it learns the buffer size — `Auto` (default, from
+MCUmgr params), `BufferSize` (named directly, when params are unavailable such as mcuboot
+serial recovery), or `BufferParams` (a constrained encoded line-buffer budget). See the
+[Serial transport API docs](https://intercreate.github.io/smpclient/transport/serial/)
+for when to use each.
+
+This is exercised against real native_sim / QEMU / mps2 Zephyr servers in the
+integration suite (`tests/integration/`): a `buf_size − 4` message round-trips while
+putting ~1.37× `buf_size` encoded bytes on the wire, across the buffer-size matrix.
+
 ## Development Quickstart
 
 > Assumes that you've already [setup your development environment](#development-environment-setup).

examples/usb/upgrade.py

@@ -18,7 +18,7 @@
 from smpclient.mcuboot import IMAGE_TLV, ImageInfo
 from smpclient.requests.image_management import ImageStatesRead, ImageStatesWrite
 from smpclient.requests.os_management import ResetWrite
-from smpclient.transport.serial import SMPSerialTransport
+from smpclient.transport.serial import BufferParams, SMPSerialTransport
 
 logging.basicConfig(
     format="%(asctime)s.%(msecs)03d %(levelname)-8s %(message)s",
@@ -108,9 +108,10 @@ async def main() -> None:
     print("Connecting to SMP DUT...", end="", flush=True)
     async with SMPClient(
         SMPSerialTransport(
-            max_smp_encoded_frame_size=max_smp_encoded_frame_size,
-            line_length=line_length,
-            line_buffers=line_buffers,
+            fragmentation_strategy=BufferParams(
+                line_length=line_length,
+                line_buffers=line_buffers,
+            )
         ),
         port_a.device,
     ) as client:
@@ -187,9 +188,10 @@ async def ensure_request(request: SMPRequest[TRep, TEr1, TEr2]) -> TRep:
     print("Connecting to B SMP DUT...", end="", flush=True)
     async with SMPClient(
         SMPSerialTransport(
-            max_smp_encoded_frame_size=max_smp_encoded_frame_size,
-            line_length=line_length,
-            line_buffers=line_buffers,
+            fragmentation_strategy=BufferParams(
+                line_length=line_length,
+                line_buffers=line_buffers,
+            )
         ),
         port_b.device,
     ) as client:

pyproject.toml

@@ -79,8 +79,9 @@ format = "ruff format ."
 lint = "ruff check . && pydoclint src/smpclient"
 fix = "ruff check --fix . && ruff format ."
 typecheck = "mypy ."
-test = "pytest --maxfail=1"
-coverage = "pytest --cov --cov-report=xml --cov-report=term-missing"
+test = "pytest --maxfail=1 --ignore=tests/integration"
+test-integration = "pytest tests/integration --log-file=integration-tests.log --log-file-level=DEBUG --log-cli-level=INFO -o log_cli=true"
+coverage = "pytest --cov --cov-report=xml --cov-report=term-missing --ignore=tests/integration"
 all = "task format && task lint && task typecheck && task test"
 matrix = """
     UV_PROJECT_ENVIRONMENT=.venv-3.10 uv run --python 3.10 task all &&
@@ -118,10 +119,14 @@ check-yield-types = false
 [tool.pytest.ini_options]
 norecursedirs = ["dutfirmware/*", ".claude/*"]
 filterwarnings = ["ignore:The --rsyncdir:DeprecationWarning"]
+markers = [
+"integration: end-to-end tests against real SMP servers (Linux only); see tests/integration/",
+]
 
 [tool.coverage.run]
 source = ["smpclient", "tests"]
 branch = true
+omit = ["tests/integration/*"]
 
 [tool.coverage.report]
 fail_under = 91

src/smpclient/__init__.py

@@ -41,7 +41,7 @@
 from collections.abc import AsyncIterator
 from hashlib import sha256
 from types import TracebackType
-from typing import Final
+from typing import Final, TypeVar
 
 from pydantic import ValidationError
 from smp import header as smpheader
@@ -62,6 +62,9 @@
 
 logger = logging.getLogger(__name__)
 
+TUploadRequest = TypeVar("TUploadRequest", ImageUploadWrite, FileUpload)
+"""A single-shot upload request whose `data` field is filled to maximize throughput."""
+
 
 class SMPClient:
     """Create a client to the SMP server `address`, using `transport`.
@@ -251,7 +254,7 @@ async def upload(
         )
 
         response = await self.request(
-            self._maximize_image_upload_write_packet(
+            self._maximize_upload_packet(
                 ImageUploadWrite(
                     off=0,
                     data=b"",
@@ -277,7 +280,7 @@ async def upload(
         # send chunks until the SMP server reports that the offset is at the end of the image
         while response.off != len(image):
             response = await self.request(
-                self._maximize_image_upload_write_packet(
+                self._maximize_upload_packet(
                     ImageUploadWrite(
                         off=response.off,
                         data=b"",
@@ -329,7 +332,7 @@ async def upload_file(
         timeout_s = timeout_s if timeout_s is not None else self._timeout_s
 
         response = await self.request(
-            self._maximize_file_upload_packet(
+            self._maximize_upload_packet(
                 FileUpload(name=file_path, off=0, data=b"", len=len(file_data)),
                 file_data,
             ),
@@ -348,7 +351,7 @@ async def upload_file(
         # send chunks until the SMP server reports that the offset is at the end of the image
         while response.off != len(file_data):
             response = await self.request(
-                self._maximize_file_upload_packet(
+                self._maximize_upload_packet(
                     FileUpload(name=file_path, off=response.off, data=b""), file_data
                 ),
                 timeout_s=timeout_s,
@@ -469,43 +472,28 @@ def get_max_cbor_and_data_size(self, request: smpmsg.WriteRequest) -> tuple[int,
 
         return cbor_size, data_size
 
-    def _maximize_image_upload_write_packet(
-        self, request: ImageUploadWrite, image: bytes
-    ) -> ImageUploadWrite:
-        """Given an `ImageUploadWrite` with empty `data`, return the largest packet possible."""
-        h: Final = request.header
-        cbor_size, data_size = self.get_max_cbor_and_data_size(request)
-
-        if data_size > len(image) - request.off:  # final packet
-            data_size = len(image) - request.off
-            cbor_size = h.length + data_size + self._cbor_integer_size(data_size)
+    def _maximize_upload_packet(self, request: TUploadRequest, data: bytes) -> TUploadRequest:
+        """Given an upload request with empty `data`, return the largest packet possible.
 
-        return ImageUploadWrite(
-            header=smpheader.Header(
-                op=h.op,
-                version=h.version,
-                flags=h.flags,
-                length=cbor_size,
-                group_id=h.group_id,
-                sequence=h.sequence,
-                command_id=h.command_id,
-            ),
-            off=request.off,
-            data=image[request.off : request.off + data_size],
-            image=request.image,
-            len=request.len,
-            sha=request.sha,
-            upgrade=request.upgrade,
-        )
-
-    def _maximize_file_upload_packet(self, request: FileUpload, data: bytes) -> FileUpload:
-        """Given an `FileUpload` with empty `data`, return the largest packet possible."""
+        Fills the transport's `max_unencoded_size` so the encoded frame put on the wire
+        is as large as the server's reassembly buffer allows (best throughput).  Works
+        for any single-shot upload request (`ImageUploadWrite`, `FileUpload`): only
+        `header` (with the buffer-filling `length`) and `data` change; every other field
+        is carried over from `request`.
+        """
         h: Final = request.header
         cbor_size, data_size = self.get_max_cbor_and_data_size(request)
+
         if data_size > len(data) - request.off:  # final packet
             data_size = len(data) - request.off
             cbor_size = h.length + data_size + self._cbor_integer_size(data_size)
-        return FileUpload(
+
+        carried_over: Final = {
+            field: getattr(request, field)
+            for field in type(request).model_fields
+            if field not in ("header", "version", "sequence", "smp_data", "data")
+        }
+        return type(request)(
             header=smpheader.Header(
                 op=h.op,
                 version=h.version,
@@ -515,10 +503,8 @@ def _maximize_file_upload_packet(self, request: FileUpload, data: bytes) -> File
                 sequence=h.sequence,
                 command_id=h.command_id,
             ),
-            name=request.name,
-            off=request.off,
             data=data[request.off : request.off + data_size],
-            len=request.len,
+            **carried_over,
         )
 
     async def _initialize(self, timeout_s: float | None = None) -> None: