fix assumptions

esphome/components/script/script.h

@@ -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_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)
+ * - Buffer size is max_runs_ (the maximum number that can be queued)
+ * - Write position is calculated as: (queue_front_ + num_queued_) % max_runs_
+ * - When an item finishes, queue_front_ advances: (queue_front_ + 1) % max_runs_
  * - 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)
+ * - Maximum total instances = 1 running + max_runs_ queued
  */
 template<typename... Ts> class QueueingScript : public Script<Ts...>, public Component {
  public:
@@ -112,19 +112,19 @@ template<typename... Ts> class QueueingScript : public Script<Ts...>, public Com
     // 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);
+      // Allocate max_runs_ slots for queued items (running item is separate)
+      this->var_queue_.init(this->max_runs_);
       // Initialize all unique_ptr slots to nullptr
-      for (int i = 0; i < this->max_runs_ - 1; i++) {
+      for (int i = 0; i < this->max_runs_; 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_) {
+      // max_runs_ is the maximum number that can be queued
+      // So we reject when num_queued_ >= max_runs_
+      if (this->num_queued_ >= this->max_runs_) {
         this->esp_logw_(__LINE__, ESPHOME_LOG_FORMAT("Script '%s' maximum number of queued runs exceeded!"),
                         LOG_STR_ARG(this->name_));
         return;
@@ -132,8 +132,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_runs_ - 1)
-      size_t write_pos = (this->queue_front_ + this->num_queued_) % (this->max_runs_ - 1);
+      // Ring buffer: write to (queue_front_ + num_queued_) % max_runs_
+      size_t write_pos = (this->queue_front_ + this->num_queued_) % this->max_runs_;
       // Use reset() to replace the unique_ptr
       this->var_queue_[write_pos].reset(new std::tuple<Ts...>(std::make_tuple(x...)));
       this->num_queued_++;
@@ -156,7 +156,7 @@ 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_runs_ - 1);
+      this->queue_front_ = (this->queue_front_ + 1) % this->max_runs_;
       this->trigger_tuple_(vars, typename gens<sizeof...(Ts)>::type());
     }
   }
@@ -169,7 +169,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_runs_ = 0;        // Maximum total instances (running + queued)
+  int max_runs_ = 0;        // Maximum number of queued instances (not including running)
   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
 };

tests/integration/fixtures/script_queued.yaml

@@ -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 (should process 1-5, reject 6) ==="
+        - logger.log: "=== TEST 1: Queue depth (max_runs=5 means 1 running + 5 queued = 6 total, reject 7) ==="
         - script.execute:
             id: queue_depth_script
             value: 1
@@ -26,6 +26,9 @@ api:
         - script.execute:
             id: queue_depth_script
             value: 6
+        - script.execute:
+            id: queue_depth_script
+            value: 7
 
     # Test 2: Ring buffer wrap test
     - action: test_ring_buffer
@@ -61,7 +64,7 @@ api:
     # Test 4: Verify rejection (max_runs=3)
     - action: test_rejection
       then:
-        - logger.log: "=== TEST 4: Verify rejection (max_runs=3, try 8) ==="
+        - logger.log: "=== TEST 4: Verify rejection (max_runs=3 means 1 running + 3 queued = 4 total, reject 5-8) ==="
         - script.execute:
             id: rejection_script
             val: 1

tests/integration/test_script_queued.py

@@ -70,7 +70,7 @@ async def test_script_queued(
 
         if match := queue_end.search(line):
             item = int(match.group(1))
-            if item == 5 and not test1_complete.done():
+            if item == 6 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 == 3 and not test4_complete.done():
+            if item == 4 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], (
-            f"Test 1: Expected to process items 1-5, got {sorted(test_results['queue_depth']['processed'])}"
+        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 test_results["queue_depth"]["rejections"] > 0, (
-            "Test 1: Expected at least one rejection warning"
+            "Test 1: Expected at least one rejection warning (item 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], (
-            f"Test 4: Expected to process items 1-3, got {sorted(test_results['rejection']['processed'])}"
+        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 test_results["rejection"]["rejections"] == 5, (
-            f"Test 4: Expected 5 rejections (items 4-8), got {test_results['rejection']['rejections']}"
+        assert test_results["rejection"]["rejections"] == 4, (
+            f"Test 4: Expected 4 rejections (items 5-8), got {test_results['rejection']['rejections']}"
         )
 
         # Test 5: No parameters