Repository navigation
feat(anan): show the G2's supply voltage, read from the input that carries it. Principle VIII - #6211
Conversation
There was a problem hiding this comment.
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.
parseHighPriorityStatusguards 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 arequint64with failed-writeDatagramreturns rejected before the cast, and that no remote input drives unbounded growth or log flooding. Its one informational note —supplyRailCountsunclamped — is nit 2 above; I traced theupdateValueByNameceiling 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.05pins 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}+ closing543genuinely 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_supplyRailSamplesis always ≥ 1 at the publish branch (incremented first). SettingwindowStart = nowMson 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 = 1000is genuinely underMeterModel::kVitalsFreshMs = 1500(MeterModel.h:304), as claimed. - Checked the threading claim.
m_clientis moved tom_ioThread(AnanBackend.cpp:174) and every call into it goes throughQMetaObject::invokeMethod(including thesetDdc0FrequencyHzat line 737, which is inside the queued lambda opened at 707), so the new signal crosses as a queuedintand the meter emit stays on the backend thread. The test'sSeamThreadAffinityProbecovers that too. - Looked for siblings and stale docs. No other backend decodes this field;
MeterSurfaces.h:91already has theRAD:+13.8Arow; theRadioCertificationtable is HL2-scoped;anan-p2-backend-design.md:246predicted this claim rather than contradicting it. Meter index 2 does not collide withSLC:LEVEL's 1. - Traced, rather than took on faith, "the row clears on disconnect and returns on reconnect":
onDisconnected→m_meterModel.clear(), anddefineMeters()re-runs in thelinkUphandler. 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
…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>
6cf6b63 to
f67fdd6
Compare
K5PTB
left a comment
There was a problem hiding this comment.
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:747readsReadBufp[53..54]asuser_adc0 // AIN3.ReadBufpskips the 4-byte sequence number (:714reads exciter power at[2], which is wire byte 6), so this is wire bytes 57-58.Console/console.cs:24860convertToVoltsis(22+1)/1.1 × 5.0/4095, which equalskSupplyRailVoltsPerCount. - deskHPSDR
11f5b9d:new_protocol.c:3713readsbuffer[57..58]intoADC0.rx_panadapter.c:2805scales it by0.02553forNEW_DEVICE_ORION2/NEW_DEVICE_SATURN. - p2app (Saturn
4b0b76f):OutHighPriority.c:155writes AIN3 at byte 57. A status packet every 200 ms while receiving is confirmed (:220).
- Thetis
- 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:162admits onlyisSaturn()(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_testandanan_backend_testpass. That was a leanRelWithDebInfobuild 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 thatkVitalsFreshMsgates the automation and radiocert observation, not the status bar. TheMainWindowvolts-row gate (hasSupplyVoltageTelemetry, with the temperature row withdrawn onpaTelemetryAudit) 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>
|
@K5PTB, thanks for the review. All six addressed in 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 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 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 4. Disconnect. 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. Static checks pass with |
jensenpat
left a comment
There was a problem hiding this comment.
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-treeof this head onto currentorigin/mainis 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_testandanan_backend_testpassed, 2/2.anan_link_telemetry_testwas 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
meterUpdateids are dropped byMeterModel::updateValueByName(findMeter< 0), so a count that closes a window beforedefineMeters()does not invent a reading.linkUpthen 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 emitssupplyRailSampled(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.
… 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>
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
AnanBackendchanges 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 same23.0/1.1divider 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:
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
P2Protocoldecodes the rail from bytes 57-58 intoHighPriorityStatus::supplyRailCounts. The AIN6 decode had no consumer and no scale, so it is removed.kSupplyRailVoltsPerCountcarries the scale; its comment cites the two reference clients, and the bench reading is in the capabilities-map row and above.P2Clientemits each status packet's count assupplyRailSampled. p2app sends one packet every 200 ms while receiving.AnanBackend:hasSupplyVoltageTelemetryand definesRAD:+13.8Ain volts, with the unit and range Icom uses.MeterModel's 1.5 s freshness window.linkUp, so a session never averages in the previous one's counts. Both are needed: a status packet can arrive beforelinkUp.For the TX phase, not this PR: under MOX p2app sends status every 1 ms (
OutHighPriority.c), so once ANAN transmits,supplyRailSampledwill cross threads at about 1 kHz. The mean window handles that; the per-packet queued signal is the part to revisit then.Blast radius
hasSupplyVoltageTelemetryEvidence
ci.yml's per-PR test gate is frozen, and these tests run in the full suite onmainafter merge (full-suite.yml).f7396082, on42b9e18d), locally on Linux with Qt 6.12.0: the 17 tests that useAnanBackend,P2ClientorP2Protocolrebuilt and passed 17/17,anan_link_telemetry_testincluded. No warnings in the changed files. Static checks: all 17 steps pass with--strict.b93217a1(feat(anan): publish link telemetry and withdraw the PA readout. Principle VIII. #6137's head, on33427611): 655 tests, 653 passed, 1 skipped, 1 failed.app_settings_safety's environment skip.pgxl_panel_test, a known local-environment failure: its sources are main's, and CI passes it.src/guifile changes, so the colour ratchet and a11y steps do not apply.P2Clientnot emitting the countsupplyRailSampledlinkUpP2Client's ownsupplyRailSampledandlinkUpon 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.Relation to #6106
The scale is a G2 hardware fact. When #6106's stage 1 extracts the G2 profile,
kSupplyRailVoltsPerCountand 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, throughP2Client's own signals.docs/architecture/radio-capabilities-map.md: the ANAN note on the row.🤖 Generated with Claude Code