fix(companion): recover radio profile settings after flash

src/Nrf52LoopStack.cpp

@@ -4,17 +4,6 @@
 #include <FreeRTOS.h>
 #include <task.h>
 #include <string.h>
-#include <helpers/nrf52/BleTaskStartup.h>
-
-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;
 }
 

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;
   }

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,

src/helpers/nrf52/BleTaskStartup.h

@@ -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

src/helpers/nrf52/SerialBLEInterface.cpp

@@ -1,5 +1,4 @@
 #include "SerialBLEInterface.h"
-#include "BleTaskStartup.h"
 #include "../BluetoothMac.h"
 #include "../CompanionFrameQueue.h"
 #include <stdio.h>
@@ -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;
   }
 

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;

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 <cstring>
 #include <helpers/BluetoothMac.h>
 #include <helpers/BleMotaProtocol.h>
-#include <helpers/nrf52/BleTaskStartup.h>
 #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<void*>(1);
-  return fail ? 0 : pdPASS;
+  if (out) *out = reinterpret_cast<void*>(1);
+  return pdPASS;
 }
 struct Settings {
   template<class... T> 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()