fix(hl2): rate-limit the repeated ADC-overload warning

docs/HERMES.md

@@ -927,7 +927,7 @@ Effort is rough: **XS** under an hour, **S** a session, **M** a few sessions,
 | 4 | Pipeline reset `0x39[7:4]=0x8` after an NCO move | A2 §B2 | Decimation state smears a transient across band-scale jumps — which `a1cbe154` made routine | XS |
 | 5 | Normalize by `2^23-1`, not `2^23` | A1 §A2 | dBFS parity with piHPSDR. Numerically trivial, but parity is the point | XS |
 | 6 | `RXASetNC` / `RXASetMP` after `OpenChannel` | A3 §7 | Selectivity vs latency; matters to CW operators. We silently take defaults | XS |
-| 6a | Rate-limit the ADC-overload warning | §15.7 | Edge-gated, but the value chatters: **~133 warnings/second** on MW, which flushes the log ring and hides everything else | XS |
+| ~~6a~~ | ~~Rate-limit the ADC-overload warning~~ **DONE** | §15.7 | The edge gate stays and a 10 s rate limit sits behind it, carrying the count of transitions the window swallowed. Note the severity here was already overstated when this row was written — see §15.7 | — |
 
 ### Tier 2 — correctness gaps
 
@@ -1751,11 +1751,26 @@ value the assertion depends on, even when it looks like a constant.
 
 ### 15.7 Noticed, not fixed
 
-- **ADC overload chatter.** On the MW broadcast band with the default +20 dB LNA
-  the overload flag dithers, and the warning in `publishTelemetry` — although
-  edge-gated — fires **~133 times/second**, flushing the log ring. The gate is on
-  the value changing, but the value genuinely chatters. It also buries every
-  other log line, which is how it obstructed the diagnosis in §15.5.
+- **ADC overload chatter — fixed, and the figure below was already stale.** On
+  the MW broadcast band with the default +20 dB LNA the overload flag dithers,
+  and the warning in `publishTelemetry` — although edge-gated — was measured at
+  **~133 times/second**, flushing the log ring and burying every other line,
+  which is how it obstructed the diagnosis in §15.5. The gate is on the value
+  changing; the value genuinely chatters.
+
+  **That rate has not been reachable since #4449.** `MetisClient` coalesces
+  `telemetryUpdated` to 10 Hz (`kTelemetryMinIntervalMs`), with no
+  change-bypass, so `publishTelemetry` cannot run faster than 10 Hz however hard
+  the comparator chatters — which capped this at ~10/s and ended the
+  ring-flushing without anyone recording that it had. **Kept rather than
+  rewritten**, because a symptom that stops being reproducible for a reason
+  nobody wrote down is worth more as a corrected entry than as a deleted one.
+
+  The remainder — one message repeating up to ten times a second for as long as
+  the band stays strong — is fixed: the edge gate stays and a 10 s rate limit
+  sits behind it, reporting the count of transitions the window swallowed.
+  That count is transitions *seen*, at the 10 Hz telemetry cadence, not
+  comparator edges, which are sampled far below their true rate and always were.
 - **The HL2 LNA gain is only settable at connect time** (`lnaGainDb` param).
   There is no seam verb for RF gain, so an operator on a strong band cannot back
   it off without reconnecting. This is why the overload above could not simply be

src/core/backends/hl2/Hl2Backend.cpp

@@ -8,6 +8,7 @@
 
 #include "core/backends/hl2/Hl2RxDsp.h"
 #include "core/backends/hl2/Hl2TxDsp.h"
+#include "core/backends/hl2/Hl2OverloadPolicy.h"
 #include "core/backends/hl2/Hl2DspSetupPolicy.h"
 #include "core/backends/hl2/Hl2TxLevelPolicy.h"
 #include "core/backends/hl2/MetisClient.h"
@@ -5035,7 +5036,59 @@ void Hl2Backend::publishTelemetry(const Hl2Telemetry& t)
     if (t.adcOverload && *t.adcOverload != m_adcOverload) {
         m_adcOverload = *t.adcOverload;
         if (m_adcOverload)
+            ++m_adcOverloadAssertions;
+    }
+    // Rate-limited, not merely edge-gated. The edge gate above is necessary and
+    // was never sufficient: the comparator genuinely chatters on a strong band,
+    // so nearly every telemetry sample is an edge and one message repeats at the
+    // full telemetry cadence (see the members' comment in the header for the
+    // rate, and for why the historical figure there is not repeated as a
+    // current one).
+    //
+    // Deliberately OUTSIDE the edge test, and this is the whole reason the two
+    // are separate: a burst that stops must still report its tally. Flushing
+    // only on the next edge would hold the count until the band goes loud
+    // again, which could be hours away or never. publishTelemetry runs on every
+    // telemetry update, so the window closes on time whether or not the
+    // condition is still happening.
+    //
+    // Reported rather than dropped because the rate IS the severity here — a
+    // flag that sets once is a hint, one that sets on every sample for a minute
+    // is a front end being slammed.
+    const AetherSDR::hl2::AdcOverloadWarn w = AetherSDR::hl2::adcOverloadWarn(
+        m_adcOverloadAssertions,
+        m_adcOverloadClock.isValid(),
+        m_adcOverloadClock.isValid() ? m_adcOverloadClock.elapsed() : 0,
+        kAdcOverloadWarnIntervalMs);
+    if (w.warn) {
+        // What the aggregate branch does NOT mean. It is not "this is the first
+        // overload ever" — it is "exactly one assertion was seen in this
+        // window". That lone assertion may have arrived at any point since the
+        // window opened, so a bare message can lag the event by up to
+        // kAdcOverloadWarnIntervalMs. Accepted deliberately: it is the cost of
+        // the rate limit, one assertion is a hint rather than an emergency, and
+        // an isolated overload after a quiet period still reports immediately
+        // because the clock is long expired by then.
+        if (w.aggregate) {
+            // noquote + one composed string: streaming "(" as its own item makes
+            // QDebug insert a space after it and print "( 51 times in 10000 ms)".
+            qWarning().noquote()
+                << "Hl2Backend: ADC OVERLOAD — reduce LNA gain or attenuate"
+                << QStringLiteral("(%1 times in %2 ms)")
+                       .arg(w.count)
+                       .arg(m_adcOverloadClock.elapsed());
+        } else {
             qWarning() << "Hl2Backend: ADC OVERLOAD — reduce LNA gain or attenuate";
+        }
+        if (w.restartClock) {
+            // start(), NOT restart(). restart() reads the elapsed time first,
+            // and reading it on a timer that was never started is undefined —
+            // which is exactly the first-assertion path, where the clock is
+            // invalid by construction. start() is defined on both, and the
+            // value restart() returns was discarded anyway. (#5381 review.)
+            m_adcOverloadClock.start();
+        }
+        m_adcOverloadAssertions = 0;
     }
 }
 

src/core/backends/hl2/Hl2Backend.h

@@ -699,6 +699,27 @@ class Hl2Backend : public IRadioBackend {
     quint64 m_linkRxPacketsAtLastTick = 0;
     static constexpr int kLinkStatsIntervalMs = 1000;
     bool m_adcOverload = false;
+    // The overload bit is a per-frame sample of a level comparator, not an
+    // event: on a strong band it dithers, so the edge gate in publishTelemetry
+    // sees an edge nearly every time it looks. docs/HERMES.md 15.7 recorded
+    // ~133 warnings/second on the MW broadcast band, flushing the log ring.
+    //
+    // That figure is stale and deliberately not repeated as a present-tense
+    // claim: MetisClient has since coalesced telemetryUpdated to 10 Hz (#4449),
+    // which caps this at ~10/s however hard the comparator chatters. What
+    // remains is one message repeating ten times a second for as long as the
+    // band stays strong — no longer ring-flushing, still enough to bury the
+    // lines around it over a session.
+    //
+    // So the edge gate stays and a rate limit sits behind it: warn on the first
+    // transition, then at most once per window, carrying the count of
+    // assertions the window swallowed. Note what that count is and is not — it
+    // counts the assertions SEEN, at the 10 Hz telemetry cadence, not
+    // comparator edges, which are sampled far below their true rate and always
+    // were.
+    QElapsedTimer m_adcOverloadClock;
+    int m_adcOverloadAssertions = 0;
+    static constexpr qint64 kAdcOverloadWarnIntervalMs = 10000;
     bool m_keyed = false;
     bool m_tuning = false;
     bool m_cwAutoKeyed = false;

src/core/backends/hl2/Hl2OverloadPolicy.h

@@ -0,0 +1,65 @@
+#pragma once
+
+// When the ADC-overload warning may be emitted, as a pure decision.
+//
+// The AD9866's overload flag is a per-frame sample of a level comparator, not
+// an event. On a strong band it chatters, so nearly every telemetry sample is a
+// rising edge and one message repeats at the full telemetry cadence. An edge
+// gate alone is necessary and never sufficient.
+//
+// Two properties make this worth a seam rather than an inline condition, and
+// both are timing-dependent in a way that is otherwise only reachable by
+// running a radio for ten seconds:
+//
+//   * THE FLUSH IS NOT ON AN EDGE. A burst that stops must still report its
+//     tally. Deciding only when the flag next asserts would hold the count
+//     until the band goes loud again -- which may be hours away, or never.
+//   * THE FIRST ASSERTION IS IMMEDIATE. A limiter that made the operator wait
+//     out a window before the first warning would be silent during exactly the
+//     interval when the front end is being slammed and nobody knows yet.
+//
+// Hl2Backend evaluates these functions rather than its own copy, so what the
+// suite exercises is what the radio runs -- the reasoning Hl2TxLevelPolicy.h
+// states, and the same reason it applies here.
+
+#include <cstdint>
+
+namespace AetherSDR::hl2 {
+
+// What publishTelemetry should do with the overload counter this update.
+struct AdcOverloadWarn {
+    bool warn = false;       // emit anything at all?
+    bool aggregate = false;  // the "(N times in M ms)" form rather than a bare line
+    int  count = 0;          // assertions being reported; meaningful when warn
+    bool restartClock = false;
+};
+
+// `assertions` counts RISING EDGES of the flag seen since the last flush -- not
+// telemetry updates, and not the flag's level. `clockValid` is false before the
+// first flush has ever run.
+inline AdcOverloadWarn adcOverloadWarn(int assertions,
+                                       bool clockValid,
+                                       std::int64_t elapsedMs,
+                                       std::int64_t intervalMs)
+{
+    AdcOverloadWarn out;
+    if (assertions <= 0) {
+        return out;                      // nothing seen; the window keeps running
+    }
+    // An invalid clock is the first assertion ever: report it now rather than
+    // making the operator wait out a window that has not started.
+    const bool windowOpen = clockValid && elapsedMs < intervalMs;
+    if (windowOpen) {
+        return out;                      // suppressed; the count keeps accruing
+    }
+    out.warn = true;
+    out.count = assertions;
+    // ONE assertion is a hint and gets the bare line. More than one is the
+    // rate, and the rate IS the severity: a flag that sets once is a hint, one
+    // that sets on every sample for a minute is a front end being slammed.
+    out.aggregate = assertions > 1;
+    out.restartClock = true;
+    return out;
+}
+
+}  // namespace AetherSDR::hl2

tests/hl2_overload_policy_test.cpp

@@ -0,0 +1,116 @@
+// The ADC-overload warning's rate limit, as a deterministic decision.
+//
+// The behaviour is timing-dependent and was previously only reachable by
+// running a radio into a strong band for ten seconds — which is why it shipped
+// with a bug the compiler could not see: the flush called restart() on a
+// QElapsedTimer that the first-assertion path guarantees is invalid, and
+// reading elapsed time from a timer that was never started is undefined.
+// (#5381 review.)
+//
+// Hl2Backend evaluates these same functions rather than its own copy, so what
+// passes here is what the radio runs
+// (core/backends/hl2/Hl2OverloadPolicy.h).
+
+#include "core/backends/hl2/Hl2OverloadPolicy.h"
+
+#include <cstdio>
+
+using AetherSDR::hl2::adcOverloadWarn;
+
+namespace {
+
+int g_failures = 0;
+
+void check(bool ok, const char* what)
+{
+    std::printf("%s %s\n", ok ? "[ OK ]" : "[FAIL]", what);
+    if (!ok) {
+        ++g_failures;
+    }
+}
+
+constexpr std::int64_t kInterval = 10000;   // kAdcOverloadWarnIntervalMs
+
+}  // namespace
+
+int main()
+{
+    // ---- 1. The FIRST assertion reports immediately ------------------------
+    //
+    // The clock is invalid before the first flush has ever run. A limiter that
+    // made the operator wait out a window here would be silent during exactly
+    // the interval when the front end is being slammed and nobody knows yet.
+    {
+        const auto w = adcOverloadWarn(/*assertions=*/1, /*clockValid=*/false,
+                                       /*elapsedMs=*/0, kInterval);
+        check(w.warn, "the first assertion warns immediately, invalid clock");
+        check(!w.aggregate, "and takes the bare form, not the count form");
+        check(w.restartClock, "and starts the window");
+    }
+
+    // ---- 2. Chatter inside the window is SUPPRESSED, and accrues -----------
+    {
+        const auto w = adcOverloadWarn(/*assertions=*/40, /*clockValid=*/true,
+                                       /*elapsedMs=*/2500, kInterval);
+        check(!w.warn, "chatter inside the window emits nothing");
+        check(!w.restartClock, "and does not restart the window");
+    }
+
+    // ---- 3. …then AGGREGATES when the window expires -----------------------
+    {
+        const auto w = adcOverloadWarn(/*assertions=*/133, /*clockValid=*/true,
+                                       /*elapsedMs=*/kInterval, kInterval);
+        check(w.warn && w.aggregate, "the expired window reports the aggregate");
+        check(w.count == 133, "carrying the full count the window swallowed");
+    }
+
+    // ---- 4. The window flushes WITHOUT a new assertion ---------------------
+    //
+    // The decision is made on every telemetry update, not on an edge. A burst
+    // that stops must still report its tally: deciding only when the flag next
+    // asserts would hold the count until the band goes loud again, which may be
+    // hours away or never. Same inputs as case 3 — the point is that reaching
+    // this decision requires no new assertion, only the update that carries it.
+    {
+        const auto w = adcOverloadWarn(/*assertions=*/7, /*clockValid=*/true,
+                                       /*elapsedMs=*/kInterval + 5000, kInterval);
+        check(w.warn && w.count == 7,
+              "a stopped burst still reports its tally once the window expires");
+    }
+
+    // ---- 5. An isolated assertion after a quiet interval is immediate ------
+    {
+        const auto w = adcOverloadWarn(/*assertions=*/1, /*clockValid=*/true,
+                                       /*elapsedMs=*/600000, kInterval);
+        check(w.warn && !w.aggregate,
+              "an isolated overload after a long quiet reports at once, bare");
+    }
+
+    // ---- 6. Nothing seen means nothing said, however long the window -------
+    //
+    // Guards the flush being unconditional on every telemetry update: without
+    // this the limiter would warn on a quiet radio forever.
+    {
+        const auto a = adcOverloadWarn(0, true, kInterval * 100, kInterval);
+        const auto b = adcOverloadWarn(0, false, 0, kInterval);
+        check(!a.warn && !a.restartClock,
+              "no assertions, expired window: silence");
+        check(!b.warn && !b.restartClock,
+              "no assertions, invalid clock: silence");
+    }
+
+    // ---- 7. The boundary is expiry, not strictly-greater --------------------
+    {
+        const auto below = adcOverloadWarn(5, true, kInterval - 1, kInterval);
+        const auto at    = adcOverloadWarn(5, true, kInterval, kInterval);
+        check(!below.warn, "one ms before expiry is still suppressed");
+        check(at.warn, "at expiry it reports — hasExpired() is inclusive");
+    }
+
+    if (g_failures == 0) {
+        std::printf("\nALL PASS\n");
+        return 0;
+    }
+    std::printf("\nFAILURES PRESENT\n");
+    return 1;
+}

tests/tests.cmake

@@ -4062,6 +4062,11 @@ add_executable(host_voice_chain_policy_test
 )
 target_include_directories(host_voice_chain_policy_test PRIVATE src)
 add_test(NAME host_voice_chain_policy_test COMMAND host_voice_chain_policy_test)
+add_executable(hl2_overload_policy_test
+    tests/hl2_overload_policy_test.cpp
+)
+target_include_directories(hl2_overload_policy_test PRIVATE src)
+add_test(NAME hl2_overload_policy_test COMMAND hl2_overload_policy_test)
 add_executable(hl2_dsp_setup_policy_test
     tests/hl2_dsp_setup_policy_test.cpp
 )