Skip to content

Update setup_oav to use plan moved to dodal - #1577

Merged
DominicOram merged 7 commits into
mainfrom
setup-pin-tip-moved-to-dodal
Jul 28, 2026
Merged

Update setup_oav to use plan moved to dodal#1577
DominicOram merged 7 commits into
mainfrom
setup-pin-tip-moved-to-dodal

Conversation

@noemifrisina

Copy link
Copy Markdown
Contributor

The setup_pin_tip_detection_params was moved to dodal in 1858 so that it could be used by non-mx beamlines too. This PR updates setup_oav to use that one.

Instructions to reviewer on how to test:

  1. Check all instances of setup_pin_tip_detection_params have been updated to use the plan in dodal
  2. Run tests

Checks for reviewer

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

@noemifrisina
noemifrisina requested a review from a team as a code owner January 26, 2026 16:59
@noemifrisina noemifrisina added the dev experience Changes relating to developer experience label Jan 26, 2026
@codecov

codecov Bot commented Jan 26, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1577      +/-   ##
==========================================
- Coverage   93.08%   93.07%   -0.01%     
==========================================
  Files         164      164              
  Lines        8997     8988       -9     
==========================================
- Hits         8375     8366       -9     
  Misses        622      622              
Components Coverage Δ
i24 SSX 77.16% <ø> (ø)
hyperion 98.77% <ø> (ø)
other 98.16% <100.00%> (-0.01%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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, just the one issue

yield from setup_general_oav_params(oav, parameters)
yield from setup_pin_tip_detection_params(pin_tip_detection_device, parameters)
yield from setup_pin_tip_detection_params(
pin_tip_detection_device, parameters, PlanGroupCheckpointConstants.READY_FOR_OAV

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.

Must: The plan you have removed does not wait at the end but the new one in dodal does by default, can you add wait=False here?

@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 enabled auto-merge (squash) January 28, 2026 15:00
@DominicOram
DominicOram merged commit 78fc102 into main Jul 28, 2026
13 checks passed
@DominicOram
DominicOram deleted the setup-pin-tip-moved-to-dodal branch July 28, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev experience Changes relating to developer experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants