Skip to content

Add Jungfrau do pedestal darks plans#1219

Merged
DominicOram merged 55 commits into
mainfrom
1200_add_jf_do_darks
Oct 17, 2025
Merged

Add Jungfrau do pedestal darks plans#1219
DominicOram merged 55 commits into
mainfrom
1200_add_jf_do_darks

Conversation

@olliesilvester

@olliesilvester olliesilvester commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

Part of #1200

This adds the pedestal dark plan. Note that we will need a new release of ophyd-async (for bluesky/ophyd-async@15fa34b) for this plan to actually work on the beamline

  1. Confirm plan does the same thing as the hotfixed messy version in /dls_sw/i24/software/bluesky/jungfrau_25
  2. Check test is sufficient

Checks for reviewer

  • Would the PR title make sense to a user on a set of release notes

@olliesilvester olliesilvester changed the base branch from main to 1200_add_jf_internal_acquisition September 2, 2025 09:00
@olliesilvester olliesilvester marked this pull request as ready for review September 2, 2025 15:58
@olliesilvester olliesilvester requested a review from a team as a code owner September 2, 2025 15:58
@olliesilvester olliesilvester changed the title 1200 add jf do darks Add Jungfrau do darks plans Sep 2, 2025
@olliesilvester olliesilvester changed the title Add Jungfrau do darks plans Add Jungfrau do pedestal darks plans Oct 3, 2025
@olliesilvester

Copy link
Copy Markdown
Contributor Author

I think this is in a mergable state now? WIll do an ophyd-async release next week so CI can pass

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is in a mergable state now? WIll do an ophyd-async release next week so CI can pass

ophyd-async release done so I think we just need to add the small tests and fix the failing CI


@patch("mx_bluesky.beamlines.i24.jungfrau_commissioning.do_darks.override_file_path")
@patch("bluesky.plan_stubs.unstage")
async def test_jungfrau_unstage(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
async def test_jungfrau_unstage(
async def test_jungfrau_unstaged_when_plan_in_error(



@patch("mx_bluesky.beamlines.i24.jungfrau_commissioning.do_darks.override_file_path")
async def test_full_do_pedestal_darks(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If they win then we should add them in

@olliesilvester

Copy link
Copy Markdown
Contributor Author

Couple of bugs caught from last bit of testing:

  • fly_jungfrau doesn't do wait on stage
  • Staging the JF puts the JF out of pedestal mode, this means the part which says "please enter pedestal mode when you next prepare" needs to be in-between stage and prepare, which means fly_jungfrau needs an extra parameter to know if is doing things in pedestal mode.

Sort of thinking about extending TriggerInfo for the jungfrau to get some proper pedestal options in? That'll be a future ticket though

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As discussed, can we move the stage out of fly_jungfrau, instead plans that call fly_jungfrau should first do @bpp.stage_decorator([jungfrau]). do_pedestal_darks can then set the pedestal PVs itself. We should add a comment in fly_jungfrau to say it should be staged.

@DominicOram DominicOram mentioned this pull request Oct 17, 2025
1 task

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, thanks. One comment. Also do_internal_acquisition and do_external_acquisition should probably include the stage decorators.

@olliesilvester

Copy link
Copy Markdown
Contributor Author

Also do_internal_acquisition and do_external_acquisition should probably include the stage decorators.

We decided that these are plans stubs so the staging goes in whatever plan is using them

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, thank you!

@DominicOram DominicOram merged commit 8f94a31 into main Oct 17, 2025
24 checks passed
@DominicOram DominicOram deleted the 1200_add_jf_do_darks branch October 17, 2025 14:57
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.

2 participants