From 62677324e040279b0c064a06564e14fe81645eb5 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 13:17:59 -0400 Subject: [PATCH 1/4] Reset completed iterations counter to 0 once target iterations are reached and detector is disarmed --- src/ophyd_async/core/_detector.py | 6 +++++- tests/core/test_flyer.py | 4 ++++ tests/fastcs/panda/test_hdf_panda.py | 8 +++++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index 125f1867a..2a0fa5da5 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -314,7 +314,10 @@ async def prepare(self, value: TriggerInfo) -> None: async def kickoff(self): assert self._trigger_info, "Prepare must be called before kickoff!" if self._iterations_completed >= self._trigger_info.iteration: - raise Exception(f"Kickoff called more than {self._trigger_info.iteration}") + raise Exception( + f"Kickoff called more than the configured number of " + f"{self._trigger_info.iteration} iteration(s)!" + ) self._iterations_completed += 1 @WatchableAsyncStatus.wrap @@ -340,6 +343,7 @@ async def complete(self): if index >= self._trigger_info.number: break if self._iterations_completed == self._trigger_info.iteration: + self._iterations_completed = 0 await self.controller.wait_for_idle() async def describe_collect(self) -> dict[str, DataKey]: diff --git a/tests/core/test_flyer.py b/tests/core/test_flyer.py index b9a318613..db8ad98ce 100644 --- a/tests/core/test_flyer.py +++ b/tests/core/test_flyer.py @@ -183,6 +183,10 @@ def flying_plan(): for detector in detectors: yield from bps.kickoff(detector) + # Since we set number of iterations to 1 (default), + # make sure it gets reset + assert detector._iterations_completed == 0 + yield from bps.complete(flyer, wait=False, group="complete") for detector in detectors: yield from bps.complete(detector, wait=False, group="complete") diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index 41f50d648..f02106895 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -224,8 +224,10 @@ def flying_plan(): yield from bps.declare_stream(mock_hdf_panda, name="main_stream", collect=True) - for _ in range(iteration): + for i in range(iteration): set_mock_value(flyer.trigger_logic.seq.active, 1) + assert mock_hdf_panda._iterations_completed == i + yield from bps.kickoff(flyer, wait=True) yield from bps.kickoff(mock_hdf_panda) @@ -250,6 +252,10 @@ def flying_plan(): name="main_stream", ) yield from bps.wait(group="complete") + + # Make sure the number of iterations completed is set to 0 after final complete. + assert mock_hdf_panda._iterations_completed == 0 + yield from bps.close_run() yield from bps.unstage_all(flyer, mock_hdf_panda) From 659def8f9d0b70635781dfad1d772b48521f85e3 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 13:37:37 -0400 Subject: [PATCH 2/4] Fix tests --- tests/core/test_flyer.py | 11 +++++++---- tests/fastcs/panda/test_hdf_panda.py | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/core/test_flyer.py b/tests/core/test_flyer.py index db8ad98ce..af6b08e92 100644 --- a/tests/core/test_flyer.py +++ b/tests/core/test_flyer.py @@ -183,13 +183,10 @@ def flying_plan(): for detector in detectors: yield from bps.kickoff(detector) - # Since we set number of iterations to 1 (default), - # make sure it gets reset - assert detector._iterations_completed == 0 - yield from bps.complete(flyer, wait=False, group="complete") for detector in detectors: yield from bps.complete(detector, wait=False, group="complete") + assert flyer._trigger_logic.state == TriggerState.null # Manually incremenet the index as if a frame was taken @@ -210,6 +207,12 @@ def flying_plan(): name="main_stream", ) yield from bps.wait(group="complete") + + for detector in detectors: + # Since we set number of iterations to 1 (default), + # make sure it gets reset on complete + assert detector._iterations_completed == 0 + yield from bps.close_run() yield from bps.unstage_all(flyer, *detectors) diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index f02106895..8b4767081 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -226,10 +226,10 @@ def flying_plan(): for i in range(iteration): set_mock_value(flyer.trigger_logic.seq.active, 1) - assert mock_hdf_panda._iterations_completed == i yield from bps.kickoff(flyer, wait=True) yield from bps.kickoff(mock_hdf_panda) + assert mock_hdf_panda._iterations_completed == i + 1 yield from bps.complete(flyer, wait=False, group="complete") yield from bps.complete(mock_hdf_panda, wait=False, group="complete") @@ -253,6 +253,10 @@ def flying_plan(): ) yield from bps.wait(group="complete") + # Make sure first complete doesn't reset iterations completed + if i == 0: + assert mock_hdf_panda._iterations_completed == 1 + # Make sure the number of iterations completed is set to 0 after final complete. assert mock_hdf_panda._iterations_completed == 0 From 4bf0b0634547f9662bf8be4e4a7f5709186cb2fe Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 13:56:28 -0400 Subject: [PATCH 3/4] Add test for double kickoff to increase coverage --- tests/core/test_flyer.py | 67 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/core/test_flyer.py b/tests/core/test_flyer.py index af6b08e92..ba570c5e2 100644 --- a/tests/core/test_flyer.py +++ b/tests/core/test_flyer.py @@ -234,6 +234,73 @@ def flying_plan(): ] +async def test_hardware_triggered_flyable_too_many_kickoffs( + RE: RunEngine, detectors: tuple[StandardDetector] +): + trigger_logic = DummyTriggerLogic() + flyer = StandardFlyer(trigger_logic, [], name="flyer") + trigger_info = TriggerInfo( + number=1, trigger=DetectorTrigger.constant_gate, deadtime=2, livetime=2 + ) + + def flying_plan(): + yield from bps.stage_all(*detectors, flyer) + assert flyer._trigger_logic.state == TriggerState.stopping + + # move the flyer to the correct place, before fly scanning. + # Prepare the flyer first to get the trigger info for the detectors + yield from bps.prepare(flyer, 1, wait=True) + + # prepare detectors second. + for detector in detectors: + yield from bps.prepare( + detector, + trigger_info, + wait=True, + ) + + + yield from bps.open_run() + yield from bps.declare_stream(*detectors, name="main_stream", collect=True) + + for _ in range(2): + yield from bps.kickoff(flyer) + for detector in detectors: + yield from bps.kickoff(detector) + + yield from bps.complete(flyer, wait=False, group="complete") + for detector in detectors: + yield from bps.complete(detector, wait=False, group="complete") + + assert flyer._trigger_logic.state == TriggerState.null + + # Manually incremenet the index as if a frame was taken + for detector in detectors: + detector.writer.index += 1 + + yield from bps.wait(group="complete") + + yield from bps.collect( + *detectors, + return_payload=False, + name="main_stream", + ) + + for detector in detectors: + # Since we set number of iterations to 1 (default), + # make sure it gets reset on complete + assert detector._iterations_completed == 0 + + yield from bps.close_run() + + yield from bps.unstage_all(flyer, *detectors) + + # fly scan + with pytest.raises(Exception, match="Kickoff called more than the configured number"): + RE(flying_plan()) + + + # To do: Populate configuration signals async def test_describe_configuration(): flyer = StandardFlyer(DummyTriggerLogic(), [], name="flyer") From 945de665cb4ca231899581f7233a131ceb22774f Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Thu, 19 Sep 2024 16:19:33 -0400 Subject: [PATCH 4/4] Make linter happy --- tests/core/test_flyer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/core/test_flyer.py b/tests/core/test_flyer.py index ba570c5e2..c7345ee03 100644 --- a/tests/core/test_flyer.py +++ b/tests/core/test_flyer.py @@ -259,7 +259,6 @@ def flying_plan(): wait=True, ) - yield from bps.open_run() yield from bps.declare_stream(*detectors, name="main_stream", collect=True) @@ -296,11 +295,12 @@ def flying_plan(): yield from bps.unstage_all(flyer, *detectors) # fly scan - with pytest.raises(Exception, match="Kickoff called more than the configured number"): + with pytest.raises( + Exception, match="Kickoff called more than the configured number" + ): RE(flying_plan()) - # To do: Populate configuration signals async def test_describe_configuration(): flyer = StandardFlyer(DummyTriggerLogic(), [], name="flyer")