Skip to content

feat(anan): show the G2's supply voltage, read from the input that carries it. Principle VIII - #6211

Merged
jensenpat merged 2 commits into
aethersdr:mainfrom
tropo1234:anan-supply-volts
Oct 10, 2026
Merged

jensenpat merged 2 commits into
aethersdr:mainfrom
tropo1234:anan-supply-volts

Conversation

@tropo1234

@tropo1234 tropo1234 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

An ANAN-G2 now shows its DC supply voltage in the status bar and the Meter applet, like a Flex or a profiled Icom.

Rebased onto main after #6137 merged (42b9e18d). It replaces #6137's AIN6 decode and relies on #6137's status-bar gate: with the PA temperature row withdrawn, this is the row that remains.

No other family changes. Only AnanBackend changes what it declares, and the meter it publishes, RAD:+13.8A, is the one Flex and Icom already feed.

Why the first attempt read 40 V

#6137 decoded bytes 49-50 (AIN6), which the spec calls "Supply Volts". It left the readout off because the published scale read 40.3 V on a 13.8 V rail. That scale was right. The input was wrong.

For the G2's volts, deskHPSDR and Thetis both read bytes 57-58 (AIN3, p2app's user_analog1) with the same 23.0/1.1 divider at a 5 V reference. I took that as a prediction to test, not as code: at 13.8 V, AIN3 should read about 541 counts.

The bench result, on a G2:

  • Setup: a receive-only probe outside the tree. It sent run=1, recorded every analog field of 26 status packets, then sent run=0. A meter on the DC input jack read 13.65 V.
  • AIN3 (bytes 57-58): median 536 counts, range 521-543. That is 13.68 V, 0.2% from the meter.
  • AIN6 (bytes 49-50): 1589 counts, 8.59 mV/count against the meter, which fits no published scale. 1589 counts through the scale above is the 40 V reading.
  • AIN4 (bytes 55-56): 0-16 counts while receiving. It is not decoded, and nothing here measured what it carries.

One rail voltage only. The supply could not be varied. The point fixes the scale where it is used, and the prediction matching to 0.2% makes a large offset unlikely, but one point does not separate gain from offset.

What changes

  • P2Protocol decodes the rail from bytes 57-58 into HighPriorityStatus::supplyRailCounts. The AIN6 decode had no consumer and no scale, so it is removed. kSupplyRailVoltsPerCount carries the scale; its comment cites the two reference clients, and the bench reading is in the capabilities-map row and above.
  • P2Client emits each status packet's count as supplyRailSampled. p2app sends one packet every 200 ms while receiving.
  • AnanBackend:
    • It claims hasSupplyVoltageTelemetry and defines RAD:+13.8A in volts, with the unit and range Icom uses.
    • It publishes the mean of every count since the last publication, once a second. A single packet wanders by about ±11 counts (±0.3 V), and one publication a second stays inside MeterModel's 1.5 s freshness window.
    • The window is fixed: each one opens where the last closed, so the cadence does not drift by a packet each time.
    • The window is emptied on disconnect and again at each new session's linkUp, so a session never averages in the previous one's counts. Both are needed: a status packet can arrive before linkUp.
    • A count outside 0-4095, the 12-bit ADC's range, is dropped. A misidentified input then shows no reading rather than a plausible wrong one.
  • feat(anan): publish link telemetry and withdraw the PA readout. Principle VIII. #6137's comments that called the user analogs "unassigned" and the rail unscalable are corrected. feat(anan): publish link telemetry and withdraw the PA readout. Principle VIII. #6137's body section "Supply voltage: decoded, deliberately not published" is superseded by this PR.

For the TX phase, not this PR: under MOX p2app sends status every 1 ms (OutHighPriority.c), so once ANAN transmits, supplyRailSampled will cross threads at about 1 kHz. The mean window handles that; the per-packet queued signal is the part to revisit then.

Blast radius

Family hasSupplyVoltageTelemetry Volts row
Flex true (unchanged) unchanged
Icom per profile (unchanged) unchanged
HL2, Sim, RTL-SDR false (unchanged) unchanged
ANAN false → true withdrawn → shown

Evidence

  • CI builds the head and runs the static checks. It runs none of the ANAN tests: ci.yml's per-PR test gate is frozen, and these tests run in the full suite on main after merge (full-suite.yml).
  • This head (f7396082, on 42b9e18d), locally on Linux with Qt 6.12.0: the 17 tests that use AnanBackend, P2Client or P2Protocol rebuilt and passed 17/17, anan_link_telemetry_test included. No warnings in the changed files. Static checks: all 17 steps pass with --strict.
  • Before the rebase, on b93217a1 (feat(anan): publish link telemetry and withdraw the PA readout. Principle VIII. #6137's head, on 33427611): 655 tests, 653 passed, 1 skipped, 1 failed.
    • The skip is app_settings_safety's environment skip.
    • The failure is pgxl_panel_test, a known local-environment failure: its sources are main's, and CI passes it.
  • No src/gui file changes, so the colour ratchet and a11y steps do not apply.
  • Sixteen mutations, each confirmed to fail the assert written for it:
    • the rail decoded from AIN6
    • the rail read one byte late
    • the scale at a 3.3 V reference
    • P2Client not emitting the count
    • the backend not connected to supplyRailSampled
    • the capability not claimed
    • the meter defined under another name
    • the meter defined in another unit
    • the meter published under another id
    • the last packet published instead of the mean
    • the next window opening at its first packet
    • the sum carried into the next window
    • a 500 ms window
    • no reset at linkUp
    • no reset on disconnect
    • no 0-4095 bound
  • No socket needed: the test emits P2Client's own supplyRailSampled and linkUp on the test thread, so the backend's slots run directly. That covers the connection and the per-session reset, which earlier versions of this body listed as unreachable.
  • On the radio, against a live ANAN-G2:
    • The status bar agrees with the meter at the DC input and holds within ±10 mV.
    • The Meter applet shows the same value.
    • The row clears to "—" on disconnect and returns on reconnect.

Relation to #6106

The scale is a G2 hardware fact. When #6106's stage 1 extracts the G2 profile, kSupplyRailVoltsPerCount and the choice of input belong in its "Drive and telemetry" record. Until then it is one constant, used in one place. It is measured on a G2 only, which is the one radio this backend names.

Review guide

  • src/core/backends/anan/P2Protocol.{h,cpp}: the decode position and the scale.
  • src/core/backends/anan/P2Client.{h,cpp}: one signal, emitted from the status branch.
  • src/core/backends/anan/AnanBackend.{h,cpp}: the capability, the meter definition, the averaging window, its bound and its resets.
  • tests/:
    • anan_p2_protocol_test: decode, byte order, and the bench point. The test packet sets AIN6 to 1589, so decoding the wrong input fails.
    • anan_link_telemetry_test: the client hands the count on.
    • anan_backend_test: the meter, the mean and the cadence, against an injected clock; the signal wiring, both resets and the bound, through P2Client's own signals.
  • docs/architecture/radio-capabilities-map.md: the ANAN note on the row.

🤖 Generated with Claude Code

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

Issue fit

No linked issue, and GOVERNANCE.md doesn't require one for a bug fix with a clear root cause — but this is a feature, so the governing precedent matters: docs/architecture/anan-p2-backend-design.md:246 (from the accepted #5143 design) already lists hasSupplyVoltageTelemetry true as a claim to verify at bring-up, which is exactly what this commit does. That puts it inside a scope a maintainer already ruled on, so I don't think a fresh RFC is owed.

Against the PR's own stated intent: Yes. The input moves to bytes 57-58, the scale is carried as one constant with its bench reading in the comment, the meter is published under the name MeterModel actually binds (MeterModel.cpp:245 matches on def.name == "+13.8A", and RadioModel.cpp:1186 resolves "RAD:+13.8A" through splitMeterId → updateValueByName, so the publish key and the def agree), and the caps-map row is updated.

Merge order is load-bearing. #6137 is still open. This branch carries its two commits (7a690ff2, b93217a1) and, per the body, only 6cf6b632 is under review here. #6137 must land first — this commit's whole visible effect depends on #6137's status-bar PA gate, and its P2Protocol.h comment edits rewrite text #6137 introduces.

Scope

Reviewed 6cf6b632 only (10 files). Everything in that commit is explained by the stated intent; the remainder of the harness diff is the base PR's.

File / group Changes Claimed? Verdict
P2Protocol.{h,cpp} AIN3 decode at 57-58, supplyVoltageRaw→supplyRailCounts, kSupplyRailVoltsPerCount, comment corrections Yes In scope
P2Client.{h,cpp} one supplyRailSampled signal + its emit in the status branch Yes In scope
AnanBackend.{h,cpp} capability claim, RAD:+13.8A def, 1 s mean window, per-session reset Yes In scope
docs/architecture/radio-capabilities-map.md ANAN note on the existing row Yes In scope. The map is not CI-enforced; no new capability field, so check_capability_records.py isn't engaged
tests/anan_p2_protocol_test.cpp, anan_link_telemetry_test.cpp, anan_backend_test.cpp decode + scale, client hand-off, meter/mean/cadence Yes In scope
RadioCapabilities.h, MainWindow.{cpp,h}, spectrum_sequence_gap_test.cpp, tests/tests.cmake, the LinkStats / sender-filter / counter hunks in P2Client and AnanBackend #6137's two commits — Belongs to #6137, reviewed there, not re-reviewed here

No CHANGELOG.md entry (correct). No new public/protocol surface: RAD:+13.8A is a meter Flex (FlexBackend.cpp:400) and Icom (IcomMeters.cpp:119) already feed, at the same unit and 0–16 range. No settings, no credentials, no family-string branch. RadioCertification.cpp:102's {"RAD","+13.8A",false,…,"no supply telemetry on this radio"} is not a stale sibling — its flag is expectedOnHl2 (RadioCertification.cpp:46), an HL2-only table.

Socket-owning test (recorded per §2, not a finding): tests/anan_link_telemetry_test.cpp — UDP, via P2Client::start() binding QUdpSocket on AnyIPv4 at an ephemeral port and sending the startup sequence to 127.0.0.1 with no listener and no fake peer. Target anan_link_telemetry_test, registered at tests/tests.cmake:1102-1108, whose block names the bind and sets SKIP_RETURN_CODE 77. The socket block is #6137's; this commit adds one socket-free block to the same file. .github/ci-test-gate.txt contains no ANAN entry, so the frozen per-PR gate does not run it — it runs in full-suite.yml on main. Disclosed in #6137's body.

Blockers

None.

Needs a maintainer decision

src/core/backends/anan/P2Protocol.h:243-250 — the client reads the field the protocol labels user_analog1 as the DC rail, and rejects the one the spec labels "Supply Volts".

inline constexpr double kSupplyRailVoltsPerCount = (23.0 / 1.1) * (5.0 / 4095.0);

This is a deliberate, well-documented deviation from the protocol authority (Principle I's HPSDR analogue), and the case for it is good: the spec-labelled input measured 1589 counts against a 13.65 V meter, which fits no published scale, and the body cites deskHPSDR and Thetis both reading AIN3 with this same divider. I could not fetch either reference client from here, so that corroboration is unverified by me — it is the strongest part of the argument and worth one reviewer confirming against deskhpsdr's own status decode.

What I'd want ruled on explicitly is the generalization, since the PR is honest about the measurement but the code is silent about its reach: the scale is the wiring of a user analog input on one unit, applied unconditionally to every radio this backend accepts (AnanBackend.cpp:416-419, with c.model hardcoded "ANAN-G2"). A G2 variant, or a unit where AIN3 has been repurposed, gets a confidently wrong voltage with nothing marking it as inferred. The single-point gain-vs-offset limit is already disclosed in the body, the commit message and the header comment, so I'm not re-litigating that — the question is only whether a bench point from one G2 is enough to publish a calibrated Volts meter for the family. #6106's per-model telemetry record is the natural home, and the PR already says so.

Nits (non-blocking)

1. src/core/backends/anan/AnanBackend.h:226-231 — the per-session reset is hooked where the comment can't be true, and disconnectRadio() leaves the window dirty.

    // -1 = no window open. Reset per session in the linkUp handler, like
    // m_sMeter, so a new session never averages in the last one's counts.
    qint64 m_supplyRailWindowStartMs = -1;

The reset is inside the linkUp handler (AnanBackend.cpp:207-218), and linkUp fires on the first valid DDC0 frame — but P2Client::handleDatagram emits supplyRailSampled from the status branch, which returns before anything DDC-shaped is required. A status packet that beats the first IQ frame therefore reaches onSupplyRailCountsAt() with the previous session's m_supplyRailCountSum / Samples still loaded (disconnectRadio() at AnanBackend.cpp:753-758 resets m_linkCounters but not these three), and m_supplyRailClock never restarts, so nowMs - m_supplyRailWindowStartMs is far past 1000 ms and that one packet publishes (oldSum + count) / (oldSamples + 1). Same shape as the Icom operator-disconnect leak. Queued supplyRailSampled deliveries can also land after the blocking stop() returns, for the same reason.

In practice the bad value is swallowed: RadioModel::onDisconnected() calls m_meterModel.clear() (RadioModel.cpp:7953) and defineMeters() only re-runs on linkUp, so a pre-linkUp publish hits findMeter() < 0 in updateValueByName and is dropped. So this is a correctness-of-the-comment and dirty-state issue rather than a visible wrong reading — reasoned from code, not reproduced. The cheap fix is to make the mechanism match the claim, next to the link-counter reset in disconnectRadio():

    m_linkCounters = {};
    m_linkCountersSeen = false;
    m_lastSnapshotRxBytes = 0;
    // The rail's window likewise: the linkUp reset runs on the first DDC0
    // frame, and a status packet can arrive before it.
    m_supplyRailWindowStartMs = -1;
    m_supplyRailCountSum = 0;
    m_supplyRailSamples = 0;

(The zoom path is fine as-is: linkUp with wasRateChange skips the reset deliberately, and carrying counts across a rate change is the same rail in the same session.)

2. src/core/backends/anan/AnanBackend.cpp:1522 — nothing bounds the published voltage, and the declared range isn't the enforcer. supplyRailCounts is documented as 12-bit but never clamped, so a malformed packet, or an AIN3 that is floating or repurposed, feeds the meter straight through: 4095 counts reads 104.5 V, well outside the rail.high = 16.0 the def declares. MeterModel::updateValueByName clamps Volts only at 32767/256 ≈ 128 V (MeterModel.cpp:537-542), and the status-bar stack reserves its width with a "99.99 V" sample, so a three-digit reading is both displayed and liable to clip. Display-only and no security consequence (the security audit reached the same conclusion — see below), but given that #6137's withdrawn readout was exactly a 40 V reading on a 13.8 V rail, a sanity floor — publish nothing outside, say, 0–4095 counts, or outside a plausible rail band — would make the next mis-identified input fail visibly instead of plausibly.

3. src/core/backends/anan/AnanBackend.h:222 — "about five packets at p2app's 200 ms receive cadence." The closing window holds six (t=0, 200 … 1000); the test at tests/anan_backend_test.cpp feeds exactly that. Trivial.

What I tried to break

  • The decode position and bounds. parseHighPriorityStatus guards on exact size 60 (P2Protocol.cpp:214); readU16be(data.data() + 57) touches 57-58, max index 59 — no over-read, and the guard is != not >=, so no short-packet path either. The required security audit (CodeGuard) returned no findings across all four flagged files, and independently confirmed the bounds, that the sender filter runs before any counter or parser, that the counters are quint64 with failed-writeDatagram returns rejected before the cast, and that no remote input drives unbounded growth or log flooding. Its one informational note — supplyRailCounts unclamped — is nit 2 above; I traced the updateValueByName ceiling myself.
  • Attacked the tests rather than trusting the mutation table. Both discriminators are real: the protocol test plants 1589 counts at bytes 49-50 as a decoy, so decoding the spec-labelled input fails the assert, and std::fabs(536 * kSupplyRailVoltsPerCount - 13.65) < 0.05 pins the scale against a literal rather than against the production constant (a 3.3 V reference reads 9.03 V and fails; the real value is 13.685, inside the 0.05 margin with 0.015 to spare — tight, but it is the bench point, not slack). The backend test's {521,543,521,543,521} + closing 543 genuinely separates the mean (532) from first (521) and last (543), and the second window's 536-only assertion would fail if the sum carried over.
  • Walked the window arithmetic for drift and divide-by-zero. m_supplyRailSamples is always ≥ 1 at the publish branch (incremented first). Setting windowStart = nowMs on the closing packet does not drift for periodic arrivals — the closing packet lands in the old window and starts the new one, so the next close is at +1000 exactly. kSupplyRailWindowMs = 1000 is genuinely under MeterModel::kVitalsFreshMs = 1500 (MeterModel.h:304), as claimed.
  • Checked the threading claim. m_client is moved to m_ioThread (AnanBackend.cpp:174) and every call into it goes through QMetaObject::invokeMethod (including the setDdc0FrequencyHz at line 737, which is inside the queued lambda opened at 707), so the new signal crosses as a queued int and the meter emit stays on the backend thread. The test's SeamThreadAffinityProbe covers that too.
  • Looked for siblings and stale docs. No other backend decodes this field; MeterSurfaces.h:91 already has the RAD:+13.8A row; the RadioCertification table is HL2-scoped; anan-p2-backend-design.md:246 predicted this claim rather than contradicting it. Meter index 2 does not collide with SLC:LEVEL's 1.
  • Traced, rather than took on faith, "the row clears on disconnect and returns on reconnect": onDisconnected → m_meterModel.clear(), and defineMeters() re-runs in the linkUp handler. Consistent.
  • Could not check: CI is green on 6cf6b632 (all 5 checks, run 37343938555), but the per-PR gate is -R-filtered and contains no ANAN target, so none of the three touched tests gated this merge. I cannot run a build, the suite, or the GUI, so every finding above is reasoned from code, not reproduced at runtime — in particular nit 1's ordering and nit 2's out-of-band display. The deskHPSDR/Thetis corroboration and the bench numbers themselves are unverifiable from here.

Recommendation

Approve with nits — merge after #6137. The reviewable commit is small, the scope is clean, the comments explain why at the right places, and the tests discriminate instead of asserting the implementation back to itself; the one measurement limit that matters is disclosed in three places rather than hidden. Next step: mirror the three rail resets into disconnectRadio() (nit 1), and get a maintainer to bless the AIN3-over-spec choice and its reach beyond the one bench unit — ideally with a second pair of eyes on deskHPSDR's own decode, since that citation is doing real work and no one in this review has verified it. Nice piece of bench work, and thank you for publishing the AIN6 number that didn't fit alongside the one that did.


🤖 aethersdr-agent · cost: $15.5906 · model: claude-opus-5

jensenpat pushed a commit that referenced this pull request Oct 7, 2026
…iple VIII.

ANAN was showing a blank Network readout because AnanBackend never published LinkStats, and a PA temperature row that Protocol 2 can never fill. P2Client now counts received bytes on every datagram from the radio, counts packets only for session DDC frames, and publishes a one-second snapshot. The status bar withdraws the PA-temperature row, and the separator after it, only when paTelemetryAudit says the wire format has no temperature field.

Bytes 49-50 are decoded and unused. The supply-voltage readout stays off. #6211 is the follow-up that moves that decode.

Squashed-from: #6137
Co-authored-by: tropo1234 <299180028+tropo1234@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rries it. Principle VIII.

The status bar and the Meter applet now show an ANAN-G2's DC supply
voltage, published as RAD:+13.8A from every High Priority status packet.

The rail is on bytes 57-58 (AIN3, p2app's "user_analog1"), not on bytes
49-50, which the spec calls "Supply Volts". Measured on a G2 with a meter
at the DC input jack reading 13.65 V: AIN3's median was 536 counts, which
the 23.0/1.1 divider at a 5 V reference reads as 13.68 V -- the figure
predicted before the run. AIN6 read 1589 counts on the same bench, which
fits no published scale. That same scale applied to AIN6 is what read
40.3 V when this readout was first tried, and why it was withdrawn. One
rail voltage only: the supply could not be varied, so the point fixes the
scale where it is used and does not separate gain from offset.

- P2Protocol decodes the rail from bytes 57-58 (supplyRailCounts) and
  drops the AIN6 decode, which had no consumer and no scale.
- P2Client emits each status packet's count.
- AnanBackend claims hasSupplyVoltageTelemetry, defines RAD:+13.8A in
  volts, and publishes the mean of every count since the last publication
  once a second -- inside MeterModel's 1.5 s freshness window. One packet
  alone wanders by about +/-11 counts (+/-0.3 V).
- The comments aethersdr#6137 added that called the user analogs unassigned and
  the rail unscalable are corrected.

Verification: build clean; ctest 655 tests, 653 passed, 1 skipped
(app_settings_safety's environment skip), 1 failed (pgxl_panel_test, a
known local-environment failure: its sources are main's and CI passes
it); 12/12 mutations each killed by the assert written for it; on a G2,
the status bar agrees with the meter at the DC input and holds within
+/-10 mV, the Meter applet shows the same value, and the row clears on
disconnect and returns on reconnect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@K5PTB K5PTB 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.

Adversarial review: head f67fdd6c

Does it do what it says?

Yes. There's no linked issue. docs/architecture/anan-p2-backend-design.md:246 (#5143) already listed hasSupplyVoltageTelemetry true as a bring-up claim, and this PR makes that claim with evidence. The earlier bot review left one thing open: that deskHPSDR and Thetis read the rail from bytes 57-58 at this scale. That claim carries the most weight in the PR, so I checked it against both clients' source (details below). Both citations hold. Together with the bench point, that settles the question the earlier review left for a maintainer: whether AIN3 should be used for the rail instead of the input the spec labels "Supply Volts".

Scope

File / group Change Claimed? Verdict
src/core/backends/anan/P2Protocol.{h,cpp} Decode moves from bytes 49-50 to 57-58; field renamed; kSupplyRailVoltsPerCount added Yes In scope
src/core/backends/anan/P2Client.{h,cpp} One supplyRailSampled signal, emitted from the status branch Yes In scope
src/core/backends/anan/AnanBackend.{h,cpp} Capability claimed, RAD:+13.8A defined, 1 s mean window, linkUp reset, test-access friend Yes In scope
docs/architecture/radio-capabilities-map.md ANAN note on the existing row Yes In scope
tests/anan_{p2_protocol,link_telemetry,backend}_test.cpp Decode and scale, client hand-off, meter, mean and cadence Yes In scope

Everything in the diff is explained by the stated intent. The branch is one commit on 42b9e18d. There's no CHANGELOG entry, no settings key, no family-string branch, and no new public surface, since RAD:+13.8A is already fed by Flex and Icom. The blast-radius table is accurate: only AnanBackend changes what it declares.

Blockers

None.

Nits (non-blocking)

1. The two "not reachable by a unit test" seams can be tested without a socket, and nothing pins them today. AnanBackend.cpp:293 (the supplyRailSampled connect) and AnanBackend.cpp:238-240 (the linkUp reset). m_client lives on anan-io, but emitting its signal from the test thread resolves the AutoConnection to a direct call into the backend. That gives a deterministic test with no socket and no radio. I verified it by mutation on the PR merged into current main:

  • M1, the connect deleted: every existing check passes. Only the probe below fails.
  • M2, the three reset lines deleted: every existing check passes. Only the probe fails. Without the reset, a reconnect's first post-linkUp packet publishes the old session's counts averaged in.
  • M3, < changed to <= at the window edge: the PR's own checks catch it.

Drop-in for tests/anan_backend_test.cpp. Add to AnanSupplyRailTestAccess (:52):

    static P2Client* client(AnanBackend& backend) { return backend.m_client; }
    static int samples(const AnanBackend& backend) { return backend.m_supplyRailSamples; }

and add before // ---- LinkStats: silence before the first snapshot (:702):

    // ---- The rail's wiring and its per-session reset, through P2Client's own signals ----
    // Emitted on this thread, so the backend's AutoConnection runs the slot
    // directly: no socket, no radio, no started session.
    {
        AnanBackend backend;
        P2Client* client = AnanSupplyRailTestAccess::client(backend);
        emit client->supplyRailSampled(536);
        check(AnanSupplyRailTestAccess::samples(backend) == 1,
              "P2Client's supplyRailSampled reaches the backend's rail window");

        QSignalSpy meter(&backend, &IRadioBackend::meterUpdate);
        AnanSupplyRailTestAccess::countsAt(backend, 4000, 0);   // the last session's count
        emit client->linkUp();                                  // a new session
        AnanSupplyRailTestAccess::countsAt(backend, 536, 5000);
        int railPublishes = 0;
        for (const auto& args : meter)
            railPublishes += args.at(0).toString() == QStringLiteral("RAD:+13.8A");
        check(railPublishes == 0,
              "a new session's first count opens a window rather than closing the last one's");
        AnanSupplyRailTestAccess::countsAt(backend, 536, 6000);
        double last = -1;
        for (const auto& args : meter)
            if (args.at(0).toString() == QStringLiteral("RAD:+13.8A"))
                last = args.at(1).toDouble();
        check(qAbs(last - 536 * kSupplyRailVoltsPerCount) < 1e-9,
              "and its first reading averages none of the last session's counts");
    }

With this in, the body's "Not reachable by a unit test" list shrinks to the live G2 run, which covers the end-to-end path anyway.

2. The body says "CI builds and tests the rebased head", but the per-PR gate runs none of the three touched tests. .github/ci-test-gate.txt is the frozen per-PR list and contains no ANAN target. CI built f67fdd6c on 2026-10-07 but never ran anan_*_test. The ctest numbers in the body come from the pre-rebase base, so no one had run this head's tests until this review. I have now (see below), so the claim holds. The body should still say "CI builds it; the ANAN tests run in the post-merge full suite" rather than imply CI tested it. That's Principle VIII's "a claim whose verification step was not actually run is demoted to a hypothesis". The title cites that principle.

3. P2Protocol.h:246-251 and :263-275 run well past AGENTS.md §Comments' "five lines or fewer" and carry history. The rule says "History does not go in code. How the bug was found … belong[s] in the commit message and PR body." The dated bench session (2026-10-05, "On the same bench that input read 1589 counts…") is that kind of narrative. The current-state fact also has a stronger authority than one bench point: the two reference clients. Suggested replacement for :246-251:

// Volts per count of HighPriorityStatus::supplyRailCounts: a (22+1)/1.1 divider
// into the 12-bit slow ADC at 5 V, as Thetis (console.cs convertToVolts) and
// deskHPSDR (rx_panadapter.c, Orion2/Saturn) scale the same input. Checked
// against a meter at one rail voltage on a G2; see docs/architecture/radio-capabilities-map.md.
inline constexpr double kSupplyRailVoltsPerCount = (23.0 / 1.1) * (5.0 / 4095.0);

and for :263-275:

    // Bytes 57-58: the DC supply rail in raw 12-bit counts. p2app labels it
    // "AIN3 user_analog1"; Thetis and deskHPSDR read the rail here. NOT bytes
    // 49-50 (AIN6), which the spec calls "supply voltage" but which fits no
    // published scale on a G2. No field in this payload is a temperature.
    std::uint16_t supplyRailCounts = 0;

The bench numbers already live in the caps-map row and the PR body, where they belong.

4. Carried over from the earlier review and still open after the rebase: disconnectRadio() doesn't reset the window. AnanBackend.cpp:818 resets m_linkCounters but not the three rail fields. A status packet that beats the next session's first DDC0 frame (and so beats linkUp) can therefore publish (oldSum + count) / (oldSamples + 1). MeterModel drops it today, because the def is cleared on disconnect and only redefined on linkUp. So this is dirty state, not a visible reading, and it's reasoned from the code rather than reproduced. Mirroring the three resets next to m_linkCounters = {}; makes the header comment at AnanBackend.h:242-243 literally true.

5. Also carried over: the published value is unbounded. AnanBackend.cpp:1587 feeds any readU16be value straight through. A 4095-count reading would show 104.5 V against rail.high = 16.0. This is display-only, but #6137's withdrawn readout was exactly a plausible-looking wrong voltage. Dropping counts outside 0-4095 would make the next misidentified input fail visibly.

6. Forward note for the TX phase, not for this PR. p2app sends status every ~1 ms while MOX is asserted (P2_app/OutHighPriority.c:220, SleepCount = (MOXAsserted) ? 2 : 400 × 500 µs). Once canTransmit becomes true, supplyRailSampled will cross threads at ~1 kHz, and P2Client.h:186's "every 200 ms" will only describe receive. The mean window handles that correctly. The per-packet queued signal is the part to revisit.

What was verified, and what I tried to break

  • The reference-client citations, against their source.
    • Thetis 852bf0e: ChannelMaster/network.c:747 reads ReadBufp[53..54] as user_adc0 // AIN3. ReadBufp skips the 4-byte sequence number (:714 reads exciter power at [2], which is wire byte 6), so this is wire bytes 57-58. Console/console.cs:24860 convertToVolts is (22+1)/1.1 × 5.0/4095, which equals kSupplyRailVoltsPerCount.
    • deskHPSDR 11f5b9d: new_protocol.c:3713 reads buffer[57..58] into ADC0. rx_panadapter.c:2805 scales it by 0.02553 for NEW_DEVICE_ORION2/NEW_DEVICE_SATURN.
    • p2app (Saturn 4b0b76f): OutHighPriority.c:155 writes AIN3 at byte 57. A status packet every 200 ms while receiving is confirmed (:220).
  • The scale reaching the wrong board. deskHPSDR uses a 3.3 V scale for the G2E (rx_panadapter.c:2795, 0.0168498), so this constant would read a G2E 1.5× high. Thetis disagrees and uses 5 V for the G2E. It can't happen here: AnanDiscovery.cpp:162 admits only isSaturn() (board 10), and the G2E reports board 20. If that filter is ever widened, this constant needs a per-board record (#6106).
  • The tests, built and run on the PR merged into current main (d6f6a4f0, 53 commits past the base). The merge was clean. anan_p2_protocol_test and anan_backend_test pass. That was a lean RelWithDebInfo build against the pinned Qt 6.12.0, macOS arm64, shared core. Mutations M1-M3 are above. The decode tests discriminate: the AIN6 decoy at bytes 49-50, the byte-swap and single-byte guards, and the scale pinned against the literal 13.65 V.
  • Not run: anan_link_telemetry_test. Its pre-existing last block binds a UDP socket, so the test-layer preflight holds it until the operator directs otherwise. The block this PR adds to it is socket-free. I read it but didn't execute it.
  • Engine-boundary gate on base and head: identical findings (102 tracked warnings, 0 that would block under --strict). No vendor include or baseline growth.
  • The window arithmetic. I walked it for divide-by-zero (samples is ≥ 1 at publish), for drift (the closing packet stays in the old window, and periodic arrivals republish at +1000 ms), and for one or two lost packets (still under MeterModel::kVitalsFreshMs = 1500). Note that kVitalsFreshMs gates the automation and radiocert observation, not the status bar. The MainWindow volts-row gate (hasSupplyVoltageTelemetry, with the temperature row withdrawn on paTelemetryAudit) is unchanged on current main.
  • Not driven: the bridge. The demo backend is SimBackend, and nothing in this diff is reachable from it. The status bar and Meter applet claims rest on the author's live G2 run.

Recommendation

Approve with nits. The input choice is now backed by both reference clients as well as the bench, the scope is clean, and the tests pass on current main. The worthwhile follow-up is nit 1: two lines in the backend have no test, and the drop-in above pins both. Nits 2-3 are body and comment edits.

…nge counts, and its wiring is tested. Principle VIII.

Review fixes on aethersdr#6211.

- AnanBackend: resetSupplyRailWindow() empties the rail's averaging
  window on disconnect as well as at linkUp, since a status packet can
  arrive before the next session's linkUp.
- A count outside 0..4095, the 12-bit ADC's range, is dropped: a
  misidentified input shows no reading rather than a plausible wrong one.
- P2Protocol.h: the scale and field comments cite Thetis and deskHPSDR
  instead of narrating the bench session, which stays in the
  capabilities-map row.
- anan_backend_test: P2Client's supplyRailSampled and linkUp signals,
  emitted on the test thread, drive the backend's wiring and its reset
  without a socket. A disconnect empties the window; 4096 is dropped and
  4095 kept.

The 17 tests that use AnanBackend, P2Client or P2Protocol rebuilt and
passed 17/17. Mutations killed: no supplyRailSampled connection, no
linkUp reset, no disconnect reset, no 0..4095 bound. Static checks pass
with --strict.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tropo1234

Copy link
Copy Markdown
Contributor Author

@K5PTB, thanks for the review. All six addressed in f7396082.

1. The two untested seams. Your test is in as given, plus three checks in the same block: a disconnect empties the window, a count of 4096 is dropped, and 4095 is kept. Mutations killed: no supplyRailSampled connection, no linkUp reset, no disconnect reset, no bound. The body's "Not reachable by a unit test" list is gone.

2. The CI claim. Corrected: CI builds the head, and the ANAN tests run in the post-merge full suite. This head's tests now have a local run too: the 17 that use AnanBackend, P2Client or P2Protocol pass, anan_link_telemetry_test included.

3. Comments. Both cut to your shape. The scale comment cites the two clients, and the bench numbers stay in the capabilities-map row and the body. I checked the citations against the clients' source: deskHPSDR rx_panadapter.c uses 0.02553 * ADC0 for Orion2/Saturn, and Thetis convertToVolts is (22+1)/1.1 × 5/4095.

4. Disconnect. disconnectRadio() now empties the window through resetSupplyRailWindow(), which linkUp also calls.

5. Bound. Counts outside 0-4095 are dropped, so a misidentified input shows no reading rather than a plausible wrong one.

6. TX cadence. Noted in the body for the TX phase. P2Client.h already says "every 200 ms while receiving".

Static checks pass with --strict, and the head merges cleanly with main a9a83e66. Would you take another look?

@jensenpat jensenpat self-assigned this Oct 10, 2026

@jensenpat jensenpat 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.

Reviewed f7396082bc1aaecf64ea0cb31ed6885bcf1757a0.

Issue fit: no linked issue. The change matches the PR's stated intent: an ANAN-G2 publishes its DC rail as RAD:+13.8A, from High Priority status bytes 57-58, so the existing status-bar and Meter applet voltage path can show it. This replaces the #6137 AIN6 decode that had no scale. Scope stays inside the ANAN backend, its three tests, and the capabilities-map note. Flex, Icom, HL2, Sim, and RTL are untouched. hasSupplyVoltageTelemetry is an existing flag, and the backend publishes the meter it names, so this does not add a capability without a verb (#5554). AnanDiscovery still admits only Saturn (isSaturn(), board 10).

Scope

Path What changed
P2Protocol.{h,cpp} Rail is bytes 57-58, big-endian, named supplyRailCounts. Scale (23.0/1.1)*(5.0/4095.0). AIN6 decode removed.
P2Client.{h,cpp} Each status packet emits supplyRailSampled with the raw count. AutoConnection from the I/O thread is queued onto the backend.
AnanBackend.{h,cpp} Claims the capability, defines RAD:+13.8A in volts 0–16, publishes the mean once a second, drops counts outside 0–4095, and empties the window on disconnect and on a non-rate-change linkUp.
tests/anan_p2_protocol_test.cpp Pins 536 at bytes 57-58 against an AIN6 decoy of 1589, byte order, a zero rail, and the bench point (within 0.05 V of 13.65 V).
tests/anan_link_telemetry_test.cpp Status packet hands the count on once; a DDC frame does not.
tests/anan_backend_test.cpp Meter name and unit, mean, fixed 1 s cadence, signal wiring, both resets, and the 4095/4096 bound.
docs/architecture/radio-capabilities-map.md ANAN note on the existing hasSupplyVoltageTelemetry row. Columns are still Flex / HL2 / Sim.

No blockers.

Verification

  • Required checks on this SHA are success: build, check-windows, check-macos, Static checks, Sanitizer option configures. The frozen per-PR gate does not run the ANAN tests.
  • git merge-tree of this head onto current origin/main is clean. Main's ANAN delta is the noise-blanker signature; it does not touch the rail window.
  • Local RelWithDebInfo, Qt 6.12.0, macOS arm64, head f7396082: anan_p2_protocol_test and anan_backend_test passed, 2/2. anan_link_telemetry_test was not executed: the target still binds a UDP socket in its last block (P2Client::start(), AnyIPv4). The new block sits above that bind and was read.
  • Unknown meterUpdate ids are dropped by MeterModel::updateValueByName (findMeter < 0), so a count that closes a window before defineMeters() does not invent a reading. linkUp then clears the window and defines the meter.
  • The app was not built. The status bar and Meter applet were not driven; the demo backend cannot reach this path.

Non-blocking follow-ups

These are not merge conditions.

  • Execute anan_link_telemetry_test's new status-packet block on a runner that already accepts that target's UDP bind, or split the socket-free block into a target that does not bind. Families: ANAN only. Scenario: one 60-byte status packet with bytes 57-58 = 536 emits supplyRailSampled(536) once, and a DDC0 frame does not.
  • The live status-bar and Meter applet path is still the author's G2 run (status bar within ±10 mV of the DC-input meter, applet matches, row clears on disconnect). A bridge drive cannot cover it.
  • The scale is one rail voltage on a G2. A second voltage would separate gain from offset. If discovery ever admits a G2E (board 20), deskHPSDR's 3.3 V scale for that board needs its own constant; that belongs with #6106.

@jensenpat
jensenpat merged commit 91260c6 into aethersdr:main Oct 10, 2026
5 checks passed
@tropo1234
tropo1234 deleted the anan-supply-volts branch October 10, 2026 17:29
ten9876 added a commit that referenced this pull request Oct 12, 2026
… and re-take the Stable snapshot. Principle VIII. (#6396)

## Summary

This brings the user guide (docs.aethersdr.com) up to date with
everything in v26.10.2 and re-takes the Stable snapshot so the default
site shows it. Docs only, apart from the TCI page prose in
`tools/docs/gen_reference.py`; no application source, test or CI
changes.

Every page was audited against the 106 PRs in the release. Every fact
that was added or kept was checked against the source at `f753dbfc4`,
the v26.10.2 release commit, not taken from the PR bodies alone.

## What changed

- **Menu paths.**
- AetherModem, Callsign Lookup, AetherMap and FreeDV Reporter are on
**Tools**. They are created on View and then moved to Tools
(`MainWindow_Menus.cpp:1519-1520`, `:1569-1575`); the style guide's
example says **Tools → AetherMap...**.
  - **Show live CW contacts** is on **View**.
  - Waveforms is on **Tools**, not File.
  - The VUDU applet button was called TXDSP.
  - Labels now match the source's `...` vs `…`.
- **Gaps from this cycle.**
- **Windows and Network Diagnostics:** Windows Firewall rules and the
Network Diagnostics Windows Firewall page (#6289); the `aether.network`
and `aether.nvafx` log categories (#6289, #6308).
  - **Remote access:** Tailscale shim revocation semantics (#6325).
  - **Sleep, App Nap and power throttling (#6195):** per-OS notes.
- **Hermes-Lite 2:** state restore, RX audio at unity, CW at TUNE level,
multi-interface discovery and a discovery troubleshooting entry (#6259,
#6254, #5975, #6009, #6293).
- **ANAN-G2:** AGC-T range, a "What is remembered" section, and supply
voltage and link status (#6282, #6129, #6137, #6211).
  - **NB2** on the HL2 and ANAN-G2 (#5966).
- **DVK:** WAV import requirements, the slot menu without Delete, status
text and licensing (#6247, #6262, #6251).
  - **Copy Assist** from unprocessed audio (#6216).
  - **Live CW contacts** replace the decoder's contact card (#6268).
  - **Status-bar indicators** work from the keyboard (#6267).
- **Volume:** controllers, hotkeys and TCI `volume` follow the active
output (#6129, #6260).
- **Peripherals:** the KPA1500 (#5905), CTR2 link extensions and the
AetherKnob (#6338), TMate 2 udev (#6250), MIDI RADE Modem and FDV modes
(#6031, #6038).
  - **RTL-SDR:** live PPM (#6082).
  - **Icom:** COMP and the MK2 squelch scale (#6174, #6184).
- **Known issues.** Entries whose GitHub issues are closed are removed
(each checked with `gh issue view`). As decided, this includes three
closed today without a demonstrated fix: #5107, #4824 and #5665.
- **US spelling.** `docs-style-guide.md` asked for British spelling; it
now asks for US, by the maintainer's decision, and every page is
converted. The generated TCI pages are converted at their source, the
prose strings in `tools/docs/gen_reference.py`. Code spans, links and
URLs were left alone. UI labels were checked against the source strings,
and none of them are British. Two in-page anchors and two Log Analyzer
rule anchors (`#bnr-is-grayed-out-…`) follow the renamed headings.
- **Stable snapshot.** `snapshot_stable.py 26.10.2` was re-run on the
release tree with these corrections, before `main` was merged in, so
Stable carries no post-release docs. `docs/` also has `main`'s
post-release pages, in US spelling. Stable alone keeps the IC-9700
waterfall-rate entry (#5996): #6402 fixed it after the 26.10.2 tag, so
26.10.2 still has it. The PDF manual already attached to the v26.10.2
release keeps the tag's docs.

## Validation

- `npm ci && npm run build` in `docs/user`: success. This is the
broken-link and broken-anchor check.
- `node tools/docs/test_log_rules.mjs`: 707 checks passed for 45 rules.
- `python3 tools/docs/gen_reference.py --check`: up to date (8 pages).
- `git diff --check`: clean.
- Menu paths on a build of this branch (offscreen, isolated settings, no
radio), `menu list`: `Tools → AetherModem...`, `Tools → Callsign
Lookup...`, `Tools → AetherMap...`, `Tools → FreeDV Reporter...`, `Tools
→ Waveforms...`, `View → Show live CW contacts`.

## Screenshots now out of date (not recaptured)

These need a recapture pass on the radio:
- **Old panadapter title strip (#6386):** `panadapter`, `main-window`,
`first-session-*`, `multi-slice-two-slices`, `split-pair`,
`tnf-markers`, `offscreen-vfo-indicator`, `pan-3d-stacked-trace`,
`default-light-theme`.
- **Pre-canon windows (#6334, #6339, #6341, #6372):** `aetherrx*`,
`aethertx*`, `aethermodem-aprs`, `bnr-panel`, `connect-dialog`,
`connect-by-ip-card`, `connect-icom-rows`, `connect-smartlink-card`,
`rx-agc-calibration`.
- **Other:**
  - `title-bar` (lowercase tab states, #6281);
  - `menu-help` (#6294);
  - `display-panel` (#6371);
  - `support-diagnostics-dialog`;
  - `waveforms-dialog` and `tailnet-remote-access` (show shim 0.4.0);
  - `about-window` (v26.10.1);
  - `radio-setup-qrz`.

## Found during the audit, not fixed here

- **App string bug:** `TailnetShimDialog.cpp:628` tells the user to
"Connect remotely with Connect → Manual"; the real path is **File →
Connect to Radio... → Connect by IP**.
- **ANAN AGC-T tooltips:** the RX applet and VFO tooltips show the
slider position as dB, which is wrong on ANAN. #6282 lists this as a
follow-up.
- **MIDI Master Volume on non-Flex radios:** it is refused while PC
Audio is off, whereas hotkeys, FlexControl and HID drive the line out.
The docs describe the code as it is.
- **RTL mode names:** `rtl-sdr.md` writes "FM-N" and "CW-R", but the
source strings are `FMN` and `CWR`. Left as is until someone confirms
what the mode menu displays.
- **IC-705 COMP readback:** inferred from the MK2 (#6174) and not
verified on an IC-705.
- **Not yet documented:** the #6230 Radio Health row "Forward responses
skipped", and the Store/MSIX package.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants