[codex] Fix profile FFT settling on load#3604
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate intermittent bogus FFT/waterfall rendering during startup and profile load by deferring spectrum drawing until panadapter dimensions/FFT scaling have stabilized, dropping stale/unmatched frames during rebuild, and allowing client-side noise-floor auto-adjust to reacquire without fighting radio-owned state.
Changes:
- Added a
RadioModelsignal for radio-reported FFT pixel height (y_pixels) and wired it intoMainWindow/SpectrumWidgetto coordinate decoder + smoothing resets. - Introduced profile-load “pan display settling” state in
MainWindowto defer drawing/restores, clear displays while topology settles, and drop unmatched stream frames. - Updated
SpectrumWidgetnoise-floor logic to support a fast-lock reacquire path (resumeNoiseFloorAutoAdjust, snap frames, and “consumed frame” tracking).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/models/RadioModel.h | Adds a new signal to publish radio-reported FFT pixel height per pan. |
| src/models/RadioModel.cpp | Emits FFT scale updates and avoids applying pixel updates to a fallback pan when IDs don’t match. |
| src/gui/SpectrumWidget.h | Adds noise-floor resume/query helpers and changes baseline updater to return “frame consumed”. |
| src/gui/SpectrumWidget.cpp | Implements fast-lock noise-floor reacquire and integrates “consumed frame” semantics into fresh-lock logic. |
| src/gui/MainWindow.h | Adds per-pan profile-load settling state tracking and extends pan-dimension push APIs for decoder timing. |
| src/gui/MainWindow_Wiring.cpp | Defers/flushes pan dimension pushes during profile load and coordinates decoder updates + noise floor hold/reacquire. |
| src/gui/MainWindow_Session.cpp | Drops stale/unmatched FFT/waterfall frames during profile rebuild and gates drawing while pan display is settling. |
There was a problem hiding this comment.
Thanks @rfoust — this is a well-structured fix for a genuinely fiddly problem (queued old-height FFT frames decoded at the new scale producing those brief high traces). The settling state machine, the panMatchedById guard, and the client-local auto-floor reacquire all read cleanly, and routing the local-decoder update through the radio echo (with a timed fallback) instead of the optimistic immediate write is a nice tightening. A few notes, none blocking:
On the Copilot comment (re: profileLoadFrameMatchesWidgetDimensions) — partially fair, partially not:
-
The naming concern is legitimate. The helper is named
…MatchesWidgetDimensionsbut only enforcesbinCount >= 128(pluspanPixelDimensionsReady); it never compares the frame's bin count againstpanXpixelsFor(sw). That's actually fine for the bug you're fixing — the visual regression is vertical (ypixels/scale), and the real ypixels alignment is enforced by the settling logic (m_profileLoadPendingFftYpixels+profileLoadPanDimensionsMatchExpected), while horizontal bin mismatch just reprojects. But the name overpromises. Consider renaming to something likeprofileLoadFrameLooksRenderableor adding a one-line comment that this is a coarse sanity floor, not a dimension match — otherwise a future reader will assume xpixels are validated here when they aren't. -
The "permanently drop valid frames for narrow pans" claim is a false positive.
profileLoadFrameMatchesWidgetDimensionsis only reached from insideprofileLoadFrameReady, which returnstrueat the top once!writesHeld && !panDisplaySettling. So the 128-floor only applies during the settle window, and on top of thatscheduleProfileLoadRecoveryclears both settling maps at 11250ms. At worst one frame is dropped on the release tick; the next frame draws. And a real panadapter pane is never < 128px wide in practice, so the floor never bites a legitimate pan. Not permanent.
One genuine question worth a look: RadioModel::handlePanadapterStatus now emits panadapterFftScaleChanged on every status carrying y_pixels (anything >= 2), and the new handler in wirePanLifecycle calls sw->prepareForFftScaleChange() unconditionally on each emit — which resets FFT smoothing, bumps the scale-settling window, and arms a fast lock. During profile load that's exactly right, but if the radio echoes an unchanged y_pixels in routine status updates, you'd be resetting smoothing and re-arming fast-lock on the steady-state display. markProfileLoadPanDimensionsReady early-returns when the pan isn't pending, so that side is safe — but prepareForFftScaleChange() isn't gated. Could you confirm the radio only sends y_pixels on actual dimension changes? If not, consider gating the prepareForFftScaleChange() call (or the emit) on the value actually differing from the last-known ypixels.
Everything else looks good — conventions are respected (no new flat-key AppSettings, QPointer guard on the deferred decoder lambda, stale-stream drops logged rather than silently swallowed). The live-radio verification in the test plan covers the path that matters most here. CI is still pending; the merge is blocked on that, not on anything above.
🤖 aethersdr-agent · cost: $3.5541 · model: claude-opus-4-8
feecf65 to
be39674
Compare
|
Pushed follow-up addressing the review notes:
Local validation re-run after the follow-up:
|
Summary
Fixes intermittent bogus FFT rendering during app startup and profile load. The panadapter display now waits until radio-reported FFT pixel height, local decoder scale, and widget dimensions are aligned before drawing restored-profile FFT/waterfall frames. It also drops stale/unmatched stream frames during profile rebuild, clears old display contents while the profile topology is settling, and lets client-side auto FFT leveling reacquire locally without writing radio-owned pan dBm state back to the radio.
Constitution principle honored
Principle XI — Fixes Are Demonstrated. This addresses a user-reported visual regression and was iterated against live radio behavior, with local build and focused regression checks run before PR.
Test plan
cmake --build build --parallel)Local checks run:
Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key(Principle V)
MeterSmoother(Principle II)