From 97d6f394dea5b7f5694ae82437efe7c08bb30d72 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Thu, 12 Feb 2026 23:04:18 +1300 Subject: [PATCH 1/8] Bump version to 2026.2.0b1 --- Doxyfile | 2 +- esphome/const.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doxyfile b/Doxyfile index efc2f464e3..16516a387f 100644 --- a/Doxyfile +++ b/Doxyfile @@ -48,7 +48,7 @@ PROJECT_NAME = ESPHome # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 2026.2.0-dev +PROJECT_NUMBER = 2026.2.0b1 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/esphome/const.py b/esphome/const.py index 00d8013cc6..3b5cccfb25 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -4,7 +4,7 @@ from enum import Enum from esphome.enum import StrEnum -__version__ = "2026.2.0-dev" +__version__ = "2026.2.0b1" ALLOWED_NAME_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789-_" VALID_SUBSTITUTIONS_CHARACTERS = ( From 0e1433329da006a6f58dfc1d5e086d93d743bf07 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Feb 2026 11:04:23 -0600 Subject: [PATCH 2/8] [api] Extract cold code from APIServer::loop() hot path (#13902) --- esphome/components/api/api_server.cpp | 130 ++++++++++++++------------ esphome/components/api/api_server.h | 5 + 2 files changed, 74 insertions(+), 61 deletions(-) diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 53b41a5c14..5503cf4db8 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -117,37 +117,7 @@ void APIServer::setup() { void APIServer::loop() { // Accept new clients only if the socket exists and has incoming connections if (this->socket_ && this->socket_->ready()) { - while (true) { - struct sockaddr_storage source_addr; - socklen_t addr_len = sizeof(source_addr); - - auto sock = this->socket_->accept_loop_monitored((struct sockaddr *) &source_addr, &addr_len); - if (!sock) - break; - - char peername[socket::SOCKADDR_STR_LEN]; - sock->getpeername_to(peername); - - // Check if we're at the connection limit - if (this->clients_.size() >= this->max_connections_) { - ESP_LOGW(TAG, "Max connections (%d), rejecting %s", this->max_connections_, peername); - // Immediately close - socket destructor will handle cleanup - sock.reset(); - continue; - } - - ESP_LOGD(TAG, "Accept %s", peername); - - auto *conn = new APIConnection(std::move(sock), this); - this->clients_.emplace_back(conn); - conn->start(); - - // First client connected - clear warning and update timestamp - if (this->clients_.size() == 1 && this->reboot_timeout_ != 0) { - this->status_clear_warning(); - this->last_connected_ = App.get_loop_component_start_time(); - } - } + this->accept_new_connections_(); } if (this->clients_.empty()) { @@ -178,46 +148,84 @@ void APIServer::loop() { while (client_index < this->clients_.size()) { auto &client = this->clients_[client_index]; - if (!client->flags_.remove) { + if (client->flags_.remove) { + // Rare case: handle disconnection (don't increment - swapped element needs processing) + this->remove_client_(client_index); + } else { // Common case: process active client client->loop(); client_index++; + } + } +} + +void APIServer::remove_client_(size_t client_index) { + auto &client = this->clients_[client_index]; + +#ifdef USE_API_USER_DEFINED_ACTION_RESPONSES + this->unregister_active_action_calls_for_connection(client.get()); +#endif + ESP_LOGV(TAG, "Remove connection %s", client->get_name()); + +#ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER + // Save client info before closing socket and removal for the trigger + char peername_buf[socket::SOCKADDR_STR_LEN]; + std::string client_name(client->get_name()); + std::string client_peername(client->get_peername_to(peername_buf)); +#endif + + // Close socket now (was deferred from on_fatal_error to allow getpeername) + client->helper_->close(); + + // Swap with the last element and pop (avoids expensive vector shifts) + if (client_index < this->clients_.size() - 1) { + std::swap(this->clients_[client_index], this->clients_.back()); + } + this->clients_.pop_back(); + + // Last client disconnected - set warning and start tracking for reboot timeout + if (this->clients_.empty() && this->reboot_timeout_ != 0) { + this->status_set_warning(); + this->last_connected_ = App.get_loop_component_start_time(); + } + +#ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER + // Fire trigger after client is removed so api.connected reflects the true state + this->client_disconnected_trigger_.trigger(client_name, client_peername); +#endif +} + +void APIServer::accept_new_connections_() { + while (true) { + struct sockaddr_storage source_addr; + socklen_t addr_len = sizeof(source_addr); + + auto sock = this->socket_->accept_loop_monitored((struct sockaddr *) &source_addr, &addr_len); + if (!sock) + break; + + char peername[socket::SOCKADDR_STR_LEN]; + sock->getpeername_to(peername); + + // Check if we're at the connection limit + if (this->clients_.size() >= this->max_connections_) { + ESP_LOGW(TAG, "Max connections (%d), rejecting %s", this->max_connections_, peername); + // Immediately close - socket destructor will handle cleanup + sock.reset(); continue; } - // Rare case: handle disconnection -#ifdef USE_API_USER_DEFINED_ACTION_RESPONSES - this->unregister_active_action_calls_for_connection(client.get()); -#endif - ESP_LOGV(TAG, "Remove connection %s", client->get_name()); + ESP_LOGD(TAG, "Accept %s", peername); -#ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER - // Save client info before closing socket and removal for the trigger - char peername_buf[socket::SOCKADDR_STR_LEN]; - std::string client_name(client->get_name()); - std::string client_peername(client->get_peername_to(peername_buf)); -#endif + auto *conn = new APIConnection(std::move(sock), this); + this->clients_.emplace_back(conn); + conn->start(); - // Close socket now (was deferred from on_fatal_error to allow getpeername) - client->helper_->close(); - - // Swap with the last element and pop (avoids expensive vector shifts) - if (client_index < this->clients_.size() - 1) { - std::swap(this->clients_[client_index], this->clients_.back()); - } - this->clients_.pop_back(); - - // Last client disconnected - set warning and start tracking for reboot timeout - if (this->clients_.empty() && this->reboot_timeout_ != 0) { - this->status_set_warning(); + // First client connected - clear warning and update timestamp + if (this->clients_.size() == 1 && this->reboot_timeout_ != 0) { + this->status_clear_warning(); this->last_connected_ = App.get_loop_component_start_time(); } - -#ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER - // Fire trigger after client is removed so api.connected reflects the true state - this->client_disconnected_trigger_.trigger(client_name, client_peername); -#endif - // Don't increment client_index since we need to process the swapped element } } diff --git a/esphome/components/api/api_server.h b/esphome/components/api/api_server.h index 6ab3cdc576..28f60343e0 100644 --- a/esphome/components/api/api_server.h +++ b/esphome/components/api/api_server.h @@ -234,6 +234,11 @@ class APIServer : public Component, #endif protected: + // Accept incoming socket connections. Only called when socket has pending connections. + void __attribute__((noinline)) accept_new_connections_(); + // Remove a disconnected client by index. Swaps with last element and pops. + void __attribute__((noinline)) remove_client_(size_t client_index); + #ifdef USE_API_NOISE bool update_noise_psk_(const SavedNoisePsk &new_psk, const LogString *save_log_msg, const LogString *fail_log_msg, const psk_t &active_psk, bool make_active); From cde8b6671932d3b75b0a323ef947f24cd9f2872f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Feb 2026 11:04:41 -0600 Subject: [PATCH 3/8] [web_server] Switch from getParam to arg API to eliminate heap allocations (#13942) --- .../captive_portal/captive_portal.cpp | 8 +- esphome/components/web_server/web_server.cpp | 94 +++++++++---------- esphome/components/web_server/web_server.h | 53 ++++------- esphome/components/web_server_idf/utils.cpp | 41 ++++---- esphome/components/web_server_idf/utils.h | 5 +- .../web_server_idf/web_server_idf.cpp | 52 ++++++++-- .../web_server_idf/web_server_idf.h | 11 +-- 7 files changed, 134 insertions(+), 130 deletions(-) diff --git a/esphome/components/captive_portal/captive_portal.cpp b/esphome/components/captive_portal/captive_portal.cpp index 8d88a10b27..5af6ab29a2 100644 --- a/esphome/components/captive_portal/captive_portal.cpp +++ b/esphome/components/captive_portal/captive_portal.cpp @@ -47,8 +47,8 @@ void CaptivePortal::handle_config(AsyncWebServerRequest *request) { request->send(stream); } void CaptivePortal::handle_wifisave(AsyncWebServerRequest *request) { - std::string ssid = request->arg("ssid").c_str(); // NOLINT(readability-redundant-string-cstr) - std::string psk = request->arg("psk").c_str(); // NOLINT(readability-redundant-string-cstr) + const auto &ssid = request->arg("ssid"); + const auto &psk = request->arg("psk"); ESP_LOGI(TAG, "Requested WiFi Settings Change:\n" " SSID='%s'\n" @@ -56,10 +56,10 @@ void CaptivePortal::handle_wifisave(AsyncWebServerRequest *request) { ssid.c_str(), psk.c_str()); #ifdef USE_ESP8266 // ESP8266 is single-threaded, call directly - wifi::global_wifi_component->save_wifi_sta(ssid, psk); + wifi::global_wifi_component->save_wifi_sta(ssid.c_str(), psk.c_str()); #else // Defer save to main loop thread to avoid NVS operations from HTTP thread - this->defer([ssid, psk]() { wifi::global_wifi_component->save_wifi_sta(ssid, psk); }); + this->defer([ssid, psk]() { wifi::global_wifi_component->save_wifi_sta(ssid.c_str(), psk.c_str()); }); #endif request->redirect(ESPHOME_F("/?save")); } diff --git a/esphome/components/web_server/web_server.cpp b/esphome/components/web_server/web_server.cpp index 7da8b49c6d..c7a0639382 100644 --- a/esphome/components/web_server/web_server.cpp +++ b/esphome/components/web_server/web_server.cpp @@ -585,8 +585,7 @@ static void set_json_icon_state_value(JsonObject &root, EntityBase *obj, const c // Helper to get request detail parameter static JsonDetail get_request_detail(AsyncWebServerRequest *request) { - auto *param = request->getParam(ESPHOME_F("detail")); - return (param && param->value() == "all") ? DETAIL_ALL : DETAIL_STATE; + return request->arg(ESPHOME_F("detail")) == "all" ? DETAIL_ALL : DETAIL_STATE; } #ifdef USE_SENSOR @@ -863,10 +862,10 @@ void WebServer::handle_fan_request(AsyncWebServerRequest *request, const UrlMatc } auto call = is_on ? obj->turn_on() : obj->turn_off(); - parse_int_param_(request, ESPHOME_F("speed_level"), call, &decltype(call)::set_speed); + parse_num_param_(request, ESPHOME_F("speed_level"), call, &decltype(call)::set_speed); - if (request->hasParam(ESPHOME_F("oscillation"))) { - auto speed = request->getParam(ESPHOME_F("oscillation"))->value(); + if (request->hasArg(ESPHOME_F("oscillation"))) { + auto speed = request->arg(ESPHOME_F("oscillation")); auto val = parse_on_off(speed.c_str()); switch (val) { case PARSE_ON: @@ -1042,14 +1041,14 @@ void WebServer::handle_cover_request(AsyncWebServerRequest *request, const UrlMa } auto traits = obj->get_traits(); - if ((request->hasParam(ESPHOME_F("position")) && !traits.get_supports_position()) || - (request->hasParam(ESPHOME_F("tilt")) && !traits.get_supports_tilt())) { + if ((request->hasArg(ESPHOME_F("position")) && !traits.get_supports_position()) || + (request->hasArg(ESPHOME_F("tilt")) && !traits.get_supports_tilt())) { request->send(409); return; } - parse_float_param_(request, ESPHOME_F("position"), call, &decltype(call)::set_position); - parse_float_param_(request, ESPHOME_F("tilt"), call, &decltype(call)::set_tilt); + parse_num_param_(request, ESPHOME_F("position"), call, &decltype(call)::set_position); + parse_num_param_(request, ESPHOME_F("tilt"), call, &decltype(call)::set_tilt); DEFER_ACTION(call, call.perform()); request->send(200); @@ -1108,7 +1107,7 @@ void WebServer::handle_number_request(AsyncWebServerRequest *request, const UrlM } auto call = obj->make_call(); - parse_float_param_(request, ESPHOME_F("value"), call, &decltype(call)::set_value); + parse_num_param_(request, ESPHOME_F("value"), call, &decltype(call)::set_value); DEFER_ACTION(call, call.perform()); request->send(200); @@ -1176,12 +1175,13 @@ void WebServer::handle_date_request(AsyncWebServerRequest *request, const UrlMat auto call = obj->make_call(); - if (!request->hasParam(ESPHOME_F("value"))) { + const auto &value = request->arg(ESPHOME_F("value")); + // Arduino String has isEmpty() not empty(), use length() for cross-platform compatibility + if (value.length() == 0) { // NOLINT(readability-container-size-empty) request->send(409); return; } - - parse_string_param_(request, ESPHOME_F("value"), call, &decltype(call)::set_date); + call.set_date(value.c_str(), value.length()); DEFER_ACTION(call, call.perform()); request->send(200); @@ -1236,12 +1236,13 @@ void WebServer::handle_time_request(AsyncWebServerRequest *request, const UrlMat auto call = obj->make_call(); - if (!request->hasParam(ESPHOME_F("value"))) { + const auto &value = request->arg(ESPHOME_F("value")); + // Arduino String has isEmpty() not empty(), use length() for cross-platform compatibility + if (value.length() == 0) { // NOLINT(readability-container-size-empty) request->send(409); return; } - - parse_string_param_(request, ESPHOME_F("value"), call, &decltype(call)::set_time); + call.set_time(value.c_str(), value.length()); DEFER_ACTION(call, call.perform()); request->send(200); @@ -1295,12 +1296,13 @@ void WebServer::handle_datetime_request(AsyncWebServerRequest *request, const Ur auto call = obj->make_call(); - if (!request->hasParam(ESPHOME_F("value"))) { + const auto &value = request->arg(ESPHOME_F("value")); + // Arduino String has isEmpty() not empty(), use length() for cross-platform compatibility + if (value.length() == 0) { // NOLINT(readability-container-size-empty) request->send(409); return; } - - parse_string_param_(request, ESPHOME_F("value"), call, &decltype(call)::set_datetime); + call.set_datetime(value.c_str(), value.length()); DEFER_ACTION(call, call.perform()); request->send(200); @@ -1479,10 +1481,14 @@ void WebServer::handle_climate_request(AsyncWebServerRequest *request, const Url parse_string_param_(request, ESPHOME_F("swing_mode"), call, &decltype(call)::set_swing_mode); // Parse temperature parameters - parse_float_param_(request, ESPHOME_F("target_temperature_high"), call, - &decltype(call)::set_target_temperature_high); - parse_float_param_(request, ESPHOME_F("target_temperature_low"), call, &decltype(call)::set_target_temperature_low); - parse_float_param_(request, ESPHOME_F("target_temperature"), call, &decltype(call)::set_target_temperature); + // static_cast needed to disambiguate overloaded setters (float vs optional) + using ClimateCall = decltype(call); + parse_num_param_(request, ESPHOME_F("target_temperature_high"), call, + static_cast(&ClimateCall::set_target_temperature_high)); + parse_num_param_(request, ESPHOME_F("target_temperature_low"), call, + static_cast(&ClimateCall::set_target_temperature_low)); + parse_num_param_(request, ESPHOME_F("target_temperature"), call, + static_cast(&ClimateCall::set_target_temperature)); DEFER_ACTION(call, call.perform()); request->send(200); @@ -1723,12 +1729,12 @@ void WebServer::handle_valve_request(AsyncWebServerRequest *request, const UrlMa } auto traits = obj->get_traits(); - if (request->hasParam(ESPHOME_F("position")) && !traits.get_supports_position()) { + if (request->hasArg(ESPHOME_F("position")) && !traits.get_supports_position()) { request->send(409); return; } - parse_float_param_(request, ESPHOME_F("position"), call, &decltype(call)::set_position); + parse_num_param_(request, ESPHOME_F("position"), call, &decltype(call)::set_position); DEFER_ACTION(call, call.perform()); request->send(200); @@ -1872,12 +1878,12 @@ void WebServer::handle_water_heater_request(AsyncWebServerRequest *request, cons parse_string_param_(request, ESPHOME_F("mode"), base_call, &water_heater::WaterHeaterCall::set_mode); // Parse temperature parameters - parse_float_param_(request, ESPHOME_F("target_temperature"), base_call, - &water_heater::WaterHeaterCall::set_target_temperature); - parse_float_param_(request, ESPHOME_F("target_temperature_low"), base_call, - &water_heater::WaterHeaterCall::set_target_temperature_low); - parse_float_param_(request, ESPHOME_F("target_temperature_high"), base_call, - &water_heater::WaterHeaterCall::set_target_temperature_high); + parse_num_param_(request, ESPHOME_F("target_temperature"), base_call, + &water_heater::WaterHeaterCall::set_target_temperature); + parse_num_param_(request, ESPHOME_F("target_temperature_low"), base_call, + &water_heater::WaterHeaterCall::set_target_temperature_low); + parse_num_param_(request, ESPHOME_F("target_temperature_high"), base_call, + &water_heater::WaterHeaterCall::set_target_temperature_high); // Parse away mode parameter parse_bool_param_(request, ESPHOME_F("away"), base_call, &water_heater::WaterHeaterCall::set_away); @@ -1981,16 +1987,16 @@ void WebServer::handle_infrared_request(AsyncWebServerRequest *request, const Ur auto call = obj->make_call(); // Parse carrier frequency (optional) - if (request->hasParam(ESPHOME_F("carrier_frequency"))) { - auto value = parse_number(request->getParam(ESPHOME_F("carrier_frequency"))->value().c_str()); + { + auto value = parse_number(request->arg(ESPHOME_F("carrier_frequency")).c_str()); if (value.has_value()) { call.set_carrier_frequency(*value); } } // Parse repeat count (optional, defaults to 1) - if (request->hasParam(ESPHOME_F("repeat_count"))) { - auto value = parse_number(request->getParam(ESPHOME_F("repeat_count"))->value().c_str()); + { + auto value = parse_number(request->arg(ESPHOME_F("repeat_count")).c_str()); if (value.has_value()) { call.set_repeat_count(*value); } @@ -1998,18 +2004,12 @@ void WebServer::handle_infrared_request(AsyncWebServerRequest *request, const Ur // Parse base64url-encoded raw timings (required) // Base64url is URL-safe: uses A-Za-z0-9-_ (no special characters needing escaping) - if (!request->hasParam(ESPHOME_F("data"))) { - request->send(400, ESPHOME_F("text/plain"), ESPHOME_F("Missing 'data' parameter")); - return; - } + const auto &data_arg = request->arg(ESPHOME_F("data")); - // .c_str() is required for Arduino framework where value() returns Arduino String instead of std::string - std::string encoded = - request->getParam(ESPHOME_F("data"))->value().c_str(); // NOLINT(readability-redundant-string-cstr) - - // Validate base64url is not empty - if (encoded.empty()) { - request->send(400, ESPHOME_F("text/plain"), ESPHOME_F("Empty 'data' parameter")); + // Validate base64url is not empty (also catches missing parameter since arg() returns empty string) + // Arduino String has isEmpty() not empty(), use length() for cross-platform compatibility + if (data_arg.length() == 0) { // NOLINT(readability-container-size-empty) + request->send(400, ESPHOME_F("text/plain"), ESPHOME_F("Missing or empty 'data' parameter")); return; } @@ -2017,7 +2017,7 @@ void WebServer::handle_infrared_request(AsyncWebServerRequest *request, const Ur // it outlives the call - set_raw_timings_base64url stores a pointer, so the string // must remain valid until perform() completes. // ESP8266 also needs this because ESPAsyncWebServer callbacks run in "sys" context. - this->defer([call, encoded = std::move(encoded)]() mutable { + this->defer([call, encoded = std::string(data_arg.c_str(), data_arg.length())]() mutable { call.set_raw_timings_base64url(encoded); call.perform(); }); diff --git a/esphome/components/web_server/web_server.h b/esphome/components/web_server/web_server.h index ce09ebf7a9..026da763ea 100644 --- a/esphome/components/web_server/web_server.h +++ b/esphome/components/web_server/web_server.h @@ -513,11 +513,9 @@ class WebServer : public Controller, template void parse_light_param_(AsyncWebServerRequest *request, ParamNameType param_name, T &call, Ret (T::*setter)(float), float scale = 1.0f) { - if (request->hasParam(param_name)) { - auto value = parse_number(request->getParam(param_name)->value().c_str()); - if (value.has_value()) { - (call.*setter)(*value / scale); - } + auto value = parse_number(request->arg(param_name).c_str()); + if (value.has_value()) { + (call.*setter)(*value / scale); } } @@ -525,34 +523,19 @@ class WebServer : public Controller, template void parse_light_param_uint_(AsyncWebServerRequest *request, ParamNameType param_name, T &call, Ret (T::*setter)(uint32_t), uint32_t scale = 1) { - if (request->hasParam(param_name)) { - auto value = parse_number(request->getParam(param_name)->value().c_str()); - if (value.has_value()) { - (call.*setter)(*value * scale); - } + auto value = parse_number(request->arg(param_name).c_str()); + if (value.has_value()) { + (call.*setter)(*value * scale); } } #endif - // Generic helper to parse and apply a float parameter - template - void parse_float_param_(AsyncWebServerRequest *request, ParamNameType param_name, T &call, Ret (T::*setter)(float)) { - if (request->hasParam(param_name)) { - auto value = parse_number(request->getParam(param_name)->value().c_str()); - if (value.has_value()) { - (call.*setter)(*value); - } - } - } - - // Generic helper to parse and apply an int parameter - template - void parse_int_param_(AsyncWebServerRequest *request, ParamNameType param_name, T &call, Ret (T::*setter)(int)) { - if (request->hasParam(param_name)) { - auto value = parse_number(request->getParam(param_name)->value().c_str()); - if (value.has_value()) { - (call.*setter)(*value); - } + // Generic helper to parse and apply a numeric parameter + template + void parse_num_param_(AsyncWebServerRequest *request, ParamNameType param_name, T &call, Ret (T::*setter)(NumT)) { + auto value = parse_number(request->arg(param_name).c_str()); + if (value.has_value()) { + (call.*setter)(*value); } } @@ -560,10 +543,9 @@ class WebServer : public Controller, template void parse_string_param_(AsyncWebServerRequest *request, ParamNameType param_name, T &call, Ret (T::*setter)(const std::string &)) { - if (request->hasParam(param_name)) { - // .c_str() is required for Arduino framework where value() returns Arduino String instead of std::string - std::string value = request->getParam(param_name)->value().c_str(); // NOLINT(readability-redundant-string-cstr) - (call.*setter)(value); + if (request->hasArg(param_name)) { + const auto &value = request->arg(param_name); + (call.*setter)(std::string(value.c_str(), value.length())); } } @@ -573,8 +555,9 @@ class WebServer : public Controller, // Invalid values are ignored (setter not called) template void parse_bool_param_(AsyncWebServerRequest *request, ParamNameType param_name, T &call, Ret (T::*setter)(bool)) { - if (request->hasParam(param_name)) { - auto param_value = request->getParam(param_name)->value(); + const auto ¶m_value = request->arg(param_name); + // Arduino String has isEmpty() not empty(), use length() for cross-platform compatibility + if (param_value.length() > 0) { // NOLINT(readability-container-size-empty) // First check on/off (default), then true/false (custom) auto val = parse_on_off(param_value.c_str()); if (val == PARSE_NONE) { diff --git a/esphome/components/web_server_idf/utils.cpp b/esphome/components/web_server_idf/utils.cpp index 81ae626277..c58ca2a24f 100644 --- a/esphome/components/web_server_idf/utils.cpp +++ b/esphome/components/web_server_idf/utils.cpp @@ -1,17 +1,13 @@ #ifdef USE_ESP32 -#include #include #include #include "esphome/core/helpers.h" -#include "esphome/core/log.h" #include "http_parser.h" #include "utils.h" namespace esphome::web_server_idf { -static const char *const TAG = "web_server_idf_utils"; - size_t url_decode(char *str) { char *start = str; char *ptr = str, buf; @@ -54,32 +50,15 @@ optional request_get_header(httpd_req_t *req, const char *name) { return {str}; } -optional request_get_url_query(httpd_req_t *req) { - auto len = httpd_req_get_url_query_len(req); - if (len == 0) { - return {}; - } - - std::string str; - str.resize(len); - - auto res = httpd_req_get_url_query_str(req, &str[0], len + 1); - if (res != ESP_OK) { - ESP_LOGW(TAG, "Can't get query for request: %s", esp_err_to_name(res)); - return {}; - } - - return {str}; -} - optional query_key_value(const char *query_url, size_t query_len, const char *key) { if (query_url == nullptr || query_len == 0) { return {}; } - // Use stack buffer for typical query strings, heap fallback for large ones - SmallBufferWithHeapFallback<256, char> val(query_len); - + // Value can't exceed query_len. Use small stack buffer for typical values, + // heap fallback for long ones (e.g. base64 IR data) to limit stack usage + // since callers may also have stack buffers for the query string. + SmallBufferWithHeapFallback<128, char> val(query_len); if (httpd_query_key_value(query_url, key, val.get(), query_len) != ESP_OK) { return {}; } @@ -88,6 +67,18 @@ optional query_key_value(const char *query_url, size_t query_len, c return {val.get()}; } +bool query_has_key(const char *query_url, size_t query_len, const char *key) { + if (query_url == nullptr || query_len == 0) { + return false; + } + // Minimal buffer — we only care if the key exists, not the value + char buf[1]; + // httpd_query_key_value returns ESP_OK if found, ESP_ERR_HTTPD_RESULT_TRUNC if found + // but value truncated (expected with 1-byte buffer), or other errors for invalid input + auto err = httpd_query_key_value(query_url, key, buf, sizeof(buf)); + return err == ESP_OK || err == ESP_ERR_HTTPD_RESULT_TRUNC; +} + // Helper function for case-insensitive string region comparison bool str_ncmp_ci(const char *s1, const char *s2, size_t n) { for (size_t i = 0; i < n; i++) { diff --git a/esphome/components/web_server_idf/utils.h b/esphome/components/web_server_idf/utils.h index 87635c0458..027a2f7b6c 100644 --- a/esphome/components/web_server_idf/utils.h +++ b/esphome/components/web_server_idf/utils.h @@ -13,11 +13,8 @@ size_t url_decode(char *str); bool request_has_header(httpd_req_t *req, const char *name); optional request_get_header(httpd_req_t *req, const char *name); -optional request_get_url_query(httpd_req_t *req); optional query_key_value(const char *query_url, size_t query_len, const char *key); -inline optional query_key_value(const std::string &query_url, const std::string &key) { - return query_key_value(query_url.c_str(), query_url.size(), key.c_str()); -} +bool query_has_key(const char *query_url, size_t query_len, const char *key); // Helper function for case-insensitive character comparison inline bool char_equals_ci(char a, char b) { return ::tolower(a) == ::tolower(b); } diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index f1f89beb49..1798159e7f 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -393,13 +393,7 @@ AsyncWebParameter *AsyncWebServerRequest::getParam(const char *name) { } // Look up value from query strings - optional val = query_key_value(this->post_query_.c_str(), this->post_query_.size(), name); - if (!val.has_value()) { - auto url_query = request_get_url_query(*this); - if (url_query.has_value()) { - val = query_key_value(url_query.value().c_str(), url_query.value().size(), name); - } - } + auto val = this->find_query_value_(name); // Don't cache misses to avoid wasting memory when handlers check for // optional parameters that don't exist in the request @@ -412,6 +406,50 @@ AsyncWebParameter *AsyncWebServerRequest::getParam(const char *name) { return param; } +/// Search post_query then URL query with a callback. +/// Returns first truthy result, or value-initialized default. +/// URL query is accessed directly from req->uri (same pattern as url_to()). +template +static auto search_query_sources(httpd_req_t *req, const std::string &post_query, const char *name, Func func) + -> decltype(func(nullptr, size_t{0}, name)) { + if (!post_query.empty()) { + auto result = func(post_query.c_str(), post_query.size(), name); + if (result) { + return result; + } + } + // Use httpd API for query length, then access string directly from URI. + // http_parser identifies components by offset/length without modifying the URI string. + // This is the same pattern used by url_to(). + auto len = httpd_req_get_url_query_len(req); + if (len == 0) { + return {}; + } + const char *query = strchr(req->uri, '?'); + if (query == nullptr) { + return {}; + } + query++; // skip '?' + return func(query, len, name); +} + +optional AsyncWebServerRequest::find_query_value_(const char *name) const { + return search_query_sources(this->req_, this->post_query_, name, + [](const char *q, size_t len, const char *k) { return query_key_value(q, len, k); }); +} + +bool AsyncWebServerRequest::hasArg(const char *name) { + return search_query_sources(this->req_, this->post_query_, name, query_has_key); +} + +std::string AsyncWebServerRequest::arg(const char *name) { + auto val = this->find_query_value_(name); + if (val.has_value()) { + return std::move(val.value()); + } + return {}; +} + void AsyncWebServerResponse::addHeader(const char *name, const char *value) { httpd_resp_set_hdr(*this->req_, name, value); } diff --git a/esphome/components/web_server_idf/web_server_idf.h b/esphome/components/web_server_idf/web_server_idf.h index 6a409de74e..12df0303de 100644 --- a/esphome/components/web_server_idf/web_server_idf.h +++ b/esphome/components/web_server_idf/web_server_idf.h @@ -170,14 +170,8 @@ class AsyncWebServerRequest { AsyncWebParameter *getParam(const std::string &name) { return this->getParam(name.c_str()); } // NOLINTNEXTLINE(readability-identifier-naming) - bool hasArg(const char *name) { return this->hasParam(name); } - std::string arg(const char *name) { - auto *param = this->getParam(name); - if (param) { - return param->value(); - } - return {}; - } + bool hasArg(const char *name); + std::string arg(const char *name); std::string arg(const std::string &name) { return this->arg(name.c_str()); } operator httpd_req_t *() const { return this->req_; } @@ -192,6 +186,7 @@ class AsyncWebServerRequest { // is faster than tree/hash overhead. AsyncWebParameter stores both name and value to avoid // duplicate storage. Only successful lookups are cached to prevent cache pollution when // handlers check for optional parameters that don't exist. + optional find_query_value_(const char *name) const; std::vector params_; std::string post_query_; AsyncWebServerRequest(httpd_req_t *req) : req_(req) {} From 0dcff82bb479b131a4dbb33d00940c7a91120dc2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Feb 2026 11:14:36 -0600 Subject: [PATCH 4/8] [wifi] Deprecate wifi_ssid() in favor of wifi_ssid_to() (#13958) --- esphome/components/wifi/wifi_component.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index ac28a1bc81..53ff0d9cad 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -502,6 +502,8 @@ class WiFiComponent : public Component { } network::IPAddresses wifi_sta_ip_addresses(); + // Remove before 2026.9.0 + ESPDEPRECATED("Use wifi_ssid_to() instead. Removed in 2026.9.0", "2026.3.0") std::string wifi_ssid(); /// Write SSID to buffer without heap allocation. /// Returns pointer to buffer, or empty string if not connected. From e3a457e40268949364f776fd5ec3ff5fd833a443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Ma=C5=88as?= Date: Thu, 12 Feb 2026 18:20:54 +0100 Subject: [PATCH 5/8] [pulse_meter] Fix early edge detection (#12360) Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> --- .../pulse_meter/pulse_meter_sensor.cpp | 75 ++++++++++--------- .../pulse_meter/pulse_meter_sensor.h | 11 ++- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/esphome/components/pulse_meter/pulse_meter_sensor.cpp b/esphome/components/pulse_meter/pulse_meter_sensor.cpp index 007deb66e5..433e1f0b7e 100644 --- a/esphome/components/pulse_meter/pulse_meter_sensor.cpp +++ b/esphome/components/pulse_meter/pulse_meter_sensor.cpp @@ -38,8 +38,7 @@ void PulseMeterSensor::setup() { } void PulseMeterSensor::loop() { - // Reset the count in get before we pass it back to the ISR as set - this->get_->count_ = 0; + State state; { // Lock the interrupt so the interrupt code doesn't interfere with itself @@ -58,31 +57,35 @@ void PulseMeterSensor::loop() { } this->last_pin_val_ = current; - // Swap out set and get to get the latest state from the ISR - std::swap(this->set_, this->get_); + // Get the latest state from the ISR and reset the count in the ISR + state.last_detected_edge_us_ = this->state_.last_detected_edge_us_; + state.last_rising_edge_us_ = this->state_.last_rising_edge_us_; + state.count_ = this->state_.count_; + this->state_.count_ = 0; } const uint32_t now = micros(); // If an edge was peeked, repay the debt - if (this->peeked_edge_ && this->get_->count_ > 0) { + if (this->peeked_edge_ && state.count_ > 0) { this->peeked_edge_ = false; - this->get_->count_--; // NOLINT(clang-diagnostic-deprecated-volatile) + state.count_--; } - // If there is an unprocessed edge, and filter_us_ has passed since, count this edge early - if (this->get_->last_rising_edge_us_ != this->get_->last_detected_edge_us_ && - now - this->get_->last_rising_edge_us_ >= this->filter_us_) { + // If there is an unprocessed edge, and filter_us_ has passed since, count this edge early. + // Wait for the debt to be repaid before counting another unprocessed edge early. + if (!this->peeked_edge_ && state.last_rising_edge_us_ != state.last_detected_edge_us_ && + now - state.last_rising_edge_us_ >= this->filter_us_) { this->peeked_edge_ = true; - this->get_->last_detected_edge_us_ = this->get_->last_rising_edge_us_; - this->get_->count_++; // NOLINT(clang-diagnostic-deprecated-volatile) + state.last_detected_edge_us_ = state.last_rising_edge_us_; + state.count_++; } // Check if we detected a pulse this loop - if (this->get_->count_ > 0) { + if (state.count_ > 0) { // Keep a running total of pulses if a total sensor is configured if (this->total_sensor_ != nullptr) { - this->total_pulses_ += this->get_->count_; + this->total_pulses_ += state.count_; const uint32_t total = this->total_pulses_; this->total_sensor_->publish_state(total); } @@ -94,15 +97,15 @@ void PulseMeterSensor::loop() { this->meter_state_ = MeterState::RUNNING; } break; case MeterState::RUNNING: { - uint32_t delta_us = this->get_->last_detected_edge_us_ - this->last_processed_edge_us_; - float pulse_width_us = delta_us / float(this->get_->count_); - ESP_LOGV(TAG, "New pulse, delta: %" PRIu32 " µs, count: %" PRIu32 ", width: %.5f µs", delta_us, - this->get_->count_, pulse_width_us); + uint32_t delta_us = state.last_detected_edge_us_ - this->last_processed_edge_us_; + float pulse_width_us = delta_us / float(state.count_); + ESP_LOGV(TAG, "New pulse, delta: %" PRIu32 " µs, count: %" PRIu32 ", width: %.5f µs", delta_us, state.count_, + pulse_width_us); this->publish_state((60.0f * 1000000.0f) / pulse_width_us); } break; } - this->last_processed_edge_us_ = this->get_->last_detected_edge_us_; + this->last_processed_edge_us_ = state.last_detected_edge_us_; } // No detected edges this loop else { @@ -141,14 +144,14 @@ void IRAM_ATTR PulseMeterSensor::edge_intr(PulseMeterSensor *sensor) { // This is an interrupt handler - we can't call any virtual method from this method // Get the current time before we do anything else so the measurements are consistent const uint32_t now = micros(); - auto &state = sensor->edge_state_; - auto &set = *sensor->set_; + auto &edge_state = sensor->edge_state_; + auto &state = sensor->state_; - if ((now - state.last_sent_edge_us_) >= sensor->filter_us_) { - state.last_sent_edge_us_ = now; - set.last_detected_edge_us_ = now; - set.last_rising_edge_us_ = now; - set.count_++; // NOLINT(clang-diagnostic-deprecated-volatile) + if ((now - edge_state.last_sent_edge_us_) >= sensor->filter_us_) { + edge_state.last_sent_edge_us_ = now; + state.last_detected_edge_us_ = now; + state.last_rising_edge_us_ = now; + state.count_++; // NOLINT(clang-diagnostic-deprecated-volatile) } // This ISR is bound to rising edges, so the pin is high @@ -160,26 +163,26 @@ void IRAM_ATTR PulseMeterSensor::pulse_intr(PulseMeterSensor *sensor) { // Get the current time before we do anything else so the measurements are consistent const uint32_t now = micros(); const bool pin_val = sensor->isr_pin_.digital_read(); - auto &state = sensor->pulse_state_; - auto &set = *sensor->set_; + auto &pulse_state = sensor->pulse_state_; + auto &state = sensor->state_; // Filter length has passed since the last interrupt - const bool length = now - state.last_intr_ >= sensor->filter_us_; + const bool length = now - pulse_state.last_intr_ >= sensor->filter_us_; - if (length && state.latched_ && !sensor->last_pin_val_) { // Long enough low edge - state.latched_ = false; - } else if (length && !state.latched_ && sensor->last_pin_val_) { // Long enough high edge - state.latched_ = true; - set.last_detected_edge_us_ = state.last_intr_; - set.count_++; // NOLINT(clang-diagnostic-deprecated-volatile) + if (length && pulse_state.latched_ && !sensor->last_pin_val_) { // Long enough low edge + pulse_state.latched_ = false; + } else if (length && !pulse_state.latched_ && sensor->last_pin_val_) { // Long enough high edge + pulse_state.latched_ = true; + state.last_detected_edge_us_ = pulse_state.last_intr_; + state.count_++; // NOLINT(clang-diagnostic-deprecated-volatile) } // Due to order of operations this includes // length && latched && rising (just reset from a long low edge) // !latched && (rising || high) (noise on the line resetting the potential rising edge) - set.last_rising_edge_us_ = !state.latched_ && pin_val ? now : set.last_detected_edge_us_; + state.last_rising_edge_us_ = !pulse_state.latched_ && pin_val ? now : state.last_detected_edge_us_; - state.last_intr_ = now; + pulse_state.last_intr_ = now; sensor->last_pin_val_ = pin_val; } diff --git a/esphome/components/pulse_meter/pulse_meter_sensor.h b/esphome/components/pulse_meter/pulse_meter_sensor.h index 5800c4ec42..e46f1e615f 100644 --- a/esphome/components/pulse_meter/pulse_meter_sensor.h +++ b/esphome/components/pulse_meter/pulse_meter_sensor.h @@ -46,17 +46,16 @@ class PulseMeterSensor : public sensor::Sensor, public Component { uint32_t total_pulses_ = 0; uint32_t last_processed_edge_us_ = 0; - // This struct (and the two pointers) are used to pass data between the ISR and loop. - // These two pointers are exchanged each loop. - // Use these to send data from the ISR to the loop not the other way around (except for resetting the values). + // This struct and variable are used to pass data between the ISR and loop. + // The data from state_ is read and then count_ in state_ is reset in each loop. + // This must be done while guarded by an InterruptLock. Use this variable to send data + // from the ISR to the loop not the other way around (except for resetting count_). struct State { uint32_t last_detected_edge_us_ = 0; uint32_t last_rising_edge_us_ = 0; uint32_t count_ = 0; }; - State state_[2]; - volatile State *set_ = state_; - volatile State *get_ = state_ + 1; + volatile State state_{}; // Only use the following variables in the ISR or while guarded by an InterruptLock ISRInternalGPIOPin isr_pin_; From 7fd535179e4cc793047ebcf802b0fe174b3987cb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Feb 2026 11:47:44 -0600 Subject: [PATCH 6/8] [helpers] Add heap warnings to format_hex_pretty, deprecate ethernet/web_server std::string APIs (#13959) --- .../components/ethernet/ethernet_component.h | 2 ++ .../components/web_server_idf/web_server_idf.h | 3 ++- esphome/core/helpers.h | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/esphome/components/ethernet/ethernet_component.h b/esphome/components/ethernet/ethernet_component.h index 5a2869c5a7..b4859c308d 100644 --- a/esphome/components/ethernet/ethernet_component.h +++ b/esphome/components/ethernet/ethernet_component.h @@ -110,6 +110,8 @@ class EthernetComponent : public Component { const char *get_use_address() const; void set_use_address(const char *use_address); void get_eth_mac_address_raw(uint8_t *mac); + // Remove before 2026.9.0 + ESPDEPRECATED("Use get_eth_mac_address_pretty_into_buffer() instead. Removed in 2026.9.0", "2026.3.0") std::string get_eth_mac_address_pretty(); const char *get_eth_mac_address_pretty_into_buffer(std::span buf); eth_duplex_t get_duplex_mode(); diff --git a/esphome/components/web_server_idf/web_server_idf.h b/esphome/components/web_server_idf/web_server_idf.h index 12df0303de..74601ffda8 100644 --- a/esphome/components/web_server_idf/web_server_idf.h +++ b/esphome/components/web_server_idf/web_server_idf.h @@ -116,7 +116,8 @@ class AsyncWebServerRequest { /// Write URL (without query string) to buffer, returns StringRef pointing to buffer. /// URL is decoded (e.g., %20 -> space). StringRef url_to(std::span buffer) const; - /// Get URL as std::string. Prefer url_to() to avoid heap allocation. + // Remove before 2026.9.0 + ESPDEPRECATED("Use url_to() instead. Removed in 2026.9.0", "2026.3.0") std::string url() const { char buffer[URL_BUF_SIZE]; return std::string(this->url_to(buffer)); diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index f7de34b6d5..34c7452484 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1083,6 +1083,9 @@ template std::string format_hex(const std::array &dat * Each byte is displayed as a two-digit uppercase hex value, separated by the specified separator. * Optionally includes the total byte count in parentheses at the end. * + * @warning Allocates heap memory. Use format_hex_pretty_to() with a stack buffer instead. + * Causes heap fragmentation on long-running devices. + * * @param data Pointer to the byte array to format. * @param length Number of bytes in the array. * @param separator Character to use between hex bytes (default: '.'). @@ -1108,6 +1111,9 @@ std::string format_hex_pretty(const uint8_t *data, size_t length, char separator * * Similar to the byte array version, but formats 16-bit words as 4-digit hex values. * + * @warning Allocates heap memory. Use format_hex_pretty_to() with a stack buffer instead. + * Causes heap fragmentation on long-running devices. + * * @param data Pointer to the 16-bit word array to format. * @param length Number of 16-bit words in the array. * @param separator Character to use between hex words (default: '.'). @@ -1131,6 +1137,9 @@ std::string format_hex_pretty(const uint16_t *data, size_t length, char separato * Convenience overload for std::vector. Formats each byte as a two-digit * uppercase hex value with customizable separator. * + * @warning Allocates heap memory. Use format_hex_pretty_to() with a stack buffer instead. + * Causes heap fragmentation on long-running devices. + * * @param data Vector of bytes to format. * @param separator Character to use between hex bytes (default: '.'). * @param show_length Whether to append the byte count in parentheses (default: true). @@ -1154,6 +1163,9 @@ std::string format_hex_pretty(const std::vector &data, char separator = * Convenience overload for std::vector. Each 16-bit word is formatted * as a 4-digit uppercase hex value in big-endian order. * + * @warning Allocates heap memory. Use format_hex_pretty_to() with a stack buffer instead. + * Causes heap fragmentation on long-running devices. + * * @param data Vector of 16-bit words to format. * @param separator Character to use between hex words (default: '.'). * @param show_length Whether to append the word count in parentheses (default: true). @@ -1176,6 +1188,9 @@ std::string format_hex_pretty(const std::vector &data, char separator * Treats each character in the string as a byte and formats it in hex. * Useful for debugging binary data stored in std::string containers. * + * @warning Allocates heap memory. Use format_hex_pretty_to() with a stack buffer instead. + * Causes heap fragmentation on long-running devices. + * * @param data String whose bytes should be formatted as hex. * @param separator Character to use between hex bytes (default: '.'). * @param show_length Whether to append the byte count in parentheses (default: true). @@ -1198,6 +1213,9 @@ std::string format_hex_pretty(const std::string &data, char separator = '.', boo * Converts the integer to big-endian byte order and formats each byte as hex. * The most significant byte appears first in the output string. * + * @warning Allocates heap memory. Use format_hex_pretty_to() with a stack buffer instead. + * Causes heap fragmentation on long-running devices. + * * @tparam T Unsigned integer type (uint8_t, uint16_t, uint32_t, uint64_t, etc.). * @param val The unsigned integer value to format. * @param separator Character to use between hex bytes (default: '.'). From 58f80292642dc5a4e23fde673ed06c7b8ebc5fcb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Feb 2026 12:21:52 -0600 Subject: [PATCH 7/8] [wifi] Use memcpy-based insertion sort for scan results Replace copy-assignment with raw memcpy in the WiFi scan result insertion sort. Copy assignment on WiFiScanResult calls CompactString's destructor then placement-new for every shift, which means delete[]/new[] per shift for heap-allocated SSIDs. With 70+ networks visible (e.g., during captive portal transition showing full scan results), this caused event loop blocking from hundreds of heap allocations in a tight loop on an 80MHz ESP8266. This optimization is safe because we're permuting elements within the same array - each slot is overwritten exactly once, so no ownership duplication occurs. CompactString stores either inline data or a heap pointer, never a self-referential pointer (unlike libstdc++ std::string SSO). This was made possible by PR#13472 which replaced std::string with CompactString. Static asserts guard the memcpy safety assumptions at compile time. Confirmed on real device: event loop blocking during captive portal transition is eliminated and WiFi connection is slightly faster. --- esphome/components/wifi/wifi_component.cpp | 46 ++++++++++++++++++++-- esphome/components/wifi/wifi_component.h | 6 +++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index 61d05d7635..99c8b5f228 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -1319,20 +1319,58 @@ void WiFiComponent::start_scanning() { // Using insertion sort instead of std::stable_sort saves flash memory // by avoiding template instantiations (std::rotate, std::stable_sort, lambdas) // IMPORTANT: This sort is stable (preserves relative order of equal elements) +// +// Uses raw memcpy instead of copy assignment to avoid CompactString's +// destructor/constructor overhead (heap delete[]/new[] for long SSIDs). +// Copy assignment calls ~CompactString() then placement-new for every shift, +// which means delete[]/new[] per shift for heap-allocated SSIDs. With 70+ +// networks (e.g., captive portal showing full scan results), this caused +// event loop blocking from hundreds of heap operations in a tight loop. +// +// This is safe because we're permuting elements within the same array — +// each slot is overwritten exactly once, so no ownership duplication occurs. +// All members of WiFiScanResult are either trivially copyable (bssid, channel, +// rssi, priority, flags) or CompactString, which stores either inline data or +// a heap pointer — never a self-referential pointer (unlike std::string's SSO +// on some implementations). This was not possible before PR#13472 replaced +// std::string with CompactString, since std::string's internal layout is +// implementation-defined and may use self-referential pointers. template static void insertion_sort_scan_results(VectorType &results) { + // memcpy-based sort requires no self-referential pointers or virtual dispatch. + // These static_asserts guard the assumptions. If any fire, the memcpy sort + // must be reviewed for safety before updating the expected values. + // + // No vtable pointers (memcpy would corrupt vptr) + static_assert(!std::is_polymorphic::value, "WiFiScanResult must not have vtable"); + static_assert(!std::is_polymorphic::value, "CompactString must not have vtable"); + // Standard layout ensures predictable memory layout with no virtual bases + // and no mixed-access-specifier reordering + static_assert(std::is_standard_layout::value, "WiFiScanResult must be standard layout"); + static_assert(std::is_standard_layout::value, "CompactString must be standard layout"); + // Size checks catch added/removed fields that may need safety review + static_assert(sizeof(WiFiScanResult) == 32, "WiFiScanResult size changed - verify memcpy sort is still safe"); + static_assert(sizeof(CompactString) == 20, "CompactString size changed - verify memcpy sort is still safe"); + // Alignment must match for reinterpret_cast of key_buf to be valid + static_assert(alignof(WiFiScanResult) <= alignof(std::max_align_t), "WiFiScanResult alignment exceeds max_align_t"); const size_t size = results.size(); + constexpr size_t elem_size = sizeof(WiFiScanResult); + // Suppress warnings for intentional memcpy on non-trivially-copyable type. + // Safety is guaranteed by the static_asserts above and the permutation invariant. + // NOLINTNEXTLINE(bugprone-undefined-memory-manipulation) + auto *memcpy_fn = &memcpy; for (size_t i = 1; i < size; i++) { - // Make a copy to avoid issues with move semantics during comparison - WiFiScanResult key = results[i]; + alignas(WiFiScanResult) uint8_t key_buf[elem_size]; + memcpy_fn(key_buf, &results[i], elem_size); + const auto &key = *reinterpret_cast(key_buf); int32_t j = i - 1; // Move elements that are worse than key to the right // For stability, we only move if key is strictly better than results[j] while (j >= 0 && wifi_scan_result_is_better(key, results[j])) { - results[j + 1] = results[j]; + memcpy_fn(&results[j + 1], &results[j], elem_size); j--; } - results[j + 1] = key; + memcpy_fn(&results[j + 1], key_buf, elem_size); } } diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index 53ff0d9cad..f0ccc95abe 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -219,6 +219,12 @@ class CompactString { }; static_assert(sizeof(CompactString) == 20, "CompactString must be exactly 20 bytes"); +// CompactString is safe to memcpy for permutation-based sorting (no ownership duplication). +// Unlike libstdc++ std::string which uses a self-referential pointer (_M_p -> _M_local_buf) +// in SSO mode, CompactString stores either inline data or an external heap pointer in +// storage_[] — never a pointer to itself. These asserts guard that property. +static_assert(std::is_standard_layout::value, "CompactString must be standard layout for memcpy safety"); +static_assert(!std::is_polymorphic::value, "CompactString must not have vtable for memcpy safety"); class WiFiAP { friend class WiFiComponent; From 282ba90f6281140fd8a13cf7e20af219cdf4cf06 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Feb 2026 12:32:06 -0600 Subject: [PATCH 8/8] Address review feedback: add explicit includes and clarify comments - Add #include to both .cpp and .h for static_asserts - Clarify CompactString comment: explicitly note it is not trivially copyable, and that memcpy safety relies on validated layout property - Use memcpy_fn indirection to suppress both GCC -Wclass-memaccess and clang-tidy bugprone-undefined-memory-manipulation without platform-specific pragma guards --- esphome/components/wifi/wifi_component.cpp | 1 + esphome/components/wifi/wifi_component.h | 15 +++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index 99c8b5f228..52188b2621 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #ifdef USE_ESP32 #if (ESP_IDF_VERSION_MAJOR >= 5 && ESP_IDF_VERSION_MINOR >= 1) diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index f0ccc95abe..7ae8a2c18a 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -10,6 +10,7 @@ #include #include +#include #include #ifdef USE_LIBRETINY @@ -219,12 +220,14 @@ class CompactString { }; static_assert(sizeof(CompactString) == 20, "CompactString must be exactly 20 bytes"); -// CompactString is safe to memcpy for permutation-based sorting (no ownership duplication). -// Unlike libstdc++ std::string which uses a self-referential pointer (_M_p -> _M_local_buf) -// in SSO mode, CompactString stores either inline data or an external heap pointer in -// storage_[] — never a pointer to itself. These asserts guard that property. -static_assert(std::is_standard_layout::value, "CompactString must be standard layout for memcpy safety"); -static_assert(!std::is_polymorphic::value, "CompactString must not have vtable for memcpy safety"); +// CompactString is not trivially copyable (non-trivial destructor/copy for heap case). +// However, its layout has no self-referential pointers: storage_[] contains either inline +// data or an external heap pointer — never a pointer to itself. This is unlike libstdc++ +// std::string SSO where _M_p points to _M_local_buf within the same object. +// This property allows memcpy-based permutation sorting where each element ends up in +// exactly one slot (no ownership duplication). These asserts document that layout property. +static_assert(std::is_standard_layout::value, "CompactString must be standard layout"); +static_assert(!std::is_polymorphic::value, "CompactString must not have vtable"); class WiFiAP { friend class WiFiComponent;