From ae5b211c8938f294bf1cbd0811b8c5eb0eab8a4e Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 29 Apr 2026 08:30:35 -0400 Subject: [PATCH 1/9] [api] Avoid JsonDocument copy-and-swap operator= in ActionResponse ctor (#16106) --- esphome/components/api/homeassistant_service.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/esphome/components/api/homeassistant_service.h b/esphome/components/api/homeassistant_service.h index 9d14061d07..aef046fbb0 100644 --- a/esphome/components/api/homeassistant_service.h +++ b/esphome/components/api/homeassistant_service.h @@ -78,7 +78,8 @@ class ActionResponse { : success_(success), error_message_(error_message) { if (data == nullptr || data_len == 0) return; - this->json_document_ = json::parse_json(data, data_len); + JsonDocument tmp = json::parse_json(data, data_len); + swap(this->json_document_, tmp); } #endif From 79da2b9704cb115624697bded6c0294f3b9db528 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 29 Apr 2026 08:30:46 -0400 Subject: [PATCH 2/9] [time] Fix bugprone-unchecked-optional-access in CronTrigger::check_time_ (#16107) --- esphome/components/time/automation.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/esphome/components/time/automation.cpp b/esphome/components/time/automation.cpp index 7eb99cfe74..3242669343 100644 --- a/esphome/components/time/automation.cpp +++ b/esphome/components/time/automation.cpp @@ -31,13 +31,14 @@ void CronTrigger::check_time_() { return; if (this->last_check_.has_value()) { - if (*this->last_check_ > time && this->last_check_->timestamp - time.timestamp > MAX_TIMESTAMP_DRIFT) { + auto &last_check = *this->last_check_; + if (last_check > time && last_check.timestamp - time.timestamp > MAX_TIMESTAMP_DRIFT) { // We went back in time (a lot), probably caused by time synchronization ESP_LOGW(TAG, "Time has jumped back!"); - } else if (*this->last_check_ >= time) { + } else if (last_check >= time) { // already handled this one return; - } else if (time > *this->last_check_ && time.timestamp - this->last_check_->timestamp > MAX_TIMESTAMP_DRIFT) { + } else if (time > last_check && time.timestamp - last_check.timestamp > MAX_TIMESTAMP_DRIFT) { // We went ahead in time (a lot), probably caused by time synchronization ESP_LOGW(TAG, "Time has jumped ahead!"); this->last_check_ = time; @@ -45,11 +46,11 @@ void CronTrigger::check_time_() { } while (true) { - this->last_check_->increment_second(); - if (*this->last_check_ >= time) + last_check.increment_second(); + if (last_check >= time) break; - if (this->matches(*this->last_check_)) + if (this->matches(last_check)) this->trigger(); } } From 0a497d3c22be8c4c83466178181f4c6b00179994 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2026 08:35:17 -0500 Subject: [PATCH 3/9] [light] Fold LightControlAction fields into a single stateless lambda (#16118) --- esphome/components/light/automation.h | 54 ++++----------------- esphome/components/light/automation.py | 67 ++++++++++++++------------ esphome/components/light/types.py | 1 + 3 files changed, 46 insertions(+), 76 deletions(-) diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index bc6fd84709..a5c73997b0 100644 --- a/esphome/components/light/automation.h +++ b/esphome/components/light/automation.h @@ -31,60 +31,26 @@ template class ToggleAction : public A transition_length_{}; }; -// Unique Empty per field so [[no_unique_address]] is guaranteed to coalesce. -namespace light_control_detail { -template struct Empty {}; -} // namespace light_control_detail - -// X-macro: (type, field_name, bit_index). Order and bit values must match -// the FIELDS table in automation.py. -#define LIGHT_CONTROL_FIELDS(X) \ - X(ColorMode, color_mode, 0) \ - X(bool, state, 1) \ - X(uint32_t, transition_length, 2) \ - X(uint32_t, flash_length, 3) \ - X(float, brightness, 4) \ - X(float, color_brightness, 5) \ - X(float, red, 6) \ - X(float, green, 7) \ - X(float, blue, 8) \ - X(float, white, 9) \ - X(float, color_temperature, 10) \ - X(float, cold_white, 11) \ - X(float, warm_white, 12) \ - X(uint32_t, effect, 13) - -template class LightControlAction : public Action { +// All configured fields are baked into a single stateless lambda whose +// constants live in flash. The action only stores one function pointer +// plus one parent pointer, regardless of how many fields the user set. +// Trigger args are forwarded to the apply function so user lambdas +// (e.g. `brightness: !lambda "return x;"`) keep working. +template class LightControlAction : public Action { public: - explicit LightControlAction(LightState *parent) : parent_(parent) {} - -#define LIGHT_FIELD_SETTER_(type, name, idx) \ - template void set_##name(V value) requires((Fields & (1 << (idx))) != 0) { this->name##_ = value; } -#define LIGHT_FIELD_APPLY_(type, name, idx) \ - if constexpr ((Fields & (1 << (idx))) != 0) \ - call.set_##name(this->name##_.value(x...)); -#define LIGHT_FIELD_DECL_(type, name, idx) \ - [[no_unique_address]] std::conditional_t<(Fields & (1 << (idx))) != 0, TemplatableFn, \ - light_control_detail::Empty<(idx)>> \ - name##_{}; - - LIGHT_CONTROL_FIELDS(LIGHT_FIELD_SETTER_) + using ApplyFn = void (*)(LightState *, LightCall &, const Ts &...); + LightControlAction(LightState *parent, ApplyFn apply) : parent_(parent), apply_(apply) {} void play(const Ts &...x) override { auto call = this->parent_->make_call(); - LIGHT_CONTROL_FIELDS(LIGHT_FIELD_APPLY_) + this->apply_(this->parent_, call, x...); call.perform(); } protected: LightState *parent_; - LIGHT_CONTROL_FIELDS(LIGHT_FIELD_DECL_) - -#undef LIGHT_FIELD_DECL_ -#undef LIGHT_FIELD_APPLY_ -#undef LIGHT_FIELD_SETTER_ + ApplyFn apply_; }; -#undef LIGHT_CONTROL_FIELDS template class DimRelativeAction : public Action { public: diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index c666c98e42..ca4018a975 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -37,6 +37,7 @@ from .types import ( AddressableSet, ColorMode, DimRelativeAction, + LightCall, LightControlAction, LightIsOffCondition, LightIsOnCondition, @@ -181,8 +182,8 @@ def _resolve_effect_index(config: ConfigType) -> int: async def light_control_to_code(config, action_id, template_arg, args): paren = await cg.get_variable(config[CONF_ID]) - # Order/bits must match LIGHT_CONTROL_FIELDS in automation.h. - # EFFECT has special handling below; setter=None skips the generic loop. + # All configured fields are folded into a single stateless lambda whose + # constants live in flash; the action stores only a function pointer. FIELDS = ( (CONF_COLOR_MODE, "set_color_mode", ColorMode), (CONF_STATE, "set_state", cg.bool_), @@ -197,49 +198,51 @@ async def light_control_to_code(config, action_id, template_arg, args): (CONF_COLOR_TEMPERATURE, "set_color_temperature", cg.float_), (CONF_COLD_WHITE, "set_cold_white", cg.float_), (CONF_WARM_WHITE, "set_warm_white", cg.float_), - (CONF_EFFECT, None, cg.uint32), ) - # Bitmask is passed as uint16_t in C++ — must stay within 16 bits. - assert len(FIELDS) <= 16, "LightControlAction Fields bitmask exceeds uint16_t" - field_mask = sum(1 << i for i, (k, _, _) in enumerate(FIELDS) if k in config) - control_template_arg = cg.TemplateArguments( - cg.RawExpression(f"static_cast({field_mask})"), *template_arg - ) - var = cg.new_Pvariable(action_id, control_template_arg, paren) + fwd_args = ", ".join(name for _, name in args) + body_lines: list[str] = [] for conf_key, setter, type_ in FIELDS: - if conf_key in config and setter is not None: - template_ = await cg.templatable(config[conf_key], args, type_) - cg.add(getattr(var, setter)(template_)) + if conf_key not in config: + continue + value = config[conf_key] + if isinstance(value, Lambda): + inner = await cg.process_lambda(value, args, return_type=type_) + body_lines.append(f"call.{setter}(({inner})({fwd_args}));") + else: + body_lines.append(f"call.{setter}({cg.safe_exp(value)});") if CONF_EFFECT in config: if isinstance(config[CONF_EFFECT], Lambda): - # Lambda returns a string — wrap in a C++ lambda that resolves - # the effect name to its uint32_t index at runtime inner_lambda = await cg.process_lambda( config[CONF_EFFECT], args, return_type=cg.std_string ) - fwd_args = ", ".join(n for _, n in args) - # capture="" is correct: paren is a global variable name - # string-interpolated into the body at codegen time, not a - # C++ runtime capture. - wrapper = LambdaExpression( - f"auto __effect_s = ({inner_lambda})({fwd_args});\n" - f"return {paren}->get_effect_index(" - f"__effect_s.c_str(), __effect_s.size());", - args, - capture="", - return_type=cg.uint32, + body_lines.append( + f"{{ auto __effect_s = ({inner_lambda})({fwd_args});\n" + f"call.set_effect(parent->get_effect_index(" + f"__effect_s.c_str(), __effect_s.size())); }}" ) - cg.add(var.set_effect(wrapper)) else: - # Static string — resolve effect name to index at codegen time - template_ = await cg.templatable( - _resolve_effect_index(config), args, cg.uint32 + # Cast disambiguates between set_effect(uint32_t) and + # set_effect(optional) when the literal is an int. + body_lines.append( + f"call.set_effect(static_cast({_resolve_effect_index(config)}));" ) - cg.add(var.set_effect(template_)) - return var + + # Match LightControlAction::ApplyFn signature: const Ts &... for trigger args. + apply_args = [ + (LightState.operator("ptr"), "parent"), + (LightCall.operator("ref"), "call"), + *((t.operator("const").operator("ref"), n) for t, n in args), + ] + apply_lambda = LambdaExpression( + ["\n".join(body_lines)], + apply_args, + capture="", + return_type=cg.void, + ) + return cg.new_Pvariable(action_id, template_arg, paren, apply_lambda) CONF_RELATIVE_BRIGHTNESS = "relative_brightness" diff --git a/esphome/components/light/types.py b/esphome/components/light/types.py index a586bcbd13..534dcd2194 100644 --- a/esphome/components/light/types.py +++ b/esphome/components/light/types.py @@ -13,6 +13,7 @@ Color = cg.esphome_ns.class_("Color") LightColorValues = light_ns.class_("LightColorValues") LightStateRTCState = light_ns.struct("LightStateRTCState") +LightCall = light_ns.class_("LightCall") # Color modes ColorMode = light_ns.enum("ColorMode", is_class=True) From 2bd28eee9d25b30eec64bd71c09c354df9630e65 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:51:31 -0400 Subject: [PATCH 4/9] [tormatic] Use .value() for checked optional access in read_gate_status_ (#16121) --- esphome/components/tormatic/tormatic_cover.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esphome/components/tormatic/tormatic_cover.cpp b/esphome/components/tormatic/tormatic_cover.cpp index a58228a219..cca7b2bba0 100644 --- a/esphome/components/tormatic/tormatic_cover.cpp +++ b/esphome/components/tormatic/tormatic_cover.cpp @@ -282,12 +282,13 @@ optional Tormatic::read_gate_status_() { } } + auto hdr = this->pending_hdr_.value(); + // Wait for all payload bytes to arrive before processing. - if (this->available() < this->pending_hdr_->payload_size()) { + if (this->available() < hdr.payload_size()) { return {}; } - auto hdr = *this->pending_hdr_; this->pending_hdr_.reset(); switch (hdr.type) { From 42b8597719f186f8bb5b2469260b1c62f67285a1 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:58:19 -0400 Subject: [PATCH 5/9] [api] Extend NOLINT to cover bugprone-random-generator-seed in MAC varint test (#16120) --- tests/components/api/test_proto_mac_varint.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/api/test_proto_mac_varint.cpp b/tests/components/api/test_proto_mac_varint.cpp index 317a6fb9d6..f2a63e96f6 100644 --- a/tests/components/api/test_proto_mac_varint.cpp +++ b/tests/components/api/test_proto_mac_varint.cpp @@ -112,7 +112,7 @@ TEST(ProtoMacVarint, AllOnes) { verify_mac(0xFFFFFFFFFFFFULL, 7); } // F // 100 deterministic-random 48-bit MACs to catch regressions across the space. TEST(ProtoMacVarint, RandomSample) { - // NOLINTNEXTLINE(cert-msc32-c,cert-msc51-cpp) -- intentional fixed seed for reproducibility. + // NOLINTNEXTLINE(cert-msc32-c,cert-msc51-cpp,bugprone-random-generator-seed) -- fixed seed for reproducibility std::mt19937_64 rng(0xC0FFEE); for (int i = 0; i < 100; i++) { uint64_t mac = rng() & 0xFFFFFFFFFFFFULL; From 241d7797e36a2ec2de37d7f7fe5ed2857a0d98aa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2026 09:18:09 -0500 Subject: [PATCH 6/9] [scheduler] Add self-keyed timer API for callers without a Component --- esphome/core/scheduler.cpp | 27 ++++- esphome/core/scheduler.h | 71 ++++++++--- .../fixtures/scheduler_self_keyed.yaml | 110 ++++++++++++++++++ .../integration/test_scheduler_self_keyed.py | 95 +++++++++++++++ 4 files changed, 283 insertions(+), 20 deletions(-) create mode 100644 tests/integration/fixtures/scheduler_self_keyed.yaml create mode 100644 tests/integration/test_scheduler_self_keyed.py diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 11884ce4ba..4feb0811c9 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -53,9 +53,13 @@ struct SchedulerNameLog { } else if (name_type == NameType::NUMERIC_ID) { ESPHOME_snprintf_P(buffer, sizeof(buffer), ESPHOME_PSTR("id:%" PRIu32), hash_or_id); return buffer; - } else { // NUMERIC_ID_INTERNAL + } else if (name_type == NameType::NUMERIC_ID_INTERNAL) { ESPHOME_snprintf_P(buffer, sizeof(buffer), ESPHOME_PSTR("iid:%" PRIu32), hash_or_id); return buffer; + } else { // SELF_POINTER + // static_name carries the void* key for SELF_POINTER (pointer-width union slot). + ESPHOME_snprintf_P(buffer, sizeof(buffer), ESPHOME_PSTR("self:%p"), static_name); + return buffer; } } }; @@ -293,6 +297,27 @@ bool HOT Scheduler::cancel_interval(Component *component, uint32_t id) { return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::INTERVAL); } +// Self-keyed scheduler API. The cancellation key is `self` (typically the caller's `this`), +// passed through the existing static_name pointer slot. Matching is by raw pointer equality +// (see matches_item_locked_'s SELF_POINTER branch). No Component pointer is stored, so +// is_failed() skip and component-based log attribution don't apply. +void HOT Scheduler::set_timeout(void *self, uint32_t timeout, std::function &&func) { + this->set_timer_common_(nullptr, SchedulerItem::TIMEOUT, NameType::SELF_POINTER, static_cast(self), 0, + timeout, std::move(func)); +} +void HOT Scheduler::set_interval(void *self, uint32_t interval, std::function &&func) { + this->set_timer_common_(nullptr, SchedulerItem::INTERVAL, NameType::SELF_POINTER, static_cast(self), 0, + interval, std::move(func)); +} +bool HOT Scheduler::cancel_timeout(void *self) { + return this->cancel_item_(nullptr, NameType::SELF_POINTER, static_cast(self), 0, + SchedulerItem::TIMEOUT); +} +bool HOT Scheduler::cancel_interval(void *self) { + return this->cancel_item_(nullptr, NameType::SELF_POINTER, static_cast(self), 0, + SchedulerItem::INTERVAL); +} + // Suppress deprecation warnings for RetryResult usage in the still-present (but deprecated) retry implementation. // Remove before 2026.8.0 along with all retry code. #pragma GCC diagnostic push diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 46b19855c3..cb75dce38a 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -148,12 +148,32 @@ class Scheduler { // Name storage type discriminator for SchedulerItem // Used to distinguish between static strings, hashed strings, numeric IDs, and internal numeric IDs enum class NameType : uint8_t { - STATIC_STRING = 0, // const char* pointer to static/flash storage - HASHED_STRING = 1, // uint32_t FNV-1a hash of a runtime string - NUMERIC_ID = 2, // uint32_t numeric identifier (component-level) - NUMERIC_ID_INTERNAL = 3 // uint32_t numeric identifier (core/internal, separate namespace) + STATIC_STRING = 0, // const char* pointer to static/flash storage + HASHED_STRING = 1, // uint32_t FNV-1a hash of a runtime string + NUMERIC_ID = 2, // uint32_t numeric identifier (component-level) + NUMERIC_ID_INTERNAL = 3, // uint32_t numeric identifier (core/internal, separate namespace) + SELF_POINTER = 4 // void* caller-supplied key (typically `this`); pointer equality }; + /** Self-keyed timeout. The cancellation key is `self` (typically the caller's `this`). + * + * Use this when the caller schedules at most one timer of a single purpose at a time and + * does not need a `Component` for `is_failed()` skip or log source attribution. Lets + * small classes drop `Component` inheritance entirely when their only Component dependency + * was the per-instance scheduler key. + * + * NOT applied for self-keyed items: + * - `is_failed()` skip — callbacks always fire (no Component to consult). + * - Log source attribution — logs use a generic "self:0x…" label. + * + * If you need either of those, use the existing `(Component *, id)` overloads. + */ + void set_timeout(void *self, uint32_t timeout, std::function &&func); + /// Self-keyed interval. See set_timeout(void *, ...) for semantics. + void set_interval(void *self, uint32_t interval, std::function &&func); + bool cancel_timeout(void *self); + bool cancel_interval(void *self); + protected: struct SchedulerItem { // Ordered by size to minimize padding @@ -182,19 +202,19 @@ class Scheduler { // std::atomic inlines correctly on all platforms. std::atomic remove{0}; - // Bit-packed fields (4 bits used, 4 bits padding in 1 byte) - enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; - NameType name_type_ : 2; // Discriminator for name_ union (0–3, see NameType enum) - bool is_retry : 1; // True if this is a retry timeout - // 4 bits padding -#else - // Single-threaded or multi-threaded without atomics: can pack all fields together // Bit-packed fields (5 bits used, 3 bits padding in 1 byte) enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; - bool remove : 1; - NameType name_type_ : 2; // Discriminator for name_ union (0–3, see NameType enum) + NameType name_type_ : 3; // Discriminator for name_ union (0–4, see NameType enum) bool is_retry : 1; // True if this is a retry timeout // 3 bits padding +#else + // Single-threaded or multi-threaded without atomics: can pack all fields together + // Bit-packed fields (6 bits used, 2 bits padding in 1 byte) + enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; + bool remove : 1; + NameType name_type_ : 3; // Discriminator for name_ union (0–4, see NameType enum) + bool is_retry : 1; // True if this is a retry timeout + // 2 bits padding #endif // Constructor @@ -231,16 +251,25 @@ class Scheduler { // Helper to get the static name (only valid for STATIC_STRING type) const char *get_name() const { return (name_type_ == NameType::STATIC_STRING) ? name_.static_name : nullptr; } - // Helper to get the hash or numeric ID (only valid for HASHED_STRING or NUMERIC_ID types) - uint32_t get_name_hash_or_id() const { return (name_type_ != NameType::STATIC_STRING) ? name_.hash_or_id : 0; } + // Helper to get the hash or numeric ID (only valid for HASHED_STRING / NUMERIC_ID / NUMERIC_ID_INTERNAL types) + uint32_t get_name_hash_or_id() const { + return (name_type_ != NameType::STATIC_STRING && name_type_ != NameType::SELF_POINTER) ? name_.hash_or_id : 0; + } + + // Helper to get the self pointer (only valid for SELF_POINTER type). + // The pointer is stored in the same union slot as `static_name` since both are pointer-width. + const void *get_self() const { + return (name_type_ == NameType::SELF_POINTER) ? static_cast(name_.static_name) : nullptr; + } // Helper to get the name type NameType get_name_type() const { return name_type_; } - // Set name storage: for STATIC_STRING stores the pointer, for all other types stores hash_or_id. - // Both union members occupy the same offset, so only one store is needed. + // Set name storage. STATIC_STRING/SELF_POINTER use the static_name pointer slot + // (both are pointer-width); other types use hash_or_id. Both union members occupy + // the same offset, so only one store is needed. void set_name(NameType type, const char *static_name, uint32_t hash_or_id) { - if (type == NameType::STATIC_STRING) { + if (type == NameType::STATIC_STRING || type == NameType::SELF_POINTER) { name_.static_name = static_name; } else { name_.hash_or_id = hash_or_id; @@ -367,10 +396,14 @@ class Scheduler { // Name type must match if (item->get_name_type() != name_type) return false; - // For static strings, compare the string content; for hash/ID, compare the value + // STATIC_STRING: compare string content. SELF_POINTER: raw pointer equality (no strcmp). + // Other types: compare hash/ID value. if (name_type == NameType::STATIC_STRING) { return this->names_match_static_(item->get_name(), static_name); } + if (name_type == NameType::SELF_POINTER) { + return item->name_.static_name == static_name; + } return item->get_name_hash_or_id() == hash_or_id; } diff --git a/tests/integration/fixtures/scheduler_self_keyed.yaml b/tests/integration/fixtures/scheduler_self_keyed.yaml new file mode 100644 index 0000000000..bee8a488b1 --- /dev/null +++ b/tests/integration/fixtures/scheduler_self_keyed.yaml @@ -0,0 +1,110 @@ +esphome: + debug_scheduler: true # Enable scheduler leak detection + name: scheduler-self-keyed-test + on_boot: + priority: -100 + then: + - logger.log: "Starting scheduler self-keyed tests" + +host: +api: +logger: + level: VERBOSE + +globals: + - id: tests_done + type: bool + initial_value: 'false' + +script: + - id: test_self_keyed + then: + - logger.log: "Testing self-keyed scheduler API" + - lambda: |- + // Two distinct heap-allocated keys - they must not collide + // even though both are self-keyed and share no Component pointer. + static int key_a_marker = 0; + static int key_b_marker = 0; + void *key_a = &key_a_marker; + void *key_b = &key_b_marker; + + // ---- Test 1: Self-keyed timeout fires ---- + App.scheduler.set_timeout(key_a, 50, []() { + ESP_LOGI("test", "Self timeout A fired"); + }); + + // ---- Test 2: Self-keyed cancel cancels only that key ---- + App.scheduler.set_timeout(key_b, 100, []() { + ESP_LOGE("test", "ERROR: Self timeout B should have been cancelled"); + }); + App.scheduler.cancel_timeout(key_b); + + // ---- Test 3: Two independent self keys don't collide ---- + // Using fresh static markers so neither matches key_a / key_b. + static int key_c_marker = 0; + static int key_d_marker = 0; + void *key_c = &key_c_marker; + void *key_d = &key_d_marker; + App.scheduler.set_timeout(key_c, 150, []() { + ESP_LOGI("test", "Self timeout C fired"); + }); + App.scheduler.set_timeout(key_d, 150, []() { + ESP_LOGI("test", "Self timeout D fired"); + }); + + // ---- Test 4: Self-keyed and component-keyed don't collide ---- + // Use a self pointer that happens to look like a Component-attached id. + // The scheduler must treat them as separate namespaces. + static int shared_marker = 0; + void *self_shared = &shared_marker; + App.scheduler.set_timeout(self_shared, 200, []() { + ESP_LOGI("test", "Self timeout shared fired"); + }); + App.scheduler.set_timeout(id(test_sensor), 7777U, 200, []() { + ESP_LOGI("test", "Component timeout 7777 fired"); + }); + + // ---- Test 5: Self-keyed interval fires multiple times then cancels ---- + static int interval_count = 0; + static int key_e_marker = 0; + void *key_e = &key_e_marker; + App.scheduler.set_interval(key_e, 80, [key_e]() { + interval_count++; + if (interval_count == 2) { + ESP_LOGI("test", "Self interval E fired twice"); + App.scheduler.cancel_interval(key_e); + } + }); + + // ---- Test 6: Re-registering same self-key replaces the timer ---- + // The old timer must NOT fire; only the new one does. + static int key_f_marker = 0; + void *key_f = &key_f_marker; + App.scheduler.set_timeout(key_f, 250, []() { + ESP_LOGE("test", "ERROR: Self timeout F first registration should have been replaced"); + }); + App.scheduler.set_timeout(key_f, 300, []() { + ESP_LOGI("test", "Self timeout F replacement fired"); + }); + + // Log completion after all timers should have fired + App.scheduler.set_timeout(id(test_sensor), 9999U, 1500, []() { + ESP_LOGI("test", "All self-keyed tests complete"); + }); + +sensor: + - platform: template + name: Test Sensor + id: test_sensor + lambda: return 1.0; + update_interval: never + +interval: + - interval: 0.1s + then: + - if: + condition: + lambda: 'return id(tests_done) == false;' + then: + - lambda: 'id(tests_done) = true;' + - script.execute: test_self_keyed diff --git a/tests/integration/test_scheduler_self_keyed.py b/tests/integration/test_scheduler_self_keyed.py new file mode 100644 index 0000000000..48c9d98ad6 --- /dev/null +++ b/tests/integration/test_scheduler_self_keyed.py @@ -0,0 +1,95 @@ +"""Test the self-keyed scheduler API. + +Verifies that `Scheduler::set_timeout(void *, ...)` / `set_interval(void *, ...)` and the +matching `cancel_*(void *)` overloads behave correctly: callbacks fire, distinct keys +don't collide, self-keyed and component-keyed namespaces are independent, and +re-registering the same key replaces the existing timer. +""" + +import asyncio +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_scheduler_self_keyed( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test self-keyed scheduler API.""" + self_a_fired = asyncio.Event() + self_b_error = asyncio.Event() + self_c_fired = asyncio.Event() + self_d_fired = asyncio.Event() + self_shared_fired = asyncio.Event() + component_7777_fired = asyncio.Event() + self_interval_done = asyncio.Event() + self_f_first_error = asyncio.Event() + self_f_replacement_fired = asyncio.Event() + all_tests_complete = asyncio.Event() + + def on_log_line(line: str) -> None: + clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) + + if "Self timeout A fired" in clean_line: + self_a_fired.set() + elif "ERROR: Self timeout B" in clean_line: + self_b_error.set() + elif "Self timeout C fired" in clean_line: + self_c_fired.set() + elif "Self timeout D fired" in clean_line: + self_d_fired.set() + elif "Self timeout shared fired" in clean_line: + self_shared_fired.set() + elif "Component timeout 7777 fired" in clean_line: + component_7777_fired.set() + elif "Self interval E fired twice" in clean_line: + self_interval_done.set() + elif "ERROR: Self timeout F first registration" in clean_line: + self_f_first_error.set() + elif "Self timeout F replacement fired" in clean_line: + self_f_replacement_fired.set() + elif "All self-keyed tests complete" in clean_line: + all_tests_complete.set() + + async with ( + run_compiled(yaml_config, line_callback=on_log_line), + api_client_connected() as client, + ): + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "scheduler-self-keyed-test" + + try: + await asyncio.wait_for(all_tests_complete.wait(), timeout=5.0) + except TimeoutError: + pytest.fail("Not all self-keyed tests completed within 5 seconds") + + # Test 1: self-keyed timeout fires + assert self_a_fired.is_set(), "Self timeout A should have fired" + + # Test 2: cancel_timeout(self) actually cancels + assert not self_b_error.is_set(), "Self timeout B should have been cancelled" + + # Test 3: distinct self keys don't collide + assert self_c_fired.is_set(), "Self timeout C should have fired" + assert self_d_fired.is_set(), "Self timeout D should have fired" + + # Test 4: self-keyed and component-keyed namespaces are independent + assert self_shared_fired.is_set(), "Self timeout shared should have fired" + assert component_7777_fired.is_set(), "Component timeout 7777 should have fired" + + # Test 5: self-keyed interval fires repeatedly and cancels cleanly + assert self_interval_done.is_set(), "Self interval E should have fired twice" + + # Test 6: re-registering same self-key replaces the previous timer + assert not self_f_first_error.is_set(), ( + "Self timeout F first registration should have been replaced" + ) + assert self_f_replacement_fired.is_set(), ( + "Self timeout F replacement should have fired" + ) From 9745a3299eeeffbdc9b966429c39c5b9282e3fa4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2026 09:34:45 -0500 Subject: [PATCH 7/9] [scheduler] Address review feedback (const void *, %p cast, buffer size) --- esphome/core/scheduler.cpp | 15 +++++++++------ esphome/core/scheduler.h | 10 +++++----- .../fixtures/scheduler_self_keyed.yaml | 6 ++++-- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 4feb0811c9..e8b8332902 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -35,7 +35,9 @@ static constexpr uint32_t MAX_INTERVAL_DELAY = 5000; // Uses a stack buffer to avoid heap allocation // Uses ESPHOME_snprintf_P/ESPHOME_PSTR for ESP8266 to keep format strings in flash struct SchedulerNameLog { - char buffer[20]; // Enough for "id:4294967295" or "hash:0xFFFFFFFF" or "(null)" + // Sized for the widest formatted output: "self:0x" + 16 hex digits (64-bit pointer) + nul. + // Also covers "id:4294967295", "hash:0xFFFFFFFF", "iid:4294967295", "(null)". + char buffer[28]; // Format a scheduler item name for logging // Returns pointer to formatted string (either static_name or internal buffer) @@ -58,7 +60,8 @@ struct SchedulerNameLog { return buffer; } else { // SELF_POINTER // static_name carries the void* key for SELF_POINTER (pointer-width union slot). - ESPHOME_snprintf_P(buffer, sizeof(buffer), ESPHOME_PSTR("self:%p"), static_name); + // Cast to const void* — %p requires a void* argument. + ESPHOME_snprintf_P(buffer, sizeof(buffer), ESPHOME_PSTR("self:%p"), static_cast(static_name)); return buffer; } } @@ -301,19 +304,19 @@ bool HOT Scheduler::cancel_interval(Component *component, uint32_t id) { // passed through the existing static_name pointer slot. Matching is by raw pointer equality // (see matches_item_locked_'s SELF_POINTER branch). No Component pointer is stored, so // is_failed() skip and component-based log attribution don't apply. -void HOT Scheduler::set_timeout(void *self, uint32_t timeout, std::function &&func) { +void HOT Scheduler::set_timeout(const void *self, uint32_t timeout, std::function &&func) { this->set_timer_common_(nullptr, SchedulerItem::TIMEOUT, NameType::SELF_POINTER, static_cast(self), 0, timeout, std::move(func)); } -void HOT Scheduler::set_interval(void *self, uint32_t interval, std::function &&func) { +void HOT Scheduler::set_interval(const void *self, uint32_t interval, std::function &&func) { this->set_timer_common_(nullptr, SchedulerItem::INTERVAL, NameType::SELF_POINTER, static_cast(self), 0, interval, std::move(func)); } -bool HOT Scheduler::cancel_timeout(void *self) { +bool HOT Scheduler::cancel_timeout(const void *self) { return this->cancel_item_(nullptr, NameType::SELF_POINTER, static_cast(self), 0, SchedulerItem::TIMEOUT); } -bool HOT Scheduler::cancel_interval(void *self) { +bool HOT Scheduler::cancel_interval(const void *self) { return this->cancel_item_(nullptr, NameType::SELF_POINTER, static_cast(self), 0, SchedulerItem::INTERVAL); } diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index cb75dce38a..671665f4c0 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -168,11 +168,11 @@ class Scheduler { * * If you need either of those, use the existing `(Component *, id)` overloads. */ - void set_timeout(void *self, uint32_t timeout, std::function &&func); - /// Self-keyed interval. See set_timeout(void *, ...) for semantics. - void set_interval(void *self, uint32_t interval, std::function &&func); - bool cancel_timeout(void *self); - bool cancel_interval(void *self); + void set_timeout(const void *self, uint32_t timeout, std::function &&func); + /// Self-keyed interval. See set_timeout(const void *, ...) for semantics. + void set_interval(const void *self, uint32_t interval, std::function &&func); + bool cancel_timeout(const void *self); + bool cancel_interval(const void *self); protected: struct SchedulerItem { diff --git a/tests/integration/fixtures/scheduler_self_keyed.yaml b/tests/integration/fixtures/scheduler_self_keyed.yaml index bee8a488b1..9a691136f3 100644 --- a/tests/integration/fixtures/scheduler_self_keyed.yaml +++ b/tests/integration/fixtures/scheduler_self_keyed.yaml @@ -21,8 +21,10 @@ script: then: - logger.log: "Testing self-keyed scheduler API" - lambda: |- - // Two distinct heap-allocated keys - they must not collide - // even though both are self-keyed and share no Component pointer. + // Two distinct keys backed by addresses of static markers — they + // must not collide even though both are self-keyed and share no + // Component pointer. Static storage gives them stable, unique + // addresses for the lifetime of the program. static int key_a_marker = 0; static int key_b_marker = 0; void *key_a = &key_a_marker; From 439d66f0f7bc98c07216b4a2d1de6b0312ff9a2f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2026 09:32:06 -0500 Subject: [PATCH 8/9] [sensor] Drop Component from filter classes, use self-keyed scheduler --- esphome/components/sensor/__init__.py | 19 ++++++------------- esphome/components/sensor/filter.cpp | 21 +++++++-------------- esphome/components/sensor/filter.h | 15 +++++---------- 3 files changed, 18 insertions(+), 37 deletions(-) diff --git a/esphome/components/sensor/__init__.py b/esphome/components/sensor/__init__.py index 48b7d25d4d..c18aa32f37 100644 --- a/esphome/components/sensor/__init__.py +++ b/esphome/components/sensor/__init__.py @@ -266,7 +266,7 @@ StreamingMovingAverageFilter = sensor_ns.class_("StreamingMovingAverageFilter", ExponentialMovingAverageFilter = sensor_ns.class_( "ExponentialMovingAverageFilter", Filter ) -ThrottleAverageFilter = sensor_ns.class_("ThrottleAverageFilter", Filter, cg.Component) +ThrottleAverageFilter = sensor_ns.class_("ThrottleAverageFilter", Filter) LambdaFilter = sensor_ns.class_("LambdaFilter", Filter) StatelessLambdaFilter = sensor_ns.class_("StatelessLambdaFilter", Filter) OffsetFilter = sensor_ns.class_("OffsetFilter", Filter) @@ -283,8 +283,8 @@ ThrottleWithPriorityNanFilter = sensor_ns.class_( TimeoutFilterBase = sensor_ns.class_("TimeoutFilterBase", Filter, cg.Component) TimeoutFilterLast = sensor_ns.class_("TimeoutFilterLast", TimeoutFilterBase) TimeoutFilterConfigured = sensor_ns.class_("TimeoutFilterConfigured", TimeoutFilterBase) -DebounceFilter = sensor_ns.class_("DebounceFilter", Filter, cg.Component) -HeartbeatFilter = sensor_ns.class_("HeartbeatFilter", Filter, cg.Component) +DebounceFilter = sensor_ns.class_("DebounceFilter", Filter) +HeartbeatFilter = sensor_ns.class_("HeartbeatFilter", Filter) DeltaFilter = sensor_ns.class_("DeltaFilter", Filter) OrFilter = sensor_ns.class_("OrFilter", Filter) CalibrateLinearFilter = sensor_ns.class_("CalibrateLinearFilter", Filter) @@ -567,9 +567,7 @@ async def exponential_moving_average_filter_to_code(config, filter_id): "throttle_average", ThrottleAverageFilter, cv.positive_time_period_milliseconds ) async def throttle_average_filter_to_code(config, filter_id): - var = cg.new_Pvariable(filter_id, config) - await cg.register_component(var, {}) - return var + return cg.new_Pvariable(filter_id, config) @FILTER_REGISTRY.register("lambda", LambdaFilter, cv.returning_lambda) @@ -698,13 +696,10 @@ HEARTBEAT_SCHEMA = cv.Schema( async def heartbeat_filter_to_code(config, filter_id): if isinstance(config, dict): var = cg.new_Pvariable(filter_id, config[CONF_PERIOD]) - await cg.register_component(var, {}) cg.add(var.set_optimistic(config[CONF_OPTIMISTIC])) return var - var = cg.new_Pvariable(filter_id, config) - await cg.register_component(var, {}) - return var + return cg.new_Pvariable(filter_id, config) TIMEOUT_SCHEMA = cv.maybe_simple_value( @@ -738,9 +733,7 @@ async def timeout_filter_to_code(config, filter_id): "debounce", DebounceFilter, cv.positive_time_period_milliseconds ) async def debounce_filter_to_code(config, filter_id): - var = cg.new_Pvariable(filter_id, config) - await cg.register_component(var, {}) - return var + return cg.new_Pvariable(filter_id, config) CONF_DATAPOINTS = "datapoints" diff --git a/esphome/components/sensor/filter.cpp b/esphome/components/sensor/filter.cpp index 4896757d3f..5f7f19769a 100644 --- a/esphome/components/sensor/filter.cpp +++ b/esphome/components/sensor/filter.cpp @@ -13,11 +13,6 @@ namespace esphome::sensor { static const char *const TAG = "sensor.filter"; -// Filter scheduler IDs. -// Each filter is its own Component instance, so the scheduler scopes -// IDs by component pointer — no risk of collisions between instances. -constexpr uint32_t FILTER_ID = 0; - // Filter void Filter::input(float value) { ESP_LOGVV(TAG, "Filter(%p)::input(%f)", this, value); @@ -185,8 +180,9 @@ optional ThrottleAverageFilter::new_value(float value) { } return {}; } -void ThrottleAverageFilter::setup() { - this->set_interval(FILTER_ID, this->time_period_, [this]() { +void ThrottleAverageFilter::initialize(Sensor *parent, Filter *next) { + Filter::initialize(parent, next); + App.scheduler.set_interval(this, this->time_period_, [this]() { ESP_LOGVV(TAG, "ThrottleAverageFilter(%p)::interval(sum=%f, n=%i)", this, this->sum_, this->n_); if (this->n_ == 0) { if (this->have_nan_) @@ -199,7 +195,6 @@ void ThrottleAverageFilter::setup() { this->have_nan_ = false; }); } -float ThrottleAverageFilter::get_setup_priority() const { return setup_priority::HARDWARE; } // LambdaFilter LambdaFilter::LambdaFilter(lambda_filter_t lambda_filter) : lambda_filter_(std::move(lambda_filter)) {} @@ -362,13 +357,12 @@ optional TimeoutFilterConfigured::new_value(float value) { // DebounceFilter optional DebounceFilter::new_value(float value) { - this->set_timeout(FILTER_ID, this->time_period_, [this, value]() { this->output(value); }); + App.scheduler.set_timeout(this, this->time_period_, [this, value]() { this->output(value); }); return {}; } DebounceFilter::DebounceFilter(uint32_t time_period) : time_period_(time_period) {} -float DebounceFilter::get_setup_priority() const { return setup_priority::HARDWARE; } // HeartbeatFilter HeartbeatFilter::HeartbeatFilter(uint32_t time_period) : time_period_(time_period), last_input_(NAN) {} @@ -384,8 +378,9 @@ optional HeartbeatFilter::new_value(float value) { return {}; } -void HeartbeatFilter::setup() { - this->set_interval(FILTER_ID, this->time_period_, [this]() { +void HeartbeatFilter::initialize(Sensor *parent, Filter *next) { + Filter::initialize(parent, next); + App.scheduler.set_interval(this, this->time_period_, [this]() { ESP_LOGVV(TAG, "HeartbeatFilter(%p)::interval(has_value=%s, last_input=%f)", this, YESNO(this->has_value_), this->last_input_); if (!this->has_value_) @@ -395,8 +390,6 @@ void HeartbeatFilter::setup() { }); } -float HeartbeatFilter::get_setup_priority() const { return setup_priority::HARDWARE; } - optional calibrate_linear_compute(const std::array *functions, size_t count, float value) { for (size_t i = 0; i < count; i++) { if (!std::isfinite(functions[i][2]) || value < functions[i][2]) diff --git a/esphome/components/sensor/filter.h b/esphome/components/sensor/filter.h index 917a1ce7d5..d61df11d9b 100644 --- a/esphome/components/sensor/filter.h +++ b/esphome/components/sensor/filter.h @@ -254,16 +254,14 @@ class ExponentialMovingAverageFilter : public Filter { * * It takes the average of all the values received in a period of time. */ -class ThrottleAverageFilter : public Filter, public Component { +class ThrottleAverageFilter : public Filter { public: explicit ThrottleAverageFilter(uint32_t time_period); - void setup() override; + void initialize(Sensor *parent, Filter *next) override; optional new_value(float value) override; - float get_setup_priority() const override; - protected: float sum_{0.0f}; unsigned int n_{0}; @@ -454,25 +452,22 @@ class TimeoutFilterConfigured : public TimeoutFilterBase { // Total: 8 (base) + 4 = 12 bytes + vtable ptr + Component overhead }; -class DebounceFilter : public Filter, public Component { +class DebounceFilter : public Filter { public: explicit DebounceFilter(uint32_t time_period); optional new_value(float value) override; - float get_setup_priority() const override; - protected: uint32_t time_period_; }; -class HeartbeatFilter : public Filter, public Component { +class HeartbeatFilter : public Filter { public: explicit HeartbeatFilter(uint32_t time_period); - void setup() override; + void initialize(Sensor *parent, Filter *next) override; optional new_value(float value) override; - float get_setup_priority() const override; void set_optimistic(bool optimistic) { this->optimistic_ = optimistic; } From 17173ba3a31fc2f3adb936c14206d53bad9b57da Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2026 10:15:30 -0500 Subject: [PATCH 9/9] [scheduler] Make get_name() return SELF_POINTER slot too So debug-only iteration logs in Scheduler::call() (the periodic items dump and the per-item Running line) print the actual self pointer instead of 'self:(nil)'. Both STATIC_STRING and SELF_POINTER use the same pointer union member, so a single accessor is the natural shape; drop the unused get_self() helper that was added for this and update the union member comments to mention SELF_POINTER and NUMERIC_ID_INTERNAL. --- esphome/core/scheduler.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 671665f4c0..a97f45b15f 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -180,8 +180,8 @@ class Scheduler { Component *component; // Optimized name storage using tagged union - zero heap allocation union { - const char *static_name; // For STATIC_STRING (string literals, no allocation) - uint32_t hash_or_id; // For HASHED_STRING or NUMERIC_ID + const char *static_name; // For STATIC_STRING (string literals) and SELF_POINTER (caller's `this`) + uint32_t hash_or_id; // For HASHED_STRING, NUMERIC_ID, and NUMERIC_ID_INTERNAL } name_; uint32_t interval; // Split time to handle millis() rollover. The scheduler combines the 32-bit millis() @@ -248,20 +248,18 @@ class Scheduler { SchedulerItem(SchedulerItem &&) = delete; SchedulerItem &operator=(SchedulerItem &&) = delete; - // Helper to get the static name (only valid for STATIC_STRING type) - const char *get_name() const { return (name_type_ == NameType::STATIC_STRING) ? name_.static_name : nullptr; } + // Helper to get the pointer-slot value (valid for STATIC_STRING and SELF_POINTER types). + // Both share the same union member, so callers (e.g. log formatters) can read either uniformly. + const char *get_name() const { + return (name_type_ == NameType::STATIC_STRING || name_type_ == NameType::SELF_POINTER) ? name_.static_name + : nullptr; + } // Helper to get the hash or numeric ID (only valid for HASHED_STRING / NUMERIC_ID / NUMERIC_ID_INTERNAL types) uint32_t get_name_hash_or_id() const { return (name_type_ != NameType::STATIC_STRING && name_type_ != NameType::SELF_POINTER) ? name_.hash_or_id : 0; } - // Helper to get the self pointer (only valid for SELF_POINTER type). - // The pointer is stored in the same union slot as `static_name` since both are pointer-width. - const void *get_self() const { - return (name_type_ == NameType::SELF_POINTER) ? static_cast(name_.static_name) : nullptr; - } - // Helper to get the name type NameType get_name_type() const { return name_type_; }