From 86a1b4cf694026ea77f2c37d406b55e84203b122 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 18 Jan 2026 19:51:11 -1000 Subject: [PATCH 1/3] [select][fan] Use StringRef for on_value/on_preset_set triggers to avoid heap allocation (#13324) --- esphome/codegen.py | 1 + esphome/components/fan/__init__.py | 4 +- esphome/components/fan/automation.h | 4 +- esphome/components/select/__init__.py | 4 +- esphome/components/select/automation.h | 4 +- esphome/core/string_ref.h | 64 ++++++++ esphome/cpp_types.py | 1 + .../fixtures/select_stringref_trigger.yaml | 85 +++++++++++ .../test_select_stringref_trigger.py | 143 ++++++++++++++++++ 9 files changed, 302 insertions(+), 8 deletions(-) create mode 100644 tests/integration/fixtures/select_stringref_trigger.yaml create mode 100644 tests/integration/test_select_stringref_trigger.py diff --git a/esphome/codegen.py b/esphome/codegen.py index 6d55c6023d..4a2a5975c6 100644 --- a/esphome/codegen.py +++ b/esphome/codegen.py @@ -69,6 +69,7 @@ from esphome.cpp_types import ( # noqa: F401 JsonObjectConst, Parented, PollingComponent, + StringRef, arduino_json_ns, bool_, const_char_ptr, diff --git a/esphome/components/fan/__init__.py b/esphome/components/fan/__init__.py index 35a351e8f1..6010aa8ed4 100644 --- a/esphome/components/fan/__init__.py +++ b/esphome/components/fan/__init__.py @@ -77,7 +77,7 @@ FanSpeedSetTrigger = fan_ns.class_( "FanSpeedSetTrigger", automation.Trigger.template(cg.int_) ) FanPresetSetTrigger = fan_ns.class_( - "FanPresetSetTrigger", automation.Trigger.template(cg.std_string) + "FanPresetSetTrigger", automation.Trigger.template(cg.StringRef) ) FanIsOnCondition = fan_ns.class_("FanIsOnCondition", automation.Condition.template()) @@ -287,7 +287,7 @@ async def setup_fan_core_(var, config): await automation.build_automation(trigger, [(cg.int_, "x")], conf) for conf in config.get(CONF_ON_PRESET_SET, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation(trigger, [(cg.std_string, "x")], conf) + await automation.build_automation(trigger, [(cg.StringRef, "x")], conf) async def register_fan(var, config): diff --git a/esphome/components/fan/automation.h b/esphome/components/fan/automation.h index 77abc2f13f..3c3b0ce519 100644 --- a/esphome/components/fan/automation.h +++ b/esphome/components/fan/automation.h @@ -208,7 +208,7 @@ class FanSpeedSetTrigger : public Trigger { int last_speed_; }; -class FanPresetSetTrigger : public Trigger { +class FanPresetSetTrigger : public Trigger { public: FanPresetSetTrigger(Fan *state) { state->add_on_state_callback([this, state]() { @@ -216,7 +216,7 @@ class FanPresetSetTrigger : public Trigger { auto should_trigger = preset_mode != this->last_preset_mode_; this->last_preset_mode_ = preset_mode; if (should_trigger) { - this->trigger(std::string(preset_mode)); + this->trigger(preset_mode); } }); this->last_preset_mode_ = state->get_preset_mode(); diff --git a/esphome/components/select/__init__.py b/esphome/components/select/__init__.py index c51131a292..84ad591ba1 100644 --- a/esphome/components/select/__init__.py +++ b/esphome/components/select/__init__.py @@ -33,7 +33,7 @@ SelectPtr = Select.operator("ptr") # Triggers SelectStateTrigger = select_ns.class_( "SelectStateTrigger", - automation.Trigger.template(cg.std_string, cg.size_t), + automation.Trigger.template(cg.StringRef, cg.size_t), ) # Actions @@ -100,7 +100,7 @@ async def setup_select_core_(var, config, *, options: list[str]): for conf in config.get(CONF_ON_VALUE, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) await automation.build_automation( - trigger, [(cg.std_string, "x"), (cg.size_t, "i")], conf + trigger, [(cg.StringRef, "x"), (cg.size_t, "i")], conf ) if (mqtt_id := config.get(CONF_MQTT_ID)) is not None: diff --git a/esphome/components/select/automation.h b/esphome/components/select/automation.h index 81e8a3561d..ffdabd5f7c 100644 --- a/esphome/components/select/automation.h +++ b/esphome/components/select/automation.h @@ -6,11 +6,11 @@ namespace esphome::select { -class SelectStateTrigger : public Trigger { +class SelectStateTrigger : public Trigger { public: explicit SelectStateTrigger(Select *parent) : parent_(parent) { parent->add_on_state_callback( - [this](size_t index) { this->trigger(std::string(this->parent_->option_at(index)), index); }); + [this](size_t index) { this->trigger(StringRef(this->parent_->option_at(index)), index); }); } protected: diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index 44ca79c81b..d502c4d27f 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -72,6 +72,7 @@ class StringRef { constexpr const char *c_str() const { return base_; } constexpr size_type size() const { return len_; } + constexpr size_type length() const { return len_; } constexpr bool empty() const { return len_ == 0; } constexpr const_reference operator[](size_type pos) const { return *(base_ + pos); } @@ -80,6 +81,32 @@ class StringRef { operator std::string() const { return str(); } + /// Find first occurrence of substring, returns std::string::npos if not found. + /// Note: Requires the underlying string to be null-terminated. + size_type find(const char *s, size_type pos = 0) const { + if (pos >= len_) + return std::string::npos; + const char *result = std::strstr(base_ + pos, s); + // Verify entire match is within bounds (strstr searches to null terminator) + if (result && result + std::strlen(s) <= base_ + len_) + return static_cast(result - base_); + return std::string::npos; + } + size_type find(char c, size_type pos = 0) const { + if (pos >= len_) + return std::string::npos; + const void *result = std::memchr(base_ + pos, static_cast(c), len_ - pos); + return result ? static_cast(static_cast(result) - base_) : std::string::npos; + } + + /// Return substring as std::string + std::string substr(size_type pos = 0, size_type count = std::string::npos) const { + if (pos >= len_) + return std::string(); + size_type actual_count = (count == std::string::npos || pos + count > len_) ? len_ - pos : count; + return std::string(base_ + pos, actual_count); + } + private: const char *base_; size_type len_; @@ -160,6 +187,43 @@ inline std::string operator+(const std::string &lhs, const StringRef &rhs) { str.append(rhs.c_str(), rhs.size()); return str; } +// String conversion functions for ADL compatibility (allows stoi(x) where x is StringRef) +// Must be in esphome namespace for ADL to find them. Uses strtol/strtod directly to avoid heap allocation. +namespace internal { +// NOLINTBEGIN(google-runtime-int) +template inline R parse_number(const StringRef &str, size_t *pos, F conv) { + char *end; + R result = conv(str.c_str(), &end); + // Set pos to 0 on conversion failure (when no characters consumed), otherwise index after number + if (pos) + *pos = (end == str.c_str()) ? 0 : static_cast(end - str.c_str()); + return result; +} +template inline R parse_number(const StringRef &str, size_t *pos, int base, F conv) { + char *end; + R result = conv(str.c_str(), &end, base); + // Set pos to 0 on conversion failure (when no characters consumed), otherwise index after number + if (pos) + *pos = (end == str.c_str()) ? 0 : static_cast(end - str.c_str()); + return result; +} +// NOLINTEND(google-runtime-int) +} // namespace internal +// NOLINTBEGIN(readability-identifier-naming,google-runtime-int) +inline int stoi(const StringRef &str, size_t *pos = nullptr, int base = 10) { + return static_cast(internal::parse_number(str, pos, base, std::strtol)); +} +inline long stol(const StringRef &str, size_t *pos = nullptr, int base = 10) { + return internal::parse_number(str, pos, base, std::strtol); +} +inline float stof(const StringRef &str, size_t *pos = nullptr) { + return internal::parse_number(str, pos, std::strtof); +} +inline double stod(const StringRef &str, size_t *pos = nullptr) { + return internal::parse_number(str, pos, std::strtod); +} +// NOLINTEND(readability-identifier-naming,google-runtime-int) + #ifdef USE_JSON // NOLINTNEXTLINE(readability-identifier-naming) inline void convertToJson(const StringRef &src, JsonVariant dst) { dst.set(src.c_str()); } diff --git a/esphome/cpp_types.py b/esphome/cpp_types.py index 0d1813f63b..7001c38857 100644 --- a/esphome/cpp_types.py +++ b/esphome/cpp_types.py @@ -44,3 +44,4 @@ gpio_Flags = gpio_ns.enum("Flags", is_class=True) EntityCategory = esphome_ns.enum("EntityCategory") Parented = esphome_ns.class_("Parented") ESPTime = esphome_ns.struct("ESPTime") +StringRef = esphome_ns.class_("StringRef") diff --git a/tests/integration/fixtures/select_stringref_trigger.yaml b/tests/integration/fixtures/select_stringref_trigger.yaml new file mode 100644 index 0000000000..bb1e1fd843 --- /dev/null +++ b/tests/integration/fixtures/select_stringref_trigger.yaml @@ -0,0 +1,85 @@ +esphome: + name: select-stringref-test + friendly_name: Select StringRef Test + +host: + +logger: + level: DEBUG + +api: + +select: + - platform: template + name: "Test Select" + id: test_select + optimistic: true + options: + - "Option A" + - "Option B" + - "Option C" + initial_option: "Option A" + on_value: + then: + # Test 1: Log the value directly (StringRef -> const char* via c_str()) + - logger.log: + format: "Select value: %s" + args: ['x.c_str()'] + # Test 2: String concatenation (StringRef + const char* -> std::string) + - lambda: |- + std::string with_suffix = x + " selected"; + ESP_LOGI("test", "Concatenated: %s", with_suffix.c_str()); + # Test 3: Comparison (StringRef == const char*) + - lambda: |- + if (x == "Option B") { + ESP_LOGI("test", "Option B was selected"); + } + # Test 4: Use index parameter (variable name is 'i') + - lambda: |- + ESP_LOGI("test", "Select index: %d", (int)i); + # Test 5: StringRef.length() method + - lambda: |- + ESP_LOGI("test", "Length: %d", (int)x.length()); + # Test 6: StringRef.find() method with substring + - lambda: |- + if (x.find("Option") != std::string::npos) { + ESP_LOGI("test", "Found 'Option' in value"); + } + # Test 7: StringRef.find() method with character + - lambda: |- + size_t space_pos = x.find(' '); + if (space_pos != std::string::npos) { + ESP_LOGI("test", "Space at position: %d", (int)space_pos); + } + # Test 8: StringRef.substr() method + - lambda: |- + std::string prefix = x.substr(0, 6); + ESP_LOGI("test", "Substr prefix: %s", prefix.c_str()); + + # Second select with numeric options to test ADL functions + - platform: template + name: "Baud Rate" + id: baud_select + optimistic: true + options: + - "9600" + - "115200" + initial_option: "9600" + on_value: + then: + # Test 9: stoi via ADL + - lambda: |- + int baud = stoi(x); + ESP_LOGI("test", "stoi result: %d", baud); + # Test 10: stol via ADL + - lambda: |- + long baud_long = stol(x); + ESP_LOGI("test", "stol result: %ld", baud_long); + # Test 11: stof via ADL + - lambda: |- + float baud_float = stof(x); + ESP_LOGI("test", "stof result: %.0f", baud_float); + # Test 12: stod via ADL + - lambda: |- + double baud_double = stod(x); + ESP_LOGI("test", "stod result: %.0f", baud_double); diff --git a/tests/integration/test_select_stringref_trigger.py b/tests/integration/test_select_stringref_trigger.py new file mode 100644 index 0000000000..7fc72a2290 --- /dev/null +++ b/tests/integration/test_select_stringref_trigger.py @@ -0,0 +1,143 @@ +"""Integration test for select on_value trigger with StringRef parameter.""" + +from __future__ import annotations + +import asyncio +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_select_stringref_trigger( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test select on_value trigger passes StringRef that works with string operations.""" + loop = asyncio.get_running_loop() + + # Track log messages to verify StringRef operations work + value_logged_future = loop.create_future() + concatenated_future = loop.create_future() + comparison_future = loop.create_future() + index_logged_future = loop.create_future() + length_future = loop.create_future() + find_substr_future = loop.create_future() + find_char_future = loop.create_future() + substr_future = loop.create_future() + # ADL functions + stoi_future = loop.create_future() + stol_future = loop.create_future() + stof_future = loop.create_future() + stod_future = loop.create_future() + + # Patterns to match in logs + value_pattern = re.compile(r"Select value: Option B") + concatenated_pattern = re.compile(r"Concatenated: Option B selected") + comparison_pattern = re.compile(r"Option B was selected") + index_pattern = re.compile(r"Select index: 1") + length_pattern = re.compile(r"Length: 8") # "Option B" is 8 chars + find_substr_pattern = re.compile(r"Found 'Option' in value") + find_char_pattern = re.compile(r"Space at position: 6") # space at index 6 + substr_pattern = re.compile(r"Substr prefix: Option") + # ADL function patterns (115200 from baud rate select) + stoi_pattern = re.compile(r"stoi result: 115200") + stol_pattern = re.compile(r"stol result: 115200") + stof_pattern = re.compile(r"stof result: 115200") + stod_pattern = re.compile(r"stod result: 115200") + + def check_output(line: str) -> None: + """Check log output for expected messages.""" + if not value_logged_future.done() and value_pattern.search(line): + value_logged_future.set_result(True) + if not concatenated_future.done() and concatenated_pattern.search(line): + concatenated_future.set_result(True) + if not comparison_future.done() and comparison_pattern.search(line): + comparison_future.set_result(True) + if not index_logged_future.done() and index_pattern.search(line): + index_logged_future.set_result(True) + if not length_future.done() and length_pattern.search(line): + length_future.set_result(True) + if not find_substr_future.done() and find_substr_pattern.search(line): + find_substr_future.set_result(True) + if not find_char_future.done() and find_char_pattern.search(line): + find_char_future.set_result(True) + if not substr_future.done() and substr_pattern.search(line): + substr_future.set_result(True) + # ADL functions + if not stoi_future.done() and stoi_pattern.search(line): + stoi_future.set_result(True) + if not stol_future.done() and stol_pattern.search(line): + stol_future.set_result(True) + if not stof_future.done() and stof_pattern.search(line): + stof_future.set_result(True) + if not stod_future.done() and stod_pattern.search(line): + stod_future.set_result(True) + + async with ( + run_compiled(yaml_config, line_callback=check_output), + api_client_connected() as client, + ): + # Verify device info + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "select-stringref-test" + + # List entities to find our select + entities, _ = await client.list_entities_services() + + select_entity = next( + (e for e in entities if hasattr(e, "options") and e.name == "Test Select"), + None, + ) + assert select_entity is not None, "Test Select entity not found" + + baud_entity = next( + (e for e in entities if hasattr(e, "options") and e.name == "Baud Rate"), + None, + ) + assert baud_entity is not None, "Baud Rate entity not found" + + # Change select to Option B - this should trigger on_value with StringRef + client.select_command(select_entity.key, "Option B") + # Change baud to 115200 - this tests ADL functions (stoi, stol, stof, stod) + client.select_command(baud_entity.key, "115200") + + # Wait for all log messages confirming StringRef operations work + try: + await asyncio.wait_for( + asyncio.gather( + value_logged_future, + concatenated_future, + comparison_future, + index_logged_future, + length_future, + find_substr_future, + find_char_future, + substr_future, + stoi_future, + stol_future, + stof_future, + stod_future, + ), + timeout=5.0, + ) + except TimeoutError: + results = { + "value_logged": value_logged_future.done(), + "concatenated": concatenated_future.done(), + "comparison": comparison_future.done(), + "index_logged": index_logged_future.done(), + "length": length_future.done(), + "find_substr": find_substr_future.done(), + "find_char": find_char_future.done(), + "substr": substr_future.done(), + "stoi": stoi_future.done(), + "stol": stol_future.done(), + "stof": stof_future.done(), + "stod": stod_future.done(), + } + pytest.fail(f"StringRef operations failed - received: {results}") From d83457bbe1619ac5e17feae627cd1db4787c4d90 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 18 Jan 2026 22:02:36 -1000 Subject: [PATCH 2/3] [mqtt] Reduce heap allocations in hot paths --- esphome/components/mqtt/mqtt_component.cpp | 92 ++++++++++++++-------- esphome/components/mqtt/mqtt_component.h | 62 ++++++++++++--- esphome/core/automation.h | 57 ++++++++++++-- 3 files changed, 156 insertions(+), 55 deletions(-) diff --git a/esphome/components/mqtt/mqtt_component.cpp b/esphome/components/mqtt/mqtt_component.cpp index 3b9290259b..51b6f29906 100644 --- a/esphome/components/mqtt/mqtt_component.cpp +++ b/esphome/components/mqtt/mqtt_component.cpp @@ -27,16 +27,10 @@ inline char *append_char(char *p, char c) { // Max lengths for stack-based topic building. // These limits are enforced at Python config validation time in mqtt/__init__.py // using cv.Length() validators for topic_prefix and discovery_prefix. -// MQTT_COMPONENT_TYPE_MAX_LEN and MQTT_SUFFIX_MAX_LEN are defined in mqtt_component.h. +// MQTT_COMPONENT_TYPE_MAX_LEN, MQTT_SUFFIX_MAX_LEN, and MQTT_DEFAULT_TOPIC_MAX_LEN are in mqtt_component.h. // ESPHOME_DEVICE_NAME_MAX_LEN and OBJECT_ID_MAX_LEN are defined in entity_base.h. // This ensures the stack buffers below are always large enough. -static constexpr size_t TOPIC_PREFIX_MAX_LEN = 64; // Validated in Python: cv.Length(max=64) static constexpr size_t DISCOVERY_PREFIX_MAX_LEN = 64; // Validated in Python: cv.Length(max=64) - -// Stack buffer sizes - safe because all inputs are length-validated at config time -// Format: prefix + "/" + type + "/" + object_id + "/" + suffix + null -static constexpr size_t DEFAULT_TOPIC_MAX_LEN = - TOPIC_PREFIX_MAX_LEN + 1 + MQTT_COMPONENT_TYPE_MAX_LEN + 1 + OBJECT_ID_MAX_LEN + 1 + MQTT_SUFFIX_MAX_LEN + 1; // Format: prefix + "/" + type + "/" + name + "/" + object_id + "/config" + null static constexpr size_t DISCOVERY_TOPIC_MAX_LEN = DISCOVERY_PREFIX_MAX_LEN + 1 + MQTT_COMPONENT_TYPE_MAX_LEN + 1 + ESPHOME_DEVICE_NAME_MAX_LEN + 1 + OBJECT_ID_MAX_LEN + 7 + 1; @@ -69,19 +63,18 @@ std::string MQTTComponent::get_discovery_topic_(const MQTTDiscoveryInfo &discove return std::string(buf, p - buf); } -std::string MQTTComponent::get_default_topic_for_(const std::string &suffix) const { +StringRef MQTTComponent::get_default_topic_for_to_(std::span buf, const char *suffix, + size_t suffix_len) const { const std::string &topic_prefix = global_mqtt_client->get_topic_prefix(); if (topic_prefix.empty()) { - // If the topic_prefix is null, the default topic should be null - return ""; + return StringRef(); // Empty topic_prefix means no default topic } const char *comp_type = this->component_type(); char object_id_buf[OBJECT_ID_MAX_LEN]; StringRef object_id = this->get_default_object_id_to_(object_id_buf); - char buf[DEFAULT_TOPIC_MAX_LEN]; - char *p = buf; + char *p = buf.data(); p = append_str(p, topic_prefix.data(), topic_prefix.size()); p = append_char(p, '/'); @@ -89,21 +82,44 @@ std::string MQTTComponent::get_default_topic_for_(const std::string &suffix) con p = append_char(p, '/'); p = append_str(p, object_id.c_str(), object_id.size()); p = append_char(p, '/'); - p = append_str(p, suffix.data(), suffix.size()); + p = append_str(p, suffix, suffix_len); + *p = '\0'; - return std::string(buf, p - buf); + return StringRef(buf.data(), p - buf.data()); +} + +std::string MQTTComponent::get_default_topic_for_(const std::string &suffix) const { + char buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; + StringRef ref = this->get_default_topic_for_to_(buf, suffix.data(), suffix.size()); + return std::string(ref.c_str(), ref.size()); +} + +StringRef MQTTComponent::get_state_topic_to_(std::span buf) const { + if (this->custom_state_topic_.has_value()) { + // Returns ref to existing data for static/value, uses buf only for lambda case + return this->custom_state_topic_.ref_or_copy_to(buf.data(), buf.size()); + } + return this->get_default_topic_for_to_(buf, "state", 5); +} + +StringRef MQTTComponent::get_command_topic_to_(std::span buf) const { + if (this->custom_command_topic_.has_value()) { + // Returns ref to existing data for static/value, uses buf only for lambda case + return this->custom_command_topic_.ref_or_copy_to(buf.data(), buf.size()); + } + return this->get_default_topic_for_to_(buf, "command", 7); } std::string MQTTComponent::get_state_topic_() const { - if (this->custom_state_topic_.has_value()) - return this->custom_state_topic_.value(); - return this->get_default_topic_for_("state"); + char buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; + StringRef ref = this->get_state_topic_to_(buf); + return std::string(ref.c_str(), ref.size()); } std::string MQTTComponent::get_command_topic_() const { - if (this->custom_command_topic_.has_value()) - return this->custom_command_topic_.value(); - return this->get_default_topic_for_("command"); + char buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; + StringRef ref = this->get_command_topic_to_(buf); + return std::string(ref.c_str(), ref.size()); } bool MQTTComponent::publish(const std::string &topic, const std::string &payload) { @@ -168,10 +184,14 @@ bool MQTTComponent::send_discovery_() { break; } - if (config.state_topic) - root[MQTT_STATE_TOPIC] = this->get_state_topic_(); - if (config.command_topic) - root[MQTT_COMMAND_TOPIC] = this->get_command_topic_(); + if (config.state_topic) { + char state_topic_buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; + root[MQTT_STATE_TOPIC] = this->get_state_topic_to_(state_topic_buf); + } + if (config.command_topic) { + char command_topic_buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; + root[MQTT_COMMAND_TOPIC] = this->get_command_topic_to_(command_topic_buf); + } if (this->command_retain_) root[MQTT_COMMAND_RETAIN] = true; @@ -288,7 +308,9 @@ void MQTTComponent::set_availability(std::string topic, std::string payload_avai } void MQTTComponent::disable_availability() { this->set_availability("", "", ""); } void MQTTComponent::call_setup() { - if (this->is_internal()) + // Cache is_internal result once during setup - topics don't change after this + this->is_internal_ = this->compute_is_internal_(); + if (this->is_internal_) return; this->setup(); @@ -344,26 +366,28 @@ StringRef MQTTComponent::get_default_object_id_to_(std::spanget_entity()->get_icon_ref(); } bool MQTTComponent::is_disabled_by_default_() const { return this->get_entity()->is_disabled_by_default(); } -bool MQTTComponent::is_internal() { +bool MQTTComponent::compute_is_internal_() { if (this->custom_state_topic_.has_value()) { - // If the custom state_topic is null, return true as it is internal and should not publish + // If the custom state_topic is empty, return true as it is internal and should not publish // else, return false, as it is explicitly set to a topic, so it is not internal and should publish - return this->get_state_topic_().empty(); + // Using is_empty() avoids heap allocation for non-lambda cases + return this->custom_state_topic_.is_empty(); } if (this->custom_command_topic_.has_value()) { - // If the custom command_topic is null, return true as it is internal and should not publish + // If the custom command_topic is empty, return true as it is internal and should not publish // else, return false, as it is explicitly set to a topic, so it is not internal and should publish - return this->get_command_topic_().empty(); + // Using is_empty() avoids heap allocation for non-lambda cases + return this->custom_command_topic_.is_empty(); } - // No custom topics have been set - if (this->get_default_topic_for_("").empty()) { - // If the default topic prefix is null, then the component, by default, is internal and should not publish + // No custom topics have been set - check topic_prefix directly to avoid allocation + if (global_mqtt_client->get_topic_prefix().empty()) { + // If the default topic prefix is empty, then the component, by default, is internal and should not publish return true; } - // Use ESPHome's component internal state if topic_prefix is not null with no custom state_topic or command_topic + // Use ESPHome's component internal state if topic_prefix is not empty with no custom state_topic or command_topic return this->get_entity()->is_internal(); } diff --git a/esphome/components/mqtt/mqtt_component.h b/esphome/components/mqtt/mqtt_component.h index 676e3ad35d..2855a3e767 100644 --- a/esphome/components/mqtt/mqtt_component.h +++ b/esphome/components/mqtt/mqtt_component.h @@ -20,16 +20,26 @@ struct SendDiscoveryConfig { bool command_topic{true}; ///< If the command topic should be included. Default to true. }; -// Max lengths for stack-based topic building (must match mqtt_component.cpp) +// Max lengths for stack-based topic building. +// These limits are enforced at Python config validation time in mqtt/__init__.py +// using cv.Length() validators for topic_prefix and discovery_prefix. +// This ensures the stack buffers are always large enough. static constexpr size_t MQTT_COMPONENT_TYPE_MAX_LEN = 20; static constexpr size_t MQTT_SUFFIX_MAX_LEN = 32; +static constexpr size_t MQTT_TOPIC_PREFIX_MAX_LEN = 64; // Validated in Python: cv.Length(max=64) +// Stack buffer size - safe because all inputs are length-validated at config time +// Format: prefix + "/" + type + "/" + object_id + "/" + suffix + null +static constexpr size_t MQTT_DEFAULT_TOPIC_MAX_LEN = + MQTT_TOPIC_PREFIX_MAX_LEN + 1 + MQTT_COMPONENT_TYPE_MAX_LEN + 1 + OBJECT_ID_MAX_LEN + 1 + MQTT_SUFFIX_MAX_LEN + 1; #define LOG_MQTT_COMPONENT(state_topic, command_topic) \ if (state_topic) { \ - ESP_LOGCONFIG(TAG, " State Topic: '%s'", this->get_state_topic_().c_str()); \ + char __mqtt_topic_buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; \ + ESP_LOGCONFIG(TAG, " State Topic: '%s'", this->get_state_topic_to_(__mqtt_topic_buf).c_str()); \ } \ if (command_topic) { \ - ESP_LOGCONFIG(TAG, " Command Topic: '%s'", this->get_command_topic_().c_str()); \ + char __mqtt_topic_buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; \ + ESP_LOGCONFIG(TAG, " Command Topic: '%s'", this->get_command_topic_to_(__mqtt_topic_buf).c_str()); \ } // Macro to define component_type() with compile-time length verification @@ -90,7 +100,8 @@ class MQTTComponent : public Component { virtual bool send_initial_state() = 0; - virtual bool is_internal(); + /// Returns cached is_internal result (computed once during setup). + bool is_internal() const { return this->is_internal_; } /// Set QOS for state messages. void set_qos(uint8_t qos); @@ -178,7 +189,16 @@ class MQTTComponent : public Component { /// Helper method to get the discovery topic for this component. std::string get_discovery_topic_(const MQTTDiscoveryInfo &discovery_info) const; - /** Get this components state/command/... topic. + /** Get this components state/command/... topic into a buffer. + * + * @param buf The buffer to write to (must be at least DEFAULT_TOPIC_MAX_LEN). + * @param suffix The suffix/key such as "state" or "command". + * @return StringRef pointing to the buffer with the topic. + */ + StringRef get_default_topic_for_to_(std::span buf, const char *suffix, + size_t suffix_len) const; + + /** Get this components state/command/... topic (allocates std::string). * * @param suffix The suffix/key such as "state" or "command". * @return The full topic. @@ -199,10 +219,20 @@ class MQTTComponent : public Component { /// Get whether the underlying Entity is disabled by default bool is_disabled_by_default_() const; - /// Get the MQTT topic that new states will be shared to. + /// Get the MQTT state topic into a buffer (no heap allocation for non-lambda custom topics). + /// @param buf Buffer of exactly MQTT_DEFAULT_TOPIC_MAX_LEN bytes. + /// @return StringRef pointing to the topic in the buffer. + StringRef get_state_topic_to_(std::span buf) const; + + /// Get the MQTT command topic into a buffer (no heap allocation for non-lambda custom topics). + /// @param buf Buffer of exactly MQTT_DEFAULT_TOPIC_MAX_LEN bytes. + /// @return StringRef pointing to the topic in the buffer. + StringRef get_command_topic_to_(std::span buf) const; + + /// Get the MQTT topic that new states will be shared to (allocates std::string). std::string get_state_topic_() const; - /// Get the MQTT topic for listening to commands. + /// Get the MQTT topic for listening to commands (allocates std::string). std::string get_command_topic_() const; bool is_connected_() const; @@ -220,12 +250,18 @@ class MQTTComponent : public Component { std::unique_ptr availability_; - bool command_retain_{false}; - bool retain_{true}; - uint8_t qos_{0}; - uint8_t subscribe_qos_{0}; - bool discovery_enabled_{true}; - bool resend_state_{false}; + // Packed bitfields - QoS values are 0-2, bools are flags + uint8_t qos_ : 2 {0}; + uint8_t subscribe_qos_ : 2 {0}; + bool command_retain_ : 1 {false}; + bool retain_ : 1 {true}; + bool discovery_enabled_ : 1 {true}; + bool resend_state_ : 1 {false}; + bool is_internal_ : 1 {false}; ///< Cached result of compute_is_internal_(), set during setup + + /// Compute is_internal status based on topics and entity state. + /// Called once during setup to cache the result. + bool compute_is_internal_(); }; } // namespace esphome::mqtt diff --git a/esphome/core/automation.h b/esphome/core/automation.h index eac469d0fc..31a2fc06f4 100644 --- a/esphome/core/automation.h +++ b/esphome/core/automation.h @@ -4,6 +4,7 @@ #include "esphome/core/defines.h" #include "esphome/core/helpers.h" #include "esphome/core/preferences.h" +#include "esphome/core/string_ref.h" #include #include #include @@ -190,15 +191,55 @@ template class TemplatableValue { /// Get the static string pointer (only valid if is_static_string() returns true) const char *get_static_string() const { return this->static_str_; } - protected: - enum : uint8_t { - NONE, - VALUE, - LAMBDA, - STATELESS_LAMBDA, - STATIC_STRING, // For const char* when T is std::string - avoids heap allocation - } type_; + /// Check if the string value is empty without allocating (for std::string specialization). + /// For NONE, returns true. For STATIC_STRING/VALUE, checks without allocation. + /// For LAMBDA/STATELESS_LAMBDA, must call value() which may allocate. + bool is_empty() const requires std::same_as { + switch (this->type_) { + case NONE: + return true; + case STATIC_STRING: + return this->static_str_ == nullptr || this->static_str_[0] == '\0'; + case VALUE: + return this->value_->empty(); + default: // LAMBDA/STATELESS_LAMBDA - must call value() + return this->value().empty(); + } + } + /// Get a StringRef to the string value without heap allocation when possible. + /// For STATIC_STRING/VALUE, returns reference to existing data (no allocation). + /// For LAMBDA/STATELESS_LAMBDA, calls value(), copies to provided buffer, returns ref to buffer. + /// @param lambda_buf Buffer used only for lambda case (must remain valid while StringRef is used). + /// @param lambda_buf_size Size of the buffer. + /// @return StringRef pointing to the string data. + StringRef ref_or_copy_to(char *lambda_buf, size_t lambda_buf_size) const requires std::same_as { + switch (this->type_) { + case NONE: + return StringRef(); + case STATIC_STRING: + if (this->static_str_ == nullptr) + return StringRef(); + return StringRef(this->static_str_, strlen(this->static_str_)); + case VALUE: + return StringRef(this->value_->data(), this->value_->size()); + default: { // LAMBDA/STATELESS_LAMBDA - must call value() and copy + std::string result = this->value(); + size_t copy_len = std::min(result.size(), lambda_buf_size - 1); + memcpy(lambda_buf, result.data(), copy_len); + lambda_buf[copy_len] = '\0'; + return StringRef(lambda_buf, copy_len); + } + } + } + + protected : enum : uint8_t { + NONE, + VALUE, + LAMBDA, + STATELESS_LAMBDA, + STATIC_STRING, // For const char* when T is std::string - avoids heap allocation + } type_; // For std::string, use heap pointer to minimize union size (4 bytes vs 12+). // For other types, store value inline as before. using ValueStorage = std::conditional_t; From 0117519c81a64f555327b3e6e17f7237d222352f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 18 Jan 2026 22:10:48 -1000 Subject: [PATCH 3/3] [mqtt] Reduce heap allocations in hot paths --- esphome/components/mqtt/mqtt_component.cpp | 9 +++++++++ esphome/components/mqtt/mqtt_component.h | 15 ++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/esphome/components/mqtt/mqtt_component.cpp b/esphome/components/mqtt/mqtt_component.cpp index 51b6f29906..defedfe0d1 100644 --- a/esphome/components/mqtt/mqtt_component.cpp +++ b/esphome/components/mqtt/mqtt_component.cpp @@ -35,6 +35,15 @@ static constexpr size_t DISCOVERY_PREFIX_MAX_LEN = 64; // Validated in Python: static constexpr size_t DISCOVERY_TOPIC_MAX_LEN = DISCOVERY_PREFIX_MAX_LEN + 1 + MQTT_COMPONENT_TYPE_MAX_LEN + 1 + ESPHOME_DEVICE_NAME_MAX_LEN + 1 + OBJECT_ID_MAX_LEN + 7 + 1; +// Function implementation of LOG_MQTT_COMPONENT macro to reduce code size +void log_mqtt_component(const char *tag, MQTTComponent *obj, bool state_topic, bool command_topic) { + char buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; + if (state_topic) + ESP_LOGCONFIG(tag, " State Topic: '%s'", obj->get_state_topic_to_(buf).c_str()); + if (command_topic) + ESP_LOGCONFIG(tag, " Command Topic: '%s'", obj->get_command_topic_to_(buf).c_str()); +} + void MQTTComponent::set_qos(uint8_t qos) { this->qos_ = qos; } void MQTTComponent::set_subscribe_qos(uint8_t qos) { this->subscribe_qos_ = qos; } diff --git a/esphome/components/mqtt/mqtt_component.h b/esphome/components/mqtt/mqtt_component.h index 2855a3e767..97234eedfc 100644 --- a/esphome/components/mqtt/mqtt_component.h +++ b/esphome/components/mqtt/mqtt_component.h @@ -32,15 +32,10 @@ static constexpr size_t MQTT_TOPIC_PREFIX_MAX_LEN = 64; // Validated in Python: static constexpr size_t MQTT_DEFAULT_TOPIC_MAX_LEN = MQTT_TOPIC_PREFIX_MAX_LEN + 1 + MQTT_COMPONENT_TYPE_MAX_LEN + 1 + OBJECT_ID_MAX_LEN + 1 + MQTT_SUFFIX_MAX_LEN + 1; -#define LOG_MQTT_COMPONENT(state_topic, command_topic) \ - if (state_topic) { \ - char __mqtt_topic_buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; \ - ESP_LOGCONFIG(TAG, " State Topic: '%s'", this->get_state_topic_to_(__mqtt_topic_buf).c_str()); \ - } \ - if (command_topic) { \ - char __mqtt_topic_buf[MQTT_DEFAULT_TOPIC_MAX_LEN]; \ - ESP_LOGCONFIG(TAG, " Command Topic: '%s'", this->get_command_topic_to_(__mqtt_topic_buf).c_str()); \ - } +class MQTTComponent; // Forward declaration +void log_mqtt_component(const char *tag, MQTTComponent *obj, bool state_topic, bool command_topic); + +#define LOG_MQTT_COMPONENT(state_topic, command_topic) log_mqtt_component(TAG, this, state_topic, command_topic) // Macro to define component_type() with compile-time length verification // Usage: MQTT_COMPONENT_TYPE(MQTTSensorComponent, "sensor") @@ -84,6 +79,8 @@ static constexpr size_t MQTT_DEFAULT_TOPIC_MAX_LEN = * a clean separation. */ class MQTTComponent : public Component { + friend void log_mqtt_component(const char *tag, MQTTComponent *obj, bool state_topic, bool command_topic); + public: /// Constructs a MQTTComponent. explicit MQTTComponent();