Merge branch 'main' into 1251_ophyd_async_sychrotron

.github/workflows/code.yml

@@ -49,7 +49,7 @@ jobs:
         run: diff-quality --violations=pyright --fail-under=100
 
       - name: Run tests
-        run: pytest -s --random-order -m "not (dlstbx or s03)"
+        run: pytest --logging -s --random-order -m "not (dlstbx or s03)"
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v4

.github/workflows/pin_versions.py

@@ -8,6 +8,7 @@
 from sys import stderr, stdout
 
 SETUP_CFG_PATTERN = re.compile("(.*?)\\s*(@(.*))?\n")
+SETUP_UNPINNED_PATTERN = re.compile("(.*?)\\s*([<>=]+(.*))?\n")
 PIP = "pip"
 
 
@@ -77,7 +78,7 @@ def write_with_comment(comment, text, output_file):
 
 def update_setup_cfg_line(version_map: dict[str, str], line, output_file):
     stripped_line, comment = strip_comment(line)
-    if match := SETUP_CFG_PATTERN.match(stripped_line):
+    if match := SETUP_UNPINNED_PATTERN.match(stripped_line):
         normalized_name = normalize(match[1].strip())
         if normalized_name not in version_map:
             stderr.write(

.github/workflows/test_data/pip_freeze.txt

@@ -112,7 +112,7 @@ mypy==1.8.0
 mypy-extensions==1.0.0
 mysql-connector-python==8.3.0
 networkx==3.2.1
-nexgen==0.8.0
+nexgen==0.8.4
 nodeenv==1.8.0
 nose2==0.14.0
 nslsii==0.9.1

.github/workflows/test_data/setup.cfg

@@ -24,7 +24,7 @@ install_requires =
     ispyb
     scanspec
     numpy
-    nexgen @ git+https://github.com/dials/nexgen.git@db4858f6d91a3d07c6c0f815ef752849c0bf79d4
+    nexgen>0.8.3
     opentelemetry-distro
     opentelemetry-exporter-jaeger
     ophyd

.github/workflows/test_data/setup.cfg.pinned

@@ -24,7 +24,7 @@ install_requires =
     ispyb @ 10.0.0
     scanspec @ 0.6.5
     numpy @ 1.26.3
-    nexgen @ 0.8.0
+    nexgen @ 0.8.4
     opentelemetry-distro @ 0.43b0
     opentelemetry-exporter-jaeger @ 1.21.0
     ophyd @ 1.9.0

.github/workflows/test_data/setup.cfg.unpinned

@@ -24,7 +24,7 @@ install_requires =
     ispyb
     scanspec
     numpy
-    nexgen
+    nexgen>0.8.3
     opentelemetry-distro
     opentelemetry-exporter-jaeger
     ophyd

.github/workflows/test_pin_versions.py

@@ -61,7 +61,7 @@ def test_write_commit_message(mock_stdout, patched_run_pip_freeze):
     installed_versions = pin_versions.fetch_pin_versions()
     pin_versions.write_commit_message(installed_versions)
     mock_stdout.write.assert_called_once_with(
-        "Pin dependencies prior to release. Dodal 1.13.1, nexgen 0.8.0"
+        "Pin dependencies prior to release. Dodal 1.13.1, nexgen 0.8.4"
     )
 
 

.vscode/hyperion-dodal-nexgen.code-workspace

@@ -16,7 +16,7 @@
 		"esbonio.sphinx.confDir": "",
 		"search.useIgnoreFiles": false,
 		"[python]": {
-			"editor.defaultFormatter": "ms-python.black-formatter"
+			"editor.defaultFormatter": "charliermarsh.ruff"
 		},
 		"python.formatting.provider": "none"
 	}

.vscode/settings.json

@@ -13,7 +13,7 @@
             "source.fixAll.ruff": "explicit",
             "source.organizeImports.ruff": "explicit"
         },
-        "editor.defaultFormatter": "ms-python.black-formatter"
+        "editor.defaultFormatter": "charliermarsh.ruff"
     },
     "terminal.integrated.gpuAcceleration": "off",
     "python.analysis.typeCheckingMode": "basic",

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.
 
@@ -49,7 +49,7 @@ python -m hyperion --skip-startup-connection
 
 Testing
 --------------
-Unit tests can be run with `python -m pytest -m "not s03" --random-order`. To see log output from tests you can use the `-s` command line option, and to set the logger levels to `DEBUG` rather than `INFO`, you can use the option `--debug-logging`. So to run the unit tests such that all logs are at `DEBUG` level and are printed to the terminal, you can use `python -m pytest -m "not s03" --random-order -s --debug-logging`. Note that this will likely overrun your terminal buffer, so you can narrow the selection of tests with the `-k "<test name pattern>"` option.
+Unit tests can be run with `python -m pytest -m "not s03" --random-order`. To see log output from tests you can turn on logging with the `--logging` command line option and then use the `-s` command line option to print logs into the console. So to run the unit tests such that all logs are at printed to the terminal, you can use `python -m pytest -m "not s03" --random-order --logging -s`. Note that this will likely overrun your terminal buffer, so you can narrow the selection of tests with the `-k "<test name pattern>"` option.
 
 To be able to run the system tests, or a complete fake scan, we need the simulated S03 beamline. This can be found at: https://gitlab.diamond.ac.uk/controls/python3/s03_utils
 

conftest.py

@@ -1,15 +1,26 @@
+from os import environ, getenv
 from typing import Iterator
 from unittest.mock import patch
 
 import pytest
 
+print("Adjusting S03 EPICS environment ...")
+s03_epics_server_port = getenv("S03_EPICS_CA_SERVER_PORT")
+s03_epics_repeater_port = getenv("S03_EPICS_CA_REPEATER_PORT")
+if s03_epics_server_port:
+    environ["EPICS_CA_SERVER_PORT"] = s03_epics_server_port
+    print(f"[EPICS_CA_SERVER_PORT] = {s03_epics_server_port}")
+if s03_epics_repeater_port:
+    environ["EPICS_CA_REPEATER_PORT"] = s03_epics_repeater_port
+    print(f"[EPICS_CA_REPEATER_PORT] = {s03_epics_repeater_port}")
+
 
 def pytest_addoption(parser):
     parser.addoption(
-        "--debug-logging",
+        "--logging",
         action="store_true",
         default=False,
-        help="initialise test loggers in DEBUG instead of INFO",
+        help="Log during all tests (not just those that are testing logging logic)",
     )
 
 

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))

setup.cfg

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

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/__init__.py

@@ -13,16 +13,16 @@
 from hyperion.experiment_plans.pin_centre_then_xray_centre_plan import (
     pin_tip_centre_then_xray_centre,
 )
-from hyperion.experiment_plans.rotation_scan_plan import rotation_scan
-from hyperion.experiment_plans.wait_for_robot_load_then_centre_plan import (
-    wait_for_robot_load_then_centre,
+from hyperion.experiment_plans.robot_load_then_centre_plan import (
+    robot_load_then_centre,
 )
+from hyperion.experiment_plans.rotation_scan_plan import rotation_scan
 
 __all__ = [
     "flyscan_xray_centre",
     "grid_detect_then_xray_centre",
     "rotation_scan",
     "pin_tip_centre_then_xray_centre",
-    "wait_for_robot_load_then_centre",
+    "robot_load_then_centre",
     "panda_flyscan_xray_centre",
 ]

src/hyperion/experiment_plans/experiment_registry.py

@@ -12,16 +12,12 @@
 from hyperion.experiment_plans import (
     grid_detect_then_xray_centre_plan,
     pin_centre_then_xray_centre_plan,
-    wait_for_robot_load_then_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,
@@ -37,14 +33,14 @@
     PinCentreThenXrayCentreInternalParameters,
     PinCentreThenXrayCentreParams,
 )
+from hyperion.parameters.plan_specific.robot_load_then_center_params import (
+    RobotLoadThenCentreInternalParameters,
+    RobotLoadThenCentreParams,
+)
 from hyperion.parameters.plan_specific.rotation_scan_internal_params import (
     RotationInternalParameters,
     RotationScanParams,
 )
-from hyperion.parameters.plan_specific.wait_for_robot_load_then_center_params import (
-    WaitForRobotLoadThenCentreInternalParameters,
-    WaitForRobotLoadThenCentreParams,
-)
 
 
 def not_implemented():
@@ -62,11 +58,11 @@ class ExperimentRegistryEntry(TypedDict):
         | GridScanWithEdgeDetectInternalParameters
         | RotationInternalParameters
         | PinCentreThenXrayCentreInternalParameters
-        | WaitForRobotLoadThenCentreInternalParameters
+        | RobotLoadThenCentreInternalParameters
         | 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,
     },
-    "wait_for_robot_load_then_centre": {
-        "setup": wait_for_robot_load_then_centre_plan.create_devices,
-        "internal_param_type": WaitForRobotLoadThenCentreInternalParameters,
-        "experiment_param_type": WaitForRobotLoadThenCentreParams,
-        "callback_collection_type": XrayCentreCallbackCollection,
+    "robot_load_then_centre": {
+        "setup": robot_load_then_centre_plan.create_devices,
+        "internal_param_type": RobotLoadThenCentreInternalParameters,
+        "experiment_param_type": RobotLoadThenCentreParams,
+        "callbacks_factory": create_gridscan_callbacks,
     },
 }
 EXPERIMENT_NAMES = list(PLAN_REGISTRY.keys())