Take snapshots before robot load

setup.cfg

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

src/hyperion/experiment_plans/robot_load_then_centre_plan.py

@@ -2,6 +2,7 @@
 
 import dataclasses
 import json
+from datetime import datetime
 from typing import cast
 
 import bluesky.plan_stubs as bps
@@ -26,6 +27,7 @@
 from dodal.devices.synchrotron import Synchrotron
 from dodal.devices.undulator import Undulator
 from dodal.devices.undulator_dcm import UndulatorDCM
+from dodal.devices.webcam import Webcam
 from dodal.devices.xbpm_feedback import XBPMFeedback
 from dodal.devices.zebra import Zebra
 from dodal.devices.zocalo import ZocaloResults
@@ -88,6 +90,7 @@ class RobotLoadThenCentreComposite:
 
     # RobotLoad fields
     robot: BartRobot
+    webcam: Webcam
 
 
 def create_devices(context: BlueskyContext) -> RobotLoadThenCentreComposite:
@@ -115,6 +118,18 @@ def wait_for_smargon_not_disabled(smargon: Smargon, timeout=60):
     )
 
 
+def take_robot_snapshots(oav: OAV, webcam: Webcam, directory: str):
+    time_now = datetime.now()
+    snapshot_format = f"{time_now.strftime('%H%M%S')}_{{device}}_after_load"
+    for device in [oav.snapshot, webcam]:
+        yield from bps.abs_set(
+            device.filename, snapshot_format.format(device=device.name)
+        )
+        yield from bps.abs_set(device.directory, directory)
+        yield from bps.trigger(device, group="snapshots")
+    yield from bps.wait("snapshots")
+
+
 def prepare_for_robot_load(composite: RobotLoadThenCentreComposite):
     yield from bps.abs_set(
         composite.aperture_scatterguard,
@@ -174,8 +189,14 @@ def robot_load():
 
         yield from bps.wait("robot_load")
 
+        yield from take_robot_snapshots(
+            composite.oav, composite.webcam, parameters.experiment_params.snapshot_dir
+        )
+
         yield from bps.create(name=CONST.DESCRIPTORS.ROBOT_LOAD)
         yield from bps.read(composite.robot.barcode)
+        yield from bps.read(composite.oav.snapshot)
+        yield from bps.read(composite.webcam)
         yield from bps.save()
 
         yield from wait_for_smargon_not_disabled(composite.smargon)

src/hyperion/external_interaction/callbacks/robot_load/ispyb_callback.py

@@ -62,7 +62,12 @@ def activity_gated_event(self, doc: Event) -> Event | None:
                 self.action_id is not None
             ), "ISPyB Robot load callback event called unexpectedly"
             barcode = doc["data"]["robot-barcode"]
-            self.expeye.update_barcode(self.action_id, barcode)
+            oav_snapshot = doc["data"]["oav_snapshot_last_saved_path"]
+            webcam_snapshot = doc["data"]["webcam-last_saved_path"]
+            # I03 uses oav/webcam snapshots in place of before/after snapshots
+            self.expeye.update_barcode_and_snapshots(
+                self.action_id, barcode, oav_snapshot, webcam_snapshot
+            )
 
         return super().activity_gated_event(doc)
 

src/hyperion/external_interaction/ispyb/exp_eye_store.py

@@ -81,16 +81,28 @@ def start_load(
         response = self._send_and_get_response(url, data, post)
         return response["robotActionId"]
 
-    def update_barcode(self, action_id: RobotActionID, barcode: str):
-        """Update the barcode of an existing robot action.
+    def update_barcode_and_snapshots(
+        self,
+        action_id: RobotActionID,
+        barcode: str,
+        snapshot_before_path: str,
+        snapshot_after_path: str,
+    ):
+        """Update the barcode and snapshots of an existing robot action.
 
         Args:
             action_id (RobotActionID): The id of the action to update
             barcode (str): The barcode to give the action
+            snapshot_before_path (str): Path to the snapshot before robot load
+            snapshot_after_path (str): Path to the snapshot after robot load
         """
         url = self.base_url + self.UPDATE_ROBOT_ACTION.format(action_id=action_id)
 
-        data = {"sampleBarcode": barcode}
+        data = {
+            "sampleBarcode": barcode,
+            "xtalSnapshotBefore": snapshot_before_path,
+            "xtalSnapshotAfter": snapshot_after_path,
+        }
         self._send_and_get_response(url, data, patch)
 
     def end_load(self, action_id: RobotActionID, status: str, reason: str):

tests/conftest.py

@@ -3,7 +3,7 @@
 import sys
 import threading
 from functools import partial
-from typing import Callable, Generator, Optional, Sequence
+from typing import Any, Callable, Generator, Optional, Sequence
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -31,10 +31,12 @@
 from dodal.devices.smargon import Smargon
 from dodal.devices.synchrotron import Synchrotron, SynchrotronMode
 from dodal.devices.undulator import Undulator
+from dodal.devices.webcam import Webcam
 from dodal.devices.zebra import Zebra
 from dodal.log import LOGGER as dodal_logger
 from dodal.log import set_up_all_logging_handlers
 from ophyd.epics_motor import EpicsMotor
+from ophyd.sim import NullStatus
 from ophyd.status import DeviceStatus, Status
 from ophyd_async.core import set_sim_value
 from ophyd_async.core.async_status import AsyncStatus
@@ -314,7 +316,9 @@ def synchrotron():
 
 @pytest.fixture
 def oav():
-    return i03.oav(fake_with_ophyd_sim=True)
+    oav = i03.oav(fake_with_ophyd_sim=True)
+    oav.snapshot.trigger = MagicMock(return_value=NullStatus())
+    return oav
 
 
 @pytest.fixture
@@ -405,6 +409,13 @@ def undulator_dcm():
     beamline_utils.clear_devices()
 
 
+@pytest.fixture
+def webcam(RE) -> Generator[Webcam, Any, Any]:
+    webcam = i03.webcam(fake_with_ophyd_sim=True)
+    with patch.object(webcam, "_write_image"):
+        yield webcam
+
+
 @pytest.fixture
 def aperture_scatterguard(done_status):
     AperturePositions.LARGE = SingleAperturePosition(

tests/system_tests/external_interaction/test_exp_eye_dev.py

@@ -10,6 +10,10 @@
 
 @pytest.mark.s03
 def test_start_and_end_robot_load():
+    """To confirm this test is successful go to
+    https://ispyb-test.diamond.ac.uk/dc/visit/cm37235-2 and see that data is added
+    when it's run.
+    """
     os.environ["ISPYB_CONFIG_PATH"] = CONST.SIM.DEV_ISPYB_DATABASE_CFG
 
     SAMPLE_ID = 5289780
@@ -23,7 +27,12 @@ def test_start_and_end_robot_load():
 
     print(f"Created {robot_action_id}")
 
-    expeye.update_barcode(robot_action_id, BARCODE)
+    test_folder = "/dls/i03/data/2024/cm37235-2/xtal_snapshots"
+    oav_snapshot = test_folder + "/235855_load_after_0.0.png"
+    webcam_snapshot = test_folder + "/235855_webcam.jpg"
+    expeye.update_barcode_and_snapshots(
+        robot_action_id, BARCODE, oav_snapshot, webcam_snapshot
+    )
 
     sleep(0.5)
 

tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py

@@ -5,14 +5,18 @@
 from bluesky.utils import Msg
 from dodal.devices.aperturescatterguard import AperturePositions
 from dodal.devices.eiger import EigerDetector
+from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.smargon import Smargon, StubPosition
+from dodal.devices.webcam import Webcam
 from numpy import isclose
 from ophyd.sim import NullStatus, instantiate_fake_device
+from ophyd_async.core import set_sim_value
 
 from hyperion.experiment_plans.robot_load_then_centre_plan import (
     RobotLoadThenCentreComposite,
     prepare_for_robot_load,
     robot_load_then_centre,
+    take_robot_snapshots,
 )
 from hyperion.external_interaction.callbacks.robot_load.ispyb_callback import (
     RobotLoadISPyBCallback,
@@ -29,7 +33,7 @@
 
 @pytest.fixture
 def robot_load_composite(
-    smargon, dcm, robot, aperture_scatterguard
+    smargon, dcm, robot, aperture_scatterguard, oav, webcam
 ) -> RobotLoadThenCentreComposite:
     composite: RobotLoadThenCentreComposite = MagicMock()
     composite.smargon = smargon
@@ -39,6 +43,8 @@ def robot_load_composite(
     composite.aperture_scatterguard = aperture_scatterguard
     composite.smargon.stub_offsets.set = MagicMock(return_value=NullStatus())
     composite.aperture_scatterguard.set = MagicMock(return_value=NullStatus())
+    composite.oav = oav
+    composite.webcam = webcam
     return composite
 
 
@@ -305,7 +311,7 @@ def test_when_prepare_for_robot_load_called_then_moves_as_expected(
     "hyperion.external_interaction.callbacks.robot_load.ispyb_callback.ExpeyeInteraction.end_load"
 )
 @patch(
-    "hyperion.external_interaction.callbacks.robot_load.ispyb_callback.ExpeyeInteraction.update_barcode"
+    "hyperion.external_interaction.callbacks.robot_load.ispyb_callback.ExpeyeInteraction.update_barcode_and_snapshots"
 )
 @patch(
     "hyperion.external_interaction.callbacks.robot_load.ispyb_callback.ExpeyeInteraction.start_load"
@@ -320,11 +326,15 @@ def test_when_prepare_for_robot_load_called_then_moves_as_expected(
 def test_given_ispyb_callback_attached_when_robot_load_then_centre_plan_called_then_ispyb_deposited(
     mock_centring_plan: MagicMock,
     start_load: MagicMock,
-    update_barcode: MagicMock,
+    update_barcode_and_snapshots: MagicMock,
     end_load: MagicMock,
     robot_load_composite: RobotLoadThenCentreComposite,
     robot_load_then_centre_params: RobotLoadThenCentreInternalParameters,
 ):
+    robot_load_composite.oav.snapshot.last_saved_path.put("test_oav_snapshot")  # type: ignore
+    set_sim_value(robot_load_composite.webcam.last_saved_path, "test_webcam_snapshot")
+    robot_load_composite.webcam.trigger = MagicMock(return_value=NullStatus())
+
     RE = RunEngine()
     RE.subscribe(RobotLoadISPyBCallback())
 
@@ -334,5 +344,30 @@ def test_given_ispyb_callback_attached_when_robot_load_then_centre_plan_called_t
     RE(robot_load_then_centre(robot_load_composite, robot_load_then_centre_params))
 
     start_load.assert_called_once_with("cm31105", 4, "12345", 40, 3)
-    update_barcode.assert_called_once_with(action_id, "BARCODE")
+    update_barcode_and_snapshots.assert_called_once_with(
+        action_id, "BARCODE", "test_oav_snapshot", "test_webcam_snapshot"
+    )
     end_load.assert_called_once_with(action_id, "success", "")
+
+
+@patch("hyperion.experiment_plans.robot_load_then_centre_plan.datetime")
+async def test_when_take_snapshots_called_then_filename_and_directory_set_and_device_triggered(
+    mock_datetime: MagicMock, oav: OAV, webcam: Webcam
+):
+    TEST_DIRECTORY = "TEST"
+
+    mock_datetime.now.return_value.strftime.return_value = "TIME"
+
+    RE = RunEngine()
+    oav.snapshot.trigger = MagicMock(side_effect=oav.snapshot.trigger)
+    webcam.trigger = MagicMock(return_value=NullStatus())
+
+    RE(take_robot_snapshots(oav, webcam, TEST_DIRECTORY))
+
+    oav.snapshot.trigger.assert_called_once()
+    assert oav.snapshot.filename.get() == "TIME_oav_snapshot_after_load"
+    assert oav.snapshot.directory.get() == TEST_DIRECTORY
+
+    webcam.trigger.assert_called_once()
+    assert (await webcam.filename.get_value()) == "TIME_webcam_after_load"
+    assert (await webcam.directory.get_value()) == TEST_DIRECTORY

tests/unit_tests/external_interaction/callbacks/robot_load/test_robot_load_ispyb_callback.py

@@ -4,7 +4,10 @@
 import bluesky.preprocessors as bpp
 import pytest
 from bluesky.run_engine import RunEngine
+from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.robot import BartRobot
+from dodal.devices.webcam import Webcam
+from ophyd_async.core import set_sim_value
 
 from hyperion.external_interaction.callbacks.robot_load.ispyb_callback import (
     RobotLoadISPyBCallback,
@@ -100,26 +103,35 @@ def test_given_end_called_but_no_start_then_exception_raised(end_load):
     "hyperion.external_interaction.callbacks.robot_load.ispyb_callback.ExpeyeInteraction.start_load"
 )
 @patch(
-    "hyperion.external_interaction.callbacks.robot_load.ispyb_callback.ExpeyeInteraction.update_barcode"
+    "hyperion.external_interaction.callbacks.robot_load.ispyb_callback.ExpeyeInteraction.update_barcode_and_snapshots"
 )
 def test_given_plan_reads_barcode_then_data_put_in_ispyb(
-    update_barcode: MagicMock,
+    update_barcode_and_snapshots: MagicMock,
     start_load: MagicMock,
     end_load: MagicMock,
     robot: BartRobot,
+    oav: OAV,
+    webcam: Webcam,
 ):
     RE = RunEngine()
     RE.subscribe(RobotLoadISPyBCallback())
     start_load.return_value = ACTION_ID
 
+    oav.snapshot.last_saved_path.put("test_oav_snapshot")  # type: ignore
+    set_sim_value(webcam.last_saved_path, "test_webcam_snapshot")
+
     @bpp.run_decorator(md=metadata)
     def my_plan():
         yield from bps.create(name=CONST.DESCRIPTORS.ROBOT_LOAD)
         yield from bps.read(robot.barcode)
+        yield from bps.read(oav.snapshot)
+        yield from bps.read(webcam)
         yield from bps.save()
 
     RE(my_plan())
 
     start_load.assert_called_once_with("cm31105", 4, SAMPLE_ID, SAMPLE_PUCK, SAMPLE_PIN)
-    update_barcode.assert_called_once_with(ACTION_ID, "BARCODE")
+    update_barcode_and_snapshots.assert_called_once_with(
+        ACTION_ID, "BARCODE", "test_oav_snapshot", "test_webcam_snapshot"
+    )
     end_load.assert_called_once_with(ACTION_ID, "success", "")

tests/unit_tests/external_interaction/ispyb/test_expeye_interaction.py

@@ -125,11 +125,15 @@ def test_when_update_barcode_called_with_success_then_correct_expected_url_poste
     mock_patch,
 ):
     expeye_interactor = ExpeyeInteraction()
-    expeye_interactor.update_barcode(3, "test")
+    expeye_interactor.update_barcode_and_snapshots(
+        3, "test", "/tmp/before.jpg", "/tmp/after.jpg"
+    )
 
     mock_patch.assert_called_once()
     assert mock_patch.call_args.args[0] == "http://blah/core/robot-actions/3"
     expected_data = {
         "sampleBarcode": "test",
+        "xtalSnapshotBefore": "/tmp/before.jpg",
+        "xtalSnapshotAfter": "/tmp/after.jpg",
     }
     assert mock_patch.call_args.kwargs["json"] == expected_data