From 5f2c2442200777d392b4c39365a344010e52a57a Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 18:42:14 -0700 Subject: [PATCH 01/22] Add runtime generation of device interface --- .../harp-data/src/harp/data/__init__.py | 3 + .../harp-data/src/harp/data/_reader.py | 13 +- .../harp-data/src/harp/data/_write.py | 45 ++ src/packages/harp-device/README.md | 25 + src/packages/harp-device/pyproject.toml | 2 + .../harp-device/src/harp/device/__init__.py | 5 + .../src/harp/device/_emit_device.py | 37 ++ .../src/harp/device/_schema/__init__.py | 45 ++ .../src/harp/device/_schema/_emit.py | 431 ++++++++++++++++++ .../src/harp/device/_schema/_model.py | 145 ++++++ .../src/harp/protocol/_register.py | 90 +++- tests/assets/common.yml | 145 ++++++ tests/assets/device.yml | 191 ++++++++ tests/conftest.py | 21 +- tests/device/__init__.py | 0 tests/device/data/__init__.py | 0 tests/device/data/converters.py | 36 ++ tests/device/data/expected_core.py | 229 ++++++++++ tests/device/data/expected_device.py | 252 ++++++++++ tests/device/test_device_emit.py | 66 +++ tests/device/test_emit.py | 182 ++++++++ tests/device/test_schema.py | 41 ++ tests/protocol/test_register.py | 43 +- uv.lock | 169 ++++++- 24 files changed, 2206 insertions(+), 10 deletions(-) create mode 100644 src/packages/harp-data/src/harp/data/_write.py create mode 100644 src/packages/harp-device/src/harp/device/_emit_device.py create mode 100644 src/packages/harp-device/src/harp/device/_schema/__init__.py create mode 100644 src/packages/harp-device/src/harp/device/_schema/_emit.py create mode 100644 src/packages/harp-device/src/harp/device/_schema/_model.py create mode 100644 tests/assets/common.yml create mode 100644 tests/assets/device.yml create mode 100644 tests/device/__init__.py create mode 100644 tests/device/data/__init__.py create mode 100644 tests/device/data/converters.py create mode 100644 tests/device/data/expected_core.py create mode 100644 tests/device/data/expected_device.py create mode 100644 tests/device/test_device_emit.py create mode 100644 tests/device/test_emit.py create mode 100644 tests/device/test_schema.py diff --git a/src/packages/harp-data/src/harp/data/__init__.py b/src/packages/harp-data/src/harp/data/__init__.py index c8208f1..5aff91a 100644 --- a/src/packages/harp-data/src/harp/data/__init__.py +++ b/src/packages/harp-data/src/harp/data/__init__.py @@ -1,6 +1,9 @@ from ._reader import parse_to_dataframe, payload_to_dataframe +from ._write import to_buffer, to_file __all__ = [ "parse_to_dataframe", "payload_to_dataframe", + "to_buffer", + "to_file", ] diff --git a/src/packages/harp-data/src/harp/data/_reader.py b/src/packages/harp-data/src/harp/data/_reader.py index b068df4..8ff9ae3 100644 --- a/src/packages/harp-data/src/harp/data/_reader.py +++ b/src/packages/harp-data/src/harp/data/_reader.py @@ -77,9 +77,12 @@ def parse_to_dataframe( ) if timestamp: if timestamps is None: - raise ValueError( - "Buffer contains no timestamp data; pass timestamp=False to suppress " - "the timestamp column." - ) - df.insert(0, "timestamp", timestamps) + if len(df) > 0: + raise ValueError( + "Buffer contains no timestamp data; pass timestamp=False to suppress " + "the timestamp column." + ) + # Empty buffer: no frames to timestamp — return the empty frame as-is. + else: + df.insert(0, "timestamp", timestamps) return df diff --git a/src/packages/harp-data/src/harp/data/_write.py b/src/packages/harp-data/src/harp/data/_write.py new file mode 100644 index 0000000..b2cd2e6 --- /dev/null +++ b/src/packages/harp-data/src/harp/data/_write.py @@ -0,0 +1,45 @@ +"""Write Harp register data to a binary buffer/file — the inverse of the readers. + +Thin wrappers over :meth:`RegisterBase.format_bulk` giving a pandas-package home +and a file sink. Useful for round-tripping data and generating typed test corpora. +""" + +from os import PathLike +from typing import Any, Union + +import numpy as np +from harp.protocol import MessageType, RegisterBase +from numpy.typing import NDArray + + +def to_buffer( + register: type[RegisterBase[Any]], + values: Any, + *, + timestamps: Any = None, + message_type: Any = MessageType.Event, + port: int = 255, +) -> NDArray[np.uint8]: + """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 + timestamped; ``message_type`` is one :class:`MessageType` or a length-N array + (e.g. the msgtype view from ``parse_bulk``). + """ + return register.format_bulk(values, timestamps=timestamps, message_type=message_type, port=port) + + +def to_file( + register: type[RegisterBase[Any]], + values: Any, + file: Union[str, PathLike], + *, + timestamps: Any = None, + message_type: Any = MessageType.Event, + port: int = 255, +) -> None: + """Write ``values`` as ``register`` frames to ``file`` (see :func:`to_buffer`).""" + to_buffer(register, values, timestamps=timestamps, message_type=message_type, port=port).tofile( + file + ) diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index f4b0f01..3221dd8 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -33,3 +33,28 @@ REGISTER_MAP = {**_CORE_REGISTER_MAP, 32: DigitalInputState, ...} A new transport is just an object implementing the `ITransport` protocol (`open`/`write`/`read`/`close`). + +## Generating a device from a `device.yml` + +If you don't have a pre-generated device package, `create_device` builds a +`Device` from Harp `device.yml` text. Registers are reached by address through +`REGISTER_MAP`; field and enum names come from the yml verbatim. + +```python +from pathlib import Path +from harp.device import create_device + +Behavior = create_device(Path("device.yml").read_text()) +reg = Behavior.REGISTER_MAP[44] +``` + +For a custom `interfaceType`, pass its converter via `converters=` (keyed by +`{InterfaceType}Converter` / `{MemberName}Converter`); an unresolved custom type +raises `UnknownConverterError`, or pass `strict=False` to decode it natively: + +```python +create_device(yml_text, converters={"DataConverter": DataConverter()}) +``` + +`parse_device_schema(yml_text)` is also public if you just want the parsed +schema model (registers, masks, and optional device identity) without a `Device`. diff --git a/src/packages/harp-device/pyproject.toml b/src/packages/harp-device/pyproject.toml index 54e8578..8efea01 100644 --- a/src/packages/harp-device/pyproject.toml +++ b/src/packages/harp-device/pyproject.toml @@ -5,6 +5,8 @@ description = "Transport-agnostic Harp device protocol layer" requires-python = ">=3.11" dependencies = [ "harp-protocol", + "pydantic>=2", + "pydantic-yaml>=1", ] [build-system] diff --git a/src/packages/harp-device/src/harp/device/__init__.py b/src/packages/harp-device/src/harp/device/__init__.py index ac81527..d352a19 100644 --- a/src/packages/harp-device/src/harp/device/__init__.py +++ b/src/packages/harp-device/src/harp/device/__init__.py @@ -1,4 +1,5 @@ from ._device import Device, EventHandler, Subscription +from ._emit_device import create_device from ._framer import HarpFramer from ._registers import ( AssemblyVersion, @@ -26,12 +27,16 @@ WhoAmI, ) from ._register_map import REGISTER_MAP +from ._schema import ConverterContext, parse_device_schema from ._transport import ITransport, TransportError __all__ = [ "Device", "EventHandler", "Subscription", + "create_device", + "parse_device_schema", + "ConverterContext", "HarpFramer", "ITransport", "TransportError", diff --git a/src/packages/harp-device/src/harp/device/_emit_device.py b/src/packages/harp-device/src/harp/device/_emit_device.py new file mode 100644 index 0000000..7425d96 --- /dev/null +++ b/src/packages/harp-device/src/harp/device/_emit_device.py @@ -0,0 +1,37 @@ +from typing import Any, Mapping, Optional, Union + +from ._device import Device +from ._register_map import REGISTER_MAP as CORE_REGISTER_MAP +from ._schema import create_registers, parse_device_schema +from ._schema._emit import ConverterValue +from ._schema._model import DeviceModel + + +def create_device( + source: Union[str, DeviceModel], + *, + name: Optional[str] = None, + converters: Optional[Mapping[str, ConverterValue]] = None, + strict: bool = True, + exclude_private: bool = True, +) -> type[Device]: + """Emit a :class:`Device` subclass from a device schema. + + The returned class exposes its registers through ``REGISTER_MAP`` (address -> + register class) and carries ``__whoami__`` from the schema (``0x0`` when + absent). The device's registers are spread on top of the core common map; on + an address clash the device's register wins. ``exclude_private=True`` drops + registers whose DSL ``visibility`` is ``private``. A header-less register + fragment yields a device with no ``device`` name (falls back to ``"Device"``). + """ + device = source if isinstance(source, DeviceModel) else parse_device_schema(source) + registers = create_registers( + device, converters=converters, strict=strict, exclude_private=exclude_private + ) + by_address = {cls.address: cls for cls in registers.values()} + + namespace: dict[str, Any] = { + "__whoami__": int(device.whoAmI or 0), + "REGISTER_MAP": {**CORE_REGISTER_MAP, **by_address}, + } + return type(name or device.device or "Device", (Device,), namespace) diff --git a/src/packages/harp-device/src/harp/device/_schema/__init__.py b/src/packages/harp-device/src/harp/device/_schema/__init__.py new file mode 100644 index 0000000..cc41ac3 --- /dev/null +++ b/src/packages/harp-device/src/harp/device/_schema/__init__.py @@ -0,0 +1,45 @@ +from ._model import ( + Access, + BitMask, + Converter, + GroupMask, + InterfaceType, + MaskType, + MaskValue, + DeviceModel, + PayloadMember, + PayloadType, + Register, + Registers, + Visibility, +) +from ._emit import ( + ConverterContext, + ConverterFactory, + ConverterValue, + UnknownConverterError, + create_registers, + parse_device_schema, +) + +__all__ = [ + "parse_device_schema", + "create_registers", + "ConverterContext", + "ConverterFactory", + "ConverterValue", + "UnknownConverterError", + "DeviceModel", + "Registers", + "Register", + "PayloadMember", + "PayloadType", + "Access", + "Visibility", + "Converter", + "BitMask", + "GroupMask", + "MaskType", + "MaskValue", + "InterfaceType", +] diff --git a/src/packages/harp-device/src/harp/device/_schema/_emit.py b/src/packages/harp-device/src/harp/device/_schema/_emit.py new file mode 100644 index 0000000..966149f --- /dev/null +++ b/src/packages/harp-device/src/harp/device/_schema/_emit.py @@ -0,0 +1,431 @@ +import enum +import types +from dataclasses import dataclass +from typing import Any, Callable, Mapping, Optional, Union + +import numpy as np +from typing_extensions import Sentinel +from pydantic_yaml import parse_yaml_raw_as +from harp.protocol import ( + AnonymousPayload, + BitMask, + BoolConverter, + Converter, + Field, + GroupMask, + HarpVersionConverter, + IdentityConverter, + RegisterBase, + RegisterFloat, + RegisterFloatArray, + RegisterS8, + RegisterS8Array, + RegisterS16, + RegisterS16Array, + RegisterS32, + RegisterS32Array, + RegisterS64, + RegisterS64Array, + RegisterU8, + RegisterU8Array, + RegisterU16, + RegisterU16Array, + RegisterU32, + RegisterU32Array, + RegisterU64, + RegisterU64Array, + StringConverter, + StructPayload, +) +from harp.protocol import PayloadType as ProtoPayloadType + +from ._model import DeviceModel, PayloadMember, PayloadType, Register, Registers, Visibility + +# Register base element: schema PayloadType -> numpy scalar type (byte size via np.dtype). +_ELEMENT: dict[PayloadType, type[np.generic]] = { + PayloadType.U8: np.uint8, + PayloadType.S8: np.int8, + PayloadType.U16: np.uint16, + PayloadType.S16: np.int16, + PayloadType.U32: np.uint32, + PayloadType.S32: np.int32, + PayloadType.U64: np.uint64, + PayloadType.S64: np.int64, + PayloadType.Float: np.float32, +} +_SCALAR_REGISTER: dict[PayloadType, Any] = { + PayloadType.U8: RegisterU8, + PayloadType.S8: RegisterS8, + PayloadType.U16: RegisterU16, + PayloadType.S16: RegisterS16, + PayloadType.U32: RegisterU32, + PayloadType.S32: RegisterS32, + PayloadType.U64: RegisterU64, + PayloadType.S64: RegisterS64, + PayloadType.Float: RegisterFloat, +} +_ARRAY_REGISTER: dict[PayloadType, Any] = { + PayloadType.U8: RegisterU8Array, + PayloadType.S8: RegisterS8Array, + PayloadType.U16: RegisterU16Array, + PayloadType.S16: RegisterS16Array, + PayloadType.U32: RegisterU32Array, + PayloadType.S32: RegisterS32Array, + PayloadType.U64: RegisterU64Array, + PayloadType.S64: RegisterS64Array, + PayloadType.Float: RegisterFloatArray, +} + + +@dataclass(frozen=True) +class ConverterContext: + """A payload value's schema definition, resolved against its register context. + + Handed to every converter factory so it can construct the converter with the + right arguments — e.g. ``StringConverter(span)``, ``HarpVersionConverter(element)``, + or ``IdentityConverter(dtype)``. + """ + + name: str # yml field key ("__value__" for a whole-register value) + interface_type: Optional[str] # the DSL interfaceType (None = raw/native) + mask: Optional[int] # bit mask, when the value is bit-packed + length: int # element count this value spans (0 = unset -> scalar) + element: np.dtype # the register's base element dtype (from PayloadType) + element_size: int # the register's base element byte size + + @property + def span(self) -> int: + """Byte span of the value (element count * element size).""" + return max(1, self.length) * self.element_size + + @property + def member_dtype(self) -> np.dtype: + """The value's own numpy dtype — a native primitive interfaceType overrides the element.""" + if self.interface_type is not None: + entry = _INTERFACES.get(self.interface_type) + if entry is not None and entry.native_dtype is not None: + return np.dtype(entry.native_dtype) + return self.element + + @property + def raw_dtype(self) -> np.dtype: + """Native passthrough dtype — a sub-array when the value spans >1 element.""" + if self.length > 1: + return np.dtype((self.element.type, (self.length,))) + return self.element + + +# A converter factory builds a converter from a field's DSL context. +ConverterFactory = Callable[[ConverterContext], Converter[Any]] +# A user-supplied converter: a ready instance, or a factory that builds one from context. +ConverterValue = Union[Converter[Any], ConverterFactory] +# Internal built-in factory — may decline (return None) when the DSL type doesn't +# actually fit (e.g. a primitive whose declared byte span isn't its native size). +_InterfaceFactory = Callable[[ConverterContext], Optional[Converter[Any]]] +# Coerces a field's yml numeric default into its typed value (``_NO_DEFAULT`` = skip). +_DefaultCoercer = Callable[[float, ConverterContext], Any] + +_NO_DEFAULT = Sentinel("_NO_DEFAULT") # this interface has no numeric default representation + + +def _numpy_default(value: float, ctx: ConverterContext) -> Any: + literal = int(value) if value == np.floor(value) else value + return np.dtype(ctx.member_dtype).type(literal) + + +def _bool_default(value: float, ctx: ConverterContext) -> Any: + return bool(value != 0) + + +def _skip_default(value: float, ctx: ConverterContext) -> Any: + return _NO_DEFAULT + + +def _native(dtype: type[np.generic]) -> _InterfaceFactory: + # A native primitive decodes as an identity passthrough. Unmasked, it only fits + # when its declared byte span matches its width; otherwise it declines (returns + # None) and the field re-interprets the bytes via a custom ``{Field}Converter``. + # Masked, it is always a native slice of the element (member_dtype == this width). + d = np.dtype(dtype) + return lambda ctx: ( + IdentityConverter(d) if ctx.mask is not None or ctx.span == d.itemsize else None + ) + + +@dataclass(frozen=True) +class _Interface: + """A built-in interfaceType: how to build its converter, how to coerce its + default value, and its native numpy dtype (fixed-width primitives only).""" + + build: _InterfaceFactory + default: _DefaultCoercer + native_dtype: Optional[type[np.generic]] = None + + +# Every interfaceType the library handles natively, in one uniform table: the +# fixed-width primitives (identity passthrough, carrying their numpy scalar as +# ``native_dtype``) beside string/bool/HarpVersion. Custom interfaceTypes are +# supplied by the caller (see ``converters=``). +_INTERFACES: dict[str, _Interface] = { + "byte": _Interface(_native(np.uint8), _numpy_default, np.uint8), + "sbyte": _Interface(_native(np.int8), _numpy_default, np.int8), + "short": _Interface(_native(np.int16), _numpy_default, np.int16), + "ushort": _Interface(_native(np.uint16), _numpy_default, np.uint16), + "int": _Interface(_native(np.int32), _numpy_default, np.int32), + "uint": _Interface(_native(np.uint32), _numpy_default, np.uint32), + "long": _Interface(_native(np.int64), _numpy_default, np.int64), + "ulong": _Interface(_native(np.uint64), _numpy_default, np.uint64), + "float": _Interface(_native(np.float32), _numpy_default, np.float32), + "string": _Interface(lambda ctx: StringConverter(ctx.span), _skip_default), + "bool": _Interface(lambda ctx: BoolConverter(), _bool_default), + "HarpVersion": _Interface(lambda ctx: HarpVersionConverter(ctx.element), _skip_default), +} + + +def _materialize(value: ConverterValue, ctx: ConverterContext) -> Converter[Any]: + """A user converter value is either a ready instance or a ``(ctx) -> Converter`` factory.""" + return value if isinstance(value, Converter) else value(ctx) + + +class UnknownConverterError(ValueError): + """A custom ``interfaceType`` needs a converter not found in ``converters=``.""" + + +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. + """ + if interface_type is None: + return True + entry = _INTERFACES.get(interface_type) + return entry is not None and entry.native_dtype is not None + + +def _new_class(name: str, bases: tuple, namespace: dict, kwds: Optional[dict] = None) -> type: + return types.new_class(name, bases, kwds or {}, lambda ns: ns.update(namespace)) + + +class _Emitter: + def __init__( + self, + device: Union[DeviceModel, Registers], + converters: Optional[Mapping[str, ConverterValue]], + strict: bool, + exclude_private: bool, + ) -> None: + self.device = device + self.converters = dict(converters or {}) + self.strict = strict + self.exclude_private = exclude_private + self.group_masks = device.groupMasks or {} + self.bit_masks = device.bitMasks or {} + self.enums = self._build_enums() + + # -- enums ------------------------------------------------------------ + def _build_enums(self) -> dict[str, Any]: + # Enum names and members are kept verbatim from the yml. + 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) + 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) + return enums + + # -- converter resolution (one uniform factory pipeline) ------------- + def _resolve_converter(self, ctx: ConverterContext) -> Converter[Any]: + """Build the converter for a payload value from its schema context.""" + it = ctx.interface_type + entry = _INTERFACES.get(it) if it is not None else None + if entry is not None: + converter = entry.build(ctx) + if converter is not None: + return converter + if ctx.mask is not None: + return IdentityConverter(ctx.member_dtype) # bit-field: native slice of the element + if it is None: + return IdentityConverter(ctx.raw_dtype) # raw passthrough / sub-array + # A known primitive that didn't fit is re-interpreted per field (``{Name}Converter``); + # an unknown interfaceType is a domain type (``{InterfaceType}Converter``). + symbol = f"{ctx.name}Converter" if entry is not None else f"{it}Converter" + return self._extension(symbol, ctx) + + def _extension(self, symbol: str, ctx: ConverterContext) -> Converter[Any]: + value = self.converters.get(symbol) + if value is not None: + return _materialize(value, ctx) + if not self.strict: + return IdentityConverter(ctx.element) + raise UnknownConverterError( + f"no converter {symbol!r} in converters=; pass " + f"converters={{{symbol!r}: Converter>}} " + f"or strict=False to decode as the native type" + ) + + # -- defaults --------------------------------------------------------- + def _default(self, member: PayloadMember, type_name: str, ctx: ConverterContext) -> Any: + """The field's typed default value, or ``_NO_DEFAULT`` when it has none.""" + _default_value = member.defaultValue if member.defaultValue is not None else member.minValue + if _default_value is None or (member.length or 0) > 1: + return _NO_DEFAULT + value = float(_default_value.root) + if type_name in self.group_masks: + e = self.enums[type_name] + for mv in self.group_masks[type_name].values.values(): + if int(mv) == int(value): + return e(int(value)) + return int(value) + if member.converter is not None: + return _NO_DEFAULT # a custom converter owns its own decoding; no numeric default + it = ctx.interface_type + entry = _INTERFACES.get(it) if it is not None else None + if entry is not None: + return entry.default(value, ctx) + if it is None: + return _numpy_default(value, ctx) # raw native passthrough + return _NO_DEFAULT # custom domain interfaceType: no numeric default + + # -- fields ----------------------------------------------------------- + def _build_field(self, key: str, member: PayloadMember, reg: Register) -> tuple[str, Any]: + elem_np = _ELEMENT[reg.type] + elem_size = np.dtype(elem_np).itemsize + offset = member.offset or 0 + it = member.interfaceType.root if member.interfaceType else None + type_name = it or (member.maskType.root if member.maskType else "") + ctx = ConverterContext( + name=key, + interface_type=it, + mask=member.mask, + length=member.length or 0, + element=np.dtype(elem_np), + element_size=elem_size, + ) + default = self._default(member, type_name, ctx) + default_kwarg = {} if default is _NO_DEFAULT else {"default": default} + + # A group mask is an enum sub-field descriptor, not a Field(converter). + if type_name in self.group_masks: + 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 + ) + + 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) + + # -- payloads --------------------------------------------------------- + def _build_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 + kwds = {"length": length} if length > 1 else {} + return _new_class(f"{name}Payload", (StructPayload[elem_np],), namespace, kwds) + + # anonymous single-value payload + mt = reg.maskType.root if reg.maskType else None + it = reg.interfaceType.root if reg.interfaceType else None + if mt in self.group_masks: + full = (1 << (elem_size * 8)) - 1 + descriptor: Any = GroupMask(enum=self.enums[mt], mask=full) + elif mt in self.bit_masks: + descriptor = BitMask(enum=self.enums[mt]) + else: + assert it is not None, ( + f"{name}: register needs a payloadSpec, maskType, or interfaceType" + ) + ctx = ConverterContext( + name="__value__", + interface_type=it, + mask=None, + length=length, + element=np.dtype(elem_np), + element_size=elem_size, + ) + descriptor = Field(self._resolve_converter(ctx)) + return _new_class(f"{name}Payload", (AnonymousPayload[elem_np],), {"__value__": descriptor}) + + # -- registers -------------------------------------------------------- + def _build_register(self, name: str, reg: Register) -> type[RegisterBase[Any]]: + length = reg.length or 1 + it = reg.interfaceType.root if reg.interfaceType else None + + # A plain scalar/array register needs no payload wrapper: its whole value is a + # native passthrough (no payloadSpec, no maskType, no custom converter). + if ( + reg.payloadSpec is None + and reg.maskType is None + and reg.converter is None + and _is_native(it) + ): + if length > 1: # plain array register + cls = _ARRAY_REGISTER[reg.type](reg.address, length=length) + cls.__name__ = cls.__qualname__ = name + return cls + return _new_class(name, (_SCALAR_REGISTER[reg.type],), {"address": reg.address}) + + payload_cls = self._build_payload(name, reg) + return _new_class( + name, + (RegisterBase,), + { + "address": reg.address, + "payload_type": ProtoPayloadType[reg.type.name], + "payload_class": payload_cls, + }, + ) + + 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) + } + + +def parse_device_schema(text: str) -> 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())``. + + Uses ``pydantic-yaml`` (ruamel-backed, YAML 1.2), so group-mask keys like + ``Off`` / ``On`` stay strings instead of being coerced to booleans. + """ + return parse_yaml_raw_as(DeviceModel, text) + + +def create_registers( + source: Union[str, DeviceModel, Registers], + *, + converters: Optional[Mapping[str, ConverterValue]] = None, + strict: bool = True, + exclude_private: bool = False, +) -> dict[str, type[RegisterBase[Any]]]: + """Emit runtime register classes from a device schema. + + ``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 + 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``. + """ + 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/_model.py b/src/packages/harp-device/src/harp/device/_schema/_model.py new file mode 100644 index 0000000..0d11f28 --- /dev/null +++ b/src/packages/harp-device/src/harp/device/_schema/_model.py @@ -0,0 +1,145 @@ +"""Pydantic object model of a Harp ``device.yml`` / ``registers`` schema. + +TODO: hand-maintained for now. Auto-generating it from the upstream +``harp-tech/protocol`` JSON schema is deferred until that schema stabilises. +""" + +from enum import Enum +from typing import Annotated, Dict, List, Optional, Union + +from pydantic import BaseModel, ConfigDict, Field, RootModel + + +class PayloadType(str, Enum): + """Register payload element type. Values match the schema enum (the yml names).""" + + U8 = "U8" + S8 = "S8" + U16 = "U16" + S16 = "S16" + U32 = "U32" + S32 = "S32" + U64 = "U64" + S64 = "S64" + Float = "Float" + + +class Access(Enum): + Read = "Read" + Write = "Write" + Event = "Event" + + +class Visibility(Enum): + public = "public" + private = "private" + + +class Converter(Enum): + None_ = "None" + Payload = "Payload" + RawPayload = "RawPayload" + + +class MaskValueItem(BaseModel): + model_config = ConfigDict(extra="forbid") + value: int = Field(..., description="Specifies the numerical mask value.") + description: Optional[str] = Field(None, description="Summary of the mask value function.") + + def __int__(self) -> int: + return self.value + + +class MaskValue(RootModel[Union[int, MaskValueItem]]): + root: Union[int, MaskValueItem] + + def __int__(self) -> int: + return int(self.root) + + +class BitMask(BaseModel): + description: Optional[str] = Field(None, description="Summary of the bit mask function.") + bits: Dict[str, MaskValue] + + +class GroupMask(BaseModel): + description: Optional[str] = Field(None, description="Summary of the group mask function.") + values: Dict[str, MaskValue] + + +class MaskType(RootModel[str]): + root: str + + +class InterfaceType(RootModel[str]): + root: str + + +class MinValue(RootModel[float]): + root: float + + +class MaxValue(RootModel[float]): + root: float + + +class DefaultValue(RootModel[float]): + root: float + + +class PayloadMember(BaseModel): + mask: Optional[int] = Field(None, description="Mask used to read/write this member.") + offset: Optional[int] = Field(None, description="Payload array offset of this member.") + length: Optional[int] = Field(None, description="Number of base elements this member spans.") + description: Optional[str] = Field(None, description="Summary of the payload member.") + minValue: Optional[MinValue] = None + maxValue: Optional[MaxValue] = None + defaultValue: Optional[DefaultValue] = None + maskType: Optional[MaskType] = None + interfaceType: Optional[InterfaceType] = None + converter: Optional[Converter] = None + + +class Register(BaseModel): + address: Annotated[int, Field(le=255, description="Unique 8-bit register address.")] + type: PayloadType + length: Annotated[Optional[int], Field(ge=1, default=1, description="Payload length.")] + access: Union[Access, List[Access]] = Field(..., description="Expected use of the register.") + description: Optional[str] = Field(None, description="Summary of the register function.") + minValue: Optional[MinValue] = None + maxValue: Optional[MaxValue] = None + defaultValue: Optional[DefaultValue] = None + maskType: Optional[MaskType] = None + visibility: Optional[Visibility] = Field( + None, description="Exposed in the high-level interface." + ) + volatile: Optional[bool] = Field(None, description="Value can be saved in non-volatile memory.") + payloadSpec: Optional[Dict[str, PayloadMember]] = None + interfaceType: Optional[InterfaceType] = None + converter: Optional[Converter] = None + + +class Registers(BaseModel): + """A bare register collection — a header-less ``device.yml`` fragment.""" + + registers: Dict[str, Register] = Field(..., description="The device's registers.") + bitMasks: Optional[Dict[str, BitMask]] = None + groupMasks: Optional[Dict[str, GroupMask]] = None + + +class DeviceModel(Registers): + """A device schema: a `Registers` collection plus (optional) device identity. + + Every identity field is optional, so a header-less fragment (just ``registers`` + / ``bitMasks`` / ``groupMasks``) is simply a ``DeviceModel`` with them all None + — parsing never needs to branch on "fragment vs full document". + """ + + device: Optional[str] = Field(None, description="The name of the device.") + whoAmI: Optional[int] = Field(None, description="Unique identifier for this device type.") + firmwareVersion: Optional[str] = Field( + None, description="Semantic version of the device firmware." + ) + hardwareTargets: Optional[str] = Field( + None, description="Semantic version of the device hardware." + ) diff --git a/src/packages/harp-protocol/src/harp/protocol/_register.py b/src/packages/harp-protocol/src/harp/protocol/_register.py index 9f221dc..6d992eb 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_register.py +++ b/src/packages/harp-protocol/src/harp/protocol/_register.py @@ -15,7 +15,7 @@ _TS_MICROS_OFFSET, ) from ._message import HarpMessage -from ._message_type import MessageType +from ._message_type import MessageType, message_type_to_byte from ._payload import ( Batch, PayloadBase, @@ -38,10 +38,26 @@ PayloadU64, PayloadU64Array, ) -from ._payload_type import PayloadType +from ._payload_type import PayloadType, encode_payload_type _MISSING = Sentinel("_MISSING") + +def _encode_message_types(message_type: Any, nrows: int) -> NDArray[np.uint8]: + """Resolve a scalar/array ``message_type`` argument to N message-type bytes. + + A single :class:`MessageType` (error-bit aware) fills all frames; a scalar int + is used verbatim; an array (e.g. the msgtype view from ``parse_bulk``, or a + list of ``MessageType``/ints) becomes the per-frame bytes. + """ + if isinstance(message_type, MessageType): + return np.full(nrows, message_type_to_byte(message_type), dtype=np.uint8) + values = np.asarray(message_type) + if values.ndim == 0: + return np.full(nrows, int(values.item()), dtype=np.uint8) + return values.astype(np.uint8) + + U = TypeVar("U") _R = TypeVar("_R") _AR = TypeVar("_AR", bound="RegisterBase[Any]") @@ -182,6 +198,76 @@ def parse_bulk( payload = payload_cls.from_array(payload_arr) return data, timestamps, msgtype_view, cast("Batch[Any]", payload) + @classmethod + def format_bulk( + cls, + values: Any, + *, + timestamps: Any = None, + message_type: Any = MessageType.Event, + port: int = _DEFAULT_PORT, + ) -> NDArray[np.uint8]: + """Build a flat buffer of N frames of this register type — the inverse of + :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 + 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 + if isinstance(values, PayloadBase): + records = np.atleast_1d(np.asarray(values.raw_payload)) + else: + records = np.atleast_1d(np.asarray(values)) + # Coerce the element type only for plain scalar payloads (e.g. an int + # list for a scalar register). Struct/sub-array records already carry + # the right byte layout and must not be re-cast. + 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 + ) + if plain and records.dtype != payload_cls.dtype: + records = records.astype(payload_cls.dtype) + nrows = len(records) + flat = np.ascontiguousarray(records).tobytes() + if len(flat) != nrows * itemsize: + raise ValueError( + f"{cls.__name__}.format_bulk: {len(flat)} payload bytes for {nrows} frames " + f"is not a multiple of itemsize {itemsize}; check the values shape/dtype" + ) + + is_timestamped = timestamps is not None + payload_offset = _TIMESTAMPED_PAYLOAD_OFFSET if is_timestamped else _HEADER_LEN + stride = payload_offset + itemsize + 1 # trailing checksum byte + + buf = np.zeros((nrows, stride), dtype=np.uint8) + buf[:, 0] = _encode_message_types(message_type, nrows) + buf[:, 1] = stride - 2 + buf[:, 2] = cls.address + buf[:, 3] = port + buf[:, 4] = encode_payload_type(cls.payload_type, has_timestamp=is_timestamped) + + if is_timestamped: + ts = np.atleast_1d(np.asarray(timestamps, dtype=np.float64)) + seconds = ts.astype(np.uint32) + micros = np.round((ts - seconds.astype(np.float64)) / _TICK_PERIOD_S).astype(np.uint16) + buf[:, _HEADER_LEN:_TS_MICROS_OFFSET] = np.frombuffer( + seconds.astype(" str: + """The generators test-metadata ``device.yml`` as text.""" + return (ASSETS / "device.yml").read_text() + + +@pytest.fixture(scope="session") +def common_yml() -> str: + """The Harp common (core) register set ``common.yml`` as text.""" + return (ASSETS / "common.yml").read_text() diff --git a/tests/device/__init__.py b/tests/device/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/device/data/__init__.py b/tests/device/data/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/device/data/converters.py b/tests/device/data/converters.py new file mode 100644 index 0000000..1957988 --- /dev/null +++ b/tests/device/data/converters.py @@ -0,0 +1,36 @@ +from typing import Any + +import numpy as np +from numpy.typing import NDArray + +from harp.protocol import Converter + + +class DataConverter(Converter[int]): + """Maps two raw little-endian signed bytes to and from a Python int. + + Models interfaceType: int over a two-byte sub-region of the CustomMemberConverter payload. + """ + + init_kwarg_type = int + + def __init__(self) -> None: + self._length = 2 + self.dtype = np.dtype((np.uint8, (self._length,))) + + def decode_scalar(self, view: np.generic) -> int: + return int.from_bytes(bytes(np.asarray(view).tolist()), "little", signed=True) + + def decode_batch(self, view: NDArray[np.generic]) -> Any: + return np.array( + [ + int.from_bytes(bytes(np.asarray(r).tolist()), "little", signed=True) + for r in np.atleast_2d(view) + ], + dtype=object, + ) + + def encode_into(self, view: NDArray[np.generic], value: int) -> None: + view[...] = np.frombuffer( + int(value).to_bytes(self._length, "little", signed=True), dtype=np.uint8 + ) diff --git a/tests/device/data/expected_core.py b/tests/device/data/expected_core.py new file mode 100644 index 0000000..6684278 --- /dev/null +++ b/tests/device/data/expected_core.py @@ -0,0 +1,229 @@ +# This file was automatically generated and should not be edited directly. +# To make changes, edit the device metadata and regenerate the interface. + +import enum +from typing import Any, ClassVar + +import numpy as np +from harp.protocol import ( + AnonymousPayload, + BitMask, + BoolConverter, + Field, + GroupMask, + PayloadType, + RegisterBase, + RegisterU16, + RegisterU32, + RegisterU8, + StringConverter, + StructPayload, +) + + +class ResetFlags(enum.IntFlag): + """Specifies the behavior of the non-volatile registers when resetting the device.""" + + RESTORE_DEFAULT = 0x1 + """The device will boot with all the registers reset to their default factory values.""" + RESTORE_EEPROM = 0x2 + """The device will boot and restore all the registers to the values stored in non-volatile memory.""" + SAVE = 0x4 + """The device will boot and save all the current register values to non-volatile memory.""" + RESTORE_NAME = 0x8 + """The device will boot with the default device name.""" + UPDATE_FIRMWARE = 0x20 + """The device will enter firmware update mode.""" + BOOT_FROM_DEFAULT = 0x40 + """Specifies that the device has booted from default factory values.""" + BOOT_FROM_EEPROM = 0x80 + """Specifies that the device has booted from non-volatile values stored in EEPROM.""" + + +class ClockConfigurationFlags(enum.IntFlag): + """Specifies configuration flags for the device synchronization clock.""" + + CLOCK_REPEATER = 0x1 + """The device will repeat the clock synchronization signal to the clock output connector, if available.""" + CLOCK_GENERATOR = 0x2 + """The device resets and generates the clock synchronization signal on the clock output connector, if available.""" + REPEATER_CAPABILITY = 0x8 + """Specifies the device has the capability to repeat the clock synchronization signal to the clock output connector.""" + GENERATOR_CAPABILITY = 0x10 + """Specifies the device has the capability to generate the clock synchronization signal to the clock output connector.""" + CLOCK_UNLOCK = 0x40 + """The device will unlock the timestamp register counter and will accept commands to set new timestamp values.""" + CLOCK_LOCK = 0x80 + """The device will lock the timestamp register counter and will not accept commands to set new timestamp values.""" + + +class OperationMode(enum.IntEnum): + """Specifies the operation mode of the device.""" + + STANDBY = 0 + """Disable all event reporting on the device.""" + ACTIVE = 1 + """Event detection is enabled. Only enabled events are reported by the device.""" + SPEED = 3 + """The device enters speed mode.""" + + +class EnableFlag(enum.IntEnum): + """Specifies whether a specific register flag is enabled or disabled.""" + + DISABLED = 0 + """Specifies that the flag is disabled.""" + ENABLED = 1 + """Specifies that the flag is enabled.""" + + +class OperationControlPayload(StructPayload[np.uint8]): + """Represents the payload of the OperationControl register.""" + + operation_mode: OperationMode = GroupMask(enum=OperationMode, mask=0x3) + """Specifies the operation mode of the device.""" + dump_registers: bool = Field(BoolConverter(), mask=0x8) + """Specifies whether the device should report the content of all registers on initialization.""" + mute_replies: bool = Field(BoolConverter(), mask=0x10) + """Specifies whether the replies to all commands will be muted, i.e. not sent by the device.""" + visual_indicators: EnableFlag = GroupMask(enum=EnableFlag, mask=0x20) + """Specifies the state of all visual indicators on the device.""" + operation_led: EnableFlag = GroupMask(enum=EnableFlag, mask=0x40) + """Specifies whether the device state LED should report the operation mode of the device.""" + heartbeat: EnableFlag = GroupMask(enum=EnableFlag, mask=0x80) + """Specifies whether the device should report the content of the seconds register each second.""" + + +class ResetDevicePayload(AnonymousPayload[np.uint8]): + """Represents the payload of the ResetDevice register.""" + + __value__: ResetFlags = BitMask(enum=ResetFlags) + + +class DeviceNamePayload(AnonymousPayload[np.uint8]): + """Represents the payload of the DeviceName register.""" + + __value__: str = Field(StringConverter(25)) + + +class ClockConfigurationPayload(AnonymousPayload[np.uint8]): + """Represents the payload of the ClockConfiguration register.""" + + __value__: ClockConfigurationFlags = BitMask(enum=ClockConfigurationFlags) + + +class WhoAmI(RegisterU16): + """Specifies the identity class of the device.""" + + address: ClassVar[int] = 0 + + +class HardwareVersionHigh(RegisterU8): + """Specifies the major hardware version of the device.""" + + address: ClassVar[int] = 1 + + +class HardwareVersionLow(RegisterU8): + """Specifies the minor hardware version of the device.""" + + address: ClassVar[int] = 2 + + +class AssemblyVersion(RegisterU8): + """Specifies the version of the assembled components in the device.""" + + address: ClassVar[int] = 3 + + +class CoreVersionHigh(RegisterU8): + """Specifies the major version of the Harp core implemented by the device.""" + + address: ClassVar[int] = 4 + + +class CoreVersionLow(RegisterU8): + """Specifies the minor version of the Harp core implemented by the device.""" + + address: ClassVar[int] = 5 + + +class FirmwareVersionHigh(RegisterU8): + """Specifies the major version of the Harp core implemented by the device.""" + + address: ClassVar[int] = 6 + + +class FirmwareVersionLow(RegisterU8): + """Specifies the minor version of the Harp core implemented by the device.""" + + address: ClassVar[int] = 7 + + +class TimestampSeconds(RegisterU32): + """Stores the integral part of the system timestamp, in seconds.""" + + address: ClassVar[int] = 8 + + +class TimestampMicroseconds(RegisterU16): + """Stores the fractional part of the system timestamp, in microseconds.""" + + address: ClassVar[int] = 9 + + +class OperationControl(RegisterBase[OperationControlPayload]): + """Stores the configuration mode of the device.""" + + address: ClassVar[int] = 10 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = OperationControlPayload + + +class ResetDevice(RegisterBase[ResetFlags]): + """Resets the device and saves non-volatile registers.""" + + address: ClassVar[int] = 11 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = ResetDevicePayload + + +class DeviceName(RegisterBase[str]): + """Stores the user-specified device name.""" + + address: ClassVar[int] = 12 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = DeviceNamePayload + + +class SerialNumber(RegisterU16): + """Specifies the unique serial number of the device.""" + + address: ClassVar[int] = 13 + + +class ClockConfiguration(RegisterBase[ClockConfigurationFlags]): + """Specifies the configuration for the device synchronization clock.""" + + address: ClassVar[int] = 14 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = ClockConfigurationPayload + + +REGISTER_MAP: dict[int, type[RegisterBase[Any]]] = { + 0: WhoAmI, + 1: HardwareVersionHigh, + 2: HardwareVersionLow, + 3: AssemblyVersion, + 4: CoreVersionHigh, + 5: CoreVersionLow, + 6: FirmwareVersionHigh, + 7: FirmwareVersionLow, + 8: TimestampSeconds, + 9: TimestampMicroseconds, + 10: OperationControl, + 11: ResetDevice, + 12: DeviceName, + 13: SerialNumber, + 14: ClockConfiguration, +} diff --git a/tests/device/data/expected_device.py b/tests/device/data/expected_device.py new file mode 100644 index 0000000..d2fbf4f --- /dev/null +++ b/tests/device/data/expected_device.py @@ -0,0 +1,252 @@ +# This file was automatically generated and should not be edited directly. +# To make changes, edit the device metadata and regenerate the interface. + +import enum +from typing import Any, ClassVar + +import numpy as np +from numpy.typing import NDArray +from harp.protocol import ( + AnonymousPayload, + BitMask, + BoolConverter, + Field, + GroupMask, + HarpVersion, + HarpVersionConverter, + IdentityConverter, + PayloadType, + RegisterBase, + RegisterS32, + RegisterU16, + RegisterU8, + StringConverter, + StructPayload, +) +from harp.device import REGISTER_MAP as _CORE_REGISTER_MAP + +from .converters import ( + DataConverter, +) + + +class PortDigitalIOS(enum.IntFlag): + DIO0 = 0x1 + DIO1 = 0x2 + DIO2 = 0x4 + DIO3 = 0x8 + DI_PORT0 = 0x100 + TEST_DI_PORT1 = 0x200 + SUPPLY_PORT0 = 0x400 + PORT_DIO1 = 0x800 + + +class PwmPort(enum.IntEnum): + PWM0 = 1 + PWM1 = 2 + PWM2 = 4 + PWM3 = 10 + + +class EncoderModeMask(enum.IntEnum): + """Specifies the type of encoder mode.""" + + POSITION = 0 + DISPLACEMENT = 1 + + +class AnalogDataPayload(StructPayload[np.float32], length=6): + """Represents the payload of the AnalogData register.""" + + analog0: np.float32 = Field(IdentityConverter(np.float32)) + 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 + ) + + +class ComplexConfigurationPayload(StructPayload[np.uint8], length=17): + """Represents the payload of the ComplexConfiguration register.""" + + pwm_port: PwmPort = GroupMask(enum=PwmPort, mask=0xFF) + 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 VersionPayload(StructPayload[np.uint8], length=32): + """Represents the payload of the Version register.""" + + protocol_version: HarpVersion = Field(HarpVersionConverter(np.uint8)) + 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 + ) + + +class CustomPayloadPayload(AnonymousPayload[np.uint32]): + """Represents the payload of the CustomPayload register.""" + + __value__: HarpVersion = Field(HarpVersionConverter(np.uint32)) + + +class CustomRawPayloadPayload(AnonymousPayload[np.uint32]): + """Represents the payload of the CustomRawPayload register.""" + + __value__: HarpVersion = Field(HarpVersionConverter(np.uint32)) + + +class CustomMemberConverterPayload(StructPayload[np.uint8], length=3): + """Represents the payload of the CustomMemberConverter register.""" + + header: np.uint8 = Field(IdentityConverter(np.uint8)) + data: np.int32 = Field(DataConverter(), offset=1) + + +class BitmaskSplitterPayload(StructPayload[np.uint8]): + """Represents the payload of the BitmaskSplitter register.""" + + low: np.int32 = Field(IdentityConverter(np.int32), mask=0xF) + high: np.int32 = Field(IdentityConverter(np.int32), mask=0xF0) + + +class PortDIOSetPayload(AnonymousPayload[np.uint8]): + """Represents the payload of the PortDIOSet register.""" + + __value__: PortDigitalIOS = BitMask(enum=PortDigitalIOS) + + +class StartPulsePayload(StructPayload[np.uint16]): + """Represents the payload of the StartPulse register.""" + + digital_output: PwmPort = GroupMask(enum=PwmPort, mask=0xC00) + pulse_width: np.uint16 = Field(IdentityConverter(np.uint16), mask=0x3FF) + + +class StartPulseTrainPayload(StructPayload[np.uint16], length=2): + """Represents the payload of the StartPulseTrain register.""" + + digital_output: PwmPort = GroupMask(enum=PwmPort, mask=0xC00) + pulse_width: np.uint16 = Field(IdentityConverter(np.uint16), mask=0x3FF) + frequency: np.uint8 = Field( + IdentityConverter(np.uint8), mask=0xFF00, offset=1, default=np.uint8(1) + ) + pulse_count: np.uint8 = Field(IdentityConverter(np.uint8), mask=0xFF, offset=1) + + +class EncoderModePayload(AnonymousPayload[np.uint8]): + """Represents the payload of the EncoderMode register.""" + + __value__: EncoderModeMask = GroupMask(enum=EncoderModeMask, mask=0xFF) + + +class DigitalInputs(RegisterU8): + address: ClassVar[int] = 32 + + +class AnalogData(RegisterBase[AnalogDataPayload]): + address: ClassVar[int] = 33 + payload_type: ClassVar[PayloadType] = PayloadType.Float + payload_class = AnalogDataPayload + + +class ComplexConfiguration(RegisterBase[ComplexConfigurationPayload]): + address: ClassVar[int] = 34 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = ComplexConfigurationPayload + + +class Version(RegisterBase[VersionPayload]): + address: ClassVar[int] = 35 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = VersionPayload + + +class CustomPayload(RegisterBase[HarpVersion]): + address: ClassVar[int] = 36 + payload_type: ClassVar[PayloadType] = PayloadType.U32 + payload_class = CustomPayloadPayload + + +class CustomRawPayload(RegisterBase[HarpVersion]): + address: ClassVar[int] = 37 + payload_type: ClassVar[PayloadType] = PayloadType.U32 + payload_class = CustomRawPayloadPayload + + +class CustomMemberConverter(RegisterBase[CustomMemberConverterPayload]): + address: ClassVar[int] = 38 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = CustomMemberConverterPayload + + +class BitmaskSplitter(RegisterBase[BitmaskSplitterPayload]): + address: ClassVar[int] = 39 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = BitmaskSplitterPayload + + +class Counter0(RegisterS32): + address: ClassVar[int] = 40 + + +class PortDIOSet(RegisterBase[PortDigitalIOS]): + address: ClassVar[int] = 41 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = PortDIOSetPayload + + +class PulseDOPort0(RegisterU16): + address: ClassVar[int] = 42 + + +class PulseDO0(RegisterU16): + address: ClassVar[int] = 43 + + +class StartPulse(RegisterBase[StartPulsePayload]): + """Starts a PWM pulse.""" + + address: ClassVar[int] = 100 + payload_type: ClassVar[PayloadType] = PayloadType.U16 + payload_class = StartPulsePayload + + +class StartPulseTrain(RegisterBase[StartPulseTrainPayload]): + """Starts a PWM pulse train.""" + + address: ClassVar[int] = 101 + payload_type: ClassVar[PayloadType] = PayloadType.U16 + payload_class = StartPulseTrainPayload + + +class EncoderMode(RegisterBase[EncoderModeMask]): + """Configures the operation mode of the encoder.""" + + address: ClassVar[int] = 103 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = EncoderModePayload + + +REGISTER_MAP: dict[int, type[RegisterBase[Any]]] = { + **_CORE_REGISTER_MAP, + 32: DigitalInputs, + 33: AnalogData, + 34: ComplexConfiguration, + 35: Version, + 36: CustomPayload, + 37: CustomRawPayload, + 38: CustomMemberConverter, + 39: BitmaskSplitter, + 40: Counter0, + 41: PortDIOSet, + 42: PulseDOPort0, + 43: PulseDO0, + 100: StartPulse, + 101: StartPulseTrain, + 103: EncoderMode, +} diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py new file mode 100644 index 0000000..c6e3770 --- /dev/null +++ b/tests/device/test_device_emit.py @@ -0,0 +1,66 @@ +import pytest +from harp.device import Device, create_device + +from .data.converters import DataConverter + +CONVERTERS = {"DataConverter": DataConverter()} + + +@pytest.fixture +def test_device(device_yml): + return create_device(device_yml, converters=CONVERTERS) + + +def test_returns_device_subclass(test_device): + assert issubclass(test_device, Device) + assert test_device.__name__ == "Tests" + + +def test_whoami_defaults_to_zero_when_absent(test_device): + # device.yml (application-device metadata) omits whoAmI. + assert test_device.__whoami__ == 0 + + +def test_whoami_from_schema(): + Dev = create_device( + "device: D\nwhoAmI: 1216\nregisters:\n Foo: {address: 40, type: U16, access: Read}\n" + ) + assert Dev.__whoami__ == 1216 + + +def test_registers_are_reachable_by_address(test_device): + reg_map = test_device.REGISTER_MAP + assert reg_map[33].__name__ == "AnalogData" + assert reg_map[103].__name__ == "EncoderMode" + + +def test_register_map_spreads_core(test_device): + reg_map = test_device.REGISTER_MAP + assert reg_map[0].__name__ == "WhoAmI" # core register, always spread in + assert reg_map[33].__name__ == "AnalogData" # device-specific + assert reg_map[103].__name__ == "EncoderMode" + + +def test_device_register_overrides_core_on_clash(): + # A device register at a core address wins over the spread-in common one. + Dev = create_device( + "device: Clash\nregisters:\n Shadow: {address: 0, type: U32, access: Read}\n" + ) + assert Dev.REGISTER_MAP[0].__name__ == "Shadow" + + +def test_headerless_fragment_builds_default_device(): + # A register-only fragment is a valid (nameless) device; name falls back to "Device". + Dev = create_device("registers:\n Foo: {address: 40, type: U16, access: Read}\n") + assert Dev.__name__ == "Device" + assert Dev.__whoami__ == 0 + assert Dev.REGISTER_MAP[40].__name__ == "Foo" + + +def test_emitted_device_registers_are_usable(test_device): + reg = test_device.REGISTER_MAP[33] # 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]) + ) + assert isinstance(frame, (bytes, bytearray)) diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py new file mode 100644 index 0000000..349ff96 --- /dev/null +++ b/tests/device/test_emit.py @@ -0,0 +1,182 @@ +import numpy as np +import pytest +from harp.protocol import HarpMessage + +from harp.device._schema import UnknownConverterError, create_registers + +from .data import expected_core, expected_device +from .data.converters import DataConverter + +CONVERTERS = {"DataConverter": DataConverter()} + + +@pytest.fixture +def device_registers(device_yml): + return create_registers(device_yml, converters=CONVERTERS) + + +def _device_registers(): + # expected_device.REGISTER_MAP spreads the core map; the device-specific + # registers (the ones the emitter builds from device.yml) are address >= 32. + return {cls.__name__: cls for addr, cls in expected_device.REGISTER_MAP.items() if addr >= 32} + + +def _layout(dt): + """Name-agnostic structural signature: element dtype + offset per field, 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. + """ + 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)) + + +# --------------------------------------------------------------------------- +# Device golden — layout/type parity with generator output +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("name", sorted(_device_registers())) +def test_device_register_matches_generator_layout(name, device_registers): + emitted = device_registers[name] + 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) + + +def test_device_emits_all_registers(device_registers): + assert set(device_registers) == set(_device_registers()) + + +# --------------------------------------------------------------------------- +# Verbatim naming — the yml is the single source of truth +# --------------------------------------------------------------------------- + + +def test_field_names_are_verbatim(device_registers): + fields = device_registers["AnalogData"].payload_class.dtype.names + assert fields == ("Analog0", "Analog1", "Analog2", "Accelerometer") + + +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__) + + +# --------------------------------------------------------------------------- +# Core golden — from protocol common.yml +# --------------------------------------------------------------------------- + + +def _core_expected(): + return {cls.__name__: cls for cls in expected_core.REGISTER_MAP.values()} + + +@pytest.mark.parametrize("name", sorted(_core_expected())) +def test_core_register_structural(name, common_yml): + emitted = create_registers(common_yml)[name] + 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 + 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) + + +# --------------------------------------------------------------------------- +# Behavioural round-trips +# --------------------------------------------------------------------------- + + +def _roundtrip(reg, value): + return reg.parse(HarpMessage.parse(reg.format(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"] + assert isinstance(parsed, enum_cls) + + +def test_whole_register_bitmask_roundtrip(device_registers): + reg = device_registers["PortDIOSet"] + flags = reg.payload_class._mro_descriptor("__value__")._enum + value = flags["DIO0"] | flags["DIO3"] + assert _roundtrip(reg, value) == value + + +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 + # matches the generator's output verbatim (GroupMask(enum=PwmPort, mask=0xC00)). + payload = payload_cls(DigitalOutput=pwm["Pwm1"], PulseWidth=np.uint16(300)) + parsed = _roundtrip(reg, payload) + assert parsed.DigitalOutput == pwm["Pwm1"] + assert int(parsed.PulseWidth) == 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 + + +# --------------------------------------------------------------------------- +# Converter registry +# --------------------------------------------------------------------------- + + +def test_unknown_converter_raises(device_yml): + with pytest.raises(UnknownConverterError): + create_registers(device_yml) # CustomMemberConverter needs DataConverter + + +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 + + +def test_converter_factory_receives_dsl_context(device_yml): + seen = {} + + def factory(ctx): + seen["name"], seen["span"], seen["interface_type"] = ctx.name, ctx.span, ctx.interface_type + return DataConverter() + + regs = create_registers(device_yml, converters={"DataConverter": factory}) + parsed = _roundtrip( + regs["CustomMemberConverter"], + 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 seen == {"name": "Data", "span": 2, "interface_type": "int"} + + +# --------------------------------------------------------------------------- +# Visibility +# --------------------------------------------------------------------------- + + +def test_exclude_private_drops_private_registers(): + yml = ( + "registers:\n" + " Pub: {address: 40, type: U16, access: Read}\n" + " Priv: {address: 41, type: U16, access: Read, visibility: private}\n" + ) + assert set(create_registers(yml)) == {"Pub", "Priv"} # kept by default + assert set(create_registers(yml, exclude_private=True)) == {"Pub"} diff --git a/tests/device/test_schema.py b/tests/device/test_schema.py new file mode 100644 index 0000000..7d01adc --- /dev/null +++ b/tests/device/test_schema.py @@ -0,0 +1,41 @@ +from harp.device._schema import DeviceModel, PayloadType, parse_device_schema + + +def test_parse_full_device(device_yml): + m = parse_device_schema(device_yml) + assert isinstance(m, DeviceModel) + assert m.device == "Tests" + assert m.whoAmI is None # this application-device metadata omits whoAmI + assert "AnalogData" in m.registers + ad = m.registers["AnalogData"] + assert ad.type is PayloadType.Float + assert ad.length == 6 + assert list(ad.payloadSpec) == ["Analog0", "Analog1", "Analog2", "Accelerometer"] + + +def test_parse_fragment_yields_null_device(): + m = parse_device_schema("registers:\n Foo: {address: 40, type: U16, access: Read}\n") + assert isinstance(m, DeviceModel) + assert m.device is None # header-less fragment -> identity fields are None + assert m.registers["Foo"].type is PayloadType.U16 + + +def test_parse_common_registers(common_yml): + c = parse_device_schema(common_yml) + assert c.device is None + assert "WhoAmI" in c.registers + # Off/On group-mask keys must stay strings (YAML 1.1 would coerce to bool). + assert {k: int(v) for k, v in c.groupMasks["LedState"].values.items()} == {"Off": 0, "On": 1} + # 'None' bit name stays a string, not YAML null. + assert "None" in c.bitMasks["ResetFlags"].bits + + +def test_bool_values_preserved(common_yml): + c = parse_device_schema(common_yml) + assert c.registers["TimestampSeconds"].volatile is True + + +def test_access_list_and_scalar(common_yml): + c = parse_device_schema(common_yml) + # TimestampSeconds has a list access [Read, Write, Event]; WhoAmI a scalar. + assert isinstance(c.registers["TimestampSeconds"].access, list) diff --git a/tests/protocol/test_register.py b/tests/protocol/test_register.py index be165e4..b63c777 100644 --- a/tests/protocol/test_register.py +++ b/tests/protocol/test_register.py @@ -4,7 +4,7 @@ import numpy as np import pytest -from harp.data import payload_to_dataframe +from harp.data import parse_to_dataframe, payload_to_dataframe, to_buffer, to_file from harp.protocol._message import HarpMessage from harp.protocol._message_type import MessageType from harp.protocol._payload import ( @@ -514,3 +514,44 @@ class P(PayloadBase): high = Field(converter=_IdentityConverter("u1"), mask=0xF0, offset=0) assert P._repr_fields == ("low", "high") + + +# --------------------------------------------------------------------------- +# format_bulk (inverse of parse_bulk) + harp.data.to_buffer / to_file +# --------------------------------------------------------------------------- + + +def test_format_bulk_single_matches_format(): + reg = RegisterU16(0x20) + one = reg.format(np.uint16(42), message_type=MessageType.Event, timestamp=1.0) + bulk = reg.format_bulk( + np.array([42], dtype=" Date: Sat, 25 Jul 2026 18:48:16 -0700 Subject: [PATCH 02/22] Organize test directory --- tests/device/{data => }/converters.py | 0 tests/device/data/__init__.py | 0 tests/device/{data => }/expected_core.py | 0 tests/device/{data => }/expected_device.py | 0 tests/device/test_device_emit.py | 2 +- tests/device/test_emit.py | 4 ++-- 6 files changed, 3 insertions(+), 3 deletions(-) rename tests/device/{data => }/converters.py (100%) delete mode 100644 tests/device/data/__init__.py rename tests/device/{data => }/expected_core.py (100%) rename tests/device/{data => }/expected_device.py (100%) diff --git a/tests/device/data/converters.py b/tests/device/converters.py similarity index 100% rename from tests/device/data/converters.py rename to tests/device/converters.py diff --git a/tests/device/data/__init__.py b/tests/device/data/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/device/data/expected_core.py b/tests/device/expected_core.py similarity index 100% rename from tests/device/data/expected_core.py rename to tests/device/expected_core.py diff --git a/tests/device/data/expected_device.py b/tests/device/expected_device.py similarity index 100% rename from tests/device/data/expected_device.py rename to tests/device/expected_device.py diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py index c6e3770..c3e996c 100644 --- a/tests/device/test_device_emit.py +++ b/tests/device/test_device_emit.py @@ -1,7 +1,7 @@ import pytest from harp.device import Device, create_device -from .data.converters import DataConverter +from .converters import DataConverter CONVERTERS = {"DataConverter": DataConverter()} diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index 349ff96..d553e39 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -4,8 +4,8 @@ from harp.device._schema import UnknownConverterError, create_registers -from .data import expected_core, expected_device -from .data.converters import DataConverter +from . import expected_core, expected_device +from .converters import DataConverter CONVERTERS = {"DataConverter": DataConverter()} From ff4a728bf5e707dc1fa5b828be744ed8c4d86dd0 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:49:50 -0700 Subject: [PATCH 03/22] Allow for out-of-range enum values --- .../src/harp/protocol/_payload.py | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/packages/harp-protocol/src/harp/protocol/_payload.py b/src/packages/harp-protocol/src/harp/protocol/_payload.py index 22e5c74..896ff2a 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_payload.py +++ b/src/packages/harp-protocol/src/harp/protocol/_payload.py @@ -180,7 +180,8 @@ def _columns( def _build_enum_lookup(enum_cls: type[enum.IntEnum]) -> "tuple[list[str], np.ndarray]": - """Helper for GroupMask to build the category list and code lookup table for a given enum.IntEnum class.""" + """Category list + a code table mapping each raw enum value (``0..max member``) to its + category index; a raw value with no member maps to -1.""" members = list(enum_cls) categories = [m.name for m in members] max_val = max(int(m) for m in members) @@ -234,10 +235,16 @@ def __init__( self._slot: str = "" self._dtype: np.dtype = _DEFAULT_ELEMENT self._categories, self._code_lookup = _build_enum_lookup(enum) + self._lookup_safe = (mask >> self._shift) < len(self._code_lookup) def _decode_raw(self, raw: Any) -> Any: - """Map an extracted (already masked + shifted) integer to its enum member.""" - return self._enum(int(raw)) + """Map an extracted (masked + shifted) integer to its enum member, preserving an + undefined code as its raw int (permissive, like C#'s unchecked enum cast).""" + value = int(raw) + try: + return self._enum(value) + except ValueError: + return value def _encode_value(self, value: Any) -> int: """Map a user value back to the integer to be masked + shifted into the slot.""" @@ -272,9 +279,24 @@ def _columns( ) -> "list[Column]": """One enum column: category codes + labels (``decode_enums``) or raw codes.""" raw = (arr[self._slot] & self._mask) >> self._shift - if decode_enums: - return [Column(name, self._code_lookup[raw], self._categories)] - return [Column(name, raw)] + if not decode_enums: + return [Column(name, raw)] + lookup = self._code_lookup + # ``_lookup_safe`` (the field's raw range fits the table) skips the bounds guard; + # an in-range gap still maps to -1, so the undefined branch below runs regardless. + if self._lookup_safe: + codes = lookup[raw] + else: + codes = np.where(raw < len(lookup), lookup.take(raw, mode="clip"), -1) + undefined = codes < 0 + if not undefined.any(): + return [Column(name, codes, self._categories)] + # An undefined code (an in-range gap, or a value past the enum's range) is kept + # as its raw integer — an extra category — matching the scalar decode and C#'s + codes = codes.astype(np.intp) + extras = np.unique(raw[undefined]) + codes[undefined] = len(self._categories) + np.searchsorted(extras, raw[undefined]) + return [Column(name, codes, list(self._categories) + extras.tolist())] class BitMask(Generic[F]): From 8697e213406293087da62a994bcad4d0d1d16dce Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:50:14 -0700 Subject: [PATCH 04/22] Add tests for non-contiguous and out-of-range enum values --- tests/protocol/test_payload.py | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/protocol/test_payload.py b/tests/protocol/test_payload.py index 4444ec1..a7c8105 100644 --- a/tests/protocol/test_payload.py +++ b/tests/protocol/test_payload.py @@ -1,7 +1,9 @@ +import enum + import numpy as np import pytest from harp.data import payload_to_dataframe -from harp.protocol import Column +from harp.protocol import AnonymousPayload, Column, GroupMask from harp.protocol._payload import PayloadBase, Field, _IdentityConverter @@ -69,3 +71,30 @@ def test_from_buffer_zero_copy(): def test_payload_property(): p = SimplePayload.from_buffer(_make_simple_bytes(2)) assert p.raw_payload.dtype == SimplePayload.dtype + + +class _SparseMode(enum.IntEnum): + Low = 0 + High = 2 # gap at code 1; largest member is 2 + + +class _SparseModePayload(AnonymousPayload[np.uint8]): + # Whole-byte GroupMask over a sparse enum: raw can be 0..255, well past the + # largest member, so decode must not IndexError on undefined codes. + __value__ = GroupMask(enum=_SparseMode, mask=0xFF) + + +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()) + 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()) + assert defined.__value__ is _SparseMode.High + undefined = _SparseModePayload.from_buffer(np.array([90], dtype=np.uint8).tobytes()) + assert undefined.__value__ == 90 + assert not isinstance(undefined.__value__, _SparseMode) From ba7c979bf4bb6c6bb1416d44a871e65608c89692 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:50:36 -0700 Subject: [PATCH 05/22] Add round-trip tests between static and runtime generated registers --- tests/device/test_emit.py | 40 ++++++++++++++++++++++++ tests/protocol/test_register_modeling.py | 10 +++--- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index d553e39..4c61667 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -1,5 +1,8 @@ +import zlib + import numpy as np import pytest +from harp.data import parse_to_dataframe from harp.protocol import HarpMessage from harp.device._schema import UnknownConverterError, create_registers @@ -180,3 +183,40 @@ def test_exclude_private_drops_private_registers(): ) assert set(create_registers(yml)) == {"Pub", "Priv"} # kept by default assert set(create_registers(yml, exclude_private=True)) == {"Pub"} + + +# --------------------------------------------------------------------------- +# Golden bulk round-trip — the emitted register and the generator oracle are +# wire- and dataframe-compatible for the same payload bytes (cross read/write). +# --------------------------------------------------------------------------- + + +def _random_records(dtype, n, seed): + """``n`` deterministic records of ``dtype`` with random ASCII-range bytes. + + Bytes are held to 0..127 so every field varies while staying valid for any + ``StringConverter`` member and free of float NaN/inf (which would defeat the + value comparison); padding bytes are filled too but never read back. + """ + rng = np.random.default_rng(seed) + raw = rng.integers(0, 128, size=n * dtype.itemsize, dtype=np.uint8) + return raw.view(dtype).copy() + + +@pytest.mark.parametrize("name", sorted(_device_registers())) +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())) + + # Cross-write: same address / payload_type / byte layout -> identical wire bytes. + buf = bytes(emitted.format_bulk(records)) + 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 + assert df_emitted.equals(df_oracle) diff --git a/tests/protocol/test_register_modeling.py b/tests/protocol/test_register_modeling.py index 50ec3ad..1f22b54 100644 --- a/tests/protocol/test_register_modeling.py +++ b/tests/protocol/test_register_modeling.py @@ -4,7 +4,6 @@ """ import numpy as np -import pytest from harp.data import payload_to_dataframe from harp.protocol import HarpMessage, HarpVersion from harp.benchmarks.register_models import ( @@ -188,16 +187,17 @@ def test_custom_payload_single_member_unwrap(): # --------------------------------------------------------------------------- -# Strict enums: an out-of-range masked code raises +# Undefined masked enum codes are preserved as their raw int (permissive, like C#) # --------------------------------------------------------------------------- -def test_strict_enum_raises_on_unknown_code(): +def test_unknown_enum_code_preserves_raw(): # StartPulse.DigitalOutput 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) - with pytest.raises(ValueError): - _ = payload.DigitalOutput + value = payload.DigitalOutput # permissive: the raw code is kept, not raised + assert value == 0b11 + assert not isinstance(value, PwmPort) # --------------------------------------------------------------------------- From 1cc73a496cf4d7d6b010aee0c9cb4e2fbd09acf7 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:50:59 -0700 Subject: [PATCH 06/22] Refactor the benchmark package to use the `format_bulk` API --- .../src/harp/benchmarks/_registers.py | 120 ++++-------------- .../src/harp/benchmarks/generate.py | 31 +++-- 2 files changed, 49 insertions(+), 102 deletions(-) diff --git a/src/packages/harp-benchmarks/src/harp/benchmarks/_registers.py b/src/packages/harp-benchmarks/src/harp/benchmarks/_registers.py index 7033400..461cb58 100644 --- a/src/packages/harp-benchmarks/src/harp/benchmarks/_registers.py +++ b/src/packages/harp-benchmarks/src/harp/benchmarks/_registers.py @@ -1,14 +1,10 @@ """Shared benchmark fixtures: every register from ``harp.benchmarks.register_models`` -paired with a representative sample value. +paired with whether its frames carry a timestamp. Both ``generate.py`` (writes the .bin corpora) and ``benchmark.py`` (times parsing) -import :data:`BENCHMARK_REGISTERS` from here so the two stay in lock-step: the value -used to *format* each frame is the same one whose parsed shape we benchmark. - -The sample values mirror ``register_models.main()``'s round-trip fixtures — they -exercise the full spread of payload shapes the Harp protocol allows (trivial -scalars, struct payloads with byte gaps, masked sub-fields, custom converters, -enum/flag single-member unwrap). +import :data:`BENCHMARK_REGISTERS` from here so the two stay in lock-step. Payloads +are synthesized as random bytes per frame at generation time (see ``generate.py``), +so no sample values live here — only the register class and its frame shape. All generated artifacts live under ``./benchmark`` in the current working directory. """ @@ -16,36 +12,24 @@ from pathlib import Path from typing import Any, NamedTuple -import numpy as np - from harp.benchmarks.register_models import ( AnalogData, - AnalogDataPayload, BitmaskSplitter, - BitmaskSplitterPayload, ComplexConfiguration, - ComplexConfigurationPayload, Counter0, CustomMemberConverter, - CustomMemberConverterPayload, CustomPayload, CustomRawPayload, DigitalInputs, EncoderMode, - EncoderModeMask, - PortDigitalIOS, PortDIOSet, PulseDO0, PulseDOPort0, - PwmPort, StartPulse, - StartPulsePayload, StartPulseTrain, - StartPulseTrainPayload, Version, - VersionPayload, ) -from harp.protocol import HarpVersion, RegisterBase +from harp.protocol import RegisterBase ARTIFACTS_DIR = Path("benchmark").resolve() DATA_DIR = ARTIFACTS_DIR / "data" @@ -53,11 +37,10 @@ class BenchmarkedRegister(NamedTuple): - """A register under benchmark together with a value that ``format()`` accepts.""" + """A register under benchmark, with whether its corpus frames are timestamped.""" name: str register: type[RegisterBase[Any]] - value: Any timestamped: bool = True @property @@ -70,77 +53,28 @@ def filename(self) -> str: def _base_registers() -> list[BenchmarkedRegister]: - """One (timestamped) fixture per register — :func:`_build` derives the untimestamped twin.""" + """One (timestamped) fixture per register — :func:`_build` derives the untimestamped twin. + + The set spans the full spread of payload shapes the Harp protocol allows: trivial + scalars, struct payloads with byte gaps, masked sub-fields, custom converters, and + enum/flag single-member unwrap. + """ return [ - BenchmarkedRegister("DigitalInputs", DigitalInputs, np.uint8(0b1010)), - BenchmarkedRegister( - "AnalogData", - AnalogData, - 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), - ), - ), - BenchmarkedRegister( - "ComplexConfiguration", - ComplexConfiguration, - ComplexConfigurationPayload( - PwmPort=PwmPort.Pwm2, - DutyCycle=np.float32(0.5), - Frequency=np.float32(1000.0), - EventsEnabled=True, - Delta=np.uint32(42), - ), - ), - BenchmarkedRegister( - "Version", - Version, - 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), - ), - ), - BenchmarkedRegister("CustomPayload", CustomPayload, HarpVersion(3, 1, 4)), - BenchmarkedRegister("CustomRawPayload", CustomRawPayload, HarpVersion(0, 0, 1)), - BenchmarkedRegister( - "CustomMemberConverter", - CustomMemberConverter, - CustomMemberConverterPayload(Header=np.uint8(7), Data=-1234), - ), - BenchmarkedRegister( - "BitmaskSplitter", - BitmaskSplitter, - BitmaskSplitterPayload(Low=np.int32(0xA), High=np.int32(0x5)), - ), - BenchmarkedRegister("Counter0", Counter0, np.int32(-100000)), - BenchmarkedRegister( - "PortDIOSet", - PortDIOSet, - PortDigitalIOS.DIO0 | PortDigitalIOS.DIO3, - ), - BenchmarkedRegister("PulseDOPort0", PulseDOPort0, np.uint16(5)), - BenchmarkedRegister("PulseDO0", PulseDO0, np.uint16(9)), - BenchmarkedRegister( - "StartPulse", - StartPulse, - StartPulsePayload(DigitalOutput=PwmPort.Pwm1, PulseWidth=np.uint16(300)), - ), - BenchmarkedRegister( - "StartPulseTrain", - StartPulseTrain, - StartPulseTrainPayload( - DigitalOutput=PwmPort.Pwm1, - PulseWidth=np.uint16(300), - Frequency=np.uint8(200), - PulseCount=np.uint8(50), - ), - ), - BenchmarkedRegister("EncoderMode", EncoderMode, EncoderModeMask.Displacement), + BenchmarkedRegister("DigitalInputs", DigitalInputs), + BenchmarkedRegister("AnalogData", AnalogData), + BenchmarkedRegister("ComplexConfiguration", ComplexConfiguration), + BenchmarkedRegister("Version", Version), + BenchmarkedRegister("CustomPayload", CustomPayload), + BenchmarkedRegister("CustomRawPayload", CustomRawPayload), + BenchmarkedRegister("CustomMemberConverter", CustomMemberConverter), + BenchmarkedRegister("BitmaskSplitter", BitmaskSplitter), + BenchmarkedRegister("Counter0", Counter0), + BenchmarkedRegister("PortDIOSet", PortDIOSet), + BenchmarkedRegister("PulseDOPort0", PulseDOPort0), + BenchmarkedRegister("PulseDO0", PulseDO0), + BenchmarkedRegister("StartPulse", StartPulse), + BenchmarkedRegister("StartPulseTrain", StartPulseTrain), + BenchmarkedRegister("EncoderMode", EncoderMode), ] diff --git a/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py b/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py index 64c62ad..1567a9d 100644 --- a/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py +++ b/src/packages/harp-benchmarks/src/harp/benchmarks/generate.py @@ -2,9 +2,11 @@ import sys from pathlib import Path +import numpy as np + from harp.benchmarks._registers import BENCHMARK_REGISTERS, DATA_DIR, BenchmarkedRegister -_TIMESTAMP = 42 +_SEED = 42 def corpus_path(reg: BenchmarkedRegister, data_dir: Path = DATA_DIR): @@ -12,19 +14,30 @@ def corpus_path(reg: BenchmarkedRegister, data_dir: Path = DATA_DIR): return data_dir / reg.filename -def _frame_timestamp(reg: BenchmarkedRegister) -> int | None: - return _TIMESTAMP if reg.timestamped else None +def _frames(reg: BenchmarkedRegister, entries: int) -> np.ndarray: + """Build ``entries`` frames of ``reg`` with a random per-frame payload. + + Bytes are held to the ASCII range (0..127) so every field varies while staying + valid for any ``StringConverter`` member and free of float NaN/inf — the corpus is + 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 + 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 + return reg.register.format_bulk(records, timestamps=timestamps) def generate_one( reg: BenchmarkedRegister, entries: int, data_dir: Path = DATA_DIR ) -> tuple[str, int, int]: """Write ``entries`` frames for ``reg``. Returns (path, frame_size, file_size).""" - frame = reg.register.format(reg.value, timestamp=_frame_timestamp(reg)) + buf = _frames(reg, entries) path = corpus_path(reg, data_dir) path.parent.mkdir(parents=True, exist_ok=True) - path.write_bytes(frame * entries) - return str(path), len(frame), path.stat().st_size + path.write_bytes(buf.tobytes()) + return str(path), len(buf) // entries, path.stat().st_size def ensure_corpus( @@ -33,13 +46,13 @@ def ensure_corpus( """Generate ``reg``'s corpus unless a matching cached file already exists. The cache is honored only when the existing file's size matches ``entries`` - exactly (frame_size * entries); a stale file (different entry count) is rebuilt. + exactly (stride * entries); a stale file (different entry count) is rebuilt. Returns (path, generated). """ path = corpus_path(reg, data_dir) if path.exists() and not force: - frame_size = len(reg.register.format(reg.value, timestamp=_frame_timestamp(reg))) - if path.stat().st_size == frame_size * entries: + stride = len(_frames(reg, 1)) + if path.stat().st_size == stride * entries: return path, False generate_one(reg, entries, data_dir) return path, True From e4cb97755d855e2c7b3e84a223533b688dc1eb13 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:04:10 -0700 Subject: [PATCH 07/22] Add type hints and a base REGISTER_MAP default Narrow the Any-typed inputs of format_bulk / to_buffer / to_file to PayloadBase | ArrayLike, ArrayLike | None, and MessageType | ArrayLike. Give Device a base REGISTER_MAP ClassVar default ({}) that generated subclasses override, and tidy the create_device docstring. --- .../harp-data/src/harp/data/_write.py | 20 +++++++++---------- .../harp-device/src/harp/device/_device.py | 3 +++ .../src/harp/device/_emit_device.py | 8 ++++---- .../src/harp/protocol/_register.py | 10 +++++----- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/packages/harp-data/src/harp/data/_write.py b/src/packages/harp-data/src/harp/data/_write.py index b2cd2e6..ab48c58 100644 --- a/src/packages/harp-data/src/harp/data/_write.py +++ b/src/packages/harp-data/src/harp/data/_write.py @@ -5,19 +5,19 @@ """ from os import PathLike -from typing import Any, Union +from typing import Any import numpy as np -from harp.protocol import MessageType, RegisterBase -from numpy.typing import NDArray +from harp.protocol import MessageType, PayloadBase, RegisterBase +from numpy.typing import ArrayLike, NDArray def to_buffer( register: type[RegisterBase[Any]], - values: Any, + values: PayloadBase | ArrayLike, *, - timestamps: Any = None, - message_type: Any = MessageType.Event, + timestamps: ArrayLike | None = None, + message_type: MessageType | ArrayLike = MessageType.Event, port: int = 255, ) -> NDArray[np.uint8]: """Encode ``values`` as a flat buffer of ``register`` frames. @@ -32,11 +32,11 @@ def to_buffer( def to_file( register: type[RegisterBase[Any]], - values: Any, - file: Union[str, PathLike], + values: PayloadBase | ArrayLike, + file: str | PathLike, *, - timestamps: Any = None, - message_type: Any = MessageType.Event, + timestamps: ArrayLike | None = None, + message_type: MessageType | ArrayLike = MessageType.Event, port: int = 255, ) -> None: """Write ``values`` as ``register`` frames to ``file`` (see :func:`to_buffer`).""" diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index 69adfd7..d765da3 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -81,6 +81,9 @@ class Device: #: Expected ``WhoAmI`` of the device this class models; ``0x0`` skips the check. __whoami__: ClassVar[int] = 0x0 + #: Address -> register class; empty on the base, overridden by generated devices. + REGISTER_MAP: ClassVar[dict[int, type[RegisterBase[Any]]]] = {} + def __init__(self, transport: ITransport, *, raise_on_error: bool = True) -> None: self._transport = transport self.raise_on_error = raise_on_error diff --git a/src/packages/harp-device/src/harp/device/_emit_device.py b/src/packages/harp-device/src/harp/device/_emit_device.py index 7425d96..82f43b4 100644 --- a/src/packages/harp-device/src/harp/device/_emit_device.py +++ b/src/packages/harp-device/src/harp/device/_emit_device.py @@ -17,10 +17,10 @@ def create_device( ) -> type[Device]: """Emit a :class:`Device` subclass from a device schema. - The returned class exposes its registers through ``REGISTER_MAP`` (address -> - register class) and carries ``__whoami__`` from the schema (``0x0`` when - absent). The device's registers are spread on top of the core common map; on - an address clash the device's register wins. ``exclude_private=True`` drops + The returned class exposes its registers through the ``REGISTER_MAP`` class + attribute (address -> register class) and carries ``__whoami__`` from the schema + (``0x0`` when absent). The device's registers are spread on top of the core common + map; on an address clash the device's register wins. ``exclude_private=True`` drops registers whose DSL ``visibility`` is ``private``. A header-less register fragment yields a device with no ``device`` name (falls back to ``"Device"``). """ diff --git a/src/packages/harp-protocol/src/harp/protocol/_register.py b/src/packages/harp-protocol/src/harp/protocol/_register.py index 6d992eb..4e07864 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_register.py +++ b/src/packages/harp-protocol/src/harp/protocol/_register.py @@ -2,7 +2,7 @@ from typing import Any, ClassVar, Generic, TypeVar, cast, final, overload import numpy as np -from numpy.typing import NDArray +from numpy.typing import ArrayLike, NDArray from typing_extensions import Sentinel from ._builder import build_message_frame @@ -43,7 +43,7 @@ _MISSING = Sentinel("_MISSING") -def _encode_message_types(message_type: Any, nrows: int) -> NDArray[np.uint8]: +def _encode_message_types(message_type: MessageType | ArrayLike, nrows: int) -> NDArray[np.uint8]: """Resolve a scalar/array ``message_type`` argument to N message-type bytes. A single :class:`MessageType` (error-bit aware) fills all frames; a scalar int @@ -201,10 +201,10 @@ def parse_bulk( @classmethod def format_bulk( cls, - values: Any, + values: PayloadBase | ArrayLike, *, - timestamps: Any = None, - message_type: Any = MessageType.Event, + timestamps: ArrayLike | None = None, + message_type: MessageType | ArrayLike = MessageType.Event, port: int = _DEFAULT_PORT, ) -> NDArray[np.uint8]: """Build a flat buffer of N frames of this register type — the inverse of From 933071c4d81a7a8debcbf02fb6824c09eb4107c7 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 09:45:14 -0700 Subject: [PATCH 08/22] Document runtime device generation Add a "Generating a Device from a Schema" example walking through `create_device`, including a win/loss discussion of runtime generation vs. pre-generated device packages. Wire it into the examples index and nav, surface `create_device`/`parse_device_schema`/`ConverterContext` in the device API reference, and add a Quickstart to the root README. --- README.md | 39 ++++++++++++++ docs/api/device.md | 3 ++ docs/examples/create_device/create_device.md | 53 ++++++++++++++++++++ docs/examples/create_device/create_device.py | 38 ++++++++++++++ docs/examples/index.md | 10 +++- mkdocs.yml | 3 +- 6 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 docs/examples/create_device/create_device.md create mode 100644 docs/examples/create_device/create_device.py diff --git a/README.md b/README.md index 9b35717..89199f6 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,45 @@ pip install harp-data `harp-benchmarks` (under `src/packages/`) is internal-only and is never published to PyPI. +## Quickstart + +Have only a device's `device.yml`? `create_device` compiles it into a typed +`Device` at runtime — no code-generation step — giving you the device's registers +(keyed by address) and its identity: + +```python +from pathlib import Path +from harp.device import create_device + +Behavior = create_device(Path("device.yml").read_text()) +Behavior.__whoami__ # device identity from the schema +AnalogData = Behavior.REGISTER_MAP[44] # registers are reached by address +``` + +The generated device works like any other. **Talk to hardware** over a serial +transport — `read`/`write` take a register class: + +```python +from harp.serial import open_serial_device + +# Use "COMx" on Windows, "/dev/ttyUSBx" on Linux. +with open_serial_device(Behavior, port="/dev/ttyUSB0") as device: + print(device.read(AnalogData).parsed) +``` + +...or use the same register classes to **decode recorded data** into a pandas +DataFrame: + +```python +from harp.data import parse_to_dataframe + +df = parse_to_dataframe(AnalogData, "Behavior_44.bin") +``` + +See the [Examples](https://harp-tech.org/pyharp/examples/) for full walkthroughs, +including reading device info, subscribing to events, and working with custom +interface-type converters. + ## Contributing harp is a [uv workspace](https://docs.astral.sh/uv/concepts/workspaces/): every package under diff --git a/docs/api/device.md b/docs/api/device.md index dfcade9..652e4c7 100644 --- a/docs/api/device.md +++ b/docs/api/device.md @@ -3,6 +3,9 @@ --- ::: harp.device.Device +::: harp.device.create_device +::: harp.device.parse_device_schema +::: harp.device.ConverterContext ::: harp.device.HarpFramer ::: harp.device.ITransport ::: harp.device.TransportError diff --git a/docs/examples/create_device/create_device.md b/docs/examples/create_device/create_device.md new file mode 100644 index 0000000..15907d9 --- /dev/null +++ b/docs/examples/create_device/create_device.md @@ -0,0 +1,53 @@ +# Generating a Device from a Schema + +This example demonstrates how to turn a Harp `device.yml` into a typed +[`Device`](../../api/device.md) at runtime with `create_device`, without a +code-generation step. This is the quickest way to get started when you have only a +device's schema and no pre-generated package for it. + +The compiled device exposes its registers through `REGISTER_MAP` (keyed by address) +and carries the device's `__whoami__` identity. From there it works exactly like a +pre-generated device class — drive it over a transport to talk to hardware, or use +its register classes to decode recorded data. + +## When to use runtime generation + +`create_device` trades statically generated device packages for schema-driven +convenience. It's worth understanding what that buys you and what it costs. + +**You gain:** + +- **No build step.** A `device.yml` — even one you just pulled off a device — + becomes a working device in a single call. There's nothing to generate, install, + 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. + +**You give up:** + +- **Named, typed access.** Registers are reached by address (`REGISTER_MAP[44]`), + not as importable, autocompleting classes (`from harp_behavior import AnalogData`). + You lose editor discovery and static type checking of register names. +- **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. + +For shipped, widely-used devices a pre-generated package from the +[Harp C# generator](https://github.com/harp-tech/generators) remains the +authoritative choice — better editor support and static typing. Reach for +`create_device` when you want to go from a schema to working code with no +generation step. + +!!! warning + Don't forget to change the `SERIAL_PORT` to the one that corresponds to your device! The `SERIAL_PORT` must be denoted as `/dev/ttyUSBx` in Linux and `COMx` in Windows, where `x` is the number of the serial port. + + +```python +[](./create_device.py) +``` + diff --git a/docs/examples/create_device/create_device.py b/docs/examples/create_device/create_device.py new file mode 100644 index 0000000..11ec1b8 --- /dev/null +++ b/docs/examples/create_device/create_device.py @@ -0,0 +1,38 @@ +from pathlib import Path + +from harp.data import parse_to_dataframe +from harp.device import create_device +from harp.serial import open_serial_device + +SERIAL_PORT = "/dev/ttyUSB0" # or "COMx" in Windows ("x" is the number of the serial port) + +# `create_device` compiles a Harp `device.yml` into a typed `Device` subclass at +# runtime — no code-generation step. This is the quickest way to work with a device +# when you don't have a pre-generated package for it: point it at the schema and you +# get the device's registers (keyed by address) plus its identity. +Behavior = create_device(Path("device.yml").read_text()) + +print("WhoAmI:", Behavior.__whoami__) # device identity, taken from the schema +AnalogData = Behavior.REGISTER_MAP[44] # registers are reached by address + +# The generated device behaves like any other `Device` class. Talk to hardware over +# a transport — `read`/`write` take a register class: +with open_serial_device(Behavior, port=SERIAL_PORT) as device: + print("AnalogData:", device.read(AnalogData).parsed) + +# ...or use the same register classes to decode a recorded binary dump into a +# pandas DataFrame (see the "Reading Data into a DataFrame" example for more): +df = parse_to_dataframe(AnalogData, "Behavior_44.bin") +print(df.head()) + + +# --- Custom interface types -------------------------------------------------- +# A register with a custom `interfaceType` needs a converter so its field decodes +# to the right Python type. Pass it via `converters=`, keyed by "Converter": +# +# Behavior = create_device(yml_text, converters={"DataConverter": DataConverter()}) +# +# An unresolved custom type raises `UnknownConverterError`; pass `strict=False` to +# decode it natively instead. `exclude_private=True` (the default) drops registers +# marked `private` in the schema. If you only want the parsed schema model rather +# than a device, `parse_device_schema(yml_text)` returns that directly. diff --git a/docs/examples/index.md b/docs/examples/index.md index 52e4245..492d90a 100644 --- a/docs/examples/index.md +++ b/docs/examples/index.md @@ -2,8 +2,16 @@ This section contains some examples to help you get started with `harp`. -Here's the complete list of available examples: +Working from a device schema: + +- [Generating a Device from a Schema](./create_device/create_device.md) - compile a `device.yml` into a typed device at runtime with `create_device`. + +Talking to a device: - [Getting Device Info](./get_info/get_info.md) - connect to a Harp device and read its information. - [Read and Write from Registers](./read_and_write_from_registers/read_and_write_from_registers.md) - connect to a Harp device and read and write its registers. +- [Subscribing to Events](./subscribing_to_events/subscribing_to_events.md) - react to messages pushed by the device without polling. + +Reading recorded data: + - [Reading Data into a DataFrame](./read_data_to_dataframe/read_data_to_dataframe.md) - load a register's binary data file into a pandas DataFrame with `harp.data`. diff --git a/mkdocs.yml b/mkdocs.yml index 48b3d6b..6cf012d 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -73,10 +73,11 @@ nav: - Home: index.md - Examples: - examples/index.md + - Generating a Device from a Schema: examples/create_device/create_device.md - Getting Device Info: examples/get_info/get_info.md - Read and Write from Registers: examples/read_and_write_from_registers/read_and_write_from_registers.md - - Reading Data into a DataFrame: examples/read_data_to_dataframe/read_data_to_dataframe.md - Subscribing to Events: examples/subscribing_to_events/subscribing_to_events.md + - Reading Data into a DataFrame: examples/read_data_to_dataframe/read_data_to_dataframe.md - API: - Protocol: api/protocol.md - Serial: api/serial.md From a858c2c6f3ddb409ef8da59c11fca87603151993 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:27:40 -0700 Subject: [PATCH 09/22] Add device reader API --- src/packages/harp-data/pyproject.toml | 1 + .../harp-data/src/harp/data/__init__.py | 3 + .../harp-data/src/harp/data/_dataset.py | 175 +++++++++++++++++ tests/data/__init__.py | 0 tests/data/test_dataset.py | 177 ++++++++++++++++++ uv.lock | 2 + 6 files changed, 358 insertions(+) create mode 100644 src/packages/harp-data/src/harp/data/_dataset.py create mode 100644 tests/data/__init__.py create mode 100644 tests/data/test_dataset.py diff --git a/src/packages/harp-data/pyproject.toml b/src/packages/harp-data/pyproject.toml index 47bdfff..118792e 100644 --- a/src/packages/harp-data/pyproject.toml +++ b/src/packages/harp-data/pyproject.toml @@ -5,6 +5,7 @@ description = "Load Harp device data into pandas DataFrames" requires-python = ">=3.11" dependencies = [ "harp-protocol", + "harp-device", "numpy>=1.24", "pandas>=2.0", ] diff --git a/src/packages/harp-data/src/harp/data/__init__.py b/src/packages/harp-data/src/harp/data/__init__.py index 5aff91a..c0347ba 100644 --- a/src/packages/harp-data/src/harp/data/__init__.py +++ b/src/packages/harp-data/src/harp/data/__init__.py @@ -1,3 +1,4 @@ +from ._dataset import DatasetReader, default_file_resolver from ._reader import parse_to_dataframe, payload_to_dataframe from ._write import to_buffer, to_file @@ -6,4 +7,6 @@ "payload_to_dataframe", "to_buffer", "to_file", + "DatasetReader", + "default_file_resolver", ] diff --git a/src/packages/harp-data/src/harp/data/_dataset.py b/src/packages/harp-data/src/harp/data/_dataset.py new file mode 100644 index 0000000..3efe56e --- /dev/null +++ b/src/packages/harp-data/src/harp/data/_dataset.py @@ -0,0 +1,175 @@ +import re +from collections.abc import Callable, Mapping +from os import PathLike +from pathlib import Path +from typing import Any, Union + +import pandas as pd +from harp.device import Device +from harp.protocol import RegisterBase +from harp.protocol._constants import _TIMESTAMP_FLAG + +from ._reader import parse_to_dataframe + +RegisterKey = Union[type[RegisterBase[Any]], int] + +FileNameResolver = Callable[[Path, str], Mapping[int, list[Path]]] + + +def default_file_resolver(root: Path, name: str) -> dict[int, list[Path]]: + """Harp file format resolver: map address -> sorted ``_
...`` files.""" + pattern = re.compile(rf"^{re.escape(name)}_(\d+)(?:_.*)?$") + files: dict[int, list[Path]] = {} + for path in sorted(root.glob("*.bin")): + match = pattern.match(path.stem) + if match is not None: + files.setdefault(int(match.group(1)), []).append(path) + return files + + +class DatasetReader: + """Reader over a de-multiplexed Harp dataset folder. + + Construct from a generated device and a dataset folder, then read a register's + frames into a DataFrame by register class or by address:: + + reader = DatasetReader(Behavior, "session.harp") + df = reader.read(AnalogData) # by register class + df = reader.read(44) # by address + everything = reader.read_all() # {register_name: DataFrame} + + ``device`` is a generated :class:`~harp.device.Device` subclass; its + ``REGISTER_MAP`` and class name are read on demand. ``name`` overrides the + ```` file prefix, which defaults to the device class name. + + File resolution defaults to the Harp file format: ``_
.bin`` and, + when a register was logged as several ``_
_.bin`` chunks, + they are concatenated in filename order. Pass ``resolver`` (a :data:`FileResolver`) + to support an alternative on-disk layout. + """ + + def __init__( + self, + device: type[Device], + root: str | PathLike[str], + *, + name: str | None = None, + resolver: FileNameResolver = default_file_resolver, + ) -> None: + self._device = device + self._root = Path(root) + self._name_override = name + self._resolver = resolver + self._files = dict(self._resolver(self._root, self.name)) + + @property + def root(self) -> Path: + """The dataset folder being read.""" + return self._root + + @property + def device(self) -> type[Device]: + """The generated device this reader parses against.""" + return self._device + + @property + def name(self) -> str: + """The ```` prefix used to match binary files.""" + return self._name_override or self._device.__name__ + + @property + def registers(self) -> Mapping[int, type[RegisterBase[Any]]]: + """The device's address -> register-class map.""" + return self._device.REGISTER_MAP + + @property + def files(self) -> Mapping[int, list[Path]]: + """The discovered address -> binary file(s) present under :attr:`root`.""" + return self._files + + def read( + self, + register: RegisterKey, + *, + suffix: Union[str, None] = None, + timestamp: Union[bool, None] = None, + message_type: bool = False, + decode_enums: bool = True, + demux_bit_masks: bool = False, + ) -> pd.DataFrame: + """Read one register's data into a DataFrame. + + ``register`` is a register class or an address. ``suffix`` selects a single + ``_
_.bin`` chunk (default: concatenate every chunk + for the address). ``timestamp`` defaults to ``None`` — auto-detect from the + frame's payload-type bit; pass ``True``/``False`` to force. The remaining + options match :func:`~harp.data.parse_to_dataframe`. + """ + cls, address = self._resolve(register) + paths = self._resolve_files(address, suffix) + raw = b"".join(p.read_bytes() for p in paths) + ts = self._first_frame_timestamped(raw) if timestamp is None else timestamp + return parse_to_dataframe( + cls, + raw, + timestamp=ts, + message_type=message_type, + decode_enums=decode_enums, + demux_bit_masks=demux_bit_masks, + ) + + def read_all( + self, + *, + timestamp: Union[bool, None] = None, + message_type: bool = False, + decode_enums: bool = True, + demux_bit_masks: bool = False, + ) -> dict[str, pd.DataFrame]: + """Read every register that has a file present, keyed by register name. + + Files whose address is not in the device's registers are skipped. + Options are forwarded to :meth:`read`. + """ + registers = self.registers + out: dict[str, pd.DataFrame] = {} + for address in sorted(self._files): + cls = registers.get(address) + if cls is None: + continue + out[cls.__name__] = self.read( + address, + timestamp=timestamp, + message_type=message_type, + decode_enums=decode_enums, + demux_bit_masks=demux_bit_masks, + ) + return out + + def _resolve(self, register: RegisterKey) -> tuple[type[RegisterBase[Any]], int]: + if isinstance(register, type): + return register, register.address + cls = self.registers.get(register) + if cls is None: + raise KeyError(f"No register at address {register} in this device's map.") + return cls, register + + def _resolve_files(self, address: int, suffix: Union[str, None]) -> list[Path]: + paths = self._files.get(address) + if not paths: + raise FileNotFoundError( + f"No data file for register address {address} under {self._root} " + f"(expected '{self.name}_{address}[_].bin')." + ) + if suffix is not None: + paths = [p for p in paths if p.stem.endswith(f"_{suffix}")] + if not paths: + raise FileNotFoundError( + f"No '_{suffix}' chunk for register address {address} under {self._root}." + ) + return paths + + @staticmethod + def _first_frame_timestamped(raw: bytes) -> bool: + """Whether the first frame carries a timestamp (payload-type bit ``0x10``).""" + return len(raw) > 4 and bool(raw[4] & _TIMESTAMP_FLAG) diff --git a/tests/data/__init__.py b/tests/data/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/data/test_dataset.py b/tests/data/test_dataset.py new file mode 100644 index 0000000..85102c2 --- /dev/null +++ b/tests/data/test_dataset.py @@ -0,0 +1,177 @@ +import re + +import numpy as np +import pytest +from harp.data import DatasetReader, parse_to_dataframe +from harp.device import create_device + + +def _records(cls, n, seed): + dtype = cls.payload_class.dtype + rng = np.random.default_rng(seed) + raw = rng.integers(0, 128, size=n * dtype.itemsize, dtype=np.uint8) + return raw.view(dtype).copy() + + +@pytest.fixture +def emitted_device(device_yml): + # strict=False: the test device.yml uses a custom DataConverter we don't inject + # here; native decoding is enough to exercise file resolution and parsing. + return create_device(device_yml, strict=False) + + +@pytest.fixture +def dataset(emitted_device, tmp_path): + """A dataset folder with three app registers; the first is timestamped.""" + dev = emitted_device + name = dev.__name__ + addresses = [a for a in sorted(dev.REGISTER_MAP) if a >= 32][:3] + specs = {} + for i, address in enumerate(addresses): + cls = dev.REGISTER_MAP[address] + records = _records(cls, 5, seed=address) + timestamped = i == 0 + timestamps = np.arange(5, dtype=np.float64) if timestamped else None + buf = bytes(cls.format_bulk(records, timestamps=timestamps)) + (tmp_path / f"{name}_{address}.bin").write_bytes(buf) + specs[address] = (cls, timestamped, buf) + return dev, name, tmp_path, specs + + +def test_read_by_class_and_by_address(dataset): + dev, _name, root, specs = dataset + reader = DatasetReader(dev, root) + for address, (cls, timestamped, buf) in specs.items(): + expected = parse_to_dataframe(cls, buf, timestamp=timestamped) + assert reader.read(cls).equals(expected) + assert reader.read(address).equals(expected) + + +def test_timestamp_is_auto_detected(dataset): + dev, _name, root, specs = dataset + reader = DatasetReader(dev, root) + for address, (_cls, timestamped, _buf) in specs.items(): + df = reader.read(address) + assert ("timestamp" in df.columns) is timestamped + + +def test_read_all_keyed_by_register_name(dataset): + dev, _name, root, specs = dataset + reader = DatasetReader(dev, root) + frames = reader.read_all() + assert set(frames) == {cls.__name__ for cls, _ts, _buf in specs.values()} + for cls, _timestamped, _buf in specs.values(): + assert frames[cls.__name__].equals(reader.read(cls.address)) + + +def test_suffix_chunks_are_concatenated(emitted_device, tmp_path): + dev = emitted_device + name = dev.__name__ + address = next(a for a in sorted(dev.REGISTER_MAP) if a >= 32) + cls = dev.REGISTER_MAP[address] + chunk0 = bytes(cls.format_bulk(_records(cls, 3, seed=1))) + chunk1 = bytes(cls.format_bulk(_records(cls, 2, seed=2))) + (tmp_path / f"{name}_{address}_0.bin").write_bytes(chunk0) + (tmp_path / f"{name}_{address}_1.bin").write_bytes(chunk1) + + reader = DatasetReader(dev, tmp_path) + combined = parse_to_dataframe(cls, chunk0 + chunk1, timestamp=False) + assert reader.read(cls).reset_index(drop=True).equals(combined) + # A specific chunk can still be selected by suffix. + only0 = parse_to_dataframe(cls, chunk0, timestamp=False) + assert reader.read(cls, suffix="0").equals(only0) + + +def test_non_device_raises_on_register_access(dataset): + _dev, _name, root, _specs = dataset + # Registers are derived lazily; a non-Device fails when they are accessed. + reader = DatasetReader(object, root) + with pytest.raises(AttributeError, match="REGISTER_MAP"): + _ = reader.registers + + +def test_explicit_name_overrides(dataset): + dev, name, root, _specs = dataset + reader = DatasetReader(dev, root, name=name) + assert isinstance(reader, DatasetReader) + assert reader.name == name + + +def test_missing_register_file_raises(dataset): + dev, _name, root, _specs = dataset + reader = DatasetReader(dev, root) + # WhoAmI (address 0) is in the map but has no file in this dataset. + with pytest.raises(FileNotFoundError): + reader.read(0) + + +def test_unknown_address_raises(dataset): + dev, _name, root, _specs = dataset + reader = DatasetReader(dev, root) + with pytest.raises(KeyError): + reader.read(9999) + + +def test_custom_file_resolver_supports_alternative_layout(emitted_device, tmp_path): + dev = emitted_device + addresses = [a for a in sorted(dev.REGISTER_MAP) if a >= 32][:2] + expected = {} + for address in addresses: + cls = dev.REGISTER_MAP[address] + buf = bytes(cls.format_bulk(_records(cls, 3, seed=address))) + (tmp_path / f"reg{address}.bin").write_bytes(buf) # not the Harp layout + expected[cls.__name__] = parse_to_dataframe(cls, buf, timestamp=False) + + def resolver(root, _name): + found = {} + for path in sorted(root.glob("reg*.bin")): + match = re.match(r"^reg(\d+)$", path.stem) + if match is not None: + found.setdefault(int(match.group(1)), []).append(path) + return found + + reader = DatasetReader(dev, tmp_path, resolver=resolver) + assert set(reader.files) == set(addresses) + frames = reader.read_all() + assert set(frames) == set(expected) + for register_name, df in frames.items(): + assert df.equals(expected[register_name]) + + +def test_files_property_lists_discovered_bins(dataset): + dev, _name, root, specs = dataset + reader = DatasetReader(dev, root) + assert set(reader.files) == set(specs) + + +def test_read_all_registers_of_mock_device(emitted_device, tmp_path): + """Write one .bin per register of the device.yml device, then read them all back.""" + dev = emitted_device + name = dev.__name__ + expected = {} + for address, cls in dev.REGISTER_MAP.items(): + records = _records(cls, 4, seed=address) + # Alternate timestamped/untimestamped to exercise both parse paths. + timestamped = address % 2 == 0 + timestamps = np.arange(4, dtype=np.float64) if timestamped else None + buf = bytes(cls.format_bulk(records, timestamps=timestamps)) + (tmp_path / f"{name}_{address}.bin").write_bytes(buf) + expected[cls.__name__] = parse_to_dataframe(cls, buf, timestamp=timestamped) + + reader = DatasetReader(dev, tmp_path) + frames = reader.read_all() + + assert set(reader.files) == set(dev.REGISTER_MAP) + assert set(frames) == set(expected) + assert len(frames) == len(dev.REGISTER_MAP) + for register_name, df in frames.items(): + assert len(df) == 4 + assert df.equals(expected[register_name]) + + +def test_reader_derives_name_and_registers_from_device(dataset): + dev, name, root, _specs = dataset + reader = DatasetReader(dev, root) + assert reader.device is dev + assert reader.name == name + assert reader.registers == dev.REGISTER_MAP diff --git a/uv.lock b/uv.lock index 5a2cb84..bff724e 100644 --- a/uv.lock +++ b/uv.lock @@ -400,6 +400,7 @@ requires-dist = [ name = "harp-data" source = { editable = "src/packages/harp-data" } dependencies = [ + { name = "harp-device" }, { name = "harp-protocol" }, { name = "numpy", version = "2.4.6", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.12'" }, { name = "numpy", version = "2.5.1", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.12'" }, @@ -408,6 +409,7 @@ dependencies = [ [package.metadata] requires-dist = [ + { name = "harp-device", editable = "src/packages/harp-device" }, { name = "harp-protocol", editable = "src/packages/harp-protocol" }, { name = "numpy", specifier = ">=1.24" }, { name = "pandas", specifier = ">=2.0" }, From c3b7497b7233e0f1e911848a21263bdfbb95c34a Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:59:22 -0700 Subject: [PATCH 10/22] Implement time index parity with harp-python --- .../harp-data/src/harp/data/__init__.py | 3 +- .../harp-data/src/harp/data/_dataset.py | 22 +++++++---- .../harp-data/src/harp/data/_reader.py | 37 +++++++++++++++---- tests/data/test_dataset.py | 27 +++++++++++++- 4 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/packages/harp-data/src/harp/data/__init__.py b/src/packages/harp-data/src/harp/data/__init__.py index c0347ba..a992f20 100644 --- a/src/packages/harp-data/src/harp/data/__init__.py +++ b/src/packages/harp-data/src/harp/data/__init__.py @@ -1,5 +1,5 @@ from ._dataset import DatasetReader, default_file_resolver -from ._reader import parse_to_dataframe, payload_to_dataframe +from ._reader import REFERENCE_EPOCH, parse_to_dataframe, payload_to_dataframe from ._write import to_buffer, to_file __all__ = [ @@ -9,4 +9,5 @@ "to_file", "DatasetReader", "default_file_resolver", + "REFERENCE_EPOCH", ] diff --git a/src/packages/harp-data/src/harp/data/_dataset.py b/src/packages/harp-data/src/harp/data/_dataset.py index 3efe56e..aeeee94 100644 --- a/src/packages/harp-data/src/harp/data/_dataset.py +++ b/src/packages/harp-data/src/harp/data/_dataset.py @@ -1,8 +1,9 @@ import re from collections.abc import Callable, Mapping +from datetime import datetime from os import PathLike from pathlib import Path -from typing import Any, Union +from typing import Any import pandas as pd from harp.device import Device @@ -11,7 +12,7 @@ from ._reader import parse_to_dataframe -RegisterKey = Union[type[RegisterBase[Any]], int] +RegisterKey = type[RegisterBase[Any]] | int FileNameResolver = Callable[[Path, str], Mapping[int, list[Path]]] @@ -91,8 +92,9 @@ def read( self, register: RegisterKey, *, - suffix: Union[str, None] = None, - timestamp: Union[bool, None] = None, + suffix: str | None = None, + timestamp: bool | None = None, + epoch: datetime | None = None, message_type: bool = False, decode_enums: bool = True, demux_bit_masks: bool = False, @@ -102,8 +104,9 @@ def read( ``register`` is a register class or an address. ``suffix`` selects a single ``_
_.bin`` chunk (default: concatenate every chunk for the address). ``timestamp`` defaults to ``None`` — auto-detect from the - frame's payload-type bit; pass ``True``/``False`` to force. The remaining - options match :func:`~harp.data.parse_to_dataframe`. + frame's payload-type bit; pass ``True``/``False`` to force. ``epoch`` makes + the ``"Time"`` index absolute (e.g. :data:`~harp.data.REFERENCE_EPOCH`). The + remaining options match :func:`~harp.data.parse_to_dataframe`. """ cls, address = self._resolve(register) paths = self._resolve_files(address, suffix) @@ -113,6 +116,7 @@ def read( cls, raw, timestamp=ts, + epoch=epoch, message_type=message_type, decode_enums=decode_enums, demux_bit_masks=demux_bit_masks, @@ -121,7 +125,8 @@ def read( def read_all( self, *, - timestamp: Union[bool, None] = None, + timestamp: bool | None = None, + epoch: datetime | None = None, message_type: bool = False, decode_enums: bool = True, demux_bit_masks: bool = False, @@ -140,6 +145,7 @@ def read_all( out[cls.__name__] = self.read( address, timestamp=timestamp, + epoch=epoch, message_type=message_type, decode_enums=decode_enums, demux_bit_masks=demux_bit_masks, @@ -154,7 +160,7 @@ def _resolve(self, register: RegisterKey) -> tuple[type[RegisterBase[Any]], int] raise KeyError(f"No register at address {register} in this device's map.") return cls, register - def _resolve_files(self, address: int, suffix: Union[str, None]) -> list[Path]: + def _resolve_files(self, address: int, suffix: str | None) -> list[Path]: paths = self._files.get(address) if not paths: raise FileNotFoundError( diff --git a/src/packages/harp-data/src/harp/data/_reader.py b/src/packages/harp-data/src/harp/data/_reader.py index 8ff9ae3..f596a7b 100644 --- a/src/packages/harp-data/src/harp/data/_reader.py +++ b/src/packages/harp-data/src/harp/data/_reader.py @@ -1,16 +1,32 @@ """Load Harp register data into pandas DataFrames.""" +from datetime import datetime from pathlib import Path from typing import Any, BinaryIO, Union import numpy as np import pandas as pd from harp.protocol import RegisterBase +from numpy.typing import NDArray Source = Union[str, Path, bytes, bytearray, memoryview, BinaryIO] _MSG_NAMES = np.array(["_NONE", "Read", "Write", "Event"]) +#: Harp reference epoch — time zero of the Harp clock (UTC). +REFERENCE_EPOCH = datetime(1904, 1, 1) + +_TIME_INDEX_NAME = "Time" + + +def _time_index(seconds: NDArray[np.float64], epoch: datetime | None) -> pd.Index: + """The Harp time axis: float seconds, or absolute datetime when ``epoch`` is set.""" + if epoch is None: + return pd.Index(seconds, name=_TIME_INDEX_NAME) + return pd.DatetimeIndex( + pd.Timestamp(epoch) + pd.to_timedelta(seconds, unit="s"), name=_TIME_INDEX_NAME + ) + def _read_bytes(source: Source) -> bytes: if isinstance(source, (bytes, bytearray, memoryview)): @@ -53,17 +69,21 @@ def parse_to_dataframe( source: Source, *, timestamp: bool = True, + epoch: Union[datetime, None] = None, message_type: bool = False, decode_enums: bool = True, demux_bit_masks: bool = False, ) -> pd.DataFrame: """Parse all frames of ``register`` from ``source`` into a DataFrame. - ``source`` may be a file path, raw bytes, or an open binary file object. - ``timestamp`` and ``message_type`` insert leading columns; ``decode_enums`` - controls whether enum fields become ``pd.Categorical`` (True) or raw codes; - ``demux_bit_masks`` expands each flag (``BitMask``) field into one boolean - column per flag member (True) or keeps it as a single raw-integer column. + ``source`` may be a file path, raw bytes, or an open binary file object. When + ``timestamp`` is set, the Harp time becomes the DataFrame index (named + ``"Time"``): float seconds by default, or an absolute ``DatetimeIndex`` when + ``epoch`` is given (e.g. :data:`REFERENCE_EPOCH`). ``message_type`` inserts a + leading column; ``decode_enums`` controls whether enum fields become + ``pd.Categorical`` (True) or raw codes; ``demux_bit_masks`` expands each flag + (``BitMask``) field into one boolean column per flag member (True) or keeps it + as a single raw-integer column. """ raw = _read_bytes(source) _data, timestamps, msg_view, payload = register.parse_bulk(raw, parse_timestamp=timestamp) @@ -80,9 +100,10 @@ def parse_to_dataframe( if len(df) > 0: raise ValueError( "Buffer contains no timestamp data; pass timestamp=False to suppress " - "the timestamp column." + "the time index." ) - # Empty buffer: no frames to timestamp — return the empty frame as-is. + seconds = np.empty(0, dtype=np.float64) # empty buffer: empty Time index else: - df.insert(0, "timestamp", timestamps) + seconds = np.asarray(timestamps, dtype=np.float64) + df.index = _time_index(seconds, epoch) return df diff --git a/tests/data/test_dataset.py b/tests/data/test_dataset.py index 85102c2..11b8d92 100644 --- a/tests/data/test_dataset.py +++ b/tests/data/test_dataset.py @@ -1,8 +1,9 @@ import re import numpy as np +import pandas as pd import pytest -from harp.data import DatasetReader, parse_to_dataframe +from harp.data import REFERENCE_EPOCH, DatasetReader, parse_to_dataframe from harp.device import create_device @@ -52,7 +53,29 @@ def test_timestamp_is_auto_detected(dataset): reader = DatasetReader(dev, root) for address, (_cls, timestamped, _buf) in specs.items(): df = reader.read(address) - assert ("timestamp" in df.columns) is timestamped + # Timestamped frames get a "Time" index; untimestamped keep a plain RangeIndex. + assert (df.index.name == "Time") is timestamped + + +def test_time_index_is_float_seconds_without_epoch(dataset): + dev, _name, root, specs = dataset + reader = DatasetReader(dev, root) + address = next(a for a, (_c, ts, _b) in specs.items() if ts) # the timestamped register + df = reader.read(address) + assert df.index.name == "Time" + assert list(df.index) == [0.0, 1.0, 2.0, 3.0, 4.0] # arange(5) seconds from the fixture + + +def test_epoch_gives_absolute_datetime_index(dataset): + dev, _name, root, specs = dataset + reader = DatasetReader(dev, root) + address = next(a for a, (_c, ts, _b) in specs.items() if ts) + df = reader.read(address, epoch=REFERENCE_EPOCH) + assert isinstance(df.index, pd.DatetimeIndex) + assert df.index.name == "Time" + # Harp seconds are measured from the reference epoch (timestamps were arange(5)). + assert df.index[0] == pd.Timestamp(REFERENCE_EPOCH) + assert df.index[2] == pd.Timestamp(REFERENCE_EPOCH) + pd.Timedelta(seconds=2) def test_read_all_keyed_by_register_name(dataset): From f9034a5695eb8188347d134cf00afb47a6516502 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 09:38:16 -0700 Subject: [PATCH 11/22] Add syntactic sugar for dataset creation --- .../harp-data/src/harp/data/__init__.py | 3 +- .../harp-data/src/harp/data/_dataset.py | 40 ++++++++++++++++++- tests/data/test_dataset.py | 34 +++++++++++++++- 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/src/packages/harp-data/src/harp/data/__init__.py b/src/packages/harp-data/src/harp/data/__init__.py index a992f20..475625b 100644 --- a/src/packages/harp-data/src/harp/data/__init__.py +++ b/src/packages/harp-data/src/harp/data/__init__.py @@ -1,4 +1,4 @@ -from ._dataset import DatasetReader, default_file_resolver +from ._dataset import DatasetReader, create_dataset_reader, default_file_resolver from ._reader import REFERENCE_EPOCH, parse_to_dataframe, payload_to_dataframe from ._write import to_buffer, to_file @@ -8,6 +8,7 @@ "to_buffer", "to_file", "DatasetReader", + "create_dataset_reader", "default_file_resolver", "REFERENCE_EPOCH", ] diff --git a/src/packages/harp-data/src/harp/data/_dataset.py b/src/packages/harp-data/src/harp/data/_dataset.py index aeeee94..bad29a6 100644 --- a/src/packages/harp-data/src/harp/data/_dataset.py +++ b/src/packages/harp-data/src/harp/data/_dataset.py @@ -6,7 +6,7 @@ from typing import Any import pandas as pd -from harp.device import Device +from harp.device import Device, create_device from harp.protocol import RegisterBase from harp.protocol._constants import _TIMESTAMP_FLAG @@ -16,6 +16,9 @@ FileNameResolver = Callable[[Path, str], Mapping[int, list[Path]]] +#: Default filename of the device schema looked up inside a dataset folder. +DEVICE_SCHEMA_FILENAME = "device.yml" + def default_file_resolver(root: Path, name: str) -> dict[int, list[Path]]: """Harp file format resolver: map address -> sorted ``_
...`` files.""" @@ -179,3 +182,38 @@ def _resolve_files(self, address: int, suffix: str | None) -> list[Path]: def _first_frame_timestamped(raw: bytes) -> bool: """Whether the first frame carries a timestamp (payload-type bit ``0x10``).""" return len(raw) > 4 and bool(raw[4] & _TIMESTAMP_FLAG) + + +def create_dataset_reader( + root: str | PathLike[str], + *, + schema: str | PathLike[str] | None = None, + name: str | None = None, + resolver: FileNameResolver = default_file_resolver, + converters: Mapping[str, Any] | None = None, + strict: bool = True, +) -> DatasetReader: + """Build a :class:`DatasetReader` for a dataset folder, device and all. + + Convenience wrapper that finds the device schema inside ``root`` (``device.yml`` + by default), generates a device from it with :func:`~harp.device.create_device`, + and returns a reader ready to :meth:`~DatasetReader.read`:: + + reader = create_dataset_reader("session.harp") + df = reader.read(44) + + ``schema`` points at the schema file explicitly when it isn't ``root/device.yml``. + ``converters`` and ``strict`` are forwarded to :func:`~harp.device.create_device` + for custom ``interfaceType`` decoding; ``name`` and ``resolver`` are forwarded to + :class:`DatasetReader`. Use ``DatasetReader(device, root)`` directly when you + already have a (e.g. pre-generated) device class. + """ + root_path = Path(root) + schema_path = Path(schema) if schema is not None else root_path / DEVICE_SCHEMA_FILENAME + if not schema_path.is_file(): + raise FileNotFoundError( + f"No device schema at '{schema_path}'. Pass schema= to point at a device.yml, " + f"or build the device yourself and use DatasetReader(device, root)." + ) + device = create_device(schema_path.read_text(), converters=converters, strict=strict) + return DatasetReader(device, root_path, name=name, resolver=resolver) diff --git a/tests/data/test_dataset.py b/tests/data/test_dataset.py index 11b8d92..581f415 100644 --- a/tests/data/test_dataset.py +++ b/tests/data/test_dataset.py @@ -3,7 +3,12 @@ import numpy as np import pandas as pd import pytest -from harp.data import REFERENCE_EPOCH, DatasetReader, parse_to_dataframe +from harp.data import ( + REFERENCE_EPOCH, + DatasetReader, + create_dataset_reader, + parse_to_dataframe, +) from harp.device import create_device @@ -198,3 +203,30 @@ def test_reader_derives_name_and_registers_from_device(dataset): assert reader.device is dev assert reader.name == name assert reader.registers == dev.REGISTER_MAP + + +def test_create_dataset_reader_builds_device_from_device_yml(dataset, device_yml): + dev, _name, root, specs = dataset + (root / "device.yml").write_text(device_yml) + # strict=False mirrors the emitted_device fixture (custom DataConverter not injected). + reader = create_dataset_reader(root, strict=False) + assert isinstance(reader, DatasetReader) + # Reads match a reader built from an explicitly-generated device. + reference = DatasetReader(dev, root) + for address, (cls, _timestamped, _buf) in specs.items(): + assert reader.read(address).equals(reference.read(cls)) + + +def test_create_dataset_reader_accepts_explicit_schema_path(dataset, device_yml, tmp_path): + _dev, _name, root, specs = dataset + schema_path = tmp_path / "elsewhere.yml" # not inside the dataset folder + schema_path.write_text(device_yml) + reader = create_dataset_reader(root, schema=schema_path, strict=False) + address = next(iter(specs)) + assert not reader.read(address).empty + + +def test_create_dataset_reader_missing_schema_raises(dataset): + _dev, _name, root, _specs = dataset # no device.yml written into the folder + with pytest.raises(FileNotFoundError, match="device.yml"): + create_dataset_reader(root) From 72522aeaa6883bc768c4a17e479e41c419c70a1e Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 09:38:30 -0700 Subject: [PATCH 12/22] Add documentation for dataset api --- README.md | 48 +++++++------ docs/api/data.md | 6 ++ docs/examples/index.md | 3 +- .../read_data_to_dataframe.md | 12 +++- .../read_data_to_dataframe.py | 15 +++- docs/examples/read_dataset/read_dataset.md | 25 +++++++ docs/examples/read_dataset/read_dataset.py | 46 ++++++++++++ mkdocs.yml | 1 + src/packages/harp-data/README.md | 70 ++++++++++++++++++- 9 files changed, 196 insertions(+), 30 deletions(-) create mode 100644 docs/examples/read_dataset/read_dataset.md create mode 100644 docs/examples/read_dataset/read_dataset.py diff --git a/README.md b/README.md index 89199f6..7ad3352 100644 --- a/README.md +++ b/README.md @@ -52,42 +52,48 @@ pip install harp-data ## Quickstart -Have only a device's `device.yml`? `create_device` compiles it into a typed -`Device` at runtime — no code-generation step — giving you the device's registers -(keyed by address) and its identity: +There are two ways you'll typically use `harp`: talking to a **live device** over a +serial connection, or reading **data recorded to disk**. + +**Talk to a live device.** Open a connection and read/write registers by class: ```python -from pathlib import Path -from harp.device import create_device +from harp.device import Device, WhoAmI, OperationControl, OperationControlPayload, OperationMode +from harp.serial import open_serial_device -Behavior = create_device(Path("device.yml").read_text()) -Behavior.__whoami__ # device identity from the schema -AnalogData = Behavior.REGISTER_MAP[44] # registers are reached by address +# Use "COMx" on Windows, "/dev/ttyUSBx" on Linux. +with open_serial_device(Device, port="/dev/ttyUSB0") as device: + print("WhoAmI:", device.read(WhoAmI).parsed) + device.write(OperationControl, OperationControlPayload(operation_mode=OperationMode.ACTIVE)) ``` -The generated device works like any other. **Talk to hardware** over a serial -transport — `read`/`write` take a register class: +**Read a recorded session.** Point a `DatasetReader` at a dataset folder and read +registers into pandas DataFrames — no hardware required: ```python -from harp.serial import open_serial_device +from harp.data import create_dataset_reader -# Use "COMx" on Windows, "/dev/ttyUSBx" on Linux. -with open_serial_device(Behavior, port="/dev/ttyUSB0") as device: - print(device.read(AnalogData).parsed) +# Finds device.yml in the folder, builds the device, returns a ready-to-use reader. +reader = create_dataset_reader("session.harp") +df = reader.read(44) # one register, by address (or pass its class) +everything = reader.read_all() # {register_name: DataFrame} ``` -...or use the same register classes to **decode recorded data** into a pandas -DataFrame: +Both paths are driven by a device schema. If you have only a `device.yml` and no +pre-generated package, `create_device` compiles it into a typed `Device` at runtime — +no code-generation step — which is exactly what `create_dataset_reader` does under +the hood: ```python -from harp.data import parse_to_dataframe +from pathlib import Path +from harp.device import create_device -df = parse_to_dataframe(AnalogData, "Behavior_44.bin") +Behavior = create_device(Path("device.yml").read_text()) +AnalogData = Behavior.REGISTER_MAP[44] # registers are reached by address ``` -See the [Examples](https://harp-tech.org/pyharp/examples/) for full walkthroughs, -including reading device info, subscribing to events, and working with custom -interface-type converters. +See the [Examples](https://harp-tech.org/pyharp/examples/) for the full walkthroughs, +including subscribing to device events and working with custom interface-type converters. ## Contributing diff --git a/docs/api/data.md b/docs/api/data.md index b79926a..c66a43f 100644 --- a/docs/api/data.md +++ b/docs/api/data.md @@ -2,5 +2,11 @@ --- +::: harp.data.create_dataset_reader +::: harp.data.DatasetReader +::: harp.data.default_file_resolver ::: harp.data.parse_to_dataframe ::: harp.data.payload_to_dataframe +::: harp.data.to_file +::: harp.data.to_buffer +::: harp.data.REFERENCE_EPOCH diff --git a/docs/examples/index.md b/docs/examples/index.md index 492d90a..ff83850 100644 --- a/docs/examples/index.md +++ b/docs/examples/index.md @@ -14,4 +14,5 @@ Talking to a device: Reading recorded data: -- [Reading Data into a DataFrame](./read_data_to_dataframe/read_data_to_dataframe.md) - load a register's binary data file into a pandas DataFrame with `harp.data`. +- [Reading a Whole Dataset Folder](./read_dataset/read_dataset.md) - load an entire recorded session folder into pandas DataFrames with `DatasetReader`. +- [Reading Data into a DataFrame](./read_data_to_dataframe/read_data_to_dataframe.md) - decode a single register's binary file into a pandas DataFrame. diff --git a/docs/examples/read_data_to_dataframe/read_data_to_dataframe.md b/docs/examples/read_data_to_dataframe/read_data_to_dataframe.md index a205b79..d0637b4 100644 --- a/docs/examples/read_data_to_dataframe/read_data_to_dataframe.md +++ b/docs/examples/read_data_to_dataframe/read_data_to_dataframe.md @@ -1,8 +1,14 @@ # Reading Data into a DataFrame -This example demonstrates how to load a Harp register's binary data file into a -pandas DataFrame using `harp.data`. The register definition tells `parse_to_dataframe` -how to decode each frame, so you get named columns (and decoded enums) for free. +This example demonstrates how to load a **single** Harp register's binary data +file into a pandas DataFrame using `harp.data`. The register definition tells +`parse_to_dataframe` how to decode each frame, so you get named columns (and +decoded enums) for free. + +!!! tip + Have a whole recorded session folder rather than one loose file? Use + [`DatasetReader`](../read_dataset/read_dataset.md), which reads every register + in a dataset folder driven by the device schema. ```python diff --git a/docs/examples/read_data_to_dataframe/read_data_to_dataframe.py b/docs/examples/read_data_to_dataframe/read_data_to_dataframe.py index fda5a2d..df67c2c 100644 --- a/docs/examples/read_data_to_dataframe/read_data_to_dataframe.py +++ b/docs/examples/read_data_to_dataframe/read_data_to_dataframe.py @@ -1,11 +1,20 @@ from harp.data import parse_to_dataframe from harp.device import OperationControl -# Parse a register's binary dump into a pandas DataFrame — one row per frame, -# one column per field, plus a leading "timestamp" column. -df = parse_to_dataframe(OperationControl, "OperationControl.bin", timestamp=True) +# Parse a single register's binary dump into a pandas DataFrame — one row per +# frame, one column per field. The register class tells `parse_to_dataframe` how +# to decode each frame, so you get named columns (and decoded enums) for free. +df = parse_to_dataframe(OperationControl, "OperationControl.bin") print(df.head()) +# When the frames are timestamped (the default), the Harp time becomes the +# DataFrame index, named "Time" — float seconds from device start. +print(df.index.name, df.index[:3].to_list()) + # `parse_to_dataframe` also accepts raw bytes or an open binary file object: with open("OperationControl.bin", "rb") as f: df = parse_to_dataframe(OperationControl, f) + +# To read a whole recorded session folder at once (many registers, driven by the +# device schema) use `harp.data.DatasetReader` — see the "Reading a Whole Dataset +# Folder" example. diff --git a/docs/examples/read_dataset/read_dataset.md b/docs/examples/read_dataset/read_dataset.md new file mode 100644 index 0000000..2cd4ea6 --- /dev/null +++ b/docs/examples/read_dataset/read_dataset.md @@ -0,0 +1,25 @@ +# Reading a Whole Dataset Folder + +A Harp acquisition is usually saved as a **de-multiplexed dataset folder**: one +binary file per register, named `_
.bin`, next to the device's +`device.yml` schema. `harp.data.DatasetReader` reads that whole folder into pandas +DataFrames, driven by a [generated device](../../api/device.md) that describes how +to decode each register. + +This is the recommended entry point when you have a recorded session on disk. To +decode a single loose `.bin` file instead, see +[Reading Data into a DataFrame](../read_data_to_dataframe/read_data_to_dataframe.md). + +The quickest way in is `create_dataset_reader(folder)`: it finds the `device.yml` +inside the folder, builds the device for you, and returns a reader ready to go. +(If you already have a device class — e.g. from a pre-generated package — construct +`DatasetReader(Device, folder)` directly instead.) You then read a register by +class or by address, or read every register at once with `read_all()`. Timestamps +are detected automatically and placed on the `"Time"` index (float seconds, or an +absolute `DatetimeIndex` when you pass an `epoch`). + + +```python +[](./read_dataset.py) +``` + diff --git a/docs/examples/read_dataset/read_dataset.py b/docs/examples/read_dataset/read_dataset.py new file mode 100644 index 0000000..1e9b834 --- /dev/null +++ b/docs/examples/read_dataset/read_dataset.py @@ -0,0 +1,46 @@ +from harp.data import REFERENCE_EPOCH, create_dataset_reader +from harp.device import OperationControl + +# A Harp acquisition is usually saved as a de-multiplexed dataset folder — one +# `.bin` file per register, named "_
.bin", next to the +# device's `device.yml` schema: +# +# 📦 session.harp +# ┣ 📜 Behavior_0.bin +# ┣ 📜 Behavior_44.bin +# ┣ ... +# ┗ 📜 device.yml +# +# `create_dataset_reader` does the right thing: it finds `device.yml` inside the +# folder, builds the device that knows how to decode each register, and hands back +# a reader ready to go. +reader = create_dataset_reader("session.harp") + +# Read one register into a DataFrame — by register class (any register in the +# device's map, including the common ones like `OperationControl`)... +df = reader.read(OperationControl) + +# ...or by address. Timestamps are auto-detected from the frames, and when present +# they become the DataFrame index, named "Time" (float seconds from device start). +df = reader.read(44) +print(df.head()) + +# Read every register that has a file on disk at once, keyed by register name. +everything = reader.read_all() +print(list(everything)) + +# Pass an epoch to turn the "Time" index into an absolute `DatetimeIndex` instead +# of float seconds. `REFERENCE_EPOCH` is time zero of the Harp clock (UTC). +absolute = reader.read(44, epoch=REFERENCE_EPOCH) +print(absolute.index[:3]) + +# --- Already have a device class? ------------------------------------------- +# A pre-generated device package, or one you built yourself with `create_device`, +# can drive the reader directly — construct `DatasetReader(Device, folder)`: +# +# from harp.data import DatasetReader +# from harp.device import create_device +# from pathlib import Path +# +# Behavior = create_device((Path("session.harp") / "device.yml").read_text()) +# reader = DatasetReader(Behavior, "session.harp") diff --git a/mkdocs.yml b/mkdocs.yml index 6cf012d..c4a63b4 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -77,6 +77,7 @@ nav: - Getting Device Info: examples/get_info/get_info.md - Read and Write from Registers: examples/read_and_write_from_registers/read_and_write_from_registers.md - Subscribing to Events: examples/subscribing_to_events/subscribing_to_events.md + - Reading a Whole Dataset Folder: examples/read_dataset/read_dataset.md - Reading Data into a DataFrame: examples/read_data_to_dataframe/read_data_to_dataframe.md - API: - Protocol: api/protocol.md diff --git a/src/packages/harp-data/README.md b/src/packages/harp-data/README.md index abfdbde..80045b8 100644 --- a/src/packages/harp-data/README.md +++ b/src/packages/harp-data/README.md @@ -4,7 +4,59 @@ Load Harp register data into pandas DataFrames. This is the package that pulls in `pandas` — [`harp-protocol`](../harp-protocol) stays numpy-only and exposes a pandas-free `ColumnData` view that this package assembles into a DataFrame. -## Read a register from a file +There are two ways in, depending on what you have on disk: + +- a whole **dataset folder** (many registers) → `DatasetReader` +- a single **register file** or buffer → `parse_to_dataframe` + +## Read a whole dataset folder + +A Harp acquisition is usually saved as a de-multiplexed folder — one binary file +per register, named `_
.bin`, alongside the device's +`device.yml` schema: + +```text +📦 session.harp + ┣ 📜 Behavior_0.bin + ┣ 📜 Behavior_44.bin + ┣ ... + ┗ 📜 device.yml +``` + +Reading is driven by a generated +[`harp.device.Device`](../harp-device) that describes how to decode each register. +`create_dataset_reader` does that for you — it finds the `device.yml` in the folder, +builds the device, and returns a ready-to-use reader: + +```python +from harp.data import create_dataset_reader + +reader = create_dataset_reader("session.harp") +df = reader.read(AnalogData) # by register class +df = reader.read(44) # by address +everything = reader.read_all() # {register_name: DataFrame} +``` + +Already have a device class (e.g. a pre-generated package, or one built with +`create_device`)? Drive `DatasetReader` with it directly: + +```python +from pathlib import Path +from harp.data import DatasetReader +from harp.device import create_device + +Behavior = create_device((Path("session.harp") / "device.yml").read_text()) +reader = DatasetReader(Behavior, "session.harp") +``` + +Timestamps are auto-detected per register and placed on the DataFrame index +(named `"Time"`): float seconds by default, or an absolute `DatetimeIndex` when +you pass `epoch=REFERENCE_EPOCH`. Multi-chunk registers logged as +`_
_.bin` are concatenated in filename order; pass a +`resolver` to support an alternative on-disk layout, or `name=` to override the +file prefix. + +## Read a single register file `parse_to_dataframe` takes a register and a source (path, bytes, or open binary file) and returns one row per frame: @@ -17,7 +69,10 @@ df = parse_to_dataframe(AnalogData, "AnalogData.bin") df = parse_to_dataframe(AnalogData, raw, timestamp=True, message_type=False, decode_enums=True) ``` -Enum fields decode to `pd.Categorical` (`decode_enums=False` keeps raw codes). +With `timestamp=True` (the default) the Harp time becomes the DataFrame index, +named `"Time"` — float seconds, or an absolute `DatetimeIndex` when you also pass +`epoch=REFERENCE_EPOCH`. Enum fields decode to `pd.Categorical` +(`decode_enums=False` keeps raw codes). ## From an already-parsed payload @@ -30,3 +85,14 @@ from harp.data import payload_to_dataframe _data, timestamps, _msg, payload = AnalogData.parse_bulk(raw) df = payload_to_dataframe(payload) ``` + +## Write data back out + +`to_file` / `to_buffer` are the inverse of the readers — encode values as Harp +frames. Handy for round-tripping data or generating test corpora: + +```python +from harp.data import to_file + +to_file(AnalogData, values, "AnalogData.bin", timestamps=seconds) +``` From eab57f3ce45f273aa33f140b194b4e3bd4a732c8 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:59:04 -0700 Subject: [PATCH 13/22] Implement register binding at the level of the device api --- README.md | 2 +- docs/api/device.md | 3 +- docs/examples/create_device/create_device.md | 12 ++- docs/examples/create_device/create_device.py | 6 +- docs/examples/get_info/get_info.py | 7 +- .../subscribing_to_events.py | 3 +- .../harp-data/src/harp/data/_dataset.py | 17 ++-- src/packages/harp-device/README.md | 23 +++-- .../harp-device/src/harp/device/__init__.py | 7 +- .../src/harp/device/_core_registers.py | 82 ++++++++++++++++ .../harp-device/src/harp/device/_device.py | 43 +++++++-- .../src/harp/device/_emit_device.py | 17 ++-- .../src/harp/device/_register_map.py | 48 ---------- .../src/harp/device/_register_namespace.py | 94 +++++++++++++++++++ tests/data/test_dataset.py | 22 ++--- tests/device/expected_core.py | 34 +++---- tests/device/expected_device.py | 65 +++++++++---- tests/device/test_device_emit.py | 36 ++++--- tests/device/test_emit.py | 9 +- 19 files changed, 373 insertions(+), 157 deletions(-) create mode 100644 src/packages/harp-device/src/harp/device/_core_registers.py delete mode 100644 src/packages/harp-device/src/harp/device/_register_map.py create mode 100644 src/packages/harp-device/src/harp/device/_register_namespace.py diff --git a/README.md b/README.md index 7ad3352..23e6d5c 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ from pathlib import Path from harp.device import create_device Behavior = create_device(Path("device.yml").read_text()) -AnalogData = Behavior.REGISTER_MAP[44] # registers are reached by address +AnalogData = Behavior.registers.AnalogData # registers are reached by name ``` See the [Examples](https://harp-tech.org/pyharp/examples/) for the full walkthroughs, diff --git a/docs/api/device.md b/docs/api/device.md index 652e4c7..4f706d5 100644 --- a/docs/api/device.md +++ b/docs/api/device.md @@ -6,10 +6,11 @@ ::: harp.device.create_device ::: harp.device.parse_device_schema ::: harp.device.ConverterContext +::: harp.device.RegisterNamespace +::: harp.device.CoreRegisters ::: harp.device.HarpFramer ::: harp.device.ITransport ::: harp.device.TransportError -::: harp.device.REGISTER_MAP ::: harp.device.OperationControl ::: harp.device.OperationMode ::: harp.device.ResetDevice diff --git a/docs/examples/create_device/create_device.md b/docs/examples/create_device/create_device.md index 15907d9..9a8686a 100644 --- a/docs/examples/create_device/create_device.md +++ b/docs/examples/create_device/create_device.md @@ -5,7 +5,8 @@ This example demonstrates how to turn a Harp `device.yml` into a typed code-generation step. This is the quickest way to get started when you have only a device's schema and no pre-generated package for it. -The compiled device exposes its registers through `REGISTER_MAP` (keyed by address) +The compiled device exposes its registers by name through `device.registers` +(e.g. `Behavior.registers.AnalogData`, or by address with `Behavior.registers[44]`) and carries the device's `__whoami__` identity. From there it works exactly like a pre-generated device class — drive it over a transport to talk to hardware, or use its register classes to decode recorded data. @@ -27,9 +28,12 @@ convenience. It's worth understanding what that buys you and what it costs. **You give up:** -- **Named, typed access.** Registers are reached by address (`REGISTER_MAP[44]`), - not as importable, autocompleting classes (`from harp_behavior import AnalogData`). - You lose editor discovery and static type checking of register names. +- **Static register types.** A runtime device still exposes its registers by name + (`device.registers.AnalogData`), but because the class is built at runtime the + editor can't autocomplete those names or check them — you get a generic + `type[RegisterBase]`, not the specific register type. A statically generated device + declares its registers, so `device.registers.AnalogData` autocompletes and + `read`/`write` infer the payload type. - **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 diff --git a/docs/examples/create_device/create_device.py b/docs/examples/create_device/create_device.py index 11ec1b8..08cb734 100644 --- a/docs/examples/create_device/create_device.py +++ b/docs/examples/create_device/create_device.py @@ -13,7 +13,11 @@ Behavior = create_device(Path("device.yml").read_text()) print("WhoAmI:", Behavior.__whoami__) # device identity, taken from the schema -AnalogData = Behavior.REGISTER_MAP[44] # registers are reached by address + +# Registers are reached by name through `.registers` — the common Harp registers +# (like WhoAmI) plus the device's own. Address lookup still works via +# `Behavior.registers[44]` or `Behavior.registers.by_address`. +AnalogData = Behavior.registers.AnalogData # The generated device behaves like any other `Device` class. Talk to hardware over # a transport — `read`/`write` take a register class: diff --git a/docs/examples/get_info/get_info.py b/docs/examples/get_info/get_info.py index cc0c350..a2cab41 100755 --- a/docs/examples/get_info/get_info.py +++ b/docs/examples/get_info/get_info.py @@ -1,4 +1,4 @@ -from harp.device import REGISTER_MAP, Device, WhoAmI +from harp.device import Device, WhoAmI from harp.serial import open_serial_device SERIAL_PORT = "/dev/ttyUSB0" # or "COMx" in Windows ("x" is the number of the serial port) @@ -8,7 +8,8 @@ # Identify the device. print("WhoAmI:", device.read(WhoAmI).parsed) - # Dump every core register. - for address, register in sorted(REGISTER_MAP.items()): + # Dump every register the device exposes. `device.registers` is name-addressable + # (device.registers.WhoAmI); `.by_address` gives the address -> register map. + for address, register in sorted(device.registers.by_address.items()): reply = device.read(register) print(f"{register.__name__:24s} (addr {address:2d}) = {reply.parsed}") diff --git a/docs/examples/subscribing_to_events/subscribing_to_events.py b/docs/examples/subscribing_to_events/subscribing_to_events.py index c649514..34a1e03 100644 --- a/docs/examples/subscribing_to_events/subscribing_to_events.py +++ b/docs/examples/subscribing_to_events/subscribing_to_events.py @@ -1,5 +1,4 @@ from harp.device import ( - REGISTER_MAP, Device, OperationControl, OperationControlPayload, @@ -17,7 +16,7 @@ def print_timestamp(msg: ParsedHarpMessage[float]) -> None: def print_any_event(msg: HarpMessage) -> None: - register = REGISTER_MAP.get(msg.address, None) + register = Device.registers.by_address.get(msg.address) value = register.parse(msg) if register is not None else msg.payload.hex() print(f"[{msg.address}] {msg.timestamp:.6f} {msg.message_type.name:<5s} {value}") diff --git a/src/packages/harp-data/src/harp/data/_dataset.py b/src/packages/harp-data/src/harp/data/_dataset.py index bad29a6..0696791 100644 --- a/src/packages/harp-data/src/harp/data/_dataset.py +++ b/src/packages/harp-data/src/harp/data/_dataset.py @@ -6,7 +6,7 @@ from typing import Any import pandas as pd -from harp.device import Device, create_device +from harp.device import Device, RegisterNamespace, create_device from harp.protocol import RegisterBase from harp.protocol._constants import _TIMESTAMP_FLAG @@ -43,7 +43,7 @@ class DatasetReader: everything = reader.read_all() # {register_name: DataFrame} ``device`` is a generated :class:`~harp.device.Device` subclass; its - ``REGISTER_MAP`` and class name are read on demand. ``name`` overrides the + ``registers`` and class name are read on demand. ``name`` overrides the ```` file prefix, which defaults to the device class name. File resolution defaults to the Harp file format: ``_
.bin`` and, @@ -82,9 +82,10 @@ def name(self) -> str: return self._name_override or self._device.__name__ @property - def registers(self) -> Mapping[int, type[RegisterBase[Any]]]: - """The device's address -> register-class map.""" - return self._device.REGISTER_MAP + def registers(self) -> RegisterNamespace: + """The device's registers, reachable by name (``reader.registers.WhoAmI``) + or address (``reader.registers[44]`` / ``reader.registers.by_address``).""" + return self._device.registers @property def files(self) -> Mapping[int, list[Path]]: @@ -139,10 +140,10 @@ def read_all( Files whose address is not in the device's registers are skipped. Options are forwarded to :meth:`read`. """ - registers = self.registers + by_address = self.registers.by_address out: dict[str, pd.DataFrame] = {} for address in sorted(self._files): - cls = registers.get(address) + cls = by_address.get(address) if cls is None: continue out[cls.__name__] = self.read( @@ -158,7 +159,7 @@ def read_all( def _resolve(self, register: RegisterKey) -> tuple[type[RegisterBase[Any]], int]: if isinstance(register, type): return register, register.address - cls = self.registers.get(register) + cls = self.registers.by_address.get(register) if cls is None: raise KeyError(f"No register at address {register} in this device's map.") return cls, register diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index 3221dd8..66dd5d8 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -19,33 +19,40 @@ device.write(OperationControl, payload) # write a register ## Extending for a specific device -Downstream (often generated) packages add their registers and spread the core -`REGISTER_MAP`, and may set `__whoami__` for identity validation on connect: +Downstream (often generated) packages declare their registers in a `__REGISTERS__` +tuple; the common Harp registers are merged in automatically. They may set +`__whoami__` for identity validation on connect. Only these two attributes are meant +to be set — the base owns the protocol methods and register derivation (`@final`): ```python -from harp.device import Device, REGISTER_MAP as _CORE_REGISTER_MAP +from harp.device import Device class MyDevice(Device): __whoami__ = 1216 - -REGISTER_MAP = {**_CORE_REGISTER_MAP, 32: DigitalInputState, ...} + __REGISTERS__ = (DigitalInputState, ...) ``` +Registers are then reached by name through `device.registers` +(`MyDevice.registers.DigitalInputState`) or by address +(`MyDevice.registers[32]` / `MyDevice.registers.by_address`). For static type +hints on `device.registers.`, subclass `CoreRegisters` and declare the +device's registers — see the [device examples](https://harp-tech.org/pyharp/examples/). + A new transport is just an object implementing the `ITransport` protocol (`open`/`write`/`read`/`close`). ## Generating a device from a `device.yml` If you don't have a pre-generated device package, `create_device` builds a -`Device` from Harp `device.yml` text. Registers are reached by address through -`REGISTER_MAP`; field and enum names come from the yml verbatim. +`Device` from Harp `device.yml` text. Registers are reached by name through +`device.registers`; field and enum names come from the yml verbatim. ```python from pathlib import Path from harp.device import create_device Behavior = create_device(Path("device.yml").read_text()) -reg = Behavior.REGISTER_MAP[44] +reg = Behavior.registers.AnalogData # by name (or Behavior.registers[44]) ``` For a custom `interfaceType`, pass its converter via `converters=` (keyed by diff --git a/src/packages/harp-device/src/harp/device/__init__.py b/src/packages/harp-device/src/harp/device/__init__.py index d352a19..c6610d7 100644 --- a/src/packages/harp-device/src/harp/device/__init__.py +++ b/src/packages/harp-device/src/harp/device/__init__.py @@ -26,7 +26,8 @@ TimestampSeconds, WhoAmI, ) -from ._register_map import REGISTER_MAP +from ._core_registers import CORE_REGISTERS, CoreRegisters +from ._register_namespace import RegisterNamespace from ._schema import ConverterContext, parse_device_schema from ._transport import ITransport, TransportError @@ -40,7 +41,9 @@ "HarpFramer", "ITransport", "TransportError", - "REGISTER_MAP", + "RegisterNamespace", + "CoreRegisters", + "CORE_REGISTERS", "WhoAmI", "HardwareVersionHigh", "HardwareVersionLow", diff --git a/src/packages/harp-device/src/harp/device/_core_registers.py b/src/packages/harp-device/src/harp/device/_core_registers.py new file mode 100644 index 0000000..72dc7c3 --- /dev/null +++ b/src/packages/harp-device/src/harp/device/_core_registers.py @@ -0,0 +1,82 @@ +"""The common Harp registers as a tuple, plus a typed namespace for them. + +Every :class:`~harp.device.Device` merges :data:`CORE_REGISTERS` with its own +``REGISTERS`` to build ``device.registers``. Statically generated devices subclass +:class:`CoreRegisters` to declare their device-specific registers with real types, +so ``device.registers.`` autocompletes and type-checks. +""" + +from typing import Any + +from harp.protocol import RegisterBase + +from ._register_namespace import RegisterNamespace +from ._registers import ( + AssemblyVersion, + ClockConfiguration, + CoreVersionHigh, + CoreVersionLow, + DeviceName, + FirmwareVersionHigh, + FirmwareVersionLow, + HardwareVersionHigh, + HardwareVersionLow, + OperationControl, + ResetDevice, + SerialNumber, + TimestampMicroseconds, + TimestampSeconds, + WhoAmI, +) + +#: The common Harp registers, present on every device. +CORE_REGISTERS: tuple[type[RegisterBase[Any]], ...] = ( + WhoAmI, + HardwareVersionHigh, + HardwareVersionLow, + AssemblyVersion, + CoreVersionHigh, + CoreVersionLow, + FirmwareVersionHigh, + FirmwareVersionLow, + TimestampSeconds, + TimestampMicroseconds, + OperationControl, + ResetDevice, + DeviceName, + SerialNumber, + ClockConfiguration, +) + + +class CoreRegisters(RegisterNamespace): + """Typed register namespace declaring the common Harp registers. + + Every :class:`~harp.device.Device` exposes at least these as ``device.registers``. + A statically generated device subclasses this to add its own registers with real + types:: + + class BehaviorRegisters(CoreRegisters): + DigitalInputState: type[DigitalInputState] + AnalogData: type[AnalogData] + + so ``device.registers.AnalogData`` autocompletes and type-checks. At runtime the + namespace is populated from the device's registers; these annotations carry no + runtime values. + """ + + WhoAmI: type[WhoAmI] + HardwareVersionHigh: type[HardwareVersionHigh] + HardwareVersionLow: type[HardwareVersionLow] + AssemblyVersion: type[AssemblyVersion] + CoreVersionHigh: type[CoreVersionHigh] + CoreVersionLow: type[CoreVersionLow] + FirmwareVersionHigh: type[FirmwareVersionHigh] + FirmwareVersionLow: type[FirmwareVersionLow] + TimestampSeconds: type[TimestampSeconds] + TimestampMicroseconds: type[TimestampMicroseconds] + OperationControl: type[OperationControl] + ResetDevice: type[ResetDevice] + DeviceName: type[DeviceName] + SerialNumber: type[SerialNumber] + ClockConfiguration: type[ClockConfiguration] diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index d765da3..ccc88ad 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -1,7 +1,7 @@ """Transport-agnostic Harp device base class.""" from collections.abc import Callable, Iterable -from typing import Any, ClassVar, Self, TypeVar +from typing import Any, ClassVar, Self, TypeVar, final import logging import queue @@ -11,6 +11,7 @@ from harp.protocol._message import ParsedHarpMessage from harp.protocol._register import RegisterBase +from ._core_registers import CORE_REGISTERS, CoreRegisters from ._framer import HarpFramer from ._transport import ITransport, TransportError from ._registers import ( @@ -71,9 +72,14 @@ class Device: """Harp device protocol logic (framing, request/reply, register access) over an :class:`~harp.device.ITransport`. - Must be opened before use, via ``with`` or :meth:`open`. Subclasses add - register class attributes and set :attr:`__whoami__` to validate device - identity on open (``0x0`` skips the check). + Must be opened before use, via ``with`` or :meth:`open`. A subclass declares its + device-specific registers in :attr:`__REGISTERS__` and sets :attr:`__whoami__` to + validate device identity on open (``0x0`` skips the check). Registers are reached + by name through :attr:`registers` (``device.registers.WhoAmI``). + + Only :attr:`__REGISTERS__` and :attr:`__whoami__` are meant to be set by a + subclass. The protocol methods and register-namespace derivation are ``@final`` — + the base owns them and they must not be overridden. """ REPLY_TIMEOUT: ClassVar[float] = 5.0 # seconds @@ -81,8 +87,25 @@ class Device: #: Expected ``WhoAmI`` of the device this class models; ``0x0`` skips the check. __whoami__: ClassVar[int] = 0x0 - #: Address -> register class; empty on the base, overridden by generated devices. - REGISTER_MAP: ClassVar[dict[int, type[RegisterBase[Any]]]] = {} + #: The device's own registers. A subclass sets this to a tuple of register + #: classes; the common Harp registers are merged in automatically. + __REGISTERS__: ClassVar[tuple[type[RegisterBase[Any]], ...]] = () + + #: Name/address-indexed view of all this device's registers (core + ``__REGISTERS__``). + #: Reach a register by name (``device.registers.WhoAmI``) or address + #: (``device.registers[0]``); see :class:`~harp.device.RegisterNamespace`. Derived + #: by :meth:`__init_subclass__`; do not set it directly. + registers: ClassVar[CoreRegisters] = CoreRegisters(CORE_REGISTERS) + + @final + def __init_subclass__(cls, **kwargs: Any) -> None: + super().__init_subclass__(**kwargs) + # Merge inherited registers (core + any parent's) with this class's own + # __REGISTERS__; on an address clash the device's register wins. + merged: dict[int, type[RegisterBase[Any]]] = dict(cls.registers.by_address) + for register in cls.__dict__.get("__REGISTERS__", ()): + merged[register.address] = register + cls.registers = CoreRegisters(merged.values()) def __init__(self, transport: ITransport, *, raise_on_error: bool = True) -> None: self._transport = transport @@ -105,6 +128,7 @@ def __init__(self, transport: ITransport, *, raise_on_error: bool = True) -> Non # Lifecycle # ------------------------------------------------------------------ + @final def open(self) -> Self: """Open the transport, start the reader thread and validate identity.""" self._transport.open() @@ -136,6 +160,7 @@ def _validate_whoami(self) -> None: f"but device reported 0x{actual:04x}." ) + @final def close(self) -> None: self._running = False if self._thread is not None: @@ -151,11 +176,13 @@ def close(self) -> None: self._registers.clear() self._catch_all.clear() + @final def __enter__(self) -> Self: if not self._running: self.open() return self + @final def __exit__(self, *args: object) -> None: self.close() @@ -163,6 +190,7 @@ def __exit__(self, *args: object) -> None: # Register access # ------------------------------------------------------------------ + @final def read( self, register: type[RegisterBase[P]], @@ -176,6 +204,7 @@ def read( msg = self._request(register.address, frame) return ParsedHarpMessage.from_message(msg, register.parse(msg)) + @final def write( self, register: type[RegisterBase[P]], @@ -194,6 +223,7 @@ def write( # Events # ------------------------------------------------------------------ + @final def subscribe( self, register: type[RegisterBase[P]], @@ -226,6 +256,7 @@ def subscribe( self._registers[register.address] = register return sub + @final def subscribe_all( self, handler: Callable[[HarpMessage], None], diff --git a/src/packages/harp-device/src/harp/device/_emit_device.py b/src/packages/harp-device/src/harp/device/_emit_device.py index 82f43b4..9a9d5af 100644 --- a/src/packages/harp-device/src/harp/device/_emit_device.py +++ b/src/packages/harp-device/src/harp/device/_emit_device.py @@ -1,7 +1,6 @@ from typing import Any, Mapping, Optional, Union from ._device import Device -from ._register_map import REGISTER_MAP as CORE_REGISTER_MAP from ._schema import create_registers, parse_device_schema from ._schema._emit import ConverterValue from ._schema._model import DeviceModel @@ -17,21 +16,21 @@ def create_device( ) -> type[Device]: """Emit a :class:`Device` subclass from a device schema. - The returned class exposes its registers through the ``REGISTER_MAP`` class - attribute (address -> register class) and carries ``__whoami__`` from the schema - (``0x0`` when absent). The device's registers are spread on top of the core common - map; on an address clash the device's register wins. ``exclude_private=True`` drops - registers whose DSL ``visibility`` is ``private``. A header-less register - fragment yields a device with no ``device`` name (falls back to ``"Device"``). + The returned class carries its device-specific registers in ``__REGISTERS__`` and + exposes all of them (merged with the common Harp registers) by name through + ``device.registers`` (e.g. ``Behavior.registers.AnalogData``). ``__whoami__`` comes + from the schema (``0x0`` when absent). On an address clash the device's register + wins over the common one. ``exclude_private=True`` drops registers whose DSL + ``visibility`` is ``private``. A header-less register fragment yields a device with + no ``device`` name (falls back to ``"Device"``). """ device = source if isinstance(source, DeviceModel) else parse_device_schema(source) registers = create_registers( device, converters=converters, strict=strict, exclude_private=exclude_private ) - by_address = {cls.address: cls for cls in registers.values()} namespace: dict[str, Any] = { "__whoami__": int(device.whoAmI or 0), - "REGISTER_MAP": {**CORE_REGISTER_MAP, **by_address}, + "__REGISTERS__": tuple(registers.values()), } return type(name or device.device or "Device", (Device,), namespace) diff --git a/src/packages/harp-device/src/harp/device/_register_map.py b/src/packages/harp-device/src/harp/device/_register_map.py deleted file mode 100644 index bc532dc..0000000 --- a/src/packages/harp-device/src/harp/device/_register_map.py +++ /dev/null @@ -1,48 +0,0 @@ -"""Address → register-class map for the core Harp registers. - -Downstream device packages spread this into their own map:: - - from harp.device import REGISTER_MAP as _CORE_REGISTER_MAP - - REGISTER_MAP = {**_CORE_REGISTER_MAP, 32: DigitalInputState, ...} -""" - -from typing import Any - -from harp.protocol import RegisterBase - -from ._registers import ( - AssemblyVersion, - ClockConfiguration, - CoreVersionHigh, - CoreVersionLow, - DeviceName, - FirmwareVersionHigh, - FirmwareVersionLow, - HardwareVersionHigh, - HardwareVersionLow, - OperationControl, - ResetDevice, - SerialNumber, - TimestampMicroseconds, - TimestampSeconds, - WhoAmI, -) - -REGISTER_MAP: dict[int, type[RegisterBase[Any]]] = { - 0: WhoAmI, - 1: HardwareVersionHigh, - 2: HardwareVersionLow, - 3: AssemblyVersion, - 4: CoreVersionHigh, - 5: CoreVersionLow, - 6: FirmwareVersionHigh, - 7: FirmwareVersionLow, - 8: TimestampSeconds, - 9: TimestampMicroseconds, - 10: OperationControl, - 11: ResetDevice, - 12: DeviceName, - 13: SerialNumber, - 14: ClockConfiguration, -} diff --git a/src/packages/harp-device/src/harp/device/_register_namespace.py b/src/packages/harp-device/src/harp/device/_register_namespace.py new file mode 100644 index 0000000..3e038a7 --- /dev/null +++ b/src/packages/harp-device/src/harp/device/_register_namespace.py @@ -0,0 +1,94 @@ +"""A name/address-indexed view over a device's register classes. + +`Device.registers` is a :class:`RegisterNamespace`, so registers are reached by +name — ``device.registers.WhoAmI`` — as well as by address +(``device.registers[0]`` / ``device.registers.by_address``). Statically generated +devices narrow the type to a :class:`CoreRegisters` subclass so editors autocomplete +the register names; see :class:`CoreRegisters`. +""" + +from collections.abc import Iterable, Iterator, Mapping +from typing import Any + +from harp.protocol import RegisterBase + +_Register = type[RegisterBase[Any]] + + +class RegisterNamespace: + """Attribute- and item-addressable collection of register classes. + + Built from an iterable of register classes; each is indexed by its + ``__name__`` and its ``address``. Attribute access returns the register class:: + + ns = RegisterNamespace([WhoAmI, OperationControl]) + ns.WhoAmI # -> type[WhoAmI] + ns["WhoAmI"] # -> type[WhoAmI] (by name) + ns[0] # -> type[WhoAmI] (by address) + ns.by_address # {0: WhoAmI, 10: OperationControl} + + Attribute access falls back to :meth:`__getattr__`, typed as + ``type[RegisterBase[Any]]`` so any register name type-checks; a + :class:`CoreRegisters` subclass declares specific names for precise types. + """ + + def __init__(self, registers: Iterable[_Register]) -> None: + by_name: dict[str, _Register] = {} + by_address: dict[int, _Register] = {} + for register in registers: + by_name[register.__name__] = register + by_address[register.address] = register + self._by_name = by_name + self._by_address = by_address + + def __getattr__(self, name: str) -> _Register: + # Only consulted when normal attribute lookup fails, so real methods and + # the ``_by_*`` internals always win. Guard dunder/private lookups so a + # missing ``_by_name`` (e.g. during copy/pickle) can't recurse. + if name.startswith("_"): + raise AttributeError(name) + try: + return self.__dict__["_by_name"][name] + except KeyError: + raise AttributeError( + f"no register named {name!r}; available: {', '.join(self.__dict__['_by_name'])}" + ) from None + + def __getitem__(self, key: str | int) -> _Register: + try: + if isinstance(key, int): + return self._by_address[key] + return self._by_name[key] + except KeyError: + kind = "address" if isinstance(key, int) else "name" + raise KeyError(f"no register with {kind} {key!r}") from None + + @property + def by_name(self) -> Mapping[str, _Register]: + """The name -> register-class map.""" + return self._by_name + + @property + def by_address(self) -> Mapping[int, _Register]: + """The address -> register-class map.""" + return self._by_address + + def __iter__(self) -> Iterator[_Register]: + return iter(self._by_name.values()) + + def __contains__(self, key: object) -> bool: + if isinstance(key, int): + return key in self._by_address + if isinstance(key, str): + return key in self._by_name + return key in self._by_name.values() + + def __len__(self) -> int: + return len(self._by_name) + + def __dir__(self) -> Iterable[str]: + return [*super().__dir__(), *self._by_name] + + def __repr__(self) -> str: + names = ", ".join(sorted(self._by_name)) + return f"{type(self).__name__}({names})" diff --git a/tests/data/test_dataset.py b/tests/data/test_dataset.py index 581f415..cdd6285 100644 --- a/tests/data/test_dataset.py +++ b/tests/data/test_dataset.py @@ -31,10 +31,10 @@ def dataset(emitted_device, tmp_path): """A dataset folder with three app registers; the first is timestamped.""" dev = emitted_device name = dev.__name__ - addresses = [a for a in sorted(dev.REGISTER_MAP) if a >= 32][:3] + addresses = [a for a in sorted(dev.registers.by_address) if a >= 32][:3] specs = {} for i, address in enumerate(addresses): - cls = dev.REGISTER_MAP[address] + cls = dev.registers.by_address[address] records = _records(cls, 5, seed=address) timestamped = i == 0 timestamps = np.arange(5, dtype=np.float64) if timestamped else None @@ -95,8 +95,8 @@ def test_read_all_keyed_by_register_name(dataset): def test_suffix_chunks_are_concatenated(emitted_device, tmp_path): dev = emitted_device name = dev.__name__ - address = next(a for a in sorted(dev.REGISTER_MAP) if a >= 32) - cls = dev.REGISTER_MAP[address] + address = next(a for a in sorted(dev.registers.by_address) if a >= 32) + cls = dev.registers.by_address[address] chunk0 = bytes(cls.format_bulk(_records(cls, 3, seed=1))) chunk1 = bytes(cls.format_bulk(_records(cls, 2, seed=2))) (tmp_path / f"{name}_{address}_0.bin").write_bytes(chunk0) @@ -114,7 +114,7 @@ def test_non_device_raises_on_register_access(dataset): _dev, _name, root, _specs = dataset # Registers are derived lazily; a non-Device fails when they are accessed. reader = DatasetReader(object, root) - with pytest.raises(AttributeError, match="REGISTER_MAP"): + with pytest.raises(AttributeError, match="registers"): _ = reader.registers @@ -142,10 +142,10 @@ def test_unknown_address_raises(dataset): def test_custom_file_resolver_supports_alternative_layout(emitted_device, tmp_path): dev = emitted_device - addresses = [a for a in sorted(dev.REGISTER_MAP) if a >= 32][:2] + addresses = [a for a in sorted(dev.registers.by_address) if a >= 32][:2] expected = {} for address in addresses: - cls = dev.REGISTER_MAP[address] + cls = dev.registers.by_address[address] buf = bytes(cls.format_bulk(_records(cls, 3, seed=address))) (tmp_path / f"reg{address}.bin").write_bytes(buf) # not the Harp layout expected[cls.__name__] = parse_to_dataframe(cls, buf, timestamp=False) @@ -177,7 +177,7 @@ def test_read_all_registers_of_mock_device(emitted_device, tmp_path): dev = emitted_device name = dev.__name__ expected = {} - for address, cls in dev.REGISTER_MAP.items(): + for address, cls in dev.registers.by_address.items(): records = _records(cls, 4, seed=address) # Alternate timestamped/untimestamped to exercise both parse paths. timestamped = address % 2 == 0 @@ -189,9 +189,9 @@ def test_read_all_registers_of_mock_device(emitted_device, tmp_path): reader = DatasetReader(dev, tmp_path) frames = reader.read_all() - assert set(reader.files) == set(dev.REGISTER_MAP) + assert set(reader.files) == set(dev.registers.by_address) assert set(frames) == set(expected) - assert len(frames) == len(dev.REGISTER_MAP) + assert len(frames) == len(dev.registers.by_address) for register_name, df in frames.items(): assert len(df) == 4 assert df.equals(expected[register_name]) @@ -202,7 +202,7 @@ def test_reader_derives_name_and_registers_from_device(dataset): reader = DatasetReader(dev, root) assert reader.device is dev assert reader.name == name - assert reader.registers == dev.REGISTER_MAP + assert reader.registers is dev.registers def test_create_dataset_reader_builds_device_from_device_yml(dataset, device_yml): diff --git a/tests/device/expected_core.py b/tests/device/expected_core.py index 6684278..ae4c505 100644 --- a/tests/device/expected_core.py +++ b/tests/device/expected_core.py @@ -210,20 +210,20 @@ class ClockConfiguration(RegisterBase[ClockConfigurationFlags]): payload_class = ClockConfigurationPayload -REGISTER_MAP: dict[int, type[RegisterBase[Any]]] = { - 0: WhoAmI, - 1: HardwareVersionHigh, - 2: HardwareVersionLow, - 3: AssemblyVersion, - 4: CoreVersionHigh, - 5: CoreVersionLow, - 6: FirmwareVersionHigh, - 7: FirmwareVersionLow, - 8: TimestampSeconds, - 9: TimestampMicroseconds, - 10: OperationControl, - 11: ResetDevice, - 12: DeviceName, - 13: SerialNumber, - 14: ClockConfiguration, -} +REGISTERS: tuple[type[RegisterBase[Any]], ...] = ( + WhoAmI, + HardwareVersionHigh, + HardwareVersionLow, + AssemblyVersion, + CoreVersionHigh, + CoreVersionLow, + FirmwareVersionHigh, + FirmwareVersionLow, + TimestampSeconds, + TimestampMicroseconds, + OperationControl, + ResetDevice, + DeviceName, + SerialNumber, + ClockConfiguration, +) diff --git a/tests/device/expected_device.py b/tests/device/expected_device.py index d2fbf4f..7353ff3 100644 --- a/tests/device/expected_device.py +++ b/tests/device/expected_device.py @@ -2,7 +2,7 @@ # To make changes, edit the device metadata and regenerate the interface. import enum -from typing import Any, ClassVar +from typing import TYPE_CHECKING, ClassVar import numpy as np from numpy.typing import NDArray @@ -23,7 +23,7 @@ StringConverter, StructPayload, ) -from harp.device import REGISTER_MAP as _CORE_REGISTER_MAP +from harp.device import CoreRegisters, Device from .converters import ( DataConverter, @@ -232,21 +232,46 @@ class EncoderMode(RegisterBase[EncoderModeMask]): payload_class = EncoderModePayload -REGISTER_MAP: dict[int, type[RegisterBase[Any]]] = { - **_CORE_REGISTER_MAP, - 32: DigitalInputs, - 33: AnalogData, - 34: ComplexConfiguration, - 35: Version, - 36: CustomPayload, - 37: CustomRawPayload, - 38: CustomMemberConverter, - 39: BitmaskSplitter, - 40: Counter0, - 41: PortDIOSet, - 42: PulseDOPort0, - 43: PulseDO0, - 100: StartPulse, - 101: StartPulseTrain, - 103: EncoderMode, -} +class Tests(Device): + """A device driven by its own registers; the common Harp registers are merged + in automatically. Registers are reached by name — ``Tests.registers.AnalogData``.""" + + __REGISTERS__ = ( + DigitalInputs, + AnalogData, + ComplexConfiguration, + Version, + CustomPayload, + CustomRawPayload, + CustomMemberConverter, + BitmaskSplitter, + Counter0, + PortDIOSet, + PulseDOPort0, + PulseDO0, + StartPulse, + StartPulseTrain, + EncoderMode, + ) + + if TYPE_CHECKING: + # Type-only facade so editors autocomplete `device.registers.` and + # `read`/`write` infer the register's payload type. No runtime values. + class _Registers(CoreRegisters): + DigitalInputs: type[DigitalInputs] + AnalogData: type[AnalogData] + ComplexConfiguration: type[ComplexConfiguration] + Version: type[Version] + CustomPayload: type[CustomPayload] + CustomRawPayload: type[CustomRawPayload] + CustomMemberConverter: type[CustomMemberConverter] + BitmaskSplitter: type[BitmaskSplitter] + Counter0: type[Counter0] + PortDIOSet: type[PortDIOSet] + PulseDOPort0: type[PulseDOPort0] + PulseDO0: type[PulseDO0] + StartPulse: type[StartPulse] + StartPulseTrain: type[StartPulseTrain] + EncoderMode: type[EncoderMode] + + registers: ClassVar[_Registers] diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py index c3e996c..c6f0768 100644 --- a/tests/device/test_device_emit.py +++ b/tests/device/test_device_emit.py @@ -28,25 +28,37 @@ def test_whoami_from_schema(): assert Dev.__whoami__ == 1216 +def test_registers_are_reachable_by_name(test_device): + regs = test_device.registers + assert regs.AnalogData.address == 33 + assert regs.EncoderMode.address == 103 + + def test_registers_are_reachable_by_address(test_device): - reg_map = test_device.REGISTER_MAP - assert reg_map[33].__name__ == "AnalogData" - assert reg_map[103].__name__ == "EncoderMode" + regs = test_device.registers + assert regs[33].__name__ == "AnalogData" + assert regs[103].__name__ == "EncoderMode" + + +def test_registers_include_core(test_device): + regs = test_device.registers + assert regs.WhoAmI.address == 0 # core register, always merged in + assert regs.AnalogData.address == 33 # device-specific + assert regs[0].__name__ == "WhoAmI" -def test_register_map_spreads_core(test_device): - reg_map = test_device.REGISTER_MAP - assert reg_map[0].__name__ == "WhoAmI" # core register, always spread in - assert reg_map[33].__name__ == "AnalogData" # device-specific - assert reg_map[103].__name__ == "EncoderMode" +def test_unknown_register_name_raises(test_device): + with pytest.raises(AttributeError, match="Nonexistent"): + _ = test_device.registers.Nonexistent def test_device_register_overrides_core_on_clash(): - # A device register at a core address wins over the spread-in common one. + # A device register at a core address wins over the merged-in common one. Dev = create_device( "device: Clash\nregisters:\n Shadow: {address: 0, type: U32, access: Read}\n" ) - assert Dev.REGISTER_MAP[0].__name__ == "Shadow" + assert Dev.registers[0].__name__ == "Shadow" + assert Dev.registers.Shadow.address == 0 def test_headerless_fragment_builds_default_device(): @@ -54,11 +66,11 @@ def test_headerless_fragment_builds_default_device(): Dev = create_device("registers:\n Foo: {address: 40, type: U16, access: Read}\n") assert Dev.__name__ == "Device" assert Dev.__whoami__ == 0 - assert Dev.REGISTER_MAP[40].__name__ == "Foo" + assert Dev.registers.Foo.address == 40 def test_emitted_device_registers_are_usable(test_device): - reg = test_device.REGISTER_MAP[33] # AnalogData + reg = test_device.registers.AnalogData # reached by name # 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]) diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index 4c61667..ca63a74 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -19,9 +19,10 @@ def device_registers(device_yml): def _device_registers(): - # expected_device.REGISTER_MAP spreads the core map; the device-specific - # registers (the ones the emitter builds from device.yml) are address >= 32. - return {cls.__name__: cls for addr, cls in expected_device.REGISTER_MAP.items() if addr >= 32} + # expected_device.Tests.__REGISTERS__ holds only the device-specific registers + # (the ones the emitter builds from device.yml); the core ones are merged in + # by Device automatically. + return {cls.__name__: cls for cls in expected_device.Tests.__REGISTERS__} def _layout(dt): @@ -75,7 +76,7 @@ def test_enum_members_are_verbatim(device_registers): def _core_expected(): - return {cls.__name__: cls for cls in expected_core.REGISTER_MAP.values()} + return {cls.__name__: cls for cls in expected_core.REGISTERS} @pytest.mark.parametrize("name", sorted(_core_expected())) From 191c22e133e8ae75677f43934708a5d66c7e8ab5 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 11:24:12 -0700 Subject: [PATCH 14/22] Simplify register collection interface --- docs/examples/create_device/create_device.md | 2 +- docs/examples/create_device/create_device.py | 4 +- .../harp-data/src/harp/data/_dataset.py | 2 +- src/packages/harp-device/README.md | 12 ++-- .../harp-device/src/harp/device/_device.py | 22 ++----- .../src/harp/device/_register_namespace.py | 62 +++++++------------ tests/device/test_device_emit.py | 26 ++++++-- 7 files changed, 60 insertions(+), 70 deletions(-) diff --git a/docs/examples/create_device/create_device.md b/docs/examples/create_device/create_device.md index 9a8686a..11bcbf2 100644 --- a/docs/examples/create_device/create_device.md +++ b/docs/examples/create_device/create_device.md @@ -6,7 +6,7 @@ code-generation step. This is the quickest way to get started when you have only device's schema and no pre-generated package for it. The compiled device exposes its registers by name through `device.registers` -(e.g. `Behavior.registers.AnalogData`, or by address with `Behavior.registers[44]`) +(e.g. `Behavior.registers.AnalogData`, or the `Behavior.registers.by_address` map) and carries the device's `__whoami__` identity. From there it works exactly like a pre-generated device class — drive it over a transport to talk to hardware, or use its register classes to decode recorded data. diff --git a/docs/examples/create_device/create_device.py b/docs/examples/create_device/create_device.py index 08cb734..be3b1ab 100644 --- a/docs/examples/create_device/create_device.py +++ b/docs/examples/create_device/create_device.py @@ -15,8 +15,8 @@ print("WhoAmI:", Behavior.__whoami__) # device identity, taken from the schema # Registers are reached by name through `.registers` — the common Harp registers -# (like WhoAmI) plus the device's own. Address lookup still works via -# `Behavior.registers[44]` or `Behavior.registers.by_address`. +# (like WhoAmI) plus the device's own. Address lookup goes through +# `Behavior.registers.by_address`. AnalogData = Behavior.registers.AnalogData # The generated device behaves like any other `Device` class. Talk to hardware over diff --git a/src/packages/harp-data/src/harp/data/_dataset.py b/src/packages/harp-data/src/harp/data/_dataset.py index 0696791..22b4072 100644 --- a/src/packages/harp-data/src/harp/data/_dataset.py +++ b/src/packages/harp-data/src/harp/data/_dataset.py @@ -84,7 +84,7 @@ def name(self) -> str: @property def registers(self) -> RegisterNamespace: """The device's registers, reachable by name (``reader.registers.WhoAmI``) - or address (``reader.registers[44]`` / ``reader.registers.by_address``).""" + or through the ``reader.registers.by_address`` map.""" return self._device.registers @property diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index 66dd5d8..eeb900e 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -22,7 +22,7 @@ device.write(OperationControl, payload) # write a register Downstream (often generated) packages declare their registers in a `__REGISTERS__` tuple; the common Harp registers are merged in automatically. They may set `__whoami__` for identity validation on connect. Only these two attributes are meant -to be set — the base owns the protocol methods and register derivation (`@final`): +to be set — the base owns the protocol methods and register derivation: ```python from harp.device import Device @@ -33,10 +33,10 @@ class MyDevice(Device): ``` Registers are then reached by name through `device.registers` -(`MyDevice.registers.DigitalInputState`) or by address -(`MyDevice.registers[32]` / `MyDevice.registers.by_address`). For static type -hints on `device.registers.`, subclass `CoreRegisters` and declare the -device's registers — see the [device examples](https://harp-tech.org/pyharp/examples/). +(`MyDevice.registers.DigitalInputState`) or through the +`MyDevice.registers.by_address` map. For static type hints on +`device.registers.`, subclass `CoreRegisters` and declare the device's +registers — see the [device examples](https://harp-tech.org/pyharp/examples/). A new transport is just an object implementing the `ITransport` protocol (`open`/`write`/`read`/`close`). @@ -52,7 +52,7 @@ from pathlib import Path from harp.device import create_device Behavior = create_device(Path("device.yml").read_text()) -reg = Behavior.registers.AnalogData # by name (or Behavior.registers[44]) +reg = Behavior.registers.AnalogData # by name (or Behavior.registers.by_address[44]) ``` For a custom `interfaceType`, pass its converter via `converters=` (keyed by diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index ccc88ad..6d6337d 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -1,7 +1,7 @@ """Transport-agnostic Harp device base class.""" from collections.abc import Callable, Iterable -from typing import Any, ClassVar, Self, TypeVar, final +from typing import Any, ClassVar, Self, TypeVar import logging import queue @@ -78,8 +78,7 @@ class Device: by name through :attr:`registers` (``device.registers.WhoAmI``). Only :attr:`__REGISTERS__` and :attr:`__whoami__` are meant to be set by a - subclass. The protocol methods and register-namespace derivation are ``@final`` — - the base owns them and they must not be overridden. + subclass; the base owns the protocol methods and the register-namespace derivation. """ REPLY_TIMEOUT: ClassVar[float] = 5.0 # seconds @@ -91,13 +90,12 @@ class Device: #: classes; the common Harp registers are merged in automatically. __REGISTERS__: ClassVar[tuple[type[RegisterBase[Any]], ...]] = () - #: Name/address-indexed view of all this device's registers (core + ``__REGISTERS__``). - #: Reach a register by name (``device.registers.WhoAmI``) or address - #: (``device.registers[0]``); see :class:`~harp.device.RegisterNamespace`. Derived - #: by :meth:`__init_subclass__`; do not set it directly. + #: Name-indexed view of all this device's registers (core + ``__REGISTERS__``). + #: Reach a register by name (``device.registers.WhoAmI``), or use + #: ``device.registers.by_address``; see :class:`~harp.device.RegisterNamespace`. + #: Derived by :meth:`__init_subclass__`; do not set it directly. registers: ClassVar[CoreRegisters] = CoreRegisters(CORE_REGISTERS) - @final def __init_subclass__(cls, **kwargs: Any) -> None: super().__init_subclass__(**kwargs) # Merge inherited registers (core + any parent's) with this class's own @@ -128,7 +126,6 @@ def __init__(self, transport: ITransport, *, raise_on_error: bool = True) -> Non # Lifecycle # ------------------------------------------------------------------ - @final def open(self) -> Self: """Open the transport, start the reader thread and validate identity.""" self._transport.open() @@ -160,7 +157,6 @@ def _validate_whoami(self) -> None: f"but device reported 0x{actual:04x}." ) - @final def close(self) -> None: self._running = False if self._thread is not None: @@ -176,13 +172,11 @@ def close(self) -> None: self._registers.clear() self._catch_all.clear() - @final def __enter__(self) -> Self: if not self._running: self.open() return self - @final def __exit__(self, *args: object) -> None: self.close() @@ -190,7 +184,6 @@ def __exit__(self, *args: object) -> None: # Register access # ------------------------------------------------------------------ - @final def read( self, register: type[RegisterBase[P]], @@ -204,7 +197,6 @@ def read( msg = self._request(register.address, frame) return ParsedHarpMessage.from_message(msg, register.parse(msg)) - @final def write( self, register: type[RegisterBase[P]], @@ -223,7 +215,6 @@ def write( # Events # ------------------------------------------------------------------ - @final def subscribe( self, register: type[RegisterBase[P]], @@ -256,7 +247,6 @@ def subscribe( self._registers[register.address] = register return sub - @final def subscribe_all( self, handler: Callable[[HarpMessage], None], diff --git a/src/packages/harp-device/src/harp/device/_register_namespace.py b/src/packages/harp-device/src/harp/device/_register_namespace.py index 3e038a7..de2046e 100644 --- a/src/packages/harp-device/src/harp/device/_register_namespace.py +++ b/src/packages/harp-device/src/harp/device/_register_namespace.py @@ -1,10 +1,10 @@ -"""A name/address-indexed view over a device's register classes. +"""A name-indexed view over a device's register classes. `Device.registers` is a :class:`RegisterNamespace`, so registers are reached by -name — ``device.registers.WhoAmI`` — as well as by address -(``device.registers[0]`` / ``device.registers.by_address``). Statically generated -devices narrow the type to a :class:`CoreRegisters` subclass so editors autocomplete -the register names; see :class:`CoreRegisters`. +name — ``device.registers.WhoAmI`` — with the ``by_name`` / ``by_address`` maps for +programmatic lookup. Statically generated devices narrow the type to a +:class:`CoreRegisters` subclass so editors autocomplete the register names; see +:class:`CoreRegisters`. """ from collections.abc import Iterable, Iterator, Mapping @@ -16,35 +16,32 @@ class RegisterNamespace: - """Attribute- and item-addressable collection of register classes. + """Attribute-addressable collection of register classes. Built from an iterable of register classes; each is indexed by its - ``__name__`` and its ``address``. Attribute access returns the register class:: + ``__name__`` and its ``address``. Registers are reached by name:: ns = RegisterNamespace([WhoAmI, OperationControl]) - ns.WhoAmI # -> type[WhoAmI] - ns["WhoAmI"] # -> type[WhoAmI] (by name) - ns[0] # -> type[WhoAmI] (by address) + ns.WhoAmI # -> type[WhoAmI] (attribute access) + ns.by_name # {"WhoAmI": WhoAmI, "OperationControl": OperationControl} ns.by_address # {0: WhoAmI, 10: OperationControl} - Attribute access falls back to :meth:`__getattr__`, typed as - ``type[RegisterBase[Any]]`` so any register name type-checks; a - :class:`CoreRegisters` subclass declares specific names for precise types. + Iteration yields the register classes, and ``in`` tests register-class + membership (``WhoAmI in ns``). Attribute access falls back to + :meth:`__getattr__`, typed as ``type[RegisterBase[Any]]`` so any register name + type-checks; a :class:`CoreRegisters` subclass declares specific names for + precise types. """ def __init__(self, registers: Iterable[_Register]) -> None: - by_name: dict[str, _Register] = {} - by_address: dict[int, _Register] = {} - for register in registers: - by_name[register.__name__] = register - by_address[register.address] = register - self._by_name = by_name - self._by_address = by_address + self._registers = tuple(registers) + self._by_name = {register.__name__: register for register in self._registers} + self._by_address = {register.address: register for register in self._registers} def __getattr__(self, name: str) -> _Register: # Only consulted when normal attribute lookup fails, so real methods and - # the ``_by_*`` internals always win. Guard dunder/private lookups so a - # missing ``_by_name`` (e.g. during copy/pickle) can't recurse. + # the ``_registers``/``_by_*`` internals always win. Guard dunder/private + # lookups so a missing ``_by_name`` (e.g. during copy/pickle) can't recurse. if name.startswith("_"): raise AttributeError(name) try: @@ -54,15 +51,6 @@ def __getattr__(self, name: str) -> _Register: f"no register named {name!r}; available: {', '.join(self.__dict__['_by_name'])}" ) from None - def __getitem__(self, key: str | int) -> _Register: - try: - if isinstance(key, int): - return self._by_address[key] - return self._by_name[key] - except KeyError: - kind = "address" if isinstance(key, int) else "name" - raise KeyError(f"no register with {kind} {key!r}") from None - @property def by_name(self) -> Mapping[str, _Register]: """The name -> register-class map.""" @@ -74,17 +62,13 @@ def by_address(self) -> Mapping[int, _Register]: return self._by_address def __iter__(self) -> Iterator[_Register]: - return iter(self._by_name.values()) + return iter(self._registers) - def __contains__(self, key: object) -> bool: - if isinstance(key, int): - return key in self._by_address - if isinstance(key, str): - return key in self._by_name - return key in self._by_name.values() + def __contains__(self, register: object) -> bool: + return any(register is r for r in self._registers) def __len__(self) -> int: - return len(self._by_name) + return len(self._registers) def __dir__(self) -> Iterable[str]: return [*super().__dir__(), *self._by_name] diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py index c6f0768..caec3c4 100644 --- a/tests/device/test_device_emit.py +++ b/tests/device/test_device_emit.py @@ -35,16 +35,16 @@ def test_registers_are_reachable_by_name(test_device): def test_registers_are_reachable_by_address(test_device): - regs = test_device.registers - assert regs[33].__name__ == "AnalogData" - assert regs[103].__name__ == "EncoderMode" + by_address = test_device.registers.by_address + assert by_address[33].__name__ == "AnalogData" + assert by_address[103].__name__ == "EncoderMode" def test_registers_include_core(test_device): regs = test_device.registers assert regs.WhoAmI.address == 0 # core register, always merged in assert regs.AnalogData.address == 33 # device-specific - assert regs[0].__name__ == "WhoAmI" + assert regs.by_address[0].__name__ == "WhoAmI" def test_unknown_register_name_raises(test_device): @@ -52,12 +52,28 @@ def test_unknown_register_name_raises(test_device): _ = test_device.registers.Nonexistent +def test_registers_membership_is_by_register_class(test_device): + from harp.device import WhoAmI + + regs = test_device.registers + assert WhoAmI in regs # register-class membership (core, merged in) + assert regs.AnalogData in regs # device-specific + assert "WhoAmI" not in regs # not by name + assert 0 not in regs # not by address + + +def test_registers_iterates_register_classes(test_device): + regs = test_device.registers + assert set(regs) == set(regs.by_address.values()) + assert len(regs) == len(regs.by_address) + + def test_device_register_overrides_core_on_clash(): # A device register at a core address wins over the merged-in common one. Dev = create_device( "device: Clash\nregisters:\n Shadow: {address: 0, type: U32, access: Read}\n" ) - assert Dev.registers[0].__name__ == "Shadow" + assert Dev.registers.by_address[0].__name__ == "Shadow" assert Dev.registers.Shadow.address == 0 From 9267cb06461bd6197c9dacfd13f21ee0ab725d77 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:46:46 -0700 Subject: [PATCH 15/22] Add examples and fix warning --- docs/examples/index.md | 3 +- docs/examples/static_device/static_device.md | 68 ++++++++++++++++ docs/examples/static_device/static_device.py | 86 ++++++++++++++++++++ mkdocs.yml | 1 + tests/device/expected_device.py | 2 +- tests/device/test_device_emit.py | 22 +++++ 6 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 docs/examples/static_device/static_device.md create mode 100644 docs/examples/static_device/static_device.py diff --git a/docs/examples/index.md b/docs/examples/index.md index ff83850..07db667 100644 --- a/docs/examples/index.md +++ b/docs/examples/index.md @@ -2,8 +2,9 @@ This section contains some examples to help you get started with `harp`. -Working from a device schema: +Defining a device: +- [Defining a Device Statically](./static_device/static_device.md) - write a device as plain, typed Python classes (the shape code generators emit). - [Generating a Device from a Schema](./create_device/create_device.md) - compile a `device.yml` into a typed device at runtime with `create_device`. Talking to a device: diff --git a/docs/examples/static_device/static_device.md b/docs/examples/static_device/static_device.md new file mode 100644 index 0000000..c7cba05 --- /dev/null +++ b/docs/examples/static_device/static_device.md @@ -0,0 +1,68 @@ +# Defining a Device Statically + +There are two ways to get a [`Device`](../../api/device.md): compile one at runtime +from a `device.yml` with [`create_device`](../create_device/create_device.md), or +define one **statically** as plain Python classes. This article covers the static +form — what you write by hand for a distributable, fully typed device package, and +exactly the shape a code generator emits from a `device.yml`. + +Prefer the static form when you want an importable device with real register classes, +editor autocomplete, and static type checking (`read`/`write` inferring payload +types). Prefer `create_device` when you only have a schema in hand and don't need a +published package. + +## The pieces + +A static device is four kinds of declaration: + +- **Register classes** — each a `RegisterBase` subclass carrying its `address`. + Scalars can use the `Register` shortcuts (e.g. `RegisterU16`); structured + registers set `payload_type` and point at a payload class. +- **Enums and payload classes** — `IntEnum` / `IntFlag` for enum and mask fields, and + `StructPayload` / `AnonymousPayload` subclasses describing multi-field payloads. +- **The `Device` subclass** — sets **only** `__whoami__` (the expected identity) and + `__REGISTERS__` (a tuple of the device's **own** registers). The common Harp + registers are merged in and `device.registers` is derived automatically. +- **A typed facade** (optional but recommended) — under `TYPE_CHECKING`, a + `CoreRegisters` subclass declaring `Name: type[Name]` for each register, so + `device.registers.` autocompletes and types precisely. + +## What the base gives you + +You never hand-build an address map. The base `Device`: + +- merges the common Harp registers with `__REGISTERS__` (device wins on an address clash); +- derives `device.registers` — reach a register by name (`device.registers.Encoder`), + or use `device.registers.by_name` / `.by_address`; +- validates the device's `WhoAmI` against `__whoami__` on connect (`0x0` skips the check). + +Do **not** declare a `REGISTER_MAP`, spread the common registers into `__REGISTERS__`, +or override the base's protocol methods (`read`, `write`, `subscribe`, lifecycle) — +they are the base's job. + +!!! note "The typed facade" + The facade under `TYPE_CHECKING` narrows the base's `registers` attribute, which + the type checker would otherwise flag as an incompatible override — hence the one + `# pyright: ignore[reportIncompatibleVariableOverride]`. It carries no runtime + values; the actual namespace is always built from `__REGISTERS__` by the base. + Skip the facade and `device.registers.` still works, typed as the generic + `type[RegisterBase]` (no per-register autocomplete). + +## For code generators + +This is the contract a generator targets. Per device, emit: + +1. Each register as a `RegisterBase` subclass (with its enums and payload classes). +2. A `Device` subclass setting only `__whoami__` and `__REGISTERS__` (the device's own + registers — **not** the common ones). +3. A `TYPE_CHECKING` facade subclassing `CoreRegisters`, one `Name: type[Name]` per + device register, plus `registers: ClassVar[]`. + +Generators must **not** emit a `REGISTER_MAP`, spread the common registers into +`__REGISTERS__`, or override any base `Device` method. + + +```python +[](./static_device.py) +``` + diff --git a/docs/examples/static_device/static_device.py b/docs/examples/static_device/static_device.py new file mode 100644 index 0000000..373e6d0 --- /dev/null +++ b/docs/examples/static_device/static_device.py @@ -0,0 +1,86 @@ +import enum +from typing import TYPE_CHECKING, ClassVar + +import numpy as np +from harp.device import CoreRegisters, Device +from harp.protocol import ( + BoolConverter, + Field, + GroupMask, + PayloadType, + RegisterBase, + RegisterU16, + StructPayload, +) +from harp.serial import open_serial_device + +# A statically defined device: plain Python classes, no schema or runtime +# generation. This is what you write by hand for a distributable, fully typed +# device package — and exactly what a code generator emits from a `device.yml`. + + +# --- Enums ------------------------------------------------------------------- +class LedMode(enum.IntEnum): + """Values for the Control register's `led` field.""" + + OFF = 0 + ON = 1 + BLINK = 2 + + +# --- Register payloads ------------------------------------------------------- +class ControlPayload(StructPayload[np.uint8]): + """Payload of the Control register: a masked enum plus a boolean flag.""" + + led: LedMode = GroupMask(enum=LedMode, mask=0x3) + enabled: bool = Field(BoolConverter(), mask=0x4) + + +# --- Registers --------------------------------------------------------------- +# Each register is a `RegisterBase` subclass carrying its `address`. Scalars can +# use the `Register` shortcuts; structured registers point at a payload class. +class Encoder(RegisterU16): + """A 16-bit counter — a plain scalar register.""" + + address: ClassVar[int] = 32 + + +class Control(RegisterBase[ControlPayload]): + """A structured register decoded through `ControlPayload`.""" + + address: ClassVar[int] = 33 + payload_type: ClassVar[PayloadType] = PayloadType.U8 + payload_class = ControlPayload + + +# --- Device ------------------------------------------------------------------ +class ExampleDevice(Device): + """A statically defined Harp device. + + A subclass sets only `__whoami__` and `__REGISTERS__`. The common Harp + registers are merged in and `device.registers` is derived automatically — do + not declare a `REGISTER_MAP` or override the base's protocol methods. + """ + + __whoami__ = 1234 + __REGISTERS__ = (Encoder, Control) + + if TYPE_CHECKING: + # Optional but recommended: makes `device.registers.` autocomplete and + # type precisely. It deliberately narrows the base `registers` attribute, so + # the override warning is silenced. Carries no runtime values. + class _Registers(CoreRegisters): + Encoder: type[Encoder] + Control: type[Control] + + registers: ClassVar[_Registers] # pyright: ignore[reportIncompatibleVariableOverride] + + +# Registers are reached by name, on the class or an instance: +assert ExampleDevice.registers.Encoder is Encoder +assert ExampleDevice.registers.Control.address == 33 + +# Use it exactly like a runtime-generated device (see the serial examples): +with open_serial_device(ExampleDevice, port="COM3") as device: + print(device.read(device.registers.Encoder).parsed) # by name + print(device.read(Encoder).parsed) # or the register class directly diff --git a/mkdocs.yml b/mkdocs.yml index c4a63b4..ea3558c 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -73,6 +73,7 @@ nav: - Home: index.md - Examples: - examples/index.md + - Defining a Device Statically: examples/static_device/static_device.md - Generating a Device from a Schema: examples/create_device/create_device.md - Getting Device Info: examples/get_info/get_info.md - Read and Write from Registers: examples/read_and_write_from_registers/read_and_write_from_registers.md diff --git a/tests/device/expected_device.py b/tests/device/expected_device.py index 7353ff3..372baec 100644 --- a/tests/device/expected_device.py +++ b/tests/device/expected_device.py @@ -274,4 +274,4 @@ class _Registers(CoreRegisters): StartPulseTrain: type[StartPulseTrain] EncoderMode: type[EncoderMode] - registers: ClassVar[_Registers] + registers: ClassVar[_Registers] # pyright: ignore[reportIncompatibleVariableOverride] diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py index caec3c4..8782b82 100644 --- a/tests/device/test_device_emit.py +++ b/tests/device/test_device_emit.py @@ -1,3 +1,5 @@ +from typing import ClassVar + import pytest from harp.device import Device, create_device @@ -68,6 +70,26 @@ def test_registers_iterates_register_classes(test_device): assert len(regs) == len(regs.by_address) +def test_static_device_subclass_derives_registers(): + # A hand-written (or generated) static device: declare __REGISTERS__ and the + # base merges in the common registers and builds `registers`. No REGISTER_MAP. + from harp.device import WhoAmI + from harp.protocol import RegisterU16 + + class Counter(RegisterU16): + address: ClassVar[int] = 40 + + class MyDevice(Device): + __whoami__ = 42 + __REGISTERS__ = (Counter,) + + assert MyDevice.__whoami__ == 42 + assert MyDevice.registers.Counter is Counter # device register, by name + assert MyDevice.registers.WhoAmI is WhoAmI # common register, merged in + assert Counter in MyDevice.registers # membership by register class + assert MyDevice.registers.by_address[40] is Counter + + def test_device_register_overrides_core_on_clash(): # A device register at a core address wins over the merged-in common one. Dev = create_device( From c68b4f3068f4a7880fc374bd23b2b00deecb093d Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 16:59:31 -0700 Subject: [PATCH 16/22] Protect against mapping mutation --- .../src/harp/device/_register_namespace.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/packages/harp-device/src/harp/device/_register_namespace.py b/src/packages/harp-device/src/harp/device/_register_namespace.py index de2046e..efa525a 100644 --- a/src/packages/harp-device/src/harp/device/_register_namespace.py +++ b/src/packages/harp-device/src/harp/device/_register_namespace.py @@ -8,6 +8,7 @@ """ from collections.abc import Iterable, Iterator, Mapping +from types import MappingProxyType from typing import Any from harp.protocol import RegisterBase @@ -35,8 +36,14 @@ class RegisterNamespace: def __init__(self, registers: Iterable[_Register]) -> None: self._registers = tuple(registers) - self._by_name = {register.__name__: register for register in self._registers} - self._by_address = {register.address: register for register in self._registers} + # We use MappingProxyType here to make the maps read-only, so they can't be + # accidentally mutated at runtime. + self._by_name: Mapping[str, _Register] = MappingProxyType( + {register.__name__: register for register in self._registers} + ) + self._by_address: Mapping[int, _Register] = MappingProxyType( + {register.address: register for register in self._registers} + ) def __getattr__(self, name: str) -> _Register: # Only consulted when normal attribute lookup fails, so real methods and From 07241c20ce7f8c288c6f2c1579d0cfdd9a591b74 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 18:47:09 -0700 Subject: [PATCH 17/22] Remove need to declare the pyright annotation --- docs/api/device.md | 2 +- docs/examples/static_device/static_device.md | 23 ++++----- docs/examples/static_device/static_device.py | 18 +++---- src/packages/harp-device/README.md | 2 +- .../harp-device/src/harp/device/__init__.py | 4 +- .../src/harp/device/_core_registers.py | 6 +-- .../harp-device/src/harp/device/_device.py | 49 ++++++++++++++---- .../src/harp/device/_register_namespace.py | 6 +-- tests/device/expected_device.py | 50 +++++++++---------- 9 files changed, 90 insertions(+), 70 deletions(-) diff --git a/docs/api/device.md b/docs/api/device.md index 4f706d5..891d662 100644 --- a/docs/api/device.md +++ b/docs/api/device.md @@ -7,7 +7,7 @@ ::: harp.device.parse_device_schema ::: harp.device.ConverterContext ::: harp.device.RegisterNamespace -::: harp.device.CoreRegisters +::: harp.device.CoreRegistersNamespace ::: harp.device.HarpFramer ::: harp.device.ITransport ::: harp.device.TransportError diff --git a/docs/examples/static_device/static_device.md b/docs/examples/static_device/static_device.md index c7cba05..453d2a0 100644 --- a/docs/examples/static_device/static_device.md +++ b/docs/examples/static_device/static_device.md @@ -23,9 +23,10 @@ A static device is four kinds of declaration: - **The `Device` subclass** — sets **only** `__whoami__` (the expected identity) and `__REGISTERS__` (a tuple of the device's **own** registers). The common Harp registers are merged in and `device.registers` is derived automatically. -- **A typed facade** (optional but recommended) — under `TYPE_CHECKING`, a - `CoreRegisters` subclass declaring `Name: type[Name]` for each register, so - `device.registers.` autocompletes and types precisely. +- **A typed facade** (optional but recommended) — a `CoreRegistersNamespace` + subclass declaring `Name: type[Name]` for each register, plus + `registers: ClassVar[]`, so `device.registers.` autocompletes and + types precisely. ## What the base gives you @@ -38,15 +39,7 @@ You never hand-build an address map. The base `Device`: Do **not** declare a `REGISTER_MAP`, spread the common registers into `__REGISTERS__`, or override the base's protocol methods (`read`, `write`, `subscribe`, lifecycle) — -they are the base's job. - -!!! note "The typed facade" - The facade under `TYPE_CHECKING` narrows the base's `registers` attribute, which - the type checker would otherwise flag as an incompatible override — hence the one - `# pyright: ignore[reportIncompatibleVariableOverride]`. It carries no runtime - values; the actual namespace is always built from `__REGISTERS__` by the base. - Skip the facade and `device.registers.` still works, typed as the generic - `type[RegisterBase]` (no per-register autocomplete). +they are the base's job. We may add a `@final` decorator to `Device` in the future to enforce this. ## For code generators @@ -55,8 +48,10 @@ This is the contract a generator targets. Per device, emit: 1. Each register as a `RegisterBase` subclass (with its enums and payload classes). 2. A `Device` subclass setting only `__whoami__` and `__REGISTERS__` (the device's own registers — **not** the common ones). -3. A `TYPE_CHECKING` facade subclassing `CoreRegisters`, one `Name: type[Name]` per - device register, plus `registers: ClassVar[]`. +3. A facade subclassing `CoreRegistersNamespace`, one `Name: type[Name]` per device + register, plus `registers: ClassVar[]`. Narrowing `registers` needs no + pyright suppression because the base declares it read-only; the facade is never + instantiated. Generators must **not** emit a `REGISTER_MAP`, spread the common registers into `__REGISTERS__`, or override any base `Device` method. diff --git a/docs/examples/static_device/static_device.py b/docs/examples/static_device/static_device.py index 373e6d0..c8a1540 100644 --- a/docs/examples/static_device/static_device.py +++ b/docs/examples/static_device/static_device.py @@ -1,8 +1,8 @@ import enum -from typing import TYPE_CHECKING, ClassVar +from typing import ClassVar import numpy as np -from harp.device import CoreRegisters, Device +from harp.device import CoreRegistersNamespace, Device from harp.protocol import ( BoolConverter, Field, @@ -65,15 +65,13 @@ class ExampleDevice(Device): __whoami__ = 1234 __REGISTERS__ = (Encoder, Control) - if TYPE_CHECKING: - # Optional but recommended: makes `device.registers.` autocomplete and - # type precisely. It deliberately narrows the base `registers` attribute, so - # the override warning is silenced. Carries no runtime values. - class _Registers(CoreRegisters): - Encoder: type[Encoder] - Control: type[Control] + # Optional but recommended: makes `device.registers.` autocomplete and + # type-aware. Never instantiated — the base builds the namespace from `__REGISTERS__`. + class _ExampleRegisters(CoreRegistersNamespace): + Encoder: type[Encoder] + Control: type[Control] - registers: ClassVar[_Registers] # pyright: ignore[reportIncompatibleVariableOverride] + registers: ClassVar[_ExampleRegisters] # Registers are reached by name, on the class or an instance: diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index eeb900e..e4688c4 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -35,7 +35,7 @@ class MyDevice(Device): Registers are then reached by name through `device.registers` (`MyDevice.registers.DigitalInputState`) or through the `MyDevice.registers.by_address` map. For static type hints on -`device.registers.`, subclass `CoreRegisters` and declare the device's +`device.registers.`, subclass `CoreRegistersNamespace` and declare the device's registers — see the [device examples](https://harp-tech.org/pyharp/examples/). A new transport is just an object implementing the `ITransport` protocol diff --git a/src/packages/harp-device/src/harp/device/__init__.py b/src/packages/harp-device/src/harp/device/__init__.py index c6610d7..cdeb5fa 100644 --- a/src/packages/harp-device/src/harp/device/__init__.py +++ b/src/packages/harp-device/src/harp/device/__init__.py @@ -26,7 +26,7 @@ TimestampSeconds, WhoAmI, ) -from ._core_registers import CORE_REGISTERS, CoreRegisters +from ._core_registers import CORE_REGISTERS, CoreRegistersNamespace from ._register_namespace import RegisterNamespace from ._schema import ConverterContext, parse_device_schema from ._transport import ITransport, TransportError @@ -42,7 +42,7 @@ "ITransport", "TransportError", "RegisterNamespace", - "CoreRegisters", + "CoreRegistersNamespace", "CORE_REGISTERS", "WhoAmI", "HardwareVersionHigh", diff --git a/src/packages/harp-device/src/harp/device/_core_registers.py b/src/packages/harp-device/src/harp/device/_core_registers.py index 72dc7c3..efcd588 100644 --- a/src/packages/harp-device/src/harp/device/_core_registers.py +++ b/src/packages/harp-device/src/harp/device/_core_registers.py @@ -2,7 +2,7 @@ Every :class:`~harp.device.Device` merges :data:`CORE_REGISTERS` with its own ``REGISTERS`` to build ``device.registers``. Statically generated devices subclass -:class:`CoreRegisters` to declare their device-specific registers with real types, +:class:`CoreRegistersNamespace` to declare their device-specific registers with real types, so ``device.registers.`` autocompletes and type-checks. """ @@ -49,14 +49,14 @@ ) -class CoreRegisters(RegisterNamespace): +class CoreRegistersNamespace(RegisterNamespace): """Typed register namespace declaring the common Harp registers. Every :class:`~harp.device.Device` exposes at least these as ``device.registers``. A statically generated device subclasses this to add its own registers with real types:: - class BehaviorRegisters(CoreRegisters): + class BehaviorRegisters(CoreRegistersNamespace): DigitalInputState: type[DigitalInputState] AnalogData: type[AnalogData] diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index 6d6337d..84dd38d 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -1,22 +1,21 @@ """Transport-agnostic Harp device base class.""" -from collections.abc import Callable, Iterable -from typing import Any, ClassVar, Self, TypeVar - import logging import queue import threading +from collections.abc import Callable, Iterable +from typing import Any, ClassVar, Self, TypeVar from harp.protocol import HarpMessage, MessageType from harp.protocol._message import ParsedHarpMessage from harp.protocol._register import RegisterBase -from ._core_registers import CORE_REGISTERS, CoreRegisters +from ._core_registers import CORE_REGISTERS, CoreRegistersNamespace from ._framer import HarpFramer -from ._transport import ITransport, TransportError from ._registers import ( WhoAmI, ) +from ._transport import ITransport, TransportError P = TypeVar("P") @@ -38,6 +37,27 @@ def _normalize_message_types(message_types: MessageTypeFilter) -> frozenset[Mess return frozenset(message_types) +class _RegisterAccessor: + """Read-only descriptor backing :attr:`Device.registers`. + + It defines ``__get__`` but no ``__set__``, which is deliberate: + + * ``device.registers`` is **read-only** — assigning to it is a type error; + * a subclass may **narrow** the attribute by re-declaring it, because a + read-only member is checked *covariantly* (a mutable one would be invariant). + + So a statically generated device can re-declare + ``registers: ClassVar[]`` to type + ``device.registers.`` precisely, with **no** + ``reportIncompatibleVariableOverride`` suppression. The real namespace is + assigned per subclass in :meth:`Device.__init_subclass__` (which shadows this + descriptor); this default only backs the bare :class:`Device` base. + """ + + def __get__(self, obj: object, owner: type | None = None) -> CoreRegistersNamespace: + return CoreRegistersNamespace(CORE_REGISTERS) + + class Subscription: """Handle returned by :meth:`Device.subscribe`. Cancel with :meth:`unsubscribe`, or use as a context manager to auto-cancel on exit.""" @@ -81,7 +101,7 @@ class Device: subclass; the base owns the protocol methods and the register-namespace derivation. """ - REPLY_TIMEOUT: ClassVar[float] = 5.0 # seconds + # === The following are class variables meant to be set by a subclass === #: Expected ``WhoAmI`` of the device this class models; ``0x0`` skips the check. __whoami__: ClassVar[int] = 0x0 @@ -90,11 +110,16 @@ class Device: #: classes; the common Harp registers are merged in automatically. __REGISTERS__: ClassVar[tuple[type[RegisterBase[Any]], ...]] = () - #: Name-indexed view of all this device's registers (core + ``__REGISTERS__``). - #: Reach a register by name (``device.registers.WhoAmI``), or use + + #: Name-indexed, **read-only** view of all this device's registers (core + + #: ``__REGISTERS__``). Reach a register by name (``device.registers.WhoAmI`` — + #: the common registers autocomplete on any device), or use #: ``device.registers.by_address``; see :class:`~harp.device.RegisterNamespace`. - #: Derived by :meth:`__init_subclass__`; do not set it directly. - registers: ClassVar[CoreRegisters] = CoreRegisters(CORE_REGISTERS) + #: A statically generated device may *narrow* this by re-declaring + #: ``registers: ClassVar[]`` for typed, autocompleting + registers = _RegisterAccessor() + + REPLY_TIMEOUT: ClassVar[float] = 5.0 # seconds def __init_subclass__(cls, **kwargs: Any) -> None: super().__init_subclass__(**kwargs) @@ -103,7 +128,9 @@ def __init_subclass__(cls, **kwargs: Any) -> None: merged: dict[int, type[RegisterBase[Any]]] = dict(cls.registers.by_address) for register in cls.__dict__.get("__REGISTERS__", ()): merged[register.address] = register - cls.registers = CoreRegisters(merged.values()) + # ``type.__setattr__`` (not ``cls.registers = ...``) keeps pyright treating + # ``registers`` as read-only; it shadows the base descriptor on the subclass. + type.__setattr__(cls, "registers", CoreRegistersNamespace(merged.values())) def __init__(self, transport: ITransport, *, raise_on_error: bool = True) -> None: self._transport = transport diff --git a/src/packages/harp-device/src/harp/device/_register_namespace.py b/src/packages/harp-device/src/harp/device/_register_namespace.py index efa525a..1a48009 100644 --- a/src/packages/harp-device/src/harp/device/_register_namespace.py +++ b/src/packages/harp-device/src/harp/device/_register_namespace.py @@ -3,8 +3,8 @@ `Device.registers` is a :class:`RegisterNamespace`, so registers are reached by name — ``device.registers.WhoAmI`` — with the ``by_name`` / ``by_address`` maps for programmatic lookup. Statically generated devices narrow the type to a -:class:`CoreRegisters` subclass so editors autocomplete the register names; see -:class:`CoreRegisters`. +:class:`CoreRegistersNamespace` subclass so editors autocomplete the register names; see +:class:`CoreRegistersNamespace`. """ from collections.abc import Iterable, Iterator, Mapping @@ -30,7 +30,7 @@ class RegisterNamespace: Iteration yields the register classes, and ``in`` tests register-class membership (``WhoAmI in ns``). Attribute access falls back to :meth:`__getattr__`, typed as ``type[RegisterBase[Any]]`` so any register name - type-checks; a :class:`CoreRegisters` subclass declares specific names for + type-checks; a :class:`CoreRegistersNamespace` subclass declares specific names for precise types. """ diff --git a/tests/device/expected_device.py b/tests/device/expected_device.py index 372baec..5629de2 100644 --- a/tests/device/expected_device.py +++ b/tests/device/expected_device.py @@ -2,10 +2,10 @@ # To make changes, edit the device metadata and regenerate the interface. import enum -from typing import TYPE_CHECKING, ClassVar +from typing import ClassVar import numpy as np -from numpy.typing import NDArray +from harp.device import CoreRegistersNamespace, Device from harp.protocol import ( AnonymousPayload, BitMask, @@ -18,12 +18,12 @@ PayloadType, RegisterBase, RegisterS32, - RegisterU16, RegisterU8, + RegisterU16, StringConverter, StructPayload, ) -from harp.device import CoreRegisters, Device +from numpy.typing import NDArray from .converters import ( DataConverter, @@ -254,24 +254,24 @@ class Tests(Device): EncoderMode, ) - if TYPE_CHECKING: - # Type-only facade so editors autocomplete `device.registers.` and - # `read`/`write` infer the register's payload type. No runtime values. - class _Registers(CoreRegisters): - DigitalInputs: type[DigitalInputs] - AnalogData: type[AnalogData] - ComplexConfiguration: type[ComplexConfiguration] - Version: type[Version] - CustomPayload: type[CustomPayload] - CustomRawPayload: type[CustomRawPayload] - CustomMemberConverter: type[CustomMemberConverter] - BitmaskSplitter: type[BitmaskSplitter] - Counter0: type[Counter0] - PortDIOSet: type[PortDIOSet] - PulseDOPort0: type[PulseDOPort0] - PulseDO0: type[PulseDO0] - StartPulse: type[StartPulse] - StartPulseTrain: type[StartPulseTrain] - EncoderMode: type[EncoderMode] - - registers: ClassVar[_Registers] # pyright: ignore[reportIncompatibleVariableOverride] + # Facade declaring the device's registers with real types so editors autocomplete + # `device.registers.` and `read`/`write` infer the payload type. Never + # instantiated — the namespace is built from `__REGISTERS__` by the base. + class _Registers(CoreRegistersNamespace): + DigitalInputs: type[DigitalInputs] + AnalogData: type[AnalogData] + ComplexConfiguration: type[ComplexConfiguration] + Version: type[Version] + CustomPayload: type[CustomPayload] + CustomRawPayload: type[CustomRawPayload] + CustomMemberConverter: type[CustomMemberConverter] + BitmaskSplitter: type[BitmaskSplitter] + Counter0: type[Counter0] + PortDIOSet: type[PortDIOSet] + PulseDOPort0: type[PulseDOPort0] + PulseDO0: type[PulseDO0] + StartPulse: type[StartPulse] + StartPulseTrain: type[StartPulseTrain] + EncoderMode: type[EncoderMode] + + registers: ClassVar[_Registers] From f130ef5a36cab7a459b26aa2376cb63760b8c282 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 18:50:14 -0700 Subject: [PATCH 18/22] Linting --- README.md | 6 +++--- src/packages/harp-data/README.md | 6 +++--- src/packages/harp-device/README.md | 7 ++++--- src/packages/harp-device/src/harp/device/_device.py | 1 - src/packages/harp-protocol/README.md | 6 ++++-- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 23e6d5c..5711c25 100644 --- a/README.md +++ b/README.md @@ -75,8 +75,8 @@ from harp.data import create_dataset_reader # Finds device.yml in the folder, builds the device, returns a ready-to-use reader. reader = create_dataset_reader("session.harp") -df = reader.read(44) # one register, by address (or pass its class) -everything = reader.read_all() # {register_name: DataFrame} +df = reader.read(44) # one register, by address (or pass its class) +everything = reader.read_all() # {register_name: DataFrame} ``` Both paths are driven by a device schema. If you have only a `device.yml` and no @@ -89,7 +89,7 @@ from pathlib import Path from harp.device import create_device Behavior = create_device(Path("device.yml").read_text()) -AnalogData = Behavior.registers.AnalogData # registers are reached by name +AnalogData = Behavior.registers.AnalogData # registers are reached by name ``` See the [Examples](https://harp-tech.org/pyharp/examples/) for the full walkthroughs, diff --git a/src/packages/harp-data/README.md b/src/packages/harp-data/README.md index 80045b8..c8c3ee3 100644 --- a/src/packages/harp-data/README.md +++ b/src/packages/harp-data/README.md @@ -32,9 +32,9 @@ builds the device, and returns a ready-to-use reader: from harp.data import create_dataset_reader reader = create_dataset_reader("session.harp") -df = reader.read(AnalogData) # by register class -df = reader.read(44) # by address -everything = reader.read_all() # {register_name: DataFrame} +df = reader.read(AnalogData) # by register class +df = reader.read(44) # by address +everything = reader.read_all() # {register_name: DataFrame} ``` Already have a device class (e.g. a pre-generated package, or one built with diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index e4688c4..3e6c8a3 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -13,8 +13,8 @@ A `Device` is driven over a transport; `read`/`write` take a register class: from harp.device import Device, WhoAmI, OperationControl # `device` is a Device opened over some transport (see harp-serial) -who = device.read(WhoAmI).parsed # -> np.uint16 -device.write(OperationControl, payload) # write a register +who = device.read(WhoAmI).parsed # -> np.uint16 +device.write(OperationControl, payload) # write a register ``` ## Extending for a specific device @@ -27,6 +27,7 @@ to be set — the base owns the protocol methods and register derivation: ```python from harp.device import Device + class MyDevice(Device): __whoami__ = 1216 __REGISTERS__ = (DigitalInputState, ...) @@ -52,7 +53,7 @@ from pathlib import Path from harp.device import create_device Behavior = create_device(Path("device.yml").read_text()) -reg = Behavior.registers.AnalogData # by name (or Behavior.registers.by_address[44]) +reg = Behavior.registers.AnalogData # by name (or Behavior.registers.by_address[44]) ``` For a custom `interfaceType`, pass its converter via `converters=` (keyed by diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index 84dd38d..c7c98ab 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -110,7 +110,6 @@ class Device: #: classes; the common Harp registers are merged in automatically. __REGISTERS__: ClassVar[tuple[type[RegisterBase[Any]], ...]] = () - #: Name-indexed, **read-only** view of all this device's registers (core + #: ``__REGISTERS__``). Reach a register by name (``device.registers.WhoAmI`` — #: the common registers autocomplete on any device), or use diff --git a/src/packages/harp-protocol/README.md b/src/packages/harp-protocol/README.md index 1afee4d..abbb64d 100644 --- a/src/packages/harp-protocol/README.md +++ b/src/packages/harp-protocol/README.md @@ -12,11 +12,13 @@ For more detail please check Harp Tech's official documentation [here](https://h import numpy as np from harp.protocol import HarpMessage, RegisterU16 + class WhoAmI(RegisterU16): address = 0 -frame = WhoAmI.format(np.uint16(1216)) # build a Write frame -value = WhoAmI.parse(HarpMessage.parse(frame)) # -> np.uint16(1216) + +frame = WhoAmI.format(np.uint16(1216)) # build a Write frame +value = WhoAmI.parse(HarpMessage.parse(frame)) # -> np.uint16(1216) ``` It carries no transport or device logic — see [`harp-device`](../harp-device) for the device layer. From 7261bbff7f786109577b656d51512601f0b22f73 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Sun, 26 Jul 2026 18:51:42 -0700 Subject: [PATCH 19/22] Fix typos --- src/packages/harp-device/src/harp/device/_device.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index c7c98ab..674d559 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -43,10 +43,10 @@ class _RegisterAccessor: It defines ``__get__`` but no ``__set__``, which is deliberate: * ``device.registers`` is **read-only** — assigning to it is a type error; - * a subclass may **narrow** the attribute by re-declaring it, because a + * a subclass may **narrow** the attribute by redeclaring it, because a read-only member is checked *covariantly* (a mutable one would be invariant). - So a statically generated device can re-declare + So a statically generated device can redeclare ``registers: ClassVar[]`` to type ``device.registers.`` precisely, with **no** ``reportIncompatibleVariableOverride`` suppression. The real namespace is @@ -114,7 +114,7 @@ class Device: #: ``__REGISTERS__``). Reach a register by name (``device.registers.WhoAmI`` — #: the common registers autocomplete on any device), or use #: ``device.registers.by_address``; see :class:`~harp.device.RegisterNamespace`. - #: A statically generated device may *narrow* this by re-declaring + #: A statically generated device may *narrow* this by redeclaring #: ``registers: ClassVar[]`` for typed, autocompleting registers = _RegisterAccessor() From 1a1ee7aacacb3d519106440f5fd83579db5d4e3d Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:07:20 -0700 Subject: [PATCH 20/22] Refactor RegisterMap API --- docs/api/device.md | 4 +- docs/examples/static_device/static_device.md | 45 ++++++----- docs/examples/static_device/static_device.py | 25 +++--- .../harp-data/src/harp/data/_dataset.py | 4 +- src/packages/harp-device/README.md | 30 ++++--- .../harp-device/src/harp/device/__init__.py | 8 +- .../src/harp/device/_core_registers.py | 53 +++++++------ .../harp-device/src/harp/device/_device.py | 76 ++++++------------ .../src/harp/device/_emit_device.py | 24 ++++-- .../src/harp/device/_register_namespace.py | 79 +++++++++++++------ tests/device/expected_device.py | 61 +++++--------- tests/device/test_device_emit.py | 36 +++++---- tests/device/test_emit.py | 15 ++-- 13 files changed, 241 insertions(+), 219 deletions(-) diff --git a/docs/api/device.md b/docs/api/device.md index 891d662..475e2b6 100644 --- a/docs/api/device.md +++ b/docs/api/device.md @@ -6,8 +6,8 @@ ::: harp.device.create_device ::: harp.device.parse_device_schema ::: harp.device.ConverterContext -::: harp.device.RegisterNamespace -::: harp.device.CoreRegistersNamespace +::: harp.device.RegisterMap +::: harp.device.CoreRegisters ::: harp.device.HarpFramer ::: harp.device.ITransport ::: harp.device.TransportError diff --git a/docs/examples/static_device/static_device.md b/docs/examples/static_device/static_device.md index 453d2a0..e529d7b 100644 --- a/docs/examples/static_device/static_device.md +++ b/docs/examples/static_device/static_device.md @@ -20,41 +20,44 @@ A static device is four kinds of declaration: registers set `payload_type` and point at a payload class. - **Enums and payload classes** — `IntEnum` / `IntFlag` for enum and mask fields, and `StructPayload` / `AnonymousPayload` subclasses describing multi-field payloads. +- **The register namespace** — a `CoreRegisters` subclass **assigning** each of the + device's own registers (`Encoder = Encoder`). Subclassing `CoreRegisters` merges in + the common Harp registers. - **The `Device` subclass** — sets **only** `__whoami__` (the expected identity) and - `__REGISTERS__` (a tuple of the device's **own** registers). The common Harp - registers are merged in and `device.registers` is derived automatically. -- **A typed facade** (optional but recommended) — a `CoreRegistersNamespace` - subclass declaring `Name: type[Name]` for each register, plus - `registers: ClassVar[]`, so `device.registers.` autocompletes and - types precisely. + `registers` (an instance of the namespace class). ## What the base gives you You never hand-build an address map. The base `Device`: -- merges the common Harp registers with `__REGISTERS__` (device wins on an address clash); -- derives `device.registers` — reach a register by name (`device.registers.Encoder`), - or use `device.registers.by_name` / `.by_address`; +- resolves the namespace class into `device.registers` — reach a register by name + (`device.registers.Encoder`), or use `device.registers.by_name` / `.by_address`; - validates the device's `WhoAmI` against `__whoami__` on connect (`0x0` skips the check). -Do **not** declare a `REGISTER_MAP`, spread the common registers into `__REGISTERS__`, -or override the base's protocol methods (`read`, `write`, `subscribe`, lifecycle) — -they are the base's job. We may add a `@final` decorator to `Device` in the future to enforce this. +`CoreRegisters` contributes the common Harp registers through normal inheritance, so +on an address clash the most-derived register wins. A device that needs a different +common set may subclass `RegisterMap` directly instead. + +Do **not** spread the common registers into your namespace, or override the base's +protocol methods (`read`, `write`, `subscribe`, lifecycle) — they are the base's job. +We may add a `@final` decorator to `Device` in the future to enforce this. ## For code generators This is the contract a generator targets. Per device, emit: -1. Each register as a `RegisterBase` subclass (with its enums and payload classes). -2. A `Device` subclass setting only `__whoami__` and `__REGISTERS__` (the device's own - registers — **not** the common ones). -3. A facade subclassing `CoreRegistersNamespace`, one `Name: type[Name]` per device - register, plus `registers: ClassVar[]`. Narrowing `registers` needs no - pyright suppression because the base declares it read-only; the facade is never - instantiated. +1. Each register as a `RegisterBase` subclass (with its enums and payload classes), at + module scope. +2. A namespace subclassing `CoreRegisters`, one `Name = Name` assignment per device + register — **not** the common ones, which come from the base. +3. A `Device` subclass setting only `__whoami__` and + `registers: ClassVar[] = ()`. Narrowing `registers` needs a + trailing `# pyright: ignore[reportIncompatibleVariableOverride]` — a mutable + `ClassVar` is invariant, so a type checker rejects the narrowing even though the + namespace *is* a `CoreRegisters`. -Generators must **not** emit a `REGISTER_MAP`, spread the common registers into -`__REGISTERS__`, or override any base `Device` method. +Generators must **not** spread the common registers into the namespace, or override +any base `Device` method. ```python diff --git a/docs/examples/static_device/static_device.py b/docs/examples/static_device/static_device.py index c8a1540..bbd8921 100644 --- a/docs/examples/static_device/static_device.py +++ b/docs/examples/static_device/static_device.py @@ -2,7 +2,7 @@ from typing import ClassVar import numpy as np -from harp.device import CoreRegistersNamespace, Device +from harp.device import CoreRegisters, Device from harp.protocol import ( BoolConverter, Field, @@ -53,25 +53,24 @@ class Control(RegisterBase[ControlPayload]): payload_class = ControlPayload +class ExampleRegisters(CoreRegisters): + Encoder = Encoder + Control = Control + + # --- Device ------------------------------------------------------------------ class ExampleDevice(Device): """A statically defined Harp device. - A subclass sets only `__whoami__` and `__REGISTERS__`. The common Harp - registers are merged in and `device.registers` is derived automatically — do - not declare a `REGISTER_MAP` or override the base's protocol methods. + A subclass sets only `__whoami__` and `registers` — do not override the base's + protocol methods. """ __whoami__ = 1234 - __REGISTERS__ = (Encoder, Control) - - # Optional but recommended: makes `device.registers.` autocomplete and - # type-aware. Never instantiated — the base builds the namespace from `__REGISTERS__`. - class _ExampleRegisters(CoreRegistersNamespace): - Encoder: type[Encoder] - Control: type[Control] - - registers: ClassVar[_ExampleRegisters] + # A mutable ClassVar is invariant, so pyright rejects narrowing it even though + # `ExampleRegisters` is a `CoreRegisters`. The suppression is the cost of the + # precise type on `device.registers.`. + registers: ClassVar[ExampleRegisters] = ExampleRegisters() # pyright: ignore[reportIncompatibleVariableOverride] # Registers are reached by name, on the class or an instance: diff --git a/src/packages/harp-data/src/harp/data/_dataset.py b/src/packages/harp-data/src/harp/data/_dataset.py index 22b4072..9b54e45 100644 --- a/src/packages/harp-data/src/harp/data/_dataset.py +++ b/src/packages/harp-data/src/harp/data/_dataset.py @@ -6,7 +6,7 @@ from typing import Any import pandas as pd -from harp.device import Device, RegisterNamespace, create_device +from harp.device import Device, RegisterMap, create_device from harp.protocol import RegisterBase from harp.protocol._constants import _TIMESTAMP_FLAG @@ -82,7 +82,7 @@ def name(self) -> str: return self._name_override or self._device.__name__ @property - def registers(self) -> RegisterNamespace: + def registers(self) -> RegisterMap: """The device's registers, reachable by name (``reader.registers.WhoAmI``) or through the ``reader.registers.by_address`` map.""" return self._device.registers diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index 3e6c8a3..9b0783d 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -19,25 +19,35 @@ device.write(OperationControl, payload) # write a register ## Extending for a specific device -Downstream (often generated) packages declare their registers in a `__REGISTERS__` -tuple; the common Harp registers are merged in automatically. They may set +Downstream (often generated) packages declare their registers as attributes on a +`CoreRegisters` subclass, and assign an instance of it to `registers`. They may set `__whoami__` for identity validation on connect. Only these two attributes are meant -to be set — the base owns the protocol methods and register derivation: +to be set — the base owns the protocol methods: ```python -from harp.device import Device +from typing import ClassVar + +from harp.device import CoreRegisters, Device + + +class MyRegisters(CoreRegisters): + DigitalInputState = DigitalInputState + ... class MyDevice(Device): __whoami__ = 1216 - __REGISTERS__ = (DigitalInputState, ...) + # A mutable ClassVar is invariant, so narrowing needs a suppression. + registers: ClassVar[MyRegisters] = MyRegisters() # pyright: ignore[reportIncompatibleVariableOverride] ``` -Registers are then reached by name through `device.registers` -(`MyDevice.registers.DigitalInputState`) or through the -`MyDevice.registers.by_address` map. For static type hints on -`device.registers.`, subclass `CoreRegistersNamespace` and declare the device's -registers — see the [device examples](https://harp-tech.org/pyharp/examples/). +That single declaration is both the runtime register set — `RegisterMap` +introspects the class to build its name and address maps — and the static type, so +`MyDevice.registers.DigitalInputState` autocompletes and `read`/`write` infer the +payload type. Registers are also reachable through the `MyDevice.registers.by_name` +and `.by_address` maps. Subclassing `CoreRegisters` is what merges in the common Harp +registers; a device needing a different common set may subclass `RegisterMap` +directly. See the [device examples](https://harp-tech.org/pyharp/examples/). A new transport is just an object implementing the `ITransport` protocol (`open`/`write`/`read`/`close`). diff --git a/src/packages/harp-device/src/harp/device/__init__.py b/src/packages/harp-device/src/harp/device/__init__.py index cdeb5fa..d36d5cc 100644 --- a/src/packages/harp-device/src/harp/device/__init__.py +++ b/src/packages/harp-device/src/harp/device/__init__.py @@ -26,8 +26,8 @@ TimestampSeconds, WhoAmI, ) -from ._core_registers import CORE_REGISTERS, CoreRegistersNamespace -from ._register_namespace import RegisterNamespace +from ._core_registers import CORE_REGISTERS, CoreRegisters +from ._register_namespace import RegisterMap from ._schema import ConverterContext, parse_device_schema from ._transport import ITransport, TransportError @@ -41,8 +41,8 @@ "HarpFramer", "ITransport", "TransportError", - "RegisterNamespace", - "CoreRegistersNamespace", + "RegisterMap", + "CoreRegisters", "CORE_REGISTERS", "WhoAmI", "HardwareVersionHigh", diff --git a/src/packages/harp-device/src/harp/device/_core_registers.py b/src/packages/harp-device/src/harp/device/_core_registers.py index efcd588..5b63e71 100644 --- a/src/packages/harp-device/src/harp/device/_core_registers.py +++ b/src/packages/harp-device/src/harp/device/_core_registers.py @@ -2,7 +2,7 @@ Every :class:`~harp.device.Device` merges :data:`CORE_REGISTERS` with its own ``REGISTERS`` to build ``device.registers``. Statically generated devices subclass -:class:`CoreRegistersNamespace` to declare their device-specific registers with real types, +:class:`CoreRegisters` to declare their device-specific registers with real types, so ``device.registers.`` autocompletes and type-checks. """ @@ -10,7 +10,7 @@ from harp.protocol import RegisterBase -from ._register_namespace import RegisterNamespace +from ._register_namespace import RegisterMap from ._registers import ( AssemblyVersion, ClockConfiguration, @@ -49,34 +49,35 @@ ) -class CoreRegistersNamespace(RegisterNamespace): +class CoreRegisters(RegisterMap): """Typed register namespace declaring the common Harp registers. Every :class:`~harp.device.Device` exposes at least these as ``device.registers``. - A statically generated device subclasses this to add its own registers with real - types:: + A statically generated device subclasses this to add its own registers:: - class BehaviorRegisters(CoreRegistersNamespace): - DigitalInputState: type[DigitalInputState] - AnalogData: type[AnalogData] + class BehaviorRegisters(CoreRegisters): + DigitalInputState = DigitalInputState + AnalogData = AnalogData - so ``device.registers.AnalogData`` autocompletes and type-checks. At runtime the - namespace is populated from the device's registers; these annotations carry no - runtime values. + so ``device.registers.AnalogData`` autocompletes and type-checks (each member is + inferred as ``type[]``, exactly as a ``: type[...]`` annotation would + be). These are **assignments**, not annotations, because + :class:`~harp.device.RegisterMap` introspects the class for real attribute values + to build its name and address maps — a bare annotation carries none. """ - WhoAmI: type[WhoAmI] - HardwareVersionHigh: type[HardwareVersionHigh] - HardwareVersionLow: type[HardwareVersionLow] - AssemblyVersion: type[AssemblyVersion] - CoreVersionHigh: type[CoreVersionHigh] - CoreVersionLow: type[CoreVersionLow] - FirmwareVersionHigh: type[FirmwareVersionHigh] - FirmwareVersionLow: type[FirmwareVersionLow] - TimestampSeconds: type[TimestampSeconds] - TimestampMicroseconds: type[TimestampMicroseconds] - OperationControl: type[OperationControl] - ResetDevice: type[ResetDevice] - DeviceName: type[DeviceName] - SerialNumber: type[SerialNumber] - ClockConfiguration: type[ClockConfiguration] + WhoAmI = WhoAmI + HardwareVersionHigh = HardwareVersionHigh + HardwareVersionLow = HardwareVersionLow + AssemblyVersion = AssemblyVersion + CoreVersionHigh = CoreVersionHigh + CoreVersionLow = CoreVersionLow + FirmwareVersionHigh = FirmwareVersionHigh + FirmwareVersionLow = FirmwareVersionLow + TimestampSeconds = TimestampSeconds + TimestampMicroseconds = TimestampMicroseconds + OperationControl = OperationControl + ResetDevice = ResetDevice + DeviceName = DeviceName + SerialNumber = SerialNumber + ClockConfiguration = ClockConfiguration diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index 674d559..7c4330d 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -10,7 +10,7 @@ from harp.protocol._message import ParsedHarpMessage from harp.protocol._register import RegisterBase -from ._core_registers import CORE_REGISTERS, CoreRegistersNamespace +from ._core_registers import CoreRegisters from ._framer import HarpFramer from ._registers import ( WhoAmI, @@ -37,27 +37,6 @@ def _normalize_message_types(message_types: MessageTypeFilter) -> frozenset[Mess return frozenset(message_types) -class _RegisterAccessor: - """Read-only descriptor backing :attr:`Device.registers`. - - It defines ``__get__`` but no ``__set__``, which is deliberate: - - * ``device.registers`` is **read-only** — assigning to it is a type error; - * a subclass may **narrow** the attribute by redeclaring it, because a - read-only member is checked *covariantly* (a mutable one would be invariant). - - So a statically generated device can redeclare - ``registers: ClassVar[]`` to type - ``device.registers.`` precisely, with **no** - ``reportIncompatibleVariableOverride`` suppression. The real namespace is - assigned per subclass in :meth:`Device.__init_subclass__` (which shadows this - descriptor); this default only backs the bare :class:`Device` base. - """ - - def __get__(self, obj: object, owner: type | None = None) -> CoreRegistersNamespace: - return CoreRegistersNamespace(CORE_REGISTERS) - - class Subscription: """Handle returned by :meth:`Device.subscribe`. Cancel with :meth:`unsubscribe`, or use as a context manager to auto-cancel on exit.""" @@ -93,12 +72,27 @@ class Device: over an :class:`~harp.device.ITransport`. Must be opened before use, via ``with`` or :meth:`open`. A subclass declares its - device-specific registers in :attr:`__REGISTERS__` and sets :attr:`__whoami__` to - validate device identity on open (``0x0`` skips the check). Registers are reached - by name through :attr:`registers` (``device.registers.WhoAmI``). - - Only :attr:`__REGISTERS__` and :attr:`__whoami__` are meant to be set by a - subclass; the base owns the protocol methods and the register-namespace derivation. + registers by assigning :attr:`registers` an instance of its own + :class:`~harp.device.CoreRegisters` subclass, and sets :attr:`__whoami__` to + validate device identity on open (``0x0`` skips the check):: + + class ExampleRegisters(CoreRegisters): + Encoder = Encoder + Control = Control + + class ExampleDevice(Device): + __whoami__ = 1234 + registers = ExampleRegisters() + + That one declaration is both the runtime register set and the static type, so + ``device.registers.Encoder`` autocompletes and ``read``/``write`` infer the + payload type. Subclassing :class:`~harp.device.CoreRegisters` (rather than + :class:`~harp.device.RegisterMap`) is what merges in the common Harp registers; + a device needing a different common set may subclass :class:`RegisterMap` + directly. On an address clash the most-derived register wins. + + Only :attr:`registers` and :attr:`__whoami__` are meant to be set by a subclass; + the base owns the protocol methods. """ # === The following are class variables meant to be set by a subclass === @@ -106,31 +100,13 @@ class Device: #: Expected ``WhoAmI`` of the device this class models; ``0x0`` skips the check. __whoami__: ClassVar[int] = 0x0 - #: The device's own registers. A subclass sets this to a tuple of register - #: classes; the common Harp registers are merged in automatically. - __REGISTERS__: ClassVar[tuple[type[RegisterBase[Any]], ...]] = () - - #: Name-indexed, **read-only** view of all this device's registers (core + - #: ``__REGISTERS__``). Reach a register by name (``device.registers.WhoAmI`` — - #: the common registers autocomplete on any device), or use - #: ``device.registers.by_address``; see :class:`~harp.device.RegisterNamespace`. - #: A statically generated device may *narrow* this by redeclaring - #: ``registers: ClassVar[]`` for typed, autocompleting - registers = _RegisterAccessor() + #: Name-indexed view of this device's registers. The bare :class:`Device` exposes + #: only the common Harp registers; a subclass replaces this with an instance of + #: its own :class:`~harp.device.CoreRegisters` subclass. + registers: ClassVar[CoreRegisters] = CoreRegisters() REPLY_TIMEOUT: ClassVar[float] = 5.0 # seconds - def __init_subclass__(cls, **kwargs: Any) -> None: - super().__init_subclass__(**kwargs) - # Merge inherited registers (core + any parent's) with this class's own - # __REGISTERS__; on an address clash the device's register wins. - merged: dict[int, type[RegisterBase[Any]]] = dict(cls.registers.by_address) - for register in cls.__dict__.get("__REGISTERS__", ()): - merged[register.address] = register - # ``type.__setattr__`` (not ``cls.registers = ...``) keeps pyright treating - # ``registers`` as read-only; it shadows the base descriptor on the subclass. - type.__setattr__(cls, "registers", CoreRegistersNamespace(merged.values())) - def __init__(self, transport: ITransport, *, raise_on_error: bool = True) -> None: self._transport = transport self.raise_on_error = raise_on_error diff --git a/src/packages/harp-device/src/harp/device/_emit_device.py b/src/packages/harp-device/src/harp/device/_emit_device.py index 9a9d5af..14c3bf4 100644 --- a/src/packages/harp-device/src/harp/device/_emit_device.py +++ b/src/packages/harp-device/src/harp/device/_emit_device.py @@ -1,5 +1,8 @@ from typing import Any, Mapping, Optional, Union +from harp.protocol import RegisterBase + +from ._core_registers import CORE_REGISTERS, CoreRegisters from ._device import Device from ._schema import create_registers, parse_device_schema from ._schema._emit import ConverterValue @@ -16,21 +19,26 @@ def create_device( ) -> type[Device]: """Emit a :class:`Device` subclass from a device schema. - The returned class carries its device-specific registers in ``__REGISTERS__`` and - exposes all of them (merged with the common Harp registers) by name through - ``device.registers`` (e.g. ``Behavior.registers.AnalogData``). ``__whoami__`` comes - from the schema (``0x0`` when absent). On an address clash the device's register - wins over the common one. ``exclude_private=True`` drops registers whose DSL - ``visibility`` is ``private``. A header-less register fragment yields a device with - no ``device`` name (falls back to ``"Device"``). + The returned class exposes the schema's registers, merged with the common Harp + registers, by name through ``device.registers`` (e.g. + ``Behavior.registers.AnalogData``). + ``__whoami__`` comes from the schema (``0x0`` + when absent). On an address clash the device's register wins over the common one. + ``exclude_private=True`` drops registers whose DSL ``visibility`` is ``private``. + A header-less register fragment yields a device with no ``device`` name (falls + back to ``"Device"``). """ device = source if isinstance(source, DeviceModel) else parse_device_schema(source) registers = create_registers( device, converters=converters, strict=strict, exclude_private=exclude_private ) + merged: dict[int, type[RegisterBase[Any]]] = {reg.address: reg for reg in CORE_REGISTERS} + for register in registers.values(): + merged[register.address] = register + namespace: dict[str, Any] = { "__whoami__": int(device.whoAmI or 0), - "__REGISTERS__": tuple(registers.values()), + "registers": CoreRegisters.from_registers(merged.values()), } return type(name or device.device or "Device", (Device,), namespace) diff --git a/src/packages/harp-device/src/harp/device/_register_namespace.py b/src/packages/harp-device/src/harp/device/_register_namespace.py index 1a48009..c66f46b 100644 --- a/src/packages/harp-device/src/harp/device/_register_namespace.py +++ b/src/packages/harp-device/src/harp/device/_register_namespace.py @@ -1,49 +1,84 @@ """A name-indexed view over a device's register classes. -`Device.registers` is a :class:`RegisterNamespace`, so registers are reached by +`Device.registers` is a :class:`RegisterMap`, so registers are reached by name — ``device.registers.WhoAmI`` — with the ``by_name`` / ``by_address`` maps for programmatic lookup. Statically generated devices narrow the type to a -:class:`CoreRegistersNamespace` subclass so editors autocomplete the register names; see -:class:`CoreRegistersNamespace`. +:class:`~harp.device.CoreRegisters` subclass so editors autocomplete the register +names; see :class:`~harp.device.CoreRegisters` (which itself is a +:class:`RegisterMap`). """ from collections.abc import Iterable, Iterator, Mapping from types import MappingProxyType -from typing import Any +from typing import Any, Self from harp.protocol import RegisterBase _Register = type[RegisterBase[Any]] -class RegisterNamespace: +class RegisterMap: """Attribute-addressable collection of register classes. - Built from an iterable of register classes; each is indexed by its - ``__name__`` and its ``address``. Registers are reached by name:: + A subclass **declares** its registers as class attributes; instantiating it + introspects the class and indexes each one by its ``__name__`` and its + ``address``:: - ns = RegisterNamespace([WhoAmI, OperationControl]) - ns.WhoAmI # -> type[WhoAmI] (attribute access) + class MyRegisters(RegisterMap): + WhoAmI = WhoAmI + OperationControl = OperationControl + + ns = MyRegisters() + ns.WhoAmI # -> type[WhoAmI] (autocompletes and type-checks) ns.by_name # {"WhoAmI": WhoAmI, "OperationControl": OperationControl} ns.by_address # {0: WhoAmI, 10: OperationControl} + Declaring them as assignments rather than ``WhoAmI: type[WhoAmI]`` annotations + is what makes this work: a bare annotation carries no runtime value, so there + would be nothing to introspect. Static typing is unaffected — each member is + still inferred as ``type[]``. + + :meth:`from_registers` builds the same map from an iterable instead, for + dynamically generated devices whose register set isn't known at author time. + Iteration yields the register classes, and ``in`` tests register-class membership (``WhoAmI in ns``). Attribute access falls back to - :meth:`__getattr__`, typed as ``type[RegisterBase[Any]]`` so any register name - type-checks; a :class:`CoreRegistersNamespace` subclass declares specific names for - precise types. + :meth:`__getattr__`, typed as ``type[RegisterBase[Any]]``, so a register that + only exists at runtime still type-checks. """ - def __init__(self, registers: Iterable[_Register]) -> None: - self._registers = tuple(registers) - # We use MappingProxyType here to make the maps read-only, so they can't be - # accidentally mutated at runtime. - self._by_name: Mapping[str, _Register] = MappingProxyType( - {register.__name__: register for register in self._registers} - ) - self._by_address: Mapping[int, _Register] = MappingProxyType( - {register.address: register for register in self._registers} - ) + _by_name: Mapping[str, _Register] + _by_address: Mapping[int, _Register] + + def __init__(self) -> None: + self._registers = tuple(self._resolve_register_map()) + self._resolve_mappings() + + @classmethod + def from_registers(cls, registers: Iterable[_Register]) -> Self: + """Construct a :class:`RegisterMap` from an iterable of register classes. + Useful for dynamically generated devices that don't have a static register list.""" + instance = cls.__new__(cls) + instance._registers = tuple(registers) + instance._resolve_mappings() + return instance + + def _resolve_mappings(self) -> None: + """Resolve the name and address mappings from the register list.""" + # MappingProxyType makes the maps read-only, so they can't be accidentally + # mutated at runtime. + self._by_name = MappingProxyType({reg.__name__: reg for reg in self._registers}) + self._by_address = MappingProxyType({reg.address: reg for reg in self._registers}) + + @classmethod + def _resolve_register_map(cls) -> Iterator[_Register]: + """Yield every register class declared as an attribute on ``cls`` or its bases.""" + for attr_name in dir(cls): + if attr_name.startswith("_"): + continue + attr = getattr(cls, attr_name) + if isinstance(attr, type) and issubclass(attr, RegisterBase): + yield attr def __getattr__(self, name: str) -> _Register: # Only consulted when normal attribute lookup fails, so real methods and diff --git a/tests/device/expected_device.py b/tests/device/expected_device.py index 5629de2..b3cda17 100644 --- a/tests/device/expected_device.py +++ b/tests/device/expected_device.py @@ -5,7 +5,7 @@ from typing import ClassVar import numpy as np -from harp.device import CoreRegistersNamespace, Device +from harp.device import CoreRegisters, Device from harp.protocol import ( AnonymousPayload, BitMask, @@ -236,42 +236,23 @@ class Tests(Device): """A device driven by its own registers; the common Harp registers are merged in automatically. Registers are reached by name — ``Tests.registers.AnalogData``.""" - __REGISTERS__ = ( - DigitalInputs, - AnalogData, - ComplexConfiguration, - Version, - CustomPayload, - CustomRawPayload, - CustomMemberConverter, - BitmaskSplitter, - Counter0, - PortDIOSet, - PulseDOPort0, - PulseDO0, - StartPulse, - StartPulseTrain, - EncoderMode, - ) - - # Facade declaring the device's registers with real types so editors autocomplete - # `device.registers.` and `read`/`write` infer the payload type. Never - # instantiated — the namespace is built from `__REGISTERS__` by the base. - class _Registers(CoreRegistersNamespace): - DigitalInputs: type[DigitalInputs] - AnalogData: type[AnalogData] - ComplexConfiguration: type[ComplexConfiguration] - Version: type[Version] - CustomPayload: type[CustomPayload] - CustomRawPayload: type[CustomRawPayload] - CustomMemberConverter: type[CustomMemberConverter] - BitmaskSplitter: type[BitmaskSplitter] - Counter0: type[Counter0] - PortDIOSet: type[PortDIOSet] - PulseDOPort0: type[PulseDOPort0] - PulseDO0: type[PulseDO0] - StartPulse: type[StartPulse] - StartPulseTrain: type[StartPulseTrain] - EncoderMode: type[EncoderMode] - - registers: ClassVar[_Registers] + class _Registers(CoreRegisters): + DigitalInputs = DigitalInputs + AnalogData = AnalogData + ComplexConfiguration = ComplexConfiguration + Version = Version + CustomPayload = CustomPayload + CustomRawPayload = CustomRawPayload + CustomMemberConverter = CustomMemberConverter + BitmaskSplitter = BitmaskSplitter + Counter0 = Counter0 + PortDIOSet = PortDIOSet + PulseDOPort0 = PulseDOPort0 + PulseDO0 = PulseDO0 + StartPulse = StartPulse + StartPulseTrain = StartPulseTrain + EncoderMode = EncoderMode + + # A mutable ClassVar is invariant, so narrowing the base's `registers` needs a + # suppression; it buys the precise type on `device.registers.`. + registers: ClassVar[_Registers] = _Registers() # pyright: ignore[reportIncompatibleVariableOverride] diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py index 8782b82..2ed0085 100644 --- a/tests/device/test_device_emit.py +++ b/tests/device/test_device_emit.py @@ -1,13 +1,27 @@ from typing import ClassVar import pytest -from harp.device import Device, create_device +from harp.device import CoreRegisters, Device, create_device +from harp.protocol import RegisterU16 from .converters import DataConverter CONVERTERS = {"DataConverter": DataConverter()} +class Counter(RegisterU16): + address: ClassVar[int] = 40 + + +class _StaticRegisters(CoreRegisters): + Counter = Counter + + +class StaticDevice(Device): + __whoami__ = 42 + registers: ClassVar[_StaticRegisters] = _StaticRegisters() # pyright: ignore[reportIncompatibleVariableOverride] + + @pytest.fixture def test_device(device_yml): return create_device(device_yml, converters=CONVERTERS) @@ -71,23 +85,13 @@ def test_registers_iterates_register_classes(test_device): def test_static_device_subclass_derives_registers(): - # A hand-written (or generated) static device: declare __REGISTERS__ and the - # base merges in the common registers and builds `registers`. No REGISTER_MAP. from harp.device import WhoAmI - from harp.protocol import RegisterU16 - - class Counter(RegisterU16): - address: ClassVar[int] = 40 - - class MyDevice(Device): - __whoami__ = 42 - __REGISTERS__ = (Counter,) - assert MyDevice.__whoami__ == 42 - assert MyDevice.registers.Counter is Counter # device register, by name - assert MyDevice.registers.WhoAmI is WhoAmI # common register, merged in - assert Counter in MyDevice.registers # membership by register class - assert MyDevice.registers.by_address[40] is Counter + assert StaticDevice.__whoami__ == 42 + assert StaticDevice.registers.Counter is Counter # device register, by name + assert StaticDevice.registers.WhoAmI is WhoAmI # common register, inherited + assert Counter in StaticDevice.registers # membership by register class + assert StaticDevice.registers.by_address[40] is Counter def test_device_register_overrides_core_on_clash(): diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index ca63a74..9063f32 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -3,7 +3,7 @@ import numpy as np import pytest from harp.data import parse_to_dataframe -from harp.protocol import HarpMessage +from harp.protocol import HarpMessage, RegisterBase from harp.device._schema import UnknownConverterError, create_registers @@ -19,10 +19,15 @@ def device_registers(device_yml): def _device_registers(): - # expected_device.Tests.__REGISTERS__ holds only the device-specific registers - # (the ones the emitter builds from device.yml); the core ones are merged in - # by Device automatically. - return {cls.__name__: cls for cls in expected_device.Tests.__REGISTERS__} + # expected_device.Tests._Registers declares only the device-specific registers + # (the ones the emitter builds from device.yml); the core ones are inherited from + # its CoreRegisters base, so read this class's own __dict__ rather than the + # resolved namespace. + return { + value.__name__: value + for name, value in vars(expected_device.Tests._Registers).items() + if not name.startswith("_") and isinstance(value, type) and issubclass(value, RegisterBase) + } def _layout(dt): From 7983d3c89e3104d04ea5e2281bd6b9bd5ccbac21 Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:18:57 -0700 Subject: [PATCH 21/22] Refactor device class to allow type hinting of register map --- docs/examples/static_device/static_device.md | 32 ++++++++--- docs/examples/static_device/static_device.py | 7 +-- .../harp-data/src/harp/data/_dataset.py | 16 +++--- src/packages/harp-device/README.md | 14 +++-- src/packages/harp-device/pyproject.toml | 1 + .../harp-device/src/harp/device/__init__.py | 3 +- .../harp-device/src/harp/device/_device.py | 37 +++++++------ .../src/harp/device/_emit_device.py | 24 +++++++-- .../harp-serial/src/harp/serial/_serial.py | 10 ++-- tests/device/expected_device.py | 54 +++++++++++-------- tests/device/test_device_emit.py | 4 +- tests/device/test_emit.py | 4 +- uv.lock | 2 + 13 files changed, 126 insertions(+), 82 deletions(-) diff --git a/docs/examples/static_device/static_device.md b/docs/examples/static_device/static_device.md index e529d7b..d7b075b 100644 --- a/docs/examples/static_device/static_device.md +++ b/docs/examples/static_device/static_device.md @@ -23,8 +23,28 @@ A static device is four kinds of declaration: - **The register namespace** — a `CoreRegisters` subclass **assigning** each of the device's own registers (`Encoder = Encoder`). Subclassing `CoreRegisters` merges in the common Harp registers. -- **The `Device` subclass** — sets **only** `__whoami__` (the expected identity) and - `registers` (an instance of the namespace class). +- **The `Device` subclass** — names the namespace as its type parameter + (`Device[ExampleRegisters]`) and sets **only** `__whoami__` (the expected identity) + and `registers` (an instance of the namespace class). + +The namespace class is the single source of truth: one declaration is both the +runtime register set and the static type, so the two can't drift. + +Two details that look arbitrary but aren't. Members are **assignments** +(`Encoder = Encoder`), not `Encoder: type[Encoder]` annotations, because +`RegisterMap` introspects the class for real attribute values — an annotation-only +namespace resolves empty. Nothing is lost: a checker infers the assignment as +`type[Encoder]` either way. And because the right-hand side resolves through module +globals, register classes must live at module scope; `Encoder = Encoder` in a class +body nested inside a function raises `NameError`. + +The namespace goes in as `Device`'s **type parameter** rather than a re-annotation of +`registers`. Re-declaring `registers: ClassVar[ExampleRegisters]` in the subclass +type-checks *worse*: a mutable attribute is invariant under override, so narrowing it +raises `reportIncompatibleVariableOverride` and every generated device would need a +suppression. Specializing a parameter isn't an override, so nothing needs suppressing +— and the checker verifies the assigned instance matches the parameter, making +`Device[ARegisters]` with `registers = BRegisters()` an error. ## What the base gives you @@ -50,11 +70,9 @@ This is the contract a generator targets. Per device, emit: module scope. 2. A namespace subclassing `CoreRegisters`, one `Name = Name` assignment per device register — **not** the common ones, which come from the base. -3. A `Device` subclass setting only `__whoami__` and - `registers: ClassVar[] = ()`. Narrowing `registers` needs a - trailing `# pyright: ignore[reportIncompatibleVariableOverride]` — a mutable - `ClassVar` is invariant, so a type checker rejects the narrowing even though the - namespace *is* a `CoreRegisters`. +3. A `Device[]` subclass setting only `__whoami__` and + `registers = ()`. Emit the namespace as the **type parameter**, never + as a re-annotation of `registers`. Generators must **not** spread the common registers into the namespace, or override any base `Device` method. diff --git a/docs/examples/static_device/static_device.py b/docs/examples/static_device/static_device.py index bbd8921..90290cc 100644 --- a/docs/examples/static_device/static_device.py +++ b/docs/examples/static_device/static_device.py @@ -59,7 +59,7 @@ class ExampleRegisters(CoreRegisters): # --- Device ------------------------------------------------------------------ -class ExampleDevice(Device): +class ExampleDevice(Device[ExampleRegisters]): """A statically defined Harp device. A subclass sets only `__whoami__` and `registers` — do not override the base's @@ -67,10 +67,7 @@ class ExampleDevice(Device): """ __whoami__ = 1234 - # A mutable ClassVar is invariant, so pyright rejects narrowing it even though - # `ExampleRegisters` is a `CoreRegisters`. The suppression is the cost of the - # precise type on `device.registers.`. - registers: ClassVar[ExampleRegisters] = ExampleRegisters() # pyright: ignore[reportIncompatibleVariableOverride] + registers = ExampleRegisters() # Registers are reached by name, on the class or an instance: diff --git a/src/packages/harp-data/src/harp/data/_dataset.py b/src/packages/harp-data/src/harp/data/_dataset.py index 9b54e45..1d28e64 100644 --- a/src/packages/harp-data/src/harp/data/_dataset.py +++ b/src/packages/harp-data/src/harp/data/_dataset.py @@ -3,10 +3,10 @@ from datetime import datetime from os import PathLike from pathlib import Path -from typing import Any +from typing import Any, Generic import pandas as pd -from harp.device import Device, RegisterMap, create_device +from harp.device import Device, TRegisterMap, create_device from harp.protocol import RegisterBase from harp.protocol._constants import _TIMESTAMP_FLAG @@ -31,7 +31,7 @@ def default_file_resolver(root: Path, name: str) -> dict[int, list[Path]]: return files -class DatasetReader: +class DatasetReader(Generic[TRegisterMap]): """Reader over a de-multiplexed Harp dataset folder. Construct from a generated device and a dataset folder, then read a register's @@ -54,13 +54,13 @@ class DatasetReader: def __init__( self, - device: type[Device], + device: type[Device[TRegisterMap]], root: str | PathLike[str], *, name: str | None = None, resolver: FileNameResolver = default_file_resolver, ) -> None: - self._device = device + self._device: type[Device[TRegisterMap]] = device self._root = Path(root) self._name_override = name self._resolver = resolver @@ -72,7 +72,7 @@ def root(self) -> Path: return self._root @property - def device(self) -> type[Device]: + def device(self) -> type[Device[TRegisterMap]]: """The generated device this reader parses against.""" return self._device @@ -82,10 +82,10 @@ def name(self) -> str: return self._name_override or self._device.__name__ @property - def registers(self) -> RegisterMap: + def registers(self) -> TRegisterMap: """The device's registers, reachable by name (``reader.registers.WhoAmI``) or through the ``reader.registers.by_address`` map.""" - return self._device.registers + return getattr(self._device, "registers") @property def files(self) -> Mapping[int, list[Path]]: diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index 9b0783d..c4b5fc2 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -20,13 +20,12 @@ device.write(OperationControl, payload) # write a register ## Extending for a specific device Downstream (often generated) packages declare their registers as attributes on a -`CoreRegisters` subclass, and assign an instance of it to `registers`. They may set -`__whoami__` for identity validation on connect. Only these two attributes are meant -to be set — the base owns the protocol methods: +`CoreRegisters` subclass, name that class as `Device`'s type parameter, and assign an +instance of it to `registers`. They may set `__whoami__` for identity validation on +connect. Only these two attributes are meant to be set — the base owns the protocol +methods: ```python -from typing import ClassVar - from harp.device import CoreRegisters, Device @@ -35,10 +34,9 @@ class MyRegisters(CoreRegisters): ... -class MyDevice(Device): +class MyDevice(Device[MyRegisters]): __whoami__ = 1216 - # A mutable ClassVar is invariant, so narrowing needs a suppression. - registers: ClassVar[MyRegisters] = MyRegisters() # pyright: ignore[reportIncompatibleVariableOverride] + registers = MyRegisters() ``` That single declaration is both the runtime register set — `RegisterMap` diff --git a/src/packages/harp-device/pyproject.toml b/src/packages/harp-device/pyproject.toml index 8efea01..ed80583 100644 --- a/src/packages/harp-device/pyproject.toml +++ b/src/packages/harp-device/pyproject.toml @@ -7,6 +7,7 @@ dependencies = [ "harp-protocol", "pydantic>=2", "pydantic-yaml>=1", + "typing-extensions>=4.15.0", # For PEP 696 TypeVar defaults. Remove this once the floor moves to 3.13. ] [build-system] diff --git a/src/packages/harp-device/src/harp/device/__init__.py b/src/packages/harp-device/src/harp/device/__init__.py index d36d5cc..0e6933a 100644 --- a/src/packages/harp-device/src/harp/device/__init__.py +++ b/src/packages/harp-device/src/harp/device/__init__.py @@ -1,4 +1,4 @@ -from ._device import Device, EventHandler, Subscription +from ._device import Device, EventHandler, Subscription, TRegisterMap from ._emit_device import create_device from ._framer import HarpFramer from ._registers import ( @@ -67,4 +67,5 @@ "ClockConfigurationFlags", "ClockConfigurationPayload", "SerialNumber", + "TRegisterMap", ] diff --git a/src/packages/harp-device/src/harp/device/_device.py b/src/packages/harp-device/src/harp/device/_device.py index 7c4330d..f888234 100644 --- a/src/packages/harp-device/src/harp/device/_device.py +++ b/src/packages/harp-device/src/harp/device/_device.py @@ -4,7 +4,9 @@ import queue import threading from collections.abc import Callable, Iterable -from typing import Any, ClassVar, Self, TypeVar +from typing import Any, ClassVar, Generic, Self, cast + +from typing_extensions import TypeVar from harp.protocol import HarpMessage, MessageType from harp.protocol._message import ParsedHarpMessage @@ -12,12 +14,14 @@ from ._core_registers import CoreRegisters from ._framer import HarpFramer +from ._register_namespace import RegisterMap from ._registers import ( WhoAmI, ) from ._transport import ITransport, TransportError P = TypeVar("P") +TRegisterMap = TypeVar("TRegisterMap", bound=RegisterMap, default=CoreRegisters) _logger = logging.getLogger(__name__) @@ -43,7 +47,7 @@ class Subscription: def __init__( self, - device: "Device", + device: "Device[Any]", address: int | None, handler: Callable[[Any], None], message_types: frozenset[MessageType], @@ -67,43 +71,46 @@ def __exit__(self, *args: object) -> None: self.unsubscribe() -class Device: +class Device(Generic[TRegisterMap]): """Harp device protocol logic (framing, request/reply, register access) over an :class:`~harp.device.ITransport`. Must be opened before use, via ``with`` or :meth:`open`. A subclass declares its - registers by assigning :attr:`registers` an instance of its own - :class:`~harp.device.CoreRegisters` subclass, and sets :attr:`__whoami__` to - validate device identity on open (``0x0`` skips the check):: + registers **once**, as a :class:`~harp.device.CoreRegisters` subclass, then names + that class as the type parameter and assigns an instance of it. It sets + :attr:`__whoami__` to validate device identity on open (``0x0`` skips the check):: class ExampleRegisters(CoreRegisters): Encoder = Encoder Control = Control - class ExampleDevice(Device): + class ExampleDevice(Device[ExampleRegisters]): __whoami__ = 1234 registers = ExampleRegisters() - That one declaration is both the runtime register set and the static type, so + The namespace class is both the runtime register set and the static type, so ``device.registers.Encoder`` autocompletes and ``read``/``write`` infer the - payload type. Subclassing :class:`~harp.device.CoreRegisters` (rather than + payload type. Passing it as the type parameter rather than re-annotating + ``registers`` is deliberate: a specialized parameter is not an override, so this + needs no ``reportIncompatibleVariableOverride`` suppression, and a type checker + verifies the assigned instance matches the parameter. + + Subclassing :class:`~harp.device.CoreRegisters` (rather than :class:`~harp.device.RegisterMap`) is what merges in the common Harp registers; a device needing a different common set may subclass :class:`RegisterMap` directly. On an address clash the most-derived register wins. + For a device built at runtime from a schema, see + :func:`~harp.device.create_device`. + Only :attr:`registers` and :attr:`__whoami__` are meant to be set by a subclass; the base owns the protocol methods. """ - # === The following are class variables meant to be set by a subclass === - #: Expected ``WhoAmI`` of the device this class models; ``0x0`` skips the check. __whoami__: ClassVar[int] = 0x0 - #: Name-indexed view of this device's registers. The bare :class:`Device` exposes - #: only the common Harp registers; a subclass replaces this with an instance of - #: its own :class:`~harp.device.CoreRegisters` subclass. - registers: ClassVar[CoreRegisters] = CoreRegisters() + registers: TRegisterMap = cast(Any, CoreRegisters()) REPLY_TIMEOUT: ClassVar[float] = 5.0 # seconds diff --git a/src/packages/harp-device/src/harp/device/_emit_device.py b/src/packages/harp-device/src/harp/device/_emit_device.py index 14c3bf4..99053c5 100644 --- a/src/packages/harp-device/src/harp/device/_emit_device.py +++ b/src/packages/harp-device/src/harp/device/_emit_device.py @@ -1,4 +1,5 @@ -from typing import Any, Mapping, Optional, Union +from types import new_class +from typing import TYPE_CHECKING, Any, Mapping, Optional, Union from harp.protocol import RegisterBase @@ -8,6 +9,14 @@ from ._schema._emit import ConverterValue from ._schema._model import DeviceModel +if TYPE_CHECKING: + # A type-checker-only stand-in for the class `create_device` returns. + # + # It exists so callers can reach `registers` on the returned *class*, before + # opening an instance (`Dev.registers.by_address[40]`). + class _AnonymousDevice(Device[CoreRegisters]): + pass + def create_device( source: Union[str, DeviceModel], @@ -16,12 +25,14 @@ def create_device( converters: Optional[Mapping[str, ConverterValue]] = None, strict: bool = True, exclude_private: bool = True, -) -> type[Device]: +) -> "type[_AnonymousDevice]": """Emit a :class:`Device` subclass from a device schema. The returned class exposes the schema's registers, merged with the common Harp registers, by name through ``device.registers`` (e.g. - ``Behavior.registers.AnalogData``). + ``Behavior.registers.AnalogData``). Because the names come from the schema at + runtime they don't autocomplete, and resolve as ``type[RegisterBase[Any]]`` — a + statically written device (see :class:`Device`) types them precisely. ``__whoami__`` comes from the schema (``0x0`` when absent). On an address clash the device's register wins over the common one. ``exclude_private=True`` drops registers whose DSL ``visibility`` is ``private``. @@ -41,4 +52,9 @@ def create_device( "__whoami__": int(device.whoAmI or 0), "registers": CoreRegisters.from_registers(merged.values()), } - return type(name or device.device or "Device", (Device,), namespace) + + return new_class( + name or device.device or "UnknownDevice", + (Device[CoreRegisters],), + exec_body=lambda ns: ns.update(namespace), + ) diff --git a/src/packages/harp-serial/src/harp/serial/_serial.py b/src/packages/harp-serial/src/harp/serial/_serial.py index 45d5b9a..57c1c0d 100644 --- a/src/packages/harp-serial/src/harp/serial/_serial.py +++ b/src/packages/harp-serial/src/harp/serial/_serial.py @@ -1,12 +1,8 @@ """Serial transport and factory for Harp devices.""" -from typing import TypeVar - import serial -from harp.device import Device, TransportError - -D = TypeVar("D", bound=Device) +from harp.device import Device, TRegisterMap, TransportError DEFAULT_BAUDRATE: int = 1_000_000 @@ -52,12 +48,12 @@ def close(self) -> None: def open_serial_device( - device: type[D], + device: type[Device[TRegisterMap]], *, port: str, baudrate: int = DEFAULT_BAUDRATE, raise_on_error: bool = True, -) -> D: +) -> Device[TRegisterMap]: """Build ``device`` over a serial transport and open it. Like the builtin :func:`open`, the returned device is already connected; diff --git a/tests/device/expected_device.py b/tests/device/expected_device.py index b3cda17..91c4bab 100644 --- a/tests/device/expected_device.py +++ b/tests/device/expected_device.py @@ -232,27 +232,35 @@ class EncoderMode(RegisterBase[EncoderModeMask]): payload_class = EncoderModePayload -class Tests(Device): +class TestsRegisters(CoreRegisters): + """The device's registers, declared once — as assignments, so the class carries + real values for the namespace to introspect while each member still types as + ``type[]``. Subclassing ``CoreRegisters`` merges in the common Harp + registers.""" + + DigitalInputs = DigitalInputs + AnalogData = AnalogData + ComplexConfiguration = ComplexConfiguration + Version = Version + CustomPayload = CustomPayload + CustomRawPayload = CustomRawPayload + CustomMemberConverter = CustomMemberConverter + BitmaskSplitter = BitmaskSplitter + Counter0 = Counter0 + PortDIOSet = PortDIOSet + PulseDOPort0 = PulseDOPort0 + PulseDO0 = PulseDO0 + StartPulse = StartPulse + StartPulseTrain = StartPulseTrain + EncoderMode = EncoderMode + + +class Tests(Device[TestsRegisters]): """A device driven by its own registers; the common Harp registers are merged - in automatically. Registers are reached by name — ``Tests.registers.AnalogData``.""" - - class _Registers(CoreRegisters): - DigitalInputs = DigitalInputs - AnalogData = AnalogData - ComplexConfiguration = ComplexConfiguration - Version = Version - CustomPayload = CustomPayload - CustomRawPayload = CustomRawPayload - CustomMemberConverter = CustomMemberConverter - BitmaskSplitter = BitmaskSplitter - Counter0 = Counter0 - PortDIOSet = PortDIOSet - PulseDOPort0 = PulseDOPort0 - PulseDO0 = PulseDO0 - StartPulse = StartPulse - StartPulseTrain = StartPulseTrain - EncoderMode = EncoderMode - - # A mutable ClassVar is invariant, so narrowing the base's `registers` needs a - # suppression; it buys the precise type on `device.registers.`. - registers: ClassVar[_Registers] = _Registers() # pyright: ignore[reportIncompatibleVariableOverride] + in automatically. Registers are reached by name — ``Tests.registers.AnalogData``. + + Naming the namespace as the type parameter (rather than re-annotating + ``registers``) is what keeps this suppression-free: specializing is not an + override, and the checker verifies the assigned instance matches the parameter.""" + + registers = TestsRegisters() diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py index 2ed0085..cdf778a 100644 --- a/tests/device/test_device_emit.py +++ b/tests/device/test_device_emit.py @@ -17,9 +17,9 @@ class _StaticRegisters(CoreRegisters): Counter = Counter -class StaticDevice(Device): +class StaticDevice(Device[_StaticRegisters]): __whoami__ = 42 - registers: ClassVar[_StaticRegisters] = _StaticRegisters() # pyright: ignore[reportIncompatibleVariableOverride] + registers = _StaticRegisters() @pytest.fixture diff --git a/tests/device/test_emit.py b/tests/device/test_emit.py index 9063f32..b2e5a7a 100644 --- a/tests/device/test_emit.py +++ b/tests/device/test_emit.py @@ -19,13 +19,13 @@ def device_registers(device_yml): def _device_registers(): - # expected_device.Tests._Registers declares only the device-specific registers + # expected_device.TestsRegisters declares only the device-specific registers # (the ones the emitter builds from device.yml); the core ones are inherited from # its CoreRegisters base, so read this class's own __dict__ rather than the # resolved namespace. return { value.__name__: value - for name, value in vars(expected_device.Tests._Registers).items() + for name, value in vars(expected_device.TestsRegisters).items() if not name.startswith("_") and isinstance(value, type) and issubclass(value, RegisterBase) } diff --git a/uv.lock b/uv.lock index bff724e..a865e07 100644 --- a/uv.lock +++ b/uv.lock @@ -422,6 +422,7 @@ dependencies = [ { name = "harp-protocol" }, { name = "pydantic" }, { name = "pydantic-yaml" }, + { name = "typing-extensions" }, ] [package.metadata] @@ -429,6 +430,7 @@ requires-dist = [ { name = "harp-protocol", editable = "src/packages/harp-protocol" }, { name = "pydantic", specifier = ">=2" }, { name = "pydantic-yaml", specifier = ">=1" }, + { name = "typing-extensions", specifier = ">=4.15.0" }, ] [[package]] From 12383e17405ad2557287d712bdf22612585addbf Mon Sep 17 00:00:00 2001 From: bruno-f-cruz <7049351+bruno-f-cruz@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:21:24 -0700 Subject: [PATCH 22/22] Fix test --- tests/device/test_device_emit.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/device/test_device_emit.py b/tests/device/test_device_emit.py index cdf778a..1d817c7 100644 --- a/tests/device/test_device_emit.py +++ b/tests/device/test_device_emit.py @@ -104,9 +104,10 @@ def test_device_register_overrides_core_on_clash(): def test_headerless_fragment_builds_default_device(): - # A register-only fragment is a valid (nameless) device; name falls back to "Device". + # A register-only fragment is a valid (nameless) device; the name falls back to + # "UnknownDevice". Dev = create_device("registers:\n Foo: {address: 40, type: U16, access: Read}\n") - assert Dev.__name__ == "Device" + assert Dev.__name__ == "UnknownDevice" assert Dev.__whoami__ == 0 assert Dev.registers.Foo.address == 40