diff --git a/esphome/config.py b/esphome/config.py index cf7a232d8e..ecd0cbb048 100644 --- a/esphome/config.py +++ b/esphome/config.py @@ -627,13 +627,15 @@ class SchemaValidationStep(ConfigValidationStep): def __init__( self, domain: str, path: ConfigPath, conf: ConfigType, comp: ComponentManifest ): + self.domain = domain self.path = path self.conf = conf self.comp = comp def run(self, result: Config) -> None: token = path_context.set(self.path) - with result.catch_error(self.path): + # The domain already contains the full component path (e.g., "sensor.template", "sensor.uptime") + with CORE.component_context(self.domain), result.catch_error(self.path): if self.comp.is_platform: # Remove 'platform' key for validation input_conf = OrderedDict(self.conf) diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index 39c6c3def1..9df5da1c78 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -1,4 +1,5 @@ from collections import defaultdict +from contextlib import contextmanager import logging import math import os @@ -38,7 +39,7 @@ from esphome.util import OrderedDict if TYPE_CHECKING: from ..cpp_generator import MockObj, MockObjClass, Statement - from ..types import ConfigType + from ..types import ConfigType, EntityMetadata _LOGGER = logging.getLogger(__name__) @@ -571,14 +572,16 @@ 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 - # Set of (device_id, platform, sanitized_name) tuples - self.unique_ids: set[tuple[str, str, str]] = set() + # Dict mapping (device_id, platform, sanitized_name) -> entity metadata + self.unique_ids: dict[tuple[str, str, str], EntityMetadata] = {} # Whether ESPHome was started in verbose mode self.verbose = False # Whether ESPHome was started in quiet mode self.quiet = False # A list of all known ID classes self.id_classes = {} + # The current component being processed during validation + self.current_component: str | None = None def reset(self): from esphome.pins import PIN_SCHEMA_REGISTRY @@ -604,9 +607,20 @@ class EsphomeCore: self.loaded_integrations = set() self.component_ids = set() self.platform_counts = defaultdict(int) - self.unique_ids = set() + self.unique_ids = {} + self.current_component = None PIN_SCHEMA_REGISTRY.reset() + @contextmanager + def component_context(self, component: str): + """Context manager to set the current component being processed.""" + old_component = self.current_component + self.current_component = component + try: + yield + finally: + self.current_component = old_component + @property def address(self) -> str | None: if self.config is None: diff --git a/esphome/core/entity_helpers.py b/esphome/core/entity_helpers.py index cc388ffb4c..107b9fd739 100644 --- a/esphome/core/entity_helpers.py +++ b/esphome/core/entity_helpers.py @@ -16,7 +16,7 @@ from esphome.core import CORE, ID from esphome.cpp_generator import MockObj, add, get_variable import esphome.final_validate as fv from esphome.helpers import sanitize, snake_case -from esphome.types import ConfigType +from esphome.types import ConfigType, EntityMetadata _LOGGER = logging.getLogger(__name__) @@ -214,14 +214,45 @@ def entity_duplicate_validator(platform: str) -> Callable[[ConfigType], ConfigTy # Check for duplicates unique_key = (device_id, platform, name_key) if unique_key in CORE.unique_ids: + # Get the existing entity metadata + existing = CORE.unique_ids[unique_key] + existing_name = existing.get("name", entity_name) + existing_device = existing.get("device_id", "") + existing_id = existing.get("entity_id", "unknown") + + # Build detailed error message device_prefix = f" on device '{device_id}'" if device_id else "" + existing_device_prefix = ( + f" on device '{existing_device}'" if existing_device else "" + ) + existing_component = existing.get("component", "unknown") + + # Provide more context about where the duplicate was found + conflict_msg = ( + f"Conflicts with entity '{existing_name}'{existing_device_prefix}" + ) + if existing_id != "unknown": + conflict_msg += f" (id: {existing_id})" + if existing_component != "unknown": + conflict_msg += f" from component '{existing_component}'" + raise cv.Invalid( f"Duplicate {platform} entity with name '{entity_name}' found{device_prefix}. " + f"{conflict_msg}. " f"Each entity on a device must have a unique name within its platform." ) - # Add to tracking set - CORE.unique_ids.add(unique_key) + # Store metadata about this entity + entity_metadata: EntityMetadata = { + "name": entity_name, + "device_id": device_id, + "platform": platform, + "entity_id": str(config.get(CONF_ID, "unknown")), + "component": CORE.current_component or "unknown", + } + + # Add to tracking dict + CORE.unique_ids[unique_key] = entity_metadata return config return validator diff --git a/esphome/types.py b/esphome/types.py index f68f503993..62499a953c 100644 --- a/esphome/types.py +++ b/esphome/types.py @@ -1,5 +1,7 @@ """This helper module tracks commonly used types in the esphome python codebase.""" +from typing import TypedDict + from esphome.core import ID, EsphomeCore, Lambda ConfigFragmentType = ( @@ -16,3 +18,13 @@ ConfigFragmentType = ( ConfigType = dict[str, ConfigFragmentType] CoreType = EsphomeCore ConfigPathType = str | int + + +class EntityMetadata(TypedDict): + """Metadata stored for each entity to help with duplicate detection.""" + + name: str + device_id: str + platform: str + entity_id: str + component: str diff --git a/tests/unit_tests/core/test_entity_helpers.py b/tests/unit_tests/core/test_entity_helpers.py index c639ad94b2..2157bc20a9 100644 --- a/tests/unit_tests/core/test_entity_helpers.py +++ b/tests/unit_tests/core/test_entity_helpers.py @@ -12,6 +12,7 @@ from esphome.const import ( CONF_DEVICE_ID, CONF_DISABLED_BY_DEFAULT, CONF_ICON, + CONF_ID, CONF_INTERNAL, CONF_NAME, ) @@ -511,12 +512,18 @@ def test_entity_duplicate_validator() -> None: validated1 = validator(config1) assert validated1 == config1 assert ("", "sensor", "temperature") in CORE.unique_ids + # Check metadata was stored + metadata = CORE.unique_ids[("", "sensor", "temperature")] + assert metadata["name"] == "Temperature" + assert metadata["platform"] == "sensor" # Second entity with different name should pass config2 = {CONF_NAME: "Humidity"} validated2 = validator(config2) assert validated2 == config2 assert ("", "sensor", "humidity") in CORE.unique_ids + metadata2 = CORE.unique_ids[("", "sensor", "humidity")] + assert metadata2["name"] == "Humidity" # Duplicate entity should fail config3 = {CONF_NAME: "Temperature"} @@ -540,11 +547,15 @@ def test_entity_duplicate_validator_with_devices() -> None: validated1 = validator(config1) assert validated1 == config1 assert ("device1", "sensor", "temperature") in CORE.unique_ids + metadata1 = CORE.unique_ids[("device1", "sensor", "temperature")] + 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 metadata2["device_id"] == "device2" # Duplicate on same device should fail config3 = {CONF_NAME: "Temperature", CONF_DEVICE_ID: device1} @@ -595,6 +606,54 @@ def test_entity_different_platforms_yaml_validation( 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 + validator = entity_duplicate_validator("sensor") + + # Set current component to simulate validation context for uptime sensor + CORE.current_component = "sensor.uptime" + + # First entity should pass + config1 = {CONF_NAME: "Battery", CONF_ID: ID("battery_1")} + validated1 = validator(config1) + assert validated1 == config1 + + # Reset component to simulate template sensor + CORE.current_component = "sensor.template" + + # Duplicate entity should fail with detailed error + config2 = {CONF_NAME: "Battery", CONF_ID: ID("battery_2")} + with pytest.raises( + Invalid, + match=r"Duplicate sensor entity with name 'Battery' found.*" + r"Conflicts with entity 'Battery' \(id: battery_1\) from component 'sensor\.uptime'", + ): + validator(config2) + + # Clean up + CORE.current_component = None + + +def test_entity_conflict_between_components_yaml( + yaml_file: Callable[[str], str], capsys: pytest.CaptureFixture[str] +) -> None: + """Test that conflicts between different components show helpful error messages.""" + result = load_config_from_fixture( + yaml_file, "entity_conflict_components.yaml", FIXTURES_DIR + ) + assert result is None + + # Check for the enhanced error message + captured = capsys.readouterr() + # The error should mention both the conflict and which component created it + assert "Duplicate sensor entity with name 'Battery' found" in captured.out + # Should mention it conflicts with an entity from a specific sensor platform + assert "from component 'sensor." in captured.out + # Should show it's a conflict between wifi_signal and template + assert "sensor.wifi_signal" in captured.out or "sensor.template" in captured.out + + def test_entity_duplicate_validator_internal_entities() -> None: """Test that internal entities are excluded from duplicate name validation.""" # Create validator for sensor platform @@ -612,14 +671,17 @@ def test_entity_duplicate_validator_internal_entities() -> None: validated2 = validator(config2) assert validated2 == config2 # Internal entity should not be added to unique_ids - assert len([k for k in CORE.unique_ids if k == ("", "sensor", "temperature")]) == 1 + # Count how many times the key appears (should still be 1) + count = sum(1 for k in CORE.unique_ids if k == ("", "sensor", "temperature")) + assert count == 1 # Another internal entity with same name should also pass config3 = {CONF_NAME: "Temperature", CONF_INTERNAL: True} validated3 = validator(config3) assert validated3 == config3 # Still only one entry in unique_ids (from the non-internal entity) - assert len([k for k in CORE.unique_ids if k == ("", "sensor", "temperature")]) == 1 + count = sum(1 for k in CORE.unique_ids if k == ("", "sensor", "temperature")) + assert count == 1 # Non-internal entity with same name should fail config4 = {CONF_NAME: "Temperature"} diff --git a/tests/unit_tests/fixtures/core/entity_helpers/entity_conflict_components.yaml b/tests/unit_tests/fixtures/core/entity_helpers/entity_conflict_components.yaml new file mode 100644 index 0000000000..6a1df0f7b4 --- /dev/null +++ b/tests/unit_tests/fixtures/core/entity_helpers/entity_conflict_components.yaml @@ -0,0 +1,20 @@ +esphome: + name: test-device + +esp32: + board: esp32dev + +# Uptime sensor +sensor: + - platform: uptime + name: "Battery" + id: uptime_battery + +# Template sensor also named "Battery" - this should conflict + - platform: template + name: "Battery" + id: template_battery + lambda: |- + return 95.0; + unit_of_measurement: "%" + update_interval: 60s