mirror of
				https://github.com/esphome/esphome.git
				synced 2025-10-30 22:53:59 +00:00 
			
		
		
		
	[core] Improve entity duplicate validation error messages (#10184)
This commit is contained in:
		| @@ -627,13 +627,15 @@ class SchemaValidationStep(ConfigValidationStep): | |||||||
|     def __init__( |     def __init__( | ||||||
|         self, domain: str, path: ConfigPath, conf: ConfigType, comp: ComponentManifest |         self, domain: str, path: ConfigPath, conf: ConfigType, comp: ComponentManifest | ||||||
|     ): |     ): | ||||||
|  |         self.domain = domain | ||||||
|         self.path = path |         self.path = path | ||||||
|         self.conf = conf |         self.conf = conf | ||||||
|         self.comp = comp |         self.comp = comp | ||||||
|  |  | ||||||
|     def run(self, result: Config) -> None: |     def run(self, result: Config) -> None: | ||||||
|         token = path_context.set(self.path) |         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: |             if self.comp.is_platform: | ||||||
|                 # Remove 'platform' key for validation |                 # Remove 'platform' key for validation | ||||||
|                 input_conf = OrderedDict(self.conf) |                 input_conf = OrderedDict(self.conf) | ||||||
|   | |||||||
| @@ -1,4 +1,5 @@ | |||||||
| from collections import defaultdict | from collections import defaultdict | ||||||
|  | from contextlib import contextmanager | ||||||
| import logging | import logging | ||||||
| import math | import math | ||||||
| import os | import os | ||||||
| @@ -38,7 +39,7 @@ from esphome.util import OrderedDict | |||||||
|  |  | ||||||
| if TYPE_CHECKING: | if TYPE_CHECKING: | ||||||
|     from ..cpp_generator import MockObj, MockObjClass, Statement |     from ..cpp_generator import MockObj, MockObjClass, Statement | ||||||
|     from ..types import ConfigType |     from ..types import ConfigType, EntityMetadata | ||||||
|  |  | ||||||
| _LOGGER = logging.getLogger(__name__) | _LOGGER = logging.getLogger(__name__) | ||||||
|  |  | ||||||
| @@ -571,14 +572,16 @@ class EsphomeCore: | |||||||
|         # Key: platform name (e.g. "sensor", "binary_sensor"), Value: count |         # Key: platform name (e.g. "sensor", "binary_sensor"), Value: count | ||||||
|         self.platform_counts: defaultdict[str, int] = defaultdict(int) |         self.platform_counts: defaultdict[str, int] = defaultdict(int) | ||||||
|         # Track entity unique IDs to handle duplicates |         # Track entity unique IDs to handle duplicates | ||||||
|         # Set of (device_id, platform, sanitized_name) tuples |         # Dict mapping (device_id, platform, sanitized_name) -> entity metadata | ||||||
|         self.unique_ids: set[tuple[str, str, str]] = set() |         self.unique_ids: dict[tuple[str, str, str], EntityMetadata] = {} | ||||||
|         # Whether ESPHome was started in verbose mode |         # Whether ESPHome was started in verbose mode | ||||||
|         self.verbose = False |         self.verbose = False | ||||||
|         # Whether ESPHome was started in quiet mode |         # Whether ESPHome was started in quiet mode | ||||||
|         self.quiet = False |         self.quiet = False | ||||||
|         # A list of all known ID classes |         # A list of all known ID classes | ||||||
|         self.id_classes = {} |         self.id_classes = {} | ||||||
|  |         # The current component being processed during validation | ||||||
|  |         self.current_component: str | None = None | ||||||
|  |  | ||||||
|     def reset(self): |     def reset(self): | ||||||
|         from esphome.pins import PIN_SCHEMA_REGISTRY |         from esphome.pins import PIN_SCHEMA_REGISTRY | ||||||
| @@ -604,9 +607,20 @@ class EsphomeCore: | |||||||
|         self.loaded_integrations = set() |         self.loaded_integrations = set() | ||||||
|         self.component_ids = set() |         self.component_ids = set() | ||||||
|         self.platform_counts = defaultdict(int) |         self.platform_counts = defaultdict(int) | ||||||
|         self.unique_ids = set() |         self.unique_ids = {} | ||||||
|  |         self.current_component = None | ||||||
|         PIN_SCHEMA_REGISTRY.reset() |         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 |     @property | ||||||
|     def address(self) -> str | None: |     def address(self) -> str | None: | ||||||
|         if self.config is None: |         if self.config is None: | ||||||
|   | |||||||
| @@ -16,7 +16,7 @@ from esphome.core import CORE, ID | |||||||
| from esphome.cpp_generator import MockObj, add, get_variable | from esphome.cpp_generator import MockObj, add, get_variable | ||||||
| import esphome.final_validate as fv | import esphome.final_validate as fv | ||||||
| from esphome.helpers import sanitize, snake_case | from esphome.helpers import sanitize, snake_case | ||||||
| from esphome.types import ConfigType | from esphome.types import ConfigType, EntityMetadata | ||||||
|  |  | ||||||
| _LOGGER = logging.getLogger(__name__) | _LOGGER = logging.getLogger(__name__) | ||||||
|  |  | ||||||
| @@ -214,14 +214,45 @@ def entity_duplicate_validator(platform: str) -> Callable[[ConfigType], ConfigTy | |||||||
|         # Check for duplicates |         # Check for duplicates | ||||||
|         unique_key = (device_id, platform, name_key) |         unique_key = (device_id, platform, name_key) | ||||||
|         if unique_key in CORE.unique_ids: |         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 "" |             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( |             raise cv.Invalid( | ||||||
|                 f"Duplicate {platform} entity with name '{entity_name}' found{device_prefix}. " |                 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." |                 f"Each entity on a device must have a unique name within its platform." | ||||||
|             ) |             ) | ||||||
|  |  | ||||||
|         # Add to tracking set |         # Store metadata about this entity | ||||||
|         CORE.unique_ids.add(unique_key) |         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 config | ||||||
|  |  | ||||||
|     return validator |     return validator | ||||||
|   | |||||||
| @@ -1,5 +1,7 @@ | |||||||
| """This helper module tracks commonly used types in the esphome python codebase.""" | """This helper module tracks commonly used types in the esphome python codebase.""" | ||||||
|  |  | ||||||
|  | from typing import TypedDict | ||||||
|  |  | ||||||
| from esphome.core import ID, EsphomeCore, Lambda | from esphome.core import ID, EsphomeCore, Lambda | ||||||
|  |  | ||||||
| ConfigFragmentType = ( | ConfigFragmentType = ( | ||||||
| @@ -16,3 +18,13 @@ ConfigFragmentType = ( | |||||||
| ConfigType = dict[str, ConfigFragmentType] | ConfigType = dict[str, ConfigFragmentType] | ||||||
| CoreType = EsphomeCore | CoreType = EsphomeCore | ||||||
| ConfigPathType = str | int | 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 | ||||||
|   | |||||||
| @@ -12,6 +12,7 @@ from esphome.const import ( | |||||||
|     CONF_DEVICE_ID, |     CONF_DEVICE_ID, | ||||||
|     CONF_DISABLED_BY_DEFAULT, |     CONF_DISABLED_BY_DEFAULT, | ||||||
|     CONF_ICON, |     CONF_ICON, | ||||||
|  |     CONF_ID, | ||||||
|     CONF_INTERNAL, |     CONF_INTERNAL, | ||||||
|     CONF_NAME, |     CONF_NAME, | ||||||
| ) | ) | ||||||
| @@ -511,12 +512,18 @@ def test_entity_duplicate_validator() -> None: | |||||||
|     validated1 = validator(config1) |     validated1 = validator(config1) | ||||||
|     assert validated1 == config1 |     assert validated1 == config1 | ||||||
|     assert ("", "sensor", "temperature") in CORE.unique_ids |     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 |     # Second entity with different name should pass | ||||||
|     config2 = {CONF_NAME: "Humidity"} |     config2 = {CONF_NAME: "Humidity"} | ||||||
|     validated2 = validator(config2) |     validated2 = validator(config2) | ||||||
|     assert validated2 == config2 |     assert validated2 == config2 | ||||||
|     assert ("", "sensor", "humidity") in CORE.unique_ids |     assert ("", "sensor", "humidity") in CORE.unique_ids | ||||||
|  |     metadata2 = CORE.unique_ids[("", "sensor", "humidity")] | ||||||
|  |     assert metadata2["name"] == "Humidity" | ||||||
|  |  | ||||||
|     # Duplicate entity should fail |     # Duplicate entity should fail | ||||||
|     config3 = {CONF_NAME: "Temperature"} |     config3 = {CONF_NAME: "Temperature"} | ||||||
| @@ -540,11 +547,15 @@ def test_entity_duplicate_validator_with_devices() -> None: | |||||||
|     validated1 = validator(config1) |     validated1 = validator(config1) | ||||||
|     assert validated1 == config1 |     assert validated1 == config1 | ||||||
|     assert ("device1", "sensor", "temperature") in CORE.unique_ids |     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} |     config2 = {CONF_NAME: "Temperature", CONF_DEVICE_ID: device2} | ||||||
|     validated2 = validator(config2) |     validated2 = validator(config2) | ||||||
|     assert validated2 == config2 |     assert validated2 == config2 | ||||||
|     assert ("device2", "sensor", "temperature") in CORE.unique_ids |     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 |     # Duplicate on same device should fail | ||||||
|     config3 = {CONF_NAME: "Temperature", CONF_DEVICE_ID: device1} |     config3 = {CONF_NAME: "Temperature", CONF_DEVICE_ID: device1} | ||||||
| @@ -595,6 +606,54 @@ def test_entity_different_platforms_yaml_validation( | |||||||
|     assert result is not None |     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: | def test_entity_duplicate_validator_internal_entities() -> None: | ||||||
|     """Test that internal entities are excluded from duplicate name validation.""" |     """Test that internal entities are excluded from duplicate name validation.""" | ||||||
|     # Create validator for sensor platform |     # Create validator for sensor platform | ||||||
| @@ -612,14 +671,17 @@ def test_entity_duplicate_validator_internal_entities() -> None: | |||||||
|     validated2 = validator(config2) |     validated2 = validator(config2) | ||||||
|     assert validated2 == config2 |     assert validated2 == config2 | ||||||
|     # Internal entity should not be added to unique_ids |     # 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 |     # Another internal entity with same name should also pass | ||||||
|     config3 = {CONF_NAME: "Temperature", CONF_INTERNAL: True} |     config3 = {CONF_NAME: "Temperature", CONF_INTERNAL: True} | ||||||
|     validated3 = validator(config3) |     validated3 = validator(config3) | ||||||
|     assert validated3 == config3 |     assert validated3 == config3 | ||||||
|     # Still only one entry in unique_ids (from the non-internal entity) |     # 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 |     # Non-internal entity with same name should fail | ||||||
|     config4 = {CONF_NAME: "Temperature"} |     config4 = {CONF_NAME: "Temperature"} | ||||||
|   | |||||||
| @@ -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 | ||||||
		Reference in New Issue
	
	Block a user