From f209651dc6d10334fc00884c0c831078a2b8c0b0 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 30 Jun 2026 19:05:55 +0000 Subject: [PATCH 01/30] Create Superconducting magnet device for i06-1 --- src/dodal/beamlines/i06_1.py | 6 + .../beamlines/i06_1/magnets/__init__.py | 0 .../devices/beamlines/i06_1/magnets/magnet.py | 180 ++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 src/dodal/devices/beamlines/i06_1/magnets/__init__.py create mode 100644 src/dodal/devices/beamlines/i06_1/magnets/magnet.py diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py index b98a1d6a9e9..f634315619b 100644 --- a/src/dodal/beamlines/i06_1.py +++ b/src/dodal/beamlines/i06_1.py @@ -2,6 +2,7 @@ from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.device_manager import DeviceManager from dodal.devices.beamlines.i06_1 import DiffractionDichroism +from dodal.devices.beamlines.i06_1.magnets.magnet import SuperConductingMagnet from dodal.devices.motors import XYThetaStage from dodal.devices.temperture_controller import Lakeshore336 from dodal.log import set_beamline as set_log_beamline @@ -34,3 +35,8 @@ def xabs() -> XYThetaStage: @devices.factory() def dd() -> DiffractionDichroism: return DiffractionDichroism(f"{PREFIX.beamline_prefix}-EA-DDIFF-01:") + + +@devices.factory() +def superconducting_magnet() -> SuperConductingMagnet: + return SuperConductingMagnet(f"{PREFIX.beamline_prefix}-EA-MAG-01:") diff --git a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py new file mode 100644 index 00000000000..14c7217af11 --- /dev/null +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -0,0 +1,180 @@ +import asyncio +import math +from dataclasses import dataclass + +from bluesky.protocols import Movable +from ophyd_async.core import ( + AsyncStatus, + SignalRW, + StandardReadable, + StrictEnum, + derived_signal_r, + soft_signal_rw, +) +from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x + + +class MagnetModes(StrictEnum): + UNIAXIAL_X = "UNIAXIAL_X" + UNIAXIAL_Y = "UNIAXIAL_Y" + UNIAXIAL_Z = "UNIAXIAL_Z" + SPHERICAL = "SPHERICAL" + PLANAR_XZ = "PLANAR_XZ" + QUADRANT_XY = "QUADRANT_XY" + CUBIC = "CUBIC" + UNDEFINED = "UNDEFINED" + + +class MagnetRampStatus(StrictEnum): + RAMPING = "RAMPING" + RAMP_MADE = "RAMP MADE" + + +class MagnetLimitStatus(StrictEnum): + VIOLTATION = "VIOLATION" + OK = "OK" + + +@dataclass +class MagnetPosition: + x: float + y: float + z: float + + +# class SphericalCoorindates(StandardReadable): + +# def __init__(self, x: SignalR[float], y: SignalR[float], z: SignalR[float]): +# self.theta = + + +def read_theta(x: float, z: float) -> float: + theta = math.degrees(math.atan2(-x, z)) + return theta % 360 + + +def read_rho(x: float, y: float, z: float) -> float: + return math.hypot(x, y, z) + + +def read_phi(x: float, y: float, z: float) -> float: + return math.degrees(math.atan2(math.hypot(x, z), y)) + + +def write_x(rho: float, theta: float, phi: float, x: SignalRW[float]) -> float: + theta = math.radians(theta) + phi = math.radians(phi) + x.set(-rho * math.sin(phi) * math.sin(theta)) + + +def write_y(rho: float, theta: float, phi: float) -> float: + phi = math.radians(phi) + return rho * math.cos(phi) + + +def write_z(rho: float, theta: float, phi: float) -> float: + theta = math.radians(theta) + phi = math.radians(phi) + return rho * math.sin(phi) * math.cos(theta) + + +# Need to add spherical and cartesian coordinates +class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): + def __init__(self, prefix: str, name: str = ""): + + # Demand values + self.xi = epics_signal_rw(float, prefix + "X:DMD") + self.yi = epics_signal_rw(float, prefix + "X:DMD") + self.zi = epics_signal_rw(float, prefix + "X:DMD") + + # Readback values + self.xo = epics_signal_r(float, prefix + "X:RBV") + self.yo = epics_signal_r(float, prefix + "X:RBV") + self.zo = epics_signal_r(float, prefix + "X:RBV") + + # Spherical coordinates + self.theta = derived_signal_r(read_theta, x=self.xo, z=self.zo) + self.rho = derived_signal_r(read_rho, x=self.xo, y=self.yo, z=self.zo) + self.phi = derived_signal_r(read_phi, x=self.xo, y=self.yo, z=self.zo) + + self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") + self.ramp_status = epics_signal_rw(MagnetRampStatus, prefix + "RAMPSTATUS") + + self.limit_status = epics_signal_rw(MagnetLimitStatus, prefix + "LIMITSTATUS") + self.start_ramp = epics_signal_x(prefix + "STARTRAMP.PROC") + + self.tolerance = soft_signal_rw(float, initial_value=0) + self.timeout = soft_signal_rw(float, initial_value=300) + self.delay = soft_signal_rw(float, initial_value=5) + + super().__init__(name) + + # Without boundary + @AsyncStatus.wrap + async def set(self, value: MagnetPosition): + await asyncio.gather( + self.xi.set(value.x), + self.yi.set(value.y), + self.zi.set(value.z), + ) + await self._ramp() + + async def _ramp(self): + await self.start_ramp.trigger(timeout=1) + # Due to EPICS flaw, after the callback, still need to wait for a few second for the magnet settle down + # Is this still needed? + await asyncio.sleep(await self.delay.get_value()) + + ramp_status, limit_status = await asyncio.gather( + self.ramp_status.get_value(), + self.limit_status.get_value(), + ) + + if ( + ramp_status == MagnetRampStatus.RAMP_MADE + and limit_status == MagnetLimitStatus.OK + ): + return + + if ramp_status == MagnetRampStatus.RAMPING: + raise RuntimeError("Magnet still ramping") + + if limit_status == MagnetLimitStatus.VIOLTATION: + raise RuntimeError("Magnet limit violation") + + async def set_within_boundary(self, value: MagnetPosition): + """Move magnet while ensuring field magnitude never increases before decreases + have completed. + """ + # For keeping the magnitude constrained to avoid quench, always do the decreasing before increasing motors + x0, y0, z0 = await asyncio.gather( + self.xo.get_value(), + self.yo.get_value(), + self.zo.get_value(), + ) + x1, y1, z1 = value.x, value.y, value.z + + dx = abs(x1) - abs(x0) + dy = abs(y1) - abs(y0) + dz = abs(z1) - abs(z0) + + # Decrease Z first + if dz < 0: + await self.zi.set(z1) + await self._ramp() + # Then decrease X + if dx < 0: + await self.xi.set(x1) + await self._ramp() + # Then decrease Y + if dy < 0: + await self.yi.set(y1) + await self._ramp() + # Finally perform all increases together + if dx > 0 or dy > 0 or dz > 0: + await asyncio.gather( + self.xi.set(x1), + self.yi.set(y1), + self.zi.set(z1), + ) + await self._ramp() From b3f90373a716201aa64995c7c5ff950c7fe1bb03 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 30 Jun 2026 19:14:38 +0000 Subject: [PATCH 02/30] correct pvs --- src/dodal/devices/beamlines/i06_1/magnets/magnet.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py index 14c7217af11..45666803e87 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -84,13 +84,13 @@ def __init__(self, prefix: str, name: str = ""): # Demand values self.xi = epics_signal_rw(float, prefix + "X:DMD") - self.yi = epics_signal_rw(float, prefix + "X:DMD") - self.zi = epics_signal_rw(float, prefix + "X:DMD") + self.yi = epics_signal_rw(float, prefix + "Y:DMD") + self.zi = epics_signal_rw(float, prefix + "Z:DMD") # Readback values self.xo = epics_signal_r(float, prefix + "X:RBV") - self.yo = epics_signal_r(float, prefix + "X:RBV") - self.zo = epics_signal_r(float, prefix + "X:RBV") + self.yo = epics_signal_r(float, prefix + "Y:RBV") + self.zo = epics_signal_r(float, prefix + "Z:RBV") # Spherical coordinates self.theta = derived_signal_r(read_theta, x=self.xo, z=self.zo) From e004d753e201551f71da4943fdbb1766d6ee89e3 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 30 Jun 2026 19:23:08 +0000 Subject: [PATCH 03/30] Update to user own coordinate device --- .../devices/beamlines/i06_1/magnets/magnet.py | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py index 45666803e87..e988d044930 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -5,6 +5,7 @@ from bluesky.protocols import Movable from ophyd_async.core import ( AsyncStatus, + SignalR, SignalRW, StandardReadable, StrictEnum, @@ -42,12 +43,6 @@ class MagnetPosition: z: float -# class SphericalCoorindates(StandardReadable): - -# def __init__(self, x: SignalR[float], y: SignalR[float], z: SignalR[float]): -# self.theta = - - def read_theta(x: float, z: float) -> float: theta = math.degrees(math.atan2(-x, z)) return theta % 360 @@ -78,10 +73,8 @@ def write_z(rho: float, theta: float, phi: float) -> float: return rho * math.sin(phi) * math.cos(theta) -# Need to add spherical and cartesian coordinates -class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): +class CartesianCoordinates(StandardReadable): def __init__(self, prefix: str, name: str = ""): - # Demand values self.xi = epics_signal_rw(float, prefix + "X:DMD") self.yi = epics_signal_rw(float, prefix + "Y:DMD") @@ -91,11 +84,27 @@ def __init__(self, prefix: str, name: str = ""): self.xo = epics_signal_r(float, prefix + "X:RBV") self.yo = epics_signal_r(float, prefix + "Y:RBV") self.zo = epics_signal_r(float, prefix + "Z:RBV") + super().__init__(name) + - # Spherical coordinates - self.theta = derived_signal_r(read_theta, x=self.xo, z=self.zo) - self.rho = derived_signal_r(read_rho, x=self.xo, y=self.yo, z=self.zo) - self.phi = derived_signal_r(read_phi, x=self.xo, y=self.yo, z=self.zo) +class SphericalCoorindates(StandardReadable): + def __init__( + self, xo: SignalR[float], yo: SignalR[float], zo: SignalR[float], name: str = "" + ): + self.theta = derived_signal_r(read_theta, x=xo, z=zo) + self.rho = derived_signal_r(read_rho, x=xo, y=yo, z=zo) + self.phi = derived_signal_r(read_phi, x=xo, y=yo, z=zo) + super().__init__(name) + + +# Need to add spherical and cartesian coordinates +class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): + def __init__(self, prefix: str, name: str = ""): + + self.cartesian = CartesianCoordinates(prefix) + self.spherical = SphericalCoorindates( + self.cartesian.xo, self.cartesian.yo, self.cartesian.zo + ) self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") self.ramp_status = epics_signal_rw(MagnetRampStatus, prefix + "RAMPSTATUS") @@ -113,9 +122,9 @@ def __init__(self, prefix: str, name: str = ""): @AsyncStatus.wrap async def set(self, value: MagnetPosition): await asyncio.gather( - self.xi.set(value.x), - self.yi.set(value.y), - self.zi.set(value.z), + self.cartesian.xi.set(value.x), + self.cartesian.yi.set(value.y), + self.cartesian.zi.set(value.z), ) await self._ramp() From a4812df9f85704845ba522ae35e4d0d2700827f3 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Jul 2026 11:43:35 +0000 Subject: [PATCH 04/30] Complete implementation --- .../devices/beamlines/i06_1/magnets/magnet.py | 182 ++++++++++-------- 1 file changed, 97 insertions(+), 85 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py index e988d044930..8191062b777 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -1,16 +1,17 @@ import asyncio import math +from collections.abc import Awaitable, Callable from dataclasses import dataclass from bluesky.protocols import Movable from ophyd_async.core import ( AsyncStatus, - SignalR, - SignalRW, StandardReadable, + StandardReadableFormat, StrictEnum, derived_signal_r, soft_signal_rw, + wait_for_value, ) from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x @@ -36,13 +37,6 @@ class MagnetLimitStatus(StrictEnum): OK = "OK" -@dataclass -class MagnetPosition: - x: float - y: float - z: float - - def read_theta(x: float, z: float) -> float: theta = math.degrees(math.atan2(-x, z)) return theta % 360 @@ -56,77 +50,98 @@ def read_phi(x: float, y: float, z: float) -> float: return math.degrees(math.atan2(math.hypot(x, z), y)) -def write_x(rho: float, theta: float, phi: float, x: SignalRW[float]) -> float: - theta = math.radians(theta) - phi = math.radians(phi) - x.set(-rho * math.sin(phi) * math.sin(theta)) - +@dataclass +class MagnetPosition: + x: float + y: float + z: float -def write_y(rho: float, theta: float, phi: float) -> float: - phi = math.radians(phi) - return rho * math.cos(phi) + def to_spherical(self) -> "SphericalMagnetPosition": + return SphericalMagnetPosition( + rho=read_rho(self.x, self.y, self.z), + theta=read_theta(self.x, self.z), + phi=read_phi(self.x, self.y, self.z), + ) -def write_z(rho: float, theta: float, phi: float) -> float: - theta = math.radians(theta) - phi = math.radians(phi) - return rho * math.sin(phi) * math.cos(theta) +@dataclass(frozen=True) +class SphericalMagnetPosition: + rho: float + theta: float # degrees + phi: float # degrees + def to_cartesian(self) -> MagnetPosition: + theta = math.radians(self.theta) + phi = math.radians(self.phi) -class CartesianCoordinates(StandardReadable): - def __init__(self, prefix: str, name: str = ""): - # Demand values - self.xi = epics_signal_rw(float, prefix + "X:DMD") - self.yi = epics_signal_rw(float, prefix + "Y:DMD") - self.zi = epics_signal_rw(float, prefix + "Z:DMD") - - # Readback values - self.xo = epics_signal_r(float, prefix + "X:RBV") - self.yo = epics_signal_r(float, prefix + "Y:RBV") - self.zo = epics_signal_r(float, prefix + "Z:RBV") - super().__init__(name) + return MagnetPosition( + x=-self.rho * math.sin(phi) * math.sin(theta), + y=self.rho * math.cos(phi), + z=self.rho * math.sin(phi) * math.cos(theta), + ) -class SphericalCoorindates(StandardReadable): +class MagnetAxis(StandardReadable, Movable[float]): def __init__( - self, xo: SignalR[float], yo: SignalR[float], zo: SignalR[float], name: str = "" + self, + prefix: str, + axis: str, + magnet_set_within_boundary: Callable[[], Awaitable[None]], + name: str = "", ): - self.theta = derived_signal_r(read_theta, x=xo, z=zo) - self.rho = derived_signal_r(read_rho, x=xo, y=yo, z=zo) - self.phi = derived_signal_r(read_phi, x=xo, y=yo, z=zo) + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): + self.readback = epics_signal_r(float, prefix + "RBV") + + with self.add_children_as_readables(): + self.demand = epics_signal_rw(float, prefix + "DMD") + + self._axis = axis + self._magnet_set_within_boundary = magnet_set_within_boundary + super().__init__(name) + @AsyncStatus.wrap + async def set(self, value: float): + values = {self._axis: value} + await self._magnet_set_within_boundary(**values) + -# Need to add spherical and cartesian coordinates class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): def __init__(self, prefix: str, name: str = ""): + with self.add_children_as_readables(): + # Cartesian and real pv values + self.x = MagnetAxis(prefix + "X:", "x", self.set_within_boundary) + self.y = MagnetAxis(prefix + "Y:", "y", self.set_within_boundary) + self.z = MagnetAxis(prefix + "Z:", "z", self.set_within_boundary) + + # Spherical representations of x, y, z + self.theta = derived_signal_r( + read_theta, x=self.x.readback, z=self.z.readback + ) + self.rho = derived_signal_r( + read_rho, x=self.x.readback, y=self.y.readback, z=self.z.readback + ) + self.phi = derived_signal_r( + read_phi, x=self.x.readback, y=self.y.readback, z=self.z.readback + ) - self.cartesian = CartesianCoordinates(prefix) - self.spherical = SphericalCoorindates( - self.cartesian.xo, self.cartesian.yo, self.cartesian.zo - ) + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") + self.timeout = soft_signal_rw(float, initial_value=300) + self.delay = soft_signal_rw(float, initial_value=0.1) - self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") self.ramp_status = epics_signal_rw(MagnetRampStatus, prefix + "RAMPSTATUS") self.limit_status = epics_signal_rw(MagnetLimitStatus, prefix + "LIMITSTATUS") self.start_ramp = epics_signal_x(prefix + "STARTRAMP.PROC") - self.tolerance = soft_signal_rw(float, initial_value=0) - self.timeout = soft_signal_rw(float, initial_value=300) - self.delay = soft_signal_rw(float, initial_value=5) - super().__init__(name) - # Without boundary @AsyncStatus.wrap - async def set(self, value: MagnetPosition): - await asyncio.gather( - self.cartesian.xi.set(value.x), - self.cartesian.yi.set(value.y), - self.cartesian.zi.set(value.z), - ) - await self._ramp() + async def set(self, value: MagnetPosition | SphericalMagnetPosition): + if isinstance(value, SphericalMagnetPosition): + value = value.to_cartesian() + await self.set_within_boundary(x=value.x, y=value.y, z=value.z) async def _ramp(self): await self.start_ramp.trigger(timeout=1) @@ -134,56 +149,53 @@ async def _ramp(self): # Is this still needed? await asyncio.sleep(await self.delay.get_value()) - ramp_status, limit_status = await asyncio.gather( - self.ramp_status.get_value(), - self.limit_status.get_value(), - ) - - if ( - ramp_status == MagnetRampStatus.RAMP_MADE - and limit_status == MagnetLimitStatus.OK - ): - return + await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, timeout=10) - if ramp_status == MagnetRampStatus.RAMPING: - raise RuntimeError("Magnet still ramping") - - if limit_status == MagnetLimitStatus.VIOLTATION: - raise RuntimeError("Magnet limit violation") + if await self.limit_status.get_value() == MagnetLimitStatus.VIOLTATION: + raise RuntimeError( + f"{self.limit_status.name} is at position {MagnetLimitStatus.VIOLTATION}" + ) - async def set_within_boundary(self, value: MagnetPosition): + async def set_within_boundary( + self, x: float | None = None, y: float | None = None, z: float | None = None + ): """Move magnet while ensuring field magnitude never increases before decreases have completed. """ + if x is None and y is None and z is None: + raise RuntimeError("Args x, y, and z cannot all be None at the same time.") + # For keeping the magnitude constrained to avoid quench, always do the decreasing before increasing motors x0, y0, z0 = await asyncio.gather( - self.xo.get_value(), - self.yo.get_value(), - self.zo.get_value(), + self.x.readback.get_value(), + self.y.readback.get_value(), + self.z.readback.get_value(), ) - x1, y1, z1 = value.x, value.y, value.z + x = x0 if x is None else x + y = y0 if y is None else y + z = z0 if z is None else z - dx = abs(x1) - abs(x0) - dy = abs(y1) - abs(y0) - dz = abs(z1) - abs(z0) + dx = abs(x) - abs(x0) + dy = abs(y) - abs(y0) + dz = abs(z) - abs(z0) # Decrease Z first if dz < 0: - await self.zi.set(z1) + await self.z.demand.set(z) await self._ramp() # Then decrease X if dx < 0: - await self.xi.set(x1) + await self.x.demand.set(x) await self._ramp() # Then decrease Y if dy < 0: - await self.yi.set(y1) + await self.y.demand.set(y) await self._ramp() # Finally perform all increases together if dx > 0 or dy > 0 or dz > 0: await asyncio.gather( - self.xi.set(x1), - self.yi.set(y1), - self.zi.set(z1), + self.x.demand.set(x), + self.y.demand.set(y), + self.z.demand.set(z), ) await self._ramp() From 56a754dcb66d57d7c5dbe3a73c5bc088602e5893 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 1 Jul 2026 13:08:20 +0000 Subject: [PATCH 05/30] Add to do comments --- src/dodal/devices/beamlines/i06_1/magnets/magnet.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py index 8191062b777..88cdd05d00a 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -81,6 +81,7 @@ def to_cartesian(self) -> MagnetPosition: ) +# ToDo - Use StandardMovable class MagnetAxis(StandardReadable, Movable[float]): def __init__( self, @@ -144,12 +145,10 @@ async def set(self, value: MagnetPosition | SphericalMagnetPosition): await self.set_within_boundary(x=value.x, y=value.y, z=value.z) async def _ramp(self): - await self.start_ramp.trigger(timeout=1) - # Due to EPICS flaw, after the callback, still need to wait for a few second for the magnet settle down - # Is this still needed? - await asyncio.sleep(await self.delay.get_value()) - - await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, timeout=10) + # ToDo - Use TimeoutCalculated from ophyd-async in new release. + timeout = await self.timeout.get_value() + await self.start_ramp.trigger(timeout=timeout) + await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, timeout) if await self.limit_status.get_value() == MagnetLimitStatus.VIOLTATION: raise RuntimeError( From d4f3dc734fa2220c6bf082e02d73ec7b920a7b30 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Mon, 6 Jul 2026 08:32:43 +0000 Subject: [PATCH 06/30] Add SuperconductingMagnetController --- src/dodal/devices/beamlines/i06_1/magnets/magnet.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py index 88cdd05d00a..1e794626b5b 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -107,6 +107,7 @@ async def set(self, value: float): await self._magnet_set_within_boundary(**values) +# Add support for GDA SuperconductingMagnetControllerClass as well class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): def __init__(self, prefix: str, name: str = ""): with self.add_children_as_readables(): @@ -198,3 +199,13 @@ async def set_within_boundary( self.z.demand.set(z), ) await self._ramp() + + +class SuperconductingMagnetController(StandardReadable): + def __init__(self, prefix: str, name: str = ""): + with self.add_children_as_readables(): + self.in_ = epics_signal_rw(float, prefix + ":SET:DMD:RAMPRATE:TPM") + self.out = epics_signal_rw(float, prefix + ":STS:RAMPRATE:TPM") + self.limit = epics_signal_rw(float, prefix + ":LIM:RAMPRATE:TPM") + + super().__init__(name) From 71b1cc0e57b335b8652ab270f265a0939820cb6f Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Jul 2026 15:59:39 +0000 Subject: [PATCH 07/30] Add RampMagnetController --- src/dodal/beamlines/i06_1.py | 14 ++++- .../devices/beamlines/i06_1/magnets/magnet.py | 60 +++++++++++++------ 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py index f634315619b..6a9a2cb3a09 100644 --- a/src/dodal/beamlines/i06_1.py +++ b/src/dodal/beamlines/i06_1.py @@ -2,7 +2,10 @@ from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.device_manager import DeviceManager from dodal.devices.beamlines.i06_1 import DiffractionDichroism -from dodal.devices.beamlines.i06_1.magnets.magnet import SuperConductingMagnet +from dodal.devices.beamlines.i06_1.magnets.magnet import ( + RampMagnetControllerGroup, + SuperConductingMagnet, +) from dodal.devices.motors import XYThetaStage from dodal.devices.temperture_controller import Lakeshore336 from dodal.log import set_beamline as set_log_beamline @@ -38,5 +41,10 @@ def dd() -> DiffractionDichroism: @devices.factory() -def superconducting_magnet() -> SuperConductingMagnet: - return SuperConductingMagnet(f"{PREFIX.beamline_prefix}-EA-MAG-01:") +def mag_ramp_rate() -> RampMagnetControllerGroup: + return RampMagnetControllerGroup(f"{PREFIX.beamline_prefix}-EA-SMC") + + +@devices.factory() +def scmc(mag_ramp_rate: RampMagnetControllerGroup) -> SuperConductingMagnet: + return SuperConductingMagnet(f"{PREFIX.beamline_prefix}-EA-MAG-01:", mag_ramp_rate) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py index 1e794626b5b..90d41aadf9c 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -6,6 +6,7 @@ from bluesky.protocols import Movable from ophyd_async.core import ( AsyncStatus, + Reference, StandardReadable, StandardReadableFormat, StrictEnum, @@ -81,20 +82,46 @@ def to_cartesian(self) -> MagnetPosition: ) -# ToDo - Use StandardMovable +class RampMagnetController(StandardReadable): + def __init__(self, prefix: str, name: str = ""): + with self.add_children_as_readables(): + self.out = epics_signal_rw(float, prefix + "STS:RAMPRATE:TPM") + self.in_ = epics_signal_rw(float, prefix + "SET:DMD:RAMPRATE:TPM") + self.limit = epics_signal_rw(float, prefix + "LIM:RAMPRATE:TPM") + super().__init__(name) + + @AsyncStatus.wrap + async def set(self, value: float): + await self.in_.set(value) + + +class RampMagnetControllerGroup(StandardReadable): + def __init__(self, prefix: str, name: str = ""): + with self.add_children_as_readables(): + self.x = RampMagnetController(prefix + "-01:") + self.y = RampMagnetController(prefix + "-02:") + self.z = RampMagnetController(prefix + "-03:") + + super().__init__(name) + + +# ToDo - Use StandardMovable? class MagnetAxis(StandardReadable, Movable[float]): def __init__( self, prefix: str, axis: str, + ramp_controller: RampMagnetController, magnet_set_within_boundary: Callable[[], Awaitable[None]], name: str = "", ): + # Used in fastfieldscan, need to add fly scan logic. + self.ramp_controller_ref = Reference(ramp_controller) + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): self.readback = epics_signal_r(float, prefix + "RBV") - with self.add_children_as_readables(): - self.demand = epics_signal_rw(float, prefix + "DMD") + self.demand = epics_signal_rw(float, prefix + "DMD") self._axis = axis self._magnet_set_within_boundary = magnet_set_within_boundary @@ -109,12 +136,20 @@ async def set(self, value: float): # Add support for GDA SuperconductingMagnetControllerClass as well class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): - def __init__(self, prefix: str, name: str = ""): + def __init__( + self, prefix: str, ramp_controllers: RampMagnetControllerGroup, name: str = "" + ): with self.add_children_as_readables(): # Cartesian and real pv values - self.x = MagnetAxis(prefix + "X:", "x", self.set_within_boundary) - self.y = MagnetAxis(prefix + "Y:", "y", self.set_within_boundary) - self.z = MagnetAxis(prefix + "Z:", "z", self.set_within_boundary) + self.x = MagnetAxis( + prefix + "X:", "x", ramp_controllers.x, self.set_within_boundary + ) + self.y = MagnetAxis( + prefix + "Y:", "y", ramp_controllers.y, self.set_within_boundary + ) + self.z = MagnetAxis( + prefix + "Z:", "z", ramp_controllers.y, self.set_within_boundary + ) # Spherical representations of x, y, z self.theta = derived_signal_r( @@ -166,6 +201,7 @@ async def set_within_boundary( raise RuntimeError("Args x, y, and z cannot all be None at the same time.") # For keeping the magnitude constrained to avoid quench, always do the decreasing before increasing motors + # Can this be simplified? x0, y0, z0 = await asyncio.gather( self.x.readback.get_value(), self.y.readback.get_value(), @@ -199,13 +235,3 @@ async def set_within_boundary( self.z.demand.set(z), ) await self._ramp() - - -class SuperconductingMagnetController(StandardReadable): - def __init__(self, prefix: str, name: str = ""): - with self.add_children_as_readables(): - self.in_ = epics_signal_rw(float, prefix + ":SET:DMD:RAMPRATE:TPM") - self.out = epics_signal_rw(float, prefix + ":STS:RAMPRATE:TPM") - self.limit = epics_signal_rw(float, prefix + ":LIM:RAMPRATE:TPM") - - super().__init__(name) From ac7224455f39967c505d80cc0f57c8abe532736c Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 7 Jul 2026 16:15:14 +0000 Subject: [PATCH 08/30] Move ramp controllers to own file --- .../devices/beamlines/i06_1/magnets/magnet.py | 30 ++++--------------- .../devices/beamlines/i06_1/magnets/ramp.py | 27 +++++++++++++++++ 2 files changed, 33 insertions(+), 24 deletions(-) create mode 100644 src/dodal/devices/beamlines/i06_1/magnets/ramp.py diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py index 90d41aadf9c..e259b3f9a43 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/magnet.py @@ -16,6 +16,11 @@ ) from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x +from dodal.devices.beamlines.i06_1.magnets.ramp import ( + RampMagnetController, + RampMagnetControllerGroup, +) + class MagnetModes(StrictEnum): UNIAXIAL_X = "UNIAXIAL_X" @@ -82,29 +87,6 @@ def to_cartesian(self) -> MagnetPosition: ) -class RampMagnetController(StandardReadable): - def __init__(self, prefix: str, name: str = ""): - with self.add_children_as_readables(): - self.out = epics_signal_rw(float, prefix + "STS:RAMPRATE:TPM") - self.in_ = epics_signal_rw(float, prefix + "SET:DMD:RAMPRATE:TPM") - self.limit = epics_signal_rw(float, prefix + "LIM:RAMPRATE:TPM") - super().__init__(name) - - @AsyncStatus.wrap - async def set(self, value: float): - await self.in_.set(value) - - -class RampMagnetControllerGroup(StandardReadable): - def __init__(self, prefix: str, name: str = ""): - with self.add_children_as_readables(): - self.x = RampMagnetController(prefix + "-01:") - self.y = RampMagnetController(prefix + "-02:") - self.z = RampMagnetController(prefix + "-03:") - - super().__init__(name) - - # ToDo - Use StandardMovable? class MagnetAxis(StandardReadable, Movable[float]): def __init__( @@ -134,7 +116,7 @@ async def set(self, value: float): await self._magnet_set_within_boundary(**values) -# Add support for GDA SuperconductingMagnetControllerClass as well +# Add spherical coordinate write values. class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): def __init__( self, prefix: str, ramp_controllers: RampMagnetControllerGroup, name: str = "" diff --git a/src/dodal/devices/beamlines/i06_1/magnets/ramp.py b/src/dodal/devices/beamlines/i06_1/magnets/ramp.py new file mode 100644 index 00000000000..bf12f29dfe7 --- /dev/null +++ b/src/dodal/devices/beamlines/i06_1/magnets/ramp.py @@ -0,0 +1,27 @@ +from bluesky.protocols import Movable +from ophyd_async.core import AsyncStatus, StandardReadable +from ophyd_async.epics.core import epics_signal_rw + + +# Equivalent to GDA SuperconductingMagnetControllerClass +class RampMagnetController(StandardReadable, Movable[float]): + def __init__(self, prefix: str, name: str = ""): + with self.add_children_as_readables(): + self.out = epics_signal_rw(float, prefix + "STS:RAMPRATE:TPM") + self.in_ = epics_signal_rw(float, prefix + "SET:DMD:RAMPRATE:TPM") + self.limit = epics_signal_rw(float, prefix + "LIM:RAMPRATE:TPM") + super().__init__(name) + + @AsyncStatus.wrap + async def set(self, value: float): + await self.in_.set(value) + + +class RampMagnetControllerGroup(StandardReadable): + def __init__(self, prefix: str, name: str = ""): + with self.add_children_as_readables(): + self.x = RampMagnetController(prefix + "-01:") + self.y = RampMagnetController(prefix + "-02:") + self.z = RampMagnetController(prefix + "-03:") + + super().__init__(name) From 770d4002ef5edd7e8fd3a709b3a56255fd4c8045 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 8 Jul 2026 12:10:00 +0000 Subject: [PATCH 09/30] Add read methods for spherical coordinates --- src/dodal/beamlines/i06_1.py | 6 +-- .../magnets/{ramp.py => ramp_controller.py} | 16 +++--- .../{magnet.py => superconducting_magnet.py} | 54 +++++++++++++------ 3 files changed, 50 insertions(+), 26 deletions(-) rename src/dodal/devices/beamlines/i06_1/magnets/{ramp.py => ramp_controller.py} (56%) rename src/dodal/devices/beamlines/i06_1/magnets/{magnet.py => superconducting_magnet.py} (78%) diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py index 6a9a2cb3a09..c6972a08ec8 100644 --- a/src/dodal/beamlines/i06_1.py +++ b/src/dodal/beamlines/i06_1.py @@ -2,7 +2,7 @@ from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.device_manager import DeviceManager from dodal.devices.beamlines.i06_1 import DiffractionDichroism -from dodal.devices.beamlines.i06_1.magnets.magnet import ( +from dodal.devices.beamlines.i06_1.magnets.superconducting_magnet import ( RampMagnetControllerGroup, SuperConductingMagnet, ) @@ -21,12 +21,12 @@ @devices.factory() -def diff_cooling_temperature_controller() -> Lakeshore336: +def ls336() -> Lakeshore336: return Lakeshore336(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-02:") @devices.factory() -def diff_heating_temperature_controller() -> Lakeshore336: +def ls336_2() -> Lakeshore336: return Lakeshore336(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-03:") diff --git a/src/dodal/devices/beamlines/i06_1/magnets/ramp.py b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py similarity index 56% rename from src/dodal/devices/beamlines/i06_1/magnets/ramp.py rename to src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py index bf12f29dfe7..785182facc3 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/ramp.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py @@ -4,24 +4,26 @@ # Equivalent to GDA SuperconductingMagnetControllerClass -class RampMagnetController(StandardReadable, Movable[float]): +class RampMagnetAxisController(StandardReadable, Movable[float]): def __init__(self, prefix: str, name: str = ""): with self.add_children_as_readables(): - self.out = epics_signal_rw(float, prefix + "STS:RAMPRATE:TPM") - self.in_ = epics_signal_rw(float, prefix + "SET:DMD:RAMPRATE:TPM") + self.ramp_rate_readback = epics_signal_rw( + float, prefix + "STS:RAMPRATE:TPM" + ) + self.ramp_rate_demand = epics_signal_rw(float, prefix + "SET:DMD:RAMPRATE:TPM") self.limit = epics_signal_rw(float, prefix + "LIM:RAMPRATE:TPM") super().__init__(name) @AsyncStatus.wrap async def set(self, value: float): - await self.in_.set(value) + await self.ramp_rate_demand.set(value) class RampMagnetControllerGroup(StandardReadable): def __init__(self, prefix: str, name: str = ""): with self.add_children_as_readables(): - self.x = RampMagnetController(prefix + "-01:") - self.y = RampMagnetController(prefix + "-02:") - self.z = RampMagnetController(prefix + "-03:") + self.x = RampMagnetAxisController(prefix + "-01:") + self.y = RampMagnetAxisController(prefix + "-02:") + self.z = RampMagnetAxisController(prefix + "-03:") super().__init__(name) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py similarity index 78% rename from src/dodal/devices/beamlines/i06_1/magnets/magnet.py rename to src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index e259b3f9a43..05fbd48ab56 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -10,14 +10,14 @@ StandardReadable, StandardReadableFormat, StrictEnum, - derived_signal_r, + derived_signal_rw, soft_signal_rw, wait_for_value, ) from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x -from dodal.devices.beamlines.i06_1.magnets.ramp import ( - RampMagnetController, +from dodal.devices.beamlines.i06_1.magnets.ramp_controller import ( + RampMagnetAxisController, RampMagnetControllerGroup, ) @@ -62,8 +62,8 @@ class MagnetPosition: y: float z: float - def to_spherical(self) -> "SphericalMagnetPosition": - return SphericalMagnetPosition( + def to_spherical(self) -> "MagnetSphericalPosition": + return MagnetSphericalPosition( rho=read_rho(self.x, self.y, self.z), theta=read_theta(self.x, self.z), phi=read_phi(self.x, self.y, self.z), @@ -71,7 +71,7 @@ def to_spherical(self) -> "SphericalMagnetPosition": @dataclass(frozen=True) -class SphericalMagnetPosition: +class MagnetSphericalPosition: rho: float theta: float # degrees phi: float # degrees @@ -93,7 +93,7 @@ def __init__( self, prefix: str, axis: str, - ramp_controller: RampMagnetController, + ramp_controller: RampMagnetAxisController, magnet_set_within_boundary: Callable[[], Awaitable[None]], name: str = "", ): @@ -132,16 +132,23 @@ def __init__( self.z = MagnetAxis( prefix + "Z:", "z", ramp_controllers.y, self.set_within_boundary ) - # Spherical representations of x, y, z - self.theta = derived_signal_r( - read_theta, x=self.x.readback, z=self.z.readback + self.theta = derived_signal_rw( + read_theta, self._set_rho, x=self.x.readback, z=self.z.readback ) - self.rho = derived_signal_r( - read_rho, x=self.x.readback, y=self.y.readback, z=self.z.readback + self.rho = derived_signal_rw( + read_rho, + self._set_rho, + x=self.x.readback, + y=self.y.readback, + z=self.z.readback, ) - self.phi = derived_signal_r( - read_phi, x=self.x.readback, y=self.y.readback, z=self.z.readback + self.phi = derived_signal_rw( + read_phi, + self._set_phi, + x=self.x.readback, + y=self.y.readback, + z=self.z.readback, ) with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): @@ -156,9 +163,24 @@ def __init__( super().__init__(name) + async def _set_rho(self, rho: float): + theta, phi = await asyncio.gather(self.theta.get_value(), self.phi.get_value()) + spherical_pos = MagnetSphericalPosition(rho=rho, theta=theta, phi=phi) + await self.set(spherical_pos) + + async def _set_theta(self, theta: float): + rho, phi = await asyncio.gather(self.rho.get_value(), self.phi.get_value()) + spherical_pos = MagnetSphericalPosition(rho=rho, theta=theta, phi=phi) + await self.set(spherical_pos) + + async def _set_phi(self, phi: float): + rho, theta = await asyncio.gather(self.rho.get_value(), self.theta.get_value()) + spherical_pos = MagnetSphericalPosition(rho=rho, theta=theta, phi=phi) + await self.set(spherical_pos) + @AsyncStatus.wrap - async def set(self, value: MagnetPosition | SphericalMagnetPosition): - if isinstance(value, SphericalMagnetPosition): + async def set(self, value: MagnetPosition | MagnetSphericalPosition): + if isinstance(value, MagnetSphericalPosition): value = value.to_cartesian() await self.set_within_boundary(x=value.x, y=value.y, z=value.z) From 706c39c5d442751f240486d68b26d97e3c1e72be Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 8 Jul 2026 16:08:19 +0000 Subject: [PATCH 10/30] Rename to ramp controllers to MagnetAxisRampRateController --- src/dodal/beamlines/i06_1.py | 10 +++---- .../beamlines/i06_1/magnets/__init__.py | 25 +++++++++++++++++ .../i06_1/magnets/superconducting_magnet.py | 28 +++++++++---------- .../beamlines/i06_1/magnets/__init__.py | 0 4 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 tests/devices/beamlines/i06_1/magnets/__init__.py diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py index c6972a08ec8..41d8f5cefbe 100644 --- a/src/dodal/beamlines/i06_1.py +++ b/src/dodal/beamlines/i06_1.py @@ -2,8 +2,8 @@ from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.device_manager import DeviceManager from dodal.devices.beamlines.i06_1 import DiffractionDichroism -from dodal.devices.beamlines.i06_1.magnets.superconducting_magnet import ( - RampMagnetControllerGroup, +from dodal.devices.beamlines.i06_1.magnets import ( + MagnetAxisRampRateControllerGroup, SuperConductingMagnet, ) from dodal.devices.motors import XYThetaStage @@ -41,10 +41,10 @@ def dd() -> DiffractionDichroism: @devices.factory() -def mag_ramp_rate() -> RampMagnetControllerGroup: - return RampMagnetControllerGroup(f"{PREFIX.beamline_prefix}-EA-SMC") +def mag_ramp_rate() -> MagnetAxisRampRateControllerGroup: + return MagnetAxisRampRateControllerGroup(f"{PREFIX.beamline_prefix}-EA-SMC") @devices.factory() -def scmc(mag_ramp_rate: RampMagnetControllerGroup) -> SuperConductingMagnet: +def scmc(mag_ramp_rate: MagnetAxisRampRateControllerGroup) -> SuperConductingMagnet: return SuperConductingMagnet(f"{PREFIX.beamline_prefix}-EA-MAG-01:", mag_ramp_rate) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py index e69de29bb2d..65076ca2731 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py @@ -0,0 +1,25 @@ +from .ramp_controller import ( + MagnetAxisRampRateController, + MagnetAxisRampRateControllerGroup, +) +from .superconducting_magnet import ( + MagnetAxis, + MagnetLimitStatus, + MagnetModes, + MagnetPosition, + MagnetRampStatus, + MagnetSphericalPosition, + SuperConductingMagnet, +) + +__all__ = [ + "MagnetAxisRampRateController", + "MagnetAxisRampRateControllerGroup", + "MagnetAxis", + "MagnetLimitStatus", + "MagnetModes", + "MagnetPosition", + "MagnetRampStatus", + "MagnetSphericalPosition", + "SuperConductingMagnet", +] diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 05fbd48ab56..53798ab6d61 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -17,8 +17,8 @@ from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x from dodal.devices.beamlines.i06_1.magnets.ramp_controller import ( - RampMagnetAxisController, - RampMagnetControllerGroup, + MagnetAxisRampRateController, + MagnetAxisRampRateControllerGroup, ) @@ -93,7 +93,7 @@ def __init__( self, prefix: str, axis: str, - ramp_controller: RampMagnetAxisController, + ramp_controller: MagnetAxisRampRateController, magnet_set_within_boundary: Callable[[], Awaitable[None]], name: str = "", ): @@ -119,7 +119,10 @@ async def set(self, value: float): # Add spherical coordinate write values. class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): def __init__( - self, prefix: str, ramp_controllers: RampMagnetControllerGroup, name: str = "" + self, + prefix: str, + ramp_controllers: MagnetAxisRampRateControllerGroup, + name: str = "", ): with self.add_children_as_readables(): # Cartesian and real pv values @@ -134,21 +137,17 @@ def __init__( ) # Spherical representations of x, y, z self.theta = derived_signal_rw( - read_theta, self._set_rho, x=self.x.readback, z=self.z.readback + read_theta, self._set_rho, x=self.x, z=self.z ) self.rho = derived_signal_rw( - read_rho, - self._set_rho, - x=self.x.readback, - y=self.y.readback, - z=self.z.readback, + read_rho, self._set_rho, x=self.x, y=self.y, z=self.z ) self.phi = derived_signal_rw( read_phi, self._set_phi, - x=self.x.readback, - y=self.y.readback, - z=self.z.readback, + x=self.x, + y=self.y, + z=self.z, ) with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): @@ -204,7 +203,8 @@ async def set_within_boundary( if x is None and y is None and z is None: raise RuntimeError("Args x, y, and z cannot all be None at the same time.") - # For keeping the magnitude constrained to avoid quench, always do the decreasing before increasing motors + # For keeping the magnitude constrained to avoid quench, always do the + # decreasing before increasing motors # Can this be simplified? x0, y0, z0 = await asyncio.gather( self.x.readback.get_value(), diff --git a/tests/devices/beamlines/i06_1/magnets/__init__.py b/tests/devices/beamlines/i06_1/magnets/__init__.py new file mode 100644 index 00000000000..e69de29bb2d From 34873130ceb516b0f8e5199c690b73749b07bed2 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 8 Jul 2026 19:45:11 +0000 Subject: [PATCH 11/30] Add ramp_controller tests --- src/dodal/beamlines/i06_1.py | 8 +-- .../beamlines/i06_1/magnets/__init__.py | 4 +- .../i06_1/magnets/ramp_controller.py | 40 +++++++++------ .../i06_1/magnets/superconducting_magnet.py | 4 +- .../i06_1/magnets/test_ramp_controller.py | 49 +++++++++++++++++++ .../magnets/test_superconducting_magnet.py | 0 6 files changed, 82 insertions(+), 23 deletions(-) create mode 100644 tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py create mode 100644 tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py index 41d8f5cefbe..280291bb3eb 100644 --- a/src/dodal/beamlines/i06_1.py +++ b/src/dodal/beamlines/i06_1.py @@ -3,7 +3,7 @@ from dodal.device_manager import DeviceManager from dodal.devices.beamlines.i06_1 import DiffractionDichroism from dodal.devices.beamlines.i06_1.magnets import ( - MagnetAxisRampRateControllerGroup, + MagnetThreeAxesRampRateController, SuperConductingMagnet, ) from dodal.devices.motors import XYThetaStage @@ -41,10 +41,10 @@ def dd() -> DiffractionDichroism: @devices.factory() -def mag_ramp_rate() -> MagnetAxisRampRateControllerGroup: - return MagnetAxisRampRateControllerGroup(f"{PREFIX.beamline_prefix}-EA-SMC") +def mag_ramp_rate() -> MagnetThreeAxesRampRateController: + return MagnetThreeAxesRampRateController(f"{PREFIX.beamline_prefix}-EA-SMC") @devices.factory() -def scmc(mag_ramp_rate: MagnetAxisRampRateControllerGroup) -> SuperConductingMagnet: +def scmc(mag_ramp_rate: MagnetThreeAxesRampRateController) -> SuperConductingMagnet: return SuperConductingMagnet(f"{PREFIX.beamline_prefix}-EA-MAG-01:", mag_ramp_rate) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py index 65076ca2731..aecf53e401b 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py @@ -1,6 +1,6 @@ from .ramp_controller import ( MagnetAxisRampRateController, - MagnetAxisRampRateControllerGroup, + MagnetThreeAxesRampRateController, ) from .superconducting_magnet import ( MagnetAxis, @@ -14,7 +14,7 @@ __all__ = [ "MagnetAxisRampRateController", - "MagnetAxisRampRateControllerGroup", + "MagnetThreeAxesRampRateController", "MagnetAxis", "MagnetLimitStatus", "MagnetModes", diff --git a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py index 785182facc3..d0286f17839 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py @@ -1,29 +1,39 @@ -from bluesky.protocols import Movable -from ophyd_async.core import AsyncStatus, StandardReadable +from functools import cached_property + +from ophyd_async.core import ( + MovableLogic, + StandardMovable, + StandardReadable, + StandardReadableFormat, + TimeoutCalculator, +) from ophyd_async.epics.core import epics_signal_rw +class RampRateMovableLogic(MovableLogic[float]): + async def move(self, new_position: float, timeout: TimeoutCalculator): + await self.setpoint.set(new_position, timeout=timeout()) + + # Equivalent to GDA SuperconductingMagnetControllerClass -class RampMagnetAxisController(StandardReadable, Movable[float]): +class MagnetAxisRampRateController(StandardMovable[float], StandardReadable): def __init__(self, prefix: str, name: str = ""): - with self.add_children_as_readables(): - self.ramp_rate_readback = epics_signal_rw( - float, prefix + "STS:RAMPRATE:TPM" - ) - self.ramp_rate_demand = epics_signal_rw(float, prefix + "SET:DMD:RAMPRATE:TPM") + with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): + self.readback = epics_signal_rw(float, prefix + "STS:RAMPRATE:TPM") + self.demand = epics_signal_rw(float, prefix + "SET:DMD:RAMPRATE:TPM") self.limit = epics_signal_rw(float, prefix + "LIM:RAMPRATE:TPM") super().__init__(name) - @AsyncStatus.wrap - async def set(self, value: float): - await self.ramp_rate_demand.set(value) + @cached_property + def movable_logic(self) -> RampRateMovableLogic: + return RampRateMovableLogic(readback=self.readback, setpoint=self.demand) -class RampMagnetControllerGroup(StandardReadable): +class MagnetThreeAxesRampRateController(StandardReadable): def __init__(self, prefix: str, name: str = ""): with self.add_children_as_readables(): - self.x = RampMagnetAxisController(prefix + "-01:") - self.y = RampMagnetAxisController(prefix + "-02:") - self.z = RampMagnetAxisController(prefix + "-03:") + self.x = MagnetAxisRampRateController(prefix + "-01:") + self.y = MagnetAxisRampRateController(prefix + "-02:") + self.z = MagnetAxisRampRateController(prefix + "-03:") super().__init__(name) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 53798ab6d61..17dffbc9fc2 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -18,7 +18,7 @@ from dodal.devices.beamlines.i06_1.magnets.ramp_controller import ( MagnetAxisRampRateController, - MagnetAxisRampRateControllerGroup, + MagnetThreeAxesRampRateController, ) @@ -121,7 +121,7 @@ class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): def __init__( self, prefix: str, - ramp_controllers: MagnetAxisRampRateControllerGroup, + ramp_controllers: MagnetThreeAxesRampRateController, name: str = "", ): with self.add_children_as_readables(): diff --git a/tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py b/tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py new file mode 100644 index 00000000000..14f9822d332 --- /dev/null +++ b/tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py @@ -0,0 +1,49 @@ +import pytest +from ophyd_async.core import init_devices +from ophyd_async.testing import assert_reading, partial_reading + +from dodal.devices.beamlines.i06_1.magnets import ( + MagnetAxisRampRateController, + MagnetThreeAxesRampRateController, +) + + +@pytest.fixture +def magx_ramp_rate() -> MagnetAxisRampRateController: + with init_devices(mock=True): + magx_ramp_rate = MagnetAxisRampRateController("TEST:") + return magx_ramp_rate + + +async def test_magx_ramp_rate_read( + magx_ramp_rate: MagnetAxisRampRateController, +) -> None: + await assert_reading(magx_ramp_rate, {"magx_ramp_rate": partial_reading(0)}) + + +async def test_magx_ramp_rate_set( + magx_ramp_rate: MagnetAxisRampRateController, +) -> None: + ramp_rate = 10 + await magx_ramp_rate.set(ramp_rate) + assert await magx_ramp_rate.readback.get_value() == ramp_rate + + +@pytest.fixture +def mag_three_axis_ramp_rate() -> MagnetThreeAxesRampRateController: + with init_devices(mock=True): + mag_three_axis_ramp_rate = MagnetThreeAxesRampRateController("TEST:") + return mag_three_axis_ramp_rate + + +async def test_mag_three_axis_ramp_rate_read( + mag_three_axis_ramp_rate: MagnetThreeAxesRampRateController, +) -> None: + await assert_reading( + mag_three_axis_ramp_rate, + { + "mag_three_axis_ramp_rate-x": partial_reading(0), + "mag_three_axis_ramp_rate-y": partial_reading(0), + "mag_three_axis_ramp_rate-z": partial_reading(0), + }, + ) diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py new file mode 100644 index 00000000000..e69de29bb2d From 1deca95923d1f148f00c4b67a6943efd49884a42 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 9 Jul 2026 12:42:56 +0000 Subject: [PATCH 12/30] Add tests for superconducting_magnet --- .../i06_1/magnets/superconducting_magnet.py | 59 ++++-- .../magnets/test_superconducting_magnet.py | 173 ++++++++++++++++++ 2 files changed, 218 insertions(+), 14 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 17dffbc9fc2..a0c2119c2d9 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -2,15 +2,22 @@ import math from collections.abc import Awaitable, Callable from dataclasses import dataclass +from functools import cached_property from bluesky.protocols import Movable from ophyd_async.core import ( AsyncStatus, + DeviceMock, + MovableLogic, Reference, + StandardMovable, StandardReadable, StandardReadableFormat, StrictEnum, + callback_on_mock_put, + default_mock_class, derived_signal_rw, + set_mock_value, soft_signal_rw, wait_for_value, ) @@ -39,8 +46,8 @@ class MagnetRampStatus(StrictEnum): class MagnetLimitStatus(StrictEnum): - VIOLTATION = "VIOLATION" OK = "OK" + VIOLTATION = "VIOLATION" def read_theta(x: float, z: float) -> float: @@ -56,7 +63,7 @@ def read_phi(x: float, y: float, z: float) -> float: return math.degrees(math.atan2(math.hypot(x, z), y)) -@dataclass +@dataclass(kw_only=True) class MagnetPosition: x: float y: float @@ -70,7 +77,7 @@ def to_spherical(self) -> "MagnetSphericalPosition": ) -@dataclass(frozen=True) +@dataclass(kw_only=True) class MagnetSphericalPosition: rho: float theta: float # degrees @@ -87,8 +94,20 @@ def to_cartesian(self) -> MagnetPosition: ) -# ToDo - Use StandardMovable? -class MagnetAxis(StandardReadable, Movable[float]): +@dataclass +class MagnetAxisMovableLogic(MovableLogic[float]): + axis: str + magnet_set_within_boundary: Callable[[], Awaitable[None]] + + async def move( + self, new_position: float, timeout: Callable[[], float | None] + ) -> None: + values = {self.axis: new_position} + # ToDo - feed timeout to here? + await self.magnet_set_within_boundary(**values) + + +class MagnetAxis(StandardReadable, StandardMovable[float]): def __init__( self, prefix: str, @@ -102,21 +121,33 @@ def __init__( with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): self.readback = epics_signal_r(float, prefix + "RBV") - self.demand = epics_signal_rw(float, prefix + "DMD") - self._axis = axis - self._magnet_set_within_boundary = magnet_set_within_boundary - + self._movable_logic = MagnetAxisMovableLogic( + readback=self.readback, + setpoint=self.demand, + axis=axis, + magnet_set_within_boundary=magnet_set_within_boundary, + ) super().__init__(name) - @AsyncStatus.wrap - async def set(self, value: float): - values = {self._axis: value} - await self._magnet_set_within_boundary(**values) + @cached_property + def movable_logic(self) -> MagnetAxisMovableLogic: + return self._movable_logic + + +class MockSuperConductingMagnet(DeviceMock["SuperConductingMagnet"]): + async def connect(self, device: "SuperConductingMagnet"): + + def _set_ramp_status(value): + set_mock_value(device.ramp_status, MagnetRampStatus.RAMPING) + set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) + + callback_on_mock_put(device.start_ramp, _set_ramp_status) + set_mock_value(device.limit_status, MagnetLimitStatus.OK) -# Add spherical coordinate write values. +@default_mock_class(MockSuperConductingMagnet) class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): def __init__( self, diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index e69de29bb2d..b5267582d4e 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -0,0 +1,173 @@ +import asyncio + +import pytest +from ophyd_async.core import init_devices, set_mock_value +from ophyd_async.testing import assert_reading, partial_reading + +from dodal.devices.beamlines.i06_1.magnets import ( + MagnetLimitStatus, + MagnetPosition, + MagnetSphericalPosition, + MagnetThreeAxesRampRateController, + SuperConductingMagnet, +) + +EXPECTED_CARTESIAN_SPHERICAL_CONVERSION = [ + (MagnetPosition(x=0, y=0, z=1), MagnetSphericalPosition(rho=1, theta=0, phi=90)), + (MagnetPosition(x=-1, y=0, z=0), MagnetSphericalPosition(rho=1, theta=90, phi=90)), + (MagnetPosition(x=0, y=1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=0)), + (MagnetPosition(x=0, y=-1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=180)), +] + + +@pytest.fixture +def ramp_rate() -> MagnetThreeAxesRampRateController: + with init_devices(mock=True): + ramp_rate = MagnetThreeAxesRampRateController("TEST:") + return ramp_rate + + +@pytest.fixture +def scmc(ramp_rate: MagnetThreeAxesRampRateController) -> SuperConductingMagnet: + with init_devices(mock=True): + scmc = SuperConductingMagnet("TEST:", ramp_rate) + return scmc + + +async def test_scmc_read(scmc: SuperConductingMagnet) -> None: + await assert_reading( + scmc, + { + "scmc-x": partial_reading(0), + "scmc-y": partial_reading(0), + "scmc-z": partial_reading(0), + "scmc-theta": partial_reading(0), + "scmc-rho": partial_reading(0), + "scmc-phi": partial_reading(0), + }, + ) + + +@pytest.mark.parametrize( + "axis, value, expected_x, expected_y, expected_z", + [ + ("x", 10, 10, 0, 0), + ("y", 20, 0, 20, 0), + ("z", 30, 0, 0, 30), + ], +) +async def test_scmc_axis_set_uses_correct_axis( + scmc: SuperConductingMagnet, + axis: str, + value: float, + expected_x: float, + expected_y: float, + expected_z: float, +) -> None: + await getattr(scmc, axis).set(value) + x, y, z = await asyncio.gather( + scmc.x.readback.get_value(), + scmc.y.readback.get_value(), + scmc.z.readback.get_value(), + ) + assert x == expected_x + assert y == expected_y + assert z == expected_z + + +def test_cartesian_and_spherical_are_inverse() -> None: + cartesian = MagnetPosition(x=10, y=20, z=30) + result = cartesian.to_spherical().to_cartesian() + assert result.x == pytest.approx(cartesian.x) + assert result.y == pytest.approx(cartesian.y) + assert result.z == pytest.approx(cartesian.z) + + +def test_spherical_and_cartesian_are_inverse() -> None: + spherical = MagnetSphericalPosition(rho=10, theta=20, phi=30) + result = spherical.to_cartesian().to_spherical() + assert result.rho == pytest.approx(spherical.rho) + assert result.theta == pytest.approx(spherical.theta) + assert result.phi == pytest.approx(spherical.phi) + + +@pytest.mark.parametrize( + "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION +) +def test_cartesian_and_spherical_conversion_is_correct( + cartesian: MagnetPosition, + spherical: MagnetSphericalPosition, +) -> None: + result = cartesian.to_spherical() + assert result.rho == pytest.approx(spherical.rho) + assert result.theta == pytest.approx(spherical.theta) + assert result.phi == pytest.approx(spherical.phi) + + result = spherical.to_cartesian() + assert result.x == pytest.approx(cartesian.x) + assert result.y == pytest.approx(cartesian.y) + assert result.z == pytest.approx(cartesian.z) + + +@pytest.mark.parametrize( + "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION +) +async def test_scmc_set_using_spherical( + scmc: SuperConductingMagnet, + cartesian: MagnetPosition, + spherical: MagnetSphericalPosition, +) -> None: + await scmc.set(spherical) + rho, theta, phi = await asyncio.gather( + scmc.rho.get_value(), + scmc.theta.get_value(), + scmc.phi.get_value(), + ) + assert rho == pytest.approx(spherical.rho) + assert theta == pytest.approx(spherical.theta) + assert phi == pytest.approx(spherical.phi) + + x, y, z = await asyncio.gather( + scmc.x.readback.get_value(), + scmc.y.readback.get_value(), + scmc.z.readback.get_value(), + ) + assert x == pytest.approx(cartesian.x) + assert y == pytest.approx(cartesian.y) + assert z == pytest.approx(cartesian.z) + + +@pytest.mark.parametrize( + "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION +) +async def test_scmc_set_using_cartesian( + scmc: SuperConductingMagnet, + cartesian: MagnetPosition, + spherical: MagnetSphericalPosition, +) -> None: + await scmc.set(cartesian) + x, y, z = await asyncio.gather( + scmc.x.readback.get_value(), + scmc.y.readback.get_value(), + scmc.z.readback.get_value(), + ) + assert x == pytest.approx(cartesian.x) + assert y == pytest.approx(cartesian.y) + assert z == pytest.approx(cartesian.z) + + rho, theta, phi = await asyncio.gather( + scmc.rho.get_value(), + scmc.theta.get_value(), + scmc.phi.get_value(), + ) + assert rho == pytest.approx(spherical.rho) + assert theta == pytest.approx(spherical.theta) + assert phi == pytest.approx(spherical.phi) + + +async def test_scmc_raises_error_if_limit_status_is_violation( + scmc: SuperConductingMagnet, +) -> None: + set_mock_value(scmc.limit_status, MagnetLimitStatus.VIOLTATION) + with pytest.raises(RuntimeError): + await scmc.x.set(10) From 712ca91571bedc1dae2e7ef66e0650e6015d364a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 9 Jul 2026 14:18:35 +0000 Subject: [PATCH 13/30] Add individual spherical axis tests --- .../i06_1/magnets/superconducting_magnet.py | 35 +++++++++++------ .../magnets/test_superconducting_magnet.py | 38 +++++++++++++++++++ 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index a0c2119c2d9..3328e53d9ed 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -164,11 +164,11 @@ def __init__( prefix + "Y:", "y", ramp_controllers.y, self.set_within_boundary ) self.z = MagnetAxis( - prefix + "Z:", "z", ramp_controllers.y, self.set_within_boundary + prefix + "Z:", "z", ramp_controllers.z, self.set_within_boundary ) # Spherical representations of x, y, z self.theta = derived_signal_rw( - read_theta, self._set_rho, x=self.x, z=self.z + read_theta, self._set_theta, x=self.x, z=self.z ) self.rho = derived_signal_rw( read_rho, self._set_rho, x=self.x, y=self.y, z=self.z @@ -193,20 +193,33 @@ def __init__( super().__init__(name) - async def _set_rho(self, rho: float): - theta, phi = await asyncio.gather(self.theta.get_value(), self.phi.get_value()) - spherical_pos = MagnetSphericalPosition(rho=rho, theta=theta, phi=phi) + async def _set_spherical( + self, + rho: float | None = None, + theta: float | None = None, + phi: float | None = None, + ): + x, y, z = await asyncio.gather( + self.x.readback.get_value(), + self.y.readback.get_value(), + self.z.readback.get_value(), + ) + current = MagnetPosition(x=x, y=y, z=z).to_spherical() + spherical_pos = MagnetSphericalPosition( + rho=current.rho if rho is None else rho, + theta=current.theta if theta is None else theta, + phi=current.phi if phi is None else phi, + ) await self.set(spherical_pos) + async def _set_rho(self, rho: float): + await self._set_spherical(rho=rho) + async def _set_theta(self, theta: float): - rho, phi = await asyncio.gather(self.rho.get_value(), self.phi.get_value()) - spherical_pos = MagnetSphericalPosition(rho=rho, theta=theta, phi=phi) - await self.set(spherical_pos) + await self._set_spherical(theta=theta) async def _set_phi(self, phi: float): - rho, theta = await asyncio.gather(self.rho.get_value(), self.theta.get_value()) - spherical_pos = MagnetSphericalPosition(rho=rho, theta=theta, phi=phi) - await self.set(spherical_pos) + await self._set_spherical(phi=phi) @AsyncStatus.wrap async def set(self, value: MagnetPosition | MagnetSphericalPosition): diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index b5267582d4e..36782c7f8ba 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -165,6 +165,44 @@ async def test_scmc_set_using_cartesian( assert phi == pytest.approx(spherical.phi) +@pytest.mark.parametrize( + "axis, value, expected_spherical", + [ + ("rho", 2, MagnetSphericalPosition(rho=2, theta=20, phi=30)), + ("theta", 40, MagnetSphericalPosition(rho=1, theta=40, phi=30)), + ("phi", 60, MagnetSphericalPosition(rho=1, theta=20, phi=60)), + ], +) +async def test_scmc_spherical_axis_set( + scmc: SuperConductingMagnet, + axis: str, + value: float, + expected_spherical: MagnetSphericalPosition, +) -> None: + # Start from a non-singular position. + await scmc.set(MagnetSphericalPosition(rho=1, theta=20, phi=30)) + await getattr(scmc, axis).set(value) + + rho, theta, phi = await asyncio.gather( + scmc.rho.get_value(), + scmc.theta.get_value(), + scmc.phi.get_value(), + ) + assert rho == pytest.approx(expected_spherical.rho) + assert theta == pytest.approx(expected_spherical.theta) + assert phi == pytest.approx(expected_spherical.phi) + + expected_cartesian = expected_spherical.to_cartesian() + x, y, z = await asyncio.gather( + scmc.x.readback.get_value(), + scmc.y.readback.get_value(), + scmc.z.readback.get_value(), + ) + assert x == pytest.approx(expected_cartesian.x) + assert y == pytest.approx(expected_cartesian.y) + assert z == pytest.approx(expected_cartesian.z) + + async def test_scmc_raises_error_if_limit_status_is_violation( scmc: SuperConductingMagnet, ) -> None: From 7da935cecb2365d8835bcb3c2b18bb9e9a9550a1 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 9 Jul 2026 14:22:24 +0000 Subject: [PATCH 14/30] Increased timeout to 600 seconds, no longer a signal --- .../beamlines/i06_1/magnets/superconducting_magnet.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 3328e53d9ed..5e4b6652ab1 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -183,13 +183,13 @@ def __init__( with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") - self.timeout = soft_signal_rw(float, initial_value=300) self.delay = soft_signal_rw(float, initial_value=0.1) self.ramp_status = epics_signal_rw(MagnetRampStatus, prefix + "RAMPSTATUS") self.limit_status = epics_signal_rw(MagnetLimitStatus, prefix + "LIMITSTATUS") self.start_ramp = epics_signal_x(prefix + "STARTRAMP.PROC") + self.timeout = 600 super().__init__(name) @@ -229,9 +229,8 @@ async def set(self, value: MagnetPosition | MagnetSphericalPosition): async def _ramp(self): # ToDo - Use TimeoutCalculated from ophyd-async in new release. - timeout = await self.timeout.get_value() - await self.start_ramp.trigger(timeout=timeout) - await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, timeout) + await self.start_ramp.trigger(timeout=self.timeout) + await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, self.timeout) if await self.limit_status.get_value() == MagnetLimitStatus.VIOLTATION: raise RuntimeError( From 27389fb505f7b169f70b77a3f6960439d8933b5a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 9 Jul 2026 14:33:01 +0000 Subject: [PATCH 15/30] Add order of set test --- .../magnets/test_superconducting_magnet.py | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 36782c7f8ba..39e9abc47e9 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -1,4 +1,5 @@ import asyncio +from unittest.mock import AsyncMock, call import pytest from ophyd_async.core import init_devices, set_mock_value @@ -209,3 +210,68 @@ async def test_scmc_raises_error_if_limit_status_is_violation( set_mock_value(scmc.limit_status, MagnetLimitStatus.VIOLTATION) with pytest.raises(RuntimeError): await scmc.x.set(10) + + +@pytest.mark.parametrize( + "initial, target, expected_calls", + [ + ( + MagnetPosition(x=10, y=10, z=10), + MagnetPosition(x=20, y=5, z=2), + [ + call.z(2), + call.ramp(), + call.y(5), + call.ramp(), + call.x(20), + call.y(5), + call.z(2), + call.ramp(), + ], + ), + ( + MagnetPosition(x=10, y=10, z=10), + MagnetPosition(x=5, y=20, z=2), + [ + call.z(2), + call.ramp(), + call.x(5), + call.ramp(), + call.x(5), + call.y(20), + call.z(2), + call.ramp(), + ], + ), + ( + MagnetPosition(x=10, y=10, z=10), + MagnetPosition(x=5, y=2, z=20), + [ + call.x(5), + call.ramp(), + call.y(2), + call.ramp(), + call.x(5), + call.y(2), + call.z(20), + call.ramp(), + ], + ), + ], +) +async def test_scmc_set_decreases_before_increases( + scmc: SuperConductingMagnet, + initial: MagnetPosition, + target: MagnetPosition, + expected_calls, +) -> None: + await scmc.set(initial) + + calls = [] + scmc.x.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.x(v))) + scmc.y.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.y(v))) + scmc.z.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.z(v))) + scmc._ramp = AsyncMock(side_effect=lambda: calls.append(call.ramp())) + + await scmc.set(target) + assert calls == expected_calls From c45047f428723bb32652b136210edaa3dd88d97c Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Thu, 9 Jul 2026 15:37:36 +0000 Subject: [PATCH 16/30] Add missing code coverage --- .../magnets/test_superconducting_magnet.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 39e9abc47e9..792ca9c6229 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -257,6 +257,18 @@ async def test_scmc_raises_error_if_limit_status_is_violation( call.ramp(), ], ), + ( + MagnetPosition(x=10, y=10, z=10), + MagnetPosition(x=5, y=5, z=2), + [ + call.z(2), + call.ramp(), + call.x(5), + call.ramp(), + call.y(5), + call.ramp(), + ], + ), ], ) async def test_scmc_set_decreases_before_increases( @@ -275,3 +287,10 @@ async def test_scmc_set_decreases_before_increases( await scmc.set(target) assert calls == expected_calls + + +async def test_scmc_set_within_boundary_raises_error_if_all_values_none( + scmc: SuperConductingMagnet, +) -> None: + with pytest.raises(RuntimeError): + await scmc.set_within_boundary() From de9bc5c1fbf4e6f659bac852e9581addb997baf9 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 10 Jul 2026 10:49:43 +0000 Subject: [PATCH 17/30] Andd Flyable and Prepare methods and tests --- .../beamlines/i06_1/magnets/__init__.py | 2 + .../i06_1/magnets/superconducting_magnet.py | 42 ++++++++++++++-- .../magnets/test_superconducting_magnet.py | 48 +++++++++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py index aecf53e401b..b33b9d2c7e5 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py @@ -3,6 +3,7 @@ MagnetThreeAxesRampRateController, ) from .superconducting_magnet import ( + FlyMagnetInfo, MagnetAxis, MagnetLimitStatus, MagnetModes, @@ -15,6 +16,7 @@ __all__ = [ "MagnetAxisRampRateController", "MagnetThreeAxesRampRateController", + "FlyMagnetInfo", "MagnetAxis", "MagnetLimitStatus", "MagnetModes", diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 5e4b6652ab1..122c5602881 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from functools import cached_property -from bluesky.protocols import Movable +from bluesky.protocols import Flyable, Movable, Preparable from ophyd_async.core import ( AsyncStatus, DeviceMock, @@ -14,14 +14,17 @@ StandardReadable, StandardReadableFormat, StrictEnum, + WatchableAsyncStatus, callback_on_mock_put, default_mock_class, derived_signal_rw, + error_if_none, set_mock_value, soft_signal_rw, wait_for_value, ) from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x +from pydantic import BaseModel, Field from dodal.devices.beamlines.i06_1.magnets.ramp_controller import ( MagnetAxisRampRateController, @@ -94,6 +97,12 @@ def to_cartesian(self) -> MagnetPosition: ) +class FlyMagnetInfo(BaseModel): + start_position: float = Field(frozen=True) + end_position: float = Field(frozen=True) + ramp_rate: float = Field(frozen=True, gt=0) + + @dataclass class MagnetAxisMovableLogic(MovableLogic[float]): axis: str @@ -107,17 +116,17 @@ async def move( await self.magnet_set_within_boundary(**values) -class MagnetAxis(StandardReadable, StandardMovable[float]): +class MagnetAxis(StandardReadable, StandardMovable[float], Flyable, Preparable): def __init__( self, prefix: str, axis: str, - ramp_controller: MagnetAxisRampRateController, + ramp_rate: MagnetAxisRampRateController, magnet_set_within_boundary: Callable[[], Awaitable[None]], name: str = "", ): # Used in fastfieldscan, need to add fly scan logic. - self.ramp_controller_ref = Reference(ramp_controller) + self.ramp_rate_ref = Reference(ramp_rate) with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): self.readback = epics_signal_r(float, prefix + "RBV") @@ -129,12 +138,37 @@ def __init__( axis=axis, magnet_set_within_boundary=magnet_set_within_boundary, ) + + self._fly_info: FlyMagnetInfo | None = None + self._fly_status: WatchableAsyncStatus | None = None super().__init__(name) @cached_property def movable_logic(self) -> MagnetAxisMovableLogic: return self._movable_logic + @AsyncStatus.wrap + async def prepare(self, value: FlyMagnetInfo): + self._fly_info = value + await self.ramp_rate_ref().set(value.ramp_rate) + await self.set(value.start_position) + + @AsyncStatus.wrap + async def kickoff(self): + fly_info = error_if_none( + self._fly_info, + f"{self.name} must be prepared before attempting to kickoff.", + ) + self._fly_status = self.set(fly_info.end_position) + # Reset state so prepare must be called with each kickoff again. + self._fly_info = None + + def complete(self) -> WatchableAsyncStatus: + """Return a ``Status`` and mark it done when acquisition has completed.""" + fly_status = error_if_none(self._fly_status, f"{self.name} kickoff not called.") + self._fly_status = None + return fly_status + class MockSuperConductingMagnet(DeviceMock["SuperConductingMagnet"]): async def connect(self, device: "SuperConductingMagnet"): diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 792ca9c6229..1ccb48d993c 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -6,6 +6,9 @@ from ophyd_async.testing import assert_reading, partial_reading from dodal.devices.beamlines.i06_1.magnets import ( + FlyMagnetInfo, + MagnetAxis, + MagnetAxisRampRateController, MagnetLimitStatus, MagnetPosition, MagnetSphericalPosition, @@ -294,3 +297,48 @@ async def test_scmc_set_within_boundary_raises_error_if_all_values_none( ) -> None: with pytest.raises(RuntimeError): await scmc.set_within_boundary() + + +@pytest.mark.parametrize("axis", ["x", "y", "z"]) +async def test_scmc_axis_with_ramp_rate_wired_correctly_with_prepare( + scmc: SuperConductingMagnet, + ramp_rate: MagnetThreeAxesRampRateController, + axis: str, +) -> None: + magnet_axis: MagnetAxis = getattr(scmc, axis) + ramp_axis: MagnetAxisRampRateController = getattr(ramp_rate, axis) + + fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=4) + await magnet_axis.prepare(fly_info) + assert await magnet_axis.demand.get_value() == fly_info.start_position + assert await ramp_axis.demand.get_value() == fly_info.ramp_rate + + +async def test_scmc_axis_kickoff_and_complete(scmc: SuperConductingMagnet) -> None: + fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=4) + assert scmc.x._fly_info is None + await scmc.x.prepare(fly_info) + assert scmc.x._fly_info is fly_info + assert scmc.x._fly_status is None + await scmc.x.kickoff() + assert scmc.x._fly_info is None + await scmc.x.complete() + assert await scmc.x.readback.get_value() == fly_info.end_position + + +async def test_scmc_axis_kickoff_and_complete_raises_error_without_prepare( + scmc: SuperConductingMagnet, +): + # Do multiple times to make sure you cannot kickoff or prepare without preparing + # each time + for _ in range(3): + with pytest.raises(RuntimeError): + await scmc.x.kickoff() + + with pytest.raises(RuntimeError): + await scmc.x.complete() + + fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=4) + await scmc.x.prepare(fly_info) + await scmc.x.kickoff() + await scmc.x.complete() From 1c359d5907c2db39d99ca50e921dde6bae479c77 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Fri, 10 Jul 2026 14:57:00 +0000 Subject: [PATCH 18/30] Add doc strings to classes --- .../i06_1/magnets/ramp_controller.py | 14 ++++++++++- .../i06_1/magnets/superconducting_magnet.py | 23 +++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py index d0286f17839..4cdd4cfccdc 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py @@ -15,8 +15,13 @@ async def move(self, new_position: float, timeout: TimeoutCalculator): await self.setpoint.set(new_position, timeout=timeout()) -# Equivalent to GDA SuperconductingMagnetControllerClass class MagnetAxisRampRateController(StandardMovable[float], StandardReadable): + """Controls the ramp rate of a single superconducting magnet axis. + + Exposes the readback ramp rate, demand ramp rate and the maximum + permitted ramp rate for one magnet axis. + """ + def __init__(self, prefix: str, name: str = ""): with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): self.readback = epics_signal_rw(float, prefix + "STS:RAMPRATE:TPM") @@ -30,6 +35,13 @@ def movable_logic(self) -> RampRateMovableLogic: class MagnetThreeAxesRampRateController(StandardReadable): + """Groups the ramp rate controllers for the x, y and z magnet axes. + + This device is passed to :class:`SuperConductingMagnet` so that each + :class:`MagnetAxis` can configure its own ramp rate during preparation for + fly scans. + """ + def __init__(self, prefix: str, name: str = ""): with self.add_children_as_readables(): self.x = MagnetAxisRampRateController(prefix + "-01:") diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 122c5602881..9070d9d4336 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -117,6 +117,16 @@ async def move( class MagnetAxis(StandardReadable, StandardMovable[float], Flyable, Preparable): + """Represents one cartesian axis of a superconducting vector magnet. + + Although each axis can be moved independently, all moves are delegated to + the parent :class:`SuperConductingMagnet` so that coordinated motion can + enforce safe field transitions before updating the underlying demand PVs. + + Each axis is associated with a :class:`MagnetAxisRampRateController`, which is used + to configure the axis ramp rate during preparation for fly scans. + """ + def __init__( self, prefix: str, @@ -125,7 +135,6 @@ def __init__( magnet_set_within_boundary: Callable[[], Awaitable[None]], name: str = "", ): - # Used in fastfieldscan, need to add fly scan logic. self.ramp_rate_ref = Reference(ramp_rate) with self.add_children_as_readables(StandardReadableFormat.HINTED_SIGNAL): @@ -160,7 +169,6 @@ async def kickoff(self): f"{self.name} must be prepared before attempting to kickoff.", ) self._fly_status = self.set(fly_info.end_position) - # Reset state so prepare must be called with each kickoff again. self._fly_info = None def complete(self) -> WatchableAsyncStatus: @@ -183,6 +191,17 @@ def _set_ramp_status(value): @default_mock_class(MockSuperConductingMagnet) class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): + """A three-axis superconducting vector magnet. + + The magnet exposes three independent cartesian axes (``x``, ``y`` and ``z``) + together with derived spherical coordinates (``rho``, ``theta`` and ``phi``). + Positions may be set using either cartesian or spherical coordinates. + + To minimise the risk of quenching the magnet, moves are performed by first + applying any decreases in field magnitude (in Z, X, Y order) before applying + all increases together. + """ + def __init__( self, prefix: str, From e52627032dea70ad77e56eb300f272f06ff4e59a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 14 Jul 2026 14:02:57 +0000 Subject: [PATCH 19/30] Refactor magnets to have moment strategies, add better mock behaviour --- .../beamlines/i06_1/magnets/__init__.py | 8 +- .../devices/beamlines/i06_1/magnets/enums.py | 22 + .../beamlines/i06_1/magnets/movement.py | 187 +++++++++ .../i06_1/magnets/superconducting_magnet.py | 337 ++++++++------- .../beamlines/i06_1/magnets/test_movements.py | 243 +++++++++++ .../magnets/test_superconducting_magnet.py | 397 +++++++++++------- .../devices/beamlines/i06_1/magnets/utils.py | 11 + 7 files changed, 891 insertions(+), 314 deletions(-) create mode 100644 src/dodal/devices/beamlines/i06_1/magnets/enums.py create mode 100644 src/dodal/devices/beamlines/i06_1/magnets/movement.py create mode 100644 tests/devices/beamlines/i06_1/magnets/test_movements.py create mode 100644 tests/devices/beamlines/i06_1/magnets/utils.py diff --git a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py index b33b9d2c7e5..9b2559bc287 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py @@ -1,3 +1,4 @@ +from .movement import MagnetPosition, MagnetPositionError, MagnetSphericalPosition from .ramp_controller import ( MagnetAxisRampRateController, MagnetThreeAxesRampRateController, @@ -7,21 +8,20 @@ MagnetAxis, MagnetLimitStatus, MagnetModes, - MagnetPosition, MagnetRampStatus, - MagnetSphericalPosition, SuperConductingMagnet, ) __all__ = [ + "MagnetPosition", + "MagnetPositionError", + "MagnetSphericalPosition", "MagnetAxisRampRateController", "MagnetThreeAxesRampRateController", "FlyMagnetInfo", "MagnetAxis", "MagnetLimitStatus", "MagnetModes", - "MagnetPosition", "MagnetRampStatus", - "MagnetSphericalPosition", "SuperConductingMagnet", ] diff --git a/src/dodal/devices/beamlines/i06_1/magnets/enums.py b/src/dodal/devices/beamlines/i06_1/magnets/enums.py new file mode 100644 index 00000000000..0de08351f74 --- /dev/null +++ b/src/dodal/devices/beamlines/i06_1/magnets/enums.py @@ -0,0 +1,22 @@ +from ophyd_async.core import StrictEnum + + +class MagnetModes(StrictEnum): + UNIAXIAL_X = "UNIAXIAL_X" + UNIAXIAL_Y = "UNIAXIAL_Y" + UNIAXIAL_Z = "UNIAXIAL_Z" + SPHERICAL = "SPHERICAL" + PLANAR_XZ = "PLANAR_XZ" + QUADRANT_XY = "QUADRANT_XY" + CUBIC = "CUBIC" + UNDEFINED = "UNDEFINED" + + +class MagnetRampStatus(StrictEnum): + RAMP_MADE = "RAMP MADE" + RAMPING = "RAMPING" + + +class MagnetLimitStatus(StrictEnum): + OK = "OK" + VIOLTATION = "VIOLATION" diff --git a/src/dodal/devices/beamlines/i06_1/magnets/movement.py b/src/dodal/devices/beamlines/i06_1/magnets/movement.py new file mode 100644 index 00000000000..99a97e09040 --- /dev/null +++ b/src/dodal/devices/beamlines/i06_1/magnets/movement.py @@ -0,0 +1,187 @@ +import math +from abc import ABC, abstractmethod +from dataclasses import dataclass + + +def read_theta(x: float, z: float) -> float: + theta = math.degrees(math.atan2(-x, z)) + return theta % 360 + + +def read_rho(x: float, y: float, z: float) -> float: + return math.hypot(x, y, z) + + +def read_phi(x: float, y: float, z: float) -> float: + return math.degrees(math.atan2(math.hypot(x, z), y)) + + +@dataclass(kw_only=True) +class MagnetPosition: + x: float + y: float + z: float + + def to_spherical(self) -> "MagnetSphericalPosition": + return MagnetSphericalPosition( + rho=read_rho(self.x, self.y, self.z), + theta=read_theta(self.x, self.z), + phi=read_phi(self.x, self.y, self.z), + ) + + +@dataclass(kw_only=True) +class MagnetSphericalPosition: + rho: float + theta: float # degrees + phi: float # degrees + + def to_cartesian(self) -> MagnetPosition: + theta = math.radians(self.theta) + phi = math.radians(self.phi) + + return MagnetPosition( + x=-self.rho * math.sin(phi) * math.sin(theta), + y=self.rho * math.cos(phi), + z=self.rho * math.sin(phi) * math.cos(theta), + ) + + +@dataclass(frozen=True, kw_only=True) +class MagnetStep: + x: float | None = None + y: float | None = None + z: float | None = None + + +class MagnetPositionError(Exception): + pass + + +class MovementStrategy(ABC): + @abstractmethod + def moves( + self, current: MagnetPosition, target: MagnetPosition + ) -> list[MagnetStep]: + """Return a sequence of safe moves.""" + + +class SphericalMovement(MovementStrategy): + def moves( + self, current: MagnetPosition, target: MagnetPosition + ) -> list[MagnetStep]: + steps: list[MagnetStep] = [] + + decreases = [ + ("z", current.z, target.z), + ("x", current.x, target.x), + ("y", current.y, target.y), + ] + + for axis, old, new in decreases: + if abs(new) < abs(old): + kwargs = {axis: new} + steps.append(MagnetStep(**kwargs)) + + if any(abs(new) > abs(old) for _, old, new in decreases): + steps.append(MagnetStep(x=target.x, y=target.y, z=target.z)) + + return steps + + +class CubicMovement(MovementStrategy): + LIMIT = 1.5 + + def moves( + self, current: MagnetPosition, target: MagnetPosition + ) -> list[MagnetStep]: + for axis in (target.x, target.y, target.z): + if abs(axis) > self.LIMIT: + raise MagnetPositionError( + f"Target {target} outside cubic operating region limit of {self.LIMIT}" + ) + + return [MagnetStep(x=target.x, y=target.y, z=target.z)] + + +class PlanarXZMovement(MovementStrategy): + LIMIT = 2.0 + + def moves( + self, current: MagnetPosition, target: MagnetPosition + ) -> list[MagnetStep]: + if target.y != 0: + raise MagnetPositionError( + f"Y must remain zero in XZ mode. Requested value was {target.y}" + ) + + hypot = math.hypot(target.x, target.z) + if hypot > self.LIMIT: + raise MagnetPositionError( + f"Outside XZ operating region. math.hypot(x={target.x}, y={target.y}) = {hypot}. Limit is {self.LIMIT}" + ) + + return [MagnetStep(x=target.x, z=target.z)] + + +@dataclass +class UniaxialMovement(MovementStrategy): + axis: str + limit: float + + def moves( + self, current: MagnetPosition, target: MagnetPosition + ) -> list[MagnetStep]: + coords = {"x": target.x, "y": target.y, "z": target.z} + for axis, value in coords.items(): + if axis == self.axis: + abs_value = abs(value) + if abs_value > self.limit: + raise MagnetPositionError( + f"{axis} value of {abs_value} exceeds limit of {self.limit}" + ) + elif value != 0: + raise MagnetPositionError( + f"{axis} must remain zero in {self.axis.upper()} mode. Requested value was {value} " + ) + + return [MagnetStep(**{self.axis: coords[self.axis]})] + + +class QuadrantXYMovement(MovementStrategy): + """Start from zero field in all axes. Appy 2.o T in + y direction. Move field + direction through positive quadrant to 2.0 T in the +x direction. At all times + | B | <= 2.0 T. Ramp field back to zero in x direction. + """ + + LIMIT = 2.0 + + def moves( + self, current: MagnetPosition, target: MagnetPosition + ) -> list[MagnetStep]: + if target.z != 0: + raise MagnetPositionError( + f"Z must remain zero in QUADRANT_XY mode. You provided {target.z}" + ) + + hypot = math.hypot(target.x, target.y) + if hypot > self.LIMIT: + raise MagnetPositionError( + f"Target math.hypot(x={target.x}, y={target.y}) {hypot} is outside the XY operating region limit of {self.LIMIT}." + ) + + steps: list[MagnetStep] = [] + + # Step 1: remove X component first + if current.x != 0: + steps.append(MagnetStep(x=0)) + + # Step 2: move Y + if current.y != target.y: + steps.append(MagnetStep(y=target.y)) + + # Step 3: move X to its final value + if target.x != 0: + steps.append(MagnetStep(x=target.x)) + + return steps diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 9070d9d4336..ab555fa59c4 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -1,8 +1,8 @@ import asyncio -import math -from collections.abc import Awaitable, Callable +from collections.abc import Callable from dataclasses import dataclass from functools import cached_property +from typing import Protocol from bluesky.protocols import Flyable, Movable, Preparable from ophyd_async.core import ( @@ -13,88 +13,45 @@ StandardMovable, StandardReadable, StandardReadableFormat, - StrictEnum, WatchableAsyncStatus, callback_on_mock_put, default_mock_class, derived_signal_rw, error_if_none, set_mock_value, - soft_signal_rw, wait_for_value, ) from ophyd_async.epics.core import epics_signal_r, epics_signal_rw, epics_signal_x from pydantic import BaseModel, Field +from dodal.devices.beamlines.i06_1.magnets.enums import ( + MagnetLimitStatus, + MagnetModes, + MagnetRampStatus, +) +from dodal.devices.beamlines.i06_1.magnets.movement import ( + CubicMovement, + MagnetPosition, + MagnetPositionError, + MagnetSphericalPosition, + MagnetStep, + MovementStrategy, + PlanarXZMovement, + QuadrantXYMovement, + SphericalMovement, + UniaxialMovement, + read_phi, + read_rho, + read_theta, +) from dodal.devices.beamlines.i06_1.magnets.ramp_controller import ( MagnetAxisRampRateController, MagnetThreeAxesRampRateController, ) -class MagnetModes(StrictEnum): - UNIAXIAL_X = "UNIAXIAL_X" - UNIAXIAL_Y = "UNIAXIAL_Y" - UNIAXIAL_Z = "UNIAXIAL_Z" - SPHERICAL = "SPHERICAL" - PLANAR_XZ = "PLANAR_XZ" - QUADRANT_XY = "QUADRANT_XY" - CUBIC = "CUBIC" - UNDEFINED = "UNDEFINED" - - -class MagnetRampStatus(StrictEnum): - RAMPING = "RAMPING" - RAMP_MADE = "RAMP MADE" - - -class MagnetLimitStatus(StrictEnum): - OK = "OK" - VIOLTATION = "VIOLATION" - - -def read_theta(x: float, z: float) -> float: - theta = math.degrees(math.atan2(-x, z)) - return theta % 360 - - -def read_rho(x: float, y: float, z: float) -> float: - return math.hypot(x, y, z) - - -def read_phi(x: float, y: float, z: float) -> float: - return math.degrees(math.atan2(math.hypot(x, z), y)) - - -@dataclass(kw_only=True) -class MagnetPosition: - x: float - y: float - z: float - - def to_spherical(self) -> "MagnetSphericalPosition": - return MagnetSphericalPosition( - rho=read_rho(self.x, self.y, self.z), - theta=read_theta(self.x, self.z), - phi=read_phi(self.x, self.y, self.z), - ) - - -@dataclass(kw_only=True) -class MagnetSphericalPosition: - rho: float - theta: float # degrees - phi: float # degrees - - def to_cartesian(self) -> MagnetPosition: - theta = math.radians(self.theta) - phi = math.radians(self.phi) - - return MagnetPosition( - x=-self.rho * math.sin(phi) * math.sin(theta), - y=self.rho * math.cos(phi), - z=self.rho * math.sin(phi) * math.cos(theta), - ) +class MagnetMoveWithinBoundary(Protocol): + async def __call__(self, x: float | None, y: float | None, z: float | None): ... class FlyMagnetInfo(BaseModel): @@ -106,7 +63,7 @@ class FlyMagnetInfo(BaseModel): @dataclass class MagnetAxisMovableLogic(MovableLogic[float]): axis: str - magnet_set_within_boundary: Callable[[], Awaitable[None]] + magnet_set_within_boundary: MagnetMoveWithinBoundary async def move( self, new_position: float, timeout: Callable[[], float | None] @@ -132,7 +89,7 @@ def __init__( prefix: str, axis: str, ramp_rate: MagnetAxisRampRateController, - magnet_set_within_boundary: Callable[[], Awaitable[None]], + magnet_set_within_boundary: MagnetMoveWithinBoundary, name: str = "", ): self.ramp_rate_ref = Reference(ramp_rate) @@ -178,74 +135,62 @@ def complete(self) -> WatchableAsyncStatus: return fly_status -class MockSuperConductingMagnet(DeviceMock["SuperConductingMagnet"]): - async def connect(self, device: "SuperConductingMagnet"): - - def _set_ramp_status(value): - set_mock_value(device.ramp_status, MagnetRampStatus.RAMPING) - set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) - - callback_on_mock_put(device.start_ramp, _set_ramp_status) - set_mock_value(device.limit_status, MagnetLimitStatus.OK) - - -@default_mock_class(MockSuperConductingMagnet) -class SuperConductingMagnet(StandardReadable, Movable[MagnetPosition]): - """A three-axis superconducting vector magnet. - - The magnet exposes three independent cartesian axes (``x``, ``y`` and ``z``) - together with derived spherical coordinates (``rho``, ``theta`` and ``phi``). - Positions may be set using either cartesian or spherical coordinates. - - To minimise the risk of quenching the magnet, moves are performed by first - applying any decreases in field magnitude (in Z, X, Y order) before applying - all increases together. - """ - +class MagnetCartesianCoorindates(StandardReadable, Movable[MagnetPosition]): def __init__( self, prefix: str, - ramp_controllers: MagnetThreeAxesRampRateController, + ramp_rate: MagnetThreeAxesRampRateController, + set_mag_within_boundary: MagnetMoveWithinBoundary, name: str = "", - ): + ) -> None: + self._set_mag_within_boundary = set_mag_within_boundary with self.add_children_as_readables(): - # Cartesian and real pv values self.x = MagnetAxis( - prefix + "X:", "x", ramp_controllers.x, self.set_within_boundary + prefix + "X:", "x", ramp_rate.x, set_mag_within_boundary ) self.y = MagnetAxis( - prefix + "Y:", "y", ramp_controllers.y, self.set_within_boundary + prefix + "Y:", "y", ramp_rate.y, set_mag_within_boundary ) self.z = MagnetAxis( - prefix + "Z:", "z", ramp_controllers.z, self.set_within_boundary + prefix + "Z:", "z", ramp_rate.z, set_mag_within_boundary ) - # Spherical representations of x, y, z + super().__init__(name) + + @AsyncStatus.wrap + async def set(self, value: MagnetPosition): + await self._set_mag_within_boundary(x=value.x, y=value.y, z=value.z) + + +class MagnetSphericalCoordinates(StandardReadable, Movable[MagnetSphericalPosition]): + def __init__( + self, + cart: MagnetCartesianCoorindates, + set_mag_within_boundary: MagnetMoveWithinBoundary, + name: str = "", + ): + with self.add_children_as_readables(): self.theta = derived_signal_rw( - read_theta, self._set_theta, x=self.x, z=self.z + read_theta, self._set_theta, x=cart.x, z=cart.z ) self.rho = derived_signal_rw( - read_rho, self._set_rho, x=self.x, y=self.y, z=self.z + read_rho, self._set_rho, x=cart.x, y=cart.y, z=cart.z ) self.phi = derived_signal_rw( read_phi, self._set_phi, - x=self.x, - y=self.y, - z=self.z, + x=cart.x, + y=cart.y, + z=cart.z, ) - - with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): - self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") - self.delay = soft_signal_rw(float, initial_value=0.1) - - self.ramp_status = epics_signal_rw(MagnetRampStatus, prefix + "RAMPSTATUS") - - self.limit_status = epics_signal_rw(MagnetLimitStatus, prefix + "LIMITSTATUS") - self.start_ramp = epics_signal_x(prefix + "STARTRAMP.PROC") - self.timeout = 600 - + self._set_mag_within_boundary = set_mag_within_boundary + self._cart_ref = Reference(cart) super().__init__(name) + @AsyncStatus.wrap + async def set(self, value: MagnetSphericalPosition): + cart = value.to_cartesian() + await self._set_mag_within_boundary(x=cart.x, y=cart.y, z=cart.z) + async def _set_spherical( self, rho: float | None = None, @@ -253,9 +198,9 @@ async def _set_spherical( phi: float | None = None, ): x, y, z = await asyncio.gather( - self.x.readback.get_value(), - self.y.readback.get_value(), - self.z.readback.get_value(), + self._cart_ref().x.readback.get_value(), + self._cart_ref().y.readback.get_value(), + self._cart_ref().z.readback.get_value(), ) current = MagnetPosition(x=x, y=y, z=z).to_spherical() spherical_pos = MagnetSphericalPosition( @@ -274,64 +219,118 @@ async def _set_theta(self, theta: float): async def _set_phi(self, phi: float): await self._set_spherical(phi=phi) - @AsyncStatus.wrap - async def set(self, value: MagnetPosition | MagnetSphericalPosition): - if isinstance(value, MagnetSphericalPosition): - value = value.to_cartesian() - await self.set_within_boundary(x=value.x, y=value.y, z=value.z) + +MODE_MOVEMENT_STRATEGY: dict[MagnetModes, MovementStrategy] = { + MagnetModes.SPHERICAL: SphericalMovement(), + MagnetModes.CUBIC: CubicMovement(), + MagnetModes.PLANAR_XZ: PlanarXZMovement(), + MagnetModes.QUADRANT_XY: QuadrantXYMovement(), + MagnetModes.UNIAXIAL_X: UniaxialMovement("x", limit=2.0), + MagnetModes.UNIAXIAL_Y: UniaxialMovement("y", limit=2.0), + MagnetModes.UNIAXIAL_Z: UniaxialMovement("z", limit=6.0), +} + + +class MockSuperConductingMagnet(DeviceMock["SuperConductingMagnet"]): + async def connect(self, device: "SuperConductingMagnet"): + + def _set_ramp_status(value): + set_mock_value(device.ramp_status, MagnetRampStatus.RAMPING) + set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) + + def _set_mode(value): + # Whenever mode is changed, ioc automatically sets everything to zero and ramps + set_mock_value(device.cart.x.demand, 0.0) + set_mock_value(device.cart.y.demand, 0.0) + set_mock_value(device.cart.z.demand, 0.0) + set_mock_value(device.ramp_status, MagnetRampStatus.RAMPING) + set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) + + callback_on_mock_put(device.mode, _set_mode) + callback_on_mock_put(device.start_ramp, _set_ramp_status) + set_mock_value(device.limit_status, MagnetLimitStatus.OK) + set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) + + +@default_mock_class(MockSuperConductingMagnet) +class SuperConductingMagnet(StandardReadable): + """A three-axis superconducting vector magnet. + + The magnet exposes three independent cartesian axes (``x``, ``y`` and ``z``) + together with derived spherical coordinates (``rho``, ``theta`` and ``phi``). + Positions may be set using either cartesian or spherical coordinates. + + To minimise the risk of quenching the magnet, moves are performed by first + applying any decreases in field magnitude (in Z, X, Y order) before applying + all increases together. + """ + + def __init__( + self, + prefix: str, + ramp_controllers: MagnetThreeAxesRampRateController, + name: str = "", + ): + with self.add_children_as_readables(): + self.cart = MagnetCartesianCoorindates( + prefix, ramp_controllers, self.set_within_boundary + ) + self.sph = MagnetSphericalCoordinates(self.cart, self.set_within_boundary) + + with self.add_children_as_readables(StandardReadableFormat.CONFIG_SIGNAL): + self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") + + self.ramp_status = epics_signal_rw(MagnetRampStatus, prefix + "RAMPSTATUS") + + self.limit_status = epics_signal_rw(MagnetLimitStatus, prefix + "LIMITSTATUS") + self.start_ramp = epics_signal_x(prefix + "STARTRAMP.PROC") + self.timeout = 600 + + super().__init__(name) async def _ramp(self): + # Setting invalid demand values should put the limit status in violation state. + # Block ramp if in violation state. + if await self.limit_status.get_value() == MagnetLimitStatus.VIOLTATION: + raise MagnetPositionError( + f"{self.limit_status.name} is at {MagnetLimitStatus.VIOLTATION}" + ) # ToDo - Use TimeoutCalculated from ophyd-async in new release. await self.start_ramp.trigger(timeout=self.timeout) await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, self.timeout) - if await self.limit_status.get_value() == MagnetLimitStatus.VIOLTATION: - raise RuntimeError( - f"{self.limit_status.name} is at position {MagnetLimitStatus.VIOLTATION}" - ) + async def _apply_step(self, step: MagnetStep) -> None: + tasks = [] + if step.x is not None: + tasks.append(self.cart.x.demand.set(step.x)) + if step.y is not None: + tasks.append(self.cart.y.demand.set(step.y)) + if step.z is not None: + tasks.append(self.cart.z.demand.set(step.z)) + + await asyncio.gather(*tasks) + await self._ramp() async def set_within_boundary( - self, x: float | None = None, y: float | None = None, z: float | None = None + self, + x: float | None = None, + y: float | None = None, + z: float | None = None, ): - """Move magnet while ensuring field magnitude never increases before decreases - have completed. - """ if x is None and y is None and z is None: - raise RuntimeError("Args x, y, and z cannot all be None at the same time.") - - # For keeping the magnitude constrained to avoid quench, always do the - # decreasing before increasing motors - # Can this be simplified? - x0, y0, z0 = await asyncio.gather( - self.x.readback.get_value(), - self.y.readback.get_value(), - self.z.readback.get_value(), + raise MagnetPositionError("x, y, and z cannot all be None.") + current = MagnetPosition( + x=await self.cart.x.readback.get_value(), + y=await self.cart.y.readback.get_value(), + z=await self.cart.z.readback.get_value(), ) - x = x0 if x is None else x - y = y0 if y is None else y - z = z0 if z is None else z - - dx = abs(x) - abs(x0) - dy = abs(y) - abs(y0) - dz = abs(z) - abs(z0) - - # Decrease Z first - if dz < 0: - await self.z.demand.set(z) - await self._ramp() - # Then decrease X - if dx < 0: - await self.x.demand.set(x) - await self._ramp() - # Then decrease Y - if dy < 0: - await self.y.demand.set(y) - await self._ramp() - # Finally perform all increases together - if dx > 0 or dy > 0 or dz > 0: - await asyncio.gather( - self.x.demand.set(x), - self.y.demand.set(y), - self.z.demand.set(z), - ) - await self._ramp() + target = MagnetPosition( + x=current.x if x is None else x, + y=current.y if y is None else y, + z=current.z if z is None else z, + ) + mode = await self.mode.get_value() + movement_strategy = MODE_MOVEMENT_STRATEGY[mode] + + for step in movement_strategy.moves(current, target): + await self._apply_step(step) diff --git a/tests/devices/beamlines/i06_1/magnets/test_movements.py b/tests/devices/beamlines/i06_1/magnets/test_movements.py new file mode 100644 index 00000000000..c94c57dad9b --- /dev/null +++ b/tests/devices/beamlines/i06_1/magnets/test_movements.py @@ -0,0 +1,243 @@ +import pytest + +from dodal.devices.beamlines.i06_1.magnets.movement import ( + CubicMovement, + MagnetPosition, + MagnetPositionError, + MagnetSphericalPosition, + MagnetStep, + PlanarXZMovement, + QuadrantXYMovement, + SphericalMovement, + UniaxialMovement, +) +from tests.devices.beamlines.i06_1.magnets.utils import ( + EXPECTED_CARTESIAN_SPHERICAL_CONVERSION, +) + + +def test_cartesian_and_spherical_are_inverse() -> None: + cartesian = MagnetPosition(x=10, y=20, z=30) + result = cartesian.to_spherical().to_cartesian() + assert result.x == pytest.approx(cartesian.x) + assert result.y == pytest.approx(cartesian.y) + assert result.z == pytest.approx(cartesian.z) + + +def test_spherical_and_cartesian_are_inverse() -> None: + spherical = MagnetSphericalPosition(rho=10, theta=20, phi=30) + result = spherical.to_cartesian().to_spherical() + assert result.rho == pytest.approx(spherical.rho) + assert result.theta == pytest.approx(spherical.theta) + assert result.phi == pytest.approx(spherical.phi) + + +@pytest.mark.parametrize( + "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION +) +def test_cartesian_and_spherical_conversion_is_correct( + cartesian: MagnetPosition, + spherical: MagnetSphericalPosition, +) -> None: + result = cartesian.to_spherical() + assert result.rho == pytest.approx(spherical.rho) + assert result.theta == pytest.approx(spherical.theta) + assert result.phi == pytest.approx(spherical.phi) + + result = spherical.to_cartesian() + assert result.x == pytest.approx(cartesian.x) + assert result.y == pytest.approx(cartesian.y) + assert result.z == pytest.approx(cartesian.z) + + +# def test_spherical_movement_decreases_z_then_x_then_y(): +# move_stragegy = SphericalMovement() + +# steps = move_stragegy.moves( +# current=MagnetPosition(x=10, y=10, z=10), +# target=MagnetPosition(x=5, y=5, z=2), +# ) +# assert steps == [MagnetStep(z=2), MagnetStep(x=5), MagnetStep(y=5)] + + +# def test_spherical_movement_adds_final_combined_increase(): +# move_stragegy = SphericalMovement() + +# steps = move_stragegy.moves( +# current=MagnetPosition(x=10, y=10, z=10), +# target=MagnetPosition(x=20, y=5, z=2), +# ) +# assert steps == [MagnetStep(z=2), MagnetStep(y=5), MagnetStep(x=20, y=5, z=2)] + + +# @pytest.mark.parametrize( +# "target", [MagnetPosition(x=0, y=1, z=0), MagnetPosition(x=0, y=-1, z=0)] +# ) +# def test_planar_xz_rejects_y(target): +# move_stragegy = PlanarXZMovement() + +# with pytest.raises(ValueError): +# move_stragegy.moves(current=MagnetPosition(x=0, y=0, z=0), target=target) + + +@pytest.mark.parametrize( + "current, target, expected", + [ + ( + MagnetPosition(x=3, y=3, z=3), + MagnetPosition(x=1, y=1, z=1), + [ + MagnetStep(z=1), + MagnetStep(x=1), + MagnetStep(y=1), + ], + ), + ( + MagnetPosition(x=3, y=3, z=3), + MagnetPosition(x=4, y=2, z=1), + [ + MagnetStep(z=1), + MagnetStep(y=2), + MagnetStep(x=4, y=2, z=1), + ], + ), + ( + MagnetPosition(x=1, y=1, z=1), + MagnetPosition(x=2, y=2, z=2), + [ + MagnetStep(x=2, y=2, z=2), + ], + ), + ( + MagnetPosition(x=1, y=2, z=3), + MagnetPosition(x=1, y=2, z=3), + [], + ), + ], +) +def test_spherical_movement( + current: MagnetPosition, + target: MagnetPosition, + expected: list[MagnetStep], +): + assert SphericalMovement().moves(current, target) == expected + + +@pytest.mark.parametrize( + "target", + [ + MagnetPosition(x=1.6, y=0, z=0), + MagnetPosition(x=0, y=-1.6, z=0), + MagnetPosition(x=0, y=0, z=2), + ], +) +def test_cubic_rejects_outside_limits(target: MagnetPosition): + with pytest.raises(MagnetPositionError): + CubicMovement().moves(MagnetPosition(x=0, y=0, z=0), target) + + +def test_cubic_returns_single_step(): + current = MagnetPosition(x=0, y=0, z=0) + target = MagnetPosition(x=1, y=1.2, z=-0.5) + + assert CubicMovement().moves(current, target) == [MagnetStep(x=1, y=1.2, z=-0.5)] + + +def test_planar_xz_returns_single_step(): + target = MagnetPosition(x=1.2, y=0, z=-0.4) + + assert PlanarXZMovement().moves( + MagnetPosition(x=0, y=0, z=0), + target, + ) == [ + MagnetStep(x=1.2, z=-0.4), + ] + + +def test_planar_xz_rejects_nonzero_y(): + with pytest.raises(MagnetPositionError): + PlanarXZMovement().moves( + MagnetPosition(x=0, y=0, z=0), MagnetPosition(x=1, y=1, z=0) + ) + + +def test_planar_xz_rejects_outside_radius(): + with pytest.raises(MagnetPositionError): + PlanarXZMovement().moves( + MagnetPosition(x=0, y=0, z=0), MagnetPosition(x=2, y=0, z=2) + ) + + +@pytest.mark.parametrize( + "axis, limit, target", + [ + ("x", 2, MagnetPosition(x=1, y=0, z=0)), + ("y", 2, MagnetPosition(x=0, y=-1, z=0)), + ("z", 5, MagnetPosition(x=0, y=0, z=3)), + ], +) +def test_uniaxial_returns_single_step(axis, limit, target): + assert UniaxialMovement(axis, limit).moves( + MagnetPosition(x=0, y=0, z=0), target + ) == [MagnetStep(**{axis: getattr(target, axis)})] + + +@pytest.mark.parametrize( + "axis,target", + [ + ("x", MagnetPosition(x=1, y=1, z=0)), + ("x", MagnetPosition(x=1, y=0, z=1)), + ("y", MagnetPosition(x=1, y=1, z=0)), + ("y", MagnetPosition(x=0, y=1, z=1)), + ("z", MagnetPosition(x=1, y=0, z=1)), + ("z", MagnetPosition(x=0, y=1, z=1)), + ], +) +def test_uniaxial_rejects_other_axes(axis, target): + with pytest.raises(MagnetPositionError): + UniaxialMovement(axis, limit=5).moves(MagnetPosition(x=0, y=0, z=0), target) + + +def test_quadrant_xy_sequence(): + + current = MagnetPosition(x=1, y=0.5, z=0) + target = MagnetPosition(x=1.5, y=1, z=0) + strategy = QuadrantXYMovement() + + assert strategy.moves(current, target) == [ + MagnetStep(x=0), + MagnetStep(y=1), + MagnetStep(x=1.5), + ] + + +def test_quadrant_xy_skips_initial_x_zero(): + current = MagnetPosition(x=0, y=0, z=0) + target = MagnetPosition(x=1, y=1, z=0) + strategy = QuadrantXYMovement() + + assert strategy.moves(current, target) == [MagnetStep(y=1), MagnetStep(x=1)] + + +def test_quadrant_xy_skips_y_move(): + target = MagnetPosition(x=1, y=1.5, z=0) + current = MagnetPosition(x=0.5, y=1.5, z=0) + strategy = QuadrantXYMovement() + + assert strategy.moves(target, current) == [MagnetStep(x=0), MagnetStep(x=0.5)] + + +def test_quadrant_xy_rejects_nonzero_z(): + with pytest.raises(MagnetPositionError): + QuadrantXYMovement().moves( + MagnetPosition(x=0, y=0, z=0), + MagnetPosition(x=1, y=1, z=1), + ) + + +def test_quadrant_xy_rejects_outside_radius(): + with pytest.raises(MagnetPositionError): + QuadrantXYMovement().moves( + MagnetPosition(x=0, y=0, z=0), + MagnetPosition(x=2, y=2, z=0), + ) diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 1ccb48d993c..327bc1b8e5c 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -1,27 +1,31 @@ import asyncio -from unittest.mock import AsyncMock, call +from unittest.mock import AsyncMock, MagicMock, call, patch import pytest +from bluesky.protocols import Reading from ophyd_async.core import init_devices, set_mock_value -from ophyd_async.testing import assert_reading, partial_reading +from ophyd_async.testing import assert_configuration, assert_reading, partial_reading from dodal.devices.beamlines.i06_1.magnets import ( FlyMagnetInfo, MagnetAxis, MagnetAxisRampRateController, MagnetLimitStatus, + MagnetModes, MagnetPosition, + MagnetPositionError, + MagnetRampStatus, MagnetSphericalPosition, MagnetThreeAxesRampRateController, SuperConductingMagnet, + movement, +) +from dodal.devices.beamlines.i06_1.magnets.superconducting_magnet import ( + MODE_MOVEMENT_STRATEGY, +) +from tests.devices.beamlines.i06_1.magnets.utils import ( + EXPECTED_CARTESIAN_SPHERICAL_CONVERSION, ) - -EXPECTED_CARTESIAN_SPHERICAL_CONVERSION = [ - (MagnetPosition(x=0, y=0, z=1), MagnetSphericalPosition(rho=1, theta=0, phi=90)), - (MagnetPosition(x=-1, y=0, z=0), MagnetSphericalPosition(rho=1, theta=90, phi=90)), - (MagnetPosition(x=0, y=1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=0)), - (MagnetPosition(x=0, y=-1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=180)), -] @pytest.fixture @@ -42,22 +46,28 @@ async def test_scmc_read(scmc: SuperConductingMagnet) -> None: await assert_reading( scmc, { - "scmc-x": partial_reading(0), - "scmc-y": partial_reading(0), - "scmc-z": partial_reading(0), - "scmc-theta": partial_reading(0), - "scmc-rho": partial_reading(0), - "scmc-phi": partial_reading(0), + "scmc-cart-x": partial_reading(0), + "scmc-cart-y": partial_reading(0), + "scmc-cart-z": partial_reading(0), + "scmc-sph-theta": partial_reading(0), + "scmc-sph-rho": partial_reading(0), + "scmc-sph-phi": partial_reading(0), }, ) +async def test_scmc_configuration(scmc: SuperConductingMagnet) -> None: + await assert_configuration( + scmc, {"scmc-mode": partial_reading(MagnetModes.UNIAXIAL_X)} + ) + + @pytest.mark.parametrize( "axis, value, expected_x, expected_y, expected_z", [ - ("x", 10, 10, 0, 0), - ("y", 20, 0, 20, 0), - ("z", 30, 0, 0, 30), + ("x", 0.1, 0.1, 0, 0), + ("y", 0.5, 0, 0.5, 0), + ("z", 0.2, 0, 0, 0.2), ], ) async def test_scmc_axis_set_uses_correct_axis( @@ -68,11 +78,12 @@ async def test_scmc_axis_set_uses_correct_axis( expected_y: float, expected_z: float, ) -> None: - await getattr(scmc, axis).set(value) + await scmc.mode.set(getattr(MagnetModes, "UNIAXIAL_" + axis.capitalize())) + await getattr(scmc.cart, axis).set(value) x, y, z = await asyncio.gather( - scmc.x.readback.get_value(), - scmc.y.readback.get_value(), - scmc.z.readback.get_value(), + scmc.cart.x.readback.get_value(), + scmc.cart.y.readback.get_value(), + scmc.cart.z.readback.get_value(), ) assert x == expected_x assert y == expected_y @@ -121,20 +132,21 @@ async def test_scmc_set_using_spherical( cartesian: MagnetPosition, spherical: MagnetSphericalPosition, ) -> None: - await scmc.set(spherical) + await scmc.mode.set(MagnetModes.SPHERICAL) + await scmc.sph.set(spherical) rho, theta, phi = await asyncio.gather( - scmc.rho.get_value(), - scmc.theta.get_value(), - scmc.phi.get_value(), + scmc.sph.rho.get_value(), + scmc.sph.theta.get_value(), + scmc.sph.phi.get_value(), ) assert rho == pytest.approx(spherical.rho) assert theta == pytest.approx(spherical.theta) assert phi == pytest.approx(spherical.phi) x, y, z = await asyncio.gather( - scmc.x.readback.get_value(), - scmc.y.readback.get_value(), - scmc.z.readback.get_value(), + scmc.cart.x.readback.get_value(), + scmc.cart.y.readback.get_value(), + scmc.cart.z.readback.get_value(), ) assert x == pytest.approx(cartesian.x) assert y == pytest.approx(cartesian.y) @@ -149,20 +161,21 @@ async def test_scmc_set_using_cartesian( cartesian: MagnetPosition, spherical: MagnetSphericalPosition, ) -> None: - await scmc.set(cartesian) + await scmc.mode.set(MagnetModes.CUBIC) + await scmc.cart.set(cartesian) x, y, z = await asyncio.gather( - scmc.x.readback.get_value(), - scmc.y.readback.get_value(), - scmc.z.readback.get_value(), + scmc.cart.x.readback.get_value(), + scmc.cart.y.readback.get_value(), + scmc.cart.z.readback.get_value(), ) assert x == pytest.approx(cartesian.x) assert y == pytest.approx(cartesian.y) assert z == pytest.approx(cartesian.z) rho, theta, phi = await asyncio.gather( - scmc.rho.get_value(), - scmc.theta.get_value(), - scmc.phi.get_value(), + scmc.sph.rho.get_value(), + scmc.sph.theta.get_value(), + scmc.sph.phi.get_value(), ) assert rho == pytest.approx(spherical.rho) assert theta == pytest.approx(spherical.theta) @@ -183,14 +196,15 @@ async def test_scmc_spherical_axis_set( value: float, expected_spherical: MagnetSphericalPosition, ) -> None: + await scmc.mode.set(MagnetModes.SPHERICAL) # Start from a non-singular position. - await scmc.set(MagnetSphericalPosition(rho=1, theta=20, phi=30)) - await getattr(scmc, axis).set(value) + await scmc.sph.set(MagnetSphericalPosition(rho=1, theta=20, phi=30)) + await getattr(scmc.sph, axis).set(value) rho, theta, phi = await asyncio.gather( - scmc.rho.get_value(), - scmc.theta.get_value(), - scmc.phi.get_value(), + scmc.sph.rho.get_value(), + scmc.sph.theta.get_value(), + scmc.sph.phi.get_value(), ) assert rho == pytest.approx(expected_spherical.rho) assert theta == pytest.approx(expected_spherical.theta) @@ -198,9 +212,9 @@ async def test_scmc_spherical_axis_set( expected_cartesian = expected_spherical.to_cartesian() x, y, z = await asyncio.gather( - scmc.x.readback.get_value(), - scmc.y.readback.get_value(), - scmc.z.readback.get_value(), + scmc.cart.x.readback.get_value(), + scmc.cart.y.readback.get_value(), + scmc.cart.z.readback.get_value(), ) assert x == pytest.approx(expected_cartesian.x) assert y == pytest.approx(expected_cartesian.y) @@ -210,102 +224,112 @@ async def test_scmc_spherical_axis_set( async def test_scmc_raises_error_if_limit_status_is_violation( scmc: SuperConductingMagnet, ) -> None: + await scmc.mode.set(MagnetModes.UNIAXIAL_X) set_mock_value(scmc.limit_status, MagnetLimitStatus.VIOLTATION) with pytest.raises(RuntimeError): - await scmc.x.set(10) - - -@pytest.mark.parametrize( - "initial, target, expected_calls", - [ - ( - MagnetPosition(x=10, y=10, z=10), - MagnetPosition(x=20, y=5, z=2), - [ - call.z(2), - call.ramp(), - call.y(5), - call.ramp(), - call.x(20), - call.y(5), - call.z(2), - call.ramp(), - ], - ), - ( - MagnetPosition(x=10, y=10, z=10), - MagnetPosition(x=5, y=20, z=2), - [ - call.z(2), - call.ramp(), - call.x(5), - call.ramp(), - call.x(5), - call.y(20), - call.z(2), - call.ramp(), - ], - ), - ( - MagnetPosition(x=10, y=10, z=10), - MagnetPosition(x=5, y=2, z=20), - [ - call.x(5), - call.ramp(), - call.y(2), - call.ramp(), - call.x(5), - call.y(2), - call.z(20), - call.ramp(), - ], - ), - ( - MagnetPosition(x=10, y=10, z=10), - MagnetPosition(x=5, y=5, z=2), - [ - call.z(2), - call.ramp(), - call.x(5), - call.ramp(), - call.y(5), - call.ramp(), - ], - ), - ], -) -async def test_scmc_set_decreases_before_increases( - scmc: SuperConductingMagnet, - initial: MagnetPosition, - target: MagnetPosition, - expected_calls, -) -> None: - await scmc.set(initial) - - calls = [] - scmc.x.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.x(v))) - scmc.y.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.y(v))) - scmc.z.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.z(v))) - scmc._ramp = AsyncMock(side_effect=lambda: calls.append(call.ramp())) - - await scmc.set(target) - assert calls == expected_calls + await scmc.cart.x.set(1) + + +# @pytest.mark.parametrize( +# "initial, target, expected_calls", +# [ +# ( +# MagnetPosition(x=10, y=10, z=10), +# MagnetPosition(x=20, y=5, z=2), +# [ +# call.z(2), +# call.ramp(), +# call.y(5), +# call.ramp(), +# call.x(20), +# call.y(5), +# call.z(2), +# call.ramp(), +# ], +# ), +# ( +# MagnetPosition(x=10, y=10, z=10), +# MagnetPosition(x=5, y=20, z=2), +# [ +# call.z(2), +# call.ramp(), +# call.x(5), +# call.ramp(), +# call.x(5), +# call.y(20), +# call.z(2), +# call.ramp(), +# ], +# ), +# ( +# MagnetPosition(x=10, y=10, z=10), +# MagnetPosition(x=5, y=2, z=20), +# [ +# call.x(5), +# call.ramp(), +# call.y(2), +# call.ramp(), +# call.x(5), +# call.y(2), +# call.z(20), +# call.ramp(), +# ], +# ), +# ( +# MagnetPosition(x=10, y=10, z=10), +# MagnetPosition(x=5, y=5, z=2), +# [ +# call.z(2), +# call.ramp(), +# call.x(5), +# call.ramp(), +# call.y(5), +# call.ramp(), +# ], +# ), +# ], +# ) +# async def test_scmc_set_decreases_before_increases( +# scmc: SuperConductingMagnet, +# initial: MagnetPosition, +# target: MagnetPosition, +# expected_calls, +# ) -> None: +# await scmc.set(initial) + +# calls = [] +# scmc.x.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.x(v))) +# scmc.y.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.y(v))) +# scmc.z.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.z(v))) +# scmc._ramp = AsyncMock(side_effect=lambda: calls.append(call.ramp())) + +# await scmc.set(target) +# assert calls == expected_calls async def test_scmc_set_within_boundary_raises_error_if_all_values_none( scmc: SuperConductingMagnet, ) -> None: - with pytest.raises(RuntimeError): + with pytest.raises(MagnetPositionError): await scmc.set_within_boundary() -@pytest.mark.parametrize("axis", ["x", "y", "z"]) +@pytest.mark.parametrize( + "axis, mode", + [ + ("x", MagnetModes.UNIAXIAL_X), + ("y", MagnetModes.UNIAXIAL_Y), + ("z", MagnetModes.UNIAXIAL_Z), + ], +) async def test_scmc_axis_with_ramp_rate_wired_correctly_with_prepare( scmc: SuperConductingMagnet, ramp_rate: MagnetThreeAxesRampRateController, axis: str, + mode: MagnetModes, ) -> None: - magnet_axis: MagnetAxis = getattr(scmc, axis) + await scmc.mode.set(mode) + magnet_axis: MagnetAxis = getattr(scmc.cart, axis) ramp_axis: MagnetAxisRampRateController = getattr(ramp_rate, axis) fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=4) @@ -315,30 +339,121 @@ async def test_scmc_axis_with_ramp_rate_wired_correctly_with_prepare( async def test_scmc_axis_kickoff_and_complete(scmc: SuperConductingMagnet) -> None: - fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=4) - assert scmc.x._fly_info is None - await scmc.x.prepare(fly_info) - assert scmc.x._fly_info is fly_info - assert scmc.x._fly_status is None - await scmc.x.kickoff() - assert scmc.x._fly_info is None - await scmc.x.complete() - assert await scmc.x.readback.get_value() == fly_info.end_position + await scmc.mode.set(MagnetModes.UNIAXIAL_X) + fly_info = FlyMagnetInfo(start_position=1, end_position=2, ramp_rate=4) + assert scmc.cart.x._fly_info is None + await scmc.cart.x.prepare(fly_info) + assert scmc.cart.x._fly_info is fly_info + assert scmc.cart.x._fly_status is None + await scmc.cart.x.kickoff() + assert scmc.cart.x._fly_info is None + await scmc.cart.x.complete() + assert await scmc.cart.x.readback.get_value() == fly_info.end_position async def test_scmc_axis_kickoff_and_complete_raises_error_without_prepare( scmc: SuperConductingMagnet, ): - # Do multiple times to make sure you cannot kickoff or prepare without preparing + await scmc.mode.set(MagnetModes.UNIAXIAL_X) + # Do multiple times to make sure you cannot kickoff or complete without preparing # each time for _ in range(3): with pytest.raises(RuntimeError): - await scmc.x.kickoff() + await scmc.cart.x.kickoff() with pytest.raises(RuntimeError): - await scmc.x.complete() + await scmc.cart.x.complete() + + fly_info = FlyMagnetInfo(start_position=1, end_position=2, ramp_rate=4) + await scmc.cart.x.prepare(fly_info) + await scmc.cart.x.kickoff() + await scmc.cart.x.complete() + + +async def test_scmc_mock_device_behaviour(scmc: SuperConductingMagnet) -> None: + assert await scmc.limit_status.get_value() == MagnetLimitStatus.OK + + set_mock_value(scmc.cart.x.demand, 1) + set_mock_value(scmc.cart.y.demand, 1) + set_mock_value(scmc.cart.z.demand, 1) + + ramp_states: list[MagnetRampStatus] = [] + + def _ramp_state_callback(value: dict[str, Reading[MagnetRampStatus]]): + ramp_states.append(value[scmc.ramp_status.name]["value"]) + + scmc.ramp_status.subscribe_reading(_ramp_state_callback) + + # Set a mode value and check to see if all values are set back to zero and ramp + # status change happened + await scmc.mode.set(MagnetModes.UNIAXIAL_X) + + x_d, y_d, z_d, x_rbv, y_rbv, z_rbv = await asyncio.gather( + scmc.cart.x.demand.get_value(), + scmc.cart.y.demand.get_value(), + scmc.cart.z.demand.get_value(), + scmc.cart.x.readback.get_value(), + scmc.cart.y.readback.get_value(), + scmc.cart.z.readback.get_value(), + ) + assert x_d == y_d == z_d == x_rbv == y_rbv == z_rbv == 0 + assert ramp_states == [ + MagnetRampStatus.RAMP_MADE, + MagnetRampStatus.RAMPING, + MagnetRampStatus.RAMP_MADE, + ] + + +@pytest.mark.parametrize( + "mode,expected", + [ + (MagnetModes.SPHERICAL, movement.SphericalMovement), + (MagnetModes.CUBIC, movement.CubicMovement), + (MagnetModes.PLANAR_XZ, movement.PlanarXZMovement), + (MagnetModes.QUADRANT_XY, movement.QuadrantXYMovement), + (MagnetModes.UNIAXIAL_X, movement.UniaxialMovement), + (MagnetModes.UNIAXIAL_Y, movement.UniaxialMovement), + (MagnetModes.UNIAXIAL_Z, movement.UniaxialMovement), + ], +) +async def test_magnet_mode_to_movement_strategy_configuration( + mode: MagnetModes, expected: type[movement.MovementStrategy] +) -> None: + assert isinstance(MODE_MOVEMENT_STRATEGY[mode], expected) + + +async def test_magnet_mode_to_uniaxial_movement_strategy_configuration(): + assert MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_X].axis == "x" # type:ignore + assert MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_Y].axis == "y" # type:ignore + assert MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_Z].axis == "z" # type:ignore + + +async def test_scmc_executes_movement_stragey_and_ramp_at_each_step( + scmc: SuperConductingMagnet, +) -> None: + + movement_strategy = MagicMock() + + mov_str_return_values = [ + movement.MagnetStep(z=2), + movement.MagnetStep(x=1, y=2, z=3), + ] + expected_apply_step_calls = [ + call(movement.MagnetStep(z=2)), + call(movement.MagnetStep(x=1, y=2, z=3)), + ] + movement_strategy.moves.return_value = mov_str_return_values + + MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_X] = movement_strategy + + scmc._ramp = AsyncMock() + + with patch( + "dodal.devices.beamlines.i06_1.magnets.superconducting_magnet.SuperConductingMagnet._apply_step", + wraps=scmc._apply_step, + ) as mock_apply_step: + await scmc.mode.set(MagnetModes.UNIAXIAL_X) + await scmc.cart.x.set(4) - fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=4) - await scmc.x.prepare(fly_info) - await scmc.x.kickoff() - await scmc.x.complete() + assert mock_apply_step.call_args_list == expected_apply_step_calls + assert scmc._ramp.call_count == len(mov_str_return_values) diff --git a/tests/devices/beamlines/i06_1/magnets/utils.py b/tests/devices/beamlines/i06_1/magnets/utils.py new file mode 100644 index 00000000000..2c9404bae9f --- /dev/null +++ b/tests/devices/beamlines/i06_1/magnets/utils.py @@ -0,0 +1,11 @@ +from dodal.devices.beamlines.i06_1.magnets.movement import ( + MagnetPosition, + MagnetSphericalPosition, +) + +EXPECTED_CARTESIAN_SPHERICAL_CONVERSION = [ + (MagnetPosition(x=0, y=0, z=1), MagnetSphericalPosition(rho=1, theta=0, phi=90)), + (MagnetPosition(x=-1, y=0, z=0), MagnetSphericalPosition(rho=1, theta=90, phi=90)), + (MagnetPosition(x=0, y=1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=0)), + (MagnetPosition(x=0, y=-1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=180)), +] From 7a55ad15e69ea96cbc13240e20e36256672e5437 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 14 Jul 2026 14:07:35 +0000 Subject: [PATCH 20/30] Remove commented out code --- .../beamlines/i06_1/magnets/test_movements.py | 38 ++--- .../magnets/test_superconducting_magnet.py | 154 +----------------- 2 files changed, 16 insertions(+), 176 deletions(-) diff --git a/tests/devices/beamlines/i06_1/magnets/test_movements.py b/tests/devices/beamlines/i06_1/magnets/test_movements.py index c94c57dad9b..79c8432e360 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_movements.py +++ b/tests/devices/beamlines/i06_1/magnets/test_movements.py @@ -50,34 +50,24 @@ def test_cartesian_and_spherical_conversion_is_correct( assert result.z == pytest.approx(cartesian.z) -# def test_spherical_movement_decreases_z_then_x_then_y(): -# move_stragegy = SphericalMovement() +def test_spherical_movement_decreases_z_then_x_then_y(): + move_stragegy = SphericalMovement() -# steps = move_stragegy.moves( -# current=MagnetPosition(x=10, y=10, z=10), -# target=MagnetPosition(x=5, y=5, z=2), -# ) -# assert steps == [MagnetStep(z=2), MagnetStep(x=5), MagnetStep(y=5)] + steps = move_stragegy.moves( + current=MagnetPosition(x=10, y=10, z=10), + target=MagnetPosition(x=5, y=5, z=2), + ) + assert steps == [MagnetStep(z=2), MagnetStep(x=5), MagnetStep(y=5)] -# def test_spherical_movement_adds_final_combined_increase(): -# move_stragegy = SphericalMovement() +def test_spherical_movement_adds_final_combined_increase(): + move_stragegy = SphericalMovement() -# steps = move_stragegy.moves( -# current=MagnetPosition(x=10, y=10, z=10), -# target=MagnetPosition(x=20, y=5, z=2), -# ) -# assert steps == [MagnetStep(z=2), MagnetStep(y=5), MagnetStep(x=20, y=5, z=2)] - - -# @pytest.mark.parametrize( -# "target", [MagnetPosition(x=0, y=1, z=0), MagnetPosition(x=0, y=-1, z=0)] -# ) -# def test_planar_xz_rejects_y(target): -# move_stragegy = PlanarXZMovement() - -# with pytest.raises(ValueError): -# move_stragegy.moves(current=MagnetPosition(x=0, y=0, z=0), target=target) + steps = move_stragegy.moves( + current=MagnetPosition(x=10, y=10, z=10), + target=MagnetPosition(x=20, y=5, z=2), + ) + assert steps == [MagnetStep(z=2), MagnetStep(y=5), MagnetStep(x=20, y=5, z=2)] @pytest.mark.parametrize( diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 327bc1b8e5c..4fbe3704c15 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -90,44 +90,10 @@ async def test_scmc_axis_set_uses_correct_axis( assert z == expected_z -def test_cartesian_and_spherical_are_inverse() -> None: - cartesian = MagnetPosition(x=10, y=20, z=30) - result = cartesian.to_spherical().to_cartesian() - assert result.x == pytest.approx(cartesian.x) - assert result.y == pytest.approx(cartesian.y) - assert result.z == pytest.approx(cartesian.z) - - -def test_spherical_and_cartesian_are_inverse() -> None: - spherical = MagnetSphericalPosition(rho=10, theta=20, phi=30) - result = spherical.to_cartesian().to_spherical() - assert result.rho == pytest.approx(spherical.rho) - assert result.theta == pytest.approx(spherical.theta) - assert result.phi == pytest.approx(spherical.phi) - - @pytest.mark.parametrize( "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION ) -def test_cartesian_and_spherical_conversion_is_correct( - cartesian: MagnetPosition, - spherical: MagnetSphericalPosition, -) -> None: - result = cartesian.to_spherical() - assert result.rho == pytest.approx(spherical.rho) - assert result.theta == pytest.approx(spherical.theta) - assert result.phi == pytest.approx(spherical.phi) - - result = spherical.to_cartesian() - assert result.x == pytest.approx(cartesian.x) - assert result.y == pytest.approx(cartesian.y) - assert result.z == pytest.approx(cartesian.z) - - -@pytest.mark.parametrize( - "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION -) -async def test_scmc_set_using_spherical( +async def test_scmc_sph_set_using_spherical( scmc: SuperConductingMagnet, cartesian: MagnetPosition, spherical: MagnetSphericalPosition, @@ -156,7 +122,7 @@ async def test_scmc_set_using_spherical( @pytest.mark.parametrize( "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION ) -async def test_scmc_set_using_cartesian( +async def test_scmc_cart_set_using_cartesian( scmc: SuperConductingMagnet, cartesian: MagnetPosition, spherical: MagnetSphericalPosition, @@ -182,45 +148,6 @@ async def test_scmc_set_using_cartesian( assert phi == pytest.approx(spherical.phi) -@pytest.mark.parametrize( - "axis, value, expected_spherical", - [ - ("rho", 2, MagnetSphericalPosition(rho=2, theta=20, phi=30)), - ("theta", 40, MagnetSphericalPosition(rho=1, theta=40, phi=30)), - ("phi", 60, MagnetSphericalPosition(rho=1, theta=20, phi=60)), - ], -) -async def test_scmc_spherical_axis_set( - scmc: SuperConductingMagnet, - axis: str, - value: float, - expected_spherical: MagnetSphericalPosition, -) -> None: - await scmc.mode.set(MagnetModes.SPHERICAL) - # Start from a non-singular position. - await scmc.sph.set(MagnetSphericalPosition(rho=1, theta=20, phi=30)) - await getattr(scmc.sph, axis).set(value) - - rho, theta, phi = await asyncio.gather( - scmc.sph.rho.get_value(), - scmc.sph.theta.get_value(), - scmc.sph.phi.get_value(), - ) - assert rho == pytest.approx(expected_spherical.rho) - assert theta == pytest.approx(expected_spherical.theta) - assert phi == pytest.approx(expected_spherical.phi) - - expected_cartesian = expected_spherical.to_cartesian() - x, y, z = await asyncio.gather( - scmc.cart.x.readback.get_value(), - scmc.cart.y.readback.get_value(), - scmc.cart.z.readback.get_value(), - ) - assert x == pytest.approx(expected_cartesian.x) - assert y == pytest.approx(expected_cartesian.y) - assert z == pytest.approx(expected_cartesian.z) - - async def test_scmc_raises_error_if_limit_status_is_violation( scmc: SuperConductingMagnet, ) -> None: @@ -230,83 +157,6 @@ async def test_scmc_raises_error_if_limit_status_is_violation( await scmc.cart.x.set(1) -# @pytest.mark.parametrize( -# "initial, target, expected_calls", -# [ -# ( -# MagnetPosition(x=10, y=10, z=10), -# MagnetPosition(x=20, y=5, z=2), -# [ -# call.z(2), -# call.ramp(), -# call.y(5), -# call.ramp(), -# call.x(20), -# call.y(5), -# call.z(2), -# call.ramp(), -# ], -# ), -# ( -# MagnetPosition(x=10, y=10, z=10), -# MagnetPosition(x=5, y=20, z=2), -# [ -# call.z(2), -# call.ramp(), -# call.x(5), -# call.ramp(), -# call.x(5), -# call.y(20), -# call.z(2), -# call.ramp(), -# ], -# ), -# ( -# MagnetPosition(x=10, y=10, z=10), -# MagnetPosition(x=5, y=2, z=20), -# [ -# call.x(5), -# call.ramp(), -# call.y(2), -# call.ramp(), -# call.x(5), -# call.y(2), -# call.z(20), -# call.ramp(), -# ], -# ), -# ( -# MagnetPosition(x=10, y=10, z=10), -# MagnetPosition(x=5, y=5, z=2), -# [ -# call.z(2), -# call.ramp(), -# call.x(5), -# call.ramp(), -# call.y(5), -# call.ramp(), -# ], -# ), -# ], -# ) -# async def test_scmc_set_decreases_before_increases( -# scmc: SuperConductingMagnet, -# initial: MagnetPosition, -# target: MagnetPosition, -# expected_calls, -# ) -> None: -# await scmc.set(initial) - -# calls = [] -# scmc.x.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.x(v))) -# scmc.y.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.y(v))) -# scmc.z.demand.set = AsyncMock(side_effect=lambda v: calls.append(call.z(v))) -# scmc._ramp = AsyncMock(side_effect=lambda: calls.append(call.ramp())) - -# await scmc.set(target) -# assert calls == expected_calls - - async def test_scmc_set_within_boundary_raises_error_if_all_values_none( scmc: SuperConductingMagnet, ) -> None: From 917718d4e8505f945da0dbc7762557c62fedd2bb Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 14 Jul 2026 14:13:14 +0000 Subject: [PATCH 21/30] Fix test --- .../beamlines/i06_1/magnets/test_superconducting_magnet.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 4fbe3704c15..767c22b9397 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -153,7 +153,7 @@ async def test_scmc_raises_error_if_limit_status_is_violation( ) -> None: await scmc.mode.set(MagnetModes.UNIAXIAL_X) set_mock_value(scmc.limit_status, MagnetLimitStatus.VIOLTATION) - with pytest.raises(RuntimeError): + with pytest.raises(MagnetPositionError): await scmc.cart.x.set(1) From 7bb955bb723d8669773e02629cc5792855306d20 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 14 Jul 2026 14:24:19 +0000 Subject: [PATCH 22/30] Update doc strings and add logging --- .../i06_1/magnets/superconducting_magnet.py | 57 +++++++++++++++++-- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index ab555fa59c4..1f8e15a7b4d 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -136,6 +136,17 @@ def complete(self) -> WatchableAsyncStatus: class MagnetCartesianCoorindates(StandardReadable, Movable[MagnetPosition]): + """Cartesian interface to the superconducting magnet. + + Exposes the physical X, Y and Z magnet axes as individual movable signals, + together with a grouped ``MagnetPosition`` interface for moving all three axes + simultaneously. + + Individual axis moves and grouped cartesian moves are delegated to the parent + ``SuperConductingMagnet``, which applies the active movement strategy for the + current operating mode before commanding the hardware. + """ + def __init__( self, prefix: str, @@ -162,6 +173,18 @@ async def set(self, value: MagnetPosition): class MagnetSphericalCoordinates(StandardReadable, Movable[MagnetSphericalPosition]): + """Spherical coordinate interface to the superconducting magnet. + + Exposes the magnet field in spherical coordinates using the derived signals + ``rho``, ``theta`` and ``phi``. These signals are calculated from the + underlying cartesian magnet axes and may also be written individually or as a + complete ``MagnetSphericalPosition``. + + Writes are converted to cartesian coordinates before being delegated to the + parent ``SuperConductingMagnet``, allowing the active movement strategy to + determine a safe sequence of cartesian moves. + """ + def __init__( self, cart: MagnetCartesianCoorindates, @@ -256,13 +279,15 @@ def _set_mode(value): class SuperConductingMagnet(StandardReadable): """A three-axis superconducting vector magnet. - The magnet exposes three independent cartesian axes (``x``, ``y`` and ``z``) - together with derived spherical coordinates (``rho``, ``theta`` and ``phi``). - Positions may be set using either cartesian or spherical coordinates. + The magnet provides both cartesian and spherical coordinate interfaces for + reading and setting the magnetic field. Cartesian coordinates expose the + physical X, Y and Z magnet axes, while spherical coordinates provide the + equivalent field magnitude and orientation. - To minimise the risk of quenching the magnet, moves are performed by first - applying any decreases in field magnitude (in Z, X, Y order) before applying - all increases together. + The permitted operating region and the sequence of axis movements are + determined by the current magnet operating mode. Each operating mode uses a + dedicated movement strategy to validate requested positions and generate a + safe sequence of cartesian magnet moves before ramping the field. """ def __init__( @@ -295,11 +320,19 @@ async def _ramp(self): raise MagnetPositionError( f"{self.limit_status.name} is at {MagnetLimitStatus.VIOLTATION}" ) + self.log.info("About to start ramping the magnet.") # ToDo - Use TimeoutCalculated from ophyd-async in new release. await self.start_ramp.trigger(timeout=self.timeout) await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, self.timeout) + self.log.info("Ramping complete.") async def _apply_step(self, step: MagnetStep) -> None: + """Apply a single movement step and wait for the magnet ramp to complete. + + A movement step may update one or more cartesian axes simultaneously. Once + the requested demand values have been written, the magnet is instructed to + ramp and this method waits for the ramp to complete before returning. + """ tasks = [] if step.x is not None: tasks.append(self.cart.x.demand.set(step.x)) @@ -317,6 +350,14 @@ async def set_within_boundary( y: float | None = None, z: float | None = None, ): + """Move the magnet to a new cartesian position. + + Any coordinates left as ``None`` retain their current values. The requested + target position is validated against the current magnet operating mode and + converted into a sequence of movement steps by the corresponding movement + strategy. Each step is then applied sequentially until the requested target + position is reached. + """ if x is None and y is None and z is None: raise MagnetPositionError("x, y, and z cannot all be None.") current = MagnetPosition( @@ -332,5 +373,9 @@ async def set_within_boundary( mode = await self.mode.get_value() movement_strategy = MODE_MOVEMENT_STRATEGY[mode] + self.log.debug( + f"Attempting move in mode {mode} with parameters {target}. Current position is {current}" + ) + for step in movement_strategy.moves(current, target): await self._apply_step(step) From d0282c6f26f6125543ab712f5f133d391b6ad95d Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Tue, 14 Jul 2026 15:35:59 +0000 Subject: [PATCH 23/30] Remove unneeded comment --- .../devices/beamlines/i06_1/magnets/superconducting_magnet.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 1f8e15a7b4d..fce04a8c292 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -129,7 +129,6 @@ async def kickoff(self): self._fly_info = None def complete(self) -> WatchableAsyncStatus: - """Return a ``Status`` and mark it done when acquisition has completed.""" fly_status = error_if_none(self._fly_status, f"{self.name} kickoff not called.") self._fly_status = None return fly_status From 55446eb23def4c8bbb8465fdf1ea97b727e8b07b Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 15 Jul 2026 09:21:04 +0000 Subject: [PATCH 24/30] Add better mocking behaviour --- .../i06_1/magnets/superconducting_magnet.py | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index fce04a8c292..c6ddc5fd34f 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -73,6 +73,9 @@ async def move( await self.magnet_set_within_boundary(**values) +@default_mock_class(DeviceMock) +# Don't want to sync readback and setpoint. IOC behaviour will move readback to demand +# once start_ramp is triggered. This logic lives in the SuperConductingMagnet. class MagnetAxis(StandardReadable, StandardMovable[float], Flyable, Preparable): """Represents one cartesian axis of a superconducting vector magnet. @@ -254,22 +257,36 @@ async def _set_phi(self, phi: float): class MockSuperConductingMagnet(DeviceMock["SuperConductingMagnet"]): + """Add additional callback logic to our device to get the mock behaviour to simulate + the hardware as best we can. + """ + async def connect(self, device: "SuperConductingMagnet"): - def _set_ramp_status(value): + async def _trigger_start_ramp(value): + # Whenever ramp is triggered for the ioc, readback values move to the + # demand values. Simulate this behaviour here. + x_d, y_d, z_d = await asyncio.gather( + device.cart.x.demand.get_value(), + device.cart.y.demand.get_value(), + device.cart.z.demand.get_value(), + ) set_mock_value(device.ramp_status, MagnetRampStatus.RAMPING) + set_mock_value(device.cart.x.readback, x_d) + set_mock_value(device.cart.y.readback, y_d) + set_mock_value(device.cart.z.readback, z_d) set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) - def _set_mode(value): - # Whenever mode is changed, ioc automatically sets everything to zero and ramps + async def _set_mode(value): + # Whenever mode is set, ioc automatically sets everything to zero and + # triggers a ramp. set_mock_value(device.cart.x.demand, 0.0) set_mock_value(device.cart.y.demand, 0.0) set_mock_value(device.cart.z.demand, 0.0) - set_mock_value(device.ramp_status, MagnetRampStatus.RAMPING) - set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) + await _trigger_start_ramp(value) callback_on_mock_put(device.mode, _set_mode) - callback_on_mock_put(device.start_ramp, _set_ramp_status) + callback_on_mock_put(device.start_ramp, _trigger_start_ramp) set_mock_value(device.limit_status, MagnetLimitStatus.OK) set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) From 424d5a361ad8bd0a72b8d4318ce06cb2ec5d2008 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 15 Jul 2026 09:21:42 +0000 Subject: [PATCH 25/30] Update SuperConductingMagnet to SuperConductingMagnetController --- src/dodal/beamlines/i06_1.py | 10 +++-- .../beamlines/i06_1/magnets/__init__.py | 4 +- .../i06_1/magnets/ramp_controller.py | 2 +- .../i06_1/magnets/superconducting_magnet.py | 18 +++++---- .../magnets/test_superconducting_magnet.py | 38 +++++++++++-------- 5 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py index 280291bb3eb..e9f94384152 100644 --- a/src/dodal/beamlines/i06_1.py +++ b/src/dodal/beamlines/i06_1.py @@ -4,7 +4,7 @@ from dodal.devices.beamlines.i06_1 import DiffractionDichroism from dodal.devices.beamlines.i06_1.magnets import ( MagnetThreeAxesRampRateController, - SuperConductingMagnet, + SuperConductingMagnetController, ) from dodal.devices.motors import XYThetaStage from dodal.devices.temperture_controller import Lakeshore336 @@ -46,5 +46,9 @@ def mag_ramp_rate() -> MagnetThreeAxesRampRateController: @devices.factory() -def scmc(mag_ramp_rate: MagnetThreeAxesRampRateController) -> SuperConductingMagnet: - return SuperConductingMagnet(f"{PREFIX.beamline_prefix}-EA-MAG-01:", mag_ramp_rate) +def scmc( + mag_ramp_rate: MagnetThreeAxesRampRateController, +) -> SuperConductingMagnetController: + return SuperConductingMagnetController( + f"{PREFIX.beamline_prefix}-EA-MAG-01:", mag_ramp_rate + ) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py index 9b2559bc287..c5e558e5a84 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/__init__.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/__init__.py @@ -9,7 +9,7 @@ MagnetLimitStatus, MagnetModes, MagnetRampStatus, - SuperConductingMagnet, + SuperConductingMagnetController, ) __all__ = [ @@ -23,5 +23,5 @@ "MagnetLimitStatus", "MagnetModes", "MagnetRampStatus", - "SuperConductingMagnet", + "SuperConductingMagnetController", ] diff --git a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py index 4cdd4cfccdc..741de292db6 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py @@ -37,7 +37,7 @@ def movable_logic(self) -> RampRateMovableLogic: class MagnetThreeAxesRampRateController(StandardReadable): """Groups the ramp rate controllers for the x, y and z magnet axes. - This device is passed to :class:`SuperConductingMagnet` so that each + This device is passed to :class:`SuperConductingMagnetController` so that each :class:`MagnetAxis` can configure its own ramp rate during preparation for fly scans. """ diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index c6ddc5fd34f..3d9b8256833 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -75,12 +75,12 @@ async def move( @default_mock_class(DeviceMock) # Don't want to sync readback and setpoint. IOC behaviour will move readback to demand -# once start_ramp is triggered. This logic lives in the SuperConductingMagnet. +# once start_ramp is triggered. This logic lives in the SuperConductingMagnetController. class MagnetAxis(StandardReadable, StandardMovable[float], Flyable, Preparable): """Represents one cartesian axis of a superconducting vector magnet. Although each axis can be moved independently, all moves are delegated to - the parent :class:`SuperConductingMagnet` so that coordinated motion can + the parent :class:`SuperConductingMagnetController` so that coordinated motion can enforce safe field transitions before updating the underlying demand PVs. Each axis is associated with a :class:`MagnetAxisRampRateController`, which is used @@ -145,7 +145,7 @@ class MagnetCartesianCoorindates(StandardReadable, Movable[MagnetPosition]): simultaneously. Individual axis moves and grouped cartesian moves are delegated to the parent - ``SuperConductingMagnet``, which applies the active movement strategy for the + ``SuperConductingMagnetController``, which applies the active movement strategy for the current operating mode before commanding the hardware. """ @@ -183,7 +183,7 @@ class MagnetSphericalCoordinates(StandardReadable, Movable[MagnetSphericalPositi complete ``MagnetSphericalPosition``. Writes are converted to cartesian coordinates before being delegated to the - parent ``SuperConductingMagnet``, allowing the active movement strategy to + parent ``SuperConductingMagnetController``, allowing the active movement strategy to determine a safe sequence of cartesian moves. """ @@ -256,12 +256,14 @@ async def _set_phi(self, phi: float): } -class MockSuperConductingMagnet(DeviceMock["SuperConductingMagnet"]): +class MockSuperConductingMagnetController( + DeviceMock["SuperConductingMagnetController"] +): """Add additional callback logic to our device to get the mock behaviour to simulate the hardware as best we can. """ - async def connect(self, device: "SuperConductingMagnet"): + async def connect(self, device: "SuperConductingMagnetController"): async def _trigger_start_ramp(value): # Whenever ramp is triggered for the ioc, readback values move to the @@ -291,8 +293,8 @@ async def _set_mode(value): set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) -@default_mock_class(MockSuperConductingMagnet) -class SuperConductingMagnet(StandardReadable): +@default_mock_class(MockSuperConductingMagnetController) +class SuperConductingMagnetController(StandardReadable): """A three-axis superconducting vector magnet. The magnet provides both cartesian and spherical coordinate interfaces for diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 767c22b9397..666df41eeb4 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -17,7 +17,7 @@ MagnetRampStatus, MagnetSphericalPosition, MagnetThreeAxesRampRateController, - SuperConductingMagnet, + SuperConductingMagnetController, movement, ) from dodal.devices.beamlines.i06_1.magnets.superconducting_magnet import ( @@ -36,13 +36,15 @@ def ramp_rate() -> MagnetThreeAxesRampRateController: @pytest.fixture -def scmc(ramp_rate: MagnetThreeAxesRampRateController) -> SuperConductingMagnet: +def scmc( + ramp_rate: MagnetThreeAxesRampRateController, +) -> SuperConductingMagnetController: with init_devices(mock=True): - scmc = SuperConductingMagnet("TEST:", ramp_rate) + scmc = SuperConductingMagnetController("TEST:", ramp_rate) return scmc -async def test_scmc_read(scmc: SuperConductingMagnet) -> None: +async def test_scmc_read(scmc: SuperConductingMagnetController) -> None: await assert_reading( scmc, { @@ -56,7 +58,7 @@ async def test_scmc_read(scmc: SuperConductingMagnet) -> None: ) -async def test_scmc_configuration(scmc: SuperConductingMagnet) -> None: +async def test_scmc_configuration(scmc: SuperConductingMagnetController) -> None: await assert_configuration( scmc, {"scmc-mode": partial_reading(MagnetModes.UNIAXIAL_X)} ) @@ -71,7 +73,7 @@ async def test_scmc_configuration(scmc: SuperConductingMagnet) -> None: ], ) async def test_scmc_axis_set_uses_correct_axis( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, axis: str, value: float, expected_x: float, @@ -94,7 +96,7 @@ async def test_scmc_axis_set_uses_correct_axis( "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION ) async def test_scmc_sph_set_using_spherical( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, cartesian: MagnetPosition, spherical: MagnetSphericalPosition, ) -> None: @@ -123,7 +125,7 @@ async def test_scmc_sph_set_using_spherical( "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION ) async def test_scmc_cart_set_using_cartesian( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, cartesian: MagnetPosition, spherical: MagnetSphericalPosition, ) -> None: @@ -149,7 +151,7 @@ async def test_scmc_cart_set_using_cartesian( async def test_scmc_raises_error_if_limit_status_is_violation( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, ) -> None: await scmc.mode.set(MagnetModes.UNIAXIAL_X) set_mock_value(scmc.limit_status, MagnetLimitStatus.VIOLTATION) @@ -158,7 +160,7 @@ async def test_scmc_raises_error_if_limit_status_is_violation( async def test_scmc_set_within_boundary_raises_error_if_all_values_none( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, ) -> None: with pytest.raises(MagnetPositionError): await scmc.set_within_boundary() @@ -173,7 +175,7 @@ async def test_scmc_set_within_boundary_raises_error_if_all_values_none( ], ) async def test_scmc_axis_with_ramp_rate_wired_correctly_with_prepare( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, ramp_rate: MagnetThreeAxesRampRateController, axis: str, mode: MagnetModes, @@ -188,7 +190,9 @@ async def test_scmc_axis_with_ramp_rate_wired_correctly_with_prepare( assert await ramp_axis.demand.get_value() == fly_info.ramp_rate -async def test_scmc_axis_kickoff_and_complete(scmc: SuperConductingMagnet) -> None: +async def test_scmc_axis_kickoff_and_complete( + scmc: SuperConductingMagnetController, +) -> None: await scmc.mode.set(MagnetModes.UNIAXIAL_X) fly_info = FlyMagnetInfo(start_position=1, end_position=2, ramp_rate=4) assert scmc.cart.x._fly_info is None @@ -202,7 +206,7 @@ async def test_scmc_axis_kickoff_and_complete(scmc: SuperConductingMagnet) -> No async def test_scmc_axis_kickoff_and_complete_raises_error_without_prepare( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, ): await scmc.mode.set(MagnetModes.UNIAXIAL_X) # Do multiple times to make sure you cannot kickoff or complete without preparing @@ -220,7 +224,9 @@ async def test_scmc_axis_kickoff_and_complete_raises_error_without_prepare( await scmc.cart.x.complete() -async def test_scmc_mock_device_behaviour(scmc: SuperConductingMagnet) -> None: +async def test_scmc_mock_device_behaviour( + scmc: SuperConductingMagnetController, +) -> None: assert await scmc.limit_status.get_value() == MagnetLimitStatus.OK set_mock_value(scmc.cart.x.demand, 1) @@ -279,7 +285,7 @@ async def test_magnet_mode_to_uniaxial_movement_strategy_configuration(): async def test_scmc_executes_movement_stragey_and_ramp_at_each_step( - scmc: SuperConductingMagnet, + scmc: SuperConductingMagnetController, ) -> None: movement_strategy = MagicMock() @@ -299,7 +305,7 @@ async def test_scmc_executes_movement_stragey_and_ramp_at_each_step( scmc._ramp = AsyncMock() with patch( - "dodal.devices.beamlines.i06_1.magnets.superconducting_magnet.SuperConductingMagnet._apply_step", + "dodal.devices.beamlines.i06_1.magnets.superconducting_magnet.SuperConductingMagnetController._apply_step", wraps=scmc._apply_step, ) as mock_apply_step: await scmc.mode.set(MagnetModes.UNIAXIAL_X) From c12f48268e643560a5c53b00a86fbcc034bde198 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 15 Jul 2026 09:53:49 +0000 Subject: [PATCH 26/30] Add test for no movement strategy --- .../i06_1/magnets/superconducting_magnet.py | 33 +++++++++-------- .../magnets/test_superconducting_magnet.py | 35 +++++++++++++------ 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 3d9b8256833..337742e0c4b 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -245,17 +245,6 @@ async def _set_phi(self, phi: float): await self._set_spherical(phi=phi) -MODE_MOVEMENT_STRATEGY: dict[MagnetModes, MovementStrategy] = { - MagnetModes.SPHERICAL: SphericalMovement(), - MagnetModes.CUBIC: CubicMovement(), - MagnetModes.PLANAR_XZ: PlanarXZMovement(), - MagnetModes.QUADRANT_XY: QuadrantXYMovement(), - MagnetModes.UNIAXIAL_X: UniaxialMovement("x", limit=2.0), - MagnetModes.UNIAXIAL_Y: UniaxialMovement("y", limit=2.0), - MagnetModes.UNIAXIAL_Z: UniaxialMovement("z", limit=6.0), -} - - class MockSuperConductingMagnetController( DeviceMock["SuperConductingMagnetController"] ): @@ -308,6 +297,16 @@ class SuperConductingMagnetController(StandardReadable): safe sequence of cartesian magnet moves before ramping the field. """ + _MODE_MOVEMENT_STRATEGY: dict[MagnetModes, MovementStrategy] = { + MagnetModes.SPHERICAL: SphericalMovement(), + MagnetModes.CUBIC: CubicMovement(), + MagnetModes.PLANAR_XZ: PlanarXZMovement(), + MagnetModes.QUADRANT_XY: QuadrantXYMovement(), + MagnetModes.UNIAXIAL_X: UniaxialMovement("x", limit=2.0), + MagnetModes.UNIAXIAL_Y: UniaxialMovement("y", limit=2.0), + MagnetModes.UNIAXIAL_Z: UniaxialMovement("z", limit=6.0), + } + def __init__( self, prefix: str, @@ -339,10 +338,11 @@ async def _ramp(self): f"{self.limit_status.name} is at {MagnetLimitStatus.VIOLTATION}" ) self.log.info("About to start ramping the magnet.") - # ToDo - Use TimeoutCalculated from ophyd-async in new release. await self.start_ramp.trigger(timeout=self.timeout) await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, self.timeout) - self.log.info("Ramping complete.") + self.log.info( + f"Ramping complete. Ramp status is now {MagnetRampStatus.RAMP_MADE}" + ) async def _apply_step(self, step: MagnetStep) -> None: """Apply a single movement step and wait for the magnet ramp to complete. @@ -359,6 +359,7 @@ async def _apply_step(self, step: MagnetStep) -> None: if step.z is not None: tasks.append(self.cart.z.demand.set(step.z)) + self.log.info(f"About to set demand values of the magnet to {step}.") await asyncio.gather(*tasks) await self._ramp() @@ -389,7 +390,11 @@ async def set_within_boundary( z=current.z if z is None else z, ) mode = await self.mode.get_value() - movement_strategy = MODE_MOVEMENT_STRATEGY[mode] + movement_strategy = self._MODE_MOVEMENT_STRATEGY.get(mode) + if movement_strategy is None: + raise ValueError( + f"No movement strategy has been configured for device {self.name} for mode {mode}." + ) self.log.debug( f"Attempting move in mode {mode} with parameters {target}. Current position is {current}" diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 666df41eeb4..5ce641a8295 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -20,9 +20,6 @@ SuperConductingMagnetController, movement, ) -from dodal.devices.beamlines.i06_1.magnets.superconducting_magnet import ( - MODE_MOVEMENT_STRATEGY, -) from tests.devices.beamlines.i06_1.magnets.utils import ( EXPECTED_CARTESIAN_SPHERICAL_CONVERSION, ) @@ -260,6 +257,18 @@ def _ramp_state_callback(value: dict[str, Reading[MagnetRampStatus]]): ] +async def test_scmc_no_movement_strategy_for_mode( + scmc: SuperConductingMagnetController, +) -> None: + mode = await scmc.mode.get_value() + with patch.dict(scmc._MODE_MOVEMENT_STRATEGY, {}, clear=True): + with pytest.raises( + ValueError, + match=f"No movement strategy has been configured for device scmc for mode {mode}", + ): + await scmc.cart.x.set(5) + + @pytest.mark.parametrize( "mode,expected", [ @@ -272,16 +281,20 @@ def _ramp_state_callback(value: dict[str, Reading[MagnetRampStatus]]): (MagnetModes.UNIAXIAL_Z, movement.UniaxialMovement), ], ) -async def test_magnet_mode_to_movement_strategy_configuration( - mode: MagnetModes, expected: type[movement.MovementStrategy] +async def test_scmc_magnet_mode_to_movement_strategy_configuration( + scmc: SuperConductingMagnetController, + mode: MagnetModes, + expected: type[movement.MovementStrategy], ) -> None: - assert isinstance(MODE_MOVEMENT_STRATEGY[mode], expected) + assert isinstance(scmc._MODE_MOVEMENT_STRATEGY[mode], expected) -async def test_magnet_mode_to_uniaxial_movement_strategy_configuration(): - assert MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_X].axis == "x" # type:ignore - assert MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_Y].axis == "y" # type:ignore - assert MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_Z].axis == "z" # type:ignore +async def test_scmc_magnet_mode_to_uniaxial_movement_strategy_configuration( + scmc: SuperConductingMagnetController, +) -> None: + assert scmc._MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_X].axis == "x" # type:ignore + assert scmc._MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_Y].axis == "y" # type:ignore + assert scmc._MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_Z].axis == "z" # type:ignore async def test_scmc_executes_movement_stragey_and_ramp_at_each_step( @@ -300,7 +313,7 @@ async def test_scmc_executes_movement_stragey_and_ramp_at_each_step( ] movement_strategy.moves.return_value = mov_str_return_values - MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_X] = movement_strategy + scmc._MODE_MOVEMENT_STRATEGY[MagnetModes.UNIAXIAL_X] = movement_strategy scmc._ramp = AsyncMock() From d11a7ee60c036b682eca8e5e880ba2f05d61bbd2 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 15 Jul 2026 10:03:09 +0000 Subject: [PATCH 27/30] Improve typing --- .../i06_1/magnets/superconducting_magnet.py | 11 +--- .../beamlines/i06_1/magnets/test_movements.py | 56 ++++++++----------- 2 files changed, 24 insertions(+), 43 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index 337742e0c4b..edf2625e5da 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -1,5 +1,4 @@ import asyncio -from collections.abc import Callable from dataclasses import dataclass from functools import cached_property from typing import Protocol @@ -13,6 +12,7 @@ StandardMovable, StandardReadable, StandardReadableFormat, + TimeoutCalculator, WatchableAsyncStatus, callback_on_mock_put, default_mock_class, @@ -65,11 +65,8 @@ class MagnetAxisMovableLogic(MovableLogic[float]): axis: str magnet_set_within_boundary: MagnetMoveWithinBoundary - async def move( - self, new_position: float, timeout: Callable[[], float | None] - ) -> None: + async def move(self, new_position: float, timeout: TimeoutCalculator) -> None: values = {self.axis: new_position} - # ToDo - feed timeout to here? await self.magnet_set_within_boundary(**values) @@ -107,7 +104,6 @@ def __init__( axis=axis, magnet_set_within_boundary=magnet_set_within_boundary, ) - self._fly_info: FlyMagnetInfo | None = None self._fly_status: WatchableAsyncStatus | None = None super().__init__(name) @@ -323,7 +319,6 @@ def __init__( self.mode = epics_signal_rw(MagnetModes, prefix + "MODE") self.ramp_status = epics_signal_rw(MagnetRampStatus, prefix + "RAMPSTATUS") - self.limit_status = epics_signal_rw(MagnetLimitStatus, prefix + "LIMITSTATUS") self.start_ramp = epics_signal_x(prefix + "STARTRAMP.PROC") self.timeout = 600 @@ -395,10 +390,8 @@ async def set_within_boundary( raise ValueError( f"No movement strategy has been configured for device {self.name} for mode {mode}." ) - self.log.debug( f"Attempting move in mode {mode} with parameters {target}. Current position is {current}" ) - for step in movement_strategy.moves(current, target): await self._apply_step(step) diff --git a/tests/devices/beamlines/i06_1/magnets/test_movements.py b/tests/devices/beamlines/i06_1/magnets/test_movements.py index 79c8432e360..53b12aadafe 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_movements.py +++ b/tests/devices/beamlines/i06_1/magnets/test_movements.py @@ -36,8 +36,7 @@ def test_spherical_and_cartesian_are_inverse() -> None: "cartesian, spherical", EXPECTED_CARTESIAN_SPHERICAL_CONVERSION ) def test_cartesian_and_spherical_conversion_is_correct( - cartesian: MagnetPosition, - spherical: MagnetSphericalPosition, + cartesian: MagnetPosition, spherical: MagnetSphericalPosition ) -> None: result = cartesian.to_spherical() assert result.rho == pytest.approx(spherical.rho) @@ -50,9 +49,8 @@ def test_cartesian_and_spherical_conversion_is_correct( assert result.z == pytest.approx(cartesian.z) -def test_spherical_movement_decreases_z_then_x_then_y(): +def test_spherical_movement_decreases_z_then_x_then_y() -> None: move_stragegy = SphericalMovement() - steps = move_stragegy.moves( current=MagnetPosition(x=10, y=10, z=10), target=MagnetPosition(x=5, y=5, z=2), @@ -60,9 +58,8 @@ def test_spherical_movement_decreases_z_then_x_then_y(): assert steps == [MagnetStep(z=2), MagnetStep(x=5), MagnetStep(y=5)] -def test_spherical_movement_adds_final_combined_increase(): +def test_spherical_movement_adds_final_combined_increase() -> None: move_stragegy = SphericalMovement() - steps = move_stragegy.moves( current=MagnetPosition(x=10, y=10, z=10), target=MagnetPosition(x=20, y=5, z=2), @@ -106,10 +103,8 @@ def test_spherical_movement_adds_final_combined_increase(): ], ) def test_spherical_movement( - current: MagnetPosition, - target: MagnetPosition, - expected: list[MagnetStep], -): + current: MagnetPosition, target: MagnetPosition, expected: list[MagnetStep] +) -> None: assert SphericalMovement().moves(current, target) == expected @@ -121,37 +116,34 @@ def test_spherical_movement( MagnetPosition(x=0, y=0, z=2), ], ) -def test_cubic_rejects_outside_limits(target: MagnetPosition): +def test_cubic_rejects_outside_limits(target: MagnetPosition) -> None: with pytest.raises(MagnetPositionError): CubicMovement().moves(MagnetPosition(x=0, y=0, z=0), target) -def test_cubic_returns_single_step(): +def test_cubic_returns_single_step() -> None: current = MagnetPosition(x=0, y=0, z=0) target = MagnetPosition(x=1, y=1.2, z=-0.5) assert CubicMovement().moves(current, target) == [MagnetStep(x=1, y=1.2, z=-0.5)] -def test_planar_xz_returns_single_step(): +def test_planar_xz_returns_single_step() -> None: target = MagnetPosition(x=1.2, y=0, z=-0.4) - assert PlanarXZMovement().moves( - MagnetPosition(x=0, y=0, z=0), - target, - ) == [ + assert PlanarXZMovement().moves(MagnetPosition(x=0, y=0, z=0), target) == [ MagnetStep(x=1.2, z=-0.4), ] -def test_planar_xz_rejects_nonzero_y(): +def test_planar_xz_rejects_nonzero_y() -> None: with pytest.raises(MagnetPositionError): PlanarXZMovement().moves( MagnetPosition(x=0, y=0, z=0), MagnetPosition(x=1, y=1, z=0) ) -def test_planar_xz_rejects_outside_radius(): +def test_planar_xz_rejects_outside_radius() -> None: with pytest.raises(MagnetPositionError): PlanarXZMovement().moves( MagnetPosition(x=0, y=0, z=0), MagnetPosition(x=2, y=0, z=2) @@ -166,7 +158,9 @@ def test_planar_xz_rejects_outside_radius(): ("z", 5, MagnetPosition(x=0, y=0, z=3)), ], ) -def test_uniaxial_returns_single_step(axis, limit, target): +def test_uniaxial_returns_single_step( + axis: str, limit: float, target: MagnetPosition +) -> None: assert UniaxialMovement(axis, limit).moves( MagnetPosition(x=0, y=0, z=0), target ) == [MagnetStep(**{axis: getattr(target, axis)})] @@ -183,17 +177,15 @@ def test_uniaxial_returns_single_step(axis, limit, target): ("z", MagnetPosition(x=0, y=1, z=1)), ], ) -def test_uniaxial_rejects_other_axes(axis, target): +def test_uniaxial_rejects_other_axes(axis: str, target: MagnetPosition) -> None: with pytest.raises(MagnetPositionError): UniaxialMovement(axis, limit=5).moves(MagnetPosition(x=0, y=0, z=0), target) -def test_quadrant_xy_sequence(): - +def test_quadrant_xy_sequence() -> None: current = MagnetPosition(x=1, y=0.5, z=0) target = MagnetPosition(x=1.5, y=1, z=0) strategy = QuadrantXYMovement() - assert strategy.moves(current, target) == [ MagnetStep(x=0), MagnetStep(y=1), @@ -201,33 +193,29 @@ def test_quadrant_xy_sequence(): ] -def test_quadrant_xy_skips_initial_x_zero(): +def test_quadrant_xy_skips_initial_x_zero() -> None: current = MagnetPosition(x=0, y=0, z=0) target = MagnetPosition(x=1, y=1, z=0) strategy = QuadrantXYMovement() - assert strategy.moves(current, target) == [MagnetStep(y=1), MagnetStep(x=1)] -def test_quadrant_xy_skips_y_move(): +def test_quadrant_xy_skips_y_move() -> None: target = MagnetPosition(x=1, y=1.5, z=0) current = MagnetPosition(x=0.5, y=1.5, z=0) strategy = QuadrantXYMovement() - assert strategy.moves(target, current) == [MagnetStep(x=0), MagnetStep(x=0.5)] -def test_quadrant_xy_rejects_nonzero_z(): +def test_quadrant_xy_rejects_nonzero_z() -> None: with pytest.raises(MagnetPositionError): QuadrantXYMovement().moves( - MagnetPosition(x=0, y=0, z=0), - MagnetPosition(x=1, y=1, z=1), + MagnetPosition(x=0, y=0, z=0), MagnetPosition(x=1, y=1, z=1) ) -def test_quadrant_xy_rejects_outside_radius(): +def test_quadrant_xy_rejects_outside_radius() -> None: with pytest.raises(MagnetPositionError): QuadrantXYMovement().moves( - MagnetPosition(x=0, y=0, z=0), - MagnetPosition(x=2, y=2, z=0), + MagnetPosition(x=0, y=0, z=0), MagnetPosition(x=2, y=2, z=0) ) From 665095f987e88f4bedeeae89eac611b19079fd2a Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 15 Jul 2026 12:17:39 +0000 Subject: [PATCH 28/30] Add check_value logic for the ramp controllers --- .../i06_1/magnets/ramp_controller.py | 27 ++++++++++++++++++- .../i06_1/magnets/superconducting_magnet.py | 11 ++++---- .../i06_1/magnets/test_ramp_controller.py | 13 ++++++++- .../magnets/test_superconducting_magnet.py | 6 ++--- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py index 741de292db6..77413ed012b 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/ramp_controller.py @@ -1,20 +1,43 @@ +from dataclasses import dataclass from functools import cached_property from ophyd_async.core import ( + InstantMovableMock, MovableLogic, + SignalR, StandardMovable, StandardReadable, StandardReadableFormat, TimeoutCalculator, + default_mock_class, + set_mock_value, ) from ophyd_async.epics.core import epics_signal_rw +@dataclass class RampRateMovableLogic(MovableLogic[float]): + limit: SignalR[float] + + async def check_move(self, new_position: float) -> None: + limit = await self.limit.get_value() + if new_position > limit: + raise ValueError( + f"Requested ramp rate {new_position} exceeds the maximum limit of {limit} for device {self.readback.name}." + ) + async def move(self, new_position: float, timeout: TimeoutCalculator): await self.setpoint.set(new_position, timeout=timeout()) +class MockMagnetAxisRampRateController(InstantMovableMock): + async def connect(self, device: StandardMovable): + await super().connect(device) + # Extend to set a sensible default value for the limit. + set_mock_value(device.limit, 2) # type: ignore + + +@default_mock_class(MockMagnetAxisRampRateController) class MagnetAxisRampRateController(StandardMovable[float], StandardReadable): """Controls the ramp rate of a single superconducting magnet axis. @@ -31,7 +54,9 @@ def __init__(self, prefix: str, name: str = ""): @cached_property def movable_logic(self) -> RampRateMovableLogic: - return RampRateMovableLogic(readback=self.readback, setpoint=self.demand) + return RampRateMovableLogic( + readback=self.readback, setpoint=self.demand, limit=self.limit + ) class MagnetThreeAxesRampRateController(StandardReadable): diff --git a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py index edf2625e5da..e3ae44ede17 100644 --- a/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py +++ b/src/dodal/devices/beamlines/i06_1/magnets/superconducting_magnet.py @@ -264,6 +264,8 @@ async def _trigger_start_ramp(value): set_mock_value(device.cart.z.readback, z_d) set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) + callback_on_mock_put(device.start_ramp, _trigger_start_ramp) + async def _set_mode(value): # Whenever mode is set, ioc automatically sets everything to zero and # triggers a ramp. @@ -273,7 +275,6 @@ async def _set_mode(value): await _trigger_start_ramp(value) callback_on_mock_put(device.mode, _set_mode) - callback_on_mock_put(device.start_ramp, _trigger_start_ramp) set_mock_value(device.limit_status, MagnetLimitStatus.OK) set_mock_value(device.ramp_status, MagnetRampStatus.RAMP_MADE) @@ -328,10 +329,9 @@ def __init__( async def _ramp(self): # Setting invalid demand values should put the limit status in violation state. # Block ramp if in violation state. - if await self.limit_status.get_value() == MagnetLimitStatus.VIOLTATION: - raise MagnetPositionError( - f"{self.limit_status.name} is at {MagnetLimitStatus.VIOLTATION}" - ) + limit_status = await self.limit_status.get_value() + if limit_status == MagnetLimitStatus.VIOLTATION: + raise MagnetPositionError(f"{self.limit_status.name} is at {limit_status}") self.log.info("About to start ramping the magnet.") await self.start_ramp.trigger(timeout=self.timeout) await wait_for_value(self.ramp_status, MagnetRampStatus.RAMP_MADE, self.timeout) @@ -353,7 +353,6 @@ async def _apply_step(self, step: MagnetStep) -> None: tasks.append(self.cart.y.demand.set(step.y)) if step.z is not None: tasks.append(self.cart.z.demand.set(step.z)) - self.log.info(f"About to set demand values of the magnet to {step}.") await asyncio.gather(*tasks) await self._ramp() diff --git a/tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py b/tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py index 14f9822d332..7d7604fd8a7 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py +++ b/tests/devices/beamlines/i06_1/magnets/test_ramp_controller.py @@ -24,11 +24,22 @@ async def test_magx_ramp_rate_read( async def test_magx_ramp_rate_set( magx_ramp_rate: MagnetAxisRampRateController, ) -> None: - ramp_rate = 10 + ramp_rate = 1 await magx_ramp_rate.set(ramp_rate) assert await magx_ramp_rate.readback.get_value() == ramp_rate +async def test_magx_ramp_rate_set_above_limit_throws_error( + magx_ramp_rate: MagnetAxisRampRateController, +) -> None: + ramp_rate = 10 + with pytest.raises( + ValueError, + match=f"Requested ramp rate {ramp_rate} exceeds the maximum limit of 2.0 for device {magx_ramp_rate.name}.", + ): + await magx_ramp_rate.set(ramp_rate) + + @pytest.fixture def mag_three_axis_ramp_rate() -> MagnetThreeAxesRampRateController: with init_devices(mock=True): diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index 5ce641a8295..f04558c63a4 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -181,7 +181,7 @@ async def test_scmc_axis_with_ramp_rate_wired_correctly_with_prepare( magnet_axis: MagnetAxis = getattr(scmc.cart, axis) ramp_axis: MagnetAxisRampRateController = getattr(ramp_rate, axis) - fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=4) + fly_info = FlyMagnetInfo(start_position=1, end_position=6, ramp_rate=1.5) await magnet_axis.prepare(fly_info) assert await magnet_axis.demand.get_value() == fly_info.start_position assert await ramp_axis.demand.get_value() == fly_info.ramp_rate @@ -191,7 +191,7 @@ async def test_scmc_axis_kickoff_and_complete( scmc: SuperConductingMagnetController, ) -> None: await scmc.mode.set(MagnetModes.UNIAXIAL_X) - fly_info = FlyMagnetInfo(start_position=1, end_position=2, ramp_rate=4) + fly_info = FlyMagnetInfo(start_position=1, end_position=2, ramp_rate=2) assert scmc.cart.x._fly_info is None await scmc.cart.x.prepare(fly_info) assert scmc.cart.x._fly_info is fly_info @@ -215,7 +215,7 @@ async def test_scmc_axis_kickoff_and_complete_raises_error_without_prepare( with pytest.raises(RuntimeError): await scmc.cart.x.complete() - fly_info = FlyMagnetInfo(start_position=1, end_position=2, ramp_rate=4) + fly_info = FlyMagnetInfo(start_position=1, end_position=2, ramp_rate=1) await scmc.cart.x.prepare(fly_info) await scmc.cart.x.kickoff() await scmc.cart.x.complete() From f8b281ed427bcd403d8c49e7c6b43f997c50af8d Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 15 Jul 2026 12:20:25 +0000 Subject: [PATCH 29/30] Rename lakeshore devices to use cooling/heating --- src/dodal/beamlines/i06_1.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dodal/beamlines/i06_1.py b/src/dodal/beamlines/i06_1.py index e9f94384152..1bbb5e8cb5c 100644 --- a/src/dodal/beamlines/i06_1.py +++ b/src/dodal/beamlines/i06_1.py @@ -21,12 +21,12 @@ @devices.factory() -def ls336() -> Lakeshore336: +def ls336_cooling() -> Lakeshore336: return Lakeshore336(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-02:") @devices.factory() -def ls336_2() -> Lakeshore336: +def ls336_heating() -> Lakeshore336: return Lakeshore336(prefix=f"{PREFIX.beamline_prefix}-EA-TCTRL-03:") From 928f0adf9a47595f25e2c3b8b58a773cde993cf3 Mon Sep 17 00:00:00 2001 From: Oli Wenman Date: Wed, 15 Jul 2026 13:08:05 +0000 Subject: [PATCH 30/30] Add missing code coverage --- .../beamlines/i06_1/magnets/test_movements.py | 7 ++++ .../magnets/test_superconducting_magnet.py | 37 +++++++++++++++++++ .../devices/beamlines/i06_1/magnets/utils.py | 25 ++++++++++++- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/tests/devices/beamlines/i06_1/magnets/test_movements.py b/tests/devices/beamlines/i06_1/magnets/test_movements.py index 53b12aadafe..5d2ac5d90de 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_movements.py +++ b/tests/devices/beamlines/i06_1/magnets/test_movements.py @@ -182,6 +182,13 @@ def test_uniaxial_rejects_other_axes(axis: str, target: MagnetPosition) -> None: UniaxialMovement(axis, limit=5).moves(MagnetPosition(x=0, y=0, z=0), target) +def test_uniaxial_raise_error_when_above_limit() -> None: + current = MagnetPosition(x=0, y=0, z=0) + target = MagnetPosition(x=10, y=0, z=0) + with pytest.raises(MagnetPositionError): + UniaxialMovement("x", limit=5).moves(current, target) + + def test_quadrant_xy_sequence() -> None: current = MagnetPosition(x=1, y=0.5, z=0) target = MagnetPosition(x=1.5, y=1, z=0) diff --git a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py index f04558c63a4..9b6bc81a237 100644 --- a/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py +++ b/tests/devices/beamlines/i06_1/magnets/test_superconducting_magnet.py @@ -1,4 +1,5 @@ import asyncio +import math from unittest.mock import AsyncMock, MagicMock, call, patch import pytest @@ -147,6 +148,42 @@ async def test_scmc_cart_set_using_cartesian( assert phi == pytest.approx(spherical.phi) +@pytest.mark.parametrize( + "axes, values, expected_rho, expected_phi, expected_theta, expected_x, expected_y, expected_z", + [ + (("rho",), (1,), 1, 0, 0, 0, 1, 0), + (("rho", "phi"), (1, 90), 1, 90, 0, 0, 0, 1), + (("rho", "phi", "theta"), (1, 45, 45), 1, 45, 45, -0.5, math.sqrt(2) / 2, 0.5), + ], +) +async def test_scmc_spherical_individual_axis_set( + scmc: SuperConductingMagnetController, + axes: tuple[str], + values: tuple[float], + expected_rho: float, + expected_phi: float, + expected_theta: float, + expected_x: float, + expected_y: float, + expected_z: float, +) -> None: + await scmc.mode.set(MagnetModes.SPHERICAL) + for axis, value in zip(axes, values, strict=True): + await getattr(scmc.sph, axis).set(value) + + await assert_reading( + scmc, + { + "scmc-cart-x": partial_reading(expected_x), + "scmc-cart-y": partial_reading(expected_y), + "scmc-cart-z": partial_reading(expected_z), + "scmc-sph-rho": partial_reading(expected_rho), + "scmc-sph-phi": partial_reading(expected_phi), + "scmc-sph-theta": partial_reading(expected_theta), + }, + ) + + async def test_scmc_raises_error_if_limit_status_is_violation( scmc: SuperConductingMagnetController, ) -> None: diff --git a/tests/devices/beamlines/i06_1/magnets/utils.py b/tests/devices/beamlines/i06_1/magnets/utils.py index 2c9404bae9f..f55e6863044 100644 --- a/tests/devices/beamlines/i06_1/magnets/utils.py +++ b/tests/devices/beamlines/i06_1/magnets/utils.py @@ -1,11 +1,32 @@ +import math + from dodal.devices.beamlines.i06_1.magnets.movement import ( MagnetPosition, MagnetSphericalPosition, ) EXPECTED_CARTESIAN_SPHERICAL_CONVERSION = [ - (MagnetPosition(x=0, y=0, z=1), MagnetSphericalPosition(rho=1, theta=0, phi=90)), - (MagnetPosition(x=-1, y=0, z=0), MagnetSphericalPosition(rho=1, theta=90, phi=90)), (MagnetPosition(x=0, y=1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=0)), + (MagnetPosition(x=0, y=0, z=1), MagnetSphericalPosition(rho=1, theta=0, phi=90)), (MagnetPosition(x=0, y=-1, z=0), MagnetSphericalPosition(rho=1, theta=0, phi=180)), + (MagnetPosition(x=-1, y=0, z=0), MagnetSphericalPosition(rho=1, theta=90, phi=90)), + (MagnetPosition(x=0, y=0, z=-1), MagnetSphericalPosition(rho=1, theta=180, phi=90)), + (MagnetPosition(x=1, y=0, z=0), MagnetSphericalPosition(rho=1, theta=270, phi=90)), + # 45 degree increments + ( + MagnetPosition(x=-math.sqrt(2) / 2, y=0, z=math.sqrt(2) / 2), + MagnetSphericalPosition(rho=1, theta=45, phi=90), + ), + ( + MagnetPosition(x=-math.sqrt(2) / 2, y=0, z=-math.sqrt(2) / 2), + MagnetSphericalPosition(rho=1, theta=135, phi=90), + ), + ( + MagnetPosition(x=math.sqrt(2) / 2, y=0, z=-math.sqrt(2) / 2), + MagnetSphericalPosition(rho=1, theta=225, phi=90), + ), + ( + MagnetPosition(x=math.sqrt(2) / 2, y=0, z=math.sqrt(2) / 2), + MagnetSphericalPosition(rho=1, theta=315, phi=90), + ), ]