feat(client): expose the SMP version on the convenience methods

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,13 +367,17 @@ 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.
 
         Args:
             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,
         )

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,

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}

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}