Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial commit #560

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 35 additions & 20 deletions src/ophyd_async/core/_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 0
Comment on lines +68 to +71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iterations or max_iterations may be slightly clearer?

can go through or will?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field also appears to not be used yet?



class DetectorControl(ABC):
Expand All @@ -73,32 +77,36 @@ class DetectorControl(ABC):
arming and disarming a detector
"""

_arm_status = AsyncStatus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_arm_status = AsyncStatus
_arm_status: AsyncStatus | None

This does require that all subclasses of DetectorControl have this field, and that it is this type. As arm is abstract, and we're leaving implementation to the implementations I'd avoid having any required fields like this.


@abstractmethod
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:
def prepare(self, trigger_info: TriggerInfo) -> None:
"""
Arm detector, do all necessary steps to prepare detector for triggers.

Prepare the detector with the trigger Info
Args:
num: Expected number of frames
TriggerInfo which contains
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update to the new prepare params

number: 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
DetectorTrigger.internal
deadtime: 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.
"""

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.
@abstractmethod
async def arm(self):
"""
Arm the detector and save the status of arm in _arm_status
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Arm the detector and save the status of arm in _arm_status
Arm the detector

"""

@abstractmethod
async def wait_for_disarm():
"""
This will wait on the internal _arm_status and wait for it to get disarmed
"""

@abstractmethod
Expand Down Expand Up @@ -248,8 +256,13 @@ async def trigger(self) -> None:
trigger=DetectorTrigger.internal,
deadtime=None,
livetime=None,
frame_timeout=None,
)
)
assert self._trigger_info, "Trigger Info cannot be none after prepare"
assert (
self._trigger_info.trigger == DetectorTrigger.internal
), "It can only be triggered for internal trigger"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
), "It can only be triggered for internal trigger"
), "Cannot internally trigger with {self._trigger_info.trigger}"

# Arm the detector and wait for it to finish.
indices_written = await self.writer.get_indices_written()
written_status = await self.controller.arm(
Expand Down Expand Up @@ -283,6 +296,7 @@ async def prepare(self, value: TriggerInfo) -> None:
Args:
value: TriggerInfo describing how to trigger the detector
"""
self.controller.prepare(value)
self._trigger_info = value
if value.trigger != DetectorTrigger.internal:
assert (
Expand All @@ -296,11 +310,12 @@ 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,
)
if value.trigger != DetectorTrigger.internal:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if value.trigger != DetectorTrigger.internal:
if value.trigger is not DetectorTrigger.internal:

Should use is for enums

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()
self._describe = await self.writer.open(value.multiplier)

Expand Down
Loading