diff --git a/homeassistant/components/mqtt/client.py b/homeassistant/components/mqtt/client.py index e8eabe887f2..07fbc0ca8c5 100644 --- a/homeassistant/components/mqtt/client.py +++ b/homeassistant/components/mqtt/client.py @@ -36,6 +36,7 @@ from homeassistant.core import ( ) from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.dispatcher import dispatcher_send +from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue from homeassistant.helpers.typing import ConfigType from homeassistant.loader import bind_hass from homeassistant.util import dt as dt_util @@ -64,6 +65,7 @@ from .const import ( DEFAULT_WILL, DEFAULT_WS_HEADERS, DEFAULT_WS_PATH, + DOMAIN, MQTT_CONNECTED, MQTT_DISCONNECTED, PROTOCOL_5, @@ -93,6 +95,10 @@ SUBSCRIBE_COOLDOWN = 0.1 UNSUBSCRIBE_COOLDOWN = 0.1 TIMEOUT_ACK = 10 +MQTT_ENTRIES_NAMING_BLOG_URL = ( + "https://developers.home-assistant.io/blog/2023-057-21-change-naming-mqtt-entities/" +) + SubscribePayloadType = str | bytes # Only bytes if encoding is None @@ -404,6 +410,7 @@ class MQTT: @callback def ha_started(_: Event) -> None: + self.register_naming_issues() self._ha_started.set() self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STARTED, ha_started) @@ -416,6 +423,25 @@ class MQTT: hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, async_stop_mqtt) ) + def register_naming_issues(self) -> None: + """Register issues with MQTT entity naming.""" + mqtt_data = get_mqtt_data(self.hass) + for issue_key, items in mqtt_data.issues.items(): + config_list = "\n".join([f"- {item}" for item in items]) + async_create_issue( + self.hass, + DOMAIN, + issue_key, + breaks_in_ha_version="2024.2.0", + is_fixable=False, + translation_key=issue_key, + translation_placeholders={ + "config": config_list, + }, + learn_more_url=MQTT_ENTRIES_NAMING_BLOG_URL, + severity=IssueSeverity.WARNING, + ) + def start( self, mqtt_data: MqttData, diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index 9f0849a4d4c..70156703155 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -1014,6 +1014,7 @@ class MqttEntity( _attr_should_poll = False _default_name: str | None _entity_id_format: str + _issue_key: str | None def __init__( self, @@ -1027,6 +1028,7 @@ class MqttEntity( self._config: ConfigType = config self._attr_unique_id = config.get(CONF_UNIQUE_ID) self._sub_state: dict[str, EntitySubscription] = {} + self._discovery = discovery_data is not None # Load config self._setup_from_config(self._config) @@ -1050,6 +1052,7 @@ class MqttEntity( @final async def async_added_to_hass(self) -> None: """Subscribe to MQTT events.""" + self.collect_issues() await super().async_added_to_hass() self._prepare_subscribe_topics() await self._subscribe_topics() @@ -1122,6 +1125,7 @@ class MqttEntity( def _set_entity_name(self, config: ConfigType) -> None: """Help setting the entity name if needed.""" + self._issue_key = None entity_name: str | None | UndefinedType = config.get(CONF_NAME, UNDEFINED) # Only set _attr_name if it is needed if entity_name is not UNDEFINED: @@ -1130,6 +1134,7 @@ class MqttEntity( # Assign the default name self._attr_name = self._default_name if CONF_DEVICE in config: + device_name: str if CONF_NAME not in config[CONF_DEVICE]: _LOGGER.info( "MQTT device information always needs to include a name, got %s, " @@ -1137,14 +1142,47 @@ class MqttEntity( "name must be included in each entity's device configuration", config, ) - elif config[CONF_DEVICE][CONF_NAME] == entity_name: + elif (device_name := config[CONF_DEVICE][CONF_NAME]) == entity_name: + self._attr_name = None + self._issue_key = ( + "entity_name_is_device_name_discovery" + if self._discovery + else "entity_name_is_device_name_yaml" + ) _LOGGER.warning( "MQTT device name is equal to entity name in your config %s, " "this is not expected. Please correct your configuration. " "The entity name will be set to `null`", config, ) - self._attr_name = None + elif isinstance(entity_name, str) and entity_name.startswith(device_name): + self._attr_name = ( + new_entity_name := entity_name[len(device_name) :].lstrip() + ) + if device_name[:1].isupper(): + # Ensure a capital if the device name first char is a capital + new_entity_name = new_entity_name[:1].upper() + new_entity_name[1:] + self._issue_key = ( + "entity_name_startswith_device_name_discovery" + if self._discovery + else "entity_name_startswith_device_name_yaml" + ) + _LOGGER.warning( + "MQTT entity name starts with the device name in your config %s, " + "this is not expected. Please correct your configuration. " + "The device name prefix will be stripped off the entity name " + "and becomes '%s'", + config, + new_entity_name, + ) + + def collect_issues(self) -> None: + """Process issues for MQTT entities.""" + if self._issue_key is None: + return + mqtt_data = get_mqtt_data(self.hass) + issues = mqtt_data.issues.setdefault(self._issue_key, set()) + issues.add(self.entity_id) def _setup_common_attributes_from_config(self, config: ConfigType) -> None: """(Re)Setup the common attributes for the entity.""" diff --git a/homeassistant/components/mqtt/models.py b/homeassistant/components/mqtt/models.py index 5a966a4455c..9afa3de3f48 100644 --- a/homeassistant/components/mqtt/models.py +++ b/homeassistant/components/mqtt/models.py @@ -305,6 +305,7 @@ class MqttData: ) discovery_unsubscribe: list[CALLBACK_TYPE] = field(default_factory=list) integration_unsubscribe: dict[str, CALLBACK_TYPE] = field(default_factory=dict) + issues: dict[str, set[str]] = field(default_factory=dict) last_discovery: float = 0.0 reload_dispatchers: list[CALLBACK_TYPE] = field(default_factory=list) reload_handlers: dict[str, Callable[[], Coroutine[Any, Any, None]]] = field( diff --git a/homeassistant/components/mqtt/strings.json b/homeassistant/components/mqtt/strings.json index f314ddd47d3..55677798a08 100644 --- a/homeassistant/components/mqtt/strings.json +++ b/homeassistant/components/mqtt/strings.json @@ -7,6 +7,22 @@ "deprecation_mqtt_legacy_vacuum_discovery": { "title": "MQTT vacuum entities with legacy schema added through MQTT discovery", "description": "MQTT vacuum entities that use the legacy schema are deprecated, please adjust your devices to use the correct schema and restart Home Assistant to fix this issue." + }, + "entity_name_is_device_name_yaml": { + "title": "Manual configured MQTT entities with a name that is equal to the device name", + "description": "Some MQTT entities have an entity name equal to the device name. This is not expected. The entity name is set to `null` as a work-a-round to avoid a duplicate name. Please update your configuration and restart Home Assistant to fix this issue.\n\nList of affected entities:\n\n{config}" + }, + "entity_name_startswith_device_name_yaml": { + "title": "Manual configured MQTT entities with a name that starts with the device name", + "description": "Some MQTT entities have an entity name that starts with the device name. This is not expected. To avoid a duplicate name the device name prefix is stripped of the entity name as a work-a-round. Please update your configuration and restart Home Assistant to fix this issue. \n\nList of affected entities:\n\n{config}" + }, + "entity_name_is_device_name_discovery": { + "title": "Discovered MQTT entities with a name that is equal to the device name", + "description": "Some MQTT entities have an entity name equal to the device name. This is not expected. The entity name is set to `null` as a work-a-round to avoid a duplicate name. Please inform the maintainer of the software application that supplies the affected entities to fix this issue.\n\nList of affected entities:\n\n{config}" + }, + "entity_name_startswith_device_name_discovery": { + "title": "Discovered entities with a name that starts with the device name", + "description": "Some MQTT entities have an entity name that starts with the device name. This is not expected. To avoid a duplicate name the device name prefix is stripped of the entity name as a work-a-round. Please inform the maintainer of the software application that supplies the affected entities to fix this issue. \n\nList of affected entities:\n\n{config}" } }, "config": { diff --git a/tests/components/mqtt/test_mixins.py b/tests/components/mqtt/test_mixins.py index 23367d7829f..18269eb6970 100644 --- a/tests/components/mqtt/test_mixins.py +++ b/tests/components/mqtt/test_mixins.py @@ -6,14 +6,19 @@ import pytest from homeassistant.components import mqtt, sensor from homeassistant.components.mqtt.sensor import DEFAULT_NAME as DEFAULT_SENSOR_NAME -from homeassistant.const import EVENT_STATE_CHANGED, Platform -from homeassistant.core import HomeAssistant, callback +from homeassistant.const import ( + EVENT_HOMEASSISTANT_STARTED, + EVENT_STATE_CHANGED, + Platform, +) +from homeassistant.core import CoreState, HomeAssistant, callback from homeassistant.helpers import ( device_registry as dr, + issue_registry as ir, ) -from tests.common import async_fire_mqtt_message -from tests.typing import MqttMockHAClientGenerator +from tests.common import MockConfigEntry, async_capture_events, async_fire_mqtt_message +from tests.typing import MqttMockHAClientGenerator, MqttMockPahoClient @pytest.mark.parametrize( @@ -80,7 +85,14 @@ async def test_availability_with_shared_state_topic( @pytest.mark.parametrize( - ("hass_config", "entity_id", "friendly_name", "device_name", "assert_log"), + ( + "hass_config", + "entity_id", + "friendly_name", + "device_name", + "assert_log", + "issue_events", + ), [ ( # default_entity_name_without_device_name { @@ -96,6 +108,7 @@ async def test_availability_with_shared_state_topic( DEFAULT_SENSOR_NAME, None, True, + 0, ), ( # default_entity_name_with_device_name { @@ -111,6 +124,7 @@ async def test_availability_with_shared_state_topic( "Test MQTT Sensor", "Test", False, + 0, ), ( # name_follows_device_class { @@ -127,6 +141,7 @@ async def test_availability_with_shared_state_topic( "Test Humidity", "Test", False, + 0, ), ( # name_follows_device_class_without_device_name { @@ -143,6 +158,7 @@ async def test_availability_with_shared_state_topic( "Humidity", None, True, + 0, ), ( # name_overrides_device_class { @@ -160,6 +176,7 @@ async def test_availability_with_shared_state_topic( "Test MySensor", "Test", False, + 0, ), ( # name_set_no_device_name_set { @@ -177,6 +194,7 @@ async def test_availability_with_shared_state_topic( "MySensor", None, True, + 0, ), ( # none_entity_name_with_device_name { @@ -194,6 +212,7 @@ async def test_availability_with_shared_state_topic( "Test", "Test", False, + 0, ), ( # none_entity_name_without_device_name { @@ -211,8 +230,9 @@ async def test_availability_with_shared_state_topic( "mqtt veryunique", None, True, + 0, ), - ( # entity_name_and_device_name_the_sane + ( # entity_name_and_device_name_the_same { mqtt.DOMAIN: { sensor.DOMAIN: { @@ -231,6 +251,49 @@ async def test_availability_with_shared_state_topic( "Hello world", "Hello world", False, + 1, + ), + ( # entity_name_startswith_device_name1 + { + mqtt.DOMAIN: { + sensor.DOMAIN: { + "name": "World automation", + "state_topic": "test-topic", + "unique_id": "veryunique", + "device_class": "humidity", + "device": { + "identifiers": ["helloworld"], + "name": "World", + }, + } + } + }, + "sensor.world_automation", + "World automation", + "World", + False, + 1, + ), + ( # entity_name_startswith_device_name2 + { + mqtt.DOMAIN: { + sensor.DOMAIN: { + "name": "world automation", + "state_topic": "test-topic", + "unique_id": "veryunique", + "device_class": "humidity", + "device": { + "identifiers": ["helloworld"], + "name": "world", + }, + } + } + }, + "sensor.world_automation", + "world automation", + "world", + False, + 1, ), ], ids=[ @@ -242,24 +305,39 @@ async def test_availability_with_shared_state_topic( "name_set_no_device_name_set", "none_entity_name_with_device_name", "none_entity_name_without_device_name", - "entity_name_and_device_name_the_sane", + "entity_name_and_device_name_the_same", + "entity_name_startswith_device_name1", + "entity_name_startswith_device_name2", ], ) @patch("homeassistant.components.mqtt.PLATFORMS", [Platform.SENSOR]) +@patch("homeassistant.components.mqtt.client.DISCOVERY_COOLDOWN", 0.0) async def test_default_entity_and_device_name( hass: HomeAssistant, - mqtt_mock_entry: MqttMockHAClientGenerator, + mqtt_client_mock: MqttMockPahoClient, + mqtt_config_entry_data, caplog: pytest.LogCaptureFixture, entity_id: str, friendly_name: str, device_name: str | None, assert_log: bool, + issue_events: int, ) -> None: """Test device name setup with and without a device_class set. This is a test helper for the _setup_common_attributes_from_config mixin. """ - await mqtt_mock_entry() + # mqtt_mock = await mqtt_mock_entry() + + events = async_capture_events(hass, ir.EVENT_REPAIRS_ISSUE_REGISTRY_UPDATED) + hass.state = CoreState.starting + await hass.async_block_till_done() + + entry = MockConfigEntry(domain=mqtt.DOMAIN, data={mqtt.CONF_BROKER: "mock-broker"}) + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) + await hass.async_block_till_done() registry = dr.async_get(hass) @@ -274,3 +352,6 @@ async def test_default_entity_and_device_name( assert ( "MQTT device information always needs to include a name" in caplog.text ) is assert_log + + # Assert that an issues ware registered + assert len(events) == issue_events