diff --git a/vesper/src/OTAManager/OTAManager.cpp b/vesper/src/OTAManager/OTAManager.cpp index f7fc032..916078f 100644 --- a/vesper/src/OTAManager/OTAManager.cpp +++ b/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 #include +#include 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(); + // 🐕 Yield every 100ms to allow other tasks (including IDLE) to run + 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) { diff --git a/vesper/src/SDCardMutex/SDCardMutex.hpp b/vesper/src/SDCardMutex/SDCardMutex.hpp new file mode 100644 index 0000000..dc81d30 --- /dev/null +++ b/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 +#include +#include +#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; +}; diff --git a/vesper/vesper.ino b/vesper/vesper.ino index 13355d0..712119a 100644 --- a/vesper/vesper.ino +++ b/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();