diff --git a/setup.cfg b/setup.cfg index bd7f67afd..7d09abc96 100644 --- a/setup.cfg +++ b/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 diff --git a/src/hyperion/experiment_plans/robot_load_then_centre_plan.py b/src/hyperion/experiment_plans/robot_load_then_centre_plan.py index 7fad38041..dccd7096a 100644 --- a/src/hyperion/experiment_plans/robot_load_then_centre_plan.py +++ b/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) diff --git a/src/hyperion/external_interaction/callbacks/robot_load/ispyb_callback.py b/src/hyperion/external_interaction/callbacks/robot_load/ispyb_callback.py index 861cf3b92..bd4d36152 100644 --- a/src/hyperion/external_interaction/callbacks/robot_load/ispyb_callback.py +++ b/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) diff --git a/src/hyperion/external_interaction/ispyb/exp_eye_store.py b/src/hyperion/external_interaction/ispyb/exp_eye_store.py index 187bbaf50..80a30057b 100644 --- a/src/hyperion/external_interaction/ispyb/exp_eye_store.py +++ b/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): diff --git a/tests/conftest.py b/tests/conftest.py index 098c970e0..8293d8c81 100644 --- a/tests/conftest.py +++ b/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( diff --git a/tests/system_tests/external_interaction/test_exp_eye_dev.py b/tests/system_tests/external_interaction/test_exp_eye_dev.py index 53337f298..da3fe3aa3 100644 --- a/tests/system_tests/external_interaction/test_exp_eye_dev.py +++ b/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) diff --git a/tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py b/tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py index aa3b42c22..c4969a125 100644 --- a/tests/unit_tests/experiment_plans/test_wait_for_robot_load_then_centre.py +++ b/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 diff --git a/tests/unit_tests/external_interaction/callbacks/robot_load/test_robot_load_ispyb_callback.py b/tests/unit_tests/external_interaction/callbacks/robot_load/test_robot_load_ispyb_callback.py index 47e57a4a3..b425f98df 100644 --- a/tests/unit_tests/external_interaction/callbacks/robot_load/test_robot_load_ispyb_callback.py +++ b/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", "") diff --git a/tests/unit_tests/external_interaction/ispyb/test_expeye_interaction.py b/tests/unit_tests/external_interaction/ispyb/test_expeye_interaction.py index 2f2fe9e2a..fd56d03c3 100644 --- a/tests/unit_tests/external_interaction/ispyb/test_expeye_interaction.py +++ b/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