From 2f33cd2db544b098141ea1476b58d5569e8824ac Mon Sep 17 00:00:00 2001 From: Oxan van Leeuwen Date: Mon, 23 Aug 2021 10:00:38 +0200 Subject: [PATCH] Remove double scheduling from addressable lights (#1963) --- .../adalight/adalight_light_effect.cpp | 2 + .../e131/e131_addressable_light_effect.cpp | 1 + .../components/fastled_base/fastled_light.cpp | 9 ++--- .../components/fastled_base/fastled_light.h | 2 +- .../components/light/addressable_light.cpp | 5 +-- esphome/components/light/addressable_light.h | 8 ++-- .../light/addressable_light_effect.h | 37 ++++++++++++------- esphome/components/light/light_output.h | 7 ++++ esphome/components/light/light_state.cpp | 14 +++---- .../neopixelbus/neopixelbus_light.h | 5 +-- .../components/partition/light_partition.h | 10 ++--- esphome/components/wled/wled_light_effect.cpp | 2 + 12 files changed, 57 insertions(+), 45 deletions(-) diff --git a/esphome/components/adalight/adalight_light_effect.cpp b/esphome/components/adalight/adalight_light_effect.cpp index 5f60fbe0b2..d9c2892d21 100644 --- a/esphome/components/adalight/adalight_light_effect.cpp +++ b/esphome/components/adalight/adalight_light_effect.cpp @@ -44,6 +44,7 @@ void AdalightLightEffect::blank_all_leds_(light::AddressableLight &it) { for (int led = it.size(); led-- > 0;) { it[led].set(Color::BLACK); } + it.schedule_show(); } void AdalightLightEffect::apply(light::AddressableLight &it, const Color ¤t_color) { @@ -133,6 +134,7 @@ AdalightLightEffect::Frame AdalightLightEffect::parse_frame_(light::AddressableL it[led].set(Color(led_data[0], led_data[1], led_data[2], white)); } + it.schedule_show(); return CONSUMED; } diff --git a/esphome/components/e131/e131_addressable_light_effect.cpp b/esphome/components/e131/e131_addressable_light_effect.cpp index f280b5bc94..f0f165b25f 100644 --- a/esphome/components/e131/e131_addressable_light_effect.cpp +++ b/esphome/components/e131/e131_addressable_light_effect.cpp @@ -84,6 +84,7 @@ bool E131AddressableLightEffect::process_(int universe, const E131Packet &packet break; } + it->schedule_show(); return true; } diff --git a/esphome/components/fastled_base/fastled_light.cpp b/esphome/components/fastled_base/fastled_light.cpp index 4d791f5709..edfeb401f1 100644 --- a/esphome/components/fastled_base/fastled_light.cpp +++ b/esphome/components/fastled_base/fastled_light.cpp @@ -20,13 +20,12 @@ void FastLEDLightOutput::dump_config() { ESP_LOGCONFIG(TAG, " Num LEDs: %u", this->num_leds_); ESP_LOGCONFIG(TAG, " Max refresh rate: %u", *this->max_refresh_rate_); } -void FastLEDLightOutput::loop() { - if (!this->should_show_()) - return; - - uint32_t now = micros(); +void FastLEDLightOutput::write_state(light::LightState *state) { // protect from refreshing too often + uint32_t now = micros(); if (*this->max_refresh_rate_ != 0 && (now - this->last_refresh_) < *this->max_refresh_rate_) { + // try again next loop iteration, so that this change won't get lost + this->schedule_show(); return; } this->last_refresh_ = now; diff --git a/esphome/components/fastled_base/fastled_light.h b/esphome/components/fastled_base/fastled_light.h index ac6acc95a5..ee85735dea 100644 --- a/esphome/components/fastled_base/fastled_light.h +++ b/esphome/components/fastled_base/fastled_light.h @@ -213,7 +213,7 @@ class FastLEDLightOutput : public light::AddressableLight { } void setup() override; void dump_config() override; - void loop() override; + void write_state(light::LightState *state) override; float get_setup_priority() const override { return setup_priority::HARDWARE; } void clear_effect_data() override { diff --git a/esphome/components/light/addressable_light.cpp b/esphome/components/light/addressable_light.cpp index 9a34dde6be..1b1bc88a6f 100644 --- a/esphome/components/light/addressable_light.cpp +++ b/esphome/components/light/addressable_light.cpp @@ -12,8 +12,7 @@ void AddressableLight::call_setup() { #ifdef ESPHOME_LOG_HAS_VERY_VERBOSE this->set_interval(5000, [this]() { const char *name = this->state_parent_ == nullptr ? "" : this->state_parent_->get_name().c_str(); - ESP_LOGVV(TAG, "Addressable Light '%s' (effect_active=%s next_show=%s)", name, YESNO(this->effect_active_), - YESNO(this->next_show_)); + ESP_LOGVV(TAG, "Addressable Light '%s' (effect_active=%s)", name, YESNO(this->effect_active_)); for (int i = 0; i < this->size(); i++) { auto color = this->get(i); ESP_LOGVV(TAG, " [%2d] Color: R=%3u G=%3u B=%3u W=%3u", i, color.get_red_raw(), color.get_green_raw(), @@ -36,7 +35,7 @@ Color esp_color_from_light_color_values(LightColorValues val) { return Color(r, g, b, w); } -void AddressableLight::write_state(LightState *state) { +void AddressableLight::update_state(LightState *state) { auto val = state->current_values; auto max_brightness = to_uint8_scale(val.get_brightness() * val.get_state()); this->correction_.set_local_brightness(max_brightness); diff --git a/esphome/components/light/addressable_light.h b/esphome/components/light/addressable_light.h index ab1efdf160..bba2158457 100644 --- a/esphome/components/light/addressable_light.h +++ b/esphome/components/light/addressable_light.h @@ -51,9 +51,9 @@ class AddressableLight : public LightOutput, public Component { amnt = this->size(); this->range(amnt, this->size()) = this->range(0, -amnt); } + // Indicates whether an effect that directly updates the output buffer is active to prevent overwriting bool is_effect_active() const { return this->effect_active_; } void set_effect_active(bool effect_active) { this->effect_active_ = effect_active; } - void write_state(LightState *state) override; std::unique_ptr create_default_transition() override; void set_correction(float red, float green, float blue, float white = 1.0f) { this->correction_.set_max_brightness( @@ -63,7 +63,8 @@ class AddressableLight : public LightOutput, public Component { this->correction_.calculate_gamma_table(state->get_gamma_correct()); this->state_parent_ = state; } - void schedule_show() { this->next_show_ = true; } + void update_state(LightState *state) override; + void schedule_show() { this->state_parent_->next_write_ = true; } #ifdef USE_POWER_SUPPLY void set_power_supply(power_supply::PowerSupply *power_supply) { this->power_.set_parent(power_supply); } @@ -74,9 +75,7 @@ class AddressableLight : public LightOutput, public Component { protected: friend class AddressableLightTransformer; - bool should_show_() const { return this->effect_active_ || this->next_show_; } void mark_shown_() { - this->next_show_ = false; #ifdef USE_POWER_SUPPLY for (auto c : *this) { if (c.get().is_on()) { @@ -90,7 +89,6 @@ class AddressableLight : public LightOutput, public Component { virtual ESPColorView get_view_internal(int32_t index) const = 0; bool effect_active_{false}; - bool next_show_{true}; ESPColorCorrection correction_{}; #ifdef USE_POWER_SUPPLY power_supply::PowerSupplyRequester power_; diff --git a/esphome/components/light/addressable_light_effect.h b/esphome/components/light/addressable_light_effect.h index 3a2ba66845..1cb29dfa4e 100644 --- a/esphome/components/light/addressable_light_effect.h +++ b/esphome/components/light/addressable_light_effect.h @@ -63,6 +63,7 @@ class AddressableLambdaLightEffect : public AddressableLightEffect { this->last_run_ = now; this->f_(it, current_color, this->initial_run_); this->initial_run_ = false; + it.schedule_show(); } } @@ -87,6 +88,7 @@ class AddressableRainbowLightEffect : public AddressableLightEffect { var = hsv; hue += add; } + it.schedule_show(); } void set_speed(uint32_t speed) { this->speed_ = speed; } void set_width(uint16_t width) { this->width_ = width; } @@ -134,6 +136,7 @@ class AddressableColorWipeEffect : public AddressableLightEffect { new_color.b = c.b; } } + it.schedule_show(); } protected: @@ -151,25 +154,27 @@ class AddressableScanEffect : public AddressableLightEffect { void set_move_interval(uint32_t move_interval) { this->move_interval_ = move_interval; } void set_scan_width(uint32_t scan_width) { this->scan_width_ = scan_width; } void apply(AddressableLight &it, const Color ¤t_color) override { - it.all() = Color::BLACK; + const uint32_t now = millis(); + if (now - this->last_move_ < this->move_interval_) + return; + if (direction_) { + this->at_led_++; + if (this->at_led_ == it.size() - this->scan_width_) + this->direction_ = false; + } else { + this->at_led_--; + if (this->at_led_ == 0) + this->direction_ = true; + } + this->last_move_ = now; + + it.all() = Color::BLACK; for (auto i = 0; i < this->scan_width_; i++) { it[this->at_led_ + i] = current_color; } - const uint32_t now = millis(); - if (now - this->last_move_ > this->move_interval_) { - if (direction_) { - this->at_led_++; - if (this->at_led_ == it.size() - this->scan_width_) - this->direction_ = false; - } else { - this->at_led_--; - if (this->at_led_ == 0) - this->direction_ = true; - } - this->last_move_ = now; - } + it.schedule_show(); } protected: @@ -210,6 +215,7 @@ class AddressableTwinkleEffect : public AddressableLightEffect { continue; addressable[pos].set_effect_data(1); } + addressable.schedule_show(); } void set_twinkle_probability(float twinkle_probability) { this->twinkle_probability_ = twinkle_probability; } void set_progress_interval(uint32_t progress_interval) { this->progress_interval_ = progress_interval; } @@ -257,6 +263,7 @@ class AddressableRandomTwinkleEffect : public AddressableLightEffect { const uint8_t color = random_uint32() & 0b111; it[pos].set_effect_data(0b1000 | color); } + it.schedule_show(); } void set_twinkle_probability(float twinkle_probability) { this->twinkle_probability_ = twinkle_probability; } void set_progress_interval(uint32_t progress_interval) { this->progress_interval_ = progress_interval; } @@ -301,6 +308,7 @@ class AddressableFireworksEffect : public AddressableLightEffect { it[pos] = current_color; } } + it.schedule_show(); } void set_update_interval(uint32_t update_interval) { this->update_interval_ = update_interval; } void set_spark_probability(float spark_probability) { this->spark_probability_ = spark_probability; } @@ -335,6 +343,7 @@ class AddressableFlickerEffect : public AddressableLightEffect { // slowly fade back to "real" value var = (var.get() * inv_intensity) + (current_color * intensity); } + it.schedule_show(); } void set_update_interval(uint32_t update_interval) { this->update_interval_ = update_interval; } void set_intensity(float intensity) { this->intensity_ = to_uint8_scale(intensity); } diff --git a/esphome/components/light/light_output.h b/esphome/components/light/light_output.h index 7568ea6831..73ba0371cd 100644 --- a/esphome/components/light/light_output.h +++ b/esphome/components/light/light_output.h @@ -19,6 +19,13 @@ class LightOutput { virtual void setup_state(LightState *state) {} + /// Called on every update of the current values of the associated LightState, + /// can optionally be used to do processing of this change. + virtual void update_state(LightState *state) {} + + /// Called from loop() every time the light state has changed, and should + /// should write the new state to hardware. Every call to write_state() is + /// preceded by (at least) one call to update_state(). virtual void write_state(LightState *state) = 0; }; diff --git a/esphome/components/light/light_state.cpp b/esphome/components/light/light_state.cpp index 030cf4b7a2..4c4eefdc30 100644 --- a/esphome/components/light/light_state.cpp +++ b/esphome/components/light/light_state.cpp @@ -114,9 +114,11 @@ void LightState::loop() { // Apply transformer (if any) if (this->transformer_ != nullptr) { auto values = this->transformer_->apply(); - this->next_write_ = values.has_value(); // don't write if transformer doesn't want us to - if (values.has_value()) + if (values.has_value()) { this->current_values = *values; + this->output_->update_state(this); + this->next_write_ = true; + } if (this->transformer_->is_finished()) { this->transformer_->stop(); @@ -127,18 +129,15 @@ void LightState::loop() { // Write state to the light if (this->next_write_) { - this->output_->write_state(this); this->next_write_ = false; + this->output_->write_state(this); } } float LightState::get_setup_priority() const { return setup_priority::HARDWARE - 1.0f; } uint32_t LightState::hash_base() { return 1114400283; } -void LightState::publish_state() { - this->remote_values_callback_.call(); - this->next_write_ = true; -} +void LightState::publish_state() { this->remote_values_callback_.call(); } LightOutput *LightState::get_output() const { return this->output_; } std::string LightState::get_effect_name() { @@ -248,6 +247,7 @@ void LightState::set_immediately_(const LightColorValues &target, bool set_remot if (set_remote_values) { this->remote_values = target; } + this->output_->update_state(this); this->next_write_ = true; } diff --git a/esphome/components/neopixelbus/neopixelbus_light.h b/esphome/components/neopixelbus/neopixelbus_light.h index 1f2cde0bd2..6fa3fb3cd9 100644 --- a/esphome/components/neopixelbus/neopixelbus_light.h +++ b/esphome/components/neopixelbus/neopixelbus_light.h @@ -83,10 +83,7 @@ class NeoPixelBusLightOutputBase : public light::AddressableLight { this->controller_->Begin(); } - void loop() override { - if (!this->should_show_()) - return; - + void write_state(light::LightState *state) override { this->mark_shown_(); this->controller_->Dirty(); diff --git a/esphome/components/partition/light_partition.h b/esphome/components/partition/light_partition.h index 687fe562d1..f74001cf75 100644 --- a/esphome/components/partition/light_partition.h +++ b/esphome/components/partition/light_partition.h @@ -50,13 +50,11 @@ class PartitionLightOutput : public light::AddressableLight { } } light::LightTraits get_traits() override { return this->segments_[0].get_src()->get_traits(); } - void loop() override { - if (this->should_show_()) { - for (auto seg : this->segments_) { - seg.get_src()->schedule_show(); - } - this->mark_shown_(); + void write_state(light::LightState *state) override { + for (auto seg : this->segments_) { + seg.get_src()->schedule_show(); } + this->mark_shown_(); } protected: diff --git a/esphome/components/wled/wled_light_effect.cpp b/esphome/components/wled/wled_light_effect.cpp index 690d2f3b00..915d1c6cc2 100644 --- a/esphome/components/wled/wled_light_effect.cpp +++ b/esphome/components/wled/wled_light_effect.cpp @@ -42,6 +42,7 @@ void WLEDLightEffect::blank_all_leds_(light::AddressableLight &it) { for (int led = it.size(); led-- > 0;) { it[led].set(Color::BLACK); } + it.schedule_show(); } void WLEDLightEffect::apply(light::AddressableLight &it, const Color ¤t_color) { @@ -134,6 +135,7 @@ bool WLEDLightEffect::parse_frame_(light::AddressableLight &it, const uint8_t *p blank_at_ = millis() + DEFAULT_BLANK_TIME; } + it.schedule_show(); return true; }