Uniform do* hooks and will/did notifications across every capability - #117
Merged
Conversation
Problem: the README's supported-hardware table listed 15 devices while the tree holds 20 drivers. VerdiGDevice, FieldMasterDevice, SR830Device, PwrUSBDevice and StellarNet were absent, the Millennia was listed without its USB identity, and the class-hierarchy diagram was stale in the same way (it also showed ThorlabsKinesisDevice as a sibling of ThorlabsDevice rather than its subclass, and credited turnOn/setPower to the LaserSourceDevice marker base). Nothing explained the capability mixins, which are the one design decision a reader meets everywhere in the library. Solution: complete the table from the drivers actually present, with a note on the devices that share the generic FTDI 0403:6001 identity and must be disambiguated by serialNumber or portPath. Add a "Capabilities" section that builds the idea from first principles for a reader with no OO background: the concrete problem (the Millennia has a shutter, the Cobolt does not), why a fat base class or a class-per-combination both fail, the Capability/Device naming convention, the public-method/do-hook split, and runtime introspection with capabilities()/hasCapability(). Refresh the hierarchy diagram, and add usage examples for the power strip and the other lasers, since the README promises one per category. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: the template method pattern was not uniform. The DAQ, lock-in and trigger capabilities declared their public method itself as the @AbstractMethod, so a driver implemented getAnalogVoltage() directly and the library had no concrete method of its own to hang argument validation, notifications or error handling on. Spectrometer had the same shape for getSpectrum/getSerialNumber. Nothing enumerated the capabilities either, so the only way to know what the library can express was to read the module. Solution: every public method of every capability is now concrete and delegates to a do* hook, and only the hook is abstract, in all nine DAQ / lock-in / trigger mixins and in Spectrometer. Hooks that are optional or that default to a composition of the others (doAcquireWaveform, doGetDemodulatedValues, doGetSupported*, doConfigure*, do*Direction) stay concrete but keep the do prefix. Public methods that carried driver-specific parameters forward them: configureStream(channels, sampleRate=None, **parameters) and getSpectrum(**parameters), so the SR830's sampleClock, the LabJack's deprecated scanRate and the Ocean Insight integrationTime still reach their driver. No public name or signature changed, so callers are unaffected; LabjackDevice, SR830Device and OISpectrometer were migrated to the hooks. Add allCapabilities() and capabilityInterface() to enumerate the mixins and describe each one as extends/publicAPI/hooks, plus a `python -m hardwarelibrary --capabilities` listing built on them, so the inventory is never maintained by hand. tests/testCapabilities.py covers both and enforces the invariants: no public method on a capability is abstract, no PhysicalDevice subclass declares an abstract method outside its do* hooks, and no capability is declared outside capabilities.py. HOPSInterface is exempt from the first two: it is a transport strategy behind VerdiGDevice, closer to CommunicationPort than to a device API. Note for the licenced StellarNet driver, which ships encrypted and is not in this repository: rename its getSpectrum/getSerialNumber to doGetSpectrum/doGetSerialNumber, or StellarNet() raises TypeError for the missing hooks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: only three family bases posted notifications (LinearMotionDevice, PowerMeterDevice, CameraDevice), and they did it by hand. A capability operation went by silently, so a GUI or a logger had to poll or wrap every driver itself, and a hook that raised told no one. The one notification enum that did exist for a capability, AnalogInputStreamCapability.Notification, was never posted at all. Solution: every capability now names a notification enum in its `notification` attribute, and every public method is wrapped in the new @notifies decorator, so a driver gets notifications by implementing hooks and writing no notification code. One decorator line per method rather than six lines of hand-written try/post/post, which is what keeps the pattern from eroding across 67 hooks. An operation that changes the instrument posts will<Stem> then did<Stem>; a read (doGet*, doReadStream) posts only did<Stem>, since bracketing a value that is merely being read doubles the traffic on hot paths like a voltage sampled in a loop. Spectrometer.getSpectrum is the one read that keeps a will, because an acquisition takes an integration time and a display wants to know it started. Measured overhead on a read is ~1.6 us with no observer, ~2.0 us with one, against millisecond-scale device I/O. did<Stem> is posted whether the operation succeeded or not, so a will is always followed by its did and an observer never has to pair two different members to know an operation ended. user_info carries the method's arguments by name plus "result" and "error", exactly one of which is non-None: the observer decides what to do from the error, while the exception itself is re-raised untouched, because a driver's exception type is part of its contract (SR830Device raises ValueError for an out-of-range Aux voltage, and callers rely on it). Capabilities related by inheritance share one enum, so `notification` is the same object on all of them and their members are interchangeable: AnalogInput/AnalogOutput/AnalogIO/AnalogInputStream all post AnalogNotification, and the digital trio posts DigitalNotification (17 enums for 22 capabilities). Sharing is what makes this work at all, because notifcenter keys observers by enum member identity: two same-named members of two enums would never cross-fire, and an observer would otherwise have to know whether a device mixed in the combined capability or the plain one. testCapabilities.py enforces the scheme: every hook has its did member, only non-reads have a will, no member exists without a hook behind it, no enum carries a separate failure member, capabilities in one chain share an enum, and the posted sequences and payloads are checked on success and on failure. `python -m hardwarelibrary --capabilities` now lists each capability's enum. Also fixes the README's "Listening for device events" example, which still used the pre-migration camelCase notifcenter API (addObserver, userInfo) and would have failed for anyone who copied it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: LinearMotionDevice, RotationDevice and PowerMeterDevice posted notifications by hand, from before the capability scheme existed, and drifted from it in three ways. PowerMeterNotification.didMeasure was named after its public method rather than its hook (doGetAbsolutePower), unlike every capability member. The payloads were bare values -- a position tuple, an angle, a float -- where notifcenter documents user_info as a dict and every capability now passes one. And none of them said anything when a driver raised, so an observer saw a willMove with no matching didMove and could not tell a slow move from a failed one. Solution: all three now go through @notifies, so they follow one rule with the capabilities and report failures for free. PowerMeterNotification. didMeasure becomes didGetAbsolutePower, named after its hook. user_info is a dict of the method's arguments by name plus "result" and "error", so a handler reads user_info["position"] where it used to unpack the notification payload directly. Each base names its enum in a `notification` attribute, as the capabilities do; Spectrometer gained the attribute it was missing when its enum was added. Motion keeps its grouped willMove/didMove rather than splitting into willMoveTo/willMoveBy/willHome: moveTo, moveBy and home are all "the stage is moving" to an observer, and the payload already tells them apart, carrying a position, a displacement, or neither. RotationMotionNotification gets the same treatment, having had the identical shape before. testCapabilities.py grows a guard for the family bases: their members are named after their hooks (loosely, since a base may group several hooks under one name), a read still posts did only, and every base that notifies is covered by those checks. That guard is what caught the missing Spectrometer.notification. CameraDeviceNotification is deliberately left alone: a capture session is a different shape, with imageCaptured firing per frame and start/stop bracketing a thread rather than one hook call. Also fixes the notification example in the project skill file, which used the pre-migration import path and the camelCase addObserver/userInfo API. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: calling an operation before initializeDevice() failed in two different unhelpful ways. On real hardware it died deep inside the driver with AttributeError: 'NoneType' object has no attribute 'writeString', naming a port rather than the mistake. On a debug device it was worse: the call succeeded, DebugMillenniaDevice().turnOn() reported the laser on while the device was still Unconfigured, and it posted a didTurnOn with error None, telling every observer the hardware had done something it had not. PhysicalDevice.NotInitialized existed for exactly this and was raised in one place, sendCommand. Solution: @notifies calls validateReady() before anything else, so all 67 capability operations plus the family bases are guarded without a driver writing a line. The error names the operation, the class and the state: "Cannot turnOn() on DebugMillenniaEv25Device: the device is Unconfigured, not Ready. Call initializeDevice() first." A device shut down again is refused too, since its state returns to Recognized. The check runs before the will is posted, not inside the try: nothing was attempted and the hardware was never touched, so an observer hears nothing at all rather than a will followed by a did carrying the error. PhysicalDevice.validateReady is the real check and sits ahead of Capability.validateReady, a no-op, in a driver's MRO. That is what lets a capability still be exercised bare, as the tests do with a mixin that has no device state, without weakening the check for anything that is a PhysicalDevice. Methods that only report what a model supports are exempt through requiresReady=False -- supportedInputSources, supportedSensitivities, supportedTimeConstants, supportedTriggerSources and outletCount all read class attributes, and a UI populates its menus before connecting. testCapabilities.py covers the guard, including that nothing is posted when it refuses, that a shut-down device is refused again, and that the exempt methods answer while Unconfigured. It also walks every capability's public API rather than naming methods, so a capability added later cannot escape the guard unnoticed. MockOISpectrometer, which bypasses PhysicalDevice.__init__ on purpose, now declares the Ready state it stands for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: validateReady was defined twice, as a no-op on Capability and for real on PhysicalDevice, with the MRO picking the right one. The no-op was never reached by any driver in the library -- PhysicalDevice sits at MRO index 2 and Capability at index 10, so it was shadowed everywhere it mattered. It only ran for a capability mixed into something that is not a device, and there it silently skipped the check. That also put a device lifecycle concept in capabilities.py, which has no business defining one. Solution: drop the fallback and keep the single definition on PhysicalDevice, where the state machine lives; capabilities.py only calls it. Anything hosting a capability without being a PhysicalDevice now has to answer for readiness itself, which is the honest contract: every capability's docstring already says to combine it with a PhysicalDevice. The three test stubs that stand in for a device -- _RecordingAnalogDevice and _FailingAnalogDevice, and _MinimalLockIn in testSR830 -- supply it, as they already supply the hooks. The trade-off is the failure mode for a capability mixed into a non-device: previously no validation at all, now an AttributeError naming validateReady. Note that delegating from Capability.validateReady to PhysicalDevice's would not have worked: it is unreachable from any driver, and on a bare mixin it raises AttributeError on the missing state attribute. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: a public method passed whatever it was given straight to the driver,
so several calls were accepted and did nothing a caller would expect.
acquireWaveform(sampleCount=0) returned an empty acquisition, sampleRate=-100
was accepted, setDigitalValue("yes") set the line True, setSensitivity(-1)
and setTimeConstant(0) were silently snapped to a step, and
matisse.setWavelength(50.0) was sent to the birefringent-filter motor even
though the driver reports a 700-1000 nm range two methods below. An empty
channel list failed inside the drain loop with "min() iterable argument is
empty", naming nothing the caller had written. Two ad-hoc private validators
had grown in the drivers meanwhile, one in PwrUSBDevice and one in
LabjackDevice.
Solution: hardwarelibrary/validation.py collects the shared checks --
requireRealNumber, requireInteger, requireBool, requireAtLeast,
requirePositive, requireWithinRange, requireNonEmpty, requireMember -- each
raising TypeError for the wrong kind of value and ValueError for one out of
bounds, and each naming the parameter, the value and the bound it broke. A
capability declares a _validateXxx method and hands it to @notifies as
validate=, which runs it before anything is posted: a refused call announces
nothing, so a will/did pair still means the driver was invoked.
The line between the two levels is deliberate. Contract-level checks, true of
every device implementing the capability, live in the capability: a sample
count of at least one, a positive time constant, a wavelength inside the
range the driver itself reports. Limits that vary by model stay in the
driver, next to the numbers they come from -- the SR830's +/-10.5 V Aux
output and the Millennia's 0.05-25 W are untouched. Channel identity is
checked by neither: an SR830 addresses its outputs with AuxOutput members
where a LabJack uses bare ints, so there is no shared rule to write.
A validator calls do* hooks rather than public methods, so validating never
posts a notification of its own: the wavelength check reads
doGetWavelengthRange(), the outlet check doGetOutletCount(). That last one
subsumes PwrUSBDevice._validateOutlet, which is removed, so every future
strip inherits the bounds instead of reimplementing them.
setInputSource and setTriggerSource now accept anything their enum accepts
and hand the driver a member, so setInputSource("Differential") works.
No dependency was added. pydantic, beartype and icontract were considered:
none can express the device-dependent bounds, which are half the checks here,
and each raises its own exception type where the drivers and their tests
rely on ValueError.
One test changed meaning: the SR830 external-clock case passed sampleRate=0
to mean "the rate is not mine", where the documented spelling is None. It now
says None.
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
1 participant
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.
Three related changes: one rule for how a public method reaches a driver, one rule for what it announces, and the documentation and introspection needed to keep both from eroding.
1.
getXxx()callsdoGetXxx(), everywhereThe DAQ, lock-in and trigger capabilities declared their public method itself as the
@abstractmethod, so a driver implementedgetAnalogVoltage()directly and the library had no concrete method of its own to hang validation, notifications or error handling on.Spectrometerhad the same shape.Now every public method is concrete and delegates; only the
do*hook is abstract. Hooks that are optional or that default to a composition of the others (doAcquireWaveform,doGetDemodulatedValues,doGetSupported*) stay concrete but keep the prefix.Callers are unaffected — every public name and signature is unchanged. Driver authors rename their implementations to the hook; a missed one fails loudly at instantiation with
TypeErrornaming it.LabjackDevice,SR830DeviceandOISpectrometerwere migrated.Driver-specific parameters would have been amputated by a fixed public signature, so
configureStream(channels, sampleRate=None, **parameters)andgetSpectrum(**parameters)forward extras to the hook — the SR830'ssampleClock, the LabJack's deprecatedscanRateand the Ocean InsightintegrationTimeall still arrive.2. will/did notifications on every capability
Only three family bases posted notifications, by hand. A capability operation went by silently, and a hook that raised told no one.
Every capability now names an enum in its
notificationattribute, and every public method is wrapped in the new@notifiesdecorator — one line per method instead of six lines of hand-written try/post/post, which is what keeps the pattern intact across 67 hooks.will<Stem>thendid<Stem>. A read postsdidonly, so a voltage sampled in a loop costs one notification, not two.Spectrometer.getSpectrumis the one read that keeps awill, since an acquisition takes an integration time.did<Stem>is posted whether the operation succeeded or not, so awillis always followed by itsdid.user_infocarries the arguments by name plus"result"and"error", exactly one of which is set. The exception is re-raised untouched —SR830DeviceraisesValueErrorfor an out-of-range Aux voltage and callers rely on it.AnalogIOCapability.notification is AnalogOutputCapability.notification. This is load-bearing:notifcenterkeys observers by member identity, so without sharing an observer would have to know whether a device mixed in the combined capability or the plain one.3. Family bases brought into the same scheme
LinearMotionDevice,RotationDeviceandPowerMeterDevicenow go through the same decorator.PowerMeterNotification.didMeasurebecomesdidGetAbsolutePower, named after its hook. Motion keeps its groupedwillMove/didMove—moveTo,moveByandhomeare all "the stage is moving" to an observer, and the payload tells them apart.CameraDeviceNotificationis deliberately untouched: a capture session is a different shape, withimageCapturedfiring per frame and start/stop bracketing a thread rather than one hook call.Introspection and docs
allCapabilities()andcapabilityInterface()enumerate the mixins and describe each as extends/publicAPI/hooks.python -m hardwarelibrary --capabilitiesprints every capability with its API, its hooks and the notifications it posts, so the inventory is never maintained by hand.VerdiGDevice,FieldMasterDevice,SR830Device,PwrUSBDevice,StellarNet) and the Millennia's USB identity; the class-hierarchy diagram was stale in the same way. Added a "Capabilities" section explaining the mixin pattern from first principles.addObserver/userInfocamelCase API and would have failed for anyone who copied them.Tests
hardwarelibrary/tests/testCapabilities.py(new, 37 tests) enforces the invariants rather than describing them: no public method on a capability is abstract; noPhysicalDevicesubclass declares an abstract method outside itsdo*hooks; every hook has itsdidmember; only non-reads have awill; no member exists without a hook behind it; capabilities in one inheritance chain share an enum; and the posted sequences and payloads are checked on both the success and the failure path. That last guard is what caught a missingSpectrometer.notificationduring review.Full suite: 522 passed, 249 skipped.
Breaking changes
Callers of the public API are unaffected. Observers and driver authors are not:
do*hook.PowerMeterNotification.didMeasureis nowdidGetAbsolutePower.LinearMotionNotification/RotationMotionNotificationkeepwillMove/didMove, butuser_infois now a dict: readuser_info["position"]where the payload used to be the position tuple.getSpectrum/getSerialNumbertodoGetSpectrum/doGetSerialNumber, orStellarNet()raisesTypeErrorfor the missing hooks.All recorded in
CHANGELOG.mdunder[Unreleased].🤖 Generated with Claude Code
https://claude.ai/code/session_01YB8L3EykgRP2HxXx5qknMB