From 0d104776bc1465562e89f7197e0530ce37681ddd Mon Sep 17 00:00:00 2001 From: Oxan van Leeuwen Date: Thu, 5 Aug 2021 01:28:39 +0200 Subject: [PATCH] Various follow-up fixes to color mode changes (#2118) --- esphome/components/light/light_call.cpp | 34 +++++++++++++------ esphome/components/light/light_call.h | 1 + esphome/components/light/light_color_values.h | 16 ++------- esphome/components/light/light_effect.h | 2 -- esphome/components/light/light_json_schema.h | 7 ++-- esphome/components/light/light_output.h | 2 -- esphome/components/light/light_state.h | 5 +++ esphome/components/light/light_transformer.h | 2 ++ 8 files changed, 38 insertions(+), 31 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 536018c33b..cb0a7bb711 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -53,6 +53,9 @@ void LightCall::perform() { ESP_LOGD(TAG, " Brightness: %.0f%%", v.get_brightness() * 100.0f); } + if (this->color_brightness_.has_value()) { + ESP_LOGD(TAG, " Color brightness: %.0f%%", v.get_color_brightness() * 100.0f); + } if (this->red_.has_value() || this->green_.has_value() || this->blue_.has_value()) { ESP_LOGD(TAG, " Red: %.0f%%, Green: %.0f%%, Blue: %.0f%%", v.get_red() * 100.0f, v.get_green() * 100.0f, v.get_blue() * 100.0f); @@ -149,7 +152,7 @@ LightColorValues LightCall::validate_() { this->transform_parameters_(); // Brightness exists check - if (this->brightness_.has_value() && !(color_mode & ColorCapability::BRIGHTNESS)) { + if (this->brightness_.has_value() && *this->brightness_ > 0.0f && !(color_mode & ColorCapability::BRIGHTNESS)) { ESP_LOGW(TAG, "'%s' - This light does not support setting brightness!", name); this->brightness_.reset(); } @@ -162,13 +165,14 @@ LightColorValues LightCall::validate_() { } // Color brightness exists check - if (this->color_brightness_.has_value() && !(color_mode & ColorCapability::RGB)) { + if (this->color_brightness_.has_value() && *this->color_brightness_ > 0.0f && !(color_mode & ColorCapability::RGB)) { ESP_LOGW(TAG, "'%s' - This color mode does not support setting RGB brightness!", name); this->color_brightness_.reset(); } // RGB exists check - if (this->red_.has_value() || this->green_.has_value() || this->blue_.has_value()) { + if ((this->red_.has_value() && *this->red_ > 0.0f) || (this->green_.has_value() && *this->green_ > 0.0f) || + (this->blue_.has_value() && *this->blue_ > 0.0f)) { if (!(color_mode & ColorCapability::RGB)) { ESP_LOGW(TAG, "'%s' - This color mode does not support setting RGB color!", name); this->red_.reset(); @@ -178,7 +182,7 @@ LightColorValues LightCall::validate_() { } // White value exists check - if (this->white_.has_value() && + if (this->white_.has_value() && *this->white_ > 0.0f && !(color_mode & ColorCapability::WHITE || color_mode & ColorCapability::COLD_WARM_WHITE)) { ESP_LOGW(TAG, "'%s' - This color mode does not support setting white value!", name); this->white_.reset(); @@ -192,7 +196,8 @@ LightColorValues LightCall::validate_() { } // Cold/warm white value exists check - if (this->cold_white_.has_value() || this->warm_white_.has_value()) { + if ((this->cold_white_.has_value() && *this->cold_white_ > 0.0f) || + (this->warm_white_.has_value() && *this->warm_white_ > 0.0f)) { if (!(color_mode & ColorCapability::COLD_WARM_WHITE)) { ESP_LOGW(TAG, "'%s' - This color mode does not support setting cold/warm white value!", name); this->cold_white_.reset(); @@ -200,15 +205,15 @@ LightColorValues LightCall::validate_() { } } -#define VALIDATE_RANGE_(name_, upper_name) \ +#define VALIDATE_RANGE_(name_, upper_name, min, max) \ if (name_##_.has_value()) { \ auto val = *name_##_; \ - if (val < 0.0f || val > 1.0f) { \ - ESP_LOGW(TAG, "'%s' - %s value %.2f is out of range [0.0 - 1.0]!", name, upper_name, val); \ - name_##_ = clamp(val, 0.0f, 1.0f); \ + if (val < (min) || val > (max)) { \ + ESP_LOGW(TAG, "'%s' - %s value %.2f is out of range [%.1f - %.1f]!", name, upper_name, val, (min), (max)); \ + name_##_ = clamp(val, (min), (max)); \ } \ } -#define VALIDATE_RANGE(name, upper_name) VALIDATE_RANGE_(name, upper_name) +#define VALIDATE_RANGE(name, upper_name) VALIDATE_RANGE_(name, upper_name, 0.0f, 1.0f) // Range checks VALIDATE_RANGE(brightness, "Brightness") @@ -219,6 +224,13 @@ LightColorValues LightCall::validate_() { VALIDATE_RANGE(white, "White") VALIDATE_RANGE(cold_white, "Cold white") VALIDATE_RANGE(warm_white, "Warm white") + VALIDATE_RANGE_(color_temperature, "Color temperature", traits.get_min_mireds(), traits.get_max_mireds()) + + // Turn off when brightness is set to zero, and reset brightness (so that it has nonzero brightness when turned on). + if (this->brightness_.has_value() && *this->brightness_ == 0.0f) { + this->state_ = optional(false); + this->brightness_ = optional(1.0f); + } // Set color brightness to 100% if currently zero and a color is set. if (this->red_.has_value() || this->green_.has_value() || this->blue_.has_value()) { @@ -250,7 +262,7 @@ LightColorValues LightCall::validate_() { if (this->warm_white_.has_value()) v.set_warm_white(*this->warm_white_); - v.normalize_color(traits); + v.normalize_color(); // Flash length check if (this->has_flash_() && *this->flash_length_ == 0) { diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index 31dc47e55d..bca2ac7b07 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -2,6 +2,7 @@ #include "esphome/core/optional.h" #include "light_color_values.h" +#include namespace esphome { namespace light { diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 1bc22977a1..a6abac8cdf 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -1,12 +1,7 @@ #pragma once #include "esphome/core/helpers.h" -#include "esphome/core/defines.h" -#include "light_traits.h" - -#ifdef USE_JSON -#include "esphome/components/json/json_util.h" -#endif +#include "color_mode.h" namespace esphome { namespace light { @@ -112,7 +107,7 @@ class LightColorValues { * * @param traits Used for determining which attributes to consider. */ - void normalize_color(const LightTraits &traits) { + void normalize_color() { if (this->color_mode_ & ColorCapability::RGB) { float max_value = fmaxf(this->get_red(), fmaxf(this->get_green(), this->get_blue())); if (max_value == 0.0f) { @@ -125,13 +120,6 @@ class LightColorValues { this->set_blue(this->get_blue() / max_value); } } - - if (this->color_mode_ & ColorCapability::BRIGHTNESS && this->get_brightness() == 0.0f) { - // 0% brightness means off - this->set_state(false); - // reset brightness to 100% - this->set_brightness(1.0f); - } } // Note that method signature of as_* methods is kept as-is for compatibility reasons, so not all parameters diff --git a/esphome/components/light/light_effect.h b/esphome/components/light/light_effect.h index f9903397b4..8da51fe8b3 100644 --- a/esphome/components/light/light_effect.h +++ b/esphome/components/light/light_effect.h @@ -3,8 +3,6 @@ #include #include "esphome/core/component.h" -#include "light_color_values.h" -#include "light_state.h" namespace esphome { namespace light { diff --git a/esphome/components/light/light_json_schema.h b/esphome/components/light/light_json_schema.h index bb7ff812ab..09a372f11c 100644 --- a/esphome/components/light/light_json_schema.h +++ b/esphome/components/light/light_json_schema.h @@ -1,10 +1,13 @@ #pragma once -#include "light_call.h" -#include "light_state.h" +#include "esphome/core/defines.h" #ifdef USE_JSON +#include "esphome/components/json/json_util.h" +#include "light_call.h" +#include "light_state.h" + namespace esphome { namespace light { diff --git a/esphome/components/light/light_output.h b/esphome/components/light/light_output.h index 9e47092b0f..d05cbf9436 100644 --- a/esphome/components/light/light_output.h +++ b/esphome/components/light/light_output.h @@ -7,8 +7,6 @@ namespace esphome { namespace light { -class LightState; - /// Interface to write LightStates to hardware. class LightOutput { public: diff --git a/esphome/components/light/light_state.h b/esphome/components/light/light_state.h index fb34b76367..98fa21d480 100644 --- a/esphome/components/light/light_state.h +++ b/esphome/components/light/light_state.h @@ -54,6 +54,8 @@ class LightState : public Nameable, public Component { * property will be changed continuously (in contrast to .remote_values, where they * are constant during transitions). * + * This value does not have gamma correction applied. + * * This property is read-only for users. Any changes to it will be ignored. */ LightColorValues current_values; @@ -64,6 +66,8 @@ class LightState : public Nameable, public Component { * continuously change the "current" values. But the remote values will immediately * switch to the target value for a transition, reducing the number of packets sent. * + * This value does not have gamma correction applied. + * * This property is read-only for users. Any changes to it will be ignored. */ LightColorValues remote_values; @@ -112,6 +116,7 @@ class LightState : public Nameable, public Component { /// Add effects for this light state. void add_effects(const std::vector &effects); + /// The result of all the current_values_as_* methods have gamma correction applied. void current_values_as_binary(bool *binary); void current_values_as_brightness(float *brightness); diff --git a/esphome/components/light/light_transformer.h b/esphome/components/light/light_transformer.h index ddd0f4dd22..7db6265a89 100644 --- a/esphome/components/light/light_transformer.h +++ b/esphome/components/light/light_transformer.h @@ -85,6 +85,8 @@ class LightTransitionTransformer : public LightTransformer { bool publish_at_end() override { return false; } bool is_transition() override { return true; } + // This looks crazy, but it reduces to 6x^5 - 15x^4 + 10x^3 which is just a smooth sigmoid-like + // transition from 0 to 1 on x = [0, 1] static float smoothed_progress(float x) { return x * x * x * (x * (x * 6.0f - 15.0f) + 10.0f); } protected: