-
Notifications
You must be signed in to change notification settings - Fork 12
Concept of checkbeam device #1975
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
Draft
oliwenmandiamond
wants to merge
1
commit into
main
Choose a base branch
from
Concept_of_pause_plan_device
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import asyncio | ||
| import contextlib | ||
| from collections.abc import Awaitable, Callable | ||
| from typing import Any | ||
|
|
||
| from bluesky.protocols import Readable, Stageable | ||
| from ophyd_async.core import ( | ||
| AsyncStatus, | ||
| Device, | ||
| SignalR, | ||
| observe_value, | ||
| ) | ||
|
|
||
|
|
||
| class PausePlanDevice(Device, Stageable, Readable): | ||
| def __init__( | ||
| self, | ||
| signals_to_condition: dict[SignalR[Any], Callable[[Any], bool]], | ||
| callable_when_paused: Callable[[], Awaitable[None]] | None = None, | ||
| callable_on_resume: Callable[[], Awaitable[None]] | None = None, | ||
| seconds_to_wait_before_resume: float = 5, | ||
| name: str = "", | ||
| ): | ||
| self._signals_to_condition = signals_to_condition | ||
| self._callable_when_paused = callable_when_paused | ||
| self._callable_on_resume = callable_on_resume | ||
| self._seconds_to_wait_before_resume = seconds_to_wait_before_resume | ||
| super().__init__(name) | ||
|
|
||
| async def _pause(self): | ||
| """Pause until all signal conditions are met, calling hooks as needed.""" | ||
| # Check if we actually need to pause | ||
| values = await asyncio.gather( | ||
| *(sig.get_value() for sig in self._signals_to_condition) | ||
| ) | ||
| all_met = all( | ||
| pred(value) | ||
| for value, (_, pred) in zip( | ||
| values, self._signals_to_condition.items(), strict=True | ||
| ) | ||
| ) | ||
| if all_met: | ||
| return # no need to pause | ||
|
|
||
| # Call pause hook | ||
| if self._callable_when_paused: | ||
| await self._callable_when_paused() | ||
|
|
||
| latest = {} | ||
| event = asyncio.Event() | ||
|
|
||
| async def watch(signal, predicate): | ||
| async for value in observe_value(signal): | ||
| latest[signal] = predicate(value) | ||
| if len(latest) == len(self._signals_to_condition) and all( | ||
| latest.values() | ||
| ): | ||
| event.set() | ||
| return | ||
|
|
||
| tasks = [ | ||
| asyncio.create_task(watch(sig, pred)) | ||
| for sig, pred in self._signals_to_condition.items() | ||
| ] | ||
|
|
||
| await event.wait() | ||
|
|
||
| # Cancel watchers | ||
| for task in tasks: | ||
| task.cancel() | ||
| with contextlib.suppress(asyncio.CancelledError): | ||
| await task | ||
|
|
||
| await asyncio.sleep(self._seconds_to_wait_before_resume) | ||
| # Call resume hook | ||
| if self._callable_on_resume: | ||
| await self._callable_on_resume() | ||
|
|
||
| @AsyncStatus.wrap | ||
| async def stage(self): | ||
| await self._pause() | ||
|
|
||
| @AsyncStatus.wrap | ||
| async def unstage(self): | ||
| pass | ||
|
|
||
| async def read(self): | ||
| await self._pause() | ||
| return {} | ||
|
|
||
| async def describe(self): | ||
| return {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import asyncio | ||
|
|
||
| import pytest | ||
| from bluesky import RunEngine | ||
| from bluesky import plan_stubs as bps | ||
| from ophyd_async.core import InOut, SignalRW, init_devices, soft_signal_rw | ||
|
|
||
| from dodal.devices.fast_shutter import GenericFastShutter | ||
| from dodal.devices.pause_plan_device import PausePlanDevice | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def shutter() -> GenericFastShutter: | ||
| with init_devices(mock=True): | ||
| shutter = GenericFastShutter( | ||
| "TEST:", open_state=InOut.OUT, close_state=InOut.IN | ||
| ) | ||
| return shutter | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sig1() -> SignalRW[int]: | ||
| with init_devices(mock=True): | ||
| sig1 = soft_signal_rw(int, initial_value=0) | ||
| return sig1 | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sig2() -> SignalRW[int]: | ||
| with init_devices(mock=True): | ||
| sig2 = soft_signal_rw(int, initial_value=0) | ||
| return sig2 | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| async def pause_plan_device( | ||
| sig1: SignalRW[float], | ||
| sig2: SignalRW[float], | ||
| shutter: GenericFastShutter, | ||
| ) -> PausePlanDevice: | ||
|
|
||
| async def _close_shutter(): | ||
| await shutter.set(shutter.close_state) | ||
|
|
||
| async def _open_shutter(): | ||
| await shutter.set(shutter.open_state) | ||
|
|
||
| with init_devices(mock=True): | ||
| pause_plan_device = PausePlanDevice( | ||
| { | ||
| sig1: lambda v: v == 1, | ||
| sig2: lambda v: v > 5, | ||
| }, | ||
| callable_when_paused=_close_shutter, | ||
| callable_on_resume=_open_shutter, | ||
| seconds_to_wait_before_resume=0, | ||
| ) | ||
| return pause_plan_device | ||
|
|
||
|
|
||
| async def test_conditions_can_arrive_in_any_order( | ||
| pause_plan_device: PausePlanDevice, | ||
| sig1: SignalRW[float], | ||
| sig2: SignalRW[float], | ||
| shutter: GenericFastShutter, | ||
| ): | ||
| await shutter.set(shutter.open_state) | ||
|
|
||
| status = pause_plan_device.stage() | ||
|
|
||
| await asyncio.sleep(0.1) | ||
|
|
||
| assert not status.done | ||
| assert await shutter.shutter_state.get_value() == shutter.close_state | ||
|
|
||
| await sig1.set(1) | ||
| await sig2.set(10) | ||
|
|
||
| await status | ||
| assert status.success | ||
| assert await shutter.shutter_state.get_value() == shutter.open_state | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_conditions_already_met( | ||
| pause_plan_device: PausePlanDevice, sig1: SignalRW[float], sig2: SignalRW[float] | ||
| ): | ||
| await sig1.set(1) | ||
| await sig2.set(10) | ||
|
|
||
| status = pause_plan_device.stage() | ||
|
|
||
| await status | ||
|
|
||
| assert status.success | ||
|
|
||
|
|
||
| async def test_pause_device_blocks_plan_until_conditions_met( | ||
| run_engine: RunEngine, | ||
| pause_plan_device: PausePlanDevice, | ||
| sig1: SignalRW[float], | ||
| sig2: SignalRW[float], | ||
| ): | ||
|
|
||
| start = asyncio.Event() | ||
|
|
||
| async def update_signals(): | ||
| await start.wait() | ||
| await sig1.set(1) | ||
| await sig2.set(6) | ||
|
|
||
| asyncio.create_task(update_signals()) | ||
|
|
||
| def plan(): | ||
| start.set() | ||
| yield from bps.stage(pause_plan_device) | ||
| yield from bps.null() | ||
|
|
||
| run_engine(plan()) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this checkbeam is missing front end shutter checking. Is this done somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just a proof of concept so not everything is added yet
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, I've made it flexible enough so that you can add any custom logic. So you would just need to inject the front end shutter here, add check to see if closed is signals_to_condions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear though, I agree with @DominicOram and I don't think a device is the way to go about this. This was just a proof of concept on how we could possibly translate the GDA code to bluesky with this being a possibility. If we make it a device and it "pauses" a scan, it won't pause the RunEngine and therefore BlueAPI will not be able to tell the scan is paused either. It will probably look like the plan is hanging as it is blocking and it may not be obvious why to the user. Using the Suspenders will give BlueAPI a lot more natural context and feedback as to what is happening with the plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with checkbeam for data collection in user mode, I would ensure it is built into RE directly as Suspender for any plans to be run as default as users data collection will always need these. However for developers or beamline staffs who will work during shutdown this must be not as deafult in RE.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is very flexible. the RunEngine has
install_suspenderandremove_suspenderhooks to remove it globally. E.g by default,run_engine.install_suspender(checkbeam)and in dummy we can easily dorun_engine.remove_suspender(checkbeam)for shutdown.We also have the option for the suspender to only suspender if
synchrotron.synchtron_modeis not"Shutdown"or"Mach. Dev"as well. So this will not be a problem.The two things we need from core team though will be for this to be supported in BlueAPI. We will need #1997 to be supported,
HasNameon objects or something equivalent.We also need the run engine hooks to be present so that from blueapi client we can do something like so
dodal side
Blueapi client side
and also inside plans, we can control it locally (inject has to be able to support things with names that aren't devices DiamondLightSource/blueapi#1462)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer your 2nd offer to auto detect if synchrotron in user mode or not. This will eliminate the need for users to know how to install and remove suspender as checkbeam will be permenantly installed in run engine which can be configured by beamline support software engineers.
However we still need to extend blueapi to support this instal/remove suspender feature for other use cases.