From 7a690ff2fe0c7d865d36c163e0a4a32b823b0da4 Mon Sep 17 00:00:00 2001 From: Sotiris Date: Sat, 3 Oct 2026 17:58:50 -0500 Subject: [PATCH 1/2] feat(anan): publish link telemetry and withdraw the PA readout. Principle VIII. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status bar's "Network:" field was blank on an ANAN, and its PA temperature row read "PA --°C" for the whole session. These are the same mistake pointing in two directions: one readout had no source wired, the other has no source that can exist. NETWORK AnanBackend never emitted linkStatsUpdated -- the dropsUpdated handler was an empty stub with a comment deferring it -- so RadioModel::usesBackendLinkStats() was always false and everything behind the label fell back to a stream source this family does not have. P2Client now counts received bytes and packets, sent bytes, and its bound local endpoint, and publishes them on a fixed one-second cadence. AnanBackend caches each snapshot and emits LinkStats built from it. Three deliberate choices: * Receive traffic is counted BEFORE any parse decides what a datagram is. The readout describes the TRANSPORT, so a session whose IQ has stalled while status packets still arrive must not read as a dead link -- and counting after the shape checks would make exactly those datagrams invisible. A mutation that moves the counting after the parse is in the test evidence below. * The counters are I/O-thread-only and reach the backend as a queued snapshot, never a cross-thread read. That is what the LinkStats contract asks for: a backend with worker-side values caches them on its own thread. * rttMs, jitterMs, gapMs and gapMaxMs stay at -1, which the struct defines as "not measured". This transport is stream-only -- the radio is told where to send, then sends -- so there is no request/response exchange to time. Reporting 0 would render as a flawless "< 1 ms" link that was never measured. RadioModel already ignores negative values and leaves m_backendLinkShape.hasRtt disengaged, so the UI omits those fields rather than inventing them. PA TEMPERATURE Protocol 2's entire analog payload is six fields (OutHighPriority.c:145-156) and none of them is a temperature. No client can ever populate that readout for this radio, so the honest change is to withdraw it rather than leave a placeholder that cannot resolve. A new RadioCapabilities::paTelemetryAudit record carries that claim. It is a statement about the WIRE FORMAT -- "the protocol defines no such field" -- as distinct from the existing hasPaTemperatureTelemetry, which says only that nothing has been received. The status bar withdraws the row when the record is engaged and no PA-current reading is available to occupy the same row instead; the whole stack comes down when neither of its rows survives, because reserveTelemetryStack() pins the container's width and hiding only the children would leave a reserved empty gap between two separators. The record defaults to std::nullopt, meaning NOT AUDITED, so every other family keeps its present behaviour exactly and only this backend declares it. That default is the point: it does not silently reclassify an existing `false` as "proven absent". A record and not another bool because the RadioCapabilities boolean count is frozen and shrink-only (#5262 M2) -- a new bool fails check_capability_records.py outright -- and because the audit can grow a current or supply field later without a second ratchet argument. SUPPLY VOLTAGE: DECODED, NOT PUBLISHED The radio does send a supply rail (AIN6, high-priority status bytes 49-50). This decodes it and pins the decode with a test, so the wire format is recorded rather than rediscovered. Nothing displays a voltage. Raw counts become volts only against the RF board's ADC reference, and nothing the radio reports identifies it: board type is a p2app launch option rather than a fact about the hardware, which is why DiscoveryReply::isSaturn() already carries an explicit warning that it must not gate backend behaviour. The candidate references differ by a factor of 1.5, so choosing one would show roughly 21 V for a 13.8 V rail on the wrong board. A confidently wrong voltage is worse than no readout, so the reference has to come from the operator or from a load-bearing field before a reading can be offered. EVIDENCE 627/627 ctest, the fifteen checks in the Static checks job run locally, and seven mutations -- every new claim reverted in turn, each killing its test: the supply decode read one byte late; receive traffic counted after the parse instead of before; sent bytes not accumulated; start() not resetting the session; the local endpoint never reported; the audit never declared; and LinkStats claiming a measured transport while idle. Confirmed on the radio: against a live ANAN-G2, the status bar's "Network:" field reports a quality verdict instead of staying blank, and the PA row is withdrawn rather than sitting on its placeholder. ctest cannot reach either of those -- one needs a real transport to measure and the other needs the status bar built against a connected backend. Co-Authored-By: Claude Opus 5 --- docs/architecture/radio-capabilities-map.md | 1 + src/core/backends/RadioCapabilities.h | 22 +++ src/core/backends/anan/AnanBackend.cpp | 72 +++++++- src/core/backends/anan/AnanBackend.h | 20 +++ src/core/backends/anan/P2Client.cpp | 81 +++++++-- src/core/backends/anan/P2Client.h | 49 ++++++ src/core/backends/anan/P2Protocol.cpp | 2 + src/core/backends/anan/P2Protocol.h | 19 +- src/gui/MainWindow.cpp | 49 +++++- src/gui/MainWindow.h | 5 + tests/anan_backend_test.cpp | 33 ++++ tests/anan_link_telemetry_test.cpp | 181 ++++++++++++++++++++ tests/anan_p2_protocol_test.cpp | 18 +- tests/tests.cmake | 12 ++ 14 files changed, 535 insertions(+), 29 deletions(-) create mode 100644 tests/anan_link_telemetry_test.cpp diff --git a/docs/architecture/radio-capabilities-map.md b/docs/architecture/radio-capabilities-map.md index a16206d740..24f0383b14 100644 --- a/docs/architecture/radio-capabilities-map.md +++ b/docs/architecture/radio-capabilities-map.md @@ -204,6 +204,7 @@ the production dialog and checks its displayed peak level and frequency. | `hasSupplyVoltageTelemetry` | ✅ | ❌ | ❌ | `MainWindow::applyCapabilitiesToUi` | PA supply-voltage readout in the status bar. Icom: ✅ only when the active model profile provides a calibrated Vd curve; unprofiled models neither publish nor poll Vd/Id | | `hasPaTemperatureTelemetry` | ✅ | ✅ | ❌ | `MainWindow::applyCapabilitiesToUi` | PA-temperature gauge and °C/°F selector in Radio Vitals. Icom: ❌ until a model profile declares and implements a PA-temperature meter; the IC-9700 has no such declared telemetry | | `hasPaCurrentTelemetry` | ❌ | ❌ | ❌ | `MainWindow::applyCapabilitiesToUi` | Calibrated PA drain-current face in Radio Vitals, used only when PA-temperature telemetry is unavailable. Icom: ✅ only for the IC-9700 profile's documented 0–20 A Id calibration. Flex remains ❌ because its PACURRENT meter is known to clip below real full-power draw | +| `paTelemetryAudit` (record) | — (absent) | — (absent) | — (absent) | `MainWindow::applyCapabilitiesToUi` | A PROTOCOL audit of the PA telemetry the wire format can carry at all, as opposed to what a session has so far RECEIVED. ENGAGED with `temperatureAbsent` means the payload was read field by field and there is no temperature in it, so the status bar WITHDRAWS its PA temperature row instead of leaving it on `PA --°C` forever. ABSENT (the default) means NOT AUDITED and every consumer keeps its historical presentation — which is what holds Flex, HL2, Icom, Sim and RTL at today's behaviour rather than reclassifying a default `false` above as "proven absent". ANAN: engaged, because Protocol 2's whole analog payload is six fields and none is a temperature. A record and not a bool per #5262 M2, whose boolean count is frozen and shrink-only; the audit can also grow a current/supply field without a second ratchet argument | | `hasMainFanTelemetry` | ✅ | ❌ | ❌ | `MainWindow::applyCapabilitiesToUi` | Main Fan gauge in Radio Vitals. All current Icom models are ❌ because the backend does not publish fan-speed telemetry | | `hostFrequencyCalibration` | ❌ | ✅ | ❌ | `RadioSetupDialog` (Calibration page), `AutomationServer::doFreqCal` | Shows the Calibration page and enables the `freqcal` bridge verb. Means "**the client** owns the frequency-error correction", not "this radio has an error" — every radio does. Flex is ❌ because it calibrates itself (`radio set cal_freq` / `pll_start`), and that surface stays in the Frequency Offset group on the Receive page. HL2 is ✅ because its 76.8 MHz NCO scale is a `localparam` in the bitstream (`radio.v` M2) and no register in the HPSDR map accepts a correction — see `docs/architecture/hl2-frequency-calibration.md` | | `hostDroopCalibration` | ❌ | ❌ | ❌ | `RadioSetupDialog` (Droop Correction page), `AutomationServer::doDroopCal` | Shows the Droop Correction page and enables the `droopcal` bridge verb. Means "the client has measured and can correct a real DDC edge droop on this radio", not "this radio has no droop" — HL2's own DDC decimation chain looks architecturally similar and has never been characterised, so its ❌ is "not yet measured", not "known absent". ANAN: ✅ — the Saturn FPGA's DDC0 CIC/decimation chain has a real, bench-measured sin(x)/x droop near the edges of the displayed span (`AnanDroopCorrection.h`), corrected in-app via `AnanDroopCalibrator` | diff --git a/src/core/backends/RadioCapabilities.h b/src/core/backends/RadioCapabilities.h index b63fe79375..d94634dac5 100644 --- a/src/core/backends/RadioCapabilities.h +++ b/src/core/backends/RadioCapabilities.h @@ -544,6 +544,28 @@ struct RadioCapabilities { // because some radios define PACURRENT with an unusable/clipped range. bool hasPaCurrentTelemetry = false; + // A PROTOCOL audit of the PA telemetry the radio's wire format can carry at + // all, as distinct from the three flags above, which report what a session + // has so far RECEIVED. Engaged means the payload was read field by field and + // the absence is a property of the protocol rather than of a radio that has + // simply not sent one yet -- so a readout may be WITHDRAWN instead of left + // printing a placeholder that can never resolve. + // + // std::nullopt (the default) means NOT AUDITED, and every consumer keeps its + // historical presentation. That is deliberate: it holds a family that has + // made no such claim at its existing behaviour instead of quietly + // reclassifying a default `false` above as "proven absent" -- the + // all-defaults-false trap check_capability_records.py was written to stop. + // + // A record rather than another bool because RadioCapabilities is AT its + // frozen boolean count and shrink-only (#5262 M2); per-feature records are + // the sanctioned shape for a new capability. + struct PaTelemetryAudit { + // The protocol defines no PA temperature field anywhere in its payload. + bool temperatureAbsent = false; + }; + std::optional paTelemetryAudit; + // The radio reports main-fan speed as live telemetry. False means the // Radio Vitals applet omits the fan gauge instead of presenting an // instrument that can never receive a sample. This is independent of PA diff --git a/src/core/backends/anan/AnanBackend.cpp b/src/core/backends/anan/AnanBackend.cpp index 9502a86149..d2d8dfe754 100644 --- a/src/core/backends/anan/AnanBackend.cpp +++ b/src/core/backends/anan/AnanBackend.cpp @@ -254,12 +254,12 @@ AnanBackend::AnanBackend(QObject* parent) }); connect(m_client, &P2Client::connectionError, this, [this](const QString& reason) { emit connectionError(reason); }); - connect(m_client, &P2Client::dropsUpdated, this, [](quint64) { - // No LinkStats wiring in this phase -- see the design plan's - // "explicitly not in this commit" list. Connected so the signal has - // a receiver rather than going nowhere; a future commit can surface - // it through IRadioBackend::linkStats(). - }); + // Drops reach the readout through the counters snapshot below, which + // carries the same total on a fixed cadence. Not wired per-drop: the + // LinkStats tick is deliberately periodic rather than event-driven, so a + // link that has gone silent still reports. + connect(m_client, &P2Client::linkCountersUpdated, this, + &AnanBackend::onLinkCounters); // Only DDC0 feeds this DSP. Like ddc0IqReady above, notification and // processing run directly on the I/O thread, with the DSP as context. // Invalidate its partial FFT before the discontinuous block arrives. @@ -405,6 +405,25 @@ RadioCapabilities AnanBackend::capabilities() const c.hasAgcThreshold = true; // Host receiver DSP implements threshold/off gain. c.manufacturer = QStringLiteral("Apache Labs"); c.model = QStringLiteral("ANAN-G2"); + // ---- PA telemetry ---- + // hasSupplyVoltageTelemetry stays FALSE even though the radio does send a + // supply-rail reading (P2Protocol decodes it). Raw ADC counts become volts + // only against the RF board's ADC reference -- 5 V on one board, 3.3 V on + // another, a 1.5x difference that would read 21 V for a 13.8 V rail -- and + // NOTHING THE RADIO REPORTS IDENTIFIES WHICH. The board id cannot answer + // it: see DiscoveryReply::isSaturn(), which is explicitly a discovery-time + // picker filter and not for gating backend behaviour, because board type is + // a p2app launch option rather than a fact about the hardware. So the row + // stays withdrawn until the reference comes from somewhere load-bearing or + // from the operator; a confidently wrong voltage is worse than none. + // + // There is NO PA temperature in this protocol. The radio's whole analog + // payload is six fields and none of them is a temperature, so the readout + // can never resolve and is withdrawn rather than left showing a placeholder. + // hasPaTemperatureTelemetry stays false on its own (nothing is received); + // this says the stronger thing -- that nothing CAN be. + c.paTelemetryAudit = RadioCapabilities::PaTelemetryAudit{ + /*temperatureAbsent=*/true}; c.canCreateSlices = false; c.maxSlices = 1; c.maxPanadapters = 1; @@ -730,6 +749,12 @@ void AnanBackend::disconnectRadio() m_tuneThrottleTimer->stop(); m_tunePendingApply = false; m_pendingBandwidthKsps = 0; + // The next session's counters restart from zero in P2Client::start(), so a + // cached snapshot from this one would make the first tick of the next read + // as a huge backward jump in every total. + m_linkCounters = {}; + m_linkCountersSeen = false; + m_lastSnapshotRxBytes = 0; // A genuine operator-initiated disconnect always fires disconnected(), // even one that lands mid-rate-change -- this is not the zoom case // m_rateChanging exists to hide. @@ -1405,6 +1430,41 @@ void AnanBackend::defineMeters() emit meterDefined(d); } +IRadioBackend::LinkStats AnanBackend::linkStats() const +{ + LinkStats s; + // Nothing measured yet: the consumer keeps whatever source it already had + // rather than being handed a snapshot of zeros. + if (!m_connected || !m_linkCountersSeen) + return s; + s.reported = true; + s.alive = m_linkCounters.rxBytes > m_lastSnapshotRxBytes; + s.rxBytes = static_cast(m_linkCounters.rxBytes); + s.txBytes = static_cast(m_linkCounters.txBytes); + s.rxPackets = m_linkCounters.rxPackets; + s.rxPacketsLost = m_linkCounters.drops; + s.localEndpoint = m_linkCounters.localEndpoint; + // LEFT AT -1, WHICH THE STRUCT DEFINES AS "NOT MEASURED" AND THE MODEL + // HONOURS BY IGNORING. This transport is stream-only: the radio is told + // where to send and then sends, with no request/response exchange to time, + // so there is no round trip here to measure. Reporting 0 instead would + // render as a perfect "< 1 ms" link -- a measurement never taken. + // rttMs, jitterMs, gapMs and gapMaxMs all keep their -1 defaults. + return s; +} + +void AnanBackend::onLinkCounters(const P2Client::LinkCounters& counters) +{ + // First snapshot of a session compares against ZERO, not against itself: + // P2Client::start() resets its counters, and comparing a first snapshot to + // itself would report a dead link on the very tick after a connect that + // only happened because DDC0 frames arrived. + m_lastSnapshotRxBytes = m_linkCountersSeen ? m_linkCounters.rxBytes : 0; + m_linkCounters = counters; + m_linkCountersSeen = true; + emit linkStatsUpdated(linkStats()); +} + void AnanBackend::onDspMeter(float dbfs) { const double dbm = static_cast(dbfs) + kSMeterDbmOffset + m_attenuationDb; diff --git a/src/core/backends/anan/AnanBackend.h b/src/core/backends/anan/AnanBackend.h index c429b2e6a0..edbe1a26ac 100644 --- a/src/core/backends/anan/AnanBackend.h +++ b/src/core/backends/anan/AnanBackend.h @@ -92,6 +92,11 @@ class AnanBackend : public IRadioBackend { void setKeying(bool key, const AetherSDR::TxCoordinator::Operation& operation, const AetherSDR::TxCoordinator::Completion& completion = {}) override; void invokeExtension(const QString& ns, const QString& verb, quint64 requestId, const QVariant& arg = {}) override; + // Transport snapshot for the status bar's Network field and Network + // Diagnostics. Synchronous, and reads only the cache onLinkCounters() keeps + // on this thread -- never the client's own counters, which belong to the + // I/O thread. The push half is onLinkCounters(), on the client's tick. + [[nodiscard]] LinkStats linkStats() const override; // ---- pure-function pieces, exposed static for testability ---- // (matches the WdspChannel/Hl2RxDsp precedent of exposing normally- @@ -183,6 +188,21 @@ class AnanBackend : public IRadioBackend { // Declares SLC:LEVEL to the meter seam; on every connect, before the // first reading can arrive. See its definition. void defineMeters(); + // One client snapshot -> one LinkStats push. The FIXED cadence is the + // client's timer, not ours: this runs on whatever the client published, and + // it must keep arriving after the radio goes quiet, because "nothing came + // this second" is the observation the heartbeat's alarm path waits for + // (IRadioBackend::LinkStats). + void onLinkCounters(const P2Client::LinkCounters& counters); + // The last snapshot the client published, cached on THIS thread so the + // synchronous linkStats() getter never reads the I/O thread's counters -- + // see P2Client::LinkCounters for why that read would be a race. + P2Client::LinkCounters m_linkCounters; + bool m_linkCountersSeen = false; + // rxBytes at the previous snapshot, for LinkStats::alive -- a link that is + // bound and counting but has gone quiet must read as dead, which a + // cumulative total alone cannot say. + quint64 m_lastSnapshotRxBytes = 0; // One WDSP S-meter reading (dBFS) -> dBm, smoothed, published on a tick. void onDspMeter(float dbfs); // The same smoother Hl2Backend publishes through, so the two receivers' diff --git a/src/core/backends/anan/P2Client.cpp b/src/core/backends/anan/P2Client.cpp index e06ca1a17a..3333cb0994 100644 --- a/src/core/backends/anan/P2Client.cpp +++ b/src/core/backends/anan/P2Client.cpp @@ -24,12 +24,21 @@ std::span asBytes(const QByteArray& d) noexcept static_cast(d.size())}; } +// txAccum is REQUIRED, not defaulted: a new send site cannot compile without +// deciding where its bytes are counted, which is what keeps the LinkStats tx +// total from silently under-reporting as this client grows. template qint64 sendTo(QUdpSocket& s, const std::array& buf, - const QHostAddress& host, quint16 port) + const QHostAddress& host, quint16 port, quint64& txAccum) { - return s.writeDatagram(reinterpret_cast(buf.data()), - static_cast(N), host, port); + const qint64 sent = s.writeDatagram(reinterpret_cast(buf.data()), + static_cast(N), host, port); + // Only what the socket ACCEPTED. Counting a refused datagram would inflate + // the readout exactly when the link is failing, which is when an operator + // is most likely to be reading it. + if (sent > 0) + txAccum += static_cast(sent); + return sent; } } // namespace @@ -52,6 +61,10 @@ P2Client::P2Client(QObject* parent) : QObject(parent) // there is no reason to hand it more than necessary. m_speakerDrainTimer->setTimerType(Qt::PreciseTimer); connect(m_speakerDrainTimer, &QTimer::timeout, this, &P2Client::onSpeakerDrainTick); + + m_linkCountersTimer = new QTimer(this); + m_linkCountersTimer->setInterval(kLinkCountersMs); + connect(m_linkCountersTimer, &QTimer::timeout, this, &P2Client::publishLinkCounters); } P2Client::~P2Client() @@ -88,6 +101,9 @@ bool P2Client::start(const Params& params, int connectTimeoutMs) m_expectedSeq.fill(std::nullopt); m_activeDdcCount = 1; // real value set once the DDC list is resolved below m_drops = 0; + m_rxBytes = 0; + m_rxPackets = 0; + m_txBytes = 0; m_warnedUnexpectedPorts.clear(); m_linkUp = false; m_discoveryInfoSent = false; @@ -111,8 +127,8 @@ bool P2Client::start(const Params& params, int connectTimeoutMs) // sends DDC0 IQ, not the source port of General/DDC-Specific/High- // Priority below. The reply (if any arrives here) is not parsed; // onReadyRead() already drops anything that isn't DDC0-shaped. - sendTo(*m_socket, buildDiscovery(), m_host, kRadioPort); - sendTo(*m_socket, buildGeneral(), m_host, kRadioPort); + sendTo(*m_socket, buildDiscovery(), m_host, kRadioPort, m_txBytes); + sendTo(*m_socket, buildGeneral(), m_host, kRadioPort, m_txBytes); // Resolve the session's DDC list ONCE: either the caller's explicit // multi-DDC list, or a one-element list from the DDC0 shorthand. See @@ -141,13 +157,14 @@ bool P2Client::start(const Params& params, int connectTimeoutMs) sendTo(*m_socket, buildDdcSpecific(ddcs, /*numAdcs=*/2, params.ditherEnabled, params.randomEnabled), - m_host, kDdcSpecificPort); + m_host, kDdcSpecificPort, m_txBytes); sendTo(*m_socket, buildHighPriority(true, freqWords, m_bypassAdc0Filters, m_bypassAdc1Filters, m_adc0AttenuationDb, m_adc1AttenuationDb), - m_host, kHighPriorityPort); + m_host, kHighPriorityPort, m_txBytes); m_keepaliveTimer->start(); + m_linkCountersTimer->start(); if (m_speakerAudioEnabled) { // Started from zero at the same moment as the drain, so the pacer's // first advance() measures from the first tick rather than from @@ -163,6 +180,7 @@ bool P2Client::start(const Params& params, int connectTimeoutMs) void P2Client::stop() { m_keepaliveTimer->stop(); + m_linkCountersTimer->stop(); m_connectTimeoutTimer->stop(); m_speakerDrainTimer->stop(); // Dropped, not drained. These samples are older than the disconnect and the @@ -181,7 +199,7 @@ void P2Client::stop() sendTo(*m_socket, buildHighPriority(false, 0, m_bypassAdc0Filters, m_bypassAdc1Filters, m_adc0AttenuationDb, m_adc1AttenuationDb), - m_host, kHighPriorityPort); + m_host, kHighPriorityPort, m_txBytes); m_socket->close(); m_socket->deleteLater(); m_socket = nullptr; @@ -193,6 +211,34 @@ void P2Client::stop() } } +void P2Client::publishLinkCounters() +{ + LinkCounters c; + c.rxBytes = m_rxBytes; + c.rxPackets = m_rxPackets; + c.txBytes = m_txBytes; + c.drops = m_drops; + c.localEndpoint = localEndpoint(); + emit linkCountersUpdated(c); +} + +QString P2Client::localEndpoint() const +{ + // Empty rather than a partial string when there is no socket or the bind + // did not take: LinkStats::localEndpoint is documented as empty when not + // bound, and the consumer falls back to its own source on empty. + if (!m_socket || m_socket->state() == QAbstractSocket::UnconnectedState) + return {}; + const quint16 port = m_socket->localPort(); + if (port == 0) + return {}; + // AnyIPv4 prints as "0.0.0.0", which is the honest answer for a wildcard + // bind -- the socket genuinely is not pinned to one interface. The PORT is + // the load-bearing half anyway: it is what the radio streams DDC0 to. + return QStringLiteral("%1:%2").arg(m_socket->localAddress().toString()) + .arg(port); +} + void P2Client::noteSpeakerFifoStatus(const HighPriorityStatus& status) { m_lastSpeakerFifoLevel = status.speakerFifoLevel; @@ -309,6 +355,10 @@ void P2Client::onSpeakerDrainTick() // stream permanently. Retried on the next tick. break; } + // Counted here rather than in sendTo(): this is the one send that does + // not go through it. Reached only on the accepted path -- the refused + // case broke out above. + m_txBytes += static_cast(sent); ++m_speakerSequence; m_speakerPacer.onPacketSent(); consumed += kPacketSamples; @@ -327,7 +377,7 @@ void P2Client::setDdc0FrequencyHz(double hz) buildHighPriority(true, sharedFreqWords(), m_bypassAdc0Filters, m_bypassAdc1Filters, m_adc0AttenuationDb, m_adc1AttenuationDb), - m_host, kHighPriorityPort); + m_host, kHighPriorityPort, m_txBytes); } void P2Client::setStepAttenuationDb(int adcIndex, int db) @@ -346,7 +396,7 @@ void P2Client::setStepAttenuationDb(int adcIndex, int db) buildHighPriority(true, sharedFreqWords(), m_bypassAdc0Filters, m_bypassAdc1Filters, m_adc0AttenuationDb, m_adc1AttenuationDb), - m_host, kHighPriorityPort); + m_host, kHighPriorityPort, m_txBytes); } } @@ -369,7 +419,7 @@ bool P2Client::setDdcRateLive(int ddcIndex, int rateKsps) sendTo(*m_socket, buildDdcSpecific(m_activeDdcs, /*numAdcs=*/2, m_ditherEnabled, m_randomEnabled), - m_host, kDdcSpecificPort); + m_host, kDdcSpecificPort, m_txBytes); // The stream's sample cadence changes underneath us from here, so the // sequence expectation for THIS DDC is no longer meaningful -- clear it @@ -393,7 +443,7 @@ void P2Client::onKeepaliveTick() buildHighPriority(true, sharedFreqWords(), m_bypassAdc0Filters, m_bypassAdc1Filters, m_adc0AttenuationDb, m_adc1AttenuationDb), - m_host, kHighPriorityPort); + m_host, kHighPriorityPort, m_txBytes); } void P2Client::onConnectTimeout() @@ -415,6 +465,13 @@ void P2Client::onReadyRead() // per-DDC sequence tracking, notification ordering and sample delivery. void P2Client::handleDatagram(std::span bytes, quint16 senderPort) { + // Before any parse decides what this datagram IS: the transport carried it + // either way, and a session whose IQ has stalled while Status packets still + // arrive must not read as a dead link. Counting after the shape checks + // below would make every rejected datagram invisible to the readout. + m_rxBytes += static_cast(bytes.size()); + ++m_rxPackets; + const auto frame = parseDdcFrame(bytes); if (!frame) { // Not DDC0-shaped: Mic Data, High Priority Status, or this session's diff --git a/src/core/backends/anan/P2Client.h b/src/core/backends/anan/P2Client.h index c84f675296..5e589c35bd 100644 --- a/src/core/backends/anan/P2Client.h +++ b/src/core/backends/anan/P2Client.h @@ -121,6 +121,32 @@ class P2Client : public QObject { [[nodiscard]] bool isRunning() const noexcept { return m_running; } [[nodiscard]] quint64 droppedPackets() const noexcept { return m_drops; } + // ---- transport counters, for IRadioBackend::LinkStats ---- + // + // I/O-THREAD ONLY, exactly like droppedPackets() above: every one of these + // is written from handleDatagram()/the send paths on this object's own + // thread, with no lock. The backend lives on another thread and must NOT + // read them directly -- it consumes linkCountersUpdated() below, which is + // delivered to its thread by the queued connection. The LinkStats contract + // says the same thing: a backend with worker-side values caches them on its + // own thread. + // + // Session-cumulative, reset by start(). EVERY datagram the socket handed us + // counts, including Mic Data and Status: this measures the TRANSPORT, not + // the IQ stream, so a session whose DDC traffic has stalled while status + // packets still arrive must not read as a dead link. + struct LinkCounters { + quint64 rxBytes = 0; + quint64 rxPackets = 0; + // Only bytes the socket ACCEPTED; see sendTo() for why a refused + // datagram is not counted. + quint64 txBytes = 0; + quint64 drops = 0; + // "ip:port" of the bound local socket, empty when not bound. The port + // is the one the radio streams DDC0 to (see the class comment). + QString localEndpoint; + }; + signals: void linkUp(); // first valid DDC0 frame seen void linkDown(); // stop() called, or a link failure once that exists @@ -140,6 +166,12 @@ class P2Client : public QObject { // connecting per-DDC signals. void ddcIqReady(int ddcIndex, const std::vector>& block); void dropsUpdated(quint64 totalDrops); + // A transport snapshot, on a FIXED cadence while running rather than when + // traffic arrives -- the tick has to keep coming after the radio goes + // quiet, because "nothing arrived this second" is what the heartbeat's + // alarm path is waiting for (IRadioBackend::LinkStats). Emitted from the + // I/O thread; cross-thread consumers get it queued. + void linkCountersUpdated(const P2Client::LinkCounters& counters); // This DDC's next IQ block is discontinuous, including accepted rewinds // and duplicates. Emitted before ddcIqReady/ddc0IqReady in the same // handleDatagram() call so direct consumers can clear partial FFTs first. @@ -239,6 +271,16 @@ private slots: // always reported) -- an operator watching for a lossy link cares that // the session is dropping, not which receiver. quint64 m_drops = 0; + // What each counts, and why they are I/O-thread-only, is on LinkCounters. + quint64 m_rxBytes = 0; + quint64 m_rxPackets = 0; + quint64 m_txBytes = 0; + // Matches the status bar's own refresh rate. Faster would publish snapshots + // no one reads; slower would delay the heartbeat's silence verdict. + static constexpr int kLinkCountersMs = 1000; + QTimer* m_linkCountersTimer = nullptr; + void publishLinkCounters(); + [[nodiscard]] QString localEndpoint() const; // How many DDCs this session enabled, so onReadyRead() knows which // sender ports belong to it. Kept alongside m_activeDdcs (rather than @@ -269,3 +311,10 @@ private slots: }; } // namespace AetherSDR::anan + +// Not a seam payload (IRadioBackend.h rule 4 covers those): this one stays +// inside the ANAN backend, client -> backend. Declared anyway because Qt 6's +// function-pointer queued delivery works without it but the NAME-based paths do +// not -- QSignalSpy in the tests being the one that would otherwise capture +// nothing and pass vacuously. +Q_DECLARE_METATYPE(AetherSDR::anan::P2Client::LinkCounters) diff --git a/src/core/backends/anan/P2Protocol.cpp b/src/core/backends/anan/P2Protocol.cpp index b48bff6a1c..755cf6e731 100644 --- a/src/core/backends/anan/P2Protocol.cpp +++ b/src/core/backends/anan/P2Protocol.cpp @@ -217,6 +217,8 @@ std::optional parseHighPriorityStatus( s.seq = readU32be(data.data()); s.speakerUnderflow = (data[30] & 0b0000'1000) != 0; s.speakerFifoLevel = readU16be(data.data() + 37); + // In bounds by the exact-size guard above, which admits only 60 bytes. + s.supplyVoltageRaw = readU16be(data.data() + 49); return s; } diff --git a/src/core/backends/anan/P2Protocol.h b/src/core/backends/anan/P2Protocol.h index ad1a51e3f8..c3eb05683b 100644 --- a/src/core/backends/anan/P2Protocol.h +++ b/src/core/backends/anan/P2Protocol.h @@ -237,8 +237,8 @@ std::array buildSpeakerAudio( std::uint32_t sequence, std::span interleavedLr) noexcept; // ---- High Priority Status, radio -> PC (spec p.47) ---- -// 60 bytes: 4-byte BE sequence, then hardware state. Only the two speaker-stream -// fields are decoded. Arrives on the same socket as DDC0 IQ (see +// 60 bytes: 4-byte BE sequence, then hardware state. The two speaker-stream +// fields and the supply rail are decoded. Arrives on the same socket as DDC0 IQ (see // kDdc0DefaultPort); parseDdcFrame() rejects it. inline constexpr std::size_t kHighPriorityStatusBytes = 60; @@ -251,6 +251,21 @@ struct HighPriorityStatus { // "2 samples per location" doubling never reaches the send, OutHighPriority.c). // Not comparable to kSpeakerFramesPerPacket; use as a trend. std::uint16_t speakerFifoLevel = 0; + // Bytes 49-50: the PA supply rail, AIN6, in RAW 12-bit ADC counts (0-4095). + // The only analog field here a receive-only client could use -- the other + // five (forward/reverse/exciter power, two user analogs) are transmit-side + // or unassigned, and there is NO TEMPERATURE anywhere in the payload, which + // is why no client can populate a PA temperature readout for this radio. + // + // DECODED BUT NOT YET CONSUMED, deliberately. Counts become volts only + // against the RF board's ADC reference, and nothing the radio reports + // identifies it: board type is a p2app launch option, not a hardware fact + // (see DiscoveryReply::isSaturn()). Publishing a scaled value would mean + // picking a reference at a 1.5x spread -- 21 V shown for a 13.8 V rail if + // wrong. The decode is pinned here so the wire format is recorded and + // tested; the reference has to come from the operator or from a + // load-bearing field before a reading can be offered. + std::uint16_t supplyVoltageRaw = 0; }; // Decode, or nullopt if this is not a status packet. Bounds-checked diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 8524e774a0..6881a15ca2 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -5992,6 +5992,7 @@ void MainWindow::buildUI() // PA temp (top) + supply voltage (bottom) stacked auto* paStack = new QWidget; + m_paStack = paStack; reserveTelemetryStack(paStack, { QStringLiteral("PA 248.0°F"), QStringLiteral("PA 120.0°C"), @@ -7768,18 +7769,41 @@ void MainWindow::applyCapabilitiesToUi(bool connected, const RadioCapabilities& m_appletPanel->setHardwareEqVisible(true); } - // ── PA supply voltage: the lower row of the status bar's PA stack ─────── + // ── The status bar's PA stack: temperature on top, supply rail below ─── // - // m_supplyVoltLabel ONLY. m_paTempLabel and the paVbox around them are left - // alone deliberately: an HL2 reports PA temperature genuinely, so hiding the - // stack would delete a working readout in order to suppress a broken - // sibling. The one being suppressed is fed from the Flex-named "+13.8A" - // meter, and MeterModel emits hwTelemetryChanged whenever EITHER half - // changes — so on a radio that reports only PA temperature the volts half - // arrives as its 0.0f initialiser on every tick. + // BOTH rows are now gated, each on its own evidence, and the stack itself + // comes down when neither survives. + // + // The volts row is suppressed on hasSupplyVoltageTelemetry because it is fed + // from the Flex-named "+13.8A" meter and MeterModel emits + // hwTelemetryChanged whenever EITHER half changes — so on a radio that + // reports only PA temperature the volts half arrives as its 0.0f + // initialiser on every tick. + // + // The temperature row was previously left alone on the grounds that hiding + // the stack would delete the HL2's genuine reading to suppress a broken + // sibling. That reasoning held against hiding the STACK; it does not apply + // to a per-radio gate, which is what this is. The row is withdrawn only + // when the radio's own capability record says the protocol HAS no + // temperature field — a claim about the wire format, not about a value that + // has yet to arrive — and only when no PA-current reading is available to + // occupy the same row. Radios that report a temperature, or that have made + // no such claim, are untouched by construction. if (m_appletPanel) { m_appletPanel->meterApplet()->setSupplyVoltageTelemetryState(connected); } + // A readout the radio can never produce, as distinct from one that has not + // arrived yet. The two capability flags are re-checked so a real reading + // always wins over the audit if a backend ever declared both. + const bool paTemperatureWithdrawn = + connected + && caps.paTelemetryAudit.has_value() + && caps.paTelemetryAudit->temperatureAbsent + && !caps.hasPaTemperatureTelemetry + && !caps.hasPaCurrentTelemetry; + if (m_paTempLabel) { + m_paTempLabel->setVisible(!paTemperatureWithdrawn); + } if (m_supplyVoltLabel) { m_supplyVoltLabel->setVisible(!connected || caps.hasSupplyVoltageTelemetry); if (!connected) { @@ -7806,6 +7830,15 @@ void MainWindow::applyCapabilitiesToUi(bool connected, const RadioCapabilities& // "PA 248.0°F" is the wider sample. updateStatusBarMinimumWidth(); } + // Both rows withdrawn: take the container down too. Its minimum width is + // PINNED by reserveTelemetryStack(), so hiding only the children would + // leave a reserved, empty gap sitting between two separators. + if (m_paStack) { + const bool anyPaRow = + (m_paTempLabel && m_paTempLabel->isVisibleTo(m_paStack)) + || (m_supplyVoltLabel && m_supplyVoltLabel->isVisibleTo(m_paStack)); + m_paStack->setVisible(anyPaRow); + } // CWX/CWK, DVK and FDX status-bar toggles are HIDDEN without the capability: // each is a firmware verb (`cwx`, `dvk`, `radio set full_duplex_enabled=`) diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index 4b283e995f..f22e131e75 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -1727,6 +1727,11 @@ private slots: QTimer* m_cpuTimer{nullptr}; QLabel* m_paTempLabel{nullptr}; QLabel* m_supplyVoltLabel{nullptr}; + // The container holding the two labels above. Held so the whole stack can + // come down when BOTH its rows are withdrawn: reserveTelemetryStack() pins + // its minimum width, so hiding only the children would leave a reserved + // empty gap between two separators. + QWidget* m_paStack{nullptr}; QLabel* m_networkLabel{nullptr}; QTimer m_networkTooltipRefreshTimer; QTimer m_perfHeartbeatTimer; diff --git a/tests/anan_backend_test.cpp b/tests/anan_backend_test.cpp index 481a1e30b9..a7fc8b64fb 100644 --- a/tests/anan_backend_test.cpp +++ b/tests/anan_backend_test.cpp @@ -506,6 +506,39 @@ int main(int argc, char** argv) check(!last.has_value(), "balance publishes nothing, having no delta field"); } + // ---- PA telemetry capability: the readout is withdrawn, not blanked ---- + { + AnanBackend backend; + const auto caps = backend.capabilities(); + check(caps.paTelemetryAudit.has_value(), + "ANAN declares a PA telemetry audit"); + check(caps.paTelemetryAudit && caps.paTelemetryAudit->temperatureAbsent, + "the audit states PA temperature is ABSENT FROM THE PROTOCOL -- the" + " claim the status bar withdraws its readout on"); + check(!caps.hasPaTemperatureTelemetry, + "and no temperature is claimed as received"); + // The radio DOES send a supply reading, but raw counts become volts only + // against an ADC reference nothing in the protocol identifies, so the row + // stays withdrawn rather than showing a value at a 1.5x guess. + check(!caps.hasSupplyVoltageTelemetry, + "no supply voltage is claimed while its ADC reference is unknown"); + check(!caps.hasPaCurrentTelemetry, + "no PA drain current is claimed"); + } + + // ---- LinkStats: silence before the first snapshot, never a zeroed one ---- + { + AnanBackend backend; + const auto idle = backend.linkStats(); + check(!idle.reported, + "an unconnected backend reports NO transport, so the consumer keeps" + " its own source instead of being handed zeros"); + check(idle.rttMs < 0 && idle.jitterMs < 0 && idle.gapMs < 0 + && idle.gapMaxMs < 0, + "every timing field defaults to the struct's negative" + " \"not measured\" sentinel, which must not render as zero"); + } + if (g_failures == 0) std::fprintf(stderr, "anan_backend_test: all checks passed\n"); return g_failures == 0 ? 0 : 1; diff --git a/tests/anan_link_telemetry_test.cpp b/tests/anan_link_telemetry_test.cpp new file mode 100644 index 0000000000..0bd203eaa0 --- /dev/null +++ b/tests/anan_link_telemetry_test.cpp @@ -0,0 +1,181 @@ +// aetherd ANAN P2 -- transport counters and the LinkStats snapshot. +// +// The status bar's "Network:" field was blank on an ANAN because the backend +// published no LinkStats at all. The counters behind it live on P2Client, are +// written only from its own I/O thread, and reach the backend as a periodic +// snapshot rather than a cross-thread read. What this pins: +// +// * EVERY datagram counts, including the ones every parser rejects. The +// readout describes the TRANSPORT, so a session whose IQ has stalled while +// Status packets still arrive must not read as a dead link -- and counting +// after the shape checks would make exactly those datagrams invisible. +// * Sent bytes are counted on every send path, including the speaker stream, +// which does not go through sendTo(). +// * start() resets the session, so a reconnect does not inherit totals. +// +// Loopback send only (127.0.0.1), no radio and no listener: the assertions are +// about the client's own counters, not about what arrives. + +#include "core/backends/anan/P2Client.h" +#include "core/backends/anan/P2Protocol.h" + +#include +#include + +#include +#include +#include +#include + +using namespace AetherSDR::anan; + +static int g_failures = 0; +static void check(bool cond, const char* what) +{ + if (!cond) { + std::fprintf(stderr, "FAIL: %s\n", what); + ++g_failures; + } +} + +namespace AetherSDR::anan { +// P2Client declares this a friend precisely so the counters can be pinned +// without widening its public surface (same pattern as the sibling ANAN tests). +struct P2ClientTestAccess { + static void feedDatagram(P2Client& c, std::span bytes, + quint16 port) + { + c.handleDatagram(bytes, port); + } + static void publish(P2Client& c) { c.publishLinkCounters(); } + static quint64 rxBytes(const P2Client& c) { return c.m_rxBytes; } + static quint64 rxPackets(const P2Client& c) { return c.m_rxPackets; } + static quint64 txBytes(const P2Client& c) { return c.m_txBytes; } +}; +} // namespace AetherSDR::anan + +static P2Client::Params loopbackParams() +{ + P2Client::Params p; + p.host = QStringLiteral("127.0.0.1"); + p.speakerAudioEnabled = false; + return p; +} + +int main(int argc, char** argv) +{ + QCoreApplication app(argc, argv); + // QSignalSpy records the argument by metatype. Registered explicitly so a + // capture failure can never present as an empty-but-passing assertion. + qRegisterMetaType(); + + // ---- a fresh client has measured nothing ---- + { + P2Client client; + check(P2ClientTestAccess::rxBytes(client) == 0 + && P2ClientTestAccess::rxPackets(client) == 0 + && P2ClientTestAccess::txBytes(client) == 0, + "a client that has not run has all counters at zero"); + } + + // ---- every datagram counts, including ones the parsers reject ---- + { + P2Client client; + + // A High Priority Status packet: 60 bytes, not DDC-shaped, so + // handleDatagram() returns without ever reaching the IQ path. + std::vector status(kHighPriorityStatusBytes, 0); + status[30] = 0b0000'1000; + P2ClientTestAccess::feedDatagram(client, status, 1025); + check(P2ClientTestAccess::rxPackets(client) == 1 + && P2ClientTestAccess::rxBytes(client) == kHighPriorityStatusBytes, + "a Status packet counts: it is transport traffic, not IQ"); + + // Garbage too short to be any known packet. THIS is the case that would + // vanish if counting happened after the shape checks instead of before. + const std::vector runt{0x01, 0x02, 0x03}; + P2ClientTestAccess::feedDatagram(client, runt, 1025); + check(P2ClientTestAccess::rxPackets(client) == 2 + && P2ClientTestAccess::rxBytes(client) + == kHighPriorityStatusBytes + runt.size(), + "a datagram every parser rejects still counts as traffic arrived"); + + // DDC-shaped but from a port this session never enabled: dropped WITHOUT + // counting a sequence drop, yet the bytes did cross the wire. + std::vector ddcish(kDdcHeaderLen + kDdcSampleBytes, 0); + ddcish[14] = 0x00; ddcish[15] = 0x01; // one sample declared + const quint64 beforeStray = P2ClientTestAccess::rxBytes(client); + P2ClientTestAccess::feedDatagram(client, ddcish, 9999); + check(P2ClientTestAccess::rxBytes(client) > beforeStray + && P2ClientTestAccess::rxPackets(client) == 3, + "a datagram from an unexpected sender port counts as traffic too"); + + check(client.droppedPackets() == 0, + "...and none of those is a sequence drop -- the two counters are" + " independent"); + } + + // ---- the snapshot carries the counters, and is what crosses threads ---- + { + P2Client client; + std::vector status(kHighPriorityStatusBytes, 0); + P2ClientTestAccess::feedDatagram(client, status, 1025); + + QSignalSpy snapshots(&client, &P2Client::linkCountersUpdated); + P2ClientTestAccess::publish(client); + check(snapshots.count() == 1, "publishing emits exactly one snapshot"); + if (snapshots.count() == 1) { + const auto counters = + snapshots.at(0).at(0).value(); + check(counters.rxPackets == 1 + && counters.rxBytes == kHighPriorityStatusBytes, + "the snapshot carries the receive counters as the client has" + " them"); + check(counters.drops == 0, "and the drop total alongside them"); + } + } + + // ---- sent bytes are counted, and start() resets the session ---- + { + P2Client client; + + // Traffic from a PREVIOUS notional session, which must not survive. + std::vector status(kHighPriorityStatusBytes, 0); + P2ClientTestAccess::feedDatagram(client, status, 1025); + check(P2ClientTestAccess::rxBytes(client) > 0, "pre-start traffic counted"); + + if (!client.start(loopbackParams())) { + std::fprintf(stderr, + "anan_link_telemetry_test: SKIP -- client could not bind" + " a local UDP socket\n"); + return 77; // SKIP_RETURN_CODE in tests.cmake + } + + check(P2ClientTestAccess::rxBytes(client) == 0 + && P2ClientTestAccess::rxPackets(client) == 0, + "start() resets the receive counters, so a reconnect does not" + " inherit the last session's totals"); + + // start() sends its own startup sequence (Discovery, General, + // DDC-Specific, High Priority) before returning. + check(P2ClientTestAccess::txBytes(client) > 0, + "the startup sequence is counted as sent bytes"); + + const QString endpoint = [&] { + QSignalSpy snapshots(&client, &P2Client::linkCountersUpdated); + P2ClientTestAccess::publish(client); + return snapshots.count() == 1 + ? snapshots.at(0).at(0).value() + .localEndpoint + : QString(); + }(); + check(endpoint.contains(QLatin1Char(':')), + "a running client reports its bound local endpoint as \"ip:port\""); + + client.stop(); + } + + if (g_failures == 0) + std::fprintf(stderr, "anan_link_telemetry_test: all checks passed\n"); + return g_failures == 0 ? 0 : 1; +} diff --git a/tests/anan_p2_protocol_test.cpp b/tests/anan_p2_protocol_test.cpp index 157792ed4d..63aa5f72d4 100644 --- a/tests/anan_p2_protocol_test.cpp +++ b/tests/anan_p2_protocol_test.cpp @@ -494,12 +494,13 @@ int main() check(clipped[258] == 0x00 && clipped[259] == 0x09, "the packet fills to its last byte"); } - // ---- High Priority Status: the two speaker fields ---- + // ---- High Priority Status: the two speaker fields and the supply rail ---- { std::vector pkt(kHighPriorityStatusBytes, 0); pkt[0] = 0x00; pkt[1] = 0x00; pkt[2] = 0x01; pkt[3] = 0x02; // seq 258 pkt[30] = 0b0000'1000; // speaker underflow, bit 3 pkt[37] = 0x01; pkt[38] = 0x40; // level 320, big-endian + pkt[49] = 0x02; pkt[50] = 0x1C; // AIN6 540 counts, big-endian const auto st = parseHighPriorityStatus(pkt); check(st.has_value(), "a 60-byte status packet parses"); @@ -507,8 +508,23 @@ int main() check(st->seq == 258, "the sequence is big-endian"); check(st->speakerUnderflow, "byte 30 bit 3 is the speaker underflow"); check(st->speakerFifoLevel == 320, "bytes 37-38 are the level, big-endian"); + check(st->supplyVoltageRaw == 540, + "bytes 49-50 are the supply rail in raw counts, big-endian"); } + // The byte ORDER is the half a wrong decode gets silently right on small + // values: 540 is 0x021C, so a byte-swapped read gives 0x1C02 = 7170. A + // single-byte read of 49 gives 2. Pinned so neither passes. + check(st && st->supplyVoltageRaw != 7170 && st->supplyVoltageRaw != 2, + "neither a byte-swapped nor a single-byte supply read passes"); + + // A rail the radio reports as zero is a REAL reading (a dead supply), not + // a missing one, and must decode as 0 rather than being treated as absent. + std::vector zeroRail(kHighPriorityStatusBytes, 0); + const auto dead = parseHighPriorityStatus(zeroRail); + check(dead.has_value() && dead->supplyVoltageRaw == 0, + "a zero supply reading decodes as zero, not as no reading"); + // Bit 3 only. Bit 2 is the DUC's underflow and must not read as ours -- // that is the mistake the bitmask exists to prevent. pkt[30] = 0b0000'0100; diff --git a/tests/tests.cmake b/tests/tests.cmake index 369fad67fd..2d0fc5c061 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -1094,6 +1094,18 @@ add_test(NAME anan_speaker_audio_test COMMAND anan_speaker_audio_test) # Without this property that is a green pass with zero checks. set_tests_properties(anan_speaker_audio_test PROPERTIES SKIP_RETURN_CODE 77) +# ANAN transport counters -> IRadioBackend::LinkStats -- what the status bar's +# "Network:" field reads. Counts every datagram the socket hands over, including +# the ones each parser rejects, because the readout describes the TRANSPORT and +# not the IQ stream. Loopback send only, no listener and no radio. +add_executable(anan_link_telemetry_test tests/anan_link_telemetry_test.cpp) +target_include_directories(anan_link_telemetry_test PRIVATE src tests) +target_link_libraries(anan_link_telemetry_test + PRIVATE aethercore Qt6::Core Qt6::Network Qt6::Test) +add_test(NAME anan_link_telemetry_test COMMAND anan_link_telemetry_test) +# Exit 77 == no local UDP socket could be bound, so nothing could be observed. +set_tests_properties(anan_link_telemetry_test PROPERTIES SKIP_RETURN_CODE 77) + # IcomCIV wire layers — pure encode/decode, standalone (no Qt / aethercore). # An Icom networked radio is two protocols stacked: CI-V is the command plane # and RS-BA1 is the UDP transport it travels inside. Both halves unit-test From b93217a1ee8cd1e6927b49ca0cc249b225a3cbdd Mon Sep 17 00:00:00 2001 From: Sotiris Date: Mon, 5 Oct 2026 09:19:23 -0500 Subject: [PATCH 2/2] fix(anan): measure loss over DDC frames only and count only the radio's datagrams. Principle VIII. Answers the 10-04 review of #6137 and the maintainer's rulings on it. rxPackets counted every datagram, while rxPacketsLost counts DDC sequence gaps. p2app streams Mic Data at about 750 packets a second against about 202 DDC frames at 48 ksps, so RadioModel's loss percentage read about 4.7x low. P2Client now counts a packet only once a DDC frame's index resolves. Bytes are still counted before any parse, so LinkStats::alive still sees Status traffic. onReadyRead() hands each datagram to handleDatagramFrom(), which drops anything not from the session host, as ruled, and logs the first foreign sender once per session. handleDatagram()'s signature is unchanged. AnanBackend::onLinkCounters() treats a backward jump in rxBytes as a new session, so a restart that skipped disconnectRadio(), or a dead session's snapshot delivered after its reset, is no longer compared against the old total. The status bar hides the separator after the PA stack along with it, and re-reads its minimum width after the hide. The comment saying that re-read could not matter is replaced. Tests: the link test's transport asserts move to rxBytes. Its stray-port frame had bitsPerSample 0, so the parser rejected it before the port check; it is now a real DDC frame, and a DDC0 frame pins the packet count. A new block pins the address filter, and exit 77 applies only when nothing has failed. anan_backend_test gains a friend that drives onLinkCounters() for alive, the first snapshot and the backward jump. The tests.cmake block names the socket it binds. Comments: the supply-rail rationale no longer blames only the 5 V / 3.3 V reference. A G2 bench read a 13.8 V rail as about 40 V through the 5 V scale, which no board choice explains. The stale spectrum_sequence_gap_test paragraph and the link test's speaker-stream claim are corrected. On 33427611: build clean; ctest 655 tests, 652 passed, 1 skipped, 2 failed (pgxl_panel_test, a known local failure, and rtl_capture_worker_test, which passes 5 of 5 alone). Every Static checks step passes with --strict. Fourteen mutations, all killed. On the G2: Network: reports and updates, one separator, and no datagram from the radio was ignored. Co-Authored-By: Claude Opus 5.5 --- src/core/backends/anan/AnanBackend.cpp | 25 +++-- src/core/backends/anan/AnanBackend.h | 1 + src/core/backends/anan/P2Client.cpp | 33 ++++++- src/core/backends/anan/P2Client.h | 17 +++- src/core/backends/anan/P2Protocol.h | 13 +-- src/gui/MainWindow.cpp | 26 +++-- src/gui/MainWindow.h | 4 +- tests/anan_backend_test.cpp | 49 +++++++++- tests/anan_link_telemetry_test.cpp | 126 +++++++++++++++++++------ tests/spectrum_sequence_gap_test.cpp | 8 +- tests/tests.cmake | 7 +- 11 files changed, 223 insertions(+), 86 deletions(-) diff --git a/src/core/backends/anan/AnanBackend.cpp b/src/core/backends/anan/AnanBackend.cpp index d2d8dfe754..a7dacd1509 100644 --- a/src/core/backends/anan/AnanBackend.cpp +++ b/src/core/backends/anan/AnanBackend.cpp @@ -406,16 +406,11 @@ RadioCapabilities AnanBackend::capabilities() const c.manufacturer = QStringLiteral("Apache Labs"); c.model = QStringLiteral("ANAN-G2"); // ---- PA telemetry ---- - // hasSupplyVoltageTelemetry stays FALSE even though the radio does send a - // supply-rail reading (P2Protocol decodes it). Raw ADC counts become volts - // only against the RF board's ADC reference -- 5 V on one board, 3.3 V on - // another, a 1.5x difference that would read 21 V for a 13.8 V rail -- and - // NOTHING THE RADIO REPORTS IDENTIFIES WHICH. The board id cannot answer - // it: see DiscoveryReply::isSaturn(), which is explicitly a discovery-time - // picker filter and not for gating backend behaviour, because board type is - // a p2app launch option rather than a fact about the hardware. So the row - // stays withdrawn until the reference comes from somewhere load-bearing or - // from the operator; a confidently wrong voltage is worse than none. + // hasSupplyVoltageTelemetry stays FALSE although the radio sends a + // supply-rail count (P2Protocol decodes it): no known scale fits it. On a + // G2 bench the 5 V-reference scale read a 13.8 V rail as ~40 V, 2.9x high, + // and the 3.3 V board's scale differs from it by only 1.5x, so no board + // choice fixes it. A volts readout waits for an operator calibration. // // There is NO PA temperature in this protocol. The radio's whole analog // payload is six fields and none of them is a temperature, so the readout @@ -1455,10 +1450,12 @@ IRadioBackend::LinkStats AnanBackend::linkStats() const void AnanBackend::onLinkCounters(const P2Client::LinkCounters& counters) { - // First snapshot of a session compares against ZERO, not against itself: - // P2Client::start() resets its counters, and comparing a first snapshot to - // itself would report a dead link on the very tick after a connect that - // only happened because DDC0 frames arrived. + // A session restart shows as a BACKWARD jump, since P2Client::start() zeroes + // its counters; the cache is then a dead session's and is discarded here, + // whichever path restarted it. The first snapshot of a session compares + // against ZERO: against itself, it would read dead on the first tick. + if (counters.rxBytes < m_linkCounters.rxBytes) + m_linkCountersSeen = false; m_lastSnapshotRxBytes = m_linkCountersSeen ? m_linkCounters.rxBytes : 0; m_linkCounters = counters; m_linkCountersSeen = true; diff --git a/src/core/backends/anan/AnanBackend.h b/src/core/backends/anan/AnanBackend.h index edbe1a26ac..084fbb8dbe 100644 --- a/src/core/backends/anan/AnanBackend.h +++ b/src/core/backends/anan/AnanBackend.h @@ -154,6 +154,7 @@ class AnanBackend : public IRadioBackend { private: friend class AnanNoiseBlankerTestAccess; + friend class AnanLinkStatsTestAccess; void beginDspSetup(); void finishDspSetup(quint64 generation, bool ok, const QString& error); // The "restart P2Client with m_pendingParams, then retune" half of what diff --git a/src/core/backends/anan/P2Client.cpp b/src/core/backends/anan/P2Client.cpp index 3333cb0994..41c45c4991 100644 --- a/src/core/backends/anan/P2Client.cpp +++ b/src/core/backends/anan/P2Client.cpp @@ -105,6 +105,7 @@ bool P2Client::start(const Params& params, int connectTimeoutMs) m_rxPackets = 0; m_txBytes = 0; m_warnedUnexpectedPorts.clear(); + m_warnedForeignSender = false; m_linkUp = false; m_discoveryInfoSent = false; @@ -457,20 +458,38 @@ void P2Client::onReadyRead() { while (m_socket && m_socket->hasPendingDatagrams()) { const QNetworkDatagram dg = m_socket->receiveDatagram(); - handleDatagram(asBytes(dg.data()), static_cast(dg.senderPort())); + handleDatagramFrom(dg.senderAddress(), asBytes(dg.data()), + static_cast(dg.senderPort())); } } +void P2Client::handleDatagramFrom(const QHostAddress& sender, + std::span bytes, quint16 senderPort) +{ + // The radio answers from the address start() sent to: Discovery is unicast + // to m_host from this same socket, so its reply passes too. A spoofed + // source still passes; this keeps out stray LAN traffic, not an attacker. + if (!sender.isEqual(m_host, QHostAddress::TolerantConversion)) { + if (!m_warnedForeignSender) { + m_warnedForeignSender = true; + qCWarning(lcAnanP2).nospace() + << "ANAN: ignoring datagram from " << sender.toString() + << " (the radio is " << m_host.toString() + << ") -- further foreign senders this session are not logged"; + } + return; + } + handleDatagram(bytes, senderPort); +} + // Socket-free ingest seam: production and regression tests share validation, // per-DDC sequence tracking, notification ordering and sample delivery. void P2Client::handleDatagram(std::span bytes, quint16 senderPort) { // Before any parse decides what this datagram IS: the transport carried it - // either way, and a session whose IQ has stalled while Status packets still - // arrive must not read as a dead link. Counting after the shape checks - // below would make every rejected datagram invisible to the readout. + // either way, and LinkStats::alive reads bytes, so a session whose IQ has + // stalled while Status packets still arrive does not read as a dead link. m_rxBytes += static_cast(bytes.size()); - ++m_rxPackets; const auto frame = parseDdcFrame(bytes); if (!frame) { @@ -517,6 +536,10 @@ void P2Client::handleDatagram(std::span bytes, quint16 sende return; } const std::size_t slot = static_cast(*ddcIndex); + // The denominator rxPacketsLost is a fraction of: sequence-tracked DDC + // frames only. Mic Data and Status would dilute RadioModel's loss + // percentage several-fold at low DDC rates. + ++m_rxPackets; // Per-DDC gap detection; see m_expectedSeq's own comment for why a // shared counter would manufacture drops once a second DDC streams. diff --git a/src/core/backends/anan/P2Client.h b/src/core/backends/anan/P2Client.h index 5e589c35bd..9b93a20a8f 100644 --- a/src/core/backends/anan/P2Client.h +++ b/src/core/backends/anan/P2Client.h @@ -131,10 +131,11 @@ class P2Client : public QObject { // says the same thing: a backend with worker-side values caches them on its // own thread. // - // Session-cumulative, reset by start(). EVERY datagram the socket handed us - // counts, including Mic Data and Status: this measures the TRANSPORT, not - // the IQ stream, so a session whose DDC traffic has stalled while status - // packets still arrive must not read as a dead link. + // Session-cumulative, reset by start(). Only the radio's own datagrams + // count (handleDatagramFrom()). Bytes count every one of them, so a session + // whose IQ has stalled while Status packets still arrive does not read as + // a dead link. Packets count DDC frames only: drops counts gaps in that + // stream, and RadioModel's loss percentage divides one by the other. struct LinkCounters { quint64 rxBytes = 0; quint64 rxPackets = 0; @@ -190,6 +191,11 @@ private slots: private: friend struct P2ClientTestAccess; + // onReadyRead()'s socket-free half: drops anything not from m_host, so + // foreign LAN traffic can neither hold LinkStats::alive true after the + // radio has gone nor reach the parsers. + void handleDatagramFrom(const QHostAddress& sender, + std::span bytes, quint16 senderPort); void handleDatagram(std::span bytes, quint16 senderPort); void noteSpeakerFifoStatus(const HighPriorityStatus& status); @@ -301,6 +307,9 @@ private slots: // nature affects every packet -- logs once per distinct port rather than // per datagram. Session-scoped; cleared with the rest of the state. QSet m_warnedUnexpectedPorts; + // One warning per session, not per sender: a set keyed on the source + // address would grow without bound under a flood of spoofed ones. + bool m_warnedForeignSender = false; // Reused decode buffer, cleared and refilled per frame rather than // reallocated -- matches MetisClient's m_blocks for the same reason. diff --git a/src/core/backends/anan/P2Protocol.h b/src/core/backends/anan/P2Protocol.h index c3eb05683b..c35bef04fe 100644 --- a/src/core/backends/anan/P2Protocol.h +++ b/src/core/backends/anan/P2Protocol.h @@ -257,14 +257,11 @@ struct HighPriorityStatus { // or unassigned, and there is NO TEMPERATURE anywhere in the payload, which // is why no client can populate a PA temperature readout for this radio. // - // DECODED BUT NOT YET CONSUMED, deliberately. Counts become volts only - // against the RF board's ADC reference, and nothing the radio reports - // identifies it: board type is a p2app launch option, not a hardware fact - // (see DiscoveryReply::isSaturn()). Publishing a scaled value would mean - // picking a reference at a 1.5x spread -- 21 V shown for a 13.8 V rail if - // wrong. The decode is pinned here so the wire format is recorded and - // tested; the reference has to come from the operator or from a - // load-bearing field before a reading can be offered. + // DECODED BUT NOT YET CONSUMED, deliberately: no known scale turns these + // counts into volts on this radio. A G2 bench read a 13.8 V rail as ~40 V + // through the 5 V-reference scale (2.9x high), and the 3.3 V board's scale + // is only 1.5x from that, so neither board explains it. The raw count is + // what an operator calibration against a meter will need. std::uint16_t supplyVoltageRaw = 0; }; diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 6881a15ca2..0aba497db8 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -6015,7 +6015,7 @@ void MainWindow::buildUI() paVbox->addWidget(m_supplyVoltLabel); hbox->addWidget(paStack); - addSep(); + m_paSeparator = addSep(); // Network label (top) + quality (bottom) stacked auto* netStack = new QWidget; @@ -7819,26 +7819,24 @@ void MainWindow::applyCapabilitiesToUi(bool connected, const RadioCapabilities& m_hasPaTempTelemetry = false; updatePaTempLabel(); } - // Republishing the minimum width cannot currently matter here, and the - // call is kept only so the gate stays correct if that stops being true: - // paStack's minimum is PINNED by reserveTelemetryStack() with a - // "99.99 V" sample, so hiding one of its children leaves - // m_statusBarContainer->minimumSizeHint() unchanged, and - // updateStatusBarMinimumWidth() only ever READS that hint. So no stale - // minimum, gap or clipped size grip is reachable today — and the HL2 - // reclaims no width from the hidden row either, nor would it: - // "PA 248.0°F" is the wider sample. - updateStatusBarMinimumWidth(); } - // Both rows withdrawn: take the container down too. Its minimum width is - // PINNED by reserveTelemetryStack(), so hiding only the children would - // leave a reserved, empty gap sitting between two separators. + // Both rows withdrawn: take the container down too, and the separator after + // it. Its minimum width is PINNED by reserveTelemetryStack(), so hiding only + // the children would leave a reserved, empty gap sitting between two + // separators. if (m_paStack) { const bool anyPaRow = (m_paTempLabel && m_paTempLabel->isVisibleTo(m_paStack)) || (m_supplyVoltLabel && m_supplyVoltLabel->isVisibleTo(m_paStack)); m_paStack->setVisible(anyPaRow); + if (m_paSeparator) { + m_paSeparator->setVisible(anyPaRow); + } } + // After the hide, not before: hiding the stack drops its pinned minimum out + // of m_statusBarContainer->minimumSizeHint(), which this only ever READS. + // Hiding one row alone changes nothing, since the pin holds the width. + updateStatusBarMinimumWidth(); // CWX/CWK, DVK and FDX status-bar toggles are HIDDEN without the capability: // each is a firmware verb (`cwx`, `dvk`, `radio set full_duplex_enabled=`) diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index f22e131e75..bf247a8720 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -1730,8 +1730,10 @@ private slots: // The container holding the two labels above. Held so the whole stack can // come down when BOTH its rows are withdrawn: reserveTelemetryStack() pins // its minimum width, so hiding only the children would leave a reserved - // empty gap between two separators. + // empty gap between two separators. The separator after it hides with it, + // or the two would sit back to back. QWidget* m_paStack{nullptr}; + QLabel* m_paSeparator{nullptr}; QLabel* m_networkLabel{nullptr}; QTimer m_networkTooltipRefreshTimer; QTimer m_perfHeartbeatTimer; diff --git a/tests/anan_backend_test.cpp b/tests/anan_backend_test.cpp index a7fc8b64fb..f99fe8ebb9 100644 --- a/tests/anan_backend_test.cpp +++ b/tests/anan_backend_test.cpp @@ -30,6 +30,24 @@ #include #include +namespace AetherSDR::anan { +// AnanBackend's friend: drives the client-snapshot handler with no client and +// no socket, which is the only deterministic way to reach LinkStats::alive. +class AnanLinkStatsTestAccess { +public: + static void setConnected(AnanBackend& backend, bool connected) + { + backend.m_connected = connected; + } + static void snapshot(AnanBackend& backend, quint64 rxBytes) + { + P2Client::LinkCounters counters; + counters.rxBytes = rxBytes; + backend.onLinkCounters(counters); + } +}; +} // namespace AetherSDR::anan + using namespace AetherSDR; using namespace AetherSDR::anan; @@ -517,11 +535,10 @@ int main(int argc, char** argv) " claim the status bar withdraws its readout on"); check(!caps.hasPaTemperatureTelemetry, "and no temperature is claimed as received"); - // The radio DOES send a supply reading, but raw counts become volts only - // against an ADC reference nothing in the protocol identifies, so the row - // stays withdrawn rather than showing a value at a 1.5x guess. + // The radio DOES send a supply count, but no known scale fits it (a + // 13.8 V rail read as ~40 V on a G2 bench), so the row stays withdrawn. check(!caps.hasSupplyVoltageTelemetry, - "no supply voltage is claimed while its ADC reference is unknown"); + "no supply voltage is claimed while no scale for its counts is known"); check(!caps.hasPaCurrentTelemetry, "no PA drain current is claimed"); } @@ -539,6 +556,30 @@ int main(int argc, char** argv) " \"not measured\" sentinel, which must not render as zero"); } + // ---- LinkStats::alive: this snapshot's bytes against the last one's ---- + { + AnanBackend backend; + AnanLinkStatsTestAccess::setConnected(backend, true); + + AnanLinkStatsTestAccess::snapshot(backend, 1000); + check(backend.linkStats().reported && backend.linkStats().alive, + "a session's first snapshot compares against zero, so a link that" + " carried bytes reads alive on its first tick"); + + AnanLinkStatsTestAccess::snapshot(backend, 1000); + check(!backend.linkStats().alive, "a second with no new bytes reads dead"); + + AnanLinkStatsTestAccess::snapshot(backend, 5000); + check(backend.linkStats().alive, "bytes resuming read alive again"); + + // A restart that skipped disconnectRadio(), or a dead session's snapshot + // delivered after its reset: the new session's total is below the cache. + AnanLinkStatsTestAccess::snapshot(backend, 300); + check(backend.linkStats().alive && backend.linkStats().rxBytes == 300, + "a backward jump is a new session, compared against zero and not" + " against the dead session's total"); + } + if (g_failures == 0) std::fprintf(stderr, "anan_backend_test: all checks passed\n"); return g_failures == 0 ? 0 : 1; diff --git a/tests/anan_link_telemetry_test.cpp b/tests/anan_link_telemetry_test.cpp index 0bd203eaa0..c6f79b8277 100644 --- a/tests/anan_link_telemetry_test.cpp +++ b/tests/anan_link_telemetry_test.cpp @@ -5,21 +5,25 @@ // written only from its own I/O thread, and reach the backend as a periodic // snapshot rather than a cross-thread read. What this pins: // -// * EVERY datagram counts, including the ones every parser rejects. The -// readout describes the TRANSPORT, so a session whose IQ has stalled while -// Status packets still arrive must not read as a dead link -- and counting -// after the shape checks would make exactly those datagrams invisible. -// * Sent bytes are counted on every send path, including the speaker stream, -// which does not go through sendTo(). +// * Every byte from the radio counts, including datagrams every parser +// rejects, so a session whose IQ has stalled while Status packets still +// arrive does not read as a dead link. +// * Packets count DDC frames only: they are the denominator of the loss +// percentage, whose numerator is DDC sequence gaps. +// * A datagram from any address but the radio's counts toward nothing. +// * Sent bytes are counted through sendTo(), which every send but the speaker +// stream uses. The speaker path counts its own and is not pinned here. // * start() resets the session, so a reconnect does not inherit totals. // -// Loopback send only (127.0.0.1), no radio and no listener: the assertions are -// about the client's own counters, not about what arrives. +// The last block BINDS A UDP SOCKET (AnyIPv4, ephemeral port) and sends the +// startup sequence to 127.0.0.1; nothing listens and no radio is involved. It +// skips (exit 77) when it cannot bind, unless an earlier check already failed. #include "core/backends/anan/P2Client.h" #include "core/backends/anan/P2Protocol.h" #include +#include #include #include @@ -47,6 +51,14 @@ struct P2ClientTestAccess { { c.handleDatagram(bytes, port); } + // The socket's half, minus the socket: the sender address is what onReadyRead() + // reads off the QNetworkDatagram. + static void feedFrom(P2Client& c, const QHostAddress& sender, + std::span bytes, quint16 port) + { + c.handleDatagramFrom(sender, bytes, port); + } + static void setRadioHost(P2Client& c, const QHostAddress& host) { c.m_host = host; } static void publish(P2Client& c) { c.publishLinkCounters(); } static quint64 rxBytes(const P2Client& c) { return c.m_rxBytes; } static quint64 rxPackets(const P2Client& c) { return c.m_rxPackets; } @@ -62,6 +74,20 @@ static P2Client::Params loopbackParams() return p; } +// A DDC I&Q frame parseDdcFrame() accepts: 24 bits per sample, one sample, +// length exactly header + one sample. +static std::vector ddcFrame(std::uint32_t seq) +{ + std::vector f(kDdcHeaderLen + kDdcSampleBytes, 0); + f[0] = static_cast(seq >> 24); + f[1] = static_cast(seq >> 16); + f[2] = static_cast(seq >> 8); + f[3] = static_cast(seq); + f[13] = 24; // bits per sample + f[15] = 1; // samples in this frame + return f; +} + int main(int argc, char** argv) { QCoreApplication app(argc, argv); @@ -78,7 +104,7 @@ int main(int argc, char** argv) "a client that has not run has all counters at zero"); } - // ---- every datagram counts, including ones the parsers reject ---- + // ---- bytes count every datagram; packets count DDC frames only ---- { P2Client client; @@ -87,39 +113,76 @@ int main(int argc, char** argv) std::vector status(kHighPriorityStatusBytes, 0); status[30] = 0b0000'1000; P2ClientTestAccess::feedDatagram(client, status, 1025); - check(P2ClientTestAccess::rxPackets(client) == 1 - && P2ClientTestAccess::rxBytes(client) == kHighPriorityStatusBytes, - "a Status packet counts: it is transport traffic, not IQ"); + check(P2ClientTestAccess::rxBytes(client) == kHighPriorityStatusBytes, + "a Status packet's bytes count: it is transport traffic"); + check(P2ClientTestAccess::rxPackets(client) == 0, + "...but it is not a packet of the stream drops are counted on"); // Garbage too short to be any known packet. THIS is the case that would - // vanish if counting happened after the shape checks instead of before. + // vanish if bytes were counted after the shape checks instead of before. const std::vector runt{0x01, 0x02, 0x03}; P2ClientTestAccess::feedDatagram(client, runt, 1025); - check(P2ClientTestAccess::rxPackets(client) == 2 - && P2ClientTestAccess::rxBytes(client) - == kHighPriorityStatusBytes + runt.size(), - "a datagram every parser rejects still counts as traffic arrived"); - - // DDC-shaped but from a port this session never enabled: dropped WITHOUT - // counting a sequence drop, yet the bytes did cross the wire. - std::vector ddcish(kDdcHeaderLen + kDdcSampleBytes, 0); - ddcish[14] = 0x00; ddcish[15] = 0x01; // one sample declared + check(P2ClientTestAccess::rxBytes(client) + == kHighPriorityStatusBytes + runt.size(), + "a datagram every parser rejects still counts as bytes arrived"); + + // A real DDC frame, but from a port this session never enabled: dropped + // WITHOUT counting a sequence drop, so it must not count as a packet + // either, yet its bytes did cross the wire. + const auto stray = ddcFrame(7); const quint64 beforeStray = P2ClientTestAccess::rxBytes(client); - P2ClientTestAccess::feedDatagram(client, ddcish, 9999); - check(P2ClientTestAccess::rxBytes(client) > beforeStray - && P2ClientTestAccess::rxPackets(client) == 3, - "a datagram from an unexpected sender port counts as traffic too"); + P2ClientTestAccess::feedDatagram(client, stray, 9999); + check(P2ClientTestAccess::rxBytes(client) == beforeStray + stray.size(), + "a DDC frame from an unexpected sender port counts as bytes"); + check(P2ClientTestAccess::rxPackets(client) == 0, + "...and not as a packet: drops are never counted for that port"); + + // The positive case: DDC0's own port. + const auto ddc0 = ddcFrame(1); + P2ClientTestAccess::feedDatagram(client, ddc0, kDdc0DefaultPort); + check(P2ClientTestAccess::rxPackets(client) == 1, + "a DDC0 frame from the session's own port is one packet"); check(client.droppedPackets() == 0, "...and none of those is a sequence drop -- the two counters are" " independent"); } + // ---- only the radio's own address counts ---- + { + P2Client client; + const QHostAddress radio(QStringLiteral("192.0.2.10")); + P2ClientTestAccess::setRadioHost(client, radio); + const auto frame = ddcFrame(1); + + P2ClientTestAccess::feedFrom(client, QHostAddress(QStringLiteral("192.0.2.99")), + frame, kDdc0DefaultPort); + check(P2ClientTestAccess::rxBytes(client) == 0 + && P2ClientTestAccess::rxPackets(client) == 0, + "a DDC frame from another LAN host counts toward nothing, so it" + " cannot hold the link alive after the radio has gone"); + + P2ClientTestAccess::feedFrom(client, radio, frame, kDdc0DefaultPort); + check(P2ClientTestAccess::rxBytes(client) == frame.size() + && P2ClientTestAccess::rxPackets(client) == 1, + "the same frame from the radio's address counts"); + + // The socket binds AnyIPv4, but a dual-stack bind would report the radio + // in v4-mapped form; the comparison must not drop it then either. + P2ClientTestAccess::feedFrom(client, + QHostAddress(QStringLiteral("::ffff:192.0.2.10")), + ddcFrame(2), kDdc0DefaultPort); + check(P2ClientTestAccess::rxPackets(client) == 2, + "the radio's address in v4-mapped form is still the radio"); + } + // ---- the snapshot carries the counters, and is what crosses threads ---- { P2Client client; std::vector status(kHighPriorityStatusBytes, 0); P2ClientTestAccess::feedDatagram(client, status, 1025); + const auto ddc0 = ddcFrame(1); + P2ClientTestAccess::feedDatagram(client, ddc0, kDdc0DefaultPort); QSignalSpy snapshots(&client, &P2Client::linkCountersUpdated); P2ClientTestAccess::publish(client); @@ -128,7 +191,7 @@ int main(int argc, char** argv) const auto counters = snapshots.at(0).at(0).value(); check(counters.rxPackets == 1 - && counters.rxBytes == kHighPriorityStatusBytes, + && counters.rxBytes == kHighPriorityStatusBytes + ddc0.size(), "the snapshot carries the receive counters as the client has" " them"); check(counters.drops == 0, "and the drop total alongside them"); @@ -142,13 +205,18 @@ int main(int argc, char** argv) // Traffic from a PREVIOUS notional session, which must not survive. std::vector status(kHighPriorityStatusBytes, 0); P2ClientTestAccess::feedDatagram(client, status, 1025); - check(P2ClientTestAccess::rxBytes(client) > 0, "pre-start traffic counted"); + P2ClientTestAccess::feedDatagram(client, ddcFrame(1), kDdc0DefaultPort); + check(P2ClientTestAccess::rxBytes(client) > 0 + && P2ClientTestAccess::rxPackets(client) > 0, + "pre-start traffic counted"); if (!client.start(loopbackParams())) { std::fprintf(stderr, "anan_link_telemetry_test: SKIP -- client could not bind" " a local UDP socket\n"); - return 77; // SKIP_RETURN_CODE in tests.cmake + // Exit 77 (SKIP_RETURN_CODE) reads green, so it is only a skip + // while nothing above has failed. + return g_failures == 0 ? 77 : 1; } check(P2ClientTestAccess::rxBytes(client) == 0 diff --git a/tests/spectrum_sequence_gap_test.cpp b/tests/spectrum_sequence_gap_test.cpp index 560235b0e1..aa3078b460 100644 --- a/tests/spectrum_sequence_gap_test.cpp +++ b/tests/spectrum_sequence_gap_test.cpp @@ -22,10 +22,10 @@ // WHAT A GAP DID REACH, before this. A counter, and only a counter: // MetisClient's EP6 path computes the gap, adds it to m_drops, and emits // dropsUpdated, which Hl2Backend mirrors onto the GUI thread for the health -// rows. On the ANAN side P2Client's dropsUpdated reaches a deliberately empty -// lambda in AnanBackend. Neither path touches a sample-path object. "A gap -// reaches nothing" would be too strong; "a gap reaches nothing that can act on -// it" is the accurate statement, and it is what this file closes. +// rows. On the ANAN side P2Client's drop total reaches only the LinkStats +// snapshot AnanBackend publishes. Neither path touches a sample-path object. +// "A gap reaches nothing" would be too strong; "a gap reaches nothing that can +// act on it" is the accurate statement, and it is what this file closes. // // FIVE CLAIMS, in the order they build on each other: // diff --git a/tests/tests.cmake b/tests/tests.cmake index 2d0fc5c061..dd5b0877da 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -1095,9 +1095,10 @@ add_test(NAME anan_speaker_audio_test COMMAND anan_speaker_audio_test) set_tests_properties(anan_speaker_audio_test PROPERTIES SKIP_RETURN_CODE 77) # ANAN transport counters -> IRadioBackend::LinkStats -- what the status bar's -# "Network:" field reads. Counts every datagram the socket hands over, including -# the ones each parser rejects, because the readout describes the TRANSPORT and -# not the IQ stream. Loopback send only, no listener and no radio. +# "Network:" field reads: bytes from every radio datagram, packets from DDC +# frames only, nothing from another host. SOCKET: binds one UDP socket on +# AnyIPv4, ephemeral port (P2Client::start()), and sends its startup sequence to +# 127.0.0.1. No listener, no fake peer, no radio. add_executable(anan_link_telemetry_test tests/anan_link_telemetry_test.cpp) target_include_directories(anan_link_telemetry_test PRIVATE src tests) target_link_libraries(anan_link_telemetry_test