Skip to content

[bug] Route CW sidetone to selected audio output#2899

Open
jensenpat wants to merge 2 commits into
aethersdr:mainfrom
jensenpat:aether/cw-sidetone-audio-device
Open

[bug] Route CW sidetone to selected audio output#2899
jensenpat wants to merge 2 commits into
aethersdr:mainfrom
jensenpat:aether/cw-sidetone-audio-device

Conversation

@jensenpat
Copy link
Copy Markdown
Collaborator

@jensenpat jensenpat commented May 21, 2026

Summary

Fix CW sidetone routing so the local sidetone sink follows AetherSDR's selected PC audio output instead of silently drifting to whatever PortAudio considers the default output.

Root Cause

The Qt sidetone fallback already accepted a QAudioDevice, but the PortAudio sidetone backend ignored the selected Qt device and always opened PortAudio's default output. That meant users who selected a non-default PC audio output in preferences, or who accepted a new output device from the audio device dialog, could still have CW sidetone routed to the wrong physical endpoint.

There was a second related default-following gap: when AetherSDR was configured to follow the system default output, the audio monitor only restarted local audio paths when device IDs were added/removed. If the OS changed the default output without removing the previous device, an existing RX/sidetone sink could stay bound to the old endpoint.

Changes

  • Map selected Qt output device names to PortAudio output devices for the CW sidetone backend.
  • Treat missing or ambiguous PortAudio mappings as a backend failure so AudioEngine falls back to the Qt QAudioSink sidetone path on the selected device.
  • Log selected-device, partial-match, ambiguity, and fallback routing decisions with qCWarning so support bundles capture the sidetone route without refactoring global logging behavior.
  • Keep the existing low-latency PortAudio default/JACK behavior when AetherSDR is following the system default and PortAudio can match that default output.
  • Pass the current Qt output device object into the sidetone backend startup path instead of letting PortAudio choose independently.
  • Track Qt default input/output IDs in MainWindow and restart default-following local audio paths when the OS default changes, even when no device was removed.
  • Document the default-device restart behavior and sidetone backend routing/fallback behavior in docs/audio-pipeline.md.

User Impact

Switching PC audio output from app settings or from the new audio-device dialog now restarts CW sidetone on the same output device as RX audio. Users following the OS default output also get a restart when the OS changes that default, keeping sidetone, RX audio, and the title-bar PC audio labels aligned.

Validation

Local macOS validation:

  • cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo
  • cmake --build build --parallel 22
  • cmake --build build --target AetherSDR --parallel 22
  • ./build/cw_sidetone_test
  • git diff --check

Built app: build/AetherSDR.app

Windows validation by @nigelfenton:

  • Environment: Windows 11, Flex 6700, paddle on the radio's HWCW key jack.
  • PC audio endpoints present: TOSHIBA-TV (NVIDIA HDMI), Headphones (Realtek(R) Audio), USB Audio, Oculus Virtual, and FlexRadio DAX cables.
  • Built clean with MSVC, filling the skipped check-windows CI gap; no errors or new warnings on the four source files this PR touches.
  • Explicit-output bug-fix test:
    • AetherSDR PC audio output set to Headphones (Realtek(R) Audio).
    • Windows default playback set to TOSHIBA-TV (NVIDIA HDMI).
    • Local CW sidetone via the CWX panel stayed on the Realtek headphones; the TV stayed silent.
  • Hotplug test:
    • Plugging Realtek headphones in mid-session opened the Audio Device Detected dialog.
    • Accepting the dialog moved both RX audio and CW sidetone to the new device cleanly.
  • Explicit selection vs OS default test:
    • While AetherSDR was explicitly set to Realtek, changing the Windows default back to TOSHIBA-TV did not restart or reroute AetherSDR audio.
    • CW sidetone correctly stayed on Realtek.
  • Not exercised on Nigel's machine: partial-match and ambiguity branches of findPortAudioOutputDevice; his Realtek mapping was unambiguous.

👨🏼‍💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat

Co-authored-by: Codex <noreply@openai.com>
@jensenpat jensenpat changed the title Route CW sidetone to selected audio output [bug] Route CW sidetone to selected audio output May 21, 2026
@jensenpat jensenpat marked this pull request as ready for review May 21, 2026 00:59
@jensenpat jensenpat added the priority: high High priority label May 21, 2026
Copy link
Copy Markdown
Contributor

@aethersdr-agent aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jensenpat — well-scoped fix, and the docs update is a nice touch.

A few observations, none blocking:

CwSidetonePortAudioSink::findPortAudioOutputDevice — partial-match false positives

The fallback candidate.contains(target) || target.contains(candidate) widens the search beyond exact matches, with the ambiguity guard catching multi-hits. That's a good safety net, but a single accidental substring match still slips through silently. For example, a Qt description of "Speakers" would partial-match a PortAudio device named "Speakers (Conf Room)" if no other candidate exists — and the user would never see why sidetone went somewhere unexpected.

Two cheap mitigations worth considering:

  • Log the partial-match path at info/warning level (something like "Qt 'X' partially matched PortAudio 'Y'"), so a misroute is at least discoverable in the support bundle.
  • Or tighten the partial match to only candidate.contains(target) (PortAudio name strictly contains Qt description, which is the common case for "Pulse vs ALSA hw:N" style names) and skip the reverse direction — the reverse direction is the one most likely to false-match.

AudioEngine.cpp line 1049 — dev = d vs dev = m_outputDevice

Good catch on using the freshly-enumerated d. Worth a one-line comment noting why — otherwise this looks like a no-op refactor and may get "fixed" back in a future cleanup pass.

MainWindow.cpp default-ID tracking — defaultInputChanged evaluation order

Logic looks right. One small thing: currentDefaultInputId/currentDefaultOutputId are read unconditionally, but defaultInputChanged/defaultOutputChanged are only consulted in the currentInput.isNull() / currentOutput.isNull() branches. Not a bug, just an observation — the two QMediaDevices::default* calls happen on every device-list-change tick now.

Scope / conventions

  • All five touched files are in scope for the stated fix.
  • No QSettings usage introduced; RAII boundaries unchanged.
  • Logging consistently uses lcAudio via qCWarning/qCInfo.

LGTM modulo the partial-match observability concern. The QAudioSink fallback when PortAudio mapping fails is the right call.

@nigelfenton
Copy link
Copy Markdown
Contributor

Windows validation (Win11, Flex 6700, paddle on radio's HWCW key jack, several PC audio endpoints: TOSHIBA-TV via NVIDIA HDMI, Realtek motherboard audio, USB Audio, Oculus Virtual, plus the usual FlexRadio DAX cables).

Built clean with MSVC — fills the SKIPPED check-windows CI gap. No errors or new warnings on any of the 4 source files this PR touches.

Distinctive bug-fix test:

AetherSDR PC audio output (explicit) Headphones (Realtek(R) Audio)
Windows default playback TOSHIBA-TV (NVIDIA HDMI)
Local CW sidetone via CWX panel Stayed on Realtek headphones; TV silent

That's the exact scenario this PR fixes — pre-PR, PortAudio's default-output would have routed sidetone to the TV regardless of AetherSDR's selection. Post-PR, the Qt → PortAudio mapping correctly wins.

Audio-device hotplug: plugging Realtek headphones in mid-session fired the "Audio Device Detected" dialog. Accepting moved both RX and sidetone to the new device cleanly:

DBG aether.audio: AudioEngine: output device set to "Headphones (Realtek(R) Audio)"
WRN aether.audio: AudioEngine: RX stream started at 48000 Hz device: "Headphones (Realtek(R) Audio)"

Default change while explicit selection set: flipped Windows default back to TOSHIBA-TV while AetherSDR was explicit on Realtek. No restart fired (correct — explicit selection should win), sidetone stayed on Realtek.

Not exercised: the partial-match / ambiguity branches of findPortAudioOutputDevice — my Realtek mapping was unambiguous on this machine. Would need a device with substring-overlapping PortAudio names to exercise that path.

Re: @aethersdr-agent's observability concern — it's worse than the source reads. CwSidetonePortAudioSink::start logs the success/mapping line at qCInfo(lcAudio), but LogManager::applyFilterRules (src/core/LogManager.cpp:118) only adds <category>.debug=true for enabled categories — .info stays off. So the success/mapping diagnostic is invisible in support bundles even with the audio log category turned on. A silent partial-match misroute would have no forensic trail at all.

Two small (non-blocking) follow-up ideas:

  • Log the partial-match path at qCWarning (or a distinctly-worded qCInfo saying "partial-match: X → Y") so substring matches always surface
  • Or extend LogManager::applyFilterRules to also emit <category>.info=true for enabled categories — broader observability win beyond this PR

LGTM on the user-visible fix.

@aethersdr-agent
Copy link
Copy Markdown
Contributor

Confirmed @nigelfenton's diagnosis after reading the code on main — and it's slightly worse than your summary suggests, in a way that strengthens your "extend applyFilterRules" recommendation over the per-call-site qCWarning rewrite.

Why qCInfo is invisible — two gates, not one

lcAudio is declared at src/core/LogManager.cpp:20:

Q_LOGGING_CATEGORY(lcAudio, "aether.audio", QtWarningMsg)

That sets the category's compiled-in minimum level to QtWarningMsg, which short-circuits qCInfo(lcAudio) before Qt's filter rules are even consulted. Even if applyFilterRules did emit aether.audio.info=true, info messages would still need that minimum level overridden via the filter rule — which setFilterRules() does in fact do, so the extension is the right lever. But note that 18 of 22 categories in that file are declared at QtWarningMsg (lines 20–39), so this isn't a sidetone-specific problem — it's a project-wide observability gap. Any support bundle today is missing every qCInfo line from those categories.

That makes the second follow-up the clear win: one-line fix, surfaces every existing qCInfo call (including the JACK selection at CwSidetonePortAudioSink.cpp:65 and the 48k-fallback at line 99, both currently silent in production).

Suggested patch — src/core/LogManager.cpp:113

void LogManager::applyFilterRules()
{
    QStringList rules;
    rules << "aether.*.debug=false";
    rules << "aether.*.info=false";
    for (const auto& c : m_categories) {
        if (c.enabled) {
            rules << QString("%1.debug=true").arg(c.id);
            rules << QString("%1.info=true").arg(c.id);
        }
    }
    QLoggingCategory::setFilterRules(rules.join('\n'));
}

The explicit aether.*.info=false baseline matters: without it, the four categories declared at QtDebugMsg (lcDiscovery/lcConnection/lcProtocol, lines 17–19) would start emitting info unconditionally regardless of the UI toggle.

Separately — the partial-match forensic gap

Even after the logging fix above, a silent partial-match misroute is the scarier failure mode because the user has no symptom until they key up. I'd still log the partial-match branch of findPortAudioOutputDevice at qCWarning (not qCInfo) — it's a "we made a choice you might not expect" event, not a routine status line, and warnings ride for free without any category toggle. Concretely something like:

qCWarning(lcAudio) << "CwSidetonePortAudioSink: Qt device" << qtDeviceName
                   << "matched PortAudio device" << paDeviceName
                   << "by substring — verify routing if sidetone is misrouted";

That gives support bundles the forensic trail nigelfenton called out, independent of any per-user log-category preferences.

Re: process — this issue isn't currently carrying the aetherclaude-eligible label (only priority: high). A maintainer can authorize the orchestrator to land either the logging extension, the partial-match warning, or both as a follow-up PR by adding the aetherclaude-eligible label. The user-visible sidetone routing fix that nigelfenton LGTM'd is independent of these observability items.

73, Jeremy KK7GWY & Claude (AI dev partner)

Co-authored-by: Codex <noreply@openai.com>
@jensenpat jensenpat linked an issue May 21, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: high High priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Track CW sidetone routing to selected PC audio output

2 participants