Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1834,6 +1834,14 @@ target_link_libraries(radio_status_ownership_test PRIVATE Qt6::Core)
enable_testing()
add_test(NAME radio_status_ownership_test COMMAND radio_status_ownership_test)

# #3212 — slice recreate policy (reuse restored pan vs create new; slice freq)
add_executable(slice_recreate_policy_test
tests/slice_recreate_policy_test.cpp
)
target_include_directories(slice_recreate_policy_test PRIVATE src)
target_link_libraries(slice_recreate_policy_test PRIVATE Qt6::Core)
add_test(NAME slice_recreate_policy_test COMMAND slice_recreate_policy_test)

add_executable(shortcut_manager_test
tests/shortcut_manager_test.cpp
src/core/ShortcutManager.cpp
Expand Down
59 changes: 37 additions & 22 deletions src/models/RadioModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "core/StreamStatus.h"
#include "core/UdpRegistrationPolicy.h"
#include "RadioStatusOwnership.h"
#include "SliceRecreatePolicy.h"
#include <QCoreApplication>
#include <QDebug>
#include <QJsonArray>
Expand Down Expand Up @@ -2186,18 +2187,9 @@ void RadioModel::registerAsGuiClient(const QString& clientId)
qCDebug(lcProtocol) << "RadioModel: slice list ->" << (ids.isEmpty() ? "(empty)" : body);

if (ids.isEmpty()) {
// Radio has no slices at all — create one
qCDebug(lcProtocol) << "RadioModel: no slices on radio, creating default";
auto& settings = AppSettings::instance();
double lastFreq = settings.value("LastFrequency", "0").toDouble();
QString lastMode = settings.value("LastMode", "").toString();
if (lastFreq > 0.0) {
createDefaultSlice(
QString::number(lastFreq, 'f', 6),
lastMode.isEmpty() ? "USB" : lastMode);
} else {
createDefaultSlice();
}
// Radio reports no slices. Reuse an already-restored pan if we
// have one; only create a fresh panafall otherwise (#3212).
ensureDefaultSlicePreferringRestoredPan();
} else if (m_slices.isEmpty()) {
// Radio has slices but we haven't matched any to our
// client_handle yet (status messages still in flight).
Expand All @@ -2208,16 +2200,7 @@ void RadioModel::registerAsGuiClient(const QString& clientId)
QTimer::singleShot(500, this, [this]() {
if (m_slices.isEmpty() && isConnected()) {
qCDebug(lcProtocol) << "RadioModel: deferred check — still no owned slices, creating default";
auto& settings = AppSettings::instance();
double lastFreq = settings.value("LastFrequency", "0").toDouble();
QString lastMode = settings.value("LastMode", "").toString();
if (lastFreq > 0.0) {
createDefaultSlice(
QString::number(lastFreq, 'f', 6),
lastMode.isEmpty() ? "USB" : lastMode);
} else {
createDefaultSlice();
}
ensureDefaultSlicePreferringRestoredPan();
} else if (!m_slices.isEmpty()) {
qCDebug(lcProtocol) << "RadioModel: deferred check — adopted"
<< m_slices.size() << "existing slice(s)";
Expand Down Expand Up @@ -4939,6 +4922,38 @@ void RadioModel::configureWaterfall()
});
}

void RadioModel::ensureDefaultSlicePreferringRestoredPan()
{
auto& settings = AppSettings::instance();

SliceRecreatePolicy::Inputs in;
in.lastFreqMhz = settings.value("LastFrequency", "0").toDouble();
in.lastMode = settings.value("LastMode", "").toString();

// If m_activePanId names a pan we already hold, the radio restored it for us
// (claimed well before the "slice list" query resolved — see #3212). Feed its
// center to the policy so the recreated slice lands inside the visible span.
PanadapterModel* restored = (!m_activePanId.isEmpty())
? m_panadapters.value(m_activePanId, nullptr)
: nullptr;
if (restored) {
in.hasRestoredPan = true;
in.restoredPanCenterMhz = restored->centerMhz();
}

const SliceRecreatePolicy::Decision d = SliceRecreatePolicy::decide(in);
const QString freqStr = QString::number(d.freqMhz, 'f', 6);

if (d.action == SliceRecreatePolicy::Action::ReuseRestoredPan) {
qCDebug(lcProtocol) << "RadioModel: no slices but pan" << m_activePanId
<< "already restored — creating slice on it at" << freqStr << d.mode;
createDefaultSliceOnPan(m_activePanId, freqStr, d.mode, d.antenna);
} else {
qCDebug(lcProtocol) << "RadioModel: no slices and no restored pan — creating default panafall + slice";
createDefaultSlice(freqStr, d.mode, d.antenna);
}
}

// Standalone mode: create panadapter + slice.
// FlexLib v4.2.18 uses "display panafall create x=100 y=100"; keep the
// legacy "panadapter create" as a fallback for older firmware.
Expand Down
6 changes: 6 additions & 0 deletions src/models/RadioModel.h
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,12 @@ private slots:
const QString& freqMhz,
const QString& mode,
const QString& antenna);
// Reconnect recovery (#3212): the radio's GUIClientID session restore can
// bring back our panadapter without a slice, so "slice list" returns empty.
// Reuse the already-claimed pan instead of allocating a second one; only
// fall back to createDefaultSlice() (which issues "display panafall create")
// when no owned pan exists yet.
void ensureDefaultSlicePreferringRestoredPan();

RadioConnection* m_connection{nullptr};
QThread* m_connThread{nullptr};
Expand Down
91 changes: 91 additions & 0 deletions src/models/SliceRecreatePolicy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#pragma once

#include <QString>

// SliceRecreatePolicy — decides how to recover a default slice when the radio
// reports zero slices at GUI-attach time ("slice list" returns empty).
//
// Background (#3212): when AetherSDR reconnects to a radio that remembers our
// persistent client_id, the radio's GUIClientID session restore can bring back
// our panadapter WITHOUT its slice (observed after an unexpected disconnect +
// auto-reconnect). The pan is "claimed" well before the "slice list" query
// resolves, so by decision time we already hold the restored pan. The old code
// unconditionally issued "display panafall create", allocating a SECOND, empty
// panadapter on top of the restored one — the duplicate-PAN bug in #3212.
//
// The decision is pulled out of RadioModel into this pure, header-only function
// so it can be unit-tested without a live radio connection (mirrors the
// RadioStatusOwnership pattern). RadioModel feeds it the runtime state and acts
// on the returned Decision.

namespace AetherSDR::SliceRecreatePolicy {

enum class Action {
// The radio already restored one of our panadapters; attach the slice to it
// instead of creating a new pan. Prevents the #3212 duplicate panadapter.
ReuseRestoredPan,
// No owned panadapter exists yet (true first-connect / standalone): create a
// fresh panafall and then a slice on it.
CreateNewPan,
};

// Runtime state captured by RadioModel at "slice list -> (empty)" time.
struct Inputs {
// True when m_activePanId names a panadapter we already hold in
// m_panadapters (i.e. the radio restored it for us).
bool hasRestoredPan{false};
// The restored pan's center frequency in MHz, as last reported by the radio
// ("display pan ... center="). <= 0 means the radio has not reported a
// center yet (pan only just claimed).
double restoredPanCenterMhz{0.0};
// Client-persisted last frequency (AppSettings "LastFrequency"). <= 0 means
// unset. Used only as a fallback — see decide().
double lastFreqMhz{0.0};
// Client-persisted last mode (AppSettings "LastMode"). Empty means unset.
QString lastMode;
};

struct Decision {
Action action{Action::CreateNewPan};
double freqMhz{14.225000};
QString mode{QStringLiteral("USB")};
QString antenna{QStringLiteral("ANT1")};
};

// Pure decision. No I/O, no Qt event loop — safe to unit-test.
//
// Frequency choice rationale:
// * Reuse path: place the slice at the RESTORED PAN'S OWN center, not at
// LastFrequency. The radio's restored pan center is authoritative for where
// that pan is displayed right now; LastFrequency is a client-side guess that
// can be on a different band entirely (in the #3212 bundle the restored pan
// was on 20m / 14.282 MHz while LastFrequency was 28.305 MHz / 10m — using
// LastFrequency would drop the slice ~14 MHz outside the visible span).
// LastFrequency / 14.225 are fallbacks only for the brief window before the
// radio has reported the pan's center.
// * Create path: no pan exists to anchor to, so LastFrequency (or the 14.225
// default) is the best available starting point.
inline Decision decide(const Inputs& in)
{
Decision d;
d.mode = in.lastMode.isEmpty() ? QStringLiteral("USB") : in.lastMode;
d.antenna = QStringLiteral("ANT1");

if (in.hasRestoredPan) {
d.action = Action::ReuseRestoredPan;
if (in.restoredPanCenterMhz > 0.0) {
d.freqMhz = in.restoredPanCenterMhz;
} else if (in.lastFreqMhz > 0.0) {
d.freqMhz = in.lastFreqMhz;
} else {
d.freqMhz = 14.225000;
}
return d;
}

d.action = Action::CreateNewPan;
d.freqMhz = in.lastFreqMhz > 0.0 ? in.lastFreqMhz : 14.225000;
return d;
}

} // namespace AetherSDR::SliceRecreatePolicy
165 changes: 165 additions & 0 deletions tests/slice_recreate_policy_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
// Regression tests for SliceRecreatePolicy (#3212).
//
// The #3212 bug: on reconnect the radio's GUIClientID session restore brought
// back our panadapter without a slice, "slice list" returned empty, and
// RadioModel unconditionally issued "display panafall create" — leaving a
// second, slice-less panadapter. These tests pin the decision so the duplicate
// PAN cannot silently come back, and lock in that the recreated slice is placed
// at the restored pan's center (not at a possibly-wrong-band LastFrequency).

#include "models/SliceRecreatePolicy.h"

#include <cstdio>
#include <cmath>

using namespace AetherSDR;
using namespace AetherSDR::SliceRecreatePolicy;

namespace {

int failures = 0;

void check(bool condition, const char* name)
{
if (condition) {
std::printf("[PASS] %s\n", name);
} else {
std::printf("[FAIL] %s\n", name);
++failures;
}
}

bool freqEq(double a, double b)
{
return std::fabs(a - b) < 1e-9;
}

// The exact scenario captured in the issue's support bundle:
// - radio restored pan 0x40000000 centered on 20m (14.282408 MHz)
// - "slice list" returned empty (slice was NOT restored)
// - AppSettings LastFrequency was 28.305 MHz (10m) — a DIFFERENT band
// The fix must reuse the restored pan (no second pan) AND place the slice at the
// pan's center, never at the stale 10m LastFrequency.
void testIssue3212BundleScenario()
{
Inputs in;
in.hasRestoredPan = true;
in.restoredPanCenterMhz = 14.282408;
in.lastFreqMhz = 28.305000; // stale, on a different band
in.lastMode = QStringLiteral("USB");

const Decision d = decide(in);

check(d.action == Action::ReuseRestoredPan,
"3212: restored pan is reused, NOT a second panafall created");
check(freqEq(d.freqMhz, 14.282408),
"3212: slice is placed at the restored pan center (14.282408)");
check(!freqEq(d.freqMhz, 28.305000),
"3212: slice is NOT placed at the stale 10m LastFrequency");
check(d.mode == QStringLiteral("USB"), "3212: mode preserved from LastMode");
check(d.antenna == QStringLiteral("ANT1"), "3212: antenna defaults to ANT1");
}

// First-ever connect / true standalone: no pan exists. We must create a new pan
// and seed it from LastFrequency (or the 14.225 default).
void testNoRestoredPanCreatesNew()
{
Inputs in;
in.hasRestoredPan = false;
in.lastFreqMhz = 7.150000;
in.lastMode = QStringLiteral("LSB");

const Decision d = decide(in);

check(d.action == Action::CreateNewPan,
"no pan: a fresh panadapter is created");
check(freqEq(d.freqMhz, 7.150000),
"no pan: new slice seeded from LastFrequency");
check(d.mode == QStringLiteral("LSB"), "no pan: mode from LastMode");
}

void testNoRestoredPanNoSettingsUsesDefaults()
{
Inputs in;
in.hasRestoredPan = false;
in.lastFreqMhz = 0.0; // unset
in.lastMode = QString(); // unset

const Decision d = decide(in);

check(d.action == Action::CreateNewPan, "cold start: creates a pan");
check(freqEq(d.freqMhz, 14.225000), "cold start: defaults to 14.225 MHz");
check(d.mode == QStringLiteral("USB"), "cold start: defaults to USB");
check(d.antenna == QStringLiteral("ANT1"), "cold start: defaults to ANT1");
}

// Edge: pan restored but its center hasn't been reported by the radio yet
// (claimed this instant, "display pan ... center=" still in flight). We still
// reuse the pan — never create a second one — and fall back to LastFrequency.
void testRestoredPanCenterNotYetKnownFallsBackToLastFreq()
{
Inputs in;
in.hasRestoredPan = true;
in.restoredPanCenterMhz = 0.0; // not reported yet
in.lastFreqMhz = 21.300000;
in.lastMode = QStringLiteral("USB");

const Decision d = decide(in);

check(d.action == Action::ReuseRestoredPan,
"center unknown: still reuse the pan (no duplicate)");
check(freqEq(d.freqMhz, 21.300000),
"center unknown: fall back to LastFrequency");
}

void testRestoredPanCenterUnknownNoSettingsFallsBackToDefault()
{
Inputs in;
in.hasRestoredPan = true;
in.restoredPanCenterMhz = 0.0;
in.lastFreqMhz = 0.0;
in.lastMode = QString();

const Decision d = decide(in);

check(d.action == Action::ReuseRestoredPan,
"center+settings unknown: still reuse the pan");
check(freqEq(d.freqMhz, 14.225000),
"center+settings unknown: fall back to 14.225 default");
check(d.mode == QStringLiteral("USB"), "center unknown: default mode USB");
}

// The restored-pan center always wins over LastFrequency, even when both are on
// the same band — the pan center is radio-authoritative for what's on screen.
void testRestoredPanCenterWinsOverLastFrequency()
{
Inputs in;
in.hasRestoredPan = true;
in.restoredPanCenterMhz = 14.250000;
in.lastFreqMhz = 14.074000; // same band, different freq
in.lastMode = QStringLiteral("USB");

const Decision d = decide(in);

check(freqEq(d.freqMhz, 14.250000),
"pan center wins over LastFrequency even on the same band");
}

} // namespace

int main()
{
testIssue3212BundleScenario();
testNoRestoredPanCreatesNew();
testNoRestoredPanNoSettingsUsesDefaults();
testRestoredPanCenterNotYetKnownFallsBackToLastFreq();
testRestoredPanCenterUnknownNoSettingsFallsBackToDefault();
testRestoredPanCenterWinsOverLastFrequency();

if (failures == 0) {
std::printf("\nAll slice_recreate_policy tests passed.\n");
return 0;
}
std::printf("\n%d slice_recreate_policy test(s) FAILED.\n", failures);
return 1;
}
Loading