diff --git a/src/helpers/CommonCLI_Management.cpp b/src/helpers/CommonCLI_Management.cpp index ab93508af..dd957255f 100644 --- a/src/helpers/CommonCLI_Management.cpp +++ b/src/helpers/CommonCLI_Management.cpp @@ -1,6 +1,7 @@ #include "CommonCLI.h" #include "ManagementReporter.h" #include "FileRead.h" +#include "FilePresence.h" #include "PersistentStoreFormat.h" #include "HilStartupTrace.h" #include @@ -138,19 +139,35 @@ static bool dataRouteSave(DataRouteState& route) { #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) AtomicFileWriter writer(route.fs, DATA_ROUTE_FILE); #else - ContactFileTransaction writer(route.fs, DATA_ROUTE_FILE); + ContactFileTransaction writer(route.fs, DATA_ROUTE_FILE, filePresence); #endif - const bool ok = writer && writer.write(data, sizeof(data)) == sizeof(data) + bool ok = writer && writer.write(data, sizeof(data)) == sizeof(data) && writer.commit(); + if (writer.outcome() == decltype(writer)::Outcome::Indeterminate) route.healthy = false; +#if defined(ESP32_PLATFORM) || defined(RP2040_PLATFORM) + if (ok || writer.outcome() == ContactFileTransaction::Outcome::Indeterminate) { + // A moved stage alone does not confirm the public bytes. Never resume the + // old RAM route when the new durable authority cannot be established. + auto file = openFileRead(route.fs, DATA_ROUTE_FILE); + uint8_t verified[DATA_ROUTE_SIZE]; + ok = file && !file.isDirectory() && file.size() == sizeof(verified) + && file.read(verified, sizeof(verified)) == (int)sizeof(verified) + && memcmp(data, verified, sizeof(data)) == 0; + file.close(); + route.healthy = ok; + } +#endif if (ok) route.persisted = true; return ok; } static bool dataRouteLoad(DataRouteState& route) { #if defined(ESP32_PLATFORM) || defined(RP2040_PLATFORM) - if (!ContactFileTransaction::recover(route.fs, DATA_ROUTE_FILE)) return false; + if (!ContactFileTransaction::recover(route.fs, DATA_ROUTE_FILE, filePresence)) return false; #endif - if (!route.fs->exists(DATA_ROUTE_FILE)) return true; + bool present = false; + if (!filePresence(route.fs, DATA_ROUTE_FILE, present)) return false; + if (!present) return true; uint8_t data[DATA_ROUTE_SIZE] = {}; for (unsigned retry = 0; retry < 3; ++retry) { auto file = openFileRead(route.fs, DATA_ROUTE_FILE); @@ -401,7 +418,9 @@ bool CommonCLI::handleDataTxCommand(char* command, char* reply) { } } if (!mesh::dataRouteSave(*_data_route)) { - *_data_route = previous; strcpy(reply, "Err - unable to save data.tx"); return true; + const bool healthy = _data_route->healthy; + *_data_route = previous; _data_route->healthy = healthy; + strcpy(reply, "Err - unable to save data.tx"); return true; } strcpy(reply, "OK"); return true; } diff --git a/test/fixtures/management/route_test.cpp b/test/fixtures/management/route_test.cpp index 7ad5ba543..6a7f5dc97 100644 --- a/test/fixtures/management/route_test.cpp +++ b/test/fixtures/management/route_test.cpp @@ -1,6 +1,8 @@ #include "IdentityStore.h" #include #include "ContactFileTransaction.h" +#include "FilePresence.h" +#include "FileRead.h" #include "PersistentStoreFormat.h" #include #include diff --git a/test/fixtures/telemetry_tx_persistence/IdentityStore.h b/test/fixtures/telemetry_tx_persistence/IdentityStore.h index 0db2a9235..f0e7eaab7 100644 --- a/test/fixtures/telemetry_tx_persistence/IdentityStore.h +++ b/test/fixtures/telemetry_tx_persistence/IdentityStore.h @@ -40,6 +40,8 @@ public: bool fail_metadata = false, fail_final_read = false; int final_read_limit = -1; bool corrupt_publish = false, fail_metadata_after_publish = false; + std::string publication_path = "/telemetry_tx"; + bool no_op_publish = false, late_publish_error = false; unsigned writes = 0, renames = 0; #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) bool rename_replaces = true; @@ -54,12 +56,14 @@ public: ++renames; if (fail_rename.count({from, to}) || !exists(from) || (!rename_replaces && exists(to))) return false; + if (from == publication_path + ".tmp" && no_op_publish) return true; files[to] = files[from]; files.erase(from); - if (!strcmp(from, "/telemetry_tx.tmp")) { + if (from == publication_path + ".tmp") { if (corrupt_publish) files[to][0] ^= 1; if (fail_final_read) fail_read_open.insert(to); if (final_read_limit >= 0) read_limit[to] = size_t(final_read_limit); if (fail_metadata_after_publish) fail_metadata = true; + if (late_publish_error) return false; } return true; } @@ -82,6 +86,7 @@ public: fail_metadata = fail_final_read = corrupt_publish = false; final_read_limit = -1; fail_metadata_after_publish = false; + no_op_publish = late_publish_error = false; } }; diff --git a/test/fixtures/telemetry_tx_persistence/fixture.cpp b/test/fixtures/telemetry_tx_persistence/fixture.cpp index 7dc59c7f9..36bf5fc66 100644 --- a/test/fixtures/telemetry_tx_persistence/fixture.cpp +++ b/test/fixtures/telemetry_tx_persistence/fixture.cpp @@ -45,12 +45,33 @@ struct Packet { struct Utils { static void toHex(char*, const uint8_t*, size_t); }; struct DataRouteState; } -class RegionMap; +struct TransportKey {}; +struct RegionEntry { const char* name; }; +class RegionMap { +public: + RegionEntry* findByNamePrefix(const char*) { return nullptr; } +}; +namespace mesh { +// Region selection is a boundary here; route storage and command handling are real. +static bool usableScope(const DataRouteState&, RegionEntry*, TransportKey&) { return false; } +} class CommonCLI { public: mesh::DataRouteState* _data_route = nullptr; + RegionMap* _region_map = nullptr; bool adoptLegacyDataTxPath(const uint8_t*, uint8_t); bool getDataTxPath(const uint8_t*&, uint8_t&) const; + bool resolveDataTxScope(TransportKey&, char*, size_t, bool* ambiguous = nullptr) const { + if (ambiguous) *ambiguous = false; + return false; + } + bool handleDataTxCommand(char*, char*); + std::string routeCommand(const char* text) { + std::vector command(text, text + strlen(text) + 1); + char reply[160] = {}; + assert(handleDataTxCommand(command.data(), reply)); + return reply; + } }; struct ClientInfo { bool admin = true; bool isAdmin() const { return admin; } }; class MyMesh { @@ -397,6 +418,119 @@ static void checkAmbiguousPublicationAndServiceGate() { assert(healthy.node.command("send telemetry.tx now").find("OK - telemetry.tx queued") == 0); } +static void checkRoutePublicationAuthority() { + for (const bool migration : {false, true}) { + Fixture f; f.boot(); + if (!migration) assert(f.node._cli.routeCommand("set data.tx path 12") == "OK"); + f.fs.publication_path = "/data_tx"; +#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) + // Atomic publication succeeds, but its mandatory public readback fails. + f.fs.fail_final_read = true; +#else + // The old image is backed up but neither publication nor rollback happens. + if (migration) assert(mesh::dataRouteSave(f.route)); + f.fs.fail_rename.insert({"/data_tx.tmp", "/data_tx"}); + f.fs.fail_rename.insert({"/data_tx.bak", "/data_tx"}); +#endif + if (migration) { + const uint8_t path = 0x34; + // Force the legacy adoption path without discarding the existing disk image. + f.route.persisted = false; + assert(!f.node._cli.adoptLegacyDataTxPath(&path, 1)); + } else { + assert(f.node._cli.routeCommand("set data.tx path 34") == "Err - unable to save data.tx"); + } + // A reported failure cannot authorize old RAM when disk authority is unknown. + assert(!f.route.healthy); + const uint8_t* path = nullptr; uint8_t length = 0; + assert(!f.node._cli.getDataTxPath(path, length)); + assert(f.node._cli.routeCommand("get data.tx") == "Err - data.tx storage fault"); + const auto writes = f.fs.writes; + assert(f.node._cli.routeCommand("set data.tx path 56") == "Err - data.tx storage fault"); + assert(f.fs.writes == writes); + f.fs.clearFaults(); f.boot(); + assert(f.route.healthy); +#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) + assert(f.route.path_len == 1 && f.route.path[0] == 0x34); +#else + assert(f.route.path_len == (migration ? 0 : 1)); + if (!migration) assert(f.route.path[0] == 0x12); +#endif + } +} + +static void checkRouteReadbackAndRetry() { + for (const bool region : {false, true}) { + const char* command = region ? "set data.tx region none" : "set data.tx path 34"; + for (const int failure : {0, 1, 2, 3, 4, 5}) { + Fixture f; f.boot(); + assert(f.node._cli.routeCommand("set data.tx path 12") == "OK"); + const auto previous = f.fs.files.at("/data_tx"); + f.fs.publication_path = "/data_tx"; + if (failure == 0) f.fs.fail_write_open.insert("/data_tx.tmp"); + if (failure == 1) f.fs.write_limit["/data_tx.tmp"] = 107; + if (failure == 2) f.fs.fail_read_open.insert("/data_tx.tmp"); + if (failure == 3) f.fs.fail_final_read = true; + if (failure == 4) f.fs.final_read_limit = 107; + if (failure == 5) f.fs.corrupt_publish = true; + assert(f.node._cli.routeCommand(command) == "Err - unable to save data.tx"); + assert(f.route.healthy == (failure < 3)); + assert(f.route.path_len == 1 && f.route.path[0] == 0x12); + assert(f.route.region_mode == mesh::DATA_REGION_AUTO); + if (failure < 3) { + assert(f.fs.files.at("/data_tx") == previous); + f.fs.clearFaults(); + assert(f.node._cli.routeCommand(command) == "OK"); + f.boot(); assert(f.route.healthy); + } else { + const auto writes = f.fs.writes; + const uint8_t* path = nullptr; uint8_t length = 0; + assert(!f.node._cli.getDataTxPath(path, length)); + assert(f.node._cli.routeCommand(command) == "Err - data.tx storage fault"); + assert(f.fs.writes == writes); + f.fs.clearFaults(); f.boot(); + assert(f.route.healthy == (failure != 5)); + } + } + } + for (const bool existing : {false, true}) { + for (const bool no_op : {false, true}) { + Fixture f; f.boot(); + if (existing) assert(f.node._cli.routeCommand("set data.tx path 12") == "OK"); + f.fs.publication_path = "/data_tx"; + f.fs.no_op_publish = no_op; + f.fs.late_publish_error = !no_op; + const auto reply = f.node._cli.routeCommand("set data.tx path 34"); + if (no_op) { + assert(reply == "Err - unable to save data.tx"); +#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) + assert(!f.route.healthy); // Atomic writer cannot prove old authority. +#else + assert(f.route.healthy); // Observed unchanged move + successful rollback. +#endif + } else { + assert(reply == "OK" && f.route.healthy && f.route.path[0] == 0x34); + } + f.fs.clearFaults(); f.boot(); assert(f.route.healthy); + if (!no_op) assert(f.route.path_len == 1 && f.route.path[0] == 0x34); + else assert(f.route.path_len == (existing ? 1 : 0)); + } + } +#if !defined(RP2040_PLATFORM) + for (const bool existing : {false, true}) { + Fixture f; f.boot(); + if (existing) assert(f.node._cli.routeCommand("set data.tx path 12") == "OK"); + const auto files = f.fs.files; + f.fs.fail_metadata = true; f.boot(); + assert(!f.route.healthy && f.fs.files == files); + const auto writes = f.fs.writes; + assert(f.node._cli.routeCommand("set data.tx path 34") == "Err - data.tx storage fault"); + assert(f.fs.writes == writes); + f.fs.clearFaults(); f.boot(); assert(f.route.healthy); + } +#endif +} + int main() { checkFreshAndValid(); checkLoadFailures(); @@ -404,5 +538,7 @@ int main() { checkWriteAndValidationFailures(); checkRecoveryAndRenameFailures(); checkAmbiguousPublicationAndServiceGate(); + checkRoutePublicationAuthority(); + checkRouteReadbackAndRetry(); puts("telemetry persistence checks passed"); } diff --git a/test/test_management_report.py b/test/test_management_report.py index 7a2bb24df..75139c7fc 100644 --- a/test/test_management_report.py +++ b/test/test_management_report.py @@ -74,7 +74,8 @@ class ManagementTests(unittest.TestCase): # Compile the unchanged production runtime and transaction writer with # a memory filesystem/radio in place of the hardware-facing headers. fixture = ROOT / "test/fixtures/management" - for name in ("ManagementReporter.cpp", "ManagementReporter.h", "FileRead.h", "ContactFileTransaction.h", "PersistentStoreFormat.h"): + for name in ("ManagementReporter.cpp", "ManagementReporter.h", "FileRead.h", "FilePresence.h", + "ContactFileTransaction.h", "PersistentStoreFormat.h"): shutil.copyfile(ROOT / "src/helpers" / name, Path(cls.work.name) / name) for name in ("CommonCLI.h", "Mesh.h"): shutil.copyfile(fixture / name, Path(cls.work.name) / name) diff --git a/test/test_telemetry_tx_persistence.py b/test/test_telemetry_tx_persistence.py index 45f26b7a5..8e4019d79 100644 --- a/test/test_telemetry_tx_persistence.py +++ b/test/test_telemetry_tx_persistence.py @@ -30,9 +30,12 @@ class TelemetryPersistenceTests(unittest.TestCase): generated += extract_braced(utils, "void Utils::toHex(") + "\n" generated += "\n".join(extract_braced(common, signature) for signature in ( "static uint32_t readRoute32(", "static void writeRoute32(", + "static uint8_t nibble(", "static char* trimDataRoute(", + "static bool parseDataPath(", "static void formatDataPath(", "static bool dataRouteSave(", "static bool dataRouteLoad(")) + "\n}\n" generated += "\n".join(extract_braced(common, signature) for signature in ( - "bool CommonCLI::adoptLegacyDataTxPath(", "bool CommonCLI::getDataTxPath(")) + "\n" + "bool CommonCLI::adoptLegacyDataTxPath(", "bool CommonCLI::getDataTxPath(", + "bool CommonCLI::handleDataTxCommand(")) + "\n" generated += '#define TELEMETRY_HISTORY_TX_PREFS_FILE "/telemetry_tx"\n' for name in ("RETRY_MILLIS", "PACKET_SPACING_MILLIS", "DEFAULT_DAYS", "MAX_DAYS", "TEMPERATURE", "VOLTAGE", "EXTERNAL_VOLTAGE"):