From fc7e55bfdc68f61c3cacb28ebdd32f0cfad3e9e8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 5 Jan 2026 07:42:18 -1000 Subject: [PATCH] [api] Avoid heap string copies in Home Assistant state subscription callbacks (#12506) --- esphome/components/api/api_connection.cpp | 16 +++-- esphome/components/api/api_server.cpp | 36 ++++++++--- esphome/components/api/api_server.h | 26 +++++--- esphome/components/api/custom_api_device.h | 59 ++++++++++++++++--- .../homeassistant_binary_sensor.cpp | 50 ++++++++-------- .../number/homeassistant_number.cpp | 17 +++--- .../number/homeassistant_number.h | 12 ++-- .../sensor/homeassistant_sensor.cpp | 32 +++++----- .../switch/homeassistant_switch.cpp | 3 +- .../text_sensor/homeassistant_text_sensor.cpp | 20 +++---- 10 files changed, 177 insertions(+), 94 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index b173ebc8cb..27344a53ec 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1692,10 +1692,18 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes continue; } - // Create temporary string for callback (callback takes const std::string &) - // Handle empty state - std::string state(!msg.state.empty() ? msg.state.c_str() : "", msg.state.size()); - it.callback(state); + // Create null-terminated state for callback (parse_number needs null-termination) + // HA state max length is 255, so 256 byte buffer covers all cases + char state_buf[256]; + size_t copy_len = msg.state.size(); + if (copy_len >= sizeof(state_buf)) { + copy_len = sizeof(state_buf) - 1; // Truncate to leave space for null terminator + } + if (copy_len > 0) { + memcpy(state_buf, msg.state.c_str(), copy_len); + } + state_buf[copy_len] = '\0'; + it.callback(StringRef(state_buf, copy_len)); } } #endif diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index eedf8c7172..a7b046447d 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -388,8 +388,8 @@ void APIServer::handle_action_response(uint32_t call_id, bool success, StringRef #ifdef USE_API_HOMEASSISTANT_STATES // Helper to add subscription (reduces duplication) -void APIServer::add_state_subscription_(const char *entity_id, const char *attribute, - std::function f, bool once) { +void APIServer::add_state_subscription_(const char *entity_id, const char *attribute, std::function f, + bool once) { this->state_subs_.push_back(HomeAssistantStateSubscription{ .entity_id = entity_id, .attribute = attribute, .callback = std::move(f), .once = once, // entity_id_dynamic_storage and attribute_dynamic_storage remain nullptr (no heap allocation) @@ -398,7 +398,7 @@ void APIServer::add_state_subscription_(const char *entity_id, const char *attri // Helper to add subscription with heap-allocated strings (reduces duplication) void APIServer::add_state_subscription_(std::string entity_id, optional attribute, - std::function f, bool once) { + std::function f, bool once) { HomeAssistantStateSubscription sub; // Allocate heap storage for the strings sub.entity_id_dynamic_storage = std::make_unique(std::move(entity_id)); @@ -418,23 +418,43 @@ void APIServer::add_state_subscription_(std::string entity_id, optional f) { + std::function f) { this->add_state_subscription_(entity_id, attribute, std::move(f), false); } void APIServer::get_home_assistant_state(const char *entity_id, const char *attribute, - std::function f) { + std::function f) { this->add_state_subscription_(entity_id, attribute, std::move(f), true); } -// Existing std::string overload (for custom_api_device.h - heap allocation) +// std::string overload with StringRef callback (zero-allocation callback) void APIServer::subscribe_home_assistant_state(std::string entity_id, optional attribute, - std::function f) { + std::function f) { this->add_state_subscription_(std::move(entity_id), std::move(attribute), std::move(f), false); } void APIServer::get_home_assistant_state(std::string entity_id, optional attribute, - std::function f) { + std::function f) { + this->add_state_subscription_(std::move(entity_id), std::move(attribute), std::move(f), true); +} + +// Legacy helper: wraps std::string callback and delegates to StringRef version +void APIServer::add_state_subscription_(std::string entity_id, optional attribute, + std::function f, bool once) { + // Wrap callback to convert StringRef -> std::string, then delegate + this->add_state_subscription_(std::move(entity_id), std::move(attribute), + std::function([f = std::move(f)](StringRef state) { f(state.str()); }), + once); +} + +// Legacy std::string overload (for custom_api_device.h - converts StringRef to std::string) +void APIServer::subscribe_home_assistant_state(std::string entity_id, optional attribute, + std::function f) { + this->add_state_subscription_(std::move(entity_id), std::move(attribute), std::move(f), false); +} + +void APIServer::get_home_assistant_state(std::string entity_id, optional attribute, + std::function f) { this->add_state_subscription_(std::move(entity_id), std::move(attribute), std::move(f), true); } diff --git a/esphome/components/api/api_server.h b/esphome/components/api/api_server.h index 2b2e8bae73..f5b57f994a 100644 --- a/esphome/components/api/api_server.h +++ b/esphome/components/api/api_server.h @@ -10,6 +10,7 @@ #include "esphome/core/component.h" #include "esphome/core/controller.h" #include "esphome/core/log.h" +#include "esphome/core/string_ref.h" #include "list_entities.h" #include "subscribe_state.h" #ifdef USE_LOGGER @@ -191,7 +192,7 @@ class APIServer : public Component, struct HomeAssistantStateSubscription { const char *entity_id; // Pointer to flash (internal) or heap (external) const char *attribute; // Pointer to flash or nullptr (nullptr means no attribute) - std::function callback; + std::function callback; bool once; // Dynamic storage for external components using std::string API (custom_api_device.h) @@ -201,14 +202,20 @@ class APIServer : public Component, }; // New const char* overload (for internal components - zero allocation) - void subscribe_home_assistant_state(const char *entity_id, const char *attribute, std::function f); - void get_home_assistant_state(const char *entity_id, const char *attribute, std::function f); + void subscribe_home_assistant_state(const char *entity_id, const char *attribute, std::function f); + void get_home_assistant_state(const char *entity_id, const char *attribute, std::function f); - // Existing std::string overload (for custom_api_device.h - heap allocation) + // std::string overload with StringRef callback (for custom_api_device.h with zero-allocation callback) void subscribe_home_assistant_state(std::string entity_id, optional attribute, - std::function f); + std::function f); void get_home_assistant_state(std::string entity_id, optional attribute, - std::function f); + std::function f); + + // Legacy std::string overload (for custom_api_device.h - converts StringRef to std::string for callback) + void subscribe_home_assistant_state(std::string entity_id, optional attribute, + std::function f); + void get_home_assistant_state(std::string entity_id, optional attribute, + std::function f); const std::vector &get_state_subs() const; #endif @@ -232,10 +239,13 @@ class APIServer : public Component, #endif // USE_API_NOISE #ifdef USE_API_HOMEASSISTANT_STATES // Helper methods to reduce code duplication - void add_state_subscription_(const char *entity_id, const char *attribute, std::function f, + void add_state_subscription_(const char *entity_id, const char *attribute, std::function f, bool once); + void add_state_subscription_(std::string entity_id, optional attribute, std::function f, + bool once); + // Legacy helper: wraps std::string callback and delegates to StringRef version void add_state_subscription_(std::string entity_id, optional attribute, - std::function f, bool once); + std::function f, bool once); #endif // USE_API_HOMEASSISTANT_STATES // Pointers and pointer-like types first (4 bytes each) std::unique_ptr socket_ = nullptr; diff --git a/esphome/components/api/custom_api_device.h b/esphome/components/api/custom_api_device.h index 5e9165326d..7f655a2479 100644 --- a/esphome/components/api/custom_api_device.h +++ b/esphome/components/api/custom_api_device.h @@ -122,21 +122,36 @@ class CustomAPIDevice { * subscribe_homeassistant_state(&CustomNativeAPI::on_state_changed, "climate.kitchen", "current_temperature"); * } * - * void on_state_changed(std::string state) { - * // State of sensor.weather_forecast is `state` + * void on_state_changed(StringRef state) { + * // State of climate.kitchen current_temperature is `state` + * // Use state.c_str() for C string, state.str() for std::string * } * ``` * * @tparam T The class type creating the service, automatically deduced from the function pointer. - * @param callback The member function to call when the entity state changes. + * @param callback The member function to call when the entity state changes (zero-allocation). * @param entity_id The entity_id to track. * @param attribute The entity state attribute to track. */ template + void subscribe_homeassistant_state(void (T::*callback)(StringRef), const std::string &entity_id, + const std::string &attribute = "") { + auto f = std::bind(callback, (T *) this, std::placeholders::_1); + global_api_server->subscribe_home_assistant_state(entity_id, optional(attribute), std::move(f)); + } + + /** Subscribe to the state (or attribute state) of an entity from Home Assistant (legacy std::string version). + * + * @deprecated Use the StringRef overload for zero-allocation callbacks. Will be removed in 2027.1.0. + */ + template + ESPDEPRECATED("Use void callback(StringRef) instead. Will be removed in 2027.1.0.", "2026.1.0") void subscribe_homeassistant_state(void (T::*callback)(std::string), const std::string &entity_id, const std::string &attribute = "") { auto f = std::bind(callback, (T *) this, std::placeholders::_1); - global_api_server->subscribe_home_assistant_state(entity_id, optional(attribute), f); + // Explicit type to disambiguate overload resolution + global_api_server->subscribe_home_assistant_state(entity_id, optional(attribute), + std::function(f)); } /** Subscribe to the state (or attribute state) of an entity from Home Assistant. @@ -148,23 +163,45 @@ class CustomAPIDevice { * subscribe_homeassistant_state(&CustomNativeAPI::on_state_changed, "sensor.weather_forecast"); * } * - * void on_state_changed(std::string entity_id, std::string state) { + * void on_state_changed(const std::string &entity_id, StringRef state) { * // State of `entity_id` is `state` * } * ``` * * @tparam T The class type creating the service, automatically deduced from the function pointer. - * @param callback The member function to call when the entity state changes. + * @param callback The member function to call when the entity state changes (zero-allocation for state). * @param entity_id The entity_id to track. * @param attribute The entity state attribute to track. */ template + void subscribe_homeassistant_state(void (T::*callback)(const std::string &, StringRef), const std::string &entity_id, + const std::string &attribute = "") { + auto f = std::bind(callback, (T *) this, entity_id, std::placeholders::_1); + global_api_server->subscribe_home_assistant_state(entity_id, optional(attribute), std::move(f)); + } + + /** Subscribe to the state (or attribute state) of an entity from Home Assistant (legacy std::string version). + * + * @deprecated Use the StringRef overload for zero-allocation callbacks. Will be removed in 2027.1.0. + */ + template + ESPDEPRECATED("Use void callback(const std::string &, StringRef) instead. Will be removed in 2027.1.0.", "2026.1.0") void subscribe_homeassistant_state(void (T::*callback)(std::string, std::string), const std::string &entity_id, const std::string &attribute = "") { auto f = std::bind(callback, (T *) this, entity_id, std::placeholders::_1); - global_api_server->subscribe_home_assistant_state(entity_id, optional(attribute), f); + // Explicit type to disambiguate overload resolution + global_api_server->subscribe_home_assistant_state(entity_id, optional(attribute), + std::function(f)); } #else + template + void subscribe_homeassistant_state(void (T::*callback)(StringRef), const std::string &entity_id, + const std::string &attribute = "") { + static_assert(sizeof(T) == 0, + "subscribe_homeassistant_state() requires 'homeassistant_states: true' in the 'api:' section " + "of your YAML configuration"); + } + template void subscribe_homeassistant_state(void (T::*callback)(std::string), const std::string &entity_id, const std::string &attribute = "") { @@ -173,6 +210,14 @@ class CustomAPIDevice { "of your YAML configuration"); } + template + void subscribe_homeassistant_state(void (T::*callback)(const std::string &, StringRef), const std::string &entity_id, + const std::string &attribute = "") { + static_assert(sizeof(T) == 0, + "subscribe_homeassistant_state() requires 'homeassistant_states: true' in the 'api:' section " + "of your YAML configuration"); + } + template void subscribe_homeassistant_state(void (T::*callback)(std::string, std::string), const std::string &entity_id, const std::string &attribute = "") { diff --git a/esphome/components/homeassistant/binary_sensor/homeassistant_binary_sensor.cpp b/esphome/components/homeassistant/binary_sensor/homeassistant_binary_sensor.cpp index 5652e7d603..b0d9135822 100644 --- a/esphome/components/homeassistant/binary_sensor/homeassistant_binary_sensor.cpp +++ b/esphome/components/homeassistant/binary_sensor/homeassistant_binary_sensor.cpp @@ -1,6 +1,7 @@ #include "homeassistant_binary_sensor.h" -#include "esphome/core/log.h" #include "esphome/components/api/api_server.h" +#include "esphome/core/log.h" +#include "esphome/core/string_ref.h" namespace esphome { namespace homeassistant { @@ -8,31 +9,30 @@ namespace homeassistant { static const char *const TAG = "homeassistant.binary_sensor"; void HomeassistantBinarySensor::setup() { - api::global_api_server->subscribe_home_assistant_state( - this->entity_id_, this->attribute_, [this](const std::string &state) { - auto val = parse_on_off(state.c_str()); - switch (val) { - case PARSE_NONE: - case PARSE_TOGGLE: - ESP_LOGW(TAG, "Can't convert '%s' to binary state!", state.c_str()); - break; - case PARSE_ON: - case PARSE_OFF: - bool new_state = val == PARSE_ON; - if (this->attribute_ != nullptr) { - ESP_LOGD(TAG, "'%s::%s': Got attribute state %s", this->entity_id_, this->attribute_, ONOFF(new_state)); - } else { - ESP_LOGD(TAG, "'%s': Got state %s", this->entity_id_, ONOFF(new_state)); - } - if (this->initial_) { - this->publish_initial_state(new_state); - } else { - this->publish_state(new_state); - } - break; + api::global_api_server->subscribe_home_assistant_state(this->entity_id_, this->attribute_, [this](StringRef state) { + auto val = parse_on_off(state.c_str()); + switch (val) { + case PARSE_NONE: + case PARSE_TOGGLE: + ESP_LOGW(TAG, "Can't convert '%s' to binary state!", state.c_str()); + break; + case PARSE_ON: + case PARSE_OFF: + bool new_state = val == PARSE_ON; + if (this->attribute_ != nullptr) { + ESP_LOGD(TAG, "'%s::%s': Got attribute state %s", this->entity_id_, this->attribute_, ONOFF(new_state)); + } else { + ESP_LOGD(TAG, "'%s': Got state %s", this->entity_id_, ONOFF(new_state)); } - this->initial_ = false; - }); + if (this->initial_) { + this->publish_initial_state(new_state); + } else { + this->publish_state(new_state); + } + break; + } + this->initial_ = false; + }); } void HomeassistantBinarySensor::dump_config() { LOG_BINARY_SENSOR("", "Homeassistant Binary Sensor", this); diff --git a/esphome/components/homeassistant/number/homeassistant_number.cpp b/esphome/components/homeassistant/number/homeassistant_number.cpp index 1ca90180eb..8c0d415c23 100644 --- a/esphome/components/homeassistant/number/homeassistant_number.cpp +++ b/esphome/components/homeassistant/number/homeassistant_number.cpp @@ -3,14 +3,15 @@ #include "esphome/components/api/api_pb2.h" #include "esphome/components/api/api_server.h" #include "esphome/core/log.h" +#include "esphome/core/string_ref.h" namespace esphome { namespace homeassistant { static const char *const TAG = "homeassistant.number"; -void HomeassistantNumber::state_changed_(const std::string &state) { - auto number_value = parse_number(state); +void HomeassistantNumber::state_changed_(StringRef state) { + auto number_value = parse_number(state.c_str()); if (!number_value.has_value()) { ESP_LOGW(TAG, "'%s': Can't convert '%s' to number!", this->entity_id_, state.c_str()); this->publish_state(NAN); @@ -23,8 +24,8 @@ void HomeassistantNumber::state_changed_(const std::string &state) { this->publish_state(number_value.value()); } -void HomeassistantNumber::min_retrieved_(const std::string &min) { - auto min_value = parse_number(min); +void HomeassistantNumber::min_retrieved_(StringRef min) { + auto min_value = parse_number(min.c_str()); if (!min_value.has_value()) { ESP_LOGE(TAG, "'%s': Can't convert 'min' value '%s' to number!", this->entity_id_, min.c_str()); return; @@ -33,8 +34,8 @@ void HomeassistantNumber::min_retrieved_(const std::string &min) { this->traits.set_min_value(min_value.value()); } -void HomeassistantNumber::max_retrieved_(const std::string &max) { - auto max_value = parse_number(max); +void HomeassistantNumber::max_retrieved_(StringRef max) { + auto max_value = parse_number(max.c_str()); if (!max_value.has_value()) { ESP_LOGE(TAG, "'%s': Can't convert 'max' value '%s' to number!", this->entity_id_, max.c_str()); return; @@ -43,8 +44,8 @@ void HomeassistantNumber::max_retrieved_(const std::string &max) { this->traits.set_max_value(max_value.value()); } -void HomeassistantNumber::step_retrieved_(const std::string &step) { - auto step_value = parse_number(step); +void HomeassistantNumber::step_retrieved_(StringRef step) { + auto step_value = parse_number(step.c_str()); if (!step_value.has_value()) { ESP_LOGE(TAG, "'%s': Can't convert 'step' value '%s' to number!", this->entity_id_, step.c_str()); return; diff --git a/esphome/components/homeassistant/number/homeassistant_number.h b/esphome/components/homeassistant/number/homeassistant_number.h index 0dffc108cb..275d2d5f03 100644 --- a/esphome/components/homeassistant/number/homeassistant_number.h +++ b/esphome/components/homeassistant/number/homeassistant_number.h @@ -1,10 +1,8 @@ #pragma once -#include -#include - #include "esphome/components/number/number.h" #include "esphome/core/component.h" +#include "esphome/core/string_ref.h" namespace esphome { namespace homeassistant { @@ -18,10 +16,10 @@ class HomeassistantNumber : public number::Number, public Component { float get_setup_priority() const override; protected: - void state_changed_(const std::string &state); - void min_retrieved_(const std::string &min); - void max_retrieved_(const std::string &max); - void step_retrieved_(const std::string &step); + void state_changed_(StringRef state); + void min_retrieved_(StringRef min); + void max_retrieved_(StringRef max); + void step_retrieved_(StringRef step); void control(float value) override; diff --git a/esphome/components/homeassistant/sensor/homeassistant_sensor.cpp b/esphome/components/homeassistant/sensor/homeassistant_sensor.cpp index 78da47f9a1..66300ebba5 100644 --- a/esphome/components/homeassistant/sensor/homeassistant_sensor.cpp +++ b/esphome/components/homeassistant/sensor/homeassistant_sensor.cpp @@ -1,6 +1,7 @@ #include "homeassistant_sensor.h" -#include "esphome/core/log.h" #include "esphome/components/api/api_server.h" +#include "esphome/core/log.h" +#include "esphome/core/string_ref.h" namespace esphome { namespace homeassistant { @@ -8,22 +9,21 @@ namespace homeassistant { static const char *const TAG = "homeassistant.sensor"; void HomeassistantSensor::setup() { - api::global_api_server->subscribe_home_assistant_state( - this->entity_id_, this->attribute_, [this](const std::string &state) { - auto val = parse_number(state); - if (!val.has_value()) { - ESP_LOGW(TAG, "'%s': Can't convert '%s' to number!", this->entity_id_, state.c_str()); - this->publish_state(NAN); - return; - } + api::global_api_server->subscribe_home_assistant_state(this->entity_id_, this->attribute_, [this](StringRef state) { + auto val = parse_number(state.c_str()); + if (!val.has_value()) { + ESP_LOGW(TAG, "'%s': Can't convert '%s' to number!", this->entity_id_, state.c_str()); + this->publish_state(NAN); + return; + } - if (this->attribute_ != nullptr) { - ESP_LOGD(TAG, "'%s::%s': Got attribute state %.2f", this->entity_id_, this->attribute_, *val); - } else { - ESP_LOGD(TAG, "'%s': Got state %.2f", this->entity_id_, *val); - } - this->publish_state(*val); - }); + if (this->attribute_ != nullptr) { + ESP_LOGD(TAG, "'%s::%s': Got attribute state %.2f", this->entity_id_, this->attribute_, *val); + } else { + ESP_LOGD(TAG, "'%s': Got state %.2f", this->entity_id_, *val); + } + this->publish_state(*val); + }); } void HomeassistantSensor::dump_config() { LOG_SENSOR("", "Homeassistant Sensor", this); diff --git a/esphome/components/homeassistant/switch/homeassistant_switch.cpp b/esphome/components/homeassistant/switch/homeassistant_switch.cpp index c4abf2295d..d08d761442 100644 --- a/esphome/components/homeassistant/switch/homeassistant_switch.cpp +++ b/esphome/components/homeassistant/switch/homeassistant_switch.cpp @@ -1,6 +1,7 @@ #include "homeassistant_switch.h" #include "esphome/components/api/api_server.h" #include "esphome/core/log.h" +#include "esphome/core/string_ref.h" namespace esphome { namespace homeassistant { @@ -10,7 +11,7 @@ static const char *const TAG = "homeassistant.switch"; using namespace esphome::switch_; void HomeassistantSwitch::setup() { - api::global_api_server->subscribe_home_assistant_state(this->entity_id_, nullptr, [this](const std::string &state) { + api::global_api_server->subscribe_home_assistant_state(this->entity_id_, nullptr, [this](StringRef state) { auto val = parse_on_off(state.c_str()); switch (val) { case PARSE_NONE: diff --git a/esphome/components/homeassistant/text_sensor/homeassistant_text_sensor.cpp b/esphome/components/homeassistant/text_sensor/homeassistant_text_sensor.cpp index 6154330a4e..6f77349535 100644 --- a/esphome/components/homeassistant/text_sensor/homeassistant_text_sensor.cpp +++ b/esphome/components/homeassistant/text_sensor/homeassistant_text_sensor.cpp @@ -1,6 +1,7 @@ #include "homeassistant_text_sensor.h" -#include "esphome/core/log.h" #include "esphome/components/api/api_server.h" +#include "esphome/core/log.h" +#include "esphome/core/string_ref.h" namespace esphome { namespace homeassistant { @@ -8,15 +9,14 @@ namespace homeassistant { static const char *const TAG = "homeassistant.text_sensor"; void HomeassistantTextSensor::setup() { - api::global_api_server->subscribe_home_assistant_state( - this->entity_id_, this->attribute_, [this](const std::string &state) { - if (this->attribute_ != nullptr) { - ESP_LOGD(TAG, "'%s::%s': Got attribute state '%s'", this->entity_id_, this->attribute_, state.c_str()); - } else { - ESP_LOGD(TAG, "'%s': Got state '%s'", this->entity_id_, state.c_str()); - } - this->publish_state(state); - }); + api::global_api_server->subscribe_home_assistant_state(this->entity_id_, this->attribute_, [this](StringRef state) { + if (this->attribute_ != nullptr) { + ESP_LOGD(TAG, "'%s::%s': Got attribute state '%s'", this->entity_id_, this->attribute_, state.c_str()); + } else { + ESP_LOGD(TAG, "'%s': Got state '%s'", this->entity_id_, state.c_str()); + } + this->publish_state(state.str()); + }); } void HomeassistantTextSensor::dump_config() { LOG_TEXT_SENSOR("", "Homeassistant Text Sensor", this);