From 35fb44da364af23d36db6e5efb47ee5f6983d2d4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Jan 2026 03:06:05 +0000 Subject: [PATCH] Address code review feedback: add comments and fix trigger initialization Co-authored-by: clydebarrow <2366188+clydebarrow@users.noreply.github.com> --- esphome/components/cover/automation.h | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/esphome/components/cover/automation.h b/esphome/components/cover/automation.h index 28f665f8835..ea3302ee51b 100644 --- a/esphome/components/cover/automation.h +++ b/esphome/components/cover/automation.h @@ -119,6 +119,8 @@ class CoverOpenTrigger : public Trigger<> { } }; +// Separate CoverOpenedTrigger class for improved naming clarity. +// Both on_open and on_opened are supported for backward compatibility. class CoverOpenedTrigger : public Trigger<> { public: CoverOpenedTrigger(Cover *a_cover) { @@ -146,15 +148,17 @@ class CoverOpeningTrigger : public Trigger<> { CoverOpeningTrigger(Cover *a_cover) { a_cover->add_on_state_callback([this, a_cover]() { auto current_op = a_cover->current_operation; - if (current_op == COVER_OPERATION_OPENING && this->last_operation_ != COVER_OPERATION_OPENING) { - this->trigger(); + if (current_op == COVER_OPERATION_OPENING) { + if (!this->last_operation_.has_value() || this->last_operation_.value() != COVER_OPERATION_OPENING) { + this->trigger(); + } } this->last_operation_ = current_op; }); } protected: - CoverOperation last_operation_{COVER_OPERATION_IDLE}; + optional last_operation_{}; }; class CoverClosingTrigger : public Trigger<> { @@ -162,15 +166,17 @@ class CoverClosingTrigger : public Trigger<> { CoverClosingTrigger(Cover *a_cover) { a_cover->add_on_state_callback([this, a_cover]() { auto current_op = a_cover->current_operation; - if (current_op == COVER_OPERATION_CLOSING && this->last_operation_ != COVER_OPERATION_CLOSING) { - this->trigger(); + if (current_op == COVER_OPERATION_CLOSING) { + if (!this->last_operation_.has_value() || this->last_operation_.value() != COVER_OPERATION_CLOSING) { + this->trigger(); + } } this->last_operation_ = current_op; }); } protected: - CoverOperation last_operation_{COVER_OPERATION_IDLE}; + optional last_operation_{}; }; class CoverIdleTrigger : public Trigger<> { @@ -178,15 +184,17 @@ class CoverIdleTrigger : public Trigger<> { CoverIdleTrigger(Cover *a_cover) { a_cover->add_on_state_callback([this, a_cover]() { auto current_op = a_cover->current_operation; - if (current_op == COVER_OPERATION_IDLE && this->last_operation_ != COVER_OPERATION_IDLE) { - this->trigger(); + if (current_op == COVER_OPERATION_IDLE) { + if (this->last_operation_.has_value() && this->last_operation_.value() != COVER_OPERATION_IDLE) { + this->trigger(); + } } this->last_operation_ = current_op; }); } protected: - CoverOperation last_operation_{COVER_OPERATION_IDLE}; + optional last_operation_{}; }; } // namespace esphome::cover