From 1652914d39f34fb31fe96e5f6d02db9bd1a51479 Mon Sep 17 00:00:00 2001 From: Oxan van Leeuwen Date: Wed, 28 Jul 2021 19:49:43 +0200 Subject: [PATCH] Make light.addressable_set color parameters behave as documented & consistent with elsewhere (#2009) --- .../components/light/addressable_light.cpp | 12 +++---- esphome/components/light/addressable_light.h | 4 +-- .../light/addressable_light_effect.h | 2 +- esphome/components/light/automation.cpp | 15 ++++++++ esphome/components/light/automation.h | 36 ++++++++++++------- esphome/components/light/automation.py | 23 +++++------- .../components/light/esp_color_correction.cpp | 5 +-- esphome/components/light/light_color_values.h | 2 ++ 8 files changed, 61 insertions(+), 38 deletions(-) create mode 100644 esphome/components/light/automation.cpp diff --git a/esphome/components/light/addressable_light.cpp b/esphome/components/light/addressable_light.cpp index 4ef293cd03..4eb9124a7b 100644 --- a/esphome/components/light/addressable_light.cpp +++ b/esphome/components/light/addressable_light.cpp @@ -25,16 +25,16 @@ void AddressableLight::call_setup() { } Color esp_color_from_light_color_values(LightColorValues val) { - auto r = static_cast(roundf(val.get_color_brightness() * val.get_red() * 255.0f)); - auto g = static_cast(roundf(val.get_color_brightness() * val.get_green() * 255.0f)); - auto b = static_cast(roundf(val.get_color_brightness() * val.get_blue() * 255.0f)); - auto w = static_cast(roundf(val.get_white() * 255.0f)); + auto r = to_uint8_scale(val.get_color_brightness() * val.get_red()); + auto g = to_uint8_scale(val.get_color_brightness() * val.get_green()); + auto b = to_uint8_scale(val.get_color_brightness() * val.get_blue()); + auto w = to_uint8_scale(val.get_white()); return Color(r, g, b, w); } void AddressableLight::write_state(LightState *state) { auto val = state->current_values; - auto max_brightness = static_cast(roundf(val.get_brightness() * val.get_state() * 255.0f)); + auto max_brightness = to_uint8_scale(val.get_brightness() * val.get_state()); this->correction_.set_local_brightness(max_brightness); this->last_transition_progress_ = 0.0f; @@ -68,7 +68,7 @@ void AddressableLight::write_state(LightState *state) { // our transition will handle brightness, disable brightness in correction. this->correction_.set_local_brightness(255); - target_color *= static_cast(roundf(end_values.get_brightness() * end_values.get_state() * 255.0f)); + target_color *= to_uint8_scale(end_values.get_brightness() * end_values.get_state()); float denom = (1.0f - new_smoothed); float alpha = denom == 0.0f ? 0.0f : (new_smoothed - prev_smoothed) / denom; diff --git a/esphome/components/light/addressable_light.h b/esphome/components/light/addressable_light.h index 460cb88935..f64abb3eec 100644 --- a/esphome/components/light/addressable_light.h +++ b/esphome/components/light/addressable_light.h @@ -54,8 +54,8 @@ class AddressableLight : public LightOutput, public Component { void set_effect_active(bool effect_active) { this->effect_active_ = effect_active; } void write_state(LightState *state) override; void set_correction(float red, float green, float blue, float white = 1.0f) { - this->correction_.set_max_brightness(Color(uint8_t(roundf(red * 255.0f)), uint8_t(roundf(green * 255.0f)), - uint8_t(roundf(blue * 255.0f)), uint8_t(roundf(white * 255.0f)))); + this->correction_.set_max_brightness( + Color(to_uint8_scale(red), to_uint8_scale(green), to_uint8_scale(blue), to_uint8_scale(white))); } void setup_state(LightState *state) override { this->correction_.calculate_gamma_table(state->get_gamma_correct()); diff --git a/esphome/components/light/addressable_light_effect.h b/esphome/components/light/addressable_light_effect.h index d1ea9e3ff0..25493a30ea 100644 --- a/esphome/components/light/addressable_light_effect.h +++ b/esphome/components/light/addressable_light_effect.h @@ -337,7 +337,7 @@ class AddressableFlickerEffect : public AddressableLightEffect { } } void set_update_interval(uint32_t update_interval) { this->update_interval_ = update_interval; } - void set_intensity(float intensity) { this->intensity_ = static_cast(roundf(intensity * 255.0f)); } + void set_intensity(float intensity) { this->intensity_ = to_uint8_scale(intensity); } protected: uint32_t update_interval_{16}; diff --git a/esphome/components/light/automation.cpp b/esphome/components/light/automation.cpp new file mode 100644 index 0000000000..8c1785f061 --- /dev/null +++ b/esphome/components/light/automation.cpp @@ -0,0 +1,15 @@ +#include "automation.h" +#include "esphome/core/log.h" + +namespace esphome { +namespace light { + +static const char *const TAG = "light.automation"; + +void addressableset_warn_about_scale(const char *field) { + ESP_LOGW(TAG, "Lambda for parameter %s of light.addressable_set should return values in range 0-1 instead of 0-255.", + field); +} + +} // namespace light +} // namespace esphome diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index e99d5c58bd..02c6163555 100644 --- a/esphome/components/light/automation.h +++ b/esphome/components/light/automation.h @@ -135,39 +135,51 @@ class LightTurnOffTrigger : public Trigger<> { } }; +// This is slightly ugly, but we can't log in headers, and can't make this a static method on AddressableSet +// due to the template. It's just a temporary warning anyway. +void addressableset_warn_about_scale(const char *field); + template class AddressableSet : public Action { public: explicit AddressableSet(LightState *parent) : parent_(parent) {} TEMPLATABLE_VALUE(int32_t, range_from) TEMPLATABLE_VALUE(int32_t, range_to) - TEMPLATABLE_VALUE(uint8_t, color_brightness) - TEMPLATABLE_VALUE(uint8_t, red) - TEMPLATABLE_VALUE(uint8_t, green) - TEMPLATABLE_VALUE(uint8_t, blue) - TEMPLATABLE_VALUE(uint8_t, white) + TEMPLATABLE_VALUE(float, color_brightness) + TEMPLATABLE_VALUE(float, red) + TEMPLATABLE_VALUE(float, green) + TEMPLATABLE_VALUE(float, blue) + TEMPLATABLE_VALUE(float, white) void play(Ts... x) override { auto *out = (AddressableLight *) this->parent_->get_output(); int32_t range_from = this->range_from_.value_or(x..., 0); int32_t range_to = this->range_to_.value_or(x..., out->size() - 1) + 1; - uint8_t remote_color_brightness = - static_cast(roundf(this->parent_->remote_values.get_color_brightness() * 255.0f)); - uint8_t color_brightness = this->color_brightness_.value_or(x..., remote_color_brightness); + uint8_t color_brightness = + to_uint8_scale(this->color_brightness_.value_or(x..., this->parent_->remote_values.get_color_brightness())); auto range = out->range(range_from, range_to); if (this->red_.has_value()) - range.set_red(esp_scale8(this->red_.value(x...), color_brightness)); + range.set_red(esp_scale8(to_uint8_compat(this->red_.value(x...), "red"), color_brightness)); if (this->green_.has_value()) - range.set_green(esp_scale8(this->green_.value(x...), color_brightness)); + range.set_green(esp_scale8(to_uint8_compat(this->green_.value(x...), "green"), color_brightness)); if (this->blue_.has_value()) - range.set_blue(esp_scale8(this->blue_.value(x...), color_brightness)); + range.set_blue(esp_scale8(to_uint8_compat(this->blue_.value(x...), "blue"), color_brightness)); if (this->white_.has_value()) - range.set_white(this->white_.value(x...)); + range.set_white(to_uint8_compat(this->white_.value(x...), "white")); out->schedule_show(); } protected: LightState *parent_; + + // Historically, this action required uint8_t (0-255) for RGBW values from lambdas. Keep compatibility. + static inline uint8_t to_uint8_compat(float value, const char *field) { + if (value > 1.0f) { + addressableset_warn_about_scale(field); + return static_cast(value); + } + return to_uint8_scale(value); + } }; } // namespace light diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index dd1148131a..85741f1ffd 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -173,6 +173,7 @@ LIGHT_ADDRESSABLE_SET_ACTION_SCHEMA = cv.Schema( cv.Required(CONF_ID): cv.use_id(AddressableLightState), cv.Optional(CONF_RANGE_FROM): cv.templatable(cv.positive_int), cv.Optional(CONF_RANGE_TO): cv.templatable(cv.positive_int), + cv.Optional(CONF_COLOR_BRIGHTNESS): cv.templatable(cv.percentage), cv.Optional(CONF_RED): cv.templatable(cv.percentage), cv.Optional(CONF_GREEN): cv.templatable(cv.percentage), cv.Optional(CONF_BLUE): cv.templatable(cv.percentage), @@ -194,28 +195,20 @@ async def light_addressable_set_to_code(config, action_id, template_arg, args): templ = await cg.templatable(config[CONF_RANGE_TO], args, cg.int32) cg.add(var.set_range_to(templ)) - def rgbw_to_exp(x): - return int(round(x * 255)) - + if CONF_COLOR_BRIGHTNESS in config: + templ = await cg.templatable(config[CONF_COLOR_BRIGHTNESS], args, cg.float_) + cg.add(var.set_color_brightness(templ)) if CONF_RED in config: - templ = await cg.templatable( - config[CONF_RED], args, cg.uint8, to_exp=rgbw_to_exp - ) + templ = await cg.templatable(config[CONF_RED], args, cg.float_) cg.add(var.set_red(templ)) if CONF_GREEN in config: - templ = await cg.templatable( - config[CONF_GREEN], args, cg.uint8, to_exp=rgbw_to_exp - ) + templ = await cg.templatable(config[CONF_GREEN], args, cg.float_) cg.add(var.set_green(templ)) if CONF_BLUE in config: - templ = await cg.templatable( - config[CONF_BLUE], args, cg.uint8, to_exp=rgbw_to_exp - ) + templ = await cg.templatable(config[CONF_BLUE], args, cg.float_) cg.add(var.set_blue(templ)) if CONF_WHITE in config: - templ = await cg.templatable( - config[CONF_WHITE], args, cg.uint8, to_exp=rgbw_to_exp - ) + templ = await cg.templatable(config[CONF_WHITE], args, cg.float_) cg.add(var.set_white(templ)) return var diff --git a/esphome/components/light/esp_color_correction.cpp b/esphome/components/light/esp_color_correction.cpp index 19a2af3da1..e5e68264cc 100644 --- a/esphome/components/light/esp_color_correction.cpp +++ b/esphome/components/light/esp_color_correction.cpp @@ -1,4 +1,5 @@ #include "esp_color_correction.h" +#include "light_color_values.h" #include "esphome/core/log.h" namespace esphome { @@ -7,7 +8,7 @@ namespace light { void ESPColorCorrection::calculate_gamma_table(float gamma) { for (uint16_t i = 0; i < 256; i++) { // corrected = val ^ gamma - auto corrected = static_cast(roundf(255.0f * gamma_correct(i / 255.0f, gamma))); + auto corrected = to_uint8_scale(gamma_correct(i / 255.0f, gamma)); this->gamma_table_[i] = corrected; } if (gamma == 0.0f) { @@ -17,7 +18,7 @@ void ESPColorCorrection::calculate_gamma_table(float gamma) { } for (uint16_t i = 0; i < 256; i++) { // val = corrected ^ (1/gamma) - auto uncorrected = static_cast(roundf(255.0f * powf(i / 255.0f, 1.0f / gamma))); + auto uncorrected = to_uint8_scale(powf(i / 255.0f, 1.0f / gamma)); this->gamma_reverse_table_[i] = uncorrected; } } diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 54dcaea5a3..344411b75e 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -11,6 +11,8 @@ namespace esphome { namespace light { +inline static uint8_t to_uint8_scale(float x) { return static_cast(roundf(x * 255.0f)); } + /** This class represents the color state for a light object. * * All values in this class (except color temperature) are represented using floats in the range