Commit 34639f68 authored by vertighel's avatar vertighel
Browse files

Fase 6 (UX-linearity) completa; fix fillheader.py; pass style Python; v0.9.5



Fase 6: audit di init/control/sequencer + macro/JS collegati. Bug reali
trovati e risolti: stage relative move morto (macro commentata + JS
mai raggiungibile, eliminati entrambi su richiesta), markup orfano in
control_panel.html. Incoerenze di interazione risolte: colori
start/stop del sequencer omogeneizzati su Expose/Guide, bottoni
webcam move e SET di NoctuaWidget allineati alla convenzione
btn-outline-primary per le azioni atomiche. Rimosso anche il selettore
CSS morto .cv-teccam (viewer.css), flaggato in fase 5.

fillheader.py: corretto bug reale, gli errori di scrittura header FITS
venivano solo loggati senza self.error/return, con salvataggio del
file comunque eseguito. Ora allineato al pattern standard.

Pass di stile Python su tutto noctua/**/*.py secondo dev/conventions/
python.md: riga vuota dopo docstring (203 casi), riga vuota prima del
return finale (269 casi), 2 righe vuote tra top-level, 1 riga vuota
tra metodi di classe, allineamento = nei blocchi self.x = ... — tutte
modifiche meccaniche di solo whitespace, verificate con py_compile.

Bump versione a 0.9.5.

Co-Authored-By: default avatarClaude Sonnet 5 <noreply@anthropic.com>
parent b4f2368c
Loading
Loading
Loading
Loading
Loading
+136 −29
Original line number Diff line number Diff line
@@ -214,9 +214,14 @@ because a file is open for another reason; re-scope explicitly first.
      module load — leaving it broken would have made the promoted
      `focus.py` unimportable too. Rest of `box.py` stays deferred as
      scoped.
- [ ] `fillheader.py:212-217` swallows FITS-writing errors silently
      (logs only, no `self.error`, no `return`, falls through to save
      anyway) — align with the standard error pattern. In scope.
- [x] `fillheader.py` swallowed FITS-writing errors silently (logs
      only, no `self.error`, no `return`, falls through to save anyway).
      Fixed: both `except FileNotFoundError`/`except Exception` blocks
      now build a message, log it, `self.error.append(msg)`, and
      `return` — matching `snapshot.py:179-183`'s pattern. A header
      update failure now correctly skips the save/archive step instead
      of copying a file whose header was never actually written.
      Verified with `py_compile`.
- [x] Real observatory fixes/features requested directly by the user
      (not from the Phase-0 audit), all in `fillheader.py`, all in
      scope:
@@ -282,10 +287,10 @@ because a file is open for another reason; re-scope explicitly first.
      names the observatory site, `Cerbero` the instrument, and
      `FILE_PREFIX` is shared with `current_log_path()`/`foc_path()`,
      not just FITS output).
- [ ] Remove dead/commented-out blocks: `fillheader.py` (large
      commented WCS block) — user is removing this one directly, not
      Claude. `observation.py:28-33` is moot, that file is gone (see
      above).
- [x] Remove dead/commented-out blocks: `fillheader.py`'s large
      commented WCS block removed directly by the user (not Claude,
      per the original scoping). `observation.py:28-33` is moot, that
      file is gone (see above).
- [ ] *(deferred)* Delete `testsonoff.py`.
- [ ] *(deferred)* Unify error handling: most templates use
      `except KeyError: log.error(...)` without `self.error.append()`
@@ -306,10 +311,18 @@ because a file is open for another reason; re-scope explicitly first.
      isn't honored.
- [ ] *(deferred)* Add docstrings to the remaining templates that
      have none: `bias.py`, `box.py`, `testlamp.py`, `testpause.py`.
- [ ] *(deferred)* Remove dead/commented-out blocks in
      `sequencer.py:37-38,218-219,252-259`.
- [ ] *(deferred)* Translate remaining Italian comments
      (`sequencer.py:40,42,47`, `skyflat.py:78`).
- [x] Remove dead/commented-out blocks in `sequencer.py` (was
      `:37-38,218-219,252-259`): the two commented-out signal-handling
      lines in `__init__`, the commented `answer = input(...)` line in
      `interrupt()`, and the commented-out `except EOFError:` block
      right after it. Scoped to `sequencer.py` only, on the user's
      request.
- [x] Translate remaining Italian comments in `sequencer.py` (was
      `:40,42,47`): `__init__`'s three comments ("Gestione Segnali
      condizionale", "Solo se lanciato come script standalone...", "Se
      usato nella API...") now in English. `skyflat.py:78` still has
      Italian — *(deferred)*, out of this pass's scope (sequencer.py
      only, per the user).
- [ ] *(deferred)* `testpause.py` is the only file using
      `@check.content_errors` — an orphaned pattern (its failure mode,
      `self.paused = True`, doesn't match the standard
@@ -589,16 +602,16 @@ Working point by point per the user's request, checking in after each
        `fits-viewer.js:98`), an Italian/English naming drift.
        Renamed both occurrences to `.cv-main`, so it now actually
        gets `user-select:none`/`-webkit-user-drag:none`/`pixelated`
        image-rendering. **Not** fixed, still dead, flagged only:
        `.cv-teccam` (same file) doesn't match anything either —
        grepped every page/macro/JS file, no `cv-teccam` class exists
        anywhere, so unlike `.cv-principale` this isn't a rename typo,
        it looks like the teccam canvas either never got this class or
        was renamed away with the rule left behind. Left alone since
        the user asked specifically about the `.cv-main` mismatch, not
        this one — separate decision needed on whether the teccam
        canvas should get the class or the dead rule should just be
        deleted.
        image-rendering. `.cv-teccam` (same file) didn't match anything
        either — grepped every page/macro/JS file, no `cv-teccam` class
        existed anywhere, so unlike `.cv-principale` this wasn't a
        rename typo, more likely the teccam canvas either never got
        this class or was renamed away with the rule left behind. Left
        alone at the time (separate decision needed); resolved later,
        during Phase 6 — user confirmed the class isn't needed, so the
        dead `.cv-teccam` selector was deleted from both rule groups in
        `viewer.css` (kept `.cv-main`/`.cv-panoramic`/`.cv-explore`,
        confirmed still live via `fits-viewer.js`/`viewer_panel.html`).
      Verified with `node --check` on all touched JS and a Jinja parse
      check on all touched HTML.
- [x] Remove dead code: `noctuaUpdateFromRest` + its window shim,
@@ -638,10 +651,90 @@ Working point by point per the user's request, checking in after each

### 6. Cross-cutting UX-linearity pass

Not started — do last, once every layer above is individually
cleaner. Walk the real user flows (init, control, guider, sequencer)
looking for inconsistent interaction patterns across the three
stations/panels.
Not started as implementation, but the audit pass is done: walked
init.html, control.html, sequencer.html, their macros
(`control_panel.html`, `sequencer_elements.html`, `widgets.html`,
`webcam_panel.html`, `widget_blueprints.html`) and the JS behind them
(`control.js`, `guider-panel.js`, `sequencer.js`). Findings below,
ordered roughly by confidence/impact — not yet scoped or actioned,
waiting on the user's prioritization.

**Real bugs found (not just style):**

- [x] **Stage relative move was completely dead in the UI.**
      `control.html:125-127` had `ctrl.stage_relative()` commented out,
      while `control.js:141-181` (written during Phase 5, with its own
      JSDoc and the mutual +/- disable logic) fully wired
      `#stage-rel-val`/`#btn-stage-rel-plus`/`#btn-stage-rel-minus`
      elements that never existed in the rendered page. User's call:
      drop the feature rather than resurrect it — removed the
      commented-out macro call from `control.html`, the `stage_relative()`
      macro itself from `control_panel.html`, and the whole dead block
      from `control.js` (the two element lookups, `moveStageRelative()`,
      and its two listeners). Verified: `grep` for `stage-rel`/
      `stage_relative` across the codebase now empty, `node --check` and
      a Jinja parse check on `control.html` both clean.
- [x] **Orphaned/malformed markup in `control_panel.html:253-263`.** A
      top-level `<fieldset>` (an Exptime input referencing an undefined
      `{{ camera_id }}`) sat between the `guider_panel` and
      `imaging_panel` macro definitions, outside any `{% macro %}`
      block — harmless at runtime (Jinja `{% import %}` never renders a
      module's top-level output) but dead, broken-if-ever-executed
      leftover, looked like a copy/paste draft of `expose_widget()`'s
      Exptime field abandoned mid-edit. Deleted. Verified: Jinja parse
      check on `control.html` clean, remaining `{{ camera_id }}`
      occurrences in the file confirmed to all be inside macros that
      define that parameter.

**Interaction-pattern inconsistencies across panels:**

- [x] **Three different color schemes for the same "start a running
      process, must explicitly stop" pattern**: Expose (control.html,
      scicam) and Guide (control.html, guider) both used solid
      `btn-primary`/solid `btn-danger`; the Sequencer page's own
      RUN/PAUSE/ABORT (`sequencer_elements.html`) used a third
      vocabulary, `btn-success`/`btn-outline-warning`/`btn-outline-danger`.
      Homogenized onto the Expose/Guide scheme (also the one the
      [[project_button_conventions]] memory documents for sequencer
      ops): RUN → `btn-primary`, ABORT → `btn-danger` (solid). PAUSE
      left as `btn-outline-warning` — it's a third action with no
      equivalent in the Expose/Guide 2-button pairs, nothing to
      homogenize it against.
- [x] **Webcam move buttons** (`webcam_panel.html:45-51`) used solid
      `btn-secondary`; the established convention
      ([[project_button_conventions]]) calls for `btn-outline-primary`
      on atomic device calls, and this panel is embedded in both
      init.html and control.html. Changed all 4 (↑↓←→).
- [x] `widget_blueprints.html` (NoctuaWidget component, only used on
      `snippet_viewer.html` — a generic schema-driven catalog/
      introspection tool for arbitrary API routes, not one of the four
      curated operator flows). Revisited with the user: the On/Off pair
      (`btn-outline-success`/`btn-outline-danger`, two buttons) staying
      different from `widgets.html`'s single swap-button
      `btn-outline-primary btn-onoff` pattern was judged a legitimate
      difference, not an inconsistency to fix — a generic widget can't
      know in advance what the two states mean, so an explicit
      color-coded ON/OFF pair reads better there than the curated
      dashboards' single button. The "SET" button (dual-input and
      single-input blueprints) *was* a real violation of the documented
      convention (solid `btn-primary` on an atomic PUT/POST write) —
      fixed, both occurrences now `btn-outline-primary`. Verified safe:
      `NoctuaWidget.js` selects these buttons by the `.btn-control`
      class, never by color class. Jinja parse check on
      `snippet_viewer.html` clean.
- [x] *(design question, not a bug)* Sequencer editor: removing a step
      (`.btn-remove-step`) has no confirmation, while deleting a whole
      OB (`sequencer.js:158`, `confirm()`) does. User confirmed this is
      intentional (different stakes/reversibility) — left as is, no
      change.

All Phase 6 audit findings closed. Not re-listing the still-open
Phase-2/4 deferred items here — those already have their own entries
above. The `.cv-teccam` dead-CSS flag (raised during Phase 5, see its
entry above) was also resolved during this phase: user confirmed the
class wasn't needed, selector deleted from `viewer.css`.

**Phase 6 done.**

## Status

@@ -669,7 +762,21 @@ blast-radius warning) deferred, not started.

Phase 5 (JavaScript) done, full scope, all 9 points worked through
one at a time. Two dead-CSS-selector bugs found and fixed along the
way (`.cv-principale``.cv-main`); `.cv-teccam` flagged but left
alone (separate decision needed).

**Phase 6 (Cross-cutting UX-linearity pass) not started.**
way (`.cv-principale``.cv-main`); `.cv-teccam` flagged, resolved in
Phase 6 (see below).

Phase 6 (Cross-cutting UX-linearity pass) done: audited init.html,
control.html, sequencer.html and their macros/JS, found and fixed two
real bugs (dead stage-relative-move feature, orphaned malformed
markup in `control_panel.html`), homogenized the three different
start/stop button color schemes, fixed two more atomic-button
convention violations (webcam move, NoctuaWidget SET), confirmed one
interaction difference (sequencer step-remove vs OB-delete
confirmation) as intentional, and closed out the `.cv-teccam` dead-CSS
flag left over from Phase 5.

**All 6 phases of the v0.9 → v1.0 cleanup plan are now done.** Open
items remaining are all explicitly deferred/tracked, not lost: the
Phase-2 deferred templates list, the Phase-4 Sass migration
sub-phase, `fillheader.py`'s silent-error-swallow fix, and the
hardware-only CCD-TEMP bug on `atik.py` (needs the physical camera).
+3 −0
Original line number Diff line number Diff line
@@ -100,8 +100,10 @@ _CAMERAS_INI = Path(__file__).parent.parent / 'config' / 'cameras.ini'
@api_blueprint.route('/cameras')
async def api_cameras():
    """Return the full cameras.ini as a JSON object keyed by cam_id."""

    cfg = configparser.ConfigParser()
    cfg.read(_CAMERAS_INI)

    return jsonify({
        cam_id: dict(cfg[cam_id])
        for cam_id in cfg.sections()
@@ -114,6 +116,7 @@ async def api_catalog():
    Return the flat JSON list of endpoints, methods, and their input
    metadata as declared through the @expects decorator.
    """

    catalog = []
    
    for url_path, resource_instance in resource_registry.items():
+10 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ class BaseResource(MethodView):
        dev : object
            The hardware device instance (Layer 1) or Sequencer (Layer 2).
        """

        super().__init__()
        self.dev = dev

@@ -37,6 +38,7 @@ class BaseResource(MethodView):
        str
            The current timestamp string.
        """

        return datetime.utcnow().isoformat()

    async def get_payload(self):
@@ -48,6 +50,7 @@ class BaseResource(MethodView):
        dict or list or None
            The parsed JSON data.
        """

        return await request.get_json(force=True, silent=True)

    async def run_blocking(self, func, *args, **kwargs):
@@ -69,6 +72,7 @@ class BaseResource(MethodView):
        any
            The result of the function call.
        """

        return await asyncio.to_thread(func, *args, **kwargs)

    def make_response(self, response_data, raw_data=None, errors=None, status_code=200):
@@ -91,6 +95,7 @@ class BaseResource(MethodView):
        tuple
            A tuple of (response dict, status_code).
        """

        dev_errors = getattr(self.dev, 'error', [])
        final_errors = errors if errors is not None else dev_errors

@@ -110,6 +115,7 @@ class BaseResource(MethodView):
        """
        Override the default dispatch to enforce cached dependency checks.
        """

        api_path = request.path.replace("/api", "", 1)
        handler = getattr(self, request.method.lower(), None)
        
@@ -142,6 +148,7 @@ def register_error_handlers(app):
    app : Quart
        The Quart application instance.
    """

    @app.errorhandler(400)
    async def bad_request(e):
        return jsonify({"error": ["Bad Request"], "timestamp": datetime.utcnow().isoformat()}), 400
@@ -167,6 +174,7 @@ def expects(param_type="single", count=1, unit=None, placeholder=None):
    decorated function.

    """

    def decorator(func):
        func._input_schema = {
            "type": param_type,       # "string", "number", "array", "boolean", or None
@@ -174,5 +182,7 @@ def expects(param_type="single", count=1, unit=None, placeholder=None):
            "unit": unit,             # unit of measurement (e.g. "°C", "mm")
            "placeholder": placeholder # example value(s) for the UI
        }

        return func

    return decorator
+5 −5
Original line number Diff line number Diff line
@@ -19,7 +19,9 @@ class BlocksList(MethodView):

    async def get(self):
        """List all OB files available in the OB folder."""

        res = await dao.list_available()

        return res


@@ -30,8 +32,8 @@ class BlockFile(MethodView):
        """Show the whole OB content."""
        
        content = await dao.read(name)
        return content if content is not None else ({"error": "OB not found"}, 404)

        return content if content is not None else ({"error": "OB not found"}, 404)

    async def post(self, name):
        """
@@ -58,8 +60,8 @@ class BlockFile(MethodView):
            final_data = []

        await dao.write(name, final_data)
        return final_data, 201

        return final_data, 201

    async def put(self, name):
        """Append a template from defaults to the existing OB."""
@@ -81,13 +83,13 @@ class BlockFile(MethodView):
                
        return {"error": f"Template '{tpl_name}' not found in defaults"}, 404


    async def delete(self, name):
        """Delete the whole OB file."""
        
        success = await dao.delete(name)
        if success:
            return {"message": f"OB {name} deleted"}, 200

        return {"error": "OB not found"}, 404


@@ -103,7 +105,6 @@ class BlockElement(MethodView):
        except (IndexError, TypeError, KeyError):
            return {"error": "Index out of range or OB not found"}, 404


    async def put(self, name, index):
        """Update a specific template inside the OB."""
        
@@ -116,7 +117,6 @@ class BlockElement(MethodView):
        except (IndexError, TypeError, KeyError):
            return {"error": "Index out of range or OB not found"}, 404


    async def delete(self, name, index):
        """Delete a specific template inside the OB."""
        
+37 −0
Original line number Diff line number Diff line
@@ -21,10 +21,13 @@ class FrameBinning(BaseResource):
        binning = await self.get_payload()
        def action():
            self.dev.binning = binning

            return self.dev.binning
        res = await self.run_blocking(action)

        return self.make_response(res)


class Gain(BaseResource):
    """Camera gain (dB)."""

@@ -32,6 +35,7 @@ class Gain(BaseResource):
        """Retrieve the current gain."""

        res = await self.run_blocking(lambda: getattr(self.dev, 'gain', None))

        return self.make_response(res)

    @expects(param_type="number", unit="dB", placeholder="1")
@@ -41,10 +45,13 @@ class Gain(BaseResource):
        gain = await self.get_payload()
        def action():
            self.dev.gain = gain

            return self.dev.gain
        res = await self.run_blocking(action)

        return self.make_response(res)


class Cooler(BaseResource):
    """Manage the CCD cooler status"""

@@ -53,6 +60,7 @@ class Cooler(BaseResource):
        
        raw = await self.run_blocking(lambda: self.dev.cooler)
        res = constants.on_off.get(raw, "N/A")

        return self.make_response(res, raw_data=raw)

    @expects(param_type="boolean")
@@ -62,10 +70,13 @@ class Cooler(BaseResource):
        state = await self.get_payload()
        def action():
            self.dev.cooler = state

            return self.dev.cooler
        res = await self.run_blocking(action)

        return self.make_response(res)


class CoolerTemperatureSetpoint(BaseResource):
    """Manage the CCD temperature"""

@@ -76,10 +87,13 @@ class CoolerTemperatureSetpoint(BaseResource):
        state = await self.get_payload()
        def action():
            self.dev.temperature = state

            return self.dev.temperature
        res = await self.run_blocking(action)

        return self.make_response(res)

    
class Filter(BaseResource):
    """Camera filter information."""

@@ -89,8 +103,10 @@ class Filter(BaseResource):
        raw = await self.run_blocking(lambda: getattr(self.dev, 'filter', 0))        
        #raw = await self.run_blocking(lambda: self.dev.filter)
        res = constants.filter_name.get(raw, "Undef.")

        return self.make_response(res, raw_data=raw)


class FilterMovement(BaseResource):
    """Manage the camera filter wheel."""

@@ -100,6 +116,7 @@ class FilterMovement(BaseResource):
        raw = await self.run_blocking(lambda: getattr(self.dev, 'is_moving', 0))
        # raw = await self.run_blocking(lambda: self.dev.is_moving)
        res = constants.filter_state.get(raw, "Off")

        return self.make_response(res, raw_data=raw)

    @expects(param_type="string", placeholder="FREE")
@@ -109,10 +126,13 @@ class FilterMovement(BaseResource):
        target = await self.get_payload()
        def action():
            self.dev.filter = target

            return self.dev.filter
        res = await self.run_blocking(action)

        return self.make_response(res)

    
class FrameFull(BaseResource):
    """Camera full frame."""

@@ -120,8 +140,10 @@ class FrameFull(BaseResource):
        """Set the ccd to full frame in current binning."""
        
        res = await self.run_blocking(self.dev.full_frame)

        return self.make_response(res)


class FrameHalf(BaseResource):
    """Camera frame of half size the full frame."""

@@ -130,8 +152,10 @@ class FrameHalf(BaseResource):
        size of the full frame in the current binning."""
        
        res = await self.run_blocking(self.dev.half_frame)

        return self.make_response(res)


class FrameSmall(BaseResource):
    """Camera frame of 2 arcmin."""

@@ -140,8 +164,10 @@ class FrameSmall(BaseResource):
        on the sky."""
        
        res = await self.run_blocking(self.dev.small_frame)

        return self.make_response(res)


class SnapshotRaw(BaseResource):
    """The acquired image."""

@@ -168,8 +194,10 @@ class SnapshotRaw(BaseResource):
        """Stop the process."""
        
        res = await self.run_blocking(self.dev.abort)

        return self.make_response(res)


class SnapshotState(BaseResource):
    """Manage the state of a raw image."""

@@ -178,8 +206,10 @@ class SnapshotState(BaseResource):
        
        raw = await self.run_blocking(lambda: self.dev.state)
        res = constants.camera_state.get(raw, "Off")

        return self.make_response(res, raw_data=raw)


class Settings(BaseResource):
    '''General camera settings.'''

@@ -187,8 +217,10 @@ class Settings(BaseResource):
        '''Retrieve all-in-one the settings of the camera.'''

        res = await self.run_blocking(lambda: self.dev.all)

        return self.make_response(res)


class Loop(BaseResource):
    """Continuous acquisition loop.

@@ -199,6 +231,7 @@ class Loop(BaseResource):

    async def get(self):
        """Return whether the acquisition loop is running."""

        return self.make_response(self.dev.looping, raw_data=self.dev.looping)

    @expects(param_type="number", unit="s", placeholder="1.0")
@@ -207,6 +240,7 @@ class Loop(BaseResource):

        Body: {"exptime": <float>, "binning": <int>, "gain": <float>}
        """

        body     = await self.get_payload()
        exposure = float(body.get('exptime', 1.0))
        binning  = body.get('binning')
@@ -230,9 +264,12 @@ class Loop(BaseResource):

        self.dev.loop_exposure = exposure
        self.dev.looping = True

        return self.make_response({'running': True, 'exposure': exposure})

    async def delete(self):
        """Stop the acquisition loop."""

        self.dev.looping = False

        return self.make_response({'running': False})
Loading