From 775c6a077d8bc8bfdc78e307db32388a0f3e6bac Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Jan 2026 08:35:37 -1000 Subject: [PATCH] [light] Return std::string_view from LightEffect::get_name() and LightState::get_effect_name() --- esphome/components/api/api_connection.cpp | 3 ++- esphome/components/e131/e131.cpp | 10 ++++++---- esphome/components/light/light_call.cpp | 12 ++++++------ esphome/components/light/light_effect.h | 6 ++++-- esphome/components/light/light_state.cpp | 5 +++-- esphome/components/light/light_state.h | 10 ++++++---- esphome/components/mqtt/mqtt_light.cpp | 3 ++- .../components/prometheus/prometheus_handler.cpp | 5 +++-- esphome/core/helpers.cpp | 3 +++ esphome/core/helpers.h | 2 ++ tests/components/light/common.yaml | 15 +++++++++++++++ 11 files changed, 52 insertions(+), 22 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index fb3548d117..1323cbd183 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -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()); + // data() is safe as effect names are null-terminated strings from codegen + effects_list.push_back(effect->get_name().data()); } } msg.effects = &effects_list; diff --git a/esphome/components/e131/e131.cpp b/esphome/components/e131/e131.cpp index c10c88faf2..c45651bf53 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.data(), + 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.data(), + 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/light/light_call.cpp b/esphome/components/light/light_call.cpp index 8161e8b814..de2f7a171a 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -1,4 +1,6 @@ #include +#include + #include "light_call.h" #include "light_state.h" #include "esphome/core/log.h" @@ -153,7 +155,7 @@ void LightCall::perform() { } else if (this->has_effect_()) { // EFFECT - const char *effect_s; + std::string_view effect_s; if (this->effect_ == 0u) { effect_s = "None"; } else { @@ -161,7 +163,7 @@ void LightCall::perform() { } if (publish) { - ESP_LOGD(TAG, " Effect: '%s'", effect_s); + ESP_LOGD(TAG, " Effect: '%.*s'", (int) effect_s.size(), effect_s.data()); } this->parent_->start_effect_(this->effect_); @@ -511,11 +513,9 @@ LightCall &LightCall::set_effect(const char *effect, size_t len) { } bool found = false; + std::string_view effect_sv(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_sv, 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..9a09c6d63e 100644 --- a/esphome/components/light/light_effect.h +++ b/esphome/components/light/light_effect.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "esphome/core/component.h" namespace esphome::light { @@ -23,9 +25,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_; } + std::string_view get_name() const { return 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_state.cpp b/esphome/components/light/light_state.cpp index 5a50bae50b..c6f337dc75 100644 --- a/esphome/components/light/light_state.cpp +++ b/esphome/components/light/light_state.cpp @@ -165,7 +165,7 @@ 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() { +std::string_view LightState::get_effect_name() { if (this->active_effect_index_ > 0) { return this->effects_[this->active_effect_index_ - 1]->get_name(); } @@ -174,7 +174,8 @@ std::string LightState::get_effect_name() { StringRef LightState::get_effect_name_ref() { if (this->active_effect_index_ > 0) { - return StringRef(this->effects_[this->active_effect_index_ - 1]->get_name()); + auto name = this->effects_[this->active_effect_index_ - 1]->get_name(); + return StringRef(name.data(), name.size()); } return EFFECT_NONE_REF; } diff --git a/esphome/components/light/light_state.h b/esphome/components/light/light_state.h index a21c2c7693..bd2bec0df6 100644 --- a/esphome/components/light/light_state.h +++ b/esphome/components/light/light_state.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "esphome/core/component.h" #include "esphome/core/entity_base.h" #include "esphome/core/optional.h" @@ -140,7 +142,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(); + std::string_view get_effect_name(); /// Return the name of the current effect as StringRef (for API usage) StringRef get_effect_name_ref(); @@ -191,11 +193,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, std::string("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(std::string_view(effect_name), this->effects_[i]->get_name())) { return i + 1; // Effects are 1-indexed in active_effect_index_ } } @@ -218,7 +220,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..164b55a9b1 100644 --- a/esphome/components/mqtt/mqtt_light.cpp +++ b/esphome/components/mqtt/mqtt_light.cpp @@ -81,7 +81,8 @@ void MQTTJSONLightComponent::send_discovery(JsonObject root, mqtt::SendDiscovery 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()); + // data() is safe as effect names are null-terminated strings from codegen + effect_list.add(effect->get_name().data()); effect_list.add(ESPHOME_F("None")); } } diff --git a/esphome/components/prometheus/prometheus_handler.cpp b/esphome/components/prometheus/prometheus_handler.cpp index 88b357041a..79727b828c 100644 --- a/esphome/components/prometheus/prometheus_handler.cpp +++ b/esphome/components/prometheus/prometheus_handler.cpp @@ -363,14 +363,15 @@ 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(); + std::string_view 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 { - stream->print(effect.c_str()); + // data() is safe as effect names are null-terminated strings from codegen + stream->print(effect.data()); stream->print(ESPHOME_F("\"} 1\n")); } } diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 8671dc7f82..1a0692a386 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(std::string_view a, std::string_view b) { + return a.size() == b.size() && strncasecmp(a.data(), b.data(), 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 acba420d3e..20b490fb27 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 string_views for equality in case-insensitive manner. +bool str_equals_case_insensitive(std::string_view a, std::string_view 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..107df13894 100644 --- a/tests/components/light/common.yaml +++ b/tests/components/light/common.yaml @@ -1,6 +1,21 @@ esphome: on_boot: then: + # Test LightEffect::get_name() returns string_view + - lambda: |- + // Test get_name() returns string_view + auto &effects = id(test_monochromatic_light).get_effects(); + if (!effects.empty()) { + std::string_view name = effects[0]->get_name(); + // Test comparison with string literal + if (name == "Strobe") { + ESP_LOGI("test", "Found Strobe effect"); + } + // Safe logging with size-limited format + ESP_LOGI("test", "Effect name: %.*s", (int) name.size(), name.data()); + // Test .data() for null-terminated functions (safe because names are from codegen) + ESP_LOGI("test", "Effect: %s", name.data()); + } - light.toggle: test_binary_light - light.turn_off: test_rgb_light - light.turn_on: