From fd58a3ddd10c6e641f4eece91aa8ee2198c59f44 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 5 Jan 2026 15:44:18 +0000 Subject: [PATCH 01/11] Use a single electron analyser detector --- .../electron_analyser/base/__init__.py | 8 - .../electron_analyser/base/base_detector.py | 173 +++++------------- 2 files changed, 49 insertions(+), 132 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/__init__.py b/src/dodal/devices/electron_analyser/base/__init__.py index f272998bac..8eb660cb33 100644 --- a/src/dodal/devices/electron_analyser/base/__init__.py +++ b/src/dodal/devices/electron_analyser/base/__init__.py @@ -3,12 +3,8 @@ GenericElectronAnalyserController, ) from .base_detector import ( - BaseElectronAnalyserDetector, ElectronAnalyserDetector, - ElectronAnalyserRegionDetector, - GenericBaseElectronAnalyserDetector, GenericElectronAnalyserDetector, - GenericElectronAnalyserRegionDetector, ) from .base_driver_io import ( AbstractAnalyserDriverIO, @@ -32,12 +28,8 @@ __all__ = [ "ElectronAnalyserController", "GenericElectronAnalyserController", - "BaseElectronAnalyserDetector", "ElectronAnalyserDetector", - "ElectronAnalyserRegionDetector", - "GenericBaseElectronAnalyserDetector", "GenericElectronAnalyserDetector", - "GenericElectronAnalyserRegionDetector", "AbstractAnalyserDriverIO", "GenericAnalyserDriverIO", "TAbstractAnalyserDriverIO", diff --git a/src/dodal/devices/electron_analyser/base/base_detector.py b/src/dodal/devices/electron_analyser/base/base_detector.py index bcd68e4fc9..6655444e1c 100644 --- a/src/dodal/devices/electron_analyser/base/base_detector.py +++ b/src/dodal/devices/electron_analyser/base/base_detector.py @@ -1,6 +1,6 @@ from typing import Generic, TypeVar -from bluesky.protocols import Reading, Stageable, Triggerable +from bluesky.protocols import Movable, Reading, Stageable, Triggerable from event_model import DataKey from ophyd_async.core import ( AsyncConfigurable, @@ -8,6 +8,7 @@ AsyncStatus, Device, TriggerInfo, + soft_signal_rw, ) from dodal.common.data_util import load_json_file_to_class @@ -26,16 +27,51 @@ ) -class BaseElectronAnalyserDetector( +class SequenceLoader(Device, Movable[str], Generic[TAbstractBaseSequence]): + """ + Docstring for SequenceLoader + + :var Args: Description + :var filename: Description + :vartype filename: Path + :var Returns: Description + :var https: Description + """ + + def __init__(self, sequence_class: type[TAbstractBaseSequence], name): + self.sequence_file = soft_signal_rw(str, initial_value="Not set") + self._sequence_class = sequence_class + + @AsyncStatus.wrap + async def set(self, filename: str) -> None: + await self.sequence_file.set(filename) + + async def load_sequence(self) -> TAbstractBaseSequence: + """ + Load the sequence data from a provided json file into a sequence class. + + Args: + filename: Path to the sequence file containing the region data. + + Returns: + Pydantic model representing the sequence file. + """ + return load_json_file_to_class( + self._sequence_class, await self.sequence_file.get_value() + ) + + +class ElectronAnalyserDetector( Device, Triggerable, AsyncReadable, AsyncConfigurable, - Generic[TAbstractAnalyserDriverIO, TAbstractBaseRegion], + Stageable, + Movable, + Generic[TAbstractBaseSequence, TAbstractAnalyserDriverIO, TAbstractBaseRegion], ): """ - Detector for data acquisition of electron analyser. Can only acquire using settings - already configured for the device. + Detector for data acquisition of electron analyser. If possible, this should be changed to inherit from a StandardDetector. Currently, StandardDetector forces you to use a file writer which doesn't apply here. @@ -44,14 +80,22 @@ class BaseElectronAnalyserDetector( def __init__( self, + sequence_class: type[TAbstractBaseSequence], controller: ElectronAnalyserController[ TAbstractAnalyserDriverIO, TAbstractBaseRegion ], name: str = "", ): self._controller = controller + self._sequence_class = sequence_class + super().__init__(name) + @AsyncStatus.wrap + async def stage(self) -> None: + """Disarm the detector.""" + await self._controller.disarm() + @AsyncStatus.wrap async def set(self, region: TAbstractBaseRegion) -> None: await self._controller.setup_with_region(region) @@ -80,130 +124,11 @@ async def read_configuration(self) -> dict[str, Reading]: async def describe_configuration(self) -> dict[str, DataKey]: return await self._controller.driver.describe_configuration() - -GenericBaseElectronAnalyserDetector = BaseElectronAnalyserDetector[ - GenericAnalyserDriverIO, GenericRegion -] - - -class ElectronAnalyserRegionDetector( - BaseElectronAnalyserDetector[TAbstractAnalyserDriverIO, TAbstractBaseRegion], - Generic[TAbstractAnalyserDriverIO, TAbstractBaseRegion], -): - """ - Extends electron analyser detector to configure specific region settings before data - acquisition. It is designed to only exist inside a plan. - """ - - def __init__( - self, - controller: ElectronAnalyserController[ - TAbstractAnalyserDriverIO, TAbstractBaseRegion - ], - region: TAbstractBaseRegion, - name: str = "", - ): - self.region = region - super().__init__(controller, name) - - @AsyncStatus.wrap - async def trigger(self) -> None: - # Configure region parameters on the driver first before data collection. - await self.set(self.region) - await super().trigger() - - -GenericElectronAnalyserRegionDetector = ElectronAnalyserRegionDetector[ - GenericAnalyserDriverIO, GenericRegion -] -TElectronAnalyserRegionDetector = TypeVar( - "TElectronAnalyserRegionDetector", - bound=ElectronAnalyserRegionDetector, -) - - -class ElectronAnalyserDetector( - BaseElectronAnalyserDetector[TAbstractAnalyserDriverIO, TAbstractBaseRegion], - Stageable, - Generic[TAbstractBaseSequence, TAbstractAnalyserDriverIO, TAbstractBaseRegion], -): - """ - Electron analyser detector with the additional functionality to load a sequence file - and create a list of temporary ElectronAnalyserRegionDetector objects. These will - setup configured region settings before data acquisition. - """ - - def __init__( - self, - sequence_class: type[TAbstractBaseSequence], - controller: ElectronAnalyserController[ - TAbstractAnalyserDriverIO, TAbstractBaseRegion - ], - name: str = "", - ): - self._sequence_class = sequence_class - super().__init__(controller, name) - - @AsyncStatus.wrap - async def stage(self) -> None: - """ - Prepare the detector for use by ensuring it is idle and ready. - - This method asynchronously stages the detector by first disarming the controller - to ensure the detector is not actively acquiring data, then invokes the driver's - stage procedure. This ensures the detector is in a known, ready state - before use. - - Raises: - Any exceptions raised by the driver's stage or controller's disarm methods. - """ - await self._controller.disarm() - @AsyncStatus.wrap async def unstage(self) -> None: """Disarm the detector.""" await self._controller.disarm() - def load_sequence(self, filename: str) -> TAbstractBaseSequence: - """ - Load the sequence data from a provided json file into a sequence class. - - Args: - filename: Path to the sequence file containing the region data. - - Returns: - Pydantic model representing the sequence file. - """ - return load_json_file_to_class(self._sequence_class, filename) - - def create_region_detector_list( - self, filename: str, enabled_only=True - ) -> list[ - ElectronAnalyserRegionDetector[TAbstractAnalyserDriverIO, TAbstractBaseRegion] - ]: - """ - Create a list of detectors equal to the number of regions in a sequence file. - Each detector is responsible for setting up a specific region. - - Args: - filename: Path to the sequence file containing the region data. - enabled_only: If true, only include the region if enabled is True. - - Returns: - List of ElectronAnalyserRegionDetector, equal to the number of regions in - the sequence file. - """ - seq = self.load_sequence(filename) - regions: list[TAbstractBaseRegion] = ( - seq.get_enabled_regions() if enabled_only else seq.regions - ) - return [ - ElectronAnalyserRegionDetector[ - TAbstractAnalyserDriverIO, TAbstractBaseRegion - ](self._controller, r, self.name + "_" + r.name) - for r in regions - ] - GenericElectronAnalyserDetector = ElectronAnalyserDetector[ GenericSequence, GenericAnalyserDriverIO, GenericRegion From 16c7351e40ab5780eca3c8734285048a740bf664 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 5 Jan 2026 15:50:17 +0000 Subject: [PATCH 02/11] Applied changes to detectors --- .../devices/electron_analyser/base/base_detector.py | 7 ++----- .../devices/electron_analyser/specs/specs_detector.py | 10 ++-------- .../electron_analyser/vgscienta/vgscienta_detector.py | 7 +------ 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/base_detector.py b/src/dodal/devices/electron_analyser/base/base_detector.py index 6655444e1c..eb98299bb5 100644 --- a/src/dodal/devices/electron_analyser/base/base_detector.py +++ b/src/dodal/devices/electron_analyser/base/base_detector.py @@ -21,7 +21,6 @@ ) from dodal.devices.electron_analyser.base.base_region import ( GenericRegion, - GenericSequence, TAbstractBaseRegion, TAbstractBaseSequence, ) @@ -68,7 +67,7 @@ class ElectronAnalyserDetector( AsyncConfigurable, Stageable, Movable, - Generic[TAbstractBaseSequence, TAbstractAnalyserDriverIO, TAbstractBaseRegion], + Generic[TAbstractAnalyserDriverIO, TAbstractBaseRegion], ): """ Detector for data acquisition of electron analyser. @@ -80,14 +79,12 @@ class ElectronAnalyserDetector( def __init__( self, - sequence_class: type[TAbstractBaseSequence], controller: ElectronAnalyserController[ TAbstractAnalyserDriverIO, TAbstractBaseRegion ], name: str = "", ): self._controller = controller - self._sequence_class = sequence_class super().__init__(name) @@ -131,7 +128,7 @@ async def unstage(self) -> None: GenericElectronAnalyserDetector = ElectronAnalyserDetector[ - GenericSequence, GenericAnalyserDriverIO, GenericRegion + GenericAnalyserDriverIO, GenericRegion ] TElectronAnalyserDetector = TypeVar( "TElectronAnalyserDetector", diff --git a/src/dodal/devices/electron_analyser/specs/specs_detector.py b/src/dodal/devices/electron_analyser/specs/specs_detector.py index 9943a892b8..d8e66ee50b 100644 --- a/src/dodal/devices/electron_analyser/specs/specs_detector.py +++ b/src/dodal/devices/electron_analyser/specs/specs_detector.py @@ -10,15 +10,11 @@ EnergySource, ) from dodal.devices.electron_analyser.specs.specs_driver_io import SpecsAnalyserDriverIO -from dodal.devices.electron_analyser.specs.specs_region import ( - SpecsRegion, - SpecsSequence, -) +from dodal.devices.electron_analyser.specs.specs_region import SpecsRegion class SpecsDetector( ElectronAnalyserDetector[ - SpecsSequence[TLensMode, TPsuMode], SpecsAnalyserDriverIO[TLensMode, TPsuMode], SpecsRegion[TLensMode, TPsuMode], ], @@ -41,6 +37,4 @@ def __init__( SpecsAnalyserDriverIO[TLensMode, TPsuMode], SpecsRegion[TLensMode, TPsuMode] ](self.driver, energy_source, 0) - sequence_class = SpecsSequence[lens_mode_type, psu_mode_type] - - super().__init__(sequence_class, controller, name) + super().__init__(controller, name) diff --git a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py index b3645f4661..6c77e47fee 100644 --- a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py +++ b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py @@ -15,13 +15,11 @@ from dodal.devices.electron_analyser.vgscienta.vgscienta_region import ( TPassEnergyEnum, VGScientaRegion, - VGScientaSequence, ) class VGScientaDetector( ElectronAnalyserDetector[ - VGScientaSequence[TLensMode, TPsuMode, TPassEnergyEnum], VGScientaAnalyserDriverIO[TLensMode, TPsuMode, TPassEnergyEnum], VGScientaRegion[TLensMode, TPassEnergyEnum], ], @@ -46,7 +44,4 @@ def __init__( VGScientaRegion[TLensMode, TPassEnergyEnum], ](self.driver, energy_source, 0) - sequence_class = VGScientaSequence[ - lens_mode_type, psu_mode_type, pass_energy_type - ] - super().__init__(sequence_class, controller, name) + super().__init__(controller, name) From 7fc80d7ff0ae7855b15b39f361f520effa8457bc Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 5 Jan 2026 15:50:33 +0000 Subject: [PATCH 03/11] Corrected SequenceLoader with name --- .../devices/electron_analyser/base/base_detector.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/base_detector.py b/src/dodal/devices/electron_analyser/base/base_detector.py index eb98299bb5..a4e7c268ab 100644 --- a/src/dodal/devices/electron_analyser/base/base_detector.py +++ b/src/dodal/devices/electron_analyser/base/base_detector.py @@ -37,9 +37,15 @@ class SequenceLoader(Device, Movable[str], Generic[TAbstractBaseSequence]): :var https: Description """ - def __init__(self, sequence_class: type[TAbstractBaseSequence], name): - self.sequence_file = soft_signal_rw(str, initial_value="Not set") + def __init__( + self, + sequence_class: type[TAbstractBaseSequence], + initial_file: str = "Not set", + name: str = "", + ): + self.sequence_file = soft_signal_rw(str, initial_value=initial_file) self._sequence_class = sequence_class + super().__init__(name) @AsyncStatus.wrap async def set(self, filename: str) -> None: From 2fa90f5dc7cc53dfd4c1f384d4a3570e5c4488d3 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 5 Jan 2026 16:22:13 +0000 Subject: [PATCH 04/11] Moved SequenceLoader to base_region --- .../electron_analyser/base/base_detector.py | 43 ---------------- .../electron_analyser/base/base_region.py | 50 ++++++++++++++++++- 2 files changed, 49 insertions(+), 44 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/base_detector.py b/src/dodal/devices/electron_analyser/base/base_detector.py index a4e7c268ab..5946c9aa3f 100644 --- a/src/dodal/devices/electron_analyser/base/base_detector.py +++ b/src/dodal/devices/electron_analyser/base/base_detector.py @@ -8,10 +8,8 @@ AsyncStatus, Device, TriggerInfo, - soft_signal_rw, ) -from dodal.common.data_util import load_json_file_to_class from dodal.devices.electron_analyser.base.base_controller import ( ElectronAnalyserController, ) @@ -22,50 +20,9 @@ from dodal.devices.electron_analyser.base.base_region import ( GenericRegion, TAbstractBaseRegion, - TAbstractBaseSequence, ) -class SequenceLoader(Device, Movable[str], Generic[TAbstractBaseSequence]): - """ - Docstring for SequenceLoader - - :var Args: Description - :var filename: Description - :vartype filename: Path - :var Returns: Description - :var https: Description - """ - - def __init__( - self, - sequence_class: type[TAbstractBaseSequence], - initial_file: str = "Not set", - name: str = "", - ): - self.sequence_file = soft_signal_rw(str, initial_value=initial_file) - self._sequence_class = sequence_class - super().__init__(name) - - @AsyncStatus.wrap - async def set(self, filename: str) -> None: - await self.sequence_file.set(filename) - - async def load_sequence(self) -> TAbstractBaseSequence: - """ - Load the sequence data from a provided json file into a sequence class. - - Args: - filename: Path to the sequence file containing the region data. - - Returns: - Pydantic model representing the sequence file. - """ - return load_json_file_to_class( - self._sequence_class, await self.sequence_file.get_value() - ) - - class ElectronAnalyserDetector( Device, Triggerable, diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 870ef87b38..909f6bf0c9 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -3,9 +3,17 @@ from collections.abc import Callable from typing import Generic, Self, TypeAlias, TypeVar -from ophyd_async.core import StrictEnum, SupersetEnum +from bluesky.protocols import Movable +from ophyd_async.core import ( + AsyncStatus, + Device, + StrictEnum, + SupersetEnum, + soft_signal_rw, +) from pydantic import BaseModel, Field, model_validator +from dodal.common.data_util import load_json_file_to_class from dodal.devices.electron_analyser.base.base_enums import EnergyMode, SelectedSource from dodal.devices.electron_analyser.base.base_util import ( to_binding_energy, @@ -200,3 +208,43 @@ def get_region_by_name(self, name: str) -> TAbstractBaseRegion | None: GenericSequence = AbstractBaseSequence[GenericRegion] TAbstractBaseSequence = TypeVar("TAbstractBaseSequence", bound=AbstractBaseSequence) + + +class SequenceLoader(Device, Movable[str], Generic[TAbstractBaseSequence]): + """ + Docstring for SequenceLoader + + :var Args: Description + :var filename: Description + :vartype filename: Path + :var Returns: Description + :var https: Description + """ + + def __init__( + self, + sequence_class: type[TAbstractBaseSequence], + initial_file: str = "Not set", + name: str = "", + ): + self.sequence_file = soft_signal_rw(str, initial_value=initial_file) + self._sequence_class = sequence_class + super().__init__(name) + + @AsyncStatus.wrap + async def set(self, filename: str) -> None: + await self.sequence_file.set(filename) + + async def load(self) -> TAbstractBaseSequence: + """ + Load the sequence data from a provided json file into a sequence class. + + Args: + filename: Path to the sequence file containing the region data. + + Returns: + Pydantic model representing the sequence file. + """ + return load_json_file_to_class( + self._sequence_class, await self.sequence_file.get_value() + ) From 32988539cad66a18f050599f7feebede2c88a40f Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 6 Jan 2026 10:13:31 +0000 Subject: [PATCH 05/11] Improved way of getting sequence data --- .../electron_analyser/base/base_region.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 909f6bf0c9..108f0868b0 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -9,7 +9,7 @@ Device, StrictEnum, SupersetEnum, - soft_signal_rw, + soft_signal_r_and_setter, ) from pydantic import BaseModel, Field, model_validator @@ -227,15 +227,21 @@ def __init__( initial_file: str = "Not set", name: str = "", ): - self.sequence_file = soft_signal_rw(str, initial_value=initial_file) + self.sequence_file, self._sequence_file_setter = soft_signal_r_and_setter( + str, initial_value=initial_file + ) self._sequence_class = sequence_class + + self.sequence: TAbstractBaseSequence | None = None super().__init__(name) @AsyncStatus.wrap async def set(self, filename: str) -> None: - await self.sequence_file.set(filename) + """Coordinate setting the sequence_file signal and also loading the data into sequence.""" + self._sequence_file_setter(filename) + self.sequence = self.load(filename) - async def load(self) -> TAbstractBaseSequence: + def load(self, filename: str) -> TAbstractBaseSequence: """ Load the sequence data from a provided json file into a sequence class. @@ -245,6 +251,4 @@ async def load(self) -> TAbstractBaseSequence: Returns: Pydantic model representing the sequence file. """ - return load_json_file_to_class( - self._sequence_class, await self.sequence_file.get_value() - ) + return load_json_file_to_class(self._sequence_class, filename) From d8b94e6548876f06b8be41ab6dae6db43ef0ab1a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 6 Jan 2026 10:55:29 +0000 Subject: [PATCH 06/11] Changed ordering --- src/dodal/devices/electron_analyser/base/base_region.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 108f0868b0..97ea7c162a 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -238,8 +238,9 @@ def __init__( @AsyncStatus.wrap async def set(self, filename: str) -> None: """Coordinate setting the sequence_file signal and also loading the data into sequence.""" - self._sequence_file_setter(filename) + # Try loading the sequence first to check it is a valid file before setting signal. self.sequence = self.load(filename) + self._sequence_file_setter(filename) def load(self, filename: str) -> TAbstractBaseSequence: """ From c4a42bed6772cc258475d5c93567a8f9587cb558 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 6 Jan 2026 11:55:24 +0000 Subject: [PATCH 07/11] Add sequence loader to analyser --- src/dodal/beamlines/i09.py | 13 ++++++++++++- .../devices/electron_analyser/base/base_detector.py | 3 +++ .../vgscienta/vgscienta_detector.py | 9 +++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/dodal/beamlines/i09.py b/src/dodal/beamlines/i09.py index f9006e6fa8..6dd54603ca 100644 --- a/src/dodal/beamlines/i09.py +++ b/src/dodal/beamlines/i09.py @@ -8,7 +8,11 @@ StationaryCrystal, ) from dodal.devices.electron_analyser.base import DualEnergySource -from dodal.devices.electron_analyser.vgscienta import VGScientaDetector +from dodal.devices.electron_analyser.base.base_region import SequenceLoader +from dodal.devices.electron_analyser.vgscienta import ( + VGScientaDetector, + VGScientaSequence, +) from dodal.devices.i09 import Grating, LensMode, PassEnergy, PsuMode from dodal.devices.pgm import PlaneGratingMonochromator from dodal.devices.synchrotron import Synchrotron @@ -46,6 +50,12 @@ def energy_source() -> DualEnergySource: return DualEnergySource(dcm().energy_in_eV, pgm().energy.user_readback) +def sequence_loader() -> SequenceLoader[ + VGScientaSequence[LensMode, PsuMode, PassEnergy] +]: + return SequenceLoader(VGScientaSequence[LensMode, PsuMode, PassEnergy]) + + # Connect will work again after this work completed # https://jira.diamond.ac.uk/browse/I09-651 @device_factory() @@ -56,4 +66,5 @@ def ew4000() -> VGScientaDetector[LensMode, PsuMode, PassEnergy]: psu_mode_type=PsuMode, pass_energy_type=PassEnergy, energy_source=energy_source(), + sequence_loader=sequence_loader(), ) diff --git a/src/dodal/devices/electron_analyser/base/base_detector.py b/src/dodal/devices/electron_analyser/base/base_detector.py index 5946c9aa3f..995502bb17 100644 --- a/src/dodal/devices/electron_analyser/base/base_detector.py +++ b/src/dodal/devices/electron_analyser/base/base_detector.py @@ -19,6 +19,7 @@ ) from dodal.devices.electron_analyser.base.base_region import ( GenericRegion, + SequenceLoader, TAbstractBaseRegion, ) @@ -45,9 +46,11 @@ def __init__( controller: ElectronAnalyserController[ TAbstractAnalyserDriverIO, TAbstractBaseRegion ], + sequence_loader: SequenceLoader, name: str = "", ): self._controller = controller + self.sequence_loader = sequence_loader super().__init__(name) diff --git a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py index 6c77e47fee..0d07d7e5be 100644 --- a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py +++ b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py @@ -4,7 +4,11 @@ ElectronAnalyserController, ) from dodal.devices.electron_analyser.base.base_detector import ElectronAnalyserDetector -from dodal.devices.electron_analyser.base.base_region import TLensMode, TPsuMode +from dodal.devices.electron_analyser.base.base_region import ( + SequenceLoader, + TLensMode, + TPsuMode, +) from dodal.devices.electron_analyser.base.energy_sources import ( DualEnergySource, EnergySource, @@ -32,6 +36,7 @@ def __init__( psu_mode_type: type[TPsuMode], pass_energy_type: type[TPassEnergyEnum], energy_source: DualEnergySource | EnergySource, + sequence_loader: SequenceLoader, name: str = "", ): # Save to class so takes part with connect() @@ -44,4 +49,4 @@ def __init__( VGScientaRegion[TLensMode, TPassEnergyEnum], ](self.driver, energy_source, 0) - super().__init__(controller, name) + super().__init__(controller, sequence_loader, name) From 12d91b82fcfa2d732a9412d6938b2554f94593ea Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 8 Jan 2026 17:12:10 +0000 Subject: [PATCH 08/11] Reworked testd --- src/dodal/beamlines/i09.py | 9 -- src/dodal/beamlines/p60.py | 4 +- .../electron_analyser/base/__init__.py | 4 + .../electron_analyser/base/base_detector.py | 3 +- .../electron_analyser/base/base_region.py | 33 ++++--- .../electron_analyser/specs/specs_detector.py | 18 +++- .../vgscienta/vgscienta_detector.py | 9 +- .../base/test_base_controller.py | 32 ++++++- .../base/test_base_detector.py | 88 ++----------------- .../base/test_base_region.py | 17 +++- tests/devices/electron_analyser/conftest.py | 77 ++++++++-------- .../specs/test_specs_driver_io.py | 12 ++- .../vgscienta/test_vgscienta_detector.py | 4 +- .../vgscienta/test_vgscienta_driver_io.py | 14 ++- 14 files changed, 162 insertions(+), 162 deletions(-) diff --git a/src/dodal/beamlines/i09.py b/src/dodal/beamlines/i09.py index 6dd54603ca..09e37a675a 100644 --- a/src/dodal/beamlines/i09.py +++ b/src/dodal/beamlines/i09.py @@ -8,10 +8,8 @@ StationaryCrystal, ) from dodal.devices.electron_analyser.base import DualEnergySource -from dodal.devices.electron_analyser.base.base_region import SequenceLoader from dodal.devices.electron_analyser.vgscienta import ( VGScientaDetector, - VGScientaSequence, ) from dodal.devices.i09 import Grating, LensMode, PassEnergy, PsuMode from dodal.devices.pgm import PlaneGratingMonochromator @@ -50,12 +48,6 @@ def energy_source() -> DualEnergySource: return DualEnergySource(dcm().energy_in_eV, pgm().energy.user_readback) -def sequence_loader() -> SequenceLoader[ - VGScientaSequence[LensMode, PsuMode, PassEnergy] -]: - return SequenceLoader(VGScientaSequence[LensMode, PsuMode, PassEnergy]) - - # Connect will work again after this work completed # https://jira.diamond.ac.uk/browse/I09-651 @device_factory() @@ -66,5 +58,4 @@ def ew4000() -> VGScientaDetector[LensMode, PsuMode, PassEnergy]: psu_mode_type=PsuMode, pass_energy_type=PassEnergy, energy_source=energy_source(), - sequence_loader=sequence_loader(), ) diff --git a/src/dodal/beamlines/p60.py b/src/dodal/beamlines/p60.py index c4a7cf4db6..7952c22a76 100644 --- a/src/dodal/beamlines/p60.py +++ b/src/dodal/beamlines/p60.py @@ -3,7 +3,9 @@ ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.electron_analyser.base import DualEnergySource -from dodal.devices.electron_analyser.vgscienta import VGScientaDetector +from dodal.devices.electron_analyser.vgscienta import ( + VGScientaDetector, +) from dodal.devices.p60 import ( LabXraySource, LabXraySourceReadable, diff --git a/src/dodal/devices/electron_analyser/base/__init__.py b/src/dodal/devices/electron_analyser/base/__init__.py index 8eb660cb33..18e21e28b0 100644 --- a/src/dodal/devices/electron_analyser/base/__init__.py +++ b/src/dodal/devices/electron_analyser/base/__init__.py @@ -17,6 +17,8 @@ AbstractBaseSequence, GenericRegion, GenericSequence, + JsonSequenceLoader, + SequenceLoader, TAbstractBaseRegion, TAbstractBaseSequence, TAcquisitionMode, @@ -39,6 +41,8 @@ "AbstractBaseSequence", "GenericRegion", "GenericSequence", + "JsonSequenceLoader", + "SequenceLoader", "TAbstractBaseRegion", "TAbstractBaseSequence", "TAcquisitionMode", diff --git a/src/dodal/devices/electron_analyser/base/base_detector.py b/src/dodal/devices/electron_analyser/base/base_detector.py index 995502bb17..8d9326af4d 100644 --- a/src/dodal/devices/electron_analyser/base/base_detector.py +++ b/src/dodal/devices/electron_analyser/base/base_detector.py @@ -18,6 +18,7 @@ TAbstractAnalyserDriverIO, ) from dodal.devices.electron_analyser.base.base_region import ( + AbstractBaseSequence, GenericRegion, SequenceLoader, TAbstractBaseRegion, @@ -46,7 +47,7 @@ def __init__( controller: ElectronAnalyserController[ TAbstractAnalyserDriverIO, TAbstractBaseRegion ], - sequence_loader: SequenceLoader, + sequence_loader: SequenceLoader[AbstractBaseSequence[TAbstractBaseRegion]], name: str = "", ): self._controller = controller diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 97ea7c162a..e1eb6dd6c5 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -1,12 +1,13 @@ import re -from abc import ABC +from abc import ABC, abstractmethod from collections.abc import Callable from typing import Generic, Self, TypeAlias, TypeVar from bluesky.protocols import Movable from ophyd_async.core import ( AsyncStatus, - Device, + StandardReadable, + StandardReadableFormat, StrictEnum, SupersetEnum, soft_signal_r_and_setter, @@ -210,15 +211,10 @@ def get_region_by_name(self, name: str) -> TAbstractBaseRegion | None: TAbstractBaseSequence = TypeVar("TAbstractBaseSequence", bound=AbstractBaseSequence) -class SequenceLoader(Device, Movable[str], Generic[TAbstractBaseSequence]): +class SequenceLoader(StandardReadable, Movable[str], Generic[TAbstractBaseSequence]): """ - Docstring for SequenceLoader - - :var Args: Description - :var filename: Description - :vartype filename: Path - :var Returns: Description - :var https: Description + Device that controls the sequence file selected that configures the electron + analyser inside plans. """ def __init__( @@ -227,9 +223,10 @@ def __init__( initial_file: str = "Not set", name: str = "", ): - self.sequence_file, self._sequence_file_setter = soft_signal_r_and_setter( - str, initial_value=initial_file - ) + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.sequence_file, self._sequence_file_setter = soft_signal_r_and_setter( + str, initial_value=initial_file + ) self._sequence_class = sequence_class self.sequence: TAbstractBaseSequence | None = None @@ -242,9 +239,10 @@ async def set(self, filename: str) -> None: self.sequence = self.load(filename) self._sequence_file_setter(filename) + @abstractmethod def load(self, filename: str) -> TAbstractBaseSequence: """ - Load the sequence data from a provided json file into a sequence class. + Load the sequence data from a provided file into a sequence class. Args: filename: Path to the sequence file containing the region data. @@ -252,4 +250,11 @@ def load(self, filename: str) -> TAbstractBaseSequence: Returns: Pydantic model representing the sequence file. """ + # return load_json_file_to_class(self._sequence_class, filename) + + +class JsonSequenceLoader(SequenceLoader[TAbstractBaseSequence]): + """Json specifc sequence loader for electron analysers""" + + def load(self, filename: str) -> TAbstractBaseSequence: return load_json_file_to_class(self._sequence_class, filename) diff --git a/src/dodal/devices/electron_analyser/specs/specs_detector.py b/src/dodal/devices/electron_analyser/specs/specs_detector.py index d8e66ee50b..ec57a2ba27 100644 --- a/src/dodal/devices/electron_analyser/specs/specs_detector.py +++ b/src/dodal/devices/electron_analyser/specs/specs_detector.py @@ -4,13 +4,21 @@ ElectronAnalyserController, ) from dodal.devices.electron_analyser.base.base_detector import ElectronAnalyserDetector -from dodal.devices.electron_analyser.base.base_region import TLensMode, TPsuMode +from dodal.devices.electron_analyser.base.base_region import ( + AbstractBaseSequence, + JsonSequenceLoader, + TLensMode, + TPsuMode, +) from dodal.devices.electron_analyser.base.energy_sources import ( DualEnergySource, EnergySource, ) from dodal.devices.electron_analyser.specs.specs_driver_io import SpecsAnalyserDriverIO -from dodal.devices.electron_analyser.specs.specs_region import SpecsRegion +from dodal.devices.electron_analyser.specs.specs_region import ( + SpecsRegion, + SpecsSequence, +) class SpecsDetector( @@ -37,4 +45,8 @@ def __init__( SpecsAnalyserDriverIO[TLensMode, TPsuMode], SpecsRegion[TLensMode, TPsuMode] ](self.driver, energy_source, 0) - super().__init__(controller, name) + sequence_loader = JsonSequenceLoader[ + AbstractBaseSequence[SpecsRegion[TLensMode, TPsuMode]] + ](SpecsSequence[lens_mode_type, psu_mode_type]) + + super().__init__(controller, sequence_loader, name) diff --git a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py index 0d07d7e5be..9d2a6ca122 100644 --- a/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py +++ b/src/dodal/devices/electron_analyser/vgscienta/vgscienta_detector.py @@ -5,7 +5,8 @@ ) from dodal.devices.electron_analyser.base.base_detector import ElectronAnalyserDetector from dodal.devices.electron_analyser.base.base_region import ( - SequenceLoader, + AbstractBaseSequence, + JsonSequenceLoader, TLensMode, TPsuMode, ) @@ -19,6 +20,7 @@ from dodal.devices.electron_analyser.vgscienta.vgscienta_region import ( TPassEnergyEnum, VGScientaRegion, + VGScientaSequence, ) @@ -36,7 +38,6 @@ def __init__( psu_mode_type: type[TPsuMode], pass_energy_type: type[TPassEnergyEnum], energy_source: DualEnergySource | EnergySource, - sequence_loader: SequenceLoader, name: str = "", ): # Save to class so takes part with connect() @@ -49,4 +50,8 @@ def __init__( VGScientaRegion[TLensMode, TPassEnergyEnum], ](self.driver, energy_source, 0) + sequence_loader = JsonSequenceLoader[ + AbstractBaseSequence[VGScientaRegion[TLensMode, TPassEnergyEnum]] + ](VGScientaSequence[lens_mode_type, psu_mode_type, pass_energy_type]) + super().__init__(controller, sequence_loader, name) diff --git a/tests/devices/electron_analyser/base/test_base_controller.py b/tests/devices/electron_analyser/base/test_base_controller.py index c2826fedc4..5b5f676d78 100644 --- a/tests/devices/electron_analyser/base/test_base_controller.py +++ b/tests/devices/electron_analyser/base/test_base_controller.py @@ -7,15 +7,19 @@ from dodal.devices.electron_analyser.base import ( AbstractAnalyserDriverIO, AbstractBaseRegion, + AbstractBaseSequence, DualEnergySource, EnergySource, + GenericSequence, + JsonSequenceLoader, ) from dodal.devices.electron_analyser.base.base_controller import ( ElectronAnalyserController, ) -from dodal.devices.electron_analyser.specs import SpecsAnalyserDriverIO +from dodal.devices.electron_analyser.specs import SpecsAnalyserDriverIO, SpecsSequence from dodal.devices.electron_analyser.vgscienta import ( VGScientaAnalyserDriverIO, + VGScientaSequence, ) from dodal.testing.electron_analyser import create_driver from tests.devices.electron_analyser.helper_util import ( @@ -41,6 +45,32 @@ async def sim_driver( return sim_detector +@pytest.fixture +def sequence_class( + sim_driver: AbstractAnalyserDriverIO, +) -> type[AbstractBaseSequence]: + # We must include the pass energy, lens and psu mode types here, otherwise the + # sequence file can't be loaded as pydantic won't be able to resolve the enums. + if isinstance(sim_driver, VGScientaAnalyserDriverIO): + return VGScientaSequence[ + sim_driver.lens_mode_type, + sim_driver.psu_mode_type, + sim_driver.pass_energy_type, + ] + elif isinstance(sim_driver, SpecsAnalyserDriverIO): + return SpecsSequence[sim_driver.lens_mode_type, sim_driver.psu_mode_type] + raise ValueError("class " + str(sim_driver) + " not recognised") + + +@pytest.fixture +def sequence_loader( + sequence_class: type[GenericSequence], +) -> JsonSequenceLoader[GenericSequence]: + with init_devices(mock=True): + sequence_loader = JsonSequenceLoader[GenericSequence](sequence_class) + return sequence_loader + + @pytest.fixture def sequence_file_path( sim_driver: AbstractAnalyserDriverIO, diff --git a/tests/devices/electron_analyser/base/test_base_detector.py b/tests/devices/electron_analyser/base/test_base_detector.py index 68191af0c7..dd11beeb47 100644 --- a/tests/devices/electron_analyser/base/test_base_detector.py +++ b/tests/devices/electron_analyser/base/test_base_detector.py @@ -3,7 +3,7 @@ import pytest from bluesky import plan_stubs as bps from bluesky.run_engine import RunEngine -from ophyd_async.core import TriggerInfo, init_devices, set_mock_value +from ophyd_async.core import init_devices, set_mock_value from ophyd_async.testing import ( assert_configuration, assert_reading, @@ -13,7 +13,6 @@ import dodal.devices.i09 as i09 from dodal.devices.electron_analyser.base import ( EnergySource, - GenericBaseElectronAnalyserDetector, GenericElectronAnalyserDetector, ) from dodal.devices.electron_analyser.base.energy_sources import EnergySource @@ -45,7 +44,7 @@ async def sim_detector( def test_base_analyser_detector_trigger( - sim_detector: GenericBaseElectronAnalyserDetector, + sim_detector: GenericElectronAnalyserDetector, run_engine: RunEngine, ) -> None: sim_detector._controller.arm = AsyncMock() @@ -58,14 +57,14 @@ def test_base_analyser_detector_trigger( async def test_base_analyser_detector_read( - sim_detector: GenericBaseElectronAnalyserDetector, + sim_detector: GenericElectronAnalyserDetector, ) -> None: driver_read = await sim_detector._controller.driver.read() await assert_reading(sim_detector, driver_read) async def test_base_analyser_describe( - sim_detector: GenericBaseElectronAnalyserDetector, + sim_detector: GenericElectronAnalyserDetector, ) -> None: energy_array = await sim_detector._controller.driver.energy_axis.get_value() angle_array = await sim_detector._controller.driver.angle_axis.get_value() @@ -77,14 +76,14 @@ async def test_base_analyser_describe( async def test_base_analyser_detector_configuration( - sim_detector: GenericBaseElectronAnalyserDetector, + sim_detector: GenericElectronAnalyserDetector, ) -> None: driver_config = await sim_detector._controller.driver.read_configuration() await assert_configuration(sim_detector, driver_config) async def test_base_analyser_detector_describe_configuration( - sim_detector: GenericBaseElectronAnalyserDetector, + sim_detector: GenericElectronAnalyserDetector, ) -> None: driver_describe_config = ( await sim_detector._controller.driver.describe_configuration() @@ -93,21 +92,6 @@ async def test_base_analyser_detector_describe_configuration( assert await sim_detector.describe_configuration() == driver_describe_config -@pytest.fixture -def sequence_file_path( - sim_detector: GenericElectronAnalyserDetector, -) -> str: - return get_test_sequence(type(sim_detector)) - - -def test_analyser_detector_loads_sequence_correctly( - sim_detector: GenericElectronAnalyserDetector, - sequence_file_path: str, -) -> None: - seq = sim_detector.load_sequence(sequence_file_path) - assert seq is not None - - async def test_analyser_detector_stage( sim_detector: GenericElectronAnalyserDetector, ) -> None: @@ -128,39 +112,6 @@ async def test_analyser_detector_unstage( sim_detector._controller.disarm.assert_awaited_once() -def test_analyser_detector_creates_region_detectors( - sim_detector: GenericElectronAnalyserDetector, - sequence_file_path: str, -) -> None: - seq = sim_detector.load_sequence(sequence_file_path) - region_detectors = sim_detector.create_region_detector_list(sequence_file_path) - - assert len(region_detectors) == len(seq.get_enabled_regions()) - for det in region_detectors: - assert det.region.enabled is True - assert det.name == sim_detector.name + "_" + det.region.name - - -def test_analyser_detector_has_driver_as_child_and_region_detector_does_not( - sim_detector: GenericElectronAnalyserDetector, - sequence_file_path: str, -) -> None: - # Remove parent name from driver name so it can be checked it exists in - # _child_devices dict - driver_name = sim_detector._controller.driver.name.replace( - sim_detector.name + "-", "" - ) - - assert sim_detector._controller.driver.parent == sim_detector - assert sim_detector._child_devices.get(driver_name) is not None - - region_detectors = sim_detector.create_region_detector_list(sequence_file_path) - - for det in region_detectors: - assert det._child_devices.get(driver_name) is None - assert det._controller.driver.parent == sim_detector - - def test_analyser_detector_trigger_called_controller_prepare( sim_detector: GenericElectronAnalyserDetector, run_engine: RunEngine, @@ -178,35 +129,10 @@ def test_analyser_detector_trigger_called_controller_prepare( def test_analyser_detector_set_called_controller_setup_with_region( sim_detector: GenericElectronAnalyserDetector, - sequence_file_path: str, run_engine: RunEngine, ) -> None: - seq = sim_detector.load_sequence(sequence_file_path) + seq = sim_detector.sequence_loader.load(get_test_sequence(type(sim_detector))) region = seq.get_enabled_regions()[0] sim_detector._controller.setup_with_region = AsyncMock() run_engine(bps.mv(sim_detector, region), wait=True) sim_detector._controller.setup_with_region.assert_awaited_once_with(region) - - -async def test_analyser_region_detector_trigger_sets_driver_with_region( - sim_detector: GenericElectronAnalyserDetector, - sequence_file_path: str, - run_engine: RunEngine, -) -> None: - region_detectors = sim_detector.create_region_detector_list( - sequence_file_path, enabled_only=False - ) - trigger_info = TriggerInfo() - - for reg_det in region_detectors: - reg_det.set = AsyncMock() - reg_det._controller.prepare = AsyncMock() - reg_det._controller.arm = AsyncMock() - reg_det._controller.wait_for_idle = AsyncMock() - - run_engine(bps.trigger(reg_det, wait=True), wait=True) - - reg_det.set.assert_awaited_once_with(reg_det.region) - reg_det._controller.prepare.assert_awaited_once_with(trigger_info) - reg_det._controller.arm.assert_awaited_once() - reg_det._controller.wait_for_idle.assert_awaited_once() diff --git a/tests/devices/electron_analyser/base/test_base_region.py b/tests/devices/electron_analyser/base/test_base_region.py index faea3fdbcf..0d9c74b175 100644 --- a/tests/devices/electron_analyser/base/test_base_region.py +++ b/tests/devices/electron_analyser/base/test_base_region.py @@ -1,14 +1,15 @@ from unittest.mock import patch import pytest +from ophyd_async.core import init_devices -from dodal.common.data_util import load_json_file_to_class from dodal.devices import b07, i09 from dodal.devices.electron_analyser.base import ( AbstractBaseRegion, EnergyMode, GenericRegion, GenericSequence, + JsonSequenceLoader, TAbstractBaseRegion, to_binding_energy, to_kinetic_energy, @@ -20,7 +21,6 @@ from dodal.devices.electron_analyser.vgscienta import VGScientaRegion, VGScientaSequence from tests.devices.electron_analyser.helper_util import ( TEST_SEQUENCE_REGION_NAMES, - get_test_sequence, ) @@ -30,8 +30,17 @@ VGScientaSequence[i09.LensMode, i09.PsuMode, i09.PassEnergy], ] ) -def sequence(request: pytest.FixtureRequest) -> GenericSequence: - return load_json_file_to_class(request.param, get_test_sequence(request.param)) +def sequence_class(request: pytest.FixtureRequest) -> GenericSequence: + return request.param + + +@pytest.fixture +def sequence_loader( + sequence_class: type[GenericSequence], +) -> JsonSequenceLoader[GenericSequence]: + with init_devices(mock=True): + sequence_loader = JsonSequenceLoader[GenericSequence](sequence_class) + return sequence_loader @pytest.fixture diff --git a/tests/devices/electron_analyser/conftest.py b/tests/devices/electron_analyser/conftest.py index 2abfd637a0..df6bff3f94 100644 --- a/tests/devices/electron_analyser/conftest.py +++ b/tests/devices/electron_analyser/conftest.py @@ -9,22 +9,11 @@ StationaryCrystal, ) from dodal.devices.electron_analyser.base import ( - AbstractAnalyserDriverIO, AbstractBaseRegion, - AbstractBaseSequence, DualEnergySource, - ElectronAnalyserController, - ElectronAnalyserDetector, EnergySource, - TAbstractBaseSequence, -) -from dodal.devices.electron_analyser.specs import ( - SpecsAnalyserDriverIO, - SpecsSequence, -) -from dodal.devices.electron_analyser.vgscienta import ( - VGScientaAnalyserDriverIO, - VGScientaSequence, + GenericSequence, + SequenceLoader, ) from dodal.devices.i09 import Grating from dodal.devices.pgm import PlaneGratingMonochromator @@ -60,41 +49,47 @@ async def dual_energy_source() -> DualEnergySource: return dual_energy_source -@pytest.fixture -def sequence_class( - sim_driver: AbstractAnalyserDriverIO, -) -> type[AbstractBaseSequence]: - # We must include the pass energy, lens and psu mode types here, otherwise the - # sequence file can't be loaded as pydantic won't be able to resolve the enums. - if isinstance(sim_driver, VGScientaAnalyserDriverIO): - return VGScientaSequence[ - sim_driver.lens_mode_type, - sim_driver.psu_mode_type, - sim_driver.pass_energy_type, - ] - elif isinstance(sim_driver, SpecsAnalyserDriverIO): - return SpecsSequence[sim_driver.lens_mode_type, sim_driver.psu_mode_type] - raise ValueError("class " + str(sim_driver) + " not recognised") +# @pytest.fixture +# def sequence_class( +# sim_driver: AbstractAnalyserDriverIO, +# ) -> type[AbstractBaseSequence]: +# # We must include the pass energy, lens and psu mode types here, otherwise the +# # sequence file can't be loaded as pydantic won't be able to resolve the enums. +# if isinstance(sim_driver, VGScientaAnalyserDriverIO): +# return VGScientaSequence[ +# sim_driver.lens_mode_type, +# sim_driver.psu_mode_type, +# sim_driver.pass_energy_type, +# ] +# elif isinstance(sim_driver, SpecsAnalyserDriverIO): +# return SpecsSequence[sim_driver.lens_mode_type, sim_driver.psu_mode_type] +# raise ValueError("class " + str(sim_driver) + " not recognised") + + +# @pytest.fixture +# def sequence_loader( +# sequence_class: type[TAbstractBaseSequence], +# ) -> JsonSequenceLoader[GenericSequence]: +# with init_devices(mock=True): +# sequence_loader = JsonSequenceLoader[GenericSequence](sequence_class) +# return sequence_loader @pytest.fixture -def sequence( - sim_driver: AbstractAnalyserDriverIO, - sequence_class: type[TAbstractBaseSequence], - single_energy_source: EnergySource, -) -> AbstractBaseSequence: - controller = ElectronAnalyserController(sim_driver, single_energy_source, 0) - det = ElectronAnalyserDetector( - sequence_class=sequence_class, - controller=controller, - ) - return det.load_sequence(get_test_sequence(type(sim_driver))) +async def sequence( + sequence_loader: SequenceLoader[GenericSequence], +) -> GenericSequence: + filename = get_test_sequence(sequence_loader._sequence_class) + await sequence_loader.set(filename) + if sequence_loader.sequence is None: + raise ValueError(f"Sequence is None when set to file: {filename}.") + return sequence_loader.sequence @pytest.fixture -def region( +async def region( request: pytest.FixtureRequest, - sequence: AbstractBaseSequence, + sequence: GenericSequence, ) -> AbstractBaseRegion: region = sequence.get_region_by_name(request.param) if region is None: diff --git a/tests/devices/electron_analyser/specs/test_specs_driver_io.py b/tests/devices/electron_analyser/specs/test_specs_driver_io.py index 898132bb6e..40cbbb1f13 100644 --- a/tests/devices/electron_analyser/specs/test_specs_driver_io.py +++ b/tests/devices/electron_analyser/specs/test_specs_driver_io.py @@ -13,12 +13,13 @@ ) from dodal.devices.b07 import LensMode, PsuMode -from dodal.devices.electron_analyser.base import EnergyMode +from dodal.devices.electron_analyser.base import EnergyMode, JsonSequenceLoader from dodal.devices.electron_analyser.base.base_enums import EnergyMode from dodal.devices.electron_analyser.specs import ( AcquisitionMode, SpecsAnalyserDriverIO, SpecsRegion, + SpecsSequence, ) from dodal.testing.electron_analyser import create_driver from tests.devices.electron_analyser.helper_util import ( @@ -36,6 +37,15 @@ async def sim_driver() -> SpecsAnalyserDriverIO[LensMode, PsuMode]: return sim_driver +@pytest.fixture +def sequence_loader() -> JsonSequenceLoader[SpecsSequence[LensMode, PsuMode]]: + with init_devices(mock=True): + sequence_loader = JsonSequenceLoader[SpecsSequence[LensMode, PsuMode]]( + SpecsSequence[LensMode, PsuMode] + ) + return sequence_loader + + @pytest.mark.parametrize("region", TEST_SEQUENCE_REGION_NAMES, indirect=True) async def test_analyser_sets_region_correctly( sim_driver: SpecsAnalyserDriverIO[LensMode, PsuMode], diff --git a/tests/devices/electron_analyser/vgscienta/test_vgscienta_detector.py b/tests/devices/electron_analyser/vgscienta/test_vgscienta_detector.py index b7d771bca5..fdd4027f21 100644 --- a/tests/devices/electron_analyser/vgscienta/test_vgscienta_detector.py +++ b/tests/devices/electron_analyser/vgscienta/test_vgscienta_detector.py @@ -3,9 +3,7 @@ from ophyd_async.core import init_devices, set_mock_value from dodal.devices.electron_analyser.base import DualEnergySource -from dodal.devices.electron_analyser.vgscienta import ( - VGScientaDetector, -) +from dodal.devices.electron_analyser.vgscienta import VGScientaDetector from dodal.devices.i09 import LensMode, PassEnergy, PsuMode from dodal.testing.electron_analyser import create_detector diff --git a/tests/devices/electron_analyser/vgscienta/test_vgscienta_driver_io.py b/tests/devices/electron_analyser/vgscienta/test_vgscienta_driver_io.py index 61f1c381db..2795f71fe4 100644 --- a/tests/devices/electron_analyser/vgscienta/test_vgscienta_driver_io.py +++ b/tests/devices/electron_analyser/vgscienta/test_vgscienta_driver_io.py @@ -13,10 +13,11 @@ partial_reading, ) -from dodal.devices.electron_analyser.base import EnergyMode +from dodal.devices.electron_analyser.base import EnergyMode, JsonSequenceLoader from dodal.devices.electron_analyser.vgscienta import ( VGScientaAnalyserDriverIO, VGScientaRegion, + VGScientaSequence, ) from dodal.devices.i09 import LensMode, PassEnergy, PsuMode from dodal.testing.electron_analyser import create_driver @@ -34,6 +35,17 @@ async def sim_driver() -> VGScientaAnalyserDriverIO[LensMode, PsuMode, PassEnerg return sim_driver +@pytest.fixture +def sequence_loader() -> JsonSequenceLoader[ + VGScientaSequence[LensMode, PsuMode, PassEnergy] +]: + with init_devices(mock=True): + sequence_loader = JsonSequenceLoader[ + VGScientaSequence[LensMode, PsuMode, PassEnergy] + ](VGScientaSequence[LensMode, PsuMode, PassEnergy]) + return sequence_loader + + @pytest.mark.parametrize("region", TEST_SEQUENCE_REGION_NAMES, indirect=True) async def test_analyser_sets_region_correctly( sim_driver: VGScientaAnalyserDriverIO[LensMode, PsuMode, PassEnergy], From 7e4232bec89ac8ad0541301e38e83832e8d6cea1 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 9 Jan 2026 16:20:48 +0000 Subject: [PATCH 09/11] Remove commented out code and added new sequence_loader test --- .../electron_analyser/base/base_region.py | 1 - .../base/test_base_region.py | 17 +++++++++++- tests/devices/electron_analyser/conftest.py | 26 ------------------- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 6234dc792c..4f6febe1c8 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -251,7 +251,6 @@ def load(self, filename: str) -> TAbstractBaseSequence: Returns: Pydantic model representing the sequence file. """ - # return load_json_file_to_class(self._sequence_class, filename) class JsonSequenceLoader(SequenceLoader[TAbstractBaseSequence]): diff --git a/tests/devices/electron_analyser/base/test_base_region.py b/tests/devices/electron_analyser/base/test_base_region.py index 0d9c74b175..ea6c89209f 100644 --- a/tests/devices/electron_analyser/base/test_base_region.py +++ b/tests/devices/electron_analyser/base/test_base_region.py @@ -2,6 +2,7 @@ import pytest from ophyd_async.core import init_devices +from ophyd_async.testing import assert_configuration, partial_reading from dodal.devices import b07, i09 from dodal.devices.electron_analyser.base import ( @@ -10,6 +11,7 @@ GenericRegion, GenericSequence, JsonSequenceLoader, + SequenceLoader, TAbstractBaseRegion, to_binding_energy, to_kinetic_energy, @@ -21,6 +23,7 @@ from dodal.devices.electron_analyser.vgscienta import VGScientaRegion, VGScientaSequence from tests.devices.electron_analyser.helper_util import ( TEST_SEQUENCE_REGION_NAMES, + get_test_sequence, ) @@ -37,12 +40,24 @@ def sequence_class(request: pytest.FixtureRequest) -> GenericSequence: @pytest.fixture def sequence_loader( sequence_class: type[GenericSequence], -) -> JsonSequenceLoader[GenericSequence]: +) -> SequenceLoader[GenericSequence]: with init_devices(mock=True): sequence_loader = JsonSequenceLoader[GenericSequence](sequence_class) return sequence_loader +async def test_sequence_loader_read_configuration( + sequence_loader: SequenceLoader[GenericSequence], +) -> None: + assert sequence_loader.sequence is None + filename = get_test_sequence(sequence_loader._sequence_class) + await sequence_loader.set(filename) + assert sequence_loader.sequence is not None + await assert_configuration( + sequence_loader, {sequence_loader.sequence_file.name: partial_reading(filename)} + ) + + @pytest.fixture def expected_region_class( sequence: GenericSequence, diff --git a/tests/devices/electron_analyser/conftest.py b/tests/devices/electron_analyser/conftest.py index 0a00be7c82..0652287255 100644 --- a/tests/devices/electron_analyser/conftest.py +++ b/tests/devices/electron_analyser/conftest.py @@ -59,32 +59,6 @@ async def dual_energy_source(source_selector: SourceSelector) -> DualEnergySourc return dual_energy_source -# @pytest.fixture -# def sequence_class( -# sim_driver: AbstractAnalyserDriverIO, -# ) -> type[AbstractBaseSequence]: -# # We must include the pass energy, lens and psu mode types here, otherwise the -# # sequence file can't be loaded as pydantic won't be able to resolve the enums. -# if isinstance(sim_driver, VGScientaAnalyserDriverIO): -# return VGScientaSequence[ -# sim_driver.lens_mode_type, -# sim_driver.psu_mode_type, -# sim_driver.pass_energy_type, -# ] -# elif isinstance(sim_driver, SpecsAnalyserDriverIO): -# return SpecsSequence[sim_driver.lens_mode_type, sim_driver.psu_mode_type] -# raise ValueError("class " + str(sim_driver) + " not recognised") - - -# @pytest.fixture -# def sequence_loader( -# sequence_class: type[TAbstractBaseSequence], -# ) -> JsonSequenceLoader[GenericSequence]: -# with init_devices(mock=True): -# sequence_loader = JsonSequenceLoader[GenericSequence](sequence_class) -# return sequence_loader - - @pytest.fixture async def sequence( sequence_loader: SequenceLoader[GenericSequence], From c54d0370e5abc76f17074362177588426e84af77 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 9 Jan 2026 17:00:14 +0000 Subject: [PATCH 10/11] Undid import change --- src/dodal/beamlines/p60.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/dodal/beamlines/p60.py b/src/dodal/beamlines/p60.py index 5fb9fe6720..3cf0dc9304 100644 --- a/src/dodal/beamlines/p60.py +++ b/src/dodal/beamlines/p60.py @@ -3,9 +3,7 @@ ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.electron_analyser.base import DualEnergySource -from dodal.devices.electron_analyser.vgscienta import ( - VGScientaDetector, -) +from dodal.devices.electron_analyser.vgscienta import VGScientaDetector from dodal.devices.p60 import ( LabXraySource, LabXraySourceReadable, From 44cade940879f04154268f256a63b3c9e3f10c84 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 13 Jan 2026 15:09:55 +0000 Subject: [PATCH 11/11] Correct spelling --- src/dodal/devices/electron_analyser/base/base_region.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dodal/devices/electron_analyser/base/base_region.py b/src/dodal/devices/electron_analyser/base/base_region.py index 4f6febe1c8..108ecd466c 100644 --- a/src/dodal/devices/electron_analyser/base/base_region.py +++ b/src/dodal/devices/electron_analyser/base/base_region.py @@ -118,7 +118,7 @@ def switch_energy_mode( """ Get a region with a new energy mode: Kinetic or Binding. It caculates new values for low_energy, centre_energy, high_energy, via the - excitation enerrgy. It doesn't calculate anything if the region is already of + excitation energy. It doesn't calculate anything if the region is already of the same energy mode. Parameters: