Skip to content

Commit cc230dd

Browse files
dccoteclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 238648c commit cc230dd

5 files changed

Lines changed: 21 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,10 @@ API changes can land even when the minor version is unchanged.
5959
`AttributeError: 'NoneType' object has no attribute ...` on a port that was never
6060
opened, or -- on a debug device -- answered as though the hardware had done it and
6161
posted a `did*` claiming success. The check runs before the `will` is posted, so a
62-
refused call announces nothing. `PhysicalDevice.validateReady` is the real check;
63-
`Capability.validateReady` is a no-op behind it in the MRO so a mixin can still be
64-
exercised on its own. Methods that only report what a model supports
62+
refused call announces nothing. `validateReady` is defined once, on
63+
`PhysicalDevice`, where the device lifecycle belongs; `capabilities.py` only calls
64+
it, so a class mixing in a capability without being a `PhysicalDevice` answers for
65+
readiness itself rather than silently skipping the check. Methods that only report what a model supports
6566
(`supportedInputSources`, `supportedSensitivities`, `supportedTimeConstants`,
6667
`supportedTriggerSources`, `outletCount`) are exempt via `requiresReady=False`,
6768
since a UI populates its menus before connecting.

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ All families **use interface-segregated capability mixins** instead of one fat b
6868
- An operation that changes the instrument posts `will<Stem>` before and `did<Stem>` after; a **read (`doGet*`, `doReadStream`) posts only `did<Stem>`**, 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.
6969
- **`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.
7070
- `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.
71-
- **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.
71+
- **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.
7272
- **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.
7373
- 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.
7474
- Composed hooks nest: `acquireWaveform` posts its own pair plus the pairs of the `configureStream` / `startStream` / `readStream` / `stopStream` calls its default implementation makes.

‎hardwarelibrary/capabilities.py‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ def notifies(did, will=None, requiresReady=True):
6565
observer should hear nothing at all. Pass requiresReady=False for a method
6666
that only reports what the instrument supports, which a UI may legitimately
6767
ask before connecting.
68+
69+
validateReady is PhysicalDevice's, which is where the device lifecycle lives;
70+
a capability is meant to be mixed alongside one, so anything else hosting a
71+
capability must answer for readiness itself.
6872
"""
6973
def decorator(method):
7074
signature = inspect.signature(method)
@@ -114,17 +118,6 @@ class Capability(ABC):
114118
# allCapabilities() also enumerates every notification the library defines.
115119
notification = None
116120

117-
def validateReady(self, operation=None):
118-
"""Confirm the device is initialized before an operation touches it.
119-
120-
A mixin standing on its own has no device state to check, so this does
121-
nothing. PhysicalDevice sits ahead of every capability in a driver's MRO
122-
and overrides it with the real check, which is the one that runs on real
123-
hardware; this fallback exists so a capability can still be exercised
124-
bare, as the tests do.
125-
"""
126-
pass
127-
128121

129122
# ---------------------------------------------------------------------------
130123
# Laser source capabilities

‎hardwarelibrary/tests/testCapabilities.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ class _RecordingAnalogDevice(AnalogIOCapability):
157157
def __init__(self):
158158
self.calls = []
159159

160+
def validateReady(self, operation=None):
161+
"""Stands in for a device that is open and ready."""
162+
pass
163+
160164
def doGetAnalogVoltage(self, channel):
161165
self.calls.append(("doGetAnalogVoltage", channel))
162166
return 1.5
@@ -327,6 +331,10 @@ class _FailingAnalogDevice(AnalogIOCapability):
327331
class Failure(RuntimeError):
328332
pass
329333

334+
def validateReady(self, operation=None):
335+
"""Stands in for a device that is open and ready."""
336+
pass
337+
330338
def doGetAnalogVoltage(self, channel):
331339
raise self.Failure("no hardware")
332340

‎hardwarelibrary/tests/testSR830.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,10 @@ class _MinimalLockIn(PhaseLockedDetectionCapability, TriggerCapability):
233233
the base-class optional hooks and the base getDemodulatedValues are exercised
234234
(SR830Device overrides all of these)."""
235235

236+
def validateReady(self, operation=None):
237+
"""Stands in for a device that is open and ready."""
238+
pass
239+
236240
def doGetInPhaseVoltage(self):
237241
return 0.1
238242

0 commit comments

Comments
 (0)