[light] Return StringRef from LightEffect::get_name() and LightState::get_effect_name() (#13105)

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;

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();

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:

esphome/components/light/light_call.cpp

@@ -1,4 +1,5 @@
 #include <cinttypes>
+
 #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;

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() {}

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();
   }

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;
 }
 

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.

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<JsonArray>();
-    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"));
   }
 }

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"));
     }

esphome/core/helpers.cpp

@@ -162,6 +162,9 @@ float random_float() { return static_cast<float>(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); }

esphome/core/helpers.h

@@ -511,6 +511,8 @@ template<typename T> 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);

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: