From 1a9a0beb86e8eb51bdb7260d7df9093f77c54462 Mon Sep 17 00:00:00 2001 From: appel_c Date: Thu, 15 Jan 2026 17:57:51 +0100 Subject: [PATCH 1/2] fix(controller): Ensure wait_for_connection calls controller.on() --- csaxs_bec/devices/npoint/npoint.py | 3 ++ csaxs_bec/devices/omny/galil/fgalil_ophyd.py | 3 +- csaxs_bec/devices/omny/galil/fupr_ophyd.py | 3 +- csaxs_bec/devices/omny/galil/galil_rio.py | 35 ++++++++++++++++ csaxs_bec/devices/omny/galil/lgalil_ophyd.py | 3 +- csaxs_bec/devices/omny/galil/ogalil_ophyd.py | 3 +- csaxs_bec/devices/omny/galil/sgalil_ophyd.py | 3 +- csaxs_bec/devices/omny/rt/rt_flomni_ophyd.py | 3 +- csaxs_bec/devices/omny/rt/rt_lamni_ophyd.py | 3 +- csaxs_bec/devices/omny/rt/rt_omny_ophyd.py | 3 +- csaxs_bec/devices/smaract/smaract_ophyd.py | 3 ++ tests/tests_devices/test_galil.py | 42 ++++++++++++++++++++ 12 files changed, 91 insertions(+), 16 deletions(-) create mode 100644 csaxs_bec/devices/omny/galil/galil_rio.py diff --git a/csaxs_bec/devices/npoint/npoint.py b/csaxs_bec/devices/npoint/npoint.py index 6148aa7..c99b185 100644 --- a/csaxs_bec/devices/npoint/npoint.py +++ b/csaxs_bec/devices/npoint/npoint.py @@ -442,6 +442,9 @@ class NPointAxis(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: + self.controller.on(timeout=timeout) + @property def limits(self): return (self.low_limit_travel.get(), self.high_limit_travel.get()) diff --git a/csaxs_bec/devices/omny/galil/fgalil_ophyd.py b/csaxs_bec/devices/omny/galil/fgalil_ophyd.py index 7a3efac..37640fb 100644 --- a/csaxs_bec/devices/omny/galil/fgalil_ophyd.py +++ b/csaxs_bec/devices/omny/galil/fgalil_ophyd.py @@ -212,8 +212,7 @@ class FlomniGalilMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/omny/galil/fupr_ophyd.py b/csaxs_bec/devices/omny/galil/fupr_ophyd.py index 8c4ac17..7ca1439 100644 --- a/csaxs_bec/devices/omny/galil/fupr_ophyd.py +++ b/csaxs_bec/devices/omny/galil/fupr_ophyd.py @@ -185,8 +185,7 @@ class FuprGalilMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/omny/galil/galil_rio.py b/csaxs_bec/devices/omny/galil/galil_rio.py new file mode 100644 index 0000000..17a88f9 --- /dev/null +++ b/csaxs_bec/devices/omny/galil/galil_rio.py @@ -0,0 +1,35 @@ +from ophyd_devices.utils.controller import Controller, threadlocked +from ophyd_devices.utils.socket import SocketSignal + +from csaxs_bec.devices.omny.galil.galil_ophyd import GalilCommunicationError, retry_once + + +class GalilRIO(Controller): + + @threadlocked + def socket_put(self, val: str) -> None: + self.sock.put(f"{val}\r".encode()) + + @retry_once + def socket_put_confirmed(self, val: str) -> None: + """Send message to controller and ensure that it is received by checking that the socket receives a colon. + + Args: + val (str): Message that should be sent to the socket + + Raises: + GalilCommunicationError: Raised if the return value is not a colon. + + """ + return_val = self.socket_put_and_receive(val) + if return_val != ":": + raise GalilCommunicationError( + f"Expected return value of ':' but instead received {return_val}" + ) + + +class GalilRIOSignalBase(SocketSignal): + def __init__(self, signal_name, **kwargs): + self.signal_name = signal_name + super().__init__(**kwargs) + self.rio_controller = self.parent.rio_controller diff --git a/csaxs_bec/devices/omny/galil/lgalil_ophyd.py b/csaxs_bec/devices/omny/galil/lgalil_ophyd.py index 4bbd0a9..2f2d304 100644 --- a/csaxs_bec/devices/omny/galil/lgalil_ophyd.py +++ b/csaxs_bec/devices/omny/galil/lgalil_ophyd.py @@ -170,8 +170,7 @@ class LamniGalilMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/omny/galil/ogalil_ophyd.py b/csaxs_bec/devices/omny/galil/ogalil_ophyd.py index 9d3b091..90f45fe 100644 --- a/csaxs_bec/devices/omny/galil/ogalil_ophyd.py +++ b/csaxs_bec/devices/omny/galil/ogalil_ophyd.py @@ -324,8 +324,7 @@ class OMNYGalilMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/omny/galil/sgalil_ophyd.py b/csaxs_bec/devices/omny/galil/sgalil_ophyd.py index 0dfd352..d0e838b 100644 --- a/csaxs_bec/devices/omny/galil/sgalil_ophyd.py +++ b/csaxs_bec/devices/omny/galil/sgalil_ophyd.py @@ -530,8 +530,7 @@ class SGalilMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/omny/rt/rt_flomni_ophyd.py b/csaxs_bec/devices/omny/rt/rt_flomni_ophyd.py index a64a468..044884f 100644 --- a/csaxs_bec/devices/omny/rt/rt_flomni_ophyd.py +++ b/csaxs_bec/devices/omny/rt/rt_flomni_ophyd.py @@ -678,8 +678,7 @@ class RtFlomniMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/omny/rt/rt_lamni_ophyd.py b/csaxs_bec/devices/omny/rt/rt_lamni_ophyd.py index 2f6c693..60e8730 100644 --- a/csaxs_bec/devices/omny/rt/rt_lamni_ophyd.py +++ b/csaxs_bec/devices/omny/rt/rt_lamni_ophyd.py @@ -588,8 +588,7 @@ class RtLamniMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/omny/rt/rt_omny_ophyd.py b/csaxs_bec/devices/omny/rt/rt_omny_ophyd.py index fd10993..a1ee947 100644 --- a/csaxs_bec/devices/omny/rt/rt_omny_ophyd.py +++ b/csaxs_bec/devices/omny/rt/rt_omny_ophyd.py @@ -1119,8 +1119,7 @@ class RtOMNYMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) - def wait_for_connection(self, timeout: int = 30, **kwargs) -> None: - """Wait for the device to be connected.""" + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: self.controller.on(timeout=timeout) @property diff --git a/csaxs_bec/devices/smaract/smaract_ophyd.py b/csaxs_bec/devices/smaract/smaract_ophyd.py index 0e810c2..8944ddf 100644 --- a/csaxs_bec/devices/smaract/smaract_ophyd.py +++ b/csaxs_bec/devices/smaract/smaract_ophyd.py @@ -153,6 +153,9 @@ class SmaractMotor(Device, PositionerBase): self.low_limit_travel.put(limits[0]) self.high_limit_travel.put(limits[1]) + def wait_for_connection(self, all_signals=False, timeout: float = 30.0) -> bool: + self.controller.on(timeout=timeout) + @property def limits(self): return (self.low_limit_travel.get(), self.high_limit_travel.get()) diff --git a/tests/tests_devices/test_galil.py b/tests/tests_devices/test_galil.py index 6ae437b..919e8d2 100644 --- a/tests/tests_devices/test_galil.py +++ b/tests/tests_devices/test_galil.py @@ -2,9 +2,19 @@ import copy from unittest import mock import pytest +from bec_server.device_server.tests.utils import DMMock from ophyd_devices.tests.utils import SocketMock +from csaxs_bec.devices.npoint.npoint import NPointAxis +from csaxs_bec.devices.omny.galil.fgalil_ophyd import FlomniGalilMotor +from csaxs_bec.devices.omny.galil.fupr_ophyd import FuprGalilMotor from csaxs_bec.devices.omny.galil.lgalil_ophyd import LamniGalilController, LamniGalilMotor +from csaxs_bec.devices.omny.galil.ogalil_ophyd import OMNYGalilMotor +from csaxs_bec.devices.omny.galil.sgalil_ophyd import SGalilMotor +from csaxs_bec.devices.omny.rt.rt_flomni_ophyd import RtFlomniMotor +from csaxs_bec.devices.omny.rt.rt_lamni_ophyd import RtLamniMotor +from csaxs_bec.devices.omny.rt.rt_omny_ophyd import RtOMNYMotor +from csaxs_bec.devices.smaract.smaract_ophyd import SmaractMotor @pytest.fixture(scope="function") @@ -161,3 +171,35 @@ def test_find_reference(leyex, axis_nr, socket_put_messages, socket_get_messages except Exception as e: print(e) assert leyex.controller.sock.buffer_put == socket_put_messages + + +def test_wait_for_connection_called(): + """Test that wait_for_connection is called on all motors that have a socket controller.""" + dm = DMMock() + with ( + mock.patch("ophyd_devices.utils.controller.Controller.on") as mock_on, + mock.patch("ophyd_devices.utils.controller.Controller.set_axis") as mock_set_axis, + ): + motors = [ + FlomniGalilMotor, + FuprGalilMotor, + LamniGalilMotor, + OMNYGalilMotor, + SGalilMotor, + RtFlomniMotor, + RtLamniMotor, + RtOMNYMotor, + SmaractMotor, + NPointAxis, + ] + for motor_cls in motors: + motor = motor_cls( + "C", + name="test_motor", + host="mpc2680.psi.ch", + port=8081, + socket_cls=SocketMock, + device_manager=dm, + ) + motor.wait_for_connection(timeout=5.0) + assert mock_on.call_args_list[-1] == mock.call(timeout=5.0) -- 2.49.1 From 2cf2f4b4e4be6566a09a1a64a786b9a5e0e42ece Mon Sep 17 00:00:00 2001 From: appel_c Date: Fri, 16 Jan 2026 10:52:24 +0100 Subject: [PATCH 2/2] test(controller): Fix test for controller wait_for_connection --- tests/tests_devices/test_galil.py | 63 +++++++++++++++++-------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/tests/tests_devices/test_galil.py b/tests/tests_devices/test_galil.py index 919e8d2..dc0ed68 100644 --- a/tests/tests_devices/test_galil.py +++ b/tests/tests_devices/test_galil.py @@ -5,16 +5,16 @@ import pytest from bec_server.device_server.tests.utils import DMMock from ophyd_devices.tests.utils import SocketMock -from csaxs_bec.devices.npoint.npoint import NPointAxis -from csaxs_bec.devices.omny.galil.fgalil_ophyd import FlomniGalilMotor -from csaxs_bec.devices.omny.galil.fupr_ophyd import FuprGalilMotor +from csaxs_bec.devices.npoint.npoint import NPointAxis, NPointController +from csaxs_bec.devices.omny.galil.fgalil_ophyd import FlomniGalilController, FlomniGalilMotor +from csaxs_bec.devices.omny.galil.fupr_ophyd import FuprGalilController, FuprGalilMotor from csaxs_bec.devices.omny.galil.lgalil_ophyd import LamniGalilController, LamniGalilMotor -from csaxs_bec.devices.omny.galil.ogalil_ophyd import OMNYGalilMotor -from csaxs_bec.devices.omny.galil.sgalil_ophyd import SGalilMotor -from csaxs_bec.devices.omny.rt.rt_flomni_ophyd import RtFlomniMotor -from csaxs_bec.devices.omny.rt.rt_lamni_ophyd import RtLamniMotor -from csaxs_bec.devices.omny.rt.rt_omny_ophyd import RtOMNYMotor -from csaxs_bec.devices.smaract.smaract_ophyd import SmaractMotor +from csaxs_bec.devices.omny.galil.ogalil_ophyd import OMNYGalilController, OMNYGalilMotor +from csaxs_bec.devices.omny.galil.sgalil_ophyd import GalilController, SGalilMotor +from csaxs_bec.devices.omny.rt.rt_flomni_ophyd import RtFlomniController, RtFlomniMotor +from csaxs_bec.devices.omny.rt.rt_lamni_ophyd import RtLamniController, RtLamniMotor +from csaxs_bec.devices.omny.rt.rt_omny_ophyd import RtOMNYController, RtOMNYMotor +from csaxs_bec.devices.smaract.smaract_ophyd import SmaractController, SmaractMotor @pytest.fixture(scope="function") @@ -176,23 +176,25 @@ def test_find_reference(leyex, axis_nr, socket_put_messages, socket_get_messages def test_wait_for_connection_called(): """Test that wait_for_connection is called on all motors that have a socket controller.""" dm = DMMock() - with ( - mock.patch("ophyd_devices.utils.controller.Controller.on") as mock_on, - mock.patch("ophyd_devices.utils.controller.Controller.set_axis") as mock_set_axis, - ): - motors = [ - FlomniGalilMotor, - FuprGalilMotor, - LamniGalilMotor, - OMNYGalilMotor, - SGalilMotor, - RtFlomniMotor, - RtLamniMotor, - RtOMNYMotor, - SmaractMotor, - NPointAxis, - ] - for motor_cls in motors: + testable_connections = [ + (NPointAxis, NPointController), + (FlomniGalilMotor, FlomniGalilController), + (FuprGalilMotor, FuprGalilController), + (LamniGalilMotor, LamniGalilController), + (OMNYGalilMotor, OMNYGalilController), + (SGalilMotor, GalilController), + (RtFlomniMotor, RtFlomniController), + (RtLamniMotor, RtLamniController), + (RtOMNYMotor, RtOMNYController), + (SmaractMotor, SmaractController), + ] + for motor_cls, controller_cls in testable_connections: + # Store values to restore later + ctrl_axis_backup = controller_cls._axes_per_controller + try: + controller_cls._reset_controller() + controller_cls._axes_per_controller = 3 + motor = motor_cls( "C", name="test_motor", @@ -201,5 +203,10 @@ def test_wait_for_connection_called(): socket_cls=SocketMock, device_manager=dm, ) - motor.wait_for_connection(timeout=5.0) - assert mock_on.call_args_list[-1] == mock.call(timeout=5.0) + with mock.patch.object(motor.controller, "on") as mock_on: + + motor.wait_for_connection(timeout=5.0) + assert mock_on.call_args_list[-1] == mock.call(timeout=5.0) + finally: + controller_cls._reset_controller() + controller_cls._axes_per_controller = ctrl_axis_backup -- 2.49.1