Merge pull request #1214 from DiamondLightSource/1068_1213_use_ophyd_…

…oav_edge_detetion_for_grid

Use ophyd oav edge detetion for grid determination

setup.cfg

@@ -35,7 +35,7 @@ install_requires =
     xarray
     doct
     databroker
-    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@54af3ccf31a1570dc4bdb0d365f8696a3d130d92
+    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@e5e629eec30f8aa1ef9edb5de0475cfcdb68365f
     pydantic<2.0 # See https://github.com/DiamondLightSource/hyperion/issues/774
     scipy
     pyzmq<25 # See https://github.com/DiamondLightSource/hyperion/issues/1103

src/hyperion/device_setup_plans/setup_oav.py

@@ -133,7 +133,8 @@ def pre_centring_setup_oav(
     )
 
     # Connect MXSC output to MJPG input for debugging
-    yield from set_using_group(oav.snapshot.input_plugin, "OAV.MXSC")
+    if isinstance(pin_tip_detection_device, MXSC):
+        yield from set_using_group(oav.snapshot.input_plugin, "OAV.MXSC")
 
     yield from bps.wait(oav_group)
 
@@ -178,7 +179,7 @@ def wait_for_tip_to_be_found(
     ophyd_pin_tip_detection: PinTipDetection | PinTipDetect,
 ) -> Generator[Msg, None, Pixel]:
     yield from bps.trigger(ophyd_pin_tip_detection, wait=True)
-    found_tip = yield from bps.rd(ophyd_pin_tip_detection)
+    found_tip = yield from bps.rd(ophyd_pin_tip_detection.triggered_tip)
     if found_tip == ophyd_pin_tip_detection.INVALID_POSITION:
         timeout = yield from bps.rd(ophyd_pin_tip_detection.validity_timeout)
         raise WarningException(f"No pin found after {timeout} seconds")

src/hyperion/experiment_plans/oav_grid_detection_plan.py

@@ -8,7 +8,6 @@
 import bluesky.preprocessors as bpp
 import numpy as np
 from blueapi.core import BlueskyContext
-from bluesky.preprocessors import finalize_wrapper
 from dodal.devices.backlight import Backlight
 from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.pin_image_recognition import PinTipDetection
@@ -43,35 +42,14 @@ def create_devices(context: BlueskyContext) -> OavGridDetectionComposite:
     return device_composite_from_context(context, OavGridDetectionComposite)
 
 
-def grid_detection_plan(
-    composite: OavGridDetectionComposite,
-    parameters: OAVParameters,
-    snapshot_template: str,
-    snapshot_dir: str,
-    grid_width_microns: float,
-    box_size_microns=20,
-):
-    yield from finalize_wrapper(
-        grid_detection_main_plan(
-            composite,
-            parameters,
-            snapshot_template,
-            snapshot_dir,
-            grid_width_microns,
-            box_size_microns,
-        ),
-        reset_oav(composite.oav),
-    )
-
-
 @bpp.run_decorator()
-def grid_detection_main_plan(
+def grid_detection_plan(
     composite: OavGridDetectionComposite,
     parameters: OAVParameters,
     snapshot_template: str,
     snapshot_dir: str,
     grid_width_microns: float,
-    box_size_um: float,
+    box_size_um: float = 20,
 ):
     """
     Creates the parameters for two grids that are 90 degrees from each other and
@@ -87,13 +65,14 @@ def grid_detection_main_plan(
     """
     oav: OAV = composite.oav
     smargon: Smargon = composite.smargon
+    pin_tip_detection: PinTipDetection = composite.pin_tip_detection
 
     LOGGER.info("OAV Centring: Starting grid detection centring")
 
     yield from bps.wait()
 
     # Set relevant PVs to whatever the config dictates.
-    yield from pre_centring_setup_oav(oav, parameters, oav.mxsc)
+    yield from pre_centring_setup_oav(oav, parameters, pin_tip_detection)
 
     LOGGER.info("OAV Centring: Camera set up")
 
@@ -114,12 +93,14 @@ def grid_detection_main_plan(
         # See #673 for improvements
         yield from bps.sleep(OAV_REFRESH_DELAY)
 
-        tip_x_px, tip_y_px = yield from wait_for_tip_to_be_found(oav.mxsc.pin_tip)
+        tip_x_px, tip_y_px = yield from wait_for_tip_to_be_found(pin_tip_detection)
 
         LOGGER.info(f"Tip is at x,y: {tip_x_px},{tip_y_px}")
 
-        top_edge = np.array((yield from bps.rd(oav.mxsc.top)))
-        bottom_edge = np.array((yield from bps.rd(oav.mxsc.bottom)))
+        top_edge = np.array((yield from bps.rd(pin_tip_detection.triggered_top_edge)))
+        bottom_edge = np.array(
+            (yield from bps.rd(pin_tip_detection.triggered_bottom_edge))
+        )
 
         full_image_height_px = yield from bps.rd(oav.cam.array_size.array_size_y)
 
@@ -198,9 +179,3 @@ def grid_detection_main_plan(
     )
 
     LOGGER.info(f"Step sizes: {box_size_um, box_size_um, box_size_um}")
-
-
-def reset_oav(oav: OAV):
-    """Changes the MJPG stream to look at the camera without the edge detection and turns off the edge detcetion plugin."""
-    yield from bps.abs_set(oav.snapshot.input_plugin, "OAV.CAM")
-    yield from bps.abs_set(oav.mxsc.enable_callbacks, 0)

src/hyperion/experiment_plans/pin_tip_centring_plan.py

@@ -44,7 +44,7 @@ def trigger_and_return_pin_tip(
     pin_tip: PinTipDetect | PinTipDetection,
 ) -> Generator[Msg, None, Pixel]:
     yield from bps.trigger(pin_tip, wait=True)
-    tip_x_y_px = yield from bps.rd(pin_tip)
+    tip_x_y_px = yield from bps.rd(pin_tip.triggered_tip)
     LOGGER.info(f"Pin tip found at {tip_x_y_px}")
     return tip_x_y_px  # type: ignore
 

tests/unit_tests/device_setup_plans/test_setup_oav.py

@@ -1,13 +1,15 @@
 from functools import partial
 from unittest.mock import AsyncMock, MagicMock, patch
 
+import numpy as np
 import pytest
 from bluesky import plan_stubs as bps
 from bluesky.run_engine import RunEngine
 from dodal.beamlines import i03
 from dodal.devices.oav.oav_detector import OAV, OAVConfigParams
 from dodal.devices.oav.oav_parameters import OAVParameters
 from dodal.devices.oav.pin_image_recognition import PinTipDetection
+from dodal.devices.oav.pin_image_recognition.utils import SampleLocation
 from dodal.devices.smargon import Smargon
 from ophyd.signal import Signal
 from ophyd.sim import instantiate_fake_device
@@ -91,7 +93,7 @@ def test_when_set_up_oav_with_different_zoom_levels_then_flat_field_applied_corr
     RE = RunEngine()
     RE(pre_centring_setup_oav(oav, mock_parameters, ophyd_pin_tip_detection))
     assert oav.mxsc.input_plugin.get() == expected_plugin
-    assert oav.snapshot.input_plugin.get() == "OAV.MXSC"
+    assert oav.snapshot.input_plugin.get() == expected_plugin
 
 
 @pytest.mark.parametrize(
@@ -157,11 +159,13 @@ async def test_given_tip_found_when_wait_for_tip_to_be_found_called_then_tip_imm
         PinTipDetection, name="pin_detect"
     )
     await mock_pin_tip_detect.connect(sim=True)
-    mock_pin_tip_detect._get_tip_position = AsyncMock(return_value=(100, 100))
+    mock_pin_tip_detect._get_tip_and_edge_data = AsyncMock(
+        return_value=SampleLocation(100, 100, np.array([]), np.array([]))
+    )
     RE = RunEngine(call_returns_result=True)
     result = RE(wait_for_tip_to_be_found(mock_pin_tip_detect))
     assert result.plan_result == (100, 100)  # type: ignore
-    mock_pin_tip_detect._get_tip_position.assert_called_once()
+    mock_pin_tip_detect._get_tip_and_edge_data.assert_called_once()
 
 
 @pytest.mark.asyncio
@@ -171,8 +175,10 @@ async def test_given_no_tip_when_wait_for_tip_to_be_found_called_then_exception_
     )
     await mock_pin_tip_detect.connect(sim=True)
     await mock_pin_tip_detect.validity_timeout.set(0.2)
-    mock_pin_tip_detect._get_tip_position = AsyncMock(
-        return_value=(PinTipDetection.INVALID_POSITION)
+    mock_pin_tip_detect._get_tip_and_edge_data = AsyncMock(
+        return_value=SampleLocation(
+            *PinTipDetection.INVALID_POSITION, np.array([]), np.array([])
+        )
     )
     RE = RunEngine(call_returns_result=True)
     with pytest.raises(WarningException):

tests/unit_tests/experiment_plans/test_grid_detection_plan.py

@@ -1,16 +1,17 @@
-from unittest.mock import MagicMock, patch
+from unittest.mock import AsyncMock, MagicMock, patch
 
-import bluesky.plan_stubs as bps
+import numpy as np
 import pytest
 from bluesky.run_engine import RunEngine
 from bluesky.utils import Msg
 from dodal.beamlines import i03
 from dodal.devices.backlight import Backlight
 from dodal.devices.fast_grid_scan import GridAxis
-from dodal.devices.oav.oav_detector import OAV, OAVConfigParams
+from dodal.devices.oav.oav_detector import OAVConfigParams
 from dodal.devices.oav.oav_parameters import OAVParameters
+from dodal.devices.oav.pin_image_recognition import PinTipDetection
+from dodal.devices.oav.pin_image_recognition.utils import SampleLocation
 from dodal.devices.smargon import Smargon
-from ophyd.sim import NullStatus
 
 from hyperion.exceptions import WarningException
 from hyperion.experiment_plans.oav_grid_detection_plan import (
@@ -44,6 +45,14 @@ def fake_devices(RE, smargon: Smargon, backlight: Backlight, test_config_files):
     oav.wait_for_connection()
 
     pin_tip_detection = i03.pin_tip_detection(fake_with_ophyd_sim=True)
+    pin_tip_detection._get_tip_and_edge_data = AsyncMock(
+        return_value=SampleLocation(
+            8,
+            5,
+            np.array([0, 0, 0, 0, 0, 0, 0, 0, 5, 5, 4, 4, 3, 3, 2, 2, 3, 3, 4, 4]),
+            np.array([0, 0, 0, 0, 0, 0, 0, 0, 5, 5, 6, 6, 7, 7, 8, 8, 7, 7, 6, 6]),
+        )
+    )
 
     oav.zoom_controller.zrst.set("1.0x")
     oav.zoom_controller.onst.set("2.0x")
@@ -52,14 +61,6 @@ def fake_devices(RE, smargon: Smargon, backlight: Backlight, test_config_files):
     oav.zoom_controller.frst.set("7.0x")
     oav.zoom_controller.fvst.set("9.0x")
 
-    # fmt: off
-    oav.mxsc.bottom.set([0,0,0,0,0,0,0,0,5,5,6,6,7,7,8,8,7,7,6,6])  # noqa: E231
-    oav.mxsc.top.set([0,0,0,0,0,0,0,0,5,5,4,4,3,3,2,2,3,3,4,4])  # noqa: E231
-    # fmt: on
-
-    oav.mxsc.pin_tip.triggered_tip.put((8, 5))
-    oav.mxsc.pin_tip.trigger = MagicMock(return_value=NullStatus())
-
     with patch("dodal.devices.areadetector.plugins.MJPG.requests"), patch(
         "dodal.devices.areadetector.plugins.MJPG.Image"
     ) as mock_image_class:
@@ -111,16 +112,23 @@ def test_grid_detection_plan_runs_and_triggers_snapshots(
 
 @patch("dodal.beamlines.beamline_utils.active_device_is_same_type", lambda a, b: True)
 @patch("bluesky.plan_stubs.sleep", new=MagicMock())
-def test_grid_detection_plan_gives_warningerror_if_tip_not_found(
+@pytest.mark.asyncio
+async def test_grid_detection_plan_gives_warningerror_if_tip_not_found(
     RE,
     test_config_files,
-    fake_devices,
+    fake_devices: tuple[OavGridDetectionComposite, MagicMock],
 ):
     composite, _ = fake_devices
-    oav: OAV = composite.oav
 
-    oav.mxsc.pin_tip.triggered_tip.put((-1, -1))
-    oav.mxsc.pin_tip.validity_timeout.put(0.01)
+    await composite.pin_tip_detection.validity_timeout._backend.put(0.01)
+    composite.pin_tip_detection._get_tip_and_edge_data = AsyncMock(
+        return_value=SampleLocation(
+            *PinTipDetection.INVALID_POSITION,
+            np.array([]),
+            np.array([]),
+        )
+    )
+
     params = OAVParameters("loopCentring", test_config_files["oav_config_json"])
 
     with pytest.raises(WarningException) as excinfo:
@@ -144,13 +152,13 @@ def test_given_when_grid_detect_then_upper_left_and_start_position_as_expected(
     test_config_files,
 ):
     params = OAVParameters("loopCentring", test_config_files["oav_config_json"])
-    box_size_microns = 0.2
+    box_size_um = 0.2
     composite, _ = fake_devices
     composite.oav.parameters.micronsPerXPixel = 0.1
     composite.oav.parameters.micronsPerYPixel = 0.1
     composite.oav.parameters.beam_centre_i = 4
     composite.oav.parameters.beam_centre_j = 4
-    box_size_y_pixels = box_size_microns / composite.oav.parameters.micronsPerYPixel
+    box_size_y_pixels = box_size_um / composite.oav.parameters.micronsPerYPixel
 
     oav_cb = OavSnapshotCallback()
     grid_param_cb = GridDetectionCallback(composite.oav.parameters, 0.004, False)
@@ -163,7 +171,7 @@ def test_given_when_grid_detect_then_upper_left_and_start_position_as_expected(
             snapshot_dir="tmp",
             snapshot_template="test_{angle}",
             grid_width_microns=161.2,
-            box_size_microns=0.2,
+            box_size_um=0.2,
         )
     )
 
@@ -224,7 +232,7 @@ def test_when_grid_detection_plan_run_then_grid_detection_callback_gets_correct_
 ):
     params = OAVParameters("loopCentring", test_config_files["oav_config_json"])
     composite, _ = fake_devices
-    box_size_microns = 20
+    box_size_um = 20
     cb = GridDetectionCallback(composite.oav.parameters, 0.5, True)
     RE.subscribe(cb)
 
@@ -253,12 +261,8 @@ def test_when_grid_detection_plan_run_then_grid_detection_callback_gets_correct_
     )
 
     assert my_grid_params.x_start == pytest.approx(-0.7942199999999999)
-    assert my_grid_params.y1_start == pytest.approx(
-        -0.53984 - (box_size_microns * 1e-3 / 2)
-    )
-    assert my_grid_params.y2_start == pytest.approx(
-        -0.53984 - (box_size_microns * 1e-3 / 2)
-    )
+    assert my_grid_params.y1_start == pytest.approx(-0.53984 - (box_size_um * 1e-3 / 2))
+    assert my_grid_params.y2_start == pytest.approx(-0.53984 - (box_size_um * 1e-3 / 2))
     assert my_grid_params.z1_start == pytest.approx(-0.53984)
     assert my_grid_params.z2_start == pytest.approx(-0.53984)
     assert my_grid_params.x_step_size == pytest.approx(0.02)
@@ -284,11 +288,9 @@ def test_when_grid_detection_plan_run_then_grid_detection_callback_gets_correct_
 )
 @patch("dodal.beamlines.beamline_utils.active_device_is_same_type", lambda a, b: True)
 @patch("bluesky.plan_stubs.sleep", new=MagicMock())
-@patch("hyperion.experiment_plans.oav_grid_detection_plan.wait_for_tip_to_be_found")
 @patch("hyperion.experiment_plans.oav_grid_detection_plan.LOGGER")
 def test_when_detected_grid_has_odd_y_steps_then_add_a_y_step_and_shift_grid(
     fake_logger: MagicMock,
-    fake_wait_for_tip: MagicMock,
     fake_devices,
     test_config_files,
     odd,
@@ -297,26 +299,20 @@ def test_when_detected_grid_has_odd_y_steps_then_add_a_y_step_and_shift_grid(
     sim = RunEngineSimulator()
     params = OAVParameters("loopCentring", test_config_files["oav_config_json"])
     grid_width_microns = 161.2
-    box_size_microns = 20
-    box_size_y_pixels = box_size_microns / composite.oav.parameters.micronsPerYPixel
+    box_size_um = 20
+    box_size_y_pixels = box_size_um / composite.oav.parameters.micronsPerYPixel
     initial_min_y = 1
 
-    tip_x_y = (8, 5)
-
-    def wait_for_tip(_):
-        yield from bps.null()
-        return tip_x_y
-
-    fake_wait_for_tip.side_effect = wait_for_tip
-
     abs_sets: dict[str, list] = {"snapshot.top_left_y": [], "snapshot.num_boxes_y": []}
 
     def handle_read(msg: Msg):
-        if msg.obj.dotted_name == "mxsc.top":
+        if msg.obj.name == "pin_tip_detection-triggered_tip":
+            return {"values": {"value": (8, 5)}}
+        if msg.obj.name == "pin_tip_detection-triggered_top_edge":
             top_edge = [0] * 20
             top_edge[19] = initial_min_y
             return {"values": {"value": top_edge}}
-        elif msg.obj.dotted_name == "mxsc.bottom":
+        elif msg.obj.name == "pin_tip_detection-triggered_bottom_edge":
             bottom_edge = [0] * 20
             bottom_edge[19] = (
                 10 if odd else 25
@@ -326,8 +322,9 @@ def handle_read(msg: Msg):
             pass
 
     def record_set(msg: Msg):
-        if msg.obj.dotted_name in abs_sets.keys():
-            abs_sets[msg.obj.dotted_name].append(msg.args[0])
+        if hasattr(msg.obj, "dotted_name"):
+            if msg.obj.dotted_name in abs_sets.keys():
+                abs_sets[msg.obj.dotted_name].append(msg.args[0])
 
     sim.add_handler(
         "set",