Merge pull request #92 from DiamondLightSource/76_make_stage_synchron…

…ous_if_arming_not_started_rebased

Make stage synchronous if arming not started

src/dodal/devices/eiger.py

@@ -41,6 +41,9 @@ def set(self, value, *, timeout=None, settle_time=None, **kwargs):
 
     detector_params: Optional[DetectorParams] = None
 
+    arming_status = Status()
+    arming_status.set_finished()
+
     @classmethod
     def with_params(
         cls,
@@ -79,7 +82,20 @@ def async_stage(self):
         status_ok, error_message = self.odin.check_odin_initialised()
         if not status_ok:
             raise Exception(f"Odin not initialised: {error_message}")
-        return self.do_arming_chain()
+
+        self.arming_status = self.do_arming_chain()
+        return self.arming_status
+
+    def is_armed(self):
+        return self.odin.fan.ready.get() == 1 and self.cam.acquire.get() == 1
+
+    def stage(self):
+        if not self.arming_status.done:
+            # Arming has started so wait for it to finish
+            self.arming_status.wait(60)
+        elif not self.is_armed():
+            # Arming hasn't started, do it asynchronously
+            self.async_stage().wait(timeout=self.GENERAL_STATUS_TIMEOUT)
 
     def unstage(self) -> bool:
         assert self.detector_params is not None
@@ -98,7 +114,6 @@ def unstage(self) -> bool:
 
         LOGGER.info("Disarming detector")
         self.disarm_detector()
-
         status_ok = self.odin.check_odin_state()
         self.disable_roi_mode()
         return status_ok
@@ -289,7 +304,7 @@ def do_arming_chain(self) -> Status:
                 self.set_odin_pvs,
                 self.set_mx_settings_pvs,
                 self.set_num_triggers_and_captures,
-                lambda: await_value(self.STALE_PARAMS_TIMEOUT, 0, 60),
+                lambda: await_value(self.stale_params, 0, 60),
                 self._wait_for_odin_status,
                 lambda: self.cam.acquire.set(1, timeout=self.GENERAL_STATUS_TIMEOUT),
                 self._wait_fan_ready,

src/dodal/devices/utils.py

@@ -41,7 +41,8 @@ def run_functions_without_blocking(
     # intermediate statuses have an exception, the full_status will timeout.
     full_status = Status(timeout=timeout)
 
-    def closing_func():
+    def closing_func(old_status):
+        check_callback_error(old_status)
         full_status.set_finished()
 
     # Wrap each function by first checking the previous status and attaching a callback to the next

tests/devices/unit_tests/test_eiger.py

@@ -29,6 +29,10 @@
 TEST_DET_DIST_TO_BEAM_CONVERTER_PATH = "tests/devices/unit_tests/test_lookup_table.txt"
 
 
+class StatusException(Exception):
+    pass
+
+
 def create_new_params() -> DetectorParams:
     return DetectorParams(
         current_energy_ev=TEST_CURRENT_ENERGY,
@@ -62,6 +66,12 @@ def finished_status():
     return status
 
 
+def get_bad_status(exception=Exception):
+    status = Status()
+    status.set_exception(Exception)
+    return status
+
+
 @pytest.fixture
 def mock_set_odin_filewriter(fake_eiger: EigerDetector):
     fake_eiger.odin.nodes.clear_odin_errors = MagicMock()
@@ -443,11 +453,24 @@ def test_when_stage_called_then_finish_arm_on_fan_ready(
 def test_check_callback_error(fake_eiger: EigerDetector, iteration):
     def get_bad_status():
         status = Status()
-        status.set_exception(Exception)
+        status.set_exception(StatusException)
+        return status
+
+    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()
+
     unwrapped_funcs = [
         (
             lambda: fake_eiger.set_detector_threshold(
@@ -467,8 +490,8 @@ def get_bad_status():
 
     unwrapped_funcs[iteration] = get_bad_status
 
-    with pytest.raises(Exception):
-        run_functions_without_blocking(unwrapped_funcs).wait()
+    with pytest.raises(StatusException):
+        run_functions_without_blocking(unwrapped_funcs).wait(timeout=10)
         LOGGER.error.assert_called_once()
 
 
@@ -499,3 +522,31 @@ def test_given_in_free_run_mode_when_unstaged_then_waiting_on_file_writer_to_fin
 
     assert fake_eiger.odin.meta.stop_writing.get() == 1
     assert fake_eiger.odin.file_writer.capture.get() == 0
+
+
+def test_if_arming_in_progress_then_stage_waits_for_completion(
+    fake_eiger: EigerDetector, mock_set_odin_filewriter
+):
+    def set_bad_arming_status():
+        fake_eiger.arming_status = get_bad_status()
+
+    assert fake_eiger.arming_status.done is True
+    fake_eiger.arming_status = get_bad_status()
+    # Should do .wait and error
+    with pytest.raises(Exception):
+        fake_eiger.stage()
+
+
+def test_if_eiger_isnt_armed_then_stage_calls_async_stage(fake_eiger: EigerDetector):
+    fake_eiger.async_stage = MagicMock()
+    fake_eiger.odin.fan.ready.sim_put(0)
+    fake_eiger.stage()
+    fake_eiger.async_stage.assert_called_once()
+
+
+def test_if_eiger_is_armed_then_stage_does_nothing(fake_eiger: EigerDetector):
+    fake_eiger.odin.fan.ready.sim_put(1)
+    fake_eiger.cam.acquire.sim_put(1)
+    fake_eiger.async_stage = MagicMock()
+    fake_eiger.stage()
+    fake_eiger.async_stage.assert_not_called()