From f387d9ec50765b75b9037f6c6176d4a180a68baa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:33:38 -1000 Subject: [PATCH 1/9] unique ptr --- esphome/components/script/script.h | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index 3a97a26985..26192b8997 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -140,8 +140,10 @@ template class QueueingScript : public Script, public Com void stop() override { // Clear all queued items to free memory immediately - for (int i = 0; i < this->max_runs_ - 1; i++) { - this->var_queue_[i].reset(); + if (this->var_queue_) { + for (int i = 0; i < this->max_runs_ - 1; i++) { + this->var_queue_[i].reset(); + } } this->num_queued_ = 0; this->queue_front_ = 0; @@ -164,13 +166,10 @@ template class QueueingScript : public Script, public Com // Lazy init queue on first use - avoids setup() ordering issues and saves memory // if script is never executed during this boot cycle inline void lazy_init_queue_() { - if (this->var_queue_.capacity() == 0) { - // Allocate max_runs_ - 1 slots for queued items (running item is separate) - 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->var_queue_) { + // Allocate array of max_runs_ - 1 slots for queued items (running item is separate) + // unique_ptr array is zero-initialized, so all slots start as nullptr + this->var_queue_ = std::make_unique>[]>(this->max_runs_ - 1); } } @@ -181,7 +180,7 @@ template class QueueingScript : public Script, public Com 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>> var_queue_; // Ring buffer of queued parameters + std::unique_ptr>[]> var_queue_; // Ring buffer of queued parameters }; /** A script type that executes new instances in parallel. From 0e513b41e4390ae5d3fc50a74896d080ec32cfa6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:45:02 -1000 Subject: [PATCH 2/9] preen --- esphome/components/script/script.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index 26192b8997..86edd3b3e4 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -111,8 +111,6 @@ template class RestartScript : public Script { template class QueueingScript : public Script, public Component { public: void execute(Ts... x) override { - this->lazy_init_queue_(); - if (this->is_action_running() || this->num_queued_ > 0) { // num_queued_ is the number of *queued* instances (waiting, not including currently running) // max_runs_ is the maximum *total* instances (running + queued) @@ -123,12 +121,15 @@ template class QueueingScript : public Script, public Com return; } + // Initialize queue on first queued item (after capacity check) + this->lazy_init_queue_(); + this->esp_logd_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' queueing new instance (mode: queued)"), LOG_STR_ARG(this->name_)); // 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(std::make_tuple(x...))); + // Use std::make_unique to replace the unique_ptr + this->var_queue_[write_pos] = std::make_unique>(x...); this->num_queued_++; return; } @@ -144,6 +145,7 @@ template class QueueingScript : public Script, public Com for (int i = 0; i < this->max_runs_ - 1; i++) { this->var_queue_[i].reset(); } + this->var_queue_.reset(); } this->num_queued_ = 0; this->queue_front_ = 0; From f5e5f4ef06c335008a79d81a45783c69e6e77516 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:47:03 -1000 Subject: [PATCH 3/9] preen --- esphome/core/helpers.h | 1 - 1 file changed, 1 deletion(-) diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index dd67836653..326718e974 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -298,7 +298,6 @@ template 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) From 7bb222a574dedea8b70522e8c9bbc904b0c52aa0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:51:51 -1000 Subject: [PATCH 4/9] Update esphome/components/script/script.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/script/script.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index 86edd3b3e4..55967ead06 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -116,7 +116,7 @@ template class QueueingScript : public Script, public Com // max_runs_ is the maximum *total* instances (running + queued) // So we reject when num_queued_ + 1 >= max_runs_ (queued + running >= max) if (this->num_queued_ + 1 >= this->max_runs_) { - this->esp_logw_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' maximum number of queued runs exceeded!"), + this->esp_logw_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' maximum total instances (running + queued) exceeded!"), LOG_STR_ARG(this->name_)); return; } From e0477e3bb19149f0e7ec5c393c1a913b78ffdb2a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Sun, 19 Oct 2025 07:53:21 +0000 Subject: [PATCH 5/9] [pre-commit.ci lite] apply automatic fixes --- esphome/components/script/script.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index 55967ead06..bb26f5b9ef 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -116,7 +116,8 @@ template class QueueingScript : public Script, public Com // max_runs_ is the maximum *total* instances (running + queued) // So we reject when num_queued_ + 1 >= max_runs_ (queued + running >= max) if (this->num_queued_ + 1 >= this->max_runs_) { - this->esp_logw_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' maximum total instances (running + queued) exceeded!"), + this->esp_logw_(__LINE__, + ESPHOME_LOG_FORMAT("Script '%s' maximum total instances (running + queued) exceeded!"), LOG_STR_ARG(this->name_)); return; } From 498dece3828281cc0658d95c73ea5db63812831c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:54:05 -1000 Subject: [PATCH 6/9] suggestions --- esphome/components/script/script.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index 55967ead06..a69049840f 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -116,7 +116,8 @@ template class QueueingScript : public Script, public Com // max_runs_ is the maximum *total* instances (running + queued) // So we reject when num_queued_ + 1 >= max_runs_ (queued + running >= max) if (this->num_queued_ + 1 >= this->max_runs_) { - this->esp_logw_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' maximum total instances (running + queued) exceeded!"), + this->esp_logw_(__LINE__, + ESPHOME_LOG_FORMAT("Script '%s' maximum total instances (running + queued) exceeded!"), LOG_STR_ARG(this->name_)); return; } @@ -126,8 +127,9 @@ template class QueueingScript : public Script, public Com this->esp_logd_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' queueing new instance (mode: queued)"), LOG_STR_ARG(this->name_)); - // 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); + // Ring buffer: write to (queue_front_ + num_queued_) % queue_capacity + const size_t queue_capacity = static_cast(this->max_runs_ - 1); + size_t write_pos = (this->queue_front_ + this->num_queued_) % queue_capacity; // Use std::make_unique to replace the unique_ptr this->var_queue_[write_pos] = std::make_unique>(x...); this->num_queued_++; @@ -142,7 +144,8 @@ template class QueueingScript : public Script, public Com void stop() override { // Clear all queued items to free memory immediately if (this->var_queue_) { - for (int i = 0; i < this->max_runs_ - 1; i++) { + const size_t queue_capacity = static_cast(this->max_runs_ - 1); + for (size_t i = 0; i < queue_capacity; i++) { this->var_queue_[i].reset(); } this->var_queue_.reset(); @@ -156,8 +159,9 @@ template class QueueingScript : public Script, public Com if (this->num_queued_ != 0 && !this->is_action_running()) { // Dequeue: decrement count, move tuple out (frees slot), advance read position this->num_queued_--; + const size_t queue_capacity = static_cast(this->max_runs_ - 1); auto tuple_ptr = std::move(this->var_queue_[this->queue_front_]); - this->queue_front_ = (this->queue_front_ + 1) % (this->max_runs_ - 1); + this->queue_front_ = (this->queue_front_ + 1) % queue_capacity; this->trigger_tuple_(*tuple_ptr, typename gens::type()); } } From 32a1e4584289df94b2f45d3c11601af0cfaf0801 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:54:20 -1000 Subject: [PATCH 7/9] suggestions --- tests/integration/test_script_queued.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_script_queued.py b/tests/integration/test_script_queued.py index 9f4bce6f31..cd6dbba7e2 100644 --- a/tests/integration/test_script_queued.py +++ b/tests/integration/test_script_queued.py @@ -32,7 +32,7 @@ async def test_script_queued( queue_start = re.compile(r"Queue test: START item (\d+)") queue_end = re.compile(r"Queue test: END item (\d+)") queue_reject = re.compile( - r"Script 'queue_depth_script' maximum number of queued runs exceeded!" + r"Script 'queue_depth_script' maximum total instances \(running \+ queued\) exceeded!" ) # Patterns for Test 2: Ring buffer From acdecafeef36067a7033f9f98058699177b8b604 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:55:25 -1000 Subject: [PATCH 8/9] suggestions --- tests/integration/test_script_queued.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_script_queued.py b/tests/integration/test_script_queued.py index cd6dbba7e2..9afaaf3286 100644 --- a/tests/integration/test_script_queued.py +++ b/tests/integration/test_script_queued.py @@ -47,7 +47,7 @@ async def test_script_queued( reject_start = re.compile(r"Rejection test: START (\d+)") reject_end = re.compile(r"Rejection test: END (\d+)") reject_reject = re.compile( - r"Script 'rejection_script' maximum number of queued runs exceeded!" + r"Script 'rejection_script' maximum total instances \(running \+ queued\) exceeded!" ) # Patterns for Test 5: No params From 70479dec0d1ffd33f8c510939519e0329cbb3ba3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 18 Oct 2025 21:57:19 -1000 Subject: [PATCH 9/9] suggestions --- esphome/components/script/script.h | 3 +-- tests/integration/test_script_queued.py | 8 ++------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/esphome/components/script/script.h b/esphome/components/script/script.h index a69049840f..84e1e95bf4 100644 --- a/esphome/components/script/script.h +++ b/esphome/components/script/script.h @@ -116,8 +116,7 @@ template class QueueingScript : public Script, public Com // max_runs_ is the maximum *total* instances (running + queued) // So we reject when num_queued_ + 1 >= max_runs_ (queued + running >= max) if (this->num_queued_ + 1 >= this->max_runs_) { - this->esp_logw_(__LINE__, - ESPHOME_LOG_FORMAT("Script '%s' maximum total instances (running + queued) exceeded!"), + this->esp_logw_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' max instances (running + queued) reached!"), LOG_STR_ARG(this->name_)); return; } diff --git a/tests/integration/test_script_queued.py b/tests/integration/test_script_queued.py index 9afaaf3286..ce1c25b649 100644 --- a/tests/integration/test_script_queued.py +++ b/tests/integration/test_script_queued.py @@ -31,9 +31,7 @@ async def test_script_queued( # Patterns for Test 1: Queue depth queue_start = re.compile(r"Queue test: START item (\d+)") queue_end = re.compile(r"Queue test: END item (\d+)") - queue_reject = re.compile( - r"Script 'queue_depth_script' maximum total instances \(running \+ queued\) exceeded!" - ) + queue_reject = re.compile(r"Script 'queue_depth_script' max instances") # Patterns for Test 2: Ring buffer ring_start = re.compile(r"Ring buffer: START '([A-Z])'") @@ -46,9 +44,7 @@ async def test_script_queued( # Patterns for Test 4: Rejection reject_start = re.compile(r"Rejection test: START (\d+)") reject_end = re.compile(r"Rejection test: END (\d+)") - reject_reject = re.compile( - r"Script 'rejection_script' maximum total instances \(running \+ queued\) exceeded!" - ) + reject_reject = re.compile(r"Script 'rejection_script' max instances") # Patterns for Test 5: No params no_params_end = re.compile(r"No params: END")