From 28685af2954198eeb25c2afc9c98d2cc2b544c91 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Fri, 14 Aug 2026 21:14:00 +0100 Subject: [PATCH 1/7] Name enums and payload classes in runtime modules A module built from a schema now names the enums and payload classes its registers are built from, so it holds the same declarations as a generated device package. Each declaration resolves as type[Any] rather than type[RegisterBase[Any]], since the module holds three kinds of class. Parsing a schema that declares one name as both a register and a mask raises a validation error. --- .../src/harp/device/schema/_model.py | 21 ++++++++++- .../src/harp/device/schema/_module.py | 36 ++++++++++--------- tests/conformance.py | 2 +- tests/device/test_create_device_module.py | 19 +++++----- tests/device/test_schema.py | 19 ++++++++++ 5 files changed, 71 insertions(+), 26 deletions(-) diff --git a/src/packages/harp-device/src/harp/device/schema/_model.py b/src/packages/harp-device/src/harp/device/schema/_model.py index 1ab7076..14d6271 100644 --- a/src/packages/harp-device/src/harp/device/schema/_model.py +++ b/src/packages/harp-device/src/harp/device/schema/_model.py @@ -7,7 +7,7 @@ from enum import Enum from typing import Annotated, Dict, List, Optional, Union -from pydantic import BaseModel, ConfigDict, Field, RootModel +from pydantic import BaseModel, ConfigDict, Field, RootModel, model_validator class PayloadType(str, Enum): @@ -225,6 +225,25 @@ class Registers(BaseModel): ), ) + @model_validator(mode="after") + def _names_are_distinct(self) -> "Registers": + """Every generator target renders registers and masks into one namespace.""" + declared = ( + ("register", self.registers), + ("bit mask", self.bitMasks or {}), + ("group mask", self.groupMasks or {}), + ) + seen: Dict[str, str] = {} + for kind, names in declared: + for name in names: + if name in seen: + raise ValueError( + f"{name!r} is declared as both a {seen[name]} and a {kind}; " + f"rename one of them" + ) + seen[name] = kind + return self + class DeviceModel(Registers): """A device schema: a `Registers` collection plus optional device identity. diff --git a/src/packages/harp-device/src/harp/device/schema/_module.py b/src/packages/harp-device/src/harp/device/schema/_module.py index 9c0ed04..2b2f45a 100644 --- a/src/packages/harp-device/src/harp/device/schema/_module.py +++ b/src/packages/harp-device/src/harp/device/schema/_module.py @@ -13,7 +13,7 @@ from harp.protocol import RegisterBase from harp.device.core import REGISTER_MAP as CORE_REGISTER_MAP -from ._emit import ConverterValue, create_registers, parse_device_schema +from ._emit import ConverterValue, _Emitter, parse_device_schema _DEFAULT_NAME = "Device" """Module name used when the schema carries no ``device`` header.""" @@ -37,11 +37,12 @@ class DeviceModuleLike(Protocol): class DeviceModule(types.ModuleType): """The module :func:`create_device_module` returns, describing what a device module holds. - Declaring the members is what lets a linter resolve them. Register names come + Declaring the members is what lets a linter resolve them. Declaration names come from the schema, so they can only be described collectively, through :meth:`__getattr__`; ``REGISTER_MAP`` and ``WHO_AM_I`` are named and keep their - own types. A statically generated device package is a plain module and needs - none of this, since its registers are written out. + own types. The module holds registers, enums and payload classes alike, so the + only type they share is being a class. A statically generated device package is a + plain module and needs none of this, since its declarations are written out. """ REGISTER_MAP: dict[int, type[RegisterBase[Any]]] @@ -50,8 +51,8 @@ class DeviceModule(types.ModuleType): WHO_AM_I: int """The device identity declared by the schema. ``0`` when absent.""" - def __getattr__(self, name: str) -> type[RegisterBase[Any]]: - raise AttributeError(f"module {self.__name__!r} has no register named {name!r}") + def __getattr__(self, name: str) -> type[Any]: + raise AttributeError(f"module {self.__name__!r} has no declaration named {name!r}") def create_device_module( @@ -64,9 +65,13 @@ def create_device_module( ) -> DeviceModule: """Emit a module of register classes from ``device.yml`` text. - The module names the registers the schema declares, so ``behavior.AnalogData`` - resolves while a common register such as ``WhoAmI`` is imported from - :mod:`harp.device`, keeping one definition of each. Beside them it holds: + The module names what the schema declares, its registers beside the enums and + payload classes they are built from, so ``behavior.AnalogData``, + ``behavior.AnalogDataPayload`` and ``behavior.EncoderModeMask`` all resolve while a + common register such as ``WhoAmI`` is imported from :mod:`harp.device.core`, keeping + one definition of each. This is the same set a generated device package holds. A + name describing two declarations is rejected rather than shadowed. Beside them + it holds: * ``REGISTER_MAP``, the device address space, so the common registers are present here even though the module does not name them; @@ -75,7 +80,7 @@ def create_device_module( (``"Device"`` for a header-less register fragment). Because the names come from the schema at runtime they don't autocomplete, and - each resolves as ``type[RegisterBase[Any]]`` rather than its own register type; + each resolves as ``type[Any]`` rather than its own type; a generated device package is a real module on disk and gives both. On an address clash the device register replaces the common one in ``REGISTER_MAP``. ``exclude_private=True`` drops registers whose DSL ``visibility`` is ``private``. @@ -89,17 +94,16 @@ def create_device_module( behavior.AnalogData """ device = parse_device_schema(text) - registers = create_registers( - device, converters=converters, strict=strict, exclude_private=exclude_private - ) + emitter = _Emitter(device, converters, strict, exclude_private) + registers = emitter.emit() module_name = name or device.device or _DEFAULT_NAME - contents: dict[str, type[RegisterBase[Any]]] = dict(registers) + contents: dict[str, Any] = {**emitter.enums, **emitter.payloads, **registers} register_map = {cls.address: cls for cls in CORE_REGISTER_MAP.values()} register_map.update({cls.address: cls for cls in registers.values()}) - for register in registers.values(): - register.__module__ = module_name + for declaration in contents.values(): + declaration.__module__ = module_name module = DeviceModule(module_name, f"Harp registers for {module_name}, from a schema.") vars(module).update( diff --git a/tests/conformance.py b/tests/conformance.py index c2df806..469dfc6 100644 --- a/tests/conformance.py +++ b/tests/conformance.py @@ -20,7 +20,7 @@ def schema_built_registers(yml: str) -> None: """A module built from a schema types its registers collectively.""" behavior = create_device_module(yml) assert_type(behavior, DeviceModule) - assert_type(behavior.AnalogData, type[RegisterBase[Any]]) + assert_type(behavior.AnalogData, type[Any]) assert_type(behavior.REGISTER_MAP, dict[int, type[RegisterBase[Any]]]) assert_type(behavior.WHO_AM_I, int) diff --git a/tests/device/test_create_device_module.py b/tests/device/test_create_device_module.py index 1517dd9..48fcc88 100644 --- a/tests/device/test_create_device_module.py +++ b/tests/device/test_create_device_module.py @@ -6,6 +6,7 @@ from harp.device.core import REGISTER_MAP as CORE_REGISTER_MAP from harp.device.core import WhoAmI from harp.device.schema import DeviceModule, DeviceModuleLike, create_device_module +from harp.protocol import RegisterBase from . import expected_device from .converters import DataConverter @@ -67,14 +68,15 @@ def test_core_registers_are_not_named_by_module(test_module): assert test_module.REGISTER_MAP[0] is WhoAmI -def test_module_names_exactly_schema_registers(test_module): +def test_module_names_schema_declarations(test_module): + # The module names what the schema declares, registers beside the enums and payload + # classes they are built from, matching what a generated device package holds. named = {n for n in vars(test_module) if not n.startswith("_") and n not in MODULE_CONSTANTS} + registers = {n for n in named if issubclass(getattr(test_module, n), RegisterBase)} addresses = {cls.address for cls in test_module.REGISTER_MAP.values()} - # Everything the module names is in the address space, and the map carries the - # common registers on top, which is the whole difference between the two. - assert all(getattr(test_module, n).address in addresses for n in named) + assert all(getattr(test_module, n).address in addresses for n in registers) + assert {"EncoderModeMask", "AnalogDataPayload"} <= named - registers assert {c.__name__ for c in CORE_REGISTER_MAP.values()}.isdisjoint(named) - assert len(test_module.REGISTER_MAP) > len(named) def test_emitted_module_matches_device_protocol(test_module): @@ -98,7 +100,7 @@ def test_common_registers_are_not_device_module(): def test_unknown_name_raises_attribute_error(test_module): # The message names the module and the register, since a schema-built module # cannot offer the name in an editor. - with pytest.raises(AttributeError, match="'Tests' has no register named 'Nonexistent'"): + with pytest.raises(AttributeError, match="'Tests' has no declaration named 'Nonexistent'"): _ = test_module.Nonexistent @@ -129,13 +131,14 @@ def test_headerless_fragment_builds_default_module(): assert mod.Foo.address == 40 -def test_all_covers_registers_and_module_constants(test_module): +def test_all_covers_declarations_and_module_constants(test_module): exported = set(test_module.__all__) assert {"REGISTER_MAP", "WHO_AM_I"} <= exported assert {"AnalogData", "EncoderMode"} <= exported + assert {"EncoderModeMask", "AnalogDataPayload"} <= exported assert "WhoAmI" not in exported # a common register is not re-exported assert exported - MODULE_CONSTANTS == { - cls.__name__ for cls in test_module.REGISTER_MAP.values() if cls.address >= 32 + n for n in vars(test_module) if not n.startswith("_") and n not in MODULE_CONSTANTS } diff --git a/tests/device/test_schema.py b/tests/device/test_schema.py index be39044..ab74d8d 100644 --- a/tests/device/test_schema.py +++ b/tests/device/test_schema.py @@ -1,3 +1,6 @@ +import pytest +from pydantic import ValidationError + from harp.device.schema import parse_device_schema from harp.device.schema._model import DeviceModel, PayloadType @@ -14,6 +17,22 @@ def test_parse_full_device(device_yml): assert list(ad.payloadSpec) == ["Analog0", "Analog1", "Analog2", "Accelerometer"] +def test_colliding_declaration_names_are_rejected(): + # Registers and masks are rendered into one namespace, so a name describing two of + # them would leave whichever came last and silently lose the other. + schema = ( + "device: Clash\n" + "registers:\n" + " Mode: {address: 32, type: U8, access: Read, maskType: Mode}\n" + "groupMasks:\n" + " Mode:\n" + " values:\n" + " Idle: {value: 0}\n" + ) + with pytest.raises(ValidationError, match="both a register and a group mask"): + parse_device_schema(schema) + + def test_parse_fragment_yields_null_device(): m = parse_device_schema("registers:\n Foo: {address: 40, type: U16, access: Read}\n") assert isinstance(m, DeviceModel) From 0ecb864b3535d47fd84056d6376024b06fa44bc2 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Sat, 15 Aug 2026 00:57:01 +0100 Subject: [PATCH 2/7] Resolve core mask types in runtime modules A register whose maskType names a mask the schema does not declare now resolves against the core register set, emitting the same enum a generated device package imports. Such a register would otherwise raise, so two of the published devices could not be built at runtime. --- .../src/harp/device/schema/_emit.py | 38 ++++++---- .../src/harp/device/schema/_module.py | 3 + tests/device/test_create_device_module.py | 16 +++++ tests/device/test_emit.py | 70 ++++++++++++++++++- 4 files changed, 114 insertions(+), 13 deletions(-) diff --git a/src/packages/harp-device/src/harp/device/schema/_emit.py b/src/packages/harp-device/src/harp/device/schema/_emit.py index b085ba4..b7b5d85 100644 --- a/src/packages/harp-device/src/harp/device/schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/schema/_emit.py @@ -41,9 +41,18 @@ from harp.protocol._payload import _reserved_field_reason from harp.protocol import PayloadType as ProtoPayloadType +from harp.device import core + from ._model import DeviceModel, PayloadMember, PayloadType, Register, Registers, Visibility from ._naming import enum_member_name, field_name + +_CORE_MASKS: dict[str, Any] = { + name: declaration + for name, declaration in vars(core).items() + if name in core.__all__ and isinstance(declaration, type) and issubclass(declaration, enum.Enum) +} + _ELEMENT: dict[PayloadType, type[np.generic]] = { PayloadType.U8: np.uint8, PayloadType.S8: np.int8, @@ -243,6 +252,9 @@ def __init__( # share one class, as the module-level payload list of the generator does. self.payloads: dict[str, type] = {} + def _find_mask(self, name: str) -> Any: + return self.enums.get(name) or _CORE_MASKS.get(name) + # -- naming ----------------------------------------------------------- def _rename( self, @@ -329,12 +341,12 @@ def _default(self, member: PayloadMember, type_name: str, ctx: ConverterContext) if _default_value is None or (member.length or 0) > 1: return _NO_DEFAULT value = float(_default_value.root) - if type_name in self.group_masks: - e = self.enums[type_name] - for mv in self.group_masks[type_name].values.values(): - if int(mv) == int(value): - return e(int(value)) - return int(value) + group_mask = self._find_mask(type_name) + if group_mask is not None and issubclass(group_mask, enum.IntEnum): + try: + return group_mask(int(value)) + except ValueError: + return int(value) if member.converter is not None: return _NO_DEFAULT # a custom converter owns its own decoding; no numeric default it = ctx.interface_type @@ -367,10 +379,11 @@ def _build_field(self, key: str, member: PayloadMember, reg: Register) -> Any: default_kwarg = {} if default is _NO_DEFAULT else {"default": default} # A group mask is an enum sub-field descriptor, not a Field(converter). - if type_name in self.group_masks: + group_mask = self._find_mask(type_name) + if group_mask is not None and issubclass(group_mask, enum.IntEnum): full = (1 << (elem_size * 8)) - 1 mask = member.mask if member.mask is not None else full - return GroupMask(enum=self.enums[type_name], mask=mask, offset=offset, **default_kwarg) + return GroupMask(enum=group_mask, mask=mask, offset=offset, **default_kwarg) field_kwargs: dict[str, Any] = {"offset": offset, **default_kwarg} if member.mask is not None: @@ -413,11 +426,12 @@ def _new_payload(self, class_name: str, owner: str, reg: Register) -> type: # anonymous single-value payload mt = reg.maskType.root if reg.maskType else None it = reg.interfaceType.root if reg.interfaceType else None - if mt in self.group_masks: + mask = self._find_mask(mt) if mt else None + if mask is not None and issubclass(mask, enum.IntFlag): + descriptor: Any = BitMask(enum=mask) + elif mask is not None: full = (1 << (elem_size * 8)) - 1 - descriptor: Any = GroupMask(enum=self.enums[mt], mask=full) - elif mt in self.bit_masks: - descriptor = BitMask(enum=self.enums[mt]) + descriptor = GroupMask(enum=mask, mask=full) else: assert it is not None, ( f"{owner}: register needs a payloadSpec, maskType, or interfaceType" diff --git a/src/packages/harp-device/src/harp/device/schema/_module.py b/src/packages/harp-device/src/harp/device/schema/_module.py index 2b2f45a..3a99dfc 100644 --- a/src/packages/harp-device/src/harp/device/schema/_module.py +++ b/src/packages/harp-device/src/harp/device/schema/_module.py @@ -51,6 +51,9 @@ class DeviceModule(types.ModuleType): WHO_AM_I: int """The device identity declared by the schema. ``0`` when absent.""" + __all__: list[str] + """The declarations of the schema, beside ``REGISTER_MAP`` and ``WHO_AM_I``.""" + def __getattr__(self, name: str) -> type[Any]: raise AttributeError(f"module {self.__name__!r} has no declaration named {name!r}") diff --git a/tests/device/test_create_device_module.py b/tests/device/test_create_device_module.py index 48fcc88..ccc2e0f 100644 --- a/tests/device/test_create_device_module.py +++ b/tests/device/test_create_device_module.py @@ -142,6 +142,22 @@ def test_all_covers_declarations_and_module_constants(test_module): } +def test_reused_core_masks_are_not_named_by_module(): + # A reused mask has one definition, in harp.device.core, so a device module resolves + # registers against it without naming it, as it does for the common registers. + mod = create_device_module( + "device: CoreMasks\n" + "registers:\n" + " EnableFlow: {address: 32, type: U8, access: Write, maskType: EnableFlag}\n" + ) + assert not hasattr(mod, "EnableFlag") + assert "EnableFlag" not in mod.__all__ + assert ( + mod.EnableFlow.payload_class._mro_descriptor("__value__")._enum + is harp.device.core.EnableFlag + ) + + def test_module_is_not_registered_in_sys_modules(test_module): # Two schemas may share a device name, so the module is handed back unbound. assert sys.modules.get(test_module.__name__) is not test_module diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index f741f3b..7a73639 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -3,8 +3,9 @@ import numpy as np import pytest from harp.data import parse_to_dataframe -from harp.protocol import HarpMessage +from harp.protocol import GroupMask, HarpMessage +from harp.device import core from harp.device.schema._emit import NameCollisionError, UnknownConverterError, create_registers from . import expected_core, expected_device @@ -202,6 +203,73 @@ def test_custom_converter_roundtrip(device_registers): assert int(parsed.data) == -1234 +# --------------------------------------------------------------------------- +# Core masks reused by a schema that does not declare them +# --------------------------------------------------------------------------- + +CORE_MASKS_YML = ( + "device: CoreMasks\n" + "registers:\n" + " EnableFlow: {address: 32, type: U8, access: Write, maskType: EnableFlag}\n" + " ResetFlow: {address: 33, type: U8, access: Write, maskType: ResetFlags}\n" + " FlowConfiguration:\n" + " address: 34\n" + " type: U8\n" + " access: Write\n" + " payloadSpec:\n" + " Indicators: {maskType: EnableFlag, mask: 0x1, defaultValue: 1}\n" +) + + +def _value_enum(reg): + return reg.payload_class._mro_descriptor("__value__")._enum + + +def test_undeclared_group_mask_resolves_to_core(): + # Published devices reference EnableFlag without declaring it, so the emitter has + # to reuse the core definition rather than fail to type the register. + regs = create_registers(CORE_MASKS_YML) + assert _value_enum(regs["EnableFlow"]) is core.EnableFlag + + +def test_undeclared_bit_mask_resolves_to_core(): + regs = create_registers(CORE_MASKS_YML) + assert _value_enum(regs["ResetFlow"]) is core.ResetFlags + + +def test_reused_core_mask_roundtrips_as_the_core_type(): + # Identity matters more than equal members: a value read through a runtime module + # must satisfy isinstance against the same enum a generated package would use. + regs = create_registers(CORE_MASKS_YML) + value = core.ResetFlags.SAVE | core.ResetFlags.RESTORE_NAME + parsed = _roundtrip(regs["ResetFlow"], value) + assert isinstance(parsed, core.ResetFlags) + assert parsed == value + + +def test_reused_core_mask_resolves_on_payload_member(): + regs = create_registers(CORE_MASKS_YML) + descriptor = regs["FlowConfiguration"].payload_class._mro_descriptor("indicators") + assert isinstance(descriptor, GroupMask) + assert descriptor._enum is core.EnableFlag + assert descriptor._default is core.EnableFlag.ENABLED + + +def test_declared_mask_shadows_core_definition(): + regs = create_registers( + "registers:\n" + " Flow: {address: 32, type: U8, access: Read, maskType: EnableFlag}\n" + "groupMasks:\n" + " EnableFlag:\n" + " values:\n" + " Closed: {value: 0}\n" + " Open: {value: 1}\n" + ) + emitted = _value_enum(regs["Flow"]) + assert emitted is not core.EnableFlag + assert list(emitted.__members__) == ["CLOSED", "OPEN"] + + # --------------------------------------------------------------------------- # Converter registry # --------------------------------------------------------------------------- From 71b9c04a4cc1aef0f72ab61630da6bace10eafe7 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Sat, 15 Aug 2026 01:12:22 +0100 Subject: [PATCH 3/7] Emit private registers from a device schema A register whose visibility is private is now always emitted with the underscore-prefixed class name the generator gives it. This means a runtime module has the same address space as the generated package for the same schema. Reading such a register by address, or resolving one from a logged dataset, would otherwise fail on the runtime path alone. The exclude_private parameter is removed rather than redefaulted, since create_registers and create_device_module disagreed on its default and the same schema therefore gave different register sets depending on which was called. --- .../create_device_module/create_device_module.py | 4 ++-- .../harp-device/src/harp/device/schema/_emit.py | 15 +++++---------- .../harp-device/src/harp/device/schema/_module.py | 4 +--- tests/device/test_emit.py | 7 +++---- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/docs/examples/create_device_module/create_device_module.py b/docs/examples/create_device_module/create_device_module.py index fa593dd..0896da8 100644 --- a/docs/examples/create_device_module/create_device_module.py +++ b/docs/examples/create_device_module/create_device_module.py @@ -36,6 +36,6 @@ # behavior = schema.create_device_module(yml_text, converters={"DataConverter": DataConverter()}) # # An unresolved custom type raises `UnknownConverterError`. Pass `strict=False` to -# decode it natively instead. `exclude_private=True`, the default, drops registers -# marked `private` in the schema. For the parsed schema model rather than a module, +# decode it natively instead. A register marked `private` in the schema is emitted +# with an underscore-prefixed name. For the parsed schema model rather than a module, # `parse_device_schema(yml_text)` returns that directly. diff --git a/src/packages/harp-device/src/harp/device/schema/_emit.py b/src/packages/harp-device/src/harp/device/schema/_emit.py index b7b5d85..cf03fe1 100644 --- a/src/packages/harp-device/src/harp/device/schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/schema/_emit.py @@ -239,12 +239,10 @@ def __init__( device: Union[DeviceModel, Registers], converters: Optional[Mapping[str, ConverterValue]], strict: bool, - exclude_private: bool, ) -> None: self.device = device self.converters = dict(converters or {}) self.strict = strict - self.exclude_private = exclude_private self.group_masks = device.groupMasks or {} self.bit_masks = device.bitMasks or {} self.enums = self._build_enums() @@ -484,8 +482,6 @@ def _build_register(self, name: str, class_name: str, reg: Register) -> type[Reg def emit(self) -> dict[str, type[RegisterBase[Any]]]: emitted: dict[str, type[RegisterBase[Any]]] = {} for name, reg in self.device.registers.items(): - if self.exclude_private and reg.visibility is Visibility.private: - continue class_name = self._class_name(name, reg) emitted[class_name] = self._build_register(name, class_name, reg) return emitted @@ -512,7 +508,6 @@ def create_registers( *, converters: Optional[Mapping[str, ConverterValue]] = None, strict: bool = True, - exclude_private: bool = False, ) -> dict[str, type[RegisterBase[Any]]]: """Emit runtime register classes from a device schema. @@ -526,10 +521,10 @@ def create_registers( ``(ctx: ConverterContext) -> Converter`` that builds one from the DSL context. A custom type with no matching converter raises ``UnknownConverterError`` when ``strict`` (the default); ``strict=False`` - decodes it as its native element type instead. ``exclude_private=True`` drops - registers whose DSL ``visibility`` is ``private``; when kept, a private register - class is underscore-prefixed (``_Reserved0``). Note that the converter symbol for a - payload field derives from its *verbatim* yml key, not the renamed field. + decodes it as its native element type instead. A register whose DSL ``visibility`` + is ``private`` is emitted with an underscore-prefixed class (``_Reserved0``), as the + generator emits it. Note that the converter symbol for a payload field derives from + its *verbatim* yml key, not the renamed field. """ device = source if isinstance(source, Registers) else parse_device_schema(source) - return _Emitter(device, converters, strict, exclude_private).emit() + return _Emitter(device, converters, strict).emit() diff --git a/src/packages/harp-device/src/harp/device/schema/_module.py b/src/packages/harp-device/src/harp/device/schema/_module.py index 3a99dfc..f1732c0 100644 --- a/src/packages/harp-device/src/harp/device/schema/_module.py +++ b/src/packages/harp-device/src/harp/device/schema/_module.py @@ -64,7 +64,6 @@ def create_device_module( name: Optional[str] = None, converters: Optional[Mapping[str, ConverterValue]] = None, strict: bool = True, - exclude_private: bool = True, ) -> DeviceModule: """Emit a module of register classes from ``device.yml`` text. @@ -86,7 +85,6 @@ def create_device_module( each resolves as ``type[Any]`` rather than its own type; a generated device package is a real module on disk and gives both. On an address clash the device register replaces the common one in ``REGISTER_MAP``. - ``exclude_private=True`` drops registers whose DSL ``visibility`` is ``private``. ``text`` is the schema itself rather than a path to it, matching :func:`parse_device_schema`, so read the file first. The module is **not** @@ -97,7 +95,7 @@ def create_device_module( behavior.AnalogData """ device = parse_device_schema(text) - emitter = _Emitter(device, converters, strict, exclude_private) + emitter = _Emitter(device, converters, strict) registers = emitter.emit() module_name = name or device.device or _DEFAULT_NAME diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index 7a73639..5bcbd2e 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -317,11 +317,10 @@ def factory(ctx): ) -def test_exclude_private_drops_private_registers(): - # Kept by default. The class of a private register is underscore-prefixed, as the - # generator emits it. +def test_private_registers_are_emitted(): + # A private register stays in the address space, as it does in a generated package, + # since the device can still send it. assert set(create_registers(_VISIBILITY_YML)) == {"Pub", "_Priv"} - assert set(create_registers(_VISIBILITY_YML, exclude_private=True)) == {"Pub"} def test_private_register_class_is_underscore_prefixed(): From 93192271dd85d106c5b1097f54c89461a8de2197 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Sat, 15 Aug 2026 01:18:57 +0100 Subject: [PATCH 4/7] Report an unresolvable mask type as a schema error A maskType naming neither a declared nor a core mask now raises UnknownMaskError naming the register, the mask and the core set to choose from, rather than an assertion claiming the register declares no maskType at all. A register carrying a converter with nothing to convert to raises as well, which is the only case the assertion actually caught. Both survive python -O, whereas the assertion was stripped and the register decoded as its raw element type instead. --- .../src/harp/device/schema/_emit.py | 16 ++++++++++--- tests/device/test_emit.py | 23 ++++++++++++++++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/packages/harp-device/src/harp/device/schema/_emit.py b/src/packages/harp-device/src/harp/device/schema/_emit.py index cf03fe1..9f79e4f 100644 --- a/src/packages/harp-device/src/harp/device/schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/schema/_emit.py @@ -210,6 +210,10 @@ class UnknownConverterError(ValueError): """A custom ``interfaceType`` needs a converter not found in ``converters=``.""" +class UnknownMaskError(ValueError): + """A ``maskType`` names neither a mask the schema declares nor a core mask.""" + + class NameCollisionError(ValueError): """Two schema identifiers collapse to one Python name, or one shadows a reserved name. @@ -430,10 +434,16 @@ def _new_payload(self, class_name: str, owner: str, reg: Register) -> type: elif mask is not None: full = (1 << (elem_size * 8)) - 1 descriptor = GroupMask(enum=mask, mask=full) - else: - assert it is not None, ( - f"{owner}: register needs a payloadSpec, maskType, or interfaceType" + elif mt is not None: + raise UnknownMaskError( + f"{owner}: maskType {mt!r} is neither declared by the schema nor a core " + f"mask; declare it or use one of {sorted(_CORE_MASKS)}" ) + elif it is None: + raise ValueError( + f"{owner}: register declares no payloadSpec, maskType, or interfaceType" + ) + else: ctx = ConverterContext( name="__value__", interface_type=it, diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index 5bcbd2e..1a677fb 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -6,7 +6,12 @@ from harp.protocol import GroupMask, HarpMessage from harp.device import core -from harp.device.schema._emit import NameCollisionError, UnknownConverterError, create_registers +from harp.device.schema._emit import ( + NameCollisionError, + UnknownConverterError, + UnknownMaskError, + create_registers, +) from . import expected_core, expected_device from .converters import DataConverter @@ -255,6 +260,22 @@ def test_reused_core_mask_resolves_on_payload_member(): assert descriptor._default is core.EnableFlag.ENABLED +def test_unresolvable_mask_type_is_rejected(): + # Naming the mask matters: the register does declare a maskType, so reporting that + # one is missing would send the reader looking in the wrong place. + with pytest.raises(UnknownMaskError, match="'EnableFlg' is neither declared"): + create_registers( + "registers:\n R: {address: 32, type: U8, access: Read, maskType: EnableFlg}\n" + ) + + +def test_register_with_nothing_to_decode_is_rejected(): + with pytest.raises(ValueError, match="declares no payloadSpec, maskType, or interfaceType"): + create_registers( + "registers:\n R: {address: 32, type: U8, access: Read, converter: Payload}\n" + ) + + def test_declared_mask_shadows_core_definition(): regs = create_registers( "registers:\n" From f16e6babfd4bdc25cec19d71d84a729c25c6fb82 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Sat, 15 Aug 2026 01:32:15 +0100 Subject: [PATCH 5/7] Document device module contract --- src/packages/harp-device/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index d0dcf0d..5ed1692 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -27,7 +27,7 @@ REGISTER_MAP = {**_CORE_REGISTER_MAP, 32: DigitalInputState, ...} This is the same structure `create_device_module` builds from a schema, so a device reads the same way whether it was generated ahead of time or compiled at runtime. A `WHO_AM_I` of `0` marks an unregistered device, used while a device is in development or outside the official registry, and identity checks are skipped for it. -A device module names only the registers its schema declares, so `REGISTER_MAP` is the device address space while the module namespace is what the device adds to it. The common registers have a single definition, in `harp.device.core`, and are not a device, so the core register set carries no `WHO_AM_I`. +A device module names only what its schema declares, the registers beside the enums and payload classes they are built from, so `REGISTER_MAP` is the device address space while the module namespace is what the device adds to it. The common registers and any core mask the schema reuses have a single definition, in `harp.device.core`, and are reached from there rather than through the device module. The core register set is not a device, so it carries no `WHO_AM_I`. Pass the module to `Device`, or to `open_serial_device`, to validate identity on open: @@ -45,7 +45,7 @@ A new transport is just an object implementing the `ITransport` protocol, with ` ## Generating registers from a `device.yml` -Without a pre-generated device package, `create_device_module` builds the same structure at runtime from Harp `device.yml` text: register classes at module level, a `REGISTER_MAP` beside them, and the identity declared by the schema as `WHO_AM_I`. Identifiers match a generated package name for name: register, enum, and payload class names come from the yml verbatim, payload fields are `snake_case`, and enum members are `SCREAMING_SNAKE_CASE`. +Without a pre-generated device package, `create_device_module` builds the same structure at runtime from Harp `device.yml` text: register, enum and payload classes at module level, a `REGISTER_MAP` beside them, and the identity declared by the schema as `WHO_AM_I`. Identifiers match a generated package name for name: register, enum, and payload class names come from the yml verbatim, payload fields are `snake_case`, and enum members are `SCREAMING_SNAKE_CASE`. A `maskType` the schema does not declare resolves against the core masks, and a register marked `private` is emitted with an underscore-prefixed name. ```python from pathlib import Path From 3dd20d15f1a7c008230ea58a69a848fc662014d9 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Sat, 15 Aug 2026 01:33:01 +0100 Subject: [PATCH 6/7] Resync the generator reference modules The reference copies of generated output carried neither __all__ nor the blank-line spacing the generator now emits, so parity ran against output three changes old. Refreshing them lets a test assert that a runtime module and the generated package publish an identical __all__ for the same schema. --- tests/device/expected_core.py | 47 +++++++++++++++++++++++ tests/device/expected_device.py | 34 ++++++++++++++++ tests/device/test_create_device_module.py | 6 +++ 3 files changed, 87 insertions(+) diff --git a/tests/device/expected_core.py b/tests/device/expected_core.py index 6684278..39970f4 100644 --- a/tests/device/expected_core.py +++ b/tests/device/expected_core.py @@ -21,21 +21,55 @@ ) +__all__ = [ + "ResetFlags", + "ClockConfigurationFlags", + "OperationMode", + "EnableFlag", + "OperationControlPayload", + "ResetDevicePayload", + "DeviceNamePayload", + "ClockConfigurationPayload", + "WhoAmI", + "HardwareVersionHigh", + "HardwareVersionLow", + "AssemblyVersion", + "CoreVersionHigh", + "CoreVersionLow", + "FirmwareVersionHigh", + "FirmwareVersionLow", + "TimestampSeconds", + "TimestampMicroseconds", + "OperationControl", + "ResetDevice", + "DeviceName", + "SerialNumber", + "ClockConfiguration", + "REGISTER_MAP", +] + + class ResetFlags(enum.IntFlag): """Specifies the behavior of the non-volatile registers when resetting the device.""" RESTORE_DEFAULT = 0x1 """The device will boot with all the registers reset to their default factory values.""" + RESTORE_EEPROM = 0x2 """The device will boot and restore all the registers to the values stored in non-volatile memory.""" + SAVE = 0x4 """The device will boot and save all the current register values to non-volatile memory.""" + RESTORE_NAME = 0x8 """The device will boot with the default device name.""" + UPDATE_FIRMWARE = 0x20 """The device will enter firmware update mode.""" + BOOT_FROM_DEFAULT = 0x40 """Specifies that the device has booted from default factory values.""" + BOOT_FROM_EEPROM = 0x80 """Specifies that the device has booted from non-volatile values stored in EEPROM.""" @@ -45,14 +79,19 @@ class ClockConfigurationFlags(enum.IntFlag): CLOCK_REPEATER = 0x1 """The device will repeat the clock synchronization signal to the clock output connector, if available.""" + CLOCK_GENERATOR = 0x2 """The device resets and generates the clock synchronization signal on the clock output connector, if available.""" + REPEATER_CAPABILITY = 0x8 """Specifies the device has the capability to repeat the clock synchronization signal to the clock output connector.""" + GENERATOR_CAPABILITY = 0x10 """Specifies the device has the capability to generate the clock synchronization signal to the clock output connector.""" + CLOCK_UNLOCK = 0x40 """The device will unlock the timestamp register counter and will accept commands to set new timestamp values.""" + CLOCK_LOCK = 0x80 """The device will lock the timestamp register counter and will not accept commands to set new timestamp values.""" @@ -62,8 +101,10 @@ class OperationMode(enum.IntEnum): STANDBY = 0 """Disable all event reporting on the device.""" + ACTIVE = 1 """Event detection is enabled. Only enabled events are reported by the device.""" + SPEED = 3 """The device enters speed mode.""" @@ -73,6 +114,7 @@ class EnableFlag(enum.IntEnum): DISABLED = 0 """Specifies that the flag is disabled.""" + ENABLED = 1 """Specifies that the flag is enabled.""" @@ -82,14 +124,19 @@ class OperationControlPayload(StructPayload[np.uint8]): operation_mode: OperationMode = GroupMask(enum=OperationMode, mask=0x3) """Specifies the operation mode of the device.""" + dump_registers: bool = Field(BoolConverter(), mask=0x8) """Specifies whether the device should report the content of all registers on initialization.""" + mute_replies: bool = Field(BoolConverter(), mask=0x10) """Specifies whether the replies to all commands will be muted, i.e. not sent by the device.""" + visual_indicators: EnableFlag = GroupMask(enum=EnableFlag, mask=0x20) """Specifies the state of all visual indicators on the device.""" + operation_led: EnableFlag = GroupMask(enum=EnableFlag, mask=0x40) """Specifies whether the device state LED should report the operation mode of the device.""" + heartbeat: EnableFlag = GroupMask(enum=EnableFlag, mask=0x80) """Specifies whether the device should report the content of the seconds register each second.""" diff --git a/tests/device/expected_device.py b/tests/device/expected_device.py index d32ad88..db3371c 100644 --- a/tests/device/expected_device.py +++ b/tests/device/expected_device.py @@ -30,6 +30,40 @@ ) +__all__ = [ + "WHO_AM_I", + "PortDigitalIOS", + "PwmPort", + "EncoderModeMask", + "AnalogDataPayload", + "ComplexConfigurationPayload", + "VersionPayload", + "CustomPayloadPayload", + "CustomRawPayloadPayload", + "CustomMemberConverterPayload", + "BitmaskSplitterPayload", + "PortDIOSetPayload", + "StartPulsePayload", + "StartPulseTrainPayload", + "EncoderModePayload", + "DigitalInputs", + "AnalogData", + "ComplexConfiguration", + "Version", + "CustomPayload", + "CustomRawPayload", + "CustomMemberConverter", + "BitmaskSplitter", + "Counter0", + "PortDIOSet", + "PulseDOPort0", + "PulseDO0", + "StartPulse", + "StartPulseTrain", + "EncoderMode", + "REGISTER_MAP", +] + WHO_AM_I: int = 0 diff --git a/tests/device/test_create_device_module.py b/tests/device/test_create_device_module.py index ccc2e0f..ad0925d 100644 --- a/tests/device/test_create_device_module.py +++ b/tests/device/test_create_device_module.py @@ -142,6 +142,12 @@ def test_all_covers_declarations_and_module_constants(test_module): } +def test_all_matches_the_generated_package(test_module): + # expected_device is generator output for the same schema, so this pins that both + # paths publish exactly the same surface, not merely equivalent registers. + assert set(test_module.__all__) == set(expected_device.__all__) + + def test_reused_core_masks_are_not_named_by_module(): # A reused mask has one definition, in harp.device.core, so a device module resolves # registers against it without naming it, as it does for the common registers. From 4e906b0462abd9d39bdc922844d848bbd4e3b360 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Sat, 15 Aug 2026 19:38:31 +0100 Subject: [PATCH 7/7] Remove the DeviceModule attribute fallback DeviceModule no longer overrides __getattr__. The standard library type stubs already declare it on types.ModuleType returning Any. --- .../src/harp/device/schema/_module.py | 22 +++++++------------ tests/conformance.py | 2 +- tests/device/test_create_device_module.py | 5 ++--- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/packages/harp-device/src/harp/device/schema/_module.py b/src/packages/harp-device/src/harp/device/schema/_module.py index f1732c0..4c1f422 100644 --- a/src/packages/harp-device/src/harp/device/schema/_module.py +++ b/src/packages/harp-device/src/harp/device/schema/_module.py @@ -35,14 +35,11 @@ class DeviceModuleLike(Protocol): class DeviceModule(types.ModuleType): - """The module :func:`create_device_module` returns, describing what a device module holds. - - Declaring the members is what lets a linter resolve them. Declaration names come - from the schema, so they can only be described collectively, through - :meth:`__getattr__`; ``REGISTER_MAP`` and ``WHO_AM_I`` are named and keep their - own types. The module holds registers, enums and payload classes alike, so the - only type they share is being a class. A statically generated device package is a - plain module and needs none of this, since its declarations are written out. + """The type of the module returned by :func:`create_device_module`. + + The declarations of the schema are reached by name and typed ``Any``, since they + exist only at runtime. ``REGISTER_MAP``, ``WHO_AM_I`` and ``__all__`` are declared + here and carry their own types. """ REGISTER_MAP: dict[int, type[RegisterBase[Any]]] @@ -54,9 +51,6 @@ class DeviceModule(types.ModuleType): __all__: list[str] """The declarations of the schema, beside ``REGISTER_MAP`` and ``WHO_AM_I``.""" - def __getattr__(self, name: str) -> type[Any]: - raise AttributeError(f"module {self.__name__!r} has no declaration named {name!r}") - def create_device_module( text: str | bytes, @@ -82,9 +76,9 @@ def create_device_module( (``"Device"`` for a header-less register fragment). Because the names come from the schema at runtime they don't autocomplete, and - each resolves as ``type[Any]`` rather than its own type; - a generated device package is a real module on disk and gives both. On an - address clash the device register replaces the common one in ``REGISTER_MAP``. + each resolves as ``Any`` rather than its own type. A generated device package is + a real module on disk and gives both. On an address clash the device register + replaces the common one in ``REGISTER_MAP``. ``text`` is the schema itself rather than a path to it, matching :func:`parse_device_schema`, so read the file first. The module is **not** diff --git a/tests/conformance.py b/tests/conformance.py index 469dfc6..bea71e7 100644 --- a/tests/conformance.py +++ b/tests/conformance.py @@ -20,7 +20,7 @@ def schema_built_registers(yml: str) -> None: """A module built from a schema types its registers collectively.""" behavior = create_device_module(yml) assert_type(behavior, DeviceModule) - assert_type(behavior.AnalogData, type[Any]) + assert_type(behavior.AnalogData, Any) assert_type(behavior.REGISTER_MAP, dict[int, type[RegisterBase[Any]]]) assert_type(behavior.WHO_AM_I, int) diff --git a/tests/device/test_create_device_module.py b/tests/device/test_create_device_module.py index ad0925d..5fa8653 100644 --- a/tests/device/test_create_device_module.py +++ b/tests/device/test_create_device_module.py @@ -98,9 +98,8 @@ def test_common_registers_are_not_device_module(): def test_unknown_name_raises_attribute_error(test_module): - # The message names the module and the register, since a schema-built module - # cannot offer the name in an editor. - with pytest.raises(AttributeError, match="'Tests' has no declaration named 'Nonexistent'"): + # A name not declared by the schema must raise rather than silently resolve. + with pytest.raises(AttributeError, match="Nonexistent"): _ = test_module.Nonexistent