(DiamondLightSource/hyperion#1069) Use the trigger functionality in t…

…he ophyd pin detection in the same way as the AD plugin

setup.cfg

@@ -35,7 +35,7 @@ install_requires =
     xarray
     doct
     databroker
-    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@328330036a23cae8748e84a26eb9b77ad113f8a5
+    dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@bede031f673c3468fe57ede1ef21905bbb74e6d5
     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

@@ -4,7 +4,7 @@
 import bluesky.plan_stubs as bps
 import numpy as np
 from bluesky.utils import Msg
-from dodal.devices.areadetector.plugins.MXSC import MXSC
+from dodal.devices.areadetector.plugins.MXSC import MXSC, PinTipDetect
 from dodal.devices.oav.oav_calculations import camera_coordinates_to_xyz
 from dodal.devices.oav.oav_detector import OAV, OAVConfigParams
 from dodal.devices.oav.oav_errors import OAVError_ZoomLevelNotFound
@@ -174,37 +174,13 @@ def get_move_required_so_that_beam_is_at_pixel(
     return calculate_x_y_z_of_pixel(current_motor_xyz, current_angle, pixel, oav_params)
 
 
-def wait_for_tip_to_be_found_ad_mxsc(
-    mxsc: MXSC,
-) -> Generator[Msg, None, Tuple[int, int]]:
-    pin_tip = mxsc.pin_tip
-    yield from bps.trigger(pin_tip, wait=True)
-    found_tip = yield from bps.rd(pin_tip)
-    if found_tip == pin_tip.INVALID_POSITION:
-        top_edge = yield from bps.rd(mxsc.top)
-        bottom_edge = yield from bps.rd(mxsc.bottom)
-        LOGGER.info(
-            f"No tip found with top/bottom of {list(top_edge), list(bottom_edge)}"
-        )
-        raise WarningException(
-            f"No pin found after {pin_tip.validity_timeout.get()} seconds"
-        )
-    return found_tip
-
-
-def wait_for_tip_to_be_found_ophyd(
-    ophyd_pin_tip_detection: PinTipDetection,
-) -> Generator[Msg, None, Tuple[int, int]]:
+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)
-
-    LOGGER.info("Pin tip not found, waiting a second and trying again")
-
-    if found_tip == ophyd_pin_tip_detection.INVALID_POSITION:
-        # Wait a second and then retry
-        yield from bps.sleep(1)
-        found_tip = yield from bps.rd(ophyd_pin_tip_detection)
-
     if found_tip == ophyd_pin_tip_detection.INVALID_POSITION:
-        raise WarningException("No pin found")
+        timeout = yield from bps.rd(ophyd_pin_tip_detection.validity_timeout)
+        raise WarningException(f"No pin found after {timeout} seconds")
 
     return found_tip  # type: ignore

src/hyperion/experiment_plans/oav_grid_detection_plan.py

@@ -17,7 +17,7 @@
 from hyperion.device_setup_plans.setup_oav import (
     get_move_required_so_that_beam_is_at_pixel,
     pre_centring_setup_oav,
-    wait_for_tip_to_be_found_ad_mxsc,
+    wait_for_tip_to_be_found,
 )
 from hyperion.log import LOGGER
 from hyperion.parameters.constants import (
@@ -114,7 +114,7 @@ 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_ad_mxsc(oav.mxsc)
+        tip_x_px, tip_y_px = yield from wait_for_tip_to_be_found(oav.mxsc.pin_tip)
 
         LOGGER.info(f"Tip is at x,y: {tip_x_px},{tip_y_px}")
 

src/hyperion/experiment_plans/pin_tip_centring_plan.py

@@ -5,7 +5,7 @@
 import numpy as np
 from blueapi.core import BlueskyContext
 from bluesky.utils import Msg
-from dodal.devices.areadetector.plugins.MXSC import MXSC, PinTipDetect
+from dodal.devices.areadetector.plugins.MXSC import PinTipDetect
 from dodal.devices.backlight import Backlight
 from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.oav_parameters import OAV_CONFIG_JSON, OAVParameters
@@ -16,8 +16,7 @@
     Pixel,
     get_move_required_so_that_beam_is_at_pixel,
     pre_centring_setup_oav,
-    wait_for_tip_to_be_found_ad_mxsc,
-    wait_for_tip_to_be_found_ophyd,
+    wait_for_tip_to_be_found,
 )
 from hyperion.exceptions import WarningException
 from hyperion.log import LOGGER
@@ -44,17 +43,8 @@ def create_devices(context: BlueskyContext) -> PinTipCentringComposite:
 def trigger_and_return_pin_tip(
     pin_tip: PinTipDetect | PinTipDetection,
 ) -> Generator[Msg, None, Pixel]:
-    if isinstance(pin_tip, PinTipDetection):
-        tip_x_y_px = yield from bps.rd(pin_tip)
-
-        if tip_x_y_px == pin_tip.INVALID_POSITION:
-            # Wait a second and then retry
-            LOGGER.info("Pin tip not found, waiting a second and trying again")
-            yield from bps.sleep(1)
-            tip_x_y_px = yield from bps.rd(pin_tip)
-    else:
-        yield from bps.trigger(pin_tip, wait=True)
-        tip_x_y_px = yield from bps.rd(pin_tip)
+    yield from bps.trigger(pin_tip, wait=True)
+    tip_x_y_px = yield from bps.rd(pin_tip)
     LOGGER.info(f"Pin tip found at {tip_x_y_px}")
     return tip_x_y_px  # type: ignore
 
@@ -197,10 +187,5 @@ def offset_and_move(tip: Pixel):
     # See #673 for improvements
     yield from bps.sleep(0.3)
 
-    if isinstance(pin_tip_setup, MXSC):
-        LOGGER.info("Acquiring pin-tip from AD MXSC plugin")
-        tip = yield from wait_for_tip_to_be_found_ad_mxsc(pin_tip_setup)
-    elif isinstance(pin_tip_setup, PinTipDetection):
-        LOGGER.info("Acquiring pin-tip from ophyd device")
-        tip = yield from wait_for_tip_to_be_found_ophyd(pin_tip_setup)
+    tip = yield from wait_for_tip_to_be_found(pin_tip_detect)
     yield from offset_and_move(tip)

tests/unit_tests/device_setup_plans/test_setup_oav.py

@@ -16,7 +16,7 @@
 from hyperion.device_setup_plans.setup_oav import (
     get_move_required_so_that_beam_is_at_pixel,
     pre_centring_setup_oav,
-    wait_for_tip_to_be_found_ophyd,
+    wait_for_tip_to_be_found,
 )
 from hyperion.exceptions import WarningException
 
@@ -152,55 +152,28 @@ def my_plan():
 
 
 @pytest.mark.asyncio
-@patch("hyperion.device_setup_plans.setup_oav.bps.sleep", autospec=True)
-async def test_given_tip_found_when_wait_for_tip_to_be_found_ophyd_called_then_tip_immediately_returned(
-    mock_sleep: MagicMock,
-):
+async def test_given_tip_found_when_wait_for_tip_to_be_found_called_then_tip_immediately_returned():
     mock_pin_tip_detect: PinTipDetection = instantiate_fake_device(
         PinTipDetection, name="pin_detect"
     )
     await mock_pin_tip_detect.connect(sim=True)
-    mock_pin_tip_detect._get_tip_position = AsyncMock(return_value=((100, 100), 0))
+    mock_pin_tip_detect._get_tip_position = AsyncMock(return_value=(100, 100))
     RE = RunEngine(call_returns_result=True)
-    result = RE(wait_for_tip_to_be_found_ophyd(mock_pin_tip_detect))
+    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_sleep.assert_not_called()
-
-
-@pytest.mark.asyncio
-@patch("hyperion.device_setup_plans.setup_oav.bps.sleep", autospec=True)
-async def test_given_no_tip_at_first_when_wait_for_tip_to_be_found_ophyd_called_then_tip_returned_after_wait(
-    mock_sleep: MagicMock,
-):
-    mock_pin_tip_detect: PinTipDetection = instantiate_fake_device(
-        PinTipDetection, name="pin_detect"
-    )
-    await mock_pin_tip_detect.connect(sim=True)
-    mock_pin_tip_detect._get_tip_position = AsyncMock(
-        side_effect=[(PinTipDetection.INVALID_POSITION, 0), (((100, 100), 0))]
-    )
-    RE = RunEngine(call_returns_result=True)
-    result = RE(wait_for_tip_to_be_found_ophyd(mock_pin_tip_detect))
-    assert result.plan_result == (100, 100)  # type: ignore
-    assert mock_pin_tip_detect._get_tip_position.call_count == 2
-    mock_sleep.assert_called_once()
 
 
 @pytest.mark.asyncio
-@patch("hyperion.device_setup_plans.setup_oav.bps.sleep", autospec=True)
-async def test_given_no_tip_when_wait_for_tip_to_be_found_ophyd_called_then_exception_thrown(
-    mock_sleep: MagicMock,
-):
+async def test_given_no_tip_when_wait_for_tip_to_be_found_called_then_exception_thrown():
     mock_pin_tip_detect: PinTipDetection = instantiate_fake_device(
         PinTipDetection, name="pin_detect"
     )
     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, 0)
+        return_value=(PinTipDetection.INVALID_POSITION)
     )
     RE = RunEngine(call_returns_result=True)
     with pytest.raises(WarningException):
-        RE(wait_for_tip_to_be_found_ophyd(mock_pin_tip_detect))
-    assert mock_pin_tip_detect._get_tip_position.call_count == 2
-    mock_sleep.assert_called_once()
+        RE(wait_for_tip_to_be_found(mock_pin_tip_detect))

tests/unit_tests/experiment_plans/test_grid_detection_plan.py

@@ -284,9 +284,7 @@ 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_ad_mxsc"
-)
+@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,

tests/unit_tests/experiment_plans/test_pin_tip_centring.py

@@ -104,7 +104,7 @@ def test_trigger_and_return_pin_tip_works_for_AD_pin_tip_detection(
 def test_trigger_and_return_pin_tip_works_for_ophyd_pin_tip_detection(
     ophyd_pin_tip_detection: PinTipDetection, RE: RunEngine
 ):
-    ophyd_pin_tip_detection._get_tip_position = AsyncMock(return_value=((100, 200), 0))
+    ophyd_pin_tip_detection._get_tip_position = AsyncMock(return_value=(100, 200))
     re_result = RE(trigger_and_return_pin_tip(ophyd_pin_tip_detection))
     assert re_result.plan_result == (100, 200)  # type: ignore
 
@@ -182,7 +182,7 @@ def return_pixel(pixel, *args):
 
 
 @patch(
-    "hyperion.experiment_plans.pin_tip_centring_plan.wait_for_tip_to_be_found_ad_mxsc",
+    "hyperion.experiment_plans.pin_tip_centring_plan.wait_for_tip_to_be_found",
     new=partial(return_pixel, (200, 200)),
 )
 @patch(
@@ -241,7 +241,7 @@ def test_when_pin_tip_centre_plan_called_then_expected_plans_called(
 
 
 @patch(
-    "hyperion.experiment_plans.pin_tip_centring_plan.wait_for_tip_to_be_found_ophyd",
+    "hyperion.experiment_plans.pin_tip_centring_plan.wait_for_tip_to_be_found",
     new=partial(return_pixel, (200, 200)),
 )
 @patch(