mirror of
https://github.com/esphome/esphome.git
synced 2026-10-14 02:02:51 +00:00
[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:
co-authored by
Claude Opus 5.5
J. Nick Koston
parent
b43630cf8e
commit
bb0546fc0b
@@ -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 ¶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();
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user