sascha/esphome
1
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2025-10-21 19:23:45 +01:00
Code Issues Releases Wiki Activity

max_runs was actually correct after re-testing dev

Browse Source
This commit is contained in:
J. Nick Koston
2025-10-16 13:20:42 -10:00
parent 353d8b8fb2
commit f2ec2c3fbf
3 changed files with 27 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
esphome/components/script/script.h
View File

@@ -99,12 +99,12 @@ template<typename... Ts> class RestartScript : public Script<Ts...> {
* 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_queued_ (the maximum number that can be queued)
* - Write position is calculated as: (queue_front_ + num_queued_) % max_queued_
* - When an item finishes, queue_front_ advances: (queue_front_ + 1) % max_queued_
* - Buffer size is max_runs_ - 1 (max total instances minus the running one)
* - 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 = 1 running + max_queued_ queued
* - Maximum total instances = max_runs_ (includes 1 running + (max_runs_ - 1) queued)
*/
template<typename... Ts> class QueueingScript : public Script<Ts...>, public Component {
public:
@@ -113,9 +113,9 @@ template<typename... Ts> class QueueingScript : public Script<Ts...>, public Com
if (this->is_action_running() || this->num_queued_ > 0) {
// num_queued_ is the number of *queued* instances (waiting, not including currently running)
// max_queued_ is the maximum number that can be queued
// So we reject when num_queued_ >= max_queued_
if (this->num_queued_ >= this->max_queued_) {
// 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!"),
LOG_STR_ARG(this->name_));
return;
@@ -123,8 +123,8 @@ 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_));
// Ring buffer: write to (queue_front_ + num_queued_) % max_queued_
size_t write_pos = (this->queue_front_ + this->num_queued_) % this->max_queued_;
// 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_++;
@@ -147,24 +147,22 @@ template<typename... Ts> class QueueingScript : public Script<Ts...>, public Com
// 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_queued_;
this->queue_front_ = (this->queue_front_ + 1) % (this->max_runs_ - 1);
this->trigger_tuple_(vars, typename gens<sizeof...(Ts)>::type());
}
}
// Note: Method named set_max_runs() for backward compatibility with existing configs,
// but internally uses max_queued_ to clarify that it sets the max *queued* instances
void set_max_runs(int max_runs) { max_queued_ = max_runs; }
void set_max_runs(int max_runs) { max_runs_ = max_runs; }
protected:
// 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_queued_ slots for queued items (running item is separate)
this->var_queue_.init(this->max_queued_);
// 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_queued_; i++) {
for (int i = 0; i < this->max_runs_ - 1; i++) {
this->var_queue_.push_back(nullptr);
}
}
@@ -175,7 +173,7 @@ template<typename... Ts> class QueueingScript : public Script<Ts...>, public Com
}
int num_queued_ = 0; // Number of queued instances (not including currently running)
int max_queued_ = 0; // Maximum number of queued instances (not including 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
};

4
tests/integration/fixtures/script_queued.yaml
View File

@@ -7,7 +7,7 @@ api:
# Test 1: Queue depth with default max_runs=5
- action: test_queue_depth
then:
- logger.log: "=== TEST 1: Queue depth (max_runs=5 means 1 running + 5 queued = 6 total, reject 7) ==="
- logger.log: "=== TEST 1: Queue depth (max_runs=5 means 5 total, reject 6-7) ==="
- script.execute:
id: queue_depth_script
value: 1
@@ -64,7 +64,7 @@ api:
# Test 4: Verify rejection (max_runs=3)
- action: test_rejection
then:
- logger.log: "=== TEST 4: Verify rejection (max_runs=3 means 1 running + 3 queued = 4 total, reject 5-8) ==="
- logger.log: "=== TEST 4: Verify rejection (max_runs=3 means 3 total, reject 4-8) ==="
- script.execute:
id: rejection_script
val: 1

20
tests/integration/test_script_queued.py
View File

@@ -70,7 +70,7 @@ async def test_script_queued(
if match := queue_end.search(line):
item = int(match.group(1))
if item == 6 and not test1_complete.done():
if item == 5 and not test1_complete.done():
test1_complete.set_result(True)
if queue_reject.search(line):
@@ -115,7 +115,7 @@ async def test_script_queued(
if match := reject_end.search(line):
item = int(match.group(1))
if item == 4 and not test4_complete.done():
if item == 3 and not test4_complete.done():
test4_complete.set_result(True)
if reject_reject.search(line):
@@ -145,11 +145,11 @@ async def test_script_queued(
await asyncio.sleep(0.1) # Give time for rejections
# Verify Test 1
assert sorted(test_results["queue_depth"]["processed"]) == [1, 2, 3, 4, 5, 6], (
f"Test 1: Expected to process items 1-6 (max_runs=5 means 5 queued + 1 running), got {sorted(test_results['queue_depth']['processed'])}"
assert sorted(test_results["queue_depth"]["processed"]) == [1, 2, 3, 4, 5], (
f"Test 1: Expected to process items 1-5 (max_runs=5 means 5 total), got {sorted(test_results['queue_depth']['processed'])}"
)
assert test_results["queue_depth"]["rejections"] > 0, (
"Test 1: Expected at least one rejection warning (item 7 should be rejected)"
assert test_results["queue_depth"]["rejections"] >= 2, (
"Test 1: Expected at least 2 rejection warnings (items 6-7 should be rejected)"
)
# Test 2: Ring buffer order
@@ -188,11 +188,11 @@ async def test_script_queued(
await asyncio.sleep(0.1) # Give time for rejections
# Verify Test 4
assert sorted(test_results["rejection"]["processed"]) == [1, 2, 3, 4], (
f"Test 4: Expected to process items 1-4 (max_runs=3 means 3 queued + 1 running), got {sorted(test_results['rejection']['processed'])}"
assert sorted(test_results["rejection"]["processed"]) == [1, 2, 3], (
f"Test 4: Expected to process items 1-3 (max_runs=3 means 3 total), got {sorted(test_results['rejection']['processed'])}"
)
assert test_results["rejection"]["rejections"] == 4, (
f"Test 4: Expected 4 rejections (items 5-8), got {test_results['rejection']['rejections']}"
assert test_results["rejection"]["rejections"] == 5, (
f"Test 4: Expected 5 rejections (items 4-8), got {test_results['rejection']['rejections']}"
)
# Test 5: No parameters