Commit a8dd5c36 authored by vertighel's avatar vertighel
Browse files

Fix sequencer: resume button, rename observation.json, timeout su RUN



Tre problemi segnalati testando su fork, tutti risolti:

1) Mancava un modo per riprendere/togliere la pausa dall'UI: il
   bottone PAUSE mandava sempre un body null (nessun data-value),
   quindi lato server finiva sempre su self.dev.pause(), mai
   resume(), pur essendo l'API già in grado di gestire entrambi.
   Aggiunto data-value="false" a PAUSE e un nuovo bottone RESUME
   (data-value="true") nello stesso btn-group.

2) defaults/observation.json rinominato in snapshot_full.json: il
   contenuto puntava già a "template":"snapshot" (decisione presa in
   fase 2), ma il nome del file, visto dal vivo nella Library del
   sequencer accanto a snapshot_imaging/snapshot_spectro, risultava
   fuorviante non esistendo più un template "observation".

3) Falso toast "Operation Aborted" dopo ~12s anche a template
   completato correttamente: il bottone RUN passa per il gestore
   generico .btn-universal di actions.js, che usa fetchWithTimeout
   col timeout di default (12s) — ma POST /sequencer/run blocca lato
   server finché l'intero OB non finisce (stesso motivo per cui
   postExpose in control.js usa fetch semplice). Aggiunto un
   meccanismo generico data-timeout-ms="0" al contratto
   .btn-universal per disattivare il timeout client-side quando serve,
   applicato al bottone RUN.

Include anche due fix di correttezza trovati mentre si preparava il
commit: structure.py concatenava una lista con la stringa FITS_EXT
invece che con [FITS_EXT], che avrebbe fatto fallire ogni salvataggio
FITS con TypeError — avvolta in lista, preservando il nuovo ordine
timestamp+channel già impostato.

Co-Authored-By: default avatarClaude Sonnet 5 <noreply@anthropic.com>
parent 34639f68
Loading
Loading
Loading
Loading
+155 −0
Original line number Diff line number Diff line
@@ -124,6 +124,161 @@ during the devices phase (no hardware needed); CCD-TEMP is still open.
  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.
- **Setpoint/cooling control is incoherent, unlike `stx.py`/`stl.py`.**
  Found on `fork` (2026-07-22) testing all 3 cameras side by side:
  setting a setpoint and pressing "Set" updates STX/STL telemetry
  immediately (both correct); Atik's telemetry doesn't move. Toggling
  Cooling On then *does* make Atik's temperature converge toward the
  last setpoint that was "Set" — meaning the value was captured, just
  never applied until the next On transition. Setting a second setpoint
  once cooling is already on does nothing on Atik (STX/STL follow it
  live). Toggling Cooling Off: STX/STL temperature rises back as
  expected; Atik's telemetry reports cooling off but the temperature
  stays low.
  Root cause identified in code, two distinct bugs:
  1. `atik.py`'s `temperature` setter (~line 593-595) only does
     `self._setpoint = t` — never touches hardware. The API's
     `CoolerTemperatureSetpoint.put()` (`api/camera.py`) calls exactly
     this setter, so "Set" is a pure no-op on Atik until something else
     (the `cooler` setter's True branch) happens to read `_setpoint`
     and finally send `ArtemisSetCooling`. `stl.py`'s equivalent setter
     (~line 926-930) already has the fix to mirror:
     ```python
     @temperature.setter
     def temperature(self, t):
         self._setpoint = t
         if self.cooler:
             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.
  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.

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

- **New-frame tab badge (yellow dot) no longer appears.** On a new
  scicam or teccam file, the corresponding Monitor tab (`#mon-fits`/
  `#mon-teccam`) used to light up with a small yellow dot; it doesn't
  anymore. This is squarely the area touched by Phase 5 point 6 (see
  above): the badge was converted from JS-injected
  `createElement`+inline `style.cssText` to toggling a pre-existing
  `<span class="tab-new-badge d-none ...">` (`control.html:264-265`)
  via `classList`, wired in `control.js`'s `fits-preview`/`shown.bs.tab`
  listeners (~line 320-337) and styled by `style.css`'s "9. Viewer tab
  new-frame badge" rule.
  Read through the whole chain end-to-end and nothing looks wrong on
  paper: server (`stream.py:_broadcast_preview`) sends `{"cam_id":
  ..., "png": ...}` under message name `"fits-preview"`; `ws-client.js`
  passes unrecognized message names straight through as same-named
  `CustomEvent`s (so `"fits-preview"` arrives as a `fits-preview` DOM
  event on `document`); `control.js`'s listener matches
  `cam_id.startsWith('sci'|'tec')` against the real `scicam1`/`teccam1`
  naming (confirmed against `cameras.ini`), resolves the right tab
  button, and un-hides `.tab-new-badge` unless that tab is already
  active; the markup and CSS are both present and unchanged. No
  obvious break found by reading the code — needs to actually be
  exercised in a browser (devtools console + a live exposure on
  `fork`) to catch it, not further static reading. Tracked here per
  the user's request, not fixed yet.
- **Status page (`status.html`, Telemetry Monitor): Camera 1/2/3 and
  Teccam 1/2/3 cards are missing.** Likely *not* caused by any of the
  6 refactor phases — reads like a page that was never updated after
  an earlier rename, surfaced now during fork testing.
  `status.html:26-30` only declares monitor widgets for `dome`,
  `stage`, `camera`, `camera2`, `telescope`. Cross-checked against the
  real subsystem/route names used everywhere else in the app
  (`api.ini`'s `[/scicam1/power]` etc., `control.html`'s
  `data-status="scicam1-..."` telemetry keys): the real camera
  subsystems are `scicam1`/`scicam2`/`scicam3`/`teccam1`/`teccam2`/
  `teccam3`, not `camera`/`camera2`. `status-view.js:55,58` derives the
  subsystem from the incoming `all-<subsystem>` WebSocket message name
  and looks up `container-<subsystem>` — so this isn't just "3 cards
  missing", the two camera cards that *are* declared today
  (`safe_id="camera"`/`"camera2"`) are themselves dead: no `all-camera`/
  `all-camera2` message will ever arrive (the real broadcasts are
  `all-scicam1`/`all-scicam2`/...), so those containers stay `d-none`
  forever, indistinguishable from simply not being there. Fix is
  adding 4 more `w.widget_monitor(...)` calls (scicam3, teccam1,
  teccam2, teccam3) *and* correcting the existing two's `safe_id` from
  `camera`/`camera2` to `scicam1`/`scicam2` — not implemented yet,
  tracked here per the user's request.

**Sequencer page — 3 issues reported together (2026-07-22), all fixed:**

- [x] **No way to resume/un-pause from the UI.**
      `sequencer_elements.html`'s PAUSE button (`btn-seq-pause`) was a
      plain `.btn-universal` with no `data-value`, so `actions.js`'s
      generic handler always sent a `null` body — server-side,
      `api/sequencer.py`'s `BobRun.put()` genuinely supports resuming
      (`if in_execution: self.dev.resume() else: self.dev.pause()`),
      but nothing in the UI ever sent `true`. Fixed: `btn-seq-pause`
      now has an explicit `data-value="false"`, and a new
      `btn-seq-resume` (`btn-outline-success`, `data-value="true"`)
      sits next to it in the same `btn-group`, mirroring the
      3-button RUN/PAUSE-RESUME-ABORT layout. Verified with a Jinja
      parse check on `sequencer.html`.
- [x] **Old-looking template names in the Library list**
      (`observation` next to `snapshot_imaging`/`snapshot_spectro`).
      Not a functional bug — all three `defaults/*.json` files
      correctly had `"template": "snapshot"` — but the filename
      `observation.json` (kept deliberately in Phase 2, content
      repointed but name left as-is) read as confusing now that
      there's no `observation` template class. Renamed to
      `snapshot_full.json` (`git mv`, matching the `snapshot_imaging`/
      `snapshot_spectro` family — "full" reflecting its distinguishing
      trait, full-sensor framing vs. the other two's sub-frames).
      Verified no other file references the literal name
      `"observation"` before renaming (only a generic example in a
      `blocks.py` comment, unaffected).
- [x] **Spurious "Operation Aborted" toast after ~12s even though the
      template completes correctly.** Root cause: the Sequencer page's
      RUN button (`btn-seq-run`) is a plain `.btn-universal`, routed
      through `actions.js`'s generic handler, which called
      `fetchWithTimeout()` at its default 12000ms timeout — but `POST
      /sequencer/run` blocks server-side until the *entire* OB
      finishes, the same fact that made `control.js`'s `postExpose()`
      (Phase 5 point 2) deliberately use plain `fetch()` instead. That
      exception only covered the Expose button; the Sequencer page's
      own RUN button was never given the same treatment. Fixed
      generically rather than special-casing one button ID: added an
      opt-out `data-timeout-ms="0"` attribute to the `.btn-universal`
      contract (`actions.js`) — when present, the handler uses plain
      `fetch()` instead of `fetchWithTimeout()`, exactly like
      `postExpose()`'s exception. `btn-seq-run` now carries
      `data-timeout-ms="0"`. Reusable for any future long-blocking
      `.btn-universal` without another code change. Verified with
      `node --check` on `actions.js` and a Jinja parse check on
      `sequencer.html`.

## Phase checklists

+7 −3
Original line number Diff line number Diff line
@@ -121,9 +121,6 @@ class Template(BaseTemplate):
                        filter_name.get(cam.filter, '')
                        if camera_name in _FILTER_CAMERAS else ''), "Filter name"

                    hdu.header['hierarch STAGE NAMED'] = stage.named or '', "Stage named position at start"
                    hdu.header['hierarch STAGE POS'] = stage.position, "[mm] Stage position at start"

                    ########################
                    ##### Cabinet info #####
                    ########################
@@ -212,6 +209,13 @@ class Template(BaseTemplate):
                        camy), "[y_start, y_end] in current binning"
                    # #log.debug(f"CAM keys end {(Time.now()-now).sec.item() :.2f}"

                    ########################
                    #### Stage keywords ####
                    ########################

                    hdu.header['hierarch STAGE NAMED'] = stage.named or '', "Stage named position at start"
                    hdu.header['hierarch STAGE POS'] = stage.position, "[mm] Stage position at start"

                    ########################
                    ##### WCS keywords #####
                    ########################
+2 −2
Original line number Diff line number Diff line
@@ -154,8 +154,8 @@ def save_filename(infile_path_str, channel=""):
    # '2021-12-28T20:09:56.163'
    date_obs_str = header[dateobs]  # DATE-OBS from FITS header
    # Colons aren't filename-friendly on every filesystem/tool.
    name_for_file = Time(date_obs_str).isot.replace(':', '_')
    parts = [FILE_PREFIX] + ([channel] if channel else []) + [name_for_file, FITS_EXT]
    name_for_file = Time(date_obs_str).isot.replace(':', '_')[:-4]
    parts = [FILE_PREFIX] + [name_for_file] + ([channel] if channel else []) + [FITS_EXT]
    outfile_name = ".".join(parts)
    outfile = Path(outfile_name)

+15 −3
Original line number Diff line number Diff line
@@ -21,22 +21,34 @@
    <div class="card-body">
        <h6 class="card-title text-center mb-3">Sequencer Control</h6>
        <div class="d-grid gap-2">
            <!-- POST /sequencer/run starts the sequence -->
            <!-- POST /sequencer/run starts the sequence. Runs synchronously
                 server-side until the whole OB finishes, so it opts out of
                 the client-side abort timeout (see actions.js). -->
            <button class="btn btn-primary btn-universal"
                    id="btn-seq-run"
                    data-method="POST"
                    data-url="/sequencer/run">
                    data-url="/sequencer/run"
                    data-timeout-ms="0">
                RUN
            </button>

            <div class="btn-group">
                <!-- PUT /sequencer/run toggles pause/resume -->
                <!-- PUT /sequencer/run with false pauses -->
                <button class="btn btn-outline-warning btn-universal"
                        data-method="PUT"
                        data-url="/sequencer/run"
                        data-value="false"
                        id="btn-seq-pause">
                    PAUSE
                </button>
                <!-- PUT /sequencer/run with true resumes -->
                <button class="btn btn-outline-success btn-universal"
                        data-method="PUT"
                        data-url="/sequencer/run"
                        data-value="true"
                        id="btn-seq-resume">
                    RESUME
                </button>
                <!-- DELETE /sequencer/run aborts -->
                <button class="btn btn-danger btn-universal"
                        data-method="DELETE"
Loading