diff --git a/src/OtaManager.cpp b/src/OtaManager.cpp index d71664b..86afd5c 100644 --- a/src/OtaManager.cpp +++ b/src/OtaManager.cpp @@ -9,7 +9,7 @@ // For heap_caps_malloc and esp_ptr_external_ram, ensure correct include if not already covered by Arduino.h/ESP-IDF basics // #include "esp_heap_caps.h" // Already in OtaManager.h but good to be mindful -#if ARDUINOJSON_VERSION_MAJOR >= 6 && ARDUINOJSON_VERSION_MINOR >= 18 +#if ARDUINOJSON_VERSION_MAJOR >= 6 && ARDUINOJSON_VERSION_MINOR >= 18 #define OTAMANAGER_ARDUINOJSON_ENABLE_PSRAM ARDUINOJSON_ENABLE_PSRAM #else #define OTAMANAGER_ARDUINOJSON_ENABLE_PSRAM 0 // Older versions might not have this macro defined @@ -89,6 +89,15 @@ OtaManager::OtaManager(const String& currentVersion, const String& repoOwner, co _lastCheckResult.updateAvailable = false; _lastCheckResult.currentVersion = _currentVersion; + // only one watchdog timer should be initialized for the whole project + // if this is needed elsewhere, it should be hoisted to a shared file/location + esp_task_wdt_config_t twdt_config_update = { + .timeout_ms = 120000, // 120 seconds + .idle_core_mask = (1 << CONFIG_FREERTOS_NUMBER_OF_CORES) - 1, + .trigger_panic = true + }; + ESP_ERROR_CHECK(esp_task_wdt_reconfigure(&twdt_config_update)); + _dataMutex = xSemaphoreCreateMutex(); if (_dataMutex == NULL) { Serial.println("Error: Failed to create OtaManager data mutex!"); @@ -100,8 +109,8 @@ OtaManager::OtaManager(const String& currentVersion, const String& repoOwner, co bool OtaManager::checkForUpdate() { if (_dataMutex) { if (xSemaphoreTake(_dataMutex, (TickType_t)10) == pdTRUE) { // Try to take mutex with short timeout - if (_currentStatus.status == UpdateStatus::State::CHECKING_VERSION || - _currentStatus.status == UpdateStatus::State::DOWNLOADING || + if (_currentStatus.status == UpdateStatus::State::CHECKING_VERSION || + _currentStatus.status == UpdateStatus::State::DOWNLOADING || _currentStatus.status == UpdateStatus::State::WRITING) { Serial.println("OtaManager: Check or update already in progress."); xSemaphoreGive(_dataMutex); @@ -128,7 +137,7 @@ bool OtaManager::checkForUpdate() { return false; // Critical error } - // Create a task to handle the update check. + // Create a task to handle the update check. // The task will be responsible for NTP sync and then the actual check. if (xTaskCreatePinnedToCore( _checkUpdateTaskRunner, /* Task function */ @@ -177,7 +186,7 @@ bool OtaManager::beginUpdate(const String& downloadUrl) { Serial.println("OtaManager: [beginUpdate] Error: Update already in progress."); if (_dataMutex) xSemaphoreGive(_dataMutex); // Release mutex before returning Serial.println("OtaManager: [beginUpdate] dataMutex given (update already in progress)."); - return false; + return false; } // If we are here, status is NOT DOWNLOADING or WRITING. Mutex is still held. // Crucial change: Release mutex BEFORE calling _setUpdateStatus, as _setUpdateStatus will take it. @@ -189,14 +198,14 @@ bool OtaManager::beginUpdate(const String& downloadUrl) { // This is a sanity check; if mutex was taken successfully, it shouldn't be NULL here. } - _setUpdateStatus(UpdateStatus::State::DOWNLOADING, "Starting update...", 0); + _setUpdateStatus(UpdateStatus::State::DOWNLOADING, "Starting update...", 0); // _setUpdateStatus will handle its own mutex locking for setting the status. // The rest of the function does not need to re-take the mutex for its operations (strdup, malloc, xTaskCreatePinnedToCore) // unless _updateTaskHandle itself needs protection, but xTaskCreatePinnedToCore is task-safe for the handle. Serial.println("OtaManager: [beginUpdate] About to allocate memory for URL copy."); - char* urlCopy = strdup(downloadUrl.c_str()); + char* urlCopy = strdup(downloadUrl.c_str()); if (!urlCopy) { Serial.println("OtaManager: [beginUpdate] Error: Failed to allocate memory for URL copy."); _setUpdateStatus(UpdateStatus::State::ERROR_UPDATE_BEGIN, "Memory allocation failed for URL."); @@ -227,7 +236,7 @@ bool OtaManager::beginUpdate(const String& downloadUrl) { &_updateTaskHandle, /* Task handle */ 0 /* Core pin */ ); - + Serial.printf("OtaManager: [beginUpdate] xTaskCreatePinnedToCore result: %s\n", (taskCreateResult == pdPASS) ? "pdPASS" : "pdFAIL"); if (taskCreateResult != pdPASS) { @@ -330,19 +339,19 @@ void OtaManager::_setUpdateStatus(UpdateStatus::State state, const String& messa Serial.println("OtaManager: [_setUpdateStatus] CRITICAL_ERROR: Failed to take dataMutex even with portMAX_DELAY."); // This situation should ideally not happen with portMAX_DELAY. // Log with passed-in values, as status wasn't actually set. - Serial.printf("CRITICAL_ERROR_DETAIL: OtaManager::_setUpdateStatus mutex fail. Requested state: [%d] %s (%d%%)\n", + Serial.printf("CRITICAL_ERROR_DETAIL: OtaManager::_setUpdateStatus mutex fail. Requested state: [%d] %s (%d%%)\n", static_cast(localState), localMessage.c_str(), localProgress == -1 ? -1 : localProgress); return; // Skip logging below as status wasn't actually set } } else { Serial.println("OtaManager: [_setUpdateStatus] CRITICAL_ERROR: _dataMutex is NULL."); - Serial.printf("CRITICAL_ERROR_DETAIL: OtaManager::_setUpdateStatus _dataMutex NULL. Requested state: [%d] %s (%d%%)\n", + Serial.printf("CRITICAL_ERROR_DETAIL: OtaManager::_setUpdateStatus _dataMutex NULL. Requested state: [%d] %s (%d%%)\n", static_cast(localState), localMessage.c_str(), localProgress == -1 ? -1 : localProgress); return; // Skip logging below } // Perform logging after mutex has been released - Serial.printf("OtaManager Status: [%d] %s (%d%%) (Logged from _setUpdateStatus after mutex release)\n", + Serial.printf("OtaManager Status: [%d] %s (%d%%) (Logged from _setUpdateStatus after mutex release)\n", static_cast(localState), localMessage.c_str(), actualProgressForLogging); Serial.println("OtaManager: [_setUpdateStatus] Exiting."); } @@ -398,7 +407,7 @@ UpdateInfo OtaManager::_parseGithubApiResponse(const String& jsonPayload) { // Capacity can be larger when using PSRAM, e.g., 10KB or 20KB depending on expected payload size // Let's try 10KB first. Max GitHub API response for releases is 30 items, but we only parse the first. // However, release notes can be long. - BasicJsonDocument doc(10240); + BasicJsonDocument doc(10240); #else Serial.println("OtaManager: Using default allocator (internal RAM) for JSON parsing."); DynamicJsonDocument doc(1536); // Previous reduced size for internal RAM fallback @@ -412,15 +421,15 @@ UpdateInfo OtaManager::_parseGithubApiResponse(const String& jsonPayload) { return info; } - if (!doc.is() || doc.as().size() == 0) { - Serial.println(F("OtaManager: JSON is not an array or is empty. No release found?")); + if (!doc.is()) { + Serial.println(F("OtaManager: JSON is not an object. No release found?")); info.error = "No release information found in API response."; return info; } - JsonObject latestRelease = doc.as()[0]; + JsonObject latestRelease = doc.as(); const char* tagName = latestRelease["tag_name"]; - + if (tagName) { info.availableVersion = String(tagName); // Simple version comparison (e.g., "v0.1.1" > "v0.1.0") @@ -437,8 +446,12 @@ UpdateInfo OtaManager::_parseGithubApiResponse(const String& jsonPayload) { JsonArray assets = latestRelease["assets"].as(); for (JsonObject asset : assets) { - if (String(asset["name"].as()) == _firmwareAssetName) { - info.downloadUrl = asset["browser_download_url"].as(); + String assetName = asset["name"].as(); + String downloadUrl = asset["browser_download_url"].as(); + + if (assetName == _firmwareAssetName) { + info.downloadUrl = downloadUrl; + Serial.printf("OtaManager: MATCHED firmware asset '%s' with URL: %s\n", _firmwareAssetName.c_str(), downloadUrl.c_str()); break; } } @@ -466,7 +479,7 @@ bool OtaManager::_ensureTimeSynced() { Serial.println("OtaManager: Attempting to sync NTP time..."); configTime(0, 0, "pool.ntp.org", "time.nist.gov"); // UTC, no DST offset, NTP servers - + time_t now = time(nullptr); int retries = 0; while (now < 8 * 3600 * 2) { // Check if time is reasonably after epoch (1 Jan 1970) @@ -488,14 +501,8 @@ bool OtaManager::_ensureTimeSynced() { // Static task runners void OtaManager::_checkUpdateTaskRunner(void* pvParameters) { Serial.println("DEBUG: _checkUpdateTaskRunner started."); - Serial.println("!!!!!!!!!! UPDATE TASK RUNNER - VERSION XYZ - CHECKING IN !!!!!!!!!!"); + Serial.println("!!!!!!!!!! UPDATE TASK RUNNER - VERSION XYZ - CHECKING IN !!!!!!!!!!"); OtaManager* self = static_cast(pvParameters); - esp_task_wdt_config_t twdt_config_check = { - .timeout_ms = 30000, // 30 seconds - .idle_core_mask = (1 << CONFIG_FREERTOS_NUMBER_OF_CORES) - 1, - .trigger_panic = true - }; - ESP_ERROR_CHECK(esp_task_wdt_init(&twdt_config_check)); ESP_ERROR_CHECK(esp_task_wdt_add(NULL)); // Add current task to WDT // Perform NTP Sync first @@ -503,7 +510,7 @@ void OtaManager::_checkUpdateTaskRunner(void* pvParameters) { // NTP failed. Update status and terminate task. UpdateStatus::State ntpErrorState = UpdateStatus::State::ERROR_HTTP_CHECK; // Or a more specific NTP error state if you add one String ntpErrorMessage = "NTP time sync failed. Cannot check for updates."; - + if (self->_dataMutex) { if (xSemaphoreTake(self->_dataMutex, portMAX_DELAY) == pdTRUE) { self->_currentStatus.status = ntpErrorState; @@ -511,7 +518,7 @@ void OtaManager::_checkUpdateTaskRunner(void* pvParameters) { self->_lastCheckResult.error = ntpErrorMessage; self->_checkTaskHandle = NULL; // Mark task as complete (failed) xSemaphoreGive(self->_dataMutex); - Serial.printf("OtaManager Status Update (NTP Fail in _checkUpdateTaskRunner): [%d] %s\n", + Serial.printf("OtaManager Status Update (NTP Fail in _checkUpdateTaskRunner): [%d] %s\n", static_cast(ntpErrorState), ntpErrorMessage.c_str()); } else { Serial.println("ERROR: _checkUpdateTaskRunner (NTP Fail) failed to take mutex!"); @@ -527,9 +534,9 @@ void OtaManager::_checkUpdateTaskRunner(void* pvParameters) { // This uses the standard _setUpdateStatus which handles its own mutex and logging. self->_setUpdateStatus(UpdateStatus::State::CHECKING_VERSION, "Checking for updates (post-NTP)...", 0); - String apiUrl = "https://api.github.com/repos/" + self->_repoOwner + "/" + self->_repoName + "/releases"; + String apiUrl = "https://api.github.com/repos/" + self->_repoOwner + "/" + self->_repoName + "/releases/latest"; String jsonPayload = self->_performHttpsRequest(apiUrl.c_str(), self->_githubApiRootCa); - + UpdateInfo localCheckResult; UpdateStatus::State newOtaState = UpdateStatus::State::IDLE; String newOtaMessage = ""; @@ -585,18 +592,18 @@ void OtaManager::_checkUpdateTaskRunner(void* pvParameters) { if (xSemaphoreTake(self->_dataMutex, portMAX_DELAY) == pdTRUE) { self->_lastCheckResult = localCheckResult; // Store the latest check result self->_lastCheckResult.currentVersion = self->_currentVersion; // Ensure current version is part of the result - + self->_currentStatus.status = newOtaState; self->_currentStatus.message = newOtaMessage; if (newOtaProgress != -1) { // Keep this progress update logic consistent self->_currentStatus.progress = newOtaProgress; } - + self->_checkTaskHandle = NULL; // Mark task as complete xSemaphoreGive(self->_dataMutex); // Log *after* releasing the mutex - Serial.printf("OtaManager Status Update (from _checkUpdateTaskRunner): [%d] %s (%d%%)\n", + Serial.printf("OtaManager Status Update (from _checkUpdateTaskRunner): [%d] %s (%d%%)\n", static_cast(newOtaState), newOtaMessage.c_str(), newOtaProgress); } else { Serial.println("ERROR: _checkUpdateTaskRunner failed to take mutex to update status!"); @@ -604,7 +611,7 @@ void OtaManager::_checkUpdateTaskRunner(void* pvParameters) { self->_checkTaskHandle = NULL; // Still try to mark as done to prevent re-triggering issues } } - + esp_task_wdt_delete(NULL); // Remove current task from WDT vTaskDelete(NULL); } @@ -631,7 +638,7 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { return; } Serial.printf("OtaManager: [_updateTaskRunner] self (OtaManager instance) address: %p\n", (void*)self); - + char* downloadUrlCStr_task = params->downloadUrl; // Keep a C-string pointer before creating String if (!downloadUrlCStr_task) { Serial.println("OtaManager: [_updateTaskRunner] CRITICAL ERROR: params->downloadUrl is NULL. Task cannot proceed."); @@ -645,23 +652,17 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { String downloadUrl = String(downloadUrlCStr_task); Serial.printf("OtaManager: [_updateTaskRunner] downloadUrl (String object created): %s\n", downloadUrl.c_str()); - + // Free the passed parameters now that we have copied/used them // Important: downloadUrlCStr_task (which is params->downloadUrl) must be freed. // The 'params' struct itself also needs to be freed. Serial.printf("OtaManager: [_updateTaskRunner] Freeing params->downloadUrl (%p)\n", (void*)params->downloadUrl); - free(params->downloadUrl); + free(params->downloadUrl); params->downloadUrl = nullptr; // Good practice after free Serial.printf("OtaManager: [_updateTaskRunner] Freeing params struct (%p)\n", (void*)params); free(params); params = nullptr; // Good practice - esp_task_wdt_config_t twdt_config_update = { - .timeout_ms = 120000, // 120 seconds - .idle_core_mask = (1 << CONFIG_FREERTOS_NUMBER_OF_CORES) - 1, - .trigger_panic = true - }; - ESP_ERROR_CHECK(esp_task_wdt_init(&twdt_config_update)); ESP_ERROR_CHECK(esp_task_wdt_add(NULL)); // Add current task to WDT HTTPClient http; @@ -687,10 +688,10 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { vTaskDelete(NULL); return; } - + // Use the CA certificate for the HTTPS connection Serial.println("OtaManager: [_updateTaskRunner] Calling http.begin() with URL and Root CA."); - http.begin(downloadUrl, self->_githubApiRootCa); + http.begin(downloadUrl, self->_githubApiRootCa); http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); // Enable following redirects http.setConnectTimeout(10000); // 10s http.setTimeout(180000); // 3 minutes for download @@ -698,7 +699,7 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { Serial.println("OtaManager: [_updateTaskRunner] About to call http.GET()."); int httpCode = http.GET(); Serial.printf("OtaManager: [_updateTaskRunner] http.GET() returned code: %d\n", httpCode); - + if (httpCode > 0) { // Moved the check for httpCode > 0 to encompass redirect handling Serial.printf("OtaManager: [_updateTaskRunner] HTTP GET successful, code: %d\n", httpCode); if (httpCode == HTTP_CODE_OK) { @@ -729,7 +730,7 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { // Log the currently running partition const esp_partition_t *running_partition = esp_ota_get_running_partition(); if (running_partition != NULL) { - Serial.printf("OtaManager: [_updateTaskRunner] Currently running from partition: %s (Type: %d, SubType: %d)\\n", + Serial.printf("OtaManager: [_updateTaskRunner] Currently running from partition: %s (Type: %d, SubType: %d)\\n", running_partition->label, running_partition->type, running_partition->subtype); } else { Serial.println("OtaManager: [_updateTaskRunner] Could not determine running partition (esp_ota_get_running_partition() returned NULL)."); @@ -737,34 +738,34 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { Serial.printf("OtaManager: [_updateTaskRunner] Attempting Update.begin(totalSize: %d, command: U_FLASH, label: \\\"ota_1\\\")\\n", totalSize); // Explicitly target the "ota_1" partition label. U_FLASH (0) is for application partitions. - bool update_begin_success = Update.begin(totalSize, U_FLASH); + bool update_begin_success = Update.begin(totalSize, U_FLASH); Serial.printf("OtaManager: [_updateTaskRunner] Update.begin(\\\"ota_1\\\") result: %s\\n", update_begin_success ? "SUCCESS" : "FAILED"); - + if (!update_begin_success) { uint8_t update_error_code = Update.getError(); - const char* update_error_cstr = Update.errorString(); + const char* update_error_cstr = Update.errorString(); Serial.printf("OtaManager: [_updateTaskRunner] Update.begin(\\\"ota_1\\\") FAILED. Error: %s (%d)\n", update_error_cstr, update_error_code); self->_setUpdateStatus(UpdateStatus::State::ERROR_NO_SPACE, "Update.begin(\\\"ota_1\\\") failed: " + String(update_error_cstr)); - + // Standard error handling: clear task handle, release mutex, end HTTP, delete WDT & task. - if (self->_dataMutex) { + if (self->_dataMutex) { if (xSemaphoreTake(self->_dataMutex, portMAX_DELAY) == pdTRUE) { self->_updateTaskHandle = NULL; xSemaphoreGive(self->_dataMutex); } else { Serial.println("ERROR: _updateTaskRunner (Update.begin fail) failed to take mutex!"); - self->_updateTaskHandle = NULL; + self->_updateTaskHandle = NULL; } } else { Serial.println("ERROR: _updateTaskRunner (Update.begin fail) _dataMutex is NULL!"); - self->_updateTaskHandle = NULL; + self->_updateTaskHandle = NULL; } http.end(); esp_task_wdt_delete(NULL); vTaskDelete(NULL); return; // Essential to exit the task here } - + // If Update.begin() was successful: Serial.printf("OtaManager: [_updateTaskRunner] Post-Update.begin(\\\"ota_1\\\") SUCCESS: Update.isRunning(): %s, Update.canRollBack(): %s\\n", Update.isRunning() ? "true" : "false", @@ -794,9 +795,9 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { uint8_t update_error_code = Update.getError(); // Get the error code const char* update_error_cstr = Update.errorString(); // Get the error string (no argument) Serial.printf("OtaManager: [_updateTaskRunner] Update Error (captured immediately): %s (%d)\n", update_error_cstr, update_error_code); - + self->_setUpdateStatus(UpdateStatus::State::ERROR_UPDATE_WRITE, "Firmware write error: " + String(update_error_cstr)); - if (self->_dataMutex) { + if (self->_dataMutex) { if (xSemaphoreTake(self->_dataMutex, portMAX_DELAY) == pdTRUE) { self->_updateTaskHandle = NULL; xSemaphoreGive(self->_dataMutex); @@ -822,7 +823,7 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { Serial.println("OtaManager: [_updateTaskRunner] stream->readBytes returned a negative value, indicating an error."); // Treat as a download error self->_setUpdateStatus(UpdateStatus::State::ERROR_HTTP_DOWNLOAD, "Stream read error during download"); - if (self->_dataMutex) { + if (self->_dataMutex) { if (xSemaphoreTake(self->_dataMutex, portMAX_DELAY) == pdTRUE) { self->_updateTaskHandle = NULL; xSemaphoreGive(self->_dataMutex); @@ -840,7 +841,7 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { vTaskDelete(NULL); return; } // If c == 0, it means no data was available right now, loop will continue based on http.connected() - + int progress = (int)(((float)written / totalSize) * 100); // Update progress not too frequently to avoid flooding serial/logs if (progress > self->_currentStatus.progress && (millis() - lastProgressUpdate > 1000 || progress == 100)) { @@ -875,19 +876,19 @@ void OtaManager::_updateTaskRunner(void* pvParameters) { } Serial.println("OtaManager: [_updateTaskRunner] Calling http.end()."); http.end(); - + // If we reach here and haven't rebooted, it means an error occurred after starting or during HTTP connection. // Ensure task handle is cleared if not already done by an error path. // Also set status to IDLE if it was an in-progress state that failed silently. if (self->_dataMutex) { // Check if mutex is valid before taking if (xSemaphoreTake(self->_dataMutex, portMAX_DELAY) == pdTRUE) { - if (self->_currentStatus.status != UpdateStatus::State::SUCCESS) { + if (self->_currentStatus.status != UpdateStatus::State::SUCCESS) { // If it was in progress but failed and didn't set a specific error state above - if(self->_currentStatus.status == UpdateStatus::State::DOWNLOADING || self->_currentStatus.status == UpdateStatus::State::WRITING || + if(self->_currentStatus.status == UpdateStatus::State::DOWNLOADING || self->_currentStatus.status == UpdateStatus::State::WRITING || self->_currentStatus.status == UpdateStatus::State::ERROR_HTTP_DOWNLOAD || // if previously set to this by this task self->_currentStatus.status == UpdateStatus::State::ERROR_UPDATE_WRITE || // if previously set to this by this task self->_currentStatus.status == UpdateStatus::State::ERROR_NO_SPACE) { // if previously set to this - self->_setUpdateStatus(UpdateStatus::State::IDLE, "Update failed or ended prematurely"); + self->_setUpdateStatus(UpdateStatus::State::IDLE, "Update failed or ended prematurely"); } } self->_updateTaskHandle = NULL;