diff --git a/CHANGES.rst b/CHANGES.rst index 289cd61fd..f94de70d4 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -67,6 +67,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 diff --git a/apstools/devices/positioner_soft_done.py b/apstools/devices/positioner_soft_done.py index 7832d579b..e290b04fd 100644 --- a/apstools/devices/positioner_soft_done.py +++ b/apstools/devices/positioner_soft_done.py @@ -24,6 +24,42 @@ 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 . Supported types include: int, float, str, and +# iterables such as list, tuple, np.ndarray, and so on. +TARGET_UNDEFINED = "undefined" + + +class _EpicsPositionerSetpointSignal(EpicsSignal): + """ + Special handling when PVPositionerSoftDone setpoint is changed. + + 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. + The ``cb_readback()`` method will compute ``done``. + """ + + def put(self, value, *args, **kwargs): + """Make sure 'done' signal goes False when setpoint is changed by us.""" + super().put(value, *args, **kwargs) + + self.parent.done.put(not self.parent.done_value) + if self.parent.update_target: + kwargs = {} + if issubclass(self.parent.target.__class__, EpicsSignalBase): + kwargs["wait"] = True # Signal.put() warns if kwargs are given + self.parent.target.put(value, **kwargs) + + # def get(self, *args, **kwargs): + # value = super().get(*args, **kwargs) + # if self.parent.update_target: + # target = self.parent.target.get() + # if target != TARGET_UNDEFINED: + # value = target + # return value + class PVPositionerSoftDone(PVPositioner): """ @@ -49,7 +85,7 @@ class PVPositionerSoftDone(PVPositioner): Defaults to ``10^(-1*precision)``, where ``precision = setpoint.precision``. update_target : bool - ``True`` when this object update the ``target`` Component directly. + ``True`` when this object updates 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``. @@ -92,7 +128,7 @@ class PVPositionerSoftDone(PVPositioner): EpicsSignalRO, "{prefix}{_readback_pv}", kind="hinted", auto_monitor=True ) setpoint = FormattedComponent( - EpicsSignal, "{prefix}{_setpoint_pv}", kind="normal", put_complete=True + _EpicsPositionerSetpointSignal, "{prefix}{_setpoint_pv}", kind="normal", put_complete=True ) # fmt: on done = Component(Signal, value=True, kind="config") @@ -101,10 +137,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, @@ -171,8 +204,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(): @@ -182,18 +213,12 @@ 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** transition to ``!= done_value``, then back to ``done_value``. + This method is called when the setpoint is changed by this code or from + some other EPICS client. - Without this response, a small move (within tolerance) will not return. - The ``cb_readback()`` method will compute ``done``. - - Since other code will also call this method, check the keys in kwargs - and do not react to the "wrong" signature. + The 'done' signal is set to False in the custom + _EpicsPositionerSetpointSignal class. """ - 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()) @property @@ -221,19 +246,18 @@ 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) + + # Write the setpoint value. self.setpoint.put(position, wait=True) + # The 'done' and 'target' signals are handled by + # the custom '_EpicsPositionerSetpointSignal' class. + 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. + + # Force the first check for done. + self.cb_readback() class PVPositionerSoftDoneWithStop(PVPositionerSoftDone): diff --git a/apstools/devices/tests/test_lakeshores.py b/apstools/devices/tests/test_lakeshores.py index b7617a896..89c369511 100644 --- a/apstools/devices/tests/test_lakeshores.py +++ b/apstools/devices/tests/test_lakeshores.py @@ -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:" @@ -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 @@ -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 diff --git a/apstools/devices/tests/test_positioner_soft_done.py b/apstools/devices/tests/test_positioner_soft_done.py index b2c9908e0..376ea7a35 100644 --- a/apstools/devices/tests/test_positioner_soft_done.py +++ b/apstools/devices/tests/test_positioner_soft_done.py @@ -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 @@ -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() @@ -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 @@ -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 @@ -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() @@ -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() @@ -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() @@ -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) diff --git a/apstools/plans/tests/test_alignment.py b/apstools/plans/tests/test_alignment.py index f85e13d13..f32ab8d02 100644 --- a/apstools/plans/tests/test_alignment.py +++ b/apstools/plans/tests/test_alignment.py @@ -100,15 +100,15 @@ def test_SynPseudoVoigt_randomize(): assert signal.noise_multiplier == 1 -TestParameters = collections.namedtuple("TestParameters", "signal mover start finish npts feature") -parms_slower = TestParameters(noisy, m1, -1.2, 1.2, 11, "max") -parms_faster = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max") -parms_cen = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen") -parms_com = TestParameters(pvoigt, axis, -1.2, 1.2, 51, "com") +_TestParameters = collections.namedtuple("TestParameters", "signal mover start finish npts feature") +parms_slower = _TestParameters(noisy, m1, -1.2, 1.2, 11, "max") +parms_faster = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max") +parms_cen = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen") +parms_com = _TestParameters(pvoigt, axis, -1.2, 1.2, 51, "com") @pytest.mark.parametrize("parms", [parms_slower, parms_faster, parms_cen, parms_com]) -def test_direct_implementation_with_rel_scan(parms: TestParameters): +def test_direct_implementation_with_rel_scan(parms: _TestParameters): RE(bps.mv(parms.mover, 0)) assert get_position(parms.mover) == 0.0 @@ -132,17 +132,17 @@ def test_direct_implementation_with_rel_scan(parms: TestParameters): assert math.isclose(position, center, abs_tol=0.001) -TestParameters = collections.namedtuple("TestParameters", "signals mover start finish npts feature rescan") -parms_motor_slower = TestParameters(noisy, m1, -1.2, 1.2, 11, "max", True) -parms_ao_faster = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", True) -parms_ao_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", True) -parms_det_list_of_1 = TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", True) -parms_det_list_of_3 = TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", True) -parms_det_tuple = TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", True) -parms_cen = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", False) -parms_com = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", False) -parms_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", False) -parms_min___pathological = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", False) +_TestParameters = collections.namedtuple("TestParameters", "signals mover start finish npts feature rescan") +parms_motor_slower = _TestParameters(noisy, m1, -1.2, 1.2, 11, "max", True) +parms_ao_faster = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", True) +parms_ao_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", True) +parms_det_list_of_1 = _TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", True) +parms_det_list_of_3 = _TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", True) +parms_det_tuple = _TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", True) +parms_cen = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", False) +parms_com = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", False) +parms_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", False) +parms_min___pathological = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", False) @pytest.mark.parametrize( @@ -160,7 +160,7 @@ def test_direct_implementation_with_rel_scan(parms: TestParameters): parms_min___pathological, ], ) -def test_lineup(parms: TestParameters): +def test_lineup(parms: _TestParameters): if isinstance(parms.signals, SynPseudoVoigt): parms.signals.randomize_parameters(scale=250_000, bkg=0.000_000_000_1) else: @@ -197,17 +197,17 @@ def test_lineup(parms: TestParameters): # # assert lo <= position <= hi, f"{bec=} {bec.peaks=} {position=} {center=} {width=}" -TestParameters = collections.namedtuple("TestParameters", "signals, mover, start, finish, npts, feature, nscans") -parms_motor_slower = TestParameters(noisy, m1, -1.2, 1.2, 11, "max", 2) -parms_ao_faster = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 2) -parms_ao_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 2) -parms_det_list_of_1 = TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", 2) -parms_det_list_of_3 = TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", 2) -parms_det_tuple = TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", 2) -parms_cen = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 1) -parms_com = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", 1) -parms_max = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 1) -parms_min___pathological = TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", 1) +_TestParameters = collections.namedtuple("TestParameters", "signals, mover, start, finish, npts, feature, nscans") +parms_motor_slower = _TestParameters(noisy, m1, -1.2, 1.2, 11, "max", 2) +parms_ao_faster = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 2) +parms_ao_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 2) +parms_det_list_of_1 = _TestParameters([pvoigt], axis, -1.2, 1.2, 11, "max", 2) +parms_det_list_of_3 = _TestParameters([pvoigt, noisy, scaler1], axis, -1.2, 1.2, 11, "max", 2) +parms_det_tuple = _TestParameters((pvoigt), axis, -1.2, 1.2, 11, "max", 2) +parms_cen = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "cen", 1) +parms_com = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "com", 1) +parms_max = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "max", 1) +parms_min___pathological = _TestParameters(pvoigt, axis, -1.2, 1.2, 11, "min", 1) @pytest.mark.parametrize( @@ -225,7 +225,7 @@ def test_lineup(parms: TestParameters): parms_min___pathological, ], ) -def test_lineup2(parms: TestParameters): +def test_lineup2(parms: _TestParameters): if isinstance(parms.signals, SynPseudoVoigt): parms.signals.randomize_parameters(scale=250_000, bkg=0.000_000_000_1) else: @@ -338,23 +338,23 @@ class ImitatorForTesting(Device): assert results.report() is None -TestParameters = collections.namedtuple( +_TestParameters = collections.namedtuple( "TestParameters", "peak base noise center sigma xlo xhi npts nscans tol outcome" ) -parms_signal_but_high_background = TestParameters(1e5, 1e6, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) -parms_model_peak = TestParameters(1e5, 0, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_high_background_poor_resolution = TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) -parms_not_much_better = TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 2, 0.05, True) -parms_neg_peak_1x_base = TestParameters(-1e5, -1e4, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) -parms_neg_base = TestParameters(1e5, -10, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_small_signal_zero_base = TestParameters(1e-5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_neg_small_signal_zero_base = TestParameters(-1e5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_small_signal_finite_base = TestParameters(1e-5, 1e-7, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) -parms_no_signal_only_noise = TestParameters(0, 0, 1e-8, 0.1, 0.2, -1.0, 0.5, 11, 1, 0.05, False) -parms_bkg_plus_noise = TestParameters(0, 1, 0.1, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) -parms_bkg_plus_big_noise = TestParameters(0, 1, 100, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) -parms_no_signal__ZeroDivisionError = TestParameters(0, 0, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) -parms_bkg_only__ZeroDivisionError = TestParameters(0, 1, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) +parms_signal_but_high_background = _TestParameters(1e5, 1e6, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) +parms_model_peak = _TestParameters(1e5, 0, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_high_background_poor_resolution = _TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) +parms_not_much_better = _TestParameters(1e5, 1e4, 10, 0.1, 0.2, -0.7, 0.5, 11, 2, 0.05, True) +parms_neg_peak_1x_base = _TestParameters(-1e5, -1e4, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.1, True) +parms_neg_base = _TestParameters(1e5, -10, 10, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_small_signal_zero_base = _TestParameters(1e-5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_neg_small_signal_zero_base = _TestParameters(-1e5, 0, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_small_signal_finite_base = _TestParameters(1e-5, 1e-7, 1e-8, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, True) +parms_no_signal_only_noise = _TestParameters(0, 0, 1e-8, 0.1, 0.2, -1.0, 0.5, 11, 1, 0.05, False) +parms_bkg_plus_noise = _TestParameters(0, 1, 0.1, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) +parms_bkg_plus_big_noise = _TestParameters(0, 1, 100, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.05, False) +parms_no_signal__ZeroDivisionError = _TestParameters(0, 0, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) +parms_bkg_only__ZeroDivisionError = _TestParameters(0, 1, 0, 0.1, 0.2, -0.7, 0.5, 11, 1, 0.005, None) @pytest.mark.parametrize( @@ -376,7 +376,7 @@ class ImitatorForTesting(Device): parms_bkg_only__ZeroDivisionError, ], ) -def test_lineup2_signal_permutations(parms: TestParameters): +def test_lineup2_signal_permutations(parms: _TestParameters): starting_position = 0.0 m1.move(starting_position) time.sleep(1) # without this, the test IOC crashes, sometimes