From 561de3d2b28e6632c2f20468d4d3fe63b8d8319e Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 30 Aug 2026 13:41:13 -0700 Subject: [PATCH 1/3] Surface descriptions to runtime module --- .../src/harp/device/schema/_emit.py | 81 +++++++++++++++---- 1 file changed, 66 insertions(+), 15 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 9411de9..5c40ea2 100644 --- a/src/packages/harp-device/src/harp/device/schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/schema/_emit.py @@ -44,7 +44,15 @@ from harp.device import core -from ._model import DeviceModel, PayloadMember, PayloadType, Register, Registers, Visibility +from ._model import ( + DeviceModel, + MaskValueItem, + PayloadMember, + PayloadType, + Register, + Registers, + Visibility, +) from ._naming import enum_member_name, field_name @@ -294,6 +302,27 @@ def _rename( return renamed # -- enums ------------------------------------------------------------ + @staticmethod + def _mask_value_doc(value: Any) -> Optional[str]: + """The per-value ``description``, when the mask value is the object form.""" + return value.root.description if isinstance(value.root, MaskValueItem) else None + + @staticmethod + def _document_enum( + enum_cls: Any, description: Optional[str], docs: Mapping[str, Optional[str]] + ) -> None: + """Set the enum class ``__doc__`` and every member's. + + Every member is assigned, even ``None``: an ``Enum`` member with no ``__doc__`` + of its own would otherwise read back the *class* docstring, since attribute + lookup falls through to it, which would misrepresent an undocumented member as + carrying the mask's own description. + """ + if description is not None: + enum_cls.__doc__ = description + for member_name, doc in docs.items(): + getattr(enum_cls, member_name).__doc__ = doc + def _build_enums(self) -> dict[str, Any]: # Enum type names stay verbatim; members take the SCREAMING_SNAKE of the generator. enums: dict[str, Any] = {} @@ -301,10 +330,22 @@ def _build_enums(self) -> dict[str, Any]: # IntFlag has no zero-valued member; drop it if present. bits = {k: v for k, v in spec.bits.items() if int(v) != 0} renamed = self._rename("bit", name, bits, enum_member_name) - enums[name] = enum.IntFlag(name, {renamed[k]: int(v) for k, v in bits.items()}) + enum_cls = enum.IntFlag(name, {renamed[k]: int(v) for k, v in bits.items()}) + self._document_enum( + enum_cls, + spec.description, + {renamed[k]: self._mask_value_doc(v) for k, v in bits.items()}, + ) + enums[name] = enum_cls for name, spec in self.group_masks.items(): renamed = self._rename("value", name, spec.values, enum_member_name) - enums[name] = enum.IntEnum(name, {renamed[k]: int(v) for k, v in spec.values.items()}) + enum_cls = enum.IntEnum(name, {renamed[k]: int(v) for k, v in spec.values.items()}) + self._document_enum( + enum_cls, + spec.description, + {renamed[k]: self._mask_value_doc(v) for k, v in spec.values.items()}, + ) + enums[name] = enum_cls return enums # -- converter resolution (one uniform factory pipeline) ------------- @@ -388,12 +429,18 @@ def _build_field(self, key: str, member: PayloadMember, reg: Register) -> Any: 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=group_mask, mask=mask, offset=offset, **default_kwarg) + field = GroupMask(enum=group_mask, mask=mask, offset=offset, **default_kwarg) + if member.description is not None: + field.__doc__ = member.description + return field field_kwargs: dict[str, Any] = {"offset": offset, **default_kwarg} if member.mask is not None: field_kwargs["mask"] = member.mask - return Field(self._resolve_converter(ctx), **field_kwargs) + field = Field(self._resolve_converter(ctx), **field_kwargs) + if member.description is not None: + field.__doc__ = member.description + return field # -- payloads --------------------------------------------------------- def _payload_name(self, name: str, reg: Register) -> str: @@ -477,19 +524,23 @@ def _build_register(self, name: str, class_name: str, reg: Register) -> type[Reg if reg.length is not None: # plain array register cls = _ARRAY_REGISTER[reg.type](reg.address, length=reg.length) cls.__name__ = cls.__qualname__ = class_name + if reg.description is not None: + cls.__doc__ = reg.description return cls - return _new_class(class_name, (_SCALAR_REGISTER[reg.type],), {"address": reg.address}) + namespace: dict[str, Any] = {"address": reg.address} + if reg.description is not None: + namespace["__doc__"] = reg.description + return _new_class(class_name, (_SCALAR_REGISTER[reg.type],), namespace) payload_cls = self._build_payload(name, reg) - return _new_class( - class_name, - (RegisterBase,), - { - "address": reg.address, - "payload_type": ProtoPayloadType[reg.type.name], - "payload_class": payload_cls, - }, - ) + namespace = { + "address": reg.address, + "payload_type": ProtoPayloadType[reg.type.name], + "payload_class": payload_cls, + } + if reg.description is not None: + namespace["__doc__"] = reg.description + return _new_class(class_name, (RegisterBase,), namespace) def emit(self) -> dict[str, type[RegisterBase[Any]]]: emitted: dict[str, type[RegisterBase[Any]]] = {} From 1efffe319a0f65c4f12e2cfe6d5dd916e37ab099 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 30 Aug 2026 19:56:14 -0700 Subject: [PATCH 2/3] Add unit tests --- .../src/harp/device/schema/_emit.py | 6 +- tests/device/test_emit.py | 133 +++++++++++++++++- 2 files changed, 134 insertions(+), 5 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 5c40ea2..dde02ad 100644 --- a/src/packages/harp-device/src/harp/device/schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/schema/_emit.py @@ -430,16 +430,14 @@ def _build_field(self, key: str, member: PayloadMember, reg: Register) -> Any: full = (1 << (elem_size * 8)) - 1 mask = member.mask if member.mask is not None else full field = GroupMask(enum=group_mask, mask=mask, offset=offset, **default_kwarg) - if member.description is not None: - field.__doc__ = member.description + field.__doc__ = member.description return field field_kwargs: dict[str, Any] = {"offset": offset, **default_kwarg} if member.mask is not None: field_kwargs["mask"] = member.mask field = Field(self._resolve_converter(ctx), **field_kwargs) - if member.description is not None: - field.__doc__ = member.description + field.__doc__ = member.description return field # -- payloads --------------------------------------------------------- diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index d8ddd16..b3ecb73 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -3,7 +3,7 @@ import numpy as np import pytest from harp.data import parse_to_dataframe -from harp.protocol import GroupMask, HarpMessage +from harp.protocol import Field, GroupMask, HarpMessage from harp.device import core from harp.device.schema._emit import ( @@ -595,3 +595,134 @@ def test_emitted_register_bulk_matches_oracle(name, device_registers): df_oracle = parse_to_dataframe(oracle, buf, time_index=False) assert list(df_emitted.columns) == list(df_oracle.columns) assert df_emitted.equals(df_oracle) + + +# --------------------------------------------------------------------------- +# Schema descriptions surfacing as __doc__ on the emitted runtime objects +# --------------------------------------------------------------------------- + +_DESCRIBED_YML = ( + "registers:\n" + " Scalar:\n" + " address: 40\n" + " type: U16\n" + " access: Read\n" + " description: A scalar register.\n" + " Array:\n" + " address: 41\n" + " type: U16\n" + " length: 2\n" + " access: Read\n" + " description: An array register.\n" + " Undescribed:\n" + " address: 42\n" + " type: U16\n" + " access: Read\n" + " StartPulse:\n" + " address: 100\n" + " type: U16\n" + " access: Write\n" + " description: Starts a PWM pulse.\n" + " payloadSpec:\n" + " DigitalOutput:\n" + " offset: 0\n" + " mask: 0xC00\n" + " maskType: PwmPort\n" + " description: Which output channel drives the pulse.\n" + " PulseWidth:\n" + " offset: 0\n" + " mask: 0x3FF\n" + " interfaceType: ushort\n" + " Indicators:\n" + " address: 43\n" + " type: U8\n" + " access: Write\n" + " maskType: PortDigitalIOS\n" + "groupMasks:\n" + " PwmPort:\n" + " description: Which PWM channel is selected.\n" + " values:\n" + " Pwm0:\n" + " value: 0x1\n" + " description: PWM channel 0.\n" + " Pwm1: 0x2\n" + "bitMasks:\n" + " PortDigitalIOS:\n" + " description: Bitmask of digital IO ports.\n" + " bits:\n" + " DIO0:\n" + " value: 0x1\n" + " description: Digital output 0.\n" + " DIO1: 0x2\n" +) + + +@pytest.fixture +def described_registers(): + return create_registers(_DESCRIBED_YML) + + +def test_scalar_register_description_becomes_class_doc(described_registers): + assert described_registers["Scalar"].__doc__ == "A scalar register." + + +def test_array_register_description_becomes_class_doc(described_registers): + assert described_registers["Array"].__doc__ == "An array register." + + +def test_payload_wrapped_register_description_becomes_class_doc(described_registers): + assert described_registers["StartPulse"].__doc__ == "Starts a PWM pulse." + + +def test_absent_register_description_leaves_doc_none(described_registers): + assert described_registers["Undescribed"].__doc__ is None + + +def test_payload_member_description_becomes_field_doc(described_registers): + # DigitalOutput has a maskType, so it is emitted as a GroupMask descriptor. + payload_cls = described_registers["StartPulse"].payload_class + field = payload_cls.__dict__["digital_output"] + assert field.__doc__ == "Which output channel drives the pulse." + + +def test_undescribed_payload_member_does_not_inherit_descriptor_class_doc(described_registers): + # Regression: an unset instance __doc__ falls back to the owning class's, which + # would misrepresent PulseWidth (no description) as carrying Field's own generic + # docstring. + payload_cls = described_registers["StartPulse"].payload_class + field = payload_cls.__dict__["pulse_width"] + assert field.__doc__ is None + assert field.__doc__ != Field.__doc__ + + +def test_group_mask_type_description_becomes_enum_doc(described_registers): + enum_cls = _enum_of(described_registers["StartPulse"], "digital_output") + assert enum_cls.__doc__ == "Which PWM channel is selected." + + +def test_bit_mask_type_description_becomes_enum_doc(described_registers): + enum_cls = _enum_of(described_registers["Indicators"], "__value__") + assert enum_cls.__doc__ == "Bitmask of digital IO ports." + + +def test_bit_mask_value_description_becomes_member_doc(described_registers): + enum_cls = _enum_of(described_registers["Indicators"], "__value__") + assert enum_cls.DIO0.__doc__ == "Digital output 0." + + +def test_group_mask_value_description_becomes_member_doc(described_registers): + enum_cls = _enum_of(described_registers["StartPulse"], "digital_output") + assert enum_cls.PWM0.__doc__ == "PWM channel 0." + + +def test_undescribed_bit_mask_value_does_not_inherit_enum_type_doc(described_registers): + # Regression: Enum attribute lookup on a member with no __doc__ of its own falls + # back to the class __doc__, which would misrepresent DIO1 as carrying the + # bitmask's own description once PortDigitalIOS.__doc__ is set. + enum_cls = _enum_of(described_registers["Indicators"], "__value__") + assert enum_cls.DIO1.__doc__ is None + + +def test_undescribed_group_mask_value_does_not_inherit_enum_type_doc(described_registers): + enum_cls = _enum_of(described_registers["StartPulse"], "digital_output") + assert enum_cls.PWM1.__doc__ is None From 3feb1a75952873c61251d7df5c62742395012844 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:02:18 -0700 Subject: [PATCH 3/3] Simplify register docstring logic --- .../src/harp/device/schema/_emit.py | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 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 dde02ad..e622a18 100644 --- a/src/packages/harp-device/src/harp/device/schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/schema/_emit.py @@ -311,15 +311,14 @@ def _mask_value_doc(value: Any) -> Optional[str]: def _document_enum( enum_cls: Any, description: Optional[str], docs: Mapping[str, Optional[str]] ) -> None: - """Set the enum class ``__doc__`` and every member's. + """Set the enum class ``__doc__`` and every member's, ``None`` included. - Every member is assigned, even ``None``: an ``Enum`` member with no ``__doc__`` - of its own would otherwise read back the *class* docstring, since attribute - lookup falls through to it, which would misrepresent an undocumented member as - carrying the mask's own description. + A member is always assigned, even when it has no description: an ``Enum`` + member with no ``__doc__`` of its own falls back to the *class* docstring via + attribute lookup, which would misrepresent it as carrying the mask's own + description. """ - if description is not None: - enum_cls.__doc__ = description + enum_cls.__doc__ = description for member_name, doc in docs.items(): getattr(enum_cls, member_name).__doc__ = doc @@ -522,23 +521,25 @@ def _build_register(self, name: str, class_name: str, reg: Register) -> type[Reg if reg.length is not None: # plain array register cls = _ARRAY_REGISTER[reg.type](reg.address, length=reg.length) cls.__name__ = cls.__qualname__ = class_name - if reg.description is not None: - cls.__doc__ = reg.description + cls.__doc__ = reg.description return cls - namespace: dict[str, Any] = {"address": reg.address} - if reg.description is not None: - namespace["__doc__"] = reg.description - return _new_class(class_name, (_SCALAR_REGISTER[reg.type],), namespace) + return _new_class( + class_name, + (_SCALAR_REGISTER[reg.type],), + {"address": reg.address, "__doc__": reg.description}, + ) payload_cls = self._build_payload(name, reg) - namespace = { - "address": reg.address, - "payload_type": ProtoPayloadType[reg.type.name], - "payload_class": payload_cls, - } - if reg.description is not None: - namespace["__doc__"] = reg.description - return _new_class(class_name, (RegisterBase,), namespace) + return _new_class( + class_name, + (RegisterBase,), + { + "address": reg.address, + "payload_type": ProtoPayloadType[reg.type.name], + "payload_class": payload_cls, + "__doc__": reg.description, + }, + ) def emit(self) -> dict[str, type[RegisterBase[Any]]]: emitted: dict[str, type[RegisterBase[Any]]] = {}