Add configurable analog S-meter themes and improve SWR meter accuracy#4274
Add configurable analog S-meter themes and improve SWR meter accuracy#4274rfoust wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes the analog meter UI more data-driven and configurable: the standard S-meter gains a versioned geometry resource and selectable face themes (defaulting to the existing Aether face), while the cross-needle PWR/SWR meter’s response/contour construction and label placement are refined. It also improves handling of radio-native SWR at low power via a time-aware validity filter, and extends deterministic tests + tooling to guard the new geometry/math.
Changes:
- Add shared, data-driven analog face themes and make the standard S-meter’s face geometry/theme configurable (with Aether default preserved).
- Refine cross-needle SWR contour generation and font-aware label placement; add persistent “Show Range” and settings migrations.
- Add a radio-native SWR validity filter plus expanded tests and diagnostics tooling.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/fit_cross_needle_response.py | Adds a diagnostics/check tool for the cross-needle response fit and SWR contour constraints. |
| tests/vu_meter_settings_test.cpp | Expands settings tests for new S-meter faceTheme and PWR “Show Range” persistence/migration. |
| tests/meter_model_test.cpp | Adds coverage ensuring radio-native SWR samples remain authoritative at low power. |
| tests/s_meter_geometry_test.cpp | Adds deterministic rendering/validation tests for the standard S-meter geometry/themes. |
| src/gui/VuMeterSettings.h | Bumps VuMeter settings version and introduces standard S-meter face theme IDs/API. |
| src/gui/VuMeterSettings.cpp | Implements face theme normalization, encoding/decoding, and version migration support. |
| src/gui/SMeterWidgetAccessible.h | Introduces an accessibility adapter/factory for the custom-painted S-meter. |
| src/gui/SMeterWidget.h | Adds geometry/theme plumbing, floating behavior, and radio-native SWR update path. |
| src/gui/SMeterWidget.cpp | Implements geometry/theme loading, background caching, accessibility announcements, and SWR filtering integration. |
| src/gui/SMeterGeometry.h | Adds versioned, validated, responsive S-meter geometry model and mapping helpers. |
| src/gui/SMeterGeometry.cpp | Implements JSON loading/validation, derived layout, and ray/intersection helpers for S-meter drawing. |
| src/gui/RadioSwrValidityFilter.h | Defines a time-aware radio-native SWR validity filter driven by instantaneous forward power. |
| src/gui/RadioSwrValidityFilter.cpp | Implements envelope/hold logic to reject low-power/reset artifacts while adapting to sustained low power. |
| src/gui/MainWindow_Wiring.cpp | Routes radio TX meter updates to the standard meter with instantaneous forward power for SWR validation. |
| src/gui/CrossNeedleMeterWidget.h | Adds shared analog theme catalog usage and a toggle for the Range legend visibility. |
| src/gui/CrossNeedleMeterWidget.cpp | Switches to shared analog materials/palettes, trims SWR contour rendering, and uses font-aware derived label centers. |
| src/gui/CrossNeedleMeterSettings.h | Bumps settings version and adds persistent showRange. |
| src/gui/CrossNeedleMeterSettings.cpp | Implements versioned decode/encode with migration for showRange. |
| src/gui/CrossNeedleMeterGeometry.h | Updates the geometry model to use Bernstein response + derived SWR guide/label behavior; adds new helpers. |
| src/gui/CrossNeedleMeterGeometry.cpp | Implements the new response model, SWR contour sampling/termination, inverse mapping, and derived label decluttering. |
| src/gui/CrossNeedleMeterApplet.h | Adds applet-level state for range legend visibility and persistence. |
| src/gui/CrossNeedleMeterApplet.cpp | Implements settings rewrite-on-load for migrations and adds context-menu “Show Range”. |
| src/gui/AppletPanel.h | Adds storage for standard meter face theme and a new radio-native TX meter setter. |
| src/gui/AppletPanel.cpp | Migrates VuMeter settings, wires dock/floating behavior, persists faceTheme, and adds theme selection to the context menu. |
| src/gui/AnalogMeterFaceTheme.h | Introduces shared analog face theme catalog (gradients, palettes, and lower-mask geometry). |
| src/gui/AnalogMeterFaceTheme.cpp | Implements JSON loading/validation, deterministic paper grain, background painting, and mask geometry mapping. |
| src/core/AutomationServer.cpp | Surfaces additional analog-meter dynamic properties for automation-bridge validation. |
| resources/resources.qrc | Adds the new meterface/theme JSON resources to the Qt resource bundle. |
| resources/meterfaces/s-meter-v1.json | Adds the versioned standard S-meter geometry resource. |
| resources/meterfaces/README.md | Documents the new S-meter geometry resource and shared physical theme catalog, plus updated cross-needle guidance. |
| resources/meterfaces/cross-needle-v12.json | Updates cross-needle geometry schema/version and SWR guide config to the new semantic construction. |
| resources/meterfaces/analog-meter-themes-v1.json | Adds the shared physical theme/material definitions and palettes. |
| docs/cross-needle-meter-math.md | Adds an authoritative model/decision record for cross-needle meter math and constraints. |
| CMakeLists.txt | Registers new GUI sources and adds the new s_meter_geometry_test executable/test. |
| AGENTS.md | Adds project guidance pointing contributors to the cross-needle math decision record. |
| if (mask.boundary.size() < 3) { | ||
| return fail(QStringLiteral("cross-needle lower mask is invalid")); | ||
| } |
| void CrossNeedleMeterApplet::setRangeLegendVisible(bool visible, bool persist) | ||
| { | ||
| m_rangeLegendVisible = visible; | ||
| m_meter->setRangeLegendVisible(visible); | ||
| setProperty("crossNeedleRangeLegendVisible", visible); | ||
| if (persist) { | ||
| persistSettings(); | ||
| } | ||
| } |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thorough, well-structured work — the JSON-as-data / code-as-math / tests-as-contract split is documented, the settings changes correctly follow the versioned-nested-JSON-under-one-key pattern (CrossNeedle v1→2, VuMeter v2→3, both with proper version >= decode guards and migration), and the resource-load path is defensive (constructor seeds fallback(), keeps it on any invalid/failed load, warns). Radio-authority for native SWR (Principle II) is preserved. Nothing here blocks merge.
One note on scope: the FramelessMessageBox changes that show up when diffing against current main are not part of this PR — the branch is based on b9735c79, which predates #4245, so that delta is a rebase artifact and will resolve cleanly on merge. No action needed.
Non-blocking notes:
RadioSwrValidityFilter: therawSwr < 1.0 → peg to full-scale SWRbranch has no confirmation window and would slam the meter to max on a single noisy sub-1.0 sample — worth confirming against the real native SWR stream (you already flagged the filter as hardware-unverified).
Polish (both align with Copilot, with caveats):
CrossNeedleMeterGeometry::isValid()accepts any mask with ≥3 points;maskBoundaryY()assumes ordered/in-bounds x. Low risk given the compiled resource + fallback, but tightening the check would make fallback trigger on a malformed mask instead of mis-placing labels.setRangeLegendVisible()re-applies unconditionally — fine as-is; see the inline note on why a naive early-return would actually break first-load property init.
Thanks for the careful test coverage and the decision-record doc.
🤖 aethersdr-agent · cost: $9.3716 · model: claude-opus-4-8
| } else if (rawSwr < 1.0f) { | ||
| m_displayedSwr = maximumDisplayedSwr; |
There was a problem hiding this comment.
This branch pegs the display to full-scale SWR whenever the radio reports rawSwr < 1.0 while meaningful power is present. SWR < 1 is physically impossible, so treating it as an over-range sentinel is reasonable — but unlike the low-SWR recovery path just below (which requires kLowSwrConfirmationSamples before trusting a drop), this raises to max on the first sub-1.0 sample with no confirmation. If the native stream ever emits a transient sub-1.0 value as measurement noise near a good 1.0 match, the meter would flash to full-scale SWR — a scary false alarm.
Since you've flagged the filter as not yet exercised on real hardware, this is the specific edge to watch: confirm the radio only sends <1.0 as a genuine over-range/unavailable sentinel, never as noise around a matched load.
| !(swrStyle.maskGap >= 0.0) || !std::isfinite(swrStyle.maskGap)) { | ||
| return fail(QStringLiteral("cross-needle SWR construction settings are invalid")); | ||
| } | ||
| if (mask.boundary.size() < 3) { |
There was a problem hiding this comment.
Confirming Copilot's finding — valid but low severity. maskBoundaryY() (line ~1030) interpolates assuming mask.boundary is ordered left→right, but isValid() only checks size() < 3. A malformed mask (unsorted or out-of-bounds x) would pass validation and silently mis-place SWR labels rather than being rejected to fallback(). It won't crash (maskBoundaryY guards the equal-x divide-by-zero), so with a compiled resource + full fallback the practical risk is small. If you want to close it, require finite, strictly-increasing x within [0, canvasWidth] here (and per your own README, symmetry around center). Non-blocking.
| } | ||
| } | ||
|
|
||
| void CrossNeedleMeterApplet::setRangeLegendVisible(bool visible, bool persist) |
There was a problem hiding this comment.
Re Copilot's early-return suggestion: note that setFaceTheme() doesn't guard either — both re-apply unconditionally, and during load persist is false, so there's no AppSettings churn (only a redundant repaint on no-op calls). Fine to leave as-is.
If you do add a guard, don't make it a naive if (m_rangeLegendVisible == visible) return; at the top: m_rangeLegendVisible defaults to true and loadSettings() calls setRangeLegendVisible(settings.showRange, false), so when the persisted value is also true the guard would skip the initial m_meter->setRangeLegendVisible(...) and setProperty("crossNeedleRangeLegendVisible", ...) sync, leaving the widget/automation property unset on first load. Any guard needs to still run that sync once.
Another set of eyes — live-radio check on a FLEX-6700Took you up on the offer to test this against real hardware. Short version: it does what
That one's covered now — the filter behaves exactly as you described it. Environment
Worth saying up front: the dynamic properties you added to 1. The native-radio SWR validity filter works ✅Two conditions on 28.180 CW:
The first row is the money shot: with essentially no RF going out, the radio still coughed The adaptive floor works too:
Also confirmed in passing: 2. Bonus data point — the 6700's two SWR sources disagree with each otherThis is not a defect in this PR and nothing here needs fixing before merge — but it The S-meter shows the radio's native SWR meter; the PWR cross-needle computes SWR from
The error tracks frequency, not mismatch magnitude — 28.180 at SWR 1.87 is +10%, while To be clear about where this is coming from: it isn't your code. I went looking for a
So both numbers are decoded faithfully and the radio's own meters simply disagree in steady And I can't tell you which one is right. Both readings come from inside the radio, so If anything, this is an argument for the call you already made:
That reads like the right instinct, and now there's hardware data behind it. 3. On "feel free to fix it" — there's nothing here to fix in your codeAppreciated, but I don't think a patch is warranted, and I'm not going to invent one. The only lever that exists is a policy one, and it's already written: the widget has both That's a design trade-off with real arguments on both sides, so it's a maintainer call, not If settling which SWR figure is actually correct would be useful to anyone, I have a KAT500 Nice work — the filter design is sound and the instrumentation made it verifiable. 73, Ozy K6OZY |
NF0T
left a comment
NF0T
left a comment
There was a problem hiding this comment.
Thorough, well-documented work — the shared analog theme catalog tying the S-meter and cross-needle meter together is a good architectural call, the settings migrations (VuMeter v2->v3, CrossNeedle v1->v2) round-trip correctly, and docs/cross-needle-meter-math.md is a genuine decision record, not boilerplate. CI is green and the commit is signed. Already covered well by Copilot's and the earlier bot review's notes on the mask-ordering validation and the SWR-peg-on-first-sample issue -- this adds two things underneath those.
1. A genuinely-improving SWR reading during a power dip gets stuck showing the stale, worse value. In RadioSwrValidityFilter::update()'s !hasMeaningfulPower branch, the display can only ever be raised, never lowered, while forward power sits in the "twilight zone" between the absolute floor and the relative envelope threshold. Concretely: displayed SWR is latched at 5.0 from an earlier fault; an ATU re-match or ALC pump briefly drops power into that zone while the raw SWR genuinely improves to 2.0 -- since 2.0 < 5.0, the code holds at 5.0 instead of updating, and given the 750ms envelope half-life this can persist for several real seconds. Distinct from the already-flagged peg-to-max issue -- this is the recovery side failing instead.
2. SMeterGeometry::isValid()'s self-check doesn't cover the aspect ratios the widget's own resize policy allows. Worked the layout math directly: the four validation sample sizes all have comfortable margins, but SMeterWidget sets QSizePolicy::Expanding on both axes for a floating/detached meter with only a 200x100 floor -- no max, no aspect lock. At a reachable size like 200x2000 (a detached meter dragged into a narrow sidebar), pivotDistance comes out to ~1136 against a smallestMovementRadius of 164 -- violating the exact invariant isValid() exists to guarantee, by roughly 7x, at a size none of the four samples come close to exercising. At runtime this silently drops inside-tick draws or places them on the wrong side of the arc. Worth either widening the validation sample sizes to match the real resize range, or clamping the widget's aspect ratio so this invariant can't actually be violated.
Worth a discussion, not necessarily blocking: RadioSwrValidityFilter's forward-power envelope (attack-immediately/release-with-half-life) is structurally an envelope-follower, and AGENTS.md says "don't write new envelope-follower code... use MeterSmoother instead" -- though this one drives a gating decision (is there enough power to trust this sample), not the displayed number directly, which may be a different category than the rule targets. The PR's own checklist checks off full MeterSmoother compliance, so worth confirming this was a deliberate call.
Non-blocking cleanup:
lowerMaskBoundary()is recomputed independently by bothCrossNeedleMeterWidgetand the newSMeterWidgeton every repaint instead of cached/shared once.SMeterGeometry::movementRayCircleIntersection()re-derives the fullLayout2-3x per frame across ticks/markers, unlikeCrossNeedleMeterGeometry's precompute-once approach.- Both settings loaders re-parse the raw JSON a second time just to read the version field
decode()already parsed internally. - Three independent hand-rolled JSON parsers now exist across
CrossNeedleMeterGeometry.cpp/SMeterGeometry.cpp/AnalogMeterFaceTheme.cpp-- worth consolidating into the last one, since it was created in this PR as the intended shared file. CrossNeedleMeterGeometry::isValid()still doesn't verifymask.boundaryis x-sorted, and this PR makes that ordering load-bearing for the newmaskBoundaryY()-based label placement -- a bit more consequential now than when Copilot/the bot first flagged it.- No
Fixes #NNNN-- this bundles three distinct concerns (new S-meter system, cross-needle SWR-contour refinement, new validity filter) under one untracked PR.
Happy to re-review once 1 and 2 are addressed -- this is close, and the shared-theme architecture is the right direction.
Summary
Adds data-driven geometry and configurable analog face themes to the standard S-meter while retaining the existing AetherSDR face as the default. The S-meter now scales consistently in docked and detached windows, shares the Classic Warm, Dark-Room Uplight, and Graphite Dark themes used by the PWR/SWR meter, and preserves calibrated RX, TX, peak-hold, and native radio-SWR behavior. This also refines the cross-needle meter's SWR contours and font-aware label placement, adds the persistent Show Range option, and replaces the S-meter's lifetime-peak SWR gate with a time-aware validity filter that rejects brief low-power/reset artifacts while adapting to sustained lower-power operation.
Constitution principle honored
Principle II — the standard S-meter continues to treat the radio's native SWR meter as authoritative rather than substituting the PWR applet's reflected-power calculation.
Principle V — meter theme and display preferences are stored as versioned, feature-owned configuration objects.
Principles VIII and XI — the meter geometry, calibration, responsive layout, SWR filtering, and rendered-font label placement are covered by deterministic tests and automation-bridge render validation.
Test plan
cmake --build build)Additional validation:
git diff --checkpassedChecklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key(Principle V)
reverse-engineered from a proprietary binary (Principle IV)
MeterSmoother(AGENTS.md convention)