From 452246e1c599f6a04b243cc415c313d2a8fe5065 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 22 Dec 2025 20:01:57 -1000 Subject: [PATCH] [core] Remove object_id RAM storage - no longer in hot path after #12627 --- esphome/core/entity_base.cpp | 74 ++++++------------ esphome/core/entity_base.h | 11 +-- esphome/core/entity_helpers.py | 35 +++------ esphome/core/helpers.h | 14 ++++ esphome/helpers.py | 28 +++++++ .../fixtures/fnv1_hash_object_id.yaml | 76 +++++++++++++++++++ tests/integration/test_fnv1_hash_object_id.py | 75 ++++++++++++++++++ tests/unit_tests/test_helpers.py | 71 +++++++++++++++++ 8 files changed, 302 insertions(+), 82 deletions(-) create mode 100644 tests/integration/fixtures/fnv1_hash_object_id.yaml create mode 100644 tests/integration/test_fnv1_hash_object_id.py diff --git a/esphome/core/entity_base.cpp b/esphome/core/entity_base.cpp index b7616a9ad3..f5d563dead 100644 --- a/esphome/core/entity_base.cpp +++ b/esphome/core/entity_base.cpp @@ -9,7 +9,8 @@ static const char *const TAG = "entity_base"; // Entity Name const StringRef &EntityBase::get_name() const { return this->name_; } -void EntityBase::set_name(const char *name) { +void EntityBase::set_name(const char *name) { this->set_name(name, 0); } +void EntityBase::set_name(const char *name, uint32_t object_id_hash) { this->name_ = StringRef(name); if (this->name_.empty()) { #ifdef USE_DEVICES @@ -21,8 +22,16 @@ void EntityBase::set_name(const char *name) { this->name_ = StringRef(App.get_friendly_name()); } this->flags_.has_own_name = false; + // Dynamic name - must calculate hash at runtime + this->calc_object_id_(); } else { this->flags_.has_own_name = true; + // Static name - use pre-computed hash if provided + if (object_id_hash != 0) { + this->object_id_hash_ = object_id_hash; + } else { + this->calc_object_id_(); + } } } @@ -45,69 +54,34 @@ void EntityBase::set_icon(const char *icon) { #endif } -// Check if the object_id is dynamic (changes with MAC suffix) -bool EntityBase::is_object_id_dynamic_() const { - return !this->flags_.has_own_name && App.is_name_add_mac_suffix_enabled(); -} - -// Entity Object ID +// Entity Object ID - computed on-demand from name std::string EntityBase::get_object_id() const { - // Check if `App.get_friendly_name()` is constant or dynamic. - if (this->is_object_id_dynamic_()) { - // `App.get_friendly_name()` is dynamic. - return str_sanitize(str_snake_case(App.get_friendly_name())); - } - // `App.get_friendly_name()` is constant. - return this->object_id_c_str_ == nullptr ? "" : this->object_id_c_str_; -} -void EntityBase::set_object_id(const char *object_id) { - this->object_id_c_str_ = object_id; - this->calc_object_id_(); -} - -void EntityBase::set_name_and_object_id(const char *name, const char *object_id) { - this->set_name(name); - this->object_id_c_str_ = object_id; - this->calc_object_id_(); -} - -// Calculate Object ID Hash from Entity Name -void EntityBase::calc_object_id_() { char buf[OBJECT_ID_MAX_LEN]; - StringRef object_id = this->get_object_id_to(buf); - this->object_id_hash_ = fnv1_hash(object_id.c_str()); + size_t len = this->write_object_id_to(buf, sizeof(buf)); + return std::string(buf, len); } -// Format dynamic object_id: sanitized snake_case of friendly_name -static size_t format_dynamic_object_id(char *buf, size_t buf_size) { - const std::string &name = App.get_friendly_name(); - size_t len = std::min(name.size(), buf_size - 1); - for (size_t i = 0; i < len; i++) { - buf[i] = to_sanitized_char(to_snake_case_char(name[i])); - } - buf[len] = '\0'; - return len; +// Calculate Object ID Hash directly from name using snake_case + sanitize +void EntityBase::calc_object_id_() { + this->object_id_hash_ = fnv1_hash_object_id(this->name_.c_str(), this->name_.size()); } size_t EntityBase::write_object_id_to(char *buf, size_t buf_size) const { - if (this->is_object_id_dynamic_()) { - return format_dynamic_object_id(buf, buf_size); + size_t len = std::min(this->name_.size(), buf_size - 1); + for (size_t i = 0; i < len; i++) { + buf[i] = to_sanitized_char(to_snake_case_char(this->name_[i])); } - const char *src = this->object_id_c_str_ == nullptr ? "" : this->object_id_c_str_; - size_t len = strlen(src); - if (len >= buf_size) - len = buf_size - 1; - memcpy(buf, src, len); buf[len] = '\0'; return len; } StringRef EntityBase::get_object_id_to(std::span buf) const { - if (this->is_object_id_dynamic_()) { - size_t len = format_dynamic_object_id(buf.data(), buf.size()); - return StringRef(buf.data(), len); + size_t len = std::min(this->name_.size(), buf.size() - 1); + for (size_t i = 0; i < len; i++) { + buf[i] = to_sanitized_char(to_snake_case_char(this->name_[i])); } - return this->object_id_c_str_ == nullptr ? StringRef() : StringRef(this->object_id_c_str_); + buf[len] = '\0'; + return StringRef(buf.data(), len); } uint32_t EntityBase::get_object_id_hash() { return this->object_id_hash_; } diff --git a/esphome/core/entity_base.h b/esphome/core/entity_base.h index 93f989934a..678040a04e 100644 --- a/esphome/core/entity_base.h +++ b/esphome/core/entity_base.h @@ -28,6 +28,9 @@ class EntityBase { // Get/set the name of this Entity const StringRef &get_name() const; void set_name(const char *name); + /// Set name with pre-computed object_id hash (avoids runtime hash calculation) + /// Use hash=0 for dynamic names that need runtime calculation + void set_name(const char *name, uint32_t object_id_hash); // Get whether this Entity has its own name or it should use the device friendly_name. bool has_own_name() const { return this->flags_.has_own_name; } @@ -43,10 +46,6 @@ class EntityBase { "which will remain available longer. get_object_id() will be removed in 2026.7.0", "2025.12.0") std::string get_object_id() const; - void set_object_id(const char *object_id); - - // Set both name and object_id in one call (reduces generated code size) - void set_name_and_object_id(const char *name, const char *object_id); // Get the unique Object ID of this Entity uint32_t get_object_id_hash(); @@ -140,11 +139,7 @@ class EntityBase { protected: void calc_object_id_(); - /// Check if the object_id is dynamic (changes with MAC suffix) - bool is_object_id_dynamic_() const; - StringRef name_; - const char *object_id_c_str_{nullptr}; #ifdef USE_ENTITY_ICON const char *icon_c_str_{nullptr}; #endif diff --git a/esphome/core/entity_helpers.py b/esphome/core/entity_helpers.py index f360b4d809..f5e57300c8 100644 --- a/esphome/core/entity_helpers.py +++ b/esphome/core/entity_helpers.py @@ -15,7 +15,7 @@ from esphome.const import ( from esphome.core import CORE, ID from esphome.cpp_generator import MockObj, add, get_variable import esphome.final_validate as fv -from esphome.helpers import sanitize, snake_case +from esphome.helpers import fnv1_hash_object_id, sanitize, snake_case from esphome.types import ConfigType, EntityMetadata _LOGGER = logging.getLogger(__name__) @@ -75,34 +75,21 @@ async def setup_entity(var: MockObj, config: ConfigType, platform: str) -> None: config: Configuration dictionary containing entity settings platform: The platform name (e.g., "sensor", "binary_sensor") """ - # Get device info - device_name: str | None = None + # Set device if configured device_id_obj: ID | None if device_id_obj := config.get(CONF_DEVICE_ID): device: MockObj = await get_variable(device_id_obj) add(var.set_device(device)) - # Get device name for object ID calculation - device_name = device_id_obj.id - # Calculate base object_id using the same logic as C++ - # This must match the C++ behavior in esphome/core/entity_base.cpp - base_object_id = get_base_entity_object_id( - config[CONF_NAME], CORE.friendly_name, device_name - ) - - if not config[CONF_NAME]: - _LOGGER.debug( - "Entity has empty name, using '%s' as object_id base", base_object_id - ) - - # Set both name and object_id in one call to reduce generated code size - add(var.set_name_and_object_id(config[CONF_NAME], base_object_id)) - _LOGGER.debug( - "Setting object_id '%s' for entity '%s' on platform '%s'", - base_object_id, - config[CONF_NAME], - platform, - ) + # Set the entity name with pre-computed object_id hash + # For entities with a name, we pre-compute the hash to avoid runtime calculation + # For empty names (use device friendly_name), pass 0 to compute at runtime + entity_name = config[CONF_NAME] + if entity_name: + object_id_hash = fnv1_hash_object_id(entity_name) + add(var.set_name(entity_name, object_id_hash)) + else: + add(var.set_name(entity_name, 0)) # Only set disabled_by_default if True (default is False) if config[CONF_DISABLED_BY_DEFAULT]: add(var.set_disabled_by_default(True)) diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 769041160c..3bbda5f8dd 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -529,6 +529,20 @@ constexpr char to_sanitized_char(char c) { /// Sanitizes the input string by removing all characters but alphanumerics, dashes and underscores. std::string str_sanitize(const std::string &str); +/// Calculate FNV-1 hash of a string while applying snake_case + sanitize transformations. +/// This computes object_id hashes directly from names without creating an intermediate buffer. +/// IMPORTANT: Must match Python fnv1_hash_object_id() in esphome/helpers.py. +/// If you modify this function, update the Python version and tests in both places. +inline uint32_t fnv1_hash_object_id(const char *str, size_t len) { + uint32_t hash = FNV1_OFFSET_BASIS; + for (size_t i = 0; i < len; i++) { + hash *= FNV1_PRIME; + // Apply snake_case (space->underscore, uppercase->lowercase) then sanitize + hash ^= static_cast(to_sanitized_char(to_snake_case_char(str[i]))); + } + return hash; +} + /// snprintf-like function returning std::string of maximum length \p len (excluding null terminator). std::string __attribute__((format(printf, 1, 3))) str_snprintf(const char *fmt, size_t len, ...); diff --git a/esphome/helpers.py b/esphome/helpers.py index d1623d1d3c..18f459d3ee 100644 --- a/esphome/helpers.py +++ b/esphome/helpers.py @@ -70,6 +70,34 @@ def fnv1a_32bit_hash(string: str) -> int: return hash_value +def fnv1_hash_object_id(name: str) -> int: + """Compute FNV-1 hash of name with snake_case + sanitize transformations. + + IMPORTANT: This must match the C++ fnv1_hash_object_id() in esphome/core/helpers.h. + If you modify this function, update the C++ version and tests in both places. + + Used for pre-computing entity object_id hashes at code generation time. + """ + hash_value = 2166136261 # FNV1_OFFSET_BASIS + for char in name: + # Apply snake_case: space -> underscore, uppercase -> lowercase + if char == " ": + c = "_" + elif "A" <= char <= "Z": + c = chr(ord(char) + 32) # lowercase + else: + c = char + # Apply sanitize: keep alphanumerics, dash, underscore; replace others with _ + if not ( + c in {"-", "_"} or "0" <= c <= "9" or "a" <= c <= "z" or "A" <= c <= "Z" + ): + c = "_" + # FNV-1: multiply then XOR + hash_value = (hash_value * 16777619) & 0xFFFFFFFF + hash_value ^= ord(c) + return hash_value + + def strip_accents(value: str) -> str: """Remove accents from a string.""" import unicodedata diff --git a/tests/integration/fixtures/fnv1_hash_object_id.yaml b/tests/integration/fixtures/fnv1_hash_object_id.yaml new file mode 100644 index 0000000000..2097b2fbf9 --- /dev/null +++ b/tests/integration/fixtures/fnv1_hash_object_id.yaml @@ -0,0 +1,76 @@ +esphome: + name: fnv1-hash-object-id-test + platformio_options: + build_flags: + - "-DDEBUG" + on_boot: + - lambda: |- + using esphome::fnv1_hash_object_id; + + // Test basic lowercase (hash matches Python fnv1_hash_object_id("foo")) + uint32_t hash_foo = fnv1_hash_object_id("foo", 3); + if (hash_foo == 0x408f5e13) { + ESP_LOGI("FNV1_OID", "foo PASSED"); + } else { + ESP_LOGE("FNV1_OID", "foo FAILED: 0x%08x != 0x408f5e13", hash_foo); + } + + // Test uppercase conversion (should match lowercase) + uint32_t hash_Foo = fnv1_hash_object_id("Foo", 3); + if (hash_Foo == 0x408f5e13) { + ESP_LOGI("FNV1_OID", "upper PASSED"); + } else { + ESP_LOGE("FNV1_OID", "upper FAILED: 0x%08x != 0x408f5e13", hash_Foo); + } + + // Test space to underscore conversion ("foo bar" -> "foo_bar") + uint32_t hash_space = fnv1_hash_object_id("foo bar", 7); + if (hash_space == 0x3ae35aa1) { + ESP_LOGI("FNV1_OID", "space PASSED"); + } else { + ESP_LOGE("FNV1_OID", "space FAILED: 0x%08x != 0x3ae35aa1", hash_space); + } + + // Test underscore preserved ("foo_bar") + uint32_t hash_underscore = fnv1_hash_object_id("foo_bar", 7); + if (hash_underscore == 0x3ae35aa1) { + ESP_LOGI("FNV1_OID", "underscore PASSED"); + } else { + ESP_LOGE("FNV1_OID", "underscore FAILED: 0x%08x != 0x3ae35aa1", hash_underscore); + } + + // Test hyphen preserved ("foo-bar") + uint32_t hash_hyphen = fnv1_hash_object_id("foo-bar", 7); + if (hash_hyphen == 0x438b12e3) { + ESP_LOGI("FNV1_OID", "hyphen PASSED"); + } else { + ESP_LOGE("FNV1_OID", "hyphen FAILED: 0x%08x != 0x438b12e3", hash_hyphen); + } + + // Test special chars become underscore ("foo!bar" -> "foo_bar") + uint32_t hash_special = fnv1_hash_object_id("foo!bar", 7); + if (hash_special == 0x3ae35aa1) { + ESP_LOGI("FNV1_OID", "special PASSED"); + } else { + ESP_LOGE("FNV1_OID", "special FAILED: 0x%08x != 0x3ae35aa1", hash_special); + } + + // Test complex name ("My Sensor Name" -> "my_sensor_name") + uint32_t hash_complex = fnv1_hash_object_id("My Sensor Name", 14); + if (hash_complex == 0x2760962a) { + ESP_LOGI("FNV1_OID", "complex PASSED"); + } else { + ESP_LOGE("FNV1_OID", "complex FAILED: 0x%08x != 0x2760962a", hash_complex); + } + + // Test empty string returns FNV1_OFFSET_BASIS + uint32_t hash_empty = fnv1_hash_object_id("", 0); + if (hash_empty == 0x811c9dc5) { + ESP_LOGI("FNV1_OID", "empty PASSED"); + } else { + ESP_LOGE("FNV1_OID", "empty FAILED: 0x%08x != 0x811c9dc5", hash_empty); + } + +host: +api: +logger: diff --git a/tests/integration/test_fnv1_hash_object_id.py b/tests/integration/test_fnv1_hash_object_id.py new file mode 100644 index 0000000000..23e8ca04c2 --- /dev/null +++ b/tests/integration/test_fnv1_hash_object_id.py @@ -0,0 +1,75 @@ +"""Integration test for fnv1_hash_object_id function. + +This test verifies that the C++ fnv1_hash_object_id() function in +esphome/core/helpers.h produces the same hash values as the Python +fnv1_hash_object_id() function in esphome/helpers.py. + +If this test fails, one of the implementations has diverged and needs +to be updated to match the other. +""" + +from __future__ import annotations + +import asyncio +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_fnv1_hash_object_id( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that C++ fnv1_hash_object_id matches Python implementation.""" + + test_results: dict[str, str] = {} + all_tests_complete = asyncio.Event() + expected_tests = { + "foo", + "upper", + "space", + "underscore", + "hyphen", + "special", + "complex", + "empty", + } + + def on_log_line(line: str) -> None: + """Capture log lines with test results.""" + # Strip ANSI escape codes + clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) + # Look for our test result messages + # Format: "[timestamp][level][FNV1_OID:line]: test_name PASSED" + match = re.search(r"\[FNV1_OID:\d+\]:\s+(\w+)\s+(PASSED|FAILED)", clean_line) + if match: + test_name = match.group(1) + result = match.group(2) + test_results[test_name] = result + if set(test_results.keys()) >= expected_tests: + 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 == "fnv1-hash-object-id-test" + + # Wait for all tests to complete or timeout + try: + await asyncio.wait_for(all_tests_complete.wait(), timeout=2.0) + except TimeoutError: + pytest.fail(f"Tests timed out. Got results for: {set(test_results.keys())}") + + # Verify all tests passed + for test_name in expected_tests: + assert test_name in test_results, f"{test_name} test not found" + assert test_results[test_name] == "PASSED", ( + f"{test_name} test failed - C++ and Python hash mismatch" + ) diff --git a/tests/unit_tests/test_helpers.py b/tests/unit_tests/test_helpers.py index 47b945e0eb..3b5ed8a424 100644 --- a/tests/unit_tests/test_helpers.py +++ b/tests/unit_tests/test_helpers.py @@ -279,6 +279,77 @@ def test_sanitize(text, expected): assert actual == expected +@pytest.mark.parametrize( + ("name", "expected_hash"), + ( + # Basic strings - hash of snake_case(sanitize(name)) + ("foo", 0x408F5E13), + ("Foo", 0x408F5E13), # Same as "foo" (lowercase) + ("FOO", 0x408F5E13), # Same as "foo" (lowercase) + # Spaces become underscores + ("foo bar", 0x3AE35AA1), # Transforms to "foo_bar" + ("Foo Bar", 0x3AE35AA1), # Same (lowercase + underscore) + # Already snake_case + ("foo_bar", 0x3AE35AA1), + # Special chars become underscores + ("foo!bar", 0x3AE35AA1), # Transforms to "foo_bar" + ("foo@bar", 0x3AE35AA1), # Transforms to "foo_bar" + # Hyphens are preserved + ("foo-bar", 0x438B12E3), + # Numbers are preserved + ("foo123", 0xF3B0067D), + # Empty string + ("", 0x811C9DC5), # FNV1_OFFSET_BASIS (no chars processed) + # Single char + ("a", 0x050C5D7E), + # Mixed case and spaces + ("My Sensor Name", 0x2760962A), # Transforms to "my_sensor_name" + ), +) +def test_fnv1_hash_object_id(name, expected_hash): + """Test fnv1_hash_object_id produces expected hashes. + + These expected values were computed to match the C++ implementation + in esphome/core/helpers.h. If this test fails after modifying either + implementation, ensure both Python and C++ versions stay in sync. + """ + actual = helpers.fnv1_hash_object_id(name) + + assert actual == expected_hash + + +def _fnv1_hash_py(s: str) -> int: + """Python implementation of FNV-1 hash for verification.""" + hash_val = 2166136261 # FNV1_OFFSET_BASIS + for c in s: + hash_val = (hash_val * 16777619) & 0xFFFFFFFF # FNV1_PRIME + hash_val ^= ord(c) + return hash_val + + +@pytest.mark.parametrize( + "name", + ( + "Simple", + "With Space", + "MixedCase", + "special!@#chars", + "already_snake_case", + "123numbers", + ), +) +def test_fnv1_hash_object_id_matches_manual_calculation(name): + """Verify fnv1_hash_object_id matches snake_case + sanitize + standard FNV-1.""" + # Manual calculation: snake_case -> sanitize -> fnv1_hash + transformed = helpers.sanitize(helpers.snake_case(name)) + expected = _fnv1_hash_py(transformed) + + # Direct calculation via fnv1_hash_object_id + actual = helpers.fnv1_hash_object_id(name) + + assert actual == expected + + @pytest.mark.parametrize( "text, expected", ((["127.0.0.1", "fe80::1", "2001::2"], ["2001::2", "127.0.0.1", "fe80::1"]),),