Helios: read CRLF replies as lines, not as CR plus dead air
The rig transcript (tools/helios_lds_probe.py) settles where the panel's
32 mA came from, and it was never the laser: LDS reads 100 mA, answers for
itself, and takes a write of 900 mA on the first attempt. 32 is LCE — bit
5, "Door switch open" — arriving in the diode-current field.
Every reply is CRLF-terminated and padded with a blank line or two:
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'
_read_line() read up to CR, so the final LF of every reply stayed in the
buffer, and the next read waited out the whole port timeout for a CR that
only the next command would bring. A second of dead air per query: replayed
against the transcript's byte timing, one status poll took 8.6 s against
the 1 s interval that schedules it. That is also what let the values drift
apart — a query whose deadline goes to a blocked read gives up while its
own reply is still on the wire, the next query flushes the port mid-line,
and the fragment it reads is " 32", the value half of LCE's reply.
Lines are now framed on CR, LF or CRLF out of a receive buffer that
_discard_input() clears along with the port, so nothing survives a flush
half-read. The same replay now polls in 0.59 s.
Tests carry the transcript's real framing (padded values, trailing blank
lines) instead of the tidied "LER = 0" it was guessed to be, plus the two
regressions: a late fragment must not become the next query's value, and a
reply must be readable without waiting out the port.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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:
|
||||
"""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
|
||||
return raw.decode('ascii', errors='replace').strip()
|
||||
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()
|
||||
|
||||
+86
-15
@@ -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,
|
||||
}
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user