From c56aab91769d8085db85469d3587b13236de86cf Mon Sep 17 00:00:00 2001
From: David Perl <david.perl@psi.ch>
Date: Thu, 3 Jul 2025 12:27:07 +0200
Subject: [PATCH] feat(#118): forward soft limits to soft signals

---
 .../base_classes/psi_positioner_base.py       | 14 ++++++--
 tests/test_psi_positioner.py                  | 36 ++++++++++++++++++-
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/ophyd_devices/interfaces/base_classes/psi_positioner_base.py b/ophyd_devices/interfaces/base_classes/psi_positioner_base.py
index 4f25a20..9be5501 100644
--- a/ophyd_devices/interfaces/base_classes/psi_positioner_base.py
+++ b/ophyd_devices/interfaces/base_classes/psi_positioner_base.py
@@ -5,7 +5,7 @@ from ophyd import Component as Cpt
 from ophyd import Device
 from ophyd.device import required_for_connection
 from ophyd.positioner import PositionerBase
-from ophyd.signal import EpicsSignalBase
+from ophyd.signal import EpicsSignalBase, Signal
 from ophyd.status import MoveStatus
 from ophyd.status import wait as status_wait
 from ophyd.utils.epics_pvs import AlarmSeverity, fmt_time
@@ -290,8 +290,8 @@ class PSIPositionerBase(PSISimplePositionerBase):
     motor_is_moving = _OPTIONAL_SIGNAL
     high_limit_switch = _OPTIONAL_SIGNAL
     low_limit_switch = _OPTIONAL_SIGNAL
-    high_limit_travel = _OPTIONAL_SIGNAL
-    low_limit_travel = _OPTIONAL_SIGNAL
+    high_limit_travel: Signal | _SignalSentinel = _OPTIONAL_SIGNAL
+    low_limit_travel: Signal | _SignalSentinel = _OPTIONAL_SIGNAL
     direction_of_travel = _OPTIONAL_SIGNAL
 
     # commands
@@ -323,3 +323,11 @@ class PSIPositionerBase(PSISimplePositionerBase):
             child_name_separator=child_name_separator,
             **kwargs,
         )
+        if self.limits is not None:
+            for sig, lim in (
+                (self.low_limit_travel, self.limits[0]),
+                (self.high_limit_travel, self.limits[1]),
+            ):
+                # If the limit signals are defined as soft signals, propagate the limits there
+                if sig is not _OPTIONAL_SIGNAL and type(sig) is Signal:
+                    sig.put(lim)
diff --git a/tests/test_psi_positioner.py b/tests/test_psi_positioner.py
index 625442c..03029b4 100644
--- a/tests/test_psi_positioner.py
+++ b/tests/test_psi_positioner.py
@@ -4,7 +4,7 @@ from unittest.mock import ANY, MagicMock, patch
 import ophyd
 import pytest
 from ophyd.device import Component as Cpt
-from ophyd.signal import EpicsSignal
+from ophyd.signal import EpicsSignal, Kind, Signal
 from ophyd.sim import FakeEpicsSignal, FakeEpicsSignalRO
 
 from ophyd_devices.devices.simple_positioner import PSISimplePositioner
@@ -33,6 +33,24 @@ def test_cannot_isntantiate_without_required_signals():
     assert dev.user_setpoint.get() == 0
 
 
+def test_override_suffixes():
+    pos = PSISimplePositioner(
+        name="name",
+        prefix="prefix:",
+        override_suffixes={"user_readback": "RDB", "motor_done_move": "DONE"},
+    )
+    assert pos.user_readback._read_pvname == "prefix:RDB"
+    assert pos.motor_done_move._read_pvname == "prefix:DONE"
+
+
+@patch("ophyd.ophydobj.LoggerAdapter")
+def test_override_suffixes_warns_on_nonimplemented(ophyd_logger):
+    _ = PSISimplePositioner(name="name", prefix="prefix:", override_suffixes={"motor_stop": "STOP"})
+    ophyd_logger().warning.assert_called_with(
+        "<class 'ophyd_devices.devices.simple_positioner.PSISimplePositioner'> does not implement overridden signal motor_stop"
+    )
+
+
 @pytest.fixture(scope="function")
 def mock_psi_positioner() -> PSISimplePositioner:
     name = "positioner"
@@ -83,3 +101,19 @@ def test_status_completed_when_req_done_sub_runs(mock_psi_positioner: PSISimpleP
     assert not st.done
     mock_psi_positioner._run_subs(sub_type=mock_psi_positioner._SUB_REQ_DONE)
     assert st.done
+
+
+def test_psi_positioner_soft_limits():
+    class PsiTestPosWSoftLimits(PSIPositionerBase):
+        user_setpoint: EpicsSignal = Cpt(FakeEpicsSignal, ".VAL", limits=True, auto_monitor=True)
+        user_readback = Cpt(FakeEpicsSignalRO, ".RBV", kind="hinted", auto_monitor=True)
+        motor_done_move = Cpt(FakeEpicsSignalRO, ".DMOV", auto_monitor=True)
+
+        low_limit_travel = Cpt(Signal, value=0, kind=Kind.omitted)
+        high_limit_travel = Cpt(Signal, value=0, kind=Kind.omitted)
+
+    device = PsiTestPosWSoftLimits(name="name", prefix="", limits=[-1.5, 1.5])
+    assert isinstance(device.low_limit_travel, Signal)
+    assert isinstance(device.high_limit_travel, Signal)
+    assert device.low_limit_travel.get() == -1.5
+    assert device.high_limit_travel.get() == 1.5