(DiamondLightSource/hyperion#1217) Use event from grid detect to do i…

…spyb deposition

* Remove oav_snapshot_callback.py

src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py

@@ -48,9 +48,6 @@
 from hyperion.external_interaction.callbacks.grid_detection_callback import (
     GridDetectionCallback,
 )
-from hyperion.external_interaction.callbacks.oav_snapshot_callback import (
-    OavSnapshotCallback,
-)
 from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import (
     ispyb_activation_wrapper,
 )
@@ -156,15 +153,14 @@ def _detect_grid_and_do_gridscan(
         f"{detector_params.prefix}_{detector_params.run_number}_{{angle}}"
     )
 
-    oav_callback = OavSnapshotCallback()
     grid_params_callback = GridDetectionCallback(
         composite.oav.parameters,
         experiment_params.exposure_time,
         experiment_params.set_stub_offsets,
         experiment_params.run_up_distance_mm,
     )
 
-    @bpp.subs_decorator([oav_callback, grid_params_callback])
+    @bpp.subs_decorator([grid_params_callback])
     def run_grid_detection_plan(
         oav_params,
         snapshot_template,
@@ -191,15 +187,6 @@ def run_grid_detection_plan(
         experiment_params.snapshot_dir,
     )
 
-    # Hack because the callback returns the list in inverted order
-    # TODO 1217 REMOVE THIS
-    parameters.hyperion_params.ispyb_params.xtal_snapshots_omega_start = (
-        oav_callback.snapshot_filenames[0][::-1]
-    )
-    parameters.hyperion_params.ispyb_params.xtal_snapshots_omega_end = (
-        oav_callback.snapshot_filenames[1][::-1]
-    )
-
     yield from bps.abs_set(composite.backlight, Backlight.OUT)
 
     LOGGER.info(

src/hyperion/external_interaction/callbacks/ispyb_callback_base.py

@@ -145,6 +145,11 @@ def _handle_oav_snapshot_triggered(self, doc):
         data = doc["data"]
         data_collection_id = None
         data_collection_info = DataCollectionInfo()
+        data_collection_info.xtal_snapshot1 = data.get(
+            "oav_snapshot_last_path_full_overlay"
+        )
+        data_collection_info.xtal_snapshot2 = data.get("oav_snapshot_last_path_outer")
+        data_collection_info.xtal_snapshot3 = data.get("oav_snapshot_last_saved_path")
         data_collection_info.n_images = (
             data["oav_snapshot_num_boxes_x"] * data["oav_snapshot_num_boxes_y"]
         )
@@ -170,7 +175,6 @@ def _handle_oav_snapshot_triggered(self, doc):
                 self._oav_snapshot_event_idx
             ]
 
-        # TODO 1217 populate xtal_snapshot1/2/3
         data_collection_grid_info = DataCollectionGridInfo(
             dx_in_mm=data["oav_snapshot_box_width"]
             * self.params.hyperion_params.ispyb_params.microns_per_pixel_x
@@ -212,10 +216,6 @@ def _handle_ispyb_transmission_flux_read(self, doc):
                 energy_ev
             )
 
-    def _deposit_grid_scan_info(self, doc: Event):
-        # TODO
-        pass
-
     def update_deposition(
         self,
         params,

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

@@ -122,7 +122,6 @@ def activity_gated_start(self, doc: RunStart):
                     None,
                     None,
                     populate_xy_data_collection_info(
-                        self.params.hyperion_params.ispyb_params,
                         self.params.hyperion_params.detector_params,
                     ),
                     self.params.hyperion_params.detector_params,
@@ -199,7 +198,6 @@ def populate_xy_scan_data_info(
             self.ispyb_ids.data_collection_ids
         ), "Expect at least one valid data collection to record scan data"
         xy_data_collection_info = populate_xy_data_collection_info(
-            params.hyperion_params.ispyb_params,
             params.hyperion_params.detector_params,
         )
 
@@ -235,7 +233,6 @@ def populate_xz_scan_data_info(
     ):
         xz_data_collection_info = populate_xz_data_collection_info(
             params,
-            params.hyperion_params.ispyb_params,
             params.hyperion_params.detector_params,
         )
         xz_data_collection_info = replace(

src/hyperion/external_interaction/callbacks/xray_centre/ispyb_mapping.py

@@ -12,41 +12,31 @@
 
 def populate_xz_data_collection_info(
     full_params,
-    ispyb_params,
     detector_params,
 ) -> DataCollectionInfo:
     assert (
         detector_params.omega_start is not None
         and detector_params.run_number is not None
-        and ispyb_params is not None
         and full_params is not None
     ), "StoreGridscanInIspyb failed to get parameters"
     omega_start = detector_params.omega_start + 90
     run_number = detector_params.run_number + 1
-    xtal_snapshots = ispyb_params.xtal_snapshots_omega_end or []
     info = DataCollectionInfo(
         omega_start=omega_start,
         data_collection_number=run_number,
         axis_range=0,
         axis_end=omega_start,
     )
-    info.xtal_snapshot1, info.xtal_snapshot2, info.xtal_snapshot3 = xtal_snapshots + [
-        None
-    ] * (3 - len(xtal_snapshots))
     return info
 
 
-def populate_xy_data_collection_info(ispyb_params, detector_params):
+def populate_xy_data_collection_info(detector_params):
     info = DataCollectionInfo(
         omega_start=detector_params.omega_start,
         data_collection_number=detector_params.run_number,
         axis_range=0,
         axis_end=detector_params.omega_start,
     )
-    snapshots = ispyb_params.xtal_snapshots_omega_start or []
-    info.xtal_snapshot1, info.xtal_snapshot2, info.xtal_snapshot3 = snapshots + [
-        None
-    ] * (3 - len(snapshots))
     return info
 
 

tests/system_tests/external_interaction/test_ispyb_dev_connection.py

@@ -62,7 +62,6 @@ def dummy_data_collection_group_info(dummy_params):
 @pytest.fixture
 def dummy_scan_data_info_for_begin(dummy_params):
     info = populate_xy_data_collection_info(
-        dummy_params.hyperion_params.ispyb_params,
         dummy_params.hyperion_params.detector_params,
     )
     info = populate_remaining_data_collection_info(
@@ -120,7 +119,6 @@ def scan_data_infos_for_update_3d(
     )
     xz_data_collection_info = populate_xz_data_collection_info(
         dummy_params,
-        dummy_params.hyperion_params.ispyb_params,
         dummy_params.hyperion_params.detector_params,
     )
 

tests/unit_tests/experiment_plans/test_grid_detect_then_xray_centre_plan.py

@@ -15,9 +15,6 @@
     detect_grid_and_do_gridscan,
     grid_detect_then_xray_centre,
 )
-from hyperion.external_interaction.callbacks.oav_snapshot_callback import (
-    OavSnapshotCallback,
-)
 from hyperion.parameters.constants import CONST
 from hyperion.parameters.plan_specific.grid_scan_with_edge_detect_params import (
     GridScanWithEdgeDetectInternalParameters,
@@ -100,23 +97,14 @@ def test_full_grid_scan(test_new_fgs_params, test_config_files):
     "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.flyscan_xray_centre",
     autospec=True,
 )
-@patch(
-    "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.OavSnapshotCallback",
-    autospec=True,
-)
 def test_detect_grid_and_do_gridscan(
-    mock_oav_callback_init: MagicMock,
     mock_flyscan_xray_centre_plan: MagicMock,
     mock_grid_detection_plan: MagicMock,
     grid_detect_devices: GridDetectThenXRayCentreComposite,
     RE: RunEngine,
     test_full_grid_scan_params: GridScanWithEdgeDetectInternalParameters,
     test_config_files: Dict,
 ):
-    mock_oav_callback = OavSnapshotCallback()
-    mock_oav_callback.out_upper_left = [[0, 1], [2, 3]]
-    mock_oav_callback.snapshot_filenames = [["test"], ["test3"]]
-    mock_oav_callback_init.return_value = mock_oav_callback
     mock_grid_detection_plan.side_effect = _fake_grid_detection
     grid_detect_devices.oav.parameters = OAVConfigParams(
         test_config_files["zoom_params_file"], test_config_files["display_config"]
@@ -142,9 +130,6 @@ def test_detect_grid_and_do_gridscan(
         # Verify we called the grid detection plan
         mock_grid_detection_plan.assert_called_once()
 
-        # Verify callback to oav snaposhot was called
-        mock_oav_callback_init.assert_called_once()
-
         # Check backlight was moved OUT
         assert grid_detect_devices.backlight.pos.get() == Backlight.OUT
 
@@ -165,12 +150,7 @@ def test_detect_grid_and_do_gridscan(
     "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.flyscan_xray_centre",
     autospec=True,
 )
-@patch(
-    "hyperion.experiment_plans.grid_detect_then_xray_centre_plan.OavSnapshotCallback",
-    autospec=True,
-)
 def test_when_full_grid_scan_run_then_parameters_sent_to_fgs_as_expected(
-    mock_oav_callback_init: MagicMock,
     mock_flyscan_xray_centre_plan: MagicMock,
     mock_grid_detection_plan: MagicMock,
     eiger: EigerDetector,
@@ -180,10 +160,6 @@ def test_when_full_grid_scan_run_then_parameters_sent_to_fgs_as_expected(
     test_config_files: Dict,
 ):
     oav_params = OAVParameters("xrayCentring", test_config_files["oav_config_json"])
-    mock_oav_callback = OavSnapshotCallback()
-    mock_oav_callback.snapshot_filenames = [["a", "b", "c"], ["d", "e", "f"]]
-    mock_oav_callback.out_upper_left = [[1, 2], [1, 3]]
-    mock_oav_callback_init.return_value = mock_oav_callback
 
     mock_grid_detection_plan.side_effect = _fake_grid_detection
 
@@ -212,18 +188,6 @@ def test_when_full_grid_scan_run_then_parameters_sent_to_fgs_as_expected(
 
         assert isinstance(params, GridscanInternalParameters)
 
-        ispyb_params = params.hyperion_params.ispyb_params
-        assert ispyb_params.xtal_snapshots_omega_start == [
-            "c",
-            "b",
-            "a",
-        ]
-        assert ispyb_params.xtal_snapshots_omega_end == [
-            "f",
-            "e",
-            "d",
-        ]
-
         assert params.hyperion_params.detector_params.num_triggers == 50
 
         assert params.experiment_params.x_axis.full_steps == 10

tests/unit_tests/experiment_plans/test_grid_detection_plan.py

@@ -23,9 +23,6 @@
 from hyperion.external_interaction.callbacks.grid_detection_callback import (
     GridDetectionCallback,
 )
-from hyperion.external_interaction.callbacks.oav_snapshot_callback import (
-    OavSnapshotCallback,
-)
 from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import (
     GridscanISPyBCallback,
     ispyb_activation_wrapper,
@@ -92,8 +89,6 @@ def test_grid_detection_plan_runs_and_triggers_snapshots(
     fake_devices,
 ):
     params = OAVParameters("loopCentring", test_config_files["oav_config_json"])
-    cb = OavSnapshotCallback()
-    RE.subscribe(cb)
     composite, image = fake_devices
 
     @bpp.run_decorator()
@@ -109,11 +104,6 @@ def decorated():
     RE(decorated())
     assert image.save.call_count == 6
 
-    assert len(cb.snapshot_filenames) == 2
-    assert len(cb.snapshot_filenames[0]) == 3
-    assert cb.snapshot_filenames[0][0] == "tmp/test_0.png"
-    assert cb.snapshot_filenames[1][2] == "tmp/test_90_grid_overlay.png"
-
 
 @patch("dodal.beamlines.beamline_utils.active_device_is_same_type", lambda a, b: True)
 @patch("bluesky.plan_stubs.sleep", new=MagicMock())
@@ -165,9 +155,7 @@ def test_given_when_grid_detect_then_upper_left_and_start_position_as_expected(
     composite.oav.parameters.beam_centre_j = 4
     box_size_y_pixels = box_size_um / composite.oav.parameters.micronsPerYPixel
 
-    oav_cb = OavSnapshotCallback()
     grid_param_cb = GridDetectionCallback(composite.oav.parameters, 0.004, False)
-    RE.subscribe(oav_cb)
     RE.subscribe(grid_param_cb)
 
     @bpp.run_decorator()
@@ -211,8 +199,6 @@ def test_when_grid_detection_plan_run_twice_then_values_do_not_persist_in_callba
     composite, _ = fake_devices
 
     for _ in range(2):
-        cb = OavSnapshotCallback()
-        RE.subscribe(cb)
 
         @bpp.run_decorator()
         def decorated():
@@ -225,7 +211,6 @@ def decorated():
             )
 
         RE(decorated())
-    assert len(cb.snapshot_filenames) == 2
 
 
 @patch("dodal.beamlines.beamline_utils.active_device_is_same_type", lambda a, b: True)
@@ -264,6 +249,9 @@ def decorated():
                 "oav_snapshot_num_boxes_x": 8,
                 "oav_snapshot_num_boxes_y": 2,
                 "oav_snapshot_box_width": 16,
+                "oav_snapshot_last_path_full_overlay": "tmp/test_0_grid_overlay.png",
+                "oav_snapshot_last_path_outer": "tmp/test_0_outer_overlay.png",
+                "oav_snapshot_last_saved_path": "tmp/test_0.png",
             },
         )
         assert_event(
@@ -274,6 +262,9 @@ def decorated():
                 "oav_snapshot_num_boxes_x": 8,
                 "oav_snapshot_num_boxes_y": 1,
                 "oav_snapshot_box_width": 16,
+                "oav_snapshot_last_path_full_overlay": "tmp/test_90_grid_overlay.png",
+                "oav_snapshot_last_path_outer": "tmp/test_90_outer_overlay.png",
+                "oav_snapshot_last_saved_path": "tmp/test_90.png",
             },
         )
 

tests/unit_tests/external_interaction/callbacks/conftest.py

@@ -158,6 +158,9 @@ class TestData:
             "oav_snapshot_num_boxes_x": 40,
             "oav_snapshot_num_boxes_y": 20,
             "oav_snapshot_box_width": 0.1 * 1000 / 1.25,  # size in pixels
+            "oav_snapshot_last_path_full_overlay": "test_1_y",
+            "oav_snapshot_last_path_outer": "test_2_y",
+            "oav_snapshot_last_saved_path": "test_3_y",
         },
     }
     test_event_document_oav_snapshot_xz: Event = {
@@ -168,6 +171,9 @@ class TestData:
             "oav_snapshot_num_boxes_x": 40,
             "oav_snapshot_num_boxes_y": 10,
             "oav_snapshot_box_width": 0.1 * 1000 / 1.25,  # size in pixels
+            "oav_snapshot_last_path_full_overlay": "test_1_z",
+            "oav_snapshot_last_path_outer": "test_2_z",
+            "oav_snapshot_last_saved_path": "test_3_z",
         },
     }
     test_event_document_pre_data_collection: Event = {