Merge branch 'main' into 685_use_vmax_for_rotation

.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

@@ -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)",
     )
 
 

setup.cfg

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

src/hyperion/experiment_plans/flyscan_xray_centre_plan.py

@@ -246,7 +246,7 @@ def run_gridscan(
 
     LOGGER.info("Setting fgs params")
     yield from set_flyscan_params(fgs_motors, parameters.experiment_params)
-
+    LOGGER.info("Waiting for gridscan validity check")
     yield from wait_for_gridscan_valid(fgs_motors)
 
     LOGGER.info("Waiting for arming to finish")
@@ -324,6 +324,10 @@ def run_gridscan_and_move(
             fgs_composite.sample_motors.stub_offsets, StubPosition.CURRENT_AS_CENTER
         )
 
+    # Wait on everything before returning to GDA (particularly apertures), can be removed
+    # when we do not return to GDA here
+    yield from bps.wait()
+
 
 def flyscan_xray_centre(
     composite: FlyScanXRayCentreComposite,
@@ -382,7 +386,7 @@ def run_gridscan_and_move_and_tidy(fgs_composite, params):
         GridscanInternalParameters,
     )
 
-    parameters = GridscanInternalParameters(**external_parameters.from_file())
+    parameters = GridscanInternalParameters(**external_parameters.conftest.from_file())
     subscriptions = XrayCentreCallbackCollection()
 
     context = setup_context(wait_for_connection=True)

src/hyperion/experiment_plans/oav_grid_detection_plan.py

@@ -2,7 +2,7 @@
 
 import dataclasses
 import math
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Tuple
 
 import bluesky.plan_stubs as bps
 import bluesky.preprocessors as bpp
@@ -11,10 +11,10 @@
 from dodal.devices.backlight import Backlight
 from dodal.devices.oav.oav_detector import OAV
 from dodal.devices.oav.pin_image_recognition import PinTipDetection
+from dodal.devices.oav.pin_image_recognition.utils import NONE_VALUE
 from dodal.devices.smargon import Smargon
 
 from hyperion.device_setup_plans.setup_oav import (
-    get_move_required_so_that_beam_is_at_pixel,
     pre_centring_setup_oav,
     wait_for_tip_to_be_found,
 )
@@ -40,6 +40,23 @@ def create_devices(context: BlueskyContext) -> OavGridDetectionComposite:
     return device_composite_from_context(context, OavGridDetectionComposite)
 
 
+def get_min_and_max_y_of_pin(
+    top: np.ndarray, bottom: np.ndarray, full_image_height_px: int
+) -> Tuple[int, int]:
+    """Gives the minimum and maximum y that would cover the whole pin.
+
+    First filters out where no edge was found or the edge covers the full image.
+    If this results in no edges found then returns a min/max that covers the full image
+    """
+    filtered_top = top[np.where((top != 0) & (top != NONE_VALUE))]
+    min_y = min(filtered_top) if len(filtered_top) else 0
+    filtered_bottom = bottom[
+        np.where((bottom != full_image_height_px) & (bottom != NONE_VALUE))
+    ]
+    max_y = max(filtered_bottom) if len(filtered_bottom) else full_image_height_px
+    return min_y, max_y
+
+
 @bpp.run_decorator()
 def grid_detection_plan(
     composite: OavGridDetectionComposite,
@@ -54,8 +71,8 @@ def grid_detection_plan(
     encompass the whole of the sample as it appears in the OAV.
 
     Args:
-        parameters (OAVParamaters): Object containing paramters for setting up the OAV
-        out_parameters (GridScanParams): The returned parameters for the gridscan
+        composite (OavGridDetectionComposite): Composite containing devices for doing a grid detection.
+        parameters (OAVParameters): Object containing parameters for setting up the OAV
         snapshot_template (str): A template for the name of the snapshots, expected to be filled in with an angle
         snapshot_dir (str): The location to save snapshots
         grid_width_microns (int): The width of the grid to scan in microns
@@ -74,9 +91,6 @@ def grid_detection_plan(
 
     LOGGER.info("OAV Centring: Camera set up")
 
-    start_positions = []
-    box_numbers = []
-
     assert isinstance(oav.parameters.micronsPerXPixel, float)
     box_size_x_pixels = box_size_um / oav.parameters.micronsPerXPixel
     assert isinstance(oav.parameters.micronsPerYPixel, float)
@@ -105,15 +119,13 @@ def grid_detection_plan(
         # only use the area from the start of the pin onwards
         top_edge = top_edge[tip_x_px : tip_x_px + grid_width_pixels]
         bottom_edge = bottom_edge[tip_x_px : tip_x_px + grid_width_pixels]
+        LOGGER.info(f"OAV Edge detection top: {list(top_edge)}")
+        LOGGER.info(f"OAV Edge detection bottom: {list(bottom_edge)}")
+
+        min_y, max_y = get_min_and_max_y_of_pin(
+            top_edge, bottom_edge, full_image_height_px
+        )
 
-        # the edge detection line can jump to the edge of the image sometimes, filter
-        # those points out, and if empty after filter use the whole image
-        filtered_top = list(top_edge[top_edge != 0]) or [0]
-        filtered_bottom = list(bottom_edge[bottom_edge != full_image_height_px]) or [
-            full_image_height_px
-        ]
-        min_y = min(filtered_top)
-        max_y = max(filtered_bottom)
         grid_height_px = max_y - min_y
 
         y_steps: int = math.ceil(grid_height_px / box_size_y_pixels)
@@ -127,23 +139,19 @@ def grid_detection_plan(
             y_steps += 1
             min_y -= box_size_y_pixels / 2
             max_y += box_size_y_pixels / 2
-            grid_height_px += 1
+            grid_height_px += box_size_y_pixels
 
         LOGGER.info(f"Drawing snapshot {grid_width_pixels} by {grid_height_px}")
 
-        boxes = (
-            math.ceil(grid_width_pixels / box_size_x_pixels),
-            y_steps,
-        )
-        box_numbers.append(boxes)
+        x_steps = math.ceil(grid_width_pixels / box_size_x_pixels)
 
         upper_left = (tip_x_px, min_y)
 
         yield from bps.abs_set(oav.snapshot.top_left_x, upper_left[0])
         yield from bps.abs_set(oav.snapshot.top_left_y, upper_left[1])
         yield from bps.abs_set(oav.snapshot.box_width, box_size_x_pixels)
-        yield from bps.abs_set(oav.snapshot.num_boxes_x, boxes[0])
-        yield from bps.abs_set(oav.snapshot.num_boxes_y, boxes[1])
+        yield from bps.abs_set(oav.snapshot.num_boxes_x, x_steps)
+        yield from bps.abs_set(oav.snapshot.num_boxes_y, y_steps)
 
         snapshot_filename = snapshot_template.format(angle=abs(angle))
 
@@ -157,23 +165,6 @@ def grid_detection_plan(
         yield from bps.read(smargon)
         yield from bps.save()
 
-        # The first frame is taken at the centre of the first box
-        centre_of_first_box = (
-            int(upper_left[0] + box_size_x_pixels / 2),
-            int(upper_left[1] + box_size_y_pixels / 2),
-        )
-
-        position = yield from get_move_required_so_that_beam_is_at_pixel(
-            smargon, centre_of_first_box, oav.parameters
+        LOGGER.info(
+            f"Grid calculated at {angle}: {x_steps}px by {y_steps}px starting at {upper_left}px"
         )
-        start_positions.append(position)
-
-    LOGGER.info(
-        f"Calculated start position {start_positions[0][0], start_positions[0][1], start_positions[1][2]}"
-    )
-
-    LOGGER.info(
-        f"Calculated number of steps {box_numbers[0][0], box_numbers[0][1], box_numbers[1][1]}"
-    )
-
-    LOGGER.info(f"Step sizes: {box_size_um, box_size_um, box_size_um}")

src/hyperion/experiment_plans/panda_flyscan_xray_centre_plan.py

@@ -249,6 +249,10 @@ def run_gridscan_and_move(
             fgs_composite.sample_motors.stub_offsets, StubPosition.CURRENT_AS_CENTER
         )
 
+    # Wait on everything before returning to GDA (particularly apertures), can be removed
+    # when we do not return to GDA here
+    yield from bps.wait()
+
 
 def panda_flyscan_xray_centre(
     composite: FlyScanXRayCentreComposite,