From 8b94d41db2154688ef1f2cbb5335701ac6914b6e Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sat, 15 Jan 2022 13:02:43 +0000 Subject: [PATCH 01/28] Mqtt Notify service draft --- homeassistant/components/mqtt/__init__.py | 1 + .../components/mqtt/abbreviations.py | 2 + homeassistant/components/mqtt/discovery.py | 9 +- homeassistant/components/mqtt/mixins.py | 4 +- homeassistant/components/mqtt/notify.py | 231 ++++++++++++++++++ tests/components/mqtt/test_notify.py | 140 +++++++++++ 6 files changed, 384 insertions(+), 3 deletions(-) create mode 100644 homeassistant/components/mqtt/notify.py create mode 100644 tests/components/mqtt/test_notify.py diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index 964f19e1d7c2bc..2511f7980f00f9 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -146,6 +146,7 @@ Platform.HUMIDIFIER, Platform.LIGHT, Platform.LOCK, + Platform.NOTIFY, Platform.NUMBER, Platform.SELECT, Platform.SCENE, diff --git a/homeassistant/components/mqtt/abbreviations.py b/homeassistant/components/mqtt/abbreviations.py index 4b4c6fb7af9d4f..c79341f568f0ea 100644 --- a/homeassistant/components/mqtt/abbreviations.py +++ b/homeassistant/components/mqtt/abbreviations.py @@ -182,6 +182,8 @@ "set_fan_spd_t": "set_fan_speed_topic", "set_pos_tpl": "set_position_template", "set_pos_t": "set_position_topic", + "title": "title", + "trgt": "target", "pos_t": "position_topic", "pos_tpl": "position_template", "spd_cmd_t": "speed_command_topic", diff --git a/homeassistant/components/mqtt/discovery.py b/homeassistant/components/mqtt/discovery.py index b31d90c76f8ca9..b096343618bc2a 100644 --- a/homeassistant/components/mqtt/discovery.py +++ b/homeassistant/components/mqtt/discovery.py @@ -48,6 +48,7 @@ "humidifier", "light", "lock", + "notify", "number", "scene", "siren", @@ -232,7 +233,13 @@ async def discovery_done(_): from . import device_automation await device_automation.async_setup_entry(hass, config_entry) - elif component == "tag": + elif component in "notify": + # Local import to avoid circular dependencies + # pylint: disable=import-outside-toplevel + from . import notify + + await notify.async_setup_entry(hass, config_entry) + elif component in "tag": # Local import to avoid circular dependencies # pylint: disable=import-outside-toplevel from . import tag diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index 722bfd51c9f8eb..f72b16bc1b0b22 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -211,10 +211,10 @@ def validate_device_has_at_least_one_identifier(value: ConfigType) -> ConfigType async def async_setup_entry_helper(hass, domain, async_setup, schema): - """Set up entity, automation or tag creation dynamically through MQTT discovery.""" + """Set up entity, automation, notify service or tag creation dynamically through MQTT discovery.""" async def async_discover(discovery_payload): - """Discover and add an MQTT entity, automation or tag.""" + """Discover and add an MQTT entity, automation, notify service or tag.""" discovery_data = discovery_payload.discovery_data try: config = schema(discovery_payload) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py new file mode 100644 index 00000000000000..b2f41de0a775f1 --- /dev/null +++ b/homeassistant/components/mqtt/notify.py @@ -0,0 +1,231 @@ +"""Support for MQTT nonify.""" +from __future__ import annotations + +import functools +import logging +from typing import Any + +import voluptuous as vol + +from homeassistant.components import notify +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_NAME, CONF_SERVICE +from homeassistant.core import HomeAssistant +import homeassistant.helpers.config_validation as cv +from homeassistant.helpers.dispatcher import ( + async_dispatcher_connect, + async_dispatcher_send, +) +from homeassistant.helpers.reload import async_setup_reload_service +from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType + +from . import PLATFORMS, MqttCommandTemplate +from .. import mqtt +from .const import ( + ATTR_DISCOVERY_HASH, + ATTR_DISCOVERY_PAYLOAD, + CONF_COMMAND_TOPIC, + CONF_ENCODING, + CONF_QOS, + CONF_RETAIN, + DOMAIN, +) +from .discovery import MQTT_DISCOVERY_DONE, MQTT_DISCOVERY_UPDATED, clear_discovery_hash +from .mixins import async_setup_entry_helper + +CONF_COMMAND_TEMPLATE = "command_template" +CONF_PAYLOAD_TITLE = "Notification" +CONF_TARGET = "target" +CONF_TITLE = "title" + +MQTT_NOTIFY_TARGET_CONFIG = "mqtt_notify_target_config" + +PLATFORM_SCHEMA = mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend( + { + vol.Required(CONF_COMMAND_TOPIC): mqtt.valid_publish_topic, + vol.Optional(CONF_COMMAND_TEMPLATE): cv.template, + vol.Optional(CONF_NAME): cv.string, + vol.Required(CONF_TARGET): cv.string, + vol.Optional(CONF_TITLE, default=notify.ATTR_TITLE_DEFAULT): cv.string, + vol.Optional(CONF_RETAIN, default=mqtt.DEFAULT_RETAIN): cv.boolean, + } +) + +DISCOVERY_SCHEMA = PLATFORM_SCHEMA.extend({}, extra=vol.REMOVE_EXTRA) + +_LOGGER = logging.getLogger(__name__) + + +async def async_setup_entry( + hass: HomeAssistant, + config_entry: ConfigEntry, +) -> None: + """Set up MQTT climate device dynamically through MQTT discovery.""" + setup = functools.partial(_async_setup_notify, hass, config_entry=config_entry) + await async_setup_entry_helper(hass, notify.DOMAIN, setup, DISCOVERY_SCHEMA) + + +async def _async_setup_notify( + hass, config: ConfigType, config_entry: ConfigEntry, discovery_data: dict[str, Any] +): + """Set up the MQTT notify service with auto discovery.""" + + await _async_setup_service(hass, config) + notify_config = hass.data.setdefault( + MQTT_NOTIFY_TARGET_CONFIG, + {CONF_TARGET: {}, CONF_SERVICE: None}, + ) + # enable support for discovery updates for the new service + await notify_config[CONF_SERVICE].async_add_updater(discovery_data) + + +async def _async_setup_service( + hass: HomeAssistant, + config: ConfigType, + discovery_info: DiscoveryInfoType | None = None, +) -> None: + """Validate the service configuration setup.""" + notify_config = hass.data.setdefault( + MQTT_NOTIFY_TARGET_CONFIG, + {CONF_TARGET: {}, CONF_SERVICE: None}, + ) + if notify_config[CONF_TARGET].get(config[CONF_TARGET]): + raise ValueError( + f"Target {config[CONF_TARGET]} in config {dict(config)} is not unique, a notify service for this target has already been setup", + ) + + target_config = notify_config[CONF_TARGET][config[CONF_TARGET]] = {} + target_config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( + config.get(CONF_COMMAND_TEMPLATE), hass=hass + ).async_render + target_config[CONF_COMMAND_TOPIC] = config[CONF_COMMAND_TOPIC] + target_config[CONF_ENCODING] = config[CONF_ENCODING] + target_config[CONF_NAME] = config.get(CONF_NAME) or config[CONF_TARGET] + target_config[CONF_QOS] = config[CONF_QOS] + target_config[CONF_TITLE] = config[CONF_TITLE] + target_config[CONF_RETAIN] = config[CONF_RETAIN] + + if notify_config[CONF_SERVICE] is None: + await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + notify_config[CONF_SERVICE] = MqttNotificationService(hass) + await notify_config[CONF_SERVICE].async_setup(hass, DOMAIN, DOMAIN) + + +async def async_get_service( + hass: HomeAssistant, + config: ConfigType, + discovery_info: DiscoveryInfoType | None = None, +) -> MqttNotificationService | None: + """Prepare the MQTT notification service.""" + await _async_setup_service(hass, config) + config[CONF_NAME] = DOMAIN + return hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_SERVICE] + + +class MqttNotificationServiceUpdater: + """Add support for auto discovery updates.""" + + def __init__(self, hass: HomeAssistant, discovery_info: DiscoveryInfoType) -> None: + """Initialize the update service.""" + + async def async_discovery_update( + discovery_payload: DiscoveryInfoType | None, + ) -> None: + """Handle discovery update.""" + async_dispatcher_send( + hass, MQTT_DISCOVERY_DONE.format(self._discovery_hash), None + ) + if not discovery_payload: + # remove notify service + del hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET][self._target] + clear_discovery_hash(hass, self._discovery_hash) + self._remove_discovery() + await hass.data[MQTT_NOTIFY_TARGET_CONFIG][ + CONF_SERVICE + ].async_register_services() + _LOGGER.info( + "Notify service %s for target %s has been removed", + self._discovery_hash, + self._target, + ) + return + + # validate the schema + config = DISCOVERY_SCHEMA( + discovery_payload["discovery_data"][ATTR_DISCOVERY_PAYLOAD] + ) + await async_get_service(hass, config) + await hass.data[MQTT_NOTIFY_TARGET_CONFIG][ + CONF_SERVICE + ].async_register_services() + _LOGGER.debug( + "Notify service %s for target %s has been updated", + self._discovery_hash, + self._target, + ) + + self._discovery_hash = discovery_info[ATTR_DISCOVERY_HASH] + self._target = discovery_info[ATTR_DISCOVERY_PAYLOAD][CONF_TARGET] + self._remove_discovery = async_dispatcher_connect( + hass, + MQTT_DISCOVERY_UPDATED.format(self._discovery_hash), + async_discovery_update, + ) + _LOGGER.info( + "Notify service %s for target %s has been initialized", + self._discovery_hash, + self._target, + ) + + +class MqttNotificationService(notify.BaseNotificationService): + """Implement the notification service for E-mail messages.""" + + def __init__( + self, + hass: HomeAssistant, + ) -> None: + """Initialize the service.""" + self.hass = hass + self._config = self.hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET] + + async def async_add_updater( + self, + discovery_info: DiscoveryInfoType, + ) -> None: + """Add an update hook to support auto discovery updates.""" + discovery_hash = discovery_info[ATTR_DISCOVERY_HASH] + MqttNotificationServiceUpdater(self.hass, discovery_info) + await self.hass.data[MQTT_NOTIFY_TARGET_CONFIG][ + CONF_SERVICE + ].async_register_services() + async_dispatcher_send( + self.hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None + ) + + @property + def targets(self) -> dict[str, str]: + """Return a dictionary of registered targets.""" + return {entry[CONF_NAME]: id for id, entry in self._config.items()} + + async def async_send_message(self, message: str = "", **kwargs): + """Build and send a MQTT message to a target.""" + for target in kwargs.get(notify.ATTR_TARGET, []): + if (target_config := self._config.get(target)) is None: + continue + variables = { + "data": kwargs.get(notify.ATTR_DATA), + "message": message, + "name": target_config[CONF_NAME], + "target": target, + "title": kwargs.get(notify.ATTR_TITLE, target_config[CONF_TITLE]), + } + payload = target_config[CONF_COMMAND_TEMPLATE](message, variables=variables) + await mqtt.async_publish( + self.hass, + target_config[CONF_COMMAND_TOPIC], + payload, + target_config[CONF_QOS], + target_config[CONF_RETAIN], + target_config[CONF_ENCODING], + ) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py new file mode 100644 index 00000000000000..4c0b923d63e031 --- /dev/null +++ b/tests/components/mqtt/test_notify.py @@ -0,0 +1,140 @@ +"""The tests for the MQTT button platform.""" +import pytest + +from homeassistant.components import notify +from homeassistant.components.mqtt import DOMAIN +from homeassistant.setup import async_setup_component + +from .test_common import help_test_publishing_with_custom_encoding, help_test_reloadable + +from tests.common import async_fire_mqtt_message + +DEFAULT_CONFIG = { + notify.DOMAIN: {"platform": "mqtt", "target": "test", "command_topic": "test-topic"} +} + + +async def test_sending_mqtt_commands(hass, mqtt_mock, caplog): + """Test the sending MQTT commands.""" + assert await async_setup_component( + hass, + notify.DOMAIN, + { + notify.DOMAIN: { + "command_topic": "command-topic", + "name": "test", + "target": "test", + "platform": "mqtt", + "qos": "2", + } + }, + ) + await hass.async_block_till_done() + assert ( + "" in caplog.text + ) + assert "" in caplog.text + + await hass.services.async_call( + notify.DOMAIN, + f"{DOMAIN}_test", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, + blocking=True, + ) + + mqtt_mock.async_publish.assert_called_once_with( + "command-topic", "Message", 2, False + ) + mqtt_mock.async_publish.reset_mock() + + +async def test_discovery(hass, mqtt_mock, caplog): + """Test discovery, update and removal of notify service.""" + data = '{ "target": "test", "command_topic": "test_topic" }' + data_update = ( + '{ "target": "test", "command_topic": "test_topic_update", "name": "New name" }' + ) + + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await hass.async_block_till_done() + + assert ( + "" in caplog.text + ) + + await hass.services.async_call( + notify.DOMAIN, + f"{DOMAIN}_test", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, + blocking=True, + ) + + mqtt_mock.async_publish.assert_called_once_with("test_topic", "Message", 0, False) + mqtt_mock.async_publish.reset_mock() + + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/bla/config", data_update + ) + await hass.async_block_till_done() + + assert ( + "" + in caplog.text + ) + assert "" in caplog.text + + assert ( + "Notify service ('notify', 'bla') for target test has been updated" + in caplog.text + ) + + await hass.services.async_call( + notify.DOMAIN, + f"{DOMAIN}_new_name", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, + blocking=True, + ) + + mqtt_mock.async_publish.assert_called_once_with( + "test_topic_update", "Message", 0, False + ) + mqtt_mock.async_publish.reset_mock() + + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", "") + await hass.async_block_till_done() + + assert "" in caplog.text + + +@pytest.mark.parametrize( + "service,topic,parameters,payload,template", + [ + (notify.SERVICE_NOTIFY, "command_topic", None, "PRESS", None), + ], +) +async def test_publishing_with_custom_encoding( + hass, mqtt_mock, caplog, service, topic, parameters, payload, template +): + """Test publishing MQTT payload with different encoding.""" + domain = notify.DOMAIN + config = DEFAULT_CONFIG[domain] + + await help_test_publishing_with_custom_encoding( + hass, + mqtt_mock, + caplog, + domain, + config, + service, + topic, + parameters, + payload, + template, + ) + + +async def test_reloadable(hass, mqtt_mock, caplog, tmp_path): + """Test reloading the MQTT platform.""" + domain = notify.DOMAIN + config = DEFAULT_CONFIG[domain] + await help_test_reloadable(hass, mqtt_mock, caplog, tmp_path, domain, config) From 47beff3d13901de548676c3d53011d7197f804e4 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Fri, 21 Jan 2022 16:43:55 +0000 Subject: [PATCH 02/28] fix updates --- homeassistant/components/mqtt/notify.py | 48 +++++++++++++++---------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index b2f41de0a775f1..d80cb9f0b31a45 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -3,7 +3,7 @@ import functools import logging -from typing import Any +from typing import Any, Callable import voluptuous as vol @@ -70,7 +70,7 @@ async def _async_setup_notify( ): """Set up the MQTT notify service with auto discovery.""" - await _async_setup_service(hass, config) + await _async_prerare_service_setup(hass, config) notify_config = hass.data.setdefault( MQTT_NOTIFY_TARGET_CONFIG, {CONF_TARGET: {}, CONF_SERVICE: None}, @@ -79,7 +79,7 @@ async def _async_setup_notify( await notify_config[CONF_SERVICE].async_add_updater(discovery_data) -async def _async_setup_service( +async def _async_prerare_service_setup( hass: HomeAssistant, config: ConfigType, discovery_info: DiscoveryInfoType | None = None, @@ -89,7 +89,7 @@ async def _async_setup_service( MQTT_NOTIFY_TARGET_CONFIG, {CONF_TARGET: {}, CONF_SERVICE: None}, ) - if notify_config[CONF_TARGET].get(config[CONF_TARGET]): + if config[CONF_TARGET] in notify_config[CONF_TARGET]: raise ValueError( f"Target {config[CONF_TARGET]} in config {dict(config)} is not unique, a notify service for this target has already been setup", ) @@ -117,7 +117,7 @@ async def async_get_service( discovery_info: DiscoveryInfoType | None = None, ) -> MqttNotificationService | None: """Prepare the MQTT notification service.""" - await _async_setup_service(hass, config) + await _async_prerare_service_setup(hass, config) config[CONF_NAME] = DOMAIN return hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_SERVICE] @@ -133,40 +133,42 @@ async def async_discovery_update( ) -> None: """Handle discovery update.""" async_dispatcher_send( - hass, MQTT_DISCOVERY_DONE.format(self._discovery_hash), None + hass, MQTT_DISCOVERY_DONE.format(self.discovery_hash), None ) if not discovery_payload: # remove notify service - del hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET][self._target] - clear_discovery_hash(hass, self._discovery_hash) + del hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET][self.target] + clear_discovery_hash(hass, self.discovery_hash) self._remove_discovery() await hass.data[MQTT_NOTIFY_TARGET_CONFIG][ CONF_SERVICE ].async_register_services() _LOGGER.info( "Notify service %s for target %s has been removed", - self._discovery_hash, - self._target, + self.discovery_hash, + self.target, ) return # validate the schema - config = DISCOVERY_SCHEMA( - discovery_payload["discovery_data"][ATTR_DISCOVERY_PAYLOAD] - ) - await async_get_service(hass, config) + config = DISCOVERY_SCHEMA(discovery_payload) + if config[CONF_TARGET] in hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET]: + # remove the current old configuration and process the updated config + del hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET][self.target] + self._target = config[CONF_TARGET] + await _async_prerare_service_setup(hass, config) await hass.data[MQTT_NOTIFY_TARGET_CONFIG][ CONF_SERVICE ].async_register_services() _LOGGER.debug( "Notify service %s for target %s has been updated", - self._discovery_hash, - self._target, + self.discovery_hash, + self.target, ) self._discovery_hash = discovery_info[ATTR_DISCOVERY_HASH] self._target = discovery_info[ATTR_DISCOVERY_PAYLOAD][CONF_TARGET] - self._remove_discovery = async_dispatcher_connect( + self._remove_discovery: Callable = async_dispatcher_connect( hass, MQTT_DISCOVERY_UPDATED.format(self._discovery_hash), async_discovery_update, @@ -177,9 +179,19 @@ async def async_discovery_update( self._target, ) + @property + def target(self) -> str: + """Return the target name.""" + return self._target + + @property + def discovery_hash(self) -> str: + """Return the discovery hash.""" + return self._discovery_hash + class MqttNotificationService(notify.BaseNotificationService): - """Implement the notification service for E-mail messages.""" + """Implement the notification service for MQTT.""" def __init__( self, From a99481b8423afe6a46cd55559e1fa016318e9deb Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sat, 22 Jan 2022 21:42:41 +0000 Subject: [PATCH 03/28] Remove TARGET config parameter --- homeassistant/components/mqtt/notify.py | 246 ++++++++++++------------ tests/components/mqtt/test_notify.py | 142 +++++++++++--- 2 files changed, 238 insertions(+), 150 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index d80cb9f0b31a45..84759725fb54e8 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -1,15 +1,15 @@ -"""Support for MQTT nonify.""" +"""Support for MQTT notify.""" from __future__ import annotations import functools import logging -from typing import Any, Callable +from typing import Any import voluptuous as vol from homeassistant.components import notify from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_NAME, CONF_SERVICE +from homeassistant.const import CONF_NAME from homeassistant.core import HomeAssistant import homeassistant.helpers.config_validation as cv from homeassistant.helpers.dispatcher import ( @@ -18,6 +18,7 @@ ) from homeassistant.helpers.reload import async_setup_reload_service from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.util import slugify from . import PLATFORMS, MqttCommandTemplate from .. import mqtt @@ -35,9 +36,10 @@ CONF_COMMAND_TEMPLATE = "command_template" CONF_PAYLOAD_TITLE = "Notification" -CONF_TARGET = "target" CONF_TITLE = "title" +MQTT_EVENT_RELOADED = "event_{}_reloaded" + MQTT_NOTIFY_TARGET_CONFIG = "mqtt_notify_target_config" PLATFORM_SCHEMA = mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend( @@ -45,7 +47,6 @@ vol.Required(CONF_COMMAND_TOPIC): mqtt.valid_publish_topic, vol.Optional(CONF_COMMAND_TEMPLATE): cv.template, vol.Optional(CONF_NAME): cv.string, - vol.Required(CONF_TARGET): cv.string, vol.Optional(CONF_TITLE, default=notify.ATTR_TITLE_DEFAULT): cv.string, vol.Optional(CONF_RETAIN, default=mqtt.DEFAULT_RETAIN): cv.boolean, } @@ -61,54 +62,35 @@ async def async_setup_entry( config_entry: ConfigEntry, ) -> None: """Set up MQTT climate device dynamically through MQTT discovery.""" + await async_setup_reload_service(hass, DOMAIN, PLATFORMS) setup = functools.partial(_async_setup_notify, hass, config_entry=config_entry) await async_setup_entry_helper(hass, notify.DOMAIN, setup, DISCOVERY_SCHEMA) async def _async_setup_notify( - hass, config: ConfigType, config_entry: ConfigEntry, discovery_data: dict[str, Any] + hass, + legacy_config: ConfigType, + config_entry: ConfigEntry, + discovery_data: dict[str, Any], ): """Set up the MQTT notify service with auto discovery.""" - - await _async_prerare_service_setup(hass, config) - notify_config = hass.data.setdefault( - MQTT_NOTIFY_TARGET_CONFIG, - {CONF_TARGET: {}, CONF_SERVICE: None}, + config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) + discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] + service = MqttNotificationService( + hass, + config[CONF_COMMAND_TOPIC], + MqttCommandTemplate(config.get(CONF_COMMAND_TEMPLATE), hass=hass), + config[CONF_ENCODING], + config.get(CONF_NAME), + config[CONF_QOS], + config[CONF_RETAIN], + config[CONF_TITLE], + discovery_hash=discovery_hash, ) - # enable support for discovery updates for the new service - await notify_config[CONF_SERVICE].async_add_updater(discovery_data) - - -async def _async_prerare_service_setup( - hass: HomeAssistant, - config: ConfigType, - discovery_info: DiscoveryInfoType | None = None, -) -> None: - """Validate the service configuration setup.""" - notify_config = hass.data.setdefault( - MQTT_NOTIFY_TARGET_CONFIG, - {CONF_TARGET: {}, CONF_SERVICE: None}, + await service.async_setup( + hass, slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])), "" ) - if config[CONF_TARGET] in notify_config[CONF_TARGET]: - raise ValueError( - f"Target {config[CONF_TARGET]} in config {dict(config)} is not unique, a notify service for this target has already been setup", - ) - - target_config = notify_config[CONF_TARGET][config[CONF_TARGET]] = {} - target_config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( - config.get(CONF_COMMAND_TEMPLATE), hass=hass - ).async_render - target_config[CONF_COMMAND_TOPIC] = config[CONF_COMMAND_TOPIC] - target_config[CONF_ENCODING] = config[CONF_ENCODING] - target_config[CONF_NAME] = config.get(CONF_NAME) or config[CONF_TARGET] - target_config[CONF_QOS] = config[CONF_QOS] - target_config[CONF_TITLE] = config[CONF_TITLE] - target_config[CONF_RETAIN] = config[CONF_RETAIN] - - if notify_config[CONF_SERVICE] is None: - await async_setup_reload_service(hass, DOMAIN, PLATFORMS) - notify_config[CONF_SERVICE] = MqttNotificationService(hass) - await notify_config[CONF_SERVICE].async_setup(hass, DOMAIN, DOMAIN) + await service.async_register_services() async def async_get_service( @@ -116,16 +98,24 @@ async def async_get_service( config: ConfigType, discovery_info: DiscoveryInfoType | None = None, ) -> MqttNotificationService | None: - """Prepare the MQTT notification service.""" - await _async_prerare_service_setup(hass, config) - config[CONF_NAME] = DOMAIN - return hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_SERVICE] + """Prepare the MQTT notification service through configuration.yaml.""" + await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + return MqttNotificationService( + hass, + config[CONF_COMMAND_TOPIC], + MqttCommandTemplate(config.get(CONF_COMMAND_TEMPLATE), hass=hass), + config[CONF_ENCODING], + config.get(CONF_NAME), + config[CONF_QOS], + config[CONF_RETAIN], + config[CONF_TITLE], + ) class MqttNotificationServiceUpdater: """Add support for auto discovery updates.""" - def __init__(self, hass: HomeAssistant, discovery_info: DiscoveryInfoType) -> None: + def __init__(self, hass: HomeAssistant, service: MqttNotificationService) -> None: """Initialize the update service.""" async def async_discovery_update( @@ -133,62 +123,41 @@ async def async_discovery_update( ) -> None: """Handle discovery update.""" async_dispatcher_send( - hass, MQTT_DISCOVERY_DONE.format(self.discovery_hash), None + hass, MQTT_DISCOVERY_DONE.format(service.discovery_hash), None ) if not discovery_payload: - # remove notify service - del hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET][self.target] - clear_discovery_hash(hass, self.discovery_hash) + # unregister notify service through auto discovery + clear_discovery_hash(hass, service.discovery_hash) self._remove_discovery() - await hass.data[MQTT_NOTIFY_TARGET_CONFIG][ - CONF_SERVICE - ].async_register_services() + await service.async_unregister_services() _LOGGER.info( - "Notify service %s for target %s has been removed", - self.discovery_hash, - self.target, + "Notify service %s has been removed", + service.discovery_hash, ) + del self._service return - # validate the schema - config = DISCOVERY_SCHEMA(discovery_payload) - if config[CONF_TARGET] in hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET]: - # remove the current old configuration and process the updated config - del hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET][self.target] - self._target = config[CONF_TARGET] - await _async_prerare_service_setup(hass, config) - await hass.data[MQTT_NOTIFY_TARGET_CONFIG][ - CONF_SERVICE - ].async_register_services() + # update notify service through auto discovery + await service.async_update_service(discovery_payload) _LOGGER.debug( - "Notify service %s for target %s has been updated", - self.discovery_hash, - self.target, + "Notify service %s has been updated", + service.discovery_hash, ) - self._discovery_hash = discovery_info[ATTR_DISCOVERY_HASH] - self._target = discovery_info[ATTR_DISCOVERY_PAYLOAD][CONF_TARGET] - self._remove_discovery: Callable = async_dispatcher_connect( + self._service = service + self._remove_discovery = async_dispatcher_connect( hass, - MQTT_DISCOVERY_UPDATED.format(self._discovery_hash), + MQTT_DISCOVERY_UPDATED.format(service.discovery_hash), async_discovery_update, ) + async_dispatcher_send( + hass, MQTT_DISCOVERY_DONE.format(service.discovery_hash), None + ) _LOGGER.info( - "Notify service %s for target %s has been initialized", - self._discovery_hash, - self._target, + "Notify service %s has been initialized", + service.discovery_hash, ) - @property - def target(self) -> str: - """Return the target name.""" - return self._target - - @property - def discovery_hash(self) -> str: - """Return the discovery hash.""" - return self._discovery_hash - class MqttNotificationService(notify.BaseNotificationService): """Implement the notification service for MQTT.""" @@ -196,48 +165,77 @@ class MqttNotificationService(notify.BaseNotificationService): def __init__( self, hass: HomeAssistant, + command_topic: str, + command_template: MqttCommandTemplate, + encoding: str, + name: str | None, + qos: int, + retain: bool, + title: str | None, + discovery_hash: tuple | None = None, ) -> None: """Initialize the service.""" self.hass = hass - self._config = self.hass.data[MQTT_NOTIFY_TARGET_CONFIG][CONF_TARGET] - - async def async_add_updater( - self, - discovery_info: DiscoveryInfoType, - ) -> None: - """Add an update hook to support auto discovery updates.""" - discovery_hash = discovery_info[ATTR_DISCOVERY_HASH] - MqttNotificationServiceUpdater(self.hass, discovery_info) - await self.hass.data[MQTT_NOTIFY_TARGET_CONFIG][ - CONF_SERVICE - ].async_register_services() - async_dispatcher_send( - self.hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None + self._command_topic = command_topic + self._command_template = command_template + self._encoding = encoding + self._name = name + self._qos = qos + self._retain = retain + self._title = title + self._discovery_hash = discovery_hash + self._service_name = slugify(name or command_topic) + + self._updater = ( + MqttNotificationServiceUpdater(hass, self) if discovery_hash else None ) @property - def targets(self) -> dict[str, str]: - """Return a dictionary of registered targets.""" - return {entry[CONF_NAME]: id for id, entry in self._config.items()} + def discovery_hash(self) -> tuple | None: + """Return the discovery hash.""" + return self._discovery_hash + + async def async_update_service( + self, + discovery_payload: DiscoveryInfoType, + ) -> None: + """Update the notify service through auto discovery.""" + config = DISCOVERY_SCHEMA(discovery_payload) + self._command_topic = config[CONF_COMMAND_TOPIC] + if config.get(CONF_COMMAND_TEMPLATE) is not None: + template = config[CONF_COMMAND_TEMPLATE] + template.hass = self.hass + self._command_template._attr_command_template = template + else: + self._command_template._attr_command_template = None + self._encoding = config[CONF_ENCODING] + self._name = config.get(CONF_NAME) + self._qos = config[CONF_QOS] + self._retain = config[CONF_RETAIN] + self._title = config[CONF_TITLE] + new_service_name = slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])) + if new_service_name != self._service_name: + await self.async_unregister_services() + self._service_name = new_service_name + await self.async_register_services() async def async_send_message(self, message: str = "", **kwargs): - """Build and send a MQTT message to a target.""" - for target in kwargs.get(notify.ATTR_TARGET, []): - if (target_config := self._config.get(target)) is None: - continue - variables = { + """Build and send a MQTT message.""" + payload = self._command_template.async_render( + message, + variables={ "data": kwargs.get(notify.ATTR_DATA), "message": message, - "name": target_config[CONF_NAME], - "target": target, - "title": kwargs.get(notify.ATTR_TITLE, target_config[CONF_TITLE]), - } - payload = target_config[CONF_COMMAND_TEMPLATE](message, variables=variables) - await mqtt.async_publish( - self.hass, - target_config[CONF_COMMAND_TOPIC], - payload, - target_config[CONF_QOS], - target_config[CONF_RETAIN], - target_config[CONF_ENCODING], - ) + "name": self._name, + "service": self._service_name, + "title": kwargs.get(notify.ATTR_TITLE, self._title), + }, + ) + await mqtt.async_publish( + self.hass, + self._command_topic, + payload, + self._qos, + self._retain, + self._encoding, + ) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index 4c0b923d63e031..673e12fbd2646e 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -1,12 +1,15 @@ """The tests for the MQTT button platform.""" -import pytest +import copy +from unittest.mock import patch +import yaml + +from homeassistant import config as hass_config from homeassistant.components import notify from homeassistant.components.mqtt import DOMAIN +from homeassistant.const import SERVICE_RELOAD from homeassistant.setup import async_setup_component -from .test_common import help_test_publishing_with_custom_encoding, help_test_reloadable - from tests.common import async_fire_mqtt_message DEFAULT_CONFIG = { @@ -106,35 +109,122 @@ async def test_discovery(hass, mqtt_mock, caplog): assert "" in caplog.text -@pytest.mark.parametrize( - "service,topic,parameters,payload,template", - [ - (notify.SERVICE_NOTIFY, "command_topic", None, "PRESS", None), - ], -) -async def test_publishing_with_custom_encoding( - hass, mqtt_mock, caplog, service, topic, parameters, payload, template -): +async def test_publishing_with_custom_encoding(hass, mqtt_mock, caplog): """Test publishing MQTT payload with different encoding.""" - domain = notify.DOMAIN - config = DEFAULT_CONFIG[domain] - - await help_test_publishing_with_custom_encoding( + assert await async_setup_component( hass, - mqtt_mock, - caplog, - domain, - config, - service, - topic, - parameters, - payload, - template, + notify.DOMAIN, + { + notify.DOMAIN: { + "command_topic": "command-topic", + "name": "test", + "target": "test", + "platform": "mqtt", + "qos": "2", + } + }, + ) + await hass.async_block_till_done() + + data = '{"target": "test", "command_topic": "test_topic", "command_template": "{{ pack(int(message), \'b\') }}" }' + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await hass.async_block_till_done() + + assert ( + "" in caplog.text ) + # test raw payload + await hass.services.async_call( + notify.DOMAIN, + f"{DOMAIN}_test", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "4"}, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with("test_topic", b"\x04", 0, False) + mqtt_mock.async_publish.reset_mock() + + # test with utf-16 + data = '{"encoding":"utf-16", "target": "test", "command_topic": "test_topic", "command_template": "{{ message }}" }' + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await hass.async_block_till_done() + + await hass.services.async_call( + notify.DOMAIN, + f"{DOMAIN}_test", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with( + "test_topic", "Message".encode("utf-16"), 0, False + ) + mqtt_mock.async_publish.reset_mock() + async def test_reloadable(hass, mqtt_mock, caplog, tmp_path): """Test reloading the MQTT platform.""" domain = notify.DOMAIN config = DEFAULT_CONFIG[domain] - await help_test_reloadable(hass, mqtt_mock, caplog, tmp_path, domain, config) + + # Create and test an old config of 2 entities based on the config supplied + old_config_1 = copy.deepcopy(config) + old_config_1["name"] = "test_old_1" + old_config_1["target"] = "test_old_1" + old_config_2 = copy.deepcopy(config) + old_config_2["name"] = "test_old_2" + old_config_2["target"] = "test_old_2" + + assert await async_setup_component(hass, domain, {domain: old_config_1}) + await hass.async_block_till_done() + assert ( + "" + in caplog.text + ) + assert await async_setup_component(hass, domain, {domain: old_config_2}) + assert ( + "" + in caplog.text + ) + caplog.clear() + + # Create temporary fixture for configuration.yaml based on the supplied config and test a reload with this new config + new_config_1 = copy.deepcopy(config) + new_config_1["name"] = "test_new_1" + new_config_1["target"] = "test_new_1" + new_config_2 = copy.deepcopy(config) + new_config_2["name"] = "test_new_2" + new_config_2["target"] = "test_new_2" + new_config_3 = copy.deepcopy(config) + new_config_3["name"] = "test_new_3" + new_config_3["target"] = "test_new_3" + new_yaml_config_file = tmp_path / "configuration.yaml" + new_yaml_config = yaml.dump({domain: [new_config_1, new_config_2, new_config_3]}) + new_yaml_config_file.write_text(new_yaml_config) + assert new_yaml_config_file.read_text() == new_yaml_config + + with patch.object(hass_config, "YAML_CONFIG_FILE", new_yaml_config_file): + await hass.services.async_call( + "mqtt", + SERVICE_RELOAD, + {}, + blocking=True, + ) + await hass.async_block_till_done() + + assert "" in caplog.text + + assert len(hass.states.async_all(domain)) == 3 + + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + caplog.clear() From 9298aece8c7c5021db8625c633759df6fee3965a Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sat, 22 Jan 2022 21:47:15 +0000 Subject: [PATCH 04/28] do not use protected attributes --- homeassistant/components/mqtt/notify.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 84759725fb54e8..32df15273e28a3 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -202,12 +202,9 @@ async def async_update_service( """Update the notify service through auto discovery.""" config = DISCOVERY_SCHEMA(discovery_payload) self._command_topic = config[CONF_COMMAND_TOPIC] - if config.get(CONF_COMMAND_TEMPLATE) is not None: - template = config[CONF_COMMAND_TEMPLATE] - template.hass = self.hass - self._command_template._attr_command_template = template - else: - self._command_template._attr_command_template = None + self._command_template = MqttCommandTemplate( + config.get(CONF_COMMAND_TEMPLATE), hass=self.hass + ) self._encoding = config[CONF_ENCODING] self._name = config.get(CONF_NAME) self._qos = config[CONF_QOS] From 9c83c043f3f373e36ca3aadddfd0739fc779d376 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sat, 22 Jan 2022 22:31:36 +0000 Subject: [PATCH 05/28] complete tests --- tests/components/mqtt/test_notify.py | 141 ++++++++++++++++----------- 1 file changed, 83 insertions(+), 58 deletions(-) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index 673e12fbd2646e..e7befb962a863c 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -12,9 +12,7 @@ from tests.common import async_fire_mqtt_message -DEFAULT_CONFIG = { - notify.DOMAIN: {"platform": "mqtt", "target": "test", "command_topic": "test-topic"} -} +DEFAULT_CONFIG = {notify.DOMAIN: {"platform": "mqtt", "command_topic": "test-topic"}} async def test_sending_mqtt_commands(hass, mqtt_mock, caplog): @@ -26,21 +24,17 @@ async def test_sending_mqtt_commands(hass, mqtt_mock, caplog): notify.DOMAIN: { "command_topic": "command-topic", "name": "test", - "target": "test", "platform": "mqtt", "qos": "2", } }, ) await hass.async_block_till_done() - assert ( - "" in caplog.text - ) - assert "" in caplog.text + assert "" in caplog.text await hass.services.async_call( notify.DOMAIN, - f"{DOMAIN}_test", + "test", {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, blocking=True, ) @@ -53,21 +47,19 @@ async def test_sending_mqtt_commands(hass, mqtt_mock, caplog): async def test_discovery(hass, mqtt_mock, caplog): """Test discovery, update and removal of notify service.""" - data = '{ "target": "test", "command_topic": "test_topic" }' - data_update = ( - '{ "target": "test", "command_topic": "test_topic_update", "name": "New name" }' - ) + data = '{ "name": "Old name", "command_topic": "test_topic" }' + data_update = '{ "command_topic": "test_topic_update", "name": "New name" }' async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) await hass.async_block_till_done() assert ( - "" in caplog.text + "" in caplog.text ) await hass.services.async_call( notify.DOMAIN, - f"{DOMAIN}_test", + "old_name", {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, blocking=True, ) @@ -80,20 +72,16 @@ async def test_discovery(hass, mqtt_mock, caplog): ) await hass.async_block_till_done() + assert "" in caplog.text assert ( - "" - in caplog.text + "" in caplog.text ) - assert "" in caplog.text - assert ( - "Notify service ('notify', 'bla') for target test has been updated" - in caplog.text - ) + assert "Notify service ('notify', 'bla') has been updated" in caplog.text await hass.services.async_call( notify.DOMAIN, - f"{DOMAIN}_new_name", + "new_name", {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, blocking=True, ) @@ -106,11 +94,12 @@ async def test_discovery(hass, mqtt_mock, caplog): async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", "") await hass.async_block_till_done() - assert "" in caplog.text + assert "" in caplog.text async def test_publishing_with_custom_encoding(hass, mqtt_mock, caplog): - """Test publishing MQTT payload with different encoding.""" + """Test publishing MQTT payload with different encoding via discovery and configuration.""" + # test with default encoding using configuration setup assert await async_setup_component( hass, notify.DOMAIN, @@ -118,7 +107,6 @@ async def test_publishing_with_custom_encoding(hass, mqtt_mock, caplog): notify.DOMAIN: { "command_topic": "command-topic", "name": "test", - "target": "test", "platform": "mqtt", "qos": "2", } @@ -126,40 +114,48 @@ async def test_publishing_with_custom_encoding(hass, mqtt_mock, caplog): ) await hass.async_block_till_done() - data = '{"target": "test", "command_topic": "test_topic", "command_template": "{{ pack(int(message), \'b\') }}" }' + # test with raw encoding and discovery + data = '{"name": "test2", "command_topic": "test_topic2", "command_template": "{{ pack(int(message), \'b\') }}" }' async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) await hass.async_block_till_done() - assert ( - "" in caplog.text - ) + assert "Notify service ('notify', 'bla') has been initialized" in caplog.text + assert "" in caplog.text - # test raw payload await hass.services.async_call( notify.DOMAIN, - f"{DOMAIN}_test", + "test2", {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "4"}, blocking=True, ) - mqtt_mock.async_publish.assert_called_once_with("test_topic", b"\x04", 0, False) + mqtt_mock.async_publish.assert_called_once_with("test_topic2", b"\x04", 0, False) mqtt_mock.async_publish.reset_mock() - # test with utf-16 - data = '{"encoding":"utf-16", "target": "test", "command_topic": "test_topic", "command_template": "{{ message }}" }' + # test with utf-16 and update discovery + data = '{"encoding":"utf-16", "name": "test3", "command_topic": "test_topic3", "command_template": "{{ message }}" }' async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) await hass.async_block_till_done() + assert ( + "Component has already been discovered: notify bla, sending update" + in caplog.text + ) await hass.services.async_call( notify.DOMAIN, - f"{DOMAIN}_test", + "test3", {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, blocking=True, ) mqtt_mock.async_publish.assert_called_once_with( - "test_topic", "Message".encode("utf-16"), 0, False + "test_topic3", "Message".encode("utf-16"), 0, False ) mqtt_mock.async_publish.reset_mock() + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", "") + await hass.async_block_till_done() + + assert "Notify service ('notify', 'bla') has been removed" in caplog.text + async def test_reloadable(hass, mqtt_mock, caplog, tmp_path): """Test reloading the MQTT platform.""" @@ -168,35 +164,42 @@ async def test_reloadable(hass, mqtt_mock, caplog, tmp_path): # Create and test an old config of 2 entities based on the config supplied old_config_1 = copy.deepcopy(config) - old_config_1["name"] = "test_old_1" - old_config_1["target"] = "test_old_1" + old_config_1["name"] = "Test old 1" old_config_2 = copy.deepcopy(config) - old_config_2["name"] = "test_old_2" - old_config_2["target"] = "test_old_2" + old_config_2["name"] = "Test old 2" - assert await async_setup_component(hass, domain, {domain: old_config_1}) + assert await async_setup_component( + hass, domain, {domain: [old_config_1, old_config_2]} + ) await hass.async_block_till_done() assert ( - "" + "" in caplog.text ) - assert await async_setup_component(hass, domain, {domain: old_config_2}) assert ( - "" + "" in caplog.text ) caplog.clear() + # Add an auto discovered notify target + data = '{"name": "Test old 3", "command_topic": "test_topic_discovery" }' + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await hass.async_block_till_done() + + assert "Notify service ('notify', 'bla') has been initialized" in caplog.text + assert ( + "" + in caplog.text + ) + # Create temporary fixture for configuration.yaml based on the supplied config and test a reload with this new config new_config_1 = copy.deepcopy(config) - new_config_1["name"] = "test_new_1" - new_config_1["target"] = "test_new_1" + new_config_1["name"] = "Test new 1" new_config_2 = copy.deepcopy(config) - new_config_2["name"] = "test_new_2" - new_config_2["target"] = "test_new_2" + new_config_2["name"] = "test new 2" new_config_3 = copy.deepcopy(config) - new_config_3["name"] = "test_new_3" - new_config_3["target"] = "test_new_3" + new_config_3["name"] = "test new 3" new_yaml_config_file = tmp_path / "configuration.yaml" new_yaml_config = yaml.dump({domain: [new_config_1, new_config_2, new_config_3]}) new_yaml_config_file.write_text(new_yaml_config) @@ -204,27 +207,49 @@ async def test_reloadable(hass, mqtt_mock, caplog, tmp_path): with patch.object(hass_config, "YAML_CONFIG_FILE", new_yaml_config_file): await hass.services.async_call( - "mqtt", + DOMAIN, SERVICE_RELOAD, {}, blocking=True, ) await hass.async_block_till_done() - assert "" in caplog.text - - assert len(hass.states.async_all(domain)) == 3 + assert ( + "" in caplog.text + ) + assert ( + "" in caplog.text + ) assert ( - "" + "" in caplog.text ) assert ( - "" + "" in caplog.text ) assert ( - "" + "" in caplog.text ) + assert "" in caplog.text caplog.clear() + + # test if the auto discovered item survived the platform reload + await hass.services.async_call( + notify.DOMAIN, + "test_old_3", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with( + "test_topic_discovery", "Message", 0, False + ) + + mqtt_mock.async_publish.reset_mock() + + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", "") + await hass.async_block_till_done() + + assert "Notify service ('notify', 'bla') has been removed" in caplog.text From c589a7b9c79e2d47f2468d773d390ded36faa6cc Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 23 Jan 2022 00:57:03 +0000 Subject: [PATCH 06/28] device support for auto discovery --- homeassistant/components/mqtt/notify.py | 90 ++++++++++++++++++++++--- 1 file changed, 79 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 32df15273e28a3..b238dc906f02f0 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -9,9 +9,10 @@ from homeassistant.components import notify from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_NAME +from homeassistant.const import CONF_DEVICE, CONF_NAME from homeassistant.core import HomeAssistant import homeassistant.helpers.config_validation as cv +from homeassistant.helpers.device_registry import EVENT_DEVICE_REGISTRY_UPDATED from homeassistant.helpers.dispatcher import ( async_dispatcher_connect, async_dispatcher_send, @@ -32,10 +33,16 @@ DOMAIN, ) from .discovery import MQTT_DISCOVERY_DONE, MQTT_DISCOVERY_UPDATED, clear_discovery_hash -from .mixins import async_setup_entry_helper +from .mixins import ( + CONF_CONNECTIONS, + CONF_IDENTIFIERS, + MQTT_ENTITY_DEVICE_INFO_SCHEMA, + async_setup_entry_helper, + cleanup_device_registry, + device_info_from_config, +) CONF_COMMAND_TEMPLATE = "command_template" -CONF_PAYLOAD_TITLE = "Notification" CONF_TITLE = "title" MQTT_EVENT_RELOADED = "event_{}_reloaded" @@ -46,6 +53,7 @@ { vol.Required(CONF_COMMAND_TOPIC): mqtt.valid_publish_topic, vol.Optional(CONF_COMMAND_TEMPLATE): cv.template, + vol.Optional(CONF_DEVICE): MQTT_ENTITY_DEVICE_INFO_SCHEMA, vol.Optional(CONF_NAME): cv.string, vol.Optional(CONF_TITLE, default=notify.ATTR_TITLE_DEFAULT): cv.string, vol.Optional(CONF_RETAIN, default=mqtt.DEFAULT_RETAIN): cv.boolean, @@ -76,6 +84,18 @@ async def _async_setup_notify( """Set up the MQTT notify service with auto discovery.""" config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] + device_id = None + if CONF_DEVICE in config: + await _update_device(hass, config_entry, config) + + device_registry = await hass.helpers.device_registry.async_get_registry() + device = device_registry.async_get_device( + {(DOMAIN, id_) for id_ in config[CONF_DEVICE][CONF_IDENTIFIERS]}, + {tuple(x) for x in config[CONF_DEVICE][CONF_CONNECTIONS]}, + ) + + device_id = device.id + service = MqttNotificationService( hass, config[CONF_COMMAND_TOPIC], @@ -86,6 +106,8 @@ async def _async_setup_notify( config[CONF_RETAIN], config[CONF_TITLE], discovery_hash=discovery_hash, + device_id=device_id, + config_entry=config_entry, ) await service.async_setup( hass, slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])), "" @@ -127,14 +149,7 @@ async def async_discovery_update( ) if not discovery_payload: # unregister notify service through auto discovery - clear_discovery_hash(hass, service.discovery_hash) - self._remove_discovery() - await service.async_unregister_services() - _LOGGER.info( - "Notify service %s has been removed", - service.discovery_hash, - ) - del self._service + await async_tear_down_service() return # update notify service through auto discovery @@ -144,12 +159,43 @@ async def async_discovery_update( service.discovery_hash, ) + async def async_device_removed(event): + """Handle the removal of a device.""" + device_id = event.data["device_id"] + if ( + event.data["action"] != "remove" + or device_id != service.device_id + or self._device_removed + ): + return + self._device_removed = True + await async_tear_down_service() + + async def async_tear_down_service(): + """Handle the removal of the service.""" + if not self._device_removed and service.device_id: + self._device_removed = True + await cleanup_device_registry(hass, service.device_id) + clear_discovery_hash(hass, service.discovery_hash) + self._remove_discovery() + await service.async_unregister_services() + _LOGGER.info( + "Notify service %s has been removed", + service.discovery_hash, + ) + del self._service + self._service = service self._remove_discovery = async_dispatcher_connect( hass, MQTT_DISCOVERY_UPDATED.format(service.discovery_hash), async_discovery_update, ) + if service.device_id: + self._remove_device_updated = hass.bus.async_listen( + EVENT_DEVICE_REGISTRY_UPDATED, async_device_removed + ) + self._device_removed = False async_dispatcher_send( hass, MQTT_DISCOVERY_DONE.format(service.discovery_hash), None ) @@ -173,6 +219,8 @@ def __init__( retain: bool, title: str | None, discovery_hash: tuple | None = None, + device_id: str | None = None, + config_entry: ConfigEntry | None = None, ) -> None: """Initialize the service.""" self.hass = hass @@ -184,6 +232,8 @@ def __init__( self._retain = retain self._title = title self._discovery_hash = discovery_hash + self._device_id = device_id + self._config_entry = config_entry self._service_name = slugify(name or command_topic) self._updater = ( @@ -195,6 +245,11 @@ def discovery_hash(self) -> tuple | None: """Return the discovery hash.""" return self._discovery_hash + @property + def device_id(self) -> str | None: + """Return the device ID.""" + return self._device_id + async def async_update_service( self, discovery_payload: DiscoveryInfoType, @@ -215,6 +270,8 @@ async def async_update_service( await self.async_unregister_services() self._service_name = new_service_name await self.async_register_services() + if self.device_id: + await _update_device(self.hass, self._config_entry, config) async def async_send_message(self, message: str = "", **kwargs): """Build and send a MQTT message.""" @@ -236,3 +293,14 @@ async def async_send_message(self, message: str = "", **kwargs): self._retain, self._encoding, ) + + +async def _update_device(hass, config_entry, config): + """Update device registry.""" + device_registry = await hass.helpers.device_registry.async_get_registry() + config_entry_id = config_entry.entry_id + device_info = device_info_from_config(config[CONF_DEVICE]) + + if config_entry_id is not None and device_info is not None: + device_info["config_entry_id"] = config_entry_id + device_registry.async_get_or_create(**device_info) From 9fad9babb9513c02fdb433b82d210f61214052b5 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 23 Jan 2022 14:09:37 +0000 Subject: [PATCH 07/28] Add targets attribute and support for data param --- .../components/mqtt/abbreviations.py | 2 +- homeassistant/components/mqtt/notify.py | 55 +++++++++++++++---- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/mqtt/abbreviations.py b/homeassistant/components/mqtt/abbreviations.py index c79341f568f0ea..bf21a0a5c968cb 100644 --- a/homeassistant/components/mqtt/abbreviations.py +++ b/homeassistant/components/mqtt/abbreviations.py @@ -183,7 +183,7 @@ "set_pos_tpl": "set_position_template", "set_pos_t": "set_position_topic", "title": "title", - "trgt": "target", + "trgts": "targets", "pos_t": "position_topic", "pos_tpl": "position_template", "spd_cmd_t": "speed_command_topic", diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index b238dc906f02f0..a9a98d854534f9 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -43,24 +43,30 @@ ) CONF_COMMAND_TEMPLATE = "command_template" +CONF_TARGETS = "targets" CONF_TITLE = "title" MQTT_EVENT_RELOADED = "event_{}_reloaded" -MQTT_NOTIFY_TARGET_CONFIG = "mqtt_notify_target_config" +MQTT_TARGET_KEY = "{}_{}" PLATFORM_SCHEMA = mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend( { vol.Required(CONF_COMMAND_TOPIC): mqtt.valid_publish_topic, vol.Optional(CONF_COMMAND_TEMPLATE): cv.template, - vol.Optional(CONF_DEVICE): MQTT_ENTITY_DEVICE_INFO_SCHEMA, vol.Optional(CONF_NAME): cv.string, + vol.Optional(CONF_TARGETS, default=[]): cv.ensure_list, vol.Optional(CONF_TITLE, default=notify.ATTR_TITLE_DEFAULT): cv.string, vol.Optional(CONF_RETAIN, default=mqtt.DEFAULT_RETAIN): cv.boolean, } ) -DISCOVERY_SCHEMA = PLATFORM_SCHEMA.extend({}, extra=vol.REMOVE_EXTRA) +DISCOVERY_SCHEMA = PLATFORM_SCHEMA.extend( + { + vol.Optional(CONF_DEVICE): MQTT_ENTITY_DEVICE_INFO_SCHEMA, + }, + extra=vol.REMOVE_EXTRA, +) _LOGGER = logging.getLogger(__name__) @@ -104,6 +110,7 @@ async def _async_setup_notify( config.get(CONF_NAME), config[CONF_QOS], config[CONF_RETAIN], + config[CONF_TARGETS], config[CONF_TITLE], discovery_hash=discovery_hash, device_id=device_id, @@ -130,6 +137,7 @@ async def async_get_service( config.get(CONF_NAME), config[CONF_QOS], config[CONF_RETAIN], + config[CONF_TARGETS], config[CONF_TITLE], ) @@ -217,6 +225,7 @@ def __init__( name: str | None, qos: int, retain: bool, + targets: list, title: str | None, discovery_hash: tuple | None = None, device_id: str | None = None, @@ -230,6 +239,7 @@ def __init__( self._name = name self._qos = qos self._retain = retain + self._targets = targets self._title = title self._discovery_hash = discovery_hash self._device_id = device_id @@ -266,24 +276,47 @@ async def async_update_service( self._retain = config[CONF_RETAIN] self._title = config[CONF_TITLE] new_service_name = slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])) - if new_service_name != self._service_name: + if ( + new_service_name != self._service_name + or config[CONF_TARGETS] != self._targets + ): await self.async_unregister_services() + self._targets = config[CONF_TARGETS] self._service_name = new_service_name await self.async_register_services() if self.device_id: await _update_device(self.hass, self._config_entry, config) + @property + def targets(self) -> dict[str, str]: + """Return a dictionary of registered targets.""" + return { + MQTT_TARGET_KEY.format(self._service_name, target): target + for target in self._targets + } + async def async_send_message(self, message: str = "", **kwargs): """Build and send a MQTT message.""" + target = kwargs.get(notify.ATTR_TARGET) + if ( + target is not None + and self._targets + and set(target) & set(self._targets) != set(target) + ): + raise AttributeError( + f"Cannot send {message}, target list {target} is invalid, valid available targets: {self._targets}" + ) + variables = { + "message": message, + "name": self._name, + "service": self._service_name, + "target": target or self._targets, + "title": kwargs.get(notify.ATTR_TITLE, self._title), + } + variables.update(kwargs.get(notify.ATTR_DATA) or {}) payload = self._command_template.async_render( message, - variables={ - "data": kwargs.get(notify.ATTR_DATA), - "message": message, - "name": self._name, - "service": self._service_name, - "title": kwargs.get(notify.ATTR_TITLE, self._title), - }, + variables=variables, ) await mqtt.async_publish( self.hass, From 60a468e9ef46ac287a836de433eb8289319b4f41 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 23 Jan 2022 23:22:14 +0000 Subject: [PATCH 08/28] Add tests and resolve naming issues --- homeassistant/components/mqtt/notify.py | 26 +- tests/components/mqtt/test_notify.py | 456 +++++++++++++++++++++++- 2 files changed, 457 insertions(+), 25 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index a9a98d854534f9..9859775b0803b9 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -48,8 +48,6 @@ MQTT_EVENT_RELOADED = "event_{}_reloaded" -MQTT_TARGET_KEY = "{}_{}" - PLATFORM_SCHEMA = mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend( { vol.Required(CONF_COMMAND_TOPIC): mqtt.valid_publish_topic, @@ -116,9 +114,8 @@ async def _async_setup_notify( device_id=device_id, config_entry=config_entry, ) - await service.async_setup( - hass, slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])), "" - ) + service_name = slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])) + await service.async_setup(hass, service_name, service_name) await service.async_register_services() @@ -129,12 +126,16 @@ async def async_get_service( ) -> MqttNotificationService | None: """Prepare the MQTT notification service through configuration.yaml.""" await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + name = config.get(CONF_NAME) + if name is None: + # use command_topic as a base for the service name if no name is defined + config[CONF_NAME] = config[CONF_COMMAND_TOPIC] return MqttNotificationService( hass, config[CONF_COMMAND_TOPIC], MqttCommandTemplate(config.get(CONF_COMMAND_TEMPLATE), hass=hass), config[CONF_ENCODING], - config.get(CONF_NAME), + name, config[CONF_QOS], config[CONF_RETAIN], config[CONF_TARGETS], @@ -290,10 +291,7 @@ async def async_update_service( @property def targets(self) -> dict[str, str]: """Return a dictionary of registered targets.""" - return { - MQTT_TARGET_KEY.format(self._service_name, target): target - for target in self._targets - } + return {target: target for target in self._targets} async def async_send_message(self, message: str = "", **kwargs): """Build and send a MQTT message.""" @@ -303,9 +301,13 @@ async def async_send_message(self, message: str = "", **kwargs): and self._targets and set(target) & set(self._targets) != set(target) ): - raise AttributeError( - f"Cannot send {message}, target list {target} is invalid, valid available targets: {self._targets}" + _LOGGER.error( + "Cannot send %s, target list %s is invalid, valid available targets: %s", + message, + target, + self._targets, ) + return variables = { "message": message, "name": self._name, diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index e7befb962a863c..7943a3e415be8a 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -1,46 +1,324 @@ """The tests for the MQTT button platform.""" import copy +import json from unittest.mock import patch +import pytest import yaml from homeassistant import config as hass_config from homeassistant.components import notify from homeassistant.components.mqtt import DOMAIN -from homeassistant.const import SERVICE_RELOAD +from homeassistant.const import CONF_NAME, SERVICE_RELOAD from homeassistant.setup import async_setup_component +from homeassistant.util import slugify -from tests.common import async_fire_mqtt_message +from tests.common import async_fire_mqtt_message, mock_device_registry DEFAULT_CONFIG = {notify.DOMAIN: {"platform": "mqtt", "command_topic": "test-topic"}} +COMMAND_TEMPLATE_TEST_PARAMS = ( + "name,service,parameters,expected_result", + [ + ( + None, + "lcd_set", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_DATA: {"par1": "val1"}, + }, + '{"message":"Message",' + '"name":"None",' + '"service":"lcd_set",' + '"par1":"val1",' + '"target":[' + "'t1', 't2'" + "]," + '"title":"Title"}', + ), + ( + None, + "lcd_set", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_DATA: {"par1": "val1"}, + notify.ATTR_TARGET: ["t2"], + }, + '{"message":"Message",' + '"name":"None",' + '"service":"lcd_set",' + '"par1":"val1",' + '"target":[' + "'t2'" + "]," + '"title":"Title"}', + ), + ( + None, + "lcd_set_t1", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_DATA: {"par1": "val2"}, + }, + '{"message":"Message",' + '"name":"None",' + '"service":"lcd_set",' + '"par1":"val2",' + '"target":[' + "'t1'" + "]," + '"title":"Title"}', + ), + ( + "My service", + "my_service_t1", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_DATA: {"par1": "val2"}, + }, + '{"message":"Message",' + '"name":"My service",' + '"service":"my_service",' + '"par1":"val2",' + '"target":[' + "'t1'" + "]," + '"title":"Title"}', + ), + ], +) + + +@pytest.fixture +def device_reg(hass): + """Return an empty, loaded, registry.""" + return mock_device_registry(hass) + async def test_sending_mqtt_commands(hass, mqtt_mock, caplog): """Test the sending MQTT commands.""" + config1 = { + "command_topic": "command-topic1", + "name": "test1", + "platform": "mqtt", + "qos": "2", + } + config2 = { + "command_topic": "command-topic2", + "name": "test2", + "targets": ["t1", "t2"], + "platform": "mqtt", + "qos": "2", + } assert await async_setup_component( hass, notify.DOMAIN, - { - notify.DOMAIN: { - "command_topic": "command-topic", - "name": "test", - "platform": "mqtt", - "qos": "2", - } - }, + {notify.DOMAIN: [config1, config2]}, ) await hass.async_block_till_done() - assert "" in caplog.text + assert "" in caplog.text + assert "" in caplog.text + assert ( + "" in caplog.text + ) + assert ( + "" in caplog.text + ) + # test1 simple call without targets await hass.services.async_call( notify.DOMAIN, - "test", + "test1", {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, blocking=True, ) mqtt_mock.async_publish.assert_called_once_with( - "command-topic", "Message", 2, False + "command-topic1", "Message", 2, False + ) + mqtt_mock.async_publish.reset_mock() + + # test2 simple call without targets + await hass.services.async_call( + notify.DOMAIN, + "test2", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, + blocking=True, + ) + + mqtt_mock.async_publish.assert_called_once_with( + "command-topic2", "Message", 2, False + ) + mqtt_mock.async_publish.reset_mock() + + # test2 simple call main service without target + await hass.services.async_call( + notify.DOMAIN, + "test2", + {notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message"}, + blocking=True, + ) + + mqtt_mock.async_publish.assert_called_once_with( + "command-topic2", "Message", 2, False + ) + mqtt_mock.async_publish.reset_mock() + + # test2 simple call main service with empty target + await hass.services.async_call( + notify.DOMAIN, + "test2", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_TARGET: [], + }, + blocking=True, + ) + + mqtt_mock.async_publish.assert_called_once_with( + "command-topic2", "Message", 2, False + ) + mqtt_mock.async_publish.reset_mock() + + # test2 simple call main service with single target + await hass.services.async_call( + notify.DOMAIN, + "test2", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_TARGET: ["t1"], + }, + blocking=True, + ) + + mqtt_mock.async_publish.assert_called_once_with( + "command-topic2", "Message", 2, False + ) + mqtt_mock.async_publish.reset_mock() + + # test2 simple call main service with invalid target + await hass.services.async_call( + notify.DOMAIN, + "test2", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_TARGET: ["invalid"], + }, + blocking=True, + ) + + assert ( + "Cannot send Message, target list ['invalid'] is invalid, valid available targets: ['t1', 't2']" + in caplog.text + ) + mqtt_mock.async_publish.call_count == 0 + mqtt_mock.async_publish.reset_mock() + + +@pytest.mark.parametrize(*COMMAND_TEMPLATE_TEST_PARAMS) +async def test_sending_with_command_templates_with_config_setup( + hass, mqtt_mock, caplog, name, service, parameters, expected_result +): + """Test the sending MQTT commands using a template using config setup.""" + config = { + "command_topic": "lcd/set", + "command_template": "{" + '"message":"{{message}}",' + '"name":"{{name}}",' + '"service":"{{service}}",' + '"par1":"{{par1}}",' + '"target":{{target}},' + '"title":"{{title}}"' + "}", + "targets": ["t1", "t2"], + "platform": "mqtt", + "qos": "1", + } + if name: + config[CONF_NAME] = name + service_base_name = slugify(name) or "lcd_set" + assert await async_setup_component( + hass, + notify.DOMAIN, + {notify.DOMAIN: config}, + ) + await hass.async_block_till_done() + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + await hass.services.async_call( + notify.DOMAIN, + service, + parameters, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with( + "lcd/set", expected_result, 1, False + ) + mqtt_mock.async_publish.reset_mock() + + +@pytest.mark.parametrize(*COMMAND_TEMPLATE_TEST_PARAMS) +async def test_sending_with_command_templates_auto_discovery( + hass, mqtt_mock, caplog, name, service, parameters, expected_result +): + """Test the sending MQTT commands using a template and auto discovery.""" + config = { + "command_topic": "lcd/set", + "command_template": "{" + '"message":"{{message}}",' + '"name":"{{name}}",' + '"service":"{{service}}",' + '"par1":"{{par1}}",' + '"target":{{target}},' + '"title":"{{title}}"' + "}", + "targets": ["t1", "t2"], + "qos": "1", + } + if name: + config[CONF_NAME] = name + service_base_name = slugify(name) or "lcd_set" + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/bla/config", json.dumps(config) + ) + await hass.async_block_till_done() + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + await hass.services.async_call( + notify.DOMAIN, + service, + parameters, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with( + "lcd/set", expected_result, 1, False ) mqtt_mock.async_publish.reset_mock() @@ -49,6 +327,8 @@ async def test_discovery(hass, mqtt_mock, caplog): """Test discovery, update and removal of notify service.""" data = '{ "name": "Old name", "command_topic": "test_topic" }' data_update = '{ "command_topic": "test_topic_update", "name": "New name" }' + data_update_with_targets1 = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"] }' + data_update_with_targets2 = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target3"] }' async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) await hass.async_block_till_done() @@ -96,6 +376,156 @@ async def test_discovery(hass, mqtt_mock, caplog): assert "" in caplog.text + # rediscover with targets + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/bla/config", data_update_with_targets1 + ) + await hass.async_block_till_done() + + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + caplog.clear() + # update available targets + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/bla/config", data_update_with_targets2 + ) + await hass.async_block_till_done() + + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + caplog.clear() + + +async def test_discovery_with_device(hass, mqtt_mock, caplog, device_reg): + """Test discovery, update and removal of notify service with a device config.""" + data = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Name" } }' + data_device_update = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Name update" } }' + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + device_id = device_entry.id + assert ( + f"" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + caplog.clear() + + # Test device update + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/bla/config", data_device_update + ) + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + assert device_id == device_entry.id + assert ( + f"" + in caplog.text + ) + caplog.clear() + + # Test removal device from device registry + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", "{}") + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is None + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + caplog.clear() + + # Re-create the device again + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + device_id = device_entry.id + assert ( + f"" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + caplog.clear() + + # Remove the device from the device registry + device_reg.async_remove_device(device_id) + await hass.async_block_till_done() + assert ( + f"" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + assert "Notify service ('notify', 'bla') has been removed" in caplog.text + caplog.clear() + async def test_publishing_with_custom_encoding(hass, mqtt_mock, caplog): """Test publishing MQTT payload with different encoding via discovery and configuration.""" From 6f265851ba87833c872e4f36ff92fa92a1f1ac62 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 23 Jan 2022 23:24:17 +0000 Subject: [PATCH 09/28] CONF_COMMAND_TEMPLATE from .const --- homeassistant/components/mqtt/notify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 9859775b0803b9..38fe09bec7b596 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -26,6 +26,7 @@ from .const import ( ATTR_DISCOVERY_HASH, ATTR_DISCOVERY_PAYLOAD, + CONF_COMMAND_TEMPLATE, CONF_COMMAND_TOPIC, CONF_ENCODING, CONF_QOS, @@ -42,7 +43,6 @@ device_info_from_config, ) -CONF_COMMAND_TEMPLATE = "command_template" CONF_TARGETS = "targets" CONF_TITLE = "title" From 197b5d858692c577221df7581ae65fc8940d2e67 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Mon, 24 Jan 2022 08:08:56 +0000 Subject: [PATCH 10/28] Use mqtt as default service name --- homeassistant/components/mqtt/notify.py | 12 ++++++------ tests/components/mqtt/test_notify.py | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 38fe09bec7b596..e6f3ae982f935d 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -114,7 +114,7 @@ async def _async_setup_notify( device_id=device_id, config_entry=config_entry, ) - service_name = slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])) + service_name = slugify(config.get(CONF_NAME, DOMAIN)) await service.async_setup(hass, service_name, service_name) await service.async_register_services() @@ -125,11 +125,11 @@ async def async_get_service( discovery_info: DiscoveryInfoType | None = None, ) -> MqttNotificationService | None: """Prepare the MQTT notification service through configuration.yaml.""" - await async_setup_reload_service(hass, DOMAIN, PLATFORMS) name = config.get(CONF_NAME) - if name is None: - # use command_topic as a base for the service name if no name is defined - config[CONF_NAME] = config[CONF_COMMAND_TOPIC] + if CONF_NAME not in config: + config[CONF_NAME] = DOMAIN + + await async_setup_reload_service(hass, DOMAIN, PLATFORMS) return MqttNotificationService( hass, config[CONF_COMMAND_TOPIC], @@ -276,7 +276,7 @@ async def async_update_service( self._qos = config[CONF_QOS] self._retain = config[CONF_RETAIN] self._title = config[CONF_TITLE] - new_service_name = slugify(config.get(CONF_NAME, config[CONF_COMMAND_TOPIC])) + new_service_name = slugify(config.get(CONF_NAME, DOMAIN)) if ( new_service_name != self._service_name or config[CONF_TARGETS] != self._targets diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index 7943a3e415be8a..2ccb895f8c98e0 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -22,7 +22,7 @@ [ ( None, - "lcd_set", + "mqtt", { notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message", @@ -30,7 +30,7 @@ }, '{"message":"Message",' '"name":"None",' - '"service":"lcd_set",' + '"service":"mqtt",' '"par1":"val1",' '"target":[' "'t1', 't2'" @@ -39,7 +39,7 @@ ), ( None, - "lcd_set", + "mqtt", { notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message", @@ -48,7 +48,7 @@ }, '{"message":"Message",' '"name":"None",' - '"service":"lcd_set",' + '"service":"mqtt",' '"par1":"val1",' '"target":[' "'t2'" @@ -57,7 +57,7 @@ ), ( None, - "lcd_set_t1", + "mqtt_t1", { notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message", @@ -65,7 +65,7 @@ }, '{"message":"Message",' '"name":"None",' - '"service":"lcd_set",' + '"service":"mqtt",' '"par1":"val2",' '"target":[' "'t1'" @@ -243,7 +243,9 @@ async def test_sending_with_command_templates_with_config_setup( } if name: config[CONF_NAME] = name - service_base_name = slugify(name) or "lcd_set" + service_base_name = slugify(name) + else: + service_base_name = DOMAIN assert await async_setup_component( hass, notify.DOMAIN, @@ -294,7 +296,9 @@ async def test_sending_with_command_templates_auto_discovery( } if name: config[CONF_NAME] = name - service_base_name = slugify(name) or "lcd_set" + service_base_name = slugify(name) + else: + service_base_name = DOMAIN async_fire_mqtt_message( hass, f"homeassistant/{notify.DOMAIN}/bla/config", json.dumps(config) ) From 7ddaff34650b88de02c838c824a58fb595b1150c Mon Sep 17 00:00:00 2001 From: jbouwh Date: Mon, 24 Jan 2022 19:42:39 +0000 Subject: [PATCH 11/28] make sure service has a unique name --- homeassistant/components/mqtt/notify.py | 58 ++++- tests/components/mqtt/test_notify.py | 298 +++++++++++++++++------- 2 files changed, 262 insertions(+), 94 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index e6f3ae982f935d..bd3534b40aee44 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -46,7 +46,7 @@ CONF_TARGETS = "targets" CONF_TITLE = "title" -MQTT_EVENT_RELOADED = "event_{}_reloaded" +MQTT_NOTIFY_SERVICES_SETUP = "mqtt_notify_services_setup" PLATFORM_SCHEMA = mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend( { @@ -87,7 +87,19 @@ async def _async_setup_notify( ): """Set up the MQTT notify service with auto discovery.""" config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) + service_name = slugify(config.get(CONF_NAME) or DOMAIN) + services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) + has_services = hass.services.has_service(notify.DOMAIN, service_name) discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] + if service_name in services or has_services: + _LOGGER.error( + "Notify service '%s' already exists, cannot register service(s)", + service_name, + ) + async_dispatcher_send(hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None) + clear_discovery_hash(hass, discovery_hash) + return + services.add(service_name) device_id = None if CONF_DEVICE in config: await _update_device(hass, config_entry, config) @@ -114,7 +126,6 @@ async def _async_setup_notify( device_id=device_id, config_entry=config_entry, ) - service_name = slugify(config.get(CONF_NAME, DOMAIN)) await service.async_setup(hass, service_name, service_name) await service.async_register_services() @@ -128,7 +139,16 @@ async def async_get_service( name = config.get(CONF_NAME) if CONF_NAME not in config: config[CONF_NAME] = DOMAIN - + service_name = slugify(name or DOMAIN) + services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) + has_services = hass.services.has_service(notify.DOMAIN, service_name) + if service_name in services or has_services: + _LOGGER.error( + "Notify service '%s' is not unique, cannot register service(s)", + service_name, + ) + return None + services.add(service_name) await async_setup_reload_service(hass, DOMAIN, PLATFORMS) return MqttNotificationService( hass, @@ -164,7 +184,7 @@ async def async_discovery_update( # update notify service through auto discovery await service.async_update_service(discovery_payload) _LOGGER.debug( - "Notify service %s has been updated", + "Notify service %s updated has been processed", service.discovery_hash, ) @@ -182,6 +202,9 @@ async def async_device_removed(event): async def async_tear_down_service(): """Handle the removal of the service.""" + services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) + if self._service.service_name in services: + services.remove(self._service.service_name) if not self._device_removed and service.device_id: self._device_removed = True await cleanup_device_registry(hass, service.device_id) @@ -245,21 +268,26 @@ def __init__( self._discovery_hash = discovery_hash self._device_id = device_id self._config_entry = config_entry - self._service_name = slugify(name or command_topic) + self._service_name = slugify(name or DOMAIN) self._updater = ( MqttNotificationServiceUpdater(hass, self) if discovery_hash else None ) + @property + def device_id(self) -> str | None: + """Return the device ID.""" + return self._device_id + @property def discovery_hash(self) -> tuple | None: """Return the discovery hash.""" return self._discovery_hash @property - def device_id(self) -> str | None: - """Return the device ID.""" - return self._device_id + def service_name(self) -> str: + """Return the service ma,e.""" + return self._service_name async def async_update_service( self, @@ -267,6 +295,15 @@ async def async_update_service( ) -> None: """Update the notify service through auto discovery.""" config = DISCOVERY_SCHEMA(discovery_payload) + new_service_name = slugify(config.get(CONF_NAME, DOMAIN)) + if new_service_name != self._service_name and self.hass.services.has_service( + notify.DOMAIN, new_service_name + ): + _LOGGER.error( + "Notify service '%s' already exists, cannot update the existing service(s)", + new_service_name, + ) + return self._command_topic = config[CONF_COMMAND_TOPIC] self._command_template = MqttCommandTemplate( config.get(CONF_COMMAND_TEMPLATE), hass=self.hass @@ -276,15 +313,18 @@ async def async_update_service( self._qos = config[CONF_QOS] self._retain = config[CONF_RETAIN] self._title = config[CONF_TITLE] - new_service_name = slugify(config.get(CONF_NAME, DOMAIN)) if ( new_service_name != self._service_name or config[CONF_TARGETS] != self._targets ): + services = self.hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) await self.async_unregister_services() + if self._service_name in services: + services.remove(self._service_name) self._targets = config[CONF_TARGETS] self._service_name = new_service_name await self.async_register_services() + services.add(new_service_name) if self.device_id: await _update_device(self.hass, self._config_entry, config) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index 2ccb895f8c98e0..aecfe172a1bff0 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -10,6 +10,7 @@ from homeassistant.components import notify from homeassistant.components.mqtt import DOMAIN from homeassistant.const import CONF_NAME, SERVICE_RELOAD +from homeassistant.exceptions import ServiceNotFound from homeassistant.setup import async_setup_component from homeassistant.util import slugify @@ -99,6 +100,111 @@ def device_reg(hass): return mock_device_registry(hass) +@pytest.mark.parametrize(*COMMAND_TEMPLATE_TEST_PARAMS) +async def test_sending_with_command_templates_with_config_setup( + hass, mqtt_mock, caplog, name, service, parameters, expected_result +): + """Test the sending MQTT commands using a template using config setup.""" + config = { + "command_topic": "lcd/set", + "command_template": "{" + '"message":"{{message}}",' + '"name":"{{name}}",' + '"service":"{{service}}",' + '"par1":"{{par1}}",' + '"target":{{target}},' + '"title":"{{title}}"' + "}", + "targets": ["t1", "t2"], + "platform": "mqtt", + "qos": "1", + } + if name: + config[CONF_NAME] = name + service_base_name = slugify(name) + else: + service_base_name = DOMAIN + assert await async_setup_component( + hass, + notify.DOMAIN, + {notify.DOMAIN: config}, + ) + await hass.async_block_till_done() + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + await hass.services.async_call( + notify.DOMAIN, + service, + parameters, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with( + "lcd/set", expected_result, 1, False + ) + mqtt_mock.async_publish.reset_mock() + + +@pytest.mark.parametrize(*COMMAND_TEMPLATE_TEST_PARAMS) +async def test_sending_with_command_templates_auto_discovery( + hass, mqtt_mock, caplog, name, service, parameters, expected_result +): + """Test the sending MQTT commands using a template and auto discovery.""" + config = { + "command_topic": "lcd/set", + "command_template": "{" + '"message":"{{message}}",' + '"name":"{{name}}",' + '"service":"{{service}}",' + '"par1":"{{par1}}",' + '"target":{{target}},' + '"title":"{{title}}"' + "}", + "targets": ["t1", "t2"], + "qos": "1", + } + if name: + config[CONF_NAME] = name + service_base_name = slugify(name) + else: + service_base_name = DOMAIN + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/bla/config", json.dumps(config) + ) + await hass.async_block_till_done() + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + await hass.services.async_call( + notify.DOMAIN, + service, + parameters, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with( + "lcd/set", expected_result, 1, False + ) + mqtt_mock.async_publish.reset_mock() + + async def test_sending_mqtt_commands(hass, mqtt_mock, caplog): """Test the sending MQTT commands.""" config1 = { @@ -222,113 +328,68 @@ async def test_sending_mqtt_commands(hass, mqtt_mock, caplog): mqtt_mock.async_publish.reset_mock() -@pytest.mark.parametrize(*COMMAND_TEMPLATE_TEST_PARAMS) -async def test_sending_with_command_templates_with_config_setup( - hass, mqtt_mock, caplog, name, service, parameters, expected_result -): - """Test the sending MQTT commands using a template using config setup.""" - config = { - "command_topic": "lcd/set", - "command_template": "{" - '"message":"{{message}}",' - '"name":"{{name}}",' - '"service":"{{service}}",' - '"par1":"{{par1}}",' - '"target":{{target}},' - '"title":"{{title}}"' - "}", +async def test_with_same_name(hass, mqtt_mock, caplog): + """Test the multiple setups with the same name.""" + config1 = { + "command_topic": "command-topic1", + "name": "test_same_name", + "platform": "mqtt", + "qos": "2", + } + config2 = { + "command_topic": "command-topic2", + "name": "test_same_name", "targets": ["t1", "t2"], "platform": "mqtt", - "qos": "1", + "qos": "2", } - if name: - config[CONF_NAME] = name - service_base_name = slugify(name) - else: - service_base_name = DOMAIN assert await async_setup_component( hass, notify.DOMAIN, - {notify.DOMAIN: config}, + {notify.DOMAIN: [config1, config2]}, ) await hass.async_block_till_done() assert ( - f"" - in caplog.text - ) - assert ( - f"" + "" in caplog.text ) assert ( - f"" + "Notify service 'test_same_name' is not unique, cannot register service(s)" in caplog.text ) - await hass.services.async_call( - notify.DOMAIN, - service, - parameters, - blocking=True, - ) - mqtt_mock.async_publish.assert_called_once_with( - "lcd/set", expected_result, 1, False - ) - mqtt_mock.async_publish.reset_mock() - -@pytest.mark.parametrize(*COMMAND_TEMPLATE_TEST_PARAMS) -async def test_sending_with_command_templates_auto_discovery( - hass, mqtt_mock, caplog, name, service, parameters, expected_result -): - """Test the sending MQTT commands using a template and auto discovery.""" - config = { - "command_topic": "lcd/set", - "command_template": "{" - '"message":"{{message}}",' - '"name":"{{name}}",' - '"service":"{{service}}",' - '"par1":"{{par1}}",' - '"target":{{target}},' - '"title":"{{title}}"' - "}", - "targets": ["t1", "t2"], - "qos": "1", - } - if name: - config[CONF_NAME] = name - service_base_name = slugify(name) - else: - service_base_name = DOMAIN - async_fire_mqtt_message( - hass, f"homeassistant/{notify.DOMAIN}/bla/config", json.dumps(config) - ) - await hass.async_block_till_done() - assert ( - f"" - in caplog.text - ) - assert ( - f"" - in caplog.text - ) - assert ( - f"" - in caplog.text - ) + # test call main service on service with multiple targets with the same name + # the first configured service should publish await hass.services.async_call( notify.DOMAIN, - service, - parameters, + "test_same_name", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + }, blocking=True, ) + mqtt_mock.async_publish.assert_called_once_with( - "lcd/set", expected_result, 1, False + "command-topic1", "Message", 2, False ) mqtt_mock.async_publish.reset_mock() + with pytest.raises(ServiceNotFound): + await hass.services.async_call( + notify.DOMAIN, + "test_same_name_t2", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_TARGET: ["t2"], + }, + blocking=True, + ) + -async def test_discovery(hass, mqtt_mock, caplog): - """Test discovery, update and removal of notify service.""" +async def test_discovery_without_device(hass, mqtt_mock, caplog): + """Test discovery, update and removal of notify service without device.""" data = '{ "name": "Old name", "command_topic": "test_topic" }' data_update = '{ "command_topic": "test_topic_update", "name": "New name" }' data_update_with_targets1 = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"] }' @@ -360,8 +421,7 @@ async def test_discovery(hass, mqtt_mock, caplog): assert ( "" in caplog.text ) - - assert "Notify service ('notify', 'bla') has been updated" in caplog.text + assert "Notify service ('notify', 'bla') updated has been processed" in caplog.text await hass.services.async_call( notify.DOMAIN, @@ -399,6 +459,7 @@ async def test_discovery(hass, mqtt_mock, caplog): in caplog.text ) caplog.clear() + # update available targets async_fire_mqtt_message( hass, f"homeassistant/{notify.DOMAIN}/bla/config", data_update_with_targets2 @@ -415,6 +476,73 @@ async def test_discovery(hass, mqtt_mock, caplog): ) caplog.clear() + # test if a new service with same name fails to setup + config1 = { + "command_topic": "command-topic-config.yaml", + "name": "test-setup1", + "platform": "mqtt", + "qos": "2", + } + assert await async_setup_component( + hass, + notify.DOMAIN, + {notify.DOMAIN: [config1]}, + ) + await hass.async_block_till_done() + data = '{ "name": "test-setup1", "command_topic": "test_topic" }' + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/test-setup1/config", data + ) + await hass.async_block_till_done() + assert ( + "Notify service 'test_setup1' already exists, cannot register service(s)" + in caplog.text + ) + await hass.services.async_call( + notify.DOMAIN, + "test_setup1", + { + notify.ATTR_TITLE: "Title", + notify.ATTR_MESSAGE: "Message", + notify.ATTR_TARGET: ["t2"], + }, + blocking=True, + ) + mqtt_mock.async_publish.assert_called_once_with( + "command-topic-config.yaml", "Message", 2, False + ) + + # Test with same discovery on new name + data = '{ "name": "testa", "command_topic": "test_topic_a" }' + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/testa/config", data) + await hass.async_block_till_done() + assert "" in caplog.text + + data = '{ "name": "testb", "command_topic": "test_topic_b" }' + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/testb/config", data) + await hass.async_block_till_done() + assert "" in caplog.text + + # Try to update from new discovery of existing service test + data = '{ "name": "testa", "command_topic": "test_topic_c" }' + caplog.clear() + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/testc/config", data) + await hass.async_block_till_done() + assert ( + "Notify service 'testa' already exists, cannot register service(s)" + in caplog.text + ) + + # Try to update the same discovery to existing service test + data = '{ "name": "testa", "command_topic": "test_topic_c" }' + caplog.clear() + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/testb/config", data) + await hass.async_block_till_done() + assert ( + "Notify service 'testa' already exists, cannot update the existing service(s)" + in caplog.text + ) + async def test_discovery_with_device(hass, mqtt_mock, caplog, device_reg): """Test discovery, update and removal of notify service with a device config.""" From 98768bb9023860fedc1b6ab0fc3293243394ee0c Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 26 Jan 2022 16:13:20 +0000 Subject: [PATCH 12/28] pylint error --- homeassistant/components/mqtt/discovery.py | 5 ++++- homeassistant/components/mqtt/notify.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/mqtt/discovery.py b/homeassistant/components/mqtt/discovery.py index b096343618bc2a..a061737c0907c8 100644 --- a/homeassistant/components/mqtt/discovery.py +++ b/homeassistant/components/mqtt/discovery.py @@ -15,6 +15,7 @@ async_dispatcher_connect, async_dispatcher_send, ) +from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.loader import async_get_mqtt from .. import mqtt @@ -238,7 +239,9 @@ async def discovery_done(_): # pylint: disable=import-outside-toplevel from . import notify - await notify.async_setup_entry(hass, config_entry) + await notify.async_setup_entry( + hass, config_entry, AddEntitiesCallback() + ) elif component in "tag": # Local import to avoid circular dependencies # pylint: disable=import-outside-toplevel diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index bd3534b40aee44..92f0400dbf7782 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -17,6 +17,7 @@ async_dispatcher_connect, async_dispatcher_send, ) +from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.reload import async_setup_reload_service from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from homeassistant.util import slugify @@ -72,6 +73,7 @@ async def async_setup_entry( hass: HomeAssistant, config_entry: ConfigEntry, + async_add_entities: AddEntitiesCallback, ) -> None: """Set up MQTT climate device dynamically through MQTT discovery.""" await async_setup_reload_service(hass, DOMAIN, PLATFORMS) From e2b0d1f74d1e05c8e20a7abad1c1b220fcc6d8e7 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 26 Jan 2022 20:13:08 +0000 Subject: [PATCH 13/28] fix type error --- homeassistant/components/mqtt/discovery.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/mqtt/discovery.py b/homeassistant/components/mqtt/discovery.py index a061737c0907c8..ed6e90fc099ebc 100644 --- a/homeassistant/components/mqtt/discovery.py +++ b/homeassistant/components/mqtt/discovery.py @@ -240,7 +240,7 @@ async def discovery_done(_): from . import notify await notify.async_setup_entry( - hass, config_entry, AddEntitiesCallback() + hass, config_entry, AddEntitiesCallback ) elif component in "tag": # Local import to avoid circular dependencies From 228270771961c97aa55bfece9185d189e91eb665 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sat, 5 Feb 2022 14:59:20 +0000 Subject: [PATCH 14/28] Conditional device removal and test --- homeassistant/components/mqtt/mixins.py | 9 +- homeassistant/components/mqtt/notify.py | 36 +++++--- tests/components/mqtt/test_notify.py | 115 ++++++++++++++++-------- 3 files changed, 105 insertions(+), 55 deletions(-) diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index f72b16bc1b0b22..d593acf5cee48e 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -23,7 +23,7 @@ CONF_UNIQUE_ID, CONF_VALUE_TEMPLATE, ) -from homeassistant.core import callback +from homeassistant.core import HomeAssistant, callback from homeassistant.helpers import ( config_validation as cv, device_registry as dr, @@ -453,11 +453,11 @@ def available(self) -> bool: return self._available_latest -async def cleanup_device_registry(hass, device_id): - """Remove device registry entry if there are no remaining entities or triggers.""" +async def cleanup_device_registry(hass: HomeAssistant, device_id: str | None) -> None: + """Remove device registry entry if there are no remaining entities, triggers or notify services.""" # Local import to avoid circular dependencies # pylint: disable=import-outside-toplevel - from . import device_trigger, tag + from . import device_trigger, notify, tag device_registry = dr.async_get(hass) entity_registry = er.async_get(hass) @@ -468,6 +468,7 @@ async def cleanup_device_registry(hass, device_id): ) and not await device_trigger.async_get_triggers(hass, device_id) and not tag.async_has_tags(hass, device_id) + and not notify.has_notify_services(hass, device_id) ): device_registry.async_remove_device(device_id) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 92f0400dbf7782..5b5d3ca575ab43 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -90,10 +90,10 @@ async def _async_setup_notify( """Set up the MQTT notify service with auto discovery.""" config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) service_name = slugify(config.get(CONF_NAME) or DOMAIN) - services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) + services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) has_services = hass.services.has_service(notify.DOMAIN, service_name) discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] - if service_name in services or has_services: + if service_name in services.keys() or has_services: _LOGGER.error( "Notify service '%s' already exists, cannot register service(s)", service_name, @@ -101,7 +101,6 @@ async def _async_setup_notify( async_dispatcher_send(hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None) clear_discovery_hash(hass, discovery_hash) return - services.add(service_name) device_id = None if CONF_DEVICE in config: await _update_device(hass, config_entry, config) @@ -128,10 +127,21 @@ async def _async_setup_notify( device_id=device_id, config_entry=config_entry, ) + services[service_name] = service + await service.async_setup(hass, service_name, service_name) await service.async_register_services() +def has_notify_services(hass: HomeAssistant, device_id: str) -> bool: + """Return a list of registered notify services.""" + services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + for key, service in services.items(): # pylint: disable=unused-variable + if service.device_id == device_id: + return True + return False + + async def async_get_service( hass: HomeAssistant, config: ConfigType, @@ -142,17 +152,16 @@ async def async_get_service( if CONF_NAME not in config: config[CONF_NAME] = DOMAIN service_name = slugify(name or DOMAIN) - services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) has_services = hass.services.has_service(notify.DOMAIN, service_name) - if service_name in services or has_services: + services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + if service_name in services.keys() or has_services: _LOGGER.error( "Notify service '%s' is not unique, cannot register service(s)", service_name, ) return None - services.add(service_name) await async_setup_reload_service(hass, DOMAIN, PLATFORMS) - return MqttNotificationService( + services[service_name] = MqttNotificationService( hass, config[CONF_COMMAND_TOPIC], MqttCommandTemplate(config.get(CONF_COMMAND_TEMPLATE), hass=hass), @@ -163,6 +172,7 @@ async def async_get_service( config[CONF_TARGETS], config[CONF_TITLE], ) + return services[service_name] class MqttNotificationServiceUpdater: @@ -204,9 +214,9 @@ async def async_device_removed(event): async def async_tear_down_service(): """Handle the removal of the service.""" - services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) - if self._service.service_name in services: - services.remove(self._service.service_name) + services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + if self._service.service_name in services.keys(): + del services[self._service.service_name] if not self._device_removed and service.device_id: self._device_removed = True await cleanup_device_registry(hass, service.device_id) @@ -319,14 +329,14 @@ async def async_update_service( new_service_name != self._service_name or config[CONF_TARGETS] != self._targets ): - services = self.hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, set()) + services = self.hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) await self.async_unregister_services() if self._service_name in services: - services.remove(self._service_name) + del services[self._service_name] self._targets = config[CONF_TARGETS] self._service_name = new_service_name await self.async_register_services() - services.add(new_service_name) + services[new_service_name] = self if self.device_id: await _update_device(self.hass, self._config_entry, config) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index aecfe172a1bff0..a1019ed6d3a353 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -9,7 +9,7 @@ from homeassistant import config as hass_config from homeassistant.components import notify from homeassistant.components.mqtt import DOMAIN -from homeassistant.const import CONF_NAME, SERVICE_RELOAD +from homeassistant.const import CONF_NAME, SERVICE_RELOAD, STATE_UNKNOWN from homeassistant.exceptions import ServiceNotFound from homeassistant.setup import async_setup_component from homeassistant.util import slugify @@ -546,39 +546,51 @@ async def test_discovery_without_device(hass, mqtt_mock, caplog): async def test_discovery_with_device(hass, mqtt_mock, caplog, device_reg): """Test discovery, update and removal of notify service with a device config.""" - data = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Name" } }' - data_device_update = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Name update" } }' - async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) - await hass.async_block_till_done() - device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) - assert device_entry is not None - device_id = device_entry.id - assert ( - f"" - in caplog.text - ) - assert ( - "" - in caplog.text - ) - assert ( - "" - in caplog.text - ) - assert ( - "" - in caplog.text + + async def async_setup_notifify_service_with_auto_discovery( + hass, mqtt_mock, caplog, device_reg + ): + """Test setup notify service with a device config.""" + data = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Test123" } }' + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + device_id = device_entry.id + assert ( + f"" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + assert ( + "" + in caplog.text + ) + caplog.clear() + + # Initial setup + await async_setup_notifify_service_with_auto_discovery( + hass, mqtt_mock, caplog, device_reg ) - caplog.clear() # Test device update + data_device_update = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Name update" } }' async_fire_mqtt_message( hass, f"homeassistant/{notify.DOMAIN}/bla/config", data_device_update ) await hass.async_block_till_done() device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) assert device_entry is not None + device_id = device_entry.id assert device_id == device_entry.id + assert device_entry.name == "Name update" assert ( f"" in caplog.text @@ -609,36 +621,64 @@ async def test_discovery_with_device(hass, mqtt_mock, caplog, device_reg): caplog.clear() # Re-create the device again - async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) + await async_setup_notifify_service_with_auto_discovery( + hass, mqtt_mock, caplog, device_reg + ) + + # Remove the device from the device registry + device_reg.async_remove_device(device_id) await hass.async_block_till_done() - device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) - assert device_entry is not None - device_id = device_entry.id assert ( - f"" + f"" in caplog.text ) assert ( - "" + "" in caplog.text ) assert ( - "" + "" in caplog.text ) assert ( - "" + "" in caplog.text ) - caplog.clear() - - # Remove the device from the device registry - device_reg.async_remove_device(device_id) - await hass.async_block_till_done() assert ( f"" in caplog.text ) + assert "Notify service ('notify', 'bla') has been removed" in caplog.text + + # Add the notify service + await async_setup_notifify_service_with_auto_discovery( + hass, mqtt_mock, caplog, device_reg + ) + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + + # Setup a switch entity with a same device + data = '{ "command_topic": "test_topic", "name": "test", "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Test123" } }' + async_fire_mqtt_message(hass, "homeassistant/switch/bla/config", data) + await hass.async_block_till_done() + state = hass.states.get("switch.test") + assert state.state == STATE_UNKNOWN + + # Remove the switch service, the device should not be removed + # since there is a notify service registered + + # Test removal device from device registry + async_fire_mqtt_message(hass, "homeassistant/switch/bla/config", "{}") + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + device_id = device_entry.id + assert device_id == device_entry.id + + # Test removal of the notify service, the device should be removed now + caplog.clear() + async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", "{}") + await hass.async_block_till_done() assert ( "" in caplog.text @@ -656,7 +696,6 @@ async def test_discovery_with_device(hass, mqtt_mock, caplog, device_reg): in caplog.text ) assert "Notify service ('notify', 'bla') has been removed" in caplog.text - caplog.clear() async def test_publishing_with_custom_encoding(hass, mqtt_mock, caplog): From 70c566e7823cdb7d33011de7bd11de1a42133aed Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 6 Feb 2022 13:25:35 +0000 Subject: [PATCH 15/28] Improve tests --- tests/components/mqtt/test_notify.py | 173 ++++++++++++++++----------- 1 file changed, 103 insertions(+), 70 deletions(-) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index a1019ed6d3a353..3802347eb52227 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -9,7 +9,7 @@ from homeassistant import config as hass_config from homeassistant.components import notify from homeassistant.components.mqtt import DOMAIN -from homeassistant.const import CONF_NAME, SERVICE_RELOAD, STATE_UNKNOWN +from homeassistant.const import CONF_NAME, SERVICE_RELOAD from homeassistant.exceptions import ServiceNotFound from homeassistant.setup import async_setup_component from homeassistant.util import slugify @@ -100,6 +100,31 @@ def device_reg(hass): return mock_device_registry(hass) +async def async_setup_notifify_service_with_auto_discovery( + hass, mqtt_mock, caplog, device_reg, data, service_name +): + """Test setup notify service with a device config.""" + caplog.clear() + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/{service_name}/config", data + ) + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + + @pytest.mark.parametrize(*COMMAND_TEMPLATE_TEST_PARAMS) async def test_sending_with_command_templates_with_config_setup( hass, mqtt_mock, caplog, name, service, parameters, expected_result @@ -544,46 +569,20 @@ async def test_discovery_without_device(hass, mqtt_mock, caplog): ) -async def test_discovery_with_device(hass, mqtt_mock, caplog, device_reg): +async def test_discovery_with_device_update(hass, mqtt_mock, caplog, device_reg): """Test discovery, update and removal of notify service with a device config.""" - async def async_setup_notifify_service_with_auto_discovery( - hass, mqtt_mock, caplog, device_reg - ): - """Test setup notify service with a device config.""" - data = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Test123" } }' - async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/bla/config", data) - await hass.async_block_till_done() - device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) - assert device_entry is not None - device_id = device_entry.id - assert ( - f"" - in caplog.text - ) - assert ( - "" - in caplog.text - ) - assert ( - "" - in caplog.text - ) - assert ( - "" - in caplog.text - ) - caplog.clear() - # Initial setup + data = '{ "command_topic": "test_topic", "name": "My notify service", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Test123" } }' + service_name = "my_notify_service" await async_setup_notifify_service_with_auto_discovery( - hass, mqtt_mock, caplog, device_reg + hass, mqtt_mock, caplog, device_reg, data, service_name ) - + assert "" in caplog.text ) - caplog.clear() - # Re-create the device again + +async def test_discovery_with_device_removal(hass, mqtt_mock, caplog, device_reg): + """Test discovery, update and removal of notify service with a device config.""" + + # Initial setup + data1 = '{ "command_topic": "test_topic", "name": "My notify service1", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Test123" } }' + data2 = '{ "command_topic": "test_topic", "name": "My notify service2", "targets": ["target1", "target2"], "device":{"identifiers":["LCD_61236812_ADBA"], "name": "Test123" } }' + service_name1 = "my_notify_service1" + service_name2 = "my_notify_service2" + await async_setup_notifify_service_with_auto_discovery( + hass, mqtt_mock, caplog, device_reg, data1, service_name1 + ) + assert "" + f"" in caplog.text ) assert ( - "" + f"" in caplog.text ) assert ( - "" + f"" in caplog.text ) assert ( - "" + f"" + not in caplog.text + ) + caplog.clear() + + # The device should still be there + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is not None + device_id = device_entry.id + assert device_id == device_entry.id + assert device_entry.name == "Test123" + + # Test removal device from device registry after removing second service + async_fire_mqtt_message( + hass, f"homeassistant/{notify.DOMAIN}/{service_name2}/config", "{}" + ) + await hass.async_block_till_done() + device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) + assert device_entry is None + assert ( + f"" + in caplog.text + ) + assert ( + f"" + in caplog.text + ) + assert ( + f"" in caplog.text ) assert ( f"" in caplog.text ) - assert "Notify service ('notify', 'bla') has been removed" in caplog.text + caplog.clear() - # Add the notify service + # Recreate the service and device await async_setup_notifify_service_with_auto_discovery( - hass, mqtt_mock, caplog, device_reg + hass, mqtt_mock, caplog, device_reg, data1, service_name1 ) - device_entry = device_reg.async_get_device({("mqtt", "LCD_61236812_ADBA")}) - assert device_entry is not None + assert "" + f"" in caplog.text ) assert ( - "" + f"" in caplog.text ) assert ( - "" + f"" in caplog.text ) assert ( f"" in caplog.text ) - assert "Notify service ('notify', 'bla') has been removed" in caplog.text async def test_publishing_with_custom_encoding(hass, mqtt_mock, caplog): From e589d18be0d46ebb12310e7eb7c2650b11179ee0 Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 6 Feb 2022 13:40:02 +0000 Subject: [PATCH 16/28] update description has_notify_services() --- homeassistant/components/mqtt/notify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 5b5d3ca575ab43..1cb4daad5f13af 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -134,7 +134,7 @@ async def _async_setup_notify( def has_notify_services(hass: HomeAssistant, device_id: str) -> bool: - """Return a list of registered notify services.""" + """Check if the device has registered notify services.""" services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) for key, service in services.items(): # pylint: disable=unused-variable if service.device_id == device_id: From 2e9fc9a2c69e21aeca2e6b4bb0a2bd8f0309d2dd Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 16 Feb 2022 13:16:22 +0000 Subject: [PATCH 17/28] Use TypedDict for service config --- homeassistant/components/mqtt/const.py | 12 +- homeassistant/components/mqtt/notify.py | 162 ++++++++++++------------ tests/components/mqtt/test_discovery.py | 2 + 3 files changed, 87 insertions(+), 89 deletions(-) diff --git a/homeassistant/components/mqtt/const.py b/homeassistant/components/mqtt/const.py index 0feb21b0010047..b4edb7488ea194 100644 --- a/homeassistant/components/mqtt/const.py +++ b/homeassistant/components/mqtt/const.py @@ -1,4 +1,6 @@ """Constants used by multiple MQTT modules.""" +from typing import Final + from homeassistant.const import CONF_PAYLOAD ATTR_DISCOVERY_HASH = "discovery_hash" @@ -12,11 +14,11 @@ CONF_AVAILABILITY = "availability" CONF_BROKER = "broker" CONF_BIRTH_MESSAGE = "birth_message" -CONF_COMMAND_TEMPLATE = "command_template" -CONF_COMMAND_TOPIC = "command_topic" -CONF_ENCODING = "encoding" -CONF_QOS = ATTR_QOS -CONF_RETAIN = ATTR_RETAIN +CONF_COMMAND_TEMPLATE: Final = "command_template" +CONF_COMMAND_TOPIC: Final = "command_topic" +CONF_ENCODING: Final = "encoding" +CONF_QOS: Final = "qos" +CONF_RETAIN: Final = "retain" CONF_STATE_TOPIC = "state_topic" CONF_TOPIC = "topic" CONF_WILL_MESSAGE = "will_message" diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 1cb4daad5f13af..25e6ae188a2770 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -3,13 +3,13 @@ import functools import logging -from typing import Any +from typing import Any, Final, TypedDict import voluptuous as vol from homeassistant.components import notify from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_DEVICE, CONF_NAME +from homeassistant.const import CONF_DEVICE, CONF_DEVICE_ID, CONF_NAME from homeassistant.core import HomeAssistant import homeassistant.helpers.config_validation as cv from homeassistant.helpers.device_registry import EVENT_DEVICE_REGISTRY_UPDATED @@ -44,8 +44,10 @@ device_info_from_config, ) -CONF_TARGETS = "targets" -CONF_TITLE = "title" +CONF_TARGETS: Final = "targets" +CONF_TITLE: Final = "title" +CONF_CONFIG_ENTRY: Final = "config_entry" +CONF_DISCOVER_HASH: Final = "discovery_hash" MQTT_NOTIFY_SERVICES_SETUP = "mqtt_notify_services_setup" @@ -75,7 +77,7 @@ async def async_setup_entry( config_entry: ConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: - """Set up MQTT climate device dynamically through MQTT discovery.""" + """Set up MQTT notify service dynamically through MQTT discovery.""" await async_setup_reload_service(hass, DOMAIN, PLATFORMS) setup = functools.partial(_async_setup_notify, hass, config_entry=config_entry) await async_setup_entry_helper(hass, notify.DOMAIN, setup, DISCOVERY_SCHEMA) @@ -89,7 +91,7 @@ async def _async_setup_notify( ): """Set up the MQTT notify service with auto discovery.""" config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) - service_name = slugify(config.get(CONF_NAME) or DOMAIN) + service_name = slugify(config.get(CONF_NAME, DOMAIN)) services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) has_services = hass.services.has_service(notify.DOMAIN, service_name) discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] @@ -113,19 +115,17 @@ async def _async_setup_notify( device_id = device.id + service_config = MqttNotificationConfig(config) # type: ignore + service_config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( + config.get(CONF_COMMAND_TEMPLATE), hass=hass + ) + service_config[CONF_DISCOVER_HASH] = discovery_hash + service_config[CONF_DEVICE_ID] = device_id + service_config[CONF_CONFIG_ENTRY] = config_entry + service = MqttNotificationService( hass, - config[CONF_COMMAND_TOPIC], - MqttCommandTemplate(config.get(CONF_COMMAND_TEMPLATE), hass=hass), - config[CONF_ENCODING], - config.get(CONF_NAME), - config[CONF_QOS], - config[CONF_RETAIN], - config[CONF_TARGETS], - config[CONF_TITLE], - discovery_hash=discovery_hash, - device_id=device_id, - config_entry=config_entry, + service_config, ) services[service_name] = service @@ -148,10 +148,7 @@ async def async_get_service( discovery_info: DiscoveryInfoType | None = None, ) -> MqttNotificationService | None: """Prepare the MQTT notification service through configuration.yaml.""" - name = config.get(CONF_NAME) - if CONF_NAME not in config: - config[CONF_NAME] = DOMAIN - service_name = slugify(name or DOMAIN) + service_name = slugify(config.get(CONF_NAME, DOMAIN)) has_services = hass.services.has_service(notify.DOMAIN, service_name) services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) if service_name in services.keys() or has_services: @@ -161,20 +158,37 @@ async def async_get_service( ) return None await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + + service_config: MqttNotificationConfig = MqttNotificationConfig(config) # type: ignore + service_config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( + config.get(CONF_COMMAND_TEMPLATE), hass=hass + ) services[service_name] = MqttNotificationService( hass, - config[CONF_COMMAND_TOPIC], - MqttCommandTemplate(config.get(CONF_COMMAND_TEMPLATE), hass=hass), - config[CONF_ENCODING], - name, - config[CONF_QOS], - config[CONF_RETAIN], - config[CONF_TARGETS], - config[CONF_TITLE], + service_config, ) + # Override the service name if name is not set + if CONF_NAME not in config: + config[CONF_NAME] = DOMAIN return services[service_name] +class MqttNotificationConfig(TypedDict, total=False): + """Supply service parameters for MqttNotificationService.""" + + command_topic: str + command_template: MqttCommandTemplate + encoding: str + name: str | None + qos: int + retain: bool + targets: list + title: str + discovery_hash: tuple | None + device_id: str | None + config_entry: ConfigEntry | None + + class MqttNotificationServiceUpdater: """Add support for auto discovery updates.""" @@ -255,46 +269,28 @@ class MqttNotificationService(notify.BaseNotificationService): def __init__( self, hass: HomeAssistant, - command_topic: str, - command_template: MqttCommandTemplate, - encoding: str, - name: str | None, - qos: int, - retain: bool, - targets: list, - title: str | None, - discovery_hash: tuple | None = None, - device_id: str | None = None, - config_entry: ConfigEntry | None = None, + service_config: MqttNotificationConfig, ) -> None: """Initialize the service.""" self.hass = hass - self._command_topic = command_topic - self._command_template = command_template - self._encoding = encoding - self._name = name - self._qos = qos - self._retain = retain - self._targets = targets - self._title = title - self._discovery_hash = discovery_hash - self._device_id = device_id - self._config_entry = config_entry - self._service_name = slugify(name or DOMAIN) + self._config = service_config + self._service_name = slugify(service_config.get(CONF_NAME, DOMAIN)) self._updater = ( - MqttNotificationServiceUpdater(hass, self) if discovery_hash else None + MqttNotificationServiceUpdater(hass, self) + if service_config.get(CONF_DISCOVER_HASH) + else None ) @property def device_id(self) -> str | None: """Return the device ID.""" - return self._device_id + return self._config.get(CONF_DEVICE_ID) @property def discovery_hash(self) -> tuple | None: """Return the discovery hash.""" - return self._discovery_hash + return self._config.get(CONF_DISCOVER_HASH) @property def service_name(self) -> str: @@ -306,7 +302,7 @@ async def async_update_service( discovery_payload: DiscoveryInfoType, ) -> None: """Update the notify service through auto discovery.""" - config = DISCOVERY_SCHEMA(discovery_payload) + config: ConfigType = DISCOVERY_SCHEMA(discovery_payload) new_service_name = slugify(config.get(CONF_NAME, DOMAIN)) if new_service_name != self._service_name and self.hass.services.has_service( notify.DOMAIN, new_service_name @@ -316,78 +312,76 @@ async def async_update_service( new_service_name, ) return - self._command_topic = config[CONF_COMMAND_TOPIC] - self._command_template = MqttCommandTemplate( - config.get(CONF_COMMAND_TEMPLATE), hass=self.hass - ) - self._encoding = config[CONF_ENCODING] - self._name = config.get(CONF_NAME) - self._qos = config[CONF_QOS] - self._retain = config[CONF_RETAIN] - self._title = config[CONF_TITLE] if ( new_service_name != self._service_name - or config[CONF_TARGETS] != self._targets + or config[CONF_TARGETS] != self._config[CONF_TARGETS] ): services = self.hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) await self.async_unregister_services() if self._service_name in services: del services[self._service_name] - self._targets = config[CONF_TARGETS] + self._config = MqttNotificationConfig(config) # type: ignore self._service_name = new_service_name await self.async_register_services() services[new_service_name] = self - if self.device_id: - await _update_device(self.hass, self._config_entry, config) + else: + self._config = MqttNotificationConfig(config) # type: ignore + self._config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( + config.get(CONF_COMMAND_TEMPLATE), hass=self.hass + ) + if self._config[CONF_DEVICE_ID]: + await _update_device(self.hass, self._config[CONF_CONFIG_ENTRY], config) @property def targets(self) -> dict[str, str]: """Return a dictionary of registered targets.""" - return {target: target for target in self._targets} + return {target: target for target in self._config[CONF_TARGETS]} async def async_send_message(self, message: str = "", **kwargs): """Build and send a MQTT message.""" target = kwargs.get(notify.ATTR_TARGET) if ( target is not None - and self._targets - and set(target) & set(self._targets) != set(target) + and self._config[CONF_TARGETS] + and set(target) & set(self._config[CONF_TARGETS]) != set(target) ): _LOGGER.error( "Cannot send %s, target list %s is invalid, valid available targets: %s", message, target, - self._targets, + self._config[CONF_TARGETS], ) return variables = { "message": message, - "name": self._name, + "name": self._config.get(CONF_NAME), "service": self._service_name, - "target": target or self._targets, - "title": kwargs.get(notify.ATTR_TITLE, self._title), + "target": target or self._config[CONF_TARGETS], + "title": kwargs.get(notify.ATTR_TITLE, self._config[CONF_TITLE]), } variables.update(kwargs.get(notify.ATTR_DATA) or {}) - payload = self._command_template.async_render( + payload = self._config[CONF_COMMAND_TEMPLATE].async_render( message, variables=variables, ) await mqtt.async_publish( self.hass, - self._command_topic, + self._config[CONF_COMMAND_TOPIC], payload, - self._qos, - self._retain, - self._encoding, + self._config[CONF_QOS], + self._config[CONF_RETAIN], + self._config[CONF_ENCODING], ) -async def _update_device(hass, config_entry, config): +async def _update_device( + hass: HomeAssistant, config_entry: ConfigEntry | None, config: ConfigType +) -> None: """Update device registry.""" device_registry = await hass.helpers.device_registry.async_get_registry() - config_entry_id = config_entry.entry_id + config_entry_id = config_entry.entry_id if config_entry else None device_info = device_info_from_config(config[CONF_DEVICE]) if config_entry_id is not None and device_info is not None: - device_info["config_entry_id"] = config_entry_id + dict(device_info)["config_entry_id"] = config_entry_id device_registry.async_get_or_create(**device_info) diff --git a/tests/components/mqtt/test_discovery.py b/tests/components/mqtt/test_discovery.py index 5d94f349c580e4..769e5aa755eba9 100644 --- a/tests/components/mqtt/test_discovery.py +++ b/tests/components/mqtt/test_discovery.py @@ -835,8 +835,10 @@ async def test_discovery_expansion_without_encoding_and_value_template_2( "CONF_CLIENT_CERT", "CONF_CLIENT_ID", "CONF_CLIENT_KEY", + "CONF_CONFIG_ENTRY", "CONF_DISCOVERY", "CONF_DISCOVERY_ID", + "CONF_DISCOVER_HASH", "CONF_DISCOVERY_PREFIX", "CONF_EMBEDDED", "CONF_KEEPALIVE", From d48f3d447d807cb0a67a80fbcc0b2802d4a5ce2f Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 16 Feb 2022 16:12:45 +0000 Subject: [PATCH 18/28] casting- fix discovery - hass.data --- homeassistant/components/mqtt/notify.py | 75 +++++++++++++------------ tests/components/mqtt/test_notify.py | 45 +++++---------- 2 files changed, 51 insertions(+), 69 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 25e6ae188a2770..2d452858cf2e51 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -3,7 +3,7 @@ import functools import logging -from typing import Any, Final, TypedDict +from typing import Any, Final, TypedDict, cast import voluptuous as vol @@ -19,6 +19,7 @@ ) from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.reload import async_setup_reload_service +from homeassistant.helpers.template import Template from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from homeassistant.util import slugify @@ -55,7 +56,7 @@ { vol.Required(CONF_COMMAND_TOPIC): mqtt.valid_publish_topic, vol.Optional(CONF_COMMAND_TEMPLATE): cv.template, - vol.Optional(CONF_NAME): cv.string, + vol.Required(CONF_NAME): cv.string, vol.Optional(CONF_TARGETS, default=[]): cv.ensure_list, vol.Optional(CONF_TITLE, default=notify.ATTR_TITLE_DEFAULT): cv.string, vol.Optional(CONF_RETAIN, default=mqtt.DEFAULT_RETAIN): cv.boolean, @@ -78,6 +79,7 @@ async def async_setup_entry( async_add_entities: AddEntitiesCallback, ) -> None: """Set up MQTT notify service dynamically through MQTT discovery.""" + hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) await async_setup_reload_service(hass, DOMAIN, PLATFORMS) setup = functools.partial(_async_setup_notify, hass, config_entry=config_entry) await async_setup_entry_helper(hass, notify.DOMAIN, setup, DISCOVERY_SCHEMA) @@ -91,8 +93,8 @@ async def _async_setup_notify( ): """Set up the MQTT notify service with auto discovery.""" config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) - service_name = slugify(config.get(CONF_NAME, DOMAIN)) - services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + service_name = slugify(config[CONF_NAME]) + services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] has_services = hass.services.has_service(notify.DOMAIN, service_name) discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] if service_name in services.keys() or has_services: @@ -115,10 +117,7 @@ async def _async_setup_notify( device_id = device.id - service_config = MqttNotificationConfig(config) # type: ignore - service_config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( - config.get(CONF_COMMAND_TEMPLATE), hass=hass - ) + service_config = cast(MqttNotificationConfig, config) service_config[CONF_DISCOVER_HASH] = discovery_hash service_config[CONF_DEVICE_ID] = device_id service_config[CONF_CONFIG_ENTRY] = config_entry @@ -135,7 +134,7 @@ async def _async_setup_notify( def has_notify_services(hass: HomeAssistant, device_id: str) -> bool: """Check if the device has registered notify services.""" - services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] for key, service in services.items(): # pylint: disable=unused-variable if service.device_id == device_id: return True @@ -148,28 +147,23 @@ async def async_get_service( discovery_info: DiscoveryInfoType | None = None, ) -> MqttNotificationService | None: """Prepare the MQTT notification service through configuration.yaml.""" - service_name = slugify(config.get(CONF_NAME, DOMAIN)) + hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + + service_name = slugify(config[CONF_NAME]) has_services = hass.services.has_service(notify.DOMAIN, service_name) - services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] if service_name in services.keys() or has_services: _LOGGER.error( "Notify service '%s' is not unique, cannot register service(s)", service_name, ) return None - await async_setup_reload_service(hass, DOMAIN, PLATFORMS) - service_config: MqttNotificationConfig = MqttNotificationConfig(config) # type: ignore - service_config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( - config.get(CONF_COMMAND_TEMPLATE), hass=hass - ) services[service_name] = MqttNotificationService( hass, - service_config, + cast(MqttNotificationConfig, config), ) - # Override the service name if name is not set - if CONF_NAME not in config: - config[CONF_NAME] = DOMAIN return services[service_name] @@ -177,7 +171,7 @@ class MqttNotificationConfig(TypedDict, total=False): """Supply service parameters for MqttNotificationService.""" command_topic: str - command_template: MqttCommandTemplate + command_template: Template encoding: str name: str | None qos: int @@ -228,7 +222,7 @@ async def async_device_removed(event): async def async_tear_down_service(): """Handle the removal of the service.""" - services = hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] if self._service.service_name in services.keys(): del services[self._service.service_name] if not self._device_removed and service.device_id: @@ -274,7 +268,13 @@ def __init__( """Initialize the service.""" self.hass = hass self._config = service_config - self._service_name = slugify(service_config.get(CONF_NAME, DOMAIN)) + self._commmand_template = MqttCommandTemplate( + service_config.get(CONF_COMMAND_TEMPLATE), hass=hass + ) + self._device_id = self._config.get(CONF_DEVICE_ID) + self._discovery_hash = self._config.get(CONF_DISCOVER_HASH) + self._config_entry = self._config.get(CONF_CONFIG_ENTRY) + self._service_name = slugify(service_config[CONF_NAME]) self._updater = ( MqttNotificationServiceUpdater(hass, self) @@ -285,12 +285,12 @@ def __init__( @property def device_id(self) -> str | None: """Return the device ID.""" - return self._config.get(CONF_DEVICE_ID) + return self._device_id @property def discovery_hash(self) -> tuple | None: """Return the discovery hash.""" - return self._config.get(CONF_DISCOVER_HASH) + return self._discovery_hash @property def service_name(self) -> str: @@ -303,7 +303,7 @@ async def async_update_service( ) -> None: """Update the notify service through auto discovery.""" config: ConfigType = DISCOVERY_SCHEMA(discovery_payload) - new_service_name = slugify(config.get(CONF_NAME, DOMAIN)) + new_service_name = slugify(config[CONF_NAME]) if new_service_name != self._service_name and self.hass.services.has_service( notify.DOMAIN, new_service_name ): @@ -320,17 +320,17 @@ async def async_update_service( await self.async_unregister_services() if self._service_name in services: del services[self._service_name] - self._config = MqttNotificationConfig(config) # type: ignore + self._config = cast(MqttNotificationConfig, config) self._service_name = new_service_name await self.async_register_services() services[new_service_name] = self else: - self._config = MqttNotificationConfig(config) # type: ignore - self._config[CONF_COMMAND_TEMPLATE] = MqttCommandTemplate( + self._config = cast(MqttNotificationConfig, config) + self._commmand_template = MqttCommandTemplate( config.get(CONF_COMMAND_TEMPLATE), hass=self.hass ) - if self._config[CONF_DEVICE_ID]: - await _update_device(self.hass, self._config[CONF_CONFIG_ENTRY], config) + if CONF_DEVICE in config and self._config_entry: + await _update_device(self.hass, self._config_entry, config) @property def targets(self) -> dict[str, str]: @@ -354,13 +354,13 @@ async def async_send_message(self, message: str = "", **kwargs): return variables = { "message": message, - "name": self._config.get(CONF_NAME), + "name": self._config[CONF_NAME], "service": self._service_name, "target": target or self._config[CONF_TARGETS], "title": kwargs.get(notify.ATTR_TITLE, self._config[CONF_TITLE]), } variables.update(kwargs.get(notify.ATTR_DATA) or {}) - payload = self._config[CONF_COMMAND_TEMPLATE].async_render( + payload = self._commmand_template.async_render( message, variables=variables, ) @@ -375,13 +375,14 @@ async def async_send_message(self, message: str = "", **kwargs): async def _update_device( - hass: HomeAssistant, config_entry: ConfigEntry | None, config: ConfigType + hass: HomeAssistant, config_entry: ConfigEntry, config: ConfigType ) -> None: """Update device registry.""" device_registry = await hass.helpers.device_registry.async_get_registry() - config_entry_id = config_entry.entry_id if config_entry else None + config_entry_id = config_entry.entry_id device_info = device_info_from_config(config[CONF_DEVICE]) if config_entry_id is not None and device_info is not None: - dict(device_info)["config_entry_id"] = config_entry_id - device_registry.async_get_or_create(**device_info) + update_device_info = dict(device_info) + update_device_info["config_entry_id"] = config_entry_id + device_registry.async_get_or_create(**update_device_info) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index 3802347eb52227..38b32c145d4b12 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -22,16 +22,16 @@ "name,service,parameters,expected_result", [ ( - None, - "mqtt", + "My service", + "my_service", { notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message", notify.ATTR_DATA: {"par1": "val1"}, }, '{"message":"Message",' - '"name":"None",' - '"service":"mqtt",' + '"name":"My service",' + '"service":"my_service",' '"par1":"val1",' '"target":[' "'t1', 't2'" @@ -39,8 +39,8 @@ '"title":"Title"}', ), ( - None, - "mqtt", + "My service", + "my_service", { notify.ATTR_TITLE: "Title", notify.ATTR_MESSAGE: "Message", @@ -48,36 +48,19 @@ notify.ATTR_TARGET: ["t2"], }, '{"message":"Message",' - '"name":"None",' - '"service":"mqtt",' + '"name":"My service",' + '"service":"my_service",' '"par1":"val1",' '"target":[' "'t2'" "]," '"title":"Title"}', ), - ( - None, - "mqtt_t1", - { - notify.ATTR_TITLE: "Title", - notify.ATTR_MESSAGE: "Message", - notify.ATTR_DATA: {"par1": "val2"}, - }, - '{"message":"Message",' - '"name":"None",' - '"service":"mqtt",' - '"par1":"val2",' - '"target":[' - "'t1'" - "]," - '"title":"Title"}', - ), ( "My service", "my_service_t1", { - notify.ATTR_TITLE: "Title", + notify.ATTR_TITLE: "Title2", notify.ATTR_MESSAGE: "Message", notify.ATTR_DATA: {"par1": "val2"}, }, @@ -88,7 +71,7 @@ '"target":[' "'t1'" "]," - '"title":"Title"}', + '"title":"Title2"}', ), ], ) @@ -131,6 +114,7 @@ async def test_sending_with_command_templates_with_config_setup( ): """Test the sending MQTT commands using a template using config setup.""" config = { + "name": name, "command_topic": "lcd/set", "command_template": "{" '"message":"{{message}}",' @@ -144,11 +128,7 @@ async def test_sending_with_command_templates_with_config_setup( "platform": "mqtt", "qos": "1", } - if name: - config[CONF_NAME] = name - service_base_name = slugify(name) - else: - service_base_name = DOMAIN + service_base_name = slugify(name) assert await async_setup_component( hass, notify.DOMAIN, @@ -185,6 +165,7 @@ async def test_sending_with_command_templates_auto_discovery( ): """Test the sending MQTT commands using a template and auto discovery.""" config = { + "name": name, "command_topic": "lcd/set", "command_template": "{" '"message":"{{message}}",' From a65d5e3a8a1a5cdbb3cc2ca62caae0a57870942a Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 16 Feb 2022 22:35:52 +0000 Subject: [PATCH 19/28] cleanup --- homeassistant/components/mqtt/notify.py | 115 +++++++++++++----------- tests/components/mqtt/test_discovery.py | 2 - tests/components/mqtt/test_notify.py | 10 +-- 3 files changed, 65 insertions(+), 62 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 2d452858cf2e51..900e10f281e214 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -9,7 +9,7 @@ from homeassistant.components import notify from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_DEVICE, CONF_DEVICE_ID, CONF_NAME +from homeassistant.const import CONF_DEVICE, CONF_NAME from homeassistant.core import HomeAssistant import homeassistant.helpers.config_validation as cv from homeassistant.helpers.device_registry import EVENT_DEVICE_REGISTRY_UPDATED @@ -73,14 +73,33 @@ _LOGGER = logging.getLogger(__name__) +async def async_initialize(hass: HomeAssistant) -> None: + """Initialize globals.""" + await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + + +def _check_notify_service_name(hass: HomeAssistant, config: ConfigType) -> str | None: + """Check if the service already exists or else return the service name.""" + service_name = slugify(config[CONF_NAME]) + has_services = hass.services.has_service(notify.DOMAIN, service_name) + services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] + if service_name in services.keys() or has_services: + _LOGGER.error( + "Notify service '%s' already exists, cannot register service", + service_name, + ) + return None + return service_name + + async def async_setup_entry( hass: HomeAssistant, config_entry: ConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: """Set up MQTT notify service dynamically through MQTT discovery.""" - hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) - await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + await async_initialize(hass) setup = functools.partial(_async_setup_notify, hass, config_entry=config_entry) await async_setup_entry_helper(hass, notify.DOMAIN, setup, DISCOVERY_SCHEMA) @@ -93,40 +112,31 @@ async def _async_setup_notify( ): """Set up the MQTT notify service with auto discovery.""" config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) - service_name = slugify(config[CONF_NAME]) - services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] - has_services = hass.services.has_service(notify.DOMAIN, service_name) discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] - if service_name in services.keys() or has_services: - _LOGGER.error( - "Notify service '%s' already exists, cannot register service(s)", - service_name, - ) + + if not (service_name := _check_notify_service_name(hass, config)): async_dispatcher_send(hass, MQTT_DISCOVERY_DONE.format(discovery_hash), None) clear_discovery_hash(hass, discovery_hash) return + device_id = None if CONF_DEVICE in config: await _update_device(hass, config_entry, config) - device_registry = await hass.helpers.device_registry.async_get_registry() device = device_registry.async_get_device( {(DOMAIN, id_) for id_ in config[CONF_DEVICE][CONF_IDENTIFIERS]}, {tuple(x) for x in config[CONF_DEVICE][CONF_CONNECTIONS]}, ) - device_id = device.id - service_config = cast(MqttNotificationConfig, config) - service_config[CONF_DISCOVER_HASH] = discovery_hash - service_config[CONF_DEVICE_ID] = device_id - service_config[CONF_CONFIG_ENTRY] = config_entry - service = MqttNotificationService( hass, - service_config, + cast(MqttNotificationConfig, config), + config_entry, + device_id, + discovery_hash, ) - services[service_name] = service + hass.data[MQTT_NOTIFY_SERVICES_SETUP][service_name] = service await service.async_setup(hass, service_name, service_name) await service.async_register_services() @@ -134,8 +144,9 @@ async def _async_setup_notify( def has_notify_services(hass: HomeAssistant, device_id: str) -> bool: """Check if the device has registered notify services.""" - services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] - for key, service in services.items(): # pylint: disable=unused-variable + for key, service in hass.data[ # pylint: disable=unused-variable + MQTT_NOTIFY_SERVICES_SETUP + ].items(): if service.device_id == device_id: return True return False @@ -147,24 +158,19 @@ async def async_get_service( discovery_info: DiscoveryInfoType | None = None, ) -> MqttNotificationService | None: """Prepare the MQTT notification service through configuration.yaml.""" - hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + await async_initialize(hass) await async_setup_reload_service(hass, DOMAIN, PLATFORMS) - service_name = slugify(config[CONF_NAME]) - has_services = hass.services.has_service(notify.DOMAIN, service_name) - services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] - if service_name in services.keys() or has_services: - _LOGGER.error( - "Notify service '%s' is not unique, cannot register service(s)", - service_name, - ) + if not (service_name := _check_notify_service_name(hass, config)): return None - services[service_name] = MqttNotificationService( + service = hass.data[MQTT_NOTIFY_SERVICES_SETUP][ + service_name + ] = MqttNotificationService( hass, cast(MqttNotificationConfig, config), ) - return services[service_name] + return service class MqttNotificationConfig(TypedDict, total=False): @@ -178,9 +184,7 @@ class MqttNotificationConfig(TypedDict, total=False): retain: bool targets: list title: str - discovery_hash: tuple | None - device_id: str | None - config_entry: ConfigEntry | None + device: ConfigType class MqttNotificationServiceUpdater: @@ -193,11 +197,11 @@ async def async_discovery_update( discovery_payload: DiscoveryInfoType | None, ) -> None: """Handle discovery update.""" - async_dispatcher_send( - hass, MQTT_DISCOVERY_DONE.format(service.discovery_hash), None - ) if not discovery_payload: # unregister notify service through auto discovery + async_dispatcher_send( + hass, MQTT_DISCOVERY_DONE.format(service.discovery_hash), None + ) await async_tear_down_service() return @@ -207,6 +211,9 @@ async def async_discovery_update( "Notify service %s updated has been processed", service.discovery_hash, ) + async_dispatcher_send( + hass, MQTT_DISCOVERY_DONE.format(service.discovery_hash), None + ) async def async_device_removed(event): """Handle the removal of a device.""" @@ -264,6 +271,9 @@ def __init__( self, hass: HomeAssistant, service_config: MqttNotificationConfig, + config_entry: ConfigEntry | None = None, + device_id: str | None = None, + discovery_hash: tuple | None = None, ) -> None: """Initialize the service.""" self.hass = hass @@ -271,15 +281,13 @@ def __init__( self._commmand_template = MqttCommandTemplate( service_config.get(CONF_COMMAND_TEMPLATE), hass=hass ) - self._device_id = self._config.get(CONF_DEVICE_ID) - self._discovery_hash = self._config.get(CONF_DISCOVER_HASH) - self._config_entry = self._config.get(CONF_CONFIG_ENTRY) + self._device_id = device_id + self._discovery_hash = discovery_hash + self._config_entry = config_entry self._service_name = slugify(service_config[CONF_NAME]) self._updater = ( - MqttNotificationServiceUpdater(hass, self) - if service_config.get(CONF_DISCOVER_HASH) - else None + MqttNotificationServiceUpdater(hass, self) if discovery_hash else None ) @property @@ -303,20 +311,19 @@ async def async_update_service( ) -> None: """Update the notify service through auto discovery.""" config: ConfigType = DISCOVERY_SCHEMA(discovery_payload) - new_service_name = slugify(config[CONF_NAME]) - if new_service_name != self._service_name and self.hass.services.has_service( - notify.DOMAIN, new_service_name - ): - _LOGGER.error( - "Notify service '%s' already exists, cannot update the existing service(s)", - new_service_name, - ) + # Do not rename a service if that service_name is already in use + if ( + new_service_name := slugify(config[CONF_NAME]) + ) != self._service_name and _check_notify_service_name( + self.hass, config + ) is None: return + # Only refresh services if service name or targets have changes if ( new_service_name != self._service_name or config[CONF_TARGETS] != self._config[CONF_TARGETS] ): - services = self.hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) + services = self.hass.data[MQTT_NOTIFY_SERVICES_SETUP] await self.async_unregister_services() if self._service_name in services: del services[self._service_name] diff --git a/tests/components/mqtt/test_discovery.py b/tests/components/mqtt/test_discovery.py index 769e5aa755eba9..5d94f349c580e4 100644 --- a/tests/components/mqtt/test_discovery.py +++ b/tests/components/mqtt/test_discovery.py @@ -835,10 +835,8 @@ async def test_discovery_expansion_without_encoding_and_value_template_2( "CONF_CLIENT_CERT", "CONF_CLIENT_ID", "CONF_CLIENT_KEY", - "CONF_CONFIG_ENTRY", "CONF_DISCOVERY", "CONF_DISCOVERY_ID", - "CONF_DISCOVER_HASH", "CONF_DISCOVERY_PREFIX", "CONF_EMBEDDED", "CONF_KEEPALIVE", diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index 38b32c145d4b12..947eb0e5432701 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -360,7 +360,7 @@ async def test_with_same_name(hass, mqtt_mock, caplog): in caplog.text ) assert ( - "Notify service 'test_same_name' is not unique, cannot register service(s)" + "Notify service 'test_same_name' already exists, cannot register service" in caplog.text ) @@ -501,7 +501,7 @@ async def test_discovery_without_device(hass, mqtt_mock, caplog): ) await hass.async_block_till_done() assert ( - "Notify service 'test_setup1' already exists, cannot register service(s)" + "Notify service 'test_setup1' already exists, cannot register service" in caplog.text ) await hass.services.async_call( @@ -535,8 +535,7 @@ async def test_discovery_without_device(hass, mqtt_mock, caplog): async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/testc/config", data) await hass.async_block_till_done() assert ( - "Notify service 'testa' already exists, cannot register service(s)" - in caplog.text + "Notify service 'testa' already exists, cannot register service" in caplog.text ) # Try to update the same discovery to existing service test @@ -545,8 +544,7 @@ async def test_discovery_without_device(hass, mqtt_mock, caplog): async_fire_mqtt_message(hass, f"homeassistant/{notify.DOMAIN}/testb/config", data) await hass.async_block_till_done() assert ( - "Notify service 'testa' already exists, cannot update the existing service(s)" - in caplog.text + "Notify service 'testa' already exists, cannot register service" in caplog.text ) From 0a98578f275ac34be1baec078b8a8f74c257db7f Mon Sep 17 00:00:00 2001 From: jbouwh Date: Wed, 16 Feb 2022 22:44:57 +0000 Subject: [PATCH 20/28] move MqttNotificationConfig after the schemas --- homeassistant/components/mqtt/notify.py | 28 ++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 900e10f281e214..a693b169d1d12f 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -73,6 +73,20 @@ _LOGGER = logging.getLogger(__name__) +class MqttNotificationConfig(TypedDict, total=False): + """Supply service parameters for MqttNotificationService.""" + + command_topic: str + command_template: Template + encoding: str + name: str | None + qos: int + retain: bool + targets: list + title: str + device: ConfigType + + async def async_initialize(hass: HomeAssistant) -> None: """Initialize globals.""" await async_setup_reload_service(hass, DOMAIN, PLATFORMS) @@ -173,20 +187,6 @@ async def async_get_service( return service -class MqttNotificationConfig(TypedDict, total=False): - """Supply service parameters for MqttNotificationService.""" - - command_topic: str - command_template: Template - encoding: str - name: str | None - qos: int - retain: bool - targets: list - title: str - device: ConfigType - - class MqttNotificationServiceUpdater: """Add support for auto discovery updates.""" From 2813e7027b36455ffd9e578a2cd6133be61916bd Mon Sep 17 00:00:00 2001 From: jbouwh Date: Thu, 17 Feb 2022 00:07:54 +0000 Subject: [PATCH 21/28] fix has_notify_services --- homeassistant/components/mqtt/notify.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index a693b169d1d12f..242d605b2ae924 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -158,6 +158,8 @@ async def _async_setup_notify( def has_notify_services(hass: HomeAssistant, device_id: str) -> bool: """Check if the device has registered notify services.""" + if MQTT_NOTIFY_SERVICES_SETUP not in hass.data: + return False for key, service in hass.data[ # pylint: disable=unused-variable MQTT_NOTIFY_SERVICES_SETUP ].items(): From 24bc03267068c591c89357058df7acbb18891e4e Mon Sep 17 00:00:00 2001 From: jbouwh Date: Thu, 17 Feb 2022 11:43:27 +0000 Subject: [PATCH 22/28] do not test log for reg update --- tests/components/mqtt/test_notify.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/components/mqtt/test_notify.py b/tests/components/mqtt/test_notify.py index 947eb0e5432701..33a32d858af6ac 100644 --- a/tests/components/mqtt/test_notify.py +++ b/tests/components/mqtt/test_notify.py @@ -569,11 +569,6 @@ async def test_discovery_with_device_update(hass, mqtt_mock, caplog, device_reg) device_id = device_entry.id assert device_id == device_entry.id assert device_entry.name == "Name update" - assert ( - f"" - in caplog.text - ) - caplog.clear() # Test removal device from device registry using discovery caplog.clear() From f5c3f5d58b1f755787d9d8b104608aae99867d6e Mon Sep 17 00:00:00 2001 From: jbouwh Date: Thu, 17 Feb 2022 13:38:56 +0000 Subject: [PATCH 23/28] Improve casting types --- homeassistant/components/mqtt/notify.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 242d605b2ae924..b1d2248bc1fa37 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -93,7 +93,9 @@ async def async_initialize(hass: HomeAssistant) -> None: hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) -def _check_notify_service_name(hass: HomeAssistant, config: ConfigType) -> str | None: +def _check_notify_service_name( + hass: HomeAssistant, config: MqttNotificationConfig +) -> str | None: """Check if the service already exists or else return the service name.""" service_name = slugify(config[CONF_NAME]) has_services = hass.services.has_service(notify.DOMAIN, service_name) @@ -125,7 +127,9 @@ async def _async_setup_notify( discovery_data: dict[str, Any], ): """Set up the MQTT notify service with auto discovery.""" - config = DISCOVERY_SCHEMA(discovery_data[ATTR_DISCOVERY_PAYLOAD]) + config: MqttNotificationConfig = DISCOVERY_SCHEMA( + discovery_data[ATTR_DISCOVERY_PAYLOAD] + ) discovery_hash = discovery_data[ATTR_DISCOVERY_HASH] if not (service_name := _check_notify_service_name(hass, config)): @@ -145,7 +149,7 @@ async def _async_setup_notify( service = MqttNotificationService( hass, - cast(MqttNotificationConfig, config), + config, config_entry, device_id, discovery_hash, @@ -175,16 +179,16 @@ async def async_get_service( ) -> MqttNotificationService | None: """Prepare the MQTT notification service through configuration.yaml.""" await async_initialize(hass) - await async_setup_reload_service(hass, DOMAIN, PLATFORMS) + notification_config: MqttNotificationConfig = cast(MqttNotificationConfig, config) - if not (service_name := _check_notify_service_name(hass, config)): + if not (service_name := _check_notify_service_name(hass, notification_config)): return None service = hass.data[MQTT_NOTIFY_SERVICES_SETUP][ service_name ] = MqttNotificationService( hass, - cast(MqttNotificationConfig, config), + notification_config, ) return service @@ -312,7 +316,7 @@ async def async_update_service( discovery_payload: DiscoveryInfoType, ) -> None: """Update the notify service through auto discovery.""" - config: ConfigType = DISCOVERY_SCHEMA(discovery_payload) + config: MqttNotificationConfig = DISCOVERY_SCHEMA(discovery_payload) # Do not rename a service if that service_name is already in use if ( new_service_name := slugify(config[CONF_NAME]) @@ -329,12 +333,12 @@ async def async_update_service( await self.async_unregister_services() if self._service_name in services: del services[self._service_name] - self._config = cast(MqttNotificationConfig, config) + self._config = config self._service_name = new_service_name await self.async_register_services() services[new_service_name] = self else: - self._config = cast(MqttNotificationConfig, config) + self._config = config self._commmand_template = MqttCommandTemplate( config.get(CONF_COMMAND_TEMPLATE), hass=self.hass ) @@ -384,7 +388,7 @@ async def async_send_message(self, message: str = "", **kwargs): async def _update_device( - hass: HomeAssistant, config_entry: ConfigEntry, config: ConfigType + hass: HomeAssistant, config_entry: ConfigEntry, config: MqttNotificationConfig ) -> None: """Update device registry.""" device_registry = await hass.helpers.device_registry.async_get_registry() From 1baf4a123656ae86ccb8682b4b7a57621e2a6bf2 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Thu, 17 Feb 2022 14:45:43 +0100 Subject: [PATCH 24/28] Simplify obtaining the device_id Co-authored-by: Erik Montnemery --- homeassistant/components/mqtt/notify.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index b1d2248bc1fa37..76d047f029e27a 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -139,13 +139,7 @@ async def _async_setup_notify( device_id = None if CONF_DEVICE in config: - await _update_device(hass, config_entry, config) - device_registry = await hass.helpers.device_registry.async_get_registry() - device = device_registry.async_get_device( - {(DOMAIN, id_) for id_ in config[CONF_DEVICE][CONF_IDENTIFIERS]}, - {tuple(x) for x in config[CONF_DEVICE][CONF_CONNECTIONS]}, - ) - device_id = device.id + device_id = _update_device(hass, config_entry, config).id service = MqttNotificationService( hass, From ac7cb2d724544d7710a94df3b3dcbcbe90bc47e9 Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Thu, 17 Feb 2022 14:46:22 +0100 Subject: [PATCH 25/28] await not needed Co-authored-by: Erik Montnemery --- homeassistant/components/mqtt/notify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 76d047f029e27a..ddd7eb85700caf 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -206,7 +206,7 @@ async def async_discovery_update( return # update notify service through auto discovery - await service.async_update_service(discovery_payload) + service.async_update_service(discovery_payload) _LOGGER.debug( "Notify service %s updated has been processed", service.discovery_hash, From 8fa82222a7c54a36d297aa5b01231e58d031816e Mon Sep 17 00:00:00 2001 From: jbouwh Date: Thu, 17 Feb 2022 16:27:42 +0000 Subject: [PATCH 26/28] Improve casting types and naming --- homeassistant/components/mqtt/mixins.py | 2 +- homeassistant/components/mqtt/notify.py | 56 +++++++++++++------------ 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index d593acf5cee48e..98ce705f4c4b75 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -468,7 +468,7 @@ async def cleanup_device_registry(hass: HomeAssistant, device_id: str | None) -> ) and not await device_trigger.async_get_triggers(hass, device_id) and not tag.async_has_tags(hass, device_id) - and not notify.has_notify_services(hass, device_id) + and not notify.device_has_notify_services(hass, device_id) ): device_registry.async_remove_device(device_id) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index ddd7eb85700caf..5ebb26fb5158c6 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -11,6 +11,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_DEVICE, CONF_NAME from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr import homeassistant.helpers.config_validation as cv from homeassistant.helpers.device_registry import EVENT_DEVICE_REGISTRY_UPDATED from homeassistant.helpers.dispatcher import ( @@ -37,8 +38,6 @@ ) from .discovery import MQTT_DISCOVERY_DONE, MQTT_DISCOVERY_UPDATED, clear_discovery_hash from .mixins import ( - CONF_CONNECTIONS, - CONF_IDENTIFIERS, MQTT_ENTITY_DEVICE_INFO_SCHEMA, async_setup_entry_helper, cleanup_device_registry, @@ -93,6 +92,18 @@ async def async_initialize(hass: HomeAssistant) -> None: hass.data.setdefault(MQTT_NOTIFY_SERVICES_SETUP, {}) +def device_has_notify_services(hass: HomeAssistant, device_id: str) -> bool: + """Check if the device has registered notify services.""" + if MQTT_NOTIFY_SERVICES_SETUP not in hass.data: + return False + for key, service in hass.data[ # pylint: disable=unused-variable + MQTT_NOTIFY_SERVICES_SETUP + ].items(): + if service.device_id == device_id: + return True + return False + + def _check_notify_service_name( hass: HomeAssistant, config: MqttNotificationConfig ) -> str | None: @@ -137,9 +148,7 @@ async def _async_setup_notify( clear_discovery_hash(hass, discovery_hash) return - device_id = None - if CONF_DEVICE in config: - device_id = _update_device(hass, config_entry, config).id + device_id = _update_device(hass, config_entry, config) service = MqttNotificationService( hass, @@ -154,18 +163,6 @@ async def _async_setup_notify( await service.async_register_services() -def has_notify_services(hass: HomeAssistant, device_id: str) -> bool: - """Check if the device has registered notify services.""" - if MQTT_NOTIFY_SERVICES_SETUP not in hass.data: - return False - for key, service in hass.data[ # pylint: disable=unused-variable - MQTT_NOTIFY_SERVICES_SETUP - ].items(): - if service.device_id == device_id: - return True - return False - - async def async_get_service( hass: HomeAssistant, config: ConfigType, @@ -206,7 +203,7 @@ async def async_discovery_update( return # update notify service through auto discovery - service.async_update_service(discovery_payload) + await service.async_update_service(discovery_payload) _LOGGER.debug( "Notify service %s updated has been processed", service.discovery_hash, @@ -336,8 +333,7 @@ async def async_update_service( self._commmand_template = MqttCommandTemplate( config.get(CONF_COMMAND_TEMPLATE), hass=self.hass ) - if CONF_DEVICE in config and self._config_entry: - await _update_device(self.hass, self._config_entry, config) + _update_device(self.hass, self._config_entry, config) @property def targets(self) -> dict[str, str]: @@ -381,15 +377,23 @@ async def async_send_message(self, message: str = "", **kwargs): ) -async def _update_device( - hass: HomeAssistant, config_entry: ConfigEntry, config: MqttNotificationConfig -) -> None: +def _update_device( + hass: HomeAssistant, + config_entry: ConfigEntry | None, + config: MqttNotificationConfig, +) -> str | None: """Update device registry.""" - device_registry = await hass.helpers.device_registry.async_get_registry() + if config_entry is None or CONF_DEVICE not in config: + return None + + device = None + device_registry = dr.async_get(hass) config_entry_id = config_entry.entry_id device_info = device_info_from_config(config[CONF_DEVICE]) if config_entry_id is not None and device_info is not None: - update_device_info = dict(device_info) + update_device_info = cast(dict, device_info) update_device_info["config_entry_id"] = config_entry_id - device_registry.async_get_or_create(**update_device_info) + device = device_registry.async_get_or_create(**update_device_info) + + return device.id if device else None From 185e672a7ff70266cd54820708b56cb15780406d Mon Sep 17 00:00:00 2001 From: jbouwh Date: Sun, 20 Feb 2022 18:44:48 +0000 Subject: [PATCH 27/28] cleanup_device_registry signature change and black --- homeassistant/components/mqtt/mixins.py | 5 ++++- homeassistant/components/mqtt/notify.py | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index 3441e99f246755..274a19b9e2e940 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -496,7 +496,9 @@ def available(self) -> bool: return self._available_latest -async def cleanup_device_registry(hass: HomeAssistant, device_id: str | None, config_entry_id: str | None) -> None: +async def cleanup_device_registry( + hass: HomeAssistant, device_id: str | None, config_entry_id: str | None +) -> None: """Remove device registry entry if there are no remaining entities, triggers or notify services.""" # Local import to avoid circular dependencies # pylint: disable=import-outside-toplevel @@ -506,6 +508,7 @@ async def cleanup_device_registry(hass: HomeAssistant, device_id: str | None, co entity_registry = er.async_get(hass) if ( device_id + and config_entry_id and not er.async_entries_for_device( entity_registry, device_id, include_disabled_entities=False ) diff --git a/homeassistant/components/mqtt/notify.py b/homeassistant/components/mqtt/notify.py index 5ebb26fb5158c6..9ba341aab0daa3 100644 --- a/homeassistant/components/mqtt/notify.py +++ b/homeassistant/components/mqtt/notify.py @@ -229,9 +229,11 @@ async def async_tear_down_service(): services = hass.data[MQTT_NOTIFY_SERVICES_SETUP] if self._service.service_name in services.keys(): del services[self._service.service_name] - if not self._device_removed and service.device_id: + if not self._device_removed and service.config_entry: self._device_removed = True - await cleanup_device_registry(hass, service.device_id) + await cleanup_device_registry( + hass, service.device_id, service.config_entry.entry_id + ) clear_discovery_hash(hass, service.discovery_hash) self._remove_discovery() await service.async_unregister_services() @@ -292,6 +294,11 @@ def device_id(self) -> str | None: """Return the device ID.""" return self._device_id + @property + def config_entry(self) -> ConfigEntry | None: + """Return the config_entry.""" + return self._config_entry + @property def discovery_hash(self) -> tuple | None: """Return the discovery hash.""" From ed3b59f9b85cc7284a6c5d44702675af4214db7a Mon Sep 17 00:00:00 2001 From: jbouwh Date: Tue, 8 Mar 2022 13:43:47 +0000 Subject: [PATCH 28/28] remove not needed condition --- homeassistant/components/mqtt/mixins.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index 274a19b9e2e940..c87e5ccba25c44 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -5,7 +5,7 @@ from collections.abc import Callable import json import logging -from typing import Any, Protocol +from typing import Any, Protocol, cast import voluptuous as vol @@ -508,7 +508,6 @@ async def cleanup_device_registry( entity_registry = er.async_get(hass) if ( device_id - and config_entry_id and not er.async_entries_for_device( entity_registry, device_id, include_disabled_entities=False ) @@ -517,7 +516,7 @@ async def cleanup_device_registry( and not notify.device_has_notify_services(hass, device_id) ): device_registry.async_update_device( - device_id, remove_config_entry_id=config_entry_id + device_id, remove_config_entry_id=cast(str, config_entry_id) )