Add plan for automatically finding the beam centre on the OAV #1496

srishtysajeev · 2025-12-04T15:42:22Z

To test:

Confirm tests pass

… and the frac_of_max + add docstrings

DominicOram

Mostly reminders to myself that I will fix but would like an answer on the question around when we optimise transmission

DominicOram · 2025-12-10T18:16:02Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

+    for zoom in zoom_levels:
+        LOGGER.info(f"Moving to zoom level {zoom}")
+        yield from bps.abs_set(oav.zoom_controller, zoom, wait=True)
+        yield from bps.sleep(1)


Should: Why do we have a sleep here and in the zoom device? I think probably just having one in the zoom device is better

DominicOram · 2025-12-10T18:16:55Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

+    zoom_level_to_dict = {
+        "7.5x": [
+            oav.zoom_controller.x_placeholder_zoom_7,
+            oav.zoom_controller.y_placeholder_zoom_7,
+        ],
+        "1.0x": [
+            oav.zoom_controller.x_placeholder_zoom_1,
+            oav.zoom_controller.y_placeholder_zoom_1,
+        ],
+        "1.5x": [
+            oav.zoom_controller.x_placeholder_zoom_2,
+            oav.zoom_controller.y_placeholder_zoom_2,
+        ],
+        "2.0x": [
+            oav.zoom_controller.x_placeholder_zoom_3,
+            oav.zoom_controller.y_placeholder_zoom_3,
+        ],
+        "2.5x": [
+            oav.zoom_controller.x_placeholder_zoom_4,
+            oav.zoom_controller.y_placeholder_zoom_4,
+        ],
+        "3.0x": [
+            oav.zoom_controller.x_placeholder_zoom_5,
+            oav.zoom_controller.y_placeholder_zoom_5,
+        ],
+        "5.0x": [
+            oav.zoom_controller.x_placeholder_zoom_6,
+            oav.zoom_controller.y_placeholder_zoom_6,
+        ],
+        "10.0x": [
+            oav.zoom_controller.x_placeholder_zoom_8,
+            oav.zoom_controller.y_placeholder_zoom_8,
+        ],
+    }


This should be stored on the device

DominicOram · 2025-12-10T18:17:39Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

+        LOGGER.info(f"Moving to zoom level {zoom}")
+        yield from bps.abs_set(oav.zoom_controller, zoom, wait=True)
+        yield from bps.sleep(1)
+        if zoom == "7.5x" or zoom == "1.0x":


Should: Why do we want to optimise transmission at specifically these zooms? Maybe a question for @olliesilvester or @aragaod

I think this was a bit arbitrary. We definitely wanted to optimise transmission for zoom 7.5 because it's apparently the most important zoom level and then we noticed that this transmission wasn't as good for the low zooms so we also decided to run the optimisation for zoom 1 too during testing.

codecov · 2025-12-16T14:44:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.36%. Comparing base (327c161) to head (ef5cabf).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1496      +/-   ##
==========================================
+ Coverage   92.29%   92.36%   +0.06%     
==========================================
  Files         142      142              
  Lines        8094     8168      +74     
==========================================
+ Hits         7470     7544      +74     
  Misses        624      624

Components	Coverage Δ
i24 SSX	`78.56% <ø> (ø)`
hyperion	`97.93% <ø> (ø)`
other	`98.12% <100.00%> (+0.04%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

olliesilvester · 2025-12-19T10:04:17Z

Notes after testing

Gives bad center on zoom 1 due to background ring. This should get fixed by using ROI (this logic is in another branch)
The binary search is a bit overkill. We can relax this by doing something like: At the beginning of a new iteration, check how much we're going to change the transmission. If it's less than x%, then we just end it there and use that transmission
XBPM trigger PV seems a bit dodgy - we think it isn't waiting long enough
Should use string enums for the zooms in initial params rather than strings. Also maybe throw an error if

olliesilvester

This worked as-is on the beamline, but the main things to address are:

Better validation if you typo the parameters
Less strict binary search (but we can put that into a new issue)
Address dodgy xbpm feedback. Could add sleeps in for now, or chase up with controls

olliesilvester · 2025-12-19T14:15:12Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

                    defaults are always correct.
    """

+    LOGGER.info("prearing beamline")


Suggested change

LOGGER.info("prearing beamline")

LOGGER.info("Preparing beamline to take scintillator images...")

olliesilvester · 2025-12-19T14:15:19Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

+        initial_wait_group,
    )
-
+    LOGGER.info("setting transmission")


Suggested change

LOGGER.info("setting transmission")

olliesilvester · 2025-12-19T14:16:07Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

    if image_name is None:
        image_name = f"{time.time_ns()}ATT{transmission * 100}"
-
+    LOGGER.info(f"using image name {image_name}")


Suggested change

LOGGER.info(f"using image name {image_name}")

LOGGER.info(f"Using image name {image_name}")

olliesilvester · 2025-12-19T14:21:49Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

 ) -> MsgGenerator:
    """
-    Prepares the beamline for oav image by making sure the pin is NOT mounted and
+    Prepares the beamline for oav image by making sure the pin is not mounted and


Could: mention in docstring that we're also opening shutter here

olliesilvester · 2025-12-19T14:24:56Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

+    return (yield from bps.rd(max_pixel.max_pixel_val))
+
+
+def optimise_transmission_with_oav(


Notes from testing:

Occasionally this would hit the StopIteration for various reasons: We are being overly precise; there is an issue with i04's xbpm PV which is causing us to not wait for long enough,;the error margin on the transmission device is much higher than the PV suggests.
We should keep the binary search, but either increase the tolerence, or call it "done" as soon as the next iteration would involve moving the transmission by <x%

Happy to put this part into a separate issue

olliesilvester · 2025-12-19T14:27:03Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

+    zoom_levels_to_centre: list[str] | None = None,
+    zoom_levels_to_optimise_transmission: list[str] | None = None,


Should: These should default to ["1.0x", "7.5x"] and ["1.0x", "7.5x"] respectively, then the if None part on lines 261 and 264 can be removed.

Also, we should use a string enum for the accepted zoom levels so that we can get proper validation. Right now there is no obvious error if someone typo's the zoom level parameters, the plan just ends up hanging somewhere.

olliesilvester · 2025-12-19T14:28:39Z

src/mx_bluesky/beamlines/i04/oav_centering_plans/oav_imaging.py

+    xbpm_feedback: XBPMFeedback,
+    transmission: float,
+):
+    yield from bps.trigger(xbpm_feedback, wait=True)


During testing we ended up adding a sleep after triggering xbpm feedback. This real fix is to address the issues with the PV though

olliesilvester and others added 15 commits November 20, 2025 14:57

WIP

f28deaa

Merge branch 'main' into test_optimise_transmission

47e341d

Add logic for getting target pixel brightness from 100% transmission…

462538c

… and the frac_of_max + add docstrings

adding comments for tests

adc209f

mock function to get transmission out

c37400c

pull changes from beamline testing

e35e079

Adding my commit

a69428b

refactoring so that 100 percent transmission is in a diff place

086ac31

attempting tests again

a2e6991

merge main

e9eaf94

run plan through run engine

9a4117f

Initial commit - add plan for comissioning day tomorrow

b2868bc

add to the init file

a740b56

add changes post testing

1174926

small changes

d0bf270

olliesilvester mentioned this pull request Dec 9, 2025

1318 optimise oav transmission new #1473

Closed

1 task

DominicOram added the i04 Changes relating to I04 label Dec 9, 2025

DominicOram reviewed Dec 10, 2025

View reviewed changes

DominicOram added 5 commits December 11, 2025 14:13

Fixes from review

d40b305

Use separate zoom controller

4e5325a

Add tests for optimise transmission

e916466

Add more tests

cc958af

Merge branch 'main' into 1489_automated_oav_centring_post_testing

9f10879

DominicOram changed the title ~~1489 automated oav centring post testing~~ Add plan for automatically finding the beam centre on the OAV Dec 16, 2025

DominicOram marked this pull request as ready for review December 16, 2025 14:49

DominicOram requested a review from a team as a code owner December 16, 2025 14:49

Remove unneeded patch

cddb954

olliesilvester self-assigned this Dec 18, 2025

Merge branch 'main' into 1489_automated_oav_centring_post_testing

ef5cabf

olliesilvester requested changes Dec 19, 2025

View reviewed changes

	LOGGER.info("prearing beamline")
	LOGGER.info("Preparing beamline to take scintillator images...")

	LOGGER.info(f"using image name {image_name}")
	LOGGER.info(f"Using image name {image_name}")

		return (yield from bps.rd(max_pixel.max_pixel_val))


		def optimise_transmission_with_oav(

		zoom_levels_to_centre: list[str] \| None = None,
		zoom_levels_to_optimise_transmission: list[str] \| None = None,

Add plan for automatically finding the beam centre on the OAV #1496

Are you sure you want to change the base?

Add plan for automatically finding the beam centre on the OAV #1496

Uh oh!

Conversation

srishtysajeev commented Dec 4, 2025 • edited by DominicOram Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DominicOram left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

olliesilvester commented Dec 19, 2025

Uh oh!

olliesilvester left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

srishtysajeev commented Dec 4, 2025 •

edited by DominicOram

Loading

codecov bot commented Dec 16, 2025 •

edited

Loading