fix(spectrum): use the radio's auto-black level for an evenly-levelled waterfall floor#3586
fix(spectrum): use the radio's auto-black level for an evenly-levelled waterfall floor#3586sblanchard wants to merge 2 commits into
Conversation
…d waterfall floor
AetherSDR computed its own client-side noise-floor estimate and disabled the
radio's auto-black (auto_black=0), giving a textured, black-speckled waterfall
floor instead of the even floor the radio's auto-levelling produces.
The radio computes a noise-floor black level and embeds it in every waterfall
tile (FlexLib's WaterfallTile.AutoBlackLevel). AetherSDR already parsed it and
emitted waterfallAutoBlackLevel(), but discarded the value (clamping the raw
uint16 to 0-125).
Wire it up:
• RadioModel sends auto_black=1 so the radio populates the tile field.
• waterfallAutoBlackLevel feeds setRadioAutoBlackLevel() into the renderer.
• intensityToRgb maps low = radio auto-black, high = wfHighThresholdRaw()
(a cubic colour-gain curve), t = (intensity - low) / (high - low).
• The client-side estimate stays as a fallback when no radio value is
available (FFT fallback, or auto-black off).
Verified on a live FLEX-8600 (fw 4.2.20.41343).
1b19c89 to
fdf9c0b
Compare
There was a problem hiding this comment.
Thanks @sblanchard — this is a well-reasoned change with a clear constitution writeup. Wiring up the radio's per-tile AutoBlackLevel instead of clamping it to 0–125 and throwing it away is the right call (Principle I), and the implementation is clean: the new m_radioAutoBlackRaw member is kept distinct from m_wfBlackLevel, the renderer path is null-guarded (if (auto* sw = m_panStack->spectrum(...))), the per-tile update() is gated behind a change check, and the client estimate is correctly preserved as a fallback. CI is green across all six checks (build/mac/win/CodeQL/cpp/a11y).
A couple of substantive points, plus confirmation of two open questions you already flagged:
1. lowRaw can go negative; black point and range then derive from different lows. In the radio-authoritative branch:
const float lowRaw = m_radioAutoBlackRaw + (50 - m_wfAutoBlackOffset) * 0.5f * 128.0f; // ±3200 raw at offset 0/100
const float highRaw = wfHighThresholdRaw(lowRaw, m_wfColorGain); // internally qBound(0, lowRaw, 65535)
blackThresh = lowRaw / 128.0f; // uses the *unbounded* lowRaw
rangeWidth = std::max(1.0f, (highRaw - lowRaw) / 128.0f); // also unbounded lowRawWith a low radio level and offset pushed toward 100, lowRaw goes negative. wfHighThresholdRaw clamps it to 0 for the white-point math, but blackThresh and rangeWidth use the raw negative value — so the black point and the range width are computed against two different low points. No crash, but the mapping is internally inconsistent at the extremes. Suggest clamping once up front (lowRaw = qBound(0.0f, lowRaw, 65535.0f);) and feeding the clamped value to all three.
2. Confirming your open question on unconditional auto_black=1. configureWaterfall() now enables radio auto-black at session start regardless of the persisted client auto-black state. If the user has client auto-black off, the radio is in auto_black=1 mode embedding a level the renderer ignores (it takes the !m_wfAutoBlack fallback). It's harmless since the native path colours client-side, but it's a state mismatch — I'd reflect the persisted client state here as you suggested, so the radio and renderer agree.
3. Maintainer sign-off on the visual default. You've correctly flagged this (Principle XIII) — it reverses a deliberate client-side decision and changes the floor rendering for all colour schemes, not just Purple. That's a feel call I can't make from code review; leaving it for the maintainer to validate on hardware.
Conventions all check out — no QSettings/flat-key AppSettings added, RAII intact, no meter UI touched. Nice work.
🤖 aethersdr-agent · cost: $2.3484 · model: claude-opus-4-8
…d range Addresses review on aethersdr#3586: with a low radio auto-black level and the offset slider toward 100, lowRaw could go negative. wfHighThresholdRaw() clamped it internally for the white-point math, but blackThresh and rangeWidth used the unclamped value, so the black point and the range derived from two different low points. Clamp lowRaw once to [0, 65535] and feed the clamped value to all three.
|
Thanks for the thorough review! 1. 2. Unconditional 3. Visual default / maintainer sign-off. Agreed — flagged under Principle XIII; it's a hardware feel call. Suggested starting point is in the description: Color Gain ≈ 30, Auto Black ≈ 30, especially on the Purple scheme. |
Summary
Makes the waterfall noise floor render as a flat, evenly-levelled band by using the radio's own per-tile auto-black level instead of a client-side estimate.
AetherSDR was computing its own noise-floor estimate and sending the radio
auto_black=0, which produced a textured, black-speckled floor. The radio already computes a noise-floor black level and embeds it in every waterfall tile (FlexLib'sWaterfallTile.AutoBlackLevel) — AetherSDR even parsed it (PanadapterStream.cpp) and emittedwaterfallAutoBlackLevel(), but the value was discarded (the handler clamped the raw uint16 to 0–125).This wires it up:
RadioModelsendsauto_black=1so the radio populates the tile field.MainWindow_Sessionfeeds the per-tile value into the renderer via the newSpectrumWidget::setRadioAutoBlackLevel().intensityToRgbmapslow= radio auto-black,high=wfHighThresholdRaw()— a cubic colour-gain curve:t = (rawValue − low) / (high − low)→ gradient. The client-side estimate stays as a fallback (FFT fallback / auto-black off).Both display paths are covered — the native waterfall is coloured on the CPU via
intensityToRgb, then uploaded as a texture (no GPU-side mapping).Reviewer note
Try it with Waterfall (Color) Gain ≈ 30 and Auto Black Level ≈ 30 — it works very well, particularly with the new Purple colour scheme (#3583). At default Color Gain and Black Level offset = 50 (no bias), the floor follows the radio's computed level directly.
Constitution principle honored
Principle I — FlexLib Is The Protocol Authority. Replaces a client-side approximation with the radio's own auto-black value (FlexLib's per-tile
AutoBlackLevel), the authoritative noise-floor figure.It also reverses a deliberate client-side decision and changes a rendering default for all waterfall schemes, so it is flagged for maintainer sign-off on the look (Principle XIII — the operator/maintainer is the authority on visual design).
Test plan
cmake --build build) — links cleanOpen questions for review
(50 − offset)·0.5·128raw); scaling is open to tuning.auto_black=1unconditionally; could instead reflect the persisted client auto-black state.Checklist
AppSettingscalls (none added)MeterSmoother— N/A (no meters touched)