Mirrors_Framework/esphome
1
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2026-05-20 01:16:26 +08:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

[ota] Polish partition-table review feedback

Browse Source 
Split the combined "missing app, otadata, or nvs" verify failure into
three separate ESP_LOGE messages so users can see which check failed,
trim trailing spaces from the (err=0x%X) log strings, and document why
the partition-table espota2 test mocks SERVER_FEATURE_SUPPORTS_COMPRESSION
(intentional protocol-path coverage; the real IDFOTABackend never sets it).
This commit is contained in:
J. Nick Koston
2026-05-03 09:17:40 -05:00
parent 51c5500809
commit b788529ad7
2 changed files with 28 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
esphome/components/ota/ota_backend_esp_idf.cpp
+19 -11
View File
@@ -208,7 +208,7 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
nullptr, ESP_PRIMARY_PARTITION_TABLE_OFFSET, ESP_PARTITION_TABLE_SIZE, "PrimaryPrtTable",
ESP_PARTITION_TYPE_PARTITION_TABLE, ESP_PARTITION_SUBTYPE_PARTITION_TABLE_PRIMARY, &this->partition_table_part_);
if (err != ESP_OK) {
ESP_LOGE(TAG, "esp_partition_register_external failed (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_partition_register_external failed (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
@@ -218,13 +218,13 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
err = esp_partition_mmap(this->partition_table_part_, 0, ESP_PARTITION_TABLE_MAX_LEN, ESP_PARTITION_MMAP_DATA,
reinterpret_cast<const void **>(&existing_partition_table), &partition_table_map);
if (err != ESP_OK) {
ESP_LOGE(TAG, "esp_partition_mmap failed (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_partition_mmap failed (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
err = esp_partition_table_verify(existing_partition_table, true, &num_partitions);
esp_partition_munmap(partition_table_map);
if (err != ESP_OK) {
ESP_LOGE(TAG, "esp_partition_table_verify failed (existing partition table) (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_partition_table_verify failed (existing partition table) (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
@@ -233,7 +233,7 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
// esp_partition_table_verify expects ESP_PARTITION_TABLE_MAX_LEN bytes of data
err = esp_partition_table_verify(new_partition_table, true, &num_partitions);
if (err != ESP_OK) {
ESP_LOGE(TAG, "esp_partition_table_verify failed (new partition table) (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_partition_table_verify failed (new partition table) (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
// Check for missing checksum
@@ -303,8 +303,16 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
ESP_LOGE(TAG, "No compatible app partition found in the new partition table");
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
if (app_partitions_found < 2 || !otadata_partition_found || !nvs_partition_found) {
ESP_LOGE(TAG, "New partition table is missing the required app, otadata or nvs partitions");
if (app_partitions_found < 2) {
ESP_LOGE(TAG, "New partition table needs at least 2 app partitions, found %d", app_partitions_found);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
if (!otadata_partition_found) {
ESP_LOGE(TAG, "New partition table is missing the required otadata partition");
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
if (!nvs_partition_found) {
ESP_LOGE(TAG, "New partition table is missing the required nvs partition");
return OTA_RESPONSE_ERROR_PARTITION_TABLE_VERIFY;
}
if (otadata_overlap) {
@@ -337,7 +345,7 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
err = esp_partition_copy(app_copy_target_part, 0, running_app_part, 0, running_app_size);
if (err != ESP_OK) {
ESP_LOGE(TAG, "esp_partition_copy failed (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_partition_copy failed (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE;
}
}
@@ -352,7 +360,7 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
if (err != ESP_OK) {
esp_ota_abort(this->update_handle_);
this->update_handle_ = 0;
ESP_LOGE(TAG, "esp_ota_begin failed (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_ota_begin failed (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE;
}
err = esp_ota_write(this->update_handle_, this->buf_, ESP_PARTITION_TABLE_MAX_LEN);
@@ -361,13 +369,13 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
// partial-write failure path self-contained.
esp_ota_abort(this->update_handle_);
this->update_handle_ = 0;
ESP_LOGE(TAG, "esp_ota_write failed (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_ota_write failed (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE;
}
err = esp_ota_end(this->update_handle_);
this->update_handle_ = 0; // esp_ota_end releases the handle internally regardless of result
if (err != ESP_OK) {
ESP_LOGE(TAG, "esp_ota_end failed (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_ota_end failed (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE;
}
// esp_partition_unload_all() invalidates every cached partition entry, including the externally
@@ -388,7 +396,7 @@ OTAResponseTypes IDFOTABackend::update_partition_table() {
ESP_LOGD(TAG, "Setting next boot partition to 0x%X", new_boot_partition->address);
err = esp_ota_set_boot_partition(new_boot_partition);
if (err != ESP_OK) {
ESP_LOGE(TAG, "esp_ota_set_boot_partition failed (err=0x%X) ", err);
ESP_LOGE(TAG, "esp_ota_set_boot_partition failed (err=0x%X)", err);
return OTA_RESPONSE_ERROR_PARTITION_TABLE_UPDATE;
}
return OTA_RESPONSE_OK;
tests/unit_tests/test_espota2.py
+9 -2
View File
@@ -843,7 +843,14 @@ def test_perform_ota_extended_protocol_app(
def test_perform_ota_successful_partition_table(
mock_socket: Mock, mock_file: io.BytesIO
) -> None:
"""Test OTA partition table update."""
"""Test OTA partition table update.
The mocked server advertises both COMPRESSION and PARTITION_ACCESS to exercise
the full extended-protocol negotiation path. Real IDFOTABackend devices return
``supports_compression() == false`` and never set the COMPRESSION flag for a
partition-table OTA; the flag here is intentional protocol-coverage, not a
description of on-device behaviour.
"""
recv_responses = [
bytes([espota2.RESPONSE_OK]), # First byte of version response
bytes([espota2.OTA_VERSION_2_0]), # Version number
@@ -853,7 +860,7 @@ def test_perform_ota_successful_partition_table(
espota2.SERVER_FEATURE_SUPPORTS_COMPRESSION
| espota2.SERVER_FEATURE_SUPPORTS_PARTITION_ACCESS
]
), # Device feature flags
), # Device feature flags (compression flag is unrealistic; see docstring)
bytes([espota2.RESPONSE_AUTH_OK]), # No auth required
bytes([espota2.RESPONSE_UPDATE_PREPARE_OK]), # Binary size OK
bytes([espota2.RESPONSE_BIN_MD5_OK]), # MD5 checksum OK