Skip to content

Commit

Permalink
Offer work- a-round for MQTT entity names that start with the device …
Browse files Browse the repository at this point in the history 
…name (home-assistant#97495)

Co-authored-by: SukramJ <[email protected]>
Co-authored-by: Franck Nijhof <[email protected]>
  • Loading branch information
@jbouwh @SukramJ @frenck
3 people authored Aug 1, 2023
1 parent 5ce8e0e commit 9c6c391
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 11 deletions.
26 changes: 26 additions & 0 deletions homeassistant/components/mqtt/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
)
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
Expand Down Expand Up @@ -64,6 +65,7 @@
DEFAULT_WILL,
DEFAULT_WS_HEADERS,
DEFAULT_WS_PATH,
DOMAIN,
MQTT_CONNECTED,
MQTT_DISCONNECTED,
PROTOCOL_5,
Expand Down Expand Up @@ -93,6 +95,10 @@
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


Expand Down Expand Up @@ -404,6 +410,7 @@ def __init__(

@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)
Expand All @@ -416,6 +423,25 @@ async def async_stop_mqtt(_event: Event) -> None:
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,
Expand Down
42 changes: 40 additions & 2 deletions homeassistant/components/mqtt/mixins.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1027,6 +1028,7 @@ def __init__(
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)
Expand All @@ -1050,6 +1052,7 @@ def _init_entity_id(self) -> None:
@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()
Expand Down Expand Up @@ -1122,6 +1125,7 @@ def config_schema() -> vol.Schema:

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:
Expand All @@ -1130,21 +1134,55 @@ def _set_entity_name(self, config: ConfigType) -> None:
# 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, "
"if device information is shared between multiple entities, the device "
"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."""
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/mqtt/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
16 changes: 16 additions & 0 deletions homeassistant/components/mqtt/strings.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
99 changes: 90 additions & 9 deletions tests/components/mqtt/test_mixins.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@

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(
Expand Down Expand Up @@ -80,7 +85,14 @@ def test_callback(event) -> None:


@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
{
Expand All @@ -96,6 +108,7 @@ def test_callback(event) -> None:
DEFAULT_SENSOR_NAME,
None,
True,
0,
),
( # default_entity_name_with_device_name
{
Expand All @@ -111,6 +124,7 @@ def test_callback(event) -> None:
"Test MQTT Sensor",
"Test",
False,
0,
),
( # name_follows_device_class
{
Expand All @@ -127,6 +141,7 @@ def test_callback(event) -> None:
"Test Humidity",
"Test",
False,
0,
),
( # name_follows_device_class_without_device_name
{
Expand All @@ -143,6 +158,7 @@ def test_callback(event) -> None:
"Humidity",
None,
True,
0,
),
( # name_overrides_device_class
{
Expand All @@ -160,6 +176,7 @@ def test_callback(event) -> None:
"Test MySensor",
"Test",
False,
0,
),
( # name_set_no_device_name_set
{
Expand All @@ -177,6 +194,7 @@ def test_callback(event) -> None:
"MySensor",
None,
True,
0,
),
( # none_entity_name_with_device_name
{
Expand All @@ -194,6 +212,7 @@ def test_callback(event) -> None:
"Test",
"Test",
False,
0,
),
( # none_entity_name_without_device_name
{
Expand All @@ -211,8 +230,9 @@ def test_callback(event) -> None:
"mqtt veryunique",
None,
True,
0,
),
( # entity_name_and_device_name_the_sane
( # entity_name_and_device_name_the_same
{
mqtt.DOMAIN: {
sensor.DOMAIN: {
Expand All @@ -231,6 +251,49 @@ def test_callback(event) -> None:
"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=[
Expand All @@ -242,24 +305,39 @@ def test_callback(event) -> None:
"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)

Expand All @@ -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

0 comments on commit 9c6c391

Please sign in to comment.