diff --git a/homeassistant/components/tasmota/discovery.py b/homeassistant/components/tasmota/discovery.py index da9e809bd8b..8afac859b88 100644 --- a/homeassistant/components/tasmota/discovery.py +++ b/homeassistant/components/tasmota/discovery.py @@ -3,7 +3,9 @@ from __future__ import annotations from collections.abc import Awaitable, Callable import logging +from typing import TypedDict, cast +from hatasmota import const as tasmota_const from hatasmota.discovery import ( TasmotaDiscovery, get_device_config as tasmota_get_device_config, @@ -17,11 +19,16 @@ from hatasmota.entity import TasmotaEntityConfig from hatasmota.models import DiscoveryHashType, TasmotaDeviceConfig from hatasmota.mqtt import TasmotaMQTTClient from hatasmota.sensor import TasmotaBaseSensorConfig +from hatasmota.utils import get_topic_command, get_topic_stat from homeassistant.components import sensor from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant -from homeassistant.helpers import device_registry as dr, entity_registry as er +from homeassistant.helpers import ( + device_registry as dr, + entity_registry as er, + issue_registry as ir, +) from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.helpers.entity_registry import async_entries_for_device @@ -30,10 +37,13 @@ from .const import DOMAIN, PLATFORMS _LOGGER = logging.getLogger(__name__) ALREADY_DISCOVERED = "tasmota_discovered_components" +DISCOVERY_DATA = "tasmota_discovery_data" TASMOTA_DISCOVERY_ENTITY_NEW = "tasmota_discovery_entity_new_{}" TASMOTA_DISCOVERY_ENTITY_UPDATED = "tasmota_discovery_entity_updated_{}_{}_{}_{}" TASMOTA_DISCOVERY_INSTANCE = "tasmota_discovery_instance" +MQTT_TOPIC_URL = "https://tasmota.github.io/docs/Home-Assistant/#tasmota-integration" + SetupDeviceCallback = Callable[[TasmotaDeviceConfig, str], Awaitable[None]] @@ -52,7 +62,64 @@ def set_discovery_hash(hass: HomeAssistant, discovery_hash: DiscoveryHashType) - hass.data[ALREADY_DISCOVERED][discovery_hash] = {} -async def async_start( +def warn_if_topic_duplicated( + hass: HomeAssistant, + command_topic: str, + own_mac: str | None, + own_device_config: TasmotaDeviceConfig, +) -> bool: + """Log and create repairs issue if several devices share the same topic.""" + duplicated = False + offenders = [] + for other_mac, other_config in hass.data[DISCOVERY_DATA].items(): + if own_mac and other_mac == own_mac: + continue + if command_topic == get_topic_command(other_config): + offenders.append((other_mac, tasmota_get_device_config(other_config))) + issue_id = f"topic_duplicated_{command_topic}" + if offenders: + if own_mac: + offenders.append((own_mac, own_device_config)) + offender_strings = [ + f"'{cfg[tasmota_const.CONF_NAME]}' ({cfg[tasmota_const.CONF_IP]})" + for _, cfg in offenders + ] + _LOGGER.warning( + "Multiple Tasmota devices are sharing the same topic '%s'. Offending devices: %s", + command_topic, + ", ".join(offender_strings), + ) + ir.async_create_issue( + hass, + DOMAIN, + issue_id, + data={ + "key": "topic_duplicated", + "mac": " ".join([mac for mac, _ in offenders]), + "topic": command_topic, + }, + is_fixable=False, + learn_more_url=MQTT_TOPIC_URL, + severity=ir.IssueSeverity.ERROR, + translation_key="topic_duplicated", + translation_placeholders={ + "topic": command_topic, + "offenders": "\n\n* " + "\n\n* ".join(offender_strings), + }, + ) + duplicated = True + return duplicated + + +class DuplicatedTopicIssueData(TypedDict): + """Typed result dict.""" + + key: str + mac: str + topic: str + + +async def async_start( # noqa: C901 hass: HomeAssistant, discovery_topic: str, config_entry: ConfigEntry, @@ -121,9 +188,72 @@ async def async_start( tasmota_device_config = tasmota_get_device_config(payload) await setup_device(tasmota_device_config, mac) + hass.data[DISCOVERY_DATA][mac] = payload + + add_entities = True + + command_topic = get_topic_command(payload) if payload else None + state_topic = get_topic_stat(payload) if payload else None + + # Create or clear issue if topic is missing prefix + issue_id = f"topic_no_prefix_{mac}" + if payload and command_topic == state_topic: + _LOGGER.warning( + "Tasmota device '%s' with IP %s doesn't set %%prefix%% in its topic", + tasmota_device_config[tasmota_const.CONF_NAME], + tasmota_device_config[tasmota_const.CONF_IP], + ) + ir.async_create_issue( + hass, + DOMAIN, + issue_id, + data={"key": "topic_no_prefix"}, + is_fixable=False, + learn_more_url=MQTT_TOPIC_URL, + severity=ir.IssueSeverity.ERROR, + translation_key="topic_no_prefix", + translation_placeholders={ + "name": tasmota_device_config[tasmota_const.CONF_NAME], + "ip": tasmota_device_config[tasmota_const.CONF_IP], + }, + ) + add_entities = False + else: + ir.async_delete_issue(hass, DOMAIN, issue_id) + + # Clear previous issues caused by duplicated topic + issue_reg = ir.async_get(hass) + tasmota_issues = [ + issue for key, issue in issue_reg.issues.items() if key[0] == DOMAIN + ] + for issue in tasmota_issues: + if issue.data and issue.data["key"] == "topic_duplicated": + issue_data: DuplicatedTopicIssueData = cast( + DuplicatedTopicIssueData, issue.data + ) + macs = issue_data["mac"].split() + if mac not in macs: + continue + if payload and command_topic == issue_data["topic"]: + continue + if len(macs) > 2: + # This device is no longer duplicated, update the issue + warn_if_topic_duplicated(hass, issue_data["topic"], None, {}) + continue + ir.async_delete_issue(hass, DOMAIN, issue.issue_id) + if not payload: return + # Warn and add issues if there are duplicated topics + if warn_if_topic_duplicated(hass, command_topic, mac, tasmota_device_config): + add_entities = False + + if not add_entities: + # Add the device entry so the user can identify the device, but do not add + # entities or triggers + return + tasmota_triggers = tasmota_get_triggers(payload) for trigger_config in tasmota_triggers: discovery_hash: DiscoveryHashType = ( @@ -194,6 +324,7 @@ async def async_start( entity_registry.async_remove(entity_id) hass.data[ALREADY_DISCOVERED] = {} + hass.data[DISCOVERY_DATA] = {} tasmota_discovery = TasmotaDiscovery(discovery_topic, tasmota_mqtt) await tasmota_discovery.start_discovery( diff --git a/homeassistant/components/tasmota/manifest.json b/homeassistant/components/tasmota/manifest.json index d6ba4cf90cc..6e3e69f59fe 100644 --- a/homeassistant/components/tasmota/manifest.json +++ b/homeassistant/components/tasmota/manifest.json @@ -3,7 +3,7 @@ "name": "Tasmota", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/tasmota", - "requirements": ["hatasmota==0.6.0"], + "requirements": ["hatasmota==0.6.1"], "dependencies": ["mqtt"], "mqtt": ["tasmota/discovery/#"], "codeowners": ["@emontnemery"], diff --git a/homeassistant/components/tasmota/strings.json b/homeassistant/components/tasmota/strings.json index 2a23912b80c..22af3304297 100644 --- a/homeassistant/components/tasmota/strings.json +++ b/homeassistant/components/tasmota/strings.json @@ -16,5 +16,15 @@ "error": { "invalid_discovery_topic": "Invalid discovery topic prefix." } + }, + "issues": { + "topic_duplicated": { + "title": "Several Tasmota devices are sharing the same topic", + "description": "Several Tasmota devices are sharing the topic {topic}.\n\n Tasmota devices with this problem: {offenders}." + }, + "topic_no_prefix": { + "title": "Tasmota device {name} has an invalid MQTT topic", + "description": "Tasmota device {name} with IP {ip} does not include `%prefix%` in its fulltopic.\n\nEntities for this devices are disabled until the configuration has been corrected." + } } } diff --git a/homeassistant/components/tasmota/translations/en.json b/homeassistant/components/tasmota/translations/en.json index 3e8b0b43bce..bb4d174a04d 100644 --- a/homeassistant/components/tasmota/translations/en.json +++ b/homeassistant/components/tasmota/translations/en.json @@ -16,5 +16,15 @@ "description": "Do you want to set up Tasmota?" } } + }, + "issues": { + "topic_duplicated": { + "description": "Several Tasmota devices are sharing the topic {topic}.\n\n Tasmota devices with this problem: {offenders}.", + "title": "Several Tasmota devices are sharing the same topic" + }, + "topic_no_prefix": { + "description": "Tasmota device {name} with IP {ip} does not include `%prefix%` in its fulltopic.\n\nEntities for this devices are disabled until the configuration has been corrected.", + "title": "Tasmota device {name} has an invalid MQTT topic" + } } } \ No newline at end of file diff --git a/requirements_all.txt b/requirements_all.txt index daaa91a5704..8d0bd4ff995 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -830,7 +830,7 @@ hass-nabucasa==0.55.0 hass_splunk==0.1.1 # homeassistant.components.tasmota -hatasmota==0.6.0 +hatasmota==0.6.1 # homeassistant.components.jewish_calendar hdate==0.10.4 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 2bae9a1d8ca..bdba84bb811 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -613,7 +613,7 @@ hangups==0.4.18 hass-nabucasa==0.55.0 # homeassistant.components.tasmota -hatasmota==0.6.0 +hatasmota==0.6.1 # homeassistant.components.jewish_calendar hdate==0.10.4 diff --git a/tests/components/tasmota/test_discovery.py b/tests/components/tasmota/test_discovery.py index a97b877d819..55bb385817d 100644 --- a/tests/components/tasmota/test_discovery.py +++ b/tests/components/tasmota/test_discovery.py @@ -5,7 +5,11 @@ from unittest.mock import ANY, patch from homeassistant.components.tasmota.const import DEFAULT_PREFIX from homeassistant.components.tasmota.discovery import ALREADY_DISCOVERED -from homeassistant.helpers import device_registry as dr +from homeassistant.helpers import ( + device_registry as dr, + entity_registry as er, + issue_registry as ir, +) from homeassistant.setup import async_setup_component from .conftest import setup_tasmota_helper @@ -495,3 +499,179 @@ async def test_entity_duplicate_removal(hass, mqtt_mock, caplog, setup_tasmota): async_fire_mqtt_message(hass, f"{DEFAULT_PREFIX}/{mac}/config", json.dumps(config)) await hass.async_block_till_done() assert "Removing entity: switch" not in caplog.text + + +async def test_same_topic( + hass, mqtt_mock, caplog, device_reg, entity_reg, setup_tasmota +): + """Test detecting devices with same topic.""" + configs = [ + copy.deepcopy(DEFAULT_CONFIG), + copy.deepcopy(DEFAULT_CONFIG), + copy.deepcopy(DEFAULT_CONFIG), + ] + configs[0]["rl"][0] = 1 + configs[1]["rl"][0] = 1 + configs[2]["rl"][0] = 1 + configs[0]["mac"] = "000000000001" + configs[1]["mac"] = "000000000002" + configs[2]["mac"] = "000000000003" + + for config in configs[0:2]: + async_fire_mqtt_message( + hass, + f"{DEFAULT_PREFIX}/{config['mac']}/config", + json.dumps(config), + ) + await hass.async_block_till_done() + + # Verify device registry entries are created for both devices + for config in configs[0:2]: + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, config["mac"])} + ) + assert device_entry is not None + assert device_entry.configuration_url == f"http://{config['ip']}/" + assert device_entry.manufacturer == "Tasmota" + assert device_entry.model == config["md"] + assert device_entry.name == config["dn"] + assert device_entry.sw_version == config["sw"] + + # Verify entities are created only for the first device + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, configs[0]["mac"])} + ) + assert len(er.async_entries_for_device(entity_reg, device_entry.id, True)) == 1 + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, configs[1]["mac"])} + ) + assert len(er.async_entries_for_device(entity_reg, device_entry.id, True)) == 0 + + # Verify a repairs issue was created + issue_id = "topic_duplicated_tasmota_49A3BC/cmnd/" + issue_registry = ir.async_get(hass) + issue = issue_registry.async_get_issue("tasmota", issue_id) + assert issue.data["mac"] == " ".join(config["mac"] for config in configs[0:2]) + + # Discover a 3rd device with same topic + async_fire_mqtt_message( + hass, + f"{DEFAULT_PREFIX}/{configs[2]['mac']}/config", + json.dumps(configs[2]), + ) + await hass.async_block_till_done() + + # Verify device registry entries was created + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, configs[2]["mac"])} + ) + assert device_entry is not None + assert device_entry.configuration_url == f"http://{configs[2]['ip']}/" + assert device_entry.manufacturer == "Tasmota" + assert device_entry.model == configs[2]["md"] + assert device_entry.name == configs[2]["dn"] + assert device_entry.sw_version == configs[2]["sw"] + + # Verify no entities were created + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, configs[2]["mac"])} + ) + assert len(er.async_entries_for_device(entity_reg, device_entry.id, True)) == 0 + + # Verify the repairs issue has been updated + issue = issue_registry.async_get_issue("tasmota", issue_id) + assert issue.data["mac"] == " ".join(config["mac"] for config in configs[0:3]) + + # Rediscover 3rd device with fixed config + configs[2]["t"] = "unique_topic_2" + async_fire_mqtt_message( + hass, + f"{DEFAULT_PREFIX}/{configs[2]['mac']}/config", + json.dumps(configs[2]), + ) + await hass.async_block_till_done() + + # Verify entities are created also for the third device + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, configs[2]["mac"])} + ) + assert len(er.async_entries_for_device(entity_reg, device_entry.id, True)) == 1 + + # Verify the repairs issue has been updated + issue = issue_registry.async_get_issue("tasmota", issue_id) + assert issue.data["mac"] == " ".join(config["mac"] for config in configs[0:2]) + + # Rediscover 2nd device with fixed config + configs[1]["t"] = "unique_topic_1" + async_fire_mqtt_message( + hass, + f"{DEFAULT_PREFIX}/{configs[1]['mac']}/config", + json.dumps(configs[1]), + ) + await hass.async_block_till_done() + + # Verify entities are created also for the second device + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, configs[1]["mac"])} + ) + assert len(er.async_entries_for_device(entity_reg, device_entry.id, True)) == 1 + + # Verify the repairs issue has been removed + assert issue_registry.async_get_issue("tasmota", issue_id) is None + + +async def test_topic_no_prefix( + hass, mqtt_mock, caplog, device_reg, entity_reg, setup_tasmota +): + """Test detecting devices with same topic.""" + config = copy.deepcopy(DEFAULT_CONFIG) + config["rl"][0] = 1 + config["ft"] = "%topic%/blah/" + + async_fire_mqtt_message( + hass, + f"{DEFAULT_PREFIX}/{config['mac']}/config", + json.dumps(config), + ) + await hass.async_block_till_done() + + # Verify device registry entry is created + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, config["mac"])} + ) + assert device_entry is not None + assert device_entry.configuration_url == f"http://{config['ip']}/" + assert device_entry.manufacturer == "Tasmota" + assert device_entry.model == config["md"] + assert device_entry.name == config["dn"] + assert device_entry.sw_version == config["sw"] + + # Verify entities are not created + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, config["mac"])} + ) + assert len(er.async_entries_for_device(entity_reg, device_entry.id, True)) == 0 + + # Verify a repairs issue was created + issue_id = "topic_no_prefix_00000049A3BC" + issue_registry = ir.async_get(hass) + assert ("tasmota", issue_id) in issue_registry.issues + + # Rediscover device with fixed config + config["ft"] = "%topic%/%prefix%/" + async_fire_mqtt_message( + hass, + f"{DEFAULT_PREFIX}/{config['mac']}/config", + json.dumps(config), + ) + await hass.async_block_till_done() + + # Verify entities are created + device_entry = device_reg.async_get_device( + set(), {(dr.CONNECTION_NETWORK_MAC, config["mac"])} + ) + assert len(er.async_entries_for_device(entity_reg, device_entry.id, True)) == 1 + + # Verify the repairs issue has been removed + issue_registry = ir.async_get(hass) + assert ("tasmota", issue_id) not in issue_registry.issues