From 72ca565a0e4e2a676748b202caef348332d88a14 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 12 Oct 2026 13:35:03 -1000 Subject: [PATCH] [core] Resume if and while from the end of their branch instead of an embedded continuation action (#20283) --- esphome/core/automation.h | 61 ++- esphome/core/base_automation.h | 100 ++--- tests/components/api/common-base.yaml | 8 +- .../core/test_action_branch_owner.cpp | 403 ++++++++++++++++++ .../fixtures/branch_resume_async.yaml | 28 ++ .../fixtures/continuation_actions.yaml | 14 +- tests/integration/test_branch_resume_async.py | 14 + .../integration/test_continuation_actions.py | 9 +- 8 files changed, 533 insertions(+), 104 deletions(-) create mode 100644 tests/components/core/test_action_branch_owner.cpp diff --git a/esphome/core/automation.h b/esphome/core/automation.h index 35c2a17c247..bbb7f08fb8e 100644 --- a/esphome/core/automation.h +++ b/esphome/core/automation.h @@ -576,20 +576,22 @@ template class Action { /// the following actions in the chain. int num_running_total() { int total = this->num_running_; - if (this->next_ != nullptr) - total += this->next_->num_running_total(); + if (Action *next = this->chain_next_()) + total += next->num_running_total(); return total; } protected: friend ActionList; - template friend class ContinuationAction; + /// For a branch owner (if, while), play() runs when the branch finishes. virtual void play(const Ts &...x) = 0; void play_next_(const Ts &...x) { if (this->num_running_ > 0) { this->num_running_--; - if (this->next_ != nullptr) { + if (this->next_is_owner_()) { + this->owner_()->play(x...); + } else if (this->next_ != nullptr) { this->next_->play_complex(x...); } } @@ -603,15 +605,24 @@ template class Action { virtual void stop() {} void stop_next_() { - if (this->next_ != nullptr) { - this->next_->stop_complex(); - } + if (Action *next = this->chain_next_()) + next->stop_complex(); } bool is_running_next_() { - if (this->next_ == nullptr) - return false; - return this->next_->is_running(); + Action *next = this->chain_next_(); + return next != nullptr && next->is_running(); + } + + // A branch's last action points at its owner with the low bit set. Only Action objects are + // tagged, never function pointers, which use that bit on ARM Thumb (#9283). + static constexpr uintptr_t OWNER_TAG = 1; + bool next_is_owner_() const { return reinterpret_cast(this->next_) & OWNER_TAG; } + /// Only valid when next_is_owner_(). + Action *owner_() const { return reinterpret_cast(reinterpret_cast(this->next_) - OWNER_TAG); } + Action *chain_next_() const { return this->next_is_owner_() ? nullptr : this->next_; } + static Action *tag_owner(Action *owner) { + return reinterpret_cast(reinterpret_cast(owner) + OWNER_TAG); } Action *next_{nullptr}; @@ -621,25 +632,30 @@ template class Action { int num_running_{0}; }; +// The vtable pointer gives every instantiation this alignment. +static_assert(alignof(Action<>) >= 2, "the owner tag needs a free low bit in Action object pointers"); + template class ActionList { public: void add_action(Action *action) { // Walk to end of chain - action lists are short and only built during setup() - Action **tail = &this->actions_; - while (*tail != nullptr) - tail = &(*tail)->next_; - *tail = action; + *this->tail_() = action; } void add_actions(const std::initializer_list *> &actions) { // Find tail once, then append all actions in a single pass - Action **tail = &this->actions_; - while (*tail != nullptr) - tail = &(*tail)->next_; + Action **tail = this->tail_(); for (auto *action : actions) { *tail = action; tail = &action->next_; } } + /// Resume `owner` via play() when the list finishes; call once, after adding actions. + void set_owner(Action *owner) { + // Nothing to tag in an empty list + if (this->actions_ == nullptr) + return; + *this->tail_() = Action::tag_owner(owner); + } // Force-inline: part of the Trigger→Automation→ActionList forwarding // chain collapsed to reduce automation call stack depth. inline void play(const Ts &...x) ESPHOME_ALWAYS_INLINE { @@ -672,6 +688,17 @@ template class ActionList { template void play_tuple_(const std::tuple &tuple, std::index_sequence /*unused*/) { this->play(std::get(tuple)...); } + // Lists with an owner are complete; never append after set_owner() + Action **tail_() { + Action **tail = &this->actions_; + while (*tail != nullptr) { + // Not checked in release builds: only automation.py builds these lists, appending each action and + // then finishing the branch once, so appending after set_owner() would be a codegen bug. + ESPHOME_DEBUG_ASSERT(!(*tail)->next_is_owner_()); + tail = &(*tail)->next_; + } + return tail; + } Action *actions_{nullptr}; }; diff --git a/esphome/core/base_automation.h b/esphome/core/base_automation.h index 09cde1261d0..c59e27f43ab 100644 --- a/esphome/core/base_automation.h +++ b/esphome/core/base_automation.h @@ -278,70 +278,33 @@ template class ApplyCondition final : public Condition< bool check(const Ts &...x) override { return Fn(x...); } }; -/// Simple continuation action that calls play_next_ on a parent action. -/// Used internally by IfAction, WhileAction, RepeatAction, etc. to chain actions. -/// Memory: 4-8 bytes (parent pointer) vs 40 bytes (LambdaAction with std::function). -template class ContinuationAction : public Action { - public: - explicit ContinuationAction(Action *parent) : parent_(parent) {} - - void play(const Ts &...x) override { this->parent_->play_next_(x...); } - - protected: - Action *parent_; -}; - -// Forward declaration for WhileLoopContinuation -template class WhileAction; - -/// Loop continuation for WhileAction that checks condition and repeats or continues. -/// Memory: 4-8 bytes (parent pointer) vs 40 bytes (LambdaAction with std::function). -template class WhileLoopContinuation : public Action { - public: - explicit WhileLoopContinuation(WhileAction *parent) : parent_(parent) {} - - void play(const Ts &...x) override; - - protected: - WhileAction *parent_; -}; - -// Wraps a ContinuationAction when Enabled, empty otherwise. -// Lets IfAction elide the else continuation when HasElse is false. -template struct OptionalContinuation { - ContinuationAction action; - explicit OptionalContinuation(Action *parent) : action(parent) {} -}; -template struct OptionalContinuation { - explicit OptionalContinuation(Action * /*parent*/) {} -}; - template class IfAction : public Action { public: explicit IfAction(Condition *condition) : condition_(condition) {} // Precondition: build each branch either with one add_then/add_else call, or with // add_then_action/add_else_action calls followed by one finish_then/finish_else, never both. - // Appending the inline continuation twice forms a self-loop in the next_ chain. + // set_owner() closes the list; appending after it is a codegen bug. void add_then(const std::initializer_list *> &actions) { this->then_.add_actions(actions); - this->then_.add_action(&this->then_continuation_); + this->then_.set_owner(this); } void add_else(const std::initializer_list *> &actions) requires(HasElse) { this->else_.add_actions(actions); - this->else_.add_action(&this->else_continuation_.action); + this->else_.set_owner(this); } // Codegen appends branch actions one at a time, then finishes each branch once; a list literal // would be a constant array in .rodata, which is RAM on ESP8266. void add_then_action(Action *action) { this->then_.add_action(action); } - void finish_then() { this->then_.add_action(&this->then_continuation_); } + void finish_then() { this->then_.set_owner(this); } void add_else_action(Action *action) requires(HasElse) { this->else_.add_action(action); } - void finish_else() requires(HasElse) { this->else_.add_action(&this->else_continuation_.action); } + void finish_else() requires(HasElse) { this->else_.set_owner(this); } - void play_complex(const Ts &...x) override { + void play_complex(const Ts &...x) final { this->num_running_++; + // The condition may stop this automation (e.g. script.stop in a lambda) if (this->condition_->check(x...)) { if (!this->then_.empty() && this->num_running_ > 0) { this->then_.play(x...); @@ -356,8 +319,8 @@ template class IfAction : public Action { this->play_next_(x...); } - void play(const Ts &...x) override { /* ignore - see play_complex */ - } + // Branch-end resume hook + void play(const Ts &...x) override { this->play_next_(x...); } void stop() override { this->then_.stop(); @@ -369,10 +332,8 @@ template class IfAction : public Action { protected: Condition *condition_; ActionList then_; - ContinuationAction then_continuation_{this}; struct NoElse {}; [[no_unique_address]] std::conditional_t, NoElse> else_; - [[no_unique_address]] OptionalContinuation else_continuation_{this}; }; template class WhileAction : public Action { @@ -383,14 +344,12 @@ template class WhileAction : public Action { // (see IfAction::add_then). void add_then(const std::initializer_list *> &actions) { this->then_.add_actions(actions); - this->then_.add_action(&this->loop_continuation_); + this->then_.set_owner(this); } void add_then_action(Action *action) { this->then_.add_action(action); } - void finish_then() { this->then_.add_action(&this->loop_continuation_); } + void finish_then() { this->then_.set_owner(this); } - friend class WhileLoopContinuation; - - void play_complex(const Ts &...x) override { + void play_complex(const Ts &...x) final { this->num_running_++; // Initial condition check if (!this->condition_->check(x...)) { @@ -400,12 +359,25 @@ template class WhileAction : public Action { return; } - if (this->num_running_ > 0) { - this->then_.play(x...); + // The condition may have stopped this automation + if (this->num_running_ == 0) + return; + // An empty body cannot resume the loop + if (this->then_.empty()) { + this->play_next_(x...); + return; } + this->then_.play(x...); } - void play(const Ts &...x) override { /* ignore - see play_complex */ + // Branch-end resume hook: loop again or continue + void play(const Ts &...x) override { + // The condition may stop this automation + if (this->num_running_ > 0 && this->condition_->check(x...) && this->num_running_ > 0) { + this->then_.play(x...); + } else { + this->play_next_(x...); + } } void stop() override { this->then_.stop(); } @@ -413,25 +385,13 @@ template class WhileAction : public Action { protected: Condition *condition_; ActionList then_; - WhileLoopContinuation loop_continuation_{this}; }; -// Implementation of WhileLoopContinuation::play -template void WhileLoopContinuation::play(const Ts &...x) { - if (this->parent_->num_running_ > 0 && this->parent_->condition_->check(x...)) { - // play again - this->parent_->then_.play(x...); - } else { - // condition false, play next - this->parent_->play_next_(x...); - } -} - // Forward declaration for RepeatLoopContinuation template class RepeatAction; /// Loop continuation for RepeatAction that increments iteration and repeats or continues. -/// Memory: 4-8 bytes (parent pointer) vs 40 bytes (LambdaAction with std::function). +/// Repeat keeps a continuation: its body takes an extra uint32_t, so it cannot be tagged. template class RepeatLoopContinuation : public Action { public: explicit RepeatLoopContinuation(RepeatAction *parent) : parent_(parent) {} @@ -447,7 +407,7 @@ template class RepeatAction : public Action { TEMPLATABLE_VALUE(uint32_t, count) // Precondition: use either add_then or add_then_action plus one finish_then, never both - // (see IfAction::add_then). + // (see IfAction::add_then); appending the continuation twice forms a self-loop. void add_then(const std::initializer_list *> &actions) { this->then_.add_actions(actions); this->then_.add_action(&this->loop_continuation_); diff --git a/tests/components/api/common-base.yaml b/tests/components/api/common-base.yaml index 1513d537c21..7f6cb83f16c 100644 --- a/tests/components/api/common-base.yaml +++ b/tests/components/api/common-base.yaml @@ -181,7 +181,7 @@ api: room: !lambda 'return room;' ratio: !lambda 'return level / 100.0f;' - homeassistant.tag_scanned: !lambda 'return room;' - # Test ContinuationAction (IfAction with then/else branches) + # Test IfAction with then/else branches - action: test_if_action variables: condition: bool @@ -199,7 +199,7 @@ api: format: "Condition false, value: %ld" args: ['(long) value'] - logger.log: "After if/else" - # Test nested IfAction (multiple ContinuationAction instances) + # Test nested IfAction - action: test_nested_if variables: outer: bool @@ -219,7 +219,7 @@ api: else: - logger.log: "Outer false" - logger.log: "After nested if" - # Test WhileLoopContinuation (WhileAction) + # Test WhileAction - action: test_while_action variables: max_count: int @@ -234,7 +234,7 @@ api: args: ['id(api_continuation_test_counter)'] - lambda: 'id(api_continuation_test_counter)++;' - logger.log: "After while loop" - # Test RepeatLoopContinuation (RepeatAction) + # Test RepeatAction - action: test_repeat_action variables: count: int diff --git a/tests/components/core/test_action_branch_owner.cpp b/tests/components/core/test_action_branch_owner.cpp new file mode 100644 index 00000000000..126f932686d --- /dev/null +++ b/tests/components/core/test_action_branch_owner.cpp @@ -0,0 +1,403 @@ +#include +#include + +#include "esphome/core/automation.h" +#include "esphome/core/base_automation.h" + +namespace esphome::core::testing { + +using Log = std::vector; + +// Records its id when it runs, then continues the chain +class RecordAction : public Action<> { + public: + RecordAction(Log *log, int id) : log_(log), id_(id) {} + void play() override { this->log_->push_back(this->id_); } + Action<> *next() const { return this->next_; } + + protected: + Log *log_; + int id_; +}; + +// Holds the chain until finish() is called, like a delay +class DeferredAction : public Action<> { + public: + explicit DeferredAction(Log *log, int id) : log_(log), id_(id) {} + void play_complex() override { + this->num_running_++; + this->log_->push_back(this->id_); + this->pending_ = true; + } + void play() override {} + void stop() override { this->pending_ = false; } + void finish() { + if (this->pending_) { + this->pending_ = false; + this->play_next_(); + } + } + bool pending() const { return this->pending_; } + + protected: + Log *log_; + int id_; + bool pending_{false}; +}; + +class FlagCondition : public Condition<> { + public: + bool check() override { return this->value; } + bool value{true}; +}; + +// Stops the given list during check(), like script.stop in a lambda condition +class StoppingCondition : public Condition<> { + public: + bool check() override { + if (this->list != nullptr) + this->list->stop(); + return true; + } + ActionList<> *list{nullptr}; +}; + +// Stops the given list on the Nth check and keeps returning true +class StopOnCheckCondition : public Condition<> { + public: + explicit StopOnCheckCondition(int stop_on) : stop_on_(stop_on) {} + bool check() override { + if (++this->checks_ == this->stop_on_ && this->list != nullptr) + this->list->stop(); + return true; + } + ActionList<> *list{nullptr}; + + protected: + int stop_on_; + int checks_{0}; +}; + +// Turns its condition off after a number of checks +class CountdownCondition : public Condition<> { + public: + explicit CountdownCondition(int passes) : passes_(passes) {} + bool check() override { return this->passes_-- > 0; } + + protected: + int passes_; +}; + +// Reads an action's next_ through Action's own tag helpers +class TaggedOwner : public RecordAction { + public: + using RecordAction::RecordAction; + static bool is_tagged(const TaggedOwner *a) { return a->next_is_owner_(); } + static Action<> *owner_of(const TaggedOwner *a) { return a->owner_(); } +}; + +class InspectList : public ActionList<> { + public: + Action<> *head() const { return this->actions_; } +}; + +TEST(ActionBranchOwner, SetOwnerTagsOnlyTheLastAction) { + Log log; + TaggedOwner a(&log, 1), b(&log, 2), c(&log, 3); + TaggedOwner owner(&log, 99); + InspectList list; + list.add_actions({&a, &b}); + list.add_action(&c); // appending before set_owner keeps order + list.set_owner(&owner); + EXPECT_EQ(list.head(), &a); + EXPECT_EQ(a.next(), &b); + EXPECT_EQ(b.next(), &c); + EXPECT_FALSE(TaggedOwner::is_tagged(&a)); + EXPECT_FALSE(TaggedOwner::is_tagged(&b)); + EXPECT_TRUE(TaggedOwner::is_tagged(&c)); + EXPECT_EQ(TaggedOwner::owner_of(&c), &owner); +} + +TEST(ActionBranchOwner, EmptyListStaysEmpty) { + Log log; + TaggedOwner owner(&log, 99); + InspectList list; + list.set_owner(&owner); + EXPECT_EQ(list.head(), nullptr); + EXPECT_TRUE(list.empty()); +} + +// ESPHOME_DEBUG_ASSERT is only live because script/cpp_unit_test.py builds with -DESPHOME_DEBUG +TEST(ActionBranchOwnerDeathTest, AppendAfterSetOwnerAsserts) { + Log log; + RecordAction a(&log, 1), late(&log, 2); + TaggedOwner owner(&log, 99); + ActionList<> list; + list.add_action(&a); + list.set_owner(&owner); + EXPECT_DEATH(list.add_action(&late), ""); +} + +TEST(ActionBranchOwner, IfThenResumesAfterTheIfOnce) { + Log log; + FlagCondition cond; + IfAction if_action(&cond); + RecordAction t1(&log, 1), t2(&log, 2), e1(&log, 3), after(&log, 4); + if_action.add_then({&t1, &t2}); + if_action.add_else({&e1}); + ActionList<> top; + top.add_actions({&if_action, &after}); + + top.play(); + EXPECT_EQ(log, (Log{1, 2, 4})); + log.clear(); + cond.value = false; + top.play(); + EXPECT_EQ(log, (Log{3, 4})); + EXPECT_FALSE(top.is_running()); + EXPECT_EQ(top.num_running(), 0); +} + +// The per-item calls automation.py emits +TEST(ActionBranchOwner, PerItemIfAndWhileResumeAfterTheirBranch) { + Log log; + FlagCondition cond; + IfAction if_action(&cond); + RecordAction t1(&log, 1), t2(&log, 2), e1(&log, 3), after(&log, 4); + if_action.add_then_action(&t1); + if_action.add_then_action(&t2); + if_action.finish_then(); + if_action.add_else_action(&e1); + if_action.finish_else(); + ActionList<> top; + top.add_actions({&if_action, &after}); + top.play(); + cond.value = false; + top.play(); + EXPECT_EQ(log, (Log{1, 2, 4, 3, 4})); + + log.clear(); + CountdownCondition loop_cond(2); + WhileAction<> loop(&loop_cond); + DeferredAction wait(&log, 5); + RecordAction after2(&log, 6); + loop.add_then_action(&wait); + loop.finish_then(); + ActionList<> top2; + top2.add_actions({&loop, &after2}); + top2.play(); + wait.finish(); + wait.finish(); + EXPECT_EQ(log, (Log{5, 5, 6})); + EXPECT_FALSE(top2.is_running()); +} + +// then: [] generates finish_then() with no actions +TEST(ActionBranchOwner, PerItemEmptyBranchesFinishImmediately) { + Log log; + FlagCondition cond; + IfAction if_action(&cond); + if_action.finish_then(); + if_action.finish_else(); + WhileAction<> loop(&cond); + loop.finish_then(); + RecordAction after(&log, 1), after2(&log, 2); + ActionList<> top, top2; + top.add_actions({&if_action, &after}); + top2.add_actions({&loop, &after2}); + top.play(); + cond.value = false; + top.play(); + cond.value = true; + top2.play(); + EXPECT_EQ(log, (Log{1, 1, 2})); + EXPECT_FALSE(top2.is_running()); +} + +TEST(ActionBranchOwner, ConditionThatStopsTheRunStartsNoBranch) { + Log log; + StoppingCondition cond; + IfAction if_action(&cond); + WhileAction<> loop(&cond); + RecordAction t1(&log, 1), e1(&log, 2), body(&log, 3), after(&log, 4); + if_action.add_then({&t1}); + if_action.add_else({&e1}); + loop.add_then({&body}); + ActionList<> if_list, while_list; + if_list.add_actions({&if_action, &after}); + while_list.add_actions({&loop, &after}); + + cond.list = &if_list; + if_list.play(); + cond.list = &while_list; + while_list.play(); + EXPECT_TRUE(log.empty()); + EXPECT_FALSE(if_list.is_running()); + EXPECT_FALSE(while_list.is_running()); +} + +TEST(ActionBranchOwner, WhileConditionStoppingOnALaterPassEndsTheLoop) { + Log log; + StopOnCheckCondition cond(2); + WhileAction<> loop(&cond); + RecordAction body(&log, 1), after(&log, 2); + loop.add_then({&body}); + ActionList<> list; + list.add_actions({&loop, &after}); + cond.list = &list; + + list.play(); + EXPECT_EQ(log, (Log{1})); + EXPECT_FALSE(list.is_running()); +} + +TEST(ActionBranchOwner, IfResumesAfterDeferredLastAction) { + Log log; + FlagCondition cond; + IfAction if_action(&cond); + DeferredAction wait(&log, 1); + RecordAction after(&log, 2); + if_action.add_then({&wait}); + ActionList<> top; + top.add_actions({&if_action, &after}); + + top.play(); + EXPECT_EQ(log, (Log{1})); + EXPECT_TRUE(top.is_running()); + // num_running() counts the top-level chain only: the if, not the action inside its branch + EXPECT_EQ(top.num_running(), 1); + wait.finish(); + EXPECT_EQ(log, (Log{1, 2})); + EXPECT_FALSE(top.is_running()); + EXPECT_EQ(top.num_running(), 0); +} + +// A deferred action that can hold two runs at once, completed in order +class QueueDeferredAction : public Action<> { + public: + void play_complex() override { + this->num_running_++; + this->pending_++; + } + void play() override {} + void finish_one() { + if (this->pending_ > 0) { + this->pending_--; + this->play_next_(); + } + } + int running() const { return this->num_running_; } + + protected: + int pending_{0}; +}; + +TEST(ActionBranchOwner, OverlappingRunsResumeOncePerRun) { + Log log; + FlagCondition cond; + IfAction if_action(&cond); + QueueDeferredAction wait; + RecordAction after(&log, 7); + if_action.add_then({&wait}); + ActionList<> top; + top.add_actions({&if_action, &after}); + + top.play(); + top.play(); + EXPECT_EQ(wait.running(), 2); // the tagged last action holds both runs + wait.finish_one(); + EXPECT_EQ(log, (Log{7})); + EXPECT_TRUE(top.is_running()); + wait.finish_one(); + EXPECT_EQ(log, (Log{7, 7})); + EXPECT_FALSE(top.is_running()); + EXPECT_EQ(wait.running(), 0); +} + +TEST(ActionBranchOwner, EmptyBranchesFinishImmediately) { + Log log; + FlagCondition cond; + IfAction if_action(&cond); + RecordAction after(&log, 1); + if_action.add_then({}); + if_action.add_else({}); + ActionList<> top; + top.add_actions({&if_action, &after}); + top.play(); + cond.value = false; + top.play(); + EXPECT_EQ(log, (Log{1, 1})); + + FlagCondition always; + WhileAction<> loop(&always); + loop.add_then({}); + ActionList<> top2; + RecordAction after2(&log, 2); + top2.add_actions({&loop, &after2}); + top2.play(); + EXPECT_EQ(log, (Log{1, 1, 2})); + EXPECT_FALSE(top2.is_running()); +} + +TEST(ActionBranchOwner, WhileRechecksEachPassAndExits) { + Log log; + CountdownCondition cond(3); + WhileAction<> loop(&cond); + RecordAction body(&log, 1); + RecordAction after(&log, 2); + loop.add_then({&body}); + ActionList<> top; + top.add_actions({&loop, &after}); + top.play(); + EXPECT_EQ(log, (Log{1, 1, 1, 2})); + EXPECT_FALSE(top.is_running()); +} + +TEST(ActionBranchOwner, StopMidBranchClearsRunning) { + Log log; + FlagCondition cond; + WhileAction<> loop(&cond); + DeferredAction wait(&log, 1); + RecordAction after(&log, 2); + loop.add_then({&wait}); + ActionList<> top; + top.add_actions({&loop, &after}); + + top.play(); + EXPECT_TRUE(top.is_running()); + top.stop(); + EXPECT_FALSE(top.is_running()); + EXPECT_EQ(top.num_running(), 0); + wait.finish(); // already stopped: nothing pending, nothing resumes + EXPECT_EQ(log, (Log{1})); + + // A fresh run works after the stop + cond.value = false; + top.play(); + EXPECT_EQ(log, (Log{1, 2})); +} + +TEST(ActionBranchOwner, NestedIfInsideWhileResumesEachOwner) { + Log log; + CountdownCondition loop_cond(2); + FlagCondition if_cond; + WhileAction<> loop(&loop_cond); + IfAction inner(&if_cond); + DeferredAction wait(&log, 1); + RecordAction after(&log, 9); + inner.add_then({&wait}); + loop.add_then({&inner}); // the if is the last action of the loop body + ActionList<> top; + top.add_actions({&loop, &after}); + + top.play(); + EXPECT_EQ(log, (Log{1})); + wait.finish(); // resumes the if, then the while, which loops again + EXPECT_EQ(log, (Log{1, 1})); + wait.finish(); // second pass ends; the while condition is now false + EXPECT_EQ(log, (Log{1, 1, 9})); + EXPECT_FALSE(top.is_running()); + EXPECT_EQ(top.num_running(), 0); +} + +} // namespace esphome::core::testing diff --git a/tests/integration/fixtures/branch_resume_async.yaml b/tests/integration/fixtures/branch_resume_async.yaml index fb093238095..76654ddaa9e 100644 --- a/tests/integration/fixtures/branch_resume_async.yaml +++ b/tests/integration/fixtures/branch_resume_async.yaml @@ -72,6 +72,26 @@ script: format: "parallel-after-if-%d" args: ['run'] + - id: stop_in_later_while_check + then: + - lambda: 'id(counter) = 0;' + - while: + condition: + lambda: |- + if (++id(counter) == 2) + id(stop_in_later_while_check).stop(); + return true; + then: + - lambda: 'ESP_LOGI("test", "later-check-body-%d", id(counter));' + - logger.log: "after-later-check-should-not-run" + - id: empty_while + then: + - while: + condition: + lambda: 'return true;' + then: [] + - logger.log: "after-empty-while" + - id: nested_if_last_in_while then: - lambda: 'id(counter) = 0;' @@ -206,6 +226,14 @@ api: ESP_LOGI("test", "stop-running=%d", id(stop_in_branch).is_running() ? 1 : 0); - delay: 600ms - logger.log: "stop-check-done" + - action: run_stop_in_later_while_check + then: + - script.execute: stop_in_later_while_check + - delay: 200ms + - logger.log: "later-check-done" + - action: run_empty_while + then: + - script.execute: empty_while - action: run_nested_if_last_in_while then: - script.execute: nested_if_last_in_while diff --git a/tests/integration/fixtures/continuation_actions.yaml b/tests/integration/fixtures/continuation_actions.yaml index bdfe149cb7b..d48569f0f4b 100644 --- a/tests/integration/fixtures/continuation_actions.yaml +++ b/tests/integration/fixtures/continuation_actions.yaml @@ -5,7 +5,7 @@ host: api: actions: - # Test 1: IfAction with ContinuationAction (then/else branches) + # Test 1: IfAction with then/else branches - action: test_if_action variables: condition: bool @@ -27,7 +27,7 @@ api: args: ['value'] - logger.log: "if completed" - # Test 2: Nested IfAction (multiple ContinuationAction instances) + # Test 2: Nested IfAction - action: test_nested_if variables: outer: bool @@ -51,7 +51,7 @@ api: - logger.log: "nested-outer-false" - logger.log: "nested if completed" - # Test 3: WhileAction with WhileLoopContinuation + # Test 3: WhileAction - action: test_while_action variables: max_count: int @@ -74,7 +74,7 @@ api: value: !lambda 'return id(continuation_test_counter) + 1;' - logger.log: "while completed" - # Test 4: RepeatAction with RepeatLoopContinuation + # Test 4: RepeatAction - action: test_repeat_action variables: count: int @@ -90,7 +90,7 @@ api: args: ['iteration'] - logger.log: "repeat completed" - # Test 5: Combined continuations (if + while + repeat) + # Test 5: Combined if + while + repeat - action: test_combined variables: do_loop: bool @@ -123,7 +123,7 @@ api: - logger.log: "combined-skipped" - logger.log: "combined completed" - # Test 6: Rapid triggers to verify memory efficiency + # Test 6: Rapid triggers - action: test_rapid_if then: - logger.log: "=== Rapid if test start ===" @@ -153,7 +153,7 @@ globals: restore_value: false initial_value: '0' -# Sensor to test rapid automation triggers with if/else (ContinuationAction) +# Sensor to test rapid automation triggers with if/else sensor: - platform: template id: rapid_sensor diff --git a/tests/integration/test_branch_resume_async.py b/tests/integration/test_branch_resume_async.py index 78064cbe305..f6fb5dbd457 100644 --- a/tests/integration/test_branch_resume_async.py +++ b/tests/integration/test_branch_resume_async.py @@ -73,6 +73,20 @@ async def test_branch_resume_async( assert count("stopped-branch-should-not-run") == 0 assert count("stopped-after-if-should-not-run") == 0 + # A while condition that stops the script on a later pass must not start the body again + done = wait_for("later-check-done") + await client.execute_service(service["run_stop_in_later_while_check"], {}) + await asyncio.wait_for(done, timeout=2.0) + assert count("later-check-body-1") == 1 + assert count("later-check-body-2") == 0 + assert count("after-later-check-should-not-run") == 0 + + # An empty loop body cannot resume the loop, so the while finishes at once + done = wait_for("after-empty-while") + await client.execute_service(service["run_empty_while"], {}) + await asyncio.wait_for(done, timeout=2.0) + assert count("after-empty-while") == 1 + done = wait_for("after-nested-while") await client.execute_service(service["run_nested_if_last_in_while"], {}) await asyncio.wait_for(done, timeout=2.0) diff --git a/tests/integration/test_continuation_actions.py b/tests/integration/test_continuation_actions.py index e6020c711a1..1c6e8891a96 100644 --- a/tests/integration/test_continuation_actions.py +++ b/tests/integration/test_continuation_actions.py @@ -1,4 +1,4 @@ -"""Test continuation actions (ContinuationAction, WhileLoopContinuation, RepeatLoopContinuation).""" +"""Test that if, while and repeat resume the actions after them once their branch finishes.""" from __future__ import annotations @@ -17,10 +17,7 @@ async def test_continuation_actions( api_client_connected: APIClientConnectedFactory, ) -> None: """ - Test that continuation actions work correctly for if/while/repeat. - - These continuation classes replace LambdaAction with simple parent pointers, - saving 32-36 bytes per instance and eliminating std::function overhead. + Test that if/while/repeat resume the actions after them once their branch finishes. """ loop = asyncio.get_running_loop() @@ -218,7 +215,7 @@ async def test_continuation_actions( ) assert test_results["combined_complete"], "Combined did not complete" - # Test 6: Rapid triggers (tests memory efficiency of ContinuationAction) + # Test 6: Rapid triggers through an if/else test_service = next((s for s in services if s.name == "test_rapid_if"), None) assert test_service is not None, "test_rapid_if service not found" await client.execute_service(test_service, {})