Refactor OTA update process and clean up code

Refactored OTA update handling and removed unused code. Improved error handling and logging during firmware updates.

code/components/jomjol_fileserver_ota/server_ota.cpp

@@ -3,13 +3,10 @@
 #include <string>
 #include "string.h"
 
-/* TODO Rethink the usage of the int watchdog. It is no longer to be used, see
-https://docs.espressif.com/projects/esp-idf/en/latest/esp32/migration-guides/release-5.x/5.0/system.html?highlight=esp_int_wdt */
 #include "esp_private/esp_int_wdt.h"
 
 #include <esp_task_wdt.h>
 
-
 #include <string.h>
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
@@ -23,7 +20,6 @@ https://docs.espressif.com/projects/esp-idf/en/latest/esp32/migration-guides/rel
 #include "esp_app_format.h"
 #include "nvs_flash.h"
 #include "driver/gpio.h"
-// #include "protocol_examples_common.h"
 #include "errno.h"
 
 #include <sys/stat.h>
@@ -37,7 +33,6 @@ https://docs.espressif.com/projects/esp-idf/en/latest/esp32/migration-guides/rel
 #include "ClassControllCamera.h"
 #include "connect_wlan.h"
 
-
 #include "ClassLogFile.h"
 
 #include "Helper.h"
@@ -56,24 +51,10 @@ static bool ota_update_task(std::string fn);
 std::string _file_name_update;
 bool initial_setup = false;
 
-
-static void infinite_loop(void)
-{
-    int i = 0;
-    LogFile.WriteToFile(ESP_LOG_INFO, TAG, "When a new firmware is available on the server, press the reset button to download it");
-    while(1) {
-        LogFile.WriteToFile(ESP_LOG_INFO, TAG, "Waiting for a new firmware... (" + to_string(++i) + ")");
-        vTaskDelay(1000 / portTICK_PERIOD_MS);
-    }
-}
-
-
 void task_do_Update_ZIP(void *pvParameter)
 {
     StatusLED(AP_OR_OTA, 1, true);  // Signaling an OTA update
-    
     std::string filetype = toUpper(getFileType(_file_name_update));
-
   	LogFile.WriteToFile(ESP_LOG_INFO, TAG, "File: " + _file_name_update + " Filetype: " + filetype);
 
     if (filetype == "ZIP")
@@ -110,7 +91,8 @@ void task_do_Update_ZIP(void *pvParameter)
 
         LogFile.WriteToFile(ESP_LOG_INFO, TAG, "Trigger reboot due to firmware update");
         doRebootOTA();
-    } else if (filetype == "BIN")
+    } 
+	else if (filetype == "BIN")
     {
        	LogFile.WriteToFile(ESP_LOG_INFO, TAG, "Do firmware update - file: " + _file_name_update);
         ota_update_task(_file_name_update);
@@ -123,11 +105,10 @@ void task_do_Update_ZIP(void *pvParameter)
     }
 }
 
-
 void CheckUpdate()
 {
- 	FILE *pfile;
-    if ((pfile = fopen("/sdcard/update.txt", "r")) == NULL)
+ 	FILE *pfile = fopen("/sdcard/update.txt", "r");
+    if (pfile == NULL)
     {
 		LogFile.WriteToFile(ESP_LOG_DEBUG, TAG, "No pending update");
         return;
@@ -136,6 +117,7 @@ void CheckUpdate()
 	char zw[1024] = "";
 	fgets(zw, 1024, pfile);
     _file_name_update = std::string(zw);
+	
     if (fgets(zw, 1024, pfile))
 	{
 		std::string _szw = std::string(zw);
@@ -149,124 +131,100 @@ void CheckUpdate()
     DeleteFile("/sdcard/update.txt");   // Prevent Boot Loop!!!
 	LogFile.WriteToFile(ESP_LOG_INFO, TAG, "Start update process (" + _file_name_update + ")");
 
-
     xTaskCreate(&task_do_Update_ZIP, "task_do_Update_ZIP", configMINIMAL_STACK_SIZE * 35, NULL, tskIDLE_PRIORITY+1, NULL);
-    while(1) { // wait until reboot within task_do_Update_ZIP
+    while(1) { 
+		// wait until reboot within task_do_Update_ZIP
         vTaskDelay(1000 / portTICK_PERIOD_MS);
     }
 }
 
-
 static bool ota_update_task(std::string fn)
 {
-    esp_err_t err;
-    /* update handle : set by esp_ota_begin(), must be freed via esp_ota_end() */
-    esp_ota_handle_t update_handle = 0 ;
-    const esp_partition_t *update_partition = NULL;
-
     ESP_LOGI(TAG, "Starting OTA update");
 
     const esp_partition_t *configured = esp_ota_get_boot_partition();
     const esp_partition_t *running = esp_ota_get_running_partition();
 
     if (configured != running) {        
-        LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Configured OTA boot partition at offset " + to_string(configured->address) + 
-                ", but running from offset " + to_string(running->address));
+        LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Configured OTA boot partition at offset " + to_string(configured->address) + ", but running from offset " + to_string(running->address));
         LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "(This can happen if either the OTA boot data or preferred boot image become somehow corrupted.)");
     }
-    ESP_LOGI(TAG, "Running partition type %d subtype %d (offset 0x%08x)",
-             running->type, running->subtype, (unsigned int)running->address);
+	
+    ESP_LOGI(TAG, "Running partition type %d subtype %d (offset 0x%08x)", running->type, running->subtype, (unsigned int)running->address);
 
+    const esp_partition_t *update_partition = esp_ota_get_next_update_partition(NULL);
+    ESP_LOGI(TAG, "Writing to partition subtype %d at offset 0x%x", update_partition->subtype, (unsigned int)update_partition->address);
 
-    update_partition = esp_ota_get_next_update_partition(NULL);
-    ESP_LOGI(TAG, "Writing to partition subtype %d at offset 0x%x",
-             update_partition->subtype, (unsigned int)update_partition->address);
-//    assert(update_partition != NULL);
-
-    int binary_file_length = 0;
-
-    // deal with all receive packet 
-    bool image_header_was_checked = false;
-
-    int data_read;     
+    FILE *f = fopen(fn.c_str(), "rb");     // previously only "r
+    if (f == NULL) { 
+		// File does not exist
+        return false;
+    }
 
-    FILE* f = fopen(fn.c_str(), "rb");     // previously only "r
+    int data_read = fread(ota_write_data, 1, SERVER_OTA_SCRATCH_BUFSIZE, f);
 
-    if (f == NULL) { // File does not exist
+    if (data_read <= 0) {
+        LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Error: SSL data read error");
         return false;
     }
 
-    data_read = fread(ota_write_data, 1, SERVER_OTA_SCRATCH_BUFSIZE, f);
-
+    esp_err_t err = ESP_OK;
+    esp_ota_handle_t update_handle = 0;
+    int binary_file_length = 0;
+    bool image_header_was_checked = false;
+	
     while (data_read > 0) {
-        if (data_read < 0) {
-            LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Error: SSL data read error");
-            return false;
-        } else if (data_read > 0) {
-            if (image_header_was_checked == false) {
-                esp_app_desc_t new_app_info;
-                if (data_read > sizeof(esp_image_header_t) + sizeof(esp_image_segment_header_t) + sizeof(esp_app_desc_t)) {
-                    // check current version with downloading
-                    memcpy(&new_app_info, &ota_write_data[sizeof(esp_image_header_t) + sizeof(esp_image_segment_header_t)], sizeof(esp_app_desc_t));
-                    ESP_LOGI(TAG, "New firmware version: %s", new_app_info.version);
-
-                    esp_app_desc_t running_app_info;
-                    if (esp_ota_get_partition_description(running, &running_app_info) == ESP_OK) {
-                        ESP_LOGI(TAG, "Running firmware version: %s", running_app_info.version);
-                    }
+        if (image_header_was_checked == false) {
+            esp_app_desc_t new_app_info;
+            if (data_read > sizeof(esp_image_header_t) + sizeof(esp_image_segment_header_t) + sizeof(esp_app_desc_t)) {
+                // check current version with downloading
+                memcpy(&new_app_info, &ota_write_data[sizeof(esp_image_header_t) + sizeof(esp_image_segment_header_t)], sizeof(esp_app_desc_t));
+                ESP_LOGI(TAG, "New firmware version: %s", new_app_info.version);
+
+                esp_app_desc_t running_app_info;
+                if (esp_ota_get_partition_description(running, &running_app_info) == ESP_OK) {
+                    ESP_LOGI(TAG, "Running firmware version: %s", running_app_info.version);
+                }
 
-                    const esp_partition_t* last_invalid_app = esp_ota_get_last_invalid_partition();
-                    esp_app_desc_t invalid_app_info;
-                    if (esp_ota_get_partition_description(last_invalid_app, &invalid_app_info) == ESP_OK) {
-                        ESP_LOGI(TAG, "Last invalid firmware version: %s", invalid_app_info.version);
-                    }
+                const esp_partition_t* last_invalid_app = esp_ota_get_last_invalid_partition();
+                esp_app_desc_t invalid_app_info;
+                if (esp_ota_get_partition_description(last_invalid_app, &invalid_app_info) == ESP_OK) {
+                    ESP_LOGI(TAG, "Last invalid firmware version: %s", invalid_app_info.version);
+                }
 
-                    // check current version with last invalid partition
-                    if (last_invalid_app != NULL) {
-                        if (memcmp(invalid_app_info.version, new_app_info.version, sizeof(new_app_info.version)) == 0) {
-                            LogFile.WriteToFile(ESP_LOG_WARN, TAG, "New version is the same as invalid version");
-                            LogFile.WriteToFile(ESP_LOG_WARN, TAG, "Previously, there was an attempt to launch the firmware with " + 
-                                    string(invalid_app_info.version) + " version, but it failed");
-                            LogFile.WriteToFile(ESP_LOG_WARN, TAG, "The firmware has been rolled back to the previous version");
-                            infinite_loop();
-                        }
+                // check current version with last invalid partition
+                if (last_invalid_app != NULL) {
+                    if (memcmp(invalid_app_info.version, new_app_info.version, sizeof(new_app_info.version)) == 0) {
+                        LogFile.WriteToFile(ESP_LOG_WARN, TAG, "New version is the same as invalid version");
+                        LogFile.WriteToFile(ESP_LOG_WARN, TAG, "Previously, there was an attempt to launch the firmware with " + string(invalid_app_info.version) + " version, but it failed");
+                        LogFile.WriteToFile(ESP_LOG_WARN, TAG, "The firmware has been rolled back to the previous version");
+                        return false;
                     }
+                }
 
-/*
-                    if (memcmp(new_app_info.version, running_app_info.version, sizeof(new_app_info.version)) == 0) {
-                        LogFile.WriteToFile(ESP_LOG_WARN, TAG, "Current running version is the same as a new. We will not continue the update");
-                        infinite_loop();
-                    }
-*/
-                    image_header_was_checked = true;
+                image_header_was_checked = true;
 
-                    err = esp_ota_begin(update_partition, OTA_SIZE_UNKNOWN, &update_handle);
-                    if (err != ESP_OK) {
-                        LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "esp_ota_begin failed (" + string(esp_err_to_name(err)) + ")");
-                        return false;
-                    }
-                    ESP_LOGI(TAG, "esp_ota_begin succeeded");
-                } else {
-                    LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "received package is not fit len");
+                err = esp_ota_begin(update_partition, OTA_SIZE_UNKNOWN, &update_handle);
+                if (err != ESP_OK) {
+                    LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "esp_ota_begin failed (" + string(esp_err_to_name(err)) + ")");
                     return false;
                 }
-            }            
-            err = esp_ota_write( update_handle, (const void *)ota_write_data, data_read);
-            if (err != ESP_OK) {
+                ESP_LOGI(TAG, "esp_ota_begin succeeded");
+            } 
+			else {
+                LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "received package is not fit len");
                 return false;
             }
-            binary_file_length += data_read;
-            ESP_LOGD(TAG, "Written image length %d", binary_file_length);
-        } else if (data_read == 0) {
-           //
-           // * As esp_http_client_read never returns negative error code, we rely on
-           // * `errno` to check for underlying transport connectivity closure if any
-           //
-            if (errno == ECONNRESET || errno == ENOTCONN) {
-                LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Connection closed, errno = " + to_string(errno));
-                break;
-            }
         }
+		
+        err = esp_ota_write( update_handle, (const void *)ota_write_data, data_read);
+        if (err != ESP_OK) {
+            return false;
+        }
+		
+        binary_file_length += data_read;
+        ESP_LOGD(TAG, "Written image length %d", binary_file_length);
+
         data_read = fread(ota_write_data, 1, SERVER_OTA_SCRATCH_BUFSIZE, f);
     }
     fclose(f);  
@@ -287,30 +245,22 @@ static bool ota_update_task(std::string fn)
         LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "esp_ota_set_boot_partition failed (" + string(esp_err_to_name(err)) + ")!");
 
     }
-//    ESP_LOGI(TAG, "Prepare to restart system!");
-//    esp_restart();
 
-    return true ;
+    return true;
 }
 
-
 static void print_sha256 (const uint8_t *image_hash, const char *label)
 {
     char hash_print[HASH_LEN * 2 + 1];
     hash_print[HASH_LEN * 2] = 0;
+	
     for (int i = 0; i < HASH_LEN; ++i) {
         sprintf(&hash_print[i * 2], "%02x", image_hash[i]);
     }
+	
     ESP_LOGI(TAG, "%s: %s", label, hash_print);
 }
 
-
-static bool diagnostic(void)
-{
-    return true;
-}
-
-
 void CheckOTAUpdate(void)
 {
     ESP_LOGI(TAG, "Start CheckOTAUpdateCheck...");
@@ -339,23 +289,21 @@ void CheckOTAUpdate(void)
     const esp_partition_t *running = esp_ota_get_running_partition();
     esp_ota_img_states_t ota_state;
     esp_err_t res_stat_partition = esp_ota_get_state_partition(running, &ota_state);
+	
     switch (res_stat_partition)
     {
         case ESP_OK:
             ESP_LOGD(TAG, "CheckOTAUpdate Partition: ESP_OK");
-            if (esp_ota_get_state_partition(running, &ota_state) == ESP_OK) {
-                if (ota_state == ESP_OTA_IMG_PENDING_VERIFY) {
-                    // run diagnostic function ...
-                    bool diagnostic_is_ok = diagnostic();
-                    if (diagnostic_is_ok) {
-                        ESP_LOGI(TAG, "Diagnostics completed successfully! Continuing execution...");
-                        esp_ota_mark_app_valid_cancel_rollback();
-                    } else {
-                        LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Diagnostics failed! Start rollback to the previous version...");
-                        esp_ota_mark_app_invalid_rollback_and_reboot();
-                    }
+            if (ota_state == ESP_OTA_IMG_PENDING_VERIFY) {
+                if (getSystemStatus() == 0) {
+                    ESP_LOGI(TAG, "Diagnostics completed successfully! Continuing execution...");
+                    esp_ota_mark_app_valid_cancel_rollback();
+                } 
+				else {
+                    LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Diagnostics failed! Start rollback to the previous version...");
+                    esp_ota_mark_app_invalid_rollback_and_reboot();
                 }
-            }            
+            }
             break;
         case ESP_ERR_INVALID_ARG:
             ESP_LOGD(TAG, "CheckOTAUpdate Partition: ESP_ERR_INVALID_ARG");
@@ -367,22 +315,8 @@ void CheckOTAUpdate(void)
             ESP_LOGD(TAG, "CheckOTAUpdate Partition: ESP_ERR_NOT_FOUND");
             break;
     }
-    if (esp_ota_get_state_partition(running, &ota_state) == ESP_OK) {
-        if (ota_state == ESP_OTA_IMG_PENDING_VERIFY) {
-            // run diagnostic function ...
-            bool diagnostic_is_ok = diagnostic();
-            if (diagnostic_is_ok) {
-                ESP_LOGI(TAG, "Diagnostics completed successfully! Continuing execution...");
-                esp_ota_mark_app_valid_cancel_rollback();
-            } else {
-                LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "Diagnostics failed! Start rollback to the previous version...");
-                esp_ota_mark_app_invalid_rollback_and_reboot();
-            }
-        }
-    }
 }
 
-
 esp_err_t handler_ota_update(httpd_req_t *req)
 {
 #ifdef DEBUG_DETAIL_ON     
@@ -462,12 +396,10 @@ esp_err_t handler_ota_update(httpd_req_t *req)
             return ESP_OK;
         }
 
-
         if ((filetype == "ZIP") || (filetype == "BIN"))
         {
-           	FILE *pfile;
             LogFile.WriteToFile(ESP_LOG_INFO, TAG, "Update for reboot");
-            pfile = fopen("/sdcard/update.txt", "w");
+            FILE *pfile = fopen("/sdcard/update.txt", "w");
             fwrite(fn.c_str(), fn.length(), 1, pfile);
             fclose(pfile);
 
@@ -475,36 +407,8 @@ esp_err_t handler_ota_update(httpd_req_t *req)
             httpd_resp_sendstr_chunk(req, zw.c_str());
             httpd_resp_sendstr_chunk(req, NULL);  
             ESP_LOGD(TAG, "Send reboot");
-            return ESP_OK;                
-
-        }
-
-/*
-        if (filetype == "BIN")
-        {
-            const char* resp_str; 
-
-            DeleteMainFlowTask();
-            gpio_handler_deinit();
-            if (ota_update_task(fn))
-            {
-                std::string zw = "reboot\n";
-                httpd_resp_sendstr_chunk(req, zw.c_str());
-                httpd_resp_sendstr_chunk(req, NULL);  
-                ESP_LOGD(TAG, "Send reboot");
-                return ESP_OK;                
-            }
-
-            resp_str = "Error during Firmware Update!!!\nPlease check output of console.";
-            httpd_resp_send(req, resp_str, strlen(resp_str));  
-
-            #ifdef DEBUG_DETAIL_ON 
-                LogFile.WriteHeapInfo("handler_ota_update - Done");    
-            #endif
-
             return ESP_OK;
         }
-*/
 
         std::string zw = "Update failed - no valid file specified (zip, bin, tfl, tlite)!";
         httpd_resp_sendstr_chunk(req, zw.c_str());
@@ -512,19 +416,17 @@ esp_err_t handler_ota_update(httpd_req_t *req)
         return ESP_OK;        
     }
 
-
     if (_task.compare("unziphtml") == 0)
     {
         ESP_LOGD(TAG, "Task unziphtml");
-        std::string in, out, zw;
 
-        in = "/sdcard/firmware/html.zip";
-        out = "/sdcard/html";
+        std::string in = "/sdcard/firmware/html.zip";
+        std::string out = "/sdcard/html";
 
         delete_all_in_directory(out);
 
         unzip(in, out+"/");
-        zw = "Web Interface Update Successfull!\nNo reboot necessary";
+        std::string zw = "Web Interface Update Successfull!\nNo reboot necessary";
         httpd_resp_send(req, zw.c_str(), strlen(zw.c_str()));
         httpd_resp_sendstr_chunk(req, NULL);  
         return ESP_OK;        
@@ -536,6 +438,7 @@ esp_err_t handler_ota_update(httpd_req_t *req)
         struct stat file_stat;
         int _result = stat(fn.c_str(), &file_stat);
         ESP_LOGD(TAG, "Ergebnis %d\n", _result);
+		
         if (_result == 0) {
             ESP_LOGD(TAG, "Deleting file: %s", fn.c_str());
             /* Delete file */
@@ -545,6 +448,7 @@ esp_err_t handler_ota_update(httpd_req_t *req)
         {
             LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "File does not exist: " + fn);
         }
+		
         /* Respond with an empty chunk to signal HTTP response completion */
         std::string zw = "file deleted\n";
         ESP_LOGD(TAG, "%s", zw.c_str());
@@ -553,30 +457,13 @@ esp_err_t handler_ota_update(httpd_req_t *req)
         return ESP_OK;
     }
 
-    string zw = "ota without parameter - should not be the case!";
+    std::string zw = "ota without parameter - should not be the case!";
     httpd_resp_set_hdr(req, "Access-Control-Allow-Origin", "*");
     httpd_resp_send(req, zw.c_str(), strlen(zw.c_str())); 
     httpd_resp_send_chunk(req, NULL, 0);  
 
     LogFile.WriteToFile(ESP_LOG_ERROR, TAG, "ota without parameter - should not be the case!");
 
-/*  
-    const char* resp_str;    
-
-    DeleteMainFlowTask();
-    gpio_handler_deinit();
-    if (ota_update_task(fn))
-    {
-        resp_str = "Firmware Update Successfull! You can restart now.";
-    }
-    else
-    {
-        resp_str = "Error during Firmware Update!!! Please check console output.";
-    }
-
-    httpd_resp_send(req, resp_str, strlen(resp_str));  
-*/
-
     #ifdef DEBUG_DETAIL_ON 
         LogFile.WriteHeapInfo("handler_ota_update - Done");    
     #endif
@@ -584,7 +471,6 @@ esp_err_t handler_ota_update(httpd_req_t *req)
     return ESP_OK;
 }
 
-
 void hard_restart() 
 {
   esp_task_wdt_config_t twdt_config = {
@@ -598,7 +484,6 @@ void hard_restart()
   while(true);
 }
 
-
 void task_reboot(void *DeleteMainFlow)
 {
     // write a reboot, to identify a reboot by purpouse
@@ -634,7 +519,6 @@ void task_reboot(void *DeleteMainFlow)
     vTaskDelete(NULL); //Delete this task if it comes to this point
 }
 
-
 void doReboot()
 {
     LogFile.WriteToFile(ESP_LOG_INFO, TAG, "Reboot triggered by Software (5s)");
@@ -649,7 +533,6 @@ void doReboot()
     vTaskDelay(10000 / portTICK_PERIOD_MS); // Prevent serving web client fetch response until system is shuting down
 }
 
-
 void doRebootOTA()
 {
     LogFile.WriteToFile(ESP_LOG_WARN, TAG, "Reboot in 5sec");
@@ -665,7 +548,6 @@ void doRebootOTA()
     hard_restart();     // Reset type: System reset (Triggered by watchdog), if esp_restart stalls (WDT needs to be activated)
 }
 
-
 esp_err_t handler_reboot(httpd_req_t *req)
 {
     #ifdef DEBUG_DETAIL_ON     
@@ -697,7 +579,6 @@ esp_err_t handler_reboot(httpd_req_t *req)
     return ESP_OK;
 }
 
-
 void register_server_ota_sdcard_uri(httpd_handle_t server)
 {
     ESP_LOGI(TAG, "Registering URI handlers");
@@ -714,5 +595,4 @@ void register_server_ota_sdcard_uri(httpd_handle_t server)
     camuri.handler = APPLY_BASIC_AUTH_FILTER(handler_reboot);
     camuri.user_ctx  = (void*) "Reboot";    
     httpd_register_uri_handler(server, &camuri);
-
 }