Commit 28c9a278 authored by vertighel's avatar vertighel
Browse files

Fase 1 devices: chiusura checklist, lock su atik.py, fix meteo.py, download() non piu' bloccante



- meteo.py: summary() leggeva chiavi (TempIn/HumIn/WindSpeed/...) che
  data() aveva gia' rinominato -- KeyError garantito se mai chiamato
  (bug da audit Fase 0, dispositivo non ancora istanziato). Corretto
  per usare le chiavi rinominate.
- atik.py: aggiunto il lock (l'RLock esistente, gia' usato per
  serializzare get()/put(), e' reentrant) attorno a
  _subframe/_last_exptime/_last_imagetype/_last_datetime, scritti dal
  loop thread e letti dai request thread senza sincronizzazione --
  stesso pattern gia' applicato a stx.py e mako.py/stl.py.
- atik.py: chiuso il bug hardware "download() si blocca per sempre se
  il cooling viene toggled a meta'" -- il poll di ArtemisImageReady()
  ora ha un deadline (durata ultima esposizione + 10s di margine),
  oltre il quale ritorna None con self.error impostato invece di
  girare all'infinito. Stesso pattern di stop_looping(). Il bug
  CCD-TEMP resta aperto, serve hardware fisico.
- devices/old/* (alpaca.py, astelco.py, stx.py): erano file locali
  gitignored/non tracciati, nessun riferimento nel codice, cancellati
  dal filesystem (non da git).
- PLAN.md: checklist Fase 1 aggiornata a riflettere sia il lavoro gia'
  fatto nei due commit precedenti (67170649, d47c4c6d) sia quanto sopra
  -- Fase 1 (Devices) ora completamente chiusa.

Co-Authored-By: default avatarClaude Sonnet 5 <noreply@anthropic.com>
parent d47c4c6d
Loading
Loading
Loading
Loading
+44 −24
Original line number Diff line number Diff line
@@ -102,7 +102,8 @@ Commits so far: `e662296` (indentation crash fix, `RLock` serializing all
Artemis SDK calls, uint16 FITS in `stl.py`), `9768f56` (check
`ArtemisImageFailed()` + `ArtemisTemperatureSensorInfo` return code),
`c4e04ca`/"atik dev" (settle `time.sleep(0.2)` before reading CCD-TEMP on
binned frames). None of the below are closed.
binned frames). The `download()` hang below was closed on this branch
during the devices phase (no hardware needed); CCD-TEMP is still open.

- **CCD-TEMP still corrupts, now correlated with cooling being on**: on
  `fork`, with cooling active, `CCD-TEMP` intermittently reads a large
@@ -117,16 +118,14 @@ binned frames). None of the below are closed.
  still unknown; needs investigation with the physical camera, not
  further blind guesses from the host side. Do not attempt another fix
  without hardware in hand.
- **`download()` hangs forever if cooling is toggled mid-download**: the
  `while True: ... ArtemisImageReady(h) ...` poll loop in `download()`
  (and the equivalent wait in `_run_loop`) has no timeout — if toggling
  `cooler` while an exposure/readout is in flight leaves
  `ArtemisImageReady()` never returning true, the loop spins
  indefinitely with no way out (no abort, no error surfaced). Pre-dates
  the recent fixes (the original code had the same unbounded loop) but
  was only now triggered/observed. Needs a bounded timeout with an
  explicit error path, matching `stop_looping()`'s bounded-wait pattern
  elsewhere in the same file.
- ~~**`download()` hangs forever if cooling is toggled mid-download**~~
  FIXED (devices phase): `download()`'s `ArtemisImageReady()` poll now
  has a deadline (`last_exptime + 10s`, matching `stop_looping()`'s
  bounded-wait pattern) and returns `None` with `self.error` set instead
  of spinning forever. The equivalent wait inside `_run_loop()` was
  already bounded via the `self._looping` flag (exits within one 0.3s
  tick of `stop_looping()`/`looping = False`), so it didn't need the
  same fix — left as is.

## Phase checklists

@@ -144,23 +143,35 @@ binned frames). None of the below are closed.
      get/put. `SystemExit`-on-unhandled-error question decided and
      implemented (`check._RECOVERABLE`, only `noctua.app:run()` opts
      in) — see python.md.
- [ ] `netio.py`/`siemens.py`/`domotics.py` don't inherit
- [x] `netio.py`/`siemens.py`/`domotics.py` don't inherit
      `BaseDevice` unlike every other device — decide if that's
      intentional or should be fixed.
- [ ] `astelco.py` exposes both `status` (str) and `state` (int) for
      intentional or should be fixed. Fixed: all three now inherit via
      `super().__init__(url)` (commit `6717064`).
- [x] `astelco.py` exposes both `status` (str) and `state` (int) for
      the telescope, plus `mercury.py` reuses `status` for a third,
      unrelated meaning (bool health check) — reconcile naming.
- [ ] `stx.py`/`atik.py` have the same background-loop-thread +
      unrelated meaning (bool health check) — reconcile naming. Fixed:
      `astelco.py`'s `state` is now a read-only bool (error-present);
      `mercury.py`'s `status` renamed to `connection` (commit `6717064`).
- [x] `stx.py`/`atik.py` have the same background-loop-thread +
      request-thread shape as `mako.py`/`stl.py` but no lock guarding
      shared mutable state (`atik.py`'s `_subframe`/`_last_exptime`/
      etc, written by the loop thread, read by request threads) — add
      a lock matching the `mako`/`stl` pattern.
- [ ] Fix the "real bugs found" items above that live in this layer.
- [ ] Translate remaining Italian comments/docstrings to English as
      each file is touched (only one found: `atik.py:110`).
- [ ] `devices/old/*` (three files) — confirmed unused anywhere,
      a lock matching the `mako`/`stl` pattern. `stx.py`'s
      `_last_command_time` done in commit `6717064`; `atik.py`'s
      `_subframe`/`_last_exptime`/`_last_imagetype`/`_last_datetime`
      now guarded too, reusing the existing SDK-serializing `RLock`
      (reentrant, so no second lock object needed).
- [x] Fix the "real bugs found" items above that live in this layer:
      `domotics.py`'s dead `return` and `ipcam.py`'s `__init__`/
      `save_image` fixed in commit `6717064`; `meteo.py`'s `summary()`
      reading pre-rename keys from `data()` fixed this session.
- [x] Translate remaining Italian comments/docstrings to English as
      each file is touched (only one found: `atik.py:110`, done in
      commit `6717064`).
- [x] `devices/old/*` (three files) — confirmed unused anywhere,
      delete once the corresponding new-style file is confirmed to
      fully replace it (already the case today).
      fully replace it (already the case today). Deleted this session
      (were gitignored/untracked, not under git, so plain `rm`).

### 2. Templates

@@ -300,5 +311,14 @@ stations/panels.
## Status

Phase 0 done: branch created, 5-layer audit complete, conventions
written to `dev/conventions/*.md`, this checklist written. **Nothing
in phases 1-6 started.** Nothing committed yet on this branch.
written to `dev/conventions/*.md`, this checklist written.

Phase 1 (Devices) done: all checklist items closed (commits `6717064`,
`d47c4c6`, plus uncommitted work — meteo.py `summary()` bug, atik.py
shared-state locking, atik.py `download()` hang, `devices/old/*`
deletion). One hardware bug remains open and tracked separately
(CCD-TEMP corruption, needs the physical camera, see the "Bugs found
during hardware testing on main" section above) — not a phase-1
checklist item, doesn't block moving on.

**Phase 2 (Templates) not started.**
+52 −20
Original line number Diff line number Diff line
@@ -79,7 +79,11 @@ class Camera(BaseDevice):
        # below: the SDK isn't thread-safe for a single handle, and this
        # camera reads most properties live from it (no cached last-frame
        # like mako.py/stl.py), so the background loop thread and request
        # threads can otherwise call into it concurrently.
        # threads can otherwise call into it concurrently. Reentrant so it
        # also guards _subframe/_last_exptime/_last_imagetype/
        # _last_datetime below, set by the loop thread in _start() and
        # read by request threads in download()/xystart/xyend/etc,
        # matching the mako.py/stl.py last-frame locking pattern.
        self._lock = threading.RLock()

        try:
@@ -234,6 +238,7 @@ class Camera(BaseDevice):
            log.error(f"Cannot start exposure, camera is not idle. State: {self.state}")
            self.error.append("Camera not idle")
            return
        with self._lock:
            self._last_exptime = duration
            self._last_imagetype = imagetype
            self._last_datetime = datetime
@@ -284,8 +289,23 @@ class Camera(BaseDevice):
        h = self._check_connection()
        if not h: return

        with self._lock:
            last_exptime = self._last_exptime
            last_imagetype = self._last_imagetype
            last_datetime = self._last_datetime

        log.debug(f"Getting original data")
        # Bounded: an unbounded poll here left download() hung forever if
        # cooling was toggled mid-download and ArtemisImageReady() never
        # returned true (observed on hardware, see PLAN.md), same failure
        # shape stop_looping() already guards against elsewhere in this file.
        deadline = time.time() + (last_exptime or 0.0) + 10.0
        while not self.get('ArtemisImageReady'):
            if time.time() > deadline:
                msg = "Atik: download() timed out waiting for image ready"
                log.error(msg)
                self.error.append(msg)
                return None
            time.sleep(0.1)

        x, y, w, h_img, bx, by = [ctypes.c_int() for _ in range(6)]
@@ -313,8 +333,8 @@ class Camera(BaseDevice):
        hdr['INSTRUME'] = (self._props.Description.decode(), "Camera model")

        exptime = self.get('ArtemisLastExposureDuration')
        if exptime == 0.0 and self._last_exptime is not None:
            exptime = self._last_exptime
        if exptime == 0.0 and last_exptime is not None:
            exptime = last_exptime
        hdr['EXPTIME'] = (float(exptime), "[s] Exposure duration")

        start_time_bytes = self.get('ArtemisLastStartTime')
@@ -322,8 +342,8 @@ class Camera(BaseDevice):
        if start_time_bytes:
            hdr['DATE-OBS'] = (f"{start_time_bytes.decode()}.{start_ms:03d}",
                               "(UTC) Date the exposure was started")
        elif self._last_datetime is not None:
            hdr['DATE-OBS'] = (str(self._last_datetime),
        elif last_datetime is not None:
            hdr['DATE-OBS'] = (str(last_datetime),
                               "(UTC) Date the exposure was started")

        ccd_temp = ctypes.c_int()
@@ -336,14 +356,17 @@ class Camera(BaseDevice):
                 ctypes.byref(setp))
        hdr['SET-TEMP'] = (setp.value / 100.0, "[C] CCD setpoint temperature")

        with self._lock:
            subframe = self._subframe

        hdr['XPIXSZ'] = (self._props.PixelMicronsX, "[um] Pixel X size")
        hdr['YPIXSZ'] = (self._props.PixelMicronsY, "[um] Pixel Y size")
        hdr['XBINNING'] = (bx.value, "X binning factor")
        hdr['YBINNING'] = (by.value, "Y binning factor")
        hdr['XORGSUBF'] = (self._subframe[0], "[px] Subframe X origin (unbinned)")
        hdr['YORGSUBF'] = (self._subframe[1], "[px] Subframe Y origin (unbinned)")
        hdr['XORGSUBF'] = (subframe[0], "[px] Subframe X origin (unbinned)")
        hdr['YORGSUBF'] = (subframe[1], "[px] Subframe Y origin (unbinned)")

        ft = self._last_imagetype
        ft = last_imagetype
        hdr['IMAGETYP'] = (_frame_type.get(ft, str(ft)) if ft is not None else "",
                           "Image type")

@@ -444,6 +467,7 @@ class Camera(BaseDevice):
        h = self._check_connection()
        if h:
            self.put('ArtemisSubframe', start_x, start_y, width, height)
            with self._lock:
                self._subframe = [start_x, start_y, width, height]

    def full_frame(self):
@@ -458,6 +482,7 @@ class Camera(BaseDevice):
        nx, ny = self._props.nPixelsX, self._props.nPixelsY
        if h:
            self.put('ArtemisSubframe', 0, 0, nx, ny)
            with self._lock:
                self._subframe = [0, 0, nx, ny]
        return [nx, ny]

@@ -480,6 +505,7 @@ class Camera(BaseDevice):
        w, hh = x1 - x0, y1 - y0
        if h:
            self.put('ArtemisSubframe', x0, y0, w, hh)
            with self._lock:
                self._subframe = [x0, y0, w, hh]
        return [w, hh]

@@ -503,6 +529,7 @@ class Camera(BaseDevice):
        w, hh = x1 - x0, y1 - y0
        if h:
            self.put('ArtemisSubframe', x0, y0, w, hh)
            with self._lock:
                self._subframe = [x0, y0, w, hh]
        return [w, hh]

@@ -590,6 +617,7 @@ class Camera(BaseDevice):
    @property
    def xystart(self):
        """list of int : Sub-frame [X, Y] origin in binned pixels."""
        with self._lock:
            x, y, w, h = self._subframe
        bx, by = self.binning
        bx = bx or 1
@@ -599,6 +627,7 @@ class Camera(BaseDevice):
    @property
    def xyend(self):
        """list of int : Sub-frame [X, Y] end corner in binned pixels."""
        with self._lock:
            x, y, w, h = self._subframe
        bx, by = self.binning
        bx = bx or 1
@@ -644,6 +673,7 @@ class Camera(BaseDevice):
    @property
    def xrange(self):
        """list of int : Sub-frame X extent [start, end] in binned pixels."""
        with self._lock:
            x, y, w, h = self._subframe
        bx, by = self.binning
        bx = bx or 1
@@ -652,6 +682,7 @@ class Camera(BaseDevice):
    @property
    def yrange(self):
        """list of int : Sub-frame Y extent [start, end] in binned pixels."""
        with self._lock:
            x, y, w, h = self._subframe
        bx, by = self.binning
        by = by or 1
@@ -690,6 +721,7 @@ class Camera(BaseDevice):
        fan_power = round((level.value / 255.0) * 100) if maxl.value > 0 else 0

        b_x, b_y = bx.value, by.value
        with self._lock:
            x_sf, y_sf, w_sf, h_sf = self._subframe
        nx, ny = self._props.nPixelsX, self._props.nPixelsY
        x_start = x_sf // b_x if b_x else 0
+5 −5
Original line number Diff line number Diff line
@@ -101,11 +101,11 @@ class Meteo:
        print(f"Summary:")
        print(f"Datetime  {params0['Datetime']}")
        print(f"Dew Point {params0['Dewpoint']:.1f}°")
        print(f"TempIn    {params0['TempIn']:.1f}°")
        print(f"TempOut   {params0['TempOut']:.1f}°")
        print(f"HumIn     {params0['HumIn']}%")
        print(f"HumOut    {params0['HumOut']}%")
        print(f"WindSpeed {params0['WindSpeed']:.1f} km/h")
        print(f"TempIn    {params0['Temperature_first_floor']:.1f}°")
        print(f"TempOut   {params0['Temperature_meteo_station']:.1f}°")
        print(f"HumIn     {params0['Humidity_first_floor']}%")
        print(f"HumOut    {params0['Humidity_meteo_station']}%")
        print(f"WindSpeed {params0['Wind_speed']:.1f} km/h")
        print(f"Barometer {params0['Barometer']:.1f} hPa")

    @property