diff --git a/esphome/cpp_helpers.py b/esphome/cpp_helpers.py index a1289485ca..746a006348 100644 --- a/esphome/cpp_helpers.py +++ b/esphome/cpp_helpers.py @@ -1,12 +1,6 @@ import logging from esphome.const import ( - CONF_DEVICE_ID, - CONF_DISABLED_BY_DEFAULT, - CONF_ENTITY_CATEGORY, - CONF_ICON, - CONF_INTERNAL, - CONF_NAME, CONF_SAFE_MODE, CONF_SETUP_PRIORITY, CONF_TYPE_ID, @@ -15,9 +9,11 @@ from esphome.const import ( ) from esphome.core import CORE, ID, coroutine from esphome.coroutine import FakeAwaitable -from esphome.cpp_generator import MockObj, add, get_variable +from esphome.cpp_generator import add, get_variable from esphome.cpp_types import App -from esphome.entity import get_base_entity_object_id +from esphome.entity import ( # noqa: F401 # pylint: disable=unused-import + setup_entity, # Import for backward compatibility +) from esphome.types import ConfigFragmentType, ConfigType from esphome.util import Registry, RegistryEntry @@ -97,75 +93,6 @@ async def register_parented(var, value): add(var.set_parent(paren)) -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_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 using the same logic as C++ - # This must match the C++ behavior in esphome/core/entity_base.cpp - base_object_id = get_base_entity_object_id(config[CONF_NAME], CORE.friendly_name) - - if not config[CONF_NAME]: - _LOGGER.debug( - "Entity has empty name, using '%s' as object_id base", base_object_id - ) - - # 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)) - _LOGGER.debug( - "Setting object_id '%s' for entity '%s' on platform '%s'", - object_id, - config[CONF_NAME], - platform, - ) - add(var.set_disabled_by_default(config[CONF_DISABLED_BY_DEFAULT])) - if CONF_INTERNAL in config: - add(var.set_internal(config[CONF_INTERNAL])) - if CONF_ICON in config: - add(var.set_icon(config[CONF_ICON])) - if CONF_ENTITY_CATEGORY in config: - add(var.set_entity_category(config[CONF_ENTITY_CATEGORY])) - - def extract_registry_entry_config( registry: Registry, full_config: ConfigType, diff --git a/esphome/entity.py b/esphome/entity.py index 732822d0ff..fa7f1ab7d9 100644 --- a/esphome/entity.py +++ b/esphome/entity.py @@ -1,10 +1,26 @@ """Entity-related helper functions.""" -from esphome.core import CORE +import logging + +from esphome.const import ( + CONF_DEVICE_ID, + CONF_DISABLED_BY_DEFAULT, + CONF_ENTITY_CATEGORY, + CONF_ICON, + CONF_INTERNAL, + CONF_NAME, +) +from esphome.core import CORE, ID +from esphome.cpp_generator import MockObj, add, get_variable from esphome.helpers import sanitize, snake_case +from esphome.types import ConfigType + +_LOGGER = logging.getLogger(__name__) -def get_base_entity_object_id(name: str, friendly_name: str | None) -> str: +def get_base_entity_object_id( + 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(). This function calculates what object_id_c_str_ should be set to in C++. @@ -21,6 +37,7 @@ def get_base_entity_object_id(name: str, friendly_name: str | None) -> str: 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() @@ -29,9 +46,12 @@ def get_base_entity_object_id(name: str, friendly_name: str | None) -> str: 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) - # Calculate what the object_id should be # C++ uses App.get_friendly_name() which returns friendly_name or device name base_str = friendly_name else: @@ -39,3 +59,77 @@ def get_base_entity_object_id(name: str, friendly_name: str | None) -> str: base_str = CORE.name return sanitize(snake_case(base_str)) + + +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 + device_name: str | None = None + if CONF_DEVICE_ID in config: + 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) + # Get device name for object ID calculation + device_name = device_id_obj.id + + add(var.set_name(config[CONF_NAME])) + + # Calculate base object_id using the same logic as C++ + # This must match the C++ behavior in esphome/core/entity_base.cpp + base_object_id = get_base_entity_object_id( + config[CONF_NAME], CORE.friendly_name, device_name + ) + + if not config[CONF_NAME]: + _LOGGER.debug( + "Entity has empty name, using '%s' as object_id base", base_object_id + ) + + # 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)) + _LOGGER.debug( + "Setting object_id '%s' for entity '%s' on platform '%s'", + object_id, + config[CONF_NAME], + platform, + ) + add(var.set_disabled_by_default(config[CONF_DISABLED_BY_DEFAULT])) + if CONF_INTERNAL in config: + add(var.set_internal(config[CONF_INTERNAL])) + if CONF_ICON in config: + add(var.set_icon(config[CONF_ICON])) + if CONF_ENTITY_CATEGORY in config: + add(var.set_entity_category(config[CONF_ENTITY_CATEGORY])) diff --git a/tests/integration/fixtures/duplicate_entities.yaml b/tests/integration/fixtures/duplicate_entities.yaml index 0f831db90d..17332fe4b2 100644 --- a/tests/integration/fixtures/duplicate_entities.yaml +++ b/tests/integration/fixtures/duplicate_entities.yaml @@ -86,6 +86,22 @@ binary_sensor: device_id: controller_2 lambda: return false; + # Issue #6953: Empty names on binary sensors + - platform: template + name: "" + lambda: return true; + - platform: template + name: "" + lambda: return false; + + - platform: template + name: "" + lambda: return true; + + - platform: template + name: "" + lambda: return false; + # Scenario 6: Test with special characters that need sanitization text_sensor: - platform: template @@ -116,3 +132,80 @@ switch: lambda: return true; turn_on_action: [] turn_off_action: [] + + # Scenario 8: Issue #6953 - Multiple entities with empty names + # Empty names on main device - should use device name with suffixes + - platform: template + name: "" + lambda: return false; + turn_on_action: [] + turn_off_action: [] + + - platform: template + name: "" + lambda: return true; + turn_on_action: [] + turn_off_action: [] + + - platform: template + name: "" + lambda: return false; + turn_on_action: [] + turn_off_action: [] + + # Scenario 9: Issue #6953 - Empty names on sub-devices + # Empty names on sub-device - should use sub-device name with suffixes + - platform: template + name: "" + device_id: controller_1 + lambda: return false; + turn_on_action: [] + turn_off_action: [] + + - platform: template + name: "" + device_id: controller_1 + lambda: return true; + turn_on_action: [] + turn_off_action: [] + + - platform: template + name: "" + device_id: controller_1 + lambda: return false; + turn_on_action: [] + turn_off_action: [] + + # Empty names on different sub-device + - platform: template + name: "" + device_id: controller_2 + lambda: return false; + turn_on_action: [] + turn_off_action: [] + + - platform: template + name: "" + device_id: controller_2 + lambda: return true; + turn_on_action: [] + turn_off_action: [] + + # Scenario 10: Issue #6953 - Duplicate "xyz" names + - platform: template + name: "xyz" + lambda: return false; + turn_on_action: [] + turn_off_action: [] + + - platform: template + name: "xyz" + lambda: return true; + turn_on_action: [] + turn_off_action: [] + + - platform: template + name: "xyz" + lambda: return false; + turn_on_action: [] + turn_off_action: [] diff --git a/tests/integration/test_duplicate_entities.py b/tests/integration/test_duplicate_entities.py index edbcb9799c..ba40e6bd23 100644 --- a/tests/integration/test_duplicate_entities.py +++ b/tests/integration/test_duplicate_entities.py @@ -161,6 +161,85 @@ async def test_duplicate_entities( "Second power switch should be power_switch_2" ) + # Scenario 6: Check empty names on main device (Issue #6953) + empty_binary = [b for b in binary_sensors if b.name == ""] + empty_binary_ids = sorted([b.object_id for b in empty_binary]) + + # Should use device name "duplicate-entities-test" (sanitized, not snake_case) + assert len(empty_binary_ids) >= 4, ( + f"Expected at least 4 empty name binary sensors, got {len(empty_binary_ids)}" + ) + assert "duplicate-entities-test" in empty_binary_ids, ( + "First empty binary sensor should use device name" + ) + assert "duplicate-entities-test_2" in empty_binary_ids, ( + "Second empty binary sensor should be duplicate-entities-test_2" + ) + assert "duplicate-entities-test_3" in empty_binary_ids, ( + "Third empty binary sensor should be duplicate-entities-test_3" + ) + assert "duplicate-entities-test_4" in empty_binary_ids, ( + "Fourth empty binary sensor should be duplicate-entities-test_4" + ) + + # Scenario 7: Check empty names on sub-devices (Issue #6953) + empty_switches = [s for s in switches if s.name == ""] + + # Group by device + c1_empty_switches = [ + s + for s in empty_switches + if getattr(s, "device_id", None) == controller_1.device_id + ] + c2_empty_switches = [ + s + for s in empty_switches + if getattr(s, "device_id", None) == controller_2.device_id + ] + main_empty_switches = [ + s + for s in empty_switches + if getattr(s, "device_id", None) + not in [controller_1.device_id, controller_2.device_id] + ] + + # Controller 1 empty switches should use "controller_1" + c1_empty_ids = sorted([s.object_id for s in c1_empty_switches]) + assert len(c1_empty_ids) >= 3, ( + f"Expected at least 3 empty switches on controller_1, got {len(c1_empty_ids)}" + ) + assert "controller_1" in c1_empty_ids, "First should be controller_1" + assert "controller_1_2" in c1_empty_ids, "Second should be controller_1_2" + assert "controller_1_3" in c1_empty_ids, "Third should be controller_1_3" + + # Controller 2 empty switches + c2_empty_ids = sorted([s.object_id for s in c2_empty_switches]) + assert len(c2_empty_ids) >= 2, ( + f"Expected at least 2 empty switches on controller_2, got {len(c2_empty_ids)}" + ) + assert "controller_2" in c2_empty_ids, "First should be controller_2" + assert "controller_2_2" in c2_empty_ids, "Second should be controller_2_2" + + # Main device empty switches + main_empty_ids = sorted([s.object_id for s in main_empty_switches]) + assert len(main_empty_ids) >= 3, ( + f"Expected at least 3 empty switches on main device, got {len(main_empty_ids)}" + ) + assert "duplicate-entities-test" in main_empty_ids + assert "duplicate-entities-test_2" in main_empty_ids + assert "duplicate-entities-test_3" in main_empty_ids + + # Scenario 8: Check "xyz" duplicates (Issue #6953) + xyz_switches = [s for s in switches if s.name == "xyz"] + xyz_ids = sorted([s.object_id for s in xyz_switches]) + + assert len(xyz_ids) >= 3, ( + f"Expected at least 3 xyz switches, got {len(xyz_ids)}" + ) + assert "xyz" in xyz_ids, "First xyz switch should be xyz" + assert "xyz_2" in xyz_ids, "Second xyz switch should be xyz_2" + assert "xyz_3" in xyz_ids, "Third xyz switch should be xyz_3" + # Verify we can get states for all entities (ensures they're functional) loop = asyncio.get_running_loop() states_future: asyncio.Future[bool] = loop.create_future() diff --git a/tests/unit_tests/test_duplicate_entities.py b/tests/unit_tests/test_duplicate_entities.py deleted file mode 100644 index ab075a02fc..0000000000 --- a/tests/unit_tests/test_duplicate_entities.py +++ /dev/null @@ -1,129 +0,0 @@ -"""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 diff --git a/tests/unit_tests/test_entity.py b/tests/unit_tests/test_entity.py new file mode 100644 index 0000000000..6cdf5369ae --- /dev/null +++ b/tests/unit_tests/test_entity.py @@ -0,0 +1,590 @@ +"""Test get_base_entity_object_id function matches C++ behavior.""" + +from collections.abc import Generator +from typing import Any + +import pytest + +from esphome import entity +from esphome.const import CONF_DEVICE_ID, CONF_DISABLED_BY_DEFAULT, CONF_ICON, CONF_NAME +from esphome.core import CORE, ID +from esphome.cpp_generator import MockObj +from esphome.entity import get_base_entity_object_id, setup_entity +from esphome.helpers import sanitize, snake_case + + +@pytest.fixture(autouse=True) +def restore_core_state() -> Generator[None, None, None]: + """Save and restore CORE state for tests.""" + original_name = CORE.name + original_friendly_name = CORE.friendly_name + yield + CORE.name = original_name + 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 + + +def test_matches_cpp_helpers() -> None: + """Test that the logic matches using snake_case and sanitize directly.""" + test_cases = [ + ("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___"), + ] + + for name, expected in test_cases: + # 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 + + # 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++, when name_add_mac_suffix is enabled and entity has no name, + get_object_id() returns str_sanitize(str_snake_case(App.get_friendly_name())) + dynamically. 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: + """Test the priority order: entity name > device name > friendly name > CORE.name.""" + CORE.name = "core-device" + + # 1. Entity name has highest priority + assert ( + get_base_entity_object_id("Entity Name", "Friendly Name", "Device Name") + == "entity_name" + ) + + # 2. Device name is next priority (when entity name is empty) + assert ( + get_base_entity_object_id("", "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" + + # 4. CORE.name is last resort + assert get_base_entity_object_id("", None, None) == "core-device" + + +def test_real_world_examples() -> None: + """Test real-world entity naming scenarios.""" + # Common ESPHome entity names + test_cases = [ + # 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"), + ] + + for name, friendly_name, device_name, expected in test_cases: + result = get_base_entity_object_id(name, friendly_name, device_name) + assert result == expected, ( + f"Failed for {name=}, {friendly_name=}, {device_name=}: {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" + + +# Tests for setup_entity function + + +@pytest.fixture +def setup_test_environment() -> Generator[list[str], None, None]: + """Set up test environment for setup_entity tests.""" + # Reset CORE state + CORE.reset() + CORE.name = "test-device" + CORE.friendly_name = "Test Device" + # Store original add function + + original_add = entity.add + # Track what gets added + added_expressions = [] + + def mock_add(expression: Any) -> Any: + added_expressions.append(str(expression)) + return original_add(expression) + + # Patch add function in entity module + entity.add = mock_add + yield added_expressions + # Clean up + entity.add = original_add + CORE.reset() + + +def extract_object_id_from_expressions(expressions: list[str]) -> str | None: + """Extract the object ID that was set from the generated expressions.""" + for expr in expressions: + # Look for set_object_id calls + if ".set_object_id(" in expr: + # Extract the ID from something like: var.set_object_id("temperature_2") + start = expr.find('"') + 1 + end = expr.rfind('"') + if start > 0 and end > start: + return expr[start:end] + return None + + +@pytest.mark.asyncio +async def test_setup_entity_no_duplicates(setup_test_environment: list[str]) -> None: + """Test setup_entity with unique names.""" + + added_expressions = setup_test_environment + + # Create mock entities + var1 = MockObj("sensor1") + var2 = MockObj("sensor2") + + # Set up first entity + config1 = { + CONF_NAME: "Temperature", + CONF_DISABLED_BY_DEFAULT: False, + } + await setup_entity(var1, config1, "sensor") + + # Get object ID from first entity + object_id1 = extract_object_id_from_expressions(added_expressions) + assert object_id1 == "temperature" + + # Clear for next entity + added_expressions.clear() + + # Set up second entity with different name + config2 = { + CONF_NAME: "Humidity", + CONF_DISABLED_BY_DEFAULT: False, + } + await setup_entity(var2, config2, "sensor") + + # Get object ID from second entity + object_id2 = extract_object_id_from_expressions(added_expressions) + assert object_id2 == "humidity" + + +@pytest.mark.asyncio +async def test_setup_entity_with_duplicates(setup_test_environment: list[str]) -> None: + """Test setup_entity with duplicate names.""" + + added_expressions = setup_test_environment + + # Create mock entities + entities = [MockObj(f"sensor{i}") for i in range(4)] + + # Set up entities with same name + config = { + CONF_NAME: "Temperature", + CONF_DISABLED_BY_DEFAULT: False, + } + + object_ids = [] + for var in entities: + added_expressions.clear() + await setup_entity(var, config, "sensor") + object_id = extract_object_id_from_expressions(added_expressions) + object_ids.append(object_id) + + # Check that object IDs were set with proper suffixes + assert object_ids[0] == "temperature" + assert object_ids[1] == "temperature_2" + assert object_ids[2] == "temperature_3" + assert object_ids[3] == "temperature_4" + + +@pytest.mark.asyncio +async def test_setup_entity_different_platforms( + setup_test_environment: list[str], +) -> None: + """Test that same name on different platforms doesn't conflict.""" + + added_expressions = setup_test_environment + + # Create mock entities + sensor = MockObj("sensor1") + binary_sensor = MockObj("binary_sensor1") + text_sensor = MockObj("text_sensor1") + + config = { + CONF_NAME: "Status", + CONF_DISABLED_BY_DEFAULT: False, + } + + # Set up entities on different platforms + platforms = [ + (sensor, "sensor"), + (binary_sensor, "binary_sensor"), + (text_sensor, "text_sensor"), + ] + + object_ids = [] + for var, platform in platforms: + added_expressions.clear() + await setup_entity(var, config, platform) + object_id = extract_object_id_from_expressions(added_expressions) + object_ids.append(object_id) + + # All should get base object ID without suffix + assert all(obj_id == "status" for obj_id in object_ids) + + +@pytest.mark.asyncio +async def test_setup_entity_with_devices(setup_test_environment: list[str]) -> None: + """Test that same name on different devices doesn't conflict.""" + + added_expressions = setup_test_environment + + # Create mock devices + device1_id = ID("device1", type="Device") + device2_id = ID("device2", type="Device") + + device1 = MockObj("device1_obj") + device2 = MockObj("device2_obj") + + # Mock get_variable to return our devices + original_get_variable = entity.get_variable + + async def mock_get_variable(device_id: ID) -> MockObj: + if device_id == device1_id: + return device1 + elif device_id == device2_id: + return device2 + return await original_get_variable(device_id) + + entity.get_variable = mock_get_variable + + try: + # Create sensors with same name on different devices + sensor1 = MockObj("sensor1") + sensor2 = MockObj("sensor2") + + config1 = { + CONF_NAME: "Temperature", + CONF_DEVICE_ID: device1_id, + CONF_DISABLED_BY_DEFAULT: False, + } + + config2 = { + CONF_NAME: "Temperature", + CONF_DEVICE_ID: device2_id, + CONF_DISABLED_BY_DEFAULT: False, + } + + # Get object IDs + object_ids = [] + for var, config in [(sensor1, config1), (sensor2, config2)]: + added_expressions.clear() + await setup_entity(var, config, "sensor") + object_id = extract_object_id_from_expressions(added_expressions) + object_ids.append(object_id) + + # Both should get base object ID without suffix (different devices) + assert object_ids[0] == "temperature" + assert object_ids[1] == "temperature" + + finally: + entity.get_variable = original_get_variable + + +@pytest.mark.asyncio +async def test_setup_entity_empty_name(setup_test_environment: list[str]) -> None: + """Test setup_entity with empty entity name.""" + + added_expressions = setup_test_environment + + var = MockObj("sensor1") + + config = { + CONF_NAME: "", + CONF_DISABLED_BY_DEFAULT: False, + } + + await setup_entity(var, config, "sensor") + + object_id = extract_object_id_from_expressions(added_expressions) + # Should use friendly name + assert object_id == "test_device" + + +@pytest.mark.asyncio +async def test_setup_entity_empty_name_duplicates( + setup_test_environment: list[str], +) -> None: + """Test setup_entity with multiple empty names.""" + + added_expressions = setup_test_environment + + entities = [MockObj(f"sensor{i}") for i in range(3)] + + config = { + CONF_NAME: "", + CONF_DISABLED_BY_DEFAULT: False, + } + + object_ids = [] + for var in entities: + added_expressions.clear() + await setup_entity(var, config, "sensor") + object_id = extract_object_id_from_expressions(added_expressions) + object_ids.append(object_id) + + # Should use device name with suffixes + assert object_ids[0] == "test_device" + assert object_ids[1] == "test_device_2" + assert object_ids[2] == "test_device_3" + + +@pytest.mark.asyncio +async def test_setup_entity_special_characters( + setup_test_environment: list[str], +) -> None: + """Test setup_entity with names containing special characters.""" + + added_expressions = setup_test_environment + + entities = [MockObj(f"sensor{i}") for i in range(3)] + + config = { + CONF_NAME: "Temperature Sensor!", + CONF_DISABLED_BY_DEFAULT: False, + } + + object_ids = [] + for var in entities: + added_expressions.clear() + await setup_entity(var, config, "sensor") + object_id = extract_object_id_from_expressions(added_expressions) + object_ids.append(object_id) + + # Special characters should be sanitized + assert object_ids[0] == "temperature_sensor_" + assert object_ids[1] == "temperature_sensor__2" + assert object_ids[2] == "temperature_sensor__3" + + +@pytest.mark.asyncio +async def test_setup_entity_with_icon(setup_test_environment: list[str]) -> None: + """Test setup_entity sets icon correctly.""" + + added_expressions = setup_test_environment + + var = MockObj("sensor1") + + config = { + CONF_NAME: "Temperature", + CONF_DISABLED_BY_DEFAULT: False, + CONF_ICON: "mdi:thermometer", + } + + await setup_entity(var, config, "sensor") + + # Check icon was set + icon_set = any( + ".set_icon(" in expr and "mdi:thermometer" in expr for expr in added_expressions + ) + assert icon_set + + +@pytest.mark.asyncio +async def test_setup_entity_disabled_by_default( + setup_test_environment: list[str], +) -> None: + """Test setup_entity sets disabled_by_default correctly.""" + + added_expressions = setup_test_environment + + var = MockObj("sensor1") + + config = { + CONF_NAME: "Temperature", + CONF_DISABLED_BY_DEFAULT: True, + } + + await setup_entity(var, config, "sensor") + + # Check disabled_by_default was set + disabled_set = any( + ".set_disabled_by_default(true)" in expr.lower() for expr in added_expressions + ) + assert disabled_set + + +@pytest.mark.asyncio +async def test_setup_entity_mixed_duplicates(setup_test_environment: list[str]) -> None: + """Test complex duplicate scenario with multiple platforms and devices.""" + + added_expressions = setup_test_environment + + # Track results + results = [] + + # 3 sensors named "Status" + for i in range(3): + added_expressions.clear() + var = MockObj(f"sensor_status_{i}") + await setup_entity( + var, {CONF_NAME: "Status", CONF_DISABLED_BY_DEFAULT: False}, "sensor" + ) + object_id = extract_object_id_from_expressions(added_expressions) + results.append(("sensor", object_id)) + + # 2 binary_sensors named "Status" + for i in range(2): + added_expressions.clear() + var = MockObj(f"binary_sensor_status_{i}") + await setup_entity( + var, {CONF_NAME: "Status", CONF_DISABLED_BY_DEFAULT: False}, "binary_sensor" + ) + object_id = extract_object_id_from_expressions(added_expressions) + results.append(("binary_sensor", object_id)) + + # 1 text_sensor named "Status" + added_expressions.clear() + var = MockObj("text_sensor_status") + await setup_entity( + var, {CONF_NAME: "Status", CONF_DISABLED_BY_DEFAULT: False}, "text_sensor" + ) + object_id = extract_object_id_from_expressions(added_expressions) + results.append(("text_sensor", object_id)) + + # Check results - each platform has its own namespace + assert results[0] == ("sensor", "status") # sensor + assert results[1] == ("sensor", "status_2") # sensor + assert results[2] == ("sensor", "status_3") # sensor + assert results[3] == ("binary_sensor", "status") # binary_sensor (new namespace) + assert results[4] == ("binary_sensor", "status_2") # binary_sensor + assert results[5] == ("text_sensor", "status") # text_sensor (new namespace) diff --git a/tests/unit_tests/test_get_base_entity_object_id.py b/tests/unit_tests/test_get_base_entity_object_id.py deleted file mode 100644 index aeea862d78..0000000000 --- a/tests/unit_tests/test_get_base_entity_object_id.py +++ /dev/null @@ -1,140 +0,0 @@ -"""Test get_base_entity_object_id function matches C++ behavior.""" - -from esphome.core import CORE -from esphome.entity import get_base_entity_object_id -from esphome.helpers import sanitize, snake_case - - -class TestGetBaseEntityObjectId: - """Test that get_base_entity_object_id matches C++ EntityBase::get_object_id behavior.""" - - def test_with_entity_name(self) -> 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" - ) - - # 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_friendly_name(self) -> None: - """Test when entity has empty name - 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(self) -> None: - """Test when entity has empty name and no friendly name - should use device name.""" - # Save original values - original_name = getattr(CORE, "name", None) - - try: - # 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" - - finally: - # Restore original value - if original_name is not None: - CORE.name = original_name - - def test_edge_cases(self) -> 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 - original_name = getattr(CORE, "name", None) - try: - CORE.name = "device" - assert get_base_entity_object_id("", "") == "device" - finally: - if original_name is not None: - CORE.name = original_name - - # 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 - - def test_matches_cpp_helpers(self) -> None: - """Test that the logic matches using snake_case and sanitize directly.""" - test_cases = [ - ("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___"), - ] - - for name, expected in test_cases: - # 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 - - # 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 - original_name = getattr(CORE, "name", None) - try: - CORE.name = "device" - assert get_base_entity_object_id("", None) == "device" # Uses device name - finally: - if original_name is not None: - CORE.name = original_name - - def test_name_add_mac_suffix_behavior(self) -> None: - """Test behavior related to name_add_mac_suffix. - - In C++, when name_add_mac_suffix is enabled and entity has no name, - get_object_id() returns str_sanitize(str_snake_case(App.get_friendly_name())) - dynamically. 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"