diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index f00ad7f4afa..efd75d27e0e 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -295,8 +295,11 @@ template class ScriptWaitAction final : public Actionnum_running_ == 0) + if (this->num_running_ == 0) { + // Nothing left to wait for, e.g. once the last queued item has run + this->disable_loop(); return; + } if (this->script_->is_running()) return; @@ -305,9 +308,10 @@ template class ScriptWaitAction final : public Actionparam_queue_.empty()) { - auto ¶ms = this->param_queue_.front(); - std::apply([this](auto &...args) { this->play_next_(args...); }, params); + // Take the item out first: the chain may call stop(), which clears the queue + auto params = std::move(this->param_queue_.front()); this->param_queue_.pop_front(); + std::apply([this](auto &...args) { this->play_next_(args...); }, params); } else { // Queue is now empty - disable loop until next play_complex this->disable_loop(); diff --git a/tests/components/script/test_script_wait_action.cpp b/tests/components/script/test_script_wait_action.cpp index 17f553996ea..c3cd55fc8ca 100644 --- a/tests/components/script/test_script_wait_action.cpp +++ b/tests/components/script/test_script_wait_action.cpp @@ -2,6 +2,7 @@ #include #include +#include #include "esphome/components/script/script.h" @@ -93,4 +94,89 @@ TEST(ScriptWaitActionTest, KeepsReferenceToNonCopyableArguments) { EXPECT_EQ(entity.value, 1); } +class StopWaitAction : public Action { + public: + void play(const std::string &value) override { + this->wait->stop_complex(); + this->seen = value; + } + Action *wait{nullptr}; + std::string seen; +}; + +// The chain after the wait may stop it, e.g. a script with mode: restart +TEST(ScriptWaitActionTest, StopDuringChainKeepsArguments) { + FakeScript script; + ScriptWaitAction wait(&script); + StopWaitAction stop; + stop.wait = &wait; + ActionList actions; + actions.add_actions({&wait, &stop}); + + // With a second item waiting, stop() clears the queue while the first item's chain runs + actions.play(std::string("argument longer than the small string buffer")); + actions.play(std::string("second argument, also longer than the buffer")); + script.running = false; + wait.loop(); + EXPECT_EQ(stop.seen, "argument longer than the small string buffer"); + wait.loop(); + EXPECT_EQ(stop.seen, "argument longer than the small string buffer"); +} + +class RestartAction : public Action { + public: + void play(const std::string &value) override { + this->seen.push_back(value); + if (this->seen.size() > 1) + return; + // What a script with mode: restart does: stop, then run again while the awaited script is busy + this->wait->stop_complex(); + this->script->running = true; + this->actions->play(std::string("restarted, longer than the small string buffer")); + } + Action *wait{nullptr}; + FakeScript *script{nullptr}; + ActionList *actions{nullptr}; + std::vector seen; +}; + +// The old pop_front() after the chain removed the item queued by the restart +TEST(ScriptWaitActionTest, RestartDuringChainKeepsNewItem) { + FakeScript script; + ScriptWaitAction wait(&script); + RestartAction restart; + ActionList actions; + actions.add_actions({&wait, &restart}); + restart.wait = &wait; + restart.script = &script; + restart.actions = &actions; + + // With a second item waiting, the stop really clears the queue; the restart then queues a new item + actions.play(std::string("first argument, longer than the small string buffer")); + actions.play(std::string("second argument, dropped by the stop")); + script.running = false; + wait.loop(); + ASSERT_EQ(restart.seen.size(), 1u); + + script.running = false; + wait.loop(); + ASSERT_EQ(restart.seen.size(), 2u); + EXPECT_EQ(restart.seen[1], "restarted, longer than the small string buffer"); +} + +TEST(ScriptWaitActionTest, LoopDisabledOnceNothingWaits) { + FakeScript script; + ScriptWaitAction wait(&script); + RecordAction record; + ActionList actions; + actions.add_actions({&wait, &record}); + + actions.play(std::string("argument")); + script.running = false; + wait.loop(); + EXPECT_EQ(record.seen, "argument"); + wait.loop(); + EXPECT_TRUE(wait.is_idle()); +} + } // namespace esphome::script::testing