test(sa818-sim): unit tests + raw-pty hardening + standalone runner by peterus · Pull Request #61 · OE5XRX/FW-RemoteStation

peterus · 2026-07-21T15:19:50Z

Why

While bringing up the D5 web control chain end-to-end against the qemux86-64 sim station, set frequency intermittently returned driver_error: native_sim's SA818 driver read the wrong AT response (a set-group read got RSSI=120, an RSSI read got +DMOSETGROUP:0) — a persistent 1-offset desync of the AT request/response stream that appeared once the agent's telemetry poll interleaved RSSI? with AT+DMOSETGROUP.

Root cause (found by live-debugging the station): stray/duplicate simulator instances on the same pty (a manual-launch hazard) each answered every command → duplicate responses → the driver's reads drifted one behind. The emulator's own framing was never at fault — but nothing pinned that, and nothing prevented the duplicate-instance hazard.

What this PR does

Pure-python unit tests — tests/sim_shell/pytest/test_sa818_simulator_unit.py, runnable under plain pytest (no twister/native_sim) by driving SA818Simulator over a local pty pair. Pins the AT contract (DMOSETGROUP/RSSI?/VOLUME/SETFILTER/DMOCONNECT/unknown→ERROR) and the request/response alignment under back-to-back and sustained interleaved RSSI?/set-group streams — the exact live condition. 11 tests, all green.
Raw-pty hardening — SA818Simulator.start() now forces tty.setraw() on attach. A real SA818 hangs off a raw UART; if the pty is handed over cooked, line-discipline echo / canonical mode would reframe the byte stream and desync AT pairing. A regression test drives a deliberately cooked pty and asserts the simulator self-raws (red before, green after). (Note: on the live station the pty was already -echo -icanon, so this closes the echo seed defensively rather than being the specific live fix.)
Standalone entrypoint — python3 sa818_simulator.py <pty> [--rssi N] with SIGTERM/SIGINT clean shutdown. This is what the linux-image sim-harness invokes to attach exactly one emulator under systemd lifecycle ownership (companion PR in linux-image), structurally preventing the duplicate-instance hazard that caused the desync.

Testing

python3 -m pytest test_sa818_simulator_unit.py --noconftest → 11 passed (contract 6, framing alignment on raw pty 4, self-raw on cooked pty 1). Sustained 25× interleaved RSSI?/set-group stay 1:1 aligned.
--help/argparse smoke-verified; py_compile clean.
The class API is unchanged, so the existing tests/sim_shell twister integration tests are unaffected — CI runs those (needs the west/Zephyr toolchain, not available in this dev loop).

Companion

linux-image: sim-harness starts a single systemd-managed SA818 emulator on native_sim's uart_1 (lifecycle coupled to native_sim) — the durable fix for the duplicate-instance hazard.

🤖 Generated with Claude Code

Copilot

Pull request overview

Adds a standalone SA818 AT-command simulator runner and introduces pure-Python unit tests that validate AT command contract + request/response framing over a local pty, including hardening to force raw TTY mode on attach (to prevent echo/canonical-mode desync).

Changes:

Add new pytest-only unit tests that drive SA818Simulator over a pty pair and pin 1:1 command/response alignment under interleaved streams.
Harden SA818Simulator.start() by forcing raw mode on the attached pty fd to avoid line discipline echo/translation corrupting the AT stream.
Add a standalone main() entrypoint to run the simulator as a long-lived process with signal-based shutdown.

Reviewed changes

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

File	Description
tests/sim_shell/pytest/test_sa818_simulator_unit.py	New pure-Python unit tests validating SA818 simulator command contract and pty framing/alignment (including cooked-pty raw-mode hardening coverage).
tests/sim_shell/pytest/sa818_simulator.py	Force raw tty mode on attach; add a standalone CLI entrypoint for single-instance lifecycle-managed simulator execution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    def read_line(self, timeout: float = 1.0) -> str:
+        """Read one CRLF-terminated response line from the master."""
+        deadline = time.time() + timeout
+        buf = b""
+        while time.time() < deadline:
+            r, _, _ = select.select([self.master_fd], [], [], max(0, deadline - time.time()))
+            if not r:
+                continue
+            buf += os.read(self.master_fd, 256)
+            if b"\n" in buf:
+                line, _, _ = buf.partition(b"\n")
+                return line.strip(b"\r").decode(errors="replace")
+        raise AssertionError(f"no response line within {timeout}s (got {buf!r})")


+    def drain(self, settle: float = 0.15) -> bytes:
+        """Return any bytes still pending on the master after a settle delay —
+        used to assert there are NO stray/late/duplicate responses."""
+        time.sleep(settle)
+        out = b""
+        while True:
+            r, _, _ = select.select([self.master_fd], [], [], 0)
+            if not r:
+                break
+            chunk = os.read(self.master_fd, 256)
+            if not chunk:
+                break
+            out += chunk
+        return out


+import os
+import select
+import termios
+import time
+import tty


+
+import pytest
+
+from sa818_simulator import SA818Simulator, SA818State


Copilot

Pull request overview

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

Comments suppressed due to low confidence (3)

tests/sim_shell/pytest/test_sa818_simulator_unit.py:30

Unused imports: termios and SA818State are imported but never used in this test module, which adds noise and can fail under stricter linting.

import os
import select
import termios
import time
import tty

import pytest

from sa818_simulator import SA818Simulator, SA818State

tests/sim_shell/pytest/test_sa818_simulator_unit.py:129

read_line() discards any bytes after the first \n (e.g., a duplicate second response line arriving in the same read). That can make the tests miss the very “duplicate response” hazard they aim to detect, because the extra line is consumed but not asserted on.

            buf += os.read(self.master_fd, 256)
            if b"\n" in buf:
                line, _, _ = buf.partition(b"\n")
                return line.strip(b"\r").decode(errors="replace")

tests/sim_shell/pytest/test_sa818_simulator_unit.py:140

drain() only reads from the fd; if read_line() has buffered extra bytes (or if it previously read more than one line), those bytes won’t be reported, so “no stray bytes” assertions can give false confidence. Include any buffered remainder in the drained output.

        time.sleep(settle)
        out = b""
        while True:
            r, _, _ = select.select([self.master_fd], [], [], 0)
            if not r:
                break

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

tests/sim_shell/pytest/test_sa818_simulator_unit.py:26

Unused import: termios is imported but never referenced in this test file, which can trip linters and adds noise.

import os
import select
import termios
import time
import tty

tests/sim_shell/pytest/test_sa818_simulator_unit.py:129

read_line() drops any bytes after the first \n in the buffer. If the simulator ever emits multiple lines quickly (the failure mode these tests are meant to catch), this can silently discard the extra response and let the test pass incorrectly.

            if b"\n" in buf:
                line, _, _ = buf.partition(b"\n")
                return line.strip(b"\r").decode(errors="replace")

+    done = threading.Event()
+    signal.signal(signal.SIGTERM, lambda *_: done.set())
+    signal.signal(signal.SIGINT, lambda *_: done.set())
+    try:
+        done.wait()
+    finally:
+        sim.stop()
+    return 0


Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

tests/sim_shell/pytest/test_sa818_simulator_unit.py:129

read_line() reads up to 256 bytes and then returns the first line, discarding any bytes after the first \n. If the simulator ever emits multiple response lines quickly (or duplicates), this test helper could consume and drop the extra bytes, and drain() would not see them—potentially hiding the exact misalignment/duplicate-response bug this suite is meant to catch. Reading one byte at a time until \n avoids over-reading and ensures any extra response bytes remain pending for drain() / subsequent reads.

            buf += os.read(self.master_fd, 256)
            if b"\n" in buf:
                line, _, _ = buf.partition(b"\n")
                return line.strip(b"\r").decode(errors="replace")

tests/sim_shell/pytest/test_sa818_simulator_unit.py:24

Unused import: termios isn't referenced anywhere in this test module (raw-mode control is done via tty.setraw). Keeping it increases noise and can trip linting in some environments.

import termios

tests/sim_shell/pytest/test_sa818_simulator_unit.py:30

Unused import: SA818State is imported but never used in this file.

from sa818_simulator import SA818Simulator, SA818State

…raw pty Adds tests/sim_shell/pytest/test_sa818_simulator_unit.py — runs under plain pytest (no twister/native_sim) by driving SA818Simulator over a local pty pair. Pins the AT contract (DMOSETGROUP/RSSI?/VOLUME/FILTER/DMOCONNECT/unknown) and, crucially, the request/response ALIGNMENT under back-to-back and sustained interleaved RSSI?/set-group streams — the condition the live telemetry poll creates and under which the production sim desynced. Finding: the emulator's framing is correct on a RAW pty; a cooked (echoing) pty desyncs it. Harden SA818Simulator.start() to force tty.setraw() on attach so line-discipline echo / canonical mode can never corrupt the AT stream — a real SA818 hangs off a raw UART. Regression test drives a deliberately cooked pty and asserts the simulator self-raws (red before the fix, green after). Note: closes the echo seed; the production 1-offset seed (late attach vs native_sim timing) is verified separately on the live station. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…tor.py <pty>) Adds a main() + __main__ so the emulator can be launched directly against a pty, with SIGTERM/SIGINT clean shutdown. This is what the linux-image sim-harness invokes to attach exactly ONE SA818 emulator to native_sim's SA818 UART under systemd lifecycle ownership — preventing the stray/duplicate emulator instances that desync the AT request/response stream. Class behaviour is unchanged (the pytest fixture still imports SA818Simulator); --help/argparse smoke-verified, unit tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- _run_loop: on pty EOF (os.read -> b'') STOP the reader instead of continue; a closed fd stays perpetually readable so continue pegs the CPU (Copilot). Regression test test_reader_stops_on_pty_eof asserts the thread stops. - release.yml: co-publish tests/sim_shell/pytest/sa818_simulator.py as a cosign-signed, SHA256SUMS'd release asset (${name}.sa818-sim.py) alongside each native_sim binary — the emulator speaks the AT protocol this firmware's driver expects, so it must ship co-versioned from the SAME release. The linux-image sim-harness fetches it pinned instead of vendoring a copy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

tests/sim_shell/pytest/test_sa818_simulator_unit.py:26

Import block has unused imports (termios, SA818State). This can cause lint/CI failures and makes the test file noisier than needed.

import os
import select
import termios
import time
import tty

tests/sim_shell/pytest/test_sa818_simulator_unit.py:122

Bridge.read_line() discards any bytes after the first \n because it uses a local buffer and returns immediately. If the simulator ever emits duplicate/extra lines (the exact regression this suite tries to catch) and they arrive in the same read, those extra bytes get dropped and drain() may incorrectly see an empty pipe, letting the bug slip through.

    def read_line(self, timeout: float = 1.0) -> str:
        """Read one CRLF-terminated response line from the master."""
        deadline = time.time() + timeout
        buf = b""
        while time.time() < deadline:

tests/sim_shell/pytest/test_sa818_simulator_unit.py:136

Bridge.drain() currently only reads from the fd; if read_line() already buffered extra bytes (e.g., multiple lines received in a single read), drain() should include and clear that buffer too so the "no stray bytes" assertions remain accurate.

    def drain(self, settle: float = 0.15) -> bytes:
        """Return any bytes still pending on the master after a settle delay —
        used to assert there are NO stray/late/duplicate responses."""
        time.sleep(settle)
        out = b""

+        try:
+            tty.setraw(self.master_fd)
+        except termios.error:
+            pass  # not a tty (e.g. a plain pipe in a degenerate test) — nothing to do


Copilot AI review requested due to automatic review settings July 21, 2026 15:19

Copilot started reviewing on behalf of peterus July 21, 2026 15:20 View session