diff --git a/src/Nrf52LoopStack.cpp b/src/Nrf52LoopStack.cpp index 51f27bd1..5eefe4b2 100644 --- a/src/Nrf52LoopStack.cpp +++ b/src/Nrf52LoopStack.cpp @@ -4,17 +4,6 @@ #include #include #include -#include - -namespace mesh { -namespace nrf52 { -static uint8_t ble_task_mask; - -void resetBleTaskStartup() { ble_task_mask = 0; } -bool bleTasksStarted() { return ble_task_mask == 3; } -} // namespace nrf52 -} // namespace mesh - #ifndef MESH_NRF52_LOOP_STACK_WORDS #define MESH_NRF52_LOOP_STACK_WORDS 2048 #endif @@ -44,15 +33,6 @@ extern "C" BaseType_t __wrap_xTaskCreate( } const BaseType_t result = __real_xTaskCreate( task_code, task_name, adjusted_depth, parameters, priority, created_task); - if (task_name != NULL) { - const uint8_t mask = strcmp(task_name, "BLE") == 0 ? 1 - : strcmp(task_name, "SOC") == 0 ? 2 : 0; - if (result == pdPASS) { - mesh::nrf52::ble_task_mask |= mask; - } else { - mesh::nrf52::ble_task_mask &= ~mask; - } - } return result; } diff --git a/src/helpers/RadioProfileCLI.cpp b/src/helpers/RadioProfileCLI.cpp index b76454a2..b7c5ce09 100644 --- a/src/helpers/RadioProfileCLI.cpp +++ b/src/helpers/RadioProfileCLI.cpp @@ -56,23 +56,34 @@ uint32_t remainingMillis(uint32_t end, uint32_t now) { } } -bool RadioProfileCLI::readImage(const char* path, uint8_t* bytes, size_t size) { +RadioProfileCLI::ImageReadResult RadioProfileCLI::readImage( + const char* path, uint8_t* bytes, size_t size) { + if (!fs_) return ImageReadResult::Unreadable; #if defined(NRF52_PLATFORM) File file(*fs_); - if (!file.open(path, FILE_O_READ)) return false; + if (!file.open(path, FILE_O_READ)) { + return fs_->exists(path) ? ImageReadResult::Unreadable + : ImageReadResult::Missing; + } #elif defined(STM32_PLATFORM) File file = fs_->open(path, FILE_O_READ); #else File file = fs_->open(path, "r"); #endif - if (!file) return false; - bool ok = file.size() == size && file.read(bytes, size) == (int)size; + if (!file) { + return fs_->exists(path) ? ImageReadResult::Unreadable + : ImageReadResult::Missing; + } + const bool right_size = file.size() == size; + const bool read_complete = right_size && file.read(bytes, size) == (int)size; file.close(); - if (!ok) return false; + if (!right_size) return ImageReadResult::Invalid; + if (!read_complete) return ImageReadResult::Unreadable; uint32_t stored; memcpy(&stored, bytes + size - 4, 4); return bytes[0] == 'R' && bytes[1] == '2' && bytes[2] == 1 - && stored == checksum(bytes, size - 4); + && stored == checksum(bytes, size - 4) + ? ImageReadResult::Valid : ImageReadResult::Invalid; } bool RadioProfileCLI::writeImage(const char* path, const uint8_t* bytes, size_t size) { @@ -90,13 +101,33 @@ bool RadioProfileCLI::writeImage(const char* path, const uint8_t* bytes, size_t file.flush(); file.close(); uint8_t verify[ImageSize]; - return ok && readImage(path, verify, size) && memcmp(bytes, verify, size) == 0; + return ok && readImage(path, verify, size) == ImageReadResult::Valid + && memcmp(bytes, verify, size) == 0; } bool RadioProfileCLI::save(const RadioProfileConfig& config, uint16_t preamble, RadioCrossMode cross) { + if (!discardCorruptSavedImagesForWrite()) return false; return prepareSavedImage(TempPath, config, preamble, cross) && commitSavedImage(TempPath); } +bool RadioProfileCLI::discardCorruptSavedImagesForWrite() { + if (!hold_) return true; + if (!fs_ || !recoverable_corrupt_store_) return false; + + // This is deliberately a user-triggered recovery, never a boot-time cleanup. + // The flag is set only after every present candidate was read completely and + // proved invalid, so no valid radio2 configuration is discarded. This store + // contains only radio-profile settings; node identity, contacts, and normal + // Companion preferences live elsewhere. + const char* const paths[] = {ImagePath, BackupPath}; + for (const char* path : paths) { + if (fs_->exists(path) && !fs_->remove(path)) return false; + } + hold_ = false; + recoverable_corrupt_store_ = false; + return true; +} + bool RadioProfileCLI::prepareSavedImage(const char* path, const RadioProfileConfig& config, uint16_t preamble, RadioCrossMode cross) { if (!fs_ || hold_) return false; @@ -129,6 +160,8 @@ bool RadioProfileCLI::commitSavedImage(const char* path) { void RadioProfileCLI::begin(FILESYSTEM* fs, Radio* radio, RTCClock* rtc, bool infrastructure_replies) { fs_ = fs; radio_ = radio; rtc_ = rtc; last_ms_ = millis(); + hold_ = false; + recoverable_corrupt_store_ = false; infrastructure_replies_ = infrastructure_replies; if (radio_ && radio_->profiles()) { radio_->profiles()->reply_tx = infrastructure_replies ? RADIO_TX_BOTH : RADIO_TX_AUTO; @@ -136,8 +169,11 @@ void RadioProfileCLI::begin(FILESYSTEM* fs, Radio* radio, RTCClock* rtc, bool in } if (!fs_ || !radio_ || !radio_->profiles()) return; uint8_t bytes[ImageSize]; - bool loaded = readImage(ImagePath, bytes, sizeof(bytes)); - if (!loaded && readImage(BackupPath, bytes, sizeof(bytes))) { + const ImageReadResult primary_image = readImage(ImagePath, bytes, sizeof(bytes)); + bool loaded = primary_image == ImageReadResult::Valid; + ImageReadResult backup_image = ImageReadResult::Missing; + if (!loaded) backup_image = readImage(BackupPath, bytes, sizeof(bytes)); + if (!loaded && backup_image == ImageReadResult::Valid) { // Use the verified backup even when repairing the interrupted save is // impossible. Keep writes held so the sole committed image stays intact. loaded = true; @@ -145,7 +181,15 @@ void RadioProfileCLI::begin(FILESYSTEM* fs, Radio* radio, RTCClock* rtc, bool in || !fs_->rename(BackupPath, ImagePath); } if (!loaded) { - hold_ = fs_->exists(ImagePath) || fs_->exists(BackupPath); + hold_ = primary_image != ImageReadResult::Missing + || backup_image != ImageReadResult::Missing; + // An unreadable file may be a transient filesystem fault. Do not remove + // it. A later explicit setter can recreate only files proven corrupt. + recoverable_corrupt_store_ = hold_ + && primary_image != ImageReadResult::Unreadable + && backup_image != ImageReadResult::Unreadable + && (primary_image == ImageReadResult::Invalid + || backup_image == ImageReadResult::Invalid); return; } saved_.mode = (RadioProfileMode)bytes[3]; @@ -159,7 +203,14 @@ void RadioProfileCLI::begin(FILESYSTEM* fs, Radio* radio, RTCClock* rtc, bool in if ((uint8_t)saved_.mode > 2 || (uint8_t)cross_ > 2 || (saved_.mode != RadioProfileMode::Off && !radio_->validateProfile(saved_.params)) || (primary_preamble_ && (primary_preamble_ < 8 || primary_preamble_ > RadioProfiles::MaxPreamble))) { + // A CRC-valid image can still be semantically stale (for example, after + // an older build wrote a now-unsupported value). Do not recreate it if a + // structurally valid backup exists; that backup is the last known-good + // radio-profile image and must remain available for manual recovery. + const ImageReadResult backup_image = readImage(BackupPath, bytes, sizeof(bytes)); hold_ = true; saved_ = {}; primary_preamble_ = 0; cross_ = RadioCrossMode::Auto; + recoverable_corrupt_store_ = backup_image != ImageReadResult::Valid + && backup_image != ImageReadResult::Unreadable; reply_setting_ = 0; // reject the entire image, including its reply override return; } diff --git a/src/helpers/RadioProfileCLI.h b/src/helpers/RadioProfileCLI.h index 16b28fa4..833d688d 100644 --- a/src/helpers/RadioProfileCLI.h +++ b/src/helpers/RadioProfileCLI.h @@ -36,7 +36,11 @@ class RadioProfileCLI { uint32_t schedule_retry_ms_ = 0; bool temp_pending_ = false, temp_active_ = false; bool hold_ = false; + // Set only after the active image is conclusively malformed and there is no + // readable backup. Read/I/O failures and an intact backup remain read-only. + bool recoverable_corrupt_store_ = false; bool publish_pending_ = false; + enum class ImageReadResult : uint8_t { Missing, Valid, Invalid, Unreadable }; enum class RemoteMutation : uint8_t { None, Saved, Temporary, Off, TempOff, DeleteTemp }; RemoteMutation remote_mutation_ = RemoteMutation::None; RadioProfileConfig remote_config_; @@ -51,8 +55,9 @@ class RadioProfileCLI { bool prepareSavedImage(const char* path, const RadioProfileConfig& config, uint16_t preamble, RadioCrossMode cross); bool commitSavedImage(const char* path); + bool discardCorruptSavedImagesForWrite(); bool applyReplyMutation(); - bool readImage(const char* path, uint8_t* bytes, size_t size); + ImageReadResult readImage(const char* path, uint8_t* bytes, size_t size); bool writeImage(const char* path, const uint8_t* bytes, size_t size); void publish(); void formatConfig(char* reply, size_t capacity, const RadioProfileConfig& config, diff --git a/src/helpers/nrf52/BleTaskStartup.h b/src/helpers/nrf52/BleTaskStartup.h deleted file mode 100644 index f40dea7d..00000000 --- a/src/helpers/nrf52/BleTaskStartup.h +++ /dev/null @@ -1,13 +0,0 @@ -#pragma once - -namespace mesh { -namespace nrf52 { - -// The pinned Bluefruit core does not check xTaskCreate's return value for its -// BLE and SOC workers. Nrf52LoopStack.cpp observes those calls through the -// existing linker wrapper so a nominally successful begin cannot hide OOM. -void resetBleTaskStartup(); -bool bleTasksStarted(); - -} // namespace nrf52 -} // namespace mesh diff --git a/src/helpers/nrf52/SerialBLEInterface.cpp b/src/helpers/nrf52/SerialBLEInterface.cpp index 01252e78..3c20dba6 100644 --- a/src/helpers/nrf52/SerialBLEInterface.cpp +++ b/src/helpers/nrf52/SerialBLEInterface.cpp @@ -1,5 +1,4 @@ #include "SerialBLEInterface.h" -#include "BleTaskStartup.h" #include "../BluetoothMac.h" #include "../CompanionFrameQueue.h" #include @@ -424,11 +423,15 @@ bool SerialBLEInterface::begin(const char* prefix, const char* name, // If we want to control BLE LED ourselves, uncomment this: // Bluefruit.autoConnLed(false); Bluefruit.configPrphBandwidth(BANDWIDTH_MAX); - mesh::nrf52::resetBleTaskStartup(); - if (!Bluefruit.begin() || !mesh::nrf52::bleTasksStarted()) { + // The pinned nRF52 core is built with LTO. Its internal xTaskCreate() calls + // are resolved before the application's linker wrappers, so a wrapper-based + // task observation incorrectly reports that the BLE/SOC workers never + // started. Bluefruit.begin() is the reliable startup result available to + // this application. + if (!Bluefruit.begin()) { instance = nullptr; mesh::usbLoggingPort().println( - "Bluetooth startup failed (SoftDevice/tasks); check runtime heap and reboot"); + "Bluetooth startup failed; reboot required"); return false; } diff --git a/test/fixtures/radio_profiles/cli_test.cpp b/test/fixtures/radio_profiles/cli_test.cpp index 73b55103..8a84ad8a 100644 --- a/test/fixtures/radio_profiles/cli_test.cpp +++ b/test/fixtures/radio_profiles/cli_test.cpp @@ -254,9 +254,16 @@ int main(int argc, char** argv) { restored.loop(); assert(radio.p.reply_tx==mesh::RADIO_TX_BOTH && !radio.p.reply_force); assert(!radio.p.enabled()); - assert(restored.handle("set tx.reply radio",f.reply)); - assert(strstr(f.reply,"Error")); - assert(f.fs.files["/radio_profiles"]==damaged); + assert(restored.handle("set radio2.cross on",f.reply)); + assert(!strcmp(f.reply,"OK")); + assert(radio.p.cross==mesh::RadioCrossMode::On); + assert(restored.handle("set tempradio2 911.3,500,8,7,rxtx,2",f.reply)); + assert(!strncmp(f.reply,"OK",2)); + g_mock_millis+=2000; restored.loop(); + assert(radio.p.secondary_temporary && radio.p.canCross()); + Radio reboot; mesh::RadioProfileCLI repaired; + repaired.begin(&f.fs,&reboot,&f.clock,true); + assert(reboot.p.cross==mesh::RadioCrossMode::On); } { Fixture companion; @@ -469,8 +476,51 @@ int main(int argc, char** argv) { Radio corrupt; mesh::RadioProfileCLI invalid; invalid.begin(&f.fs, &corrupt, &f.clock); assert(!corrupt.p.enabled()); - assert(invalid.handle("set radio2 off", f.reply)); - assert(strstr(f.reply,"Error")); // newer/corrupt image is not overwritten + const auto damaged=f.fs.files["/radio_profiles"]; + f.fs.fail_remove=true; + assert(invalid.handle("set radio2.cross on", f.reply)); + assert(strstr(f.reply,"Error")); // failed cleanup never overwrites corruption + assert(f.fs.files["/radio_profiles"]==damaged); + f.fs.fail_remove=false; + assert(invalid.handle("set radio2.cross on", f.reply)); + assert(!strcmp(f.reply,"OK")); + assert(corrupt.p.cross==mesh::RadioCrossMode::On); + assert(invalid.handle("set tempradio2 911.3,500,8,7,rxtx,2", f.reply)); + assert(!strncmp(f.reply,"OK",2)); + g_mock_millis+=2000; invalid.loop(); + assert(corrupt.p.secondary_temporary && corrupt.p.canCross()); + } + { + Fixture f; + f.cmd("set radio2 910.5,500,8,5,rxtx,80"); + const auto saved=f.fs.files["/radio_profiles"]; + f.fs.fail_read_open=true; + Radio radio; mesh::RadioProfileCLI unavailable; + unavailable.begin(&f.fs, &radio, &f.clock); + f.fs.fail_read_open=false; + assert(unavailable.handle("set radio2.cross on", f.reply)); + assert(strstr(f.reply,"Error")); // an I/O fault is never treated as corruption + assert(f.fs.files["/radio_profiles"]==saved); + } + { + Fixture f; + f.cmd("set radio2 910.5,500,8,5,rxtx,80"); + const auto backup=f.fs.files["/radio_profiles"]; + f.fs.files["/radio_profiles.bak"]=backup; + // This versioned image has a valid CRC but an unsupported saved mode. + // An intact backup must prevent automatic discard/recreation. + f.fs.files["/radio_profiles"][3]=3; + uint32_t crc=0xffffffffU; + for (size_t i=0;i<20;++i) { + crc ^= f.fs.files["/radio_profiles"][i]; + for (unsigned bit=0;bit<8;++bit) crc=(crc>>1)^((crc&1)?0xedb88320U:0); + } + memcpy(f.fs.files["/radio_profiles"].data()+20,&crc,4); + Radio radio; mesh::RadioProfileCLI protected_backup; + protected_backup.begin(&f.fs, &radio, &f.clock); + assert(protected_backup.handle("set radio2.cross on", f.reply)); + assert(strstr(f.reply,"Error")); + assert(f.fs.files["/radio_profiles.bak"]==backup); } { Fixture f; diff --git a/test/test_nrf52_ble_startup.py b/test/test_nrf52_ble_startup.py index f8180425..42159f1f 100644 --- a/test/test_nrf52_ble_startup.py +++ b/test/test_nrf52_ble_startup.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Run the actual task wrapper and BLE startup with injected allocation failures.""" +"""Run the actual task wrapper and BLE startup failure handling.""" from pathlib import Path import subprocess @@ -29,7 +29,6 @@ HARNESS = r''' #include #include #include -#include #ifndef COMPANION_FEATURE_BLE_MOTA_SOURCE #define COMPANION_FEATURE_BLE_MOTA_SOURCE 1 #endif @@ -53,7 +52,6 @@ struct Logging { void println(const char*) {} }; Logging& usbLoggingPort() { static Logging port; return port; } } -static const char* fail_task = nullptr; static int fail_service = 0; static bool fail_softdevice = false; static unsigned last_depth; @@ -62,9 +60,8 @@ extern "C" BaseType_t __wrap_xTaskCreate(TaskFunction_t, const char*, extern "C" BaseType_t __real_xTaskCreate(TaskFunction_t, const char* name, configSTACK_DEPTH_TYPE depth, void*, UBaseType_t, TaskHandle_t* out) { last_depth = depth; - bool fail = fail_task && name && strcmp(name, fail_task) == 0; - if (out) *out = fail ? nullptr : reinterpret_cast(1); - return fail ? 0 : pdPASS; + if (out) *out = reinterpret_cast(1); + return pdPASS; } struct Settings { template void clearBonds(T...) {} @@ -93,7 +90,8 @@ struct BluefruitStub { ++starts; if (fail_softdevice) return false; TaskHandle_t task; - // Match the pinned core: it ignores both worker creation return values. + // The pinned LTO core resolves these internal calls directly, rather than + // through the application's linker wrapper. __wrap_xTaskCreate(nullptr, "BLE", 1280, nullptr, 3, &task); __wrap_xTaskCreate(nullptr, "SOC", 200, nullptr, 3, &task); return true; @@ -142,21 +140,18 @@ struct SerialBLEInterface { static SerialBLEInterface* instance = nullptr; @BEGIN@ int main() { - mesh::nrf52::resetBleTaskStartup(); - assert(!mesh::nrf52::bleTasksStarted()); __wrap_xTaskCreate(nullptr, "loop", 1024, nullptr, 1, nullptr); - assert(last_depth == 2048 && !mesh::nrf52::bleTasksStarted()); + assert(last_depth == 2048); __wrap_xTaskCreate(nullptr, "callback", 768, nullptr, 1, nullptr); assert(last_depth == 768); __wrap_xTaskCreate(nullptr, nullptr, 100, nullptr, 1, nullptr); assert(last_depth == 100); - for (int fault = 0; fault <= 8; ++fault) { + for (int fault = 0; fault <= 6; ++fault) { #if !COMPANION_FEATURE_BLE_MOTA_SOURCE - if (fault >= 5 && fault <= 7) continue; + if (fault >= 3 && fault <= 5) continue; #endif fail_softdevice = fault == 1; - fail_task = fault == 2 ? "BLE" : fault == 3 ? "SOC" : nullptr; - fail_service = fault >= 4 ? fault - 3 : 0; + fail_service = fault >= 2 ? fault - 1 : 0; SerialBLEInterface port; const int starts = Bluefruit.starts; const bool expected = fault == 0; @@ -174,7 +169,7 @@ int main() { class Nrf52BleStartupTest(unittest.TestCase): - def test_all_startup_failures_are_reported_without_reinitializing(self): + def test_reported_startup_failures_are_not_reinitialized(self): begin = method((ROOT / "src/helpers/nrf52/SerialBLEInterface.cpp").read_text(), "bool SerialBLEInterface::begin(") header = (ROOT / "src/helpers/nrf52/SerialBLEInterface.h").read_text()