Sync WNB panadapter status from radio#2860
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines WNB (Wideband Noise Blanker) state synchronization in the panadapter so that the radio's reported state is the single source of truth. PanadapterModel now parses wnb, wnb_level, and the new FlexLib wnb_updating flag independently, emitting a new wnbStateChanged(active, level, updating) signal. SpectrumWidget and SpectrumOverlayMenu gain syncWnbState entry points; the spectrum WNB label is rendered dim while the radio reports normalization in progress and full-brightness once cleared. The optimistic click-time call to setWnbActive is removed so the UI follows the radio echo.
Changes:
- Independent parsing of
wnb,wnb_level,wnb_updatingplus a newwnbStateChangedsignal inPanadapterModel. SpectrumWidgetandSpectrumOverlayMenuexposesyncWnbState; spectrum indicator strip is refactored to per-segment drawing so the WNB segment can use a dim color while updating.MainWindowwireswnbStateChangedto both the spectrum widget and overlay menu, and drops the optimisticsetWnbActive(on)on the WNB toggle handler.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/models/PanadapterModel.h | Adds wnbUpdating() getter, wnbStateChanged signal, and m_wnbUpdating member. |
| src/models/PanadapterModel.cpp | Splits parsing of wnb / wnb_level / wnb_updating; clamps level; emits both wnbChanged and wnbStateChanged. |
| src/gui/SpectrumWidget.h | Adds wnbUpdating(), syncWnbState(), redirects setWnbActive() to it, and tracks m_wnbUpdating / last-detect counterpart. |
| src/gui/SpectrumWidget.cpp | Refactors GPU and CPU paint paths to draw indicator segments individually so WNB can be drawn dim while m_wnbUpdating is true; tracks leftEdge for hit-test rect. |
| src/gui/SpectrumOverlayMenu.h | Declares syncWnbState(bool, int, bool). |
| src/gui/SpectrumOverlayMenu.cpp | Implements syncWnbState; existing setWnbState now delegates to it (level still applied; updating currently unused in menu). |
| src/gui/MainWindow.cpp | Connects PanadapterModel::wnbStateChanged to both the spectrum widget and overlay menu in both panadapter wiring paths; removes optimistic sw->setWnbActive(on) from the WNB button handler. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Thanks @rfoust — this is a nice refactor and the radio-authoritative pattern is the right move. The split of wnb / wnb_level / wnb_updating parsing in PanadapterModel::applyPanStatus is cleaner than the old combined branch, and the segment-drawing lambdas in SpectrumWidget are a noticeable readability win.
A few small observations — none are blocking:
1. Lingering setWnbActive() callers reset m_wnbUpdating to false
Two callers remain after this PR:
MainWindow.cpp:2593(settings restore on radio connect)MainWindow.cpp:13160(snapshot restore)
Both now route through syncWnbState(on, 0, false), so they will overwrite m_wnbUpdating to false even if the radio is mid-normalization. In practice the next radio echo will correct it within a frame or two, so this is almost certainly fine — but worth a mental note that the "updating" indicator can briefly flicker out at startup/recall before the radio's status reasserts it.
2. SpectrumWidget::syncWnbState(bool on, int level, bool updating) ignores level
The widget never uses the level value (Q_UNUSED(level)), only the overlay menu does. Slightly noisy API but consistent across the three sync points, so I won't push back on it.
3. wnbChanged(active, level) semantics widened
Previously emitted only when kvs.contains("wnb") was true; now emits when any of wnb / wnb_level / wnb_updating changes. This is the correct fix (the signal name implies "WNB state changed"), but any listener that implicitly assumed the active flag was the trigger should be re-confirmed. A quick grep of wnbChanged connections looked safe to me.
4. WNB click no longer optimistically lights the label
MainWindow.cpp:11190 correctly drops the local sw->setWnbActive(on) and lets the radio echo drive the indicator. Good — this matches the design described in the PR body. The same removal could conceptually apply to the overlay menu button checked-state during the click, but syncWnbState will catch up on the echo and the QSignalBlocker guards make this safe.
Input clamping on wnb_level (0–100) and the Qt::UniqueConnection usage are both good defensive touches.
LGTM overall.
a11c325 to
7075fde
Compare
jensenpat
left a comment
There was a problem hiding this comment.
Thanks Robbie for full WNB status support
Summary
wnb_levelupdates independently fromwnbinPanadapterModel.wnb_updatingpanadapter status and expose it to spectrum widgets.Verification
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfocmake --build build -j8Note: I did not launch the app.