Three more drivers on the protocol description, and a format that stays about the wire - #119
Open
dccote wants to merge 3 commits into
Open
Three more drivers on the protocol description, and a format that stays about the wire#119dccote wants to merge 3 commits into
dccote wants to merge 3 commits into
Conversation
The module docstring began with the sans-I/O principle and the mechanics that follow from it, which answers "how does this work" before "why would I want it". It now opens with the purpose: describe a device's protocol generally enough to build a debug port that tests it without hardware, and to keep the details of sending out of the driver. The sans-I/O property is still there, stated as what it is -- a protocol formats bytes and never performs the send or the read. Prose by DCC; two typos fixed in passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: two more drivers built their bytes with the commands dict. Each hid a
different fault behind it.
EchoDevice's three commands were timing out, every time, and saying nothing. An
echo answers with the payload and no terminator, so reading a line ran to the end
of the buffer and raised -- and Command.send catches every exception into an
attribute and returns, so testEchoCommands passed while nothing worked. The test
could not fail.
IntegraDevice had no debug path at all: it opened a USBPort unconditionally, so
all twelve of its tests skipped unless the meter was plugged in. Nobody without
the hardware could change that driver and know.
Solution: both keep their protocol as data and send through performTransaction.
An echo is described as fixed-size frames whose every byte is a constant, which
is both the truthful shape -- there is no terminator to read up to -- and a
stronger claim: a constant is required on the way in, so a payload that comes
back altered is refused rather than merely received. testEchoCommands now calls
performTransaction and needs no try/fail wrapper, since a failure raises.
IntegraDevice("debug") builds a ProtocolDebugPort, and five tests now run without
the meter: the description validated against itself, the usage text, and setting
a wavelength then reading it back, which goes out as "*PWC00532" exactly as
before. One command did not survive: "*STS" answers with a run of lines until one
matches ":100000000", and a reply of an unknown number of lines is the one shape
the description cannot state. Nothing ever called it, and the module says where
it goes when multi-line replies exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: a command could state a "sets" clause -- what receiving it does to the instrument's state, for what its bytes do not carry -- so that a debug port needed no code. Two clauses of that kind had appeared: "sets" per command, and an "initial" for what an instrument reads as when switched on. Both describe the instrument rather than the wire, in a format whose whole premise is that it describes the wire, and both were about to be paid for by every description. Measured, they were not worth it. Neutralising "sets" broke one pre-existing test; the other two failures were the tests written to justify it. Neutralising "initial" broke two. All three belong to Cobolt -- the one device that already carries a debug-port subclass, because a laser does not reach a new power at once and doSetPower's convergence loop needs something to converge to. Solution: no such clauses. A description is the wire again. CoboltDevice keeps its power-up state and its four state-changing commands in the subclass it already had, next to the ramp, which is where behaviour lives. SutterDevice gains a four-line subclass so HOME still sends the stage to the origin -- no test required it, but a debug port that lies about homing is exactly what the rest of this design refuses. EchoDevice and IntegraDevice need no subclass at all. The description also corrects the old mock: it never updated requestedPower, so GET_REQUESTED_POWER answered 0 forever. On a Cobolt, "p" sets the power asked for and "p?" reads it back, while "pa?" reads the power reached. Naming that field requestedPower rather than power keeps the two quantities apart, and the ramp between them means something again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
EchoDevice,IntegraDeviceandCoboltDevicenow keep their protocol as data and send throughperformTransaction, asSutterDevicedoes.communication/commands.pyhas one user left,IntellidriveDevice.Converting them turned up a fault in each, which is most of the value here.
What the conversions found
EchoDevice's three commands were timing out, every time, silently. An echo answers with the payload and no terminator, so reading a line ran to the end of the buffer and raised — and the old
send()catches every exception into an attribute and returns, sotestEchoCommandspassed while nothing worked. That test could not fail. Described as fixed-size frames whose every byte is a constant, the echo is now checked: a payload that comes back altered is refused, not merely received.IntegraDevice had no debug path at all. It opened a
USBPortunconditionally, so all twelve of its tests skipped unless the meter was plugged in — nobody without the hardware could change that driver and know.IntegraDevice("debug")now builds aProtocolDebugPort, and five tests run without it: the description validated against itself, the usage text, and setting a wavelength then reading it back, which goes out as*PWC00532exactly as before.CoboltDevice's debug port never updated
requestedPower, soGET_REQUESTED_POWERanswered 0 forever. On a Cobolt,psets the power asked for andp?reads it back, whilepa?reads the power reached. The description names those two apart, so both read correctly and the mock's power ramp means something again.The format stays about the wire
A command could state a
"sets"clause — what receiving it does to the instrument, for what its bytes do not carry — so that a debug port needed no code. A second clause,"initial", was about to join it for what an instrument reads as when switched on.Both describe the instrument rather than its protocol, in a format whose whole premise is that it describes the protocol. Measured, they were not worth it:
"sets""initial"All three belong to Cobolt, the one device that already carries a debug-port subclass, because a laser does not reach a new power at once and
doSetPower's convergence loop needs something to converge to.So neither clause exists. Cobolt keeps its power-up state and its four state-changing commands in that subclass, next to the ramp, where behaviour lives.
SutterDevicegains a four-line subclass soHOMEstill sends the stage to the origin — no test required it, but a debug port that lies about homing is what the rest of this design refuses. Echo and Integra need no subclass at all.Known and deliberate
A
ProtocolDebugPortanswers0for a reading no request ever sets — a measured power, a serial number. Those turn out to be, almost exactly, the properties the capabilities already declare:isOn,power,interlock,autostart,serialNumberpower,versionMaking a debug port fulfil declared properties rather than invent zeros is worth doing, and it is a design conversation of its own — the properties belong to the capabilities, not to the protocol, and identity (
serialNumber,version) is neither. Left for a separate piece of work rather than smuggled in here.IntellidriveDeviceis not converted. Itsdo*methods bypass its own command dict and write literals, and its template sayst {mode}\nwhere the driver sendst 1\r; converting it means changing a byte sent to hardware I cannot test.Testing
666 passed, 250 skipped. Net +4 tests that run without hardware, on drivers that previously had 0 (Integra) or a test that could not fail (Echo).
🤖 Generated with Claude Code