Merge branch 'retry_string_cleanup' into integration

esphome/core/scheduler.cpp

@@ -83,6 +83,7 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type
   item->type = type;
   item->callback = std::move(func);
   item->remove = false;
+  item->is_retry = is_retry;
 
 #ifndef ESPHOME_THREAD_SINGLE
   // Special handling for defer() (delay = 0, type = TIMEOUT)
@@ -134,8 +135,8 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type
 
   // For retries, check if there's a cancelled timeout first
   if (is_retry && name_cstr != nullptr && type == SchedulerItem::TIMEOUT &&
-      (has_cancelled_timeout_in_container_(this->items_, component, name_cstr) ||
-       has_cancelled_timeout_in_container_(this->to_add_, component, name_cstr))) {
+      (has_cancelled_timeout_in_container_(this->items_, component, name_cstr, /* match_retry= */ true) ||
+       has_cancelled_timeout_in_container_(this->to_add_, component, name_cstr, /* match_retry= */ true))) {
     // Skip scheduling - the retry was cancelled
 #ifdef ESPHOME_DEBUG_SCHEDULER
     ESP_LOGD(TAG, "Skipping retry '%s' - found cancelled item", name_cstr);
@@ -198,25 +199,27 @@ void retry_handler(const std::shared_ptr<RetryArgs> &args) {
   // second execution of `func` happens after `initial_wait_time`
   args->scheduler->set_timer_common_(
       args->component, Scheduler::SchedulerItem::TIMEOUT, false, &args->name, args->current_interval,
-      [args]() { retry_handler(args); }, true);
+      [args]() { retry_handler(args); }, /* is_retry= */ true);
   // backoff_increase_factor applied to third & later executions
   args->current_interval *= args->backoff_increase_factor;
 }
 
-void HOT Scheduler::set_retry(Component *component, const std::string &name, uint32_t initial_wait_time,
-                              uint8_t max_attempts, std::function<RetryResult(uint8_t)> func,
-                              float backoff_increase_factor) {
-  if (!name.empty())
-    this->cancel_retry(component, name);
+void HOT Scheduler::set_retry_common_(Component *component, bool is_static_string, const void *name_ptr,
+                                      uint32_t initial_wait_time, uint8_t max_attempts,
+                                      std::function<RetryResult(uint8_t)> func, float backoff_increase_factor) {
+  const char *name_cstr = this->get_name_cstr_(is_static_string, name_ptr);
+
+  if (name_cstr != nullptr)
+    this->cancel_retry(component, name_cstr);
 
   if (initial_wait_time == SCHEDULER_DONT_RUN)
     return;
 
   ESP_LOGVV(TAG, "set_retry(name='%s', initial_wait_time=%" PRIu32 ", max_attempts=%u, backoff_factor=%0.1f)",
-            name.c_str(), initial_wait_time, max_attempts, backoff_increase_factor);
+            name_cstr ? name_cstr : "", initial_wait_time, max_attempts, backoff_increase_factor);
 
   if (backoff_increase_factor < 0.0001) {
-    ESP_LOGE(TAG, "backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, name.c_str());
+    ESP_LOGE(TAG, "backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, name_cstr ? name_cstr : "");
     backoff_increase_factor = 1;
   }
 
@@ -225,15 +228,36 @@ void HOT Scheduler::set_retry(Component *component, const std::string &name, uin
   args->retry_countdown = max_attempts;
   args->current_interval = initial_wait_time;
   args->component = component;
-  args->name = "retry$" + name;
+  args->name = name_cstr ? name_cstr : "";  // Convert to std::string for RetryArgs
   args->backoff_increase_factor = backoff_increase_factor;
   args->scheduler = this;
 
-  // First execution of `func` immediately
-  this->set_timeout(component, args->name, 0, [args]() { retry_handler(args); });
+  // First execution of `func` immediately - use set_timer_common_ with is_retry=true
+  this->set_timer_common_(
+      component, SchedulerItem::TIMEOUT, false, &args->name, 0, [args]() { retry_handler(args); },
+      /* is_retry= */ true);
+}
+
+void HOT Scheduler::set_retry(Component *component, const std::string &name, uint32_t initial_wait_time,
+                              uint8_t max_attempts, std::function<RetryResult(uint8_t)> func,
+                              float backoff_increase_factor) {
+  this->set_retry_common_(component, false, &name, initial_wait_time, max_attempts, std::move(func),
+                          backoff_increase_factor);
+}
+
+void HOT Scheduler::set_retry(Component *component, const char *name, uint32_t initial_wait_time, uint8_t max_attempts,
+                              std::function<RetryResult(uint8_t)> func, float backoff_increase_factor) {
+  this->set_retry_common_(component, true, name, initial_wait_time, max_attempts, std::move(func),
+                          backoff_increase_factor);
 }
 bool HOT Scheduler::cancel_retry(Component *component, const std::string &name) {
-  return this->cancel_timeout(component, "retry$" + name);
+  return this->cancel_retry(component, name.c_str());
+}
+
+bool HOT Scheduler::cancel_retry(Component *component, const char *name) {
+  // Cancel timeouts that have is_retry flag set
+  LockGuard guard{this->lock_};
+  return this->cancel_item_locked_(component, name, SchedulerItem::TIMEOUT, /* match_retry= */ true);
 }
 
 optional<uint32_t> HOT Scheduler::next_schedule_in(uint32_t now) {
@@ -479,7 +503,8 @@ bool HOT Scheduler::cancel_item_(Component *component, bool is_static_string, co
 }
 
 // Helper to cancel items by name - must be called with lock held
-bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_cstr, SchedulerItem::Type type) {
+bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_cstr, SchedulerItem::Type type,
+                                        bool match_retry) {
   // Early return if name is invalid - no items to cancel
   if (name_cstr == nullptr) {
     return false;
@@ -492,7 +517,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c
   // Only check defer queue for timeouts (intervals never go there)
   if (type == SchedulerItem::TIMEOUT) {
     for (auto &item : this->defer_queue_) {
-      if (this->matches_item_(item, component, name_cstr, type)) {
+      if (this->matches_item_(item, component, name_cstr, type, match_retry)) {
         item->remove = true;
         total_cancelled++;
       }
@@ -502,7 +527,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c
 
   // Cancel items in the main heap
   for (auto &item : this->items_) {
-    if (this->matches_item_(item, component, name_cstr, type)) {
+    if (this->matches_item_(item, component, name_cstr, type, match_retry)) {
       item->remove = true;
       total_cancelled++;
       this->to_remove_++;  // Track removals for heap items
@@ -511,7 +536,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c
 
   // Cancel items in to_add_
   for (auto &item : this->to_add_) {
-    if (this->matches_item_(item, component, name_cstr, type)) {
+    if (this->matches_item_(item, component, name_cstr, type, match_retry)) {
       item->remove = true;
       total_cancelled++;
       // Don't track removals for to_add_ items

esphome/core/scheduler.h

@@ -61,7 +61,10 @@ class Scheduler {
   bool cancel_interval(Component *component, const char *name);
   void set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts,
                  std::function<RetryResult(uint8_t)> func, float backoff_increase_factor = 1.0f);
+  void set_retry(Component *component, const char *name, uint32_t initial_wait_time, uint8_t max_attempts,
+                 std::function<RetryResult(uint8_t)> func, float backoff_increase_factor = 1.0f);
   bool cancel_retry(Component *component, const std::string &name);
+  bool cancel_retry(Component *component, const char *name);
 
   // Calculate when the next scheduled item should run
   // @param now Fresh timestamp from millis() - must not be stale/cached
@@ -98,11 +101,18 @@ class Scheduler {
     enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1;
     bool remove : 1;
     bool name_is_dynamic : 1;  // True if name was dynamically allocated (needs delete[])
-    // 5 bits padding
+    bool is_retry : 1;         // True if this is a retry timeout
+    // 4 bits padding
 
     // Constructor
     SchedulerItem()
-        : component(nullptr), interval(0), next_execution_(0), type(TIMEOUT), remove(false), name_is_dynamic(false) {
+        : component(nullptr),
+          interval(0),
+          next_execution_(0),
+          type(TIMEOUT),
+          remove(false),
+          name_is_dynamic(false),
+          is_retry(false) {
       name_.static_name = nullptr;
     }
 
@@ -156,6 +166,10 @@ class Scheduler {
   void set_timer_common_(Component *component, SchedulerItem::Type type, bool is_static_string, const void *name_ptr,
                          uint32_t delay, std::function<void()> func, bool is_retry = false);
 
+  // Common implementation for retry
+  void set_retry_common_(Component *component, bool is_static_string, const void *name_ptr, uint32_t initial_wait_time,
+                         uint8_t max_attempts, std::function<RetryResult(uint8_t)> func, float backoff_increase_factor);
+
   uint64_t millis_64_(uint32_t now);
   // Cleanup logically deleted items from the scheduler
   // Returns the number of items remaining after cleanup
@@ -165,7 +179,7 @@ class Scheduler {
 
  private:
   // Helper to cancel items by name - must be called with lock held
-  bool cancel_item_locked_(Component *component, const char *name, SchedulerItem::Type type);
+  bool cancel_item_locked_(Component *component, const char *name, SchedulerItem::Type type, bool match_retry = false);
 
   // Helper to extract name as const char* from either static string or std::string
   inline const char *get_name_cstr_(bool is_static_string, const void *name_ptr) {
@@ -177,8 +191,9 @@ class Scheduler {
 
   // Helper function to check if item matches criteria for cancellation
   inline bool HOT matches_item_(const std::unique_ptr<SchedulerItem> &item, Component *component, const char *name_cstr,
-                                SchedulerItem::Type type, bool skip_removed = true) const {
-    if (item->component != component || item->type != type || (skip_removed && item->remove)) {
+                                SchedulerItem::Type type, bool match_retry, bool skip_removed = true) const {
+    if (item->component != component || item->type != type || (skip_removed && item->remove) ||
+        (match_retry && !item->is_retry)) {
       return false;
     }
     const char *item_name = item->get_name();
@@ -206,10 +221,11 @@ class Scheduler {
 
   // Template helper to check if any item in a container matches our criteria
   template<typename Container>
-  bool has_cancelled_timeout_in_container_(const Container &container, Component *component,
-                                           const char *name_cstr) const {
+  bool has_cancelled_timeout_in_container_(const Container &container, Component *component, const char *name_cstr,
+                                           bool match_retry) const {
     for (const auto &item : container) {
-      if (item->remove && this->matches_item_(item, component, name_cstr, SchedulerItem::TIMEOUT, false)) {
+      if (item->remove && this->matches_item_(item, component, name_cstr, SchedulerItem::TIMEOUT, match_retry,
+                                              /* skip_removed= */ false)) {
         return true;
       }
     }

tests/integration/fixtures/scheduler_retry_test.yaml

@@ -37,6 +37,15 @@ globals:
   - id: multiple_same_name_counter
     type: int
     initial_value: '0'
+  - id: const_char_retry_counter
+    type: int
+    initial_value: '0'
+  - id: static_char_retry_counter
+    type: int
+    initial_value: '0'
+  - id: mixed_cancel_result
+    type: bool
+    initial_value: 'false'
 
 # Using different component types for each test to ensure isolation
 sensor:
@@ -229,6 +238,56 @@ script:
               return RetryResult::RETRY;
             });
 
+      # Test 8: Const char* overloads
+      - logger.log: "=== Test 8: Const char* overloads ==="
+      - lambda: |-
+          auto *component = id(simple_retry_sensor);
+
+          // Test 8a: Direct string literal
+          App.scheduler.set_retry(component, "const_char_test", 30, 2,
+            [](uint8_t retry_countdown) {
+              id(const_char_retry_counter)++;
+              ESP_LOGI("test", "Const char retry %d", id(const_char_retry_counter));
+              return RetryResult::DONE;
+            });
+
+      # Test 9: Static const char* variable
+      - logger.log: "=== Test 9: Static const char* ==="
+      - lambda: |-
+          auto *component = id(backoff_retry_sensor);
+
+          static const char* STATIC_NAME = "static_retry_test";
+          App.scheduler.set_retry(component, STATIC_NAME, 20, 1,
+            [](uint8_t retry_countdown) {
+              id(static_char_retry_counter)++;
+              ESP_LOGI("test", "Static const char retry %d", id(static_char_retry_counter));
+              return RetryResult::DONE;
+            });
+
+          // Cancel with same static const char*
+          App.scheduler.set_timeout(component, "static_cancel", 10, []() {
+            static const char* STATIC_NAME = "static_retry_test";
+            bool result = App.scheduler.cancel_retry(id(backoff_retry_sensor), STATIC_NAME);
+            ESP_LOGI("test", "Static cancel result: %s", result ? "true" : "false");
+          });
+
+      # Test 10: Mix string and const char* cancel
+      - logger.log: "=== Test 10: Mixed string/const char* ==="
+      - lambda: |-
+          auto *component = id(immediate_done_sensor);
+
+          // Set with std::string
+          std::string str_name = "mixed_retry";
+          App.scheduler.set_retry(component, str_name, 40, 3,
+            [](uint8_t retry_countdown) {
+              ESP_LOGI("test", "Mixed retry - should be cancelled");
+              return RetryResult::RETRY;
+            });
+
+          // Cancel with const char*
+          id(mixed_cancel_result) = App.scheduler.cancel_retry(component, "mixed_retry");
+          ESP_LOGI("test", "Mixed cancel result: %s", id(mixed_cancel_result) ? "true" : "false");
+
       # Wait for all tests to complete before reporting
       - delay: 500ms
 
@@ -242,4 +301,7 @@ script:
           ESP_LOGI("test", "Empty name retry counter: %d (expected 1-2)", id(empty_name_retry_counter));
           ESP_LOGI("test", "Component retry counter: %d (expected 2)", id(script_retry_counter));
           ESP_LOGI("test", "Multiple same name counter: %d (expected 20+)", id(multiple_same_name_counter));
+          ESP_LOGI("test", "Const char retry counter: %d (expected 1)", id(const_char_retry_counter));
+          ESP_LOGI("test", "Static char retry counter: %d (expected 1)", id(static_char_retry_counter));
+          ESP_LOGI("test", "Mixed cancel result: %s (expected true)", id(mixed_cancel_result) ? "true" : "false");
           ESP_LOGI("test", "All retry tests completed");

tests/integration/test_scheduler_retry_test.py

@@ -23,6 +23,9 @@ async def test_scheduler_retry_test(
     empty_name_retry_done = asyncio.Event()
     component_retry_done = asyncio.Event()
     multiple_name_done = asyncio.Event()
+    const_char_done = asyncio.Event()
+    static_char_done = asyncio.Event()
+    mixed_cancel_done = asyncio.Event()
     test_complete = asyncio.Event()
 
     # Track retry counts
@@ -33,16 +36,20 @@ async def test_scheduler_retry_test(
     empty_name_retry_count = 0
     component_retry_count = 0
     multiple_name_count = 0
+    const_char_retry_count = 0
+    static_char_retry_count = 0
 
     # Track specific test results
     cancel_result = None
     empty_cancel_result = None
+    mixed_cancel_result = None
     backoff_intervals = []
 
     def on_log_line(line: str) -> None:
         nonlocal simple_retry_count, backoff_retry_count, immediate_done_count
         nonlocal cancel_retry_count, empty_name_retry_count, component_retry_count
-        nonlocal multiple_name_count, cancel_result, empty_cancel_result
+        nonlocal multiple_name_count, const_char_retry_count, static_char_retry_count
+        nonlocal cancel_result, empty_cancel_result, mixed_cancel_result
 
         # Strip ANSI color codes
         clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line)
@@ -106,6 +113,27 @@ async def test_scheduler_retry_test(
                 if multiple_name_count >= 20:
                     multiple_name_done.set()
 
+        # Const char retry test
+        elif "Const char retry" in clean_line:
+            if match := re.search(r"Const char retry (\d+)", clean_line):
+                const_char_retry_count = int(match.group(1))
+                const_char_done.set()
+
+        # Static const char retry test
+        elif "Static const char retry" in clean_line:
+            if match := re.search(r"Static const char retry (\d+)", clean_line):
+                static_char_retry_count = int(match.group(1))
+                static_char_done.set()
+
+        elif "Static cancel result:" in clean_line:
+            # This is part of test 9, but we don't track it separately
+            pass
+
+        # Mixed cancel test
+        elif "Mixed cancel result:" in clean_line:
+            mixed_cancel_result = "true" in clean_line
+            mixed_cancel_done.set()
+
         # Test completion
         elif "All retry tests completed" in clean_line:
             test_complete.set()
@@ -227,6 +255,40 @@ async def test_scheduler_retry_test(
             f"Expected multiple name count >= 20 (second retry only), got {multiple_name_count}"
         )
 
+        # Wait for const char retry test
+        try:
+            await asyncio.wait_for(const_char_done.wait(), timeout=1.0)
+        except TimeoutError:
+            pytest.fail(
+                f"Const char retry test did not complete. Count: {const_char_retry_count}"
+            )
+
+        assert const_char_retry_count == 1, (
+            f"Expected 1 const char retry call, got {const_char_retry_count}"
+        )
+
+        # Wait for static char retry test
+        try:
+            await asyncio.wait_for(static_char_done.wait(), timeout=1.0)
+        except TimeoutError:
+            pytest.fail(
+                f"Static char retry test did not complete. Count: {static_char_retry_count}"
+            )
+
+        assert static_char_retry_count == 1, (
+            f"Expected 1 static char retry call, got {static_char_retry_count}"
+        )
+
+        # Wait for mixed cancel test
+        try:
+            await asyncio.wait_for(mixed_cancel_done.wait(), timeout=1.0)
+        except TimeoutError:
+            pytest.fail("Mixed cancel test did not complete")
+
+        assert mixed_cancel_result is True, (
+            "Mixed string/const char cancel should have succeeded"
+        )
+
         # Wait for test completion
         try:
             await asyncio.wait_for(test_complete.wait(), timeout=1.0)