Skip to content

Commit

Permalink
made code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
ZohebShaikh committed Sep 12, 2024
1 parent 0fbf618 commit 85583e8
Show file tree
Hide file tree
Showing 20 changed files with 204 additions and 69 deletions.
22 changes: 11 additions & 11 deletions src/ophyd_async/core/_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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
Expand Down
9 changes: 5 additions & 4 deletions src/ophyd_async/epics/adaravis/_aravis_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
10 changes: 7 additions & 3 deletions src/ophyd_async/epics/adkinetix/_kinetix_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
13 changes: 9 additions & 4 deletions src/ophyd_async/epics/adpilatus/_pilatus_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
11 changes: 7 additions & 4 deletions src/ophyd_async/epics/adsimdetector/_sim_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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
Expand All @@ -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
Expand Down
13 changes: 9 additions & 4 deletions src/ophyd_async/epics/advimba/_vimba_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
4 changes: 2 additions & 2 deletions src/ophyd_async/epics/eiger/_eiger_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ 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
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):
async def wait_for_idle(self):
if self._arm_status:
await self._arm_status

Expand Down
13 changes: 4 additions & 9 deletions src/ophyd_async/fastcs/panda/_control.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import asyncio

from ophyd_async.core import (
AsyncStatus,
DetectorControl,
DetectorTrigger,
wait_for_value,
Expand All @@ -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))
Expand Down
2 changes: 2 additions & 0 deletions src/ophyd_async/plan_stubs/_fly.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion tests/core/test_flyer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions tests/epics/adcore/test_scans.py
Original file line number Diff line number Diff line change
Expand Up @@ -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): ...
Expand Down
4 changes: 2 additions & 2 deletions tests/epics/adkinetix/test_kinetix.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions tests/epics/adpilatus/test_pilatus.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions tests/epics/adpilatus/test_pilatus_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 85583e8

Please sign in to comment.