From 0b2f79480be3f703b84ea616425987c6bdab93eb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Feb 2026 19:51:39 -0600 Subject: [PATCH 01/22] [api] Use StringRef for user service string arguments Replace std::string with StringRef (non-owning string view) for user service string arguments in YAML-generated services. This avoids unnecessary heap allocation when the protobuf decode buffer already contains the string data. Key changes: - Frame helpers reserve +1 byte in rx_buf_ so string fields can be safely null-terminated in-place after decode - Add (null_terminate) protobuf field option to target only fields that need it (ExecuteServiceArgument.string_ and HomeAssistantStateResponse.state) - Add StringRef template specializations for get_execute_arg_value and to_service_arg_type - Python codegen uses StringRef for string service args, with automatic std::string fallback when deferred actions (delay, wait_until, script.wait) are present in the action chain - Add deferred flag to action registry for detecting actions that store trigger args for later execution - Simplify HomeAssistantStateResponse handler by removing SmallBufferWithHeapFallback copy (state is null-terminated in-place) - Add compare() method to StringRef for external component compatibility Saves ~240 bytes of flash on ESP8266 by eliminating std::string template instantiations for user service string arguments. --- esphome/automation.py | 37 +++++++++++++++++-- esphome/components/api/__init__.py | 9 ++++- esphome/components/api/api.proto | 4 +- esphome/components/api/api_connection.cpp | 23 +++--------- .../components/api/api_frame_helper_noise.cpp | 7 ++-- .../api/api_frame_helper_plaintext.cpp | 7 ++-- esphome/components/api/api_options.proto | 9 +++++ esphome/components/api/api_pb2.cpp | 9 +++++ esphome/components/api/api_pb2.h | 1 + esphome/components/api/user_services.cpp | 5 +++ esphome/components/script/__init__.py | 1 + esphome/core/string_ref.h | 13 +++++++ esphome/util.py | 14 ++++++- script/api_protobuf/api_protobuf.py | 17 ++++++++- 14 files changed, 121 insertions(+), 35 deletions(-) diff --git a/esphome/automation.py b/esphome/automation.py index 2439b1ddc4..c937470eee 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -57,8 +57,14 @@ 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, + *, + deferred: bool = False, +): + return ACTION_REGISTRY.register(name, action_type, schema, deferred=deferred) def register_condition(name: str, condition_type: MockObjClass, schema: cv.Schema): @@ -335,7 +341,10 @@ 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), + deferred=True, ) async def delay_action_to_code( config: ConfigType, @@ -445,7 +454,7 @@ _validate_wait_until = cv.maybe_simple_value( ) -@register_action("wait_until", WaitUntilAction, _validate_wait_until) +@register_action("wait_until", WaitUntilAction, _validate_wait_until, deferred=True) async def wait_until_action_to_code( config: ConfigType, action_id: ID, @@ -578,6 +587,26 @@ async def build_condition_list( return conditions +def has_deferred_actions(actions: ConfigType) -> bool: + """Check if a validated action list contains any deferred actions. + + Deferred actions (delay, wait_until, script.wait) store trigger args + for later execution, making non-owning types like StringRef unsafe. + """ + if isinstance(actions, list): + return any(has_deferred_actions(item) for item in actions) + if isinstance(actions, dict): + for key in actions: + if key in ACTION_REGISTRY and ACTION_REGISTRY[key].deferred: + return True + return any( + has_deferred_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 9bff9f5635..7df23ae1ba 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,16 @@ async def to_code(config: ConfigType) -> None: if is_optional: func_args.append((cg.bool_, "return_response")) + # Check if action chain has deferred actions that would make + # non-owning StringRef dangle (rx_buf_ reused after delay) + has_deferred = automation.has_deferred_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 deferred actions exist + if has_deferred and native is cg.StringRef: + native = cg.std_string service_template_args.append(native) func_args.append((native, name)) service_arg_names.append(name) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 18dac6a2d1..b8dfb71a6a 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -824,7 +824,7 @@ message HomeAssistantStateResponse { option (ifdef) = "USE_API_HOMEASSISTANT_STATES"; string entity_id = 1; - string state = 2; + string state = 2 [(null_terminate) = true]; string attribute = 3; } @@ -882,7 +882,7 @@ message ExecuteServiceArgument { bool bool_ = 1; int32 legacy_int = 2; float float_ = 3; - string string_ = 4; + string string_ = 4 [(null_terminate) = true]; // ESPHome 1.14 (api v1.3) make int a signed value sint32 int_ = 5; repeated bool bool_array = 6 [packed=false, (fixed_vector) = true]; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 4d564af9e2..bffcd490ac 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1683,31 +1683,18 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes } 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)) { + // it.attribute can be nullptr (meaning no attribute filter) + 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)); + // msg.state is already null-terminated in-place after protobuf decode + it.callback(msg.state); } } #endif diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 1ae848dead..a6928bb936 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -201,9 +201,10 @@ APIError APINoiseFrameHelper::try_read_frame_() { return (state_ == State::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 (+1 for null terminator so protobuf StringRef fields + // can be safely null-terminated in-place after decode) + if (this->rx_buf_.size() != msg_size + 1) { + this->rx_buf_.resize(msg_size + 1); } if (rx_buf_len_ < msg_size) { diff --git a/esphome/components/api/api_frame_helper_plaintext.cpp b/esphome/components/api/api_frame_helper_plaintext.cpp index 5069dbf68b..b721843d07 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 (+1 for 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_ + 1) { + this->rx_buf_.resize(this->rx_header_parsed_len_ + 1); } if (rx_buf_len_ < rx_header_parsed_len_) { diff --git a/esphome/components/api/api_options.proto b/esphome/components/api/api_options.proto index a863f2c7a8..163a170fb9 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -90,4 +90,13 @@ extend google.protobuf.FieldOptions { // - uint16_t _length_{0}; // - uint16_t _count_{0}; optional bool packed_buffer = 50015 [default=false]; + + // null_terminate: Write a null byte after string data in the decode buffer. + // When set on a string field in a SOURCE_CLIENT (decodable) message, the + // generated decode() override writes '\0' at data[length] after decoding. + // This makes the StringRef safe for c_str() usage without copying. + // Safe because: (1) frame helpers reserve +1 byte in rx_buf_, and + // (2) the overwritten byte was already consumed during decode. + // Only mark fields that actually need null-terminated access. + optional bool null_terminate = 50016 [default=false]; } diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 743f51dac7..015dae37ab 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -953,6 +953,12 @@ bool HomeAssistantStateResponse::decode_length(uint32_t field_id, ProtoLengthDel } return true; } +void HomeAssistantStateResponse::decode(const uint8_t *buffer, size_t length) { + ProtoDecodableMessage::decode(buffer, length); + if (!this->state.empty()) { + const_cast(this->state.c_str())[this->state.size()] = '\0'; + } +} #endif bool GetTimeResponse::decode_length(uint32_t field_id, ProtoLengthDelimited value) { switch (field_id) { @@ -1057,6 +1063,9 @@ void ExecuteServiceArgument::decode(const uint8_t *buffer, size_t length) { uint32_t count_string_array = ProtoDecodableMessage::count_repeated_field(buffer, length, 9); this->string_array.init(count_string_array); ProtoDecodableMessage::decode(buffer, length); + if (!this->string_.empty()) { + const_cast(this->string_.c_str())[this->string_.size()] = '\0'; + } } bool ExecuteServiceRequest::decode_varint(uint32_t field_id, ProtoVarInt value) { switch (field_id) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index d001f869c5..6cf60bee0a 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1095,6 +1095,7 @@ class HomeAssistantStateResponse final : public ProtoDecodableMessage { StringRef entity_id{}; StringRef state{}; StringRef attribute{}; + void decode(const uint8_t *buffer, size_t length) override; #ifdef HAS_PROTO_MESSAGE_DUMP const char *dump_to(DumpBuffer &out) const override; #endif 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/script/__init__.py b/esphome/components/script/__init__.py index 8d69981db0..0a9e289511 100644 --- a/esphome/components/script/__init__.py +++ b/esphome/components/script/__init__.py @@ -219,6 +219,7 @@ async def script_stop_action_to_code(config, action_id, template_arg, args): "script.wait", ScriptWaitAction, maybe_simple_id({cv.Required(CONF_ID): cv.use_id(Script)}), + deferred=True, ) async def script_wait_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/core/string_ref.h b/esphome/core/string_ref.h index d502c4d27f..89ea9dd797 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -81,6 +81,19 @@ class StringRef { operator std::string() const { return str(); } + /// Compare with a null-terminated C string (compatible with std::string::compare) + int compare(const char *s) const { + size_t s_len = std::strlen(s); + int result = std::memcmp(base_, s, std::min(len_, s_len)); + if (result != 0) + return result; + if (len_ < s_len) + return -1; + if (len_ > s_len) + return 1; + return 0; + } + /// 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..9fb7ef6227 100644 --- a/esphome/util.py +++ b/esphome/util.py @@ -24,11 +24,14 @@ class RegistryEntry: fun: Callable[..., Any], type_id: "MockObjClass", schema: "Schema", + *, + deferred: bool = False, ): self.name = name self.fun = fun self.type_id = type_id self.raw_schema = schema + self.deferred = deferred @property def coroutine_fun(self): @@ -49,9 +52,16 @@ 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", + *, + deferred: bool = False, + ): def decorator(fun: Callable[..., Any]): - self[name] = RegistryEntry(name, fun, type_id, schema) + self[name] = RegistryEntry(name, fun, type_id, schema, deferred=deferred) return fun return decorator diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 4fbee49dae..30e827e9a7 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -2020,6 +2020,8 @@ def build_message_type( # Collect fixed_vector fields for custom decode generation fixed_vector_fields = [] + # Collect fields with (null_terminate) = true option + null_terminate_fields = [] for field in desc.field: # Skip deprecated fields completely @@ -2062,6 +2064,10 @@ def build_message_type( ti = create_field_type_info(field, needs_decode, needs_encode) + # Collect fields with (null_terminate) = true for post-decode null-termination + if needs_decode and get_field_opt(field, pb.null_terminate, False): + null_terminate_fields.append(ti.field_name) + # Skip field declarations for fields that are in the base class # but include their encode/decode logic if field.name not in common_field_names: @@ -2168,8 +2174,8 @@ def build_message_type( prot = "bool decode_64bit(uint32_t field_id, Proto64Bit value) override;" protected_content.insert(0, prot) - # Generate custom decode() override for messages with FixedVector fields - if fixed_vector_fields: + # Generate custom decode() override for messages with FixedVector or null_terminate fields + if fixed_vector_fields or null_terminate_fields: # Generate the decode() implementation in cpp o = f"void {desc.name}::decode(const uint8_t *buffer, size_t length) {{\n" # Count and init each FixedVector field @@ -2178,6 +2184,13 @@ def build_message_type( o += f" this->{field_name}.init(count_{field_name});\n" # Call parent decode to populate the fields o += " ProtoDecodableMessage::decode(buffer, length);\n" + # Null-terminate fields marked with (null_terminate) = true in-place. + # Safe: decode is complete, byte after string was already parsed (next field tag) + # or is the +1 reserved byte at end of rx_buf_. + for field_name in null_terminate_fields: + o += f" if (!this->{field_name}.empty()) {{\n" + o += f" const_cast(this->{field_name}.c_str())[this->{field_name}.size()] = '\\0';\n" + o += " }\n" o += "}\n" cpp += o # Generate the decode() declaration in header (public method) From 66a2a0d62ea8585656c420e11b49ec1ad9893bb0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 19:20:24 -0600 Subject: [PATCH 02/22] only during data --- esphome/components/api/api_frame_helper_noise.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 34b5ba3c3f..882a9e86f4 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -201,10 +201,13 @@ APIError APINoiseFrameHelper::try_read_frame_() { return (state_ == State::DATA) ? APIError::BAD_DATA_PACKET : APIError::BAD_HANDSHAKE_PACKET_LEN; } - // Reserve space for body (+1 for null terminator so protobuf StringRef fields - // can be safely null-terminated in-place after decode) - if (this->rx_buf_.size() != msg_size + 1) { - this->rx_buf_.resize(msg_size + 1); + // Reserve space for body (+1 for 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 + (state_ == State::DATA ? 1 : 0); + if (this->rx_buf_.size() != alloc_size) { + this->rx_buf_.resize(alloc_size); } if (rx_buf_len_ < msg_size) { From 367775dfd9e20d965ef010bf1bfe5084e0aeec58 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 19:24:05 -0600 Subject: [PATCH 03/22] only during data --- esphome/components/api/api_frame_helper.h | 4 ++++ esphome/components/api/api_frame_helper_noise.cpp | 9 ++++++--- esphome/components/api/api_frame_helper_plaintext.cpp | 6 +++--- 3 files changed, 13 insertions(+), 6 deletions(-) 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 882a9e86f4..3fc5040e95 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -201,11 +201,11 @@ APIError APINoiseFrameHelper::try_read_frame_() { return (state_ == State::DATA) ? APIError::BAD_DATA_PACKET : APIError::BAD_HANDSHAKE_PACKET_LEN; } - // Reserve space for body (+1 for null terminator in DATA state so protobuf + // 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 + (state_ == State::DATA ? 1 : 0); + uint16_t alloc_size = msg_size + (state_ == State::DATA ? RX_BUF_NULL_TERMINATOR : 0); if (this->rx_buf_.size() != alloc_size) { this->rx_buf_.resize(alloc_size); } @@ -411,7 +411,10 @@ 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()); + // rx_buf_ has RX_BUF_NULL_TERMINATOR extra byte for null termination, + // but only the actual message bytes contain encrypted data + size_t msg_size = this->rx_buf_.size() - RX_BUF_NULL_TERMINATOR; + noise_buffer_set_inout(mbuf, this->rx_buf_.data(), msg_size, msg_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); diff --git a/esphome/components/api/api_frame_helper_plaintext.cpp b/esphome/components/api/api_frame_helper_plaintext.cpp index b721843d07..e2bb56e0ac 100644 --- a/esphome/components/api/api_frame_helper_plaintext.cpp +++ b/esphome/components/api/api_frame_helper_plaintext.cpp @@ -163,10 +163,10 @@ APIError APIPlaintextFrameHelper::try_read_frame_() { } // header reading done - // Reserve space for body (+1 for null terminator so protobuf StringRef fields + // 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_ + 1) { - this->rx_buf_.resize(this->rx_header_parsed_len_ + 1); + 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_) { From 57d5151c318c1fffaff232b35dd0cc59975a0ad1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 19:24:48 -0600 Subject: [PATCH 04/22] fix shadow --- esphome/components/api/api_frame_helper_noise.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 3fc5040e95..5a16b8018a 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -413,8 +413,8 @@ APIError APINoiseFrameHelper::read_packet(ReadPacketBuffer *buffer) { noise_buffer_init(mbuf); // rx_buf_ has RX_BUF_NULL_TERMINATOR extra byte for null termination, // but only the actual message bytes contain encrypted data - size_t msg_size = this->rx_buf_.size() - RX_BUF_NULL_TERMINATOR; - noise_buffer_set_inout(mbuf, this->rx_buf_.data(), msg_size, msg_size); + 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); From a4682615234ac61ff7825e2a922807b11e6772fa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 19:41:26 -0600 Subject: [PATCH 05/22] [scheduler] De-template and consolidate scheduler helper functions (#14164) --- esphome/core/scheduler.cpp | 14 +++++++++---- esphome/core/scheduler.h | 41 +++++++------------------------------- 2 files changed, 17 insertions(+), 38 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 36b65f6ff7..e4e0751e10 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -119,10 +119,16 @@ uint32_t Scheduler::calculate_interval_offset_(uint32_t delay) { // Remove before 2026.8.0 along with all retry code bool Scheduler::is_retry_cancelled_locked_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id) { - return has_cancelled_timeout_in_container_locked_(this->items_, component, name_type, static_name, hash_or_id, - /* match_retry= */ true) || - has_cancelled_timeout_in_container_locked_(this->to_add_, component, name_type, static_name, hash_or_id, - /* match_retry= */ true); + for (auto *container : {&this->items_, &this->to_add_}) { + for (auto &item : *container) { + if (item && this->is_item_removed_locked_(item.get()) && + this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, + /* match_retry= */ true, /* skip_removed= */ false)) { + return true; + } + } + } + return false; } // Common implementation for both timeout and interval diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 384d76b6b0..16b0ded312 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -308,8 +308,8 @@ class Scheduler { SchedulerItem::Type type, bool match_retry, bool skip_removed = true) const { // THREAD SAFETY: Check for nullptr first to prevent LoadProhibited crashes. On multi-threaded // platforms, items can be moved out of defer_queue_ during processing, leaving nullptr entries. - // PR #11305 added nullptr checks in callers (mark_matching_items_removed_locked_() and - // has_cancelled_timeout_in_container_locked_()), but this check provides defense-in-depth: helper + // PR #11305 added nullptr checks in callers (mark_matching_items_removed_locked_()), but this check + // provides defense-in-depth: helper // functions should be safe regardless of caller behavior. // Fixes: https://github.com/esphome/esphome/issues/11940 if (!item) @@ -403,8 +403,7 @@ class Scheduler { // SAFETY: Moving out the unique_ptr leaves a nullptr in the vector at defer_queue_front_. // This is intentional and safe because: // 1. The vector is only cleaned up by cleanup_defer_queue_locked_() at the end of this function - // 2. Any code iterating defer_queue_ MUST check for nullptr items (see mark_matching_items_removed_locked_ - // and has_cancelled_timeout_in_container_locked_ in scheduler.h) + // 2. Any code iterating defer_queue_ MUST check for nullptr items (see mark_matching_items_removed_locked_) // 3. The lock protects concurrent access, but the nullptr remains until cleanup item = std::move(this->defer_queue_[this->defer_queue_front_]); this->defer_queue_front_++; @@ -497,19 +496,16 @@ class Scheduler { // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id // Returns the number of items marked for removal // IMPORTANT: Must be called with scheduler lock held - template - size_t mark_matching_items_removed_locked_(Container &container, Component *component, NameType name_type, - const char *static_name, uint32_t hash_or_id, SchedulerItem::Type type, - bool match_retry) { + size_t mark_matching_items_removed_locked_(std::vector> &container, + Component *component, NameType name_type, const char *static_name, + uint32_t hash_or_id, SchedulerItem::Type type, bool match_retry) { size_t count = 0; for (auto &item : container) { // Skip nullptr items (can happen in defer_queue_ when items are being processed) // The defer_queue_ uses index-based processing: items are std::moved out but left in the // vector as nullptr until cleanup. Even though this function is called with lock held, // the vector can still contain nullptr items from the processing loop. This check prevents crashes. - if (!item) - continue; - if (this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) { + if (item && this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) { this->set_item_removed_(item.get(), true); count++; } @@ -517,29 +513,6 @@ class Scheduler { return count; } - // Template helper to check if any item in a container matches our criteria - // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id - // IMPORTANT: Must be called with scheduler lock held - template - bool has_cancelled_timeout_in_container_locked_(const Container &container, Component *component, NameType name_type, - const char *static_name, uint32_t hash_or_id, - bool match_retry) const { - for (const auto &item : container) { - // Skip nullptr items (can happen in defer_queue_ when items are being processed) - // The defer_queue_ uses index-based processing: items are std::moved out but left in the - // vector as nullptr until cleanup. If this function is called during defer queue processing, - // it will iterate over these nullptr items. This check prevents crashes. - if (!item) - continue; - if (this->is_item_removed_locked_(item.get()) && - this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, - match_retry, /* skip_removed= */ false)) { - return true; - } - } - return false; - } - Mutex lock_; std::vector> items_; std::vector> to_add_; From d5c9c56fdfcdd0112e1913f05f84e297908460a7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 19:41:43 -0600 Subject: [PATCH 06/22] [platformio] Add exponential backoff and session reset to download retries (#14191) --- esphome/platformio_api.py | 41 ++++- tests/unit_tests/test_platformio_api.py | 196 +++++++++++++++++++++++- 2 files changed, 231 insertions(+), 6 deletions(-) diff --git a/esphome/platformio_api.py b/esphome/platformio_api.py index d42f89d029..5d4065207f 100644 --- a/esphome/platformio_api.py +++ b/esphome/platformio_api.py @@ -5,6 +5,7 @@ import os from pathlib import Path import re import subprocess +import time from typing import Any from esphome.const import CONF_COMPILE_PROCESS_LIMIT, CONF_ESPHOME, KEY_CORE @@ -44,31 +45,61 @@ def patch_structhash(): def patch_file_downloader(): - """Patch PlatformIO's FileDownloader to retry on PackageException errors.""" + """Patch PlatformIO's FileDownloader to retry on PackageException errors. + + PlatformIO's FileDownloader uses HTTPSession which lacks built-in retry + for 502/503 errors. We add retries with exponential backoff and close the + session between attempts to force a fresh TCP connection, which may route + to a different CDN edge node. + """ from platformio.package.download import FileDownloader from platformio.package.exception import PackageException + if getattr(FileDownloader.__init__, "_esphome_patched", False): + return + original_init = FileDownloader.__init__ def patched_init(self, *args: Any, **kwargs: Any) -> None: - max_retries = 3 + max_retries = 5 for attempt in range(max_retries): try: - return original_init(self, *args, **kwargs) + original_init(self, *args, **kwargs) + return except PackageException as e: if attempt < max_retries - 1: + # Exponential backoff: 2, 4, 8, 16 seconds + delay = 2 ** (attempt + 1) _LOGGER.warning( - "Package download failed: %s. Retrying... (attempt %d/%d)", + "Package download failed: %s. " + "Retrying in %d seconds... (attempt %d/%d)", str(e), + delay, attempt + 1, max_retries, ) + # Close the response and session to free resources + # and force a new TCP connection on retry, which may + # route to a different CDN edge node + # pylint: disable=protected-access,broad-except + try: + if ( + hasattr(self, "_http_response") + and self._http_response is not None + ): + self._http_response.close() + if hasattr(self, "_http_session"): + self._http_session.close() + except Exception: + pass + # pylint: enable=protected-access,broad-except + time.sleep(delay) else: # Final attempt - re-raise raise - return None + patched_init._esphome_patched = True # type: ignore[attr-defined] # pylint: disable=protected-access FileDownloader.__init__ = patched_init diff --git a/tests/unit_tests/test_platformio_api.py b/tests/unit_tests/test_platformio_api.py index 4d7b635e59..1686144277 100644 --- a/tests/unit_tests/test_platformio_api.py +++ b/tests/unit_tests/test_platformio_api.py @@ -6,7 +6,7 @@ import os from pathlib import Path import shutil from types import SimpleNamespace -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, call, patch import pytest @@ -673,6 +673,200 @@ def test_process_stacktrace_bad_alloc( assert state is False +def test_patch_file_downloader_succeeds_first_try() -> None: + """Test patch_file_downloader succeeds on first attempt.""" + mock_exception_cls = type("PackageException", (Exception,), {}) + original_init = MagicMock() + + with patch.dict( + "sys.modules", + { + "platformio": MagicMock(), + "platformio.package": MagicMock(), + "platformio.package.download": SimpleNamespace( + FileDownloader=type("FileDownloader", (), {"__init__": original_init}) + ), + "platformio.package.exception": SimpleNamespace( + PackageException=mock_exception_cls + ), + }, + ): + platformio_api.patch_file_downloader() + + from platformio.package.download import FileDownloader + + instance = object.__new__(FileDownloader) + FileDownloader.__init__(instance, "http://example.com/file.zip") + + original_init.assert_called_once() + + +def test_patch_file_downloader_retries_on_failure() -> None: + """Test patch_file_downloader retries with backoff on PackageException.""" + mock_exception_cls = type("PackageException", (Exception,), {}) + call_count = 0 + + def failing_init(self, *args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count < 3: + raise mock_exception_cls(f"502 error attempt {call_count}") + + with ( + patch.dict( + "sys.modules", + { + "platformio": MagicMock(), + "platformio.package": MagicMock(), + "platformio.package.download": SimpleNamespace( + FileDownloader=type( + "FileDownloader", (), {"__init__": failing_init} + ) + ), + "platformio.package.exception": SimpleNamespace( + PackageException=mock_exception_cls + ), + }, + ), + patch("time.sleep") as mock_sleep, + ): + platformio_api.patch_file_downloader() + + from platformio.package.download import FileDownloader + + instance = object.__new__(FileDownloader) + FileDownloader.__init__(instance, "http://example.com/file.zip") + + # Should have been called 3 times (2 failures + 1 success) + assert call_count == 3 + + # Should have slept with exponential backoff: 2s, 4s + assert mock_sleep.call_count == 2 + mock_sleep.assert_any_call(2) + mock_sleep.assert_any_call(4) + + +def test_patch_file_downloader_raises_after_max_retries() -> None: + """Test patch_file_downloader raises after exhausting all retries.""" + mock_exception_cls = type("PackageException", (Exception,), {}) + + def always_failing_init(self, *args, **kwargs): + raise mock_exception_cls("502 error") + + with ( + patch.dict( + "sys.modules", + { + "platformio": MagicMock(), + "platformio.package": MagicMock(), + "platformio.package.download": SimpleNamespace( + FileDownloader=type( + "FileDownloader", (), {"__init__": always_failing_init} + ) + ), + "platformio.package.exception": SimpleNamespace( + PackageException=mock_exception_cls + ), + }, + ), + patch("time.sleep") as mock_sleep, + ): + platformio_api.patch_file_downloader() + + from platformio.package.download import FileDownloader + + instance = object.__new__(FileDownloader) + with pytest.raises(mock_exception_cls, match="502 error"): + FileDownloader.__init__(instance, "http://example.com/file.zip") + + # Should have slept 4 times (before attempts 2-5), not on final attempt + assert mock_sleep.call_count == 4 + mock_sleep.assert_has_calls([call(2), call(4), call(8), call(16)]) + + +def test_patch_file_downloader_closes_session_and_response_between_retries() -> None: + """Test patch_file_downloader closes HTTP session and response between retries.""" + mock_exception_cls = type("PackageException", (Exception,), {}) + mock_session = MagicMock() + mock_response = MagicMock() + call_count = 0 + + def failing_init_with_session(self, *args, **kwargs): + nonlocal call_count + call_count += 1 + self._http_session = mock_session + self._http_response = mock_response + if call_count < 2: + raise mock_exception_cls("502 error") + + with ( + patch.dict( + "sys.modules", + { + "platformio": MagicMock(), + "platformio.package": MagicMock(), + "platformio.package.download": SimpleNamespace( + FileDownloader=type( + "FileDownloader", + (), + {"__init__": failing_init_with_session}, + ) + ), + "platformio.package.exception": SimpleNamespace( + PackageException=mock_exception_cls + ), + }, + ), + patch("time.sleep"), + ): + platformio_api.patch_file_downloader() + + from platformio.package.download import FileDownloader + + instance = object.__new__(FileDownloader) + FileDownloader.__init__(instance, "http://example.com/file.zip") + + # Both response and session should have been closed between retries + mock_response.close.assert_called_once() + mock_session.close.assert_called_once() + + +def test_patch_file_downloader_idempotent() -> None: + """Test patch_file_downloader does not stack wrappers when called multiple times.""" + mock_exception_cls = type("PackageException", (Exception,), {}) + call_count = 0 + + def counting_init(self, *args, **kwargs): + nonlocal call_count + call_count += 1 + + with patch.dict( + "sys.modules", + { + "platformio": MagicMock(), + "platformio.package": MagicMock(), + "platformio.package.download": SimpleNamespace( + FileDownloader=type("FileDownloader", (), {"__init__": counting_init}) + ), + "platformio.package.exception": SimpleNamespace( + PackageException=mock_exception_cls + ), + }, + ): + # Patch multiple times + platformio_api.patch_file_downloader() + platformio_api.patch_file_downloader() + platformio_api.patch_file_downloader() + + from platformio.package.download import FileDownloader + + instance = object.__new__(FileDownloader) + FileDownloader.__init__(instance, "http://example.com/file.zip") + + # Should only be called once, not 3 times from stacked wrappers + assert call_count == 1 + + def test_platformio_log_filter_allows_non_platformio_messages() -> None: """Test that non-platformio logger messages are allowed through.""" log_filter = platformio_api.PlatformioLogFilter() From ab7e02d6bfb9fae43d36113d2094fdb237633f32 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:21:34 -0600 Subject: [PATCH 07/22] tweak --- esphome/components/api/api.proto | 4 ++-- esphome/components/api/api_connection.cpp | 16 +++++++++++++++- esphome/components/api/api_options.proto | 9 --------- esphome/components/api/api_pb2.cpp | 9 --------- esphome/components/api/api_pb2.h | 1 - script/api_protobuf/api_protobuf.py | 17 ++--------------- 6 files changed, 19 insertions(+), 37 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index b8dfb71a6a..18dac6a2d1 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -824,7 +824,7 @@ message HomeAssistantStateResponse { option (ifdef) = "USE_API_HOMEASSISTANT_STATES"; string entity_id = 1; - string state = 2 [(null_terminate) = true]; + string state = 2; string attribute = 3; } @@ -882,7 +882,7 @@ message ExecuteServiceArgument { bool bool_ = 1; int32 legacy_int = 2; float float_ = 3; - string string_ = 4 [(null_terminate) = true]; + string string_ = 4; // ESPHome 1.14 (api v1.3) make int a signed value sint32 int_ = 5; repeated bool bool_array = 6 [packed=false, (fixed_vector) = true]; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 58917af968..5ee64c44f1 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1702,6 +1702,13 @@ 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_. + if (!msg.state.empty()) { + const_cast(msg.state.c_str())[msg.state.size()] = '\0'; + } + for (auto &it : this->parent_->get_state_subs()) { if (msg.entity_id != it.entity_id) { continue; @@ -1713,13 +1720,20 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes continue; } - // msg.state is already null-terminated in-place after protobuf decode 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. + for (auto &arg : 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_options.proto b/esphome/components/api/api_options.proto index 163a170fb9..a863f2c7a8 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -90,13 +90,4 @@ extend google.protobuf.FieldOptions { // - uint16_t _length_{0}; // - uint16_t _count_{0}; optional bool packed_buffer = 50015 [default=false]; - - // null_terminate: Write a null byte after string data in the decode buffer. - // When set on a string field in a SOURCE_CLIENT (decodable) message, the - // generated decode() override writes '\0' at data[length] after decoding. - // This makes the StringRef safe for c_str() usage without copying. - // Safe because: (1) frame helpers reserve +1 byte in rx_buf_, and - // (2) the overwritten byte was already consumed during decode. - // Only mark fields that actually need null-terminated access. - optional bool null_terminate = 50016 [default=false]; } diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index ac72b20c72..5c50a8aa5b 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -953,12 +953,6 @@ bool HomeAssistantStateResponse::decode_length(uint32_t field_id, ProtoLengthDel } return true; } -void HomeAssistantStateResponse::decode(const uint8_t *buffer, size_t length) { - ProtoDecodableMessage::decode(buffer, length); - if (!this->state.empty()) { - const_cast(this->state.c_str())[this->state.size()] = '\0'; - } -} #endif bool GetTimeResponse::decode_length(uint32_t field_id, ProtoLengthDelimited value) { switch (field_id) { @@ -1063,9 +1057,6 @@ void ExecuteServiceArgument::decode(const uint8_t *buffer, size_t length) { uint32_t count_string_array = ProtoDecodableMessage::count_repeated_field(buffer, length, 9); this->string_array.init(count_string_array); ProtoDecodableMessage::decode(buffer, length); - if (!this->string_.empty()) { - const_cast(this->string_.c_str())[this->string_.size()] = '\0'; - } } bool ExecuteServiceRequest::decode_varint(uint32_t field_id, ProtoVarInt value) { switch (field_id) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index b27d25ac2d..c90873d993 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1095,7 +1095,6 @@ class HomeAssistantStateResponse final : public ProtoDecodableMessage { StringRef entity_id{}; StringRef state{}; StringRef attribute{}; - void decode(const uint8_t *buffer, size_t length) override; #ifdef HAS_PROTO_MESSAGE_DUMP const char *dump_to(DumpBuffer &out) const override; #endif diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 6a338e1559..cc881caa5c 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -2028,8 +2028,6 @@ def build_message_type( # Collect fixed_vector fields for custom decode generation fixed_vector_fields = [] - # Collect fields with (null_terminate) = true option - null_terminate_fields = [] for field in desc.field: # Skip deprecated fields completely @@ -2072,10 +2070,6 @@ def build_message_type( ti = create_field_type_info(field, needs_decode, needs_encode) - # Collect fields with (null_terminate) = true for post-decode null-termination - if needs_decode and get_field_opt(field, pb.null_terminate, False): - null_terminate_fields.append(ti.field_name) - # Skip field declarations for fields that are in the base class # but include their encode/decode logic if field.name not in common_field_names: @@ -2182,8 +2176,8 @@ def build_message_type( prot = "bool decode_64bit(uint32_t field_id, Proto64Bit value) override;" protected_content.insert(0, prot) - # Generate custom decode() override for messages with FixedVector or null_terminate fields - if fixed_vector_fields or null_terminate_fields: + # Generate custom decode() override for messages with FixedVector fields + if fixed_vector_fields: # Generate the decode() implementation in cpp o = f"void {desc.name}::decode(const uint8_t *buffer, size_t length) {{\n" # Count and init each FixedVector field @@ -2192,13 +2186,6 @@ def build_message_type( o += f" this->{field_name}.init(count_{field_name});\n" # Call parent decode to populate the fields o += " ProtoDecodableMessage::decode(buffer, length);\n" - # Null-terminate fields marked with (null_terminate) = true in-place. - # Safe: decode is complete, byte after string was already parsed (next field tag) - # or is the +1 reserved byte at end of rx_buf_. - for field_name in null_terminate_fields: - o += f" if (!this->{field_name}.empty()) {{\n" - o += f" const_cast(this->{field_name}.c_str())[this->{field_name}.size()] = '\\0';\n" - o += " }\n" o += "}\n" cpp += o # Generate the decode() declaration in header (public method) From 15a125ca00b5075329546d8e96b4bfa9d10cfbef Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:28:07 -0600 Subject: [PATCH 08/22] overkill --- esphome/core/string_ref.h | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index 89ea9dd797..d502c4d27f 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -81,19 +81,6 @@ class StringRef { operator std::string() const { return str(); } - /// Compare with a null-terminated C string (compatible with std::string::compare) - int compare(const char *s) const { - size_t s_len = std::strlen(s); - int result = std::memcmp(base_, s, std::min(len_, s_len)); - if (result != 0) - return result; - if (len_ < s_len) - return -1; - if (len_ > s_len) - return 1; - return 0; - } - /// 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 { From ec1dbd39aee49e99d627a54101d61b02da7684e2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:37:28 -0600 Subject: [PATCH 09/22] tests --- tests/unit_tests/test_automation.py | 159 ++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 tests/unit_tests/test_automation.py diff --git a/tests/unit_tests/test_automation.py b/tests/unit_tests/test_automation.py new file mode 100644 index 0000000000..b1170c74bb --- /dev/null +++ b/tests/unit_tests/test_automation.py @@ -0,0 +1,159 @@ +"""Tests for esphome.automation module.""" + +from collections.abc import Generator +from unittest.mock import patch + +import pytest + +from esphome.automation import has_deferred_actions +from esphome.util import RegistryEntry + + +def _make_registry(deferred_actions: set[str]) -> dict[str, RegistryEntry]: + """Create a mock ACTION_REGISTRY with specified deferred actions.""" + registry: dict[str, RegistryEntry] = {} + for name in deferred_actions: + registry[name] = RegistryEntry(name, lambda: None, None, None, deferred=True) + return registry + + +@pytest.fixture +def mock_registry() -> Generator[dict[str, RegistryEntry]]: + """Fixture that patches ACTION_REGISTRY with delay, wait_until, script.wait as deferred.""" + registry: dict[str, RegistryEntry] = _make_registry( + {"delay", "wait_until", "script.wait"} + ) + registry["logger.log"] = RegistryEntry( + "logger.log", lambda: None, None, None, deferred=False + ) + with patch("esphome.automation.ACTION_REGISTRY", registry): + yield registry + + +def test_has_deferred_actions_empty_list( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_deferred_actions([]) is False + + +def test_has_deferred_actions_empty_dict( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_deferred_actions({}) is False + + +def test_has_deferred_actions_non_dict_non_list( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_deferred_actions("string") is False + assert has_deferred_actions(42) is False + assert has_deferred_actions(None) is False + + +def test_has_deferred_actions_delay(mock_registry: dict[str, RegistryEntry]) -> None: + assert has_deferred_actions([{"delay": "1s"}]) is True + + +def test_has_deferred_actions_wait_until( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_deferred_actions([{"wait_until": {"condition": {}}}]) is True + + +def test_has_deferred_actions_script_wait( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_deferred_actions([{"script.wait": "script_id"}]) is True + + +def test_has_deferred_actions_non_deferred( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert has_deferred_actions([{"logger.log": "hello"}]) is False + + +def test_has_deferred_actions_unknown(mock_registry: dict[str, RegistryEntry]) -> None: + assert has_deferred_actions([{"unknown.action": "value"}]) is False + + +def test_has_deferred_actions_nested_in_then( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Deferred action nested inside a non-deferred action's then block.""" + actions: list[dict[str, object]] = [ + { + "logger.log": "first", + "then": [{"delay": "1s"}], + } + ] + assert has_deferred_actions(actions) is True + + +def test_has_deferred_actions_deeply_nested( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Deferred action deeply nested in action structure.""" + actions: list[dict[str, object]] = [ + { + "if": { + "then": [ + {"logger.log": "hello"}, + {"delay": "500ms"}, + ] + } + } + ] + assert has_deferred_actions(actions) is True + + +def test_has_deferred_actions_no_deferred_in_nested( + mock_registry: dict[str, RegistryEntry], +) -> None: + """No deferred actions even with nesting.""" + actions: list[dict[str, object]] = [ + { + "if": { + "then": [ + {"logger.log": "hello"}, + ] + } + } + ] + assert has_deferred_actions(actions) is False + + +def test_has_deferred_actions_multiple_one_deferred( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert ( + has_deferred_actions( + [ + {"logger.log": "first"}, + {"delay": "1s"}, + {"logger.log": "second"}, + ] + ) + is True + ) + + +def test_has_deferred_actions_multiple_none_deferred( + mock_registry: dict[str, RegistryEntry], +) -> None: + assert ( + has_deferred_actions( + [ + {"logger.log": "first"}, + {"logger.log": "second"}, + ] + ) + is False + ) + + +def test_has_deferred_actions_dict_input( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Direct dict input (single action).""" + assert has_deferred_actions({"delay": "1s"}) is True + assert has_deferred_actions({"logger.log": "hello"}) is False From a4d19cdf57b84e0028a2970c4bc7f77a546ec2db Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:39:23 -0600 Subject: [PATCH 10/22] tweaks --- esphome/components/api/api_connection.cpp | 8 ++++---- esphome/components/api/api_frame_helper_noise.cpp | 5 +++-- esphome/core/string_ref.h | 5 +++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 5ee64c44f1..9714f8bd60 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1706,7 +1706,7 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes // 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_. if (!msg.state.empty()) { - const_cast(msg.state.c_str())[msg.state.size()] = '\0'; + msg.state.null_terminate_in_place(); } for (auto &it : this->parent_->get_state_subs()) { @@ -1714,8 +1714,8 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes continue; } - // Compare attribute: either both have matching attribute, or both have none - // it.attribute can be nullptr (meaning no attribute filter) + // 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; } @@ -1731,7 +1731,7 @@ void APIConnection::on_execute_service_request(const ExecuteServiceRequest &msg) // and frame helpers reserve RX_BUF_NULL_TERMINATOR extra byte for the last field. for (auto &arg : msg.args) { if (!arg.string_.empty()) { - const_cast(arg.string_.c_str())[arg.string_.size()] = '\0'; + arg.string_.null_terminate_in_place(); } } bool found = false; diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 5a16b8018a..5cd3cbc307 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -411,8 +411,9 @@ APIError APINoiseFrameHelper::read_packet(ReadPacketBuffer *buffer) { NoiseBuffer mbuf; noise_buffer_init(mbuf); - // rx_buf_ has RX_BUF_NULL_TERMINATOR extra byte for null termination, - // but only the actual message bytes contain encrypted data + // 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); diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index d502c4d27f..60e5fc76b1 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -81,6 +81,11 @@ class StringRef { operator std::string() const { return str(); } + /// Write a null terminator at base_[len_] in-place. + /// Caller must guarantee that the byte at base_[len_] is writable memory + /// (e.g., the RX_BUF_NULL_TERMINATOR byte reserved by frame helpers after decode). + void null_terminate_in_place() const { const_cast(base_)[len_] = '\0'; } + /// 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 { From b85878e1eec7f2250f5db7c3110113f89f2e035a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:42:12 -0600 Subject: [PATCH 11/22] tweaks --- esphome/automation.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/esphome/automation.py b/esphome/automation.py index c937470eee..fad344fe1f 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -64,6 +64,13 @@ def register_action( *, deferred: bool = False, ): + """Register an action type. + + Set ``deferred=True`` if this action stores trigger arguments for later + execution (e.g. delay, wait_until, script.wait). This tells the code + generator to use owning types (std::string) instead of non-owning views + (StringRef) for string arguments, preventing dangling references. + """ return ACTION_REGISTRY.register(name, action_type, schema, deferred=deferred) From fa343aa1ba3b8fdf4e04e70c4a5f27bfa0825be8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:49:49 -0600 Subject: [PATCH 12/22] tweaks --- esphome/automation.py | 23 +++++++++++++++-------- esphome/components/ble_client/__init__.py | 12 +++++++++--- esphome/components/script/__init__.py | 3 ++- esphome/util.py | 4 ++-- tests/unit_tests/test_automation.py | 22 +++++++++++++++++++--- 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/esphome/automation.py b/esphome/automation.py index fad344fe1f..366d23e882 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -62,14 +62,16 @@ def register_action( action_type: MockObjClass, schema: cv.Schema, *, - deferred: bool = False, + deferred: bool = True, ): """Register an action type. - Set ``deferred=True`` if this action stores trigger arguments for later - execution (e.g. delay, wait_until, script.wait). This tells the code - generator to use owning types (std::string) instead of non-owning views - (StringRef) for string arguments, preventing dangling references. + Actions default to ``deferred=True`` (safe default), meaning string + arguments use owning std::string to prevent dangling references. + + Set ``deferred=False`` 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, deferred=deferred) @@ -351,7 +353,6 @@ async def component_is_idle_condition_to_code( "delay", DelayAction, cv.templatable(cv.positive_time_period_milliseconds), - deferred=True, ) async def delay_action_to_code( config: ConfigType, @@ -382,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), ), + deferred=False, ) async def if_action_to_code( config: ConfigType, @@ -410,6 +412,7 @@ async def if_action_to_code( cv.Required(CONF_THEN): validate_action_list, } ), + deferred=False, ) async def while_action_to_code( config: ConfigType, @@ -433,6 +436,7 @@ async def while_action_to_code( cv.Required(CONF_THEN): validate_action_list, } ), + deferred=False, ) async def repeat_action_to_code( config: ConfigType, @@ -461,7 +465,7 @@ _validate_wait_until = cv.maybe_simple_value( ) -@register_action("wait_until", WaitUntilAction, _validate_wait_until, deferred=True) +@register_action("wait_until", WaitUntilAction, _validate_wait_until) async def wait_until_action_to_code( config: ConfigType, action_id: ID, @@ -477,7 +481,7 @@ async def wait_until_action_to_code( return var -@register_action("lambda", LambdaAction, cv.lambda_) +@register_action("lambda", LambdaAction, cv.lambda_, deferred=False) async def lambda_action_to_code( config: ConfigType, action_id: ID, @@ -496,6 +500,7 @@ async def lambda_action_to_code( cv.Required(CONF_ID): cv.use_id(cg.PollingComponent), } ), + deferred=False, ) async def component_update_action_to_code( config: ConfigType, @@ -515,6 +520,7 @@ async def component_update_action_to_code( cv.Required(CONF_ID): cv.use_id(cg.PollingComponent), } ), + deferred=False, ) async def component_suspend_action_to_code( config: ConfigType, @@ -537,6 +543,7 @@ async def component_suspend_action_to_code( ), } ), + deferred=False, ) async def component_resume_action_to_code( config: ConfigType, diff --git a/esphome/components/ble_client/__init__.py b/esphome/components/ble_client/__init__.py index 37db181584..e7e96b6da5 100644 --- a/esphome/components/ble_client/__init__.py +++ b/esphome/components/ble_client/__init__.py @@ -172,7 +172,9 @@ BLE_REMOVE_BOND_ACTION_SCHEMA = cv.Schema( @automation.register_action( - "ble_client.disconnect", BLEDisconnectAction, BLE_CONNECT_ACTION_SCHEMA + "ble_client.disconnect", + BLEDisconnectAction, + BLE_CONNECT_ACTION_SCHEMA, ) async def ble_disconnect_to_code(config, action_id, template_arg, args): parent = await cg.get_variable(config[CONF_ID]) @@ -180,7 +182,9 @@ async def ble_disconnect_to_code(config, action_id, template_arg, args): @automation.register_action( - "ble_client.connect", BLEConnectAction, BLE_CONNECT_ACTION_SCHEMA + "ble_client.connect", + BLEConnectAction, + BLE_CONNECT_ACTION_SCHEMA, ) async def ble_connect_to_code(config, action_id, template_arg, args): parent = await cg.get_variable(config[CONF_ID]) @@ -188,7 +192,9 @@ async def ble_connect_to_code(config, action_id, template_arg, args): @automation.register_action( - "ble_client.ble_write", BLEWriteAction, BLE_WRITE_ACTION_SCHEMA + "ble_client.ble_write", + BLEWriteAction, + BLE_WRITE_ACTION_SCHEMA, ) async def ble_write_to_code(config, action_id, template_arg, args): parent = await cg.get_variable(config[CONF_ID]) diff --git a/esphome/components/script/__init__.py b/esphome/components/script/__init__.py index 0a9e289511..4ff3d27ceb 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), }, ), + deferred=False, ) 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)}), + deferred=False, ) 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]) @@ -219,7 +221,6 @@ async def script_stop_action_to_code(config, action_id, template_arg, args): "script.wait", ScriptWaitAction, maybe_simple_id({cv.Required(CONF_ID): cv.use_id(Script)}), - deferred=True, ) async def script_wait_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/util.py b/esphome/util.py index 9fb7ef6227..c4d82fbd92 100644 --- a/esphome/util.py +++ b/esphome/util.py @@ -25,7 +25,7 @@ class RegistryEntry: type_id: "MockObjClass", schema: "Schema", *, - deferred: bool = False, + deferred: bool = True, ): self.name = name self.fun = fun @@ -58,7 +58,7 @@ class Registry(dict[str, RegistryEntry]): type_id: "MockObjClass", schema: "Schema", *, - deferred: bool = False, + deferred: bool = True, ): def decorator(fun: Callable[..., Any]): self[name] = RegistryEntry(name, fun, type_id, schema, deferred=deferred) diff --git a/tests/unit_tests/test_automation.py b/tests/unit_tests/test_automation.py index b1170c74bb..98e0b1eea7 100644 --- a/tests/unit_tests/test_automation.py +++ b/tests/unit_tests/test_automation.py @@ -10,10 +10,13 @@ from esphome.util import RegistryEntry def _make_registry(deferred_actions: set[str]) -> dict[str, RegistryEntry]: - """Create a mock ACTION_REGISTRY with specified deferred actions.""" + """Create a mock ACTION_REGISTRY with specified deferred actions. + + Uses the default deferred=True, matching the real registry behavior. + """ registry: dict[str, RegistryEntry] = {} for name in deferred_actions: - registry[name] = RegistryEntry(name, lambda: None, None, None, deferred=True) + registry[name] = RegistryEntry(name, lambda: None, None, None) return registry @@ -72,10 +75,23 @@ def test_has_deferred_actions_non_deferred( assert has_deferred_actions([{"logger.log": "hello"}]) is False -def test_has_deferred_actions_unknown(mock_registry: dict[str, RegistryEntry]) -> None: +def test_has_deferred_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_deferred_actions([{"unknown.action": "value"}]) is False +def test_has_deferred_actions_default_deferred( + mock_registry: dict[str, RegistryEntry], +) -> None: + """Actions registered without explicit deferred=False default to deferred=True.""" + mock_registry["some.action"] = RegistryEntry( + "some.action", lambda: None, None, None + ) + assert has_deferred_actions([{"some.action": "value"}]) is True + + def test_has_deferred_actions_nested_in_then( mock_registry: dict[str, RegistryEntry], ) -> None: From 23a57b922419d49728e53c55b186921433573ff4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:53:08 -0600 Subject: [PATCH 13/22] revert format change --- esphome/components/ble_client/__init__.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/esphome/components/ble_client/__init__.py b/esphome/components/ble_client/__init__.py index e7e96b6da5..37db181584 100644 --- a/esphome/components/ble_client/__init__.py +++ b/esphome/components/ble_client/__init__.py @@ -172,9 +172,7 @@ BLE_REMOVE_BOND_ACTION_SCHEMA = cv.Schema( @automation.register_action( - "ble_client.disconnect", - BLEDisconnectAction, - BLE_CONNECT_ACTION_SCHEMA, + "ble_client.disconnect", BLEDisconnectAction, BLE_CONNECT_ACTION_SCHEMA ) async def ble_disconnect_to_code(config, action_id, template_arg, args): parent = await cg.get_variable(config[CONF_ID]) @@ -182,9 +180,7 @@ async def ble_disconnect_to_code(config, action_id, template_arg, args): @automation.register_action( - "ble_client.connect", - BLEConnectAction, - BLE_CONNECT_ACTION_SCHEMA, + "ble_client.connect", BLEConnectAction, BLE_CONNECT_ACTION_SCHEMA ) async def ble_connect_to_code(config, action_id, template_arg, args): parent = await cg.get_variable(config[CONF_ID]) @@ -192,9 +188,7 @@ async def ble_connect_to_code(config, action_id, template_arg, args): @automation.register_action( - "ble_client.ble_write", - BLEWriteAction, - BLE_WRITE_ACTION_SCHEMA, + "ble_client.ble_write", BLEWriteAction, BLE_WRITE_ACTION_SCHEMA ) async def ble_write_to_code(config, action_id, template_arg, args): parent = await cg.get_variable(config[CONF_ID]) From 8bf757d9897aba7bf3343a7b9660d3b8cdce67da Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 20:59:09 -0600 Subject: [PATCH 14/22] deferred --- esphome/components/api/__init__.py | 4 ++++ 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/select/__init__.py | 7 +++++++ esphome/components/switch/__init__.py | 14 ++++++++++---- esphome/components/text/__init__.py | 1 + 13 files changed, 75 insertions(+), 21 deletions(-) diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 7df23ae1ba..1e0e0a7e4f 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -516,11 +516,13 @@ HOMEASSISTANT_ACTION_ACTION_SCHEMA = cv.All( "homeassistant.action", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, + deferred=False, ) @automation.register_action( "homeassistant.service", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, + deferred=False, ) async def homeassistant_service_to_code( config: ConfigType, @@ -611,6 +613,7 @@ HOMEASSISTANT_EVENT_ACTION_SCHEMA = cv.Schema( "homeassistant.event", HomeAssistantServiceCallAction, HOMEASSISTANT_EVENT_ACTION_SCHEMA, + deferred=False, ) async def homeassistant_event_to_code(config, action_id, template_arg, args): cg.add_define("USE_API_HOMEASSISTANT_SERVICES") @@ -651,6 +654,7 @@ HOMEASSISTANT_TAG_SCANNED_ACTION_SCHEMA = cv.maybe_simple_value( "homeassistant.tag_scanned", HomeAssistantServiceCallAction, HOMEASSISTANT_TAG_SCANNED_ACTION_SCHEMA, + deferred=False, ) 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/button/__init__.py b/esphome/components/button/__init__.py index d2f143b97e..38e05380d0 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, deferred=False +) 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..61bb3df129 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, deferred=False +) 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, deferred=False +) 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, deferred=False +) 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, deferred=False +) 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, deferred=False +) 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..e90c179821 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, deferred=False +) 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, deferred=False +) 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): ), } ), + deferred=False, ) 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..c2eaf35a85 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), } ), + deferred=False, ) 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..fb90f5d33c 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -51,6 +51,7 @@ from .types import ( ), } ), + deferred=False, ) 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, deferred=False ) @automation.register_action( - "light.turn_on", LightControlAction, LIGHT_TURN_ON_ACTION_SCHEMA + "light.turn_on", LightControlAction, LIGHT_TURN_ON_ACTION_SCHEMA, deferred=False ) @automation.register_action( - "light.control", LightControlAction, LIGHT_CONTROL_ACTION_SCHEMA + "light.control", LightControlAction, LIGHT_CONTROL_ACTION_SCHEMA, deferred=False ) 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, + deferred=False, ) 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 c8f3c52911..784c3bc0d7 100644 --- a/esphome/components/logger/__init__.py +++ b/esphome/components/logger/__init__.py @@ -518,7 +518,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, deferred=False +) 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..7c44313731 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, deferred=False ) 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, + deferred=False, ) 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 b23da7799f..a4942f2003 100644 --- a/esphome/components/number/__init__.py +++ b/esphome/components/number/__init__.py @@ -347,6 +347,7 @@ OPERATION_BASE_SCHEMA = cv.Schema( cv.Required(CONF_VALUE): cv.templatable(cv.float_), } ), + deferred=False, ) async def number_set_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -369,6 +370,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + deferred=False, ) @automation.register_action( "number.decrement", @@ -383,6 +385,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + deferred=False, ) @automation.register_action( "number.to_min", @@ -396,6 +399,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + deferred=False, ) @automation.register_action( "number.to_max", @@ -409,6 +413,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), + deferred=False, ) @automation.register_action( "number.operation", @@ -421,6 +426,7 @@ async def number_set_to_code(config, action_id, template_arg, args): cv.Optional(CONF_CYCLE, default=True): cv.templatable(cv.boolean), } ), + deferred=False, ) 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..f2f287f149 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, deferred=False +) 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, deferred=False ) 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), } ), + deferred=False, ) 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/select/__init__.py b/esphome/components/select/__init__.py index 84ad591ba1..31c3702919 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), } ), + deferred=False, ) 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), } ), + deferred=False, ) 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), } ), + deferred=False, ) @automation.register_action( "select.next", @@ -229,6 +232,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + deferred=False, ) @automation.register_action( "select.previous", @@ -243,6 +247,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + deferred=False, ) @automation.register_action( "select.first", @@ -254,6 +259,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + deferred=False, ) @automation.register_action( "select.last", @@ -265,6 +271,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), + deferred=False, ) 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 7424d7c92f..9a47a33c42 100644 --- a/esphome/components/switch/__init__.py +++ b/esphome/components/switch/__init__.py @@ -198,7 +198,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, deferred=False ) async def switch_control_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -208,9 +208,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, deferred=False +) +@automation.register_action( + "switch.turn_off", TurnOffAction, SWITCH_ACTION_SCHEMA, deferred=False +) +@automation.register_action( + "switch.turn_on", TurnOnAction, SWITCH_ACTION_SCHEMA, deferred=False +) 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..2d87ad92ea 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), } ), + deferred=False, ) async def text_set_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) From 38058d0308b8630be42cebef552e594872bff1d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:07:21 -0600 Subject: [PATCH 15/22] deferred --- esphome/automation.py | 31 +++++++++++++------------- esphome/components/api/__init__.py | 18 ++++++++------- esphome/components/button/__init__.py | 2 +- esphome/components/cover/__init__.py | 10 ++++----- esphome/components/fan/__init__.py | 6 ++--- esphome/components/globals/__init__.py | 2 +- esphome/components/light/automation.py | 10 ++++----- esphome/components/logger/__init__.py | 2 +- esphome/components/mqtt/__init__.py | 4 ++-- esphome/components/number/__init__.py | 12 +++++----- esphome/components/output/__init__.py | 6 ++--- esphome/components/script/__init__.py | 4 ++-- esphome/components/select/__init__.py | 14 ++++++------ esphome/components/switch/__init__.py | 8 +++---- esphome/components/text/__init__.py | 2 +- esphome/util.py | 10 +++++---- tests/unit_tests/test_automation.py | 16 ++++++------- 17 files changed, 81 insertions(+), 76 deletions(-) diff --git a/esphome/automation.py b/esphome/automation.py index 366d23e882..14a716cdff 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -62,18 +62,18 @@ def register_action( action_type: MockObjClass, schema: cv.Schema, *, - deferred: bool = True, + synchronous: bool = False, ): """Register an action type. - Actions default to ``deferred=True`` (safe default), meaning string + Actions default to ``synchronous=False`` (safe default), meaning string arguments use owning std::string to prevent dangling references. - Set ``deferred=False`` only for actions that complete synchronously + 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, deferred=deferred) + return ACTION_REGISTRY.register(name, action_type, schema, synchronous=synchronous) def register_condition(name: str, condition_type: MockObjClass, schema: cv.Schema): @@ -383,7 +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), ), - deferred=False, + synchronous=True, ) async def if_action_to_code( config: ConfigType, @@ -412,7 +412,7 @@ async def if_action_to_code( cv.Required(CONF_THEN): validate_action_list, } ), - deferred=False, + synchronous=True, ) async def while_action_to_code( config: ConfigType, @@ -436,7 +436,7 @@ async def while_action_to_code( cv.Required(CONF_THEN): validate_action_list, } ), - deferred=False, + synchronous=True, ) async def repeat_action_to_code( config: ConfigType, @@ -481,7 +481,7 @@ async def wait_until_action_to_code( return var -@register_action("lambda", LambdaAction, cv.lambda_, deferred=False) +@register_action("lambda", LambdaAction, cv.lambda_, synchronous=True) async def lambda_action_to_code( config: ConfigType, action_id: ID, @@ -500,7 +500,7 @@ async def lambda_action_to_code( cv.Required(CONF_ID): cv.use_id(cg.PollingComponent), } ), - deferred=False, + synchronous=True, ) async def component_update_action_to_code( config: ConfigType, @@ -520,7 +520,7 @@ async def component_update_action_to_code( cv.Required(CONF_ID): cv.use_id(cg.PollingComponent), } ), - deferred=False, + synchronous=True, ) async def component_suspend_action_to_code( config: ConfigType, @@ -543,7 +543,7 @@ async def component_suspend_action_to_code( ), } ), - deferred=False, + synchronous=True, ) async def component_resume_action_to_code( config: ConfigType, @@ -602,16 +602,17 @@ async def build_condition_list( def has_deferred_actions(actions: ConfigType) -> bool: - """Check if a validated action list contains any deferred actions. + """Check if a validated action list contains any non-synchronous actions. - Deferred actions (delay, wait_until, script.wait) store trigger args - for later execution, making non-owning types like StringRef unsafe. + 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_deferred_actions(item) for item in actions) if isinstance(actions, dict): for key in actions: - if key in ACTION_REGISTRY and ACTION_REGISTRY[key].deferred: + if key in ACTION_REGISTRY and not ACTION_REGISTRY[key].synchronous: return True return any( has_deferred_actions(v) diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 1e0e0a7e4f..8d92d219e8 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -380,15 +380,17 @@ async def to_code(config: ConfigType) -> None: if is_optional: func_args.append((cg.bool_, "return_response")) - # Check if action chain has deferred actions that would make + # Check if action chain has non-synchronous actions that would make # non-owning StringRef dangle (rx_buf_ reused after delay) - has_deferred = automation.has_deferred_actions(conf.get(CONF_THEN, [])) + has_non_synchronous = automation.has_deferred_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 deferred actions exist - if has_deferred and native is cg.StringRef: + # 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)) @@ -516,13 +518,13 @@ HOMEASSISTANT_ACTION_ACTION_SCHEMA = cv.All( "homeassistant.action", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, - deferred=False, + synchronous=True, ) @automation.register_action( "homeassistant.service", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, - deferred=False, + synchronous=True, ) async def homeassistant_service_to_code( config: ConfigType, @@ -613,7 +615,7 @@ HOMEASSISTANT_EVENT_ACTION_SCHEMA = cv.Schema( "homeassistant.event", HomeAssistantServiceCallAction, HOMEASSISTANT_EVENT_ACTION_SCHEMA, - deferred=False, + synchronous=True, ) async def homeassistant_event_to_code(config, action_id, template_arg, args): cg.add_define("USE_API_HOMEASSISTANT_SERVICES") @@ -654,7 +656,7 @@ HOMEASSISTANT_TAG_SCANNED_ACTION_SCHEMA = cv.maybe_simple_value( "homeassistant.tag_scanned", HomeAssistantServiceCallAction, HOMEASSISTANT_TAG_SCANNED_ACTION_SCHEMA, - deferred=False, + 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/button/__init__.py b/esphome/components/button/__init__.py index 38e05380d0..94816a0974 100644 --- a/esphome/components/button/__init__.py +++ b/esphome/components/button/__init__.py @@ -124,7 +124,7 @@ BUTTON_PRESS_SCHEMA = maybe_simple_id( @automation.register_action( - "button.press", PressAction, BUTTON_PRESS_SCHEMA, deferred=False + "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]) diff --git a/esphome/components/cover/__init__.py b/esphome/components/cover/__init__.py index 61bb3df129..17095f41f6 100644 --- a/esphome/components/cover/__init__.py +++ b/esphome/components/cover/__init__.py @@ -249,7 +249,7 @@ COVER_ACTION_SCHEMA = maybe_simple_id( @automation.register_action( - "cover.open", OpenAction, COVER_ACTION_SCHEMA, deferred=False + "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]) @@ -257,7 +257,7 @@ async def cover_open_to_code(config, action_id, template_arg, args): @automation.register_action( - "cover.close", CloseAction, COVER_ACTION_SCHEMA, deferred=False + "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]) @@ -265,7 +265,7 @@ async def cover_close_to_code(config, action_id, template_arg, args): @automation.register_action( - "cover.stop", StopAction, COVER_ACTION_SCHEMA, deferred=False + "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]) @@ -273,7 +273,7 @@ async def cover_stop_to_code(config, action_id, template_arg, args): @automation.register_action( - "cover.toggle", ToggleAction, COVER_ACTION_SCHEMA, deferred=False + "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]) @@ -292,7 +292,7 @@ COVER_CONTROL_ACTION_SCHEMA = cv.Schema( @automation.register_action( - "cover.control", ControlAction, COVER_CONTROL_ACTION_SCHEMA, deferred=False + "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]) diff --git a/esphome/components/fan/__init__.py b/esphome/components/fan/__init__.py index e90c179821..e839df6aee 100644 --- a/esphome/components/fan/__init__.py +++ b/esphome/components/fan/__init__.py @@ -312,7 +312,7 @@ FAN_ACTION_SCHEMA = maybe_simple_id( @automation.register_action( - "fan.toggle", ToggleAction, FAN_ACTION_SCHEMA, deferred=False + "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]) @@ -320,7 +320,7 @@ async def fan_toggle_to_code(config, action_id, template_arg, args): @automation.register_action( - "fan.turn_off", TurnOffAction, FAN_ACTION_SCHEMA, deferred=False + "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]) @@ -340,7 +340,7 @@ async def fan_turn_off_to_code(config, action_id, template_arg, args): ), } ), - deferred=False, + 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 c2eaf35a85..fe11a93a4b 100644 --- a/esphome/components/globals/__init__.py +++ b/esphome/components/globals/__init__.py @@ -102,7 +102,7 @@ async def to_code(config): cv.Required(CONF_VALUE): cv.templatable(cv.string_strict), } ), - deferred=False, + 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 fb90f5d33c..89b2fc0fb2 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -51,7 +51,7 @@ from .types import ( ), } ), - deferred=False, + synchronous=True, ) async def light_toggle_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -112,13 +112,13 @@ LIGHT_TURN_ON_ACTION_SCHEMA = automation.maybe_simple_id( @automation.register_action( - "light.turn_off", LightControlAction, LIGHT_TURN_OFF_ACTION_SCHEMA, deferred=False + "light.turn_off", LightControlAction, LIGHT_TURN_OFF_ACTION_SCHEMA, synchronous=True ) @automation.register_action( - "light.turn_on", LightControlAction, LIGHT_TURN_ON_ACTION_SCHEMA, deferred=False + "light.turn_on", LightControlAction, LIGHT_TURN_ON_ACTION_SCHEMA, synchronous=True ) @automation.register_action( - "light.control", LightControlAction, LIGHT_CONTROL_ACTION_SCHEMA, deferred=False + "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]) @@ -197,7 +197,7 @@ LIGHT_DIM_RELATIVE_ACTION_SCHEMA = cv.Schema( "light.dim_relative", DimRelativeAction, LIGHT_DIM_RELATIVE_ACTION_SCHEMA, - deferred=False, + 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 784c3bc0d7..1425f022d2 100644 --- a/esphome/components/logger/__init__.py +++ b/esphome/components/logger/__init__.py @@ -519,7 +519,7 @@ LOGGER_LOG_ACTION_SCHEMA = cv.All( @automation.register_action( - CONF_LOGGER_LOG, LambdaAction, LOGGER_LOG_ACTION_SCHEMA, deferred=False + 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]] diff --git a/esphome/components/mqtt/__init__.py b/esphome/components/mqtt/__init__.py index 7c44313731..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, deferred=False + "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]) @@ -524,7 +524,7 @@ MQTT_PUBLISH_JSON_ACTION_SCHEMA = cv.Schema( "mqtt.publish_json", MQTTPublishJsonAction, MQTT_PUBLISH_JSON_ACTION_SCHEMA, - deferred=False, + 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 a4942f2003..4a51ebcd40 100644 --- a/esphome/components/number/__init__.py +++ b/esphome/components/number/__init__.py @@ -347,7 +347,7 @@ OPERATION_BASE_SCHEMA = cv.Schema( cv.Required(CONF_VALUE): cv.templatable(cv.float_), } ), - deferred=False, + synchronous=True, ) async def number_set_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -370,7 +370,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), - deferred=False, + synchronous=True, ) @automation.register_action( "number.decrement", @@ -385,7 +385,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), - deferred=False, + synchronous=True, ) @automation.register_action( "number.to_min", @@ -399,7 +399,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), - deferred=False, + synchronous=True, ) @automation.register_action( "number.to_max", @@ -413,7 +413,7 @@ async def number_set_to_code(config, action_id, template_arg, args): } ) ), - deferred=False, + synchronous=True, ) @automation.register_action( "number.operation", @@ -426,7 +426,7 @@ async def number_set_to_code(config, action_id, template_arg, args): cv.Optional(CONF_CYCLE, default=True): cv.templatable(cv.boolean), } ), - deferred=False, + 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 f2f287f149..a4c960927b 100644 --- a/esphome/components/output/__init__.py +++ b/esphome/components/output/__init__.py @@ -75,7 +75,7 @@ BINARY_OUTPUT_ACTION_SCHEMA = maybe_simple_id( @automation.register_action( - "output.turn_on", TurnOnAction, BINARY_OUTPUT_ACTION_SCHEMA, deferred=False + "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]) @@ -83,7 +83,7 @@ async def output_turn_on_to_code(config, action_id, template_arg, args): @automation.register_action( - "output.turn_off", TurnOffAction, BINARY_OUTPUT_ACTION_SCHEMA, deferred=False + "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]) @@ -99,7 +99,7 @@ async def output_turn_off_to_code(config, action_id, template_arg, args): cv.Required(CONF_LEVEL): cv.templatable(cv.percentage), } ), - deferred=False, + 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 4ff3d27ceb..369cefad91 100644 --- a/esphome/components/script/__init__.py +++ b/esphome/components/script/__init__.py @@ -160,7 +160,7 @@ async def to_code(config): cv.Optional(validate_parameter_name): cv.templatable(cv.valid), }, ), - deferred=False, + synchronous=True, ) async def script_execute_action_to_code(config, action_id, template_arg, args): def convert(type: str): @@ -209,7 +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)}), - deferred=False, + 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 31c3702919..c114b140a9 100644 --- a/esphome/components/select/__init__.py +++ b/esphome/components/select/__init__.py @@ -145,7 +145,7 @@ OPERATION_BASE_SCHEMA = cv.Schema( cv.Required(CONF_OPTION): cv.templatable(cv.string_strict), } ), - deferred=False, + synchronous=True, ) async def select_set_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -163,7 +163,7 @@ async def select_set_to_code(config, action_id, template_arg, args): cv.Required(CONF_INDEX): cv.templatable(cv.positive_int), } ), - deferred=False, + synchronous=True, ) async def select_set_index_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) @@ -219,7 +219,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): cv.Optional(CONF_CYCLE, default=True): cv.templatable(cv.boolean), } ), - deferred=False, + synchronous=True, ) @automation.register_action( "select.next", @@ -232,7 +232,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), - deferred=False, + synchronous=True, ) @automation.register_action( "select.previous", @@ -247,7 +247,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), - deferred=False, + synchronous=True, ) @automation.register_action( "select.first", @@ -259,7 +259,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), - deferred=False, + synchronous=True, ) @automation.register_action( "select.last", @@ -271,7 +271,7 @@ async def select_is_to_code(config, condition_id, template_arg, args): } ) ), - deferred=False, + 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 9a47a33c42..9c39160e53 100644 --- a/esphome/components/switch/__init__.py +++ b/esphome/components/switch/__init__.py @@ -198,7 +198,7 @@ SWITCH_CONTROL_ACTION_SCHEMA = automation.maybe_simple_id( @automation.register_action( - "switch.control", ControlAction, SWITCH_CONTROL_ACTION_SCHEMA, deferred=False + "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]) @@ -209,13 +209,13 @@ async def switch_control_to_code(config, action_id, template_arg, args): @automation.register_action( - "switch.toggle", ToggleAction, SWITCH_ACTION_SCHEMA, deferred=False + "switch.toggle", ToggleAction, SWITCH_ACTION_SCHEMA, synchronous=True ) @automation.register_action( - "switch.turn_off", TurnOffAction, SWITCH_ACTION_SCHEMA, deferred=False + "switch.turn_off", TurnOffAction, SWITCH_ACTION_SCHEMA, synchronous=True ) @automation.register_action( - "switch.turn_on", TurnOnAction, SWITCH_ACTION_SCHEMA, deferred=False + "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]) diff --git a/esphome/components/text/__init__.py b/esphome/components/text/__init__.py index 2d87ad92ea..61f7119cad 100644 --- a/esphome/components/text/__init__.py +++ b/esphome/components/text/__init__.py @@ -164,7 +164,7 @@ OPERATION_BASE_SCHEMA = cv.Schema( cv.Required(CONF_VALUE): cv.templatable(cv.string_strict), } ), - deferred=False, + 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/util.py b/esphome/util.py index c4d82fbd92..686aa74306 100644 --- a/esphome/util.py +++ b/esphome/util.py @@ -25,13 +25,13 @@ class RegistryEntry: type_id: "MockObjClass", schema: "Schema", *, - deferred: bool = True, + synchronous: bool = False, ): self.name = name self.fun = fun self.type_id = type_id self.raw_schema = schema - self.deferred = deferred + self.synchronous = synchronous @property def coroutine_fun(self): @@ -58,10 +58,12 @@ class Registry(dict[str, RegistryEntry]): type_id: "MockObjClass", schema: "Schema", *, - deferred: bool = True, + synchronous: bool = False, ): def decorator(fun: Callable[..., Any]): - self[name] = RegistryEntry(name, fun, type_id, schema, deferred=deferred) + 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 index 98e0b1eea7..33a8134f78 100644 --- a/tests/unit_tests/test_automation.py +++ b/tests/unit_tests/test_automation.py @@ -9,25 +9,25 @@ from esphome.automation import has_deferred_actions from esphome.util import RegistryEntry -def _make_registry(deferred_actions: set[str]) -> dict[str, RegistryEntry]: - """Create a mock ACTION_REGISTRY with specified deferred actions. +def _make_registry(non_synchronous_actions: set[str]) -> dict[str, RegistryEntry]: + """Create a mock ACTION_REGISTRY with specified non-synchronous actions. - Uses the default deferred=True, matching the real registry behavior. + Uses the default synchronous=False, matching the real registry behavior. """ registry: dict[str, RegistryEntry] = {} - for name in deferred_actions: + 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 deferred.""" + """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, deferred=False + "logger.log", lambda: None, None, None, synchronous=True ) with patch("esphome.automation.ACTION_REGISTRY", registry): yield registry @@ -82,10 +82,10 @@ def test_has_deferred_actions_unknown_not_in_registry( assert has_deferred_actions([{"unknown.action": "value"}]) is False -def test_has_deferred_actions_default_deferred( +def test_has_deferred_actions_default_non_synchronous( mock_registry: dict[str, RegistryEntry], ) -> None: - """Actions registered without explicit deferred=False default to deferred=True.""" + """Actions registered without explicit synchronous=True default to non-synchronous.""" mock_registry["some.action"] = RegistryEntry( "some.action", lambda: None, None, None ) From d21d8977666695d6d3cd363747b08118055a67d4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:12:31 -0600 Subject: [PATCH 16/22] deferred --- esphome/automation.py | 6 +-- esphome/components/api/__init__.py | 2 +- esphome/core/string_ref.h | 2 + tests/unit_tests/test_automation.py | 80 +++++++++++++++-------------- 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/esphome/automation.py b/esphome/automation.py index 14a716cdff..9926043711 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -601,7 +601,7 @@ async def build_condition_list( return conditions -def has_deferred_actions(actions: ConfigType) -> bool: +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 @@ -609,13 +609,13 @@ def has_deferred_actions(actions: ConfigType) -> bool: unsafe. Actions that haven't been audited default to non-synchronous. """ if isinstance(actions, list): - return any(has_deferred_actions(item) for item in actions) + 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_deferred_actions(v) + has_non_synchronous_actions(v) for v in actions.values() if isinstance(v, (list, dict)) ) diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 8d92d219e8..0d60ce1cab 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -382,7 +382,7 @@ async def to_code(config: ConfigType) -> None: # 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_deferred_actions( + has_non_synchronous = automation.has_non_synchronous_actions( conf.get(CONF_THEN, []) ) diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index 60e5fc76b1..3a66f3d9d5 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -84,6 +84,8 @@ class StringRef { /// Write a null terminator at base_[len_] in-place. /// Caller must guarantee that the byte at base_[len_] is writable memory /// (e.g., the RX_BUF_NULL_TERMINATOR byte reserved by frame helpers after decode). + /// Marked const because StringRef itself is not modified; the underlying buffer + /// (owned by frame helper rx_buf_) is mutated via const_cast. void null_terminate_in_place() const { const_cast(base_)[len_] = '\0'; } /// Find first occurrence of substring, returns std::string::npos if not found. diff --git a/tests/unit_tests/test_automation.py b/tests/unit_tests/test_automation.py index 33a8134f78..61fef8201d 100644 --- a/tests/unit_tests/test_automation.py +++ b/tests/unit_tests/test_automation.py @@ -5,7 +5,7 @@ from unittest.mock import patch import pytest -from esphome.automation import has_deferred_actions +from esphome.automation import has_non_synchronous_actions from esphome.util import RegistryEntry @@ -33,82 +33,84 @@ def mock_registry() -> Generator[dict[str, RegistryEntry]]: yield registry -def test_has_deferred_actions_empty_list( +def test_has_non_synchronous_actions_empty_list( mock_registry: dict[str, RegistryEntry], ) -> None: - assert has_deferred_actions([]) is False + assert has_non_synchronous_actions([]) is False -def test_has_deferred_actions_empty_dict( +def test_has_non_synchronous_actions_empty_dict( mock_registry: dict[str, RegistryEntry], ) -> None: - assert has_deferred_actions({}) is False + assert has_non_synchronous_actions({}) is False -def test_has_deferred_actions_non_dict_non_list( +def test_has_non_synchronous_actions_non_dict_non_list( mock_registry: dict[str, RegistryEntry], ) -> None: - assert has_deferred_actions("string") is False - assert has_deferred_actions(42) is False - assert has_deferred_actions(None) is False + 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_deferred_actions_delay(mock_registry: dict[str, RegistryEntry]) -> None: - assert has_deferred_actions([{"delay": "1s"}]) is True - - -def test_has_deferred_actions_wait_until( +def test_has_non_synchronous_actions_delay( mock_registry: dict[str, RegistryEntry], ) -> None: - assert has_deferred_actions([{"wait_until": {"condition": {}}}]) is True + assert has_non_synchronous_actions([{"delay": "1s"}]) is True -def test_has_deferred_actions_script_wait( +def test_has_non_synchronous_actions_wait_until( mock_registry: dict[str, RegistryEntry], ) -> None: - assert has_deferred_actions([{"script.wait": "script_id"}]) is True + assert has_non_synchronous_actions([{"wait_until": {"condition": {}}}]) is True -def test_has_deferred_actions_non_deferred( +def test_has_non_synchronous_actions_script_wait( mock_registry: dict[str, RegistryEntry], ) -> None: - assert has_deferred_actions([{"logger.log": "hello"}]) is False + assert has_non_synchronous_actions([{"script.wait": "script_id"}]) is True -def test_has_deferred_actions_unknown_not_in_registry( +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_deferred_actions([{"unknown.action": "value"}]) is False + assert has_non_synchronous_actions([{"unknown.action": "value"}]) is False -def test_has_deferred_actions_default_non_synchronous( +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_deferred_actions([{"some.action": "value"}]) is True + assert has_non_synchronous_actions([{"some.action": "value"}]) is True -def test_has_deferred_actions_nested_in_then( +def test_has_non_synchronous_actions_nested_in_then( mock_registry: dict[str, RegistryEntry], ) -> None: - """Deferred action nested inside a non-deferred action's then block.""" + """Non-synchronous action nested inside a synchronous action's then block.""" actions: list[dict[str, object]] = [ { "logger.log": "first", "then": [{"delay": "1s"}], } ] - assert has_deferred_actions(actions) is True + assert has_non_synchronous_actions(actions) is True -def test_has_deferred_actions_deeply_nested( +def test_has_non_synchronous_actions_deeply_nested( mock_registry: dict[str, RegistryEntry], ) -> None: - """Deferred action deeply nested in action structure.""" + """Non-synchronous action deeply nested in action structure.""" actions: list[dict[str, object]] = [ { "if": { @@ -119,13 +121,13 @@ def test_has_deferred_actions_deeply_nested( } } ] - assert has_deferred_actions(actions) is True + assert has_non_synchronous_actions(actions) is True -def test_has_deferred_actions_no_deferred_in_nested( +def test_has_non_synchronous_actions_none_in_nested( mock_registry: dict[str, RegistryEntry], ) -> None: - """No deferred actions even with nesting.""" + """No non-synchronous actions even with nesting.""" actions: list[dict[str, object]] = [ { "if": { @@ -135,14 +137,14 @@ def test_has_deferred_actions_no_deferred_in_nested( } } ] - assert has_deferred_actions(actions) is False + assert has_non_synchronous_actions(actions) is False -def test_has_deferred_actions_multiple_one_deferred( +def test_has_non_synchronous_actions_multiple_one_non_synchronous( mock_registry: dict[str, RegistryEntry], ) -> None: assert ( - has_deferred_actions( + has_non_synchronous_actions( [ {"logger.log": "first"}, {"delay": "1s"}, @@ -153,11 +155,11 @@ def test_has_deferred_actions_multiple_one_deferred( ) -def test_has_deferred_actions_multiple_none_deferred( +def test_has_non_synchronous_actions_multiple_all_synchronous( mock_registry: dict[str, RegistryEntry], ) -> None: assert ( - has_deferred_actions( + has_non_synchronous_actions( [ {"logger.log": "first"}, {"logger.log": "second"}, @@ -167,9 +169,9 @@ def test_has_deferred_actions_multiple_none_deferred( ) -def test_has_deferred_actions_dict_input( +def test_has_non_synchronous_actions_dict_input( mock_registry: dict[str, RegistryEntry], ) -> None: """Direct dict input (single action).""" - assert has_deferred_actions({"delay": "1s"}) is True - assert has_deferred_actions({"logger.log": "hello"}) is False + assert has_non_synchronous_actions({"delay": "1s"}) is True + assert has_non_synchronous_actions({"logger.log": "hello"}) is False From 83ccdc36cda328f5b35746922416fc9e8cdb550d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:26:31 -0600 Subject: [PATCH 17/22] deferred --- esphome/automation.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/esphome/automation.py b/esphome/automation.py index 9926043711..d9b8b2ec57 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -481,6 +481,11 @@ async def wait_until_action_to_code( return var +# 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, From 1003247a410c7ce7f47aad2e2488d7bfefe0d707 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:32:59 -0600 Subject: [PATCH 18/22] deferred --- esphome/components/api/api_frame_helper_noise.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 5cd3cbc307..48694702b1 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -411,6 +411,13 @@ APIError APINoiseFrameHelper::read_packet(ReadPacketBuffer *buffer) { NoiseBuffer mbuf; noise_buffer_init(mbuf); + // 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. @@ -582,7 +589,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) From b6e8e924165b8bc1e1dfa27528a343d46547fb68 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:34:07 -0600 Subject: [PATCH 19/22] deferred --- esphome/components/api/api_connection.cpp | 8 ++++++-- esphome/core/string_ref.h | 4 +--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 9714f8bd60..d9bb2f5fd4 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1705,8 +1705,10 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes // 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 rx_buf_ data which is mutable; the const& handler + // signature is a generated protobuf pattern, not a true immutability contract. if (!msg.state.empty()) { - msg.state.null_terminate_in_place(); + const_cast(msg.state).null_terminate_in_place(); } for (auto &it : this->parent_->get_state_subs()) { @@ -1729,7 +1731,9 @@ 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. - for (auto &arg : msg.args) { + // const_cast is safe: msg references rx_buf_ data which is mutable; 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()) { arg.string_.null_terminate_in_place(); } diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index 3a66f3d9d5..d25b59632c 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -84,9 +84,7 @@ class StringRef { /// Write a null terminator at base_[len_] in-place. /// Caller must guarantee that the byte at base_[len_] is writable memory /// (e.g., the RX_BUF_NULL_TERMINATOR byte reserved by frame helpers after decode). - /// Marked const because StringRef itself is not modified; the underlying buffer - /// (owned by frame helper rx_buf_) is mutated via const_cast. - void null_terminate_in_place() const { const_cast(base_)[len_] = '\0'; } + void null_terminate_in_place() { const_cast(base_)[len_] = '\0'; } /// Find first occurrence of substring, returns std::string::npos if not found. /// Note: Requires the underlying string to be null-terminated. From 1183aae319bfa785891d3262ac2f433ec83d9664 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:38:03 -0600 Subject: [PATCH 20/22] deferred --- esphome/components/api/api_connection.cpp | 8 ++++---- esphome/components/api/api_frame_helper_noise.cpp | 7 ++++--- esphome/core/string_ref.h | 5 ----- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index d9bb2f5fd4..e21af787cb 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1705,10 +1705,10 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes // 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 rx_buf_ data which is mutable; the const& handler + // 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).null_terminate_in_place(); + const_cast(msg.state.c_str())[msg.state.size()] = '\0'; } for (auto &it : this->parent_->get_state_subs()) { @@ -1731,11 +1731,11 @@ 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 rx_buf_ data which is mutable; the const& handler + // 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()) { - arg.string_.null_terminate_in_place(); + const_cast(arg.string_.c_str())[arg.string_.size()] = '\0'; } } bool found = false; diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 48694702b1..3ae35e9be8 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -194,18 +194,19 @@ 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 (+ 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 + (state_ == State::DATA ? RX_BUF_NULL_TERMINATOR : 0); + 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); } diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index d25b59632c..d502c4d27f 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -81,11 +81,6 @@ class StringRef { operator std::string() const { return str(); } - /// Write a null terminator at base_[len_] in-place. - /// Caller must guarantee that the byte at base_[len_] is writable memory - /// (e.g., the RX_BUF_NULL_TERMINATOR byte reserved by frame helpers after decode). - void null_terminate_in_place() { const_cast(base_)[len_] = '\0'; } - /// 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 { From e1e20422a65666819f591d21f5bbd29a0c26a5ef Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:47:03 -0600 Subject: [PATCH 21/22] deferred --- esphome/core/string_ref.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index d502c4d27f..89ea9dd797 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -81,6 +81,19 @@ class StringRef { operator std::string() const { return str(); } + /// Compare with a null-terminated C string (compatible with std::string::compare) + int compare(const char *s) const { + size_t s_len = std::strlen(s); + int result = std::memcmp(base_, s, std::min(len_, s_len)); + if (result != 0) + return result; + if (len_ < s_len) + return -1; + if (len_ > s_len) + return 1; + return 0; + } + /// 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 { From c636984e3a39ec4d42eae845b17a2cdb327df3fe Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 21 Feb 2026 21:53:57 -0600 Subject: [PATCH 22/22] deferred --- esphome/core/string_ref.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index 89ea9dd797..6047202753 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -81,18 +81,19 @@ class StringRef { operator std::string() const { return str(); } - /// Compare with a null-terminated C string (compatible with std::string::compare) - int compare(const char *s) const { - size_t s_len = std::strlen(s); - int result = std::memcmp(base_, s, std::min(len_, s_len)); + /// 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_ < s_len) + if (len_ < other.len_) return -1; - if (len_ > s_len) + 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.