From 45155e0096630be46b2a77abb82683da75edffd4 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 11 Sep 2024 11:15:44 +0100 Subject: [PATCH 01/12] added interface changes for detectors --- docs/examples/foo_detector.py | 18 +++----- src/ophyd_async/core/_detector.py | 45 +++++++++---------- .../epics/adaravis/_aravis_controller.py | 20 ++++----- .../epics/adkinetix/_kinetix_controller.py | 19 ++++---- .../epics/adpilatus/_pilatus_controller.py | 20 ++++----- .../epics/adsimdetector/_sim_controller.py | 23 +++++----- .../epics/advimba/_vimba_controller.py | 25 +++++------ .../epics/eiger/_eiger_controller.py | 26 +++++------ tests/epics/adaravis/test_aravis.py | 10 ++++- tests/epics/adcore/test_scans.py | 11 ++--- tests/epics/adkinetix/test_kinetix.py | 5 ++- tests/epics/adpilatus/test_pilatus.py | 12 ++--- .../adpilatus/test_pilatus_controller.py | 4 +- .../adsimdetector/test_adsim_controller.py | 4 +- tests/epics/adsimdetector/test_sim.py | 3 -- tests/epics/advimba/test_vimba.py | 3 +- tests/epics/eiger/test_eiger_controller.py | 13 ++++-- 17 files changed, 131 insertions(+), 130 deletions(-) diff --git a/docs/examples/foo_detector.py b/docs/examples/foo_detector.py index bf3e41e6f6..c93f4c6e41 100644 --- a/docs/examples/foo_detector.py +++ b/docs/examples/foo_detector.py @@ -1,14 +1,13 @@ import asyncio -from typing import Optional from bluesky.protocols import HasHints, Hints from ophyd_async.core import ( AsyncStatus, DetectorControl, - DetectorTrigger, PathProvider, StandardDetector, + TriggerInfo, ) from ophyd_async.epics import adcore from ophyd_async.epics.signal import epics_signal_rw_rbv @@ -28,19 +27,16 @@ def get_deadtime(self, exposure: float) -> float: # FooDetector deadtime handling return 0.001 - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: + async def prepare(self, trigger_info: TriggerInfo): await asyncio.gather( - self._drv.num_images.set(num), + self._drv.num_images.set(trigger_info.number), self._drv.image_mode.set(adcore.ImageMode.multiple), - self._drv.trigger_mode.set(f"FOO{trigger}"), + self._drv.trigger_mode.set(f"FOO{trigger_info}"), ) - if exposure is not None: + if exposure := trigger_info.livetime is not None: await self._drv.acquire_time.set(exposure) + + async def arm(self) -> AsyncStatus: return await adcore.start_acquiring_driver_and_ensure_status(self._drv) async def disarm(self): diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index 7d22fa6ace..7567308d78 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -53,13 +53,13 @@ class TriggerInfo(BaseModel): #: Number of triggers that will be sent, 0 means infinite number: int = Field(gt=0) #: Sort of triggers that will be sent - trigger: DetectorTrigger = Field() + trigger: DetectorTrigger = Field(default=DetectorTrigger.internal) #: What is the minimum deadtime between triggers - deadtime: float | None = Field(ge=0) + deadtime: float | None = Field(default=None, ge=0) #: What is the maximum high time of the triggers - livetime: float | None = Field(ge=0) + livetime: float | None = Field(default=None, ge=0) #: What is the maximum timeout on waiting for a frame - frame_timeout: float | None = Field(None, gt=0) + frame_timeout: float | None = Field(default=None, gt=0) #: How many triggers make up a single StreamDatum index, to allow multiple frames #: from a faster detector to be zipped with a single frame from a slow detector #: e.g. if num=10 and multiplier=5 then the detector will take 10 frames, @@ -78,15 +78,8 @@ def get_deadtime(self, exposure: float | None) -> float: """For a given exposure, how long should the time between exposures be""" @abstractmethod - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: - """ - Arm detector, do all necessary steps to prepare detector for triggers. - + async def prepare(self, trigger_info: TriggerInfo): + """, do all necessary steps to prepare detector for triggers. Args: num: Expected number of frames trigger: Type of trigger for which to prepare the detector. Defaults to @@ -94,6 +87,13 @@ async def arm( exposure: Exposure time with which to set up the detector. Defaults to None if not applicable or the detector is expected to use its previously-set exposure time. + """ + ... + + @abstractmethod + async def arm(self) -> AsyncStatus: + """ + Arm detector Returns: AsyncStatus: Status representing the arm operation. This function returning @@ -248,14 +248,14 @@ async def trigger(self) -> None: trigger=DetectorTrigger.internal, deadtime=None, livetime=None, + frame_timeout=None, ) ) + assert self._trigger_info + assert self._trigger_info.trigger is DetectorTrigger.internal # Arm the detector and wait for it to finish. indices_written = await self.writer.get_indices_written() - written_status = await self.controller.arm( - num=self._trigger_info.number, - trigger=self._trigger_info.trigger, - ) + written_status = await self.controller.arm() await written_status end_observation = indices_written + 1 @@ -296,12 +296,10 @@ async def prepare(self, value: TriggerInfo) -> None: ) self._initial_frame = await self.writer.get_indices_written() self._last_frame = self._initial_frame + self._trigger_info.number - self._arm_status = await self.controller.arm( - num=self._trigger_info.number, - trigger=self._trigger_info.trigger, - exposure=self._trigger_info.livetime, - ) - self._fly_start = time.monotonic() + await self.controller.prepare(value) + if self._trigger_info.trigger is not DetectorTrigger.internal: + self._arm_status = await self.controller.arm() + self._fly_start = time.monotonic() self._describe = await self.writer.open(value.multiplier) @AsyncStatus.wrap @@ -312,6 +310,7 @@ async def kickoff(self): @WatchableAsyncStatus.wrap async def complete(self): assert self._arm_status, "Prepare not run" + await self._arm_status assert self._trigger_info async for index in self.writer.observe_indices_written( self._trigger_info.frame_timeout diff --git a/src/ophyd_async/epics/adaravis/_aravis_controller.py b/src/ophyd_async/epics/adaravis/_aravis_controller.py index 6349d111b1..12fc962ba9 100644 --- a/src/ophyd_async/epics/adaravis/_aravis_controller.py +++ b/src/ophyd_async/epics/adaravis/_aravis_controller.py @@ -1,10 +1,11 @@ import asyncio -from typing import Literal, Optional, Tuple +from typing import Literal, Tuple from ophyd_async.core import ( AsyncStatus, DetectorControl, DetectorTrigger, + TriggerInfo, set_and_wait_for_value, ) from ophyd_async.epics import adcore @@ -27,20 +28,15 @@ def __init__(self, driver: AravisDriverIO, gpio_number: GPIO_NUMBER) -> None: def get_deadtime(self, exposure: float) -> float: return _HIGHEST_POSSIBLE_DEADTIME - async def arm( - self, - num: int = 0, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: - if num == 0: + async def prepare(self, trigger_info: TriggerInfo): + if (num := trigger_info.number) == 0: image_mode = adcore.ImageMode.continuous else: image_mode = adcore.ImageMode.multiple - if exposure is not None: + if (exposure := trigger_info.livetime) is not None: await self._drv.acquire_time.set(exposure) - trigger_mode, trigger_source = self._get_trigger_info(trigger) + trigger_mode, trigger_source = self._get_trigger_info(trigger_info.trigger) # trigger mode must be set first and on it's own! await self._drv.trigger_mode.set(trigger_mode) @@ -50,8 +46,8 @@ async def arm( self._drv.image_mode.set(image_mode), ) - status = await set_and_wait_for_value(self._drv.acquire, True) - return status + async def arm(self) -> AsyncStatus: + return await set_and_wait_for_value(self._drv.acquire, True) def _get_trigger_info( self, trigger: DetectorTrigger diff --git a/src/ophyd_async/epics/adkinetix/_kinetix_controller.py b/src/ophyd_async/epics/adkinetix/_kinetix_controller.py index 9469fda68a..21e339fc95 100644 --- a/src/ophyd_async/epics/adkinetix/_kinetix_controller.py +++ b/src/ophyd_async/epics/adkinetix/_kinetix_controller.py @@ -1,7 +1,7 @@ import asyncio -from typing import Optional from ophyd_async.core import AsyncStatus, DetectorControl, DetectorTrigger +from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adcore from ._kinetix_io import KinetixDriverIO, KinetixTriggerMode @@ -24,22 +24,19 @@ def __init__( def get_deadtime(self, exposure: float) -> float: return 0.001 - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: + async def prepare(self, trigger_info: TriggerInfo): await asyncio.gather( - self._drv.trigger_mode.set(KINETIX_TRIGGER_MODE_MAP[trigger]), - self._drv.num_images.set(num), + self._drv.trigger_mode.set(KINETIX_TRIGGER_MODE_MAP[trigger_info.trigger]), + self._drv.num_images.set(trigger_info.number), self._drv.image_mode.set(adcore.ImageMode.multiple), ) - if exposure is not None and trigger not in [ + if trigger_info.livetime is not None and trigger_info.trigger not in [ DetectorTrigger.variable_gate, DetectorTrigger.constant_gate, ]: - await self._drv.acquire_time.set(exposure) + await self._drv.acquire_time.set(trigger_info.livetime) + + async def arm(self) -> AsyncStatus: return await adcore.start_acquiring_driver_and_ensure_status(self._drv) async def disarm(self): diff --git a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py index dd48eaf50c..65548e91b2 100644 --- a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py +++ b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py @@ -1,5 +1,4 @@ import asyncio -from typing import Optional from ophyd_async.core import ( DEFAULT_TIMEOUT, @@ -8,6 +7,7 @@ DetectorTrigger, wait_for_value, ) +from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adcore from ._pilatus_io import PilatusDriverIO, PilatusTriggerMode @@ -31,22 +31,20 @@ def __init__( def get_deadtime(self, exposure: float) -> float: return self._readout_time - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: - if exposure is not None: + async def prepare(self, trigger_info: TriggerInfo): + if trigger_info.livetime is not None: await adcore.set_exposure_time_and_acquire_period_if_supplied( - self, self._drv, exposure + self, self._drv, trigger_info.livetime ) await asyncio.gather( - self._drv.trigger_mode.set(self._get_trigger_mode(trigger)), - self._drv.num_images.set(999_999 if num == 0 else num), + self._drv.trigger_mode.set(self._get_trigger_mode(trigger_info.trigger)), + self._drv.num_images.set( + 999_999 if trigger_info.number == 0 else trigger_info.number + ), self._drv.image_mode.set(adcore.ImageMode.multiple), ) + async def arm(self) -> AsyncStatus: # Standard arm the detector and wait for the acquire PV to be True idle_status = await adcore.start_acquiring_driver_and_ensure_status(self._drv) diff --git a/src/ophyd_async/epics/adsimdetector/_sim_controller.py b/src/ophyd_async/epics/adsimdetector/_sim_controller.py index 789f89701c..cc39f54f3f 100644 --- a/src/ophyd_async/epics/adsimdetector/_sim_controller.py +++ b/src/ophyd_async/epics/adsimdetector/_sim_controller.py @@ -1,5 +1,5 @@ import asyncio -from typing import Optional, Set +from typing import Set from ophyd_async.core import ( DEFAULT_TIMEOUT, @@ -7,6 +7,7 @@ DetectorControl, DetectorTrigger, ) +from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adcore @@ -18,26 +19,26 @@ def __init__( ) -> None: self.driver = driver self.good_states = good_states + self.frame_timeout: float def get_deadtime(self, exposure: float) -> float: return 0.002 - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: + async def prepare(self, trigger_info: TriggerInfo): assert ( - trigger == DetectorTrigger.internal + trigger_info.trigger == DetectorTrigger.internal ), "fly scanning (i.e. external triggering) is not supported for this device" - frame_timeout = DEFAULT_TIMEOUT + await self.driver.acquire_time.get_value() + self.frame_timeout = ( + DEFAULT_TIMEOUT + await self.driver.acquire_time.get_value() + ) await asyncio.gather( - self.driver.num_images.set(num), + self.driver.num_images.set(trigger_info.number), self.driver.image_mode.set(adcore.ImageMode.multiple), ) + + async def arm(self) -> AsyncStatus: return await adcore.start_acquiring_driver_and_ensure_status( - self.driver, good_states=self.good_states, timeout=frame_timeout + self.driver, good_states=self.good_states, timeout=self.frame_timeout ) async def disarm(self): diff --git a/src/ophyd_async/epics/advimba/_vimba_controller.py b/src/ophyd_async/epics/advimba/_vimba_controller.py index fa0232dd2a..ec0996dfdb 100644 --- a/src/ophyd_async/epics/advimba/_vimba_controller.py +++ b/src/ophyd_async/epics/advimba/_vimba_controller.py @@ -1,7 +1,7 @@ import asyncio -from typing import Optional from ophyd_async.core import AsyncStatus, DetectorControl, DetectorTrigger +from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adcore from ._vimba_io import VimbaDriverIO, VimbaExposeOutMode, VimbaOnOff, VimbaTriggerSource @@ -31,27 +31,26 @@ def __init__( def get_deadtime(self, exposure: float) -> float: return 0.001 - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: + async def prepare(self, trigger_info: TriggerInfo): await asyncio.gather( - self._drv.trigger_mode.set(TRIGGER_MODE[trigger]), - self._drv.exposure_mode.set(EXPOSE_OUT_MODE[trigger]), - self._drv.num_images.set(num), + self._drv.trigger_mode.set(TRIGGER_MODE[trigger_info.trigger]), + self._drv.exposure_mode.set(EXPOSE_OUT_MODE[trigger_info.trigger]), + self._drv.num_images.set(trigger_info.number), self._drv.image_mode.set(adcore.ImageMode.multiple), ) - if exposure is not None and trigger not in [ + if trigger_info.livetime is not None and trigger_info.trigger not in [ DetectorTrigger.variable_gate, DetectorTrigger.constant_gate, ]: - await self._drv.acquire_time.set(exposure) - if trigger != DetectorTrigger.internal: + await self._drv.acquire_time.set(trigger_info.livetime) + if trigger_info.trigger != DetectorTrigger.internal: self._drv.trigger_source.set(VimbaTriggerSource.line1) else: self._drv.trigger_source.set(VimbaTriggerSource.freerun) + + async def arm( + self, + ) -> AsyncStatus: return await adcore.start_acquiring_driver_and_ensure_status(self._drv) async def disarm(self): diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py index fa37bbebaf..a23c937834 100644 --- a/src/ophyd_async/epics/eiger/_eiger_controller.py +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -1,5 +1,4 @@ import asyncio -from typing import Optional from ophyd_async.core import ( DEFAULT_TIMEOUT, @@ -8,6 +7,7 @@ DetectorTrigger, set_and_wait_for_other_value, ) +from ophyd_async.core._detector import TriggerInfo from ._eiger_io import EigerDriverIO, EigerTriggerMode @@ -37,26 +37,26 @@ async def set_energy(self, energy: float, tolerance: float = 0.1): if abs(current_energy - energy) > tolerance: await self._drv.photon_energy.set(energy) - @AsyncStatus.wrap - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ): + async def prepare(self, trigger_info: TriggerInfo): coros = [ - self._drv.trigger_mode.set(EIGER_TRIGGER_MODE_MAP[trigger].value), - self._drv.num_images.set(num), + self._drv.trigger_mode.set( + EIGER_TRIGGER_MODE_MAP[trigger_info.trigger].value + ), + self._drv.num_images.set(trigger_info.number), ] - if exposure is not None: + if trigger_info.livetime is not None: coros.extend( [ - self._drv.acquire_time.set(exposure), - self._drv.acquire_period.set(exposure), + self._drv.acquire_time.set(trigger_info.livetime), + self._drv.acquire_period.set(trigger_info.livetime), ] ) await asyncio.gather(*coros) + @AsyncStatus.wrap + async def arm( + self, + ): # TODO: Detector state should be an enum see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 await set_and_wait_for_other_value( self._drv.arm, 1, self._drv.state, "ready", timeout=DEFAULT_TIMEOUT diff --git a/tests/epics/adaravis/test_aravis.py b/tests/epics/adaravis/test_aravis.py index efefae2eb2..fb1ac89832 100644 --- a/tests/epics/adaravis/test_aravis.py +++ b/tests/epics/adaravis/test_aravis.py @@ -36,7 +36,15 @@ async def test_trigger_source_set_to_gpio_line(test_adaravis: adaravis.AravisDet set_mock_value(test_adaravis.drv.trigger_source, "Freerun") async def trigger_and_complete(): - await test_adaravis.controller.arm(num=1, trigger=DetectorTrigger.edge_trigger) + await test_adaravis.controller.prepare( + TriggerInfo( + number=1, + trigger=DetectorTrigger.edge_trigger, + livetime=None, + deadtime=None, + frame_timeout=None, + ) + ) # Prevent timeouts set_mock_value(test_adaravis.drv.acquire, True) diff --git a/tests/epics/adcore/test_scans.py b/tests/epics/adcore/test_scans.py index 3936885083..31c0166841 100644 --- a/tests/epics/adcore/test_scans.py +++ b/tests/epics/adcore/test_scans.py @@ -37,14 +37,11 @@ async def stop(self): ... class DummyController(DetectorControl): def __init__(self) -> None: ... + async def prepare(self, trigger_info: TriggerInfo): + return AsyncStatus(asyncio.sleep(0.01)) - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = None, - ) -> AsyncStatus: - return AsyncStatus(asyncio.sleep(0.1)) + async def arm(self) -> AsyncStatus: + return AsyncStatus(asyncio.sleep(0.01)) async def disarm(self): ... diff --git a/tests/epics/adkinetix/test_kinetix.py b/tests/epics/adkinetix/test_kinetix.py index 3a53091f40..d2ff03e433 100644 --- a/tests/epics/adkinetix/test_kinetix.py +++ b/tests/epics/adkinetix/test_kinetix.py @@ -33,7 +33,10 @@ async def test_trigger_modes(test_adkinetix: adkinetix.KinetixDetector): set_mock_value(test_adkinetix.drv.trigger_mode, "Internal") async def setup_trigger_mode(trig_mode: DetectorTrigger): - await test_adkinetix.controller.arm(num=1, trigger=trig_mode) + await test_adkinetix.controller.prepare( + TriggerInfo(number=1, trigger=trig_mode) + ) + await test_adkinetix.controller.arm() # Prevent timeouts set_mock_value(test_adkinetix.drv.acquire, True) diff --git a/tests/epics/adpilatus/test_pilatus.py b/tests/epics/adpilatus/test_pilatus.py index 00b3c119a8..d0917a850b 100644 --- a/tests/epics/adpilatus/test_pilatus.py +++ b/tests/epics/adpilatus/test_pilatus.py @@ -61,10 +61,10 @@ async def test_trigger_mode_set( ): async def trigger_and_complete(): set_mock_value(test_adpilatus.drv.armed, True) - status = await test_adpilatus.controller.arm( - num=1, - trigger=detector_trigger, + await test_adpilatus.controller.prepare( + TriggerInfo(number=1, trigger=detector_trigger) ) + status = await test_adpilatus.controller.arm() await status await _trigger(test_adpilatus, expected_trigger_mode, trigger_and_complete) @@ -74,10 +74,10 @@ async def test_trigger_mode_set_without_armed_pv( test_adpilatus: adpilatus.PilatusDetector, ): async def trigger_and_complete(): - status = await test_adpilatus.controller.arm( - num=1, - trigger=DetectorTrigger.internal, + await test_adpilatus.controller.prepare( + TriggerInfo(number=1, trigger=DetectorTrigger.internal) ) + status = await test_adpilatus.controller.arm() await status with patch( diff --git a/tests/epics/adpilatus/test_pilatus_controller.py b/tests/epics/adpilatus/test_pilatus_controller.py index 00194fdb94..6ead5b2bcb 100644 --- a/tests/epics/adpilatus/test_pilatus_controller.py +++ b/tests/epics/adpilatus/test_pilatus_controller.py @@ -1,6 +1,7 @@ import pytest from ophyd_async.core import DetectorTrigger, DeviceCollector, set_mock_value +from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adcore, adpilatus @@ -28,7 +29,8 @@ async def test_pilatus_controller( pilatus_driver: adpilatus.PilatusDriverIO, ): set_mock_value(pilatus_driver.armed, True) - status = await pilatus.arm(num=1, trigger=DetectorTrigger.constant_gate) + await pilatus.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate)) + status = await pilatus.arm() await status assert await pilatus_driver.num_images.get_value() == 1 diff --git a/tests/epics/adsimdetector/test_adsim_controller.py b/tests/epics/adsimdetector/test_adsim_controller.py index b2610fe31b..b9667eeb3e 100644 --- a/tests/epics/adsimdetector/test_adsim_controller.py +++ b/tests/epics/adsimdetector/test_adsim_controller.py @@ -3,6 +3,7 @@ import pytest from ophyd_async.core import DeviceCollector +from ophyd_async.core._detector import DetectorTrigger, TriggerInfo from ophyd_async.epics import adcore, adsimdetector @@ -17,7 +18,8 @@ async def ad(RE) -> adsimdetector.SimController: async def test_ad_controller(RE, ad: adsimdetector.SimController): with patch("ophyd_async.core._signal.wait_for_value", return_value=None): - await ad.arm(num=1) + await ad.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.internal)) + await ad.arm() driver = ad.driver assert await driver.num_images.get_value() == 1 diff --git a/tests/epics/adsimdetector/test_sim.py b/tests/epics/adsimdetector/test_sim.py index 17494e1bb5..60381cc364 100644 --- a/tests/epics/adsimdetector/test_sim.py +++ b/tests/epics/adsimdetector/test_sim.py @@ -117,9 +117,6 @@ async def test_two_detectors_fly_different_rate( trigger_info = TriggerInfo( number=15, trigger=DetectorTrigger.internal, - deadtime=None, - livetime=None, - frame_timeout=None, ) docs = defaultdict(list) diff --git a/tests/epics/advimba/test_vimba.py b/tests/epics/advimba/test_vimba.py index 0bc32b887b..31bba268ba 100644 --- a/tests/epics/advimba/test_vimba.py +++ b/tests/epics/advimba/test_vimba.py @@ -35,7 +35,8 @@ async def test_arming_trig_modes(test_advimba: advimba.VimbaDetector): set_mock_value(test_advimba.drv.exposure_mode, "Timed") async def setup_trigger_mode(trig_mode: DetectorTrigger): - await test_advimba.controller.arm(num=1, trigger=trig_mode) + await test_advimba.controller.prepare(TriggerInfo(number=1, trigger=trig_mode)) + await test_advimba.controller.arm() # Prevent timeouts set_mock_value(test_advimba.drv.acquire, True) diff --git a/tests/epics/eiger/test_eiger_controller.py b/tests/epics/eiger/test_eiger_controller.py index 70f3078163..01e76a9653 100644 --- a/tests/epics/eiger/test_eiger_controller.py +++ b/tests/epics/eiger/test_eiger_controller.py @@ -8,6 +8,7 @@ get_mock_put, set_mock_value, ) +from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics.eiger._eiger_controller import EigerController from ophyd_async.epics.eiger._eiger_io import EigerDriverIO @@ -43,7 +44,8 @@ async def test_when_arm_with_exposure_then_time_and_period_set( ): driver, controller = eiger_driver_and_controller test_exposure = 0.002 - await controller.arm(10, exposure=test_exposure) + await controller.prepare(TriggerInfo(number=10, livetime=test_exposure)) + await controller.arm() assert (await driver.acquire_period.get_value()) == test_exposure assert (await driver.acquire_time.get_value()) == test_exposure @@ -52,7 +54,8 @@ async def test_when_arm_with_no_exposure_then_arm_set_correctly( eiger_driver_and_controller: DriverAndController, ): driver, controller = eiger_driver_and_controller - await controller.arm(10, exposure=None) + await controller.prepare(TriggerInfo(number=10)) + await controller.arm() get_mock_put(driver.arm).assert_called_once_with(1, wait=ANY, timeout=ANY) @@ -61,7 +64,8 @@ async def test_when_arm_with_number_of_images_then_number_of_images_set_correctl ): driver, controller = eiger_driver_and_controller test_number_of_images = 40 - await controller.arm(test_number_of_images, exposure=None) + await controller.prepare(TriggerInfo(number=test_number_of_images)) + await controller.arm() get_mock_put(driver.num_images).assert_called_once_with( test_number_of_images, wait=ANY, timeout=ANY ) @@ -73,7 +77,8 @@ async def test_given_detector_fails_to_go_ready_when_arm_called_then_fails( ): driver, controller = eiger_driver_and_controller_no_arm with raises(TimeoutError): - await controller.arm(10) + await controller.prepare(TriggerInfo(number=10)) + await controller.arm() async def test_when_disarm_called_on_controller_then_disarm_called_on_driver( From dcd96328632d44de3e0cd51e842181aad0ea30e7 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 11 Sep 2024 11:53:23 +0100 Subject: [PATCH 02/12] added detector code for panda and pattern_detector --- src/ophyd_async/fastcs/panda/_control.py | 15 ++++---- .../_pattern_detector_controller.py | 36 ++++++++++--------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/ophyd_async/fastcs/panda/_control.py b/src/ophyd_async/fastcs/panda/_control.py index 08c17df1ef..ed4b330c64 100644 --- a/src/ophyd_async/fastcs/panda/_control.py +++ b/src/ophyd_async/fastcs/panda/_control.py @@ -1,5 +1,4 @@ import asyncio -from typing import Optional from ophyd_async.core import ( AsyncStatus, @@ -7,6 +6,7 @@ DetectorTrigger, wait_for_value, ) +from ophyd_async.core._detector import TriggerInfo from ._block import PcapBlock @@ -18,18 +18,15 @@ def __init__(self, pcap: PcapBlock) -> None: def get_deadtime(self, exposure: float) -> float: return 0.000000008 - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.constant_gate, - exposure: Optional[float] = None, - ) -> AsyncStatus: - assert trigger in ( + async def prepare(self, trigger_info: TriggerInfo): + assert trigger_info.trigger in ( DetectorTrigger.constant_gate, - trigger == DetectorTrigger.variable_gate, + trigger_info.trigger == DetectorTrigger.variable_gate, ), "Only constant_gate and variable_gate triggering is supported on the PandA" await asyncio.gather(self.pcap.arm.set(True)) await wait_for_value(self.pcap.active, True, timeout=1) + + async def arm(self) -> AsyncStatus: return AsyncStatus(wait_for_value(self.pcap.active, False, timeout=None)) async def disarm(self) -> AsyncStatus: diff --git a/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py b/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py index 064bea873f..d739712ffa 100644 --- a/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py +++ b/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py @@ -1,7 +1,10 @@ import asyncio from typing import Optional -from ophyd_async.core import AsyncStatus, DetectorControl, DetectorTrigger, PathProvider +from pydantic import Field + +from ophyd_async.core import AsyncStatus, DetectorControl, PathProvider +from ophyd_async.core._detector import TriggerInfo from ._pattern_generator import PatternGenerator @@ -11,30 +14,29 @@ def __init__( self, pattern_generator: PatternGenerator, path_provider: PathProvider, - exposure: float = 0.1, + exposure: float = Field(default=0.1), ) -> None: self.pattern_generator: PatternGenerator = pattern_generator - if exposure is None: - exposure = 0.1 self.pattern_generator.set_exposure(exposure) self.path_provider: PathProvider = path_provider self.task: Optional[asyncio.Task] = None super().__init__() - async def arm( - self, - num: int, - trigger: DetectorTrigger = DetectorTrigger.internal, - exposure: Optional[float] = 0.01, - ) -> AsyncStatus: - if exposure is None: - exposure = 0.1 - period: float = exposure + self.get_deadtime(exposure) - task = asyncio.create_task( - self._coroutine_for_image_writing(exposure, period, num) + async def prepare( + self, trigger_info: TriggerInfo = TriggerInfo(number=1, livetime=0.01) + ): + if trigger_info.livetime is None: + trigger_info.livetime = 0.01 + period: float = trigger_info.livetime + self.get_deadtime(trigger_info.livetime) + self.task = asyncio.create_task( + self._coroutine_for_image_writing( + trigger_info.livetime, period, trigger_info.number + ) ) - self.task = task - return AsyncStatus(task) + + async def arm(self) -> AsyncStatus: + assert self.task + return AsyncStatus(self.task) async def disarm(self): if self.task: From aab9884233c1005d3c229200523afa0c5e0cf406 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 11 Sep 2024 12:50:24 +0100 Subject: [PATCH 03/12] added prepare for pandapcapcontroller --- tests/fastcs/panda/test_panda_control.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/fastcs/panda/test_panda_control.py b/tests/fastcs/panda/test_panda_control.py index b73d907f89..bbe06af5bf 100644 --- a/tests/fastcs/panda/test_panda_control.py +++ b/tests/fastcs/panda/test_panda_control.py @@ -5,6 +5,7 @@ import pytest from ophyd_async.core import DEFAULT_TIMEOUT, DetectorTrigger, Device, DeviceCollector +from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics.pvi import fill_pvi_entries from ophyd_async.epics.signal import epics_signal_rw from ophyd_async.fastcs.panda import CommonPandaBlocks, PandaPcapController @@ -36,18 +37,26 @@ class PcapBlock(Device): pandaController = PandaPcapController(pcap=PcapBlock()) with patch("ophyd_async.fastcs.panda._control.wait_for_value", return_value=None): with pytest.raises(AttributeError) as exc: - await pandaController.arm(num=1, trigger=DetectorTrigger.constant_gate) + await pandaController.prepare( + TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate) + ) + await pandaController.arm() assert ("'PcapBlock' object has no attribute 'arm'") in str(exc.value) async def test_panda_controller_arm_disarm(mock_panda): pandaController = PandaPcapController(mock_panda.pcap) with patch("ophyd_async.fastcs.panda._control.wait_for_value", return_value=None): - await pandaController.arm(num=1, trigger=DetectorTrigger.constant_gate) + await pandaController.prepare( + TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate) + ) + await pandaController.arm() await pandaController.disarm() async def test_panda_controller_wrong_trigger(): pandaController = PandaPcapController(None) with pytest.raises(AssertionError): - await pandaController.arm(num=1, trigger=DetectorTrigger.internal) + await pandaController.prepare( + TriggerInfo(number=1, trigger=DetectorTrigger.internal) + ) From 35cdddc4bcbb9b784e11647b3181f41aff6c231e Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 11 Sep 2024 13:44:53 +0100 Subject: [PATCH 04/12] update to pandapmac --- src/ophyd_async/fastcs/panda/_control.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ophyd_async/fastcs/panda/_control.py b/src/ophyd_async/fastcs/panda/_control.py index ed4b330c64..9357b7fc92 100644 --- a/src/ophyd_async/fastcs/panda/_control.py +++ b/src/ophyd_async/fastcs/panda/_control.py @@ -21,7 +21,7 @@ def get_deadtime(self, exposure: float) -> float: async def prepare(self, trigger_info: TriggerInfo): assert trigger_info.trigger in ( DetectorTrigger.constant_gate, - trigger_info.trigger == DetectorTrigger.variable_gate, + DetectorTrigger.variable_gate, ), "Only constant_gate and variable_gate triggering is supported on the PandA" await asyncio.gather(self.pcap.arm.set(True)) await wait_for_value(self.pcap.active, True, timeout=1) @@ -29,7 +29,6 @@ async def prepare(self, trigger_info: TriggerInfo): async def arm(self) -> AsyncStatus: return AsyncStatus(wait_for_value(self.pcap.active, False, timeout=None)) - async def disarm(self) -> AsyncStatus: + async def disarm(self): await asyncio.gather(self.pcap.arm.set(False)) await wait_for_value(self.pcap.active, False, timeout=1) - return AsyncStatus(wait_for_value(self.pcap.active, False, timeout=None)) From 97e8fd0fd93f5973db287144336600c04a4ae2ff Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 11 Sep 2024 13:48:26 +0100 Subject: [PATCH 05/12] comment not working test --- docs/examples/foo_detector.py | 18 +- tests/fastcs/panda/test_hdf_panda.py | 245 +++++++++++++-------------- 2 files changed, 129 insertions(+), 134 deletions(-) diff --git a/docs/examples/foo_detector.py b/docs/examples/foo_detector.py index c93f4c6e41..bf3e41e6f6 100644 --- a/docs/examples/foo_detector.py +++ b/docs/examples/foo_detector.py @@ -1,13 +1,14 @@ import asyncio +from typing import Optional from bluesky.protocols import HasHints, Hints from ophyd_async.core import ( AsyncStatus, DetectorControl, + DetectorTrigger, PathProvider, StandardDetector, - TriggerInfo, ) from ophyd_async.epics import adcore from ophyd_async.epics.signal import epics_signal_rw_rbv @@ -27,16 +28,19 @@ def get_deadtime(self, exposure: float) -> float: # FooDetector deadtime handling return 0.001 - async def prepare(self, trigger_info: TriggerInfo): + async def arm( + self, + num: int, + trigger: DetectorTrigger = DetectorTrigger.internal, + exposure: Optional[float] = None, + ) -> AsyncStatus: await asyncio.gather( - self._drv.num_images.set(trigger_info.number), + self._drv.num_images.set(num), self._drv.image_mode.set(adcore.ImageMode.multiple), - self._drv.trigger_mode.set(f"FOO{trigger_info}"), + self._drv.trigger_mode.set(f"FOO{trigger}"), ) - if exposure := trigger_info.livetime is not None: + if exposure is not None: await self._drv.acquire_time.set(exposure) - - async def arm(self) -> AsyncStatus: return await adcore.start_acquiring_driver_and_ensure_status(self._drv) async def disarm(self): diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index 43b969ab5e..b5365c60ad 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -1,19 +1,13 @@ import os -from typing import Dict -from unittest.mock import ANY import numpy as np import pytest -from bluesky import plan_stubs as bps -from bluesky.run_engine import RunEngine from ophyd_async.core import ( Device, SignalR, - StandardFlyer, StaticFilenameProvider, StaticPathProvider, - assert_emitted, callback_on_mock_put, set_mock_value, ) @@ -21,10 +15,6 @@ DatasetTable, HDFPanda, PandaHdf5DatasetType, - StaticSeqTableTriggerLogic, -) -from ophyd_async.plan_stubs import ( - prepare_static_seq_table_flyer_and_detectors_with_same_trigger, ) @@ -74,120 +64,121 @@ async def test_hdf_panda_passes_blocks_to_controller(mock_hdf_panda: HDFPanda): assert mock_hdf_panda.controller.pcap is mock_hdf_panda.pcap -async def test_hdf_panda_hardware_triggered_flyable( - RE: RunEngine, - mock_hdf_panda, - tmp_path, -): - docs = {} - - def append_and_print(name, doc): - if name not in docs: - docs[name] = [] - docs[name] += [doc] - - RE.subscribe(append_and_print) - - shutter_time = 0.004 - exposure = 1 - - trigger_logic = StaticSeqTableTriggerLogic(mock_hdf_panda.seq[1]) - flyer = StandardFlyer(trigger_logic, [], name="flyer") - - def flying_plan(): - yield from bps.stage_all(mock_hdf_panda, flyer) - - yield from prepare_static_seq_table_flyer_and_detectors_with_same_trigger( - flyer, - [mock_hdf_panda], - number_of_frames=1, - exposure=exposure, - shutter_time=shutter_time, - ) - - yield from bps.open_run() - yield from bps.declare_stream(mock_hdf_panda, name="main_stream", collect=True) - - set_mock_value(flyer.trigger_logic.seq.active, 1) - - yield from bps.kickoff(flyer, wait=True) - yield from bps.kickoff(mock_hdf_panda) - - yield from bps.complete(flyer, wait=False, group="complete") - yield from bps.complete(mock_hdf_panda, wait=False, group="complete") - - # Manually incremenet the index as if a frame was taken - set_mock_value(mock_hdf_panda.data.num_captured, 1) - set_mock_value(flyer.trigger_logic.seq.active, 0) - - done = False - while not done: - try: - yield from bps.wait(group="complete", timeout=0.5) - except TimeoutError: - pass - else: - done = True - yield from bps.collect( - mock_hdf_panda, - return_payload=False, - name="main_stream", - ) - yield from bps.wait(group="complete") - yield from bps.close_run() - - yield from bps.unstage_all(flyer, mock_hdf_panda) - yield from bps.wait_for([lambda: mock_hdf_panda.controller.disarm()]) - - # fly scan - RE(flying_plan()) - - assert_emitted( - docs, start=1, descriptor=1, stream_resource=2, stream_datum=2, stop=1 - ) - - # test descriptor - data_key_names: Dict[str, str] = docs["descriptor"][0]["object_keys"]["panda"] - assert data_key_names == ["x", "y"] - for data_key_name in data_key_names: - assert ( - docs["descriptor"][0]["data_keys"][data_key_name]["source"] - == "mock+soft://panda-data-hdf_directory" - ) - - # test stream resources - for dataset_name, stream_resource, data_key_name in zip( - ("x", "y"), docs["stream_resource"], data_key_names - ): - - def assert_resource_document(): - assert stream_resource == { - "run_start": docs["start"][0]["uid"], - "uid": ANY, - "data_key": data_key_name, - "mimetype": "application/x-hdf5", - "uri": "file://localhost" + str(tmp_path / "test-panda.h5"), - "parameters": { - "dataset": f"/{dataset_name}", - "swmr": False, - "multiplier": 1, - }, - } - assert "test-panda.h5" in stream_resource["uri"] - - assert_resource_document() - - # test stream datum - for stream_datum in docs["stream_datum"]: - assert stream_datum["descriptor"] == docs["descriptor"][0]["uid"] - assert stream_datum["seq_nums"] == { - "start": 1, - "stop": 2, - } - assert stream_datum["indices"] == { - "start": 0, - "stop": 1, - } - assert stream_datum["stream_resource"] in [ - sd["uid"].split("/")[0] for sd in docs["stream_datum"] - ] +# async def test_hdf_panda_hardware_triggered_flyable( +# RE: RunEngine, +# mock_hdf_panda, +# tmp_path, +# ): +# docs = {} + +# def append_and_print(name, doc): +# if name not in docs: +# docs[name] = [] +# docs[name] += [doc] + +# RE.subscribe(append_and_print) + +# shutter_time = 0.004 +# exposure = 1 + +# trigger_logic = StaticSeqTableTriggerLogic(mock_hdf_panda.seq[1]) +# flyer = StandardFlyer(trigger_logic, [], name="flyer") + +# def flying_plan(): +# yield from bps.stage_all(mock_hdf_panda, flyer) + +# yield from prepare_static_seq_table_flyer_and_detectors_with_same_trigger( +# flyer, +# [mock_hdf_panda], +# number_of_frames=1, +# exposure=exposure, +# shutter_time=shutter_time, +# ) + +# yield from bps.open_run() +# yield from bps.declare_stream(mock_hdf_panda, name="main_stream", +# collect=True) + +# set_mock_value(flyer.trigger_logic.seq.active, 1) + +# yield from bps.kickoff(flyer, wait=True) +# yield from bps.kickoff(mock_hdf_panda) + +# yield from bps.complete(flyer, wait=False, group="complete") +# yield from bps.complete(mock_hdf_panda, wait=False, group="complete") + +# # Manually incremenet the index as if a frame was taken +# set_mock_value(mock_hdf_panda.data.num_captured, 1) +# set_mock_value(flyer.trigger_logic.seq.active, 0) + +# done = False +# while not done: +# try: +# yield from bps.wait(group="complete", timeout=0.5) +# except TimeoutError: +# pass +# else: +# done = True +# yield from bps.collect( +# mock_hdf_panda, +# return_payload=False, +# name="main_stream", +# ) +# yield from bps.wait(group="complete") +# yield from bps.close_run() + +# yield from bps.unstage_all(flyer, mock_hdf_panda) +# yield from bps.wait_for([lambda: mock_hdf_panda.controller.disarm()]) + +# # fly scan +# RE(flying_plan()) + +# assert_emitted( +# docs, start=1, descriptor=1, stream_resource=2, stream_datum=2, stop=1 +# ) + +# # test descriptor +# data_key_names: Dict[str, str] = docs["descriptor"][0]["object_keys"]["panda"] +# assert data_key_names == ["x", "y"] +# for data_key_name in data_key_names: +# assert ( +# docs["descriptor"][0]["data_keys"][data_key_name]["source"] +# == "mock+soft://panda-data-hdf_directory" +# ) + +# # test stream resources +# for dataset_name, stream_resource, data_key_name in zip( +# ("x", "y"), docs["stream_resource"], data_key_names +# ): + +# def assert_resource_document(): +# assert stream_resource == { +# "run_start": docs["start"][0]["uid"], +# "uid": ANY, +# "data_key": data_key_name, +# "mimetype": "application/x-hdf5", +# "uri": "file://localhost" + str(tmp_path / "test-panda.h5"), +# "parameters": { +# "dataset": f"/{dataset_name}", +# "swmr": False, +# "multiplier": 1, +# }, +# } +# assert "test-panda.h5" in stream_resource["uri"] + +# assert_resource_document() + +# # test stream datum +# for stream_datum in docs["stream_datum"]: +# assert stream_datum["descriptor"] == docs["descriptor"][0]["uid"] +# assert stream_datum["seq_nums"] == { +# "start": 1, +# "stop": 2, +# } +# assert stream_datum["indices"] == { +# "start": 0, +# "stop": 1, +# } +# assert stream_datum["stream_resource"] in [ +# sd["uid"].split("/")[0] for sd in docs["stream_datum"] +# ] From 6d13ddde79742a85e6331238a9fd176ce8b4e79b Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 11 Sep 2024 14:03:03 +0000 Subject: [PATCH 06/12] docs changes --- src/ophyd_async/core/_detector.py | 25 ++- tests/fastcs/panda/test_hdf_panda.py | 245 ++++++++++++++------------- 2 files changed, 143 insertions(+), 127 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index 7567308d78..573668dafc 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -79,16 +79,23 @@ def get_deadtime(self, exposure: float | None) -> float: @abstractmethod async def prepare(self, trigger_info: TriggerInfo): - """, do all necessary steps to prepare detector for triggers. + """ + Do all necessary steps to prepare the detector for triggers. + Args: - num: Expected number of frames - trigger: Type of trigger for which to prepare the detector. Defaults to - DetectorTrigger.internal. - exposure: Exposure time with which to set up the detector. Defaults to None - if not applicable or the detector is expected to use its previously-set - exposure time. + trigger_info: This is a Pydantic model which contains + number Expected number of frames. + trigger Type of trigger for which to prepare the detector. Defaults + to DetectorTrigger.internal. + livetime Livetime / Exposure time with which to set up the detector. + Defaults to None + if not applicable or the detector is expected to use its previously-set + exposure time. + deadtime Defaults to None. This is the minimum deadtime between + triggers. + multiplier The number of triggers grouped into a single StreamDatum + index. """ - ... @abstractmethod async def arm(self) -> AsyncStatus: @@ -310,7 +317,6 @@ async def kickoff(self): @WatchableAsyncStatus.wrap async def complete(self): assert self._arm_status, "Prepare not run" - await self._arm_status assert self._trigger_info async for index in self.writer.observe_indices_written( self._trigger_info.frame_timeout @@ -331,6 +337,7 @@ async def complete(self): ) if index >= self._trigger_info.number: break + self._arm_status = None async def describe_collect(self) -> Dict[str, DataKey]: return self._describe diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index b5365c60ad..34fe42de85 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -1,7 +1,11 @@ import os +from typing import Dict +from unittest.mock import ANY +import bluesky.plan_stubs as bps import numpy as np import pytest +from bluesky import RunEngine from ophyd_async.core import ( Device, @@ -11,11 +15,17 @@ callback_on_mock_put, set_mock_value, ) +from ophyd_async.core._flyer import StandardFlyer +from ophyd_async.core._signal import assert_emitted from ophyd_async.fastcs.panda import ( DatasetTable, HDFPanda, PandaHdf5DatasetType, ) +from ophyd_async.fastcs.panda._trigger import StaticSeqTableTriggerLogic +from ophyd_async.plan_stubs._fly import ( + prepare_static_seq_table_flyer_and_detectors_with_same_trigger, +) @pytest.fixture @@ -64,121 +74,120 @@ async def test_hdf_panda_passes_blocks_to_controller(mock_hdf_panda: HDFPanda): assert mock_hdf_panda.controller.pcap is mock_hdf_panda.pcap -# async def test_hdf_panda_hardware_triggered_flyable( -# RE: RunEngine, -# mock_hdf_panda, -# tmp_path, -# ): -# docs = {} - -# def append_and_print(name, doc): -# if name not in docs: -# docs[name] = [] -# docs[name] += [doc] - -# RE.subscribe(append_and_print) - -# shutter_time = 0.004 -# exposure = 1 - -# trigger_logic = StaticSeqTableTriggerLogic(mock_hdf_panda.seq[1]) -# flyer = StandardFlyer(trigger_logic, [], name="flyer") - -# def flying_plan(): -# yield from bps.stage_all(mock_hdf_panda, flyer) - -# yield from prepare_static_seq_table_flyer_and_detectors_with_same_trigger( -# flyer, -# [mock_hdf_panda], -# number_of_frames=1, -# exposure=exposure, -# shutter_time=shutter_time, -# ) - -# yield from bps.open_run() -# yield from bps.declare_stream(mock_hdf_panda, name="main_stream", -# collect=True) - -# set_mock_value(flyer.trigger_logic.seq.active, 1) - -# yield from bps.kickoff(flyer, wait=True) -# yield from bps.kickoff(mock_hdf_panda) - -# yield from bps.complete(flyer, wait=False, group="complete") -# yield from bps.complete(mock_hdf_panda, wait=False, group="complete") - -# # Manually incremenet the index as if a frame was taken -# set_mock_value(mock_hdf_panda.data.num_captured, 1) -# set_mock_value(flyer.trigger_logic.seq.active, 0) - -# done = False -# while not done: -# try: -# yield from bps.wait(group="complete", timeout=0.5) -# except TimeoutError: -# pass -# else: -# done = True -# yield from bps.collect( -# mock_hdf_panda, -# return_payload=False, -# name="main_stream", -# ) -# yield from bps.wait(group="complete") -# yield from bps.close_run() - -# yield from bps.unstage_all(flyer, mock_hdf_panda) -# yield from bps.wait_for([lambda: mock_hdf_panda.controller.disarm()]) - -# # fly scan -# RE(flying_plan()) - -# assert_emitted( -# docs, start=1, descriptor=1, stream_resource=2, stream_datum=2, stop=1 -# ) - -# # test descriptor -# data_key_names: Dict[str, str] = docs["descriptor"][0]["object_keys"]["panda"] -# assert data_key_names == ["x", "y"] -# for data_key_name in data_key_names: -# assert ( -# docs["descriptor"][0]["data_keys"][data_key_name]["source"] -# == "mock+soft://panda-data-hdf_directory" -# ) - -# # test stream resources -# for dataset_name, stream_resource, data_key_name in zip( -# ("x", "y"), docs["stream_resource"], data_key_names -# ): - -# def assert_resource_document(): -# assert stream_resource == { -# "run_start": docs["start"][0]["uid"], -# "uid": ANY, -# "data_key": data_key_name, -# "mimetype": "application/x-hdf5", -# "uri": "file://localhost" + str(tmp_path / "test-panda.h5"), -# "parameters": { -# "dataset": f"/{dataset_name}", -# "swmr": False, -# "multiplier": 1, -# }, -# } -# assert "test-panda.h5" in stream_resource["uri"] - -# assert_resource_document() - -# # test stream datum -# for stream_datum in docs["stream_datum"]: -# assert stream_datum["descriptor"] == docs["descriptor"][0]["uid"] -# assert stream_datum["seq_nums"] == { -# "start": 1, -# "stop": 2, -# } -# assert stream_datum["indices"] == { -# "start": 0, -# "stop": 1, -# } -# assert stream_datum["stream_resource"] in [ -# sd["uid"].split("/")[0] for sd in docs["stream_datum"] -# ] +async def test_hdf_panda_hardware_triggered_flyable( + RE: RunEngine, + mock_hdf_panda, + tmp_path, +): + docs = {} + + def append_and_print(name, doc): + if name not in docs: + docs[name] = [] + docs[name] += [doc] + + RE.subscribe(append_and_print) + + shutter_time = 0.004 + exposure = 1 + + trigger_logic = StaticSeqTableTriggerLogic(mock_hdf_panda.seq[1]) + flyer = StandardFlyer(trigger_logic, [], name="flyer") + + def flying_plan(): + yield from bps.stage_all(mock_hdf_panda, flyer) + + yield from prepare_static_seq_table_flyer_and_detectors_with_same_trigger( + flyer, + [mock_hdf_panda], + number_of_frames=1, + exposure=exposure, + shutter_time=shutter_time, + ) + + yield from bps.open_run() + yield from bps.declare_stream(mock_hdf_panda, name="main_stream", collect=True) + + set_mock_value(flyer.trigger_logic.seq.active, 1) + + yield from bps.kickoff(flyer, wait=True) + yield from bps.kickoff(mock_hdf_panda) + + yield from bps.complete(flyer, wait=False, group="complete") + yield from bps.complete(mock_hdf_panda, wait=False, group="complete") + + # Manually incremenet the index as if a frame was taken + set_mock_value(mock_hdf_panda.data.num_captured, 1) + set_mock_value(flyer.trigger_logic.seq.active, 0) + + done = False + while not done: + try: + yield from bps.wait(group="complete", timeout=0.5) + except TimeoutError: + pass + else: + done = True + yield from bps.collect( + mock_hdf_panda, + return_payload=False, + name="main_stream", + ) + yield from bps.wait(group="complete") + yield from bps.close_run() + + yield from bps.unstage_all(flyer, mock_hdf_panda) + yield from bps.wait_for([lambda: mock_hdf_panda.controller.disarm()]) + + # fly scan + RE(flying_plan()) + + assert_emitted( + docs, start=1, descriptor=1, stream_resource=2, stream_datum=2, stop=1 + ) + + # test descriptor + data_key_names: Dict[str, str] = docs["descriptor"][0]["object_keys"]["panda"] + assert data_key_names == ["x", "y"] + for data_key_name in data_key_names: + assert ( + docs["descriptor"][0]["data_keys"][data_key_name]["source"] + == "mock+soft://panda-data-hdf_directory" + ) + + # test stream resources + for dataset_name, stream_resource, data_key_name in zip( + ("x", "y"), docs["stream_resource"], data_key_names + ): + + def assert_resource_document(): + assert stream_resource == { + "run_start": docs["start"][0]["uid"], + "uid": ANY, + "data_key": data_key_name, + "mimetype": "application/x-hdf5", + "uri": "file://localhost" + str(tmp_path / "test-panda.h5"), + "parameters": { + "dataset": f"/{dataset_name}", + "swmr": False, + "multiplier": 1, + }, + } + assert "test-panda.h5" in stream_resource["uri"] + + assert_resource_document() + + # test stream datum + for stream_datum in docs["stream_datum"]: + assert stream_datum["descriptor"] == docs["descriptor"][0]["uid"] + assert stream_datum["seq_nums"] == { + "start": 1, + "stop": 2, + } + assert stream_datum["indices"] == { + "start": 0, + "stop": 1, + } + assert stream_datum["stream_resource"] in [ + sd["uid"].split("/")[0] for sd in docs["stream_datum"] + ] From 98e07455d50e3ffe47852ec3b566c48cd1c074ab Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Wed, 11 Sep 2024 14:56:31 +0000 Subject: [PATCH 07/12] allowed zero for continuous mode --- src/ophyd_async/core/_detector.py | 2 +- tests/epics/adaravis/test_aravis.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index 573668dafc..951c5df9af 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -51,7 +51,7 @@ class TriggerInfo(BaseModel): """Minimal set of information required to setup triggering on a detector""" #: Number of triggers that will be sent, 0 means infinite - number: int = Field(gt=0) + number: int = Field(ge=0) #: Sort of triggers that will be sent trigger: DetectorTrigger = Field(default=DetectorTrigger.internal) #: What is the minimum deadtime between triggers diff --git a/tests/epics/adaravis/test_aravis.py b/tests/epics/adaravis/test_aravis.py index fb1ac89832..3c34fa49eb 100644 --- a/tests/epics/adaravis/test_aravis.py +++ b/tests/epics/adaravis/test_aravis.py @@ -166,7 +166,7 @@ async def test_unsupported_trigger_excepts(test_adaravis: adaravis.AravisDetecto ): await test_adaravis.prepare( TriggerInfo( - number=1, + number=0, trigger=DetectorTrigger.variable_gate, deadtime=1, livetime=1, From 0fbf6187684e46bd28f8b8c213f4d064f19cff99 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Thu, 12 Sep 2024 14:22:45 +0100 Subject: [PATCH 08/12] added wait for armed --- src/ophyd_async/core/_detector.py | 36 +++++++++++-------- .../epics/adaravis/_aravis_controller.py | 10 ++++-- .../epics/adkinetix/_kinetix_controller.py | 10 ++++-- .../epics/adpilatus/_pilatus_controller.py | 9 +++-- .../epics/adsimdetector/_sim_controller.py | 8 +++-- .../epics/advimba/_vimba_controller.py | 11 +++--- .../epics/eiger/_eiger_controller.py | 10 +++--- src/ophyd_async/fastcs/panda/_control.py | 10 ++++-- .../_pattern_detector_controller.py | 23 +++++++----- tests/epics/adcore/test_scans.py | 7 ++-- tests/epics/adkinetix/test_kinetix.py | 3 +- tests/epics/adpilatus/test_pilatus.py | 8 ++--- .../adpilatus/test_pilatus_controller.py | 4 +-- .../adsimdetector/test_adsim_controller.py | 3 +- tests/epics/advimba/test_vimba.py | 3 +- tests/epics/eiger/test_eiger_controller.py | 12 ++++--- tests/fastcs/panda/test_panda_control.py | 3 +- tests/plan_stubs/test_fly.py | 5 ++- 18 files changed, 108 insertions(+), 67 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index 951c5df9af..ac8f4775ce 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -65,6 +65,10 @@ class TriggerInfo(BaseModel): #: e.g. if num=10 and multiplier=5 then the detector will take 10 frames, #: but publish 2 indices, and describe() will show a shape of (5, h, w) multiplier: int = 1 + #: The number of times the detector can go through a complete cycle of kickoff and + #: complete without needing to re-arm. This is important for detectors where the + #: process of arming is expensive in terms of time + iteration: int = 1 class DetectorControl(ABC): @@ -98,14 +102,14 @@ async def prepare(self, trigger_info: TriggerInfo): """ @abstractmethod - async def arm(self) -> AsyncStatus: + def arm(self) -> None: + """ + Arm the detector """ - Arm detector - Returns: - AsyncStatus: Status representing the arm operation. This function returning - represents the start of the arm. The returned status completing means - the detector is now armed. + async def wait_for_armed(self): + """ + This will wait on the internal _arm_status and wait for it to get disarmed """ @abstractmethod @@ -193,7 +197,7 @@ def __init__( self._watchers: List[Callable] = [] self._fly_status: Optional[WatchableAsyncStatus] = None self._fly_start: float - + self._iterations_completed: int = 0 self._intial_frame: int self._last_frame: int super().__init__(name) @@ -262,8 +266,8 @@ async def trigger(self) -> None: assert self._trigger_info.trigger is DetectorTrigger.internal # Arm the detector and wait for it to finish. indices_written = await self.writer.get_indices_written() - written_status = await self.controller.arm() - await written_status + self.controller.arm() + await self.controller.wait_for_armed() end_observation = indices_written + 1 async for index in self.writer.observe_indices_written( @@ -304,19 +308,21 @@ async def prepare(self, value: TriggerInfo) -> None: self._initial_frame = await self.writer.get_indices_written() self._last_frame = self._initial_frame + self._trigger_info.number await self.controller.prepare(value) - if self._trigger_info.trigger is not DetectorTrigger.internal: - self._arm_status = await self.controller.arm() - self._fly_start = time.monotonic() self._describe = await self.writer.open(value.multiplier) @AsyncStatus.wrap async def kickoff(self): - if not self._arm_status: - raise Exception("Detector not armed!") + 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}") + if self._trigger_info.trigger is not DetectorTrigger.internal: + self.controller.arm() + await self.controller.wait_for_armed() + self._fly_start = time.monotonic() + self._iterations_completed += 1 @WatchableAsyncStatus.wrap async def complete(self): - assert self._arm_status, "Prepare not run" assert self._trigger_info async for index in self.writer.observe_indices_written( self._trigger_info.frame_timeout diff --git a/src/ophyd_async/epics/adaravis/_aravis_controller.py b/src/ophyd_async/epics/adaravis/_aravis_controller.py index 12fc962ba9..7736d29b16 100644 --- a/src/ophyd_async/epics/adaravis/_aravis_controller.py +++ b/src/ophyd_async/epics/adaravis/_aravis_controller.py @@ -2,7 +2,6 @@ from typing import Literal, Tuple from ophyd_async.core import ( - AsyncStatus, DetectorControl, DetectorTrigger, TriggerInfo, @@ -24,6 +23,7 @@ class AravisController(DetectorControl): def __init__(self, driver: AravisDriverIO, gpio_number: GPIO_NUMBER) -> None: self._drv = driver self.gpio_number = gpio_number + self._arm_status = None def get_deadtime(self, exposure: float) -> float: return _HIGHEST_POSSIBLE_DEADTIME @@ -46,8 +46,12 @@ async def prepare(self, trigger_info: TriggerInfo): self._drv.image_mode.set(image_mode), ) - async def arm(self) -> AsyncStatus: - return await set_and_wait_for_value(self._drv.acquire, True) + def arm(self): + self._arm_status = set_and_wait_for_value(self._drv.acquire, True) + + async def wait_for_armed(self): + if self._arm_status: + await self._arm_status def _get_trigger_info( self, trigger: DetectorTrigger diff --git a/src/ophyd_async/epics/adkinetix/_kinetix_controller.py b/src/ophyd_async/epics/adkinetix/_kinetix_controller.py index 21e339fc95..506d6377fe 100644 --- a/src/ophyd_async/epics/adkinetix/_kinetix_controller.py +++ b/src/ophyd_async/epics/adkinetix/_kinetix_controller.py @@ -1,6 +1,6 @@ import asyncio -from ophyd_async.core import AsyncStatus, DetectorControl, DetectorTrigger +from ophyd_async.core import DetectorControl, DetectorTrigger from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adcore @@ -36,8 +36,12 @@ async def prepare(self, trigger_info: TriggerInfo): ]: await self._drv.acquire_time.set(trigger_info.livetime) - async def arm(self) -> AsyncStatus: - return await adcore.start_acquiring_driver_and_ensure_status(self._drv) + def arm(self): + self._arm_status = adcore.start_acquiring_driver_and_ensure_status(self._drv) + + async def wait_for_armed(self): + if self._arm_status: + await self._arm_status async def disarm(self): await adcore.stop_busy_record(self._drv.acquire, False, timeout=1) diff --git a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py index 65548e91b2..f48a6de458 100644 --- a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py +++ b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py @@ -2,7 +2,6 @@ from ophyd_async.core import ( DEFAULT_TIMEOUT, - AsyncStatus, DetectorControl, DetectorTrigger, wait_for_value, @@ -44,10 +43,12 @@ async def prepare(self, trigger_info: TriggerInfo): self._drv.image_mode.set(adcore.ImageMode.multiple), ) - async def arm(self) -> AsyncStatus: + def arm(self): # Standard arm the detector and wait for the acquire PV to be True - idle_status = await adcore.start_acquiring_driver_and_ensure_status(self._drv) + self._arm_status = adcore.start_acquiring_driver_and_ensure_status(self._drv) + async def wait_for_armed(self): + await self._arm_status # The pilatus has an additional PV that goes True when the camserver # is actually ready. Should wait for that too or we risk dropping # a frame @@ -57,8 +58,6 @@ async def arm(self) -> AsyncStatus: timeout=DEFAULT_TIMEOUT, ) - return idle_status - @classmethod def _get_trigger_mode(cls, trigger: DetectorTrigger) -> PilatusTriggerMode: if trigger not in cls._supported_trigger_types.keys(): diff --git a/src/ophyd_async/epics/adsimdetector/_sim_controller.py b/src/ophyd_async/epics/adsimdetector/_sim_controller.py index cc39f54f3f..c9be91ecac 100644 --- a/src/ophyd_async/epics/adsimdetector/_sim_controller.py +++ b/src/ophyd_async/epics/adsimdetector/_sim_controller.py @@ -3,7 +3,6 @@ from ophyd_async.core import ( DEFAULT_TIMEOUT, - AsyncStatus, DetectorControl, DetectorTrigger, ) @@ -36,11 +35,14 @@ async def prepare(self, trigger_info: TriggerInfo): self.driver.image_mode.set(adcore.ImageMode.multiple), ) - async def arm(self) -> AsyncStatus: - return await adcore.start_acquiring_driver_and_ensure_status( + def arm(self): + self._arm_status = adcore.start_acquiring_driver_and_ensure_status( self.driver, good_states=self.good_states, timeout=self.frame_timeout ) + async def wait_for_armed(self): + await self._arm_status + async def disarm(self): # We can't use caput callback as we already used it in arm() and we can't have # 2 or they will deadlock diff --git a/src/ophyd_async/epics/advimba/_vimba_controller.py b/src/ophyd_async/epics/advimba/_vimba_controller.py index ec0996dfdb..02b379731d 100644 --- a/src/ophyd_async/epics/advimba/_vimba_controller.py +++ b/src/ophyd_async/epics/advimba/_vimba_controller.py @@ -1,6 +1,6 @@ import asyncio -from ophyd_async.core import AsyncStatus, DetectorControl, DetectorTrigger +from ophyd_async.core import DetectorControl, DetectorTrigger from ophyd_async.core._detector import TriggerInfo from ophyd_async.epics import adcore @@ -48,10 +48,11 @@ async def prepare(self, trigger_info: TriggerInfo): else: self._drv.trigger_source.set(VimbaTriggerSource.freerun) - async def arm( - self, - ) -> AsyncStatus: - return await adcore.start_acquiring_driver_and_ensure_status(self._drv) + def arm(self): + self._arm_status = adcore.start_acquiring_driver_and_ensure_status(self._drv) + + async def wait_for_armed(self): + await self._arm_status async def disarm(self): await adcore.stop_busy_record(self._drv.acquire, False, timeout=1) diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py index a23c937834..72183e1994 100644 --- a/src/ophyd_async/epics/eiger/_eiger_controller.py +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -2,7 +2,6 @@ from ophyd_async.core import ( DEFAULT_TIMEOUT, - AsyncStatus, DetectorControl, DetectorTrigger, set_and_wait_for_other_value, @@ -53,14 +52,17 @@ async def prepare(self, trigger_info: TriggerInfo): ) await asyncio.gather(*coros) - @AsyncStatus.wrap - async def arm( + def arm( self, ): # TODO: Detector state should be an enum see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 - await set_and_wait_for_other_value( + self._arm_status = set_and_wait_for_other_value( self._drv.arm, 1, self._drv.state, "ready", timeout=DEFAULT_TIMEOUT ) + async def wait_for_armed(self): + if self._arm_status: + await self._arm_status + async def disarm(self): await self._drv.disarm.set(1) diff --git a/src/ophyd_async/fastcs/panda/_control.py b/src/ophyd_async/fastcs/panda/_control.py index 9357b7fc92..65b5e4a7c1 100644 --- a/src/ophyd_async/fastcs/panda/_control.py +++ b/src/ophyd_async/fastcs/panda/_control.py @@ -26,8 +26,14 @@ async def prepare(self, trigger_info: TriggerInfo): await asyncio.gather(self.pcap.arm.set(True)) await wait_for_value(self.pcap.active, True, timeout=1) - async def arm(self) -> AsyncStatus: - return AsyncStatus(wait_for_value(self.pcap.active, False, timeout=None)) + def arm(self): + self._arm_status = AsyncStatus( + wait_for_value(self.pcap.active, False, timeout=None) + ) + + async def wait_for_armed(self): + if self._arm_status: + await self._arm_status async def disarm(self): await asyncio.gather(self.pcap.arm.set(False)) diff --git a/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py b/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py index d739712ffa..db195131b9 100644 --- a/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py +++ b/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py @@ -3,7 +3,7 @@ from pydantic import Field -from ophyd_async.core import AsyncStatus, DetectorControl, PathProvider +from ophyd_async.core import DetectorControl, PathProvider from ophyd_async.core._detector import TriggerInfo from ._pattern_generator import PatternGenerator @@ -25,18 +25,25 @@ def __init__( async def prepare( self, trigger_info: TriggerInfo = TriggerInfo(number=1, livetime=0.01) ): - if trigger_info.livetime is None: - trigger_info.livetime = 0.01 - period: float = trigger_info.livetime + self.get_deadtime(trigger_info.livetime) + self._trigger_info = trigger_info + if self._trigger_info.livetime is None: + self._trigger_info.livetime = 0.01 + self.period: float = self._trigger_info.livetime + self.get_deadtime( + trigger_info.livetime + ) + + def arm(self): + assert self._trigger_info.livetime + assert self.period self.task = asyncio.create_task( self._coroutine_for_image_writing( - trigger_info.livetime, period, trigger_info.number + self._trigger_info.livetime, self.period, self._trigger_info.number ) ) - async def arm(self) -> AsyncStatus: - assert self.task - return AsyncStatus(self.task) + async def wait_for_armed(self): + if self.task: + await self.task async def disarm(self): if self.task: diff --git a/tests/epics/adcore/test_scans.py b/tests/epics/adcore/test_scans.py index 31c0166841..3025bc0181 100644 --- a/tests/epics/adcore/test_scans.py +++ b/tests/epics/adcore/test_scans.py @@ -40,8 +40,11 @@ def __init__(self) -> None: ... async def prepare(self, trigger_info: TriggerInfo): return AsyncStatus(asyncio.sleep(0.01)) - async def arm(self) -> AsyncStatus: - return AsyncStatus(asyncio.sleep(0.01)) + def arm(self): + self._arm_status = AsyncStatus(asyncio.sleep(0.01)) + + async def wait_for_armed(self): + await self._arm_status async def disarm(self): ... diff --git a/tests/epics/adkinetix/test_kinetix.py b/tests/epics/adkinetix/test_kinetix.py index d2ff03e433..b9d72cc304 100644 --- a/tests/epics/adkinetix/test_kinetix.py +++ b/tests/epics/adkinetix/test_kinetix.py @@ -36,7 +36,8 @@ async def setup_trigger_mode(trig_mode: DetectorTrigger): await test_adkinetix.controller.prepare( TriggerInfo(number=1, trigger=trig_mode) ) - await test_adkinetix.controller.arm() + test_adkinetix.controller.arm() + await test_adkinetix.controller.wait_for_armed() # Prevent timeouts set_mock_value(test_adkinetix.drv.acquire, True) diff --git a/tests/epics/adpilatus/test_pilatus.py b/tests/epics/adpilatus/test_pilatus.py index d0917a850b..1d305c0515 100644 --- a/tests/epics/adpilatus/test_pilatus.py +++ b/tests/epics/adpilatus/test_pilatus.py @@ -64,8 +64,8 @@ async def trigger_and_complete(): await test_adpilatus.controller.prepare( TriggerInfo(number=1, trigger=detector_trigger) ) - status = await test_adpilatus.controller.arm() - await status + test_adpilatus.controller.arm() + await test_adpilatus.controller.wait_for_armed() await _trigger(test_adpilatus, expected_trigger_mode, trigger_and_complete) @@ -77,8 +77,8 @@ async def trigger_and_complete(): await test_adpilatus.controller.prepare( TriggerInfo(number=1, trigger=DetectorTrigger.internal) ) - status = await test_adpilatus.controller.arm() - await status + test_adpilatus.controller.arm() + await test_adpilatus.controller.wait_for_armed() with patch( "ophyd_async.epics.adpilatus._pilatus_controller.DEFAULT_TIMEOUT", diff --git a/tests/epics/adpilatus/test_pilatus_controller.py b/tests/epics/adpilatus/test_pilatus_controller.py index 6ead5b2bcb..88bc1d6620 100644 --- a/tests/epics/adpilatus/test_pilatus_controller.py +++ b/tests/epics/adpilatus/test_pilatus_controller.py @@ -30,8 +30,8 @@ async def test_pilatus_controller( ): set_mock_value(pilatus_driver.armed, True) await pilatus.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate)) - status = await pilatus.arm() - await status + pilatus.arm() + await pilatus.wait_for_armed() assert await pilatus_driver.num_images.get_value() == 1 assert await pilatus_driver.image_mode.get_value() == adcore.ImageMode.multiple diff --git a/tests/epics/adsimdetector/test_adsim_controller.py b/tests/epics/adsimdetector/test_adsim_controller.py index 07326ae54d..5c3ea50b88 100644 --- a/tests/epics/adsimdetector/test_adsim_controller.py +++ b/tests/epics/adsimdetector/test_adsim_controller.py @@ -19,7 +19,8 @@ async def ad(RE) -> adsimdetector.SimController: async def test_ad_controller(RE, ad: adsimdetector.SimController): with patch("ophyd_async.core._signal.wait_for_value", return_value=None): await ad.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.internal)) - await ad.arm() + ad.arm() + await ad.wait_for_armed() driver = ad.driver assert await driver.num_images.get_value() == 1 diff --git a/tests/epics/advimba/test_vimba.py b/tests/epics/advimba/test_vimba.py index 31bba268ba..6270e71480 100644 --- a/tests/epics/advimba/test_vimba.py +++ b/tests/epics/advimba/test_vimba.py @@ -36,7 +36,8 @@ async def test_arming_trig_modes(test_advimba: advimba.VimbaDetector): async def setup_trigger_mode(trig_mode: DetectorTrigger): await test_advimba.controller.prepare(TriggerInfo(number=1, trigger=trig_mode)) - await test_advimba.controller.arm() + test_advimba.controller.arm() + await test_advimba.controller.wait_for_armed() # Prevent timeouts set_mock_value(test_advimba.drv.acquire, True) diff --git a/tests/epics/eiger/test_eiger_controller.py b/tests/epics/eiger/test_eiger_controller.py index 01e76a9653..93f9829732 100644 --- a/tests/epics/eiger/test_eiger_controller.py +++ b/tests/epics/eiger/test_eiger_controller.py @@ -45,7 +45,8 @@ async def test_when_arm_with_exposure_then_time_and_period_set( driver, controller = eiger_driver_and_controller test_exposure = 0.002 await controller.prepare(TriggerInfo(number=10, livetime=test_exposure)) - await controller.arm() + controller.arm() + await controller.wait_for_armed() assert (await driver.acquire_period.get_value()) == test_exposure assert (await driver.acquire_time.get_value()) == test_exposure @@ -55,7 +56,8 @@ async def test_when_arm_with_no_exposure_then_arm_set_correctly( ): driver, controller = eiger_driver_and_controller await controller.prepare(TriggerInfo(number=10)) - await controller.arm() + controller.arm() + await controller.wait_for_armed() get_mock_put(driver.arm).assert_called_once_with(1, wait=ANY, timeout=ANY) @@ -65,7 +67,8 @@ async def test_when_arm_with_number_of_images_then_number_of_images_set_correctl driver, controller = eiger_driver_and_controller test_number_of_images = 40 await controller.prepare(TriggerInfo(number=test_number_of_images)) - await controller.arm() + controller.arm() + await controller.wait_for_armed() get_mock_put(driver.num_images).assert_called_once_with( test_number_of_images, wait=ANY, timeout=ANY ) @@ -78,7 +81,8 @@ async def test_given_detector_fails_to_go_ready_when_arm_called_then_fails( driver, controller = eiger_driver_and_controller_no_arm with raises(TimeoutError): await controller.prepare(TriggerInfo(number=10)) - await controller.arm() + controller.arm() + await controller.wait_for_armed() async def test_when_disarm_called_on_controller_then_disarm_called_on_driver( diff --git a/tests/fastcs/panda/test_panda_control.py b/tests/fastcs/panda/test_panda_control.py index bbe06af5bf..7024986112 100644 --- a/tests/fastcs/panda/test_panda_control.py +++ b/tests/fastcs/panda/test_panda_control.py @@ -50,7 +50,8 @@ async def test_panda_controller_arm_disarm(mock_panda): await pandaController.prepare( TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate) ) - await pandaController.arm() + pandaController.arm() + await pandaController.wait_for_armed() await pandaController.disarm() diff --git a/tests/plan_stubs/test_fly.py b/tests/plan_stubs/test_fly.py index 791935c111..06e40e29db 100644 --- a/tests/plan_stubs/test_fly.py +++ b/tests/plan_stubs/test_fly.py @@ -115,7 +115,6 @@ def __init__( @WatchableAsyncStatus.wrap async def complete(self): - assert self._arm_status, "Prepare not run" assert self._trigger_info self.writer.increment_index() async for index in self.writer.observe_indices_written( @@ -145,10 +144,10 @@ async def detectors(RE: RunEngine) -> tuple[MockDetector, MockDetector]: await writers[0].dummy_signal.connect(mock=True) await writers[1].dummy_signal.connect(mock=True) - async def dummy_arm_1(self=None, trigger=None, num=0, exposure=None): + def dummy_arm_1(self=None): return writers[0].dummy_signal.set(1) - async def dummy_arm_2(self=None, trigger=None, num=0, exposure=None): + def dummy_arm_2(self=None): return writers[1].dummy_signal.set(1) detector_1 = MockDetector( From 85583e8bbb27ac0e7f55cf4c4421d2249b82b14d Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Thu, 12 Sep 2024 16:06:26 +0100 Subject: [PATCH 09/12] made code review changes --- src/ophyd_async/core/_detector.py | 22 ++-- .../epics/adaravis/_aravis_controller.py | 9 +- .../epics/adkinetix/_kinetix_controller.py | 10 +- .../epics/adpilatus/_pilatus_controller.py | 13 +- .../epics/adsimdetector/_sim_controller.py | 11 +- .../epics/advimba/_vimba_controller.py | 13 +- .../epics/eiger/_eiger_controller.py | 4 +- src/ophyd_async/fastcs/panda/_control.py | 13 +- src/ophyd_async/plan_stubs/_fly.py | 2 + .../_pattern_detector_controller.py | 4 +- tests/core/test_flyer.py | 2 +- tests/epics/adcore/test_scans.py | 4 +- tests/epics/adkinetix/test_kinetix.py | 4 +- tests/epics/adpilatus/test_pilatus.py | 8 +- .../adpilatus/test_pilatus_controller.py | 4 +- .../adsimdetector/test_adsim_controller.py | 4 +- tests/epics/advimba/test_vimba.py | 4 +- tests/epics/eiger/test_eiger_controller.py | 16 +-- tests/fastcs/panda/test_hdf_panda.py | 122 +++++++++++++++++- tests/fastcs/panda/test_panda_control.py | 4 +- 20 files changed, 204 insertions(+), 69 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index ac8f4775ce..2141ce0dd7 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -102,12 +102,12 @@ async def prepare(self, trigger_info: TriggerInfo): """ @abstractmethod - def arm(self) -> None: + async def arm(self) -> None: """ Arm the detector """ - async def wait_for_armed(self): + async def wait_for_idle(self): """ This will wait on the internal _arm_status and wait for it to get disarmed """ @@ -266,8 +266,8 @@ async def trigger(self) -> None: assert self._trigger_info.trigger is DetectorTrigger.internal # Arm the detector and wait for it to finish. indices_written = await self.writer.get_indices_written() - self.controller.arm() - await self.controller.wait_for_armed() + await self.controller.arm() + await self.controller.wait_for_idle() end_observation = indices_written + 1 async for index in self.writer.observe_indices_written( @@ -294,31 +294,30 @@ async def prepare(self, value: TriggerInfo) -> None: Args: value: TriggerInfo describing how to trigger the detector """ - self._trigger_info = value if value.trigger != DetectorTrigger.internal: assert ( value.deadtime ), "Deadtime must be supplied when in externally triggered mode" if value.deadtime: - required = self.controller.get_deadtime(self._trigger_info.livetime) + required = self.controller.get_deadtime(value.livetime) assert required <= value.deadtime, ( f"Detector {self.controller} needs at least {required}s deadtime, " f"but trigger logic provides only {value.deadtime}s" ) + self._trigger_info = value self._initial_frame = await self.writer.get_indices_written() self._last_frame = self._initial_frame + self._trigger_info.number await self.controller.prepare(value) self._describe = await self.writer.open(value.multiplier) + if value.trigger != DetectorTrigger.internal: + await self.controller.arm() + self._fly_start = time.monotonic() @AsyncStatus.wrap 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}") - if self._trigger_info.trigger is not DetectorTrigger.internal: - self.controller.arm() - await self.controller.wait_for_armed() - self._fly_start = time.monotonic() self._iterations_completed += 1 @WatchableAsyncStatus.wrap @@ -343,7 +342,8 @@ async def complete(self): ) if index >= self._trigger_info.number: break - self._arm_status = None + if self._iterations_completed == self._trigger_info.iteration: + await self.controller.wait_for_idle() async def describe_collect(self) -> Dict[str, DataKey]: return self._describe diff --git a/src/ophyd_async/epics/adaravis/_aravis_controller.py b/src/ophyd_async/epics/adaravis/_aravis_controller.py index 7736d29b16..894a46c008 100644 --- a/src/ophyd_async/epics/adaravis/_aravis_controller.py +++ b/src/ophyd_async/epics/adaravis/_aravis_controller.py @@ -7,6 +7,7 @@ TriggerInfo, set_and_wait_for_value, ) +from ophyd_async.core._status import AsyncStatus from ophyd_async.epics import adcore from ._aravis_io import AravisDriverIO, AravisTriggerMode, AravisTriggerSource @@ -23,7 +24,7 @@ class AravisController(DetectorControl): def __init__(self, driver: AravisDriverIO, gpio_number: GPIO_NUMBER) -> None: self._drv = driver self.gpio_number = gpio_number - self._arm_status = None + self._arm_status: AsyncStatus | None = None def get_deadtime(self, exposure: float) -> float: return _HIGHEST_POSSIBLE_DEADTIME @@ -46,10 +47,10 @@ async def prepare(self, trigger_info: TriggerInfo): self._drv.image_mode.set(image_mode), ) - def arm(self): - self._arm_status = set_and_wait_for_value(self._drv.acquire, True) + async def arm(self): + self._arm_status = await set_and_wait_for_value(self._drv.acquire, True) - async def wait_for_armed(self): + async def wait_for_idle(self): if self._arm_status: await self._arm_status diff --git a/src/ophyd_async/epics/adkinetix/_kinetix_controller.py b/src/ophyd_async/epics/adkinetix/_kinetix_controller.py index 506d6377fe..70d32e6a78 100644 --- a/src/ophyd_async/epics/adkinetix/_kinetix_controller.py +++ b/src/ophyd_async/epics/adkinetix/_kinetix_controller.py @@ -2,6 +2,7 @@ from ophyd_async.core import DetectorControl, DetectorTrigger from ophyd_async.core._detector import TriggerInfo +from ophyd_async.core._status import AsyncStatus from ophyd_async.epics import adcore from ._kinetix_io import KinetixDriverIO, KinetixTriggerMode @@ -20,6 +21,7 @@ def __init__( driver: KinetixDriverIO, ) -> None: self._drv = driver + self._arm_status: AsyncStatus | None = None def get_deadtime(self, exposure: float) -> float: return 0.001 @@ -36,10 +38,12 @@ async def prepare(self, trigger_info: TriggerInfo): ]: await self._drv.acquire_time.set(trigger_info.livetime) - def arm(self): - self._arm_status = adcore.start_acquiring_driver_and_ensure_status(self._drv) + async def arm(self): + self._arm_status = await adcore.start_acquiring_driver_and_ensure_status( + self._drv + ) - async def wait_for_armed(self): + async def wait_for_idle(self): if self._arm_status: await self._arm_status diff --git a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py index f48a6de458..b5e8400f83 100644 --- a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py +++ b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py @@ -7,6 +7,7 @@ wait_for_value, ) from ophyd_async.core._detector import TriggerInfo +from ophyd_async.core._status import AsyncStatus from ophyd_async.epics import adcore from ._pilatus_io import PilatusDriverIO, PilatusTriggerMode @@ -26,6 +27,7 @@ def __init__( ) -> None: self._drv = driver self._readout_time = readout_time + self._arm_status: AsyncStatus | None = None def get_deadtime(self, exposure: float) -> float: return self._readout_time @@ -43,12 +45,15 @@ async def prepare(self, trigger_info: TriggerInfo): self._drv.image_mode.set(adcore.ImageMode.multiple), ) - def arm(self): + async def arm(self): # Standard arm the detector and wait for the acquire PV to be True - self._arm_status = adcore.start_acquiring_driver_and_ensure_status(self._drv) + self._arm_status = await adcore.start_acquiring_driver_and_ensure_status( + self._drv + ) - async def wait_for_armed(self): - await self._arm_status + async def wait_for_idle(self): + if self._arm_status: + await self._arm_status # The pilatus has an additional PV that goes True when the camserver # is actually ready. Should wait for that too or we risk dropping # a frame diff --git a/src/ophyd_async/epics/adsimdetector/_sim_controller.py b/src/ophyd_async/epics/adsimdetector/_sim_controller.py index c9be91ecac..6561ee24f1 100644 --- a/src/ophyd_async/epics/adsimdetector/_sim_controller.py +++ b/src/ophyd_async/epics/adsimdetector/_sim_controller.py @@ -7,6 +7,7 @@ DetectorTrigger, ) from ophyd_async.core._detector import TriggerInfo +from ophyd_async.core._status import AsyncStatus from ophyd_async.epics import adcore @@ -19,6 +20,7 @@ def __init__( self.driver = driver self.good_states = good_states self.frame_timeout: float + self._arm_status: AsyncStatus | None = None def get_deadtime(self, exposure: float) -> float: return 0.002 @@ -35,13 +37,14 @@ async def prepare(self, trigger_info: TriggerInfo): self.driver.image_mode.set(adcore.ImageMode.multiple), ) - def arm(self): - self._arm_status = adcore.start_acquiring_driver_and_ensure_status( + async def arm(self): + self._arm_status = await adcore.start_acquiring_driver_and_ensure_status( self.driver, good_states=self.good_states, timeout=self.frame_timeout ) - async def wait_for_armed(self): - await self._arm_status + async def wait_for_idle(self): + if self._arm_status: + await self._arm_status async def disarm(self): # We can't use caput callback as we already used it in arm() and we can't have diff --git a/src/ophyd_async/epics/advimba/_vimba_controller.py b/src/ophyd_async/epics/advimba/_vimba_controller.py index 02b379731d..9b87d37872 100644 --- a/src/ophyd_async/epics/advimba/_vimba_controller.py +++ b/src/ophyd_async/epics/advimba/_vimba_controller.py @@ -2,6 +2,7 @@ from ophyd_async.core import DetectorControl, DetectorTrigger from ophyd_async.core._detector import TriggerInfo +from ophyd_async.core._status import AsyncStatus from ophyd_async.epics import adcore from ._vimba_io import VimbaDriverIO, VimbaExposeOutMode, VimbaOnOff, VimbaTriggerSource @@ -27,6 +28,7 @@ def __init__( driver: VimbaDriverIO, ) -> None: self._drv = driver + self._arm_status: AsyncStatus | None = None def get_deadtime(self, exposure: float) -> float: return 0.001 @@ -48,11 +50,14 @@ async def prepare(self, trigger_info: TriggerInfo): else: self._drv.trigger_source.set(VimbaTriggerSource.freerun) - def arm(self): - self._arm_status = adcore.start_acquiring_driver_and_ensure_status(self._drv) + async def arm(self): + self._arm_status = await adcore.start_acquiring_driver_and_ensure_status( + self._drv + ) - async def wait_for_armed(self): - await self._arm_status + async def wait_for_idle(self): + if self._arm_status: + await self._arm_status async def disarm(self): await adcore.stop_busy_record(self._drv.acquire, False, timeout=1) diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py index 72183e1994..7a69da97ca 100644 --- a/src/ophyd_async/epics/eiger/_eiger_controller.py +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -52,7 +52,7 @@ async def prepare(self, trigger_info: TriggerInfo): ) await asyncio.gather(*coros) - def arm( + async def arm( self, ): # TODO: Detector state should be an enum see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 @@ -60,7 +60,7 @@ def arm( self._drv.arm, 1, self._drv.state, "ready", timeout=DEFAULT_TIMEOUT ) - async def wait_for_armed(self): + async def wait_for_idle(self): if self._arm_status: await self._arm_status diff --git a/src/ophyd_async/fastcs/panda/_control.py b/src/ophyd_async/fastcs/panda/_control.py index 65b5e4a7c1..5f801a748d 100644 --- a/src/ophyd_async/fastcs/panda/_control.py +++ b/src/ophyd_async/fastcs/panda/_control.py @@ -1,7 +1,6 @@ import asyncio from ophyd_async.core import ( - AsyncStatus, DetectorControl, DetectorTrigger, wait_for_value, @@ -23,17 +22,13 @@ async def prepare(self, trigger_info: TriggerInfo): DetectorTrigger.constant_gate, DetectorTrigger.variable_gate, ), "Only constant_gate and variable_gate triggering is supported on the PandA" + + async def arm(self): await asyncio.gather(self.pcap.arm.set(True)) await wait_for_value(self.pcap.active, True, timeout=1) - def arm(self): - self._arm_status = AsyncStatus( - wait_for_value(self.pcap.active, False, timeout=None) - ) - - async def wait_for_armed(self): - if self._arm_status: - await self._arm_status + async def wait_for_idle(self): + pass async def disarm(self): await asyncio.gather(self.pcap.arm.set(False)) diff --git a/src/ophyd_async/plan_stubs/_fly.py b/src/ophyd_async/plan_stubs/_fly.py index 087ec62dd1..31a4d06218 100644 --- a/src/ophyd_async/plan_stubs/_fly.py +++ b/src/ophyd_async/plan_stubs/_fly.py @@ -48,6 +48,7 @@ def prepare_static_seq_table_flyer_and_detectors_with_same_trigger( repeats: int = 1, period: float = 0.0, frame_timeout: Optional[float] = None, + iteration: int = 1, ): """Prepare a hardware triggered flyable and one or more detectors. @@ -70,6 +71,7 @@ def prepare_static_seq_table_flyer_and_detectors_with_same_trigger( deadtime=deadtime, livetime=exposure, frame_timeout=frame_timeout, + iteration=iteration, ) trigger_time = number_of_frames * (exposure + deadtime) pre_delay = max(period - 2 * shutter_time - trigger_time, 0) diff --git a/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py b/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py index db195131b9..039ddb066c 100644 --- a/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py +++ b/src/ophyd_async/sim/demo/_pattern_detector/_pattern_detector_controller.py @@ -32,7 +32,7 @@ async def prepare( trigger_info.livetime ) - def arm(self): + async def arm(self): assert self._trigger_info.livetime assert self.period self.task = asyncio.create_task( @@ -41,7 +41,7 @@ def arm(self): ) ) - async def wait_for_armed(self): + async def wait_for_idle(self): if self.task: await self.task diff --git a/tests/core/test_flyer.py b/tests/core/test_flyer.py index 9d968526d9..6d9c9142aa 100644 --- a/tests/core/test_flyer.py +++ b/tests/core/test_flyer.py @@ -117,7 +117,7 @@ async def detectors(RE: RunEngine) -> tuple[StandardDetector, StandardDetector]: await writers[0].dummy_signal.connect(mock=True) await writers[1].dummy_signal.connect(mock=True) - async def dummy_arm_1(self=None, trigger=None, num=0, exposure=None): + def dummy_arm_1(self=None, trigger=None, num=0, exposure=None): return writers[0].dummy_signal.set(1) async def dummy_arm_2(self=None, trigger=None, num=0, exposure=None): diff --git a/tests/epics/adcore/test_scans.py b/tests/epics/adcore/test_scans.py index 3025bc0181..6889fbbe3b 100644 --- a/tests/epics/adcore/test_scans.py +++ b/tests/epics/adcore/test_scans.py @@ -40,10 +40,10 @@ def __init__(self) -> None: ... async def prepare(self, trigger_info: TriggerInfo): return AsyncStatus(asyncio.sleep(0.01)) - def arm(self): + async def arm(self): self._arm_status = AsyncStatus(asyncio.sleep(0.01)) - async def wait_for_armed(self): + async def wait_for_idle(self): await self._arm_status async def disarm(self): ... diff --git a/tests/epics/adkinetix/test_kinetix.py b/tests/epics/adkinetix/test_kinetix.py index b9d72cc304..a17be5e5b3 100644 --- a/tests/epics/adkinetix/test_kinetix.py +++ b/tests/epics/adkinetix/test_kinetix.py @@ -36,8 +36,8 @@ async def setup_trigger_mode(trig_mode: DetectorTrigger): await test_adkinetix.controller.prepare( TriggerInfo(number=1, trigger=trig_mode) ) - test_adkinetix.controller.arm() - await test_adkinetix.controller.wait_for_armed() + await test_adkinetix.controller.arm() + await test_adkinetix.controller.wait_for_idle() # Prevent timeouts set_mock_value(test_adkinetix.drv.acquire, True) diff --git a/tests/epics/adpilatus/test_pilatus.py b/tests/epics/adpilatus/test_pilatus.py index 1d305c0515..ee6302234b 100644 --- a/tests/epics/adpilatus/test_pilatus.py +++ b/tests/epics/adpilatus/test_pilatus.py @@ -64,8 +64,8 @@ async def trigger_and_complete(): await test_adpilatus.controller.prepare( TriggerInfo(number=1, trigger=detector_trigger) ) - test_adpilatus.controller.arm() - await test_adpilatus.controller.wait_for_armed() + await test_adpilatus.controller.arm() + await test_adpilatus.controller.wait_for_idle() await _trigger(test_adpilatus, expected_trigger_mode, trigger_and_complete) @@ -77,8 +77,8 @@ async def trigger_and_complete(): await test_adpilatus.controller.prepare( TriggerInfo(number=1, trigger=DetectorTrigger.internal) ) - test_adpilatus.controller.arm() - await test_adpilatus.controller.wait_for_armed() + await test_adpilatus.controller.arm() + await test_adpilatus.controller.wait_for_idle() with patch( "ophyd_async.epics.adpilatus._pilatus_controller.DEFAULT_TIMEOUT", diff --git a/tests/epics/adpilatus/test_pilatus_controller.py b/tests/epics/adpilatus/test_pilatus_controller.py index 88bc1d6620..bb825b8744 100644 --- a/tests/epics/adpilatus/test_pilatus_controller.py +++ b/tests/epics/adpilatus/test_pilatus_controller.py @@ -30,8 +30,8 @@ async def test_pilatus_controller( ): set_mock_value(pilatus_driver.armed, True) await pilatus.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate)) - pilatus.arm() - await pilatus.wait_for_armed() + await pilatus.arm() + await pilatus.wait_for_idle() assert await pilatus_driver.num_images.get_value() == 1 assert await pilatus_driver.image_mode.get_value() == adcore.ImageMode.multiple diff --git a/tests/epics/adsimdetector/test_adsim_controller.py b/tests/epics/adsimdetector/test_adsim_controller.py index 5c3ea50b88..64d53ce590 100644 --- a/tests/epics/adsimdetector/test_adsim_controller.py +++ b/tests/epics/adsimdetector/test_adsim_controller.py @@ -19,8 +19,8 @@ async def ad(RE) -> adsimdetector.SimController: async def test_ad_controller(RE, ad: adsimdetector.SimController): with patch("ophyd_async.core._signal.wait_for_value", return_value=None): await ad.prepare(TriggerInfo(number=1, trigger=DetectorTrigger.internal)) - ad.arm() - await ad.wait_for_armed() + await ad.arm() + await ad.wait_for_idle() driver = ad.driver assert await driver.num_images.get_value() == 1 diff --git a/tests/epics/advimba/test_vimba.py b/tests/epics/advimba/test_vimba.py index 6270e71480..ec93cc07d3 100644 --- a/tests/epics/advimba/test_vimba.py +++ b/tests/epics/advimba/test_vimba.py @@ -36,8 +36,8 @@ async def test_arming_trig_modes(test_advimba: advimba.VimbaDetector): async def setup_trigger_mode(trig_mode: DetectorTrigger): await test_advimba.controller.prepare(TriggerInfo(number=1, trigger=trig_mode)) - test_advimba.controller.arm() - await test_advimba.controller.wait_for_armed() + await test_advimba.controller.arm() + await test_advimba.controller.wait_for_idle() # Prevent timeouts set_mock_value(test_advimba.drv.acquire, True) diff --git a/tests/epics/eiger/test_eiger_controller.py b/tests/epics/eiger/test_eiger_controller.py index 93f9829732..39204142e5 100644 --- a/tests/epics/eiger/test_eiger_controller.py +++ b/tests/epics/eiger/test_eiger_controller.py @@ -45,8 +45,8 @@ async def test_when_arm_with_exposure_then_time_and_period_set( driver, controller = eiger_driver_and_controller test_exposure = 0.002 await controller.prepare(TriggerInfo(number=10, livetime=test_exposure)) - controller.arm() - await controller.wait_for_armed() + await controller.arm() + await controller.wait_for_idle() assert (await driver.acquire_period.get_value()) == test_exposure assert (await driver.acquire_time.get_value()) == test_exposure @@ -56,8 +56,8 @@ async def test_when_arm_with_no_exposure_then_arm_set_correctly( ): driver, controller = eiger_driver_and_controller await controller.prepare(TriggerInfo(number=10)) - controller.arm() - await controller.wait_for_armed() + await controller.arm() + await controller.wait_for_idle() get_mock_put(driver.arm).assert_called_once_with(1, wait=ANY, timeout=ANY) @@ -67,8 +67,8 @@ async def test_when_arm_with_number_of_images_then_number_of_images_set_correctl driver, controller = eiger_driver_and_controller test_number_of_images = 40 await controller.prepare(TriggerInfo(number=test_number_of_images)) - controller.arm() - await controller.wait_for_armed() + await controller.arm() + await controller.wait_for_idle() get_mock_put(driver.num_images).assert_called_once_with( test_number_of_images, wait=ANY, timeout=ANY ) @@ -81,8 +81,8 @@ async def test_given_detector_fails_to_go_ready_when_arm_called_then_fails( driver, controller = eiger_driver_and_controller_no_arm with raises(TimeoutError): await controller.prepare(TriggerInfo(number=10)) - controller.arm() - await controller.wait_for_armed() + await controller.arm() + await controller.wait_for_idle() async def test_when_disarm_called_on_controller_then_disarm_called_on_driver( diff --git a/tests/fastcs/panda/test_hdf_panda.py b/tests/fastcs/panda/test_hdf_panda.py index 34fe42de85..ab5acd5c20 100644 --- a/tests/fastcs/panda/test_hdf_panda.py +++ b/tests/fastcs/panda/test_hdf_panda.py @@ -111,7 +111,7 @@ def flying_plan(): set_mock_value(flyer.trigger_logic.seq.active, 1) yield from bps.kickoff(flyer, wait=True) - yield from bps.kickoff(mock_hdf_panda) + yield from bps.kickoff(mock_hdf_panda, wait=True) yield from bps.complete(flyer, wait=False, group="complete") yield from bps.complete(mock_hdf_panda, wait=False, group="complete") @@ -191,3 +191,123 @@ def assert_resource_document(): assert stream_datum["stream_resource"] in [ sd["uid"].split("/")[0] for sd in docs["stream_datum"] ] + + +async def test_hdf_panda_hardware_triggered_flyable_with_iterations( + RE: RunEngine, + mock_hdf_panda, + tmp_path, +): + docs = {} + + def append_and_print(name, doc): + if name not in docs: + docs[name] = [] + docs[name] += [doc] + + RE.subscribe(append_and_print) + + shutter_time = 0.004 + exposure = 1 + + trigger_logic = StaticSeqTableTriggerLogic(mock_hdf_panda.seq[1]) + flyer = StandardFlyer(trigger_logic, [], name="flyer") + + def flying_plan(): + iteration = 2 + yield from bps.stage_all(mock_hdf_panda, flyer) + yield from bps.open_run() + yield from prepare_static_seq_table_flyer_and_detectors_with_same_trigger( # noqa: E501 + flyer, + [mock_hdf_panda], + number_of_frames=1, + exposure=exposure, + shutter_time=shutter_time, + iteration=iteration, + ) + + yield from bps.declare_stream(mock_hdf_panda, name="main_stream", collect=True) + + for i in range(iteration): + set_mock_value(flyer.trigger_logic.seq.active, 1) + yield from bps.kickoff(flyer, wait=True) + yield from bps.kickoff(mock_hdf_panda) + + yield from bps.complete(flyer, wait=False, group="complete") + yield from bps.complete(mock_hdf_panda, wait=False, group="complete") + + # Manually incremenet the index as if a frame was taken + set_mock_value(mock_hdf_panda.data.num_captured, 1) + set_mock_value(flyer.trigger_logic.seq.active, 0) + + done = False + while not done: + try: + yield from bps.wait(group="complete", timeout=0.5) + except TimeoutError: + pass + else: + done = True + yield from bps.collect( + mock_hdf_panda, + return_payload=False, + name="main_stream", + ) + yield from bps.wait(group="complete") + yield from bps.close_run() + + yield from bps.unstage_all(flyer, mock_hdf_panda) + yield from bps.wait_for([lambda: mock_hdf_panda.controller.disarm()]) + + # fly scan + RE(flying_plan()) + + assert_emitted( + docs, start=1, descriptor=1, stream_resource=2, stream_datum=2, stop=1 + ) + + # test descriptor + data_key_names: Dict[str, str] = docs["descriptor"][0]["object_keys"]["panda"] + assert data_key_names == ["x", "y"] + for data_key_name in data_key_names: + assert ( + docs["descriptor"][0]["data_keys"][data_key_name]["source"] + == "mock+soft://panda-data-hdf_directory" + ) + + # test stream resources + for dataset_name, stream_resource, data_key_name in zip( + ("x", "y"), docs["stream_resource"], data_key_names + ): + + def assert_resource_document(): + assert stream_resource == { + "run_start": docs["start"][0]["uid"], + "uid": ANY, + "data_key": data_key_name, + "mimetype": "application/x-hdf5", + "uri": "file://localhost" + str(tmp_path / "test-panda.h5"), + "parameters": { + "dataset": f"/{dataset_name}", + "swmr": False, + "multiplier": 1, + }, + } + assert "test-panda.h5" in stream_resource["uri"] + + assert_resource_document() + + # test stream datum + for stream_datum in docs["stream_datum"]: + assert stream_datum["descriptor"] == docs["descriptor"][0]["uid"] + assert stream_datum["seq_nums"] == { + "start": 1, + "stop": 2, + } + assert stream_datum["indices"] == { + "start": 0, + "stop": 1, + } + assert stream_datum["stream_resource"] in [ + sd["uid"].split("/")[0] for sd in docs["stream_datum"] + ] diff --git a/tests/fastcs/panda/test_panda_control.py b/tests/fastcs/panda/test_panda_control.py index 7024986112..6c0abf298f 100644 --- a/tests/fastcs/panda/test_panda_control.py +++ b/tests/fastcs/panda/test_panda_control.py @@ -50,8 +50,8 @@ async def test_panda_controller_arm_disarm(mock_panda): await pandaController.prepare( TriggerInfo(number=1, trigger=DetectorTrigger.constant_gate) ) - pandaController.arm() - await pandaController.wait_for_armed() + await pandaController.arm() + await pandaController.wait_for_idle() await pandaController.disarm() From 03a3d4af444f212224c0ce16212ef8b481878e99 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Fri, 13 Sep 2024 09:07:42 +0000 Subject: [PATCH 10/12] added code review changes --- src/ophyd_async/core/_detector.py | 8 +++++--- src/ophyd_async/epics/adpilatus/_pilatus_controller.py | 8 ++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/ophyd_async/core/_detector.py b/src/ophyd_async/core/_detector.py index 2141ce0dd7..45d48fa43f 100644 --- a/src/ophyd_async/core/_detector.py +++ b/src/ophyd_async/core/_detector.py @@ -107,9 +107,10 @@ async def arm(self) -> None: Arm the detector """ + @abstractmethod async def wait_for_idle(self): """ - This will wait on the internal _arm_status and wait for it to get disarmed + This will wait on the internal _arm_status and wait for it to get disarmed/idle """ @abstractmethod @@ -307,8 +308,9 @@ async def prepare(self, value: TriggerInfo) -> None: self._trigger_info = value self._initial_frame = await self.writer.get_indices_written() self._last_frame = self._initial_frame + self._trigger_info.number - await self.controller.prepare(value) - self._describe = await self.writer.open(value.multiplier) + self._describe, _ = await asyncio.gather( + self.writer.open(value.multiplier), self.controller.prepare(value) + ) if value.trigger != DetectorTrigger.internal: await self.controller.arm() self._fly_start = time.monotonic() diff --git a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py index b5e8400f83..54e0d41d5d 100644 --- a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py +++ b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py @@ -50,10 +50,6 @@ async def arm(self): self._arm_status = await adcore.start_acquiring_driver_and_ensure_status( self._drv ) - - async def wait_for_idle(self): - if self._arm_status: - await self._arm_status # The pilatus has an additional PV that goes True when the camserver # is actually ready. Should wait for that too or we risk dropping # a frame @@ -63,6 +59,10 @@ async def wait_for_idle(self): timeout=DEFAULT_TIMEOUT, ) + async def wait_for_idle(self): + if self._arm_status: + await self._arm_status + @classmethod def _get_trigger_mode(cls, trigger: DetectorTrigger) -> PilatusTriggerMode: if trigger not in cls._supported_trigger_types.keys(): From 338f946769e65408bc2dc7a6430fee7548a43df0 Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Fri, 13 Sep 2024 09:36:51 +0000 Subject: [PATCH 11/12] added gather to controllers --- .../epics/adpilatus/_pilatus_controller.py | 27 ++++++++++--------- .../epics/eiger/_eiger_controller.py | 4 +-- src/ophyd_async/fastcs/panda/_control.py | 7 +++-- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py index 54e0d41d5d..f4f79c78ec 100644 --- a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py +++ b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py @@ -27,7 +27,7 @@ def __init__( ) -> None: self._drv = driver self._readout_time = readout_time - self._arm_status: AsyncStatus | None = None + self._arm_status: tuple[AsyncStatus, None] def get_deadtime(self, exposure: float) -> float: return self._readout_time @@ -47,21 +47,22 @@ async def prepare(self, trigger_info: TriggerInfo): async def arm(self): # Standard arm the detector and wait for the acquire PV to be True - self._arm_status = await adcore.start_acquiring_driver_and_ensure_status( - self._drv - ) - # The pilatus has an additional PV that goes True when the camserver - # is actually ready. Should wait for that too or we risk dropping - # a frame - await wait_for_value( - self._drv.armed, - True, - timeout=DEFAULT_TIMEOUT, + self._arm_status = await asyncio.gather( + adcore.start_acquiring_driver_and_ensure_status(self._drv), + # The pilatus has an additional PV that goes True when the camserver + # is actually ready. Should wait for that too or we risk dropping + # a frame + wait_for_value( + self._drv.armed, + True, + timeout=DEFAULT_TIMEOUT, + ), ) async def wait_for_idle(self): - if self._arm_status: - await self._arm_status + for status in self._arm_status: + if status: + await status @classmethod def _get_trigger_mode(cls, trigger: DetectorTrigger) -> PilatusTriggerMode: diff --git a/src/ophyd_async/epics/eiger/_eiger_controller.py b/src/ophyd_async/epics/eiger/_eiger_controller.py index 7a69da97ca..c7542bc741 100644 --- a/src/ophyd_async/epics/eiger/_eiger_controller.py +++ b/src/ophyd_async/epics/eiger/_eiger_controller.py @@ -52,9 +52,7 @@ async def prepare(self, trigger_info: TriggerInfo): ) await asyncio.gather(*coros) - async def arm( - self, - ): + async def arm(self): # TODO: Detector state should be an enum see https://github.com/DiamondLightSource/eiger-fastcs/issues/43 self._arm_status = set_and_wait_for_other_value( self._drv.arm, 1, self._drv.state, "ready", timeout=DEFAULT_TIMEOUT diff --git a/src/ophyd_async/fastcs/panda/_control.py b/src/ophyd_async/fastcs/panda/_control.py index 5f801a748d..d0de65ead8 100644 --- a/src/ophyd_async/fastcs/panda/_control.py +++ b/src/ophyd_async/fastcs/panda/_control.py @@ -6,6 +6,7 @@ wait_for_value, ) from ophyd_async.core._detector import TriggerInfo +from ophyd_async.core._status import AsyncStatus from ._block import PcapBlock @@ -13,6 +14,7 @@ class PandaPcapController(DetectorControl): def __init__(self, pcap: PcapBlock) -> None: self.pcap = pcap + self._arm_status: tuple[AsyncStatus, None] def get_deadtime(self, exposure: float) -> float: return 0.000000008 @@ -24,8 +26,9 @@ async def prepare(self, trigger_info: TriggerInfo): ), "Only constant_gate and variable_gate triggering is supported on the PandA" async def arm(self): - await asyncio.gather(self.pcap.arm.set(True)) - await wait_for_value(self.pcap.active, True, timeout=1) + self._arm_status = await asyncio.gather( + self.pcap.arm.set(True), wait_for_value(self.pcap.active, True, timeout=1) + ) async def wait_for_idle(self): pass From c261a80edb35635f81a0b90f928435972af6b78e Mon Sep 17 00:00:00 2001 From: Zoheb Shaikh Date: Fri, 13 Sep 2024 10:13:22 +0000 Subject: [PATCH 12/12] removed asyncio.gather --- .../epics/adpilatus/_pilatus_controller.py | 27 +++++++++---------- src/ophyd_async/fastcs/panda/_control.py | 7 +++-- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py index f4f79c78ec..54e0d41d5d 100644 --- a/src/ophyd_async/epics/adpilatus/_pilatus_controller.py +++ b/src/ophyd_async/epics/adpilatus/_pilatus_controller.py @@ -27,7 +27,7 @@ def __init__( ) -> None: self._drv = driver self._readout_time = readout_time - self._arm_status: tuple[AsyncStatus, None] + self._arm_status: AsyncStatus | None = None def get_deadtime(self, exposure: float) -> float: return self._readout_time @@ -47,22 +47,21 @@ async def prepare(self, trigger_info: TriggerInfo): async def arm(self): # Standard arm the detector and wait for the acquire PV to be True - self._arm_status = await asyncio.gather( - adcore.start_acquiring_driver_and_ensure_status(self._drv), - # The pilatus has an additional PV that goes True when the camserver - # is actually ready. Should wait for that too or we risk dropping - # a frame - wait_for_value( - self._drv.armed, - True, - timeout=DEFAULT_TIMEOUT, - ), + self._arm_status = await adcore.start_acquiring_driver_and_ensure_status( + self._drv + ) + # The pilatus has an additional PV that goes True when the camserver + # is actually ready. Should wait for that too or we risk dropping + # a frame + await wait_for_value( + self._drv.armed, + True, + timeout=DEFAULT_TIMEOUT, ) async def wait_for_idle(self): - for status in self._arm_status: - if status: - await status + if self._arm_status: + await self._arm_status @classmethod def _get_trigger_mode(cls, trigger: DetectorTrigger) -> PilatusTriggerMode: diff --git a/src/ophyd_async/fastcs/panda/_control.py b/src/ophyd_async/fastcs/panda/_control.py index d0de65ead8..aeb8e750cd 100644 --- a/src/ophyd_async/fastcs/panda/_control.py +++ b/src/ophyd_async/fastcs/panda/_control.py @@ -14,7 +14,7 @@ class PandaPcapController(DetectorControl): def __init__(self, pcap: PcapBlock) -> None: self.pcap = pcap - self._arm_status: tuple[AsyncStatus, None] + self._arm_status: AsyncStatus | None = None def get_deadtime(self, exposure: float) -> float: return 0.000000008 @@ -26,9 +26,8 @@ async def prepare(self, trigger_info: TriggerInfo): ), "Only constant_gate and variable_gate triggering is supported on the PandA" async def arm(self): - self._arm_status = await asyncio.gather( - self.pcap.arm.set(True), wait_for_value(self.pcap.active, True, timeout=1) - ) + self._arm_status = self.pcap.arm.set(True) + await wait_for_value(self.pcap.active, True, timeout=1) async def wait_for_idle(self): pass