Enhance MQTT cover platform (#46059)

* Enhance MQTT cover platform

Allow combining of position and state of MQTT cover
Add template and fix optimistic in set tilt position
Add tests

* Add abbreviations

* Add tests and stopped state

* Cleanup & fix range for templates

* Apply suggestions from code review

Co-authored-by: Erik Montnemery <erik@montnemery.com>

homeassistant/components/mqtt/abbreviations.py

@@ -136,6 +136,7 @@ ABBREVIATIONS = {
     "set_pos_tpl": "set_position_template",
     "set_pos_t": "set_position_topic",
     "pos_t": "position_topic",
+    "pos_tpl": "position_template",
     "spd_cmd_t": "speed_command_topic",
     "spd_stat_t": "speed_state_topic",
     "spd_val_tpl": "speed_value_template",
@@ -147,6 +148,7 @@ ABBREVIATIONS = {
     "stat_on": "state_on",
     "stat_open": "state_open",
     "stat_opening": "state_opening",
+    "stat_stopped": "state_stopped",
     "stat_locked": "state_locked",
     "stat_unlocked": "state_unlocked",
     "stat_t": "state_topic",
@@ -173,6 +175,7 @@ ABBREVIATIONS = {
     "temp_unit": "temperature_unit",
     "tilt_clsd_val": "tilt_closed_value",
     "tilt_cmd_t": "tilt_command_topic",
+    "tilt_cmd_tpl": "tilt_command_template",
     "tilt_inv_stat": "tilt_invert_state",
     "tilt_max": "tilt_max",
     "tilt_min": "tilt_min",

homeassistant/components/mqtt/cover.py

@@ -59,9 +59,11 @@ from .mixins import (
 _LOGGER = logging.getLogger(__name__)
 
 CONF_GET_POSITION_TOPIC = "position_topic"
-CONF_SET_POSITION_TEMPLATE = "set_position_template"
+CONF_GET_POSITION_TEMPLATE = "position_template"
 CONF_SET_POSITION_TOPIC = "set_position_topic"
+CONF_SET_POSITION_TEMPLATE = "set_position_template"
 CONF_TILT_COMMAND_TOPIC = "tilt_command_topic"
+CONF_TILT_COMMAND_TEMPLATE = "tilt_command_template"
 CONF_TILT_STATUS_TOPIC = "tilt_status_topic"
 CONF_TILT_STATUS_TEMPLATE = "tilt_status_template"
 
@@ -74,6 +76,7 @@ CONF_STATE_CLOSED = "state_closed"
 CONF_STATE_CLOSING = "state_closing"
 CONF_STATE_OPEN = "state_open"
 CONF_STATE_OPENING = "state_opening"
+CONF_STATE_STOPPED = "state_stopped"
 CONF_TILT_CLOSED_POSITION = "tilt_closed_value"
 CONF_TILT_INVERT_STATE = "tilt_invert_state"
 CONF_TILT_MAX = "tilt_max"
@@ -92,6 +95,7 @@ DEFAULT_PAYLOAD_STOP = "STOP"
 DEFAULT_POSITION_CLOSED = 0
 DEFAULT_POSITION_OPEN = 100
 DEFAULT_RETAIN = False
+DEFAULT_STATE_STOPPED = "stopped"
 DEFAULT_TILT_CLOSED_POSITION = 0
 DEFAULT_TILT_INVERT_STATE = False
 DEFAULT_TILT_MAX = 100
@@ -115,8 +119,27 @@ def validate_options(value):
     """
     if CONF_SET_POSITION_TOPIC in value and CONF_GET_POSITION_TOPIC not in value:
         raise vol.Invalid(
-            "set_position_topic must be set together with position_topic."
+            "'set_position_topic' must be set together with 'position_topic'."
         )
+
+    if (
+        CONF_GET_POSITION_TOPIC in value
+        and CONF_STATE_TOPIC not in value
+        and CONF_VALUE_TEMPLATE in value
+    ):
+        _LOGGER.warning(
+            "using 'value_template' for 'position_topic' is deprecated "
+            "and will be removed from Home Assistant in version 2021.6"
+            "please replace it with 'position_template'"
+        )
+
+    if CONF_TILT_INVERT_STATE in value:
+        _LOGGER.warning(
+            "'tilt_invert_state' is deprecated "
+            "and will be removed from Home Assistant in version 2021.6"
+            "please invert tilt using 'tilt_min' & 'tilt_max'"
+        )
+
     return value
 
 
@@ -143,6 +166,7 @@ PLATFORM_SCHEMA = vol.All(
             vol.Optional(CONF_STATE_CLOSING, default=STATE_CLOSING): cv.string,
             vol.Optional(CONF_STATE_OPEN, default=STATE_OPEN): cv.string,
             vol.Optional(CONF_STATE_OPENING, default=STATE_OPENING): cv.string,
+            vol.Optional(CONF_STATE_STOPPED, default=DEFAULT_STATE_STOPPED): cv.string,
             vol.Optional(CONF_STATE_TOPIC): mqtt.valid_subscribe_topic,
             vol.Optional(
                 CONF_TILT_CLOSED_POSITION, default=DEFAULT_TILT_CLOSED_POSITION
@@ -163,6 +187,8 @@ PLATFORM_SCHEMA = vol.All(
             vol.Optional(CONF_TILT_STATUS_TEMPLATE): cv.template,
             vol.Optional(CONF_UNIQUE_ID): cv.string,
             vol.Optional(CONF_VALUE_TEMPLATE): cv.template,
+            vol.Optional(CONF_GET_POSITION_TEMPLATE): cv.template,
+            vol.Optional(CONF_TILT_COMMAND_TEMPLATE): cv.template,
         }
     )
     .extend(MQTT_AVAILABILITY_SCHEMA.schema)
@@ -228,6 +254,9 @@ class MqttCover(MqttEntity, CoverEntity):
         set_position_template = self._config.get(CONF_SET_POSITION_TEMPLATE)
         if set_position_template is not None:
             set_position_template.hass = self.hass
+        set_tilt_template = self._config.get(CONF_TILT_COMMAND_TEMPLATE)
+        if set_tilt_template is not None:
+            set_tilt_template.hass = self.hass
         tilt_status_template = self._config.get(CONF_TILT_STATUS_TEMPLATE)
         if tilt_status_template is not None:
             tilt_status_template.hass = self.hass
@@ -266,17 +295,31 @@ class MqttCover(MqttEntity, CoverEntity):
             if template is not None:
                 payload = template.async_render_with_possible_json_value(payload)
 
-            if payload == self._config[CONF_STATE_OPEN]:
+            if payload == self._config[CONF_STATE_STOPPED]:
-                self._state = STATE_OPEN
+                if (
+                    self._optimistic
+                    or self._config.get(CONF_GET_POSITION_TOPIC) is None
+                ):
+                    self._state = (
+                        STATE_CLOSED if self._state == STATE_CLOSING else STATE_OPEN
+                    )
+                else:
+                    self._state = (
+                        STATE_CLOSED
+                        if self._position == DEFAULT_POSITION_CLOSED
+                        else STATE_OPEN
+                    )
             elif payload == self._config[CONF_STATE_OPENING]:
                 self._state = STATE_OPENING
-            elif payload == self._config[CONF_STATE_CLOSED]:
-                self._state = STATE_CLOSED
             elif payload == self._config[CONF_STATE_CLOSING]:
                 self._state = STATE_CLOSING
+            elif payload == self._config[CONF_STATE_OPEN]:
+                self._state = STATE_OPEN
+            elif payload == self._config[CONF_STATE_CLOSED]:
+                self._state = STATE_CLOSED
             else:
                 _LOGGER.warning(
-                    "Payload is not supported (e.g. open, closed, opening, closing): %s",
+                    "Payload is not supported (e.g. open, closed, opening, closing, stopped): %s",
                     payload,
                 )
                 return
@@ -286,9 +329,16 @@ class MqttCover(MqttEntity, CoverEntity):
         @callback
         @log_messages(self.hass, self.entity_id)
         def position_message_received(msg):
-            """Handle new MQTT state messages."""
+            """Handle new MQTT position messages."""
             payload = msg.payload
-            template = self._config.get(CONF_VALUE_TEMPLATE)
+
+            template = self._config.get(CONF_GET_POSITION_TEMPLATE)
+
+            # To be removed in 2021.6:
+            # allow using `value_template` as position template if no `state_topic`
+            if template is None and self._config.get(CONF_STATE_TOPIC) is None:
+                template = self._config.get(CONF_VALUE_TEMPLATE)
+
             if template is not None:
                 payload = template.async_render_with_possible_json_value(payload)
 
@@ -297,13 +347,14 @@ class MqttCover(MqttEntity, CoverEntity):
                     float(payload), COVER_PAYLOAD
                 )
                 self._position = percentage_payload
-                self._state = (
+                if self._config.get(CONF_STATE_TOPIC) is None:
-                    STATE_CLOSED
+                    self._state = (
-                    if percentage_payload == DEFAULT_POSITION_CLOSED
+                        STATE_CLOSED
-                    else STATE_OPEN
+                        if percentage_payload == DEFAULT_POSITION_CLOSED
-                )
+                        else STATE_OPEN
+                    )
             else:
-                _LOGGER.warning("Payload is not integer within range: %s", payload)
+                _LOGGER.warning("Payload '%s' is not numeric", payload)
                 return
             self.async_write_ha_state()
 
@@ -313,13 +364,18 @@ class MqttCover(MqttEntity, CoverEntity):
                 "msg_callback": position_message_received,
                 "qos": self._config[CONF_QOS],
             }
-        elif self._config.get(CONF_STATE_TOPIC):
+
+        if self._config.get(CONF_STATE_TOPIC):
             topics["state_topic"] = {
                 "topic": self._config.get(CONF_STATE_TOPIC),
                 "msg_callback": state_message_received,
                 "qos": self._config[CONF_QOS],
             }
-        else:
+
+        if (
+            self._config.get(CONF_GET_POSITION_TOPIC) is None
+            and self._config.get(CONF_STATE_TOPIC) is None
+        ):
             # Force into optimistic mode.
             self._optimistic = True
 
@@ -488,28 +544,32 @@ class MqttCover(MqttEntity, CoverEntity):
 
     async def async_set_cover_tilt_position(self, **kwargs):
         """Move the cover tilt to a specific position."""
-        position = kwargs[ATTR_TILT_POSITION]
+        set_tilt_template = self._config.get(CONF_TILT_COMMAND_TEMPLATE)
-
+        tilt = kwargs[ATTR_TILT_POSITION]
-        # The position needs to be between min and max
+        percentage_tilt = tilt
-        level = self.find_in_range_from_percent(position)
+        tilt = self.find_in_range_from_percent(tilt)
+        if set_tilt_template is not None:
+            tilt = set_tilt_template.async_render(parse_result=False, **kwargs)
 
         mqtt.async_publish(
             self.hass,
             self._config.get(CONF_TILT_COMMAND_TOPIC),
-            level,
+            tilt,
             self._config[CONF_QOS],
             self._config[CONF_RETAIN],
         )
+        if self._tilt_optimistic:
+            self._tilt_value = percentage_tilt
+            self.async_write_ha_state()
 
     async def async_set_cover_position(self, **kwargs):
         """Move the cover to a specific position."""
         set_position_template = self._config.get(CONF_SET_POSITION_TEMPLATE)
         position = kwargs[ATTR_POSITION]
         percentage_position = position
+        position = self.find_in_range_from_percent(position, COVER_PAYLOAD)
         if set_position_template is not None:
             position = set_position_template.async_render(parse_result=False, **kwargs)
-        else:
-            position = self.find_in_range_from_percent(position, COVER_PAYLOAD)
 
         mqtt.async_publish(
             self.hass,

tests/components/mqtt/test_cover.py

@@ -1082,6 +1082,23 @@ async def test_tilt_given_value_optimistic(hass, mqtt_mock):
     )
     mqtt_mock.async_publish.reset_mock()
 
+    await hass.services.async_call(
+        cover.DOMAIN,
+        SERVICE_SET_COVER_TILT_POSITION,
+        {ATTR_ENTITY_ID: "cover.test", ATTR_TILT_POSITION: 50},
+        blocking=True,
+    )
+
+    current_cover_tilt_position = hass.states.get("cover.test").attributes[
+        ATTR_CURRENT_TILT_POSITION
+    ]
+    assert current_cover_tilt_position == 50
+
+    mqtt_mock.async_publish.assert_called_once_with(
+        "tilt-command-topic", "50", 0, False
+    )
+    mqtt_mock.async_publish.reset_mock()
+
     await hass.services.async_call(
         cover.DOMAIN,
         SERVICE_CLOSE_COVER_TILT,
@@ -1381,6 +1398,41 @@ async def test_tilt_position(hass, mqtt_mock):
     )
 
 
+async def test_tilt_position_templated(hass, mqtt_mock):
+    """Test tilt position via template."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "state_topic": "state-topic",
+                "command_topic": "command-topic",
+                "qos": 0,
+                "payload_open": "OPEN",
+                "payload_close": "CLOSE",
+                "payload_stop": "STOP",
+                "tilt_command_topic": "tilt-command-topic",
+                "tilt_status_topic": "tilt-status-topic",
+                "tilt_command_template": "{{100-32}}",
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    await hass.services.async_call(
+        cover.DOMAIN,
+        SERVICE_SET_COVER_TILT_POSITION,
+        {ATTR_ENTITY_ID: "cover.test", ATTR_TILT_POSITION: 100},
+        blocking=True,
+    )
+
+    mqtt_mock.async_publish.assert_called_once_with(
+        "tilt-command-topic", "68", 0, False
+    )
+
+
 async def test_tilt_position_altered_range(hass, mqtt_mock):
     """Test tilt via method invocation with altered range."""
     assert await async_setup_component(
@@ -1978,3 +2030,298 @@ async def test_entity_debug_info_message(hass, mqtt_mock):
     await help_test_entity_debug_info_message(
         hass, mqtt_mock, cover.DOMAIN, DEFAULT_CONFIG
     )
+
+
+async def test_deprecated_value_template_for_position_topic_warnning(
+    hass, caplog, mqtt_mock
+):
+    """Test warnning when value_template is used for position_topic."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "command_topic": "command-topic",
+                "set_position_topic": "set-position-topic",
+                "position_topic": "position-topic",
+                "value_template": "{{100-62}}",
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    assert (
+        "using 'value_template' for 'position_topic' is deprecated "
+        "and will be removed from Home Assistant in version 2021.6"
+        "please replace it with 'position_template'"
+    ) in caplog.text
+
+
+async def test_deprecated_tilt_invert_state_warnning(hass, caplog, mqtt_mock):
+    """Test warnning when tilt_invert_state is used."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "command_topic": "command-topic",
+                "tilt_invert_state": True,
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    assert (
+        "'tilt_invert_state' is deprecated "
+        "and will be removed from Home Assistant in version 2021.6"
+        "please invert tilt using 'tilt_min' & 'tilt_max'"
+    ) in caplog.text
+
+
+async def test_no_deprecated_warning_for_position_topic_using_position_template(
+    hass, caplog, mqtt_mock
+):
+    """Test no warning when position_template is used for position_topic."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "command_topic": "command-topic",
+                "set_position_topic": "set-position-topic",
+                "position_topic": "position-topic",
+                "position_template": "{{100-62}}",
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    assert (
+        "using 'value_template' for 'position_topic' is deprecated "
+        "and will be removed from Home Assistant in version 2021.6"
+        "please replace it with 'position_template'"
+    ) not in caplog.text
+
+
+async def test_state_and_position_topics_state_not_set_via_position_topic(
+    hass, mqtt_mock
+):
+    """Test state is not set via position topic when both state and position topics are set."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "state_topic": "state-topic",
+                "position_topic": "get-position-topic",
+                "position_open": 100,
+                "position_closed": 0,
+                "state_open": "OPEN",
+                "state_closed": "CLOSE",
+                "command_topic": "command-topic",
+                "qos": 0,
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_UNKNOWN
+    assert not state.attributes.get(ATTR_ASSUMED_STATE)
+
+    async_fire_mqtt_message(hass, "state-topic", "OPEN")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "get-position-topic", "0")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "get-position-topic", "100")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "state-topic", "CLOSE")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSED
+
+    async_fire_mqtt_message(hass, "get-position-topic", "0")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSED
+
+    async_fire_mqtt_message(hass, "get-position-topic", "100")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSED
+
+
+async def test_set_state_via_position_using_stopped_state(hass, mqtt_mock):
+    """Test the controlling state via position topic using stopped state."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "state_topic": "state-topic",
+                "position_topic": "get-position-topic",
+                "position_open": 100,
+                "position_closed": 0,
+                "state_open": "OPEN",
+                "state_closed": "CLOSE",
+                "state_stopped": "STOPPED",
+                "command_topic": "command-topic",
+                "qos": 0,
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_UNKNOWN
+    assert not state.attributes.get(ATTR_ASSUMED_STATE)
+
+    async_fire_mqtt_message(hass, "state-topic", "OPEN")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "get-position-topic", "0")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "state-topic", "STOPPED")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSED
+
+    async_fire_mqtt_message(hass, "get-position-topic", "100")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSED
+
+    async_fire_mqtt_message(hass, "state-topic", "STOPPED")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+
+async def test_set_state_via_stopped_state_optimistic(hass, mqtt_mock):
+    """Test the controlling state via stopped state in optimistic mode."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "state_topic": "state-topic",
+                "position_topic": "get-position-topic",
+                "position_open": 100,
+                "position_closed": 0,
+                "state_open": "OPEN",
+                "state_closed": "CLOSE",
+                "state_stopped": "STOPPED",
+                "state_opening": "OPENING",
+                "state_closing": "CLOSING",
+                "command_topic": "command-topic",
+                "qos": 0,
+                "optimistic": True,
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    async_fire_mqtt_message(hass, "state-topic", "OPEN")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "get-position-topic", "50")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "state-topic", "OPENING")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPENING
+
+    async_fire_mqtt_message(hass, "state-topic", "STOPPED")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "state-topic", "CLOSING")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSING
+
+    async_fire_mqtt_message(hass, "state-topic", "STOPPED")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSED
+
+
+async def test_set_state_via_stopped_state_no_position_topic(hass, mqtt_mock):
+    """Test the controlling state via stopped state when no position topic."""
+    assert await async_setup_component(
+        hass,
+        cover.DOMAIN,
+        {
+            cover.DOMAIN: {
+                "platform": "mqtt",
+                "name": "test",
+                "state_topic": "state-topic",
+                "state_open": "OPEN",
+                "state_closed": "CLOSE",
+                "state_stopped": "STOPPED",
+                "state_opening": "OPENING",
+                "state_closing": "CLOSING",
+                "command_topic": "command-topic",
+                "qos": 0,
+                "optimistic": False,
+            }
+        },
+    )
+    await hass.async_block_till_done()
+
+    async_fire_mqtt_message(hass, "state-topic", "OPEN")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "state-topic", "OPENING")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPENING
+
+    async_fire_mqtt_message(hass, "state-topic", "STOPPED")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_OPEN
+
+    async_fire_mqtt_message(hass, "state-topic", "CLOSING")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSING
+
+    async_fire_mqtt_message(hass, "state-topic", "STOPPED")
+
+    state = hass.states.get("cover.test")
+    assert state.state == STATE_CLOSED