KissTncServer: move ownership from dialog to MainWindow (architectural follow-up to #3279)

[ten9876]

Summary

Tracking the architectural follow-up to PR #3279 (merged via admin-merge with my fix-up commit c3ef6e3b). @NF0T's original Blocker 2 in his 2026-06-04 review was that KissTncServer lifetime is owned by Ax25HfPacketDecodeDialog, and the dialog should become a config/status view only with ownership moved to MainWindow. I took the minimum-viable fix in the merged PR (don't set WA_DeleteOnClose on this dialog so closing the window doesn't destroy the server) — transparently noted in the fix-up commit and coordination comment — but the deeper ownership refactor is still worth doing for the reasons @NF0T originally raised.

Why the deeper refactor still matters

The minimum-viable fix solves the user-visible bug (close → kill server) but leaves these architectural rough edges:

1. TX queue + counters + the modem TX path live in the dialog. A KISS frame coming in over TCP requires the dialog to exist to be transmitted on RF. With Start-on-Startup the dialog is constructed at launch, but the conceptual coupling between "TCP server lifecycle" and "modem TX path lifecycle" is inverted from what makes architectural sense — TX is a transport concern, not a UI concern.

2. Slice attachment plumbing is implicit. It works today because MainWindow::setActiveSlice happens to call setAttachedSlice on the dialog (MainWindow.cpp:11701), and that fires on the -1 → first-slice transition too. But this contract isn't documented and a future refactor of setActiveSlice could silently break it.

3. The dialog becoming hidden-but-alive to keep the server running is awkward. It's a config/status view that has to stay constructed for an unrelated transport reason.

Proposed direction

Per @NF0T's original prescription, paraphrased here for the tracking issue:

• Move `KissTncServer`, `m_kissTxQueue`, `m_kissRxCount`/`m_kissTxCount`, and the TX-from-KISS handler to `MainWindow`.
• `MainWindow` reads `TncSettings::startOnStartup()` and constructs/starts the server independently of the dialog.
• The dialog becomes a config/status view: it reads/writes `TncSettings` for its UI, calls `MainWindow::setKissTncEnabled(bool)` / `setKissTncPort(int)`, and subscribes to a `MainWindow::kissTncStatusChanged` signal for the status line.
• Slice add/remove wired at `MainWindow` level so the server (or whichever owns the modem TX path) gets slices directly.
• The trickiest piece is the modem TX path itself (currently `m_shim`, `m_txPaceTimer`, etc. in the dialog). Two reasonable approaches:
  • (a) Extract the modem TX state machine into a separate model owned by `MainWindow`. Dialog observes it for UI display.
  • (b) Keep modem TX in the dialog as a UI concern (matches current intent — the dialog also handles direct UI TX) but have `MainWindow` forward KISS-originated frames to it. Drop frames silently if dialog isn't constructed (with a log warning).

(a) is cleaner long-term, (b) is closer to the existing structure. @jensenpat's call on the right shape.

Out-of-scope until this is decided

PR #3290 (PMS / PBBS) is stacked on the KISS TNC code from #3279 and was waiting on that PR to clear. It can now rebase against current main and become reviewable independently of this architectural follow-up — the minimum-viable fix in #3279 is sufficient for the PMS PR's needs.

Tagging

@jensenpat — you're the author and have the deepest context. No urgency on this — the user-visible bug is fixed in main, and this is the kind of refactor that benefits from sitting with the design for a few days before committing. Feel free to bounce ideas in the issue or open a draft PR when you're ready.

Principle IX (Surface Only What Survives).

[aethersdr-agent]

Triaged. Verified the claims against current main (post-#3279 merge 01a4d17a + fixup c3ef6e3b) and pulled together a handoff for the design decision.

State of play in current main

KISS-server ownership lives in the dialog, exactly as described:

• m_kissServer constructed as a child of the dialog at src/gui/Ax25HfPacketDecodeDialog.cpp:591.
• TX queue + counters declared at src/gui/Ax25HfPacketDecodeDialog.h:183-190 (m_kissTxQueue, m_kissTxBusyRetries, m_kissTxCount, m_kissRxCount).
• KISS-side handlers at src/gui/Ax25HfPacketDecodeDialog.cpp:1813-1897 (handleKissFrameFromClient → maybeStartNextKissTx → m_shim->buildTransmitAudioFromFrame → beginTransmission(tx, /*fromKiss=*/true)).
• Destructor stops the server at src/gui/Ax25HfPacketDecodeDialog.cpp:874-875.

The minimum-viable fix is in place:

• MainWindow::showAx25HfPacketDecodeDialog at src/gui/MainWindow.cpp:6668 explicitly bypasses showOrRaisePersistent() to avoid WA_DeleteOnClose, with a comment naming the exact reason ("closing the window would destroy the dialog and along with it the KISS TCP server").
• MainWindow::startKissTncOnStartupIfConfigured() at src/gui/MainWindow.cpp:6693 constructs the dialog hidden at launch when TncSettings::startOnStartup() is true.

A couple of additions to NF0T's analysis

1. The implicit slice plumbing has two sites, not one. The issue cites MainWindow.cpp:11701 (stale — the file has grown since the issue was filed; the actual setActiveSlice → setAttachedSlice call is now at src/gui/MainWindow.cpp:12299-12300). But there is a second site at src/gui/MainWindow.cpp:11973-11974 for the "last slice removed → detach" case. So a future refactor of setActiveSlice is not the only way to silently break the contract — anyone touching the slice-removal path could too. Strengthens the case for making the wiring explicit at the MainWindow level.

2. KissTncServer itself is already cleanly encapsulated (src/core/tnc/KissTncServer.h). Its public surface is start/stop/isListening/port/clientCount/lastError/framesTo|FromClients/setMaxClients plus signals ax25FrameFromClient, listeningChanged, clientCountChanged, activity, kissParameterReceived and slot broadcastAx25Frame. No dialog dependencies — moving its ownership is a near-mechanical change. The complication is entirely on the TX side.

Weighing (a) vs (b)

A grep for the TX-path symbols (beginTransmission|paceTransmitAudio|finishTransmit|beginTransmitWhenReady|m_txActive|m_txPendingStream|m_txPcm|m_txOffsetBytes|m_txChunkIndex|m_txRestoreAudioDaxMode|m_txPaceTimer|m_txPaceClock) returns 79 occurrences in Ax25HfPacketDecodeDialog.cpp alone. The state machine is intertwined with DAX-stream readiness (txAudioStreamReady), PTT-block handling, audio-DAX-mode save/restore, pace-clock health metrics, and UI bookkeeping (appendTransmitLine, refreshTransmitControls).

• (a) Extract a TX engine model owned by MainWindow — cleaner long-term but the dialog also TXes for direct UI input, so the model needs a clean two-source interface (UI text vs. KISS frame) and the dialog has to become an observer for status. This is a meaningful refactor, not a rename pass.
• (b) Keep modem TX in the dialog, have MainWindow forward KISS frames to it — much smaller; the dialog still has to exist for KISS TX to work, but MainWindow owns the server/queue/counters and just calls a single dialog->transmitKissFrame(ax25NoFcs) slot (drop with warning if dialog is null). Solves the architectural-rough-edge list for the server but leaves the TX path coupling intact.

On balance: (b) gets you most of the value (server lifecycle truly independent, slice wiring explicit, no hidden-but-alive contract for the server) for ~a tenth of the churn, and it doesn't preclude (a) later. (a) is the right end state if the dialog is ever to become a true config/status view (PR #3290 stacking, future TNC clients) but doesn't need to land in one shot.

Triage classification

• State: maintainer-review — design decision is yours to make per the issue body; not eligible for automated implementation until you pick (a) or (b).
• Kind: enhancement (architectural refactor; user-visible bug is already fixed in main).
• Subsystem: protocol, GUI.
• Priority: unset (issue explicitly says no urgency; feat(modem): Personal Mailbox System (PMS) over connected-mode AX.25 #3290 unblocked).

Happy to draft a PR for whichever direction you choose once you tag aetherclaude-eligible.

---

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

---

_{📍 Citation health check (Foundry Constitution Principle I): 8 resolved, 1 unresolved against /Users/aetherclaude/workspace/AetherSDR at origin/main.}

Unresolved citations (1)

Citation	Reason
MainWindow.cpp:11701	file not found in workspace