From 4a24bc6dc34d5526b2f5a6fa04d93798aa3e6373 Mon Sep 17 00:00:00 2001 From: jensenpat Date: Sat, 29 Aug 2026 22:31:34 -0700 Subject: [PATCH 1/5] Restore durable Icom memory ownership (Principle XI) --- .../aetherd-icom-civ-backend-design.md | 20 +++++--- docs/architecture/radio-capabilities-map.md | 4 +- src/core/LocalMemoryBank.cpp | 13 +++++ src/core/LocalMemoryBank.h | 4 ++ src/core/LocalMemoryStore.cpp | 20 ++++++++ src/core/LocalMemoryStore.h | 12 ++--- src/core/backends/MemoryDelta.h | 2 + src/core/backends/RadioCapabilities.h | 10 ++-- src/core/backends/icom/IcomCivBackend.cpp | 24 ++++++++- src/core/backends/icom/IcomCivBackend.h | 1 + src/gui/MemoryDialog.cpp | 11 ++++ src/models/MemoryEntry.h | 5 ++ src/models/RadioModel.cpp | 51 +++++++++++++++++-- src/models/RadioModel.h | 22 ++++---- tests/local_memory_bank_test.cpp | 36 +++++++++++++ tests/local_memory_store_test.cpp | 10 ++++ tests/radio_capability_gating_test.cpp | 9 ++++ 17 files changed, 215 insertions(+), 39 deletions(-) diff --git a/docs/architecture/aetherd-icom-civ-backend-design.md b/docs/architecture/aetherd-icom-civ-backend-design.md index e7d03143d..a2f64ecfa 100644 --- a/docs/architecture/aetherd-icom-civ-backend-design.md +++ b/docs/architecture/aetherd-icom-civ-backend-design.md @@ -680,14 +680,18 @@ captures from our own radio. ## 9. Explicitly out of scope for phase 1 - **IQ.** It does not exist on this radio. Not deferred — absent. -- **Writable memory channels.** Initial IC-705, IC-7300MK2, and IC-9700 support - reads their model-specific ordinary-channel records with `1A 00`, exposes occupied - channels through the shared memory model, and permits tuning to the cached - channel state. Reads are button-only; IC-705 requires a selected group so a - click queues 100 requests rather than scanning its 10,000-address space. - Writing, adding, deleting, scan-edge, call, and satellite - memories remain deferred. Other Icom models continue to use the client-side - bank until their own published record layouts are implemented and verified. +- **Writing radio memory channels.** All Icom radios use AetherSDR's shared, + writable memory database as the working model. For IC-705, IC-7300MK2, and + IC-9700, **Sync Memories** reads the model-specific ordinary-channel records + with `1A 00` and ingests occupied channels into that database; Tune then + recalls the durable database row like a manual or CSV-imported memory. + Repeated syncs update rows from the same radio/channel rather than duplicating + them. Reads are button-only; IC-705 requires a selected group so a click + queues 100 requests rather than scanning its 10,000-address space. Writing or + deleting the radio's own channels, plus scan-edge, call, and satellite + memories, remain deferred. Other Icom models still use the same client-side + database, but expose no Sync action until their published record layout is + implemented and verified. - **D-STAR / DV.** A large command surface (`22 xx`, `23 xx`) and a separate feature. - **Bluetooth transport.** Unknown whether it carries all three streams. diff --git a/docs/architecture/radio-capabilities-map.md b/docs/architecture/radio-capabilities-map.md index 2ca2b6bc6..94aa08f40 100644 --- a/docs/architecture/radio-capabilities-map.md +++ b/docs/architecture/radio-capabilities-map.md @@ -93,8 +93,8 @@ traps and why the DAX crash guard is deliberately *not* the DAX capability. | `hasPaCurrentTelemetry` | ❌ | ❌ | ❌ | `MainWindow::applyCapabilitiesToUi` | Calibrated PA drain-current face in Radio Vitals, used only when PA-temperature telemetry is unavailable. Icom: ✅ only for the IC-9700 profile's documented 0–20 A Id calibration. Flex remains ❌ because its PACURRENT meter is known to clip below real full-power draw | | `hasMainFanTelemetry` | ✅ | ❌ | ❌ | `MainWindow::applyCapabilitiesToUi` | Main Fan gauge in Radio Vitals. All current Icom models are ❌ because the backend does not publish fan-speed telemetry | | `hostFrequencyCalibration` | ❌ | ✅ | ❌ | `RadioSetupDialog` (Calibration page), `AutomationServer::doFreqCal` | Shows the Calibration page and enables the `freqcal` bridge verb. Means "**the client** owns the frequency-error correction", not "this radio has an error" — every radio does. Flex is ❌ because it calibrates itself (`radio set cal_freq` / `pll_start`), and that surface stays in the Frequency Offset group on the Receive page. HL2 is ✅ because its 76.8 MHz NCO scale is a `localparam` in the bitstream (`radio.v` M2) and no register in the HPSDR map accepts a correction — see `docs/architecture/hl2-frequency-calibration.md` | -| `persistsMemories` | ✅ | ❌ | ❌ | `LocalMemoryBank` engagement (#4590) | host-side memory bank vs radio-side slots — the bank's ONE shared document lives at `radio_settings (local, '', MemoryBank)` since RFC #4603 PR 6, covered by settings backup/export; legacy `memories.json` is a frozen import source. Icom is profile-gated for IC-705, IC-7300MK2, and IC-9700; unprofiled Icom models remain ❌. | -| `canWriteMemories` | ✅ | ❌ | ❌ | `RadioModel::memoriesWritable`, memory dialog and panadapter memory panel | Separates radio ownership from mutation support. The initial Icom implementation is read-only, so Add, Import, inline edits, and Remove cannot leak vendor-incompatible memory commands. | +| `persistsMemories` | ✅ | ❌ | ❌ | `LocalMemoryBank` engagement (#4590) | selects the active working store: native radio slots or the host database. The bank's ONE shared document lives at `radio_settings (local, '', MemoryBank)` since RFC #4603 PR 6, covered by settings backup/export; legacy `memories.json` is a frozen import source. Icom is always ❌ because its working model is the host database; model-specific Sync support is declared independently by `canRefreshMemories`. | +| `canWriteMemories` | ✅ | ❌ | ❌ | `RadioModel::memoriesWritable`, memory dialog and panadapter memory panel | Separates native ownership from mutation support. Icom's radio-side store stays read-only, while the shared AetherSDR database remains writable for Add, Import, inline edits, Remove, and Tune on every Icom model. | | `canApplyMemories` | ✅ | ❌ | ❌ | `RadioModel::tryMemoryCommand` | True means the backend accepts its native memory-apply command. Initial Icom support is ❌ and applies recallable cached fields through the existing neutral slice setters instead of entering vendor Memory mode; split/RPS/DV/DD records are display-only. | | `canRefreshMemories` | ❌ | ❌ | ❌ | Memory Channels dialog → `RadioModel::refreshMemories` | Explicit, button-only radio-memory snapshots. IC-7300MK2 reads 99 channels; IC-9700 reads all 297 or one selected band; IC-705 requires one selected group and reads only its 100 channels. No memory scan runs during connection. | | `clientSettingsDomains` | empty | Tuning\|Passband\|SpanRate\|RfGain\|TxSetpoints\|Memories\|Agc | empty | `RadioStateMemory::shouldEngage` → `RadioModel::handRestoredStateToBackend` | connect-time operating-state restore + debounced capture (RFC #4603 PR 3): `Hl2Backend::applyRestoredState` seeds rate/freq/LNA at connect, `pushInitialState` applies restored mode+passband (reconciled with #4484 — restored as a pair, so mode and passband cannot disagree) and the start band's drive; per-band LNA/drive maps ride the extension document and follow TX-slice band changes. `Agc` (#4909) carries the mode + threshold pair as typed universal fields — FLAT, not per-band, and seeded onto EVERY receiver by `Hl2Backend::seedReceiverAgc()`, because the AGC runs in host-side WDSP and no HPSDR register can be asked what it is. Seeding runs from `connectRadio` when the connect SERIAL changes or the receivers were rebuilt from nothing — never on a plain auto-reconnect, because `handRestoredStateToBackend` re-hands the document before every connect and `buildReceivers` preserves live receiver state, so an unconditional seed flattened per-receiver AGC on each dropped link. Memories is declarative only — the bank engages on `persistsMemories` and keeps its own shared document (PR 6). Flex/Sim: no-op by empty declaration. | diff --git a/src/core/LocalMemoryBank.cpp b/src/core/LocalMemoryBank.cpp index dc7bc9356..e948f1dc4 100644 --- a/src/core/LocalMemoryBank.cpp +++ b/src/core/LocalMemoryBank.cpp @@ -148,6 +148,19 @@ int LocalMemoryBank::allocateSlot() const return index; } +int LocalMemoryBank::importedSlot(const QString& source, const QString& key) const +{ + if (source.isEmpty() || key.isEmpty()) { + return -1; + } + for (auto it = m_entries.constBegin(); it != m_entries.constEnd(); ++it) { + if (it->importSource == source && it->importKey == key) { + return it.key(); + } + } + return -1; +} + LocalMemoryBank::CommandResult LocalMemoryBank::handleCommand(const QString& command) { CommandResult result; diff --git a/src/core/LocalMemoryBank.h b/src/core/LocalMemoryBank.h index 3127da412..d572dcf27 100644 --- a/src/core/LocalMemoryBank.h +++ b/src/core/LocalMemoryBank.h @@ -79,6 +79,10 @@ class LocalMemoryBank : public QObject { bool isWritable() const { return m_writable; } const QMap& entries() const { return m_entries; } + // Locate a row previously ingested from the same external source. The pair + // is deliberately independent of the client slot number: native radio + // channel numbers and CSV row numbers may collide with manual memories. + int importedSlot(const QString& source, const QString& key) const; // Handle one `memory …` command. Returns handled=false for anything outside // the four verbs above. diff --git a/src/core/LocalMemoryStore.cpp b/src/core/LocalMemoryStore.cpp index 5d5e2387c..3c25bb83a 100644 --- a/src/core/LocalMemoryStore.cpp +++ b/src/core/LocalMemoryStore.cpp @@ -23,14 +23,24 @@ QJsonObject entryToJson(const MemoryEntry& m) o["index"] = m.index; o["group"] = m.group; o["owner"] = m.owner; + o["channel"] = m.channel; + o["importSource"] = m.importSource; + o["importKey"] = m.importKey; o["freq"] = m.freq; o["name"] = m.name; o["mode"] = m.mode; + o["nativeFilter"] = m.nativeFilter; + o["dataMode"] = m.dataMode; o["step"] = m.step; o["offsetDir"] = m.offsetDir; o["repeaterOffset"] = m.repeaterOffset; o["toneMode"] = m.toneMode; o["toneValue"] = m.toneValue; + o["rxToneValue"] = m.rxToneValue; + o["dtcsCode"] = m.dtcsCode; + o["dtcsTxReverse"] = m.dtcsTxReverse; + o["dtcsRxReverse"] = m.dtcsRxReverse; + o["recallable"] = m.recallable; o["squelch"] = m.squelch; o["squelchLevel"] = m.squelchLevel; o["rxFilterLow"] = m.rxFilterLow; @@ -48,14 +58,24 @@ MemoryEntry entryFromJson(const QJsonObject& o) m.index = o.value("index").toInt(m.index); m.group = o.value("group").toString(m.group); m.owner = o.value("owner").toString(m.owner); + m.channel = o.value("channel").toString(m.channel); + m.importSource = o.value("importSource").toString(m.importSource); + m.importKey = o.value("importKey").toString(m.importKey); m.freq = o.value("freq").toDouble(m.freq); m.name = o.value("name").toString(m.name); m.mode = o.value("mode").toString(m.mode); + m.nativeFilter = o.value("nativeFilter").toInt(m.nativeFilter); + m.dataMode = o.value("dataMode").toInt(m.dataMode); m.step = o.value("step").toInt(m.step); m.offsetDir = o.value("offsetDir").toString(m.offsetDir); m.repeaterOffset = o.value("repeaterOffset").toDouble(m.repeaterOffset); m.toneMode = o.value("toneMode").toString(m.toneMode); m.toneValue = o.value("toneValue").toDouble(m.toneValue); + m.rxToneValue = o.value("rxToneValue").toDouble(m.rxToneValue); + m.dtcsCode = o.value("dtcsCode").toInt(m.dtcsCode); + m.dtcsTxReverse = o.value("dtcsTxReverse").toBool(m.dtcsTxReverse); + m.dtcsRxReverse = o.value("dtcsRxReverse").toBool(m.dtcsRxReverse); + m.recallable = o.value("recallable").toBool(m.recallable); m.squelch = o.value("squelch").toBool(m.squelch); m.squelchLevel = o.value("squelchLevel").toInt(m.squelchLevel); m.rxFilterLow = o.value("rxFilterLow").toInt(m.rxFilterLow); diff --git a/src/core/LocalMemoryStore.h b/src/core/LocalMemoryStore.h index 0b62b3795..3dce5f77c 100644 --- a/src/core/LocalMemoryStore.h +++ b/src/core/LocalMemoryStore.h @@ -9,15 +9,15 @@ namespace AetherSDR { -// Portable, versioned JSON persistence for the CLIENT-side memory bank — the -// channels an operator saves on a radio that has no memory storage of its own -// (Hermes-Lite 2, Kiwi, the demo backend). On a Flex the radio owns the slots -// and this file is never touched; see RadioCapabilities::persistsMemories. +// Portable, versioned JSON persistence for the CLIENT-side memory bank — both +// channels the operator creates here and snapshots explicitly imported from a +// radio. On a Flex the radio owns and mutates the active slots, so this document +// is not the session store; see RadioCapabilities::persistsMemories. // // Envelope: // { // "format": "aether.memories", -// "version": 1, +// "version": 2, // "savedAt": "2026-07-29T14:00:00Z", // "savedBy": "AetherSDR", // "memories": [ { "index": 0, ...MemoryEntry... } ] @@ -35,7 +35,7 @@ namespace AetherSDR { // bank is sparse. class LocalMemoryStore { public: - static constexpr int kFormatVersion = 1; + static constexpr int kFormatVersion = 2; static constexpr const char* kFormatId = "aether.memories"; // The bank's home since RFC #4603 PR 6: ONE shared feature document in diff --git a/src/core/backends/MemoryDelta.h b/src/core/backends/MemoryDelta.h index 5feb76429..405b90447 100644 --- a/src/core/backends/MemoryDelta.h +++ b/src/core/backends/MemoryDelta.h @@ -21,6 +21,8 @@ struct MemoryDelta { std::optional group; std::optional owner; std::optional channel; + std::optional importSource; + std::optional importKey; std::optional name; std::optional mode; std::optional offsetDir; // wire key "repeater" diff --git a/src/core/backends/RadioCapabilities.h b/src/core/backends/RadioCapabilities.h index f2e35f74d..2b2b05c10 100644 --- a/src/core/backends/RadioCapabilities.h +++ b/src/core/backends/RadioCapabilities.h @@ -234,11 +234,11 @@ struct RadioCapabilities { // this true when it can prove the radio gives the slots back. bool persistsMemories = false; - // Whether the radio-backed memory store accepts mutations and native - // recalls. These are deliberately separate from persistsMemories: an - // initial backend may prove that it can enumerate radio-owned channels - // before it is safe to overwrite them, and may expose those channels as - // tune presets without putting the radio into its vendor Memory mode. + // Whether the active memory store accepts mutations/native recalls, and + // whether the radio can be read as an explicit import source. Refresh is + // deliberately independent of persistsMemories: Icom keeps AetherSDR's + // shared client database as the working store while model-specific codecs + // ingest snapshots from the radio into it. bool canWriteMemories = false; bool canApplyMemories = false; bool canRefreshMemories = false; diff --git a/src/core/backends/icom/IcomCivBackend.cpp b/src/core/backends/icom/IcomCivBackend.cpp index f8a2dfc39..e07bfeafc 100644 --- a/src/core/backends/icom/IcomCivBackend.cpp +++ b/src/core/backends/icom/IcomCivBackend.cpp @@ -461,10 +461,14 @@ RadioCapabilities IcomCivBackend::capabilities() const c.hasRadioSideWaterfallAutoBlack = false; const MemoryProfile* memory = m_model && profileFor(*m_model).memory ? &*profileFor(*m_model).memory : nullptr; - c.persistsMemories = memory != nullptr; + // The AetherSDR memory model is always the shared client database for + // Icom. A model-specific codec only adds an explicit radio-to-database + // Sync source; it does not hand ownership of the working store to the + // radio. + c.persistsMemories = false; c.canWriteMemories = false; c.canApplyMemories = false; - c.canRefreshMemories = c.persistsMemories; + c.canRefreshMemories = memory != nullptr; if (memory) { c.memoryGroupColumnTitle = QString::fromLatin1(memory->groupColumnTitle.data(), static_cast(memory->groupColumnTitle.size())); @@ -726,6 +730,14 @@ void IcomCivBackend::connectRadio(const RadioConnectRequest& request) { disconnectRadio(); + // Radio memory reads are an ingestion path into AetherSDR's shared memory + // database. Prefer the discovery identity so repeated syncs from the same + // radio update their rows; retain a deterministic endpoint fallback for a + // manually-entered radio that reports no serial. + const QString memoryIdentity = request.serial.trimmed().isEmpty() + ? request.host.trimmed() : request.serial.trimmed(); + m_memoryImportSource = QStringLiteral("icom:%1").arg(memoryIdentity); + IcomSession::Params p; p.host = QHostAddress(request.host); p.controlPort = request.port ? request.port : kControlPort; @@ -2546,6 +2558,14 @@ void IcomCivBackend::onCivFrame(const CivFrame& frame, } MemoryDelta delta; delta.index = index; + delta.importSource = m_memoryImportSource; + delta.importKey = QStringLiteral("%1:%2") + .arg(memory->group) + .arg(memory->channel); + delta.owner = m_model + ? QString::fromLatin1(m_model->name.data(), + static_cast(m_model->name.size())) + : QStringLiteral("Icom"); if (!memory->occupied) { delta.removed = true; emit memoryChanged(delta); diff --git a/src/core/backends/icom/IcomCivBackend.h b/src/core/backends/icom/IcomCivBackend.h index 74b6e648a..d7b863846 100644 --- a/src/core/backends/icom/IcomCivBackend.h +++ b/src/core/backends/icom/IcomCivBackend.h @@ -394,6 +394,7 @@ private slots: QTimer* m_tuneTimer = nullptr; QString m_deviceName; + QString m_memoryImportSource; std::uint64_t m_frequencyHz = 0; CivMode m_mode = CivMode::Usb; bool m_dataMode = false; diff --git a/src/gui/MemoryDialog.cpp b/src/gui/MemoryDialog.cpp index 6f97c2d70..7ea8c719f 100644 --- a/src/gui/MemoryDialog.cpp +++ b/src/gui/MemoryDialog.cpp @@ -743,6 +743,9 @@ void MemoryDialog::populateTable() m_table->setRowCount(0); const auto& memories = m_model->memories(); const RadioCapabilities capabilities = m_model->backendCapabilities(); + // Radio sync folds rows into the shared writable AetherSDR bank. Once + // imported they use the same durable schema and recall path as manual/CSV + // rows, rather than replacing the dialog with a transient radio-only view. const bool usesNativeMemorySchema = capabilities.family == QLatin1String("icom") && capabilities.persistsMemories; for (int column = 0; column < COLUMNS.size(); ++column) { @@ -1475,6 +1478,14 @@ void MemoryDialog::rebuildFilterCombo() filterNames.append(profile); } } + // Read-only radio stores are import sources for this same database. + // Include their groups so group-selecting radios (IC-705) can still be + // synced without switching the dialog away from the local model. + for (const QString& group : capabilities.memoryGroups) { + if (!filterNames.contains(group)) { + filterNames.append(group); + } + } } filterNames.sort(Qt::CaseInsensitive); diff --git a/src/models/MemoryEntry.h b/src/models/MemoryEntry.h index b28571e16..92e8c29db 100644 --- a/src/models/MemoryEntry.h +++ b/src/models/MemoryEntry.h @@ -9,6 +9,11 @@ struct MemoryEntry { QString group; QString owner; QString channel; // native radio channel label, when one exists + // Stable provenance for a channel ingested from a radio. Empty for manual + // and CSV-created entries. Sync uses this pair to update the same database + // row on the next pass instead of duplicating it. + QString importSource; + QString importKey; double freq{0.0}; QString name; QString mode; diff --git a/src/models/RadioModel.cpp b/src/models/RadioModel.cpp index 6356d172e..b28a0e124 100644 --- a/src/models/RadioModel.cpp +++ b/src/models/RadioModel.cpp @@ -8068,15 +8068,51 @@ bool RadioModel::recallCachedMemory(int index) void RadioModel::applyMemoryChanges(const MemoryDelta& d) { + // A backend-provided import identity means this is a radio snapshot to fold + // into the one client database. Its native slot number is not a client slot: + // find the row previously imported from that radio/channel, or allocate a + // new client slot. This keeps manual/CSV memories visible and prevents a + // radio's channel 1 from overwriting the operator's client slot 1. + int targetIndex = d.index; + const bool isImport = d.importSource && !d.importSource->isEmpty() + && d.importKey && !d.importKey->isEmpty(); + if (isImport) { + m_localMemories.load(); + targetIndex = m_localMemories.importedSlot(*d.importSource, *d.importKey); + + if (d.removed) { + if (targetIndex >= 0) { + m_localMemories.forget(targetIndex); + if (m_memories.remove(targetIndex) > 0) { + emit memoryRemoved(targetIndex); + } + } + return; + } + + if (targetIndex < 0) { + const LocalMemoryBank::CommandResult created = + m_localMemories.handleCommand(QStringLiteral("memory create")); + bool indexOk = false; + targetIndex = created.body.toInt(&indexOk); + if (created.code != 0 || !indexOk) { + qCWarning(lcProtocol).noquote() + << "RadioModel: could not import radio memory" << *d.importKey + << "from" << *d.importSource << created.body; + return; + } + } + } + if (d.removed) { - if (m_memories.remove(d.index) > 0) { - emit memoryRemoved(d.index); + if (m_memories.remove(targetIndex) > 0) { + emit memoryRemoved(targetIndex); } return; } - auto& m = m_memories[d.index]; - m.index = d.index; + auto& m = m_memories[targetIndex]; + m.index = targetIndex; // Decode the protocol space-encoding (0x7f -> ' ') for free-text fields, // then strip any NUL/control bytes so corrupt values from the radio (or a @@ -8093,6 +8129,8 @@ void RadioModel::applyMemoryChanges(const MemoryDelta& d) if (d.group) m.group = decodeText(*d.group); if (d.owner) m.owner = decodeText(*d.owner); if (d.channel) m.channel = decodeText(*d.channel); + if (d.importSource) m.importSource = sanitize(*d.importSource); + if (d.importKey) m.importKey = sanitize(*d.importKey); if (d.name) m.name = decodeText(*d.name); if (d.mode) m.mode = sanitize(*d.mode); if (d.offsetDir) m.offsetDir = sanitize(*d.offsetDir); @@ -8117,7 +8155,10 @@ void RadioModel::applyMemoryChanges(const MemoryDelta& d) if (d.diglOffset) m.diglOffset = *d.diglOffset; if (d.diguOffset) m.diguOffset = *d.diguOffset; - emit memoryChanged(d.index); + if (isImport) { + m_localMemories.record(targetIndex, m); + } + emit memoryChanged(targetIndex); } // ─── Raw message handler (for meter status with '#' separators) ────────────── diff --git a/src/models/RadioModel.h b/src/models/RadioModel.h index 1bf2cef74..a77d32cfa 100644 --- a/src/models/RadioModel.h +++ b/src/models/RadioModel.h @@ -559,16 +559,17 @@ class RadioModel : public QObject { // ── Memory command routing ────────────────────────────────────────────── // - // Answer a `memory …` command from the local bank or the cached read-only - // radio view. Returns the sequence number sendCmd() would have returned + // Answer a `memory …` command from the local bank or a native writable + // radio. Returns the sequence number sendCmd() would have returned // (non-zero — sendCommand() reads that as "dispatched"), or nullopt when a // writable/native radio backend must take its normal path. // (spelled out rather than the ResponseCallback alias — that is declared // further down this class.) std::optional tryMemoryCommand( const QString& command, const RadioConnection::ResponseCallback& cb); - // Settle which store owns the memory cache for the session being started: - // the local bank, or the radio's own slots. + // Settle which store owns the memory cache for the session being started. + // Read-only radio snapshots are ingested into the local bank; only a native + // writable store takes exclusive ownership. void syncMemoryStoreForSession(); // Push the loaded bank into m_memories, emitting per-slot memoryChanged so // the browse panel and the panadapter memory-spot feed populate exactly as @@ -626,14 +627,13 @@ class RadioModel : public QObject { const QMap& memories() const { return m_memories; } void handleMemoryStatus(int index, const QMap& kvs); - // True when memory channels live in a file on THIS host rather than in the - // radio — the HL2/Kiwi/demo case, and the disconnected case. Driven by - // RadioCapabilities::persistsMemories, so a new backend gets the local bank - // by default rather than writing channels into a radio that drops them. + // True when the working memory model lives in the database on THIS host — + // including every Icom (radio sync is an ingestion path), HL2/Kiwi/demo, + // and the disconnected case. Only a native writable radio store opts out. bool usesLocalMemoryBank() const; - // True when the active store accepts create/edit/remove. A radio-backed - // read-only snapshot (initial Icom support) returns false while the - // existing host bank and Flex radio return true. + // True when the active working store accepts create/edit/remove. Icom uses + // the host bank even though its radio-side source is read-only; Flex writes + // its own native store. bool memoriesWritable() const; bool memoriesRefreshable() const; void refreshMemories(const QString& group = QString()); diff --git a/tests/local_memory_bank_test.cpp b/tests/local_memory_bank_test.cpp index 985257af0..2f08117e4 100644 --- a/tests/local_memory_bank_test.cpp +++ b/tests/local_memory_bank_test.cpp @@ -131,6 +131,42 @@ int main(int argc, char** argv) ok &= expect(!apply.delta.has_value(), "apply changes no stored state"); } + // --- imported radio identity ----------------------------------------- + { + resetBankDocument(); + LocalMemoryBank bank; + bank.setFilePath(dir.path() + "/imports.json"); + bank.handleCommand("memory create"); + + MemoryEntry imported; + imported.importSource = QStringLiteral("icom:7300-serial"); + imported.importKey = QStringLiteral("-1:42"); + imported.channel = QStringLiteral("42"); + imported.freq = 14.074; + imported.name = QStringLiteral("FT8"); + imported.nativeFilter = 2; + imported.dataMode = 1; + bank.record(0, imported); + + ok &= expect(bank.importedSlot("icom:7300-serial", "-1:42") == 0, + "a repeated radio sync finds its existing database row"); + ok &= expect(bank.importedSlot("icom:other-radio", "-1:42") == -1, + "the same channel on another radio does not alias"); + ok &= expect(bank.importedSlot("icom:7300-serial", "-1:43") == -1, + "a different native channel does not alias"); + + bank.flush(); + LocalMemoryBank reopened; + reopened.setFilePath(dir.path() + "/unused-imports.json"); + reopened.load(); + ok &= expect(reopened.importedSlot("icom:7300-serial", "-1:42") == 0, + "radio import identity survives a database reopen"); + const MemoryEntry stored = reopened.entries().value(0); + ok &= expect(stored.channel == "42" && stored.nativeFilter == 2 + && stored.dataMode == 1, + "radio recall fields survive a database reopen"); + } + // --- rejections ------------------------------------------------------- { resetBankDocument(); diff --git a/tests/local_memory_store_test.cpp b/tests/local_memory_store_test.cpp index 99eeb8af1..16edbaf77 100644 --- a/tests/local_memory_store_test.cpp +++ b/tests/local_memory_store_test.cpp @@ -24,14 +24,24 @@ MemoryEntry sampleMemory(int index) m.index = index; m.group = "Local Repeaters"; m.owner = "KI6BCJ"; + m.channel = "42"; + m.importSource = "icom:radio-serial"; + m.importKey = "-1:42"; m.freq = 146.94; m.name = "W6ABC Mt Diablo"; m.mode = "FM"; + m.nativeFilter = 2; + m.dataMode = 1; m.step = 5000; m.offsetDir = "down"; m.repeaterOffset = 0.6; m.toneMode = "ctcss_tx"; m.toneValue = 103.5; + m.rxToneValue = 88.5; + m.dtcsCode = 23; + m.dtcsTxReverse = true; + m.dtcsRxReverse = false; + m.recallable = true; m.squelch = true; m.squelchLevel = 20; m.rxFilterLow = -8000; diff --git a/tests/radio_capability_gating_test.cpp b/tests/radio_capability_gating_test.cpp index ea09b5590..63b6e6d5d 100644 --- a/tests/radio_capability_gating_test.cpp +++ b/tests/radio_capability_gating_test.cpp @@ -647,6 +647,9 @@ int main(int argc, char** argv) check(caps.fmDtcsCodes.size() == 104 && caps.fmToneModes.contains(QStringLiteral("dtcs_txrx")), "IC-9700 declares the complete DTCS operator vocabulary"); + check(!caps.persistsMemories && !caps.canWriteMemories + && caps.canRefreshMemories, + "IC-9700 keeps the client database writable and exposes radio sync"); } { @@ -662,6 +665,9 @@ int main(int argc, char** argv) check(caps.fmDtcsCodes.size() == 104 && caps.fmToneModes.contains(QStringLiteral("dtcs_txrx")), "IC-705 declares its documented DTCS operator vocabulary"); + check(!caps.persistsMemories && !caps.canWriteMemories + && caps.canRefreshMemories, + "IC-705 keeps the client database writable and exposes grouped sync"); } } @@ -677,6 +683,9 @@ int main(int argc, char** argv) "IC-7300MK2 retains the legacy PROC presentation"); check(caps.fmDtcsCodes.isEmpty(), "Icom models without documented DTCS do not activate controls"); + check(!caps.persistsMemories && !caps.canWriteMemories + && caps.canRefreshMemories, + "IC-7300MK2 keeps the client database writable and exposes radio sync"); } } From 6b9192a641d0f8a38420eb550b4cdb4b011f59ae Mon Sep 17 00:00:00 2001 From: jensenpat Date: Sat, 29 Aug 2026 22:39:12 -0700 Subject: [PATCH 2/5] Fix IC-7300MK2 synced memory recall (Principle VIII) --- src/core/backends/icom/IcomMemoryCodec.cpp | 8 +++++++- tests/icom_memory_test.cpp | 11 ++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/core/backends/icom/IcomMemoryCodec.cpp b/src/core/backends/icom/IcomMemoryCodec.cpp index 0d4cfd214..485a57b36 100644 --- a/src/core/backends/icom/IcomMemoryCodec.cpp +++ b/src/core/backends/icom/IcomMemoryCodec.cpp @@ -161,7 +161,13 @@ std::optional decodeMemory( return std::nullopt; } - memory.split = payload.size() == static_cast(layout.splitBytes); + // The IC-7300MK2 always returns the second 4..17 block documented by its + // CI-V guide, even when Split is OFF. Its byte-3 SPLIT flag, decoded + // below, is therefore the authority; treating the 47-byte reply length as + // Split made every live channel display-only and prevented spot recall. + // The older dialects retain their established variable-length contract. + memory.split = dialect != MemoryDialect::Ic7300Mk2 + && payload.size() == static_cast(layout.splitBytes); const std::optional frequency = decodeFreqExact( payload.subspan(static_cast(layout.frequencyOffset), kFreqBytes), kFreqBytes); diff --git a/tests/icom_memory_test.cpp b/tests/icom_memory_test.cpp index ce9af9d00..9b45d3691 100644 --- a/tests/icom_memory_test.cpp +++ b/tests/icom_memory_test.cpp @@ -62,7 +62,10 @@ std::vector ic705Record() std::vector ic7300Record() { - std::vector p(33, 0); + // The IC-7300MK2 guide documents both 4..17 blocks in every record and a + // separate SPLIT flag in byte 3. Live radio replies are consequently 47 + // bytes even for ordinary non-split memories. + std::vector p(47, 0); p[0] = 0x00; p[1] = 0x42; p[2] = 0x00; putFrequency(p, 3); p[8] = 0x05; p[9] = 0x02; p[10] = 0x11; @@ -101,6 +104,12 @@ int main() check(m7300 && m7300->group == -1 && m7300->channel == 42 && m7300->mode == "DFM" && m7300->toneMode == 1 && m7300->recallable, "IC-7300MK2 official compact memory layout decodes"); + auto ic7300SplitRecord = ic7300Record(); + ic7300SplitRecord[2] = 0x10; + const auto m7300Split = decodeMemory( + MemoryDialect::Ic7300Mk2, ic7300SplitRecord); + check(m7300Split && m7300Split->split && !m7300Split->recallable, + "IC-7300MK2 split flag, not normal reply length, gates recall"); for (int toneMode = 0; toneMode <= 3; ++toneMode) { auto record = ic9700Record(); From 776c7ca796636e52728270782e4b610fde74d9bc Mon Sep 17 00:00:00 2001 From: jensenpat Date: Sat, 29 Aug 2026 22:58:04 -0700 Subject: [PATCH 3/5] Fix persisted Icom memory navigation (Principle XI) --- .../aetherd-icom-civ-backend-design.md | 16 +++-- src/core/LocalMemoryBank.cpp | 24 +++++++ src/core/backends/icom/IcomCivBackend.cpp | 30 +++++++-- src/core/backends/icom/IcomMemoryCodec.cpp | 7 +- src/core/backends/icom/IcomProtocol.cpp | 17 +++++ src/core/backends/icom/IcomProtocol.h | 4 ++ src/core/backends/icom/IcomSession.h | 1 + src/gui/MemoryDialog.cpp | 65 +++++-------------- src/gui/MemoryFilterPolicy.h | 64 ++++++++++++++++++ tests/icom_memory_test.cpp | 10 +-- tests/icom_protocol_test.cpp | 4 ++ tests/local_memory_bank_test.cpp | 15 +++++ tests/radio_capability_gating_test.cpp | 32 +++++++++ tests/tests.cmake | 1 + 14 files changed, 222 insertions(+), 68 deletions(-) create mode 100644 src/gui/MemoryFilterPolicy.h diff --git a/docs/architecture/aetherd-icom-civ-backend-design.md b/docs/architecture/aetherd-icom-civ-backend-design.md index a2f64ecfa..6818227a1 100644 --- a/docs/architecture/aetherd-icom-civ-backend-design.md +++ b/docs/architecture/aetherd-icom-civ-backend-design.md @@ -685,13 +685,15 @@ captures from our own radio. IC-9700, **Sync Memories** reads the model-specific ordinary-channel records with `1A 00` and ingests occupied channels into that database; Tune then recalls the durable database row like a manual or CSV-imported memory. - Repeated syncs update rows from the same radio/channel rather than duplicating - them. Reads are button-only; IC-705 requires a selected group so a click - queues 100 requests rather than scanning its 10,000-address space. Writing or - deleting the radio's own channels, plus scan-edge, call, and satellite - memories, remain deferred. Other Icom models still use the same client-side - database, but expose no Sync action until their published record layout is - implemented and verified. + Imported rows are keyed by the 16-byte radio GUID from the authenticated + RS-BA1 capabilities record plus the native group/channel, so DHCP, mDNS and + NAT endpoint changes cannot duplicate a radio's channel set. Reads are + button-only; IC-705 requires a selected native group so a click queues 100 + requests rather than scanning its 10,000-address space. Flex global/TX + profiles are not valid Icom group selectors. Writing or deleting the radio's + own channels, plus scan-edge, call, and satellite memories, remain deferred. + Other Icom models still use the same client-side database, but expose no Sync + action until their published record layout is implemented and verified. - **D-STAR / DV.** A large command surface (`22 xx`, `23 xx`) and a separate feature. - **Bluetooth transport.** Unknown whether it carries all three streams. diff --git a/src/core/LocalMemoryBank.cpp b/src/core/LocalMemoryBank.cpp index e948f1dc4..4cec85a3a 100644 --- a/src/core/LocalMemoryBank.cpp +++ b/src/core/LocalMemoryBank.cpp @@ -3,6 +3,7 @@ #include "core/AppSettings.h" #include "core/LocalMemoryStore.h" #include "core/LogManager.h" +#include "core/MemoryFieldValues.h" #include "core/backends/MemoryWireCodec.h" #include @@ -120,6 +121,29 @@ void LocalMemoryBank::load() return; } + // The first Icom memory-import build persisted every IC-7300MK2 channel as + // display-only after mistaking its fixed-length RX+TX record for proof that + // Split was enabled. Recallability is a property of the neutral database + // row: if its mode can be applied to an AetherSDR slice, stale Icom import + // metadata must not permanently prevent navigation. Repair the rows once + // on load and persist the corrected document atomically. + int repairedRecallability = 0; + for (auto it = m_entries.begin(); it != m_entries.end(); ++it) { + if (!it->recallable + && it->importSource.startsWith(QLatin1String("icom:")) + && MemoryFields::isKnownMode(it->mode)) { + it->recallable = true; + ++repairedRecallability; + } + } + if (repairedRecallability > 0) { + m_dirty = true; + flush(); + qCInfo(lcProtocol).noquote() + << "LocalMemoryBank: repaired recallability for" + << repairedRecallability << "Icom-imported memories"; + } + if (importedFromLegacy) { // Claim the legacy channels into the document now, so the migration // is not contingent on the operator making an edit first. The legacy diff --git a/src/core/backends/icom/IcomCivBackend.cpp b/src/core/backends/icom/IcomCivBackend.cpp index e07bfeafc..eb973623b 100644 --- a/src/core/backends/icom/IcomCivBackend.cpp +++ b/src/core/backends/icom/IcomCivBackend.cpp @@ -730,13 +730,10 @@ void IcomCivBackend::connectRadio(const RadioConnectRequest& request) { disconnectRadio(); - // Radio memory reads are an ingestion path into AetherSDR's shared memory - // database. Prefer the discovery identity so repeated syncs from the same - // radio update their rows; retain a deterministic endpoint fallback for a - // manually-entered radio that reports no serial. - const QString memoryIdentity = request.serial.trimmed().isEmpty() - ? request.host.trimmed() : request.serial.trimmed(); - m_memoryImportSource = QStringLiteral("icom:%1").arg(memoryIdentity); + // The stable import identity arrives in the authenticated RS-BA1 + // capabilities record. An endpoint is deliberately not used here: DHCP, + // mDNS and NAT changes must not turn one radio into a second import source. + m_memoryImportSource.clear(); IcomSession::Params p; p.host = QHostAddress(request.host); @@ -1122,6 +1119,14 @@ void IcomCivBackend::refreshMemories(const QString& groupName) return; } const MemoryProfile& memory = *profileFor(*m_model).memory; + if (m_memoryImportSource.isEmpty()) { + qCWarning(lcIcomCiv) + << "memory sync refused: RS-BA1 supplied no stable radio identity"; + emit configurationWarning( + QStringLiteral("This radio did not provide a stable RS-BA1 identity, so its " + "memories cannot be synced safely.")); + return; + } int selectedGroup = -1; if (!groupName.isEmpty() && memory.firstGroup >= 0) { for (int group = memory.firstGroup; group <= memory.lastGroup; ++group) { @@ -1132,6 +1137,10 @@ void IcomCivBackend::refreshMemories(const QString& groupName) } } if (memory.requiresGroupSelection && selectedGroup < memory.firstGroup) { + qCWarning(lcIcomCiv) + << "memory sync refused: invalid group selection" << groupName; + emit configurationWarning( + QStringLiteral("Choose a valid Icom memory group before syncing.")); return; } m_memoryRefreshActive = true; @@ -1391,6 +1400,13 @@ void IcomCivBackend::adoptReportedCivAddress(std::uint8_t reported) void IcomCivBackend::onSessionConnected(const QString& deviceName) { m_deviceName = deviceName.trimmed(); + const std::string stableRadioId = radioIdHex(m_session->radioId()); + if (stableRadioId.empty()) { + m_memoryImportSource.clear(); + } else { + m_memoryImportSource = QStringLiteral("icom:%1").arg( + QString::fromStdString(stableRadioId)); + } m_connected = true; m_connectedAtMs = nowMs(); m_lastIncident.clear(); diff --git a/src/core/backends/icom/IcomMemoryCodec.cpp b/src/core/backends/icom/IcomMemoryCodec.cpp index 485a57b36..6eb5e2e50 100644 --- a/src/core/backends/icom/IcomMemoryCodec.cpp +++ b/src/core/backends/icom/IcomMemoryCodec.cpp @@ -223,7 +223,12 @@ std::optional decodeMemory( } memory.split = memory.split || ((selectByte >> 4) != 0); } - memory.recallable = mode.mode.has_value() && !memory.split && duplex != 3; + // AetherSDR recalls the RX side of a stored channel onto the active slice. + // Split/RPS metadata is useful provenance, but it must not make a memory + // impossible to navigate to: the local database already has a complete, + // neutral RX frequency and mode. Modes with no neutral representation + // (currently DV/DD) remain display-only. + memory.recallable = mode.mode.has_value(); if (layout.dtcsOffset >= 0) { const std::optional dtcs = decodeRepeaterToneRegister( diff --git a/src/core/backends/icom/IcomProtocol.cpp b/src/core/backends/icom/IcomProtocol.cpp index 2bb6eda05..13424cf4e 100644 --- a/src/core/backends/icom/IcomProtocol.cpp +++ b/src/core/backends/icom/IcomProtocol.cpp @@ -441,6 +441,23 @@ bool parseCapabilities(std::span pkt, RadioId& radioId) return true; } +std::string radioIdHex(const RadioId& radioId) +{ + static constexpr char kHex[] = "0123456789abcdef"; + if (std::all_of(radioId.begin(), radioId.end(), + [](std::uint8_t byte) { return byte == 0; })) { + return {}; + } + + std::string encoded; + encoded.reserve(radioId.size() * 2); + for (const std::uint8_t byte : radioId) { + encoded.push_back(kHex[(byte >> 4) & 0x0f]); + encoded.push_back(kHex[byte & 0x0f]); + } + return encoded; +} + std::string parseCapabilitiesName(std::span pkt) { if (!startsWith(pkt, kLenCapabilities, 0xa8)) diff --git a/src/core/backends/icom/IcomProtocol.h b/src/core/backends/icom/IcomProtocol.h index bdf367b0a..cbcf8c755 100644 --- a/src/core/backends/icom/IcomProtocol.h +++ b/src/core/backends/icom/IcomProtocol.h @@ -334,6 +334,10 @@ struct AuthReply { // Extract the radio identity from the 0xA8 capabilities packet. [[nodiscard]] bool parseCapabilities(std::span pkt, RadioId& radioId); +// Stable, address-independent identity for one radio advertised by the RS-BA1 +// capabilities record. Empty means the record supplied no usable identity. +[[nodiscard]] std::string radioIdHex(const RadioId& radioId); + // The radio's own name ("IC-705") from the same packet. Parsed rather than // hardcoded: the stream request has to name the radio it wants, and a literal // there is exactly what stops this backend reaching an IC-9700 or an RS-BA1 diff --git a/src/core/backends/icom/IcomSession.h b/src/core/backends/icom/IcomSession.h index 035cf870f..c41569ea8 100644 --- a/src/core/backends/icom/IcomSession.h +++ b/src/core/backends/icom/IcomSession.h @@ -75,6 +75,7 @@ class IcomSession : public QObject { [[nodiscard]] bool isConnected() const noexcept { return m_connected; } [[nodiscard]] QString deviceName() const { return m_deviceName; } + [[nodiscard]] const RadioId& radioId() const noexcept { return m_radioId; } [[nodiscard]] std::uint8_t civAddress() const noexcept { return m_params.civAddress; } // RETARGET the session at a different CI-V address, mid-session. diff --git a/src/gui/MemoryDialog.cpp b/src/gui/MemoryDialog.cpp index 7ea8c719f..8c637129f 100644 --- a/src/gui/MemoryDialog.cpp +++ b/src/gui/MemoryDialog.cpp @@ -1,4 +1,5 @@ #include "MemoryDialog.h" +#include "MemoryFilterPolicy.h" #include "MemoryCommands.h" #include "core/MemoryCsvCompat.h" #include "core/MemoryFieldValues.h" @@ -405,17 +406,12 @@ MemoryDialog::MemoryDialog(RadioModel* model, QWidget* parent) QStringList groups; for (const auto& m : m_model->memories()) { const QString g = m.group.trimmed(); - if (!g.isEmpty() && !groups.contains(g)) - groups << g; + appendMemoryFilterName(groups, g); } - for (const QString& p : m_model->globalProfiles()) - if (!p.isEmpty() && !groups.contains(p)) - groups << p; - for (const QString& p : m_model->transmitModel().profileList()) - if (!p.isEmpty() && !groups.contains(p)) - groups << p; - groups.sort(Qt::CaseInsensitive); - return groups; + const RadioCapabilities capabilities = m_model->backendCapabilities(); + return memoryFilterSpec(capabilities, groups, + m_model->globalProfiles(), + m_model->transmitModel().profileList()).names; }, true, Validator::None, this)); m_table->setItemDelegateForColumn(4, new MemoryFieldDelegate( staticList(MemoryFields::modes()), false, Validator::None, this)); @@ -1452,44 +1448,17 @@ void MemoryDialog::rebuildFilterCombo() // "All" shows every memory regardless of group m_filterCombo->addItem("All Memories", QString()); - const RadioCapabilities capabilities = m_model->backendCapabilities(); - const bool usesNativeMemorySchema = capabilities.family == QLatin1String("icom") - && capabilities.persistsMemories; - m_filterLabel->setText(usesNativeMemorySchema ? QStringLiteral("Group:") - : QStringLiteral("Profile:")); - QStringList filterNames; - if (usesNativeMemorySchema) { - filterNames = capabilities.memoryGroups; - for (const MemoryEntry& memory : m_model->memories()) { - const QString group = memory.group.trimmed(); - if (!group.isEmpty() - && !filterNames.contains(group, Qt::CaseInsensitive)) { - filterNames.append(group); - } - } - } else { - for (const QString& profile : m_model->globalProfiles()) { - if (!filterNames.contains(profile)) { - filterNames.append(profile); - } - } - for (const QString& profile : m_model->transmitModel().profileList()) { - if (!filterNames.contains(profile)) { - filterNames.append(profile); - } - } - // Read-only radio stores are import sources for this same database. - // Include their groups so group-selecting radios (IC-705) can still be - // synced without switching the dialog away from the local model. - for (const QString& group : capabilities.memoryGroups) { - if (!filterNames.contains(group)) { - filterNames.append(group); - } - } + QStringList storedGroups; + for (const MemoryEntry& memory : m_model->memories()) { + appendMemoryFilterName(storedGroups, memory.group); } - filterNames.sort(Qt::CaseInsensitive); + const RadioCapabilities capabilities = m_model->backendCapabilities(); + const MemoryFilterSpec filterSpec = memoryFilterSpec( + capabilities, storedGroups, m_model->globalProfiles(), + m_model->transmitModel().profileList()); + m_filterLabel->setText(filterSpec.label); - for (const QString& name : filterNames) { + for (const QString& name : filterSpec.names) { m_filterCombo->addItem(name, name); } @@ -1560,8 +1529,8 @@ void MemoryDialog::updateSelectionActions() if (m_syncBtn) { const bool refreshable = m_model->memoriesRefreshable(); const RadioCapabilities capabilities = m_model->backendCapabilities(); - const bool hasRequiredGroup = !capabilities.memoryRefreshRequiresGroup - || !m_filterCombo->currentData().toString().isEmpty(); + const bool hasRequiredGroup = memoryRefreshSelectionValid( + capabilities, m_filterCombo->currentData().toString()); m_syncBtn->setVisible(refreshable); m_syncBtn->setEnabled(refreshable && hasRequiredGroup && !m_syncInProgress); m_syncBtn->setToolTip(hasRequiredGroup diff --git a/src/gui/MemoryFilterPolicy.h b/src/gui/MemoryFilterPolicy.h new file mode 100644 index 000000000..5c8bd1f4d --- /dev/null +++ b/src/gui/MemoryFilterPolicy.h @@ -0,0 +1,64 @@ +#pragma once + +#include "core/backends/RadioCapabilities.h" + +#include +#include + +namespace AetherSDR { + +struct MemoryFilterSpec { + QString label; + QStringList names; +}; + +inline void appendMemoryFilterName(QStringList& names, const QString& candidate) +{ + const QString normalized = candidate.trimmed(); + if (!normalized.isEmpty() + && !names.contains(normalized, Qt::CaseInsensitive)) { + names.append(normalized); + } +} + +// Flex memories use radio-owned global/TX profiles. Client-owned memory banks +// use their own groups, supplemented only by native groups an import source +// requires. Keeping the two vocabularies separate prevents stale Flex profile +// state from becoming an Icom Sync argument after a radio-family switch. +[[nodiscard]] inline MemoryFilterSpec memoryFilterSpec( + const RadioCapabilities& capabilities, + const QStringList& storedGroups, + const QStringList& globalProfiles, + const QStringList& txProfiles) +{ + MemoryFilterSpec spec; + if (capabilities.hasProfiles) { + spec.label = QStringLiteral("Profile:"); + for (const QString& profile : globalProfiles) { + appendMemoryFilterName(spec.names, profile); + } + for (const QString& profile : txProfiles) { + appendMemoryFilterName(spec.names, profile); + } + } else { + spec.label = QStringLiteral("Group:"); + for (const QString& group : capabilities.memoryGroups) { + appendMemoryFilterName(spec.names, group); + } + for (const QString& group : storedGroups) { + appendMemoryFilterName(spec.names, group); + } + } + spec.names.sort(Qt::CaseInsensitive); + return spec; +} + +[[nodiscard]] inline bool memoryRefreshSelectionValid( + const RadioCapabilities& capabilities, + const QString& selection) +{ + return !capabilities.memoryRefreshRequiresGroup + || capabilities.memoryGroups.contains(selection.trimmed(), Qt::CaseInsensitive); +} + +} // namespace AetherSDR diff --git a/tests/icom_memory_test.cpp b/tests/icom_memory_test.cpp index 9b45d3691..b468a5c64 100644 --- a/tests/icom_memory_test.cpp +++ b/tests/icom_memory_test.cpp @@ -108,8 +108,8 @@ int main() ic7300SplitRecord[2] = 0x10; const auto m7300Split = decodeMemory( MemoryDialect::Ic7300Mk2, ic7300SplitRecord); - check(m7300Split && m7300Split->split && !m7300Split->recallable, - "IC-7300MK2 split flag, not normal reply length, gates recall"); + check(m7300Split && m7300Split->split && m7300Split->recallable, + "IC-7300MK2 split memory can recall its neutral RX side"); for (int toneMode = 0; toneMode <= 3; ++toneMode) { auto record = ic9700Record(); @@ -125,10 +125,10 @@ int main() auto reverseSplit = ic9700Record(); reverseSplit[12] = 0x31; const auto rps = decodeMemory(MemoryDialect::Ic9700, reverseSplit); - check(rps && !rps->recallable, "IC-9700 RPS remains visible but cannot be recalled"); + check(rps && rps->recallable, "IC-9700 RPS can recall its neutral RX side"); const auto split = decodeMemory(MemoryDialect::Ic9700, ic9700Record(114)); - check(split && split->split && !split->recallable, - "split records remain visible but cannot discard their TX half on recall"); + check(split && split->split && split->recallable, + "split records can recall their neutral RX side"); auto dvRecord = ic705Record(); dvRecord[10] = 0x17; dvRecord[11] = 0x00; const auto dv = decodeMemory(MemoryDialect::Ic705, dvRecord); diff --git a/tests/icom_protocol_test.cpp b/tests/icom_protocol_test.cpp index 03b271b8c..56292bb8e 100644 --- a/tests/icom_protocol_test.cpp +++ b/tests/icom_protocol_test.cpp @@ -274,6 +274,10 @@ static void testCapabilities() RadioId id{}; check(parseCapabilities(caps, id), "recognises the capabilities packet"); check(id[0] == 1 && id[15] == 16, "radio id is the 16 bytes at 0x42"); + check(radioIdHex(id) == "0102030405060708090a0b0c0d0e0f10", + "radio id has a stable address-independent import key"); + check(radioIdHex(RadioId{}).empty(), + "an absent radio id is rejected rather than aliasing every radio"); check(parseCapabilitiesName(caps) == "IC-705", "the radio names itself — hardcoding IC-705 is what blocks other models"); } diff --git a/tests/local_memory_bank_test.cpp b/tests/local_memory_bank_test.cpp index 2f08117e4..4e1273739 100644 --- a/tests/local_memory_bank_test.cpp +++ b/tests/local_memory_bank_test.cpp @@ -144,8 +144,13 @@ int main(int argc, char** argv) imported.channel = QStringLiteral("42"); imported.freq = 14.074; imported.name = QStringLiteral("FT8"); + imported.mode = QStringLiteral("DIGU"); imported.nativeFilter = 2; imported.dataMode = 1; + // Reproduce the poisoned flag written by the first IC-7300MK2 import + // implementation. Reopening the bank must repair it without requiring + // another live-radio sync. + imported.recallable = false; bank.record(0, imported); ok &= expect(bank.importedSlot("icom:7300-serial", "-1:42") == 0, @@ -165,6 +170,16 @@ int main(int argc, char** argv) ok &= expect(stored.channel == "42" && stored.nativeFilter == 2 && stored.dataMode == 1, "radio recall fields survive a database reopen"); + ok &= expect(stored.recallable, + "stale Icom display-only metadata is repaired on reopen"); + ok &= expect(bankDocument() + .value(QStringLiteral("memories")) + .toArray() + .at(0) + .toObject() + .value(QStringLiteral("recallable")) + .toBool(), + "the repaired Icom recallability is persisted"); } // --- rejections ------------------------------------------------------- diff --git a/tests/radio_capability_gating_test.cpp b/tests/radio_capability_gating_test.cpp index 63b6e6d5d..7f8ed13ec 100644 --- a/tests/radio_capability_gating_test.cpp +++ b/tests/radio_capability_gating_test.cpp @@ -107,6 +107,7 @@ #include "core/AppSettings.h" #include "models/ModelCapabilities.h" #include "gui/DvkAvailabilityGate.h" +#include "gui/MemoryFilterPolicy.h" #include "gui/VoiceModeGate.h" #include "core/RadioDiscovery.h" #include "core/backends/flex/FlexBackend.h" @@ -229,6 +230,36 @@ static bool gpsUiWouldShow(bool connected, bool familySupportsGps, bool unitHasG return !connected || (familySupportsGps && unitHasGps); } +static void testMemoryFilterPolicy() +{ + RadioCapabilities flex; + flex.hasProfiles = true; + const MemoryFilterSpec flexFilters = memoryFilterSpec( + flex, {QStringLiteral("Local group")}, {QStringLiteral("Global")}, + {QStringLiteral("Transmit")}); + check(flexFilters.label == QStringLiteral("Profile:") + && flexFilters.names == QStringList({QStringLiteral("Global"), + QStringLiteral("Transmit")}), + "Flex memory filter retains radio-owned global and TX profiles"); + + RadioCapabilities icom; + icom.canRefreshMemories = true; + icom.memoryRefreshRequiresGroup = true; + icom.memoryGroups = {QStringLiteral("Group 00"), QStringLiteral("Group 01")}; + const MemoryFilterSpec icomFilters = memoryFilterSpec( + icom, {QStringLiteral("Local group")}, {QStringLiteral("Stale Flex global")}, + {QStringLiteral("Stale Flex TX")}); + check(icomFilters.label == QStringLiteral("Group:") + && icomFilters.names.contains(QStringLiteral("Group 00")) + && icomFilters.names.contains(QStringLiteral("Local group")) + && !icomFilters.names.contains(QStringLiteral("Stale Flex global")) + && !icomFilters.names.contains(QStringLiteral("Stale Flex TX")), + "Icom memory filter excludes Flex profile state"); + check(!memoryRefreshSelectionValid(icom, QStringLiteral("Stale Flex global")) + && memoryRefreshSelectionValid(icom, QStringLiteral("group 01")), + "group-selecting Icom accepts only a declared native memory group"); +} + static RadioInfo hl2Info() { RadioInfo i; @@ -265,6 +296,7 @@ int main(int argc, char** argv) TestSettingsProfile profile(QStringLiteral("radio-capability-gating-test")); QCoreApplication app(argc, argv); AppSettings::instance().load(); + testMemoryFilterPolicy(); // ---- Flex declares every gated capability ---------------------------- // diff --git a/tests/tests.cmake b/tests/tests.cmake index 4f743661f..cb41b9d28 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -3538,6 +3538,7 @@ add_executable(local_memory_bank_test tests/local_memory_bank_test.cpp src/core/LocalMemoryBank.cpp src/core/LocalMemoryStore.cpp + src/core/MemoryFieldValues.cpp src/core/backends/MemoryWireCodec.cpp src/core/AsyncLogWriter.cpp src/core/LogManager.cpp From e87d783a3ba8167c9cfb98d457f359db3b45ac27 Mon Sep 17 00:00:00 2001 From: Robbie Foust Date: Fri, 4 Sep 2026 23:40:42 -0400 Subject: [PATCH 4/5] Preserve safe Icom memory sync and recall. Principle XI. --- .../aetherd-icom-civ-backend-design.md | 13 +- src/core/LocalMemoryBank.cpp | 28 +--- src/core/LocalMemoryStore.cpp | 16 ++- src/core/LocalMemoryStore.h | 5 +- src/core/backends/icom/IcomMemoryCodec.cpp | 11 +- src/models/RadioModel.cpp | 31 +++-- tests/icom_memory_test.cpp | 17 ++- tests/local_memory_bank_test.cpp | 48 +++++-- tests/local_memory_store_test.cpp | 18 +++ tests/memory_import_test.cpp | 128 ++++++++++++++++++ tests/tests.cmake | 8 +- 11 files changed, 262 insertions(+), 61 deletions(-) create mode 100644 tests/memory_import_test.cpp diff --git a/docs/architecture/aetherd-icom-civ-backend-design.md b/docs/architecture/aetherd-icom-civ-backend-design.md index a2146d0c4..a8153f6fd 100644 --- a/docs/architecture/aetherd-icom-civ-backend-design.md +++ b/docs/architecture/aetherd-icom-civ-backend-design.md @@ -709,8 +709,17 @@ captures from our own radio. recalls the durable database row like a manual or CSV-imported memory. Imported rows are keyed by the 16-byte radio GUID from the authenticated RS-BA1 capabilities record plus the native group/channel, so DHCP, mDNS and - NAT endpoint changes cannot duplicate a radio's channel set. Reads are - button-only; IC-705 requires a selected native group so a click queues 100 + NAT endpoint changes cannot duplicate a radio's channel set. Repeat Sync + refreshes tuning fields while preserving the name, owner and group assigned + at first import or edited locally. Clearing a native channel removes its + matching imported row. Split/RPS/DV/DD records remain display-only. + Existing experimental imports with incorrect recallability need one explicit + Sync: they did not retain enough split metadata for a safe load-time repair. + Loading an existing bank never rewrites it. Ordinary local memories remain + schema 1; saves containing native recall fields use schema 2 so an older + writer cannot erase recallability, DTCS state or provenance. Downgrading + after such a save requires a compatible build or a pre-Sync settings backup. + Reads are button-only; IC-705 requires a selected native group so a click queues 100 requests rather than scanning its 10,000-address space. Flex global/TX profiles are not valid Icom group selectors. Writing or deleting the radio's own channels, plus scan-edge, call, and satellite memories, remain deferred. diff --git a/src/core/LocalMemoryBank.cpp b/src/core/LocalMemoryBank.cpp index 4cec85a3a..17c8adff3 100644 --- a/src/core/LocalMemoryBank.cpp +++ b/src/core/LocalMemoryBank.cpp @@ -3,7 +3,6 @@ #include "core/AppSettings.h" #include "core/LocalMemoryStore.h" #include "core/LogManager.h" -#include "core/MemoryFieldValues.h" #include "core/backends/MemoryWireCodec.h" #include @@ -121,28 +120,9 @@ void LocalMemoryBank::load() return; } - // The first Icom memory-import build persisted every IC-7300MK2 channel as - // display-only after mistaking its fixed-length RX+TX record for proof that - // Split was enabled. Recallability is a property of the neutral database - // row: if its mode can be applied to an AetherSDR slice, stale Icom import - // metadata must not permanently prevent navigation. Repair the rows once - // on load and persist the corrected document atomically. - int repairedRecallability = 0; - for (auto it = m_entries.begin(); it != m_entries.end(); ++it) { - if (!it->recallable - && it->importSource.startsWith(QLatin1String("icom:")) - && MemoryFields::isKnownMode(it->mode)) { - it->recallable = true; - ++repairedRecallability; - } - } - if (repairedRecallability > 0) { - m_dirty = true; - flush(); - qCInfo(lcProtocol).noquote() - << "LocalMemoryBank: repaired recallability for" - << repairedRecallability << "Icom-imported memories"; - } + // Loading never guesses recallability or upgrades an existing document. + // Experimental Icom imports did not preserve split/RPS metadata, so only + // an explicit Sync with the corrected codec can safely repair those rows. if (importedFromLegacy) { // Claim the legacy channels into the document now, so the migration @@ -378,7 +358,7 @@ void LocalMemoryBank::flush() if (!AppSettings::instance().setRadioFeature( LocalMemoryStore::documentFamily(), QString(), LocalMemoryStore::documentFeature(), - LocalMemoryStore::kFormatVersion, envelope)) { + LocalMemoryStore::formatVersionFor(m_entries), envelope)) { m_lastError = QStringLiteral("the settings store refused the write"); qCWarning(lcProtocol).noquote() << "LocalMemoryBank: save failed —" << m_lastError; diff --git a/src/core/LocalMemoryStore.cpp b/src/core/LocalMemoryStore.cpp index 3c25bb83a..14d59045c 100644 --- a/src/core/LocalMemoryStore.cpp +++ b/src/core/LocalMemoryStore.cpp @@ -89,12 +89,26 @@ MemoryEntry entryFromJson(const QJsonObject& o) } // namespace +int LocalMemoryStore::formatVersionFor(const QMap& memories) +{ + for (const MemoryEntry& memory : memories) { + if (!memory.channel.isEmpty() || !memory.importSource.isEmpty() + || !memory.importKey.isEmpty() || memory.nativeFilter != 0 + || memory.dataMode != 0 || memory.rxToneValue != 0.0 + || memory.dtcsCode != 23 || memory.dtcsTxReverse + || memory.dtcsRxReverse || !memory.recallable) { + return kFormatVersion; + } + } + return 1; +} + QByteArray LocalMemoryStore::serialize(const QMap& memories, const QString& savedAtIso) { QJsonObject root; root["format"] = kFormatId; - root["version"] = kFormatVersion; + root["version"] = formatVersionFor(memories); if (!savedAtIso.isEmpty()) root["savedAt"] = savedAtIso; root["savedBy"] = "AetherSDR"; diff --git a/src/core/LocalMemoryStore.h b/src/core/LocalMemoryStore.h index 3dce5f77c..c107a4209 100644 --- a/src/core/LocalMemoryStore.h +++ b/src/core/LocalMemoryStore.h @@ -17,7 +17,7 @@ namespace AetherSDR { // Envelope: // { // "format": "aether.memories", -// "version": 2, +// "version": 1 or 2, // "savedAt": "2026-07-29T14:00:00Z", // "savedBy": "AetherSDR", // "memories": [ { "index": 0, ...MemoryEntry... } ] @@ -36,6 +36,9 @@ namespace AetherSDR { class LocalMemoryStore { public: static constexpr int kFormatVersion = 2; + // Keep ordinary client memories readable by version-1 builds. Imported + // recall state needs version 2: older writers would drop safety metadata. + static int formatVersionFor(const QMap& memories); static constexpr const char* kFormatId = "aether.memories"; // The bank's home since RFC #4603 PR 6: ONE shared feature document in diff --git a/src/core/backends/icom/IcomMemoryCodec.cpp b/src/core/backends/icom/IcomMemoryCodec.cpp index 6eb5e2e50..8ae91be20 100644 --- a/src/core/backends/icom/IcomMemoryCodec.cpp +++ b/src/core/backends/icom/IcomMemoryCodec.cpp @@ -223,12 +223,11 @@ std::optional decodeMemory( } memory.split = memory.split || ((selectByte >> 4) != 0); } - // AetherSDR recalls the RX side of a stored channel onto the active slice. - // Split/RPS metadata is useful provenance, but it must not make a memory - // impossible to navigate to: the local database already has a complete, - // neutral RX frequency and mode. Modes with no neutral representation - // (currently DV/DD) remain display-only. - memory.recallable = mode.mode.has_value(); + // Neutral recall cannot represent the second frequency block or RPS. + // Keep those records visible but non-recallable until their complete + // semantics are implemented. A fixed-length IC-7300MK2 reply alone is + // not split: the explicit flag above remains authoritative. + memory.recallable = mode.mode.has_value() && !memory.split && duplex != 3; if (layout.dtcsOffset >= 0) { const std::optional dtcs = decodeRepeaterToneRegister( diff --git a/src/models/RadioModel.cpp b/src/models/RadioModel.cpp index d4a383694..ce5f05d2a 100644 --- a/src/models/RadioModel.cpp +++ b/src/models/RadioModel.cpp @@ -8151,12 +8151,19 @@ void RadioModel::applyMemoryChanges(const MemoryDelta& d) // find the row previously imported from that radio/channel, or allocate a // new client slot. This keeps manual/CSV memories visible and prevents a // radio's channel 1 from overwriting the operator's client slot 1. + const QString importSource = MemoryFields::sanitizeText(d.importSource.value_or(QString())); + const QString importKey = MemoryFields::sanitizeText(d.importKey.value_or(QString())); + if ((d.importSource || d.importKey) && (importSource.isEmpty() || importKey.isEmpty())) { + qCWarning(lcProtocol) << "RadioModel: refused incomplete memory import identity"; + return; + } int targetIndex = d.index; - const bool isImport = d.importSource && !d.importSource->isEmpty() - && d.importKey && !d.importKey->isEmpty(); + const bool isImport = !importSource.isEmpty() && !importKey.isEmpty(); + bool preserveAnnotations = false; if (isImport) { m_localMemories.load(); - targetIndex = m_localMemories.importedSlot(*d.importSource, *d.importKey); + targetIndex = m_localMemories.importedSlot(importSource, importKey); + preserveAnnotations = targetIndex >= 0; if (d.removed) { if (targetIndex >= 0) { @@ -8190,6 +8197,10 @@ void RadioModel::applyMemoryChanges(const MemoryDelta& d) } auto& m = m_memories[targetIndex]; + if (preserveAnnotations) { + // A fresh session may not have published its local cache yet. + m = m_localMemories.entries().value(targetIndex); + } m.index = targetIndex; // Decode the protocol space-encoding (0x7f -> ' ') for free-text fields, @@ -8204,12 +8215,16 @@ void RadioModel::applyMemoryChanges(const MemoryDelta& d) return AetherSDR::MemoryFields::sanitizeText(v); }; - if (d.group) m.group = decodeText(*d.group); - if (d.owner) m.owner = decodeText(*d.owner); + // Sync refreshes tuning state; the operator owns these annotations after + // the first insert, including deliberately empty names/groups/owners. + if (!preserveAnnotations) { + if (d.group) { m.group = decodeText(*d.group); } + if (d.owner) { m.owner = decodeText(*d.owner); } + if (d.name) { m.name = decodeText(*d.name); } + } if (d.channel) m.channel = decodeText(*d.channel); - if (d.importSource) m.importSource = sanitize(*d.importSource); - if (d.importKey) m.importKey = sanitize(*d.importKey); - if (d.name) m.name = decodeText(*d.name); + if (d.importSource) m.importSource = importSource; + if (d.importKey) m.importKey = importKey; if (d.mode) m.mode = sanitize(*d.mode); if (d.offsetDir) m.offsetDir = sanitize(*d.offsetDir); if (d.toneMode) m.toneMode = sanitize(*d.toneMode); diff --git a/tests/icom_memory_test.cpp b/tests/icom_memory_test.cpp index b468a5c64..b3c9d091c 100644 --- a/tests/icom_memory_test.cpp +++ b/tests/icom_memory_test.cpp @@ -108,8 +108,15 @@ int main() ic7300SplitRecord[2] = 0x10; const auto m7300Split = decodeMemory( MemoryDialect::Ic7300Mk2, ic7300SplitRecord); - check(m7300Split && m7300Split->split && m7300Split->recallable, - "IC-7300MK2 split memory can recall its neutral RX side"); + check(m7300Split && m7300Split->split && !m7300Split->recallable, + "IC-7300MK2 split memory remains display-only"); + + auto cwRecord = ic7300Record(); + cwRecord[8] = 0x07; + cwRecord[10] = 0x00; + const auto cw = decodeMemory(MemoryDialect::Ic7300Mk2, cwRecord); + check(cw && cw->mode == "CWL" && cw->recallable && !cw->split, + "ordinary 47-byte CW-R memory is recallable on explicit sync"); for (int toneMode = 0; toneMode <= 3; ++toneMode) { auto record = ic9700Record(); @@ -125,10 +132,10 @@ int main() auto reverseSplit = ic9700Record(); reverseSplit[12] = 0x31; const auto rps = decodeMemory(MemoryDialect::Ic9700, reverseSplit); - check(rps && rps->recallable, "IC-9700 RPS can recall its neutral RX side"); + check(rps && !rps->recallable, "IC-9700 RPS remains display-only"); const auto split = decodeMemory(MemoryDialect::Ic9700, ic9700Record(114)); - check(split && split->split && split->recallable, - "split records can recall their neutral RX side"); + check(split && split->split && !split->recallable, + "split records remain display-only"); auto dvRecord = ic705Record(); dvRecord[10] = 0x17; dvRecord[11] = 0x00; const auto dv = decodeMemory(MemoryDialect::Ic705, dvRecord); diff --git a/tests/local_memory_bank_test.cpp b/tests/local_memory_bank_test.cpp index 4e1273739..a9f0809ea 100644 --- a/tests/local_memory_bank_test.cpp +++ b/tests/local_memory_bank_test.cpp @@ -147,9 +147,9 @@ int main(int argc, char** argv) imported.mode = QStringLiteral("DIGU"); imported.nativeFilter = 2; imported.dataMode = 1; - // Reproduce the poisoned flag written by the first IC-7300MK2 import - // implementation. Reopening the bank must repair it without requiring - // another live-radio sync. + // The bank cannot distinguish a poisoned experimental import from a + // deliberately blocked split record. Load must preserve the codec's + // decision; an explicit Sync is required to reclassify it. imported.recallable = false; bank.record(0, imported); @@ -161,6 +161,7 @@ int main(int argc, char** argv) "a different native channel does not alias"); bank.flush(); + const QJsonObject beforeLoad = bankDocument(); LocalMemoryBank reopened; reopened.setFilePath(dir.path() + "/unused-imports.json"); reopened.load(); @@ -170,16 +171,37 @@ int main(int argc, char** argv) ok &= expect(stored.channel == "42" && stored.nativeFilter == 2 && stored.dataMode == 1, "radio recall fields survive a database reopen"); - ok &= expect(stored.recallable, - "stale Icom display-only metadata is repaired on reopen"); - ok &= expect(bankDocument() - .value(QStringLiteral("memories")) - .toArray() - .at(0) - .toObject() - .value(QStringLiteral("recallable")) - .toBool(), - "the repaired Icom recallability is persisted"); + ok &= expect(!stored.recallable, + "load preserves a display-only import instead of guessing"); + ok &= expect(bankDocument() == beforeLoad, + "loading existing imports does not write or upgrade the document"); + } + + // Opening a version-1 database is not a schema migration. Ordinary local + // edits also keep version 1; only the new recall fields require version 2. + { + resetBankDocument(); + QJsonObject legacy{{"format", "aether.memories"}, {"version", 1}, + {"savedAt", "2026-08-01T00:00:00Z"}, + {"memories", QJsonArray{QJsonObject{{"index", 0}, + {"freq", 14.074}, {"mode", "DIGU"}}}}}; + ok &= expect(AppSettings::instance().setRadioFeature( + LocalMemoryStore::documentFamily(), QString(), + LocalMemoryStore::documentFeature(), 1, legacy), + "version-1 database fixture saved"); + LocalMemoryBank bank; + bank.load(); + ok &= expect(bankDocument() == legacy, "load leaves version-1 bytes unchanged"); + MemoryEntry edited = bank.entries().value(0); + edited.name = "Local annotation"; + bank.record(0, edited); + bank.flush(); + int rowVersion = 0; + const QJsonObject saved = AppSettings::instance().radioFeatureExact( + LocalMemoryStore::documentFamily(), QString(), + LocalMemoryStore::documentFeature(), &rowVersion); + ok &= expect(rowVersion == 1 && saved.value("version").toInt() == 1, + "ordinary local edit keeps row and envelope at version 1"); } // --- rejections ------------------------------------------------------- diff --git a/tests/local_memory_store_test.cpp b/tests/local_memory_store_test.cpp index 16edbaf77..5662694e6 100644 --- a/tests/local_memory_store_test.cpp +++ b/tests/local_memory_store_test.cpp @@ -80,6 +80,24 @@ int main() ok &= expect(g == sampleMemory(7), "every MemoryEntry field preserved"); } + // A version bump is needed only for fields older writers cannot preserve. + { + MemoryEntry ordinary; + ordinary.index = 0; + ordinary.mode = "USB"; + ordinary.freq = 14.25; + QMap bank{{0, ordinary}}; + ok &= expect(LocalMemoryStore::parse(LocalMemoryStore::serialize(bank)).version == 1, + "ordinary memories keep the version-1 format"); + bank[0].recallable = false; + ok &= expect(LocalMemoryStore::parse(LocalMemoryStore::serialize(bank)).version == 2, + "display-only safety metadata requires the protected version-2 format"); + bank[0] = ordinary; + bank[0].nativeFilter = 2; + ok &= expect(LocalMemoryStore::formatVersionFor(bank) == 2, + "native recall metadata requires version 2 even without provenance"); + } + // --- the map key wins over a disagreeing index field ------------------ { QMap memories; diff --git a/tests/memory_import_test.cpp b/tests/memory_import_test.cpp new file mode 100644 index 000000000..9f5c1683f --- /dev/null +++ b/tests/memory_import_test.cpp @@ -0,0 +1,128 @@ +#include "TestSettingsProfile.h" +#include "core/AppSettings.h" +#include "core/LocalMemoryStore.h" +#include "core/backends/IRadioBackend.h" +#include "models/RadioModel.h" + +#include +#include +#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) +{ + TestSettingsProfile profile(QStringLiteral("memory-import-test")); + if (!profile.isValid()) { + return 1; + } + QCoreApplication app(argc, argv); + AppSettings::instance().load(); + // Seed an exact empty bank so no host-side legacy import is consulted. + const QJsonObject empty = QJsonDocument::fromJson(LocalMemoryStore::serialize({})).object(); + check(AppSettings::instance().setRadioFeature("local", "", "MemoryBank", 1, empty), + "isolated bank seeded"); + + MemoryDelta imported; + imported.index = 0; // Deliberately collides with the manual client slot. + imported.importSource = QStringLiteral("icom:radio-a"); + imported.importKey = QStringLiteral("-1:42"); + imported.name = QStringLiteral("Native name"); + imported.owner = QStringLiteral("IC-7300MK2"); + imported.group = QStringLiteral("Native group"); + imported.mode = QStringLiteral("DIGU"); + imported.freq = 14.074; + imported.nativeFilter = 2; + imported.dataMode = 1; + imported.recallable = false; + int importedIndex = -1; + { + RadioModel model; + // Inject the real backend signal. No connectRadio, sockets or peer. + model.sendCmdPublic("memory create", {}); + model.sendCmdPublic("memory set 0 name=Manual freq=7.2 mode=LSB", {}); + model.backend()->memoryChanged(imported); + check(model.memories().size() == 2 && model.memories().value(0).name == "Manual", + "native slot collision allocates a new row and preserves the manual row"); + for (const MemoryEntry& memory : model.memories()) { + if (memory.importSource == *imported.importSource) { + importedIndex = memory.index; + } + } + check(importedIndex == 1, "import uses the lowest free client slot"); + model.sendCmdPublic(QString("memory set %1 name=Operator owner=Trustee group=Local") + .arg(importedIndex), {}); + // Include stripped control bytes: lookup and persistence must normalize + // identically, or the next Sync allocates a duplicate forever. + imported.importSource->append(QChar(1)); + imported.importKey->append(QChar(0x7f)); + imported.freq = 14.25; + imported.mode = QStringLiteral("CWL"); + imported.dataMode = 0; + imported.recallable = true; + model.backend()->memoryChanged(imported); + const MemoryEntry updated = model.memories().value(importedIndex); + check(model.memories().size() == 2 && updated.freq == 14.25, + "resync updates the existing row instead of duplicating it"); + check(updated.name == "Operator" && updated.owner == "Trustee" && updated.group == "Local", + "resync preserves operator annotations"); + check(updated.mode == "CWL" && updated.recallable, + "explicit sync repairs an ordinary CW-R row using the codec decision"); + check(updated.importSource == "icom:radio-a" && updated.importKey == "-1:42", + "stored provenance matches the normalized lookup"); + + MemoryDelta invalid = imported; + invalid.importSource = QString(QChar(1)); + invalid.index = 0; + invalid.removed = true; + model.backend()->memoryChanged(invalid); + check(model.memories().contains(0) && model.memories().size() == 2, + "empty normalized identity cannot fall through to a native-slot removal"); + + MemoryDelta other = imported; + other.importSource = QStringLiteral("icom:radio-b"); + model.backend()->memoryChanged(other); + check(model.memories().size() == 3, + "same native channel on another radio gets a separate slot"); + other.removed = true; + model.backend()->memoryChanged(other); + model.backend()->memoryChanged(other); + check(model.memories().size() == 2 && model.memories().contains(importedIndex), + "empty channel removes only its matching source and is idempotent"); + } + { + // Resync BEFORE the constructor's queued local-bank publication: the + // authoritative persisted annotations must survive an empty GUI cache. + RadioModel reopened; + imported.freq = 14.3; + reopened.backend()->memoryChanged(imported); + check(reopened.memories().value(importedIndex).name == "Operator" + && reopened.memories().value(importedIndex).freq == 14.3, + "resync after restart reuses persisted identity and annotations"); + imported.removed = true; + reopened.backend()->memoryChanged(imported); + check(!reopened.memories().contains(importedIndex), + "cleared native channel removes the imported cache row"); + } + const QJsonObject saved = AppSettings::instance().radioFeatureExact("local", "", "MemoryBank"); + const auto parsed = LocalMemoryStore::parse(QJsonDocument(saved).toJson()); + check(parsed.memories.size() == 1 && parsed.memories.value(0).name == "Manual", + "deletion persists and manual row survives restart"); + if (failures == 0) { + std::printf("memory_import_test: all checks passed\n"); + } + return failures == 0 ? 0 : 1; +} diff --git a/tests/tests.cmake b/tests/tests.cmake index 553f13dd8..ef13afdf0 100644 --- a/tests/tests.cmake +++ b/tests/tests.cmake @@ -3642,7 +3642,6 @@ add_executable(local_memory_bank_test tests/local_memory_bank_test.cpp src/core/LocalMemoryBank.cpp src/core/LocalMemoryStore.cpp - src/core/MemoryFieldValues.cpp src/core/backends/MemoryWireCodec.cpp src/core/AsyncLogWriter.cpp src/core/LogManager.cpp @@ -3652,6 +3651,12 @@ target_include_directories(local_memory_bank_test PRIVATE src) target_link_libraries(local_memory_bank_test PRIVATE Qt6::Core) add_test(NAME local_memory_bank_test COMMAND local_memory_bank_test) +# Socket-free injection of backend memory deltas; no radio connection or peer. +add_executable(memory_import_test tests/memory_import_test.cpp) +target_include_directories(memory_import_test PRIVATE src tests) +target_link_libraries(memory_import_test PRIVATE aethercore Qt6::Core) +add_test(NAME memory_import_test COMMAND memory_import_test) + add_executable(memory_csv_compat_test tests/memory_csv_compat_test.cpp src/core/MemoryCsvCompat.cpp @@ -4354,6 +4359,7 @@ set(AETHER_SETTINGS_CONSUMERS meter_applet_voltage_state_test perf_telemetry_test local_memory_bank_test + memory_import_test transmit_model_apd_test help_dialog_test flex_control_dialog_size_test From 8077d6b97ea7f1f203b1393ab74e23ea80bb8cab Mon Sep 17 00:00:00 2001 From: Robbie Foust Date: Fri, 4 Sep 2026 23:46:38 -0400 Subject: [PATCH 5/5] Refresh memory capability touchpoint manifest. Principle VIII. --- docs/architecture/aetherd-touchpoints.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/aetherd-touchpoints.md b/docs/architecture/aetherd-touchpoints.md index 3bca9f927..01194be3d 100644 --- a/docs/architecture/aetherd-touchpoints.md +++ b/docs/architecture/aetherd-touchpoints.md @@ -167,7 +167,7 @@ Burndown manifest for the engine/UI decoupling ([RFC](../aetherd-headless-engine | `core/aprs/AprsStationList.h` | 1 | universal — Heard-APRS-station model (calls/positions/last-heard); radio-agnostic spot-like data. | unconverted | | `core/backends/ConnectionSharingPolicy.h` | 2 | universal — Fail-closed discovery-time policy for whether a busy radio family permits another client. Canonical cross-family connection safety pending capability descriptors. | unconverted | | `core/backends/IRadioBackend.h` | 1 | universal — THE radio seam (RFC §5.5) — the canonical intent verbs, typed deltas and normalized signals every family implements. Universal by definition: the UI reaching this header is the seam working as designed, not coupling. Everything below it in core/backends// is family-private. | unconverted | -| `core/backends/RadioCapabilities.h` | 2 | universal — Backend-neutral capability descriptor consumed above the radio seam. Universal by definition; family implementations populate it. | unconverted | +| `core/backends/RadioCapabilities.h` | 3 | universal — Backend-neutral capability descriptor consumed above the radio seam. Universal by definition; family implementations populate it. | unconverted | | `core/backends/anan/AnanDiscovery.h` | 2 | vendor(anan) — openHPSDR Protocol 2 discovery and ANAN-G2 identity handling. Family-specific discovery belongs below the ANAN backend seam. | unconverted | | `core/backends/anan/AnanSettings.h` | 1 | ui-support — Client-side connection and ADC configuration for the ANAN backend. Persisted setup plumbing, not live radio state. | unconverted | | `core/backends/anan/P2Protocol.h` | 1 | vendor(anan) — openHPSDR Ethernet Protocol 2 wire primitives for the ANAN-G2 backend. Family protocol code belongs below the radio seam. | unconverted |