From 56a6d2833fab1cafbe01bc161571d915d327d8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20C=C3=B4t=C3=A9?= Date: Mon, 6 Jul 2026 13:08:30 -0400 Subject: [PATCH] Fix intermittent hang in OISpectrometer.getSpectrum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: getSpectrum waited for the 'spectrum ready' flag in an unbounded loop; on a transient USB glitch the flag could stay down, so it re-requested a spectrum forever (process pinned ~30% CPU, never returning). Seen intermittently on an USB2000+ during long repeated acquisitions. Solution: bound the wait — at most maxRequests requests (default 4), each awaited up to maxWait seconds (default 2.0). Raise the existing SpectrumRequestTimeoutError in bounded time instead of looping. Absorb transient usb.core.USBError (incl. USBTimeoutError) from requestSpectrum()/isSpectrumReady() and retry rather than propagating on the first hiccup. integrationTime stays the first optional argument for back-compatibility; subclasses that override getSpectrumData still inherit this getSpectrum. Adds testOISpectrometer.py (hardware-free mock) verifying the bounded-timeout and happy paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- hardwarelibrary/spectrometers/oceaninsight.py | 44 ++++++++++----- hardwarelibrary/tests/testOISpectrometer.py | 53 +++++++++++++++++++ 2 files changed, 84 insertions(+), 13 deletions(-) create mode 100644 hardwarelibrary/tests/testOISpectrometer.py diff --git a/hardwarelibrary/spectrometers/oceaninsight.py b/hardwarelibrary/spectrometers/oceaninsight.py index 7595def..39dbe8e 100644 --- a/hardwarelibrary/spectrometers/oceaninsight.py +++ b/hardwarelibrary/spectrometers/oceaninsight.py @@ -473,18 +473,29 @@ def getSpectrumData(self): """ raise NotImplementedError('You must implemented getSpectrumData for your subclass.') - def getSpectrum(self, integrationTime=None): + def getSpectrum(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, - 3- waiting until ready, then + 3- waiting until ready, then 4- actually retrieving and returning the data. - + + The wait is bounded: at most `maxRequests` requests are issued, each + awaited up to `maxWait` seconds. Rather than looping forever when the + 'spectrum ready' flag never rises (a transient USB glitch can leave it + stuck), SpectrumRequestTimeoutError is raised in bounded time. Transient + USB errors raised by requestSpectrum()/isSpectrumReady() are absorbed + and retried instead of propagating on the first hiccup. + Parameters ---------- - integrationTime: int, default None + integrationTime: int, default None integration time in milliseconds if not the currently configured time. + maxRequests: int, default 4 + maximum number of spectrum requests before giving up. + maxWait: float, default 2.0 + seconds to wait for the 'spectrum ready' flag after each request. Returns ------- @@ -496,15 +507,22 @@ def getSpectrum(self, integrationTime=None): if integrationTime is not None: self.setIntegrationTime(integrationTime) - self.requestSpectrum() - timeOut = time.time() + 1 - while not self.isSpectrumReady(): - time.sleep(0.001) - if time.time() > timeOut: - self.requestSpectrum() # makes no sense, let's request another one - timeOut = time.time() + 1 - - return self.getSpectrumData() + lastError = None + for _ in range(maxRequests): + try: + self.requestSpectrum() + deadline = time.time() + maxWait + while time.time() < deadline: + if self.isSpectrumReady(): + return self.getSpectrumData() + time.sleep(0.001) + except usb.core.USBError as err: # includes USBTimeoutError + lastError = err + time.sleep(0.02) + + raise SpectrumRequestTimeoutError( + "Spectrum never became ready after {0} requests " + "(last USB error: {1})".format(maxRequests, lastError)) def sendCommand(self, cmdBytes, payloadBytes=None): """ Main entry point to write to the device in order to have diff --git a/hardwarelibrary/tests/testOISpectrometer.py b/hardwarelibrary/tests/testOISpectrometer.py new file mode 100644 index 0000000..92d0885 --- /dev/null +++ b/hardwarelibrary/tests/testOISpectrometer.py @@ -0,0 +1,53 @@ +import env +import unittest +import time + +from hardwarelibrary.spectrometers.oceaninsight import ( + OISpectrometer, SpectrumRequestTimeoutError, +) + + +class MockOISpectrometer(OISpectrometer): + """Hardware-free OISpectrometer for exercising getSpectrum() without USB. + + Bypasses the USB-connecting __init__ and stubs the three hooks getSpectrum + relies on. `spectrumReady` controls whether the 'spectrum ready' flag rises. + """ + + def __init__(self, spectrumReady=False): + self.spectrumReady = spectrumReady + self.requestCount = 0 + + def requestSpectrum(self): + self.requestCount += 1 + + def isSpectrumReady(self): + return self.spectrumReady + + def getSpectrumData(self): + return "spectrum" + + +class TestGetSpectrumBounded(unittest.TestCase): + def testRaisesInsteadOfBlockingWhenNeverReady(self): + device = MockOISpectrometer(spectrumReady=False) + start = time.time() + with self.assertRaises(SpectrumRequestTimeoutError): + device.getSpectrum(maxRequests=2, maxWait=0.05) + elapsed = time.time() - start + self.assertLess(elapsed, 10.0) + + def testStopsAfterMaxRequests(self): + device = MockOISpectrometer(spectrumReady=False) + with self.assertRaises(SpectrumRequestTimeoutError): + device.getSpectrum(maxRequests=2, maxWait=0.05) + self.assertEqual(device.requestCount, 2) + + def testReturnsDataWhenReady(self): + device = MockOISpectrometer(spectrumReady=True) + self.assertEqual(device.getSpectrum(maxRequests=2, maxWait=0.05), "spectrum") + self.assertEqual(device.requestCount, 1) + + +if __name__ == "__main__": + unittest.main()