From 7aa1eba607e10fc8b52d9a76b1bd2ce61cb8d189 Mon Sep 17 00:00:00 2001 From: eastagiletracker <310448263+eastagiletracker@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:11:05 +0700 Subject: [PATCH] feat(client): expose the SMP version on the convenience methods `request()` can send any SMP version, because its caller builds the request. The routines that build their own requests could not: `upload()`, `upload_file()`, `download_file()` and `ICUploadClient.ic_upload()` always put SMP version 2 on the wire, so a server that predates it could only be driven by hand-rolling the chunking loop that these routines exist to provide. Add a `version` keyword argument to each, defaulting to `Version.V2` so existing callers are unaffected, and carry the version through the packet-maximizing helpers so every chunk of a multi-packet transfer uses it. The helpers now pass `version` alongside the header they build, which also keeps `smp` from logging an "Overriding self.version" warning for each maximized chunk when the two disagree. --- src/smpclient/__init__.py | 28 ++++- src/smpclient/extensions/intercreate.py | 31 ++++- tests/extensions/test_intercreate.py | 33 +++++ tests/test_smp_client.py | 155 ++++++++++++++++++++++++ 4 files changed, 239 insertions(+), 8 deletions(-) diff --git a/src/smpclient/__init__.py b/src/smpclient/__init__.py index 9db4a55..23dc6ed 100644 --- a/src/smpclient/__init__.py +++ b/src/smpclient/__init__.py @@ -266,6 +266,7 @@ async def upload( first_timeout_s: float = 40.0, subsequent_timeout_s: float | None = None, use_sha: bool = True, + version: smpheader.Version = smpheader.Version.V2, ) -> AsyncIterator[int]: """Iteratively upload an `image` to `slot`, yielding the offset. @@ -288,6 +289,9 @@ async def upload( Zephyr's SMP server will fail with `MGMT_ERR.EINVAL` if the MTU is too small to include both the SHA256 and the first 32-bytes of the image. Increase the MTU or set `use_sha=False` in this case. + version: the SMP version of the requests sent by this routine. The + default, `Version.V2`, is what current SMP servers expect; pass + `Version.V1` for servers that predate SMP version 2. Yields: the offset of the image upload @@ -308,6 +312,7 @@ async def upload( len=len(image), sha=sha256(image).digest() if use_sha else None, upgrade=upgrade, + version=version, ), image, ), @@ -333,6 +338,7 @@ async def upload( len=len(image) if response.off == 0 else None, image=slot if response.off == 0 else None, upgrade=upgrade if response.off == 0 else None, + version=version, ), image, ), @@ -361,6 +367,7 @@ async def upload_file( file_data: bytes, file_path: str, timeout_s: float | None = None, + version: smpheader.Version = smpheader.Version.V2, ) -> AsyncIterator[int]: """Iteratively upload a `file_data` to `file_path`, yielding the offset. @@ -368,6 +375,9 @@ async def upload_file( file_data: the `bytes` to upload file_path: the path to upload to timeout_s: the timeout for each `FileUpload` request + version: the SMP version of the requests sent by this routine. The + default, `Version.V2`, is what current SMP servers expect; pass + `Version.V1` for servers that predate SMP version 2. Yields: int: the offset of the file upload @@ -379,7 +389,7 @@ async def upload_file( response = await self.request( self._maximize_upload_packet( - FileUpload(name=file_path, off=0, data=b"", len=len(file_data)), + FileUpload(name=file_path, off=0, data=b"", len=len(file_data), version=version), file_data, ), timeout_s=timeout_s, @@ -398,7 +408,8 @@ async def upload_file( while response.off != len(file_data): response = await self.request( self._maximize_upload_packet( - FileUpload(name=file_path, off=response.off, data=b""), file_data + FileUpload(name=file_path, off=response.off, data=b"", version=version), + file_data, ), timeout_s=timeout_s, ) @@ -415,12 +426,16 @@ async def download_file( self, file_path: str, timeout_s: float | None = None, + version: smpheader.Version = smpheader.Version.V2, ) -> bytes: """Download a file from the SMP server. Args: file_path: the path to download timeout_s: the timeout for each `FileDownload` request + version: the SMP version of the requests sent by this routine. The + default, `Version.V2`, is what current SMP servers expect; pass + `Version.V1` for servers that predate SMP version 2. Returns: The downloaded file as `bytes` @@ -430,7 +445,9 @@ async def download_file( """ timeout_s = timeout_s if timeout_s is not None else self._timeout_s - response = await self.request(FileDownload(off=0, name=file_path), timeout_s=timeout_s) + response = await self.request( + FileDownload(off=0, name=file_path, version=version), timeout_s=timeout_s + ) file_length = 0 if error(response): @@ -447,7 +464,9 @@ async def download_file( # send chunks until the SMP server reports that the offset is at the end of the image while response.off + len(response.data) != file_length: response = await self.request( - FileDownload(off=response.off + len(response.data), name=file_path), + FileDownload( + off=response.off + len(response.data), name=file_path, version=version + ), timeout_s=timeout_s, ) if error(response): @@ -549,6 +568,7 @@ def _maximize_upload_packet(self, request: TUploadRequest, data: bytes) -> TUplo sequence=h.sequence, command_id=h.command_id, ), + version=h.version, data=data[request.off : request.off + data_size], **carried_over, ) diff --git a/src/smpclient/extensions/intercreate.py b/src/smpclient/extensions/intercreate.py index 1ce34e8..3811f7d 100644 --- a/src/smpclient/extensions/intercreate.py +++ b/src/smpclient/extensions/intercreate.py @@ -14,10 +14,30 @@ class ICUploadClient(SMPClient): """Support for Intercreate Group Upload.""" - async def ic_upload(self, data: bytes, image: int = 0) -> AsyncIterator[int]: - """Iteratively upload `data` to the SMP server, yielding the offset.""" + async def ic_upload( + self, + data: bytes, + image: int = 0, + version: smpheader.Version = smpheader.Version.V2, + ) -> AsyncIterator[int]: + """Iteratively upload `data` to the SMP server, yielding the offset. + + Args: + data: the `bytes` to upload + image: the image to upload to + version: the SMP version of the requests sent by this routine. The + default, `Version.V2`, is what current SMP servers expect; pass + `Version.V1` for servers that predate SMP version 2. + + Yields: + the offset of the upload + + Raises: + SMPUploadError: if the upload routine fails + Exception: if the response is neither a success nor an error + """ response = await self.request( - ic.ImageUploadWrite(off=0, data=b'', image=image, len=len(data)) + ic.ImageUploadWrite(off=0, data=b'', image=image, len=len(data), version=version) ) if error(response): @@ -30,7 +50,9 @@ async def ic_upload(self, data: bytes, image: int = 0) -> AsyncIterator[int]: # send chunks until the SMP server reports that the offset is at the end of the image while response.off != len(data): response = await self.request( - self._ic_maximize_packet(ic.ImageUploadWrite(off=response.off, data=b''), data) + self._ic_maximize_packet( + ic.ImageUploadWrite(off=response.off, data=b'', version=version), data + ) ) if error(response): raise SMPUploadError(response) @@ -58,6 +80,7 @@ def _ic_maximize_packet(self, request: ic.ImageUploadWrite, data: bytes) -> ic.I sequence=h.sequence, command_id=h.command_id, ), + version=h.version, off=request.off, data=data[request.off : request.off + data_size], image=request.image, diff --git a/tests/extensions/test_intercreate.py b/tests/extensions/test_intercreate.py index 5b0c9e6..e9a68a1 100644 --- a/tests/extensions/test_intercreate.py +++ b/tests/extensions/test_intercreate.py @@ -4,12 +4,14 @@ from unittest.mock import PropertyMock, patch import pytest +from smp import header as smphdr from smp import packet as smppacket from smp.user import intercreate as smpic from smpclient.extensions.intercreate import ICUploadClient from smpclient.requests.user import intercreate as ic from smpclient.transport.serial import SMPSerialTransport +from tests.test_smp_client import SMPMockTransport @patch('tests.test_smp_client.SMPSerialTransport.mtu', new_callable=PropertyMock) @@ -70,3 +72,34 @@ async def mock_request(request: ic.ImageUploadWrite) -> smpic.ImageUploadWriteRe next(decoder) assert reconstructed_image == image + + +@pytest.mark.asyncio +@pytest.mark.parametrize("version", [smphdr.Version.V1, smphdr.Version.V2]) +async def test_ic_upload_uses_the_requested_smp_version(version: smphdr.Version) -> None: + """Every chunk that `ic_upload()` sends carries the requested SMP version.""" + m = SMPMockTransport() + m._mtu = 498 + m._max_unencoded_size = 498 + s = ICUploadClient(m, "address", 2.5) + + data = bytes([i % 255 for i in range(4097)]) + sent: list[bytes] = [] + + async def send(frame: bytes) -> None: + sent.append(frame) + + async def receive() -> bytes: + request = ic.ImageUploadWrite.loads(sent[-1]) + return smpic.ImageUploadWriteResponse( + sequence=request.header.sequence, off=request.off + len(request.data) + ).BYTES + + m.send = send # type: ignore[assignment] + m.receive = receive # type: ignore[assignment] + + async for _ in s.ic_upload(data, version=version): + pass + + assert len(sent) > 1, "the data should not fit in a single chunk" + assert {smphdr.Header.loads(frame[: smphdr.Header.SIZE]).version for frame in sent} == {version} diff --git a/tests/test_smp_client.py b/tests/test_smp_client.py index 22666e8..b1c5281 100644 --- a/tests/test_smp_client.py +++ b/tests/test_smp_client.py @@ -1017,3 +1017,158 @@ def test_maximize_upload_packet_fills_decoded_buffer( on_wire = b"".join(smppacket.encode(maximized.BYTES, line_length=128)) assert len(on_wire) == encoded_frame_size assert len(on_wire) > buf_size + + +def _smp_versions_sent(frames: list[bytes]) -> set[smphdr.Version]: + """The SMP versions of every request frame put on the wire.""" + return {smphdr.Header.loads(frame[: smphdr.Header.SIZE]).version for frame in frames} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("version", [smphdr.Version.V1, smphdr.Version.V2]) +async def test_upload_uses_the_requested_smp_version(version: smphdr.Version) -> None: + """Every chunk that `upload()` sends carries the requested SMP version.""" + m = SMPMockTransport() + m._mtu = 498 + m._max_unencoded_size = 498 + s = SMPClient(m, "address", 2.5) + + image = bytes([i % 255 for i in range(4097)]) + sent: list[bytes] = [] + + async def send(data: bytes) -> None: + sent.append(data) + + async def receive() -> bytes: + request = ImageUploadWrite.loads(sent[-1]) + return ImageUploadWriteResponse( + sequence=request.header.sequence, off=request.off + len(request.data) + ).BYTES + + m.send = send # type: ignore[assignment] + m.receive = receive # type: ignore[assignment] + + async for _ in s.upload(image, version=version): + pass + + assert len(sent) > 1, "the image should not fit in a single chunk" + assert _smp_versions_sent(sent) == {version} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("version", [smphdr.Version.V1, smphdr.Version.V2]) +async def test_upload_file_uses_the_requested_smp_version(version: smphdr.Version) -> None: + """Every chunk that `upload_file()` sends carries the requested SMP version.""" + m = SMPMockTransport() + m._mtu = 498 + m._max_unencoded_size = 498 + s = SMPClient(m, "address", 2.5) + + data = bytes([i % 255 for i in range(4097)]) + sent: list[bytes] = [] + + async def send(frame: bytes) -> None: + sent.append(frame) + + async def receive() -> bytes: + request = FileUpload.loads(sent[-1]) + return FileUploadResponse( + sequence=request.header.sequence, off=request.off + len(request.data) + ).BYTES + + m.send = send # type: ignore[assignment] + m.receive = receive # type: ignore[assignment] + + async for _ in s.upload_file(data, file_path="test.txt", version=version): + pass + + assert len(sent) > 1, "the file should not fit in a single chunk" + assert _smp_versions_sent(sent) == {version} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("version", [smphdr.Version.V1, smphdr.Version.V2]) +async def test_download_file_uses_the_requested_smp_version(version: smphdr.Version) -> None: + """Every request that `download_file()` sends carries the requested SMP version.""" + m = SMPMockTransport() + m._mtu = 498 + m._max_unencoded_size = 498 + s = SMPClient(m, "address", 2.5) + + data = bytes([i % 255 for i in range(4097)]) + sent: list[bytes] = [] + + async def send(frame: bytes) -> None: + sent.append(frame) + + async def receive() -> bytes: + request = FileDownload.loads(sent[-1]) + chunk = data[request.off : request.off + 456] + if request.off == 0: + return FileDownloadResponse( + sequence=request.header.sequence, off=0, data=chunk, len=len(data) + ).BYTES + return FileDownloadResponse( + sequence=request.header.sequence, off=request.off, data=chunk + ).BYTES + + m.send = send # type: ignore[assignment] + m.receive = receive # type: ignore[assignment] + + assert await s.download_file(file_path="test.txt", version=version) == data + + assert len(sent) > 1, "the file should not fit in a single response" + assert _smp_versions_sent(sent) == {version} + + +@pytest.mark.asyncio +async def test_convenience_methods_default_to_smp_version_2() -> None: + """Callers that do not ask for a version keep getting SMP version 2.""" + m = SMPMockTransport() + m._mtu = 498 + m._max_unencoded_size = 498 + s = SMPClient(m, "address", 2.5) + + data = bytes([i % 255 for i in range(4097)]) + sent: list[bytes] = [] + + async def send(frame: bytes) -> None: + sent.append(frame) + + async def receive_upload() -> bytes: + request = ImageUploadWrite.loads(sent[-1]) + return ImageUploadWriteResponse( + sequence=request.header.sequence, off=request.off + len(request.data) + ).BYTES + + async def receive_upload_file() -> bytes: + request = FileUpload.loads(sent[-1]) + return FileUploadResponse( + sequence=request.header.sequence, off=request.off + len(request.data) + ).BYTES + + async def receive_download_file() -> bytes: + request = FileDownload.loads(sent[-1]) + chunk = data[request.off : request.off + 456] + if request.off == 0: + return FileDownloadResponse( + sequence=request.header.sequence, off=0, data=chunk, len=len(data) + ).BYTES + return FileDownloadResponse( + sequence=request.header.sequence, off=request.off, data=chunk + ).BYTES + + m.send = send # type: ignore[assignment] + + m.receive = receive_upload # type: ignore[assignment] + async for _ in s.upload(data): + pass + + m.receive = receive_upload_file # type: ignore[assignment] + async for _ in s.upload_file(data, file_path="test.txt"): + pass + + m.receive = receive_download_file # type: ignore[assignment] + assert await s.download_file(file_path="test.txt") == data + + assert _smp_versions_sent(sent) == {smphdr.Version.V2}