From d708df69685a0f67b40b2f31377e498369632056 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 13 Mar 2026 15:15:53 +0000 Subject: [PATCH 1/2] Improved error handling --- src/dodal/devices/beamlines/i15_1/robot.py | 29 ++++++++++++++++++++- tests/devices/beamlines/i15_1/test_robot.py | 25 ++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/dodal/devices/beamlines/i15_1/robot.py b/src/dodal/devices/beamlines/i15_1/robot.py index f63c52bd69..e1938898f0 100644 --- a/src/dodal/devices/beamlines/i15_1/robot.py +++ b/src/dodal/devices/beamlines/i15_1/robot.py @@ -1,7 +1,7 @@ import asyncio from collections.abc import Sequence from dataclasses import dataclass -from enum import StrEnum +from enum import IntEnum, StrEnum from functools import partial from bluesky.protocols import Movable @@ -27,9 +27,23 @@ @dataclass class SampleLocation: + MAX_PUCK = 20 + MAX_POSITION = 22 + puck: int position: int + def __post_init__(self): + if not (1 <= self.puck <= self.MAX_PUCK or self.puck == -1): + raise ValueError( + f"Puck must be between 1 and {self.MAX_PUCK}, got {self.puck}" + ) + + if not (1 <= self.position <= self.MAX_POSITION or self.position == -1): + raise ValueError( + f"position must be between 1 and {self.MAX_POSITION}, got {self.position}" + ) + SAMPLE_LOCATION_EMPTY = SampleLocation(-1, -1) @@ -40,6 +54,11 @@ class ProgramNames(StrEnum): SPINNER = "MOTOR.MB6" +class ErrorCodes(IntEnum): + NO_SAMPLE = 9030 + OK = 0 + + class ProgramRunning(StrictEnum): NO_PROGRAM_RUNNING = "No Program Running" PROGRAM_RUNNING = "Program Running" @@ -182,6 +201,14 @@ async def _load(self, location: SampleLocation): await self._trigger_program_and_wait_for_complete(self.puck_pick) + if ( + int(await self.controller_err_code.get_value()) + == ErrorCodes.NO_SAMPLE.value + ): + raise ValueError( + f"Robot load failed, no sample found at puck {location.puck}, position {location.position}" + ) + await self._load_program_and_wait_for_loaded( self.beam_load_program, ProgramNames.BEAM ) diff --git a/tests/devices/beamlines/i15_1/test_robot.py b/tests/devices/beamlines/i15_1/test_robot.py index 0a2e68b212..5dea5cdcae 100644 --- a/tests/devices/beamlines/i15_1/test_robot.py +++ b/tests/devices/beamlines/i15_1/test_robot.py @@ -237,3 +237,28 @@ async def test_after_robot_unload_new_0_0_is_put_in_index_pvs(robot: Robot): assert await robot.current_sample.puck.get_value() == 0 assert await robot.current_sample.position.get_value() == 0 + + +async def test_given_no_pin_in_location_when_loaded_then_sensible_error( + robot: Robot, +) -> None: + set_mock_value(robot.current_sample.puck, 0) + set_mock_value(robot.current_sample.position, 0) + set_mock_value(robot.controller_err_code, 9030.0) + + set_location = SampleLocation(puck=1, position=2) + with pytest.raises(ValueError) as e: + await robot.set(set_location) + + assert "no sample found at puck 1, position 2" in str(e.value) + + assert await robot.current_sample.puck.get_value() != 1 + assert await robot.current_sample.position.get_value() != 2 + + +@pytest.mark.parametrize("puck, position", [(90, 2), (10, 24), (-9, 8)]) +async def test_when_puck_or_position_out_of_bounds_then_error_raised( + puck: int, position: int +) -> None: + with pytest.raises(ValueError): + SampleLocation(puck, position) From d62fa3dfb33bc06e1274a8c54edd4fffc6d57154 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Fri, 13 Mar 2026 15:55:34 +0000 Subject: [PATCH 2/2] Remove the ability to set the attenuation using floats or ints --- .../devices/beamlines/i15_1/attenuator.py | 20 ++------ .../beamlines/i15_1/test_attenuator.py | 50 ------------------- 2 files changed, 3 insertions(+), 67 deletions(-) diff --git a/src/dodal/devices/beamlines/i15_1/attenuator.py b/src/dodal/devices/beamlines/i15_1/attenuator.py index 76103f74fe..6d6f380e01 100644 --- a/src/dodal/devices/beamlines/i15_1/attenuator.py +++ b/src/dodal/devices/beamlines/i15_1/attenuator.py @@ -17,24 +17,12 @@ class AttenuatorPositions(StrictEnum): TRANS_0_01 = "0.01%" TRANS_0_001 = "0.001%" - @classmethod - def from_float(cls, value): - string_representation = f"{value}%" - try: - return AttenuatorPositions(string_representation) - except ValueError as e: - raise ValueError( - f"{string_representation} is not a valid transmission. Options are {', '.join([a.value for a in AttenuatorPositions])}" - ) from e - - -class Attenuator(StandardReadable, Movable[float | AttenuatorPositions]): + +class Attenuator(StandardReadable, Movable[AttenuatorPositions]): """A device to change the attenuation of the beam. This can be done by doing: - >>> bps.mv(attenuator, 10) - or >>> bps.mv(attenuator, AttenuatorPositions.TRANS_10) Where 10 is the transmission in percent. @@ -51,11 +39,9 @@ def __init__(self, prefix: str, name: str = "") -> None: super().__init__(name) @AsyncStatus.wrap - async def set(self, value: float | AttenuatorPositions): + async def set(self, value: AttenuatorPositions): """Change the transmission to the specified percentage. Will raise ValueError if the percentage is not possible. """ - if isinstance(value, float) or isinstance(value, int): - value = AttenuatorPositions.from_float(value) await self.transmission.set(value) diff --git a/tests/devices/beamlines/i15_1/test_attenuator.py b/tests/devices/beamlines/i15_1/test_attenuator.py index 64b4eebe8a..b83b95b03b 100644 --- a/tests/devices/beamlines/i15_1/test_attenuator.py +++ b/tests/devices/beamlines/i15_1/test_attenuator.py @@ -5,29 +5,6 @@ from dodal.devices.beamlines.i15_1.attenuator import Attenuator, AttenuatorPositions -@pytest.mark.parametrize( - "number, enum", - [ - [100, AttenuatorPositions.TRANS_100], - [10, AttenuatorPositions.TRANS_10], - [1, AttenuatorPositions.TRANS_1], - [0.1, AttenuatorPositions.TRANS_0_1], - [0.01, AttenuatorPositions.TRANS_0_01], - [0.001, AttenuatorPositions.TRANS_0_001], - ], -) -def test_given_valid_position_can_convert_into_enum( - number: int | float, enum: AttenuatorPositions -): - assert AttenuatorPositions.from_float(number) == enum - - -def test_given_invalid_position_then_sensible_error_on_converting_into_enum(): - with pytest.raises(ValueError) as e: - AttenuatorPositions.from_float(2) - assert "100%, 50%," in e.value.args[0] - - @pytest.fixture async def attenuator(): with init_devices(mock=True): @@ -44,33 +21,6 @@ async def test_given_an_attenuator_device_setting_a_valid_position_enum_sets_thi ) -@pytest.mark.parametrize( - "number, enum", - [ - [100, AttenuatorPositions.TRANS_100], - [10, AttenuatorPositions.TRANS_10], - [1, AttenuatorPositions.TRANS_1], - [0.1, AttenuatorPositions.TRANS_0_1], - [0.01, AttenuatorPositions.TRANS_0_01], - [0.001, AttenuatorPositions.TRANS_0_001], - ], -) -async def test_given_an_attenuator_device_setting_a_valid_float_sets_this_to_the_device( - attenuator, number, enum -): - await attenuator.set(number) - get_mock_put(attenuator.transmission).assert_called_once_with(enum) - - -async def test_given_an_attenuator_device_setting_an_invalid_float_raises_and_doesnt_set_device( - attenuator, -): - with pytest.raises(ValueError) as e: - await attenuator.set(2) - assert "100%, 50%," in e.value.args[0] - get_mock_put(attenuator.transmission).assert_not_called() - - async def test_given_attenuator_device_in_position_then_can_read(attenuator): set_mock_value(attenuator.transmission, "100%") await assert_reading(