From 6a0f53384ceaa7ca2afeda1e0cb355811a99546e Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 28 May 2026 12:38:19 +0000 Subject: [PATCH 1/9] Add Mbs slit entrance --- .../devices/electron_analyser/mbs/__init__.py | 8 ++ .../mbs/mbs_analyser_slits.py | 84 +++++++++++++++++++ .../mbs/test_mbs_analyser_slits.py | 44 ++++++++++ 3 files changed, 136 insertions(+) create mode 100644 src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py create mode 100644 tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py diff --git a/src/dodal/devices/electron_analyser/mbs/__init__.py b/src/dodal/devices/electron_analyser/mbs/__init__.py index befe299bcca..ce84c501b6e 100644 --- a/src/dodal/devices/electron_analyser/mbs/__init__.py +++ b/src/dodal/devices/electron_analyser/mbs/__init__.py @@ -1,9 +1,17 @@ +from .mbs_analyser_slits import ( + EntranceSlitInformation, + EntranceSlitInformationDevice, + SlitPositions, +) from .mbs_detector import MbsDetector from .mbs_driver_io import MbsAnalyserDriverIO from .mbs_enums import AcquisitionMode from .mbs_region import MbsRegion, MbsSequence __all__ = [ + "EntranceSlitInformationDevice", + "EntranceSlitInformation", + "SlitPositions", "MbsDetector", "MbsAnalyserDriverIO", "AcquisitionMode", diff --git a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py new file mode 100644 index 00000000000..b8b5cf7dc14 --- /dev/null +++ b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py @@ -0,0 +1,84 @@ +from bluesky.protocols import Reading +from ophyd_async.core import ( + DEFAULT_TIMEOUT, + AsyncStatus, + DeviceMock, + StandardReadable, + StrictEnum, + soft_signal_r_and_setter, +) +from ophyd_async.epics.core import epics_signal_rw +from pydantic import BaseModel + + +class SlitPositions(StrictEnum): + P100_0_1_CURVED = "100 0.1 curved" + P200_0_1_STRAIGHT = "200 0.1 straight" + P300_0_2_CURVED = "300 0.2 curved" + P400_0_2_STRAIGHT = "400 0.2 straight" + P500_0_2_STRAIGHT = "500 0.2 straight" + P600_0_3_STRAIGHT = "600 0.3 straight" + P700_0_5_STRAIGHT = "700 0.5 straight" + P800_0_8_STRAIGHT = "800 0.8 straight" + P850_3_HOLE = "850 3 hole" + P900_1_5_STRAIGHT = "900 1.5 straight" + + +class EntranceSlitInformation(BaseModel): + direction: str = "vertical" + setting: int = 100 + size: float = 0.1 + shape: str = "curved" + + @classmethod + def from_slit_positions(cls, pos: SlitPositions): + setting, size, shape = str(pos).split() + return cls(setting=int(setting), size=float(size), shape=shape) + + def to_slit_postions(self) -> SlitPositions: + return SlitPositions[f"{self.size} {self.setting} {self.shape}"] + + +class EntranceSlitInformationDevice(StandardReadable): + def __init__(self, prefix: str, name: str = ""): + self.slit_info = epics_signal_rw(SlitPositions, prefix) + + default_positions = EntranceSlitInformation() + with self.add_children_as_readables(): + self.direction, self._direction_w = soft_signal_r_and_setter( + str, initial_value=default_positions.direction + ) + self.setting, self._setting_w = soft_signal_r_and_setter( + int, initial_value=default_positions.setting + ) + self.size, self._size_w = soft_signal_r_and_setter( + float, initial_value=default_positions.size + ) + self.shape, self._shape_w = soft_signal_r_and_setter( + str, initial_value=default_positions.shape + ) + super().__init__(name) + + @AsyncStatus.wrap + async def set(self, value: SlitPositions): + await self.slit_info.set(value) + + async def connect( + self, + mock: bool | DeviceMock = False, + timeout: float = DEFAULT_TIMEOUT, + force_reconnect: bool = False, + ) -> None: + await super().connect(mock, timeout, force_reconnect) + + def _sync_soft_signals_with_epics( + value: dict[str, Reading[SlitPositions]], + ) -> None: + val = value[self.slit_info.name]["value"] + new_slit_info = EntranceSlitInformation.from_slit_positions(val) + self._direction_w(new_slit_info.direction) + self._setting_w(new_slit_info.setting) + self._size_w(new_slit_info.size) + self._shape_w(new_slit_info.shape) + + self.slit_info.subscribe(_sync_soft_signals_with_epics) diff --git a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py new file mode 100644 index 00000000000..7d89d05f3d1 --- /dev/null +++ b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py @@ -0,0 +1,44 @@ +import pytest +from ophyd_async.core import init_devices + +from dodal.devices.electron_analyser.mbs import ( + EntranceSlitInformation, + EntranceSlitInformationDevice, + SlitPositions, +) + + +def test_entrance_slit_info_from_slit_positions(): + slit_info = EntranceSlitInformation.from_slit_positions(SlitPositions.P850_3_HOLE) + assert slit_info.setting == 850 + assert slit_info.size == 3.0 + assert slit_info.shape == "hole" + assert slit_info.direction == "vertical" + + slit_info = EntranceSlitInformation.from_slit_positions( + SlitPositions.P300_0_2_CURVED + ) + assert slit_info.setting == 300 + assert slit_info.size == 0.2 + assert slit_info.shape == "curved" + assert slit_info.direction == "vertical" + + +@pytest.fixture +def slit_info_device() -> EntranceSlitInformationDevice: + with init_devices(mock=True): + slit_info_device = EntranceSlitInformationDevice("TEST:") + return slit_info_device + + +@pytest.mark.parametrize("slit_pos", [pos.value for pos in SlitPositions]) +async def test_slit_info_device_soft_signals_sync_with_epics( + slit_info_device: EntranceSlitInformationDevice, slit_pos: SlitPositions +) -> None: + await slit_info_device.set(slit_pos) + + slit_info = EntranceSlitInformation.from_slit_positions(slit_pos) + assert await slit_info_device.setting.get_value() == slit_info.setting + assert await slit_info_device.shape.get_value() == slit_info.shape + assert await slit_info_device.size.get_value() == slit_info.size + assert await slit_info_device.direction.get_value() == slit_info.direction From f2f1b7fefe6359a12b23e4ffad1e524e792ad475 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 28 May 2026 12:47:35 +0000 Subject: [PATCH 2/9] Simplify code --- .../devices/electron_analyser/mbs/__init__.py | 4 +-- .../mbs/mbs_analyser_slits.py | 31 +++++++------------ .../mbs/test_mbs_analyser_slits.py | 18 +++++++---- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/dodal/devices/electron_analyser/mbs/__init__.py b/src/dodal/devices/electron_analyser/mbs/__init__.py index ce84c501b6e..9a70f9801b1 100644 --- a/src/dodal/devices/electron_analyser/mbs/__init__.py +++ b/src/dodal/devices/electron_analyser/mbs/__init__.py @@ -1,7 +1,7 @@ from .mbs_analyser_slits import ( EntranceSlitInformation, EntranceSlitInformationDevice, - SlitPositions, + SlitPosition, ) from .mbs_detector import MbsDetector from .mbs_driver_io import MbsAnalyserDriverIO @@ -11,7 +11,7 @@ __all__ = [ "EntranceSlitInformationDevice", "EntranceSlitInformation", - "SlitPositions", + "SlitPosition", "MbsDetector", "MbsAnalyserDriverIO", "AcquisitionMode", diff --git a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py index b8b5cf7dc14..a78e3fa58f4 100644 --- a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py +++ b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py @@ -11,7 +11,7 @@ from pydantic import BaseModel -class SlitPositions(StrictEnum): +class SlitPosition(StrictEnum): P100_0_1_CURVED = "100 0.1 curved" P200_0_1_STRAIGHT = "200 0.1 straight" P300_0_2_CURVED = "300 0.2 curved" @@ -31,36 +31,27 @@ class EntranceSlitInformation(BaseModel): shape: str = "curved" @classmethod - def from_slit_positions(cls, pos: SlitPositions): + def from_slit_positions(cls, pos: SlitPosition): setting, size, shape = str(pos).split() return cls(setting=int(setting), size=float(size), shape=shape) - def to_slit_postions(self) -> SlitPositions: - return SlitPositions[f"{self.size} {self.setting} {self.shape}"] + def to_slit_position(self) -> SlitPosition: + return SlitPosition(f"{self.setting} {self.size:g} {self.shape}") class EntranceSlitInformationDevice(StandardReadable): def __init__(self, prefix: str, name: str = ""): - self.slit_info = epics_signal_rw(SlitPositions, prefix) + self.slit_info = epics_signal_rw(SlitPosition, prefix) - default_positions = EntranceSlitInformation() with self.add_children_as_readables(): - self.direction, self._direction_w = soft_signal_r_and_setter( - str, initial_value=default_positions.direction - ) - self.setting, self._setting_w = soft_signal_r_and_setter( - int, initial_value=default_positions.setting - ) - self.size, self._size_w = soft_signal_r_and_setter( - float, initial_value=default_positions.size - ) - self.shape, self._shape_w = soft_signal_r_and_setter( - str, initial_value=default_positions.shape - ) + self.direction, self._direction_w = soft_signal_r_and_setter(str) + self.setting, self._setting_w = soft_signal_r_and_setter(int) + self.size, self._size_w = soft_signal_r_and_setter(float) + self.shape, self._shape_w = soft_signal_r_and_setter(str) super().__init__(name) @AsyncStatus.wrap - async def set(self, value: SlitPositions): + async def set(self, value: SlitPosition): await self.slit_info.set(value) async def connect( @@ -72,7 +63,7 @@ async def connect( await super().connect(mock, timeout, force_reconnect) def _sync_soft_signals_with_epics( - value: dict[str, Reading[SlitPositions]], + value: dict[str, Reading[SlitPosition]], ) -> None: val = value[self.slit_info.name]["value"] new_slit_info = EntranceSlitInformation.from_slit_positions(val) diff --git a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py index 7d89d05f3d1..aa61be68afd 100644 --- a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py +++ b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py @@ -4,19 +4,25 @@ from dodal.devices.electron_analyser.mbs import ( EntranceSlitInformation, EntranceSlitInformationDevice, - SlitPositions, + SlitPosition, ) -def test_entrance_slit_info_from_slit_positions(): - slit_info = EntranceSlitInformation.from_slit_positions(SlitPositions.P850_3_HOLE) +@pytest.mark.parametrize("slit_pos", [pos.value for pos in SlitPosition]) +def test_entrance_slit_info_to_slit_position(slit_pos: SlitPosition): + slit_info = EntranceSlitInformation.from_slit_positions(slit_pos) + assert slit_info.to_slit_position() == slit_pos + + +def test_entrance_slit_info_from_slit_position(): + slit_info = EntranceSlitInformation.from_slit_positions(SlitPosition.P850_3_HOLE) assert slit_info.setting == 850 assert slit_info.size == 3.0 assert slit_info.shape == "hole" assert slit_info.direction == "vertical" slit_info = EntranceSlitInformation.from_slit_positions( - SlitPositions.P300_0_2_CURVED + SlitPosition.P300_0_2_CURVED ) assert slit_info.setting == 300 assert slit_info.size == 0.2 @@ -31,9 +37,9 @@ def slit_info_device() -> EntranceSlitInformationDevice: return slit_info_device -@pytest.mark.parametrize("slit_pos", [pos.value for pos in SlitPositions]) +@pytest.mark.parametrize("slit_pos", [pos.value for pos in SlitPosition]) async def test_slit_info_device_soft_signals_sync_with_epics( - slit_info_device: EntranceSlitInformationDevice, slit_pos: SlitPositions + slit_info_device: EntranceSlitInformationDevice, slit_pos: SlitPosition ) -> None: await slit_info_device.set(slit_pos) From eeff228d201c4b94cb680d2dc5eeff2050f55770 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 28 May 2026 13:21:36 +0000 Subject: [PATCH 3/9] Update doc string --- .../devices/electron_analyser/mbs/mbs_analyser_slits.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py index a78e3fa58f4..37be5d490cf 100644 --- a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py +++ b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py @@ -40,9 +40,14 @@ def to_slit_position(self) -> SlitPosition: class EntranceSlitInformationDevice(StandardReadable): - def __init__(self, prefix: str, name: str = ""): - self.slit_info = epics_signal_rw(SlitPosition, prefix) + """Device that connects to epics signal containing slit information from an enum + value. This is synced with soft signals as individual signals which can be added as + config_signals to give to detectors to save as nicely formatted data. + """ + def __init__(self, pv: str, name: str = ""): + self.slit_info = epics_signal_rw(SlitPosition, pv) + # Formatted slit info as individual soft signals for metadata with self.add_children_as_readables(): self.direction, self._direction_w = soft_signal_r_and_setter(str) self.setting, self._setting_w = soft_signal_r_and_setter(int) From 6c4a6483f6e7183f73b9ae35c6954ecb3dcf3f2f Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 28 May 2026 13:22:01 +0000 Subject: [PATCH 4/9] Add analyser_slits to beamlines --- src/dodal/beamlines/i05.py | 21 ++++++++++++++++-- src/dodal/beamlines/i05_1.py | 22 +++++++++++++++++-- .../electron_analyser/mbs/mbs_detector.py | 3 +++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/dodal/beamlines/i05.py b/src/dodal/beamlines/i05.py index c0a3d525923..f03e69e6101 100644 --- a/src/dodal/beamlines/i05.py +++ b/src/dodal/beamlines/i05.py @@ -4,7 +4,10 @@ from dodal.devices.beamlines.i05 import I05Goniometer from dodal.devices.beamlines.i05_shared import LensMode, M4M5Mirror, PassEnergy from dodal.devices.common_mirror import XYZSwitchingMirror -from dodal.devices.electron_analyser.mbs import MbsDetector +from dodal.devices.electron_analyser.mbs import ( + EntranceSlitInformationDevice, + MbsDetector, +) from dodal.devices.hutch_shutter import HutchShutter from dodal.devices.pgm import PlaneGratingMonochromator from dodal.devices.temperture_controller import Lakeshore336 @@ -51,10 +54,24 @@ def sa() -> I05Goniometer: @devices.factory -def analyser(pgm: PlaneGratingMonochromator) -> MbsDetector[LensMode, PassEnergy]: +def analyser_slits() -> EntranceSlitInformationDevice: + return EntranceSlitInformationDevice(f"{PREFIX.beamline_prefix}-EA-SLITS-01:POS") + + +@devices.factory +def analyser( + pgm: PlaneGratingMonochromator, analyser_slits: EntranceSlitInformationDevice +) -> MbsDetector[LensMode, PassEnergy]: + config_sigs = ( + analyser_slits.direction, + analyser_slits.size, + analyser_slits.shape, + analyser_slits.setting, + ) return MbsDetector[LensMode, PassEnergy]( prefix=f"{PREFIX.beamline_prefix}-EA-DET-02:CAM:", lens_mode_type=LensMode, pass_energy_type=PassEnergy, energy_source=pgm.energy.user_readback, + config_sigs=config_sigs, ) diff --git a/src/dodal/beamlines/i05_1.py b/src/dodal/beamlines/i05_1.py index 62e62d68c56..d1bdfa1c9cd 100644 --- a/src/dodal/beamlines/i05_1.py +++ b/src/dodal/beamlines/i05_1.py @@ -4,7 +4,10 @@ from dodal.devices.beamlines.i05_1 import XYZAzimuthPolarDefocusStage from dodal.devices.beamlines.i05_shared import LensMode, Mj7j8Mirror, PassEnergy from dodal.devices.common_mirror import XYZPiezoSwitchingMirror -from dodal.devices.electron_analyser.mbs import MbsDetector +from dodal.devices.electron_analyser.mbs import ( + EntranceSlitInformationDevice, + MbsDetector, +) from dodal.devices.hutch_shutter import HutchShutter from dodal.devices.pgm import PlaneGratingMonochromator from dodal.log import set_beamline as set_log_beamline @@ -39,11 +42,26 @@ def sm() -> XYZAzimuthPolarDefocusStage: return XYZAzimuthPolarDefocusStage(prefix=f"{PREFIX.beamline_prefix}-EA-SM-01:") +# Note: Currently fails due to i05-XXX @devices.factory -def analyser(pgm: PlaneGratingMonochromator) -> MbsDetector[LensMode, PassEnergy]: +def analyser_slits() -> EntranceSlitInformationDevice: + return EntranceSlitInformationDevice(f"{PREFIX.beamline_prefix}-EA-SLITS-01:POS") + + +@devices.factory +def analyser( + pgm: PlaneGratingMonochromator, analyser_slits: EntranceSlitInformationDevice +) -> MbsDetector[LensMode, PassEnergy]: + config_sigs = ( + analyser_slits.direction, + analyser_slits.size, + analyser_slits.shape, + analyser_slits.setting, + ) return MbsDetector[LensMode, PassEnergy]( prefix=f"{PREFIX.beamline_prefix}-EA-DET-04:CAM:", lens_mode_type=LensMode, pass_energy_type=PassEnergy, energy_source=pgm.energy.user_readback, + config_sigs=config_sigs, ) diff --git a/src/dodal/devices/electron_analyser/mbs/mbs_detector.py b/src/dodal/devices/electron_analyser/mbs/mbs_detector.py index 1ac189d36f5..40dda68c3e9 100644 --- a/src/dodal/devices/electron_analyser/mbs/mbs_detector.py +++ b/src/dodal/devices/electron_analyser/mbs/mbs_detector.py @@ -1,3 +1,4 @@ +from collections.abc import Sequence from typing import Generic from ophyd_async.core import SignalR, soft_signal_rw @@ -32,6 +33,7 @@ def __init__( energy_source: SignalR[float], shutter: GenericFastShutter | None = None, source_selector: SourceSelector | None = None, + config_sigs: Sequence[SignalR] = (), name: str = "", ): # Make attribute of class so connect applies to driver and populates parent. @@ -47,6 +49,7 @@ def __init__( ) trigger_logic = ElectronAnalayserTriggerLogic(self.driver, set()) config_sigs = ( + *config_sigs, self.driver.region_name, self.driver.energy_mode, self.driver.acquisition_mode, From 14fa5dd207c1137154e10a1b1e2db9f3cb633a3e Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 28 May 2026 13:33:55 +0000 Subject: [PATCH 5/9] Add read test, update variable names --- .../mbs/mbs_analyser_slits.py | 8 ++++---- .../mbs/test_mbs_analyser_slits.py | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py index 37be5d490cf..3306ce11f04 100644 --- a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py +++ b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py @@ -46,7 +46,7 @@ class EntranceSlitInformationDevice(StandardReadable): """ def __init__(self, pv: str, name: str = ""): - self.slit_info = epics_signal_rw(SlitPosition, pv) + self.slit_pos = epics_signal_rw(SlitPosition, pv) # Formatted slit info as individual soft signals for metadata with self.add_children_as_readables(): self.direction, self._direction_w = soft_signal_r_and_setter(str) @@ -57,7 +57,7 @@ def __init__(self, pv: str, name: str = ""): @AsyncStatus.wrap async def set(self, value: SlitPosition): - await self.slit_info.set(value) + await self.slit_pos.set(value) async def connect( self, @@ -70,11 +70,11 @@ async def connect( def _sync_soft_signals_with_epics( value: dict[str, Reading[SlitPosition]], ) -> None: - val = value[self.slit_info.name]["value"] + val = value[self.slit_pos.name]["value"] new_slit_info = EntranceSlitInformation.from_slit_positions(val) self._direction_w(new_slit_info.direction) self._setting_w(new_slit_info.setting) self._size_w(new_slit_info.size) self._shape_w(new_slit_info.shape) - self.slit_info.subscribe(_sync_soft_signals_with_epics) + self.slit_pos.subscribe(_sync_soft_signals_with_epics) diff --git a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py index aa61be68afd..072a00c5566 100644 --- a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py +++ b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py @@ -1,5 +1,6 @@ import pytest from ophyd_async.core import init_devices +from ophyd_async.testing import assert_reading, partial_reading from dodal.devices.electron_analyser.mbs import ( EntranceSlitInformation, @@ -48,3 +49,21 @@ async def test_slit_info_device_soft_signals_sync_with_epics( assert await slit_info_device.shape.get_value() == slit_info.shape assert await slit_info_device.size.get_value() == slit_info.size assert await slit_info_device.direction.get_value() == slit_info.direction + + +@pytest.mark.parametrize("slit_pos", [pos.value for pos in SlitPosition]) +async def test_slit_info_device_read_and_soft_signals_sync_with_epics( + slit_info_device: EntranceSlitInformationDevice, slit_pos: SlitPosition +) -> None: + await slit_info_device.set(slit_pos) + slit_info = EntranceSlitInformation.from_slit_positions(slit_pos) + + await assert_reading( + slit_info_device, + { + "slit_info_device-size": partial_reading(slit_info.size), + "slit_info_device-shape": partial_reading(slit_info.shape), + "slit_info_device-setting": partial_reading(slit_info.setting), + "slit_info_device-direction": partial_reading(slit_info.direction), + }, + ) From ae3db99db8be9f278dfe4f28004b356f30d5fa19 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 28 May 2026 13:39:34 +0000 Subject: [PATCH 6/9] Link Jira ticket --- src/dodal/beamlines/i05_1.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dodal/beamlines/i05_1.py b/src/dodal/beamlines/i05_1.py index d1bdfa1c9cd..75b834843be 100644 --- a/src/dodal/beamlines/i05_1.py +++ b/src/dodal/beamlines/i05_1.py @@ -42,7 +42,7 @@ def sm() -> XYZAzimuthPolarDefocusStage: return XYZAzimuthPolarDefocusStage(prefix=f"{PREFIX.beamline_prefix}-EA-SM-01:") -# Note: Currently fails due to i05-XXX +# Note: Currently fails. Requires https://jira.diamond.ac.uk/browse/I05-764 @devices.factory def analyser_slits() -> EntranceSlitInformationDevice: return EntranceSlitInformationDevice(f"{PREFIX.beamline_prefix}-EA-SLITS-01:POS") From 239e45ab0611cda88cea746b027a71c5fd85cd46 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 2 Jun 2026 12:33:07 +0000 Subject: [PATCH 7/9] Add test to confirm only one subscriber added on connect --- .../mbs/mbs_analyser_slits.py | 30 +++++++++++-------- .../mbs/test_mbs_analyser_slits.py | 13 ++++++++ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py index 3306ce11f04..af877354dcb 100644 --- a/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py +++ b/src/dodal/devices/electron_analyser/mbs/mbs_analyser_slits.py @@ -1,3 +1,5 @@ +from typing import Self + from bluesky.protocols import Reading from ophyd_async.core import ( DEFAULT_TIMEOUT, @@ -31,7 +33,7 @@ class EntranceSlitInformation(BaseModel): shape: str = "curved" @classmethod - def from_slit_positions(cls, pos: SlitPosition): + def from_slit_positions(cls, pos: SlitPosition) -> Self: setting, size, shape = str(pos).split() return cls(setting=int(setting), size=float(size), shape=shape) @@ -59,6 +61,17 @@ def __init__(self, pv: str, name: str = ""): async def set(self, value: SlitPosition): await self.slit_pos.set(value) + def _sync_soft_signals_with_epics( + self, + value: dict[str, Reading[SlitPosition]], + ) -> None: + val = value[self.slit_pos.name]["value"] + new_slit_info = EntranceSlitInformation.from_slit_positions(val) + self._direction_w(new_slit_info.direction) + self._setting_w(new_slit_info.setting) + self._size_w(new_slit_info.size) + self._shape_w(new_slit_info.shape) + async def connect( self, mock: bool | DeviceMock = False, @@ -66,15 +79,6 @@ async def connect( force_reconnect: bool = False, ) -> None: await super().connect(mock, timeout, force_reconnect) - - def _sync_soft_signals_with_epics( - value: dict[str, Reading[SlitPosition]], - ) -> None: - val = value[self.slit_pos.name]["value"] - new_slit_info = EntranceSlitInformation.from_slit_positions(val) - self._direction_w(new_slit_info.direction) - self._setting_w(new_slit_info.setting) - self._size_w(new_slit_info.size) - self._shape_w(new_slit_info.shape) - - self.slit_pos.subscribe(_sync_soft_signals_with_epics) + # subscribe_reading listeners are stored in a set, so repeatedly subscribing + # this bound method is harmless and does not create duplicate callbacks. + self.slit_pos.subscribe_reading(self._sync_soft_signals_with_epics) diff --git a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py index 072a00c5566..7e2e0251b30 100644 --- a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py +++ b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py @@ -67,3 +67,16 @@ async def test_slit_info_device_read_and_soft_signals_sync_with_epics( "slit_info_device-direction": partial_reading(slit_info.direction), }, ) + + +async def test_slit_info_device_multiple_connects_has_one_subscribe( + slit_info_device: EntranceSlitInformationDevice, +): + expected_subscribers = 1 + number_of_subscribers = len(slit_info_device.slit_pos._get_cache()._listeners) + assert number_of_subscribers == expected_subscribers + # Test if we connect again if another subscriber is added, checking for memory leaks. + await slit_info_device.connect(mock=True) + assert ( + len(slit_info_device.slit_pos._get_cache()._listeners) == expected_subscribers + ) From d2c26dbd48e7638a1ddacfe77943be59ed8f293d Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 2 Jun 2026 13:00:15 +0000 Subject: [PATCH 8/9] Apply the config_sigs directly --- src/dodal/beamlines/i05.py | 13 ++++++------- src/dodal/beamlines/i05_1.py | 13 ++++++------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/dodal/beamlines/i05.py b/src/dodal/beamlines/i05.py index f03e69e6101..42e3a9292e2 100644 --- a/src/dodal/beamlines/i05.py +++ b/src/dodal/beamlines/i05.py @@ -62,16 +62,15 @@ def analyser_slits() -> EntranceSlitInformationDevice: def analyser( pgm: PlaneGratingMonochromator, analyser_slits: EntranceSlitInformationDevice ) -> MbsDetector[LensMode, PassEnergy]: - config_sigs = ( - analyser_slits.direction, - analyser_slits.size, - analyser_slits.shape, - analyser_slits.setting, - ) return MbsDetector[LensMode, PassEnergy]( prefix=f"{PREFIX.beamline_prefix}-EA-DET-02:CAM:", lens_mode_type=LensMode, pass_energy_type=PassEnergy, energy_source=pgm.energy.user_readback, - config_sigs=config_sigs, + config_sigs=( + analyser_slits.direction, + analyser_slits.size, + analyser_slits.shape, + analyser_slits.setting, + ), ) diff --git a/src/dodal/beamlines/i05_1.py b/src/dodal/beamlines/i05_1.py index 75b834843be..6d80d0ddf0d 100644 --- a/src/dodal/beamlines/i05_1.py +++ b/src/dodal/beamlines/i05_1.py @@ -52,16 +52,15 @@ def analyser_slits() -> EntranceSlitInformationDevice: def analyser( pgm: PlaneGratingMonochromator, analyser_slits: EntranceSlitInformationDevice ) -> MbsDetector[LensMode, PassEnergy]: - config_sigs = ( - analyser_slits.direction, - analyser_slits.size, - analyser_slits.shape, - analyser_slits.setting, - ) return MbsDetector[LensMode, PassEnergy]( prefix=f"{PREFIX.beamline_prefix}-EA-DET-04:CAM:", lens_mode_type=LensMode, pass_energy_type=PassEnergy, energy_source=pgm.energy.user_readback, - config_sigs=config_sigs, + config_sigs=( + analyser_slits.direction, + analyser_slits.size, + analyser_slits.shape, + analyser_slits.setting, + ), ) From 685f269b356835bf5d347fe554bedc63a489d856 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 2 Jun 2026 13:43:37 +0000 Subject: [PATCH 9/9] Add for loop for mbs test --- .../electron_analyser/mbs/test_mbs_analyser_slits.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py index 7e2e0251b30..be8ef567243 100644 --- a/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py +++ b/tests/devices/electron_analyser/mbs/test_mbs_analyser_slits.py @@ -76,7 +76,9 @@ async def test_slit_info_device_multiple_connects_has_one_subscribe( number_of_subscribers = len(slit_info_device.slit_pos._get_cache()._listeners) assert number_of_subscribers == expected_subscribers # Test if we connect again if another subscriber is added, checking for memory leaks. - await slit_info_device.connect(mock=True) - assert ( - len(slit_info_device.slit_pos._get_cache()._listeners) == expected_subscribers - ) + for _ in range(3): + await slit_info_device.connect(mock=True) + assert ( + len(slit_info_device.slit_pos._get_cache()._listeners) + == expected_subscribers + )