From 88d4e2d2787e5ab0e6e1e5ea477bd67d5b79fbb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Wed, 5 Aug 2026 15:17:35 -0400 Subject: [PATCH 01/17] Sketch a sans-I/O protocol description, in its test file Problem: the Command architecture conflates four jobs -- describing the protocol, building the request bytes, performing the exchange, and storing what came back -- and the fourth is why IntegraDevice.commands["GETPOWER"] is one object shared by every instance, with the reply written onto it. Before any driver depends on a replacement, the description itself should be worth looking at. Solution: a prototype living entirely inside hardwarelibrary/tests/testProtocolPrototype.py, imported by nothing. Request turns named arguments into bytes; Reply turns bytes into named values; Exchange pairs one with the other and performs neither. Nothing is stored on a description, so two callers of one Exchange cannot overwrite each other -- asserted directly in a test. Two consequences fall out of the split. A Reply says how it must be read, a readLength taken from the struct format or a terminator, so the caller knows what to ask the port for without the description reaching for one. And decoding failure raises, naming both the pattern and what actually arrived, where the current TextCommand.send swallows it into an attribute and returns True for failure. Everything is named: a reply decodes to {"x": 100, "y": 200, "z": 300} rather than a positional matchGroups the caller indexes by number. The last test class is the real test of the design -- it says what the drivers in this repo already speak, in the proposed form: Cobolt power and on/off, Sutter MOVE and GET_POSITION, Integra wavelength both ways, Intellidrive registers, and the SR830 SNAP? reply that the current Command cannot describe at all. Only the first half is built: writing a request, decoding a reply. The mirror image a table-driven mock needs, decoding a request and encoding a reply, is deliberately left out. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 434 ++++++++++++++++++ 1 file changed, 434 insertions(+) create mode 100644 hardwarelibrary/tests/testProtocolPrototype.py diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py new file mode 100644 index 00000000..bfb65f56 --- /dev/null +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -0,0 +1,434 @@ +"""A prototype protocol description, developed in isolation inside its test file. + +Nothing here is imported by the library, and nothing here touches a port: this is +a sketch of how an instrument protocol could be *described*, so the design can be +judged before any driver depends on it. + +The idea is sans-I/O, the principle behind h11 and wsproto: the protocol is a pure +transformation over bytes and never performs the exchange. A Request turns named +arguments into the bytes to write; a Reply turns the bytes read back into named +values; neither owns a port, and neither stores what happened. That is what +separates this from the current Command, which describes the protocol, builds the +bytes, performs the I/O, and then keeps the reply on itself -- on a class +attribute shared by every instance of a driver. + +Consequences worth noticing while reading: + + - a description is immutable and shareable; the result of an exchange is a plain + dict returned to the caller, so two instruments cannot overwrite each other; + - a Reply says how it must be read (a fixed length, or up to a terminator), so + the caller knows what to ask the port for without the description doing it; + - decoding failure raises, naming what did not match, instead of being recorded + in an attribute nobody checks. + +Only the first half is built here: writing a request and decoding a reply. The +mirror image -- decoding a request and encoding a reply, which is what a +table-driven mock needs -- is deliberately left out. +""" + +import env +import re +import string +import unittest +from abc import ABC, abstractmethod +from struct import calcsize, error as StructError, pack, unpack + + +# --------------------------------------------------------------------------- +# The prototype +# --------------------------------------------------------------------------- + +class ProtocolError(Exception): + """Base for anything the description itself can refuse.""" + + +class MissingArgument(ProtocolError): + """A request needs a value the caller did not supply.""" + + +class ReplyDidNotMatch(ProtocolError): + """The bytes read back are not what this reply describes.""" + + +class Request(ABC): + """How to turn named arguments into the bytes to write.""" + + @abstractmethod + def encode(self, **arguments) -> bytes: + ... + + +class TextRequest(Request): + """An ASCII request built from a format template. + + The template uses named fields, so a caller writes setPower(power=0.5) and + never counts positional arguments: "p {power:0.3f}" with the terminator the + instrument expects appended. + """ + + def __init__(self, template: str, terminator: str = "\r"): + self.template = template + self.terminator = terminator + + @property + def fields(self) -> tuple: + """The argument names this request needs, in the order they appear.""" + return tuple(name for _, name, _, _ in string.Formatter().parse(self.template) + if name) + + def encode(self, **arguments) -> bytes: + try: + text = self.template.format(**arguments) + except KeyError as error: + raise MissingArgument("{0} needs {1}, got {2}".format( + self.template, error, sorted(arguments))) from None + return (text + self.terminator).encode("utf-8") + + +class BinaryRequest(Request): + """A packed binary request. + + fields names each value in the struct format, in order; constants are the ones + the caller never supplies, such as a header byte or a trailing carriage return. + """ + + def __init__(self, format: str, fields: tuple = (), constants: dict = None): + self.format = format + self.fields = tuple(fields) + self.constants = dict(constants or {}) + + @property + def arguments(self) -> tuple: + """The names a caller must supply: every field that is not a constant.""" + return tuple(name for name in self.fields if name not in self.constants) + + def encode(self, **arguments) -> bytes: + values = [] + for name in self.fields: + if name in self.constants: + values.append(self.constants[name]) + elif name in arguments: + values.append(arguments[name]) + else: + raise MissingArgument("{0} needs {1}, got {2}".format( + self.format, name, sorted(arguments))) + try: + return pack(self.format, *values) + except StructError as error: + raise ProtocolError("cannot pack {0} into {1}: {2}".format( + values, self.format, error)) from None + + +class Reply(ABC): + """How to turn the bytes read back into named values. + + A reply also says how it must be read: readLength for a fixed-size frame, or + terminator for a line. Exactly one of the two is set, so a caller can always + tell what to ask the port for. + """ + + readLength = None + terminator = None + + @abstractmethod + def decode(self, data: bytes) -> dict: + ... + + +class TextReply(Reply): + """A line matched by a regular expression. + + fields maps a name to the converter for its capture group, in group order: + {"power": float} turns r"(\\d+\\.\\d+)" into {"power": 0.123}. A reply with no + capture groups, such as an "OK" acknowledgement, decodes to an empty dict -- + it either matched or it raised. + """ + + def __init__(self, pattern: str, fields: dict = None, terminator: str = "\r\n"): + self.pattern = pattern + self.fields = dict(fields or {}) + self.terminator = terminator + + def decode(self, data) -> dict: + text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data + match = re.search(self.pattern, text) + if match is None: + raise ReplyDidNotMatch("expected {0!r}, got {1!r}".format(self.pattern, text)) + + groups = match.groups() + if len(groups) != len(self.fields): + raise ProtocolError( + "{0!r} captured {1} group(s) but {2} field(s) were named".format( + self.pattern, len(groups), len(self.fields))) + return {name: converter(value) + for (name, converter), value in zip(self.fields.items(), groups)} + + +class BinaryReply(Reply): + """A fixed-size binary frame, named field by field. + + The struct format decides how many bytes to read, so readLength never has to + be kept in step by hand. Padding in the format ('x') yields no value and so + takes no field name. + """ + + def __init__(self, format: str, fields: tuple = ()): + self.format = format + self.fields = tuple(fields) + + @property + def readLength(self) -> int: + return calcsize(self.format) + + def decode(self, data) -> dict: + if len(data) != self.readLength: + raise ReplyDidNotMatch("expected {0} bytes for {1}, got {2}".format( + self.readLength, self.format, len(data))) + values = unpack(self.format, bytes(data)) + if len(values) != len(self.fields): + raise ProtocolError( + "{0} unpacks {1} value(s) but {2} field(s) were named".format( + self.format, len(values), len(self.fields))) + return dict(zip(self.fields, values)) + + +class Exchange: + """One request and the reply it expects, named for a driver to call by name. + + Called Exchange rather than Command because it describes the round trip and + performs none of it: encode() gives the caller the bytes to write, decode() + turns what came back into values, and the caller owns the port in between. + """ + + def __init__(self, name: str, request: Request, reply: Reply = None): + self.name = name + self.request = request + self.reply = reply + + @property + def expectsReply(self) -> bool: + return self.reply is not None + + def encode(self, **arguments) -> bytes: + return self.request.encode(**arguments) + + def decode(self, data) -> dict: + if self.reply is None: + raise ProtocolError("{0} expects no reply".format(self.name)) + return self.reply.decode(data) + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + +class TestTextRequest(unittest.TestCase): + def testBuildsAConstantRequest(self): + self.assertEqual(TextRequest("pa?").encode(), b"pa?\r") + + def testSubstitutesNamedArguments(self): + request = TextRequest("p {power:0.3f}") + self.assertEqual(request.encode(power=0.5), b"p 0.500\r") + + def testTerminatorIsPartOfTheDescription(self): + self.assertEqual(TextRequest("*GWL", terminator="").encode(), b"*GWL") + self.assertEqual(TextRequest("g r0xc9", terminator="\n").encode(), b"g r0xc9\n") + + def testNamesTheFieldsItNeeds(self): + self.assertEqual(TextRequest("s r{register} {value}").fields, + ("register", "value")) + + def testAMissingArgumentSaysWhichOne(self): + with self.assertRaises(MissingArgument) as raised: + TextRequest("p {power:0.3f}").encode() + self.assertIn("power", str(raised.exception)) + + +class TestBinaryRequest(unittest.TestCase): + def testPacksConstantsOnly(self): + request = BinaryRequest(" Date: Thu, 6 Aug 2026 10:33:14 -0400 Subject: [PATCH 02/17] Add a ctypes-layout variant of the binary description Problem: the struct-format description carries the layout in one place and the field names in another, so the two can drift; the read length is a third thing, computed from the format; and a reply's terminator arrives as a pad byte that is discarded rather than a value that can be checked. Meanwhile struct.pack is positional, so nothing is named at the point of the call. Solution: FrameRequest and FrameReply, describing the same frames as a ctypes Structure. One declaration serves both directions -- bytes(frame) writes it, from_buffer_copy() reads it back by name -- and sizeof() gives the read length, so it cannot drift from the layout. Tests assert byte-for-byte equality with the struct-format version on the MP-285 MOVE and GET_POSITION frames, so the two can be compared on the same protocol rather than in the abstract. The variant brings a trap with it, which requirePackedLayout turns into an error at description time: without _pack_ = 1, ctypes aligns each field to its natural boundary and the 14-byte MOVE frame silently becomes 20, correct for a C compiler and wrong for the stage. struct's "<" gets that right by default, and the test pins the numbers. Both variants plug into the same Exchange, so the choice is not load-bearing yet. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index bfb65f56..2d7e49b8 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -31,6 +31,7 @@ import string import unittest from abc import ABC, abstractmethod +from ctypes import LittleEndianStructure, c_char, c_int32, sizeof from struct import calcsize, error as StructError, pack, unpack @@ -192,6 +193,96 @@ def decode(self, data) -> dict: return dict(zip(self.fields, values)) +# --- The same binary frames, declared as a ctypes layout instead ------------- +# +# A second way to say the same thing, for comparison. Where BinaryRequest and +# BinaryReply each carry a struct format and a tuple of names, a ctypes Structure +# carries both in one declaration and works in both directions: bytes(instance) +# writes the frame, from_buffer_copy() reads it back by name. sizeof() then gives +# the read length, so it cannot drift from the layout. +# +# The cost is a trap, which is why layoutIsPacked exists below: without +# _pack_ = 1, ctypes aligns each field to its natural boundary and a 14-byte +# frame silently becomes 20. struct's "<" gets that right by default. + + +def layoutIsPacked(layout) -> bool: + """True when a ctypes layout adds no alignment padding between its fields.""" + declared = sum(sizeof(fieldType) for _, fieldType in layout._fields_) + return sizeof(layout) == declared + + +def requirePackedLayout(layout): + """Refuse a layout that would put padding on the wire. + + Loudly, at description time: the frame it produces is the right shape for a C + compiler and the wrong shape for the instrument, and nothing downstream would + notice. + """ + if not layoutIsPacked(layout): + declared = sum(sizeof(fieldType) for _, fieldType in layout._fields_) + raise ProtocolError( + "{0} packs to {1} bytes but its fields are {2}: set _pack_ = 1, or " + "ctypes aligns them and the frame is wrong".format( + layout.__name__, sizeof(layout), declared)) + + +class FrameRequest(Request): + """A binary request declared as a ctypes Structure. + + constants are the fields the caller never supplies, as with BinaryRequest -- + a header byte, a trailing carriage return. + """ + + def __init__(self, layout, constants: dict = None): + requirePackedLayout(layout) + self.layout = layout + self.constants = dict(constants or {}) + + @property + def fields(self) -> tuple: + return tuple(name for name, _ in self.layout._fields_) + + @property + def arguments(self) -> tuple: + return tuple(name for name in self.fields if name not in self.constants) + + def encode(self, **arguments) -> bytes: + frame = self.layout() + for name in self.fields: + if name in self.constants: + setattr(frame, name, self.constants[name]) + elif name in arguments: + setattr(frame, name, arguments[name]) + else: + raise MissingArgument("{0} needs {1}, got {2}".format( + self.layout.__name__, name, sorted(arguments))) + return bytes(frame) + + +class FrameReply(Reply): + """A binary reply declared as the same kind of ctypes Structure. + + Every field is named, including the terminator, so an acknowledgement byte can + be asserted rather than discarded as padding. + """ + + def __init__(self, layout): + requirePackedLayout(layout) + self.layout = layout + + @property + def readLength(self) -> int: + return sizeof(self.layout) + + def decode(self, data) -> dict: + if len(data) != self.readLength: + raise ReplyDidNotMatch("expected {0} bytes for {1}, got {2}".format( + self.readLength, self.layout.__name__, len(data))) + frame = self.layout.from_buffer_copy(bytes(data)) + return {name: getattr(frame, name) for name, _ in self.layout._fields_} + + class Exchange: """One request and the reply it expects, named for a driver to call by name. @@ -430,5 +521,95 @@ def testTheSR830SnapReplyThatHasNoDescriptionToday(self): {"x": 0.001, "y": -0.002, "magnitude": 0.002236, "phase": -63.4}) +class MoveFrame(LittleEndianStructure): + """The MP-285 MOVE request: a header, three int32 microstep counts, a return.""" + + _pack_ = 1 + _fields_ = [("header", c_char), ("x", c_int32), ("y", c_int32), + ("z", c_int32), ("terminator", c_char)] + + +class PositionFrame(LittleEndianStructure): + """Its GET_POSITION reply: three int32s and the carriage return that ends them.""" + + _pack_ = 1 + _fields_ = [("x", c_int32), ("y", c_int32), ("z", c_int32), + ("terminator", c_char)] + + +class AlignedFrame(LittleEndianStructure): + """The same MOVE fields with _pack_ forgotten, which is the trap.""" + + _fields_ = MoveFrame._fields_ + + +class TestTheCTypesVariant(unittest.TestCase): + def testItWritesTheSameBytesAsTheStructFormat(self): + byFormat = BinaryRequest( + " Date: Thu, 6 Aug 2026 10:50:41 -0400 Subject: [PATCH 03/17] Name the ctypes layout explicitly, for Python 3.14 Problem: running the prototype on Python 3.14 warns twice -- DeprecationWarning: Due to '_pack_', the 'MoveFrame' Structure will use memory layout compatible with MSVC (Windows). If this is intended, set _layout_ to 'ms'. The implicit default is deprecated and slated to become an error in Python 3.19. It does not appear on the 3.13 in the venv, so the tests looked clean while the CI matrix, which runs up to 3.14, would have shown it. Solution: state _layout_ = "ms" on both packed structures. With _pack_ = 1 there is no padding for the two layouts to disagree about, so this only names what was already happening -- the test asserting byte-for-byte equality with the struct format still passes on both interpreters. Python 3.13 and earlier ignore the attribute, so it is safe across the supported range. AlignedFrame is deliberately left alone: it declares no _pack_, because its whole purpose is to be the aligned layout the description refuses. Worth remembering when choosing between the two binary variants: ctypes carries this deprecation, and struct does not. Co-Authored-By: Claude Opus 5 (1M context) --- hardwarelibrary/tests/testProtocolPrototype.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 2d7e49b8..f7ad41fc 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -525,6 +525,11 @@ class MoveFrame(LittleEndianStructure): """The MP-285 MOVE request: a header, three int32 microstep counts, a return.""" _pack_ = 1 + # Naming the layout is required alongside _pack_ from Python 3.14, where + # leaving it implicit warns and becomes an error in 3.19. With _pack_ = 1 + # there is no padding for the two layouts to disagree about, so "ms" only + # states what was already happening. Ignored by Python 3.13 and earlier. + _layout_ = "ms" _fields_ = [("header", c_char), ("x", c_int32), ("y", c_int32), ("z", c_int32), ("terminator", c_char)] @@ -533,6 +538,7 @@ class PositionFrame(LittleEndianStructure): """Its GET_POSITION reply: three int32s and the carriage return that ends them.""" _pack_ = 1 + _layout_ = "ms" _fields_ = [("x", c_int32), ("y", c_int32), ("z", c_int32), ("terminator", c_char)] From 91c7ba086eab84651f9a4472b01f4902867a2f4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Thu, 6 Aug 2026 12:34:50 -0400 Subject: [PATCH 04/17] Drop the ctypes variant: string templates and struct formats it is The ctypes layout was worth trying and is not worth keeping. It brought two real advantages -- one declaration serving both directions, and a named terminator that a driver can assert instead of a discarded pad byte -- but it charges for them twice in the same area. _pack_ = 1 is mandatory or the frame silently gains alignment padding, which needed requirePackedLayout to catch; and Python 3.14 then deprecates leaving the layout implicit alongside _pack_, which needed _layout_ = "ms". Two pieces of ceremony to obtain the bytes that " --- .../tests/testProtocolPrototype.py | 187 ------------------ 1 file changed, 187 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index f7ad41fc..bfb65f56 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -31,7 +31,6 @@ import string import unittest from abc import ABC, abstractmethod -from ctypes import LittleEndianStructure, c_char, c_int32, sizeof from struct import calcsize, error as StructError, pack, unpack @@ -193,96 +192,6 @@ def decode(self, data) -> dict: return dict(zip(self.fields, values)) -# --- The same binary frames, declared as a ctypes layout instead ------------- -# -# A second way to say the same thing, for comparison. Where BinaryRequest and -# BinaryReply each carry a struct format and a tuple of names, a ctypes Structure -# carries both in one declaration and works in both directions: bytes(instance) -# writes the frame, from_buffer_copy() reads it back by name. sizeof() then gives -# the read length, so it cannot drift from the layout. -# -# The cost is a trap, which is why layoutIsPacked exists below: without -# _pack_ = 1, ctypes aligns each field to its natural boundary and a 14-byte -# frame silently becomes 20. struct's "<" gets that right by default. - - -def layoutIsPacked(layout) -> bool: - """True when a ctypes layout adds no alignment padding between its fields.""" - declared = sum(sizeof(fieldType) for _, fieldType in layout._fields_) - return sizeof(layout) == declared - - -def requirePackedLayout(layout): - """Refuse a layout that would put padding on the wire. - - Loudly, at description time: the frame it produces is the right shape for a C - compiler and the wrong shape for the instrument, and nothing downstream would - notice. - """ - if not layoutIsPacked(layout): - declared = sum(sizeof(fieldType) for _, fieldType in layout._fields_) - raise ProtocolError( - "{0} packs to {1} bytes but its fields are {2}: set _pack_ = 1, or " - "ctypes aligns them and the frame is wrong".format( - layout.__name__, sizeof(layout), declared)) - - -class FrameRequest(Request): - """A binary request declared as a ctypes Structure. - - constants are the fields the caller never supplies, as with BinaryRequest -- - a header byte, a trailing carriage return. - """ - - def __init__(self, layout, constants: dict = None): - requirePackedLayout(layout) - self.layout = layout - self.constants = dict(constants or {}) - - @property - def fields(self) -> tuple: - return tuple(name for name, _ in self.layout._fields_) - - @property - def arguments(self) -> tuple: - return tuple(name for name in self.fields if name not in self.constants) - - def encode(self, **arguments) -> bytes: - frame = self.layout() - for name in self.fields: - if name in self.constants: - setattr(frame, name, self.constants[name]) - elif name in arguments: - setattr(frame, name, arguments[name]) - else: - raise MissingArgument("{0} needs {1}, got {2}".format( - self.layout.__name__, name, sorted(arguments))) - return bytes(frame) - - -class FrameReply(Reply): - """A binary reply declared as the same kind of ctypes Structure. - - Every field is named, including the terminator, so an acknowledgement byte can - be asserted rather than discarded as padding. - """ - - def __init__(self, layout): - requirePackedLayout(layout) - self.layout = layout - - @property - def readLength(self) -> int: - return sizeof(self.layout) - - def decode(self, data) -> dict: - if len(data) != self.readLength: - raise ReplyDidNotMatch("expected {0} bytes for {1}, got {2}".format( - self.readLength, self.layout.__name__, len(data))) - frame = self.layout.from_buffer_copy(bytes(data)) - return {name: getattr(frame, name) for name, _ in self.layout._fields_} - - class Exchange: """One request and the reply it expects, named for a driver to call by name. @@ -521,101 +430,5 @@ def testTheSR830SnapReplyThatHasNoDescriptionToday(self): {"x": 0.001, "y": -0.002, "magnitude": 0.002236, "phase": -63.4}) -class MoveFrame(LittleEndianStructure): - """The MP-285 MOVE request: a header, three int32 microstep counts, a return.""" - - _pack_ = 1 - # Naming the layout is required alongside _pack_ from Python 3.14, where - # leaving it implicit warns and becomes an error in 3.19. With _pack_ = 1 - # there is no padding for the two layouts to disagree about, so "ms" only - # states what was already happening. Ignored by Python 3.13 and earlier. - _layout_ = "ms" - _fields_ = [("header", c_char), ("x", c_int32), ("y", c_int32), - ("z", c_int32), ("terminator", c_char)] - - -class PositionFrame(LittleEndianStructure): - """Its GET_POSITION reply: three int32s and the carriage return that ends them.""" - - _pack_ = 1 - _layout_ = "ms" - _fields_ = [("x", c_int32), ("y", c_int32), ("z", c_int32), - ("terminator", c_char)] - - -class AlignedFrame(LittleEndianStructure): - """The same MOVE fields with _pack_ forgotten, which is the trap.""" - - _fields_ = MoveFrame._fields_ - - -class TestTheCTypesVariant(unittest.TestCase): - def testItWritesTheSameBytesAsTheStructFormat(self): - byFormat = BinaryRequest( - " Date: Thu, 6 Aug 2026 12:40:47 -0400 Subject: [PATCH 05/17] Put the request terminator in the template, where it can be seen On the way out a terminator is simply more literal text, so a separate parameter bought nothing and cost clarity: TextRequest("pa?") silently wrote b"pa?\r", and the Integra, which terminates nothing, had to opt out of the default with terminator="". A description whose visible content differs from what goes on the wire is the wrong kind of surprise. Now every template carries its own line ending, and the differences between instruments are legible side by side: "p {power:0.3f}\r" for the Cobolt, "g r{register}\n" for the Intellidrive, "*GWL" for the Integra with nothing at all. TextReply keeps its terminator, because there it is not content. It is the instruction for how far to read, the counterpart of BinaryReply.readLength, and it cannot be written into a pattern that is only applied after the reply has already been read. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index bfb65f56..9bfc36a1 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -62,13 +62,16 @@ class TextRequest(Request): """An ASCII request built from a format template. The template uses named fields, so a caller writes setPower(power=0.5) and - never counts positional arguments: "p {power:0.3f}" with the terminator the - instrument expects appended. + never counts positional arguments: "p {power:0.3f}\r". + + Whatever ends the line is written into the template, where it can be seen, + rather than passed alongside it: on the way out a terminator is simply more + literal text, and instruments disagree about it enough -- \r, \n, \r\n, or + nothing at all for the Integra -- that no default would serve. """ - def __init__(self, template: str, terminator: str = "\r"): + def __init__(self, template: str): self.template = template - self.terminator = terminator @property def fields(self) -> tuple: @@ -82,7 +85,7 @@ def encode(self, **arguments) -> bytes: except KeyError as error: raise MissingArgument("{0} needs {1}, got {2}".format( self.template, error, sorted(arguments))) from None - return (text + self.terminator).encode("utf-8") + return text.encode("utf-8") class BinaryRequest(Request): @@ -224,23 +227,24 @@ def decode(self, data) -> dict: class TestTextRequest(unittest.TestCase): def testBuildsAConstantRequest(self): - self.assertEqual(TextRequest("pa?").encode(), b"pa?\r") + self.assertEqual(TextRequest("pa?\r").encode(), b"pa?\r") def testSubstitutesNamedArguments(self): - request = TextRequest("p {power:0.3f}") + request = TextRequest("p {power:0.3f}\r") self.assertEqual(request.encode(power=0.5), b"p 0.500\r") - def testTerminatorIsPartOfTheDescription(self): - self.assertEqual(TextRequest("*GWL", terminator="").encode(), b"*GWL") - self.assertEqual(TextRequest("g r0xc9", terminator="\n").encode(), b"g r0xc9\n") + def testWhateverEndsTheLineIsVisibleInTheTemplate(self): + self.assertEqual(TextRequest("*GWL").encode(), b"*GWL") + self.assertEqual(TextRequest("g r0xc9\n").encode(), b"g r0xc9\n") + self.assertEqual(TextRequest("SYST:ERR?\r\n").encode(), b"SYST:ERR?\r\n") def testNamesTheFieldsItNeeds(self): - self.assertEqual(TextRequest("s r{register} {value}").fields, + self.assertEqual(TextRequest("s r{register} {value}\r").fields, ("register", "value")) def testAMissingArgumentSaysWhichOne(self): with self.assertRaises(MissingArgument) as raised: - TextRequest("p {power:0.3f}").encode() + TextRequest("p {power:0.3f}\r").encode() self.assertIn("power", str(raised.exception)) @@ -331,15 +335,14 @@ def testItSaysHowItMustBeRead(self): class TestExchange(unittest.TestCase): def testCarriesARequestAndItsReply(self): - exchange = Exchange("GET_POWER", TextRequest("pa?"), + exchange = Exchange("GET_POWER", TextRequest("pa?\r"), TextReply(r"(\d+\.\d+)", fields={"power": float})) self.assertEqual(exchange.encode(), b"pa?\r") self.assertEqual(exchange.decode(b"0.250\r\n"), {"power": 0.25}) self.assertTrue(exchange.expectsReply) def testAnExchangeMayExpectNothingBack(self): - exchange = Exchange("SET_WAVELENGTH", TextRequest("*PWC{wavelength:05d}", - terminator="")) + exchange = Exchange("SET_WAVELENGTH", TextRequest("*PWC{wavelength:05d}")) self.assertFalse(exchange.expectsReply) self.assertEqual(exchange.encode(wavelength=532), b"*PWC00532") with self.assertRaises(ProtocolError): @@ -348,7 +351,7 @@ def testAnExchangeMayExpectNothingBack(self): def testTheDescriptionIsSharedButTheResultIsNot(self): # The point of the whole exercise: two callers of one description cannot # overwrite each other, because nothing is stored on it. - exchange = Exchange("GET_POWER", TextRequest("pa?"), + exchange = Exchange("GET_POWER", TextRequest("pa?\r"), TextReply(r"(\d+\.\d+)", fields={"power": float})) first = exchange.decode(b"0.100\r\n") second = exchange.decode(b"0.900\r\n") @@ -361,17 +364,17 @@ class TestItCanExpressTheProtocolsWeAlreadyHave(unittest.TestCase): """The real test of the design: say what the drivers in this repo actually speak.""" def testCoboltSetAndReadPower(self): - setPower = Exchange("SET_POWER", TextRequest("p {power:0.3f}"), TextReply("OK")) + setPower = Exchange("SET_POWER", TextRequest("p {power:0.3f}\r"), TextReply("OK")) self.assertEqual(setPower.encode(power=0.05), b"p 0.050\r") self.assertEqual(setPower.decode(b"OK\r\n"), {}) - getPower = Exchange("GET_POWER", TextRequest("pa?"), + getPower = Exchange("GET_POWER", TextRequest("pa?\r"), TextReply(r"(\d+\.\d+)", fields={"power": float})) self.assertEqual(getPower.encode(), b"pa?\r") self.assertEqual(getPower.decode(b"0.0499\r\n"), {"power": 0.0499}) def testCoboltOnOffStateAsABoolean(self): - getOnOff = Exchange("GET_ON_OFF", TextRequest("l?"), + getOnOff = Exchange("GET_ON_OFF", TextRequest("l?\r"), TextReply(r"(0|1)", fields={"isOn": lambda text: text == "1"})) self.assertEqual(getOnOff.decode(b"1\r\n"), {"isOn": True}) self.assertEqual(getOnOff.decode(b"0\r\n"), {"isOn": False}) @@ -398,22 +401,22 @@ def testSutterMoveAndPosition(self): def testIntegraWavelengthBothWays(self): getWavelength = Exchange( - "GETWAVELENGTH", TextRequest("*GWL", terminator=""), + "GETWAVELENGTH", TextRequest("*GWL"), TextReply(r"PWC\s*:\s*(.+?)\r\n", fields={"wavelength": float})) self.assertEqual(getWavelength.encode(), b"*GWL") self.assertEqual(getWavelength.decode(b"PWC : 532.0\r\n"), {"wavelength": 532.0}) setWavelength = Exchange("SETWAVELENGTH", - TextRequest("*PWC{wavelength:05d}", terminator="")) + TextRequest("*PWC{wavelength:05d}")) self.assertEqual(setWavelength.encode(wavelength=1064), b"*PWC01064") def testIntellidriveRegisters(self): setRegister = Exchange("SET_REGISTER", - TextRequest("s r{register} {value}"), TextReply("ok")) + TextRequest("s r{register} {value}\r"), TextReply("ok")) self.assertEqual(setRegister.encode(register="0x24", value=31), b"s r0x24 31\r") self.assertEqual(setRegister.decode(b"ok\r"), {}) - getRegister = Exchange("GET_REGISTER", TextRequest("g r{register}", terminator="\n"), + getRegister = Exchange("GET_REGISTER", TextRequest("g r{register}\n"), TextReply(r"v\s(-?\d+)", fields={"value": int})) self.assertEqual(getRegister.encode(register="0xc9"), b"g r0xc9\n") self.assertEqual(getRegister.decode(b"v -1234\r"), {"value": -1234}) @@ -421,7 +424,7 @@ def testIntellidriveRegisters(self): def testTheSR830SnapReplyThatHasNoDescriptionToday(self): # SNAP? returns several comma-separated floats at one instant; the current # Command cannot say that at all, so SR830Device parses it by hand. - snap = Exchange("SNAP", TextRequest("SNAP? 1,2,3,4", terminator="\n"), + snap = Exchange("SNAP", TextRequest("SNAP? 1,2,3,4\n"), TextReply(r"([-\d.eE+]+),([-\d.eE+]+),([-\d.eE+]+),([-\d.eE+]+)", fields={"x": float, "y": float, "magnitude": float, "phase": float})) From f8c4c229bcedd4bec0fa8f5eb9777175dc9e125b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Thu, 6 Aug 2026 12:53:11 -0400 Subject: [PATCH 06/17] A reply parses what it is handed; reading is the caller's business TextReply carried a terminator it never used: decode() only ever ran a regex over whatever arrived. It was there to tell a caller how far to read, but the port already knows that -- CommunicationPort.readString reads up to its own terminator -- so the description was duplicating, and could contradict, a setting that lives elsewhere. Its default of "\r\n" was already wrong for the Intellidrive, which answers with "\r". Now the same description reads a line however that line happened to arrive: the test decodes b"0.123\r\n", b"0.123\n", b"0.123\r", b"0.123" and "0.123" through one TextReply. BinaryReply keeps readLength, and the asymmetry is the point rather than an oversight: a fixed-size frame has no terminator to stop at, so the number of bytes to ask for can come from nowhere but the description. It is the one thing a caller cannot work out for itself. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 9bfc36a1..438b0092 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -16,8 +16,8 @@ - a description is immutable and shareable; the result of an exchange is a plain dict returned to the caller, so two instruments cannot overwrite each other; - - a Reply says how it must be read (a fixed length, or up to a terminator), so - the caller knows what to ask the port for without the description doing it; + - a Reply parses whatever it is handed, and says how many bytes to read only + where nothing else could know: a fixed-size binary frame; - decoding failure raises, naming what did not match, instead of being recorded in an attribute nobody checks. @@ -125,13 +125,14 @@ def encode(self, **arguments) -> bytes: class Reply(ABC): """How to turn the bytes read back into named values. - A reply also says how it must be read: readLength for a fixed-size frame, or - terminator for a line. Exactly one of the two is set, so a caller can always - tell what to ask the port for. + Reading is the caller's business: a reply parses whatever it is handed. The + one thing it must say is readLength, and only when nothing else could know it + -- a fixed-size binary frame has no terminator to stop at, so the number of + bytes to ask for can come from nowhere but the description. A line needs no + such answer, since the port already reads up to its own terminator. """ readLength = None - terminator = None @abstractmethod def decode(self, data: bytes) -> dict: @@ -145,12 +146,14 @@ class TextReply(Reply): {"power": float} turns r"(\\d+\\.\\d+)" into {"power": 0.123}. A reply with no capture groups, such as an "OK" acknowledgement, decodes to an empty dict -- it either matched or it raised. + + It parses whatever it is handed and says nothing about how to read it: where + a line ends is the port's business, not the protocol's. """ - def __init__(self, pattern: str, fields: dict = None, terminator: str = "\r\n"): + def __init__(self, pattern: str, fields: dict = None): self.pattern = pattern self.fields = dict(fields or {}) - self.terminator = terminator def decode(self, data) -> dict: text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data @@ -300,9 +303,12 @@ def testNamingTheWrongNumberOfFieldsIsCaught(self): with self.assertRaises(ProtocolError): reply.decode("1 2") - def testItSaysHowItMustBeRead(self): - reply = TextReply("OK") - self.assertEqual(reply.terminator, "\r\n") + def testItParsesWhateverItIsHanded(self): + # Trailing bytes, or none, are the port's business: the same description + # reads a line however that line happened to arrive. + reply = TextReply(r"(\d+\.\d+)", fields={"power": float}) + for arrival in (b"0.123\r\n", b"0.123\n", b"0.123\r", b"0.123", "0.123"): + self.assertEqual(reply.decode(arrival), {"power": 0.123}) self.assertIsNone(reply.readLength) @@ -327,10 +333,11 @@ def testNamingTheWrongNumberOfFieldsIsCaught(self): with self.assertRaises(ProtocolError): reply.decode(pack(" Date: Thu, 6 Aug 2026 13:10:28 -0400 Subject: [PATCH 07/17] Require an explicit byte order, and rename Exchange to Transaction Two changes to the prototype. Require a byte-order prefix on every struct format. Without one, struct falls back to native sizes and native alignment: "clllc" is 33 bytes on this machine rather than the 14 the MP-285 expects, an "l" is whatever a C long happens to be, and padding appears between the fields. pack("cl", b"M", 1) returns 16 bytes, seven of them padding. The frame is then right for the compiler and wrong for the wire, and nothing downstream would notice. That is the same failure the ctypes variant needed _pack_ = 1 to avoid, which was half the reason for dropping it -- so it is worth saying plainly that struct has the trap too, and that the difference is one character rather than two class attributes. requireExplicitByteOrder now refuses the format at description time and names both sizes. Rename Exchange to Transaction. It is what the library already calls the pairing of a write and its read: CommunicationPort.transactionLock guards exactly that, keeping the two together against other threads. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 109 +++++++++++++----- 1 file changed, 78 insertions(+), 31 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 438b0092..e14f244b 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -5,7 +5,7 @@ judged before any driver depends on it. The idea is sans-I/O, the principle behind h11 and wsproto: the protocol is a pure -transformation over bytes and never performs the exchange. A Request turns named +transformation over bytes and never performs the transaction. A Request turns named arguments into the bytes to write; a Reply turns the bytes read back into named values; neither owns a port, and neither stores what happened. That is what separates this from the current Command, which describes the protocol, builds the @@ -14,7 +14,7 @@ Consequences worth noticing while reading: - - a description is immutable and shareable; the result of an exchange is a plain + - a description is immutable and shareable; the result of an transaction is a plain dict returned to the caller, so two instruments cannot overwrite each other; - a Reply parses whatever it is handed, and says how many bytes to read only where nothing else could know: a fixed-size binary frame; @@ -50,6 +50,26 @@ class ReplyDidNotMatch(ProtocolError): """The bytes read back are not what this reply describes.""" +byteOrderPrefixes = ("<", ">", "!", "=") + + +def requireExplicitByteOrder(format: str): + """Refuse a struct format that does not begin with a byte-order prefix. + + Without one, struct uses native sizes and native alignment: "clllc" is 33 + bytes on this machine rather than the 14 the instrument expects, an "l" is + whatever a C long happens to be, and padding appears between the fields. The + frame is then correct for the compiler and wrong for the wire, and nothing + downstream would notice -- the same failure the ctypes variant needed + _pack_ = 1 to avoid, at the price of one character here. + """ + if not format.startswith(byteOrderPrefixes): + raise ProtocolError( + "{0!r} has no byte-order prefix, so struct would use native sizes and " + "alignment: {1} bytes instead of {2}. Write {3!r}.".format( + format, calcsize(format), calcsize("<" + format), "<" + format)) + + class Request(ABC): """How to turn named arguments into the bytes to write.""" @@ -96,6 +116,7 @@ class BinaryRequest(Request): """ def __init__(self, format: str, fields: tuple = (), constants: dict = None): + requireExplicitByteOrder(format) self.format = format self.fields = tuple(fields) self.constants = dict(constants or {}) @@ -179,6 +200,7 @@ class BinaryReply(Reply): """ def __init__(self, format: str, fields: tuple = ()): + requireExplicitByteOrder(format) self.format = format self.fields = tuple(fields) @@ -198,12 +220,15 @@ def decode(self, data) -> dict: return dict(zip(self.fields, values)) -class Exchange: +class Transaction: """One request and the reply it expects, named for a driver to call by name. - Called Exchange rather than Command because it describes the round trip and - performs none of it: encode() gives the caller the bytes to write, decode() - turns what came back into values, and the caller owns the port in between. + Called Transaction rather than Command because it describes the round trip + and performs none of it: encode() gives the caller the bytes to write, + decode() turns what came back into values, and the caller owns the port in + between. The word is already the library's for that pairing -- it is what + CommunicationPort.transactionLock guards, a write and its read kept together + against other threads. """ def __init__(self, name: str, request: Request, reply: Reply = None): @@ -340,54 +365,76 @@ def testItSaysHowManyBytesToRead(self): self.assertEqual(BinaryReply("", "!", "="): + self.assertEqual(BinaryReply(prefix + "l", fields=("value",)).readLength, 4) + + def testWithoutTheGuardTheLengthWouldBeWrong(self): + # What is actually being prevented: not a crash, a wrong frame. + self.assertEqual(calcsize(" Date: Thu, 6 Aug 2026 14:47:12 -0400 Subject: [PATCH 08/17] Rename Transaction to Command The class describes what to send and what should come back. It carries neither the data nor the transmission, so Command is the accurate word: Exchange and Transaction both name the round trip, which is precisely the part it does not do. It takes the name of the class it would replace, which is right rather than awkward -- the difference is not in what the thing is called but in what it holds. Today's Command describes the protocol, builds the bytes, performs the exchange, and then keeps the reply on itself, on a class attribute shared by every instance of a driver. This one keeps a request and a reply description, and hands the result to the caller. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 76 ++++++++++--------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index e14f244b..668a904f 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -5,16 +5,16 @@ judged before any driver depends on it. The idea is sans-I/O, the principle behind h11 and wsproto: the protocol is a pure -transformation over bytes and never performs the transaction. A Request turns named +transformation over bytes and never performs the exchange. A Request turns named arguments into the bytes to write; a Reply turns the bytes read back into named values; neither owns a port, and neither stores what happened. That is what -separates this from the current Command, which describes the protocol, builds the -bytes, performs the I/O, and then keeps the reply on itself -- on a class -attribute shared by every instance of a driver. +separates this from the Command it would replace, which describes the protocol, +builds the bytes, performs the I/O, and then keeps the reply on itself -- on a +class attribute shared by every instance of a driver. Consequences worth noticing while reading: - - a description is immutable and shareable; the result of an transaction is a plain + - a description is immutable and shareable; the result of a command is a plain dict returned to the caller, so two instruments cannot overwrite each other; - a Reply parses whatever it is handed, and says how many bytes to read only where nothing else could know: a fixed-size binary frame; @@ -220,15 +220,17 @@ def decode(self, data) -> dict: return dict(zip(self.fields, values)) -class Transaction: +class Command: """One request and the reply it expects, named for a driver to call by name. - Called Transaction rather than Command because it describes the round trip - and performs none of it: encode() gives the caller the bytes to write, - decode() turns what came back into values, and the caller owns the port in - between. The word is already the library's for that pairing -- it is what - CommunicationPort.transactionLock guards, a write and its read kept together - against other threads. + A command is a description and nothing else: it says what to send and what + should come back, and carries neither the data nor the transmission. encode() + hands the caller the bytes to write, decode() turns what came back into + values, and the port in between is the caller's. + + That is the whole difference from the Command this replaces, which described + the protocol, built the bytes, performed the exchange, and then kept the + reply on itself. """ def __init__(self, name: str, request: Request, reply: Reply = None): @@ -387,54 +389,54 @@ def testWithoutTheGuardTheLengthWouldBeWrong(self): self.assertNotEqual(calcsize("clllc"), 14) -class TestTransaction(unittest.TestCase): +class TestCommand(unittest.TestCase): def testCarriesARequestAndItsReply(self): - transaction = Transaction("GET_POWER", TextRequest("pa?\r"), + command = Command("GET_POWER", TextRequest("pa?\r"), TextReply(r"(\d+\.\d+)", fields={"power": float})) - self.assertEqual(transaction.encode(), b"pa?\r") - self.assertEqual(transaction.decode(b"0.250\r\n"), {"power": 0.25}) - self.assertTrue(transaction.expectsReply) - - def testATransactionMayExpectNothingBack(self): - transaction = Transaction("SET_WAVELENGTH", TextRequest("*PWC{wavelength:05d}")) - self.assertFalse(transaction.expectsReply) - self.assertEqual(transaction.encode(wavelength=532), b"*PWC00532") + self.assertEqual(command.encode(), b"pa?\r") + self.assertEqual(command.decode(b"0.250\r\n"), {"power": 0.25}) + self.assertTrue(command.expectsReply) + + def testACommandMayExpectNothingBack(self): + command = Command("SET_WAVELENGTH", TextRequest("*PWC{wavelength:05d}")) + self.assertFalse(command.expectsReply) + self.assertEqual(command.encode(wavelength=532), b"*PWC00532") with self.assertRaises(ProtocolError): - transaction.decode(b"anything") + command.decode(b"anything") def testTheDescriptionIsSharedButTheResultIsNot(self): # The point of the whole exercise: two callers of one description cannot # overwrite each other, because nothing is stored on it. - transaction = Transaction("GET_POWER", TextRequest("pa?\r"), + command = Command("GET_POWER", TextRequest("pa?\r"), TextReply(r"(\d+\.\d+)", fields={"power": float})) - first = transaction.decode(b"0.100\r\n") - second = transaction.decode(b"0.900\r\n") + first = command.decode(b"0.100\r\n") + second = command.decode(b"0.900\r\n") self.assertEqual(first, {"power": 0.1}) self.assertEqual(second, {"power": 0.9}) - self.assertEqual(vars(transaction).keys(), {"name", "request", "reply"}) + self.assertEqual(vars(command).keys(), {"name", "request", "reply"}) class TestItCanExpressTheProtocolsWeAlreadyHave(unittest.TestCase): """The real test of the design: say what the drivers in this repo actually speak.""" def testCoboltSetAndReadPower(self): - setPower = Transaction("SET_POWER", TextRequest("p {power:0.3f}\r"), TextReply("OK")) + setPower = Command("SET_POWER", TextRequest("p {power:0.3f}\r"), TextReply("OK")) self.assertEqual(setPower.encode(power=0.05), b"p 0.050\r") self.assertEqual(setPower.decode(b"OK\r\n"), {}) - getPower = Transaction("GET_POWER", TextRequest("pa?\r"), + getPower = Command("GET_POWER", TextRequest("pa?\r"), TextReply(r"(\d+\.\d+)", fields={"power": float})) self.assertEqual(getPower.encode(), b"pa?\r") self.assertEqual(getPower.decode(b"0.0499\r\n"), {"power": 0.0499}) def testCoboltOnOffStateAsABoolean(self): - getOnOff = Transaction("GET_ON_OFF", TextRequest("l?\r"), + getOnOff = Command("GET_ON_OFF", TextRequest("l?\r"), TextReply(r"(0|1)", fields={"isOn": lambda text: text == "1"})) self.assertEqual(getOnOff.decode(b"1\r\n"), {"isOn": True}) self.assertEqual(getOnOff.decode(b"0\r\n"), {"isOn": False}) def testSutterMoveAndPosition(self): - move = Transaction( + move = Command( "MOVE", BinaryRequest(" Date: Thu, 6 Aug 2026 15:04:16 -0400 Subject: [PATCH 09/17] Read a device's commands from JSON into a CommandDictionary A driver's protocol is a table, and a table is data. CommandDictionary. fromFile reads one device's commands and builds the same objects the tests build by hand -- asserted directly: the JSON MOVE and the hand-written MOVE produce the same bytes, and both equal pack(" --- .../tests/testProtocolPrototype.py | 271 ++++++++++++++++++ 1 file changed, 271 insertions(+) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 668a904f..e11a8faa 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -27,8 +27,11 @@ class attribute shared by every instance of a driver. """ import env +import json +import os import re import string +import tempfile import unittest from abc import ABC, abstractmethod from struct import calcsize, error as StructError, pack, unpack @@ -50,6 +53,10 @@ class ReplyDidNotMatch(ProtocolError): """The bytes read back are not what this reply describes.""" +class BadDescription(ProtocolError): + """A command description, usually read from a file, does not make sense.""" + + byteOrderPrefixes = ("<", ">", "!", "=") @@ -251,6 +258,126 @@ def decode(self, data) -> dict: return self.reply.decode(data) +# --- The commands of one device, as data ------------------------------------ +# +# A driver's protocol is a table, and a table is data: this reads one from JSON +# and builds the same objects the tests above build by hand. JSON rather than +# TOML or YAML because it is in the standard library of every Python the package +# supports, where tomllib arrives only in 3.11 and YAML is a dependency. +# +# The one thing a file cannot carry is a callable, so a text field names its +# converter and the name is looked up here. That list is deliberately short: a +# protocol file describes a protocol, and anything needing real code belongs in +# the driver. + +converters = { + "float": float, + "integer": int, + "text": str, + "boolean01": lambda text: text == "1", + "hexInteger": lambda text: int(text, 16), +} + + +def convertersFor(fields: dict, where: str) -> dict: + """Turn {"power": "float"} from a file into {"power": float}.""" + resolved = {} + for name, converterName in fields.items(): + if converterName not in converters: + raise BadDescription("{0}: {1} names the converter {2!r}, which is not one of {3}".format( + where, name, converterName, ", ".join(sorted(converters)))) + resolved[name] = converters[converterName] + return resolved + + +def bytesFrom(text: str) -> bytes: + """A constant byte from a file, latin-1 so that \xfe stays one byte.""" + return text.encode("latin-1") + + +def requestFrom(description: dict, where: str) -> Request: + """Build the Request half of a command from its description.""" + if "template" in description: + return TextRequest(description["template"]) + if "format" in description: + return BinaryRequest( + description["format"], + fields=tuple(description.get("fields", ())), + constants={name: bytesFrom(value) + for name, value in description.get("constants", {}).items()}) + raise BadDescription( + "{0}: a request needs a template, for text, or a format, for binary".format(where)) + + +def replyFrom(description: dict, where: str) -> Reply: + """Build the Reply half, when there is one.""" + if "pattern" in description: + return TextReply(description["pattern"], + fields=convertersFor(description.get("fields", {}), where)) + if "format" in description: + return BinaryReply(description["format"], fields=tuple(description.get("fields", ()))) + raise BadDescription( + "{0}: a reply needs a pattern, for text, or a format, for binary".format(where)) + + +def commandFrom(name: str, description: dict) -> Command: + """Build one named command from its description.""" + where = "command {0!r}".format(name) + if "request" not in description: + raise BadDescription("{0}: no request".format(where)) + reply = description.get("reply") + return Command(name, requestFrom(description["request"], where), + replyFrom(reply, where) if reply is not None else None) + + +class CommandDictionary: + """Every command one device understands, by name. + + Reads like a dict and is built from a file, so a protocol can be read, + reviewed and corrected without touching the driver that speaks it. + """ + + def __init__(self, commands: dict, deviceName: str = None): + self.commands = dict(commands) + self.deviceName = deviceName + + @classmethod + def fromDescription(cls, description: dict) -> "CommandDictionary": + if "commands" not in description: + raise BadDescription("no commands: expected {'device': ..., 'commands': {...}}") + return cls({name: commandFrom(name, one) + for name, one in description["commands"].items()}, + deviceName=description.get("device")) + + @classmethod + def fromJSON(cls, text: str) -> "CommandDictionary": + return cls.fromDescription(json.loads(text)) + + @classmethod + def fromFile(cls, path: str) -> "CommandDictionary": + with open(path, "r") as file: + return cls.fromDescription(json.load(file)) + + @property + def names(self) -> tuple: + return tuple(self.commands) + + def __getitem__(self, name: str) -> Command: + if name not in self.commands: + raise KeyError("{0} has no command {1!r}; it has {2}".format( + self.deviceName or "this device", name, ", ".join(sorted(self.commands)))) + return self.commands[name] + + def __contains__(self, name) -> bool: + return name in self.commands + + def __iter__(self): + return iter(self.commands) + + def __len__(self) -> int: + return len(self.commands) + + # --------------------------------------------------------------------------- # Tests # --------------------------------------------------------------------------- @@ -489,5 +616,149 @@ def testTheSR830SnapReplyThatHasNoDescriptionToday(self): {"x": 0.001, "y": -0.002, "magnitude": 0.002236, "phase": -63.4}) +COBOLT = """ +{ + "device": "Cobolt laser", + "commands": { + "GET_POWER": { + "request": {"template": "pa?\\r"}, + "reply": {"pattern": "(\\\\d+\\\\.\\\\d+)", "fields": {"power": "float"}} + }, + "SET_POWER": { + "request": {"template": "p {power:0.3f}\\r"}, + "reply": {"pattern": "OK"} + }, + "GET_ON_OFF": { + "request": {"template": "l?\\r"}, + "reply": {"pattern": "(0|1)", "fields": {"isOn": "boolean01"}} + }, + "TURN_ON": { + "request": {"template": "l1\\r"}, + "reply": {"pattern": "OK"} + } + } +} +""" + +SUTTER = """ +{ + "device": "Sutter MP-285", + "commands": { + "MOVE": { + "request": {"format": " Date: Thu, 6 Aug 2026 15:07:48 -0400 Subject: [PATCH 10/17] Fold the description builders into CommandDictionary Five module-level functions and a dict existed only to build one class's objects, so they belonged to it. converters is now a class attribute and commandFrom, requestFrom, replyFrom, convertersFor and bytesFrom are classmethods on CommandDictionary; the module namespace keeps only requireExplicitByteOrder, which guards a format wherever one is written. Reading order follows use: the entry points first -- fromFile, fromJSON, fromDescription -- then the machinery they call, then the dict protocol. Being classmethods rather than functions also makes them the extension point. A device whose protocol needs something the description does not cover now subclasses and overrides one of them instead of the module growing another function, and because convertersFor reads cls.converters, a subclass adding a converter of its own works with no other change: class MillenniaCommands(CommandDictionary): converters = dict(CommandDictionary.converters, wattsFromMilliwatts=lambda text: float(text) / 1000) Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 184 ++++++++++-------- 1 file changed, 102 insertions(+), 82 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index e11a8faa..7e41d658 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -258,105 +258,125 @@ def decode(self, data) -> dict: return self.reply.decode(data) -# --- The commands of one device, as data ------------------------------------ -# -# A driver's protocol is a table, and a table is data: this reads one from JSON -# and builds the same objects the tests above build by hand. JSON rather than -# TOML or YAML because it is in the standard library of every Python the package -# supports, where tomllib arrives only in 3.11 and YAML is a dependency. -# -# The one thing a file cannot carry is a callable, so a text field names its -# converter and the name is looked up here. That list is deliberately short: a -# protocol file describes a protocol, and anything needing real code belongs in -# the driver. - -converters = { - "float": float, - "integer": int, - "text": str, - "boolean01": lambda text: text == "1", - "hexInteger": lambda text: int(text, 16), -} - - -def convertersFor(fields: dict, where: str) -> dict: - """Turn {"power": "float"} from a file into {"power": float}.""" - resolved = {} - for name, converterName in fields.items(): - if converterName not in converters: - raise BadDescription("{0}: {1} names the converter {2!r}, which is not one of {3}".format( - where, name, converterName, ", ".join(sorted(converters)))) - resolved[name] = converters[converterName] - return resolved - - -def bytesFrom(text: str) -> bytes: - """A constant byte from a file, latin-1 so that \xfe stays one byte.""" - return text.encode("latin-1") - - -def requestFrom(description: dict, where: str) -> Request: - """Build the Request half of a command from its description.""" - if "template" in description: - return TextRequest(description["template"]) - if "format" in description: - return BinaryRequest( - description["format"], - fields=tuple(description.get("fields", ())), - constants={name: bytesFrom(value) - for name, value in description.get("constants", {}).items()}) - raise BadDescription( - "{0}: a request needs a template, for text, or a format, for binary".format(where)) - - -def replyFrom(description: dict, where: str) -> Reply: - """Build the Reply half, when there is one.""" - if "pattern" in description: - return TextReply(description["pattern"], - fields=convertersFor(description.get("fields", {}), where)) - if "format" in description: - return BinaryReply(description["format"], fields=tuple(description.get("fields", ()))) - raise BadDescription( - "{0}: a reply needs a pattern, for text, or a format, for binary".format(where)) - - -def commandFrom(name: str, description: dict) -> Command: - """Build one named command from its description.""" - where = "command {0!r}".format(name) - if "request" not in description: - raise BadDescription("{0}: no request".format(where)) - reply = description.get("reply") - return Command(name, requestFrom(description["request"], where), - replyFrom(reply, where) if reply is not None else None) - - class CommandDictionary: - """Every command one device understands, by name. - - Reads like a dict and is built from a file, so a protocol can be read, - reviewed and corrected without touching the driver that speaks it. + """Every command one device understands, by name, read from a file. + + A driver's protocol is a table, and a table is data: a description read from + JSON builds the same objects a driver would write by hand, so a protocol can + be read, reviewed and corrected without touching the code that speaks it. + + JSON rather than TOML or YAML because it is in the standard library of every + Python the package supports; tomllib arrives only in 3.11, and YAML would be a + dependency for a file nobody edits at runtime. + + A command is described by a request and, when there is one, a reply. Which key + is present says which kind it is, so nothing declares a type twice: + + "GET_POWER": { + "request": {"template": "pa?\r"}, + "reply": {"pattern": "(\\d+\\.\\d+)", "fields": {"power": "float"}} + }, + "MOVE": { + "request": {"format": " "CommandDictionary": + """Read one device's commands from a JSON file.""" + with open(path, "r") as file: + return cls.fromDescription(json.load(file)) + + @classmethod + def fromJSON(cls, text: str) -> "CommandDictionary": + """Read them from JSON already in hand.""" + return cls.fromDescription(json.loads(text)) + @classmethod def fromDescription(cls, description: dict) -> "CommandDictionary": + """Build from the description itself, however it was obtained.""" if "commands" not in description: raise BadDescription("no commands: expected {'device': ..., 'commands': {...}}") - return cls({name: commandFrom(name, one) + return cls({name: cls.commandFrom(name, one) for name, one in description["commands"].items()}, deviceName=description.get("device")) @classmethod - def fromJSON(cls, text: str) -> "CommandDictionary": - return cls.fromDescription(json.loads(text)) + def commandFrom(cls, name: str, description: dict) -> Command: + """Build one named command from its description.""" + where = "command {0!r}".format(name) + if "request" not in description: + raise BadDescription("{0}: no request".format(where)) + reply = description.get("reply") + return Command(name, cls.requestFrom(description["request"], where), + cls.replyFrom(reply, where) if reply is not None else None) @classmethod - def fromFile(cls, path: str) -> "CommandDictionary": - with open(path, "r") as file: - return cls.fromDescription(json.load(file)) + def requestFrom(cls, description: dict, where: str) -> Request: + """Build the request half: a template for text, a format for binary.""" + if "template" in description: + return TextRequest(description["template"]) + if "format" in description: + return BinaryRequest( + description["format"], + fields=tuple(description.get("fields", ())), + constants={name: cls.bytesFrom(value) + for name, value in description.get("constants", {}).items()}) + raise BadDescription( + "{0}: a request needs a template, for text, or a format, for binary".format(where)) + + @classmethod + def replyFrom(cls, description: dict, where: str) -> Reply: + """Build the reply half: a pattern for text, a format for binary.""" + if "pattern" in description: + return TextReply(description["pattern"], + fields=cls.convertersFor(description.get("fields", {}), where)) + if "format" in description: + return BinaryReply(description["format"], + fields=tuple(description.get("fields", ()))) + raise BadDescription( + "{0}: a reply needs a pattern, for text, or a format, for binary".format(where)) + + @classmethod + def convertersFor(cls, fields: dict, where: str) -> dict: + """Turn {"power": "float"} from a file into {"power": float}.""" + resolved = {} + for name, converterName in fields.items(): + if converterName not in cls.converters: + raise BadDescription( + "{0}: {1} names the converter {2!r}, which is not one of {3}".format( + where, name, converterName, ", ".join(sorted(cls.converters)))) + resolved[name] = cls.converters[converterName] + return resolved + + @staticmethod + def bytesFrom(text: str) -> bytes: + """A constant byte from a file, latin-1 so that \xfe stays one byte.""" + return text.encode("latin-1") @property def names(self) -> tuple: From 58b13171fc5031a9c4a617fd1366eddd9ad4c949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Thu, 6 Aug 2026 15:09:21 -0400 Subject: [PATCH 11/17] Docstring every method of the prototype classes Fifteen methods had none: the two abstract hooks, every __init__, every encode and decode, and the dict protocol on CommandDictionary. The project asks for docstrings on all methods, and these are the ones a driver author reads first. They say what is returned and, where it matters, what is raised and why -- that TextRequest.encode raises MissingArgument naming the field rather than letting a KeyError out of str.format, that decode raises ReplyDidNotMatch quoting both sides, that Command.decode refuses when the command expects no reply because decoding one means the caller read something it should not have. BinaryReply.readLength says the length is taken from the format itself, which is the property the tests pin. Test methods are left alone: their names are already the sentences a docstring would repeat. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 7e41d658..d0b38bc3 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -82,6 +82,7 @@ class Request(ABC): @abstractmethod def encode(self, **arguments) -> bytes: + """Returns the bytes to write, built from the arguments named here.""" ... @@ -98,6 +99,7 @@ class TextRequest(Request): """ def __init__(self, template: str): + """Describe a request as a format template, terminator included.""" self.template = template @property @@ -107,6 +109,11 @@ def fields(self) -> tuple: if name) def encode(self, **arguments) -> bytes: + """Returns the request as bytes, with the arguments substituted. + + Raises MissingArgument, naming the field, rather than letting a KeyError + out of str.format. + """ try: text = self.template.format(**arguments) except KeyError as error: @@ -123,6 +130,11 @@ class BinaryRequest(Request): """ def __init__(self, format: str, fields: tuple = (), constants: dict = None): + """Describe a request as a struct format, one name per packed value. + + The format must state its byte order, or the frame it builds is the one a + C compiler would want rather than the one the instrument expects. + """ requireExplicitByteOrder(format) self.format = format self.fields = tuple(fields) @@ -134,6 +146,11 @@ def arguments(self) -> tuple: return tuple(name for name in self.fields if name not in self.constants) def encode(self, **arguments) -> bytes: + """Returns the packed request, constants and arguments in field order. + + Raises MissingArgument for a field the caller left out, and ProtocolError + for a value struct cannot pack into its format. + """ values = [] for name in self.fields: if name in self.constants: @@ -164,6 +181,7 @@ class Reply(ABC): @abstractmethod def decode(self, data: bytes) -> dict: + """Returns what the reply carried, as {field: value}.""" ... @@ -180,10 +198,17 @@ class TextReply(Reply): """ def __init__(self, pattern: str, fields: dict = None): + """Describe a reply as a pattern, and a converter per capture group.""" self.pattern = pattern self.fields = dict(fields or {}) def decode(self, data) -> dict: + """Returns the captured values, converted and named. + + Accepts bytes or str, since what a port hands back varies. Raises + ReplyDidNotMatch when the pattern does not match, quoting both sides, and + ProtocolError when the pattern and the field names disagree in number. + """ text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data match = re.search(self.pattern, text) if match is None: @@ -207,15 +232,22 @@ class BinaryReply(Reply): """ def __init__(self, format: str, fields: tuple = ()): + """Describe a reply as a struct format, one name per unpacked value.""" requireExplicitByteOrder(format) self.format = format self.fields = tuple(fields) @property def readLength(self) -> int: + """Returns how many bytes to read, taken from the format itself.""" return calcsize(self.format) def decode(self, data) -> dict: + """Returns the unpacked values, named field by field. + + Raises ReplyDidNotMatch when the frame is not the length the format + expects, and ProtocolError when the format and the names disagree. + """ if len(data) != self.readLength: raise ReplyDidNotMatch("expected {0} bytes for {1}, got {2}".format( self.readLength, self.format, len(data))) @@ -241,18 +273,30 @@ class Command: """ def __init__(self, name: str, request: Request, reply: Reply = None): + """Pair a request with the reply it expects, under the name a driver uses. + + reply is None for a command the instrument does not answer. + """ self.name = name self.request = request self.reply = reply @property def expectsReply(self) -> bool: + """True when the instrument answers this command, so a caller knows + whether to read at all.""" return self.reply is not None def encode(self, **arguments) -> bytes: + """Returns the bytes to write for this command.""" return self.request.encode(**arguments) def decode(self, data) -> dict: + """Returns what the reply carried, as {field: value}. + + Raises ProtocolError if this command expects no reply, since decoding one + means the caller read something it should not have. + """ if self.reply is None: raise ProtocolError("{0} expects no reply".format(self.name)) return self.reply.decode(data) @@ -302,6 +346,10 @@ class CommandDictionary: } def __init__(self, commands: dict, deviceName: str = None): + """Hold already-built commands by name. Use fromFile to read a description. + + deviceName is carried only to name the instrument in error messages. + """ self.commands = dict(commands) self.deviceName = deviceName @@ -380,21 +428,26 @@ def bytesFrom(text: str) -> bytes: @property def names(self) -> tuple: + """Returns every command name, in the order the description listed them.""" return tuple(self.commands) def __getitem__(self, name: str) -> Command: + """Returns one command by name, or raises KeyError naming what does exist.""" if name not in self.commands: raise KeyError("{0} has no command {1!r}; it has {2}".format( self.deviceName or "this device", name, ", ".join(sorted(self.commands)))) return self.commands[name] def __contains__(self, name) -> bool: + """True when the device understands a command of that name.""" return name in self.commands def __iter__(self): + """Iterate over the command names, as a dict does.""" return iter(self.commands) def __len__(self) -> int: + """Returns how many commands the device understands.""" return len(self.commands) From 49a6deac731578b191dceaca2963dc20aeb2fdb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Wed, 12 Aug 2026 18:32:12 -0400 Subject: [PATCH 12/17] Describe both directions of a protocol, and let it explain itself Problem: the prototype could only say half of what a driver needs. It built a request and read a reply, but not the mirror -- reading a request and writing a reply -- which is exactly what a table-driven mock does. The Sutter protocol made the gap concrete: SutterDevice's commands dict needed a second struct format to write a position (' --- .../tests/testProtocolPrototype.py | 976 ++++++++++++++---- 1 file changed, 792 insertions(+), 184 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index d0b38bc3..0fa7de7c 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -18,12 +18,26 @@ class attribute shared by every instance of a driver. dict returned to the caller, so two instruments cannot overwrite each other; - a Reply parses whatever it is handed, and says how many bytes to read only where nothing else could know: a fixed-size binary frame; + - a constant is stated once and serves both directions -- written on the way + out, required on the way in -- which is what lets a mock tell one command + from another, and a driver notice an acknowledgement that is not its own; - decoding failure raises, naming what did not match, instead of being recorded in an attribute nobody checks. -Only the first half is built here: writing a request and decoding a reply. The -mirror image -- decoding a request and encoding a reply, which is what a -table-driven mock needs -- is deliberately left out. +Both directions are built here. A driver writes the request and reads the reply; a +mock standing in for the instrument reads the request and writes the reply, out of +the same objects. + +Where the two directions are the same statement, they are written once; where they +are not, they are both written out. A binary frame is packed and unpacked from one +struct format, because pack and unpack are exactly each other's inverse -- asking +for a second format there is how the old description ended up with '", "!", "=") -def requireExplicitByteOrder(format: str): +def requireExplicitByteOrder(struct: str): """Refuse a struct format that does not begin with a byte-order prefix. Without one, struct uses native sizes and native alignment: "clllc" is 33 @@ -70,24 +96,145 @@ def requireExplicitByteOrder(format: str): downstream would notice -- the same failure the ctypes variant needed _pack_ = 1 to avoid, at the price of one character here. """ - if not format.startswith(byteOrderPrefixes): + if not struct.startswith(byteOrderPrefixes): raise ProtocolError( "{0!r} has no byte-order prefix, so struct would use native sizes and " "alignment: {1} bytes instead of {2}. Write {3!r}.".format( - format, calcsize(format), calcsize("<" + format), "<" + format)) + struct, calcsize(struct), calcsize("<" + struct), "<" + struct)) + + +structTypeNames = { + "c": "byte", "?": "bool", "b": "int8", "B": "uint8", + "h": "int16", "H": "uint16", "i": "int32", "I": "uint32", + "l": "int32", "L": "uint32", "q": "int64", "Q": "uint64", + "e": "float16", "f": "float", "d": "double", "s": "bytes", "p": "bytes", +} + + +def structCodes(struct: str) -> tuple: + """The type code of each value a struct format packs, repeat counts expanded. + + Padding yields no value and so no code, which keeps the codes lined up with the + field names one for one. A count on 's' means one string that long, not that + many strings, which is the one place the rule is not simply repetition. + """ + codes = [] + count = "" + for character in struct[1:]: + if character.isdigit(): + count += character + continue + repeats = int(count) if count else 1 + count = "" + if character == "x": + continue + codes.extend([character] if character in "sp" else [character] * repeats) + return tuple(codes) + + +def typeNameOf(converter) -> str: + """The name to show for a converter when explaining a command. + + A converter is often a type, and then its own name is the answer. When it is a + function, what a reader wants is what it returns, which its annotation already + says -- so a "0" or "1" field is announced as a bool rather than by the name of + the function that makes one. + """ + returned = getattr(converter, "__annotations__", {}).get("return") + if returned is not None: + return getattr(returned, "__name__", str(returned)) + return getattr(converter, "__name__", None) or str(converter) + + +def encodeTemplate(template: str, arguments: dict) -> bytes: + """Returns a str.format template as bytes, with the arguments substituted. + + Raises MissingArgument, naming the field, rather than letting a KeyError out + of str.format. + """ + try: + return template.format(**arguments).encode("utf-8") + except KeyError as error: + raise MissingArgument("{0} needs {1}, got {2}".format( + template, error, sorted(arguments))) from None + + +def matchAndConvert(regex: str, converters: dict, data, mismatch) -> dict: + """Returns the capture groups of regex, converted and named. + + Accepts bytes or str, since what a port hands over varies. Raises mismatch -- + which half is being read decides which -- when the expression does not match, + quoting both sides, and ProtocolError when it and the field names disagree in + number. + """ + text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data + match = re.search(regex, text) + if match is None: + raise mismatch("expected {0!r}, got {1!r}".format(regex, text)) + + groups = match.groups() + if len(groups) != len(converters): + raise ProtocolError( + "{0!r} captured {1} group(s) but {2} field(s) were named".format( + regex, len(groups), len(converters))) + return {name: converter(value) + for (name, converter), value in zip(converters.items(), groups)} -class Request(ABC): - """How to turn named arguments into the bytes to write.""" +class Frame(ABC): + """One half of a command: how to write it, and how to read it back. + + Both halves are needed in both directions, which is the whole reason a + description is worth having. A driver writes the request and reads the reply; + a mock standing in for the instrument reads the request and writes the reply. + Same objects, opposite roles -- the protocol is stated once and neither side + owns it. + + readLength is what a reader must be told when nothing else could know it: a + fixed-size binary frame has no terminator to stop at, so the number of bytes to + ask for can come from nowhere but the description. A line needs no such answer, + since the port already reads up to its own terminator. + """ + + mismatch = DidNotMatch + readLength = None @abstractmethod - def encode(self, **arguments) -> bytes: - """Returns the bytes to write, built from the arguments named here.""" + def encode(self, **values) -> bytes: + """Returns the bytes to write, built from the values named here.""" ... + @abstractmethod + def decode(self, data) -> dict: + """Returns what the bytes carried, as {field: value}.""" + ... + + @property + @abstractmethod + def parameters(self) -> tuple: + """Every value this half carries, as (name, type name) in order. + + Not used to encode or decode anything: it is what lets a dictionary explain + itself, so that the names a caller passes and the names it gets back are + read off the description rather than out of a comment. + """ + ... + + +class Request(Frame): + """The half a driver writes and a mock reads.""" + + mismatch = RequestDidNotMatch + + +class Reply(Frame): + """The half a driver reads and a mock writes.""" + + mismatch = ReplyDidNotMatch + class TextRequest(Request): - """An ASCII request built from a format template. + """An ASCII request built from a str.format template. The template uses named fields, so a caller writes setPower(power=0.5) and never counts positional arguments: "p {power:0.3f}\r". @@ -96,60 +243,98 @@ class TextRequest(Request): rather than passed alongside it: on the way out a terminator is simply more literal text, and instruments disagree about it enough -- \r, \n, \r\n, or nothing at all for the Integra -- that no default would serve. + + regex is how a mock recognises the request and reads its arguments back, and it + is written out rather than worked out from the template. A template could be + turned into an expression by guessing a pattern per format spec, and the guess + would be right often enough to be trusted and wrong quietly: a field with no + spec would come back as text, and nothing would say so. This module already + refuses to let struct guess a byte order; guessing an expression is not of a + different kind. """ - def __init__(self, template: str): - """Describe a request as a format template, terminator included.""" + def __init__(self, template: str, regex: str, fields: dict = None): + """Describe a request as the template that writes it and the expression + that reads it back, with a converter per capture group.""" self.template = template + self.regex = regex + self.fields = dict(fields or {}) @property - def fields(self) -> tuple: + def arguments(self) -> tuple: """The argument names this request needs, in the order they appear.""" return tuple(name for _, name, _, _ in string.Formatter().parse(self.template) if name) + @property + def parameters(self) -> tuple: + """Each argument with its type, taken from the converter named for it.""" + return tuple((name, typeNameOf(self.fields[name]) if name in self.fields else "text") + for name in self.arguments) + def encode(self, **arguments) -> bytes: - """Returns the request as bytes, with the arguments substituted. + """Returns the request as bytes, with the arguments substituted.""" + return encodeTemplate(self.template, arguments) - Raises MissingArgument, naming the field, rather than letting a KeyError - out of str.format. - """ - try: - text = self.template.format(**arguments) - except KeyError as error: - raise MissingArgument("{0} needs {1}, got {2}".format( - self.template, error, sorted(arguments))) from None - return text.encode("utf-8") + def decode(self, data) -> dict: + """Returns the arguments the request carried -- what a mock reads.""" + return matchAndConvert(self.regex, self.fields, data, self.mismatch) -class BinaryRequest(Request): - """A packed binary request. +class BinaryFrame(Frame): + """A fixed-size binary frame, named field by field, readable and writable. fields names each value in the struct format, in order; constants are the ones - the caller never supplies, such as a header byte or a trailing carriage return. + nobody supplies, such as a header byte or a trailing carriage return. A + constant is written on the way out and required on the way in, so the one line + that produces b"M" also refuses a frame that does not begin with it -- which is + how a mock tells one command from another, and how a driver is told that an + acknowledgement was not the one it expected. + + Padding ('x') has no place here. It can be unpacked but not packed, so a frame + that skipped its terminator could be read and never written, and the mock + direction would need a second format for the same bytes. Name the byte and make + it a constant instead. """ - def __init__(self, format: str, fields: tuple = (), constants: dict = None): - """Describe a request as a struct format, one name per packed value. + def __init__(self, struct: str, fields: tuple = (), constants: dict = None): + """Describe a frame as a struct format, one name per packed value. The format must state its byte order, or the frame it builds is the one a C compiler would want rather than the one the instrument expects. """ - requireExplicitByteOrder(format) - self.format = format + requireExplicitByteOrder(struct) + self.struct = struct self.fields = tuple(fields) self.constants = dict(constants or {}) + unknown = sorted(set(self.constants) - set(self.fields)) + if unknown: + raise BadDescription("{0} fixes {1}, which {2} does not name".format( + self.struct, ", ".join(unknown), list(self.fields))) + + @property + def readLength(self) -> int: + """Returns how many bytes the frame occupies, taken from the format itself.""" + return calcsize(self.struct) + @property def arguments(self) -> tuple: """The names a caller must supply: every field that is not a constant.""" return tuple(name for name in self.fields if name not in self.constants) + @property + def parameters(self) -> tuple: + """Each non-constant field with the type its format packs it as.""" + codeOf = dict(zip(self.fields, structCodes(self.struct))) + return tuple((name, structTypeNames.get(codeOf.get(name), "unknown")) + for name in self.arguments) + def encode(self, **arguments) -> bytes: - """Returns the packed request, constants and arguments in field order. + """Returns the packed frame, constants and arguments in field order. - Raises MissingArgument for a field the caller left out, and ProtocolError - for a value struct cannot pack into its format. + Raises MissingArgument for a field nobody supplied, and ProtocolError for a + value struct cannot pack into its format. """ values = [] for name in self.fields: @@ -159,30 +344,41 @@ def encode(self, **arguments) -> bytes: values.append(arguments[name]) else: raise MissingArgument("{0} needs {1}, got {2}".format( - self.format, name, sorted(arguments))) + self.struct, name, sorted(arguments))) try: - return pack(self.format, *values) + return pack(self.struct, *values) except StructError as error: raise ProtocolError("cannot pack {0} into {1}: {2}".format( - values, self.format, error)) from None + values, self.struct, error)) from None + def decode(self, data) -> dict: + """Returns the values the frame carried, named, constants left out. -class Reply(ABC): - """How to turn the bytes read back into named values. + A constant is checked rather than returned: it carries no information, and + a frame whose header is wrong is not this frame at all. Raises mismatch + when the length or a constant is wrong, and ProtocolError when the format + and the names disagree. + """ + if len(data) != self.readLength: + raise self.mismatch("expected {0} bytes for {1}, got {2}".format( + self.readLength, self.struct, len(data))) + values = unpack(self.struct, bytes(data)) + if len(values) != len(self.fields): + raise ProtocolError( + "{0} unpacks {1} value(s) but {2} field(s) were named".format( + self.struct, len(values), len(self.fields))) - Reading is the caller's business: a reply parses whatever it is handed. The - one thing it must say is readLength, and only when nothing else could know it - -- a fixed-size binary frame has no terminator to stop at, so the number of - bytes to ask for can come from nowhere but the description. A line needs no - such answer, since the port already reads up to its own terminator. - """ + named = dict(zip(self.fields, values)) + for name, constant in self.constants.items(): + if named[name] != constant: + raise self.mismatch("{0} expects {1}={2!r}, got {3!r}".format( + self.struct, name, constant, named[name])) + return {name: value for name, value in named.items() + if name not in self.constants} - readLength = None - @abstractmethod - def decode(self, data: bytes) -> dict: - """Returns what the reply carried, as {field: value}.""" - ... +class BinaryRequest(BinaryFrame, Request): + """The binary half a driver writes and a mock recognises.""" class TextReply(Reply): @@ -195,68 +391,43 @@ class TextReply(Reply): It parses whatever it is handed and says nothing about how to read it: where a line ends is the port's business, not the protocol's. - """ - - def __init__(self, pattern: str, fields: dict = None): - """Describe a reply as a pattern, and a converter per capture group.""" - self.pattern = pattern - self.fields = dict(fields or {}) - - def decode(self, data) -> dict: - """Returns the captured values, converted and named. - - Accepts bytes or str, since what a port hands back varies. Raises - ReplyDidNotMatch when the pattern does not match, quoting both sides, and - ProtocolError when the pattern and the field names disagree in number. - """ - text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data - match = re.search(self.pattern, text) - if match is None: - raise ReplyDidNotMatch("expected {0!r}, got {1!r}".format(self.pattern, text)) - - groups = match.groups() - if len(groups) != len(self.fields): - raise ProtocolError( - "{0!r} captured {1} group(s) but {2} field(s) were named".format( - self.pattern, len(groups), len(self.fields))) - return {name: converter(value) - for (name, converter), value in zip(self.fields.items(), groups)} + template is what a mock writes to play the instrument, and it is required for + the same reason the request's regex is: nothing here guesses. A reply that + cannot be written is a reply that cannot be tested, and every device in this + library is meant to ship a debug twin. -class BinaryReply(Reply): - """A fixed-size binary frame, named field by field. - - The struct format decides how many bytes to read, so readLength never has to - be kept in step by hand. Padding in the format ('x') yields no value and so - takes no field name. + It also carries what the regex deliberately leaves out. "OK" matches b"OK\r\n" + because re.search ignores what follows, but a mock has to send the terminator, + so the template is where the line the instrument really sends is finally + written down -- exactly as a request writes its own terminator into its + template rather than passing it alongside. """ - def __init__(self, format: str, fields: tuple = ()): - """Describe a reply as a struct format, one name per unpacked value.""" - requireExplicitByteOrder(format) - self.format = format - self.fields = tuple(fields) + def __init__(self, regex: str, template: str, fields: dict = None): + """Describe a reply as the expression that reads it and the template that + writes it, with a converter per capture group.""" + self.regex = regex + self.template = template + self.fields = dict(fields or {}) @property - def readLength(self) -> int: - """Returns how many bytes to read, taken from the format itself.""" - return calcsize(self.format) + def parameters(self) -> tuple: + """Each value the reply carries, with the type it is converted to.""" + return tuple((name, typeNameOf(converter)) + for name, converter in self.fields.items()) def decode(self, data) -> dict: - """Returns the unpacked values, named field by field. + """Returns the captured values, converted and named.""" + return matchAndConvert(self.regex, self.fields, data, self.mismatch) - Raises ReplyDidNotMatch when the frame is not the length the format - expects, and ProtocolError when the format and the names disagree. - """ - if len(data) != self.readLength: - raise ReplyDidNotMatch("expected {0} bytes for {1}, got {2}".format( - self.readLength, self.format, len(data))) - values = unpack(self.format, bytes(data)) - if len(values) != len(self.fields): - raise ProtocolError( - "{0} unpacks {1} value(s) but {2} field(s) were named".format( - self.format, len(values), len(self.fields))) - return dict(zip(self.fields, values)) + def encode(self, **values) -> bytes: + """Returns the line the instrument would answer -- what a mock writes.""" + return encodeTemplate(self.template, values) + + +class BinaryReply(BinaryFrame, Reply): + """The binary half a driver reads and a mock answers with.""" class Command: @@ -267,6 +438,12 @@ class Command: hands the caller the bytes to write, decode() turns what came back into values, and the port in between is the caller's. + The same description read from the other end plays the instrument: + decodeRequest() is what a mock hears, encodeReply() what it answers. Two pairs + of methods, one protocol, no second table to keep in step -- where the Command + this replaces needed a request decoder and a reply encoder written out beside + the request encoder and the reply decoder. + That is the whole difference from the Command this replaces, which described the protocol, built the bytes, performed the exchange, and then kept the reply on itself. @@ -301,6 +478,34 @@ def decode(self, data) -> dict: raise ProtocolError("{0} expects no reply".format(self.name)) return self.reply.decode(data) + def decodeRequest(self, data) -> dict: + """Returns the arguments a request carried -- the mock's half of encode. + + Raises RequestDidNotMatch when the bytes are some other command's request, + which is how a mock picks the right one out of a dictionary. + """ + return self.request.decode(data) + + def encodeReply(self, **values) -> bytes: + """Returns the bytes the instrument would answer -- the mock's half of decode. + + Raises ProtocolError if this command expects no reply, since a mock that + answers one would be answering a command the instrument leaves silent. + """ + if self.reply is None: + raise ProtocolError("{0} expects no reply".format(self.name)) + return self.reply.encode(**values) + + +def booleanFromZeroOrOne(text: str) -> bool: + """A reply of "0" or "1" as a boolean.""" + return text == "1" + + +def integerFromHexadecimal(text: str) -> int: + """A reply written in hexadecimal as an integer.""" + return int(text, 16) + class CommandDictionary: """Every command one device understands, by name, read from a file. @@ -314,19 +519,30 @@ class CommandDictionary: dependency for a file nobody edits at runtime. A command is described by a request and, when there is one, a reply. Which key - is present says which kind it is, so nothing declares a type twice: + is present says which kind it is, so nothing declares a type twice, and the key + names the notation its string is written in: a str.format template on the way + out, a regular expression on the way in, a struct format for bytes in either + direction. "GET_POWER": { - "request": {"template": "pa?\r"}, - "reply": {"pattern": "(\\d+\\.\\d+)", "fields": {"power": "float"}} + "request": {"template": "pa?\r", "regex": "pa\\?\r"}, + "reply": {"regex": "(\\d+\\.\\d+)", "template": "{power:0.4f}\r\n", + "fields": {"power": "float"}} }, "MOVE": { - "request": {"format": " Command: @classmethod def requestFrom(cls, description: dict, where: str) -> Request: - """Build the request half: a template for text, a format for binary.""" + """Build the request half: a template for text, a struct for binary.""" if "template" in description: - return TextRequest(description["template"]) - if "format" in description: - return BinaryRequest( - description["format"], - fields=tuple(description.get("fields", ())), - constants={name: cls.bytesFrom(value) - for name, value in description.get("constants", {}).items()}) + if "regex" not in description: + raise BadDescription( + "{0}: a text request needs a regex as well as its template, " + "so that a mock can read what the driver writes".format(where)) + return TextRequest(description["template"], description["regex"], + fields=cls.convertersFor(description.get("fields", {}), where)) + if "struct" in description: + return BinaryRequest(description["struct"], + fields=tuple(description.get("fields", ())), + constants=cls.constantsFrom(description)) raise BadDescription( - "{0}: a request needs a template, for text, or a format, for binary".format(where)) + "{0}: a request needs a template, for text, or a struct, for binary".format(where)) @classmethod def replyFrom(cls, description: dict, where: str) -> Reply: - """Build the reply half: a pattern for text, a format for binary.""" - if "pattern" in description: - return TextReply(description["pattern"], + """Build the reply half: a regex for text, a struct for binary. + + A text reply states both notations, like a text request; a binary one needs + only its struct, since pack and unpack are already each other's inverse. + """ + if "regex" in description: + if "template" not in description: + raise BadDescription( + "{0}: a text reply needs a template as well as its regex, " + "so that a mock can write what the driver reads".format(where)) + return TextReply(description["regex"], description["template"], fields=cls.convertersFor(description.get("fields", {}), where)) - if "format" in description: - return BinaryReply(description["format"], - fields=tuple(description.get("fields", ()))) + if "struct" in description: + return BinaryReply(description["struct"], + fields=tuple(description.get("fields", ())), + constants=cls.constantsFrom(description)) raise BadDescription( - "{0}: a reply needs a pattern, for text, or a format, for binary".format(where)) + "{0}: a reply needs a regex, for text, or a struct, for binary".format(where)) + + @classmethod + def constantsFrom(cls, description: dict) -> dict: + """Turn the constants of a binary frame from text in a file into bytes.""" + return {name: cls.bytesFrom(value) + for name, value in description.get("constants", {}).items()} @classmethod def convertersFor(cls, fields: dict, where: str) -> dict: @@ -438,6 +672,60 @@ def __getitem__(self, name: str) -> Command: self.deviceName or "this device", name, ", ".join(sorted(self.commands)))) return self.commands[name] + def recognize(self, data) -> tuple: + """Returns the command a request belongs to, and the arguments it carried. + + Every command is asked in turn whether the bytes are its request, and the + first that says yes wins -- so a mock dispatches on the same descriptions + the driver sends with, and no prefix table is written twice. + + It is handed one complete request. Deciding where a request ends in a + stream is the port's business, exactly as it is on the reply side. + """ + for command in self.commands.values(): + try: + return command, command.decodeRequest(data) + except DidNotMatch: + continue + raise RequestDidNotMatch("{0} has no command matching {1!r}".format( + self.deviceName or "this device", data)) + + def usage(self) -> str: + """Returns every command, what to pass it and what it answers, as text. + + This is what someone needs before writing a single call: the names to give + and the names that come back. It is read off the description, so unlike a + comment or a README it cannot say something the protocol no longer does. + + Commands appear in the order the description listed them, since that order + is usually the one whoever wrote the file thought in. + """ + lines = ["{0}: {1} commands".format(self.deviceName or "This device", len(self))] + for name in self.names: + lines.extend(self.usageFor(self.commands[name])) + return "\n".join(lines) + + def usageFor(self, command: Command) -> list: + """The lines explaining one command: how to call it, what comes back.""" + lines = ["", " {0}({1})".format(command.name, self.listed(command.request))] + if not command.expectsReply: + lines.append(" the instrument does not answer") + elif not command.reply.parameters: + lines.append(" answers, carrying no values") + else: + lines.append(" answers {0}".format(self.listed(command.reply))) + return lines + + @staticmethod + def listed(frame: Frame) -> str: + """One half's values as "name: type, name: type", for a usage line.""" + return ", ".join("{0}: {1}".format(name, typeName) + for name, typeName in frame.parameters) + + def __str__(self) -> str: + """The usage text, so that printing a dictionary explains it.""" + return self.usage() + def __contains__(self, name) -> bool: """True when the device understands a command of that name.""" return name in self.commands @@ -457,24 +745,27 @@ def __len__(self) -> int: class TestTextRequest(unittest.TestCase): def testBuildsAConstantRequest(self): - self.assertEqual(TextRequest("pa?\r").encode(), b"pa?\r") + self.assertEqual(TextRequest("pa?\r", r"pa\?\r").encode(), b"pa?\r") def testSubstitutesNamedArguments(self): - request = TextRequest("p {power:0.3f}\r") + request = TextRequest("p {power:0.3f}\r", r"p ([0-9.]+)\r", + fields={"power": float}) self.assertEqual(request.encode(power=0.5), b"p 0.500\r") def testWhateverEndsTheLineIsVisibleInTheTemplate(self): - self.assertEqual(TextRequest("*GWL").encode(), b"*GWL") - self.assertEqual(TextRequest("g r0xc9\n").encode(), b"g r0xc9\n") - self.assertEqual(TextRequest("SYST:ERR?\r\n").encode(), b"SYST:ERR?\r\n") + self.assertEqual(TextRequest("*GWL", r"\*GWL").encode(), b"*GWL") + self.assertEqual(TextRequest("g r0xc9\n", "g r0xc9\n").encode(), b"g r0xc9\n") + self.assertEqual(TextRequest("SYST:ERR?\r\n", r"SYST:ERR\?\r\n").encode(), + b"SYST:ERR?\r\n") - def testNamesTheFieldsItNeeds(self): - self.assertEqual(TextRequest("s r{register} {value}\r").fields, - ("register", "value")) + def testNamesTheArgumentsItNeeds(self): + request = TextRequest("s r{register} {value}\r", r"s r(\S+) (-?\d+)\r", + fields={"register": str, "value": int}) + self.assertEqual(request.arguments, ("register", "value")) def testAMissingArgumentSaysWhichOne(self): with self.assertRaises(MissingArgument) as raised: - TextRequest("p {power:0.3f}\r").encode() + TextRequest("p {power:0.3f}\r", r"p ([0-9.]+)\r").encode() self.assertIn("power", str(raised.exception)) @@ -509,35 +800,44 @@ def testAValueOfTheWrongKindIsRefusedWithItsFormat(self): class TestTextReply(unittest.TestCase): def testDecodesNamedGroupsThroughTheirConverters(self): - reply = TextReply(r"(\d+\.\d+)", fields={"power": float}) + reply = TextReply(r"(\d+\.\d+)", "{power:0.3f}\r\n", fields={"power": float}) self.assertEqual(reply.decode(b"0.123\r\n"), {"power": 0.123}) def testDecodesSeveralFieldsInGroupOrder(self): - reply = TextReply(r"v\s(-?\d+)\s(\d+)", fields={"position": int, "status": int}) + reply = TextReply(r"v\s(-?\d+)\s(\d+)", "v {position} {status}\r", + fields={"position": int, "status": int}) self.assertEqual(reply.decode("v -42 3"), {"position": -42, "status": 3}) def testAnAcknowledgementCarriesNoValues(self): - self.assertEqual(TextReply("OK").decode(b"OK\r\n"), {}) + self.assertEqual(TextReply("OK", "OK\r\n").decode(b"OK\r\n"), {}) def testAReplyThatDoesNotMatchRaisesWithBothSides(self): - reply = TextReply(r"(\d+\.\d+)", fields={"power": float}) + reply = TextReply(r"(\d+\.\d+)", "{power:0.3f}\r\n", fields={"power": float}) with self.assertRaises(ReplyDidNotMatch) as raised: reply.decode(b"syntax error\r\n") self.assertIn("syntax error", str(raised.exception)) def testNamingTheWrongNumberOfFieldsIsCaught(self): - reply = TextReply(r"(\d+)\s(\d+)", fields={"only": int}) + reply = TextReply(r"(\d+)\s(\d+)", "{only}\r", fields={"only": int}) with self.assertRaises(ProtocolError): reply.decode("1 2") def testItParsesWhateverItIsHanded(self): # Trailing bytes, or none, are the port's business: the same description # reads a line however that line happened to arrive. - reply = TextReply(r"(\d+\.\d+)", fields={"power": float}) + reply = TextReply(r"(\d+\.\d+)", "{power:0.3f}\r\n", fields={"power": float}) for arrival in (b"0.123\r\n", b"0.123\n", b"0.123\r", b"0.123", "0.123"): self.assertEqual(reply.decode(arrival), {"power": 0.123}) self.assertIsNone(reply.readLength) + def testTheTemplateSaysWhatTheRegexLeavesOut(self): + # The regex matches with or without the terminator, because re.search does + # not care what follows. The template has to be exact: it is what a mock + # actually puts on the wire. + reply = TextReply("OK", "OK\r\n") + self.assertEqual(reply.encode(), b"OK\r\n") + self.assertEqual(reply.decode(reply.encode()), {}) + class TestBinaryReply(unittest.TestCase): def testLengthComesFromTheFormat(self): @@ -589,16 +889,24 @@ def testWithoutTheGuardTheLengthWouldBeWrong(self): self.assertNotEqual(calcsize("clllc"), 14) +def getPowerCommand() -> Command: + """The Cobolt power query, stated in full: both halves, both directions.""" + return Command("GET_POWER", + TextRequest("pa?\r", r"pa\?\r"), + TextReply(r"(\d+\.\d+)", "{power:0.4f}\r\n", fields={"power": float})) + + class TestCommand(unittest.TestCase): def testCarriesARequestAndItsReply(self): - command = Command("GET_POWER", TextRequest("pa?\r"), - TextReply(r"(\d+\.\d+)", fields={"power": float})) + command = getPowerCommand() self.assertEqual(command.encode(), b"pa?\r") self.assertEqual(command.decode(b"0.250\r\n"), {"power": 0.25}) self.assertTrue(command.expectsReply) def testACommandMayExpectNothingBack(self): - command = Command("SET_WAVELENGTH", TextRequest("*PWC{wavelength:05d}")) + command = Command("SET_WAVELENGTH", + TextRequest("*PWC{wavelength:05d}", r"\*PWC(\d{5})", + fields={"wavelength": int})) self.assertFalse(command.expectsReply) self.assertEqual(command.encode(wavelength=532), b"*PWC00532") with self.assertRaises(ProtocolError): @@ -607,8 +915,7 @@ def testACommandMayExpectNothingBack(self): def testTheDescriptionIsSharedButTheResultIsNot(self): # The point of the whole exercise: two callers of one description cannot # overwrite each other, because nothing is stored on it. - command = Command("GET_POWER", TextRequest("pa?\r"), - TextReply(r"(\d+\.\d+)", fields={"power": float})) + command = getPowerCommand() first = command.decode(b"0.100\r\n") second = command.decode(b"0.900\r\n") self.assertEqual(first, {"power": 0.1}) @@ -616,24 +923,119 @@ def testTheDescriptionIsSharedButTheResultIsNot(self): self.assertEqual(vars(command).keys(), {"name", "request", "reply"}) +class TestTheMirrorDirection(unittest.TestCase): + """Reading a request and writing a reply: the same descriptions, other way round.""" + + def testWhatATemplateWroteItsRegexReadsBack(self): + # The two notations are written separately and have to agree. Nothing + # enforces that but a round trip, which is why every command here has one. + request = TextRequest("p {power:0.3f}\r", r"p ([0-9.]+)\r", + fields={"power": float}) + self.assertEqual(request.decode(request.encode(power=0.5)), {"power": 0.5}) + + def testTheRegexSaysWhatEachFieldWasRatherThanTheFormatSpec(self): + # A converter is named, not guessed from "05d" or "x". The description says + # what comes back, so "0x24" stays text and 31 comes back an int because + # each was asked for. + wavelength = TextRequest("*PWC{wavelength:05d}", r"\*PWC(\d{5})", + fields={"wavelength": int}) + self.assertEqual(wavelength.decode(b"*PWC00532"), {"wavelength": 532}) + + register = TextRequest("s r{register} {value}\r", r"s r(\S+) (-?\d+)\r", + fields={"register": str, "value": int}) + self.assertEqual(register.decode(register.encode(register="0x24", value=31)), + {"register": "0x24", "value": 31}) + + def testARequestThatBelongsToAnotherCommandIsDeclined(self): + with self.assertRaises(RequestDidNotMatch): + TextRequest("pa?\r", r"pa\?\r").decode(b"l?\r") + + def testABinaryRequestGivesBackOnlyWhatWasNotConstant(self): + request = BinaryRequest(" Date: Wed, 12 Aug 2026 19:11:50 -0400 Subject: [PATCH 13/17] Collapse the frame hierarchy: a frame has no role, a command gives it one Problem: nine classes described two notations. Frame, Request, Reply, TextFrame, TextRequest, TextReply, BinaryFrame, BinaryRequest and BinaryReply, of which six were empty or nearly so, in a 2x2 grid that a third notation would have grown by three. Request and Reply carried one attribute between them, mismatch, which is not a property of a frame at all: whether bytes are a request or a reply depends on the slot of the command they belong to, and that was already written there. The same fact was stated twice, which is what this description exists to avoid. The text pair had no common base either, so everything they shared had leaked out to module scope: encodeTemplate, matchAndConvert, typeNameOf, structCodes. The clearest symptom was matchAndConvert taking a mismatch parameter, which was only ever self.mismatch -- an attribute passed as an argument because the code holding it was not a method. Solution: TextFrame, the symmetric counterpart of BinaryFrame, holds everything the two text halves shared. encodeTemplate and matchAndConvert stop existing, because they were encode and decode; typeNameOf and structCodes become methods of the frame that uses them, and the struct type names a class attribute a subclass can extend, the way CommandDictionary does with converters. Request, Reply and the four role classes are then gone. A frame raises the plain DidNotMatch; Command.decodeHalf turns that into RequestDidNotMatch or ReplyDidNotMatch depending on which slot was being read, and names itself while it is there -- so the message gained the one thing no frame could supply: before ReplyDidNotMatch: expected '(\d+\.\d+)', got 'syntax error' after ReplyDidNotMatch: GET_POWER: expected '(\d+\.\d+)', got 'syntax error' Text now has one signature in both slots, (template, regex, fields), so the two strings can no longer be given the wrong way round. Two tests hold the decision: one that a frame reports only that it did not match while the command supplies which half of which command, and one that puts a single frame in both slots of an echo command, which the old hierarchy made impossible to write. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 527 +++++++++--------- 1 file changed, 258 insertions(+), 269 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 0fa7de7c..0ed4655b 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -5,19 +5,22 @@ judged before any driver depends on it. The idea is sans-I/O, the principle behind h11 and wsproto: the protocol is a pure -transformation over bytes and never performs the exchange. A Request turns named -arguments into the bytes to write; a Reply turns the bytes read back into named -values; neither owns a port, and neither stores what happened. That is what -separates this from the Command it would replace, which describes the protocol, -builds the bytes, performs the I/O, and then keeps the reply on itself -- on a -class attribute shared by every instance of a driver. +transformation over bytes and never performs the exchange. A Frame turns named +values into the bytes to write and the bytes read back into named values; it owns +no port and stores nothing of what happened. That is what separates this from the +Command it would replace, which describes the protocol, builds the bytes, performs +the I/O, and then keeps the reply on itself -- on a class attribute shared by every +instance of a driver. Consequences worth noticing while reading: - a description is immutable and shareable; the result of a command is a plain dict returned to the caller, so two instruments cannot overwrite each other; - - a Reply parses whatever it is handed, and says how many bytes to read only + - a frame parses whatever it is handed, and says how many bytes to read only where nothing else could know: a fixed-size binary frame; + - there is no request class and no reply class: a frame becomes one or the + other by the slot of the Command it sits in, which is also the only place + that knows enough to name what failed to match; - a constant is stated once and serves both directions -- written on the way out, required on the way in -- which is what lets a mock tell one command from another, and a driver notice an acknowledgement that is not its own; @@ -103,87 +106,13 @@ def requireExplicitByteOrder(struct: str): struct, calcsize(struct), calcsize("<" + struct), "<" + struct)) -structTypeNames = { - "c": "byte", "?": "bool", "b": "int8", "B": "uint8", - "h": "int16", "H": "uint16", "i": "int32", "I": "uint32", - "l": "int32", "L": "uint32", "q": "int64", "Q": "uint64", - "e": "float16", "f": "float", "d": "double", "s": "bytes", "p": "bytes", -} - - -def structCodes(struct: str) -> tuple: - """The type code of each value a struct format packs, repeat counts expanded. - - Padding yields no value and so no code, which keeps the codes lined up with the - field names one for one. A count on 's' means one string that long, not that - many strings, which is the one place the rule is not simply repetition. - """ - codes = [] - count = "" - for character in struct[1:]: - if character.isdigit(): - count += character - continue - repeats = int(count) if count else 1 - count = "" - if character == "x": - continue - codes.extend([character] if character in "sp" else [character] * repeats) - return tuple(codes) - - -def typeNameOf(converter) -> str: - """The name to show for a converter when explaining a command. - - A converter is often a type, and then its own name is the answer. When it is a - function, what a reader wants is what it returns, which its annotation already - says -- so a "0" or "1" field is announced as a bool rather than by the name of - the function that makes one. - """ - returned = getattr(converter, "__annotations__", {}).get("return") - if returned is not None: - return getattr(returned, "__name__", str(returned)) - return getattr(converter, "__name__", None) or str(converter) - - -def encodeTemplate(template: str, arguments: dict) -> bytes: - """Returns a str.format template as bytes, with the arguments substituted. - - Raises MissingArgument, naming the field, rather than letting a KeyError out - of str.format. - """ - try: - return template.format(**arguments).encode("utf-8") - except KeyError as error: - raise MissingArgument("{0} needs {1}, got {2}".format( - template, error, sorted(arguments))) from None - - -def matchAndConvert(regex: str, converters: dict, data, mismatch) -> dict: - """Returns the capture groups of regex, converted and named. - - Accepts bytes or str, since what a port hands over varies. Raises mismatch -- - which half is being read decides which -- when the expression does not match, - quoting both sides, and ProtocolError when it and the field names disagree in - number. - """ - text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data - match = re.search(regex, text) - if match is None: - raise mismatch("expected {0!r}, got {1!r}".format(regex, text)) - - groups = match.groups() - if len(groups) != len(converters): - raise ProtocolError( - "{0!r} captured {1} group(s) but {2} field(s) were named".format( - regex, len(groups), len(converters))) - return {name: converter(value) - for (name, converter), value in zip(converters.items(), groups)} - - class Frame(ABC): """One half of a command: how to write it, and how to read it back. + This is not "the command and its reply", it is the command and the + reverse operation to read the command (and confirm proper formatting) so + we can create a DebugPort easily. + Both halves are needed in both directions, which is the whole reason a description is worth having. A driver writes the request and reads the reply; a mock standing in for the instrument reads the request and writes the reply. @@ -194,9 +123,13 @@ class Frame(ABC): fixed-size binary frame has no terminator to stop at, so the number of bytes to ask for can come from nowhere but the description. A line needs no such answer, since the port already reads up to its own terminator. + + A frame does not know whether it is a request or a reply, and there is no class + for either. Which one it is depends only on the slot of the Command it sits in, + and that is where the two are told apart -- a frame that failed to match says + only that, and the Command says which half of which command was being read. """ - mismatch = DidNotMatch readLength = None @abstractmethod @@ -221,64 +154,99 @@ def parameters(self) -> tuple: ... -class Request(Frame): - """The half a driver writes and a mock reads.""" - - mismatch = RequestDidNotMatch - - -class Reply(Frame): - """The half a driver reads and a mock writes.""" - - mismatch = ReplyDidNotMatch - - -class TextRequest(Request): - """An ASCII request built from a str.format template. +class TextFrame(Frame): + """A line of ASCII, stated as the template that writes it and the expression + that reads it. The template uses named fields, so a caller writes setPower(power=0.5) and - never counts positional arguments: "p {power:0.3f}\r". - - Whatever ends the line is written into the template, where it can be seen, - rather than passed alongside it: on the way out a terminator is simply more - literal text, and instruments disagree about it enough -- \r, \n, \r\n, or - nothing at all for the Integra -- that no default would serve. - - regex is how a mock recognises the request and reads its arguments back, and it - is written out rather than worked out from the template. A template could be - turned into an expression by guessing a pattern per format spec, and the guess - would be right often enough to be trusted and wrong quietly: a field with no - spec would come back as text, and nothing would say so. This module already - refuses to let struct guess a byte order; guessing an expression is not of a - different kind. + never counts positional arguments: "p {power:0.3f}\r". Whatever ends the line is + written into it, where it can be seen, rather than passed alongside: a + terminator is simply more literal text, and instruments disagree about it enough + -- \r, \n, \r\n, or nothing at all for the Integra -- that no default would + serve. + + fields maps a name to the converter for its capture group, in group order: + {"power": float} turns r"(\\d+\\.\\d+)" into {"power": 0.123}. A line with no + capture groups, such as an "OK" acknowledgement, decodes to an empty dict -- it + either matched or it raised. + + Neither notation is worked out from the other. A template could be turned into + an expression by guessing a pattern per format spec, and the guess would be + right often enough to be trusted and wrong quietly: a field with no spec would + come back as text and nothing would say so. An expression cannot be turned back + into a line at all. This module already refuses to let struct guess a byte + order; guessing here is not of a different kind. + + One class serves both halves of a command. The template is what a driver sends + when the frame is a request and what a mock answers when it is a reply; the + expression is read by whichever of the two is listening. """ def __init__(self, template: str, regex: str, fields: dict = None): - """Describe a request as the template that writes it and the expression - that reads it back, with a converter per capture group.""" + """Describe a line as the template that writes it, the expression that + reads it, and a converter per capture group.""" self.template = template self.regex = regex self.fields = dict(fields or {}) @property def arguments(self) -> tuple: - """The argument names this request needs, in the order they appear.""" + """The names this line carries, in the order the template lays them out.""" return tuple(name for _, name, _, _ in string.Formatter().parse(self.template) if name) @property def parameters(self) -> tuple: - """Each argument with its type, taken from the converter named for it.""" - return tuple((name, typeNameOf(self.fields[name]) if name in self.fields else "text") + """Each name with its type, taken from the converter named for it.""" + return tuple((name, self.typeNameOf(self.fields[name]) + if name in self.fields else "text") for name in self.arguments) - def encode(self, **arguments) -> bytes: - """Returns the request as bytes, with the arguments substituted.""" - return encodeTemplate(self.template, arguments) + @staticmethod + def typeNameOf(converter) -> str: + """The name to show for a converter when explaining a command. + + A converter is often a type, and then its own name is the answer. When it is + a function, what a reader wants is what it returns, which its annotation + already says -- so a "0" or "1" field is announced as a bool rather than by + the name of the function that makes one. + """ + returned = getattr(converter, "__annotations__", {}).get("return") + if returned is not None: + return getattr(returned, "__name__", str(returned)) + return getattr(converter, "__name__", None) or str(converter) + + def encode(self, **values) -> bytes: + """Returns the line as bytes, with the values substituted. + + Raises MissingArgument, naming the field, rather than letting a KeyError out + of str.format. + """ + try: + return self.template.format(**values).encode("utf-8") + except KeyError as error: + raise MissingArgument("{0} needs {1}, got {2}".format( + self.template, error, sorted(values))) from None def decode(self, data) -> dict: - """Returns the arguments the request carried -- what a mock reads.""" - return matchAndConvert(self.regex, self.fields, data, self.mismatch) + """Returns the captured values, converted and named. + + Accepts bytes or str, since what a port hands over varies. Raises + DidNotMatch when the expression does not match, quoting both sides, and + ProtocolError when it and the field names disagree in number. + """ + text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data + match = re.search(self.regex, text) + if match is None: + raise DidNotMatch("expected {0!r}, got {1!r}".format(self.regex, text)) + + groups = match.groups() + if len(groups) != len(self.fields): + raise ProtocolError( + "{0!r} captured {1} group(s) but {2} field(s) were named".format( + self.regex, len(groups), len(self.fields))) + return {name: converter(value) + for (name, converter), value in zip(self.fields.items(), groups)} class BinaryFrame(Frame): @@ -297,6 +265,17 @@ class BinaryFrame(Frame): it a constant instead. """ + # What each struct code is called when a command explains itself. A class + # attribute rather than a module one, so a frame for an instrument that reads + # its own kind of value can add to it, the way CommandDictionary does with + # converters. + typeNames = { + "c": "byte", "?": "bool", "b": "int8", "B": "uint8", + "h": "int16", "H": "uint16", "i": "int32", "I": "uint32", + "l": "int32", "L": "uint32", "q": "int64", "Q": "uint64", + "e": "float16", "f": "float", "d": "double", "s": "bytes", "p": "bytes", + } + def __init__(self, struct: str, fields: tuple = (), constants: dict = None): """Describe a frame as a struct format, one name per packed value. @@ -326,10 +305,31 @@ def arguments(self) -> tuple: @property def parameters(self) -> tuple: """Each non-constant field with the type its format packs it as.""" - codeOf = dict(zip(self.fields, structCodes(self.struct))) - return tuple((name, structTypeNames.get(codeOf.get(name), "unknown")) + codeOf = dict(zip(self.fields, self.structCodes(self.struct))) + return tuple((name, self.typeNames.get(codeOf.get(name), "unknown")) for name in self.arguments) + @staticmethod + def structCodes(struct: str) -> tuple: + """The type code of each value a struct format packs, repeats expanded. + + Padding yields no value and so no code, which keeps the codes lined up with + the field names one for one. A count on 's' means one string that long, not + that many strings, which is the one place the rule is not repetition. + """ + codes = [] + count = "" + for character in struct[1:]: + if character.isdigit(): + count += character + continue + repeats = int(count) if count else 1 + count = "" + if character == "x": + continue + codes.extend([character] if character in "sp" else [character] * repeats) + return tuple(codes) + def encode(self, **arguments) -> bytes: """Returns the packed frame, constants and arguments in field order. @@ -355,12 +355,12 @@ def decode(self, data) -> dict: """Returns the values the frame carried, named, constants left out. A constant is checked rather than returned: it carries no information, and - a frame whose header is wrong is not this frame at all. Raises mismatch + a frame whose header is wrong is not this frame at all. Raises DidNotMatch when the length or a constant is wrong, and ProtocolError when the format and the names disagree. """ if len(data) != self.readLength: - raise self.mismatch("expected {0} bytes for {1}, got {2}".format( + raise DidNotMatch("expected {0} bytes for {1}, got {2}".format( self.readLength, self.struct, len(data))) values = unpack(self.struct, bytes(data)) if len(values) != len(self.fields): @@ -371,65 +371,12 @@ def decode(self, data) -> dict: named = dict(zip(self.fields, values)) for name, constant in self.constants.items(): if named[name] != constant: - raise self.mismatch("{0} expects {1}={2!r}, got {3!r}".format( + raise DidNotMatch("{0} expects {1}={2!r}, got {3!r}".format( self.struct, name, constant, named[name])) return {name: value for name, value in named.items() if name not in self.constants} -class BinaryRequest(BinaryFrame, Request): - """The binary half a driver writes and a mock recognises.""" - - -class TextReply(Reply): - """A line matched by a regular expression. - - fields maps a name to the converter for its capture group, in group order: - {"power": float} turns r"(\\d+\\.\\d+)" into {"power": 0.123}. A reply with no - capture groups, such as an "OK" acknowledgement, decodes to an empty dict -- - it either matched or it raised. - - It parses whatever it is handed and says nothing about how to read it: where - a line ends is the port's business, not the protocol's. - - template is what a mock writes to play the instrument, and it is required for - the same reason the request's regex is: nothing here guesses. A reply that - cannot be written is a reply that cannot be tested, and every device in this - library is meant to ship a debug twin. - - It also carries what the regex deliberately leaves out. "OK" matches b"OK\r\n" - because re.search ignores what follows, but a mock has to send the terminator, - so the template is where the line the instrument really sends is finally - written down -- exactly as a request writes its own terminator into its - template rather than passing it alongside. - """ - - def __init__(self, regex: str, template: str, fields: dict = None): - """Describe a reply as the expression that reads it and the template that - writes it, with a converter per capture group.""" - self.regex = regex - self.template = template - self.fields = dict(fields or {}) - - @property - def parameters(self) -> tuple: - """Each value the reply carries, with the type it is converted to.""" - return tuple((name, typeNameOf(converter)) - for name, converter in self.fields.items()) - - def decode(self, data) -> dict: - """Returns the captured values, converted and named.""" - return matchAndConvert(self.regex, self.fields, data, self.mismatch) - - def encode(self, **values) -> bytes: - """Returns the line the instrument would answer -- what a mock writes.""" - return encodeTemplate(self.template, values) - - -class BinaryReply(BinaryFrame, Reply): - """The binary half a driver reads and a mock answers with.""" - - class Command: """One request and the reply it expects, named for a driver to call by name. @@ -447,9 +394,15 @@ class Command: That is the whole difference from the Command this replaces, which described the protocol, built the bytes, performed the exchange, and then kept the reply on itself. + + A command is also where the two halves are told apart. A Frame is neither a + request nor a reply -- it becomes one by being put in one of these two slots -- + so a frame that fails to match says only that, and a command turns it into a + RequestDidNotMatch or a ReplyDidNotMatch naming itself. Which is more than + either half could say: a frame does not know what command it belongs to. """ - def __init__(self, name: str, request: Request, reply: Reply = None): + def __init__(self, name: str, request: Frame, reply: Frame = None): """Pair a request with the reply it expects, under the name a driver uses. reply is None for a command the instrument does not answer. @@ -472,11 +425,12 @@ def decode(self, data) -> dict: """Returns what the reply carried, as {field: value}. Raises ProtocolError if this command expects no reply, since decoding one - means the caller read something it should not have. + means the caller read something it should not have, and ReplyDidNotMatch + naming this command when the instrument answered something else. """ if self.reply is None: raise ProtocolError("{0} expects no reply".format(self.name)) - return self.reply.decode(data) + return self.decodeHalf(self.reply, data, ReplyDidNotMatch) def decodeRequest(self, data) -> dict: """Returns the arguments a request carried -- the mock's half of encode. @@ -484,7 +438,19 @@ def decodeRequest(self, data) -> dict: Raises RequestDidNotMatch when the bytes are some other command's request, which is how a mock picks the right one out of a dictionary. """ - return self.request.decode(data) + return self.decodeHalf(self.request, data, RequestDidNotMatch) + + def decodeHalf(self, half: Frame, data, mismatch) -> dict: + """Decode one half, saying which half of which command failed to match. + + A frame knows only that the bytes are not the ones it describes. Naming the + command, and which end of it was being read, is something only a command can + do -- so it is done here rather than passed down. + """ + try: + return half.decode(data) + except DidNotMatch as error: + raise mismatch("{0}: {1}".format(self.name, error)) from None def encodeReply(self, **values) -> bytes: """Returns the bytes the instrument would answer -- the mock's half of decode. @@ -600,24 +566,24 @@ def commandFrom(cls, name: str, description: dict) -> Command: cls.replyFrom(reply, where) if reply is not None else None) @classmethod - def requestFrom(cls, description: dict, where: str) -> Request: + def requestFrom(cls, description: dict, where: str) -> Frame: """Build the request half: a template for text, a struct for binary.""" if "template" in description: if "regex" not in description: raise BadDescription( "{0}: a text request needs a regex as well as its template, " "so that a mock can read what the driver writes".format(where)) - return TextRequest(description["template"], description["regex"], + return TextFrame(description["template"], description["regex"], fields=cls.convertersFor(description.get("fields", {}), where)) if "struct" in description: - return BinaryRequest(description["struct"], + return BinaryFrame(description["struct"], fields=tuple(description.get("fields", ())), constants=cls.constantsFrom(description)) raise BadDescription( "{0}: a request needs a template, for text, or a struct, for binary".format(where)) @classmethod - def replyFrom(cls, description: dict, where: str) -> Reply: + def replyFrom(cls, description: dict, where: str) -> Frame: """Build the reply half: a regex for text, a struct for binary. A text reply states both notations, like a text request; a binary one needs @@ -628,10 +594,10 @@ def replyFrom(cls, description: dict, where: str) -> Reply: raise BadDescription( "{0}: a text reply needs a template as well as its regex, " "so that a mock can write what the driver reads".format(where)) - return TextReply(description["regex"], description["template"], + return TextFrame(description["template"], description["regex"], fields=cls.convertersFor(description.get("fields", {}), where)) if "struct" in description: - return BinaryReply(description["struct"], + return BinaryFrame(description["struct"], fields=tuple(description.get("fields", ())), constants=cls.constantsFrom(description)) raise BadDescription( @@ -743,41 +709,41 @@ def __len__(self) -> int: # Tests # --------------------------------------------------------------------------- -class TestTextRequest(unittest.TestCase): +class TestTextFrameWritingALine(unittest.TestCase): def testBuildsAConstantRequest(self): - self.assertEqual(TextRequest("pa?\r", r"pa\?\r").encode(), b"pa?\r") + self.assertEqual(TextFrame("pa?\r", r"pa\?\r").encode(), b"pa?\r") def testSubstitutesNamedArguments(self): - request = TextRequest("p {power:0.3f}\r", r"p ([0-9.]+)\r", + request = TextFrame("p {power:0.3f}\r", r"p ([0-9.]+)\r", fields={"power": float}) self.assertEqual(request.encode(power=0.5), b"p 0.500\r") def testWhateverEndsTheLineIsVisibleInTheTemplate(self): - self.assertEqual(TextRequest("*GWL", r"\*GWL").encode(), b"*GWL") - self.assertEqual(TextRequest("g r0xc9\n", "g r0xc9\n").encode(), b"g r0xc9\n") - self.assertEqual(TextRequest("SYST:ERR?\r\n", r"SYST:ERR\?\r\n").encode(), + self.assertEqual(TextFrame("*GWL", r"\*GWL").encode(), b"*GWL") + self.assertEqual(TextFrame("g r0xc9\n", "g r0xc9\n").encode(), b"g r0xc9\n") + self.assertEqual(TextFrame("SYST:ERR?\r\n", r"SYST:ERR\?\r\n").encode(), b"SYST:ERR?\r\n") def testNamesTheArgumentsItNeeds(self): - request = TextRequest("s r{register} {value}\r", r"s r(\S+) (-?\d+)\r", + request = TextFrame("s r{register} {value}\r", r"s r(\S+) (-?\d+)\r", fields={"register": str, "value": int}) self.assertEqual(request.arguments, ("register", "value")) def testAMissingArgumentSaysWhichOne(self): with self.assertRaises(MissingArgument) as raised: - TextRequest("p {power:0.3f}\r", r"p ([0-9.]+)\r").encode() + TextFrame("p {power:0.3f}\r", r"p ([0-9.]+)\r").encode() self.assertIn("power", str(raised.exception)) -class TestBinaryRequest(unittest.TestCase): +class TestBinaryFrameWritingAFrame(unittest.TestCase): def testPacksConstantsOnly(self): - request = BinaryRequest("", "!", "="): - self.assertEqual(BinaryReply(prefix + "l", fields=("value",)).readLength, 4) + self.assertEqual(BinaryFrame(prefix + "l", fields=("value",)).readLength, 4) def testWithoutTheGuardTheLengthWouldBeWrong(self): # What is actually being prevented: not a crash, a wrong frame. @@ -892,8 +858,8 @@ def testWithoutTheGuardTheLengthWouldBeWrong(self): def getPowerCommand() -> Command: """The Cobolt power query, stated in full: both halves, both directions.""" return Command("GET_POWER", - TextRequest("pa?\r", r"pa\?\r"), - TextReply(r"(\d+\.\d+)", "{power:0.4f}\r\n", fields={"power": float})) + TextFrame("pa?\r", r"pa\?\r"), + TextFrame("{power:0.4f}\r\n", r"(\d+\.\d+)", fields={"power": float})) class TestCommand(unittest.TestCase): @@ -905,7 +871,7 @@ def testCarriesARequestAndItsReply(self): def testACommandMayExpectNothingBack(self): command = Command("SET_WAVELENGTH", - TextRequest("*PWC{wavelength:05d}", r"\*PWC(\d{5})", + TextFrame("*PWC{wavelength:05d}", r"\*PWC(\d{5})", fields={"wavelength": int})) self.assertFalse(command.expectsReply) self.assertEqual(command.encode(wavelength=532), b"*PWC00532") @@ -922,6 +888,28 @@ def testTheDescriptionIsSharedButTheResultIsNot(self): self.assertEqual(second, {"power": 0.9}) self.assertEqual(vars(command).keys(), {"name", "request", "reply"}) + def testAFrameSaysOnlyThatItDidNotMatchAndTheCommandSaysWhichHalf(self): + # A frame has no idea which command it belongs to, or which end of it, so + # naming both is the command's job -- and it is the reason there is no + # request class and no reply class to carry that name instead. + command = getPowerCommand() + with self.assertRaises(ReplyDidNotMatch) as raised: + command.decode(b"syntax error\r\n") + self.assertIn("GET_POWER", str(raised.exception)) + self.assertIn("syntax error", str(raised.exception)) + + with self.assertRaises(RequestDidNotMatch) as raised: + command.decodeRequest(b"l?\r") + self.assertIn("GET_POWER", str(raised.exception)) + + def testOneFrameCanServeEitherHalf(self): + # Nothing marks a frame as a request or a reply: an instrument that echoes + # its own line back is described once and put in both slots. + line = TextFrame("OK\r\n", "OK") + echo = Command("ECHO", line, line) + self.assertEqual(echo.encode(), echo.encodeReply()) + self.assertEqual(echo.decodeRequest(b"OK\r\n"), echo.decode(b"OK\r\n")) + class TestTheMirrorDirection(unittest.TestCase): """Reading a request and writing a reply: the same descriptions, other way round.""" @@ -929,7 +917,7 @@ class TestTheMirrorDirection(unittest.TestCase): def testWhatATemplateWroteItsRegexReadsBack(self): # The two notations are written separately and have to agree. Nothing # enforces that but a round trip, which is why every command here has one. - request = TextRequest("p {power:0.3f}\r", r"p ([0-9.]+)\r", + request = TextFrame("p {power:0.3f}\r", r"p ([0-9.]+)\r", fields={"power": float}) self.assertEqual(request.decode(request.encode(power=0.5)), {"power": 0.5}) @@ -937,46 +925,46 @@ def testTheRegexSaysWhatEachFieldWasRatherThanTheFormatSpec(self): # A converter is named, not guessed from "05d" or "x". The description says # what comes back, so "0x24" stays text and 31 comes back an int because # each was asked for. - wavelength = TextRequest("*PWC{wavelength:05d}", r"\*PWC(\d{5})", + wavelength = TextFrame("*PWC{wavelength:05d}", r"\*PWC(\d{5})", fields={"wavelength": int}) self.assertEqual(wavelength.decode(b"*PWC00532"), {"wavelength": 532}) - register = TextRequest("s r{register} {value}\r", r"s r(\S+) (-?\d+)\r", + register = TextFrame("s r{register} {value}\r", r"s r(\S+) (-?\d+)\r", fields={"register": str, "value": int}) self.assertEqual(register.decode(register.encode(register="0x24", value=31)), {"register": "0x24", "value": 31}) def testARequestThatBelongsToAnotherCommandIsDeclined(self): - with self.assertRaises(RequestDidNotMatch): - TextRequest("pa?\r", r"pa\?\r").decode(b"l?\r") + with self.assertRaises(DidNotMatch): + TextFrame("pa?\r", r"pa\?\r").decode(b"l?\r") def testABinaryRequestGivesBackOnlyWhatWasNotConstant(self): - request = BinaryRequest(" Date: Wed, 12 Aug 2026 19:53:30 -0400 Subject: [PATCH 14/17] Document every argument and return value, and type-hint the signatures Problem: the docstrings explained why the design is what it is and almost never what to pass or what comes back. Several signatures left the answer out too -- decode(self, data), typeNameOf(converter), decodeHalf(self, half, data, mismatch) -- so nothing said whether data was bytes or a str, or that mismatch was an exception class rather than an instance. Where the prose did try, it reached for words that mean something else in Python: "one keyword per {name}" for what is simply a named value. And the JSON example in CommandDictionary showed only a request that takes no arguments, so nothing in the documentation revealed that a text request can carry any, or what fields means on either side of a command. Solution: every method of the prototype now carries Args, Returns and Raises, in the Google style the recent core files already use (devicecontroller.py, debugport.py, commands.py), keeping the prose that was there and adding the sections underneath. An AST pass over the file checks that no method is missing one and that no parameter or return is left unannotated. The annotations use typing.Union rather than the 3.10 pipe, since pyproject sets the floor at 3.9. **values stays object on purpose, and now says why: the type a value must have belongs to the field it goes into, not to the method -- an int for a struct "l", anything formattable for a text template -- so parameters answers it, one name at a time, and a Union would be at once too narrow for text and too wide for binary. Along the way, and prompted by reading it back: requireExplicitByteOrder becomes a classmethod of BinaryFrame with its prefixes as a class attribute, so a subclass can override the pair instead of working around a module function; a frame is called a frame rather than "this half", which now only means one of a command's two sides; and the vaguest docstrings were rewritten to show what they describe ("power",) for "p {power:0.3f}\r" rather than "the names this line carries". Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/testProtocolPrototype.py | 717 +++++++++++++++--- 1 file changed, 598 insertions(+), 119 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 0ed4655b..8a20ece4 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -38,7 +38,7 @@ a position and '", "!", "=") - - -def requireExplicitByteOrder(struct: str): - """Refuse a struct format that does not begin with a byte-order prefix. - - Without one, struct uses native sizes and native alignment: "clllc" is 33 - bytes on this machine rather than the 14 the instrument expects, an "l" is - whatever a C long happens to be, and padding appears between the fields. The - frame is then correct for the compiler and wrong for the wire, and nothing - downstream would notice -- the same failure the ctypes variant needed - _pack_ = 1 to avoid, at the price of one character here. - """ - if not struct.startswith(byteOrderPrefixes): - raise ProtocolError( - "{0!r} has no byte-order prefix, so struct would use native sizes and " - "alignment: {1} bytes instead of {2}. Write {3!r}.".format( - struct, calcsize(struct), calcsize("<" + struct), "<" + struct)) - - class Frame(ABC): """One half of a command: how to write it, and how to read it back. @@ -124,6 +105,20 @@ class Frame(ABC): ask for can come from nowhere but the description. A line needs no such answer, since the port already reads up to its own terminator. + Three properties say what a frame carries, and they are easy to confuse: + + - fields is the raw list, in whichever shape the notation needs -- a + converter per capture group for text, every packed name including the + constants for binary; + - arguments is the names a caller supplies, so the constants are gone; + - parameters is those same names with their types, read off the converter + for text and off the struct code for binary. + + parameters is built from arguments, so arguments is always the names half of + parameters, and it is parameters alone that the contract below requires -- + arguments is a convenience the two concrete frames happen to offer. Neither is + "arguments" in the ordinary Python sense: both are names, never values. + A frame does not know whether it is a request or a reply, and there is no class for either. Which one it is depends only on the slot of the Command it sits in, and that is where the two are told apart -- a frame that failed to match says @@ -133,23 +128,54 @@ class Frame(ABC): readLength = None @abstractmethod - def encode(self, **values) -> bytes: - """Returns the bytes to write, built from the values named here.""" + def encode(self, **values: object) -> bytes: + """Build the bytes this frame puts on the wire. + + Args: + **values: the values to write, passed by name, one per entry of + parameters -- encode(power=0.5) for a frame that carries a power. + Anything the description fixes is supplied by the description + and must not be passed. The annotation is object because the + type each value must have is not a property of this method but + of the field it goes into, which only the description knows: + parameters reports it, one name at a time. + + Returns: + The bytes to write, terminator included, ready to hand to a port. + """ ... @abstractmethod - def decode(self, data) -> dict: - """Returns what the bytes carried, as {field: value}.""" + def decode(self, data: Union[bytes, str]) -> dict: + """Read bytes this frame describes and name what they carried. + + Args: + data: the bytes read, or a str -- what a port hands over varies. + + Returns: + A dict of {field name: value}, empty when the frame carries nothing + but its own literal text or its own fixed bytes. + + Raises: + DidNotMatch: when the bytes are not the ones this frame describes. + """ ... @property @abstractmethod def parameters(self) -> tuple: - """Every value this half carries, as (name, type name) in order. - - Not used to encode or decode anything: it is what lets a dictionary explain - itself, so that the names a caller passes and the names it gets back are - read off the description rather than out of a comment. + """The names to pass this frame, and the names it hands back, with types. + + The two are the same list, since a frame is written and read from one + description: encode takes these names as keywords and decode returns them + as keys. Not used to encode or decode anything, though -- it exists so + that a dictionary can explain itself out of the description rather than + out of a comment. + + Returns: + A (name, type name) pair per value, in the order the frame lays them + out, such as (("x", "int32"), ("y", "int32"), ("z", "int32")). Empty + when the frame is nothing but fixed text or fixed bytes. """ ... @@ -182,45 +208,107 @@ class TextFrame(Frame): expression is read by whichever of the two is listening. """ - def __init__(self, template: str, regex: str, fields: dict = None): - """Describe a line as the template that writes it, the expression that - reads it, and a converter per capture group.""" + def __init__(self, template: str, regex: str, + fields: Optional[dict] = None): + """Describe a line by its two notations. + + The three arguments describe one line three ways, and they have to agree: + "p {power:0.3f}\r" is written with r"p ([0-9.]+)\r" and {"power": float}. + One placeholder, one capture group, one converter, all called power. + + Args: + template: the str.format template that writes the line, terminator + included, with a {name} where each value goes + regex: the expression that reads the line back, with one capture + group per value, in the same order the template writes them + fields: the converter to run on each capture group, keyed by the + name the template uses -- float for a power, int for a count. + None or empty for a line with no values at all, such as an "OK". + """ self.template = template self.regex = regex self.fields = dict(fields or {}) @property def arguments(self) -> tuple: - """The names this line carries, in the order the template lays them out.""" + """The names a caller must supply. + + They are the {name} placeholders of the template, and they are found by + handing the template to string.Formatter().parse, which splits it into + literal text and field names so that nothing has to be scanned by hand. + + Returns: + Every placeholder name, in the order the template lays them out: + ("power",) for "p {power:0.3f}\r", and ("register", "value") for + "s r{register} {value}\r". Empty for a template of literal text + only, such as "pa?\r", which is a request that takes no arguments. + """ return tuple(name for _, name, _, _ in string.Formatter().parse(self.template) if name) @property def parameters(self) -> tuple: - """Each name with its type, taken from the converter named for it.""" + """Those same names, each with the type it is read back as. + + The names come from the template and the types from the converters, which + are two independent lists: a name the converters do not mention is + reported as text, since nothing says otherwise. + + Returns: + A (name, type name) pair per placeholder, in template order: + (("power", "float"),) for "p {power:0.3f}\r" described with + {"power": float}. + """ return tuple((name, self.typeNameOf(self.fields[name]) if name in self.fields else "text") for name in self.arguments) @staticmethod - def typeNameOf(converter) -> str: - """The name to show for a converter when explaining a command. + def typeNameOf(converter: Callable[[str], object]) -> str: + """Name a converter for the benefit of someone reading a usage line. A converter is often a type, and then its own name is the answer. When it is a function, what a reader wants is what it returns, which its annotation already says -- so a "0" or "1" field is announced as a bool rather than by the name of the function that makes one. + + This is a guess, and it is allowed to be one because nothing it returns + ever reaches the wire: parameters is its only caller and usage() is the + only thing that reads parameters. The rule that nothing here is inferred + governs the protocol, not the sentence that explains it. Its worst answer + is an ugly one -- "" -- never a wrong frame. + + Args: + converter: anything callable on a captured string -- a type such as + float, or a function such as booleanFromZeroOrOne + + Returns: + The name of what the converter returns when it is annotated, its own + name otherwise, and its repr as a last resort for a lambda. """ returned = getattr(converter, "__annotations__", {}).get("return") if returned is not None: return getattr(returned, "__name__", str(returned)) return getattr(converter, "__name__", None) or str(converter) - def encode(self, **values) -> bytes: - """Returns the line as bytes, with the values substituted. + def encode(self, **values: object) -> bytes: + """Write the line, substituting the values into the template. + + Args: + **values: the values to substitute, passed by name, one per + {name} in the template. A value the template never mentions is + ignored, as str.format ignores it. Any object will do, since + a format spec is applied to whatever it is handed -- a float for + "{power:0.3f}", but equally a datetime for "{when:%H:%M}". - Raises MissingArgument, naming the field, rather than letting a KeyError out - of str.format. + Returns: + The line as UTF-8 bytes, terminator included since the template + carries it. + + Raises: + MissingArgument: when a {name} of the template was not supplied, + naming the field and listing what was given, rather than letting + a bare KeyError out of str.format. """ try: return self.template.format(**values).encode("utf-8") @@ -228,12 +316,22 @@ def encode(self, **values) -> bytes: raise MissingArgument("{0} needs {1}, got {2}".format( self.template, error, sorted(values))) from None - def decode(self, data) -> dict: - """Returns the captured values, converted and named. + def decode(self, data: Union[bytes, str]) -> dict: + """Read the line back and convert each captured group. + + Args: + data: the line as read, bytes or str -- what a port hands over + varies. Anything around the match is ignored, since re.search is + used: where a line ends is the port's business. - Accepts bytes or str, since what a port hands over varies. Raises - DidNotMatch when the expression does not match, quoting both sides, and - ProtocolError when it and the field names disagree in number. + Returns: + A dict of {field name: converted value}, empty when the expression + has no capture groups. + + Raises: + DidNotMatch: when the expression does not match, quoting both sides. + ProtocolError: when the expression and the fields disagree on how + many values the line carries. """ text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data match = re.search(self.regex, text) @@ -265,6 +363,8 @@ class BinaryFrame(Frame): it a constant instead. """ + byteOrderPrefixes = ("<", ">", "!", "=") + # What each struct code is called when a command explains itself. A class # attribute rather than a module one, so a frame for an instrument that reads # its own kind of value can add to it, the way CommandDictionary does with @@ -276,13 +376,26 @@ class BinaryFrame(Frame): "e": "float16", "f": "float", "d": "double", "s": "bytes", "p": "bytes", } - def __init__(self, struct: str, fields: tuple = (), constants: dict = None): + def __init__(self, struct: str, fields: tuple = (), + constants: Optional[dict] = None): """Describe a frame as a struct format, one name per packed value. - The format must state its byte order, or the frame it builds is the one a - C compiler would want rather than the one the instrument expects. + Args: + struct: a struct format, byte order included, or the frame it builds + is the one a C compiler would want rather than the one the + instrument expects + fields: one name per value the format packs, in order, padding + excluded since padding packs no value + constants: the values nobody supplies, keyed by field name -- a + header byte, a terminator, a fixed acknowledgement. Written on + the way out, required on the way in. + + Raises: + ProtocolError: when the format states no byte order. + BadDescription: when constants fixes a name fields does not list, + which would otherwise fail much later and much less clearly. """ - requireExplicitByteOrder(struct) + self.requireExplicitByteOrder(struct) self.struct = struct self.fields = tuple(fields) self.constants = dict(constants or {}) @@ -294,28 +407,84 @@ def __init__(self, struct: str, fields: tuple = (), constants: dict = None): @property def readLength(self) -> int: - """Returns how many bytes the frame occupies, taken from the format itself.""" + """How many bytes to read before this frame can be decoded. + + Returns: + The size of the format, so it never has to be kept in step by hand. + This is the one thing a caller could not work out for itself: a + fixed-size frame has no terminator to stop at. + """ return calcsize(self.struct) @property def arguments(self) -> tuple: - """The names a caller must supply: every field that is not a constant.""" + """The names a caller must supply. + + Returns: + A tuple of every field that is not a constant, in format order. A + frame made only of constants returns an empty tuple. + """ return tuple(name for name in self.fields if name not in self.constants) @property def parameters(self) -> tuple: - """Each non-constant field with the type its format packs it as.""" + """Each non-constant field with the type its format packs it as. + + Returns: + A tuple of (name, type name) pairs, in format order, the type being + read off the struct code -- "l" is reported as int32. A code this + class does not name is reported as "unknown" rather than hidden. + """ codeOf = dict(zip(self.fields, self.structCodes(self.struct))) return tuple((name, self.typeNames.get(codeOf.get(name), "unknown")) for name in self.arguments) + @classmethod + def requireExplicitByteOrder(cls, struct: str) -> None: + """Refuse a struct format that does not begin with a byte-order prefix. + + Without one, struct uses native sizes and native alignment: "clllc" is 33 + bytes on this machine rather than the 14 the instrument expects, an "l" is + whatever a C long happens to be, and padding appears between the fields. + The frame is then correct for the compiler and wrong for the wire, and + nothing downstream would notice -- the same failure the ctypes variant + needed _pack_ = 1 to avoid, at the price of one character here. + + A classmethod, and the prefixes a class attribute, so that a frame for a + machine whose formats are spelled differently overrides the pair rather + than working around a module function it cannot reach. + + Args: + struct: a struct format, expected to start with one of the prefixes + byteOrderPrefixes lists + + Returns: + Nothing. It is a guard, called for its refusal. + + Raises: + ProtocolError: when the prefix is missing, giving both the length the + format would really produce and the one that was meant, and the + corrected format to write instead. + """ + if not struct.startswith(cls.byteOrderPrefixes): + raise ProtocolError( + "{0!r} has no byte-order prefix, so struct would use native sizes " + "and alignment: {1} bytes instead of {2}. Write {3!r}.".format( + struct, calcsize(struct), calcsize("<" + struct), "<" + struct)) + @staticmethod def structCodes(struct: str) -> tuple: - """The type code of each value a struct format packs, repeats expanded. + """Split a struct format into the type code of each value it packs. - Padding yields no value and so no code, which keeps the codes lined up with - the field names one for one. A count on 's' means one string that long, not - that many strings, which is the one place the rule is not repetition. + Args: + struct: a struct format, byte-order prefix included -- the prefix is + skipped, not treated as a code + + Returns: + One code per packed value, repeat counts expanded, so that the codes + line up with the field names one for one. Padding yields no value and + so no code. A count on "s" means one string that long, not that many + strings, which is the one place the rule is not repetition. """ codes = [] count = "" @@ -330,11 +499,25 @@ def structCodes(struct: str) -> tuple: codes.extend([character] if character in "sp" else [character] * repeats) return tuple(codes) - def encode(self, **arguments) -> bytes: - """Returns the packed frame, constants and arguments in field order. - - Raises MissingArgument for a field nobody supplied, and ProtocolError for a - value struct cannot pack into its format. + def encode(self, **arguments: object) -> bytes: + """Pack the frame, constants and arguments interleaved in field order. + + Args: + **arguments: the values to pack, passed by name, one per entry of + arguments. A constant must not be passed: the description + supplies it. What each value must be is decided by its struct + code and by nothing here -- an int for "l", bytes for "c" -- so + it is parameters that answers, and a wrong kind is refused by + pack rather than by the signature. + + Returns: + The packed frame, exactly readLength bytes long. + + Raises: + MissingArgument: for a field nobody supplied, naming it and listing + what was given. + ProtocolError: for a value struct cannot pack into its format, such + as a string where a long was expected. """ values = [] for name in self.fields: @@ -351,13 +534,26 @@ def encode(self, **arguments) -> bytes: raise ProtocolError("cannot pack {0} into {1}: {2}".format( values, self.struct, error)) from None - def decode(self, data) -> dict: - """Returns the values the frame carried, named, constants left out. + def decode(self, data: bytes) -> dict: + """Unpack the frame and name what it carried, constants left out. A constant is checked rather than returned: it carries no information, and - a frame whose header is wrong is not this frame at all. Raises DidNotMatch - when the length or a constant is wrong, and ProtocolError when the format - and the names disagree. + a frame whose header is wrong is not this frame at all. + + Args: + data: exactly readLength bytes. Unlike a line, a frame cannot be + surrounded by anything, since its length is its only boundary. + + Returns: + A dict of {field name: value} for the non-constant fields only, so a + frame made of constants alone decodes to an empty dict. + + Raises: + DidNotMatch: when the length is wrong, or when a constant is not the + value the description fixed -- which is how a mock tells one + command from another. + ProtocolError: when the format and the field names disagree on how + many values the frame carries. """ if len(data) != self.readLength: raise DidNotMatch("expected {0} bytes for {1}, got {2}".format( @@ -399,13 +595,19 @@ class Command: request nor a reply -- it becomes one by being put in one of these two slots -- so a frame that fails to match says only that, and a command turns it into a RequestDidNotMatch or a ReplyDidNotMatch naming itself. Which is more than - either half could say: a frame does not know what command it belongs to. + either one could say on its own: a frame does not know what command it + belongs to. """ - def __init__(self, name: str, request: Frame, reply: Frame = None): + def __init__(self, name: str, request: Frame, reply: Optional[Frame] = None): """Pair a request with the reply it expects, under the name a driver uses. - reply is None for a command the instrument does not answer. + Args: + name: how a driver asks for this command, and how it names itself in + an error + request: the frame the driver writes and a mock reads + reply: the frame the driver reads and a mock writes, or None for a + command the instrument does not answer """ self.name = name self.request = request @@ -413,50 +615,106 @@ def __init__(self, name: str, request: Frame, reply: Frame = None): @property def expectsReply(self) -> bool: - """True when the instrument answers this command, so a caller knows - whether to read at all.""" + """Whether the instrument answers this command at all. + + Returns: + True when a reply was described, so a caller knows whether to read. + """ return self.reply is not None - def encode(self, **arguments) -> bytes: - """Returns the bytes to write for this command.""" + def encode(self, **arguments: object) -> bytes: + """Build the bytes to send -- the driver's half of the exchange. + + Args: + **arguments: the values the request carries, passed by name -- + one per entry of request.parameters. + + Returns: + The bytes to write. What happens to them is the caller's business: + nothing here touches a port. + """ return self.request.encode(**arguments) - def decode(self, data) -> dict: - """Returns what the reply carried, as {field: value}. + def decode(self, data: Union[bytes, str]) -> dict: + """Read what the instrument answered -- the driver's other half. + + Args: + data: the bytes read back. How many to read is reply.readLength when + the reply is binary, and the port's own terminator when it is a + line. - Raises ProtocolError if this command expects no reply, since decoding one - means the caller read something it should not have, and ReplyDidNotMatch - naming this command when the instrument answered something else. + Returns: + A dict of {field name: value}, returned to the caller and stored + nowhere, so two callers of one description cannot overwrite each + other. + + Raises: + ProtocolError: when this command expects no reply, since decoding one + means the caller read something it should not have. + ReplyDidNotMatch: when the instrument answered something else, naming + this command. """ if self.reply is None: raise ProtocolError("{0} expects no reply".format(self.name)) return self.decodeHalf(self.reply, data, ReplyDidNotMatch) - def decodeRequest(self, data) -> dict: - """Returns the arguments a request carried -- the mock's half of encode. + def decodeRequest(self, data: Union[bytes, str]) -> dict: + """Read a request addressed to us -- the mock's half of encode. + + Args: + data: one complete request as received. - Raises RequestDidNotMatch when the bytes are some other command's request, - which is how a mock picks the right one out of a dictionary. + Returns: + A dict of the arguments it carried, empty for a command that takes + none. + + Raises: + RequestDidNotMatch: when the bytes are some other command's request, + which is how a mock picks the right one out of a dictionary + rather than an error a driver would ever see. """ return self.decodeHalf(self.request, data, RequestDidNotMatch) - def decodeHalf(self, half: Frame, data, mismatch) -> dict: + def decodeHalf(self, half: Frame, data: Union[bytes, str], + mismatch: Type[DidNotMatch]) -> dict: """Decode one half, saying which half of which command failed to match. A frame knows only that the bytes are not the ones it describes. Naming the command, and which end of it was being read, is something only a command can do -- so it is done here rather than passed down. + + Args: + half: the frame to decode with, this command's request or its reply + data: the bytes to read + mismatch: the exception class to raise on failure, which is what + records the role -- RequestDidNotMatch or ReplyDidNotMatch + + Returns: + Whatever the frame decoded, untouched. + + Raises: + The mismatch given, with this command's name prefixed to the frame's + own account of what did not match. """ try: return half.decode(data) except DidNotMatch as error: raise mismatch("{0}: {1}".format(self.name, error)) from None - def encodeReply(self, **values) -> bytes: - """Returns the bytes the instrument would answer -- the mock's half of decode. + def encodeReply(self, **values: object) -> bytes: + """Build the answer the instrument would give -- the mock's half of decode. + + Args: + **values: the values the reply carries, passed by name -- one per + entry of reply.parameters. + + Returns: + The bytes a mock writes back, terminator included. - Raises ProtocolError if this command expects no reply, since a mock that - answers one would be answering a command the instrument leaves silent. + Raises: + ProtocolError: when this command expects no reply, since a mock that + answered would be answering a command the instrument leaves + silent. """ if self.reply is None: raise ProtocolError("{0} expects no reply".format(self.name)) @@ -464,12 +722,32 @@ def encodeReply(self, **values) -> bytes: def booleanFromZeroOrOne(text: str) -> bool: - """A reply of "0" or "1" as a boolean.""" + """Read a "0" or "1" field as a boolean. + + Args: + text: one capture group, expected to be "0" or "1" + + Returns: + True for "1", False for anything else -- an instrument that answers + neither is refused by the expression, not here. + """ return text == "1" def integerFromHexadecimal(text: str) -> int: - """A reply written in hexadecimal as an integer.""" + """Read a field written in hexadecimal as an integer. + + Args: + text: one capture group of hexadecimal digits, with or without a 0x + prefix, which int accepts either way + + Returns: + The value the digits spell. + + Raises: + ValueError: when the group is not hexadecimal, which means the + expression captured something it should not have. + """ return int(text, 16) @@ -490,6 +768,12 @@ class CommandDictionary: out, a regular expression on the way in, a struct format for bytes in either direction. + "SET_POWER": { + "request": {"template": "p {power:0.3f}\r", + "regex": "p ([0-9.]+)\r", + "fields": {"power": "float"}}, + "reply": {"regex": "OK", "template": "OK\r\n"} + }, "GET_POWER": { "request": {"template": "pa?\r", "regex": "pa\\?\r"}, "reply": {"regex": "(\\d+\\.\\d+)", "template": "{power:0.4f}\r\n", @@ -503,12 +787,32 @@ class CommandDictionary: "constants": {"acknowledgement": "\r"}} } - A text half states both of its notations and a binary half states one, because + A text frame states both of its notations and a binary one states a single + format, because a struct already reads both ways and a template and a regex do not. Both are required: a request nobody can read and a reply nobody can write are half descriptions, and the mock they are meant to serve would only find that out at the moment it failed. + Either frame may carry values and either may carry none, in any + combination. SET_POWER above takes an argument and is answered by a bare + acknowledgement; GET_POWER takes none and is answered by a value; MOVE takes + three and is answered by a fixed byte. What a frame carries is written in + fields, and what fields means follows the notation: + + - on a text frame, one entry per capture group of the regex, in group + order, + naming the converter to run on it. Those names are also the {names} of + the template, since a value written and the same value read back are not + two different things; + - on a binary frame, one name per value the struct packs, in order, and + constants picks out the ones nobody supplies -- a header byte, a + terminator, a fixed acknowledgement. + + Nothing checks that a template and its regex agree about what they carry; only + sending a command and reading it back does, which is why every command in the + tests below is put through that round trip. + Reads like a dict. The methods that turn a description into objects are here rather than beside it, so a device whose protocol needs something this does not cover subclasses and overrides one of them -- adding a converter, or a @@ -527,28 +831,64 @@ class CommandDictionary: "hexInteger": integerFromHexadecimal, } - def __init__(self, commands: dict, deviceName: str = None): + def __init__(self, commands: dict, deviceName: Optional[str] = None): """Hold already-built commands by name. Use fromFile to read a description. - deviceName is carried only to name the instrument in error messages. + Args: + commands: {name: Command}, copied so that the dictionary cannot be + changed behind its back + deviceName: the instrument's name, carried only so that an error can + say whose protocol was being read """ self.commands = dict(commands) self.deviceName = deviceName @classmethod def fromFile(cls, path: str) -> "CommandDictionary": - """Read one device's commands from a JSON file.""" + """Read one device's commands from a JSON file. + + Args: + path: the file to read + + Returns: + A dictionary of the commands it describes. + + Raises: + OSError: when the file cannot be read. + json.JSONDecodeError: when it is not JSON. + BadDescription: when it is JSON but not a protocol. + """ with open(path, "r") as file: return cls.fromDescription(json.load(file)) @classmethod def fromJSON(cls, text: str) -> "CommandDictionary": - """Read them from JSON already in hand.""" + """Read them from JSON already in hand. + + Args: + text: the description as JSON, from wherever it came + + Returns: + A dictionary of the commands it describes. + """ return cls.fromDescription(json.loads(text)) @classmethod def fromDescription(cls, description: dict) -> "CommandDictionary": - """Build from the description itself, however it was obtained.""" + """Build from the description itself, however it was obtained. + + Args: + description: {"device": name, "commands": {name: {...}}}. The device + name is optional; the commands are not. + + Returns: + A dictionary whose command order is the description's order, since + that is usually the order whoever wrote the file thought in. + + Raises: + BadDescription: when there are no commands at all, or when any one of + them does not make sense. + """ if "commands" not in description: raise BadDescription("no commands: expected {'device': ..., 'commands': {...}}") return cls({name: cls.commandFrom(name, one) @@ -557,7 +897,21 @@ def fromDescription(cls, description: dict) -> "CommandDictionary": @classmethod def commandFrom(cls, name: str, description: dict) -> Command: - """Build one named command from its description.""" + """Build one named command from its description. + + Args: + name: the command's name, used both for the Command and to say which + command an error is about + description: {"request": {...}} and, when the instrument answers, + {"reply": {...}} + + Returns: + The command, its reply None when none was described. + + Raises: + BadDescription: when there is no request, or when either half does + not make sense. + """ where = "command {0!r}".format(name) if "request" not in description: raise BadDescription("{0}: no request".format(where)) @@ -567,18 +921,33 @@ def commandFrom(cls, name: str, description: dict) -> Command: @classmethod def requestFrom(cls, description: dict, where: str) -> Frame: - """Build the request half: a template for text, a struct for binary.""" + """Build the request half: a template for text, a struct for binary. + + Args: + description: the "request" object. Which key is present says which + notation it is written in. + where: how to name this command in an error + + Returns: + A TextFrame or a BinaryFrame, according to the key found. + + Raises: + BadDescription: when neither notation is present, or when a text + request states a template without the expression that reads it + back. + ProtocolError: when a struct format states no byte order. + """ if "template" in description: if "regex" not in description: raise BadDescription( "{0}: a text request needs a regex as well as its template, " "so that a mock can read what the driver writes".format(where)) return TextFrame(description["template"], description["regex"], - fields=cls.convertersFor(description.get("fields", {}), where)) + fields=cls.convertersFor(description.get("fields", {}), where)) if "struct" in description: return BinaryFrame(description["struct"], - fields=tuple(description.get("fields", ())), - constants=cls.constantsFrom(description)) + fields=tuple(description.get("fields", ())), + constants=cls.constantsFrom(description)) raise BadDescription( "{0}: a request needs a template, for text, or a struct, for binary".format(where)) @@ -588,6 +957,18 @@ def replyFrom(cls, description: dict, where: str) -> Frame: A text reply states both notations, like a text request; a binary one needs only its struct, since pack and unpack are already each other's inverse. + + Args: + description: the "reply" object + where: how to name this command in an error + + Returns: + A TextFrame or a BinaryFrame, according to the key found. + + Raises: + BadDescription: when neither notation is present, or when a text + reply states an expression without the template that writes it. + ProtocolError: when a struct format states no byte order. """ if "regex" in description: if "template" not in description: @@ -605,13 +986,33 @@ def replyFrom(cls, description: dict, where: str) -> Frame: @classmethod def constantsFrom(cls, description: dict) -> dict: - """Turn the constants of a binary frame from text in a file into bytes.""" + """Turn the constants of a binary frame from text in a file into bytes. + + Args: + description: a "request" or "reply" object, whose "constants" is read + if present + + Returns: + {field name: bytes}, empty when the frame fixes nothing. + """ return {name: cls.bytesFrom(value) for name, value in description.get("constants", {}).items()} @classmethod def convertersFor(cls, fields: dict, where: str) -> dict: - """Turn {"power": "float"} from a file into {"power": float}.""" + """Turn {"power": "float"} from a file into {"power": float}. + + Args: + fields: {field name: converter name}, as a file writes it + where: how to name this command in an error + + Returns: + {field name: callable}, in the same order, ready for a TextFrame. + + Raises: + BadDescription: when a converter name is not one this class knows, + listing the ones that do exist. + """ resolved = {} for name, converterName in fields.items(): if converterName not in cls.converters: @@ -623,30 +1024,62 @@ def convertersFor(cls, fields: dict, where: str) -> dict: @staticmethod def bytesFrom(text: str) -> bytes: - """A constant byte from a file, latin-1 so that \xfe stays one byte.""" + """Turn a constant written in a file into the byte it stands for. + + Args: + text: one character of a JSON string, such as "M" or "\r" + + Returns: + The bytes, encoded latin-1 so that \xfe stays one byte rather than + becoming the two UTF-8 would spell it with. + """ return text.encode("latin-1") @property def names(self) -> tuple: - """Returns every command name, in the order the description listed them.""" + """Every command this device understands. + + Returns: + The names, in the order the description listed them. + """ return tuple(self.commands) def __getitem__(self, name: str) -> Command: - """Returns one command by name, or raises KeyError naming what does exist.""" + """Look one command up by name. + + Args: + name: the command's name, as the description spells it + + Returns: + The command. + + Raises: + KeyError: naming the device and listing the commands it does have, + since a typo is the likeliest reason to be here. + """ if name not in self.commands: raise KeyError("{0} has no command {1!r}; it has {2}".format( self.deviceName or "this device", name, ", ".join(sorted(self.commands)))) return self.commands[name] - def recognize(self, data) -> tuple: - """Returns the command a request belongs to, and the arguments it carried. + def recognize(self, data: Union[bytes, str]) -> Tuple[Command, dict]: + """Work out which command a request belongs to, and what it carried. Every command is asked in turn whether the bytes are its request, and the first that says yes wins -- so a mock dispatches on the same descriptions the driver sends with, and no prefix table is written twice. - It is handed one complete request. Deciding where a request ends in a - stream is the port's business, exactly as it is on the reply side. + Args: + data: one complete request. Deciding where a request ends in a stream + is the port's business, exactly as it is on the reply side. + + Returns: + A (command, arguments) pair: the command that recognised the bytes, + and the dict its request decoded to. + + Raises: + RequestDidNotMatch: when no command recognises the bytes, naming the + device and quoting them. """ for command in self.commands.values(): try: @@ -665,6 +1098,9 @@ def usage(self) -> str: Commands appear in the order the description listed them, since that order is usually the one whoever wrote the file thought in. + + Returns: + The whole text, one paragraph per command, ready to print. """ lines = ["{0}: {1} commands".format(self.deviceName or "This device", len(self))] for name in self.names: @@ -672,7 +1108,17 @@ def usage(self) -> str: return "\n".join(lines) def usageFor(self, command: Command) -> list: - """The lines explaining one command: how to call it, what comes back.""" + """Explain one command: how to call it, and what comes back. + + Args: + command: the command to describe + + Returns: + Its lines, starting with a blank one so that paragraphs separate when + they are joined. A command is shown as a call with its arguments, + then one line for the answer: the values it carries, or that it + carries none, or that the instrument does not answer at all. + """ lines = ["", " {0}({1})".format(command.name, self.listed(command.request))] if not command.expectsReply: lines.append(" the instrument does not answer") @@ -684,24 +1130,52 @@ def usageFor(self, command: Command) -> list: @staticmethod def listed(frame: Frame) -> str: - """One half's values as "name: type, name: type", for a usage line.""" + """Set out one frame's values for a usage line. + + Args: + frame: a command's request or its reply + + Returns: + "name: type, name: type" in the order the frame carries them, and + an empty string for a frame that carries none -- which is what makes an + argumentless command print as NAME(). + """ return ", ".join("{0}: {1}".format(name, typeName) for name, typeName in frame.parameters) def __str__(self) -> str: - """The usage text, so that printing a dictionary explains it.""" + """Explain the whole protocol, so that printing a dictionary is enough. + + Returns: + What usage() returns. + """ return self.usage() - def __contains__(self, name) -> bool: - """True when the device understands a command of that name.""" + def __contains__(self, name: str) -> bool: + """Whether the device understands a command of that name. + + Args: + name: a command name to look for + + Returns: + True when it is one of this device's commands. + """ return name in self.commands - def __iter__(self): - """Iterate over the command names, as a dict does.""" + def __iter__(self) -> Iterator[str]: + """Iterate over the command names, as a dict does. + + Returns: + An iterator over the names, in description order. + """ return iter(self.commands) def __len__(self) -> int: - """Returns how many commands the device understands.""" + """How many commands the device understands. + + Returns: + The count, which is also what usage() announces. + """ return len(self.commands) @@ -856,7 +1330,12 @@ def testWithoutTheGuardTheLengthWouldBeWrong(self): def getPowerCommand() -> Command: - """The Cobolt power query, stated in full: both halves, both directions.""" + """Build the Cobolt power query, stated in full: both halves, both directions. + + Returns: + A fresh command each call, so that a test cannot be affected by what + another did to it. + """ return Command("GET_POWER", TextFrame("pa?\r", r"pa\?\r"), TextFrame("{power:0.4f}\r\n", r"(\d+\.\d+)", fields={"power": float})) From 70d651fa00e222d39dbd9f8f5cdca5b707a275a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Wed, 12 Aug 2026 19:53:57 -0400 Subject: [PATCH 15/17] Skip whitespace in a struct format, as struct itself does Problem: struct allows whitespace between codes and ignores it, so " --- .../tests/testProtocolPrototype.py | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index 8a20ece4..c83a9ca5 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -480,15 +480,22 @@ def structCodes(struct: str) -> tuple: struct: a struct format, byte-order prefix included -- the prefix is skipped, not treated as a code + Unlike typeNameOf, which also serves nothing but the usage text, this is + not a guess: the struct codes are a closed, documented set, so it can be + read exactly and is expected to be. + Returns: One code per packed value, repeat counts expanded, so that the codes line up with the field names one for one. Padding yields no value and - so no code. A count on "s" means one string that long, not that many - strings, which is the one place the rule is not repetition. + so no code, and whitespace is skipped as struct itself skips it. A + count on "s" means one string that long, not that many strings, which + is the one place the rule is not repetition. """ codes = [] count = "" for character in struct[1:]: + if character.isspace(): + continue if character.isdigit(): count += character continue @@ -1833,6 +1840,16 @@ def testPaddingAndRepeatsDoNotThrowTheTypesOutOfLine(self): self.assertEqual(codesOf("<3l"), ("l", "l", "l")) self.assertEqual(codesOf("<10s"), ("s",)) + def testWhitespaceIsSkippedAsStructItselfSkipsIt(self): + # struct allows spaces between codes, and a space taken for a code would + # shift every name after it against its type. + self.assertEqual(calcsize(" Date: Wed, 12 Aug 2026 20:43:22 -0400 Subject: [PATCH 16/17] Describe a protocol in the library, and let SutterDevice speak through it Problem: the protocol description lived inside its own test file, so nothing could use it. SutterDevice still spoke through the commands dict of communication/commands.py, whose DataCommand describes the protocol, builds the bytes, performs the I/O, and then keeps the reply on itself -- on a class attribute shared by every instance of the driver. It needed a second struct format to write a position (' --- CHANGELOG.md | 39 + hardwarelibrary/communication/debugport.py | 82 +- hardwarelibrary/communication/protocol.py | 1423 +++++++++++++++++ hardwarelibrary/motion/sutterdevice.py | 202 ++- hardwarelibrary/physicaldevice.py | 60 + .../tests/testProtocolDebugPort.py | 167 ++ .../tests/testProtocolPrototype.py | 1325 ++------------- hardwarelibrary/tests/testSutterDevice.py | 11 + hardwarelibrary/tests/testSutterSerialPort.py | 13 +- 9 files changed, 2048 insertions(+), 1274 deletions(-) create mode 100644 hardwarelibrary/communication/protocol.py create mode 100644 hardwarelibrary/tests/testProtocolDebugPort.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e24caa5..6f677e3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,45 @@ API changes can land even when the minor version is unchanged. ## [Unreleased] +### Added +- **A protocol is described, not performed** (`communication/protocol.py`). A + `Frame` turns named values into the bytes to write and the bytes read back into + named values; it owns no port and stores nothing of what happened. `TextFrame` + states a `str.format` template and the regular expression that reads it back; + `BinaryFrame` states one struct format, whose constants are written on the way + out and *required* on the way in. A `Command` pairs two frames, a + `CommandDictionary` holds a device's commands and reads them from JSON. Nothing + is inferred anywhere: a byte order must be explicit, and so must both notations + of a text line. + - Both directions come out of the same objects. A driver writes the request and + reads the reply; a debug port reads the request and writes the reply. That is + what the old `DataCommand` needed a second encoder and decoder for, and those + could -- and did -- drift apart. + - `CommandDictionary.usage()` prints every command, its arguments and what it + answers, with types read off the description rather than a comment. + - `CommandDictionary.validate()` refuses a description that is wrong about + itself: a template and an expression that describe different lines, a struct + that packs a different number of values than it names, a command whose request + another command answers to first. It writes each command out with specimen + values made from the declared types and reads it straight back. +- **`ProtocolDebugPort`** (`communication/debugport.py`), a debug port with no code + of its own: hand it a `CommandDictionary` and it stands in for the instrument. + Whatever a request carries is remembered by name and whatever a reply carries is + answered from that memory. A command may add a `"sets"` clause for a state change + its bytes cannot express -- `HOME` carries nothing and still moves a stage. +- **`PhysicalDevice.protocol`** (default `None`) and **`PhysicalDevice.performTransaction()`**, + which performs one command of that protocol under the port's `transactionLock`, + reading by `readLength` for a binary reply and up to the terminator for a line. + A driver that sets `protocol` needs no send-and-receive code of its own. + +### Changed +- **`SutterDevice` speaks through a description.** Its protocol is data, it sends + through `performTransaction`, and it no longer checks acknowledgements by hand: + the description says `MOVE`, `HOME` and `WORK` answer `b"\r"`. Its nested + `DebugSerialPort` is gone; `serialNumber="debug"` now gets a `ProtocolDebugPort`. + Callers building the debug port themselves should use + `ProtocolDebugPort(SutterDevice.protocol)`. + ## [2.1.0] - 2026-08-04 ### Added diff --git a/hardwarelibrary/communication/debugport.py b/hardwarelibrary/communication/debugport.py index f2b54088..9225605d 100644 --- a/hardwarelibrary/communication/debugport.py +++ b/hardwarelibrary/communication/debugport.py @@ -160,4 +160,84 @@ def process_command(self, name, params, endPointIndex): - bytes/str: written to the output buffer as-is - None: no response is sent """ - raise NotImplementedError("Subclasses must implement process_command") \ No newline at end of file + raise NotImplementedError("Subclasses must implement process_command") + +class ProtocolDebugPort(DebugPort): + """An instrument stood in for entirely by its own protocol description. + + Where TableDrivenDebugPort needs a subclass with a process_command full of + branches, this needs nothing at all: hand it a CommandDictionary and it + answers every command that dictionary describes. Two rules do the whole job, + and both fall out of the fact that a command's two halves name their values + the same way. + + - whatever a request carries is remembered, by name. MOVE arrives with x, y + and z, so x, y and z are what the instrument now holds. + - whatever a reply carries is answered from that memory. GET_POSITION asks + for x, y and z, and gets back what MOVE left there. + + A value never set reads as 0, which is the state of an instrument that has + just been switched on. A reply that cannot pack a 0 -- one carrying text, say + -- will say so rather than invent something. + + The one thing no description of bytes can express is a command that changes + the instrument without carrying anything: HOME takes no arguments and yet + moves the stage to its origin. A command may therefore state a "sets" clause, + which is applied on arrival exactly as a request's own values are. That + clause is the only part of a description that talks about the instrument + rather than the wire, and a driver never reads it. + + Recognising a request is the dictionary's work, not this class's: recognize() + asks each command in turn, and the header constants settle it. So there is no + second table of prefixes to keep in step with the driver. + """ + + def __init__(self, protocol, delay=0, numberOfEndPoints=1): + """Stand in for the instrument a description describes. + + Args: + protocol: the CommandDictionary to answer from + delay: seconds of jitter to add to a read, as DebugPort applies it + numberOfEndPoints: how many endpoints to pretend to have + """ + super().__init__(delay=delay, numberOfEndPoints=numberOfEndPoints) + self.protocol = protocol + self.values = {} + + def processInputBuffers(self, endPointIndex): + """Answer whatever complete request has arrived. + + Args: + endPointIndex: which endpoint was written to + + Raises: + RequestDidNotMatch: when the bytes are no command of this protocol, + which means whatever wrote them is at fault -- a debug port that + quietly dropped them would hide the bug it exists to find. + """ + received = bytes(self.inputBuffers[endPointIndex]) + if len(received) == 0: + return + + command, arguments = self.protocol.recognize(received) + self.inputBuffers[endPointIndex] = bytearray() + + self.values.update(arguments) + self.values.update(command.sets) + if command.expectsReply: + self.writeToOutputBuffer(command.encodeReply(**self.answerFor(command)), + endPointIndex) + + def answerFor(self, command) -> dict: + """The values to answer one command with, taken from what is remembered. + + Args: + command: the command that was recognized + + Returns: + {field name: value} for every value its reply carries, 0 for one that + was never set. Empty for a reply that is nothing but a fixed + acknowledgement, which the description supplies on its own. + """ + return {name: self.values.get(name, 0) + for name, _ in command.reply.parameters} diff --git a/hardwarelibrary/communication/protocol.py b/hardwarelibrary/communication/protocol.py new file mode 100644 index 00000000..1c525ce8 --- /dev/null +++ b/hardwarelibrary/communication/protocol.py @@ -0,0 +1,1423 @@ +"""Describing an instrument protocol, without performing it. + +The idea is sans-I/O, the principle behind h11 and wsproto: a protocol is a pure +transformation over bytes and never performs the exchange. A Frame turns named +values into the bytes to write and the bytes read back into named values; it owns +no port and stores nothing of what happened. A driver keeps the port and calls the +primitives of CommunicationPort itself, so the same description serves a driver +speaking to hardware and a debug port standing in for it. + +Consequences worth noticing while reading: + + - a description is immutable and shareable; the result of a command is a plain + dict returned to the caller, so two instruments cannot overwrite each other; + - a frame parses whatever it is handed, and says how many bytes to read only + where nothing else could know: a fixed-size binary frame; + - a constant is stated once and serves both directions -- written on the way + out, required on the way in -- which is what lets a debug port tell one + command from another, and a driver notice an acknowledgement that is not its + own; + - there is no request class and no reply class: a frame becomes one or the + other by the slot of the Command it sits in, which is also the only place + that knows enough to name what failed to match; + - decoding failure raises, naming what did not match, instead of being recorded + in an attribute nobody checks. + +Both directions are described. A driver writes the request and reads the reply; a +debug port reads the request and writes the reply, out of the same objects. + +Where the two directions are the same statement, they are written once; where they +are not, they are both written out. A binary frame is packed and unpacked from one +struct format, because pack and unpack are exactly each other's inverse -- asking +for a second format there is how the old description ended up with ' bytes: + """Build the bytes this frame puts on the wire. + + Args: + **values: the values to write, passed by name, one per entry of + parameters -- encode(power=0.5) for a frame that carries a power. + Anything the description fixes is supplied by the description + and must not be passed. The annotation is object because the + type each value must have is not a property of this method but + of the field it goes into, which only the description knows: + parameters reports it, one name at a time. + + Returns: + The bytes to write, terminator included, ready to hand to a port. + """ + ... + + @abstractmethod + def decode(self, data: Union[bytes, str]) -> dict: + """Read bytes this frame describes and name what they carried. + + Args: + data: the bytes read, or a str -- what a port hands over varies. + + Returns: + A dict of {field name: value}, empty when the frame carries nothing + but its own literal text or its own fixed bytes. + + Raises: + DidNotMatch: when the bytes are not the ones this frame describes. + """ + ... + + def faults(self) -> list: + """Everything wrong with this frame that can be seen without using it. + + Only what the notation alone can settle: whether the two statements of a + text line agree on how many values there are and what they are called, + whether a struct format packs as many values as it names. Whether they + agree about the line *itself* takes a round trip, which needs values and + so belongs to the dictionary, not here. + + Returns: + One sentence per fault, empty when there is nothing to say. A frame + of a notation with nothing to check inherits this and says nothing. + """ + return [] + + @property + @abstractmethod + def parameters(self) -> tuple: + """The names to pass this frame, and the names it hands back, with types. + + The two are the same list, since a frame is written and read from one + description: encode takes these names as keywords and decode returns them + as keys. Not used to encode or decode anything, though -- it exists so + that a dictionary can explain itself out of the description rather than + out of a comment. + + Returns: + A (name, type name) pair per value, in the order the frame lays them + out, such as (("x", "int32"), ("y", "int32"), ("z", "int32")). Empty + when the frame is nothing but fixed text or fixed bytes. + """ + ... + + +class TextFrame(Frame): + """A line of ASCII, stated as the template that writes it and the expression + that reads it. + + The template uses named fields, so a caller writes setPower(power=0.5) and + never counts positional arguments: "p {power:0.3f}\r". Whatever ends the line is + written into it, where it can be seen, rather than passed alongside: a + terminator is simply more literal text, and instruments disagree about it enough + -- \r, \n, \r\n, or nothing at all for the Integra -- that no default would + serve. + + fields maps a name to the converter for its capture group, in group order: + {"power": float} turns r"(\\d+\\.\\d+)" into {"power": 0.123}. A line with no + capture groups, such as an "OK" acknowledgement, decodes to an empty dict -- it + either matched or it raised. + + Neither notation is worked out from the other. A template could be turned into + an expression by guessing a pattern per format spec, and the guess would be + right often enough to be trusted and wrong quietly: a field with no spec would + come back as text and nothing would say so. An expression cannot be turned back + into a line at all. This module already refuses to let struct guess a byte + order; guessing here is not of a different kind. + + One class serves both halves of a command. The template is what a driver sends + when the frame is a request and what a mock answers when it is a reply; the + expression is read by whichever of the two is listening. + """ + + def __init__(self, template: str, regex: str, + fields: Optional[dict] = None): + """Describe a line by its two notations. + + The three arguments describe one line three ways, and they have to agree: + "p {power:0.3f}\r" is written with r"p ([0-9.]+)\r" and {"power": float}. + One placeholder, one capture group, one converter, all called power. + + Args: + template: the str.format template that writes the line, terminator + included, with a {name} where each value goes + regex: the expression that reads the line back, with one capture + group per value, in the same order the template writes them + fields: the converter to run on each capture group, keyed by the + name the template uses -- float for a power, int for a count. + None or empty for a line with no values at all, such as an "OK". + """ + self.template = template + self.regex = regex + self.fields = dict(fields or {}) + + @property + def arguments(self) -> tuple: + """The names a caller must supply. + + They are the {name} placeholders of the template, and they are found by + handing the template to string.Formatter().parse, which splits it into + literal text and field names so that nothing has to be scanned by hand. + + Returns: + Every placeholder name, in the order the template lays them out: + ("power",) for "p {power:0.3f}\r", and ("register", "value") for + "s r{register} {value}\r". Empty for a template of literal text + only, such as "pa?\r", which is a request that takes no arguments. + """ + return tuple(name for _, name, _, _ in string.Formatter().parse(self.template) + if name) + + @property + def parameters(self) -> tuple: + """Those same names, each with the type it is read back as. + + The names come from the template and the types from the converters, which + are two independent lists: a name the converters do not mention is + reported as text, since nothing says otherwise. + + Returns: + A (name, type name) pair per placeholder, in template order: + (("power", "float"),) for "p {power:0.3f}\r" described with + {"power": float}. + """ + return tuple((name, self.typeNameOf(self.fields[name]) + if name in self.fields else "text") + for name in self.arguments) + + def faults(self) -> list: + """What the template and the expression can be seen to disagree about. + + They are written out separately, so nothing but a check like this holds + them together. Two disagreements show without sending anything: how many + values there are, and what they are called. + + Returns: + One sentence per fault, empty when the two notations line up. + """ + try: + expression = re.compile(self.regex) + except re.error as error: + return ["{0!r} is not a regular expression: {1}".format(self.regex, error)] + + if expression.groups != len(self.fields): + return ["{0!r} captures {1} value(s) but {2} field(s) are named: {3}".format( + self.regex, expression.groups, len(self.fields), + ", ".join(self.fields) or "none")] + + written, read = set(self.arguments), set(self.fields) + if written != read: + return ["the template writes {0} and the fields name {1}".format( + ", ".join(sorted(written)) or "nothing", + ", ".join(sorted(read)) or "nothing")] + return [] + + @staticmethod + def typeNameOf(converter: Callable[[str], object]) -> str: + """Name a converter for the benefit of someone reading a usage line. + + A converter is often a type, and then its own name is the answer. When it is + a function, what a reader wants is what it returns, which its annotation + already says -- so a "0" or "1" field is announced as a bool rather than by + the name of the function that makes one. + + This is a guess, and it is allowed to be one because nothing it returns + ever reaches the wire: parameters is its only caller and usage() is the + only thing that reads parameters. The rule that nothing here is inferred + governs the protocol, not the sentence that explains it. Its worst answer + is an ugly one -- "" -- never a wrong frame. + + Args: + converter: anything callable on a captured string -- a type such as + float, or a function such as booleanFromZeroOrOne + + Returns: + The name of what the converter returns when it is annotated, its own + name otherwise, and its repr as a last resort for a lambda. + """ + returned = getattr(converter, "__annotations__", {}).get("return") + if returned is not None: + return getattr(returned, "__name__", str(returned)) + return getattr(converter, "__name__", None) or str(converter) + + def encode(self, **values: object) -> bytes: + """Write the line, substituting the values into the template. + + Args: + **values: the values to substitute, passed by name, one per + {name} in the template. A value the template never mentions is + ignored, as str.format ignores it. Any object will do, since + a format spec is applied to whatever it is handed -- a float for + "{power:0.3f}", but equally a datetime for "{when:%H:%M}". + + Returns: + The line as UTF-8 bytes, terminator included since the template + carries it. + + Raises: + MissingArgument: when a {name} of the template was not supplied, + naming the field and listing what was given, rather than letting + a bare KeyError out of str.format. + """ + try: + return self.template.format(**values).encode("utf-8") + except KeyError as error: + raise MissingArgument("{0} needs {1}, got {2}".format( + self.template, error, sorted(values))) from None + + def decode(self, data: Union[bytes, str]) -> dict: + """Read the line back and convert each captured group. + + Args: + data: the line as read, bytes or str -- what a port hands over + varies. Anything around the match is ignored, since re.search is + used: where a line ends is the port's business. + + Returns: + A dict of {field name: converted value}, empty when the expression + has no capture groups. + + Raises: + DidNotMatch: when the expression does not match, quoting both sides. + ProtocolError: when the expression and the fields disagree on how + many values the line carries. + """ + text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data + match = re.search(self.regex, text) + if match is None: + raise DidNotMatch("expected {0!r}, got {1!r}".format(self.regex, text)) + + groups = match.groups() + if len(groups) != len(self.fields): + raise ProtocolError( + "{0!r} captured {1} group(s) but {2} field(s) were named".format( + self.regex, len(groups), len(self.fields))) + return {name: converter(value) + for (name, converter), value in zip(self.fields.items(), groups)} + + +class BinaryFrame(Frame): + """A fixed-size binary frame, named field by field, readable and writable. + + fields names each value in the struct format, in order; constants are the ones + nobody supplies, such as a header byte or a trailing carriage return. A + constant is written on the way out and required on the way in, so the one line + that produces b"M" also refuses a frame that does not begin with it -- which is + how a mock tells one command from another, and how a driver is told that an + acknowledgement was not the one it expected. + + Padding ('x') has no place here. It can be unpacked but not packed, so a frame + that skipped its terminator could be read and never written, and the mock + direction would need a second format for the same bytes. Name the byte and make + it a constant instead. + """ + + byteOrderPrefixes = ("<", ">", "!", "=") + + # What each struct code is called when a command explains itself. A class + # attribute rather than a module one, so a frame for an instrument that reads + # its own kind of value can add to it, the way CommandDictionary does with + # converters. + typeNames = { + "c": "byte", "?": "bool", "b": "int8", "B": "uint8", + "h": "int16", "H": "uint16", "i": "int32", "I": "uint32", + "l": "int32", "L": "uint32", "q": "int64", "Q": "uint64", + "e": "float16", "f": "float", "d": "double", "s": "bytes", "p": "bytes", + } + + def __init__(self, struct: str, fields: tuple = (), + constants: Optional[dict] = None): + """Describe a frame as a struct format, one name per packed value. + + Args: + struct: a struct format, byte order included, or the frame it builds + is the one a C compiler would want rather than the one the + instrument expects + fields: one name per value the format packs, in order, padding + excluded since padding packs no value + constants: the values nobody supplies, keyed by field name -- a + header byte, a terminator, a fixed acknowledgement. Written on + the way out, required on the way in. + + Raises: + ProtocolError: when the format states no byte order. + BadDescription: when constants fixes a name fields does not list, + which would otherwise fail much later and much less clearly. + """ + self.requireExplicitByteOrder(struct) + self.struct = struct + self.fields = tuple(fields) + self.constants = dict(constants or {}) + + unknown = sorted(set(self.constants) - set(self.fields)) + if unknown: + raise BadDescription("{0} fixes {1}, which {2} does not name".format( + self.struct, ", ".join(unknown), list(self.fields))) + + @property + def readLength(self) -> int: + """How many bytes to read before this frame can be decoded. + + Returns: + The size of the format, so it never has to be kept in step by hand. + This is the one thing a caller could not work out for itself: a + fixed-size frame has no terminator to stop at. + """ + return calcsize(self.struct) + + @property + def arguments(self) -> tuple: + """The names a caller must supply. + + Returns: + A tuple of every field that is not a constant, in format order. A + frame made only of constants returns an empty tuple. + """ + return tuple(name for name in self.fields if name not in self.constants) + + @property + def parameters(self) -> tuple: + """Each non-constant field with the type its format packs it as. + + Returns: + A tuple of (name, type name) pairs, in format order, the type being + read off the struct code -- "l" is reported as int32. A code this + class does not name is reported as "unknown" rather than hidden. + """ + codeOf = dict(zip(self.fields, self.structCodes(self.struct))) + return tuple((name, self.typeNames.get(codeOf.get(name), "unknown")) + for name in self.arguments) + + def faults(self) -> list: + """Whether the format and the field names agree on how many values there are. + + Caught here rather than at the first decode, where it surfaces today as a + ProtocolError in the middle of an exchange. + + Returns: + One sentence per fault, empty when the counts line up. + """ + codes = self.structCodes(self.struct) + if len(codes) != len(self.fields): + return ["{0} packs {1} value(s) but {2} field(s) are named: {3}".format( + self.struct, len(codes), len(self.fields), + ", ".join(self.fields) or "none")] + return [] + + @classmethod + def requireExplicitByteOrder(cls, struct: str) -> None: + """Refuse a struct format that does not begin with a byte-order prefix. + + Without one, struct uses native sizes and native alignment: "clllc" is 33 + bytes on this machine rather than the 14 the instrument expects, an "l" is + whatever a C long happens to be, and padding appears between the fields. + The frame is then correct for the compiler and wrong for the wire, and + nothing downstream would notice -- the same failure the ctypes variant + needed _pack_ = 1 to avoid, at the price of one character here. + + A classmethod, and the prefixes a class attribute, so that a frame for a + machine whose formats are spelled differently overrides the pair rather + than working around a module function it cannot reach. + + Args: + struct: a struct format, expected to start with one of the prefixes + byteOrderPrefixes lists + + Returns: + Nothing. It is a guard, called for its refusal. + + Raises: + ProtocolError: when the prefix is missing, giving both the length the + format would really produce and the one that was meant, and the + corrected format to write instead. + """ + if not struct.startswith(cls.byteOrderPrefixes): + raise ProtocolError( + "{0!r} has no byte-order prefix, so struct would use native sizes " + "and alignment: {1} bytes instead of {2}. Write {3!r}.".format( + struct, calcsize(struct), calcsize("<" + struct), "<" + struct)) + + @staticmethod + def structCodes(struct: str) -> tuple: + """Split a struct format into the type code of each value it packs. + + Args: + struct: a struct format, byte-order prefix included -- the prefix is + skipped, not treated as a code + + Unlike typeNameOf, which also serves nothing but the usage text, this is + not a guess: the struct codes are a closed, documented set, so it can be + read exactly and is expected to be. + + Returns: + One code per packed value, repeat counts expanded, so that the codes + line up with the field names one for one. Padding yields no value and + so no code, and whitespace is skipped as struct itself skips it. A + count on "s" means one string that long, not that many strings, which + is the one place the rule is not repetition. + """ + codes = [] + count = "" + for character in struct[1:]: + if character.isspace(): + continue + if character.isdigit(): + count += character + continue + repeats = int(count) if count else 1 + count = "" + if character == "x": + continue + codes.extend([character] if character in "sp" else [character] * repeats) + return tuple(codes) + + def encode(self, **arguments: object) -> bytes: + """Pack the frame, constants and arguments interleaved in field order. + + Args: + **arguments: the values to pack, passed by name, one per entry of + arguments. A constant must not be passed: the description + supplies it. What each value must be is decided by its struct + code and by nothing here -- an int for "l", bytes for "c" -- so + it is parameters that answers, and a wrong kind is refused by + pack rather than by the signature. + + Returns: + The packed frame, exactly readLength bytes long. + + Raises: + MissingArgument: for a field nobody supplied, naming it and listing + what was given. + ProtocolError: for a value struct cannot pack into its format, such + as a string where a long was expected. + """ + values = [] + for name in self.fields: + if name in self.constants: + values.append(self.constants[name]) + elif name in arguments: + values.append(arguments[name]) + else: + raise MissingArgument("{0} needs {1}, got {2}".format( + self.struct, name, sorted(arguments))) + try: + return pack(self.struct, *values) + except StructError as error: + raise ProtocolError("cannot pack {0} into {1}: {2}".format( + values, self.struct, error)) from None + + def decode(self, data: bytes) -> dict: + """Unpack the frame and name what it carried, constants left out. + + A constant is checked rather than returned: it carries no information, and + a frame whose header is wrong is not this frame at all. + + Args: + data: exactly readLength bytes. Unlike a line, a frame cannot be + surrounded by anything, since its length is its only boundary. + + Returns: + A dict of {field name: value} for the non-constant fields only, so a + frame made of constants alone decodes to an empty dict. + + Raises: + DidNotMatch: when the length is wrong, or when a constant is not the + value the description fixed -- which is how a mock tells one + command from another. + ProtocolError: when the format and the field names disagree on how + many values the frame carries. + """ + if len(data) != self.readLength: + raise DidNotMatch("expected {0} bytes for {1}, got {2}".format( + self.readLength, self.struct, len(data))) + values = unpack(self.struct, bytes(data)) + if len(values) != len(self.fields): + raise ProtocolError( + "{0} unpacks {1} value(s) but {2} field(s) were named".format( + self.struct, len(values), len(self.fields))) + + named = dict(zip(self.fields, values)) + for name, constant in self.constants.items(): + if named[name] != constant: + raise DidNotMatch("{0} expects {1}={2!r}, got {3!r}".format( + self.struct, name, constant, named[name])) + return {name: value for name, value in named.items() + if name not in self.constants} + + +class Command: + """One request and the reply it expects, named for a driver to call by name. + + A command is a description and nothing else: it says what to send and what + should come back, and carries neither the data nor the transmission. encode() + hands the caller the bytes to write, decode() turns what came back into + values, and the port in between is the caller's. + + The same description read from the other end plays the instrument: + decodeRequest() is what a mock hears, encodeReply() what it answers. Two pairs + of methods, one protocol, no second table to keep in step -- where the Command + this replaces needed a request decoder and a reply encoder written out beside + the request encoder and the reply decoder. + + That is the whole difference from the Command this replaces, which described + the protocol, built the bytes, performed the exchange, and then kept the + reply on itself. + + A command is also where the two halves are told apart. A Frame is neither a + request nor a reply -- it becomes one by being put in one of these two slots -- + so a frame that fails to match says only that, and a command turns it into a + RequestDidNotMatch or a ReplyDidNotMatch naming itself. Which is more than + either one could say on its own: a frame does not know what command it + belongs to. + """ + + def __init__(self, name: str, request: Frame, reply: Optional[Frame] = None, + sets: Optional[dict] = None, specimen: Optional[dict] = None): + """Pair a request with the reply it expects, under the name a driver uses. + + Args: + name: how a driver asks for this command, and how it names itself in + an error + request: the frame the driver writes and a mock reads + reply: the frame the driver reads and a mock writes, or None for a + command the instrument does not answer + sets: what receiving this command does to the instrument's state, + as {field name: value}, for what the request does not carry. + HOME takes no arguments and yet moves the stage to the origin, + and nothing about the bytes on the wire could say so. This is the + one thing here that describes the instrument rather than the + protocol, and it exists so that a debug port needs no code of its + own; a driver never reads it. + specimen: a value per field for validate() to send through this + command and expect back, when the ones it makes from the declared + types will not do -- an expression that refuses a zero, say. + Like sets, it is not part of the protocol, and only validate() + reads it. + """ + self.name = name + self.request = request + self.reply = reply + self.sets = dict(sets or {}) + self.specimen = dict(specimen or {}) + + @property + def expectsReply(self) -> bool: + """Whether the instrument answers this command at all. + + Returns: + True when a reply was described, so a caller knows whether to read. + """ + return self.reply is not None + + def encode(self, **arguments: object) -> bytes: + """Build the bytes to send -- the driver's half of the exchange. + + Args: + **arguments: the values the request carries, passed by name -- + one per entry of request.parameters. + + Returns: + The bytes to write. What happens to them is the caller's business: + nothing here touches a port. + """ + return self.request.encode(**arguments) + + def decode(self, data: Union[bytes, str]) -> dict: + """Read what the instrument answered -- the driver's other half. + + Args: + data: the bytes read back. How many to read is reply.readLength when + the reply is binary, and the port's own terminator when it is a + line. + + Returns: + A dict of {field name: value}, returned to the caller and stored + nowhere, so two callers of one description cannot overwrite each + other. + + Raises: + ProtocolError: when this command expects no reply, since decoding one + means the caller read something it should not have. + ReplyDidNotMatch: when the instrument answered something else, naming + this command. + """ + if self.reply is None: + raise ProtocolError("{0} expects no reply".format(self.name)) + return self.decodeHalf(self.reply, data, ReplyDidNotMatch) + + def decodeRequest(self, data: Union[bytes, str]) -> dict: + """Read a request addressed to us -- the mock's half of encode. + + Args: + data: one complete request as received. + + Returns: + A dict of the arguments it carried, empty for a command that takes + none. + + Raises: + RequestDidNotMatch: when the bytes are some other command's request, + which is how a mock picks the right one out of a dictionary + rather than an error a driver would ever see. + """ + return self.decodeHalf(self.request, data, RequestDidNotMatch) + + def decodeHalf(self, half: Frame, data: Union[bytes, str], + mismatch: Type[DidNotMatch]) -> dict: + """Decode one half, saying which half of which command failed to match. + + A frame knows only that the bytes are not the ones it describes. Naming the + command, and which end of it was being read, is something only a command can + do -- so it is done here rather than passed down. + + Args: + half: the frame to decode with, this command's request or its reply + data: the bytes to read + mismatch: the exception class to raise on failure, which is what + records the role -- RequestDidNotMatch or ReplyDidNotMatch + + Returns: + Whatever the frame decoded, untouched. + + Raises: + The mismatch given, with this command's name prefixed to the frame's + own account of what did not match. + """ + try: + return half.decode(data) + except DidNotMatch as error: + raise mismatch("{0}: {1}".format(self.name, error)) from None + + def encodeReply(self, **values: object) -> bytes: + """Build the answer the instrument would give -- the mock's half of decode. + + Args: + **values: the values the reply carries, passed by name -- one per + entry of reply.parameters. + + Returns: + The bytes a mock writes back, terminator included. + + Raises: + ProtocolError: when this command expects no reply, since a mock that + answered would be answering a command the instrument leaves + silent. + """ + if self.reply is None: + raise ProtocolError("{0} expects no reply".format(self.name)) + return self.reply.encode(**values) + + +def booleanFromZeroOrOne(text: str) -> bool: + """Read a "0" or "1" field as a boolean. + + Args: + text: one capture group, expected to be "0" or "1" + + Returns: + True for "1", False for anything else -- an instrument that answers + neither is refused by the expression, not here. + """ + return text == "1" + + +def integerFromHexadecimal(text: str) -> int: + """Read a field written in hexadecimal as an integer. + + Args: + text: one capture group of hexadecimal digits, with or without a 0x + prefix, which int accepts either way + + Returns: + The value the digits spell. + + Raises: + ValueError: when the group is not hexadecimal, which means the + expression captured something it should not have. + """ + return int(text, 16) + + +class CommandDictionary: + """Every command one device understands, by name, read from a file. + + A driver's protocol is a table, and a table is data: a description read from + JSON builds the same objects a driver would write by hand, so a protocol can + be read, reviewed and corrected without touching the code that speaks it. + + JSON rather than TOML or YAML because it is in the standard library of every + Python the package supports; tomllib arrives only in 3.11, and YAML would be a + dependency for a file nobody edits at runtime. + + A command is described by a request and, when there is one, a reply. Which key + is present says which kind it is, so nothing declares a type twice, and the key + names the notation its string is written in: a str.format template on the way + out, a regular expression on the way in, a struct format for bytes in either + direction. + + "SET_POWER": { + "request": {"template": "p {power:0.3f}\r", + "regex": "p ([0-9.]+)\r", + "fields": {"power": "float"}}, + "reply": {"regex": "OK", "template": "OK\r\n"} + }, + "GET_POWER": { + "request": {"template": "pa?\r", "regex": "pa\\?\r"}, + "reply": {"regex": "(\\d+\\.\\d+)", "template": "{power:0.4f}\r\n", + "fields": {"power": "float"}} + }, + "MOVE": { + "request": {"struct": " "CommandDictionary": + """Read one device's commands from a JSON file. + + Args: + path: the file to read + + Returns: + A dictionary of the commands it describes. + + Raises: + OSError: when the file cannot be read. + json.JSONDecodeError: when it is not JSON. + BadDescription: when it is JSON but not a protocol. + """ + with open(path, "r") as file: + return cls.fromDescription(json.load(file)) + + @classmethod + def fromJSON(cls, text: str) -> "CommandDictionary": + """Read them from JSON already in hand. + + Args: + text: the description as JSON, from wherever it came + + Returns: + A dictionary of the commands it describes. + """ + return cls.fromDescription(json.loads(text)) + + @classmethod + def fromDescription(cls, description: dict) -> "CommandDictionary": + """Build from the description itself, however it was obtained. + + Args: + description: {"device": name, "commands": {name: {...}}}. The device + name is optional; the commands are not. + + Returns: + A dictionary whose command order is the description's order, since + that is usually the order whoever wrote the file thought in. + + Raises: + BadDescription: when there are no commands at all, or when any one of + them does not make sense. + """ + if "commands" not in description: + raise BadDescription("no commands: expected {'device': ..., 'commands': {...}}") + return cls({name: cls.commandFrom(name, one) + for name, one in description["commands"].items()}, + deviceName=description.get("device")) + + @classmethod + def commandFrom(cls, name: str, description: dict) -> Command: + """Build one named command from its description. + + Args: + name: the command's name, used both for the Command and to say which + command an error is about + description: {"request": {...}} and, when the instrument answers, + {"reply": {...}}, plus an optional {"sets": {...}} for a state + change the request does not carry + + Returns: + The command, its reply None when none was described. + + Raises: + BadDescription: when there is no request, or when either half does + not make sense. + """ + where = "command {0!r}".format(name) + if "request" not in description: + raise BadDescription("{0}: no request".format(where)) + reply = description.get("reply") + return Command(name, cls.requestFrom(description["request"], where), + cls.replyFrom(reply, where) if reply is not None else None, + sets=description.get("sets"), + specimen=description.get("specimen")) + + @classmethod + def requestFrom(cls, description: dict, where: str) -> Frame: + """Build the request half: a template for text, a struct for binary. + + Args: + description: the "request" object. Which key is present says which + notation it is written in. + where: how to name this command in an error + + Returns: + A TextFrame or a BinaryFrame, according to the key found. + + Raises: + BadDescription: when neither notation is present, or when a text + request states a template without the expression that reads it + back. + ProtocolError: when a struct format states no byte order. + """ + if "template" in description: + if "regex" not in description: + raise BadDescription( + "{0}: a text request needs a regex as well as its template, " + "so that a mock can read what the driver writes".format(where)) + return TextFrame(description["template"], description["regex"], + fields=cls.convertersFor(description.get("fields", {}), where)) + if "struct" in description: + return BinaryFrame(description["struct"], + fields=tuple(description.get("fields", ())), + constants=cls.constantsFrom(description)) + raise BadDescription( + "{0}: a request needs a template, for text, or a struct, for binary".format(where)) + + @classmethod + def replyFrom(cls, description: dict, where: str) -> Frame: + """Build the reply half: a regex for text, a struct for binary. + + A text reply states both notations, like a text request; a binary one needs + only its struct, since pack and unpack are already each other's inverse. + + Args: + description: the "reply" object + where: how to name this command in an error + + Returns: + A TextFrame or a BinaryFrame, according to the key found. + + Raises: + BadDescription: when neither notation is present, or when a text + reply states an expression without the template that writes it. + ProtocolError: when a struct format states no byte order. + """ + if "regex" in description: + if "template" not in description: + raise BadDescription( + "{0}: a text reply needs a template as well as its regex, " + "so that a mock can write what the driver reads".format(where)) + return TextFrame(description["template"], description["regex"], + fields=cls.convertersFor(description.get("fields", {}), where)) + if "struct" in description: + return BinaryFrame(description["struct"], + fields=tuple(description.get("fields", ())), + constants=cls.constantsFrom(description)) + raise BadDescription( + "{0}: a reply needs a regex, for text, or a struct, for binary".format(where)) + + @classmethod + def constantsFrom(cls, description: dict) -> dict: + """Turn the constants of a binary frame from text in a file into bytes. + + Args: + description: a "request" or "reply" object, whose "constants" is read + if present + + Returns: + {field name: bytes}, empty when the frame fixes nothing. + """ + return {name: cls.bytesFrom(value) + for name, value in description.get("constants", {}).items()} + + @classmethod + def convertersFor(cls, fields: dict, where: str) -> dict: + """Turn {"power": "float"} from a file into {"power": float}. + + Args: + fields: {field name: converter name}, as a file writes it + where: how to name this command in an error + + Returns: + {field name: callable}, in the same order, ready for a TextFrame. + + Raises: + BadDescription: when a converter name is not one this class knows, + listing the ones that do exist. + """ + resolved = {} + for name, converterName in fields.items(): + if converterName not in cls.converters: + raise BadDescription( + "{0}: {1} names the converter {2!r}, which is not one of {3}".format( + where, name, converterName, ", ".join(sorted(cls.converters)))) + resolved[name] = cls.converters[converterName] + return resolved + + @staticmethod + def bytesFrom(text: str) -> bytes: + """Turn a constant written in a file into the byte it stands for. + + Args: + text: one character of a JSON string, such as "M" or "\r" + + Returns: + The bytes, encoded latin-1 so that \xfe stays one byte rather than + becoming the two UTF-8 would spell it with. + """ + return text.encode("latin-1") + + @property + def names(self) -> tuple: + """Every command this device understands. + + Returns: + The names, in the order the description listed them. + """ + return tuple(self.commands) + + def __getitem__(self, name: str) -> Command: + """Look one command up by name. + + Args: + name: the command's name, as the description spells it + + Returns: + The command. + + Raises: + KeyError: naming the device and listing the commands it does have, + since a typo is the likeliest reason to be here. + """ + if name not in self.commands: + raise KeyError("{0} has no command {1!r}; it has {2}".format( + self.deviceName or "this device", name, ", ".join(sorted(self.commands)))) + return self.commands[name] + + def recognize(self, data: Union[bytes, str]) -> Tuple[Command, dict]: + """Work out which command a request belongs to, and what it carried. + + Every command is asked in turn whether the bytes are its request, and the + first that says yes wins -- so a mock dispatches on the same descriptions + the driver sends with, and no prefix table is written twice. + + Args: + data: one complete request. Deciding where a request ends in a stream + is the port's business, exactly as it is on the reply side. + + Returns: + A (command, arguments) pair: the command that recognised the bytes, + and the dict its request decoded to. + + Raises: + RequestDidNotMatch: when no command recognises the bytes, naming the + device and quoting them. + """ + for command in self.commands.values(): + try: + return command, command.decodeRequest(data) + except DidNotMatch: + continue + raise RequestDidNotMatch("{0} has no command matching {1!r}".format( + self.deviceName or "this device", data)) + + # A value of each type, for validate() to send through a description and get + # back. Deliberately dull: a specimen is not test data, it only has to survive + # the trip. 1 rather than 0, and "1" rather than "", because an expression that + # excludes a leading zero or an empty group is common and would refuse those. + # A subclass may add to it; one command may override it with "specimen". + specimens = { + "float": 0.5, "double": 0.5, "float16": 0.5, + "int": 1, "int8": 1, "uint8": 1, "int16": 1, "uint16": 1, + "int32": 1, "uint32": 1, "int64": 1, "uint64": 1, + "bool": True, + "str": "1", "text": "1", + "byte": b"1", + } + + def validate(self) -> "CommandDictionary": + """Refuse a description that is wrong about itself. + + Reading a file only catches what is missing or misspelled. What it cannot + catch is a description that parses and then does not work: a template and + an expression that describe different lines, a struct that packs a + different number of values than it names, a command whose request another + command answers to first. None of those show until something speaks the + protocol -- usually a debug port, months later. + + So this speaks it. Every command is written out with specimen values and + read straight back, and every request is handed to recognize() to check it + is taken for itself. Call it in a driver's test, once, over the protocol + that driver ships. + + Returns: + The dictionary, so a description can be read and checked in one + breath: CommandDictionary.fromFile(path).validate(). + + Raises: + BadDescription: listing every fault at once, since a file with three + mistakes should not be corrected three times. + """ + found = self.faults() + if found: + raise BadDescription("{0} is not consistent with itself:\n {1}".format( + self.deviceName or "this device", "\n ".join(found))) + return self + + def faults(self) -> list: + """Everything wrong with this description, without raising. + + Returns: + One sentence per fault, each naming the command and the half it is + about. Empty for a description that is right about itself. + """ + found = [] + for name, command in self.commands.items(): + ofRequest = self.faultsOfHalf(name, "request", command.request, + command.specimen) + found.extend(ofRequest) + if command.reply is not None: + found.extend(self.faultsOfHalf(name, "reply", command.reply, + command.specimen)) + if not ofRequest: + found.extend(self.faultsOfRecognition(name, command)) + return found + + def faultsOfHalf(self, name: str, role: str, frame: Frame, specimen: dict) -> list: + """Everything wrong with one half of one command. + + A frame that does not add up is not then written out: the round trip + would fail for the reason already given, and one mistake deserves one + sentence. + + Args: + name: the command's name + role: "request" or "reply", to say which half + frame: the frame to check + specimen: values this command states for itself + + Returns: + One sentence per fault, each prefixed with the command and the half. + """ + where = "{0} {1}".format(name, role) + found = ["{0}: {1}".format(where, fault) for fault in frame.faults()] + if found: + return found + return self.faultsOfRoundTrip(where, frame, specimen) + + def faultsOfRoundTrip(self, where: str, frame: Frame, specimen: dict) -> list: + """Write a frame out with specimen values and read them straight back. + + This is the only check that can catch two notations describing different + lines, because it is the only one that makes a line. + + Args: + where: the command and half being checked, for the message + frame: the frame to write and read + specimen: values this command gives for itself, overriding the ones + made from the declared types + + Returns: + One sentence per fault, empty when what went out came back. + """ + values, missing = self.specimenFor(frame, specimen) + if missing is not None: + return ["{0}: {1}".format(where, missing)] + try: + readBack = frame.decode(frame.encode(**values)) + except Exception as error: + return ["{0}: {1} cannot be written and read back: {2}".format( + where, values, error)] + if readBack != values: + return ["{0}: wrote {1} and read back {2}".format(where, values, readBack)] + return [] + + def faultsOfRecognition(self, name: str, command: Command) -> list: + """Check that a command's own request is taken for that command. + + Two commands can describe requests that each other's descriptions accept + -- a text one whose expression is loose enough to match another's line, or + a binary one that forgot to fix its header. recognize() then hands a debug + port the wrong command, and the driver is answered as though it had asked + something else. + + Args: + name: the command's name + command: the command itself + + Only reached when the request is sound on its own, since a request that + cannot be written and read back has already been reported and would fail + this too, for the same reason. + + Returns: + One sentence per fault, empty when the command recognises itself. + Silent when no specimen could be made, since the round trip has + already said so. + """ + values, missing = self.specimenFor(command.request, command.specimen) + if missing is not None: + return [] + try: + recognized, _ = self.recognize(command.request.encode(**values)) + except Exception as error: + return ["{0}: no command recognises its own request: {1}".format(name, error)] + if recognized.name != name: + return ["{0}: its request is taken for {1}, which is described first".format( + name, recognized.name)] + return [] + + def specimenFor(self, frame: Frame, given: dict) -> Tuple[Optional[dict], Optional[str]]: + """A value for each name a frame carries, to send through it and back. + + Args: + frame: the frame to make values for + given: what the command states for itself, which wins over the types + + Returns: + A (values, complaint) pair, exactly one of which is None: the values + to use, or the reason there are none -- a type this class has no + specimen for, which the description must then supply itself. + """ + values = {} + for name, typeName in frame.parameters: + if name in given: + values[name] = given[name] + elif typeName in self.specimens: + values[name] = self.specimens[typeName] + else: + return None, ('no specimen for {0}, of type {1}; state one with ' + '"specimen"'.format(name, typeName)) + return values, None + + def usage(self) -> str: + """Returns every command, what to pass it and what it answers, as text. + + This is what someone needs before writing a single call: the names to give + and the names that come back. It is read off the description, so unlike a + comment or a README it cannot say something the protocol no longer does. + + Commands appear in the order the description listed them, since that order + is usually the one whoever wrote the file thought in. + + Returns: + The whole text, one paragraph per command, ready to print. + """ + lines = ["{0}: {1} commands".format(self.deviceName or "This device", len(self))] + for name in self.names: + lines.extend(self.usageFor(self.commands[name])) + return "\n".join(lines) + + def usageFor(self, command: Command) -> list: + """Explain one command: how to call it, and what comes back. + + Args: + command: the command to describe + + Returns: + Its lines, starting with a blank one so that paragraphs separate when + they are joined. A command is shown as a call with its arguments, + then one line for the answer: the values it carries, or that it + carries none, or that the instrument does not answer at all. + """ + lines = ["", " {0}({1})".format(command.name, self.listed(command.request))] + if not command.expectsReply: + lines.append(" the instrument does not answer") + elif not command.reply.parameters: + lines.append(" answers, carrying no values") + else: + lines.append(" answers {0}".format(self.listed(command.reply))) + return lines + + @staticmethod + def listed(frame: Frame) -> str: + """Set out one frame's values for a usage line. + + Args: + frame: a command's request or its reply + + Returns: + "name: type, name: type" in the order the frame carries them, and + an empty string for a frame that carries none -- which is what makes an + argumentless command print as NAME(). + """ + return ", ".join("{0}: {1}".format(name, typeName) + for name, typeName in frame.parameters) + + def __str__(self) -> str: + """Explain the whole protocol, so that printing a dictionary is enough. + + Returns: + What usage() returns. + """ + return self.usage() + + def __contains__(self, name: str) -> bool: + """Whether the device understands a command of that name. + + Args: + name: a command name to look for + + Returns: + True when it is one of this device's commands. + """ + return name in self.commands + + def __iter__(self) -> Iterator[str]: + """Iterate over the command names, as a dict does. + + Returns: + An iterator over the names, in description order. + """ + return iter(self.commands) + + def __len__(self) -> int: + """How many commands the device understands. + + Returns: + The count, which is also what usage() announces. + """ + return len(self.commands) diff --git a/hardwarelibrary/motion/sutterdevice.py b/hardwarelibrary/motion/sutterdevice.py index eda5bb6d..dc5a3a6f 100644 --- a/hardwarelibrary/motion/sutterdevice.py +++ b/hardwarelibrary/motion/sutterdevice.py @@ -1,44 +1,78 @@ -from hardwarelibrary.physicaldevice import * -from hardwarelibrary.motion.linearmotiondevice import * -from hardwarelibrary.communication.communicationport import * -from hardwarelibrary.communication.usbport import USBPort +from hardwarelibrary.communication.debugport import ProtocolDebugPort +from hardwarelibrary.communication.protocol import CommandDictionary from hardwarelibrary.communication.serialport import SerialPort -from hardwarelibrary.communication.commands import DataCommand, DataEncoder, DataDecoder -from hardwarelibrary.communication.debugport import TableDrivenDebugPort +from hardwarelibrary.motion.linearmotiondevice import Direction, LinearMotionDevice +from hardwarelibrary.physicaldevice import PhysicalDevice + + +# The whole protocol of the MP-285, as data. Every frame is binary and +# fixed-size: a header byte, the values, a carriage return. Naming the header and +# the terminator as constants rather than padding them over is what lets the same +# description serve both directions -- a constant is written on the way out and +# required on the way in, so a ProtocolDebugPort recognises a request by the very +# bytes the driver sends, and the driver is told when an acknowledgement is not +# the b"\r" it expected. + +sutterProtocol = { + "device": "Sutter MP-285", + "commands": { + "MOVE": { + "request": {"struct": " (int, int, int): # for compatibility + def positionInMicrosteps(self) -> (int, int, int): + """The position in microsteps. Kept for compatibility with doGetPosition.""" return self.doGetPosition() def doGetPosition(self) -> (int, int, int): - """ Returns the position in microsteps """ - cmd = self.sendCommand("GET_POSITION") - (x, y, z) = cmd.matchGroups - return (x, y, z) + """Ask the stage where it is. + + Returns: + The (x, y, z) position in microsteps. + """ + position = self.performTransaction("GET_POSITION") + return (position["x"], position["y"], position["z"]) def doMoveTo(self, position): - """ Move to a position in microsteps """ + """Move to an absolute position. + + Args: + position: an (x, y, z) triplet in microsteps, rounded to whole steps + since the frame packs them as longs + """ x, y, z = position - cmd = self.sendCommand("MOVE", x=int(x), y=int(y), z=int(z)) - if cmd.matchGroups != (b'\r',): - raise Exception(f"Expected carriage return, but got '{cmd.matchGroups}' instead.") + self.performTransaction("MOVE", x=int(x), y=int(y), z=int(z)) def doMoveBy(self, displacement): + """Move by a relative displacement, reading the position first. + + Two transactions, held under one lock: a relative move is a read, an + addition and a write, and a stage that another caller moved in between + would be displaced from a position that is no longer where it is. + + Args: + displacement: a (dx, dy, dz) triplet in microsteps + + Raises: + Exception: when the position cannot be read, since there is then + nothing to add the displacement to. + """ dx, dy, dz = displacement - x, y, z = self.doGetPosition() - if x is not None: + with self.port.transactionLock: + x, y, z = self.doGetPosition() + if x is None: + raise Exception("Unable to read position from device") self.doMoveTo((x+dx, y+dy, z+dz)) - else: - raise Exception("Unable to read position from device") def doHome(self): - cmd = self.sendCommand("HOME") - if cmd.matchGroups != (b'\r',): - raise Exception(f"Expected carriage return, but got {cmd.matchGroups} instead.") + """Send the stage to its home position.""" + self.performTransaction("HOME") def work(self): + """Send the stage home, then to its work position.""" self.home() - cmd = self.sendCommand("WORK") - if cmd.matchGroups != (b'\r',): - raise Exception(f"Expected carriage return, but got {cmd.matchGroups} instead.") - - - class DebugSerialPort(TableDrivenDebugPort): - def __init__(self): - super().__init__(commands=SutterDevice.commands) - self.xSteps = 0 - self.ySteps = 0 - self.zSteps = 0 - - def process_command(self, name, params, endPointIndex): - if name == 'MOVE': - self.xSteps = params['x'] - self.ySteps = params['y'] - self.zSteps = params['z'] - return b'\r' - elif name == 'GET_POSITION': - return {'x': self.xSteps, 'y': self.ySteps, 'z': self.zSteps, - 'terminator': b'\r'} - elif name == 'HOME': - self.xSteps = self.ySteps = self.zSteps = 0 - return b'\r' - elif name == 'WORK': - return b'\r' + self.performTransaction("WORK") diff --git a/hardwarelibrary/physicaldevice.py b/hardwarelibrary/physicaldevice.py index dd3b8857..1f3cc91c 100644 --- a/hardwarelibrary/physicaldevice.py +++ b/hardwarelibrary/physicaldevice.py @@ -84,6 +84,11 @@ class NotInitialized(Exception): classIdProduct = None commands = None + # The CommandDictionary this driver speaks, for performTransaction below. + # None until a driver sets it, which is every driver still using the older + # commands dict above. SutterDevice is the first to set it. + protocol = None + # True when classIdVendor/classIdProduct are those of a generic, off-the-shelf # USB/RS-232 converter (a stock FTDI/Prolific/CP210x/CH34x cable) rather than # the instrument's own identity. Such a device shares its VID/PID with every @@ -393,6 +398,61 @@ def sendCommand(self, name, **params): command.send(port=self.port, **params) return command + def performTransaction(self, name, **arguments) -> dict: + """Perform one command of self.protocol once, and return what came back. + + A Command says what an exchange is; this carries it out. The description + supplies the bytes to write and says how to read the answer, so a driver + that sets protocol needs no send-and-receive code of its own: it calls + this by name and gets a dict. + + How much to read is the reply's own business. A binary reply gives its + readLength, since a fixed-size frame has no terminator to stop at; a text + reply gives None, and the port reads up to its own terminator instead. + Both go through the primitives of CommunicationPort and nothing else. + + The write and the read are held together under the port's + transactionLock, so a concurrent caller cannot slip a command in between + and be handed this one's reply. Both locks are reentrant, so the port + taking portLock inside each primitive costs nothing here. + + This does not require the device to be Ready. A driver needs it inside + doInitializeDevice, to confirm the instrument answers before the state can + become Ready, and readiness is in any case the business of the public + methods that validateReady guards -- not of the wire. The sendCommand this + replaces did check, which is exactly why SutterDevice had to reach around + it during initialization. + + Args: + name: the command to perform, one of the names in self.protocol + **arguments: the values the command carries, passed by name + + Returns: + What the reply carried, as {field name: value}. Empty for a command + the instrument does not answer, and for one whose whole answer is a + fixed acknowledgement the description already states. + + Raises: + NotImplementedError: when this driver has no protocol, which means it + either still uses the commands dict or has nothing to speak with. + KeyError: when no command goes by that name, listing the ones that do. + ReplyDidNotMatch: when the instrument answers something the + description does not allow, naming the command. + """ + if self.protocol is None: + raise NotImplementedError( + "{0} has no protocol to perform {1!r} with".format( + type(self).__name__, name)) + + command = self.protocol[name] + with self.port.transactionLock: + self.port.writeData(command.encode(**arguments)) + if not command.expectsReply: + return {} + if command.reply.readLength is None: + return command.decode(self.port.readString()) + return command.decode(self.port.readData(command.reply.readLength)) + @classmethod def any(cls): """Incomplete: enumerates, discards the result, and returns None. diff --git a/hardwarelibrary/tests/testProtocolDebugPort.py b/hardwarelibrary/tests/testProtocolDebugPort.py new file mode 100644 index 00000000..cad8844f --- /dev/null +++ b/hardwarelibrary/tests/testProtocolDebugPort.py @@ -0,0 +1,167 @@ +"""A debug port that needs no code of its own, only a protocol description.""" + +import env +import unittest +from struct import pack + +from hardwarelibrary.communication.debugport import ProtocolDebugPort +from hardwarelibrary.communication.protocol import ( + CommandDictionary, RequestDidNotMatch, +) +from hardwarelibrary.motion.sutterdevice import SutterDevice +from hardwarelibrary.physicaldevice import PhysicalDevice + + +LAMP = { + "device": "A lamp that answers in text", + "commands": { + "SET_POWER": { + "request": {"template": "p {power:0.3f}\r", "regex": r"p ([0-9.]+)\r", + "fields": {"power": "float"}}, + "reply": {"regex": "OK", "template": "OK\r\n"}, + }, + "GET_POWER": { + "request": {"template": "pa?\r", "regex": r"pa\?\r"}, + "reply": {"regex": r"(\d+\.\d+)", "template": "{power:0.3f}\r\n", + "fields": {"power": "float"}}, + }, + "TURN_OFF": { + "request": {"template": "l0\r", "regex": r"l0\r"}, + "reply": {"regex": "OK", "template": "OK\r\n"}, + "sets": {"power": 0.0}, + }, + }, +} + + +class DebugLampDevice(PhysicalDevice): + """A device with nothing but a protocol, to exercise performTransaction.""" + + classIdVendor = 0xFFFF + classIdProduct = 0xFFF1 + protocol = CommandDictionary.fromDescription(LAMP) + + def __init__(self): + """Bind to nothing: this instrument only ever exists in a debug port.""" + super().__init__(serialNumber="debug", idVendor=self.classIdVendor, + idProduct=self.classIdProduct) + + def doInitializeDevice(self): + """Stand the instrument up out of its own description.""" + self.port = ProtocolDebugPort(self.protocol) + self.port.open() + + def doShutdownDevice(self): + """Put it away.""" + self.port.close() + self.port = None + + +class TestPerformTransactionBelongsToEveryDevice(unittest.TestCase): + """A driver that sets protocol needs no send-and-receive code of its own.""" + + def setUp(self): + self.device = DebugLampDevice() + self.device.initializeDevice() + + def tearDown(self): + self.device.shutdownDevice() + + def testALineIsReadToItsTerminatorAndAFrameByItsLength(self): + # The reply says which: a text reply has no readLength, so the port reads + # up to its own terminator instead. Nothing in the driver chooses. + self.assertIsNone(self.device.protocol["GET_POWER"].reply.readLength) + self.assertEqual(self.device.performTransaction("SET_POWER", power=0.25), {}) + self.assertEqual(self.device.performTransaction("GET_POWER"), {"power": 0.25}) + + def testADeviceWithNoProtocolSaysSoBeforeTouchingThePort(self): + # Never initialized, so its port is None: reaching the port at all would + # raise an AttributeError instead of saying what is actually wrong. + speechless = DebugLampDevice() + speechless.protocol = None + self.assertIsNone(speechless.port) + with self.assertRaises(NotImplementedError) as raised: + speechless.performTransaction("GET_POWER") + self.assertIn("DebugLampDevice", str(raised.exception)) + + +class TestItStandsInForTheSutter(unittest.TestCase): + """The binary case, against the description SutterDevice itself speaks.""" + + def setUp(self): + self.port = ProtocolDebugPort(SutterDevice.protocol) + self.port.open() + + def tearDown(self): + self.port.close() + + def ask(self, name: str, **arguments) -> dict: + """Send one command the way the driver does, and read its reply.""" + command = SutterDevice.protocol[name] + self.port.writeData(command.encode(**arguments)) + return command.decode(self.port.readData(command.reply.readLength)) + + def testItAnswersFromNothingButTheDescription(self): + # No subclass, no process_command, no table of prefixes: the port was + # handed a dictionary and that is all it has. + self.assertEqual(type(self.port), ProtocolDebugPort) + self.assertEqual(self.ask("GET_POSITION"), {"x": 0, "y": 0, "z": 0}) + + def testWhatARequestCarriesIsWhatALaterReplyGivesBack(self): + self.assertEqual(self.ask("MOVE", x=4000, y=5000, z=6000), {}) + self.assertEqual(self.ask("GET_POSITION"), {"x": 4000, "y": 5000, "z": 6000}) + + def testACommandThatCarriesNothingCanStillChangeTheInstrument(self): + # HOME is the case no description of bytes can reach: it takes no + # arguments and moves the stage anyway. Its "sets" clause says so. + self.ask("MOVE", x=1, y=2, z=3) + self.assertEqual(self.ask("HOME"), {}) + self.assertEqual(self.ask("GET_POSITION"), {"x": 0, "y": 0, "z": 0}) + + def testACommandWithNoSetsClauseLeavesTheInstrumentAlone(self): + self.ask("MOVE", x=1, y=2, z=3) + self.ask("WORK") + self.assertEqual(self.ask("GET_POSITION"), {"x": 1, "y": 2, "z": 3}) + + def testAnUnrecognizedRequestIsRefusedRatherThanIgnored(self): + # A debug port that dropped it would hide the driver bug it exists to find. + with self.assertRaises(RequestDidNotMatch): + self.port.writeData(pack(" dict: + """Send one command and read the line it answers with.""" + command = self.protocol[name] + self.port.writeData(command.encode(**arguments)) + return command.decode(self.port.readString()) + + def testWhatWasSetIsWhatIsRead(self): + self.assertEqual(self.ask("SET_POWER", power=0.25), {}) + self.assertEqual(self.ask("GET_POWER"), {"power": 0.25}) + + def testAValueNeverSetReadsAsZero(self): + self.assertEqual(self.ask("GET_POWER"), {"power": 0.0}) + + def testASetsClauseWorksTheSameWayOnText(self): + self.ask("SET_POWER", power=0.25) + self.assertEqual(self.ask("TURN_OFF"), {}) + self.assertEqual(self.ask("GET_POWER"), {"power": 0.0}) + + +if __name__ == "__main__": + unittest.main() diff --git a/hardwarelibrary/tests/testProtocolPrototype.py b/hardwarelibrary/tests/testProtocolPrototype.py index c83a9ca5..36e9f77d 100644 --- a/hardwarelibrary/tests/testProtocolPrototype.py +++ b/hardwarelibrary/tests/testProtocolPrototype.py @@ -1,1189 +1,24 @@ -"""A prototype protocol description, developed in isolation inside its test file. - -Nothing here is imported by the library, and nothing here touches a port: this is -a sketch of how an instrument protocol could be *described*, so the design can be -judged before any driver depends on it. - -The idea is sans-I/O, the principle behind h11 and wsproto: the protocol is a pure -transformation over bytes and never performs the exchange. A Frame turns named -values into the bytes to write and the bytes read back into named values; it owns -no port and stores nothing of what happened. That is what separates this from the -Command it would replace, which describes the protocol, builds the bytes, performs -the I/O, and then keeps the reply on itself -- on a class attribute shared by every -instance of a driver. - -Consequences worth noticing while reading: - - - a description is immutable and shareable; the result of a command is a plain - dict returned to the caller, so two instruments cannot overwrite each other; - - a frame parses whatever it is handed, and says how many bytes to read only - where nothing else could know: a fixed-size binary frame; - - there is no request class and no reply class: a frame becomes one or the - other by the slot of the Command it sits in, which is also the only place - that knows enough to name what failed to match; - - a constant is stated once and serves both directions -- written on the way - out, required on the way in -- which is what lets a mock tell one command - from another, and a driver notice an acknowledgement that is not its own; - - decoding failure raises, naming what did not match, instead of being recorded - in an attribute nobody checks. - -Both directions are built here. A driver writes the request and reads the reply; a -mock standing in for the instrument reads the request and writes the reply, out of -the same objects. - -Where the two directions are the same statement, they are written once; where they -are not, they are both written out. A binary frame is packed and unpacked from one -struct format, because pack and unpack are exactly each other's inverse -- asking -for a second format there is how the old description ended up with ' bytes: - """Build the bytes this frame puts on the wire. - - Args: - **values: the values to write, passed by name, one per entry of - parameters -- encode(power=0.5) for a frame that carries a power. - Anything the description fixes is supplied by the description - and must not be passed. The annotation is object because the - type each value must have is not a property of this method but - of the field it goes into, which only the description knows: - parameters reports it, one name at a time. - - Returns: - The bytes to write, terminator included, ready to hand to a port. - """ - ... - - @abstractmethod - def decode(self, data: Union[bytes, str]) -> dict: - """Read bytes this frame describes and name what they carried. - - Args: - data: the bytes read, or a str -- what a port hands over varies. - - Returns: - A dict of {field name: value}, empty when the frame carries nothing - but its own literal text or its own fixed bytes. - - Raises: - DidNotMatch: when the bytes are not the ones this frame describes. - """ - ... - - @property - @abstractmethod - def parameters(self) -> tuple: - """The names to pass this frame, and the names it hands back, with types. - - The two are the same list, since a frame is written and read from one - description: encode takes these names as keywords and decode returns them - as keys. Not used to encode or decode anything, though -- it exists so - that a dictionary can explain itself out of the description rather than - out of a comment. - - Returns: - A (name, type name) pair per value, in the order the frame lays them - out, such as (("x", "int32"), ("y", "int32"), ("z", "int32")). Empty - when the frame is nothing but fixed text or fixed bytes. - """ - ... - - -class TextFrame(Frame): - """A line of ASCII, stated as the template that writes it and the expression - that reads it. - - The template uses named fields, so a caller writes setPower(power=0.5) and - never counts positional arguments: "p {power:0.3f}\r". Whatever ends the line is - written into it, where it can be seen, rather than passed alongside: a - terminator is simply more literal text, and instruments disagree about it enough - -- \r, \n, \r\n, or nothing at all for the Integra -- that no default would - serve. - - fields maps a name to the converter for its capture group, in group order: - {"power": float} turns r"(\\d+\\.\\d+)" into {"power": 0.123}. A line with no - capture groups, such as an "OK" acknowledgement, decodes to an empty dict -- it - either matched or it raised. - - Neither notation is worked out from the other. A template could be turned into - an expression by guessing a pattern per format spec, and the guess would be - right often enough to be trusted and wrong quietly: a field with no spec would - come back as text and nothing would say so. An expression cannot be turned back - into a line at all. This module already refuses to let struct guess a byte - order; guessing here is not of a different kind. - - One class serves both halves of a command. The template is what a driver sends - when the frame is a request and what a mock answers when it is a reply; the - expression is read by whichever of the two is listening. - """ - - def __init__(self, template: str, regex: str, - fields: Optional[dict] = None): - """Describe a line by its two notations. - - The three arguments describe one line three ways, and they have to agree: - "p {power:0.3f}\r" is written with r"p ([0-9.]+)\r" and {"power": float}. - One placeholder, one capture group, one converter, all called power. - - Args: - template: the str.format template that writes the line, terminator - included, with a {name} where each value goes - regex: the expression that reads the line back, with one capture - group per value, in the same order the template writes them - fields: the converter to run on each capture group, keyed by the - name the template uses -- float for a power, int for a count. - None or empty for a line with no values at all, such as an "OK". - """ - self.template = template - self.regex = regex - self.fields = dict(fields or {}) - - @property - def arguments(self) -> tuple: - """The names a caller must supply. - - They are the {name} placeholders of the template, and they are found by - handing the template to string.Formatter().parse, which splits it into - literal text and field names so that nothing has to be scanned by hand. - - Returns: - Every placeholder name, in the order the template lays them out: - ("power",) for "p {power:0.3f}\r", and ("register", "value") for - "s r{register} {value}\r". Empty for a template of literal text - only, such as "pa?\r", which is a request that takes no arguments. - """ - return tuple(name for _, name, _, _ in string.Formatter().parse(self.template) - if name) - - @property - def parameters(self) -> tuple: - """Those same names, each with the type it is read back as. - - The names come from the template and the types from the converters, which - are two independent lists: a name the converters do not mention is - reported as text, since nothing says otherwise. - - Returns: - A (name, type name) pair per placeholder, in template order: - (("power", "float"),) for "p {power:0.3f}\r" described with - {"power": float}. - """ - return tuple((name, self.typeNameOf(self.fields[name]) - if name in self.fields else "text") - for name in self.arguments) - - @staticmethod - def typeNameOf(converter: Callable[[str], object]) -> str: - """Name a converter for the benefit of someone reading a usage line. - - A converter is often a type, and then its own name is the answer. When it is - a function, what a reader wants is what it returns, which its annotation - already says -- so a "0" or "1" field is announced as a bool rather than by - the name of the function that makes one. - - This is a guess, and it is allowed to be one because nothing it returns - ever reaches the wire: parameters is its only caller and usage() is the - only thing that reads parameters. The rule that nothing here is inferred - governs the protocol, not the sentence that explains it. Its worst answer - is an ugly one -- "" -- never a wrong frame. - - Args: - converter: anything callable on a captured string -- a type such as - float, or a function such as booleanFromZeroOrOne - - Returns: - The name of what the converter returns when it is annotated, its own - name otherwise, and its repr as a last resort for a lambda. - """ - returned = getattr(converter, "__annotations__", {}).get("return") - if returned is not None: - return getattr(returned, "__name__", str(returned)) - return getattr(converter, "__name__", None) or str(converter) - - def encode(self, **values: object) -> bytes: - """Write the line, substituting the values into the template. - - Args: - **values: the values to substitute, passed by name, one per - {name} in the template. A value the template never mentions is - ignored, as str.format ignores it. Any object will do, since - a format spec is applied to whatever it is handed -- a float for - "{power:0.3f}", but equally a datetime for "{when:%H:%M}". - - Returns: - The line as UTF-8 bytes, terminator included since the template - carries it. - - Raises: - MissingArgument: when a {name} of the template was not supplied, - naming the field and listing what was given, rather than letting - a bare KeyError out of str.format. - """ - try: - return self.template.format(**values).encode("utf-8") - except KeyError as error: - raise MissingArgument("{0} needs {1}, got {2}".format( - self.template, error, sorted(values))) from None - - def decode(self, data: Union[bytes, str]) -> dict: - """Read the line back and convert each captured group. - - Args: - data: the line as read, bytes or str -- what a port hands over - varies. Anything around the match is ignored, since re.search is - used: where a line ends is the port's business. - - Returns: - A dict of {field name: converted value}, empty when the expression - has no capture groups. - - Raises: - DidNotMatch: when the expression does not match, quoting both sides. - ProtocolError: when the expression and the fields disagree on how - many values the line carries. - """ - text = data.decode("utf-8") if isinstance(data, (bytes, bytearray)) else data - match = re.search(self.regex, text) - if match is None: - raise DidNotMatch("expected {0!r}, got {1!r}".format(self.regex, text)) - - groups = match.groups() - if len(groups) != len(self.fields): - raise ProtocolError( - "{0!r} captured {1} group(s) but {2} field(s) were named".format( - self.regex, len(groups), len(self.fields))) - return {name: converter(value) - for (name, converter), value in zip(self.fields.items(), groups)} - - -class BinaryFrame(Frame): - """A fixed-size binary frame, named field by field, readable and writable. - - fields names each value in the struct format, in order; constants are the ones - nobody supplies, such as a header byte or a trailing carriage return. A - constant is written on the way out and required on the way in, so the one line - that produces b"M" also refuses a frame that does not begin with it -- which is - how a mock tells one command from another, and how a driver is told that an - acknowledgement was not the one it expected. - - Padding ('x') has no place here. It can be unpacked but not packed, so a frame - that skipped its terminator could be read and never written, and the mock - direction would need a second format for the same bytes. Name the byte and make - it a constant instead. - """ - - byteOrderPrefixes = ("<", ">", "!", "=") - - # What each struct code is called when a command explains itself. A class - # attribute rather than a module one, so a frame for an instrument that reads - # its own kind of value can add to it, the way CommandDictionary does with - # converters. - typeNames = { - "c": "byte", "?": "bool", "b": "int8", "B": "uint8", - "h": "int16", "H": "uint16", "i": "int32", "I": "uint32", - "l": "int32", "L": "uint32", "q": "int64", "Q": "uint64", - "e": "float16", "f": "float", "d": "double", "s": "bytes", "p": "bytes", - } - - def __init__(self, struct: str, fields: tuple = (), - constants: Optional[dict] = None): - """Describe a frame as a struct format, one name per packed value. - - Args: - struct: a struct format, byte order included, or the frame it builds - is the one a C compiler would want rather than the one the - instrument expects - fields: one name per value the format packs, in order, padding - excluded since padding packs no value - constants: the values nobody supplies, keyed by field name -- a - header byte, a terminator, a fixed acknowledgement. Written on - the way out, required on the way in. - - Raises: - ProtocolError: when the format states no byte order. - BadDescription: when constants fixes a name fields does not list, - which would otherwise fail much later and much less clearly. - """ - self.requireExplicitByteOrder(struct) - self.struct = struct - self.fields = tuple(fields) - self.constants = dict(constants or {}) - - unknown = sorted(set(self.constants) - set(self.fields)) - if unknown: - raise BadDescription("{0} fixes {1}, which {2} does not name".format( - self.struct, ", ".join(unknown), list(self.fields))) - - @property - def readLength(self) -> int: - """How many bytes to read before this frame can be decoded. - - Returns: - The size of the format, so it never has to be kept in step by hand. - This is the one thing a caller could not work out for itself: a - fixed-size frame has no terminator to stop at. - """ - return calcsize(self.struct) - - @property - def arguments(self) -> tuple: - """The names a caller must supply. - - Returns: - A tuple of every field that is not a constant, in format order. A - frame made only of constants returns an empty tuple. - """ - return tuple(name for name in self.fields if name not in self.constants) - - @property - def parameters(self) -> tuple: - """Each non-constant field with the type its format packs it as. - - Returns: - A tuple of (name, type name) pairs, in format order, the type being - read off the struct code -- "l" is reported as int32. A code this - class does not name is reported as "unknown" rather than hidden. - """ - codeOf = dict(zip(self.fields, self.structCodes(self.struct))) - return tuple((name, self.typeNames.get(codeOf.get(name), "unknown")) - for name in self.arguments) - - @classmethod - def requireExplicitByteOrder(cls, struct: str) -> None: - """Refuse a struct format that does not begin with a byte-order prefix. - - Without one, struct uses native sizes and native alignment: "clllc" is 33 - bytes on this machine rather than the 14 the instrument expects, an "l" is - whatever a C long happens to be, and padding appears between the fields. - The frame is then correct for the compiler and wrong for the wire, and - nothing downstream would notice -- the same failure the ctypes variant - needed _pack_ = 1 to avoid, at the price of one character here. - - A classmethod, and the prefixes a class attribute, so that a frame for a - machine whose formats are spelled differently overrides the pair rather - than working around a module function it cannot reach. - - Args: - struct: a struct format, expected to start with one of the prefixes - byteOrderPrefixes lists - - Returns: - Nothing. It is a guard, called for its refusal. - - Raises: - ProtocolError: when the prefix is missing, giving both the length the - format would really produce and the one that was meant, and the - corrected format to write instead. - """ - if not struct.startswith(cls.byteOrderPrefixes): - raise ProtocolError( - "{0!r} has no byte-order prefix, so struct would use native sizes " - "and alignment: {1} bytes instead of {2}. Write {3!r}.".format( - struct, calcsize(struct), calcsize("<" + struct), "<" + struct)) - - @staticmethod - def structCodes(struct: str) -> tuple: - """Split a struct format into the type code of each value it packs. - - Args: - struct: a struct format, byte-order prefix included -- the prefix is - skipped, not treated as a code - - Unlike typeNameOf, which also serves nothing but the usage text, this is - not a guess: the struct codes are a closed, documented set, so it can be - read exactly and is expected to be. - - Returns: - One code per packed value, repeat counts expanded, so that the codes - line up with the field names one for one. Padding yields no value and - so no code, and whitespace is skipped as struct itself skips it. A - count on "s" means one string that long, not that many strings, which - is the one place the rule is not repetition. - """ - codes = [] - count = "" - for character in struct[1:]: - if character.isspace(): - continue - if character.isdigit(): - count += character - continue - repeats = int(count) if count else 1 - count = "" - if character == "x": - continue - codes.extend([character] if character in "sp" else [character] * repeats) - return tuple(codes) - - def encode(self, **arguments: object) -> bytes: - """Pack the frame, constants and arguments interleaved in field order. - - Args: - **arguments: the values to pack, passed by name, one per entry of - arguments. A constant must not be passed: the description - supplies it. What each value must be is decided by its struct - code and by nothing here -- an int for "l", bytes for "c" -- so - it is parameters that answers, and a wrong kind is refused by - pack rather than by the signature. - - Returns: - The packed frame, exactly readLength bytes long. - - Raises: - MissingArgument: for a field nobody supplied, naming it and listing - what was given. - ProtocolError: for a value struct cannot pack into its format, such - as a string where a long was expected. - """ - values = [] - for name in self.fields: - if name in self.constants: - values.append(self.constants[name]) - elif name in arguments: - values.append(arguments[name]) - else: - raise MissingArgument("{0} needs {1}, got {2}".format( - self.struct, name, sorted(arguments))) - try: - return pack(self.struct, *values) - except StructError as error: - raise ProtocolError("cannot pack {0} into {1}: {2}".format( - values, self.struct, error)) from None - - def decode(self, data: bytes) -> dict: - """Unpack the frame and name what it carried, constants left out. - - A constant is checked rather than returned: it carries no information, and - a frame whose header is wrong is not this frame at all. - - Args: - data: exactly readLength bytes. Unlike a line, a frame cannot be - surrounded by anything, since its length is its only boundary. - - Returns: - A dict of {field name: value} for the non-constant fields only, so a - frame made of constants alone decodes to an empty dict. - - Raises: - DidNotMatch: when the length is wrong, or when a constant is not the - value the description fixed -- which is how a mock tells one - command from another. - ProtocolError: when the format and the field names disagree on how - many values the frame carries. - """ - if len(data) != self.readLength: - raise DidNotMatch("expected {0} bytes for {1}, got {2}".format( - self.readLength, self.struct, len(data))) - values = unpack(self.struct, bytes(data)) - if len(values) != len(self.fields): - raise ProtocolError( - "{0} unpacks {1} value(s) but {2} field(s) were named".format( - self.struct, len(values), len(self.fields))) - - named = dict(zip(self.fields, values)) - for name, constant in self.constants.items(): - if named[name] != constant: - raise DidNotMatch("{0} expects {1}={2!r}, got {3!r}".format( - self.struct, name, constant, named[name])) - return {name: value for name, value in named.items() - if name not in self.constants} - - -class Command: - """One request and the reply it expects, named for a driver to call by name. - - A command is a description and nothing else: it says what to send and what - should come back, and carries neither the data nor the transmission. encode() - hands the caller the bytes to write, decode() turns what came back into - values, and the port in between is the caller's. - - The same description read from the other end plays the instrument: - decodeRequest() is what a mock hears, encodeReply() what it answers. Two pairs - of methods, one protocol, no second table to keep in step -- where the Command - this replaces needed a request decoder and a reply encoder written out beside - the request encoder and the reply decoder. - - That is the whole difference from the Command this replaces, which described - the protocol, built the bytes, performed the exchange, and then kept the - reply on itself. - - A command is also where the two halves are told apart. A Frame is neither a - request nor a reply -- it becomes one by being put in one of these two slots -- - so a frame that fails to match says only that, and a command turns it into a - RequestDidNotMatch or a ReplyDidNotMatch naming itself. Which is more than - either one could say on its own: a frame does not know what command it - belongs to. - """ - - def __init__(self, name: str, request: Frame, reply: Optional[Frame] = None): - """Pair a request with the reply it expects, under the name a driver uses. - - Args: - name: how a driver asks for this command, and how it names itself in - an error - request: the frame the driver writes and a mock reads - reply: the frame the driver reads and a mock writes, or None for a - command the instrument does not answer - """ - self.name = name - self.request = request - self.reply = reply - - @property - def expectsReply(self) -> bool: - """Whether the instrument answers this command at all. - - Returns: - True when a reply was described, so a caller knows whether to read. - """ - return self.reply is not None - - def encode(self, **arguments: object) -> bytes: - """Build the bytes to send -- the driver's half of the exchange. - - Args: - **arguments: the values the request carries, passed by name -- - one per entry of request.parameters. - - Returns: - The bytes to write. What happens to them is the caller's business: - nothing here touches a port. - """ - return self.request.encode(**arguments) - - def decode(self, data: Union[bytes, str]) -> dict: - """Read what the instrument answered -- the driver's other half. - - Args: - data: the bytes read back. How many to read is reply.readLength when - the reply is binary, and the port's own terminator when it is a - line. - - Returns: - A dict of {field name: value}, returned to the caller and stored - nowhere, so two callers of one description cannot overwrite each - other. - - Raises: - ProtocolError: when this command expects no reply, since decoding one - means the caller read something it should not have. - ReplyDidNotMatch: when the instrument answered something else, naming - this command. - """ - if self.reply is None: - raise ProtocolError("{0} expects no reply".format(self.name)) - return self.decodeHalf(self.reply, data, ReplyDidNotMatch) - - def decodeRequest(self, data: Union[bytes, str]) -> dict: - """Read a request addressed to us -- the mock's half of encode. - - Args: - data: one complete request as received. - - Returns: - A dict of the arguments it carried, empty for a command that takes - none. - - Raises: - RequestDidNotMatch: when the bytes are some other command's request, - which is how a mock picks the right one out of a dictionary - rather than an error a driver would ever see. - """ - return self.decodeHalf(self.request, data, RequestDidNotMatch) - - def decodeHalf(self, half: Frame, data: Union[bytes, str], - mismatch: Type[DidNotMatch]) -> dict: - """Decode one half, saying which half of which command failed to match. - - A frame knows only that the bytes are not the ones it describes. Naming the - command, and which end of it was being read, is something only a command can - do -- so it is done here rather than passed down. - - Args: - half: the frame to decode with, this command's request or its reply - data: the bytes to read - mismatch: the exception class to raise on failure, which is what - records the role -- RequestDidNotMatch or ReplyDidNotMatch - - Returns: - Whatever the frame decoded, untouched. - - Raises: - The mismatch given, with this command's name prefixed to the frame's - own account of what did not match. - """ - try: - return half.decode(data) - except DidNotMatch as error: - raise mismatch("{0}: {1}".format(self.name, error)) from None - - def encodeReply(self, **values: object) -> bytes: - """Build the answer the instrument would give -- the mock's half of decode. - - Args: - **values: the values the reply carries, passed by name -- one per - entry of reply.parameters. - - Returns: - The bytes a mock writes back, terminator included. - - Raises: - ProtocolError: when this command expects no reply, since a mock that - answered would be answering a command the instrument leaves - silent. - """ - if self.reply is None: - raise ProtocolError("{0} expects no reply".format(self.name)) - return self.reply.encode(**values) - - -def booleanFromZeroOrOne(text: str) -> bool: - """Read a "0" or "1" field as a boolean. - - Args: - text: one capture group, expected to be "0" or "1" - - Returns: - True for "1", False for anything else -- an instrument that answers - neither is refused by the expression, not here. - """ - return text == "1" - - -def integerFromHexadecimal(text: str) -> int: - """Read a field written in hexadecimal as an integer. - - Args: - text: one capture group of hexadecimal digits, with or without a 0x - prefix, which int accepts either way - - Returns: - The value the digits spell. - - Raises: - ValueError: when the group is not hexadecimal, which means the - expression captured something it should not have. - """ - return int(text, 16) - - -class CommandDictionary: - """Every command one device understands, by name, read from a file. - - A driver's protocol is a table, and a table is data: a description read from - JSON builds the same objects a driver would write by hand, so a protocol can - be read, reviewed and corrected without touching the code that speaks it. - - JSON rather than TOML or YAML because it is in the standard library of every - Python the package supports; tomllib arrives only in 3.11, and YAML would be a - dependency for a file nobody edits at runtime. - - A command is described by a request and, when there is one, a reply. Which key - is present says which kind it is, so nothing declares a type twice, and the key - names the notation its string is written in: a str.format template on the way - out, a regular expression on the way in, a struct format for bytes in either - direction. - - "SET_POWER": { - "request": {"template": "p {power:0.3f}\r", - "regex": "p ([0-9.]+)\r", - "fields": {"power": "float"}}, - "reply": {"regex": "OK", "template": "OK\r\n"} - }, - "GET_POWER": { - "request": {"template": "pa?\r", "regex": "pa\\?\r"}, - "reply": {"regex": "(\\d+\\.\\d+)", "template": "{power:0.4f}\r\n", - "fields": {"power": "float"}} - }, - "MOVE": { - "request": {"struct": " "CommandDictionary": - """Read one device's commands from a JSON file. - - Args: - path: the file to read - - Returns: - A dictionary of the commands it describes. - - Raises: - OSError: when the file cannot be read. - json.JSONDecodeError: when it is not JSON. - BadDescription: when it is JSON but not a protocol. - """ - with open(path, "r") as file: - return cls.fromDescription(json.load(file)) - - @classmethod - def fromJSON(cls, text: str) -> "CommandDictionary": - """Read them from JSON already in hand. - - Args: - text: the description as JSON, from wherever it came - - Returns: - A dictionary of the commands it describes. - """ - return cls.fromDescription(json.loads(text)) - - @classmethod - def fromDescription(cls, description: dict) -> "CommandDictionary": - """Build from the description itself, however it was obtained. - - Args: - description: {"device": name, "commands": {name: {...}}}. The device - name is optional; the commands are not. - - Returns: - A dictionary whose command order is the description's order, since - that is usually the order whoever wrote the file thought in. - - Raises: - BadDescription: when there are no commands at all, or when any one of - them does not make sense. - """ - if "commands" not in description: - raise BadDescription("no commands: expected {'device': ..., 'commands': {...}}") - return cls({name: cls.commandFrom(name, one) - for name, one in description["commands"].items()}, - deviceName=description.get("device")) - - @classmethod - def commandFrom(cls, name: str, description: dict) -> Command: - """Build one named command from its description. - - Args: - name: the command's name, used both for the Command and to say which - command an error is about - description: {"request": {...}} and, when the instrument answers, - {"reply": {...}} - - Returns: - The command, its reply None when none was described. - - Raises: - BadDescription: when there is no request, or when either half does - not make sense. - """ - where = "command {0!r}".format(name) - if "request" not in description: - raise BadDescription("{0}: no request".format(where)) - reply = description.get("reply") - return Command(name, cls.requestFrom(description["request"], where), - cls.replyFrom(reply, where) if reply is not None else None) - - @classmethod - def requestFrom(cls, description: dict, where: str) -> Frame: - """Build the request half: a template for text, a struct for binary. - - Args: - description: the "request" object. Which key is present says which - notation it is written in. - where: how to name this command in an error - - Returns: - A TextFrame or a BinaryFrame, according to the key found. - - Raises: - BadDescription: when neither notation is present, or when a text - request states a template without the expression that reads it - back. - ProtocolError: when a struct format states no byte order. - """ - if "template" in description: - if "regex" not in description: - raise BadDescription( - "{0}: a text request needs a regex as well as its template, " - "so that a mock can read what the driver writes".format(where)) - return TextFrame(description["template"], description["regex"], - fields=cls.convertersFor(description.get("fields", {}), where)) - if "struct" in description: - return BinaryFrame(description["struct"], - fields=tuple(description.get("fields", ())), - constants=cls.constantsFrom(description)) - raise BadDescription( - "{0}: a request needs a template, for text, or a struct, for binary".format(where)) - - @classmethod - def replyFrom(cls, description: dict, where: str) -> Frame: - """Build the reply half: a regex for text, a struct for binary. - - A text reply states both notations, like a text request; a binary one needs - only its struct, since pack and unpack are already each other's inverse. - - Args: - description: the "reply" object - where: how to name this command in an error - - Returns: - A TextFrame or a BinaryFrame, according to the key found. - - Raises: - BadDescription: when neither notation is present, or when a text - reply states an expression without the template that writes it. - ProtocolError: when a struct format states no byte order. - """ - if "regex" in description: - if "template" not in description: - raise BadDescription( - "{0}: a text reply needs a template as well as its regex, " - "so that a mock can write what the driver reads".format(where)) - return TextFrame(description["template"], description["regex"], - fields=cls.convertersFor(description.get("fields", {}), where)) - if "struct" in description: - return BinaryFrame(description["struct"], - fields=tuple(description.get("fields", ())), - constants=cls.constantsFrom(description)) - raise BadDescription( - "{0}: a reply needs a regex, for text, or a struct, for binary".format(where)) - - @classmethod - def constantsFrom(cls, description: dict) -> dict: - """Turn the constants of a binary frame from text in a file into bytes. - - Args: - description: a "request" or "reply" object, whose "constants" is read - if present - - Returns: - {field name: bytes}, empty when the frame fixes nothing. - """ - return {name: cls.bytesFrom(value) - for name, value in description.get("constants", {}).items()} - - @classmethod - def convertersFor(cls, fields: dict, where: str) -> dict: - """Turn {"power": "float"} from a file into {"power": float}. - - Args: - fields: {field name: converter name}, as a file writes it - where: how to name this command in an error - - Returns: - {field name: callable}, in the same order, ready for a TextFrame. - - Raises: - BadDescription: when a converter name is not one this class knows, - listing the ones that do exist. - """ - resolved = {} - for name, converterName in fields.items(): - if converterName not in cls.converters: - raise BadDescription( - "{0}: {1} names the converter {2!r}, which is not one of {3}".format( - where, name, converterName, ", ".join(sorted(cls.converters)))) - resolved[name] = cls.converters[converterName] - return resolved - - @staticmethod - def bytesFrom(text: str) -> bytes: - """Turn a constant written in a file into the byte it stands for. - - Args: - text: one character of a JSON string, such as "M" or "\r" - - Returns: - The bytes, encoded latin-1 so that \xfe stays one byte rather than - becoming the two UTF-8 would spell it with. - """ - return text.encode("latin-1") - - @property - def names(self) -> tuple: - """Every command this device understands. - - Returns: - The names, in the order the description listed them. - """ - return tuple(self.commands) - - def __getitem__(self, name: str) -> Command: - """Look one command up by name. - - Args: - name: the command's name, as the description spells it - - Returns: - The command. - - Raises: - KeyError: naming the device and listing the commands it does have, - since a typo is the likeliest reason to be here. - """ - if name not in self.commands: - raise KeyError("{0} has no command {1!r}; it has {2}".format( - self.deviceName or "this device", name, ", ".join(sorted(self.commands)))) - return self.commands[name] - - def recognize(self, data: Union[bytes, str]) -> Tuple[Command, dict]: - """Work out which command a request belongs to, and what it carried. - - Every command is asked in turn whether the bytes are its request, and the - first that says yes wins -- so a mock dispatches on the same descriptions - the driver sends with, and no prefix table is written twice. - - Args: - data: one complete request. Deciding where a request ends in a stream - is the port's business, exactly as it is on the reply side. - - Returns: - A (command, arguments) pair: the command that recognised the bytes, - and the dict its request decoded to. - - Raises: - RequestDidNotMatch: when no command recognises the bytes, naming the - device and quoting them. - """ - for command in self.commands.values(): - try: - return command, command.decodeRequest(data) - except DidNotMatch: - continue - raise RequestDidNotMatch("{0} has no command matching {1!r}".format( - self.deviceName or "this device", data)) - - def usage(self) -> str: - """Returns every command, what to pass it and what it answers, as text. - - This is what someone needs before writing a single call: the names to give - and the names that come back. It is read off the description, so unlike a - comment or a README it cannot say something the protocol no longer does. - - Commands appear in the order the description listed them, since that order - is usually the one whoever wrote the file thought in. - - Returns: - The whole text, one paragraph per command, ready to print. - """ - lines = ["{0}: {1} commands".format(self.deviceName or "This device", len(self))] - for name in self.names: - lines.extend(self.usageFor(self.commands[name])) - return "\n".join(lines) - - def usageFor(self, command: Command) -> list: - """Explain one command: how to call it, and what comes back. - - Args: - command: the command to describe - - Returns: - Its lines, starting with a blank one so that paragraphs separate when - they are joined. A command is shown as a call with its arguments, - then one line for the answer: the values it carries, or that it - carries none, or that the instrument does not answer at all. - """ - lines = ["", " {0}({1})".format(command.name, self.listed(command.request))] - if not command.expectsReply: - lines.append(" the instrument does not answer") - elif not command.reply.parameters: - lines.append(" answers, carrying no values") - else: - lines.append(" answers {0}".format(self.listed(command.reply))) - return lines - - @staticmethod - def listed(frame: Frame) -> str: - """Set out one frame's values for a usage line. - - Args: - frame: a command's request or its reply - - Returns: - "name: type, name: type" in the order the frame carries them, and - an empty string for a frame that carries none -- which is what makes an - argumentless command print as NAME(). - """ - return ", ".join("{0}: {1}".format(name, typeName) - for name, typeName in frame.parameters) - - def __str__(self) -> str: - """Explain the whole protocol, so that printing a dictionary is enough. - - Returns: - What usage() returns. - """ - return self.usage() - - def __contains__(self, name: str) -> bool: - """Whether the device understands a command of that name. - - Args: - name: a command name to look for - - Returns: - True when it is one of this device's commands. - """ - return name in self.commands - - def __iter__(self) -> Iterator[str]: - """Iterate over the command names, as a dict does. - - Returns: - An iterator over the names, in description order. - """ - return iter(self.commands) - - def __len__(self) -> int: - """How many commands the device understands. - - Returns: - The count, which is also what usage() announces. - """ - return len(self.commands) +from hardwarelibrary.communication.protocol import ( + BadDescription, BinaryFrame, Command, CommandDictionary, DidNotMatch, Frame, + MissingArgument, ProtocolError, ReplyDidNotMatch, RequestDidNotMatch, TextFrame, + booleanFromZeroOrOne, integerFromHexadecimal, +) # --------------------------------------------------------------------------- @@ -1191,24 +26,42 @@ def __len__(self) -> int: # --------------------------------------------------------------------------- class TestTextFrameWritingALine(unittest.TestCase): + def assertReadsBackWhatItWrote(self, frame: TextFrame, **values): + """The regex must match the line the template just produced. + + The two notations are written out separately and nothing holds them + together but this. A template whose own regex does not match what it + writes describes a line no debug port could ever recognise, and the + driver would still work -- which is exactly how such a mistake survives + until someone runs against a mock. + """ + self.assertEqual(frame.decode(frame.encode(**values)), values) + def testBuildsAConstantRequest(self): - self.assertEqual(TextFrame("pa?\r", r"pa\?\r").encode(), b"pa?\r") + request = TextFrame("pa?\r", r"pa\?\r") + self.assertEqual(request.encode(), b"pa?\r") + self.assertReadsBackWhatItWrote(request) def testSubstitutesNamedArguments(self): request = TextFrame("p {power:0.3f}\r", r"p ([0-9.]+)\r", - fields={"power": float}) + fields={"power": float}) self.assertEqual(request.encode(power=0.5), b"p 0.500\r") + self.assertReadsBackWhatItWrote(request, power=0.5) def testWhateverEndsTheLineIsVisibleInTheTemplate(self): - self.assertEqual(TextFrame("*GWL", r"\*GWL").encode(), b"*GWL") - self.assertEqual(TextFrame("g r0xc9\n", "g r0xc9\n").encode(), b"g r0xc9\n") - self.assertEqual(TextFrame("SYST:ERR?\r\n", r"SYST:ERR\?\r\n").encode(), - b"SYST:ERR?\r\n") + for template, regex, written in ( + ("*GWL", r"\*GWL", b"*GWL"), + ("g r0xc9\n", "g r0xc9\n", b"g r0xc9\n"), + ("SYST:ERR?\r\n", r"SYST:ERR\?\r\n", b"SYST:ERR?\r\n")): + request = TextFrame(template, regex) + self.assertEqual(request.encode(), written) + self.assertReadsBackWhatItWrote(request) def testNamesTheArgumentsItNeeds(self): request = TextFrame("s r{register} {value}\r", r"s r(\S+) (-?\d+)\r", - fields={"register": str, "value": int}) + fields={"register": str, "value": int}) self.assertEqual(request.arguments, ("register", "value")) + self.assertReadsBackWhatItWrote(request, register="0x24", value=31) def testAMissingArgumentSaysWhichOne(self): with self.assertRaises(MissingArgument) as raised: @@ -1217,11 +70,22 @@ def testAMissingArgumentSaysWhichOne(self): class TestBinaryFrameWritingAFrame(unittest.TestCase): + def assertReadsBackWhatItWrote(self, frame: BinaryFrame, **values): + """The frame must recognise what it just packed. + + Cheaper to satisfy than its text counterpart, since pack and unpack are + each other's inverse over one format -- but not free: it is the constants + that can be wrong here, and a frame whose own header it refuses is a + frame no debug port would ever recognise. + """ + self.assertEqual(frame.decode(frame.encode(**values)), values) + def testPacksConstantsOnly(self): request = BinaryFrame(" list: + return CommandDictionary.fromDescription( + {"device": "test", "commands": commands}).faults() + + def testARealDescriptionHasNothingToSay(self): + cobolt = CommandDictionary.fromJSON(COBOLT) + self.assertEqual(cobolt.faults(), []) + self.assertIs(cobolt.validate(), cobolt) + self.assertEqual(CommandDictionary.fromJSON(SUTTER).faults(), []) + + def testATemplateAndARegexThatDescribeDifferentLines(self): + # Everything counts out right; they simply are not the same line. Only + # writing one and reading it back can tell. + faults = self.faultsOf({"SET": {"request": { + "template": "p {power:0.3f}\r", "regex": r"q ([0-9.]+)\r", + "fields": {"power": "float"}}}}) + self.assertEqual(len(faults), 1) + self.assertIn("SET request", faults[0]) + self.assertIn("cannot be written and read back", faults[0]) + + def testATemplateAndFieldsThatDisagreeOnAName(self): + faults = self.faultsOf({"SET": {"request": { + "template": "p {power:0.3f}\r", "regex": r"p ([0-9.]+)\r", + "fields": {"powr": "float"}}}}) + self.assertIn("the template writes power and the fields name powr", + "\n".join(faults)) + + def testAnExpressionThatCapturesMoreThanIsNamed(self): + faults = self.faultsOf({"GET": { + "request": {"template": "v?\r", "regex": r"v\?\r"}, + "reply": {"regex": r"(\d+) (\d+)", "template": "{a} {b}\r\n", + "fields": {"a": "integer"}}}}) + self.assertIn("GET reply", faults[0]) + self.assertIn("captures 2 value(s) but 1 field(s) are named", faults[0]) + + def testAStructThatPacksMoreThanItNames(self): + faults = self.faultsOf({"MOVE": {"request": { + "struct": " Date: Wed, 12 Aug 2026 20:44:53 -0400 Subject: [PATCH 17/17] Delete PhysicalDevice.sendCommand, which nothing called Problem: sendCommand looked a command up in self.commands, sent it through self.port, and handed the Command object back so a caller could read the reply off it -- .reply, .matchGroups, .exceptions. That is the pattern the protocol description exists to replace: the reply lands on an object shared by every instance of the driver, and two callers of one command overwrite each other. It also refused to run unless the device was Ready, which is why SutterDevice used to reach around it during initialization to check the stage was answering. Nothing in the library called it. The two tests that did -- testEchoCommands, in its hardware and debug flavours -- iterated EchoDevice.commands and sent each one. Solution: delete it. Those two tests now call the Command's own send() with the device's port, which is all sendCommand ever did once the state check is gone. performTransaction is its successor for a driver that sets a protocol. self.commands stays for now: EchoDevice, CoboltDevice and IntellidriveDevice use theirs to build a TableDrivenDebugPort, and IntegraDevice is the last one that still sends with it. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 9 +++++++++ hardwarelibrary/physicaldevice.py | 16 ---------------- hardwarelibrary/tests/testPhysicalDevice.py | 10 ++++++++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f677e3c..f5c4120a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,15 @@ API changes can land even when the minor version is unchanged. Callers building the debug port themselves should use `ProtocolDebugPort(SutterDevice.protocol)`. +### Removed +- **`PhysicalDevice.sendCommand()`**. It looked a command up in `self.commands`, + sent it through `self.port` and handed the `Command` object back so a caller + could read the reply off it -- the pattern the description above exists to + replace. No driver in the library called it; the two tests that did now send + through the `Command` itself, which is all it ever did. Use + `performTransaction()` with a `protocol`, or a `Command`'s own `send()` while a + device still carries a `commands` dict. + ## [2.1.0] - 2026-08-04 ### Added diff --git a/hardwarelibrary/physicaldevice.py b/hardwarelibrary/physicaldevice.py index 1f3cc91c..580963f0 100644 --- a/hardwarelibrary/physicaldevice.py +++ b/hardwarelibrary/physicaldevice.py @@ -382,22 +382,6 @@ def stopBackgroundStatusUpdates(self): else: raise RuntimeError("No status loop running") - def sendCommand(self, name, **params): - """Look up the named command in self.commands, send it through - self.port, and return the Command object so callers can read - .reply / .matchGroups / .exceptions / .isSentSuccessfully. - - Params are passed through to Command.send: TextCommand uses them - for .format(**params) substitution into text_format; DataCommand - uses them for buildSendData(**params) when sendFormat is set. - """ - if self.state != DeviceState.Ready: - raise PhysicalDevice.NotInitialized - - command = self.commands[name] - command.send(port=self.port, **params) - return command - def performTransaction(self, name, **arguments) -> dict: """Perform one command of self.protocol once, and return what came back. diff --git a/hardwarelibrary/tests/testPhysicalDevice.py b/hardwarelibrary/tests/testPhysicalDevice.py index ad1a0dc9..e23eac67 100644 --- a/hardwarelibrary/tests/testPhysicalDevice.py +++ b/hardwarelibrary/tests/testPhysicalDevice.py @@ -263,7 +263,10 @@ def testEchoCommands(self): self.device.initializeDevice() for name, command in self.device.commands.items(): try: - self.device.sendCommand(name) + # PhysicalDevice.sendCommand used to wrap these two lines and + # nothing else. It is gone; a device that still carries a + # commands dict sends through the Command itself. + command.send(port=self.device.port) except Exception as err: self.fail("Unable to send command {0} to device {1}: {2}".format(name, self.device, err)) self.device.shutdownDevice() @@ -277,7 +280,10 @@ def testEchoCommands(self): self.device.initializeDevice() for name, command in self.device.commands.items(): try: - self.device.sendCommand(name) + # PhysicalDevice.sendCommand used to wrap these two lines and + # nothing else. It is gone; a device that still carries a + # commands dict sends through the Command itself. + command.send(port=self.device.port) except Exception as err: self.fail("Unable to send command {0} to device {1}: {2}".format(name, self.device, err)) self.device.shutdownDevice()