Remove double scheduling from addressable lights (#1963)

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 &current_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;
 }
 

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;
 }
 

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() {
+void FastLEDLightOutput::write_state(light::LightState *state) {
-  if (!this->should_show_())
-    return;
-
-  uint32_t now = micros();
   // 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;

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 {

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_),
+    ESP_LOGVV(TAG, "Addressable Light '%s' (effect_active=%s)", name, YESNO(this->effect_active_));
-              YESNO(this->next_show_));
     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);

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<LightTransformer> 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_;

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,14 +154,10 @@ 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 &current_color) override {
-    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 (now - this->last_move_ < this->move_interval_)
+      return;
+
     if (direction_) {
       this->at_led_++;
       if (this->at_led_ == it.size() - this->scan_width_)
@@ -169,7 +168,13 @@ class AddressableScanEffect : public AddressableLightEffect {
         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;
     }
+
+    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); }

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;
 };
 

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() {
+void LightState::publish_state() { this->remote_values_callback_.call(); }
-  this->remote_values_callback_.call();
-  this->next_write_ = true;
-}
 
 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;
 }
 

esphome/components/neopixelbus/neopixelbus_light.h

@@ -83,10 +83,7 @@ class NeoPixelBusLightOutputBase : public light::AddressableLight {
     this->controller_->Begin();
   }
 
-  void loop() override {
+  void write_state(light::LightState *state) override {
-    if (!this->should_show_())
-      return;
-
     this->mark_shown_();
     this->controller_->Dirty();
 

esphome/components/partition/light_partition.h

@@ -50,14 +50,12 @@ class PartitionLightOutput : public light::AddressableLight {
     }
   }
   light::LightTraits get_traits() override { return this->segments_[0].get_src()->get_traits(); }
-  void loop() override {
+  void write_state(light::LightState *state) override {
-    if (this->should_show_()) {
     for (auto seg : this->segments_) {
       seg.get_src()->schedule_show();
     }
     this->mark_shown_();
   }
-  }
 
  protected:
   light::ESPColorView get_view_internal(int32_t index) const override {

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 &current_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;
 }