From 74d89ad27b6d566715ab695ede0d25106c668670 Mon Sep 17 00:00:00 2001 From: Sequoia Ploeg Date: Mon, 5 Oct 2026 11:25:16 -0600 Subject: [PATCH 1/5] ppcl55x: raise real errors, and don't hang after one _receive raised CommunicationError, which was never imported, so a bad checksum or failed read became a NameError (#88), and a timeout returned the in-band value (0xFF, 0xFF, 0xFF, 0xFF) (#58). Now a timeout raises TimeoutError and a failed read or bad checksum raises OSError, chained to the serial error (#60). They are built-in exceptions because Pyro can only rebuild those on a client, and a Pyro CommunicationError (the issue's suggestion) would make the client's proxy drop its connection and lock. _communicate's lock was created fresh on every call, so it serialized nothing, and a failed exchange never left the ticket queue, so every later call spun forever. A single per-laser lock replaces both. The wait for a reply now sleeps instead of busy-polling. on() and off() send ten no-ops as a delay; one going unanswered is still tolerated, as it was when timeouts returned a value. Fixes #88, fixes #58 Co-Authored-By: Claude Opus 5.5 --- pyrolab/drivers/lasers/ppcl55x.py | 95 ++++++++------ tests/drivers/lasers/test_ppcl55x.py | 186 +++++++++++++++++++++++++++ 2 files changed, 241 insertions(+), 40 deletions(-) create mode 100644 tests/drivers/lasers/test_ppcl55x.py diff --git a/pyrolab/drivers/lasers/ppcl55x.py b/pyrolab/drivers/lasers/ppcl55x.py index 13b6a4f..a0a741b 100644 --- a/pyrolab/drivers/lasers/ppcl55x.py +++ b/pyrolab/drivers/lasers/ppcl55x.py @@ -37,6 +37,8 @@ log = logging.getLogger(__name__) +# How long to wait for the laser to reply to a message, in seconds. +REPLY_TIMEOUT = 0.5 ITLA_NOERROR = 0x00 ITLA_EXERROR = 0x01 @@ -140,8 +142,8 @@ def connect(self, port: str = "", baudrate: int = 9600) -> None: log.debug("Entering connect()") self.latest_register = 0 - self.queue = [] - self.max_row_ticket = 0 + if not hasattr(self, "_lock"): + self._lock = threading.Lock() if hasattr(self, "device") and self.device.is_open: log.debug("Already connected") @@ -391,9 +393,8 @@ def on(self) -> int: """ # start communication by sending 8 to REG_Resena register response = self._communicate(REG_Resena, 8, 1) - for i in range(10): - # send 0 to REG_Nop to allow time for laser diode to turn on - response = self._communicate(REG_Nop, 0, 0) + # send 0 to REG_Nop to allow time for laser diode to turn on + response = self._wait_with_nops(response) self.is_on = True return response @@ -408,12 +409,26 @@ def off(self) -> int: """ # stop communication by sending 0 to REG_Resena register response = self._communicate(REG_Resena, 0, 1) - for i in range(10): - # send 0 to REG_Nop to allow time for laser diode to turn on - response = self._communicate(REG_Nop, 0, 0) + # send 0 to REG_Nop to allow time for laser diode to turn off + response = self._wait_with_nops(response) self.is_on = False return response + def _wait_with_nops(self, response: List[int], count: int = 10) -> List[int]: + """ + Send ``count`` no-ops to give the laser time to change state. + + The no-ops are only a delay, so one going unanswered is not an error + (the reply to the command they follow already showed the laser is + there). Returns the last reply received, or ``response`` if none was. + """ + for _ in range(count): + try: + response = self._communicate(REG_Nop, 0, 0) + except TimeoutError: + log.debug("No reply to a no-op while waiting; continuing") + return response + def _communicate(self, register: int, data: int, write_read: int) -> int: """ Function that implements the commmunication with the laser. It will @@ -434,27 +449,24 @@ def _communicate(self, register: int, data: int, write_read: int) -> int: ------- int Integer representing error message, 0 if no error. - """ - lock = threading.Lock() - lock.acquire() - row_ticket = self.max_row_ticket + 1 - self.max_row_ticket = self.max_row_ticket + 1 - self.queue.append(row_ticket) - lock.release() - while self.queue[0] != row_ticket: - row_ticket = row_ticket + Raises + ------ + TimeoutError + If the laser doesn't reply in time. + OSError + If the reply can't be read or fails its checksum. + """ data_byte_0 = int(data / 256) data_byte_1 = int(data - data_byte_0 * 256) - self.latest_register = register # modify bytes for sending message = [write_read, register, data_byte_0, data_byte_1] - self._send(message) # send the message - received_message = self._receive() # receive the response from the laser - lock.acquire() - self.queue.pop(0) - lock.release() - # error_message = int(received_message[0] & 0x03) - return received_message + # One exchange at a time: a reply must be read before the next + # message is sent, or replies would be matched to the wrong requests. + with self._lock: + self.latest_register = register # modify bytes for sending + self._send(message) # send the message + # error_message = int(received_message[0] & 0x03) + return self._receive() # receive the response from the laser """ Function sends message of four bytes to the laser. @@ -489,30 +501,33 @@ def _receive(self) -> List[int]: Raises ------ - PyroLabException.CommunicationException + TimeoutError + If the laser doesn't reply within ``REPLY_TIMEOUT`` seconds. + OSError + If the reply can't be read or fails its checksum. """ - - reference_time = time.time() + # Built-in exceptions only: Pyro can't send PyroLab's or pyserial's + # exception classes to a client, and a Pyro CommunicationError would + # make the client's proxy drop its connection (and any lock it holds). + deadline = time.monotonic() + REPLY_TIMEOUT while self.device.inWaiting() < 4: # wait until 4 bytes are received - if ( - time.time() > reference_time + 0.5 - ): # if it takes longer than 0.5 seconds break - return (0xFF, 0xFF, 0xFF, 0xFF) + if time.monotonic() > deadline: + raise TimeoutError( + f"No reply from the laser within {REPLY_TIMEOUT} seconds" + ) + time.sleep(0.001) try: num = self.device.inWaiting() # get number of bytes for debugging purposes - message = [] bytes_read = self.device.read(num) # read the four bytes from serial - for b in bytes_read: - message.append(b) # construct array of bytes from the message - message = message[0:4] - except: - raise CommunicationError("No response from laser") + except serial.SerialException as e: + raise OSError(f"Reading the laser's reply failed: {e}") from e + # construct array of bytes from the message + message = list(bytes_read)[0:4] if self._checksum(message) == message[0] >> 4: # ensure the checksum is correct log.debug(f"message received: {message[2]} {message[3]}") return message # return the message received else: - # if the checksum is wrong, log a CS error - raise CommunicationError("Incorrect checksum returned") + raise OSError(f"The laser's reply failed its checksum: {message}") def _checksum(self, message: List[int]) -> int: # calculate checksum """ diff --git a/tests/drivers/lasers/test_ppcl55x.py b/tests/drivers/lasers/test_ppcl55x.py new file mode 100644 index 0000000..5caa432 --- /dev/null +++ b/tests/drivers/lasers/test_ppcl55x.py @@ -0,0 +1,186 @@ +""" +PPCL55x laser driver tests, with a fake serial port in place of the laser. +""" + +import threading + +import pytest + +pytest.importorskip("serial") + +import serial # noqa: E402 +from Pyro5 import serializers # noqa: E402 + +from pyrolab.drivers.lasers import ppcl55x # noqa: E402 + + +class FakeSerial: + """ + Answers each message with the next queued reply: bytes to deliver, None to + stay silent (a timeout), or an exception to raise from ``read``. + """ + + def __init__(self, replies): + self.replies = list(replies) + self.sent = [] + self.buffer = b"" + self.read_error = None + self.is_open = True + + def flush(self): + pass + + def write(self, data): + self.sent.append(data) + reply = self.replies.pop(0) if self.replies else None + if isinstance(reply, Exception): + self.buffer, self.read_error = b"\x00" * 4, reply + elif reply is not None: + self.buffer = reply + + def inWaiting(self): + return len(self.buffer) + + def read(self, n): + if self.read_error is not None: + error, self.read_error = self.read_error, None + raise error + data, self.buffer = self.buffer[:n], self.buffer[n:] + return data + + def close(self): + self.is_open = False + + +def reply(register=0, data=0, corrupt=False): + """A reply from the laser, with a valid checksum unless ``corrupt``.""" + message = [0, register, data >> 8, data & 0xFF] + message[0] |= ppcl55x.PPCL55xBase._checksum(None, message) << 4 + if corrupt: + message[3] ^= 0x01 + return bytes(message) + + +@pytest.fixture +def laser(monkeypatch): + """Returns a function making a connected laser that gives ``replies``.""" + monkeypatch.setattr(ppcl55x, "REPLY_TIMEOUT", 0.05) + + def make(*replies): + device = FakeSerial(replies) + monkeypatch.setattr(ppcl55x.serial, "Serial", lambda *a, **k: device) + laser = ppcl55x.PPCL550() + laser.connect(port="COM1") + return laser, device + + return make + + +def test_a_reply_is_returned(laser): + laser, _ = laser(reply(ppcl55x.REG_Oop, 1234)) + message = laser._communicate(ppcl55x.REG_Oop, 0, 0) + assert (message[2] << 8) | message[3] == 1234 + + +def test_no_reply_raises_timeout_error(laser): + # Regression: a timeout returned (0xFF, 0xFF, 0xFF, 0xFF), which callers + # couldn't tell from data (#58) + laser, _ = laser(None) + with pytest.raises(TimeoutError): + laser._communicate(ppcl55x.REG_Oop, 0, 0) + + +def test_a_bad_checksum_raises_os_error(laser): + # Regression: raised CommunicationError, which was never imported, so it + # became a NameError (#88) + laser, _ = laser(reply(corrupt=True)) + with pytest.raises(OSError, match="checksum"): + laser._communicate(ppcl55x.REG_Oop, 0, 0) + + +def test_a_failed_read_keeps_the_serial_error(laser): + # Regression: a bare except replaced the serial error (#60) + error = serial.SerialException( + "device reports readiness to read but returned no data" + ) + laser, _ = laser(error) + with pytest.raises(OSError) as info: + laser._communicate(ppcl55x.REG_Oop, 0, 0) + assert info.value.__cause__ is error + + +def test_a_failed_exchange_does_not_block_the_next(laser): + # Regression: a failed exchange never gave up its place in the queue, so + # every later call spun forever. + laser, _ = laser(None, reply(ppcl55x.REG_Oop, 7)) + with pytest.raises(TimeoutError): + laser._communicate(ppcl55x.REG_Oop, 0, 0) + + result = [] + worker = threading.Thread( + target=lambda: result.append(laser._communicate(ppcl55x.REG_Oop, 0, 0)), + daemon=True, + ) + worker.start() + worker.join(timeout=5) + assert not worker.is_alive(), "the next exchange hung" + assert result[0][3] == 7 + + +def test_concurrent_exchanges_are_not_interleaved(laser): + laser, device = laser(*[reply(ppcl55x.REG_Oop, n) for n in range(40)]) + original_write = device.write + inside = [] + + def write(data): + inside.append(1) + assert len(inside) == 1, "two exchanges overlapped" + original_write(data) + + original_read = device.read + + def read(n): + data = original_read(n) + inside.pop() + return data + + device.write, device.read = write, read + errors = [] + + def talk(): + try: + for _ in range(10): + laser._communicate(ppcl55x.REG_Oop, 0, 0) + except Exception as e: + errors.append(e) + + workers = [threading.Thread(target=talk) for _ in range(4)] + for w in workers: + w.start() + for w in workers: + w.join(timeout=10) + assert not errors + + +def test_turning_on_tolerates_unanswered_no_ops(laser): + # The no-ops after enabling the laser are only a delay. + laser, device = laser(reply(ppcl55x.REG_Resena), *[None] * 10) + laser.on() + assert laser.is_on + assert len(device.sent) == 11 + + +def test_turning_on_an_unresponsive_laser_fails(laser): + laser, _ = laser(None) + with pytest.raises(TimeoutError): + laser.on() + + +@pytest.mark.parametrize("error", [TimeoutError("late"), OSError("bad checksum")]) +def test_the_errors_reach_pyro_clients_intact(error): + # Pyro can only rebuild built-in (and Pyro's own) exception classes on + # the client; anything else arrives as a SerializeError. + serializer = serializers.serializers["serpent"] + received = serializer.loads(serializer.dumps(error)) + assert type(received) is type(error) + assert received.args == error.args From 16ee564adaa5664d9e37afeb2373f13494acff42 Mon Sep 17 00:00:00 2001 From: Sequoia Ploeg Date: Mon, 5 Oct 2026 11:25:16 -0600 Subject: [PATCH 2/5] Replace the remaining bare except clauses They also caught KeyboardInterrupt and SystemExit, and threw away the original error. TSL550.wavelength_logging's loop never repeated; it now raises TimeoutError saying how far the data got, or OSError chained to the serial error, instead of an opaque RuntimeError. The commented-out TSL550 test scaffold is replaced with real tests. Fixes #60 Co-Authored-By: Claude Opus 5.5 --- pyrolab/drivers/cameras/uc480.py | 4 +- pyrolab/drivers/lasers/tsl550.py | 24 ++++---- pyrolab/drivers/motion/kinesis/kdc101.py | 5 +- pyrolab/nameserver.py | 2 +- tests/drivers/lasers/test_tsl550.py | 76 +++++++++++++++++++----- 5 files changed, 82 insertions(+), 29 deletions(-) diff --git a/pyrolab/drivers/cameras/uc480.py b/pyrolab/drivers/cameras/uc480.py index b5edb26..c159240 100644 --- a/pyrolab/drivers/cameras/uc480.py +++ b/pyrolab/drivers/cameras/uc480.py @@ -36,8 +36,8 @@ try: from thorlabs_kinesis import thor_camera as tc -except: - pass +except (ImportError, OSError) as e: # not installed, or its DLL can't be loaded + logging.getLogger(__name__).debug("ThorCam SDK unavailable: %s", e) from pyrolab.api import expose from pyrolab.drivers.cameras.thorcam import ThorCamBase, ThorCamClient diff --git a/pyrolab/drivers/lasers/tsl550.py b/pyrolab/drivers/lasers/tsl550.py index 9608f4e..0dc173f 100644 --- a/pyrolab/drivers/lasers/tsl550.py +++ b/pyrolab/drivers/lasers/tsl550.py @@ -1405,16 +1405,20 @@ def wavelength_logging(self) -> List[float]: # Iterate through wavelength points for nWave in range(int(num_points)): - while True: - try: - # A data is in 4-byte binary, and transmitted in Big endians - in_byte = self.device.read(4) - # Data format is integer number in 0.1 pm units - current_wavelength = float(struct.unpack(">I", in_byte)[0]) / 1e4 - break - except: - raise RuntimeError("Error reading wavelength data from laser") - + try: + # A data is in 4-byte binary, and transmitted in Big endians + in_byte = self.device.read(4) + except serial.SerialException as e: + raise OSError( + f"Reading wavelength data from the laser failed: {e}" + ) from e + if len(in_byte) < 4: # the read timed out + raise TimeoutError( + f"The laser stopped sending wavelength data after " + f"{nWave} of {int(num_points)} points" + ) + # Data format is integer number in 0.1 pm units + current_wavelength = float(struct.unpack(">I", in_byte)[0]) / 1e4 wavelength_points.append(current_wavelength) # stop laser from outputting diff --git a/pyrolab/drivers/motion/kinesis/kdc101.py b/pyrolab/drivers/motion/kinesis/kdc101.py index e652b72..649acf9 100644 --- a/pyrolab/drivers/motion/kinesis/kdc101.py +++ b/pyrolab/drivers/motion/kinesis/kdc101.py @@ -64,9 +64,10 @@ def check_error(status): log.info("Building ThorLabs device list (requires ThorLabs Kinesis DLL)") try: kcdc.TLI_BuildDeviceList() -except: +except Exception: log.warning( - "Building ThorLabs device list failed; unable to connect to any instruments" + "Building ThorLabs device list failed; unable to connect to any instruments", + exc_info=True, ) diff --git a/pyrolab/nameserver.py b/pyrolab/nameserver.py index e8f92c3..22b7288 100644 --- a/pyrolab/nameserver.py +++ b/pyrolab/nameserver.py @@ -118,7 +118,7 @@ def start_ns_loop( try: # Placed in a try block because this fails with pythonw.exe sys.stdout.flush() - except: + except Exception: # under pythonw, stdout is None log.warning( "Couldn't flush stdout! (Not a problem if running under pythonw.exe)" ) diff --git a/tests/drivers/lasers/test_tsl550.py b/tests/drivers/lasers/test_tsl550.py index bdcba91..021ede1 100644 --- a/tests/drivers/lasers/test_tsl550.py +++ b/tests/drivers/lasers/test_tsl550.py @@ -1,19 +1,67 @@ -# from pathlib import Path -# import os -# import pytest +""" +TSL550 laser driver tests, with a fake serial port in place of the laser. +""" -# if +import struct +import pytest -# # pytest.mark.skipif(True) +pytest.importorskip("serial") -# # @pytest.fixture(scope="module") -# # def script_loc(request): -# # return Path(request.fspath).parent +import serial # noqa: E402 -# def test_script_loc(driver_tests): -# # baseline = script_loc / ("baseline") -# # print(baseline) -# # print(os.getenv('PYTEST_CURRENT_TEST')) -# print(driver_tests) -# assert 0 +from pyrolab.drivers.lasers.tsl550 import TSL550 # noqa: E402 + + +class FakeSerial: + def __init__(self, data=b"", error=None): + self.data = data + self.error = error + + def read(self, n): + if self.error is not None: + raise self.error + chunk, self.data = self.data[:n], self.data[n:] + return chunk + + def close(self): + pass + + +@pytest.fixture +def laser(monkeypatch): + def make(points, device): + laser = TSL550() + laser.device = device + monkeypatch.setattr(laser, "wavelength_logging_number", lambda: points) + monkeypatch.setattr(laser, "write", lambda command: None) + monkeypatch.setattr(laser, "query", lambda command: "") + monkeypatch.setattr("time.sleep", lambda seconds: None) + return laser + + return make + + +def encode(*wavelengths_nm): + return b"".join(struct.pack(">I", round(w * 1e4)) for w in wavelengths_nm) + + +def test_wavelength_logging_reads_every_point(laser): + laser = laser(3, FakeSerial(encode(1550.0, 1550.1, 1550.2))) + assert laser.wavelength_logging() == [1550.0, 1550.1, 1550.2] + + +def test_wavelength_logging_reports_where_the_data_stopped(laser): + # Regression: a short read became an opaque "Error reading wavelength + # data", with the original error discarded (#60) + laser = laser(3, FakeSerial(encode(1550.0) + b"\x00\x01")) + with pytest.raises(TimeoutError, match="after 1 of 3 points"): + laser.wavelength_logging() + + +def test_wavelength_logging_keeps_the_serial_error(laser): + error = serial.SerialException("port closed") + laser = laser(2, FakeSerial(error=error)) + with pytest.raises(OSError) as info: + laser.wavelength_logging() + assert info.value.__cause__ is error From e0dc2644e2a12175c2d7c659373688530233a58a Mon Sep 17 00:00:00 2001 From: Sequoia Ploeg Date: Mon, 5 Oct 2026 11:25:16 -0600 Subject: [PATCH 3/5] Arduino: give up on an analog pin that never reports analog_read spun at full CPU until the pin reported, forever if it never did, hanging the whole daemon. It now takes a timeout (default 1 s), sleeps between polls, and raises TimeoutError. Fixes #57 Co-Authored-By: Claude Opus 5.5 --- pyrolab/drivers/arduino/arduino.py | 21 ++++++++++++++++++--- tests/drivers/test_arduino.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/pyrolab/drivers/arduino/arduino.py b/pyrolab/drivers/arduino/arduino.py index bbff6cb..93024a5 100644 --- a/pyrolab/drivers/arduino/arduino.py +++ b/pyrolab/drivers/arduino/arduino.py @@ -26,6 +26,7 @@ import inspect import logging +import time import pyfirmata.pyfirmata from pyfirmata import ( @@ -181,7 +182,7 @@ def digital_read(self, pin: int) -> int: self.board.digital[pin].mode = INPUT return self.board.digital[pin].read() - def analog_read(self, pin: int) -> float: + def analog_read(self, pin: int, timeout: float = 1.0) -> float: """ Tell the arduino to read a value from an analog pin. @@ -189,20 +190,34 @@ def analog_read(self, pin: int) -> float: ---------- pin : int Integer that represents the analog in pin number on an arduino + timeout : float, optional + How long to wait for the pin to report a value, in seconds + (default 1). Returns ------- float The value read by the analog pin (0 - 1.0) + + Raises + ------ + TimeoutError + If the pin doesn't report a value within ``timeout`` (for + example, because the board was reset or disconnected). """ # self.board.iterate() self.board.analog[pin].mode = INPUT self.board.analog[pin].enable_reporting() + deadline = time.monotonic() + timeout while True: value = self.board.analog[pin].read() if value is not None: - break - return value + return value + if time.monotonic() > deadline: + raise TimeoutError( + f"Analog pin {pin} did not report a value within {timeout} seconds" + ) + time.sleep(0.001) def close(self) -> None: """ diff --git a/tests/drivers/test_arduino.py b/tests/drivers/test_arduino.py index 27f2a10..abade43 100644 --- a/tests/drivers/test_arduino.py +++ b/tests/drivers/test_arduino.py @@ -31,3 +31,33 @@ def test_compat_shim_delegates_everything_else(): assert shim.signature is inspect.signature assert shim.getargspec(lambda x, y: None).args == ["x", "y"] + + +class FakePin: + def __init__(self, values): + self.values = list(values) + self.mode = None + + def enable_reporting(self): + pass + + def read(self): + return self.values.pop(0) if self.values else None + + +def arduino_with_pin(values): + from pyrolab.drivers.arduino.arduino import BaseArduinoDriver + + arduino = BaseArduinoDriver() + arduino.board = SimpleNamespace(analog=[FakePin(values)], exit=lambda: None) + return arduino + + +def test_analog_read_waits_for_the_pin_to_report(): + assert arduino_with_pin([None, None, 0.5]).analog_read(0) == 0.5 + + +def test_analog_read_gives_up_on_a_silent_pin(): + # Regression: waited forever, hanging the daemon (#57) + with pytest.raises(TimeoutError, match="Analog pin 0"): + arduino_with_pin([]).analog_read(0, timeout=0.05) From 2881374135059964c5f42d4581e41259ff087b28 Mon Sep 17 00:00:00 2001 From: Sequoia Ploeg Date: Mon, 5 Oct 2026 11:25:17 -0600 Subject: [PATCH 4/5] RTO: keep the VISA ResourceManager for the connection's lifetime connect() dropped its ResourceManager as soon as it returned; it is now kept and closed by close(). The timeout docstring said the default was 1 ms; it is 1000 ms. Fixes #79 Co-Authored-By: Claude Opus 5.5 --- pyrolab/drivers/scopes/rohdeschwarz.py | 22 +++++++++---- tests/drivers/scopes/test_rohdeschwarz.py | 40 +++++++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 tests/drivers/scopes/test_rohdeschwarz.py diff --git a/pyrolab/drivers/scopes/rohdeschwarz.py b/pyrolab/drivers/scopes/rohdeschwarz.py index 14794e5..61dc0ad 100644 --- a/pyrolab/drivers/scopes/rohdeschwarz.py +++ b/pyrolab/drivers/scopes/rohdeschwarz.py @@ -109,15 +109,17 @@ def connect( The IP address of the instrument. hislip : bool, optional Whether to use the HiSLIP protocol or not (default ``False``). - timeout : int, optional - The device response timeout in milliseconds (default 1 ms). - Pass ``None`` for infinite timeout. + timeout : float, optional + The device response timeout in milliseconds (default 1000, one + second). Pass ``None`` for infinite timeout. """ - rm = visa.ResourceManager() + # pyvisa expects the ResourceManager to outlive the resources it + # opens, so it is kept until close(). + self._rm = visa.ResourceManager() if hislip: - self.device = rm.open_resource(f"TCPIP::{address}::hislip0") + self.device = self._rm.open_resource(f"TCPIP::{address}::hislip0") else: - self.device = rm.open_resource(f"TCPIP::{address}") + self.device = self._rm.open_resource(f"TCPIP::{address}") self.device.timeout = timeout self.write_termination = "" self.device.ext_clear_status() @@ -129,7 +131,13 @@ def connect( return True def close(self): - self.device.close() + """ + Close the connection to the scope and release its VISA session. + """ + try: + self.device.close() + finally: + self._rm.close() @property def timeout(self): diff --git a/tests/drivers/scopes/test_rohdeschwarz.py b/tests/drivers/scopes/test_rohdeschwarz.py new file mode 100644 index 0000000..9d62687 --- /dev/null +++ b/tests/drivers/scopes/test_rohdeschwarz.py @@ -0,0 +1,40 @@ +""" +R&S RTO oscilloscope driver tests, with a fake VISA resource manager. +""" + +from unittest.mock import MagicMock + +import pytest + +pytest.importorskip("pyvisa") + +from pyrolab.drivers.scopes import rohdeschwarz # noqa: E402 + + +@pytest.fixture +def scope(monkeypatch): + rm = MagicMock(name="ResourceManager") + monkeypatch.setattr(rohdeschwarz.visa, "ResourceManager", lambda: rm) + scope = rohdeschwarz.RTO() + scope.connect(address="10.0.0.2") + return scope, rm + + +def test_the_resource_manager_lives_as_long_as_the_connection(scope): + # Regression: the ResourceManager was a local, dropped as soon as connect + # returned (#79) + scope, rm = scope + assert scope._rm is rm + assert scope.device is rm.open_resource.return_value + + scope.close() + scope.device.close.assert_called_once() + rm.close.assert_called_once() + + +def test_closing_releases_the_resource_manager_even_if_the_device_fails(scope): + scope, rm = scope + scope.device.close.side_effect = OSError("connection lost") + with pytest.raises(OSError): + scope.close() + rm.close.assert_called_once() From 5cf5f71c188876163b647d296c79201fd03ad222 Mon Sep 17 00:00:00 2001 From: Sequoia Ploeg Date: Mon, 5 Oct 2026 11:25:17 -0600 Subject: [PATCH 5/5] Fix the motion examples' instrument setup They passed serial numbers to the constructor; instruments are constructed without arguments and connected with connect(). The examples now close the stage in a finally block, drop the Kinesis DLL path workaround (the thorlabs_kinesis package adds that directory itself), and correct comments that disagreed with the drivers. Fixes #41 Co-Authored-By: Claude Opus 5.5 --- examples/903.motion/kcube_jogging.py | 71 ++++++++++---------- examples/903.motion/kcube_limits.py | 96 ++++++++++++++------------- examples/903.motion/kcube_rotation.py | 46 ++++++------- examples/903.motion/kcubes.py | 34 +++++----- examples/903.motion/kcubes_linear.py | 46 ++++++------- examples/903.motion/nanomax300.py | 66 ++++++++---------- 6 files changed, 174 insertions(+), 185 deletions(-) diff --git a/examples/903.motion/kcube_jogging.py b/examples/903.motion/kcube_jogging.py index 7337873..c95193a 100644 --- a/examples/903.motion/kcube_jogging.py +++ b/examples/903.motion/kcube_jogging.py @@ -1,40 +1,39 @@ -# Example of how to use the linear stage (Z825B) with software limits locally - -# import the Kinesis Library and make sure the path is correct -import os - -os.add_dll_directory("C:\\Program Files\\Thorlabs\\Kinesis") +# Example of how to jog the linear stage (Z825B) locally +# thorlabs_kinesis finds the Kinesis DLLs in C:\Program Files\Thorlabs\Kinesis +# on its own; set THORLABS_DLL_PATHS if they are installed somewhere else. from pyrolab.drivers.motion.z8xx import Z825B # Look at the connected KCube for correct serial num -linear = Z825B("27504851", home=True) - -# Set and read a few of the parameters pertaining to jogging -print(f"Jog mode: {linear.jog_mode}") -print(f"Stop mode: {linear.stop_mode}") -print(f"Jog step size: {linear.jog_step_size}") -linear.jog_step_size = 1 -print(f"Jog step size: {linear.jog_step_size}") -linear.max_pos = 15 -print(f"jog velocity: {linear.jog_velocity}") -print(f"jog acceleration: {linear.jog_acceleration}") -print(f"Soft Limit mode: {linear.soft_limits_mode}") - -# Jog the position twice and measure position after each step -linear.jog("forward") -print(f"current pos after 1 jog: {linear.get_position()} mm") -linear.jog("forward") -print(f"current pos after 2 jogs: {linear.get_position()} mm") - -# change to continuous jog then measure the position after reaching 10 mm -linear.jog_mode = "continuous" -linear.jog_velocity = 1 -linear.jog("forward") -while linear.get_position() < 10.0: - pass -linear.stop() -print(f"current pos after continuous jog: {linear.get_position()} mm") -linear.move_to(0) - -linear.close() +linear = Z825B() +linear.connect(serialno="27504851", home=True) + +try: + # Set and read a few of the parameters pertaining to jogging + print(f"Jog mode: {linear.jog_mode}") + print(f"Stop mode: {linear.stop_mode}") + print(f"Jog step size: {linear.jog_step_size}") + linear.jog_step_size = 1 + print(f"Jog step size: {linear.jog_step_size}") + linear.max_pos = 15 + print(f"jog velocity: {linear.jog_velocity}") + print(f"jog acceleration: {linear.jog_acceleration}") + print(f"Soft Limit mode: {linear.soft_limits_mode}") + + # Jog the position twice and measure position after each step + linear.jog("forward") + print(f"current pos after 1 jog: {linear.get_position()} mm") + linear.jog("forward") + print(f"current pos after 2 jogs: {linear.get_position()} mm") + + # change to continuous jog then measure the position after reaching 10 mm + linear.jog_mode = "continuous" + linear.jog_velocity = 1 + linear.jog("forward") + while linear.get_position() < 10.0: + pass + linear.stop() + print(f"current pos after continuous jog: {linear.get_position()} mm") + linear.move_to(0) +finally: + linear.close() diff --git a/examples/903.motion/kcube_limits.py b/examples/903.motion/kcube_limits.py index 9e4a1f4..7bdf4be 100644 --- a/examples/903.motion/kcube_limits.py +++ b/examples/903.motion/kcube_limits.py @@ -1,53 +1,55 @@ # Example of how to use the linear stage (Z825B) with software limits locally -# import the Kinesis Library and make sure the path is correct -import os - -os.add_dll_directory("C:\\Program Files\\Thorlabs\\Kinesis") - +# thorlabs_kinesis finds the Kinesis DLLs in C:\Program Files\Thorlabs\Kinesis +# on its own; set THORLABS_DLL_PATHS if they are installed somewhere else. from pyrolab.drivers.motion.z8xx import Z825B # Look at the connected KCube for correct serial num -linear = Z825B("27504851", home=True) - -# The stage defaults to the "disallow" limit mode -# and defaults to software limits of 0 and 25 mm - -# read and set all the different types of software limits modes -print(linear.soft_limits_mode) -linear.soft_limits_mode = "partial" -print(linear.soft_limits_mode) -linear.soft_limits_mode = "all" -print(linear.soft_limits_mode) -linear.soft_limits_mode = "disallow" -print(linear.soft_limits_mode) - -# read and set the max position -print(f"max pos: {linear.max_pos} mm") -linear.max_pos = 10 -print(f"max pos: {linear.max_pos} mm") - -# We will now test the limit mode "disallow" -# it should error if we try to move it outside the limits set -print(f"Before Move: {linear.get_position()} mm") -linear.move_to(5) # Within the limits so no error -print(f"Moved to 5: {linear.get_position()} mm") +linear = Z825B() +linear.connect(serialno="27504851", home=True) + +# connect() puts the stage in the "partial" limit mode, and the limits are +# whatever is saved on the device (0 and 25 mm for the Z825B) + try: - linear.move_to(15) # outside the limits so we must catch the error -except RuntimeError: - pass -print(f"Should have dissalowed move: {linear.get_position()} mm") - -# We will now test the limit mode "partial" -linear.soft_limits_mode = "partial" -linear.move_to(15) # It will move but only until it has reached it's limit -print(f"Should have truncated move to {linear.max_pos}: {linear.get_position()} mm") - -# We will now test the limit mode "all" -linear.soft_limits_mode = "all" -linear.move_to(15) # It will simply ignore any software limit set -print(f"Should have ignored max pos at {linear.max_pos}: {linear.get_position()} mm") -linear.move_to(0) - -# min and max pos will reset to 0 and 25 mm and soft_limits_mode resets to "disallow" -linear.close() + # read and set all the different types of software limits modes + print(linear.soft_limits_mode) + linear.soft_limits_mode = "partial" + print(linear.soft_limits_mode) + linear.soft_limits_mode = "all" + print(linear.soft_limits_mode) + linear.soft_limits_mode = "disallow" + print(linear.soft_limits_mode) + + # read and set the max position + print(f"max pos: {linear.max_pos} mm") + linear.max_pos = 10 + print(f"max pos: {linear.max_pos} mm") + + # We will now test the limit mode "disallow" + # it should error if we try to move it outside the limits set + print(f"Before Move: {linear.get_position()} mm") + linear.move_to(5) # Within the limits so no error + print(f"Moved to 5: {linear.get_position()} mm") + try: + linear.move_to(15) # outside the limits so we must catch the error + except RuntimeError: + pass + print(f"Should have dissalowed move: {linear.get_position()} mm") + + # We will now test the limit mode "partial" + linear.soft_limits_mode = "partial" + linear.move_to(15) # It will move but only until it has reached it's limit + print(f"Should have truncated move to {linear.max_pos}: {linear.get_position()} mm") + + # We will now test the limit mode "all" + linear.soft_limits_mode = "all" + linear.move_to(15) # It will simply ignore any software limit set + print( + f"Should have ignored max pos at {linear.max_pos}: {linear.get_position()} mm" + ) + linear.move_to(0) +finally: + # the next connect() reloads the limits saved on the device and goes back + # to the "partial" limit mode + linear.close() diff --git a/examples/903.motion/kcube_rotation.py b/examples/903.motion/kcube_rotation.py index d268e59..80d9265 100644 --- a/examples/903.motion/kcube_rotation.py +++ b/examples/903.motion/kcube_rotation.py @@ -1,30 +1,30 @@ # Example of how to use the rotation stage (PRM1Z8) locally -# import the Kinesis Library and make sure the path is correct -import os - -os.add_dll_directory("C:\\Program Files\\Thorlabs\\Kinesis") - +# thorlabs_kinesis finds the Kinesis DLLs in C:\Program Files\Thorlabs\Kinesis +# on its own; set THORLABS_DLL_PATHS if they are installed somewhere else. from pyrolab.drivers.motion.prm1z8 import PRM1Z8 # Look at the connected KCube for correct serial num -rot = PRM1Z8("27003366", home=False) +rot = PRM1Z8() +rot.connect(serialno="27003366", home=False) -# Prints out the starting postition to get an idea of where it currently is -starting_pos = rot.get_position() -print(f"Stating position: {starting_pos} degrees") +try: + # Prints out the starting postition to get an idea of where it currently is + starting_pos = rot.get_position() + print(f"Stating position: {starting_pos} degrees") -# A simple script to test moving the stage -while True: - # Input a position in degrees - move_pos = float(input("Rotation Position (deg):")) - # Input a 0 to go back to the starting position and exit the program - if move_pos == 0: - break - pos = rot.get_position() - print(f"Before Move: {pos} degrees") - rot.move_to(move_pos) - pos = rot.get_position() - print(f"After Move: {pos} degrees") -rot.move_to(starting_pos) -rot.close() + # A simple script to test moving the stage + while True: + # Input a position in degrees + move_pos = float(input("Rotation Position (deg):")) + # Input a 0 to go back to the starting position and exit the program + if move_pos == 0: + break + pos = rot.get_position() + print(f"Before Move: {pos} degrees") + rot.move_to(move_pos) + pos = rot.get_position() + print(f"After Move: {pos} degrees") + rot.move_to(starting_pos) +finally: + rot.close() diff --git a/examples/903.motion/kcubes.py b/examples/903.motion/kcubes.py index 980bfef..daf25f1 100644 --- a/examples/903.motion/kcubes.py +++ b/examples/903.motion/kcubes.py @@ -1,21 +1,17 @@ -import os - -os.add_dll_directory("C:\\Program Files\\Thorlabs\\Kinesis") - -import time -from ctypes import byref, c_double, c_int, pointer - from pyrolab.drivers.motion.z8xx import Z825B -linear = Z825B("27003497", home=False) -while True: - move_pos = int(input("Translation Position:")) - if move_pos == 0: - break - pos = linear.get_position() - print(f"Before Move: {pos}") - linear.move_to(move_pos) - pos = linear.get_position() - print(f"After Move: {pos}") -linear.move_to(0) -linear.close() +linear = Z825B() +linear.connect(serialno="27003497", home=False) +try: + while True: + move_pos = int(input("Translation Position:")) + if move_pos == 0: + break + pos = linear.get_position() + print(f"Before Move: {pos}") + linear.move_to(move_pos) + pos = linear.get_position() + print(f"After Move: {pos}") + linear.move_to(0) +finally: + linear.close() diff --git a/examples/903.motion/kcubes_linear.py b/examples/903.motion/kcubes_linear.py index 0f95822..9239725 100644 --- a/examples/903.motion/kcubes_linear.py +++ b/examples/903.motion/kcubes_linear.py @@ -1,30 +1,30 @@ # Example of how to use the linear stage (Z825B) locally -# import the Kinesis Library and make sure the path is correct -import os - -os.add_dll_directory("C:\\Program Files\\Thorlabs\\Kinesis") - +# thorlabs_kinesis finds the Kinesis DLLs in C:\Program Files\Thorlabs\Kinesis +# on its own; set THORLABS_DLL_PATHS if they are installed somewhere else. from pyrolab.drivers.motion.z8xx import Z825B # Look at the connected KCube for correct serial num -linear = Z825B("27504851", home=False) +linear = Z825B() +linear.connect(serialno="27504851", home=False) -# Prints out the starting postition to get an idea of where it currently is -starting_pos = linear.get_position() -print(f"Stating position: {starting_pos} mm") +try: + # Prints out the starting postition to get an idea of where it currently is + starting_pos = linear.get_position() + print(f"Stating position: {starting_pos} mm") -# A simple script to test moving the stage -while True: - # Input a position in degrees - move_pos = float(input("linear Position (mm):")) - # Input a 0 to go back to the starting position and exit the program - if move_pos == 0: - break - pos = linear.get_position() - print(f"Before Move: {pos} mm") - linear.move_to(move_pos) - pos = linear.get_position() - print(f"After Move: {pos} mm") -linear.move_to(starting_pos) -linear.close() + # A simple script to test moving the stage + while True: + # Input a position in mm + move_pos = float(input("linear Position (mm):")) + # Input a 0 to go back to the starting position and exit the program + if move_pos == 0: + break + pos = linear.get_position() + print(f"Before Move: {pos} mm") + linear.move_to(move_pos) + pos = linear.get_position() + print(f"After Move: {pos} mm") + linear.move_to(starting_pos) +finally: + linear.close() diff --git a/examples/903.motion/nanomax300.py b/examples/903.motion/nanomax300.py index 7218dec..20b632d 100644 --- a/examples/903.motion/nanomax300.py +++ b/examples/903.motion/nanomax300.py @@ -1,6 +1,3 @@ -import os - -os.add_dll_directory("C:\\Program Files\\Thorlabs\\Kinesis") import time from pyrolab.drivers.motion.max31x import MAX31X @@ -8,38 +5,33 @@ SER_NUM = 71874833 print(f"connecting to {SER_NUM}...") -nm = MAX31X(str(SER_NUM), closed_loop=True) +nm = MAX31X() +nm.connect(serialno=str(SER_NUM), closed_loop=True) print("connected") -print("zeroing all channels...") -nm.zero() -print("zeroed") -( - x, - y, - z, -) = nm.get_position() -print(f"position of x: {x}") -print(f"position of y: {y}") -print(f"position of z: {z}") -print("Input Jogging Parameters:") -while True: - channel = int(input("Input channel:")) - distance = float(input("Input jog distance:")) - print(f"jogging channel {channel} a distance of {distance}...") - nm.jog(channel, distance) - time.sleep(1) - print("jog complete") - ( - x, - y, - z, - ) = nm.get_position() - print(f"new position of x: {x}") - print(f"new position of y: {y}") - print(f"new position of z: {z}") - cond = input("Break?") - if (cond == "y") | (cond == "yes"): - break -print("closing nanomax...") -nm.close() -print("closed") +try: + print("zeroing all channels...") + nm.zero() + print("zeroed") + x, y, z = nm.get_position() + print(f"position of x: {x}") + print(f"position of y: {y}") + print(f"position of z: {z}") + print("Input Jogging Parameters:") + while True: + channel = int(input("Input channel:")) + distance = float(input("Input jog distance:")) + print(f"jogging channel {channel} a distance of {distance}...") + nm.jog(channel, distance) + time.sleep(1) + print("jog complete") + x, y, z = nm.get_position() + print(f"new position of x: {x}") + print(f"new position of y: {y}") + print(f"new position of z: {z}") + cond = input("Break?") + if cond in ("y", "yes"): + break +finally: + print("closing nanomax...") + nm.close() + print("closed")