Repository navigation
Fix 10 latent bugs uncovered by static review - #74
Merged
Merged
Conversation
Problem: USBDeviceDescriptor.matchesPhysicalDevice() returned False when the serialNumber regex *matched* the device's serial number, which is the opposite of the intended semantics. As a result, DeviceManager.usbDeviceDisconnected could not identify and remove the matching PhysicalDevice from its set when a USB device was unplugged. Solution: invert the regex condition (is not None -> is None) so that a successful regex match falls through to return True, and a non-match returns False. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: DebugRotationDevice.doMoveBy(displacement) incremented self._debugOrientation by 'angle', which is not defined in this method. Any call to doMoveBy would raise NameError. Solution: reference the actual parameter, 'displacement'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: Spectrometer.showHelp formatted its help string with ', '.join(Spectrometer.supportedClassNames) -- without parentheses. supportedClassNames is a classmethod, so the join was attempting to iterate a bound method object, which would raise TypeError whenever showHelp was invoked (i.e. when guiding a user through a startup error). Solution: add the missing () so the classmethod is actually called, returning the list of supported class names. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: OISpectrometer.doInitializeDevice sent the four ASCII bytes b'0x01' (i.e. the characters '0','x','0','1') to the spectrometer instead of the single byte 0x01. The spectrometer protocol expects a one-byte initialization opcode, so the device was almost certainly receiving an unrecognized command. Every other sendCommand call in the file uses the proper escape form (b'\\x05', b'\\xfe', b'\\x02'). Solution: replace b'0x01' with b'\\x01' so the single init byte is transmitted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: physicaldevice.py used 'import hardwarelibrary.utils', which only binds the dotted name 'hardwarelibrary.utils' and not the short name 'utils'. Every call to utils.getAllUSBIds(...), utils.connectedUSBDevices(...), and utils.getCandidateDeviceClasses(...) in PhysicalDevice.any, .connectedDevices, and .anyDevice would therefore raise NameError. Solution: switch to 'from hardwarelibrary import utils', which binds the short name and matches the existing call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: PowerMeterDevice defined measureAbsolutePower twice with identical bodies. Python silently retained only the second definition; the first was dead code, and the duplication invited divergence on future edits. Solution: delete the second copy, keeping the original definition that sits next to __init__ and the rest of the public API. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: DebugPort.readData and DebugPort.writeData set endPointIndex = 0 only when their endPoint argument was None. Any caller passing an explicit endPoint left endPointIndex unbound, so the subsequent reference (self.outputBuffers[endPointIndex] / self.inputBuffers[endPointIndex]) would raise UnboundLocalError. Solution: collapse the conditional into a single expression -- 'endPointIndex = 0 if endPoint is None else endPoint' -- so the variable is always bound. Mirrors the default-to-0 pattern already used by bytesAvailable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: SutterDevice.doInitializeDevice formatted its
'Cannot allocate port {0}' error with self.portPath, but
SutterDevice never sets self.portPath -- the port path is only a
local variable inside the non-debug branch. If the 'self.port is
None' guard ever fired, the error message would raise
AttributeError and mask the real failure.
Solution: format the message with self.serialNumber, which is
always defined on the device and conveys the relevant identifier.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: CoboltDebugSerial.open/close/read/write and the helper increasePowerSlowlyInBackground all used 'with globalLock:' to serialize access from a background power-ramp thread, but globalLock was never defined or imported in cobolt.py (the only real definition lived in tests/testCommunicationPorts.py). Any debug-mode operation would raise NameError. Solution: define a module-level 'globalLock = RLock()' near the top of cobolt.py. RLock was already imported. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Problem: CoboltDevice.doInitializeDevice had two broken issues in
its except clauses. The first 'except Exception as error' caught
*everything* -- including the explicit
'PhysicalDevice.UnableToInitialize(\"Cannot allocate port {0}\"...)'
raised on the previous line -- and re-raised a blank
UnableToInitialize() with no message, discarding the diagnostic
text. The second clause referenced 'PhysicalDeviceUnableToInitialize'
which is not defined anywhere, and was unreachable in any case
because of the broad except above it.
Solution: reorder so the specific exception is caught first and
re-raised unchanged ('raise'), preserving its message; the generic
'except Exception' then wraps anything else as
UnableToInitialize. Also flattened the nested 'is not None / isOpen'
checks for readability.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 18, 2026
dccote
added a commit
that referenced
this pull request
May 18, 2026
Two strings/references were carried over from sutterdevice.py unchanged when this file was first copied: - Error message said "No Sutter Device connected" -- updated to "No Thorlabs Device connected". - The "Cannot allocate port" message referenced self.portPath, which is never set on ThorlabsDevice (the value is a local in the non-debug branch only). Replaced with self.serialNumber, matching the fix landed on sutterdevice.py in PR #74. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ten targeted fixes for latent bugs found during a static review of
master. Each commit stands alone with a Problem/Solution body in its message — none are bundled.devicemanager.pyglobalLockbrokeCoboltDebugSerialin debug modesources/cobolt.pyexcept Exceptiondiscarded theUnableToInitializemessage; the follow-upexceptclause referenced an undefined namesources/cobolt.pyDebugRotationDevice.doMoveByused the wrong identifier (angleinstead ofdisplacement)motion/rotationdevice.pySpectrometer.showHelpjoined a bound method instead of calling itspectrometers/base.pyb'0x01'instead of the single byteb'\x01'spectrometers/oceaninsight.pyimport hardwarelibrary.utilsdidn't bind the short nameutilsused throughout the filephysicaldevice.pyPowerMeterDevice.measureAbsolutePowerwas defined twice (duplicate)powermeters/powermeterdevice.pyDebugPort.readData/writeDataleftendPointIndexunbound on any explicitendPointargumentcommunication/debugport.pySutterDeviceformatted an init error withself.portPath, which is never set on the instancemotion/sutterdevice.pyNo behavior changes for working code paths; all fixes target code that was either dead or already broken.
Test plan
python3 -m pytest hardwarelibrary/tests/testCommandRecognition.py hardwarelibrary/tests/testTableDrivenDebugPort.py hardwarelibrary/tests/testCommunicationPorts.py hardwarelibrary/tests/testPhysicalDevice.py -vDeviceManagerdisconnect flow at least once (the inverted-match fix First echo implementation #1 is the only one that changes behavior for a currently-working call site)portPath="debug") — exercises fixes PhysicalDevice base class #2 and Merge changes from 'Physical device' to 'MaxCobolt' #3doInitializeDevicesucceeds (fix Added testAutostartOffOn and modified CoboltDebugSerial behavior to m… #6 changes the wire bytes sent during init)🤖 Generated with Claude Code