From 0b57cfb004acf973a5a90b47c3e585afad77c410 Mon Sep 17 00:00:00 2001 From: emontnemery Date: Thu, 3 Jan 2019 15:51:18 +0100 Subject: [PATCH 1/3] Cleanup if discovered mqtt sensor can't be added --- homeassistant/components/sensor/mqtt.py | 15 ++++++++--- tests/components/sensor/test_mqtt.py | 35 ++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/sensor/mqtt.py b/homeassistant/components/sensor/mqtt.py index 49d090f7e1e3..ae4a0f2f1ad7 100644 --- a/homeassistant/components/sensor/mqtt.py +++ b/homeassistant/components/sensor/mqtt.py @@ -17,7 +17,8 @@ from homeassistant.components.mqtt import ( ATTR_DISCOVERY_HASH, CONF_AVAILABILITY_TOPIC, CONF_PAYLOAD_AVAILABLE, CONF_PAYLOAD_NOT_AVAILABLE, CONF_QOS, CONF_STATE_TOPIC, MqttAttributes, MqttAvailability, MqttDiscoveryUpdate, MqttEntityDeviceInfo, subscription) -from homeassistant.components.mqtt.discovery import MQTT_DISCOVERY_NEW +from homeassistant.components.mqtt.discovery import ( + ALREADY_DISCOVERED, MQTT_DISCOVERY_NEW) from homeassistant.components.sensor import DEVICE_CLASSES_SCHEMA from homeassistant.const import ( CONF_FORCE_UPDATE, CONF_NAME, CONF_VALUE_TEMPLATE, STATE_UNKNOWN, @@ -66,9 +67,15 @@ async def async_setup_entry(hass, config_entry, async_add_entities): """Set up MQTT sensors dynamically through MQTT discovery.""" async def async_discover_sensor(discovery_payload): """Discover and add a discovered MQTT sensor.""" - config = PLATFORM_SCHEMA(discovery_payload) - await _async_setup_entity(config, async_add_entities, - discovery_payload[ATTR_DISCOVERY_HASH]) + try: + discovery_hash = discovery_payload[ATTR_DISCOVERY_HASH] + config = PLATFORM_SCHEMA(discovery_payload) + await _async_setup_entity(config, async_add_entities, + discovery_hash) + except: # noqa: E722 + if discovery_hash: + del hass.data[ALREADY_DISCOVERED][discovery_hash] + raise async_dispatcher_connect(hass, MQTT_DISCOVERY_NEW.format(sensor.DOMAIN, 'mqtt'), diff --git a/tests/components/sensor/test_mqtt.py b/tests/components/sensor/test_mqtt.py index 79ba2c7a512c..739e81258c20 100644 --- a/tests/components/sensor/test_mqtt.py +++ b/tests/components/sensor/test_mqtt.py @@ -474,7 +474,7 @@ async def test_discovery_removal_sensor(hass, mqtt_mock, caplog): async def test_discovery_update_sensor(hass, mqtt_mock, caplog): - """Test removal of discovered sensor.""" + """Test update of discovered sensor.""" entry = MockConfigEntry(domain=mqtt.DOMAIN) await async_start(hass, 'homeassistant', {}, entry) data1 = ( @@ -506,6 +506,39 @@ async def test_discovery_update_sensor(hass, mqtt_mock, caplog): assert state is None +async def test_discovery_broken(hass, mqtt_mock, caplog): + """Test handling of bad discovery message.""" + entry = MockConfigEntry(domain=mqtt.DOMAIN) + await async_start(hass, 'homeassistant', {}, entry) + + data1 = ( + '{ "name": "Beer",' + ' "state_topic": "test_topic#" }' + ) + data2 = ( + '{ "name": "Milk",' + ' "state_topic": "test_topic" }' + ) + + async_fire_mqtt_message(hass, 'homeassistant/sensor/bla/config', + data1) + await hass.async_block_till_done() + + state = hass.states.get('sensor.beer') + assert state is None + + async_fire_mqtt_message(hass, 'homeassistant/sensor/bla/config', + data2) + await hass.async_block_till_done() + await hass.async_block_till_done() + + state = hass.states.get('sensor.milk') + assert state is not None + assert state.name == 'Milk' + state = hass.states.get('sensor.beer') + assert state is None + + async def test_entity_device_info_with_identifier(hass, mqtt_mock): """Test MQTT sensor device registry integration.""" entry = MockConfigEntry(domain=mqtt.DOMAIN) From 8701be095b967fd3283761031f1d3016e54862d0 Mon Sep 17 00:00:00 2001 From: emontnemery Date: Sat, 5 Jan 2019 14:01:58 +0100 Subject: [PATCH 2/3] No bare except --- homeassistant/components/sensor/mqtt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/sensor/mqtt.py b/homeassistant/components/sensor/mqtt.py index ae4a0f2f1ad7..25711ce39d65 100644 --- a/homeassistant/components/sensor/mqtt.py +++ b/homeassistant/components/sensor/mqtt.py @@ -72,7 +72,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): config = PLATFORM_SCHEMA(discovery_payload) await _async_setup_entity(config, async_add_entities, discovery_hash) - except: # noqa: E722 + except Exception: if discovery_hash: del hass.data[ALREADY_DISCOVERED][discovery_hash] raise From 08ac6da8a6f52934e1d8f2472cb49dcc2b8a7a50 Mon Sep 17 00:00:00 2001 From: emontnemery Date: Sat, 5 Jan 2019 14:07:00 +0100 Subject: [PATCH 3/3] Clear ALREADY_DISCOVERED list with helper --- homeassistant/components/sensor/mqtt.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/sensor/mqtt.py b/homeassistant/components/sensor/mqtt.py index 25711ce39d65..b78ebb048ad4 100644 --- a/homeassistant/components/sensor/mqtt.py +++ b/homeassistant/components/sensor/mqtt.py @@ -18,7 +18,7 @@ from homeassistant.components.mqtt import ( CONF_PAYLOAD_NOT_AVAILABLE, CONF_QOS, CONF_STATE_TOPIC, MqttAttributes, MqttAvailability, MqttDiscoveryUpdate, MqttEntityDeviceInfo, subscription) from homeassistant.components.mqtt.discovery import ( - ALREADY_DISCOVERED, MQTT_DISCOVERY_NEW) + MQTT_DISCOVERY_NEW, clear_discovery_hash) from homeassistant.components.sensor import DEVICE_CLASSES_SCHEMA from homeassistant.const import ( CONF_FORCE_UPDATE, CONF_NAME, CONF_VALUE_TEMPLATE, STATE_UNKNOWN, @@ -74,7 +74,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): discovery_hash) except Exception: if discovery_hash: - del hass.data[ALREADY_DISCOVERED][discovery_hash] + clear_discovery_hash(hass, discovery_hash) raise async_dispatcher_connect(hass,