bluesky / ophyd-async

Hardware abstraction for bluesky written using asyncio
https://blueskyproject.io/ophyd-async
BSD 3-Clause "New" or "Revised" License
7 stars 21 forks source link

Rename `DetectorControl.arm` to make the function of the returned status clearer #357

Closed callumforrester closed 1 week ago

callumforrester commented 1 month ago
          Suggestion from @coretl rename the `Controller.arm` method to something clearer e.g. `arm_and_provide_complete_status`

Originally posted by @callumforrester in https://github.com/bluesky/ophyd-async/issues/354#issuecomment-2144758388

As a developer I would like to have a clearer understanding of what should and shouldn't happen when I call arm on a detector vs. synchronise on the status it returns.

Currently the DetectorControl.arm method performs some setup and then returns a status object for when acquisition is complete:

controller: MySpecialDetectorController

idle_status = await controller.arm(1)  # This blocks until the detector is armed
await idle_status  # This blocks until the detector has finished acquisition, disarmed and returned to an idle state

This is useful, but not very clear, especially when implementing your own detector controller

    async def arm(
        self,
        num: int,
        trigger: DetectorTrigger = DetectorTrigger.internal,
        exposure: Optional[float] = None,
    ) -> AsyncStatus:
        # Arm detector, this must be done (awaited) before returning
        await self._drv.arm.set_value(True)
        await wait_for_value(self._drv.armed, True)

        # Wait until detector is disarmed again, this must not be awaited,
        # but returned in a status so the user can await themselves if desired
        return AsyncStatus(wait_for_value(self._drv.armed, False))

From discussion with @coretl, we should give the method a longer, more explicit name to make its complexity clearer.

Possible Solution

Name suggestions (more welcome):

  1. arm_and_provide_acquisition_status
  2. arm_and_provide_idle_status
  3. arm_and_await_completion

Acceptance Criteria

  1. Name agreed
  2. Method renamed for all ophyd-async detectors
  3. Tests pass
coretl commented 1 week ago

Decided to split into arm and wait_for_disarmed in #405