diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 3dc2a06c85..cb57db9ce8 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -440,7 +440,7 @@ void APIConnection::on_disconnect_response() { uint16_t APIConnection::fill_and_encode_entity_state(EntityBase *entity, StateResponseProtoMessage &msg, CalculateSizeFn size_fn, MessageEncodeFn encode_fn, APIConnection *conn, uint32_t remaining_size) { - msg.key = entity->get_object_id_hash(); + msg.key = entity->get_entity_key(); #ifdef USE_DEVICES msg.device_id = entity->get_device_id(); #endif @@ -451,7 +451,7 @@ uint16_t APIConnection::fill_and_encode_entity_info(EntityBase *entity, InfoResp CalculateSizeFn size_fn, MessageEncodeFn encode_fn, APIConnection *conn, uint32_t remaining_size) { // Set common fields that are shared by all entity types - msg.key = entity->get_object_id_hash(); + msg.key = entity->get_entity_key(); if (entity->has_own_name()) { msg.name = entity->get_name(); @@ -1141,7 +1141,7 @@ void APIConnection::try_send_camera_image_() { bool done = this->image_reader_->available() == to_send; CameraImageResponse msg; - msg.key = camera::Camera::instance()->get_object_id_hash(); + msg.key = camera::Camera::instance()->get_entity_key(); msg.set_data(this->image_reader_->peek_data_buffer(), to_send); msg.done = done; #ifdef USE_DEVICES diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index 6491b9a5e6..40a03c97b8 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -2193,6 +2193,8 @@ async def to_code(config): cg.set_cpp_standard("gnu++20") cg.add_build_flag("-DUSE_ESP32") cg.add_define("USE_NATIVE_64BIT_TIME") + # NVS finds stored preferences by key, so preference key migration is possible + cg.add_define("USE_PREFERENCE_KEY_LOOKUP") cg.add_build_flag("-Wl,-z,noexecstack") # Deferred so KEY_COMPONENTS is fully populated -- see the coroutine. CORE.add_job(_finalize_arduino_aware_flags) diff --git a/esphome/components/esp32/preferences.cpp b/esphome/components/esp32/preferences.cpp index dc2b40455c..f3d5844cd7 100644 --- a/esphome/components/esp32/preferences.cpp +++ b/esphome/components/esp32/preferences.cpp @@ -176,12 +176,21 @@ ESPPreferenceObject ESP32Preferences::make_preference(size_t length, uint32_t ty } s_open_err = ESP_OK; } - auto *pref = new ESP32PreferenceBackend(); // NOLINT(cppcoreguidelines-owning-memory) - pref->nvs_handle = this->nvs_handle; - pref->key = type; - pref->in_flash = true; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + return ESPPreferenceObject(new ESP32PreferenceBackend(this->make_backend_(type))); +} - return ESPPreferenceObject(pref); +ESP32PreferenceBackend ESP32Preferences::make_backend_(uint32_t type) const { + // in_flash keeps its default of true, selecting the NVS path + ESP32PreferenceBackend backend; + backend.nvs_handle = this->nvs_handle; + backend.key = type; + return backend; +} + +bool ESP32Preferences::load_from_key(uint32_t type, uint8_t *data, size_t len) { + ESP32PreferenceBackend backend = this->make_backend_(type); + return backend.load(data, len); } #ifdef USE_ESP32_RTC_PREFERENCES_STORAGE diff --git a/esphome/components/esp32/preferences.h b/esphome/components/esp32/preferences.h index 864d22312b..9125843958 100644 --- a/esphome/components/esp32/preferences.h +++ b/esphome/components/esp32/preferences.h @@ -23,12 +23,15 @@ class ESP32Preferences final : public PreferencesMixin { ESPPreferenceObject make_preference(size_t length, uint32_t type, bool in_flash); // Two-argument form defaults to NVS (flash) storage, preserving historic ESP32 behavior. ESPPreferenceObject make_preference(size_t length, uint32_t type); + /// One-shot read of a stored preference by key, without allocating a backend + bool load_from_key(uint32_t type, uint8_t *data, size_t len); bool sync(); bool reset(); uint32_t nvs_handle; protected: + ESP32PreferenceBackend make_backend_(uint32_t type) const; bool is_changed_(uint32_t nvs_handle, const NVSData &to_save, const char *key_str); #ifdef USE_ESP32_RTC_PREFERENCES_STORAGE diff --git a/esphome/components/host/__init__.py b/esphome/components/host/__init__.py index 795c1a556d..b6a3b8b615 100644 --- a/esphome/components/host/__init__.py +++ b/esphome/components/host/__init__.py @@ -43,6 +43,8 @@ CONFIG_SCHEMA = cv.All( async def to_code(config): cg.add_build_flag("-DUSE_HOST") cg.add_define("USE_NATIVE_64BIT_TIME") + # The prefs file finds stored preferences by key, so key migration is possible + cg.add_define("USE_PREFERENCE_KEY_LOOKUP") cg.add_define("USE_ESPHOME_HOST_MAC_ADDRESS", config[CONF_MAC_ADDRESS].parts) cg.add_build_flag("-std=gnu++20") cg.add_define("ESPHOME_BOARD", "host") diff --git a/esphome/components/host/preferences.h b/esphome/components/host/preferences.h index 5f723e0675..b591fa0aab 100644 --- a/esphome/components/host/preferences.h +++ b/esphome/components/host/preferences.h @@ -27,6 +27,9 @@ class HostPreferences final : public PreferencesMixin { return true; } + /// One-shot read of a stored preference by key, without allocating a backend + bool load_from_key(uint32_t type, uint8_t *data, size_t len) { return this->load(type, data, len); } + bool load(uint32_t key, uint8_t *data, size_t len) { if (len > 255) return false; diff --git a/esphome/components/infrared/infrared.cpp b/esphome/components/infrared/infrared.cpp index 9b97995a96..288b1e5c40 100644 --- a/esphome/components/infrared/infrared.cpp +++ b/esphome/components/infrared/infrared.cpp @@ -154,12 +154,8 @@ bool Infrared::on_receive(remote_base::RemoteReceiveData data) { // Forward received IR data to API server #if defined(USE_API) && defined(USE_IR_RF) if (api::global_api_server != nullptr) { -#ifdef USE_DEVICES - uint32_t device_id = this->get_device_id(); -#else - uint32_t device_id = 0; -#endif - api::global_api_server->send_infrared_rf_receive_event(device_id, this->get_object_id_hash(), &data.get_raw_data()); + api::global_api_server->send_infrared_rf_receive_event(this->get_device_id_or_zero(), this->get_entity_key(), + &data.get_raw_data()); } #endif return false; // Don't consume the event, allow other listeners to process it diff --git a/esphome/components/libretiny/__init__.py b/esphome/components/libretiny/__init__.py index 7dbce7a07c..c51af373b3 100644 --- a/esphome/components/libretiny/__init__.py +++ b/esphome/components/libretiny/__init__.py @@ -461,6 +461,8 @@ async def component_to_code(config): # setup board config cg.add_platformio_option("board", config[CONF_BOARD]) cg.add_build_flag("-DUSE_LIBRETINY") + # FlashDB finds stored preferences by key, so preference key migration is possible + cg.add_define("USE_PREFERENCE_KEY_LOOKUP") cg.add_build_flag(f"-DUSE_{config[CONF_COMPONENT_ID].upper()}") cg.add_build_flag(f"-DUSE_LIBRETINY_VARIANT_{config[CONF_FAMILY]}") cg.add_define("ESPHOME_BOARD", config[CONF_BOARD]) diff --git a/esphome/components/libretiny/preferences.cpp b/esphome/components/libretiny/preferences.cpp index 313b36d31e..d0bd3bf26b 100644 --- a/esphome/components/libretiny/preferences.cpp +++ b/esphome/components/libretiny/preferences.cpp @@ -70,12 +70,21 @@ void LibreTinyPreferences::open() { } ESPPreferenceObject LibreTinyPreferences::make_preference(size_t length, uint32_t type) { - auto *pref = new LibreTinyPreferenceBackend(); // NOLINT(cppcoreguidelines-owning-memory) - pref->db = &this->db; - pref->blob = &this->blob; - pref->key = type; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + return ESPPreferenceObject(new LibreTinyPreferenceBackend(this->make_backend_(type))); +} - return ESPPreferenceObject(pref); +LibreTinyPreferenceBackend LibreTinyPreferences::make_backend_(uint32_t type) { + LibreTinyPreferenceBackend backend; + backend.key = type; + backend.db = &this->db; + backend.blob = &this->blob; + return backend; +} + +bool LibreTinyPreferences::load_from_key(uint32_t type, uint8_t *data, size_t len) { + LibreTinyPreferenceBackend backend = this->make_backend_(type); + return backend.load(data, len); } bool LibreTinyPreferences::sync() { diff --git a/esphome/components/libretiny/preferences.h b/esphome/components/libretiny/preferences.h index 8365d590c2..fd86c48b20 100644 --- a/esphome/components/libretiny/preferences.h +++ b/esphome/components/libretiny/preferences.h @@ -16,6 +16,8 @@ class LibreTinyPreferences final : public PreferencesMixin return this->make_preference(length, type); } ESPPreferenceObject make_preference(size_t length, uint32_t type); + /// One-shot read of a stored preference by key, without allocating a backend + bool load_from_key(uint32_t type, uint8_t *data, size_t len); bool sync(); bool reset(); @@ -23,6 +25,7 @@ class LibreTinyPreferences final : public PreferencesMixin struct fdb_blob blob; protected: + LibreTinyPreferenceBackend make_backend_(uint32_t type); bool is_changed_(fdb_kvdb_t db, const NVSData &to_save, const char *key_str); }; diff --git a/esphome/components/mqtt/__init__.py b/esphome/components/mqtt/__init__.py index 4a5eacf449..35d496adeb 100644 --- a/esphome/components/mqtt/__init__.py +++ b/esphome/components/mqtt/__init__.py @@ -62,6 +62,7 @@ from esphome.const import ( PlatformFramework, ) from esphome.core import CORE, CoroPriority, coroutine_with_priority +from esphome.core.entity_helpers import ObjectIdEntity, validate_no_object_id_conflicts from esphome.types import ConfigType DEPENDENCIES = ["network"] @@ -332,6 +333,68 @@ CONFIG_SCHEMA = cv.All( ) +# Platforms whose MQTT components subscribe to an object_id-derived command topic. +# Keep in sync with the platforms extending cv.MQTT_COMMAND_COMPONENT_SCHEMA, plus +# text, whose MQTT component subscribes a command topic that cannot be overridden. +_COMMAND_TOPIC_PLATFORMS = frozenset( + { + "alarm_control_panel", + "button", + "climate", + "cover", + "datetime", + "fan", + "light", + "lock", + "number", + "select", + "switch", + "text", + "update", + "valve", + } +) + + +# Platforms whose MQTT components derive extra sub-topics (position/command, +# mode/command, speed/command, ...) from the object_id, each with its own config +# key; custom state and command topics cannot exempt them from conflicting. +_SUB_TOPIC_PLATFORMS = frozenset({"climate", "cover", "fan", "valve"}) + + +def _topics_conflict(entities: list[ObjectIdEntity], config: ConfigType) -> bool: + """Check whether more than one entity actually uses an object_id-derived topic. + + An empty topic_prefix disables default topics entirely, custom state and + command topics avoid the default topics, and disabling discovery (globally + or per entity) avoids the discovery config topic. + """ + if config[CONF_TOPIC_PREFIX]: + platform = entities[0].platform + if platform in _SUB_TOPIC_PLATFORMS: + return True + if sum(CONF_STATE_TOPIC not in entity.config for entity in entities) > 1: + return True + if ( + platform in _COMMAND_TOPIC_PLATFORMS + and sum(CONF_COMMAND_TOPIC not in entity.config for entity in entities) > 1 + ): + return True + if not config[CONF_DISCOVERY]: + return False + discovery_entities = sum( + entity.config.get(CONF_DISCOVERY, True) for entity in entities + ) + return discovery_entities > 1 + + +FINAL_VALIDATE_SCHEMA = validate_no_object_id_conflicts( + "mqtt builds default topics and discovery topics from the entity object_id, " + "which is the name converted to ASCII", + conflict_filter=_topics_conflict, +) + + def exp_mqtt_message(config): if config is None: return cg.optional(cg.TemplateArguments(MQTTMessage)) diff --git a/esphome/components/prometheus/__init__.py b/esphome/components/prometheus/__init__.py index cc1541ce80..0a69160fc1 100644 --- a/esphome/components/prometheus/__init__.py +++ b/esphome/components/prometheus/__init__.py @@ -3,6 +3,7 @@ from esphome.components import web_server_base from esphome.components.web_server_base import CONF_WEB_SERVER_BASE_ID import esphome.config_validation as cv from esphome.const import CONF_ID, CONF_INCLUDE_INTERNAL, CONF_NAME, CONF_RELABEL +from esphome.core.entity_helpers import validate_no_object_id_conflicts from esphome.cpp_types import EntityBase AUTO_LOAD = ["web_server_base"] @@ -35,6 +36,11 @@ CONFIG_SCHEMA = cv.Schema( }, ).extend(cv.COMPONENT_SCHEMA) +FINAL_VALIDATE_SCHEMA = validate_no_object_id_conflicts( + "prometheus builds metric labels from the entity object_id, " + "which is the name converted to ASCII" +) + async def to_code(config): paren = await cg.get_variable(config[CONF_WEB_SERVER_BASE_ID]) diff --git a/esphome/components/radio_frequency/radio_frequency.cpp b/esphome/components/radio_frequency/radio_frequency.cpp index 3e0a905737..fe6c6a9cb5 100644 --- a/esphome/components/radio_frequency/radio_frequency.cpp +++ b/esphome/components/radio_frequency/radio_frequency.cpp @@ -99,12 +99,8 @@ bool RadioFrequency::on_receive(remote_base::RemoteReceiveData data) { // Forward received RF data to API server #if defined(USE_API) && defined(USE_RADIO_FREQUENCY) if (api::global_api_server != nullptr) { -#ifdef USE_DEVICES - uint32_t device_id = this->get_device_id(); -#else - uint32_t device_id = 0; -#endif - api::global_api_server->send_infrared_rf_receive_event(device_id, this->get_object_id_hash(), &data.get_raw_data()); + api::global_api_server->send_infrared_rf_receive_event(this->get_device_id_or_zero(), this->get_entity_key(), + &data.get_raw_data()); } #endif return false; // Don't consume the event, allow other listeners to process it diff --git a/esphome/components/template/text/template_text.cpp b/esphome/components/template/text/template_text.cpp index af134e6ed4..ffe11cf229 100644 --- a/esphome/components/template/text/template_text.cpp +++ b/esphome/components/template/text/template_text.cpp @@ -20,18 +20,14 @@ void TemplateText::setup() { // Need std::string for pref_->setup() to fill from flash std::string value{this->initial_value_ != nullptr ? this->initial_value_ : ""}; - // For future hash migration: use migrate_entity_preference_() with: - // old_key = get_preference_hash() + extra - // new_key = get_preference_hash_v2() + extra - // See: https://github.com/esphome/backlog/issues/85 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - uint32_t key = this->get_preference_hash(); -#pragma GCC diagnostic pop - key += this->traits.get_min_length() << 2; - key += this->traits.get_max_length() << 4; - key += fnv1_hash(this->traits.get_pattern_c_str()) << 6; - this->pref_->setup(key, value); + uint32_t extra = 0; + extra += this->traits.get_min_length() << 2; + extra += this->traits.get_max_length() << 4; + extra += fnv1_hash(this->traits.get_pattern_c_str()) << 6; + // TextSaver::setup() picks the key for the platform and migrates old data once + uint32_t key = this->preference_key_base_() + extra; + uint32_t old_key = this->old_preference_key_base_() + extra; + this->pref_->setup(key, old_key, value); if (!value.empty()) this->publish_state(value); } diff --git a/esphome/components/template/text/template_text.h b/esphome/components/template/text/template_text.h index 229a61d9b8..beeea4396a 100644 --- a/esphome/components/template/text/template_text.h +++ b/esphome/components/template/text/template_text.h @@ -14,7 +14,9 @@ class TemplateTextSaverBase { public: virtual bool save(const std::string &value) { return true; } - virtual void setup(uint32_t id, std::string &value) {} + /// old_id is the pre-2026.8.0 preference key; data stored under it is moved to id once. + /// See: https://github.com/esphome/backlog/issues/85 + virtual void setup(uint32_t id, uint32_t old_id, std::string &value) {} protected: ESPPreferenceObject pref_; @@ -45,11 +47,16 @@ template class TextSaver : public TemplateTextSaverBase { // Make the preference object. Fill the provided location with the saved data // If it is available, else leave it alone - void setup(uint32_t id, std::string &value) override { - this->pref_ = global_preferences->make_preference(id); - + void setup(uint32_t id, uint32_t old_id, std::string &value) override { char temp[SZ + 1]; +#ifdef USE_PREFERENCE_KEY_LOOKUP + this->pref_ = global_preferences->make_preference(id); + bool hasdata = migrate_preference(this->pref_, reinterpret_cast(temp), SZ + 1, old_id, id); +#else + // Slot-based backends keep the old key; it is only a validity tag on a positional slot + this->pref_ = global_preferences->make_preference(old_id); bool hasdata = this->pref_.load(&temp); +#endif if (hasdata) { size_t len = static_cast(temp[0]); diff --git a/esphome/components/zephyr/__init__.py b/esphome/components/zephyr/__init__.py index 9f755a6eea..338d1986ea 100644 --- a/esphome/components/zephyr/__init__.py +++ b/esphome/components/zephyr/__init__.py @@ -158,6 +158,8 @@ def add_extra_script(stage: str, filename: str, path: Path) -> None: def zephyr_to_code(config: ConfigType) -> None: cg.add_build_flag("-DUSE_ZEPHYR") cg.add_define("USE_NATIVE_64BIT_TIME") + # The settings subsystem finds stored preferences by key, so key migration is possible + cg.add_define("USE_PREFERENCE_KEY_LOOKUP") cg.set_cpp_standard("gnu++20") # c++ support zephyr_add_prj_conf("FPU", True) diff --git a/esphome/components/zephyr/preferences.cpp b/esphome/components/zephyr/preferences.cpp index c26a1d6d53..ed22613625 100644 --- a/esphome/components/zephyr/preferences.cpp +++ b/esphome/components/zephyr/preferences.cpp @@ -58,12 +58,19 @@ void ZephyrPreferences::open() { ESP_LOGD(TAG, "Loaded %zu settings.", this->backends_.size()); } -ESPPreferenceObject ZephyrPreferences::make_preference(size_t length, uint32_t type) { +ZephyrPreferenceBackend *ZephyrPreferences::find_backend_(uint32_t type) { for (auto *backend : this->backends_) { if (backend->get_type() == type) { - return ESPPreferenceObject(backend); + return backend; } } + return nullptr; +} + +ESPPreferenceObject ZephyrPreferences::make_preference(size_t length, uint32_t type) { + if (auto *backend = this->find_backend_(type)) { + return ESPPreferenceObject(backend); + } auto *pref = new ZephyrPreferenceBackend(type); // NOLINT(cppcoreguidelines-owning-memory) char key_buf[KEY_BUFFER_SIZE]; pref->format_key(key_buf, sizeof(key_buf)); @@ -72,6 +79,13 @@ ESPPreferenceObject ZephyrPreferences::make_preference(size_t length, uint32_t t return ESPPreferenceObject(pref); } +bool ZephyrPreferences::load_from_key(uint32_t type, uint8_t *data, size_t len) { + // Stored settings are preloaded into backends_ at boot by settings_load_subtree(), + // so a key with no registered backend has no stored data. + auto *backend = this->find_backend_(type); + return backend != nullptr && backend->load(data, len); +} + bool ZephyrPreferences::sync() { ESP_LOGD(TAG, "Save settings"); int err = settings_save(); diff --git a/esphome/components/zephyr/preferences.h b/esphome/components/zephyr/preferences.h index 9e2555f910..b1ad95fd74 100644 --- a/esphome/components/zephyr/preferences.h +++ b/esphome/components/zephyr/preferences.h @@ -16,10 +16,13 @@ class ZephyrPreferences final : public PreferencesMixin { return this->make_preference(length, type); } ESPPreferenceObject make_preference(size_t length, uint32_t type); + /// One-shot read of a stored preference by key, without allocating or registering a backend + bool load_from_key(uint32_t type, uint8_t *data, size_t len); bool sync(); bool reset(); protected: + ZephyrPreferenceBackend *find_backend_(uint32_t type); std::vector backends_; static int load_setting(const char *name, size_t len, settings_read_cb read_cb, void *cb_arg); diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index bf637d4c1f..deee127f49 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -621,8 +621,8 @@ class EsphomeCore: # Key: platform name (e.g. "sensor", "binary_sensor"), Value: count self.platform_counts: defaultdict[str, int] = defaultdict(int) # Track entity unique IDs to handle duplicates - # Dict mapping (device_id, platform, sanitized_name) -> entity metadata - self.unique_ids: dict[tuple[str, str, str], EntityMetadata] = {} + # Dict mapping (device_id, platform, name_hash) -> entity metadata + self.unique_ids: dict[tuple[str, str, int], EntityMetadata] = {} # Whether ESPHome was started in verbose mode self.verbose = False # Whether ESPHome was started in quiet mode diff --git a/esphome/core/application.h b/esphome/core/application.h index a12cdc4ac8..a18a6b31c8 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -120,8 +120,8 @@ class Application { // NOLINTBEGIN(bugprone-macro-parentheses) #define ENTITY_TYPE_(type, singular, plural, count, upper) \ void register_##singular(type *obj) { this->plural##_.push_back(obj); } \ - void register_##singular(type *obj, const char *name, uint32_t object_id_hash, uint32_t entity_fields) { \ - obj->configure_entity_(name, object_id_hash, entity_fields); \ + void register_##singular(type *obj, const char *name, uint32_t entity_key, uint32_t entity_fields) { \ + obj->configure_entity_(name, entity_key, entity_fields); \ this->plural##_.push_back(obj); \ } #define ENTITY_CONTROLLER_TYPE_(type, singular, plural, count, upper, callback) \ @@ -329,7 +329,7 @@ class Application { #define GET_ENTITY_METHOD(entity_type, entity_name, entities_member) \ entity_type *get_##entity_name##_by_key(uint32_t key, uint32_t device_id, bool include_internal = false) { \ for (auto *obj : this->entities_member##_) { \ - if (obj->get_object_id_hash() == key && obj->get_device_id() == device_id && \ + if (obj->get_entity_key() == key && obj->get_device_id() == device_id && \ (include_internal || !obj->is_internal())) \ return obj; \ } \ @@ -340,7 +340,7 @@ class Application { #define GET_ENTITY_METHOD(entity_type, entity_name, entities_member) \ entity_type *get_##entity_name##_by_key(uint32_t key, bool include_internal = false) { \ for (auto *obj : this->entities_member##_) { \ - if (obj->get_object_id_hash() == key && (include_internal || !obj->is_internal())) \ + if (obj->get_entity_key() == key && (include_internal || !obj->is_internal())) \ return obj; \ } \ return nullptr; \ diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 1d4cbcfc37..b184300681 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -157,6 +157,9 @@ #define USE_OUTPUT_FLOAT_POWER_SCALING #define USE_POWER_SUPPLY #define USE_PREFERENCES_SYNC_EVERY_LOOP +// Only defined by key-lookup preference backends (esp32, libretiny, host, zephyr); +// slot-based platforms (esp8266, rp2040) never set it in generated builds +#define USE_PREFERENCE_KEY_LOOKUP #define USE_PROVISIONING #define USE_QR_CODE #define USE_SAFE_MODE_BOOT_IS_GOOD_ON_SHUTDOWN diff --git a/esphome/core/entity_base.cpp b/esphome/core/entity_base.cpp index 32135860bb..328de05302 100644 --- a/esphome/core/entity_base.cpp +++ b/esphome/core/entity_base.cpp @@ -8,7 +8,7 @@ namespace esphome { static const char *const TAG = "entity_base"; -void EntityBase::configure_entity_(const char *name, uint32_t object_id_hash, uint32_t entity_fields) { +void EntityBase::configure_entity_(const char *name, uint32_t entity_key, uint32_t entity_fields) { this->name_ = StringRef(name); if (this->name_.empty()) { #ifdef USE_DEVICES @@ -30,15 +30,15 @@ void EntityBase::configure_entity_(const char *name, uint32_t object_id_hash, ui } } this->flags_.has_own_name = false; - // Dynamic name - must calculate hash at runtime - this->calc_object_id_(); + // Dynamic name - must calculate key at runtime + this->calc_entity_key_(); } 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; + // Static name - use pre-computed key if provided + if (entity_key != 0) { + this->entity_key_ = entity_key; } else { - this->calc_object_id_(); + this->calc_entity_key_(); } } // Unpack entity string table indices and flags from entity_fields. @@ -147,9 +147,15 @@ std::string EntityBase::get_icon() const { } #endif // !USE_ESP8266 -// 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()); +// Calculate the entity key directly from the raw name (no transformations) +void EntityBase::calc_entity_key_() { this->entity_key_ = fnv1_hash_bytes(this->name_.c_str(), this->name_.size()); } + +// Reconstruct the OLD (pre-2026.8.0) object_id-based hash for preference key compatibility. +// Named entities historically used the hash pre-computed by Python code generation, which +// sanitized per UTF-8 code point; entities without their own name computed the hash at +// runtime per byte. See https://github.com/esphome/backlog/issues/85 +uint32_t EntityBase::calc_old_object_id_hash_() const { + return fnv1_hash_object_id(this->name_.c_str(), this->name_.size(), this->flags_.has_own_name); } size_t EntityBase::write_object_id_to(char *buf, size_t buf_size) const { @@ -166,46 +172,23 @@ StringRef EntityBase::get_object_id_to(std::span buf) c return StringRef(buf.data(), len); } -// Migrate preference data from old_key to new_key if they differ. -// This helper is exposed so callers with custom key computation (like TextPrefs) -// can use it for manual migration. See: https://github.com/esphome/backlog/issues/85 -// -// FUTURE IMPLEMENTATION: -// This will require raw load/save methods on ESPPreferenceObject that take uint8_t* and size. -// void EntityBase::migrate_entity_preference_(size_t size, uint32_t old_key, uint32_t new_key) { -// if (old_key == new_key) -// return; -// auto old_pref = global_preferences->make_preference(size, old_key); -// auto new_pref = global_preferences->make_preference(size, new_key); -// SmallBufferWithHeapFallback<64> buffer(size); -// if (old_pref.load(buffer.data(), size)) { -// new_pref.save(buffer.data(), size); -// } -// } - ESPPreferenceObject EntityBase::make_entity_preference_(size_t size, uint32_t version) { - // This helper centralizes preference creation to enable fixing hash collisions. + // The old key hashed the sanitized object_id, so multiple entity names could collide on + // one key and overwrite each other's stored preferences; the new key hashes the raw name. // See: https://github.com/esphome/backlog/issues/85 - // - // COLLISION PROBLEM: get_preference_hash() uses fnv1_hash on sanitized object_id. - // Multiple entity names can sanitize to the same object_id: - // - "Living Room" and "living_room" both become "living_room" - // - UTF-8 names like "温度" and "湿度" both become "__" (underscores) - // This causes entities to overwrite each other's stored preferences. - // - // FUTURE MIGRATION: When implementing get_preference_hash_v2() that hashes - // the original entity name (not sanitized object_id): - // - // uint32_t old_key = this->get_preference_hash() ^ version; - // uint32_t new_key = this->get_preference_hash_v2() ^ version; - // this->migrate_entity_preference_(size, old_key, new_key); - // return global_preferences->make_preference(size, new_key); - // -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" - uint32_t key = this->get_preference_hash() ^ version; -#pragma GCC diagnostic pop - return global_preferences->make_preference(size, key); + uint32_t old_key = this->old_preference_key_base_() ^ version; +#ifdef USE_PREFERENCE_KEY_LOOKUP + uint32_t new_key = this->preference_key_base_() ^ version; + auto pref = global_preferences->make_preference(size, new_key); + // All in-tree entity preferences fit the stack buffer, so migration never hits the heap + SmallBufferWithHeapFallback<64> buffer(size); + migrate_preference(pref, buffer.get(), size, old_key, new_key); + return pref; +#else + // Slot-based backends keep the old key: it is only a validity tag on a positional slot, + // so collisions cannot corrupt data there and keeping it preserves stored state. + return global_preferences->make_preference(size, old_key); +#endif } #ifdef USE_ENTITY_ICON diff --git a/esphome/core/entity_base.h b/esphome/core/entity_base.h index 4f708209d4..7f8e5f2630 100644 --- a/esphome/core/entity_base.h +++ b/esphome/core/entity_base.h @@ -73,8 +73,17 @@ class EntityBase { // 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; } - // Get the unique Object ID of this Entity - uint32_t get_object_id_hash() const { return this->object_id_hash_; } + // Get the unique key of this Entity: FNV-1 hash of the raw entity name. + // This is the key sent to API clients and used to route entity state. + uint32_t get_entity_key() const { return this->entity_key_; } + + /// Returns the LEGACY object_id hash, unchanged from previous releases, so existing + /// callers keep getting stable values (for example preference keys). This is no longer + /// the key sent to API clients; that is get_entity_key(). + ESPDEPRECATED("Use get_entity_key() for the entity key sent to API clients, or " + "make_entity_preference() for preference storage. Will be removed in 2027.1.0.", + "2026.8.0") + uint32_t get_object_id_hash() const { return this->calc_old_object_id_hash_(); } /// Get object_id with zero heap allocation /// For static case: returns StringRef to internal storage (buffer unused) @@ -181,40 +190,24 @@ class EntityBase { // Set has_state - for components that need to manually set this void set_has_state(bool state) { this->flags_.has_state = state; } - /** - * @brief Get a unique hash for storing preferences/settings for this entity. - * - * This method returns a hash that uniquely identifies the entity for the purpose of - * storing preferences (such as calibration, state, etc.). Unlike get_object_id_hash(), - * this hash also incorporates the device_id (if devices are enabled), ensuring uniqueness - * across multiple devices that may have entities with the same object_id. - * - * Use this method when storing or retrieving preferences/settings that should be unique - * per device-entity pair. Use get_object_id_hash() when you need a hash that identifies - * the entity regardless of the device it belongs to. - * - * For backward compatibility, if device_id is 0 (the main device), the hash is unchanged - * from previous versions, so existing single-device configurations will continue to work. - * - * @return uint32_t The unique hash for preferences, including device_id if available. - * @deprecated Use make_entity_preference() instead, or preferences won't be migrated. - * See https://github.com/esphome/backlog/issues/85 - */ - ESPDEPRECATED("Use make_entity_preference() instead, or preferences won't be migrated. " - "See https://github.com/esphome/backlog/issues/85. Will be removed in 2027.1.0.", - "2026.7.0") - uint32_t get_preference_hash() { + /// Get this entity's device id, or 0 when devices are not compiled in (main device). + uint32_t get_device_id_or_zero() const { #ifdef USE_DEVICES - // Combine object_id_hash with device_id to ensure uniqueness across devices - // Note: device_id is 0 for the main device, so XORing with 0 preserves the original hash - // This ensures backward compatibility for existing single-device configurations - return this->get_object_id_hash() ^ this->get_device_id(); + return this->get_device_id(); #else - // Without devices, just use object_id_hash as before - return this->get_object_id_hash(); + return 0; #endif } + /// Get the LEGACY preference key: FNV-1 hash of the sanitized object_id, XOR device_id. + /// Intentionally keeps the old algorithm so external callers that store preferences under + /// this key keep stable keys; make_entity_preference() migrates to the new raw-name key, + /// this method never will. + ESPDEPRECATED("Use make_entity_preference() instead, or preferences won't be migrated. " + "See https://github.com/esphome/backlog/issues/85. Will be removed in 2027.1.0.", + "2026.8.0") + uint32_t get_preference_hash() { return this->old_preference_key_base_(); } + /// Create a preference object for storing this entity's state/settings. /// @tparam T The type of data to store (must be trivially copyable) /// @param version Optional version hash XORed with preference key (change when struct layout changes) @@ -230,9 +223,9 @@ class EntityBase { // before push_back, so codegen can emit a single combined call per entity. friend class Application; - /// Combined entity setup from codegen: set name, object_id hash, entity string indices, and flags. + /// Combined entity setup from codegen: set name, entity key, entity string indices, and flags. /// Bit layout of entity_fields is defined by the ENTITY_FIELD_*_SHIFT constants above. - void configure_entity_(const char *name, uint32_t object_id_hash, uint32_t entity_fields); + void configure_entity_(const char *name, uint32_t entity_key, uint32_t entity_fields); #ifdef USE_DEVICES // Codegen-only setter — only accessible from setup() via friend declaration. @@ -240,13 +233,24 @@ class EntityBase { #endif /// Non-template helper for make_entity_preference() to avoid code bloat. - /// When preference hash algorithm changes, migration logic goes here. + /// Migrates preferences from the old sanitized-object_id key to the raw-name key + /// on key-lookup platforms. See: https://github.com/esphome/backlog/issues/85 ESPPreferenceObject make_entity_preference_(size_t size, uint32_t version); - void calc_object_id_(); + void calc_entity_key_(); + + /// Reconstruct the OLD (pre-2026.8.0) sanitized-object_id hash for preference keys. + uint32_t calc_old_object_id_hash_() const; + + /// Preference key base for this entity: raw-name entity key XOR device_id. + uint32_t preference_key_base_() const { return this->entity_key_ ^ this->get_device_id_or_zero(); } + + /// Legacy preference key base: sanitized-object_id hash XOR device_id. + /// Note: device_id is 0 for the main device, so XORing with 0 preserves the original hash. + uint32_t old_preference_key_base_() const { return this->calc_old_object_id_hash_() ^ this->get_device_id_or_zero(); } StringRef name_; - uint32_t object_id_hash_{}; + uint32_t entity_key_{}; #ifdef USE_DEVICES Device *device_{}; #endif diff --git a/esphome/core/entity_helpers.py b/esphome/core/entity_helpers.py index 38c7f3ca43..5060e32a2d 100644 --- a/esphome/core/entity_helpers.py +++ b/esphome/core/entity_helpers.py @@ -25,19 +25,86 @@ from esphome.core.config import ( from esphome.cpp_generator import MockObj, RawStatement, add, get_variable from esphome.cpp_types import App import esphome.final_validate as fv -from esphome.helpers import cpp_string_escape, fnv1_hash_object_id, sanitize, snake_case +from esphome.helpers import cpp_string_escape, fnv1_hash_name, sanitize, snake_case from esphome.types import ConfigType, EntityMetadata _LOGGER = logging.getLogger(__name__) DOMAIN = "entity_string_pool" +_OBJECT_ID_DOMAIN = "entity_object_ids" + + +@dataclass +class ObjectIdEntity: + """An entity tracked by the sanitized object_id its name resolves to.""" + + name: str + platform: str + config: ConfigType + + +def _get_object_id_registry() -> dict[tuple[str, str, str], list[ObjectIdEntity]]: + """(device_id, platform, sanitized object_id) -> entities resolving to it.""" + return CORE.data.setdefault(_OBJECT_ID_DOMAIN, {}) + + +def validate_no_object_id_conflicts( + reason: str, + conflict_filter: Callable[[list[ObjectIdEntity], ConfigType], bool] | None = None, +) -> Callable[[ConfigType], ConfigType]: + """Create a final-validate step that rejects entities with colliding object_ids. + + Entity keys are hashed from the raw name, so names that only differ in characters + lost during sanitizing (for example two UTF-8 names) validate fine in general. + Components that still address entities by the sanitized object_id string must + reject those configs until they are migrated to raw names. + + Args: + reason: One sentence stating what the component builds from the object_id, + e.g. "mqtt builds default topics from the entity object_id" + conflict_filter: Optional predicate receiving the colliding entities and the + component config; return False when the component is not affected + + Returns: + A validator function for use as (or within) FINAL_VALIDATE_SCHEMA + """ + + def validator(config: ConfigType) -> ConfigType: + # Skip in testing_mode, which is used for grouped component testing + if CORE.testing_mode: + return config + conflicts = { + key: entities + for key, entities in _get_object_id_registry().items() + if len(entities) > 1 + and (conflict_filter is None or conflict_filter(entities, config)) + } + if not conflicts: + return config + lines = [f"{reason}, so these entities would conflict:"] + lines.extend( + f" - {platform} entities " + + ", ".join(f"'{e.name}'" for e in entities) + + (f" on device '{device_id}'" if device_id else "") + + f" share the object_id '{object_id}'" + for (device_id, platform, object_id), entities in conflicts.items() + ) + lines.append( + "To fix: Add unique ASCII characters (e.g., '1', '2', or 'A', 'B') " + "to distinguish the names" + ) + raise cv.Invalid("\n".join(lines)) + + return validator + + # Private config keys for storing registered string indices _KEY_DC_IDX = "_entity_dc_idx" _KEY_UOM_IDX = "_entity_uom_idx" _KEY_ICON_IDX = "_entity_icon_idx" _KEY_ENTITY_NAME = "_entity_name" -_KEY_OBJECT_ID_HASH = "_entity_object_id_hash" +_KEY_ENTITY_KEY = "_entity_key" # Bit layout for entity_fields in configure_entity_(). # Keep in sync with ENTITY_FIELD_*_SHIFT constants in esphome/core/entity_base.h @@ -300,7 +367,7 @@ def finalize_entity_strings(var: MockObj, config: ConfigType) -> None: standalone ``var->configure_entity_(name, hash, packed)``. """ entity_name = config[_KEY_ENTITY_NAME] - object_id_hash = config[_KEY_OBJECT_ID_HASH] + entity_key = config[_KEY_ENTITY_KEY] dc_idx = config.get(_KEY_DC_IDX, 0) uom_idx = config.get(_KEY_UOM_IDX, 0) icon_idx = config.get(_KEY_ICON_IDX, 0) @@ -320,57 +387,30 @@ def finalize_entity_strings(var: MockObj, config: ConfigType) -> None: register_method = config.get(_KEY_REGISTER_METHOD) if register_method is not None: expr = getattr(App, f"register_{register_method}")( - var, entity_name, object_id_hash, packed + var, entity_name, entity_key, packed ) else: - expr = var.configure_entity_(entity_name, object_id_hash, packed) + expr = var.configure_entity_(entity_name, entity_key, packed) if comment: add(RawStatement(f"{expr}; // {comment}")) else: add(expr) -def get_base_entity_object_id( +def get_base_entity_name( name: str, friendly_name: str | None, device_name: str | None = None ) -> str: - """Calculate the base object ID for an entity that will be set via set_object_id(). + """Return the base name whose hash becomes this entity's key on the device. - This function calculates what object_id_c_str_ should be set to in C++. + Follows the name selection in C++ EntityBase::configure_entity_() (entity_base.cpp): + entity name, then sub-device name, then friendly name, then the device name. - The C++ EntityBase::write_object_id_to() (entity_base.cpp) works as: - - If !has_own_name && is_name_add_mac_suffix_enabled(): - return str_sanitize(str_snake_case(App.get_friendly_name())) // Dynamic - - Else: - return object_id_c_str_ ?? "" // What we set via set_object_id() - - Since we're calculating what to pass to set_object_id(), we always need to - generate the object_id the same way, regardless of name_add_mac_suffix setting. - - Args: - name: The entity name (empty string if no name) - friendly_name: The friendly name from CORE.friendly_name - device_name: The device name if entity is on a sub-device - - Returns: - The base object ID to use for duplicate checking and to pass to set_object_id() + This is a config-time approximation for duplicate checking: when + name_add_mac_suffix is enabled the device appends the MAC suffix at runtime, + which is unknown here and identical for every entity on the device, so + ignoring it cannot change whether two entities collide with each other. """ - - if name: - # Entity has its own name (has_own_name will be true) - base_str = name - elif device_name: - # Entity has empty name and is on a sub-device - # C++ EntityBase::set_name() uses device->get_name() when device is set - base_str = device_name - elif friendly_name: - # Entity has empty name (has_own_name will be false) - # C++ uses App.get_friendly_name() which returns friendly_name or device name - base_str = friendly_name - else: - # Fallback to device name - base_str = CORE.name - - return sanitize(snake_case(base_str)) + return name or device_name or friendly_name or CORE.name def setup_entity(var_or_platform, config=None, platform=None): @@ -429,15 +469,15 @@ async def _setup_entity_impl(var: MockObj, config: ConfigType, platform: str) -> device: MockObj = await get_variable(device_id_obj) add(var.set_device_(device)) - # Pre-compute entity name and object_id hash for configure_entity_() + # Pre-compute entity name and entity key for configure_entity_() # which is emitted later by finalize_entity_strings(). - # For named entities: pre-compute hash from entity name - # For empty-name entities: pass 0, C++ calculates hash at runtime from - # device name, friendly_name, or app name (bug-for-bug compatibility) + # For named entities: pre-compute the key from the raw entity name + # For empty-name entities: pass 0, C++ calculates the key at runtime from + # device name, friendly_name, or app name entity_name = config[CONF_NAME] - object_id_hash = fnv1_hash_object_id(entity_name) if entity_name else 0 + entity_key = fnv1_hash_name(entity_name) if entity_name else 0 config[_KEY_ENTITY_NAME] = entity_name - config[_KEY_OBJECT_ID_HASH] = object_id_hash + config[_KEY_ENTITY_KEY] = entity_key # Store flags for packing into configure_entity_() config[_KEY_DISABLED_BY_DEFAULT] = int(config[CONF_DISABLED_BY_DEFAULT]) if CONF_INTERNAL in config: @@ -550,14 +590,14 @@ def entity_duplicate_validator(platform: str) -> Callable[[ConfigType], ConfigTy # Use the device ID string directly for uniqueness device_id = device_id_obj.id - # Calculate what object_id will actually be used - # This handles empty names correctly by using device/friendly names - name_key = get_base_entity_object_id( - entity_name, CORE.friendly_name, device_name - ) + # Hash the same raw name the device hashes into the entity key at runtime. + # This handles empty names correctly by using device/friendly names. + base_name = get_base_entity_name(entity_name, CORE.friendly_name, device_name) + name_hash = fnv1_hash_name(base_name) - # Check for duplicates - unique_key = (device_id, platform, name_key) + # Check for duplicates: two entities on the same device and platform must not + # share an entity key, since the key is what routes state to API clients + unique_key = (device_id, platform, name_hash) if unique_key in CORE.unique_ids: # Get the existing entity metadata existing = CORE.unique_ids[unique_key] @@ -581,14 +621,13 @@ def entity_duplicate_validator(platform: str) -> Callable[[ConfigType], ConfigTy if existing_component != "unknown": conflict_msg += f" from component '{existing_component}'" - # Show both original names and their ASCII-only versions if they differ - sanitized_msg = "" + # Different names can only clash here through a genuine hash collision + collision_msg = "" if entity_name != existing_name: - sanitized_msg = ( - f"\n Original names: '{entity_name}' and '{existing_name}'" - f"\n Both convert to ASCII ID: '{name_key}'" - "\n To fix: Add unique ASCII characters (e.g., '1', '2', or 'A', 'B')" - "\n to distinguish them" + collision_msg = ( + f"\n The names '{entity_name}' and '{existing_name}' produce the" + f"\n same entity key hash ({name_hash:#010x})." + "\n To fix: Rename one of the entities" ) # Skip duplicate entity name validation when testing_mode is enabled @@ -598,9 +637,22 @@ def entity_duplicate_validator(platform: str) -> Callable[[ConfigType], ConfigTy f"Duplicate {platform} entity with name '{entity_name}' found{device_prefix}. " f"{conflict_msg}. " "Each entity on a device must have a unique name within its platform." - f"{sanitized_msg}" + f"{collision_msg}" ) + # Components that still address entities by the sanitized object_id reject + # colliding names in final validation via validate_no_object_id_conflicts(), + # so track every entity by the object_id its name resolves to. Scoped per + # device and platform to match the strictness configs had before entity keys + # moved to raw names: same-named entities on different sub-devices were + # already accepted then, internal entities were already skipped (above), and + # overlaps between platforms that share an MQTT component type (sensor and + # text_sensor both publish under "sensor") were already possible. + object_id = sanitize(snake_case(base_name)) + _get_object_id_registry().setdefault( + (device_id, platform, object_id), [] + ).append(ObjectIdEntity(base_name, platform, config)) + # Store metadata about this entity entity_metadata: EntityMetadata = { "name": entity_name, diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 7940df8780..3a243289ae 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -799,6 +799,19 @@ constexpr uint32_t FNV1_OFFSET_BASIS = 2166136261UL; /// FNV-1 32-bit prime constexpr uint32_t FNV1_PRIME = 16777619UL; +/// Calculate a FNV-1 hash over raw bytes with an explicit length. Unlike fnv1_hash(const char *), +/// each byte is hashed as an unsigned value, so results are platform-independent for bytes >= 0x80. +/// IMPORTANT: Must match Python fnv1_hash_name() in esphome/helpers.py, which hashes the UTF-8 +/// encoded bytes of the name. Used to compute entity keys from raw names. +inline uint32_t fnv1_hash_bytes(const char *str, size_t len) { + uint32_t hash = FNV1_OFFSET_BASIS; + for (size_t i = 0; i < len; i++) { + hash *= FNV1_PRIME; + hash ^= static_cast(str[i]); + } + return hash; +} + /// Extend a FNV-1 hash with an integer (hashes each byte). template constexpr uint32_t fnv1_hash_extend(uint32_t hash, T value) { using UnsignedT = std::make_unsigned_t; @@ -1003,12 +1016,20 @@ template inline char *str_sanitize_to(char (&buffer)[N], const char *s // str_sanitize moved to alloc_helpers.h - remove this comment before 2026.11.0 /// 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) { +/// This is the LEGACY entity hash, kept only to reconstruct preference keys that existing +/// devices already have stored; see https://github.com/esphome/backlog/issues/85. +/// With per_code_point set, UTF-8 continuation bytes are skipped so each multi-byte character +/// contributes one underscore — this matches Python fnv1_hash_object_id() in esphome/helpers.py, +/// which produced the hash for named entities. The per-byte form (default) matches the old +/// runtime hash for entities without their own name. Do not change either behavior. +/// Known limitation: Python's lower() is Unicode aware, so the rare code points it maps to a +/// different number of characters or to ASCII (e.g. 'İ', the Kelvin sign) reconstruct wrong; +/// such names skip migration once and fall back to their defaults. +inline uint32_t fnv1_hash_object_id(const char *str, size_t len, bool per_code_point = false) { uint32_t hash = FNV1_OFFSET_BASIS; for (size_t i = 0; i < len; i++) { + if (per_code_point && (static_cast(str[i]) & 0xC0) == 0x80) + continue; // UTF-8 continuation byte, already counted via its lead byte hash *= FNV1_PRIME; // Apply snake_case (space->underscore, uppercase->lowercase) then sanitize hash ^= static_cast(to_sanitized_char(to_snake_case_char(str[i]))); diff --git a/esphome/core/preference_backend.h b/esphome/core/preference_backend.h index 34bf84409d..b9bb9a0252 100644 --- a/esphome/core/preference_backend.h +++ b/esphome/core/preference_backend.h @@ -22,6 +22,12 @@ #include "esphome/components/zephyr/preference_backend.h" #endif +// Key-lookup preference backends find stored data by key; their platforms add the +// USE_PREFERENCE_KEY_LOOKUP define from Python codegen, which enables preference key +// migration. Slot-based backends (ESP8266, RP2040) instead allocate a storage slot for +// every make_preference() call and use the key only as a validity tag on that slot; +// migration is not possible there, and key collisions cannot corrupt data. + namespace esphome { #if !defined(USE_ESP32) && !defined(USE_ESP8266) && !defined(USE_RP2) && !defined(USE_LIBRETINY) && \ @@ -40,16 +46,22 @@ class ESPPreferenceObject { ESPPreferenceObject() = default; explicit ESPPreferenceObject(PreferenceBackend *backend) : backend_(backend) {} - template bool save(const T *src) { + template bool save(const T *src) { return this->save(reinterpret_cast(src), sizeof(T)); } + + template bool load(T *dest) { return this->load(reinterpret_cast(dest), sizeof(T)); } + + /// Raw save with explicit length, for callers that only know the size at runtime. + bool save(const uint8_t *src, size_t len) { if (this->backend_ == nullptr) return false; - return this->backend_->save(reinterpret_cast(src), sizeof(T)); + return this->backend_->save(src, len); } - template bool load(T *dest) { + /// Raw load with explicit length, for callers that only know the size at runtime. + bool load(uint8_t *dest, size_t len) { if (this->backend_ == nullptr) return false; - return this->backend_->load(reinterpret_cast(dest), sizeof(T)); + return this->backend_->load(dest, len); } protected: diff --git a/esphome/core/preferences.cpp b/esphome/core/preferences.cpp new file mode 100644 index 0000000000..8508647255 --- /dev/null +++ b/esphome/core/preferences.cpp @@ -0,0 +1,25 @@ +#include "esphome/core/preferences.h" +#include "esphome/core/log.h" +#include + +namespace esphome { + +#ifdef USE_PREFERENCE_KEY_LOOKUP +static const char *const TAG = "preferences"; + +bool migrate_preference(ESPPreferenceObject &new_pref, uint8_t *scratch, size_t size, uint32_t old_key, + uint32_t new_key) { + if (new_pref.load(scratch, size)) + return true; // Current data present - never overwrite newer data with the old copy + // One-shot read by key: no backend is allocated for the old key, so boots with + // nothing to migrate (for example fresh installs) cost no heap + if (old_key == new_key || !global_preferences->load_from_key(old_key, scratch, size)) + return false; // No data stored under the old key, nothing to migrate + if (!new_pref.save(scratch, size)) { + ESP_LOGW(TAG, "Pref migration %" PRIx32 " -> %" PRIx32 " failed", old_key, new_key); + } + return true; +} +#endif // USE_PREFERENCE_KEY_LOOKUP + +} // namespace esphome diff --git a/esphome/core/preferences.h b/esphome/core/preferences.h index 1efce5af51..d24d51164a 100644 --- a/esphome/core/preferences.h +++ b/esphome/core/preferences.h @@ -23,6 +23,7 @@ struct Preferences : public PreferencesMixin { using PreferencesMixin::make_preference; ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + bool load_from_key(uint32_t, uint8_t *, size_t) { return false; } /** * Commit pending writes to flash. @@ -43,3 +44,19 @@ using ESPPreferences = Preferences; extern ESPPreferences *global_preferences; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) } // namespace esphome #endif + +#ifdef USE_PREFERENCE_KEY_LOOKUP +namespace esphome { +/// Copy preference data stored under old_key into new_pref (created for new_key) if the keys +/// differ and new_pref has no data yet. scratch must hold at least size bytes. +/// Returns true when scratch holds the entity's current data (loaded or just migrated). +/// The old entry is intentionally left in place so a firmware downgrade still finds its data. +/// If saving under the new key fails, callers that consume scratch (like TextSaver) still get +/// valid data for this boot, callers that reload from the preference fall back to their +/// defaults, and the migration simply runs again on the next boot. +/// Only available on key-lookup preference backends; slot-based backends keep their old +/// keys instead. See: https://github.com/esphome/backlog/issues/85 +bool migrate_preference(ESPPreferenceObject &new_pref, uint8_t *scratch, size_t size, uint32_t old_key, + uint32_t new_key); +} // namespace esphome +#endif // USE_PREFERENCE_KEY_LOOKUP diff --git a/esphome/helpers.py b/esphome/helpers.py index 683aaedcf5..5c57a2823b 100644 --- a/esphome/helpers.py +++ b/esphome/helpers.py @@ -1,6 +1,6 @@ from __future__ import annotations -from collections.abc import MutableMapping +from collections.abc import Iterable, MutableMapping from contextlib import suppress import ipaddress import logging @@ -56,15 +56,20 @@ def ensure_unique_string(preferred_string, current_strings): return test_string -def fnv1_hash(string: str) -> int: - """FNV-1 32-bit hash function (multiply then XOR).""" +def _fnv1_hash(values: Iterable[int]) -> int: + """FNV-1 32-bit hash (multiply then XOR) over a sequence of integer values.""" hash_value = FNV1_OFFSET_BASIS - for char in string: + for value in values: hash_value = (hash_value * FNV1_PRIME) & 0xFFFFFFFF - hash_value ^= ord(char) + hash_value ^= value return hash_value +def fnv1_hash(string: str) -> int: + """FNV-1 32-bit hash function (multiply then XOR) over code points.""" + return _fnv1_hash(map(ord, string)) + + def fnv1a_32bit_hash(string: str) -> int: """FNV-1a 32-bit hash function (XOR then multiply). @@ -89,12 +94,27 @@ def fnv1a_32bit_hash(string: str) -> int: def fnv1_hash_object_id(name: str) -> int: """Compute FNV-1 hash of name with snake_case + sanitize transformations. - IMPORTANT: Must produce same result as C++ fnv1_hash_object_id() in helpers.h. - Used for pre-computing entity object_id hashes at code generation time. + IMPORTANT: Must produce same result as C++ fnv1_hash_object_id() in helpers.h + with per_code_point set. This is the OLD entity hash; it computes preference + keys that existing devices already have stored (see + https://github.com/esphome/backlog/issues/85) and is also still used for live + keys derived from config IDs (see the motion component's calibration key). + Note: lower() here is Unicode aware while the C++ reconstruction is not; see + the known limitation note on the C++ function. """ return fnv1_hash(sanitize(snake_case(name))) +def fnv1_hash_name(name: str) -> int: + """Compute FNV-1 hash of the raw entity name (UTF-8 bytes, no transformations). + + IMPORTANT: Must produce same result as C++ fnv1_hash_bytes() in helpers.h, + which hashes the name bytes as stored on the device. + Used for pre-computing entity keys at code generation time. + """ + return _fnv1_hash(name.encode("utf-8")) + + def strip_accents(value: str) -> str: """Remove accents from a string.""" import unicodedata @@ -627,7 +647,7 @@ def add_class_to_obj(value, cls): raise -def snake_case(value): +def snake_case(value: str) -> str: """Same behaviour as `helpers.cpp` method `str_snake_case`.""" return value.replace(" ", "_").lower() @@ -635,7 +655,7 @@ def snake_case(value): _DISALLOWED_CHARS = re.compile(r"[^a-zA-Z0-9-_]") -def sanitize(value): +def sanitize(value: str) -> str: """Same behaviour as `helpers.cpp` method `str_sanitize`.""" return _DISALLOWED_CHARS.sub("_", value) diff --git a/tests/integration/entity_utils.py b/tests/integration/entity_utils.py index 7596983ee2..95f6a0321e 100644 --- a/tests/integration/entity_utils.py +++ b/tests/integration/entity_utils.py @@ -8,7 +8,7 @@ from __future__ import annotations from typing import TYPE_CHECKING -from esphome.helpers import fnv1_hash_object_id, sanitize, snake_case +from esphome.helpers import fnv1_hash_name, sanitize, snake_case if TYPE_CHECKING: from aioesphomeapi import DeviceInfo, EntityInfo @@ -25,15 +25,16 @@ def infer_name_add_mac_suffix(device_info: DeviceInfo) -> bool: return device_info.name.endswith(f"-{mac_suffix}") -def _get_name_for_object_id( +def _resolve_entity_name( entity: EntityInfo, device_info: DeviceInfo, device_id_to_name: dict[int, str], ) -> str: - """Get the name used for object_id computation. + """Resolve the effective name for an entity. This is the algorithm that aioesphomeapi will use to determine which - name to use for computing object_id client-side from API data. + name to use for computing object_id client-side from API data; the same + name is what the device hashes into the entity key. Args: entity: The entity to get name for @@ -72,27 +73,27 @@ def compute_entity_object_id( Returns: The computed object_id string """ - name_for_id = _get_name_for_object_id(entity, device_info, device_id_to_name) - return compute_object_id(name_for_id) + name = _resolve_entity_name(entity, device_info, device_id_to_name) + return compute_object_id(name) -def compute_entity_hash( +def compute_entity_key( entity: EntityInfo, device_info: DeviceInfo, device_id_to_name: dict[int, str], ) -> int: - """Compute expected object_id hash for an entity. + """Compute expected entity key for an entity. Args: - entity: The entity to compute hash for + entity: The entity to compute the key for device_info: Device info from the API device_id_to_name: Mapping of device_id to device name for sub-devices Returns: - The computed FNV-1 hash + The computed FNV-1 hash of the raw name """ - name_for_id = _get_name_for_object_id(entity, device_info, device_id_to_name) - return fnv1_hash_object_id(name_for_id) + name = _resolve_entity_name(entity, device_info, device_id_to_name) + return fnv1_hash_name(name) def verify_entity_object_id( @@ -118,7 +119,7 @@ def verify_entity_object_id( f"expected '{expected_object_id}', got '{entity.object_id}'" ) - expected_hash = compute_entity_hash(entity, device_info, device_id_to_name) + expected_hash = compute_entity_key(entity, device_info, device_id_to_name) assert entity.key == expected_hash, ( f"hash mismatch for entity '{entity.name}': " f"expected {expected_hash:#x}, got {entity.key:#x}" diff --git a/tests/integration/fixtures/fnv1_hash_object_id.yaml b/tests/integration/fixtures/fnv1_hash_object_id.yaml index 2097b2fbf9..d4511bb8c6 100644 --- a/tests/integration/fixtures/fnv1_hash_object_id.yaml +++ b/tests/integration/fixtures/fnv1_hash_object_id.yaml @@ -71,6 +71,38 @@ esphome: ESP_LOGE("FNV1_OID", "empty FAILED: 0x%08x != 0x811c9dc5", hash_empty); } + // Raw name hash: matches Python fnv1_hash_name("My Sensor Name") + uint32_t hash_raw = esphome::fnv1_hash_bytes("My Sensor Name", 14); + if (hash_raw == 0x8cec6fb0) { + ESP_LOGI("FNV1_OID", "raw PASSED"); + } else { + ESP_LOGE("FNV1_OID", "raw FAILED: 0x%08x != 0x8cec6fb0", hash_raw); + } + + // Raw name hash over UTF-8 bytes: matches Python fnv1_hash_name("Température") + uint32_t hash_raw_utf8 = esphome::fnv1_hash_bytes("Temp\xc3\xa9rature", 12); + if (hash_raw_utf8 == 0x531a74aa) { + ESP_LOGI("FNV1_OID", "raw_utf8 PASSED"); + } else { + ESP_LOGE("FNV1_OID", "raw_utf8 FAILED: 0x%08x != 0x531a74aa", hash_raw_utf8); + } + + // Old-key UTF-8 variant: matches Python fnv1_hash_object_id("Température") + uint32_t hash_old_utf8 = esphome::fnv1_hash_object_id("Temp\xc3\xa9rature", 12, true); + if (hash_old_utf8 == 0x965698f3) { + ESP_LOGI("FNV1_OID", "old_utf8 PASSED"); + } else { + ESP_LOGE("FNV1_OID", "old_utf8 FAILED: 0x%08x != 0x965698f3", hash_old_utf8); + } + + // Old-key UTF-8 variant with multi-byte only name: Python fnv1_hash_object_id("温度") + uint32_t hash_old_cjk = esphome::fnv1_hash_object_id("\xe6\xb8\xa9\xe5\xba\xa6", 6, true); + if (hash_old_cjk == 0x3276cb9f) { + ESP_LOGI("FNV1_OID", "old_cjk PASSED"); + } else { + ESP_LOGE("FNV1_OID", "old_cjk FAILED: 0x%08x != 0x3276cb9f", hash_old_cjk); + } + host: api: logger: diff --git a/tests/integration/fixtures/multi_device_preferences.yaml b/tests/integration/fixtures/multi_device_preferences.yaml index 01e4394559..582add90a8 100644 --- a/tests/integration/fixtures/multi_device_preferences.yaml +++ b/tests/integration/fixtures/multi_device_preferences.yaml @@ -156,10 +156,17 @@ button: ESP_LOGI("test", "Device A Mode: %s", id(mode_device_a).current_option().c_str()); ESP_LOGI("test", "Device B Mode: %s", id(mode_device_b).current_option().c_str()); ESP_LOGI("test", "Main Mode: %s", id(mode_main).current_option().c_str()); - // Log preference hashes for entities that actually store preferences - ESP_LOGI("test", "Device A Switch Pref Hash: %u", id(light_device_a).get_preference_hash()); - ESP_LOGI("test", "Device B Switch Pref Hash: %u", id(light_device_b).get_preference_hash()); - ESP_LOGI("test", "Main Switch Pref Hash: %u", id(light_main).get_preference_hash()); - ESP_LOGI("test", "Device A Number Pref Hash: %u", id(setpoint_device_a).get_preference_hash()); - ESP_LOGI("test", "Device B Number Pref Hash: %u", id(setpoint_device_b).get_preference_hash()); - ESP_LOGI("test", "Main Number Pref Hash: %u", id(setpoint_main).get_preference_hash()); + // Log preference key bases for entities that actually store preferences. + // This is the key base make_entity_preference() uses: entity key XOR device id. + ESP_LOGI("test", "Device A Switch Pref Hash: %u", + id(light_device_a).get_entity_key() ^ id(light_device_a).get_device_id_or_zero()); + ESP_LOGI("test", "Device B Switch Pref Hash: %u", + id(light_device_b).get_entity_key() ^ id(light_device_b).get_device_id_or_zero()); + ESP_LOGI("test", "Main Switch Pref Hash: %u", + id(light_main).get_entity_key() ^ id(light_main).get_device_id_or_zero()); + ESP_LOGI("test", "Device A Number Pref Hash: %u", + id(setpoint_device_a).get_entity_key() ^ id(setpoint_device_a).get_device_id_or_zero()); + ESP_LOGI("test", "Device B Number Pref Hash: %u", + id(setpoint_device_b).get_entity_key() ^ id(setpoint_device_b).get_device_id_or_zero()); + ESP_LOGI("test", "Main Number Pref Hash: %u", + id(setpoint_main).get_entity_key() ^ id(setpoint_main).get_device_id_or_zero()); diff --git a/tests/integration/fixtures/preference_key_migration.yaml b/tests/integration/fixtures/preference_key_migration.yaml new file mode 100644 index 0000000000..a9b01fc2d2 --- /dev/null +++ b/tests/integration/fixtures/preference_key_migration.yaml @@ -0,0 +1,35 @@ +esphome: + name: host-pref-key-migration + +host: +api: +logger: + +switch: + - platform: template + id: test_switch_restore + name: Test Switch + optimistic: true + restore_mode: RESTORE_DEFAULT_OFF + +number: + - platform: template + id: test_number_restore + name: Test Number + optimistic: true + restore_value: true + initial_value: 1.0 + min_value: 0 + max_value: 100 + step: 0.5 + +text: + - platform: template + id: test_text_restore + name: Test Text + mode: text + optimistic: true + restore_value: true + initial_value: fallback + min_length: 0 + max_length: 20 diff --git a/tests/integration/host_prefs.py b/tests/integration/host_prefs.py index f835bee3bc..c7f21d8a01 100644 --- a/tests/integration/host_prefs.py +++ b/tests/integration/host_prefs.py @@ -25,15 +25,25 @@ def clear_host_prefs(device_name: str) -> None: host_prefs_path(device_name).unlink(missing_ok=True) +def write_host_prefs(device_name: str, entries: dict[int, bytes]) -> Path: + """Write preference entries, replacing the file's contents. + + Returns the path that was written. + """ + payload = b"" + for key, data in entries.items(): + if len(data) > 255: + raise ValueError(f"Preference data too long: {len(data)} bytes (max 255)") + payload += struct.pack(" Path: """Write a single preference entry, replacing the file's contents. Returns the path that was written. """ - if len(data) > 255: - raise ValueError(f"Preference data too long: {len(data)} bytes (max 255)") - path = host_prefs_path(device_name) - path.parent.mkdir(parents=True, exist_ok=True) - payload = struct.pack(" None: diff --git a/tests/integration/test_object_id_api_verification.py b/tests/integration/test_object_id_api_verification.py index c8603e0682..8dafb37c64 100644 --- a/tests/integration/test_object_id_api_verification.py +++ b/tests/integration/test_object_id_api_verification.py @@ -2,8 +2,8 @@ This test verifies a three-way match between: 1. C++ object_id generation (get_object_id_to using to_sanitized_char/to_snake_case_char) -2. C++ hash generation (fnv1_hash_object_id in helpers.h) -3. Python computation (sanitize/snake_case in helpers.py, fnv1_hash_object_id) +2. C++ entity key generation (fnv1_hash of the raw name in helpers.h) +3. Python computation (sanitize/snake_case and fnv1_hash_name in helpers.py) The API response contains C++ computed values, so verifying API == Python implicitly verifies C++ == Python == API for both object_id and hash. @@ -25,7 +25,7 @@ from __future__ import annotations import pytest -from esphome.helpers import fnv1_hash_object_id +from esphome.helpers import fnv1_hash_name from .entity_utils import compute_object_id, verify_all_entities from .types import APIClientConnectedFactory, RunCompiledFunction @@ -123,7 +123,7 @@ async def test_object_id_api_verification( ) # Verify hash can be computed from the name - hash_from_name = fnv1_hash_object_id(entity_name) + hash_from_name = fnv1_hash_name(entity_name) assert hash_from_name == entity.key, ( f"Entity '{entity_name}': hash mismatch. " f"Python hash {hash_from_name:#x}, API key {entity.key:#x}" @@ -164,7 +164,7 @@ async def test_object_id_api_verification( ) # Verify hash matches - expected_hash = fnv1_hash_object_id(expected_name) + expected_hash = fnv1_hash_name(expected_name) assert entity.key == expected_hash, ( f"Empty-name entity (device_id={entity.device_id}): hash mismatch. " f"API key: {entity.key:#x}, expected: {expected_hash:#x}" diff --git a/tests/integration/test_object_id_friendly_name_no_mac_suffix.py b/tests/integration/test_object_id_friendly_name_no_mac_suffix.py index 7199a2b371..b58593f2ef 100644 --- a/tests/integration/test_object_id_friendly_name_no_mac_suffix.py +++ b/tests/integration/test_object_id_friendly_name_no_mac_suffix.py @@ -11,7 +11,7 @@ from __future__ import annotations import pytest -from esphome.helpers import fnv1_hash_object_id +from esphome.helpers import fnv1_hash_name from .entity_utils import ( compute_object_id, @@ -62,7 +62,7 @@ async def test_object_id_friendly_name_no_mac_suffix( ) # Hash should match friendly_name - expected_hash = fnv1_hash_object_id("My Friendly Device") + expected_hash = fnv1_hash_name("My Friendly Device") assert entity.key == expected_hash, ( f"Expected hash {expected_hash:#x}, got {entity.key:#x}" ) diff --git a/tests/integration/test_object_id_no_friendly_name.py b/tests/integration/test_object_id_no_friendly_name.py index b548f02fde..45b5f730a6 100644 --- a/tests/integration/test_object_id_no_friendly_name.py +++ b/tests/integration/test_object_id_no_friendly_name.py @@ -17,7 +17,7 @@ from __future__ import annotations import pytest -from esphome.helpers import fnv1_hash_object_id +from esphome.helpers import fnv1_hash_name from .entity_utils import compute_object_id, verify_all_entities from .types import APIClientConnectedFactory, RunCompiledFunction @@ -96,7 +96,7 @@ async def test_object_id_no_friendly_name_no_mac_suffix( OLD behavior: - is_object_id_dynamic_() returned false (mac suffix not enabled) - Used object_id_c_str_ which was pre-computed in Python - - Python used get_base_entity_object_id() with fallback to CORE.name + - Python used get_base_entity_name() with fallback to CORE.name Result: object_id = sanitize(snake_case(device_name)) """ @@ -126,7 +126,7 @@ async def test_object_id_no_friendly_name_no_mac_suffix( ) # Hash should match device name - expected_hash = fnv1_hash_object_id("test-device") + expected_hash = fnv1_hash_name("test-device") assert entity.key == expected_hash, ( f"Expected hash {expected_hash:#x}, got {entity.key:#x}" ) diff --git a/tests/integration/test_preference_key_migration.py b/tests/integration/test_preference_key_migration.py new file mode 100644 index 0000000000..e7f699bb12 --- /dev/null +++ b/tests/integration/test_preference_key_migration.py @@ -0,0 +1,165 @@ +"""Integration test for entity preference key migration. + +Entity keys are now the FNV-1 hash of the raw name instead of the sanitized +object_id (https://github.com/esphome/backlog/issues/85). On key-lookup +preference backends, make_entity_preference() must move data stored under the +old key to the new key, so devices keep their restored state after upgrading. + +This test seeds the host preferences file the way a pre-migration firmware +would have written it and verifies: +1. Data stored under the OLD key is restored (migration happened, no data loss) +2. Data already stored under the NEW key is never overwritten by old data +""" + +from __future__ import annotations + +import socket +import struct + +from aioesphomeapi import ( + NumberInfo, + NumberState, + SwitchInfo, + SwitchState, + TextInfo, + TextState, +) +import pytest + +from esphome.helpers import fnv1_hash, fnv1_hash_name, fnv1_hash_object_id + +from .conftest import run_binary_and_wait_for_port, wait_and_connect_api_client +from .host_prefs import clear_host_prefs, write_host_prefs +from .state_utils import InitialStateHelper, require_entity +from .types import CompileFunction, ConfigWriter + +DEVICE_NAME = "host-pref-key-migration" + +# The pre-migration preference key was the sanitized object_id hash; the new +# key is the raw-name hash. All entities are on the main device (device_id 0) +# and their preferences use no version salt, so the key is just the hash. +SWITCH_OLD_KEY = fnv1_hash_object_id("Test Switch") +SWITCH_NEW_KEY = fnv1_hash_name("Test Switch") +NUMBER_OLD_KEY = fnv1_hash_object_id("Test Number") +NUMBER_NEW_KEY = fnv1_hash_name("Test Number") + +# template_text salts its key with the length limits and pattern hash; this must +# match TemplateText::setup() in template_text.cpp (min_length 0, max_length 20, +# no pattern configured) +TEXT_KEY_EXTRA = (0 << 2) + (20 << 4) + (fnv1_hash("") << 6) +TEXT_OLD_KEY = (fnv1_hash_object_id("Test Text") + TEXT_KEY_EXTRA) & 0xFFFFFFFF +TEXT_NEW_KEY = (fnv1_hash_name("Test Text") + TEXT_KEY_EXTRA) & 0xFFFFFFFF + +# TextSaver<20> stores a length-prefixed buffer of max_length + 1 bytes +TEXT_MAX_LENGTH = 20 + + +def text_pref_payload(value: str) -> bytes: + """Build the length-prefixed buffer TextSaver stores for a value.""" + data = value.encode("utf-8") + assert len(data) <= TEXT_MAX_LENGTH + return bytes([len(data)]) + data + b"\x00" * (TEXT_MAX_LENGTH - len(data)) + + +@pytest.mark.asyncio +async def test_preference_key_migration( + yaml_config: str, + write_yaml_config: ConfigWriter, + compile_esphome: CompileFunction, + reserved_tcp_port: tuple[int, socket.socket], +) -> None: + """Test that preferences stored under the old key survive the upgrade.""" + port, port_socket = reserved_tcp_port + + assert SWITCH_OLD_KEY != SWITCH_NEW_KEY + assert NUMBER_OLD_KEY != NUMBER_NEW_KEY + assert TEXT_OLD_KEY != TEXT_NEW_KEY + + # Write and compile once + config_path = await write_yaml_config(yaml_config) + binary_path = await compile_esphome(config_path) + + # Release the reserved port so the binary can bind to it + port_socket.close() + + async def boot_and_get_initial_states() -> tuple[ + SwitchState, NumberState, TextState + ]: + """Boot the binary and return the restored entity states.""" + async with ( + run_binary_and_wait_for_port(binary_path, "127.0.0.1", port), + wait_and_connect_api_client(port=port) as client, + ): + device_info = await client.device_info() + assert device_info.name == DEVICE_NAME + + entities, _ = await client.list_entities_services() + switch_entity = require_entity( + entities, "test_switch", SwitchInfo, "Test Switch" + ) + number_entity = require_entity( + entities, "test_number", NumberInfo, "Test Number" + ) + text_entity = require_entity(entities, "test_text", TextInfo, "Test Text") + + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states( + initial_state_helper.on_state_wrapper(lambda s: None) + ) + await initial_state_helper.wait_for_initial_states() + + switch_state = initial_state_helper.initial_states[switch_entity.key] + number_state = initial_state_helper.initial_states[number_entity.key] + text_state = initial_state_helper.initial_states[text_entity.key] + assert isinstance(switch_state, SwitchState) + assert isinstance(number_state, NumberState) + assert isinstance(text_state, TextState) + return switch_state, number_state, text_state + + try: + # --- Run 1: only OLD keys present, as written by pre-migration firmware. + # The restored states prove the data was migrated to the new keys. + write_host_prefs( + DEVICE_NAME, + { + SWITCH_OLD_KEY: b"\x01", # bool: switch was ON + NUMBER_OLD_KEY: struct.pack(" None: + """Verify _COMMAND_TOPIC_PLATFORMS matches the MQTT components that subscribe. + + Drift silently reintroduces shared subscribe topics, so this derives the set + from the C++ components that actually call subscribe(); that also catches + platforms like text that subscribe a command topic without exposing a + command_topic key in their schema. + """ + expected: set[str] = set() + for path in (COMPONENTS_DIR / "mqtt").glob("mqtt_*.cpp"): + if path.stem in _NON_ENTITY_MQTT_SOURCES: + continue + if "this->subscribe" not in path.read_text(encoding="utf-8"): + continue + stem = path.stem.removeprefix("mqtt_") + expected.add("datetime" if stem in _DATETIME_STEMS else stem) + assert expected == _COMMAND_TOPIC_PLATFORMS + + +def test_sub_topic_platforms_in_sync() -> None: + """Verify _SUB_TOPIC_PLATFORMS matches the MQTT components with sub-topics. + + Platforms whose MQTT headers use MQTT_COMPONENT_CUSTOM_TOPIC derive extra + topics such as position/command from the object_id. + """ + expected = { + path.stem.removeprefix("mqtt_") + for path in (COMPONENTS_DIR / "mqtt").glob("mqtt_*.h") + if path.stem != "mqtt_component" + and "MQTT_COMPONENT_CUSTOM_TOPIC" in path.read_text(encoding="utf-8") + } + assert expected == _SUB_TOPIC_PLATFORMS + + +def test_conflict_filter_exempts_custom_topics() -> None: + """Test that custom state topics with discovery off avoid the conflict.""" + validator = entity_duplicate_validator("sensor") + # Both entities have custom state topics and discovery disabled per entity, + # so no object_id-derived MQTT topic is used + validator( + { + CONF_NAME: "Датчик открытия", + CONF_STATE_TOPIC: "custom/topic/a", + CONF_DISCOVERY: False, + } + ) + validator( + { + CONF_NAME: "Датчик закрытия", + CONF_STATE_TOPIC: "custom/topic/b", + CONF_DISCOVERY: False, + } + ) + + component_validator = validate_no_object_id_conflicts( + REASON, conflict_filter=_topics_conflict + ) + config: dict = {CONF_DISCOVERY: True, CONF_TOPIC_PREFIX: "test-device"} + assert component_validator(config) is config + + # Without the filter the same conflicts are fatal + with pytest.raises(Invalid, match=r"mqtt builds default topics"): + validate_no_object_id_conflicts(REASON)({}) + + +def test_conflict_on_default_command_topic() -> None: + """Test that commandable platforms conflict through their default command topic. + + Custom state topics with discovery off are not enough for platforms that also + subscribe to an object_id-derived command topic. + """ + validator = entity_duplicate_validator("switch") + validator( + { + CONF_NAME: "Датчик открытия", + CONF_STATE_TOPIC: "custom/topic/a", + CONF_DISCOVERY: False, + } + ) + validator( + { + CONF_NAME: "Датчик закрытия", + CONF_STATE_TOPIC: "custom/topic/b", + CONF_DISCOVERY: False, + } + ) + + component_validator = validate_no_object_id_conflicts( + REASON, conflict_filter=_topics_conflict + ) + mqtt_config: dict = {CONF_DISCOVERY: True, CONF_TOPIC_PREFIX: "test-device"} + # Both switches share the default command topic: rejected + with pytest.raises(Invalid, match=r"mqtt builds default topics"): + component_validator(mqtt_config) + + # With custom command topics as well, nothing derives from the object_id + CORE.reset() + validator = entity_duplicate_validator("switch") + validator( + { + CONF_NAME: "Датчик открытия", + CONF_STATE_TOPIC: "custom/topic/a", + CONF_COMMAND_TOPIC: "custom/cmd/a", + CONF_DISCOVERY: False, + } + ) + validator( + { + CONF_NAME: "Датчик закрытия", + CONF_STATE_TOPIC: "custom/topic/b", + CONF_COMMAND_TOPIC: "custom/cmd/b", + CONF_DISCOVERY: False, + } + ) + assert component_validator(mqtt_config) is mqtt_config + + +def test_conflict_on_sub_topic_platforms() -> None: + """Test that platforms with extra object_id sub-topics always conflict. + + Covers derive topics like position/command from the object_id through their + own config keys, so custom state and command topics cannot exempt them. + """ + validator = entity_duplicate_validator("cover") + validator( + { + CONF_NAME: "Датчик открытия", + CONF_STATE_TOPIC: "custom/topic/a", + CONF_COMMAND_TOPIC: "custom/cmd/a", + CONF_DISCOVERY: False, + } + ) + validator( + { + CONF_NAME: "Датчик закрытия", + CONF_STATE_TOPIC: "custom/topic/b", + CONF_COMMAND_TOPIC: "custom/cmd/b", + CONF_DISCOVERY: False, + } + ) + + component_validator = validate_no_object_id_conflicts( + REASON, conflict_filter=_topics_conflict + ) + with pytest.raises(Invalid, match=r"mqtt builds default topics"): + component_validator({CONF_DISCOVERY: True, CONF_TOPIC_PREFIX: "test-device"}) + + +def test_no_conflict_on_disjoint_default_topics() -> None: + """Test that entities whose default topics are disjoint do not conflict. + + One entity uses only the default command topic and the other only the default + state topic, so they never share a topic. + """ + validator = entity_duplicate_validator("switch") + validator( + { + CONF_NAME: "Датчик открытия", + CONF_STATE_TOPIC: "custom/topic/a", + CONF_DISCOVERY: False, + } + ) + validator( + { + CONF_NAME: "Датчик закрытия", + CONF_COMMAND_TOPIC: "custom/cmd/b", + CONF_DISCOVERY: False, + } + ) + + component_validator = validate_no_object_id_conflicts( + REASON, conflict_filter=_topics_conflict + ) + config: dict = {CONF_DISCOVERY: True, CONF_TOPIC_PREFIX: "test-device"} + assert component_validator(config) is config + + +def test_no_conflict_on_empty_topic_prefix() -> None: + """Test that an empty topic_prefix disables the default topic conflict. + + With topic_prefix set to null no default topics exist at runtime, so entities + without custom state topics cannot conflict; only discovery still matters. + """ + validator = entity_duplicate_validator("sensor") + validator({CONF_NAME: "Датчик открытия"}) + validator({CONF_NAME: "Датчик закрытия"}) + + component_validator = validate_no_object_id_conflicts( + REASON, conflict_filter=_topics_conflict + ) + # No default topics and no discovery: valid + config: dict = {CONF_DISCOVERY: False, CONF_TOPIC_PREFIX: ""} + assert component_validator(config) is config + + # Discovery still uses object_id-derived config topics: rejected + with pytest.raises(Invalid, match=r"mqtt builds default topics"): + component_validator({CONF_DISCOVERY: True, CONF_TOPIC_PREFIX: ""}) diff --git a/tests/unit_tests/core/common.py b/tests/unit_tests/core/common.py index daa429dc96..96fcc5b1c6 100644 --- a/tests/unit_tests/core/common.py +++ b/tests/unit_tests/core/common.py @@ -29,5 +29,5 @@ def load_config_from_fixture( ) -> Config | None: """Load configuration from a fixture file.""" fixture_path = fixtures_dir / fixture_name - yaml_content = fixture_path.read_text() + yaml_content = fixture_path.read_text(encoding="utf-8") return load_config_from_yaml(yaml_file, yaml_content) diff --git a/tests/unit_tests/core/conftest.py b/tests/unit_tests/core/conftest.py index 42e59c15e6..9ef31a82b9 100644 --- a/tests/unit_tests/core/conftest.py +++ b/tests/unit_tests/core/conftest.py @@ -12,7 +12,7 @@ def yaml_file(tmp_path: Path) -> Callable[[str], Path]: def _yaml_file(content: str) -> Path: yaml_path = tmp_path / "test.yaml" - yaml_path.write_text(content) + yaml_path.write_text(content, encoding="utf-8") return yaml_path return _yaml_file diff --git a/tests/unit_tests/core/test_entity_helpers.py b/tests/unit_tests/core/test_entity_helpers.py index 3ac4ce27af..64400c4fd4 100644 --- a/tests/unit_tests/core/test_entity_helpers.py +++ b/tests/unit_tests/core/test_entity_helpers.py @@ -1,4 +1,4 @@ -"""Test get_base_entity_object_id function matches C++ behavior.""" +"""Tests for entity helpers: name selection, entity key hashing, duplicate checks.""" from collections.abc import Callable, Generator from pathlib import Path @@ -25,16 +25,17 @@ from esphome.core.entity_helpers import ( _setup_entity_impl, entity_duplicate_validator, finalize_entity_strings, - get_base_entity_object_id, + get_base_entity_name, register_device_class, register_icon, register_unit_of_measurement, setup_device_class, setup_entity, setup_unit_of_measurement, + validate_no_object_id_conflicts, ) from esphome.cpp_generator import MockObj -from esphome.helpers import sanitize, snake_case +from esphome.helpers import fnv1_hash_name, sanitize, snake_case from .common import load_config_from_fixture @@ -57,206 +58,26 @@ def restore_core_state() -> Generator[None, None, None]: CORE.friendly_name = original_friendly_name -def test_with_entity_name() -> None: - """Test when entity has its own name - should use entity name.""" - # Simple name - assert get_base_entity_object_id("Temperature Sensor", None) == "temperature_sensor" - assert ( - get_base_entity_object_id("Temperature Sensor", "Device Name") - == "temperature_sensor" - ) - # Even with device name, entity name takes precedence - assert ( - get_base_entity_object_id("Temperature Sensor", "Device Name", "Sub Device") - == "temperature_sensor" - ) - - # Name with special characters - assert ( - get_base_entity_object_id("Temp!@#$%^&*()Sensor", None) - == "temp__________sensor" - ) - assert get_base_entity_object_id("Temp-Sensor_123", None) == "temp-sensor_123" - - # Already snake_case - assert get_base_entity_object_id("temperature_sensor", None) == "temperature_sensor" - - # Mixed case - assert get_base_entity_object_id("TemperatureSensor", None) == "temperaturesensor" - assert get_base_entity_object_id("TEMPERATURE SENSOR", None) == "temperature_sensor" - - -def test_empty_name_with_device_name() -> None: - """Test when entity has empty name and is on a sub-device - should use device name.""" - # C++ behavior: when has_own_name is false and device is set, uses device->get_name() - assert ( - get_base_entity_object_id("", "Friendly Device", "Sub Device 1") - == "sub_device_1" - ) - assert ( - get_base_entity_object_id("", "Kitchen Controller", "controller_1") - == "controller_1" - ) - assert get_base_entity_object_id("", None, "Test-Device_123") == "test-device_123" - - -def test_empty_name_with_friendly_name() -> None: - """Test when entity has empty name and no device - should use friendly name.""" - # C++ behavior: when has_own_name is false, uses App.get_friendly_name() - assert get_base_entity_object_id("", "Friendly Device") == "friendly_device" - assert get_base_entity_object_id("", "Kitchen Controller") == "kitchen_controller" - assert get_base_entity_object_id("", "Test-Device_123") == "test-device_123" - - # Special characters in friendly name - assert get_base_entity_object_id("", "Device!@#$%") == "device_____" - - -def test_empty_name_no_friendly_name() -> None: - """Test when entity has empty name and no friendly name - should use device name.""" - # Test with CORE.name set - CORE.name = "device-name" - assert get_base_entity_object_id("", None) == "device-name" - - CORE.name = "Test Device" - assert get_base_entity_object_id("", None) == "test_device" - - -def test_edge_cases() -> None: - """Test edge cases.""" - # Only spaces - assert get_base_entity_object_id(" ", None) == "___" - - # Unicode characters (should be replaced) - assert get_base_entity_object_id("Température", None) == "temp_rature" - assert get_base_entity_object_id("测试", None) == "__" - - # Empty string with empty friendly name (empty friendly name is treated as None) - # Falls back to CORE.name - CORE.name = "device" - assert get_base_entity_object_id("", "") == "device" - - # Very long name (should work fine) - long_name = "a" * 100 + " " + "b" * 100 - expected = "a" * 100 + "_" + "b" * 100 - assert get_base_entity_object_id(long_name, None) == expected - - -@pytest.mark.parametrize( - ("name", "expected"), - [ - ("Temperature Sensor", "temperature_sensor"), - ("Living Room Light", "living_room_light"), - ("Test-Device_123", "test-device_123"), - ("Special!@#Chars", "special___chars"), - ("UPPERCASE NAME", "uppercase_name"), - ("lowercase name", "lowercase_name"), - ("Mixed Case Name", "mixed_case_name"), - (" Spaces ", "___spaces___"), - ], -) -def test_matches_cpp_helpers(name: str, expected: str) -> None: - """Test that the logic matches using snake_case and sanitize directly.""" - # For non-empty names, verify our function produces same result as direct snake_case + sanitize - assert get_base_entity_object_id(name, None) == sanitize(snake_case(name)) - assert get_base_entity_object_id(name, None) == expected - - -def test_empty_name_fallback() -> None: - """Test empty name handling which falls back to friendly_name or CORE.name.""" - # Empty name is handled specially - it doesn't just use sanitize(snake_case("")) - # Instead it falls back to friendly_name or CORE.name - assert sanitize(snake_case("")) == "" # Direct conversion gives empty string - # But our function returns a fallback - CORE.name = "device" - assert get_base_entity_object_id("", None) == "device" # Uses device name - - -def test_name_add_mac_suffix_behavior() -> None: - """Test behavior related to name_add_mac_suffix. - - In C++, an entity's object_id is computed from its name_ via - write_object_id_to() (sanitized snake_case). When an entity has no name, - configure_entity_() sets name_ from the friendly name, with the MAC suffix - appended when name_add_mac_suffix is enabled. Our function always returns - the same result since we're calculating the base for duplicate tracking. - """ - # The function should always return the same result regardless of - # name_add_mac_suffix setting, as we're calculating the base object_id - assert get_base_entity_object_id("", "Test Device") == "test_device" - assert get_base_entity_object_id("Entity Name", "Test Device") == "entity_name" - - -def test_priority_order() -> None: +def test_get_base_entity_name_priority_order() -> None: """Test the priority order: entity name > device name > friendly name > CORE.name.""" CORE.name = "core-device" - # 1. Entity name has highest priority + # 1. Entity name has highest priority and is used as-is, no transformations assert ( - get_base_entity_object_id("Entity Name", "Friendly Name", "Device Name") - == "entity_name" + get_base_entity_name("Entity Name", "Friendly Name", "Device Name") + == "Entity Name" ) + assert get_base_entity_name("Température", None) == "Température" # 2. Device name is next priority (when entity name is empty) - assert ( - get_base_entity_object_id("", "Friendly Name", "Device Name") == "device_name" - ) + assert get_base_entity_name("", "Friendly Name", "Device Name") == "Device Name" # 3. Friendly name is next (when entity and device names are empty) - assert get_base_entity_object_id("", "Friendly Name", None) == "friendly_name" + assert get_base_entity_name("", "Friendly Name", None) == "Friendly Name" - # 4. CORE.name is last resort - assert get_base_entity_object_id("", None, None) == "core-device" - - -@pytest.mark.parametrize( - ("name", "friendly_name", "device_name", "expected"), - [ - # name, friendly_name, device_name, expected - ("Living Room Light", None, None, "living_room_light"), - ("", "Kitchen Controller", None, "kitchen_controller"), - ( - "", - "ESP32 Device", - "controller_1", - "controller_1", - ), # Device name takes precedence - ("GPIO2 Button", None, None, "gpio2_button"), - ("WiFi Signal", "My Device", None, "wifi_signal"), - ("", None, "esp32_node", "esp32_node"), - ("Front Door Sensor", "Home Assistant", "door_controller", "front_door_sensor"), - ], -) -def test_real_world_examples( - name: str, friendly_name: str | None, device_name: str | None, expected: str -) -> None: - """Test real-world entity naming scenarios.""" - result = get_base_entity_object_id(name, friendly_name, device_name) - assert result == expected - - -def test_issue_6953_scenarios() -> None: - """Test specific scenarios from issue #6953.""" - # Scenario 1: Multiple empty names on main device with name_add_mac_suffix - # The Python code calculates the base, C++ might append MAC suffix dynamically - CORE.name = "device-name" - CORE.friendly_name = "Friendly Device" - - # All empty names should resolve to same base - assert get_base_entity_object_id("", CORE.friendly_name) == "friendly_device" - assert get_base_entity_object_id("", CORE.friendly_name) == "friendly_device" - assert get_base_entity_object_id("", CORE.friendly_name) == "friendly_device" - - # Scenario 2: Empty names on sub-devices - assert ( - get_base_entity_object_id("", "Main Device", "controller_1") == "controller_1" - ) - assert ( - get_base_entity_object_id("", "Main Device", "controller_2") == "controller_2" - ) - - # Scenario 3: xyz duplicates - assert get_base_entity_object_id("xyz", None) == "xyz" - assert get_base_entity_object_id("xyz", "Device") == "xyz" + # 4. CORE.name is last resort; an empty friendly name falls through to it + assert get_base_entity_name("", None, None) == "core-device" + assert get_base_entity_name("", "") == "core-device" # Tests for setup_entity function @@ -515,9 +336,10 @@ def test_entity_duplicate_validator() -> None: config1 = {CONF_NAME: "Temperature"} validated1 = validator(config1) assert validated1 == config1 - assert ("", "sensor", "temperature") in CORE.unique_ids + temperature_key = ("", "sensor", fnv1_hash_name("Temperature")) + assert temperature_key in CORE.unique_ids # Check metadata was stored - metadata = CORE.unique_ids[("", "sensor", "temperature")] + metadata = CORE.unique_ids[temperature_key] assert metadata["name"] == "Temperature" assert metadata["platform"] == "sensor" @@ -525,8 +347,9 @@ def test_entity_duplicate_validator() -> None: config2 = {CONF_NAME: "Humidity"} validated2 = validator(config2) assert validated2 == config2 - assert ("", "sensor", "humidity") in CORE.unique_ids - metadata2 = CORE.unique_ids[("", "sensor", "humidity")] + humidity_key = ("", "sensor", fnv1_hash_name("Humidity")) + assert humidity_key in CORE.unique_ids + metadata2 = CORE.unique_ids[humidity_key] assert metadata2["name"] == "Humidity" # Duplicate entity should fail @@ -547,18 +370,19 @@ def test_entity_duplicate_validator_with_devices() -> None: device2 = ID("device2", type="Device") # Same name on different devices should pass + name_hash = fnv1_hash_name("Temperature") config1 = {CONF_NAME: "Temperature", CONF_DEVICE_ID: device1} validated1 = validator(config1) assert validated1 == config1 - assert ("device1", "sensor", "temperature") in CORE.unique_ids - metadata1 = CORE.unique_ids[("device1", "sensor", "temperature")] + assert ("device1", "sensor", name_hash) in CORE.unique_ids + metadata1 = CORE.unique_ids[("device1", "sensor", name_hash)] assert metadata1["device_id"] == "device1" config2 = {CONF_NAME: "Temperature", CONF_DEVICE_ID: device2} validated2 = validator(config2) assert validated2 == config2 - assert ("device2", "sensor", "temperature") in CORE.unique_ids - metadata2 = CORE.unique_ids[("device2", "sensor", "temperature")] + assert ("device2", "sensor", name_hash) in CORE.unique_ids + metadata2 = CORE.unique_ids[("device2", "sensor", name_hash)] assert metadata2["device_id"] == "device2" # Duplicate on same device should fail @@ -610,6 +434,33 @@ def test_entity_different_platforms_yaml_validation( assert result is not None +def test_object_id_conflict_mqtt_yaml_validation( + yaml_file: Callable[[str], str], capsys: pytest.CaptureFixture[str] +) -> None: + """Test that names sanitizing to the same object_id fail when mqtt is configured.""" + result = load_config_from_fixture( + yaml_file, "object_id_conflict_mqtt.yaml", FIXTURES_DIR + ) + assert result is None + + captured = capsys.readouterr() + assert ( + "mqtt builds default topics and discovery topics from the entity object_id" + in captured.out + ) + + +def test_object_id_conflict_without_mqtt_yaml_validation( + yaml_file: Callable[[str], str], +) -> None: + """Test that names sanitizing to the same object_id pass without mqtt/prometheus.""" + result = load_config_from_fixture( + yaml_file, "object_id_conflict_no_mqtt.yaml", FIXTURES_DIR + ) + # This should succeed + assert result is not None + + def test_entity_duplicate_validator_error_message() -> None: """Test that duplicate entity error messages include helpful metadata.""" # Create validator for sensor platform @@ -668,7 +519,8 @@ def test_entity_duplicate_validator_internal_entities() -> None: validated1 = validator(config1) assert validated1 == config1 # New format includes device_id (empty string for main device) - assert ("", "sensor", "temperature") in CORE.unique_ids + temperature_key = ("", "sensor", fnv1_hash_name("Temperature")) + assert temperature_key in CORE.unique_ids # Internal entity with same name should pass (not added to unique_ids) config2 = {CONF_NAME: "Temperature", CONF_INTERNAL: True} @@ -676,7 +528,7 @@ def test_entity_duplicate_validator_internal_entities() -> None: assert validated2 == config2 # Internal entity should not be added to unique_ids # Count how many times the key appears (should still be 1) - count = sum(1 for k in CORE.unique_ids if k == ("", "sensor", "temperature")) + count = sum(1 for k in CORE.unique_ids if k == temperature_key) assert count == 1 # Another internal entity with same name should also pass @@ -684,7 +536,7 @@ def test_entity_duplicate_validator_internal_entities() -> None: validated3 = validator(config3) assert validated3 == config3 # Still only one entry in unique_ids (from the non-internal entity) - count = sum(1 for k in CORE.unique_ids if k == ("", "sensor", "temperature")) + count = sum(1 for k in CORE.unique_ids if k == temperature_key) assert count == 1 # Non-internal entity with same name should fail @@ -712,30 +564,148 @@ def test_empty_or_null_device_id_on_entity() -> None: def test_entity_duplicate_validator_non_ascii_names() -> None: - """Test that non-ASCII names show helpful error messages.""" + """Test that distinct non-ASCII names no longer collide. + + These names used to be rejected because both sanitize to only underscores; + the entity key now hashes the raw name so they stay distinct. + """ # Create validator for binary_sensor platform validator = entity_duplicate_validator("binary_sensor") - # First Russian sensor should pass + # Both Russian sensors should pass even though they sanitize identically config1 = {CONF_NAME: "Датчик открытия основного крана"} validated1 = validator(config1) assert validated1 == config1 - # Second Russian sensor with different text but same ASCII conversion should fail config2 = {CONF_NAME: "Датчик закрытия основного крана"} + validated2 = validator(config2) + assert validated2 == config2 + + # An exact duplicate still fails + config3 = {CONF_NAME: "Датчик открытия основного крана"} + with pytest.raises( + Invalid, + match=r"Duplicate binary_sensor entity with name 'Датчик открытия основного крана' found", + ): + validator(config3) + + +def test_entity_duplicate_validator_hash_collision() -> None: + """Test that two different names with the same FNV-1 hash are rejected.""" + # Brute-forced FNV-1 32-bit collision pair; both hash to 0x0ee5ff7b + name_a = "Sensor m2CZ" + name_b = "Sensor qCaa" + assert name_a != name_b + assert fnv1_hash_name(name_a) == fnv1_hash_name(name_b) + + validator = entity_duplicate_validator("sensor") + + config1 = {CONF_NAME: name_a} + validated1 = validator(config1) + assert validated1 == config1 + + config2 = {CONF_NAME: name_b} with pytest.raises( Invalid, match=re.compile( - r"Duplicate binary_sensor entity with name 'Датчик закрытия основного крана' found.*" - r"Original names: 'Датчик закрытия основного крана' and 'Датчик открытия основного крана'.*" - r"Both convert to ASCII ID: '_______________________________'.*" - r"To fix: Add unique ASCII characters \(e\.g\., '1', '2', or 'A', 'B'\)", + rf"Duplicate sensor entity with name '{name_b}' found.*" + rf"The names '{name_b}' and '{name_a}' produce the.*" + r"same entity key hash \(0x0ee5ff7b\).*" + r"To fix: Rename one of the entities", re.DOTALL, ), ): validator(config2) +def test_object_id_conflicts_rejected_by_component_validator() -> None: + """Test that object_id conflicts pass entity validation but fail for mqtt/prometheus.""" + validator = entity_duplicate_validator("sensor") + + # Both names validate fine in general (distinct raw names, distinct keys) + validator({CONF_NAME: "Датчик открытия"}) + validator({CONF_NAME: "Датчик закрытия"}) + + # A component that addresses entities by object_id must reject the config + component_validator = validate_no_object_id_conflicts( + "mqtt builds default topics from the entity object_id" + ) + with pytest.raises( + Invalid, + match=re.compile( + r"mqtt builds default topics from the entity object_id.*" + r"sensor entities 'Датчик открытия', 'Датчик закрытия' " + r"share the object_id '_______________'.*" + r"To fix: Add unique ASCII characters", + re.DOTALL, + ), + ): + component_validator({}) + + +def test_object_id_conflicts_skipped_in_testing_mode() -> None: + """Test that testing_mode skips the conflict check, as used for grouped testing.""" + validator = entity_duplicate_validator("sensor") + validator({CONF_NAME: "Датчик открытия"}) + validator({CONF_NAME: "Датчик закрытия"}) + + component_validator = validate_no_object_id_conflicts( + "mqtt builds default topics from the entity object_id" + ) + CORE.testing_mode = True + try: + config: dict = {} + assert component_validator(config) is config + finally: + CORE.testing_mode = False + + +def test_object_id_conflicts_none_recorded() -> None: + """Test that distinct object_ids produce no conflicts.""" + validator = entity_duplicate_validator("sensor") + validator({CONF_NAME: "Temperature"}) + validator({CONF_NAME: "Humidity"}) + + component_validator = validate_no_object_id_conflicts( + "mqtt builds default topics from the entity object_id" + ) + config: dict = {} + assert component_validator(config) is config + + +def test_object_id_conflicts_device_scoped() -> None: + """Test that the object_id conflict check is scoped per device. + + Same-named entities on different sub-devices were accepted before entity keys + moved to raw names, so the check keeps that scope; conflicts within one device + are still reported with the device named in the message. + """ + validator = entity_duplicate_validator("sensor") + validator({CONF_NAME: "Temperature", CONF_DEVICE_ID: ID("device1", type="Device")}) + validator({CONF_NAME: "Temperature", CONF_DEVICE_ID: ID("device2", type="Device")}) + + component_validator = validate_no_object_id_conflicts( + "prometheus builds metric labels from the entity object_id" + ) + config: dict = {} + assert component_validator(config) is config + + # Two names sanitizing identically on the same sub-device still conflict + validator( + {CONF_NAME: "Датчик открытия", CONF_DEVICE_ID: ID("device1", type="Device")} + ) + validator( + {CONF_NAME: "Датчик закрытия", CONF_DEVICE_ID: ID("device1", type="Device")} + ) + with pytest.raises( + Invalid, + match=re.compile( + r"prometheus builds metric labels.*on device 'device1'", re.DOTALL + ), + ): + component_validator({}) + + def test_entity_duplicate_validator_same_name_no_enhanced_message() -> None: """Test that identical names don't show the enhanced message.""" # Create validator for sensor platform @@ -793,7 +763,7 @@ async def test_setup_entity_empty_name_with_device( # For empty-name entities, Python stores hash 0 - C++ calculates hash at runtime assert config.get("_entity_name") == "" - assert config.get("_entity_object_id_hash") == 0 + assert config.get("_entity_key") == 0 @pytest.mark.asyncio @@ -822,7 +792,7 @@ async def test_setup_entity_empty_name_with_mac_suffix( # For empty-name entities, Python stores hash 0 - C++ calculates hash at runtime assert config.get("_entity_name") == "" - assert config.get("_entity_object_id_hash") == 0 + assert config.get("_entity_key") == 0 @pytest.mark.asyncio @@ -852,7 +822,7 @@ async def test_setup_entity_empty_name_with_mac_suffix_no_friendly_name( # For empty-name entities, Python stores hash 0 - C++ calculates hash at runtime assert config.get("_entity_name") == "" - assert config.get("_entity_object_id_hash") == 0 + assert config.get("_entity_key") == 0 @pytest.mark.asyncio @@ -883,7 +853,7 @@ async def test_setup_entity_empty_name_no_mac_suffix_no_friendly_name( # For empty-name entities, Python stores hash 0 - C++ calculates hash at runtime assert config.get("_entity_name") == "" - assert config.get("_entity_object_id_hash") == 0 + assert config.get("_entity_key") == 0 def test_register_string_overflow() -> None: diff --git a/tests/unit_tests/fixtures/core/entity_helpers/object_id_conflict_mqtt.yaml b/tests/unit_tests/fixtures/core/entity_helpers/object_id_conflict_mqtt.yaml new file mode 100644 index 0000000000..4a6f56f473 --- /dev/null +++ b/tests/unit_tests/fixtures/core/entity_helpers/object_id_conflict_mqtt.yaml @@ -0,0 +1,22 @@ +esphome: + name: test-object-id-conflict + +esp32: + board: esp32dev + +wifi: + ssid: MySSID + password: password1 + +mqtt: + broker: test.mosquitto.org + +sensor: + # Distinct raw names are fine in general, but both sanitize to the same + # object_id, which MQTT still uses to build default topics - should fail + - platform: template + name: "Датчик открытия" + lambda: return 21.0; + - platform: template + name: "Датчик закрытия" + lambda: return 22.0; diff --git a/tests/unit_tests/fixtures/core/entity_helpers/object_id_conflict_no_mqtt.yaml b/tests/unit_tests/fixtures/core/entity_helpers/object_id_conflict_no_mqtt.yaml new file mode 100644 index 0000000000..c0fbd5cbba --- /dev/null +++ b/tests/unit_tests/fixtures/core/entity_helpers/object_id_conflict_no_mqtt.yaml @@ -0,0 +1,15 @@ +esphome: + name: test-object-id-ok + +esp32: + board: esp32dev + +sensor: + # Distinct raw names that sanitize to the same object_id are allowed when no + # component addresses entities by object_id (no mqtt or prometheus configured) + - platform: template + name: "Датчик открытия" + lambda: return 21.0; + - platform: template + name: "Датчик закрытия" + lambda: return 22.0; diff --git a/tests/unit_tests/test_preference_hash_stability.py b/tests/unit_tests/test_preference_hash_stability.py new file mode 100644 index 0000000000..d3e5fac36a --- /dev/null +++ b/tests/unit_tests/test_preference_hash_stability.py @@ -0,0 +1,239 @@ +"""Tests to verify preference and entity key hash values remain stable. + +These tests ensure the hash algorithms do NOT change, as any change would cause +users to lose stored preferences (calibration values, restore states, etc.) on +firmware upgrades, or break entity state routing to API clients. + +Two algorithms are locked here (see https://github.com/esphome/backlog/issues/85): +1. `fnv1_hash_object_id(name)` - the LEGACY hash (snake_case + sanitize, then FNV-1). + Existing devices have preferences stored under keys derived from it; slot-based + backends (ESP8266, RP2040) keep using it, and key-lookup backends migrate FROM it. +2. `fnv1_hash_name(name)` - the entity key (FNV-1 over the raw UTF-8 name bytes). + Sent to API clients and used as the preference key base on key-lookup backends. + +DO NOT CHANGE THE EXPECTED VALUES - if tests fail after modifying a hash algorithm, +the change breaks backward compatibility and will cause data loss. +""" + +import pytest + +from esphome.helpers import ( + FNV1_OFFSET_BASIS, + FNV1_PRIME, + fnv1_hash_name, + fnv1_hash_object_id, +) + +# ============================================================================= +# Test: fnv1_hash_object_id produces stable hashes for entity names +# ============================================================================= + + +@pytest.mark.parametrize( + ("entity_name", "expected_object_id_hash"), + [ + # ===================================================================== + # Core entity types - these names appear in many ESPHome configurations + # ===================================================================== + # Basic single-word names + ("Light", 0x735CF023), + ("Switch", 0xBEDF78E5), + ("Sensor", 0x75E61B1B), + ("Fan", 0x468F6780), + ("Climate", 0xAA22FD4A), + ("Cover", 0xA630D0A2), + ("Lock", 0x1D2FD708), + ("Valve", 0x25ED5F65), + ("Button", 0x3A42C455), + ("Number", 0xB900E22A), + ("Select", 0x556391B5), + ("Text", 0xB12BFA38), + # Multi-word names (spaces become underscores, lowercase) + ("Living Room Light", 0xC6F81EC9), + ("Kitchen Switch", 0xC63C0F6E), + ("Temperature Sensor", 0x16AF55B6), + ("Garage Door Cover", 0x685E5281), + ("Bedroom Fan", 0x21AB1DED), + ("Front Door Lock", 0xB9BEF8E1), + # Already snake_case names (should hash same as space-separated) + ("living_room_light", 0xC6F81EC9), # Same as "Living Room Light" + ("kitchen_switch", 0xC63C0F6E), # Same as "Kitchen Switch" + # Names with numbers + ("Sensor 1", 0x99828E4B), + ("Relay 2", 0x6FFEF2FB), + ("Zone 10", 0xFD83AA95), + # Names with special characters (become underscores) + ("AC Unit", 0x336C6886), + ("WiFi Signal", 0x2FA52175), + ("CO2 Level", 0x31049870), + # Mixed case handling + ("mySwitch", 0x9AA10553), + ("MySwitch", 0x9AA10553), # Same as lowercase + ("MYSWITCH", 0x9AA10553), # Same as lowercase + # ===================================================================== + # Edge cases + # ===================================================================== + # Empty name (hashes to the FNV-1 offset basis since no chars processed) + ("", 0x811C9DC5), + # Single character + ("a", 0x050C5D7E), + ("A", 0x050C5D7E), # Same after lowercase + ("1", 0x050C5D2E), + ("_", 0x050C5D40), + # Names that differ only in case (should hash identically) + ("test", 0xBC2C0BE9), + ("Test", 0xBC2C0BE9), + ("TEST", 0xBC2C0BE9), + # Names that differ only in spaces vs underscores (should hash identically) + ("foo bar", 0x3AE35AA1), + ("foo_bar", 0x3AE35AA1), + ("Foo Bar", 0x3AE35AA1), + ("FOO_BAR", 0x3AE35AA1), + # Non-ASCII names (sanitized per code point, one underscore per character) + ("äöü", 0x10028B12), + ("温度", 0x3276CB9F), + ("Température", 0x965698F3), + # ===================================================================== + # Real-world component entity names from ESPHome codebase + # ===================================================================== + # From fan.cpp - FanRestoreState + ("Ceiling Fan", 0x640DEF00), + # From climate.cpp - ClimateRestoreState + ("HVAC", 0xDD68438B), + ("Thermostat", 0x30A5B7C6), + # From light/light_state.cpp + ("LED Strip", 0x2A068423), + ("Dimmable Light", 0xD70393F3), + # From cover/cover.cpp + ("Garage Door", 0x53987A5D), + ("Window Blind", 0x851291A5), + # From switch/switch.cpp + ("Relay", 0xD3A92FE4), + ("Power Switch", 0x5C4A47B3), + # From number/automation.cpp + ("Brightness", 0xF46E252C), + ("Volume", 0x8FFEBE43), + # From template datetime entities + ("Wake Time", 0xEE612B53), + ("Schedule Date", 0xF538C8DD), + ], +) +def test_entity_object_id_hash_stability( + entity_name: str, expected_object_id_hash: int +) -> None: + """Verify fnv1_hash_object_id produces stable hashes for entity names. + + CRITICAL: These expected values MUST NOT CHANGE. Existing devices have + preferences stored under keys derived from this legacy hash; changing it + breaks the old-to-new key migration and loses stored preferences. + """ + actual = fnv1_hash_object_id(entity_name) + assert actual == expected_object_id_hash, ( + f"Hash for '{entity_name}' changed from {expected_object_id_hash:#010x} to {actual:#010x}. " + f"This will cause users to lose stored preferences!" + ) + + +# ============================================================================= +# Test: Legacy preference key computation formula +# ============================================================================= + + +def compute_legacy_preference_key( + entity_name: str, version: int = 0, device_id: int = 0 +) -> int: + """Compute the legacy preference key: (object_id_hash ^ device_id) ^ version. + + This is the key existing devices have data stored under. Slot-based backends + (ESP8266, RP2040) still use it directly; key-lookup backends compute it as the + migration source in EntityBase::make_entity_preference_() (entity_base.cpp). + """ + object_id_hash = fnv1_hash_object_id(entity_name) + preference_hash = object_id_hash ^ device_id + key = preference_hash ^ version + return key & 0xFFFFFFFF + + +# Restore state version constants from ESPHome components +# These MUST match the RESTORE_STATE_VERSION values in the C++ code +FAN_RESTORE_STATE_VERSION = 0x71700ABA # From fan/fan.cpp +CLIMATE_RESTORE_STATE_VERSION = 0x848EA6AD # From climate/climate.cpp + + +@pytest.mark.parametrize( + ("entity_name", "version", "device_id", "expected_key"), + [ + # No version, main device (key equals the plain object_id hash) + ("Test Sensor", 0, 0, 0x5D74FA46), + ("Light", 0, 0, 0x735CF023), + # Restore state versions on the main device + ("Ceiling Fan", FAN_RESTORE_STATE_VERSION, 0, 0x157DE5BA), + ("HVAC", CLIMATE_RESTORE_STATE_VERSION, 0, 0x59E6E526), + # Sub-devices: same entity name on different devices gets different keys + ("Light", 0, 1, 0x735CF022), + ("Fan", FAN_RESTORE_STATE_VERSION, 0xABCD, 0x37FFC6F7), + ], +) +def test_legacy_preference_key_computation( + entity_name: str, version: int, device_id: int, expected_key: int +) -> None: + """Verify legacy preference key computation matches expected values. + + This test ensures the formula doesn't change, which would break both slot-based + preference storage and the migration source keys on key-lookup backends. + """ + actual_key = compute_legacy_preference_key(entity_name, version, device_id) + + assert actual_key == expected_key, ( + f"Preference key for '{entity_name}' (version={version:#x}, device_id={device_id}) " + f"changed from {expected_key:#010x} to {actual_key:#010x}. " + f"This will cause users to lose stored preferences!" + ) + + +# ============================================================================= +# Test: fnv1_hash_name produces stable entity keys (raw name, UTF-8 bytes) +# ============================================================================= + + +@pytest.mark.parametrize( + ("entity_name", "expected_key"), + [ + # ASCII names + ("Temperature Sensor", 0x801C3665), + ("LED Strip", 0xD5C7B082), + ("Garage Door", 0x2D70E086), + ("Relay", 0x565177C4), + # Raw names are case and space sensitive, unlike the old object_id hash + ("temperature sensor", 0xF9F431E5), + # Non-ASCII names hash their UTF-8 bytes and stay distinct + ("Датчик открытия", 0x001861C1), + ("温度", 0x8EDF61C9), + ("Température", 0x531A74AA), + # Empty name hashes to the FNV-1 offset basis + ("", 0x811C9DC5), + ], +) +def test_entity_key_hash_stability(entity_name: str, expected_key: int) -> None: + """Verify fnv1_hash_name produces stable entity keys. + + CRITICAL: These expected values MUST NOT CHANGE. The entity key is sent to + API clients and is the new preference key base; changing the algorithm + would break state routing and lose stored preferences. + Must match C++ fnv1_hash_bytes() in esphome/core/helpers.h. + """ + actual = fnv1_hash_name(entity_name) + assert actual == expected_key, ( + f"Entity key for '{entity_name}' changed from {expected_key:#010x} to {actual:#010x}. " + f"This breaks state routing and stored preferences!" + ) + + +def test_fnv1_hash_name_matches_utf8_byte_hash() -> None: + """Verify fnv1_hash_name hashes the UTF-8 encoded bytes of the name.""" + name = "Température 温度" + hash_value = FNV1_OFFSET_BASIS + for byte in name.encode("utf-8"): + hash_value = (hash_value * FNV1_PRIME) & 0xFFFFFFFF + hash_value ^= byte + assert fnv1_hash_name(name) == hash_value