(#1466) Wait for aperture move before DC and read it during DC (#1472)

* (#1466) Wait for aperture move before DC and read it during DC

* Use constant for dc group

* Fix connecting smargon

---------

Co-authored-by: Dominic Oram <[email protected]>

src/hyperion/device_setup_plans/read_hardware_for_setup.py

@@ -20,7 +20,6 @@ def read_hardware_for_ispyb_pre_collection(
     undulator: Undulator,
     synchrotron: Synchrotron,
     s4_slit_gaps: S4SlitGaps,
-    aperture_scatterguard: ApertureScatterguard,
     robot: BartRobot,
     smargon: Smargon,
 ):
@@ -32,20 +31,21 @@ def read_hardware_for_ispyb_pre_collection(
     yield from bps.read(synchrotron.synchrotron_mode)
     yield from bps.read(s4_slit_gaps.xgap)
     yield from bps.read(s4_slit_gaps.ygap)
-    yield from bps.read(aperture_scatterguard)
     yield from bps.read(smargon.x)
     yield from bps.read(smargon.y)
     yield from bps.read(smargon.z)
     yield from bps.save()
 
 
 def read_hardware_for_ispyb_during_collection(
+    aperture_scatterguard: ApertureScatterguard,
     attenuator: Attenuator,
     flux: Flux,
     dcm: DCM,
 ):
     LOGGER.info("Reading status of beamline for ispyb deposition, during collection.")
     yield from bps.create(name=CONST.DESCRIPTORS.ISPYB_TRANSMISSION_FLUX_READ)
+    yield from bps.read(aperture_scatterguard)
     yield from bps.read(attenuator.actual_transmission)
     yield from bps.read(flux.flux_reading)
     yield from bps.read(dcm.energy_in_kev)

src/hyperion/experiment_plans/flyscan_xray_centre_plan.py

@@ -269,13 +269,17 @@ def run_gridscan(
             fgs_composite.undulator,
             fgs_composite.synchrotron,
             fgs_composite.s4_slit_gaps,
-            fgs_composite.aperture_scatterguard,
             fgs_composite.robot,
             fgs_composite.smargon,
         )
-        yield from read_hardware_for_ispyb_during_collection(
-            fgs_composite.attenuator, fgs_composite.flux, fgs_composite.dcm
-        )
+
+    read_during_collection = partial(
+        read_hardware_for_ispyb_during_collection,
+        fgs_composite.aperture_scatterguard,
+        fgs_composite.attenuator,
+        fgs_composite.flux,
+        fgs_composite.dcm,
+    )
 
     LOGGER.info("Setting fgs params")
     yield from feature_controlled.set_flyscan_params()
@@ -284,7 +288,7 @@ def run_gridscan(
     yield from wait_for_gridscan_valid(feature_controlled.fgs_motors)
 
     LOGGER.info("Waiting for arming to finish")
-    yield from bps.wait("ready_for_data_collection")
+    yield from bps.wait(CONST.WAIT.GRID_READY_FOR_DC)
     yield from bps.stage(fgs_composite.eiger)
 
     # This needs to occur after eiger is armed so that
@@ -299,6 +303,7 @@ def run_gridscan(
         parameters.zocalo_environment,
         [parameters.scan_points_first_grid, parameters.scan_points_second_grid],
         parameters.scan_indices,
+        do_during_run=read_during_collection,
     )
     yield from bps.abs_set(feature_controlled.fgs_motors.z_steps, 0, wait=False)
 
@@ -310,6 +315,7 @@ def kickoff_and_complete_gridscan(
     zocalo_environment: str,
     scan_points: list[AxesPoints[Axis]],
     scan_start_indices: list[int],
+    do_during_run: Callable[[], MsgGenerator] | None = None,
 ):
     @TRACER.start_as_current_span(CONST.PLAN.DO_FGS)
     @bpp.set_run_key_decorator(CONST.PLAN.DO_FGS)
@@ -345,6 +351,9 @@ def do_fgs():
         yield from bps.wait(
             ZOCALO_STAGE_GROUP
         )  # Make sure ZocaloResults queue is clear and ready to accept our new data
+        if do_during_run:
+            LOGGER.info(f"Running {do_during_run} during FGS")
+            yield from do_during_run()
         LOGGER.info("completing FGS")
         yield from bps.complete(gridscan, wait=True)
 

src/hyperion/experiment_plans/grid_detect_then_xray_centre_plan.py

@@ -49,6 +49,7 @@
     ispyb_activation_wrapper,
 )
 from hyperion.log import LOGGER
+from hyperion.parameters.constants import CONST
 from hyperion.parameters.gridscan import GridScanWithEdgeDetect, ThreeDGridScan
 from hyperion.utils.aperturescatterguard import (
     load_default_aperture_scatterguard_positions_if_unset,
@@ -154,7 +155,9 @@ def run_grid_detection_plan(
     yield from bps.abs_set(composite.backlight, BacklightPosition.OUT)
 
     yield from move_aperture_if_required(
-        composite.aperture_scatterguard, parameters.selected_aperture
+        composite.aperture_scatterguard,
+        parameters.selected_aperture,
+        group=CONST.WAIT.GRID_READY_FOR_DC,
     )
 
     yield from flyscan_xray_centre(
@@ -210,4 +213,5 @@ def grid_detect_then_xray_centre(
         composite.detector_motion,
         parameters.detector_params.detector_distance,
         plan_to_perform,
+        group=CONST.WAIT.GRID_READY_FOR_DC,
     )

src/hyperion/experiment_plans/pin_centre_then_xray_centre_plan.py

@@ -18,6 +18,7 @@
     pin_tip_centre_plan,
 )
 from hyperion.log import LOGGER
+from hyperion.parameters.constants import CONST
 from hyperion.parameters.gridscan import (
     GridScanWithEdgeDetect,
     PinTipCentreThenXrayCentre,
@@ -93,4 +94,5 @@ def pin_tip_centre_then_xray_centre(
         composite.detector_motion,
         parameters.detector_params.detector_distance,
         pin_centre_then_xray_centre_plan(composite, parameters, oav_config_file),
+        group=CONST.WAIT.GRID_READY_FOR_DC,
     )

src/hyperion/experiment_plans/robot_load_then_centre_plan.py

@@ -224,4 +224,5 @@ def robot_load_then_centre(
         composite.detector_motion,
         parameters.detector_distance_mm,
         robot_load_then_centre_plan(composite, parameters),
+        group=CONST.WAIT.GRID_READY_FOR_DC,
     )

src/hyperion/experiment_plans/rotation_scan_plan.py

@@ -210,7 +210,6 @@ def _rotation_scan_plan(
         yield from bps.wait("setup_senv")
         yield from bps.wait("move_gonio_to_start")
         yield from bps.wait("move_to_rotation_start")
-        yield from bps.wait("setup_zebra")
 
         # get some information for the ispyb deposition and trigger the callback
         yield from read_hardware_for_zocalo(composite.eiger)
@@ -219,19 +218,18 @@ def _rotation_scan_plan(
             composite.undulator,
             composite.synchrotron,
             composite.s4_slit_gaps,
-            composite.aperture_scatterguard,
             composite.robot,
             composite.smargon,
         )
-        yield from read_hardware_for_ispyb_during_collection(
-            composite.attenuator, composite.flux, composite.dcm
-        )
+
         yield from read_hardware_for_nexus_writer(composite.eiger)
 
         # Get ready for the actual scan
         yield from bps.abs_set(
             axis.velocity, motion_values.speed_for_rotation_deg_s, wait=True
         )
+
+        yield from bps.wait("setup_zebra")
         yield from arm_zebra(composite.zebra)
 
         # Check topup gate
@@ -244,6 +242,13 @@ def _rotation_scan_plan(
         LOGGER.info("Executing rotation scan")
         yield from bps.rel_set(axis, motion_values.distance_to_move_deg, wait=True)
 
+        yield from read_hardware_for_ispyb_during_collection(
+            composite.aperture_scatterguard,
+            composite.attenuator,
+            composite.flux,
+            composite.dcm,
+        )
+
     yield from _rotation_scan_plan(motion_values, composite)
 
 

src/hyperion/external_interaction/callbacks/ispyb_callback_base.py

@@ -103,17 +103,8 @@ def _handle_ispyb_hardware_read(self, doc) -> Sequence[ScanDataInfo]:
             synchrotron_mode := doc["data"]["synchrotron-synchrotron_mode"],
             SynchrotronMode,
         )
-        aperture_size = SingleAperturePosition(
-            **doc["data"]["aperture_scatterguard-selected_aperture"]
-        )
-        beamsize = beam_size_from_aperture(aperture_size)
-        beamsize_x_mm = beamsize.x_um / 1000 if beamsize.x_um else None
-        beamsize_y_mm = beamsize.y_um / 1000 if beamsize.y_um else None
+
         hwscan_data_collection_info = DataCollectionInfo(
-            beamsize_at_samplex=beamsize_x_mm,
-            beamsize_at_sampley=beamsize_y_mm,
-            focal_spot_size_at_samplex=beamsize_x_mm,
-            focal_spot_size_at_sampley=beamsize_y_mm,
             undulator_gap1=doc["data"]["undulator-current_gap"],
             synchrotron_mode=synchrotron_mode.value,
             slitgap_horizontal=doc["data"]["s4_slit_gaps_xgap"],
@@ -132,8 +123,18 @@ def _handle_ispyb_hardware_read(self, doc) -> Sequence[ScanDataInfo]:
 
     def _handle_ispyb_transmission_flux_read(self, doc) -> Sequence[ScanDataInfo]:
         assert self.params
+        aperture_size = SingleAperturePosition(
+            **doc["data"]["aperture_scatterguard-selected_aperture"]
+        )
+        beamsize = beam_size_from_aperture(aperture_size)
+        beamsize_x_mm = beamsize.x_um / 1000 if beamsize.x_um else None
+        beamsize_y_mm = beamsize.y_um / 1000 if beamsize.y_um else None
         hwscan_data_collection_info = DataCollectionInfo(
-            flux=doc["data"]["flux_flux_reading"]
+            beamsize_at_samplex=beamsize_x_mm,
+            beamsize_at_sampley=beamsize_y_mm,
+            focal_spot_size_at_samplex=beamsize_x_mm,
+            focal_spot_size_at_sampley=beamsize_y_mm,
+            flux=doc["data"]["flux_flux_reading"],
         )
         if transmission := doc["data"]["attenuator-actual_transmission"]:
             # Ispyb wants the transmission in a percentage, we use fractions
@@ -152,6 +153,7 @@ def _handle_ispyb_transmission_flux_read(self, doc) -> Sequence[ScanDataInfo]:
             hwscan_data_collection_info, None, self.params
         )
         ISPYB_LOGGER.info("Updating ispyb data collection after flux read.")
+        self.append_to_comment(f"Aperture: {aperture_size.name}. ")
         return scan_data_infos
 
     @abstractmethod

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

@@ -3,8 +3,6 @@
 from collections.abc import Sequence
 from typing import TYPE_CHECKING, Any, Callable, Optional
 
-from dodal.devices.aperturescatterguard import SingleAperturePosition
-
 from hyperion.external_interaction.callbacks.common.ispyb_mapping import (
     populate_data_collection_group,
     populate_remaining_data_collection_info,
@@ -125,10 +123,6 @@ def populate_info_for_update(
     def _handle_ispyb_hardware_read(self, doc: Event):
         """Use the hardware read values to create the ispyb comment"""
         scan_data_infos = super()._handle_ispyb_hardware_read(doc)
-        aperture_size = SingleAperturePosition(
-            **doc["data"]["aperture_scatterguard-selected_aperture"]
-        )
-
         motor_positions = [
             doc["data"]["smargon-x"],
             doc["data"]["smargon-y"],
@@ -137,7 +131,7 @@ def _handle_ispyb_hardware_read(self, doc: Event):
         assert (
             self.params
         ), "handle_ispyb_hardware_read triggered beore activity_gated_start"
-        comment = f"Sample position: ({motor_positions[0]}, {motor_positions[1]}, {motor_positions[2]}) {self.params.comment} Aperture: {aperture_size.name}"
+        comment = f"Sample position: ({motor_positions[0]}, {motor_positions[1]}, {motor_positions[2]}) {self.params.comment} "
         scan_data_infos[0].data_collection_info.comments = comment
         return scan_data_infos
 

src/hyperion/parameters/constants.py

@@ -34,6 +34,13 @@ class PlanNameConstants:
     ROTATION_MAIN = "rotation_scan_main"
 
 
+@dataclass(frozen=True)
+class PlanGroupCheckpointConstants:
+    # For places to synchronise / stop and wait in plans, use as bluesky group names
+    # Gridscan
+    GRID_READY_FOR_DC = "ready_for_data_collection"
+
+
 @dataclass(frozen=True)
 class DocDescriptorNames:
     # Robot load event descriptor
@@ -106,6 +113,7 @@ class HyperionConstants:
     I03 = I03Constants()
     PARAM = ExperimentParamConstants()
     PLAN = PlanNameConstants()
+    WAIT = PlanGroupCheckpointConstants()
     SIM = SimConstants()
     TRIGGER = TriggerConstants()
     CALLBACK_0MQ_PROXY_PORTS = (5577, 5578)

src/hyperion/utils/validation.py

@@ -64,7 +64,10 @@ def fake_rotation_scan(
     )
     def plan():
         yield from read_hardware_for_ispyb_during_collection(
-            rotation_devices.attenuator, rotation_devices.flux, rotation_devices.dcm
+            rotation_devices.aperture_scatterguard,
+            rotation_devices.attenuator,
+            rotation_devices.flux,
+            rotation_devices.dcm,
         )
         yield from read_hardware_for_nexus_writer(rotation_devices.eiger)
 

tests/system_tests/experiment_plans/test_fgs_plan.py

@@ -141,14 +141,9 @@ def test_read_hardware_for_ispyb_pre_collection(
     @bpp.run_decorator()
     def read_run(u, s, g, r, a, f, dcm, ap_sg, sm):
         yield from read_hardware_for_ispyb_pre_collection(
-            undulator=u,
-            synchrotron=s,
-            s4_slit_gaps=g,
-            aperture_scatterguard=ap_sg,
-            robot=r,
-            smargon=sm,
+            undulator=u, synchrotron=s, s4_slit_gaps=g, robot=r, smargon=sm
         )
-        yield from read_hardware_for_ispyb_during_collection(a, f, dcm)
+        yield from read_hardware_for_ispyb_during_collection(ap_sg, a, f, dcm)
 
     RE(
         read_run(

tests/system_tests/experiment_plans/test_plan_system.py

@@ -29,23 +29,23 @@ async def test_getting_data_for_ispyb():
     )
     smargon = i03.smargon(fake_with_ophyd_sim=True)
 
-    undulator.wait_for_connection()
+    await undulator.connect()
     await synchrotron.connect()
     slit_gaps.wait_for_connection()
-    attenuator.wait_for_connection()
+    await attenuator.connect()
     flux.wait_for_connection()
-    aperture_scatterguard.wait_for_connection()
-    smargon.wait_for_connection()
+    await aperture_scatterguard.connect()
+    await smargon.connect()
     robot = i03.robot(fake_with_ophyd_sim=True)
 
     RE = RunEngine()
 
     @bpp.run_decorator()
     def standalone_read_hardware(und, syn, slits, robot, att, flux, ap_sg, sm):
         yield from read_hardware_for_ispyb_pre_collection(
-            und, syn, slits, robot, ap_sg, smargon=sm
+            und, syn, slits, robot, smargon=sm
         )
-        yield from read_hardware_for_ispyb_during_collection(att, flux, dcm)
+        yield from read_hardware_for_ispyb_during_collection(ap_sg, att, flux, dcm)
 
     RE(
         standalone_read_hardware(

tests/system_tests/external_interaction/test_nexgen.py

@@ -162,7 +162,10 @@ def _fake_rotation_scan(
     )
     def plan():
         yield from read_hardware_for_ispyb_during_collection(
-            rotation_devices.attenuator, rotation_devices.flux, rotation_devices.dcm
+            rotation_devices.aperture_scatterguard,
+            rotation_devices.attenuator,
+            rotation_devices.flux,
+            rotation_devices.dcm,
         )
         yield from read_hardware_for_nexus_writer(rotation_devices.eiger)