diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index 87e6489..420466d 100644 --- a/KNOWN_ISSUES.md +++ b/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 diff --git a/hardware/helios_laser.py b/hardware/helios_laser.py index 7dad247..d54d41c 100755 --- a/hardware/helios_laser.py +++ b/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) diff --git a/sc3-aui-helios.ui b/sc3-aui-helios.ui index 34cd8a1..7450264 100755 --- a/sc3-aui-helios.ui +++ b/sc3-aui-helios.ui @@ -109,6 +109,29 @@ + + + + The diode current the controller reports. The spin box is the value that will be sent when Set is pressed. + + + laser: --- mA + + + + + + + Qt::Orientation::Horizontal + + + + 40 + 20 + + + + diff --git a/sc3_aui_app.py b/sc3_aui_app.py index 2a811c9..a510adc 100755 --- a/sc3_aui_app.py +++ b/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("---") diff --git a/tests/test_helios_laser.py b/tests/test_helios_laser.py index 1c036b9..b471636 100644 --- a/tests/test_helios_laser.py +++ b/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 == []