From 4988349677a4c078f011c76b176b329baa7453ce Mon Sep 17 00:00:00 2001 From: MartinWelsch Date: Sun, 11 Oct 2020 03:50:53 +0200 Subject: [PATCH] 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 --- esphome/components/light/automation.h | 21 +++++++-------------- esphome/components/light/light_state.cpp | 8 ++++++++ esphome/components/light/light_state.h | 13 +++++++++++++ 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index b240f84e8f..d1fb2a0bcb 100644 --- a/esphome/components/light/automation.h +++ b/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_; - // Set new state immediately so that trigger() doesn't devolve - // into infinite loop - last_on_ = is_on; - if (should_trigger) { + if (!is_on) { this->trigger(); } }); - last_on_ = a_light->current_values.is_on(); } - - protected: - bool last_on_; }; template class AddressableSet : public Action { diff --git a/esphome/components/light/light_state.cpp b/esphome/components/light/light_state.cpp index d34bc88f53..346ab1876d 100644 --- a/esphome/components/light/light_state.cpp +++ b/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 &&send_callback) { this->remote_values_callback_.add(std::move(send_callback)); } +void LightState::add_new_target_state_reached_callback(std::function &&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; diff --git a/esphome/components/light/light_state.h b/esphome/components/light/light_state.h index e48cf9f864..e761e576e3 100644 --- a/esphome/components/light/light_state.h +++ b/esphome/components/light/light_state.h @@ -242,6 +242,13 @@ class LightState : public Nameable, public Component { */ void add_new_remote_values_callback(std::function &&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 &&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 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 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};