feat: Add esp_custom_part_ota component (IEC-195) #328

hmalpani · 2024-05-16T11:33:11Z

Add new component to support OTA updates for non-app type partitions in ESP-IDF

Checklist

Component contains License
Component contains README.md
Component contains idf_component.yml file with url field defined
Add Example
Add Component Unit tests
Component was added to upload job
Component was added to build job
Optional: Component contains unit tests
CI passing

Change description

Please describe your change here

esp_custom_part_ota/examples/custom_part_ota_example/CMakeLists.txt

esp_custom_part_ota/examples/custom_part_ota_example/main/CMakeLists.txt

esp_custom_part_ota/examples/custom_part_ota_example/main/non_app_ota_example.c

esp_custom_part_ota/include/esp_custom_part_ota.h

esp_custom_part_ota/src/esp_custom_part_ota.c

esp_custom_part_ota/examples/custom_part_ota_example/README.md

esp_custom_part_ota/src/esp_custom_part_ota.c

mahavirj · 2024-05-27T09:39:55Z

esp_custom_part_ota/src/esp_custom_part_ota.c

+        ESP_LOGI(TAG, "No backup present in the backup partition. Nothing to restore");
+        return ESP_OK;
+    }
+    if (handle->backup_len > 0) {


Could there be a scenario where this length is partial and not for full image? Do we need any image integrity checks (e.g., SHA sum) in the NVS?

Yes, the user can set the length of the data to be backed-up. I don't think we need any image integrity checks. We store the size of data in NVS. Also in case of any error, the APIs will return relevant error code.

mahavirj · 2024-05-29T13:07:06Z

esp_custom_part_ota/src/esp_custom_part_ota.c

+        return ESP_ERR_NO_MEM;
+    }
+
+    esp_err_t ret = esp_partition_erase_range(ctx->backup_partition, 0, ctx->backup_len);


Can we allow this API to be executed twice for 2 different partitions? If not, how can we detect if there exist a backup for any previous partition? Do we need some API to invalidate the previous backup here?

Oh right. I didn't think of this case. I think we can store some information in NVS regarding this and iterate over the data to check if some other backup is stored in the partition.

mahavirj · 2024-05-29T13:08:18Z

esp_custom_part_ota/src/esp_custom_part_ota.c

+static const char *TAG = "esp_custom_part_ota";
+
+#define BACKUP_STORAGE_NAMESPACE "esp_custom_ota"
+#define BACKUP_STORAGE_DATA_LEN "backup_len"


Would it help if we make this variable partition specific:

snprintf(str, size, "%s_backup_len", part_name)

This component can be used for performing OTA upgrades for custom partitions in ESP-IDF

AdityaHPatwardhan · 2024-08-20T14:19:38Z

esp_custom_part_ota/README.md

@@ -0,0 +1,43 @@
+# ESP Non-App OTA Updates
+
+This component provides an API interface to update the Non-App type partitions using ESP_IDF.


Suggested change

This component provides an API interface to update the Non-App type partitions using ESP_IDF.

This component provides an API interface to update the Non-App type partitions using ESP-IDF.

AdityaHPatwardhan · 2024-08-20T14:20:35Z

esp_custom_part_ota/README.md

+
+`esp_custom_part_ota` component supports OTA update using two schemes:
+
+### Scheme 1


It would be better to provide a name for these schemes rather than just calling them Scheme 1 and Scheme 2

given name as custom_part_ota_backup and custom_part_ota

AdityaHPatwardhan · 2024-08-20T14:21:23Z

esp_custom_part_ota/README.md

+Here the non-app partition which we are trying to update is spiffs. When the user wants to update the spiffs partition, the workflow will be as follows: 
+
+* Consider that ota_0 is the current running partition. 
+* Copy the data from the spiffs partition to the backup partition. 


We should first explain what the backup partition is.

added this line (backup partition will be always ota passive partition), backup word.

AdityaHPatwardhan · 2024-08-20T14:23:56Z

esp_custom_part_ota/README.md

+Backup partition can be specified using `backup_partition` field in `esp_custom_part_ota_cfg_t` structure. If no backup partition is specified, the passive app partition will be set as the backup partition by default.
+
+### Scheme 2
+


It would be better if we provide some clear reasoning for when to choose scheme 1 and when to use scheme 2.
One might be as follows

There is limited flash available on the chip and no backup partition can be assigned for the OTA process. In that case scheme 2 might be helpful

This is unsafe and can result in loss of data if the OTA is interrupted.

AdityaHPatwardhan · 2024-08-20T14:30:56Z

esp_custom_part_ota/examples/custom_part_ota_example/main/custom_part_ota_example.c

+    esp_http_client_cleanup(client);
+}
+
+static void custom_part_ota_example_task(void *pvParameter)


This is question more related to the structure of the component itself.
Considering this is the core logic of the component which performs the actual OTA process. Should this be located in the src directory rather than the shown as an example?
I would prefer if simple APIs are provided to the user just to perform the OTA process.

AdityaHPatwardhan · 2024-08-20T14:35:20Z

esp_custom_part_ota/examples/custom_part_ota_example/main/custom_part_ota_example.c

+        esp_http_client_cleanup(client);
+        task_fatal_error();
+    }
+    esp_http_client_fetch_headers(client);


Should we take backup of the partition here after ensuring the ability to perform OTA has been confirmed?

We should also add the check here that the data length given to us by the server fits in the partition to be updated.

added checks which ensures whether the firmware length is greater than partition length

AdityaHPatwardhan · 2024-08-20T14:58:10Z

esp_custom_part_ota/src/esp_custom_part_ota.c

+        ret = nvs_set_u32(backup_info_handle, BACKUP_STORAGE_DATA_LEN, 0);
+        if (ret != ESP_OK) {
+            ESP_LOGE(TAG, "Failed to store backup information: %s", esp_err_to_name(ret));
+            return ret;


I was wondering if we should give an option to explicitly clear the backup information in the NVS

AdityaHPatwardhan · 2024-08-20T14:59:09Z

esp_custom_part_ota/src/esp_custom_part_ota.c

+            ESP_LOGE(TAG, "Failed to store backup information: %s", esp_err_to_name(ret));
+            return ret;
+        }
+        ret = nvs_set_u32(backup_info_handle, BACKUP_STORAGE_DATA_LEN, 0);


Should we also store the name of the backup partition?

AdityaHPatwardhan · 2024-09-04T11:10:14Z

esp_custom_part_ota/src/esp_custom_part_ota.c

+        return ESP_ERR_INVALID_ARG;
+    }
+    esp_custom_part_ota_t *ctx = (esp_custom_part_ota_t *)handle;
+    free(ctx);


maybe we should update the NVS entry at this point

hmalpani marked this pull request as draft May 16, 2024 11:33

hmalpani force-pushed the feat/non_app_ota_update branch 2 times, most recently from a14c6c1 to 0bac912 Compare May 23, 2024 07:05

hmalpani changed the title ~~feat: Add esp_non_app_ota component~~ feat: Add esp_custom_part_ota component May 23, 2024

hmalpani force-pushed the feat/non_app_ota_update branch 2 times, most recently from 3551754 to a337719 Compare May 24, 2024 08:57

mahavirj reviewed May 27, 2024

View reviewed changes

hmalpani force-pushed the feat/non_app_ota_update branch 2 times, most recently from 6f3f35c to 553bc57 Compare May 29, 2024 10:22

hmalpani mentioned this pull request May 29, 2024

reusing the OTA update facility to update the SPIFFS partition (IDFGH-11309) espressif/esp-idf#12462

Closed

mahavirj reviewed May 29, 2024

View reviewed changes

feat: Add esp_custom_part_ota component

7a712e4

This component can be used for performing OTA upgrades for custom partitions in ESP-IDF

hmalpani force-pushed the feat/non_app_ota_update branch from 553bc57 to 7a712e4 Compare June 5, 2024 09:07

AdityaHPatwardhan reviewed Aug 20, 2024

View reviewed changes

hrushikesh430 mentioned this pull request Aug 23, 2024

Feat/non_app_ota_update #366

Open

6 tasks

AdityaHPatwardhan reviewed Sep 4, 2024

View reviewed changes

espressif-bot added the Status: Opened label Oct 14, 2024

github-actions bot changed the title ~~feat: Add esp_custom_part_ota component~~ feat: Add esp_custom_part_ota component (IEC-195) Oct 14, 2024

espressif-bot assigned hrushikesh430 Oct 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add esp_custom_part_ota component (IEC-195) #328

feat: Add esp_custom_part_ota component (IEC-195) #328

hmalpani commented May 16, 2024 •

edited

Loading

mahavirj May 27, 2024

hmalpani May 29, 2024

mahavirj May 29, 2024

hmalpani May 31, 2024

mahavirj May 29, 2024 •

edited

Loading

AdityaHPatwardhan Aug 20, 2024

hrushikesh430 Aug 26, 2024

AdityaHPatwardhan Aug 20, 2024

hrushikesh430 Aug 26, 2024

AdityaHPatwardhan Aug 20, 2024

hrushikesh430 Aug 26, 2024

AdityaHPatwardhan Aug 20, 2024

AdityaHPatwardhan Aug 20, 2024

AdityaHPatwardhan Aug 20, 2024

AdityaHPatwardhan Aug 20, 2024

hrushikesh430 Aug 26, 2024

AdityaHPatwardhan Aug 20, 2024

AdityaHPatwardhan Aug 20, 2024

AdityaHPatwardhan Sep 4, 2024

		@@ -0,0 +1,43 @@
		# ESP Non-App OTA Updates

		This component provides an API interface to update the Non-App type partitions using ESP_IDF.


		`esp_custom_part_ota` component supports OTA update using two schemes:

		### Scheme 1

		Backup partition can be specified using `backup_partition` field in `esp_custom_part_ota_cfg_t` structure. If no backup partition is specified, the passive app partition will be set as the backup partition by default.

		### Scheme 2

feat: Add esp_custom_part_ota component (IEC-195) #328

Are you sure you want to change the base?

feat: Add esp_custom_part_ota component (IEC-195) #328

Conversation

hmalpani commented May 16, 2024 • edited Loading

Checklist

Change description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahavirj May 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hmalpani commented May 16, 2024 •

edited

Loading

mahavirj May 29, 2024 •

edited

Loading