diff --git a/src/gui/AtuPreTuneDialog.cpp b/src/gui/AtuPreTuneDialog.cpp index 25183e640..9687ee8eb 100644 --- a/src/gui/AtuPreTuneDialog.cpp +++ b/src/gui/AtuPreTuneDialog.cpp @@ -642,16 +642,10 @@ void AtuPreTuneDialog::beginNextPoint() if (firstOfBand && !m_originalPanId.isEmpty() && p.bandHighMhz > p.bandLowMhz) { const double center = (p.bandLowMhz + p.bandHighMhz) / 2.0; const double width = (p.bandHighMhz - p.bandLowMhz) * 1.10; - const QString centerStr = QString::number(center, 'f', 6); - const QString widthStr = QString::number(width, 'f', 6); - if (auto* pan = m_radio->panadapter(m_originalPanId)) { - // Local model nudge before the radio echo (aetherd RFC 2.3: the - // normalized setter replaces applyPanStatus({center,bandwidth})). - pan->setCenterBandwidth(center, width); - } - m_radio->sendCommand( - QString("display pan set %1 center=%2 bandwidth=%3") - .arg(m_originalPanId, centerStr, widthStr)); + // Center and bandwidth together, and deferred rather than dropped if a + // profile load is holding radio-state writes (#4142). The local model + // advances only when the command reaches the wire. + m_radio->requestPanCenter(m_originalPanId, center, width); } // Move slice to target. SliceModel::setFrequency uses autopan=0 — no recenter. @@ -885,16 +879,12 @@ void AtuPreTuneDialog::restoreOriginalFrequency() // Restore the panadapter zoom captured at sweep start — same // optimistic-update pattern used for band transitions. if (!m_originalPanId.isEmpty() && m_originalPanBandwidthMhz > 0.0) { - const QString centerStr = QString::number(m_originalPanCenterMhz, 'f', 6); - const QString widthStr = QString::number(m_originalPanBandwidthMhz, 'f', 6); - if (auto* pan = m_radio->panadapter(m_originalPanId)) { - // Local model nudge before the radio echo (aetherd RFC 2.3: the - // normalized setter replaces applyPanStatus({center,bandwidth})). - pan->setCenterBandwidth(m_originalPanCenterMhz, m_originalPanBandwidthMhz); - } - m_radio->sendCommand( - QString("display pan set %1 center=%2 bandwidth=%3") - .arg(m_originalPanId, centerStr, widthStr)); + // Restoring the pre-sweep zoom is a user-visible promise: dropping it + // during a profile load would strand the pan on the sweep's band view. + // Deferred and replayed instead (#4142). + m_radio->requestPanCenter(m_originalPanId, + m_originalPanCenterMhz, + m_originalPanBandwidthMhz); } } diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index ab6db6f99..2500de1aa 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -6080,28 +6080,28 @@ void MainWindow::applyPanRangeRequest(const QString& panId, double centerMhz, } auto* pan = m_radioModel.panadapter(panId); - const QString centerStr = QString::number(centerMhz, 'f', 6); - const QString bandwidthStr = QString::number(bandwidthMhz, 'f', 6); if (pan) { if (qFuzzyCompare(pan->centerMhz(), centerMhz) && qFuzzyCompare(pan->bandwidthMhz(), bandwidthMhz)) { return; } - // Update both values together before the radio echo arrives. Explicit - // zoom workflows are especially sensitive to center/bandwidth skew; - // splitting them produced the P1/P2 waterfall-loss and zoom-drift bugs. - // (aetherd RFC 2.3: setCenterBandwidth replaces applyPanStatus here.) - pan->setCenterBandwidth(centerMhz, bandwidthMhz); } - m_radioModel.sendCommand( - QString("display pan set %1 center=%2 bandwidth=%3") - .arg(panId, centerStr, bandwidthStr)); + // Center and bandwidth travel together in one command. Explicit zoom + // workflows are especially sensitive to center/bandwidth skew; splitting + // them produced the P1/P2 waterfall-loss and zoom-drift bugs. + // + // #4142: this pair is classified profile-owned, so it was silently DROPPED + // during a profile load — zoom and drag, not just typed frequency entry. + // requestPanCenter() defers and replays it, and only advances the local + // model when the command actually reaches the wire. + const bool sent = m_radioModel.requestPanCenter(panId, centerMhz, bandwidthMhz); qDebug() << "Pan range request:" << source << "center" << centerMhz - << "bandwidth" << bandwidthMhz; + << "bandwidth" << bandwidthMhz + << (sent ? "" : "(deferred: profile load)"); } void MainWindow::setActiveSlice(int sliceId) @@ -7898,16 +7898,24 @@ void MainWindow::centerActiveSliceInPanadapter(bool forceRadioCenter, double cen m_panStack->setActivePan(applet->panId()); } - // Keep the local spectrum centered immediately so the active slice marker - // is visible before the radio's status echo arrives. - sw->setFrequencyRange(targetMhz, bandwidthMhz); - pushSliceFrequencyToOverlays(s, targetMhz); - + // #4142 — ask the radio FIRST, so the local view can only advance if the + // command actually reached the wire. During a profile load this center write + // is suppressed; centering the spectrum on a center the radio never took is + // exactly what projects honest tiles into a lying frame and bakes black rows + // into waterfall history. requestPanCenter() defers and replays it instead. + bool centerDeferred = false; if (forceRadioCenter && m_radioModel.isConnected()) { - m_radioModel.sendCommand( - QString("display pan set %1 center=%2") - .arg(s->panId()).arg(targetMhz, 0, 'f', 6)); + centerDeferred = !m_radioModel.requestPanCenter(s->panId(), targetMhz); + } + + // Keep the local spectrum centered immediately so the active slice marker is + // visible before the radio's status echo arrives — unless the center was + // deferred, in which case the pan must keep showing truthful spectrum for the + // span the radio still has. + if (!centerDeferred) { + sw->setFrequencyRange(targetMhz, bandwidthMhz); } + pushSliceFrequencyToOverlays(s, targetMhz); TuneCenteringResult result; result.oldCenterMhz = pan ? pan->centerMhz() : targetMhz; diff --git a/src/gui/MainWindow_DigitalModes.cpp b/src/gui/MainWindow_DigitalModes.cpp index c5b78d71e..c326dd991 100644 --- a/src/gui/MainWindow_DigitalModes.cpp +++ b/src/gui/MainWindow_DigitalModes.cpp @@ -1194,18 +1194,18 @@ void MainWindow::activateWFM(int sliceId) m_wfmSliceId = sliceId; // Centre the pan (and with it the DAX IQ stream) on the slice — once. - // applyPanStatus updates the local model immediately so offsets computed - // before the radio echoes the new centre are already correct. + // requestPanCenter() updates the local model as it puts the command on the + // wire, so offsets computed before the radio echoes the new centre are + // already correct — and during a profile load it defers the write instead of + // letting it be dropped, which would have left the DAX IQ stream centred + // somewhere the client no longer believed it was (#4142). auto centerPanAtSlice = [this, s]() { const QString panId = s->panId(); if (panId.isEmpty()) return; const double freq = s->frequency(); auto* pan = m_radioModel.panadapter(panId); if (pan && qFuzzyCompare(pan->centerMhz(), freq)) return; - const QString freqStr = QString::number(freq, 'f', 6); - if (pan) pan->setCenterBandwidth(freq, -1.0); // aetherd RFC 2.3 - m_radioModel.sendCommand( - QString("display pan set %1 center=%2").arg(panId, freqStr)); + m_radioModel.requestPanCenter(panId, freq); }; centerPanAtSlice(); diff --git a/src/gui/MainWindow_Wiring.cpp b/src/gui/MainWindow_Wiring.cpp index edb3aaf56..c9094b64b 100644 --- a/src/gui/MainWindow_Wiring.cpp +++ b/src/gui/MainWindow_Wiring.cpp @@ -467,23 +467,31 @@ bool MainWindow::snapCenterLockForSlice(SliceModel* slice, double mhz, bool send bandwidthMhz > 0.0 ? std::max(mhz, bandwidthMhz / 2.0) : mhz; bool changed = false; - if (sw && bandwidthMhz > 0.0 - && (!qFuzzyCompare(sw->centerMhz(), targetCenterMhz) - || !qFuzzyCompare(sw->bandwidthMhz(), bandwidthMhz))) { - sw->setFrequencyRangeImmediate(targetCenterMhz, bandwidthMhz); - changed = true; - } - const bool modelNeedsCenter = !qFuzzyCompare(pan->centerMhz(), targetCenterMhz); + bool centerDeferred = false; + if (!kiwiDisplayActive && modelNeedsCenter) { - pan->setCenterBandwidth(targetCenterMhz, -1.0); // aetherd RFC 2.3 - changed = true; + if (sendCommand) { + // #4142 — defer, never drop. requestPanCenter() advances the local + // model only when the command actually reaches the wire, and queues + // it for replay when a profile load is holding radio-state writes. + centerDeferred = !m_radioModel.requestPanCenter(panId, targetCenterMhz); + } else { + // Local-only snap: the caller explicitly wants no radio write, so + // there is no wire command for the model to diverge from. + pan->setCenterBandwidth(targetCenterMhz, -1.0); // aetherd RFC 2.3 + } + changed = !centerDeferred; } - if (sendCommand && !kiwiDisplayActive && modelNeedsCenter) { - m_radioModel.sendCommand( - QStringLiteral("display pan set %1 center=%2") - .arg(panId, QString::number(targetCenterMhz, 'f', 6))); + // Never move the visible span while the radio stays put. A view that claims + // a center the radio never took is precisely what projects honest tiles into + // a lying frame and bakes black rows into waterfall history (#4142). + if (!centerDeferred && sw && bandwidthMhz > 0.0 + && (!qFuzzyCompare(sw->centerMhz(), targetCenterMhz) + || !qFuzzyCompare(sw->bandwidthMhz(), bandwidthMhz))) { + sw->setFrequencyRangeImmediate(targetCenterMhz, bandwidthMhz); + changed = true; } return changed; @@ -1952,6 +1960,15 @@ void MainWindow::scheduleProfileLoadRecovery(const QString& profileType, }); QTimer::singleShot(kProfileLoadDeferredPanFlushDelayMs, this, [this]() { flushPendingProfileLoadPanDimensions(); + // Ordered after dimensions on the SAME timer — no fourth scheduler. + // Bin size is bandwidth/xpixels, so the pan must know its pixel geometry + // before it is recentered. This timer is the first flush point past hold + // expiry; the two earlier dimension flushes (1500/3500 ms) run INSIDE the + // hold, where xpixels/ypixels are exempt but a center write is not. + // flushPendingProfileLoadPanCenters() re-checks the hold itself and + // early-returns while it is armed, so no center can escape early even if + // this call site were moved. (#4142) + m_radioModel.flushPendingProfileLoadPanCenters(); }); QTimer::singleShot(kProfileLoadPostHoldRecoveryDelayMs, this, [this]() { m_profileLoadPendingFftYpixels.clear(); @@ -2411,13 +2428,13 @@ void MainWindow::wirePanadapter(PanadapterApplet* applet) } if (auto* pan = m_radioModel.panadapter(applet->panId())) { center = std::max(center, pan->bandwidthMhz() / 2.0); - // The radio may acknowledge this command without echoing a display - // status to the setting client. Keep the canonical model aligned - // with the visible pan so TCI dds: follows the IQ center. - pan->setCenterBandwidth(center, -1.0); } - m_radioModel.sendCommand( - QString("display pan set %1 center=%2").arg(applet->panId()).arg(center, 0, 'f', 6)); + // The radio may acknowledge this command without echoing a display status + // to the setting client, so requestPanCenter() keeps the canonical model + // aligned with the wire command (TCI dds: follows this center) — and + // during a profile load it defers rather than letting sendCmd drop it + // silently (#4142). + m_radioModel.requestPanCenter(applet->panId(), center); }); // Band/Segment Zoom toggle off the pan's radio-authoritative model state // (togglePanZoomModeForPan) — shared with the keyboard/MIDI shortcuts @@ -3214,9 +3231,11 @@ void MainWindow::wirePanadapter(PanadapterApplet* applet) // In-window drag moves pass the unchanged center purely to tune // without reveal — only emit the pan command when it actually moves. if (!qFuzzyCompare(centerMhz, pan->centerMhz())) { - m_radioModel.sendCommand( - QString("display pan set %1 center=%2") - .arg(pan->panId()).arg(centerMhz, 0, 'f', 6)); + // Deferred rather than dropped during a profile load (#4142). + // The SpectrumWidget owns its own view during an edge-pan drag, + // so the gesture still tracks the cursor; the radio catches up + // when the deferred center flushes. + m_radioModel.requestPanCenter(pan->panId(), centerMhz); } } queueActiveSliceForSpectrumTarget(target->sliceId()); @@ -3665,10 +3684,12 @@ MainWindow::TuneCenteringResult MainWindow::revealFrequencyIfNeeded( ? kPanFollowAnimationDurationMs : 0; - pan->setCenterBandwidth(result.newCenterMhz, -1.0); // aetherd RFC 2.3 - m_radioModel.sendCommand( - QString("display pan set %1 center=%2") - .arg(pan->panId()).arg(result.newCenterMhz, 0, 'f', 6)); + // #4142 — defer, never drop. During a profile load the pan center write + // is suppressed at the wire; advancing the local center anyway is what + // made the waterfall go black (the view claimed a center the radio never + // took). requestPanCenter() applies the model update only when the + // command actually goes out, and replays it once the hold lifts. + m_radioModel.requestPanCenter(pan->panId(), result.newCenterMhz); return result; } @@ -3752,13 +3773,13 @@ MainWindow::TuneCenteringResult MainWindow::revealFrequencyIfNeeded( ? kPanFollowAnimationDurationMs : 0; - // Apply the center optimistically so the owning spectrum repaints - // immediately; SpectrumWidget decides whether that becomes a short - // retargetable animation or an immediate snap based on shift size. - pan->setCenterBandwidth(result.newCenterMhz, -1.0); // aetherd RFC 2.3 - m_radioModel.sendCommand( - QString("display pan set %1 center=%2") - .arg(pan->panId()).arg(result.newCenterMhz, 0, 'f', 6)); + // Apply the center so the owning spectrum repaints immediately; + // SpectrumWidget decides whether that becomes a short retargetable animation + // or an immediate snap based on shift size. During a profile load this + // defers instead (#4142) — the model does not advance ahead of the radio, so + // the pan keeps showing truthful spectrum for its real span until the + // deferred center flushes. + m_radioModel.requestPanCenter(pan->panId(), result.newCenterMhz); return result; } diff --git a/src/models/ProfileLoadCommand.h b/src/models/ProfileLoadCommand.h index 7f2115056..9062688b0 100644 --- a/src/models/ProfileLoadCommand.h +++ b/src/models/ProfileLoadCommand.h @@ -1,7 +1,9 @@ #pragma once +#include #include #include +#include #include namespace AetherSDR { @@ -44,4 +46,42 @@ inline ProfileLoadCommand parseProfileLoadCommand(const QString& command) }; } +// Classifies a command as a write to radio state that a profile load owns and +// will rebuild. RadioModel::sendCmd() consults this as a low-level backstop and +// suppresses matches while the profile-load hold is armed. +// +// Callers must not rely on that backstop to swallow user-initiated writes: a +// suppressed command never reaches the wire and is lost. Route pan center / +// bandwidth writes through RadioModel::requestPanCenter(), which defers and +// coalesces them until the hold lifts instead of dropping them (#4142). +inline bool isProfileOwnedRadioStateWrite(const QString& command) +{ + const QString trimmed = command.trimmed(); + const QStringList tokens = trimmed.simplified().split(QLatin1Char(' '), Qt::SkipEmptyParts); + if (tokens.size() >= 5 + && tokens[0] == QStringLiteral("display") + && tokens[1] == QStringLiteral("pan") + && tokens[2] == QStringLiteral("set")) { + bool hasPixelDimension = false; + for (int i = 4; i < tokens.size(); ++i) { + const QString key = tokens[i].section(QLatin1Char('='), 0, 0); + if (key != QStringLiteral("xpixels") && key != QStringLiteral("ypixels")) { + return true; + } + hasPixelDimension = true; + } + // xpixels/ypixels are allowed past this low-level guard because + // MainWindow queues them during a profile load and flushes them after + // the radio accepts the profile. Do not bypass + // MainWindow::requestPanDimensionsForRadio(); early dimension writes + // can cause the radio to save a partial GUIClient slice layout. + return !hasPixelDimension; + } + + return trimmed.startsWith(QStringLiteral("slice set ")) + || trimmed.startsWith(QStringLiteral("display pan set ")) + || trimmed.startsWith(QStringLiteral("display panafall set ")) + || trimmed.startsWith(QStringLiteral("display waterfall set ")); +} + } // namespace AetherSDR diff --git a/src/models/RadioModel.cpp b/src/models/RadioModel.cpp index 3691ea348..e1bc672db 100644 --- a/src/models/RadioModel.cpp +++ b/src/models/RadioModel.cpp @@ -103,35 +103,9 @@ bool statusFlagSet(const QMap& kvs, const QString& key) || value.compare(QStringLiteral("true"), Qt::CaseInsensitive) == 0; } -bool isProfileOwnedRadioStateWrite(const QString& command) -{ - const QString trimmed = command.trimmed(); - const QStringList tokens = trimmed.simplified().split(QLatin1Char(' '), Qt::SkipEmptyParts); - if (tokens.size() >= 5 - && tokens[0] == QStringLiteral("display") - && tokens[1] == QStringLiteral("pan") - && tokens[2] == QStringLiteral("set")) { - bool hasPixelDimension = false; - for (int i = 4; i < tokens.size(); ++i) { - const QString key = tokens[i].section(QLatin1Char('='), 0, 0); - if (key != QStringLiteral("xpixels") && key != QStringLiteral("ypixels")) { - return true; - } - hasPixelDimension = true; - } - // xpixels/ypixels are allowed past this low-level guard because - // MainWindow queues them during a profile load and flushes them after - // the radio accepts the profile. Do not bypass - // MainWindow::requestPanDimensionsForRadio(); early dimension writes - // can cause the radio to save a partial GUIClient slice layout. - return !hasPixelDimension; - } - - return trimmed.startsWith(QStringLiteral("slice set ")) - || trimmed.startsWith(QStringLiteral("display pan set ")) - || trimmed.startsWith(QStringLiteral("display panafall set ")) - || trimmed.startsWith(QStringLiteral("display waterfall set ")); -} +// isProfileOwnedRadioStateWrite() moved to ProfileLoadCommand.h so the +// classification contract has a light, dependency-free test target +// (profile_load_command_test). Behaviour unchanged. (#4142) void appendUniqueAntennaToken(QStringList& tokens, const QString& token) { @@ -2755,11 +2729,155 @@ void RadioModel::setPanCenter(double centerMhz) // out-of-range center would be optimistically stored and advertised via // TCI dds: even though the radio rejects it. centerMhz = std::max(centerMhz, pan->bandwidthMhz() / 2.0); - pan->setCenterBandwidth(centerMhz, -1.0); } - sendCmd( - QString("display pan set %1 center=%2") - .arg(m_activePanId).arg(centerMhz, 0, 'f', 6)); + requestPanCenter(m_activePanId, centerMhz); +} + +bool RadioModel::requestPanCenter(const QString& panId, + double centerMhz, + double bandwidthMhz) +{ + if (panId.isEmpty()) { + return false; + } + + // A pan center is profile-owned radio state, so sendCmd() would DROP this + // while a profile load is rebuilding the radio's topology. Defer it instead: + // queue the REQUESTED value and replay it once the hold lifts. + // + // Gate on exactly the predicate sendCmd() guards on, so "if sendCmd would + // drop it, we queue it" is an identity rather than an approximation — two + // separate clocks could skew and re-open the same silent drop. + if (profileLoadRadioStateWritesHeld()) { + PendingPanCenter& pending = m_pendingProfileLoadPanCenters[panId]; + pending.centerMhz = centerMhz; + if (bandwidthMhz > 0.0) { + pending.bandwidthMhz = bandwidthMhz; + } + + // Deliberately do NOT touch PanadapterModel here. The optimistic local + // update is the second half of #4142: with the command dropped, a client + // that advanced its own center claimed a center the radio never took. + // Honest VITA-49 tiles (each carrying its own FrameLowFreq/BinBandwidth) + // were then projected into a view that lied about its span, and the + // non-overlapping region rendered black — permanently, into history. + // Local state may only advance when a command actually reaches the wire. + qCDebug(lcProtocol).noquote() + << "RadioModel: deferring pan center during profile load" + << QStringLiteral("pan=%1").arg(panId) + << QStringLiteral("center=%1").arg(centerMhz, 0, 'f', 6); + + // Whoever defers a write owns scheduling its replay. Do NOT rely on the + // profile-load ACK to schedule the flush: the hold is armed when the + // profile-load command is SENT, but MainWindow's recovery pass (and its + // flush timers) only runs on profileLoadCompleted, which is emitted on + // ACK. A large topology (8 pans / 8 slices, verified on a 6700) can stall + // the radio long enough that it misses pings and the client force- + // disconnects BEFORE the ACK ever arrives — in which case no flush would + // ever have been scheduled and this request would be stranded forever. + armProfileLoadPanCenterFlush(); + return false; + } + + sendPanCenterToRadio(panId, centerMhz, bandwidthMhz); + return true; +} + +void RadioModel::sendPanCenterToRadio(const QString& panId, + double centerMhz, + double bandwidthMhz) +{ + if (PanadapterModel* pan = panadapter(panId)) { + // Keep the canonical model aligned with the command we are about to put + // on the wire: the radio may ACK without echoing a display status back + // to the setting client, and TCI dds: follows this center. + pan->setCenterBandwidth(centerMhz, bandwidthMhz > 0.0 ? bandwidthMhz : -1.0); + } + + // Center and bandwidth must travel together when both are requested; + // splitting them produced the P1/P2 waterfall-loss and zoom-drift bugs. + const QString command = bandwidthMhz > 0.0 + ? QString("display pan set %1 center=%2 bandwidth=%3") + .arg(panId) + .arg(centerMhz, 0, 'f', 6) + .arg(bandwidthMhz, 0, 'f', 6) + : QString("display pan set %1 center=%2") + .arg(panId) + .arg(centerMhz, 0, 'f', 6); + + sendCommand(command); +} + +void RadioModel::armProfileLoadPanCenterFlush() +{ + // Re-arm for exactly when the hold lifts. The hold can be EXTENDED after we + // arm (the ACK pushes it out again, and a second profile load pushes it out + // further), so the flush re-checks and re-arms itself rather than trusting a + // single deadline computed up front. + const qint64 remainingMs = + m_profileLoadRadioStateWriteHoldUntilMs - QDateTime::currentMSecsSinceEpoch(); + const int delayMs = static_cast(std::max(remainingMs, 0)) + 100; + + QTimer::singleShot(delayMs, this, [this]() { + flushPendingProfileLoadPanCenters(); + }); +} + +void RadioModel::flushPendingProfileLoadPanCenters() +{ + if (m_pendingProfileLoadPanCenters.isEmpty()) { + return; + } + + if (!isConnected()) { + // The session these requests belonged to is gone. Their pan ids and the + // user's intent both died with it, and the radio will rebuild its own + // topology on reconnect — replaying a pre-disconnect center could land a + // stale frequency on a pan that is no longer the same pan. Void them + // loudly rather than stranding or misapplying them. (onDisconnected() + // normally clears these first; this is the backstop.) + qCWarning(lcProtocol).noquote() + << "RadioModel: discarding" << m_pendingProfileLoadPanCenters.size() + << "deferred pan center(s) — disconnected before the profile load settled"; + m_pendingProfileLoadPanCenters.clear(); + return; + } + + // THE NON-REGRESSION PROOF, in one branch: while the hold is armed this + // returns without sending, so a deferred pan center can never put a byte on + // the wire inside the hold window. Nothing here can reintroduce the + // missing-slices corruption the hold exists to prevent. + // + // Returning WITHOUT clearing is deliberate — we re-arm instead. The hold is + // still armed only because a further topology-rebuilding profile load pushed + // it out; dropping the request here would be the exact bug this method exists + // to fix. + if (profileLoadRadioStateWritesHeld()) { + armProfileLoadPanCenterFlush(); + return; + } + + const QHash pending = + std::exchange(m_pendingProfileLoadPanCenters, {}); + + int sent = 0; + for (auto it = pending.cbegin(); it != pending.cend(); ++it) { + // Re-resolve against the LIVE pan topology. A pan that the profile load + // removed — or replaced under a new id — simply never matches, so a + // stale center can never be applied to a pan that no longer exists. + if (!panadapter(it.key())) { + continue; + } + + sendPanCenterToRadio(it.key(), it.value().centerMhz, it.value().bandwidthMhz); + ++sent; + } + + if (sent > 0) { + qCDebug(lcProtocol).noquote() + << "RadioModel: flushed deferred profile-load pan centers" + << QStringLiteral("count=%1").arg(sent); + } } void RadioModel::setPanDbmRange(float minDbm, float maxDbm) @@ -3734,6 +3852,19 @@ void RadioModel::onDisconnected() { qCDebug(lcProtocol) << "RadioModel: disconnected"; + // #4142 — void any pan centers deferred during a profile load. The session + // they belonged to is gone: the radio rebuilds its topology on reconnect, so + // a queued center could land a stale frequency on a pan that is no longer the + // same pan. This is a real path, not a theoretical one — a large profile + // (8 pans / 8 slices on a 6700) can stall the radio past the ping timeout and + // force a disconnect BEFORE the profile load is ever ACKed. + if (!m_pendingProfileLoadPanCenters.isEmpty()) { + qCWarning(lcProtocol).noquote() + << "RadioModel: discarding" << m_pendingProfileLoadPanCenters.size() + << "deferred pan center(s) — disconnected before the profile load settled"; + m_pendingProfileLoadPanCenters.clear(); + } + // Release sleep inhibition on disconnect (#1420) m_sleepInhibitor.release(); @@ -4850,9 +4981,33 @@ quint32 RadioModel::sendCmd(const QString& command, ResponseCallback cb) if (!profileLoad.valid && profileLoadRadioStateWritesHeld() && isProfileOwnedRadioStateWrite(command)) { - qCDebug(lcProtocol).noquote() - << "RadioModel: suppressing profile-load radio-state write" - << command; + // Defense-in-depth backstop only. A command that reaches here is + // DROPPED — it never gets a sequence number and never reaches the wire. + // + // Warn only for pan geometry, which is the class routed through + // requestPanCenter(): one of those arriving here means a caller bypassed + // the defer path and a user command is being lost — a real bug (#4142). + // + // The other suppressions on this path are model-echo/reconcile writers + // that #3563 drops BY DESIGN (active-slice and TX-slice reasserts, + // panafall/waterfall auto-black defaults). Several fire on every profile + // load, so warning on them would cry wolf and bury the one line that + // actually means something. + const bool panGeometryWrite = + command.startsWith(QStringLiteral("display pan set ")) + && (command.contains(QStringLiteral("center=")) + || command.contains(QStringLiteral("bandwidth="))); + + if (panGeometryWrite) { + qCWarning(lcProtocol).noquote() + << "RadioModel: DROPPED a pan geometry write during profile load —" + << "this should have been deferred via requestPanCenter()" + << command; + } else { + qCDebug(lcProtocol).noquote() + << "RadioModel: suppressing profile-load radio-state write" + << command; + } if (cb) { cb(kProfileLoadSuppressedCommandCode, QStringLiteral("suppressed during profile load")); diff --git a/src/models/RadioModel.h b/src/models/RadioModel.h index 4e0a3abb6..c9bbc33c8 100644 --- a/src/models/RadioModel.h +++ b/src/models/RadioModel.h @@ -444,6 +444,39 @@ class RadioModel : public QObject { void setPanBandwidth(double bandwidthMhz); void setPanCenter(double centerMhz); void setPanDbmRange(float minDbm, float maxDbm); + + // #4142 — the ONLY supported way to write a pan center (and, optionally, a + // coupled bandwidth) to the radio. + // + // `display pan set center=…` is classified as a profile-owned radio + // state write, so sendCmd() DROPS it while the profile-load hold is armed: + // it returns before a sequence number is allocated and the command never + // reaches the wire. A user action that lands in that window (typed + // frequency, zoom, drag, ATU sweep, automation) was silently lost, and the + // client kept its optimistic center — leaving the pan permanently claiming + // a center the radio never took. + // + // requestPanCenter() defers instead of dropping: while the hold is armed it + // coalesces the request per pan (last-write-wins) and returns false WITHOUT + // advancing local model state, so the client never claims a center the radio + // does not have. flushPendingProfileLoadPanCenters() replays it once the + // hold lifts. + // + // Pass bandwidthMhz > 0 to set center and bandwidth coherently in one + // command (zoom paths must never split the pair); pass <= 0 to leave the + // radio's bandwidth untouched. + // + // Returns true if the command reached the wire, false if it was deferred. + // Callers that also advance view state optimistically must gate that on the + // return value, or they will re-create the black-waterfall divergence. + bool requestPanCenter(const QString& panId, + double centerMhz, + double bandwidthMhz = -1.0); + + // Replays deferred pan-center writes. Hard invariant: this early-returns + // while the hold is armed, so no deferred center can reach the wire inside + // the hold window. + void flushPendingProfileLoadPanCenters(); void setPanWnb(bool on); void setPanWnbLevel(int level); void setPanRfGain(int gain); @@ -683,6 +716,16 @@ private slots: bool m_wfAutoBlackOn{true}; // mirrors the client auto-black on/off bool m_wfAutoBlackRadioSide{false}; // false = client-side, true = radio-side bool profileLoadRadioStateWritesHeld() const; + // Raw sender for a pan center (+ optional coupled bandwidth). Applies the + // local model update and puts the command on the wire together, so the two + // can never diverge. Everything else must go through requestPanCenter(). + void sendPanCenterToRadio(const QString& panId, + double centerMhz, + double bandwidthMhz); + // Schedules the deferred-pan-center replay for when the hold lifts. Armed by + // the act of deferring, NOT by the profile-load ACK — a large topology can + // stall the radio into a disconnect before it ever ACKs. (#4142) + void armProfileLoadPanCenterFlush(); void registerAsGuiClient(const QString& clientId); void disconnectPendingClientsThen(std::function continuation); // LAN-only: subscribe to radio+client topics early, wait 400 ms for @@ -1054,6 +1097,19 @@ private slots: int m_rttyMarkDefault{2125}; quint32 m_txClientHandle{0}; // handle of the client that owns TX qint64 m_profileLoadRadioStateWriteHoldUntilMs{0}; + + // #4142 — pan-center writes deferred while the profile-load hold is armed. + // bandwidthMhz <= 0 means "center only; leave the radio's bandwidth alone". + struct PendingPanCenter { + double centerMhz{0.0}; + double bandwidthMhz{-1.0}; + }; + // panId → last requested center. Coalesced last-write-wins, but field-wise: + // a later center-only request must NOT erase a bandwidth an earlier zoom + // asked for, or that zoom's bandwidth would be the very thing we set out to + // stop dropping. + QHash m_pendingProfileLoadPanCenters; + QMap m_clientInfoMap; // handle → full client info std::function m_multiFlexContinuation; // saved continuation during conflict pause QMap m_clientStations; // handle → station name (legacy, kept in sync) diff --git a/tests/profile_load_command_test.cpp b/tests/profile_load_command_test.cpp index af444c462..144394e4b 100644 --- a/tests/profile_load_command_test.cpp +++ b/tests/profile_load_command_test.cpp @@ -61,5 +61,65 @@ int main() check(kProfileLoadPostHoldRecoveryDelayMs > kProfileLoadDeferredPanFlushDelayMs, "post-hold recovery runs after deferred pan flush"); + // ── isProfileOwnedRadioStateWrite() — the classification contract (#4142) ── + // + // sendCmd() DROPS every command this returns true for while the profile-load + // hold is armed: it returns before a sequence number is allocated, so the + // command never reaches the wire. These cases pin down exactly which writes + // are lost, and therefore which ones MUST be deferred by + // RadioModel::requestPanCenter() rather than handed to sendCmd() and hoped for. + + // A pan center is profile-owned — this is the write that #4142 lost. Direct + // frequency entry emits an atomic pair; the `slice tune` half survives and the + // `center=` half was silently dropped, so the slice retuned and the pan did not. + check(isProfileOwnedRadioStateWrite( + QStringLiteral("display pan set 0x40000000 center=7.086000")), + "pan center write is profile-owned (the #4142 drop)"); + check(isProfileOwnedRadioStateWrite( + QStringLiteral("display pan set 0x40000000 center=7.086000 bandwidth=0.200000")), + "coupled pan center+bandwidth write is profile-owned (zoom/drag also dropped)"); + check(isProfileOwnedRadioStateWrite( + QStringLiteral("display pan set 0x40000000 bandwidth=0.200000")), + "pan bandwidth write is profile-owned"); + + // xpixels/ypixels are the ONE exemption, and only because the client alone + // knows its pixel geometry — the display is broken until they are sent. + // MainWindow defers and coalesces them (requestPanDimensionsForRadio); this + // exemption is a necessity, NOT evidence that early pan writes are safe. + check(!isProfileOwnedRadioStateWrite( + QStringLiteral("display pan set 0x40000000 xpixels=1920 ypixels=800")), + "pan pixel dimensions are exempt (client-owned display geometry)"); + check(!isProfileOwnedRadioStateWrite( + QStringLiteral("display pan set 0x40000000 xpixels=1920")), + "xpixels alone is exempt"); + + // A single non-pixel field anywhere in the argument list re-arms the guard: + // the exemption is all-or-nothing, so center can never ride in on a dimension + // write's coat-tails. + check(isProfileOwnedRadioStateWrite( + QStringLiteral("display pan set 0x40000000 xpixels=1920 center=7.086000")), + "pixel dimensions mixed with a center write are NOT exempt"); + + // Reads are never suppressed — only writes. + check(!isProfileOwnedRadioStateWrite(QStringLiteral("display pan info 0x40000000")), + "pan info read is not a profile-owned write"); + check(!isProfileOwnedRadioStateWrite(QStringLiteral("sub slice all")), + "subscription is not a profile-owned write"); + + // `slice tune` is NOT profile-owned: it is the half of the typed-frequency + // pair that survived the hold. That asymmetry is the bug — the slice moved + // and the pan could not follow. + check(!isProfileOwnedRadioStateWrite(QStringLiteral("slice tune 0 7.086000 autopan=0")), + "slice tune survives the hold (the surviving half of the #4142 pair)"); + check(isProfileOwnedRadioStateWrite(QStringLiteral("slice set 0 rfgain=20")), + "slice set write is profile-owned"); + + check(isProfileOwnedRadioStateWrite( + QStringLiteral("display panafall set 0x40000000 center=7.086000")), + "panafall write is profile-owned"); + check(isProfileOwnedRadioStateWrite( + QStringLiteral("display waterfall set 0x42000000 color_gain=50")), + "waterfall write is profile-owned"); + return failures == 0 ? 0 : 1; }