From 359763793c237526cf72df0ecce8a9242bcb3293 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:20:18 -0700 Subject: [PATCH 1/8] Add missing reserved field names to pass to the runtime generator --- .../src/harp/protocol/__init__.py | 2 ++ .../src/harp/protocol/_payload.py | 20 +++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/packages/harp-protocol/src/harp/protocol/__init__.py b/src/packages/harp-protocol/src/harp/protocol/__init__.py index abce77c..741e174 100644 --- a/src/packages/harp-protocol/src/harp/protocol/__init__.py +++ b/src/packages/harp-protocol/src/harp/protocol/__init__.py @@ -10,6 +10,7 @@ HarpVersion, ) from ._payload import ( + RESERVED_FIELD_NAMES, PayloadBase, StructPayload, Field, @@ -88,6 +89,7 @@ "PayloadBase", "StructPayload", "AnonymousPayload", + "RESERVED_FIELD_NAMES", "Column", "Field", "GroupMask", diff --git a/src/packages/harp-protocol/src/harp/protocol/_payload.py b/src/packages/harp-protocol/src/harp/protocol/_payload.py index 896ff2a..359a49c 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_payload.py +++ b/src/packages/harp-protocol/src/harp/protocol/_payload.py @@ -524,7 +524,23 @@ def __getattr__(self, name: str) -> "NDArray[Any]": ... # type: ignore[empty-bo # value/raw_payload deliberately omitted: overriding them is the intended # pattern for single-slot converter-driven payloads. -_RESERVED_FIELD_NAMES = frozenset({"_arr", "_dtype", "_repr_fields", "Batch"}) +#: Attribute names :class:`PayloadBase` owns; a field may not shadow one. Exposed so +#: code that derives field names from an external schema can reject a clash up front +#: with a message naming the original identifier. +RESERVED_FIELD_NAMES = frozenset( + { + "_arr", + "_dtype", + "_repr_fields", + "Batch", + "dtype", + "_scalar_cls", + "_batch_cls", + "_defaults", + "_elem_dtype", + "_single_member", + } +) def _batch_init_disabled(self: "PayloadBase", *args: object, **kwargs: object) -> None: @@ -766,7 +782,7 @@ def __init_subclass__( cls._single_member = None for name, val in cls.__dict__.items(): - if isinstance(val, _DECLARATION_TYPES) and name in _RESERVED_FIELD_NAMES: + if isinstance(val, _DECLARATION_TYPES) and name in RESERVED_FIELD_NAMES: raise TypeError(f"{cls.__name__}: field name {name!r} is reserved by PayloadBase") own_declarations = [ From f553f6621a70e56f2349a4fc3677e2e442e0fd58 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:35:29 -0700 Subject: [PATCH 2/8] Add naming processors to mimic upstream generators package --- .../create_device_module.md | 9 +- src/packages/harp-device/README.md | 4 +- .../src/harp/device/_schema/__init__.py | 5 + .../src/harp/device/_schema/_emit.py | 145 ++++++++++++++---- .../src/harp/device/_schema/_naming.py | 80 ++++++++++ 5 files changed, 207 insertions(+), 36 deletions(-) create mode 100644 src/packages/harp-device/src/harp/device/_schema/_naming.py diff --git a/docs/examples/create_device_module/create_device_module.md b/docs/examples/create_device_module/create_device_module.md index 5eb61e7..6a4721b 100644 --- a/docs/examples/create_device_module/create_device_module.md +++ b/docs/examples/create_device_module/create_device_module.md @@ -25,8 +25,9 @@ convenience. It's worth understanding what that buys you and what it costs. or keep in sync with the schema. - **Coverage for any device.** You don't need a published package for the device; unreleased, custom, or one-off schemas work immediately. -- **The schema stays the single source of truth.** Register, field, and enum names - come straight from the `device.yml`, verbatim. +- **The schema stays the single source of truth.** Registers, fields, and enums come + straight from the `device.yml`, under the same naming convention a generated + package uses — so code written against either lines up name for name. **You give up:** @@ -34,10 +35,6 @@ convenience. It's worth understanding what that buys you and what it costs. so an editor can't offer them and a type checker can't verify them. A generated package is a real module on disk, so both work. The module also isn't in `sys.modules`, so you bind it yourself rather than `import`-ing it. -- **Generator naming conventions.** Identifiers are kept verbatim from the yml - (`AnalogInput0`, `DIO0`) rather than the C# generator's snake_case fields and - `UPPER_SNAKE` enum members, so code written against a generated package won't line - up name-for-name. - **Turn-key custom types.** A custom `interfaceType` must be injected yourself via `converters=` (see below), whereas a generated package ships its own converters. diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index fe70936..14cfa0e 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -66,7 +66,9 @@ A new transport is just an object implementing the `ITransport` protocol 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`. -Field and enum names come from the yml verbatim. +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`. ```python from pathlib import Path diff --git a/src/packages/harp-device/src/harp/device/_schema/__init__.py b/src/packages/harp-device/src/harp/device/_schema/__init__.py index cc41ac3..75f066f 100644 --- a/src/packages/harp-device/src/harp/device/_schema/__init__.py +++ b/src/packages/harp-device/src/harp/device/_schema/__init__.py @@ -17,10 +17,12 @@ ConverterContext, ConverterFactory, ConverterValue, + NameCollisionError, UnknownConverterError, create_registers, parse_device_schema, ) +from ._naming import enum_member_name, field_name __all__ = [ "parse_device_schema", @@ -28,7 +30,10 @@ "ConverterContext", "ConverterFactory", "ConverterValue", + "NameCollisionError", "UnknownConverterError", + "enum_member_name", + "field_name", "DeviceModel", "Registers", "Register", 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 966149f..6aa3d16 100644 --- a/src/packages/harp-device/src/harp/device/_schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/_schema/_emit.py @@ -1,7 +1,8 @@ import enum import types from dataclasses import dataclass -from typing import Any, Callable, Mapping, Optional, Union +from typing import Any, Callable, Iterable, Mapping, Optional, Union, cast + import numpy as np from typing_extensions import Sentinel @@ -36,10 +37,13 @@ RegisterU64Array, StringConverter, StructPayload, + PayloadBase, ) +from harp.protocol import RESERVED_FIELD_NAMES from harp.protocol import PayloadType as ProtoPayloadType from ._model import DeviceModel, PayloadMember, PayloadType, Register, Registers, Visibility +from ._naming import enum_member_name, field_name # Register base element: schema PayloadType -> numpy scalar type (byte size via np.dtype). _ELEMENT: dict[PayloadType, type[np.generic]] = { @@ -191,6 +195,15 @@ class UnknownConverterError(ValueError): """A custom ``interfaceType`` needs a converter not found in ``converters=``.""" +class NameCollisionError(ValueError): + """Two schema identifiers collapse to one Python name, or one shadows a reserved name. + + Casing is not significant to the generator naming convention, so distinct yml + keys (``DIO0`` / ``Dio0``) can converge — which would silently alias an enum + member or drop a payload field. + """ + + def _is_native(interface_type: Optional[str]) -> bool: """True when a value decodes as a native numpy passthrough: no interfaceType, or a fixed-width primitive one. Such a whole-register value needs no payload wrapper. @@ -220,18 +233,55 @@ def __init__( self.group_masks = device.groupMasks or {} self.bit_masks = device.bitMasks or {} self.enums = self._build_enums() + # Payload classes are cached by name so registers sharing an ``interfaceType`` + # share one class, as the generator's module-level payload list does. + self.payloads: dict[str, type] = {} + + # -- naming ----------------------------------------------------------- + def _rename( + self, + kind: str, + owner: str, + keys: Iterable[str], + convert: Callable[[str], str], + reserved: bool = False, + ) -> dict[str, str]: + """Map yml keys to their Python names, rejecting collisions. + + Type-level names (registers, enums, payloads) stay verbatim; only enum + members and payload fields are converted, so only those can collide. + """ + renamed: dict[str, str] = {} + origin: dict[str, str] = {} + for key in keys: + name = convert(key) + clash = origin.get(name) + if clash is not None: + raise NameCollisionError( + f"{owner}: {kind}s {clash!r} and {key!r} both map to {name!r}; " + f"rename one in the schema" + ) + if reserved and name in RESERVED_FIELD_NAMES: + raise NameCollisionError( + f"{owner}: {kind} {key!r} maps to {name!r}, which is reserved by " + f"the payload base class; rename it in the schema" + ) + origin[name] = key + renamed[key] = name + return renamed # -- enums ------------------------------------------------------------ def _build_enums(self) -> dict[str, Any]: - # Enum names and members are kept verbatim from the yml. + # Enum type names stay verbatim; members take the generator's SCREAMING_SNAKE. enums: dict[str, Any] = {} for name, spec in self.bit_masks.items(): # IntFlag has no zero-valued member; drop it if present. - members = {k: int(v) for k, v in spec.bits.items() if int(v) != 0} - enums[name] = enum.IntFlag(name, members) + 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()}) for name, spec in self.group_masks.items(): - members = {k: int(v) for k, v in spec.values.items()} - enums[name] = enum.IntEnum(name, members) + 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()}) return enums # -- converter resolution (one uniform factory pipeline) ------------- @@ -288,7 +338,10 @@ def _default(self, member: PayloadMember, type_name: str, ctx: ConverterContext) return _NO_DEFAULT # custom domain interfaceType: no numeric default # -- fields ----------------------------------------------------------- - def _build_field(self, key: str, member: PayloadMember, reg: Register) -> tuple[str, Any]: + def _build_field(self, key: str, member: PayloadMember, reg: Register) -> Any: + # ``key`` stays the verbatim yml name: it feeds ``ConverterContext.name``, and + # a custom converter symbol is derived from the pre-rename key ("Data" -> + # "DataConverter"). The renamed attribute name is applied by the caller. elem_np = _ELEMENT[reg.type] elem_size = np.dtype(elem_np).itemsize offset = member.offset or 0 @@ -309,28 +362,52 @@ def _build_field(self, key: str, member: PayloadMember, reg: Register) -> tuple[ if type_name in self.group_masks: full = (1 << (elem_size * 8)) - 1 mask = member.mask if member.mask is not None else full - return key, GroupMask( - enum=self.enums[type_name], mask=mask, offset=offset, **default_kwarg - ) + return GroupMask(enum=self.enums[type_name], mask=mask, offset=offset, **default_kwarg) field_kwargs: dict[str, Any] = {"offset": offset, **default_kwarg} if member.mask is not None: field_kwargs["mask"] = member.mask - return key, Field(self._resolve_converter(ctx), **field_kwargs) + return Field(self._resolve_converter(ctx), **field_kwargs) # -- payloads --------------------------------------------------------- + def _payload_name(self, name: str, reg: Register) -> str: + """The payload class name: the ``interfaceType`` when a structured register + declares one (so registers sharing that type share a class), else + ``{Register}Payload``.""" + it = reg.interfaceType.root if reg.interfaceType else None + if reg.payloadSpec is not None and it: + return it + return f"{name}Payload" + def _build_payload(self, name: str, reg: Register) -> type: + payload_name = self._payload_name(name, reg) + cached = cast(type[PayloadBase[Any]], self.payloads.get(payload_name)) + if cached is not None: + # Reuse is keyed on the name alone (as the generator's payload list is), so + # refuse to hand back a class whose layout can't describe this register. + if cached.dtype.itemsize != np.dtype(_ELEMENT[reg.type]).itemsize * (reg.length or 1): + raise NameCollisionError( + f"{name}: interfaceType {payload_name!r} is already used by a payload " + f"of a different size; the two registers cannot share one payload class" + ) + return cached + payload = self._new_payload(payload_name, reg) + self.payloads[payload_name] = payload + return payload + + def _new_payload(self, name: str, reg: Register) -> type: elem_np = _ELEMENT[reg.type] elem_size = np.dtype(elem_np).itemsize length = reg.length or 1 if reg.payloadSpec is not None: - namespace = {} - for key, member in reg.payloadSpec.items(): - fname, descriptor = self._build_field(key, member, reg) - namespace[fname] = descriptor + renamed = self._rename("field", name, reg.payloadSpec, field_name, reserved=True) + namespace = { + renamed[key]: self._build_field(key, member, reg) + for key, member in reg.payloadSpec.items() + } kwds = {"length": length} if length > 1 else {} - return _new_class(f"{name}Payload", (StructPayload[elem_np],), namespace, kwds) + return _new_class(name, (StructPayload[elem_np],), namespace, kwds) # anonymous single-value payload mt = reg.maskType.root if reg.maskType else None @@ -353,10 +430,14 @@ def _build_payload(self, name: str, reg: Register) -> type: element_size=elem_size, ) descriptor = Field(self._resolve_converter(ctx)) - return _new_class(f"{name}Payload", (AnonymousPayload[elem_np],), {"__value__": descriptor}) + return _new_class(name, (AnonymousPayload[elem_np],), {"__value__": descriptor}) # -- registers -------------------------------------------------------- - def _build_register(self, name: str, reg: Register) -> type[RegisterBase[Any]]: + def _class_name(self, name: str, reg: Register) -> str: + """A private register's class is underscore-prefixed; its payload class is not.""" + return f"_{name}" if reg.visibility is Visibility.private else name + + def _build_register(self, name: str, class_name: str, reg: Register) -> type[RegisterBase[Any]]: length = reg.length or 1 it = reg.interfaceType.root if reg.interfaceType else None @@ -370,13 +451,13 @@ def _build_register(self, name: str, reg: Register) -> type[RegisterBase[Any]]: ): if length > 1: # plain array register cls = _ARRAY_REGISTER[reg.type](reg.address, length=length) - cls.__name__ = cls.__qualname__ = name + cls.__name__ = cls.__qualname__ = class_name return cls - return _new_class(name, (_SCALAR_REGISTER[reg.type],), {"address": reg.address}) + return _new_class(class_name, (_SCALAR_REGISTER[reg.type],), {"address": reg.address}) payload_cls = self._build_payload(name, reg) return _new_class( - name, + class_name, (RegisterBase,), { "address": reg.address, @@ -386,11 +467,13 @@ def _build_register(self, name: str, reg: Register) -> type[RegisterBase[Any]]: ) def emit(self) -> dict[str, type[RegisterBase[Any]]]: - return { - name: self._build_register(name, reg) - for name, reg in self.device.registers.items() - if not (self.exclude_private and reg.visibility is Visibility.private) - } + 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 def parse_device_schema(text: str) -> DeviceModel: @@ -417,15 +500,19 @@ def create_registers( """Emit runtime register classes from a device schema. ``source`` is yaml text or an already-parsed :class:`DeviceModel` / - :class:`Registers`. Identifiers (fields, enum members) are - kept verbatim from the yml. ``converters`` supplies custom converters keyed by + :class:`Registers`. Identifiers follow the same conventions as the statically + generated device packages: register, enum, and payload class names stay verbatim + from the yml, while payload fields become ``snake_case`` and enum members + ``SCREAMING_SNAKE_CASE``. ``converters`` supplies custom converters keyed by symbol name (e.g. ``{"DataConverter": ...}``); a value is either a ready :class:`~harp.protocol.Converter` instance or a factory ``(ctx: ConverterContext) -> Converter`` that builds one from the field's 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``. + registers whose DSL ``visibility`` is ``private``; when kept, a private register's + class is underscore-prefixed (``_Reserved0``). 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() diff --git a/src/packages/harp-device/src/harp/device/_schema/_naming.py b/src/packages/harp-device/src/harp/device/_schema/_naming.py new file mode 100644 index 0000000..b7106fc --- /dev/null +++ b/src/packages/harp-device/src/harp/device/_schema/_naming.py @@ -0,0 +1,80 @@ +"""How a schema identifier becomes a Python one. + +The runtime emitter must produce the *same* identifiers as the statically +generated device packages, so code written against either lines up name for name: + +* enum members -> :func:`enum_member_name` (``DIPort0`` -> ``DI_PORT0``) +* payload fields -> :func:`field_name` (``DutyCycle`` -> ``duty_cycle``) + +Type-level identifiers (register classes, enum classes, ``{Name}Payload``) are +*not* transformed — the generator keeps those verbatim from the yml too. + +See the upstream generator's package for more information: +https://github.com/harp-tech/generators +""" + +import re + +_SEPARATOR = "_" + +# The generator's regex: an uppercase letter, optionally preceded by a separator. +# The separator is part of the match, so a match starting on ``_``/``-`` has its +# index on the separator rather than on the letter (mirrored in ``_replace``). +_BOUNDARY = re.compile(r"(?P[_\-])?(?P[A-Z])") + + +def _screaming_snake(value: str) -> str: + """Convert a camel/Pascal-case yml identifier to ``SCREAMING_SNAKE_CASE``. + + A direct port of ``FirmwareNamingConvention.Apply``. Consecutive capitals are + kept as one run (``TestDIPort1`` -> ``TEST_DI_PORT1``, ``DIO0`` -> ``DIO0``), + which is why this can't be replaced with a naive boundary regex. + """ + # Skip the leading run of capitals/non-letters, stopping one short of a + # capital that begins a new lowercase word (the ``P`` of ``DIPort0``). + start = 0 + length = len(value) + while start < length and (value[start].isupper() or not value[start].isalpha()): + if ( + start > 1 + and (start + 1) < length + and value[start + 1].isalpha() + and value[start + 1].islower() + ): + break + start += 1 + value = value[:start].lower() + value[start:] + + previous_match = 0 + + def _replace(match: "re.Match[str]") -> str: + nonlocal previous_match + index = match.start() + run = index - previous_match + previous_match = index + char = match.group("char").lower() + # Separate unless this capital continues a run of capitals — and a run's + # final capital still separates when it starts a new lowercase word. + follower = index + 1 + separate = run != 1 or (follower < len(value) and value[follower].islower()) + return _SEPARATOR + char if separate else char + + # ``value`` is read inside ``_replace``; the rebind happens only afterwards, + # so the lookahead always sees the pre-substitution string (as in the C#). + return _BOUNDARY.sub(_replace, value).upper() + + +def enum_member_name(value: str) -> str: + """The Python enum member name for a yml bit-mask or group-mask key. + + ``DIPort0`` -> ``DI_PORT0``. + """ + return _screaming_snake(value) + + +def field_name(value: str) -> str: + """The Python payload field name for a yml ``payloadSpec`` key. + + ``DutyCycle`` -> ``duty_cycle``. Matches the generator's ``GetPythonFieldName``. + """ + return _screaming_snake(value).lower() From 8cc9e02d9a7f96a83670a788f92e791dacb3d748 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:35:35 -0700 Subject: [PATCH 3/8] Add unit tests --- tests/device/test_create_device_module.py | 2 +- tests/device/test_emit.py | 253 +++++++++++++++++++--- tests/device/test_naming.py | 111 ++++++++++ 3 files changed, 330 insertions(+), 36 deletions(-) create mode 100644 tests/device/test_naming.py diff --git a/tests/device/test_create_device_module.py b/tests/device/test_create_device_module.py index a56fb43..2e2c9d2 100644 --- a/tests/device/test_create_device_module.py +++ b/tests/device/test_create_device_module.py @@ -151,6 +151,6 @@ def test_emitted_registers_are_usable(test_module): reg = test_module.AnalogData # The emitted register class round-trips through the Device.read/write frame path. frame = reg.format( - reg.payload_class(Analog0=1.0, Analog1=2.0, Analog2=3.0, Accelerometer=[4, 5, 6]) + reg.payload_class(analog0=1.0, analog1=2.0, analog2=3.0, accelerometer=[4, 5, 6]) ) assert isinstance(frame, (bytes, bytearray)) diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index 4c61667..28df59f 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -5,7 +5,7 @@ from harp.data import parse_to_dataframe from harp.protocol import HarpMessage -from harp.device._schema import UnknownConverterError, create_registers +from harp.device._schema import NameCollisionError, UnknownConverterError, create_registers from . import expected_core, expected_device from .converters import DataConverter @@ -25,14 +25,14 @@ def _device_registers(): def _layout(dt): - """Name-agnostic structural signature: element dtype + offset per field, and itemsize. + """Full structural signature: field name + element dtype + offset, and itemsize. - Ignores field names (we keep the yml's verbatim names; the generator - snake_cases them) while still verifying the byte layout matches exactly. + Name-exact — the emitter applies the same naming convention as the generator, so + the golden comparison covers identifiers as well as byte layout. """ if dt.names is None: return ("scalar", dt.str, dt.shape, dt.itemsize) - return ("struct", dt.itemsize, tuple((dt.fields[n][0], dt.fields[n][1]) for n in dt.names)) + return ("struct", dt.itemsize, tuple((n, dt.fields[n][0], dt.fields[n][1]) for n in dt.names)) # --------------------------------------------------------------------------- @@ -54,19 +54,78 @@ def test_device_emits_all_registers(device_registers): # --------------------------------------------------------------------------- -# Verbatim naming — the yml is the single source of truth +# Naming — identical to the statically generated device package # --------------------------------------------------------------------------- -def test_field_names_are_verbatim(device_registers): +def _enum_of(reg, field): + return reg.payload_class._mro_descriptor(field)._enum + + +def test_field_names_are_snake_case(device_registers): fields = device_registers["AnalogData"].payload_class.dtype.names - assert fields == ("Analog0", "Analog1", "Analog2", "Accelerometer") + assert fields == ("analog0", "analog1", "analog2", "accelerometer") + + +def test_field_names_match_generator(device_registers): + # A run of capitals stays one word; a trailing digit never separates. + fields = device_registers["Version"].payload_class.dtype.names + assert fields == ( + "protocol_version", + "firmware_version", + "hardware_version", + "core_id", + "interface_hash", + ) -def test_enum_members_are_verbatim(device_registers): - flags = device_registers["PortDIOSet"].payload_class._mro_descriptor("__value__")._enum - # yml bit names are kept as-is (the generator would UPPER_SNAKE these). - assert {"DIO0", "DIPort0", "TestDIPort1", "PortDIO1"} <= set(flags.__members__) +def test_enum_members_are_screaming_snake_case(device_registers): + flags = _enum_of(device_registers["PortDIOSet"], "__value__") + assert list(flags.__members__) == [ + "DIO0", + "DIO1", + "DIO2", + "DIO3", + "DI_PORT0", + "TEST_DI_PORT1", + "SUPPLY_PORT0", + "PORT_DIO1", + ] + + +def test_group_mask_members_match_generator(device_registers): + assert list(_enum_of(device_registers["StartPulse"], "digital_output").__members__) == [ + "PWM0", + "PWM1", + "PWM2", + "PWM3", + ] + + +def test_register_and_payload_class_names_stay_verbatim(device_registers): + # Only fields and enum members are renamed; type-level names come from the yml. + reg = device_registers["AnalogData"] + assert reg.__name__ == "AnalogData" + assert reg.payload_class.__name__ == "AnalogDataPayload" + assert _enum_of(device_registers["EncoderMode"], "__value__").__name__ == "EncoderModeMask" + + +def test_enum_names_match_generator_for_every_enum(device_registers): + """Every enum the golden module declares has identical members in the emitter.""" + for name, reg in _device_registers().items(): + payload = reg.payload_class + if payload.dtype.names is None: + continue + for field in payload._repr_fields: + expected_desc = payload._mro_descriptor(field) + expected_enum = getattr(expected_desc, "_enum", None) + if expected_enum is None: + continue + emitted_enum = _enum_of(device_registers[name], field) + assert emitted_enum.__name__ == expected_enum.__name__ + assert {m.name: int(m.value) for m in emitted_enum} == { + m.name: int(m.value) for m in expected_enum + } # --------------------------------------------------------------------------- @@ -104,8 +163,8 @@ def _roundtrip(reg, value): def test_whole_register_groupmask_unwraps_to_enum(device_registers): reg = device_registers["EncoderMode"] enum_cls = reg.payload_class._mro_descriptor("__value__")._enum - parsed = _roundtrip(reg, enum_cls["Displacement"]) - assert parsed == enum_cls["Displacement"] + parsed = _roundtrip(reg, enum_cls["DISPLACEMENT"]) + assert parsed == enum_cls["DISPLACEMENT"] assert isinstance(parsed, enum_cls) @@ -119,21 +178,21 @@ def test_whole_register_bitmask_roundtrip(device_registers): def test_struct_masked_members_roundtrip(device_registers): reg = device_registers["StartPulse"] payload_cls = reg.payload_class - pwm = payload_cls._mro_descriptor("DigitalOutput")._enum - # DigitalOutput is a 2-bit field (mask 0xC00); only Pwm0/Pwm1 fit it. This + pwm = payload_cls._mro_descriptor("digital_output")._enum + # digital_output is a 2-bit field (mask 0xC00); only PWM0/PWM1 fit it. This # matches the generator's output verbatim (GroupMask(enum=PwmPort, mask=0xC00)). - payload = payload_cls(DigitalOutput=pwm["Pwm1"], PulseWidth=np.uint16(300)) + payload = payload_cls(digital_output=pwm["PWM1"], pulse_width=np.uint16(300)) parsed = _roundtrip(reg, payload) - assert parsed.DigitalOutput == pwm["Pwm1"] - assert int(parsed.PulseWidth) == 300 + assert parsed.digital_output == pwm["PWM1"] + assert int(parsed.pulse_width) == 300 def test_custom_converter_roundtrip(device_registers): reg = device_registers["CustomMemberConverter"] payload_cls = reg.payload_class - parsed = _roundtrip(reg, payload_cls(Header=np.uint8(7), Data=-1234)) - assert int(parsed.Header) == 7 - assert int(parsed.Data) == -1234 + parsed = _roundtrip(reg, payload_cls(header=np.uint8(7), data=-1234)) + assert int(parsed.header) == 7 + assert int(parsed.data) == -1234 # --------------------------------------------------------------------------- @@ -163,10 +222,11 @@ def factory(ctx): regs = create_registers(device_yml, converters={"DataConverter": factory}) parsed = _roundtrip( regs["CustomMemberConverter"], - regs["CustomMemberConverter"].payload_class(Header=np.uint8(1), Data=42), + regs["CustomMemberConverter"].payload_class(header=np.uint8(1), data=42), ) - assert int(parsed.Data) == 42 - # the factory was handed the Data field's resolved DSL context + assert int(parsed.data) == 42 + # The factory was handed the Data field's resolved DSL context, keyed by the + # verbatim yml name — the converter symbol derives from that, not from "data". assert seen == {"name": "Data", "span": 2, "interface_type": "int"} @@ -175,14 +235,138 @@ def factory(ctx): # --------------------------------------------------------------------------- +_VISIBILITY_YML = ( + "registers:\n" + " Pub: {address: 40, type: U16, access: Read}\n" + " Priv: {address: 41, type: U16, access: Read, visibility: private}\n" +) + + def test_exclude_private_drops_private_registers(): - yml = ( + # Kept by default; a private register's class is underscore-prefixed, as the + # generator emits 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(): + regs = create_registers(_VISIBILITY_YML) + assert regs["_Priv"].__name__ == "_Priv" + assert regs["_Priv"].address == 41 + assert regs["Pub"].__name__ == "Pub" + + +def test_private_payload_class_is_not_prefixed(): + # Only the register class takes the underscore; its payload keeps {Name}Payload. + regs = create_registers( "registers:\n" - " Pub: {address: 40, type: U16, access: Read}\n" - " Priv: {address: 41, type: U16, access: Read, visibility: private}\n" + " Priv:\n" + " address: 41\n" + " type: U8\n" + " access: Read\n" + " visibility: private\n" + " payloadSpec:\n" + " Foo: {offset: 0}\n" ) - assert set(create_registers(yml)) == {"Pub", "Priv"} # kept by default - assert set(create_registers(yml, exclude_private=True)) == {"Pub"} + assert regs["_Priv"].payload_class.__name__ == "PrivPayload" + + +# --------------------------------------------------------------------------- +# Payload class sharing — a structured register with an interfaceType names its +# payload after that type, so registers sharing the type share one class. +# --------------------------------------------------------------------------- + + +def test_structured_register_payload_named_after_interface_type(): + regs = create_registers( + "registers:\n" + " A:\n" + " address: 40\n" + " type: U8\n" + " access: Read\n" + " interfaceType: Shared\n" + " payloadSpec:\n" + " Foo: {offset: 0}\n" + " B:\n" + " address: 41\n" + " type: U8\n" + " access: Read\n" + " interfaceType: Shared\n" + " payloadSpec:\n" + " Foo: {offset: 0}\n" + ) + assert regs["A"].payload_class.__name__ == "Shared" + # One class, reused — not two structurally identical copies. + assert regs["A"].payload_class is regs["B"].payload_class + + +def test_shared_payload_of_mismatched_size_raises(): + # Reuse is keyed on the interfaceType name alone, so a same-named payload over a + # different element width would silently mis-describe the second register. + with pytest.raises(NameCollisionError, match="different size"): + create_registers( + "registers:\n" + " A:\n" + " address: 40\n" + " type: U8\n" + " access: Read\n" + " interfaceType: Shared\n" + " payloadSpec:\n" + " Foo: {offset: 0}\n" + " B:\n" + " address: 41\n" + " type: U32\n" + " access: Read\n" + " interfaceType: Shared\n" + " payloadSpec:\n" + " Foo: {offset: 0}\n" + ) + + +# --------------------------------------------------------------------------- +# Name collisions introduced by the convention +# --------------------------------------------------------------------------- + + +def test_colliding_field_names_raise(): + with pytest.raises(NameCollisionError, match="both map to 'foo'"): + create_registers( + "registers:\n" + " R:\n" + " address: 40\n" + " type: U8\n" + " access: Read\n" + " payloadSpec:\n" + " Foo: {offset: 0}\n" + " FOO: {offset: 1}\n" + ) + + +def test_colliding_enum_members_raise(): + with pytest.raises(NameCollisionError, match="both map to 'ON'"): + create_registers( + "registers:\n" + " R: {address: 40, type: U8, access: Read, maskType: M}\n" + "groupMasks:\n" + " M:\n" + " values:\n" + " On: 0\n" + " ON: 1\n" + ) + + +def test_field_name_shadowing_payload_attribute_raises(): + with pytest.raises(NameCollisionError, match="reserved"): + create_registers( + "registers:\n" + " R:\n" + " address: 40\n" + " type: U8\n" + " access: Read\n" + " payloadSpec:\n" + " Dtype: {offset: 0}\n" + " Other: {offset: 1}\n" + ) # --------------------------------------------------------------------------- @@ -214,9 +398,8 @@ def test_emitted_register_bulk_matches_oracle(name, device_registers): assert buf == bytes(oracle.format_bulk(records)) # Cross-read via harp.data: the shared bytes decode to equal frames through - # either class. Enum labels and field names diverge (verbatim yml vs generator - # snake_case), so compare raw codes by column position, not by name. - df_emitted = parse_to_dataframe(emitted, buf, timestamp=False, decode_enums=False) - df_oracle = parse_to_dataframe(oracle, buf, timestamp=False, decode_enums=False) - df_oracle.columns = df_emitted.columns + # either class — including column names and decoded enum labels, which now agree. + df_emitted = parse_to_dataframe(emitted, buf, timestamp=False) + df_oracle = parse_to_dataframe(oracle, buf, timestamp=False) + assert list(df_emitted.columns) == list(df_oracle.columns) assert df_emitted.equals(df_oracle) diff --git a/tests/device/test_naming.py b/tests/device/test_naming.py new file mode 100644 index 0000000..2c09a7a --- /dev/null +++ b/tests/device/test_naming.py @@ -0,0 +1,111 @@ +"""The naming convention must match ``FirmwareNamingConvention`` in harp-tech/generators. + +Every pair below is taken from the generator's own committed expected output +(``tests/ExpectedOutput/{core,device}.py`` against ``tests/Metadata/{core,device}.yml``), +so these lock the port to the C# behaviour rather than to a re-derivation of it. +""" + +import pytest +from harp.device._schema import enum_member_name, field_name + +# yml identifier -> generated enum member (SCREAMING_SNAKE_CASE) +ENUM_MEMBERS = [ + # Runs of capitals stay one word, and a trailing digit never separates. + ("DIO0", "DIO0"), + ("DIO1", "DIO1"), + ("DIO2", "DIO2"), + ("DIO3", "DIO3"), + # A capital run ending where a lowercase word starts does separate. + ("DIPort0", "DI_PORT0"), + ("TestDIPort1", "TEST_DI_PORT1"), + ("SupplyPort0", "SUPPLY_PORT0"), + ("PortDIO1", "PORT_DIO1"), + ("Pwm0", "PWM0"), + ("Pwm1", "PWM1"), + ("Pwm2", "PWM2"), + ("Pwm3", "PWM3"), + ("Position", "POSITION"), + ("Displacement", "DISPLACEMENT"), + # core.yml + ("RestoreDefault", "RESTORE_DEFAULT"), + ("RestoreEeprom", "RESTORE_EEPROM"), + ("Save", "SAVE"), + ("RestoreName", "RESTORE_NAME"), + ("UpdateFirmware", "UPDATE_FIRMWARE"), + ("BootFromDefault", "BOOT_FROM_DEFAULT"), + ("BootFromEeprom", "BOOT_FROM_EEPROM"), + ("ClockRepeater", "CLOCK_REPEATER"), + ("ClockGenerator", "CLOCK_GENERATOR"), + ("RepeaterCapability", "REPEATER_CAPABILITY"), + ("GeneratorCapability", "GENERATOR_CAPABILITY"), + ("ClockUnlock", "CLOCK_UNLOCK"), + ("ClockLock", "CLOCK_LOCK"), + ("Standby", "STANDBY"), + ("Active", "ACTIVE"), + ("Speed", "SPEED"), + ("Disabled", "DISABLED"), + ("Enabled", "ENABLED"), +] + +# yml identifier -> generated payload field (snake_case) +PAYLOAD_FIELDS = [ + ("Analog0", "analog0"), + ("Analog1", "analog1"), + ("Analog2", "analog2"), + ("Accelerometer", "accelerometer"), + ("PwmPort", "pwm_port"), + ("DutyCycle", "duty_cycle"), + ("Frequency", "frequency"), + ("EventsEnabled", "events_enabled"), + ("Delta", "delta"), + ("ProtocolVersion", "protocol_version"), + ("FirmwareVersion", "firmware_version"), + ("HardwareVersion", "hardware_version"), + ("CoreId", "core_id"), + ("InterfaceHash", "interface_hash"), + ("Header", "header"), + ("Data", "data"), + ("Low", "low"), + ("High", "high"), + ("DigitalOutput", "digital_output"), + ("PulseWidth", "pulse_width"), + ("PulseCount", "pulse_count"), + # core.yml — a trailing capital run collapses either way it is spelled. + ("OperationMode", "operation_mode"), + ("DumpRegisters", "dump_registers"), + ("MuteReplies", "mute_replies"), + ("VisualIndicators", "visual_indicators"), + ("OperationLed", "operation_led"), + ("OperationLED", "operation_led"), + ("Heartbeat", "heartbeat"), +] + + +@pytest.mark.parametrize(("source", "expected"), ENUM_MEMBERS) +def test_enum_member_name_matches_generator(source, expected): + assert enum_member_name(source) == expected + + +@pytest.mark.parametrize(("source", "expected"), PAYLOAD_FIELDS) +def test_field_name_matches_generator(source, expected): + assert field_name(source) == expected + + +def test_both_conventions_share_one_casing_pass(): + # The generator derives its field names from the same pass, differing only in case, + # so the two can never disagree about where a word boundary falls. + for source, _ in ENUM_MEMBERS + PAYLOAD_FIELDS: + assert field_name(source) == enum_member_name(source).lower() + + +def test_already_converted_names_are_stable(): + # The generator's own output is a fixed point, so regenerating never drifts. + for _, generated in ENUM_MEMBERS: + assert enum_member_name(generated) == generated + for _, generated in PAYLOAD_FIELDS: + assert field_name(generated) == generated + + +@pytest.mark.parametrize("source", ["", "_", "0", "A", "a"]) +def test_degenerate_inputs_do_not_raise(source): + assert enum_member_name(source) == source.upper() From 314f689ef9aa93104637882cd2cc32da00fcdf78 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:53:09 -0700 Subject: [PATCH 4/8] Ensure benchmark package is using the new convention --- .../src/harp/benchmarks/register_models.py | 144 +++++++++--------- tests/protocol/test_register_modeling.py | 104 ++++++------- 2 files changed, 123 insertions(+), 125 deletions(-) diff --git a/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py b/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py index d6aac12..bacb185 100644 --- a/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py +++ b/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py @@ -40,26 +40,26 @@ class PortDigitalIOS(enum.IntFlag): DIO1 = 0x2 DIO2 = 0x4 DIO3 = 0x8 - DIPort0 = 0x100 - TestDIPort1 = 0x200 - SupplyPort0 = 0x400 - PortDIO1 = 0x800 + DI_PORT0 = 0x100 + TEST_DI_PORT1 = 0x200 + SUPPLY_PORT0 = 0x400 + PORT_DIO1 = 0x800 class PwmPort(enum.IntEnum): - """device.yml groupMasks.PwmPort (note Pwm3 = 0xA).""" + """device.yml groupMasks.PwmPort (note PWM3 = 0xA).""" - Pwm0 = 0x1 - Pwm1 = 0x2 - Pwm2 = 0x4 - Pwm3 = 0xA + PWM0 = 0x1 + PWM1 = 0x2 + PWM2 = 0x4 + PWM3 = 0xA class EncoderModeMask(enum.IntEnum): """device.yml groupMasks.EncoderModeMask.""" - Position = 0x0 - Displacement = 0x1 + POSITION = 0x0 + DISPLACEMENT = 0x1 # =========================================================================== @@ -106,10 +106,10 @@ class DigitalInputs(RegisterU8): class AnalogDataPayload(StructPayload[np.float32], length=6): - Analog0: np.float32 = Field(IdentityConverter(np.float32), offset=0) - Analog1: np.float32 = Field(IdentityConverter(np.float32), offset=1) - Analog2: np.float32 = Field(IdentityConverter(np.float32), offset=2) - Accelerometer: NDArray[np.float32] = Field( + analog0: np.float32 = Field(IdentityConverter(np.float32), offset=0) + analog1: np.float32 = Field(IdentityConverter(np.float32), offset=1) + analog2: np.float32 = Field(IdentityConverter(np.float32), offset=2) + accelerometer: NDArray[np.float32] = Field( IdentityConverter(np.dtype((np.float32, (3,)))), offset=3 ) @@ -126,13 +126,11 @@ class AnalogData(RegisterBase[AnalogDataPayload]): class ComplexConfigurationPayload(StructPayload[np.uint8], length=17): - PwmPort: "PwmPort" = GroupMask( - enum=PwmPort, mask=0xFF, offset=0 - ) # quoted: member name shadows enum type - DutyCycle: np.float32 = Field(IdentityConverter(np.float32), offset=4) - Frequency: np.float32 = Field(IdentityConverter(np.float32), offset=8) - EventsEnabled: bool = Field(BoolConverter(), offset=12) - Delta: np.uint32 = Field(IdentityConverter(np.uint32), offset=13) + pwm_port: PwmPort = GroupMask(enum=PwmPort, mask=0xFF, offset=0) + duty_cycle: np.float32 = Field(IdentityConverter(np.float32), offset=4) + frequency: np.float32 = Field(IdentityConverter(np.float32), offset=8) + events_enabled: bool = Field(BoolConverter(), offset=12) + delta: np.uint32 = Field(IdentityConverter(np.uint32), offset=13) class ComplexConfiguration(RegisterBase[ComplexConfigurationPayload]): @@ -147,11 +145,11 @@ class ComplexConfiguration(RegisterBase[ComplexConfigurationPayload]): class VersionPayload(StructPayload[np.uint8], length=32): - ProtocolVersion: HarpVersion = Field(HarpVersionConverter(np.uint8), offset=0) - FirmwareVersion: HarpVersion = Field(HarpVersionConverter(np.uint8), offset=3) - HardwareVersion: HarpVersion = Field(HarpVersionConverter(np.uint8), offset=6) - CoreId: str = Field(StringConverter(3), offset=9) - InterfaceHash: NDArray[np.uint8] = Field( + protocol_version: HarpVersion = Field(HarpVersionConverter(np.uint8), offset=0) + firmware_version: HarpVersion = Field(HarpVersionConverter(np.uint8), offset=3) + hardware_version: HarpVersion = Field(HarpVersionConverter(np.uint8), offset=6) + core_id: str = Field(StringConverter(3), offset=9) + interface_hash: NDArray[np.uint8] = Field( IdentityConverter(np.dtype((np.uint8, (20,)))), offset=12 ) @@ -194,8 +192,8 @@ class CustomRawPayload(RegisterBase[HarpVersion]): class CustomMemberConverterPayload(StructPayload[np.uint8], length=3): - Header: np.uint8 = Field(IdentityConverter(np.uint8)) - Data: int = Field(BytesToIntConverter(2, signed=True), offset=1) + header: np.uint8 = Field(IdentityConverter(np.uint8)) + data: int = Field(BytesToIntConverter(2, signed=True), offset=1) class CustomMemberConverter(RegisterBase[CustomMemberConverterPayload]): @@ -210,8 +208,8 @@ class CustomMemberConverter(RegisterBase[CustomMemberConverterPayload]): class BitmaskSplitterPayload(StructPayload[np.uint8]): - Low: np.int32 = Field(IdentityConverter(np.int32), mask=0x0F) - High: np.int32 = Field(IdentityConverter(np.int32), mask=0xF0) + low: np.int32 = Field(IdentityConverter(np.int32), mask=0x0F) + high: np.int32 = Field(IdentityConverter(np.int32), mask=0xF0) class BitmaskSplitter(RegisterBase[BitmaskSplitterPayload]): @@ -265,8 +263,8 @@ class PulseDO0(RegisterU16): class StartPulsePayload(StructPayload[np.uint16]): - DigitalOutput: PwmPort = GroupMask(enum=PwmPort, mask=0xC00) - PulseWidth: np.uint16 = Field(IdentityConverter(np.uint16), mask=0x3FF) + digital_output: PwmPort = GroupMask(enum=PwmPort, mask=0xC00) + pulse_width: np.uint16 = Field(IdentityConverter(np.uint16), mask=0x3FF) class StartPulse(RegisterBase[StartPulsePayload]): @@ -281,12 +279,12 @@ class StartPulse(RegisterBase[StartPulsePayload]): class StartPulseTrainPayload(StructPayload[np.uint16], length=2): - DigitalOutput: PwmPort = GroupMask(enum=PwmPort, mask=0xC00, offset=0) - PulseWidth: np.uint16 = Field(IdentityConverter(np.uint16), mask=0x3FF, offset=0) - Frequency: np.uint8 = Field( + digital_output: PwmPort = GroupMask(enum=PwmPort, mask=0xC00, offset=0) + pulse_width: np.uint16 = Field(IdentityConverter(np.uint16), mask=0x3FF, offset=0) + frequency: np.uint8 = Field( IdentityConverter(np.uint8), mask=0xFF00, offset=1, default=np.uint8(1) ) - PulseCount: np.uint8 = Field(IdentityConverter(np.uint8), mask=0xFF, offset=1) + pulse_count: np.uint8 = Field(IdentityConverter(np.uint8), mask=0xFF, offset=1) class StartPulseTrain(RegisterBase[StartPulseTrainPayload]): @@ -327,26 +325,26 @@ def main() -> None: # pragma: no cover - manual exploration entry point print("DigitalInputs OK") ad = AnalogDataPayload( - Analog0=np.float32(1.0), - Analog1=np.float32(2.0), - Analog2=np.float32(3.0), - Accelerometer=np.array([4, 5, 6], dtype=np.float32), + analog0=np.float32(1.0), + analog1=np.float32(2.0), + analog2=np.float32(3.0), + accelerometer=np.array([4, 5, 6], dtype=np.float32), ) p = _roundtrip(AnalogData, ad) - assert float(p.Analog0) == 1.0 and float(p.Analog2) == 3.0 - np.testing.assert_array_equal(p.Accelerometer, [4, 5, 6]) + assert float(p.analog0) == 1.0 and float(p.analog2) == 3.0 + np.testing.assert_array_equal(p.accelerometer, [4, 5, 6]) print(f"AnalogData OK ({AnalogDataPayload.dtype.itemsize} bytes)") cc = ComplexConfigurationPayload( - PwmPort=PwmPort.Pwm2, - DutyCycle=np.float32(0.5), - Frequency=np.float32(1000.0), - EventsEnabled=True, - Delta=np.uint32(42), + pwm_port=PwmPort.PWM2, + duty_cycle=np.float32(0.5), + frequency=np.float32(1000.0), + events_enabled=True, + delta=np.uint32(42), ) p = _roundtrip(ComplexConfiguration, cc) - assert p.PwmPort == PwmPort.Pwm2 and p.EventsEnabled is True and int(p.Delta) == 42 - assert float(p.DutyCycle) == 0.5 + assert p.pwm_port == PwmPort.PWM2 and p.events_enabled is True and int(p.delta) == 42 + assert float(p.duty_cycle) == 0.5 assert ComplexConfigurationPayload.dtype.itemsize == 17 assert cc.raw_payload.tobytes()[1:4] == b"\x00\x00\x00" print( @@ -354,15 +352,15 @@ def main() -> None: # pragma: no cover - manual exploration entry point ) ver = VersionPayload( - ProtocolVersion=HarpVersion(2, 0, 0), - FirmwareVersion=HarpVersion(1, 2, 3), - HardwareVersion=HarpVersion(1, 0, 0), - CoreId="abc", - InterfaceHash=np.arange(20, dtype=np.uint8), + protocol_version=HarpVersion(2, 0, 0), + firmware_version=HarpVersion(1, 2, 3), + hardware_version=HarpVersion(1, 0, 0), + core_id="abc", + interface_hash=np.arange(20, dtype=np.uint8), ) p = _roundtrip(Version, ver) - assert p.ProtocolVersion == HarpVersion(2, 0, 0) and p.CoreId == "abc" - np.testing.assert_array_equal(p.InterfaceHash, np.arange(20)) + assert p.protocol_version == HarpVersion(2, 0, 0) and p.core_id == "abc" + np.testing.assert_array_equal(p.interface_hash, np.arange(20)) print(f"Version OK ({VersionPayload.dtype.itemsize} bytes)") p = _roundtrip(CustomPayload, HarpVersion(3, 1, 4)) @@ -372,13 +370,13 @@ def main() -> None: # pragma: no cover - manual exploration entry point print("CustomPayload/RawPayload OK (single-member unwrap)") p = _roundtrip( - CustomMemberConverter, CustomMemberConverterPayload(Header=np.uint8(7), Data=-1234) + CustomMemberConverter, CustomMemberConverterPayload(header=np.uint8(7), data=-1234) ) - assert int(p.Header) == 7 and int(p.Data) == -1234 + assert int(p.header) == 7 and int(p.data) == -1234 print("CustomMemberConverter OK") - p = _roundtrip(BitmaskSplitter, BitmaskSplitterPayload(Low=0xA, High=0x5)) - assert int(p.Low) == 0xA and int(p.High) == 0x5 + p = _roundtrip(BitmaskSplitter, BitmaskSplitterPayload(low=0xA, high=0x5)) + assert int(p.low) == 0xA and int(p.high) == 0x5 assert p.raw_payload.tobytes() == bytes([0x5A]) print("BitmaskSplitter OK") @@ -396,28 +394,28 @@ def main() -> None: # pragma: no cover - manual exploration entry point print("PulseDOPort0 / PulseDO0 OK") p = _roundtrip( - StartPulse, StartPulsePayload(DigitalOutput=PwmPort.Pwm1, PulseWidth=np.uint16(300)) + StartPulse, StartPulsePayload(digital_output=PwmPort.PWM1, pulse_width=np.uint16(300)) ) - assert p.DigitalOutput == PwmPort.Pwm1 and int(p.PulseWidth) == 300 + assert p.digital_output == PwmPort.PWM1 and int(p.pulse_width) == 300 print("StartPulse OK") p = _roundtrip( StartPulseTrain, StartPulseTrainPayload( - DigitalOutput=PwmPort.Pwm1, - PulseWidth=np.uint16(300), - Frequency=np.uint8(200), - PulseCount=np.uint8(50), + digital_output=PwmPort.PWM1, + pulse_width=np.uint16(300), + frequency=np.uint8(200), + pulse_count=np.uint8(50), ), ) - assert p.DigitalOutput == PwmPort.Pwm1 and int(p.PulseWidth) == 300 - assert int(p.Frequency) == 200 and int(p.PulseCount) == 50 + assert p.digital_output == PwmPort.PWM1 and int(p.pulse_width) == 300 + assert int(p.frequency) == 200 and int(p.pulse_count) == 50 assert StartPulseTrainPayload.dtype.itemsize == 4 - assert int(StartPulseTrainPayload(PulseCount=np.uint8(3)).Frequency) == 1 # defaultValue - print("StartPulseTrain OK (4 masked members, 2 words, default Frequency=1)") + assert int(StartPulseTrainPayload(pulse_count=np.uint8(3)).frequency) == 1 # defaultValue + print("StartPulseTrain OK (4 masked members, 2 words, default frequency=1)") - p = _roundtrip(EncoderMode, EncoderModeMask.Displacement) - assert p == EncoderModeMask.Displacement # single-member unwrap + p = _roundtrip(EncoderMode, EncoderModeMask.DISPLACEMENT) + assert p == EncoderModeMask.DISPLACEMENT # single-member unwrap print("EncoderMode OK") print("\nAll device.yml registers round-trip cleanly.") diff --git a/tests/protocol/test_register_modeling.py b/tests/protocol/test_register_modeling.py index 1f22b54..5ffbe66 100644 --- a/tests/protocol/test_register_modeling.py +++ b/tests/protocol/test_register_modeling.py @@ -54,43 +54,43 @@ def test_scalar_registers_roundtrip(): def test_analog_data_roundtrip(): ad = AnalogDataPayload( - Analog0=np.float32(1.0), - Analog1=np.float32(2.0), - Analog2=np.float32(3.0), - Accelerometer=np.array([4, 5, 6], dtype=np.float32), + analog0=np.float32(1.0), + analog1=np.float32(2.0), + analog2=np.float32(3.0), + accelerometer=np.array([4, 5, 6], dtype=np.float32), ) p = _roundtrip(AnalogData, ad) - assert float(p.Analog0) == 1.0 and float(p.Analog2) == 3.0 - np.testing.assert_array_equal(p.Accelerometer, [4, 5, 6]) + assert float(p.analog0) == 1.0 and float(p.analog2) == 3.0 + np.testing.assert_array_equal(p.accelerometer, [4, 5, 6]) assert AnalogDataPayload.dtype.itemsize == 24 # 6 floats def test_version_roundtrip(): ver = VersionPayload( - ProtocolVersion=HarpVersion(2, 0, 0), - FirmwareVersion=HarpVersion(1, 2, 3), - HardwareVersion=HarpVersion(1, 0, 0), - CoreId="abc", - InterfaceHash=np.arange(20, dtype=np.uint8), + protocol_version=HarpVersion(2, 0, 0), + firmware_version=HarpVersion(1, 2, 3), + hardware_version=HarpVersion(1, 0, 0), + core_id="abc", + interface_hash=np.arange(20, dtype=np.uint8), ) p = _roundtrip(Version, ver) - assert p.ProtocolVersion == HarpVersion(2, 0, 0) - assert p.CoreId == "abc" - np.testing.assert_array_equal(p.InterfaceHash, np.arange(20)) + assert p.protocol_version == HarpVersion(2, 0, 0) + assert p.core_id == "abc" + np.testing.assert_array_equal(p.interface_hash, np.arange(20)) assert VersionPayload.dtype.itemsize == 32 def test_custom_member_converter_roundtrip(): p = _roundtrip( - CustomMemberConverter, CustomMemberConverterPayload(Header=np.uint8(7), Data=-1234) + CustomMemberConverter, CustomMemberConverterPayload(header=np.uint8(7), data=-1234) ) - assert int(p.Header) == 7 and int(p.Data) == -1234 + assert int(p.header) == 7 and int(p.data) == -1234 def test_encoder_mode_roundtrip(): # Single whole-register groupMask -> parse() unwraps to the bare enum. - p = _roundtrip(EncoderMode, EncoderModeMask.Displacement) - assert p == EncoderModeMask.Displacement + p = _roundtrip(EncoderMode, EncoderModeMask.DISPLACEMENT) + assert p == EncoderModeMask.DISPLACEMENT assert isinstance(p, EncoderModeMask) @@ -101,11 +101,11 @@ def test_encoder_mode_roundtrip(): def test_complex_configuration_gap_and_offsets(): cc = ComplexConfigurationPayload( - PwmPort=PwmPort.Pwm2, - DutyCycle=np.float32(0.5), - Frequency=np.float32(1000.0), - EventsEnabled=True, - Delta=np.uint32(42), + pwm_port=PwmPort.PWM2, + duty_cycle=np.float32(0.5), + frequency=np.float32(1000.0), + events_enabled=True, + delta=np.uint32(42), ) # itemsize from the register length (17), not the member extent. assert ComplexConfigurationPayload.dtype.itemsize == 17 @@ -113,14 +113,14 @@ def test_complex_configuration_gap_and_offsets(): assert cc.raw_payload.tobytes()[1:4] == b"\x00\x00\x00" # explicit byte offsets (base element = uint8, so element units == bytes). fields = ComplexConfigurationPayload.dtype.fields - assert fields["DutyCycle"][1] == 4 - assert fields["Delta"][1] == 13 + assert fields["duty_cycle"][1] == 4 + assert fields["delta"][1] == 13 p = _roundtrip(ComplexConfiguration, cc) - assert p.PwmPort == PwmPort.Pwm2 - assert float(p.DutyCycle) == 0.5 - assert p.EventsEnabled is True - assert int(p.Delta) == 42 + assert p.pwm_port == PwmPort.PWM2 + assert float(p.duty_cycle) == 0.5 + assert p.events_enabled is True + assert int(p.delta) == 42 # --------------------------------------------------------------------------- @@ -133,33 +133,33 @@ def test_start_pulse_overlapping_masks(): assert StartPulsePayload.dtype.itemsize == 2 assert len(StartPulsePayload.dtype.names) == 1 p = _roundtrip( - StartPulse, StartPulsePayload(DigitalOutput=PwmPort.Pwm1, PulseWidth=np.uint16(300)) + StartPulse, StartPulsePayload(digital_output=PwmPort.PWM1, pulse_width=np.uint16(300)) ) - assert p.DigitalOutput == PwmPort.Pwm1 - assert int(p.PulseWidth) == 300 + assert p.digital_output == PwmPort.PWM1 + assert int(p.pulse_width) == 300 def test_start_pulse_train_two_words_and_default(): p = _roundtrip( StartPulseTrain, StartPulseTrainPayload( - DigitalOutput=PwmPort.Pwm1, - PulseWidth=np.uint16(300), - Frequency=np.uint8(200), - PulseCount=np.uint8(50), + digital_output=PwmPort.PWM1, + pulse_width=np.uint16(300), + frequency=np.uint8(200), + pulse_count=np.uint8(50), ), ) - assert p.DigitalOutput == PwmPort.Pwm1 and int(p.PulseWidth) == 300 - assert int(p.Frequency) == 200 and int(p.PulseCount) == 50 + assert p.digital_output == PwmPort.PWM1 and int(p.pulse_width) == 300 + assert int(p.frequency) == 200 and int(p.pulse_count) == 50 assert StartPulseTrainPayload.dtype.itemsize == 4 # two U16 words - # defaultValue: Frequency defaults to 1 when not provided. - assert int(StartPulseTrainPayload(PulseCount=np.uint8(3)).Frequency) == 1 + # defaultValue: frequency defaults to 1 when not provided. + assert int(StartPulseTrainPayload(pulse_count=np.uint8(3)).frequency) == 1 def test_bitmask_splitter_masked_ints(): - p = _roundtrip(BitmaskSplitter, BitmaskSplitterPayload(Low=0xA, High=0x5)) - assert int(p.Low) == 0xA and int(p.High) == 0x5 - assert p.raw_payload.tobytes() == bytes([0x5A]) # High packs into the top nibble + p = _roundtrip(BitmaskSplitter, BitmaskSplitterPayload(low=0xA, high=0x5)) + assert int(p.low) == 0xA and int(p.high) == 0x5 + assert p.raw_payload.tobytes() == bytes([0x5A]) # high packs into the top nibble def test_port_dio_set_bitmask(): @@ -192,10 +192,10 @@ def test_custom_payload_single_member_unwrap(): def test_unknown_enum_code_preserves_raw(): - # StartPulse.DigitalOutput is a 2-bit field; code 0b11 has no PwmPort member. + # StartPulse.digital_output is a 2-bit field; code 0b11 has no PwmPort member. raw = np.array(0b11 << 10, dtype=np.uint16).tobytes() payload = StartPulsePayload.from_buffer(raw) - value = payload.DigitalOutput # permissive: the raw code is kept, not raised + value = payload.digital_output # permissive: the raw code is kept, not raised assert value == 0b11 assert not isinstance(value, PwmPort) @@ -207,14 +207,14 @@ def test_unknown_enum_code_preserves_raw(): def test_complex_configuration_to_dataframe(): cc = ComplexConfigurationPayload( - PwmPort=PwmPort.Pwm2, - DutyCycle=np.float32(0.5), - Frequency=np.float32(1.0), - EventsEnabled=True, - Delta=np.uint32(42), + pwm_port=PwmPort.PWM2, + duty_cycle=np.float32(0.5), + frequency=np.float32(1.0), + events_enabled=True, + delta=np.uint32(42), ) batch = ComplexConfigurationPayload.from_buffer(cc.raw_payload.tobytes() * 2) df = payload_to_dataframe(batch) assert len(df) == 2 - assert list(df["PwmPort"]) == ["Pwm2", "Pwm2"] - np.testing.assert_array_equal(df["Delta"], [42, 42]) + assert list(df["pwm_port"]) == ["PWM2", "PWM2"] + np.testing.assert_array_equal(df["delta"], [42, 42]) From 9b6de6bf2b0f210d35efda93fc2e773c56a65976 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Mon, 10 Aug 2026 22:46:10 +0100 Subject: [PATCH 5/8] Share payload classes by name without a size check Reuse a cached payload class whenever a structured register names the same interfaceType, matching the payload lookup both generator targets perform. The previous check compared the cached payload against the element size times the declared length, which is only how a payload is sized when a length is given; a payloadSpec spanning several elements without one takes its size from the member offsets instead, so two identical registers sharing an interfaceType were rejected. The check also passed a genuine mismatch whose total width happened to agree. Cover the reuse with the anchor and merge form the published schemas use, and with an offset-sized payload carrying no declared length. --- .../src/harp/device/_schema/_emit.py | 12 +--- tests/device/test_emit.py | 68 +++++++++++++------ 2 files changed, 49 insertions(+), 31 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 6aa3d16..1736dd1 100644 --- a/src/packages/harp-device/src/harp/device/_schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/_schema/_emit.py @@ -1,7 +1,7 @@ import enum import types from dataclasses import dataclass -from typing import Any, Callable, Iterable, Mapping, Optional, Union, cast +from typing import Any, Callable, Iterable, Mapping, Optional, Union import numpy as np @@ -37,7 +37,6 @@ RegisterU64Array, StringConverter, StructPayload, - PayloadBase, ) from harp.protocol import RESERVED_FIELD_NAMES from harp.protocol import PayloadType as ProtoPayloadType @@ -381,15 +380,8 @@ def _payload_name(self, name: str, reg: Register) -> str: def _build_payload(self, name: str, reg: Register) -> type: payload_name = self._payload_name(name, reg) - cached = cast(type[PayloadBase[Any]], self.payloads.get(payload_name)) + cached = self.payloads.get(payload_name) if cached is not None: - # Reuse is keyed on the name alone (as the generator's payload list is), so - # refuse to hand back a class whose layout can't describe this register. - if cached.dtype.itemsize != np.dtype(_ELEMENT[reg.type]).itemsize * (reg.length or 1): - raise NameCollisionError( - f"{name}: interfaceType {payload_name!r} is already used by a payload " - f"of a different size; the two registers cannot share one payload class" - ) return cached payload = self._new_payload(payload_name, reg) self.payloads[payload_name] = payload diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index 28df59f..ea326a5 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -300,27 +300,53 @@ def test_structured_register_payload_named_after_interface_type(): assert regs["A"].payload_class is regs["B"].payload_class -def test_shared_payload_of_mismatched_size_raises(): - # Reuse is keyed on the interfaceType name alone, so a same-named payload over a - # different element width would silently mis-describe the second register. - with pytest.raises(NameCollisionError, match="different size"): - create_registers( - "registers:\n" - " A:\n" - " address: 40\n" - " type: U8\n" - " access: Read\n" - " interfaceType: Shared\n" - " payloadSpec:\n" - " Foo: {offset: 0}\n" - " B:\n" - " address: 41\n" - " type: U32\n" - " access: Read\n" - " interfaceType: Shared\n" - " payloadSpec:\n" - " Foo: {offset: 0}\n" - ) +def test_anchored_registers_share_one_payload_class(): + # How the published schemas actually reuse a payload: device.behavior anchors Rgb0 + # and merges it into Rgb1, so both carry the same interfaceType and payloadSpec by + # construction. Reuse is keyed on the name alone, matching the generator, which + # keeps one struct per interfaceType for the C# target too. + regs = create_registers( + "registers:\n" + " Rgb0: &rgbRegister\n" + " address: 71\n" + " type: U8\n" + " length: 3\n" + " access: Write\n" + " interfaceType: RgbPayload\n" + " payloadSpec:\n" + " Green: {offset: 0}\n" + " Red: {offset: 1}\n" + " Blue: {offset: 2}\n" + " Rgb1:\n" + " <<: *rgbRegister\n" + " address: 72\n" + ) + shared = regs["Rgb0"].payload_class + assert shared is regs["Rgb1"].payload_class + assert shared.__name__ == "RgbPayload" + assert shared.dtype.names == ("green", "red", "blue") + + +def test_shared_payload_spanning_elements_without_length_is_reused(): + # A payloadSpec may span several elements without declaring a length, in which case + # the payload takes its size from the member offsets. Sharing has to survive that, + # since nothing in the schema requires the length to be spelled out. + regs = create_registers( + "registers:\n" + " A: &shared\n" + " address: 40\n" + " type: U16\n" + " access: Read\n" + " interfaceType: Combo\n" + " payloadSpec:\n" + " Alpha: {offset: 0}\n" + " Beta: {offset: 1}\n" + " B:\n" + " <<: *shared\n" + " address: 41\n" + ) + assert regs["A"].payload_class is regs["B"].payload_class + assert regs["A"].payload_class.dtype.itemsize == 4 # --------------------------------------------------------------------------- From 9ebd50598cc6376c91a738cee1ceeb40163f65a4 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Tue, 11 Aug 2026 23:38:28 +0100 Subject: [PATCH 6/8] Reserve a payload_ prefix for payload members Payload members take a payload_ prefix or become private, so the public surface is payload_dtype, payload_array, payload_from_buffer and payload_as_columns. A field name is barred from those prefixes rather than from a list of the members that exist today, and is also rejected when it is a Python keyword or not a valid identifier. A schema field can no longer shadow a payload member. Keys such as ToColumns, RawPayload and Dtype now decode to ordinary fields, where before they replaced the member of that name, and a key such as Break is rejected rather than producing a field reachable only through getattr. A rejected field or an unusable register is reported against the register as the schema spells it, rather than against the payload class name the emitter derives from it. --- .../src/harp/benchmarks/benchmark.py | 6 +- .../src/harp/benchmarks/generate.py | 2 +- .../src/harp/benchmarks/register_models.py | 16 +-- .../harp-data/src/harp/data/_reader.py | 2 +- .../harp-data/src/harp/data/_write.py | 2 +- .../src/harp/device/_schema/_emit.py | 26 ++--- .../src/harp/protocol/__init__.py | 2 - .../src/harp/protocol/_payload.py | 98 ++++++++++--------- .../src/harp/protocol/_register.py | 32 +++--- tests/data/test_dataset.py | 2 +- tests/device/test_emit.py | 68 ++++++++----- tests/protocol/test_converter.py | 48 ++++----- tests/protocol/test_payload.py | 36 ++++--- tests/protocol/test_register.py | 56 ++++++----- tests/protocol/test_register_modeling.py | 26 ++--- 15 files changed, 229 insertions(+), 193 deletions(-) diff --git a/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py b/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py index 2d75563..e567f6c 100644 --- a/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py +++ b/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py @@ -84,7 +84,7 @@ def benchmark_register(reg: BenchmarkedRegister, path: Path, *, runs: int) -> Re register = reg.register raw = path.read_bytes() file_bytes = len(raw) - payload_bytes = register.payload_class.dtype.itemsize + payload_bytes = register.payload_class.payload_dtype.itemsize frames, stride = _dataset_info(raw, payload_bytes) bulk_pre = _time( @@ -104,7 +104,7 @@ def benchmark_register(reg: BenchmarkedRegister, path: Path, *, runs: int) -> Re # pandas DataFrame construction. Matches parse_to_dataframe's decode options. _, _, _, payload = register.parse_bulk(raw, parse_timestamp=True) cols = _time( - lambda: payload.to_columns(decode_enums=True, demux_bit_masks=False), + lambda: payload.payload_as_columns(decode_enums=True, demux_bit_masks=False), runs=runs, frames=frames, file_bytes=file_bytes, @@ -212,7 +212,7 @@ def _table( lines.append("## `to_columns` (decode only — where converters run)\n") lines.append( "Isolates the decode step: `parse_bulk` views are built once up front, then " - "only `payload.to_columns()` is timed. This is where each field's " + "only `payload.payload_as_columns()` is timed. This is where each field's " "`converter.decode_batch` executes. Registers whose converters loop in Python " "(`HarpVersionConverter`, `StringConverter`, `BytesToIntConverter` → object " "dtype) dominate here; vectorized converters stay cheap.\n" diff --git a/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py b/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py index 1567a9d..222dd58 100644 --- a/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py +++ b/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py @@ -22,7 +22,7 @@ def _frames(reg: BenchmarkedRegister, entries: int) -> np.ndarray: decoded (``to_columns`` / ``parse_to_dataframe``) during the benchmark. Timestamps, when present, are a monotonic ramp. Returns the flat uint8 wire buffer. """ - dtype = reg.register.payload_class.dtype + dtype = reg.register.payload_class.payload_dtype rng = np.random.default_rng(_SEED + reg.address) records = rng.integers(0, 128, size=entries * dtype.itemsize, dtype=np.uint8).view(dtype) timestamps = np.arange(entries, dtype=np.float64) if reg.timestamped else None diff --git a/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py b/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py index bacb185..510a18c 100644 --- a/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py +++ b/src/packages/harp-benchmarks/src/harp/benchmarks/register_models.py @@ -333,7 +333,7 @@ def main() -> None: # pragma: no cover - manual exploration entry point p = _roundtrip(AnalogData, ad) assert float(p.analog0) == 1.0 and float(p.analog2) == 3.0 np.testing.assert_array_equal(p.accelerometer, [4, 5, 6]) - print(f"AnalogData OK ({AnalogDataPayload.dtype.itemsize} bytes)") + print(f"AnalogData OK ({AnalogDataPayload.payload_dtype.itemsize} bytes)") cc = ComplexConfigurationPayload( pwm_port=PwmPort.PWM2, @@ -345,10 +345,10 @@ def main() -> None: # pragma: no cover - manual exploration entry point p = _roundtrip(ComplexConfiguration, cc) assert p.pwm_port == PwmPort.PWM2 and p.events_enabled is True and int(p.delta) == 42 assert float(p.duty_cycle) == 0.5 - assert ComplexConfigurationPayload.dtype.itemsize == 17 - assert cc.raw_payload.tobytes()[1:4] == b"\x00\x00\x00" + assert ComplexConfigurationPayload.payload_dtype.itemsize == 17 + assert cc.payload_array.tobytes()[1:4] == b"\x00\x00\x00" print( - f"ComplexConfiguration OK ({ComplexConfigurationPayload.dtype.itemsize} bytes, gap 1..3)" + f"ComplexConfiguration OK ({ComplexConfigurationPayload.payload_dtype.itemsize} bytes, gap 1..3)" ) ver = VersionPayload( @@ -361,7 +361,7 @@ def main() -> None: # pragma: no cover - manual exploration entry point p = _roundtrip(Version, ver) assert p.protocol_version == HarpVersion(2, 0, 0) and p.core_id == "abc" np.testing.assert_array_equal(p.interface_hash, np.arange(20)) - print(f"Version OK ({VersionPayload.dtype.itemsize} bytes)") + print(f"Version OK ({VersionPayload.payload_dtype.itemsize} bytes)") p = _roundtrip(CustomPayload, HarpVersion(3, 1, 4)) assert p == HarpVersion(3, 1, 4) # single-member unwrap -> bare HarpVersion @@ -377,7 +377,7 @@ def main() -> None: # pragma: no cover - manual exploration entry point p = _roundtrip(BitmaskSplitter, BitmaskSplitterPayload(low=0xA, high=0x5)) assert int(p.low) == 0xA and int(p.high) == 0x5 - assert p.raw_payload.tobytes() == bytes([0x5A]) + assert p.payload_array.tobytes() == bytes([0x5A]) print("BitmaskSplitter OK") assert int(_roundtrip(Counter0, np.int32(-100000))) == -100000 @@ -386,7 +386,7 @@ def main() -> None: # pragma: no cover - manual exploration entry point p = _roundtrip(PortDIOSet, PortDigitalIOS.DIO0 | PortDigitalIOS.DIO3) assert p == PortDigitalIOS.DIO0 | PortDigitalIOS.DIO3 # single-member unwrap assert PortDigitalIOS.DIO1 not in p - assert PortDIOSetPayload.dtype.itemsize == 1 + assert PortDIOSetPayload.payload_dtype.itemsize == 1 print("PortDIOSet OK") assert int(_roundtrip(PulseDOPort0, np.uint16(5))) == 5 @@ -410,7 +410,7 @@ def main() -> None: # pragma: no cover - manual exploration entry point ) assert p.digital_output == PwmPort.PWM1 and int(p.pulse_width) == 300 assert int(p.frequency) == 200 and int(p.pulse_count) == 50 - assert StartPulseTrainPayload.dtype.itemsize == 4 + assert StartPulseTrainPayload.payload_dtype.itemsize == 4 assert int(StartPulseTrainPayload(pulse_count=np.uint8(3)).frequency) == 1 # defaultValue print("StartPulseTrain OK (4 masked members, 2 words, default frequency=1)") diff --git a/src/packages/harp-data/src/harp/data/_reader.py b/src/packages/harp-data/src/harp/data/_reader.py index f596a7b..855da48 100644 --- a/src/packages/harp-data/src/harp/data/_reader.py +++ b/src/packages/harp-data/src/harp/data/_reader.py @@ -50,7 +50,7 @@ def payload_to_dataframe( # TODO: we may need to account for cases where columns have the same name. # this can happen when demuxing bitmasks, for example, where each bitmask column # is expanded into multiple boolean columns with the same name. - cols = payload.to_columns(decode_enums=decode_enums, demux_bit_masks=demux_bit_masks) + cols = payload.payload_as_columns(decode_enums=decode_enums, demux_bit_masks=demux_bit_masks) return pd.DataFrame( { (c.name if c.name is not None else _DEFAULT_COLUMN_NAME): ( diff --git a/src/packages/harp-data/src/harp/data/_write.py b/src/packages/harp-data/src/harp/data/_write.py index ab48c58..8da9b03 100644 --- a/src/packages/harp-data/src/harp/data/_write.py +++ b/src/packages/harp-data/src/harp/data/_write.py @@ -23,7 +23,7 @@ def to_buffer( """Encode ``values`` as a flat buffer of ``register`` frames. ``values`` is a payload (scalar or batch) or an ndarray of the register's - ``payload_class.dtype``; ``timestamps`` (length-N seconds) makes every frame + ``payload_class.payload_dtype``; ``timestamps`` (length-N seconds) makes every frame timestamped; ``message_type`` is one :class:`MessageType` or a length-N array (e.g. the msgtype view from ``parse_bulk``). """ 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 1736dd1..d139f4c 100644 --- a/src/packages/harp-device/src/harp/device/_schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/_schema/_emit.py @@ -38,7 +38,7 @@ StringConverter, StructPayload, ) -from harp.protocol import RESERVED_FIELD_NAMES +from harp.protocol._payload import _reserved_field_reason from harp.protocol import PayloadType as ProtoPayloadType from ._model import DeviceModel, PayloadMember, PayloadType, Register, Registers, Visibility @@ -260,11 +260,13 @@ def _rename( f"{owner}: {kind}s {clash!r} and {key!r} both map to {name!r}; " f"rename one in the schema" ) - if reserved and name in RESERVED_FIELD_NAMES: - raise NameCollisionError( - f"{owner}: {kind} {key!r} maps to {name!r}, which is reserved by " - f"the payload base class; rename it in the schema" - ) + if reserved: + unusable = _reserved_field_reason(name) + if unusable is not None: + raise NameCollisionError( + f"{owner}: {kind} {key!r} maps to {name!r}, which {unusable}; " + f"rename it in the schema" + ) origin[name] = key renamed[key] = name return renamed @@ -383,23 +385,23 @@ def _build_payload(self, name: str, reg: Register) -> type: cached = self.payloads.get(payload_name) if cached is not None: return cached - payload = self._new_payload(payload_name, reg) + payload = self._new_payload(payload_name, name, reg) self.payloads[payload_name] = payload return payload - def _new_payload(self, name: str, reg: Register) -> type: + def _new_payload(self, class_name: str, owner: str, reg: Register) -> type: elem_np = _ELEMENT[reg.type] elem_size = np.dtype(elem_np).itemsize length = reg.length or 1 if reg.payloadSpec is not None: - renamed = self._rename("field", name, reg.payloadSpec, field_name, reserved=True) + renamed = self._rename("field", owner, reg.payloadSpec, field_name, reserved=True) namespace = { renamed[key]: self._build_field(key, member, reg) for key, member in reg.payloadSpec.items() } kwds = {"length": length} if length > 1 else {} - return _new_class(name, (StructPayload[elem_np],), namespace, kwds) + return _new_class(class_name, (StructPayload[elem_np],), namespace, kwds) # anonymous single-value payload mt = reg.maskType.root if reg.maskType else None @@ -411,7 +413,7 @@ def _new_payload(self, name: str, reg: Register) -> type: descriptor = BitMask(enum=self.enums[mt]) else: assert it is not None, ( - f"{name}: register needs a payloadSpec, maskType, or interfaceType" + f"{owner}: register needs a payloadSpec, maskType, or interfaceType" ) ctx = ConverterContext( name="__value__", @@ -422,7 +424,7 @@ def _new_payload(self, name: str, reg: Register) -> type: element_size=elem_size, ) descriptor = Field(self._resolve_converter(ctx)) - return _new_class(name, (AnonymousPayload[elem_np],), {"__value__": descriptor}) + return _new_class(class_name, (AnonymousPayload[elem_np],), {"__value__": descriptor}) # -- registers -------------------------------------------------------- def _class_name(self, name: str, reg: Register) -> str: diff --git a/src/packages/harp-protocol/src/harp/protocol/__init__.py b/src/packages/harp-protocol/src/harp/protocol/__init__.py index 741e174..abce77c 100644 --- a/src/packages/harp-protocol/src/harp/protocol/__init__.py +++ b/src/packages/harp-protocol/src/harp/protocol/__init__.py @@ -10,7 +10,6 @@ HarpVersion, ) from ._payload import ( - RESERVED_FIELD_NAMES, PayloadBase, StructPayload, Field, @@ -89,7 +88,6 @@ "PayloadBase", "StructPayload", "AnonymousPayload", - "RESERVED_FIELD_NAMES", "Column", "Field", "GroupMask", diff --git a/src/packages/harp-protocol/src/harp/protocol/_payload.py b/src/packages/harp-protocol/src/harp/protocol/_payload.py index 359a49c..cac281c 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_payload.py +++ b/src/packages/harp-protocol/src/harp/protocol/_payload.py @@ -1,3 +1,4 @@ +import keyword import enum from dataclasses import dataclass from typing import ( @@ -497,19 +498,19 @@ class Batch(Protocol[_PT]): Statically, ``Batch[P]`` is a distinct type from ``P`` so the type checker knows ``read_frames`` returns an ndarray-shaped view rather than a single record. At runtime, the value is the auto-derived - ``P.Batch`` sibling whose descriptors return ``NDArray`` views. + ``P._batch`` sibling whose descriptors return ``NDArray`` views. Per-field dtype precision is intentionally dropped — every declared field reports ``NDArray[Any]`` — to keep ``RegisterBase[P]`` parameterized by a single TypeVar. """ - raw_payload: "NDArray[Any]" + payload_array: "NDArray[Any]" value: "NDArray[Any]" def __len__(self) -> int: ... # type: ignore[empty-body] - def to_columns( # type: ignore[empty-body] + def payload_as_columns( # type: ignore[empty-body] self, *, decode_enums: bool = True, demux_bit_masks: bool = False ) -> "list[Column]": ... @@ -522,25 +523,24 @@ def __getattr__(self, name: str) -> "NDArray[Any]": ... # type: ignore[empty-bo _DECLARATION_TYPES = _SCALAR_DECLARATION_TYPES + _BATCH_DECLARATION_TYPES -# value/raw_payload deliberately omitted: overriding them is the intended -# pattern for single-slot converter-driven payloads. -#: Attribute names :class:`PayloadBase` owns; a field may not shadow one. Exposed so -#: code that derives field names from an external schema can reject a clash up front -#: with a message naming the original identifier. -RESERVED_FIELD_NAMES = frozenset( - { - "_arr", - "_dtype", - "_repr_fields", - "Batch", - "dtype", - "_scalar_cls", - "_batch_cls", - "_defaults", - "_elem_dtype", - "_single_member", - } -) +#: Every member the payload classes own carries one of these prefixes, so a field name +#: is barred from them rather than from a list of the members themselves. Dunders are +#: exempt because ``__value__`` is how a single-slot payload declares its root field. +_RESERVED_FIELD_PREFIXES = ("_", "payload_") + + +def _reserved_field_reason(name: str) -> "str | None": + """Returns why ``name`` cannot be a payload field, or ``None`` when it can.""" + if name.startswith("__") and name.endswith("__"): + return None + if not name.isidentifier(): + return "is not a valid Python identifier" + if keyword.iskeyword(name): + return "is a Python keyword" + for prefix in _RESERVED_FIELD_PREFIXES: + if name.startswith(prefix): + return f"starts with {prefix!r}, which is reserved for payload members" + return None def _batch_init_disabled(self: "PayloadBase", *args: object, **kwargs: object) -> None: @@ -659,7 +659,7 @@ class PayloadBase(Generic[NpStructT]): """ # Structured numpy dtype describing the memory layout of a single payload record. - dtype: ClassVar[np.dtype] + payload_dtype: ClassVar[np.dtype] # Field names shown in __repr__ and used as the column order. _repr_fields: ClassVar[tuple[str, ...]] # The scalar twin of this class (identity for scalar classes, points to scalar from Batch). @@ -669,7 +669,7 @@ class PayloadBase(Generic[NpStructT]): # Cached map of attribute name → default value for fields that declare one. _defaults: ClassVar[dict[str, Any]] # Auto-generated sibling class whose descriptors return NDArray views instead of scalars. - Batch: ClassVar["type[PayloadBase]"] + _batch: ClassVar["type[PayloadBase]"] # Base element dtype (from the ``StructPayload[...]`` type arg); governs offset # arithmetic and the integer width used for masked reads. Defaults to uint8. _elem_dtype: ClassVar[np.dtype] = _DEFAULT_ELEMENT @@ -680,7 +680,7 @@ class PayloadBase(Generic[NpStructT]): def __init__(self, *args: object, **kwargs: object) -> None: cls = type(self) - names = self.dtype.names + names = self.payload_dtype.names if names is None: raise TypeError(f"{type(self).__name__}.dtype has no named fields") @@ -701,7 +701,7 @@ def __init__(self, *args: object, **kwargs: object) -> None: merged.update(kwargs) kwargs = merged - arr = np.zeros((), dtype=self.dtype) + arr = np.zeros((), dtype=self.payload_dtype) # Route each kwarg by its descriptor kind, not by whether its name happens # to match a numpy slot — masked descriptors may share a slot whose name @@ -769,7 +769,7 @@ def __init_subclass__( if _batch_of is not None: # Auto-generated Batch sibling: borrow dtype/_repr_fields from its # scalar twin and wire the scalar↔batch pointers. - cls.dtype = _batch_of.dtype + cls.payload_dtype = _batch_of.payload_dtype cls._repr_fields = _batch_of._repr_fields cls._elem_dtype = _batch_of._elem_dtype cls._single_member = _batch_of._single_member @@ -782,8 +782,10 @@ def __init_subclass__( cls._single_member = None for name, val in cls.__dict__.items(): - if isinstance(val, _DECLARATION_TYPES) and name in RESERVED_FIELD_NAMES: - raise TypeError(f"{cls.__name__}: field name {name!r} is reserved by PayloadBase") + if isinstance(val, _DECLARATION_TYPES): + reason = _reserved_field_reason(name) + if reason is not None: + raise TypeError(f"{cls.__name__}: field name {name!r} {reason}") own_declarations = [ (name, val) @@ -792,7 +794,7 @@ def __init_subclass__( ] if own_declarations: - cls.dtype = _build_struct_dtype(cls, own_declarations, length) + cls.payload_dtype = _build_struct_dtype(cls, own_declarations, length) # Only an AnonymousPayload root (its lone __value__ field) unwraps on # parse; a StructPayload always returns the wrapper, never auto-unwraps. if getattr(cls, "_root", False): @@ -804,12 +806,12 @@ def __init_subclass__( cls._scalar_cls = cls cls._batch_cls = cls # rebound below once Batch is generated - if hasattr(cls, "dtype"): + if hasattr(cls, "payload_dtype"): batch_attrs: dict[str, Any] = {"__init__": _batch_init_disabled} for name, val in cls.__dict__.items(): if isinstance(val, _SCALAR_DECLARATION_TYPES): batch_attrs[name] = val._to_batch() - cls.Batch = type( + cls._batch = type( f"{cls.__name__}Batch", (cls,), batch_attrs, @@ -819,22 +821,22 @@ def __init_subclass__( cls._defaults = cls._collect_defaults() @classmethod - def from_array(cls, arr: "np.ndarray") -> Self: + def _from_array(cls, arr: "np.ndarray") -> Self: target = cls._scalar_cls if arr.ndim == 0 else cls._batch_cls obj = target.__new__(target) obj._arr = arr return obj # type: ignore[return-value] @classmethod - def from_buffer(cls, buf: bytes | bytearray | memoryview) -> Self: - arr = np.frombuffer(buf, dtype=cls.dtype) - return cls.from_array(arr[0] if len(arr) == 1 else arr) + def payload_from_buffer(cls, buf: bytes | bytearray | memoryview) -> Self: + arr = np.frombuffer(buf, dtype=cls.payload_dtype) + return cls._from_array(arr[0] if len(arr) == 1 else arr) @property - def raw_payload(self) -> NDArray[NpStructT]: + def payload_array(self) -> NDArray[NpStructT]: return self._arr - def to_columns( + def payload_as_columns( self, *, decode_enums: bool = True, demux_bit_masks: bool = False ) -> list[Column]: """Returns a list of Column where each member represents a field from a payload across multiple messages. @@ -871,7 +873,7 @@ def __str__(self) -> str: return repr(self) @classmethod - def unwrap(cls, arr: "np.ndarray") -> Any: + def _unwrap(cls, arr: "np.ndarray") -> Any: """Dispatch hook used by ``RegisterBase.parse``. Struct payloads always return a typed wrapper so descriptors like @@ -880,7 +882,7 @@ def unwrap(cls, arr: "np.ndarray") -> Any: the unwrapped ``__value__`` (the single-member branch below, reached via the override's ``super()`` call). A struct payload never auto-unwraps. """ - obj = cls.from_array(arr) + obj = cls._from_array(arr) if cls._single_member is not None and arr.ndim == 0: return getattr(obj, cls._single_member) return obj @@ -992,14 +994,18 @@ def __init_subclass__( super().__init_subclass__(**kwargs) # pyright: ignore[reportArgumentType] return # Raw scalar slot required, unless a Batch twin / array concrete supplies dtype. - if scalar_dtype is None and "_batch_of" not in kwargs and "dtype" not in cls.__dict__: + if ( + scalar_dtype is None + and "_batch_of" not in kwargs + and "payload_dtype" not in cls.__dict__ + ): raise TypeError( f"{cls.__name__}: an AnonymousPayload subclass must define its single slot via a " f"{cls._VALUE_FIELD!r} descriptor field or scalar_dtype= (a codec is a " f"{cls._VALUE_FIELD!r} Field with a Converter)." ) if scalar_dtype is not None: - cls.dtype = np.dtype(scalar_dtype) + cls.payload_dtype = np.dtype(scalar_dtype) cls._repr_fields = () super().__init_subclass__(**kwargs) # pyright: ignore[reportArgumentType] @@ -1018,12 +1024,12 @@ def __init__(self, value: object = _MISSING_INIT, /, **kwargs: object) -> None: raise TypeError(f"{type(self).__name__}() requires a value") if kwargs: raise TypeError(f"{type(self).__name__}() got unexpected kwargs: {sorted(kwargs)}") - self._arr = np.asarray(value, dtype=self.dtype) + self._arr = np.asarray(value, dtype=self.payload_dtype) @classmethod - def unwrap(cls, arr: "np.ndarray") -> Any: + def _unwrap(cls, arr: "np.ndarray") -> Any: if cls._root: - return super().unwrap(arr) # PayloadBase single-member unwrap (.__value__) + return super()._unwrap(arr) # PayloadBase single-member unwrap (.__value__) # 0-D → numpy scalar via item-like access (preserves dtype). # 1-D / sub-array → return the ndarray as-is. return arr if arr.ndim > 0 else arr[()] @@ -1036,7 +1042,7 @@ def _repr_kwargs(self) -> str: def __repr__(self) -> str: return f"{type(self).__name__}({self._repr_kwargs()})" - def to_columns( + def payload_as_columns( self, *, decode_enums: bool = True, demux_bit_masks: bool = False ) -> list[Column]: # Anonymous values carry no name (name=None); the consumer supplies the label. diff --git a/src/packages/harp-protocol/src/harp/protocol/_register.py b/src/packages/harp-protocol/src/harp/protocol/_register.py index 4e07864..21ce8ee 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_register.py +++ b/src/packages/harp-protocol/src/harp/protocol/_register.py @@ -148,8 +148,8 @@ def parse(cls, value: HarpMessage | bytes | bytearray | memoryview) -> U: registers) return the raw numpy scalar or ndarray directly. """ buf = value.payload if isinstance(value, HarpMessage) else value - record = np.frombuffer(buf, dtype=cls.payload_class.dtype, count=1)[0] - return cast(U, cls.payload_class.unwrap(record)) + record = np.frombuffer(buf, dtype=cls.payload_class.payload_dtype, count=1)[0] + return cast(U, cls.payload_class._unwrap(record)) @classmethod def parse_bulk( @@ -165,7 +165,7 @@ def parse_bulk( if len(data) == 0: # No frames, but still need to return a Batch with the right dtype. - payload = payload_cls.from_array(np.empty(0, dtype=payload_cls.dtype)) + payload = payload_cls._from_array(np.empty(0, dtype=payload_cls.payload_dtype)) return data, None, None, cast("Batch[Any]", payload) stride = ( @@ -189,13 +189,13 @@ def parse_bulk( payload_arr = np.ndarray( nrows, - dtype=payload_cls.dtype, + dtype=payload_cls.payload_dtype, buffer=data, offset=payload_offset, strides=stride, ) - payload = payload_cls.from_array(payload_arr) + payload = payload_cls._from_array(payload_arr) return data, timestamps, msgtype_view, cast("Batch[Any]", payload) @classmethod @@ -211,15 +211,15 @@ def format_bulk( :meth:`parse_bulk`. ``values`` is a payload (scalar or :class:`Batch`) or an ndarray of the - register's ``payload_class.dtype``. ``timestamps`` (a length-N array of + register's ``payload_class.payload_dtype``. ``timestamps`` (a length-N array of seconds) makes every frame timestamped. ``message_type`` is one :class:`MessageType` for all frames, or a length-N array of message-type bytes / values (e.g. the ``msgtype`` view returned by ``parse_bulk``). """ payload_cls = cls.payload_class - itemsize = payload_cls.dtype.itemsize + itemsize = payload_cls.payload_dtype.itemsize if isinstance(values, PayloadBase): - records = np.atleast_1d(np.asarray(values.raw_payload)) + records = np.atleast_1d(np.asarray(values.payload_array)) else: records = np.atleast_1d(np.asarray(values)) # Coerce the element type only for plain scalar payloads (e.g. an int @@ -228,11 +228,11 @@ def format_bulk( plain = ( records.dtype.names is None and records.dtype.subdtype is None - and payload_cls.dtype.names is None - and payload_cls.dtype.subdtype is None + and payload_cls.payload_dtype.names is None + and payload_cls.payload_dtype.subdtype is None ) - if plain and records.dtype != payload_cls.dtype: - records = records.astype(payload_cls.dtype) + if plain and records.dtype != payload_cls.payload_dtype: + records = records.astype(payload_cls.payload_dtype) nrows = len(records) flat = np.ascontiguousarray(records).tobytes() if len(flat) != nrows * itemsize: @@ -309,14 +309,14 @@ def format( else: mt = MessageType.Write if message_type is None else message_type if isinstance(value, PayloadBase): - raw = value.raw_payload.tobytes() + raw = value.payload_array.tobytes() elif isinstance(value, np.ndarray): raw = value.tobytes() else: # A bare high-level value (the symmetric counterpart of what # parse() returns): let the payload class encode it, so any # converter (e.g. a str via StringConverter) is applied. - raw = cls.payload_class(value).raw_payload.tobytes() + raw = cls.payload_class(value).payload_array.tobytes() return build_message_frame( mt, cls.address, cls.payload_type, raw, port=port, timestamp=timestamp ) @@ -393,12 +393,12 @@ def __call__(cls: "type[_AR]", address: int, *, length: int) -> "type[_AR]": # # Anonymous payloads carry a plain (non-structured) dtype. The array # variant uses a sub-dtype (inner_dtype, (length,)) so a single buffer # element decodes directly to an ndarray of shape (length,). - inner = base_payload.dtype + inner = base_payload.payload_dtype sub_dtype = np.dtype((inner, (length,))) concrete_payload = type( f"{base_payload.__name__}_{length}", (base_payload,), - {"dtype": sub_dtype}, + {"payload_dtype": sub_dtype}, ) return cast( "type[_AR]", diff --git a/tests/data/test_dataset.py b/tests/data/test_dataset.py index ba30ee1..0aea937 100644 --- a/tests/data/test_dataset.py +++ b/tests/data/test_dataset.py @@ -13,7 +13,7 @@ def _records(cls, n, seed): - dtype = cls.payload_class.dtype + dtype = cls.payload_class.payload_dtype rng = np.random.default_rng(seed) raw = rng.integers(0, 128, size=n * dtype.itemsize, dtype=np.uint8) return raw.view(dtype).copy() diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index ea326a5..597a0fc 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -46,7 +46,9 @@ def test_device_register_matches_generator_layout(name, device_registers): expected = _device_registers()[name] assert emitted.address == expected.address assert emitted.payload_type == expected.payload_type - assert _layout(emitted.payload_class.dtype) == _layout(expected.payload_class.dtype) + assert _layout(emitted.payload_class.payload_dtype) == _layout( + expected.payload_class.payload_dtype + ) def test_device_emits_all_registers(device_registers): @@ -63,13 +65,13 @@ def _enum_of(reg, field): def test_field_names_are_snake_case(device_registers): - fields = device_registers["AnalogData"].payload_class.dtype.names + fields = device_registers["AnalogData"].payload_class.payload_dtype.names assert fields == ("analog0", "analog1", "analog2", "accelerometer") def test_field_names_match_generator(device_registers): # A run of capitals stays one word; a trailing digit never separates. - fields = device_registers["Version"].payload_class.dtype.names + fields = device_registers["Version"].payload_class.payload_dtype.names assert fields == ( "protocol_version", "firmware_version", @@ -114,7 +116,7 @@ def test_enum_names_match_generator_for_every_enum(device_registers): """Every enum the golden module declares has identical members in the emitter.""" for name, reg in _device_registers().items(): payload = reg.payload_class - if payload.dtype.names is None: + if payload.payload_dtype.names is None: continue for field in payload._repr_fields: expected_desc = payload._mro_descriptor(field) @@ -143,12 +145,17 @@ def test_core_register_structural(name, common_yml): expected = _core_expected()[name] assert emitted.address == expected.address assert emitted.payload_type == expected.payload_type - assert emitted.payload_class.dtype.itemsize == expected.payload_class.dtype.itemsize + assert ( + emitted.payload_class.payload_dtype.itemsize + == expected.payload_class.payload_dtype.itemsize + ) if name == "DeviceName": # Generator enriches DeviceName to interfaceType: string; protocol's # common.yml does not, so only the layout size matches here. return - assert _layout(emitted.payload_class.dtype) == _layout(expected.payload_class.dtype) + assert _layout(emitted.payload_class.payload_dtype) == _layout( + expected.payload_class.payload_dtype + ) # --------------------------------------------------------------------------- @@ -209,7 +216,7 @@ def test_non_strict_falls_back_to_native(device_yml): regs = create_registers(device_yml, strict=False) # Data decodes as the raw native element (u8[2]) rather than the custom int. reg = regs["CustomMemberConverter"] - assert reg.payload_class.dtype.itemsize == 3 + assert reg.payload_class.payload_dtype.itemsize == 3 def test_converter_factory_receives_dsl_context(device_yml): @@ -324,7 +331,7 @@ def test_anchored_registers_share_one_payload_class(): shared = regs["Rgb0"].payload_class assert shared is regs["Rgb1"].payload_class assert shared.__name__ == "RgbPayload" - assert shared.dtype.names == ("green", "red", "blue") + assert shared.payload_dtype.names == ("green", "red", "blue") def test_shared_payload_spanning_elements_without_length_is_reused(): @@ -346,7 +353,7 @@ def test_shared_payload_spanning_elements_without_length_is_reused(): " address: 41\n" ) assert regs["A"].payload_class is regs["B"].payload_class - assert regs["A"].payload_class.dtype.itemsize == 4 + assert regs["A"].payload_class.payload_dtype.itemsize == 4 # --------------------------------------------------------------------------- @@ -381,18 +388,33 @@ def test_colliding_enum_members_raise(): ) -def test_field_name_shadowing_payload_attribute_raises(): - with pytest.raises(NameCollisionError, match="reserved"): - create_registers( - "registers:\n" - " R:\n" - " address: 40\n" - " type: U8\n" - " access: Read\n" - " payloadSpec:\n" - " Dtype: {offset: 0}\n" - " Other: {offset: 1}\n" - ) +def _one_field_schema(key: str) -> str: + return ( + "registers:\n" + " R:\n" + " address: 40\n" + " type: U8\n" + " access: Read\n" + " payloadSpec:\n" + f" {key}: {{offset: 0}}\n" + " Other: {offset: 1}\n" + ) + + +@pytest.mark.parametrize("key", ["PayloadDtype", "PayloadColumns", "PayloadAnything"]) +def test_field_name_taking_reserved_prefix_raises(key): + # Payload members all carry the payload_ prefix, so a field is barred from the + # prefix rather than from a list of the members that happen to exist today. + with pytest.raises(NameCollisionError, match="reserved for payload members"): + create_registers(_one_field_schema(key)) + + +@pytest.mark.parametrize("key", ["Break", "Class", "Return"]) +def test_field_name_renaming_to_keyword_raises(key): + # `Break` renames to `break`, which is only reachable through getattr and is a + # syntax error in a statically generated module. + with pytest.raises(NameCollisionError, match="is a Python keyword"): + create_registers(_one_field_schema(key)) # --------------------------------------------------------------------------- @@ -417,7 +439,9 @@ def _random_records(dtype, n, seed): def test_emitted_register_bulk_matches_oracle(name, device_registers): emitted = device_registers[name] oracle = _device_registers()[name] - records = _random_records(emitted.payload_class.dtype, 5, seed=zlib.crc32(name.encode())) + records = _random_records( + emitted.payload_class.payload_dtype, 5, seed=zlib.crc32(name.encode()) + ) # Cross-write: same address / payload_type / byte layout -> identical wire bytes. buf = bytes(emitted.format_bulk(records)) diff --git a/tests/protocol/test_converter.py b/tests/protocol/test_converter.py index 901095d..861c1d2 100644 --- a/tests/protocol/test_converter.py +++ b/tests/protocol/test_converter.py @@ -49,15 +49,15 @@ class _NumericPayload(PayloadBase): def test_identity_converter_scalar_view(): - rec = np.array((-7, 99), dtype=_NumericPayload.dtype) - p = _NumericPayload.from_array(rec) + rec = np.array((-7, 99), dtype=_NumericPayload.payload_dtype) + p = _NumericPayload._from_array(rec) assert int(p.a) == -7 assert int(p.b) == 99 def test_identity_converter_batch_view(): - arr = np.array([(-7, 99), (1, 2)], dtype=_NumericPayload.dtype) - p = _NumericPayload.from_buffer(arr.tobytes()) + arr = np.array([(-7, 99), (1, 2)], dtype=_NumericPayload.payload_dtype) + p = _NumericPayload.payload_from_buffer(arr.tobytes()) np.testing.assert_array_equal(p.a, [-7, 1]) np.testing.assert_array_equal(p.b, [99, 2]) @@ -74,16 +74,16 @@ class DeclaredPayload(PayloadBase): def test_declared_dtype_synthesised_from_fields(): # Field declarations alone build a structured dtype in declaration order. - assert DeclaredPayload.dtype.names == ("delta", "flag") - assert DeclaredPayload.dtype.fields["delta"][0] == np.dtype(np.uint32) - assert DeclaredPayload.dtype.fields["flag"][0] == np.dtype(np.uint8) + assert DeclaredPayload.payload_dtype.names == ("delta", "flag") + assert DeclaredPayload.payload_dtype.fields["delta"][0] == np.dtype(np.uint32) + assert DeclaredPayload.payload_dtype.fields["flag"][0] == np.dtype(np.uint8) def test_declared_dtype_kwarg_init_round_trip(): p = DeclaredPayload(delta=42, flag=1) assert int(p.delta) == 42 assert int(p.flag) == 1 - p2 = DeclaredPayload.from_buffer(p.raw_payload.tobytes()) + p2 = DeclaredPayload.payload_from_buffer(p.payload_array.tobytes()) assert int(p2.delta) == 42 assert int(p2.flag) == 1 @@ -100,8 +100,8 @@ class _NamedPayload(PayloadBase): def test_string_converter_dtype_synthesis(): # name should occupy 8 bytes, delta 2 bytes. - assert _NamedPayload.dtype.itemsize == 10 - assert _NamedPayload.dtype.fields["name"][0].subdtype is not None + assert _NamedPayload.payload_dtype.itemsize == 10 + assert _NamedPayload.payload_dtype.fields["name"][0].subdtype is not None def test_string_converter_scalar_decode_roundtrip(): @@ -109,23 +109,23 @@ def test_string_converter_scalar_decode_roundtrip(): assert p.name == "abc" assert int(p.delta) == 7 # raw bytes are zero-padded - raw = bytes(p.raw_payload["name"]) + raw = bytes(p.payload_array["name"]) assert raw == b"abc\x00\x00\x00\x00\x00" def test_string_converter_batch_decode(): - rec1 = _NamedPayload(name="hi", delta=1).raw_payload.tobytes() - rec2 = _NamedPayload(name="bye", delta=2).raw_payload.tobytes() - batch = _NamedPayload.from_buffer(rec1 + rec2) + rec1 = _NamedPayload(name="hi", delta=1).payload_array.tobytes() + rec2 = _NamedPayload(name="bye", delta=2).payload_array.tobytes() + batch = _NamedPayload.payload_from_buffer(rec1 + rec2) names = batch.name assert list(names) == ["hi", "bye"] np.testing.assert_array_equal(batch.delta, [1, 2]) def test_string_converter_to_dataframe(): - rec1 = _NamedPayload(name="hi", delta=1).raw_payload.tobytes() - rec2 = _NamedPayload(name="bye", delta=2).raw_payload.tobytes() - batch = _NamedPayload.from_buffer(rec1 + rec2) + rec1 = _NamedPayload(name="hi", delta=1).payload_array.tobytes() + rec2 = _NamedPayload(name="bye", delta=2).payload_array.tobytes() + batch = _NamedPayload.payload_from_buffer(rec1 + rec2) df = payload_to_dataframe(batch) # Non-identity converter produces one column per field — no sub-array # expansion for the string field. @@ -155,9 +155,9 @@ def test_groupmask_struct_field_scalar_decode(): def test_groupmask_struct_field_batch_decode_returns_ints(): - p1 = _ConfigPayload(color=_Color.Red, delta=1).raw_payload.tobytes() - p2 = _ConfigPayload(color=_Color.Blue, delta=2).raw_payload.tobytes() - batch = _ConfigPayload.from_buffer(p1 + p2) + p1 = _ConfigPayload(color=_Color.Red, delta=1).payload_array.tobytes() + p2 = _ConfigPayload(color=_Color.Blue, delta=2).payload_array.tobytes() + batch = _ConfigPayload.payload_from_buffer(p1 + p2) np.testing.assert_array_equal(batch.color, [int(_Color.Red), int(_Color.Blue)]) @@ -195,14 +195,14 @@ class _Flags(PayloadBase): group = GroupMask(mask=0x06, enum=_Color) # 0-D scalar record: flag=1, group bits=01 (Green) - scalar = _Flags.from_array(np.array((0x03,), dtype=_Flags.dtype)) + scalar = _Flags._from_array(np.array((0x03,), dtype=_Flags.payload_dtype)) assert type(scalar) is _Flags assert scalar.flag is _Flag.A assert scalar.group is _Color.Green # 1-D batch — Batch sibling, ndarray-typed accessors. - batch = _Flags.from_buffer(bytes([0x01, 0x02])) - assert type(batch) is _Flags.Batch + batch = _Flags.payload_from_buffer(bytes([0x01, 0x02])) + assert type(batch) is _Flags._batch assert isinstance(batch, _Flags) np.testing.assert_array_equal(batch.flag, [1, 0]) np.testing.assert_array_equal(batch.group, [0, 1]) @@ -220,4 +220,4 @@ class _Flags(PayloadBase): assert p.group is _Color.Green # Wire byte: flag bit + (Green << 1) = 0x01 | 0x02 = 0x03. Masked fields on one # element share a slot named after the first declared field ("flag"). - assert int(p.raw_payload["flag"]) == 0x03 + assert int(p.payload_array["flag"]) == 0x03 diff --git a/tests/protocol/test_payload.py b/tests/protocol/test_payload.py index a7c8105..d4e0855 100644 --- a/tests/protocol/test_payload.py +++ b/tests/protocol/test_payload.py @@ -15,17 +15,17 @@ class SimplePayload(PayloadBase): class BitPackedPayload(PayloadBase): packed = Field(converter=_IdentityConverter("u1")) - def to_columns( + def payload_as_columns( self, *, decode_enums: bool = True, demux_bit_masks: bool = False ) -> list[Column]: return [ - Column("flag_a", (self.raw_payload["packed"] & 0x01).astype(bool)), - Column("flag_b", ((self.raw_payload["packed"] >> 1) & 0x01).astype(bool)), + Column("flag_a", (self.payload_array["packed"] & 0x01).astype(bool)), + Column("flag_b", ((self.payload_array["packed"] >> 1) & 0x01).astype(bool)), ] def _make_simple_bytes(n: int) -> bytes: - arr = np.zeros(n, dtype=SimplePayload.dtype) + arr = np.zeros(n, dtype=SimplePayload.payload_dtype) arr["x"] = np.arange(n, dtype=np.int16) * -1 arr["y"] = np.arange(n, dtype=np.uint8) return arr.tobytes() @@ -33,27 +33,29 @@ def _make_simple_bytes(n: int) -> bytes: def test_from_buffer_shape(): data = _make_simple_bytes(5) - p = SimplePayload.from_buffer(data) + p = SimplePayload.payload_from_buffer(data) assert len(p) == 5 def test_from_buffer_values(): data = _make_simple_bytes(3) - p = SimplePayload.from_buffer(data) + p = SimplePayload.payload_from_buffer(data) np.testing.assert_array_equal(p.x, [0, -1, -2]) np.testing.assert_array_equal(p.y, [0, 1, 2]) def test_to_dataframe_columns(): - p = SimplePayload.from_buffer(_make_simple_bytes(3)) + p = SimplePayload.payload_from_buffer(_make_simple_bytes(3)) df = payload_to_dataframe(p) assert list(df.columns) == ["x", "y"] assert len(df) == 3 def test_to_dataframe_override(): - arr = np.array([(0b00000011,), (0b00000001,), (0b00000010,)], dtype=BitPackedPayload.dtype) - p = BitPackedPayload.from_buffer(arr.tobytes()) + arr = np.array( + [(0b00000011,), (0b00000001,), (0b00000010,)], dtype=BitPackedPayload.payload_dtype + ) + p = BitPackedPayload.payload_from_buffer(arr.tobytes()) df = payload_to_dataframe(p) assert list(df.columns) == ["flag_a", "flag_b"] assert list(df["flag_a"]) == [True, True, False] @@ -62,15 +64,15 @@ def test_to_dataframe_override(): def test_from_buffer_zero_copy(): data = _make_simple_bytes(4) - p = SimplePayload.from_buffer(data) + p = SimplePayload.payload_from_buffer(data) # np.frombuffer returns a read-only view — writes should raise with pytest.raises((ValueError, TypeError)): - p.raw_payload["x"][0] = 999 + p.payload_array["x"][0] = 999 def test_payload_property(): - p = SimplePayload.from_buffer(_make_simple_bytes(2)) - assert p.raw_payload.dtype == SimplePayload.dtype + p = SimplePayload.payload_from_buffer(_make_simple_bytes(2)) + assert p.payload_array.dtype == SimplePayload.payload_dtype class _SparseMode(enum.IntEnum): @@ -87,14 +89,16 @@ class _SparseModePayload(AnonymousPayload[np.uint8]): def test_groupmask_undefined_code_preserves_raw(): # Codes: defined (0->Low, 2->High), an in-range gap (1), and out-of-range (90, 255). # Every undefined code is preserved as its raw int (like C#'s unchecked cast) — - batch = _SparseModePayload.from_buffer(np.array([0, 2, 1, 90, 255], dtype=np.uint8).tobytes()) + batch = _SparseModePayload.payload_from_buffer( + np.array([0, 2, 1, 90, 255], dtype=np.uint8).tobytes() + ) assert list(payload_to_dataframe(batch)["value"]) == ["Low", "High", 1, 90, 255] def test_groupmask_scalar_matches_batch_for_undefined(): # Scalar decode is permissive the same way - defined = _SparseModePayload.from_buffer(np.array([2], dtype=np.uint8).tobytes()) + defined = _SparseModePayload.payload_from_buffer(np.array([2], dtype=np.uint8).tobytes()) assert defined.__value__ is _SparseMode.High - undefined = _SparseModePayload.from_buffer(np.array([90], dtype=np.uint8).tobytes()) + undefined = _SparseModePayload.payload_from_buffer(np.array([90], dtype=np.uint8).tobytes()) assert undefined.__value__ == 90 assert not isinstance(undefined.__value__, _SparseMode) diff --git a/tests/protocol/test_register.py b/tests/protocol/test_register.py index b63c777..131a1fb 100644 --- a/tests/protocol/test_register.py +++ b/tests/protocol/test_register.py @@ -174,7 +174,7 @@ def test_format_with_payload_instance(reg_cls, payload_cls, value): frame = reg.format(payload) msg = _parse_frame(frame) assert msg.message_type == MessageType.Write - assert msg.payload == payload.raw_payload.tobytes() + assert msg.payload == payload.payload_array.tobytes() def test_format_with_payload_instance_via_register(): @@ -187,7 +187,7 @@ def test_format_with_payload_instance_via_register(): def test_structured_register_format_single_sample(): - sample = np.array([(100, 512, -200)], dtype=AnalogDataPayload.dtype) + sample = np.array([(100, 512, -200)], dtype=AnalogDataPayload.payload_dtype) frame = AnalogData.format(sample) msg = _parse_frame(frame) parsed = AnalogData.parse(msg) @@ -201,10 +201,10 @@ def test_structured_register_format_single_sample(): def test_structured_register_to_dataframe(): raw = np.array( [(1, 2, 3), (4, 5, 6)], - dtype=AnalogDataPayload.dtype, + dtype=AnalogDataPayload.payload_dtype, ).tobytes() - # Bulk decode goes through .Batch; from_buffer handles the redirect. - bulk = AnalogDataPayload.from_buffer(raw) + # Bulk decode goes through ._batch; from_buffer handles the redirect. + bulk = AnalogDataPayload.payload_from_buffer(raw) df = payload_to_dataframe(bulk) assert list(df.columns) == ["analog_input0", "encoder", "analog_input1"] assert len(df) == 2 @@ -328,13 +328,13 @@ def test_format_write_with_timestamp(): def test_anonymous_payload_roundtrip(payload_cls, raw_value, np_dtype): """Anonymous payload constructor + raw_payload roundtrips through bytes.""" payload = payload_cls(raw_value) - assert payload.raw_payload.dtype == np_dtype - assert payload.raw_payload.tobytes() == np.asarray(raw_value, dtype=np_dtype).tobytes() + assert payload.payload_array.dtype == np_dtype + assert payload.payload_array.tobytes() == np.asarray(raw_value, dtype=np_dtype).tobytes() def test_structured_payload_descriptors_single(): - buf = np.array([(100, 512, -200)], dtype=AnalogDataPayload.dtype).tobytes() - parsed = AnalogDataPayload.from_buffer(buf) + buf = np.array([(100, 512, -200)], dtype=AnalogDataPayload.payload_dtype).tobytes() + parsed = AnalogDataPayload.payload_from_buffer(buf) # 1-D batch (frombuffer always returns at least 1-D); descriptors return ndarrays. np.testing.assert_array_equal(parsed.analog_input0, [100]) np.testing.assert_array_equal(parsed.encoder, [512]) @@ -343,8 +343,8 @@ def test_structured_payload_descriptors_single(): def test_structured_payload_descriptors_multi(): records = [(100, 512, -200), (110, 513, -210), (120, 514, -220)] - buf = np.array(records, dtype=AnalogDataPayload.dtype).tobytes() - parsed = AnalogDataPayload.from_buffer(buf) + buf = np.array(records, dtype=AnalogDataPayload.payload_dtype).tobytes() + parsed = AnalogDataPayload.payload_from_buffer(buf) assert len(parsed) == 3 np.testing.assert_array_equal(parsed.analog_input0, [100, 110, 120]) np.testing.assert_array_equal(parsed.encoder, [512, 513, 514]) @@ -370,10 +370,10 @@ class DeviceName(RegisterBase): # dtype derives from the converter (one structured slot); raw bytes are the # encoded, null-padded value. - assert PayloadDeviceName.dtype.names == ("__value__",) - assert PayloadDeviceName.dtype.itemsize == 25 + assert PayloadDeviceName.payload_dtype.names == ("__value__",) + assert PayloadDeviceName.payload_dtype.itemsize == 25 payload = PayloadDeviceName("Behavior") - assert payload.raw_payload.tobytes() == b"Behavior".ljust(25, b"\x00") + assert payload.payload_array.tobytes() == b"Behavior".ljust(25, b"\x00") # Register round-trip decodes back to the high-level str, whether format() # is given a payload instance or the bare value (symmetric with parse()). @@ -384,10 +384,10 @@ class DeviceName(RegisterBase): # to_dataframe decodes both a single record and a batch. assert payload_to_dataframe(PayloadDeviceName("Behavior"))["value"].tolist() == ["Behavior"] two = ( - PayloadDeviceName("Foo").raw_payload.tobytes() - + PayloadDeviceName("Bar").raw_payload.tobytes() + PayloadDeviceName("Foo").payload_array.tobytes() + + PayloadDeviceName("Bar").payload_array.tobytes() ) - batch = PayloadDeviceName.from_buffer(two) + batch = PayloadDeviceName.payload_from_buffer(two) assert payload_to_dataframe(batch)["value"].tolist() == ["Foo", "Bar"] @@ -406,10 +406,10 @@ class Color(enum.IntEnum): class PayloadColor(AnonymousPayload[np.uint8]): __value__: Color = Field(EnumConverter(Color)) - assert PayloadColor.dtype.itemsize == 1 - raw = PayloadColor(Color.BLUE).raw_payload.tobytes() - record = np.frombuffer(raw, dtype=PayloadColor.dtype, count=1)[0] - assert PayloadColor.unwrap(record) == Color.BLUE + assert PayloadColor.payload_dtype.itemsize == 1 + raw = PayloadColor(Color.BLUE).payload_array.tobytes() + record = np.frombuffer(raw, dtype=PayloadColor.payload_dtype, count=1)[0] + assert PayloadColor._unwrap(record) == Color.BLUE def test_array_register_parse_returns_ndarray(): @@ -425,7 +425,7 @@ def test_array_register_parse_returns_ndarray(): # --------------------------------------------------------------------------- -# 10. parse vs read_frames / .Batch contract +# 10. parse vs read_frames / ._batch contract # --------------------------------------------------------------------------- @@ -456,8 +456,8 @@ def test_batch_payload_routes_to_batch_twin(): """ reg = RegisterU32Array(0x08, length=3) rows = np.array([[1, 2, 3], [4, 5, 6]], dtype=np.dtype(" 12-byte buffer (3 x u32), same converter class. - assert CustomPayloadPayload.dtype.itemsize == 12 + assert CustomPayloadPayload.payload_dtype.itemsize == 12 parsed = _roundtrip(CustomPayload, CustomPayloadPayload(HarpVersion(3, 1, 4))) assert isinstance(parsed, HarpVersion) assert parsed == HarpVersion(3, 1, 4) @@ -194,7 +194,7 @@ def test_custom_payload_single_member_unwrap(): def test_unknown_enum_code_preserves_raw(): # StartPulse.digital_output is a 2-bit field; code 0b11 has no PwmPort member. raw = np.array(0b11 << 10, dtype=np.uint16).tobytes() - payload = StartPulsePayload.from_buffer(raw) + payload = StartPulsePayload.payload_from_buffer(raw) value = payload.digital_output # permissive: the raw code is kept, not raised assert value == 0b11 assert not isinstance(value, PwmPort) @@ -213,7 +213,7 @@ def test_complex_configuration_to_dataframe(): events_enabled=True, delta=np.uint32(42), ) - batch = ComplexConfigurationPayload.from_buffer(cc.raw_payload.tobytes() * 2) + batch = ComplexConfigurationPayload.payload_from_buffer(cc.payload_array.tobytes() * 2) df = payload_to_dataframe(batch) assert len(df) == 2 assert list(df["pwm_port"]) == ["PWM2", "PWM2"] From e578a45e521bc1fdd83465222698053e8494729f Mon Sep 17 00:00:00 2001 From: glopesdev Date: Wed, 12 Aug 2026 00:32:04 +0100 Subject: [PATCH 7/8] Accept schema bytes so examples need no encoding parse_device_schema and create_device_module take str or bytes, so a schema can be read with read_bytes() and the YAML stream declares its own encoding. Reading it with read_text() and no explicit encoding follows the locale instead, which silently mangles a non-ASCII description under cp1252 and fails to parse at all under latin-1. The benchmark report and its README name payload_as_columns, matching the method they measure. --- README.md | 2 +- .../create_device_module.py | 2 +- docs/examples/read_dataset/read_dataset.py | 2 +- src/packages/harp-benchmarks/README.md | 8 ++++---- .../src/harp/benchmarks/benchmark.py | 18 ++++++++++-------- src/packages/harp-data/README.md | 2 +- src/packages/harp-device/README.md | 2 +- .../src/harp/device/_emit_module.py | 4 ++-- .../src/harp/device/_schema/_emit.py | 8 +++++--- tests/device/test_schema.py | 18 ++++++++++++++++++ 10 files changed, 44 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 9368bfd..5aadd64 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ does under the hood: from pathlib import Path from harp.device import create_device_module -behavior = create_device_module(Path("device.yml").read_text()) +behavior = create_device_module(Path("device.yml").read_bytes()) AnalogData = behavior.AnalogData # registers are reached by name... assert behavior.REGISTER_MAP[44] is AnalogData # ...or by address ``` diff --git a/docs/examples/create_device_module/create_device_module.py b/docs/examples/create_device_module/create_device_module.py index 4bca277..e51bc0a 100644 --- a/docs/examples/create_device_module/create_device_module.py +++ b/docs/examples/create_device_module/create_device_module.py @@ -11,7 +11,7 @@ # when you don't have a pre-generated package for it: point it at the schema and you # get the same shape a generated package has, registers at module level beside a # `REGISTER_MAP`. -behavior = create_device_module(Path("device.yml").read_text()) +behavior = create_device_module(Path("device.yml").read_bytes()) print("WhoAmI:", behavior.WHO_AM_I) # device identity, taken from the schema AnalogData = behavior.AnalogData # registers are reached by name... diff --git a/docs/examples/read_dataset/read_dataset.py b/docs/examples/read_dataset/read_dataset.py index b2186ae..4a350c4 100644 --- a/docs/examples/read_dataset/read_dataset.py +++ b/docs/examples/read_dataset/read_dataset.py @@ -42,5 +42,5 @@ # from harp.device import create_device_module # from pathlib import Path # -# behavior = create_device_module((Path("session.harp") / "device.yml").read_text()) +# behavior = create_device_module((Path("session.harp") / "device.yml").read_bytes()) # reader = DatasetReader(behavior, "session.harp") diff --git a/src/packages/harp-benchmarks/README.md b/src/packages/harp-benchmarks/README.md index 35a77c3..beaf6fb 100644 --- a/src/packages/harp-benchmarks/README.md +++ b/src/packages/harp-benchmarks/README.md @@ -16,7 +16,7 @@ device.yml coverage model — also imported by the acceptance tests under `tests | `src/harp/benchmarks/register_models.py` | Reference models for every device.yml register (fixtures shared with the acceptance tests). | | `src/harp/benchmarks/_registers.py` | Registry: each register + a representative sample value; artifact paths. | | `src/harp/benchmarks/generate.py` | Writes `./benchmark/data/_.bin`; exposes `ensure_corpus` (cache-aware). | -| `src/harp/benchmarks/benchmark.py` | Ensures corpora exist, then times `parse_bulk`, `parse_to_dataframe`, `to_columns`; writes `./benchmark/report.md`. | +| `src/harp/benchmarks/benchmark.py` | Ensures corpora exist, then times `parse_bulk`, `parse_to_dataframe`, `payload_as_columns`; writes `./benchmark/report.md`. | All generated artifacts (corpora + report) are written under **`./benchmark`** in the current working directory — git-ignored and fully regenerable. @@ -45,8 +45,8 @@ Equivalent module invocations: `uv run python -m harp.benchmarks.benchmark` / - **`parse_bulk`** — the core zero-copy strided-view parse into a `Batch` payload. This is **lazy**: it builds strided views only and runs **no** converters. - **`parse_to_dataframe`** — the full path to a pandas `DataFrame` (`copy=False`). -- **`to_columns`** (decode only) — `parse_bulk` views built once up front, then only - `payload.to_columns()` timed. This is where each field's `converter.decode_batch` +- **`payload_as_columns`** (decode only) — `parse_bulk` views built once up front, then only + `payload.payload_as_columns()` timed. This is where each field's `converter.decode_batch` actually runs, with no file read and no pandas construction. `parse_bulk` and `parse_to_dataframe` are each timed in two modes: @@ -54,4 +54,4 @@ Equivalent module invocations: `uv run python -m harp.benchmarks.benchmark` / - **pre-read** — file read once up front; only deserialization is timed (isolates library speed). - **re-read** — file re-read from disk on every run (real-world "load a dump" path, includes disk). -The report also decomposes `parse_to_dataframe ≈ parse_bulk + to_columns + pandas overhead`. +The report also decomposes `parse_to_dataframe ≈ parse_bulk + payload_as_columns + pandas overhead`. diff --git a/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py b/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py index e567f6c..da3c684 100644 --- a/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py +++ b/src/packages/harp-benchmarks/src/harp/benchmarks/benchmark.py @@ -99,7 +99,7 @@ def benchmark_register(reg: BenchmarkedRegister, path: Path, *, runs: int) -> Re frames=frames, file_bytes=file_bytes, ) - # Decode only: pre-parse the bulk views once, then time to_columns() alone — + # Decode only: pre-parse the bulk views once, then time payload_as_columns() alone — # this is where every converter's decode_batch runs, with no file read and no # pandas DataFrame construction. Matches parse_to_dataframe's decode options. _, _, _, payload = register.parse_bulk(raw, parse_timestamp=True) @@ -207,9 +207,9 @@ def _table( lambda r: (r.df_preread, r.df_reread), ) - # Decode-only table (single mode): to_columns() runs every field's + # Decode-only table (single mode): payload_as_columns() runs every field's # converter.decode_batch, with no file read and no pandas construction. - lines.append("## `to_columns` (decode only — where converters run)\n") + lines.append("## `payload_as_columns` (decode only — where converters run)\n") lines.append( "Isolates the decode step: `parse_bulk` views are built once up front, then " "only `payload.payload_as_columns()` is timed. This is where each field's " @@ -227,16 +227,18 @@ def _table( ) lines.append("") - # Decomposition: parse_to_dataframe(pre) ≈ parse_bulk(pre) + to_columns + pandas. + # Decomposition: parse_to_dataframe(pre) ≈ parse_bulk(pre) + payload_as_columns + pandas. lines.append("## Decomposition (pre-read means, ms)\n") lines.append( - "`parse_to_dataframe` ≈ `parse_bulk` (build views) + `to_columns` (decode) + " - "pandas DataFrame construction. The residual column is `df − bulk − to_columns`, " + "`parse_to_dataframe` ≈ `parse_bulk` (build views) + `payload_as_columns` (decode) + " + "pandas DataFrame construction. The residual column is `df − bulk − payload_as_columns`, " "i.e. the pandas/column-assembly overhead. Note the three terms are timed in " "separate loops, so for converter-dominated registers (large mean, large stdev) " "the residual is within noise and can even go slightly negative.\n" ) - lines.append("| Register | parse_bulk | to_columns | parse_to_dataframe | pandas residual |") + lines.append( + "| Register | parse_bulk | payload_as_columns | parse_to_dataframe | pandas residual |" + ) lines.append("| --- | ---: | ---: | ---: | ---: |") for r in results: residual = r.df_preread.mean - r.bulk_preread.mean - r.cols.mean @@ -329,7 +331,7 @@ def main() -> None: results.append(res) print( f"bulk={_fmt_ms(res.bulk_preread.mean):>8s}ms " - f"to_columns={_fmt_ms(res.cols.mean):>9s}ms " + f"payload_as_columns={_fmt_ms(res.cols.mean):>9s}ms " f"df={_fmt_ms(res.df_preread.mean):>9s}ms" ) if args.head: diff --git a/src/packages/harp-data/README.md b/src/packages/harp-data/README.md index ebc9aba..ddba84a 100644 --- a/src/packages/harp-data/README.md +++ b/src/packages/harp-data/README.md @@ -45,7 +45,7 @@ from pathlib import Path from harp.data import DatasetReader from harp.device import create_device_module -behavior = create_device_module((Path("session.harp") / "device.yml").read_text()) +behavior = create_device_module((Path("session.harp") / "device.yml").read_bytes()) reader = DatasetReader(behavior, "session.harp") ``` diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index 14cfa0e..0625631 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -74,7 +74,7 @@ names come from the yml verbatim, payload fields are `snake_case`, and enum memb from pathlib import Path from harp.device import create_device_module -behavior = create_device_module(Path("device.yml").read_text()) +behavior = create_device_module(Path("device.yml").read_bytes()) reg = behavior.AnalogData # by name reg = behavior.REGISTER_MAP[44] # or by address ``` diff --git a/src/packages/harp-device/src/harp/device/_emit_module.py b/src/packages/harp-device/src/harp/device/_emit_module.py index 19e23db..d468373 100644 --- a/src/packages/harp-device/src/harp/device/_emit_module.py +++ b/src/packages/harp-device/src/harp/device/_emit_module.py @@ -55,7 +55,7 @@ def __getattr__(self, name: str) -> type[RegisterBase[Any]]: def create_device_module( - text: str, + text: str | bytes, *, name: Optional[str] = None, converters: Optional[Mapping[str, ConverterValue]] = None, @@ -85,7 +85,7 @@ def create_device_module( registered in :data:`sys.modules`, so it cannot be reached by ``import`` and two schemas may share a name without clashing. Bind it yourself:: - behavior = create_device_module(Path("device.yml").read_text()) + behavior = create_device_module(Path("device.yml").read_bytes()) behavior.AnalogData """ device = parse_device_schema(text) 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 d139f4c..8377cbc 100644 --- a/src/packages/harp-device/src/harp/device/_schema/_emit.py +++ b/src/packages/harp-device/src/harp/device/_schema/_emit.py @@ -470,13 +470,15 @@ def emit(self) -> dict[str, type[RegisterBase[Any]]]: return emitted -def parse_device_schema(text: str) -> DeviceModel: +def parse_device_schema(text: str | bytes) -> DeviceModel: """Parse a Harp ``device.yml`` (or a header-less fragment) into a :class:`DeviceModel`. A header-less fragment (just ``registers`` / ``bitMasks`` / ``groupMasks``) parses fine — the identity fields (``device`` / ``whoAmI`` / ...) are simply ``None``. Read files yourself, e.g. - ``parse_device_schema(Path("device.yml").read_text())``. + ``parse_device_schema(Path("device.yml").read_bytes())``. Prefer reading bytes: + a YAML stream declares its own encoding, so the parser decodes it, whereas + ``read_text()`` without an explicit encoding uses the locale default. Uses ``pydantic-yaml`` (ruamel-backed, YAML 1.2), so group-mask keys like ``Off`` / ``On`` stay strings instead of being coerced to booleans. @@ -485,7 +487,7 @@ def parse_device_schema(text: str) -> DeviceModel: def create_registers( - source: Union[str, DeviceModel, Registers], + source: str | bytes | DeviceModel | Registers, *, converters: Optional[Mapping[str, ConverterValue]] = None, strict: bool = True, diff --git a/tests/device/test_schema.py b/tests/device/test_schema.py index 7d01adc..200e1e9 100644 --- a/tests/device/test_schema.py +++ b/tests/device/test_schema.py @@ -20,6 +20,24 @@ def test_parse_fragment_yields_null_device(): assert m.registers["Foo"].type is PayloadType.U16 +def test_parse_bytes_decodes_as_utf8_regardless_of_locale(): + # A YAML stream declares its own encoding, so reading a schema as bytes decodes it + # correctly where read_text() without an explicit encoding follows the locale. + schema = ( + "registers:\n" + " Poke:\n" + " address: 40\n" + " type: U8\n" + " access: Read\n" + " description: µV threshold\n" + ) + m = parse_device_schema(schema.encode("utf-8")) + assert m.registers["Poke"].description == "µV threshold" + assert ( + m.registers["Poke"].description == parse_device_schema(schema).registers["Poke"].description + ) + + def test_parse_common_registers(common_yml): c = parse_device_schema(common_yml) assert c.device is None From 727d4e0d55ad57a032aed3ab8e913792c97939b6 Mon Sep 17 00:00:00 2001 From: glopesdev Date: Wed, 12 Aug 2026 01:09:21 +0100 Subject: [PATCH 8/8] Name the generated batch class after its type The auto-generated batch sibling is _PayloadBatchType rather than _batch, so the attribute reads as the class it holds. It stays private, since the Batch protocol is what callers annotate against. --- src/packages/harp-protocol/src/harp/protocol/_payload.py | 6 +++--- tests/protocol/test_converter.py | 2 +- tests/protocol/test_register.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/packages/harp-protocol/src/harp/protocol/_payload.py b/src/packages/harp-protocol/src/harp/protocol/_payload.py index cac281c..12bebcc 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_payload.py +++ b/src/packages/harp-protocol/src/harp/protocol/_payload.py @@ -498,7 +498,7 @@ class Batch(Protocol[_PT]): Statically, ``Batch[P]`` is a distinct type from ``P`` so the type checker knows ``read_frames`` returns an ndarray-shaped view rather than a single record. At runtime, the value is the auto-derived - ``P._batch`` sibling whose descriptors return ``NDArray`` views. + ``P._PayloadBatchType`` sibling whose descriptors return ``NDArray`` views. Per-field dtype precision is intentionally dropped — every declared field reports ``NDArray[Any]`` — to keep ``RegisterBase[P]`` @@ -669,7 +669,7 @@ class PayloadBase(Generic[NpStructT]): # Cached map of attribute name → default value for fields that declare one. _defaults: ClassVar[dict[str, Any]] # Auto-generated sibling class whose descriptors return NDArray views instead of scalars. - _batch: ClassVar["type[PayloadBase]"] + _PayloadBatchType: ClassVar["type[PayloadBase]"] # Base element dtype (from the ``StructPayload[...]`` type arg); governs offset # arithmetic and the integer width used for masked reads. Defaults to uint8. _elem_dtype: ClassVar[np.dtype] = _DEFAULT_ELEMENT @@ -811,7 +811,7 @@ def __init_subclass__( for name, val in cls.__dict__.items(): if isinstance(val, _SCALAR_DECLARATION_TYPES): batch_attrs[name] = val._to_batch() - cls._batch = type( + cls._PayloadBatchType = type( f"{cls.__name__}Batch", (cls,), batch_attrs, diff --git a/tests/protocol/test_converter.py b/tests/protocol/test_converter.py index 861c1d2..9809c5d 100644 --- a/tests/protocol/test_converter.py +++ b/tests/protocol/test_converter.py @@ -202,7 +202,7 @@ class _Flags(PayloadBase): # 1-D batch — Batch sibling, ndarray-typed accessors. batch = _Flags.payload_from_buffer(bytes([0x01, 0x02])) - assert type(batch) is _Flags._batch + assert type(batch) is _Flags._PayloadBatchType assert isinstance(batch, _Flags) np.testing.assert_array_equal(batch.flag, [1, 0]) np.testing.assert_array_equal(batch.group, [0, 1]) diff --git a/tests/protocol/test_register.py b/tests/protocol/test_register.py index 131a1fb..4309193 100644 --- a/tests/protocol/test_register.py +++ b/tests/protocol/test_register.py @@ -203,7 +203,7 @@ def test_structured_register_to_dataframe(): [(1, 2, 3), (4, 5, 6)], dtype=AnalogDataPayload.payload_dtype, ).tobytes() - # Bulk decode goes through ._batch; from_buffer handles the redirect. + # Bulk decode goes through ._PayloadBatchType; from_buffer handles the redirect. bulk = AnalogDataPayload.payload_from_buffer(raw) df = payload_to_dataframe(bulk) assert list(df.columns) == ["analog_input0", "encoder", "analog_input1"] @@ -425,7 +425,7 @@ def test_array_register_parse_returns_ndarray(): # --------------------------------------------------------------------------- -# 10. parse vs read_frames / ._batch contract +# 10. parse vs read_frames / ._PayloadBatchType contract # --------------------------------------------------------------------------- @@ -457,7 +457,7 @@ def test_batch_payload_routes_to_batch_twin(): reg = RegisterU32Array(0x08, length=3) rows = np.array([[1, 2, 3], [4, 5, 6]], dtype=np.dtype("