Merge branch 'main' into 1234_robot_load

README.md

@@ -15,7 +15,7 @@ Left to do is:
 Development Installation
 =================
 
-Run `dls_dev_env.sh` (This assumes you're on a DLS machine. If you are not, you should be able to just run a subset of this script)
+Run `./utility_scripts/dls_dev_env.sh` (This assumes you're on a DLS machine. If you are not, you should be able to just run a subset of this script)
 
 Note that because Hyperion makes heavy use of [Dodal](https://github.com/DiamondLightSource/dodal) this will also pull a local editable version of dodal to the parent folder of this repo.
 

contributing.md

@@ -8,7 +8,7 @@ General Workflow
 1. An issue is created for the work. This issue should describe in as much detail as possible the work that needs to be done. Anyone is free to make a ticket and/or comment on one.
 2. If a developer is going to do the work they assign themselves to the issue.
 3. The developer makes a new branch with the format `issue_short_description` e.g. `122_create_a_contributing_file`. (External developers are also welcome to make forks)
-4. The developer does the work on this branch, adding their work in small commits. Commit messages should be informative and prefixed with the issue number e.g. `#122: Added contributing file`.
+4. The developer does the work on this branch, adding their work in small commits. Commit messages should be informative and prefixed with the issue number e.g. `(#122) Added contributing file`.
 5. The developer submits a PR for the work. In the pull request should start with `Fixes #issue_num` e.g. `Fixes #122`, this will ensure the issue is automatically closed when the PR is merged. The developer should also add some background on how the reviewer might test the change.
 6. If the developer has a particular person in mind to review the work they should assign that person to the PR as a reviewer.
 7. The reviewer and developer go back and forth on the code until the reviewer approves it. (See [Reviewing Work](#reviewing-work))

src/hyperion/__main__.py

@@ -13,16 +13,19 @@
 from pydantic.dataclasses import dataclass
 
 from hyperion.exceptions import WarningException
-from hyperion.experiment_plans.experiment_registry import PLAN_REGISTRY, PlanNotFound
+from hyperion.experiment_plans.experiment_registry import (
+    PLAN_REGISTRY,
+    PlanNotFound,
+)
 from hyperion.external_interaction.callbacks.__main__ import (
     setup_logging as setup_callback_logging,
 )
-from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import (
-    AbstractPlanCallbackCollection,
-)
 from hyperion.external_interaction.callbacks.aperture_change_callback import (
     ApertureChangeCallback,
 )
+from hyperion.external_interaction.callbacks.common.callback_util import (
+    CallbacksFactory,
+)
 from hyperion.external_interaction.callbacks.log_uid_tag_callback import (
     LogUidTaggingCallback,
 )
@@ -45,7 +48,7 @@ class Command:
     devices: Optional[Any] = None
     experiment: Optional[Callable[[Any, Any], MsgGenerator]] = None
     parameters: Optional[InternalParameters] = None
-    callbacks: Optional[type[AbstractPlanCallbackCollection]] = None
+    callbacks: Optional[CallbacksFactory] = None
 
 
 @dataclass
@@ -108,7 +111,7 @@ def start(
         experiment: Callable,
         parameters: InternalParameters,
         plan_name: str,
-        callback_type: Optional[type[AbstractPlanCallbackCollection]],
+        callbacks: Optional[CallbacksFactory],
     ) -> StatusAndMessage:
         LOGGER.info(f"Started with parameters: {parameters}")
 
@@ -122,7 +125,13 @@ def start(
         else:
             self.current_status = StatusAndMessage(Status.BUSY)
             self.command_queue.put(
-                Command(Actions.START, devices, experiment, parameters, callback_type)
+                Command(
+                    action=Actions.START,
+                    devices=devices,
+                    experiment=experiment,
+                    parameters=parameters,
+                    callbacks=callbacks,
+                )
             )
             return StatusAndMessage(Status.SUCCESS)
 
@@ -149,7 +158,7 @@ def shutdown(self):
         """Stops the run engine and the loop waiting for messages."""
         print("Shutting down: Stopping the run engine gracefully")
         self.stop()
-        self.command_queue.put(Command(Actions.SHUTDOWN))
+        self.command_queue.put(Command(action=Actions.SHUTDOWN))
 
     def wait_on_queue(self):
         while True:
@@ -163,7 +172,7 @@ def wait_on_queue(self):
                     if (
                         not self.use_external_callbacks
                         and command.callbacks
-                        and (cbs := list(command.callbacks()))
+                        and (cbs := command.callbacks())
                     ):
                         LOGGER.info(
                             f"Using callbacks for this plan: {not self.use_external_callbacks} - {cbs}"

src/hyperion/experiment_plans/experiment_registry.py

@@ -14,14 +14,10 @@
     pin_centre_then_xray_centre_plan,
     robot_load_then_centre_plan,
 )
-from hyperion.external_interaction.callbacks.abstract_plan_callback_collection import (
-    AbstractPlanCallbackCollection,
-)
-from hyperion.external_interaction.callbacks.rotation.callback_collection import (
-    RotationCallbackCollection,
-)
-from hyperion.external_interaction.callbacks.xray_centre.callback_collection import (
-    XrayCentreCallbackCollection,
+from hyperion.external_interaction.callbacks.common.callback_util import (
+    CallbacksFactory,
+    create_gridscan_callbacks,
+    create_rotation_callbacks,
 )
 from hyperion.parameters.plan_specific.grid_scan_with_edge_detect_params import (
     GridScanWithEdgeDetectInternalParameters,
@@ -66,7 +62,7 @@ class ExperimentRegistryEntry(TypedDict):
         | PandAGridscanInternalParameters
     ]
     experiment_param_type: type[AbstractExperimentParameterBase]
-    callback_collection_type: type[AbstractPlanCallbackCollection]
+    callbacks_factory: CallbacksFactory
 
 
 EXPERIMENT_TYPES = Union[GridScanParams, RotationScanParams]
@@ -75,37 +71,37 @@ class ExperimentRegistryEntry(TypedDict):
         "setup": panda_flyscan_xray_centre_plan.create_devices,
         "internal_param_type": PandAGridscanInternalParameters,
         "experiment_param_type": PandAGridScanParams,
-        "callback_collection_type": XrayCentreCallbackCollection,
+        "callbacks_factory": create_gridscan_callbacks,
     },
     "flyscan_xray_centre": {
         "setup": flyscan_xray_centre_plan.create_devices,
         "internal_param_type": GridscanInternalParameters,
         "experiment_param_type": GridScanParams,
-        "callback_collection_type": XrayCentreCallbackCollection,
+        "callbacks_factory": create_gridscan_callbacks,
     },
     "grid_detect_then_xray_centre": {
         "setup": grid_detect_then_xray_centre_plan.create_devices,
         "internal_param_type": GridScanWithEdgeDetectInternalParameters,
         "experiment_param_type": GridScanWithEdgeDetectParams,
-        "callback_collection_type": XrayCentreCallbackCollection,
+        "callbacks_factory": create_gridscan_callbacks,
     },
     "rotation_scan": {
         "setup": rotation_scan_plan.create_devices,
         "internal_param_type": RotationInternalParameters,
         "experiment_param_type": RotationScanParams,
-        "callback_collection_type": RotationCallbackCollection,
+        "callbacks_factory": create_rotation_callbacks,
     },
     "pin_tip_centre_then_xray_centre": {
         "setup": pin_centre_then_xray_centre_plan.create_devices,
         "internal_param_type": PinCentreThenXrayCentreInternalParameters,
         "experiment_param_type": PinCentreThenXrayCentreParams,
-        "callback_collection_type": XrayCentreCallbackCollection,
+        "callbacks_factory": create_gridscan_callbacks,
     },
     "robot_load_then_centre": {
         "setup": robot_load_then_centre_plan.create_devices,
         "internal_param_type": RobotLoadThenCentreInternalParameters,
         "experiment_param_type": RobotLoadThenCentreParams,
-        "callback_collection_type": XrayCentreCallbackCollection,
+        "callbacks_factory": create_gridscan_callbacks,
     },
 }
 EXPERIMENT_NAMES = list(PLAN_REGISTRY.keys())

src/hyperion/experiment_plans/flyscan_xray_centre_plan.py

@@ -1,15 +1,12 @@
 from __future__ import annotations
 
-import argparse
 import dataclasses
 from typing import TYPE_CHECKING, Any, List
 
 import bluesky.plan_stubs as bps
 import bluesky.preprocessors as bpp
 import numpy as np
 from blueapi.core import BlueskyContext, MsgGenerator
-from bluesky.run_engine import RunEngine
-from bluesky.utils import ProgressBarManager
 from dodal.devices.aperturescatterguard import (
     ApertureScatterguard,
     SingleAperturePosition,
@@ -52,17 +49,13 @@
     transmission_and_xbpm_feedback_for_collection_decorator,
 )
 from hyperion.exceptions import WarningException
-from hyperion.external_interaction.callbacks.xray_centre.callback_collection import (
-    XrayCentreCallbackCollection,
-)
 from hyperion.log import LOGGER
-from hyperion.parameters import external_parameters
 from hyperion.parameters.constants import CONST
 from hyperion.tracing import TRACER
 from hyperion.utils.aperturescatterguard import (
     load_default_aperture_scatterguard_positions_if_unset,
 )
-from hyperion.utils.context import device_composite_from_context, setup_context
+from hyperion.utils.context import device_composite_from_context
 
 if TYPE_CHECKING:
     from hyperion.parameters.plan_specific.gridscan_internal_params import (
@@ -369,27 +362,3 @@ def run_gridscan_and_move_and_tidy(fgs_composite, params):
         yield from run_gridscan_and_move(fgs_composite, params)
 
     return run_gridscan_and_move_and_tidy(composite, parameters)
-
-
-if __name__ == "__main__":
-    parser = argparse.ArgumentParser()
-    parser.add_argument(
-        "--beamline",
-        help="The beamline prefix this is being run on",
-        default=CONST.SIM.BEAMLINE,
-    )
-    args = parser.parse_args()
-
-    RE = RunEngine({})
-    RE.waiting_hook = ProgressBarManager()  # type: ignore
-    from hyperion.parameters.plan_specific.gridscan_internal_params import (
-        GridscanInternalParameters,
-    )
-
-    parameters = GridscanInternalParameters(**external_parameters.conftest.from_file())
-    subscriptions = XrayCentreCallbackCollection()
-
-    context = setup_context(wait_for_connection=True)
-    composite = create_devices(context)
-
-    RE(flyscan_xray_centre(composite, parameters))

src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py

@@ -40,8 +40,8 @@
     set_aperture_for_bbox_size,
     wait_for_gridscan_valid,
 )
-from hyperion.external_interaction.callbacks.xray_centre.callback_collection import (
-    XrayCentreCallbackCollection,
+from hyperion.external_interaction.callbacks.common.callback_util import (
+    create_gridscan_callbacks,
 )
 from hyperion.log import LOGGER
 from hyperion.parameters import external_parameters
@@ -313,7 +313,7 @@ def run_gridscan_and_move_and_tidy(fgs_composite, params):
     )
 
     parameters = GridscanInternalParameters(**external_parameters.from_file())
-    subscriptions = XrayCentreCallbackCollection()
+    subscriptions = create_gridscan_callbacks()
 
     context = setup_context(wait_for_connection=True)
     composite = create_devices(context)

src/hyperion/external_interaction/callbacks/common/callback_util.py

@@ -0,0 +1,31 @@
+from typing import Callable, Tuple
+
+from bluesky.callbacks import CallbackBase
+
+from hyperion.external_interaction.callbacks.rotation.ispyb_callback import (
+    RotationISPyBCallback,
+)
+from hyperion.external_interaction.callbacks.rotation.nexus_callback import (
+    RotationNexusFileCallback,
+)
+from hyperion.external_interaction.callbacks.xray_centre.ispyb_callback import (
+    GridscanISPyBCallback,
+)
+from hyperion.external_interaction.callbacks.xray_centre.nexus_callback import (
+    GridscanNexusFileCallback,
+)
+from hyperion.external_interaction.callbacks.zocalo_callback import ZocaloCallback
+
+CallbacksFactory = Callable[[], Tuple[CallbackBase, CallbackBase]]
+
+
+def create_gridscan_callbacks() -> (
+    Tuple[GridscanNexusFileCallback, GridscanISPyBCallback]
+):
+    return (GridscanNexusFileCallback(), GridscanISPyBCallback(emit=ZocaloCallback()))
+
+
+def create_rotation_callbacks() -> (
+    Tuple[RotationNexusFileCallback, RotationISPyBCallback]
+):
+    return (RotationNexusFileCallback(), RotationISPyBCallback(emit=ZocaloCallback()))

src/hyperion/external_interaction/callbacks/ispyb_callback_base.py

@@ -111,10 +111,10 @@ def activity_gated_event(self, doc: Event) -> Event:
         return self._tag_doc(doc)
 
     @abstractmethod
-    def update_deposition(self, params):
+    def update_deposition(self, params) -> IspybIds:
         pass
 
-    def activity_gated_stop(self, doc: RunStop) -> Optional[RunStop]:
+    def activity_gated_stop(self, doc: RunStop) -> RunStop:
         """Subclasses must check that they are recieving a stop document for the correct
         uid to use this method!"""
         assert isinstance(

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

@@ -42,8 +42,6 @@ class RotationISPyBCallback(BaseISPyBCallback):
     Or decorate a plan using bluesky.preprocessors.subs_decorator.
 
     See: https://blueskyproject.io/bluesky/callbacks.html#ways-to-invoke-callbacks
-
-    Usually used as part of a RotationCallbackCollection.
     """
 
     def __init__(
@@ -151,7 +149,7 @@ def activity_gated_event(self, doc: Event):
         set_dcgid_tag(self.ispyb_ids.data_collection_group_id)
         return doc
 
-    def activity_gated_stop(self, doc: RunStop) -> None:
+    def activity_gated_stop(self, doc: RunStop) -> RunStop:
         if doc.get("run_start") == self.uid_to_finalize_on:
             self.uid_to_finalize_on = None
             return super().activity_gated_stop(doc)

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

@@ -34,8 +34,6 @@ class RotationNexusFileCallback(PlanReactiveCallback):
     Or decorate a plan using bluesky.preprocessors.subs_decorator.
 
     See: https://blueskyproject.io/bluesky/callbacks.html#ways-to-invoke-callbacks
-
-    Usually used as part of a RotationCallbackCollection.
     """
 
     def __init__(self) -> None: