Skip to content

Add a small dummy plan to find the beam centre - #123

Merged
DominicOram merged 17 commits into
mainfrom
122-beam_centre
Jul 22, 2026
Merged

Add a small dummy plan to find the beam centre #123
DominicOram merged 17 commits into
mainfrom
122-beam_centre

Conversation

@noemifrisina

Copy link
Copy Markdown
Collaborator

Closes #122

@noemifrisina
noemifrisina marked this pull request as draft February 25, 2026 17:30

@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, minor comment

Comment thread src/i19_bluesky/eh1/find_beam_centre.py Outdated
beam_centre: CentreEllipseMethod = inject("beam_centre"),
) -> MsgGenerator:
yield from bps.abs_set(
beam_centre.roi_box_size, 10000

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: Let's make this a bit more formal. Can you read the image size from the PV and then use the largest of width/height?

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.

Could: Or we could put this logic in the device itself? So you could set something like beam_centre.roi_box_size, None and it would do this.

@codecov

codecov Bot commented Mar 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.77%. Comparing base (d59ec26) to head (c7ba092).

Files with missing lines Patch % Lines
src/i19_bluesky/eh1/find_beam_centre.py 80.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
- Coverage   99.36%   98.77%   -0.60%     
==========================================
  Files          33       34       +1     
  Lines         631      652      +21     
==========================================
+ Hits          627      644      +17     
- Misses          4        8       +4     

☔ View full report in Codecov by Harness.
📢 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.

@noemifrisina
noemifrisina marked this pull request as ready for review July 10, 2026 16:59
@noemifrisina
noemifrisina requested a review from DominicOram July 10, 2026 16:59

@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, some comments in code

Comment thread src/i19_bluesky/eh1/find_beam_centre.py Outdated
roi_size = max(x_size, y_size)
yield from bps.abs_set(beam_centre.roi_box_size, roi_size)
LOGGER.info(f"Finding beam centre with roi_box_size set to {roi_size}")
yield from bps.trigger(beam_centre)

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: the wait on trigger defaults to false for some reason, you probably want yield from bps.trigger(beam_centre, wait=True)

Comment thread src/i19_bluesky/eh1/find_beam_centre.py Outdated
x_size = yield from bps.rd(oav.grid_snapshot.x_size)
y_size = yield from bps.rd(oav.grid_snapshot.y_size)
roi_size = max(x_size, y_size)
yield from bps.abs_set(beam_centre.roi_box_size, roi_size)

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: You want to make sure this is set before triggering, either with wait=True or change to a mv

from i19_bluesky.log import LOGGER


def find_beam_centre_plan(

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.

Could: It feels like this should actually return the found centre?

Comment thread src/i19_bluesky/eh1/find_beam_centre.py Outdated
Comment on lines +15 to +16
x_size = yield from bps.rd(oav.grid_snapshot.x_size)
y_size = yield from bps.rd(oav.grid_snapshot.y_size)

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.

Could: Can we use snapshot rather than grid_snapshot? Either is quite weird tbh but that's another issue

@noemifrisina
noemifrisina requested a review from DominicOram July 13, 2026 16:45

@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 f64820d into main Jul 22, 2026
9 of 11 checks passed
@DominicOram
DominicOram deleted the 122-beam_centre branch July 22, 2026 09:53
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.

Test code for finding beam centre

2 participants