From 5584f3964ca915a4100c56aa756af9a3a3f89d74 Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Fri, 12 Apr 2024 13:44:41 -0700 Subject: [PATCH 1/9] BUG: fixed tpr MultiDerivedSignals --- pcdsdevices/tpr.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pcdsdevices/tpr.py b/pcdsdevices/tpr.py index 9caa5e2c6ea..326ee911326 100644 --- a/pcdsdevices/tpr.py +++ b/pcdsdevices/tpr.py @@ -21,17 +21,17 @@ class TimingMode(Enum): def _get_delay(mds: MultiDerivedSignal, items: SignalToValue) -> float: """Calculates delay in ns from ticks and taps""" - return items[mds.attrs[0]] * TPR_TICK_NS + items[mds.attrs[1] * TPR_TAP_NS] + return items[mds.signals[0]] * TPR_TICK_NS + items[mds.signals[1]] * TPR_TAP_NS def _get_width(mds: MultiDerivedSignal, items: SignalToValue) -> float: """Calculates width in ns from ticks""" - return items[mds.attrs[0]] * TPR_TICK_NS + return items[mds.signals[0]] * TPR_TICK_NS def _put_last(mds: MultiDerivedSignal, value: float) -> SignalToValue: """Only writes value to last attr""" - return {mds.attrs[-1]: value} + return {mds.signals[-1]: value} class TprMotor(PVPositionerIsClose): From b8e07aa8311bd0b7d5b2ebccd250d3f216a815f8 Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Fri, 12 Apr 2024 13:51:36 -0700 Subject: [PATCH 2/9] DOC: added pre-release notes --- ...Fixing_Tpr_Bug_For_MultiDerivedSignals.rst | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst diff --git a/docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst b/docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst new file mode 100644 index 00000000000..844a2527578 --- /dev/null +++ b/docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst @@ -0,0 +1,32 @@ +1216 Fixing_Tpr_Bug_For_MultiDerivedSignals +################# + +API Breaks +---------- +- N/A + +Features +-------- +- N/A + +Device Updates +-------------- +- N/A + +New Devices +----------- +- N/A + +Bugfixes +-------- +- Previously, calculate_on_get/put functions used in MultiDerivedSignals in tpr classes were not accessing + their attrs correctly and would throw KeyErrors when called +- Specifically, the name of the attr was being used as the key for items dictionary instead of the actual signal object + +Maintenance +----------- +- N/A + +Contributors +------------ +- KaushikMalapati From d73804839ecf5b5f9dea74c761c11ba3052ce626 Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Fri, 12 Apr 2024 15:54:42 -0700 Subject: [PATCH 3/9] TST: added unit tests for tpr MultiDerivedSignals --- pcdsdevices/tests/test_tpr.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pcdsdevices/tests/test_tpr.py b/pcdsdevices/tests/test_tpr.py index 4d87931cc3d..b22991d710d 100644 --- a/pcdsdevices/tests/test_tpr.py +++ b/pcdsdevices/tests/test_tpr.py @@ -1,7 +1,7 @@ import pytest from ophyd.sim import make_fake_device -from ..tpr import TimingMode, TprTrigger +from ..tpr import TPR_TAP_NS, TPR_TICK_NS, TimingMode, TprTrigger @pytest.fixture(scope='function') @@ -22,3 +22,29 @@ def test_enable(fake_trigger): @pytest.mark.timeout(5) def test_disconnected_trigger(): TprTrigger('TST', channel=1, timing_mode=TimingMode.LCLS1, name='test') + + +def put_equals_setpoint(mds, setpoint, ns_time): + mds.put(ns_time) + return setpoint.value == ns_time + + +def mds_get(mds, tick_signal, tap_signal, num_ticks, num_taps): + tick_signal.put(num_ticks) + tap_signal.put(num_taps) + return mds.get() == (TPR_TICK_NS * num_ticks + TPR_TAP_NS * num_taps) + + +def test_ns_delay(fake_trigger): + assert put_equals_setpoint(fake_trigger.ns_delay, fake_trigger.delay_setpoint, 100) + assert mds_get(fake_trigger.ns_delay, fake_trigger.delay_ticks, fake_trigger.delay_taps, 0, 7) + + +def test_width(fake_trigger): + assert put_equals_setpoint(fake_trigger.width, fake_trigger.width_setpoint, 100) + assert fake_trigger.width.get() == fake_trigger.width_ticks.get() * TPR_TICK_NS + + +def test_motor(fake_trigger): + assert mds_get(fake_trigger.ns_delay_scan.readback, fake_trigger.ns_delay_scan.delay_ticks, + fake_trigger.ns_delay_scan.delay_taps, 3, 3) From a56b7b80648fdedea23d459c1ea2caf1720a9508 Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Fri, 12 Apr 2024 15:55:17 -0700 Subject: [PATCH 4/9] DOC: mentioned adding unit tests to prerelease notes --- .../1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst b/docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst index 844a2527578..10137bf0744 100644 --- a/docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst +++ b/docs/source/upcoming_release_notes/1216-Fixing_Tpr_Bug_For_MultiDerivedSignals.rst @@ -22,6 +22,7 @@ Bugfixes - Previously, calculate_on_get/put functions used in MultiDerivedSignals in tpr classes were not accessing their attrs correctly and would throw KeyErrors when called - Specifically, the name of the attr was being used as the key for items dictionary instead of the actual signal object +- Also added unit tests for these MultiDerivedSignals Maintenance ----------- From 22713047c4cac1548bcd1098c76a23a65c5deb22 Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Fri, 12 Apr 2024 16:05:56 -0700 Subject: [PATCH 5/9] TST: using get() instead of .value for setpoint --- pcdsdevices/tests/test_tpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pcdsdevices/tests/test_tpr.py b/pcdsdevices/tests/test_tpr.py index b22991d710d..1003d396315 100644 --- a/pcdsdevices/tests/test_tpr.py +++ b/pcdsdevices/tests/test_tpr.py @@ -26,7 +26,7 @@ def test_disconnected_trigger(): def put_equals_setpoint(mds, setpoint, ns_time): mds.put(ns_time) - return setpoint.value == ns_time + return setpoint.get() == ns_time def mds_get(mds, tick_signal, tap_signal, num_ticks, num_taps): From 5d10e9b5db015e3061a1455cc86b8be2ff78925e Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Fri, 12 Apr 2024 16:26:14 -0700 Subject: [PATCH 6/9] Using tick multiple for ns_time --- pcdsdevices/tests/test_tpr.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pcdsdevices/tests/test_tpr.py b/pcdsdevices/tests/test_tpr.py index 1003d396315..7919c708788 100644 --- a/pcdsdevices/tests/test_tpr.py +++ b/pcdsdevices/tests/test_tpr.py @@ -26,6 +26,8 @@ def test_disconnected_trigger(): def put_equals_setpoint(mds, setpoint, ns_time): mds.put(ns_time) + print("ns_time is", ns_time) + print("setpoint is", setpoint.get()) return setpoint.get() == ns_time @@ -36,12 +38,12 @@ def mds_get(mds, tick_signal, tap_signal, num_ticks, num_taps): def test_ns_delay(fake_trigger): - assert put_equals_setpoint(fake_trigger.ns_delay, fake_trigger.delay_setpoint, 100) + assert put_equals_setpoint(fake_trigger.ns_delay, fake_trigger.delay_setpoint, 538.4) assert mds_get(fake_trigger.ns_delay, fake_trigger.delay_ticks, fake_trigger.delay_taps, 0, 7) def test_width(fake_trigger): - assert put_equals_setpoint(fake_trigger.width, fake_trigger.width_setpoint, 100) + assert put_equals_setpoint(fake_trigger.width, fake_trigger.width_setpoint, 53.84) assert fake_trigger.width.get() == fake_trigger.width_ticks.get() * TPR_TICK_NS From 7a047f8d52760d6966dc5a51c1fa78b4d4a02829 Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Fri, 12 Apr 2024 19:09:04 -0700 Subject: [PATCH 7/9] Removing print --- pcdsdevices/tests/test_tpr.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pcdsdevices/tests/test_tpr.py b/pcdsdevices/tests/test_tpr.py index 7919c708788..1003d396315 100644 --- a/pcdsdevices/tests/test_tpr.py +++ b/pcdsdevices/tests/test_tpr.py @@ -26,8 +26,6 @@ def test_disconnected_trigger(): def put_equals_setpoint(mds, setpoint, ns_time): mds.put(ns_time) - print("ns_time is", ns_time) - print("setpoint is", setpoint.get()) return setpoint.get() == ns_time @@ -38,12 +36,12 @@ def mds_get(mds, tick_signal, tap_signal, num_ticks, num_taps): def test_ns_delay(fake_trigger): - assert put_equals_setpoint(fake_trigger.ns_delay, fake_trigger.delay_setpoint, 538.4) + assert put_equals_setpoint(fake_trigger.ns_delay, fake_trigger.delay_setpoint, 100) assert mds_get(fake_trigger.ns_delay, fake_trigger.delay_ticks, fake_trigger.delay_taps, 0, 7) def test_width(fake_trigger): - assert put_equals_setpoint(fake_trigger.width, fake_trigger.width_setpoint, 53.84) + assert put_equals_setpoint(fake_trigger.width, fake_trigger.width_setpoint, 100) assert fake_trigger.width.get() == fake_trigger.width_ticks.get() * TPR_TICK_NS From 47f23cbf441df5e66b2cdb40117be64b1f13ab67 Mon Sep 17 00:00:00 2001 From: KaushikMalapati <80156796+KaushikMalapati@users.noreply.github.com> Date: Mon, 15 Apr 2024 14:07:52 -0700 Subject: [PATCH 8/9] Update pcdsdevices/tests/test_tpr.py Co-authored-by: Robert Tang-Kong <35379409+tangkong@users.noreply.github.com> --- pcdsdevices/tests/test_tpr.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pcdsdevices/tests/test_tpr.py b/pcdsdevices/tests/test_tpr.py index 1003d396315..9add4271caf 100644 --- a/pcdsdevices/tests/test_tpr.py +++ b/pcdsdevices/tests/test_tpr.py @@ -35,6 +35,7 @@ def mds_get(mds, tick_signal, tap_signal, num_ticks, num_taps): return mds.get() == (TPR_TICK_NS * num_ticks + TPR_TAP_NS * num_taps) +@pytest.mark.timeout(5) def test_ns_delay(fake_trigger): assert put_equals_setpoint(fake_trigger.ns_delay, fake_trigger.delay_setpoint, 100) assert mds_get(fake_trigger.ns_delay, fake_trigger.delay_ticks, fake_trigger.delay_taps, 0, 7) From 4b85e39f53375afe8171657ff53abea887891db1 Mon Sep 17 00:00:00 2001 From: Kaushik Malapati Date: Tue, 16 Apr 2024 10:36:35 -0700 Subject: [PATCH 9/9] TST: using Status.wait after put to make sure put is complete before reading setpoint --- pcdsdevices/tests/test_tpr.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pcdsdevices/tests/test_tpr.py b/pcdsdevices/tests/test_tpr.py index 9add4271caf..fd655bda99d 100644 --- a/pcdsdevices/tests/test_tpr.py +++ b/pcdsdevices/tests/test_tpr.py @@ -25,7 +25,8 @@ def test_disconnected_trigger(): def put_equals_setpoint(mds, setpoint, ns_time): - mds.put(ns_time) + status = mds.put(ns_time) + status.wait() return setpoint.get() == ns_time @@ -41,6 +42,7 @@ def test_ns_delay(fake_trigger): assert mds_get(fake_trigger.ns_delay, fake_trigger.delay_ticks, fake_trigger.delay_taps, 0, 7) +@pytest.mark.timeout(5) def test_width(fake_trigger): assert put_equals_setpoint(fake_trigger.width, fake_trigger.width_setpoint, 100) assert fake_trigger.width.get() == fake_trigger.width_ticks.get() * TPR_TICK_NS