From abd638afc5872cc764a5a38a32ad433eeaa6d819 Mon Sep 17 00:00:00 2001 From: W5JWP Date: Sun, 30 Aug 2026 12:41:30 -0500 Subject: [PATCH 1/3] Fix PGXL startup status synchronization Principle XI. --- src/models/AmpModel.cpp | 18 ++++++++---- tests/pgxl_status_state_test.cpp | 48 ++++++++++++++++++++++++++++++++ tests/tests.cmake | 6 ++++ 3 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 tests/pgxl_status_state_test.cpp diff --git a/src/models/AmpModel.cpp b/src/models/AmpModel.cpp index 7214423ca..bf1bb380d 100644 --- a/src/models/AmpModel.cpp +++ b/src/models/AmpModel.cpp @@ -19,6 +19,19 @@ void AmpModel::applyChanges(const AmpDelta& d) // Presence latch: a detected (non-TGXL) power-amp model marks us present. if (d.detectedModel) { m_handle = d.handle; + } + + if (!m_handle.isEmpty() && d.handle == m_handle) { + // Apply state before publishing first presence. The presence signal + // makes the applet visible and reads operate() immediately; publishing + // first used to paint a real operating PGXL as STANDBY during startup. + if (d.operate && m_operate != *d.operate) { + m_operate = *d.operate; + emit stateChanged(); + } + } + + if (d.detectedModel) { if (!m_present) { m_present = true; // Strict parity with the prior applyStatus (m_ip = kvs.value("ip"), @@ -30,11 +43,6 @@ void AmpModel::applyChanges(const AmpDelta& d) } if (!m_handle.isEmpty() && d.handle == m_handle) { - // Operate is change-gated; a status without a "state" leaves it as-is. - if (d.operate && m_operate != *d.operate) { - m_operate = *d.operate; - emit stateChanged(); - } // Forward telemetry (drain current, mains voltage, meffa, temp, …) so // the GUI updates without a direct PGXL TCP connection. emit telemetryUpdated(d.telemetry); diff --git a/tests/pgxl_status_state_test.cpp b/tests/pgxl_status_state_test.cpp new file mode 100644 index 000000000..edba3339b --- /dev/null +++ b/tests/pgxl_status_state_test.cpp @@ -0,0 +1,48 @@ +#include "core/backends/AmpDelta.h" +#include "models/AmpModel.h" + +#include +#include + +#include + +using namespace AetherSDR; + +namespace { +int failures = 0; + +void check(bool condition, const char* message) +{ + if (!condition) { + std::fprintf(stderr, "FAIL: %s\n", message); + ++failures; + } +} +} + +int main(int argc, char** argv) +{ + QCoreApplication app(argc, argv); + + AmpModel model; + bool operateAtPresence = false; + QObject::connect(&model, &AmpModel::presenceChanged, &model, + [&model, &operateAtPresence](bool present) { + if (present) { + operateAtPresence = model.operate(); + } + }); + AmpDelta initial; + initial.handle = QStringLiteral("0x1000"); + initial.detectedModel = QStringLiteral("PowerGeniusXL"); + initial.operate = true; + model.applyChanges(initial); + check(model.present(), "initial amplifier delta establishes presence"); + check(operateAtPresence, "first presence observer sees the decoded operate state"); + + if (failures == 0) { + std::printf("pgxl_status_state_test: all checks passed\n"); + return 0; + } + return 1; +} diff --git a/tests/tests.cmake b/tests/tests.cmake index 4f743661f..96cfbd16e 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -3387,6 +3387,12 @@ target_link_libraries(aetherd_amp_decode_test PRIVATE aethercore Qt6::Core Qt6:: set_target_properties(aetherd_amp_decode_test PROPERTIES AUTOMOC ON) add_test(NAME aetherd_amp_decode_test COMMAND aetherd_amp_decode_test) +add_executable(pgxl_status_state_test tests/pgxl_status_state_test.cpp) +target_include_directories(pgxl_status_state_test PRIVATE src) +target_link_libraries(pgxl_status_state_test PRIVATE aethercore Qt6::Core Qt6::Test) +set_target_properties(pgxl_status_state_test PROPERTIES AUTOMOC ON) +add_test(NAME pgxl_status_state_test COMMAND pgxl_status_state_test) + add_executable(tuner_model_test tests/tuner_model_test.cpp) target_include_directories(tuner_model_test PRIVATE src) target_link_libraries(tuner_model_test PRIVATE aethercore Qt6::Core Qt6::Test) From f8876849a23b8746e99f2956a9b1b173d82f5246 Mon Sep 17 00:00:00 2001 From: W5JWP Date: Sun, 30 Aug 2026 13:38:15 -0500 Subject: [PATCH 2/3] Handle PGXL placeholder identity Principle XI. --- src/models/AmpModel.cpp | 21 +++++++++++--- tests/amp_model_test.cpp | 37 ++++++++++++++++++++++-- tests/pgxl_status_state_test.cpp | 48 -------------------------------- tests/tests.cmake | 6 ---- 4 files changed, 52 insertions(+), 60 deletions(-) delete mode 100644 tests/pgxl_status_state_test.cpp diff --git a/src/models/AmpModel.cpp b/src/models/AmpModel.cpp index bf1bb380d..14303ba48 100644 --- a/src/models/AmpModel.cpp +++ b/src/models/AmpModel.cpp @@ -4,6 +4,7 @@ namespace AetherSDR { void AmpModel::applyChanges(const AmpDelta& d) { + const bool placeholderHandle = d.handle == QLatin1String("0x00000000"); if (d.removed) { // Clear only if it's our amp (matches the original removal semantics — // leaves m_ip/m_operate untouched; consumers gate on present()). @@ -17,17 +18,25 @@ void AmpModel::applyChanges(const AmpDelta& d) } // Presence latch: a detected (non-TGXL) power-amp model marks us present. - if (d.detectedModel) { + if (d.detectedModel && !d.handle.isEmpty() && !placeholderHandle) { + m_handle = d.handle; + } else if (m_present && m_handle.isEmpty() + && !d.handle.isEmpty() && !placeholderHandle) { + // A first status may identify the amp with the placeholder handle. + // Adopt the first real handle so subsequent model-less updates match. m_handle = d.handle; } - if (!m_handle.isEmpty() && d.handle == m_handle) { + const bool appliesToAmp = d.detectedModel.has_value() + || (!m_handle.isEmpty() && d.handle == m_handle); + bool stateDidChange = false; + if (appliesToAmp) { // Apply state before publishing first presence. The presence signal // makes the applet visible and reads operate() immediately; publishing // first used to paint a real operating PGXL as STANDBY during startup. if (d.operate && m_operate != *d.operate) { m_operate = *d.operate; - emit stateChanged(); + stateDidChange = true; } } @@ -42,7 +51,11 @@ void AmpModel::applyChanges(const AmpDelta& d) } } - if (!m_handle.isEmpty() && d.handle == m_handle) { + if (stateDidChange) { + emit stateChanged(); + } + + if (appliesToAmp) { // Forward telemetry (drain current, mains voltage, meffa, temp, …) so // the GUI updates without a direct PGXL TCP connection. emit telemetryUpdated(d.telemetry); diff --git a/tests/amp_model_test.cpp b/tests/amp_model_test.cpp index 7988d58eb..64135b303 100644 --- a/tests/amp_model_test.cpp +++ b/tests/amp_model_test.cpp @@ -51,19 +51,52 @@ int main(int argc, char** argv) { AmpModel amp; QSignalSpy presence(&, &AmpModel::presenceChanged); - amp.applyChanges(detected("0x1000", "PowerGeniusXL", "192.168.1.50", false)); + QSignalSpy state(&, &AmpModel::stateChanged); + bool operateAtPresence = false; + bool stateWasDeferred = false; + QObject::connect(&, &AmpModel::presenceChanged, &, + [&, &state, &operateAtPresence, &stateWasDeferred](bool present) { + if (present) { + operateAtPresence = amp.operate(); + stateWasDeferred = state.count() == 0; + } + }); + amp.applyChanges(detected("0x1000", "PowerGeniusXL", "192.168.1.50", true)); CHECK(amp.present()); CHECK(amp.handle() == "0x1000"); CHECK(amp.ip() == "192.168.1.50"); CHECK(amp.modelName() == "PowerGeniusXL"); - CHECK(!amp.operate()); + CHECK(amp.operate() && operateAtPresence && stateWasDeferred); CHECK(presence.count() == 1 && presence.takeFirst().at(0).toBool() == true); + CHECK(state.count() == 1); // A second detect does not re-latch ip/model or re-emit presence. amp.applyChanges(detected("0x1000", "PowerGeniusXL", "10.0.0.9", true)); CHECK(amp.ip() == "192.168.1.50"); // unchanged CHECK(presence.count() == 0); } + // ---- placeholder handle recovers when the radio publishes the real one ---- + { + AmpModel amp; + bool operateAtPresence = false; + QObject::connect(&, &AmpModel::presenceChanged, &, + [&, &operateAtPresence](bool present) { + if (present) { + operateAtPresence = amp.operate(); + } + }); + amp.applyChanges(detected("0x00000000", "PowerGeniusXL", + "192.168.1.50", true)); + CHECK(amp.present() && amp.handle().isEmpty()); + CHECK(amp.operate() && operateAtPresence); + + QSignalSpy state(&, &AmpModel::stateChanged); + QSignalSpy telemetry(&, &AmpModel::telemetryUpdated); + amp.applyChanges(update("0x1000", false, {{"state", "STANDBY"}})); + CHECK(amp.handle() == "0x1000"); + CHECK(!amp.operate() && state.count() == 1 && telemetry.count() == 1); + } + // ---- a delta with no detectedModel + unknown handle is a no-op (TGXL case) ---- { AmpModel amp; diff --git a/tests/pgxl_status_state_test.cpp b/tests/pgxl_status_state_test.cpp deleted file mode 100644 index edba3339b..000000000 --- a/tests/pgxl_status_state_test.cpp +++ /dev/null @@ -1,48 +0,0 @@ -#include "core/backends/AmpDelta.h" -#include "models/AmpModel.h" - -#include -#include - -#include - -using namespace AetherSDR; - -namespace { -int failures = 0; - -void check(bool condition, const char* message) -{ - if (!condition) { - std::fprintf(stderr, "FAIL: %s\n", message); - ++failures; - } -} -} - -int main(int argc, char** argv) -{ - QCoreApplication app(argc, argv); - - AmpModel model; - bool operateAtPresence = false; - QObject::connect(&model, &AmpModel::presenceChanged, &model, - [&model, &operateAtPresence](bool present) { - if (present) { - operateAtPresence = model.operate(); - } - }); - AmpDelta initial; - initial.handle = QStringLiteral("0x1000"); - initial.detectedModel = QStringLiteral("PowerGeniusXL"); - initial.operate = true; - model.applyChanges(initial); - check(model.present(), "initial amplifier delta establishes presence"); - check(operateAtPresence, "first presence observer sees the decoded operate state"); - - if (failures == 0) { - std::printf("pgxl_status_state_test: all checks passed\n"); - return 0; - } - return 1; -} diff --git a/tests/tests.cmake b/tests/tests.cmake index 96cfbd16e..4f743661f 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -3387,12 +3387,6 @@ target_link_libraries(aetherd_amp_decode_test PRIVATE aethercore Qt6::Core Qt6:: set_target_properties(aetherd_amp_decode_test PROPERTIES AUTOMOC ON) add_test(NAME aetherd_amp_decode_test COMMAND aetherd_amp_decode_test) -add_executable(pgxl_status_state_test tests/pgxl_status_state_test.cpp) -target_include_directories(pgxl_status_state_test PRIVATE src) -target_link_libraries(pgxl_status_state_test PRIVATE aethercore Qt6::Core Qt6::Test) -set_target_properties(pgxl_status_state_test PROPERTIES AUTOMOC ON) -add_test(NAME pgxl_status_state_test COMMAND pgxl_status_state_test) - add_executable(tuner_model_test tests/tuner_model_test.cpp) target_include_directories(tuner_model_test PRIVATE src) target_link_libraries(tuner_model_test PRIVATE aethercore Qt6::Core Qt6::Test) From 40f2e1a891c20cdccebdceb80203796258f79329 Mon Sep 17 00:00:00 2001 From: W5JWP Date: Tue, 1 Sep 2026 16:30:08 -0500 Subject: [PATCH 3/3] Prevent PGXL from adopting tuner handles Principle VIII. --- src/core/backends/flex/FlexBackend.cpp | 17 +++++++++++------ src/models/AmpModel.cpp | 8 +------- tests/aetherd_amp_decode_test.cpp | 14 ++++++++++++++ tests/amp_model_test.cpp | 14 +++++++++++--- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/core/backends/flex/FlexBackend.cpp b/src/core/backends/flex/FlexBackend.cpp index 0bbd7a659..06431cc5c 100644 --- a/src/core/backends/flex/FlexBackend.cpp +++ b/src/core/backends/flex/FlexBackend.cpp @@ -938,10 +938,11 @@ void FlexBackend::decodeAtuStatus(const QMap& kvs) void FlexBackend::decodeAmplifierStatus(const QString& handle, const QString& model, const QMap& kvs, bool removed) { - // Stateless translation of the SmartSDR "amplifier …" wire → AmpDelta - // (#4094). The presence latch, operate change-gating, and handle matching are - // the model's job (AmpModel::applyChanges) — this only reports what the wire - // said. Command/encode is the reverse path — invokeExtension("flex", + // Translation of the SmartSDR "amplifier …" wire → AmpDelta + // (#4094). Placeholder handles are normalized here so the vendor-neutral + // model never needs SmartSDR sentinel knowledge. The presence latch, operate + // change-gating, and handle matching are the model's job + // (AmpModel::applyChanges). Command/encode is the reverse path — invokeExtension("flex", // "amp.operate", …) below translates AmpModel's neutral intent (#4094). AmpDelta d; d.handle = handle; @@ -955,14 +956,18 @@ void FlexBackend::decodeAmplifierStatus(const QString& handle, const QString& mo emit amplifierChanged(d); return; } + if (handle == QLatin1String("0x00000000")) { + d.handle.clear(); + } // RadioModel routes only power amps (PGXL) into this decode, so the handle is // the amp's — cache it for the encode path (#4198). Ignore the placeholder // handle a first status can carry before the real one is assigned. Defense in // depth (#4203): a pre-existing routing edge — a model-less TGXL status arriving // before its handle is known — can fall through to here; refuse to cache a // known-tuner handle so a later amp.operate can never mis-target the TGXL. - if (!handle.isEmpty() && handle != QLatin1String("0x00000000") && handle != m_tunerHandle) - m_ampHandle = handle; + if (!d.handle.isEmpty() && d.handle != m_tunerHandle) { + m_ampHandle = d.handle; + } // A non-empty, non-TGXL model marks a power amp (PGXL); the TunerGeniusXL is // the tuner and routes to TunerModel, not here. if (!model.isEmpty() && model != QLatin1String("TunerGeniusXL")) { diff --git a/src/models/AmpModel.cpp b/src/models/AmpModel.cpp index 14303ba48..1ad8e04e4 100644 --- a/src/models/AmpModel.cpp +++ b/src/models/AmpModel.cpp @@ -4,7 +4,6 @@ namespace AetherSDR { void AmpModel::applyChanges(const AmpDelta& d) { - const bool placeholderHandle = d.handle == QLatin1String("0x00000000"); if (d.removed) { // Clear only if it's our amp (matches the original removal semantics — // leaves m_ip/m_operate untouched; consumers gate on present()). @@ -18,12 +17,7 @@ void AmpModel::applyChanges(const AmpDelta& d) } // Presence latch: a detected (non-TGXL) power-amp model marks us present. - if (d.detectedModel && !d.handle.isEmpty() && !placeholderHandle) { - m_handle = d.handle; - } else if (m_present && m_handle.isEmpty() - && !d.handle.isEmpty() && !placeholderHandle) { - // A first status may identify the amp with the placeholder handle. - // Adopt the first real handle so subsequent model-less updates match. + if (d.detectedModel && !d.handle.isEmpty()) { m_handle = d.handle; } diff --git a/tests/aetherd_amp_decode_test.cpp b/tests/aetherd_amp_decode_test.cpp index 33e81cf64..3eec3c072 100644 --- a/tests/aetherd_amp_decode_test.cpp +++ b/tests/aetherd_amp_decode_test.cpp @@ -75,6 +75,20 @@ int main(int argc, char** argv) CHECK(!d.detectedModel.has_value() && !d.operate.has_value()); } + // ---- SmartSDR's startup placeholder is hidden from the neutral model ---- + { + const AmpDelta d = decode(b, "0x00000000", "PowerGeniusXL", + {{"state", "OPERATE"}}, false); + CHECK(d.handle.isEmpty()); + CHECK(d.detectedModel.has_value() && *d.detectedModel == "PowerGeniusXL"); + CHECK(d.operate.has_value() && *d.operate == true); + + // Removal keeps the wire handle so cache/removal matching retains its + // existing semantics; only live status is normalized. + const AmpDelta removed = decode(b, "0x00000000", QString(), {}, true); + CHECK(removed.removed && removed.handle == "0x00000000"); + } + // ---- #4203: a known-tuner handle mis-routed into the amp decode must NOT be // cached as m_ampHandle. Observe via the encode path: with no amp handle // cached, amp.operate fails closed (extensionError), never targeting the TGXL. diff --git a/tests/amp_model_test.cpp b/tests/amp_model_test.cpp index 64135b303..67f6a35de 100644 --- a/tests/amp_model_test.cpp +++ b/tests/amp_model_test.cpp @@ -75,7 +75,7 @@ int main(int argc, char** argv) CHECK(presence.count() == 0); } - // ---- placeholder handle recovers when the radio publishes the real one ---- + // ---- unidentified detection cannot adopt a model-less TGXL handle ---- { AmpModel amp; bool operateAtPresence = false; @@ -85,14 +85,22 @@ int main(int argc, char** argv) operateAtPresence = amp.operate(); } }); - amp.applyChanges(detected("0x00000000", "PowerGeniusXL", + // FlexBackend normalizes the SmartSDR placeholder to an empty handle. + amp.applyChanges(detected(QString(), "PowerGeniusXL", "192.168.1.50", true)); CHECK(amp.present() && amp.handle().isEmpty()); CHECK(amp.operate() && operateAtPresence); QSignalSpy state(&, &AmpModel::stateChanged); QSignalSpy telemetry(&, &AmpModel::telemetryUpdated); - amp.applyChanges(update("0x1000", false, {{"state", "STANDBY"}})); + amp.applyChanges(update("0x2000", false, {{"state", "STANDBY"}})); + CHECK(amp.handle().isEmpty()); + CHECK(amp.operate() && state.count() == 0 && telemetry.count() == 0); + + // A later model-bearing PGXL status safely establishes identity and + // applies its state; model-less updates can only match after that. + amp.applyChanges(detected("0x1000", "PowerGeniusXL", QString(), false, + {{"state", "STANDBY"}})); CHECK(amp.handle() == "0x1000"); CHECK(!amp.operate() && state.count() == 1 && telemetry.count() == 1); }