From 646bc5367699c9953467f3b752a001ffecfbff77 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Thu, 1 Feb 2024 17:47:39 +0000 Subject: [PATCH 01/31] (DiamondLightSource/hyperion#1109) Add PV to read the barcode from the robot --- src/dodal/beamlines/i03.py | 16 ++++++ src/dodal/devices/robot.py | 59 +++++++++++++++++++-- tests/devices/unit_tests/test_bart_robot.py | 24 +++++++++ tests/system_tests/test_i03_system.py | 5 ++ 4 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 tests/devices/unit_tests/test_bart_robot.py diff --git a/src/dodal/beamlines/i03.py b/src/dodal/beamlines/i03.py index 129a2d5522..e773ffab67 100644 --- a/src/dodal/beamlines/i03.py +++ b/src/dodal/beamlines/i03.py @@ -18,6 +18,7 @@ from dodal.devices.oav.pin_image_recognition import PinTipDetection from dodal.devices.panda_fast_grid_scan import PandAFastGridScan from dodal.devices.qbpm1 import QBPM1 +from dodal.devices.robot import BartRobot from dodal.devices.s4_slit_gaps import S4SlitGaps from dodal.devices.sample_shutter import SampleShutter from dodal.devices.smargon import Smargon @@ -433,3 +434,18 @@ def zocalo( wait_for_connection, fake_with_ophyd_sim, ) + + +def robot( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> BartRobot: + """Get the i03 robot device, instantiate it if it hasn't already been. + If this is called when already instantiated in i03, it will return the existing object. + """ + return device_instantiation( + BartRobot, + "robot", + "-MO-ROBOT-01:", + wait_for_connection, + fake_with_ophyd_sim, + ) diff --git a/src/dodal/devices/robot.py b/src/dodal/devices/robot.py index ef7733031c..4824536c24 100644 --- a/src/dodal/devices/robot.py +++ b/src/dodal/devices/robot.py @@ -1,6 +1,57 @@ -from ophyd import Component as Cpt -from ophyd import Device, EpicsSignalRO +from collections import OrderedDict +from typing import Dict, Sequence +from bluesky.protocols import Descriptor, Reading +from ophyd_async.core import StandardReadable +from ophyd_async.epics.signal import epics_signal_r -class BART(Device): - GonioPinSensor = Cpt(EpicsSignalRO, "-MO-ROBOT-01:PIN_MOUNTED") + +class IndexingReadable(StandardReadable): + """Wraps a waveform PV that contains a list of strings into a device where only one + of them is returned when read. + """ + + def __init__( + self, + pv: str, + name="", + index: int = 0, + ) -> None: + """ + Args: + pv (str): The waveform PV that contains a list of strings + index (int, optional): The index to read. Defaults to 0. + """ + self.bare_signal = epics_signal_r(Sequence[str], pv) + self.index = index + super().__init__(name=name) + + async def read(self) -> Dict[str, Reading]: + underlying_read = await self.bare_signal.read() + pv_reading = underlying_read[self.bare_signal.name] + pv_reading["value"] = pv_reading["value"][self.index] + return OrderedDict([(self._name, pv_reading)]) + + async def describe(self) -> dict[str, Descriptor]: + desc = OrderedDict( + [ + ( + self._name, + (await self.bare_signal.describe())[self.bare_signal.name], + ) + ], + ) + return desc + + +class BartRobot(StandardReadable): + """The sample changing robot.""" + + def __init__( + self, + name: str, + prefix: str, + ) -> None: + self.barcode = IndexingReadable(prefix + "BARCODE") + self.gonio_pin_sensor = epics_signal_r(bool, "PIN_MOUNTED") + super().__init__(name=name) diff --git a/tests/devices/unit_tests/test_bart_robot.py b/tests/devices/unit_tests/test_bart_robot.py new file mode 100644 index 0000000000..30ee449f9c --- /dev/null +++ b/tests/devices/unit_tests/test_bart_robot.py @@ -0,0 +1,24 @@ +import pytest +from ophyd_async.core import set_sim_value + +from dodal.devices.robot import BartRobot + + +async def _get_bart_robot() -> BartRobot: + device = BartRobot("robot", "-MO-ROBOT-01:") + await device.connect(sim=True) + return device + + +@pytest.mark.asyncio +async def test_bart_robot_can_be_connected_in_sim_mode(): + device = await _get_bart_robot() + await device.connect(sim=True) + + +@pytest.mark.asyncio +async def test_when_barcode_updates_then_new_barcode_read(): + device = await _get_bart_robot() + expected_barcode = "expected" + set_sim_value(device.barcode.bare_signal, [expected_barcode, "other_barcode"]) + assert (await device.barcode.read())["robot-barcode"]["value"] == expected_barcode diff --git a/tests/system_tests/test_i03_system.py b/tests/system_tests/test_i03_system.py index 565023574f..94062b2d9a 100644 --- a/tests/system_tests/test_i03_system.py +++ b/tests/system_tests/test_i03_system.py @@ -1,5 +1,7 @@ import os +from bluesky.run_engine import RunEngine + from dodal.utils import make_all_devices if __name__ == "__main__": @@ -13,6 +15,9 @@ os.environ["BEAMLINE"] = "i03" from dodal.beamlines import i03 + # Need to start a run engine so that the bluesky event loop is running + RunEngine() + print("Making all i03 devices") make_all_devices(i03) print("Successfully connected") From f07764b2e0d444790d73d2d011ab638371cf6b74 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 6 Feb 2024 14:33:42 +0000 Subject: [PATCH 02/31] (DiamondLightSource#1109) Fix barcode reading after review --- src/dodal/devices/robot.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/robot.py b/src/dodal/devices/robot.py index 4824536c24..0a9b3dcd46 100644 --- a/src/dodal/devices/robot.py +++ b/src/dodal/devices/robot.py @@ -6,7 +6,7 @@ from ophyd_async.epics.signal import epics_signal_r -class IndexingReadable(StandardReadable): +class SingleIndexWaveformReadable(StandardReadable): """Wraps a waveform PV that contains a list of strings into a device where only one of them is returned when read. """ @@ -52,6 +52,6 @@ def __init__( name: str, prefix: str, ) -> None: - self.barcode = IndexingReadable(prefix + "BARCODE") - self.gonio_pin_sensor = epics_signal_r(bool, "PIN_MOUNTED") + self.barcode = SingleIndexWaveformReadable(prefix + "BARCODE") + self.gonio_pin_sensor = epics_signal_r(bool, prefix + "PIN_MOUNTED") super().__init__(name=name) From 061ec0415e966e7c5139756619d710699489b570 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 12 Feb 2024 10:13:42 +0000 Subject: [PATCH 03/31] #325 hold on to and close zocalo connection --- src/dodal/devices/zocalo/zocalo_results.py | 26 ++++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/dodal/devices/zocalo/zocalo_results.py b/src/dodal/devices/zocalo/zocalo_results.py index c29cd77629..2c5f6d3146 100644 --- a/src/dodal/devices/zocalo/zocalo_results.py +++ b/src/dodal/devices/zocalo/zocalo_results.py @@ -12,6 +12,7 @@ from numpy.typing import NDArray from ophyd_async.core import StandardReadable from ophyd_async.core.async_status import AsyncStatus +from workflows.transport.common_transport import CommonTransport from dodal.devices.ophyd_async_utils import create_soft_signal_r from dodal.devices.zocalo.zocalo_interaction import _get_zocalo_connection @@ -77,7 +78,7 @@ def __init__( self.timeout_s = timeout_s self._prefix = prefix self._raw_results_received: Queue = Queue() - self.subscription: Optional[int] = None + self.transport: Optional[CommonTransport] = None self.results = create_soft_signal_r(list[XrcResult], "results", self.name) self.centres_of_mass = create_soft_signal_r( @@ -126,11 +127,10 @@ async def stage(self): @AsyncStatus.wrap async def unstage(self): - transport = _get_zocalo_connection(self.zocalo_environment) - if self.subscription: - LOGGER.info("Disconnecting from Zocalo") - transport.disconnect() - self.subscription = None + LOGGER.info("Disconnecting from Zocalo") + if self.transport: + self.transport.disconnect() + self.transport = None @AsyncStatus.wrap async def trigger(self): @@ -141,7 +141,7 @@ async def trigger(self): "unstaged at the end of the experiment to avoid consuming results not " "meant for it" ) - if not self.subscription: + if not self.transport: LOGGER.warning( msg # AsyncStatus exception messages are poorly propagated, remove after https://github.com/bluesky/ophyd-async/issues/103 ) @@ -214,7 +214,7 @@ async def describe(self) -> dict[str, Descriptor]: ) def _subscribe_to_results(self): - transport = _get_zocalo_connection(self.zocalo_environment) + self.transport = _get_zocalo_connection(self.zocalo_environment) def _receive_result( rw: workflows.recipe.RecipeWrapper, header: dict, message: dict @@ -222,21 +222,23 @@ def _receive_result( LOGGER.info(f"Received {message}") recipe_parameters = rw.recipe_step["parameters"] # type: ignore # this rw is initialised with a message so recipe step is not None LOGGER.info(f"Recipe step parameters: {recipe_parameters}") - transport.ack(header) + self.transport.ack(header) # type: ignore # we create transport here results = message.get("results", []) self._raw_results_received.put( {"results": results, "ispyb_ids": recipe_parameters} ) - self.subscription = workflows.recipe.wrap_subscribe( - transport, + subscription = workflows.recipe.wrap_subscribe( + self.transport, self.channel, _receive_result, acknowledgement=True, allow_non_recipe_messages=False, ) - LOGGER.info(f"Made zocalo queue subscription: {self.subscription}.") + LOGGER.info( + f"Made zocalo queue subscription: {subscription} - stored transport connection {self.transport}." + ) def get_processing_result( From 05ab615825f85caef0d1cdb64ef7d3f512483e40 Mon Sep 17 00:00:00 2001 From: David Perl Date: Mon, 12 Feb 2024 11:06:51 +0000 Subject: [PATCH 04/31] clear devices before i24 test --- tests/beamlines/unit_tests/test_i24.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/beamlines/unit_tests/test_i24.py b/tests/beamlines/unit_tests/test_i24.py index aa985b285b..d6860df919 100644 --- a/tests/beamlines/unit_tests/test_i24.py +++ b/tests/beamlines/unit_tests/test_i24.py @@ -9,6 +9,8 @@ def test_device_creation(): + beamline_utils.BL = "i24" + beamline_utils.clear_devices() devices = make_all_devices(i24, fake_with_ophyd_sim=True) assert len(devices) > 0 for device_name in devices.keys(): From 775781913587b9058063aa4e80ac21e3eac3ea7d Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 12 Feb 2024 17:27:26 +0000 Subject: [PATCH 05/31] (DiamondLightSource/hyperion#1125) Correctly use array data name --- .../oav/pin_image_recognition/__init__.py | 4 +-- .../image_recognition/test_pin_tip_detect.py | 33 ++++++++++++++++--- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/dodal/devices/oav/pin_image_recognition/__init__.py b/src/dodal/devices/oav/pin_image_recognition/__init__.py index c069c30d9e..f0e1eb1329 100644 --- a/src/dodal/devices/oav/pin_image_recognition/__init__.py +++ b/src/dodal/devices/oav/pin_image_recognition/__init__.py @@ -116,8 +116,8 @@ async def _get_tip_position( ) array_reading: dict[str, Reading] = await self.array_data.read() - array_data: NDArray[np.uint8] = array_reading[""]["value"] - timestamp: float = array_reading[""]["timestamp"] + array_data: NDArray[np.uint8] = array_reading[self.array_data.name]["value"] + timestamp: float = array_reading[self.array_data.name]["timestamp"] try: start_time = time.time() diff --git a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py index e5940263ba..e685d5b8fc 100644 --- a/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py +++ b/tests/devices/unit_tests/oav/image_recognition/test_pin_tip_detect.py @@ -1,19 +1,22 @@ import asyncio from unittest.mock import patch +import numpy as np import pytest from ophyd_async.core import set_sim_value from dodal.devices.oav.pin_image_recognition import MxSampleDetect, PinTipDetection +from dodal.devices.oav.pin_image_recognition.utils import SampleLocation EVENT_LOOP = asyncio.new_event_loop() pytest_plugins = ("pytest_asyncio",) +DEVICE_NAME = "pin_tip_detection" async def _get_pin_tip_detection_device() -> PinTipDetection: - device = PinTipDetection("-DI-OAV-01") + device = PinTipDetection("-DI-OAV-01", name=DEVICE_NAME) await device.connect(sim=True) return device @@ -73,10 +76,9 @@ async def test_invalid_processing_func_uses_identity_function(): set_sim_value(device.preprocess_operation, 50) # Invalid index - with patch.object( - MxSampleDetect, "__init__", return_value=None - ) as mock_init, patch.object( - MxSampleDetect, "processArray", return_value=((None, None), None) + with ( + patch.object(MxSampleDetect, "__init__", return_value=None) as mock_init, + patch.object(MxSampleDetect, "processArray", return_value=((None, None), None)), ): await device.read() @@ -87,3 +89,24 @@ async def test_invalid_processing_func_uses_identity_function(): # Assert captured preprocess function is the identitiy function arg = object() assert arg == captured_func(arg) + + +@pytest.mark.asyncio +async def test_given_valid_data_reading_then_used_to_find_location(): + device = await _get_pin_tip_detection_device() + image_array = np.array([1, 2, 3]) + test_sample_location = SampleLocation(100, 200, np.array([]), np.array([])) + set_sim_value(device.array_data, image_array) + + with ( + patch.object(MxSampleDetect, "__init__", return_value=None), + patch.object( + MxSampleDetect, "processArray", return_value=test_sample_location + ) as mock_process_array, + ): + location = await device.read() + + process_call = mock_process_array.call_args[0][0] + assert np.array_equal(process_call, image_array) + assert location[DEVICE_NAME]["value"] == (200, 100) + assert location[DEVICE_NAME]["timestamp"] > 0 From defdc0b0649641fb1dbdbedbc8b12987e088274a Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Mon, 12 Feb 2024 19:25:26 +0000 Subject: [PATCH 06/31] (#329) Fix I24 unit tests --- tests/beamlines/unit_tests/test_i24.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/beamlines/unit_tests/test_i24.py b/tests/beamlines/unit_tests/test_i24.py index aa985b285b..2be285dc73 100644 --- a/tests/beamlines/unit_tests/test_i24.py +++ b/tests/beamlines/unit_tests/test_i24.py @@ -1,13 +1,17 @@ -import sys +import os from unittest.mock import patch from dodal.devices.i24.i24_vgonio import VGonio -with patch.dict("os.environ", {"BEAMLINE": "i24"}, clear=True): +with patch.dict(os.environ, {"BEAMLINE": "i24"}, clear=True): from dodal.beamlines import beamline_utils, i24 from dodal.utils import make_all_devices +def setup_module(): + beamline_utils.set_beamline("i24") + + def test_device_creation(): devices = make_all_devices(i24, fake_with_ophyd_sim=True) assert len(devices) > 0 @@ -21,7 +25,5 @@ def test_device_creation(): def teardown_module(): - beamline_utils.BL = "i03" - for module in list(sys.modules): - if module.endswith("beamline_utils"): - del sys.modules[module] + beamline_utils.set_beamline("i03") + beamline_utils.clear_devices() From 19c0a2539d997954780558654a3d04fc24d3506c Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Tue, 13 Feb 2024 10:07:41 +0000 Subject: [PATCH 07/31] Update p38, p45 to use device_instantiation --- src/dodal/beamlines/p38.py | 33 ++++++++++++++--- src/dodal/beamlines/p45.py | 75 ++++++++++++++++++++++++++++---------- 2 files changed, 83 insertions(+), 25 deletions(-) diff --git a/src/dodal/beamlines/p38.py b/src/dodal/beamlines/p38.py index 6bfe6c287b..1c0a30923a 100644 --- a/src/dodal/beamlines/p38.py +++ b/src/dodal/beamlines/p38.py @@ -1,12 +1,33 @@ +from dodal.beamlines.beamline_utils import device_instantiation +from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.areadetector import AdAravisDetector -from dodal.utils import BeamlinePrefix +from dodal.log import set_beamline as set_log_beamline +from dodal.utils import get_beamline_name -PREFIX: str = BeamlinePrefix("p38").beamline_prefix +BL = get_beamline_name("p38") +set_log_beamline(BL) +set_utils_beamline(BL) -def d11(name: str = "D11") -> AdAravisDetector: - return AdAravisDetector(name=name, prefix=f"{PREFIX}-DI-DCAM-03:") +def d11( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> AdAravisDetector: + return device_instantiation( + AdAravisDetector, + "D11", + "-DI-DCAM-03:", + wait_for_connection, + fake_with_ophyd_sim, + ) -def d12(name: str = "D12") -> AdAravisDetector: - return AdAravisDetector(name=name, prefix=f"{PREFIX}-DI-DCAM-04:") +def d12( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> AdAravisDetector: + return device_instantiation( + AdAravisDetector, + "D12", + "-DI-DCAM-04:", + wait_for_connection, + fake_with_ophyd_sim, + ) diff --git a/src/dodal/beamlines/p45.py b/src/dodal/beamlines/p45.py index 4210cdf1c9..f308318a8f 100644 --- a/src/dodal/beamlines/p45.py +++ b/src/dodal/beamlines/p45.py @@ -1,21 +1,58 @@ +from dodal.beamlines.beamline_utils import device_instantiation +from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.devices.areadetector import AdAravisDetector from dodal.devices.p45 import Choppers, TomoStageWithStretchAndSkew -from dodal.utils import BeamlinePrefix - -PREFIX: str = BeamlinePrefix("p45").beamline_prefix - - -def sample(name: str = "sample_stage") -> TomoStageWithStretchAndSkew: - return TomoStageWithStretchAndSkew(name=name, prefix=f"{PREFIX}-MO-STAGE-01:") - - -def choppers(name: str = "chopper") -> Choppers: - return Choppers(name=name, prefix=f"{PREFIX}-MO-CHOP-01:") - - -def det(name: str = "det") -> AdAravisDetector: - return AdAravisDetector(name=name, prefix=f"{PREFIX}-EA-MAP-01:") - - -def diff(name: str = "diff") -> AdAravisDetector: - return AdAravisDetector(name=name, prefix=f"{PREFIX}-EA-DIFF-01:") +from dodal.log import set_beamline as set_log_beamline +from dodal.utils import get_beamline_name + +BL = get_beamline_name("p45") +set_log_beamline(BL) +set_utils_beamline(BL) + + +def sample( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> TomoStageWithStretchAndSkew: + return device_instantiation( + TomoStageWithStretchAndSkew, + "sample_stage", + "-MO-STAGE-01:", + wait_for_connection, + fake_with_ophyd_sim, + ) + + +def choppers( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> Choppers: + return device_instantiation( + Choppers, + "chopper", + "-MO-CHOP-01:", + wait_for_connection, + fake_with_ophyd_sim, + ) + + +def det( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> AdAravisDetector: + return device_instantiation( + AdAravisDetector, + "det", + "-EA-MAP-01:", + wait_for_connection, + fake_with_ophyd_sim, + ) + + +def diff( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> AdAravisDetector: + return device_instantiation( + AdAravisDetector, + "diff", + "-EA-DIFF-01:", + wait_for_connection, + fake_with_ophyd_sim, + ) From ec88e42c46852e2f4ce1006fc51f32745ab2894b Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Tue, 13 Feb 2024 10:07:41 +0000 Subject: [PATCH 08/31] Add example new beamline to readme --- docs/user/tutorials/get_started.rst | 61 +++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/docs/user/tutorials/get_started.rst b/docs/user/tutorials/get_started.rst index 07ebd244d9..d9e1b0c549 100644 --- a/docs/user/tutorials/get_started.rst +++ b/docs/user/tutorials/get_started.rst @@ -8,6 +8,67 @@ The Purpose of Dodal If you have some code that you think would be useful to be added please do so! + +Creating a new beamline +----------------------- + +A beamline is a collection of devices that can be used together to run experiments, they may be read-only or capable of being set. +They include motors in the experiment hutch, optical components in the optics hutch, the synchrotron "machine" and more. + +The following example creates a fictitious beamline `w41`, with a simulated twin `s41`. +`w41` wishes to monitor the status of the Synchrotron and has a AdAravisDetector +`s41` has a simulated clone of the AdAravisDetector, but not of the Synchrotron machine + +```python +from dodal.beamlines.beamline_utils import device_instantiation +from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline +from dodal.devices.areadetector.adaravis import AdAravisDetector +from dodal.devices.synchrotron import Synchrotron +from dodal.log import set_beamline as set_log_beamline +from dodal.utils import get_beamline_name, skip_device + +BL = get_beamline_name("s41") # Default used when not on a live beamline +set_log_beamline(BL) # Configure logging and util functions +set_utils_beamline(BL) + + +""" +Define device factory functions below this point. +A device factory function is any function that has a return type of a Device +A Device must conform to one or more Ophyd Protocols, e.g. Readable. +""" + + +""" +A valid factory function which is: +- instantiated only on the live beamline +- a maximum of once +- can optionally be faked with ophyd simulated axes +- can optionally be connected concurrently by not waiting for connect to complete +- if constructor took a prefix, could optionally exclude the BLIXX prefix +"""" +@skip_device(lambda: BL == "s41") # Conditionally do not instantiate this device +def synchrotron( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> Synchrotron: + """Calls the Synchrotron class's constructor with name="synchrotron", prefix="" + If this is called when already instantiated, it will return the existing object. + """ + return device_instantiation( + Synchrotron, + "synchrotron", + "", + wait_for_connection, + fake_with_ophyd_sim, + bl_prefix=False, + ) + +# Also a valid device factory function +def d11(name: str = "D11") -> AdAravisDetector: + return AdAravisDetector(name=name, prefix=f"{BL}-DI-DCAM-01:") + +``` + Using Devices ------------- From 0b6c2cf74746ce00e1335b2746b3d1e92473fa67 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Tue, 13 Feb 2024 10:07:41 +0000 Subject: [PATCH 09/31] Add tests for modified p38, p45 --- tests/beamlines/unit_tests/test_p38.py | 10 ++++++++++ tests/beamlines/unit_tests/test_p45.py | 10 ++++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/beamlines/unit_tests/test_p38.py create mode 100644 tests/beamlines/unit_tests/test_p45.py diff --git a/tests/beamlines/unit_tests/test_p38.py b/tests/beamlines/unit_tests/test_p38.py new file mode 100644 index 0000000000..89ff2e37ee --- /dev/null +++ b/tests/beamlines/unit_tests/test_p38.py @@ -0,0 +1,10 @@ +from dodal.beamlines import beamline_utils, p38 +from dodal.utils import make_all_devices + + +def test_device_creation(): + beamline_utils.clear_devices() + devices = make_all_devices(p38, fake_with_ophyd_sim=True) + for device_name in devices.keys(): + assert device_name in beamline_utils.ACTIVE_DEVICES + assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) diff --git a/tests/beamlines/unit_tests/test_p45.py b/tests/beamlines/unit_tests/test_p45.py new file mode 100644 index 0000000000..f183513299 --- /dev/null +++ b/tests/beamlines/unit_tests/test_p45.py @@ -0,0 +1,10 @@ +from dodal.beamlines import beamline_utils, p45 +from dodal.utils import make_all_devices + + +def test_device_creation(): + beamline_utils.clear_devices() + devices = make_all_devices(p45, fake_with_ophyd_sim=True) + for device_name in devices.keys(): + assert device_name in beamline_utils.ACTIVE_DEVICES + assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) From 449e3f6a5ad21ef062860d3e5fec23877bb6efb6 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Tue, 13 Feb 2024 10:08:13 +0000 Subject: [PATCH 10/31] Unify common test behaviour - Prevents issue with RunEngine event_loop not being available - Reduces duplication - Expandable to new beamlines --- docs/user/tutorials/get_started.rst | 14 +++++--- .../unit_tests/test_device_instantiation.py | 36 +++++++++++++++++++ tests/beamlines/unit_tests/test_i03.py | 17 --------- tests/beamlines/unit_tests/test_i04.py | 10 ------ tests/beamlines/unit_tests/test_p38.py | 10 ------ tests/beamlines/unit_tests/test_p45.py | 10 ------ 6 files changed, 46 insertions(+), 51 deletions(-) create mode 100644 tests/beamlines/unit_tests/test_device_instantiation.py delete mode 100644 tests/beamlines/unit_tests/test_i04.py delete mode 100644 tests/beamlines/unit_tests/test_p38.py delete mode 100644 tests/beamlines/unit_tests/test_p45.py diff --git a/docs/user/tutorials/get_started.rst b/docs/user/tutorials/get_started.rst index d9e1b0c549..3a7aa729b0 100644 --- a/docs/user/tutorials/get_started.rst +++ b/docs/user/tutorials/get_started.rst @@ -16,8 +16,8 @@ A beamline is a collection of devices that can be used together to run experimen They include motors in the experiment hutch, optical components in the optics hutch, the synchrotron "machine" and more. The following example creates a fictitious beamline `w41`, with a simulated twin `s41`. -`w41` wishes to monitor the status of the Synchrotron and has a AdAravisDetector -`s41` has a simulated clone of the AdAravisDetector, but not of the Synchrotron machine +`w41` needs to monitor the status of the Synchrotron and has a AdAravisDetector. +`s41` has a simulated clone of the AdAravisDetector, but not of the Synchrotron machine. ```python from dodal.beamlines.beamline_utils import device_instantiation @@ -63,12 +63,18 @@ def synchrotron( bl_prefix=False, ) -# Also a valid device factory function +# Also a valid device factory function, but will not pass all tests! def d11(name: str = "D11") -> AdAravisDetector: return AdAravisDetector(name=name, prefix=f"{BL}-DI-DCAM-01:") - ``` +`w41` should also be added to the list of 'ALL_BEAMLINES' in `tests/beamlines/test_device_instantiation`. +This test checks that the function returns a type that conforms to Bluesky protocols, +that it always returns the same instance of the device and that the arguments passed +into the Device class constructor are valid. + + + Using Devices ------------- diff --git a/tests/beamlines/unit_tests/test_device_instantiation.py b/tests/beamlines/unit_tests/test_device_instantiation.py new file mode 100644 index 0000000000..83b6e9a117 --- /dev/null +++ b/tests/beamlines/unit_tests/test_device_instantiation.py @@ -0,0 +1,36 @@ +from typing import Any + +from dodal.beamlines import beamline_utils, i03, i04, i04_1, i23, i24, p38, p45 +from dodal.utils import BLUESKY_PROTOCOLS, make_all_devices + +ALL_BEAMLINES = {i03, i04, i04_1, i23, i24, p38, p45} + + +def follows_bluesky_protocols(obj: Any) -> bool: + return any((isinstance(obj, protocol) for protocol in BLUESKY_PROTOCOLS)) + + +def test_device_creation(RE): + """ + Ensures that for every beamline all device factories are using valid args + and creating types that conform to Bluesky protocols. + """ + for beamline in ALL_BEAMLINES: + devices = make_all_devices(beamline, fake_with_ophyd_sim=True) + for device_name, device in devices.items(): + assert device_name in beamline_utils.ACTIVE_DEVICES + assert follows_bluesky_protocols(device) + assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) + beamline_utils.clear_devices() + + +def test_devices_are_identical(RE): + """ + Ensures that for every beamline all device functions prevent duplicate instantiation. + """ + for beamline in ALL_BEAMLINES: + devices_a = make_all_devices(beamline, fake_with_ophyd_sim=True) + devices_b = make_all_devices(beamline, fake_with_ophyd_sim=True) + for device_name in devices_a.keys(): + assert devices_a[device_name] is devices_b[device_name] + beamline_utils.clear_devices() diff --git a/tests/beamlines/unit_tests/test_i03.py b/tests/beamlines/unit_tests/test_i03.py index c440eae725..227634811f 100644 --- a/tests/beamlines/unit_tests/test_i03.py +++ b/tests/beamlines/unit_tests/test_i03.py @@ -1,6 +1,5 @@ from dodal.beamlines import beamline_utils, i03 from dodal.devices.aperturescatterguard import AperturePositions, ApertureScatterguard -from dodal.utils import make_all_devices def test_list(): @@ -15,22 +14,6 @@ def test_list(): ] -def test_device_creation(RE): - beamline_utils.clear_devices() - devices = make_all_devices(i03, fake_with_ophyd_sim=True) - for device_name in devices.keys(): - assert device_name in beamline_utils.ACTIVE_DEVICES - assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) - - -def test_devices_are_identical(RE): - beamline_utils.clear_devices() - devices_a = make_all_devices(i03, fake_with_ophyd_sim=True) - devices_b = make_all_devices(i03, fake_with_ophyd_sim=True) - for device_name in devices_a.keys(): - assert devices_a[device_name] is devices_b[device_name] - - def test_getting_second_aperture_scatterguard_gives_valid_device(): beamline_utils.clear_devices() test_positions = AperturePositions( diff --git a/tests/beamlines/unit_tests/test_i04.py b/tests/beamlines/unit_tests/test_i04.py deleted file mode 100644 index 80bc934a45..0000000000 --- a/tests/beamlines/unit_tests/test_i04.py +++ /dev/null @@ -1,10 +0,0 @@ -from dodal.beamlines import beamline_utils, i04 -from dodal.utils import make_all_devices - - -def test_device_creation(): - beamline_utils.clear_devices() - devices = make_all_devices(i04, fake_with_ophyd_sim=True) - for device_name in devices.keys(): - assert device_name in beamline_utils.ACTIVE_DEVICES - assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) diff --git a/tests/beamlines/unit_tests/test_p38.py b/tests/beamlines/unit_tests/test_p38.py deleted file mode 100644 index 89ff2e37ee..0000000000 --- a/tests/beamlines/unit_tests/test_p38.py +++ /dev/null @@ -1,10 +0,0 @@ -from dodal.beamlines import beamline_utils, p38 -from dodal.utils import make_all_devices - - -def test_device_creation(): - beamline_utils.clear_devices() - devices = make_all_devices(p38, fake_with_ophyd_sim=True) - for device_name in devices.keys(): - assert device_name in beamline_utils.ACTIVE_DEVICES - assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) diff --git a/tests/beamlines/unit_tests/test_p45.py b/tests/beamlines/unit_tests/test_p45.py deleted file mode 100644 index f183513299..0000000000 --- a/tests/beamlines/unit_tests/test_p45.py +++ /dev/null @@ -1,10 +0,0 @@ -from dodal.beamlines import beamline_utils, p45 -from dodal.utils import make_all_devices - - -def test_device_creation(): - beamline_utils.clear_devices() - devices = make_all_devices(p45, fake_with_ophyd_sim=True) - for device_name in devices.keys(): - assert device_name in beamline_utils.ACTIVE_DEVICES - assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) From f8513429c6965790eb387283c21b0e8763431876 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Tue, 13 Feb 2024 10:08:50 +0000 Subject: [PATCH 11/31] Lint docs --- docs/user/tutorials/get_started.rst | 110 ++++++++++++++-------------- 1 file changed, 56 insertions(+), 54 deletions(-) diff --git a/docs/user/tutorials/get_started.rst b/docs/user/tutorials/get_started.rst index 3a7aa729b0..4e411ce865 100644 --- a/docs/user/tutorials/get_started.rst +++ b/docs/user/tutorials/get_started.rst @@ -15,66 +15,68 @@ Creating a new beamline A beamline is a collection of devices that can be used together to run experiments, they may be read-only or capable of being set. They include motors in the experiment hutch, optical components in the optics hutch, the synchrotron "machine" and more. -The following example creates a fictitious beamline `w41`, with a simulated twin `s41`. -`w41` needs to monitor the status of the Synchrotron and has a AdAravisDetector. -`s41` has a simulated clone of the AdAravisDetector, but not of the Synchrotron machine. - -```python -from dodal.beamlines.beamline_utils import device_instantiation -from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline -from dodal.devices.areadetector.adaravis import AdAravisDetector -from dodal.devices.synchrotron import Synchrotron -from dodal.log import set_beamline as set_log_beamline -from dodal.utils import get_beamline_name, skip_device - -BL = get_beamline_name("s41") # Default used when not on a live beamline -set_log_beamline(BL) # Configure logging and util functions -set_utils_beamline(BL) - - -""" -Define device factory functions below this point. -A device factory function is any function that has a return type of a Device -A Device must conform to one or more Ophyd Protocols, e.g. Readable. -""" - - -""" -A valid factory function which is: -- instantiated only on the live beamline -- a maximum of once -- can optionally be faked with ophyd simulated axes -- can optionally be connected concurrently by not waiting for connect to complete -- if constructor took a prefix, could optionally exclude the BLIXX prefix -"""" -@skip_device(lambda: BL == "s41") # Conditionally do not instantiate this device -def synchrotron( - wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False -) -> Synchrotron: - """Calls the Synchrotron class's constructor with name="synchrotron", prefix="" - If this is called when already instantiated, it will return the existing object. +The following example creates a fictitious beamline ``w41``, with a simulated twin ``s41``. +``w41`` needs to monitor the status of the Synchrotron and has a AdAravisDetector. +``s41`` has a simulated clone of the AdAravisDetector, but not of the Synchrotron machine. + +.. code-block:: python + + from dodal.beamlines.beamline_utils import device_instantiation + from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline + from dodal.devices.areadetector.adaravis import AdAravisDetector + from dodal.devices.synchrotron import Synchrotron + from dodal.log import set_beamline as set_log_beamline + from dodal.utils import get_beamline_name, skip_device + + BL = get_beamline_name("s41") # Default used when not on a live beamline + set_log_beamline(BL) # Configure logging and util functions + set_utils_beamline(BL) + + """ - return device_instantiation( - Synchrotron, - "synchrotron", - "", - wait_for_connection, - fake_with_ophyd_sim, - bl_prefix=False, - ) - -# Also a valid device factory function, but will not pass all tests! -def d11(name: str = "D11") -> AdAravisDetector: - return AdAravisDetector(name=name, prefix=f"{BL}-DI-DCAM-01:") -``` - -`w41` should also be added to the list of 'ALL_BEAMLINES' in `tests/beamlines/test_device_instantiation`. + Define device factory functions below this point. + A device factory function is any function that has a return type which conforms + to one or more Bluesky Protocols. + """ + + + """ + A valid factory function which is: + - instantiated only on the live beamline + - a maximum of once + - can optionally be faked with ophyd simulated axes + - can optionally be connected concurrently by not waiting for connect to complete + - if constructor took a prefix, could optionally exclude the BLIXX prefix + """" + @skip_device(lambda: BL == "s41") # Conditionally do not instantiate this device + def synchrotron( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False + ) -> Synchrotron: + """Calls the Synchrotron class's constructor with name="synchrotron", prefix="" + If this is called when already instantiated, it will return the existing object. + """ + return device_instantiation( + Synchrotron, + "synchrotron", + "", + wait_for_connection, + fake_with_ophyd_sim, + bl_prefix=False, + ) + + def d11(name: str = "D11") -> AdAravisDetector: + """ + Also a valid Device factory function, but as multiple calls would instantiate + multiple copies of a device, discouraged. + """ + return AdAravisDetector(name=name, prefix=f"{BL}-DI-DCAM-01:") + +``w41`` should also be added to the list of ``ALL_BEAMLINES`` in ``tests/beamlines/test_device_instantiation``. This test checks that the function returns a type that conforms to Bluesky protocols, that it always returns the same instance of the device and that the arguments passed into the Device class constructor are valid. - Using Devices ------------- From 217cc5244f628967389cb2978eeb95978bef7ddd Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Tue, 13 Feb 2024 11:06:33 +0000 Subject: [PATCH 12/31] Move create-beamline into a how-to --- docs/user/how-to/create-beamline.rst | 67 +++++++++++++++++++++++++++ docs/user/index.rst | 1 + docs/user/tutorials/get_started.rst | 69 ---------------------------- 3 files changed, 68 insertions(+), 69 deletions(-) create mode 100644 docs/user/how-to/create-beamline.rst diff --git a/docs/user/how-to/create-beamline.rst b/docs/user/how-to/create-beamline.rst new file mode 100644 index 0000000000..a1ab9c3236 --- /dev/null +++ b/docs/user/how-to/create-beamline.rst @@ -0,0 +1,67 @@ +Creating a new beamline +----------------------- + +A beamline is a collection of devices that can be used together to run experiments, they may be read-only or capable of being set. +They include motors in the experiment hutch, optical components in the optics hutch, the synchrotron "machine" and more. + +The following example creates a fictitious beamline ``w41``, with a simulated twin ``s41``. +``w41`` needs to monitor the status of the Synchrotron and has an AdAravisDetector. +``s41`` has a simulated clone of the AdAravisDetector, but not of the Synchrotron machine. + +.. code-block:: python + + from dodal.beamlines.beamline_utils import device_instantiation + from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline + from dodal.devices.areadetector.adaravis import AdAravisDetector + from dodal.devices.synchrotron import Synchrotron + from dodal.log import set_beamline as set_log_beamline + from dodal.utils import get_beamline_name, skip_device + + BL = get_beamline_name("s41") # Default used when not on a live beamline + set_log_beamline(BL) # Configure logging and util functions + set_utils_beamline(BL) + + + """ + Define device factory functions below this point. + A device factory function is any function that has a return type which conforms + to one or more Bluesky Protocols. + """ + + + """ + A valid factory function which is: + - instantiated only on the live beamline + - a maximum of once + - can optionally be faked with ophyd simulated axes + - can optionally be connected concurrently by not waiting for connect to complete + - if constructor took a prefix, could optionally exclude the BLIXX prefix + """" + @skip_device(lambda: BL == "s41") # Conditionally do not instantiate this device + def synchrotron( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False + ) -> Synchrotron: + """Calls the Synchrotron class's constructor with name="synchrotron", prefix="" + If this is called when already instantiated, it will return the existing object. + """ + return device_instantiation( + Synchrotron, + "synchrotron", + "", + wait_for_connection, + fake_with_ophyd_sim, + bl_prefix=False, + ) + + def d11(name: str = "D11") -> AdAravisDetector: + """ + Also a valid Device factory function, but as multiple calls would instantiate + multiple copies of a device, discouraged. + """ + return AdAravisDetector(name=name, prefix=f"{BL}-DI-DCAM-01:") + +``w41`` should also be added to the list of ``ALL_BEAMLINES`` in ``tests/beamlines/test_device_instantiation``. +This test checks that the function returns a type that conforms to Bluesky protocols, +that it always returns the same instance of the device and that the arguments passed +into the Device class constructor are valid. + diff --git a/docs/user/index.rst b/docs/user/index.rst index 4619b7114b..c3ba4b7c0c 100644 --- a/docs/user/index.rst +++ b/docs/user/index.rst @@ -27,6 +27,7 @@ side-bar. :maxdepth: 1 how-to/run-container + how-to/create-beamline.rst +++ diff --git a/docs/user/tutorials/get_started.rst b/docs/user/tutorials/get_started.rst index 4e411ce865..07ebd244d9 100644 --- a/docs/user/tutorials/get_started.rst +++ b/docs/user/tutorials/get_started.rst @@ -8,75 +8,6 @@ The Purpose of Dodal If you have some code that you think would be useful to be added please do so! - -Creating a new beamline ------------------------ - -A beamline is a collection of devices that can be used together to run experiments, they may be read-only or capable of being set. -They include motors in the experiment hutch, optical components in the optics hutch, the synchrotron "machine" and more. - -The following example creates a fictitious beamline ``w41``, with a simulated twin ``s41``. -``w41`` needs to monitor the status of the Synchrotron and has a AdAravisDetector. -``s41`` has a simulated clone of the AdAravisDetector, but not of the Synchrotron machine. - -.. code-block:: python - - from dodal.beamlines.beamline_utils import device_instantiation - from dodal.beamlines.beamline_utils import set_beamline as set_utils_beamline - from dodal.devices.areadetector.adaravis import AdAravisDetector - from dodal.devices.synchrotron import Synchrotron - from dodal.log import set_beamline as set_log_beamline - from dodal.utils import get_beamline_name, skip_device - - BL = get_beamline_name("s41") # Default used when not on a live beamline - set_log_beamline(BL) # Configure logging and util functions - set_utils_beamline(BL) - - - """ - Define device factory functions below this point. - A device factory function is any function that has a return type which conforms - to one or more Bluesky Protocols. - """ - - - """ - A valid factory function which is: - - instantiated only on the live beamline - - a maximum of once - - can optionally be faked with ophyd simulated axes - - can optionally be connected concurrently by not waiting for connect to complete - - if constructor took a prefix, could optionally exclude the BLIXX prefix - """" - @skip_device(lambda: BL == "s41") # Conditionally do not instantiate this device - def synchrotron( - wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False - ) -> Synchrotron: - """Calls the Synchrotron class's constructor with name="synchrotron", prefix="" - If this is called when already instantiated, it will return the existing object. - """ - return device_instantiation( - Synchrotron, - "synchrotron", - "", - wait_for_connection, - fake_with_ophyd_sim, - bl_prefix=False, - ) - - def d11(name: str = "D11") -> AdAravisDetector: - """ - Also a valid Device factory function, but as multiple calls would instantiate - multiple copies of a device, discouraged. - """ - return AdAravisDetector(name=name, prefix=f"{BL}-DI-DCAM-01:") - -``w41`` should also be added to the list of ``ALL_BEAMLINES`` in ``tests/beamlines/test_device_instantiation``. -This test checks that the function returns a type that conforms to Bluesky protocols, -that it always returns the same instance of the device and that the arguments passed -into the Device class constructor are valid. - - Using Devices ------------- From 37d479d050fa568bd8c61c31ede46679eda91d0a Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Tue, 13 Feb 2024 12:16:48 +0000 Subject: [PATCH 13/31] Expose global DirectoryProvider for OphydAsync devices --- src/dodal/beamlines/beamline_utils.py | 17 ++++++++++++++++ .../unit_tests/test_beamline_utils.py | 20 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/dodal/beamlines/beamline_utils.py b/src/dodal/beamlines/beamline_utils.py index 43f5bdc335..21a7fd42df 100644 --- a/src/dodal/beamlines/beamline_utils.py +++ b/src/dodal/beamlines/beamline_utils.py @@ -1,10 +1,12 @@ import inspect +import tempfile from typing import Callable, Dict, Final, List, Optional, TypeVar, cast from bluesky.run_engine import call_in_bluesky_event_loop from ophyd import Device as OphydV1Device from ophyd.sim import make_fake_device from ophyd_async.core import Device as OphydV2Device +from ophyd_async.core import DirectoryProvider, StaticDirectoryProvider from ophyd_async.core import wait_for_connection as v2_device_wait_for_connection from dodal.utils import AnyDevice, BeamlinePrefix, skip_device @@ -13,6 +15,7 @@ ACTIVE_DEVICES: Dict[str, AnyDevice] = {} BL = "" +DIRECTORY_PROVIDER: Optional[DirectoryProvider] = None def set_beamline(beamline: str): @@ -115,3 +118,17 @@ def device_instantiation( if post_create: post_create(device_instance) return device_instance + + +def set_directory_provider(provider: DirectoryProvider): + global DIRECTORY_PROVIDER + + DIRECTORY_PROVIDER = provider + + +def get_directory_provider() -> DirectoryProvider: + if DIRECTORY_PROVIDER is None: + set_directory_provider( + StaticDirectoryProvider(tempfile.NamedTemporaryFile().name, "") + ) + return DIRECTORY_PROVIDER diff --git a/tests/beamlines/unit_tests/test_beamline_utils.py b/tests/beamlines/unit_tests/test_beamline_utils.py index 7477287ad2..f7df9985e8 100644 --- a/tests/beamlines/unit_tests/test_beamline_utils.py +++ b/tests/beamlines/unit_tests/test_beamline_utils.py @@ -102,3 +102,23 @@ def test_wait_for_v2_device_connection_passes_through_timeout( beamline_utils._wait_for_connection(device, **kwargs) call_in_bluesky_el.assert_called_once_with(ANY, timeout=expected_timeout) + + +def test_default_directory_provider_is_singleton(): + provider1 = beamline_utils.get_directory_provider() + provider2 = beamline_utils.get_directory_provider() + assert provider1 is provider2 + + +def test_set_directory_provider_is_singleton(): + beamline_utils.set_directory_provider(lambda: "") + provider1 = beamline_utils.get_directory_provider() + provider2 = beamline_utils.get_directory_provider() + assert provider1 is provider2 + + +def test_set_overrides_singleton(): + provider1 = beamline_utils.get_directory_provider() + beamline_utils.set_directory_provider(lambda: "") + provider2 = beamline_utils.get_directory_provider() + assert provider1 is not provider2 From 0f611b71a1f9bb8ec608aad05a1b2e26669e1656 Mon Sep 17 00:00:00 2001 From: David Perl Date: Tue, 13 Feb 2024 16:06:47 +0000 Subject: [PATCH 14/31] #331 fix device tests --- src/dodal/beamlines/beamline_parameters.py | 19 +++--- src/dodal/beamlines/i24.py | 8 ++- src/dodal/devices/DCM.py | 4 +- .../unit_tests/test_device_instantiation.py | 66 ++++++++++++++----- tests/beamlines/unit_tests/test_i24.py | 3 +- 5 files changed, 70 insertions(+), 30 deletions(-) diff --git a/src/dodal/beamlines/beamline_parameters.py b/src/dodal/beamlines/beamline_parameters.py index d680b81459..ae3a64e6aa 100644 --- a/src/dodal/beamlines/beamline_parameters.py +++ b/src/dodal/beamlines/beamline_parameters.py @@ -1,4 +1,4 @@ -from typing import Any, Tuple, cast +from typing import Any, Optional, Tuple, cast from dodal.log import LOGGER from dodal.utils import get_beamline_name @@ -87,11 +87,14 @@ def parse_list(cls, value: str): return list_output -def get_beamline_parameters(): - beamline_name = get_beamline_name("s03") - beamline_param_path = BEAMLINE_PARAMETER_PATHS.get(beamline_name) - if beamline_param_path is None: - raise KeyError( - "No beamline parameter path found, maybe 'BEAMLINE' environment variable is not set!" - ) +def get_beamline_parameters(beamline_param_path: Optional[str] = None): + """Loads the beamline parameters from the specified path, or according to the + environment variable if none is given""" + if not beamline_param_path: + beamline_name = get_beamline_name("s03") + beamline_param_path = BEAMLINE_PARAMETER_PATHS.get(beamline_name) + if beamline_param_path is None: + raise KeyError( + "No beamline parameter path found, maybe 'BEAMLINE' environment variable is not set!" + ) return GDABeamlineParameters.from_file(beamline_param_path) diff --git a/src/dodal/beamlines/i24.py b/src/dodal/beamlines/i24.py index 9d5660a569..4c0e54a683 100644 --- a/src/dodal/beamlines/i24.py +++ b/src/dodal/beamlines/i24.py @@ -10,14 +10,14 @@ from dodal.devices.i24.pmac import PMAC from dodal.devices.oav.oav_detector import OAV, OAVConfigParams from dodal.devices.zebra import Zebra -from dodal.log import set_beamline +from dodal.log import set_beamline as set_log_beamline from dodal.utils import get_beamline_name, skip_device ZOOM_PARAMS_FILE = "/dls_sw/i24/software/gda/config/xml/jCameraManZoomLevels.xml" DISPLAY_CONFIG = "/dls_sw/i24/software/gda_versions/var/display.configuration" BL = get_beamline_name("s24") -set_beamline(BL) +set_log_beamline(BL) set_utils_beamline(BL) @@ -108,7 +108,9 @@ def oav(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> @skip_device(lambda: BL == "s24") -def vgonio(wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False) -> OAV: +def vgonio( + wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False +) -> VGonio: """Get the i24 vgonio device, instantiate it if it hasn't already been. If this is called when already instantiated, it will return the existing object. """ diff --git a/src/dodal/devices/DCM.py b/src/dodal/devices/DCM.py index 706981f600..41b26cbab4 100644 --- a/src/dodal/devices/DCM.py +++ b/src/dodal/devices/DCM.py @@ -15,7 +15,9 @@ def __init__(self, *args, daq_configuration_path: str, **kwargs): ) # I03 configures the DCM Perp as a side effect of applying this fixed value to the DCM Offset after an energy change # Nb this parameter is misleadingly named to confuse you - self.fixed_offset_mm = get_beamline_parameters()["DCM_Perp_Offset_FIXED"] + self.fixed_offset_mm = get_beamline_parameters( + daq_configuration_path + "/domain/beamlineParameters" + )["DCM_Perp_Offset_FIXED"] """ A double crystal monochromator (DCM), used to select the energy of the beam. diff --git a/tests/beamlines/unit_tests/test_device_instantiation.py b/tests/beamlines/unit_tests/test_device_instantiation.py index 83b6e9a117..e5c3378a1f 100644 --- a/tests/beamlines/unit_tests/test_device_instantiation.py +++ b/tests/beamlines/unit_tests/test_device_instantiation.py @@ -1,36 +1,68 @@ +import importlib +import os from typing import Any +from unittest.mock import patch -from dodal.beamlines import beamline_utils, i03, i04, i04_1, i23, i24, p38, p45 +import pytest + +from dodal.beamlines import beamline_utils from dodal.utils import BLUESKY_PROTOCOLS, make_all_devices -ALL_BEAMLINES = {i03, i04, i04_1, i23, i24, p38, p45} +ALL_BEAMLINES = {"i03", "i04", "i04_1", "i23", "i24", "p38", "p45"} + +mock_paths = [ + ("DAQ_CONFIGURATION_PATH", "tests/devices/unit_tests/test_daq_configuration"), + ("ZOOM_PARAMS_FILE", "tests/devices/unit_tests/test_jCameraManZoomLevels.xml"), + ("DISPLAY_CONFIG", "tests/devices/unit_tests/test_display.configuration"), +] +mock_attributes_table = { + "i03": mock_paths, + "s03": mock_paths, + "i04": mock_paths, + "s04": mock_paths, +} def follows_bluesky_protocols(obj: Any) -> bool: return any((isinstance(obj, protocol) for protocol in BLUESKY_PROTOCOLS)) -def test_device_creation(RE): +def mock_bl(beamline): + bl_mod = importlib.import_module("dodal.beamlines." + beamline) + if mock_attributes := mock_attributes_table.get(beamline): + [bl_mod.__setattr__(attr[0], attr[1]) for attr in mock_attributes] + return bl_mod + + +@pytest.mark.parametrize("beamline", ALL_BEAMLINES) +def test_device_creation(RE, beamline): """ Ensures that for every beamline all device factories are using valid args and creating types that conform to Bluesky protocols. """ - for beamline in ALL_BEAMLINES: - devices = make_all_devices(beamline, fake_with_ophyd_sim=True) - for device_name, device in devices.items(): - assert device_name in beamline_utils.ACTIVE_DEVICES - assert follows_bluesky_protocols(device) - assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) - beamline_utils.clear_devices() + for bl in [beamline, "s" + beamline[1:]]: + with patch.dict(os.environ, {"BEAMLINE": bl}, clear=True): + bl_mod = mock_bl(beamline) + devices = make_all_devices(bl_mod, fake_with_ophyd_sim=True) + for device_name, device in devices.items(): + assert device_name in beamline_utils.ACTIVE_DEVICES + assert follows_bluesky_protocols(device) + assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) + beamline_utils.clear_devices() + del bl_mod -def test_devices_are_identical(RE): +@pytest.mark.parametrize("beamline", ALL_BEAMLINES) +def test_devices_are_identical(RE, beamline): """ Ensures that for every beamline all device functions prevent duplicate instantiation. """ - for beamline in ALL_BEAMLINES: - devices_a = make_all_devices(beamline, fake_with_ophyd_sim=True) - devices_b = make_all_devices(beamline, fake_with_ophyd_sim=True) - for device_name in devices_a.keys(): - assert devices_a[device_name] is devices_b[device_name] - beamline_utils.clear_devices() + for bl in [beamline, "s" + beamline[1:]]: + with patch.dict(os.environ, {"BEAMLINE": beamline}, clear=True): + bl_mod = mock_bl(beamline) + devices_a = make_all_devices(bl_mod, fake_with_ophyd_sim=True) + devices_b = make_all_devices(bl_mod, fake_with_ophyd_sim=True) + for device_name in devices_a.keys(): + assert devices_a[device_name] is devices_b[device_name] + beamline_utils.clear_devices() + del bl_mod diff --git a/tests/beamlines/unit_tests/test_i24.py b/tests/beamlines/unit_tests/test_i24.py index 2be285dc73..500ef771c3 100644 --- a/tests/beamlines/unit_tests/test_i24.py +++ b/tests/beamlines/unit_tests/test_i24.py @@ -13,13 +13,14 @@ def setup_module(): def test_device_creation(): + i24.BL = "i24" devices = make_all_devices(i24, fake_with_ophyd_sim=True) assert len(devices) > 0 for device_name in devices.keys(): assert device_name in beamline_utils.ACTIVE_DEVICES assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) - vgonio: VGonio = beamline_utils.ACTIVE_DEVICES["vgonio"] + vgonio: VGonio = beamline_utils.ACTIVE_DEVICES["vgonio"] # type: ignore assert vgonio.prefix == "BL24I-MO-VGON-01:" assert vgonio.kappa.prefix == "BL24I-MO-VGON-01:KAPPA" From ae2690cdf6a2304836870ea46e49b23a37c223dc Mon Sep 17 00:00:00 2001 From: David Perl Date: Tue, 13 Feb 2024 16:07:20 +0000 Subject: [PATCH 15/31] #331 add test data --- .../domain/beamlineParameters | 299 ++++++++++++++++++ .../BeamLineEnergy_DCM_Pitch_converter.txt | 25 ++ .../BeamLineEnergy_DCM_Roll_converter.txt | 7 + 3 files changed, 331 insertions(+) create mode 100644 tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters create mode 100644 tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Pitch_converter.txt create mode 100644 tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Roll_converter.txt diff --git a/tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters b/tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters new file mode 100644 index 0000000000..20e7eca27f --- /dev/null +++ b/tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters @@ -0,0 +1,299 @@ +# +# +BeamLine BL03I + +## BLSE=FB switches between scan alignment and feedback alignment +## by creating bl energy scannable with beamLineSpecificEnergy_FB +## after changing you must restart servers or >>> reset_namespace +BLSE=FB + +## BPFB (Beam Position FeedBack) +## HALF (default) only off during data collection +## FULL only off for XBPM2 during attenuation optimisation, fluo when trans < 2% and wedged MAD +## UNAVAILABLE (not default) prevents xbpm_feedback.py trying to access EPICS IOC that may not be running +BPFB=FULL +## Note: only beamline scientists control whether feedback is enabled +## via the XBPM feedback EDM screen in Synoptic + +# DCM parameters +DCM_Perp_Offset_FIXED = 25.6 +# +# beamstop +# +parked_x = 4.49 +parked_y = -50.0 +parked_y_plate = -50.5 +parked_z = -49.5 +parked_z_robot = 30.0 + +in_beam_z_MIN_START_POS = 60.0 + +in_beam_x_HIGHRES = 1.52 +in_beam_y_HIGHRES = 44.78 +in_beam_z_HIGHRES = 30.0 + +in_beam_x_STANDARD = 1.52 +in_beam_y_STANDARD = 44.78 +in_beam_z_STANDARD = 30.0 + +in_beam_x_LOWRES = 1.52 +in_beam_y_LOWRES = 44.78 +in_beam_z_LOWRES = 48 + +checkCryojet = No +#If is to be moved in by the script. If not Yes then control is handed to the robot on activate script +#To force the cryojet run hutch_utilities.hutch.forceCryoOut() +manualCryojet = Yes + +######################################################### +############# All these need checking! ############ +######################################################### + +#Aperture - Scatterguard positions +# 100 micron ap +miniap_x_LARGE_APERTURE = 2.389 +miniap_y_LARGE_APERTURE = 40.986 +miniap_z_LARGE_APERTURE = 15.8 + +sg_x_LARGE_APERTURE = 5.25 +sg_y_LARGE_APERTURE = 4.43 + +# 50 micron ap +miniap_x_MEDIUM_APERTURE = 2.384 +miniap_y_MEDIUM_APERTURE = 44.967 +miniap_z_MEDIUM_APERTURE = 15.8 +sg_x_MEDIUM_APERTURE = 5.285 +sg_y_MEDIUM_APERTURE = 0.46 + +# 20 micron ap +miniap_x_SMALL_APERTURE = 2.430 +miniap_y_SMALL_APERTURE = 48.974 +miniap_z_SMALL_APERTURE = 15.8 +sg_x_SMALL_APERTURE = 5.3375 +sg_y_SMALL_APERTURE = -3.55 + +# Robot load +miniap_x_ROBOT_LOAD = 2.386 +miniap_y_ROBOT_LOAD = 31.40 +miniap_z_ROBOT_LOAD = 15.8 +sg_x_ROBOT_LOAD = 5.25 +sg_y_ROBOT_LOAD = 4.43 + +# manual mount +miniap_x_MANUAL_LOAD = -4.91 +miniap_y_MANUAL_LOAD = -49.0 +miniap_z_MANUAL_LOAD = -10.0 + +sg_x_MANUAL_LOAD = -4.7 +sg_y_MANUAL_LOAD = 1.8 + +miniap_x_SCIN_MOVE = -4.91 +# prion setting +#miniap_x_SCIN_MOVE = 0.0 +sg_x_SCIN_MOVE = -4.75 + +scin_y_SCIN_IN = 100.855 +scin_y_SCIN_OUT = -0.02 +scin_z_SCIN_IN = 101.5115 + + +scin_z_SCIN_OUT = 0.1 + +#distance to move gonx,y,z when scintillator is put in with standard pins +# For old gonio: +gon_x_SCIN_OUT_DISTANCE = 1.0 +# For SmarGon: +gon_x_SCIN_OUT_DISTANCE_smargon = 1 + +gon_y_SCIN_OUT_DISTANCE = 2.0 +gon_z_SCIN_OUT_DISTANCE = -0.5 + +#CASS motor position tolerances (mm) +miniap_x_tolerance = 0.004 +miniap_y_tolerance = 0.1 +miniap_z_tolerance = 0.1 +sg_x_tolerance = 0.1 +sg_y_tolerance = 0.1 +scin_y_tolerance = 0.1 +scin_z_tolerance = 0.12 +gon_x_tolerance = 0.01 +gon_y_tolerance = 0.1 +gon_z_tolerance = 0.001 +bs_x_tolerance = 0.02 +bs_y_tolerance = 0.005 +bs_z_tolerance = 0.3 +crl_x_tolerance = 0.01 +crl_y_tolerance = 0.01 +crl_pitch_tolerance = 0.01 +crl_yaw_tolerance = 0.01 +sg_y_up_movement_tolerance = 1.0 + +sg_x_timeout = 10 +sg_y_timeout = 10 +miniap_x_timeout = 60 +miniap_y_timeout = 10 +gon_x_timeout = 60 +gon_y_timeout = 30 +gon_z_timeout = 30 +crl_x_timeout = 10 +crl_y_timeout = 10 +crl_pitch_timeout = 10 +crl_yaw_timeout = 10 + +col_inbeam_tolerance = 1.0 + +# robot load collimation table reference positions (mm) +col_parked_tolerance = 1.0 +col_parked_upstream_x = 0.0 +col_parked_downstream_x = 0.0 +col_parked_upstream_y = 0.0 +col_parked_inboard_y = 0.0 +col_parked_outboard_y = 0.0 + +## CRL positions for low and high energy lens sets. Should deliver beam to same position on scintillator. +## Normally should only adjust the low energy set to match the position of the high energy that you've +## already checked on the scintillator screen. + +crl_x_LOWE = -11.78 +crl_y_LOWE = -4.3 +crl_pitch_LOWE = -4.75 +crl_yaw_LOWE = -1.0 + +crl_x_HIGHE = 2.22 +crl_y_HIGHE = -4.30 +crl_pitch_HIGHE = -2.75 +crl_yaw_HIGHE = 0 + + +######################################################### +########## End of new parameters ########### +######################################################### + + +#Beam visualisation parameters +MinBackStopZ = 30.0 +BackStopYsafe = 20.0 +BackStopXyag = -4.8 +BackStopYyag = 17.20 +BackStopZyag = 19.1 +SampleYnormal = 2.65 +SampleYshift = 2.0 +parked_fluo_x = -18.0 +in_beam_fluo_x = 12.0 +move_fluo = Yes +safe_det_z_default = 900 +safe_det_z_sampleChanger = 337 +store_data_collections_in_ispyb = Yes +TakePNGsOfSample = Yes + +#robot requires these values +gonio_parked_x = 0.0 +gonio_parked_y = 0.0 +gonio_parked_z = 0.0 +gonio_parked_omega = 0 +gonio_parked_chi = 0 +gonio_parked_phi = 0 + +# The following used by setupBeamLine script +setupBeamLine_energyStart = 7000.0 +setupBeamLine_energyEnd = 17000.0 +setupBeamLine_energyStep = 500 +setupBeamLine_rollStart = -4 +setupBeamLine_rollEnd = 4 +setupBeamLine_rollSteps = 21 +setupBeamLine_pitchStart = -3.7 +setupBeamLine_pitchEnd = -3.5 +setupBeamLine_pitchSteps = 200 +#values below in microns +beamXCentre = 0 +beamYCentre = 0 +beamXYSettleTime = 6.0 +beamXYTolerance = 5.0 +DataCollection_TurboMode = Yes +#time in seconds. If not set then the default is 0.1 + +#The following are used by beamLineenergy script +beamLineEnergy_rollBeamX 50 +beamLineEnergy_rollBeamY 200 +beamLineEnergy__rollWidth = .2 +beamLineEnergy__rollStep = .02 +beamLineEnergy__pitchWidth = .02 +beamLineEnergy__pitchStep = .002 +beamLineEnergy__fpitchWidth = .02 +beamLineEnergy__fpitchStep = .001 +beamLineEnergy__adjustSlits = No +#dataCollectionMinSampleCurrent = 0.245 +dataCollectionMinSampleCurrent = 0.000 +dataCollectionSampleCurrent qbpm3 + +#Mark is using the following in some test scripts +MinIPin = 1.0 +YAGPin = 1 +RotationAxisPin = 2 +PtPin = 3 +PowderPin = 4 + +iPinInDetZ = 340.0 + +DataCollectionDetX = -7.8504 +DataCollectionDetYaw = 6.499 +DataCollectionDetY = 48.0 + +# StandardEnergy on i03 is 12700eV +StandardEnergy = 12700 + +keyence_max_attempts = 1 +# Move gonio 100 microns, see difference in keyence values +# Then do 100/difference, put that number below +# Sign may change between Smargon and MiniKappa +keyence_slopeYToX = 2.5 +keyence_slopeYToY = -2.5 +keyence_slopeXToZ = 3.23 + +YAGSamX = 1022 +YAGSamY = -98.0 +YAGSamZ = -147 +YAGOmega = 0.0 + +#ipin value must be < ipin_threshold above background for data collection +ipin_threshold = 0.1 + +# energy thresholds for mirror stripes +# - first threshold is between bare/Rh stripes (e.g. 7000) +# - second threshold is between Rh/Pt stripes (e.g. 18000) +mirror_threshold_bare_rh = 6900 +mirror_threshold_rh_pt = 30000 + +# flux conversion factors +flux_factor_no_aperture = 1 +flux_factor_LARGE_APERTURE = 0.738 +flux_factor_MEDIUM_APERTURE = 0.36 +flux_factor_SMALL_APERTURE = 0.084 +flux_factor_no_aperture_plate = 1 +flux_factor_LARGE_APERTURE_plate = 0.738 +flux_factor_MEDIUM_APERTURE_plate = 0.36 +flux_factor_SMALL_APERTURE_plate = 0.084 + +# assuming gain 10^3 +pin_diode_factor = 2.66E19 + +# Fluorescence/Vortex detector settings +attenuation_optimisation_type = deadtime # deadtime or total_counts + +#Deadtime settings +fluorescence_analyser_deadtimeThreshold=0.002 # used by edge scans +fluorescence_spectrum_deadtimeThreshold=0.0005 # used by spectrum + +#Other settings +fluorescence_attenuation_low_roi = 100 +fluorescence_attenuation_high_roi = 2048 +attenuation_optimisation_optimisation_cycles = 10 +attenuation_optimisation_start_transmission = 0.1 # per cent +fluorescence_mca_sca_offset = 400 + +#Total count settings +attenuation_optimisation_multiplier = 2 +attenuation_optimisation_target_count = 2000 +attenuation_optimisation_upper_limit = 50000 +attenuation_optimisation_lower_limit = 20000 + diff --git a/tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Pitch_converter.txt b/tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Pitch_converter.txt new file mode 100644 index 0000000000..449e920f73 --- /dev/null +++ b/tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Pitch_converter.txt @@ -0,0 +1,25 @@ +# Bragg pitch +# Degree values for pitch are interpreted as mrad +# The values cannot change direction. +# last update 2023/06/26 NP +Units Deg mrad +Units Deg Deg +19.24347 -0.79775 +16.40949 -0.78679 +14.31123 -0.77838 +12.69287 -0.77276 +11.40555 -0.77276 +10.35662 -0.77031 +9.48522 -0.76693 +8.95826 -0.76387 +8.74953 -0.76387 +8.12020 -0.76387 +7.57556 -0.76354 +7.09950 -0.76166 +6.67997 -0.76044 +6.30732 -0.75953 +5.97411 -0.75845 +5.67434 -0.75796 +5.40329 -0.75789 +5.15700 -0.75551 +4.93218 -0.75513 diff --git a/tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Roll_converter.txt b/tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Roll_converter.txt new file mode 100644 index 0000000000..329f29f099 --- /dev/null +++ b/tests/devices/unit_tests/test_daq_configuration/lookup/BeamLineEnergy_DCM_Roll_converter.txt @@ -0,0 +1,7 @@ +#Bragg angle against roll( absolute number) +#reloadLookupTables() +# last update 2023/01/19 NP +Units Deg mrad +26.4095 -0.2799 +6.3075 -0.2799 + From 54565137cbe7808c574963f56f7a2383c9cdb40a Mon Sep 17 00:00:00 2001 From: David Perl Date: Tue, 13 Feb 2024 16:57:55 +0000 Subject: [PATCH 16/31] #331 fix mocking of config gile paths --- .../unit_tests/test_beamline_utils.py | 20 +++++++++++-------- .../unit_tests/test_device_instantiation.py | 17 +++------------- tests/conftest.py | 18 +++++++++++++++++ .../devices/unit_tests/test_undulator_dcm.py | 5 +++-- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/tests/beamlines/unit_tests/test_beamline_utils.py b/tests/beamlines/unit_tests/test_beamline_utils.py index 7477287ad2..87ef6e66aa 100644 --- a/tests/beamlines/unit_tests/test_beamline_utils.py +++ b/tests/beamlines/unit_tests/test_beamline_utils.py @@ -13,6 +13,14 @@ from dodal.devices.zebra import Zebra from dodal.utils import make_all_devices +from ...conftest import mock_beamline_module_filepaths + + +@pytest.fixture +def reset_i03(): + beamline_utils.clear_devices() + mock_beamline_module_filepaths("i03", i03) + def test_instantiate_function_makes_supplied_device(): device_types = [Zebra, ApertureScatterguard, Smargon] @@ -24,8 +32,7 @@ def test_instantiate_function_makes_supplied_device(): assert isinstance(dev, device) -def test_instantiating_different_device_with_same_name(): - beamline_utils.clear_devices() +def test_instantiating_different_device_with_same_name(reset_i03): dev1 = beamline_utils.device_instantiation( # noqa Zebra, "device", "", False, False, None ) @@ -43,8 +50,7 @@ def test_instantiating_different_device_with_same_name(): assert dev2 in beamline_utils.ACTIVE_DEVICES.values() -def test_instantiate_function_fake_makes_fake(): - beamline_utils.clear_devices() +def test_instantiate_function_fake_makes_fake(reset_i03): fake_zeb: Zebra = beamline_utils.device_instantiation( i03.Zebra, "zebra", "", True, True, None ) @@ -52,8 +58,8 @@ def test_instantiate_function_fake_makes_fake(): assert isinstance(fake_zeb.pc.arm_source, FakeEpicsSignal) -def test_clear_devices(RE): - beamline_utils.clear_devices() +def test_clear_devices(RE, reset_i03): + mock_beamline_module_filepaths("i03", i03) devices = make_all_devices(i03, fake_with_ophyd_sim=True) assert len(beamline_utils.ACTIVE_DEVICES) == len(devices.keys()) beamline_utils.clear_devices() @@ -61,8 +67,6 @@ def test_clear_devices(RE): def test_device_is_new_after_clearing(RE): - beamline_utils.clear_devices() - def _make_devices_and_get_id(): return [ id(device) diff --git a/tests/beamlines/unit_tests/test_device_instantiation.py b/tests/beamlines/unit_tests/test_device_instantiation.py index e5c3378a1f..648d674325 100644 --- a/tests/beamlines/unit_tests/test_device_instantiation.py +++ b/tests/beamlines/unit_tests/test_device_instantiation.py @@ -8,19 +8,9 @@ from dodal.beamlines import beamline_utils from dodal.utils import BLUESKY_PROTOCOLS, make_all_devices -ALL_BEAMLINES = {"i03", "i04", "i04_1", "i23", "i24", "p38", "p45"} +from ...conftest import mock_beamline_module_filepaths -mock_paths = [ - ("DAQ_CONFIGURATION_PATH", "tests/devices/unit_tests/test_daq_configuration"), - ("ZOOM_PARAMS_FILE", "tests/devices/unit_tests/test_jCameraManZoomLevels.xml"), - ("DISPLAY_CONFIG", "tests/devices/unit_tests/test_display.configuration"), -] -mock_attributes_table = { - "i03": mock_paths, - "s03": mock_paths, - "i04": mock_paths, - "s04": mock_paths, -} +ALL_BEAMLINES = {"i03", "i04", "i04_1", "i23", "i24", "p38", "p45"} def follows_bluesky_protocols(obj: Any) -> bool: @@ -29,8 +19,7 @@ def follows_bluesky_protocols(obj: Any) -> bool: def mock_bl(beamline): bl_mod = importlib.import_module("dodal.beamlines." + beamline) - if mock_attributes := mock_attributes_table.get(beamline): - [bl_mod.__setattr__(attr[0], attr[1]) for attr in mock_attributes] + mock_beamline_module_filepaths(beamline, bl_mod) return bl_mod diff --git a/tests/conftest.py b/tests/conftest.py index 815a14d4ed..3830a24463 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,24 @@ from dodal.devices.focusing_mirror import VFMMirrorVoltages from dodal.log import LOGGER, GELFTCPHandler, set_up_logging_handlers +MOCK_DAQ_CONFIG_PATH = "tests/devices/unit_tests/test_daq_configuration" +mock_paths = [ + ("DAQ_CONFIGURATION_PATH", MOCK_DAQ_CONFIG_PATH), + ("ZOOM_PARAMS_FILE", "tests/devices/unit_tests/test_jCameraManZoomLevels.xml"), + ("DISPLAY_CONFIG", "tests/devices/unit_tests/test_display.configuration"), +] +mock_attributes_table = { + "i03": mock_paths, + "s03": mock_paths, + "i04": mock_paths, + "s04": mock_paths, +} + + +def mock_beamline_module_filepaths(bl_name, bl_module): + if mock_attributes := mock_attributes_table.get(bl_name): + [bl_module.__setattr__(attr[0], attr[1]) for attr in mock_attributes] + def pytest_runtest_setup(item): beamline_utils.clear_devices() diff --git a/tests/devices/unit_tests/test_undulator_dcm.py b/tests/devices/unit_tests/test_undulator_dcm.py index 5893b29125..69679936d4 100644 --- a/tests/devices/unit_tests/test_undulator_dcm.py +++ b/tests/devices/unit_tests/test_undulator_dcm.py @@ -5,7 +5,6 @@ from ophyd.sim import make_fake_device from ophyd.status import Status -from dodal.beamlines.i03 import DAQ_CONFIGURATION_PATH from dodal.devices.DCM import DCM from dodal.devices.undulator import Undulator, UndulatorGapAccess from dodal.devices.undulator_dcm import ( @@ -15,6 +14,8 @@ _get_energy_distance_table, ) +from ...conftest import MOCK_DAQ_CONFIG_PATH + @pytest.fixture def fake_undulator_dcm() -> UndulatorDCM: @@ -23,7 +24,7 @@ def fake_undulator_dcm() -> UndulatorDCM: lookup_table_path="./tests/devices/unit_tests/test_beamline_undulator_to_gap_lookup_table.txt", ) dcm: DCM = make_fake_device(DCM)( - name="dcm", daq_configuration_path=DAQ_CONFIGURATION_PATH + name="dcm", daq_configuration_path=MOCK_DAQ_CONFIG_PATH ) undulator_dcm: UndulatorDCM = make_fake_device(UndulatorDCM)( undulator, dcm, name="undulator_dcm" From b6ea322d42ff5502062b84b78fa7d8fc37f1ca52 Mon Sep 17 00:00:00 2001 From: David Perl Date: Tue, 13 Feb 2024 17:12:33 +0000 Subject: [PATCH 17/31] #331 add inits for pytest --- tests/beamlines/__init__.py | 0 tests/beamlines/unit_tests/__init__.py | 0 tests/devices/i04/__init__.py | 0 tests/devices/unit_tests/i24/__init__.py | 0 tests/unit_tests/__init__.py | 0 5 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/beamlines/__init__.py create mode 100644 tests/beamlines/unit_tests/__init__.py create mode 100644 tests/devices/i04/__init__.py create mode 100644 tests/devices/unit_tests/i24/__init__.py create mode 100644 tests/unit_tests/__init__.py diff --git a/tests/beamlines/__init__.py b/tests/beamlines/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/beamlines/unit_tests/__init__.py b/tests/beamlines/unit_tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/devices/i04/__init__.py b/tests/devices/i04/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/devices/unit_tests/i24/__init__.py b/tests/devices/unit_tests/i24/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit_tests/__init__.py b/tests/unit_tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 From de66891d214071cbc6146a6862c8e6d6a6086c85 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 13 Feb 2024 18:03:11 +0000 Subject: [PATCH 18/31] (#325) Added test to confirm zocalo connections get cleaned up --- .../system_tests/test_zocalo_results.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/devices/system_tests/test_zocalo_results.py b/tests/devices/system_tests/test_zocalo_results.py index 918328ec39..18849f0c46 100644 --- a/tests/devices/system_tests/test_zocalo_results.py +++ b/tests/devices/system_tests/test_zocalo_results.py @@ -1,6 +1,8 @@ import asyncio +import os import bluesky.plan_stubs as bps +import psutil import pytest import pytest_asyncio from bluesky.preprocessors import stage_decorator @@ -99,3 +101,24 @@ def plan_with_stage(): # But waiting for stage should clear them RE(bps.stage(zocalo_device, wait=True)) assert zocalo_device._raw_results_received.empty() + + +@pytest.mark.s03 +@pytest.mark.asyncio +async def test_stale_connections_closed_after_unstage( + zocalo_device: ZocaloResults, +): + this_process = psutil.Process(os.getpid()) + + connections_before = len(this_process.connections()) + + def stage_unstage(): + yield from bps.stage(zocalo_device) + yield from bps.unstage(zocalo_device) + + RE = RunEngine() + RE(stage_unstage()) + + connections_after = len(this_process.connections()) + + assert connections_before == connections_after From 40b83a06dfd804d6e174d0949e3ee87460f2aa40 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 13 Feb 2024 18:06:09 +0000 Subject: [PATCH 19/31] (#325) Added psutil dependency for tests --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 08056ec8ae..612197bb09 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,6 +40,7 @@ dev = [ "mockito", "pipdeptree", "pre-commit", + "psutil", "pydata-sphinx-theme>=0.12", "pytest", "pytest-asyncio", From 4f36cf45b1bc2e21f52b3fb12adac36a003f3175 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 12:10:29 +0000 Subject: [PATCH 20/31] #325 remove i24 test changes --- tests/beamlines/unit_tests/test_i24.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/beamlines/unit_tests/test_i24.py b/tests/beamlines/unit_tests/test_i24.py index a5d148f836..2be285dc73 100644 --- a/tests/beamlines/unit_tests/test_i24.py +++ b/tests/beamlines/unit_tests/test_i24.py @@ -13,8 +13,6 @@ def setup_module(): def test_device_creation(): - beamline_utils.BL = "i24" - beamline_utils.clear_devices() devices = make_all_devices(i24, fake_with_ophyd_sim=True) assert len(devices) > 0 for device_name in devices.keys(): From b2c71b352ea3f6ec02db24a8b526c9630d40a19e Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 12:27:41 +0000 Subject: [PATCH 21/31] #331 only run tests once for each BL and include skipped --- src/dodal/utils.py | 14 +++++-- .../unit_tests/test_device_instantiation.py | 42 ++++++++++--------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/dodal/utils.py b/src/dodal/utils.py index 893ccc61d8..2becfb1755 100644 --- a/src/dodal/utils.py +++ b/src/dodal/utils.py @@ -110,7 +110,7 @@ def wrapper(*args, **kwds) -> T: def make_all_devices( - module: Union[str, ModuleType, None] = None, **kwargs + module: Union[str, ModuleType, None] = None, include_skipped: bool = False, **kwargs ) -> Dict[str, AnyDevice]: """Makes all devices in the given beamline module. @@ -126,7 +126,7 @@ def make_all_devices( """ if isinstance(module, str) or module is None: module = import_module(module or __name__) - factories = collect_factories(module) + factories = collect_factories(module, include_skipped) devices: dict[str, AnyDevice] = invoke_factories(factories, **kwargs) return devices @@ -167,11 +167,17 @@ def extract_dependencies( yield name -def collect_factories(module: ModuleType) -> dict[str, AnyDeviceFactory]: +def collect_factories( + module: ModuleType, include_skipped: bool = False +) -> dict[str, AnyDeviceFactory]: factories: dict[str, AnyDeviceFactory] = {} for var in module.__dict__.values(): - if callable(var) and is_any_device_factory(var) and not _is_device_skipped(var): + if ( + callable(var) + and is_any_device_factory(var) + and (include_skipped or not _is_device_skipped(var)) + ): factories[var.__name__] = var return factories diff --git a/tests/beamlines/unit_tests/test_device_instantiation.py b/tests/beamlines/unit_tests/test_device_instantiation.py index 648d674325..c8330d860e 100644 --- a/tests/beamlines/unit_tests/test_device_instantiation.py +++ b/tests/beamlines/unit_tests/test_device_instantiation.py @@ -29,16 +29,17 @@ def test_device_creation(RE, beamline): Ensures that for every beamline all device factories are using valid args and creating types that conform to Bluesky protocols. """ - for bl in [beamline, "s" + beamline[1:]]: - with patch.dict(os.environ, {"BEAMLINE": bl}, clear=True): - bl_mod = mock_bl(beamline) - devices = make_all_devices(bl_mod, fake_with_ophyd_sim=True) - for device_name, device in devices.items(): - assert device_name in beamline_utils.ACTIVE_DEVICES - assert follows_bluesky_protocols(device) - assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) - beamline_utils.clear_devices() - del bl_mod + with patch.dict(os.environ, {"BEAMLINE": beamline}, clear=True): + bl_mod = mock_bl(beamline) + devices = make_all_devices( + bl_mod, include_skipped=True, fake_with_ophyd_sim=True + ) + for device_name, device in devices.items(): + assert device_name in beamline_utils.ACTIVE_DEVICES + assert follows_bluesky_protocols(device) + assert len(beamline_utils.ACTIVE_DEVICES) == len(devices) + beamline_utils.clear_devices() + del bl_mod @pytest.mark.parametrize("beamline", ALL_BEAMLINES) @@ -46,12 +47,15 @@ def test_devices_are_identical(RE, beamline): """ Ensures that for every beamline all device functions prevent duplicate instantiation. """ - for bl in [beamline, "s" + beamline[1:]]: - with patch.dict(os.environ, {"BEAMLINE": beamline}, clear=True): - bl_mod = mock_bl(beamline) - devices_a = make_all_devices(bl_mod, fake_with_ophyd_sim=True) - devices_b = make_all_devices(bl_mod, fake_with_ophyd_sim=True) - for device_name in devices_a.keys(): - assert devices_a[device_name] is devices_b[device_name] - beamline_utils.clear_devices() - del bl_mod + with patch.dict(os.environ, {"BEAMLINE": beamline}, clear=True): + bl_mod = mock_bl(beamline) + devices_a = make_all_devices( + bl_mod, include_skipped=True, fake_with_ophyd_sim=True + ) + devices_b = make_all_devices( + bl_mod, include_skipped=True, fake_with_ophyd_sim=True + ) + for device_name in devices_a.keys(): + assert devices_a[device_name] is devices_b[device_name] + beamline_utils.clear_devices() + del bl_mod From 0029972dd3bcd523b2d899111fa94778cf955806 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 12:32:48 +0000 Subject: [PATCH 22/31] #331 delete some unused bulk from test params file --- .../domain/beamlineParameters | 163 +----------------- 1 file changed, 2 insertions(+), 161 deletions(-) diff --git a/tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters b/tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters index 20e7eca27f..df3951b8fc 100644 --- a/tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters +++ b/tests/devices/unit_tests/test_daq_configuration/domain/beamlineParameters @@ -1,10 +1,8 @@ # # -BeamLine BL03I +BeamLine BL03S -## BLSE=FB switches between scan alignment and feedback alignment -## by creating bl energy scannable with beamLineSpecificEnergy_FB -## after changing you must restart servers or >>> reset_namespace +## Test data for device instantiation BLSE=FB ## BPFB (Beam Position FeedBack) @@ -28,26 +26,6 @@ parked_z_robot = 30.0 in_beam_z_MIN_START_POS = 60.0 -in_beam_x_HIGHRES = 1.52 -in_beam_y_HIGHRES = 44.78 -in_beam_z_HIGHRES = 30.0 - -in_beam_x_STANDARD = 1.52 -in_beam_y_STANDARD = 44.78 -in_beam_z_STANDARD = 30.0 - -in_beam_x_LOWRES = 1.52 -in_beam_y_LOWRES = 44.78 -in_beam_z_LOWRES = 48 - -checkCryojet = No -#If is to be moved in by the script. If not Yes then control is handed to the robot on activate script -#To force the cryojet run hutch_utilities.hutch.forceCryoOut() -manualCryojet = Yes - -######################################################### -############# All these need checking! ############ -######################################################### #Aperture - Scatterguard positions # 100 micron ap @@ -108,137 +86,6 @@ gon_x_SCIN_OUT_DISTANCE_smargon = 1 gon_y_SCIN_OUT_DISTANCE = 2.0 gon_z_SCIN_OUT_DISTANCE = -0.5 -#CASS motor position tolerances (mm) -miniap_x_tolerance = 0.004 -miniap_y_tolerance = 0.1 -miniap_z_tolerance = 0.1 -sg_x_tolerance = 0.1 -sg_y_tolerance = 0.1 -scin_y_tolerance = 0.1 -scin_z_tolerance = 0.12 -gon_x_tolerance = 0.01 -gon_y_tolerance = 0.1 -gon_z_tolerance = 0.001 -bs_x_tolerance = 0.02 -bs_y_tolerance = 0.005 -bs_z_tolerance = 0.3 -crl_x_tolerance = 0.01 -crl_y_tolerance = 0.01 -crl_pitch_tolerance = 0.01 -crl_yaw_tolerance = 0.01 -sg_y_up_movement_tolerance = 1.0 - -sg_x_timeout = 10 -sg_y_timeout = 10 -miniap_x_timeout = 60 -miniap_y_timeout = 10 -gon_x_timeout = 60 -gon_y_timeout = 30 -gon_z_timeout = 30 -crl_x_timeout = 10 -crl_y_timeout = 10 -crl_pitch_timeout = 10 -crl_yaw_timeout = 10 - -col_inbeam_tolerance = 1.0 - -# robot load collimation table reference positions (mm) -col_parked_tolerance = 1.0 -col_parked_upstream_x = 0.0 -col_parked_downstream_x = 0.0 -col_parked_upstream_y = 0.0 -col_parked_inboard_y = 0.0 -col_parked_outboard_y = 0.0 - -## CRL positions for low and high energy lens sets. Should deliver beam to same position on scintillator. -## Normally should only adjust the low energy set to match the position of the high energy that you've -## already checked on the scintillator screen. - -crl_x_LOWE = -11.78 -crl_y_LOWE = -4.3 -crl_pitch_LOWE = -4.75 -crl_yaw_LOWE = -1.0 - -crl_x_HIGHE = 2.22 -crl_y_HIGHE = -4.30 -crl_pitch_HIGHE = -2.75 -crl_yaw_HIGHE = 0 - - -######################################################### -########## End of new parameters ########### -######################################################### - - -#Beam visualisation parameters -MinBackStopZ = 30.0 -BackStopYsafe = 20.0 -BackStopXyag = -4.8 -BackStopYyag = 17.20 -BackStopZyag = 19.1 -SampleYnormal = 2.65 -SampleYshift = 2.0 -parked_fluo_x = -18.0 -in_beam_fluo_x = 12.0 -move_fluo = Yes -safe_det_z_default = 900 -safe_det_z_sampleChanger = 337 -store_data_collections_in_ispyb = Yes -TakePNGsOfSample = Yes - -#robot requires these values -gonio_parked_x = 0.0 -gonio_parked_y = 0.0 -gonio_parked_z = 0.0 -gonio_parked_omega = 0 -gonio_parked_chi = 0 -gonio_parked_phi = 0 - -# The following used by setupBeamLine script -setupBeamLine_energyStart = 7000.0 -setupBeamLine_energyEnd = 17000.0 -setupBeamLine_energyStep = 500 -setupBeamLine_rollStart = -4 -setupBeamLine_rollEnd = 4 -setupBeamLine_rollSteps = 21 -setupBeamLine_pitchStart = -3.7 -setupBeamLine_pitchEnd = -3.5 -setupBeamLine_pitchSteps = 200 -#values below in microns -beamXCentre = 0 -beamYCentre = 0 -beamXYSettleTime = 6.0 -beamXYTolerance = 5.0 -DataCollection_TurboMode = Yes -#time in seconds. If not set then the default is 0.1 - -#The following are used by beamLineenergy script -beamLineEnergy_rollBeamX 50 -beamLineEnergy_rollBeamY 200 -beamLineEnergy__rollWidth = .2 -beamLineEnergy__rollStep = .02 -beamLineEnergy__pitchWidth = .02 -beamLineEnergy__pitchStep = .002 -beamLineEnergy__fpitchWidth = .02 -beamLineEnergy__fpitchStep = .001 -beamLineEnergy__adjustSlits = No -#dataCollectionMinSampleCurrent = 0.245 -dataCollectionMinSampleCurrent = 0.000 -dataCollectionSampleCurrent qbpm3 - -#Mark is using the following in some test scripts -MinIPin = 1.0 -YAGPin = 1 -RotationAxisPin = 2 -PtPin = 3 -PowderPin = 4 - -iPinInDetZ = 340.0 - -DataCollectionDetX = -7.8504 -DataCollectionDetYaw = 6.499 -DataCollectionDetY = 48.0 - # StandardEnergy on i03 is 12700eV StandardEnergy = 12700 @@ -274,12 +121,6 @@ flux_factor_LARGE_APERTURE_plate = 0.738 flux_factor_MEDIUM_APERTURE_plate = 0.36 flux_factor_SMALL_APERTURE_plate = 0.084 -# assuming gain 10^3 -pin_diode_factor = 2.66E19 - -# Fluorescence/Vortex detector settings -attenuation_optimisation_type = deadtime # deadtime or total_counts - #Deadtime settings fluorescence_analyser_deadtimeThreshold=0.002 # used by edge scans fluorescence_spectrum_deadtimeThreshold=0.0005 # used by spectrum From 9371d277d2afe06b358dbbcffafb3701e8113b5c Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 14:44:13 +0000 Subject: [PATCH 23/31] #334 clean up fast grid scan devicestatuses --- tests/devices/unit_tests/test_gridscan.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/devices/unit_tests/test_gridscan.py b/tests/devices/unit_tests/test_gridscan.py index 7d9537d7e8..4eeedba2f6 100644 --- a/tests/devices/unit_tests/test_gridscan.py +++ b/tests/devices/unit_tests/test_gridscan.py @@ -6,6 +6,7 @@ from mockito import mock, verify, when from mockito.matchers import ANY, ARGS, KWARGS from ophyd.sim import make_fake_device +from ophyd.status import DeviceStatus, Status from dodal.devices.fast_grid_scan import ( FastGridScan, @@ -15,6 +16,13 @@ from dodal.devices.smargon import Smargon +def discard_status(st: Status | DeviceStatus): + try: + st.wait(0.01) + except BaseException: + pass + + @pytest.fixture def fast_grid_scan(): FakeFastGridScan = make_fake_device(FastGridScan) @@ -68,13 +76,15 @@ def run_test_on_complete_watcher( def test_when_new_image_then_complete_watcher_notified(fast_grid_scan: FastGridScan): - run_test_on_complete_watcher(fast_grid_scan, 2, 1, 3 / 4) + status = run_test_on_complete_watcher(fast_grid_scan, 2, 1, 3 / 4) + discard_status(status) def test_given_0_expected_images_then_complete_watcher_correct( fast_grid_scan: FastGridScan, ): - run_test_on_complete_watcher(fast_grid_scan, 0, 1, 0) + status = run_test_on_complete_watcher(fast_grid_scan, 0, 1, 0) + discard_status(status) @pytest.mark.parametrize( From 2c7a578c8a3ffb30db093ff953e7e076b67b9862 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 15:23:21 +0000 Subject: [PATCH 24/31] #334 mock grid scan running and wait for status --- tests/devices/unit_tests/test_gridscan.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/devices/unit_tests/test_gridscan.py b/tests/devices/unit_tests/test_gridscan.py index 4eeedba2f6..4469ea4beb 100644 --- a/tests/devices/unit_tests/test_gridscan.py +++ b/tests/devices/unit_tests/test_gridscan.py @@ -1,3 +1,5 @@ +from typing import Union + import numpy as np import pytest from bluesky import plan_stubs as bps @@ -16,7 +18,7 @@ from dodal.devices.smargon import Smargon -def discard_status(st: Status | DeviceStatus): +def discard_status(st: Union[Status, DeviceStatus]): try: st.wait(0.01) except BaseException: @@ -24,11 +26,12 @@ def discard_status(st: Status | DeviceStatus): @pytest.fixture -def fast_grid_scan(): +def fast_grid_scan(request): FakeFastGridScan = make_fake_device(FastGridScan) - fast_grid_scan: FastGridScan = FakeFastGridScan(name="test fake FGS") + fast_grid_scan: FastGridScan = FakeFastGridScan( + name="test fake FGS: " + request.node.name + ) fast_grid_scan.scan_invalid.pvname = "" - yield fast_grid_scan @@ -335,8 +338,13 @@ def test_given_various_x_y_z_when_get_motor_positions_then_expected_positions_re def test_can_run_fast_grid_scan_in_run_engine(fast_grid_scan): @bpp.run_decorator() def kickoff_and_complete(device): - yield from bps.kickoff(device) - yield from bps.complete(device) + yield from bps.kickoff(device, group="kickoff") + device.status.sim_put(1) + yield from bps.wait("kickoff") + yield from bps.complete(device, group="complete") + device.position_counter.sim_put(device.expected_images) + device.status.sim_put(0) + yield from bps.wait("complete") RE = RunEngine() RE(kickoff_and_complete(fast_grid_scan)) From 8d26f88d6e405d80daaf283e1bba2d7f0505210e Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 15:55:07 +0000 Subject: [PATCH 25/31] #334 clean up eiger test staging statuses --- tests/devices/unit_tests/test_eiger.py | 36 ++++++++++++++------------ 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/devices/unit_tests/test_eiger.py b/tests/devices/unit_tests/test_eiger.py index 0c2021cf52..43146739f8 100644 --- a/tests/devices/unit_tests/test_eiger.py +++ b/tests/devices/unit_tests/test_eiger.py @@ -52,14 +52,26 @@ def create_new_params() -> DetectorParams: @pytest.fixture -def fake_eiger(): +def fake_eiger(request): FakeEigerDetector: EigerDetector = make_fake_device(EigerDetector) fake_eiger: EigerDetector = FakeEigerDetector.with_params( - params=create_new_params(), name="test fake Eiger" + params=create_new_params(), name="test fake Eiger: " + request.node.name ) return fake_eiger +def get_good_status(): + status = Status() + status.set_finished() + return status + + +def mock_eiger_odin_statuses(eiger): + eiger.set_odin_pvs = MagicMock(return_value=get_good_status()) + eiger._wait_for_odin_status = MagicMock(return_value=get_good_status()) + eiger._wait_fan_ready = MagicMock(return_value=get_good_status()) + + def finished_status(): status = Status() status.set_finished() @@ -356,12 +368,14 @@ def test_given_stale_parameters_goes_high_before_callbacks_then_stale_parameters ): mock_await.return_value = Status(done=True) fake_eiger.odin.nodes.clear_odin_errors = MagicMock() - fake_eiger.odin.check_odin_initialised = MagicMock() - fake_eiger.odin.check_odin_initialised.return_value = (True, "") + fake_eiger.odin.check_odin_initialised = MagicMock(return_value=(True, "")) fake_eiger.odin.file_writer.file_path.put(True) + mock_eiger_odin_statuses(fake_eiger) + def wait_on_staging(): - fake_eiger.async_stage() + st = fake_eiger.async_stage() + st.wait() waiting_status = Status() fake_eiger.cam.num_images.set = MagicMock(return_value=waiting_status) @@ -462,20 +476,10 @@ def test_when_stage_called_then_finish_arm_on_fan_ready( range(10), ) def test_check_callback_error(fake_eiger: EigerDetector, iteration): - def get_good_status(): - status = Status() - status.set_finished() - return status - LOGGER.error = MagicMock() # These functions timeout without extra tweaking rather than give us the specific status error for the test - fake_eiger.set_odin_pvs = MagicMock() - fake_eiger.set_odin_pvs.return_value = get_good_status() - fake_eiger._wait_for_odin_status = MagicMock() - fake_eiger._wait_for_odin_status.return_value = get_good_status() - fake_eiger._wait_fan_ready = MagicMock() - fake_eiger._wait_fan_ready.return_value = get_good_status() + mock_eiger_odin_statuses(fake_eiger) unwrapped_funcs = [ ( From 514a5578c89864ff15abe47fb31173dc9c343e6f Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 16:04:20 +0000 Subject: [PATCH 26/31] #334 make fake devices properly --- tests/beamlines/unit_tests/test_beamline_utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/beamlines/unit_tests/test_beamline_utils.py b/tests/beamlines/unit_tests/test_beamline_utils.py index e0e48dc060..7bcb0868df 100644 --- a/tests/beamlines/unit_tests/test_beamline_utils.py +++ b/tests/beamlines/unit_tests/test_beamline_utils.py @@ -27,22 +27,22 @@ def test_instantiate_function_makes_supplied_device(): for device in device_types: beamline_utils.clear_devices() dev = beamline_utils.device_instantiation( - device, "device", "", False, False, None + device, device.__name__, "", False, True, None ) assert isinstance(dev, device) def test_instantiating_different_device_with_same_name(reset_i03): dev1 = beamline_utils.device_instantiation( # noqa - Zebra, "device", "", False, False, None + Zebra, "device", "", False, True, None ) with pytest.raises(TypeError): dev2 = beamline_utils.device_instantiation( - Smargon, "device", "", False, False, None + Smargon, "device", "", False, True, None ) beamline_utils.clear_device("device") dev2 = beamline_utils.device_instantiation( # noqa - Smargon, "device", "", False, False, None + Smargon, "device", "", False, True, None ) assert dev1.name == dev2.name assert type(dev1) != type(dev2) From e0b6a6944602e47fcdf2f3660f892509a461c301 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 14 Feb 2024 16:40:06 +0000 Subject: [PATCH 27/31] #334 use NullStatus where relevant --- .../unit_tests/test_beamline_utils.py | 14 ++++++------ tests/devices/unit_tests/oav/test_oav.py | 8 +++++-- tests/devices/unit_tests/test_eiger.py | 12 +++------- tests/devices/unit_tests/test_utils.py | 22 +++++++++---------- tests/devices/unit_tests/test_xspress3mini.py | 10 +-------- 5 files changed, 27 insertions(+), 39 deletions(-) diff --git a/tests/beamlines/unit_tests/test_beamline_utils.py b/tests/beamlines/unit_tests/test_beamline_utils.py index 7bcb0868df..1042294808 100644 --- a/tests/beamlines/unit_tests/test_beamline_utils.py +++ b/tests/beamlines/unit_tests/test_beamline_utils.py @@ -16,9 +16,7 @@ from ...conftest import mock_beamline_module_filepaths -@pytest.fixture -def reset_i03(): - beamline_utils.clear_devices() +def setup_module(): mock_beamline_module_filepaths("i03", i03) @@ -32,7 +30,8 @@ def test_instantiate_function_makes_supplied_device(): assert isinstance(dev, device) -def test_instantiating_different_device_with_same_name(reset_i03): +def test_instantiating_different_device_with_same_name(): + beamline_utils.clear_devices() dev1 = beamline_utils.device_instantiation( # noqa Zebra, "device", "", False, True, None ) @@ -50,7 +49,8 @@ def test_instantiating_different_device_with_same_name(reset_i03): assert dev2 in beamline_utils.ACTIVE_DEVICES.values() -def test_instantiate_function_fake_makes_fake(reset_i03): +def test_instantiate_function_fake_makes_fake(): + beamline_utils.clear_devices() fake_zeb: Zebra = beamline_utils.device_instantiation( i03.Zebra, "zebra", "", True, True, None ) @@ -58,8 +58,8 @@ def test_instantiate_function_fake_makes_fake(reset_i03): assert isinstance(fake_zeb.pc.arm_source, FakeEpicsSignal) -def test_clear_devices(RE, reset_i03): - mock_beamline_module_filepaths("i03", i03) +def test_clear_devices(RE): + beamline_utils.clear_devices() devices = make_all_devices(i03, fake_with_ophyd_sim=True) assert len(beamline_utils.ACTIVE_DEVICES) == len(devices.keys()) beamline_utils.clear_devices() diff --git a/tests/devices/unit_tests/oav/test_oav.py b/tests/devices/unit_tests/oav/test_oav.py index 60c5f1dcdd..0fe1148996 100644 --- a/tests/devices/unit_tests/oav/test_oav.py +++ b/tests/devices/unit_tests/oav/test_oav.py @@ -53,10 +53,10 @@ def test_when_zoom_level_changed_then_oav_rewired(zoom, expected_plugin, oav: OA def test_when_zoom_level_changed_then_status_waits_for_all_plugins_to_be_updated( oav: OAV, ): - mxsc_status = Status() + mxsc_status = Status(obj="msxc - test_when_zoom_level...") oav.mxsc.input_plugin.set = MagicMock(return_value=mxsc_status) - mjpg_status = Status() + mjpg_status = Status("mjpg - test_when_zoom_level...") oav.snapshot.input_plugin.set = MagicMock(return_value=mjpg_status) full_status = oav.zoom_controller.set("1.0x") @@ -64,6 +64,10 @@ def test_when_zoom_level_changed_then_status_waits_for_all_plugins_to_be_updated assert mxsc_status in full_status assert mjpg_status in full_status + mxsc_status.set_finished() + mjpg_status.set_finished() + full_status.wait() + def test_load_microns_per_pixel_entry_not_found(oav: OAV): with pytest.raises(OAVError_ZoomLevelNotFound): diff --git a/tests/devices/unit_tests/test_eiger.py b/tests/devices/unit_tests/test_eiger.py index 43146739f8..7a179b4328 100644 --- a/tests/devices/unit_tests/test_eiger.py +++ b/tests/devices/unit_tests/test_eiger.py @@ -60,16 +60,10 @@ def fake_eiger(request): return fake_eiger -def get_good_status(): - status = Status() - status.set_finished() - return status - - def mock_eiger_odin_statuses(eiger): - eiger.set_odin_pvs = MagicMock(return_value=get_good_status()) - eiger._wait_for_odin_status = MagicMock(return_value=get_good_status()) - eiger._wait_fan_ready = MagicMock(return_value=get_good_status()) + eiger.set_odin_pvs = MagicMock(return_value=finished_status()) + eiger._wait_for_odin_status = MagicMock(return_value=finished_status()) + eiger._wait_fan_ready = MagicMock(return_value=finished_status()) def finished_status(): diff --git a/tests/devices/unit_tests/test_utils.py b/tests/devices/unit_tests/test_utils.py index 0cb6df5628..ca8e5355a0 100644 --- a/tests/devices/unit_tests/test_utils.py +++ b/tests/devices/unit_tests/test_utils.py @@ -1,7 +1,7 @@ from unittest.mock import MagicMock, patch import pytest -from ophyd.sim import make_fake_device +from ophyd.sim import NullStatus, make_fake_device from ophyd.status import Status from dodal.devices.util.epics_util import SetWhenEnabled, run_functions_without_blocking @@ -30,17 +30,11 @@ def reset_logs(): def get_bad_status(): - status = Status(obj="Dodal test utils - get good status") + status = Status(obj="Dodal test utils - get bad status") status.set_exception(StatusException()) return status -def get_good_status(): - status = Status(obj="Dodal test utils - get good status", timeout=0.1) - status.set_finished() - return status - - def test_run_functions_without_blocking_errors_on_invalid_func(): def bad_func(): return 5 @@ -69,15 +63,19 @@ def test_check_call_back_error_gives_correct_error(): def test_wrap_function_callback(): dummy_func = MagicMock(return_value=Status()) - returned_status = run_functions_without_blocking( - [lambda: get_good_status(), dummy_func] - ) + returned_status = run_functions_without_blocking([lambda: NullStatus(), dummy_func]) discard_status(returned_status) dummy_func.assert_called_once() def test_wrap_function_callback_errors_on_wrong_return_type(): reset_logs() + + def get_good_status(): + status = Status(obj="Dodal test utils - get good status", timeout=0.1) + status.set_finished() + return status + dummy_func = MagicMock(return_value=3) returned_status = Status(done=True, success=True) returned_status = run_functions_without_blocking( @@ -102,7 +100,7 @@ def test_wrap_function_callback_errors_on_wrong_return_type(): def test_status_points_to_provided_device_object(): expected_obj = MagicMock() returned_status = run_functions_without_blocking( - [get_good_status], associated_obj=expected_obj + [NullStatus], associated_obj=expected_obj ) returned_status.wait(0.1) assert returned_status.obj == expected_obj diff --git a/tests/devices/unit_tests/test_xspress3mini.py b/tests/devices/unit_tests/test_xspress3mini.py index 1f47e2c314..fb72658d48 100644 --- a/tests/devices/unit_tests/test_xspress3mini.py +++ b/tests/devices/unit_tests/test_xspress3mini.py @@ -9,14 +9,8 @@ from dodal.devices.xspress3_mini.xspress3_mini import DetectorState, Xspress3Mini -def get_good_status() -> Status: - status = Status() - status.set_finished() - return status - - def get_bad_status() -> Status: - status = Status() + status = Status("get_bad_status") status.set_exception(Exception) return status @@ -51,8 +45,6 @@ def test_stage_in_busy_state(mock_await_value, fake_xspress3mini): def test_stage_fails_in_failed_acquire_state(fake_xspress3mini): - bad_status = Status() - bad_status.set_exception(Exception) RE = RunEngine() with pytest.raises(Exception): RE(bps.stage(fake_xspress3mini)) From 10a16f9623bc6e3972953170e930221de22f3b94 Mon Sep 17 00:00:00 2001 From: Noemi Frisina <54588199+noemifrisina@users.noreply.github.com> Date: Wed, 14 Feb 2024 16:50:37 +0000 Subject: [PATCH 28/31] Add pulses and sources to Zebra device (#299) --- src/dodal/devices/zebra.py | 19 ++++++++++++++++++- tests/beamlines/unit_tests/test_i24.py | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/dodal/devices/zebra.py b/src/dodal/devices/zebra.py index 1f5c4fae1a..3a1066041d 100644 --- a/src/dodal/devices/zebra.py +++ b/src/dodal/devices/zebra.py @@ -33,6 +33,9 @@ AND4 = 35 OR1 = 36 PULSE1 = 52 +PULSE2 = 53 +SOFT_IN1 = 60 +SOFT_IN2 = 61 SOFT_IN3 = 62 # Instrument specific @@ -66,6 +69,11 @@ class ArmDemand(IntEnum): DISARM = 0 +class FastShutterAction(IntEnum): + OPEN = 1 + CLOSE = 0 + + class ArmingDevice(Device): """A useful device that can abstract some of the logic of arming. Allows a user to just call arm.set(ArmDemand.ARM)""" @@ -90,12 +98,14 @@ class PositionCompare(Device): gate_input = epics_signal_put_wait("PC_GATE_INP") gate_width = epics_signal_put_wait("PC_GATE_WID") gate_start = epics_signal_put_wait("PC_GATE_START") + gate_step = epics_signal_put_wait("PC_GATE_STEP") pulse_source = epics_signal_put_wait("PC_PULSE_SEL") pulse_input = epics_signal_put_wait("PC_PULSE_INP") pulse_start = epics_signal_put_wait("PC_PULSE_START") pulse_width = epics_signal_put_wait("PC_PULSE_WID") pulse_step = epics_signal_put_wait("PC_PULSE_STEP") + pulse_max = epics_signal_put_wait("PC_PULSE_MAX") dir = Component(EpicsSignal, "PC_DIR") arm_source = epics_signal_put_wait("PC_ARM_SEL") @@ -107,8 +117,15 @@ def is_armed(self) -> bool: return self.arm.armed.get() == 1 +class PulseOutput(Device): + input = epics_signal_put_wait("_INP") + delay = epics_signal_put_wait("_DLY") + width = epics_signal_put_wait("_WID") + + class ZebraOutputPanel(Device): - pulse_1_input = epics_signal_put_wait("PULSE1_INP") + pulse_1 = Component(PulseOutput, "PULSE1") + pulse_2 = Component(PulseOutput, "PULSE2") out_1 = epics_signal_put_wait("OUT1_TTL") out_2 = epics_signal_put_wait("OUT2_TTL") diff --git a/tests/beamlines/unit_tests/test_i24.py b/tests/beamlines/unit_tests/test_i24.py index 500ef771c3..4223f01931 100644 --- a/tests/beamlines/unit_tests/test_i24.py +++ b/tests/beamlines/unit_tests/test_i24.py @@ -26,5 +26,5 @@ def test_device_creation(): def teardown_module(): - beamline_utils.set_beamline("i03") + beamline_utils.set_beamline("i24") beamline_utils.clear_devices() From 57b732948756944a0ed4f88a1a7f875fd8ced5e4 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 15 Feb 2024 11:14:26 +0000 Subject: [PATCH 29/31] #334 tidy up --- tests/devices/unit_tests/test_eiger.py | 2 +- tests/devices/unit_tests/test_gridscan.py | 2 +- tests/devices/unit_tests/test_panda_gridscan.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/devices/unit_tests/test_eiger.py b/tests/devices/unit_tests/test_eiger.py index 7a179b4328..64c60bd0b7 100644 --- a/tests/devices/unit_tests/test_eiger.py +++ b/tests/devices/unit_tests/test_eiger.py @@ -55,7 +55,7 @@ def create_new_params() -> DetectorParams: def fake_eiger(request): FakeEigerDetector: EigerDetector = make_fake_device(EigerDetector) fake_eiger: EigerDetector = FakeEigerDetector.with_params( - params=create_new_params(), name="test fake Eiger: " + request.node.name + params=create_new_params(), name=f"test fake Eiger: {request.node.name}" ) return fake_eiger diff --git a/tests/devices/unit_tests/test_gridscan.py b/tests/devices/unit_tests/test_gridscan.py index 4469ea4beb..2a5ee6de74 100644 --- a/tests/devices/unit_tests/test_gridscan.py +++ b/tests/devices/unit_tests/test_gridscan.py @@ -29,7 +29,7 @@ def discard_status(st: Union[Status, DeviceStatus]): def fast_grid_scan(request): FakeFastGridScan = make_fake_device(FastGridScan) fast_grid_scan: FastGridScan = FakeFastGridScan( - name="test fake FGS: " + request.node.name + name=f"test fake FGS: {request.node.name}" ) fast_grid_scan.scan_invalid.pvname = "" yield fast_grid_scan diff --git a/tests/devices/unit_tests/test_panda_gridscan.py b/tests/devices/unit_tests/test_panda_gridscan.py index a200f814e3..c3db08245a 100644 --- a/tests/devices/unit_tests/test_panda_gridscan.py +++ b/tests/devices/unit_tests/test_panda_gridscan.py @@ -15,9 +15,9 @@ @pytest.fixture -def fast_grid_scan(): +def fast_grid_scan(request): fast_grid_scan: PandAFastGridScan = instantiate_fake_device( - PandAFastGridScan, name="test fake FGS" + PandAFastGridScan, name=f"test fake FGS: {request.node.name}" ) fast_grid_scan.scan_invalid.pvname = "" From 358c7dbc4f130da6282e05009ef2949d86ed5cd2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:27:00 +0000 Subject: [PATCH 30/31] Bump the github-artifacts group with 2 updates Bumps the github-artifacts group with 2 updates: [actions/upload-artifact](https://github.com/actions/upload-artifact) and [actions/download-artifact](https://github.com/actions/download-artifact). Updates `actions/upload-artifact` from 4.1.0 to 4.3.1 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v4.1.0...v4.3.1) Updates `actions/download-artifact` from 4.1.1 to 4.1.2 - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](https://github.com/actions/download-artifact/compare/v4.1.1...v4.1.2) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-artifacts - dependency-name: actions/download-artifact dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-artifacts ... Signed-off-by: dependabot[bot] --- .github/workflows/code.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/code.yml b/.github/workflows/code.yml index a26b38ef89..9a8954befe 100644 --- a/.github/workflows/code.yml +++ b/.github/workflows/code.yml @@ -92,7 +92,7 @@ jobs: pipx run build - name: Upload sdist and wheel as artifacts - uses: actions/upload-artifact@v4.1.0 + uses: actions/upload-artifact@v4.3.1 with: name: ${{ env.DIST_WHEEL_PATH }} path: dist @@ -136,7 +136,7 @@ jobs: echo "DIST_LOCKFILE_PATH=lockfiles-${{ env.CONTAINER_PYTHON }}-dist-${{ github.sha }}" >> $GITHUB_ENV - name: Download wheel and lockfiles - uses: actions/download-artifact@v4.1.1 + uses: actions/download-artifact@v4.1.2 with: path: artifacts/ pattern: "*dist*" @@ -216,7 +216,7 @@ jobs: steps: - name: Download wheel and lockfiles - uses: actions/download-artifact@v4.1.1 + uses: actions/download-artifact@v4.1.2 with: pattern: "*dist*" From a2dea33a196a891c1d9104df81e62916782610c4 Mon Sep 17 00:00:00 2001 From: David Perl Date: Thu, 15 Feb 2024 12:32:38 +0000 Subject: [PATCH 31/31] #334 clear devices before mocking file paths --- tests/beamlines/unit_tests/test_beamline_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/beamlines/unit_tests/test_beamline_utils.py b/tests/beamlines/unit_tests/test_beamline_utils.py index 1042294808..b977e2283f 100644 --- a/tests/beamlines/unit_tests/test_beamline_utils.py +++ b/tests/beamlines/unit_tests/test_beamline_utils.py @@ -17,6 +17,7 @@ def setup_module(): + beamline_utils.clear_devices() mock_beamline_module_filepaths("i03", i03)