Skip to content

fix(laser-engine): hide temperature columns for channels the engine doesn't measure - #613

Open
Alpaca233 wants to merge 1 commit into
masterfrom
fix/laser-engine-tab-hide-unmeasured-temps
Open

fix(laser-engine): hide temperature columns for channels the engine doesn't measure#613
Alpaca233 wants to merge 1 commit into
masterfrom
fix/laser-engine-tab-hide-unmeasured-temps

Conversation

@Alpaca233

Copy link
Copy Markdown
Collaborator

Problem

On engines where a channel's TEC controller is not wired to the laser engine, the firmware still emits a TCM block for that channel, but its temperature and ΔT are floor readings rather than measurements. The Laser Engine tab rendered them verbatim:

638  OFF  ACTIVE     0.4 °C  ΔT -24.6
730  OFF  ACTIVE     0.3 °C  ΔT -24.7

An ACTIVE channel sitting ~25 °C below setpoint reads as a fault to anyone standing at the scope, when in fact the laser is fine and stabilized by a standalone controller.

Approach

Detect it from the data instead of from configuration. Firmware only holds ACTIVE while a channel is at setpoint — it drops back to WARMING_UP below −0.5 °C and to CHECK_ERROR at the +5 °C error threshold. So a module reporting ACTIVE well outside that band cannot be one this engine regulates:

def _is_engine_measured(module) -> bool:
    if module.state != LaserChannelState.ACTIVE:
        return True
    return -0.5 < module.setpoint_diff_c < 5.0

Those channels now render as:

638  OFF  ACTIVE     TEC not engine-controlled
730  OFF  ACTIVE     TEC not engine-controlled

Why not a config option

No config surface and nothing machine-specific — it self-configures, doing the right thing on a fully wired engine and on one with any channel unwired, with no per-machine setting to keep in sync.

The rule is deliberately narrow: it suppresses numbers only for ACTIVE modules. A channel genuinely WARMING_UP far from setpoint, or SLEEP drifting away from it, still shows its real readings — those numbers are meaningful.

Tradeoff

This couples the display to a firmware constant. If TEMP_ERROR_THRESHOLD or the −0.5 °C band changes, the numbers simply reappear for these channels — cosmetic, not functional.

Testing

New software/tests/control/test_laser_engine_widget.py — 6 tests, using values measured on real hardware (ACTIVE at ΔT −24.6 suppressed; WARMING_UP at ΔT −66.9 and SLEEP at ΔT −42.3 both preserved). Full suite: 49 passed. Verified live against a Squid laser engine; black clean.

🤖 Generated with Claude Code

…oesn't measure

On engines where a channel's TEC controller is not wired to the laser engine,
the firmware still emits a TCM block for it, but the temperature and dT columns
are floor readings rather than measurements. The tab rendered them verbatim, so
such a channel appeared ACTIVE while sitting ~25 C below setpoint - which reads
as a fault to anyone at the scope.

Detect this from the data rather than from configuration: firmware only holds
ACTIVE while a channel is at setpoint (it drops to WARMING_UP below -0.5 C and
to CHECK_ERROR at the +5 C error threshold), so a module reporting ACTIVE well
outside that band cannot be one this engine regulates. Those channels now show
their state and "TEC not engine-controlled" in place of the numbers.

The rule is deliberately narrow: it suppresses numbers only for ACTIVE modules,
so a channel genuinely WARMING_UP far from setpoint, or SLEEP drifting away
from it, still shows its real readings. No config surface and nothing
machine-specific, so it also does the right thing on a fully wired engine.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Laser Engine tab UI logic to suppress misleading temperature/ΔT readouts for channels whose TECs are not actually regulated/measured by the engine (detected heuristically from an ACTIVE state while far from setpoint), and adds unit tests covering the new rule.

Changes:

  • Added _is_engine_measured() heuristic to detect when ACTIVE temperature/ΔT values are likely floor readings rather than real measurements.
  • Updated channel line rendering to show a “TEC not engine-controlled” message instead of temperature/ΔT when a channel appears unmeasured.
  • Added unit tests validating measured vs unmeasured behavior across ACTIVE, WARMING_UP, and SLEEP.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
software/control/laser_engine_widget.py Adds the “engine-measured” heuristic and suppresses temperature/ΔT display when readings are likely not engine-controlled.
software/tests/control/test_laser_engine_widget.py Adds unit tests for the new _is_engine_measured() display rule.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +133 to +137
if not any(_is_engine_measured(m) for m in info.modules):
# Showing the floor readings here would look like an ACTIVE
# channel sitting far below setpoint. Report the state only.
self._channel_lines[key].setText(f"{key:>4} {on_off:<3} {state.name:<14} TEC not engine-controlled")
continue
Comment on lines +52 to +54
Firmware only holds ACTIVE while a channel sits at setpoint: it drops back
to WARMING_UP below -0.5 °C and to CHECK_ERROR at or above the +5 °C error
threshold. A module reporting ACTIVE far outside that band therefore is not
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants