Skip to content

Commit

Permalink
Merge branch 'main' into 990-listdevice-RuntimeError
Browse files Browse the repository at this point in the history
  • Loading branch information
prjemian committed Aug 28, 2024
2 parents 387b38c + f01cce5 commit 4b980a8
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 105 deletions.
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ Fixes
* lineup2() should work with low intensity peaks.
* lineup2() would raise ZeroDivideError in some cases.
* Increase minimum aps-dm-api version to 8.
* PVPositionerSoftDone should set 'done' to False at start of a move.
* Race condition with SR570 pre-amp.

Maintenance
Expand Down
2 changes: 1 addition & 1 deletion apstools/devices/eurotherm_2216e.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def __init__(self, prefix="", *, tolerance=1, **kwargs):
readback_pv="ignoreRBV",
setpoint_pv="ignore",
tolerance=tolerance,
update_target=False,
use_target=False,
**kwargs,
)
self.sensor.subscribe(self.cb_sensor)
Expand Down
4 changes: 2 additions & 2 deletions apstools/devices/lakeshore_controllers.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class LakeShore336_LoopControl(PVPositionerSoftDoneWithStop):

def __init__(self, *args, loop_number=None, timeout=10 * HOUR, **kwargs):
self.loop_number = loop_number
super().__init__(*args, timeout=timeout, tolerance=0.1, readback_pv=f"IN{loop_number}", **kwargs)
super().__init__(*args, timeout=timeout, tolerance=0.1, use_target=True, readback_pv=f"IN{loop_number}", **kwargs)
self._settle_time = 0

@property
Expand Down Expand Up @@ -171,7 +171,7 @@ class LS340_LoopBase(PVPositionerSoftDoneWithStop):

def __init__(self, *args, loop_number=None, timeout=10 * HOUR, **kwargs):
self.loop_number = loop_number
super().__init__(*args, readback_pv="ignore", timeout=timeout, tolerance=0.1, **kwargs)
super().__init__(*args, readback_pv="ignore", timeout=timeout, use_target=True, tolerance=0.1, **kwargs)
self._settle_time = 0

@property
Expand Down
50 changes: 18 additions & 32 deletions apstools/devices/positioner_soft_done.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@
from ophyd import FormattedComponent
from ophyd import PVPositioner
from ophyd import Signal
from ophyd.signal import EpicsSignalBase

# from ..tests import timed_pause

logger = logging.getLogger(__name__)

# Must use a data type that can be serialized as json (Python's None cannot be serialized)
# This ValueError: Cannot determine the appropriate bluesky-friendly data type for value
# None of Python type <class 'NoneType'>. Supported types include: int, float, str, and
# iterables such as list, tuple, np.ndarray, and so on.
TARGET_UNDEFINED = "undefined"


class PVPositionerSoftDone(PVPositioner):
"""
Expand All @@ -48,11 +53,11 @@ class PVPositionerSoftDone(PVPositioner):
Defaults to ``10^(-1*precision)``,
where ``precision = setpoint.precision``.
update_target : bool
use_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``.
Defaults to ``False``.
kwargs :
Passed to `ophyd.PVPositioner`
Expand Down Expand Up @@ -101,10 +106,7 @@ 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")

_rb_count = 0
_sp_count = 0
target = Component(Signal, value=TARGET_UNDEFINED, kind="config")

def __init__(
self,
Expand All @@ -113,7 +115,7 @@ def __init__(
readback_pv="",
setpoint_pv="",
tolerance=None,
update_target=True,
use_target=False,
**kwargs,
):
# fmt: off
Expand All @@ -132,10 +134,12 @@ def __init__(
# 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.use_target = use_target

self.readback.subscribe(self.cb_readback)
self.setpoint.subscribe(self.cb_setpoint)
self.setpoint.subscribe(self.cb_update_target, event_type="setpoint")

# cancel subscriptions before object is garbage collected
weakref.finalize(self.readback, self.readback.unsubscribe_all)
weakref.finalize(self.setpoint, self.setpoint.unsubscribe_all)
Expand All @@ -159,6 +163,9 @@ def actual_tolerance(self):
)
# fmt: on

def cb_update_target(self, value, *args, **kwargs):
self.target.put(value)

def cb_readback(self, *args, **kwargs):
"""
Called when readback changes (EPICS CA monitor event) or on-demand.
Expand All @@ -171,8 +178,6 @@ def cb_readback(self, *args, **kwargs):
if idle:
return

self._rb_count += 1

if self.inposition:
self.done.put(self.done_value)
if self.report_dmov_changes.get():
Expand All @@ -192,7 +197,6 @@ def cb_setpoint(self, *args, **kwargs):
and do not react to the "wrong" signature.
"""
if "value" in kwargs and "status" not in kwargs:
self._sp_count += 1
self.done.put(not self.done_value)
logger.debug("cb_setpoint: done=%s, setpoint=%s", self.done.get(), self.setpoint.get())

Expand All @@ -208,7 +212,7 @@ def inposition(self):
# Since this method must execute quickly, do NOT force
# EPICS CA gets using `use_monitor=False`.
rb = self.readback.get()
sp = self.setpoint.get()
sp = self.setpoint.get() if self.use_target is False else self.target.get()
tol = self.actual_tolerance
inpos = math.isclose(rb, sp, abs_tol=tol)
logger.debug("inposition: inpos=%s rb=%s sp=%s tol=%s", inpos, rb, sp, tol)
Expand All @@ -221,18 +225,11 @@ def precision(self):
def _setup_move(self, position):
"""Move and do not wait until motion is complete (asynchronous)"""
self.log.debug("%s.setpoint = %s", self.name, position)
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)
self.done.put(False)
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)
# This is needed because in a special case the setpoint.put does not
# run the "sub_value" subscriptions.
self.cb_setpoint()
self.cb_readback() # This is needed to force the first check.


Expand All @@ -255,14 +252,3 @@ def stop(self, *, success=False):
self.setpoint.put(self.position)
time.sleep(2.0 / 60) # two clock ticks, allow for EPICS record processing
self.cb_readback() # re-evaluate soft done Signal


# -----------------------------------------------------------------------------
# :author: Pete R. Jemian
# :email: jemian@anl.gov
# :copyright: (c) 2017-2024, UChicago Argonne, LLC
#
# Distributed under the terms of the Argonne National Laboratory Open Source License.
#
# The full license is in the file LICENSE.txt, distributed with this software.
# -----------------------------------------------------------------------------
2 changes: 1 addition & 1 deletion apstools/devices/tests/test_eurotherm_2216e.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def test_device():
assert not euro.connected

assert euro.tolerance.get() == 1
assert euro.update_target is False
assert euro.use_target is False
assert euro.target is None

cns = """
Expand Down
9 changes: 5 additions & 4 deletions apstools/devices/tests/test_lakeshores.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from ...tests import IOC_GP
from ..lakeshore_controllers import LakeShore336Device
from ..lakeshore_controllers import LakeShore340Device
from ..positioner_soft_done import TARGET_UNDEFINED

PV_PREFIX = f"phony:{IOC_GP}lakeshore:"

Expand All @@ -16,8 +17,8 @@ def test_lakeshore_336():
assert not t336.connected

# Signal components
assert t336.loop1.target.get() == "None"
assert t336.loop2.target.get() == "None"
assert t336.loop1.target.get() == TARGET_UNDEFINED
assert t336.loop2.target.get() == TARGET_UNDEFINED

assert t336.loop1.tolerance.get() == 0.1
assert t336.loop2.tolerance.get() == 0.1
Expand Down Expand Up @@ -68,8 +69,8 @@ def test_lakeshore_340():
assert not t340.connected

# Signal components
assert t340.control.target.get() == "None"
assert t340.sample.target.get() == "None"
assert t340.control.target.get() == TARGET_UNDEFINED
assert t340.sample.target.get() == TARGET_UNDEFINED

assert t340.control.tolerance.get() == 0.1
assert t340.sample.tolerance.get() == 0.1
Expand Down
31 changes: 12 additions & 19 deletions apstools/devices/tests/test_positioner_soft_done.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from ...utils import run_in_thread
from ..positioner_soft_done import PVPositionerSoftDone
from ..positioner_soft_done import PVPositionerSoftDoneWithStop
from ..positioner_soft_done import TARGET_UNDEFINED

PV_PREFIX = f"{IOC_GP}gp:"
delay_active = False
Expand All @@ -35,7 +36,7 @@ def pos():
"""Test Positioner based on two analogout PVs."""
# fmt: off
pos = PVPositionerSoftDoneWithStop(
PV_PREFIX, readback_pv="float1", setpoint_pv="float2", name="pos"
PV_PREFIX, readback_pv="float1", setpoint_pv="float2", use_target=True, name="pos"
)
# fmt: on
pos.wait_for_connection()
Expand Down Expand Up @@ -93,9 +94,8 @@ def confirm_in_position(p, dt):
p.readback.get(use_monitor=False) # force a read from the IOC
p.cb_readback() # update self.done

# collect these values at one instant (as close as possible).
c_rb = p._rb_count # do not expect any new callbacks during this method
c_sp = p._sp_count
# Collect these values at one instant (as close in time as possible).
# Do not expect any new callbacks during this function.
dmov = p.done.get()
rb = p.readback.get()
sp = p.setpoint.get()
Expand All @@ -106,18 +106,13 @@ def confirm_in_position(p, dt):
f"{p.name=}"
f" {rb=:.5f} {sp=:.5f} {tol=}"
f" {dt=:.4f}s"
f" {p._sp_count=}"
f" {p._rb_count=}"
f" {p.done=}"
f" {p.done_value=}"
f" {time.time()=:.4f}"
)
# fmt: on

assert p._rb_count == c_rb, diagnostics
assert p._sp_count == c_sp, diagnostics
assert dmov == p.done_value, diagnostics
# assert math.isclose(rb, sp, abs_tol=tol), diagnostics


@run_in_thread
Expand Down Expand Up @@ -202,20 +197,17 @@ def test_structure(device, has_inposition):
assert pos.setpoint.pvname == "v"
assert pos.done.get() is True
assert pos.done_value is True
assert pos.target.get() == "None"
assert pos.target.get() == TARGET_UNDEFINED
assert pos.tolerance.get() == -1


@pytest.mark.local
def test_put_and_stop(rbv, prec, pos):
assert pos.tolerance.get() == -1
assert pos.precision == prec.get()

def motion(rb_initial, target, rb_mid=None):
rbv.put(rb_initial) # make the readback to different
c_sp = pos._sp_count
pos.setpoint.put(target)
assert pos._sp_count == c_sp + 1
assert math.isclose(pos.readback.get(use_monitor=False), rb_initial, abs_tol=0.02)
assert math.isclose(pos.setpoint.get(use_monitor=False), target, abs_tol=0.02)
assert pos.done.get() != pos.done_value
Expand Down Expand Up @@ -248,7 +240,6 @@ def motion(rb_initial, target, rb_mid=None):
motion(1, 0, 0.5) # interrupted move


@pytest.mark.local
def test_move_and_stop_nonzero(rbv, pos):
timed_pause()

Expand All @@ -271,9 +262,14 @@ def test_move_and_stop_nonzero(rbv, pos):
assert pos.inposition


@pytest.mark.local
def test_move_and_stopped_early(rbv, pos):
def motion(target, delay, interrupt=False):
"""
Test moving pos to target. Update rbv after delay.
If interrupt is True, stop the move before it is done
(at a time that is less than the 'delay' value).
"""
timed_pause(0.1) # allow previous activities to settle down

t0 = time.time()
Expand All @@ -286,8 +282,7 @@ def motion(target, delay, interrupt=False):
rb_new = pos.readback.get(use_monitor=False)
arrived = math.isclose(rb_new, target, abs_tol=pos.actual_tolerance)
# fmt: on
if interrupt:
assert not status.done
if interrupt and not status.done:
assert not status.success
assert not arrived, f"{dt=:.3f}"
pos.stop()
Expand Down Expand Up @@ -350,11 +345,9 @@ def test_position_sequence_calcpos(target, calcpos):
def motion(p, goal):
timed_pause(0.1) # allow previous activities to settle down

c_sp = p._sp_count
t0 = time.time()
status = p.move(goal)
dt = time.time() - t0
assert p._sp_count == c_sp + 1
assert status.elapsed > 0, str(status)
assert status.done, str(status)

Expand Down
Loading

0 comments on commit 4b980a8

Please sign in to comment.