Fix Light Trigger (#1308)

* make LightTransitionTransformer publish at end

* adjust on trigger + cleanup

* add target_state_reached_callback_

* fix format

* revert publish_at_end change

* fix bug for rapid on/off switching + remove debug logging

* formatting

* call state reached callback when no transition

Co-authored-by: Guillermo Ruffino <glm.net@gmail.com>

esphome/components/light/automation.h

@@ -102,17 +102,18 @@ class LightTurnOnTrigger : public Trigger<> {
  public:
   LightTurnOnTrigger(LightState *a_light) {
     a_light->add_new_remote_values_callback([this, a_light]() {
-      auto is_on = a_light->current_values.is_on();
+      // using the remote value because of transitions we need to trigger as early as possible
+      auto is_on = a_light->remote_values.is_on();
       // only trigger when going from off to on
-      auto should_trigger = is_on && !last_on_;
+      auto should_trigger = is_on && !this->last_on_;
       // Set new state immediately so that trigger() doesn't devolve
       // into infinite loop
-      last_on_ = is_on;
+      this->last_on_ = is_on;
       if (should_trigger) {
         this->trigger();
       }
     });
-    last_on_ = a_light->current_values.is_on();
+    this->last_on_ = a_light->current_values.is_on();
   }
 
  protected:
@@ -122,22 +123,14 @@ class LightTurnOnTrigger : public Trigger<> {
 class LightTurnOffTrigger : public Trigger<> {
  public:
   LightTurnOffTrigger(LightState *a_light) {
-    a_light->add_new_remote_values_callback([this, a_light]() {
+    a_light->add_new_target_state_reached_callback([this, a_light]() {
       auto is_on = a_light->current_values.is_on();
       // only trigger when going from on to off
-      auto should_trigger = !is_on && last_on_;
+      if (!is_on) {
-      // Set new state immediately so that trigger() doesn't devolve
-      // into infinite loop
-      last_on_ = is_on;
-      if (should_trigger) {
         this->trigger();
       }
     });
-    last_on_ = a_light->current_values.is_on();
   }
-
- protected:
-  bool last_on_;
 };
 
 template<typename... Ts> class AddressableSet : public Action<Ts...> {

esphome/components/light/light_state.cpp

@@ -145,6 +145,7 @@ void LightState::loop() {
   if (this->transformer_ != nullptr) {
     if (this->transformer_->is_finished()) {
       this->remote_values = this->current_values = this->transformer_->get_end_values();
+      this->target_state_reached_callback_.call();
       if (this->transformer_->publish_at_end())
         this->publish_state();
       this->transformer_ = nullptr;
@@ -336,6 +337,9 @@ void LightCall::perform() {
     this->parent_->set_immediately_(v, this->publish_);
   }
 
+  if (!this->has_transition_()) {
+    this->parent_->target_state_reached_callback_.call();
+  }
   if (this->publish_) {
     this->parent_->publish_state();
   }
@@ -752,6 +756,10 @@ void LightState::current_values_as_cwww(float *cold_white, float *warm_white, bo
 void LightState::add_new_remote_values_callback(std::function<void()> &&send_callback) {
   this->remote_values_callback_.add(std::move(send_callback));
 }
+void LightState::add_new_target_state_reached_callback(std::function<void()> &&send_callback) {
+  this->target_state_reached_callback_.add(std::move(send_callback));
+}
+
 LightEffect *LightState::get_active_effect_() {
   if (this->active_effect_index_ == 0)
     return nullptr;

esphome/components/light/light_state.h

@@ -242,6 +242,13 @@ class LightState : public Nameable, public Component {
    */
   void add_new_remote_values_callback(std::function<void()> &&send_callback);
 
+  /**
+   * The callback is called once the state of current_values and remote_values are equal
+   *
+   * @param send_callback
+   */
+  void add_new_target_state_reached_callback(std::function<void()> &&send_callback);
+
   /// Return whether the light has any effects that meet the trait requirements.
   bool supports_effects();
 
@@ -318,6 +325,12 @@ class LightState : public Nameable, public Component {
    * starting with the beginning of the transition.
    */
   CallbackManager<void()> remote_values_callback_{};
+
+  /** Callback to call when the state of current_values and remote_values are equal
+   * This should be called once the state of current_values changed and equals the state of remote_values
+   */
+  CallbackManager<void()> target_state_reached_callback_{};
+
   LightOutput *output_;  ///< Store the output to allow effects to have more access.
   /// Whether the light value should be written in the next cycle.
   bool next_write_{true};