Offer work- a-round for MQTT entity names that start with the device name (#97495)

Co-authored-by: SukramJ <sukramj@icloud.com>
Co-authored-by: Franck Nijhof <git@frenck.dev>
This commit is contained in:
Jan Bouwhuis 2023-08-01 10:03:08 +02:00 committed by GitHub
parent 5ce8e0e33e
commit 9c6c391eea
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 173 additions and 11 deletions

View file

@ -36,6 +36,7 @@ from homeassistant.core import (
) )
from homeassistant.exceptions import HomeAssistantError from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.dispatcher import dispatcher_send 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.helpers.typing import ConfigType
from homeassistant.loader import bind_hass from homeassistant.loader import bind_hass
from homeassistant.util import dt as dt_util from homeassistant.util import dt as dt_util
@ -64,6 +65,7 @@ from .const import (
DEFAULT_WILL, DEFAULT_WILL,
DEFAULT_WS_HEADERS, DEFAULT_WS_HEADERS,
DEFAULT_WS_PATH, DEFAULT_WS_PATH,
DOMAIN,
MQTT_CONNECTED, MQTT_CONNECTED,
MQTT_DISCONNECTED, MQTT_DISCONNECTED,
PROTOCOL_5, PROTOCOL_5,
@ -93,6 +95,10 @@ SUBSCRIBE_COOLDOWN = 0.1
UNSUBSCRIBE_COOLDOWN = 0.1 UNSUBSCRIBE_COOLDOWN = 0.1
TIMEOUT_ACK = 10 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 SubscribePayloadType = str | bytes # Only bytes if encoding is None
@ -404,6 +410,7 @@ class MQTT:
@callback @callback
def ha_started(_: Event) -> None: def ha_started(_: Event) -> None:
self.register_naming_issues()
self._ha_started.set() self._ha_started.set()
self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STARTED, ha_started) 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) 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( def start(
self, self,
mqtt_data: MqttData, mqtt_data: MqttData,

View file

@ -1014,6 +1014,7 @@ class MqttEntity(
_attr_should_poll = False _attr_should_poll = False
_default_name: str | None _default_name: str | None
_entity_id_format: str _entity_id_format: str
_issue_key: str | None
def __init__( def __init__(
self, self,
@ -1027,6 +1028,7 @@ class MqttEntity(
self._config: ConfigType = config self._config: ConfigType = config
self._attr_unique_id = config.get(CONF_UNIQUE_ID) self._attr_unique_id = config.get(CONF_UNIQUE_ID)
self._sub_state: dict[str, EntitySubscription] = {} self._sub_state: dict[str, EntitySubscription] = {}
self._discovery = discovery_data is not None
# Load config # Load config
self._setup_from_config(self._config) self._setup_from_config(self._config)
@ -1050,6 +1052,7 @@ class MqttEntity(
@final @final
async def async_added_to_hass(self) -> None: async def async_added_to_hass(self) -> None:
"""Subscribe to MQTT events.""" """Subscribe to MQTT events."""
self.collect_issues()
await super().async_added_to_hass() await super().async_added_to_hass()
self._prepare_subscribe_topics() self._prepare_subscribe_topics()
await self._subscribe_topics() await self._subscribe_topics()
@ -1122,6 +1125,7 @@ class MqttEntity(
def _set_entity_name(self, config: ConfigType) -> None: def _set_entity_name(self, config: ConfigType) -> None:
"""Help setting the entity name if needed.""" """Help setting the entity name if needed."""
self._issue_key = None
entity_name: str | None | UndefinedType = config.get(CONF_NAME, UNDEFINED) entity_name: str | None | UndefinedType = config.get(CONF_NAME, UNDEFINED)
# Only set _attr_name if it is needed # Only set _attr_name if it is needed
if entity_name is not UNDEFINED: if entity_name is not UNDEFINED:
@ -1130,6 +1134,7 @@ class MqttEntity(
# Assign the default name # Assign the default name
self._attr_name = self._default_name self._attr_name = self._default_name
if CONF_DEVICE in config: if CONF_DEVICE in config:
device_name: str
if CONF_NAME not in config[CONF_DEVICE]: if CONF_NAME not in config[CONF_DEVICE]:
_LOGGER.info( _LOGGER.info(
"MQTT device information always needs to include a name, got %s, " "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", "name must be included in each entity's device configuration",
config, 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( _LOGGER.warning(
"MQTT device name is equal to entity name in your config %s, " "MQTT device name is equal to entity name in your config %s, "
"this is not expected. Please correct your configuration. " "this is not expected. Please correct your configuration. "
"The entity name will be set to `null`", "The entity name will be set to `null`",
config, 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: def _setup_common_attributes_from_config(self, config: ConfigType) -> None:
"""(Re)Setup the common attributes for the entity.""" """(Re)Setup the common attributes for the entity."""

View file

@ -305,6 +305,7 @@ class MqttData:
) )
discovery_unsubscribe: list[CALLBACK_TYPE] = field(default_factory=list) discovery_unsubscribe: list[CALLBACK_TYPE] = field(default_factory=list)
integration_unsubscribe: dict[str, CALLBACK_TYPE] = field(default_factory=dict) integration_unsubscribe: dict[str, CALLBACK_TYPE] = field(default_factory=dict)
issues: dict[str, set[str]] = field(default_factory=dict)
last_discovery: float = 0.0 last_discovery: float = 0.0
reload_dispatchers: list[CALLBACK_TYPE] = field(default_factory=list) reload_dispatchers: list[CALLBACK_TYPE] = field(default_factory=list)
reload_handlers: dict[str, Callable[[], Coroutine[Any, Any, None]]] = field( reload_handlers: dict[str, Callable[[], Coroutine[Any, Any, None]]] = field(

View file

@ -7,6 +7,22 @@
"deprecation_mqtt_legacy_vacuum_discovery": { "deprecation_mqtt_legacy_vacuum_discovery": {
"title": "MQTT vacuum entities with legacy schema added through MQTT 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." "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": { "config": {

View file

@ -6,14 +6,19 @@ import pytest
from homeassistant.components import mqtt, sensor from homeassistant.components import mqtt, sensor
from homeassistant.components.mqtt.sensor import DEFAULT_NAME as DEFAULT_SENSOR_NAME from homeassistant.components.mqtt.sensor import DEFAULT_NAME as DEFAULT_SENSOR_NAME
from homeassistant.const import EVENT_STATE_CHANGED, Platform from homeassistant.const import (
from homeassistant.core import HomeAssistant, callback EVENT_HOMEASSISTANT_STARTED,
EVENT_STATE_CHANGED,
Platform,
)
from homeassistant.core import CoreState, HomeAssistant, callback
from homeassistant.helpers import ( from homeassistant.helpers import (
device_registry as dr, device_registry as dr,
issue_registry as ir,
) )
from tests.common import async_fire_mqtt_message from tests.common import MockConfigEntry, async_capture_events, async_fire_mqtt_message
from tests.typing import MqttMockHAClientGenerator from tests.typing import MqttMockHAClientGenerator, MqttMockPahoClient
@pytest.mark.parametrize( @pytest.mark.parametrize(
@ -80,7 +85,14 @@ async def test_availability_with_shared_state_topic(
@pytest.mark.parametrize( @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 ( # default_entity_name_without_device_name
{ {
@ -96,6 +108,7 @@ async def test_availability_with_shared_state_topic(
DEFAULT_SENSOR_NAME, DEFAULT_SENSOR_NAME,
None, None,
True, True,
0,
), ),
( # default_entity_name_with_device_name ( # default_entity_name_with_device_name
{ {
@ -111,6 +124,7 @@ async def test_availability_with_shared_state_topic(
"Test MQTT Sensor", "Test MQTT Sensor",
"Test", "Test",
False, False,
0,
), ),
( # name_follows_device_class ( # name_follows_device_class
{ {
@ -127,6 +141,7 @@ async def test_availability_with_shared_state_topic(
"Test Humidity", "Test Humidity",
"Test", "Test",
False, False,
0,
), ),
( # name_follows_device_class_without_device_name ( # name_follows_device_class_without_device_name
{ {
@ -143,6 +158,7 @@ async def test_availability_with_shared_state_topic(
"Humidity", "Humidity",
None, None,
True, True,
0,
), ),
( # name_overrides_device_class ( # name_overrides_device_class
{ {
@ -160,6 +176,7 @@ async def test_availability_with_shared_state_topic(
"Test MySensor", "Test MySensor",
"Test", "Test",
False, False,
0,
), ),
( # name_set_no_device_name_set ( # name_set_no_device_name_set
{ {
@ -177,6 +194,7 @@ async def test_availability_with_shared_state_topic(
"MySensor", "MySensor",
None, None,
True, True,
0,
), ),
( # none_entity_name_with_device_name ( # none_entity_name_with_device_name
{ {
@ -194,6 +212,7 @@ async def test_availability_with_shared_state_topic(
"Test", "Test",
"Test", "Test",
False, False,
0,
), ),
( # none_entity_name_without_device_name ( # none_entity_name_without_device_name
{ {
@ -211,8 +230,9 @@ async def test_availability_with_shared_state_topic(
"mqtt veryunique", "mqtt veryunique",
None, None,
True, True,
0,
), ),
( # entity_name_and_device_name_the_sane ( # entity_name_and_device_name_the_same
{ {
mqtt.DOMAIN: { mqtt.DOMAIN: {
sensor.DOMAIN: { sensor.DOMAIN: {
@ -231,6 +251,49 @@ async def test_availability_with_shared_state_topic(
"Hello world", "Hello world",
"Hello world", "Hello world",
False, 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=[ ids=[
@ -242,24 +305,39 @@ async def test_availability_with_shared_state_topic(
"name_set_no_device_name_set", "name_set_no_device_name_set",
"none_entity_name_with_device_name", "none_entity_name_with_device_name",
"none_entity_name_without_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.PLATFORMS", [Platform.SENSOR])
@patch("homeassistant.components.mqtt.client.DISCOVERY_COOLDOWN", 0.0)
async def test_default_entity_and_device_name( async def test_default_entity_and_device_name(
hass: HomeAssistant, hass: HomeAssistant,
mqtt_mock_entry: MqttMockHAClientGenerator, mqtt_client_mock: MqttMockPahoClient,
mqtt_config_entry_data,
caplog: pytest.LogCaptureFixture, caplog: pytest.LogCaptureFixture,
entity_id: str, entity_id: str,
friendly_name: str, friendly_name: str,
device_name: str | None, device_name: str | None,
assert_log: bool, assert_log: bool,
issue_events: int,
) -> None: ) -> None:
"""Test device name setup with and without a device_class set. """Test device name setup with and without a device_class set.
This is a test helper for the _setup_common_attributes_from_config mixin. 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) registry = dr.async_get(hass)
@ -274,3 +352,6 @@ async def test_default_entity_and_device_name(
assert ( assert (
"MQTT device information always needs to include a name" in caplog.text "MQTT device information always needs to include a name" in caplog.text
) is assert_log ) is assert_log
# Assert that an issues ware registered
assert len(events) == issue_events