Skip to content

Move CentreEllipseMethod to a more common location #1942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
noemifrisina wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Move CentreEllipseMethod to a more common location #1942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
36ff602
First try just leaving everything as is
noemifrisina Feb 25, 2026
57cf794
Move beam_centre device to a more common location and instantiate on …
noemifrisina Feb 25, 2026
f4d7d6f
Merge branch 'main' into i19-122_find-beam-centre
noemifrisina Feb 25, 2026
8630165
Actually fix tests
noemifrisina Feb 25, 2026
6e2ef54
That was not a fix - this hopefully is
noemifrisina Feb 25, 2026
9afaec5
Merge branch 'main' into i19-122_find-beam-centre
noemifrisina Feb 27, 2026
233766f
Merge branch 'main' into i19-122_find-beam-centre
noemifrisina Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dodal/beamlines/i04.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from dodal.devices.backlight import Backlight
from dodal.devices.baton import Baton
from dodal.devices.beamlines.i03.dcm import DCM
from dodal.devices.beamlines.i04.beam_centre import CentreEllipseMethod
from dodal.devices.beamlines.i04.beamsize import Beamsize
from dodal.devices.beamlines.i04.constants import RedisConstants
from dodal.devices.beamlines.i04.max_pixel import MaxPixel
Expand All @@ -26,6 +25,7 @@
from dodal.devices.ipin import IPin
from dodal.devices.motors import XYZStage
from dodal.devices.mx_phase1.beamstop import Beamstop
from dodal.devices.oav.beam_centre.beam_centre import CentreEllipseMethod
from dodal.devices.oav.oav_detector import (
OAVBeamCentrePV,
ZoomControllerWithBeamCentres,
Expand Down
9 changes: 9 additions & 0 deletions src/dodal/beamlines/i19_1.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
)
from dodal.devices.beamlines.i19.beamstop import BeamStop
from dodal.devices.beamlines.i19.pin_tip import PinTipCentreHolder
from dodal.devices.oav.beam_centre.beam_centre import CentreEllipseMethod
from dodal.devices.oav.oav_detector import OAVBeamCentreFile
from dodal.devices.oav.oav_parameters import OAVConfigBeamCentre
from dodal.devices.oav.pin_image_recognition import PinTipDetection
Expand Down Expand Up @@ -60,6 +61,14 @@ def beamstop() -> BeamStop:
return BeamStop(prefix=f"{PREFIX.beamline_prefix}-RS-ABSB-01:")


@devices.factory()
def beam_centre() -> CentreEllipseMethod:
return CentreEllipseMethod(
prefix=f"{PREFIX.beamline_prefix}-EA-OAV-01:",
overlay_channel=7,
)


@devices.fixture
def oav_config() -> OAVConfigBeamCentre:
return OAVConfigBeamCentre(ZOOM_PARAMS_FILE, DISPLAY_CONFIG)
Expand Down
Empty file added src/dodal/devices/oav/beam_centre/__init__.py
View file
Open in desktop
Empty file.
1 change: 1 addition & 0 deletions ...odal/devices/beamlines/i04/beam_centre.py → ...al/devices/oav/beam_centre/beam_centre.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ async def trigger(self):
ellipse_fit = self._fit_ellipse(roi_binary)
roi_centre_x = ellipse_fit[0][0]
roi_centre_y = ellipse_fit[0][1]
LOGGER.info(f"Beam centre founf at ({roi_centre_x}, {roi_centre_y})")
Copy link
Copy Markdown
Contributor

@DominicOram DominicOram Mar 2, 2026

Choose a reason for hiding this comment

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

Nit:

Suggested change
LOGGER.info(f"Beam centre founf at ({roi_centre_x}, {roi_centre_y})")
LOGGER.info(f"Beam centre found at ({roi_centre_x}, {roi_centre_y})")

Copy link
Copy Markdown
Contributor

@DominicOram DominicOram Mar 2, 2026

Choose a reason for hiding this comment

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

Also, this is the centre in the ROI co-ordinates, I think the full image co-ordinates would be more expected (or specify in the log that they're ROI)

# convert back to full screen image coords and set beam centre
self._center_x_val_setter(roi_centre_x + top_left_corner[0])
self._center_y_val_setter(roi_centre_y + top_left_corner[1])
16 changes: 8 additions & 8 deletions ...devices/beamlines/i04/test_beam_centre.py → tests/devices/oav/test_beam_centre.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import pytest
from ophyd_async.core import init_devices, set_mock_value

from dodal.devices.beamlines.i04.beam_centre import (
from dodal.devices.oav.beam_centre.beam_centre import (
CentreEllipseMethod,
convert_image_to_binary,
get_roi,
Expand Down Expand Up @@ -35,8 +35,8 @@ def test_image_array():
)


@patch("dodal.devices.beamlines.i04.beam_centre.cv2.threshold")
@patch("dodal.devices.beamlines.i04.beam_centre.convert_to_gray_and_blur")
@patch("dodal.devices.oav.beam_centre.beam_centre.cv2.threshold")
@patch("dodal.devices.oav.beam_centre.beam_centre.convert_to_gray_and_blur")
async def test_convert_image_to_binary_calls_threshold_twice(
mock_convert, mock_threshold
):
Expand All @@ -60,8 +60,8 @@ async def test_convert_image_to_binary_calls_threshold_twice(
assert second_call_args[1] == 147


@patch("dodal.devices.beamlines.i04.beam_centre.cv2.findContours")
@patch("dodal.devices.beamlines.i04.beam_centre.cv2.fitEllipse")
@patch("dodal.devices.oav.beam_centre.beam_centre.cv2.findContours")
@patch("dodal.devices.oav.beam_centre.beam_centre.cv2.fitEllipse")
async def test_fit_ellipse_good_params(
fit_ellipse: MagicMock,
find_contours_mock: MagicMock,
Expand Down Expand Up @@ -95,7 +95,7 @@ async def test_fit_ellipse_raises_error_if_not_enough_image_points(
], # 4 points (square)
],
)
@patch("dodal.devices.beamlines.i04.beam_centre.cv2.findContours")
@patch("dodal.devices.oav.beam_centre.beam_centre.cv2.findContours")
async def test_fit_ellipse_raises_error_if_not_enough_contour_points(
find_contours_mock: MagicMock,
centre_device: CentreEllipseMethod,
Expand All @@ -108,8 +108,8 @@ async def test_fit_ellipse_raises_error_if_not_enough_contour_points(
await centre_device.trigger()


@patch("dodal.devices.beamlines.i04.beam_centre.CentreEllipseMethod._fit_ellipse")
@patch("dodal.devices.beamlines.i04.beam_centre.convert_image_to_binary")
@patch("dodal.devices.oav.beam_centre.beam_centre.CentreEllipseMethod._fit_ellipse")
@patch("dodal.devices.oav.beam_centre.beam_centre.convert_image_to_binary")
async def test_trigger_converts_to_binary_then_finds_ellipse(
mock_convert_to_binary: MagicMock,
mock_fit_ellipse: MagicMock,
Expand Down