diff --git a/src/dodal/beamlines/i03.py b/src/dodal/beamlines/i03.py index 64aa23b1c85..adcd92d4696 100644 --- a/src/dodal/beamlines/i03.py +++ b/src/dodal/beamlines/i03.py @@ -7,6 +7,7 @@ from ophyd_async.fastcs.panda import HDFPanda from yarl import URL +from dodal.common.beamlines.beamline_parameters import CONFIG_SERVER_URL_ENV_VAR from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.common.beamlines.beamline_utils import set_config_client, set_path_provider from dodal.common.beamlines.commissioning_mode import set_commissioning_signal @@ -100,7 +101,9 @@ def path_provider() -> PathProvider: @devices.fixture @cache def config_client() -> ConfigClient: - config_server_url = getenv("CONFIG_SERVER_URL", DEFAULT_CONFIG_SERVER_ENDPOINT) + config_server_url = getenv( + CONFIG_SERVER_URL_ENV_VAR, DEFAULT_CONFIG_SERVER_ENDPOINT + ) client = ConfigClient(config_server_url) set_config_client(client) return client diff --git a/src/dodal/common/beamlines/beamline_parameters.py b/src/dodal/common/beamlines/beamline_parameters.py index f0645a85eb4..7f5ebd0f1a9 100644 --- a/src/dodal/common/beamlines/beamline_parameters.py +++ b/src/dodal/common/beamlines/beamline_parameters.py @@ -8,6 +8,9 @@ } +CONFIG_SERVER_URL_ENV_VAR = "CONFIG_SERVER_URL" + + def get_beamline_parameters(beamline: str) -> dict[str, Any]: """Loads the beamline parameters for a specified beamline from the config server. diff --git a/src/dodal/devices/eiger.py b/src/dodal/devices/eiger.py index 1662cc86a15..9a4b2899a8b 100644 --- a/src/dodal/devices/eiger.py +++ b/src/dodal/devices/eiger.py @@ -5,7 +5,7 @@ from ophyd import Component, Device, EpicsSignalRO, Signal from ophyd.areadetector.cam import EigerDetectorCam from ophyd.signal import AttributeSignal -from ophyd.status import AndStatus, Status, StatusBase, SubscriptionStatus +from ophyd.status import Status, StatusBase, SubscriptionStatus from dodal.devices.detector import DetectorParams, TriggerMode from dodal.devices.eiger_odin import EigerOdin @@ -24,6 +24,7 @@ class EigerTimeouts: all_frames_timeout: int = 120 arming_timeout: int = 60 odin_stop_timeout: int = 30 + odin_meta_active_timeout: int = 60 class InternalEigerTriggerMode(Enum): @@ -225,46 +226,54 @@ def change_roi_mode(self, enable: bool) -> StatusBase: if enable else self.detector_params.detector_size_constants.det_size_pixels ) - status = self.cam.roi_mode.set( + LOGGER.info(f"Setting cam.roi_mode to {1 if enable else 0}") + self.cam.roi_mode.set( 1 if enable else 0, timeout=self.timeouts.general_status_timeout - ) - status &= self.odin.file_writer.image_height.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug(f"Setting image_height to {detector_dimensions.height}") + self.odin.file_writer.image_height.set( detector_dimensions.height, timeout=self.timeouts.general_status_timeout - ) - status &= self.odin.file_writer.image_width.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug(f"Setting image_width to {detector_dimensions.width}") + self.odin.file_writer.image_width.set( detector_dimensions.width, timeout=self.timeouts.general_status_timeout - ) - status &= self.odin.file_writer.num_row_chunks.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug(f"Setting num_row_chunks to {detector_dimensions.height}") + self.odin.file_writer.num_row_chunks.set( detector_dimensions.height, timeout=self.timeouts.general_status_timeout - ) - status &= self.odin.file_writer.num_col_chunks.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug(f"Setting num_col_chunks to {detector_dimensions.width}") + status = self.odin.file_writer.num_col_chunks.set( detector_dimensions.width, timeout=self.timeouts.general_status_timeout ) return status - def set_cam_pvs(self) -> AndStatus: + def set_cam_pvs(self) -> StatusBase: LOGGER.info("Eiger arming: Setting eiger camera PVs...") assert self.detector_params is not None - status = self.cam.acquire_time.set( + self.cam.acquire_time.set( self.detector_params.exposure_time_s, timeout=self.timeouts.general_status_timeout, - ) - status &= self.cam.acquire_period.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug("Setting cam acquire_period...") + self.cam.acquire_period.set( self.detector_params.exposure_time_s, timeout=self.timeouts.general_status_timeout, - ) - status &= self.cam.num_exposures.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug("Setting cam num_exposures...") + self.cam.num_exposures.set( 1, timeout=self.timeouts.general_status_timeout - ) - status &= self.cam.image_mode.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug("Setting cam image_mode...") + self.cam.image_mode.set( self.cam.ImageMode.MULTIPLE, # pyright: ignore[reportAttributeAccessIssue] timeout=self.timeouts.general_status_timeout, - ) - status &= self.cam.trigger_mode.set( + ).wait(timeout=self.timeouts.general_status_timeout) + LOGGER.debug("Setting cam trigger_mode...") + return self.cam.trigger_mode.set( InternalEigerTriggerMode.EXTERNAL_SERIES.value, timeout=self.timeouts.general_status_timeout, ) - return status def set_odin_number_of_frame_chunks(self) -> Status: LOGGER.info("Eiger arming: Setting odin number of frames chunks...") @@ -379,7 +388,9 @@ def set_num_triggers_and_captures(self) -> StatusBase: def _wait_for_odin_status(self) -> StatusBase: self.forward_bit_depth_to_filewriter() - await_value(self.odin.meta.active, 1).wait(self.timeouts.general_status_timeout) + await_value(self.odin.meta.active, 1).wait( + self.timeouts.odin_meta_active_timeout + ) status = self.odin.file_writer.capture.set( 1, timeout=self.timeouts.general_status_timeout diff --git a/tests/devices/test_eiger.py b/tests/devices/test_eiger.py index e42dd11f9d3..aa644c7988e 100644 --- a/tests/devices/test_eiger.py +++ b/tests/devices/test_eiger.py @@ -275,44 +275,26 @@ def test_change_roi_mode_sets_cam_roi_mode_correctly( ) -# Also tests transition from change ROI to set_detector_threshold -@patch("ophyd.status.Status.__and__") -def test_unsuccessful_true_roi_mode_change_results_in_callback_error( - mock_and, fake_eiger: EigerDetector -): - bad_status = Status() - bad_status.set_exception(StatusError("Failed setting ROI mode True")) - mock_and.return_value = bad_status - LOGGER.error = MagicMock() - - unwrapped_funcs = [ - lambda: fake_eiger.change_roi_mode(enable=True), - lambda: fake_eiger.set_detector_threshold( - energy=fake_eiger.detector_params.expected_energy_ev - ), - ] - with pytest.raises(StatusError): - run_functions_without_blocking(unwrapped_funcs).wait() - LOGGER.error.assert_called() - - -@patch("ophyd.status.Status.__and__") -def test_unsuccessful_false_roi_mode_change_results_in_callback_error( - mock_and, fake_eiger: EigerDetector +def test_unsuccessful_roi_mode_change_results_in_callback_error( + fake_eiger: EigerDetector, ): - bad_status = Status() - bad_status.set_exception(StatusError("Failed setting ROI mode False")) - mock_and.return_value = bad_status - LOGGER.error = MagicMock() - - unwrapped_funcs = [ - lambda: fake_eiger.change_roi_mode(enable=False), - lambda: fake_eiger.set_detector_threshold( - energy=fake_eiger.detector_params.expected_energy_ev - ), - ] - with pytest.raises(StatusError): - run_functions_without_blocking(unwrapped_funcs).wait() + for signal in [ + fake_eiger.odin.file_writer.num_col_chunks, + fake_eiger.odin.file_writer.num_row_chunks, + fake_eiger.odin.file_writer.image_width, + fake_eiger.odin.file_writer.image_height, + fake_eiger.cam.roi_mode, + ]: + bad_status = Status() + bad_status.set_exception(StatusError(f"Failed setting {signal.name}")) + signal.set = MagicMock(return_value=bad_status) + LOGGER.error = MagicMock() + + unwrapped_funcs = [ + lambda: fake_eiger.change_roi_mode(enable=True), + ] + with pytest.raises(StatusError, match=f"Failed setting {signal.name}"): + run_functions_without_blocking(unwrapped_funcs).wait() @patch("dodal.devices.eiger.EigerOdin.check_and_wait_for_odin_state") @@ -354,8 +336,8 @@ def test_stage_runs_successfully( set_up_eiger_to_stage_happily(fake_eiger) fake_eiger.stage() fake_eiger.arming_status.wait(1) # This should complete long before 1s - # One log message kicking off arming, then one for each of the 13 arming stages - assert len(caplog.messages) == 14 + # One log message kicking off arming, then one for each of the 17 arming stages + assert len(caplog.messages) == 18 def test_given_stale_parameters_goes_high_before_callbacks_then_stale_parameters_waited_on(