Add JSON attribute topic to MQTT cover (#20190)

* Add JSON attribute topic to MQTT cover

* Lint

homeassistant/components/mqtt/cover.py

@@ -21,7 +21,7 @@ from homeassistant.const import (
     STATE_CLOSED, STATE_UNKNOWN, CONF_DEVICE)
 from homeassistant.components.mqtt import (
     ATTR_DISCOVERY_HASH, CONF_COMMAND_TOPIC, CONF_QOS, CONF_RETAIN,
-    CONF_STATE_TOPIC, MqttAvailability, MqttDiscoveryUpdate,
+    CONF_STATE_TOPIC, MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
     MqttEntityDeviceInfo, subscription)
 from homeassistant.components.mqtt.discovery import (
     MQTT_DISCOVERY_NEW, clear_discovery_hash)
@@ -123,7 +123,8 @@ PLATFORM_SCHEMA = vol.All(mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend({
                  default=DEFAULT_TILT_INVERT_STATE): cv.boolean,
     vol.Optional(CONF_UNIQUE_ID): cv.string,
     vol.Optional(CONF_DEVICE): mqtt.MQTT_ENTITY_DEVICE_INFO_SCHEMA,
-}).extend(mqtt.MQTT_AVAILABILITY_SCHEMA.schema), validate_options)
+}).extend(mqtt.MQTT_AVAILABILITY_SCHEMA.schema).extend(
+    mqtt.MQTT_JSON_ATTRS_SCHEMA.schema), validate_options)
 
 
 async def async_setup_platform(hass: HomeAssistantType, config: ConfigType,
@@ -156,8 +157,8 @@ async def _async_setup_entity(config, async_add_entities, discovery_hash=None):
     async_add_entities([MqttCover(config, discovery_hash)])
 
 
-class MqttCover(MqttAvailability, MqttDiscoveryUpdate, MqttEntityDeviceInfo,
+class MqttCover(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
-                CoverDevice):
+                MqttEntityDeviceInfo, CoverDevice):
     """Representation of a cover that can be controlled using MQTT."""
 
     def __init__(self, config, discovery_hash):
@@ -176,6 +177,7 @@ class MqttCover(MqttAvailability, MqttDiscoveryUpdate, MqttEntityDeviceInfo,
 
         device_config = config.get(CONF_DEVICE)
 
+        MqttAttributes.__init__(self, config)
         MqttAvailability.__init__(self, config)
         MqttDiscoveryUpdate.__init__(self, discovery_hash,
                                      self.discovery_update)
@@ -190,6 +192,7 @@ class MqttCover(MqttAvailability, MqttDiscoveryUpdate, MqttEntityDeviceInfo,
         """Handle updated discovery message."""
         config = PLATFORM_SCHEMA(discovery_payload)
         self._setup_from_config(config)
+        await self.attributes_discovery_update(config)
         await self.availability_discovery_update(config)
         await self._subscribe_topics()
         self.async_schedule_update_ha_state()
@@ -290,6 +293,7 @@ class MqttCover(MqttAvailability, MqttDiscoveryUpdate, MqttEntityDeviceInfo,
         """Unsubscribe when removed."""
         self._sub_state = await subscription.async_unsubscribe_topics(
             self.hass, self._sub_state)
+        await MqttAttributes.async_will_remove_from_hass(self)
         await MqttAvailability.async_will_remove_from_hass(self)
 
     @property

tests/components/mqtt/test_binary_sensor.py

@@ -1,7 +1,7 @@
 """The tests for the  MQTT binary sensor platform."""
 import json
 import unittest
-from unittest.mock import ANY, Mock, patch
+from unittest.mock import ANY, Mock
 from datetime import timedelta
 
 import homeassistant.core as ha
@@ -283,66 +283,105 @@ class TestSensorMQTT(unittest.TestCase):
         assert STATE_OFF == state.state
         assert 3 == len(events)
 
-    def test_setting_sensor_attribute_via_mqtt_json_message(self):
-        """Test the setting of attribute via MQTT with JSON payload."""
-        mock_component(self.hass, 'mqtt')
-        assert setup_component(self.hass, binary_sensor.DOMAIN, {
-            binary_sensor.DOMAIN: {
-                'platform': 'mqtt',
-                'name': 'test',
-                'state_topic': 'test-topic',
-                'json_attributes_topic': 'attr-topic'
-            }
-        })
 
-        fire_mqtt_message(self.hass, 'attr-topic', '{ "val": "100" }')
+async def test_setting_attribute_via_mqtt_json_message(hass, mqtt_mock):
-        self.hass.block_till_done()
+    """Test the setting of attribute via MQTT with JSON payload."""
-        state = self.hass.states.get('binary_sensor.test')
+    assert await async_setup_component(hass, binary_sensor.DOMAIN, {
+        binary_sensor.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
 
-        assert '100' == \
+    async_fire_mqtt_message(hass, 'attr-topic', '{ "val": "100" }')
-            state.attributes.get('val')
+    await hass.async_block_till_done()
+    state = hass.states.get('binary_sensor.test')
 
-    @patch('homeassistant.components.mqtt._LOGGER')
+    assert '100' == state.attributes.get('val')
-    def test_update_with_json_attrs_not_dict(self, mock_logger):
-        """Test attributes get extracted from a JSON result."""
-        mock_component(self.hass, 'mqtt')
-        assert setup_component(self.hass, binary_sensor.DOMAIN, {
-            binary_sensor.DOMAIN: {
-                'platform': 'mqtt',
-                'name': 'test',
-                'state_topic': 'test-topic',
-                'json_attributes_topic': 'attr-topic'
-            }
-        })
 
-        fire_mqtt_message(self.hass, 'attr-topic', '[ "list", "of", "things"]')
-        self.hass.block_till_done()
-        state = self.hass.states.get('binary_sensor.test')
 
-        assert state.attributes.get('val') is None
+async def test_update_with_json_attrs_not_dict(hass, mqtt_mock, caplog):
-        mock_logger.warning.assert_called_with(
+    """Test attributes get extracted from a JSON result."""
-            'JSON result was not a dictionary')
+    assert await async_setup_component(hass, binary_sensor.DOMAIN, {
+        binary_sensor.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
 
-    @patch('homeassistant.components.mqtt._LOGGER')
+    async_fire_mqtt_message(hass, 'attr-topic', '[ "list", "of", "things"]')
-    def test_update_with_json_attrs_bad_JSON(self, mock_logger):
+    await hass.async_block_till_done()
-        """Test attributes get extracted from a JSON result."""
+    state = hass.states.get('binary_sensor.test')
-        mock_component(self.hass, 'mqtt')
-        assert setup_component(self.hass, binary_sensor.DOMAIN, {
-            binary_sensor.DOMAIN: {
-                'platform': 'mqtt',
-                'name': 'test',
-                'state_topic': 'test-topic',
-                'json_attributes_topic': 'attr-topic'
-            }
-        })
 
-        fire_mqtt_message(self.hass, 'attr-topic', 'This is not JSON')
+    assert state.attributes.get('val') is None
-        self.hass.block_till_done()
+    assert 'JSON result was not a dictionary' in caplog.text
 
-        state = self.hass.states.get('binary_sensor.test')
+
-        assert state.attributes.get('val') is None
+async def test_update_with_json_attrs_bad_JSON(hass, mqtt_mock, caplog):
-        mock_logger.warning.assert_called_with(
+    """Test attributes get extracted from a JSON result."""
-            'Erroneous JSON: %s', 'This is not JSON')
+    assert await async_setup_component(hass, binary_sensor.DOMAIN, {
+        binary_sensor.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
+
+    async_fire_mqtt_message(hass, 'attr-topic', 'This is not JSON')
+    await hass.async_block_till_done()
+
+    state = hass.states.get('binary_sensor.test')
+    assert state.attributes.get('val') is None
+    assert 'Erroneous JSON: This is not JSON' in caplog.text
+
+
+async def test_discovery_update_attr(hass, mqtt_mock, caplog):
+    """Test update of discovered MQTTAttributes."""
+    entry = MockConfigEntry(domain=mqtt.DOMAIN)
+    await async_start(hass, 'homeassistant', {}, entry)
+    data1 = (
+        '{ "name": "Beer",'
+        '  "command_topic": "test_topic",'
+        '  "json_attributes_topic": "attr-topic1" }'
+    )
+    data2 = (
+        '{ "name": "Beer",'
+        '  "command_topic": "test_topic",'
+        '  "json_attributes_topic": "attr-topic2" }'
+    )
+    async_fire_mqtt_message(hass, 'homeassistant/binary_sensor/bla/config',
+                            data1)
+    await hass.async_block_till_done()
+    async_fire_mqtt_message(hass, 'attr-topic1', '{ "val": "100" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('binary_sensor.beer')
+    assert '100' == state.attributes.get('val')
+
+    # Change json_attributes_topic
+    async_fire_mqtt_message(hass, 'homeassistant/binary_sensor/bla/config',
+                            data2)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+
+    # Verify we are no longer subscribing to the old topic
+    async_fire_mqtt_message(hass, 'attr-topic1', '{ "val": "50" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('binary_sensor.beer')
+    assert '100' == state.attributes.get('val')
+
+    # Verify we are subscribing to the new topic
+    async_fire_mqtt_message(hass, 'attr-topic2', '{ "val": "75" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('binary_sensor.beer')
+    assert '75' == state.attributes.get('val')
 
 
 async def test_unique_id(hass):

tests/components/mqtt/test_cover.py

@@ -16,8 +16,8 @@ from homeassistant.const import (
 from homeassistant.setup import setup_component, async_setup_component
 
 from tests.common import (
-    get_test_home_assistant, mock_mqtt_component, async_fire_mqtt_message,
+    MockConfigEntry, async_fire_mqtt_message, async_mock_mqtt_component,
-    fire_mqtt_message, MockConfigEntry, async_mock_mqtt_component,
+    fire_mqtt_message, get_test_home_assistant, mock_mqtt_component,
     mock_registry)
 
 
@@ -1044,6 +1044,106 @@ class TestCoverMQTT(unittest.TestCase):
         assert STATE_UNAVAILABLE == state.state
 
 
+async def test_setting_attribute_via_mqtt_json_message(hass, mqtt_mock):
+    """Test the setting of attribute via MQTT with JSON payload."""
+    assert await async_setup_component(hass, cover.DOMAIN, {
+        cover.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
+
+    async_fire_mqtt_message(hass, 'attr-topic', '{ "val": "100" }')
+    await hass.async_block_till_done()
+    state = hass.states.get('cover.test')
+
+    assert '100' == state.attributes.get('val')
+
+
+async def test_update_with_json_attrs_not_dict(hass, mqtt_mock, caplog):
+    """Test attributes get extracted from a JSON result."""
+    assert await async_setup_component(hass, cover.DOMAIN, {
+        cover.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
+
+    async_fire_mqtt_message(hass, 'attr-topic', '[ "list", "of", "things"]')
+    await hass.async_block_till_done()
+    state = hass.states.get('cover.test')
+
+    assert state.attributes.get('val') is None
+    assert 'JSON result was not a dictionary' in caplog.text
+
+
+async def test_update_with_json_attrs_bad_JSON(hass, mqtt_mock, caplog):
+    """Test attributes get extracted from a JSON result."""
+    assert await async_setup_component(hass, cover.DOMAIN, {
+        cover.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
+
+    async_fire_mqtt_message(hass, 'attr-topic', 'This is not JSON')
+    await hass.async_block_till_done()
+
+    state = hass.states.get('cover.test')
+    assert state.attributes.get('val') is None
+    assert 'Erroneous JSON: This is not JSON' in caplog.text
+
+
+async def test_discovery_update_attr(hass, mqtt_mock, caplog):
+    """Test update of discovered MQTTAttributes."""
+    entry = MockConfigEntry(domain=mqtt.DOMAIN)
+    await async_start(hass, 'homeassistant', {}, entry)
+    data1 = (
+        '{ "name": "Beer",'
+        '  "command_topic": "test_topic",'
+        '  "json_attributes_topic": "attr-topic1" }'
+    )
+    data2 = (
+        '{ "name": "Beer",'
+        '  "command_topic": "test_topic",'
+        '  "json_attributes_topic": "attr-topic2" }'
+    )
+    async_fire_mqtt_message(hass, 'homeassistant/cover/bla/config',
+                            data1)
+    await hass.async_block_till_done()
+    async_fire_mqtt_message(hass, 'attr-topic1', '{ "val": "100" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('cover.beer')
+    assert '100' == state.attributes.get('val')
+
+    # Change json_attributes_topic
+    async_fire_mqtt_message(hass, 'homeassistant/cover/bla/config',
+                            data2)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+
+    # Verify we are no longer subscribing to the old topic
+    async_fire_mqtt_message(hass, 'attr-topic1', '{ "val": "50" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('cover.beer')
+    assert '100' == state.attributes.get('val')
+
+    # Verify we are subscribing to the new topic
+    async_fire_mqtt_message(hass, 'attr-topic2', '{ "val": "75" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('cover.beer')
+    assert '75' == state.attributes.get('val')
+
+
 async def test_discovery_removal_cover(hass, mqtt_mock, caplog):
     """Test removal of discovered cover."""
     entry = MockConfigEntry(domain=mqtt.DOMAIN)

tests/components/mqtt/test_sensor.py

@@ -333,67 +333,6 @@ class TestSensorMQTT(unittest.TestCase):
             state.attributes.get('val')
         assert '100' == state.state
 
-    def test_setting_sensor_attribute_via_mqtt_json_topic(self):
-        """Test the setting of attribute via MQTT with JSON payload."""
-        mock_component(self.hass, 'mqtt')
-        assert setup_component(self.hass, sensor.DOMAIN, {
-            sensor.DOMAIN: {
-                'platform': 'mqtt',
-                'name': 'test',
-                'state_topic': 'test-topic',
-                'json_attributes_topic': 'attr-topic'
-            }
-        })
-
-        fire_mqtt_message(self.hass, 'attr-topic', '{ "val": "100" }')
-        self.hass.block_till_done()
-        state = self.hass.states.get('sensor.test')
-
-        assert '100' == \
-            state.attributes.get('val')
-
-    @patch('homeassistant.components.mqtt._LOGGER')
-    def test_update_with_json_attrs_topic_not_dict(self, mock_logger):
-        """Test attributes get extracted from a JSON result."""
-        mock_component(self.hass, 'mqtt')
-        assert setup_component(self.hass, sensor.DOMAIN, {
-            sensor.DOMAIN: {
-                'platform': 'mqtt',
-                'name': 'test',
-                'state_topic': 'test-topic',
-                'json_attributes_topic': 'attr-topic'
-            }
-        })
-
-        fire_mqtt_message(self.hass, 'attr-topic', '[ "list", "of", "things"]')
-        self.hass.block_till_done()
-        state = self.hass.states.get('sensor.test')
-
-        assert state.attributes.get('val') is None
-        mock_logger.warning.assert_called_with(
-            'JSON result was not a dictionary')
-
-    @patch('homeassistant.components.mqtt._LOGGER')
-    def test_update_with_json_attrs_topic_bad_JSON(self, mock_logger):
-        """Test attributes get extracted from a JSON result."""
-        mock_component(self.hass, 'mqtt')
-        assert setup_component(self.hass, sensor.DOMAIN, {
-            sensor.DOMAIN: {
-                'platform': 'mqtt',
-                'name': 'test',
-                'state_topic': 'test-topic',
-                'json_attributes_topic': 'attr-topic'
-            }
-        })
-
-        fire_mqtt_message(self.hass, 'attr-topic', 'This is not JSON')
-        self.hass.block_till_done()
-
-        state = self.hass.states.get('sensor.test')
-        assert state.attributes.get('val') is None
-        mock_logger.warning.assert_called_with(
-            'Erroneous JSON: %s', 'This is not JSON')
-
     def test_invalid_device_class(self):
         """Test device_class option with invalid value."""
         with assert_setup_component(0):
@@ -428,6 +367,106 @@ class TestSensorMQTT(unittest.TestCase):
         assert 'device_class' not in state.attributes
 
 
+async def test_setting_attribute_via_mqtt_json_message(hass, mqtt_mock):
+    """Test the setting of attribute via MQTT with JSON payload."""
+    assert await async_setup_component(hass, sensor.DOMAIN, {
+        sensor.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
+
+    async_fire_mqtt_message(hass, 'attr-topic', '{ "val": "100" }')
+    await hass.async_block_till_done()
+    state = hass.states.get('sensor.test')
+
+    assert '100' == state.attributes.get('val')
+
+
+async def test_update_with_json_attrs_not_dict(hass, mqtt_mock, caplog):
+    """Test attributes get extracted from a JSON result."""
+    assert await async_setup_component(hass, sensor.DOMAIN, {
+        sensor.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
+
+    async_fire_mqtt_message(hass, 'attr-topic', '[ "list", "of", "things"]')
+    await hass.async_block_till_done()
+    state = hass.states.get('sensor.test')
+
+    assert state.attributes.get('val') is None
+    assert 'JSON result was not a dictionary' in caplog.text
+
+
+async def test_update_with_json_attrs_bad_JSON(hass, mqtt_mock, caplog):
+    """Test attributes get extracted from a JSON result."""
+    assert await async_setup_component(hass, sensor.DOMAIN, {
+        sensor.DOMAIN: {
+            'platform': 'mqtt',
+            'name': 'test',
+            'state_topic': 'test-topic',
+            'json_attributes_topic': 'attr-topic'
+        }
+    })
+
+    async_fire_mqtt_message(hass, 'attr-topic', 'This is not JSON')
+    await hass.async_block_till_done()
+
+    state = hass.states.get('sensor.test')
+    assert state.attributes.get('val') is None
+    assert 'Erroneous JSON: This is not JSON' in caplog.text
+
+
+async def test_discovery_update_attr(hass, mqtt_mock, caplog):
+    """Test update of discovered MQTTAttributes."""
+    entry = MockConfigEntry(domain=mqtt.DOMAIN)
+    await async_start(hass, 'homeassistant', {}, entry)
+    data1 = (
+        '{ "name": "Beer",'
+        '  "command_topic": "test_topic",'
+        '  "json_attributes_topic": "attr-topic1" }'
+    )
+    data2 = (
+        '{ "name": "Beer",'
+        '  "command_topic": "test_topic",'
+        '  "json_attributes_topic": "attr-topic2" }'
+    )
+    async_fire_mqtt_message(hass, 'homeassistant/sensor/bla/config',
+                            data1)
+    await hass.async_block_till_done()
+    async_fire_mqtt_message(hass, 'attr-topic1', '{ "val": "100" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('sensor.beer')
+    assert '100' == state.attributes.get('val')
+
+    # Change json_attributes_topic
+    async_fire_mqtt_message(hass, 'homeassistant/sensor/bla/config',
+                            data2)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+
+    # Verify we are no longer subscribing to the old topic
+    async_fire_mqtt_message(hass, 'attr-topic1', '{ "val": "50" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('sensor.beer')
+    assert '100' == state.attributes.get('val')
+
+    # Verify we are subscribing to the new topic
+    async_fire_mqtt_message(hass, 'attr-topic2', '{ "val": "75" }')
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('sensor.beer')
+    assert '75' == state.attributes.get('val')
+
+
 async def test_unique_id(hass):
     """Test unique id option only creates one sensor per unique_id."""
     await async_mock_mqtt_component(hass)