From 7bfc0399023a64caa94f6bf46c89084fdd359099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Tue, 4 Aug 2026 12:46:57 -0400 Subject: [PATCH 1/7] Document every supported device and explain the capability mixins 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) --- README.md | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 190 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c12cfa2f..11768b93 100644 --- a/README.md +++ b/README.md @@ -39,18 +39,151 @@ The library currently supports the following hardware: | | Ocean Insight USB4000 | `USB4000` | `0x2457:0x1022` | USB (PyUSB) | | | Ocean Insight USB650 | `USB650` | `0x2457:0x1014` | USB (PyUSB) | | | Ocean Insight SAS | `SAS` | `0x2457:0x1006` | USB (PyUSB) | +| | StellarNet | `StellarNet` | `0x0BD7:0xA012` | USB (licensed module, see note) | | **Motion (linear)** | Sutter MP-285 | `SutterDevice` | `0x1342:0x0001` | Serial (FTDI) | | | Thorlabs (Kinesis) | `ThorlabsDevice` | `0x0403:0xFAF0` | Kinesis (pylablib) | | **Motion (rotation)** | Intellidrive | `IntellidriveDevice` | `0x0403:0x6001` | Serial (FTDI) | | **Laser sources** | Cobolt laser | `CoboltDevice` | (serial) | Serial | -| | Spectra-Physics Millennia eV | `MillenniaEv25Device` | (serial) | Serial | +| | Spectra-Physics Millennia eV | `MillenniaEv25Device` (alias `MillenniaDevice`) | `0x0483:0x5740` | Serial (STM32 USB-CDC) | +| | Coherent Verdi G / Genesis (HOPS supply) | `VerdiGDevice` | `0x0403:0x6010` | I2C over FTDI (pyftdi) or `CohrHOPS.dll` | | | Sirah Matisse | `MatisseDevice` | (TCP) | TCP/IP | | **Power meters** | Gentec-EO Integra | `IntegraDevice` | `0x1AD5:0x0300` | USB (PyUSB) | +| | Coherent FieldMaster GS | `FieldMasterDevice` | `0x0403:0x6001` | Serial (FTDI) | | **DAQ** | LabJack U3 | `LabjackDevice` | `0x0CD5:0x0003` | USB (LabJackPython) | +| | SRS SR830 lock-in amplifier | `SR830Device` | `0x0403:0x6001` | GPIB via Prologix adaptor (serial) | | **Oscilloscopes** | Tektronix TDS series | `OscilloscopeDevice` | `0x0403:0x6001` | Serial (FTDI/SCPI) | +| **Power strips** | PwrUSB switched power strip | `PwrUSBDevice` | `0x04D8:0x003F` | USB HID (hidapi) | | **Cameras** | Any OpenCV camera | `OpenCVCamera` | (OS driver) | OpenCV | -Every device above also has a debug/simulated counterpart (e.g., `DebugLinearMotionDevice`, `DebugSpectro`) that works without hardware, useful for development and testing. +Several devices have no USB identity of their own and connect through a generic FTDI RS-232 adaptor (`0x0403:0x6001`). When more than one such adaptor is plugged in, disambiguate with the adaptor's `serialNumber` or by passing an explicit `portPath`. + +The StellarNet driver ships encrypted and must be licenced and decrypted by StellarNet; run `python -m hardwarelibrary --stellar` and enter the password to unlock it. + +Every device above also has a debug/simulated counterpart (e.g., `DebugLinearMotionDevice`, `DebugMillenniaDevice`, `DebugSR830Device`, `DebugSpectro`) that works without hardware, useful for development and testing. + +## Capabilities: what a device can actually do + +Before using the library, it helps to understand one design decision that shows up everywhere: devices are described by *what they can do*, not only by *what they are*. This section explains the idea from scratch; no prior knowledge of object-oriented jargon is assumed. + +### The problem + +Look at two lasers from the table above. The Spectra-Physics Millennia can be turned on and off, has a mechanical shutter, and lets you set its output power. The Cobolt can also be turned on and off and lets you set its power, but it has **no shutter** — and it does have an interlock and an "autostart" mode that the Millennia does not expose. Both are lasers, yet neither is a subset of the other. Motion stages, power meters and DAQ cards are the same story: every model implements a slightly different mix of features. + +So how do we write one common interface? Two obvious answers are both bad: + +* **One big `Laser` class containing every method any laser might have.** Then `CoboltDevice` inherits an `openShutter()` it cannot honour. Your code can call it, the editor will autocomplete it, and you only discover the truth at runtime when it raises an error — in the middle of an experiment. +* **One class per combination of features** (`LaserWithShutter`, `LaserWithShutterAndInterlock`, ...). The number of classes explodes and nothing is reusable. + +### The solution: small classes, one skill each + +We use a third approach. Each individual skill gets its own small class, called a **capability**: + +* `OnOffCapability` — knows about `turnOn()`, `turnOff()`, `isLaserOn()` +* `ShutterCapability` — knows about `openShutter()`, `closeShutter()`, `isShutterOpen()` +* `PowerCapability` — knows about `setPower()`, `power()` +* `InterlockCapability`, `AutostartCapability`, `WavelengthCapability`, ... + +A capability is **not** a device: you can never create one on its own, it has no port, no serial number, and it cannot talk to anything. It is a small, reusable bundle of methods, meant to be *mixed into* a real device class. That is why such a class is traditionally called a **mixin**. + +In Python a class may inherit from several classes at once, and this is exactly what a driver does. It says "I am a physical device, and I happen to have these skills": + +```python +class MillenniaEv25Device(LaserSourceDevice, OnOffCapability, ShutterCapability, PowerCapability): + ... + +class CoboltDevice(LaserSourceDevice, OnOffCapability, PowerCapability, + InterlockCapability, AutostartCapability): + ... +``` + +Read those two lines as sentences. They *are* the specification of each laser: the Millennia has on/off, a shutter and power control; the Cobolt has on/off, power, an interlock and autostart. There is no `openShutter()` on the Cobolt at all — calling it raises `AttributeError` immediately, instead of pretending to work. The list of parent classes is the documentation, and it cannot go out of date, because it is the code. + +Two conventions make this readable at a glance: + +* a class whose name ends in **`Capability`** is a mixin: a skill, never instantiated by itself; +* a class whose name ends in **`Device`** is real hardware you can create, connect to, and use. + +### Who writes what: public methods and `do` hooks + +Each capability provides the *public* method that you, the user, call — and it delegates the actual hardware work to a companion method whose name starts with `do`, which the driver author must write. For instance `OnOffCapability` provides `turnOn()` and requires `doTurnOn()`: + +```python +class MillenniaEv25Device(LaserSourceDevice, OnOffCapability, ShutterCapability, PowerCapability): + def doTurnOn(self): + self.writeActionAndConfirm("ON", "?D", "1", "diodes on") # what this laser expects +``` + +The benefit is a clean division of labour. The public method is written once and is the same for every laser, so it is the name your scripts should use; the driver author only supplies the few lines that are genuinely model-specific. It also gives the library a single place to put behaviour shared by all models, whenever there is any: `LinearMotionDevice.moveTo()`, for instance, posts a `willMove` notification, calls `doMoveTo()`, then posts `didMove`, so any GUI or logger watching the stage is informed without the driver author writing a line for it (see [Listening for device events](#listening-for-device-events)). Most laser capabilities have nothing to add and simply forward to the `do` method. + +Better still, the `do` methods are declared *abstract*, which is Python's way of saying "a subclass must provide this". If you declare `ShutterCapability` on your new driver but forget `doOpenShutter()`, Python refuses to even create the object and tells you which method is missing: + +``` +TypeError: Can't instantiate abstract class MyLaser without an implementation for abstract method 'doOpenShutter' +``` + +That is a mistake caught the first time you run your code, not the day you are aligning an experiment. + +This split is uniform across the whole library: every public method of every capability is concrete and delegates, and only the `do` hooks are abstract. There are no exceptions to remember, and a test enforces it, so the rule cannot quietly erode as drivers are added. + +### Asking a device what it can do + +Because the capabilities are ordinary classes, a device can be asked about them at runtime. Every `PhysicalDevice` offers two methods: + +```python +from hardwarelibrary.sources import DebugMillenniaDevice, CoboltDevice, ShutterCapability + +laser = DebugMillenniaDevice() +print([c.__name__ for c in laser.capabilities()]) +# ['OnOffCapability', 'ShutterCapability', 'PowerCapability'] + +laser.hasCapability(ShutterCapability) # True +CoboltDevice().hasCapability(ShutterCapability) # False +``` + +This is what lets you write code that adapts to whatever is on the bench, rather than code that only works with one model: + +```python +if laser.hasCapability(ShutterCapability): + laser.closeShutter() +else: + laser.turnOff() +``` + +A GUI can use the very same trick to decide which buttons to display, and a generic acquisition script can decide whether it is allowed to block the beam without shutting the laser down. + +### Where they live + +All capabilities are defined in the single module `hardwarelibrary/capabilities.py`, and they are re-exported by the family they belong to, so you can import them from where you already import the device (`from hardwarelibrary.sources import ShutterCapability`). Capabilities exist for every family, not just lasers: + +| Family | Typical capabilities | +|---|---| +| Laser sources | `OnOffCapability`, `ShutterCapability`, `PowerCapability`, `InterlockCapability`, `AutostartCapability`, `WavelengthCapability`, `DispersionCapability` | +| DAQ | `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability`, `AnalogInputStreamCapability`, `DigitalInputCapability`, `DigitalOutputCapability`, `DigitalIOCapability`, `PhaseLockedDetectionCapability`, `TriggerCapability` | +| Power meters | `WavelengthCalibrationCapability`, `AutoScaleCapability`, `ScaleCapability` | +| Power strips | `OutletSwitchingCapability`, `DefaultOutletCapability`, `CurrentMeteringCapability` | + +A capability may also be built out of others: `AnalogIOCapability` is simply `AnalogInputCapability` plus `AnalogOutputCapability`, which is why `LabjackDevice` declares the combined one and gets both sets of methods. + +You never have to trust a list in a document to be current: ask the library itself. + +```shell +python -m hardwarelibrary --capabilities +``` + +This prints every capability, the capabilities it extends, the public methods it defines, and the `do` hooks a driver must implement: + +``` +OnOffCapability + isLaserOn() -> bool + turnOn() + turnOff() + canTurnOn() -> bool + - hook: doTurnOn() + - hook: doTurnOff() + - hook: doGetOnOffState() -> bool +``` + +The same information is available from Python through `allCapabilities()` and `capabilityInterface()` in `hardwarelibrary/capabilities.py`. ## Getting started with using devices @@ -167,6 +300,26 @@ laser.turnOff() laser.shutdownDevice() ``` +The other lasers use the same methods, minus or plus the ones their capabilities declare. A Millennia or a Verdi adds a shutter, and the Matisse is tuned by wavelength: + +```python +from hardwarelibrary.sources import MillenniaDevice, VerdiGDevice, MatisseDevice + +pump = MillenniaDevice() # or VerdiGDevice() +pump.initializeDevice() +pump.turnOn() +pump.setPower(5.0) # watts +pump.openShutter() # not available on the Cobolt +pump.closeShutter() +pump.shutdownDevice() + +matisse = MatisseDevice(host="192.168.1.10") +matisse.initializeDevice() +matisse.setWavelength(780.0) # nm +print(matisse.wavelength()) +matisse.shutdownDevice() +``` + ### Power meters (Gentec-EO Integra) ```python @@ -212,6 +365,22 @@ waveform = scope.getWaveform(channel="CH1") # list of (time, voltage) scope.shutdownDevice() ``` +### Power strips (PwrUSB) + +```python +from hardwarelibrary.powerstrips import PwrUSBDevice + +strip = PwrUSBDevice() +strip.initializeDevice() + +strip.turnOutletOn(1) # outlets are 1-based, as labelled +strip.turnOutletOff(2) +print(strip.isOutletOn(1)) # True +print(strip.current()) # amperes drawn by the whole strip + +strip.shutdownDevice() +``` + ### Cameras (OpenCV) ```python @@ -289,30 +458,39 @@ This is essential for writing and running tests on machines where the physical d ### Class hierarchy -All devices inherit from `PhysicalDevice`, which provides lifecycle management (initialize/shutdown), state tracking, background monitoring, and notification support. Intermediate classes define category-specific interfaces: +All devices inherit from `PhysicalDevice`, which provides lifecycle management (initialize/shutdown), state tracking, background monitoring, and notification support. Intermediate classes define category-specific interfaces, and the capability mixins described in [Capabilities](#capabilities-what-a-device-can-actually-do) add the per-model features on top: ``` PhysicalDevice ├── LinearMotionDevice ──── moveTo(), moveBy(), position(), home() │ ├── SutterDevice │ ├── ThorlabsDevice -│ ├── ThorlabsKinesisDevice +│ │ └── ThorlabsKinesisDevice │ └── DebugLinearMotionDevice ├── RotationDevice ───────── moveTo(), moveBy(), orientation(), home() │ └── IntellidriveDevice -├── LaserSourceDevice ────── turnOn(), turnOff(), setPower(), power() -│ └── CoboltDevice +├── LaserSourceDevice ────── marker base; the methods come from the capabilities +│ ├── CoboltDevice ─────── OnOff, Power, Interlock, Autostart +│ ├── MillenniaEv25Device OnOff, Shutter, Power +│ └── VerdiGDevice ─────── OnOff, Shutter, Power, Interlock +├── MatisseDevice ────────── Wavelength (a laser, but wired as a PhysicalDevice) ├── PowerMeterDevice ─────── measureAbsolutePower(), setCalibrationWavelength() -│ └── IntegraDevice +│ ├── IntegraDevice +│ └── FieldMasterDevice ├── Spectrometer ─────────── getSpectrum(), setIntegrationTime(), display() -│ └── OISpectrometer -│ ├── USB2000 / USB2000Plus -│ ├── USB4000 -│ └── USB650 +│ ├── OISpectrometer +│ │ ├── USB2000 / USB2000Plus +│ │ ├── USB4000 +│ │ ├── USB650 +│ │ └── SAS +│ └── StellarNet (licenced module, not distributed) ├── OscilloscopeDevice ───── getWaveform(), displayWaveforms() ├── CameraDevice ─────────── captureFrames(), livePreview(), start(), stop() │ └── OpenCVCamera -└── LabjackDevice ────────── getAnalogVoltage(), setAnalogVoltage(), get/setDigitalValue() +├── PowerStripDevice ─────── OutletSwitching, DefaultOutlet, CurrentMetering +│ └── PwrUSBDevice +├── LabjackDevice ────────── AnalogIO, DigitalIO, AnalogInputStream +└── SR830Device ──────────── AnalogInputStream, AnalogOutput, PhaseLockedDetection, Trigger ``` ### Communication layer From bcc15ab409ae0d39e294cfb84e5e2b02e026f7b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Tue, 4 Aug 2026 12:47:13 -0400 Subject: [PATCH 2/7] Make every capability follow getXxx -> doGetXxx, and list them 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) --- .claude/skills/pyhardwarelibrary/SKILL.md | 2 +- CHANGELOG.md | 67 ++++ CLAUDE.md | 14 +- hardwarelibrary/__main__.py | 27 ++ hardwarelibrary/capabilities.py | 318 ++++++++++++++---- hardwarelibrary/daq/labjackdevice.py | 42 +-- hardwarelibrary/daq/sr830device.py | 50 +-- hardwarelibrary/spectrometers/base.py | 22 +- hardwarelibrary/spectrometers/oceaninsight.py | 4 +- hardwarelibrary/tests/testCapabilities.py | 205 +++++++++++ hardwarelibrary/tests/testSR830.py | 28 +- 11 files changed, 644 insertions(+), 135 deletions(-) create mode 100644 hardwarelibrary/tests/testCapabilities.py diff --git a/.claude/skills/pyhardwarelibrary/SKILL.md b/.claude/skills/pyhardwarelibrary/SKILL.md index 9fe94e63..b375930d 100644 --- a/.claude/skills/pyhardwarelibrary/SKILL.md +++ b/.claude/skills/pyhardwarelibrary/SKILL.md @@ -122,7 +122,7 @@ rot.shutdownDevice() ### Spectrometers — Ocean Insight USB2000 / USB2000+ / USB4000 / USB650 / SAS -Base: `Spectrometer`. The public method *is* the hardware hook (no `do*` wrapper). +Base: `Spectrometer`. Drivers implement `doGetSpectrum` / `doGetSerialNumber`; `getSpectrum()` forwards any keyword argument to the hook. ```python from hardwarelibrary.spectrometers import Spectrometer diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e3a68b3..efae9669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,73 @@ API changes can land even when the minor version is unchanged. ## [Unreleased] +### Added +- `allCapabilities()` in `hardwarelibrary/capabilities.py`: returns every capability + mixin the library defines, in declaration order. It answers the library-wide + question ("what can be expressed?"), where `PhysicalDevice.capabilities()` answers + the per-device one ("what does this instrument support?"). Enumerating the module + rather than walking `Capability.__subclasses__()` keeps the answer independent of + which device modules happen to be imported, and excludes the drivers, which are + `Capability` subclasses themselves. +- `capabilityInterface()` in `hardwarelibrary/capabilities.py`: describes one capability + as `extends` / `publicAPI` / `hooks` lists of `CapabilityMember(name, signature, + isAbstract)` tuples. The `do` prefix is what separates a hook from the public API, + not abstractness: the DAQ capabilities make the public method itself abstract with + no `do*` counterpart. Members a parent capability declares are left to that parent. +- `python -m hardwarelibrary --capabilities` (`-c`): prints every capability with the + methods it defines and the hooks a driver must implement, so the list never has to + be maintained by hand. +- `hardwarelibrary/tests/testCapabilities.py`: covers `allCapabilities()` and + `capabilityInterface()`, and enforces the invariant that every capability mixin is + declared in `capabilities.py`, by comparing the module listing against a full walk + of the `Capability` subclass graph. + +### Changed +- **The public/`do*` template method pattern is now uniform across every capability.** + The DAQ, lock-in and trigger capabilities used to declare their public method + itself as the `@abstractmethod`; they now follow the same rule as every other + family: `getXxx()` is concrete and calls `doGetXxx()`, and only the hook is + abstract. This keeps the public method free for the argument validation, + notifications and error handling to be added there. Affected: + `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability`, + `AnalogInputStreamCapability`, `PhaseLockedDetectionCapability`, + `TriggerCapability`, `DigitalInputCapability`, `DigitalOutputCapability`, + `DigitalIOCapability`. + - **Callers are unaffected**: every public name and signature is unchanged. + - **Driver authors must rename their implementations** to the `do*` hook, e.g. + `getAnalogVoltage` -> `doGetAnalogVoltage`, `setDigitalValue` -> + `doSetDigitalValue`, `configureStream` -> `doConfigureStream`, + `softwareTrigger` -> `doSoftwareTrigger`, `supportedSensitivities` -> + `doGetSupportedSensitivities`. A driver that misses one fails loudly at + instantiation with `TypeError`, naming the missing hook. `LabjackDevice` and + `SR830Device` (and their debug counterparts) were migrated. + - `configureStream(channels, sampleRate=None, **parameters)` forwards extra + keyword arguments to `doConfigureStream`, so instrument-specific options + (the SR830's `sampleClock`, the LabJack's deprecated `scanRate`) still reach + the driver through the shared public method. +- **`Spectrometer` follows the same pattern**: `getSpectrum()` and + `getSerialNumber()` are now concrete and delegate to the abstract + `doGetSpectrum()` / `doGetSerialNumber()`, which is the last place in the + library where the public method was itself the hook. `getSpectrum(**parameters)` + forwards keywords to the driver, so `getSpectrum(maxRequests=2, maxWait=0.05)` + still reaches `OISpectrometer`. `OISpectrometer` was migrated; `DebugSpectro` + is unaffected because it does not subclass `Spectrometer`. + - **ACTION REQUIRED for the licenced StellarNet driver**, which is distributed + encrypted and is not in this repository: rename its `getSpectrum` and + `getSerialNumber` to `doGetSpectrum` and `doGetSerialNumber`. Until then + `StellarNet()` raises `TypeError` for the missing hooks. +- `hardwarelibrary/tests/testCapabilities.py` now also asserts that no + `PhysicalDevice` subclass in the library declares an abstract method outside its + `do*` hooks, so the pattern is enforced for family base classes, not just mixins. + `HOPSInterface` (`sources/verdig.py`) is deliberately exempt: it is a transport + strategy behind `VerdiGDevice`, closer to `CommunicationPort` than to a device API. +- README: the supported-hardware table now lists every driver in the tree. It was + missing `VerdiGDevice`, `FieldMasterDevice`, `SR830Device`, `PwrUSBDevice` and + `StellarNet`, and carried the Millennia without its USB identity + (`0x0483:0x5740`). Added a "Capabilities" section explaining the mixin pattern + from first principles, and refreshed the class-hierarchy diagram, which was + stale in the same way. + ## [1.5.0] - 2026-07-22 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index c5ee73df..a5b29e5a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,19 +51,25 @@ Each family has an **abstract base class**. A driver subclasses it and implement | Linear motion | `motion/` | `LinearMotionDevice` | `doMoveTo`, `doMoveBy`, `doGetPosition`, `doHome` | | Rotation | `motion/` | `RotationDevice` | `doMoveTo`, `doMoveBy`, `doGetOrientation`, `doHome` | | Power meter | `powermeters/` | `PowerMeterDevice` | `doGetAbsolutePower`, `doGetCalibrationWavelength`, `doSetCalibrationWavelength` | -| Spectrometer | `spectrometers/` | `Spectrometer` (`base.py`) | `getSpectrum`, `getSerialNumber` (here the public method *is* the hook; no `doXxx` wrapper) | +| Spectrometer | `spectrometers/` | `Spectrometer` (`base.py`) | `doGetSpectrum`, `doGetSerialNumber` | | Oscilloscope | `oscilloscope/` | `OscilloscopeDevice` | instantiated directly (Tektronix), no family/driver split; per-instrument SCPI methods | | Camera | `cameras/` | `CameraDevice` | `doCaptureFrame` | | Laser source | `sources/` | `LaserSourceDevice` (marker) + capability mixins (see below) | the `do*` hooks of whichever mixins the driver declares | -| DAQ | `daq/` | capability mixins (see below) | one method per mixin: `getAnalogVoltage` / `setAnalogVoltage` / `getDigitalValue` / `setDigitalValue` | +| DAQ | `daq/` | capability mixins (see below) | the `do*` hooks of whichever mixins the driver declares | **Thorlabs linear motion** is a backend dispatcher: `ThorlabsDevice` (in `motion/thorlabs.py`) routes to `ThorlabsKinesisDevice`, which drives the stage through `pylablib`'s Kinesis support. The old FTDI path (`ThorlabsFTDIDevice`) was dropped. Install with the `thorlabs` extra (`pip install -e .[thorlabs]`). All families **use interface-segregated capability mixins** instead of one fat base class, because a device may implement any subset of the capabilities. Every mixin lives in the single module `hardwarelibrary/capabilities.py` and subclasses the one `Capability` base there; mixins carry the `*Capability` suffix, and only instantiable hardware drivers are named `*Device`. `PhysicalDevice` provides `capabilities()` / `hasCapability(cls)`, which introspect any device by walking its MRO for `Capability` subclasses — so every family gets capability introspection for free. -- DAQ: `AnalogInputCapability` (`getAnalogVoltage`), `AnalogOutputCapability` (`setAnalogVoltage`), `DigitalInputCapability` (`getDigitalValue`), `DigitalOutputCapability` (`setDigitalValue`), plus `AnalogIOCapability` / `DigitalIOCapability` that combine each pair, and `AnalogInputStreamCapability` for hardware-timed acquisition. The `configure*` and `direction*` methods are optional no-op hooks. Example: `class LabjackDevice(PhysicalDevice, AnalogIOCapability, DigitalIOCapability, AnalogInputStreamCapability)`. -- Laser sources: `OnOffCapability` (`turnOn`/`turnOff`/`isLaserOn`), `ShutterCapability` (`openShutter`/`closeShutter`/`isShutterOpen`), `PowerCapability` (`setPower`/`power`), `InterlockCapability` (`interlock`), `AutostartCapability`, `WavelengthCapability` (`setWavelength`/`wavelength`), `DispersionCapability`. Each exposes a public method that calls a `do*` abstract hook the driver implements. `LaserSourceDevice` is a thin marker base; the behavior comes from the mixins. Examples: `class CoboltDevice(LaserSourceDevice, OnOffCapability, PowerCapability, ...)`, `class MillenniaEv25Device(LaserSourceDevice, OnOffCapability, ShutterCapability, PowerCapability)`, `class MatisseDevice(PhysicalDevice, WavelengthCapability)`. +**Every capability follows the same template**, without exception: the public method is concrete and delegates to a `do*` hook, `getXxx()` calling `doGetXxx()`, and only the hook is `@abstractmethod`. A driver implements hooks and never overrides the public method, which is where argument validation and notifications live (or will). Hooks that are optional, or that default to a composition of the other hooks (`doAcquireWaveform`, `doGetDemodulatedValues`, `doGetSupported*`), are concrete but still carry the `do` prefix. `hardwarelibrary/tests/testCapabilities.py` enforces this: no public method on any capability may be abstract. + +- DAQ: `AnalogInputCapability` (`getAnalogVoltage`), `AnalogOutputCapability` (`setAnalogVoltage`), `DigitalInputCapability` (`getDigitalValue`), `DigitalOutputCapability` (`setDigitalValue`), plus `AnalogIOCapability` / `DigitalIOCapability` that combine each pair, and `AnalogInputStreamCapability` for hardware-timed acquisition. Drivers implement `doGetAnalogVoltage` / `doSetAnalogVoltage` / `doGetDigitalValue` / `doSetDigitalValue` and the streaming hooks; the `doConfigure*` and `do*Direction` hooks are optional no-ops. `configureStream(channels, sampleRate, **parameters)` forwards any extra keyword to `doConfigureStream`, which is how instrument-specific options reach a driver (the SR830's `sampleClock`). Example: `class LabjackDevice(PhysicalDevice, AnalogIOCapability, DigitalIOCapability, AnalogInputStreamCapability)`. +- Laser sources: `OnOffCapability` (`turnOn`/`turnOff`/`isLaserOn`), `ShutterCapability` (`openShutter`/`closeShutter`/`isShutterOpen`), `PowerCapability` (`setPower`/`power`), `InterlockCapability` (`interlock`), `AutostartCapability`, `WavelengthCapability` (`setWavelength`/`wavelength`), `DispersionCapability`. `LaserSourceDevice` is a thin marker base; the behavior comes from the mixins. Examples: `class CoboltDevice(LaserSourceDevice, OnOffCapability, PowerCapability, ...)`, `class MillenniaEv25Device(LaserSourceDevice, OnOffCapability, ShutterCapability, PowerCapability)`, `class MatisseDevice(PhysicalDevice, WavelengthCapability)`. - Power meters: `WavelengthCalibrationCapability`, `AutoScaleCapability`, `ScaleCapability` (on top of the base `doGetAbsolutePower` every meter implements). +- Lock-in / triggering: `PhaseLockedDetectionCapability`, `TriggerCapability` (`SR830Device`). +- Power strips: `OutletSwitchingCapability`, `DefaultOutletCapability`, `CurrentMeteringCapability` (`PwrUSBDevice`). + +To see every capability with the methods it defines and the hooks it requires: `python -m hardwarelibrary --capabilities`, or `allCapabilities()` / `capabilityInterface()` from `hardwarelibrary/capabilities.py`. ## Adding a new device diff --git a/hardwarelibrary/__main__.py b/hardwarelibrary/__main__.py index 86a807f8..48b483c0 100644 --- a/hardwarelibrary/__main__.py +++ b/hardwarelibrary/__main__.py @@ -32,6 +32,8 @@ def signal_handler(sig, frame): const=True, help="Show notifications from DeviceManager") ap.add_argument("-d", "--debugusb", required=False, action='store_const', const=True, help="Help debugging USB libraries issues") +ap.add_argument("-c", "--capabilities", required=False, action='store_const', + const=True, help="List every capability and the API it defines") args = vars(ap.parse_args()) decrypt = args['stellarnet'] @@ -39,6 +41,7 @@ def signal_handler(sig, frame): deviceManager = args['devicemanager'] debugUSB = args['debugusb'] listAll = args['list'] +listCapabilities = args['capabilities'] if not any(args.values()): ap.print_help() @@ -71,6 +74,30 @@ def signal_handler(sig, frame): dm.showNotifications() dm.startMonitoring() +if listCapabilities == True: + from hardwarelibrary import capabilities as capabilitiesModule + from hardwarelibrary.capabilities import allCapabilities, capabilityInterface + + def readableSignature(member): + return member.signature.replace("{0}.".format(capabilitiesModule.__name__), "") + + everyCapability = allCapabilities() + print("{0} capabilities defined in {1}\n".format( + len(everyCapability), capabilitiesModule.__file__)) + + for capability in everyCapability: + interface = capabilityInterface(capability) + extends = ", ".join(klass.__name__ for klass in interface["extends"]) + print("{0}{1}".format(capability.__name__, + " (extends {0})".format(extends) if extends else "")) + for member in interface["publicAPI"]: + print(" {0}{1}{2}".format(member.name, readableSignature(member), + " [must be implemented]" if member.isAbstract else "")) + for member in interface["hooks"]: + print(" - hook: {0}{1}{2}".format(member.name, readableSignature(member), + "" if member.isAbstract else " [optional]")) + print() + if debugUSB == True: from hardwarelibrary.communication import validateUSBBackend backend = validateUSBBackend(verbose=True) diff --git a/hardwarelibrary/capabilities.py b/hardwarelibrary/capabilities.py index c8749840..8691d4bd 100644 --- a/hardwarelibrary/capabilities.py +++ b/hardwarelibrary/capabilities.py @@ -2,9 +2,12 @@ A capability is a feature an instrument may have (turn on/off, open a shutter, read a voltage, ...). A driver declares the capabilities it supports by mixing -them alongside a PhysicalDevice subclass; the mixin's public methods delegate to -the do* hooks (or, for some families, are themselves the abstract hook) the -driver implements. Mixins carry the *Capability suffix; only instantiable +them alongside a PhysicalDevice subclass. Every public method here is concrete and +delegates to a do* hook the driver implements: getXxx() calls doGetXxx(), and the +hook is the abstract one, so the public method stays free to validate arguments and +post notifications on every driver's behalf. A hook a driver may leave alone (an +optional feature, or a default built on the other hooks) is not abstract, but it +still carries the do prefix. Mixins carry the *Capability suffix; only instantiable hardware drivers are named *Device. PhysicalDevice.capabilities() / hasCapability() introspect these by walking the @@ -12,7 +15,9 @@ subclass the single Capability base defined here. """ +import inspect from abc import ABC, abstractmethod +from collections import namedtuple from enum import Enum @@ -291,16 +296,24 @@ def doGetAvailableScales(self) -> list: class AnalogInputCapability(Capability): """Analog input capability (ADC). Combine with PhysicalDevice in a driver.""" - @abstractmethod def getAnalogVoltage(self, channel): + """Returns the voltage measured on channel, in volts.""" + return self.doGetAnalogVoltage(channel) + + @abstractmethod + def doGetAnalogVoltage(self, channel): ... class AnalogOutputCapability(Capability): """Analog output capability (DAC). Combine with PhysicalDevice in a driver.""" - @abstractmethod def setAnalogVoltage(self, value, channel): + """Set the output on channel to value, in volts.""" + return self.doSetAnalogVoltage(value, channel) + + @abstractmethod + def doSetAnalogVoltage(self, value, channel): ... @@ -311,12 +324,24 @@ class AnalogIOCapability(AnalogInputCapability, AnalogOutputCapability): """ def configureAnalogIO(self, parameters: dict): - pass + """Apply the driver-specific analog configuration in parameters.""" + return self.doConfigureAnalogIO(parameters) def getAnalogDirection(self, channel): - pass + """Returns whether channel is configured as an input or an output.""" + return self.doGetAnalogDirection(channel) def setAnalogDirection(self, channel): + """Configure the direction of channel.""" + return self.doSetAnalogDirection(channel) + + def doConfigureAnalogIO(self, parameters: dict): + pass + + def doGetAnalogDirection(self, channel): + pass + + def doSetAnalogDirection(self, channel): pass @@ -350,32 +375,56 @@ class Notification(Enum): willAcquire = "willAcquire" didAcquire = "didAcquire" + def configureStream(self, channels, sampleRate=None, **parameters): + """Set up a hardware-timed acquisition of channels at sampleRate (Hz). + + Any further keyword argument is passed on to the driver, which is where + instrument-specific options live (the SR830 takes a sampleClock, for + instance). A driver that ignores sampleRate, because its clock is + external, accepts None for it. + """ + return self.doConfigureStream(channels, sampleRate, **parameters) + + def startStream(self): + """Start the configured acquisition.""" + return self.doStartStream() + + def readStream(self): + """Returns the samples acquired since the last read, as {channel: [volts, ...]}.""" + return self.doReadStream() + + def stopStream(self): + """Stop the acquisition and release any hardware streaming resources.""" + return self.doStopStream() + + def acquireWaveform(self, channels, sampleRate, sampleCount): + """Acquire exactly sampleCount samples per channel, blocking until done. + + Returns {channel: [volts, ...]} truncated to sampleCount per channel. + """ + return self.doAcquireWaveform(channels, sampleRate, sampleCount) + @abstractmethod - def configureStream(self, channels, sampleRate): - """Set up a hardware-timed acquisition of channels at sampleRate (Hz).""" + def doConfigureStream(self, channels, sampleRate): ... @abstractmethod - def startStream(self): - """Start the configured acquisition.""" + def doStartStream(self): ... @abstractmethod - def readStream(self): - """Return the samples acquired since the last read, as {channel: [volts, ...]}.""" + def doReadStream(self): ... @abstractmethod - def stopStream(self): - """Stop the acquisition and release any hardware streaming resources.""" + def doStopStream(self): ... - def acquireWaveform(self, channels, sampleRate, sampleCount): - """Acquire exactly sampleCount samples per channel, blocking until done. + def doAcquireWaveform(self, channels, sampleRate, sampleCount): + """Configure, start, and drain the stream, then stop it. - Configures, starts, and drains the stream (looping readStream) on the - caller's behalf, then stops it; returns {channel: [volts, ...]} truncated - to sampleCount per channel. + Optional: a driver whose hardware has a native one-shot acquisition + overrides this instead of being drained a block at a time. """ self.configureStream(channels, sampleRate) samples = {channel: [] for channel in channels} @@ -414,79 +463,127 @@ class PhaseLockedDetectionCapability(Capability): contract; a driver snaps a requested value to its nearest supported step. """ - @abstractmethod def getInPhaseVoltage(self): """Returns the in-phase component X, in volts.""" - ... + return self.doGetInPhaseVoltage() - @abstractmethod def getQuadratureVoltage(self): """Returns the quadrature component Y, in volts.""" - ... + return self.doGetQuadratureVoltage() - @abstractmethod def getMagnitude(self): """Returns the magnitude R = sqrt(X^2 + Y^2), in volts.""" - ... + return self.doGetMagnitude() - @abstractmethod def getPhase(self): """Returns the phase theta, in degrees.""" - ... + return self.doGetPhase() - @abstractmethod def getReferenceFrequency(self): """Returns the reference frequency, in Hz.""" - ... + return self.doGetReferenceFrequency() - @abstractmethod def getInputSource(self) -> InputSource: """Returns the signal input the demodulator currently measures.""" - ... + return self.doGetInputSource() - @abstractmethod def setInputSource(self, source: InputSource): """Select which signal input (an InputSource member) the demodulator measures.""" - ... + return self.doSetInputSource(source) - @abstractmethod def getSensitivity(self): """Returns the full-scale sensitivity, in volts.""" - ... + return self.doGetSensitivity() - @abstractmethod def setSensitivity(self, volts): """Set the full-scale sensitivity to the nearest supported step, in volts.""" - ... + return self.doSetSensitivity(volts) - @abstractmethod def getTimeConstant(self): """Returns the time constant, in seconds.""" - ... + return self.doGetTimeConstant() - @abstractmethod def setTimeConstant(self, seconds): """Set the time constant to the nearest supported step, in seconds.""" - ... + return self.doSetTimeConstant(seconds) def supportedInputSources(self): - """Optional: the InputSource members this instrument supports, or None.""" - return None + """Returns the InputSource members this instrument supports, or None.""" + return self.doGetSupportedInputSources() def supportedSensitivities(self): - """Optional: the full-scale sensitivities (volts) this instrument supports, or None.""" - return None + """Returns the full-scale sensitivities (volts) this instrument supports, or None.""" + return self.doGetSupportedSensitivities() def supportedTimeConstants(self): - """Optional: the time constants (seconds) this instrument supports, or None.""" - return None + """Returns the time constants (seconds) this instrument supports, or None.""" + return self.doGetSupportedTimeConstants() def getDemodulatedValues(self): - """One reading of all demodulated outputs plus the reference frequency. + """One reading of all demodulated outputs plus the reference frequency.""" + return self.doGetDemodulatedValues() + + @abstractmethod + def doGetInPhaseVoltage(self): + ... + + @abstractmethod + def doGetQuadratureVoltage(self): + ... + + @abstractmethod + def doGetMagnitude(self): + ... + + @abstractmethod + def doGetPhase(self): + ... + + @abstractmethod + def doGetReferenceFrequency(self): + ... + + @abstractmethod + def doGetInputSource(self) -> InputSource: + ... + + @abstractmethod + def doSetInputSource(self, source: InputSource): + ... + + @abstractmethod + def doGetSensitivity(self): + ... + + @abstractmethod + def doSetSensitivity(self, volts): + ... + + @abstractmethod + def doGetTimeConstant(self): + ... + + @abstractmethod + def doSetTimeConstant(self, seconds): + ... + + def doGetSupportedInputSources(self): + """Optional: None means the instrument does not advertise a list.""" + return None + + def doGetSupportedSensitivities(self): + """Optional: None means the instrument does not advertise a list.""" + return None + + def doGetSupportedTimeConstants(self): + """Optional: None means the instrument does not advertise a list.""" + return None + + def doGetDemodulatedValues(self): + """Read the outputs one at a time. - Built on the individual getters; a driver may override it to read the - outputs atomically (a single coherent timepoint) when the hardware - supports it. + Optional: a driver overrides this when the hardware can read them + atomically, at a single coherent timepoint. """ return { "X": self.getInPhaseVoltage(), @@ -526,39 +623,60 @@ class TriggerCapability(Capability): PhysicalDevice in a driver. """ - @abstractmethod def setTriggerSource(self, source: 'TriggerSource'): """Select whether the acquisition starts immediately or on an external trigger.""" - ... + return self.doSetTriggerSource(source) - @abstractmethod def getTriggerSource(self) -> 'TriggerSource': """Returns the currently selected TriggerSource.""" - ... + return self.doGetTriggerSource() - @abstractmethod def softwareTrigger(self): """Issue a manual (software) trigger edge.""" - ... + return self.doSoftwareTrigger() def supportedTriggerSources(self): - """Optional: the TriggerSource members this device supports, or None.""" + """Returns the TriggerSource members this device supports, or None.""" + return self.doGetSupportedTriggerSources() + + @abstractmethod + def doSetTriggerSource(self, source: 'TriggerSource'): + ... + + @abstractmethod + def doGetTriggerSource(self) -> 'TriggerSource': + ... + + @abstractmethod + def doSoftwareTrigger(self): + ... + + def doGetSupportedTriggerSources(self): + """Optional: None means the device does not advertise a list.""" return None class DigitalInputCapability(Capability): """Digital input capability. Combine with PhysicalDevice in a driver.""" - @abstractmethod def getDigitalValue(self, channel): + """Returns the logic level read on channel.""" + return self.doGetDigitalValue(channel) + + @abstractmethod + def doGetDigitalValue(self, channel): ... class DigitalOutputCapability(Capability): """Digital output capability. Combine with PhysicalDevice in a driver.""" - @abstractmethod def setDigitalValue(self, value, channel): + """Drive channel to the logic level value.""" + return self.doSetDigitalValue(value, channel) + + @abstractmethod + def doSetDigitalValue(self, value, channel): ... @@ -569,12 +687,24 @@ class DigitalIOCapability(DigitalInputCapability, DigitalOutputCapability): """ def configureDigitalIO(self, parameters: dict): - pass + """Apply the driver-specific digital configuration in parameters.""" + return self.doConfigureDigitalIO(parameters) def getDigitalDirection(self, channel): - pass + """Returns whether channel is configured as an input or an output.""" + return self.doGetDigitalDirection(channel) def setDigitalDirection(self, channel): + """Configure the direction of channel.""" + return self.doSetDigitalDirection(channel) + + def doConfigureDigitalIO(self, parameters: dict): + pass + + def doGetDigitalDirection(self, channel): + pass + + def doSetDigitalDirection(self, channel): pass @@ -675,3 +805,65 @@ def doGetAccumulatedCharge(self) -> float: @abstractmethod def doResetAccumulatedCharge(self): ... + + +# --------------------------------------------------------------------------- +# Introspection +# --------------------------------------------------------------------------- + + +def allCapabilities() -> list: + """Return every capability mixin defined here, in declaration order. + + Use this to enumerate what the library can express, as opposed to + PhysicalDevice.capabilities(), which reports what one device supports. The + Capability marker base is excluded, and so are the drivers that mix these + in: a driver is a Capability subclass too, but it is declared in its own + module. + """ + # A module's __dict__ is insertion-ordered, so filtering it in place yields + # the classes in the order they are declared above, grouped by family. + return [candidate for candidate in list(globals().values()) + if isinstance(candidate, type) + and issubclass(candidate, Capability) + and candidate is not Capability + and candidate.__module__ == __name__] + + +CapabilityMember = namedtuple("CapabilityMember", ["name", "signature", "isAbstract"]) + + +def capabilityInterface(aCapability) -> dict: + """Describe what a capability declares, as three lists under the keys + extends, publicAPI and hooks. + + The publicAPI entries are the methods a user calls; the hooks are the ones a + driver implements. Both are CapabilityMember tuples, with an empty signature + for a property. Members a parent capability declares are left to that parent, + so a listing built from allCapabilities() never repeats them. + """ + publicAPI, hooks = [], [] + for name, member in vars(aCapability).items(): + if name.startswith("_"): + continue + + if isinstance(member, property): + signature = "" + isAbstract = getattr(member.fget, "__isabstractmethod__", False) + elif inspect.isfunction(member): + signature = inspect.signature(member) + signature = str(signature.replace( + parameters=list(signature.parameters.values())[1:])) + isAbstract = getattr(member, "__isabstractmethod__", False) + else: + continue + + # The do prefix is what marks a hook, not abstractness: a hook that is + # optional, or that defaults to a composition of the other hooks, is + # concrete and would otherwise be mistaken for public API. + destination = hooks if name.startswith("do") else publicAPI + destination.append(CapabilityMember(name, signature, isAbstract)) + + extends = [klass for klass in aCapability.__mro__[1:] + if issubclass(klass, Capability) and klass is not Capability] + return {"extends": extends, "publicAPI": publicAPI, "hooks": hooks} diff --git a/hardwarelibrary/daq/labjackdevice.py b/hardwarelibrary/daq/labjackdevice.py index c13d015f..3acbe0ed 100644 --- a/hardwarelibrary/daq/labjackdevice.py +++ b/hardwarelibrary/daq/labjackdevice.py @@ -51,17 +51,17 @@ def setConfiguration(self, parameters: dict): def configuration(self): return self.dev.configU3() - def configureAnalogIO(self, parameters: dict): + def doConfigureAnalogIO(self, parameters: dict): self._validateConfigIOParameters(parameters) self.dev.configIO(**parameters) - def configureDigitalIO(self, parameters: dict): + def doConfigureDigitalIO(self, parameters: dict): self._validateConfigIOParameters(parameters) self.dev.configIO(**parameters) @staticmethod def _validateConfigIOParameters(parameters): - # configureAnalogIO and configureDigitalIO both forward to the U3's single + # doConfigureAnalogIO and doConfigureDigitalIO both forward to the U3's single # configIO command; validate keys against its actual signature so an # unexpected key fails clearly here instead of deep inside configIO. import u3 @@ -73,11 +73,11 @@ def _validateConfigIOParameters(parameters): f"valid keys are {sorted(valid)}" ) - def getAnalogVoltage(self, channel): + def doGetAnalogVoltage(self, channel): """Returns volts.""" return self.dev.getAIN(channel) - def setAnalogVoltage(self, value, channel): + def doSetAnalogVoltage(self, value, channel): """value in volts, on DAC0 (channel 0) or DAC1 (channel 1).""" # 5000/5002 are the U3 Modbus registers for DAC0/DAC1. if channel == 0: @@ -88,10 +88,10 @@ def setAnalogVoltage(self, value, channel): raise ValueError(f"DAC channel must be 0 or 1, got {channel}") self.dev.writeRegister(register, value) - def setDigitalValue(self, value, channel): + def doSetDigitalValue(self, value, channel): self.dev.setDOState(channel, value) - def getDigitalValue(self, channel): + def doGetDigitalValue(self, channel): return self.dev.getDIState(channel) != 0 def getTemperature(self): @@ -101,7 +101,7 @@ def getTemperature(self): def toggleLED(self): self.dev.toggleLED() - def configureStream(self, channels, sampleRate=None, scanRate=None): + def doConfigureStream(self, channels, sampleRate=None, scanRate=None): # scanRate is a deprecated synonym for sampleRate, kept temporarily for # callers written against the pre-1.4.0 configureStream signature. if sampleRate is None: @@ -115,17 +115,17 @@ def configureStream(self, channels, sampleRate=None, scanRate=None): ScanFrequency=sampleRate, ) - def startStream(self): + def doStartStream(self): self.dev.streamStart() self._streamData = self.dev.streamData(convert=True) - def readStream(self): + def doReadStream(self): packet = next(self._streamData) if packet is None: return {channel: [] for channel in self._streamChannels} return {channel: packet[f'AIN{channel}'] for channel in self._streamChannels} - def stopStream(self): + def doStopStream(self): self.dev.streamStop() self._streamData = None @@ -165,24 +165,24 @@ def setConfiguration(self, parameters: dict): def configuration(self): return {'DeviceName': 'DebugU3', 'SerialNumber': 0} - def configureAnalogIO(self, parameters: dict): + def doConfigureAnalogIO(self, parameters: dict): self._validateConfigIOParameters(parameters) - def configureDigitalIO(self, parameters: dict): + def doConfigureDigitalIO(self, parameters: dict): self._validateConfigIOParameters(parameters) - def getAnalogVoltage(self, channel): + def doGetAnalogVoltage(self, channel): return self._analogValues.get(channel, 0.0) - def setAnalogVoltage(self, value, channel): + def doSetAnalogVoltage(self, value, channel): if channel not in (0, 1): raise ValueError(f"DAC channel must be 0 or 1, got {channel}") self._analogValues[channel] = value - def setDigitalValue(self, value, channel): + def doSetDigitalValue(self, value, channel): self._digitalValues[channel] = bool(value) - def getDigitalValue(self, channel): + def doGetDigitalValue(self, channel): return self._digitalValues.get(channel, False) def getTemperature(self): @@ -191,16 +191,16 @@ def getTemperature(self): def toggleLED(self): pass - def configureStream(self, channels, sampleRate=None, scanRate=None): + def doConfigureStream(self, channels, sampleRate=None, scanRate=None): self._streamChannels = list(channels) - def startStream(self): + def doStartStream(self): pass - def readStream(self): + def doReadStream(self): blockSize = 25 return {channel: [self._analogValues.get(channel, 0.0)] * blockSize for channel in self._streamChannels} - def stopStream(self): + def doStopStream(self): pass diff --git a/hardwarelibrary/daq/sr830device.py b/hardwarelibrary/daq/sr830device.py index b0bc9cbb..c7831c26 100644 --- a/hardwarelibrary/daq/sr830device.py +++ b/hardwarelibrary/daq/sr830device.py @@ -233,14 +233,14 @@ def readIdentity(self) -> str: self.idn = self.query("*IDN?") return self.idn - def getAnalogVoltage(self, channel): + def doGetAnalogVoltage(self, channel): """Read an Aux Input, in volts (SR830 OAUX?). channel is an AuxInput member; a bare 1-4 is also accepted and coerced, and anything else raises ValueError.""" channel = AuxInput(channel) return self.queryFloat("OAUX? {0}".format(channel.value)) - def setAnalogVoltage(self, value, channel): + def doSetAnalogVoltage(self, value, channel): """Set an Aux Output to value volts (SR830 AUXV). channel is an AuxOutput member (a bare 1-4 is also accepted and coerced). value must be within [-10.5, 10.5] V or ValueError is raised.""" @@ -257,23 +257,23 @@ def getAnalogOutputVoltage(self, channel): channel = AuxOutput(channel) return self.queryFloat("AUXV? {0}".format(channel.value)) - def getInPhaseVoltage(self): + def doGetInPhaseVoltage(self): """Returns the in-phase component X, in volts (SR830 OUTP? 1).""" return self.queryFloat("OUTP? 1") - def getQuadratureVoltage(self): + def doGetQuadratureVoltage(self): """Returns the quadrature component Y, in volts (SR830 OUTP? 2).""" return self.queryFloat("OUTP? 2") - def getMagnitude(self): + def doGetMagnitude(self): """Returns the magnitude R = sqrt(X^2 + Y^2), in volts (SR830 OUTP? 3).""" return self.queryFloat("OUTP? 3") - def getPhase(self): + def doGetPhase(self): """Returns the phase theta, in degrees (SR830 OUTP? 4).""" return self.queryFloat("OUTP? 4") - def getReferenceFrequency(self): + def doGetReferenceFrequency(self): """Returns the reference frequency, in Hz (SR830 FREQ?).""" return self.queryFloat("FREQ?") @@ -286,7 +286,7 @@ def snap(self, *parameters): command = "SNAP? " + ",".join(str(int(parameter)) for parameter in parameters) return tuple(float(value) for value in self.query(command).split(",")) - def getDemodulatedValues(self): + def doGetDemodulatedValues(self): """Read X, Y, R, and theta at one instant, plus the reference frequency. Overrides the base implementation to read the four outputs atomically via @@ -300,7 +300,7 @@ def getDemodulatedValues(self): # buffer. channels are StreamChannel members (at most one CH1 and one CH2 # quantity). acquireWaveform (from the base) loops readStream for you. - def configureStream(self, channels, sampleRate, sampleClock=SampleClock.Internal): + def doConfigureStream(self, channels, sampleRate, sampleClock=SampleClock.Internal): """Set up buffered acquisition. channels is a list of StreamChannel members (1 or 2, not both on the same display). With an Internal sampleClock, sampleRate (Hz) is snapped to the nearest SRAT step. With an @@ -327,7 +327,7 @@ def configureStream(self, channels, sampleRate, sampleClock=SampleClock.Internal self._streamChannels = channels self._streamReadIndex = 0 - def startStream(self): + def doStartStream(self): """Clear the data buffer (REST) and start acquisition (STRT). With an External trigger source the SR830 arms here but does not record @@ -337,7 +337,7 @@ def startStream(self): self._streamReadIndex = 0 self.writeCommand("STRT") - def readStream(self): + def doReadStream(self): """Return the samples buffered since the last read, as {channel: [volts, ...]}. Reads how many points the buffer holds (SPTS?) and transfers only the new @@ -357,7 +357,7 @@ def readStream(self): self._streamReadIndex = available return block - def stopStream(self): + def doStopStream(self): """Pause acquisition into the data buffer (SR830 PAUS).""" self.writeCommand("PAUS") @@ -373,7 +373,7 @@ def _sampleRateIndexFor(self, rate): # TriggerCapability: the rear-panel TRIG IN. setTriggerSource(External) arms # the scan to start on a trigger edge (TSTR); trigger() issues a software edge. - def setTriggerSource(self, source: TriggerSource): + def doSetTriggerSource(self, source: TriggerSource): """Select immediate (Internal) or external-trigger (External) scan start (SR830 TSTR). source must be a supported TriggerSource or ValueError is raised.""" @@ -381,54 +381,54 @@ def setTriggerSource(self, source: TriggerSource): raise ValueError("Unsupported trigger source {0}".format(source)) self.writeCommand("TSTR {0}".format(_TRIGGER_SOURCE_TO_INDEX[source])) - def getTriggerSource(self) -> TriggerSource: + def doGetTriggerSource(self) -> TriggerSource: """Returns the current scan-start TriggerSource (SR830 TSTR?).""" return _INDEX_TO_TRIGGER_SOURCE[self.queryInteger("TSTR?")] - def softwareTrigger(self): + def doSoftwareTrigger(self): """Issue a software trigger edge (SR830 TRIG), as if TRIG IN pulsed.""" self.writeCommand("TRIG") - def supportedTriggerSources(self): + def doGetSupportedTriggerSources(self): """Returns the trigger sources the SR830 supports (Internal, External).""" return list(TriggerSource) - def getInputSource(self) -> InputSource: + def doGetInputSource(self) -> InputSource: """Returns the demodulator's signal input source (SR830 ISRC?).""" return _INDEX_TO_INPUT_SOURCE[self.queryInteger("ISRC?")] - def setInputSource(self, source: InputSource): + def doSetInputSource(self, source: InputSource): """Select the demodulator's signal input source (SR830 ISRC). source must be a supported InputSource or ValueError is raised.""" if source not in _INPUT_SOURCE_TO_INDEX: raise ValueError("Unsupported input source {0}".format(source)) self.writeCommand("ISRC {0}".format(_INPUT_SOURCE_TO_INDEX[source])) - def supportedInputSources(self): + def doGetSupportedInputSources(self): """Returns the four input sources the SR830 supports.""" return list(InputSource) - def getSensitivity(self): + def doGetSensitivity(self): """Returns the current full-scale sensitivity, in volts (SR830 SENS?).""" return self.sensitivities[self.queryInteger("SENS?")] - def setSensitivity(self, volts): + def doSetSensitivity(self, volts): """Set the full-scale sensitivity to the smallest step >= volts (SR830 SENS).""" self.writeCommand("SENS {0}".format(self._sensitivityIndexFor(volts))) - def getTimeConstant(self): + def doGetTimeConstant(self): """Returns the current time constant, in seconds (SR830 OFLT?).""" return self.timeConstants[self.queryInteger("OFLT?")] - def setTimeConstant(self, seconds): + def doSetTimeConstant(self, seconds): """Set the time constant to the nearest step, in seconds (SR830 OFLT).""" self.writeCommand("OFLT {0}".format(self._timeConstantIndexFor(seconds))) - def supportedSensitivities(self): + def doGetSupportedSensitivities(self): """Returns the SR830's discrete full-scale sensitivities, in volts.""" return list(self.sensitivities) - def supportedTimeConstants(self): + def doGetSupportedTimeConstants(self): """Returns the SR830's discrete time constants, in seconds.""" return list(self.timeConstants) diff --git a/hardwarelibrary/spectrometers/base.py b/hardwarelibrary/spectrometers/base.py index 791036a5..6c6198ab 100644 --- a/hardwarelibrary/spectrometers/base.py +++ b/hardwarelibrary/spectrometers/base.py @@ -39,15 +39,27 @@ def __init__(self, serialNumber=None, idProduct:int = None, idVendor:int = None) self.wavelength = np.linspace(400,1000,1024) self.integrationTime = 10 - # The contract a driver must implement. For spectrometers the public - # method is the hook itself (no doXxx wrapper), on top of - # doInitializeDevice and doShutdownDevice inherited from PhysicalDevice. - @abstractmethod def getSerialNumber(self): + """Returns the serial number, which tells two connected spectrometers apart.""" + return self.doGetSerialNumber() + + def getSpectrum(self, **parameters) -> np.array: + """Returns one spectrum, as an array of intensities. + + Any keyword argument is passed on to the driver, which is where + instrument-specific options live (the Ocean Insight units take an + integrationTime and bounds on how long to wait for the data). + """ + return self.doGetSpectrum(**parameters) + + # The contract a driver must implement, on top of doInitializeDevice and + # doShutdownDevice inherited from PhysicalDevice. + @abstractmethod + def doGetSerialNumber(self): ... @abstractmethod - def getSpectrum(self) -> np.array: + def doGetSpectrum(self) -> np.array: ... def display(self): diff --git a/hardwarelibrary/spectrometers/oceaninsight.py b/hardwarelibrary/spectrometers/oceaninsight.py index d694a43b..e729a75e 100644 --- a/hardwarelibrary/spectrometers/oceaninsight.py +++ b/hardwarelibrary/spectrometers/oceaninsight.py @@ -311,7 +311,7 @@ def getIntegrationTime(self): status = self.getStatus() return float(status.integrationTime)/self.timeScale - def getSerialNumber(self): + def doGetSerialNumber(self): """ Get the serial nunmber of the spectrometer. This can be used to differentiate two connected spectrometers. """ @@ -466,7 +466,7 @@ def getSpectrumData(self): """ raise NotImplementedError('You must implemented getSpectrumData for your subclass.') - def getSpectrum(self, integrationTime=None, maxRequests=4, maxWait=2.0): + def doGetSpectrum(self, integrationTime=None, maxRequests=4, maxWait=2.0): """ Obtain a spectrum from the spectrometer. This implies: 1- changing the integration time if needed. 2- requesting a spectrum, diff --git a/hardwarelibrary/tests/testCapabilities.py b/hardwarelibrary/tests/testCapabilities.py new file mode 100644 index 00000000..9f380bd5 --- /dev/null +++ b/hardwarelibrary/tests/testCapabilities.py @@ -0,0 +1,205 @@ +import env +import os +import sys +import unittest + +import hardwarelibrary.daq +import hardwarelibrary.powermeters +import hardwarelibrary.powerstrips +import hardwarelibrary.sources +import hardwarelibrary.spectrometers +from hardwarelibrary.capabilities import ( + Capability, allCapabilities, capabilityInterface, + OnOffCapability, ShutterCapability, PowerCapability, + AnalogInputCapability, AnalogOutputCapability, AnalogIOCapability, + OutletSwitchingCapability) +from hardwarelibrary.physicaldevice import PhysicalDevice + + +testsDirectory = os.path.dirname(os.path.abspath(__file__)) + + +def everySubclassOf(aClass): + """Yield every subclass of aClass, at any depth. A class reachable through + more than one branch is yielded more than once.""" + for subclass in aClass.__subclasses__(): + yield subclass + yield from everySubclassOf(subclass) + + +def isDeclaredInATestModule(aClass) -> bool: + """True when aClass comes from a file in this directory. Matching on the + file, not on the module name, because a test module is imported as either + testFoo or hardwarelibrary.tests.testFoo depending on the runner.""" + module = sys.modules.get(aClass.__module__) + fileName = getattr(module, "__file__", None) + if fileName is None: + return False + return os.path.dirname(os.path.abspath(fileName)) == testsDirectory + + +class TestAllCapabilities(unittest.TestCase): + def testEveryEntryIsACapability(self): + for capability in allCapabilities(): + self.assertTrue(issubclass(capability, Capability), capability.__name__) + + def testMarkerBaseIsExcluded(self): + self.assertNotIn(Capability, allCapabilities()) + + def testNoDeviceIsReportedAsACapability(self): + for capability in allCapabilities(): + self.assertFalse(issubclass(capability, PhysicalDevice), capability.__name__) + + def testEveryCapabilityCarriesTheCapabilitySuffix(self): + for capability in allCapabilities(): + self.assertTrue(capability.__name__.endswith("Capability"), capability.__name__) + + def testCapabilitiesFromEveryFamilyAreListed(self): + capabilities = allCapabilities() + self.assertIn(OnOffCapability, capabilities) + self.assertIn(AnalogIOCapability, capabilities) + self.assertIn(OutletSwitchingCapability, capabilities) + + def testCapabilitiesThatOthersExtendAreStillListed(self): + # AnalogIOCapability extends both of these, so a walk that only kept the + # leaves of the class graph would drop them from the listing. + capabilities = allCapabilities() + self.assertIn(AnalogInputCapability, capabilities) + self.assertIn(AnalogOutputCapability, capabilities) + self.assertIn(AnalogIOCapability, capabilities) + self.assertTrue(issubclass(AnalogIOCapability, AnalogInputCapability)) + self.assertTrue(issubclass(AnalogIOCapability, AnalogOutputCapability)) + + def testDeclarationOrderIsPreserved(self): + capabilities = allCapabilities() + self.assertLess(capabilities.index(OnOffCapability), capabilities.index(ShutterCapability)) + self.assertLess(capabilities.index(ShutterCapability), capabilities.index(PowerCapability)) + self.assertLess(capabilities.index(PowerCapability), capabilities.index(AnalogInputCapability)) + + def testNoDuplicateEntries(self): + capabilities = allCapabilities() + self.assertEqual(len(capabilities), len(set(capabilities))) + + def testNoCapabilityIsDeclaredOutsideTheCapabilitiesModule(self): + # Walking the subclass graph finds capabilities wherever they are + # declared, so comparing it against allCapabilities() is what enforces + # the single-module rule. Test modules are exempt: they legitimately mix + # capabilities into throwaway stand-ins for a driver. + walked = {klass for klass in everySubclassOf(Capability) + if not issubclass(klass, PhysicalDevice) + and not isDeclaredInATestModule(klass)} + self.assertEqual(walked, set(allCapabilities())) + + +class TestDeviceHookPattern(unittest.TestCase): + def testNoDeviceDeclaresAnAbstractMethodOutsideItsDoHooks(self): + # The same rule beyond the mixins: a family base (Spectrometer, + # PowerMeterDevice, CameraDevice, ...) declares only do* hooks as + # abstract, so its public API stays concrete and free to delegate. + for klass in everySubclassOf(PhysicalDevice): + if isDeclaredInATestModule(klass): + continue + for methodName in getattr(klass, "__abstractmethods__", ()): + if methodName in vars(klass): + self.assertTrue(methodName.startswith("do"), + "{0}.{1}".format(klass.__name__, methodName)) + + +class _RecordingAnalogDevice(AnalogIOCapability): + """Implements only the hooks, and records the calls the public methods make.""" + + def __init__(self): + self.calls = [] + + def doGetAnalogVoltage(self, channel): + self.calls.append(("doGetAnalogVoltage", channel)) + return 1.5 + + def doSetAnalogVoltage(self, value, channel): + self.calls.append(("doSetAnalogVoltage", value, channel)) + + def doConfigureAnalogIO(self, parameters: dict): + self.calls.append(("doConfigureAnalogIO", parameters)) + + +class TestCapabilityInterface(unittest.TestCase): + def namesOf(self, members): + return [member.name for member in members] + + def testPublicAPIAndHooksAreSeparated(self): + interface = capabilityInterface(OnOffCapability) + self.assertEqual(set(self.namesOf(interface["publicAPI"])), + {"isLaserOn", "turnOn", "turnOff", "canTurnOn"}) + self.assertEqual(set(self.namesOf(interface["hooks"])), + {"doTurnOn", "doTurnOff", "doGetOnOffState"}) + + def testNoPublicMethodIsMistakenForAHook(self): + for capability in allCapabilities(): + interface = capabilityInterface(capability) + for member in interface["publicAPI"]: + self.assertFalse(member.name.startswith("do"), member.name) + for member in interface["hooks"]: + self.assertTrue(member.name.startswith("do"), member.name) + + def testPrivateMembersAreExcluded(self): + for capability in allCapabilities(): + interface = capabilityInterface(capability) + for member in interface["publicAPI"] + interface["hooks"]: + self.assertFalse(member.name.startswith("_"), member.name) + + def testHooksAreReportedAsAbstract(self): + interface = capabilityInterface(ShutterCapability) + for member in interface["hooks"]: + self.assertTrue(member.isAbstract, member.name) + + def testNoPublicMethodIsAbstract(self): + # The library-wide pattern: a public method is concrete and delegates, so + # it stays free to validate arguments and post notifications, and only the + # do* hook a driver implements is abstract. + for capability in allCapabilities(): + interface = capabilityInterface(capability) + for member in interface["publicAPI"]: + self.assertFalse(member.isAbstract, + "{0}.{1}".format(capability.__name__, member.name)) + + def testEveryCapabilityDeclaresAtLeastOneHook(self): + for capability in allCapabilities(): + self.assertNotEqual(capabilityInterface(capability)["hooks"], [], + capability.__name__) + + def testPublicMethodsDelegateToTheirHook(self): + device = _RecordingAnalogDevice() + self.assertEqual(device.getAnalogVoltage(3), 1.5) + device.setAnalogVoltage(2.5, channel=1) + device.configureAnalogIO({"key": "value"}) + self.assertEqual(device.calls, [("doGetAnalogVoltage", 3), + ("doSetAnalogVoltage", 2.5, 1), + ("doConfigureAnalogIO", {"key": "value"})]) + + def testSignatureOmitsSelf(self): + interface = capabilityInterface(PowerCapability) + signatures = {member.name: member.signature for member in interface["publicAPI"]} + self.assertEqual(signatures["setPower"], "(power: float)") + self.assertEqual(signatures["power"], "() -> float") + + def testPropertiesAreListedWithoutASignature(self): + interface = capabilityInterface(OutletSwitchingCapability) + outletCount = [member for member in interface["publicAPI"] + if member.name == "outletCount"] + self.assertEqual(len(outletCount), 1) + self.assertEqual(outletCount[0].signature, "") + + def testInheritedMembersAreLeftToTheCapabilityThatDeclaresThem(self): + interface = capabilityInterface(AnalogIOCapability) + self.assertNotIn("getAnalogVoltage", self.namesOf(interface["publicAPI"])) + self.assertIn("getAnalogVoltage", + self.namesOf(capabilityInterface(AnalogInputCapability)["publicAPI"])) + + def testExtendsReportsTheCapabilitiesCombined(self): + self.assertEqual(set(capabilityInterface(AnalogIOCapability)["extends"]), + {AnalogInputCapability, AnalogOutputCapability}) + self.assertEqual(capabilityInterface(OnOffCapability)["extends"], []) + + +if __name__ == "__main__": + unittest.main() diff --git a/hardwarelibrary/tests/testSR830.py b/hardwarelibrary/tests/testSR830.py index 7ef7bb7a..69d1e8b5 100644 --- a/hardwarelibrary/tests/testSR830.py +++ b/hardwarelibrary/tests/testSR830.py @@ -233,46 +233,46 @@ class _MinimalLockIn(PhaseLockedDetectionCapability, TriggerCapability): the base-class optional hooks and the base getDemodulatedValues are exercised (SR830Device overrides all of these).""" - def getInPhaseVoltage(self): + def doGetInPhaseVoltage(self): return 0.1 - def getQuadratureVoltage(self): + def doGetQuadratureVoltage(self): return 0.2 - def getMagnitude(self): + def doGetMagnitude(self): return 0.3 - def getPhase(self): + def doGetPhase(self): return 45.0 - def getReferenceFrequency(self): + def doGetReferenceFrequency(self): return 1000.0 - def getInputSource(self): + def doGetInputSource(self): return InputSource.SingleEnded - def setInputSource(self, source): + def doSetInputSource(self, source): pass - def getSensitivity(self): + def doGetSensitivity(self): return 1.0 - def setSensitivity(self, volts): + def doSetSensitivity(self, volts): pass - def getTimeConstant(self): + def doGetTimeConstant(self): return 0.1 - def setTimeConstant(self, seconds): + def doSetTimeConstant(self, seconds): pass - def setTriggerSource(self, source): + def doSetTriggerSource(self, source): pass - def getTriggerSource(self): + def doGetTriggerSource(self): return TriggerSource.Internal - def softwareTrigger(self): + def doSoftwareTrigger(self): pass From a761d93936181ee7682c92933ab273c3311859ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Tue, 4 Aug 2026 13:20:27 -0400 Subject: [PATCH 3/7] Post will/did notifications from every capability 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 then did; a read (doGet*, doReadStream) posts only did, 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 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) --- CHANGELOG.md | 28 ++ CLAUDE.md | 11 +- README.md | 40 ++- hardwarelibrary/__main__.py | 3 + hardwarelibrary/capabilities.py | 376 +++++++++++++++++++++- hardwarelibrary/spectrometers/base.py | 13 + hardwarelibrary/tests/testCapabilities.py | 170 +++++++++- 7 files changed, 624 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efae9669..52b24a92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ API changes can land even when the minor version is unchanged. ## [Unreleased] ### Added +- **Notifications on every capability.** Each capability owns a + `Notification` enum, reachable as its `notification` attribute, and + every public method is wrapped in the new `@notifies(will=..., did=...)` + decorator, so a driver gets notifications by implementing hooks and writing no + notification code at all. An operation that changes the instrument posts + `will` then `did`; a read (`doGet*`, `doReadStream`) posts only + `did`, to keep a voltage sampled in a loop at one notification instead of + two. `Spectrometer` gets `SpectrometerNotification`, whose `getSpectrum` keeps a + `will` because an acquisition takes an integration time. + - `did*` is posted whether the operation succeeded or not, so a `will*` is always + followed by its `did*` and there is no separate failure member to pair up. The + exception is still **re-raised untouched**, since a driver's exception type is + part of its contract, so a caller sees it exactly as before while an observer + decides what to do from the payload. + - `user_info` is a dict of the public method's arguments by name, plus `"result"` + and `"error"`, exactly one of which is non-None. + - Capabilities related by inheritance share one enum, so `notification` is the + same object on all of them and the members are interchangeable: + `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability` and + `AnalogInputStreamCapability` all post `AnalogNotification`, and the digital + trio posts `DigitalNotification` (17 enums for 22 capabilities). Members are + keyed by identity, so without sharing an observer would have to know which + variant a device mixed in. + - Measured overhead on a read with no observer is ~1.6 us per call (~2.0 us with + one observer), against millisecond-scale device I/O. + - Removes the unused nested `AnalogInputStreamCapability.Notification` + (`willAcquire` / `didAcquire`), which was never posted; the equivalent members + are now `AnalogInputStreamNotification.willAcquireWaveform` / `didAcquireWaveform`. - `allCapabilities()` in `hardwarelibrary/capabilities.py`: returns every capability mixin the library defines, in declaration order. It answers the library-wide question ("what can be expressed?"), where `PhysicalDevice.capabilities()` answers diff --git a/CLAUDE.md b/CLAUDE.md index a5b29e5a..c84cdba8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,7 +61,16 @@ Each family has an **abstract base class**. A driver subclasses it and implement All families **use interface-segregated capability mixins** instead of one fat base class, because a device may implement any subset of the capabilities. Every mixin lives in the single module `hardwarelibrary/capabilities.py` and subclasses the one `Capability` base there; mixins carry the `*Capability` suffix, and only instantiable hardware drivers are named `*Device`. `PhysicalDevice` provides `capabilities()` / `hasCapability(cls)`, which introspect any device by walking its MRO for `Capability` subclasses — so every family gets capability introspection for free. -**Every capability follows the same template**, without exception: the public method is concrete and delegates to a `do*` hook, `getXxx()` calling `doGetXxx()`, and only the hook is `@abstractmethod`. A driver implements hooks and never overrides the public method, which is where argument validation and notifications live (or will). Hooks that are optional, or that default to a composition of the other hooks (`doAcquireWaveform`, `doGetDemodulatedValues`, `doGetSupported*`), are concrete but still carry the `do` prefix. `hardwarelibrary/tests/testCapabilities.py` enforces this: no public method on any capability may be abstract. +**Every capability follows the same template**, without exception: the public method is concrete and delegates to a `do*` hook, `getXxx()` calling `doGetXxx()`, and only the hook is `@abstractmethod`. A driver implements hooks and never overrides the public method, which is where argument validation and notifications live. Hooks that are optional, or that default to a composition of the other hooks (`doAcquireWaveform`, `doGetDemodulatedValues`, `doGetSupported*`), are concrete but still carry the `do` prefix. `hardwarelibrary/tests/testCapabilities.py` enforces this: no public method on any capability may be abstract. + +**Every capability posts notifications.** Each names its enum in its `notification` attribute, and every public method is wrapped in `@notifies(will=..., did=...)` from `capabilities.py`, so a driver gets notifications for free by implementing hooks. 17 enums cover the 22 capabilities. The rules, all enforced by `testCapabilities.py`: + +- An operation that changes the instrument posts `will` before and `did` after; a **read (`doGet*`, `doReadStream`) posts only `did`**, to keep hot paths (a voltage sampled in a loop) at one notification instead of two. `Spectrometer.getSpectrum` is the one read that keeps a `will`, because acquiring takes an integration time. +- **`did*` is posted whether the operation succeeded or not**, so a `will*` is always followed by its `did*` and there is no separate failure member to pair up. The **exception is still re-raised untouched** — 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). An observer decides what to do from the payload; a caller still sees the exception. +- `user_info` is a dict of the public method's arguments by name, plus `"result"` and `"error"`, exactly one of which is non-None. Test `user_info["error"]`, not the member, to tell success from failure. +- **Capabilities related by inheritance share one enum**, so `notification` is literally the same object on all of them and their members are interchangeable: `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability` and `AnalogInputStreamCapability` all post `AnalogNotification`; the digital trio posts `DigitalNotification`. This matters because members are keyed by identity — two same-named members in two enums would never cross-fire, so an observer would otherwise have to know whether a device mixed in the combined capability or the plain one. +- Members are keyed by enum identity, not by their string value, so `didFail` in all 17 enums does not collide. +- Composed hooks nest: `acquireWaveform` posts its own pair plus the pairs of the `configureStream` / `startStream` / `readStream` / `stopStream` calls its default implementation makes. - DAQ: `AnalogInputCapability` (`getAnalogVoltage`), `AnalogOutputCapability` (`setAnalogVoltage`), `DigitalInputCapability` (`getDigitalValue`), `DigitalOutputCapability` (`setDigitalValue`), plus `AnalogIOCapability` / `DigitalIOCapability` that combine each pair, and `AnalogInputStreamCapability` for hardware-timed acquisition. Drivers implement `doGetAnalogVoltage` / `doSetAnalogVoltage` / `doGetDigitalValue` / `doSetDigitalValue` and the streaming hooks; the `doConfigure*` and `do*Direction` hooks are optional no-ops. `configureStream(channels, sampleRate, **parameters)` forwards any extra keyword to `doConfigureStream`, which is how instrument-specific options reach a driver (the SR830's `sampleClock`). Example: `class LabjackDevice(PhysicalDevice, AnalogIOCapability, DigitalIOCapability, AnalogInputStreamCapability)`. - Laser sources: `OnOffCapability` (`turnOn`/`turnOff`/`isLaserOn`), `ShutterCapability` (`openShutter`/`closeShutter`/`isShutterOpen`), `PowerCapability` (`setPower`/`power`), `InterlockCapability` (`interlock`), `AutostartCapability`, `WavelengthCapability` (`setWavelength`/`wavelength`), `DispersionCapability`. `LaserSourceDevice` is a thin marker base; the behavior comes from the mixins. Examples: `class CoboltDevice(LaserSourceDevice, OnOffCapability, PowerCapability, ...)`, `class MillenniaEv25Device(LaserSourceDevice, OnOffCapability, ShutterCapability, PowerCapability)`, `class MatisseDevice(PhysicalDevice, WavelengthCapability)`. diff --git a/README.md b/README.md index be1d1142..b9615be6 100644 --- a/README.md +++ b/README.md @@ -398,25 +398,43 @@ cam.shutdownDevice() ### Listening for device events -All devices post notifications through the `NotificationCenter`. You can observe device events without polling: +All devices post notifications through the `NotificationCenter`, so you can observe what the hardware does without polling: ```python -from hardwarelibrary import NotificationCenter -from hardwarelibrary.motion.linearmotiondevice import LinearMotionNotification +from notificationcenter import NotificationCenter +from hardwarelibrary.capabilities import ShutterNotification -def onMove(notification): - print(f"Stage moved to {notification.userInfo}") +def onShutter(notification): + print("shutter opened on", notification.object) -nc = NotificationCenter() -nc.addObserver( +center = NotificationCenter() +center.add_observer( observer=self, - method=onMove, - notificationName=LinearMotionNotification.didMove, - observedObject=stage + method=onShutter, + notification_name=ShutterNotification.didOpenShutter, + observed_object=laser, # omit to hear it from every device ) ``` -Available notification enums include `PhysicalDeviceNotification`, `LinearMotionNotification`, `RotationMotionNotification`, `PowerMeterNotification`, `CameraDeviceNotification`, and `DeviceManagerNotification`. +**Every capability posts its own notifications**, and you get them for free: a driver only implements the `do` hooks, and the public method does the announcing. Each capability owns an enum, reachable as `ShutterCapability.notification`, following three rules: + +* an operation that **changes** the instrument posts `will...` before and `did...` after, e.g. `willOpenShutter` then `didOpenShutter`; +* a **read** posts only `did...`, e.g. `didGetPower` — bracketing a value that is merely being read would double the traffic on hot paths like a voltage sampled in a loop, for no added information. The exception is `SpectrometerNotification.willGetSpectrum`, because an acquisition takes an integration time and a display has something to show while it waits; +* the `did...` is posted **whether the operation worked or not**, so a `will...` is always followed by its `did...` and you never have to wonder whether an operation is still running. If the driver raised, the exception continues on its way untouched — your code still sees it — and the notification carries it. + +The payload in `notification.user_info` is a dict of the method's arguments by name, plus `"result"` and `"error"`. Exactly one of those two is set, which is how an observer tells the outcome: + +```python +def onPowerSet(notification): + if notification.user_info["error"] is not None: + log.warning("could not set the power: %s", notification.user_info["error"]) + else: + display.update(notification.user_info["power"]) +``` + +Capabilities related by inheritance share one enum, so you never have to know which variant a device mixed in: `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability` and `AnalogInputStreamCapability` all post `AnalogNotification`, and the three digital ones post `DigitalNotification`. Observing `AnalogNotification.didSetAnalogVoltage` catches the event from a LabJack (which mixes in the combined `AnalogIOCapability`) and from a lock-in amplifier (which mixes in only `AnalogOutputCapability`) alike. + +`python -m hardwarelibrary --capabilities` prints every capability with the notifications it posts. Beyond the capabilities, the device-wide enums are `PhysicalDeviceNotification`, `LinearMotionNotification`, `RotationMotionNotification`, `PowerMeterNotification`, `CameraDeviceNotification`, `SpectrometerNotification`, `DeviceControllerNotification`, and `DeviceManagerNotification`. ### Testing without hardware diff --git a/hardwarelibrary/__main__.py b/hardwarelibrary/__main__.py index 48b483c0..9727eb1b 100644 --- a/hardwarelibrary/__main__.py +++ b/hardwarelibrary/__main__.py @@ -90,6 +90,9 @@ def readableSignature(member): extends = ", ".join(klass.__name__ for klass in interface["extends"]) print("{0}{1}".format(capability.__name__, " (extends {0})".format(extends) if extends else "")) + print(" posts: {0} ({1})".format( + capability.notification.__name__, + ", ".join(member.name for member in capability.notification))) for member in interface["publicAPI"]: print(" {0}{1}{2}".format(member.name, readableSignature(member), " [must be implemented]" if member.isAbstract else "")) diff --git a/hardwarelibrary/capabilities.py b/hardwarelibrary/capabilities.py index 8691d4bd..d07c85f1 100644 --- a/hardwarelibrary/capabilities.py +++ b/hardwarelibrary/capabilities.py @@ -10,16 +10,88 @@ still carries the do prefix. Mixins carry the *Capability suffix; only instantiable hardware drivers are named *Device. +Each capability names its notification enum in its `notification` attribute, and +every public method is wrapped in @notifies so an observer hears about the +operation without the driver writing a line for it. An operation that changes the +instrument posts will* before and did* after; a read posts only did*, because +bracketing a value that is merely being read doubles the traffic on the hot paths +(a voltage sampled in a loop) for no added information. + +did* is posted whether the operation succeeded or not, so a will* is always +followed by its did*: the user_info carries "result" and "error", one of which is +None, and an observer decides what to do from the presence of an error. A hook that +raises still lets its exception through untouched, so a caller sees it as before. + +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 and AnalogInputStream all post AnalogNotification, and the +digital trio posts DigitalNotification. Sharing is what makes an observer of +AnalogNotification.didSetAnalogVoltage hear the post whether the device mixed in +AnalogOutputCapability or the combined AnalogIOCapability -- members are keyed by +identity, so two same-named members of two enums would never cross-fire. + PhysicalDevice.capabilities() / hasCapability() introspect these by walking the MRO for Capability subclasses, so every capability across every family must subclass the single Capability base defined here. """ +import functools import inspect from abc import ABC, abstractmethod from collections import namedtuple from enum import Enum +from notificationcenter import NotificationCenter + + +def notifies(did, will=None): + """Bracket a capability's public method with notifications. + + Posts `will` (when the operation changes the instrument) before the call and + `did` after it, whether the call succeeded or not: an observer that saw a + will always sees the matching did, so it never has to guess whether an + operation is still running. + + user_info carries the method's arguments by name, plus "result" and "error". + On success "error" is None; when the hook raises, "result" is None, "error" + holds the exception, and the exception is then re-raised as it was -- a + driver's own exception type is part of its contract, and the library must + not disguise it. An observer decides what to do from the presence of an + error; a caller still gets the exception. + """ + def decorator(method): + signature = inspect.signature(method) + takesArguments = len(signature.parameters) > 1 + + @functools.wraps(method) + def wrapper(self, *args, **keywordArguments): + arguments = {} + if takesArguments: + bound = signature.bind(self, *args, **keywordArguments) + bound.apply_defaults() + arguments = dict(bound.arguments) + arguments.pop("self") + + center = NotificationCenter() + if will is not None: + center.post_notification(will, notifying_object=self, + user_info=dict(arguments)) + try: + result = method(self, *args, **keywordArguments) + except Exception as error: + center.post_notification( + did, notifying_object=self, + user_info={**arguments, "result": None, "error": error}) + raise + center.post_notification( + did, notifying_object=self, + user_info={**arguments, "result": result, "error": None}) + return result + + return wrapper + + return decorator + class Capability(ABC): # Capability mixins are combined with a PhysicalDevice subclass, which sits @@ -27,20 +99,36 @@ class Capability(ABC): # super().__init__() after consuming the device-identity arguments), so a # mixin that holds per-instance state may define __init__ as long as it # takes no required arguments and forwards with super().__init__(). - pass + + # The Notification enum each mixin posts, so that + # allCapabilities() also enumerates every notification the library defines. + notification = None # --------------------------------------------------------------------------- # Laser source capabilities # --------------------------------------------------------------------------- +class OnOffNotification(Enum): + willTurnOn = "willTurnOn" + didTurnOn = "didTurnOn" + willTurnOff = "willTurnOff" + didTurnOff = "didTurnOff" + didGetOnOffState = "didGetOnOffState" + + class OnOffCapability(Capability): + notification = OnOffNotification + + @notifies(did=OnOffNotification.didGetOnOffState) def isLaserOn(self) -> bool: return self.doGetOnOffState() + @notifies(will=OnOffNotification.willTurnOn, did=OnOffNotification.didTurnOn) def turnOn(self): self.doTurnOn() + @notifies(will=OnOffNotification.willTurnOff, did=OnOffNotification.didTurnOff) def turnOff(self): self.doTurnOff() @@ -63,15 +151,28 @@ def doGetOnOffState(self) -> bool: ... +class ShutterNotification(Enum): + willOpenShutter = "willOpenShutter" + didOpenShutter = "didOpenShutter" + willCloseShutter = "willCloseShutter" + didCloseShutter = "didCloseShutter" + didGetShutterState = "didGetShutterState" + + class ShutterCapability(Capability): # Distinct from OnOffCapability: the shutter is a mechanical block in front of # the output, so it can be opened or closed while the laser stays on. + notification = ShutterNotification + + @notifies(did=ShutterNotification.didGetShutterState) def isShutterOpen(self) -> bool: return self.doGetShutterState() + @notifies(will=ShutterNotification.willOpenShutter, did=ShutterNotification.didOpenShutter) def openShutter(self): self.doOpenShutter() + @notifies(will=ShutterNotification.willCloseShutter, did=ShutterNotification.didCloseShutter) def closeShutter(self): self.doCloseShutter() @@ -88,14 +189,23 @@ def doGetShutterState(self) -> bool: ... +class PowerNotification(Enum): + willSetPower = "willSetPower" + didSetPower = "didSetPower" + didGetPower = "didGetPower" + + class PowerCapability(Capability): unit = "W" isReadable = True isWritable = True + notification = PowerNotification + @notifies(will=PowerNotification.willSetPower, did=PowerNotification.didSetPower) def setPower(self, power: float): return self.doSetPower(power) + @notifies(did=PowerNotification.didGetPower) def power(self) -> float: return self.doGetPower() @@ -108,10 +218,16 @@ def doGetPower(self) -> float: ... +class InterlockNotification(Enum): + didGetInterlockState = "didGetInterlockState" + + class InterlockCapability(Capability): isReadable = True isWritable = False + notification = InterlockNotification + @notifies(did=InterlockNotification.didGetInterlockState) def interlock(self) -> bool: return self.doGetInterlockState() @@ -120,13 +236,28 @@ def doGetInterlockState(self) -> bool: ... +class AutostartNotification(Enum): + willTurnAutostartOn = "willTurnAutostartOn" + didTurnAutostartOn = "didTurnAutostartOn" + willTurnAutostartOff = "willTurnAutostartOff" + didTurnAutostartOff = "didTurnAutostartOff" + didGetAutostart = "didGetAutostart" + + class AutostartCapability(Capability): + notification = AutostartNotification + + @notifies(did=AutostartNotification.didGetAutostart) def autostartIsOn(self) -> bool: return self.doGetAutostart() + @notifies(will=AutostartNotification.willTurnAutostartOn, + did=AutostartNotification.didTurnAutostartOn) def turnAutostartOn(self): self.doTurnAutostartOn() + @notifies(will=AutostartNotification.willTurnAutostartOff, + did=AutostartNotification.didTurnAutostartOff) def turnAutostartOff(self): self.doTurnAutostartOff() @@ -143,17 +274,29 @@ def doTurnAutostartOff(self): ... +class WavelengthNotification(Enum): + willSetWavelength = "willSetWavelength" + didSetWavelength = "didSetWavelength" + didGetWavelength = "didGetWavelength" + didGetWavelengthRange = "didGetWavelengthRange" + + class WavelengthCapability(Capability): unit = "nm" isReadable = True isWritable = True + notification = WavelengthNotification + @notifies(will=WavelengthNotification.willSetWavelength, + did=WavelengthNotification.didSetWavelength) def setWavelength(self, wavelength: float): return self.doSetWavelength(wavelength) + @notifies(did=WavelengthNotification.didGetWavelength) def wavelength(self) -> float: return self.doGetWavelength() + @notifies(did=WavelengthNotification.didGetWavelengthRange) def wavelengthRange(self) -> tuple: return self.doGetWavelengthRange() @@ -170,17 +313,29 @@ def doGetWavelengthRange(self) -> tuple: ... +class DispersionNotification(Enum): + willSetDispersion = "willSetDispersion" + didSetDispersion = "didSetDispersion" + didGetDispersion = "didGetDispersion" + didGetDispersionRange = "didGetDispersionRange" + + class DispersionCapability(Capability): unit = "fs^2" # group delay dispersion (GDD) isReadable = True isWritable = True + notification = DispersionNotification + @notifies(will=DispersionNotification.willSetDispersion, + did=DispersionNotification.didSetDispersion) def setDispersion(self, dispersion: float): return self.doSetDispersion(dispersion) + @notifies(did=DispersionNotification.didGetDispersion) def dispersion(self) -> float: return self.doGetDispersion() + @notifies(did=DispersionNotification.didGetDispersionRange) def dispersionRange(self) -> tuple: return self.doGetDispersionRange() @@ -201,19 +356,29 @@ def doGetDispersionRange(self) -> tuple: # Power meter capabilities # --------------------------------------------------------------------------- +class WavelengthCalibrationNotification(Enum): + willSetCalibrationWavelength = "willSetCalibrationWavelength" + didSetCalibrationWavelength = "didSetCalibrationWavelength" + didGetCalibrationWavelength = "didGetCalibrationWavelength" + + class WavelengthCalibrationCapability(Capability): unit = "nm" isReadable = True isWritable = True + notification = WavelengthCalibrationNotification def __init__(self): super().__init__() self.calibrationWavelength = None + @notifies(did=WavelengthCalibrationNotification.didGetCalibrationWavelength) def getCalibrationWavelength(self): self.doGetCalibrationWavelength() return self.calibrationWavelength + @notifies(will=WavelengthCalibrationNotification.willSetCalibrationWavelength, + did=WavelengthCalibrationNotification.didSetCalibrationWavelength) def setCalibrationWavelength(self, wavelength): self.doSetCalibrationWavelength(wavelength) self.doGetCalibrationWavelength() @@ -227,16 +392,31 @@ def doSetCalibrationWavelength(self, wavelength): ... +class AutoScaleNotification(Enum): + willTurnAutoScaleOn = "willTurnAutoScaleOn" + didTurnAutoScaleOn = "didTurnAutoScaleOn" + willTurnAutoScaleOff = "willTurnAutoScaleOff" + didTurnAutoScaleOff = "didTurnAutoScaleOff" + didGetAutoScale = "didGetAutoScale" + + class AutoScaleCapability(Capability): # The meter picks its measurement range automatically when auto-scaling is # on; turning it off pins the range to whatever scale is active. A meter may # also expose ScaleCapability to choose that range by hand. + notification = AutoScaleNotification + + @notifies(did=AutoScaleNotification.didGetAutoScale) def autoScaleIsOn(self) -> bool: return self.doGetAutoScale() + @notifies(will=AutoScaleNotification.willTurnAutoScaleOn, + did=AutoScaleNotification.didTurnAutoScaleOn) def turnAutoScaleOn(self): self.doTurnAutoScaleOn() + @notifies(will=AutoScaleNotification.willTurnAutoScaleOff, + did=AutoScaleNotification.didTurnAutoScaleOff) def turnAutoScaleOff(self): self.doTurnAutoScaleOff() @@ -253,6 +433,13 @@ def doTurnAutoScaleOff(self): ... +class ScaleNotification(Enum): + willSetScale = "willSetScale" + didSetScale = "didSetScale" + didGetScale = "didGetScale" + didGetAvailableScales = "didGetAvailableScales" + + class ScaleCapability(Capability): # The full-scale measurement range (e.g. 200e-3 W). Independent of # AutoScaleCapability: setting a scale by hand generally requires auto-scaling @@ -260,19 +447,23 @@ class ScaleCapability(Capability): unit = "W" isReadable = True isWritable = True + notification = ScaleNotification def __init__(self): super().__init__() self.scale = None + @notifies(did=ScaleNotification.didGetScale) def getScale(self): self.doGetScale() return self.scale + @notifies(will=ScaleNotification.willSetScale, did=ScaleNotification.didSetScale) def setScale(self, scale): self.doSetScale(scale) self.doGetScale() + @notifies(did=ScaleNotification.didGetAvailableScales) def availableScales(self) -> list: return self.doGetAvailableScales() @@ -293,9 +484,37 @@ def doGetAvailableScales(self) -> list: # DAQ capabilities # --------------------------------------------------------------------------- +class AnalogNotification(Enum): + """Posted by every analog capability: input, output, the combined IO, and + streaming. One enum for the whole inheritance chain, so a device mixing in + AnalogIOCapability and the plain AnalogOutputCapability post the very same + member and an observer registers once.""" + + didGetAnalogVoltage = "didGetAnalogVoltage" + willSetAnalogVoltage = "willSetAnalogVoltage" + didSetAnalogVoltage = "didSetAnalogVoltage" + willConfigureAnalogIO = "willConfigureAnalogIO" + didConfigureAnalogIO = "didConfigureAnalogIO" + willSetAnalogDirection = "willSetAnalogDirection" + didSetAnalogDirection = "didSetAnalogDirection" + didGetAnalogDirection = "didGetAnalogDirection" + willConfigureStream = "willConfigureStream" + didConfigureStream = "didConfigureStream" + willStartStream = "willStartStream" + didStartStream = "didStartStream" + willStopStream = "willStopStream" + didStopStream = "didStopStream" + willAcquireWaveform = "willAcquireWaveform" + didAcquireWaveform = "didAcquireWaveform" + didReadStream = "didReadStream" + + class AnalogInputCapability(Capability): """Analog input capability (ADC). Combine with PhysicalDevice in a driver.""" + notification = AnalogNotification + + @notifies(did=AnalogNotification.didGetAnalogVoltage) def getAnalogVoltage(self, channel): """Returns the voltage measured on channel, in volts.""" return self.doGetAnalogVoltage(channel) @@ -308,6 +527,10 @@ def doGetAnalogVoltage(self, channel): class AnalogOutputCapability(Capability): """Analog output capability (DAC). Combine with PhysicalDevice in a driver.""" + notification = AnalogNotification + + @notifies(will=AnalogNotification.willSetAnalogVoltage, + did=AnalogNotification.didSetAnalogVoltage) def setAnalogVoltage(self, value, channel): """Set the output on channel to value, in volts.""" return self.doSetAnalogVoltage(value, channel) @@ -323,14 +546,21 @@ class AnalogIOCapability(AnalogInputCapability, AnalogOutputCapability): The configure and direction hooks are optional and default to no-ops. """ + notification = AnalogNotification + + @notifies(will=AnalogNotification.willConfigureAnalogIO, + did=AnalogNotification.didConfigureAnalogIO) def configureAnalogIO(self, parameters: dict): """Apply the driver-specific analog configuration in parameters.""" return self.doConfigureAnalogIO(parameters) + @notifies(did=AnalogNotification.didGetAnalogDirection) def getAnalogDirection(self, channel): """Returns whether channel is configured as an input or an output.""" return self.doGetAnalogDirection(channel) + @notifies(will=AnalogNotification.willSetAnalogDirection, + did=AnalogNotification.didSetAnalogDirection) def setAnalogDirection(self, channel): """Configure the direction of channel.""" return self.doSetAnalogDirection(channel) @@ -371,10 +601,10 @@ class AnalogInputStreamCapability(AnalogInputCapability): device.stopStream() """ - class Notification(Enum): - willAcquire = "willAcquire" - didAcquire = "didAcquire" + notification = AnalogNotification + @notifies(will=AnalogNotification.willConfigureStream, + did=AnalogNotification.didConfigureStream) def configureStream(self, channels, sampleRate=None, **parameters): """Set up a hardware-timed acquisition of channels at sampleRate (Hz). @@ -385,18 +615,25 @@ def configureStream(self, channels, sampleRate=None, **parameters): """ return self.doConfigureStream(channels, sampleRate, **parameters) + @notifies(will=AnalogNotification.willStartStream, + did=AnalogNotification.didStartStream) def startStream(self): """Start the configured acquisition.""" return self.doStartStream() + @notifies(did=AnalogNotification.didReadStream) def readStream(self): """Returns the samples acquired since the last read, as {channel: [volts, ...]}.""" return self.doReadStream() + @notifies(will=AnalogNotification.willStopStream, + did=AnalogNotification.didStopStream) def stopStream(self): """Stop the acquisition and release any hardware streaming resources.""" return self.doStopStream() + @notifies(will=AnalogNotification.willAcquireWaveform, + did=AnalogNotification.didAcquireWaveform) def acquireWaveform(self, channels, sampleRate, sampleCount): """Acquire exactly sampleCount samples per channel, blocking until done. @@ -453,6 +690,27 @@ class InputSource(Enum): Current100M = "Current100M" +class PhaseLockedDetectionNotification(Enum): + willSetInputSource = "willSetInputSource" + didSetInputSource = "didSetInputSource" + willSetSensitivity = "willSetSensitivity" + didSetSensitivity = "didSetSensitivity" + willSetTimeConstant = "willSetTimeConstant" + didSetTimeConstant = "didSetTimeConstant" + didGetInPhaseVoltage = "didGetInPhaseVoltage" + didGetQuadratureVoltage = "didGetQuadratureVoltage" + didGetMagnitude = "didGetMagnitude" + didGetPhase = "didGetPhase" + didGetReferenceFrequency = "didGetReferenceFrequency" + didGetInputSource = "didGetInputSource" + didGetSensitivity = "didGetSensitivity" + didGetTimeConstant = "didGetTimeConstant" + didGetSupportedInputSources = "didGetSupportedInputSources" + didGetSupportedSensitivities = "didGetSupportedSensitivities" + didGetSupportedTimeConstants = "didGetSupportedTimeConstants" + didGetDemodulatedValues = "didGetDemodulatedValues" + + class PhaseLockedDetectionCapability(Capability): """Phase-locked (lock-in) detection capability. Combine with PhysicalDevice. @@ -463,62 +721,82 @@ class PhaseLockedDetectionCapability(Capability): contract; a driver snaps a requested value to its nearest supported step. """ + notification = PhaseLockedDetectionNotification + + @notifies(did=PhaseLockedDetectionNotification.didGetInPhaseVoltage) def getInPhaseVoltage(self): """Returns the in-phase component X, in volts.""" return self.doGetInPhaseVoltage() + @notifies(did=PhaseLockedDetectionNotification.didGetQuadratureVoltage) def getQuadratureVoltage(self): """Returns the quadrature component Y, in volts.""" return self.doGetQuadratureVoltage() + @notifies(did=PhaseLockedDetectionNotification.didGetMagnitude) def getMagnitude(self): """Returns the magnitude R = sqrt(X^2 + Y^2), in volts.""" return self.doGetMagnitude() + @notifies(did=PhaseLockedDetectionNotification.didGetPhase) def getPhase(self): """Returns the phase theta, in degrees.""" return self.doGetPhase() + @notifies(did=PhaseLockedDetectionNotification.didGetReferenceFrequency) def getReferenceFrequency(self): """Returns the reference frequency, in Hz.""" return self.doGetReferenceFrequency() + @notifies(did=PhaseLockedDetectionNotification.didGetInputSource) def getInputSource(self) -> InputSource: """Returns the signal input the demodulator currently measures.""" return self.doGetInputSource() + @notifies(will=PhaseLockedDetectionNotification.willSetInputSource, + did=PhaseLockedDetectionNotification.didSetInputSource) def setInputSource(self, source: InputSource): """Select which signal input (an InputSource member) the demodulator measures.""" return self.doSetInputSource(source) + @notifies(did=PhaseLockedDetectionNotification.didGetSensitivity) def getSensitivity(self): """Returns the full-scale sensitivity, in volts.""" return self.doGetSensitivity() + @notifies(will=PhaseLockedDetectionNotification.willSetSensitivity, + did=PhaseLockedDetectionNotification.didSetSensitivity) def setSensitivity(self, volts): """Set the full-scale sensitivity to the nearest supported step, in volts.""" return self.doSetSensitivity(volts) + @notifies(did=PhaseLockedDetectionNotification.didGetTimeConstant) def getTimeConstant(self): """Returns the time constant, in seconds.""" return self.doGetTimeConstant() + @notifies(will=PhaseLockedDetectionNotification.willSetTimeConstant, + did=PhaseLockedDetectionNotification.didSetTimeConstant) def setTimeConstant(self, seconds): """Set the time constant to the nearest supported step, in seconds.""" return self.doSetTimeConstant(seconds) + @notifies(did=PhaseLockedDetectionNotification.didGetSupportedInputSources) def supportedInputSources(self): """Returns the InputSource members this instrument supports, or None.""" return self.doGetSupportedInputSources() + @notifies(did=PhaseLockedDetectionNotification.didGetSupportedSensitivities) def supportedSensitivities(self): """Returns the full-scale sensitivities (volts) this instrument supports, or None.""" return self.doGetSupportedSensitivities() + @notifies(did=PhaseLockedDetectionNotification.didGetSupportedTimeConstants) def supportedTimeConstants(self): """Returns the time constants (seconds) this instrument supports, or None.""" return self.doGetSupportedTimeConstants() + @notifies(did=PhaseLockedDetectionNotification.didGetDemodulatedValues) def getDemodulatedValues(self): """One reading of all demodulated outputs plus the reference frequency.""" return self.doGetDemodulatedValues() @@ -614,6 +892,15 @@ class SampleClock(Enum): External = "External" +class TriggerNotification(Enum): + willSetTriggerSource = "willSetTriggerSource" + didSetTriggerSource = "didSetTriggerSource" + willSoftwareTrigger = "willSoftwareTrigger" + didSoftwareTrigger = "didSoftwareTrigger" + didGetTriggerSource = "didGetTriggerSource" + didGetSupportedTriggerSources = "didGetSupportedTriggerSources" + + class TriggerCapability(Capability): """Capability for a device whose acquisition can be armed to a trigger. @@ -623,18 +910,26 @@ class TriggerCapability(Capability): PhysicalDevice in a driver. """ + notification = TriggerNotification + + @notifies(will=TriggerNotification.willSetTriggerSource, + did=TriggerNotification.didSetTriggerSource) def setTriggerSource(self, source: 'TriggerSource'): """Select whether the acquisition starts immediately or on an external trigger.""" return self.doSetTriggerSource(source) + @notifies(did=TriggerNotification.didGetTriggerSource) def getTriggerSource(self) -> 'TriggerSource': """Returns the currently selected TriggerSource.""" return self.doGetTriggerSource() + @notifies(will=TriggerNotification.willSoftwareTrigger, + did=TriggerNotification.didSoftwareTrigger) def softwareTrigger(self): """Issue a manual (software) trigger edge.""" return self.doSoftwareTrigger() + @notifies(did=TriggerNotification.didGetSupportedTriggerSources) def supportedTriggerSources(self): """Returns the TriggerSource members this device supports, or None.""" return self.doGetSupportedTriggerSources() @@ -656,9 +951,26 @@ def doGetSupportedTriggerSources(self): return None +class DigitalNotification(Enum): + """Posted by every digital capability: input, output, and the combined IO. + One enum for the whole inheritance chain (see AnalogNotification).""" + + didGetDigitalValue = "didGetDigitalValue" + willSetDigitalValue = "willSetDigitalValue" + didSetDigitalValue = "didSetDigitalValue" + willConfigureDigitalIO = "willConfigureDigitalIO" + didConfigureDigitalIO = "didConfigureDigitalIO" + willSetDigitalDirection = "willSetDigitalDirection" + didSetDigitalDirection = "didSetDigitalDirection" + didGetDigitalDirection = "didGetDigitalDirection" + + class DigitalInputCapability(Capability): """Digital input capability. Combine with PhysicalDevice in a driver.""" + notification = DigitalNotification + + @notifies(did=DigitalNotification.didGetDigitalValue) def getDigitalValue(self, channel): """Returns the logic level read on channel.""" return self.doGetDigitalValue(channel) @@ -671,6 +983,10 @@ def doGetDigitalValue(self, channel): class DigitalOutputCapability(Capability): """Digital output capability. Combine with PhysicalDevice in a driver.""" + notification = DigitalNotification + + @notifies(will=DigitalNotification.willSetDigitalValue, + did=DigitalNotification.didSetDigitalValue) def setDigitalValue(self, value, channel): """Drive channel to the logic level value.""" return self.doSetDigitalValue(value, channel) @@ -686,14 +1002,21 @@ class DigitalIOCapability(DigitalInputCapability, DigitalOutputCapability): The configure and direction hooks are optional and default to no-ops. """ + notification = DigitalNotification + + @notifies(will=DigitalNotification.willConfigureDigitalIO, + did=DigitalNotification.didConfigureDigitalIO) def configureDigitalIO(self, parameters: dict): """Apply the driver-specific digital configuration in parameters.""" return self.doConfigureDigitalIO(parameters) + @notifies(did=DigitalNotification.didGetDigitalDirection) def getDigitalDirection(self, channel): """Returns whether channel is configured as an input or an output.""" return self.doGetDigitalDirection(channel) + @notifies(will=DigitalNotification.willSetDigitalDirection, + did=DigitalNotification.didSetDigitalDirection) def setDigitalDirection(self, channel): """Configure the direction of channel.""" return self.doSetDigitalDirection(channel) @@ -713,27 +1036,47 @@ def doSetDigitalDirection(self, channel): # --------------------------------------------------------------------------- +class OutletSwitchingNotification(Enum): + willSetOutletState = "willSetOutletState" + didSetOutletState = "didSetOutletState" + didGetOutletState = "didGetOutletState" + didGetOutletCount = "didGetOutletCount" + + class OutletSwitchingCapability(Capability): """Switch individual outlets on and off and read their state. Outlets are addressed by their physical label (1-based): the first switchable outlet is outlet 1. Some strips also carry an always-on outlet that is not switchable and is not counted here. + + turnOutletOn, turnOutletOff and setOutletState share one hook, so they share + one will/did pair; the outlet and its requested state are in the user_info. """ + notification = OutletSwitchingNotification + + @notifies(will=OutletSwitchingNotification.willSetOutletState, + did=OutletSwitchingNotification.didSetOutletState) def turnOutletOn(self, outlet: int): self.doSetOutletState(outlet, True) + @notifies(will=OutletSwitchingNotification.willSetOutletState, + did=OutletSwitchingNotification.didSetOutletState) def turnOutletOff(self, outlet: int): self.doSetOutletState(outlet, False) + @notifies(will=OutletSwitchingNotification.willSetOutletState, + did=OutletSwitchingNotification.didSetOutletState) def setOutletState(self, outlet: int, isOn: bool): self.doSetOutletState(outlet, isOn) + @notifies(did=OutletSwitchingNotification.didGetOutletState) def isOutletOn(self, outlet: int) -> bool: return self.doGetOutletState(outlet) @property + @notifies(did=OutletSwitchingNotification.didGetOutletCount) def outletCount(self) -> int: return self.doGetOutletCount() @@ -750,6 +1093,11 @@ def doGetOutletCount(self) -> int: ... +class DefaultOutletNotification(Enum): + willSetOutletDefaultState = "willSetOutletDefaultState" + didSetOutletDefaultState = "didSetOutletDefaultState" + + class DefaultOutletCapability(Capability): """Set the power-on (boot) state of individual outlets. @@ -758,12 +1106,20 @@ class DefaultOutletCapability(Capability): state right now. """ + notification = DefaultOutletNotification + + @notifies(will=DefaultOutletNotification.willSetOutletDefaultState, + did=DefaultOutletNotification.didSetOutletDefaultState) def setOutletDefaultOn(self, outlet: int): self.doSetOutletDefaultState(outlet, True) + @notifies(will=DefaultOutletNotification.willSetOutletDefaultState, + did=DefaultOutletNotification.didSetOutletDefaultState) def setOutletDefaultOff(self, outlet: int): self.doSetOutletDefaultState(outlet, False) + @notifies(will=DefaultOutletNotification.willSetOutletDefaultState, + did=DefaultOutletNotification.didSetOutletDefaultState) def setOutletDefaultState(self, outlet: int, isOn: bool): self.doSetOutletDefaultState(outlet, isOn) @@ -772,6 +1128,13 @@ def doSetOutletDefaultState(self, outlet: int, isOn: bool): ... +class CurrentMeteringNotification(Enum): + willResetAccumulatedCharge = "willResetAccumulatedCharge" + didResetAccumulatedCharge = "didResetAccumulatedCharge" + didGetCurrent = "didGetCurrent" + didGetAccumulatedCharge = "didGetAccumulatedCharge" + + class CurrentMeteringCapability(Capability): """Measure the strip's total current draw and accumulated charge. @@ -784,13 +1147,18 @@ class CurrentMeteringCapability(Capability): unit = "A" isReadable = True isWritable = False + notification = CurrentMeteringNotification + @notifies(did=CurrentMeteringNotification.didGetCurrent) def current(self) -> float: return self.doGetCurrent() + @notifies(did=CurrentMeteringNotification.didGetAccumulatedCharge) def accumulatedCharge(self) -> float: return self.doGetAccumulatedCharge() + @notifies(will=CurrentMeteringNotification.willResetAccumulatedCharge, + did=CurrentMeteringNotification.didResetAccumulatedCharge) def resetAccumulatedCharge(self): self.doResetAccumulatedCharge() diff --git a/hardwarelibrary/spectrometers/base.py b/hardwarelibrary/spectrometers/base.py index 6c6198ab..152ade8c 100644 --- a/hardwarelibrary/spectrometers/base.py +++ b/hardwarelibrary/spectrometers/base.py @@ -16,9 +16,19 @@ import usb.util import usb.backend.libusb1 +from enum import Enum from pathlib import * +from hardwarelibrary.capabilities import notifies from hardwarelibrary.physicaldevice import PhysicalDevice, DeviceState +class SpectrometerNotification(Enum): + # getSpectrum keeps a will, unlike the other reads in the library: acquiring + # a spectrum takes an integration time, so a display has something to show + # while it waits. + willGetSpectrum = "willGetSpectrum" + didGetSpectrum = "didGetSpectrum" + didGetSerialNumber = "didGetSerialNumber" + class NoSpectrometerConnected(RuntimeError): pass class UnableToInitialize(RuntimeError): @@ -39,10 +49,13 @@ def __init__(self, serialNumber=None, idProduct:int = None, idVendor:int = None) self.wavelength = np.linspace(400,1000,1024) self.integrationTime = 10 + @notifies(did=SpectrometerNotification.didGetSerialNumber) def getSerialNumber(self): """Returns the serial number, which tells two connected spectrometers apart.""" return self.doGetSerialNumber() + @notifies(will=SpectrometerNotification.willGetSpectrum, + did=SpectrometerNotification.didGetSpectrum) def getSpectrum(self, **parameters) -> np.array: """Returns one spectrum, as an array of intensities. diff --git a/hardwarelibrary/tests/testCapabilities.py b/hardwarelibrary/tests/testCapabilities.py index 9f380bd5..0f22c25a 100644 --- a/hardwarelibrary/tests/testCapabilities.py +++ b/hardwarelibrary/tests/testCapabilities.py @@ -2,6 +2,7 @@ import os import sys import unittest +from enum import Enum import hardwarelibrary.daq import hardwarelibrary.powermeters @@ -12,8 +13,9 @@ Capability, allCapabilities, capabilityInterface, OnOffCapability, ShutterCapability, PowerCapability, AnalogInputCapability, AnalogOutputCapability, AnalogIOCapability, - OutletSwitchingCapability) + AnalogNotification, OutletSwitchingCapability) from hardwarelibrary.physicaldevice import PhysicalDevice +from notificationcenter import NotificationCenter testsDirectory = os.path.dirname(os.path.abspath(__file__)) @@ -201,5 +203,171 @@ def testExtendsReportsTheCapabilitiesCombined(self): self.assertEqual(capabilityInterface(OnOffCapability)["extends"], []) +class _FailingAnalogDevice(AnalogIOCapability): + """Every hook raises, so the failure path can be exercised.""" + + class Failure(RuntimeError): + pass + + def doGetAnalogVoltage(self, channel): + raise self.Failure("no hardware") + + def doSetAnalogVoltage(self, value, channel): + raise self.Failure("no hardware") + + +class NotificationRecorder: + """Collects every notification a capability posts, in order.""" + + def __init__(self): + self.received = [] + + def observe(self, notificationEnum): + for member in notificationEnum: + NotificationCenter().add_observer(self, self.record, member) + + def record(self, notification): + self.received.append(notification) + + def names(self): + return [notification.name.name for notification in self.received] + + def stop(self): + NotificationCenter().remove_observer(self) + + +class TestCapabilityNotifications(unittest.TestCase): + def setUp(self): + self.recorder = NotificationRecorder() + + def tearDown(self): + self.recorder.stop() + + def testEveryCapabilityOwnsANotificationEnum(self): + for capability in allCapabilities(): + self.assertTrue(issubclass(capability.notification, Enum), capability.__name__) + + def testNoEnumCarriesASeparateFailureMember(self): + # A failure is reported through the operation's own did*, so a will* is + # always followed by its did* and an observer never has to pair two + # different members to know an operation ended. + for capability in allCapabilities(): + self.assertNotIn("didFail", capability.notification.__members__, + capability.__name__) + + def testEveryHookHasItsDidNotification(self): + for capability in allCapabilities(): + members = capability.notification.__members__ + for hook in capabilityInterface(capability)["hooks"]: + stem = hook.name[len("do"):] + self.assertIn("did" + stem, members, + "{0}.{1}".format(capability.__name__, hook.name)) + + def testOnlyOperationsThatChangeTheInstrumentHaveAWillNotification(self): + # A read posts did only: bracketing a value being read doubles the + # traffic on the hot paths for no added information. + for capability in allCapabilities(): + members = capability.notification.__members__ + for hook in capabilityInterface(capability)["hooks"]: + stem = hook.name[len("do"):] + isRead = hook.name.startswith("doGet") or hook.name.startswith("doRead") + self.assertEqual("will" + stem not in members, isRead, + "{0}.{1}".format(capability.__name__, hook.name)) + + def testNoNotificationIsDefinedForAHookThatDoesNotExist(self): + # An enum shared by a family of capabilities carries the members of every + # hook in that family, so the stems are checked against their union. + stemsByEnum = {} + for capability in allCapabilities(): + stems = stemsByEnum.setdefault(capability.notification, set()) + stems.update(hook.name[len("do"):] + for hook in capabilityInterface(capability)["hooks"]) + for notificationEnum, hookStems in stemsByEnum.items(): + for memberName in notificationEnum.__members__: + stem = memberName[len("will"):] if memberName.startswith("will") \ + else memberName[len("did"):] + self.assertIn(stem, hookStems, + "{0}.{1}".format(notificationEnum.__name__, memberName)) + + def testCapabilitiesInOneInheritanceChainShareOneEnum(self): + # Sharing is what makes the members interchangeable: a device mixing in + # AnalogIOCapability posts the same member as one mixing in only + # AnalogOutputCapability, so an observer registers once. + for capability in allCapabilities(): + for other in allCapabilities(): + if capability is not other and issubclass(capability, other): + self.assertIs(capability.notification, other.notification, + "{0} vs {1}".format(capability.__name__, other.__name__)) + + def testAnActionPostsWillThenDidWithItsArgumentsAndResult(self): + self.recorder.observe(AnalogNotification) + device = _RecordingAnalogDevice() + device.setAnalogVoltage(2.5, channel=1) + self.assertEqual(self.recorder.names(), + ["willSetAnalogVoltage", "didSetAnalogVoltage"]) + willNotification, didNotification = self.recorder.received + self.assertEqual(willNotification.user_info, {"value": 2.5, "channel": 1}) + self.assertEqual(didNotification.user_info, + {"value": 2.5, "channel": 1, "result": None, "error": None}) + + def testAReadPostsDidOnly(self): + self.recorder.observe(AnalogNotification) + device = _RecordingAnalogDevice() + device.getAnalogVoltage(3) + self.assertEqual(self.recorder.names(), ["didGetAnalogVoltage"]) + self.assertEqual(self.recorder.received[0].user_info, + {"channel": 3, "result": 1.5, "error": None}) + + def testTheNotifyingObjectIsTheDevice(self): + self.recorder.observe(AnalogNotification) + device = _RecordingAnalogDevice() + device.getAnalogVoltage(0) + self.assertIs(self.recorder.received[0].object, device) + + def testAFailingHookStillPostsDidCarryingTheErrorAndReRaisesUntouched(self): + self.recorder.observe(AnalogNotification) + device = _FailingAnalogDevice() + with self.assertRaises(_FailingAnalogDevice.Failure): + device.setAnalogVoltage(2.5, channel=1) + self.assertEqual(self.recorder.names(), + ["willSetAnalogVoltage", "didSetAnalogVoltage"]) + payload = self.recorder.received[-1].user_info + self.assertEqual(payload["value"], 2.5) + self.assertEqual(payload["channel"], 1) + self.assertIsNone(payload["result"]) + self.assertIsInstance(payload["error"], _FailingAnalogDevice.Failure) + + def testAFailingReadPostsItsDidWithTheError(self): + self.recorder.observe(AnalogNotification) + device = _FailingAnalogDevice() + with self.assertRaises(_FailingAnalogDevice.Failure): + device.getAnalogVoltage(0) + self.assertEqual(self.recorder.names(), ["didGetAnalogVoltage"]) + self.assertIsInstance(self.recorder.received[0].user_info["error"], + _FailingAnalogDevice.Failure) + + def testAnObserverTellsSuccessFromFailureByTheErrorAlone(self): + self.recorder.observe(AnalogNotification) + _RecordingAnalogDevice().getAnalogVoltage(0) + try: + _FailingAnalogDevice().getAnalogVoltage(0) + except _FailingAnalogDevice.Failure: + pass + succeeded, failed = self.recorder.received + self.assertIs(succeeded.name, failed.name) + self.assertIsNone(succeeded.user_info["error"]) + self.assertIsNotNone(failed.user_info["error"]) + + def testDecoratedMethodsKeepTheirIdentity(self): + # capabilityInterface and the docs read the public signature, so the + # decorator must not replace it with (*args, **kwargs). + interface = capabilityInterface(AnalogOutputCapability) + member = interface["publicAPI"][0] + self.assertEqual(member.name, "setAnalogVoltage") + self.assertEqual(member.signature, "(value, channel)") + self.assertEqual(AnalogOutputCapability.setAnalogVoltage.__doc__, + "Set the output on channel to value, in volts.") + + if __name__ == "__main__": unittest.main() From 6da1193eae46f439d1a40c1c588a614938bc69e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Tue, 4 Aug 2026 14:21:38 -0400 Subject: [PATCH 4/7] Align the family-base notifications with the capability scheme 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) --- .claude/skills/pyhardwarelibrary/SKILL.md | 25 ++++++--- CHANGELOG.md | 17 +++++++ README.md | 4 +- hardwarelibrary/motion/linearmotiondevice.py | 51 ++++++------------- hardwarelibrary/motion/rotationdevice.py | 25 +++++---- .../powermeters/powermeterdevice.py | 11 ++-- hardwarelibrary/spectrometers/base.py | 1 + hardwarelibrary/tests/testCapabilities.py | 41 +++++++++++++++ .../tests/testFieldMasterDevice.py | 4 +- 9 files changed, 120 insertions(+), 59 deletions(-) diff --git a/.claude/skills/pyhardwarelibrary/SKILL.md b/.claude/skills/pyhardwarelibrary/SKILL.md index b375930d..0cdcdb4c 100644 --- a/.claude/skills/pyhardwarelibrary/SKILL.md +++ b/.claude/skills/pyhardwarelibrary/SKILL.md @@ -252,20 +252,33 @@ Devices post Cocoa-style notifications (state changes, measurements, moves) inst requiring polling. Observe them from anywhere: ```python -from hardwarelibrary.notificationcenter import NotificationCenter +from notificationcenter import NotificationCenter from hardwarelibrary.powermeters.powermeterdevice import PowerMeterNotification def onMeasure(notification): - print("power:", notification.userInfo) + if notification.user_info["error"] is None: + print("power:", notification.user_info["result"]) -NotificationCenter().addObserver(self, onMeasure, PowerMeterNotification.didMeasure) -# ... measurements now call onMeasure with the value in notification.userInfo -NotificationCenter().removeObserver(self) +NotificationCenter().add_observer(self, onMeasure, PowerMeterNotification.didGetAbsolutePower) +# ... measurements now call onMeasure +NotificationCenter().remove_observer(self) ``` +Every capability posts notifications too, named after the hook: an operation that +changes the instrument posts `will` then `did`, a read posts `did` +only, and the `did` is posted even when the driver raised, with the exception under +`user_info["error"]` (the exception itself still propagates to the caller). The +payload also carries the method's arguments by name and the return value under +`"result"`. Each capability names its enum in its `notification` attribute, and +capabilities related by inheritance share one enum, so `AnalogNotification` covers +analog input, output, IO and streaming alike. `python -m hardwarelibrary +--capabilities` lists them all. + Useful notification enums: `PhysicalDeviceNotification` (will/did initialize/shutdown, status), `LinearMotionNotification` (willMove/didMove/didGetPosition), -`PowerMeterNotification.didMeasure`. +`RotationMotionNotification` (willMove/didMove/didGetOrientation), +`PowerMeterNotification.didGetAbsolutePower`, `SpectrometerNotification` +(willGetSpectrum/didGetSpectrum/didGetSerialNumber). ## Headless / GUI apps: DeviceController diff --git a/CHANGELOG.md b/CHANGELOG.md index 52b24a92..082bc79e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,23 @@ API changes can land even when the minor version is unchanged. - Removes the unused nested `AnalogInputStreamCapability.Notification` (`willAcquire` / `didAcquire`), which was never posted; the equivalent members are now `AnalogInputStreamNotification.willAcquireWaveform` / `didAcquireWaveform`. +- The family bases that already posted notifications now follow the same scheme, + through the same `@notifies` decorator, and name their enum in a `notification` + attribute like the capabilities do. **Breaking for observers**: + - `PowerMeterNotification.didMeasure` is now `didGetAbsolutePower`, named after + its hook like everywhere else. + - `LinearMotionNotification` and `RotationMotionNotification` keep their grouped + `willMove` / `didMove` (`moveTo`, `moveBy` and `home` are one operation to an + observer), but the payload changed: `user_info` is now a dict carrying the + method's arguments by name plus `"result"` and `"error"`, where it used to be + the bare position, displacement or angle. A handler reading + `notification.user_info` as a tuple must now read `user_info["position"]`, + `user_info["displacement"]` or `user_info["angle"]`. + - Every one of them now also reports failures: the `did*` is posted even when the + driver raised, with the exception under `user_info["error"]`. + - `Spectrometer` gained the `notification` attribute it was missing. + - `CameraDeviceNotification` is left alone: a capture session is a different + shape (`imageCaptured` fires per frame), not a will/did pair around one hook. - `allCapabilities()` in `hardwarelibrary/capabilities.py`: returns every capability mixin the library defines, in declaration order. It answers the library-wide question ("what can be expressed?"), where `PhysicalDevice.capabilities()` answers diff --git a/README.md b/README.md index b9615be6..19f5e835 100644 --- a/README.md +++ b/README.md @@ -434,7 +434,9 @@ def onPowerSet(notification): Capabilities related by inheritance share one enum, so you never have to know which variant a device mixed in: `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability` and `AnalogInputStreamCapability` all post `AnalogNotification`, and the three digital ones post `DigitalNotification`. Observing `AnalogNotification.didSetAnalogVoltage` catches the event from a LabJack (which mixes in the combined `AnalogIOCapability`) and from a lock-in amplifier (which mixes in only `AnalogOutputCapability`) alike. -`python -m hardwarelibrary --capabilities` prints every capability with the notifications it posts. Beyond the capabilities, the device-wide enums are `PhysicalDeviceNotification`, `LinearMotionNotification`, `RotationMotionNotification`, `PowerMeterNotification`, `CameraDeviceNotification`, `SpectrometerNotification`, `DeviceControllerNotification`, and `DeviceManagerNotification`. +`python -m hardwarelibrary --capabilities` prints every capability with the notifications it posts. + +The family base classes follow the same scheme, and name their enum in a `notification` attribute too: `LinearMotionNotification` (`willMove`/`didMove`/`didGetPosition`), `RotationMotionNotification` (`willMove`/`didMove`/`didGetOrientation`), `PowerMeterNotification` (`didGetAbsolutePower`) and `SpectrometerNotification`. Motion groups `moveTo`, `moveBy` and `home` under a single `willMove`/`didMove` pair, because to an observer they are all "the stage is moving"; the payload tells them apart, carrying a `position`, a `displacement`, or neither. The remaining enums are `PhysicalDeviceNotification` (device lifecycle), `CameraDeviceNotification`, `DeviceControllerNotification` and `DeviceManagerNotification`. ### Testing without hardware diff --git a/hardwarelibrary/motion/linearmotiondevice.py b/hardwarelibrary/motion/linearmotiondevice.py index db3117bf..c9059b80 100644 --- a/hardwarelibrary/motion/linearmotiondevice.py +++ b/hardwarelibrary/motion/linearmotiondevice.py @@ -1,13 +1,17 @@ from abc import abstractmethod from enum import Enum +from hardwarelibrary.capabilities import notifies from hardwarelibrary.physicaldevice import * from notificationcenter import NotificationCenter, Notification class LinearMotionNotification(Enum): - willMove = "willMove" - didMove = "didMove" + # moveTo, moveBy and home are all "the stage is moving" as far as an observer + # is concerned, so they share one pair; the user_info tells them apart, with a + # position for moveTo, a displacement for moveBy, and neither for home. + willMove = "willMove" + didMove = "didMove" didGetPosition = "didGetPosition" @@ -17,6 +21,8 @@ class Direction(Enum): class LinearMotionDevice(PhysicalDevice): + notification = LinearMotionNotification + def __init__(self, serialNumber: str, idProduct: int, idVendor: int): super().__init__(serialNumber, idProduct, idVendor) self.x = None @@ -48,49 +54,24 @@ def doGetPosition(self) -> tuple: def doHome(self): ... + @notifies(will=LinearMotionNotification.willMove, + did=LinearMotionNotification.didMove) def moveTo(self, position): - NotificationCenter().post_notification( - LinearMotionNotification.willMove, - notifying_object=self, - user_info=position, - ) self.doMoveTo(position) - NotificationCenter().post_notification( - LinearMotionNotification.didMove, - notifying_object=self, - user_info=position, - ) + @notifies(will=LinearMotionNotification.willMove, + did=LinearMotionNotification.didMove) def moveBy(self, displacement): - NotificationCenter().post_notification( - LinearMotionNotification.willMove, - notifying_object=self, - user_info=displacement, - ) self.doMoveBy(displacement) - NotificationCenter().post_notification( - LinearMotionNotification.didMove, - notifying_object=self, - user_info=displacement, - ) + @notifies(did=LinearMotionNotification.didGetPosition) def position(self) -> (): - position = self.doGetPosition() - NotificationCenter().post_notification( - LinearMotionNotification.didGetPosition, - notifying_object=self, - user_info=position, - ) - return position + return self.doGetPosition() + @notifies(will=LinearMotionNotification.willMove, + did=LinearMotionNotification.didMove) def home(self) -> (): - NotificationCenter().post_notification( - LinearMotionNotification.willMove, notifying_object=self - ) self.doHome() - NotificationCenter().post_notification( - LinearMotionNotification.didMove, notifying_object=self - ) def moveInMicronsTo(self, position): nativePosition = [x * self.nativeStepsPerMicrons for x in position] diff --git a/hardwarelibrary/motion/rotationdevice.py b/hardwarelibrary/motion/rotationdevice.py index c7008e4d..90b2e838 100644 --- a/hardwarelibrary/motion/rotationdevice.py +++ b/hardwarelibrary/motion/rotationdevice.py @@ -1,12 +1,15 @@ from abc import abstractmethod from enum import Enum +from hardwarelibrary.capabilities import notifies from hardwarelibrary.physicaldevice import * from notificationcenter import NotificationCenter, Notification class RotationMotionNotification(Enum): - willMove = "willMove" - didMove = "didMove" + # moveTo, moveBy and home share one pair, as in LinearMotionNotification: the + # user_info carries the angle or the delta, and neither for home. + willMove = "willMove" + didMove = "didMove" didGetOrientation = "didGetOrientation" class Direction(Enum): @@ -14,6 +17,7 @@ class Direction(Enum): bidirectional = "bidirectional" class RotationDevice(PhysicalDevice): + notification = RotationMotionNotification def __init__(self, serialNumber:str, idProduct:int, idVendor:int): super().__init__(serialNumber, idProduct, idVendor) @@ -37,25 +41,24 @@ def doGetOrientation(self) -> float: def doHome(self): ... + @notifies(will=RotationMotionNotification.willMove, + did=RotationMotionNotification.didMove) def moveTo(self, angle): - NotificationCenter().post_notification(RotationMotionNotification.willMove, notifying_object=self, user_info=angle) self.doMoveTo(angle) - NotificationCenter().post_notification(RotationMotionNotification.didMove, notifying_object=self, user_info=angle) + @notifies(will=RotationMotionNotification.willMove, + did=RotationMotionNotification.didMove) def moveBy(self, deltaTheta): - NotificationCenter().post_notification(RotationMotionNotification.willMove, notifying_object=self, user_info=deltaTheta) self.doMoveBy(deltaTheta) - NotificationCenter().post_notification(RotationMotionNotification.didMove, notifying_object=self, user_info=deltaTheta) + @notifies(did=RotationMotionNotification.didGetOrientation) def orientation(self) -> (): - orientation = self.doGetOrientation() - NotificationCenter().post_notification(RotationMotionNotification.didGetOrientation, notifying_object=self, user_info=orientation) - return orientation + return self.doGetOrientation() + @notifies(will=RotationMotionNotification.willMove, + did=RotationMotionNotification.didMove) def home(self) -> (): - NotificationCenter().post_notification(RotationMotionNotification.willMove, notifying_object=self) self.doHome() - NotificationCenter().post_notification(RotationMotionNotification.didMove, notifying_object=self) class DebugRotationDevice(RotationDevice): diff --git a/hardwarelibrary/powermeters/powermeterdevice.py b/hardwarelibrary/powermeters/powermeterdevice.py index 0c23cd69..77507a6a 100644 --- a/hardwarelibrary/powermeters/powermeterdevice.py +++ b/hardwarelibrary/powermeters/powermeterdevice.py @@ -2,14 +2,18 @@ from abc import abstractmethod from enum import Enum +from hardwarelibrary.capabilities import notifies from hardwarelibrary.communication import USBPort, TextCommand from hardwarelibrary.physicaldevice import * from notificationcenter import NotificationCenter, Notification class PowerMeterNotification(Enum): - didMeasure = "didMeasure" + # Named after the hook, like every capability: measureAbsolutePower is a + # read, so it posts a did only. + didGetAbsolutePower = "didGetAbsolutePower" class PowerMeterDevice(PhysicalDevice): + notification = PowerMeterNotification def __init__(self, serialNumber:str, idProduct:int, idVendor:int): super().__init__(serialNumber, idProduct, idVendor) @@ -23,11 +27,10 @@ def __init__(self, serialNumber:str, idProduct:int, idVendor:int): def doGetAbsolutePower(self): ... + @notifies(did=PowerMeterNotification.didGetAbsolutePower) def measureAbsolutePower(self): self.doGetAbsolutePower() - power = self.absolutePower - NotificationCenter().post_notification(PowerMeterNotification.didMeasure, notifying_object=self, user_info=power) - return power + return self.absolutePower def doGetStatusUserInfo(self): return self.measureAbsolutePower() diff --git a/hardwarelibrary/spectrometers/base.py b/hardwarelibrary/spectrometers/base.py index 152ade8c..7edbfb83 100644 --- a/hardwarelibrary/spectrometers/base.py +++ b/hardwarelibrary/spectrometers/base.py @@ -41,6 +41,7 @@ class SpectrumRequestTimeoutError(RuntimeError): class Spectrometer(PhysicalDevice): idVendor = None idProduct = None + notification = SpectrometerNotification def __init__(self, serialNumber=None, idProduct:int = None, idVendor:int = None): import numpy as np diff --git a/hardwarelibrary/tests/testCapabilities.py b/hardwarelibrary/tests/testCapabilities.py index 0f22c25a..74da8b87 100644 --- a/hardwarelibrary/tests/testCapabilities.py +++ b/hardwarelibrary/tests/testCapabilities.py @@ -94,6 +94,47 @@ def testNoCapabilityIsDeclaredOutsideTheCapabilitiesModule(self): class TestDeviceHookPattern(unittest.TestCase): + # A family base that posts notifications names its enum the same way a + # capability does, so the scheme is one rule across the whole library. + readsThatKeepAWill = {"willGetSpectrum"} # an acquisition, not a state read + + def familyBasesThatNotify(self): + return [klass for klass in everySubclassOf(PhysicalDevice) + if not isDeclaredInATestModule(klass) + and "notification" in vars(klass)] + + def testFamilyBaseNotificationsAreNamedAfterTheirHooks(self): + # Looser than the capability rule on purpose: a family base may group + # several hooks under one name when they are one operation to an observer + # (moveTo, moveBy and home all post willMove/didMove), so a member's stem + # only has to begin a hook it stands for. + for klass in self.familyBasesThatNotify(): + hookStems = [name[len("do"):] for name in vars(klass) + if name.startswith("do")] + for memberName in klass.notification.__members__: + self.assertRegex(memberName, r"^(will|did)") + stem = memberName[len("will"):] if memberName.startswith("will") \ + else memberName[len("did"):] + self.assertTrue(any(hook.startswith(stem) for hook in hookStems), + "{0}.{1}".format(klass.notification.__name__, memberName)) + + def testFamilyBaseReadsPostDidOnly(self): + for klass in self.familyBasesThatNotify(): + for memberName in klass.notification.__members__: + if not memberName.startswith("will"): + continue + stem = memberName[len("will"):] + isRead = stem.startswith("Get") or stem.startswith("Read") + self.assertTrue(not isRead or memberName in self.readsThatKeepAWill, + "{0}.{1}".format(klass.notification.__name__, memberName)) + + def testEveryFamilyBaseThatNotifiesIsCovered(self): + # Guards the guard: if a family base stops declaring `notification`, + # the two tests above would quietly check nothing. + covered = {klass.__name__ for klass in self.familyBasesThatNotify()} + self.assertTrue({"LinearMotionDevice", "RotationDevice", "PowerMeterDevice", + "Spectrometer"}.issubset(covered), covered) + def testNoDeviceDeclaresAnAbstractMethodOutsideItsDoHooks(self): # The same rule beyond the mixins: a family base (Spectrometer, # PowerMeterDevice, CameraDevice, ...) declares only do* hooks as diff --git a/hardwarelibrary/tests/testFieldMasterDevice.py b/hardwarelibrary/tests/testFieldMasterDevice.py index 274e6e9c..ae83f365 100644 --- a/hardwarelibrary/tests/testFieldMasterDevice.py +++ b/hardwarelibrary/tests/testFieldMasterDevice.py @@ -32,9 +32,9 @@ def testMeasureAbsolutePowerPostsNotification(self): def handler(notification): self.received = notification.user_info - NotificationCenter().add_observer(self, handler, PowerMeterNotification.didMeasure) + NotificationCenter().add_observer(self, handler, PowerMeterNotification.didGetAbsolutePower) power = self.device.measureAbsolutePower() - self.assertEqual(self.received, power) + self.assertEqual(self.received, {"result": power, "error": None}) def testGetCalibrationWavelength(self): self.assertEqual(self.device.getCalibrationWavelength(), 1064.0) From 238648c880952d84e50164fcd698f919a86be350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Tue, 4 Aug 2026 14:47:02 -0400 Subject: [PATCH 5/7] Refuse capability operations on a device that is not initialized 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) --- CHANGELOG.md | 17 ++++- CLAUDE.md | 3 +- README.md | 2 + hardwarelibrary/capabilities.py | 38 ++++++++-- hardwarelibrary/physicaldevice.py | 16 +++++ hardwarelibrary/tests/testCapabilities.py | 79 ++++++++++++++++++++- hardwarelibrary/tests/testOISpectrometer.py | 5 ++ 7 files changed, 150 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 082bc79e..3a4da620 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,19 @@ API changes can land even when the minor version is unchanged. - `Spectrometer` gained the `notification` attribute it was missing. - `CameraDeviceNotification` is left alone: a capture session is a different shape (`imageCaptured` fires per frame), not a will/did pair around one hook. +- **Every notified operation now requires an initialized device.** `@notifies` calls + `validateReady()` before anything else, raising `PhysicalDevice.NotInitialized` + unless the device is `Ready` and naming the operation, the class and the actual + state. Previously such a call either failed deep inside the driver with + `AttributeError: 'NoneType' object has no attribute ...` on a port that was never + opened, or -- on a debug device -- answered as though the hardware had done it and + posted a `did*` claiming success. The check runs before the `will` is posted, so a + refused call announces nothing. `PhysicalDevice.validateReady` is the real check; + `Capability.validateReady` is a no-op behind it in the MRO so a mixin can still be + exercised on its own. Methods that only report what a model supports + (`supportedInputSources`, `supportedSensitivities`, `supportedTimeConstants`, + `supportedTriggerSources`, `outletCount`) are exempt via `requiresReady=False`, + since a UI populates its menus before connecting. - `allCapabilities()` in `hardwarelibrary/capabilities.py`: returns every capability mixin the library defines, in declaration order. It answers the library-wide question ("what can be expressed?"), where `PhysicalDevice.capabilities()` answers @@ -62,8 +75,8 @@ API changes can land even when the minor version is unchanged. - `capabilityInterface()` in `hardwarelibrary/capabilities.py`: describes one capability as `extends` / `publicAPI` / `hooks` lists of `CapabilityMember(name, signature, isAbstract)` tuples. The `do` prefix is what separates a hook from the public API, - not abstractness: the DAQ capabilities make the public method itself abstract with - no `do*` counterpart. Members a parent capability declares are left to that parent. + not abstractness: a hook that is optional, or that defaults to a composition of the + others, is concrete. Members a parent capability declares are left to that parent. - `python -m hardwarelibrary --capabilities` (`-c`): prints every capability with the methods it defines and the hooks a driver must implement, so the list never has to be maintained by hand. diff --git a/CLAUDE.md b/CLAUDE.md index c84cdba8..deba9b31 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,8 +68,9 @@ All families **use interface-segregated capability mixins** instead of one fat b - An operation that changes the instrument posts `will` before and `did` after; a **read (`doGet*`, `doReadStream`) posts only `did`**, to keep hot paths (a voltage sampled in a loop) at one notification instead of two. `Spectrometer.getSpectrum` is the one read that keeps a `will`, because acquiring takes an integration time. - **`did*` is posted whether the operation succeeded or not**, so a `will*` is always followed by its `did*` and there is no separate failure member to pair up. The **exception is still re-raised untouched** — 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). An observer decides what to do from the payload; a caller still sees the exception. - `user_info` is a dict of the public method's arguments by name, plus `"result"` and `"error"`, exactly one of which is non-None. Test `user_info["error"]`, not the member, to tell success from failure. +- **The device must be initialized.** `@notifies` calls `validateReady()` first, which raises `PhysicalDevice.NotInitialized` unless the state is `Ready`, naming the operation, the class and the actual state. It runs *before* the `will` is posted, since nothing was attempted, so a guarded call posts nothing at all. `PhysicalDevice.validateReady` is the real check and sits ahead of `Capability.validateReady` (a no-op) in a driver's MRO, so a capability exercised bare in a test still works. Methods that only report what a model supports (`supported*`, `outletCount`) pass `requiresReady=False`, because a UI populates its menus before connecting. - **Capabilities related by inheritance share one enum**, so `notification` is literally the same object on all of them and their members are interchangeable: `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability` and `AnalogInputStreamCapability` all post `AnalogNotification`; the digital trio posts `DigitalNotification`. This matters because members are keyed by identity — two same-named members in two enums would never cross-fire, so an observer would otherwise have to know whether a device mixed in the combined capability or the plain one. -- Members are keyed by enum identity, not by their string value, so `didFail` in all 17 enums does not collide. +- Members are keyed by enum identity, not by their string value, so same-named members in two enums never cross-fire — which is exactly why the sharing above is necessary rather than cosmetic. - Composed hooks nest: `acquireWaveform` posts its own pair plus the pairs of the `configureStream` / `startStream` / `readStream` / `stopStream` calls its default implementation makes. - DAQ: `AnalogInputCapability` (`getAnalogVoltage`), `AnalogOutputCapability` (`setAnalogVoltage`), `DigitalInputCapability` (`getDigitalValue`), `DigitalOutputCapability` (`setDigitalValue`), plus `AnalogIOCapability` / `DigitalIOCapability` that combine each pair, and `AnalogInputStreamCapability` for hardware-timed acquisition. Drivers implement `doGetAnalogVoltage` / `doSetAnalogVoltage` / `doGetDigitalValue` / `doSetDigitalValue` and the streaming hooks; the `doConfigure*` and `do*Direction` hooks are optional no-ops. `configureStream(channels, sampleRate, **parameters)` forwards any extra keyword to `doConfigureStream`, which is how instrument-specific options reach a driver (the SR830's `sampleClock`). Example: `class LabjackDevice(PhysicalDevice, AnalogIOCapability, DigitalIOCapability, AnalogInputStreamCapability)`. diff --git a/README.md b/README.md index 19f5e835..67526ef0 100644 --- a/README.md +++ b/README.md @@ -422,6 +422,8 @@ center.add_observer( * a **read** posts only `did...`, e.g. `didGetPower` — bracketing a value that is merely being read would double the traffic on hot paths like a voltage sampled in a loop, for no added information. The exception is `SpectrometerNotification.willGetSpectrum`, because an acquisition takes an integration time and a display has something to show while it waits; * the `did...` is posted **whether the operation worked or not**, so a `will...` is always followed by its `did...` and you never have to wonder whether an operation is still running. If the driver raised, the exception continues on its way untouched — your code still sees it — and the notification carries it. +Every one of these operations also requires an initialized device: calling `laser.turnOn()` before `initializeDevice()` raises `PhysicalDevice.NotInitialized` telling you which operation, which device and what state it is in, instead of failing deep inside the driver on a port that was never opened. Nothing is posted in that case, since nothing was attempted. Asking what a model supports (`supportedSensitivities()`, `outletCount`) is exempt, so a UI can populate its menus before connecting. + The payload in `notification.user_info` is a dict of the method's arguments by name, plus `"result"` and `"error"`. Exactly one of those two is set, which is how an observer tells the outcome: ```python diff --git a/hardwarelibrary/capabilities.py b/hardwarelibrary/capabilities.py index d07c85f1..9cd8eab0 100644 --- a/hardwarelibrary/capabilities.py +++ b/hardwarelibrary/capabilities.py @@ -44,7 +44,7 @@ from notificationcenter import NotificationCenter -def notifies(did, will=None): +def notifies(did, will=None, requiresReady=True): """Bracket a capability's public method with notifications. Posts `will` (when the operation changes the instrument) before the call and @@ -58,6 +58,13 @@ def notifies(did, will=None): driver's own exception type is part of its contract, and the library must not disguise it. An observer decides what to do from the presence of an error; a caller still gets the exception. + + The device must be initialized: validateReady() runs first and raises + PhysicalDevice.NotInitialized otherwise. It runs before the will is posted, + because nothing was attempted and the hardware was never touched, so an + observer should hear nothing at all. Pass requiresReady=False for a method + that only reports what the instrument supports, which a UI may legitimately + ask before connecting. """ def decorator(method): signature = inspect.signature(method) @@ -65,6 +72,9 @@ def decorator(method): @functools.wraps(method) def wrapper(self, *args, **keywordArguments): + if requiresReady: + self.validateReady(operation=method.__name__) + arguments = {} if takesArguments: bound = signature.bind(self, *args, **keywordArguments) @@ -104,6 +114,17 @@ class Capability(ABC): # allCapabilities() also enumerates every notification the library defines. notification = None + def validateReady(self, operation=None): + """Confirm the device is initialized before an operation touches it. + + A mixin standing on its own has no device state to check, so this does + nothing. PhysicalDevice sits ahead of every capability in a driver's MRO + and overrides it with the real check, which is the one that runs on real + hardware; this fallback exists so a capability can still be exercised + bare, as the tests do. + """ + pass + # --------------------------------------------------------------------------- # Laser source capabilities @@ -781,17 +802,20 @@ def setTimeConstant(self, seconds): """Set the time constant to the nearest supported step, in seconds.""" return self.doSetTimeConstant(seconds) - @notifies(did=PhaseLockedDetectionNotification.didGetSupportedInputSources) + @notifies(did=PhaseLockedDetectionNotification.didGetSupportedInputSources, + requiresReady=False) def supportedInputSources(self): """Returns the InputSource members this instrument supports, or None.""" return self.doGetSupportedInputSources() - @notifies(did=PhaseLockedDetectionNotification.didGetSupportedSensitivities) + @notifies(did=PhaseLockedDetectionNotification.didGetSupportedSensitivities, + requiresReady=False) def supportedSensitivities(self): """Returns the full-scale sensitivities (volts) this instrument supports, or None.""" return self.doGetSupportedSensitivities() - @notifies(did=PhaseLockedDetectionNotification.didGetSupportedTimeConstants) + @notifies(did=PhaseLockedDetectionNotification.didGetSupportedTimeConstants, + requiresReady=False) def supportedTimeConstants(self): """Returns the time constants (seconds) this instrument supports, or None.""" return self.doGetSupportedTimeConstants() @@ -929,7 +953,8 @@ def softwareTrigger(self): """Issue a manual (software) trigger edge.""" return self.doSoftwareTrigger() - @notifies(did=TriggerNotification.didGetSupportedTriggerSources) + @notifies(did=TriggerNotification.didGetSupportedTriggerSources, + requiresReady=False) def supportedTriggerSources(self): """Returns the TriggerSource members this device supports, or None.""" return self.doGetSupportedTriggerSources() @@ -1076,7 +1101,8 @@ def isOutletOn(self, outlet: int) -> bool: return self.doGetOutletState(outlet) @property - @notifies(did=OutletSwitchingNotification.didGetOutletCount) + # How many outlets the model has is a fact about the strip, not a reading. + @notifies(did=OutletSwitchingNotification.didGetOutletCount, requiresReady=False) def outletCount(self) -> int: return self.doGetOutletCount() diff --git a/hardwarelibrary/physicaldevice.py b/hardwarelibrary/physicaldevice.py index 04424f6d..59ca7973 100644 --- a/hardwarelibrary/physicaldevice.py +++ b/hardwarelibrary/physicaldevice.py @@ -79,6 +79,22 @@ def __init__(self, serialNumber:str, idProduct:int, idVendor:int): # arguments and call super().__init__() itself. super().__init__() + def validateReady(self, operation=None): + """Raise PhysicalDevice.NotInitialized unless the device is Ready. + + Overrides the no-op on Capability, which sits behind PhysicalDevice in a + driver's MRO, so every notified operation is guarded without a driver + writing anything. Without it, an operation on an unopened device fails + deep inside the driver on a port that is still None -- or worse, a debug + device answers as though the hardware had done it. + """ + if self.state != DeviceState.Ready: + raise PhysicalDevice.NotInitialized( + "Cannot {0} on {1}: the device is {2}, not Ready. Call " + "initializeDevice() first.".format( + "{0}()".format(operation) if operation else "operate", + type(self).__name__, self.state.name)) + def capabilities(self) -> list: # The capability mixins, not the Capability marker nor the device class # itself (a driver is a Capability subclass too, but it is a diff --git a/hardwarelibrary/tests/testCapabilities.py b/hardwarelibrary/tests/testCapabilities.py index 74da8b87..28b014a0 100644 --- a/hardwarelibrary/tests/testCapabilities.py +++ b/hardwarelibrary/tests/testCapabilities.py @@ -14,7 +14,10 @@ OnOffCapability, ShutterCapability, PowerCapability, AnalogInputCapability, AnalogOutputCapability, AnalogIOCapability, AnalogNotification, OutletSwitchingCapability) -from hardwarelibrary.physicaldevice import PhysicalDevice +from hardwarelibrary.daq import DebugSR830Device +from hardwarelibrary.physicaldevice import DeviceState, PhysicalDevice +from hardwarelibrary.powerstrips import DebugPwrUSBDevice +from hardwarelibrary.sources import DebugMillenniaDevice from notificationcenter import NotificationCenter @@ -244,6 +247,80 @@ def testExtendsReportsTheCapabilitiesCombined(self): self.assertEqual(capabilityInterface(OnOffCapability)["extends"], []) +class TestStateGuard(unittest.TestCase): + def setUp(self): + self.laser = DebugMillenniaDevice() + self.recorder = NotificationRecorder() + self.recorder.observe(OnOffCapability.notification) + + def tearDown(self): + self.recorder.stop() + if self.laser.state == DeviceState.Ready: + self.laser.shutdownDevice() + + def testAnOperationBeforeInitializationRaises(self): + with self.assertRaises(PhysicalDevice.NotInitialized): + self.laser.turnOn() + + def testTheErrorNamesTheOperationTheDeviceAndTheState(self): + with self.assertRaises(PhysicalDevice.NotInitialized) as raised: + self.laser.turnOn() + message = str(raised.exception) + self.assertIn("turnOn()", message) + self.assertIn("DebugMillenniaEv25Device", message) + self.assertIn("Unconfigured", message) + + def testNothingIsPostedWhenTheDeviceIsNotReady(self): + # Not even a will: nothing was attempted and the hardware was never + # touched, so an observer should hear nothing at all. + with self.assertRaises(PhysicalDevice.NotInitialized): + self.laser.turnOn() + self.assertEqual(self.recorder.names(), []) + + def testAReadIsGuardedToo(self): + with self.assertRaises(PhysicalDevice.NotInitialized): + self.laser.isLaserOn() + + def testTheOperationRunsOnceTheDeviceIsReady(self): + self.laser.initializeDevice() + self.laser.turnOn() + self.assertTrue(self.laser.isLaserOn()) + self.assertEqual(self.recorder.names(), + ["willTurnOn", "didTurnOn", "didGetOnOffState"]) + + def testAShutdownDeviceIsGuardedAgain(self): + self.laser.initializeDevice() + self.laser.shutdownDevice() + with self.assertRaises(PhysicalDevice.NotInitialized): + self.laser.turnOn() + + def testWhatAnInstrumentSupportsCanBeAskedBeforeConnecting(self): + # A UI populates its menus before the device is opened, so the methods + # that only report capabilities of the model are exempt from the guard. + lockin = DebugSR830Device() + self.assertEqual(lockin.state, DeviceState.Unconfigured) + self.assertIsNotNone(lockin.supportedSensitivities()) + self.assertIsNotNone(lockin.supportedTimeConstants()) + self.assertIsNotNone(lockin.supportedInputSources()) + self.assertIsNotNone(lockin.supportedTriggerSources()) + self.assertEqual(DebugPwrUSBDevice().outletCount, 3) + + def testEveryOtherOperationOfEveryCapabilityIsGuarded(self): + # Walks the public API rather than naming methods, so a capability added + # later cannot quietly escape the guard. + exempt = {"supportedInputSources", "supportedSensitivities", + "supportedTimeConstants", "supportedTriggerSources", + "outletCount", "canTurnOn"} + for capability in allCapabilities(): + for member in capabilityInterface(capability)["publicAPI"]: + if member.name in exempt: + continue + method = getattr(capability, member.name) + self.assertTrue(hasattr(method, "__wrapped__"), + "{0}.{1} is not wrapped by @notifies".format( + capability.__name__, member.name)) + + class _FailingAnalogDevice(AnalogIOCapability): """Every hook raises, so the failure path can be exercised.""" diff --git a/hardwarelibrary/tests/testOISpectrometer.py b/hardwarelibrary/tests/testOISpectrometer.py index 92d08855..9cf90f04 100644 --- a/hardwarelibrary/tests/testOISpectrometer.py +++ b/hardwarelibrary/tests/testOISpectrometer.py @@ -2,6 +2,7 @@ import unittest import time +from hardwarelibrary.physicaldevice import DeviceState from hardwarelibrary.spectrometers.oceaninsight import ( OISpectrometer, SpectrumRequestTimeoutError, ) @@ -15,6 +16,10 @@ class MockOISpectrometer(OISpectrometer): """ def __init__(self, spectrumReady=False): + # PhysicalDevice.__init__ is bypassed, so declare the state it would + # have set: this mock stands in for a spectrometer already connected, + # and getSpectrum refuses to run on a device that is not Ready. + self.state = DeviceState.Ready self.spectrumReady = spectrumReady self.requestCount = 0 From cc230ddb59ddd5c39d08e730ca250010f02407a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Tue, 4 Aug 2026 14:54:39 -0400 Subject: [PATCH 6/7] Define validateReady only on PhysicalDevice 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) --- CHANGELOG.md | 7 ++++--- CLAUDE.md | 2 +- hardwarelibrary/capabilities.py | 15 ++++----------- hardwarelibrary/tests/testCapabilities.py | 8 ++++++++ hardwarelibrary/tests/testSR830.py | 4 ++++ 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a4da620..e817452e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,9 +59,10 @@ API changes can land even when the minor version is unchanged. `AttributeError: 'NoneType' object has no attribute ...` on a port that was never opened, or -- on a debug device -- answered as though the hardware had done it and posted a `did*` claiming success. The check runs before the `will` is posted, so a - refused call announces nothing. `PhysicalDevice.validateReady` is the real check; - `Capability.validateReady` is a no-op behind it in the MRO so a mixin can still be - exercised on its own. Methods that only report what a model supports + refused call announces nothing. `validateReady` is defined once, on + `PhysicalDevice`, where the device lifecycle belongs; `capabilities.py` only calls + it, so a class mixing in a capability without being a `PhysicalDevice` answers for + readiness itself rather than silently skipping the check. Methods that only report what a model supports (`supportedInputSources`, `supportedSensitivities`, `supportedTimeConstants`, `supportedTriggerSources`, `outletCount`) are exempt via `requiresReady=False`, since a UI populates its menus before connecting. diff --git a/CLAUDE.md b/CLAUDE.md index deba9b31..d81c7479 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,7 +68,7 @@ All families **use interface-segregated capability mixins** instead of one fat b - An operation that changes the instrument posts `will` before and `did` after; a **read (`doGet*`, `doReadStream`) posts only `did`**, to keep hot paths (a voltage sampled in a loop) at one notification instead of two. `Spectrometer.getSpectrum` is the one read that keeps a `will`, because acquiring takes an integration time. - **`did*` is posted whether the operation succeeded or not**, so a `will*` is always followed by its `did*` and there is no separate failure member to pair up. The **exception is still re-raised untouched** — 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). An observer decides what to do from the payload; a caller still sees the exception. - `user_info` is a dict of the public method's arguments by name, plus `"result"` and `"error"`, exactly one of which is non-None. Test `user_info["error"]`, not the member, to tell success from failure. -- **The device must be initialized.** `@notifies` calls `validateReady()` first, which raises `PhysicalDevice.NotInitialized` unless the state is `Ready`, naming the operation, the class and the actual state. It runs *before* the `will` is posted, since nothing was attempted, so a guarded call posts nothing at all. `PhysicalDevice.validateReady` is the real check and sits ahead of `Capability.validateReady` (a no-op) in a driver's MRO, so a capability exercised bare in a test still works. Methods that only report what a model supports (`supported*`, `outletCount`) pass `requiresReady=False`, because a UI populates its menus before connecting. +- **The device must be initialized.** `@notifies` calls `validateReady()` first, which raises `PhysicalDevice.NotInitialized` unless the state is `Ready`, naming the operation, the class and the actual state. It runs *before* the `will` is posted, since nothing was attempted, so a guarded call posts nothing at all. `validateReady` is defined once, on `PhysicalDevice`, where the lifecycle lives; `capabilities.py` only calls it. A class that mixes in a capability without being a `PhysicalDevice` must therefore answer for readiness itself — the test stubs that stand in for a device do exactly that. Methods that only report what a model supports (`supported*`, `outletCount`) pass `requiresReady=False`, because a UI populates its menus before connecting. - **Capabilities related by inheritance share one enum**, so `notification` is literally the same object on all of them and their members are interchangeable: `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability` and `AnalogInputStreamCapability` all post `AnalogNotification`; the digital trio posts `DigitalNotification`. This matters because members are keyed by identity — two same-named members in two enums would never cross-fire, so an observer would otherwise have to know whether a device mixed in the combined capability or the plain one. - Members are keyed by enum identity, not by their string value, so same-named members in two enums never cross-fire — which is exactly why the sharing above is necessary rather than cosmetic. - Composed hooks nest: `acquireWaveform` posts its own pair plus the pairs of the `configureStream` / `startStream` / `readStream` / `stopStream` calls its default implementation makes. diff --git a/hardwarelibrary/capabilities.py b/hardwarelibrary/capabilities.py index 9cd8eab0..3a3a1b14 100644 --- a/hardwarelibrary/capabilities.py +++ b/hardwarelibrary/capabilities.py @@ -65,6 +65,10 @@ def notifies(did, will=None, requiresReady=True): observer should hear nothing at all. Pass requiresReady=False for a method that only reports what the instrument supports, which a UI may legitimately ask before connecting. + + validateReady is PhysicalDevice's, which is where the device lifecycle lives; + a capability is meant to be mixed alongside one, so anything else hosting a + capability must answer for readiness itself. """ def decorator(method): signature = inspect.signature(method) @@ -114,17 +118,6 @@ class Capability(ABC): # allCapabilities() also enumerates every notification the library defines. notification = None - def validateReady(self, operation=None): - """Confirm the device is initialized before an operation touches it. - - A mixin standing on its own has no device state to check, so this does - nothing. PhysicalDevice sits ahead of every capability in a driver's MRO - and overrides it with the real check, which is the one that runs on real - hardware; this fallback exists so a capability can still be exercised - bare, as the tests do. - """ - pass - # --------------------------------------------------------------------------- # Laser source capabilities diff --git a/hardwarelibrary/tests/testCapabilities.py b/hardwarelibrary/tests/testCapabilities.py index 28b014a0..6485e324 100644 --- a/hardwarelibrary/tests/testCapabilities.py +++ b/hardwarelibrary/tests/testCapabilities.py @@ -157,6 +157,10 @@ class _RecordingAnalogDevice(AnalogIOCapability): def __init__(self): self.calls = [] + def validateReady(self, operation=None): + """Stands in for a device that is open and ready.""" + pass + def doGetAnalogVoltage(self, channel): self.calls.append(("doGetAnalogVoltage", channel)) return 1.5 @@ -327,6 +331,10 @@ class _FailingAnalogDevice(AnalogIOCapability): class Failure(RuntimeError): pass + def validateReady(self, operation=None): + """Stands in for a device that is open and ready.""" + pass + def doGetAnalogVoltage(self, channel): raise self.Failure("no hardware") diff --git a/hardwarelibrary/tests/testSR830.py b/hardwarelibrary/tests/testSR830.py index 69d1e8b5..056f46f6 100644 --- a/hardwarelibrary/tests/testSR830.py +++ b/hardwarelibrary/tests/testSR830.py @@ -233,6 +233,10 @@ class _MinimalLockIn(PhaseLockedDetectionCapability, TriggerCapability): the base-class optional hooks and the base getDemodulatedValues are exercised (SR830Device overrides all of these).""" + def validateReady(self, operation=None): + """Stands in for a device that is open and ready.""" + pass + def doGetInPhaseVoltage(self): return 0.1 From 58258b31c442b75b8c8a42b118189271fab9933f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Tue, 4 Aug 2026 15:27:21 -0400 Subject: [PATCH 7/7] Validate capability arguments against the contract 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) --- CHANGELOG.md | 24 ++++ CLAUDE.md | 1 + hardwarelibrary/capabilities.py | 160 ++++++++++++++++++---- hardwarelibrary/powerstrips/pwrusb.py | 8 -- hardwarelibrary/tests/testCapabilities.py | 98 ++++++++++++- hardwarelibrary/tests/testSR830.py | 3 +- hardwarelibrary/tests/testValidation.py | 119 ++++++++++++++++ hardwarelibrary/validation.py | 110 +++++++++++++++ 8 files changed, 486 insertions(+), 37 deletions(-) create mode 100644 hardwarelibrary/tests/testValidation.py create mode 100644 hardwarelibrary/validation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e817452e..df5fadff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,30 @@ API changes can land even when the minor version is unchanged. - `Spectrometer` gained the `notification` attribute it was missing. - `CameraDeviceNotification` is left alone: a capture session is a different shape (`imageCaptured` fires per frame), not a will/did pair around one hook. +- **Contract-level argument validation on the capability public methods**, through a + new `validate=` parameter on `@notifies` and a new `hardwarelibrary/validation.py` + holding the shared `require*` checks. The validator runs before anything is posted, + so a refused call announces nothing and a will/did pair still means the driver was + invoked. **Breaking**: calls that used to be accepted silently now raise. + - `acquireWaveform(sampleCount=0)` returned an empty acquisition; now `ValueError`. + An empty `channels` list failed with `min() iterable argument is empty`; now it + names the parameter. + - `configureStream(sampleRate=-100)` was accepted; a rate must be positive, or + `None` to say the clock is external. `sampleRate=0` no longer means "external": + pass `None`, which is what the docstring always said. + - `setDigitalValue("yes", channel)` set the line True; a logic level must be a bool + (0 and 1 accepted). `setAnalogVoltage("2.5", channel)` now raises `TypeError`. + - `setSensitivity(-1)` and `setTimeConstant(0)` were silently snapped to a step; + both must be positive. + - `setWavelength` and `setDispersion` are checked against the range the driver + reports, so `matisse.setWavelength(50.0)` no longer drives the birefringent + filter outside the installed optics' 700-1000 nm. + - Outlets are checked against `doGetOutletCount()` in `OutletSwitchingCapability` + and `DefaultOutletCapability`, so `PwrUSBDevice._validateOutlet` is gone and every + future strip inherits the rule. + - `setInputSource` and `setTriggerSource` accept anything their enum accepts + (`setInputSource("Differential")`) and hand the driver a member. + - Instrument-specific limits stay in the drivers, unchanged. - **Every notified operation now requires an initialized device.** `@notifies` calls `validateReady()` before anything else, raising `PhysicalDevice.NotInitialized` unless the device is `Ready` and naming the operation, the class and the actual diff --git a/CLAUDE.md b/CLAUDE.md index d81c7479..b7952578 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,6 +68,7 @@ All families **use interface-segregated capability mixins** instead of one fat b - An operation that changes the instrument posts `will` before and `did` after; a **read (`doGet*`, `doReadStream`) posts only `did`**, to keep hot paths (a voltage sampled in a loop) at one notification instead of two. `Spectrometer.getSpectrum` is the one read that keeps a `will`, because acquiring takes an integration time. - **`did*` is posted whether the operation succeeded or not**, so a `will*` is always followed by its `did*` and there is no separate failure member to pair up. The **exception is still re-raised untouched** — 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). An observer decides what to do from the payload; a caller still sees the exception. - `user_info` is a dict of the public method's arguments by name, plus `"result"` and `"error"`, exactly one of which is non-None. Test `user_info["error"]`, not the member, to tell success from failure. +- **Contract-level argument checks live in the capability**, in a `_validateXxx` method handed to `@notifies` as `validate=`, built from the `require*` helpers in `hardwarelibrary/validation.py` (`requireRealNumber`, `requireBool`, `requireAtLeast`, `requirePositive`, `requireWithinRange`, `requireNonEmpty`, `requireMember`). They raise `TypeError` for the wrong kind of value and `ValueError` for one out of bounds. The validator runs **before** anything is posted, so a refused call announces nothing: a will/did pair means the driver really was invoked. Two rules: limits that vary by model (the SR830's ±10.5 V, the Millennia's 0.05–25 W) stay in the driver's hook, next to the numbers they come from; and a validator calls `do*` hooks, never public methods, so validating never posts a notification of its own. Channel identity is deliberately unchecked — an SR830 addresses outputs with `AuxOutput` members where a LabJack uses bare ints. - **The device must be initialized.** `@notifies` calls `validateReady()` first, which raises `PhysicalDevice.NotInitialized` unless the state is `Ready`, naming the operation, the class and the actual state. It runs *before* the `will` is posted, since nothing was attempted, so a guarded call posts nothing at all. `validateReady` is defined once, on `PhysicalDevice`, where the lifecycle lives; `capabilities.py` only calls it. A class that mixes in a capability without being a `PhysicalDevice` must therefore answer for readiness itself — the test stubs that stand in for a device do exactly that. Methods that only report what a model supports (`supported*`, `outletCount`) pass `requiresReady=False`, because a UI populates its menus before connecting. - **Capabilities related by inheritance share one enum**, so `notification` is literally the same object on all of them and their members are interchangeable: `AnalogInputCapability`, `AnalogOutputCapability`, `AnalogIOCapability` and `AnalogInputStreamCapability` all post `AnalogNotification`; the digital trio posts `DigitalNotification`. This matters because members are keyed by identity — two same-named members in two enums would never cross-fire, so an observer would otherwise have to know whether a device mixed in the combined capability or the plain one. - Members are keyed by enum identity, not by their string value, so same-named members in two enums never cross-fire — which is exactly why the sharing above is necessary rather than cosmetic. diff --git a/hardwarelibrary/capabilities.py b/hardwarelibrary/capabilities.py index 3a3a1b14..efbe700b 100644 --- a/hardwarelibrary/capabilities.py +++ b/hardwarelibrary/capabilities.py @@ -41,10 +41,13 @@ from collections import namedtuple from enum import Enum +from hardwarelibrary.validation import ( + requireAtLeast, requireBool, requireInteger, requireMember, requireNonEmpty, + requirePositive, requireRealNumber, requireWithinRange) from notificationcenter import NotificationCenter -def notifies(did, will=None, requiresReady=True): +def notifies(did, will=None, requiresReady=True, validate=None): """Bracket a capability's public method with notifications. Posts `will` (when the operation changes the instrument) before the call and @@ -69,6 +72,12 @@ def notifies(did, will=None, requiresReady=True): validateReady is PhysicalDevice's, which is where the device lifecycle lives; a capability is meant to be mixed alongside one, so anything else hosting a capability must answer for readiness itself. + + `validate` is an optional function taking the same arguments as the method, + called before anything is posted so that a refused call announces nothing: a + will/did pair means the driver really was invoked. It holds the contract-level + checks (see hardwarelibrary.validation); limits that vary by model belong in + the driver's hook. """ def decorator(method): signature = inspect.signature(method) @@ -78,6 +87,8 @@ def decorator(method): def wrapper(self, *args, **keywordArguments): if requiresReady: self.validateReady(operation=method.__name__) + if validate is not None: + validate(self, *args, **keywordArguments) arguments = {} if takesArguments: @@ -215,7 +226,11 @@ class PowerCapability(Capability): isWritable = True notification = PowerNotification - @notifies(will=PowerNotification.willSetPower, did=PowerNotification.didSetPower) + def _validateSetPower(self, power: float): + requireAtLeast(power, 0, "power", self.unit) + + @notifies(will=PowerNotification.willSetPower, did=PowerNotification.didSetPower, + validate=_validateSetPower) def setPower(self, power: float): return self.doSetPower(power) @@ -301,8 +316,14 @@ class WavelengthCapability(Capability): isWritable = True notification = WavelengthNotification + def _validateSetWavelength(self, wavelength: float): + # Against the hook, not wavelengthRange(): validating must not post a + # notification of its own. + requireWithinRange(wavelength, self.doGetWavelengthRange(), "wavelength", self.unit) + @notifies(will=WavelengthNotification.willSetWavelength, - did=WavelengthNotification.didSetWavelength) + did=WavelengthNotification.didSetWavelength, + validate=_validateSetWavelength) def setWavelength(self, wavelength: float): return self.doSetWavelength(wavelength) @@ -340,8 +361,12 @@ class DispersionCapability(Capability): isWritable = True notification = DispersionNotification + def _validateSetDispersion(self, dispersion: float): + requireWithinRange(dispersion, self.doGetDispersionRange(), "dispersion", self.unit) + @notifies(will=DispersionNotification.willSetDispersion, - did=DispersionNotification.didSetDispersion) + did=DispersionNotification.didSetDispersion, + validate=_validateSetDispersion) def setDispersion(self, dispersion: float): return self.doSetDispersion(dispersion) @@ -391,8 +416,12 @@ def getCalibrationWavelength(self): self.doGetCalibrationWavelength() return self.calibrationWavelength + def _validateSetCalibrationWavelength(self, wavelength): + requirePositive(wavelength, "wavelength", self.unit) + @notifies(will=WavelengthCalibrationNotification.willSetCalibrationWavelength, - did=WavelengthCalibrationNotification.didSetCalibrationWavelength) + did=WavelengthCalibrationNotification.didSetCalibrationWavelength, + validate=_validateSetCalibrationWavelength) def setCalibrationWavelength(self, wavelength): self.doSetCalibrationWavelength(wavelength) self.doGetCalibrationWavelength() @@ -472,7 +501,11 @@ def getScale(self): self.doGetScale() return self.scale - @notifies(will=ScaleNotification.willSetScale, did=ScaleNotification.didSetScale) + def _validateSetScale(self, scale): + requirePositive(scale, "scale", self.unit) + + @notifies(will=ScaleNotification.willSetScale, did=ScaleNotification.didSetScale, + validate=_validateSetScale) def setScale(self, scale): self.doSetScale(scale) self.doGetScale() @@ -543,8 +576,15 @@ class AnalogOutputCapability(Capability): notification = AnalogNotification + def _validateSetAnalogVoltage(self, value, channel): + # The channel is deliberately not checked: an SR830 addresses its outputs + # with AuxOutput members where a LabJack uses bare ints, so there is no + # shared rule. The driver knows its own channels. + requireRealNumber(value, "value", "V") + @notifies(will=AnalogNotification.willSetAnalogVoltage, - did=AnalogNotification.didSetAnalogVoltage) + did=AnalogNotification.didSetAnalogVoltage, + validate=_validateSetAnalogVoltage) def setAnalogVoltage(self, value, channel): """Set the output on channel to value, in volts.""" return self.doSetAnalogVoltage(value, channel) @@ -617,8 +657,14 @@ class AnalogInputStreamCapability(AnalogInputCapability): notification = AnalogNotification + def _validateConfigureStream(self, channels, sampleRate=None, **parameters): + requireNonEmpty(channels, "channels") + if sampleRate is not None: + requirePositive(sampleRate, "sampleRate", "Hz") + @notifies(will=AnalogNotification.willConfigureStream, - did=AnalogNotification.didConfigureStream) + did=AnalogNotification.didConfigureStream, + validate=_validateConfigureStream) def configureStream(self, channels, sampleRate=None, **parameters): """Set up a hardware-timed acquisition of channels at sampleRate (Hz). @@ -646,8 +692,14 @@ def stopStream(self): """Stop the acquisition and release any hardware streaming resources.""" return self.doStopStream() + def _validateAcquireWaveform(self, channels, sampleRate, sampleCount): + requireNonEmpty(channels, "channels") + requirePositive(sampleRate, "sampleRate", "Hz") + requireAtLeast(requireInteger(sampleCount, "sampleCount"), 1, "sampleCount") + @notifies(will=AnalogNotification.willAcquireWaveform, - did=AnalogNotification.didAcquireWaveform) + did=AnalogNotification.didAcquireWaveform, + validate=_validateAcquireWaveform) def acquireWaveform(self, channels, sampleRate, sampleCount): """Acquire exactly sampleCount samples per channel, blocking until done. @@ -767,19 +819,30 @@ def getInputSource(self) -> InputSource: """Returns the signal input the demodulator currently measures.""" return self.doGetInputSource() + def _validateSetInputSource(self, source: InputSource): + requireMember(source, InputSource, "source") + @notifies(will=PhaseLockedDetectionNotification.willSetInputSource, - did=PhaseLockedDetectionNotification.didSetInputSource) + did=PhaseLockedDetectionNotification.didSetInputSource, + validate=_validateSetInputSource) def setInputSource(self, source: InputSource): - """Select which signal input (an InputSource member) the demodulator measures.""" - return self.doSetInputSource(source) + """Select which signal input (an InputSource member) the demodulator measures. + + A name or value the enum accepts works too; the driver always sees a member. + """ + return self.doSetInputSource(InputSource(source)) @notifies(did=PhaseLockedDetectionNotification.didGetSensitivity) def getSensitivity(self): """Returns the full-scale sensitivity, in volts.""" return self.doGetSensitivity() + def _validateSetSensitivity(self, volts): + requirePositive(volts, "volts", "V") + @notifies(will=PhaseLockedDetectionNotification.willSetSensitivity, - did=PhaseLockedDetectionNotification.didSetSensitivity) + did=PhaseLockedDetectionNotification.didSetSensitivity, + validate=_validateSetSensitivity) def setSensitivity(self, volts): """Set the full-scale sensitivity to the nearest supported step, in volts.""" return self.doSetSensitivity(volts) @@ -789,8 +852,12 @@ def getTimeConstant(self): """Returns the time constant, in seconds.""" return self.doGetTimeConstant() + def _validateSetTimeConstant(self, seconds): + requirePositive(seconds, "seconds", "s") + @notifies(will=PhaseLockedDetectionNotification.willSetTimeConstant, - did=PhaseLockedDetectionNotification.didSetTimeConstant) + did=PhaseLockedDetectionNotification.didSetTimeConstant, + validate=_validateSetTimeConstant) def setTimeConstant(self, seconds): """Set the time constant to the nearest supported step, in seconds.""" return self.doSetTimeConstant(seconds) @@ -929,11 +996,18 @@ class TriggerCapability(Capability): notification = TriggerNotification + def _validateSetTriggerSource(self, source: 'TriggerSource'): + requireMember(source, TriggerSource, "source") + @notifies(will=TriggerNotification.willSetTriggerSource, - did=TriggerNotification.didSetTriggerSource) + did=TriggerNotification.didSetTriggerSource, + validate=_validateSetTriggerSource) def setTriggerSource(self, source: 'TriggerSource'): - """Select whether the acquisition starts immediately or on an external trigger.""" - return self.doSetTriggerSource(source) + """Select whether the acquisition starts immediately or on an external trigger. + + A name or value the enum accepts works too; the driver always sees a member. + """ + return self.doSetTriggerSource(TriggerSource(source)) @notifies(did=TriggerNotification.didGetTriggerSource) def getTriggerSource(self) -> 'TriggerSource': @@ -1003,8 +1077,12 @@ class DigitalOutputCapability(Capability): notification = DigitalNotification + def _validateSetDigitalValue(self, value, channel): + requireBool(value, "value") + @notifies(will=DigitalNotification.willSetDigitalValue, - did=DigitalNotification.didSetDigitalValue) + did=DigitalNotification.didSetDigitalValue, + validate=_validateSetDigitalValue) def setDigitalValue(self, value, channel): """Drive channel to the logic level value.""" return self.doSetDigitalValue(value, channel) @@ -1074,22 +1152,40 @@ class OutletSwitchingCapability(Capability): notification = OutletSwitchingNotification + def _validateOutlet(self, outlet: int): + """Require an outlet the strip actually has, addressed by its label. + + Reads doGetOutletCount() rather than outletCount, so validating posts no + notification of its own; a driver whose count costs a hardware query + should cache it at initialization. + """ + requireInteger(outlet, "outlet") + requireWithinRange(outlet, (1, self.doGetOutletCount()), "outlet") + + def _validateSetOutletState(self, outlet: int, isOn: bool): + self._validateOutlet(outlet) + requireBool(isOn, "isOn") + @notifies(will=OutletSwitchingNotification.willSetOutletState, - did=OutletSwitchingNotification.didSetOutletState) + did=OutletSwitchingNotification.didSetOutletState, + validate=_validateOutlet) def turnOutletOn(self, outlet: int): self.doSetOutletState(outlet, True) @notifies(will=OutletSwitchingNotification.willSetOutletState, - did=OutletSwitchingNotification.didSetOutletState) + did=OutletSwitchingNotification.didSetOutletState, + validate=_validateOutlet) def turnOutletOff(self, outlet: int): self.doSetOutletState(outlet, False) @notifies(will=OutletSwitchingNotification.willSetOutletState, - did=OutletSwitchingNotification.didSetOutletState) + did=OutletSwitchingNotification.didSetOutletState, + validate=_validateSetOutletState) def setOutletState(self, outlet: int, isOn: bool): self.doSetOutletState(outlet, isOn) - @notifies(did=OutletSwitchingNotification.didGetOutletState) + @notifies(did=OutletSwitchingNotification.didGetOutletState, + validate=_validateOutlet) def isOutletOn(self, outlet: int) -> bool: return self.doGetOutletState(outlet) @@ -1127,18 +1223,32 @@ class DefaultOutletCapability(Capability): notification = DefaultOutletNotification + def _validateDefaultOutlet(self, outlet: int): + """Same bounds as OutletSwitchingCapability, which a strip exposing boot + defaults also has; repeated rather than inherited, since the two + capabilities are independent.""" + requireInteger(outlet, "outlet") + requireWithinRange(outlet, (1, self.doGetOutletCount()), "outlet") + + def _validateSetOutletDefaultState(self, outlet: int, isOn: bool): + self._validateDefaultOutlet(outlet) + requireBool(isOn, "isOn") + @notifies(will=DefaultOutletNotification.willSetOutletDefaultState, - did=DefaultOutletNotification.didSetOutletDefaultState) + did=DefaultOutletNotification.didSetOutletDefaultState, + validate=_validateDefaultOutlet) def setOutletDefaultOn(self, outlet: int): self.doSetOutletDefaultState(outlet, True) @notifies(will=DefaultOutletNotification.willSetOutletDefaultState, - did=DefaultOutletNotification.didSetOutletDefaultState) + did=DefaultOutletNotification.didSetOutletDefaultState, + validate=_validateDefaultOutlet) def setOutletDefaultOff(self, outlet: int): self.doSetOutletDefaultState(outlet, False) @notifies(will=DefaultOutletNotification.willSetOutletDefaultState, - did=DefaultOutletNotification.didSetOutletDefaultState) + did=DefaultOutletNotification.didSetOutletDefaultState, + validate=_validateSetOutletDefaultState) def setOutletDefaultState(self, outlet: int, isOn: bool): self.doSetOutletDefaultState(outlet, isOn) diff --git a/hardwarelibrary/powerstrips/pwrusb.py b/hardwarelibrary/powerstrips/pwrusb.py index e26b1a7f..db599432 100644 --- a/hardwarelibrary/powerstrips/pwrusb.py +++ b/hardwarelibrary/powerstrips/pwrusb.py @@ -90,11 +90,6 @@ def doShutdownDevice(self): self.port.close() self.port = None - def _validateOutlet(self, outlet: int): - if outlet not in range(1, self.switchableOutletCount + 1): - raise ValueError("Outlet must be 1..{0}, got {1}".format( - self.switchableOutletCount, outlet)) - def deviceType(self) -> str: # flush() first so the reply is read from a clean buffer: a report can be # longer than the bytes we need, leaving a remainder buffered from the @@ -108,17 +103,14 @@ def doGetOutletCount(self) -> int: return self.switchableOutletCount def doSetOutletState(self, outlet: int, isOn: bool): - self._validateOutlet(outlet) key = "on" if isOn else "off" self.port.writeData(bytearray([self.outletCommands[key][outlet - 1]])) self._outletStateCache[outlet - 1] = bool(isOn) def doGetOutletState(self, outlet: int) -> bool: - self._validateOutlet(outlet) return self._outletStateCache[outlet - 1] def doSetOutletDefaultState(self, outlet: int, isOn: bool): - self._validateOutlet(outlet) key = "defaultOn" if isOn else "defaultOff" self.port.writeData(bytearray([self.outletCommands[key][outlet - 1]])) diff --git a/hardwarelibrary/tests/testCapabilities.py b/hardwarelibrary/tests/testCapabilities.py index 6485e324..bed27bc2 100644 --- a/hardwarelibrary/tests/testCapabilities.py +++ b/hardwarelibrary/tests/testCapabilities.py @@ -13,11 +13,11 @@ Capability, allCapabilities, capabilityInterface, OnOffCapability, ShutterCapability, PowerCapability, AnalogInputCapability, AnalogOutputCapability, AnalogIOCapability, - AnalogNotification, OutletSwitchingCapability) -from hardwarelibrary.daq import DebugSR830Device + AnalogNotification, InputSource, OutletSwitchingCapability) +from hardwarelibrary.daq import DebugLabjackDevice, DebugSR830Device from hardwarelibrary.physicaldevice import DeviceState, PhysicalDevice from hardwarelibrary.powerstrips import DebugPwrUSBDevice -from hardwarelibrary.sources import DebugMillenniaDevice +from hardwarelibrary.sources import DebugMatisseDevice, DebugMillenniaDevice from notificationcenter import NotificationCenter @@ -325,6 +325,98 @@ def testEveryOtherOperationOfEveryCapabilityIsGuarded(self): capability.__name__, member.name)) +class TestArgumentValidation(unittest.TestCase): + def setUp(self): + self.daq = DebugLabjackDevice() + self.daq.initializeDevice() + self.strip = DebugPwrUSBDevice() + self.strip.initializeDevice() + self.recorder = NotificationRecorder() + + def tearDown(self): + self.recorder.stop() + self.daq.shutdownDevice() + self.strip.shutdownDevice() + + def testARejectedCallPostsNothing(self): + # The invariant the validate= hook buys: a will/did pair means the driver + # really was invoked, so a refused call announces nothing at all. + self.recorder.observe(AnalogNotification) + with self.assertRaises(ValueError): + self.daq.acquireWaveform([0], sampleRate=100, sampleCount=0) + self.assertEqual(self.recorder.names(), []) + + def testAnEmptyChannelListIsRefusedClearly(self): + # It used to fail inside the drain loop with "min() iterable argument is + # empty", which named nothing the caller had written. + with self.assertRaises(ValueError) as raised: + self.daq.acquireWaveform([], sampleRate=100, sampleCount=10) + self.assertIn("channels", str(raised.exception)) + + def testASampleCountOfZeroNoLongerReturnsAnEmptyAcquisition(self): + for sampleCount in (0, -5): + with self.assertRaises(ValueError): + self.daq.acquireWaveform([0], sampleRate=100, sampleCount=sampleCount) + + def testANegativeSampleRateIsRefused(self): + with self.assertRaises(ValueError): + self.daq.configureStream([0], sampleRate=-100) + + def testAnExternalClockMaySayItHasNoRate(self): + self.daq.configureStream([0], sampleRate=None) # accepted, means "not mine" + + def testALogicLevelMustBeOne(self): + with self.assertRaises(TypeError): + self.daq.setDigitalValue("yes", channel=4) + self.daq.setDigitalValue(1, channel=4) # 0 and 1 still work + self.assertTrue(self.daq.getDigitalValue(4)) + + def testAVoltageMustBeANumber(self): + with self.assertRaises(TypeError): + self.daq.setAnalogVoltage("2.5", channel=0) + + def testAnOutletMustBeOneTheStripHas(self): + for outlet in (0, 4, 1.5): + with self.assertRaises((ValueError, TypeError)): + self.strip.turnOutletOn(outlet) + self.strip.turnOutletOn(3) + + def testAWavelengthMustBeInTheRangeTheDriverReports(self): + matisse = DebugMatisseDevice() + matisse.initializeDevice() + try: + self.assertEqual(matisse.wavelengthRange(), (700.0, 1000.0)) + with self.assertRaises(ValueError) as raised: + matisse.setWavelength(50.0) + self.assertIn("700.0", str(raised.exception)) + matisse.setWavelength(780.0) + finally: + matisse.shutdownDevice() + + def testAnEnumArgumentAcceptsAnythingTheEnumAccepts(self): + lockin = DebugSR830Device() + lockin.initializeDevice() + try: + lockin.setInputSource("Differential") # coerced for the driver + self.assertEqual(lockin.getInputSource(), InputSource.Differential) + with self.assertRaises(ValueError) as raised: + lockin.setInputSource("Telepathy") + self.assertIn("SingleEnded", str(raised.exception)) + finally: + lockin.shutdownDevice() + + def testInstrumentLimitsStayWithTheDriver(self): + # The capability checks the contract (a real number); the SR830's own + # +/-10.5 V limit is the driver's business and still applies. + lockin = DebugSR830Device() + lockin.initializeDevice() + try: + with self.assertRaises(ValueError): + lockin.setAnalogVoltage(50.0, channel=1) + finally: + lockin.shutdownDevice() + + class _FailingAnalogDevice(AnalogIOCapability): """Every hook raises, so the failure path can be exercised.""" diff --git a/hardwarelibrary/tests/testSR830.py b/hardwarelibrary/tests/testSR830.py index 056f46f6..49aecb29 100644 --- a/hardwarelibrary/tests/testSR830.py +++ b/hardwarelibrary/tests/testSR830.py @@ -164,8 +164,9 @@ def testStreamPrimitives(self): def testExternalSampleClockAdvancesOnSoftwareTrigger(self): # With an External sample clock, no samples accrue until each trigger edge. self.device.setTriggerSource(TriggerSource.Internal) + # None, not 0: the documented way to say the rate is the external clock's. self.device.configureStream( - channels=[StreamChannel.X], sampleRate=0, sampleClock=SampleClock.External) + channels=[StreamChannel.X], sampleRate=None, sampleClock=SampleClock.External) self.device.startStream() try: self.assertEqual(self.device.readStream()[StreamChannel.X], []) diff --git a/hardwarelibrary/tests/testValidation.py b/hardwarelibrary/tests/testValidation.py new file mode 100644 index 00000000..ae34af00 --- /dev/null +++ b/hardwarelibrary/tests/testValidation.py @@ -0,0 +1,119 @@ +import env +import unittest +from enum import Enum + +from hardwarelibrary.validation import ( + requireAtLeast, requireBool, requireInteger, requireMember, requireNonEmpty, + requirePositive, requireRealNumber, requireWithinRange) + + +class Colour(Enum): + red = "red" + green = "green" + + +class TestRequireRealNumber(unittest.TestCase): + def testAcceptsIntsAndFloats(self): + self.assertEqual(requireRealNumber(3, "value"), 3) + self.assertEqual(requireRealNumber(-2.5, "value"), -2.5) + + def testRejectsBool(self): + # bool is a subclass of int, so True would otherwise pass as 1. + with self.assertRaises(TypeError): + requireRealNumber(True, "value") + + def testRejectsStringsAndNone(self): + for value in ("2.5", None, [1]): + with self.assertRaises(TypeError): + requireRealNumber(value, "value") + + def testMessageNamesTheParameterAndTheUnit(self): + with self.assertRaises(TypeError) as raised: + requireRealNumber("high", "power", "W") + self.assertIn("power", str(raised.exception)) + self.assertIn("W", str(raised.exception)) + + +class TestRequireInteger(unittest.TestCase): + def testAcceptsInts(self): + self.assertEqual(requireInteger(4, "outlet"), 4) + + def testRejectsFloatsAndBool(self): + for value in (1.5, True): + with self.assertRaises(TypeError): + requireInteger(value, "outlet") + + +class TestRequireBool(unittest.TestCase): + def testAcceptsBools(self): + self.assertIs(requireBool(True, "isOn"), True) + self.assertIs(requireBool(False, "isOn"), False) + + def testAcceptsZeroAndOneAsLogicLevels(self): + self.assertIs(requireBool(1, "isOn"), True) + self.assertIs(requireBool(0, "isOn"), False) + + def testRejectsAnythingElse(self): + for value in ("yes", 2, None, 0.0): + with self.assertRaises(TypeError): + requireBool(value, "isOn") + + +class TestBounds(unittest.TestCase): + def testRequireAtLeastIncludesTheBound(self): + self.assertEqual(requireAtLeast(1, 1, "sampleCount"), 1) + with self.assertRaises(ValueError): + requireAtLeast(0, 1, "sampleCount") + + def testRequirePositiveExcludesZero(self): + self.assertEqual(requirePositive(0.001, "seconds"), 0.001) + for value in (0, -1): + with self.assertRaises(ValueError): + requirePositive(value, "seconds") + + def testRequireWithinRangeIncludesBothEnds(self): + for value in (700.0, 850.0, 1000.0): + self.assertEqual(requireWithinRange(value, (700.0, 1000.0), "wavelength"), value) + for value in (699.9, 1000.1): + with self.assertRaises(ValueError): + requireWithinRange(value, (700.0, 1000.0), "wavelength") + + def testRequireWithinRangePassesWhenNoRangeIsAdvertised(self): + # The check is only as good as what the driver reports. + self.assertEqual(requireWithinRange(1e9, None, "wavelength"), 1e9) + + def testTheMessageCarriesTheBoundAndTheUnit(self): + with self.assertRaises(ValueError) as raised: + requireWithinRange(50.0, (700.0, 1000.0), "wavelength", "nm") + message = str(raised.exception) + self.assertIn("wavelength", message) + self.assertIn("700.0", message) + self.assertIn("1000.0", message) + self.assertIn("nm", message) + + +class TestRequireNonEmpty(unittest.TestCase): + def testAcceptsAPopulatedCollection(self): + self.assertEqual(requireNonEmpty([0, 1], "channels"), [0, 1]) + + def testRejectsAnEmptyOne(self): + with self.assertRaises(ValueError): + requireNonEmpty([], "channels") + + +class TestRequireMember(unittest.TestCase): + def testAcceptsAMemberAndAnythingItIsBuiltFrom(self): + self.assertIs(requireMember(Colour.red, Colour, "colour"), Colour.red) + self.assertIs(requireMember("green", Colour, "colour"), Colour.green) + + def testRejectsAnythingElseNamingWhatWasAllowed(self): + with self.assertRaises(ValueError) as raised: + requireMember("purple", Colour, "colour") + message = str(raised.exception) + self.assertIn("colour", message) + self.assertIn("red", message) + self.assertIn("green", message) + + +if __name__ == "__main__": + unittest.main() diff --git a/hardwarelibrary/validation.py b/hardwarelibrary/validation.py new file mode 100644 index 00000000..755303da --- /dev/null +++ b/hardwarelibrary/validation.py @@ -0,0 +1,110 @@ +"""Argument checks shared by the public methods of the device families. + +These are the contract-level checks: the ones true of every device implementing a +capability, such as a sample count of at least one, or a wavelength inside the +range the instrument itself reports. Limits that vary by model -- the SR830's ++/-10.5 V Aux output, the Millennia's 0.05-25 W -- stay in the driver, next to the +numbers they come from. + +Every function raises TypeError for a value of the wrong kind and ValueError for a +value out of bounds, matching what the drivers already raise, and names the +parameter, the offending value and the bound it broke. Each returns the value, so +a caller may write `power = requireAtLeast(power, 0, "power")`. + +A capability calls these from the validator it hands to @notifies, which runs them +before the operation is announced: a rejected call never touched the hardware, so +no will/did pair is posted for it. +""" + +import numbers + + +def describeValue(value, unit=None) -> str: + """Format a value for an error message, with its unit when there is one.""" + return "{0!r} {1}".format(value, unit) if unit else "{0!r}".format(value) + + +def requireRealNumber(value, name, unit=None): + """Require a real number: an int or a float, but not a bool and not a string. + + bool is a subclass of int in Python, so True would otherwise pass as the + number 1 -- a logic level is not a measurement. + """ + if isinstance(value, bool) or not isinstance(value, numbers.Real): + raise TypeError("{0} must be a real number, got {1}".format( + name, describeValue(value, unit))) + return value + + +def requireInteger(value, name): + """Require a whole number, again excluding bool.""" + if isinstance(value, bool) or not isinstance(value, numbers.Integral): + raise TypeError("{0} must be an integer, got {1!r}".format(name, value)) + return value + + +def requireBool(value, name): + """Require a logic level. 0 and 1 are accepted and converted, since a digital + line is as often written as an int as as a bool; anything else is refused.""" + if isinstance(value, bool): + return value + if isinstance(value, numbers.Integral) and value in (0, 1): + return bool(value) + raise TypeError( + "{0} must be True or False (0 and 1 are accepted), got {1!r}".format(name, value)) + + +def requireAtLeast(value, minimum, name, unit=None): + """Require a real number no smaller than minimum.""" + requireRealNumber(value, name, unit) + if value < minimum: + raise ValueError("{0} must be at least {1}, got {2}".format( + name, describeValue(minimum, unit), describeValue(value, unit))) + return value + + +def requirePositive(value, name, unit=None): + """Require a real number strictly greater than zero.""" + requireRealNumber(value, name, unit) + if value <= 0: + raise ValueError("{0} must be greater than zero, got {1}".format( + name, describeValue(value, unit))) + return value + + +def requireWithinRange(value, limits, name, unit=None): + """Require a real number inside limits, a (low, high) pair, ends included. + + limits of None means the instrument does not report one, and the value passes: + the check is only as good as what the driver advertises. + """ + requireRealNumber(value, name, unit) + if limits is None: + return value + low, high = limits + if not low <= value <= high: + raise ValueError( + "{0} must be within [{1}, {2}], got {3}".format( + name, describeValue(low, unit), describeValue(high, unit), + describeValue(value, unit))) + return value + + +def requireNonEmpty(collection, name): + """Require a collection with at least one element.""" + if len(collection) == 0: + raise ValueError("{0} must not be empty".format(name)) + return collection + + +def requireMember(value, enumeration, name): + """Require a member of enumeration, accepting anything it can be built from. + + Enum() already raises for an unknown value, but its message names neither the + parameter nor what was allowed, which is what a user needs to fix the call. + """ + try: + return enumeration(value) + except ValueError: + raise ValueError("{0} must be one of {1}, got {2!r}".format( + name, ", ".join(member.name for member in enumeration), value)) from None