Improve shutter retry on error (#21)

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## Next version
+
+### ✨ Improved
+
+* [#21](https://github.com/sdss/lvmscp/issues/21) Improve how the delegate retries opening/closing the shutter when the first attempt fails. If the shutter fails twice closing, the exposure is read anyway.
+
+
 ## 0.6.1 - July 20, 2023
 
 ### ✨ Improved

python/lvmscp/delegate.py

@@ -21,7 +21,9 @@
 
 
 if TYPE_CHECKING:
-    from .actor import SCPActor  # noqa
+    from clu import Command
+
+    from .actor import SCPActor
 
 
 class LVMExposeDelegate(ExposureDelegate["SCPActor"]):
@@ -30,10 +32,11 @@ class LVMExposeDelegate(ExposureDelegate["SCPActor"]):
     def __init__(self, actor: SCPActor):
         super().__init__(actor)
 
-        self.use_shutter = True
+        self.use_shutter: bool = True
+        self.shutter_failed: bool = False
 
         # Additional data from the IEB and environmental sensors.
-        self.extra_data = {}
+        self.extra_data: dict[str, Any] = {}
 
     def reset(self):
         self.extra_data = {}
@@ -63,7 +66,7 @@ async def check_expose(self) -> bool:
 
         return True
 
-    async def shutter(self, open, retry=False):
+    async def shutter(self, open, is_retry=False):
         """Operate the shutter."""
 
         if not self.use_shutter:
@@ -85,17 +88,45 @@ async def shutter(self, open, retry=False):
         results = await asyncio.gather(*jobs, return_exceptions=True)
 
         if not all(results):
-            if action == "close" and retry is False:
+            self.shutter_failed = True
+            if is_retry is False:
                 self.command.warning(text="Some shutters failed to close. Retrying.")
-                return await self.shutter(False, retry=True)
+                await asyncio.sleep(3)
+                return await self.shutter(open, is_retry=True)
             else:
-                return self.fail("Some shutters failed to move.")
+                self.command.error("Some shutters failed to move.")
+        else:
+            self.shutter_failed = False
 
-        if retry is True:
-            return self.fail("Closed all shutters but failing now.")
+        if self.shutter_failed:
+            if open is True:
+                return False
+            else:
+                self.command.warning(
+                    "Shutter failed to close. Reading out exposure and failing."
+                )
 
         return True
 
+    async def readout(
+        self,
+        command: Command[SCPActor],
+        extra_header: dict[str, Any] = {},
+        delay_readout: int = 0,
+        write: bool = True,
+    ):
+        """Reads detectors."""
+
+        if self.shutter_failed:
+            self.command.warning(
+                "Frame was read out but shutter failed to close. "
+                "There may be contamination in the image."
+            )
+
+        read_result = await super().readout(command, extra_header, delay_readout, write)
+
+        return False if (self.shutter_failed or not read_result) else True
+
     async def expose_cotasks(self):
         """Grab sensor data when the exposure begins to save time.
 

tests/test_delegate.py

@@ -36,6 +36,8 @@ def delegate(actor: SCPActor, monkeypatch, tmp_path: pathlib.Path, mocker):
         return_value=(numpy.ones((2048, 6144)), 1),
     )
 
+    mocker.patch.object(actor.expose_delegate, "get_telescope_info", return_value={})
+
     mocker.patch.object(
         actor.expose_delegate,
         "_get_ccd_data",
@@ -150,3 +152,50 @@ async def test_expose(delegate, command, actor: SCPActor, mocker):
     await command
 
     assert command.status.did_succeed
+
+
+async def test_shutter_fails_to_open(delegate, command, mocker):
+    move_shutter = mocker.patch.object(delegate, "move_shutter", return_value=False)
+
+    result = await delegate.expose(
+        command,
+        [delegate.actor.controllers["sp1"]],
+        flavour="object",
+        exposure_time=0.1,
+        readout=True,
+    )
+
+    assert not result
+    assert move_shutter.call_count == 2
+    assert delegate.shutter_failed
+
+
+async def test_shutter_fails_to_close(delegate, command, mocker):
+    async def _move_shutter(_, action):
+        if action == "close":
+            return False
+        return True
+
+    move_shutter = mocker.patch.object(
+        delegate,
+        "move_shutter",
+        side_effect=_move_shutter,
+    )
+
+    result = await delegate.expose(
+        command,
+        [delegate.actor.controllers["sp1"]],
+        flavour="object",
+        exposure_time=0.1,
+        readout=True,
+    )
+
+    assert not result
+    assert move_shutter.call_count == 3
+    assert delegate.shutter_failed
+
+    replies = [reply.body["text"] for reply in command.replies if "text" in reply.body]
+    assert (
+        "Frame was read out but shutter failed to close. "
+        "There may be contamination in the image." in replies
+    )