feat(modem): KISS-over-TCP TNC + AetherModem UX overhaul

[jensenpat]

Summary

Adds a KISS-over-TCP TNC to AetherModem so any host packet/APRS app (Xastir,
YAAC, APRSdroid, UISS, Dire Wolf clients, terminal programs) can send and receive
AX.25 through AetherModem's AFSK modem — plus the requested AetherModem UX cleanup.

KISS TNC server

• src/core/tnc/KissFraming.{h,cpp} — pure KISS framing (FEND/FESC escaping,
  resync-safe incremental decoder, data-frame encode). No Qt::Network dependency,
  so it is unit-tested standalone.
• src/core/tnc/KissTncServer.{h,cpp} — cross-platform QTcpServer:
  • multiple simultaneous clients, each with its own KISS decoder;
  • proper timeouts: TCP keepalive + slow-consumer write-backlog cap + idle
    sweep + max-client cap, so dead/stuck clients are reaped, not leaked;
  • RX → all clients (KISS data frames); client → ax25FrameFromClient signal.
• Shim support:
  • RX captures the exact on-air AX.25 bytes (no FCS) into
    Ax25DecodedFrame::ax25FrameNoFcs, so decodes forward byte-faithfully;
  • buildTransmitAudioFromFrame() keys a raw KISS frame (computes/appends FCS),
    refactored to share AFSK rendering with the text TX path.
• Dialog: functional AX.25 / KISS TNC tabs (QStackedWidget); Enable
  TNC, Start TNC on Startup, TCP port (default 8001) controls, all
  persisted; a serialized KISS TX queue feeding the existing PTT/DAX/pacing path;
  a TNC STATUS panel (port, client count, RX/TX counters). Enabling the TNC
  also enables the modem. MainWindow::startKissTncOnStartupIfConfigured()
  constructs the window hidden + persistent at launch when Start-on-Startup is set
  (see the follow-up note above re: the manual-enable case).

AetherModem UX overhaul (per request)

• Window renamed AetherModem - Packet Decoder (Experimental) → AetherModem.
• Removed the Experimental banner entirely.
• Removed the Tone Polarity radios; polarity is always Normal (the correct
  sense for the supported HF DIGU / VHF FM paths).
• Packet Activity: taller bars + boosted levels so 1–2 packets/sec are clearly
  visible instead of sitting on the floor; PACKET ACTIVITY label moved above
  the graphic to line up with the MODEM STATUS / GAIN STAGE panels.

Logging

All KISS lifecycle/per-frame activity logs on the aether.ax25 (AetherModem)
category prefixed KISS (listen/stop, client connect/disconnect/refuse/reap, RX
broadcast, TX from client, parse/param) so issues triage as client-side vs RF-side.

Tests

ax25_libmodem_shim_test gains:

• KISS framing round-trip (incl. FEND/FESC escaping and byte-split reassembly);
• KISS TX-from-frame → AFSK → RX loopback, also asserting the captured RX raw
  frame matches the keyed frame.

All tests pass; full app builds clean on macOS against current main.

Test plan

• KISS TNC tab → Enable TNC (8001) → connect Xastir / YAAC / APRSdroid / a
  Dire Wolf KISS client at host:8001; confirm connect logged on aether.ax25
• Decode a 2m APRS frame → appears in the host app (RX → client)
• Send from the host app → keyed on the air at the selected baud (client → TX)
• Multiple clients simultaneously; kill one rudely → reaped, others fine
• Toggle Start on Startup, relaunch → TNC listening without opening the window
• AX.25 RX/TX unchanged; window title, removed banner/polarity, taller activity
  bars, label alignment all as expected
• ax25_libmodem_shim_test passes

🤖 Generated with Claude Code

[aethersdr-agent]

Thanks @jensenpat — this is a clean piece of work, particularly the way pure KISS framing is split out from KissTncServer so it gets standalone unit-test coverage. A few notes below; nothing here is a blocker.

Looks good

• KissTncServer defensive posture — TCP keepalive + LowDelayOption, kMaxWriteBacklogBytes slow-consumer cap, idle sweep, max-clients gate, and kMaxFrameBytes resync recovery in KissFraming.cpp:73 is a thorough set of failure-mode guards.
• KissFraming purity — no Qt::Network dependency, so testKissFramingRoundTrip exercises it directly and the byte-split feed test (tests/ax25_libmodem_shim_test.cpp:631) covers the real TCP fragmentation case.
• Ax25DecodedFrame::ax25FrameNoFcs — capturing the on-air bytes minus FCS in the shim and round-tripping them through the loopback test is a nice correctness guarantee.
• TX-path refactor — initTxResult / renderBitsToResult / logTxSummary extraction in AetherAx25LibmodemShim.cpp lets buildTransmitAudioFromFrame share AFSK rendering with the text path without duplication. Good factoring.
• Settings use AppSettings and persist correctly; no QSettings smuggled in.

Worth considering before merge

1. No cap on m_kissTxQueue size (Ax25HfPacketDecodeDialog.cpp, handleKissFrameFromClient). The RX side has a slow-consumer write-backlog cap, but a fast/misbehaving client can enqueue TX frames faster than RF can drain them — particularly while the radio is busy and maybeStartNextKissTx is on its 250 ms retry. Consider a queue depth cap (drop oldest with a qCWarning, similar to kMaxWriteBacklogBytes) so a runaway client can't grow the queue without bound.

2. m_radio->isRadioTransmitting() busy-retry has no backoff or cap (maybeStartNextKissTx, 250 ms singleShot). If the radio is genuinely stuck transmitting, the queue spins indefinitely. Pairs naturally with the cap above — once the queue cap drops frames, the loop self-bounds.

3. Manual-enable + close path — already acknowledged in the PR description ("planned fix: move KissTncServer ownership to MainWindow"). Just confirming the issue: showOrRaisePersistent sets WA_DeleteOnClose (MainWindow.h:867), so a user who manually toggles Enable TNC and then closes the window will lose the server. The Start-on-Startup path correctly bypasses this by constructing without WA_DeleteOnClose in startKissTncOnStartupIfConfigured. Worth gating draft → ready on this since it's the obvious user-visible footgun.

4. docs/MODEM.md typo — MODEM.md:200 reads "computes/->appends" (looks like a stray ->). Should probably be "computes/appends".

5. Minor: refreshTncStatus calls m_tncStatusDot->setFixedSize(12, 12) on every refresh — could move that to buildKissTncPage so refresh only updates the styleSheet.

6. Minor: when m_kissServer->start(port) fails inside setTncEnabled, the kTncEnabledSetting was already persisted as "True" earlier in the function. If persist was true and start fails, the user's persisted setting will say enabled but nothing is listening on next launch (Start-on-Startup will then re-try and may succeed if the port conflict has cleared — so this might actually be the desired behavior, just noting it).

Test coverage looks proportionate to the change. Suggested follow-ups (not blockers): a test for kMaxFrameBytes overflow resync, and a malformed-after-FEND case.

---

_{🤖 aethersdr-agent · cost: $11.4602 · model: claude-opus-4-7}

[aethersdr-agent]

Thanks @jensenpat — nice piece of work. The split between KissFraming (pure, unit‑tested) and KissTncServer (Qt::Network) is clean, the dialog refactor into a QStackedWidget is tidy, and the TX path was nicely DRY'd via initTxResult / renderBitsToResult / logTxSummary. AppSettings is used throughout, RAII is respected, and the test coverage of KISS escape/un-escape with byte-split feeds plus the TX-from-frame loopback (with raw-bytes round-trip) is exactly the right shape.

A few things worth thinking about:

1. Headless dialog never receives slice updates (likely related to your planned follow-up)

startKissTncOnStartupIfConfigured() (MainWindow.cpp:6433) calls activeSlice() at app launch, which is almost always nullptr — the radio hasn't connected yet. There's no later code path that invokes setAttachedSlice() on the hidden dialog when slices arrive (the menu path showAx25HfPacketDecodeDialog() is the only caller, and only runs if the user manually opens the window). So a Start‑on‑Startup user who never opens the window will have the TNC listening on TCP but no slice attached — RX won't capture and TX queues will sit unable to key.

When you move KissTncServer ownership to MainWindow per the follow-up, consider wiring it directly to the slice add/remove signals there rather than going through the dialog. That sidesteps both this and the WA_DeleteOnClose problem.

2. kTncEnabledSetting is written but never read

Ax25HfPacketDecodeDialog.cpp:48 defines kTncEnabledSetting and setTncEnabled(..., persist=true) writes it, but nothing ever reads it back — only kTncStartOnStartupSetting is used to seed the initial state. If session-only is the intent, dropping the setValue/save in setTncEnabled would avoid misleading future readers (and a stray write per toggle).

3. Port-change restart can fail silently to the user

Ax25HfPacketDecodeDialog.cpp port valueChanged does setTncEnabled(false, false); setTncEnabled(true, false). If the new port fails to bind, the checkbox gets force-unchecked via QSignalBlocker and only the status panel reflects "Stopped". A KissTncServer::start() already emits activity() with the bind error which will reach the log — but worth confirming that path actually fires for an EADDRINUSE on the rebind so the user gets a clear breadcrumb rather than just an unchecked box.

4. Minor: m_kissTxQueue.clear() on !m_audio || !m_radio in maybeStartNextKissTx drops the entire backlog without a log line. handleKissFrameFromClient guards the same condition with an appendSystemLine — would be nice to mirror that here for symmetry.

None of these are merge blockers and the WA_DeleteOnClose one you're already on; the slice-update gap is the only behaviour issue I'd want to see settled (likely by the same ownership-move follow-up).

---

_{🤖 aethersdr-agent · cost: $13.9258 · model: claude-opus-4-7}

[K5PTB]

@jensenpat I pulled this feature down into my local repository and built it for testing. Claude wrote a test for me to compare the text from the KISS port to the MQTT port that I have implemented, but not yet PR'ed. They match, over 500+ packets, confirming they at least agree.

My MQTT topic also publishes the confidence level for the decode. I've made similar changes locally to graywolf and direwolf, so they provide that info and I can compare the average values. They are very similar, and even when one or two don't decode, the values are still very similar.

In one of your earlier PR's, I commented that I had written a ax25 comparator script to match decodes from three different sources, on the same audio stream. It was clear the libmodem in AetherSDR was falling behind.

I asked Claude to implement the same API as libmodem, but use the algorithms in Direwolf. There's a compile time option to build with one or the other. The new algorithm performs very similar to Direwolf. Over a couple of hour of capture, AetherSDR decoded 1303 packets, graywolf 1256, and Direwolf 1290.

I'm holding both features for your feedback, as I don't want to muddy the waters.

Do you plan to put iGate functionality into AetherSDR?

[NF0T]

Thanks @jensenpat — the fundamentals here are solid. KissFraming splitting out pure framing for standalone unit testing is the right architecture, KissTncServer's defensive posture (keepalive, write-backlog cap, idle sweep, max-clients gate, kMaxFrameBytes resync) is thorough, and the TX-path refactor into initTxResult/renderBitsToResult/buildTransmitAudioFromFrame eliminates duplication cleanly. The ax25FrameNoFcs loopback round-trip is a strong correctness guarantee.

Three blockers need to be addressed before this is ready to merge.

---

Blocker 1 — Constitution Principle V: flat settings keys

The PR introduces three new flat AppSettings keys (AetherModemKissTncEnabled, AetherModemKissTncStartOnStartup, AetherModemKissTncPort). Principle V requires new features to store configuration as a single nested JSON blob under one root key, not a stack of flat keys. Suggested shape:

// read
auto cfg = QJsonDocument::fromJson(
    AppSettings::instance().value("AetherModemKissTnc", "{}").toByteArray()).object();
bool enabled        = cfg["enabled"].toBool(false);
bool startOnStartup = cfg["startOnStartup"].toBool(false);
int  port           = cfg["port"].toInt(8001);

// write (atomic, single key)
QJsonObject cfg;
cfg["enabled"]        = enabled;
cfg["startOnStartup"] = startOnStartup;
cfg["port"]           = port;
AppSettings::instance().setValue("AetherModemKissTnc",
    QString(QJsonDocument(cfg).toJson(QJsonDocument::Compact)));
AppSettings::instance().save();

This also resolves the dead-code issue flagged in the prior review: kTncEnabledSetting is written but never read back, because the nested write forces you to load and write the whole blob atomically.

---

Blocker 2 — KissTncServer owned by the dialog; Start-on-Startup is functionally broken

Two related issues with the same root cause:

A — dialog lifetime: showOrRaisePersistent() sets WA_DeleteOnClose. A user who manually enables the TNC and then closes the window silently destroys the TCP server and drops all connected clients. The dialog close and the TNC server lifecycle are not the same thing.

B — headless slice attachment: startKissTncOnStartupIfConfigured() calls activeSlice() at MainWindow construction — almost always nullptr since the radio hasn't connected yet. No subsequent code path calls setAttachedSlice() on the hidden dialog when slices later arrive. The TNC listens on TCP but has no slice attached, so RX never captures audio and TX queues can never key.

Both issues share the same fix: move KissTncServer ownership to MainWindow (not the dialog). The dialog becomes a config/status view only. Wire slice add/remove directly at the MainWindow level. The prior bot review already recommended this path; the PR description acknowledges a "follow-up" but the Start-on-Startup behavior is broken as submitted, not deferred-but-working.

---

Blocker 3 — unbounded TX queue

m_kissTxQueue has no depth cap. A misbehaving KISS client can push TX frames faster than RF can drain them (250 ms retry while the radio is busy; no backoff, no cap). The RX side already has kMaxWriteBacklogBytes for slow consumers — the TX queue needs the same discipline. Suggest a constexpr int kMaxKissTxQueueDepth (e.g. 64 frames), drop-oldest on overflow with a qCWarning, and a maximum retry count or exponential backoff in maybeStartNextKissTx so a stuck-transmitting radio doesn't spin indefinitely.

---

Should-fix (not blockers, but worth addressing in the same pass)

4. Port-change failure not surfaced to user. When KissTncServer::start(newPort) fails on a port change, the checkbox silently unchecks. Emit an activity() line from the error path so the user gets a log entry (EADDRINUSE is common on port changes).

5. docs/MODEM.md typo. Line 46 of the new section reads "The modem computes/->appends the FCS" — the -> is a stray artifact. Should be "computes/appends".

6. refreshTncStatus layout churn. m_tncStatusDot->setFixedSize(12, 12) is called on every status refresh. Move to buildKissTncPage().

7. maybeStartNextKissTx silent backlog drop. m_kissTxQueue.clear() when !m_audio || !m_radio has no log line. handleKissFrameFromClient has an appendSystemLine for the same condition — should mirror that for consistency.

---

CODEOWNERS note

This PR touches CMakeLists.txt, docs/, and tests/ (Tier 2 — @aethersdr/infrastructure). Since you're the author, a second Tier 2 approver (@ten9876) is required before merge even after Tier 3 review clears.

[ten9876]

@jensenpat / @NF0T — pushed a fix-up commit (c3ef6e3) addressing the three blockers from @NF0T's 2026-06-04 review. Same maintainer-fix-up pattern as #3286/#3335/#3350/#3391. Happy to revert any of it if either of you would rather take a swing — I'm doing this to unblock @jensenpat's stacked PMS work in #3290, not to claim the design.

Blocker 1 — Constitution Principle V (nested JSON) ✅

New `TncSettings` class in `Ax25HfPacketDecodeDialog.h` mirrors `CwDecodeSettings`. All persistence now lives as a nested JSON blob under `AetherModemKissTnc`. `TncSettings::migrateLegacy()` is called once from `startKissTncOnStartupIfConfigured()`; legacy flat keys left in place for now (harmless after migration; cleanup can drop them later). Call sites switched to `TncSettings::enabled()` / `::startOnStartup()` / `::port()` accessors.

Blocker 2 — KissTncServer lifetime ✅ (minimum-viable fix, not the architectural move)

I took the minimum viable fix here rather than the ownership-move-to-MainWindow @NF0T originally proposed. The user-visible bug — close the window, lose the server — is addressed by not setting `WA_DeleteOnClose` on this dialog:

```cpp
// showAx25HfPacketDecodeDialog() now constructs directly instead of
// going through showOrRaisePersistent (which sets WA_DeleteOnClose).
```

The dialog now persists past close like the start-on-startup path already did. Slice plumbing was already wired at `MainWindow.cpp:11701` (`setActiveSlice(s)` calls `setAttachedSlice(s)` on -1 → first-slice transitions too), so the "headless dialog never gets a slice" symptom is also handled by existing code as long as the dialog persists.

I think this is honest about what's fixed and what isn't. If @NF0T still wants the full ownership refactor as a follow-up for cleanliness, I'm happy to file it as a tracking issue — but it isn't required to fix the bugs as described.

Blocker 3 — Unbounded TX queue ✅

Two caps, both with operator-visible system-line messages + `qCWarning(lcAx25)` log entries:

• `kMaxKissTxQueueDepth` = 64 frames — drop-oldest on overflow. Symmetric with `KissTncServer::kMaxWriteBacklogBytes` on RX. Newer data > stale backlog.
• `kMaxKissTxBusyRetries` = 60 × 250 ms = 15 s — abandon the head-of-queue frame after this many consecutive radio-busy retries, then try the next one. Prevents a stuck-PTT radio from permanently jamming every subsequent frame behind it. Counter resets on each successful dequeue.

---

CI will rerun against the new commit. @NF0T — happy for you to re-review whenever; I've left your CHANGES_REQUESTED in place (it's pinned to the older commit) so the gate stays meaningful until you've actually re-looked.

Once this clears, #3290 (PMS) can rebase cleanly against the updated head and become reviewable.