From 9550dbb4b8aa54d0f25fa4b2729f91bdd151d3bc Mon Sep 17 00:00:00 2001 From: glopesdev Date: Wed, 19 Aug 2026 10:39:38 +0100 Subject: [PATCH 1/3] Parameterize HarpMessage by its payload type HarpMessage is now generic over the type of its payload, and the ParsedHarpMessage subclass is removed. A message read from the wire is a HarpMessage[Any] and decoding it with a register yields a HarpMessage[P], so read, write and subscribe all deliver one type carrying both the frame and the decoded value. payload is now that decoded value, replacing parsed, and the byte view it displaces becomes raw_payload. Reading payload before a register has decoded the message raises rather than falling back to the bytes, and has_payload reports which state a message is in. with_payload attaches a decoded value to a copy, so decoding never mutates a frame the dispatch loop has already handed elsewhere. --- README.md | 4 +- .../create_device_module.py | 2 +- docs/examples/get_info/get_info.py | 4 +- .../read_and_write_from_registers.py | 6 +- .../subscribing_to_events.md | 2 +- .../subscribing_to_events.py | 10 +-- src/packages/harp-device/README.md | 2 +- .../src/harp/device/client/_device.py | 25 +++--- .../src/harp/protocol/__init__.py | 3 +- .../src/harp/protocol/_message.py | 87 ++++++++++--------- .../src/harp/protocol/_register.py | 2 +- src/packages/harp-serial/README.md | 4 +- tests/conformance.py | 10 +-- tests/protocol/test_framer.py | 6 +- tests/protocol/test_message.py | 41 ++++++++- tests/protocol/test_register.py | 10 +-- 16 files changed, 125 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index 7ffb633..a340ffd 100644 --- a/README.md +++ b/README.md @@ -56,8 +56,8 @@ from harp.device import behavior, core # Use "COMx" on Windows, "/dev/ttyUSBx" on Linux. with serial.open_device(behavior, port="COM3") as device: - print(device.read(core.WhoAmI).parsed) # a common register - print(device.read(behavior.AnalogData).parsed) # a device register + print(device.read(core.WhoAmI).payload) # a common register + print(device.read(behavior.AnalogData).payload) # a device register device.write( core.OperationControl, core.OperationControlPayload(operation_mode=core.OperationMode.ACTIVE), diff --git a/docs/examples/create_device_module/create_device_module.py b/docs/examples/create_device_module/create_device_module.py index 7c5af65..b0a7636 100644 --- a/docs/examples/create_device_module/create_device_module.py +++ b/docs/examples/create_device_module/create_device_module.py @@ -21,7 +21,7 @@ # any `Device` over a transport. Passing the module itself validates the device # identity on open, against its `WHO_AM_I`, which a value of `0` skips. with serial.open_device(behavior, port=SERIAL_PORT) as device: - print("AnalogData:", device.read(AnalogData).parsed) + print("AnalogData:", device.read(AnalogData).payload) # The same register classes also decode a recorded binary dump into a pandas # DataFrame. See the "Reading Data into a DataFrame" example for more. diff --git a/docs/examples/get_info/get_info.py b/docs/examples/get_info/get_info.py index ca9109a..77f439f 100755 --- a/docs/examples/get_info/get_info.py +++ b/docs/examples/get_info/get_info.py @@ -7,9 +7,9 @@ # check, so this works against any device. The connection closes on exit. with serial.open_device(port=SERIAL_PORT) as device: # Identify the device. - print("WhoAmI:", device.read(core.WhoAmI).parsed) + print("WhoAmI:", device.read(core.WhoAmI).payload) # Dump every core register. for address, register in sorted(core.REGISTER_MAP.items()): reply = device.read(register) - print(f"{register.__name__:24s} (addr {address:2d}) = {reply.parsed}") + print(f"{register.__name__:24s} (addr {address:2d}) = {reply.payload}") diff --git a/docs/examples/read_and_write_from_registers/read_and_write_from_registers.py b/docs/examples/read_and_write_from_registers/read_and_write_from_registers.py index 99ab010..63ff3c6 100755 --- a/docs/examples/read_and_write_from_registers/read_and_write_from_registers.py +++ b/docs/examples/read_and_write_from_registers/read_and_write_from_registers.py @@ -5,10 +5,10 @@ with serial.open_device(client.Device, port=SERIAL_PORT) as device: # Read a scalar register. - print("WhoAmI:", device.read(core.WhoAmI).parsed) + print("WhoAmI:", device.read(core.WhoAmI).payload) # Read a structured register and inspect a field. - control = device.read(core.OperationControl).parsed + control = device.read(core.OperationControl).payload print("operation_mode before:", control.operation_mode) # Write the register, then read it back to confirm the change. A struct payload @@ -24,5 +24,5 @@ heartbeat=core.EnableFlag.DISABLED, ), ) - control = device.read(core.OperationControl).parsed + control = device.read(core.OperationControl).payload print("operation_mode after:", control.operation_mode) diff --git a/docs/examples/subscribing_to_events/subscribing_to_events.md b/docs/examples/subscribing_to_events/subscribing_to_events.md index ce11dcb..0df82f9 100644 --- a/docs/examples/subscribing_to_events/subscribing_to_events.md +++ b/docs/examples/subscribing_to_events/subscribing_to_events.md @@ -2,7 +2,7 @@ This example demonstrates how to react to messages pushed by the device, e.g. unsolicited `Event` messages, without polling, using two subscription styles: -- `device.subscribe(register, handler)`, where the handler receives a typed, parsed `ParsedHarpMessage` for a single register. +- `device.subscribe(register, handler)`, where the handler receives a `HarpMessage` typed by the payload of a single register. - `device.subscribe_all(handler)`, a catch-all handler that receives the raw `HarpMessage` for every register. Handlers run on a dedicated event thread, so they never block `read()` or `write()`. Both methods return a `Subscription`. Call `.unsubscribe()`, or use it as a context manager, to stop receiving events. diff --git a/docs/examples/subscribing_to_events/subscribing_to_events.py b/docs/examples/subscribing_to_events/subscribing_to_events.py index c04b91d..ca9dcec 100644 --- a/docs/examples/subscribing_to_events/subscribing_to_events.py +++ b/docs/examples/subscribing_to_events/subscribing_to_events.py @@ -2,23 +2,23 @@ from harp import serial from harp.device import client, core -from harp.protocol import HarpMessage, ParsedHarpMessage +from harp.protocol import HarpMessage SERIAL_PORT = "/dev/ttyUSB0" # or "COMx" in Windows, where "x" is the serial port number -def print_timestamp(msg: ParsedHarpMessage[np.uint32]) -> None: - print(f"[timestamp] {msg.timestamp:.6f} {msg.parsed}") +def print_timestamp(msg: HarpMessage[np.uint32]) -> None: + print(f"[timestamp] {msg.timestamp:.6f} {msg.payload}") def print_any_event(msg: HarpMessage) -> None: register = core.REGISTER_MAP.get(msg.address, None) - value = register.parse(msg) if register is not None else msg.payload.hex() + value = register.parse(msg) if register is not None else msg.raw_payload.hex() print(f"[{msg.address}] {msg.timestamp:.6f} {msg.message_type.name:<5s} {value}") with serial.open_device(client.Device, port=SERIAL_PORT) as device: - # Subscribe to a single, typed register: the handler receives a parsed payload. + # Subscribe to a single register: the handler receives a message typed by its payload. timestamp_subscription = device.subscribe(core.TimestampSeconds, print_timestamp) # Subscribe to every register at once: the handler receives the raw message. diff --git a/src/packages/harp-device/README.md b/src/packages/harp-device/README.md index 8446684..32d74f7 100644 --- a/src/packages/harp-device/README.md +++ b/src/packages/harp-device/README.md @@ -10,7 +10,7 @@ A `Device` operates over a transport. `read` and `write` take a register class: from harp.device import core # `device` is a Device opened over some transport, see harp-serial -who = device.read(core.WhoAmI).parsed # -> np.uint16 +who = device.read(core.WhoAmI).payload # -> np.uint16 device.write(core.OperationControl, payload) # write a register ``` diff --git a/src/packages/harp-device/src/harp/device/client/_device.py b/src/packages/harp-device/src/harp/device/client/_device.py index 0bcbb9c..8fe3e68 100644 --- a/src/packages/harp-device/src/harp/device/client/_device.py +++ b/src/packages/harp-device/src/harp/device/client/_device.py @@ -8,7 +8,6 @@ import threading from harp.protocol import HarpMessage, MessageType -from harp.protocol._message import ParsedHarpMessage from harp.protocol._register import RegisterBase from harp.device.schema import DeviceModuleLike @@ -23,8 +22,8 @@ _logger = logging.getLogger(__name__) -EventHandler = Callable[[ParsedHarpMessage[P]], None] -"""A callback receiving a typed, parsed event for a specific register.""" +EventHandler = Callable[[HarpMessage[P]], None] +"""A callback receiving a message typed by the payload of a specific register.""" MessageTypeFilter = MessageType | Iterable[MessageType] """Message types a subscription reacts to, as a single type or an iterable.""" @@ -164,7 +163,7 @@ def _validate_whoami(self) -> None: expected = module.WHO_AM_I if expected == 0x0: return - actual = int(self.read(WhoAmI).parsed) + actual = int(self.read(WhoAmI).payload) if actual != expected: raise RuntimeError( f"WhoAmI mismatch: {module.DEVICE_NAME} expects 0x{expected:04x} " @@ -204,12 +203,12 @@ def read( *, timestamp: float | None = None, port: int = 255, - ) -> ParsedHarpMessage[P]: + ) -> HarpMessage[P]: # Note: ty can't correctly infer the return type, and this is a known issue: # https://github.com/astral-sh/ty/issues/623 frame = register.format(message_type=MessageType.Read, timestamp=timestamp, port=port) msg = self._request(register.address, frame) - return ParsedHarpMessage.from_message(msg, register.parse(msg)) + return msg.with_payload(register.parse(msg)) def write( self, @@ -218,12 +217,12 @@ def write( *, timestamp: float | None = None, port: int = 255, - ) -> ParsedHarpMessage[P]: + ) -> HarpMessage[P]: frame = register.format( value, message_type=MessageType.Write, timestamp=timestamp, port=port ) msg = self._request(register.address, frame) - return ParsedHarpMessage.from_message(msg, register.parse(msg)) + return msg.with_payload(register.parse(msg)) # ------------------------------------------------------------------ # Events @@ -236,8 +235,8 @@ def subscribe( *, message_types: MessageTypeFilter = MessageType.Event, ) -> Subscription: - """Call ``handler`` with a typed, parsed :class:`ParsedHarpMessage` each - time the device emits a message for ``register``. + """Call ``handler`` with a :class:`~harp.protocol.HarpMessage` typed by the + payload of ``register``, each time the device emits a message for it. By default only unsolicited ``Event`` messages are delivered. Pass ``message_types`` (a :class:`MessageType` or an iterable of them) to also @@ -310,14 +309,14 @@ def _deliver_event(self, msg: HarpMessage) -> None: matching = [s for s in subs if msg.message_type in s._message_types] if matching and register is not None: try: - parsed = ParsedHarpMessage.from_message(msg, register.parse(msg)) + typed = msg.with_payload(register.parse(msg)) except Exception: _logger.exception( "Failed to parse %r for address 0x%02x", msg.message_type, msg.address ) else: for sub in matching: - self._safe_call(sub._handler, parsed) + self._safe_call(sub._handler, typed) for sub in catch_all: if msg.message_type in sub._message_types: @@ -372,7 +371,7 @@ def _request(self, address: int, frame: bytes) -> HarpMessage: if msg.has_error and self.raise_on_error: raise RuntimeError( f"Device returned error for register address {address} " - f"(0x{address:02x}). Payload: {msg.payload.hex()}" + f"(0x{address:02x}). Payload: {msg.raw_payload.hex()}" ) return msg except queue.Empty as exc: diff --git a/src/packages/harp-protocol/src/harp/protocol/__init__.py b/src/packages/harp-protocol/src/harp/protocol/__init__.py index abce77c..989af34 100644 --- a/src/packages/harp-protocol/src/harp/protocol/__init__.py +++ b/src/packages/harp-protocol/src/harp/protocol/__init__.py @@ -1,4 +1,4 @@ -from ._message import HarpMessage, HarpParseError, ParsedHarpMessage +from ._message import HarpMessage, HarpParseError from ._message_type import MessageType from ._payload_converters import ( BoolConverter, @@ -74,7 +74,6 @@ "encode_payload_type", # Message "HarpMessage", - "ParsedHarpMessage", "HarpParseError", # Converters "Converter", diff --git a/src/packages/harp-protocol/src/harp/protocol/_message.py b/src/packages/harp-protocol/src/harp/protocol/_message.py index 9192245..0d2a947 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_message.py +++ b/src/packages/harp-protocol/src/harp/protocol/_message.py @@ -1,7 +1,7 @@ """Harp message container.""" import struct -from typing import Generic, TypeVar, cast +from typing import Any, Generic, TypeVar, cast from ._builder import build_message_frame from ._checksum import validate as _validate_checksum @@ -17,6 +17,10 @@ from ._payload_type import PayloadType, decode_payload_type P = TypeVar("P") +_T = TypeVar("_T") + +_UNDECODED: Any = object() +"""Marks a message whose payload no register has decoded yet.""" class HarpParseError(Exception): @@ -25,30 +29,34 @@ class HarpParseError(Exception): pass -class HarpMessage: - """A Harp message backed by its raw frame bytes. +class HarpMessage(Generic[P]): + """A Harp message backed by its raw frame bytes, parameterized by its payload type. Build with the constructor or parse from wire bytes with ``HarpMessage.parse()``. + A message off the wire is a ``HarpMessage[Any]``, since a frame declares only how + its payload is encoded and not which register contract it satisfies. Decoding it + with a register yields a ``HarpMessage[P]``, whose ``payload`` is that contract. """ - __slots__ = ("_bytes",) + __slots__ = ("_bytes", "_payload") def __init__( self, message_type: MessageType, address: int, payload_type: PayloadType, - payload: bytes = b"", + raw_payload: bytes = b"", *, port: int = _DEFAULT_PORT, timestamp: float | None = None, ) -> None: self._bytes: bytes = build_message_frame( - message_type, address, payload_type, payload, port=port, timestamp=timestamp + message_type, address, payload_type, raw_payload, port=port, timestamp=timestamp ) + self._payload: P = _UNDECODED @classmethod - def parse(cls, data: bytes | bytearray | memoryview) -> "HarpMessage": + def parse(cls, data: bytes | bytearray | memoryview) -> "HarpMessage[Any]": """Parse and validate a complete Harp Message from a byte sequence. Raises ``HarpParseError`` on failure.""" raw = data if isinstance(data, bytes) else bytes(data) @@ -77,6 +85,7 @@ def parse(cls, data: bytes | bytearray | memoryview) -> "HarpMessage": obj = cls.__new__(cls) obj._bytes = raw + obj._payload = _UNDECODED return obj @property @@ -118,11 +127,38 @@ def timestamp(self) -> float | None: return cast(int, seconds) + cast(int, microseconds) * _TICK_PERIOD_S @property - def payload(self) -> memoryview: + def raw_payload(self) -> memoryview: """Payload bytes, excluding timestamp and checksum.""" offset = _TIMESTAMPED_PAYLOAD_OFFSET if self.has_timestamp else _HEADER_LEN return memoryview(self._bytes)[offset:-1] + @property + def has_payload(self) -> bool: + """Return True if a register has decoded the payload of this message.""" + return self._payload is not _UNDECODED + + @property + def payload(self) -> P: + """The decoded payload, as the register that parsed this message defines it. + + Only a register knows which contract a frame satisfies, so a message read from + the wire carries no payload until one decodes it. Raises ``ValueError`` in that + case; test with ``has_payload``, or read ``raw_payload`` for the frame bytes. + """ + if self._payload is _UNDECODED: + raise ValueError( + "No register has decoded this message, so it has no payload. " + "Parse it with a register, or read raw_payload for the frame bytes." + ) + return self._payload + + def with_payload(self, payload: _T) -> "HarpMessage[_T]": + """Return a copy of this message carrying ``payload`` as its decoded payload.""" + obj: HarpMessage[_T] = HarpMessage.__new__(HarpMessage) + obj._bytes = self._bytes + obj._payload = payload + return obj + @property def bytes(self) -> bytes: """The complete raw message frame, including checksum.""" @@ -133,38 +169,3 @@ def __str__(self) -> str: f"HarpMessage(message_type={self.message_type!r}, address={self.address:#04x}, " f"payload_type={self.payload_type!r}, timestamp={self.timestamp!r})" ) - - -class ParsedHarpMessage(HarpMessage, Generic[P]): - """A ``HarpMessage`` with a typed parsed payload attached.""" - - __slots__ = ("_parsed",) - - def __init__( - self, - message_type: MessageType, - address: int, - payload_type: PayloadType, - payload: bytes = b"", - *, - port: int = _DEFAULT_PORT, - timestamp: float | None = None, - parsed: P, - ) -> None: - super().__init__( - message_type, address, payload_type, payload, port=port, timestamp=timestamp - ) - self._parsed = parsed - - @classmethod - def from_message(cls, msg: HarpMessage, parsed: P) -> "ParsedHarpMessage[P]": - """Wrap a ``HarpMessage`` with a pre-parsed payload.""" - obj = cls.__new__(cls) - obj._bytes = msg.bytes - obj._parsed = parsed - return obj - - @property - def parsed(self) -> P: - """Returns the parsed payload.""" - return self._parsed diff --git a/src/packages/harp-protocol/src/harp/protocol/_register.py b/src/packages/harp-protocol/src/harp/protocol/_register.py index 129897f..9808a4e 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_register.py +++ b/src/packages/harp-protocol/src/harp/protocol/_register.py @@ -164,7 +164,7 @@ def parse(cls, value: HarpMessage | bytes | bytearray | memoryview) -> U: ``payload.Channel0`` works). Anonymous payloads (scalar / array registers) return the raw numpy scalar or ndarray directly. """ - buf = value.payload if isinstance(value, HarpMessage) else value + buf = value.raw_payload if isinstance(value, HarpMessage) else value record = np.frombuffer(buf, dtype=cls.payload_class.payload_dtype, count=1)[0] return cast(U, cls.payload_class._unwrap(record)) diff --git a/src/packages/harp-serial/README.md b/src/packages/harp-serial/README.md index ea824f2..d748ce2 100644 --- a/src/packages/harp-serial/README.md +++ b/src/packages/harp-serial/README.md @@ -12,8 +12,8 @@ from harp.device import behavior, core # Use "COMx" on Windows, "/dev/ttyUSBx" on Linux. with serial.open_device(behavior, port="COM3") as device: - print(device.read(core.WhoAmI).parsed) # a common register - print(device.read(behavior.AnalogData).parsed) # a device register + print(device.read(core.WhoAmI).payload) # a common register + print(device.read(behavior.AnalogData).payload) # a device register ``` Passing a device module validates the device identity on open. Pass a `Device` subclass instead to preserve its own type, or omit the argument entirely for schema-free access, which skips the identity check. diff --git a/tests/conformance.py b/tests/conformance.py index 3d27ade..a6aa72a 100644 --- a/tests/conformance.py +++ b/tests/conformance.py @@ -12,7 +12,7 @@ from harp.device.client import Device, ITransport from harp.device.core import OperationControl, OperationControlPayload, WhoAmI from harp.device.schema import DeviceModule, DeviceModuleLike, create_device_module -from harp.protocol import ParsedHarpMessage, RegisterBase +from harp.protocol import HarpMessage, RegisterBase from harp.serial import open_device @@ -27,14 +27,14 @@ def schema_built_registers(yml: str) -> None: def statically_declared_registers(device: Device) -> None: """A register written out in a module carries its payload type through read.""" - assert_type(device.read(WhoAmI), ParsedHarpMessage[np.uint16]) - assert_type(device.read(WhoAmI).parsed, np.uint16) - assert_type(device.read(OperationControl).parsed, OperationControlPayload) + assert_type(device.read(WhoAmI), HarpMessage[np.uint16]) + assert_type(device.read(WhoAmI).payload, np.uint16) + assert_type(device.read(OperationControl).payload, OperationControlPayload) def register_writes(device: Device, payload: OperationControlPayload) -> None: """Write accepts the payload type its register parses to.""" - assert_type(device.write(OperationControl, payload).parsed, OperationControlPayload) + assert_type(device.write(OperationControl, payload).payload, OperationControlPayload) def device_with_module(transport: ITransport, module: DeviceModule) -> None: diff --git a/tests/protocol/test_framer.py b/tests/protocol/test_framer.py index 182a2bf..1e587b9 100644 --- a/tests/protocol/test_framer.py +++ b/tests/protocol/test_framer.py @@ -11,7 +11,7 @@ def test_single_message(): msgs = HarpFramer.parse_bytes(frame) assert len(msgs) == 1 assert msgs[0].address == 10 - assert msgs[0].payload == b"\x01" + assert msgs[0].raw_payload == b"\x01" def test_back_to_back_messages(): @@ -67,7 +67,7 @@ def test_incremental_feed(): framer.feed(bytes([byte])) results.extend(framer.frames()) assert len(results) == 1 - assert results[0].payload == b"\x42" + assert results[0].raw_payload == b"\x42" def test_all_scalar_types(): @@ -89,7 +89,7 @@ def test_array_payload(): frame = make_frame_from_raw(0x03, 32, 0xFF, 0x02, payload) msgs = HarpFramer.parse_bytes(frame) assert len(msgs) == 1 - assert len(msgs[0].payload) == 10 + assert len(msgs[0].raw_payload) == 10 def test_parse_file(tmp_path): diff --git a/tests/protocol/test_message.py b/tests/protocol/test_message.py index c22465f..c3bb91b 100644 --- a/tests/protocol/test_message.py +++ b/tests/protocol/test_message.py @@ -17,7 +17,7 @@ def test_parse_read_request(): assert msg.has_error is False assert msg.address == 8 assert msg.port == 0xFF - assert msg.payload == b"" + assert msg.raw_payload == b"" assert msg.timestamp is None @@ -25,7 +25,7 @@ def test_parse_write_u8_payload(): frame = make_frame_from_raw(0x02, address=10, port=0xFF, payload_type=0x01, payload=b"\x05") msg = HarpMessage.parse(frame) assert msg.message_type == MessageType.Write - assert msg.payload == b"\x05" + assert msg.raw_payload == b"\x05" assert msg.payload_type == PayloadType.U8 @@ -41,7 +41,7 @@ def test_parse_with_timestamp(): msg = HarpMessage.parse(frame) assert msg.message_type == MessageType.Event assert msg.timestamp == pytest.approx(1.0) - assert msg.payload == b"\x7f" + assert msg.raw_payload == b"\x7f" def test_parse_error_flag(): @@ -55,7 +55,7 @@ def test_parse_u16_array(): payload = struct.pack(" Date: Wed, 19 Aug 2026 16:29:14 +0100 Subject: [PATCH 2/3] Use a sentinel and rename the byte view The undecoded payload marker is a typing_extensions Sentinel rather than a bare object, so _payload is typed as P | _UNDECODED and narrows on the identity check instead of being hidden behind Any. payload_bytes replaces raw_payload, pairing with the existing bytes property that returns the whole frame. typing-extensions rises to 4.14, the version that introduces Sentinel, which three modules already import under the previous lower bound. --- .../subscribing_to_events.py | 2 +- .../src/harp/device/client/_device.py | 2 +- src/packages/harp-protocol/pyproject.toml | 2 +- .../src/harp/protocol/_message.py | 22 ++++++++++--------- .../src/harp/protocol/_register.py | 2 +- tests/protocol/test_framer.py | 6 ++--- tests/protocol/test_message.py | 12 +++++----- tests/protocol/test_register.py | 12 +++++----- uv.lock | 2 +- 9 files changed, 32 insertions(+), 30 deletions(-) diff --git a/docs/examples/subscribing_to_events/subscribing_to_events.py b/docs/examples/subscribing_to_events/subscribing_to_events.py index ca9dcec..c69cc9a 100644 --- a/docs/examples/subscribing_to_events/subscribing_to_events.py +++ b/docs/examples/subscribing_to_events/subscribing_to_events.py @@ -13,7 +13,7 @@ def print_timestamp(msg: HarpMessage[np.uint32]) -> None: def print_any_event(msg: HarpMessage) -> None: register = core.REGISTER_MAP.get(msg.address, None) - value = register.parse(msg) if register is not None else msg.raw_payload.hex() + value = register.parse(msg) if register is not None else msg.payload_bytes.hex() print(f"[{msg.address}] {msg.timestamp:.6f} {msg.message_type.name:<5s} {value}") diff --git a/src/packages/harp-device/src/harp/device/client/_device.py b/src/packages/harp-device/src/harp/device/client/_device.py index 8fe3e68..89b606f 100644 --- a/src/packages/harp-device/src/harp/device/client/_device.py +++ b/src/packages/harp-device/src/harp/device/client/_device.py @@ -371,7 +371,7 @@ def _request(self, address: int, frame: bytes) -> HarpMessage: if msg.has_error and self.raise_on_error: raise RuntimeError( f"Device returned error for register address {address} " - f"(0x{address:02x}). Payload: {msg.raw_payload.hex()}" + f"(0x{address:02x}). Payload: {msg.payload_bytes.hex()}" ) return msg except queue.Empty as exc: diff --git a/src/packages/harp-protocol/pyproject.toml b/src/packages/harp-protocol/pyproject.toml index c903d03..c6fb392 100644 --- a/src/packages/harp-protocol/pyproject.toml +++ b/src/packages/harp-protocol/pyproject.toml @@ -8,7 +8,7 @@ keywords = ['python', 'harp'] requires-python = ">=3.11" dependencies = [ "numpy>=1.24", - "typing-extensions>=4.0", + "typing-extensions>=4.14", ] [build-system] diff --git a/src/packages/harp-protocol/src/harp/protocol/_message.py b/src/packages/harp-protocol/src/harp/protocol/_message.py index 0d2a947..c5ff8e4 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_message.py +++ b/src/packages/harp-protocol/src/harp/protocol/_message.py @@ -3,6 +3,8 @@ import struct from typing import Any, Generic, TypeVar, cast +from typing_extensions import Sentinel + from ._builder import build_message_frame from ._checksum import validate as _validate_checksum from ._constants import ( @@ -17,9 +19,9 @@ from ._payload_type import PayloadType, decode_payload_type P = TypeVar("P") -_T = TypeVar("_T") +_P = TypeVar("_P") -_UNDECODED: Any = object() +_UNDECODED = Sentinel("_UNDECODED") """Marks a message whose payload no register has decoded yet.""" @@ -45,15 +47,15 @@ def __init__( message_type: MessageType, address: int, payload_type: PayloadType, - raw_payload: bytes = b"", + payload_bytes: bytes = b"", *, port: int = _DEFAULT_PORT, timestamp: float | None = None, ) -> None: self._bytes: bytes = build_message_frame( - message_type, address, payload_type, raw_payload, port=port, timestamp=timestamp + message_type, address, payload_type, payload_bytes, port=port, timestamp=timestamp ) - self._payload: P = _UNDECODED + self._payload: P | _UNDECODED = _UNDECODED @classmethod def parse(cls, data: bytes | bytearray | memoryview) -> "HarpMessage[Any]": @@ -127,7 +129,7 @@ def timestamp(self) -> float | None: return cast(int, seconds) + cast(int, microseconds) * _TICK_PERIOD_S @property - def raw_payload(self) -> memoryview: + def payload_bytes(self) -> memoryview: """Payload bytes, excluding timestamp and checksum.""" offset = _TIMESTAMPED_PAYLOAD_OFFSET if self.has_timestamp else _HEADER_LEN return memoryview(self._bytes)[offset:-1] @@ -143,18 +145,18 @@ def payload(self) -> P: Only a register knows which contract a frame satisfies, so a message read from the wire carries no payload until one decodes it. Raises ``ValueError`` in that - case; test with ``has_payload``, or read ``raw_payload`` for the frame bytes. + case; test with ``has_payload`` first, or read ``payload_bytes`` instead. """ if self._payload is _UNDECODED: raise ValueError( "No register has decoded this message, so it has no payload. " - "Parse it with a register, or read raw_payload for the frame bytes." + "Parse it with a register, or read payload_bytes instead." ) return self._payload - def with_payload(self, payload: _T) -> "HarpMessage[_T]": + def with_payload(self, payload: _P) -> "HarpMessage[_P]": """Return a copy of this message carrying ``payload`` as its decoded payload.""" - obj: HarpMessage[_T] = HarpMessage.__new__(HarpMessage) + obj: HarpMessage[_P] = HarpMessage.__new__(HarpMessage) obj._bytes = self._bytes obj._payload = payload return obj diff --git a/src/packages/harp-protocol/src/harp/protocol/_register.py b/src/packages/harp-protocol/src/harp/protocol/_register.py index 9808a4e..effcfe0 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_register.py +++ b/src/packages/harp-protocol/src/harp/protocol/_register.py @@ -164,7 +164,7 @@ def parse(cls, value: HarpMessage | bytes | bytearray | memoryview) -> U: ``payload.Channel0`` works). Anonymous payloads (scalar / array registers) return the raw numpy scalar or ndarray directly. """ - buf = value.raw_payload if isinstance(value, HarpMessage) else value + buf = value.payload_bytes if isinstance(value, HarpMessage) else value record = np.frombuffer(buf, dtype=cls.payload_class.payload_dtype, count=1)[0] return cast(U, cls.payload_class._unwrap(record)) diff --git a/tests/protocol/test_framer.py b/tests/protocol/test_framer.py index 1e587b9..ef44513 100644 --- a/tests/protocol/test_framer.py +++ b/tests/protocol/test_framer.py @@ -11,7 +11,7 @@ def test_single_message(): msgs = HarpFramer.parse_bytes(frame) assert len(msgs) == 1 assert msgs[0].address == 10 - assert msgs[0].raw_payload == b"\x01" + assert msgs[0].payload_bytes == b"\x01" def test_back_to_back_messages(): @@ -67,7 +67,7 @@ def test_incremental_feed(): framer.feed(bytes([byte])) results.extend(framer.frames()) assert len(results) == 1 - assert results[0].raw_payload == b"\x42" + assert results[0].payload_bytes == b"\x42" def test_all_scalar_types(): @@ -89,7 +89,7 @@ def test_array_payload(): frame = make_frame_from_raw(0x03, 32, 0xFF, 0x02, payload) msgs = HarpFramer.parse_bytes(frame) assert len(msgs) == 1 - assert len(msgs[0].raw_payload) == 10 + assert len(msgs[0].payload_bytes) == 10 def test_parse_file(tmp_path): diff --git a/tests/protocol/test_message.py b/tests/protocol/test_message.py index c3bb91b..e5cec5b 100644 --- a/tests/protocol/test_message.py +++ b/tests/protocol/test_message.py @@ -17,7 +17,7 @@ def test_parse_read_request(): assert msg.has_error is False assert msg.address == 8 assert msg.port == 0xFF - assert msg.raw_payload == b"" + assert msg.payload_bytes == b"" assert msg.timestamp is None @@ -25,7 +25,7 @@ def test_parse_write_u8_payload(): frame = make_frame_from_raw(0x02, address=10, port=0xFF, payload_type=0x01, payload=b"\x05") msg = HarpMessage.parse(frame) assert msg.message_type == MessageType.Write - assert msg.raw_payload == b"\x05" + assert msg.payload_bytes == b"\x05" assert msg.payload_type == PayloadType.U8 @@ -41,7 +41,7 @@ def test_parse_with_timestamp(): msg = HarpMessage.parse(frame) assert msg.message_type == MessageType.Event assert msg.timestamp == pytest.approx(1.0) - assert msg.raw_payload == b"\x7f" + assert msg.payload_bytes == b"\x7f" def test_parse_error_flag(): @@ -55,7 +55,7 @@ def test_parse_u16_array(): payload = struct.pack(" Date: Wed, 19 Aug 2026 20:42:31 +0100 Subject: [PATCH 3/3] Bind payload decoding to the message it came from HarpMessage.decode takes anything satisfying the new PayloadDecoder protocol, which every register does, and returns a copy carrying the decoded payload. The payload is derived from the frame in the same call, so the two cannot disagree. Decoding checks the payload type and the byte count, which parse does not, so a U32 frame read through a U16 register and a 4-byte payload read through a scalar register both raise instead of returning the leading bytes. The address is not checked, so a frame can be decoded by any register describing the same layout. The protocol is structural, keeping registers out of the message imports. --- .../src/harp/device/client/_device.py | 6 +-- .../src/harp/protocol/_message.py | 45 +++++++++++++++++-- tests/protocol/test_message.py | 35 ++++++++++----- 3 files changed, 69 insertions(+), 17 deletions(-) diff --git a/src/packages/harp-device/src/harp/device/client/_device.py b/src/packages/harp-device/src/harp/device/client/_device.py index 89b606f..1ff0333 100644 --- a/src/packages/harp-device/src/harp/device/client/_device.py +++ b/src/packages/harp-device/src/harp/device/client/_device.py @@ -208,7 +208,7 @@ def read( # https://github.com/astral-sh/ty/issues/623 frame = register.format(message_type=MessageType.Read, timestamp=timestamp, port=port) msg = self._request(register.address, frame) - return msg.with_payload(register.parse(msg)) + return msg.decode(register) def write( self, @@ -222,7 +222,7 @@ def write( value, message_type=MessageType.Write, timestamp=timestamp, port=port ) msg = self._request(register.address, frame) - return msg.with_payload(register.parse(msg)) + return msg.decode(register) # ------------------------------------------------------------------ # Events @@ -309,7 +309,7 @@ def _deliver_event(self, msg: HarpMessage) -> None: matching = [s for s in subs if msg.message_type in s._message_types] if matching and register is not None: try: - typed = msg.with_payload(register.parse(msg)) + typed = msg.decode(register) except Exception: _logger.exception( "Failed to parse %r for address 0x%02x", msg.message_type, msg.address diff --git a/src/packages/harp-protocol/src/harp/protocol/_message.py b/src/packages/harp-protocol/src/harp/protocol/_message.py index c5ff8e4..7578ad5 100644 --- a/src/packages/harp-protocol/src/harp/protocol/_message.py +++ b/src/packages/harp-protocol/src/harp/protocol/_message.py @@ -1,8 +1,9 @@ """Harp message container.""" import struct -from typing import Any, Generic, TypeVar, cast +from typing import Any, ClassVar, Generic, Protocol, TypeVar, cast +import numpy as np from typing_extensions import Sentinel from ._builder import build_message_frame @@ -20,6 +21,7 @@ P = TypeVar("P") _P = TypeVar("_P") +_P_co = TypeVar("_P_co", covariant=True) _UNDECODED = Sentinel("_UNDECODED") """Marks a message whose payload no register has decoded yet.""" @@ -31,6 +33,23 @@ class HarpParseError(Exception): pass +class PayloadDecoder(Protocol[_P_co]): + """Reads a payload of type ``_P_co`` out of a message. + + Structural rather than nominal, so a message never has to know about registers, and + anything declaring a payload type, a length and a ``parse`` satisfies it. Every + ``RegisterBase`` does. ``length`` is the element count, or ``None`` for a single + value, and together with ``payload_type`` it fixes how many payload bytes the + decoder consumes. + """ + + payload_type: ClassVar["PayloadType"] + length: ClassVar[int | None] + + @classmethod + def parse(cls, value: Any) -> _P_co: ... + + class HarpMessage(Generic[P]): """A Harp message backed by its raw frame bytes, parameterized by its payload type. @@ -154,11 +173,29 @@ def payload(self) -> P: ) return self._payload - def with_payload(self, payload: _P) -> "HarpMessage[_P]": - """Return a copy of this message carrying ``payload`` as its decoded payload.""" + def decode(self, decoder: type[PayloadDecoder[_P]]) -> "HarpMessage[_P]": + """Return a copy of this message with its payload decoded by ``decoder``. + + The payload is derived from the frame in the same call, so the two cannot + disagree. The payload type and the byte count are both checked, since together + they decide whether these bytes can be read as this payload at all. The address + is not, so a frame may be decoded by anything describing the same layout. + """ + if self.payload_type is not decoder.payload_type: + raise HarpParseError( + f"{decoder.__name__} declares {decoder.payload_type!r} but this " + f"message declares {self.payload_type!r}." + ) + expected = (decoder.length or 1) * np.dtype(decoder.payload_type.value).itemsize + actual = len(self.payload_bytes) + if actual != expected: + raise HarpParseError( + f"{decoder.__name__} reads {expected} payload bytes but this message " + f"carries {actual}." + ) obj: HarpMessage[_P] = HarpMessage.__new__(HarpMessage) obj._bytes = self._bytes - obj._payload = payload + obj._payload = decoder.parse(self) return obj @property diff --git a/tests/protocol/test_message.py b/tests/protocol/test_message.py index e5cec5b..8f70ea4 100644 --- a/tests/protocol/test_message.py +++ b/tests/protocol/test_message.py @@ -3,6 +3,7 @@ import numpy as np import pytest from harp.protocol._message import HarpMessage, HarpParseError +from harp.protocol._register import RegisterU8, RegisterU16 from harp.protocol._message_type import MessageType from harp.protocol._payload_type import PayloadType @@ -102,22 +103,36 @@ def test_wire_message_carries_no_payload(): msg.payload -def test_with_payload_attaches_without_touching_the_frame(): - msg = HarpMessage.parse( +def _u8_frame(): + return HarpMessage.parse( make_frame_from_raw(0x02, address=10, port=0xFF, payload_type=0x01, payload=b"\x05") ) - typed = msg.with_payload(5) + + +def test_decode_attaches_without_touching_the_frame(): + typed = _u8_frame().decode(RegisterU8(0x0A)) assert typed.has_payload is True assert typed.payload == 5 - assert typed.bytes == msg.bytes assert typed.payload_bytes == b"\x05" - assert typed.address == msg.address + assert typed.address == 10 -def test_with_payload_leaves_the_source_undecoded(): +def test_decode_leaves_the_source_undecoded(): # The dispatch loop hands one frame to several places, so decoding must not mutate it. - msg = HarpMessage.parse( - make_frame_from_raw(0x02, address=10, port=0xFF, payload_type=0x01, payload=b"\x05") - ) - msg.with_payload(5) + msg = _u8_frame() + msg.decode(RegisterU8(0x0A)) assert msg.has_payload is False + + +def test_decode_rejects_a_payload_type_mismatch(): + # Without the check the register reads the low bytes at its own width and returns a + # silently wrong value, which is what parse does on its own. + with pytest.raises(HarpParseError, match="declares"): + _u8_frame().decode(RegisterU16(0x0A)) + + +def test_decode_accepts_a_register_at_another_address(): + # The payload type decides whether these bytes can be read as this register at all. + # The address says which register the device meant, so an identical layout decodes + # either way and a frame can be read through more than one register. + assert _u8_frame().decode(RegisterU8(0x2A)).payload == 5