Skip to content

Add configurable analog S-meter themes and improve SWR meter accuracy#4274

Merged
ten9876 merged 2 commits into
aethersdr:mainfrom
rfoust:codex/s-meter-geometry
Jul 18, 2026
Merged

Add configurable analog S-meter themes and improve SWR meter accuracy#4274
ten9876 merged 2 commits into
aethersdr:mainfrom
rfoust:codex/s-meter-geometry

Conversation

@rfoust

@rfoust rfoust commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

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

  • Local build passes (cmake --build build)
  • Behavior verified on a real radio if applicable
  • Existing tests pass (CI)
  • Reproduction steps documented if user-reported bug

Additional validation:

  • Full local test suite: 113 passed, 1 intentionally skipped, 0 failed
  • Cross-needle response and SWR-contour fit check passed
  • Accessibility check passed with 0 findings
  • Strict engine-boundary check passed with 0 blockers
  • git diff --check passed
  • Automation bridge successfully rendered and inspected both the standard S-meter and PWR/SWR meter using an isolated no-autoconnect session
  • Final native-radio SWR validity-filter changes have not yet been exercised on a real radio

Checklist

  • Commits are signed (docs/COMMIT-SIGNING.md)
  • No new flat-key AppSettings calls — use nested-JSON-under-one-key
    (Principle V)
  • Code is clean-room — not decompiled, disassembled, or
    reverse-engineered from a proprietary binary (Principle IV)
  • All meter UI uses MeterSmoother (AGENTS.md convention)
  • Documentation updated if user-visible behavior changed
  • Security-sensitive changes reference a GHSA if applicable

@rfoust
rfoust requested review from a team as code owners July 16, 2026 20:12
Copilot AI review requested due to automatic review settings July 16, 2026 20:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/gui/CrossNeedleMeterGeometry.cpp Outdated
Comment thread src/gui/CrossNeedleMeterApplet.cpp

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: the rawSwr < 1.0 → peg to full-scale SWR branch 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

Comment thread src/gui/RadioSwrValidityFilter.cpp Outdated
Comment thread src/gui/CrossNeedleMeterGeometry.cpp Outdated
Comment thread src/gui/CrossNeedleMeterApplet.cpp
@Ozy311

Ozy311 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Another set of eyes — live-radio check on a FLEX-6700

Took you up on the offer to test this against real hardware. Short version: it does what
it says on the tin.
Details below, including the box your test plan left open:

"Final native-radio SWR validity-filter changes have not yet been exercised on a real radio."

That one's covered now — the filter behaves exactly as you described it.

Environment

Build PR head 2714cdab (codex/s-meter-geometry), dedicated worktree
Linux Nobara 43, Qt 6.10.3, gcc 15.2.1, ninja 1.13.1 — 616/616, 0 errors
macOS also built clean from the same head — 616/616, 0 errors
Radio FLEX-6700, firmware 4.2.20.41343 (API version V1.4.0.0 per TCP greeting)
Antenna Ant1 — AV640 vertical (real antenna, not a dummy load)
Method in-process Automation Bridge, AETHER_AUTOMATION=1, AETHER_AUTOMATION_ALLOW_TX=1
TX licensed operator authorized and supervised throughout; TUNE carriers, Tune power 40, RF power 50

Worth saying up front: the dynamic properties you added to describeWidget are what made
this measurable instead of eyeballable. I could read txSwrRaw vs txSwr and watch the
filter make its decision. That instrumentation earned its keep immediately.

1. The native-radio SWR validity filter works ✅

Two conditions on 28.180 CW:

Condition txSwrForwardWatts txSwrRaw txSwr (displayed) txSwrMinimumForwardWatts
PTT asserted, no carrier 0.0017 W 1.125 1.000 0.05
TUNE carrier 41.49 W 1.875 1.875 8.298

The first row is the money shot: with essentially no RF going out, the radio still coughed
up a raw SWR of 1.125 — exactly the meaningless low-power artifact this filter exists
to reject — and the filter rejected it, displaying 1.000. Reproduced on hardware.

The adaptive floor works too: txSwrMinimumForwardWatts tracked 0.05 → 8.298 W under
sustained carrier, i.e. 20% of the 41.49 W envelope. That's your "adapts to sustained
lower-power operation" half, confirmed.

txSwrSource read "radio" in every sample, so the Principle II claim holds on the wire
and not just in the summary. Render agrees with the model as well — the TX face painted
SWR 1.9 while the properties said 1.875.

Also confirmed in passing: rangeLegendVisible (the new Show Range option) round-trips
cleanly through the bridge.

2. Bonus data point — the 6700's two SWR sources disagree with each other

This is not a defect in this PR and nothing here needs fixing before merge — but it
turned up while measuring, it's about your applet's inputs, and you'd probably rather know.

The S-meter shows the radio's native SWR meter; the PWR cross-needle computes SWR from
the radio's FWDPWR/REFPWR coupler pair. On real hardware those two disagree:

Freq Native SWR (S-meter) Cross-needle SWR Δ %
28.550 1.055 1.144 +0.09 +8.4%
28.450 1.203 1.301 +0.10 +8.1%
28.350 1.445 1.577 +0.13 +9.1%
28.900 1.570 2.016 +0.45 +28.4%
28.250 1.711 1.873 +0.16 +9.5%
29.000 1.750 2.297 +0.55 +31.3%
28.180 1.867 2.057 +0.19 +10.1%
29.200 2.000 2.652 +0.65 +32.6%
7.290 3.680 4.504 +0.83 +22.4%

The error tracks frequency, not mismatch magnitude — 28.180 at SWR 1.87 is +10%, while
29.200 at SWR 2.00 is +33%. Tight clusters by band segment (~+9% across 28.18–28.55,
~+30% across 28.90–29.20).

To be clear about where this is coming from: it isn't your code. I went looking for a
client-side bug first and there isn't one:

  • Your math is exact. The cross-needle's SWR is precisely
    (1+√(Pr/Pf))/(1−√(Pr/Pf)) on its own forward/reflected — recomputed all nine points,
    agreement to 1e-9.
  • The decode is clean. MeterModel::convertRaw is unit-driven (it honours the unit the
    radio declares rather than assuming), and FWDPWR/REFPWR take the identical dBm→watt
    path — so a scaling error would cancel in the Pr/Pf ratio anyway. Native SWR checks out
    too: values quantize exactly on raw/128 (1.8671875 = 239/128, 3.6796875 = 471/128).
  • It isn't settling lag. Held one carrier on 7.290 for 6.3 s sampling every ~0.45 s —
    the delta never converged (+0.77…+0.85), forward/reflected steady at 51.2 W / 20.8 W.

So both numbers are decoded faithfully and the radio's own meters simply disagree in steady
state. For the native 3.68 to be true at 51.2 W forward, reflected would have to be 16.8 W;
the coupler reports 20.8 W. That's a ratio discrepancy rather than a gain error (a common
scale factor cancels), which is consistent with frequency-dependent coupler calibration the
native SWR meter corrects for and the raw pair doesn't.

And I can't tell you which one is right. Both readings come from inside the radio, so
this measurement shows a disagreement, not a verdict — settling it would need an external
analyzer or wattmeter at the antenna, which I didn't do. Treat the direction of the error
as unattributed.

If anything, this is an argument for the call you already made:

"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."

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 code

Appreciated, but I don't think a patch is warranted, and I'm not going to invent one.
Accuracy can't be corrected from the client: without ground truth, "fixing" the
cross-needle would mean fitting a fudge factor against a reference we don't have.

The only lever that exists is a policy one, and it's already written: the widget has both
setTxPowers(fwd, refl) (reflectedPowerMeasured = true, SWR from the coupler pair — what
runs whenever REFPWR is present) and setTxMeters(fwd, swr) (reflectedPowerMeasured = false, reflected derived from native SWR — currently only the fallback). Preferring the
derived path would make the needles cross at the authoritative SWR and stop the two applets
disagreeing on screen; the cost is losing the independently-measured reflected reading, and
a crossing that's just a re-projection of the SWR meter is arguably a less honest
instrument.

That's a design trade-off with real arguments on both sides, so it's a maintainer call, not
something to bolt onto this PR. Flagging it as worth a decision sometime, not as a blocker.

If settling which SWR figure is actually correct would be useful to anyone, I have a KAT500
inline on this radio and can capture external measurements as a follow-up — just say the
word.

Nice work — the filter design is sound and the instrumentation made it verifiable.


73, Ozy K6OZY
AI compute partnership: cloaked.agency — (model: claude-opus-4-8)

@jensenpat jensenpat added help wanted Extra attention needed priority: high High priority labels Jul 17, 2026
NF0T
NF0T previously requested changes Jul 18, 2026

@NF0T NF0T left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 both CrossNeedleMeterWidget and the new SMeterWidget on every repaint instead of cached/shared once.
  • SMeterGeometry::movementRayCircleIntersection() re-derives the full Layout 2-3x per frame across ticks/markers, unlike CrossNeedleMeterGeometry'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 verify mask.boundary is x-sorted, and this PR makes that ordering load-bearing for the new maskBoundaryY()-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.

@NF0T NF0T self-assigned this Jul 18, 2026
@rfoust
rfoust force-pushed the codex/s-meter-geometry branch from 2714cda to 57b989a Compare July 18, 2026 05:33
@rfoust

rfoust commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 57b989a8 and addressed the requested changes.

  1. Improving SWR during a low-relative-power dip: every physically valid decrease now uses one continuous 250 ms confirmation window while measurable RF remains present, including SWR 1.0–1.2. Loss of carrier, a non-finite sample, or another invalidating transition clears the pending window; worsening readings remain immediate. Regression coverage includes a changing 1.10/1.15 recovery, the exact 1.0 and 1.2 boundaries, and carrier loss followed by a fresh full confirmation period.

  2. Detached S-meter aspect ratios: the drawable face now uses data-driven minimum/maximum aspect limits and centers a bounded viewport inside pathological host sizes. The movement/pivot invariant and rendered needle visibility are exercised at both 200x2000 and 2000x100, in addition to the normal docked/floating sizes.

The previously noted sub-unity and mask-validation edges are also covered: below-unity native SWR requires sustained measurable-power confirmation before reaching the upper stop, and the cross-needle lower mask now rejects non-finite, unsorted, duplicate-x, out-of-bounds, or asymmetric boundaries.

This update also fixes the preexisting settings/profile-loss failure mode uncovered while testing the review changes. AppSettings saves by writing .tmp, rotating the live file to .bak, then promoting .tmp. If the app stopped between those last steps, the live path was missing even though valid recovery artifacts remained. The old loader treated any missing live file as a first run and created defaults; later saves could replace the remaining backup, making profiles and other settings appear permanently removed.

The recovery order is now: valid live file → valid pending .tmp → valid .bak → fail closed. Defaults are created only when no live or recovery artifacts exist, and the known-good backup is retained through the first post-recovery save. Tests cover valid pending promotion, invalid-temp/valid-backup fallback, both artifacts corrupt, and preservation across the first save.

Validation on the pushed tree:

  • Signed single commit, one commit ahead of current upstream/main
  • Full local build passed
  • 124 runnable tests passed; 1 expected quarantined skip
  • Four auxiliary GUI/policy harnesses passed
  • Automation bridge startup and ping passed under an isolated profile (authRequired: true)
  • Production AetherSDR.settings and .bak remained byte-for-byte unchanged
  • Accessibility and strict engine-boundary checks passed with existing warning baselines only
  • git diff --check passed

CI is running against the new head now. Ready for re-review when it finishes.

@nigelfenton

Copy link
Copy Markdown
Contributor

Independent red-team pass — built and ran the affected tests on a Linux host (Qt 6, real X), and render-proofed the cross-needle meter in all three themes. Not a maintainer verdict; complementing the existing review rather than overriding it.

Baseline heads-up: the current head 57b989a8 was force-pushed ~5.5 h after the CHANGES_REQUESTED review, so that review — and @Ozy311's FLEX-6700 validation — were against 2714cdab, which is no longer an ancestor of the head. The SWR-recovery branch that appears to address the "recovery latches to the stale worse value" note is in the new head but hasn't been re-reviewed or re-tested on hardware.

1. CI green doesn't cover this PR's own tests. .github/workflows/ci.yml only runs ctest -R "aether_dv_waveform_no_args|thumbdv_queue_test|dstar_model_test|digital_voice_waveform_process_test". It compiles s_meter_geometry_test and cross_needle_meter_test but never executes them, so "existing tests pass (CI)" doesn't apply to the ~2000 lines of new meter tests. Worth adding both targets to the CI ctest step.

2. The new accessibility tests can't pass headless. Running s_meter_geometry_test (offscreen and real X), the six "…emits exactly one settled value update" assertions fail with the captured update count == 0 — the throttled QAccessibleValueChangeEvent isn't delivered without an active a11y bridge. cross_needle_meter_test's "SWR label placement cache is keyed by the rendered font" fails for the same class of reason (fontconfig substitutes the two fonts to one, so metrics don't differ). These are environment dependencies, not product bugs — every non-a11y assertion passes and app_settings_safety_test is 9/9 — but combined with #1 they'd go red the moment CI runs them. Worth gating those assertions on bridge/font availability.

3. Stale txSwr* telemetry during RX after a transmit. SMeterWidget::setTransmitting(false) clears txSwr/txSwrHeld/txSwrPowerEnvelopeWatts/txSwrMinimumForwardWatts but not txSwrSource, txSwrRaw, or txSwrForwardWatts. Since AutomationServer gates the block on txSwrSource.isValid() and it's never cleared, after one TX an automation consumer polling during RX reads txSwrSource:"radio" with a frozen last-TX txSwrRaw/txSwrForwardWatts as if live — the exact surface the automation-bridge validation reads. Suggest clearing txSwrSource (or an "idle" sentinel) and zeroing txSwrRaw/txSwrForwardWatts on un-key.

4. validScale() doesn't require scale maxima to be positive (Principle VII). A meter-face JSON with strictly-increasing but non-positive values (e.g. [-4, 0]) makes reflectedMaximum == 0inf/NaN in swrGuidePath. It fails closed via the path element-count check, but with a misleading "path is invalid" message. Suggest requiring values.first() >= 0 && values.last() > 0 in validScale.

For balance: the merge is conflict-free, no new flat AppSettings keys (Principle V) and no vendor include added above the seam (EB3), the Principle XIV atomic-persistence hardening is solid (app_settings_safety_test 9/9), and the cross-needle faces render cleanly in all three themes. Nice work overall — none of the above is a hard blocker, mostly test-coverage and a couple of small boundary/telemetry fixes.

…mple (aethersdr#4274)

The radio-native SWR validity filter's peg-to-full-scale confirmation could
fire on a NON-continuous run of below-unity sentinels. The low-power
recovery-held path advanced the recovery window but left
m_belowUnityStartedAtMs untouched, so a genuine in-range reading landing
between two sub-1.0 samples did not restart the below-unity window; a stale
start time could then satisfy the confirmation and peg the meter to full scale
on an interrupted streak.

Reset m_belowUnityStartedAtMs in the recovery-held branch (rawSwr >= 1.0 there,
so the sub-1.0 run is broken) while keeping the recovery window advancing. Adds
testRadioSwrValidityFilterInterruptedBelowUnityDoesNotPeg, which fails without
the fix.

Also documents why the forward-power envelope is outside the AGENTS.md
MeterSmoother rule: it gates sample trust and never smooths the displayed SWR.

Addresses review findings on aethersdr#4274.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer approval (Tier 1 / AGENTS.md gate) — blockers resolved + one follow-up fix pushed

Ran a high-effort pass accounting for the Copilot, bot, and @NF0T reviews. The amended commit 57b989a8 resolves all four previously-flagged blockers, and I pushed one additional correctness fix on top (318b783a).

Verified fixed in 57b989a8

  • SWR recovery stuck at stale-worse value (@NF0T #1): the !hasMeaningfulPower branch now applies a genuine improvement after a bounded m_lowPowerRecoveryStartedAtMs confirmation instead of holding until the 750 ms envelope releases. ✅
  • SMeterGeometry::isValid() vs reachable aspect ratios (@NF0T #2): layoutFor() now letterbox-clamps both < minimumAspectRatio and > maximumAspectRatio (SMeterGeometry.cpp:659-667), and the validation sizes now exercise the real resize extremes (minimum.height * 20, minimum.width * 10) with the pivotDistance < smallestMovementRadius invariant checked on each. The 200×2000 case NF0T constructed is now covered. ✅
  • Peg-to-max on a single sub-1.0 sample (Copilot / bot): now gated behind an m_belowUnityStartedAtMs confirmation window. ✅
  • Mask boundary ordering (Copilot / bot / @NF0T): isValid() now requires finite, in-bounds, strictly increasing x (point.x() > previousMaskX) and symmetry about the canvas center. ✅

Fix I pushed (318b783a)

While verifying the new peg logic I found a residual of the same asymmetry class on the peg side: the low-power recovery-held path advanced the recovery window but left m_belowUnityStartedAtMs untouched, so a run of below-unity sentinels interrupted by a genuine in-range reading could still peg the meter to full scale on a stale start time. Fix resets the below-unity window in that branch (rawSwr ≥ 1.0 there, so the sentinel run is broken) while keeping the recovery window advancing. Added testRadioSwrValidityFilterInterruptedBelowUnityDoesNotPeg, which fails without the fix (verified by negative control) and passes with it. s_meter_geometry_test is green. Also documented why the forward-power envelope is deliberately outside the AGENTS.md MeterSmoother rule (it gates sample trust; it never smooths the displayed value) — closing @NF0T's "confirm this was deliberate" question.

Non-blocking follow-ups (from @NF0T, still open — fine post-merge)

  • movementRayCircleIntersection() re-derives the Layout 2× per call across ticks/markers each repaint (SMeterGeometry.cpp:721/739) — precompute once.
  • lowerMaskBoundary() recomputed per repaint in both meter widgets — cache per theme.
  • Three hand-rolled JSON parsers — consolidate onto AnalogMeterFaceTheme.
  • Settings loaders re-parse raw JSON just to read the version decode() already parsed.

Approving

Tier 1 / AGENTS.md approval granted on 318b783a. Thanks for the thorough decision-record doc and test coverage, @rfoust — the shared-theme architecture is the right call. Merge remains gated on CI going green for the new commit.

@ten9876
ten9876 dismissed NF0T’s stale review July 18, 2026 16:16

Changes requested have been applied.

@ten9876
ten9876 merged commit 613297d into aethersdr:main Jul 18, 2026
7 checks passed
nonoo pushed a commit to nonoo/AetherSDR that referenced this pull request Jul 19, 2026
…aethersdr#4274)

## 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

- [x] Local build passes (`cmake --build build`)
- [ ] Behavior verified on a real radio if applicable
- [x] Existing tests pass (CI)
- [x] Reproduction steps documented if user-reported bug

Additional validation:

- Full local test suite: 113 passed, 1 intentionally skipped, 0 failed
- Cross-needle response and SWR-contour fit check passed
- Accessibility check passed with 0 findings
- Strict engine-boundary check passed with 0 blockers
- `git diff --check` passed
- Automation bridge successfully rendered and inspected both the
standard S-meter and PWR/SWR meter using an isolated no-autoconnect
session
- Final native-radio SWR validity-filter changes have not yet been
exercised on a real radio

## Checklist

- [x] Commits are signed (`docs/COMMIT-SIGNING.md`)
- [x] No new flat-key `AppSettings` calls — use
nested-JSON-under-one-key
      (Principle V)
- [x] Code is clean-room — not decompiled, disassembled, or
      reverse-engineered from a proprietary binary (Principle IV)
- [x] All meter UI uses `MeterSmoother` (AGENTS.md convention)
- [x] Documentation updated if user-visible behavior changed
- [x] Security-sensitive changes reference a GHSA if applicable

---------

Co-authored-by: Jeremy [KK7GWY] <kk7gwy@aethersdr.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention needed priority: high High priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants