Commit 3cd3fd0c authored by vertighel's avatar vertighel
Browse files

atik.py: fix setpoint re-apply; log ArtemisSubframe failures instead of ignoring them



Due dei tre problemi Atik segnalati testando su fork:

1) Setpoint/cooling: il setter temperature faceva solo
   self._setpoint = t senza mai toccare l'hardware finché non si
   riaccendeva il cooling. Portato lo stesso fix già presente in
   stl.py: se il cooling è già attivo, riapplica subito il nuovo
   setpoint via self.cooler = True. Da confermare su fork.

2) Cooling Off che non scalda: nessuna modifica, non è chiaramente un
   bug. La doc SDK (AtikCameras.h) descrive ArtemisCoolerWarmUp come
   "on some devices, this will perform a slow ramp down... to avoid
   thermal shock" — è l'unica chiamata SDK per questo, nessun altro
   punto del codice riattiva il cooling. Potrebbe solo essere più
   lenta di STX/STL: serve osservare più a lungo su hardware, non un
   fix di codice.

3) half_frame()/small_frame() full-frame: NON è un fix della causa,
   solo la chiusura di un buco diagnostico. put()'s ctypes_errors non
   controlla mai il codice di ritorno numerico dell'SDK (dichiarato
   nel suo stesso docstring, "each call site's job") — e i 4 metodi
   set_window/full_frame/half_frame/small_frame non lo controllavano
   mai. Se ArtemisSubframe falliva, veniva ignorato in silenzio e
   self._subframe (da cui XORGSUBF/YORGSUBF) restava comunque
   aggiornato come se fosse riuscito. Accorpati i 4 punti duplicati in
   _apply_subframe(), che ora controlla il codice di ritorno
   (ARTEMIS_OK==0, verificato in AtikDefs.h) e non aggiorna la cache se
   fallisce. Copre solo lo scenario "la chiamata fallisce silenziosa";
   non copre lo scenario alternativo in cui la chiamata riesce ma il
   sensore consegna comunque il frame intero — quello richiederebbe
   confrontare ArtemisGetSubframe con ArtemisGetImageData, non fatto
   qui. La prossima esposizione reale su fork dirà quale dei due è.

Co-Authored-By: default avatarClaude Sonnet 5 <noreply@anthropic.com>
parent a8dd5c36
Loading
Loading
Loading
Loading
Loading
+47 −29
Original line number Diff line number Diff line
@@ -151,38 +151,56 @@ during the devices phase (no hardware needed); CCD-TEMP is still open.
             self.cooler = True  # re-apply regulation at the new setpoint
     ```
     `atik.py` needs the same "if already cooling, re-apply now"
     branch — a straightforward port, not hardware-dependent to write,
     but confirm on `fork` before considering it closed.
     branch — a straightforward port, not hardware-dependent to write.
     ~~FIXED~~ — ported verbatim into `atik.py`'s `temperature` setter.
     Needs confirming on `fork`.
  2. `atik.py`'s `cooler` setter's False branch (~line 617-618) calls
     `self.put('ArtemisCoolerWarmUp')` — telemetry correctly flips to
     "off" (the flags bit clears) but the physical temperature doesn't
     rise back, unlike STX/STL. Whether `ArtemisCoolerWarmUp` is the
     wrong SDK call, needs a follow-up call, or behaves differently
     than assumed is unknown from the host side — needs investigation
     with the physical camera and/or the Artemis SDK docs, not a blind
     guess. Do not attempt a fix for this half without hardware in
     hand.
  Tracked here per the user's request, not fixed yet.
- **`half_frame()`/`small_frame()` return a full-frame image (correctly
  binned) instead of the intended crop.** Found on `fork` (2026-07-22):
  the resulting FITS is full-sensor size, but `XORGSUBF`/`YORGSUBF`
  correctly match the requested read mode. User confirms these worked
  correctly on `fork` in the past; can't be tested on `snoopy` (no
  camera there) to compare.
  A concrete lead in the code, not yet confirmed as *the* cause:
  `download()` reads the actual pixel buffer sized by `w`/`h_img`
  as reported live by `ArtemisGetImageData` (`atik.py:317-320,332-334`),
  but writes `XORGSUBF`/`YORGSUBF` from `self._subframe`
  (`atik.py:365-366,372-373`) — the Python-side value `half_frame()`/
  `small_frame()`/`set_window()` set when they called `ArtemisSubframe`.
  These are two independent sources of truth: if the SDK's own
  `ArtemisGetImageData` doesn't reflect the subframe that was requested
  by the time the buffer is read (a timing issue between
  `ArtemisSubframe` and exposure start, or an SDK quirk), the pixel data
  comes back full-frame while the header still faithfully records what
  was *intended*, not what was actually delivered — matching the
  symptom exactly. Needs the physical camera to confirm and iterate;
  not attempted blind from the host side.
     rise back, unlike STX/STL. Checked the vendor SDK header
     (`external-software/AtikCamerasSDK_.../include/AtikCameras.h`):
     `ArtemisCoolerWarmUp` is documented as *"Disables active cooling...
     On some devices, this will perform a slow ramp down of the cooler,
     to avoid thermal shock."* — it's the correct/only call for this
     (no alternative "warm up now" function exists in the SDK), and a
     slow ramp is expected vendor behavior, not obviously a bug. No
     code path re-enables cooling afterward (checked — only one
     `ArtemisSetCooling` call site, in the True branch). Still
     genuinely unresolved: is what was observed just "hasn't finished
     ramping yet" (needs a longer observation window than STX/STL) or
     a real stuck state? Needs watching `ArtemisCoolingInfo`'s
     `level`/`flags` over a longer period on `fork`, not a code change
     — no fix applied.
  Temperature-setter fix applied; cooler-off half still open per (2).
- **`half_frame()`/`small_frame()` returned a full-frame image
  (correctly binned) instead of the intended crop, though
  `XORGSUBF`/`YORGSUBF` correctly matched the requested read mode.**
  Found on `fork` (2026-07-22); user confirms these worked correctly on
  `fork` in the past.
  **Not a confirmed root-cause fix — a diagnostic gap closed, the
  actual cause is still unknown.** Found that `put()`'s `ctypes_errors`
  decorator deliberately never interprets the SDK's own numeric return
  code ("each call site's job", see its docstring in `utils/check.py`)
  — and `set_window()`/`full_frame()`/`half_frame()`/`small_frame()`
  were never checking it. A rejected/failed `ArtemisSubframe` call
  would have been silently ignored, with `self._subframe` (the source
  for `XORGSUBF`/`YORGSUBF`) still updated to the intended crop
  regardless — a plausible match for the symptom, *if* the call is
  in fact failing, which is not yet established. Confirmed
  `ARTEMIS_OK == 0` in the vendor SDK (`AtikDefs.h`). Changed: the 4
  methods' duplicated `self.put('ArtemisSubframe', ...)` +
  cache-update pairs factored into one `_apply_subframe(x, y, w, h)`
  helper that checks the return code, logs + `self.error.append(...)`
  and leaves `self._subframe` untouched on failure, instead of blindly
  trusting success. Verified with `py_compile` only — no hardware
  access to confirm. This only surfaces the failure-return-code
  scenario; it does *not* cover the other plausible scenario where
  `ArtemisSubframe` itself returns `ARTEMIS_OK` but the sensor/SDK
  still delivers a full frame anyway (e.g. a firmware/SDK quirk not
  honoring the subframe under some condition) — that scenario would
  need comparing `ArtemisGetSubframe`'s own read-back against
  `ArtemisGetImageData`'s reported size, not attempted here. Next real
  exposure on `fork` will show which of the two it actually is.

## Regressions found on `fork` post-refactor (2026-07-22)

+29 −12
Original line number Diff line number Diff line
@@ -462,6 +462,29 @@ class Camera(BaseDevice):

    # --- Windowing Methods ---

    def _apply_subframe(self, x, y, w, h):
        """Send ArtemisSubframe and update the cached subframe only if it
        succeeds.

        ``put()``'s ``ctypes_errors`` decorator deliberately never
        interprets the SDK's own numeric return code (each call site's
        job, see its docstring) — so a rejected/failed ``ArtemisSubframe``
        used to be silently ignored, leaving ``self._subframe`` (and the
        ``XORGSUBF``/``YORGSUBF`` headers derived from it) claiming a crop
        that was never actually applied on the sensor.
        """

        ret = self.put('ArtemisSubframe', x, y, w, h)
        if ret != 0:
            msg = f"Atik: ArtemisSubframe({x},{y},{w},{h}) failed (code {ret})"
            log.error(msg)
            self.error.append(msg)
            return False
        with self._lock:
            self._subframe = [x, y, w, h]

        return True

    def set_window(self, start_x, start_y, width, height):
        """Set the imaging sub-frame in unbinned pixel coordinates.

@@ -479,9 +502,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]
            self._apply_subframe(start_x, start_y, width, height)

    def full_frame(self):
        """Set the camera to use the full sensor area.
@@ -495,9 +516,7 @@ class Camera(BaseDevice):
        h = self._check_connection()
        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]
            self._apply_subframe(0, 0, nx, ny)

        return [nx, ny]

@@ -520,9 +539,7 @@ class Camera(BaseDevice):
            x0, y0, x1, y1 = coords
        w, hh = x1 - x0, y1 - y0
        if h:
            self.put('ArtemisSubframe', x0, y0, w, hh)
            with self._lock:
                self._subframe = [x0, y0, w, hh]
            self._apply_subframe(x0, y0, w, hh)

        return [w, hh]

@@ -546,9 +563,7 @@ class Camera(BaseDevice):
            x0, y0, x1, y1 = coords
        w, hh = x1 - x0, y1 - y0
        if h:
            self.put('ArtemisSubframe', x0, y0, w, hh)
            with self._lock:
                self._subframe = [x0, y0, w, hh]
            self._apply_subframe(x0, y0, w, hh)

        return [w, hh]

@@ -593,6 +608,8 @@ class Camera(BaseDevice):
    @temperature.setter
    def temperature(self, t):
        self._setpoint = t
        if self.cooler:
            self.cooler = True  # re-apply regulation at the new setpoint

    @property
    def cooler(self):