[script] Keep the running script.wait item out of the queue while its chain runs (#20570)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: J. Nick Koston <nick@koston.org>
This commit is contained in:
Frédéric Metrich
2026-10-13 09:31:19 -10:00
committed by GitHub
co-authored by Claude Opus 5.5 J. Nick Koston
parent 417ab4d2e9
commit 918b7d2553
2 changed files with 93 additions and 3 deletions
+7 -3
View File
@@ -295,8 +295,11 @@ template<class C, typename... Ts> class ScriptWaitAction final : public Action<T
}
void loop() override {
if (this->num_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 C, typename... Ts> class ScriptWaitAction final : public Action<T
// Processing all items in a while loop causes infinite loops because
// play_next_() can trigger more items to be queued
if (!this->param_queue_.empty()) {
auto &params = 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();
@@ -2,6 +2,7 @@
#include <memory>
#include <string>
#include <vector>
#include "esphome/components/script/script.h"
@@ -93,4 +94,89 @@ TEST(ScriptWaitActionTest, KeepsReferenceToNonCopyableArguments) {
EXPECT_EQ(entity.value, 1);
}
class StopWaitAction : public Action<const std::string &> {
public:
void play(const std::string &value) override {
this->wait->stop_complex();
this->seen = value;
}
Action<const std::string &> *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<FakeScript, const std::string &> wait(&script);
StopWaitAction stop;
stop.wait = &wait;
ActionList<const std::string &> 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<const std::string &> {
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<const std::string &> *wait{nullptr};
FakeScript *script{nullptr};
ActionList<const std::string &> *actions{nullptr};
std::vector<std::string> seen;
};
// The old pop_front() after the chain removed the item queued by the restart
TEST(ScriptWaitActionTest, RestartDuringChainKeepsNewItem) {
FakeScript script;
ScriptWaitAction<FakeScript, const std::string &> wait(&script);
RestartAction restart;
ActionList<const std::string &> 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<FakeScript, const std::string &> wait(&script);
RecordAction record;
ActionList<const std::string &> 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