Skip to content

Add external triggering jungfrau plan#1202

Merged
olliesilvester merged 13 commits into
mainfrom
1200_add_basic_jf_plans
Sep 17, 2025
Merged

Add external triggering jungfrau plan#1202
olliesilvester merged 13 commits into
mainfrom
1200_add_basic_jf_plans

Conversation

@olliesilvester

@olliesilvester olliesilvester commented Aug 13, 2025

Copy link
Copy Markdown
Contributor

CI will fail as the jungfrau isn't merged into ophyd-async yet.

Begins #1200

Requires:

Instructions to reviewer on how to test:

  1. Use the branches in the above PR's for ophyd-async and dodal
  2. Check new plan
  3. Check new tests

Checks for reviewer

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

@olliesilvester olliesilvester requested a review from a team as a code owner August 13, 2025 08:57

@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.

Thanks, couple of comments in code. Do we also need to add the jungfrau to dodal.beamlines.i24?

from mx_bluesky.common.utils.log import LOGGER


def do_external_acquisition(

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.

Should: I'm not sure how useful this plan is in isolation because to get the external triggering working we will need to set the zebra up etc. The issue also says that the first one to start with is do_manual_acquisition, which is an internal acquisition, not an external one, like the rotation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, this plan probably isn't useful as an entry point. I won't add it to the BlueAPI i24 entry plans. We will still need to use it within other plans though.

Yes, I seem to have got mixed up between external and internal triggering at some point during this PR... I will make another one for internal triggering now

@olliesilvester

olliesilvester commented Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

Do we also need to add the jungfrau to dodal.beamlines.i24?

I just added a draft PR here. Not sure what the prefix is / the number of odin nodes / the default path + filename to use yet 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.

Looks good, thanks. I think we need to get the ophyd-async one merged and released first then we can do this one

@olliesilvester

olliesilvester commented Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

Once this CI passes I will merge this PR. The plan here is outdated compared to the one tested on i24, but that is addressed in #1216

@codecov

codecov Bot commented Sep 17, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.14%. Comparing base (a5c8dc2) to head (042c5c9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1202      +/-   ##
==========================================
+ Coverage   90.10%   90.14%   +0.03%     
==========================================
  Files         129      130       +1     
  Lines        7802     7830      +28     
==========================================
+ Hits         7030     7058      +28     
  Misses        772      772              
Components Coverage Δ
i24 SSX 75.27% <ø> (ø)
hyperion 97.79% <ø> (ø)
other 97.98% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@olliesilvester olliesilvester merged commit cbd40b0 into main Sep 17, 2025
24 checks passed
@olliesilvester olliesilvester deleted the 1200_add_basic_jf_plans branch September 17, 2025 14:16
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