mirror of
https://github.com/esphome/esphome.git
synced 2025-10-22 19:53:46 +01:00
[script] BREAKING: Fix unbounded queue growth, optimize queued mode (default max_runs=5)
This commit is contained in:
@@ -45,8 +45,13 @@ def get_script(script_id):
|
||||
|
||||
|
||||
def check_max_runs(value):
|
||||
# Set default for queued mode to prevent unbounded queue growth
|
||||
if CONF_MAX_RUNS not in value and value[CONF_MODE] == CONF_QUEUED:
|
||||
value[CONF_MAX_RUNS] = 5
|
||||
|
||||
if CONF_MAX_RUNS not in value:
|
||||
return value
|
||||
|
||||
if value[CONF_MODE] not in [CONF_QUEUED, CONF_PARALLEL]:
|
||||
raise cv.Invalid(
|
||||
"The option 'max_runs' is only valid in 'queue' and 'parallel' mode.",
|
||||
@@ -106,7 +111,7 @@ CONFIG_SCHEMA = automation.validate_automation(
|
||||
cv.Optional(CONF_MODE, default=CONF_SINGLE): cv.one_of(
|
||||
*SCRIPT_MODES, lower=True
|
||||
),
|
||||
cv.Optional(CONF_MAX_RUNS): cv.positive_int,
|
||||
cv.Optional(CONF_MAX_RUNS): cv.int_range(min=1, max=100),
|
||||
cv.Optional(CONF_PARAMETERS, default={}): cv.Schema(
|
||||
{
|
||||
validate_parameter_name: validate_parameter_type,
|
||||
|
@@ -2,9 +2,8 @@
|
||||
|
||||
#include "esphome/core/automation.h"
|
||||
#include "esphome/core/component.h"
|
||||
#include "esphome/core/helpers.h"
|
||||
#include "esphome/core/log.h"
|
||||
|
||||
#include <queue>
|
||||
namespace esphome {
|
||||
namespace script {
|
||||
|
||||
@@ -96,14 +95,36 @@ template<typename... Ts> class RestartScript : public Script<Ts...> {
|
||||
/** A script type that queues new instances that are created.
|
||||
*
|
||||
* Only one instance of the script can be active at a time.
|
||||
*
|
||||
* Ring buffer implementation:
|
||||
* - num_queued_ tracks the number of queued (waiting) instances, NOT including the currently running one
|
||||
* - queue_front_ points to the next item to execute (read position)
|
||||
* - Buffer size is (max_runs_ - 1) since one instance is always running (not queued)
|
||||
* - Write position is calculated as: (queue_front_ + num_queued_) % (max_runs_ - 1)
|
||||
* - When an item finishes, queue_front_ advances: (queue_front_ + 1) % (max_runs_ - 1)
|
||||
* - First execute() runs immediately without queuing (num_queued_ stays 0)
|
||||
* - Subsequent executes while running are queued starting at position 0
|
||||
* - Maximum total instances = max_runs_ (one running + max_runs_-1 queued)
|
||||
*/
|
||||
template<typename... Ts> class QueueingScript : public Script<Ts...>, public Component {
|
||||
public:
|
||||
void execute(Ts... x) override {
|
||||
if (this->is_action_running() || this->num_runs_ > 0) {
|
||||
// num_runs_ is the number of *queued* instances, so total number of instances is
|
||||
// num_runs_ + 1
|
||||
if (this->max_runs_ != 0 && this->num_runs_ + 1 >= this->max_runs_) {
|
||||
// Lazy init on first use - avoids setup() ordering issues and saves memory
|
||||
// if script is never executed during this boot cycle
|
||||
if (this->var_queue_.capacity() == 0) {
|
||||
// Allocate max_runs_ - 1 slots since one instance is always running (not queued)
|
||||
this->var_queue_.init(this->max_runs_ - 1);
|
||||
// Initialize all unique_ptr slots to nullptr
|
||||
for (int i = 0; i < this->max_runs_ - 1; i++) {
|
||||
this->var_queue_.push_back(nullptr);
|
||||
}
|
||||
}
|
||||
|
||||
if (this->is_action_running() || this->num_queued_ > 0) {
|
||||
// num_queued_ is the number of *queued* instances (waiting, not including currently running)
|
||||
// Total active instances = 1 (running) + num_queued_ (queued)
|
||||
// So we reject when num_queued_ + 1 >= max_runs_
|
||||
if (this->num_queued_ + 1 >= this->max_runs_) {
|
||||
this->esp_logw_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' maximum number of queued runs exceeded!"),
|
||||
LOG_STR_ARG(this->name_));
|
||||
return;
|
||||
@@ -111,8 +132,11 @@ template<typename... Ts> class QueueingScript : public Script<Ts...>, public Com
|
||||
|
||||
this->esp_logd_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' queueing new instance (mode: queued)"),
|
||||
LOG_STR_ARG(this->name_));
|
||||
this->num_runs_++;
|
||||
this->var_queue_.push(std::make_tuple(x...));
|
||||
// Ring buffer: write to (queue_front_ + num_queued_) % (max_runs_ - 1)
|
||||
size_t write_pos = (this->queue_front_ + this->num_queued_) % (this->max_runs_ - 1);
|
||||
// Use reset() to replace the unique_ptr
|
||||
this->var_queue_[write_pos].reset(new std::tuple<Ts...>(std::make_tuple(x...)));
|
||||
this->num_queued_++;
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -122,15 +146,17 @@ template<typename... Ts> class QueueingScript : public Script<Ts...>, public Com
|
||||
}
|
||||
|
||||
void stop() override {
|
||||
this->num_runs_ = 0;
|
||||
this->num_queued_ = 0;
|
||||
this->queue_front_ = 0;
|
||||
Script<Ts...>::stop();
|
||||
}
|
||||
|
||||
void loop() override {
|
||||
if (this->num_runs_ != 0 && !this->is_action_running()) {
|
||||
this->num_runs_--;
|
||||
auto &vars = this->var_queue_.front();
|
||||
this->var_queue_.pop();
|
||||
if (this->num_queued_ != 0 && !this->is_action_running()) {
|
||||
// Dequeue: decrement count, read from front, advance read position
|
||||
this->num_queued_--;
|
||||
auto &vars = *this->var_queue_[this->queue_front_];
|
||||
this->queue_front_ = (this->queue_front_ + 1) % (this->max_runs_ - 1);
|
||||
this->trigger_tuple_(vars, typename gens<sizeof...(Ts)>::type());
|
||||
}
|
||||
}
|
||||
@@ -142,9 +168,10 @@ template<typename... Ts> class QueueingScript : public Script<Ts...>, public Com
|
||||
this->trigger(std::get<S>(tuple)...);
|
||||
}
|
||||
|
||||
int num_runs_ = 0;
|
||||
int max_runs_ = 0;
|
||||
std::queue<std::tuple<Ts...>> var_queue_;
|
||||
int num_queued_ = 0; // Number of queued instances (not including currently running)
|
||||
int max_runs_ = 0; // Maximum total instances (running + queued)
|
||||
size_t queue_front_ = 0; // Ring buffer read position (next item to execute)
|
||||
FixedVector<std::unique_ptr<std::tuple<Ts...>>> var_queue_; // Ring buffer of queued parameters
|
||||
};
|
||||
|
||||
/** A script type that executes new instances in parallel.
|
||||
|
@@ -298,6 +298,7 @@ template<typename T> class FixedVector {
|
||||
const T &back() const { return data_[size_ - 1]; }
|
||||
|
||||
size_t size() const { return size_; }
|
||||
size_t capacity() const { return capacity_; }
|
||||
bool empty() const { return size_ == 0; }
|
||||
|
||||
/// Access element without bounds checking (matches std::vector behavior)
|
||||
|
Reference in New Issue
Block a user