feat: Add esp_custom_part_ota component #328
Conversation
hmalpani
force-pushed
the
feat/non_app_ota_update
branch
2 times, most recently
from
a14c6c1
to
0bac912
Compare
hmalpani
changed the title
hmalpani
force-pushed
the
feat/non_app_ota_update
branch
2 times, most recently
from
3551754
to
a337719
Compare
esp_custom_part_ota/examples/custom_part_ota_example/CMakeLists.txt
Outdated
Show resolved
Hide resolved
esp_custom_part_ota/examples/custom_part_ota_example/main/CMakeLists.txt
Outdated
Show resolved
Hide resolved
esp_custom_part_ota/examples/custom_part_ota_example/main/non_app_ota_example.c
Outdated
Show resolved
Hide resolved
| * - ESP_OK | ||
| * - ESP_ERR_INVALID_ARG | ||
| */ | ||
| esp_err_t esp_custom_part_ota_set_backup_partition(esp_custom_part_ota_handle_t ctx, const esp_partition_t *backup_partition); |
There was a problem hiding this comment.
This can be done in the begin API too, or this API has some special purpose?
There was a problem hiding this comment.
Yes, it seems redundant. I will remove this API
|
|
||
| // Store backup data information in the default NVS partition. | ||
| nvs_handle_t backup_info_handle = 0; | ||
| ret = nvs_open(BACKUP_STORAGE_NAMESPACE, NVS_READWRITE, &backup_info_handle); |
There was a problem hiding this comment.
If the power goes at this moment then backup length is not yet stored and the primary partition is already erased. So this will result in data loss?
There was a problem hiding this comment.
Yes, it would have resulted in data loss. I have updated the code to first update the backup info in NVS and then erase the partition.
|
|
||
| Custom Data Partition OTA aims at enabling Over-the-Air data update for custom data partitions in ESP-IDF. | ||
|
|
||
| ## ESP Custom Partition OTA Abstraction Layer |
There was a problem hiding this comment.
This example uses which scheme, should we mention about it here?
There was a problem hiding this comment.
I have added a config option to enable/disable the backing-up of the data
| ESP_LOGI(TAG, "No backup present in the backup partition. Nothing to restore"); | ||
| return ESP_OK; | ||
| } | ||
| if (handle->backup_len > 0) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
hmalpani
force-pushed
the
feat/non_app_ota_update
branch
from
a337719
to
6f3f35c
Compare
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
6f3f35c
to
553bc57
Compare
| return ESP_ERR_NO_MEM; | ||
| } | ||
|
|
||
| esp_err_t ret = esp_partition_erase_range(ctx->backup_partition, 0, ctx->backup_len); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| static const char *TAG = "esp_custom_part_ota"; | ||
|
|
||
| #define BACKUP_STORAGE_NAMESPACE "esp_custom_ota" | ||
| #define BACKUP_STORAGE_DATA_LEN "backup_len" |
There was a problem hiding this comment.
Would it help if we make this variable partition specific:
snprintf(str, size, "%s_backup_len", part_name)
Add new component to support OTA updates for non-app type partitions in ESP-IDF
Checklist
urlfield definedChange description
Please describe your change here