Repository navigation
RTL: add SSB/CW and AM/SAM reception with selected decoder audio. Principle II. - #6432
Conversation
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Issue fit
No bug issue is linked — the PR cites RFC #5468 as its umbrella, which is the right authority for an architectural increment. I read #5468 and its thread: it conditionally approves the shared-capture/viewport/slice architecture, WDSP/r8brain reuse and 48 kHz audio, and it explicitly names "AM/SSB/CW are a later sprint" as in-scope-but-future for this RFC. So the feature is sanctioned. What #5468 does not settle, and says so in its own words, is the per-mode detail this PR now fixes in code:
"Final preset widths and FM deviation values need reviewer agreement and test coverage."
"SAM must be validated for synchronous tracking, not just recovery of a centered AM carrier."
"CW requires the established BFO/filter translation; selecting a WDSP CW enum alone does not create the expected pitch."
"resolve the currentFMN/NFMandCWR/CWLdifferences deliberately"
Mapping those to the diff: the BFO/filter translation requirement is met (RtlReceiveRecipe.h, extractor translation and WDSP filter shift cancelling at the RF boundary). The SAM requirement is met and met well — tests/rtl_am_sam_pipeline_test.cpp:173 asserts ≥15 dB coherent rejection of a quadrature artifact that an envelope detector would demodulate, which is a real discriminator rather than a restatement of the implementation. The CWR/CWL vocabulary question is not resolved deliberately; see nit 3. The preset widths are chosen unilaterally and still want a maintainer's eye.
Verdict: solves the increment it claims. The signal path is the most carefully built RTL work I have reviewed on this RFC. I found no correctness blocker in the DSP, persistence, capability or lifetime paths.
Scope
| File / group | What it changes | Claimed in body? | Verdict |
|---|---|---|---|
src/core/backends/rtl/*, src/core/dsp/WdspChannel.*, third_party/wdsp/include/aether_wdsp.h |
Native SSB/CW + AM/SAM recipes, tracking-notch bank, amd.c/ANR/ANF declarations |
Yes | In scope |
src/core/RtlSliceSettings.* |
Per-mode filter recall, receiveDsp object, notch bank persistence |
Yes (Principle V) | In scope |
src/core/backends/RadioCapabilities.h, ReceiveCommand.h |
New public capability records ReceiveDspControl, ReceiveCwPitchControl |
Yes | Needs maintainer decision — new capability surface third parties and other backends bind to |
src/models/TnfModel.*, TransmitModel.*, SliceModel.* |
ReceiveControlPolicy::Confirmed extended to notches and CW pitch |
Yes | In scope, but see decision item 1 |
src/asr/AsrEngine.*, src/gui/AsrAudioTap.*, CopyAssistController.*, src/core/PcmFrame.h |
Source lease + generation through queue→inference→delivery | Yes | In scope |
src/gui/ControlAvailabilityRegistry.*, RxApplet.cpp, VfoWidget.cpp |
registerRestriction(), per-mode receive-control dimming, RX/TX ANT availability |
Yes | In scope |
src/gui/AudioButtonIcon.h, TitleBar.cpp, RxApplet.cpp, VfoWidget.cpp (commit bb42653) |
Extracts the existing buildAudioIcon out of TitleBar so slice speaker buttons stop depending on an emoji glyph |
Yes ("operator-reported speaker-icon fix") | Bundled fix — correct and authority-backed (the deleted TitleBar comment itself documents why emoji was wrong), but it is a separate operator report with no issue cited |
src/gui/TitleBar.cpp (commit 3867c56) |
setRadioTabs() now clears m_discovering whenever any tab reads Connected |
Yes ("Connected indicator fix") | Bundled, and cross-family — see decision item 2 |
src/gui/MainWindow.cpp (commit 0b0e9c0) |
Deletes m_copyAssistController in preparePanadapterUiForShutdown() |
Yes ("Copy Assist teardown correction") | Bundled fix, no issue cited |
src/core/AutomationServer.cpp |
receiveDspControl gate for slice dsp; plus setAnfLevel now honoured |
Gate yes; setAnfLevel no |
Cross-family widening, undisclosed — nit 2 |
src/core/AetherDspModePolicy.* |
CWR added to the disable list; mix predicate now requires receiving |
Not explicitly | In scope (CWR is a new RTL mode) |
docs/user/src/components/LogAnalyzer/rules.json |
Line-number churn tracking RadioModel.cpp growth |
No | In scope — required maintenance, Static checks green |
docs/architecture/aetherd-touchpoints.md |
Generated include counts (ThemeManager 178→179, DecoderAudioModel 1→3, RadioModel 53→54, SliceModel 36→37) | Checklist mentions touchpoints | In scope |
docs/rtl-ssb-cw.md, docs/rtl-am-sam.md, docs/user/docs/* |
New signal contracts + user guide | Yes | In scope, but rtl-m1-runtime.md is now self-contradictory — nit 1 |
Four distinct operator-reported fixes ride along with the feature. All four are correct as far as I can tell; none is smuggled preference. But none cites an issue number, and one of them is cross-family.
Blockers
None. I could not find a correctness, canon or lifetime defect that breaks users or fails on current main.
Needs maintainer decision
-
New public capability surface.
src/core/backends/RadioCapabilities.h:63-80addsReceiveDspControlandReceiveCwPitchControl, andReceiveControlPolicy::Confirmednow reachesTnfModelandTransmitModel. These are declarations other backends and the caps-map doc will bind to for a long time. Per my reading, noFROZEN_BOOL_COUNTruling is needed — these arestd::optional<struct>records, not bools — but the shape itself is a maintainer call, especially given item 3 below.docs/architecture/radio-capabilities-map.md:978-1009documents them, which is the right half of the AGENTS.md contract. -
src/gui/TitleBar.cpp:659-666is a cross-family behavior change in an RTL PR.for (const RadioTabEntry& radio : radios) { if (radio.status == RadioTabStatus::Connected) { setDiscovering(false); break; } }
setRadioTabs()runs for every family. Before this,m_discoveringwas cleared only byTitleBar::onHeartbeat()(TitleBar.cpp:1601) — a network heartbeat. For Flex/HL2/Icom the end state is unchanged (the first heartbeat already cleared it), so the practical effect on network radios is that amber→green happens on tab status rather than first beat. But it also means that starting a new discovery scan (MainWindow.cpp:2449,MainWindow_Session.cpp:439setsetDiscovering(true)) is now silently undone on the next tab refresh whenever any radio is connected. The RTL need — "a USB receiver does not need a network heartbeat" — is real, but the fix is ungated: it does not ask the backend whether it has a heartbeat. I would want this either gated on a capability/backend property or split into its own PR with the operator report linked. The commit message and thertl-sdr.md:29paragraph both frame it as RTL-specific; the code is not. -
Preset widths and the
CWRvocabulary.RtlSdrBackend.cpp:313-321pins the per-mode filter bounds (USB0..12000, CW±12000, AM/SAM±100..±12000) andinitialReceiver()picks100..2900for the USB bootstrap. #5468 reserved these for reviewer agreement. Same forCWRas the user-facing token (nit 3).
Nits (non-blocking)
1. docs/rtl-m1-runtime.md now states the opposite of what the code does for AM/SAM — three places. The SSB/CW commit (ed59db3) edited this file; the later AM/SAM commit (a2a45b0) did not come back to it.
-
docs/rtl-m1-runtime.md:103:"USB, LSB, CW and CWR use native independent 48 kHz receiver recipes … AM/SAM retain their exclusive legacy DDC and 24 kHz format."
docs/rtl-am-sam.md:3says the opposite ("AM and SAM use the existing shared-capture extractor and prepared WDSP receiver bank at 48 kHz"), as doesdocs/user/docs/rtl-sdr.md:63("Both modes have independent 48 kHz audio"). Suggested replacement for that sentence:AM and SAM use the same native bank; their carrier and audio-filter contract is described in [AM/SAM reception](rtl-am-sam.md).
-
docs/rtl-m1-runtime.md:139— "FM/FM-N and CW/CWR establishment and necessary capture recentering prefer a hardware center…" omits AM/SAM, butRtlReceiveRecipe.h:25-26isamplitude(mode) || cw(mode) || Fm || Fmn, so AM/SAM are in the DC-clear preference, anddocs/rtl-am-sam.md:27says so explicitly. -
docs/rtl-m1-runtime.md:153— "This DC preference does not alter WFM, SSB or the remaining legacy demodulators." SSB and WFM are right (USB/LSB are not indcSensitive), but AM/SAM are no longer "remaining legacy demodulators".
This is the one item I would want fixed before merge: DEVELOPER-GUIDE.md now points reviewers at these contract docs, and a maintainer reading rtl-m1-runtime.md would conclude AM/SAM is still legacy 24 kHz.
2. src/core/AutomationServer.cpp:7884-7892 — the confirmed branch refuses turning a control off, where the legacy branch only refused turning it on.
if (receiveDsp && which != QLatin1String("squelch")) {
using Feature = ReceiveDspControl::Feature;
const Feature feature = which == QLatin1String("nr") ? Feature::Nr
: which == QLatin1String("anf") ? Feature::Anf : Feature::Nb;
if (!receiveDsp->supports(feature, s->mode())) {The old code guarded on on && …. A teardown script that issues slice dsp nr off across every slice now gets refused: nr is unavailable in receive mode FM on an RTL FM slice, where it previously succeeded as a no-op. Drop-in:
if (on && !receiveDsp->supports(feature, s->mode())) {Also in this hunk: src/core/AutomationServer.cpp:7917 adds if (level >= 0) { s->setAnfLevel(level); }. That is consistent with the nr branch above it and almost certainly right, but it changes slice dsp anf on 50 for every family (the level argument was previously parsed and ignored for anf). Worth a line in the PR body.
3. ReceiveDspControl::supports() is a flat modes × features product, so it cannot express "APF is CW-only" — and APF is declared over all six modes.
src/core/backends/RadioCapabilities.h:70-73:
bool supports(Feature feature, const QString& mode) const
{
return features.contains(feature) && modes.contains(mode);
}src/core/backends/rtl/RtlSdrBackend.cpp:268-272 declares modes = {AM, SAM, USB, LSB, CW, CWR} and features including Apf, so supports(Apf, "AM") == true. But src/core/backends/rtl/RtlReceiveRecipe.h:101 applies it only in CW:
dsp.apfEnabled = cw(receiver.mode) && receiver.apfEnabled;while RtlSdrBackend.cpp:1137 stores the request unconditionally and RtlSdrBackend.cpp:2180 reports it back as accepted (delta.apf = receiver.apfEnabled). So on an AM slice, a setApf(true) is accepted, published, and persisted (RtlSliceSettings.cpp:77) with no peaking filter running. This contradicts the record's own documented contract in docs/architecture/radio-capabilities-map.md:989 — "a present record makes unlisted features/modes unavailable" — and the repeated doc claim that "APF remains CW-only".
I could not reach it as a user-visible bug: src/gui/VfoWidget.cpp:4940 and :5531 already do m_apfBtn->setVisible(isCw), and the automation bridge's kWhich list has no apf. So today it is latent. If a per-mode feature map is out of scope for this PR, the honest minimum is to drop Feature::Apf from the declared set and keep APF gated by mode as it already is.
4. docs/user/docs/rtl-sdr.md regresses the user-facing CW-reverse label from CW-R to CWR, and the PR is internally inconsistent about it. rtl-sdr.md:33, :49, :55, :91 now say CWR (previously CW-R), while cw-decoder.md:41 and :63 — added in the same PR — say CW-R, and cw-decoder.md:21 says "CWR (shown as CW-R on some radios)". CWR is the internal mode token; the user guide had the friendlier form. This is exactly the "resolve the CWR/CWL difference deliberately" item #5468 asked for, so it is worth one decision rather than two spellings.
5. src/gui/VfoWidget.cpp:6815-6826 — the shared DSP level slider's predicate falls through to return false.
default: return false;On any receiveDspControl radio this disables the slider (with "This DSP parameter is unavailable in the current receive mode") for every target that is not LvlNB/LvlNR/LvlAnf/LvlMn — including LvlNone and the LvlNrl/LvlNrs/LvlNrf/LvlAnfl client-chain targets. Benign today: VfoWidget.cpp:3516 hides m_nrlBtn behind m_hasRadioSideDsp && m_hasLmsNoiseFilters, so those targets are unreachable on RTL. But a catch-all false on a widget whose reason string names the receive mode will mislabel the next target someone adds.
6. tests/rtl_slice_settings_test.cpp has no duplicate-notch-id refusal case. RtlTrackingNotches::valid() enforces uniqueness (RtlTrackingNotches.h:28-30) and decode() gets it for free from JSON key uniqueness, but patch() accepts a caller-supplied vector where duplicates are constructible. The width-199 refusal case at :68-71 is exactly the right shape — one more line in the same style would cover it.
7. tests/tests.cmake:1774 — missing blank line between add_test(NAME cw_mode_gate_test …) and add_executable(flex_slice_mode_intent_test. Cosmetic only; every other block in the file is separated.
Socket-test disclosure
No socket-owning test is added, modified or removed by this PR. I grepped every added/changed file under tests/ in the PR head for QTcpServer/QTcpSocket/QUdpSocket/QLocalServer/QWebSocketServer/bind(/listen(/connectToHost/class Fake. The only hit is tests/asr_engine_test.cpp:52 class FakeBackend : public IAsrBackend — a local inference stub, not a fake radio/amplifier/tuner peer, and pre-existing on main. The three new signal-path targets (rtl_am_sam_pipeline_test, rtl_ssb_cw_pipeline_test, cw_mode_gate_test) and asr_native_receiver_test are socket-free, which matches the body's claim. asr_native_receiver_test is correctly joined to AETHER_SETTINGS_CONSUMERS (tests/tests.cmake:8569).
Security audit
A CodeGuard security audit ran over the 41 flagged files and returned no confirmed findings. What it refuted, so this is not mistaken for a blanket clean bill:
- The ~400
CS-VLN-CMDI-*hits inAutomationServer.cppare lexical matches on Flex/TCI wire-protocol string literals and Markdown-quoted verbs in comments. No shell is invoked in any changed file. - Every
CS-PII-*,CS-VLN-SSRF-*,CS-VLN-SQLI-SLEEPandCS-VLN-PATH-*hit is insidedocs/user/src/components/LogAnalyzer/rules.json— a data file of regexes whose job is to match diagnostic log lines. The patterns are the rule content. CS-SEC-PASSWORDinCopyAssistController.cppis the pre-existing keychain-backed remote-ASR key field, line-shifted only.- The listener is unchanged: still
QLocalServerwithUserAccessOption, and the ~11 net new lines inAutomationServer.cppsit insidedoSlice, behind the shared-secret gate. No new bind, no new pre-auth verb.
What I tried to break
Everything below is reasoned from the code in the PR head checkout — I have no build and no way to run the GUI, so none of it was reproduced at runtime.
- The AM post-detector filter math, because it is the claim most likely to be hand-waved. It holds, and the comment at
WdspChannel.cpp:555-558is accurate.amd.c:142-143(AM) and:218-219(SAM) both write identical real audio intoout_buff[2i+0]and[2i+1].RXA.c:647/652/664putsbp1afterxamd, so the 50 Hz..max(-low, high)bandpass set atWdspChannel.cpp:1594-1597is a genuine post-detector audio filter and not an RF cut that would strip the carrier;RXANBPSetFreqskeeps the signed RF cuts onnbp0. For inputa+ja, a positive-frequency analytic filter yields(a−H{a}) + j(a+H{a}), so0.5*(L+R)recovers exactlya— andRXAbp1Check(RXA.c:906-920) only adjusts gain, never frequencies, so it cannot silently override the 50 Hz low cut.tests/rtl_am_sam_pipeline_test.cpp:150pins the resulting absolute level to within 5%, which is what makes that chain of reasoning checkable. - Whether
AetherDspSliceAudioState::receivingsilently disables the CW/NNR auto-disable on Flex. It does not.MainWindow_DspApplets.cpp:223feedsslice->inCapture();SliceModel.h:671defaultsm_inCapture{true}andSliceModel.cpp:1901only moves it when a backend sendsSliceDelta::inCapture, which only RTL does. Flex behavior is unchanged.MainWindow_Wiring.cpp:1960correctly re-runs the policy oninCaptureChanged, so parking an RTL CW slice actually re-evaluates. - Whether
ReceiveControlPolicy::Confirmedleaks across a radio swap and strands Flex notches. It does not.RadioModel.cpp:4337and:4348both reset toOptimistic/falsethroughpublishCapabilities(connected)on disconnect, andTransmitModel::resetState()clearsm_confirmReceiveCwPitchas well. - Whether the confirmed TNF loop actually closes. It does.
TnfModelnow returns early onconfirmsControls()for freq/width/depth/permanent/remove/global-enable, andRtlSdrBackend.cpp:2403/2411/2415emitnotchRemoved,notchChangedandRadioDelta::tnfEnabledafter adoption.createTnfneeded no guard — it was already intent-only.MainWindow_Wiring.cpp:4480rebuilds the markers onnotchChangeRequestedonly when confirmed, which is the right side of the fork. - Whether
maxNotchFiltersstays 0 for the whole session. It does not. It is conditional onhasAnalogDsp(RtlSdrBackend.cpp:273), andRtlSdrBackend.cpp:2426emitscapabilitiesChanged()precisely whenhasAnalogDsp(prior) != hasAnalogDsp(state), whichRadioModel.cpp:1159turns back intopublishCapabilities(). state.receivers.front()atRtlSdrBackend.cpp:2419with no empty guard, ten lines after:2259defensively handlesstate.receivers.empty(). Safe:RtlCaptureTransaction.cpp:228rejectsdesired.receivers.empty(), andremoveSlice()(:871) refuses atsize() <= 1, so a confirmed state always has ≥1 receiver.- Notch-id overflow and reuse.
m_nextNotchIdis re-seeded from restored entries with an explicitINT_MAXbranch (:1646), andcreateNotchrefuses atINT_MAX(:1156), so the restored-INT_MAXcollision I went looking for is closed. Ids are never used as indices — every consumer isfind_if/erase_if, and the WDSP slot is a separate local counter (WdspChannel.cpp:1639). - The ASR generation/lease race.
AsrEngine::reset()bumpsm_generationand callssetInputGeneration()directly (atomic, main thread) before queueingrequestReset, so an in-flight blocking transcribe failsinputCurrent()immediately rather than after the queued slot runs.requestProcessis queued after, so a stale chunk deterministically failsgeneration != m_requestedGeneration. Thesource == 0sentinel for the legacypushAudiopath is sound:PcmProducer::start()rejectsm_source == 0ands_nextSourcestarts at 1, so no real producer can masquerade as "no lease". pushNativePcmindexing (AsrEngine.cpp:658-667).frame.current()guarantees a non-null epoch,PcmFormat::channels()is 1 or 2 by construction, andframeCount()issamples.size()/channels, soi*channels + 1cannot run past the buffer.- A hand-edited settings file as a denial-of-reception.
RtlReceiveRecipe.h:113-120drops every notch outside[filterLowHz, filterHighHz]before WDSP, so a persisted 1.7 GHz notch cannot trip the stricter bank validation atWdspChannel.cpp:1406-1413.decode()only assignsoutputon full success (RtlSliceSettings.cpp:244), andpatch()re-decodes before writing, so a refused bank cannot overwrite good state —tests/rtl_slice_settings_test.cpp:68-71asserts exactly that. - The tests themselves, for whether they would pass against unfixed code. They would not.
rtl_am_sam_pipeline_test.cppmeasures demodulated audio — absolute modulation level (:150), second-harmonic distortion (:151), carrier-DC suppression (:152), quadrature rejection separating SAM from envelope AM (:173), both-sideband MN depth (:288), and epoch invalidation across retune/park/recreate (:221/227/237/243). Receiver controls are verified on audio, not inferred from a setter returning true.rtl_slice_settings_test.cpphas real refusal cases, not just round-trips. - The bundled speaker-icon commit, for smuggled preference. It is not.
bb42653extracts the existingbuildAudioIconout ofTitleBar.cppverbatim intoAudioButtonIcon.hand reuses it fromRxApplet/VfoWidget; the deleted TitleBar comment is itself the authority ("emoji render in the platform's own colour and weight, so they never matched the bar around them and could not follow the theme"). Extending that to the slice speaker buttons is a fix, not a restyle.
Could not check: live RF behavior, SAM PLL acquisition against a real station, and whether the ±2 kHz create_amd bound (RXA.c:180-181) is adequate in practice — the docs are honest that it is not qualified. I also did not evaluate the eight-receiver evaluation path beyond reading its guards.
CI: all 10 checks are green on head 35ea31ae3, including check-windows (the -DENABLE_ASR=OFF lane, which is where an unguarded ASR reference would surface) and check-macos. The body's unchecked "Hosted PR CI" box is now satisfied. Standing caveat: ctest in ci.yml is -R-filtered, so green proves the full-tree build plus a named subset, not the ~240-test suite — the three new pipeline targets are not in that gate.
Recommendation
Needs maintainer decision. There is no blocker: the DSP is correct and well-reasoned, the persistence is capability-shaped and refusal-safe, the confirmed-control loop actually closes, and the tests measure outcomes rather than asserting the implementation back to itself. This is high-quality work and it survived every attack I could mount from static reading.
What needs a human ruling is not correctness but surface: two new public capability records, the unilateral filter-preset and CWR-vocabulary choices that #5468 explicitly reserved for reviewer agreement, and one bundled commit (3867c56, the TitleBar discovery clear) that changes behavior for every radio family while being described as RTL-specific.
Concrete next step for the author, none of which needs a maintainer first: fix the three stale rtl-m1-runtime.md sentences about AM/SAM (nit 1), settle on one user-facing spelling of CW-reverse (nit 4), restore the on && guard in the automation branch (nit 2), and either drop Feature::Apf from RTL's declared set or say in the body that the record is a coarse product (nit 3). Then the open questions are purely the maintainer's: the capability shape, the presets, and whether 3867c56 should be gated or split out.
Thank you for the unusually careful validation write-up — scoping the earlier platform evidence to the pre-integration candidate rather than claiming it for 35ea31ae is the right call and made this much easier to review.
🤖 aethersdr-agent · cost: $21.8387 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (431 finding(s))
Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them. 🤖 aethersdr-agent · cost: $22.2882 · model: claude-opus-5 |
|
Antares Detector — candidate vulnerable file(s)
RtlRfExtractor uses std::memcpy with an index-based pointer expression (m_convertedI.constData() + index * sizeof(float)). The code increments m_stagedOutput and checks against m_config.blockSize, but does not validate that the computed index is less than m_convertedI.size()/sizeof(float). If index can exceed the number of valid stages, this will write out of bounds, causing a memory-safety vulnerability (CWE-190). Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting. 🤖 aethersdr-agent · cost: $22.2882 · model: claude-opus-5 |
rfoust
left a comment
rfoust
left a comment
There was a problem hiding this comment.
This implements the SSB/CW and AM/SAM increment of RFC #5468: actual filtering and demodulation, accepted-state controls and persistence, and selected-receiver decoder audio. It preserves the one-receiver production limit. No confirmed correctness blockers; recommendation: Needs maintainer decision on the new capability records and mode-default filters. The RFC reserves final preset widths for reviewer agreement, and GOVERNANCE.md reserves final architecture/UX direction to Jeremy.
Scope
| Change group | Claimed in title/body? | Assessment |
|---|---|---|
| RTL extraction, WDSP recipes, filters and DSP controls | Yes | Within RFC scope |
| Models, capabilities and persistence | Yes | Necessary; new capability shape needs maintainer agreement |
| CW decoder and Copy Assist routing/lifecycle | Yes | Within stated scope |
| Speaker artwork and antenna availability | Yes | Disclosed related fixes |
| Connection indicator | Yes | Disclosed; affects every radio family |
| Tests and documentation | Yes | Relevant; no CI gate expansion or CHANGELOG entry |
No unrelated commits or unexplained file groups found. The body's scope/dependency/settings checklist holds up, subject to the documentation and capability nits below.
Blockers
None confirmed.
Non-blocking findings
- Stale runtime documentation: inline comment on
docs/rtl-m1-runtime.md:103. AM/SAM now use independent 48 kHz receiver recipes. The bootstrap and DC-placement descriptions in the same guide also need updating. - APF capability overstates support: inline comment on
src/core/backends/rtl/RtlSdrBackend.cpp:271. The record advertises APF in every listed analog mode, while dispatch accepts it only in CW/CWR. Existing UI checks mask this, but the capability should describe actual feature-by-mode support.
Verification and attacks that survived
Reviewed main 47aa4098d2b3182c8608d9bd7f4b6d842758d84a, head 35ea31ae3f17cd56b79d24ae934b57c3854e14dd, and merge result 2ae0e10f6bd2421163872f5be7b381c124645da2. The head/base remain unchanged at posting. Current CI is green (release PDF attachment skipped); no existing inline review threads. CI is filtered and does not establish a full-suite pass.
- Native Apple Silicon application build passed with the prescribed ARM64 toolchain, RADE enabled, ARM64 host/system/executable checks, and no RNNoise x86 sources.
- All nine focused tests passed:
rtl_am_sam_pipeline_test,rtl_ssb_cw_pipeline_test,rtl_model_acceptance_test,rtl_slice_settings_test,receive_intent_routing_test,asr_engine_test,asr_native_receiver_test,control_availability_registry_test, andunified_title_bar_test. - Sideband rejection, SAM discrimination, filter/RIT boundaries, sparse restoration, parked-receiver handling, stale PCM/transcript rejection and cross-radio control routing survived these checks. The three signal/model tests passed in 178.13 seconds locally.
- Mutation evidence: temporarily removing the final ASR delivery generation/lease guards caused exactly two assertions to fail:
queued transcript from old generation is suppressedandproducer retirement suppresses queued native transcript without a reset callback. Restoring the exact source made the test pass again. - Engine boundary, touchpoint manifest, capability-record and command-plane checks passed on main and the PR. Per-file comparison found no increased boundary violations.
- Socket preflight inspected every changed test source. No socket-owning test was added or changed; the inference stub is not a synthetic firmware peer.
- Drove the merge-result application offscreen with isolated settings, explicit socket
/private/tmp/pr6432-review-971e.sock, and onlyDEMO-0001.whoamireportedtxAllowed: false;get radioreportedmodel: "AetherSDR Demo",serial: "DEMO-0001", andtransmitting: false. Verified green Connected presentation, mute convergence, Copy Assist visibility across USB/CW transitions, and simulator reconnect. After the mute/CW actions, model output includedmode: "CW",audioMute: true,frequency: 14.1,inCapture: true; Copy Assist was hidden in CW. - Copy Assist teardown logged
phase=asr.controller.destroy event=end result=ok elapsed_ms=0; the window and bridge closed, but the process lingered and required termination. Clean process exit remains unverified. This is an unattributed verification limit, not a PR regression finding without a baseline comparison. Both owned processes were cleaned up.
No live hardware, TX, sustained receiver-capacity qualification, or real-model speech inference was independently performed. The demo cannot exercise RTL USB capture/SAM reception; those conclusions rely on production-pipeline tests. No separate automated review tool was available; the second pass was manual.
Next step: correct the two non-blocking findings and obtain Jeremy's agreement on the new capability shape and mode defaults before landing. This is a comment review, not approval or a merge authorization.
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
|
Addressed the concrete review findings in signed commit d3ce283 (GitHub Verified).
Validation:
Advisory triage: the 19 accessibility warnings are identical to the trusted main snapshot (no additions). The The new capability shape, confirmed-control integration and filter presets remain for Jeremy's decision. The rescan regression is corrected, and the shared scope of the indicator fix is explicit; whether to keep that bundled correction here is still a maintainer scope call. No approval or merge is implied by this fix report. 73, Ozy K6OZY · Codex |
|
Resolved the new main conflicts in signed commit 3988aa2, incorporating main 83883fc. The integration preserves the new CW pitch slider and main's passive DSP level readback. RTL pitch requests keep both the slider and editor at the accepted value until backend adoption. The combined generated manifest and log references were regenerated. Validation on Nobara (Linux/GCC): 9/9 focused control/model/UI tests and 2/2 AM/SAM + SSB/CW pipeline tests pass. Temporarily removing the pitch-confirmation guard produces two assertion failures; restored source passes and the checkout is clean. Docs build and static/generated checks pass. Fresh hosted platform CI is required for this head; prior Windows/macOS results are not being represented as new native runs. The earlier review fixes remain in place. The capability-shape discussion remains open for maintainer agreement. No merge or auto-merge was requested. |
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
3988aa2 to
2e49962
Compare
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
jensenpat
left a comment
jensenpat
left a comment
There was a problem hiding this comment.
Reviewed exact head 2e49962504bcae7e50ea84e4e65c54a3275506e9, rebased onto main 3c81b3d81df3675122dd7688b2d2e8b343d0cc54. No confirmed correctness blockers remain. The operator explicitly requested review, rebase and merge of this proposed change; this landing retains the corrected capability records, filter presets and disclosed shared title-bar behavior under that direction.
The rebase preserves Ozy311's authorship and the prior review fixes. APF advertisement now matches CW/CWR-only dispatch; runtime documentation matches the 48 kHz native recipes; unsupported DSP-on requests retain cleanup-off behavior; connected-tab refreshes preserve rescans. Current-main pitch-slider confirmation, passive DSP readback, neural CW decoder integration, transmit acknowledgement routing and settings isolation are preserved. The CW/CWR helpers were integrated into main's existing model-layer ModeVocabulary owner rather than duplicated in the GUI header.
Verification on this exact head:
- Native ARM64 Qt 6.12.0 app, daemon and 22 regression targets built, ASR and RADE enabled; no RNNoise x86 sources in the ARM graph. The initial iconutil sandbox failure cleared on the unchanged permitted incremental build.
- 22/22 selected headless CTests passed in 89.54 s, including generated-IQ AM/SAM, SSB/CW and FM/WFM production pipelines, accepted RTL model state, transactional/scoped settings, typed cross-radio receive intents, ASR generation and native-source retirement, control availability, title bar, Phone/CW accepted pitch/APF, filter policy, TNF removal, CW mode gates and main's CW RX model.
- An isolated mutation omitting CWR from the shared mode vocabulary produced four CW mode-gate failures; the exact current implementation reports zero failures. Existing submitted regressions also cover the prior APF, discovery and pitch-confirmation findings; their current passes are included above.
- Owned offscreen bridge instance PID 6014, isolated settings, explicit socket and disabled autoconnect: only DEMO-0001 connected.
whoamireportedtxAllowed:false; radio state reportedtransmitting:false. Observed Copy Assist enabled in USB, disabled and unchecked after CW, enabled again in USB; mute readback wasaudioMute:true, and the demo reconnected successfully. Closing its MainWindow retired Copy Assist withphase=asr.controller.destroy event=end result=ok elapsed_ms=0; the owned process exited. - Main/head boundary comparison passes with the same 101 tracked findings, 70 capability booleans and 249 raw above-seam command strings. Generated touchpoints, seam probes, references and bridge docs; log rules; test registration; frozen test gate; WDSP test links; and QSettings checks pass. Accessibility findings match main's 19 existing warnings.
- Hosted Static checks, Docs and all three CodeQL analyses pass on this head. Linux, Windows and macOS builds are still running; protected auto-merge must wait for all required checks to pass. CI is a build/platform-specific filtered-test gate, not evidence of a full-suite pass.
Both review threads have evidence-bearing replies and are resolved. The extractor security advisory is not a demonstrated out-of-bounds path: Resampler::process sizes the byte output for its returned sample count, and the unchanged consumer bounds that count before indexing. The CodeGuard shell-injection candidates are protocol strings with no shell invocation; the log-rule findings are matching data. The added GUI/model includes consume existing receiver-selection models and existing ThemeManager artwork; no vendor-boundary or baseline weakening was introduced.
No separate automated code-review tool was available; the adversarial pass was manual. No live hardware, RF/TX, current-head real-model speech inference or sustained eight-receiver qualification was performed. The demo cannot prove RTL USB/SAM reception; those current conclusions rely on the generated-IQ production-pipeline tests. Earlier author/operator hardware and inference evidence remains explicitly historical.
… II. Rebase the accepted RTL analog receive increment onto main 9dfe8f6, preserving accepted-state pitch controls, current CW decoder integration, transmit acknowledgement routing and settings isolation. Keep CW/CWR vocabulary in the existing model-layer owner. Preserve all production-pipeline, model, UI and stale-audio regression coverage. Co-authored-by: jensenpat <patjensen@gmail.com>
2e49962 to
9df9135
Compare
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
jensenpat
left a comment
jensenpat
left a comment
There was a problem hiding this comment.
Reviewed exact head 9df9135d197aadba5f1fefb20abdff4a71ac0241, rebased onto main 9dfe8f666775b51a6012534bff7e7737ff0412b1. No confirmed correctness blockers remain. The operator explicitly requested review, rebase and merge of this proposed change; this landing retains the corrected capability records, filter presets and disclosed shared title-bar behavior under that direction.
The rebase preserves Ozy311's authorship and the prior review fixes. The final conflict against main #6430 was solely generated log-reference line numbers; regenerated references preserve the exact production/test patch. Current-main stored-control routing and dialog regressions are included in the new 24-test run. APF advertisement now matches CW/CWR-only dispatch; runtime documentation matches the 48 kHz native recipes; unsupported DSP-on requests retain cleanup-off behavior; connected-tab refreshes preserve rescans. Current-main pitch-slider confirmation, passive DSP readback, neural CW decoder integration, transmit acknowledgement routing and settings isolation are preserved. The CW/CWR helpers were integrated into main's existing model-layer ModeVocabulary owner rather than duplicated in the GUI header.
Verification on this exact head:
- Native ARM64 Qt 6.12.0 app, daemon and 24 regression targets built, ASR and RADE enabled; no RNNoise x86 sources in the ARM graph. The initial iconutil sandbox failure cleared on the unchanged permitted incremental build.
- 24/24 selected headless CTests passed in 92.61 s, including generated-IQ AM/SAM, SSB/CW and FM/WFM production pipelines, accepted RTL model state, transactional/scoped settings, typed cross-radio receive intents, ASR generation and native-source retirement, control availability, title bar, Phone/CW accepted pitch/APF, filter policy, TNF removal, CW mode gates main's CW RX model and stored-control routing/dialog regressions.
- An isolated mutation on the preceding
2e4996250head, with byte-identical helper/test code on this head, omitting CWR from the shared mode vocabulary produced four CW mode-gate failures; the exact current implementation reports zero failures. Existing submitted regressions also cover the prior APF, discovery and pitch-confirmation findings; their current passes are included above. - Owned offscreen bridge instance PID 10312, isolated settings, explicit socket and disabled autoconnect: only DEMO-0001 connected.
whoamireportedtxAllowed:false; radio state reportedtransmitting:false. Observed Copy Assist enabled in USB, disabled and unchecked after CW, enabled again in USB; mute readback wasaudioMute:true, and the demo reconnected successfully. Closing its MainWindow retired Copy Assist withphase=asr.controller.destroy event=end result=ok elapsed_ms=0; the owned process exited with code 0. - Main/head boundary comparison passes with the same 101 tracked findings, 70 capability booleans and 221 raw above-seam command strings. Generated touchpoints, seam probes, references and bridge docs; log rules; test registration; frozen test gate; WDSP test links; and QSettings checks pass. Accessibility findings match main's 19 existing warnings.
- Hosted checks are running again on this rebased head; protected auto-merge must wait for all required checks to pass. The preceding head
2e4996250passed Linux, Windows, macOS, Static checks, Docs and CodeQL; those results are scoped to that head. CI is a build/platform-specific filtered-test gate, not evidence of a full-suite pass.
Both review threads have evidence-bearing replies and are resolved. The extractor security advisory is not a demonstrated out-of-bounds path: Resampler::process sizes the byte output for its returned sample count, and the unchanged consumer bounds that count before indexing. The CodeGuard shell-injection candidates are protocol strings with no shell invocation; the log-rule findings are matching data. The added GUI/model includes consume existing receiver-selection models and existing ThemeManager artwork; no vendor-boundary or baseline weakening was introduced.
No separate automated code-review tool was available; the adversarial pass was manual. No live hardware, RF/TX, current-head real-model speech inference or sustained eight-receiver qualification was performed. The demo cannot prove RTL USB/SAM reception; those current conclusions rely on the generated-IQ production-pipeline tests. Earlier author/operator hardware and inference evidence remains explicitly historical.
jensenpat
changed the title
…#6637). Principle VIII. Repair stale test expectations on main after production changes in aethersdr#6432 and aethersdr#6524: - control_receive_test: locate FM, FMN, and WFM modes dynamically across declared filter modes instead of indexing by position (aethersdr#6432). - rtl_capture_worker_test: remove obsolete backend-level legacy AM filter block; native receive rejects AM filters wider than ±12 kHz (aethersdr#6432). - radio_setup_operations_test: account for per-band RCA off/TUNE toggle in TX Band Settings (aethersdr#6524). Addresses aethersdr#6555. Co-Authored-By: Jeremy [KK7GWY] <kk7gwy@aethersdr.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
Adds the SSB/CW and AM/SAM receive increment of RFC #5468 through RTL's existing shared capture, extractor and WDSP receiver bank. USB, LSB, CW, CWR, AM and synchronous AM produce independent 48 kHz receiver audio. Filter edits affect the passband; each mode recalls its last accepted filter. Receiver AGC, blanking, noise reduction, notches, APF and RIT publish and persist accepted state. CW pitch preserves the displayed RF carrier, and explicit CW/AM tuning prefers a capture clear of converter DC.
The selected receiver feeds CW decoding and Copy Assist before speaker gain/mute. Retuning, parking and selection changes retire stale audio and transcription results. The receive DSP capability record restricts APF to CW/CWR. The receive pitch record enables pitch control without granting TX. RTL retains the existing CWR mode token and scoped settings document.
Includes the reported speaker-icon fix, fixed receive-only antenna availability and Copy Assist teardown ordering. The shared Connected-indicator correction ends discovery on a newly confirmed connection, retains real link-loss alarms, and preserves rescans on repeated connected-tab refreshes. Automation forwards the existing ANF level argument; unsupported DSP-on requests are refused while cleanup off intents remain allowed.
Uses existing DSP dependencies and threads. Normal receiver capacity remains one; 2/4/8 are process-only evaluation settings. FM/FM-N/WFM retain their recipes. Digital broadcast decoding remains in #6002 / RFC #6079. This does not close the umbrella RFC.
Integration and review
Rebased onto main
9dfe8f666775b51a6012534bff7e7737ff0412b1as signed commit9df9135d197aadba5f1fefb20abdff4a71ac0241, preserving Ozy311's authorship. Keeps main's neural CW decoder integration, pitch slider, passive DSP readback, transmit acknowledgement route, profile/memory routing and settings consumers. The only final rebase conflict was regenerated log-reference line numbers; the production/test patch is unchanged. CW/CWR helpers use the existing model-layerModeVocabulary.howner.The operator requested review, rebase and merge of this proposed change. The prior runtime-documentation, APF contract, automation cleanup, rescan and pitch-confirmation findings are corrected in this head. No workflow edits, CHANGELOG entry, frozen test-gate expansion or boundary-baseline increase.
New GUI/model touchpoints consume existing selected-receiver models: AsrAudioTap and CopyAssistController use DecoderAudioModel; RadioModel/SliceModel observations provide selection and lifecycle state. AudioButtonIcon shares the existing ThemeManager artwork. No new vendor-header crossing or engine-to-GUI dependency.
Validation
Current head
9df9135d1:2e4996250(unchanged helper and test on the final head): omitting CWR from the shared mode vocabulary causes four CW mode-gate failures; the exact current source passes. No production source was altered for the isolated mutation.pr6432-final-20261017, PID 10312, explicit socket, fresh settings and autoconnect disabled: only DEMO-0001 connected;whoamireportedtxAllowed:false, radio state reportedtransmitting:false. Confirmed USB/CW/USB Copy Assist menu and panel transitions, slice mute readback, reconnect and window shutdown. Copy Assist destruction loggedresult=ok elapsed_ms=0; the owned process exited with code 0.2e4996250head passed all hosted checks; that evidence is scoped to that head.Earlier operator validation on the pre-rebase candidates confirmed USB, LSB, CW, AM/SAM reception and CW decoding with an IC-705 signal source, plus ASR on the SSB/CW candidate. Earlier explicit-fixture Whisper inference on demodulated AM/SAM speech produced nonempty transcripts. These are retained historical evidence, not new live RF or inference runs for the rebased head.
Limits
The demo does not exercise RTL USB capture or real SAM acquisition. Current local evidence is generated-IQ production-pipeline testing and shared demo UI behavior. No new live hardware, TX, instrumented RF sensitivity/distortion, portable macOS packaging, transcription accuracy or sustained eight-receiver qualification is claimed. SAM retains the existing ±2 kHz PLL bound; large carrier changes may require retuning. AM/SAM squelch is unavailable; APF is CW-only.
Generated with OpenAI Codex (GPT-6 Astra)