From e370872ec1baeb48b864daee44a6a00f7aee0e23 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 24 Jun 2025 16:13:34 +0200 Subject: [PATCH] fix conflicts --- .../alarm_control_panel/__init__.py | 2 +- esphome/components/binary_sensor/__init__.py | 2 +- esphome/components/button/__init__.py | 2 +- esphome/components/climate/__init__.py | 2 +- esphome/components/cover/__init__.py | 2 +- esphome/components/datetime/__init__.py | 2 +- esphome/components/esp32_camera/__init__.py | 2 +- esphome/components/event/__init__.py | 2 +- esphome/components/fan/__init__.py | 2 +- esphome/components/light/__init__.py | 2 +- esphome/components/lock/__init__.py | 2 +- esphome/components/media_player/__init__.py | 2 +- esphome/components/number/__init__.py | 2 +- esphome/components/select/__init__.py | 2 +- esphome/components/sensor/__init__.py | 2 +- esphome/components/switch/__init__.py | 2 +- esphome/components/text/__init__.py | 2 +- esphome/components/text_sensor/__init__.py | 2 +- esphome/components/update/__init__.py | 2 +- esphome/components/valve/__init__.py | 2 +- esphome/core/__init__.py | 4 + esphome/core/entity_base.cpp | 12 +- esphome/cpp_helpers.py | 61 ++++++++- tests/unit_tests/conftest.py | 9 ++ tests/unit_tests/test_duplicate_entities.py | 129 ++++++++++++++++++ 25 files changed, 219 insertions(+), 36 deletions(-) create mode 100644 tests/unit_tests/test_duplicate_entities.py diff --git a/esphome/components/alarm_control_panel/__init__.py b/esphome/components/alarm_control_panel/__init__.py index e88050132a..3c35076de9 100644 --- a/esphome/components/alarm_control_panel/__init__.py +++ b/esphome/components/alarm_control_panel/__init__.py @@ -190,7 +190,7 @@ ALARM_CONTROL_PANEL_CONDITION_SCHEMA = maybe_simple_id( async def setup_alarm_control_panel_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "alarm_control_panel") for conf in config.get(CONF_ON_STATE, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) await automation.build_automation(trigger, [], conf) diff --git a/esphome/components/binary_sensor/__init__.py b/esphome/components/binary_sensor/__init__.py index bc26c09622..b34477d30a 100644 --- a/esphome/components/binary_sensor/__init__.py +++ b/esphome/components/binary_sensor/__init__.py @@ -521,7 +521,7 @@ BINARY_SENSOR_SCHEMA.add_extra(cv.deprecated_schema_constant("binary_sensor")) async def setup_binary_sensor_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "binary_sensor") if (device_class := config.get(CONF_DEVICE_CLASS)) is not None: cg.add(var.set_device_class(device_class)) diff --git a/esphome/components/button/__init__.py b/esphome/components/button/__init__.py index 892bf62f3a..c63073dd38 100644 --- a/esphome/components/button/__init__.py +++ b/esphome/components/button/__init__.py @@ -87,7 +87,7 @@ BUTTON_SCHEMA.add_extra(cv.deprecated_schema_constant("button")) async def setup_button_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "button") for conf in config.get(CONF_ON_PRESS, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) diff --git a/esphome/components/climate/__init__.py b/esphome/components/climate/__init__.py index 52938a17d0..ff00565abf 100644 --- a/esphome/components/climate/__init__.py +++ b/esphome/components/climate/__init__.py @@ -273,7 +273,7 @@ CLIMATE_SCHEMA.add_extra(cv.deprecated_schema_constant("climate")) async def setup_climate_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "climate") visual = config[CONF_VISUAL] if (min_temp := visual.get(CONF_MIN_TEMPERATURE)) is not None: diff --git a/esphome/components/cover/__init__.py b/esphome/components/cover/__init__.py index 9fe7593eab..c7aec6493b 100644 --- a/esphome/components/cover/__init__.py +++ b/esphome/components/cover/__init__.py @@ -154,7 +154,7 @@ COVER_SCHEMA.add_extra(cv.deprecated_schema_constant("cover")) async def setup_cover_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "cover") if (device_class := config.get(CONF_DEVICE_CLASS)) is not None: cg.add(var.set_device_class(device_class)) diff --git a/esphome/components/datetime/__init__.py b/esphome/components/datetime/__init__.py index 24fbf5a1ec..42b29227c3 100644 --- a/esphome/components/datetime/__init__.py +++ b/esphome/components/datetime/__init__.py @@ -133,7 +133,7 @@ def datetime_schema(class_: MockObjClass) -> cv.Schema: async def setup_datetime_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "datetime") if (mqtt_id := config.get(CONF_MQTT_ID)) is not None: mqtt_ = cg.new_Pvariable(mqtt_id, var) diff --git a/esphome/components/esp32_camera/__init__.py b/esphome/components/esp32_camera/__init__.py index 05522265ae..68ba1ae549 100644 --- a/esphome/components/esp32_camera/__init__.py +++ b/esphome/components/esp32_camera/__init__.py @@ -284,7 +284,7 @@ SETTERS = { async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) - await setup_entity(var, config) + await setup_entity(var, config, "camera") await cg.register_component(var, config) for key, setter in SETTERS.items(): diff --git a/esphome/components/event/__init__.py b/esphome/components/event/__init__.py index e7ab489a25..1ff0d4e3d5 100644 --- a/esphome/components/event/__init__.py +++ b/esphome/components/event/__init__.py @@ -88,7 +88,7 @@ EVENT_SCHEMA.add_extra(cv.deprecated_schema_constant("event")) async def setup_event_core_(var, config, *, event_types: list[str]): - await setup_entity(var, config) + await setup_entity(var, config, "event") for conf in config.get(CONF_ON_EVENT, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) diff --git a/esphome/components/fan/__init__.py b/esphome/components/fan/__init__.py index c6ff938cd6..bebf760b0b 100644 --- a/esphome/components/fan/__init__.py +++ b/esphome/components/fan/__init__.py @@ -225,7 +225,7 @@ def validate_preset_modes(value): async def setup_fan_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "fan") cg.add(var.set_restore_mode(config[CONF_RESTORE_MODE])) diff --git a/esphome/components/light/__init__.py b/esphome/components/light/__init__.py index a013029fc2..902d661eb5 100644 --- a/esphome/components/light/__init__.py +++ b/esphome/components/light/__init__.py @@ -207,7 +207,7 @@ def validate_color_temperature_channels(value): async def setup_light_core_(light_var, output_var, config): - await setup_entity(light_var, config) + await setup_entity(light_var, config, "light") cg.add(light_var.set_restore_mode(config[CONF_RESTORE_MODE])) diff --git a/esphome/components/lock/__init__.py b/esphome/components/lock/__init__.py index 0fb67e3948..aa1061de53 100644 --- a/esphome/components/lock/__init__.py +++ b/esphome/components/lock/__init__.py @@ -94,7 +94,7 @@ LOCK_SCHEMA.add_extra(cv.deprecated_schema_constant("lock")) async def _setup_lock_core(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "lock") for conf in config.get(CONF_ON_LOCK, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) diff --git a/esphome/components/media_player/__init__.py b/esphome/components/media_player/__init__.py index ef76419de3..c01bd24890 100644 --- a/esphome/components/media_player/__init__.py +++ b/esphome/components/media_player/__init__.py @@ -81,7 +81,7 @@ IsAnnouncingCondition = media_player_ns.class_( async def setup_media_player_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "media_player") for conf in config.get(CONF_ON_STATE, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) await automation.build_automation(trigger, [], conf) diff --git a/esphome/components/number/__init__.py b/esphome/components/number/__init__.py index 2567d9ffe1..65a00bfe2f 100644 --- a/esphome/components/number/__init__.py +++ b/esphome/components/number/__init__.py @@ -237,7 +237,7 @@ NUMBER_SCHEMA.add_extra(cv.deprecated_schema_constant("number")) async def setup_number_core_( var, config, *, min_value: float, max_value: float, step: float ): - await setup_entity(var, config) + await setup_entity(var, config, "number") cg.add(var.traits.set_min_value(min_value)) cg.add(var.traits.set_max_value(max_value)) diff --git a/esphome/components/select/__init__.py b/esphome/components/select/__init__.py index e14a9351a0..c3f8abec8f 100644 --- a/esphome/components/select/__init__.py +++ b/esphome/components/select/__init__.py @@ -89,7 +89,7 @@ SELECT_SCHEMA.add_extra(cv.deprecated_schema_constant("select")) async def setup_select_core_(var, config, *, options: list[str]): - await setup_entity(var, config) + await setup_entity(var, config, "select") cg.add(var.traits.set_options(options)) diff --git a/esphome/components/sensor/__init__.py b/esphome/components/sensor/__init__.py index 1ad3cfabee..749b7992b8 100644 --- a/esphome/components/sensor/__init__.py +++ b/esphome/components/sensor/__init__.py @@ -787,7 +787,7 @@ async def build_filters(config): async def setup_sensor_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "sensor") if (device_class := config.get(CONF_DEVICE_CLASS)) is not None: cg.add(var.set_device_class(device_class)) diff --git a/esphome/components/switch/__init__.py b/esphome/components/switch/__init__.py index 0211c648fc..322d547e95 100644 --- a/esphome/components/switch/__init__.py +++ b/esphome/components/switch/__init__.py @@ -131,7 +131,7 @@ SWITCH_SCHEMA.add_extra(cv.deprecated_schema_constant("switch")) async def setup_switch_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "switch") if (inverted := config.get(CONF_INVERTED)) is not None: cg.add(var.set_inverted(inverted)) diff --git a/esphome/components/text/__init__.py b/esphome/components/text/__init__.py index 40b3a90d6b..fc1b3d1b05 100644 --- a/esphome/components/text/__init__.py +++ b/esphome/components/text/__init__.py @@ -94,7 +94,7 @@ async def setup_text_core_( max_length: int | None, pattern: str | None, ): - await setup_entity(var, config) + await setup_entity(var, config, "text") cg.add(var.traits.set_min_length(min_length)) cg.add(var.traits.set_max_length(max_length)) diff --git a/esphome/components/text_sensor/__init__.py b/esphome/components/text_sensor/__init__.py index c7ac17c35a..38f0ae451e 100644 --- a/esphome/components/text_sensor/__init__.py +++ b/esphome/components/text_sensor/__init__.py @@ -186,7 +186,7 @@ async def build_filters(config): async def setup_text_sensor_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "text_sensor") if (device_class := config.get(CONF_DEVICE_CLASS)) is not None: cg.add(var.set_device_class(device_class)) diff --git a/esphome/components/update/__init__.py b/esphome/components/update/__init__.py index 09b0698903..061dd4589f 100644 --- a/esphome/components/update/__init__.py +++ b/esphome/components/update/__init__.py @@ -87,7 +87,7 @@ UPDATE_SCHEMA.add_extra(cv.deprecated_schema_constant("update")) async def setup_update_core_(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "update") if device_class_config := config.get(CONF_DEVICE_CLASS): cg.add(var.set_device_class(device_class_config)) diff --git a/esphome/components/valve/__init__.py b/esphome/components/valve/__init__.py index a6f1428cd2..98c96f9afc 100644 --- a/esphome/components/valve/__init__.py +++ b/esphome/components/valve/__init__.py @@ -132,7 +132,7 @@ VALVE_SCHEMA.add_extra(cv.deprecated_schema_constant("valve")) async def _setup_valve_core(var, config): - await setup_entity(var, config) + await setup_entity(var, config, "valve") if device_class_config := config.get(CONF_DEVICE_CLASS): cg.add(var.set_device_class(device_class_config)) diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index bc98ff54db..00c1db33ee 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -522,6 +522,9 @@ class EsphomeCore: # Dict to track platform entity counts for pre-allocation # 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 + # Key: (device_id, platform, object_id), Value: count of duplicates + self.unique_ids: dict[tuple[int, str, str], int] = {} # Whether ESPHome was started in verbose mode self.verbose = False # Whether ESPHome was started in quiet mode @@ -553,6 +556,7 @@ class EsphomeCore: self.loaded_integrations = set() self.component_ids = set() self.platform_counts = defaultdict(int) + self.unique_ids = {} PIN_SCHEMA_REGISTRY.reset() @property diff --git a/esphome/core/entity_base.cpp b/esphome/core/entity_base.cpp index cf91e17a6a..7b86130f2f 100644 --- a/esphome/core/entity_base.cpp +++ b/esphome/core/entity_base.cpp @@ -36,21 +36,15 @@ void EntityBase::set_icon(const char *icon) { this->icon_c_str_ = icon; } // Entity Object ID std::string EntityBase::get_object_id() const { - std::string suffix = ""; -#ifdef USE_DEVICES - if (this->device_ != nullptr) { - suffix = "@" + str_sanitize(str_snake_case(this->device_->get_name())); - } -#endif // Check if `App.get_friendly_name()` is constant or dynamic. if (!this->flags_.has_own_name && App.is_name_add_mac_suffix_enabled()) { // `App.get_friendly_name()` is dynamic. - return str_sanitize(str_snake_case(App.get_friendly_name())) + suffix; + return str_sanitize(str_snake_case(App.get_friendly_name())); } else { // `App.get_friendly_name()` is constant. if (this->object_id_c_str_ == nullptr) { - return suffix; + return ""; } - return this->object_id_c_str_ + suffix; + return this->object_id_c_str_; } } void EntityBase::set_object_id(const char *object_id) { diff --git a/esphome/cpp_helpers.py b/esphome/cpp_helpers.py index e50be56092..ee91ac6132 100644 --- a/esphome/cpp_helpers.py +++ b/esphome/cpp_helpers.py @@ -15,7 +15,7 @@ from esphome.const import ( ) from esphome.core import CORE, ID, coroutine from esphome.coroutine import FakeAwaitable -from esphome.cpp_generator import add, get_variable +from esphome.cpp_generator import MockObj, add, get_variable from esphome.cpp_types import App from esphome.helpers import sanitize, snake_case from esphome.types import ConfigFragmentType, ConfigType @@ -97,18 +97,65 @@ async def register_parented(var, value): add(var.set_parent(paren)) -async def setup_entity(var, config): - """Set up generic properties of an Entity""" +async def setup_entity(var: MockObj, config: ConfigType, platform: str) -> None: + """Set up generic properties of an Entity. + + This function handles duplicate entity names by automatically appending + a suffix (_2, _3, etc.) when multiple entities have the same object_id + within the same platform and device combination. + + Args: + var: The entity variable to set up + config: Configuration dictionary containing entity settings + platform: The platform name (e.g., "sensor", "binary_sensor") + """ + # Get device info + device_id: int = 0 if CONF_DEVICE_ID in config: - device_id: ID = config[CONF_DEVICE_ID] - device = await get_variable(device_id) + device_id_obj: ID = config[CONF_DEVICE_ID] + device: MockObj = await get_variable(device_id_obj) add(var.set_device(device)) + # Use the device's ID hash as device_id + from esphome.helpers import fnv1a_32bit_hash + + device_id = fnv1a_32bit_hash(device_id_obj.id) add(var.set_name(config[CONF_NAME])) + + # Calculate base object_id + base_object_id: str if not config[CONF_NAME]: - add(var.set_object_id(sanitize(snake_case(CORE.friendly_name)))) + # Use the friendly name if available, otherwise use the device name + if CORE.friendly_name: + base_object_id = sanitize(snake_case(CORE.friendly_name)) + else: + base_object_id = sanitize(snake_case(CORE.name)) + _LOGGER.debug( + "Entity has empty name, using '%s' as object_id base", base_object_id + ) else: - add(var.set_object_id(sanitize(snake_case(config[CONF_NAME])))) + base_object_id = sanitize(snake_case(config[CONF_NAME])) + + # Handle duplicates + # Check for duplicates + unique_key: tuple[int, str, str] = (device_id, platform, base_object_id) + if unique_key in CORE.unique_ids: + # Found duplicate, add suffix + count = CORE.unique_ids[unique_key] + 1 + CORE.unique_ids[unique_key] = count + object_id = f"{base_object_id}_{count}" + _LOGGER.info( + "Duplicate %s entity '%s' found. Renaming to '%s'", + platform, + config[CONF_NAME], + object_id, + ) + else: + # First occurrence + CORE.unique_ids[unique_key] = 1 + object_id = base_object_id + + add(var.set_object_id(object_id)) add(var.set_disabled_by_default(config[CONF_DISABLED_BY_DEFAULT])) if CONF_INTERNAL in config: add(var.set_internal(config[CONF_INTERNAL])) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 955869b799..aac5a642f6 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -14,6 +14,8 @@ import sys import pytest +from esphome.core import CORE + here = Path(__file__).parent # Configure location of package root @@ -21,6 +23,13 @@ package_root = here.parent.parent sys.path.insert(0, package_root.as_posix()) +@pytest.fixture(autouse=True) +def reset_core(): + """Reset CORE after each test.""" + yield + CORE.reset() + + @pytest.fixture def fixture_path() -> Path: """ diff --git a/tests/unit_tests/test_duplicate_entities.py b/tests/unit_tests/test_duplicate_entities.py new file mode 100644 index 0000000000..ab075a02fc --- /dev/null +++ b/tests/unit_tests/test_duplicate_entities.py @@ -0,0 +1,129 @@ +"""Test duplicate entity object ID handling.""" + +import pytest + +from esphome.core import CORE +from esphome.helpers import sanitize, snake_case + + +@pytest.fixture +def setup_test_device() -> None: + """Set up test device configuration.""" + CORE.name = "test-device" + CORE.friendly_name = "Test Device" + + +def test_unique_key_generation() -> None: + """Test that unique keys are generated correctly.""" + # Test with no device + key1: tuple[int, str, str] = (0, "binary_sensor", "temperature") + assert key1 == (0, "binary_sensor", "temperature") + + # Test with device + key2: tuple[int, str, str] = (12345, "sensor", "humidity") + assert key2 == (12345, "sensor", "humidity") + + +def test_duplicate_tracking() -> None: + """Test that duplicates are tracked correctly.""" + # First occurrence + key: tuple[int, str, str] = (0, "sensor", "temperature") + assert key not in CORE.unique_ids + + CORE.unique_ids[key] = 1 + assert CORE.unique_ids[key] == 1 + + # Second occurrence + count: int = CORE.unique_ids[key] + 1 + CORE.unique_ids[key] = count + assert CORE.unique_ids[key] == 2 + + +def test_object_id_sanitization() -> None: + """Test that object IDs are properly sanitized.""" + # Test various inputs + assert sanitize(snake_case("Temperature Sensor")) == "temperature_sensor" + assert sanitize(snake_case("Living Room Light!")) == "living_room_light_" + assert sanitize(snake_case("Test-Device")) == "test-device" + assert sanitize(snake_case("")) == "" + + +def test_suffix_generation() -> None: + """Test that suffixes are generated correctly.""" + base_id: str = "temperature" + + # No suffix for first occurrence + object_id_1: str = base_id + assert object_id_1 == "temperature" + + # Add suffix for duplicates + count: int = 2 + object_id_2: str = f"{base_id}_{count}" + assert object_id_2 == "temperature_2" + + count = 3 + object_id_3: str = f"{base_id}_{count}" + assert object_id_3 == "temperature_3" + + +def test_different_platforms_same_name() -> None: + """Test that same name on different platforms doesn't conflict.""" + # Simulate two entities with same name on different platforms + key1: tuple[int, str, str] = (0, "binary_sensor", "status") + key2: tuple[int, str, str] = (0, "text_sensor", "status") + + # They should be different keys + assert key1 != key2 + + # Track them separately + CORE.unique_ids[key1] = 1 + CORE.unique_ids[key2] = 1 + + # Both should be at count 1 (no conflict) + assert CORE.unique_ids[key1] == 1 + assert CORE.unique_ids[key2] == 1 + + +def test_different_devices_same_name_platform() -> None: + """Test that same name+platform on different devices doesn't conflict.""" + # Simulate two entities with same name and platform but different devices + key1: tuple[int, str, str] = (12345, "sensor", "temperature") + key2: tuple[int, str, str] = (67890, "sensor", "temperature") + + # They should be different keys + assert key1 != key2 + + # Track them separately + CORE.unique_ids[key1] = 1 + CORE.unique_ids[key2] = 1 + + # Both should be at count 1 (no conflict) + assert CORE.unique_ids[key1] == 1 + assert CORE.unique_ids[key2] == 1 + + +def test_empty_name_handling(setup_test_device: None) -> None: + """Test handling of entities with empty names.""" + # When name is empty, it should use the device name + empty_name: str = "" + base_id: str + if not empty_name: + if CORE.friendly_name: + base_id = sanitize(snake_case(CORE.friendly_name)) + else: + base_id = sanitize(snake_case(CORE.name)) + + assert base_id == "test_device" # Uses friendly name + + +def test_reset_clears_unique_ids() -> None: + """Test that CORE.reset() clears the unique_ids tracking.""" + # Add some tracked IDs + CORE.unique_ids[(0, "sensor", "test")] = 2 + CORE.unique_ids[(0, "binary_sensor", "test")] = 3 + + assert len(CORE.unique_ids) == 2 + + # Reset should clear them + CORE.reset() + assert len(CORE.unique_ids) == 0