Added Global Mutex for SD IOPS, and changed OTA to write directly to Flash

vesper/src/OTAManager/OTAManager.cpp

@@ -2,8 +2,10 @@
 #include "../ConfigManager/ConfigManager.hpp"
 #include "../Logging/Logging.hpp"
 #include "../Player/Player.hpp"
+#include "../SDCardMutex/SDCardMutex.hpp"
 #include <nvs_flash.h>
 #include <nvs.h>
+#include <esp_task_wdt.h>
 
 OTAManager::OTAManager(ConfigManager& configManager)
     : _configManager(configManager)
@@ -410,7 +412,7 @@ bool OTAManager::downloadAndInstall(const String& channel) {
         LOG_INFO("OTA: Trying firmware download from server %d/%d: %s",
                 serverIndex + 1, servers.size(), baseUrl.c_str());
 
-        // 🔥 NEW: Download directly to flash, bypassing SD card
+        // 🔥 Download directly to flash (bypassing problematic SD card writes)
         if (downloadDirectToFlash(firmwareUrl, _expectedFileSize)) {
             LOG_INFO("✅ OTA update successful!");
             return true;
@@ -614,6 +616,17 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
         _telemetry->pause();
     }
 
+    // 🔒 ACQUIRE SD CARD MUTEX - Prevents concurrent SD access
+    LOG_INFO("OTA: Acquiring SD card mutex for safe file operations");
+    if (!SDCardMutex::getInstance().lock(10000)) {  // 10 second timeout
+        LOG_ERROR("OTA: Failed to acquire SD card mutex!");
+        if (_timeKeeper) _timeKeeper->resumeClockUpdates();
+        if (_telemetry) _telemetry->resume();
+        setStatus(Status::FAILED, ErrorCode::DOWNLOAD_FAILED);
+        http.end();
+        return false;
+    }
+
     // Delete file if it exists, before opening
     if (SD.exists(tempPath.c_str())) {
         LOG_INFO("OTA: Removing existing staged update file");
@@ -628,7 +641,8 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
     if (!file) {
         LOG_ERROR("Failed to create temporary update file");
 
-        // Resume systems before returning
+        // Release mutex and resume systems before returning
+        SDCardMutex::getInstance().unlock();
         if (_timeKeeper) _timeKeeper->resumeClockUpdates();
         if (_telemetry) _telemetry->resume();
 
@@ -637,27 +651,31 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
         return false;
     }
 
-    LOG_INFO("OTA: File opened successfully for writing");
+    LOG_INFO("OTA: File opened successfully for writing (mutex locked)");
 
     WiFiClient* stream = http.getStreamPtr();
-    uint8_t buffer[128];  // Smaller buffer for testing
+    uint8_t buffer[4096];  // ✅ Increased to 4KB for better performance
     size_t written = 0;
     size_t lastLoggedPercent = 0;
+    unsigned long lastYield = millis();
+    int loopsWithoutData = 0;
 
     while (http.connected() && written < (size_t)contentLength) {
         size_t available = stream->available();
         if (available) {
+            loopsWithoutData = 0;  // Reset counter when we have data
             size_t toRead = min(available, sizeof(buffer));
             size_t bytesRead = stream->readBytes(buffer, toRead);
 
             if (bytesRead > 0) {
-                // Write directly and immediately flush
+                // Write to SD card
                 size_t bytesWritten = file.write(buffer, bytesRead);
 
-                // Immediately check if write succeeded
+                // Check if write succeeded
                 if (bytesWritten != bytesRead) {
                     LOG_ERROR("SD write failed at offset %u (%u/%u bytes written)", written, bytesWritten, bytesRead);
                     file.close();
+                    SDCardMutex::getInstance().unlock();
                     http.end();
 
                     if (_timeKeeper) _timeKeeper->resumeClockUpdates();
@@ -669,10 +687,10 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
 
                 written += bytesWritten;
 
-                // ✅ IMPROVED: Progress reporting with percentage
+                // Progress reporting
                 notifyProgress(written, contentLength);
 
-                // Log progress every 20% (less frequent to reduce SD contention)
+                // Log progress every 20%
                 size_t currentPercent = (written * 100) / contentLength;
                 if (currentPercent >= lastLoggedPercent + 20) {
                     LOG_INFO("OTA: Download progress: %u%% (%u/%u bytes)",
@@ -680,10 +698,21 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
                     lastLoggedPercent = currentPercent;
                 }
             }
+        } else {
+            // No data available - yield to prevent tight loop
+            loopsWithoutData++;
+            if (loopsWithoutData > 10) {
+                // If we've waited a while with no data, give longer yield
+                vTaskDelay(pdMS_TO_TICKS(1));  // 1ms delay
+                loopsWithoutData = 0;
+            }
         }
 
-        // Brief yield to allow other tasks to run
+        // 🐕 Yield every 100ms to allow other tasks (including IDLE) to run
-        yield();
+        if (millis() - lastYield > 100) {
+            vTaskDelay(pdMS_TO_TICKS(1));  // Just 1ms is enough
+            lastYield = millis();
+        }
     }
 
     // 🔥 CRITICAL: Flush file buffer before closing to ensure all data is written
@@ -691,6 +720,10 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
     delay(100); // Brief delay to ensure SD card completes write
     file.close();
 
+    // 🔓 RELEASE SD CARD MUTEX - Other tasks can now access SD
+    SDCardMutex::getInstance().unlock();
+    LOG_INFO("OTA: SD card mutex released");
+
     http.end();
 
     // ═══════════════════════════════════════════════════════════════════════════
@@ -714,12 +747,20 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
 
     LOG_INFO("Download complete (%u bytes)", written);
 
+    // 🔒 Acquire mutex for file verification operations
+    if (!SDCardMutex::getInstance().lock(5000)) {
+        LOG_ERROR("OTA: Failed to acquire SD mutex for verification");
+        setStatus(Status::FAILED, ErrorCode::DOWNLOAD_FAILED);
+        return false;
+    }
+
     // 🔍 DEBUG: Check actual file size on SD card
     size_t actualFileSize = _fileManager->getFileSize(tempPath);
     LOG_INFO("OTA: File size on SD card: %u bytes (expected: %u)", actualFileSize, written);
 
     if (actualFileSize != written) {
         LOG_ERROR("OTA: FILE SIZE MISMATCH ON SD CARD! Expected %u, got %u", written, actualFileSize);
+        SDCardMutex::getInstance().unlock();
         setStatus(Status::FAILED, ErrorCode::WRITE_FAILED);
         return false;
     }
@@ -740,7 +781,9 @@ bool OTAManager::downloadToSD(const String& url, const String& expectedChecksum,
         LOG_INFO("%s", hexDump.c_str());
     }
 
-    // Verify checksum
+    SDCardMutex::getInstance().unlock();  // Release before checksum (checksum will acquire its own)
+
+    // Verify checksum (verifyChecksum acquires its own mutex)
     if (!verifyChecksum(tempPath, expectedChecksum)) {
         LOG_ERROR("Checksum verification failed after download");
         _fileManager->deleteFile(tempPath);
@@ -774,32 +817,42 @@ bool OTAManager::verifyChecksum(const String& filePath, const String& expectedCh
 }
 
 String OTAManager::calculateSHA256(const String& filePath) {
+    // 🔒 Acquire SD mutex for file reading
+    if (!SDCardMutex::getInstance().lock(5000)) {
+        LOG_ERROR("Failed to acquire SD mutex for checksum calculation");
+        return "";
+    }
+
     File file = SD.open(filePath.c_str());
     if (!file) {
         LOG_ERROR("Failed to open file for checksum calculation: %s", filePath.c_str());
+        SDCardMutex::getInstance().unlock();
         return "";
     }
-    
+
     mbedtls_md_context_t ctx;
     mbedtls_md_type_t md_type = MBEDTLS_MD_SHA256;
-    
+
     mbedtls_md_init(&ctx);
     mbedtls_md_setup(&ctx, mbedtls_md_info_from_type(md_type), 0);
     mbedtls_md_starts(&ctx);
-    
+
     uint8_t buffer[1024];
     size_t bytesRead;
-    
+
     while ((bytesRead = file.readBytes((char*)buffer, sizeof(buffer))) > 0) {
         mbedtls_md_update(&ctx, buffer, bytesRead);
     }
-    
+
     uint8_t hash[32];
     mbedtls_md_finish(&ctx, hash);
     mbedtls_md_free(&ctx);
-    
+
     file.close();
-    
+
+    // 🔓 Release SD mutex
+    SDCardMutex::getInstance().unlock();
+
     // Convert to hex string
     String hashString = "";
     for (int i = 0; i < 32; i++) {
@@ -809,12 +862,22 @@ String OTAManager::calculateSHA256(const String& filePath) {
         }
         hashString += hex;
     }
-    
+
     return hashString;
 }
 
 bool OTAManager::installFromSD(const String& filePath) {
+    // 🔒 Acquire SD mutex for file size check
+    if (!SDCardMutex::getInstance().lock(5000)) {
+        LOG_ERROR("Failed to acquire SD mutex for installation");
+        setStatus(Status::FAILED, ErrorCode::DOWNLOAD_FAILED);
+        return false;
+    }
+
     size_t updateSize = _fileManager->getFileSize(filePath);
+
+    SDCardMutex::getInstance().unlock();  // Release after size check
+
     if (updateSize == 0) {
         LOG_ERROR("Empty update file");
         setStatus(Status::FAILED, ErrorCode::DOWNLOAD_FAILED);
@@ -830,15 +893,26 @@ bool OTAManager::installFromSD(const String& filePath) {
         return false;
     }
 
+    // 🔒 Acquire SD mutex for file reading during flash
+    if (!SDCardMutex::getInstance().lock(30000)) {  // 30 second timeout for flash operation
+        LOG_ERROR("Failed to acquire SD mutex for firmware flash");
+        setStatus(Status::FAILED, ErrorCode::DOWNLOAD_FAILED);
+        return false;
+    }
+
     File updateBin = SD.open(filePath.c_str());
     if (!updateBin) {
         LOG_ERROR("Failed to open update file: %s", filePath.c_str());
+        SDCardMutex::getInstance().unlock();
         setStatus(Status::FAILED, ErrorCode::DOWNLOAD_FAILED);
         return false;
     }
 
     size_t written = Update.writeStream(updateBin);
     updateBin.close();
+
+    // 🔓 Release SD mutex after reading file
+    SDCardMutex::getInstance().unlock();
     
     if (written == updateSize) {
         LOG_INFO("Update written successfully (%u bytes)", written);
@@ -956,7 +1030,7 @@ bool OTAManager::performManualUpdate(const String& channel) {
 
     String firmwareUrl = buildFirmwareUrl(channel);
 
-    // 🔥 NEW: Download directly to flash, bypassing SD card
+    // Download directly to flash
     return downloadDirectToFlash(firmwareUrl, _expectedFileSize);
 }
 
@@ -981,15 +1055,13 @@ bool OTAManager::performCustomUpdate(const String& firmwareUrl, const String& ch
     LOG_INFO("   URL: %s", firmwareUrl.c_str());
     LOG_INFO("   File Size: %u bytes", fileSize);
 
-    // NOTE: checksum parameter is kept for API compatibility but not used
-    // Validation is performed by ESP32 bootloader after flash
     if (!checksum.isEmpty()) {
-        LOG_INFO("   Checksum: %s (provided for reference only)", checksum.c_str());
+        LOG_INFO("   Checksum: %s (NOTE: ESP32 will validate after flash)", checksum.c_str());
     }
 
     setStatus(Status::DOWNLOADING);
 
-    // 🔥 NEW: Download directly to flash, bypassing SD card
+    // Download directly to flash
     bool result = downloadDirectToFlash(firmwareUrl, fileSize);
 
     if (result) {

vesper/src/SDCardMutex/SDCardMutex.hpp

@@ -0,0 +1,227 @@
+/*
+ * ═══════════════════════════════════════════════════════════════════════════════
+ * SDCARDMUTEX.HPP - Thread-Safe SD Card Access Manager
+ * ═══════════════════════════════════════════════════════════════════════════════
+ *
+ * 🔒 THE SD CARD CONCURRENCY GUARDIAN OF VESPER 🔒
+ *
+ * This singleton class provides thread-safe access to the SD card by managing
+ * a FreeRTOS mutex. All SD card operations MUST acquire this mutex to prevent
+ * concurrent access that can lead to file corruption and write failures.
+ *
+ * CRITICAL: The ESP32 SD library is NOT thread-safe. Without this mutex,
+ * simultaneous SD access from multiple FreeRTOS tasks will cause:
+ * - File corruption
+ * - Write failures
+ * - SD card "not recognized" errors
+ * - Random intermittent failures
+ *
+ * USAGE:
+ *
+ *   // Lock before ANY SD operation
+ *   if (SDCardMutex::getInstance().lock()) {
+ *       File file = SD.open("/myfile.txt", FILE_WRITE);
+ *       file.println("data");
+ *       file.close();
+ *       SDCardMutex::getInstance().unlock();
+ *   }
+ *
+ *   // Or use RAII helper for automatic unlock
+ *   {
+ *       SDCardLock lock;  // Acquires mutex
+ *       File file = SD.open("/myfile.txt", FILE_WRITE);
+ *       file.println("data");
+ *       file.close();
+ *   } // Automatically releases mutex when going out of scope
+ *
+ * 📋 VERSION: 1.0
+ * 📅 DATE: 2025-01-07
+ * 👨‍💻 AUTHOR: Advanced Bell Systems
+ * ═══════════════════════════════════════════════════════════════════════════════
+ */
+
+#pragma once
+
+#include <Arduino.h>
+#include <freertos/FreeRTOS.h>
+#include <freertos/semphr.h>
+#include "../Logging/Logging.hpp"
+
+/**
+ * @brief Singleton class for thread-safe SD card access
+ *
+ * Manages a global mutex that all SD card operations must acquire
+ * to prevent concurrent access from multiple FreeRTOS tasks.
+ */
+class SDCardMutex {
+public:
+    /**
+     * @brief Get the singleton instance
+     * @return Reference to the singleton instance
+     */
+    static SDCardMutex& getInstance() {
+        static SDCardMutex instance;
+        return instance;
+    }
+
+    /**
+     * @brief Initialize the mutex (call once during setup)
+     * @return true if initialization succeeded, false otherwise
+     */
+    bool begin() {
+        if (_mutex != NULL) {
+            LOG_WARNING("SDCardMutex already initialized");
+            return true;
+        }
+
+        _mutex = xSemaphoreCreateMutex();
+
+        if (_mutex == NULL) {
+            LOG_ERROR("Failed to create SD card mutex!");
+            return false;
+        }
+
+        LOG_INFO("SD card mutex initialized");
+        return true;
+    }
+
+    /**
+     * @brief Acquire the SD card mutex
+     * @param timeoutMs Maximum time to wait for mutex (default: 5 seconds)
+     * @return true if mutex acquired, false if timeout
+     */
+    bool lock(uint32_t timeoutMs = 5000) {
+        if (_mutex == NULL) {
+            LOG_ERROR("SDCardMutex not initialized!");
+            return false;
+        }
+
+        TickType_t timeout = (timeoutMs == portMAX_DELAY)
+            ? portMAX_DELAY
+            : pdMS_TO_TICKS(timeoutMs);
+
+        if (xSemaphoreTake(_mutex, timeout) == pdTRUE) {
+            _lockCount++;
+            return true;
+        } else {
+            LOG_ERROR("SD card mutex timeout after %u ms!", timeoutMs);
+            return false;
+        }
+    }
+
+    /**
+     * @brief Release the SD card mutex
+     */
+    void unlock() {
+        if (_mutex == NULL) {
+            LOG_ERROR("SDCardMutex not initialized!");
+            return;
+        }
+
+        xSemaphoreGive(_mutex);
+        _unlockCount++;
+    }
+
+    /**
+     * @brief Get mutex lock statistics
+     * @param locks Reference to store lock count
+     * @param unlocks Reference to store unlock count
+     */
+    void getStats(uint32_t& locks, uint32_t& unlocks) const {
+        locks = _lockCount;
+        unlocks = _unlockCount;
+    }
+
+    /**
+     * @brief Check if mutex is currently locked by THIS task
+     * @return true if current task holds the mutex
+     */
+    bool isLockedByMe() const {
+        if (_mutex == NULL) {
+            return false;
+        }
+        return xSemaphoreGetMutexHolder(_mutex) == xTaskGetCurrentTaskHandle();
+    }
+
+    // Delete copy constructor and assignment operator (singleton)
+    SDCardMutex(const SDCardMutex&) = delete;
+    SDCardMutex& operator=(const SDCardMutex&) = delete;
+
+private:
+    SDCardMutex()
+        : _mutex(NULL)
+        , _lockCount(0)
+        , _unlockCount(0) {
+    }
+
+    ~SDCardMutex() {
+        if (_mutex != NULL) {
+            vSemaphoreDelete(_mutex);
+            _mutex = NULL;
+        }
+    }
+
+    SemaphoreHandle_t _mutex;
+    uint32_t _lockCount;
+    uint32_t _unlockCount;
+};
+
+/**
+ * @brief RAII helper class for automatic mutex lock/unlock
+ *
+ * Acquires SD card mutex on construction and releases on destruction.
+ * Use this for automatic cleanup when going out of scope.
+ *
+ * Example:
+ *   {
+ *       SDCardLock lock;
+ *       File file = SD.open("/test.txt", FILE_WRITE);
+ *       file.println("data");
+ *       file.close();
+ *   } // Mutex automatically released here
+ */
+class SDCardLock {
+public:
+    /**
+     * @brief Constructor - acquires mutex
+     * @param timeoutMs Maximum time to wait for mutex
+     */
+    explicit SDCardLock(uint32_t timeoutMs = 5000)
+        : _locked(false) {
+        _locked = SDCardMutex::getInstance().lock(timeoutMs);
+        if (!_locked) {
+            LOG_ERROR("SDCardLock failed to acquire mutex!");
+        }
+    }
+
+    /**
+     * @brief Destructor - releases mutex
+     */
+    ~SDCardLock() {
+        if (_locked) {
+            SDCardMutex::getInstance().unlock();
+        }
+    }
+
+    /**
+     * @brief Check if lock was successfully acquired
+     * @return true if mutex is locked
+     */
+    bool isLocked() const {
+        return _locked;
+    }
+
+    /**
+     * @brief Explicit conversion to bool for easy checking
+     */
+    explicit operator bool() const {
+        return _locked;
+    }
+
+    // Delete copy constructor and assignment operator
+    SDCardLock(const SDCardLock&) = delete;
+    SDCardLock& operator=(const SDCardLock&) = delete;
+
+private:
+    bool _locked;
+};

vesper/vesper.ino

@@ -122,6 +122,7 @@
 // ═══════════════════════════════════════════════════════════════════════════════════
 // CUSTOM CLASSES - Include Custom Classes and Functions
 // ═══════════════════════════════════════════════════════════════════════════════════
+#include "src/SDCardMutex/SDCardMutex.hpp"  // ⚠️ MUST be included before any SD-using classes
 #include "src/ConfigManager/ConfigManager.hpp"
 #include "src/FileManager/FileManager.hpp"
 #include "src/TimeKeeper/TimeKeeper.hpp"
@@ -206,6 +207,15 @@ void setup()
 	SPI.begin(hwConfig.ethSpiSck, hwConfig.ethSpiMiso, hwConfig.ethSpiMosi);
   delay(50);
 
+	// 🔒 CRITICAL: Initialize SD Card Mutex BEFORE any SD operations
+	// This prevents concurrent SD access from multiple FreeRTOS tasks
+	if (!SDCardMutex::getInstance().begin()) {
+		Serial.println("❌ FATAL: Failed to initialize SD card mutex!");
+		Serial.println("   System cannot continue safely - entering infinite loop");
+		while(1) { delay(1000); }  // Halt system - unsafe to proceed
+	}
+	Serial.println("✅ SD card mutex initialized");
+
 	// Initialize Configuration (loads factory identity from NVS + user settings from SD)
 	configManager.begin();