Helios diode current: stop the status poll overwriting the setpoint

The current spin box could not hold a typed value: the 1 Hz status poll
read LDS and wrote it straight into the spin box, so the operator's number
was replaced by the laser's within a second — before Set could be pressed.
The spin box is now seeded once on connect and belongs to the operator
after that; the poll's reading goes to a read-back label beside the Set
button, so the value about to be sent and the value the laser holds are
separate readouts.

The write itself was also unverified. Section 6 of the operator's manual:
"Commands or set values can be discarded by the controller unintentionally.
It is recommended to query the set value after the command is entered to
confirm the actual value." set_current_ma() wrote LDS and returned True
regardless, so a discarded write looked exactly like a good one.
_write_verified() now writes, reads back, and retries up to three times;
set_current_ma() and set_frequency_hz() use it, and HeliosWorker reports a
refusal on the status line instead of echoing the requested value.

Also from the manual, recorded but not acted on: LDS accepts 0-7000 mA
(the driver's 2000 mA ceiling is this rig's, not the protocol's), LDF has
to be re-sent after LDG changes, and the power-monitor mnemonic is HMP —
which this laser does not implement, per the operator.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

KNOWN_ISSUES.md

@@ -25,11 +25,12 @@ anywhere in this repo's protocol notes. `helios_test_app.py` called it
 anyway and raised `AttributeError` into a popup; the button is now disabled
 and the handler reports the gap instead.
 
-**Bench check:** find the power-read command in the Helios manual (the
-other reads are three-letter mnemonics like `LDO`, `LDS`, `LTA`). If one
-exists, add `get_power_mw()` to `hardware/helios_laser.py` using
-`_query_int`, then re-enable the button. If it doesn't, delete the Power
-Monitoring group from the test app and fix the README.
+**Answered (2026-09-08):** the mnemonic is `HMP` (Table 6-3, "Laser Power
+Monitor", read-only, 0-5000 mW) — but the table adds "not available on all
+models", and the operator confirms this rig's laser has no power meter.  So
+the button stays disabled; what remains is to delete the Power Monitoring
+group from `helios_test_app.py` and drop the power-monitoring claims from
+`docs/hardware/HELIOS_DRIVER_README.md`.
 
 ## Genesis laser: forked protocol implementations disagree
 

hardware/helios_laser.py

@@ -210,6 +210,38 @@ class HeliosLaser:
             logger.error(f"Failed to read the reply to '{command}': {e}")
         return True
 
+    # Section 6 of the operator's manual, under Syntax:
+    #
+    #     Commands or set values can be discarded by the controller
+    #     unintentionally.  It is recommended to query the set value after
+    #     the command is entered to confirm the actual value.
+    #
+    # (The command table repeats it: "Query the command to confirm it was
+    # accepted.")  A setter that only writes therefore cannot report whether
+    # it worked, and the panel's next status poll reads back the old value —
+    # which looks exactly like the GUI refusing the operator's number.
+    SET_RETRIES = 3
+    SET_SETTLE_S = 0.05         # let the controller store it before reading
+
+    def _write_verified(self, mnemonic: str, value: int) -> bool:
+        """Write `value` to `mnemonic`, and confirm the controller took it.
+
+        Returns False if the read-back never matches, leaving the controller
+        holding whatever value it kept — the caller is expected to say so
+        rather than let the discarded write pass for a successful one.
+        """
+        for attempt in range(1, self.SET_RETRIES + 1):
+            if not self._write_command(f"{mnemonic} {value}"):
+                return False
+            time.sleep(self.SET_SETTLE_S)
+            readback = self._query_int(mnemonic)
+            if readback == value:
+                return True
+            logger.warning(
+                f"'{mnemonic} {value}' not accepted: controller reports "
+                f"{readback} (attempt {attempt}/{self.SET_RETRIES})")
+        return False
+
     def _query_int(self, command: str) -> Optional[int]:
         """Query a value that should parse as an int; None if absent/unparseable."""
         raw = self._query(command)
@@ -235,20 +267,28 @@ class HeliosLaser:
             logger.error(f"Period {period_ns} ns out of range (8000-60000)")
             return False
 
-        command = f"LDF {period_ns}"
-        return self._write_command(command)
+        return self._write_verified("LDF", period_ns)
 
     def set_current_ma(self, current: int) -> bool:
-        """Set pump diode current in mA."""
+        """Set pump diode pulse current (LDS) in mA.
+
+        False means the controller did not take the value — see
+        _write_verified.  The manual's range for LDS is 0-7000 mA; the
+        2000 mA ceiling here is this rig's limit, not the protocol's.
+        """
         if not (0 <= current <= 2000):
             logger.error(f"Current {current} mA out of range (0-2000)")
             return False
 
-        command = f"LDS {current}"
-        return self._write_command(command)
+        return self._write_verified("LDS", current)
 
     def set_pulse_mode(self, mode: PulseMode) -> bool:
-        """Set pulse mode."""
+        """Set pulse mode.
+
+        Note from the manual's LDG entry: "LDF has to be set again after LDG
+        is changed, except for single pulse triggering" — so a caller that
+        changes the mode has to re-send the frequency.
+        """
         command = f"LDG {mode.value}"
         return self._write_command(command)
 

sc3-aui-helios.ui

@@ -109,6 +109,29 @@
         </property>
        </widget>
       </item>
+      <item>
+       <widget class="QLabel" name="helios_current_readback_label">
+        <property name="toolTip">
+         <string>The diode current the controller reports. The spin box is the value that will be sent when Set is pressed.</string>
+        </property>
+        <property name="text">
+         <string>laser: --- mA</string>
+        </property>
+       </widget>
+      </item>
+      <item>
+       <spacer name="horizontalSpacer_current">
+        <property name="orientation">
+         <enum>Qt::Orientation::Horizontal</enum>
+        </property>
+        <property name="sizeHint" stdset="0">
+         <size>
+          <width>40</width>
+          <height>20</height>
+         </size>
+        </property>
+       </spacer>
+      </item>
      </layout>
     </widget>
    </item>

sc3_aui_app.py

@@ -399,8 +399,19 @@ class HeliosWorker(PollingQueueWorker):
     def _do_set_current(self, current_ma: int):
         if not self._laser:
             return
-        self._laser.set_current_ma(current_ma)
-        self.current_updated.emit(current_ma)
+        if self._laser.set_current_ma(current_ma):
+            self.current_updated.emit(current_ma)
+            return
+        # The manual warns the controller drops set values unintentionally,
+        # so the driver verifies the write.  Say when it did not take: this
+        # used to emit the requested value regardless, and the next poll
+        # then quietly restored the old one.
+        actual = self._laser.get_current_ma()
+        if actual is not None:
+            self.current_updated.emit(actual)
+        self.error_occurred.emit(
+            f"Diode current: controller kept {actual} mA, "
+            f"did not accept {current_ma} mA")
 
     def _poll_once(self):
         """Read the full status set.  Individual reads are allowed to fail
@@ -982,6 +993,7 @@ class HeliosWindow(QWidget):
         self._worker = worker
         # Polling is driven by the worker itself (self-rescheduling), so
         # there is no timer here to outpace the device.
+        self._seed_current_spin = False
         self._wire_signals()
         self._update_controls(False)
         self._set_emission_indicator(False)
@@ -1029,6 +1041,8 @@ class HeliosWindow(QWidget):
             self._worker.queue_disconnect()
 
     def _on_connected(self):
+        # Take the setpoint from the laser once, on connect.
+        self._seed_current_spin = True
         self.helios_connect_toggle.setEnabled(True)
         self.helios_connect_toggle.setText("Disconnect")
         self.helios_status_label.setText("Connected")
@@ -1077,9 +1091,20 @@ class HeliosWindow(QWidget):
         self._worker.queue_set_current(self.helios_current_spin.value())
 
     def _on_current_updated(self, current_ma: int):
-        self.helios_current_spin.blockSignals(True)
-        self.helios_current_spin.setValue(current_ma)
-        self.helios_current_spin.blockSignals(False)
+        """Show what the laser reports; leave the spin box to the operator.
+
+        The 1 Hz poll used to write its reading straight into the spin box,
+        so a number typed there was overwritten within a second — the
+        setpoint appeared to snap back to the controller's value before it
+        could be sent.  The spin box is now seeded once per connection and
+        is the operator's alone after that; the label is the read-back.
+        """
+        self.helios_current_readback_label.setText(f"laser: {current_ma} mA")
+        if self._seed_current_spin:
+            self._seed_current_spin = False
+            self.helios_current_spin.blockSignals(True)
+            self.helios_current_spin.setValue(current_ma)
+            self.helios_current_spin.blockSignals(False)
 
     def _reset_temperatures(self):
         self.helios_temp_diode_label.setText("--- °C")
@@ -1101,6 +1126,8 @@ class HeliosWindow(QWidget):
             self.helios_enable_btn.setText("Enable Laser")
             self.helios_enable_btn.blockSignals(False)
             self._set_emission_indicator(False)
+            self._seed_current_spin = False
+            self.helios_current_readback_label.setText("laser: --- mA")
             self._reset_temperatures()
             self.helios_ler_label.setText("---")
             self.helios_lce_label.setText("---")

tests/test_helios_laser.py

@@ -41,19 +41,33 @@ class FakePort:
     asked for, not by discarding what it happens to find.
     """
 
-    def __init__(self, replies=None, timeout=1.0):
-        self.replies = REPLIES if replies is None else replies
+    def __init__(self, replies=None, timeout=1.0, discard_writes=False):
+        self.replies = dict(REPLIES) if replies is None else replies
         self.timeout = timeout
         self.is_open = True
+        # "Commands or set values can be discarded by the controller
+        # unintentionally" (manual, Section 6) — the case a verified write
+        # exists to catch.
+        self.discard_writes = discard_writes
         self.written: list[str] = []
         self._buf = bytearray()
 
+    def _store(self, mnemonic: str, value: str):
+        """Keep a written value, so a later query reads back what was set."""
+        if self.discard_writes:
+            return
+        previous = self.replies.get(mnemonic, [f"{mnemonic} = 0"])[0]
+        unit = previous.split()[3:]     # "LDS = 1500 mA" -> ["mA"]
+        self.replies[mnemonic] = [" ".join([f"{mnemonic} =", value, *unit])]
+
     # ── the bits of pyserial.Serial the driver uses ──────────────────────────
 
     def write(self, data: bytes) -> int:
         text = data.decode("ascii").strip()
         self.written.append(text)
         fields = text.split()
+        if len(fields) > 1:
+            self._store(fields[0].upper(), fields[1])
         for line in self.replies.get(fields[0].upper() if fields else "", []):
             self._buf += line.encode("utf-8") + b"\r\n"
         return len(data)
@@ -96,6 +110,18 @@ def laser():
     drv.serial = FakePort()
     drv.is_connected = True
     drv.TRAILING_QUIET_S = 0.0
+    drv.SET_SETTLE_S = 0.0
+    return drv
+
+
+@pytest.fixture
+def stubborn_laser():
+    """A controller that answers every query but keeps its own set values."""
+    drv = HeliosLaser(timeout=1.0)
+    drv.serial = FakePort(discard_writes=True)
+    drv.is_connected = True
+    drv.TRAILING_QUIET_S = 0.0
+    drv.SET_SETTLE_S = 0.0
     return drv
 
 
@@ -158,3 +184,54 @@ def test_raw_command_returns_every_line(laser):
     assert laser.send_raw_command("LCE") == (
         "LCE = 2\nBit 15..0: 0000 0000 0000 0010"
     )
+
+
+# ── Set values the controller may discard ────────────────────────────────────
+#
+# Section 6 of the manual: "Commands or set values can be discarded by the
+# controller unintentionally.  It is recommended to query the set value
+# after the command is entered to confirm the actual value."  A write that
+# reports success without reading back leaves the panel showing a setpoint
+# the laser never took, until the next poll replaces it with the old value.
+
+def test_a_set_current_is_read_back(laser):
+    assert laser.set_current_ma(900) is True
+    assert laser.get_current_ma() == 900
+    assert "LDS 900" in laser.serial.written
+
+
+def test_a_discarded_set_current_is_retried_then_reported(stubborn_laser):
+    assert stubborn_laser.set_current_ma(900) is False
+    # Retried, not given up on after one write.
+    assert stubborn_laser.serial.written.count("LDS 900") == \
+        stubborn_laser.SET_RETRIES
+    # And the controller's own value is what it still holds.
+    assert stubborn_laser.get_current_ma() == 1500
+
+
+def test_a_set_that_takes_on_a_retry_succeeds(laser):
+    """One dropped write, then the controller accepts — still a success."""
+    real_write = laser.serial.write
+    state = {"drops": 1}
+
+    def flaky(data: bytes) -> int:
+        if data.decode("ascii").strip().startswith("LDS ") and state["drops"]:
+            state["drops"] -= 1
+            laser.serial.written.append(data.decode("ascii").strip())
+            return len(data)        # swallowed: nothing stored, no reply
+        return real_write(data)
+
+    laser.serial.write = flaky
+    assert laser.set_current_ma(900) is True
+    assert laser.get_current_ma() == 900
+
+
+def test_a_set_frequency_is_read_back(laser):
+    assert laser.set_frequency_hz(25000) is True    # 40000 ns
+    assert laser.get_frequency_hz() == 25000
+    assert "LDF 40000" in laser.serial.written
+
+
+def test_an_out_of_range_current_is_not_sent(laser):
+    assert laser.set_current_ma(9000) is False
+    assert laser.serial.written == []