Repository navigation
feat(gui): FFT Smooth and FFT De-comb shape the panadapter trace across frequency. Principle VIII. - #6371
Conversation
…ss frequency. Principle VIII. The Display panel's FFT AVG and Wt Avg average over time. Two new sliders, both off by default, work across frequency on the trace only; the waterfall, noise-floor readout and auto-squelch keep the raw bins. Both are client-side, so every radio gets them. FFT Smooth (0-16 px, 0 = off) is the panadapter's counterpart of the AetherRX/AetherTX EQ analyzer's Smoothing: each trace point becomes the mean of its neighbors in linear power, then back to dB, so the noise floor does not drop the way a dB mean would. The EQ's window is a fraction of an octave on a log axis; the panadapter's axis is linear, so here it is a half-width in logical pixels, scaled to the GPU's device-pixel and the CPU path's oversampled point density. FFT De-comb (0-600 Hz, 0 = off) is for a zoomed-in SSB voice, which at ~15 Hz per point resolves into a comb of pitch harmonics. Measured off a 40 m pan, three stations combed at ~105, ~180 and ~180 Hz -- each station's own pitch, so the comb is real signal, not a receiver artifact. A mean wide enough to hide it drops the peaks several dB, so De-comb replaces the trace with its cepstral true envelope (Röbel & Rodet, core/dsp/SpectralEnvelope): a DCT of the dB trace, coefficients finer than the width tapered out, iterated with over-relaxation (alpha 3, at most 12 passes; plain iteration still sat ~4 dB under the peaks after 6). The width is in Hz because the comb is a fixed spacing in Hz at any zoom; under two points per width the span cannot resolve a comb and it does nothing. Being a smooth curve it rounds a signal's steep edges over about half the width; the tooltip and docs say so. FFTW r2r runs on 2^a3^b5^c-padded lengths (prime lengths are ~5x slower). Plans are cached per length and created or freed only on a try_lock of the FFTW planner lock, since other threads hold it for seconds (#5895): a first frame at a new length that cannot plan draws raw, and closing a pan never waits. The envelope would lift a noise floor toward its own peaks, so a gate blends back to the raw trace within 8-14 dB of the estimated floor. Both apply last in buildFftDisplayTrace, De-comb then Smooth, on top of the existing plateau-gated 5-tap blend, so at 0 the trace is today's bit for bit. Settings are one owned object per pan (Principle V/XIV): DisplayFftTraceShaping = {version, smoothPx, decombHz}, written in one save; carried by Clone to all pans, reset by Reset, and reported as fftSmoothing / fftDecombHz by the bridge. SpectralEnvelope.h is tagged ui-support in the aetherd touchpoint manifest. fft_trace_smoothing_test pins off-is-identity, the linear-power mean, 2N+1 impulse spread, edge clamping, and an exact floor beside a +10 dBm carrier (why it sums directly rather than through a prefix sum). fft_trace_decomb_test drives a synthetic 2.7 kHz voice with 105 and 180 Hz combs, 18 dB deep, over unaveraged noise: the outline holds within -1.32..+2.42 dB, the skirts do not widen, the half-way point stays within one window of the true edge, and the floor median does not rise. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
ten9876
left a comment
ten9876
left a comment
There was a problem hiding this comment.
Review of 874126715f1bfc5a3bbc7e60103caaee1ed3633d: Approve with nits. No linked issue or RFC; I reviewed against the stated intent. The two optional, off-by-default frequency-shaping controls, display-only processing and owned settings document are implemented and survived the focused attacks below. Native rendered traces and real SSB appearance remain unverified. Jeremy should confirm the new control direction through normal maintainer/CODEOWNERS review; I found no demonstrated architectural change requiring an RFC.
Scope audit
| File/group | Claimed? | Verdict |
|---|---|---|
src/core/dsp/SpectralEnvelope.{cpp,h}, src/gui/FftTraceSmoothing.h, FftTraceDecomb.h |
Yes: envelope, smoothing, floor gate | In scope |
SpectrumWidget.{cpp,h}, SpectrumOverlayMenu.{cpp,h}, MainWindow_Wiring.cpp |
Yes: rendering, sliders, persistence, clone/reset, bridge state | In scope |
CMakeLists.txt, tests/tests.cmake, both new FFT trace tests |
Yes: build and socket-free coverage | In scope |
docs/architecture/aetherd-touchpoint-tags.json, aetherd-touchpoints.md |
Yes: classify new display-support include and regenerate manifest | In scope |
docs/automation-bridge.md, docs/user/docs/panadapter-controls.md |
Yes: controls and state documentation | In scope |
All 17 changed files are explained by the body. The single relevant commit has a verified good GPG signature. No unrelated hunks, changed existing defaults, new external dependency, protocol verb, or enforcement weakening found.
Blockers
None.
Non-blocking nits
-
De-comb eligibility depends on interpolated drawing density.
src/gui/SpectrumWidget.cpp:9379–9382uses the resampled point count for the two-points-per-width gate. At the same 1000 logical-pixel width, 200 kHz span and 300 Hz setting, the DPR-1 GPU path has about 1.50 points per width and skips De-comb; the CPU two-times sampling / DPR-2 GPU path has about 3.00 and enables it. Interpolation changes eligibility without adding source FFT resolution.I reproduced this by calling the actual head's production trace builder on identical 512-bin input (alternating −130/−135 dB with a three-bin carrier), copying each result before its scratch buffer could be reused:
points=1000 maxDeltaDb=0.000000 changed=0 rawCarrier=-40.00 points=2000 maxDeltaDb=0.385841 changed=14 rawCarrier=-40.00This is a measured small inconsistency, not a demonstrated blocker. Please choose a consistent resolution criterion for eligibility (for example, source-bin resolution), retain the appropriate resampled spacing for the envelope calculation, and add a socket-free density-boundary test. The numerical reproduction uses production code; assigning these counts to CPU/GPU paths comes from source inspection, not native GPU pixel capture.
-
The one-frame planner-contention claim is too strong.
src/gui/SpectrumWidget.cpp:9389–9391and the PR body's “first frame” worst-case description imply that the next frame necessarily gets a plan. An uncached length returns false on every attempt while another thread holds the planner mutex; cached lengths can still compute. The contention probe confirmed the fresh-length failure leaves output untouched. Replace the comment with:// At an uncached length, frames draw without De-comb until // the planner lock becomes available; each frame retries.
Please make the body equally precise. FFT Smooth can still run on those frames, so “without De-comb” describes the fallback more accurately than “raw.”
What I tried to break
- Exact-head build and focused tests: lean RelWithDebInfo, Qt 6.12, shared-core review configuration; built
AetherSDRand the five named tests only. All five passed:fft_trace_smoothing_test,fft_trace_decomb_test,spectrum_overlay_fft_fps_bounds_test,waterfall_manual_black_test,waterfall_impulse_blanker_test. Test-boundary preflight found no socket ownership in these targets. No broad suite was run. - Mutation and numerical attacks: replacing linear-power smoothing with a dB mean made four assertions fail. Reducing envelope iterations from 12 to 1 failed both comb-outline fixtures by approximately 10.6 dB. Unmodified fixtures stayed within −0.76/+1.91 dB (105 Hz) and −1.32/+2.42 dB (180 Hz), with 0.00 dB median floor rise. Flat input remained flat; cached-plan computation survived concurrent planner-lock ownership; fresh-length fallback preserved output. The public-method reference was checked against Röbel & Rodet, DAFx 2005; this implementation has its own iteration/taper choices.
- Demo GUI and persistence: drove only isolated, offscreen
DEMO-0001instances on explicit socketaethersdr-pr6371-head, with TX disabled.get displayreported defaults 0/0, extrema 16/600, then Reset 0/0. Setting 7/300 wrote one whole object,{"decombHz":300,"smoothPx":7,"version":1}, and both values returned after a process restart. The Display panel screenshot showed both controls fitting without label overlap; accessible names/descriptions and slider ranges were present. Both review-owned instances were closed; the operator's instance was untouched. Separate actual-widget probes showed malformed JSON/object type falls back to off without destroying the stored row, and out-of-range values clamp to 0/600. - Source and boundary attacks: raw waterfall, noise-floor, FFT Floor Auto and autosquelch consumers retain their unshaped data; scratch-buffer mutation does not overwrite source bins. Clone wiring and reset/menu synchronization include both controls. Clone/second-pan isolation was inspected, not exercised in the single-pan demo. Base/head gate comparisons retain 102 existing engine-boundary findings, 70 capability records and 32 command-plane files / 368 commands; the touchpoint manifest grows from 251 to 252 for the disclosed display-support include. Manifest and generated reference/bridge checks pass. All six accessibility-bot warnings predate this PR. A model-invocable automated code-review skill was unavailable; this was a manual review with independent code/governance audits.
CI: nine applicable checks passed; release-only PDF attachment was skipped. Earliest head workflow creation was 2026-10-11 03:18:07 UTC, after current main e56cb0ca664db9b5c4c12a36d8810faa8e40c8fd at 02:02:48 UTC, so the reviewed green runs were fresh at the final check.
Verification limit: offscreen QRhi cannot render SpectrumWidget; its capture returned grab produced an empty image for SpectrumWidget. Control pixels, state, restart persistence and production trace-builder numerics were verified, but native GPU/CPU trace pixels, multiple pans, hardware-family equivalence and real SSB visual quality were not. A native rendering capture would settle the visual claim; real firmware work needs a separately authorized review session.
Recommendation: Approve with nits. No reproduced correctness or canon blocker. Address the density consistency and contention wording, and have the maintainer confirm the additive control direction through the normal review. This is a COMMENT review; formal approval remains the operator's call.
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
1. Issue fit
No linked issue and no RFC. I searched (FFT smoothing spectrum display, panadapter trace smoothing comb harmonics) and found no issue this closes, so I reviewed against the commit message's own stated intent. Against that, it delivers what it claims: two off-by-default client-side controls that shape the drawn trace across frequency, a cepstral envelope in src/core/dsp/, per-pan persistence, Clone/Reset/bridge coverage, and two tests.
On canon: I read GOVERNANCE.md §"What requires an RFC" and it does not cleanly cover this — two off-by-default rows added to an existing Display panel, no change to default UX behaviour, no new external dependency (FFTW3 is already a hard requirement at CMakeLists.txt:500). "New applets or dialogs that don't change existing UX" is explicitly on the not required list. So I am not calling this an RFC violation. What does need a maintainer's eye is the new public surface (below) and the adjacency to open RFC #5782.
#5782 (open, maintainer-review) is adjacent context, not a rule this breaks. Its §5 Option C — rejected — is about temporal averaging above the backend seam, on three grounds: wrong domain (dB), wrong frames (post-fps-cap), doesn't serve the waterfall. This PR is spatial, display-only, trace-only, and smoothFftTraceLinearPower averages in linear power, so the domain objection does not apply and #5980 ("backend-side, as #5782 ruled") is about a different axis. But #5782 §8 third bullet explicitly flags SpectrumWidget's existing client-side shaping as something a maintainer should rule on, and this adds two more stages to that same chain. Worth reconciling before merge; not a blocker.
2. Scope
The harness handed me a wider audit list than the diff. For the record, git show --stat on 8741267 touches 17 files; src/gui/MainWindow.cpp, src/gui/MainWindow_Menus.cpp and docs/user/screens.json are not among them, which is where most of the CodeGuard findings live (see §4).
| File / group | What it changes | Claimed? | Verdict |
|---|---|---|---|
src/core/dsp/SpectralEnvelope.{h,cpp} |
New cepstral true-envelope DSP | Yes | In scope |
src/gui/FftTraceSmoothing.h, FftTraceDecomb.h |
New header-only smoothing + floor gate | Yes | In scope |
src/gui/SpectrumWidget.{h,cpp} |
Two members, setters, DisplayFftTraceShaping load/save, shaping in buildFftDisplayTrace, two snapshot keys |
Yes | In scope |
src/gui/SpectrumOverlayMenu.{h,cpp} |
Two slider rows, two signals, syncExtraDisplaySettings params |
Yes | In scope |
src/gui/MainWindow_Wiring.cpp |
Clone / Reset / Defaults / two connects | Yes | In scope |
CMakeLists.txt |
One line: SpectralEnvelope.cpp into CORE_SOURCES |
Yes | In scope |
tests/fft_trace_{smoothing,decomb}_test.cpp, tests/tests.cmake |
Two new socket-free CTest targets | Yes | In scope |
docs/architecture/aetherd-touchpoint{s.md,-tags.json} |
Tags core/dsp/SpectralEnvelope.h ui-support, 251→252 / 212→213 |
Yes | In scope (Static checks green, so the count check agrees) |
docs/automation-bridge.md, docs/user/docs/panadapter-controls.md |
Bridge example + two user-doc rows | Yes | In scope |
Nothing in the diff is unexplained by the stated intent. No CHANGELOG.md entry (correct — please keep it that way). No deleted guard, confirmation or early return: the only - lines are the two syncExtraDisplaySettings signatures, the buildFftDisplayTrace signature, and the two return displayBins / return trace statements that became applyFftSmoothing(...). No preference change smuggled in — both controls default to Off and applyFftSmoothing is a straight pass-through at 0, so the existing trace is untouched for every existing user.
New public surface → maintainer call (per the review table, not a defect): fftSmoothing / fftDecombHz in both panstatsSnapshot and automationDisplaySettingsSnapshot, the DisplayFftTraceShaping settings document, and two new menu signals. The bridge keys are documented at docs/automation-bridge.md:1375; the panstats pair is not, but that snapshot has no documented key inventory, so there is nothing to diverge from.
Socket tests: none added, none removed, no QTcpServer/QUdpSocket/QLocalServer/bind/listen/connectToHost/Fake* peer in either new test. Both are pure in-process computation.
3. Blockers
None. The findings below are worth fixing but none of them breaks an existing user or fails on current main.
4. Findings (non-blocking)
4.1 The De-comb floor gate is calibrated against a different statistic than its comment reasons about, and the test fixture can't see the gap
src/gui/FftTraceDecomb.h:22-26:
The envelope rides peaks, so over noise it would lift the floor toward its
own noise peaks. [...] A single unaveraged frame's noise peaks reach ~8-10 dB
over the median, so the ramp starts at 8 dB abovefloorDbmand is fully on
at 14 dB.
But the floorDbm actually passed in is estimateNoiseFloorDbm (SpectrumWidget.cpp:9394), which is the mean of the bins at or below the dB mean (src/gui/NoiseFloorEstimator.h:32-39) — not the median. On exponentially-distributed per-bin power (a single unaveraged periodogram) that statistic sits ~5.5 dB below the median, and that offset is what moves the gate out of its intended regime.
Arithmetic on the gate formula, 600-point trace, 20-point window (= 300 Hz at the 15 Hz/point the test uses), envelope modelled as the local max over one window (what a peak-riding true envelope converges to):
| floor model | estimateNoiseFloorDbm |
median | local peak | peak − estimator | gate w |
floor rise |
|---|---|---|---|---|---|---|
| true unaveraged periodogram (noise only) | −136.6 | −131.1 | −123.7 | 12.9 dB | 0.82 | +6.1 dB |
| the test's floor (const −130 + noise) | −128.6 | −127.5 | −122.8 | 5.9 dB | 0.00 | +0.00 dB |
The reason the fixture lands in the safe row is tests/fft_trace_decomb_test.cpp:46-49 — voiceComb() returns the constant kFloor outside the signal, and main() at :84-89 then power-sums noiseFloor() on top. Summing a constant pedestal with equal-power noise makes the floor ~3 dB hotter and roughly halves its skew, so its local peak is 5.9 dB over the estimator, just below the 8 dB ramp start — w == 0 across the whole floor region and "noise floor kept" passes with a rise of exactly 0.00 dB.
To be fair to the test: it is not vacuous. Delete the gate and out == env, the floor median rises ~+4.7 dB in that same fixture and the rise < 1.0 assertion fails. The gate is covered; what is not covered is the gate's calibration against the floor distribution the comment names.
Fix is one of two, author's choice:
// in tests/fft_trace_decomb_test.cpp main(), floor region = noise only
for (int i = 0; i < kPoints; ++i) {
auto& x = in[static_cast<std::size_t>(i)];
if (x == kFloor) {
x = noise[static_cast<std::size_t>(i)];
continue;
}
x = 10.0f * std::log10(std::pow(10.0f, x / 10.0f)
+ std::pow(10.0f, noise[static_cast<std::size_t>(i)] / 10.0f));
}and then raise the ramp so the fixture passes honestly (kRampStartDb = 14.0f, kRampSpanDb = 6.0f puts "fully on" above the ~13 dB local-peak level), or keep 8/14 and express them relative to a median rather than estimateNoiseFloorDbm. Either way the comment should name the statistic the code actually uses.
Reasoned from code plus a Python model of the gate formula with local-max as the envelope proxy — I did not run SpectralEnvelope. Severity is medium-to-nit rather than blocker because on a Flex the frames arrive radio-averaged and m_smoothed is itself a client EMA (#5782 §8), so the true-periodogram row needs FFT AVG at 0 on a raw-spectrum backend.
4.2 renderedTrace.floorDbm now means different things in the two renderers
src/gui/SpectrumWidget.cpp:15233 (GPU):
renderedTraceFloorDbm = estimateNoiseFloorDbm(trace);trace is now the shaped output. SpectrumWidget.cpp:15708 (CPU) still records estimateNoiseFloorDbm(displaySpectrumBins()) — raw bins. Because FFT Smooth collapses the floor's scatter toward its mean power, that estimator moves a long way: on an exponential floor it reads −7.5 dB (relative to mean power) raw, −3.0 at halfWidth 1, −1.4 at 4, and −0.6 at 16 — i.e. up to ~7 dB higher on the GPU path than the CPU path for the same frame. A bridge or radiocert consumer comparing panstats.renderedTrace.floorDbm across render modes, or across an FFT Smooth change, sees a step that isn't a radio change. Worth either measuring both paths on the same array or noting the shaping dependency in docs/automation-bridge.md.
(The user-facing readouts are fine: noiseFloorAutoLevelBins() returns raw bins, so FFT Floor Auto and measuredNoiseFloorDbm are genuinely unaffected, as the tooltip claims.)
5. Nits
src/gui/SpectrumOverlayMenu.cpp:1838-1840anddocs/user/docs/panadapter-controls.md:203— "Averages in linear power, so the noise floor stays put" / "so the noise floor stays put". True of mean power (exact to 0.01 dB in my model at every width), but the floor the eye reads is the bottom of the grass, and collapsing the scatter raises the drawn median ~1.5 dB and the estimator-measured floor by up to ~7 dB (§4.2). Something like "the floor's average level stays put — the scatter collapses toward it, so the bottom of the grass rises" would survive a user measuring it.src/gui/SpectrumWidget.cpp:9388-9390— "False only on the first frame at a new trace length while another thread holds FFTW's planner lock".FftwPlannerLock.hsays holds are "tens of seconds for a coldWdspChannel::open()". So resizing a pan while a slice opens leaves De-comb silently off for that whole window and then snaps on, which is more than "the first frame". The behaviour is right; the comment undersells it.src/gui/SpectrumWidget.cpp:4426-4434— each setter callssaveFftTraceShaping()→setValue+save(), and the slider is wired onQSlider::valueChanged(SpectrumOverlayMenu.cpp:1845), so a drag commits a SQLite transaction per tick, andcloneDisplaySettingsToAllPansdoes two per pan.setFftLineWidthon the same panel has exactly this shape (:1913), so this is precedent rather than a new defect — but a single combined write (orsliderReleased) would be cheaper.tests/— nothing covers the Hz→points mapping atSpectrumWidget.cpp:9374-9379(the only non-header logic in the feature) or theDisplayFftTraceShapingclamp on an out-of-range stored value. Both are small; the clamp in particular is a two-line test againstTestSettingsProfile.h. Non-gating.
6. What I tried to break (and didn't)
- "At 0 the trace is today's bit for bit." Holds.
applyFftSmoothingreturnspointsuntouched when both are 0, with no allocation and no extra pass, andfft_trace_smoothing_testpinsout[i] != in[i]as a failure at halfWidth 0. Verified bothreturnsites atSpectrumWidget.cpp:9414/:9439. - A stale
syncExtraDisplaySettingscaller snapping both sliders to Off (defaulted params +QSignalBlocker) while the widget members stayed live. All four call sites are updated:MainWindow_Wiring.cpp:3564,:5023,:5140,SpectrumWidget.cpp:2618. No stale caller in the head checkout. - Reset / Clone / Defaults asymmetry.
displaySettingsResetsets both to 0 and syncs the panel with0, 0;cloneDisplaySettingsToAllPanscopies both and re-syncs the destination menu; the Defaults lambda's..., 8, 0, 0)isfreqScaleFontPt=8+ the two new zeros, which is correct against the header's parameter order. - "The waterfall and the 3D surface keep the raw bins." Holds.
buildFftDisplayTracehas exactly two callers (:15229,:16032), both the 2D trace; the waterfall and DSS go throughcroppedBinsForDisplaydirectly (:9078,:12766). - hzPerPoint against the ANAN edge crop.
effectiveBandwidthMhz()already narrows viapanDisplayBandwidthMhz(.., panEdgeCropActive()), matching the cropped bin array, so De-comb's width-in-Hz stays honest with the crop on. Not a bug. - Logical-vs-device pixel mismatch between renderers for FFT Smooth. Both pass logical width (
specContentWisspecRect.width() - DBM_STRIP_W; the CPU path passesr.width()), andpointsPerPxis derived from the actualn, so the on-screen window is the same at either sampling density. - The narrow-span degenerate case — a nonzero De-comb flattening the whole trace when the window approaches the span. Only the lower guard (
windowPoints >= 2.0) exists, but at the 10 kHz default minimum (m_minBwMhz{0.010}) a 600 Hz width leaves ~33 DCT terms and at a 5 kHz Flex minimum ~17, so I could not reach a flat trace through the UI. Not a finding. - Memory safety and FFTW lifetime in
SpectralEnvelope.m >= nalways (smoothLengthstarts atn), every buffer is resized toMbefore use,!(cutoffPoints >= 2.0)rejects NaN,kc > 0strictly, in/out aliasing is safe becauseoutis written only at:191-193, andFFTW_UNALIGNED+ new-array execute makes the plan-time local vectors going out of scope legal. Thetry_to_locktakes the double-precision mutex matching itsfftw_plan_r2r_1d, and that mutex has real contenders, so it serializes against something. The security-audit subagent (CodeGuard) reached the same conclusion independently. - Attacking the tests.
fft_trace_smoothing_testfails if the implementation is replaced by a dB-domain mean (-84.6vs-93.3at :49), by a prefix sum (the exact-140.0beside a +10 dBm carrier at :92), or by edge padding instead of clamping (:83).fft_trace_decomb_testfails ifcomputereturns false, if the gate is removed (§4.1), or if the envelope sits under the comb peaks. These are real assertions, not the implementation echoed back. The one gap is §4.1's fixture distribution. - CMake / linkage.
aethercoregets${FFTW3_INCLUDE_DIRS}PUBLIC and links${FFTW3_LIBRARIES}(CMakeLists.txt:3016,:3019), andfft_trace_decomb_test'saethercore Qt6::Core ${FFTW3_LIBRARIES}is the same pattern ashl2_spectrum_test:1466andhl2_pan_averaging_test:1484— not a new linkage shape. Neither new target touchesAppSettings.cpp, so theAETHER_SETTINGS_SOURCES/AETHER_SETTINGS_CONSUMERScontract doesn't apply. - Settings canon.
docs/agents/settings.mdputs "client-only display appearance preferences" in flatAppSettingskeys, andDisplayFftTraceShapingis one versioned JSON document written in a singlesetValue+save— the same shape assaveDisplay3DSettingsatSpectrumWidget.cpp:2909, Principle V satisfied, no migration needed for a new key. The "check the write result" rule in that doc attaches toradio_settings'setFeature(), notAppSettings::setValue(), so not applicable. - a11y. Both new sliders are
GuardedSliderviamakeRow(wheel guard inherited), carrysetAccessibleName+setAccessibleDescriptionand object names, and the value column already reserves width for"-260 dBm", so"Off"/"600"cannot clip.
CodeGuard (§4 of the brief): all 98 findings refuted. src/gui/FftTraceSmoothing.h:22 CMDI-BACKTICK is a lexical hit on Markdown-quoted parameter names in a // comment; the MainWindow.cpp and docs/user/screens.json findings are in files this PR does not touch (RFC 5737 documentation addresses and screenshot alt-text); the homoglyph hits are ±, Röbel and em-dashes in comments. No shell, SQL or path sink exists in the new code. Credited to security-audit (CodeGuard), which returned no confirmed findings.
On green CI: build, check-macos, check-windows, Static checks and the sanitizer configure all pass on 8741267. But every ctest step in ci.yml is -R-filtered (lines 819-820, 832, 1174, 1189) and neither fft_trace_smoothing_test nor fft_trace_decomb_test is in any of those filters — green proves they compile and link on all three platforms, not that they pass. I could not run them.
Everything above is reasoned from the head checkout at 8741267 plus arithmetic; I am headless, so nothing here was reproduced at runtime, and §4.1 in particular is a model of the gate formula rather than a run of the real envelope.
7. Recommendation
Needs maintainer decision — on the new public surface, not on correctness. The code is genuinely careful: the planner-lock discipline, the no-prefix-sum justification, the linear-power domain, the honest "rounds steep edges by about half this width" in the tooltip, and two tests that would actually catch regressions are all better than this change needed to be. I found no blocker.
What a maintainer should rule on: (a) the two new bridge keys, the new settings document and two new panel rows arriving without an issue; (b) whether a third and fourth client-side shaping stage in the SpectrumWidget chain should wait on #5782, which explicitly asks for a ruling on the stages already there.
Concrete next step for @ten9876, independent of that ruling: fix §4.1 (make the test's floor region noise-only and re-tune or re-reference the 8/14 dB ramp — right now the ramp's own justification cites the median while the code gates on estimateNoiseFloorDbm), decide §4.2, and soften the "noise floor stays put" wording. Thanks for the unusually well-documented commit message — the alpha=3 / 12-pass justification and the 5-smooth-length note made this much faster to review.
🤖 aethersdr-agent · cost: $16.5949 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (103 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: $16.7845 · model: claude-opus-5 |
|
Antares Detector — candidate vulnerable file(s)
The SpectralEnvelope implementation creates FFTW plans with sizes derived from the input size via std::size_t conversion (lines 94-99). This can silently truncate or wrap values, potentially allocating insufficient memory if the input size is not an exact power of two. The fftw_plan_r2r_1d call then uses these buffers, which could lead to buffer-overflow or memory-safety issues. 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: $16.7845 · model: claude-opus-5 |
…n magnifies the shaped trace. Principle VIII. De-comb's envelope rides peaks, and keyed on its own level against a floor estimate that reads ~6 dB under the noise median, it rode the noise peaks too: over plain unaveraged noise it lifted the median floor 4.8 dB on average (5.7 worst of 50 seeds). The input is now clipped from below at 12 dB over the trace's 20th percentile before enveloping -- unaveraged noise crosses that ~0.1% of the time, and the percentile holds while a signal fills up to ~80% of the span -- and the envelope blends in only where it rises 1-6 dB clear of the clip. Over noise the median now moves +0.10 dB (worst +0.28); voices ~20 dB over the noise still fill to -0.76 dB of their outline. Non-finite (no-data) points are left as they are rather than painted with envelope. Both controls now shape the source bins before the resample, in one shapeFftTrace(): whether De-comb runs is decided by the source resolution (the width must span two bins), so the GPU path, the oversampled CPU path and the device pixel ratio cannot disagree, and the UI thread shapes srcCount points instead of up to 4x as many. The mini-pan -- the main pan's magnifier -- shapes its source frame the same way before reslicing, so it no longer draws the raw comb under a de-combed main trace. The FFTW plan cache keeps the four most recent lengths, evicting older plans under the lock it already holds, so a drag-resize no longer accumulates plans. The setters skip unchanged values, so a slider drag or Clone no longer rewrites the settings object for nothing, and both labels read back the clamped slider value through one helper. The FFT Smooth tooltip and docs now say what the linear-power mean does to the floor: it reads its true average power, ~1-1.5 dB over the unsmoothed middle. fft_trace_decomb_test adds 50-seed pure noise (median and per-point change), a weak ~20 dB voice, preserved non-finite points, the source-bin eligibility boundary, and the plan cache across many lengths. With the clip disabled the noise checks fail at +6.4 dB. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
|
Remediation pushed in Review nits (review 5481750048)
Automated
Accessibility bot (6 warnings): none are in this diff. They flag Verification at
|
… and re-take the Stable snapshot. Principle VIII. (#6396) ## Summary This brings the user guide (docs.aethersdr.com) up to date with everything in v26.10.2 and re-takes the Stable snapshot so the default site shows it. Docs only, apart from the TCI page prose in `tools/docs/gen_reference.py`; no application source, test or CI changes. Every page was audited against the 106 PRs in the release. Every fact that was added or kept was checked against the source at `f753dbfc4`, the v26.10.2 release commit, not taken from the PR bodies alone. ## What changed - **Menu paths.** - AetherModem, Callsign Lookup, AetherMap and FreeDV Reporter are on **Tools**. They are created on View and then moved to Tools (`MainWindow_Menus.cpp:1519-1520`, `:1569-1575`); the style guide's example says **Tools → AetherMap...**. - **Show live CW contacts** is on **View**. - Waveforms is on **Tools**, not File. - The VUDU applet button was called TXDSP. - Labels now match the source's `...` vs `…`. - **Gaps from this cycle.** - **Windows and Network Diagnostics:** Windows Firewall rules and the Network Diagnostics Windows Firewall page (#6289); the `aether.network` and `aether.nvafx` log categories (#6289, #6308). - **Remote access:** Tailscale shim revocation semantics (#6325). - **Sleep, App Nap and power throttling (#6195):** per-OS notes. - **Hermes-Lite 2:** state restore, RX audio at unity, CW at TUNE level, multi-interface discovery and a discovery troubleshooting entry (#6259, #6254, #5975, #6009, #6293). - **ANAN-G2:** AGC-T range, a "What is remembered" section, and supply voltage and link status (#6282, #6129, #6137, #6211). - **NB2** on the HL2 and ANAN-G2 (#5966). - **DVK:** WAV import requirements, the slot menu without Delete, status text and licensing (#6247, #6262, #6251). - **Copy Assist** from unprocessed audio (#6216). - **Live CW contacts** replace the decoder's contact card (#6268). - **Status-bar indicators** work from the keyboard (#6267). - **Volume:** controllers, hotkeys and TCI `volume` follow the active output (#6129, #6260). - **Peripherals:** the KPA1500 (#5905), CTR2 link extensions and the AetherKnob (#6338), TMate 2 udev (#6250), MIDI RADE Modem and FDV modes (#6031, #6038). - **RTL-SDR:** live PPM (#6082). - **Icom:** COMP and the MK2 squelch scale (#6174, #6184). - **Known issues.** Entries whose GitHub issues are closed are removed (each checked with `gh issue view`). As decided, this includes three closed today without a demonstrated fix: #5107, #4824 and #5665. - **US spelling.** `docs-style-guide.md` asked for British spelling; it now asks for US, by the maintainer's decision, and every page is converted. The generated TCI pages are converted at their source, the prose strings in `tools/docs/gen_reference.py`. Code spans, links and URLs were left alone. UI labels were checked against the source strings, and none of them are British. Two in-page anchors and two Log Analyzer rule anchors (`#bnr-is-grayed-out-…`) follow the renamed headings. - **Stable snapshot.** `snapshot_stable.py 26.10.2` was re-run on the release tree with these corrections, before `main` was merged in, so Stable carries no post-release docs. `docs/` also has `main`'s post-release pages, in US spelling. Stable alone keeps the IC-9700 waterfall-rate entry (#5996): #6402 fixed it after the 26.10.2 tag, so 26.10.2 still has it. The PDF manual already attached to the v26.10.2 release keeps the tag's docs. ## Validation - `npm ci && npm run build` in `docs/user`: success. This is the broken-link and broken-anchor check. - `node tools/docs/test_log_rules.mjs`: 707 checks passed for 45 rules. - `python3 tools/docs/gen_reference.py --check`: up to date (8 pages). - `git diff --check`: clean. - Menu paths on a build of this branch (offscreen, isolated settings, no radio), `menu list`: `Tools → AetherModem...`, `Tools → Callsign Lookup...`, `Tools → AetherMap...`, `Tools → FreeDV Reporter...`, `Tools → Waveforms...`, `View → Show live CW contacts`. ## Screenshots now out of date (not recaptured) These need a recapture pass on the radio: - **Old panadapter title strip (#6386):** `panadapter`, `main-window`, `first-session-*`, `multi-slice-two-slices`, `split-pair`, `tnf-markers`, `offscreen-vfo-indicator`, `pan-3d-stacked-trace`, `default-light-theme`. - **Pre-canon windows (#6334, #6339, #6341, #6372):** `aetherrx*`, `aethertx*`, `aethermodem-aprs`, `bnr-panel`, `connect-dialog`, `connect-by-ip-card`, `connect-icom-rows`, `connect-smartlink-card`, `rx-agc-calibration`. - **Other:** - `title-bar` (lowercase tab states, #6281); - `menu-help` (#6294); - `display-panel` (#6371); - `support-diagnostics-dialog`; - `waveforms-dialog` and `tailnet-remote-access` (show shim 0.4.0); - `about-window` (v26.10.1); - `radio-setup-qrz`. ## Found during the audit, not fixed here - **App string bug:** `TailnetShimDialog.cpp:628` tells the user to "Connect remotely with Connect → Manual"; the real path is **File → Connect to Radio... → Connect by IP**. - **ANAN AGC-T tooltips:** the RX applet and VFO tooltips show the slider position as dB, which is wrong on ANAN. #6282 lists this as a follow-up. - **MIDI Master Volume on non-Flex radios:** it is refused while PC Audio is off, whereas hotkeys, FlexControl and HID drive the line out. The docs describe the code as it is. - **RTL mode names:** `rtl-sdr.md` writes "FM-N" and "CW-R", but the source strings are `FMN` and `CWR`. Left as is until someone confirms what the mode menu displays. - **IC-705 COMP readback:** inferred from the MK2 (#6174) and not verified on an IC-705. - **Not yet documented:** the #6230 Radio Health row "Forward responses skipped", and the Store/MSIX package. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
Adds two sliders to Display → PANADAPTER, below FFT FPS. Both are off by default and change only the spectrum trace: the main pan's and the mini-pan's, which magnifies it. The waterfall, the noise-floor readout, FFT Floor Auto and auto-squelch keep the raw bins. Both run in the client, so FLEX, HL2, ANAN, RTL and Kiwi behave the same.
FFT Smooth (0–16 px, 0 = off). This is the panadapter version of the Smoothing setting in the AetherRX/AetherTX EQ analyzer. Each point becomes the mean of its neighbors, averaged in linear power and converted back to dB, so a dB average doesn't drag the noise floor down. The smoothed floor reads its true average power, about 1–1.5 dB above the middle of the unsmoothed noise; the tooltip and docs say so. The EQ's window is a fraction of an octave because its axis is logarithmic. The panadapter's axis is linear, so here the window is set in the main trace's logical pixels.
FFT De-comb (0–600 Hz, 0 = off). Zoomed in to about 15 Hz per point, an SSB voice shows up as a comb of its pitch harmonics. I measured a 40 m screenshot: three stations combed at about 105, 180 and 180 Hz, each matching that speaker's own pitch. So the comb is real signal, not a receiver artifact. A mean wide enough to hide it drops the peaks by several dB. De-comb instead replaces the trace with its cepstral true envelope (Röbel & Rodet,
core/dsp/SpectralEnvelope), a smooth curve over the harmonic peaks that keeps the signal's outline and peak level:try_lockon the FFTW planner lock (Two independent mutexes guard one process-global FFTW planner: SpectralNR never takes WdspChannel::fftwSetupLock() #5895), so the UI thread never waits on another thread's planning. At an uncached length, frames draw without De-comb until the lock is free, and each frame retries.Both shape the source bins before the trace is resampled for drawing (De-comb, then Smooth), after the existing plateau-gated 5-tap blend. At 0 the trace is exactly what it is today.
Docs: added rows to
docs/user/docs/panadapter-controls.mdand updated theget displayexample indocs/automation-bridge.md.Known, unchanged diagnostic: the bridge's
rendered.floorDbmis measured from the drawn trace on the GPU path and from the raw bins on the CPU path. That predates this PR. With either slider on, the GPU figure reports the floor of the shaped trace.Constitution principle honored
DisplayFftTraceShaping = {version, smoothPx, decombHz}, written in a single save and skipped when unchanged. No flat keys were added.Test plan
fft_trace_smoothing_test: off is bit-identical; the mean is in linear power; an impulse spreads to 2N+1 points; the window clamps at the edges; a floor point beside a +10 dBm carrier stays exact.fft_trace_decomb_test:spectrum_overlay_fft_fps_bounds_test,waterfall_manual_black_test,waterfall_impulse_blanker_test,mini_pan_widget_test.ui-supporttouchpoint row (core/dsp/SpectralEnvelope.h, one includer), which the maintainer approved.fftSmoothing/fftDecombHzand persists as oneDisplayFftTraceShapingobject.Checklist
AppSettingscalls; one nested object per pan (Principle V)MeterSmoother(no meter UI touched)🤖 Generated with Claude Code