From b103df1bb1e57175fcd632521f645096538d69fd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:33:24 -0500 Subject: [PATCH 01/13] Bump ruff from 0.16.3 to 0.16.4 (#18734) Signed-off-by: dependabot[bot] --- requirements_test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_test.txt b/requirements_test.txt index 079c375c01..cf4b028b0e 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,6 +1,6 @@ pylint==4.0.7 flake8==7.3.0 # also change in .pre-commit-config.yaml when updating -ruff==0.16.3 # also change in .pre-commit-config.yaml when updating +ruff==0.16.4 # also change in .pre-commit-config.yaml when updating pyupgrade==3.21.2 # also change in .pre-commit-config.yaml when updating prek==0.4.14 # also change in .github/workflows/ci.yml when updating From 32a0a5c55cb8a9c5b392272d2d04b3dbf40401f4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:33:39 -0500 Subject: [PATCH 02/13] Bump github/codeql-action/analyze from 4.37.7 to 4.37.8 (#18735) Signed-off-by: dependabot[bot] --- .github/workflows/codeql.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 103cecc1f9..46eed02656 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -84,6 +84,6 @@ jobs: exit 1 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7 + uses: github/codeql-action/analyze@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8 with: category: "/language:${{matrix.language}}" From ad508d79afdb578503ae7768b8d6bd14fd0474c4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:33:52 -0500 Subject: [PATCH 03/13] Bump github/codeql-action/init from 4.37.7 to 4.37.8 (#18736) Signed-off-by: dependabot[bot] --- .github/workflows/codeql.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 46eed02656..b46f9adab6 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -56,7 +56,7 @@ jobs: # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7 + uses: github/codeql-action/init@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8 with: languages: ${{ matrix.language }} build-mode: ${{ matrix.build-mode }} From 3b6c6e3dadf6edb8902feb72de945d30ea8e47a6 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:50:35 -0500 Subject: [PATCH 04/13] Bump aioesphomeapi from 46.0.0 to 46.1.0 (#18737) --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 822eebc1f2..c10ddc3f53 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ pyserial==3.5 platformio==6.1.19 esptool==5.3.1 click==8.3.3 -aioesphomeapi==46.0.0 +aioesphomeapi==46.1.0 aiohappyeyeballs==2.7.1 # Happy Eyeballs for requests downloads; already pulled in by aioesphomeapi zeroconf==0.150.0 puremagic==2.2.0 From ca9636514362b5f270dc98c896d04c9da98edb00 Mon Sep 17 00:00:00 2001 From: guillempages Date: Mon, 24 Aug 2026 22:07:01 +0200 Subject: [PATCH 05/13] [runtime_image] Add MIME types to image formats (#16361) Co-authored-by: J. Nick Koston Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: J. Nick Koston --- .../components/online_image/online_image.cpp | 62 +++++-------------- .../components/runtime_image/image_format.cpp | 44 +++++++++++++ .../components/runtime_image/image_format.h | 7 +++ .../runtime_image/runtime_image.cpp | 1 - .../runtime_image/test_mime_types.cpp | 61 ++++++++++++++++++ 5 files changed, 127 insertions(+), 48 deletions(-) create mode 100644 esphome/components/runtime_image/image_format.cpp create mode 100644 tests/components/runtime_image/test_mime_types.cpp diff --git a/esphome/components/online_image/online_image.cpp b/esphome/components/online_image/online_image.cpp index a2662ff0e3..3f2382accd 100644 --- a/esphome/components/online_image/online_image.cpp +++ b/esphome/components/online_image/online_image.cpp @@ -3,6 +3,7 @@ #include "esphome/core/helpers.h" #include "esphome/core/log.h" #include +#include static const char *const TAG = "online_image"; static const char *const CONTENT_TYPE_HEADER_NAME = "content-type"; @@ -62,30 +63,11 @@ void OnlineImage::update() { headers.push_back({IF_MODIFIED_SINCE_HEADER_NAME, this->last_modified_}); } - // Add Accept header based on image format - const char *accept_mime_type; runtime_image::ImageFormat format = this->get_format(); - switch (format) { -#ifdef USE_RUNTIME_IMAGE_BMP - case runtime_image::BMP: - accept_mime_type = "image/bmp,*/*;q=0.8"; - break; -#endif -#ifdef USE_RUNTIME_IMAGE_JPEG - case runtime_image::JPEG: - accept_mime_type = "image/jpeg,*/*;q=0.8"; - break; -#endif -#ifdef USE_RUNTIME_IMAGE_PNG - case runtime_image::PNG: - accept_mime_type = "image/png,*/*;q=0.8"; - break; -#endif - default: - accept_mime_type = "image/*,*/*;q=0.8"; - break; - } - headers.push_back({"Accept", accept_mime_type}); + // Accept: ",*/*;q=0.8"; 32 covers the longest MIME type plus the suffix + char accept_header[32]; + snprintf(accept_header, sizeof(accept_header), "%s,*/*;q=0.8", runtime_image::get_mime_type_for_format(format)); + headers.push_back({"Accept", accept_header}); // User headers last so they can override any of the above for (auto &header : this->request_headers_) { @@ -122,32 +104,18 @@ void OnlineImage::update() { if (format == runtime_image::AUTO) { // Try to auto-detect format from Content-Type header - auto content_type_header = this->downloader_->get_response_header(CONTENT_TYPE_HEADER_NAME); - const char *content_type = content_type_header.c_str(); - ESP_LOGV(TAG, "Content-Type: %s", content_type); - // Includes aliases seen from real servers (older IIS, CDNs, S3) - if (str_contains_ignore_case(content_type, "image/bmp") || - str_contains_ignore_case(content_type, "image/x-ms-bmp") || - str_contains_ignore_case(content_type, "image/x-bmp")) { - format = runtime_image::BMP; - } else if (str_contains_ignore_case(content_type, "image/jpeg") || - str_contains_ignore_case(content_type, "image/jpg")) { - format = runtime_image::JPEG; - } else if (str_contains_ignore_case(content_type, "image/png") || - str_contains_ignore_case(content_type, "image/x-png")) { - format = runtime_image::PNG; - } else if (str_contains_ignore_case(content_type, "image/")) { - ESP_LOGW(TAG, "Unsupported image type: '%s'", content_type); - this->end_connection_(); - this->download_error_callback_.call(); - return; + auto content_type = this->downloader_->get_response_header(CONTENT_TYPE_HEADER_NAME); + ESP_LOGV(TAG, "Content-Type: %s", content_type.c_str()); + auto mime_format = esphome::runtime_image::get_format_for_mime_type(content_type.c_str()); + if (mime_format.has_value()) { + format = *mime_format; } else { - // TODO: implement auto-detection in runtime_image by sniffing the first few bytes of the image data - if (content_type_header.empty()) { - ESP_LOGW(TAG, "Server sent no Content-Type header; cannot determine image format. Set `format:` explicitly"); + if (content_type.empty()) { + ESP_LOGE(TAG, "Server sent no Content-Type header; cannot determine image format. Set `format:` explicitly"); + } else if (str_contains_ignore_case(content_type.c_str(), "image/")) { + ESP_LOGE(TAG, "Image format '%s' not supported.", content_type.c_str()); } else { - ESP_LOGE(TAG, "Could not determine image format from Content-Type: '%s'. Set `format:` explicitly", - content_type); + ESP_LOGE(TAG, "Server did not return an image (Content-Type: '%s')", content_type.c_str()); } this->end_connection_(); this->download_error_callback_.call(); diff --git a/esphome/components/runtime_image/image_format.cpp b/esphome/components/runtime_image/image_format.cpp new file mode 100644 index 0000000000..3ba8871862 --- /dev/null +++ b/esphome/components/runtime_image/image_format.cpp @@ -0,0 +1,44 @@ +#include "esphome/core/helpers.h" +#include "image_format.h" +#include "image_decoder.h" + +namespace esphome::runtime_image { + +struct MimeLookup { + const char *mime_type; + ImageFormat format; +}; + +// The first entry per format is its canonical MIME type; the rest are aliases +// seen from real servers (older IIS, CDNs, S3) +static constexpr MimeLookup MIME_LOOKUP_TABLE[] = { +#ifdef USE_RUNTIME_IMAGE_BMP + {"image/bmp", ImageFormat::BMP}, {"image/x-ms-bmp", ImageFormat::BMP}, {"image/x-bmp", ImageFormat::BMP}, +#endif +#ifdef USE_RUNTIME_IMAGE_JPEG + {"image/jpeg", ImageFormat::JPEG}, {"image/jpg", ImageFormat::JPEG}, +#endif +#ifdef USE_RUNTIME_IMAGE_PNG + {"image/png", ImageFormat::PNG}, {"image/x-png", ImageFormat::PNG}, +#endif +}; + +const char *get_mime_type_for_format(ImageFormat format) { + for (const auto &entry : MIME_LOOKUP_TABLE) { + if (entry.format == format) { + return entry.mime_type; + } + } + return "image/*"; // AUTO or compiled-out format +} + +std::optional get_format_for_mime_type(const char *mime_type) { + for (const auto &entry : MIME_LOOKUP_TABLE) { + if (str_contains_ignore_case(mime_type, entry.mime_type)) { + return entry.format; + } + } + return std::nullopt; +} + +} // namespace esphome::runtime_image diff --git a/esphome/components/runtime_image/image_format.h b/esphome/components/runtime_image/image_format.h index ca6e0782b9..aff0c026b9 100644 --- a/esphome/components/runtime_image/image_format.h +++ b/esphome/components/runtime_image/image_format.h @@ -1,5 +1,7 @@ #pragma once +#include + namespace esphome::runtime_image { /** @@ -17,4 +19,9 @@ enum ImageFormat { BMP, }; +/// Canonical MIME type for a format; "image/*" for AUTO/unknown +const char *get_mime_type_for_format(ImageFormat format); +/// Case-insensitive substring match of known media types; nullopt if none found +std::optional get_format_for_mime_type(const char *mime_type); + } // namespace esphome::runtime_image diff --git a/esphome/components/runtime_image/runtime_image.cpp b/esphome/components/runtime_image/runtime_image.cpp index 254624caf4..f7417c2c8e 100644 --- a/esphome/components/runtime_image/runtime_image.cpp +++ b/esphome/components/runtime_image/runtime_image.cpp @@ -1,7 +1,6 @@ #include "runtime_image.h" #include "image_decoder.h" #include "esphome/core/log.h" -#include "esphome/core/helpers.h" #include #include #include diff --git a/tests/components/runtime_image/test_mime_types.cpp b/tests/components/runtime_image/test_mime_types.cpp new file mode 100644 index 0000000000..de8bbc76be --- /dev/null +++ b/tests/components/runtime_image/test_mime_types.cpp @@ -0,0 +1,61 @@ +#include + +#include + +#include "esphome/components/runtime_image/runtime_image.h" + +namespace esphome::runtime_image::testing { + +TEST(RuntimeImageMime, FormatForKnownMimeTypes) { + EXPECT_EQ(get_format_for_mime_type("image/bmp"), BMP); + EXPECT_EQ(get_format_for_mime_type("image/x-ms-bmp"), BMP); + EXPECT_EQ(get_format_for_mime_type("image/x-bmp"), BMP); + EXPECT_EQ(get_format_for_mime_type("image/png"), PNG); + EXPECT_EQ(get_format_for_mime_type("image/x-png"), PNG); +#ifdef USE_RUNTIME_IMAGE_JPEG + EXPECT_EQ(get_format_for_mime_type("image/jpeg"), JPEG); + EXPECT_EQ(get_format_for_mime_type("image/jpg"), JPEG); +#endif // USE_RUNTIME_IMAGE_JPEG +} + +TEST(RuntimeImageMime, FormatMatchingIsCaseInsensitive) { + EXPECT_EQ(get_format_for_mime_type("Image/PNG"), PNG); + EXPECT_EQ(get_format_for_mime_type("IMAGE/BMP"), BMP); +} + +TEST(RuntimeImageMime, FormatMatchesContentTypeWithParameters) { + // Content-Type headers may carry parameters after the media type + EXPECT_EQ(get_format_for_mime_type("image/png; charset=binary"), PNG); + EXPECT_EQ(get_format_for_mime_type("image/bmp;name=\"a.bmp\""), BMP); +} + +TEST(RuntimeImageMime, UnknownMimeTypeHasNoFormat) { + EXPECT_EQ(get_format_for_mime_type("text/html"), std::nullopt); + EXPECT_EQ(get_format_for_mime_type("application/octet-stream"), std::nullopt); + EXPECT_EQ(get_format_for_mime_type("image/*"), std::nullopt); + EXPECT_EQ(get_format_for_mime_type(""), std::nullopt); + EXPECT_EQ(get_format_for_mime_type(nullptr), std::nullopt); +} + +TEST(RuntimeImageMime, MimeTypeForFormatRoundTrip) { + EXPECT_STREQ(get_mime_type_for_format(BMP), "image/bmp"); + EXPECT_STREQ(get_mime_type_for_format(PNG), "image/png"); +#ifdef USE_RUNTIME_IMAGE_JPEG + EXPECT_STREQ(get_mime_type_for_format(JPEG), "image/jpeg"); +#endif // USE_RUNTIME_IMAGE_JPEG + // AUTO has no single MIME type and falls back to the wildcard + EXPECT_STREQ(get_mime_type_for_format(AUTO), "image/*"); + + // Every decodable format must resolve back to itself through its MIME type + for (ImageFormat format : { + BMP, + PNG, +#ifdef USE_RUNTIME_IMAGE_JPEG + JPEG, +#endif // USE_RUNTIME_IMAGE_JPEG + }) { + EXPECT_EQ(get_format_for_mime_type(get_mime_type_for_format(format)), format) << format; + } +} + +} // namespace esphome::runtime_image::testing From b97c87799255ce62eed22bcdcb22aae30e168a63 Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Mon, 24 Aug 2026 15:27:24 -0500 Subject: [PATCH 06/13] [api] Acknowledge proxy subscribe and configuration requests (#18312) Co-authored-by: J. Nick Koston --- esphome/components/api/api.proto | 29 ++++- esphome/components/api/api_connection.cpp | 109 ++++++++++++------ esphome/components/api/api_pb2.cpp | 16 +++ esphome/components/api/api_pb2.h | 30 ++++- esphome/components/api/api_pb2_dump.cpp | 28 +++++ .../components/serial_proxy/serial_proxy.cpp | 74 ++++++++---- .../components/serial_proxy/serial_proxy.h | 28 ++++- .../components/zwave_proxy/zwave_proxy.cpp | 16 +-- esphome/components/zwave_proxy/zwave_proxy.h | 3 +- .../components/serial_proxy/serial_proxy.h | 36 +++--- .../components/zwave_proxy/zwave_proxy.h | 4 +- .../components/api/test_api_proto.py | 11 ++ 12 files changed, 298 insertions(+), 86 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 6ea124d155..1942ff568b 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -232,6 +232,7 @@ enum SerialProxyPortType { message SerialProxyInfo { string name = 1; // Human-readable port name SerialProxyPortType port_type = 2; // Port type (RS232, RS485) + uint32 configured_line_states = 3; // Bitmask of SerialProxyLineStateFlags this instance can drive } // DeviceInfoResponse max_data_length values: @@ -2626,6 +2627,22 @@ message ZWaveProxyRequest { bytes data = 2; } +enum ZWaveProxyStatus { + ZWAVE_PROXY_STATUS_OK = 0; // Request completed successfully + ZWAVE_PROXY_STATUS_IN_USE = 1; // Denied: another client is already subscribed + ZWAVE_PROXY_STATUS_NOT_SUPPORTED = 2; // Request type not supported +} + +// Acknowledges a ZWaveProxyRequest (subscribe/unsubscribe). Sent since API 1.16. +message ZWaveProxyRequestResponse { + option (id) = 151; + option (source) = SOURCE_SERVER; + option (ifdef) = "USE_ZWAVE_PROXY"; + + ZWaveProxyRequestType type = 1; // Which request type this responds to + ZWaveProxyStatus status = 2; // Result status +} + // ==================== INFRARED ==================== // Note: Feature and capability flag enums are defined in // esphome/components/infrared/infrared.h @@ -2769,12 +2786,18 @@ message SerialProxyGetModemPinsResponse { uint32 instance = 1; // Instance index (0-based) uint32 line_states = 2; // Bitmask of SerialProxyLineStateFlags + SerialProxyStatus status = 3; // INVALID_ARGUMENT if the instance index is out of range (since API 1.16) } enum SerialProxyRequestType { SERIAL_PROXY_REQUEST_TYPE_SUBSCRIBE = 0; // Subscribe to receive data from this serial proxy instance SERIAL_PROXY_REQUEST_TYPE_UNSUBSCRIBE = 1; // Unsubscribe from this serial proxy instance SERIAL_PROXY_REQUEST_TYPE_FLUSH = 2; // Flush the serial port (block until all TX data is sent) + // Values below are only valid in SerialProxyRequestResponse.type, identifying which + // operation is being acknowledged. Sending them in SerialProxyRequest.type is an + // error the device answers with INVALID_ARGUMENT. + SERIAL_PROXY_REQUEST_TYPE_CONFIGURE = 3; // Acknowledges a SerialProxyConfigureRequest + SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS = 4; // Acknowledges a SerialProxySetModemPinsRequest } enum SerialProxyStatus { @@ -2783,6 +2806,8 @@ enum SerialProxyStatus { SERIAL_PROXY_STATUS_ERROR = 2; // Driver or hardware error SERIAL_PROXY_STATUS_TIMEOUT = 3; // Timed out before TX completed SERIAL_PROXY_STATUS_NOT_SUPPORTED = 4; // Request type not supported by this instance + SERIAL_PROXY_STATUS_PORT_IN_USE = 5; // Denied: another client holds the port + SERIAL_PROXY_STATUS_INVALID_ARGUMENT = 6; // Invalid instance index or parameter value } // Generic request message for simple serial proxy operations @@ -2795,7 +2820,9 @@ message SerialProxyRequest { SerialProxyRequestType type = 2; // Request type } -// Response to a SerialProxyRequest (e.g. flush completion or failure) +// Acknowledges a serial proxy operation; the type field identifies which +// operation is being acknowledged. Flush has been acknowledged since the +// message was introduced; all other acknowledgements are sent since API 1.16. message SerialProxyRequestResponse { option (id) = 147; option (source) = SOURCE_SERVER; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index bb6c1695dd..05abbf0b75 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1381,7 +1381,12 @@ void APIConnection::on_z_wave_proxy_frame(const ZWaveProxyFrame &msg) { } void APIConnection::on_z_wave_proxy_request(const ZWaveProxyRequest &msg) { - zwave_proxy::global_zwave_proxy->zwave_proxy_request(this, msg.type); + ZWaveProxyRequestResponse resp{}; + resp.type = msg.type; + resp.status = zwave_proxy::global_zwave_proxy->zwave_proxy_request(this, msg.type); + if (!this->send_message(resp)) { + API_LOG_MSG_DROPPED(TAG, "Z-Wave proxy response"); + } } #endif @@ -1550,15 +1555,50 @@ void APIConnection::send_infrared_rf_receive_event(const InfraredRFReceiveEvent #endif #ifdef USE_SERIAL_PROXY +static enums::SerialProxyStatus serial_proxy_result_to_status(serial_proxy::SerialProxyResult result) { + switch (result) { + case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_OK: + return enums::SERIAL_PROXY_STATUS_OK; + case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_ASSUMED_SUCCESS: + return enums::SERIAL_PROXY_STATUS_ASSUMED_SUCCESS; + case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE: + return enums::SERIAL_PROXY_STATUS_PORT_IN_USE; + case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_INVALID_ARGUMENT: + return enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT; + case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_TIMEOUT: + return enums::SERIAL_PROXY_STATUS_TIMEOUT; + case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_NOT_SUPPORTED: + return enums::SERIAL_PROXY_STATUS_NOT_SUPPORTED; + case serial_proxy::SerialProxyResult::SERIAL_PROXY_RESULT_ERROR: + return enums::SERIAL_PROXY_STATUS_ERROR; + } + return enums::SERIAL_PROXY_STATUS_ERROR; // Unreachable; all enum values handled above +} + +static void send_serial_proxy_ack(APIConnection *conn, uint32_t instance, enums::SerialProxyRequestType type, + enums::SerialProxyStatus status) { + SerialProxyRequestResponse resp{}; + resp.instance = instance; + resp.type = type; + resp.status = status; + if (!conn->send_message(resp)) { + API_LOG_MSG_DROPPED(TAG, "Serial proxy response"); + } +} + void APIConnection::on_serial_proxy_configure_request(const SerialProxyConfigureRequest &msg) { auto &proxies = App.get_serial_proxies(); if (msg.instance >= proxies.size()) { ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range (max %" PRIu32 ")", msg.instance, static_cast(proxies.size())); + send_serial_proxy_ack(this, msg.instance, enums::SERIAL_PROXY_REQUEST_TYPE_CONFIGURE, + enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT); return; } - proxies[msg.instance]->configure(this, msg.baudrate, msg.flow_control, static_cast(msg.parity), - msg.stop_bits, msg.data_size); + serial_proxy::SerialProxyResult result = proxies[msg.instance]->configure( + this, msg.baudrate, msg.flow_control, static_cast(msg.parity), msg.stop_bits, msg.data_size); + send_serial_proxy_ack(this, msg.instance, enums::SERIAL_PROXY_REQUEST_TYPE_CONFIGURE, + serial_proxy_result_to_status(result)); } void APIConnection::on_serial_proxy_write_request(const SerialProxyWriteRequest &msg) { @@ -1574,20 +1614,30 @@ void APIConnection::on_serial_proxy_set_modem_pins_request(const SerialProxySetM auto &proxies = App.get_serial_proxies(); if (msg.instance >= proxies.size()) { ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range", msg.instance); + send_serial_proxy_ack(this, msg.instance, enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS, + enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT); return; } - proxies[msg.instance]->set_modem_pins(this, msg.line_states); + serial_proxy::SerialProxyResult result = proxies[msg.instance]->set_modem_pins(this, msg.line_states); + send_serial_proxy_ack(this, msg.instance, enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS, + serial_proxy_result_to_status(result)); } void APIConnection::on_serial_proxy_get_modem_pins_request(const SerialProxyGetModemPinsRequest &msg) { auto &proxies = App.get_serial_proxies(); - if (msg.instance >= proxies.size()) { - ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range", msg.instance); - return; - } SerialProxyGetModemPinsResponse resp{}; resp.instance = msg.instance; - resp.line_states = proxies[msg.instance]->get_modem_pins(); + if (msg.instance >= proxies.size()) { + ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range", msg.instance); + // Pre-1.16 clients do not read the status field and would take this error + // for a successful "both pins deasserted" answer; let them time out as before + if (!this->client_supports_api_version(1, 16)) { + return; + } + resp.status = enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT; + } else { + resp.line_states = proxies[msg.instance]->get_modem_pins(); + } if (!this->send_message(resp)) { API_LOG_MSG_DROPPED(TAG, "Serial proxy response"); } @@ -1597,40 +1647,31 @@ void APIConnection::on_serial_proxy_request(const SerialProxyRequest &msg) { auto &proxies = App.get_serial_proxies(); if (msg.instance >= proxies.size()) { ESP_LOGW(TAG, "Serial proxy instance %" PRIu32 " out of range", msg.instance); + send_serial_proxy_ack(this, msg.instance, msg.type, enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT); return; } + auto *proxy = proxies[msg.instance]; + enums::SerialProxyStatus status; switch (msg.type) { case enums::SERIAL_PROXY_REQUEST_TYPE_SUBSCRIBE: case enums::SERIAL_PROXY_REQUEST_TYPE_UNSUBSCRIBE: - proxies[msg.instance]->serial_proxy_request(this, msg.type); + status = serial_proxy_result_to_status(proxy->serial_proxy_request(this, msg.type)); break; - case enums::SERIAL_PROXY_REQUEST_TYPE_FLUSH: { - SerialProxyRequestResponse resp{}; - resp.instance = msg.instance; - resp.type = enums::SERIAL_PROXY_REQUEST_TYPE_FLUSH; - switch (proxies[msg.instance]->flush_port()) { - case uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS: - resp.status = enums::SERIAL_PROXY_STATUS_OK; - break; - case uart::UARTFlushResult::UART_FLUSH_RESULT_ASSUMED_SUCCESS: - resp.status = enums::SERIAL_PROXY_STATUS_ASSUMED_SUCCESS; - break; - case uart::UARTFlushResult::UART_FLUSH_RESULT_TIMEOUT: - resp.status = enums::SERIAL_PROXY_STATUS_TIMEOUT; - break; - case uart::UARTFlushResult::UART_FLUSH_RESULT_FAILED: - resp.status = enums::SERIAL_PROXY_STATUS_ERROR; - break; - } - if (!this->send_message(resp)) { - API_LOG_MSG_DROPPED(TAG, "Serial proxy response"); - } + case enums::SERIAL_PROXY_REQUEST_TYPE_FLUSH: + status = serial_proxy_result_to_status(proxy->flush_port(this)); + break; + case enums::SERIAL_PROXY_REQUEST_TYPE_CONFIGURE: + case enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS: + // Response-only discriminators; never valid in a request + ESP_LOGW(TAG, "Response-only serial proxy request type: %" PRIu32, static_cast(msg.type)); + status = enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT; break; - } default: ESP_LOGW(TAG, "Unknown serial proxy request type: %" PRIu32, static_cast(msg.type)); + status = enums::SERIAL_PROXY_STATUS_NOT_SUPPORTED; break; } + send_serial_proxy_ack(this, msg.instance, msg.type, status); } void APIConnection::send_serial_proxy_data(const SerialProxyDataReceived &msg) { @@ -1757,7 +1798,7 @@ bool APIConnection::send_hello_response_(const HelloRequest &msg) { HelloResponse resp; resp.api_version_major = 1; - resp.api_version_minor = 15; + resp.api_version_minor = 16; // Send only the version string - the client only logs this for debugging and doesn't use it otherwise resp.server_info = ESPHOME_VERSION_REF; resp.name = StringRef(App.get_name()); @@ -1891,6 +1932,7 @@ bool APIConnection::send_device_info_response_() { auto &info = resp.serial_proxies[serial_proxy_index++]; info.name = StringRef(proxy->get_name()); info.port_type = proxy->get_port_type(); + info.configured_line_states = proxy->get_configured_modem_pins(); } #endif #ifdef USE_API_NOISE @@ -1951,6 +1993,7 @@ bool APIConnection::send_device_capabilities_response_() { auto &info = resp.serial_proxies[serial_proxy_index++]; info.name = StringRef(proxy->get_name()); info.port_type = proxy->get_port_type(); + info.configured_line_states = proxy->get_configured_modem_pins(); } #endif return this->send_message(resp); diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 33611c5ee1..b5062f9e9f 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -102,12 +102,14 @@ uint8_t *SerialProxyInfo::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PAR uint8_t *__restrict__ pos = buffer.get_pos(); ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 1, this->name); ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 2, static_cast(this->port_type)); + ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 3, this->configured_line_states); return pos; } uint32_t SerialProxyInfo::calculate_size() const { uint32_t size = 0; size += ProtoSize::calc_length(1, this->name.size()); size += this->port_type ? 2 : 0; + size += ProtoSize::calc_uint32(1, this->configured_line_states); return size; } #endif @@ -3942,6 +3944,18 @@ uint32_t ZWaveProxyRequest::calculate_size() const { size += ProtoSize::calc_length(1, this->data_len); return size; } +uint8_t *ZWaveProxyRequestResponse::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { + uint8_t *__restrict__ pos = buffer.get_pos(); + ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 1, static_cast(this->type)); + ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 2, static_cast(this->status)); + return pos; +} +uint32_t ZWaveProxyRequestResponse::calculate_size() const { + uint32_t size = 0; + size += this->type ? 2 : 0; + size += this->status ? 2 : 0; + return size; +} #endif #ifdef USE_INFRARED uint8_t *ListEntitiesInfraredResponse::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { @@ -4184,12 +4198,14 @@ uint8_t *SerialProxyGetModemPinsResponse::encode(ProtoWriteBuffer &buffer PROTO_ uint8_t *__restrict__ pos = buffer.get_pos(); ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 1, this->instance); ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 2, this->line_states); + ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 3, static_cast(this->status)); return pos; } uint32_t SerialProxyGetModemPinsResponse::calculate_size() const { uint32_t size = 0; size += ProtoSize::calc_uint32(1, this->instance); size += ProtoSize::calc_uint32(1, this->line_states); + size += this->status ? 2 : 0; return size; } bool SerialProxyRequest::decode_varint(uint32_t field_id, proto_varint_value_t value) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 13db857467..cd2f32deaf 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -334,6 +334,11 @@ enum ZWaveProxyRequestType : uint32_t { ZWAVE_PROXY_REQUEST_TYPE_UNSUBSCRIBE = 1, ZWAVE_PROXY_REQUEST_TYPE_HOME_ID_CHANGE = 2, }; +enum ZWaveProxyStatus : uint32_t { + ZWAVE_PROXY_STATUS_OK = 0, + ZWAVE_PROXY_STATUS_IN_USE = 1, + ZWAVE_PROXY_STATUS_NOT_SUPPORTED = 2, +}; #endif #ifdef USE_SERIAL_PROXY enum SerialProxyParity : uint32_t { @@ -345,6 +350,8 @@ enum SerialProxyRequestType : uint32_t { SERIAL_PROXY_REQUEST_TYPE_SUBSCRIBE = 0, SERIAL_PROXY_REQUEST_TYPE_UNSUBSCRIBE = 1, SERIAL_PROXY_REQUEST_TYPE_FLUSH = 2, + SERIAL_PROXY_REQUEST_TYPE_CONFIGURE = 3, + SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS = 4, }; enum SerialProxyStatus : uint32_t { SERIAL_PROXY_STATUS_OK = 0, @@ -352,6 +359,8 @@ enum SerialProxyStatus : uint32_t { SERIAL_PROXY_STATUS_ERROR = 2, SERIAL_PROXY_STATUS_TIMEOUT = 3, SERIAL_PROXY_STATUS_NOT_SUPPORTED = 4, + SERIAL_PROXY_STATUS_PORT_IN_USE = 5, + SERIAL_PROXY_STATUS_INVALID_ARGUMENT = 6, }; #endif @@ -523,6 +532,7 @@ class SerialProxyInfo final : public ProtoMessage { public: StringRef name{}; enums::SerialProxyPortType port_type{}; + uint32_t configured_line_states{0}; uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; uint32_t calculate_size() const; #ifdef HAS_PROTO_MESSAGE_DUMP @@ -3130,6 +3140,23 @@ class ZWaveProxyRequest final : public ProtoDecodableMessage { bool decode_length(uint32_t field_id, ProtoLengthDelimited value) override; bool decode_varint(uint32_t field_id, proto_varint_value_t value) override; }; +class ZWaveProxyRequestResponse final : public ProtoMessage { + public: + static constexpr uint8_t MESSAGE_TYPE = 151; + static constexpr uint8_t ESTIMATED_SIZE = 4; +#ifdef HAS_PROTO_MESSAGE_DUMP + const LogString *message_name() const override { return LOG_STR("z_wave_proxy_request_response"); } +#endif + enums::ZWaveProxyRequestType type{}; + enums::ZWaveProxyStatus status{}; + uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; + uint32_t calculate_size() const; +#ifdef HAS_PROTO_MESSAGE_DUMP + const char *dump_to(DumpBuffer &out) const override; +#endif + + protected: +}; #endif #ifdef USE_INFRARED class ListEntitiesInfraredResponse final : public InfoResponseProtoMessage { @@ -3314,12 +3341,13 @@ class SerialProxyGetModemPinsRequest final : public ProtoDecodableMessage { class SerialProxyGetModemPinsResponse final : public ProtoMessage { public: static constexpr uint8_t MESSAGE_TYPE = 143; - static constexpr uint8_t ESTIMATED_SIZE = 8; + static constexpr uint8_t ESTIMATED_SIZE = 10; #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("serial_proxy_get_modem_pins_response"); } #endif uint32_t instance{0}; uint32_t line_states{0}; + enums::SerialProxyStatus status{}; uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; uint32_t calculate_size() const; #ifdef HAS_PROTO_MESSAGE_DUMP diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index d54215ba2e..9f53438531 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -816,6 +816,18 @@ template<> const char *proto_enum_to_string(enums: return ESPHOME_PSTR("UNKNOWN"); } } +template<> const char *proto_enum_to_string(enums::ZWaveProxyStatus value) { + switch (value) { + case enums::ZWAVE_PROXY_STATUS_OK: + return ESPHOME_PSTR("ZWAVE_PROXY_STATUS_OK"); + case enums::ZWAVE_PROXY_STATUS_IN_USE: + return ESPHOME_PSTR("ZWAVE_PROXY_STATUS_IN_USE"); + case enums::ZWAVE_PROXY_STATUS_NOT_SUPPORTED: + return ESPHOME_PSTR("ZWAVE_PROXY_STATUS_NOT_SUPPORTED"); + default: + return ESPHOME_PSTR("UNKNOWN"); + } +} #endif #ifdef USE_SERIAL_PROXY template<> const char *proto_enum_to_string(enums::SerialProxyParity value) { @@ -838,6 +850,10 @@ template<> const char *proto_enum_to_string(enums return ESPHOME_PSTR("SERIAL_PROXY_REQUEST_TYPE_UNSUBSCRIBE"); case enums::SERIAL_PROXY_REQUEST_TYPE_FLUSH: return ESPHOME_PSTR("SERIAL_PROXY_REQUEST_TYPE_FLUSH"); + case enums::SERIAL_PROXY_REQUEST_TYPE_CONFIGURE: + return ESPHOME_PSTR("SERIAL_PROXY_REQUEST_TYPE_CONFIGURE"); + case enums::SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS: + return ESPHOME_PSTR("SERIAL_PROXY_REQUEST_TYPE_SET_MODEM_PINS"); default: return ESPHOME_PSTR("UNKNOWN"); } @@ -854,6 +870,10 @@ template<> const char *proto_enum_to_string(enums::Ser return ESPHOME_PSTR("SERIAL_PROXY_STATUS_TIMEOUT"); case enums::SERIAL_PROXY_STATUS_NOT_SUPPORTED: return ESPHOME_PSTR("SERIAL_PROXY_STATUS_NOT_SUPPORTED"); + case enums::SERIAL_PROXY_STATUS_PORT_IN_USE: + return ESPHOME_PSTR("SERIAL_PROXY_STATUS_PORT_IN_USE"); + case enums::SERIAL_PROXY_STATUS_INVALID_ARGUMENT: + return ESPHOME_PSTR("SERIAL_PROXY_STATUS_INVALID_ARGUMENT"); default: return ESPHOME_PSTR("UNKNOWN"); } @@ -914,6 +934,7 @@ const char *SerialProxyInfo::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("SerialProxyInfo")); dump_field(out, ESPHOME_PSTR("name"), this->name); dump_field(out, ESPHOME_PSTR("port_type"), static_cast(this->port_type)); + dump_field(out, ESPHOME_PSTR("configured_line_states"), this->configured_line_states); return out.c_str(); } #endif @@ -2644,6 +2665,12 @@ const char *ZWaveProxyRequest::dump_to(DumpBuffer &out) const { dump_bytes_field(out, ESPHOME_PSTR("data"), this->data, this->data_len); return out.c_str(); } +const char *ZWaveProxyRequestResponse::dump_to(DumpBuffer &out) const { + MessageDumpHelper helper(out, ESPHOME_PSTR("ZWaveProxyRequestResponse")); + dump_field(out, ESPHOME_PSTR("type"), static_cast(this->type)); + dump_field(out, ESPHOME_PSTR("status"), static_cast(this->status)); + return out.c_str(); +} #endif #ifdef USE_INFRARED const char *ListEntitiesInfraredResponse::dump_to(DumpBuffer &out) const { @@ -2753,6 +2780,7 @@ const char *SerialProxyGetModemPinsResponse::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("SerialProxyGetModemPinsResponse")); dump_field(out, ESPHOME_PSTR("instance"), this->instance); dump_field(out, ESPHOME_PSTR("line_states"), this->line_states); + dump_field(out, ESPHOME_PSTR("status"), static_cast(this->status)); return out.c_str(); } const char *SerialProxyRequest::dump_to(DumpBuffer &out) const { diff --git a/esphome/components/serial_proxy/serial_proxy.cpp b/esphome/components/serial_proxy/serial_proxy.cpp index 4b3a907416..94cefc8700 100644 --- a/esphome/components/serial_proxy/serial_proxy.cpp +++ b/esphome/components/serial_proxy/serial_proxy.cpp @@ -89,12 +89,12 @@ void SerialProxy::dump_config() { this->dtr_pin_ != nullptr ? "configured" : "not configured"); } -void SerialProxy::configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity, - uint8_t stop_bits, uint8_t data_size) { +SerialProxyResult SerialProxy::configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, + uint8_t parity, uint8_t stop_bits, uint8_t data_size) { #ifdef USE_API if (this->port_claimed_by_other_(api_connection)) { ESP_LOGW(TAG, "Ignoring configure request from client without port access [%" PRIu32 "]", this->instance_index_); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } #endif ESP_LOGD(TAG, @@ -105,25 +105,29 @@ void SerialProxy::configure(api::APIConnection *api_connection, uint32_t baudrat auto *uart_comp = this->parent_; if (uart_comp == nullptr) { ESP_LOGE(TAG, "UART component not available"); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_ERROR; } // Validate all parameters before applying any (values come from a remote client) if (baudrate == 0) { ESP_LOGW(TAG, "Invalid baud rate: 0"); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_INVALID_ARGUMENT; } if (stop_bits < 1 || stop_bits > 2) { ESP_LOGW(TAG, "Invalid stop bits: %u (must be 1 or 2)", stop_bits); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_INVALID_ARGUMENT; } if (data_size < 5 || data_size > 8) { ESP_LOGW(TAG, "Invalid data bits: %u (must be 5-8)", data_size); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_INVALID_ARGUMENT; } if (parity > 2) { ESP_LOGW(TAG, "Invalid parity: %u (must be 0-2)", parity); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_INVALID_ARGUMENT; + } + if (flow_control) { + ESP_LOGW(TAG, "Hardware flow control requested but is not yet supported"); + return SerialProxyResult::SERIAL_PROXY_RESULT_NOT_SUPPORTED; } // Apply validated parameters @@ -143,10 +147,7 @@ void SerialProxy::configure(api::APIConnection *api_connection, uint32_t baudrat #if defined(USE_ESP8266) || defined(USE_ESP32) uart_comp->load_settings(true); #endif - - if (flow_control) { - ESP_LOGW(TAG, "Hardware flow control requested but is not yet supported"); - } + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } void SerialProxy::write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len) { @@ -163,13 +164,20 @@ void SerialProxy::write_from_client(api::APIConnection *api_connection, const ui this->write_array(data, len); } -void SerialProxy::set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) { +SerialProxyResult SerialProxy::set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) { #ifdef USE_API if (this->port_claimed_by_other_(api_connection)) { ESP_LOGW(TAG, "Ignoring modem pin request from client without port access [%" PRIu32 "]", this->instance_index_); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } #endif + // Asserting a pin that is not configured must fail so the client learns the signal never + // reached the wire; deasserting an absent pin is harmless and stays allowed. Clients can + // avoid this by masking against SerialProxyInfo.configured_line_states. + if ((line_states & ~this->get_configured_modem_pins()) != 0) { + ESP_LOGW(TAG, "Requested modem pin not configured on serial proxy [%" PRIu32 "]", this->instance_index_); + return SerialProxyResult::SERIAL_PROXY_RESULT_NOT_SUPPORTED; + } const bool rts = (line_states & SERIAL_PROXY_LINE_STATE_FLAG_RTS) != 0; const bool dtr = (line_states & SERIAL_PROXY_LINE_STATE_FLAG_DTR) != 0; ESP_LOGV(TAG, "Setting modem pins [%" PRIu32 "]: RTS=%s, DTR=%s", this->instance_index_, ONOFF(rts), ONOFF(dtr)); @@ -182,6 +190,7 @@ void SerialProxy::set_modem_pins(api::APIConnection *api_connection, uint32_t li this->dtr_state_ = dtr; this->dtr_pin_->digital_write(dtr); } + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } uint32_t SerialProxy::get_modem_pins() const { @@ -189,9 +198,26 @@ uint32_t SerialProxy::get_modem_pins() const { (this->dtr_state_ ? static_cast(SERIAL_PROXY_LINE_STATE_FLAG_DTR) : 0u); } -uart::UARTFlushResult SerialProxy::flush_port() { +SerialProxyResult SerialProxy::flush_port(api::APIConnection *api_connection) { +#ifdef USE_API + // Flushing stalls the port, so it gets the same ownership check as writes + if (this->port_claimed_by_other_(api_connection)) { + ESP_LOGW(TAG, "Ignoring flush from client without port access [%" PRIu32 "]", this->instance_index_); + return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; + } +#endif ESP_LOGV(TAG, "Flushing serial proxy [%" PRIu32 "]", this->instance_index_); - return this->flush(); + switch (this->flush()) { + case uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS: + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; + case uart::UARTFlushResult::UART_FLUSH_RESULT_ASSUMED_SUCCESS: + return SerialProxyResult::SERIAL_PROXY_RESULT_ASSUMED_SUCCESS; + case uart::UARTFlushResult::UART_FLUSH_RESULT_TIMEOUT: + return SerialProxyResult::SERIAL_PROXY_RESULT_TIMEOUT; + case uart::UARTFlushResult::UART_FLUSH_RESULT_FAILED: + return SerialProxyResult::SERIAL_PROXY_RESULT_ERROR; + } + return SerialProxyResult::SERIAL_PROXY_RESULT_ERROR; // Unreachable; all enum values handled above } #ifdef USE_API @@ -200,12 +226,13 @@ bool SerialProxy::port_claimed_by_other_(api::APIConnection *api_connection) con this->api_connection_->is_connection_setup(); } -void SerialProxy::serial_proxy_request(api::APIConnection *api_connection, api::enums::SerialProxyRequestType type) { +SerialProxyResult SerialProxy::serial_proxy_request(api::APIConnection *api_connection, + api::enums::SerialProxyRequestType type) { switch (type) { case api::enums::SERIAL_PROXY_REQUEST_TYPE_SUBSCRIBE: if (this->api_connection_ == api_connection) { ESP_LOGV(TAG, "API connection is already subscribed to serial proxy [%" PRIu32 "]", this->instance_index_); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } if (this->api_connection_ != nullptr) { // A living subscriber keeps exclusive access. Its connection may be dead without @@ -213,26 +240,27 @@ void SerialProxy::serial_proxy_request(api::APIConnection *api_connection, api:: // in that case let the new client take over instead of locking it out. if (this->api_connection_->is_connection_setup()) { ESP_LOGE(TAG, "Only one API subscription is allowed at a time"); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_PORT_IN_USE; } ESP_LOGW(TAG, "Previous subscriber disconnected; taking over subscription"); } this->api_connection_ = api_connection; this->enable_loop(); ESP_LOGV(TAG, "API connection subscribed to serial proxy [%" PRIu32 "]", this->instance_index_); - break; + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; case api::enums::SERIAL_PROXY_REQUEST_TYPE_UNSUBSCRIBE: + // Unsubscribe is idempotent: not being subscribed is not an error if (this->api_connection_ != api_connection) { ESP_LOGV(TAG, "API connection is not subscribed to serial proxy [%" PRIu32 "]", this->instance_index_); - return; + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } this->api_connection_ = nullptr; this->disable_loop(); ESP_LOGV(TAG, "API connection unsubscribed from serial proxy [%" PRIu32 "]", this->instance_index_); - break; + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; default: ESP_LOGW(TAG, "Unknown serial proxy request type: %" PRIu32, static_cast(type)); - break; + return SerialProxyResult::SERIAL_PROXY_RESULT_NOT_SUPPORTED; } } #endif diff --git a/esphome/components/serial_proxy/serial_proxy.h b/esphome/components/serial_proxy/serial_proxy.h index 268c1b52be..a0e47ee686 100644 --- a/esphome/components/serial_proxy/serial_proxy.h +++ b/esphome/components/serial_proxy/serial_proxy.h @@ -38,6 +38,17 @@ enum SerialProxyLineStateFlag : uint32_t { SERIAL_PROXY_LINE_STATE_FLAG_DTR = 1 << 1, ///< DTR (Data Terminal Ready) }; +/// Result of a client-initiated operation; mapped to api::enums::SerialProxyStatus by the API layer +enum class SerialProxyResult : uint8_t { + SERIAL_PROXY_RESULT_OK, ///< Operation completed or request accepted + SERIAL_PROXY_RESULT_ASSUMED_SUCCESS, ///< Platform cannot confirm TX drain; success assumed + SERIAL_PROXY_RESULT_PORT_IN_USE, ///< Denied: another live client holds the port + SERIAL_PROXY_RESULT_INVALID_ARGUMENT, ///< A parameter value is out of range + SERIAL_PROXY_RESULT_ERROR, ///< Driver or hardware error + SERIAL_PROXY_RESULT_TIMEOUT, ///< Timed out before TX completed + SERIAL_PROXY_RESULT_NOT_SUPPORTED, ///< Requested feature is not available on this instance +}; + /// Maximum bytes to read from UART in a single loop iteration inline constexpr size_t SERIAL_PROXY_MAX_READ_SIZE = 256; @@ -73,14 +84,14 @@ class SerialProxy final : public uart::UARTDevice, public Component { /// @param parity Parity setting (0=none, 1=even, 2=odd) /// @param stop_bits Number of stop bits (1 or 2) /// @param data_size Number of data bits (5-8) - void configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity, - uint8_t stop_bits, uint8_t data_size); + SerialProxyResult configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity, + uint8_t stop_bits, uint8_t data_size); /// Get the currently subscribed API connection (nullptr if none) api::APIConnection *get_api_connection() { return this->api_connection_; } /// Handle a subscribe/unsubscribe request from an API client - void serial_proxy_request(api::APIConnection *api_connection, api::enums::SerialProxyRequestType type); + SerialProxyResult serial_proxy_request(api::APIConnection *api_connection, api::enums::SerialProxyRequestType type); /// Write data received from an API client to the serial device /// @param api_connection The API connection sending the data @@ -89,13 +100,20 @@ class SerialProxy final : public uart::UARTDevice, public Component { void write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len); /// Set modem pin states from a bitmask of SerialProxyLineStateFlag values - void set_modem_pins(api::APIConnection *api_connection, uint32_t line_states); + SerialProxyResult set_modem_pins(api::APIConnection *api_connection, uint32_t line_states); /// Get current modem pin states as a bitmask of SerialProxyLineStateFlag values uint32_t get_modem_pins() const; + /// Get the modem pins this instance can drive as a bitmask of SerialProxyLineStateFlag values + uint32_t get_configured_modem_pins() const { + return (this->rts_pin_ != nullptr ? static_cast(SERIAL_PROXY_LINE_STATE_FLAG_RTS) : 0u) | + (this->dtr_pin_ != nullptr ? static_cast(SERIAL_PROXY_LINE_STATE_FLAG_DTR) : 0u); + } + /// Flush the serial port (block until all TX data is sent) - uart::UARTFlushResult flush_port(); + /// @param api_connection The API connection requesting the flush + SerialProxyResult flush_port(api::APIConnection *api_connection); /// Set the RTS GPIO pin (from YAML configuration) void set_rts_pin(GPIOPin *pin) { this->rts_pin_ = pin; } diff --git a/esphome/components/zwave_proxy/zwave_proxy.cpp b/esphome/components/zwave_proxy/zwave_proxy.cpp index 6e3f109ca1..68750295e1 100644 --- a/esphome/components/zwave_proxy/zwave_proxy.cpp +++ b/esphome/components/zwave_proxy/zwave_proxy.cpp @@ -194,12 +194,13 @@ void ZWaveProxy::api_connection_authenticated(api::APIConnection *conn) { } } -void ZWaveProxy::zwave_proxy_request(api::APIConnection *api_connection, api::enums::ZWaveProxyRequestType type) { +api::enums::ZWaveProxyStatus ZWaveProxy::zwave_proxy_request(api::APIConnection *api_connection, + api::enums::ZWaveProxyRequestType type) { switch (type) { case api::enums::ZWAVE_PROXY_REQUEST_TYPE_SUBSCRIBE: if (this->api_connection_ == api_connection) { ESP_LOGV(TAG, "API connection is already subscribed"); - return; + return api::enums::ZWAVE_PROXY_STATUS_OK; } if (this->api_connection_ != nullptr) { // A living subscriber keeps exclusive access. Its connection may be dead without @@ -207,25 +208,26 @@ void ZWaveProxy::zwave_proxy_request(api::APIConnection *api_connection, api::en // in that case let the new client take over instead of locking it out. if (this->api_connection_->is_connection_setup()) { ESP_LOGE(TAG, "Only one API subscription is allowed at a time"); - return; + return api::enums::ZWAVE_PROXY_STATUS_IN_USE; } ESP_LOGW(TAG, "Previous subscriber disconnected; taking over subscription"); } this->api_connection_ = api_connection; ESP_LOGV(TAG, "API connection is now subscribed"); - break; + return api::enums::ZWAVE_PROXY_STATUS_OK; case api::enums::ZWAVE_PROXY_REQUEST_TYPE_UNSUBSCRIBE: + // Unsubscribe is idempotent: not being subscribed is not an error if (this->api_connection_ != api_connection) { ESP_LOGV(TAG, "API connection is not subscribed"); - return; + return api::enums::ZWAVE_PROXY_STATUS_OK; } this->api_connection_ = nullptr; - break; + return api::enums::ZWAVE_PROXY_STATUS_OK; default: ESP_LOGW(TAG, "Unknown request type: %" PRIu32, static_cast(type)); - break; + return api::enums::ZWAVE_PROXY_STATUS_NOT_SUPPORTED; } } diff --git a/esphome/components/zwave_proxy/zwave_proxy.h b/esphome/components/zwave_proxy/zwave_proxy.h index cb60139ef8..75225d84a8 100644 --- a/esphome/components/zwave_proxy/zwave_proxy.h +++ b/esphome/components/zwave_proxy/zwave_proxy.h @@ -60,7 +60,8 @@ class ZWaveProxy final : public uart::UARTDevice, public Component { bool can_proceed() override; void api_connection_authenticated(api::APIConnection *conn); - void zwave_proxy_request(api::APIConnection *api_connection, api::enums::ZWaveProxyRequestType type); + api::enums::ZWaveProxyStatus zwave_proxy_request(api::APIConnection *api_connection, + api::enums::ZWaveProxyRequestType type); api::APIConnection *get_api_connection() { return this->api_connection_; } uint32_t get_feature_flags() const { return ZWaveProxyFeature::FEATURE_ZWAVE_PROXY_ENABLED; } diff --git a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h b/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h index d8b068fb36..6fc20f3350 100644 --- a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h +++ b/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h @@ -13,17 +13,18 @@ namespace api { class APIConnection; } // namespace api -namespace uart { -enum class UARTFlushResult : uint8_t { - UART_FLUSH_RESULT_SUCCESS, - UART_FLUSH_RESULT_ASSUMED_SUCCESS, - UART_FLUSH_RESULT_TIMEOUT, - UART_FLUSH_RESULT_FAILED, -}; -} // namespace uart - namespace serial_proxy { +enum class SerialProxyResult : uint8_t { + SERIAL_PROXY_RESULT_OK, + SERIAL_PROXY_RESULT_ASSUMED_SUCCESS, + SERIAL_PROXY_RESULT_PORT_IN_USE, + SERIAL_PROXY_RESULT_INVALID_ARGUMENT, + SERIAL_PROXY_RESULT_ERROR, + SERIAL_PROXY_RESULT_TIMEOUT, + SERIAL_PROXY_RESULT_NOT_SUPPORTED, +}; + class SerialProxy { public: void set_instance_index(uint32_t index) { this->instance_index_ = index; } @@ -31,13 +32,20 @@ class SerialProxy { const char *get_name() const { return ""; } api::enums::SerialProxyPortType get_port_type() const { return {}; } api::APIConnection *get_api_connection() { return nullptr; } - void serial_proxy_request(api::APIConnection *conn, api::enums::SerialProxyRequestType type) {} - void configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity, - uint32_t stop_bits, uint32_t data_size) {} + SerialProxyResult serial_proxy_request(api::APIConnection *conn, api::enums::SerialProxyRequestType type) { + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; + } + SerialProxyResult configure(api::APIConnection *api_connection, uint32_t baudrate, bool flow_control, uint8_t parity, + uint8_t stop_bits, uint8_t data_size) { + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; + } void write_from_client(api::APIConnection *api_connection, const uint8_t *data, size_t len) {} - void set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) {} + SerialProxyResult set_modem_pins(api::APIConnection *api_connection, uint32_t line_states) { + return SerialProxyResult::SERIAL_PROXY_RESULT_OK; + } uint32_t get_modem_pins() const { return 0; } - uart::UARTFlushResult flush_port() { return uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS; } + uint32_t get_configured_modem_pins() const { return 0; } + SerialProxyResult flush_port(api::APIConnection *api_connection) { return SerialProxyResult::SERIAL_PROXY_RESULT_OK; } protected: uint32_t instance_index_{0}; diff --git a/tests/benchmarks/stubs/esphome/components/zwave_proxy/zwave_proxy.h b/tests/benchmarks/stubs/esphome/components/zwave_proxy/zwave_proxy.h index b4ccd8fd00..3abbf7d257 100644 --- a/tests/benchmarks/stubs/esphome/components/zwave_proxy/zwave_proxy.h +++ b/tests/benchmarks/stubs/esphome/components/zwave_proxy/zwave_proxy.h @@ -15,7 +15,9 @@ namespace zwave_proxy { class ZWaveProxy { public: api::APIConnection *get_api_connection() { return nullptr; } - void zwave_proxy_request(api::APIConnection *conn, api::enums::ZWaveProxyRequestType type) {} + api::enums::ZWaveProxyStatus zwave_proxy_request(api::APIConnection *conn, api::enums::ZWaveProxyRequestType type) { + return api::enums::ZWAVE_PROXY_STATUS_OK; + } void send_frame(api::APIConnection *api_connection, const uint8_t *data, size_t length) {} void api_connection_authenticated(api::APIConnection *conn) {} uint32_t get_feature_flags() const { return 0; } diff --git a/tests/unit_tests/components/api/test_api_proto.py b/tests/unit_tests/components/api/test_api_proto.py index 35aa5ff529..31297911f5 100644 --- a/tests/unit_tests/components/api/test_api_proto.py +++ b/tests/unit_tests/components/api/test_api_proto.py @@ -263,6 +263,17 @@ def test_device_capabilities_response_has_id_150() -> None: ) +def test_z_wave_proxy_request_response_has_id_151() -> None: + body = _extract_proto_message(PROTO_TEXT, "ZWaveProxyRequestResponse") + match = re.search(r"option \(id\) = (\d+);", body) + assert match is not None, "ZWaveProxyRequestResponse is missing `option (id)`" + assert int(match.group(1)) == 151, ( + f"ZWaveProxyRequestResponse has id {match.group(1)}, expected 151. " + "Message ids are part of the wire protocol and must not change once " + "assigned." + ) + + def test_superseded_fields_are_not_marked_deprecated_in_proto() -> None: """The six superseded fields must not carry `[deprecated = true]` in api.proto, or the generator drops them and old clients stop receiving From 7c5105b16ea3f44a500494884415de6d3d6e3111 Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:36:06 +1000 Subject: [PATCH 07/13] [key_collector][mipi_rgb] Suppress compilation warnings (#18721) --- esphome/components/key_collector/key_collector.cpp | 2 ++ esphome/components/mipi_rgb/mipi_rgb.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/esphome/components/key_collector/key_collector.cpp b/esphome/components/key_collector/key_collector.cpp index cb7d47b7f0..69b7a6a7c6 100644 --- a/esphome/components/key_collector/key_collector.cpp +++ b/esphome/components/key_collector/key_collector.cpp @@ -14,6 +14,7 @@ void KeyCollector::loop() { } void KeyCollector::dump_config() { +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_CONFIG ESP_LOGCONFIG(TAG, "Key Collector:"); if (this->min_length_ > 0) ESP_LOGCONFIG(TAG, " min length: %d", this->min_length_); @@ -35,6 +36,7 @@ void KeyCollector::dump_config() { ESP_LOGCONFIG(TAG, " allowed keys '%s'", this->allowed_keys_.c_str()); if (this->timeout_ > 0) ESP_LOGCONFIG(TAG, " entry timeout: %0.1f", this->timeout_ / 1000.0); +#endif } void KeyCollector::add_provider(key_provider::KeyProvider *provider) { diff --git a/esphome/components/mipi_rgb/mipi_rgb.cpp b/esphome/components/mipi_rgb/mipi_rgb.cpp index b07460fdba..7421d8ad83 100644 --- a/esphome/components/mipi_rgb/mipi_rgb.cpp +++ b/esphome/components/mipi_rgb/mipi_rgb.cpp @@ -345,7 +345,7 @@ int MipiRgb::get_height() { } } -static const char *get_pin_name(GPIOPin *pin, std::span buffer) { +[[maybe_unused]] static const char *get_pin_name(GPIOPin *pin, std::span buffer) { if (pin == nullptr) return "None"; pin->dump_summary(buffer.data(), buffer.size()); From 3f0ec0b1572376d8234ba9e61f710b21e2d0043e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 24 Aug 2026 15:56:59 -0500 Subject: [PATCH 08/13] [espidf] Always reconfigure after component discovery (#18730) --- esphome/espidf/toolchain.py | 38 ++++++++---------- tests/unit_tests/test_espidf_toolchain.py | 49 +++++++++++++++++++++++ 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 3c5c4803c2..baf316a4ee 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -398,27 +398,23 @@ def run_compile(config, verbose: bool) -> int: return rc _LOGGER.info("Regenerating CMakeLists.txt with discovered components...") write_project(minimal=False) - # Restamp the reference file has_outdated_files() compares against. - # A reconfigure that only changes properties or plain variables - # (sdkconfig options, the exclusion set) does not rewrite - # CMakeCache.txt, so without this the watched inputs stay newer - # forever and every subsequent build repeats the discovery pass. - # Done after the full write so an interrupt cannot leave a minimal - # CMakeLists behind that is already marked fresh. - cmakecache = CORE.relative_build_path("build/CMakeCache.txt") - if cmakecache.is_file(): - os.utime(cmakecache) - if CORE.testing_mode: - # Reconfigure again so cmake is up to date with the full - # component list before the build's idf.py invocation runs -- - # idf.py build would otherwise re-run cmake and regenerate - # memory.ld, wiping the DRAM/IRAM patches applied below. - # Outside testing mode ninja's own configure-time dep on - # CMakeLists.txt handles the re-run as part of the build step. - rc = run_reconfigure() - if rc != 0: - _LOGGER.error("Reconfigure with discovered components failed") - return rc + # Explicit reconfigure: ninja only re-runs cmake when CMakeLists.txt + # is strictly newer than build.ninja, which fails on coarse-mtime + # filesystems (#18682). Also keeps idf.py from regenerating memory.ld + # in testing mode. + rc = run_reconfigure() + if rc != 0: + _LOGGER.error("Reconfigure with discovered components failed") + return rc + # cmake does not rewrite CMakeCache.txt when only properties change, + # so restamp it or every build repeats discovery. Only after success, + # or a failed reconfigure would be marked fresh. build.ninja is + # restamped too so the cache is not newer and ninja does not + # re-run cmake. + for name in ("build/CMakeCache.txt", "build/build.ninja"): + path = CORE.relative_build_path(name) + if path.is_file(): + os.utime(path) # In testing mode, generate the linker script first, patch DRAM/IRAM sizes, # then build. memory.ld is regenerated by ninja during the build phase, diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 6c11a74d48..c54daec6a4 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -299,10 +299,13 @@ def test_run_compile_restamps_cmakecache_after_discovery(setup_core: Path) -> No _setup_build(setup_core) config = {CONF_ESPHOME: {}} cmakecache = CORE.relative_build_path("build/CMakeCache.txt") + build_ninja = CORE.relative_build_path("build/build.ninja") cmakecache.parent.mkdir(parents=True, exist_ok=True) cmakecache.write_text("") + build_ninja.write_text("") old = cmakecache.stat().st_mtime - 100 os.utime(cmakecache, (old, old)) + os.utime(build_ninja, (old, old)) with ( patch.object(toolchain, "need_reconfigure", return_value=True), @@ -314,6 +317,8 @@ def test_run_compile_restamps_cmakecache_after_discovery(setup_core: Path) -> No assert toolchain.run_compile(config, verbose=False) == 0 assert cmakecache.stat().st_mtime > old + # build.ninja must not be older than the cache or ninja re-runs cmake + assert build_ninja.stat().st_mtime >= cmakecache.stat().st_mtime def test_run_compile_discovery_without_cmakecache(setup_core: Path) -> None: @@ -334,6 +339,50 @@ def test_run_compile_discovery_without_cmakecache(setup_core: Path) -> None: assert not CORE.relative_build_path("build/CMakeCache.txt").exists() +def test_run_compile_reconfigures_after_full_write_outside_testing_mode( + setup_core: Path, +) -> None: + """The full CMakeLists write is followed by a reconfigure (#18682); a + failure there stops the build and leaves the cache unstamped.""" + _setup_build(setup_core) + config = {CONF_ESPHOME: {}} + cmakecache = CORE.relative_build_path("build/CMakeCache.txt") + cmakecache.parent.mkdir(parents=True, exist_ok=True) + cmakecache.write_text("") + old = cmakecache.stat().st_mtime - 100 + os.utime(cmakecache, (old, old)) + calls: list[tuple] = [] + reconfigures = 0 + + def record_write(minimal: bool = False) -> None: + calls.append(("write_project", minimal)) + + def record_reconfigure() -> int: + nonlocal reconfigures + reconfigures += 1 + calls.append(("run_reconfigure",)) + return 1 if reconfigures == 2 else 0 + + with ( + patch.object(toolchain, "need_reconfigure", return_value=True), + patch("esphome.build_gen.espidf.write_project", side_effect=record_write), + patch.object(toolchain, "run_reconfigure", side_effect=record_reconfigure), + patch.object(toolchain, "run_idf_py", return_value=0) as mock_build, + patch.object(toolchain, "print_summary"), + ): + assert not CORE.testing_mode + assert toolchain.run_compile(config, verbose=False) == 1 + + assert calls == [ + ("write_project", True), + ("run_reconfigure",), + ("write_project", False), + ("run_reconfigure",), + ] + mock_build.assert_not_called() + assert cmakecache.stat().st_mtime == old + + def test_run_compile_passes_compile_process_limit(setup_core: Path) -> None: """compile_process_limit is forwarded to run_idf_py as the job limit.""" _setup_build(setup_core) From 19866c60ee7b88cfbc5417ebea453029e7326422 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 24 Aug 2026 15:59:00 -0500 Subject: [PATCH 09/13] [core] Parallelize registry and tool downloads (#18662) --- esphome/espidf/framework.py | 100 +++-- esphome/framework_helpers.py | 237 +++++++++++- esphome/helpers.py | 13 +- esphome/platformio/library.py | 393 +++++++++++++------- tests/unit_tests/test_espidf_component.py | 16 +- tests/unit_tests/test_espidf_framework.py | 186 ++++++++- tests/unit_tests/test_espota2.py | 8 +- tests/unit_tests/test_framework_helpers.py | 275 ++++++++++++++ tests/unit_tests/test_helpers.py | 14 + tests/unit_tests/test_platformio_library.py | 195 +++++++++- 10 files changed, 1242 insertions(+), 195 deletions(-) diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 0f6ef873b8..179346e072 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -2,6 +2,7 @@ from collections.abc import Callable from ctypes.util import find_library +from functools import partial import json import logging import os @@ -20,9 +21,11 @@ from esphome.framework_helpers import ( create_venv, download_from_mirrors, download_with_resume, + failure_reason, get_python_env_executable_path, get_system_python_path, rmdir, + run_batch_downloads, run_command, run_command_ok, str_to_lst_of_str, @@ -690,6 +693,18 @@ def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None: ) +def _download_tool( + dist_path: Path, entry: dict, tracker: Callable[[int], None] +) -> None: + download_with_resume( + entry["url"], + dist_path / entry["dest"], + sha256=entry["sha256"], + size=entry["size"], + progress=tracker, + ) + + def _prefetch_idf_tool_archives( framework_path: Path, targets_str: str, @@ -702,10 +717,10 @@ def _prefetch_idf_tool_archives( which makes large archives effectively impossible to fetch on unstable connections (#17703). This asks the framework's idf_tools (via ``get_tool_downloads.py``) which archives the coming install needs, then - downloads each into ``/dist`` with - ``download_with_resume``. The installer then finds the verified archives - already in place ("file ... is already downloaded") and never touches the - network. + downloads them into ``/dist`` with + ``download_with_resume``, a few at a time under one combined progress + bar. The installer then finds the verified archives already in place + ("file ... is already downloaded") and never touches the network. Strictly best-effort: any failure here just logs and returns, leaving ``idf_tools.py install`` to download whatever is missing exactly as @@ -727,30 +742,67 @@ def _prefetch_idf_tool_archives( ) return dist_path = get_idf_tools_path() / "dist" - entries = [ - entry - for entry in json.loads(stdout) - if not (dist_path / entry["dest"]).is_file() - ] - for index, entry in enumerate(entries, start=1): - _LOGGER.info( - "Downloading %s (%d/%d) ...", entry["name"], index, len(entries) - ) - try: - download_with_resume( - entry["url"], - dist_path / entry["dest"], - sha256=entry["sha256"], - size=entry["size"], + entries = [] + seen_dests: set[str] = set() + for entry in json.loads(stdout): + if (dist_path / entry["dest"]).is_file(): + continue + # Never download unverified: an entry without sha256/size is + # left to the installer, which fails loudly on a bad archive. + # Checked before the dedupe so it cannot shadow a verifiable + # duplicate of the same dest. + if not (entry.get("sha256") and entry.get("size")): + _LOGGER.warning( + "Tool %s has no sha256/size in the download list; " + "leaving it to the installer", + entry["name"], ) - except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught - # Keep prefetching the remaining archives; the installer - # will retry this one itself (without resume). - _LOGGER.warning("Could not prefetch %s: %s", entry["name"], e) + continue + if entry["dest"] in seen_dests: + # Two workers on one .part file would interleave + # seek/truncate writes; mirror the library prefetch's dedupe + continue + seen_dests.add(entry["dest"]) + entries.append(entry) + if not entries: + return + _LOGGER.info( + "Downloading %d ESP-IDF tool archive(s): %s", + len(entries), + ", ".join(entry["name"] for entry in entries), + ) + + # No sequential fallback here: skipping the prefetch would lose the + # resume workaround for #17703, and every entry has a size (above). + # A failed archive is retried by the installer itself (without + # resume); keep prefetching the rest. + failures = run_batch_downloads( + "Downloading ESP-IDF tools", + [ + ( + entry["name"], + entry["size"], + partial(_download_tool, dist_path, entry), + ) + for entry in entries + ], + ) + for name, e in failures: + # failure_reason: a message-less exception must not log blank + _LOGGER.warning("Could not prefetch %s: %s", name, failure_reason(e)) + _LOGGER.debug("Prefetch failure detail", exc_info=e) + if len(failures) == len(entries): + # A systematic fault, not one flaky mirror: the resume + # workaround (#17703) is off for this whole install + _LOGGER.error( + "Every ESP-IDF tool prefetch failed; the installer will " + "download without resume" + ) except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught # The installer downloads anything missing itself; never let the # prefetch become a new way for the install to fail. - _LOGGER.warning("ESP-IDF tool prefetch failed: %s", e) + _LOGGER.warning("ESP-IDF tool prefetch failed: %s", failure_reason(e)) + _LOGGER.debug("Prefetch failure detail", exc_info=True) def _check_esphome_idf_framework_install( diff --git a/esphome/framework_helpers.py b/esphome/framework_helpers.py index 105791c518..2a2ce6dacf 100644 --- a/esphome/framework_helpers.py +++ b/esphome/framework_helpers.py @@ -1,7 +1,8 @@ """Generic toolchain installation helpers shared across framework implementations.""" -from collections.abc import Iterable -from contextlib import ExitStack +from collections.abc import Callable, Iterable, Iterator +from concurrent.futures import ThreadPoolExecutor +from contextlib import ExitStack, contextmanager, suppress import hashlib import io import json @@ -10,6 +11,7 @@ import os from pathlib import Path import subprocess import sys +import threading import time from typing import IO, TYPE_CHECKING @@ -24,6 +26,7 @@ PathType = str | os.PathLike _LOGGER = logging.getLogger(__name__) + # Attempts per mirror URL before falling through to the next mirror; only # mid-stream drops retry (resuming when the server gave a validator), # connect errors move on to the next mirror immediately. @@ -699,7 +702,11 @@ def _response_validator(resp: "requests.Response") -> str | None: def _stream_response_to_file( - resp: "requests.Response", f: IO[bytes], offset: int, size: int | None = None + resp: "requests.Response", + f: IO[bytes], + offset: int, + size: int | None = None, + progress: Callable[[int], None] | None = None, ) -> None: """Stream an open ``_open_ranged`` response body into ``f`` at ``offset``. @@ -707,21 +714,187 @@ def _stream_response_to_file( (effective offset 0) discards the stale bytes. ``offset`` also seeds the progress bar so a resumed download shows overall progress. ``size`` is the known full file size; when None it is derived from the response's - content-length, and without either there is no progress bar. + content-length, and without either there is no bar. With ``progress`` + set no bar is drawn here; the callback gets the absolute byte count. """ f.seek(offset) f.truncate(offset) total_size = size or offset + _content_length(resp) downloaded = offset - progress = ProgressBar("Downloading") if total_size > 0 else None + own_bar: ProgressBar | None = None + if progress is None: + own_bar = ProgressBar("Downloading") if total_size > 0 else None + progress = ( + (lambda done: own_bar.update(done / total_size)) + if own_bar + else (lambda _: None) + ) + progress(downloaded) for chunk in resp.iter_content(chunk_size=256 * 1024): if chunk: f.write(chunk) downloaded += len(chunk) - if progress is not None: - progress.update(downloaded / total_size) - if progress is not None: - progress.update(1) + progress(downloaded) + if own_bar is not None: + own_bar.update(1) + + +# Concurrent downloads per batch; enough to hide latency without +# hammering the host or the mirrors. +BATCH_DOWNLOAD_WORKERS = 4 + + +def run_batch_downloads( + header: str, + jobs: list[tuple[str, int, Callable[[Callable[[int], None]], None]]], + max_workers: int = BATCH_DOWNLOAD_WORKERS, +) -> list[tuple[str, BaseException]]: + """Run ``(name, size, fetch)`` download jobs concurrently under one bar. + + Each ``fetch(tracker)`` reports absolute byte counts; the bar total is + the sum of the sizes. Failures are returned after the bar is done so + warnings never land on its row. Ctrl-C drops queued jobs and aborts + in-flight ones at their next progress tick or backoff boundary (a + parked socket read defers that by its timeout, and an in-progress + archive extraction runs to completion); resumable destinations + (``download_with_resume``) keep their fetched ``.part`` bytes. + ``jobs`` must be non-empty. + """ + progress = _BatchDownloadProgress(header, sum(size for _, size, _ in jobs)) + cancelled = threading.Event() + + def _run( + name: str, fetch: Callable[[Callable[[int], None]], None] + ) -> tuple[str, BaseException] | None: + tracker = progress.tracker() + + def checked(done: int) -> None: + if cancelled.is_set(): + raise _BatchDownloadCancelled + tracker(done) + + try: + fetch(checked) + except (_BatchDownloadCancelled, Exception) as err: # noqa: BLE001 # pylint: disable=broad-exception-caught + # The cancelled arm exists for the tracker rollback below; the + # batch re-raises the interrupt, so the list is never returned + # after Ctrl-C. A bar-frame write failure must not displace the + # download error. + with suppress(Exception): + tracker(0) + failure = (name, err) + else: + failure = None + return failure + + ex = ThreadPoolExecutor(max_workers=max_workers) + try: + with progress.logging_guard(): + futures = [ex.submit(_run, name, fetch) for name, _, fetch in jobs] + return [failure for f in futures if (failure := f.result()) is not None] + except BaseException: + # Without this the non-daemon workers download to completion before + # the interpreter can exit, making Ctrl-C ineffective for minutes + cancelled.set() + raise + finally: + ex.shutdown(wait=True, cancel_futures=True) + progress.done() + + +class _BatchDownloadCancelled(BaseException): + """Raised inside a download job to abandon it after Ctrl-C. + + BaseException, like KeyboardInterrupt: a broad ``except Exception`` in + the download layers must not convert an abort into a retry. + """ + + +class _BatchDownloadProgress: + """One bar across several concurrent downloads, summing tracker bytes. + + The lock also serialises stderr writes so workers never interleave + frames; a ``total`` of 0 draws nothing. Call ``done()`` at the end so a + bar short of 100% still ends its line. + """ + + def __init__(self, header: str, total: int) -> None: + self._bar = ProgressBar(header) if total > 0 else None + self._total = total + self._sum = 0 + self._lock = threading.Lock() + + def tracker(self) -> Callable[[int], None]: + if self._bar is None: + return lambda _: None + last = 0 + + def update(done: int) -> None: + nonlocal last + with self._lock: + self._sum += done - last + last = done + # A bar-write failure (broken stderr pipe) must not surface + # as a download failure and cost the .part file + with suppress(Exception): + self._bar.update(min(self._sum / self._total, 1)) + + return update + + def done(self) -> None: + if self._bar is not None: + self._bar.done() + + @contextmanager + def logging_guard(self) -> Iterator[None]: + r"""End a partial bar row before any log record while active. + + Worker warnings (mirror retries) share stderr with the bar's \r + frames; without this the record lands mid-row and the next frame + overwrites it. A handler-level filter runs just before emit, so + only a tiny window remains for a concurrent frame. + """ + the_bar = self._bar + if the_bar is None: + yield + return + lock = self._lock + + class _EndRow(logging.Filter): + def filter(self, record: logging.LogRecord) -> bool: + # Handler.handle() runs filters outside handleError's try; a + # stderr write failure must not escape through the log call + with lock, suppress(Exception): + the_bar.interrupt() + return True + + end_row = _EndRow() + handlers = logging.getLogger().handlers + for handler in handlers: + handler.addFilter(end_row) + try: + yield + finally: + for handler in handlers: + handler.removeFilter(end_row) + + +def _part_path(dest: Path) -> Path: + """The in-progress sidecar ``download_with_resume`` streams into.""" + return dest.with_name(dest.name + ".part") + + +def _cancellable_sleep( + delay: float, progress: Callable[[int], None] | None, done: int +) -> None: + """Backoff sleep that still observes a batch cancellation tick.""" + if progress is None: + time.sleep(delay) + return + end = time.monotonic() + delay + while (remaining := end - time.monotonic()) > 0: + progress(done) # raises when the batch was cancelled + time.sleep(min(0.5, remaining)) def download_with_resume( @@ -734,6 +907,7 @@ def download_with_resume( attempts: int = 5, timeout: int = 30, retry_connect_errors: bool = True, + progress: Callable[[int], None] | None = None, ) -> None: """Download ``url`` to ``dest``, resuming partial downloads. @@ -756,6 +930,9 @@ def download_with_resume( of consuming attempts — for callers with their own fallback, like ``download_from_mirrors``. + ``progress`` replaces the built-in bar: it receives the absolute bytes of + ``dest`` obtained so far (see ``_BatchDownloadProgress``). + Raises EsphomeError when all attempts are exhausted. """ # Imported lazily: requests is a heavy import (~85ms) and is only needed @@ -767,7 +944,7 @@ def download_with_resume( ensure_happy_eyeballs() dest = Path(dest) - part = dest.with_name(dest.name + ".part") + part = _part_path(dest) meta = part.with_name(part.name + ".meta") dest.parent.mkdir(parents=True, exist_ok=True) last_error: Exception | None = None @@ -779,6 +956,8 @@ def download_with_resume( if dest.is_file() and (sha256 is not None or size is not None): try: _verify_file(dest, sha256, size) + if progress is not None: + progress(size if size is not None else dest.stat().st_size) return except EsphomeError: dest.unlink() @@ -824,7 +1003,7 @@ def download_with_resume( # Recorded so a later run can prove an If-Range # resume of this part file safe. _write_download_meta(meta, url, validator, expected_total) - _stream_response_to_file(resp, f, offset, size) + _stream_response_to_file(resp, f, offset, size, progress) # else: a previous run already wrote every byte (or more) but # was killed before the rename below. Skip the network entirely # — a Range request past EOF would draw HTTP 416 — and let @@ -833,6 +1012,10 @@ def download_with_resume( expected_size = size if size is not None else expected_total _verify_file(part, sha256, expected_size or None) + if progress is not None: + # Also credits a part file an earlier run completed without + # streaming anything this time. + progress(expected_size or part.stat().st_size) if not expected_size and sha256 is None: # No sha, no size, and the server sent no usable # content-length: nothing can prove the download complete @@ -880,11 +1063,11 @@ def download_with_resume( raise EsphomeError( f"Failed to download {url} after {attempts} attempts: " - f"{_failure_reason(last_error)}" + f"{failure_reason(last_error)}" ) from last_error -def _failure_reason(e: Exception) -> str: +def failure_reason(e: BaseException) -> str: """Format a download exception for the aggregated error message. ``requests`` appends " for url: " to HTTP errors; the URL is already @@ -900,7 +1083,7 @@ def _spent_attempts_error(e: Exception, attempts: int) -> Exception: the sweep classifies it as permanent.""" from esphome.core import EsphomeError - err = EsphomeError(f"failed after {attempts} attempts: {_failure_reason(e)}") + err = EsphomeError(f"failed after {attempts} attempts: {failure_reason(e)}") err.__cause__ = e return err @@ -911,6 +1094,7 @@ def _try_mirrors_once( f: IO[bytes] | None, timeout: int, failures: list[tuple[str, Exception]], + progress: Callable[[int], None] | None = None, ) -> str | None: """Single pass over the resolved mirror ``urls``, one try per URL. @@ -939,6 +1123,7 @@ def _try_mirrors_once( # next mirror immediately; only mid-stream drops # retry-with-resume on the same URL. retry_connect_errors=False, + progress=progress, ) return url except (requests.RequestException, OSError, EsphomeError) as e: @@ -980,7 +1165,7 @@ def _try_mirrors_once( if offset == 0: validator = _response_validator(resp) expected_total = _content_length(resp) - _stream_response_to_file(resp, f, offset) + _stream_response_to_file(resp, f, offset, progress=progress) if expected_total and f.tell() != expected_total: raise EsphomeError( @@ -1029,6 +1214,7 @@ def download_from_mirrors( substitutions: dict[str, str], target: io.RawIOBase | IO[bytes] | PathType, timeout: int = 30, + progress: Callable[[int], None] | None = None, ) -> str: """ Download file from multiple mirrors with substitution support. @@ -1038,6 +1224,8 @@ def download_from_mirrors( substitutions: Dictionary of substitutions to apply to URLs target: Target file path or file-like object timeout: Download timeout in seconds + progress: Passed through to the download (see ``download_with_resume``); + replaces the built-in per-file bar Returns: The source URL. @@ -1102,7 +1290,9 @@ def download_from_mirrors( for sweep in range(1, _MIRROR_SWEEP_ATTEMPTS + 1): sweep_failures: list[tuple[str, Exception]] = [] if ( - url := _try_mirrors_once(urls, path_target, f, timeout, sweep_failures) + url := _try_mirrors_once( + urls, path_target, f, timeout, sweep_failures, progress + ) ) is not None: return url failures.extend(sweep_failures) @@ -1119,12 +1309,21 @@ def download_from_mirrors( _LOGGER.warning( "Download of %s failed (%s); retrying in %d seconds (attempt %d/%d)", transient[0], - _failure_reason(transient[1]), + failure_reason(transient[1]), delay, sweep + 1, _MIRROR_SWEEP_ATTEMPTS, ) - time.sleep(delay) + # Tick with the bytes already on disk so a combined bar holds + # steady during the backoff instead of rewinding to zero + done = 0 + if progress is not None: + if f is not None: + done = f.tell() + else: + part = _part_path(path_target) + done = part.stat().st_size if part.is_file() else 0 + _cancellable_sleep(delay, progress, done) # 4. Report every attempted URL if all mirrors failed. failures spans # all sweeps (deduplicated by URL and reason), so neither an early @@ -1133,7 +1332,7 @@ def download_from_mirrors( seen: set[tuple[str, str]] = set() attempts = "" for url, e in failures: - reason = _failure_reason(e) + reason = failure_reason(e) if (url, reason) not in seen: seen.add((url, reason)) attempts += f"\n {url}\n {reason}" diff --git a/esphome/helpers.py b/esphome/helpers.py index b3102ca277..d30e9b16a2 100644 --- a/esphome/helpers.py +++ b/esphome/helpers.py @@ -738,11 +738,22 @@ class ProgressBar: sys.stderr.flush() def done(self) -> None: - if not self.enabled: + # No frame drawn, or the 100% frame already ended its own line + if not self.enabled or self.last_progress is None or self.last_progress == 100: return sys.stderr.write("\n") sys.stderr.flush() + def interrupt(self) -> None: + """End a mid-row frame so the next write starts on its own row. + + The next ``update()`` redraws the bar; a finished bar stays done. + """ + if self.last_progress == 100: + return + self.done() + self.last_progress = None + def docs_url(path: str) -> str: """Return the URL to the documentation for a given path.""" diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index a3899fa860..bf9c323b84 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -15,6 +15,7 @@ regardless of which toolchain consumes the result. from collections import deque from collections.abc import Callable, Iterable from dataclasses import dataclass, field +from functools import partial import glob import hashlib import itertools @@ -30,7 +31,13 @@ from urllib.request import url2pathname from esphome import git from esphome.core import CORE, EsphomeError, Library -from esphome.framework_helpers import archive_extract_all, download_from_mirrors, rmdir +from esphome.framework_helpers import ( + archive_extract_all, + download_from_mirrors, + failure_reason, + rmdir, + run_batch_downloads, +) _LOGGER = logging.getLogger(__name__) @@ -70,6 +77,10 @@ SRC_FILE_EXTENSIONS = list(SOURCE_KIND_FOR_SUFFIX) DOMAIN = "pio_components" +# Marks a cache dir whose archive finished extracting; a missing marker +# means a torn extraction that must be redone +_EXTRACTED_MARKER = ".esphome_extracted" + ESPHOME_DATA_KEY = "ESPHOME" ESPHOME_DATA_EXTRA_CMAKE_KEY = "EXTRA_CMAKE" # Captured extra-script LINKFLAGS; kept apart from build.flags so they reach @@ -93,12 +104,13 @@ class Source: class URLSource(Source): - def __init__(self, url: str): + def __init__(self, url: str, size: int | None = None): self.url = url + # Archive size as reported by the registry, when known; sizes the + # combined prefetch bar without any extra network probe + self.size = size - def download( - self, dir_suffix: str, force: bool = False, salt: str = "", namespace: str = "" - ) -> Path: + def _cache_dir(self, dir_suffix: str, salt: str, namespace: str) -> Path: # Namespace the cache per backend (e.g. pio_components/idf, .../zephyr) so # the build files each backend writes into the library dir can't collide. base_dir = Path(CORE.data_dir) / DOMAIN @@ -108,22 +120,40 @@ class URLSource(Source): h.update(self.url.encode()) if salt: h.update(salt.encode()) - path = base_dir / h.hexdigest()[:8] / dir_suffix + return base_dir / h.hexdigest()[:8] / dir_suffix + + def is_cached(self, dir_suffix: str, salt: str = "", namespace: str = "") -> bool: + """Whether a completed extraction already exists for this source.""" + return ( + self._cache_dir(dir_suffix, salt, namespace) / _EXTRACTED_MARKER + ).is_file() + + def download( + self, + dir_suffix: str, + force: bool = False, + salt: str = "", + namespace: str = "", + progress: Callable[[int], None] | None = None, + ) -> Path: + path = self._cache_dir(dir_suffix, salt, namespace) # Marker file written last to signal a complete extraction. Using a # marker (instead of just `path.is_dir()`) means an interrupted # extraction is correctly detected and re-run on the next invocation, # and lets us extract directly into ``path`` — avoiding a # post-extraction rename that races with antivirus on Windows. - extracted_marker = path / ".esphome_extracted" + extracted_marker = path / _EXTRACTED_MARKER if not extracted_marker.is_file() or force: rmdir(path, msg=f"Clean up library directory {path}") # Download in temporary file with tempfile.NamedTemporaryFile() as tmp: - _LOGGER.info("Downloading %s ...", self.url) + if progress is None: + # A batch caller draws one combined bar and logs the list + _LOGGER.info("Downloading %s ...", self.url) _LOGGER.debug("Location: %s", path) - download_from_mirrors([self.url], {}, tmp.file) + download_from_mirrors([self.url], {}, tmp.file, progress=progress) _LOGGER.debug("Extracting archive to %s ...", path) archive_extract_all(tmp.file, path) @@ -415,6 +445,27 @@ def split_list_by_condition( return matched, non_matched +def _valid_manifest_shape(data: Any) -> bool: + """Whether the manifest has the dict shapes every backend dereferences. + + A bare json.load imposes no shape; validating once here means a + malformed third-party manifest fails by library name instead of a raw + TypeError/AttributeError in a backend. + """ + if not isinstance(data, dict): + return False + build = data.get("build", {}) + esphome_data = data.get(ESPHOME_DATA_KEY, {}) + return ( + isinstance(build, dict) + and isinstance(esphome_data, dict) + and isinstance(esphome_data.get(ESPHOME_DATA_LINK_FLAGS_KEY, []), list) + and isinstance(build.get("srcDir", ""), str) + and isinstance(build.get("includeDir", ""), str) + and isinstance(build.get("srcFilter", ""), (str, list)) + ) + + def check_library_data(data: dict, platform: str | None, framework: str): """ Check whether a library manifest is compatible with the target toolchain. @@ -537,9 +588,10 @@ def _make_registry_client() -> Any: def _resolve_registry_version( owner: str | None, pkgname: str, requirements: set[str] -) -> tuple[str, str, str, str]: +) -> tuple[str, str, str, str, int | None]: """Resolve a registry package to the single highest version satisfying ALL - the given requirements; return ``(owner, name, version, download_url)``. + the given requirements; return ``(owner, name, version, download_url, + size)`` (``size`` is None when the registry omits it). Intersecting every requirement (rather than resolving each consumer in isolation) makes the result independent of processing order and guarantees @@ -569,7 +621,7 @@ def _resolve_registry_version( pkgfile = registry.pick_compatible_pkg_file(best["files"]) if not pkgfile: raise RuntimeError(f"No package file for {owner}/{name}@{best['name']}") - return owner, name, best["name"], pkgfile["download_url"] + return owner, name, best["name"], pkgfile["download_url"], pkgfile.get("size") def split_flag_entry(entry: Any, owner: str) -> list[str]: @@ -859,6 +911,85 @@ def is_lib_ignored(name: str | None, lib_ignore: set[str]) -> bool: ) +def _fetch_source( + component: ConvertedLibrary, + salt: str, + namespace: str, + tracker: Callable[[int], None], +) -> None: + # Straight to URLSource: only it takes progress, and mutating the + # shared component from a worker is the authoritative loop's job + component.source.download( + component.get_sanitized_name(), salt=salt, namespace=namespace, progress=tracker + ) + + +def _prefetch_wave( + wave: list[tuple[str, ConvertedLibrary]], salt: str, namespace: str +) -> None: + """Best-effort parallel download of a wave's registry archives. + + The walk's own ``download()`` stays authoritative; duplicate URLs + prefetch once so two threads never share a cache directory. Archives + whose size the registry did not report are left to the sequential + loop, whose per-file bars don't interleave. A node a sibling in the + same wave supersedes has its archive fetched in vain (knowing better + would need the manifests being downloaded). + """ + try: + components: list[ConvertedLibrary] = [] + seen: set[str] = set() + for _key, component in wave: + source = component.source + if not isinstance(source, URLSource) or not source.size: + continue + if source.url in seen: + continue + seen.add(source.url) + try: + cached = source.is_cached( + component.get_sanitized_name(), salt=salt, namespace=namespace + ) + except OSError as err: + # Best-effort, but visibly: a systematic probe failure makes + # every warm build re-download every archive + _LOGGER.warning("Cache probe for %s failed: %s", component.name, err) + cached = False + if cached: + # A warm build must stay silent + continue + components.append(component) + if not components: + return + # Single-item waves (a dependency chain discovers one archive per + # wave) go through the same runner: one download method, one bar + _LOGGER.info( + "Downloading %d library archive(s): %s", + len(components), + ", ".join(c.name for c in components), + ) + failures = run_batch_downloads( + "Downloading libraries", + [ + (c.name, c.source.size, partial(_fetch_source, c, salt, namespace)) + for c in components + ], + ) + for name, err in failures: + # The sequential call below retries and raises the real error + _LOGGER.warning( + "Prefetch of %s failed (retrying sequentially): %s", + name, + failure_reason(err), + ) + _LOGGER.debug("Prefetch failure detail", exc_info=err) + except Exception as err: # noqa: BLE001 # pylint: disable=broad-exception-caught + # Same policy as the ESP-IDF twin: the prefetch must never become a + # new way for the build to fail + _LOGGER.warning("Library prefetch failed: %s", failure_reason(err)) + _LOGGER.debug("Prefetch failure detail", exc_info=True) + + def convert_libraries( libraries: list[Library], backend: LibraryBackend ) -> list[ConvertedLibrary]: @@ -955,136 +1086,134 @@ def convert_libraries( top_level_keys = set(top_level) worklist = deque(dict.fromkeys(top_level)) while worklist: - key = worklist.popleft() - node = nodes[key] + # Drain the frontier sequentially (spec resolution mutates shared + # state), then prefetch the wave in parallel + wave: list[tuple[str, ConvertedLibrary]] = [] + while worklist: + key = worklist.popleft() + node = nodes[key] - # Re-resolve only when the requirement set grew; requirements - # only ever grow, so the fixpoint converges and cycles terminate - requirements = frozenset(node.requirements) - if resolved_requirements.get(key) == requirements: - continue - resolved_requirements[key] = requirements + # Re-resolve only when the requirement set grew; requirements + # only ever grow, so the fixpoint converges and cycles terminate + requirements = frozenset(node.requirements) + if resolved_requirements.get(key) == requirements: + continue + resolved_requirements[key] = requirements - if node.is_git: - component = ConvertedLibrary(key, "*", GitSource(node.url, node.ref)) - elif node.is_local: - component = ConvertedLibrary(key, "*", LocalSource(node.local_path)) - else: - owner, name, version, url = _resolve_registry_version( - node.owner, node.pkgname, node.requirements - ) - component = ConvertedLibrary( - _owner_pkgname_to_name(owner, name), version, URLSource(url) - ) - component.download(salt=salt, namespace=backend.cache_key) + if node.is_git: + component = ConvertedLibrary(key, "*", GitSource(node.url, node.ref)) + elif node.is_local: + component = ConvertedLibrary(key, "*", LocalSource(node.local_path)) + else: + owner, name, version, url, size = _resolve_registry_version( + node.owner, node.pkgname, node.requirements + ) + component = ConvertedLibrary( + _owner_pkgname_to_name(owner, name), version, URLSource(url, size) + ) + wave.append((key, component)) + _prefetch_wave(wave, salt, backend.cache_key) + for key, component in wave: + node = nodes[key] + if frozenset(node.requirements) != resolved_requirements[key]: + # Requirements grew mid-wave: skip parsing a manifest the + # next wave will re-resolve and replace + worklist.append(key) + continue + component.download(salt=salt, namespace=backend.cache_key) - source_dir = component.source_dir - library_json_path = source_dir / "library.json" - library_properties_path = source_dir / "library.properties" - has_json = library_json_path.is_file() - has_properties = library_properties_path.is_file() - if not has_json and not has_properties and not node.is_local: - # An interrupted clone/extraction self-heals with one forced - # re-download; a local source has nothing to re-download - _LOGGER.warning( - "Library %s at %s is missing library.json and library.properties; " - "re-downloading", - key, - source_dir, - ) - component.download(force=True, salt=salt, namespace=backend.cache_key) + source_dir = component.source_dir + library_json_path = source_dir / "library.json" + library_properties_path = source_dir / "library.properties" has_json = library_json_path.is_file() has_properties = library_properties_path.is_file() - if has_json: - component.data = parse_library_json(library_json_path) - elif has_properties: - component.data = parse_library_properties(library_properties_path) - else: - # Local sources are user input (EsphomeError); a registry/git - # miss means a corrupt cache (RuntimeError) - error_cls = EsphomeError if node.is_local else RuntimeError - raise error_cls( - f"Invalid PIO library {key}: missing library.json and " - f"library.properties in {source_dir}" - ) - - # A bare json.load imposes no shape; every backend dereferences - # these fields, so validate once here and name the library - malformed = not isinstance(component.data, dict) - if not malformed: - build = component.data.get("build", {}) - esphome_data = component.data.get(ESPHOME_DATA_KEY, {}) - malformed = ( - not isinstance(build, dict) - or not isinstance(esphome_data, dict) - or not isinstance( - esphome_data.get(ESPHOME_DATA_LINK_FLAGS_KEY, []), list + if not has_json and not has_properties and not node.is_local: + # An interrupted clone/extraction self-heals with one forced + # re-download; a local source has nothing to re-download + _LOGGER.warning( + "Library %s at %s is missing library.json and library.properties; " + "re-downloading", + key, + source_dir, ) - or not isinstance(build.get("srcDir", ""), str) - or not isinstance(build.get("includeDir", ""), str) - or not isinstance(build.get("srcFilter", ""), (str, list)) - ) - if malformed: - # Fail fast only for a library the user asked for; a defect in - # an unrequested corner of the graph must not block the build - if key in top_level_keys: - raise EsphomeError(f"Library {key} has a malformed manifest") - _LOGGER.warning("Skipping dependency %s: malformed manifest", key) - continue - warn_properties_depends(component.name, component.data) - - try: - check_library_data(component.data, backend.platform, backend.framework) - except InvalidLibrary as e: - # An explicitly requested library fails fast; the routine - # cross-platform skip stays at debug, other causes warn - if key in top_level_keys: - reason = ( - f"is not compatible with {backend.framework}" - if isinstance(e, IncompatiblePlatform) - else "has a malformed manifest" - ) - raise RuntimeError(f"Requested library {key} {reason}: {e}") from e - if isinstance(e, IncompatiblePlatform): - _LOGGER.debug("Skip incompatible dependency %s: %s", key, str(e)) + component.download(force=True, salt=salt, namespace=backend.cache_key) + has_json = library_json_path.is_file() + has_properties = library_properties_path.is_file() + if has_json: + component.data = parse_library_json(library_json_path) + elif has_properties: + component.data = parse_library_properties(library_properties_path) else: - _LOGGER.warning("Skipping dependency %s: %s", key, str(e)) - continue - components[key] = component - - # Requirements changed (we got past the short-circuit above), so - # (re)walk this component's dependencies. - node.edges = set() - for dependency in normalize_dependencies( - component.data.get("dependencies"), component.name - ): - if "version" not in dependency: - # Cannot resolve from the registry; common for bundled - # names (Wire, SPI) -- unactionable noise above debug - _LOGGER.debug( - "Skip version-less dependency %r of %s", - dependency.get("name"), - component.name, + # Local sources are user input (EsphomeError); a registry/git + # miss means a corrupt cache (RuntimeError) + error_cls = EsphomeError if node.is_local else RuntimeError + raise error_cls( + f"Invalid PIO library {key}: missing library.json and " + f"library.properties in {source_dir}" ) + + if not _valid_manifest_shape(component.data): + # Fail fast only for a library the user asked for; a defect + # in an unrequested corner of the graph must not block the + # build + if key in top_level_keys: + raise EsphomeError(f"Library {key} has a malformed manifest") + _LOGGER.warning("Skipping dependency %s: malformed manifest", key) continue - if not dependency_is_usable( - dependency, backend.platform, backend.framework, component.name + warn_properties_depends(component.name, component.data) + + try: + check_library_data(component.data, backend.platform, backend.framework) + except InvalidLibrary as e: + # An explicitly requested library fails fast; the routine + # cross-platform skip stays at debug, other causes warn + if key in top_level_keys: + reason = ( + f"is not compatible with {backend.framework}" + if isinstance(e, IncompatiblePlatform) + else "has a malformed manifest" + ) + raise RuntimeError(f"Requested library {key} {reason}: {e}") from e + if isinstance(e, IncompatiblePlatform): + _LOGGER.debug("Skip incompatible dependency %s: %s", key, str(e)) + else: + _LOGGER.warning("Skipping dependency %s: %s", key, str(e)) + continue + components[key] = component + + # Requirements changed (we got past the short-circuit above), so + # (re)walk this component's dependencies. + node.edges = set() + for dependency in normalize_dependencies( + component.data.get("dependencies"), component.name ): - continue - dep_name = _owner_pkgname_to_name( - dependency.get("owner"), dependency.get("name") - ) - if is_lib_ignored(dep_name, lib_ignore): - _LOGGER.debug("Skip ignored dependency %s", dep_name) - continue - # The version field may actually be a URL (git/archive dependency). - dep_version = dependency["version"] - dep_url = _url_or_none(dep_version) - if dep_url is not None: - dep_version = None - dep_key = add_spec(dep_name, dep_version, dep_url) - node.edges.add(dep_key) - worklist.append(dep_key) + if "version" not in dependency: + # Cannot resolve from the registry; common for bundled + # names (Wire, SPI) -- unactionable noise above debug + _LOGGER.debug( + "Skip version-less dependency %r of %s", + dependency.get("name"), + component.name, + ) + continue + if not dependency_is_usable( + dependency, backend.platform, backend.framework, component.name + ): + continue + dep_name = _owner_pkgname_to_name( + dependency.get("owner"), dependency.get("name") + ) + if is_lib_ignored(dep_name, lib_ignore): + _LOGGER.debug("Skip ignored dependency %s", dep_name) + continue + # The version field may actually be a URL (git/archive dependency). + dep_version = dependency["version"] + dep_url = _url_or_none(dep_version) + if dep_url is not None: + dep_version = None + dep_key = add_spec(dep_name, dep_version, dep_url) + node.edges.add(dep_key) + worklist.append(dep_key) # A git or local source wins over the same component requested from the # registry. That's intentional, but warn so the dropped registry spec isn't diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 7b4f848979..3789eefc64 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -660,7 +660,7 @@ def _patch_registry(monkeypatch, versions): def test_resolve_registry_version_intersects_constraints(monkeypatch): _patch_registry(monkeypatch, ["1.10018.1", "1.10021.0", "1.10021.1"]) - owner, name, version, url = _resolve_registry_version( + owner, name, version, url, _size = _resolve_registry_version( "esphome", "libsodium", {"==1.10021.0", "^1.10018.1"} ) assert (owner, name, version) == ("esphome", "libsodium", "1.10021.0") @@ -669,7 +669,9 @@ def test_resolve_registry_version_intersects_constraints(monkeypatch): def test_resolve_registry_version_picks_highest_satisfying(monkeypatch): _patch_registry(monkeypatch, ["1.0.0", "1.5.0", "2.0.0"]) - _owner, _name, version, _url = _resolve_registry_version("o", "p", {"^1.0.0"}) + _owner, _name, version, _url, _size = _resolve_registry_version( + "o", "p", {"^1.0.0"} + ) assert version == "1.5.0" @@ -719,7 +721,7 @@ def test_generate_idf_components_dedupes_shared_dependency( resolve_calls.append(pkgname) captured[f"{owner}/{pkgname}"] = set(requirements) version = "1.10021.0" if pkgname == "C" else "1.0.0" - return owner, pkgname, version, f"http://x/{pkgname}.tar.gz" + return owner, pkgname, version, f"http://x/{pkgname}.tar.gz", None monkeypatch.setattr( esphome.platformio.library, "_resolve_registry_version", fake_resolve @@ -778,7 +780,7 @@ def test_generate_idf_components_lib_ignore_filters_top_level_and_dependencies( def fake_resolve(owner, pkgname, requirements): resolve_calls.append(pkgname) - return owner, pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz" + return owner, pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", None monkeypatch.setattr( esphome.platformio.library, "_resolve_registry_version", fake_resolve @@ -834,6 +836,7 @@ def test_generate_idf_components_handles_dependency_cycle( pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", + None, ), ) @@ -891,6 +894,7 @@ def test_generate_idf_components_git_overrides_registry_warns( pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", + None, ), ) @@ -927,6 +931,7 @@ def test_generate_idf_components_missing_manifest_raises( pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", + None, ), ) @@ -971,6 +976,7 @@ def test_generate_idf_components_warns_on_noncanonical_duplicate( pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", + None, ), ) @@ -1004,6 +1010,7 @@ def test_generate_idf_components_incompatible_top_level_raises( pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", + None, ), ) @@ -1040,6 +1047,7 @@ def test_generate_idf_components_incompatible_dependency_skipped( pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", + None, ), ) diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index d8e7738569..6288933a6a 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -2,6 +2,7 @@ # pylint: disable=protected-access +from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager import importlib.util import io @@ -14,7 +15,7 @@ import subprocess import sys import tarfile from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -887,6 +888,78 @@ _PREFETCH_JSON = json.dumps( ) +def test_prefetch_leaves_unverifiable_entries_to_the_installer( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """An entry missing sha256 or size must not download unverified; the + installer handles it and fails loudly on a bad archive.""" + entries = json.loads(_PREFETCH_JSON) + del entries[0]["sha256"] + del entries[1]["size"] + entries.append( + { + "name": "gcc@14.2.0", + "url": "https://example.com/gcc.tar.gz", + "size": 67, + "sha256": "ef" * 32, + "dest": "gcc.tar.gz", + } + ) + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.framework_helpers._BatchDownloadProgress") as progress_cls, + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + assert [call[0][0] for call in download.call_args_list] == [ + "https://example.com/gcc.tar.gz" + ] + assert download.call_args[1]["sha256"] == "ef" * 32 + progress_cls.assert_called_once_with("Downloading ESP-IDF tools", 67) + assert "cmake@3.30.2 has no sha256/size" in caplog.text + assert "ninja@1.12.1 has no sha256/size" in caplog.text + + +def test_prefetch_all_entries_unverifiable_is_a_noop(tmp_path: Path) -> None: + entries = json.loads(_PREFETCH_JSON) + for entry in entries: + del entry["sha256"] + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + download.assert_not_called() + + +def test_prefetch_dedupes_entries_by_dest(tmp_path: Path) -> None: + """Two entries resolving to one dest would interleave writes into the + same .part file; only the first downloads.""" + entries = json.loads(_PREFETCH_JSON) + dup = dict(entries[0]) | {"name": "cmake-alias@3.30.2"} + entries.append(dup) + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.framework_helpers._BatchDownloadProgress"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + dests = [call[0][1].name for call in download.call_args_list] + assert dests.count("cmake-3.30.2.tar.gz") == 1 + + def test_prefetch_downloads_each_archive_with_resume(tmp_path: Path) -> None: with ( patch( @@ -895,16 +968,58 @@ def test_prefetch_downloads_each_archive_with_resume(tmp_path: Path) -> None: ), patch("esphome.espidf.framework.download_with_resume") as download, patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.framework_helpers._BatchDownloadProgress") as progress_cls, ): + # Materialize the lazy mock before threads race its first creation + tracker = progress_cls.return_value.tracker.return_value _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) dist = get_idf_tools_path() / "dist" - assert download.call_count == 2 - assert download.call_args_list[0][0] == ( - "https://example.com/cmake.tar.gz", - dist / "cmake-3.30.2.tar.gz", - ) - assert download.call_args_list[0][1] == {"sha256": "ab" * 32, "size": 123} + # Archives download concurrently, so the call order is not fixed. + calls = {call[0]: call[1] for call in download.call_args_list} + assert set(calls) == { + ("https://example.com/cmake.tar.gz", dist / "cmake-3.30.2.tar.gz"), + ("https://example.com/ninja.zip", dist / "ninja.zip"), + } + kwargs = calls[("https://example.com/cmake.tar.gz", dist / "cmake-3.30.2.tar.gz")] + assert kwargs["sha256"] == "ab" * 32 + assert kwargs["size"] == 123 + # every archive reports into the one combined progress bar via the + # cancellation-checked wrapper; verify it delegates to the tracker + progress_cls.assert_called_once_with("Downloading ESP-IDF tools", 123 + 45) + before = tracker.call_count + for kw in calls.values(): + kw["progress"](7) + assert tracker.call_count == before + len(calls) + + +def test_prefetch_downloads_archives_concurrently(tmp_path: Path) -> None: + """More than one archive fans out over a bounded thread pool.""" + entries = [ + { + "name": f"tool{i}@1", + "url": f"https://example.com/tool{i}.tar.gz", + "size": 10, + "sha256": "ab" * 32, + "dest": f"tool{i}.tar.gz", + } + for i in range(6) + ] + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, json.dumps(entries), ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch( + "esphome.framework_helpers.ThreadPoolExecutor", wraps=ThreadPoolExecutor + ) as pool, + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + pool.assert_called_once_with(max_workers=4) + assert download.call_count == 6 def test_prefetch_skips_already_downloaded_archives(tmp_path: Path) -> None: @@ -959,11 +1074,12 @@ def test_prefetch_failures_never_raise( assert expected_log in caplog.text -def test_prefetch_one_failed_archive_does_not_stop_the_rest( +def test_prefetch_total_failure_logs_error( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: - """A single archive failing its download must not abort the prefetch of - the remaining archives.""" + """Every archive failing is a systematic fault (proxy, bad kwarg), not + a flaky mirror; it must be distinguishable at ERROR because the resume + workaround is off for the whole install.""" with ( patch( "esphome.espidf.framework.run_command", @@ -971,7 +1087,32 @@ def test_prefetch_one_failed_archive_does_not_stop_the_rest( ), patch( "esphome.espidf.framework.download_with_resume", - side_effect=[OSError("network down"), None], + side_effect=OSError("proxy refuses everything"), + ), + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + assert "Every ESP-IDF tool prefetch failed" in caplog.text + + +def test_prefetch_one_failed_archive_does_not_stop_the_rest( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A single archive failing its download must not abort the prefetch of + the remaining archives.""" + + def _fail_cmake_download(url: str, *args, **kwargs) -> None: + if "cmake" in url: + raise OSError("network down") + + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, _PREFETCH_JSON, ""), + ), + patch( + "esphome.espidf.framework.download_with_resume", + side_effect=_fail_cmake_download, ) as download, patch("esphome.espidf.framework.get_system_python_path", return_value="python"), ): @@ -979,6 +1120,29 @@ def test_prefetch_one_failed_archive_does_not_stop_the_rest( assert download.call_count == 2 assert "Could not prefetch cmake@3.30.2" in caplog.text + # One flaky archive is routine, never the systematic-fault ERROR + assert "Every ESP-IDF tool prefetch failed" not in caplog.text + + +def test_prefetch_finishes_progress_bar_and_cancels_queue(tmp_path: Path) -> None: + """The batch bar is closed out after the pool, and the pool is shut down + with cancel_futures so Ctrl-C does not drain every queued archive.""" + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, _PREFETCH_JSON, ""), + ), + patch("esphome.espidf.framework.download_with_resume"), + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + patch("esphome.framework_helpers._BatchDownloadProgress") as progress_cls, + patch("esphome.framework_helpers.ThreadPoolExecutor") as pool_cls, + ): + pool = MagicMock(wraps=ThreadPoolExecutor(max_workers=2)) + pool_cls.return_value = pool + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + pool.shutdown.assert_called_once_with(wait=True, cancel_futures=True) + progress_cls.return_value.done.assert_called_once_with() def test_prefetch_passes_targets_and_tools_to_script(tmp_path: Path) -> None: diff --git a/tests/unit_tests/test_espota2.py b/tests/unit_tests/test_espota2.py index e0e9185e1c..8867e2c215 100644 --- a/tests/unit_tests/test_espota2.py +++ b/tests/unit_tests/test_espota2.py @@ -998,10 +998,10 @@ def test_progress_bar(capsys: CaptureFixture[str]) -> None: assert "100%" in captured.err assert "Done" in captured.err - # Test done method + # done() after the 100% frame adds nothing; that frame ended its line progress.done() captured = capsys.readouterr() - assert captured.err == "\n" + assert captured.err == "" # Test same progress doesn't update progress.update(0.5) @@ -1010,6 +1010,10 @@ def test_progress_bar(capsys: CaptureFixture[str]) -> None: # Should only see one update (second call shouldn't write) assert captured.err.count("50%") == 1 + # done() after a mid-way frame ends the line + progress.done() + assert capsys.readouterr().err == "\n" + # Tests for SHA256 authentication @pytest.mark.usefixtures("mock_time") diff --git a/tests/unit_tests/test_framework_helpers.py b/tests/unit_tests/test_framework_helpers.py index 500705ef67..5916a2fd60 100644 --- a/tests/unit_tests/test_framework_helpers.py +++ b/tests/unit_tests/test_framework_helpers.py @@ -12,6 +12,8 @@ from pathlib import Path import subprocess import sys import tarfile +import threading +import time from unittest.mock import MagicMock, Mock, call, patch import zipfile @@ -22,6 +24,7 @@ from esphome import framework_helpers from esphome.core import EsphomeError from esphome.framework_helpers import ( _7z_extract_all, + _BatchDownloadProgress, _detect_archive_root, _rename_with_retry, _tar_extract_all, @@ -36,6 +39,7 @@ from esphome.framework_helpers import ( get_python_env_executable_path, get_system_python_path, rmdir, + run_batch_downloads, run_command, run_command_ok, str_to_lst_of_str, @@ -1111,6 +1115,218 @@ class TestDownloadWithResume: assert mock_get.call_args[1]["headers"] == {} assert dest.read_bytes() == b"data" + def test_progress_callback_reports_absolute_bytes(self, tmp_path: Path) -> None: + """With a callback no bar is drawn; the callback sees the running + byte count of this file, then its final verified size.""" + dest = tmp_path / "tool.tar.gz" + resp = _mock_response(b"") + resp.headers = {"content-length": "7"} + resp.iter_content.return_value = [b"1234", b"567"] + seen: list[int] = [] + with ( + patch("requests.get", return_value=resp), + patch("esphome.framework_helpers.ProgressBar") as bar_cls, + ): + download_with_resume( + "https://example.com/t", dest, size=7, progress=seen.append + ) + assert seen == [0, 4, 7, 7] + bar_cls.assert_not_called() + + def test_progress_callback_seeds_with_resume_offset(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"12345") + good = hashlib.sha256(b"12345678").hexdigest() + seen: list[int] = [] + with patch("requests.get", return_value=_resumed_response(b"678")): + download_with_resume( + "https://example.com/t", dest, sha256=good, size=8, progress=seen.append + ) + assert seen[0] == 5 + assert seen[-1] == 8 + + def test_progress_callback_credits_already_complete_download( + self, tmp_path: Path + ) -> None: + """A verified dest from an earlier run still counts toward the batch.""" + dest = tmp_path / "tool.tar.gz" + dest.write_bytes(b"12345678") + seen: list[int] = [] + with patch("requests.get") as mock_get: + download_with_resume( + "https://example.com/t", dest, size=8, progress=seen.append + ) + mock_get.assert_not_called() + assert seen == [8] + + +def test_run_batch_downloads_ctrl_c_aborts_in_flight_jobs() -> None: + """Ctrl-C cancels in-flight downloads at their next tick instead of + letting non-daemon workers download to completion.""" + started = threading.Event() + ticks: list[int] = [] + + def interrupter(tracker) -> None: + started.wait(5) + raise KeyboardInterrupt + + def slow_download(tracker) -> None: + started.set() + for i in range(500): + tracker(i) + ticks.append(i) + time.sleep(0.01) + + t0 = time.monotonic() + with pytest.raises(KeyboardInterrupt): + run_batch_downloads( + "Downloading", + [("boom", 0, interrupter), ("slow", 0, slow_download)], + max_workers=2, + ) + # Uncancelled, slow_download alone takes ~5s + assert time.monotonic() - t0 < 3 + assert len(ticks) < 500 + + +def test_cancellation_escapes_broad_except_in_fetch() -> None: + """A fetch that wraps its work in except Exception cannot swallow the + Ctrl-C sentinel (it is a BaseException).""" + from esphome.framework_helpers import _BatchDownloadCancelled + + started = threading.Event() + swallowed = [] + + def interrupter(tracker) -> None: + started.wait(5) + raise KeyboardInterrupt + + def greedy_fetch(tracker) -> None: + started.set() + try: + for i in range(500): + tracker(i) + time.sleep(0.01) + except Exception as err: # noqa: BLE001 # pylint: disable=broad-exception-caught + swallowed.append(err) + + t0 = time.monotonic() + with pytest.raises(KeyboardInterrupt): + run_batch_downloads( + "Downloading", + [("boom", 0, interrupter), ("greedy", 0, greedy_fetch)], + max_workers=2, + ) + assert time.monotonic() - t0 < 3 + assert not swallowed + assert issubclass(_BatchDownloadCancelled, BaseException) + assert not issubclass(_BatchDownloadCancelled, Exception) + + +def test_logging_guard_ends_the_bar_row_before_a_record() -> None: + r"""A worker warning gets its own line instead of the bar's \r row.""" + stream = io.StringIO() + stream.isatty = lambda: True # type: ignore[method-assign] + with patch("esphome.helpers.sys.stderr", stream): + progress = _BatchDownloadProgress("Downloading", 10) + progress.tracker()(5) + with progress.logging_guard(): + logging.getLogger("esphome.test").warning("mirror retry") + # The partial 50% frame ended its line before the record was emitted + assert stream.getvalue().endswith("50% \n") + # And the next tick redraws the frame on a fresh row + progress.tracker()(2) + assert stream.getvalue().endswith("70% ") + + +def test_logging_guard_without_a_bar_is_a_no_op() -> None: + """An unknown total draws no bar; the guard passes records through.""" + progress = _BatchDownloadProgress("Downloading", 0) + with progress.logging_guard(): + logging.getLogger("esphome.test").warning("plain record") + + +def test_cancellable_sleep_sleeps_between_ticks() -> None: + """An uncancelled backoff actually waits out its delay in slices.""" + from esphome.framework_helpers import _cancellable_sleep + + ticks: list[int] = [] + t0 = time.monotonic() + _cancellable_sleep(0.05, ticks.append, 3) + assert time.monotonic() - t0 >= 0.05 + assert ticks and all(t == 3 for t in ticks) + + +def test_cancellable_sleep_aborts_at_the_tick() -> None: + """A backoff sleep observes the cancellation raise promptly.""" + from esphome.framework_helpers import _BatchDownloadCancelled, _cancellable_sleep + + def cancelled_tick(done: int) -> None: + raise _BatchDownloadCancelled + + t0 = time.monotonic() + with pytest.raises(_BatchDownloadCancelled): + _cancellable_sleep(30, cancelled_tick, 0) + assert time.monotonic() - t0 < 1 + + +class Test_BatchDownloadProgress: + def test_sums_trackers_into_one_bar(self) -> None: + with patch("esphome.framework_helpers.ProgressBar") as bar_cls: + progress = _BatchDownloadProgress("Downloading", 100) + a = progress.tracker() + b = progress.tracker() + a(10) + b(20) + a(30) + a(0) # a restart from zero takes that file's bytes back out + bar_cls.assert_called_once_with("Downloading") + updates = [c[0][0] for c in bar_cls.return_value.update.call_args_list] + assert updates == [0.1, 0.3, 0.5, 0.2] + + def test_clamps_at_one(self) -> None: + """Sizes are advisory; an over-delivering server never pushes past 100%.""" + with patch("esphome.framework_helpers.ProgressBar") as bar_cls: + progress = _BatchDownloadProgress("Downloading", 10) + progress.tracker()(25) + assert bar_cls.return_value.update.call_args[0][0] == 1 + + def test_unknown_total_draws_nothing(self) -> None: + with patch("esphome.framework_helpers.ProgressBar") as bar_cls: + progress = _BatchDownloadProgress("Downloading", 0) + progress.tracker()(5) + progress.done() + bar_cls.assert_not_called() + + def test_done_ends_an_unfinished_bar(self) -> None: + """A batch that stops short of 100% (a failed archive) still ends its + line so the next log message starts on a fresh row.""" + stream = io.StringIO() + stream.isatty = lambda: True # type: ignore[method-assign] + with patch("esphome.helpers.sys.stderr", stream): + progress = _BatchDownloadProgress("Downloading", 10) + progress.tracker()(5) + progress.done() + assert stream.getvalue().endswith("50% \n") + + def test_done_before_any_frame_writes_nothing(self) -> None: + """A batch aborted before any tracker fired must not emit a stray + newline for a bar that was never drawn.""" + stream = io.StringIO() + stream.isatty = lambda: True # type: ignore[method-assign] + with patch("esphome.helpers.sys.stderr", stream): + _BatchDownloadProgress("Downloading", 10).done() + assert stream.getvalue() == "" + + def test_done_after_full_bar_adds_nothing(self) -> None: + stream = io.StringIO() + stream.isatty = lambda: True # type: ignore[method-assign] + with patch("esphome.helpers.sys.stderr", stream): + progress = _BatchDownloadProgress("Downloading", 10) + progress.tracker()(10) + progress.done() + assert stream.getvalue().endswith("100% Done...\r\n") + class TestDownloadFromMirrors: def test_success_returns_url_and_writes_content(self, tmp_path: Path) -> None: @@ -1123,6 +1339,22 @@ class TestDownloadFromMirrors: assert url == "https://example.com/f" assert target.read_bytes() == b"filedata" + def test_file_object_target_reports_progress(self) -> None: + """The library prefetch's production path: a file-object target + streams through the mirror fallback and ticks the tracker.""" + buf = io.BytesIO() + ticks: list[int] = [] + with patch( + "requests.get", + return_value=_mock_response(b"filedata"), + ): + url = download_from_mirrors( + ["https://example.com/f"], {}, buf, progress=ticks.append + ) + assert url == "https://example.com/f" + assert buf.getvalue() == b"filedata" + assert ticks and ticks[-1] == len(b"filedata") + def test_substitutions_applied_to_url(self, tmp_path: Path) -> None: with patch( "requests.get", @@ -1468,6 +1700,49 @@ class TestDownloadFromMirrors: assert mock_get.call_count == 2 mock_sleep.assert_called_once_with(2) + def test_backoff_tick_reports_filelike_bytes(self) -> None: + """For a file-like target the backoff tick carries f.tell(), so the + combined bar holds steady through the sweep retry.""" + target = io.BytesIO() + ticks: list[int] = [] + with ( + patch( + "requests.get", + side_effect=[ + req.ConnectionError("down"), + _mock_response(b"data"), + ], + ), + patch("esphome.framework_helpers._cancellable_sleep") as mock_sleep, + ): + download_from_mirrors( + ["https://mirror1.com/f"], {}, target, progress=ticks.append + ) + # No bytes had streamed at backoff time, so the tick carries 0 + assert mock_sleep.call_args == call(2, ticks.append, 0) + assert target.getvalue() == b"data" + + def test_backoff_tick_reports_partial_bytes(self, tmp_path: Path) -> None: + """The backoff tick carries the bytes already in the part file, so a + combined bar holds steady instead of rewinding to zero.""" + dest = tmp_path / "out.bin" + (tmp_path / "out.bin.part").write_bytes(b"12345") + ticks: list[int] = [] + with ( + patch( + "requests.get", + side_effect=[ + req.ConnectionError("down"), + _mock_response(b"data"), + ], + ), + patch("esphome.framework_helpers._cancellable_sleep") as mock_sleep, + ): + download_from_mirrors( + ["https://mirror1.com/f"], {}, dest, progress=ticks.append + ) + assert mock_sleep.call_args == call(2, ticks.append, 5) + def test_permanent_failure_does_not_retry_sweep(self, tmp_path: Path) -> None: """An HTTP 404 will not heal on its own; fail after a single pass.""" with ( diff --git a/tests/unit_tests/test_helpers.py b/tests/unit_tests/test_helpers.py index 3160469063..683fef22cf 100644 --- a/tests/unit_tests/test_helpers.py +++ b/tests/unit_tests/test_helpers.py @@ -1124,6 +1124,20 @@ def test_progressbar_enabled_on_pipe_with_dashboard(monkeypatch) -> None: assert bar.enabled is True +def test_progressbar_interrupt_keeps_finished_bar_done(monkeypatch) -> None: + """interrupt() on a bar whose 100% frame already ended its own line + must not reset it, or the next tick would redraw a second Done row.""" + stream = MagicMock(spec=io.TextIOWrapper) + stream.isatty.return_value = True + monkeypatch.setattr(CORE, "dashboard", False) + + bar = ProgressBar("Uploading", stream=stream) + bar.update(1) + assert bar.last_progress == 100 + bar.interrupt() + assert bar.last_progress == 100 + + @pytest.mark.parametrize( ("seconds", "expected"), [ diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index bf8340cea0..792d7dab61 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -153,13 +153,15 @@ def test_localsource_download_returns_empty_build_dir(setup_core: Path) -> None: assert plain != out -def test_urlsource_download_extracts_then_reuses_marker(setup_core, monkeypatch): +def test_urlsource_download_extracts_then_reuses_marker( + setup_core, monkeypatch, caplog +): monkeypatch.setattr(lib, "rmdir", lambda path, msg="": None) dl_calls: list[list[str]] = [] monkeypatch.setattr( lib, "download_from_mirrors", - lambda urls, headers, f: dl_calls.append(urls), + lambda urls, headers, f, progress=None: dl_calls.append(urls), ) def fake_extract(fileobj, path): @@ -178,6 +180,12 @@ def test_urlsource_download_extracts_then_reuses_marker(setup_core, monkeypatch) assert out2 == out assert len(dl_calls) == 1 + # A batch caller passes a tracker and owns the messaging; no per-file INFO + caplog.set_level("INFO") + src.download("mylib-batch", progress=lambda done: None) + assert len(dl_calls) == 2 + assert "Downloading" not in caplog.text + def test_resolve_registry_version_raises_without_pkg_file(monkeypatch): registry = lib._make_registry_client() @@ -211,6 +219,7 @@ def _patch_registry_resolve(monkeypatch: pytest.MonkeyPatch) -> None: pkgname, "1.0.0", f"http://x/{pkgname}.tar.gz", + None, ), ) @@ -230,6 +239,38 @@ def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properti _patch_registry_resolve(monkeypatch) +def test_wave_requirement_growth_defers_the_superseded_download(tmp_path, monkeypatch): + """A's manifest constrains B while B sits in the same wave: B's + drain-time resolution is superseded, so its download defers to the + next wave instead of fetching a version that is immediately replaced.""" + download_names: list[str] = [] + manifests = { + "esphome/A": { + "name": "A", + "build": {}, + "dependencies": {"esphome/B": ">=1.0"}, + }, + "esphome/B": {"name": "B", "build": {}}, + } + + def fake_download(self, force=False, salt="", namespace="", progress=None): + download_names.append(self.name) + self.path = tmp_path / self.get_require_name() + self.path.mkdir(parents=True, exist_ok=True) + (self.path / "library.json").write_text(json.dumps(manifests[self.name])) + + monkeypatch.setattr(ConvertedLibrary, "download", fake_download) + # Hermetic: the stubbed registry reports no size, so no batch prefetch + _patch_registry_resolve(monkeypatch) + top = convert_libraries( + [Library("esphome/A", "1.0.0", None), Library("esphome/B", None, None)], + _backend(), + ) + assert sorted(c.name for c in top) == ["esphome/A", "esphome/B"] + # B downloads exactly once, after its requirement set stabilized + assert download_names.count("esphome/B") == 1 + + def test_convert_libraries_parses_library_properties(tmp_path, monkeypatch): # A manifest provided as library.properties (Arduino style) instead of # library.json must still be parsed and converted. @@ -574,6 +615,65 @@ def test_lex_build_flags_dangling_flag_does_not_cross_entries( assert "Ignoring trailing '-I'" in caplog.text +def test_prefetch_wave_downloads_registry_archives_in_parallel( + setup_core, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Registry archives in one wave download concurrently, deduped by URL; + git/local sources and failures are left to the sequential call.""" + calls: list[str] = [] + + def fake_download( + self, dir_suffix, force=False, salt="", namespace="", progress=None + ): + calls.append(self.url) + if progress is not None: + progress(0) + if "boom" in self.url: + raise RuntimeError("boom") + + monkeypatch.setattr(URLSource, "download", fake_download) + wave = [ + ("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz", 1))), + ("b", ConvertedLibrary("b", "1.0", URLSource("https://x/b.tar.gz", 1))), + # Duplicate URL must prefetch once (two threads must never extract + # into the same cache directory) + ("b2", ConvertedLibrary("b2", "1.0", URLSource("https://x/b.tar.gz", 1))), + ("c", ConvertedLibrary("c", "1.0", URLSource("https://x/boom.tar.gz", 1))), + ("g", ConvertedLibrary("g", "*", lib.GitSource("https://x/g.git", None))), + ] + lib._prefetch_wave(wave, "", "idf") + assert sorted(calls) == [ + "https://x/a.tar.gz", + "https://x/b.tar.gz", + "https://x/boom.tar.gz", + ] + # The failure surfaces at default verbosity, after the bar + assert "Prefetch of c failed (retrying sequentially)" in caplog.text + + +def test_prefetch_wave_unknown_size_left_to_sequential( + setup_core, monkeypatch: pytest.MonkeyPatch +) -> None: + """Archives without a registry-reported size skip the batch (their + sequential per-file bars don't interleave); the known subset still + prefetches.""" + calls: list[str] = [] + monkeypatch.setattr( + URLSource, + "download", + lambda self, dir_suffix, force=False, salt="", namespace="", progress=None: ( + calls.append(self.url) + ), + ) + wave = [ + ("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz", 1))), + ("b", ConvertedLibrary("b", "1.0", URLSource("https://x/b.tar.gz", 1))), + ("u", ConvertedLibrary("u", "1.0", URLSource("https://x/u.tar.gz"))), + ] + lib._prefetch_wave(wave, "", "idf") + assert sorted(calls) == ["https://x/a.tar.gz", "https://x/b.tar.gz"] + + def test_join_flag_args_empty_argument_warns_and_drops( caplog: pytest.LogCaptureFixture, ) -> None: @@ -582,6 +682,97 @@ def test_join_flag_args_empty_argument_warns_and_drops( assert "Ignoring '-D' with empty argument in build_flags" in caplog.text +def test_prefetch_wave_cache_probe_failure_still_prefetches( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """A filesystem probe failure warns (a systematic one re-downloads + everything) but still prefetches; a programming error is NOT swallowed + here, it reaches the outer blanket guard.""" + calls: list[str] = [] + monkeypatch.setattr( + URLSource, + "download", + lambda self, dir_suffix, **kw: calls.append(self.url), + ) + monkeypatch.setattr( + URLSource, + "is_cached", + lambda self, *a, **kw: (_ for _ in ()).throw(OSError("cache root denied")), + ) + wave = [ + ("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz", 1))), + ("b", ConvertedLibrary("b", "1.0", URLSource("https://x/b.tar.gz", 1))), + ] + lib._prefetch_wave(wave, "", "idf") + assert sorted(calls) == ["https://x/a.tar.gz", "https://x/b.tar.gz"] + assert "Cache probe for a failed: cache root denied" in caplog.text + + +def test_prefetch_wave_internal_error_never_fails_the_build( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """The blanket guard keeps a prefetch bug from failing the walk.""" + monkeypatch.setattr(URLSource, "is_cached", lambda self, *a, **kw: False) + monkeypatch.setattr( + lib, + "run_batch_downloads", + lambda *a, **kw: (_ for _ in ()).throw(RuntimeError("bug")), + ) + wave = [ + ("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz", 1))), + ("b", ConvertedLibrary("b", "1.0", URLSource("https://x/b.tar.gz", 1))), + ] + lib._prefetch_wave(wave, "", "idf") + assert "Library prefetch failed: bug" in caplog.text + + +def test_prefetch_wave_warm_cache_is_silent( + setup_core, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """Already-extracted archives download nothing; a warm build must not + print a Downloading line or draw a bar.""" + monkeypatch.setattr( + URLSource, + "download", + lambda self, dir_suffix, **kw: (_ for _ in ()).throw( + AssertionError("downloaded") + ), + ) + wave = [] + for name in ("a", "b", "c"): + comp = ConvertedLibrary(name, "1.0", URLSource(f"https://x/{name}.tar.gz", 1)) + marker_dir = comp.source._cache_dir(comp.get_sanitized_name(), "", "idf") + marker_dir.mkdir(parents=True) + (marker_dir / ".esphome_extracted").touch() + wave.append((name, comp)) + lib._prefetch_wave(wave, "", "idf") + assert "Downloading" not in caplog.text + + +def test_prefetch_wave_single_archive_uses_the_batch( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """A dependency chain discovers one archive per wave; it downloads + through the same runner so there is one download method and one bar.""" + caplog.set_level("INFO") + calls: list[str] = [] + monkeypatch.setattr(URLSource, "is_cached", lambda self, *a, **kw: False) + monkeypatch.setattr( + URLSource, + "download", + lambda self, dir_suffix, force=False, salt="", namespace="", progress=None: ( + calls.append(self.url) + ), + ) + lib._prefetch_wave( + [("a", ConvertedLibrary("a", "1.0", URLSource("https://x/a.tar.gz", 1)))], + "", + "idf", + ) + assert calls == ["https://x/a.tar.gz"] + assert "Downloading 1 library archive(s): a" in caplog.text + + def test_normalize_dependencies_forms(caplog) -> None: """Every PIO-legal spelling normalizes; unrecognizable entries warn.""" from esphome.platformio.library import normalize_dependencies From 2a05372a5193cfaaf70de782365c64a0ce3922c8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 24 Aug 2026 16:20:25 -0500 Subject: [PATCH 10/13] [gpio] Fix one_wire reset busy-waiting with interrupts off when delay wraps (#18733) --- esphome/components/gpio/one_wire/gpio_one_wire.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/esphome/components/gpio/one_wire/gpio_one_wire.cpp b/esphome/components/gpio/one_wire/gpio_one_wire.cpp index 1fecfbf0dd..f445efeca3 100644 --- a/esphome/components/gpio/one_wire/gpio_one_wire.cpp +++ b/esphome/components/gpio/one_wire/gpio_one_wire.cpp @@ -55,8 +55,11 @@ int HOT IRAM_ATTR GPIOOneWireBus::reset_int() { delayMicroseconds(1); } - // delay J - delayMicroseconds(start + 480 - micros()); + // delay J: finish the 480us slot, but never spin if it already elapsed + // (unsigned wrap here would busy-wait for minutes with interrupts off) + uint32_t elapsed = micros() - start; + if (elapsed < 480) + delayMicroseconds(480 - elapsed); this->pin_.digital_write(true); this->pin_.pin_mode(gpio::FLAG_OUTPUT); return r ? 1 : 0; From c866add3209df7a500b51ffc0caafccef835163d Mon Sep 17 00:00:00 2001 From: guillempages Date: Mon, 24 Aug 2026 23:23:07 +0200 Subject: [PATCH 11/13] [runtime_image] Fix include (#18738) Co-authored-by: J. Nick Koston --- esphome/components/runtime_image/image_format.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/runtime_image/image_format.cpp b/esphome/components/runtime_image/image_format.cpp index 3ba8871862..9575b30887 100644 --- a/esphome/components/runtime_image/image_format.cpp +++ b/esphome/components/runtime_image/image_format.cpp @@ -1,6 +1,6 @@ +#include "esphome/core/defines.h" #include "esphome/core/helpers.h" #include "image_format.h" -#include "image_decoder.h" namespace esphome::runtime_image { From a15fae6a1e55918253fe6d38a128025341b723c2 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:15:09 +0000 Subject: [PATCH 12/13] Bump aioesphomeapi from 46.1.0 to 46.2.0 (#18740) --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index c10ddc3f53..4f6bdbad4c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ pyserial==3.5 platformio==6.1.19 esptool==5.3.1 click==8.3.3 -aioesphomeapi==46.1.0 +aioesphomeapi==46.2.0 aiohappyeyeballs==2.7.1 # Happy Eyeballs for requests downloads; already pulled in by aioesphomeapi zeroconf==0.150.0 puremagic==2.2.0 From 37b78ad46a4a76ad930d58b4105a98d358dc2e5b Mon Sep 17 00:00:00 2001 From: luar123 <49960470+luar123@users.noreply.github.com> Date: Tue, 25 Aug 2026 00:24:57 +0200 Subject: [PATCH 13/13] [zigbee][time] Add zigbee time component on esp32 (#18656) Co-authored-by: J. Nick Koston --- esphome/components/zigbee/const_esp32.py | 3 + esphome/components/zigbee/time/__init__.py | 48 +++++-- .../zigbee/time/zigbee_time_esp32.cpp | 118 ++++++++++++++++++ .../zigbee/time/zigbee_time_esp32.h | 34 +++++ esphome/components/zigbee/zigbee_ep_esp32.py | 53 ++++++-- .../components/zigbee/zigbee_helpers_esp32.c | 4 + tests/components/zigbee/common.yaml | 3 + tests/components/zigbee/common_nrf52.yaml | 3 - .../zigbee/test-router.esp32-c6-idf.yaml | 3 + 9 files changed, 249 insertions(+), 20 deletions(-) create mode 100644 esphome/components/zigbee/time/zigbee_time_esp32.cpp create mode 100644 esphome/components/zigbee/time/zigbee_time_esp32.h diff --git a/esphome/components/zigbee/const_esp32.py b/esphome/components/zigbee/const_esp32.py index bfc4d93d5b..2e1b09fb22 100644 --- a/esphome/components/zigbee/const_esp32.py +++ b/esphome/components/zigbee/const_esp32.py @@ -9,6 +9,7 @@ SCALE = "scale" CONF_ATTRIBUTE_ID = "attribute_id" KEY_ZIGBEE_EP = "zigbee_ep" KEY_ZIGBEE_EP_NO_NUM = "zigbee_ep_no_num" +KEY_ZIGBEE_FIRST_EP_CL = "zigbee_first_ep_cl" DEVICE_ID = { "RANGE_EXTENDER": cg.RawExpression("EZB_ZHA_RANGE_EXTENDER_DEVICE_ID"), @@ -18,11 +19,13 @@ DEVICE_ID = { cluster_id = cg.esphome_ns.enum("ezb_zcl_cluster_id_e") CLUSTER_ID = { "BASIC": cluster_id.EZB_ZCL_CLUSTER_ID_BASIC, + "TIME": cluster_id.EZB_ZCL_CLUSTER_ID_TIME, "BINARY_INPUT": cluster_id.EZB_ZCL_CLUSTER_ID_BINARY_INPUT, "ANALOG_INPUT": cluster_id.EZB_ZCL_CLUSTER_ID_ANALOG_INPUT, } CLUSTER_ROLE = { "SERVER": cg.RawExpression("EZB_ZCL_CLUSTER_SERVER"), + "CLIENT": cg.RawExpression("EZB_ZCL_CLUSTER_CLIENT"), } attr_type = cg.esphome_ns.enum("ezb_zcl_attr_type_e") ATTR_TYPE = { diff --git a/esphome/components/zigbee/time/__init__.py b/esphome/components/zigbee/time/__init__.py index 3acab0076f..74f81df2e9 100644 --- a/esphome/components/zigbee/time/__init__.py +++ b/esphome/components/zigbee/time/__init__.py @@ -1,13 +1,15 @@ import esphome.codegen as cg from esphome.components import time as time_ import esphome.config_validation as cv -from esphome.const import CONF_ID +from esphome.const import CONF_ID, CONF_UPDATE_INTERVAL from esphome.core import CORE from esphome.types import ConfigType from .. import consume_endpoint from ..const import zigbee_ns +from ..const_esp32 import ROLE from ..const_zephyr import CONF_ZIGBEE_ID +from ..zigbee_ep_esp32 import add_clusters_to_first_ep, get_first_ep_num from ..zigbee_zephyr import ( ZigbeeClusterDesc, ZigbeeComponent, @@ -22,26 +24,52 @@ DEPENDENCIES = ["zigbee"] ZigbeeTime = zigbee_ns.class_("ZigbeeTime", time_.RealTimeClock) + +def _validate_zigbee_time(config: ConfigType) -> ConfigType: + if CORE.is_nrf52: + return consume_endpoint(config) + if CORE.is_esp32: + cl = [ + { + CONF_ID: "TIME", + ROLE: "CLIENT", + }, + { + CONF_ID: "TIME", + ROLE: "SERVER", + }, + ] + add_clusters_to_first_ep(cl) + return config + + CONFIG_SCHEMA = cv.All( time_.TIME_SCHEMA.extend( { cv.GenerateID(): cv.declare_id(ZigbeeTime), - cv.OnlyWith(CONF_ZIGBEE_ID, ["nrf52", "zigbee"]): cv.use_id( - ZigbeeComponent - ), + cv.GenerateID(CONF_ZIGBEE_ID): cv.use_id(ZigbeeComponent), + cv.SplitDefault( + CONF_UPDATE_INTERVAL, + nrf52="1s", + esp32="15min", + ): cv.update_interval, # override default from TIME_SCHEMA. Remove once nrf52 implementation is aligned. } - ) - .extend(cv.COMPONENT_SCHEMA) - .extend(cv.polling_component_schema("1s")), - consume_endpoint, + ).extend(cv.COMPONENT_SCHEMA), + _validate_zigbee_time, ) async def to_code(config: ConfigType) -> None: - CORE.add_job(_add_time, config) + if CORE.using_zephyr: + CORE.add_job(_add_time_zephyr, config) + if CORE.is_esp32: + zb = await cg.get_variable(config[CONF_ZIGBEE_ID]) + var = cg.new_Pvariable(config[CONF_ID], zb, get_first_ep_num()) + await cg.register_component(var, config) + await time_.register_time(var, config) -async def _add_time(config: ConfigType) -> None: +async def _add_time_zephyr(config: ConfigType) -> None: slot_index = get_slot_index() # Create unique names for this sensor's variables based on slot index diff --git a/esphome/components/zigbee/time/zigbee_time_esp32.cpp b/esphome/components/zigbee/time/zigbee_time_esp32.cpp new file mode 100644 index 0000000000..5567295782 --- /dev/null +++ b/esphome/components/zigbee/time/zigbee_time_esp32.cpp @@ -0,0 +1,118 @@ +#include "zigbee_time_esp32.h" +#if defined(USE_ZIGBEE) && defined(USE_ESP32) && defined(USE_TIME) +#include "esphome/core/log.h" +#include "esphome/core/application.h" + +namespace esphome::zigbee { + +static const char *const TAG = "zigbee.time"; + +// This time standard is the number of +// seconds since 0 hrs 0 mins 0 sec on 1st January 2000 UTC (Universal Coordinated Time). +constexpr time_t EPOCH_2000 = 946684800; + +static ZigbeeTime *global_time = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + +void ZigbeeTime::setup() { + global_time = this; + if (this->parent_->is_started()) { + this->register_zb_time_(); + } else { + this->parent_->add_on_start_callback([this]() { this->register_zb_time_(); }); + } +} + +void ZigbeeTime::register_zb_time_() { + ezb_zcl_time_interface_t time_interface = { + .get_utc_time = esphome::zigbee::ZigbeeTime::get_utc_time, + .set_utc_time = esphome::zigbee::ZigbeeTime::set_utc_time, + }; + ezb_err_t ret; + if (!esp_zigbee_lock_acquire(10 / portTICK_PERIOD_MS)) { + this->set_timeout("zb_time_register", 100, [this]() { this->register_zb_time_(); }); + return; + } + ret = ezb_zcl_time_server_interface_register(this->endpoint_, time_interface); + esp_zigbee_lock_release(); + if (ret != EZB_ERR_NONE) { + ESP_LOGW(TAG, "Setup failed: %d", ret); + this->mark_failed(); + return; + } + this->registered_ = true; + this->parent_->add_on_join_callback([this](bool x) { this->update(); }); + if (this->parent_->is_joined()) { + this->update(); + } +} + +void ZigbeeTime::status_cb(ezb_err_t status) { + if (status == EZB_ERR_NONE) { + ESP_LOGV(TAG, "Time synchronization successful"); + } else if (status == EZB_ERR_TIMEOUT) { + ESP_LOGW(TAG, "Time synchronization timed out"); + } else { + ESP_LOGW(TAG, "Time synchronization failed with error: %d", status); + } +} + +void ZigbeeTime::update() { + if (this->parent_->is_joined() && this->registered_) { + if (esp_zigbee_lock_acquire(10 / portTICK_PERIOD_MS)) { + ESP_LOGV(TAG, "Updating time sync from Zigbee network..."); + ezb_zcl_time_server_synchronize_time(this->endpoint_, 10, esphome::zigbee::ZigbeeTime::status_cb, + EZB_ZCL_TIME_SERVER_RANK_MASTER); + esp_zigbee_lock_release(); + this->retry_count_ = 0; + } else { + if (this->retry_count_ == 0) { + ESP_LOGW(TAG, "Could not acquire Zigbee lock to synchronize time, will retry maximum 3 times"); + } + if (this->retry_count_ < 3) { + this->set_timeout("zb_time_sync", 100, [this]() { this->update(); }); + this->retry_count_++; + } else { + ESP_LOGW(TAG, "Could not acquire Zigbee lock to synchronize time"); + this->retry_count_ = 0; + } + } + } else { + ESP_LOGD(TAG, "Not connected to Zigbee network, cannot synchronize time"); + } +} + +uint32_t ZigbeeTime::get_utc_time() { + const time_t now = global_time->timestamp_now(); + if (now < EPOCH_2000) { + return 0xFFFFFFFF; // ZCL invalid UTCTime + } + return (uint32_t) (now - EPOCH_2000); +} + +void ZigbeeTime::set_utc_time(uint32_t utc) { + // prevent overflow + if (utc <= (std::numeric_limits::max() - EPOCH_2000)) { + global_time->set_epoch_time(utc + EPOCH_2000); + } +} + +void ZigbeeTime::set_epoch_time(uint32_t utc) { + // called from zigbee task, defer to main loop + this->defer([this, utc]() { + ESP_LOGV(TAG, "Setting device time to UTC: %u", static_cast(utc)); + this->synchronize_epoch_(utc); + }); + App.wake_loop_threadsafe(); +} + +void ZigbeeTime::dump_config() { + ESP_LOGCONFIG(TAG, + "Zigbee Time\n" + " Endpoint: %u", + this->endpoint_); + RealTimeClock::dump_config(); +} + +} // namespace esphome::zigbee + +#endif diff --git a/esphome/components/zigbee/time/zigbee_time_esp32.h b/esphome/components/zigbee/time/zigbee_time_esp32.h new file mode 100644 index 0000000000..4a47137745 --- /dev/null +++ b/esphome/components/zigbee/time/zigbee_time_esp32.h @@ -0,0 +1,34 @@ +#pragma once +#include "esphome/core/defines.h" +#if defined(USE_ZIGBEE) && defined(USE_ESP32) && defined(USE_TIME) + +#include "esphome/core/component.h" +#include "esphome/components/time/real_time_clock.h" +#include "../zigbee_esp32.h" + +namespace esphome::zigbee { + +class ZigbeeComponent; + +class ZigbeeTime final : public time::RealTimeClock { + public: + ZigbeeTime(ZigbeeComponent *parent, uint8_t ep) : parent_(parent), endpoint_(ep) {} + void setup() override; + void update() override; + void dump_config() override; + void set_epoch_time(uint32_t utc); + + protected: + void register_zb_time_(); + static void set_utc_time(uint32_t utc); + static uint32_t get_utc_time(); + static void status_cb(ezb_err_t status); + + ZigbeeComponent *parent_; + uint8_t endpoint_; + uint8_t retry_count_{0}; + bool registered_{false}; +}; + +} // namespace esphome::zigbee +#endif diff --git a/esphome/components/zigbee/zigbee_ep_esp32.py b/esphome/components/zigbee/zigbee_ep_esp32.py index c2001c66d6..700267ef50 100644 --- a/esphome/components/zigbee/zigbee_ep_esp32.py +++ b/esphome/components/zigbee/zigbee_ep_esp32.py @@ -18,6 +18,7 @@ from .const_esp32 import ( DEVICE_TYPE, KEY_ZIGBEE_EP, KEY_ZIGBEE_EP_NO_NUM, + KEY_ZIGBEE_FIRST_EP_CL, ROLE, ) @@ -95,11 +96,11 @@ def _get_next_ep_num(eps: list[int]) -> int: def _compare_clusters( - existing_ep: dict[str, Any], - ep: dict[str, Any], + existing_cl_list: list[dict[str, Any]], + cl_list: list[dict[str, Any]], ) -> tuple[str | int, str] | None: - existing_clusters = [(cl[CONF_ID], cl[ROLE]) for cl in existing_ep[CONF_CLUSTERS]] - for cl in [(cl[CONF_ID], cl[ROLE]) for cl in ep[CONF_CLUSTERS]]: + existing_clusters = [(cl[CONF_ID], cl[ROLE]) for cl in existing_cl_list] + for cl in [(cl[CONF_ID], cl[ROLE]) for cl in cl_list]: if cl in existing_clusters: return cl return None @@ -110,7 +111,7 @@ def _merge_endpoints( ep: dict[str, Any], use_type: bool | None, ) -> bool: - if _compare_clusters(existing_ep, ep): + if _compare_clusters(existing_ep.get(CONF_CLUSTERS, []), ep.get(CONF_CLUSTERS, [])): return False if ( ep.get(DEVICE_TYPE) @@ -200,6 +201,17 @@ def create_ep(router: bool) -> None: # clear list so that it is not processed again del zb_data[KEY_ZIGBEE_EP_NO_NUM] + # Add clusters to first ep + cl_list: list[dict] = zb_data.setdefault(KEY_ZIGBEE_FIRST_EP_CL, []) + if cl_list: + first_ep = ep_dict[get_first_ep_num()] + first_ep.setdefault(CONF_CLUSTERS, []) + if cl := _compare_clusters(first_ep[CONF_CLUSTERS], cl_list): + raise cv.Invalid( + f"Endpoint {get_first_ep_num()} has more than one cluster with cluster id {cl[0]} and role {cl[1]}." + ) + first_ep[CONF_CLUSTERS] += cl_list + del zb_data[KEY_ZIGBEE_FIRST_EP_CL] # Add default device type to endpoints that have none for ep in ep_dict.values(): @@ -207,6 +219,15 @@ def create_ep(router: bool) -> None: ep[DEVICE_TYPE] = "CUSTOM_ATTR" +def get_first_ep_num() -> int | None: + """Return the number of the first endpoint.""" + zb_data = CORE.data.setdefault(KEY_ZIGBEE, {}) + ep_dict: dict[int, dict] = zb_data.setdefault(KEY_ZIGBEE_EP, {}) + if ep_dict: + return min(ep_dict.keys()) + return None + + def add_ep(ep: dict[str, Any], ep_num: int | None, use_type: bool | None) -> None: """Add a Zigbee endpoint configuration to CORE.data. @@ -230,8 +251,8 @@ def add_ep(ep: dict[str, Any], ep_num: int | None, use_type: bool | None) -> Non # check if the existing endpoint has same clusters existing_ep = ep_dict[ep_num] if cl := _compare_clusters( - existing_ep, - ep, + existing_ep.get(CONF_CLUSTERS, []), + ep.get(CONF_CLUSTERS, []), ): raise cv.Invalid( f"Endpoint {ep_num} has more than one cluster with cluster id {cl[0]} and role {cl[1]}." @@ -245,3 +266,21 @@ def add_ep(ep: dict[str, Any], ep_num: int | None, use_type: bool | None) -> Non if use_type or ep.get(DEVICE_TYPE): ep[CONF_USE_DEVICE_TYPE] = {ep.get(DEVICE_TYPE): use_type} ep_dict[ep_num] = ep + + +def add_clusters_to_first_ep(cl: list[dict[str, Any]]) -> None: + """Add a list of Zigbee clusters to CORE.data. + + Args: + cl: list of cluster dictonaries. + """ + zb_data = CORE.data.setdefault(KEY_ZIGBEE, {}) + cl_list: list[dict] = zb_data.setdefault(KEY_ZIGBEE_FIRST_EP_CL, []) + if cluster := _compare_clusters( + cl_list, + cl, + ): + raise cv.Invalid( + f"Only one cluster with cluster id {cluster[0]} and role {cluster[1]} can be added to first endpoint." + ) + cl_list += cl diff --git a/esphome/components/zigbee/zigbee_helpers_esp32.c b/esphome/components/zigbee/zigbee_helpers_esp32.c index 150be612f6..0793669955 100644 --- a/esphome/components/zigbee/zigbee_helpers_esp32.c +++ b/esphome/components/zigbee/zigbee_helpers_esp32.c @@ -30,6 +30,8 @@ ezb_zcl_cluster_desc_t esphome_zb_default_cluster_dscr_create(uint16_t cluster_i return ezb_zcl_basic_create_cluster_desc(NULL, role_mask); case EZB_ZCL_CLUSTER_ID_IDENTIFY: return ezb_zcl_identify_create_cluster_desc(NULL, role_mask); + case EZB_ZCL_CLUSTER_ID_TIME: + return ezb_zcl_time_create_cluster_desc(NULL, role_mask); case EZB_ZCL_CLUSTER_ID_ANALOG_INPUT: return ezb_zcl_analog_input_create_cluster_desc(NULL, role_mask); case EZB_ZCL_CLUSTER_ID_BINARY_INPUT: @@ -49,6 +51,8 @@ ezb_err_t esphome_zb_cluster_add_attr(uint16_t cluster_id, ezb_zcl_cluster_desc_ return ezb_zcl_basic_cluster_desc_add_attr(cluster_desc, attr_id, value_p); case EZB_ZCL_CLUSTER_ID_IDENTIFY: return ezb_zcl_identify_cluster_desc_add_attr(cluster_desc, attr_id, value_p); + case EZB_ZCL_CLUSTER_ID_TIME: + return ezb_zcl_time_cluster_desc_add_attr(cluster_desc, attr_id, value_p); case EZB_ZCL_CLUSTER_ID_ANALOG_INPUT: return ezb_zcl_analog_input_cluster_desc_add_attr(cluster_desc, attr_id, value_p); case EZB_ZCL_CLUSTER_ID_BINARY_INPUT: diff --git a/tests/components/zigbee/common.yaml b/tests/components/zigbee/common.yaml index cc0d28ea61..4518e39060 100644 --- a/tests/components/zigbee/common.yaml +++ b/tests/components/zigbee/common.yaml @@ -41,3 +41,6 @@ number: min_value: 2 max_value: 100 step: 1 + +time: + - platform: zigbee diff --git a/tests/components/zigbee/common_nrf52.yaml b/tests/components/zigbee/common_nrf52.yaml index c05c4053a5..2da6570618 100644 --- a/tests/components/zigbee/common_nrf52.yaml +++ b/tests/components/zigbee/common_nrf52.yaml @@ -10,6 +10,3 @@ zigbee: on_start: then: - logger.log: "Started zigbee stack" - -time: - - platform: zigbee diff --git a/tests/components/zigbee/test-router.esp32-c6-idf.yaml b/tests/components/zigbee/test-router.esp32-c6-idf.yaml index 228fe331e5..557cc08f04 100644 --- a/tests/components/zigbee/test-router.esp32-c6-idf.yaml +++ b/tests/components/zigbee/test-router.esp32-c6-idf.yaml @@ -5,3 +5,6 @@ zigbee: on_join: then: - logger.log: "Joined network" + +time: + - platform: zigbee