From eeeae53f76b57c52af9ca442d6e9aba7ca8521db Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 11 Jan 2026 17:40:09 -1000 Subject: [PATCH 1/7] [fan] Return StringRef from get_preset_mode() for safety and modern API (#13092) --- esphome/components/api/api_connection.cpp | 2 +- esphome/components/copy/fan/copy_fan.cpp | 18 ++++--- esphome/components/copy/fan/copy_fan.h | 2 +- esphome/components/fan/automation.h | 7 ++- esphome/components/fan/fan.cpp | 45 ++++++++++++----- esphome/components/fan/fan.h | 14 ++++-- .../components/hbridge/fan/hbridge_fan.cpp | 2 +- esphome/components/speed/fan/speed_fan.cpp | 2 +- .../components/template/fan/template_fan.cpp | 2 +- tests/components/copy/common.yaml | 3 ++ tests/components/fan/common.yaml | 48 +++++++++++++++++++ 11 files changed, 113 insertions(+), 32 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index fb3548d117..4bc19a8bad 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -443,7 +443,7 @@ uint16_t APIConnection::try_send_fan_state(EntityBase *entity, APIConnection *co if (traits.supports_direction()) msg.direction = static_cast(fan->direction); if (traits.supports_preset_modes() && fan->has_preset_mode()) - msg.preset_mode = StringRef(fan->get_preset_mode()); + msg.preset_mode = fan->get_preset_mode(); return fill_and_encode_entity_state(fan, msg, FanStateResponse::MESSAGE_TYPE, conn, remaining_size, is_single); } uint16_t APIConnection::try_send_fan_info(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, diff --git a/esphome/components/copy/fan/copy_fan.cpp b/esphome/components/copy/fan/copy_fan.cpp index d35ece950b..b4a43cf2f1 100644 --- a/esphome/components/copy/fan/copy_fan.cpp +++ b/esphome/components/copy/fan/copy_fan.cpp @@ -8,20 +8,24 @@ static const char *const TAG = "copy.fan"; void CopyFan::setup() { source_->add_on_state_callback([this]() { - this->state = source_->state; - this->oscillating = source_->oscillating; - this->speed = source_->speed; - this->direction = source_->direction; - this->set_preset_mode_(source_->get_preset_mode()); + this->copy_state_from_source_(); this->publish_state(); }); + this->copy_state_from_source_(); + this->publish_state(); +} + +void CopyFan::copy_state_from_source_() { this->state = source_->state; this->oscillating = source_->oscillating; this->speed = source_->speed; this->direction = source_->direction; - this->set_preset_mode_(source_->get_preset_mode()); - this->publish_state(); + if (source_->has_preset_mode()) { + this->set_preset_mode_(source_->get_preset_mode()); + } else { + this->clear_preset_mode_(); + } } void CopyFan::dump_config() { LOG_FAN("", "Copy Fan", this); } diff --git a/esphome/components/copy/fan/copy_fan.h b/esphome/components/copy/fan/copy_fan.h index b474975bc4..988129f07b 100644 --- a/esphome/components/copy/fan/copy_fan.h +++ b/esphome/components/copy/fan/copy_fan.h @@ -16,7 +16,7 @@ class CopyFan : public fan::Fan, public Component { protected: void control(const fan::FanCall &call) override; - ; + void copy_state_from_source_(); fan::Fan *source_; }; diff --git a/esphome/components/fan/automation.h b/esphome/components/fan/automation.h index ce1db6fc64..77abc2f13f 100644 --- a/esphome/components/fan/automation.h +++ b/esphome/components/fan/automation.h @@ -212,19 +212,18 @@ class FanPresetSetTrigger : public Trigger { public: FanPresetSetTrigger(Fan *state) { state->add_on_state_callback([this, state]() { - const auto *preset_mode = state->get_preset_mode(); + auto preset_mode = state->get_preset_mode(); auto should_trigger = preset_mode != this->last_preset_mode_; this->last_preset_mode_ = preset_mode; if (should_trigger) { - // Trigger with empty string when nullptr to maintain backward compatibility - this->trigger(preset_mode != nullptr ? preset_mode : ""); + this->trigger(std::string(preset_mode)); } }); this->last_preset_mode_ = state->get_preset_mode(); } protected: - const char *last_preset_mode_{nullptr}; + StringRef last_preset_mode_{}; }; } // namespace fan diff --git a/esphome/components/fan/fan.cpp b/esphome/components/fan/fan.cpp index 0ffb60e50d..2e48d84eb9 100644 --- a/esphome/components/fan/fan.cpp +++ b/esphome/components/fan/fan.cpp @@ -61,7 +61,7 @@ void FanCall::perform() { if (this->direction_.has_value()) { ESP_LOGD(TAG, " Direction: %s", LOG_STR_ARG(fan_direction_to_string(*this->direction_))); } - if (this->has_preset_mode()) { + if (this->preset_mode_ != nullptr) { ESP_LOGD(TAG, " Preset Mode: %s", this->preset_mode_); } this->parent_.control(*this); @@ -83,7 +83,7 @@ void FanCall::validate_() { *this->binary_state_ // ..,and no preset mode will be active... && !this->has_preset_mode() && - this->parent_.get_preset_mode() == nullptr + !this->parent_.has_preset_mode() // ...and neither current nor new speed is available... && traits.supports_speed() && this->parent_.speed == 0 && !this->speed_.has_value()) { // ...set speed to 100% @@ -154,16 +154,16 @@ const char *Fan::find_preset_mode_(const char *preset_mode, size_t len) { return this->get_traits().find_preset_mode(preset_mode, len); } -bool Fan::set_preset_mode_(const char *preset_mode) { - if (preset_mode == nullptr) { - // Treat nullptr as clearing the preset mode +bool Fan::set_preset_mode_(const char *preset_mode, size_t len) { + if (preset_mode == nullptr || len == 0) { + // Treat nullptr or empty string as clearing the preset mode (no valid preset is "") if (this->preset_mode_ == nullptr) { return false; // No change } this->clear_preset_mode_(); return true; } - const char *validated = this->find_preset_mode_(preset_mode); + const char *validated = this->find_preset_mode_(preset_mode, len); if (validated == nullptr || this->preset_mode_ == validated) { return false; // Preset mode not supported or no change } @@ -171,10 +171,31 @@ bool Fan::set_preset_mode_(const char *preset_mode) { return true; } -bool Fan::set_preset_mode_(const std::string &preset_mode) { return this->set_preset_mode_(preset_mode.c_str()); } +bool Fan::set_preset_mode_(const char *preset_mode) { + return this->set_preset_mode_(preset_mode, preset_mode ? strlen(preset_mode) : 0); +} + +bool Fan::set_preset_mode_(const std::string &preset_mode) { + return this->set_preset_mode_(preset_mode.data(), preset_mode.size()); +} + +bool Fan::set_preset_mode_(StringRef preset_mode) { + // Safe: find_preset_mode_ only uses the input for comparison and returns + // a pointer from traits, so the input StringRef's lifetime doesn't matter. + return this->set_preset_mode_(preset_mode.c_str(), preset_mode.size()); +} void Fan::clear_preset_mode_() { this->preset_mode_ = nullptr; } +void Fan::apply_preset_mode_(const FanCall &call) { + if (call.has_preset_mode()) { + this->set_preset_mode_(call.get_preset_mode()); + } else if (call.get_speed().has_value()) { + // Manually setting speed clears preset (per Home Assistant convention) + this->clear_preset_mode_(); + } +} + void Fan::add_on_state_callback(std::function &&callback) { this->state_callback_.add(std::move(callback)); } void Fan::publish_state() { auto traits = this->get_traits(); @@ -192,9 +213,8 @@ void Fan::publish_state() { if (traits.supports_direction()) { ESP_LOGD(TAG, " Direction: %s", LOG_STR_ARG(fan_direction_to_string(this->direction))); } - const char *preset = this->get_preset_mode(); - if (preset != nullptr) { - ESP_LOGD(TAG, " Preset Mode: %s", preset); + if (this->preset_mode_ != nullptr) { + ESP_LOGD(TAG, " Preset Mode: %s", this->preset_mode_); } this->state_callback_.call(); #if defined(USE_FAN) && defined(USE_CONTROLLER_REGISTRY) @@ -249,12 +269,11 @@ void Fan::save_state_() { state.speed = this->speed; state.direction = this->direction; - const char *preset = this->get_preset_mode(); - if (preset != nullptr) { + if (this->has_preset_mode()) { const auto &preset_modes = traits.supported_preset_modes(); // Find index of current preset mode (pointer comparison is safe since preset is from traits) for (size_t i = 0; i < preset_modes.size(); i++) { - if (preset_modes[i] == preset) { + if (preset_modes[i] == this->preset_mode_) { state.preset_mode = i; break; } diff --git a/esphome/components/fan/fan.h b/esphome/components/fan/fan.h index 7c79fda83e..55d4ba8825 100644 --- a/esphome/components/fan/fan.h +++ b/esphome/components/fan/fan.h @@ -5,6 +5,7 @@ #include "esphome/core/log.h" #include "esphome/core/optional.h" #include "esphome/core/preferences.h" +#include "esphome/core/string_ref.h" #include "fan_traits.h" namespace esphome { @@ -128,8 +129,11 @@ class Fan : public EntityBase { /// Set the restore mode of this fan. void set_restore_mode(FanRestoreMode restore_mode) { this->restore_mode_ = restore_mode; } - /// Get the current preset mode (returns pointer to string stored in traits, or nullptr if not set) - const char *get_preset_mode() const { return this->preset_mode_; } + /// Get the current preset mode. + /// Returns a StringRef of the string stored in traits, or empty ref if not set. + /// The returned ref points to string literals from codegen (static storage). + /// Traits are set once at startup and valid for the lifetime of the program. + StringRef get_preset_mode() const { return StringRef::from_maybe_nullptr(this->preset_mode_); } /// Check if a preset mode is currently active bool has_preset_mode() const { return this->preset_mode_ != nullptr; } @@ -146,11 +150,15 @@ class Fan : public EntityBase { void dump_traits_(const char *tag, const char *prefix); /// Set the preset mode (finds and stores pointer from traits). Returns true if changed. + /// Passing nullptr or empty string clears the preset mode. + bool set_preset_mode_(const char *preset_mode, size_t len); bool set_preset_mode_(const char *preset_mode); - /// Set the preset mode (finds and stores pointer from traits). Returns true if changed. bool set_preset_mode_(const std::string &preset_mode); + bool set_preset_mode_(StringRef preset_mode); /// Clear the preset mode void clear_preset_mode_(); + /// Apply preset mode from a FanCall (handles speed-clears-preset convention) + void apply_preset_mode_(const FanCall &call); /// Find and return the matching preset mode pointer from traits, or nullptr if not found. const char *find_preset_mode_(const char *preset_mode); const char *find_preset_mode_(const char *preset_mode, size_t len); diff --git a/esphome/components/hbridge/fan/hbridge_fan.cpp b/esphome/components/hbridge/fan/hbridge_fan.cpp index 488208b725..9bf58f9d1e 100644 --- a/esphome/components/hbridge/fan/hbridge_fan.cpp +++ b/esphome/components/hbridge/fan/hbridge_fan.cpp @@ -57,7 +57,7 @@ void HBridgeFan::control(const fan::FanCall &call) { this->oscillating = *call.get_oscillating(); if (call.get_direction().has_value()) this->direction = *call.get_direction(); - this->set_preset_mode_(call.get_preset_mode()); + this->apply_preset_mode_(call); this->write_state_(); this->publish_state(); diff --git a/esphome/components/speed/fan/speed_fan.cpp b/esphome/components/speed/fan/speed_fan.cpp index 801593c2ac..af98e3a51f 100644 --- a/esphome/components/speed/fan/speed_fan.cpp +++ b/esphome/components/speed/fan/speed_fan.cpp @@ -29,7 +29,7 @@ void SpeedFan::control(const fan::FanCall &call) { this->oscillating = *call.get_oscillating(); if (call.get_direction().has_value()) this->direction = *call.get_direction(); - this->set_preset_mode_(call.get_preset_mode()); + this->apply_preset_mode_(call); this->write_state_(); this->publish_state(); diff --git a/esphome/components/template/fan/template_fan.cpp b/esphome/components/template/fan/template_fan.cpp index 384e6b0ca1..0e1920a984 100644 --- a/esphome/components/template/fan/template_fan.cpp +++ b/esphome/components/template/fan/template_fan.cpp @@ -28,7 +28,7 @@ void TemplateFan::control(const fan::FanCall &call) { this->oscillating = *call.get_oscillating(); if (call.get_direction().has_value() && this->has_direction_) this->direction = *call.get_direction(); - this->set_preset_mode_(call.get_preset_mode()); + this->apply_preset_mode_(call); this->publish_state(); } diff --git a/tests/components/copy/common.yaml b/tests/components/copy/common.yaml index a73b3467e6..a376004b2f 100644 --- a/tests/components/copy/common.yaml +++ b/tests/components/copy/common.yaml @@ -7,6 +7,9 @@ fan: - platform: speed id: fan_speed output: fan_output_1 + preset_modes: + - Eco + - Turbo - platform: copy source_id: fan_speed name: Fan Speed Copy diff --git a/tests/components/fan/common.yaml b/tests/components/fan/common.yaml index 55c2a656fd..099bbfef08 100644 --- a/tests/components/fan/common.yaml +++ b/tests/components/fan/common.yaml @@ -9,3 +9,51 @@ fan: has_oscillating: true has_direction: true speed_count: 3 + +# Test lambdas using get_preset_mode() which returns StringRef +# These examples match the migration guide in the PR description +binary_sensor: + - platform: template + id: fan_has_preset + name: "Fan Has Preset" + lambda: |- + // Migration guide: Checking if preset mode is set + // Use empty() or has_preset_mode() + if (!id(test_fan).get_preset_mode().empty()) { + // preset is set + } + if (id(test_fan).has_preset_mode()) { + // preset is set + } + + // Migration guide: Comparing preset mode + // Use == operator directly (safe, works even when empty) + if (id(test_fan).get_preset_mode() == "Eco") { + return true; + } + + // Migration guide: Checking for no preset + if (id(test_fan).get_preset_mode().empty()) { + // no preset + } + if (!id(test_fan).has_preset_mode()) { + // no preset + } + + // Migration guide: Getting as std::string + std::string preset = std::string(id(test_fan).get_preset_mode()); + + // Migration guide: Logging option 1 + // Use .c_str() - works because StringRef points to null-terminated string in traits + ESP_LOGD("test", "Preset: %s", id(test_fan).get_preset_mode().c_str()); + + // Migration guide: Logging option 2 + // Use %.*s format (safer, no null-termination assumption) + auto preset_ref = id(test_fan).get_preset_mode(); + ESP_LOGD("test", "Preset: %.*s", (int)preset_ref.size(), preset_ref.c_str()); + + // Test != comparison + if (id(test_fan).get_preset_mode() != "Sleep") { + return true; + } + return false; From 23f9f70b7187c4b8a292ccf3f0751ccbd6167c96 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 11 Jan 2026 17:40:43 -1000 Subject: [PATCH 2/7] [select] Return StringRef from current_option() (#13095) --- esphome/components/api/api_connection.cpp | 2 +- .../display_menu_base/menu_item.cpp | 3 +- esphome/components/ld2410/ld2410.cpp | 7 ++-- esphome/components/ld2412/ld2412.cpp | 7 ++-- esphome/components/ld2450/ld2450.cpp | 6 ++-- esphome/components/mqtt/mqtt_select.cpp | 3 +- .../prometheus/prometheus_handler.cpp | 3 +- esphome/components/select/select.cpp | 4 ++- esphome/components/select/select.h | 11 ++++--- esphome/components/web_server/web_server.cpp | 11 ++++--- esphome/components/web_server/web_server.h | 2 +- tests/components/template/common-base.yaml | 32 +++++++++++++++++++ 12 files changed, 68 insertions(+), 23 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 4bc19a8bad..a4df75630c 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -914,7 +914,7 @@ uint16_t APIConnection::try_send_select_state(EntityBase *entity, APIConnection bool is_single) { auto *select = static_cast(entity); SelectStateResponse resp; - resp.state = StringRef(select->current_option()); + resp.state = select->current_option(); resp.missing_state = !select->has_state(); return fill_and_encode_entity_state(select, resp, SelectStateResponse::MESSAGE_TYPE, conn, remaining_size, is_single); } diff --git a/esphome/components/display_menu_base/menu_item.cpp b/esphome/components/display_menu_base/menu_item.cpp index 08f758045e..ad8b03de60 100644 --- a/esphome/components/display_menu_base/menu_item.cpp +++ b/esphome/components/display_menu_base/menu_item.cpp @@ -42,7 +42,8 @@ std::string MenuItemSelect::get_value_text() const { result = this->value_getter_.value()(this); } else { if (this->select_var_ != nullptr) { - result = this->select_var_->current_option(); + auto option = this->select_var_->current_option(); + result.assign(option.c_str(), option.size()); } } diff --git a/esphome/components/ld2410/ld2410.cpp b/esphome/components/ld2410/ld2410.cpp index c9b4333f7e..5294f7cd36 100644 --- a/esphome/components/ld2410/ld2410.cpp +++ b/esphome/components/ld2410/ld2410.cpp @@ -442,7 +442,8 @@ bool LD2410Component::handle_ack_data_() { ESP_LOGV(TAG, "Baud rate change"); #ifdef USE_SELECT if (this->baud_rate_select_ != nullptr) { - ESP_LOGE(TAG, "Change baud rate to %s and reinstall", this->baud_rate_select_->current_option()); + auto baud = this->baud_rate_select_->current_option(); + ESP_LOGE(TAG, "Change baud rate to %.*s and reinstall", (int) baud.size(), baud.c_str()); } #endif break; @@ -766,10 +767,10 @@ void LD2410Component::set_light_out_control() { #endif #ifdef USE_SELECT if (this->light_function_select_ != nullptr && this->light_function_select_->has_state()) { - this->light_function_ = find_uint8(LIGHT_FUNCTIONS_BY_STR, this->light_function_select_->current_option()); + this->light_function_ = find_uint8(LIGHT_FUNCTIONS_BY_STR, this->light_function_select_->current_option().c_str()); } if (this->out_pin_level_select_ != nullptr && this->out_pin_level_select_->has_state()) { - this->out_pin_level_ = find_uint8(OUT_PIN_LEVELS_BY_STR, this->out_pin_level_select_->current_option()); + this->out_pin_level_ = find_uint8(OUT_PIN_LEVELS_BY_STR, this->out_pin_level_select_->current_option().c_str()); } #endif this->set_config_mode_(true); diff --git a/esphome/components/ld2412/ld2412.cpp b/esphome/components/ld2412/ld2412.cpp index 620ac9886b..c2f441e472 100644 --- a/esphome/components/ld2412/ld2412.cpp +++ b/esphome/components/ld2412/ld2412.cpp @@ -486,7 +486,8 @@ bool LD2412Component::handle_ack_data_() { ESP_LOGV(TAG, "Baud rate change"); #ifdef USE_SELECT if (this->baud_rate_select_ != nullptr) { - ESP_LOGW(TAG, "Change baud rate to %s and reinstall", this->baud_rate_select_->current_option()); + auto baud = this->baud_rate_select_->current_option(); + ESP_LOGW(TAG, "Change baud rate to %.*s and reinstall", (int) baud.size(), baud.c_str()); } #endif break; @@ -790,7 +791,7 @@ void LD2412Component::set_basic_config() { 1, TOTAL_GATES, DEFAULT_PRESENCE_TIMEOUT, 0, #endif #ifdef USE_SELECT - find_uint8(OUT_PIN_LEVELS_BY_STR, this->out_pin_level_select_->current_option()), + find_uint8(OUT_PIN_LEVELS_BY_STR, this->out_pin_level_select_->current_option().c_str()), #else 0x01, // Default value if not using select #endif @@ -844,7 +845,7 @@ void LD2412Component::set_light_out_control() { #endif #ifdef USE_SELECT if (this->light_function_select_ != nullptr && this->light_function_select_->has_state()) { - this->light_function_ = find_uint8(LIGHT_FUNCTIONS_BY_STR, this->light_function_select_->current_option()); + this->light_function_ = find_uint8(LIGHT_FUNCTIONS_BY_STR, this->light_function_select_->current_option().c_str()); } #endif uint8_t value[2] = {this->light_function_, this->light_threshold_}; diff --git a/esphome/components/ld2450/ld2450.cpp b/esphome/components/ld2450/ld2450.cpp index 3b85694bc0..58d469b2a7 100644 --- a/esphome/components/ld2450/ld2450.cpp +++ b/esphome/components/ld2450/ld2450.cpp @@ -637,7 +637,8 @@ bool LD2450Component::handle_ack_data_() { ESP_LOGV(TAG, "Baud rate change"); #ifdef USE_SELECT if (this->baud_rate_select_ != nullptr) { - ESP_LOGE(TAG, "Change baud rate to %s and reinstall", this->baud_rate_select_->current_option()); + auto baud = this->baud_rate_select_->current_option(); + ESP_LOGE(TAG, "Change baud rate to %.*s and reinstall", (int) baud.size(), baud.c_str()); } #endif break; @@ -718,7 +719,8 @@ bool LD2450Component::handle_ack_data_() { this->publish_zone_type(); #ifdef USE_SELECT if (this->zone_type_select_ != nullptr) { - ESP_LOGV(TAG, "Change zone type to: %s", this->zone_type_select_->current_option()); + auto zone = this->zone_type_select_->current_option(); + ESP_LOGV(TAG, "Change zone type to: %.*s", (int) zone.size(), zone.c_str()); } #endif if (this->buffer_data_[10] == 0x00) { diff --git a/esphome/components/mqtt/mqtt_select.cpp b/esphome/components/mqtt/mqtt_select.cpp index 09d90ed46e..03ab82312b 100644 --- a/esphome/components/mqtt/mqtt_select.cpp +++ b/esphome/components/mqtt/mqtt_select.cpp @@ -43,7 +43,8 @@ void MQTTSelectComponent::send_discovery(JsonObject root, mqtt::SendDiscoveryCon } bool MQTTSelectComponent::send_initial_state() { if (this->select_->has_state()) { - return this->publish_state(this->select_->current_option()); + auto option = this->select_->current_option(); + return this->publish_state(std::string(option.c_str(), option.size())); } else { return true; } diff --git a/esphome/components/prometheus/prometheus_handler.cpp b/esphome/components/prometheus/prometheus_handler.cpp index 88b357041a..75910fa73d 100644 --- a/esphome/components/prometheus/prometheus_handler.cpp +++ b/esphome/components/prometheus/prometheus_handler.cpp @@ -709,7 +709,8 @@ void PrometheusHandler::select_row_(AsyncResponseStream *stream, select::Select stream->print(ESPHOME_F("\",name=\"")); stream->print(relabel_name_(obj).c_str()); stream->print(ESPHOME_F("\",value=\"")); - stream->print(obj->current_option()); + // c_str() is safe as option values are null-terminated strings from codegen + stream->print(obj->current_option().c_str()); stream->print(ESPHOME_F("\"} ")); stream->print(ESPHOME_F("1.0")); stream->print(ESPHOME_F("\n")); diff --git a/esphome/components/select/select.cpp b/esphome/components/select/select.cpp index 28d7eb07d4..3d70e94d47 100644 --- a/esphome/components/select/select.cpp +++ b/esphome/components/select/select.cpp @@ -38,7 +38,9 @@ void Select::publish_state(size_t index) { #endif } -const char *Select::current_option() const { return this->has_state() ? this->option_at(this->active_index_) : ""; } +StringRef Select::current_option() const { + return this->has_state() ? StringRef(this->option_at(this->active_index_)) : StringRef(); +} void Select::add_on_state_callback(std::function &&callback) { this->state_callback_.add(std::move(callback)); diff --git a/esphome/components/select/select.h b/esphome/components/select/select.h index 330d18ce6f..8b05487704 100644 --- a/esphome/components/select/select.h +++ b/esphome/components/select/select.h @@ -3,6 +3,7 @@ #include "esphome/core/component.h" #include "esphome/core/entity_base.h" #include "esphome/core/helpers.h" +#include "esphome/core/string_ref.h" #include "select_call.h" #include "select_traits.h" @@ -33,8 +34,8 @@ class Select : public EntityBase { #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" - /// @deprecated Use current_option() instead. This member will be removed in ESPHome 2026.5.0. - ESPDEPRECATED("Use current_option() instead of .state. Will be removed in 2026.5.0", "2025.11.0") + /// @deprecated Use current_option() instead. This member will be removed in ESPHome 2026.7.0. + ESPDEPRECATED("Use current_option() instead of .state. Will be removed in 2026.7.0", "2026.1.0") std::string state{}; Select() = default; @@ -45,8 +46,10 @@ class Select : public EntityBase { void publish_state(const char *state); void publish_state(size_t index); - /// Return the currently selected option (as const char* from flash). - const char *current_option() const; + /// Return the currently selected option, or empty StringRef if no state. + /// The returned StringRef points to string literals from codegen (static storage). + /// Traits are set once at startup and valid for the lifetime of the program. + StringRef current_option() const; /// Instantiate a SelectCall object to modify this select component's state. SelectCall make_call() { return SelectCall(this); } diff --git a/esphome/components/web_server/web_server.cpp b/esphome/components/web_server/web_server.cpp index 12115083f6..41d225c0d8 100644 --- a/esphome/components/web_server/web_server.cpp +++ b/esphome/components/web_server/web_server.cpp @@ -1393,7 +1393,7 @@ void WebServer::handle_select_request(AsyncWebServerRequest *request, const UrlM if (request->method() == HTTP_GET && entity_match.action_is_empty) { auto detail = get_request_detail(request); - std::string data = this->select_json_(obj, obj->has_state() ? obj->current_option() : "", detail); + std::string data = this->select_json_(obj, obj->has_state() ? obj->current_option() : StringRef(), detail); request->send(200, "application/json", data.c_str()); return; } @@ -1414,17 +1414,18 @@ void WebServer::handle_select_request(AsyncWebServerRequest *request, const UrlM } std::string WebServer::select_state_json_generator(WebServer *web_server, void *source) { auto *obj = (select::Select *) (source); - return web_server->select_json_(obj, obj->has_state() ? obj->current_option() : "", DETAIL_STATE); + return web_server->select_json_(obj, obj->has_state() ? obj->current_option() : StringRef(), DETAIL_STATE); } std::string WebServer::select_all_json_generator(WebServer *web_server, void *source) { auto *obj = (select::Select *) (source); - return web_server->select_json_(obj, obj->has_state() ? obj->current_option() : "", DETAIL_ALL); + return web_server->select_json_(obj, obj->has_state() ? obj->current_option() : StringRef(), DETAIL_ALL); } -std::string WebServer::select_json_(select::Select *obj, const char *value, JsonDetail start_config) { +std::string WebServer::select_json_(select::Select *obj, StringRef value, JsonDetail start_config) { json::JsonBuilder builder; JsonObject root = builder.root(); - set_json_icon_state_value(root, obj, "select", value, value, start_config); + // value points to null-terminated string literals from codegen (via current_option()) + set_json_icon_state_value(root, obj, "select", value.c_str(), value.c_str(), start_config); if (start_config == DETAIL_ALL) { JsonArray opt = root[ESPHOME_F("option")].to(); for (auto &option : obj->traits.get_options()) { diff --git a/esphome/components/web_server/web_server.h b/esphome/components/web_server/web_server.h index c52cf981e0..b62686f0aa 100644 --- a/esphome/components/web_server/web_server.h +++ b/esphome/components/web_server/web_server.h @@ -627,7 +627,7 @@ class WebServer : public Controller, std::string text_json_(text::Text *obj, const std::string &value, JsonDetail start_config); #endif #ifdef USE_SELECT - std::string select_json_(select::Select *obj, const char *value, JsonDetail start_config); + std::string select_json_(select::Select *obj, StringRef value, JsonDetail start_config); #endif #ifdef USE_CLIMATE std::string climate_json_(climate::Climate *obj, JsonDetail start_config); diff --git a/tests/components/template/common-base.yaml b/tests/components/template/common-base.yaml index e050c0b307..134ad4d046 100644 --- a/tests/components/template/common-base.yaml +++ b/tests/components/template/common-base.yaml @@ -243,6 +243,7 @@ number: select: - platform: template + id: template_select name: "Template select" optimistic: true options: @@ -250,6 +251,37 @@ select: - two - three initial_option: two + # Test current_option() returning std::string_view - migration guide examples + on_value: + - lambda: |- + // Migration guide: Check if select has a state + // OLD: if (id(template_select).current_option() != nullptr) + // NEW: Check with .empty() + if (!id(template_select).current_option().empty()) { + ESP_LOGI("test", "Select has state"); + } + + // Migration guide: Compare option values + // OLD: if (strcmp(id(template_select).current_option(), "one") == 0) + // NEW: Direct comparison works safely even when empty + if (id(template_select).current_option() == "one") { + ESP_LOGI("test", "Option is 'one'"); + } + if (id(template_select).current_option() != "two") { + ESP_LOGI("test", "Option is not 'two'"); + } + + // Migration guide: Logging options + // Option 1: Using .c_str() - StringRef guarantees null-termination + ESP_LOGI("test", "Current option: %s", id(template_select).current_option().c_str()); + + // Option 2: Using %.*s format with size + auto option = id(template_select).current_option(); + ESP_LOGI("test", "Current option (safe): %.*s", (int) option.size(), option.c_str()); + + // Migration guide: Store in std::string + std::string stored_option(id(template_select).current_option()); + ESP_LOGI("test", "Stored: %s", stored_option.c_str()); lock: - platform: template From f1b11b185548d21fec2be3ee2e45b4346e7e5929 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 11 Jan 2026 17:52:39 -1000 Subject: [PATCH 3/7] [light] Return StringRef from LightEffect::get_name() and LightState::get_effect_name() (#13105) --- esphome/components/api/api_connection.cpp | 5 +- esphome/components/e131/e131.cpp | 10 ++-- .../e131/e131_addressable_light_effect.cpp | 5 +- esphome/components/light/light_call.cpp | 13 ++-- esphome/components/light/light_effect.h | 5 +- .../components/light/light_json_schema.cpp | 2 +- esphome/components/light/light_state.cpp | 10 +--- esphome/components/light/light_state.h | 10 ++-- esphome/components/mqtt/mqtt_light.cpp | 6 +- .../prometheus/prometheus_handler.cpp | 3 +- esphome/core/helpers.cpp | 3 + esphome/core/helpers.h | 2 + tests/components/light/common.yaml | 59 +++++++++++++++++++ 13 files changed, 97 insertions(+), 36 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index a4df75630c..f0fc5ba71c 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -499,7 +499,7 @@ uint16_t APIConnection::try_send_light_state(EntityBase *entity, APIConnection * resp.cold_white = values.get_cold_white(); resp.warm_white = values.get_warm_white(); if (light->supports_effects()) { - resp.effect = light->get_effect_name_ref(); + resp.effect = light->get_effect_name(); } return fill_and_encode_entity_state(light, resp, LightStateResponse::MESSAGE_TYPE, conn, remaining_size, is_single); } @@ -522,7 +522,8 @@ uint16_t APIConnection::try_send_light_info(EntityBase *entity, APIConnection *c effects_list.init(light_effects.size() + 1); effects_list.push_back("None"); for (auto *effect : light_effects) { - effects_list.push_back(effect->get_name()); + // c_str() is safe as effect names are null-terminated strings from codegen + effects_list.push_back(effect->get_name().c_str()); } } msg.effects = &effects_list; diff --git a/esphome/components/e131/e131.cpp b/esphome/components/e131/e131.cpp index c10c88faf2..f11e7f4fe3 100644 --- a/esphome/components/e131/e131.cpp +++ b/esphome/components/e131/e131.cpp @@ -82,8 +82,9 @@ void E131Component::add_effect(E131AddressableLightEffect *light_effect) { return; } - ESP_LOGD(TAG, "Registering '%s' for universes %d-%d.", light_effect->get_name(), light_effect->get_first_universe(), - light_effect->get_last_universe()); + auto effect_name = light_effect->get_name(); + ESP_LOGD(TAG, "Registering '%.*s' for universes %d-%d.", (int) effect_name.size(), effect_name.c_str(), + light_effect->get_first_universe(), light_effect->get_last_universe()); light_effects_.push_back(light_effect); @@ -98,8 +99,9 @@ void E131Component::remove_effect(E131AddressableLightEffect *light_effect) { return; } - ESP_LOGD(TAG, "Unregistering '%s' for universes %d-%d.", light_effect->get_name(), light_effect->get_first_universe(), - light_effect->get_last_universe()); + auto effect_name = light_effect->get_name(); + ESP_LOGD(TAG, "Unregistering '%.*s' for universes %d-%d.", (int) effect_name.size(), effect_name.c_str(), + light_effect->get_first_universe(), light_effect->get_last_universe()); // Swap with last element and pop for O(1) removal (order doesn't matter) *it = light_effects_.back(); diff --git a/esphome/components/e131/e131_addressable_light_effect.cpp b/esphome/components/e131/e131_addressable_light_effect.cpp index 780e181f04..7d62f739a2 100644 --- a/esphome/components/e131/e131_addressable_light_effect.cpp +++ b/esphome/components/e131/e131_addressable_light_effect.cpp @@ -58,8 +58,9 @@ bool E131AddressableLightEffect::process_(int universe, const E131Packet &packet std::min(it->size(), std::min(output_offset + get_lights_per_universe(), output_offset + packet.count - 1)); auto *input_data = packet.values + 1; - ESP_LOGV(TAG, "Applying data for '%s' on %d universe, for %" PRId32 "-%d.", get_name(), universe, output_offset, - output_end); + auto effect_name = get_name(); + ESP_LOGV(TAG, "Applying data for '%.*s' on %d universe, for %" PRId32 "-%d.", (int) effect_name.size(), + effect_name.c_str(), universe, output_offset, output_end); switch (channels_) { case E131_MONO: diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 8161e8b814..234d641f0d 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -1,4 +1,5 @@ #include + #include "light_call.h" #include "light_state.h" #include "esphome/core/log.h" @@ -153,15 +154,15 @@ void LightCall::perform() { } else if (this->has_effect_()) { // EFFECT - const char *effect_s; + StringRef effect_s; if (this->effect_ == 0u) { - effect_s = "None"; + effect_s = StringRef::from_lit("None"); } else { effect_s = this->parent_->effects_[this->effect_ - 1]->get_name(); } if (publish) { - ESP_LOGD(TAG, " Effect: '%s'", effect_s); + ESP_LOGD(TAG, " Effect: '%.*s'", (int) effect_s.size(), effect_s.c_str()); } this->parent_->start_effect_(this->effect_); @@ -511,11 +512,9 @@ LightCall &LightCall::set_effect(const char *effect, size_t len) { } bool found = false; + StringRef effect_ref(effect, len); for (uint32_t i = 0; i < this->parent_->effects_.size(); i++) { - LightEffect *e = this->parent_->effects_[i]; - const char *name = e->get_name(); - - if (strncasecmp(effect, name, len) == 0 && name[len] == '\0') { + if (str_equals_case_insensitive(effect_ref, this->parent_->effects_[i]->get_name())) { this->set_effect(i + 1); found = true; break; diff --git a/esphome/components/light/light_effect.h b/esphome/components/light/light_effect.h index aa1f6f7899..a89e3fec5a 100644 --- a/esphome/components/light/light_effect.h +++ b/esphome/components/light/light_effect.h @@ -1,6 +1,7 @@ #pragma once #include "esphome/core/component.h" +#include "esphome/core/string_ref.h" namespace esphome::light { @@ -23,9 +24,9 @@ class LightEffect { /** * Returns the name of this effect. - * The returned pointer is valid for the lifetime of the program and must not be freed. + * The underlying data is valid for the lifetime of the program (static string from codegen). */ - const char *get_name() const { return this->name_; } + StringRef get_name() const { return StringRef(this->name_); } /// Internal method called by the LightState when this light effect is registered in it. virtual void init() {} diff --git a/esphome/components/light/light_json_schema.cpp b/esphome/components/light/light_json_schema.cpp index 98b03f9458..f370980737 100644 --- a/esphome/components/light/light_json_schema.cpp +++ b/esphome/components/light/light_json_schema.cpp @@ -36,7 +36,7 @@ static const char *get_color_mode_json_str(ColorMode mode) { void LightJSONSchema::dump_json(LightState &state, JsonObject root) { // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) false positive with ArduinoJson if (state.supports_effects()) { - root[ESPHOME_F("effect")] = state.get_effect_name_ref(); + root[ESPHOME_F("effect")] = state.get_effect_name().c_str(); root[ESPHOME_F("effect_index")] = state.get_current_effect_index(); root[ESPHOME_F("effect_count")] = state.get_effect_count(); } diff --git a/esphome/components/light/light_state.cpp b/esphome/components/light/light_state.cpp index 5a50bae50b..91bb2e2f1f 100644 --- a/esphome/components/light/light_state.cpp +++ b/esphome/components/light/light_state.cpp @@ -162,20 +162,12 @@ void LightState::publish_state() { LightOutput *LightState::get_output() const { return this->output_; } -static constexpr const char *EFFECT_NONE = "None"; static constexpr auto EFFECT_NONE_REF = StringRef::from_lit("None"); -std::string LightState::get_effect_name() { +StringRef LightState::get_effect_name() { if (this->active_effect_index_ > 0) { return this->effects_[this->active_effect_index_ - 1]->get_name(); } - return EFFECT_NONE; -} - -StringRef LightState::get_effect_name_ref() { - if (this->active_effect_index_ > 0) { - return StringRef(this->effects_[this->active_effect_index_ - 1]->get_name()); - } return EFFECT_NONE_REF; } diff --git a/esphome/components/light/light_state.h b/esphome/components/light/light_state.h index a21c2c7693..83b9226d03 100644 --- a/esphome/components/light/light_state.h +++ b/esphome/components/light/light_state.h @@ -140,9 +140,7 @@ class LightState : public EntityBase, public Component { LightOutput *get_output() const; /// Return the name of the current effect, or if no effect is active "None". - std::string get_effect_name(); - /// Return the name of the current effect as StringRef (for API usage) - StringRef get_effect_name_ref(); + StringRef get_effect_name(); /** Add a listener for remote values changes. * Listener is notified when the light's remote values change (state, brightness, color, etc.) @@ -191,11 +189,11 @@ class LightState : public EntityBase, public Component { /// Get effect index by name. Returns 0 if effect not found. uint32_t get_effect_index(const std::string &effect_name) const { - if (strcasecmp(effect_name.c_str(), "none") == 0) { + if (str_equals_case_insensitive(effect_name, "none")) { return 0; } for (size_t i = 0; i < this->effects_.size(); i++) { - if (strcasecmp(effect_name.c_str(), this->effects_[i]->get_name()) == 0) { + if (str_equals_case_insensitive(effect_name, this->effects_[i]->get_name())) { return i + 1; // Effects are 1-indexed in active_effect_index_ } } @@ -218,7 +216,7 @@ class LightState : public EntityBase, public Component { if (index > this->effects_.size()) { return ""; // Invalid index } - return this->effects_[index - 1]->get_name(); + return std::string(this->effects_[index - 1]->get_name()); } /// The result of all the current_values_as_* methods have gamma correction applied. diff --git a/esphome/components/mqtt/mqtt_light.cpp b/esphome/components/mqtt/mqtt_light.cpp index 2d588ed10b..fac19f3210 100644 --- a/esphome/components/mqtt/mqtt_light.cpp +++ b/esphome/components/mqtt/mqtt_light.cpp @@ -80,8 +80,10 @@ void MQTTJSONLightComponent::send_discovery(JsonObject root, mqtt::SendDiscovery if (this->state_->supports_effects()) { root[ESPHOME_F("effect")] = true; JsonArray effect_list = root[MQTT_EFFECT_LIST].to(); - for (auto *effect : this->state_->get_effects()) - effect_list.add(effect->get_name()); + for (auto *effect : this->state_->get_effects()) { + // c_str() is safe as effect names are null-terminated strings from codegen + effect_list.add(effect->get_name().c_str()); + } effect_list.add(ESPHOME_F("None")); } } diff --git a/esphome/components/prometheus/prometheus_handler.cpp b/esphome/components/prometheus/prometheus_handler.cpp index 75910fa73d..f4c6f6804b 100644 --- a/esphome/components/prometheus/prometheus_handler.cpp +++ b/esphome/components/prometheus/prometheus_handler.cpp @@ -363,13 +363,14 @@ void PrometheusHandler::light_row_(AsyncResponseStream *stream, light::LightStat // Skip effect metrics if light has no effects if (!obj->get_effects().empty()) { // Effect - std::string effect = obj->get_effect_name(); + StringRef effect = obj->get_effect_name(); print_metric_labels_(stream, ESPHOME_F("esphome_light_effect_active"), obj, area, node, friendly_name); stream->print(ESPHOME_F("\",effect=\"")); // Only vary based on effect if (effect == "None") { stream->print(ESPHOME_F("None\"} 0\n")); } else { + // c_str() is safe as effect names are null-terminated strings from codegen stream->print(effect.c_str()); stream->print(ESPHOME_F("\"} 1\n")); } diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 8671dc7f82..309407fbec 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -162,6 +162,9 @@ float random_float() { return static_cast(random_uint32()) / static_cast< bool str_equals_case_insensitive(const std::string &a, const std::string &b) { return strcasecmp(a.c_str(), b.c_str()) == 0; } +bool str_equals_case_insensitive(StringRef a, StringRef b) { + return a.size() == b.size() && strncasecmp(a.c_str(), b.c_str(), a.size()) == 0; +} #if __cplusplus >= 202002L bool str_startswith(const std::string &str, const std::string &start) { return str.starts_with(start); } bool str_endswith(const std::string &str, const std::string &end) { return str.ends_with(end); } diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index cd43709f7d..bf559d2bc6 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -511,6 +511,8 @@ template constexpr T convert_little_endian(T val) { /// Compare strings for equality in case-insensitive manner. bool str_equals_case_insensitive(const std::string &a, const std::string &b); +/// Compare StringRefs for equality in case-insensitive manner. +bool str_equals_case_insensitive(StringRef a, StringRef b); /// Check whether a string starts with a value. bool str_startswith(const std::string &str, const std::string &start); diff --git a/tests/components/light/common.yaml b/tests/components/light/common.yaml index 247fc19aba..55525fc67f 100644 --- a/tests/components/light/common.yaml +++ b/tests/components/light/common.yaml @@ -1,6 +1,65 @@ esphome: on_boot: then: + # Test LightEffect::get_name() returns StringRef + - lambda: |- + // Test LightEffect::get_name() returns StringRef + auto &effects = id(test_monochromatic_light).get_effects(); + if (!effects.empty()) { + // Test: get_name() returns StringRef + StringRef name = effects[0]->get_name(); + + // Test: comparison with string literal works directly + if (name == "Strobe") { + ESP_LOGI("test", "Found Strobe effect"); + } + + // Test: safe logging with %.*s format + ESP_LOGI("test", "Effect name: %.*s", (int) name.size(), name.c_str()); + + // Test: .c_str() for functions expecting const char* + ESP_LOGI("test", "Effect: %s", name.c_str()); + + // Test: explicit conversion to std::string + std::string name_str(name.c_str(), name.size()); + ESP_LOGI("test", "As string: %s", name_str.c_str()); + + // Test: size() method + ESP_LOGI("test", "Name length: %d", (int) name.size()); + } + + # Test LightState::get_effect_name() returns StringRef + - lambda: |- + // Test LightState::get_effect_name() returns StringRef + StringRef current_effect = id(test_monochromatic_light).get_effect_name(); + + // Test: comparison with "None" works directly + if (current_effect == "None") { + ESP_LOGI("test", "No effect active"); + } + + // Test: safe logging + ESP_LOGI("test", "Current effect: %.*s", (int) current_effect.size(), current_effect.c_str()); + + # Test str_equals_case_insensitive with StringRef + - lambda: |- + // Test str_equals_case_insensitive(StringRef, StringRef) + auto &effects = id(test_monochromatic_light).get_effects(); + if (!effects.empty()) { + StringRef name = effects[0]->get_name(); + + // Test: case-insensitive comparison + if (str_equals_case_insensitive(name, "STROBE")) { + ESP_LOGI("test", "Case-insensitive match works"); + } + + // Test: case-insensitive with StringRef from string + std::string search = "strobe"; + if (str_equals_case_insensitive(StringRef(search.c_str(), search.size()), name)) { + ESP_LOGI("test", "Reverse comparison works"); + } + } + - light.toggle: test_binary_light - light.turn_off: test_rgb_light - light.turn_on: From e1aac7601d95adeddb19f1877cf97408de2e1381 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 11 Jan 2026 17:52:54 -1000 Subject: [PATCH 4/7] [event] Return StringRef from get_last_event_type() (#13104) --- esphome/components/api/api_connection.cpp | 11 ++++++----- esphome/components/api/api_connection.h | 4 ++-- esphome/components/event/event.h | 8 ++++++-- esphome/components/prometheus/prometheus_handler.cpp | 5 +++-- esphome/components/web_server/web_server.cpp | 9 +++------ esphome/components/web_server/web_server.h | 2 +- tests/components/event/common.yaml | 11 +++++++++++ 7 files changed, 32 insertions(+), 18 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index f0fc5ba71c..656dbedb8a 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1415,14 +1415,15 @@ void APIConnection::on_water_heater_command_request(const WaterHeaterCommandRequ #endif #ifdef USE_EVENT -void APIConnection::send_event(event::Event *event, const char *event_type) { - this->send_message_smart_(event, MessageCreator(event_type), EventResponse::MESSAGE_TYPE, +void APIConnection::send_event(event::Event *event, StringRef event_type) { + // get_last_event_type() returns StringRef pointing to null-terminated string literals from codegen + this->send_message_smart_(event, MessageCreator(event_type.c_str()), EventResponse::MESSAGE_TYPE, EventResponse::ESTIMATED_SIZE); } -uint16_t APIConnection::try_send_event_response(event::Event *event, const char *event_type, APIConnection *conn, +uint16_t APIConnection::try_send_event_response(event::Event *event, StringRef event_type, APIConnection *conn, uint32_t remaining_size, bool is_single) { EventResponse resp; - resp.event_type = StringRef(event_type); + resp.event_type = event_type; return fill_and_encode_entity_state(event, resp, EventResponse::MESSAGE_TYPE, conn, remaining_size, is_single); } @@ -2056,7 +2057,7 @@ uint16_t APIConnection::MessageCreator::operator()(EntityBase *entity, APIConnec // Special case: EventResponse uses const char * pointer if (message_type == EventResponse::MESSAGE_TYPE) { auto *e = static_cast(entity); - return APIConnection::try_send_event_response(e, data_.const_char_ptr, conn, remaining_size, is_single); + return APIConnection::try_send_event_response(e, StringRef(data_.const_char_ptr), conn, remaining_size, is_single); } #endif diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 15d79a25ec..0289b3d2ff 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -173,7 +173,7 @@ class APIConnection final : public APIServerConnection { #endif #ifdef USE_EVENT - void send_event(event::Event *event, const char *event_type); + void send_event(event::Event *event, StringRef event_type); #endif #ifdef USE_UPDATE @@ -469,7 +469,7 @@ class APIConnection final : public APIServerConnection { bool is_single); #endif #ifdef USE_EVENT - static uint16_t try_send_event_response(event::Event *event, const char *event_type, APIConnection *conn, + static uint16_t try_send_event_response(event::Event *event, StringRef event_type, APIConnection *conn, uint32_t remaining_size, bool is_single); static uint16_t try_send_event_info(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single); #endif diff --git a/esphome/components/event/event.h b/esphome/components/event/event.h index 0d5850d339..27700e32d8 100644 --- a/esphome/components/event/event.h +++ b/esphome/components/event/event.h @@ -7,6 +7,7 @@ #include "esphome/core/component.h" #include "esphome/core/entity_base.h" #include "esphome/core/helpers.h" +#include "esphome/core/string_ref.h" namespace esphome { namespace event { @@ -44,8 +45,11 @@ class Event : public EntityBase, public EntityBase_DeviceClass { /// Return the event types supported by this event. const FixedVector &get_event_types() const { return this->types_; } - /// Return the last triggered event type (pointer to string in types_), or nullptr if no event triggered yet. - const char *get_last_event_type() const { return this->last_event_type_; } + /// Return the last triggered event type, or empty StringRef if no event triggered yet. + StringRef get_last_event_type() const { return StringRef::from_maybe_nullptr(this->last_event_type_); } + + /// Check if an event has been triggered. + bool has_event() const { return this->last_event_type_ != nullptr; } void add_on_event_callback(std::function &&callback); diff --git a/esphome/components/prometheus/prometheus_handler.cpp b/esphome/components/prometheus/prometheus_handler.cpp index f4c6f6804b..dd577a4dbc 100644 --- a/esphome/components/prometheus/prometheus_handler.cpp +++ b/esphome/components/prometheus/prometheus_handler.cpp @@ -600,7 +600,7 @@ void PrometheusHandler::event_row_(AsyncResponseStream *stream, event::Event *ob std::string &friendly_name) { if (obj->is_internal() && !this->include_internal_) return; - if (obj->get_last_event_type() != nullptr) { + if (obj->has_event()) { // We have a valid event type, output this value stream->print(ESPHOME_F("esphome_event_failed{id=\"")); stream->print(relabel_id_(obj).c_str()); @@ -619,7 +619,8 @@ void PrometheusHandler::event_row_(AsyncResponseStream *stream, event::Event *ob stream->print(ESPHOME_F("\",name=\"")); stream->print(relabel_name_(obj).c_str()); stream->print(ESPHOME_F("\",last_event_type=\"")); - stream->print(obj->get_last_event_type()); + // get_last_event_type() returns StringRef (null-terminated) + stream->print(obj->get_last_event_type().c_str()); stream->print(ESPHOME_F("\"} ")); stream->print(ESPHOME_F("1.0")); stream->print(ESPHOME_F("\n")); diff --git a/esphome/components/web_server/web_server.cpp b/esphome/components/web_server/web_server.cpp index 41d225c0d8..76a516d90f 100644 --- a/esphome/components/web_server/web_server.cpp +++ b/esphome/components/web_server/web_server.cpp @@ -1958,7 +1958,7 @@ void WebServer::handle_event_request(AsyncWebServerRequest *request, const UrlMa // Note: request->method() is always HTTP_GET here (canHandle ensures this) if (entity_match.action_is_empty) { auto detail = get_request_detail(request); - std::string data = this->event_json_(obj, "", detail); + std::string data = this->event_json_(obj, StringRef(), detail); request->send(200, "application/json", data.c_str()); return; } @@ -1966,10 +1966,7 @@ void WebServer::handle_event_request(AsyncWebServerRequest *request, const UrlMa request->send(404); } -static std::string get_event_type(event::Event *event) { - const char *last_type = event ? event->get_last_event_type() : nullptr; - return last_type ? last_type : ""; -} +static StringRef get_event_type(event::Event *event) { return event ? event->get_last_event_type() : StringRef(); } std::string WebServer::event_state_json_generator(WebServer *web_server, void *source) { auto *event = static_cast(source); @@ -1980,7 +1977,7 @@ std::string WebServer::event_all_json_generator(WebServer *web_server, void *sou auto *event = static_cast(source); return web_server->event_json_(event, get_event_type(event), DETAIL_ALL); } -std::string WebServer::event_json_(event::Event *obj, const std::string &event_type, JsonDetail start_config) { +std::string WebServer::event_json_(event::Event *obj, StringRef event_type, JsonDetail start_config) { json::JsonBuilder builder; JsonObject root = builder.root(); diff --git a/esphome/components/web_server/web_server.h b/esphome/components/web_server/web_server.h index b62686f0aa..55fa89679e 100644 --- a/esphome/components/web_server/web_server.h +++ b/esphome/components/web_server/web_server.h @@ -643,7 +643,7 @@ class WebServer : public Controller, alarm_control_panel::AlarmControlPanelState value, JsonDetail start_config); #endif #ifdef USE_EVENT - std::string event_json_(event::Event *obj, const std::string &event_type, JsonDetail start_config); + std::string event_json_(event::Event *obj, StringRef event_type, JsonDetail start_config); #endif #ifdef USE_WATER_HEATER std::string water_heater_json_(water_heater::WaterHeater *obj, JsonDetail start_config); diff --git a/tests/components/event/common.yaml b/tests/components/event/common.yaml index 71cc19a6b0..555d049c70 100644 --- a/tests/components/event/common.yaml +++ b/tests/components/event/common.yaml @@ -7,3 +7,14 @@ event: - template_event_type2 on_event: - logger.log: Event fired + - lambda: |- + // Test get_last_event_type() returns StringRef + if (id(some_event).has_event()) { + auto event_type = id(some_event).get_last_event_type(); + // Compare with string literal using == + if (event_type == "template_event_type1") { + ESP_LOGD("test", "Event type is template_event_type1"); + } + // Log using %.*s format for StringRef + ESP_LOGD("test", "Event type: %.*s", (int) event_type.size(), event_type.c_str()); + } From ea8ae2ae6066cc1e8ad3abaf749f74d579d09296 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 11 Jan 2026 17:53:06 -1000 Subject: [PATCH 5/7] [climate] Return StringRef from get_custom_fan_mode() and get_custom_preset() (#13103) --- esphome/components/api/api_connection.cpp | 4 ++-- .../bedjet/climate/bedjet_climate.cpp | 21 ++++++++++--------- esphome/components/climate/climate.cpp | 10 ++++----- esphome/components/climate/climate.h | 21 ++++++++++++------- esphome/components/midea/air_conditioner.cpp | 6 ++++-- esphome/components/mqtt/mqtt_climate.cpp | 4 ++-- .../thermostat/thermostat_climate.cpp | 7 ++++--- .../thermostat/thermostat_climate.h | 8 ++++++- tests/components/midea/common.yaml | 19 +++++++++++++++++ tests/components/thermostat/common.yaml | 14 +++++++++++++ 10 files changed, 81 insertions(+), 33 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 656dbedb8a..d6f0d84550 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -676,13 +676,13 @@ uint16_t APIConnection::try_send_climate_state(EntityBase *entity, APIConnection if (traits.get_supports_fan_modes() && climate->fan_mode.has_value()) resp.fan_mode = static_cast(climate->fan_mode.value()); if (!traits.get_supported_custom_fan_modes().empty() && climate->has_custom_fan_mode()) { - resp.custom_fan_mode = StringRef(climate->get_custom_fan_mode()); + resp.custom_fan_mode = climate->get_custom_fan_mode(); } if (traits.get_supports_presets() && climate->preset.has_value()) { resp.preset = static_cast(climate->preset.value()); } if (!traits.get_supported_custom_presets().empty() && climate->has_custom_preset()) { - resp.custom_preset = StringRef(climate->get_custom_preset()); + resp.custom_preset = climate->get_custom_preset(); } if (traits.get_supports_swing_modes()) resp.swing_mode = static_cast(climate->swing_mode); diff --git a/esphome/components/bedjet/climate/bedjet_climate.cpp b/esphome/components/bedjet/climate/bedjet_climate.cpp index 716d4d4241..68a0342873 100644 --- a/esphome/components/bedjet/climate/bedjet_climate.cpp +++ b/esphome/components/bedjet/climate/bedjet_climate.cpp @@ -164,21 +164,21 @@ void BedJetClimate::control(const ClimateCall &call) { return; } } else if (call.has_custom_preset()) { - const char *preset = call.get_custom_preset(); + auto preset = call.get_custom_preset(); bool result; - if (strcmp(preset, "M1") == 0) { + if (preset == "M1") { result = this->parent_->button_memory1(); - } else if (strcmp(preset, "M2") == 0) { + } else if (preset == "M2") { result = this->parent_->button_memory2(); - } else if (strcmp(preset, "M3") == 0) { + } else if (preset == "M3") { result = this->parent_->button_memory3(); - } else if (strcmp(preset, "LTD HT") == 0) { + } else if (preset == "LTD HT") { result = this->parent_->button_heat(); - } else if (strcmp(preset, "EXT HT") == 0) { + } else if (preset == "EXT HT") { result = this->parent_->button_ext_heat(); } else { - ESP_LOGW(TAG, "Unsupported preset: %s", preset); + ESP_LOGW(TAG, "Unsupported preset: %.*s", (int) preset.size(), preset.c_str()); return; } @@ -208,10 +208,11 @@ void BedJetClimate::control(const ClimateCall &call) { this->set_fan_mode_(fan_mode); } } else if (call.has_custom_fan_mode()) { - const char *fan_mode = call.get_custom_fan_mode(); - auto fan_index = bedjet_fan_speed_to_step(fan_mode); + auto fan_mode = call.get_custom_fan_mode(); + auto fan_index = bedjet_fan_speed_to_step(fan_mode.c_str()); if (fan_index <= 19) { - ESP_LOGV(TAG, "[%s] Converted fan mode %s to bedjet fan step %d", this->get_name().c_str(), fan_mode, fan_index); + ESP_LOGV(TAG, "[%s] Converted fan mode %.*s to bedjet fan step %d", this->get_name().c_str(), + (int) fan_mode.size(), fan_mode.c_str(), fan_index); bool result = this->parent_->set_fan_index(fan_index); if (result) { this->set_custom_fan_mode_(fan_mode); diff --git a/esphome/components/climate/climate.cpp b/esphome/components/climate/climate.cpp index 2d35509493..7611d33cbf 100644 --- a/esphome/components/climate/climate.cpp +++ b/esphome/components/climate/climate.cpp @@ -682,19 +682,19 @@ bool Climate::set_fan_mode_(ClimateFanMode mode) { return set_primary_mode(this->fan_mode, this->custom_fan_mode_, mode); } -bool Climate::set_custom_fan_mode_(const char *mode) { +bool Climate::set_custom_fan_mode_(const char *mode, size_t len) { auto traits = this->get_traits(); - return set_custom_mode(this->custom_fan_mode_, this->fan_mode, traits.find_custom_fan_mode_(mode), - this->has_custom_fan_mode()); + return set_custom_mode(this->custom_fan_mode_, this->fan_mode, + traits.find_custom_fan_mode_(mode, len), this->has_custom_fan_mode()); } void Climate::clear_custom_fan_mode_() { this->custom_fan_mode_ = nullptr; } bool Climate::set_preset_(ClimatePreset preset) { return set_primary_mode(this->preset, this->custom_preset_, preset); } -bool Climate::set_custom_preset_(const char *preset) { +bool Climate::set_custom_preset_(const char *preset, size_t len) { auto traits = this->get_traits(); - return set_custom_mode(this->custom_preset_, this->preset, traits.find_custom_preset_(preset), + return set_custom_mode(this->custom_preset_, this->preset, traits.find_custom_preset_(preset, len), this->has_custom_preset()); } diff --git a/esphome/components/climate/climate.h b/esphome/components/climate/climate.h index 06adb580cf..6fac254502 100644 --- a/esphome/components/climate/climate.h +++ b/esphome/components/climate/climate.h @@ -5,6 +5,7 @@ #include "esphome/core/helpers.h" #include "esphome/core/log.h" #include "esphome/core/preferences.h" +#include "esphome/core/string_ref.h" #include "climate_mode.h" #include "climate_traits.h" @@ -110,8 +111,8 @@ class ClimateCall { const optional &get_fan_mode() const; const optional &get_swing_mode() const; const optional &get_preset() const; - const char *get_custom_fan_mode() const { return this->custom_fan_mode_; } - const char *get_custom_preset() const { return this->custom_preset_; } + StringRef get_custom_fan_mode() const { return StringRef::from_maybe_nullptr(this->custom_fan_mode_); } + StringRef get_custom_preset() const { return StringRef::from_maybe_nullptr(this->custom_preset_); } bool has_custom_fan_mode() const { return this->custom_fan_mode_ != nullptr; } bool has_custom_preset() const { return this->custom_preset_ != nullptr; } @@ -266,11 +267,11 @@ class Climate : public EntityBase { /// The active swing mode of the climate device. ClimateSwingMode swing_mode{CLIMATE_SWING_OFF}; - /// Get the active custom fan mode (read-only access). - const char *get_custom_fan_mode() const { return this->custom_fan_mode_; } + /// Get the active custom fan mode (read-only access). Returns StringRef. + StringRef get_custom_fan_mode() const { return StringRef::from_maybe_nullptr(this->custom_fan_mode_); } - /// Get the active custom preset (read-only access). - const char *get_custom_preset() const { return this->custom_preset_; } + /// Get the active custom preset (read-only access). Returns StringRef. + StringRef get_custom_preset() const { return StringRef::from_maybe_nullptr(this->custom_preset_); } protected: friend ClimateCall; @@ -280,7 +281,9 @@ class Climate : public EntityBase { bool set_fan_mode_(ClimateFanMode mode); /// Set custom fan mode. Reset primary fan mode. Return true if fan mode has been changed. - bool set_custom_fan_mode_(const char *mode); + bool set_custom_fan_mode_(const char *mode) { return this->set_custom_fan_mode_(mode, strlen(mode)); } + bool set_custom_fan_mode_(const char *mode, size_t len); + bool set_custom_fan_mode_(StringRef mode) { return this->set_custom_fan_mode_(mode.c_str(), mode.size()); } /// Clear custom fan mode. void clear_custom_fan_mode_(); @@ -288,7 +291,9 @@ class Climate : public EntityBase { bool set_preset_(ClimatePreset preset); /// Set custom preset. Reset primary preset. Return true if preset has been changed. - bool set_custom_preset_(const char *preset); + bool set_custom_preset_(const char *preset) { return this->set_custom_preset_(preset, strlen(preset)); } + bool set_custom_preset_(const char *preset, size_t len); + bool set_custom_preset_(StringRef preset) { return this->set_custom_preset_(preset.c_str(), preset.size()); } /// Clear custom preset. void clear_custom_preset_(); diff --git a/esphome/components/midea/air_conditioner.cpp b/esphome/components/midea/air_conditioner.cpp index a6a8d52549..bc750e3713 100644 --- a/esphome/components/midea/air_conditioner.cpp +++ b/esphome/components/midea/air_conditioner.cpp @@ -65,12 +65,14 @@ void AirConditioner::control(const ClimateCall &call) { if (call.get_preset().has_value()) { ctrl.preset = Converters::to_midea_preset(call.get_preset().value()); } else if (call.has_custom_preset()) { - ctrl.preset = Converters::to_midea_preset(call.get_custom_preset()); + // get_custom_preset() returns StringRef pointing to null-terminated string literals from codegen + ctrl.preset = Converters::to_midea_preset(call.get_custom_preset().c_str()); } if (call.get_fan_mode().has_value()) { ctrl.fanMode = Converters::to_midea_fan_mode(call.get_fan_mode().value()); } else if (call.has_custom_fan_mode()) { - ctrl.fanMode = Converters::to_midea_fan_mode(call.get_custom_fan_mode()); + // get_custom_fan_mode() returns StringRef pointing to null-terminated string literals from codegen + ctrl.fanMode = Converters::to_midea_fan_mode(call.get_custom_fan_mode().c_str()); } this->base_.control(ctrl); } diff --git a/esphome/components/mqtt/mqtt_climate.cpp b/esphome/components/mqtt/mqtt_climate.cpp index 77aabb2461..625fb715a7 100644 --- a/esphome/components/mqtt/mqtt_climate.cpp +++ b/esphome/components/mqtt/mqtt_climate.cpp @@ -357,7 +357,7 @@ bool MQTTClimateComponent::publish_state_() { } } if (this->device_->has_custom_preset()) - payload = this->device_->get_custom_preset(); + payload = this->device_->get_custom_preset().c_str(); if (!this->publish(this->get_preset_state_topic(), payload)) success = false; } @@ -429,7 +429,7 @@ bool MQTTClimateComponent::publish_state_() { } } if (this->device_->has_custom_fan_mode()) - payload = this->device_->get_custom_fan_mode(); + payload = this->device_->get_custom_fan_mode().c_str(); if (!this->publish(this->get_fan_mode_state_topic(), payload)) success = false; } diff --git a/esphome/components/thermostat/thermostat_climate.cpp b/esphome/components/thermostat/thermostat_climate.cpp index d5fb259dad..0416438dcd 100644 --- a/esphome/components/thermostat/thermostat_climate.cpp +++ b/esphome/components/thermostat/thermostat_climate.cpp @@ -1218,11 +1218,12 @@ void ThermostatClimate::change_preset_(climate::ClimatePreset preset) { } } -void ThermostatClimate::change_custom_preset_(const char *custom_preset) { +void ThermostatClimate::change_custom_preset_(const char *custom_preset, size_t len) { // Linear search through custom preset configurations const ThermostatClimateTargetTempConfig *config = nullptr; for (const auto &entry : this->custom_preset_config_) { - if (strcmp(entry.name, custom_preset) == 0) { + // Compare first len chars, then verify entry.name ends there (same length) + if (strncmp(entry.name, custom_preset, len) == 0 && entry.name[len] == '\0') { config = &entry.config; break; } @@ -1231,7 +1232,7 @@ void ThermostatClimate::change_custom_preset_(const char *custom_preset) { if (config != nullptr) { ESP_LOGV(TAG, "Custom preset %s requested", custom_preset); if (this->change_preset_internal_(*config) || !this->has_custom_preset() || - strcmp(this->get_custom_preset(), custom_preset) != 0) { + this->get_custom_preset() != custom_preset) { // Fire any preset changed trigger if defined Trigger<> *trig = this->preset_change_trigger_; // Use the base class method which handles pointer lookup and preset reset internally diff --git a/esphome/components/thermostat/thermostat_climate.h b/esphome/components/thermostat/thermostat_climate.h index 564b6127b3..d37c9a68a6 100644 --- a/esphome/components/thermostat/thermostat_climate.h +++ b/esphome/components/thermostat/thermostat_climate.h @@ -214,7 +214,13 @@ class ThermostatClimate : public climate::Climate, public Component { /// Change to a provided preset setting; will reset temperature, mode, fan, and swing modes accordingly void change_preset_(climate::ClimatePreset preset); /// Change to a provided custom preset setting; will reset temperature, mode, fan, and swing modes accordingly - void change_custom_preset_(const char *custom_preset); + void change_custom_preset_(const char *custom_preset) { + this->change_custom_preset_(custom_preset, strlen(custom_preset)); + } + void change_custom_preset_(const char *custom_preset, size_t len); + void change_custom_preset_(StringRef custom_preset) { + this->change_custom_preset_(custom_preset.c_str(), custom_preset.size()); + } /// Applies the temperature, mode, fan, and swing modes of the provided config. /// This is agnostic of custom vs built in preset diff --git a/tests/components/midea/common.yaml b/tests/components/midea/common.yaml index fec85aee96..c7b18a6701 100644 --- a/tests/components/midea/common.yaml +++ b/tests/components/midea/common.yaml @@ -12,6 +12,25 @@ climate: x.set_mode(CLIMATE_MODE_FAN_ONLY); on_state: - logger.log: State changed! + - lambda: |- + // Test get_custom_fan_mode() returns StringRef + if (id(midea_unit).has_custom_fan_mode()) { + auto fan_mode = id(midea_unit).get_custom_fan_mode(); + // Compare with string literal using == + if (fan_mode == "SILENT") { + ESP_LOGD("test", "Fan mode is SILENT"); + } + // Log using %.*s format for StringRef + ESP_LOGD("test", "Custom fan mode: %.*s", (int) fan_mode.size(), fan_mode.c_str()); + } + // Test get_custom_preset() returns StringRef + if (id(midea_unit).has_custom_preset()) { + auto preset = id(midea_unit).get_custom_preset(); + // Check if empty + if (!preset.empty()) { + ESP_LOGD("test", "Custom preset: %.*s", (int) preset.size(), preset.c_str()); + } + } transmitter_id: xmitr period: 1s num_attempts: 5 diff --git a/tests/components/thermostat/common.yaml b/tests/components/thermostat/common.yaml index 63bd174e14..69e258f2e3 100644 --- a/tests/components/thermostat/common.yaml +++ b/tests/components/thermostat/common.yaml @@ -5,6 +5,7 @@ sensor: climate: - platform: thermostat + id: test_thermostat name: Test Thermostat sensor: thermostat_sensor humidity_sensor: thermostat_sensor @@ -15,6 +16,19 @@ climate: - name: Away default_target_temperature_low: 16°C default_target_temperature_high: 20°C + on_state: + - lambda: |- + // Test get_custom_preset() returns std::string_view + // "Default Preset" is a custom preset (not a standard ClimatePreset name) + if (id(test_thermostat).has_custom_preset()) { + auto preset = id(test_thermostat).get_custom_preset(); + // Compare with string literal using == + if (preset == "Default Preset") { + ESP_LOGD("test", "Preset is Default Preset"); + } + // Log using %.*s format for StringRef + ESP_LOGD("test", "Custom preset: %.*s", (int) preset.size(), preset.c_str()); + } idle_action: - logger.log: idle_action cool_action: From 912f94d1e825a35f98d6dc3731962f6e0f69dcc2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 11 Jan 2026 17:54:06 -1000 Subject: [PATCH 6/7] [api] Use StringRef for HomeassistantServiceMap.value to eliminate heap allocations (#13154) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/api/api.proto | 4 +- esphome/components/api/api_options.proto | 1 - esphome/components/api/api_pb2.h | 4 +- esphome/components/api/custom_api_device.h | 4 +- .../components/api/homeassistant_service.h | 43 ++++++++++++++--- .../number/homeassistant_number.cpp | 7 ++- .../switch/homeassistant_switch.cpp | 2 +- esphome/core/automation.h | 6 +++ script/api_protobuf/api_protobuf.py | 48 +++---------------- 9 files changed, 61 insertions(+), 58 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 652b456850..d6384456d5 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -763,7 +763,7 @@ message SubscribeHomeassistantServicesRequest { message HomeassistantServiceMap { string key = 1; - string value = 2 [(no_zero_copy) = true]; + string value = 2; } message HomeassistantActionRequest { @@ -779,7 +779,7 @@ message HomeassistantActionRequest { bool is_event = 5; uint32 call_id = 6 [(field_ifdef) = "USE_API_HOMEASSISTANT_ACTION_RESPONSES"]; bool wants_response = 7 [(field_ifdef) = "USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON"]; - string response_template = 8 [(no_zero_copy) = true, (field_ifdef) = "USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON"]; + string response_template = 8 [(field_ifdef) = "USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON"]; } // Message sent by Home Assistant to ESPHome with service call response data diff --git a/esphome/components/api/api_options.proto b/esphome/components/api/api_options.proto index 1916e84625..a863f2c7a8 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -27,7 +27,6 @@ extend google.protobuf.MessageOptions { extend google.protobuf.FieldOptions { optional string field_ifdef = 1042; optional uint32 fixed_array_size = 50007; - optional bool no_zero_copy = 50008 [default=false]; optional bool fixed_array_skip_zero = 50009 [default=false]; optional string fixed_array_size_define = 50010; optional string fixed_array_with_length_define = 50011; diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 01fe44d7c7..e21b8596ca 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1053,7 +1053,7 @@ class SubscribeHomeassistantServicesRequest final : public ProtoMessage { class HomeassistantServiceMap final : public ProtoMessage { public: StringRef key{}; - std::string value{}; + StringRef value{}; void encode(ProtoWriteBuffer buffer) const override; void calculate_size(ProtoSize &size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP @@ -1081,7 +1081,7 @@ class HomeassistantActionRequest final : public ProtoMessage { bool wants_response{false}; #endif #ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON - std::string response_template{}; + StringRef response_template{}; #endif void encode(ProtoWriteBuffer buffer) const override; void calculate_size(ProtoSize &size) const override; diff --git a/esphome/components/api/custom_api_device.h b/esphome/components/api/custom_api_device.h index b16164270b..2fd9cb0dd2 100644 --- a/esphome/components/api/custom_api_device.h +++ b/esphome/components/api/custom_api_device.h @@ -265,7 +265,7 @@ class CustomAPIDevice { for (auto &it : data) { auto &kv = resp.data.emplace_back(); kv.key = StringRef(it.first); - kv.value = it.second; // value is std::string (no_zero_copy), assign directly + kv.value = StringRef(it.second); // data map lives until send completes } global_api_server->send_homeassistant_action(resp); } @@ -308,7 +308,7 @@ class CustomAPIDevice { for (auto &it : data) { auto &kv = resp.data.emplace_back(); kv.key = StringRef(it.first); - kv.value = it.second; // value is std::string (no_zero_copy), assign directly + kv.value = StringRef(it.second); // data map lives until send completes } global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/components/api/homeassistant_service.h b/esphome/components/api/homeassistant_service.h index a17c99b8ba..9bffe18764 100644 --- a/esphome/components/api/homeassistant_service.h +++ b/esphome/components/api/homeassistant_service.h @@ -149,11 +149,21 @@ template class HomeAssistantServiceCallAction : public Actionservice_.value(x...); resp.service = StringRef(service_value); resp.is_event = this->flags_.is_event; - this->populate_service_map(resp.data, this->data_, x...); - this->populate_service_map(resp.data_template, this->data_template_, x...); - this->populate_service_map(resp.variables, this->variables_, x...); + + // Local storage for lambda-evaluated strings - lives until after send + FixedVector data_storage; + FixedVector data_template_storage; + FixedVector variables_storage; + + this->populate_service_map(resp.data, this->data_, data_storage, x...); + this->populate_service_map(resp.data_template, this->data_template_, data_template_storage, x...); + this->populate_service_map(resp.variables, this->variables_, variables_storage, x...); #ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES +#ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON + // IMPORTANT: Declare at outer scope so it lives until send_homeassistant_action returns. + std::string response_template_value; +#endif if (this->flags_.wants_status) { // Generate a unique call ID for this service call static uint32_t call_id_counter = 1; @@ -164,8 +174,8 @@ template class HomeAssistantServiceCallAction : public Actionflags_.has_response_template) { - std::string response_template_value = this->response_template_.value(x...); - resp.response_template = response_template_value; + response_template_value = this->response_template_.value(x...); + resp.response_template = StringRef(response_template_value); } } #endif @@ -205,12 +215,31 @@ template class HomeAssistantServiceCallAction : public Action - static void populate_service_map(VectorType &dest, SourceType &source, Ts... x) { + static void populate_service_map(VectorType &dest, SourceType &source, FixedVector &value_storage, + Ts... x) { dest.init(source.size()); + + // Count non-static strings to allocate exact storage needed + size_t lambda_count = 0; + for (const auto &it : source) { + if (!it.value.is_static_string()) { + lambda_count++; + } + } + value_storage.init(lambda_count); + for (auto &it : source) { auto &kv = dest.emplace_back(); kv.key = StringRef(it.key); - kv.value = it.value.value(x...); + + if (it.value.is_static_string()) { + // Static string from YAML - zero allocation + kv.value = StringRef(it.value.get_static_string()); + } else { + // Lambda evaluation - store result, reference it + value_storage.push_back(it.value.value(x...)); + kv.value = StringRef(value_storage.back()); + } } } diff --git a/esphome/components/homeassistant/number/homeassistant_number.cpp b/esphome/components/homeassistant/number/homeassistant_number.cpp index 82387a81e9..92ecd5ea39 100644 --- a/esphome/components/homeassistant/number/homeassistant_number.cpp +++ b/esphome/components/homeassistant/number/homeassistant_number.cpp @@ -91,11 +91,14 @@ void HomeassistantNumber::control(float value) { resp.data.init(2); auto &entity_id = resp.data.emplace_back(); entity_id.key = ENTITY_ID_KEY; - entity_id.value = this->entity_id_; + entity_id.value = StringRef(this->entity_id_); auto &entity_value = resp.data.emplace_back(); entity_value.key = VALUE_KEY; - entity_value.value = to_string(value); + // Stack buffer - no heap allocation; %g produces shortest representation + char value_buf[16]; + snprintf(value_buf, sizeof(value_buf), "%g", value); + entity_value.value = StringRef(value_buf); api::global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/components/homeassistant/switch/homeassistant_switch.cpp b/esphome/components/homeassistant/switch/homeassistant_switch.cpp index 79d17eb290..cc3d582bf3 100644 --- a/esphome/components/homeassistant/switch/homeassistant_switch.cpp +++ b/esphome/components/homeassistant/switch/homeassistant_switch.cpp @@ -55,7 +55,7 @@ void HomeassistantSwitch::write_state(bool state) { resp.data.init(1); auto &entity_id_kv = resp.data.emplace_back(); entity_id_kv.key = ENTITY_ID_KEY; - entity_id_kv.value = this->entity_id_; + entity_id_kv.value = StringRef(this->entity_id_); api::global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/core/automation.h b/esphome/core/automation.h index 61d2944acf..585b434bb2 100644 --- a/esphome/core/automation.h +++ b/esphome/core/automation.h @@ -159,6 +159,12 @@ template class TemplatableValue { return this->value(x...); } + /// Check if this holds a static string (const char* stored without allocation) + bool is_static_string() const { return this->type_ == STATIC_STRING; } + + /// Get the static string pointer (only valid if is_static_string() returns true) + const char *get_static_string() const { return this->static_str_; } + protected: enum : uint8_t { NONE, diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index c61555805e..118c87356e 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -376,10 +376,8 @@ def create_field_type_info( return BytesType(field, needs_decode, needs_encode) - # Special handling for string fields - use StringRef for zero-copy unless no_zero_copy is set + # Special handling for string fields - use StringRef for zero-copy if field.type == 9: - if get_field_opt(field, pb.no_zero_copy, False): - return StringType(field, needs_decode, needs_encode) return PointerToStringBufferType(field, None) validate_field_type(field.type, field.name) @@ -585,15 +583,12 @@ class StringType(TypeInfo): def public_content(self) -> list[str]: content: list[str] = [] - # Check if no_zero_copy option is set - no_zero_copy = get_field_opt(self._field, pb.no_zero_copy, False) - - # Add std::string storage if message needs decoding OR if no_zero_copy is set - if self._needs_decode or no_zero_copy: + # Add std::string storage if message needs decoding + if self._needs_decode: content.append(f"std::string {self.field_name}{{}};") - # Only add StringRef if encoding is needed AND no_zero_copy is not set - if self._needs_encode and not no_zero_copy: + # Add StringRef if encoding is needed + if self._needs_encode: content.extend( [ # Add StringRef field if message needs encoding @@ -608,27 +603,14 @@ class StringType(TypeInfo): @property def encode_content(self) -> str: - # Check if no_zero_copy option is set - no_zero_copy = get_field_opt(self._field, pb.no_zero_copy, False) - - if no_zero_copy: - # Use the std::string directly - return f"buffer.encode_string({self.number}, this->{self.field_name});" # Use the StringRef return f"buffer.encode_string({self.number}, this->{self.field_name}_ref_);" def dump(self, name): - # Check if no_zero_copy option is set - no_zero_copy = get_field_opt(self._field, pb.no_zero_copy, False) - # If name is 'it', this is a repeated field element - always use string if name == "it": return "append_quoted_string(out, StringRef(it));" - # If no_zero_copy is set, always use std::string - if no_zero_copy: - return f'out.append("\'").append(this->{self.field_name}).append("\'");' - # For SOURCE_CLIENT only, always use std::string if not self._needs_encode: return f'out.append("\'").append(this->{self.field_name}).append("\'");' @@ -648,13 +630,6 @@ class StringType(TypeInfo): @property def dump_content(self) -> str: - # Check if no_zero_copy option is set - no_zero_copy = get_field_opt(self._field, pb.no_zero_copy, False) - - # If no_zero_copy is set, always use std::string - if no_zero_copy: - return f'dump_field(out, "{self.name}", this->{self.field_name});' - # For SOURCE_CLIENT only, use std::string if not self._needs_encode: return f'dump_field(out, "{self.name}", this->{self.field_name});' @@ -670,17 +645,8 @@ class StringType(TypeInfo): return o def get_size_calculation(self, name: str, force: bool = False) -> str: - # Check if no_zero_copy option is set - no_zero_copy = get_field_opt(self._field, pb.no_zero_copy, False) - - # For SOURCE_CLIENT only messages or no_zero_copy, use the string field directly - if not self._needs_encode or no_zero_copy: - # For no_zero_copy, we need to use .size() on the string - if no_zero_copy and name != "it": - field_id_size = self.calculate_field_id_size() - return ( - f"size.add_length({field_id_size}, this->{self.field_name}.size());" - ) + # For SOURCE_CLIENT only messages, use the string field directly + if not self._needs_encode: return self._get_simple_size_calculation(name, force, "add_length") # Check if this is being called from a repeated field context From d7e7e7849f311a1584e69c021fb9b336e5233e21 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 11 Jan 2026 19:59:05 -1000 Subject: [PATCH 7/7] [api] Use stack buffer for bytes field dumping in proto message logs --- esphome/components/api/api_pb2_dump.cpp | 62 +++++++++---------------- script/api_protobuf/api_protobuf.py | 52 ++++++++++++++++----- 2 files changed, 64 insertions(+), 50 deletions(-) diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 160a9a93c9..999107956a 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -100,6 +100,16 @@ template static void dump_field(std::string &out, const char *field_ out.append("\n"); } +// Helper for bytes fields - uses stack buffer to avoid heap allocation +// Buffer sized for 160 bytes of data (480 chars with separators) to fit typical log buffer +static void dump_bytes_field(std::string &out, const char *field_name, const uint8_t *data, size_t len, + int indent = 2) { + char hex_buf[format_hex_pretty_size(160)]; + append_field_prefix(out, field_name, indent); + format_hex_pretty_to(hex_buf, data, len); + append_with_newline(out, hex_buf); +} + template<> const char *proto_enum_to_string(enums::EntityCategory value) { switch (value) { case enums::ENTITY_CATEGORY_NONE: @@ -1127,16 +1137,12 @@ void SubscribeLogsRequest::dump_to(std::string &out) const { void SubscribeLogsResponse::dump_to(std::string &out) const { MessageDumpHelper helper(out, "SubscribeLogsResponse"); dump_field(out, "level", static_cast(this->level)); - out.append(" message: "); - out.append(format_hex_pretty(this->message_ptr_, this->message_len_)); - out.append("\n"); + dump_bytes_field(out, "message", this->message_ptr_, this->message_len_); } #ifdef USE_API_NOISE void NoiseEncryptionSetKeyRequest::dump_to(std::string &out) const { MessageDumpHelper helper(out, "NoiseEncryptionSetKeyRequest"); - out.append(" key: "); - out.append(format_hex_pretty(this->key, this->key_len)); - out.append("\n"); + dump_bytes_field(out, "key", this->key, this->key_len); } void NoiseEncryptionSetKeyResponse::dump_to(std::string &out) const { MessageDumpHelper helper(out, "NoiseEncryptionSetKeyResponse"); @@ -1189,9 +1195,7 @@ void HomeassistantActionResponse::dump_to(std::string &out) const { dump_field(out, "success", this->success); dump_field(out, "error_message", this->error_message); #ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON - out.append(" response_data: "); - out.append(format_hex_pretty(this->response_data, this->response_data_len)); - out.append("\n"); + dump_bytes_field(out, "response_data", this->response_data, this->response_data_len); #endif } #endif @@ -1278,9 +1282,7 @@ void ExecuteServiceResponse::dump_to(std::string &out) const { dump_field(out, "success", this->success); dump_field(out, "error_message", this->error_message); #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES_JSON - out.append(" response_data: "); - out.append(format_hex_pretty(this->response_data, this->response_data_len)); - out.append("\n"); + dump_bytes_field(out, "response_data", this->response_data, this->response_data_len); #endif } #endif @@ -1302,9 +1304,7 @@ void ListEntitiesCameraResponse::dump_to(std::string &out) const { void CameraImageResponse::dump_to(std::string &out) const { MessageDumpHelper helper(out, "CameraImageResponse"); dump_field(out, "key", this->key); - out.append(" data: "); - out.append(format_hex_pretty(this->data_ptr_, this->data_len_)); - out.append("\n"); + dump_bytes_field(out, "data", this->data_ptr_, this->data_len_); dump_field(out, "done", this->done); #ifdef USE_DEVICES dump_field(out, "device_id", this->device_id); @@ -1705,9 +1705,7 @@ void BluetoothLERawAdvertisement::dump_to(std::string &out) const { dump_field(out, "address", this->address); dump_field(out, "rssi", this->rssi); dump_field(out, "address_type", this->address_type); - out.append(" data: "); - out.append(format_hex_pretty(this->data, this->data_len)); - out.append("\n"); + dump_bytes_field(out, "data", this->data, this->data_len); } void BluetoothLERawAdvertisementsResponse::dump_to(std::string &out) const { MessageDumpHelper helper(out, "BluetoothLERawAdvertisementsResponse"); @@ -1792,18 +1790,14 @@ void BluetoothGATTReadResponse::dump_to(std::string &out) const { MessageDumpHelper helper(out, "BluetoothGATTReadResponse"); dump_field(out, "address", this->address); dump_field(out, "handle", this->handle); - out.append(" data: "); - out.append(format_hex_pretty(this->data_ptr_, this->data_len_)); - out.append("\n"); + dump_bytes_field(out, "data", this->data_ptr_, this->data_len_); } void BluetoothGATTWriteRequest::dump_to(std::string &out) const { MessageDumpHelper helper(out, "BluetoothGATTWriteRequest"); dump_field(out, "address", this->address); dump_field(out, "handle", this->handle); dump_field(out, "response", this->response); - out.append(" data: "); - out.append(format_hex_pretty(this->data, this->data_len)); - out.append("\n"); + dump_bytes_field(out, "data", this->data, this->data_len); } void BluetoothGATTReadDescriptorRequest::dump_to(std::string &out) const { MessageDumpHelper helper(out, "BluetoothGATTReadDescriptorRequest"); @@ -1814,9 +1808,7 @@ void BluetoothGATTWriteDescriptorRequest::dump_to(std::string &out) const { MessageDumpHelper helper(out, "BluetoothGATTWriteDescriptorRequest"); dump_field(out, "address", this->address); dump_field(out, "handle", this->handle); - out.append(" data: "); - out.append(format_hex_pretty(this->data, this->data_len)); - out.append("\n"); + dump_bytes_field(out, "data", this->data, this->data_len); } void BluetoothGATTNotifyRequest::dump_to(std::string &out) const { MessageDumpHelper helper(out, "BluetoothGATTNotifyRequest"); @@ -1828,9 +1820,7 @@ void BluetoothGATTNotifyDataResponse::dump_to(std::string &out) const { MessageDumpHelper helper(out, "BluetoothGATTNotifyDataResponse"); dump_field(out, "address", this->address); dump_field(out, "handle", this->handle); - out.append(" data: "); - out.append(format_hex_pretty(this->data_ptr_, this->data_len_)); - out.append("\n"); + dump_bytes_field(out, "data", this->data_ptr_, this->data_len_); } void SubscribeBluetoothConnectionsFreeRequest::dump_to(std::string &out) const { out.append("SubscribeBluetoothConnectionsFreeRequest {}"); @@ -1934,9 +1924,7 @@ void VoiceAssistantEventResponse::dump_to(std::string &out) const { } void VoiceAssistantAudio::dump_to(std::string &out) const { MessageDumpHelper helper(out, "VoiceAssistantAudio"); - out.append(" data: "); - out.append(format_hex_pretty(this->data, this->data_len)); - out.append("\n"); + dump_bytes_field(out, "data", this->data, this->data_len); dump_field(out, "end", this->end); } void VoiceAssistantTimerEventResponse::dump_to(std::string &out) const { @@ -2297,16 +2285,12 @@ void UpdateCommandRequest::dump_to(std::string &out) const { #ifdef USE_ZWAVE_PROXY void ZWaveProxyFrame::dump_to(std::string &out) const { MessageDumpHelper helper(out, "ZWaveProxyFrame"); - out.append(" data: "); - out.append(format_hex_pretty(this->data, this->data_len)); - out.append("\n"); + dump_bytes_field(out, "data", this->data, this->data_len); } void ZWaveProxyRequest::dump_to(std::string &out) const { MessageDumpHelper helper(out, "ZWaveProxyRequest"); dump_field(out, "type", static_cast(this->type)); - out.append(" data: "); - out.append(format_hex_pretty(this->data, this->data_len)); - out.append("\n"); + dump_bytes_field(out, "data", this->data, this->data_len); } #endif diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 118c87356e..a10a912186 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -786,10 +786,32 @@ class BytesType(TypeInfo): @property def dump_content(self) -> str: - o = f'out.append(" {self.name}: ");\n' - o += self.dump(f"this->{self.field_name}") + "\n" - o += 'out.append("\\n");' - return o + # For SOURCE_CLIENT only, always use std::string + if not self._needs_encode: + return ( + f'dump_bytes_field(out, "{self.name}", ' + f"reinterpret_cast(this->{self.field_name}.data()), " + f"this->{self.field_name}.size());" + ) + + # For SOURCE_SERVER, always use pointer/length + if not self._needs_decode: + return ( + f'dump_bytes_field(out, "{self.name}", ' + f"this->{self.field_name}_ptr_, this->{self.field_name}_len_);" + ) + + # For SOURCE_BOTH, check if pointer is set (sending) or use string (received) + return ( + f"if (this->{self.field_name}_ptr_ != nullptr) {{\n" + f' dump_bytes_field(out, "{self.name}", ' + f"this->{self.field_name}_ptr_, this->{self.field_name}_len_);\n" + f"}} else {{\n" + f' dump_bytes_field(out, "{self.name}", ' + f"reinterpret_cast(this->{self.field_name}.data()), " + f"this->{self.field_name}.size());\n" + f"}}" + ) def get_size_calculation(self, name: str, force: bool = False) -> str: return f"size.add_length({self.calculate_field_id_size()}, this->{self.field_name}_len_);" @@ -862,9 +884,8 @@ class PointerToBytesBufferType(PointerToBufferTypeBase): @property def dump_content(self) -> str: return ( - f'out.append(" {self.name}: ");\n' - + f"out.append({self.dump(self.field_name)});\n" - + 'out.append("\\n");' + f'dump_bytes_field(out, "{self.name}", ' + f"this->{self.field_name}, this->{self.field_name}_len);" ) def get_size_calculation(self, name: str, force: bool = False) -> str: @@ -1062,10 +1083,10 @@ class FixedArrayBytesType(TypeInfo): @property def dump_content(self) -> str: - o = f'out.append(" {self.name}: ");\n' - o += f"out.append(format_hex_pretty(this->{self.field_name}, this->{self.field_name}_len));\n" - o += 'out.append("\\n");' - return o + return ( + f'dump_bytes_field(out, "{self.name}", ' + f"this->{self.field_name}, this->{self.field_name}_len);" + ) def get_size_calculation(self, name: str, force: bool = False) -> str: # Use the actual length stored in the _len field @@ -2658,6 +2679,15 @@ static void dump_field(std::string &out, const char *field_name, T value, int in out.append("\\n"); } +// Helper for bytes fields - uses stack buffer to avoid heap allocation +// Buffer sized for 160 bytes of data (480 chars with separators) to fit typical log buffer +static void dump_bytes_field(std::string &out, const char *field_name, const uint8_t *data, size_t len, int indent = 2) { + char hex_buf[format_hex_pretty_size(160)]; + append_field_prefix(out, field_name, indent); + format_hex_pretty_to(hex_buf, data, len); + append_with_newline(out, hex_buf); +} + """ content += "namespace enums {\n\n"