feat(cw): independent RX/TX CW decode toggles (#2417). Principle V.#2638
Conversation
Splits the legacy single "CwDecodeOverlay" decode flag into two independent toggles — RX (received signal, existing behaviour) and TX (operator's own keying, fed from the client-side sidetone) — wrapped in a nested AppSettings["CwDecoder"] JSON blob so the feature owns one root key rather than two new flat ones. Legacy flat key is migrated on first read so existing users keep their RX-on default. TX path taps the CwSidetoneGenerator's per-block mono signal via a SampleTap callback set once at AudioEngine ctor. AudioEngine downsamples 48→24 kHz, duplicates to stereo float32 (matching the CwDecoder feedAudio shape), and emits txDecodeAudioReady when the runtime gate is on. MainWindow flips that gate from moxChanged and toggle changes so audio only flows during transmit. A dedicated CwDecoder instance handles TX so decoded text lands in PanadapterApplet::appendCwTextTx and renders with a "[TX]" prefix and distinct cyan color, distinguishable from received CW in the same panel. RX/TX toggles in RadioSetupDialog replace the single "Decode" button. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Nice work — this is a clean implementation. The refreshCwDecodeState() chokepoint is the right call: collapsing every CW-decode panel/run/tap decision behind one helper removes the cluster of duplicated isCw && decodeOn branches that used to drift out of sync across buildMenuBar, buildUI, onSliceAdded, and setActiveSliceInternal. The CwDecodeSettings helper with nested-JSON storage matches the constitution Principle V convention, and the legacy-key migration preserves existing users' RX setting.
A few observations:
Audio thread / lambda lifetime (looks fine, just confirming): AudioEngine::AudioEngine calls m_cwSidetone->setSampleTap([this]...) from the constructor body, after m_cwSidetone is initialized in the init list. m_cwSidetone is owned by AudioEngine (unique_ptr), so the captured this cannot dangle. CwSidetoneGenerator.h:60 even warns "not safe to swap while audio is running" — and it isn't swapped, so good.
TX decoder run-state vs. close button: routeCwDecoderOutput() now wires cwPanelCloseRequested to stop both m_cwDecoder and m_cwDecoderTx. With the new logic in refreshCwDecodeState(), the TX decoder will be restarted on the next MOX edge / slice mode change / dialog close because the gating only looks at txOn, not at any "user dismissed the panel" flag. Same transient-stop behavior the RX decoder had before, but worth noting if testers report the close button feeling "sticky" — clicking close while TX-decode is enabled will silently come back. If you want close to mean "off until the operator re-enables in the dialog", flipping CwDecodeSettings::setRxEnabled(false) / setTxEnabled(false) on cwPanelCloseRequested would make the action durable.
MOX edge race (minor, won't cause real issues): m_cwDecodeTxTapEnabled is memory_order_relaxed. On a MOX-on edge, the audio thread may run one block of process() between moxChanged firing and the tap actually flipping on. Result is at most one block (~10 ms) of dropped tap audio at the front of a TX. Given the decoder timeline already has the inter-character silence-mirror to absorb gaps, this is fine — flagging it only so a reviewer doesn't trip on it later.
Format mismatch in CwDecoder::feedAudio docstring (pre-existing, out of scope): CwDecoder.h says // Feed 24kHz stereo int16 PCM but the implementation in CwDecoder.cpp actually reads float32 stereo. The new TX path correctly produces float32 stereo to match the real format. No PR action needed — but worth a docstring fix in a follow-up since the PR comments in AudioEngine.h correctly describe the actual contract.
Conventions check: AppSettings (not QSettings), RAII via unique_ptr and QVarLengthArray, atomic gating, AutoConnection across threads via the receiver-object form of connect() — all aligned with house style. Files touched all map to scope.
Thanks @aethersdr-agent — LGTM as a COMMENT.
Follow-up fixes after live-testing the TX-side CW decoder feature. - **CwxLocalKeyer word gap**: produced 4u of silence at every CWX word boundary instead of the standard 7u — `firstChar = true` after a space suppressed the next letter's CharGap. 4u fell inside ggmorse's 3u inter-character classification window, erasing word boundaries in the TX decode panel and also diverging from standard CW timing for the local sidetone listener. Remove the firstChar reset so the next letter contributes its 3u CharGap, giving WordGap(4u) + CharGap(3u) = 7u total. - **TX-tap MOX gate**: AudioEngine's CW-decode-TX tap was AND'd with `isMox() || isTransmitting()`, but CLAUDE.md is explicit that the radio never sends `mox=` in status, and CW keying goes through netcw rather than the conventional MOX interlock. The result was a tap that never fired on actual CW transmissions. Gate solely on the `CwDecodeSettings::txEnabled()` toggle; the sidetone generator already self-gates on key state so silence buffers between bursts cost nothing. - **CwDecoder::setKnownParameters**: new API on CwDecoder that locks both pitch and speed to externally-supplied values (with widened pitch range) and pushes ggmorse params live. Used by MainWindow to feed the TX decoder the operator's known cw_pitch / cw_speed from the P/CW applet, so ggmorse classifies gap durations against the correct unit length and word boundaries become " " separators. Hook TransmitModel::phoneStateChanged into refreshCwDecodeState so the values track slider movement live. - **Renderer cleanup**: drop the per-emit "[TX] " prefix that produced unreadable "[TX] E [TX] M [TX] T …" output. Cyan color alone is the visual signal; a single space separator is inserted on Rx↔Tx transitions so the two colored runs don't merge. Tested live on FLEX-8600 with CWX-driven keying: `CQ CQ CQ DE KK7GWY K` decodes correctly with proper word spacing on the panel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ten9876
left a comment
There was a problem hiding this comment.
Approved. Live-tested on FLEX-8600 — CQ CQ CQ DE KK7GWY K decodes with correct word spacing in cyan. Original feature + our hardware-tested follow-up cleanly resolves all four defects that surfaced during testing (CwxLocalKeyer word gap, MOX gate, P/CW-locked decoder params, renderer prefix).
Summary
Fixes #2417
What was changed
feat(cw): independent RX/TX CW decode toggles (#2417). Principle V.
Files modified
src/core/AudioEngine.cppsrc/core/AudioEngine.hsrc/core/CwSidetoneGenerator.cppsrc/core/CwSidetoneGenerator.hsrc/gui/CwDecodeSettings.hsrc/gui/MainWindow.cppsrc/gui/MainWindow.hsrc/gui/PanadapterApplet.cppsrc/gui/PanadapterApplet.hsrc/gui/RadioSetupDialog.cppGenerated by AetherClaude (automated agent for AetherSDR)