diff --git a/homeassistant/components/mqtt/mixins.py b/homeassistant/components/mqtt/mixins.py index 51d7ca401ca..e90301875e4 100644 --- a/homeassistant/components/mqtt/mixins.py +++ b/homeassistant/components/mqtt/mixins.py @@ -744,8 +744,12 @@ class MqttDiscoveryDeviceUpdate(ABC): self.log_name, discovery_hash, ) - await self.async_update(discovery_payload) - if not discovery_payload: + try: + await self.async_update(discovery_payload) + finally: + send_discovery_done(self.hass, self._discovery_data) + self._discovery_data[ATTR_DISCOVERY_PAYLOAD] = discovery_payload + elif not discovery_payload: # Unregister and clean up the current discovery instance stop_discovery_updates( self.hass, self._discovery_data, self._remove_discovery_updated @@ -869,15 +873,19 @@ class MqttDiscoveryUpdate(Entity): _LOGGER.info("Removing component: %s", self.entity_id) self._cleanup_discovery_on_remove() await _async_remove_state_and_registry_entry(self) + send_discovery_done(self.hass, self._discovery_data) elif self._discovery_update: if old_payload != self._discovery_data[ATTR_DISCOVERY_PAYLOAD]: # Non-empty, changed payload: Notify component _LOGGER.info("Updating component: %s", self.entity_id) - await self._discovery_update(payload) + try: + await self._discovery_update(payload) + finally: + send_discovery_done(self.hass, self._discovery_data) else: # Non-empty, unchanged payload: Ignore to avoid changing states _LOGGER.debug("Ignoring unchanged update for: %s", self.entity_id) - send_discovery_done(self.hass, self._discovery_data) + send_discovery_done(self.hass, self._discovery_data) if discovery_hash: assert self._discovery_data is not None diff --git a/tests/components/mqtt/test_discovery.py b/tests/components/mqtt/test_discovery.py index 89a56903c3b..251c0af24a6 100644 --- a/tests/components/mqtt/test_discovery.py +++ b/tests/components/mqtt/test_discovery.py @@ -6,6 +6,7 @@ import re from unittest.mock import AsyncMock, call, patch import pytest +from voluptuous import MultipleInvalid from homeassistant import config_entries from homeassistant.components import mqtt @@ -1494,7 +1495,7 @@ async def test_clean_up_registry_monitoring( async def test_unique_id_collission_has_priority( hass, mqtt_mock_entry_no_yaml_config, entity_reg ): - """Test tehe unique_id collision detection has priority over registry disabled items.""" + """Test the unique_id collision detection has priority over registry disabled items.""" await mqtt_mock_entry_no_yaml_config() config = { "name": "sbfspot_12345", @@ -1534,3 +1535,57 @@ async def test_unique_id_collission_has_priority( assert entity_reg.async_get("sensor.sbfspot_12345_1") is not None # Verify the second entity is not created because it is not unique assert entity_reg.async_get("sensor.sbfspot_12345_2") is None + + +@pytest.mark.xfail(raises=MultipleInvalid) +@patch("homeassistant.components.mqtt.PLATFORMS", [Platform.SENSOR]) +async def test_update_with_bad_config_not_breaks_discovery( + hass: ha.HomeAssistant, mqtt_mock_entry_no_yaml_config, entity_reg +) -> None: + """Test a bad update does not break discovery.""" + await mqtt_mock_entry_no_yaml_config() + # discover a sensor + config1 = { + "name": "sbfspot_12345", + "state_topic": "homeassistant_test/sensor/sbfspot_0/state", + } + async_fire_mqtt_message( + hass, + "homeassistant/sensor/sbfspot_0/config", + json.dumps(config1), + ) + await hass.async_block_till_done() + assert hass.states.get("sensor.sbfspot_12345") is not None + # update with a breaking config + config2 = { + "name": "sbfspot_12345", + "availability": 1, + "state_topic": "homeassistant_test/sensor/sbfspot_0/state", + } + async_fire_mqtt_message( + hass, + "homeassistant/sensor/sbfspot_0/config", + json.dumps(config2), + ) + await hass.async_block_till_done() + # update the state topic + config3 = { + "name": "sbfspot_12345", + "state_topic": "homeassistant_test/sensor/sbfspot_0/new_state_topic", + } + async_fire_mqtt_message( + hass, + "homeassistant/sensor/sbfspot_0/config", + json.dumps(config3), + ) + await hass.async_block_till_done() + + # Send an update for the state + async_fire_mqtt_message( + hass, + "homeassistant_test/sensor/sbfspot_0/new_state_topic", + "new_value", + ) + await hass.async_block_till_done() + + assert hass.states.get("sensor.sbfspot_12345").state == "new_value" diff --git a/tests/components/mqtt/test_tag.py b/tests/components/mqtt/test_tag.py index ed33ed0dcdf..9276f9658b6 100644 --- a/tests/components/mqtt/test_tag.py +++ b/tests/components/mqtt/test_tag.py @@ -4,10 +4,12 @@ import json from unittest.mock import ANY, patch import pytest +from voluptuous import MultipleInvalid from homeassistant.components.device_automation import DeviceAutomationType from homeassistant.components.mqtt.const import DOMAIN as MQTT_DOMAIN from homeassistant.const import Platform +from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr from homeassistant.setup import async_setup_component @@ -805,6 +807,50 @@ async def test_cleanup_device_with_entity2( assert device_entry is None +@pytest.mark.xfail(raises=MultipleInvalid) +async def test_update_with_bad_config_not_breaks_discovery( + hass: HomeAssistant, + mqtt_mock_entry_no_yaml_config, + tag_mock, +) -> None: + """Test a bad update does not break discovery.""" + await mqtt_mock_entry_no_yaml_config() + config1 = { + "topic": "test-topic", + "device": {"identifiers": ["helloworld"]}, + } + config2 = { + "topic": "test-topic", + "device": {"bad_key": "some bad value"}, + } + + config3 = { + "topic": "test-topic-update", + "device": {"identifiers": ["helloworld"]}, + } + + data1 = json.dumps(config1) + data2 = json.dumps(config2) + data3 = json.dumps(config3) + + async_fire_mqtt_message(hass, "homeassistant/tag/bla1/config", data1) + await hass.async_block_till_done() + + # Update with bad identifier + async_fire_mqtt_message(hass, "homeassistant/tag/bla1/config", data2) + await hass.async_block_till_done() + + # Topic update + async_fire_mqtt_message(hass, "homeassistant/tag/bla1/config", data3) + await hass.async_block_till_done() + + # Fake tag scan. + async_fire_mqtt_message(hass, "test-topic-update", "12345") + + await hass.async_block_till_done() + tag_mock.assert_called_once_with(ANY, "12345", ANY) + + async def test_unload_entry(hass, device_reg, mqtt_mock, tag_mock, tmp_path) -> None: """Test unloading the MQTT entry."""