Skip to content

Concept of checkbeam device#1975

Draft
oliwenmandiamond wants to merge 1 commit intomainfrom
Concept_of_pause_plan_device
Draft

Concept of checkbeam device#1975
oliwenmandiamond wants to merge 1 commit intomainfrom
Concept_of_pause_plan_device

Conversation

@oliwenmandiamond
Copy link
Copy Markdown
Contributor

Fixes #ISSUE

Instructions to reviewer on how to test:

  1. Do thing x
  2. Confirm thing y happens

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 91.22807% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.07%. Comparing base (e243681) to head (b349dee).

Files with missing lines Patch % Lines
src/dodal/devices/pause_plan_device.py 93.75% 3 Missing ⚠️
src/dodal/beamlines/i09.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1975      +/-   ##
==========================================
- Coverage   99.10%   99.07%   -0.04%     
==========================================
  Files         318      319       +1     
  Lines       12133    12190      +57     
==========================================
+ Hits        12025    12077      +52     
- Misses        108      113       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rtuck99
Copy link
Copy Markdown
Contributor

rtuck99 commented Apr 17, 2026

I wonder if there would be any benefit in supporting WatchableAsyncStatus with this - I have done something similar for hyperion waiting-for-beam, because it enables the supervisor to monitor it for progress to determine whether the plan is genuinely stuck or merely waiting for beam.

@DominicOram
Copy link
Copy Markdown
Contributor

Have you looked at using suspenders for this? I think it's the more Bluesky-native way of doing it

@oliwenmandiamond
Copy link
Copy Markdown
Contributor Author

Yeah, I've had a look and they do look to be what we want. My only concern is that they are installed globally on the RunEngine so applied to every plan. This is the opposite of what GDA currently does where it is only applied when user specifies it in the scan. We would also need BlueAPI to support being able to add them.

e.g

@devices.suspender
def checkbeam() -> MySuspender:
     ...

then on blueapiclient side we could maybe do this

bc = BlueapiClient()
devs = bc.devices
plans = bc.plans

# Installed suspenders by default
`>>> plans.scan(...)`

#Run a plan without the suspender
>>> bc.remove_suspender("checkbeam")
>>> bc.my_plan()

>>> bc.install_suspender("checkbeam")
>>> bc.scan(...)

As I mentioned in one of the previous athena drop in sessions, it feels like DeviceManager will have to evolve to a BeamlineManager to being able to support things that aren't devices. Or maybe we create lots of sub managers to manage specific things e.g SuspenderManager which blueapi can also register like DeviceManager. However, suspenders need access to device and their signals to construct one.

@DominicOram
Copy link
Copy Markdown
Contributor

My only concern is that they are installed globally on the RunEngine so applied to every plan.

You can do it inside the plan. Using install or remove e.g.

def my_plan():
   yield from bps.install_suspender(checkbeam)
   ...
   yield from bps.remove_suspender(checkbeam)

or more cleanly (and to cover the finally case):

@bpp.suspend_wrapper(checkbeam)
def my_plan():
    ...

@oliwenmandiamond
Copy link
Copy Markdown
Contributor Author

oliwenmandiamond commented Apr 24, 2026

My only concern is that they are installed globally on the RunEngine so applied to every plan.

You can do it inside the plan. Using install or remove e.g.

def my_plan():
   yield from bps.install_suspender(checkbeam)
   ...
   yield from bps.remove_suspender(checkbeam)

or more cleanly (and to cover the finally case):

@bpp.suspend_wrapper(checkbeam)
def my_plan():
    ...

Okay I didn't know this. My issue with doing it like this though is that you now have to wrap every plan with this where as with it being a device like GDA, it can be used with any plan whenever required dynamically as you just specify the device to the plan arguments. This way is less flexible.

Also, how do we apply this with BlueAPI?

Do we have to define a suspender instance in dodal and then have our plan repos import the suspender and add this as a decorator?

@oliwenmandiamond
Copy link
Copy Markdown
Contributor Author

oliwenmandiamond commented Apr 24, 2026

To make it generic, I think we would have to have blueapi support it as server object so then from client side we could do

>>> plans.install_suspender(bc.suspenders.checkbeam)

So we can turn on / off whenever user requires it.

If it is a server object too, again we can use inject for plans to auto include them. Another use case for #1997 and DiamondLightSource/blueapi#1462

@DominicOram
Copy link
Copy Markdown
Contributor

My issue with doing it like this though is that you now have to wrap every plan with this where as with it being a device like GDA, it can be used with any plan whenever required dynamically as you just specify the device to the plan arguments. This way is less flexible.

Ok, in which case having an endpoint or configuration option on BlueAPI to add it to the RE seems like a better way to go.

Also, how do we apply this with BlueAPI?

I'm not sure I understand, BlueAPI runs your plan, your plan adds the suspender. Do you mean how do we apply it to the out of the box scans? In which case, yh, creating a way of asking BlueAPI to hook it into the RE would be the way to do it.

Do we have to define a suspender instance in dodal and then have our plan repos import the suspender and add this as a decorator?
Another use case for #1997 and DiamondLightSource/blueapi#1462

Yh, I agree we should do this.

I really think we should go down the suspender route if possible though, this is literally what they're designed for. And using a device like this seems like we're going back down the route of having "Scannables are everything". @DiamondLightSource/developers-daq-core how do you envisage suspenders being added to plans?

@oliwenmandiamond
Copy link
Copy Markdown
Contributor Author

I really think we should go down the suspender route if possible though, this is literally what they're designed for. And using a device like this seems like we're going back down the route of having "Scannables are everything". @DiamondLightSource/developers-daq-core how do you envisage suspenders being added to plans?

Agreed, we should find a way to support suspenders to do checkbeam and any other conditions.

@fajinyuan
Copy link
Copy Markdown
Contributor

Agreed that we should make use of bluesky offered mechanism - suppender - for this use case. BlueAPI client has to support this requirement to enable user control of this. As to the implementation as Device or not is debateable, in my personal view Ophyd device can be a virtual device implementing special logics using other devices. After all, suspender is to pause data collection under certain detected conditions. This can be done by suspend Run Engine or making a virtual device busy (at least for step scan this will work).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants