diff --git a/hardware/helios_laser.py b/hardware/helios_laser.py index 345cd11..f992f3f 100755 --- a/hardware/helios_laser.py +++ b/hardware/helios_laser.py @@ -34,6 +34,7 @@ class HeliosLaser: self.timeout = timeout self.serial = None self.is_connected = False + self._rx = bytearray() # bytes read off the port, not yet a line @staticmethod def list_available_ports() -> List[str]: @@ -52,6 +53,7 @@ class HeliosLaser: try: self.serial = open_8n1(self.port, baudrate=9600, timeout=self.timeout) + self._rx.clear() time.sleep(0.1) # Allow time for connection to stabilize self.is_connected = True logger.info(f"Connected to Helios laser on {self.port}") @@ -108,20 +110,58 @@ class HeliosLaser: TRAILING_QUIET_S = 0.05 # the line counts as idle after this long MAX_REPLY_LINES = 8 + def _discard_input(self): + """Drop anything unread, on the wire and already taken off it.""" + self._rx.clear() + self.serial.reset_input_buffer() + def _read_line(self) -> Optional[str]: - """One CR-terminated line without its framing; None if nothing came.""" - raw = self.serial.read_until(b'\r') - if not raw: - return None - return raw.decode('ascii', errors='replace').strip() + """One line, however it is framed; None if nothing came in time. + + The controller ends every line with CRLF and pads a reply with a + blank line or two: + + b'LDS = 100 mA\r\n\r\n' + + Reading up to CR alone leaves the trailing LF behind, and the next + read then waits out the whole port timeout for a CR that will not + come until some later command is answered. That was a second of + dead air per query — a status poll took ~8.6 s against the 1 s + interval that schedules it — and worse, a query that spends its + deadline blocked gives up while its own reply is still arriving. + The next query then flushes the port mid-line, and the fragment it + reads is a bare number: "LCE = 32" cut after the "=" is where a + diode current of 32 mA came from. + """ + deadline = time.monotonic() + self.timeout + while True: + cut = min((i for i in (self._rx.find(b'\r'), self._rx.find(b'\n')) + if i >= 0), default=-1) + if cut >= 0: + line = bytes(self._rx[:cut]) + # CRLF is one terminator, not an empty line between two. + end = cut + (2 if self._rx[cut:cut + 2] == b'\r\n' else 1) + del self._rx[:end] + return line.decode('ascii', errors='replace').strip() + if time.monotonic() >= deadline: + return None + chunk = self.serial.read(self.serial.in_waiting or 1) + if not chunk: + return None # port timeout: nothing more is coming + self._rx += chunk def _read_pending_lines(self) -> List[str]: """Every further line the controller sends before the line goes quiet.""" lines: List[str] = [] deadline = time.monotonic() + self.TRAILING_QUIET_S while True: - if self.serial.in_waiting: + # What has already arrived is read whatever the quiet window + # says: the window is for deciding when to stop waiting, not + # for leaving a line in the buffer to confuse the next query. + if self._rx or self.serial.in_waiting: line = self._read_line() + if line is None: + return lines # a partial line, nothing behind it if line: lines.append(line) deadline = time.monotonic() + self.TRAILING_QUIET_S @@ -175,7 +215,7 @@ class HeliosLaser: mnemonic = fields[0].upper() if fields else "" try: # Anything volunteered while the port was idle answers no command. - self.serial.reset_input_buffer() + self._discard_input() if not self._send_command(command): return None @@ -395,7 +435,7 @@ class HeliosLaser: logger.error("Not connected to laser") return None try: - self.serial.reset_input_buffer() + self._discard_input() if not self._send_command(command): return None first = self._read_line() diff --git a/tests/test_helios_laser.py b/tests/test_helios_laser.py index 517b828..3eb798f 100644 --- a/tests/test_helios_laser.py +++ b/tests/test_helios_laser.py @@ -12,23 +12,34 @@ import pytest from hardware.helios_laser import HeliosLaser, PulseMode -# What the controller actually sends back, per Section 6 of the operator's -# manual and the LER/LCE/CCE tables in Section 8. +# What the controller actually sends back, transcribed from a session with +# the laser (tools/helios_lds_probe.py): CRLF line ends, the value padded +# out to a fixed width, and one or two blank lines closing every reply. +# +# b'LDS = 100 mA\r\n\r\n' +# b'LCE = 32\r\nBit 15..0: 0000 0000 0010 0000\r\n\r\n\r\n' +# +# The blank lines matter: a reader that stops at CR leaves the LF of the +# last one behind, and the next read waits out the port timeout for a CR +# that only the next command will bring. +_PAD = [""] +_REGISTER_PAD = ["", ""] + REPLIES = { - "LDO": ["LDO = 1"], - "LDF": ["LDF = 20000 ns"], - "LDS": ["LDS = 1500 mA"], - "LDG": ["LDG = 14"], - "LRE": ["LRE = 0"], - "LTA": ["LTA = 25400 m°C"], - "LTT": ["LTT = 31200 m°C"], - "EOA": ["EOA = 40100 m°C"], - "CSR": ["CSR = 1234567"], - "HSR": ["HSR = 7654321"], + "LDO": ["LDO = 1 "] + _PAD, + "LDF": ["LDF = 20000 ns"] + _PAD, + "LDS": ["LDS = 1500 mA"] + _PAD, + "LDG": ["LDG = 14 "] + _PAD, + "LRE": ["LRE = 0 "] + _PAD, + "LTA": ["LTA = 25400 m°C"] + _PAD, + "LTT": ["LTT = 31200 m°C"] + _PAD, + "EOA": ["EOA = 40100 m°C"] + _PAD, + "CSR": ["CSR = 1234567"] + _PAD, + "HSR": ["HSR = 7654321"] + _PAD, # The registers are the multi-line ones. - "LER": ["LER = 0", "Bit 15..0: 0000 0000 0000 0000"], - "LCE": ["LCE = 2", "Bit 15..0: 0000 0000 0000 0010"], - "CCE": ["CCE = 0", "Bit 15..0: 0000 0000 0000 0000"], + "LER": ["LER = 0", "Bit 15..0: 0000 0000 0000 0000"] + _REGISTER_PAD, + "LCE": ["LCE = 2", "Bit 15..0: 0000 0000 0000 0010"] + _REGISTER_PAD, + "CCE": ["CCE = 0", "Bit 15..0: 0000 0000 0000 0000"] + _REGISTER_PAD, } @@ -182,7 +193,7 @@ def test_set_commands_clear_their_acknowledgement(laser): def test_raw_command_returns_every_line(laser): """The diagnostics console is where a multi-line reply should be visible.""" assert laser.send_raw_command("LCE") == ( - "LCE = 2\nBit 15..0: 0000 0000 0000 0010" + "LCE = 2\nBit 15..0: 0000 0000 0000 0010" ) @@ -254,3 +265,63 @@ def test_a_serial_number_still_comes_back_bare(laser): laser.serial.replies = {"CSR": ["A1B2C3D4"], "HSR": ["7654321"]} assert laser.get_controller_serial() == "A1B2C3D4" assert laser.get_head_serial() == "7654321" + + +class SplitReplyPort(FakePort): + """Answers LCE in two pieces, the tail arriving after the next command. + + That is what the wire looks like when a query gives up early: at 9600 + baud the rest of the reply is still coming, and reset_input_buffer() + cannot drop bytes that have not arrived. The fragment left over is + " 32" — the value half of "LCE = 32", which is a plausible + diode current and was read as one. + """ + + def __init__(self): + super().__init__() + self._late = b"" + + def write(self, data: bytes) -> int: + text = data.decode("ascii").strip() + self.written.append(text) + mnemonic = text.split()[0].upper() if text.split() else "" + # Whatever is asked next, the last reply's tail lands in front of it. + self._buf += self._late + self._late = b"" + if mnemonic == "LCE": + self._buf += b"LCE =" # ...and no line ending yet + self._late = b" 32\r\n\r\n\r\n" + return len(data) + for line in self.replies.get(mnemonic, []): + self._buf += line.encode("utf-8") + b"\r\n" + return len(data) + + +def test_a_late_fragment_is_not_the_next_query_s_value(): + """The regression this branch exists for. + + LCE's reply is cut in half, so the register read gives up. The tail + arrives while the *next* query is being answered, and "32" is what the + panel showed as the pump diode current — LCE bit 5, "Door switch open", + read as milliamps. + """ + drv = HeliosLaser(timeout=1.0) + drv.serial = SplitReplyPort() + drv.is_connected = True + drv.TRAILING_QUIET_S = 0.0 + + assert drv._query_int("LCE") is None # cut off mid-reply + assert drv.get_current_ma() == 1500 # not 32 + + +def test_a_reply_is_read_without_waiting_out_the_port(laser): + """Nothing is left in either buffer once a reply has been read. + + A leftover LF costs a whole port timeout on the next read, which is + what made a status poll take ~8.6 s against a 1 s interval. + """ + laser.timeout = 0.01 # a wait would show up as a failure below + assert laser.get_status_registers() == (0, 2, 0) + assert laser.get_current_ma() == 1500 + assert laser.serial.in_waiting == 0 + assert laser._rx == bytearray() diff --git a/tools/helios_lds_probe.py b/tools/helios_lds_probe.py index 8756617..d0a9f34 100644 --- a/tools/helios_lds_probe.py +++ b/tools/helios_lds_probe.py @@ -43,8 +43,9 @@ REGISTERS = {"LER": LER_FLAGS, "LCE": LCE_FLAGS, "CCE": CCE_FLAGS} # Reads that should not change anything, in the order the panel's poll # issues them, plus the two the panel never asks for: LDG (pulse mode, which -# is what decides whether LDS is applied at all) and LMA (an actual-current -# read, per the manual's unit column). +# is what decides whether LDS is applied at all) and LMA — whose unit column +# in the manual says mA, but which this controller answers in m°C, so it is +# a resonator temperature and not a second current reading. READ_SWEEP = ["LER", "LCE", "CCE", "LDO", "LDG", "LDF", "LDS", "LMA", "HTR"] QUIET_S = 0.4 # a reply is over once the line is idle this long