Merge pull request #535 from BCDA-APS/530-target

remove target_attr, keep target signal

@gfabbris Thanks!

apstools/_devices/positioner_soft_done.py

@@ -8,6 +8,7 @@
 from ophyd import Signal
 from ophyd import EpicsSignal
 from ophyd import EpicsSignalRO
+from ophyd.signal import EpicsSignalBase
 import logging
 
 
@@ -37,10 +38,11 @@ class PVPositionerSoftDone(PVPositioner):
 
         Defaults to ``10^(-1*precision)``,
         where ``precision = setpoint.precision``.
-    target_attr : str, optional
-        Used if the setpoint is controlled incrementally by EPICS
-        (like with a ramp). Then a target attribute signal must be
-        defined, and its name passed in this variable.
+    update_target : bool
+        ``True`` when this object update the ``target`` Component directly.
+        Use ``False`` if the ``target`` Component will be updated externally,
+        such as by the controller when ``target`` is an ``EpicsSignal``.
+        Defaults to ``True``.
     kwargs :
         Passed to `ophyd.PVPositioner`
 
@@ -59,8 +61,19 @@ class PVPositionerSoftDone(PVPositioner):
         The stop PV to set when motion should be stopped
     stop_value : any, optional
         The value sent to stop_signal when a stop is requested
+    target : Signal
+        The target value of a move request.
 
-    (new in apstools 1.5.2)
+        Override (in subclass) with `EpicsSignal` to connect with a PV.
+
+        In some controllers (such as temperature controllers), the setpoint
+        may be changed incrementally towards this target value (such as a
+        ramp or controlled trajectory).  In such cases, the ``target`` will
+        be final value while ``setpoint`` will be the current desired position.
+
+        Otherwise, both ``setpoint`` and ``target`` will be set to the same value.
+
+    (new in apstools 1.5.3)
     """
 
     # positioner
@@ -76,15 +89,21 @@ class PVPositionerSoftDone(PVPositioner):
     tolerance = Component(Signal, value=-1, kind="config")
     report_dmov_changes = Component(Signal, value=False, kind="omitted")
 
+    target = Component(Signal, value=None, kind="config")
+
     @property
     def precision(self):
         return self.setpoint.precision
 
     def cb_readback(self, *args, **kwargs):
         """
         Called when readback changes (EPICS CA monitor event).
+
+        Computes if the positioner is done moving::
+
+            done = |readback - setpoint| <= tolerance
         """
-        diff = self.readback.get() - self._target.get()
+        diff = self.readback.get() - self.setpoint.get()
         _tolerance = (
             self.tolerance.get()
             if self.tolerance.get() >= 0
@@ -98,10 +117,12 @@ def cb_readback(self, *args, **kwargs):
     def cb_setpoint(self, *args, **kwargs):
         """
         Called when setpoint changes (EPICS CA monitor event).
-        When the setpoint is changed, force done=False.  For any move,
-        done must go != done_value, then back to done_value (True).
+
+        When the setpoint is changed, force done=False.  For any move, done
+        **must** transition to ``!= done_value``, then back to ``done_value``.
+
         Without this response, a small move (within tolerance) will not return.
-        Next update of readback will compute self.done.
+        Next update of readback will compute ``self.done``.
         """
         self.done.put(not self.done_value)
 
@@ -112,7 +133,7 @@ def __init__(
         readback_pv="",
         setpoint_pv="",
         tolerance=None,
-        target_attr="setpoint",
+        update_target=True,
         **kwargs,
     ):
 
@@ -121,11 +142,10 @@ def __init__(
 
         super().__init__(prefix=prefix, **kwargs)
 
-        self._target = getattr(self, target_attr)
-
         # Make the default alias for the readback the name of the
         # positioner itself as in EpicsMotor.
         self.readback.name = self.name
+        self.update_target = update_target
 
         self.readback.subscribe(self.cb_readback)
         self.setpoint.subscribe(self.cb_setpoint)
@@ -135,9 +155,12 @@ def __init__(
     def _setup_move(self, position):
         """Move and do not wait until motion is complete (asynchronous)"""
         self.log.debug("%s.setpoint = %s", self.name, position)
-        self._target.put(position, wait=True)
-        if self._target != self.setpoint:
-            self.setpoint.put(position, wait=True)
+        if self.update_target:
+            kwargs = {}
+            if issubclass(self.target.__class__, EpicsSignalBase):
+                kwargs["wait"] = True  # Signal.put() warns if kwargs are given
+            self.target.put(position, **kwargs)
+        self.setpoint.put(position, wait=True)
         if self.actuate is not None:
             self.log.debug("%s.actuate = %s", self.name, self.actuate_value)
             self.actuate.put(self.actuate_value, wait=False)