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: