fix(front-ends): ARDOP, KISS and the TUI ignored [audio] device (#1311) - #1314
Merged
Conversation
`ModemEngine::set_default_device` was called at exactly ONE construction site in the workspace — the daemon's main engine. ARDOP, KISS and the TUI built engines and passed a hardcoded `None` for the per-call device, so on any multi-card host they took the OS default card and `[audio] device` was silently ignored. `openpulse-tnc` is the shipped Pat surface; an operator with a USB soundcard interface plus onboard audio got the onboard card, with no diagnostic. The engine resolves `device.or(self.default_device)`, so a front-end may name its device per call OR once at construction. The two correct surfaces used two DIFFERENT mechanisms — the daemon a default, the CLI `--device` threaded per call — nothing documents which a front-end should use, and three of them used neither. Seven copies of the backend-selection match exist across the workspace, so there was no shared place where pinning the device would have been obvious. Fixed with one guarded `set_default_device` each, which is sufficient because the per-call argument falls back to the default. The TUI's config load moved ahead of the engine: it takes its backend from `--backend`, has no device flag, and read the config too late to use it. The recurrence guard is a source scan, not the shared engine builder #1308's review floated. That builder is right in principle — `ptt_builder` (#1258) is the in-repo precedent for the same shape — but it needs cpal feature forwarding across five crates, which is too wide to carry inside this fix. Deduping the seven matches stays separable. The scan is validated in three directions plus a real one: planted violation flagged, planted fix accepted, out-of-window mention rejected, and deleting the pin from openpulse-kiss itself fails it. Not covered, with the reason written into the scan's source: the CLI (correct by the other mechanism), openpulse-mesh (real-audio capability removed), and the daemon's two repeater engines (#1308, blocked on a ruling about which device they should use at all). Implements: REQ-DEV-01 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1311.
What was wrong
ModemEngine::set_default_devicewas called at exactly one construction site in the workspace — the daemon's main engine. ARDOP, KISS and the TUI built engines and passed a hardcodedNonefor the per-call device, so on any multi-card host they took the OS default card and[audio] devicewas silently ignored.openpulse-tncis the shipped Pat surface. An operator with a USB soundcard interface plus onboard audio — the ordinary amateur configuration — got the onboard card, with no diagnostic.Why it happened six times
The engine resolves a device as
device.or(self.default_device), so a front-end may name it per call or once at construction. The two correct surfaces used two different mechanisms:set_default_device--deviceinto every callNothing documents which a front-end should use, and three of them used neither. There are also seven copies of the backend-selection
matchacross the workspace, so there was no shared place where pinning the device would have been the obvious next line.Because the per-call argument falls back to the default, one guarded line each is sufficient — no call sites needed touching.
The TUI needed one extra change: it takes its backend from
--backend, has no device flag at all, and loaded its config after building the engine. The config load moved ahead of it.Census, corrected twice
My issue filing said "four front-ends", which was a claim about a filter that had only looked at the sites #1308's review named. Reading every site:
set_default_deviceopenpulse-cli--deviceper callopenpulse-ardopopenpulse-kissopenpulse-tuiopenpulse-meshno_real_audio.rskeeps it outWhy a source scan and not the shared builder
#1308's review floated a shared engine builder, and
ptt_builder(#1258) is the in-repo precedent for exactly this shape — N front-ends each open-coding a backend match. It is right in principle.It also needs
cpalfeature forwarding across five crates, which is too wide to carry inside a fix for a silent-wrong-card bug. Deduping the seven matches stays separable and available.What actually prevents recurrence is the scan — the instrument this repo already uses for keying (
ptt_keys_every_transmit). Its exclusions are written into its own source with a reason each, so the next front-end gets added to it rather than quietly omitted.Validation, in four directions
A scan that matches nothing passes exactly like a scan over compliant code, and this repo has been bitten by that before — a pattern copied between crates matched nothing because the calls were multi-line chains, and its planted control still passed. So:
set_default_deviceplaced outside the 12-line window → rejectedopenpulse-kissitself fails with "openpulse-kiss builds a ModemEngine at line(s) [82] without calling set_default_device"Consumer
openpulse-tnc(ARDOP),openpulse-kisstnc,openpulse-tui— the three shipped binaries whosemainbuilds an engine and whose call sites all passNone.Prior art
server.rs:111is the one correctset_default_device, the line this copies.ptt_builder(#1258) is the shared-builder precedent, deliberately not followed here for scope.ptt_keys_every_transmitis the source-scan precedent, which is followed.Twins
The census above is the twins check, and it is complete rather than sampled: every
ModemEngine::new(in the workspace was read, not grepped. Two sites are correct by different mechanisms, three are fixed, one is moot, two are #1308.Implements: REQ-DEV-01
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6