fix(daemon): the QSY scan's per-candidate receive was inert and opened a second capture stream (#1312) - #1320
Merged
Merged
Conversation
…d a second capture stream (#1312) `execute_qsy_actions` called `engine.receive(mode, None)` per scanned candidate while the daemon's own `rx_stream` was still held — two concurrent captures on one device, which #1007 established must never happen. The receive was also inert, which is the larger finding. `last_rx_snr_db()` is written by `record_rx_snr`, and since #1142 that runs only after magic/CRC/ sequence validate. A dwell on an empty candidate never decodes a frame, so every candidate was scored with the same stale number from the HOME frequency — exactly what the no-rig fallback branch does openly. On cpal it could not even try: the stream opens AFTER the dwell and `read` returns ~10 ms. Anything it did decode was discarded rather than reaching `process_received_bytes`. So it is deleted, not fixed. That removes the double-open at every entry point, needs no counter, and does not wait on #1308. The scan now openly measures nothing; making it real needs a stream it may read on the candidate frequency, which is #1308's ownership question and not something a second `open_input` substitutes for. My proposed fix — a capture-open counter extending the post-command drop — was wrong three ways: a post-hoc drop cannot close an overlap that exists during the scan (which is why the OTA drop is placed before the transmit and keyed on the variant); the condition would have fired on zero existing paths, since two of the three callers run on the rx tick arm and the third transmits first; and the EBUSY consequence is inconsequential, because the scan scores the same stale number either way. Gated at the BACKEND with the counting-backend idiom #1007's own test established, rather than on an instrument this change would have added. Sabotage yields exactly 3 opens, one per candidate, which is what proves the backend is counting. Refactors: CAP-45 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 #1312, re-scoped: the real defect is that the scan's per-candidate
receivedid nothing, and the fix is to delete it.What was wrong
execute_qsy_actionscalledengine.receive(mode, None)per scanned candidate while the daemon's ownrx_streamwas still held — two concurrent captures on one device, which #1007 established must never happen.The receive was inert, which is the larger finding
last_rx_snr_db()is written byrecord_rx_snr, and since #1142 that runs only after magic/CRC/sequence validate. A dwell on an empty candidate never decodes a frame, so the value never moved: every candidate was scored with the same stale number from the home frequency — precisely what the no-rig fallback branch does openly, a few lines below.On cpal it could not even try: the stream is opened after the
sleep(dwell), andreadreturns only what arrived in ~10 ms. And anything it did decode was discarded (Ok(_) => {}) rather than reachingprocess_received_bytes.So it is deleted, not fixed. That removes the double-open at every entry point — including the two that run on the rx tick arm, where no drop exists at all — needs no counter, and does not wait on #1308.
The scan now openly measures nothing and says so where the call used to be. Making it real needs a stream the scan may read on the candidate frequency, which is #1308's ownership question; a second
open_inputwas never a substitute for it.My proposed fix was wrong three ways
I had proposed adding a
capture_opens()counter and extending the post-commandrx_streamdrop. Review killed it::865runs afterapply_command_to_enginereturns; the two streams coexist for the whole scan. The OTA drop at:826is placed before the transmit and keyed on the variant precisely because a pre-drop cannot be effect-keyed. I transplanted "counter, not variant" to the one place it cannot work — fix(daemon): never hold two capture streams open on one device #1007's ownCountingBackendgate would still have readopen_peak == 2.execute_qsy_actionshas three callers: two run on the rx tick arm, which a command-arm snapshot cannot see, and the third transmits the REQ before scanning, so the existing drop already fires. My design's Consumer field had looked at the drop site instead of at the callers.Also corrected: the stale-audio consequence I claimed is real but conditional and smaller than the OTA analogy implies (~0.5 s per candidate, under the 30 s burst cap, and dependent on the DCD verdict), and it is a property of blocking the select loop rather than of the second stream — so this deletion does not address it.
Test
the_qsy_scan_opens_no_capture_stream_of_its_ownasserts zero opens at the backend, using the counting-backend idiom #1007's own test established — not a new engine counter, which is what I had proposed and would have meant asserting on an instrument this change was adding.Sabotage-verified: reinstating the per-candidate receive fails it with exactly 3 opens, one per candidate. That predicted count is what proves the backend is counting rather than the assertion passing vacuously.
It needs
flavor = "multi_thread"— the scan's rig calls useblock_in_place, which panics on the current-thread runtime the other tests in that module use. That is the #1264 constraint showing up in practice.GATE: PASS 4a033e82dd14c2f6ab5db517936f811d25792e55 clean— 328 suites, 2515 passed, 0 failed.Consumer
execute_qsy_actions(lib.rs:1262), reached from three sites:lib.rs:1537(responder, rx tick arm),:2079(auto-QSY, rx tick arm),:2540(AcceptQsy, command arm and rendezvous). Read, not grepped — getting this wrong is what made my first proposal ineffective.Prior art
#1007 (
8711a06c) is the rule and the source of the counting-backend instrument. #1142 is why the receive is inert — it narrowedrecord_rx_snrto validated frames, and this call site was not revisited.Twins
Capture entry points in daemon non-test code are exactly two: this one and the tick at
server.rs:888. So on the capture axis the issue is not wider than filed.The opposite twin is wider, and is now #1319:
rx_streamis assigned at four sites, none on the rx tick arm — yet that arm transmits (OTA ACK, CONACK, every QSY reply). #1007's post-transmit rule is enforced on the command arm only, and a QSY line at BPSK31 is tens of seconds of own-transmit audio handed to the next tick.Refactors: CAP-45
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6