diff --git a/docs/architecture/spe-expert-amplifier-design.md b/docs/architecture/spe-expert-amplifier-design.md index 7a26ebca6..3280103cb 100644 --- a/docs/architecture/spe-expert-amplifier-design.md +++ b/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). diff --git a/src/core/SpeConnection.cpp b/src/core/SpeConnection.cpp index f34817a6b..62cefe022 100644 --- a/src/core/SpeConnection.cpp +++ b/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(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; } diff --git a/src/core/SpeConnection.h b/src/core/SpeConnection.h index 95f239c5a..7957ac27c 100644 --- a/src/core/SpeConnection.h +++ b/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; diff --git a/src/core/SpeProtocol.cpp b/src/core/SpeProtocol.cpp index 2ca1d011e..2460450d6 100644 --- a/src/core/SpeProtocol.cpp +++ b/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; } diff --git a/src/core/SpeProtocol.h b/src/core/SpeProtocol.h index 64f3ea8a9..45b8a1c7e 100644 --- a/src/core/SpeProtocol.h +++ b/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 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 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 m_onFrame; std::function m_onDisplay; + std::function m_onDisplayReject; }; // ── Status string decode (spec §5) ─────────────────────────────────────── diff --git a/src/gui/SpeApplet.cpp b/src/gui/SpeApplet.cpp index f888f6c78..59cd2c0cf 100644 --- a/src/gui/SpeApplet.cpp +++ b/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(); } diff --git a/src/gui/SpeLcdWidget.h b/src/gui/SpeLcdWidget.h index b1d393dff..270d94727 100644 --- a/src/gui/SpeLcdWidget.h +++ b/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; diff --git a/tests/spe_protocol_test.cpp b/tests/spe_protocol_test.cpp index 824986bbd..eb72a7911 100644 --- a/tests/spe_protocol_test.cpp +++ b/tests/spe_protocol_test.cpp @@ -409,6 +409,27 @@ int main() report("bad display checksum resyncs without consuming the following ACK", afterBadChecksum.isEmpty() && ackAfterBadChecksum.size() == 1); + // The reject callback is the mirror's retry trigger: it must fire + // exactly once per display frame that died on the wire, never for a + // frame that is merely still arriving, and a clean frame afterward + // must still be handed out (mid-transmit RF corrupting most display + // replies is the field case this recovers). + int rejects = 0; + QList afterReject; + FrameParser rejectParser; + rejectParser.setDisplayRejectCallback([&]() { ++rejects; }); + rejectParser.setDisplayCallback( + [&](const QByteArray& d) { afterReject.append(d); }); + rejectParser.feed(badChecksum.left(200)); + report("no reject while the display frame is still incomplete", + rejects == 0); + rejectParser.feed(badChecksum.mid(200)); + report("a complete corrupted display frame fires one reject", + rejects == 1); + rejectParser.feed(raw); + report("a clean display frame after a reject is still handed out", + afterReject.size() == 1 && afterReject.at(0) == raw && rejects == 1); + QByteArray telnet; for (char byte : raw) { telnet.append(byte);