PR comments

Author: jacob720

src/dodal/beamlines/i03.py

@@ -461,7 +461,7 @@ def collimation_table() -> CollimationTable:
 
 @device_factory()
 def beamsize() -> Beamsize:
-    """Get the i03 beamstop device, instantiate it if it hasn't already been.
+    """Get the i03 beamsize device, instantiate it if it hasn't already been.
     If this is called when already instantiated in i03, it will return the existing object.
     """
     return Beamsize(

src/dodal/beamlines/i04.py

@@ -382,7 +382,7 @@ def scintillator() -> Scintillator:
 
 @device_factory()
 def beamsize() -> Beamsize:
-    """Get the i04 beamstop device, instantiate it if it hasn't already been.
+    """Get the i04 beamsize device, instantiate it if it hasn't already been.
     If this is called when already instantiated in i04, it will return the existing object.
     """
     return Beamsize(

src/dodal/devices/aperturescatterguard.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import asyncio
+from math import inf
 
 from bluesky.protocols import Preparable
 from ophyd_async.core import (
@@ -112,7 +113,7 @@ def load_positions_from_beamline_parameters(
 ) -> dict[ApertureValue, AperturePosition]:
     return {
         ApertureValue.OUT_OF_BEAM: AperturePosition.from_gda_params(
-            _GDAParamApertureValue.ROBOT_LOAD, 0, params
+            _GDAParamApertureValue.ROBOT_LOAD, inf, params
         ),
         ApertureValue.SMALL: AperturePosition.from_gda_params(
             _GDAParamApertureValue.SMALL, 20, params
@@ -124,7 +125,7 @@ def load_positions_from_beamline_parameters(
             _GDAParamApertureValue.LARGE, 100, params
         ),
         ApertureValue.PARKED: AperturePosition.from_gda_params(
-            _GDAParamApertureValue.MANUAL_LOAD, 0, params
+            _GDAParamApertureValue.MANUAL_LOAD, inf, params
         ),
     }
 

src/dodal/devices/beamsize/beamsize.py

@@ -1,9 +1,9 @@
+from typing import Annotated
+
 from ophyd_async.core import SignalR, StandardReadable
+from ophyd_async.core import StandardReadableFormat as Format
 
 
 class BeamsizeBase(StandardReadable):
-    x_um: SignalR[float]
-    y_um: SignalR[float]
-
-    def __init__(self, name="beamsize"):
-        super().__init__(name=name)
+    x_um: Annotated[SignalR[float], Format.HINTED_SIGNAL]
+    y_um: Annotated[SignalR[float], Format.HINTED_SIGNAL]

src/dodal/devices/i03/beamsize.py

@@ -6,7 +6,7 @@
 
 
 class Beamsize(BeamsizeBase):
-    def __init__(self, aperture_scatterguard: ApertureScatterguard, name="beamsize"):
+    def __init__(self, aperture_scatterguard: ApertureScatterguard, name=""):
         super().__init__(name=name)
         self._aperture_scatterguard_ref = Reference(aperture_scatterguard)
 

src/dodal/devices/i04/beamsize.py

@@ -10,7 +10,7 @@ def __init__(
         self,
         transfocator: Transfocator,
         aperture_scatterguard: ApertureScatterguard,
-        name="beamsize",
+        name="",
     ):
         super().__init__(name=name)
         self._transfocator_ref = Reference(transfocator)

tests/devices/i03/test_beamsize.py

@@ -1,41 +1,43 @@
+from math import inf
+from unittest.mock import AsyncMock
+
 import pytest
 from ophyd_async.testing import set_mock_value
 
 from dodal.devices.aperturescatterguard import (
-    AperturePosition,
     ApertureScatterguard,
-    ApertureValue,
 )
 from dodal.devices.i03.beamsize import Beamsize
-from tests.devices.conftest import set_to_position
 
 
 @pytest.mark.parametrize(
-    "selected_aperture, aperture_signal_values, expected_beamsize",
+    "aperture_radius, expected_beamsize",
     [
-        (ApertureValue.SMALL, (1, 0, 0), (20.0, 20.0)),
-        (ApertureValue.MEDIUM, (0, 1, 0), (50.0, 20.0)),
-        (ApertureValue.LARGE, (0, 0, 1), (80.0, 20.0)),
-        (ApertureValue.OUT_OF_BEAM, (0, 0, 0), (0.0, 0.0)),
-        (ApertureValue.PARKED, (0, 0, 0), (0.0, 0.0)),
+        (10.0, (10.0, 10.0)),
+        (50, (50.0, 20.0)),
+        (90, (80.0, 20.0)),
+        (inf, (80.0, 20.0)),
     ],
 )
 async def test_beamsize_gives_min_of_aperture_and_beam_width_and_height(
-    selected_aperture: ApertureValue,
-    aperture_signal_values: tuple[int, int, int],
+    aperture_radius: float,
     expected_beamsize: tuple[float, float],
     ap_sg: ApertureScatterguard,
-    aperture_positions: dict[ApertureValue, AperturePosition],
 ):
-    await set_to_position(ap_sg, aperture_positions[selected_aperture])
+    set_mock_value(ap_sg.aperture.medium, 1)
 
-    for i, signal in enumerate(
-        (ap_sg.aperture.small, ap_sg.aperture.medium, ap_sg.aperture.large)
-    ):
-        set_mock_value(signal, aperture_signal_values[i])
+    ap_sg.radius.read = AsyncMock(
+        return_value={
+            "test_ap_sg-radius": {
+                "value": aperture_radius,
+                "timestamp": 1763051436.7372239,
+                "alarm_severity": 0,
+            }
+        }
+    )  # see https://github.com/bluesky/ophyd-async/issues/1132
 
     beamsize = Beamsize(aperture_scatterguard=ap_sg)
+
     beamsize_x = await beamsize.x_um.get_value()
     beamsize_y = await beamsize.y_um.get_value()
-    assert beamsize_x == expected_beamsize[0]
-    assert beamsize_y == expected_beamsize[1]
+    assert (beamsize_x, beamsize_y) == expected_beamsize

tests/devices/i04/test_beamsize.py

@@ -1,52 +1,45 @@
+from math import inf
+from unittest.mock import AsyncMock
+
 import pytest
 from ophyd_async.testing import set_mock_value
 
 from dodal.devices.aperturescatterguard import (
-    AperturePosition,
     ApertureScatterguard,
-    ApertureValue,
 )
 from dodal.devices.i04.beamsize import Beamsize
 from dodal.devices.i04.transfocator import Transfocator
-from tests.devices.conftest import set_to_position
 
 
 @pytest.mark.parametrize(
-    "selected_aperture, aperture_signal_values, transfocator_sizes, expected_beamsize",
+    "aperture_radius, transfocator_sizes, expected_beamsize",
     [
-        (ApertureValue.SMALL, (1, 0, 0), (10.0, 10.0), (10.0, 10.0)),
-        (ApertureValue.SMALL, (1, 0, 0), (30.0, 10.0), (20.0, 10.0)),
-        (ApertureValue.SMALL, (1, 0, 0), (10.0, 30.0), (10.0, 20.0)),
-        (ApertureValue.SMALL, (1, 0, 0), (55.0, 30.0), (20.0, 20.0)),
-        (ApertureValue.MEDIUM, (0, 1, 0), (10.0, 10.0), (10.0, 10.0)),
-        (ApertureValue.MEDIUM, (0, 1, 0), (30.0, 70.0), (30.0, 50.0)),
-        (ApertureValue.MEDIUM, (0, 1, 0), (70.0, 30.0), (50.0, 30.0)),
-        (ApertureValue.MEDIUM, (0, 1, 0), (90.0, 90.0), (50.0, 50.0)),
-        (ApertureValue.LARGE, (0, 0, 1), (90.0, 55.0), (90.0, 55.0)),
-        (ApertureValue.LARGE, (0, 0, 1), (110.0, 55.0), (100.0, 55.0)),
-        (ApertureValue.LARGE, (0, 0, 1), (90.0, 110.0), (90.0, 100.0)),
-        (ApertureValue.LARGE, (0, 0, 1), (120.0, 120.0), (100.0, 100.0)),
-        (ApertureValue.OUT_OF_BEAM, (0, 0, 0), (120.0, 10.0), (0.0, 0.0)),
-        (ApertureValue.OUT_OF_BEAM, (0, 0, 0), (10.0, 120.0), (0.0, 0.0)),
-        (ApertureValue.PARKED, (0, 0, 0), (10.0, 10.0), (0.0, 0.0)),
-        (ApertureValue.PARKED, (0, 0, 0), (120.0, 120.0), (0.0, 0.0)),
+        (10.0, (50.0, 60.0), (10.0, 10.0)),
+        (20.0, (10.0, 30.0), (10.0, 20.0)),
+        (20.0, (30.0, 10.0), (20.0, 10.0)),
+        (100.0, (50.0, 60.0), (50.0, 60.0)),
+        (inf, (50.0, 60.0), (50.0, 60.0)),
     ],
 )
 async def test_beamsize_gives_min_of_aperture_and_transfocator_width_and_height(
-    selected_aperture: ApertureValue,
-    aperture_signal_values: tuple[int, int, int],
+    aperture_radius: float,
     transfocator_sizes: tuple[float, float],
     expected_beamsize: tuple[float, float],
     fake_transfocator: Transfocator,
     ap_sg: ApertureScatterguard,
-    aperture_positions: dict[ApertureValue, AperturePosition],
 ):
-    await set_to_position(ap_sg, aperture_positions[selected_aperture])
+    set_mock_value(ap_sg.aperture.medium, 1)
+
+    ap_sg.radius.read = AsyncMock(
+        return_value={
+            "test_ap_sg-radius": {
+                "value": aperture_radius,
+                "timestamp": 1763051436.7372239,
+                "alarm_severity": 0,
+            }
+        }
+    )  # see https://github.com/bluesky/ophyd-async/issues/1132
 
-    for i, signal in enumerate(
-        (ap_sg.aperture.small, ap_sg.aperture.medium, ap_sg.aperture.large)
-    ):
-        set_mock_value(signal, aperture_signal_values[i])
     set_mock_value(fake_transfocator.current_horizontal_size_rbv, transfocator_sizes[0])
     set_mock_value(fake_transfocator.current_vertical_size_rbv, transfocator_sizes[1])
 

tests/devices/test_aperture_scatterguard.py

@@ -1,4 +1,5 @@
 from collections.abc import AsyncGenerator
+from math import inf
 from typing import Any
 from unittest.mock import AsyncMock, call
 
@@ -218,7 +219,7 @@ async def test_aperture_positions_robot_load(
     await ap_sg.aperture.z.set(robot_load.aperture_z)
     reading = await ap_sg.read()
     assert isinstance(reading, dict)
-    assert reading[f"{ap_sg.name}-radius"]["value"] == 0.0
+    assert reading[f"{ap_sg.name}-radius"]["value"] == inf
     assert (
         reading[f"{ap_sg.name}-selected_aperture"]["value"] == ApertureValue.OUT_OF_BEAM
     )
@@ -238,7 +239,7 @@ async def test_aperture_positions_robot_load_within_tolerance(
     await ap_sg.aperture.z.set(robot_load.aperture_z)
     reading = await ap_sg.read()
     assert isinstance(reading, dict)
-    assert reading[f"{ap_sg.name}-radius"]["value"] == 0.0
+    assert reading[f"{ap_sg.name}-radius"]["value"] == inf
     assert (
         reading[f"{ap_sg.name}-selected_aperture"]["value"] == ApertureValue.OUT_OF_BEAM
     )
@@ -272,7 +273,7 @@ async def test_aperture_positions_parked(
     await ap_sg.aperture.z.set(parked.aperture_z)
     reading = await ap_sg.read()
     assert isinstance(reading, dict)
-    assert reading[f"{ap_sg.name}-radius"]["value"] == 0.0
+    assert reading[f"{ap_sg.name}-radius"]["value"] == inf
     assert reading[f"{ap_sg.name}-selected_aperture"]["value"] == ApertureValue.PARKED
 
 
@@ -290,7 +291,7 @@ async def test_aperture_positions_parked_within_tolerance(
     await ap_sg.aperture.z.set(parked_z + tolerance)
     reading = await ap_sg.read()
     assert isinstance(reading, dict)
-    assert reading[f"{ap_sg.name}-radius"]["value"] == 0.0
+    assert reading[f"{ap_sg.name}-radius"]["value"] == inf
     assert reading[f"{ap_sg.name}-selected_aperture"]["value"] == ApertureValue.PARKED