mirror of
https://github.com/esphome/esphome.git
synced 2026-09-13 00:07:32 +00:00
[ota] Rename copy_source_part -> copy_dest_part and harden running-app lookup
The PartitionTablePlan field that drives the optional app copy stores the partition that will receive the running app (i.e. the destination in the current table at the new slot's flash offset). The old name ``copy_source_part`` described the value backwards and risked future callers swapping the esp_partition_copy() arguments; renamed to ``copy_dest_part`` along with the matching local ``app_copy_dest_part`` and the comments around them. Also replace esp_ota_get_running_partition() in the copy path with find_app_partition_at(running_app_offset, running_app_size). The IDF call can return nullptr after a prior aborted partition-table OTA in the same boot called esp_partition_unload_all() (the same condition the cache in get_running_app_position() exists for); the previous code would have dereferenced nullptr on the retry that the client error message explicitly suggests. No functional change on the success path; nullptr deref on the failed- retry path is now reported as OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE.
This commit is contained in:
@@ -27,11 +27,13 @@ class IDFOTABackend final {
|
||||
|
||||
protected:
|
||||
#ifdef USE_OTA_PARTITIONS
|
||||
// copy_source_part non-null means the running app must be copied from this slot in the current
|
||||
// table into target_app_index in the new table before the table is committed.
|
||||
// copy_dest_part non-null means the running app must be copied INTO this slot of the current
|
||||
// table before the new partition table is committed. The destination is in the current table
|
||||
// because that's where esp_partition_copy can write; once the new table replaces it, the same
|
||||
// flash region becomes target_app_index in the new table.
|
||||
struct PartitionTablePlan {
|
||||
int target_app_index{-1};
|
||||
const esp_partition_t *copy_source_part{nullptr};
|
||||
const esp_partition_t *copy_dest_part{nullptr};
|
||||
};
|
||||
|
||||
OTAResponseTypes validate_new_partition_table_(uint32_t running_app_offset, size_t running_app_size,
|
||||
|
||||
@@ -108,7 +108,7 @@ OTAResponseTypes IDFOTABackend::validate_new_partition_table_(uint32_t running_a
|
||||
int app_partitions_found = 0;
|
||||
int new_app_part_index = -1;
|
||||
int new_app_part_index_with_copy = -1;
|
||||
const esp_partition_t *app_copy_source_part = nullptr;
|
||||
const esp_partition_t *app_copy_dest_part = nullptr;
|
||||
bool otadata_partition_found = false;
|
||||
bool otadata_overlap = false;
|
||||
bool nvs_partition_found = false;
|
||||
@@ -123,11 +123,12 @@ OTAResponseTypes IDFOTABackend::validate_new_partition_table_(uint32_t running_a
|
||||
}
|
||||
} else if (new_app_part_index_with_copy == -1 &&
|
||||
!check_overlap(running_app_offset, running_app_size, new_part->pos.offset, running_app_size)) {
|
||||
// esp_partition_copy needs a registered source partition in the current table.
|
||||
// esp_partition_copy writes into a registered partition; need one at this offset in the
|
||||
// current table.
|
||||
const esp_partition_t *p = find_app_partition_at(new_part->pos.offset, running_app_size);
|
||||
if (p != nullptr) {
|
||||
new_app_part_index_with_copy = i;
|
||||
app_copy_source_part = p;
|
||||
app_copy_dest_part = p;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -165,10 +166,10 @@ OTAResponseTypes IDFOTABackend::validate_new_partition_table_(uint32_t running_a
|
||||
|
||||
if (new_app_part_index != -1) {
|
||||
plan.target_app_index = new_app_part_index;
|
||||
plan.copy_source_part = nullptr;
|
||||
plan.copy_dest_part = nullptr;
|
||||
} else {
|
||||
plan.target_app_index = new_app_part_index_with_copy;
|
||||
plan.copy_source_part = app_copy_source_part;
|
||||
plan.copy_dest_part = app_copy_dest_part;
|
||||
}
|
||||
return OTA_RESPONSE_OK;
|
||||
}
|
||||
@@ -208,15 +209,18 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
|
||||
esp_err_t err;
|
||||
const esp_partition_info_t *new_partition_table = reinterpret_cast<const esp_partition_info_t *>(this->buf_);
|
||||
|
||||
// esp_ota_get_running_partition() is still valid here (esp_partition_unload_all() has not run)
|
||||
// so use it directly instead of repeating the iterator walk.
|
||||
if (plan.copy_source_part != nullptr) {
|
||||
const esp_partition_t *running_app_part = esp_ota_get_running_partition();
|
||||
if (plan.copy_dest_part != nullptr) {
|
||||
// Resolve the source via running_app_offset rather than esp_ota_get_running_partition() in
|
||||
// case a prior aborted partition-table OTA called esp_partition_unload_all() in this boot,
|
||||
// which leaves esp_ota_get_running_partition() returning nullptr.
|
||||
const esp_partition_t *running_app_part = find_app_partition_at(running_app_offset, running_app_size);
|
||||
if (running_app_part == nullptr) {
|
||||
ESP_LOGE(TAG, "Cannot resolve running app partition at offset 0x%X", running_app_offset);
|
||||
return OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE;
|
||||
}
|
||||
ESP_LOGD(TAG, "Copying running app from 0x%X to 0x%X (size: 0x%X)", running_app_part->address,
|
||||
plan.copy_source_part->address, running_app_size);
|
||||
|
||||
err = esp_partition_copy(plan.copy_source_part, 0, running_app_part, 0, running_app_size);
|
||||
|
||||
plan.copy_dest_part->address, running_app_size);
|
||||
err = esp_partition_copy(plan.copy_dest_part, 0, running_app_part, 0, running_app_size);
|
||||
if (err != ESP_OK) {
|
||||
ESP_LOGE(TAG, "esp_partition_copy failed (err=0x%X)", err);
|
||||
return OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE;
|
||||
|
||||
Reference in New Issue
Block a user