Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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: |
Expand Down Expand Up @@ -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 }}

Expand Down Expand Up @@ -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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ __pycache__
dist
site
coverage.xml
integration-tests.log
.claude/
flash.bin
14 changes: 8 additions & 6 deletions examples/usb/upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=SMPSerialTransport.BufferParams(
line_length=line_length,
line_buffers=line_buffers,
)
),
port_a.device,
) as client:
Expand Down Expand Up @@ -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=SMPSerialTransport.BufferParams(
line_length=line_length,
line_buffers=line_buffers,
)
),
port_b.device,
) as client:
Expand Down
9 changes: 7 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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
Expand Down
150 changes: 116 additions & 34 deletions src/smpclient/transport/serial.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
import math
import time
from enum import IntEnum, unique
from functools import cached_property
from typing import Final
from typing import Final, NamedTuple

try:
from serial import Serial, SerialException
Expand Down Expand Up @@ -59,11 +58,29 @@ class BufferState(IntEnum):
`_buffer` is being parsed as serial data.
"""

class Auto(NamedTuple):
"""Automatically determine buffer parameters from the SMP server.

On connect, queries the server's MCUMGR_PARAM for `buf_size`
(CONFIG_MCUMGR_TRANSPORT_NETBUF_SIZE) and calculates:
- line_length: 128 (standard MTU for uart/usb/shell)
- line_buffers: buf_size / line_length

Falls back to BufferParams() if server doesn't support MCUMGR_PARAM.
"""

class BufferParams(NamedTuple):
"""Buffer parameters for the serial transport."""

line_length: int = 128
"""The maximum SMP packet size."""

line_buffers: int = 1
"""The number of line buffers in the serial buffer."""

def __init__( # noqa: DOC301
self,
max_smp_encoded_frame_size: int = 256,
line_length: int = 128,
line_buffers: int = 2,
fragmentation_strategy: Auto | BufferParams = Auto(),
baudrate: int = 115200,
bytesize: int = 8,
parity: str = "N",
Expand All @@ -79,11 +96,10 @@ def __init__( # noqa: DOC301
"""Initialize the serial transport.

Args:
max_smp_encoded_frame_size: The maximum size of an encoded SMP
frame. The SMP server needs to have a buffer large enough to
receive the encoded frame packets and to store the decoded frame.
line_length: The maximum SMP packet size.
line_buffers: The number of line buffers in the serial buffer.
fragmentation_strategy: The fragmentation strategy to use. Either
`SMPSerialTransport.Auto()` to automatically determine buffer
parameters from the SMP server, or `SMPSerialTransport.BufferParams`
to manually specify buffer parameters.
baudrate: The baudrate of the serial connection. OK to ignore for
USB CDC ACM.
bytesize: The number of data bits.
Expand All @@ -98,18 +114,7 @@ def __init__( # noqa: DOC301
exclusive: The exclusive access timeout.

"""
if max_smp_encoded_frame_size < line_length * line_buffers:
logger.error(
f"{max_smp_encoded_frame_size=} is less than {line_length=} * {line_buffers=}!"
)
elif max_smp_encoded_frame_size != line_length * line_buffers:
logger.warning(
f"{max_smp_encoded_frame_size=} is not equal to {line_length=} * {line_buffers=}!"
)

self._max_smp_encoded_frame_size: Final = max_smp_encoded_frame_size
self._line_length: Final = line_length
self._line_buffers: Final = line_buffers
self._fragmentation_strategy: Final = fragmentation_strategy
self._conn: Final = Serial(
baudrate=baudrate,
bytesize=bytesize,
Expand Down Expand Up @@ -142,6 +147,62 @@ def _reset_state(self) -> None:
self._buffer = bytearray([])
self._buffer_state = SMPSerialTransport.BufferState.SERIAL

@property
def _line_length(self) -> int:
"""The maximum SMP packet size."""
if isinstance(self._fragmentation_strategy, SMPSerialTransport.Auto):
return self.BufferParams().line_length
else:
return self._fragmentation_strategy.line_length

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we dropped 3.9 support, so this should be replaced with exhaustive match case.


@property
def _line_buffers(self) -> int:
"""The number of line buffers."""
if isinstance(self._fragmentation_strategy, SMPSerialTransport.Auto):
if self._smp_server_transport_buffer_size is not None:
return self._smp_server_transport_buffer_size // self.BufferParams().line_length
return self.BufferParams().line_buffers
else:
return self._fragmentation_strategy.line_buffers

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the floor-division inconsistency — fixed in a714fc8, though the empirical result is the opposite of an overflow.

I probed the real servers (native_sim / QEMU / mps2): buf_size is CONFIG_MCUMGR_TRANSPORT_NETBUF_SIZE, which bounds the decoded SMP serial frame uint16 length | message | uint16 CRC16 reassembled into one netbuf — the base64/line framing is stripped before the netbuf. So the limit is simply buf_size - 4 (2-byte length + 2-byte CRC16), and buf_count is irrelevant to a single serial message (one netbuf per message).

Boundary, confirmed at three sizes (client allowed to send past the old cap):

buf_size buf_size - 4 buf_size - 3
96 accepted dropped
256 accepted dropped
384 accepted dropped

So the old base64_max(buf_size) - per_line_framing * (buf_size // line_length) was actually ~25-30% conservative (e.g. buf384 → 255 instead of 380), not over-permissive — and the floor-division line count you flagged fed that wrong framing term.

The fix drops the line-count dependency in Auto: max_unencoded_size = buf_size - (FRAME_LENGTH + CRC16). BufferParams mode (the encoded line-buffer model, e.g. MCUboot's 128×8 UART buffer) is unchanged. Locked by tests: a unit assert (buf_size - 4), an integration assert across the buffer matrix, and test_max_payload_roundtrip actually round-tripping a max-size message (including the sub-line-length buf96) against the real fixtures.


@property
def _max_smp_encoded_frame_size(self) -> int:
"""The maximum encoded frame size (line_length * line_buffers)."""
if isinstance(self._fragmentation_strategy, SMPSerialTransport.Auto):
if self._smp_server_transport_buffer_size is not None:
return self._smp_server_transport_buffer_size
return self._line_length * self._line_buffers
else:
return (
self._fragmentation_strategy.line_length * self._fragmentation_strategy.line_buffers
)

@override
def initialize(self, smp_server_transport_buffer_size: int) -> None:
"""Initialize with the server's buffer size from MCUMGR_PARAM.

Args:
smp_server_transport_buffer_size: The server's CONFIG_MCUMGR_TRANSPORT_NETBUF_SIZE
"""
super().initialize(smp_server_transport_buffer_size)

if isinstance(self._fragmentation_strategy, SMPSerialTransport.Auto):
logger.info(
f"Auto-configured from server: {self._line_length=}, "
f"{self._line_buffers=}, mtu={self._max_smp_encoded_frame_size}"
)
else:
# Validate user's BufferParams against server capabilities
calculated_size = (
self._fragmentation_strategy.line_length * self._fragmentation_strategy.line_buffers
)
if calculated_size > smp_server_transport_buffer_size:
logger.warning(
f"BufferParams (line_length={self._fragmentation_strategy.line_length} * "
f"line_buffers={self._fragmentation_strategy.line_buffers} = {calculated_size}) " # noqa: E501
f"exceeds server buffer size ({smp_server_transport_buffer_size})"
)

@override
async def connect(self, address: str, timeout_s: float) -> None:
self._reset_state()
Expand Down Expand Up @@ -353,19 +414,40 @@ def mtu(self) -> int:
return self._max_smp_encoded_frame_size

@override
@cached_property
@property
def max_unencoded_size(self) -> int:
"""The serial transport encodes each packet instead of sending SMP messages as raw bytes."""
# For each packet, AKA line_buffer, include the cost of the base64
# encoded frame_length and CRC16 and the start/continue delimiter.
# Add to that the cost of the stop delimiter.
"""The maximum unencoded SMP message size, in bytes.

`Auto` (server `buf_size` known): `buf_size` is
`CONFIG_MCUMGR_TRANSPORT_NETBUF_SIZE`, which bounds the *decoded* SMP serial
frame `[length][message][CRC16]` reassembled into the server's netbuf -- the
base64/line framing is stripped before the netbuf. So the message is bounded
by `buf_size` minus the 2-byte frame length and 2-byte CRC, and the full
netbuf is usable. (Verified against native_sim/QEMU/mps2: a `buf_size - 4`
message round-trips; `buf_size - 3` is dropped.)

`BufferParams` (or `Auto` before initialization): the user specifies the
*encoded* line-buffer capacity (`line_length * line_buffers`, e.g. MCUboot's
128*8 UART buffer), so the limit is how many unencoded bytes survive base64
expansion and per-line framing within that encoded budget.

SMP serial framing (the 2-byte length + 2-byte CRC16):
https://docs.zephyrproject.org/latest/services/device_mgmt/smp_transport.html
"""
# The SMP serial frame wraps the message as `uint16 length | message | uint16
# CRC16`; the server reassembles that decoded frame into one buf_size netbuf.
frame_overhead: Final = smppacket.FRAME_LENGTH_STRUCT.size + smppacket.CRC16_STRUCT.size

if (
isinstance(self._fragmentation_strategy, SMPSerialTransport.Auto)
and self._smp_server_transport_buffer_size is not None
):
return self._smp_server_transport_buffer_size - frame_overhead

# Encoded-budget model: subtract per-line framing (base64 frame_length + CRC16
# + start/continue delimiter, per line buffer) and the stop delimiter, then
# convert the remaining encoded budget to its unencoded capacity.
packet_framing_size: Final = (
_base64_cost(smppacket.FRAME_LENGTH_STRUCT.size + smppacket.CRC16_STRUCT.size)
+ smppacket.DELIMITER_SIZE
_base64_cost(frame_overhead) + smppacket.DELIMITER_SIZE
) * self._line_buffers + len(smppacket.END_DELIMITER)

# Get the number of unencoded bytes that can fit in self.mtu and
# subtract the cost of framing the separate packets.
# This is the maximum number of unencoded bytes that can be received by
# the SMP server with this transport configuration.
return _base64_max(self.mtu) - packet_framing_size
43 changes: 43 additions & 0 deletions tests/fixtures/smp-server/SHA256SUMS
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
ddc72e438e364199b20108a8173a141e36d4268b35a6cfa2e03ff9b88e34d871 manifest.json
29176dac9fa963db772f0ff8d7e59103543261c9db0ba27b31d659c499819500 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial.elf
b17a5dbbd22e00e6e7b081e4e7e0996d8bfb4f87c1de245b96800a6e96b54d32 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery.hex
5dd5061002766e3aca3c5a3c37143d08bd8ad1be56acc7ada6acaf8d2d7b0a85 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery.signed.bin
b17a5dbbd22e00e6e7b081e4e7e0996d8bfb4f87c1de245b96800a6e96b54d32 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf1024.hex
f08bc569707f95f86eb545426001555c86b2da744fc58d7f9029aa0dbaa4f840 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf1024.signed.bin
b17a5dbbd22e00e6e7b081e4e7e0996d8bfb4f87c1de245b96800a6e96b54d32 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf2048.hex
c9e9782a7dedfcd83c4da82da82f703c26aa65522532709ddadfc7df176eed86 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf2048.signed.bin
b17a5dbbd22e00e6e7b081e4e7e0996d8bfb4f87c1de245b96800a6e96b54d32 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf400.hex
8add8b9048847a8311abe078c50005c8264d9fd78e33a562e898e8cfeae74259 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf400.signed.bin
b17a5dbbd22e00e6e7b081e4e7e0996d8bfb4f87c1de245b96800a6e96b54d32 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf512.hex
e2c3de570008a6d44c71352aba917d0aba2675cbf22ec1ec1fab14eff4f78cc9 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_serial_recovery_buf512.signed.bin
38e5c253f472422333d2403b1659ead31639b0e67a212f59f322b0cd0e9649a1 zephyr_4.4.0_smp_server_0eae053d_mps2_an385_shell.elf
3f0ed0fdc2dd9193b48df7069acc47dc6ce136856b3633c8d776844b11612b10 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial.exe
888a421a9251cedab04067233df5106549dcfebb2dd69593a69de3c2871203cd zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_bigrx.exe
836df02142a31313189893a6355dcf407a21eda5321e93fecfac9ffca43d3ffc zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_buf1024.exe
cda3e0b9af1b8641fb1376b5f16e8888cacc17cfbeb47444f059cc9dd944e2d9 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_buf128.exe
32531f4b65a0705bc33934257ac9012819ce24ba40e798910ba51d79b2188c5b zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_buf2048.exe
5049d07ee5df22b40ed0c9d1e70c9b19220cab434fa6c8cdacde8e76e82f4fc2 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_buf256.exe
35bccc468a44354e45af381a2bdcf93344662351af89cc662e30ec70b312daf0 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_buf400.exe
dafc320c58990b3a259200b521339d19fd49353db8ff9313c29ceee7c14ffa7a zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_buf512.exe
76c450f317837ee7f29c414dc14d10eb8fb925b863dfc85999cdf6484250dd42 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_buf96.exe
c619167e2772de4f1eee99c08c3db711833332b0aa0b98df79162673f2f176e0 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_fs.exe
2b771b9f181f011ad0bd0da262e46002ad4775ce54046ad3875ab6fcc2f411d7 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_line512.exe
8dc5a6569cdb2227145d2254309cb833d272e3bfb17b1239ee2f76c351dcbed7 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_noparams.exe
a5973945677536d057ecf872908e0fc3599e68a4663573167ca2c8ec86827785 zephyr_4.4.0_smp_server_0eae053d_native_sim_serial_raw.exe
ddb3414ef23dcfbb4ac6b48222d4c7dccc7388cd45b31e3328cd97d4b214fdff zephyr_4.4.0_smp_server_0eae053d_native_sim_shell.exe
9affd08d35402b94b3f0b75dd3bfd31616151c5016bc00fcc58f8f2207a3600f zephyr_4.4.0_smp_server_0eae053d_native_sim_udp.exe
b4dc889715cb50ee478bf851ed2b6ae8c14dec84efd8c92670e164d84b18de85 zephyr_4.4.0_smp_server_0eae053d_native_sim_udp6.exe
88dfdc9e5cfb974cb448c2d73e1d0694e001e1a59f3fbde31dbeae8f102bd526 zephyr_4.4.0_smp_server_0eae053d_native_sim_udp_buf1024.exe
194ccc77d5e8eff902bf43b60b8f5c9cb6b0f45b87b41e842b35ffcfdebc51f1 zephyr_4.4.0_smp_server_0eae053d_native_sim_udp_buf2048.exe
d488ced1b915daa9bcfc5db08c1d6b5687a1607f965ce1c4e802fc7757ca6406 zephyr_4.4.0_smp_server_0eae053d_native_sim_udp_buf256.exe
156828474033566bf9c8685c71f901a31ac3e5673185af25a7fb709447532ae1 zephyr_4.4.0_smp_server_0eae053d_native_sim_udp_fs.exe
f4dd773e186ace02f19906af335749aaadc17e99773916d6b8d89d219475bee6 zephyr_4.4.0_smp_server_0eae053d_nrf52840dk_ble.merged.hex
ce54143602724caf9c104b95f7c5025be5f3e50a0c252a9d4ace7295b8836899 zephyr_4.4.0_smp_server_0eae053d_nrf52840dk_ble.signed.bin
14f0568cacf53459549c852233b2bdc3be9f0858b6f7c47b5d9d544e005cf582 zephyr_4.4.0_smp_server_0eae053d_nrf52840dk_serial.merged.hex
c32299e6711d95558df01da315b921f736dc5be2fb8988d2506931892b4f045c zephyr_4.4.0_smp_server_0eae053d_nrf52840dk_serial.signed.bin
7f8443372a6583434372e0eea84127482c971983f00e3f86e30f9a0e4b6754e8 zephyr_4.4.0_smp_server_0eae053d_nrf52840dk_serial_recovery.merged.hex
23a6bca472174cdc859959baadb1f736964e06ad81a040835c3dda3a36401902 zephyr_4.4.0_smp_server_0eae053d_nrf52840dk_serial_recovery.signed.bin
2e0cda996515b444f324f26d794e55e5f08b3ddff210d6fcec6f994647eb289e zephyr_4.4.0_smp_server_0eae053d_qemu_cortex_m0_serial.merged.hex
e96f3b40d10418018caa1c3f9776d3bbdd542f5612e3caad3a223bb0b2a24c54 zephyr_4.4.0_smp_server_0eae053d_qemu_cortex_m0_serial.signed.bin
bf5b803bc5b3a108e7c5bfba025a6b923bf39603525b9e980638940dd24dd482 zephyr_4.4.0_smp_server_0eae053d_qemu_cortex_m0_serial_buf256.hex
5cb70b1a4dee7ec367dd298813e40e01f05f62f408025c607880a3f1f8e96fa5 zephyr_4.4.0_smp_server_0eae053d_qemu_cortex_m0_serial_buf512.hex
19 changes: 19 additions & 0 deletions tests/fixtures/smp-server/VERSION
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Vendored SMP server fixtures
#
# Prebuilt Zephyr SMP servers used by the integration suite (tests/integration/).
# Source: https://github.com/intercreate/smp-server-fixtures
#
# Pinned release (permanent, per-commit — NOT the rolling `latest` tag, which
# can drift / be replaced):
# tag: 0eae053d https://github.com/intercreate/smp-server-fixtures/releases/tag/0eae053d
# commit: 0eae053d6ae74cbbc28237d333737387ba9db899
# zephyr: 4.4.0
#
# The integration harness builds its fixture registry from the vendored
# `manifest.json` (also from this release) and verifies each artifact against
# `SHA256SUMS` before launch; see tests/integration/servers.py. Only the subset
# of artifacts actually committed here become live fixtures — the manifest lists
# the full release so adding one is just `gh release download <tag> --pattern ...`.
#
# To bump: re-download the artifacts + SHA256SUMS + manifest.json from a newer
# per-commit release and update the `tag` above.
Loading
Loading