From 92e9383164cb6df41c61fa2e3c235b7270613040 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 26 Jul 2025 21:18:58 -1000 Subject: [PATCH 1/4] light_opt_part2 --- esphome/components/light/light_call.cpp | 153 +++++++++++------- esphome/components/light/light_color_values.h | 25 +-- .../components/light/light_json_schema.cpp | 130 ++++++++------- esphome/components/light/light_state.cpp | 17 +- 4 files changed, 189 insertions(+), 136 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 1b856ad580..ac5a2e9fde 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -9,11 +9,28 @@ namespace light { static const char *const TAG = "light"; -// Helper function to reduce code size for validation warnings +// Helper functions to reduce code size for logging +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_WARN static void log_validation_warning(const char *name, const char *param_name, float val, float min, float max) { ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, param_name, val, min, max); } +static void log_feature_not_supported(const char *name, const char *feature) { + ESP_LOGW(TAG, "'%s': %s not supported", name, feature); +} + +static void log_color_mode_not_supported(const char *name, const char *feature) { + ESP_LOGW(TAG, "'%s': color mode does not support setting %s", name, feature); +} + +static void log_invalid_parameter(const char *name, const char *message) { ESP_LOGW(TAG, "'%s': %s", name, message); } +#else +#define log_validation_warning(name, param_name, val, min, max) +#define log_feature_not_supported(name, feature) +#define log_color_mode_not_supported(name, feature) +#define log_invalid_parameter(name, message) +#endif + // Macro to reduce repetitive setter code #define IMPLEMENT_LIGHT_CALL_SETTER(name, type, flag) \ LightCall &LightCall::set_##name(optional(name)) { \ @@ -49,11 +66,17 @@ static const LogString *color_mode_to_human(ColorMode color_mode) { return LOG_STR(""); } +// Helper to log percentage values +static inline void log_percent(const char *name, const char *param, float value) { + ESP_LOGD(TAG, " %s: %.0f%%", param, value * 100.0f); +} + void LightCall::perform() { const char *name = this->parent_->get_name().c_str(); LightColorValues v = this->validate_(); + const bool publish = this->get_publish_(); - if (this->get_publish_()) { + if (publish) { ESP_LOGD(TAG, "'%s' Setting:", name); // Only print color mode when it's being changed @@ -71,11 +94,11 @@ void LightCall::perform() { } if (this->has_brightness()) { - ESP_LOGD(TAG, " Brightness: %.0f%%", v.get_brightness() * 100.0f); + log_percent(name, "Brightness", v.get_brightness()); } if (this->has_color_brightness()) { - ESP_LOGD(TAG, " Color brightness: %.0f%%", v.get_color_brightness() * 100.0f); + log_percent(name, "Color brightness", v.get_color_brightness()); } if (this->has_red() || this->has_green() || this->has_blue()) { ESP_LOGD(TAG, " Red: %.0f%%, Green: %.0f%%, Blue: %.0f%%", v.get_red() * 100.0f, v.get_green() * 100.0f, @@ -83,7 +106,7 @@ void LightCall::perform() { } if (this->has_white()) { - ESP_LOGD(TAG, " White: %.0f%%", v.get_white() * 100.0f); + log_percent(name, "White", v.get_white()); } if (this->has_color_temperature()) { ESP_LOGD(TAG, " Color temperature: %.1f mireds", v.get_color_temperature()); @@ -97,26 +120,26 @@ void LightCall::perform() { if (this->has_flash_()) { // FLASH - if (this->get_publish_()) { + if (publish) { ESP_LOGD(TAG, " Flash length: %.1fs", this->flash_length_ / 1e3f); } - this->parent_->start_flash_(v, this->flash_length_, this->get_publish_()); + this->parent_->start_flash_(v, this->flash_length_, publish); } else if (this->has_transition_()) { // TRANSITION - if (this->get_publish_()) { + if (publish) { ESP_LOGD(TAG, " Transition length: %.1fs", this->transition_length_ / 1e3f); } // Special case: Transition and effect can be set when turning off if (this->has_effect_()) { - if (this->get_publish_()) { + if (publish) { ESP_LOGD(TAG, " Effect: 'None'"); } this->parent_->stop_effect_(); } - this->parent_->start_transition_(v, this->transition_length_, this->get_publish_()); + this->parent_->start_transition_(v, this->transition_length_, publish); } else if (this->has_effect_()) { // EFFECT @@ -127,7 +150,7 @@ void LightCall::perform() { effect_s = this->parent_->effects_[this->effect_ - 1]->get_name().c_str(); } - if (this->get_publish_()) { + if (publish) { ESP_LOGD(TAG, " Effect: '%s'", effect_s); } @@ -138,13 +161,13 @@ void LightCall::perform() { this->parent_->set_immediately_(v, true); } else { // INSTANT CHANGE - this->parent_->set_immediately_(v, this->get_publish_()); + this->parent_->set_immediately_(v, publish); } if (!this->has_transition_()) { this->parent_->target_state_reached_callback_.call(); } - if (this->get_publish_()) { + if (publish) { this->parent_->publish_state(); } if (this->get_save_()) { @@ -156,14 +179,27 @@ LightColorValues LightCall::validate_() { auto *name = this->parent_->get_name().c_str(); auto traits = this->parent_->get_traits(); + // Cache frequently used flags + const bool has_color_mode = this->has_color_mode(); + const bool has_state = this->has_state(); + const bool has_brightness = this->has_brightness(); + const bool has_color_brightness = this->has_color_brightness(); + const bool has_red = this->has_red(); + const bool has_green = this->has_green(); + const bool has_blue = this->has_blue(); + const bool has_white = this->has_white(); + const bool has_color_temperature = this->has_color_temperature(); + const bool has_cold_white = this->has_cold_white(); + const bool has_warm_white = this->has_warm_white(); + // Color mode check - if (this->has_color_mode() && !traits.supports_color_mode(this->color_mode_)) { + if (has_color_mode && !traits.supports_color_mode(this->color_mode_)) { ESP_LOGW(TAG, "'%s' does not support color mode %s", name, LOG_STR_ARG(color_mode_to_human(this->color_mode_))); this->set_flag_(FLAG_HAS_COLOR_MODE, false); } // Ensure there is always a color mode set - if (!this->has_color_mode()) { + if (!has_color_mode) { this->color_mode_ = this->compute_color_mode_(); this->set_flag_(FLAG_HAS_COLOR_MODE, true); } @@ -173,28 +209,27 @@ LightColorValues LightCall::validate_() { this->transform_parameters_(); // Brightness exists check - if (this->has_brightness() && this->brightness_ > 0.0f && !(color_mode & ColorCapability::BRIGHTNESS)) { - ESP_LOGW(TAG, "'%s': setting brightness not supported", name); + if (has_brightness && this->brightness_ > 0.0f && !(color_mode & ColorCapability::BRIGHTNESS)) { + log_feature_not_supported(name, "brightness"); this->set_flag_(FLAG_HAS_BRIGHTNESS, false); } // Transition length possible check if (this->has_transition_() && this->transition_length_ != 0 && !(color_mode & ColorCapability::BRIGHTNESS)) { - ESP_LOGW(TAG, "'%s': transitions not supported", name); + log_feature_not_supported(name, "transitions"); this->set_flag_(FLAG_HAS_TRANSITION, false); } // Color brightness exists check - if (this->has_color_brightness() && this->color_brightness_ > 0.0f && !(color_mode & ColorCapability::RGB)) { - ESP_LOGW(TAG, "'%s': color mode does not support setting RGB brightness", name); + if (has_color_brightness && this->color_brightness_ > 0.0f && !(color_mode & ColorCapability::RGB)) { + log_color_mode_not_supported(name, "RGB brightness"); this->set_flag_(FLAG_HAS_COLOR_BRIGHTNESS, false); } // RGB exists check - if ((this->has_red() && this->red_ > 0.0f) || (this->has_green() && this->green_ > 0.0f) || - (this->has_blue() && this->blue_ > 0.0f)) { + if ((has_red && this->red_ > 0.0f) || (has_green && this->green_ > 0.0f) || (has_blue && this->blue_ > 0.0f)) { if (!(color_mode & ColorCapability::RGB)) { - ESP_LOGW(TAG, "'%s': color mode does not support setting RGB color", name); + log_color_mode_not_supported(name, "RGB color"); this->set_flag_(FLAG_HAS_RED, false); this->set_flag_(FLAG_HAS_GREEN, false); this->set_flag_(FLAG_HAS_BLUE, false); @@ -202,23 +237,23 @@ LightColorValues LightCall::validate_() { } // White value exists check - if (this->has_white() && this->white_ > 0.0f && + if (has_white && this->white_ > 0.0f && !(color_mode & ColorCapability::WHITE || color_mode & ColorCapability::COLD_WARM_WHITE)) { - ESP_LOGW(TAG, "'%s': color mode does not support setting white value", name); + log_color_mode_not_supported(name, "white value"); this->set_flag_(FLAG_HAS_WHITE, false); } // Color temperature exists check - if (this->has_color_temperature() && + if (has_color_temperature && !(color_mode & ColorCapability::COLOR_TEMPERATURE || color_mode & ColorCapability::COLD_WARM_WHITE)) { - ESP_LOGW(TAG, "'%s': color mode does not support setting color temperature", name); + log_color_mode_not_supported(name, "color temperature"); this->set_flag_(FLAG_HAS_COLOR_TEMPERATURE, false); } // Cold/warm white value exists check - if ((this->has_cold_white() && this->cold_white_ > 0.0f) || (this->has_warm_white() && this->warm_white_ > 0.0f)) { + if ((has_cold_white && this->cold_white_ > 0.0f) || (has_warm_white && this->warm_white_ > 0.0f)) { if (!(color_mode & ColorCapability::COLD_WARM_WHITE)) { - ESP_LOGW(TAG, "'%s': color mode does not support setting cold/warm white value", name); + log_color_mode_not_supported(name, "cold/warm white value"); this->set_flag_(FLAG_HAS_COLD_WHITE, false); this->set_flag_(FLAG_HAS_WARM_WHITE, false); } @@ -246,18 +281,18 @@ LightColorValues LightCall::validate_() { VALIDATE_RANGE_(color_temperature, "Color temperature", traits.get_min_mireds(), traits.get_max_mireds()) // Flag whether an explicit turn off was requested, in which case we'll also stop the effect. - bool explicit_turn_off_request = this->has_state() && !this->state_; + bool explicit_turn_off_request = has_state && !this->state_; // Turn off when brightness is set to zero, and reset brightness (so that it has nonzero brightness when turned on). - if (this->has_brightness() && this->brightness_ == 0.0f) { + if (has_brightness && this->brightness_ == 0.0f) { this->state_ = false; this->set_flag_(FLAG_HAS_STATE, true); this->brightness_ = 1.0f; } // Set color brightness to 100% if currently zero and a color is set. - if (this->has_red() || this->has_green() || this->has_blue()) { - if (!this->has_color_brightness() && this->parent_->remote_values.get_color_brightness() == 0.0f) { + if (has_red || has_green || has_blue) { + if (!has_color_brightness && this->parent_->remote_values.get_color_brightness() == 0.0f) { this->color_brightness_ = 1.0f; this->set_flag_(FLAG_HAS_COLOR_BRIGHTNESS, true); } @@ -265,34 +300,34 @@ LightColorValues LightCall::validate_() { // Create color values for the light with this call applied. auto v = this->parent_->remote_values; - if (this->has_color_mode()) + if (has_color_mode) v.set_color_mode(this->color_mode_); - if (this->has_state()) + if (has_state) v.set_state(this->state_); - if (this->has_brightness()) + if (has_brightness) v.set_brightness(this->brightness_); - if (this->has_color_brightness()) + if (has_color_brightness) v.set_color_brightness(this->color_brightness_); - if (this->has_red()) + if (has_red) v.set_red(this->red_); - if (this->has_green()) + if (has_green) v.set_green(this->green_); - if (this->has_blue()) + if (has_blue) v.set_blue(this->blue_); - if (this->has_white()) + if (has_white) v.set_white(this->white_); - if (this->has_color_temperature()) + if (has_color_temperature) v.set_color_temperature(this->color_temperature_); - if (this->has_cold_white()) + if (has_cold_white) v.set_cold_white(this->cold_white_); - if (this->has_warm_white()) + if (has_warm_white) v.set_warm_white(this->warm_white_); v.normalize_color(); // Flash length check if (this->has_flash_() && this->flash_length_ == 0) { - ESP_LOGW(TAG, "'%s': flash length must be greater than zero", name); + log_invalid_parameter(name, "flash length must be greater than zero"); this->set_flag_(FLAG_HAS_FLASH, false); } @@ -311,13 +346,13 @@ LightColorValues LightCall::validate_() { } if (this->has_effect_() && (this->has_transition_() || this->has_flash_())) { - ESP_LOGW(TAG, "'%s': effect cannot be used with transition/flash", name); + log_invalid_parameter(name, "effect cannot be used with transition/flash"); this->set_flag_(FLAG_HAS_TRANSITION, false); this->set_flag_(FLAG_HAS_FLASH, false); } if (this->has_flash_() && this->has_transition_()) { - ESP_LOGW(TAG, "'%s': flash cannot be used with transition", name); + log_invalid_parameter(name, "flash cannot be used with transition"); this->set_flag_(FLAG_HAS_TRANSITION, false); } @@ -334,17 +369,17 @@ LightColorValues LightCall::validate_() { } if (this->has_transition_() && !supports_transition) { - ESP_LOGW(TAG, "'%s': transitions not supported", name); + log_feature_not_supported(name, "transitions"); this->set_flag_(FLAG_HAS_TRANSITION, false); } // If not a flash and turning the light off, then disable the light // Do not use light color values directly, so that effects can set 0% brightness // Reason: When user turns off the light in frontend, the effect should also stop - bool target_state = this->has_state() ? this->state_ : v.is_on(); + bool target_state = has_state ? this->state_ : v.is_on(); if (!this->has_flash_() && !target_state) { if (this->has_effect_()) { - ESP_LOGW(TAG, "'%s': cannot start effect when turning off", name); + log_invalid_parameter(name, "cannot start effect when turning off"); this->set_flag_(FLAG_HAS_EFFECT, false); } else if (this->parent_->active_effect_index_ != 0 && explicit_turn_off_request) { // Auto turn off effect @@ -368,21 +403,27 @@ void LightCall::transform_parameters_() { // - RGBWW lights with color_interlock=true, which also sets "brightness" and // "color_temperature" (without color_interlock, CW/WW are set directly) // - Legacy Home Assistant (pre-colormode), which sets "white" and "color_temperature" + + // Cache min/max mireds to avoid repeated calls + const float min_mireds = traits.get_min_mireds(); + const float max_mireds = traits.get_max_mireds(); + if (((this->has_white() && this->white_ > 0.0f) || this->has_color_temperature()) && // (this->color_mode_ & ColorCapability::COLD_WARM_WHITE) && // !(this->color_mode_ & ColorCapability::WHITE) && // !(this->color_mode_ & ColorCapability::COLOR_TEMPERATURE) && // - traits.get_min_mireds() > 0.0f && traits.get_max_mireds() > 0.0f) { + min_mireds > 0.0f && max_mireds > 0.0f) { ESP_LOGD(TAG, "'%s': setting cold/warm white channels using white/color temperature values", this->parent_->get_name().c_str()); if (this->has_color_temperature()) { - const float color_temp = clamp(this->color_temperature_, traits.get_min_mireds(), traits.get_max_mireds()); - const float ww_fraction = - (color_temp - traits.get_min_mireds()) / (traits.get_max_mireds() - traits.get_min_mireds()); + const float color_temp = clamp(this->color_temperature_, min_mireds, max_mireds); + const float range = max_mireds - min_mireds; + const float ww_fraction = (color_temp - min_mireds) / range; const float cw_fraction = 1.0f - ww_fraction; const float max_cw_ww = std::max(ww_fraction, cw_fraction); - this->cold_white_ = gamma_uncorrect(cw_fraction / max_cw_ww, this->parent_->get_gamma_correct()); - this->warm_white_ = gamma_uncorrect(ww_fraction / max_cw_ww, this->parent_->get_gamma_correct()); + const float gamma = this->parent_->get_gamma_correct(); + this->cold_white_ = gamma_uncorrect(cw_fraction / max_cw_ww, gamma); + this->warm_white_ = gamma_uncorrect(ww_fraction / max_cw_ww, gamma); this->set_flag_(FLAG_HAS_COLD_WHITE, true); this->set_flag_(FLAG_HAS_WARM_WHITE, true); } diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 5653a8d2a5..9a37f6b424 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -84,18 +84,21 @@ class LightColorValues { * @return The linearly interpolated LightColorValues. */ static LightColorValues lerp(const LightColorValues &start, const LightColorValues &end, float completion) { + // Directly interpolate the raw values to avoid getter/setter overhead + // Linear interpolation between two clamped values produces a clamped result, + // so we can skip the setters which include redundant clamping logic LightColorValues v; - v.set_color_mode(end.color_mode_); - v.set_state(std::lerp(start.get_state(), end.get_state(), completion)); - v.set_brightness(std::lerp(start.get_brightness(), end.get_brightness(), completion)); - v.set_color_brightness(std::lerp(start.get_color_brightness(), end.get_color_brightness(), completion)); - v.set_red(std::lerp(start.get_red(), end.get_red(), completion)); - v.set_green(std::lerp(start.get_green(), end.get_green(), completion)); - v.set_blue(std::lerp(start.get_blue(), end.get_blue(), completion)); - v.set_white(std::lerp(start.get_white(), end.get_white(), completion)); - v.set_color_temperature(std::lerp(start.get_color_temperature(), end.get_color_temperature(), completion)); - v.set_cold_white(std::lerp(start.get_cold_white(), end.get_cold_white(), completion)); - v.set_warm_white(std::lerp(start.get_warm_white(), end.get_warm_white(), completion)); + v.color_mode_ = end.color_mode_; + v.state_ = std::lerp(start.state_, end.state_, completion); + v.brightness_ = std::lerp(start.brightness_, end.brightness_, completion); + v.color_brightness_ = std::lerp(start.color_brightness_, end.color_brightness_, completion); + v.red_ = std::lerp(start.red_, end.red_, completion); + v.green_ = std::lerp(start.green_, end.green_, completion); + v.blue_ = std::lerp(start.blue_, end.blue_, completion); + v.white_ = std::lerp(start.white_, end.white_, completion); + v.color_temperature_ = std::lerp(start.color_temperature_, end.color_temperature_, completion); + v.cold_white_ = std::lerp(start.cold_white_, end.cold_white_, completion); + v.warm_white_ = std::lerp(start.warm_white_, end.warm_white_, completion); return v; } diff --git a/esphome/components/light/light_json_schema.cpp b/esphome/components/light/light_json_schema.cpp index 26615bae5c..84e1ee9f1d 100644 --- a/esphome/components/light/light_json_schema.cpp +++ b/esphome/components/light/light_json_schema.cpp @@ -8,6 +8,46 @@ namespace light { // See https://www.home-assistant.io/integrations/light.mqtt/#json-schema for documentation on the schema +// Helper to convert float 0-1 to uint8_t 0-255 +static inline uint8_t to_uint8_scaled(float value) { return uint8_t(value * 255); } + +// Helper to parse color component from JSON +static float parse_color_component(JsonObject &color, const char *key, LightCall &call, + LightCall &(LightCall::*setter)(float) ) { + if (color[key].is()) { + float val = float(color[key]) / 255.0f; + (call.*setter)(val); + return val; + } + return 0.0f; +} + +// Lookup table for color mode strings +static const char *get_color_mode_json_str(ColorMode mode) { + switch (mode) { + case ColorMode::ON_OFF: + return "onoff"; + case ColorMode::BRIGHTNESS: + return "brightness"; + case ColorMode::WHITE: + return "white"; // not supported by HA in MQTT + case ColorMode::COLOR_TEMPERATURE: + return "color_temp"; + case ColorMode::COLD_WARM_WHITE: + return "cwww"; // not supported by HA + case ColorMode::RGB: + return "rgb"; + case ColorMode::RGB_WHITE: + return "rgbw"; + case ColorMode::RGB_COLOR_TEMPERATURE: + return "rgbct"; // not supported by HA + case ColorMode::RGB_COLD_WARM_WHITE: + return "rgbww"; + default: + return nullptr; + } +} + void LightJSONSchema::dump_json(LightState &state, JsonObject root) { // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) false positive with ArduinoJson if (state.supports_effects()) @@ -16,60 +56,36 @@ void LightJSONSchema::dump_json(LightState &state, JsonObject root) { auto values = state.remote_values; auto traits = state.get_output()->get_traits(); - switch (values.get_color_mode()) { - case ColorMode::UNKNOWN: // don't need to set color mode if we don't know it - break; - case ColorMode::ON_OFF: - root["color_mode"] = "onoff"; - break; - case ColorMode::BRIGHTNESS: - root["color_mode"] = "brightness"; - break; - case ColorMode::WHITE: // not supported by HA in MQTT - root["color_mode"] = "white"; - break; - case ColorMode::COLOR_TEMPERATURE: - root["color_mode"] = "color_temp"; - break; - case ColorMode::COLD_WARM_WHITE: // not supported by HA - root["color_mode"] = "cwww"; - break; - case ColorMode::RGB: - root["color_mode"] = "rgb"; - break; - case ColorMode::RGB_WHITE: - root["color_mode"] = "rgbw"; - break; - case ColorMode::RGB_COLOR_TEMPERATURE: // not supported by HA - root["color_mode"] = "rgbct"; - break; - case ColorMode::RGB_COLD_WARM_WHITE: - root["color_mode"] = "rgbww"; - break; + const auto color_mode = values.get_color_mode(); + const char *mode_str = get_color_mode_json_str(color_mode); + if (mode_str != nullptr) { + root["color_mode"] = mode_str; } - if (values.get_color_mode() & ColorCapability::ON_OFF) + if (color_mode & ColorCapability::ON_OFF) root["state"] = (values.get_state() != 0.0f) ? "ON" : "OFF"; - if (values.get_color_mode() & ColorCapability::BRIGHTNESS) - root["brightness"] = uint8_t(values.get_brightness() * 255); + if (color_mode & ColorCapability::BRIGHTNESS) + root["brightness"] = to_uint8_scaled(values.get_brightness()); JsonObject color = root["color"].to(); - if (values.get_color_mode() & ColorCapability::RGB) { - color["r"] = uint8_t(values.get_color_brightness() * values.get_red() * 255); - color["g"] = uint8_t(values.get_color_brightness() * values.get_green() * 255); - color["b"] = uint8_t(values.get_color_brightness() * values.get_blue() * 255); + if (color_mode & ColorCapability::RGB) { + float color_brightness = values.get_color_brightness(); + color["r"] = to_uint8_scaled(color_brightness * values.get_red()); + color["g"] = to_uint8_scaled(color_brightness * values.get_green()); + color["b"] = to_uint8_scaled(color_brightness * values.get_blue()); } - if (values.get_color_mode() & ColorCapability::WHITE) { - color["w"] = uint8_t(values.get_white() * 255); - root["white_value"] = uint8_t(values.get_white() * 255); // legacy API + if (color_mode & ColorCapability::WHITE) { + uint8_t white_val = to_uint8_scaled(values.get_white()); + color["w"] = white_val; + root["white_value"] = white_val; // legacy API } - if (values.get_color_mode() & ColorCapability::COLOR_TEMPERATURE) { + if (color_mode & ColorCapability::COLOR_TEMPERATURE) { // this one isn't under the color subkey for some reason root["color_temp"] = uint32_t(values.get_color_temperature()); } - if (values.get_color_mode() & ColorCapability::COLD_WARM_WHITE) { - color["c"] = uint8_t(values.get_cold_white() * 255); - color["w"] = uint8_t(values.get_warm_white() * 255); + if (color_mode & ColorCapability::COLD_WARM_WHITE) { + color["c"] = to_uint8_scaled(values.get_cold_white()); + color["w"] = to_uint8_scaled(values.get_warm_white()); } } @@ -99,22 +115,16 @@ void LightJSONSchema::parse_color_json(LightState &state, LightCall &call, JsonO JsonObject color = root["color"]; // HA also encodes brightness information in the r, g, b values, so extract that and set it as color brightness. float max_rgb = 0.0f; - if (color["r"].is()) { - float r = float(color["r"]) / 255.0f; - max_rgb = fmaxf(max_rgb, r); - call.set_red(r); - } - if (color["g"].is()) { - float g = float(color["g"]) / 255.0f; - max_rgb = fmaxf(max_rgb, g); - call.set_green(g); - } - if (color["b"].is()) { - float b = float(color["b"]) / 255.0f; - max_rgb = fmaxf(max_rgb, b); - call.set_blue(b); - } - if (color["r"].is() || color["g"].is() || color["b"].is()) { + + float r = parse_color_component(color, "r", call, &LightCall::set_red); + float g = parse_color_component(color, "g", call, &LightCall::set_green); + float b = parse_color_component(color, "b", call, &LightCall::set_blue); + + max_rgb = fmaxf(max_rgb, r); + max_rgb = fmaxf(max_rgb, g); + max_rgb = fmaxf(max_rgb, b); + + if (max_rgb > 0.0f) { call.set_color_brightness(max_rgb); } diff --git a/esphome/components/light/light_state.cpp b/esphome/components/light/light_state.cpp index fd0aafe4c6..5b57707d6b 100644 --- a/esphome/components/light/light_state.cpp +++ b/esphome/components/light/light_state.cpp @@ -24,7 +24,8 @@ void LightState::setup() { } // When supported color temperature range is known, initialize color temperature setting within bounds. - float min_mireds = this->get_traits().get_min_mireds(); + auto traits = this->get_traits(); + float min_mireds = traits.get_min_mireds(); if (min_mireds > 0) { this->remote_values.set_color_temperature(min_mireds); this->current_values.set_color_temperature(min_mireds); @@ -43,11 +44,8 @@ void LightState::setup() { this->rtc_ = global_preferences->make_preference(this->get_object_id_hash()); // Attempt to load from preferences, else fall back to default values if (!this->rtc_.load(&recovered)) { - recovered.state = false; - if (this->restore_mode_ == LIGHT_RESTORE_DEFAULT_ON || - this->restore_mode_ == LIGHT_RESTORE_INVERTED_DEFAULT_ON) { - recovered.state = true; - } + recovered.state = (this->restore_mode_ == LIGHT_RESTORE_DEFAULT_ON || + this->restore_mode_ == LIGHT_RESTORE_INVERTED_DEFAULT_ON); } else if (this->restore_mode_ == LIGHT_RESTORE_INVERTED_DEFAULT_OFF || this->restore_mode_ == LIGHT_RESTORE_INVERTED_DEFAULT_ON) { // Inverted restore state @@ -88,17 +86,18 @@ void LightState::setup() { } void LightState::dump_config() { ESP_LOGCONFIG(TAG, "Light '%s'", this->get_name().c_str()); - if (this->get_traits().supports_color_capability(ColorCapability::BRIGHTNESS)) { + auto traits = this->get_traits(); + if (traits.supports_color_capability(ColorCapability::BRIGHTNESS)) { ESP_LOGCONFIG(TAG, " Default Transition Length: %.1fs\n" " Gamma Correct: %.2f", this->default_transition_length_ / 1e3f, this->gamma_correct_); } - if (this->get_traits().supports_color_capability(ColorCapability::COLOR_TEMPERATURE)) { + if (traits.supports_color_capability(ColorCapability::COLOR_TEMPERATURE)) { ESP_LOGCONFIG(TAG, " Min Mireds: %.1f\n" " Max Mireds: %.1f", - this->get_traits().get_min_mireds(), this->get_traits().get_max_mireds()); + traits.get_min_mireds(), traits.get_max_mireds()); } } void LightState::loop() { From 10434ac2a3c23fbef7991b21196b6ce22d5da3b2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 26 Jul 2025 21:24:24 -1000 Subject: [PATCH 2/4] fixes --- .../components/light/light_json_schema.cpp | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/esphome/components/light/light_json_schema.cpp b/esphome/components/light/light_json_schema.cpp index 84e1ee9f1d..20127fab33 100644 --- a/esphome/components/light/light_json_schema.cpp +++ b/esphome/components/light/light_json_schema.cpp @@ -12,14 +12,14 @@ namespace light { static inline uint8_t to_uint8_scaled(float value) { return uint8_t(value * 255); } // Helper to parse color component from JSON -static float parse_color_component(JsonObject &color, const char *key, LightCall &call, - LightCall &(LightCall::*setter)(float) ) { +static bool parse_color_component(JsonObject &color, const char *key, LightCall &call, + LightCall &(LightCall::*setter)(float), float &out_value) { if (color[key].is()) { - float val = float(color[key]) / 255.0f; - (call.*setter)(val); - return val; + out_value = float(color[key]) / 255.0f; + (call.*setter)(out_value); + return true; } - return 0.0f; + return false; } // Lookup table for color mode strings @@ -115,16 +115,23 @@ void LightJSONSchema::parse_color_json(LightState &state, LightCall &call, JsonO JsonObject color = root["color"]; // HA also encodes brightness information in the r, g, b values, so extract that and set it as color brightness. float max_rgb = 0.0f; + bool has_rgb = false; + float r, g, b; - float r = parse_color_component(color, "r", call, &LightCall::set_red); - float g = parse_color_component(color, "g", call, &LightCall::set_green); - float b = parse_color_component(color, "b", call, &LightCall::set_blue); + if (parse_color_component(color, "r", call, &LightCall::set_red, r)) { + max_rgb = fmaxf(max_rgb, r); + has_rgb = true; + } + if (parse_color_component(color, "g", call, &LightCall::set_green, g)) { + max_rgb = fmaxf(max_rgb, g); + has_rgb = true; + } + if (parse_color_component(color, "b", call, &LightCall::set_blue, b)) { + max_rgb = fmaxf(max_rgb, b); + has_rgb = true; + } - max_rgb = fmaxf(max_rgb, r); - max_rgb = fmaxf(max_rgb, g); - max_rgb = fmaxf(max_rgb, b); - - if (max_rgb > 0.0f) { + if (has_rgb) { call.set_color_brightness(max_rgb); } From 29e61c8913f2316e2c9180aeeb95cdcc49d4f64f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 26 Jul 2025 21:27:44 -1000 Subject: [PATCH 3/4] revert --- .../components/light/light_json_schema.cpp | 32 ++++++------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/esphome/components/light/light_json_schema.cpp b/esphome/components/light/light_json_schema.cpp index 20127fab33..7af6dadd12 100644 --- a/esphome/components/light/light_json_schema.cpp +++ b/esphome/components/light/light_json_schema.cpp @@ -11,17 +11,6 @@ namespace light { // Helper to convert float 0-1 to uint8_t 0-255 static inline uint8_t to_uint8_scaled(float value) { return uint8_t(value * 255); } -// Helper to parse color component from JSON -static bool parse_color_component(JsonObject &color, const char *key, LightCall &call, - LightCall &(LightCall::*setter)(float), float &out_value) { - if (color[key].is()) { - out_value = float(color[key]) / 255.0f; - (call.*setter)(out_value); - return true; - } - return false; -} - // Lookup table for color mode strings static const char *get_color_mode_json_str(ColorMode mode) { switch (mode) { @@ -115,23 +104,22 @@ void LightJSONSchema::parse_color_json(LightState &state, LightCall &call, JsonO JsonObject color = root["color"]; // HA also encodes brightness information in the r, g, b values, so extract that and set it as color brightness. float max_rgb = 0.0f; - bool has_rgb = false; - float r, g, b; - - if (parse_color_component(color, "r", call, &LightCall::set_red, r)) { + if (color["r"].is()) { + float r = float(color["r"]) / 255.0f; max_rgb = fmaxf(max_rgb, r); - has_rgb = true; + call.set_red(r); } - if (parse_color_component(color, "g", call, &LightCall::set_green, g)) { + if (color["g"].is()) { + float g = float(color["g"]) / 255.0f; max_rgb = fmaxf(max_rgb, g); - has_rgb = true; + call.set_green(g); } - if (parse_color_component(color, "b", call, &LightCall::set_blue, b)) { + if (color["b"].is()) { + float b = float(color["b"]) / 255.0f; max_rgb = fmaxf(max_rgb, b); - has_rgb = true; + call.set_blue(b); } - - if (has_rgb) { + if (color["r"].is() || color["g"].is() || color["b"].is()) { call.set_color_brightness(max_rgb); } From 28dbf3bbcc3ef45bf60065db283b135811dbb8d9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 26 Jul 2025 21:32:34 -1000 Subject: [PATCH 4/4] revert --- esphome/components/light/light_color_values.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 9a37f6b424..04d7d1e7d8 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -84,9 +84,11 @@ class LightColorValues { * @return The linearly interpolated LightColorValues. */ static LightColorValues lerp(const LightColorValues &start, const LightColorValues &end, float completion) { - // Directly interpolate the raw values to avoid getter/setter overhead - // Linear interpolation between two clamped values produces a clamped result, - // so we can skip the setters which include redundant clamping logic + // Directly interpolate the raw values to avoid getter/setter overhead. + // This is safe because: + // - All LightColorValues have their values clamped when set via the setters + // - std::lerp guarantees output is in the same range as inputs + // - Therefore the output doesn't need clamping, so we can skip the setters LightColorValues v; v.color_mode_ = end.color_mode_; v.state_ = std::lerp(start.state_, end.state_, completion);