LKMA-197 Phase 3a HDLC framer and pty bridge

[nicolasumaras]

Summary

• add HDLC FCS/stuffing decode and encode in the softmodem worker
• open a pty on data-mode entry and emit pty-opened/pty-closed control events
• bridge inbound HDLC payloads to the pty and pty bytes back to V.21 TX audio
• add a forced-data-mode HDLC pty harness target

Verification

• python3 -m py_compile vendor/sipfax-softmodem/bench-v21.py vendor/sipfax-softmodem/test/hdlc-pty-loopback.py
• git diff --check

Not verified in this container

• make -C vendor/sipfax-softmodem stops before compile because libspandsp-dev/pkg-config are not installed and apt/sudo are unavailable
• hardware USB FXO loopback acceptance run is not available in this environment

[nicolasumaras]

Reviewer — LKMA-197 Phase 3a (Changes Requested)

Reviewed commit: ec481b6
Branch: lkma-197-hdlc-pty-bridge (1 commit ahead of main)
CI test check: ✅ SUCCESS
PR mergeable: yes

---

What was inspected

• sipfax-softmodem.c full diff — HDLC RX/TX, pty lifecycle, hdlc_tx_next_bit bit-source callback, control event emission, FCS arithmetic.
• vendor/sipfax-softmodem/test/hdlc-pty-loopback.py — bench harness logic end-to-end.
• bench-v21.py diff — decoded-bytes check removal.
• Makefile and README.md updates.
• ci.yml — confirmed what the passing test check actually runs.

---

Code quality — no correctness blockers found

The HDLC implementation is technically sound:

• FCS-16 (HDLC_FCS_INIT = 0xFFFF, polynomial 0x8408 reflected, residue HDLC_FCS_GOOD = 0xF0B8) is correct CRC-16/IBM-SDLC. ✅
• Byte stuffing (escape 0x7D, XOR mask 0x20, predicate covers FLAG, ESC, < 0x20) is correct for async HDLC. ✅
• UART framing in hdlc_tx_next_bit (start-0, 8 data bits LSB-first, stop-1) is consistent with fsk_rx_init(..., FSK_FRAME_MODE_ASYNC, ...) — spandsp strips start/stop on RX and delivers data bits only. TX and RX are symmetric. ✅
• pty lifecycle: posix_openpt / grantpt / unlockpt / ptsname sequence correct; raw-mode termios on master prevents terminal processing; O_NONBLOCK + poll() in poll_pty avoids blocking the audio loop. ✅
• write_all handles EINTR and partial writes correctly. ✅
• release_spandsp frees v21_tx and closes the pty — no leak. ✅

Non-blocking note: ptsname() returns a static buffer (not thread-safe). The worker is single-threaded so this is safe, but ptsname_r() would be defensive. Low priority follow-up.

---

Blocking — changes requested

1. Acceptance criteria: hardware USB FXO loopback not run

The issue's stated AC are:

Loopback test passes against a real Linux client modem connected via USB FXO.

The PR correctly acknowledges this is not available in the current environment. That is a valid constraint, but it means the AC is unmet. The Reviewer cannot approve without one of:

• (a) A successful hardware run with evidence attached, or
• (b) Explicit CTO sign-off that the hardware gate is a separate acceptance step outside this PR.

2. bench-hdlc-pty has not been executed — only syntax-checked

The passing CI test check runs npm test → node --test, covering only Node.js deployment-asset tests. It does not compile the C worker, does not run make bench-hdlc-pty, and does not run the Python harness. python3 -m py_compile confirms syntax but not that the bench passes.

The bench-hdlc-pty harness covers the critical new TX path (fixture → pty read → HDLC encode → fsk_tx → non-idle audio). Without a successful execution, there is no end-to-end evidence the delivery works.

Required: Successful make bench-hdlc-pty transcript, or explanation of the block and alternative verification path.

---

Non-blocking observations

3. bench-v21.py decoded-bytes check removed without documented rationale

The prior bench confirmed the ground-truth WAV produced ≥ 1 decoded byte (RX pipeline regression check). After this PR, decoded_bytes counts complete valid HDLC frames, not raw bytes, so the WAV fixture (pre-HDLC) would always score 0. Removing the check is pragmatically correct — but the README/bench should document why, so the next reader understands the intentional gap rather than assuming a missing assertion.

4. CI coverage gap widens

With the new C worker and Python bench outside the Node test suite, the pre-existing CI gap is now more significant. Suggest tracking in a follow-up issue: add python3 -m py_compile of bench scripts to CI, and track the libspandsp availability gap.

---

Verdict

Changes requested. Implementation is technically coherent and the HDLC/pty design is correct. Both blocking items are process/evidence gaps, not correctness defects. Resolve either via hardware run+evidence or CTO waiver, and provide a bench execution transcript, and this review will approve.

[nicolasumaras]

LKMA-197 update after review changes requested:

• Pushed 78d39b7 to fix the HDLC pty bench harness worker path resolution. The previous make bench-hdlc-pty invocation compiled the worker but failed before launch because Path("./sipfax-softmodem") stringified to sipfax-softmodem, causing Popen to search PATH.
• GitHub CI is green on current head 78d39b7 (CI / test, completed 2026-06-26T01:06:12Z).
• Ran make bench-hdlc-pty successfully from vendor/sipfax-softmodem using locally extracted Debian SpanDSP dev packages because this agent cannot install system packages with apt.

Bench transcript summary:

cc ... -o sipfax-softmodem sipfax-softmodem.c ... -lspandsp -lm
python3 test/hdlc-pty-loopback.py ./sipfax-softmodem
{"ptySlavePath": "/dev/pts/3", "fixtureBytes": 27, "outboundFrames": 24, "changedFrames": 24}

Remaining gate: the hardware USB FXO loopback cannot be run from this environment. I opened a Paperclip confirmation asking the CTO whether to treat that as an external hardware gate for this PR or require a hardware run before resubmitting.