From 095d97d1cfcef491dcde532e26f9464d0674e04a Mon Sep 17 00:00:00 2001 From: JP Hutchins Date: Fri, 28 Aug 2026 12:41:46 -0700 Subject: [PATCH] refactor(mcuboot): replace pydantic dataclasses with stdlib dataclasses smp's `screaming-goblin` moves off pydantic to msgspec, so `pip install smp` will stop pulling pydantic in. smpclient never declared pydantic itself -- it has been relying on it arriving transitively via smp 4.x -- so rather than declare a dependency we want gone, remove the need for it. Closes #133. Every class keeps `@dataclass(frozen=True)`; only the import moves. That keeps `ImageTLVValue.__post_init__`, `ImageInfo._map_tlv_type_to_value`'s `cached_property`, and the generic `ImageTLVInfo[T]` exactly as they were, and leaves the public types untouched -- `tlvs` is still a `list`. ## What pydantic was actually doing Three things, and only one of them was interesting: 1. `@dataclass(frozen=True)` on six classes, validating values that `struct.Struct.unpack()` had already produced as ints. 2. Coercion. Two fields were silently narrowed on construction, and both are load-bearing: `ImageHeader.flags` (a bare int from `unpack`) became `IMAGE_F`, and `ImageTLV.type` was resolved left-to-right through `Annotated[Union[IMAGE_TLV, VendorTLV, int], Field(union_mode=...)]`. 3. `VendorTLV.__get_pydantic_core_schema__`, which existed only to expose (2) to pydantic. The actual range check was already plain Python in `__new__`. So `ImageTLVType` becomes a plain `IMAGE_TLV | VendorTLV | int`, and the coercion becomes `_narrow_tlv_type()`, which walks the union in declaration order and lets each member's own constructor decide whether it accepts the value -- so the vendor range stays owned by `VendorTLV` instead of being restated. `ImageHeader.loads` narrows `flags` where the int is produced, and `ImageTLV.__post_init__` narrows `type`, which is what pydantic's coercion did and what the existing tests pin. msgspec was considered and rejected: this module does no CBOR/JSON de/serialisation (`struct` parses the binary), and `IMAGE_TLV | VendorTLV | int` is a union of three int-like types, which msgspec cannot decode anyway. ## Equivalence A characterisation script captured the pydantic behaviour before the change -- field types, every constructor coercion, `IMAGE_F` handling of undeclared bits, length validation, equality/hash, and the full `str()` of both fixture images -- and its output is byte-for-byte identical afterwards. There is no behavioural change at all. The 21 existing tests are unchanged except `test_tlv_type_union_order`, which drove pydantic's `TypeAdapter` directly and now drives `_narrow_tlv_type`; its vendor case is strengthened, since asserting `isinstance(result, int)` was also true of the `VendorTLV` it was meant to pin. `camas matrix` green on 3.10-3.14; `mcuboot.py` at 100% coverage; the integration suite passes 229/229. Note this does not yet make pydantic unreachable: `smpclient/__init__.py` still catches `pydantic.ValidationError`, which is intrinsic to smp 4.x and goes away with the `screaming-goblin` port (intercreate/smpmgr#103). pydantic is not declared in `pyproject.toml` and never was, so nothing changes there. Co-Authored-By: Claude Opus 5 (1M context) --- src/smpclient/mcuboot.py | 44 +++++++++++++++++-------------------- tests/test_mcuboot_tools.py | 32 ++++++++++++--------------- 2 files changed, 34 insertions(+), 42 deletions(-) diff --git a/src/smpclient/mcuboot.py b/src/smpclient/mcuboot.py index ebf17fc..78aa33b 100644 --- a/src/smpclient/mcuboot.py +++ b/src/smpclient/mcuboot.py @@ -8,15 +8,13 @@ import argparse import pathlib import struct +from dataclasses import dataclass from enum import IntEnum, IntFlag, unique from functools import cached_property from io import BufferedReader, BytesIO -from typing import Annotated, Any, Final, Generic, Literal, TypeVar, Union +from typing import Final, Generic, Literal, TypeVar from intelhex import hex2bin # type: ignore -from pydantic import Field, GetCoreSchemaHandler -from pydantic.dataclasses import dataclass -from pydantic_core import CoreSchema, core_schema ImageMagic = Literal[0x96F3B83D] IMAGE_MAGIC: Final[ImageMagic] = 0x96F3B83D @@ -146,31 +144,25 @@ def __new__(cls, value: int) -> 'VendorTLV': ) return int.__new__(cls, value) - @classmethod - def __get_pydantic_core_schema__( - cls, _source_type: Any, _handler: GetCoreSchemaHandler - ) -> CoreSchema: - def validate(value: int) -> VendorTLV: - return cls(value) - return core_schema.no_info_after_validator_function( - validate, - core_schema.int_schema(), - ) - - -ImageTLVType = Annotated[Union[IMAGE_TLV, VendorTLV, int], Field(union_mode="left_to_right")] +ImageTLVType = IMAGE_TLV | VendorTLV | int """TLV type that accepts standard IMAGE_TLV enums, vendor-defined TLVs, or any integer. -This uses Pydantic's "left to right" union mode to: -1. First try to match against IMAGE_TLV enum values -2. Then try to validate as a VendorTLV (0xXXA0-0xXXFE ranges) -3. Finally accept any integer as a fallback - -This ensures backward compatibility and supports future TLV types without validation errors. +`ImageTLV` narrows a raw type field to the leftmost member that accepts it, so an +unrecognized type stays readable as an `int` instead of failing the parse. """ +def _narrow_tlv_type(value: int) -> ImageTLVType: + """Return `value` as the leftmost `ImageTLVType` that accepts it.""" + for tlv_type in (IMAGE_TLV, VendorTLV): + try: + return tlv_type(value) + except ValueError: + continue + return value + + @dataclass(frozen=True) class ImageVersion: """An MCUBoot image_version struct.""" @@ -223,7 +215,7 @@ def loads(data: bytes) -> ImageHeader: hdr_size=hdr_size, protect_tlv_size=protect_tlv_size, img_size=img_size, - flags=flags, + flags=IMAGE_F(flags), ver=ImageVersion(*ver), ) @@ -282,6 +274,10 @@ class ImageTLV: len: int """Data length (not including TLV header).""" + def __post_init__(self) -> None: + """Narrow `type` to the leftmost `ImageTLVType` that accepts it.""" + object.__setattr__(self, "type", _narrow_tlv_type(self.type)) + @staticmethod def load_from(file: BytesIO | BufferedReader) -> ImageTLV: """Load an `ImageTLV` from a file.""" diff --git a/tests/test_mcuboot_tools.py b/tests/test_mcuboot_tools.py index 3b2b8a8..2fbddeb 100644 --- a/tests/test_mcuboot_tools.py +++ b/tests/test_mcuboot_tools.py @@ -21,12 +21,12 @@ ImageTLVInfo, ImageTLVInfoMagic, ImageTLVProtInfoMagic, - ImageTLVType, ImageTLVValue, ImageVersion, MCUBootImageError, TLVNotFound, VendorTLV, + _narrow_tlv_type, mcuimg, ) @@ -195,24 +195,20 @@ def test_unknown_tlv_fallback() -> None: def test_tlv_type_union_order() -> None: """Test that union resolution follows left-to-right order.""" - from pydantic import TypeAdapter - - adapter: TypeAdapter[ImageTLVType] = TypeAdapter(ImageTLVType) - # Standard TLV should match IMAGE_TLV first - result = adapter.validate_python(0x02) - assert isinstance(result, IMAGE_TLV) - assert result == IMAGE_TLV.PUBKEY - - # Vendor TLV should validate - result = adapter.validate_python(0xA0) - assert isinstance(result, int) - assert result == 0xA0 - - # Unknown TLV should fallback to int - result = adapter.validate_python(0x99) - assert isinstance(result, int) - assert result == 0x99 + standard = _narrow_tlv_type(0x02) + assert isinstance(standard, IMAGE_TLV) + assert standard == IMAGE_TLV.PUBKEY + + # Vendor TLV should validate; VendorTLV rather than a bare int is the whole point + vendor = _narrow_tlv_type(0xA0) + assert isinstance(vendor, VendorTLV) + assert vendor == 0xA0 + + # Unknown TLV should fallback to int, matching neither of the narrower members + unknown = _narrow_tlv_type(0x99) + assert not isinstance(unknown, (IMAGE_TLV, VendorTLV)) + assert unknown == 0x99 def test_tlv_value_str_standard() -> None: