SPE: reply-paced LCD polling and a calmer freshness gate — no more mirror blink

docs/architecture/spe-expert-amplifier-design.md

@@ -315,9 +315,22 @@ control application and re-validated against the real 1.5K-FA (see
 `THIRD_PARTY_LICENSES` for the provenance chain, which ends at the
 MIT-licensed expert-amp-server project) — is:
 
-- **Request**: the standard keystroke-style packet with code `0x80`,
-  polled at 600 ms (the field-proven cadence; the frame is ~5x a Status
-  reply, and the mirror is for eyes, not telemetry).
+- **Request**: the standard keystroke-style packet with code `0x80`.
+  Polling is reply-paced on a single-shot timer: a request arms only a
+  1 s lost-reply fallback, and each decoded display re-arms the short
+  250 ms gap — so the effective cadence is gap plus round trip plus the
+  link's own serialization time for the 371-byte frame (~285 ms total at
+  115200; a 19200 proxy serial side stretches it to ~450 ms on its own).
+  As long as the round trip stays under the 1 s fallback — which covers
+  the slowest plausible link, a 9600 baud serial side spending ~390 ms on
+  the frame alone — a second request is never issued while the previous
+  reply is still arriving: the amplifier is never asked to interleave
+  display blocks, and a slow link stretches the cadence instead of
+  accumulating a request backlog. A round trip beyond the fallback is
+  treated as a lost reply and retried, accepting the overlap risk on a
+  link that degenerate. At a 9600 baud proxy serial side the 100 ms
+  Status poll alone consumes ~80% of the wire, so ser2net serial sides
+  should be configured at 57600 or above.
 - **Reply**: `AA AA AA | 6A 01` (16-bit payload length, 362) `| 95 FE |
   ` 2-byte inverted flag word |` 320 character bytes (8 rows x 40
   columns, row-major) + 40 attribute bytes (one per column, bit N =
@@ -343,6 +356,34 @@ With the mirror on screen, the FRONT PANEL keys stop being blind — the
 operator navigates the amplifier's menu watching the amplifier's screen,
 which is what unlocked the §4 ruling change. Those keys remain disabled
 until the first checksum-valid display arrives and are disabled again after
-two missed 600 ms refreshes. Every acknowledged keystroke requests an
-immediate display refresh and resets the periodic cadence, so a fast menu
-sequence does not have to wait a full polling interval to show its result.
+2.4 s without one — an absolute window sized to cover a lost frame plus a
+retry even on a 9600 baud proxy serial side AND the amplifier's own quiet
+spells around OPERATE/STANDBY relay transitions, because routine events
+must read as a hiccup, not flap the gate. A display frame that arrives
+complete but fails validation triggers a prompt re-request (80 ms pause;
+each retry is itself provoked by a full received-and-rejected frame, so
+the retry stream is self-limited by the link's serialization time): the
+field case is strong RF near the serial run mid-transmit, where the
+371-byte display reply dies to bit errors far more often than the 76-byte
+Status reply, and one clean frame every second or two is all the mirror
+needs to stay live through a transmission. Losing freshness changes
+nothing on the glass: the mirror holds its newest image at full
+brightness, exactly like the amplifier's own LCD holds its picture, and
+the disabled key group is the one and only not-live signal. (Both
+alternatives were field-tested and rejected: blanking the glass made the
+mirror blink in and out, and even a light dim read as the LCD switching
+off — display gaps of one to several seconds are ROUTINE on a
+best-effort link, in plain standby on a quiet band, so any visible
+staleness treatment fires constantly and punishes the operator without
+adding safety the key gate doesn't already provide.) The mirror only
+returns to the idle glass when the image is truly obsolete (disconnect,
+or a docked⇄floating switch). Every
+acknowledged keystroke requests an immediate display refresh, and the
+cadence re-arms from each display *reply* rather than free-running: the
+original free-running 600 ms period was an exact multiple of the 100 ms
+Status poll, and two such timers phase-lock with every display reply
+straddling a status poll on the wire, dropping display frames in bursts
+until clock drift walks the alignment out. Pacing from the reply folds the
+amplifier's variable response latency into the period, so no stable phase
+relationship can form — and it is also what makes the small 250 ms gap
+safe on slow links (see the request bullet above).

src/core/SpeConnection.cpp

@@ -19,8 +19,27 @@ SpeConnection::SpeConnection(QObject* parent)
         // a moving display must not keep stale telemetry/buttons looking live.
         if (const auto frame = Spe::Lcd::decode(raw)) {
             emit lcdFrameReceived(*frame);
-            setLcdFresh(true);
-            m_lcdStaleTimer.start();
+            if (m_lcdWanted && m_connected) {
+                setLcdFresh(true);
+                m_lcdStaleTimer.start();
+                m_lcdRetryTimer.stop();  // a good frame supersedes a pending retry
+                // Pace the next request from the REPLY: the request path
+                // armed only the long lost-reply fallback, so this re-arm
+                // to the short gap is the one that sets the cadence. Two
+                // free-running timers whose periods divide evenly (the
+                // original 600 ms cadence was an exact multiple of the
+                // 100 ms Status poll) phase-lock — Qt's coarse timers
+                // actively coalesce them — with every display reply
+                // straddling a status poll on the wire, and hold that
+                // alignment for many seconds until clock drift walks out
+                // of it, dropping several display frames in a row. Pacing
+                // from the reply folds the amp's own (variable) response
+                // latency into the period, so no stable phase relationship
+                // with the status poll can form — and it is what lets
+                // kLcdPollIntervalMs be a small idle gap rather than a
+                // conservative worst-case-link period.
+                m_lcdTimer.start(kLcdPollIntervalMs);
+            }
         }
     });
 
@@ -46,14 +65,46 @@ SpeConnection::SpeConnection(QObject* parent)
     m_powerOnTimer.setSingleShot(true);
     connect(&m_powerOnTimer, &QTimer::timeout, this, &SpeConnection::powerOnStep);
 
+    // Single-shot on purpose: the cadence is reply-paced (armed with the
+    // short gap from each decoded display reply), and a request arms only
+    // the LONG lost-reply interval — so the next request cannot fire while
+    // the previous reply is still in flight unless the round trip exceeds
+    // kLcdLostReplyMs, which the design note states as the condition.
+    m_lcdTimer.setSingleShot(true);
     m_lcdTimer.setInterval(kLcdPollIntervalMs);
     connect(&m_lcdTimer, &QTimer::timeout, this, &SpeConnection::requestLcdFrame);
 
     m_lcdStaleTimer.setSingleShot(true);
     m_lcdStaleTimer.setInterval(kLcdStaleTimeoutMs);
     connect(&m_lcdStaleTimer, &QTimer::timeout, this, [this]() {
+        // Routine on best-effort links (a stall, an amp quiet spell, RF
+        // mid-transmit) — logged so field reports can measure the gaps.
+        qCDebug(lcTuner) << "SpeConnection: no valid display frame for"
+                         << kLcdStaleTimeoutMs << "ms — menu keys gated until"
+                            " the next one";
         setLcdFresh(false);
     });
+
+    // A display frame that died on the wire is re-requested promptly (the
+    // field case: strong RF near the serial run mid-transmit corrupts the
+    // long display replies far more often than the short Status ones, and
+    // one clean frame every second or two is all the mirror needs to stay
+    // live). The short pause is the flood guard: each retry can only be
+    // provoked by a complete received-and-rejected frame, so the loop is
+    // additionally self-limited by the link's own serialization time.
+    m_lcdRetryTimer.setSingleShot(true);
+    m_lcdRetryTimer.setInterval(kLcdRetryGapMs);
+    connect(&m_lcdRetryTimer, &QTimer::timeout, this, &SpeConnection::requestLcdFrame);
+    m_parser.setDisplayRejectCallback([this]() {
+        if (!m_lcdWanted || !m_connected) {
+            return;
+        }
+        qCDebug(lcTuner) << "SpeConnection: display frame failed validation —"
+                            " re-requesting";
+        if (!m_lcdRetryTimer.isActive()) {
+            m_lcdRetryTimer.start();
+        }
+    });
 }
 
 void SpeConnection::setLcdPolling(bool on)
@@ -68,6 +119,7 @@ void SpeConnection::setLcdPolling(bool on)
     } else {
         m_lcdTimer.stop();
         m_lcdStaleTimer.stop();
+        m_lcdRetryTimer.stop();
         setLcdFresh(false);
     }
 }
@@ -78,9 +130,11 @@ void SpeConnection::requestLcdFrame()
         return;
     }
     sendRaw(Spe::Lcd::buildRequest());
-    // An ACK-triggered refresh resets the periodic cadence, avoiding an
-    // immediate duplicate request from the timer that may already be near due.
-    m_lcdTimer.start();
+    // A request arms only the lost-reply fallback. The short-gap re-arm
+    // lives in the display callback, so on a slow link the loop waits for
+    // the reply (or this timeout) rather than free-running a fixed cadence
+    // into a still-transmitting frame — the review-caught failure mode.
+    m_lcdTimer.start(kLcdLostReplyMs);
 }
 
 void SpeConnection::setLcdFresh(bool fresh)
@@ -198,6 +252,7 @@ void SpeConnection::disconnect()
     m_pollTimer.stop();
     m_lcdTimer.stop();
     m_lcdStaleTimer.stop();
+    m_lcdRetryTimer.stop();
     setLcdFresh(false);
     m_powerOnTimer.stop();
     m_powerOnStep = -1;
@@ -259,6 +314,7 @@ void SpeConnection::onTransportDown()
     m_pollTimer.stop();
     m_lcdTimer.stop();
     m_lcdStaleTimer.stop();
+    m_lcdRetryTimer.stop();
     setLcdFresh(false);
     m_powerOnTimer.stop();
     m_powerOnStep = -1;
@@ -360,8 +416,8 @@ void SpeConnection::onFrameReceived(const Spe::Frame& f)
         qCDebug(lcTuner) << "SpeConnection: ACK for command"
                           << QString::number(static_cast<quint8>(f.data.at(0)), 16);
         // The keys are safe only beside a fresh mirror. Pull the resulting
-        // screen immediately instead of making a fast menu sequence wait up
-        // to the next 600 ms periodic refresh.
+        // screen immediately instead of making a fast menu sequence wait
+        // out the rest of the current poll gap.
         requestLcdFrame();
         return;
     }

src/core/SpeConnection.h

@@ -75,8 +75,9 @@ class SpeConnection : public QObject {
     void switchOff() { sendKey(Spe::Key::SwitchOff); }
 
     // Remote LCD mirroring: while enabled (and connected) the amplifier's
-    // display is polled with the 0x80 request at kLcdPollIntervalMs and
-    // every decoded refresh arrives via lcdFrameReceived. Driven by the
+    // display is polled with the 0x80 request — each reply schedules the
+    // next request kLcdPollIntervalMs later — and every decoded refresh
+    // arrives via lcdFrameReceived. Driven by the
     // applet's floating state — the docked rail has no room for the LCD,
     // so polling it there would be pure link noise.
     void setLcdPolling(bool on);
@@ -108,9 +109,10 @@ class SpeConnection : public QObject {
     void connectionFailed(const QString& errorString);
     void statusUpdated(const AetherSDR::Spe::Status& status);
     void lcdFrameReceived(const AetherSDR::Spe::Lcd::Frame& frame);
-    // True only after a checksum-valid LCD reply, and false again after two
-    // missed 600 ms refreshes or whenever LCD polling/transport stops. The
-    // floating menu keys use this independently of Status liveness.
+    // True only after a checksum-valid LCD reply, and false again after
+    // kLcdStaleTimeoutMs without one, or whenever LCD polling/transport
+    // stops. The floating menu keys use this independently of Status
+    // liveness.
     void lcdFreshChanged(bool fresh);
     // Fires on the first Status reply of a connection and again if the
     // reported ID ever changes (in practice: never mid-session). The GUI
@@ -173,10 +175,42 @@ private slots:
     // against Status's ~76, and the panel is for eyes, not telemetry.
     QTimer m_lcdTimer;
     QTimer m_lcdStaleTimer;
+    QTimer m_lcdRetryTimer;  // single-shot reject->re-request pause
     bool   m_lcdWanted{false};
     bool   m_lcdFresh{false};
-    static constexpr int kLcdPollIntervalMs = 600;
-    static constexpr int kLcdStaleTimeoutMs = kLcdPollIntervalMs * 2;
+    // The IDLE GAP between a display reply and the next request, not a
+    // free-running period: m_lcdTimer is single-shot, a REQUEST arms only
+    // the kLcdLostReplyMs fallback, and only a decoded REPLY re-arms this
+    // short gap — so the effective cadence is gap + round trip + the
+    // link's serialization time for the 371-byte frame (~32 ms at 115200,
+    // ~193 ms at 19200), and a second request cannot be issued while the
+    // previous reply is still arriving unless the round trip exceeds
+    // kLcdLostReplyMs. That self-clocking is what makes a small gap safe
+    // on slow links: the amp is never asked to interleave display blocks
+    // and the cadence stretches instead of piling up. (At ≤9600 the
+    // 100 ms Status poll alone nearly saturates the wire — see the design
+    // note §11's proxy baud recommendation.)
+    static constexpr int kLcdPollIntervalMs = 250;
+    // Lost-reply fallback: armed at request time, superseded by the reply
+    // re-arm above. Sized above the worst plausible round trip (a 9600
+    // baud proxy serial side spends ~390 ms serializing the frame alone),
+    // so within it a request is either answered or genuinely lost — never
+    // merely still in flight.
+    static constexpr int kLcdLostReplyMs = 1000;
+    // Prompt-retry pause after a display frame fails validation (see the
+    // parser's reject callback). Short enough that a mostly-corrupted
+    // mid-transmit stream still lands a clean frame within the staleness
+    // window whenever one gets through at all; long enough that the retry
+    // stream (each retry also provoked by a full received frame) stays
+    // well under the wire's capacity even at 115200 with Status polling.
+    static constexpr int kLcdRetryGapMs = 80;
+    // Absolute, deliberately decoupled from the poll gap: it must cover a
+    // full lost frame plus a retry on the slowest plausible link (a 9600
+    // baud proxy serial side spends ~390 ms per display frame) AND the
+    // amplifier's own quiet spells — it stops serving the display for a
+    // moment around OPERATE/STANDBY relay transitions — so routine events
+    // never flap the gate. On a fast link the margin only calms things.
+    static constexpr int kLcdStaleTimeoutMs = 2400;
 
     QString m_currentModelId;
 

src/core/SpeProtocol.cpp

@@ -201,6 +201,13 @@ void FrameParser::feed(const QByteArray& bytes)
                     || telnet.state == DisplayCandidateState::Incomplete) {
                     return;
                 }
+                // Complete in both readings and valid in neither: a display
+                // frame died on the wire. Report it before resyncing so the
+                // owner can re-request promptly instead of waiting out the
+                // rest of its poll gap with a dead mirror.
+                if (m_onDisplayReject) {
+                    m_onDisplayReject();
+                }
                 if (!resyncToNextSync()) {
                     return;
                 }

src/core/SpeProtocol.h

@@ -114,6 +114,13 @@ class FrameParser {
     // payload-length + type-marker bytes and hands the complete raw frame
     // here instead of misreading the length field as a CNT byte.
     void setDisplayCallback(std::function<void(const QByteArray&)> cb) { m_onDisplay = std::move(cb); }
+    // Fires when a complete display-shaped frame fails validation in both
+    // its raw and telnet-escaped readings and is dropped. A display reply
+    // is 371 bytes against Status's ~76, so under the same bit-error rate
+    // (strong RF near the serial run mid-transmit is the field case) it is
+    // ~5x as likely to die — and unlike Status, nothing re-polls it for
+    // most of a poll gap. The owner uses this to schedule a prompt retry.
+    void setDisplayRejectCallback(std::function<void()> cb) { m_onDisplayReject = std::move(cb); }
     void feed(const QByteArray& bytes);
     void reset() { m_buf.clear(); }
 
@@ -126,6 +133,7 @@ class FrameParser {
     QByteArray m_buf;
     std::function<void(const Frame&)> m_onFrame;
     std::function<void(const QByteArray&)> m_onDisplay;
+    std::function<void()> m_onDisplayReject;
 };
 
 // ── Status string decode (spec §5) ───────────────────────────────────────

src/gui/SpeApplet.cpp

@@ -380,6 +380,10 @@ void SpeApplet::setFloating(bool floating)
         return;
     }
     m_floating = floating;
+    // A presentation switch starts the mirror over — whatever image is held
+    // is from the previous floating session, not merely stale, so drop it
+    // to the idle glass before the freshness gate re-applies.
+    m_lcd->clear();
     setLcdFresh(false);
     applyDensity();
     // The LCD mirror only exists in the floating presentation — start (or
@@ -394,10 +398,18 @@ void SpeApplet::setLcdFrame(const AetherSDR::Spe::Lcd::Frame& frame)
 
 void SpeApplet::setLcdFresh(bool fresh)
 {
+    // Freshness gates the FRONT PANEL keys and nothing else. The glass
+    // deliberately keeps its last image at full brightness: display frames
+    // stop for seconds at a time in routine operation (link stalls, the
+    // amp's own quiet spells around relay transitions, RF bursts on the
+    // serial run mid-transmit), and every attempt to mark those moments on
+    // the glass — blanking it, then dimming it — field-tested as the
+    // mirror visibly "switching off" over and over. The authoritative
+    // not-live signal is the disabled key group; the mirror, like the
+    // amplifier's own LCD, just shows the newest picture it has. Hard
+    // clears remain where the image is truly obsolete (disconnect,
+    // presentation switch).
     m_lcdFresh = fresh;
-    if (!fresh) {
-        m_lcd->clear();
-    }
     updateCommandsEnabled();
 }
 

src/gui/SpeLcdWidget.h

@@ -23,8 +23,14 @@ class SpeLcdWidget : public QWidget {
     explicit SpeLcdWidget(QWidget* parent = nullptr);
 
     void setFrame(const Spe::Lcd::Frame& frame);
-    // Back to the idle glass (dim "no display data" hint) — used when the
-    // amplifier goes silent or the connection drops.
+    // Back to the idle glass ("waiting for display…" hint) — used only
+    // when the held image is truly obsolete: the connection dropped, or a
+    // presentation switch started the mirror over. While connected, the
+    // widget keeps its newest frame at full brightness no matter how old
+    // it is — display frames routinely pause for seconds (link stalls,
+    // the amp's relay transitions, RF mid-transmit), and marking those
+    // moments on the glass field-tested as the mirror "switching off";
+    // staleness gates the applet's menu keys, never this glass.
     void clear();
 
     QSize minimumSizeHint() const override;