From 48b5cae6c4692c6f563b1c250aa266e31b0b1b98 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Mar 2026 11:32:44 -1000 Subject: [PATCH 1/2] [api] Use StringRef for user service string arguments (#13974) --- esphome/automation.py | 57 +++++- esphome/components/api/__init__.py | 15 +- esphome/components/api/api_connection.cpp | 43 +++-- esphome/components/api/api_frame_helper.h | 4 + .../components/api/api_frame_helper_noise.cpp | 30 ++- .../api/api_frame_helper_plaintext.cpp | 7 +- esphome/components/api/user_services.cpp | 5 + esphome/components/button/__init__.py | 4 +- esphome/components/cover/__init__.py | 20 +- esphome/components/fan/__init__.py | 9 +- esphome/components/globals/__init__.py | 1 + esphome/components/light/automation.py | 12 +- esphome/components/logger/__init__.py | 4 +- esphome/components/mqtt/__init__.py | 7 +- esphome/components/number/__init__.py | 6 + esphome/components/output/__init__.py | 7 +- esphome/components/script/__init__.py | 2 + esphome/components/select/__init__.py | 7 + esphome/components/switch/__init__.py | 14 +- esphome/components/text/__init__.py | 1 + esphome/core/string_ref.h | 14 ++ esphome/util.py | 16 +- tests/unit_tests/test_automation.py | 177 ++++++++++++++++++ 23 files changed, 406 insertions(+), 56 deletions(-) create mode 100644 tests/unit_tests/test_automation.py diff --git a/esphome/automation.py b/esphome/automation.py index 2439b1ddc4..d9b8b2ec57 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -57,8 +57,23 @@ def maybe_conf(conf, *validators): return validate -def register_action(name: str, action_type: MockObjClass, schema: cv.Schema): - return ACTION_REGISTRY.register(name, action_type, schema) +def register_action( + name: str, + action_type: MockObjClass, + schema: cv.Schema, + *, + synchronous: bool = False, +): + """Register an action type. + + Actions default to ``synchronous=False`` (safe default), meaning string + arguments use owning std::string to prevent dangling references. + + Set ``synchronous=True`` only for actions that complete synchronously + and never store trigger arguments for later execution. This allows + the code generator to use non-owning StringRef for zero-copy access. + """ + return ACTION_REGISTRY.register(name, action_type, schema, synchronous=synchronous) def register_condition(name: str, condition_type: MockObjClass, schema: cv.Schema): @@ -335,7 +350,9 @@ async def component_is_idle_condition_to_code( @register_action( - "delay", DelayAction, cv.templatable(cv.positive_time_period_milliseconds) + "delay", + DelayAction, + cv.templatable(cv.positive_time_period_milliseconds), ) async def delay_action_to_code( config: ConfigType, @@ -366,6 +383,7 @@ async def delay_action_to_code( cv.has_at_least_one_key(CONF_THEN, CONF_ELSE), cv.has_at_least_one_key(CONF_CONDITION, CONF_ANY, CONF_ALL), ), + synchronous=True, ) async def if_action_to_code( config: ConfigType, @@ -394,6 +412,7 @@ async def if_action_to_code( cv.Required(CONF_THEN): validate_action_list, } ), + synchronous=True, ) async def while_action_to_code( config: ConfigType, @@ -417,6 +436,7 @@ async def while_action_to_code( cv.Required(CONF_THEN): validate_action_list, } ), + synchronous=True, ) async def repeat_action_to_code( config: ConfigType, @@ -461,7 +481,12 @@ async def wait_until_action_to_code( return var -@register_action("lambda", LambdaAction, cv.lambda_) +# Lambda executes user C++ inline and returns — synchronous by execution model. +# User code could theoretically store the StringRef for deferred use, but StringRef +# is a view type and storing views beyond their scope is always unsafe regardless +# of this optimization. Marking non-synchronous would disable StringRef for nearly +# all user services since most use lambda. +@register_action("lambda", LambdaAction, cv.lambda_, synchronous=True) async def lambda_action_to_code( config: ConfigType, action_id: ID, @@ -480,6 +505,7 @@ async def lambda_action_to_code( cv.Required(CONF_ID): cv.use_id(cg.PollingComponent), } ), + synchronous=True, ) async def component_update_action_to_code( config: ConfigType, @@ -499,6 +525,7 @@ async def component_update_action_to_code( cv.Required(CONF_ID): cv.use_id(cg.PollingComponent), } ), + synchronous=True, ) async def component_suspend_action_to_code( config: ConfigType, @@ -521,6 +548,7 @@ async def component_suspend_action_to_code( ), } ), + synchronous=True, ) async def component_resume_action_to_code( config: ConfigType, @@ -578,6 +606,27 @@ async def build_condition_list( return conditions +def has_non_synchronous_actions(actions: ConfigType) -> bool: + """Check if a validated action list contains any non-synchronous actions. + + Non-synchronous actions (delay, wait_until, script.wait, etc.) store + trigger args for later execution, making non-owning types like StringRef + unsafe. Actions that haven't been audited default to non-synchronous. + """ + if isinstance(actions, list): + return any(has_non_synchronous_actions(item) for item in actions) + if isinstance(actions, dict): + for key in actions: + if key in ACTION_REGISTRY and not ACTION_REGISTRY[key].synchronous: + return True + return any( + has_non_synchronous_actions(v) + for v in actions.values() + if isinstance(v, (list, dict)) + ) + return False + + async def build_automation( trigger: MockObj, args: TemplateArgsType, config: ConfigType ) -> MockObj: diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 3f7cafb485..d7b6bec357 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -76,7 +76,7 @@ SERVICE_ARG_NATIVE_TYPES: dict[str, MockObj] = { "bool": cg.bool_, "int": cg.int32, "float": cg.float_, - "string": cg.std_string, + "string": cg.StringRef, "bool[]": cg.FixedVector.template(cg.bool_).operator("const").operator("ref"), "int[]": cg.FixedVector.template(cg.int32).operator("const").operator("ref"), "float[]": cg.FixedVector.template(cg.float_).operator("const").operator("ref"), @@ -380,9 +380,18 @@ async def to_code(config: ConfigType) -> None: if is_optional: func_args.append((cg.bool_, "return_response")) + # Check if action chain has non-synchronous actions that would make + # non-owning StringRef dangle (rx_buf_ reused after delay) + has_non_synchronous = automation.has_non_synchronous_actions( + conf.get(CONF_THEN, []) + ) + service_arg_names: list[str] = [] for name, var_ in conf[CONF_VARIABLES].items(): native = SERVICE_ARG_NATIVE_TYPES[var_] + # Fall back to std::string for string args if non-synchronous actions exist + if has_non_synchronous and native is cg.StringRef: + native = cg.std_string service_template_args.append(native) func_args.append((native, name)) service_arg_names.append(name) @@ -509,11 +518,13 @@ HOMEASSISTANT_ACTION_ACTION_SCHEMA = cv.All( "homeassistant.action", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, + synchronous=True, ) @automation.register_action( "homeassistant.service", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, + synchronous=True, ) async def homeassistant_service_to_code( config: ConfigType, @@ -604,6 +615,7 @@ HOMEASSISTANT_EVENT_ACTION_SCHEMA = cv.Schema( "homeassistant.event", HomeAssistantServiceCallAction, HOMEASSISTANT_EVENT_ACTION_SCHEMA, + synchronous=True, ) async def homeassistant_event_to_code(config, action_id, template_arg, args): cg.add_define("USE_API_HOMEASSISTANT_SERVICES") @@ -644,6 +656,7 @@ HOMEASSISTANT_TAG_SCANNED_ACTION_SCHEMA = cv.maybe_simple_value( "homeassistant.tag_scanned", HomeAssistantServiceCallAction, HOMEASSISTANT_TAG_SCANNED_ACTION_SCHEMA, + synchronous=True, ) async def homeassistant_tag_scanned_to_code(config, action_id, template_arg, args): cg.add_define("USE_API_HOMEASSISTANT_SERVICES") diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 8b2efdde51..215af611db 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1711,37 +1711,42 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes return; } + // Null-terminate state in-place for safe c_str() usage (e.g., parse_number in callbacks). + // Safe: decode is complete, byte after string data was already consumed during parse, + // and frame helpers reserve RX_BUF_NULL_TERMINATOR extra byte in rx_buf_. + // const_cast is safe: msg references mutable rx_buf_ data; the const& handler + // signature is a generated protobuf pattern, not a true immutability contract. + if (!msg.state.empty()) { + const_cast(msg.state.c_str())[msg.state.size()] = '\0'; + } + for (auto &it : this->parent_->get_state_subs()) { - // Compare entity_id: check length matches and content matches - size_t entity_id_len = strlen(it.entity_id); - if (entity_id_len != msg.entity_id.size() || - memcmp(it.entity_id, msg.entity_id.c_str(), msg.entity_id.size()) != 0) { + if (msg.entity_id != it.entity_id) { continue; } - // Compare attribute: either both have matching attribute, or both have none - size_t sub_attr_len = it.attribute != nullptr ? strlen(it.attribute) : 0; - if (sub_attr_len != msg.attribute.size() || - (sub_attr_len > 0 && memcmp(it.attribute, msg.attribute.c_str(), sub_attr_len) != 0)) { + // If subscriber has attribute filter (non-null), message attribute must match it; + // if subscriber has no filter (nullptr), message must have no attribute. + if (it.attribute != nullptr ? msg.attribute != it.attribute : !msg.attribute.empty()) { continue; } - // Create null-terminated state for callback (parse_number needs null-termination) - // HA state max length is 255 characters, but attributes can be much longer - // Use stack buffer for common case (states), heap fallback for large attributes - size_t state_len = msg.state.size(); - SmallBufferWithHeapFallback state_buf_alloc(state_len + 1); - char *state_buf = reinterpret_cast(state_buf_alloc.get()); - if (state_len > 0) { - memcpy(state_buf, msg.state.c_str(), state_len); - } - state_buf[state_len] = '\0'; - it.callback(StringRef(state_buf, state_len)); + it.callback(msg.state); } } #endif #ifdef USE_API_USER_DEFINED_ACTIONS void APIConnection::on_execute_service_request(const ExecuteServiceRequest &msg) { + // Null-terminate string args in-place for safe c_str() usage in YAML service triggers. + // Safe: full ExecuteServiceRequest decode is complete, all bytes in rx_buf_ consumed, + // and frame helpers reserve RX_BUF_NULL_TERMINATOR extra byte for the last field. + // const_cast is safe: msg references mutable rx_buf_ data; the const& handler + // signature is a generated protobuf pattern, not a true immutability contract. + for (auto &arg : const_cast(msg).args) { + if (!arg.string_.empty()) { + const_cast(arg.string_.c_str())[arg.string_.size()] = '\0'; + } + } bool found = false; #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES // Register the call and get a unique server-generated action_call_id diff --git a/esphome/components/api/api_frame_helper.h b/esphome/components/api/api_frame_helper.h index 03f3814bb9..2b4e9ea3cd 100644 --- a/esphome/components/api/api_frame_helper.h +++ b/esphome/components/api/api_frame_helper.h @@ -29,6 +29,10 @@ static constexpr uint16_t MAX_MESSAGE_SIZE = 8192; // 8 KiB for ESP8266 static constexpr uint16_t MAX_MESSAGE_SIZE = 32768; // 32 KiB for ESP32 and other platforms #endif +// Extra byte reserved in rx_buf_ beyond the message size so protobuf +// StringRef fields can be null-terminated in-place after decode. +static constexpr uint16_t RX_BUF_NULL_TERMINATOR = 1; + // Maximum number of messages to batch in a single write operation // Must be >= MAX_INITIAL_PER_BATCH in api_connection.h (enforced by static_assert there) static constexpr size_t MAX_MESSAGES_PER_BATCH = 34; diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 2aad732f7f..3ae35e9be8 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -194,16 +194,21 @@ APIError APINoiseFrameHelper::try_read_frame_() { uint16_t msg_size = (((uint16_t) rx_header_buf_[1]) << 8) | rx_header_buf_[2]; // Check against size limits to prevent OOM: MAX_HANDSHAKE_SIZE for handshake, MAX_MESSAGE_SIZE for data - uint16_t limit = (state_ == State::DATA) ? MAX_MESSAGE_SIZE : MAX_HANDSHAKE_SIZE; + bool is_data = (state_ == State::DATA); + uint16_t limit = is_data ? MAX_MESSAGE_SIZE : MAX_HANDSHAKE_SIZE; if (msg_size > limit) { state_ = State::FAILED; HELPER_LOG("Bad packet: message size %u exceeds maximum %u", msg_size, limit); - return (state_ == State::DATA) ? APIError::BAD_DATA_PACKET : APIError::BAD_HANDSHAKE_PACKET_LEN; + return is_data ? APIError::BAD_DATA_PACKET : APIError::BAD_HANDSHAKE_PACKET_LEN; } - // Reserve space for body - if (this->rx_buf_.size() != msg_size) { - this->rx_buf_.resize(msg_size); + // Reserve space for body (+ null terminator in DATA state so protobuf + // StringRef fields can be safely null-terminated in-place after decode. + // During handshake, rx_buf_.size() is used in prologue construction, so + // the buffer must be exactly msg_size to avoid prologue mismatch.) + uint16_t alloc_size = msg_size + (is_data ? RX_BUF_NULL_TERMINATOR : 0); + if (this->rx_buf_.size() != alloc_size) { + this->rx_buf_.resize(alloc_size); } if (rx_buf_len_ < msg_size) { @@ -407,7 +412,18 @@ APIError APINoiseFrameHelper::read_packet(ReadPacketBuffer *buffer) { NoiseBuffer mbuf; noise_buffer_init(mbuf); - noise_buffer_set_inout(mbuf, this->rx_buf_.data(), this->rx_buf_.size(), this->rx_buf_.size()); + // read_packet() must only be called in DATA state; the extra + // RX_BUF_NULL_TERMINATOR byte is only allocated in DATA state + // (see try_read_frame_), so calling this during handshake would + // underflow the size calculation below. +#ifdef ESPHOME_DEBUG_API + assert(this->state_ == State::DATA); +#endif + // rx_buf_ has RX_BUF_NULL_TERMINATOR extra byte for null termination + // (only added in DATA state — see try_read_frame_), so subtract it + // to get the actual encrypted data size for decryption. + size_t encrypted_size = this->rx_buf_.size() - RX_BUF_NULL_TERMINATOR; + noise_buffer_set_inout(mbuf, this->rx_buf_.data(), encrypted_size, encrypted_size); int err = noise_cipherstate_decrypt(this->recv_cipher_, &mbuf); APIError decrypt_err = handle_noise_error_(err, LOG_STR("noise_cipherstate_decrypt"), APIError::CIPHERSTATE_DECRYPT_FAILED); @@ -574,7 +590,9 @@ APIError APINoiseFrameHelper::init_handshake_() { } APIError APINoiseFrameHelper::check_handshake_finished_() { +#ifdef ESPHOME_DEBUG_API assert(state_ == State::HANDSHAKE); +#endif int action = noise_handshakestate_get_action(handshake_); if (action == NOISE_ACTION_READ_MESSAGE || action == NOISE_ACTION_WRITE_MESSAGE) diff --git a/esphome/components/api/api_frame_helper_plaintext.cpp b/esphome/components/api/api_frame_helper_plaintext.cpp index 5069dbf68b..e2bb56e0ac 100644 --- a/esphome/components/api/api_frame_helper_plaintext.cpp +++ b/esphome/components/api/api_frame_helper_plaintext.cpp @@ -163,9 +163,10 @@ APIError APIPlaintextFrameHelper::try_read_frame_() { } // header reading done - // Reserve space for body - if (this->rx_buf_.size() != this->rx_header_parsed_len_) { - this->rx_buf_.resize(this->rx_header_parsed_len_); + // Reserve space for body (+ null terminator so protobuf StringRef fields + // can be safely null-terminated in-place after decode) + if (this->rx_buf_.size() != this->rx_header_parsed_len_ + RX_BUF_NULL_TERMINATOR) { + this->rx_buf_.resize(this->rx_header_parsed_len_ + RX_BUF_NULL_TERMINATOR); } if (rx_buf_len_ < rx_header_parsed_len_) { diff --git a/esphome/components/api/user_services.cpp b/esphome/components/api/user_services.cpp index 9c2b4aa79a..28a43c656c 100644 --- a/esphome/components/api/user_services.cpp +++ b/esphome/components/api/user_services.cpp @@ -1,5 +1,6 @@ #include "user_services.h" #include "esphome/core/log.h" +#include "esphome/core/string_ref.h" namespace esphome::api { @@ -11,6 +12,8 @@ template<> int32_t get_execute_arg_value(const ExecuteServiceArgument & } template<> float get_execute_arg_value(const ExecuteServiceArgument &arg) { return arg.float_; } template<> std::string get_execute_arg_value(const ExecuteServiceArgument &arg) { return arg.string_; } +// Zero-copy StringRef version for YAML-generated services (string_ is null-terminated after decode) +template<> StringRef get_execute_arg_value(const ExecuteServiceArgument &arg) { return arg.string_; } // Legacy std::vector versions for external components using custom_api_device.h - optimized with reserve template<> std::vector get_execute_arg_value>(const ExecuteServiceArgument &arg) { @@ -61,6 +64,8 @@ template<> enums::ServiceArgType to_service_arg_type() { return enums::SER template<> enums::ServiceArgType to_service_arg_type() { return enums::SERVICE_ARG_TYPE_INT; } template<> enums::ServiceArgType to_service_arg_type() { return enums::SERVICE_ARG_TYPE_FLOAT; } template<> enums::ServiceArgType to_service_arg_type() { return enums::SERVICE_ARG_TYPE_STRING; } +// Zero-copy StringRef version for YAML-generated services +template<> enums::ServiceArgType to_service_arg_type() { return enums::SERVICE_ARG_TYPE_STRING; } // Legacy std::vector versions for external components using custom_api_device.h template<> enums::ServiceArgType to_service_arg_type>() { return enums::SERVICE_ARG_TYPE_BOOL_ARRAY; } diff --git a/esphome/components/button/__init__.py b/esphome/components/button/__init__.py index d2f143b97e..94816a0974 100644 --- a/esphome/components/button/__init__.py +++ b/esphome/components/button/__init__.py @@ -123,7 +123,9 @@ BUTTON_PRESS_SCHEMA = maybe_simple_id( ) -@automation.register_action("button.press", PressAction, BUTTON_PRESS_SCHEMA) +@automation.register_action( + "button.press", PressAction, BUTTON_PRESS_SCHEMA, synchronous=True +) async def button_press_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) diff --git a/esphome/components/cover/__init__.py b/esphome/components/cover/__init__.py index 648fe7decf..17095f41f6 100644 --- a/esphome/components/cover/__init__.py +++ b/esphome/components/cover/__init__.py @@ -248,25 +248,33 @@ COVER_ACTION_SCHEMA = maybe_simple_id( ) -@automation.register_action("cover.open", OpenAction, COVER_ACTION_SCHEMA) +@automation.register_action( + "cover.open", OpenAction, COVER_ACTION_SCHEMA, synchronous=True +) async def cover_open_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) -@automation.register_action("cover.close", CloseAction, COVER_ACTION_SCHEMA) +@automation.register_action( + "cover.close", CloseAction, COVER_ACTION_SCHEMA, synchronous=True +) async def cover_close_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) -@automation.register_action("cover.stop", StopAction, COVER_ACTION_SCHEMA) +@automation.register_action( + "cover.stop", StopAction, COVER_ACTION_SCHEMA, synchronous=True +) async def cover_stop_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) -@automation.register_action("cover.toggle", ToggleAction, COVER_ACTION_SCHEMA) +@automation.register_action( + "cover.toggle", ToggleAction, COVER_ACTION_SCHEMA, synchronous=True +) async def cover_toggle_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) @@ -283,7 +291,9 @@ COVER_CONTROL_ACTION_SCHEMA = cv.Schema( ) -@automation.register_action("cover.control", ControlAction, COVER_CONTROL_ACTION_SCHEMA) +@automation.register_action( + "cover.control", ControlAction, COVER_CONTROL_ACTION_SCHEMA, synchronous=True +) async def cover_control_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) var = cg.new_Pvariable(action_id, template_arg, paren) diff --git a/esphome/components/fan/__init__.py b/esphome/components/fan/__init__.py index 6010aa8ed4..e839df6aee 100644 --- a/esphome/components/fan/__init__.py +++ b/esphome/components/fan/__init__.py @@ -311,13 +311,17 @@ FAN_ACTION_SCHEMA = maybe_simple_id( ) -@automation.register_action("fan.toggle", ToggleAction, FAN_ACTION_SCHEMA) +@automation.register_action( + "fan.toggle", ToggleAction, FAN_ACTION_SCHEMA, synchronous=True +) async def fan_toggle_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) -@automation.register_action("fan.turn_off", TurnOffAction, FAN_ACTION_SCHEMA) +@automation.register_action( + "fan.turn_off", TurnOffAction, FAN_ACTION_SCHEMA, synchronous=True +) async def fan_turn_off_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) @@ -336,6 +340,7 @@ async def fan_turn_off_to_code(config, action_id, template_arg, args): ), } ), + synchronous=True, ) async def fan_turn_on_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/components/globals/__init__.py b/esphome/components/globals/__init__.py index fc400c5dd1..fe11a93a4b 100644 --- a/esphome/components/globals/__init__.py +++ b/esphome/components/globals/__init__.py @@ -102,6 +102,7 @@ async def to_code(config): cv.Required(CONF_VALUE): cv.templatable(cv.string_strict), } ), + synchronous=True, ) async def globals_set_to_code(config, action_id, template_arg, args): full_id, paren = await cg.get_variable_with_full_id(config[CONF_ID]) diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index e5aa8fa0e9..89b2fc0fb2 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -51,6 +51,7 @@ from .types import ( ), } ), + synchronous=True, ) async def light_toggle_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -111,13 +112,13 @@ LIGHT_TURN_ON_ACTION_SCHEMA = automation.maybe_simple_id( @automation.register_action( - "light.turn_off", LightControlAction, LIGHT_TURN_OFF_ACTION_SCHEMA + "light.turn_off", LightControlAction, LIGHT_TURN_OFF_ACTION_SCHEMA, synchronous=True ) @automation.register_action( - "light.turn_on", LightControlAction, LIGHT_TURN_ON_ACTION_SCHEMA + "light.turn_on", LightControlAction, LIGHT_TURN_ON_ACTION_SCHEMA, synchronous=True ) @automation.register_action( - "light.control", LightControlAction, LIGHT_CONTROL_ACTION_SCHEMA + "light.control", LightControlAction, LIGHT_CONTROL_ACTION_SCHEMA, synchronous=True ) async def light_control_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -193,7 +194,10 @@ LIGHT_DIM_RELATIVE_ACTION_SCHEMA = cv.Schema( @automation.register_action( - "light.dim_relative", DimRelativeAction, LIGHT_DIM_RELATIVE_ACTION_SCHEMA + "light.dim_relative", + DimRelativeAction, + LIGHT_DIM_RELATIVE_ACTION_SCHEMA, + synchronous=True, ) async def light_dim_relative_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/components/logger/__init__.py b/esphome/components/logger/__init__.py index 264197c175..026b8aaf24 100644 --- a/esphome/components/logger/__init__.py +++ b/esphome/components/logger/__init__.py @@ -519,7 +519,9 @@ LOGGER_LOG_ACTION_SCHEMA = cv.All( ) -@automation.register_action(CONF_LOGGER_LOG, LambdaAction, LOGGER_LOG_ACTION_SCHEMA) +@automation.register_action( + CONF_LOGGER_LOG, LambdaAction, LOGGER_LOG_ACTION_SCHEMA, synchronous=True +) async def logger_log_action_to_code(config, action_id, template_arg, args): esp_log = LOG_LEVEL_TO_ESP_LOG[config[CONF_LEVEL]] args_ = [cg.RawExpression(str(x)) for x in config[CONF_ARGS]] diff --git a/esphome/components/mqtt/__init__.py b/esphome/components/mqtt/__init__.py index 44e8836487..c25c472038 100644 --- a/esphome/components/mqtt/__init__.py +++ b/esphome/components/mqtt/__init__.py @@ -492,7 +492,7 @@ MQTT_PUBLISH_ACTION_SCHEMA = cv.Schema( @automation.register_action( - "mqtt.publish", MQTTPublishAction, MQTT_PUBLISH_ACTION_SCHEMA + "mqtt.publish", MQTTPublishAction, MQTT_PUBLISH_ACTION_SCHEMA, synchronous=True ) async def mqtt_publish_action_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -521,7 +521,10 @@ MQTT_PUBLISH_JSON_ACTION_SCHEMA = cv.Schema( @automation.register_action( - "mqtt.publish_json", MQTTPublishJsonAction, MQTT_PUBLISH_JSON_ACTION_SCHEMA + "mqtt.publish_json", + MQTTPublishJsonAction, + MQTT_PUBLISH_JSON_ACTION_SCHEMA, + synchronous=True, ) async def mqtt_publish_json_action_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/components/number/__init__.py b/esphome/components/number/__init__.py index d12ec7463b..2238f2c037 100644 --- a/esphome/components/number/__init__.py +++ b/esphome/components/number/__init__.py @@ -352,6 +352,7 @@ OPERATION_BASE_SCHEMA = cv.Schema( cv.Required(CONF_VALUE): cv.templatable(cv.float_), } ), + synchronous=True, ) async def number_set_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -374,6 +375,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + synchronous=True, ) @automation.register_action( "number.decrement", @@ -388,6 +390,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + synchronous=True, ) @automation.register_action( "number.to_min", @@ -401,6 +404,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + synchronous=True, ) @automation.register_action( "number.to_max", @@ -414,6 +418,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + synchronous=True, ) @automation.register_action( "number.operation", @@ -426,6 +431,7 @@ async def number_set_to_code(config, action_id, template_arg, args): cv.Optional(CONF_CYCLE, default=True): cv.templatable(cv.boolean), } ), + synchronous=True, ) async def number_to_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/components/output/__init__.py b/esphome/components/output/__init__.py index bde106b085..a4c960927b 100644 --- a/esphome/components/output/__init__.py +++ b/esphome/components/output/__init__.py @@ -74,14 +74,16 @@ BINARY_OUTPUT_ACTION_SCHEMA = maybe_simple_id( ) -@automation.register_action("output.turn_on", TurnOnAction, BINARY_OUTPUT_ACTION_SCHEMA) +@automation.register_action( + "output.turn_on", TurnOnAction, BINARY_OUTPUT_ACTION_SCHEMA, synchronous=True +) async def output_turn_on_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) @automation.register_action( - "output.turn_off", TurnOffAction, BINARY_OUTPUT_ACTION_SCHEMA + "output.turn_off", TurnOffAction, BINARY_OUTPUT_ACTION_SCHEMA, synchronous=True ) async def output_turn_off_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -97,6 +99,7 @@ async def output_turn_off_to_code(config, action_id, template_arg, args): cv.Required(CONF_LEVEL): cv.templatable(cv.percentage), } ), + synchronous=True, ) async def output_set_level_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/components/script/__init__.py b/esphome/components/script/__init__.py index 8d69981db0..369cefad91 100644 --- a/esphome/components/script/__init__.py +++ b/esphome/components/script/__init__.py @@ -160,6 +160,7 @@ async def to_code(config): cv.Optional(validate_parameter_name): cv.templatable(cv.valid), }, ), + synchronous=True, ) async def script_execute_action_to_code(config, action_id, template_arg, args): def convert(type: str): @@ -208,6 +209,7 @@ async def script_execute_action_to_code(config, action_id, template_arg, args): "script.stop", ScriptStopAction, maybe_simple_id({cv.Required(CONF_ID): cv.use_id(Script)}), + synchronous=True, ) async def script_stop_action_to_code(config, action_id, template_arg, args): full_id, paren = await cg.get_variable_with_full_id(config[CONF_ID]) diff --git a/esphome/components/select/__init__.py b/esphome/components/select/__init__.py index 84ad591ba1..c114b140a9 100644 --- a/esphome/components/select/__init__.py +++ b/esphome/components/select/__init__.py @@ -145,6 +145,7 @@ OPERATION_BASE_SCHEMA = cv.Schema( cv.Required(CONF_OPTION): cv.templatable(cv.string_strict), } ), + synchronous=True, ) async def select_set_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -162,6 +163,7 @@ async def select_set_to_code(config, action_id, template_arg, args): cv.Required(CONF_INDEX): cv.templatable(cv.positive_int), } ), + synchronous=True, ) async def select_set_index_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -217,6 +219,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): cv.Optional(CONF_CYCLE, default=True): cv.templatable(cv.boolean), } ), + synchronous=True, ) @automation.register_action( "select.next", @@ -229,6 +232,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + synchronous=True, ) @automation.register_action( "select.previous", @@ -243,6 +247,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + synchronous=True, ) @automation.register_action( "select.first", @@ -254,6 +259,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + synchronous=True, ) @automation.register_action( "select.last", @@ -265,6 +271,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + synchronous=True, ) async def select_operation_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/components/switch/__init__.py b/esphome/components/switch/__init__.py index cfc5e2b6e8..6f1be7d53d 100644 --- a/esphome/components/switch/__init__.py +++ b/esphome/components/switch/__init__.py @@ -204,7 +204,7 @@ SWITCH_CONTROL_ACTION_SCHEMA = automation.maybe_simple_id( @automation.register_action( - "switch.control", ControlAction, SWITCH_CONTROL_ACTION_SCHEMA + "switch.control", ControlAction, SWITCH_CONTROL_ACTION_SCHEMA, synchronous=True ) async def switch_control_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -214,9 +214,15 @@ async def switch_control_to_code(config, action_id, template_arg, args): return var -@automation.register_action("switch.toggle", ToggleAction, SWITCH_ACTION_SCHEMA) -@automation.register_action("switch.turn_off", TurnOffAction, SWITCH_ACTION_SCHEMA) -@automation.register_action("switch.turn_on", TurnOnAction, SWITCH_ACTION_SCHEMA) +@automation.register_action( + "switch.toggle", ToggleAction, SWITCH_ACTION_SCHEMA, synchronous=True +) +@automation.register_action( + "switch.turn_off", TurnOffAction, SWITCH_ACTION_SCHEMA, synchronous=True +) +@automation.register_action( + "switch.turn_on", TurnOnAction, SWITCH_ACTION_SCHEMA, synchronous=True +) async def switch_toggle_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) return cg.new_Pvariable(action_id, template_arg, paren) diff --git a/esphome/components/text/__init__.py b/esphome/components/text/__init__.py index 9ceea0dfdf..61f7119cad 100644 --- a/esphome/components/text/__init__.py +++ b/esphome/components/text/__init__.py @@ -164,6 +164,7 @@ OPERATION_BASE_SCHEMA = cv.Schema( cv.Required(CONF_VALUE): cv.templatable(cv.string_strict), } ), + synchronous=True, ) async def text_set_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index d502c4d27f..6047202753 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -81,6 +81,20 @@ class StringRef { operator std::string() const { return str(); } + /// Compare (compatible with std::string::compare) + int compare(const StringRef &other) const { + int result = std::memcmp(base_, other.base_, std::min(len_, other.len_)); + if (result != 0) + return result; + if (len_ < other.len_) + return -1; + if (len_ > other.len_) + return 1; + return 0; + } + int compare(const char *s) const { return compare(StringRef(s)); } + int compare(const std::string &s) const { return compare(StringRef(s)); } + /// Find first occurrence of substring, returns std::string::npos if not found. /// Note: Requires the underlying string to be null-terminated. size_type find(const char *s, size_type pos = 0) const { diff --git a/esphome/util.py b/esphome/util.py index 7b896de27e..686aa74306 100644 --- a/esphome/util.py +++ b/esphome/util.py @@ -24,11 +24,14 @@ class RegistryEntry: fun: Callable[..., Any], type_id: "MockObjClass", schema: "Schema", + *, + synchronous: bool = False, ): self.name = name self.fun = fun self.type_id = type_id self.raw_schema = schema + self.synchronous = synchronous @property def coroutine_fun(self): @@ -49,9 +52,18 @@ class Registry(dict[str, RegistryEntry]): self.base_schema = base_schema or {} self.type_id_key = type_id_key - def register(self, name: str, type_id: "MockObjClass", schema: "Schema"): + def register( + self, + name: str, + type_id: "MockObjClass", + schema: "Schema", + *, + synchronous: bool = False, + ): def decorator(fun: Callable[..., Any]): - self[name] = RegistryEntry(name, fun, type_id, schema) + self[name] = RegistryEntry( + name, fun, type_id, schema, synchronous=synchronous + ) return fun return decorator diff --git a/tests/unit_tests/test_automation.py b/tests/unit_tests/test_automation.py new file mode 100644 index 0000000000..61fef8201d --- /dev/null +++ b/tests/unit_tests/test_automation.py @@ -0,0 +1,177 @@ +"""Tests for esphome.automation module.""" + +from collections.abc import Generator +from unittest.mock import patch + +import pytest + +from esphome.automation import has_non_synchronous_actions +from esphome.util import RegistryEntry + + +def _make_registry(non_synchronous_actions: set[str]) -> dict[str, RegistryEntry]: + """Create a mock ACTION_REGISTRY with specified non-synchronous actions. + + Uses the default synchronous=False, matching the real registry behavior. + """ + registry: dict[str, RegistryEntry] = {} + for name in non_synchronous_actions: + registry[name] = RegistryEntry(name, lambda: None, None, None) + return registry + + +@pytest.fixture +def mock_registry() -> Generator[dict[str, RegistryEntry]]: + """Fixture that patches ACTION_REGISTRY with delay, wait_until, script.wait as non-synchronous.""" + registry: dict[str, RegistryEntry] = _make_registry( + {"delay", "wait_until", "script.wait"} + ) + registry["logger.log"] = RegistryEntry( + "logger.log", lambda: None, None, None, synchronous=True + ) + with patch("esphome.automation.ACTION_REGISTRY", registry): + yield registry + + +def test_has_non_synchronous_actions_empty_list( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_non_synchronous_actions([]) is False + + +def test_has_non_synchronous_actions_empty_dict( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_non_synchronous_actions({}) is False + + +def test_has_non_synchronous_actions_non_dict_non_list( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_non_synchronous_actions("string") is False + assert has_non_synchronous_actions(42) is False + assert has_non_synchronous_actions(None) is False + + +def test_has_non_synchronous_actions_delay( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_non_synchronous_actions([{"delay": "1s"}]) is True + + +def test_has_non_synchronous_actions_wait_until( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_non_synchronous_actions([{"wait_until": {"condition": {}}}]) is True + + +def test_has_non_synchronous_actions_script_wait( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_non_synchronous_actions([{"script.wait": "script_id"}]) is True + + +def test_has_non_synchronous_actions_synchronous( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_non_synchronous_actions([{"logger.log": "hello"}]) is False + + +def test_has_non_synchronous_actions_unknown_not_in_registry( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Unknown actions not in registry are not flagged (only registered actions count).""" + assert has_non_synchronous_actions([{"unknown.action": "value"}]) is False + + +def test_has_non_synchronous_actions_default_non_synchronous( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Actions registered without explicit synchronous=True default to non-synchronous.""" + mock_registry["some.action"] = RegistryEntry( + "some.action", lambda: None, None, None + ) + assert has_non_synchronous_actions([{"some.action": "value"}]) is True + + +def test_has_non_synchronous_actions_nested_in_then( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Non-synchronous action nested inside a synchronous action's then block.""" + actions: list[dict[str, object]] = [ + { + "logger.log": "first", + "then": [{"delay": "1s"}], + } + ] + assert has_non_synchronous_actions(actions) is True + + +def test_has_non_synchronous_actions_deeply_nested( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Non-synchronous action deeply nested in action structure.""" + actions: list[dict[str, object]] = [ + { + "if": { + "then": [ + {"logger.log": "hello"}, + {"delay": "500ms"}, + ] + } + } + ] + assert has_non_synchronous_actions(actions) is True + + +def test_has_non_synchronous_actions_none_in_nested( + mock_registry: dict[str, RegistryEntry], +) -> None: + """No non-synchronous actions even with nesting.""" + actions: list[dict[str, object]] = [ + { + "if": { + "then": [ + {"logger.log": "hello"}, + ] + } + } + ] + assert has_non_synchronous_actions(actions) is False + + +def test_has_non_synchronous_actions_multiple_one_non_synchronous( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert ( + has_non_synchronous_actions( + [ + {"logger.log": "first"}, + {"delay": "1s"}, + {"logger.log": "second"}, + ] + ) + is True + ) + + +def test_has_non_synchronous_actions_multiple_all_synchronous( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert ( + has_non_synchronous_actions( + [ + {"logger.log": "first"}, + {"logger.log": "second"}, + ] + ) + is False + ) + + +def test_has_non_synchronous_actions_dict_input( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Direct dict input (single action).""" + assert has_non_synchronous_actions({"delay": "1s"}) is True + assert has_non_synchronous_actions({"logger.log": "hello"}) is False From 3e7424b307b5763bba217d5f0d5d18130b67be4a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Mar 2026 15:22:55 -1000 Subject: [PATCH 2/2] [preferences] Reduce heap churn with small inline buffer optimization (#13259) --- esphome/components/esp32/preferences.cpp | 33 ++++----- esphome/components/libretiny/preferences.cpp | 33 ++++----- esphome/core/helpers.h | 72 ++++++++++++++++++++ 3 files changed, 96 insertions(+), 42 deletions(-) diff --git a/esphome/components/esp32/preferences.cpp b/esphome/components/esp32/preferences.cpp index 8d6fdc86f6..a3ef10b21f 100644 --- a/esphome/components/esp32/preferences.cpp +++ b/esphome/components/esp32/preferences.cpp @@ -19,16 +19,7 @@ static constexpr size_t KEY_BUFFER_SIZE = 12; struct NVSData { uint32_t key; - std::unique_ptr data; - size_t len; - - void set_data(const uint8_t *src, size_t size) { - if (!this->data || this->len != size) { - this->data = std::make_unique(size); - this->len = size; - } - memcpy(this->data.get(), src, size); - } + SmallInlineBuffer<8> data; // Most prefs fit in 8 bytes (covers fan, cover, select, etc.) }; static std::vector s_pending_save; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) @@ -41,14 +32,14 @@ class ESP32PreferenceBackend : public ESPPreferenceBackend { // try find in pending saves and update that for (auto &obj : s_pending_save) { if (obj.key == this->key) { - obj.set_data(data, len); + obj.data.set(data, len); return true; } } NVSData save{}; save.key = this->key; - save.set_data(data, len); - s_pending_save.emplace_back(std::move(save)); + save.data.set(data, len); + s_pending_save.push_back(std::move(save)); ESP_LOGVV(TAG, "s_pending_save: key: %" PRIu32 ", len: %zu", this->key, len); return true; } @@ -56,11 +47,11 @@ class ESP32PreferenceBackend : public ESPPreferenceBackend { // try find in pending saves and load from that for (auto &obj : s_pending_save) { if (obj.key == this->key) { - if (obj.len != len) { + if (obj.data.size() != len) { // size mismatch return false; } - memcpy(data, obj.data.get(), len); + memcpy(data, obj.data.data(), len); return true; } } @@ -133,10 +124,10 @@ class ESP32Preferences : public ESPPreferences { snprintf(key_str, sizeof(key_str), "%" PRIu32, save.key); ESP_LOGVV(TAG, "Checking if NVS data %s has changed", key_str); if (this->is_changed_(this->nvs_handle, save, key_str)) { - esp_err_t err = nvs_set_blob(this->nvs_handle, key_str, save.data.get(), save.len); - ESP_LOGV(TAG, "sync: key: %s, len: %zu", key_str, save.len); + esp_err_t err = nvs_set_blob(this->nvs_handle, key_str, save.data.data(), save.data.size()); + ESP_LOGV(TAG, "sync: key: %s, len: %zu", key_str, save.data.size()); if (err != 0) { - ESP_LOGV(TAG, "nvs_set_blob('%s', len=%zu) failed: %s", key_str, save.len, esp_err_to_name(err)); + ESP_LOGV(TAG, "nvs_set_blob('%s', len=%zu) failed: %s", key_str, save.data.size(), esp_err_to_name(err)); failed++; last_err = err; last_key = save.key; @@ -144,7 +135,7 @@ class ESP32Preferences : public ESPPreferences { } written++; } else { - ESP_LOGV(TAG, "NVS data not changed skipping %" PRIu32 " len=%zu", save.key, save.len); + ESP_LOGV(TAG, "NVS data not changed skipping %" PRIu32 " len=%zu", save.key, save.data.size()); cached++; } } @@ -176,7 +167,7 @@ class ESP32Preferences : public ESPPreferences { return true; } // Check size first before allocating memory - if (actual_len != to_save.len) { + if (actual_len != to_save.data.size()) { return true; } // Most preferences are small, use stack buffer with heap fallback for large ones @@ -186,7 +177,7 @@ class ESP32Preferences : public ESPPreferences { ESP_LOGV(TAG, "nvs_get_blob('%s') failed: %s", key_str, esp_err_to_name(err)); return true; } - return memcmp(to_save.data.get(), stored_data.get(), to_save.len) != 0; + return memcmp(to_save.data.data(), stored_data.get(), to_save.data.size()) != 0; } bool reset() override { diff --git a/esphome/components/libretiny/preferences.cpp b/esphome/components/libretiny/preferences.cpp index 740c1a233a..1c101136e1 100644 --- a/esphome/components/libretiny/preferences.cpp +++ b/esphome/components/libretiny/preferences.cpp @@ -17,16 +17,7 @@ static constexpr size_t KEY_BUFFER_SIZE = 12; struct NVSData { uint32_t key; - std::unique_ptr data; - size_t len; - - void set_data(const uint8_t *src, size_t size) { - if (!this->data || this->len != size) { - this->data = std::make_unique(size); - this->len = size; - } - memcpy(this->data.get(), src, size); - } + SmallInlineBuffer<8> data; // Most prefs fit in 8 bytes (covers fan, cover, select, etc.) }; static std::vector s_pending_save; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) @@ -41,14 +32,14 @@ class LibreTinyPreferenceBackend : public ESPPreferenceBackend { // try find in pending saves and update that for (auto &obj : s_pending_save) { if (obj.key == this->key) { - obj.set_data(data, len); + obj.data.set(data, len); return true; } } NVSData save{}; save.key = this->key; - save.set_data(data, len); - s_pending_save.emplace_back(std::move(save)); + save.data.set(data, len); + s_pending_save.push_back(std::move(save)); ESP_LOGVV(TAG, "s_pending_save: key: %" PRIu32 ", len: %zu", this->key, len); return true; } @@ -57,11 +48,11 @@ class LibreTinyPreferenceBackend : public ESPPreferenceBackend { // try find in pending saves and load from that for (auto &obj : s_pending_save) { if (obj.key == this->key) { - if (obj.len != len) { + if (obj.data.size() != len) { // size mismatch return false; } - memcpy(data, obj.data.get(), len); + memcpy(data, obj.data.data(), len); return true; } } @@ -122,11 +113,11 @@ class LibreTinyPreferences : public ESPPreferences { snprintf(key_str, sizeof(key_str), "%" PRIu32, save.key); ESP_LOGVV(TAG, "Checking if FDB data %s has changed", key_str); if (this->is_changed_(&this->db, save, key_str)) { - ESP_LOGV(TAG, "sync: key: %s, len: %zu", key_str, save.len); - fdb_blob_make(&this->blob, save.data.get(), save.len); + ESP_LOGV(TAG, "sync: key: %s, len: %zu", key_str, save.data.size()); + fdb_blob_make(&this->blob, save.data.data(), save.data.size()); fdb_err_t err = fdb_kv_set_blob(&this->db, key_str, &this->blob); if (err != FDB_NO_ERR) { - ESP_LOGV(TAG, "fdb_kv_set_blob('%s', len=%zu) failed: %d", key_str, save.len, err); + ESP_LOGV(TAG, "fdb_kv_set_blob('%s', len=%zu) failed: %d", key_str, save.data.size(), err); failed++; last_err = err; last_key = save.key; @@ -134,7 +125,7 @@ class LibreTinyPreferences : public ESPPreferences { } written++; } else { - ESP_LOGD(TAG, "FDB data not changed; skipping %" PRIu32 " len=%zu", save.key, save.len); + ESP_LOGD(TAG, "FDB data not changed; skipping %" PRIu32 " len=%zu", save.key, save.data.size()); cached++; } } @@ -159,7 +150,7 @@ class LibreTinyPreferences : public ESPPreferences { } // Check size first - if different, data has changed - if (kv.value_len != to_save.len) { + if (kv.value_len != to_save.data.size()) { return true; } @@ -173,7 +164,7 @@ class LibreTinyPreferences : public ESPPreferences { } // Compare the actual data - return memcmp(to_save.data.get(), stored_data.get(), kv.value_len) != 0; + return memcmp(to_save.data.data(), stored_data.get(), kv.value_len) != 0; } bool reset() override { diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index b606e68df3..65d590a5e6 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -133,6 +133,78 @@ template class ConstVector { size_t size_; }; +/// Small buffer optimization - stores data inline when small, heap-allocates for large data +/// This avoids heap fragmentation for common small allocations while supporting arbitrary sizes. +/// Memory management is encapsulated - callers just use set() and data(). +template class SmallInlineBuffer { + public: + SmallInlineBuffer() = default; + ~SmallInlineBuffer() { + if (!this->is_inline_()) + delete[] this->heap_; + } + + // Move constructor + SmallInlineBuffer(SmallInlineBuffer &&other) noexcept : len_(other.len_) { + if (other.is_inline_()) { + memcpy(this->inline_, other.inline_, this->len_); + } else { + this->heap_ = other.heap_; + other.heap_ = nullptr; + } + other.len_ = 0; + } + + // Move assignment + SmallInlineBuffer &operator=(SmallInlineBuffer &&other) noexcept { + if (this != &other) { + if (!this->is_inline_()) + delete[] this->heap_; + this->len_ = other.len_; + if (other.is_inline_()) { + memcpy(this->inline_, other.inline_, this->len_); + } else { + this->heap_ = other.heap_; + other.heap_ = nullptr; + } + other.len_ = 0; + } + return *this; + } + + // Disable copy (would need deep copy of heap data) + SmallInlineBuffer(const SmallInlineBuffer &) = delete; + SmallInlineBuffer &operator=(const SmallInlineBuffer &) = delete; + + /// Set buffer contents, allocating heap if needed + void set(const uint8_t *src, size_t size) { + // Free existing heap allocation if switching from heap to inline or different heap size + if (!this->is_inline_() && (size <= InlineSize || size != this->len_)) { + delete[] this->heap_; + this->heap_ = nullptr; // Defensive: prevent use-after-free if logic changes + } + // Allocate new heap buffer if needed + if (size > InlineSize && (this->is_inline_() || size != this->len_)) { + this->heap_ = new uint8_t[size]; // NOLINT(cppcoreguidelines-owning-memory) + } + this->len_ = size; + memcpy(this->data(), src, size); + } + + uint8_t *data() { return this->is_inline_() ? this->inline_ : this->heap_; } + const uint8_t *data() const { return this->is_inline_() ? this->inline_ : this->heap_; } + size_t size() const { return this->len_; } + + protected: + bool is_inline_() const { return this->len_ <= InlineSize; } + + size_t len_{0}; + union { + uint8_t inline_[InlineSize]{}; // Zero-init ensures clean initial state + uint8_t *heap_; + }; +}; + /// Minimal static vector - saves memory by avoiding std::vector overhead template class StaticVector { public: