CONFIG_APP_ROLLBACK_ENABLE is not working

[jar-in]

I am working on a project where it would be very appreciated to use the possibility to rollback new sketch if there is any problem with it. It makes it possible to solve the problems with remote administration, because if the module stops communication with the administrator aftre the load of new sketch has been done and there is some errror in the sketch, then rolling back to the previous version makes the module remotely accesible.
Because this feature is part of esp-idf, and in the Arduino it is off by default, I made my own recompilation of esp-idf and tried to use it with Arduino. Note that I am using the latest version of Arduino. Unfortunately, the feature is not working. I created issue under esp-idf
espressif/esp-idf#4799
but the further investigation shows that the esp-idf part was not the problem.
I was working on it and found the file
cores/esp32/esp32-hal-misc.c
in Arduino.
There is the following code

void initArduino()
{
#ifdef CONFIG_APP_ROLLBACK_ENABLE
const esp_partition_t *running = esp_ota_get_running_partition();
esp_ota_img_states_t ota_state;
if (esp_ota_get_state_partition(running, &ota_state) == ESP_OK) {
if (ota_state == ESP_OTA_IMG_PENDING_VERIFY) {
if (verifyOta()) {
esp_ota_mark_app_valid_cancel_rollback();
} else {
log_e("OTA verification failed! Start rollback to the previous version ...");
esp_ota_mark_app_invalid_rollback_and_reboot();
}
}
}
#endif
and so on...
the function verifyOta returns simply true...
The line esp_ota_mark_app_valid_cancel_rollback(); confirms that the sketch is valid without any administrator intervention at the very first start of the sketch. Therefore, if the sketch code has error, and stops to communicate with remote site, it cannot be rolled back anymore.

I suggest to remove this part of Arduino initialization and left the validation to the user.
If the Arduino is designed not to use this feature, it would be better to have the underlying sdk precompiled without CONFIG_APP_ROLLBACK_ENABLE set. In such a way the Arduino will not use this new feature but it makes it more simple to use it if someone would want to use it by preparing new own recompiled esp-idf under the sdk directory. Thank you

[atanisoft]

verifyOta() is a weakly defined function that should be overridden by the application if it needs to verify the application prior to rollback.

[jar-in]

Seriously, I cannot imagine, how the administrator can verify the functionality of the sketch by any "verification" function. For example if the programmer makes mistake, and forget, for example, to start the web server for remote communication... it definitely cannot be verified by any prepared function in the initArduino... i.e. in the stage where the system did not pass the control neither to setup(), nor loop(). Also there is no solution to this to return false by verifyOta() as in this case the code continues by immediate restart and the new code is set as invalid, but the possibility to let the administrator decide later is not available. for ilustration I am using this code to mark the sketch as valid:
httpServer.on("/mark_app_valid", HTTP_GET, [](AsyncWebServerRequest * request) {
String htmlStr = "Version ";
htmlStr += version;
htmlStr += " marked as valid";
esp_ota_mark_app_valid_cancel_rollback();
request->send(200, "text/plain", htmlStr);
});
When the sketch is not marked as valid by the initArduino then if there is a problem which will be catched by WDT and WDT reboots the module, then it returs automatically to previous good version. If the WDT will not catch the problem, then user can have some other control that can power off the module and then power on. This also will cause return to previous good version of the application. Fortunately anyone can comment on this part in the library if needed. It only takes time to find why the feature is not working because I did not expected that Arduino code may validate the application at the stage where there is no idea if it will be good or not. Many thanks.

[atanisoft]

Your use-case is definitely not supportable with the current approach. Commenting the code each time there is a new version is also not a sustainable/supportable "fix". I would suggest submit a PR which moves this code out of initArduino() into it's own method that is weakly defined so you can override it from your code (to be a no-op) so you can mark the app as valid later via the web interface after everything is started.

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[stale]

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.