Fix leftover tasks when aborting an exposure (#51)

* Fix leftover tasks when aborting an exposure

* Merge branch 'main' into albireox-fix-exposure-abort

* Install cfitsio in tests

* libcfitsio -> cfitsio

* Only libcfitsio-dev

* Install libbz2

* libbz2 -> bzip2

* Add libbz2-dev

* Update changelog

.github/workflows/test.yml

@@ -25,6 +25,11 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - name: Install build dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y bzip2 libbz2-dev
+
       - name: Set up Python
         uses: actions/setup-python@v5
         with:
@@ -43,8 +48,6 @@ jobs:
         run: |
           uv pip install pytest pytest-mock pytest-asyncio pytest-cov
           uv run pytest
-        env:
-          OBSERVATORY: LCO
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v5

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## Next version
+
+### 🔧 Fixed
+
+* [#51](https://github.com/sdss/archon/pull/51) Fix an issue in which when an exposure was cancelled during integration, the task that updates the completion of the exposure and sets the `IDLE` state could be left running and affect a future exposure.
+
+
 ## 0.15.0 - November 7, 2024
 
 ### 🔥 Breaking changes

archon/actor/commands/expose.py

@@ -323,7 +323,7 @@ async def abort(
         return command.fail(error=f"Failed aborting exposures: {err}")
     finally:
         # This will also cancel any ongoing exposure or readout.
-        delegate.fail("Exposure was aborted")
+        await delegate.fail("Exposure was aborted")
 
     if reset:
         command.debug(text="Resetting controllers")

archon/actor/delegate.py

@@ -38,6 +38,7 @@
 
 from sdsstools.configuration import Configuration
 from sdsstools.time import get_sjd
+from sdsstools.utils import cancel_task
 
 from archon import __version__
 from archon.controller.controller import ArchonController
@@ -126,34 +127,29 @@ def command(self, value: Command[Actor_co] | None):
 
         self._command = value
 
-    def reset(self):
+    async def reset(self):
         """Resets the exposure delegate."""
 
         self.expose_data = None
         self.command = None
         self.is_writing = False
 
-        if self._expose_cotasks is not None and not self._expose_cotasks.done():
-            self._expose_cotasks.cancel()
-
-        self._expose_cotasks = None
+        self._expose_cotasks = await cancel_task(self._expose_cotasks)
 
         if self.lock.locked():
             self.lock.release()
 
-    def fail(self, message: str | None = None):
+    async def fail(self, message: str | None = None):
         """Fails a command."""
 
-        if self._current_task:
-            self._current_task.cancel()
-            self._current_task = None
+        self._current_task = await cancel_task(self._current_task)
 
         if message:
             self.command.fail(error=message)
         else:
             self.command.fail()
 
-        self.reset()
+        await self.reset()
 
         return False
 
@@ -184,13 +180,15 @@ async def expose(
         self.command = command
 
         if self.lock.locked():
-            return self.fail("The expose delegate is locked.")
+            await self.fail("The expose delegate is locked.")
+            return False
 
         if flavour == "bias":
             exposure_time = 0.0
         else:
             if exposure_time is None:
-                return self.fail(f"Exposure time required for flavour {flavour!r}.")
+                await self.fail(f"Exposure time required for flavour {flavour!r}.")
+                return False
 
         if window_mode:
             if window_mode == "default":
@@ -200,7 +198,8 @@ async def expose(
                 window_params = self.config["window_modes"][window_mode]
                 window_params.update(extra_window_params)
             else:
-                return self.fail(f"Invalid window mode {window_mode!r}.")
+                await self.fail(f"Invalid window mode {window_mode!r}.")
+                return False
 
         self.expose_data = ExposeData(
             exposure_time=exposure_time,
@@ -217,7 +216,11 @@ async def expose(
         await self.lock.acquire()
 
         will_write = readout_params.get("write", True)
-        if not self._set_exposure_no(controllers, increase=will_write, seqno=seqno):
+        if not await self._set_exposure_no(
+            controllers,
+            increase=will_write,
+            seqno=seqno,
+        ):
             return False
 
         self.command.debug(next_exposure_no=self.expose_data.exposure_no)
@@ -236,7 +239,7 @@ async def expose(
         except BaseException as err:
             self.command.error("One controller failed setting the exposure window.")
             self.command.error(error=str(err))
-            return self.fail()
+            return await self.fail()
 
         self._expose_cotasks = asyncio.create_task(self.expose_cotasks())
 
@@ -257,19 +260,19 @@ async def expose(
                     with suppress(asyncio.CancelledError):
                         job.cancel()
                         await job
-            return self.fail()
+            return await self.fail()
 
         # Operate the shutter
         if self.use_shutter:
             if not (await self.shutter(True)):
-                return self.fail("Shutter failed to open.")
+                return await self.fail("Shutter failed to open.")
 
         await asyncio.sleep(exposure_time)
 
         # Close shutter.
         if self.use_shutter:
             if not (await self.shutter(False)):
-                return self.fail("Shutter failed to close.")
+                return await self.fail("Shutter failed to close.")
 
         if readout:
             return await self.readout(self.command, **readout_params)
@@ -284,11 +287,11 @@ async def check_expose(self) -> bool:
         for controller in self.expose_data.controllers:
             cname = controller.name
             if controller.status & ControllerStatus.EXPOSING:
-                return self.fail(f"Controller {cname} is exposing.")
+                return await self.fail(f"Controller {cname} is exposing.")
             elif controller.status & ControllerStatus.READOUT_PENDING:
-                return self.fail(f"Controller {cname} has a read out pending.")
+                return await self.fail(f"Controller {cname} has a read out pending.")
             elif controller.status & ControllerStatus.ERROR:
-                return self.fail(f"Controller {cname} has status ERROR.")
+                return await self.fail(f"Controller {cname} has status ERROR.")
 
         return True
 
@@ -303,7 +306,7 @@ async def readout(
         extra_header={},
         delay_readout: int = 0,
         write: bool = True,
-    ):
+    ) -> bool:
         """Reads the exposure, fetches the buffer, and writes to disk."""
 
         self.command = command
@@ -315,10 +318,10 @@ async def readout(
             self.command.set_status = MagicMock()
 
         if not self.lock.locked():
-            return self.fail("Expose delegator is not locked.")
+            return await self.fail("Expose delegator is not locked.")
 
         if self.expose_data is None:
-            return self.fail("No exposure data found.")
+            return await self.fail("No exposure data found.")
 
         controllers = self.expose_data.controllers
 
@@ -329,7 +332,7 @@ async def readout(
         t0 = time()
 
         if any([c.status & ControllerStatus.EXPOSING for c in controllers]):
-            return self.fail(
+            return await self.fail(
                 "Found controllers exposing. Wait before reading or "
                 "manually abort them."
             )
@@ -350,17 +353,17 @@ async def readout(
             c_fdata = await asyncio.gather(*[self.fetch_data(c) for c in controllers])
 
         except Exception as err:
-            return self.fail(f"Failed reading out: {err}")
+            return await self.fail(f"Failed reading out: {err}")
 
         if len(c_fdata) == 0:
             self.command.error("No data was fetched.")
             return False
 
-        self.command.debug(f"Readout completed in {time()-t0:.1f} seconds.")
+        self.command.debug(f"Readout completed in {time() - t0:.1f} seconds.")
 
         if write is False:
             self.command.warning("Not saving images to disk.")
-            self.reset()
+            await self.reset()
             return True
 
         # c_fdata is a list of lists. The top level list is one per controller,
@@ -452,7 +455,7 @@ async def readout(
 
         self.command.info(filenames=filenames)
 
-        self.reset()
+        await self.reset()
 
         return not failed_to_write
 
@@ -712,18 +715,18 @@ async def build_base_header(
 
         if isinstance(gain, (list, tuple)):
             for channel_idx in range(len(gain)):
-                header[f"GAIN{channel_idx+1}"] = [
+                header[f"GAIN{channel_idx + 1}"] = [
                     gain[channel_idx],
-                    f"CCD gain AD{channel_idx+1} [e-/ADU]",
+                    f"CCD gain AD{channel_idx + 1} [e-/ADU]",
                 ]
         else:
             header["GAIN"] = [gain, "CCD gain [e-/ADU]"]
 
         if isinstance(readnoise, (list, tuple)):
             for channel_idx in range(len(readnoise)):
-                header[f"RDNOISE{channel_idx+1}"] = [
+                header[f"RDNOISE{channel_idx + 1}"] = [
                     readnoise[channel_idx],
-                    f"CCD read noise AD{channel_idx+1} [e-]",
+                    f"CCD read noise AD{channel_idx + 1} [e-]",
                 ]
         else:
             header["RDNOISE"] = [readnoise, "CCD read noise [e-]"]
@@ -839,7 +842,7 @@ async def build_base_header(
 
         return header
 
-    def _set_exposure_no(
+    async def _set_exposure_no(
         self,
         controllers: list[ArchonController],
         increase: bool = True,
@@ -878,7 +881,7 @@ def _set_exposure_no(
                 try:
                     self._get_ccd_filepath(controller, ccd)
                 except FileExistsError as err:
-                    self.fail(f"{err} Check the nextExposureNumber file.")
+                    await self.fail(f"{err} Check the nextExposureNumber file.")
                     return False
 
         if increase:

archon/controller/command.py

@@ -210,7 +210,7 @@ def __init__(self, raw_reply: bytes, command: ArchonCommand):
         parsed = REPLY_RE.match(raw_reply)
         if not parsed:
             raise ArchonError(
-                f"Received unparseable reply to command " f"{command.raw}: {raw_reply}"
+                f"Received unparseable reply to command {command.raw}: {raw_reply}"
             )
 
         self.command = command

archon/controller/controller.py

@@ -21,6 +21,7 @@
 import numpy
 
 from clu.device import Device
+from sdsstools.utils import cancel_task
 
 from archon import config as lib_config
 from archon import log
@@ -88,6 +89,8 @@ def __init__(
         # call get_running_loop() which fails in iPython.
         self._job = asyncio.get_event_loop().create_task(self.__track_commands())
 
+        self._update_state_task: asyncio.Task | None = None
+
     async def start(self, reset: bool = True, read_acf: bool = True):
         """Starts the controller connection. If ``reset=True``, resets the status."""
 
@@ -319,7 +322,7 @@ async def send_and_wait(
             else:
                 warnings.warn(f"Failed running {command_string}.", ArchonUserWarning)
 
-    async def process_message(self, line: bytes) -> None:
+    async def process_message(self, line: bytes) -> None:  # type: ignore
         """Processes a message from the Archon and associates it with its command."""
 
         match = re.match(b"^[<|?]([0-9A-F]{2})", line)
@@ -796,6 +799,10 @@ async def reset(
                         f"Failed sending {cmd_str} ({cmd.status.name})"
                     )
 
+        # If we are exposing, we have reset everything so we cancel the
+        # task that waits until the exposure is done.
+        self._update_state_task = await cancel_task(self._update_state_task)
+
         if update_status:
             self._status = ControllerStatus.IDLE
             await self.power()  # Sets power bit.
@@ -1043,7 +1050,8 @@ async def update_state():
             else:
                 raise ArchonControllerError("Controller is not reading.")
 
-        return asyncio.create_task(update_state())
+        self._update_state_task = asyncio.create_task(update_state())
+        return self._update_state_task
 
     async def abort(self, readout: bool = False):
         """Aborts the current exposure.
@@ -1054,6 +1062,9 @@ async def abort(self, readout: bool = False):
 
         log.info(f"{self.name}: aborting controller.")
 
+        # First cancel the exposing task, if it exists.
+        self._update_state_task = await cancel_task(self._update_state_task)
+
         CS = ControllerStatus
 
         if not self.status & ControllerStatus.EXPOSING:

pyproject.toml

@@ -10,9 +10,9 @@ readme = "README.md"
 requires-python = ">=3.10,<4"
 
 dependencies = [
-    "sdsstools>=1.8.1",
+    "sdsstools>=1.9.1",
     "numpy>=2.0.0",
-    "sdss-clu>=2.2.7",
+    "sdss-clu>=2.4.3",
     "click-default-group>=1.2.2",
     "astropy>=6.0"
 ]
@@ -83,7 +83,7 @@ markers = [
     "commands: commands and replies for the test Archon"
 ]
 asyncio_mode = "auto"
-syncio_default_fixture_loop_scope = "function"
+asyncio_default_fixture_loop_scope = "function"
 
 [tool.coverage.run]
 branch = true

tests/controller/test_expose.py

@@ -55,19 +55,15 @@ async def test_expose_no_readout(controller: ArchonController, mocker):
 
 
 async def test_abort(controller: ArchonController, mocker):
-    task = await controller.expose(0.01)
-
+    await controller.expose(0.01)
     await controller.abort(readout=True)
-    await task
 
     assert controller.status & ControllerStatus.READING
 
 
 async def test_abort_no_readout(controller: ArchonController, mocker):
-    task = await controller.expose(0.01)
-
+    await controller.expose(0.01)
     await controller.abort(readout=False)
-    await task
 
     assert controller.status & ControllerStatus.IDLE
     assert controller.status & ControllerStatus.READOUT_PENDING