Fix bugs in MQTT vacuum (#23048)

homeassistant/components/mqtt/vacuum.py

@@ -63,58 +63,57 @@ ALL_SERVICES = DEFAULT_SERVICES | SUPPORT_PAUSE | SUPPORT_LOCATE |\
                SUPPORT_FAN_SPEED | SUPPORT_SEND_COMMAND
 
 CONF_SUPPORTED_FEATURES = ATTR_SUPPORTED_FEATURES
-CONF_PAYLOAD_TURN_ON = 'payload_turn_on'
+CONF_BATTERY_LEVEL_TEMPLATE = 'battery_level_template'
-CONF_PAYLOAD_TURN_OFF = 'payload_turn_off'
+CONF_BATTERY_LEVEL_TOPIC = 'battery_level_topic'
-CONF_PAYLOAD_RETURN_TO_BASE = 'payload_return_to_base'
+CONF_CHARGING_TEMPLATE = 'charging_template'
-CONF_PAYLOAD_STOP = 'payload_stop'
+CONF_CHARGING_TOPIC = 'charging_topic'
+CONF_CLEANING_TEMPLATE = 'cleaning_template'
+CONF_CLEANING_TOPIC = 'cleaning_topic'
+CONF_DOCKED_TEMPLATE = 'docked_template'
+CONF_DOCKED_TOPIC = 'docked_topic'
+CONF_ERROR_TEMPLATE = 'error_template'
+CONF_ERROR_TOPIC = 'error_topic'
+CONF_FAN_SPEED_LIST = 'fan_speed_list'
+CONF_FAN_SPEED_TEMPLATE = 'fan_speed_template'
+CONF_FAN_SPEED_TOPIC = 'fan_speed_topic'
 CONF_PAYLOAD_CLEAN_SPOT = 'payload_clean_spot'
 CONF_PAYLOAD_LOCATE = 'payload_locate'
+CONF_PAYLOAD_RETURN_TO_BASE = 'payload_return_to_base'
 CONF_PAYLOAD_START_PAUSE = 'payload_start_pause'
-CONF_BATTERY_LEVEL_TOPIC = 'battery_level_topic'
+CONF_PAYLOAD_STOP = 'payload_stop'
-CONF_BATTERY_LEVEL_TEMPLATE = 'battery_level_template'
+CONF_PAYLOAD_TURN_OFF = 'payload_turn_off'
-CONF_CHARGING_TOPIC = 'charging_topic'
+CONF_PAYLOAD_TURN_ON = 'payload_turn_on'
-CONF_CHARGING_TEMPLATE = 'charging_template'
-CONF_CLEANING_TOPIC = 'cleaning_topic'
-CONF_CLEANING_TEMPLATE = 'cleaning_template'
-CONF_DOCKED_TOPIC = 'docked_topic'
-CONF_DOCKED_TEMPLATE = 'docked_template'
-CONF_ERROR_TOPIC = 'error_topic'
-CONF_ERROR_TEMPLATE = 'error_template'
-CONF_STATE_TOPIC = 'state_topic'
-CONF_STATE_TEMPLATE = 'state_template'
-CONF_FAN_SPEED_TOPIC = 'fan_speed_topic'
-CONF_FAN_SPEED_TEMPLATE = 'fan_speed_template'
-CONF_SET_FAN_SPEED_TOPIC = 'set_fan_speed_topic'
-CONF_FAN_SPEED_LIST = 'fan_speed_list'
 CONF_SEND_COMMAND_TOPIC = 'send_command_topic'
+CONF_SET_FAN_SPEED_TOPIC = 'set_fan_speed_topic'
 
 DEFAULT_NAME = 'MQTT Vacuum'
-DEFAULT_RETAIN = False
-DEFAULT_SERVICE_STRINGS = services_to_strings(DEFAULT_SERVICES)
-DEFAULT_PAYLOAD_TURN_ON = 'turn_on'
-DEFAULT_PAYLOAD_TURN_OFF = 'turn_off'
-DEFAULT_PAYLOAD_RETURN_TO_BASE = 'return_to_base'
-DEFAULT_PAYLOAD_STOP = 'stop'
 DEFAULT_PAYLOAD_CLEAN_SPOT = 'clean_spot'
 DEFAULT_PAYLOAD_LOCATE = 'locate'
+DEFAULT_PAYLOAD_RETURN_TO_BASE = 'return_to_base'
 DEFAULT_PAYLOAD_START_PAUSE = 'start_pause'
+DEFAULT_PAYLOAD_STOP = 'stop'
+DEFAULT_PAYLOAD_TURN_OFF = 'turn_off'
+DEFAULT_PAYLOAD_TURN_ON = 'turn_on'
+DEFAULT_RETAIN = False
+DEFAULT_SERVICE_STRINGS = services_to_strings(DEFAULT_SERVICES)
 
 PLATFORM_SCHEMA = mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend({
-    vol.Optional(CONF_BATTERY_LEVEL_TEMPLATE): cv.template,
+    vol.Inclusive(CONF_BATTERY_LEVEL_TEMPLATE, 'battery'): cv.template,
-    vol.Optional(CONF_BATTERY_LEVEL_TOPIC): mqtt.valid_publish_topic,
+    vol.Inclusive(CONF_BATTERY_LEVEL_TOPIC,
-    vol.Optional(CONF_CHARGING_TEMPLATE): cv.template,
+                  'battery'): mqtt.valid_publish_topic,
-    vol.Optional(CONF_CHARGING_TOPIC): mqtt.valid_publish_topic,
+    vol.Inclusive(CONF_CHARGING_TEMPLATE, 'charging'): cv.template,
-    vol.Optional(CONF_CLEANING_TEMPLATE): cv.template,
+    vol.Inclusive(CONF_CHARGING_TOPIC, 'charging'): mqtt.valid_publish_topic,
-    vol.Optional(CONF_CLEANING_TOPIC): mqtt.valid_publish_topic,
+    vol.Inclusive(CONF_CLEANING_TEMPLATE, 'cleaning'): cv.template,
+    vol.Inclusive(CONF_CLEANING_TOPIC, 'cleaning'): mqtt.valid_publish_topic,
     vol.Optional(CONF_DEVICE): mqtt.MQTT_ENTITY_DEVICE_INFO_SCHEMA,
-    vol.Optional(CONF_DOCKED_TEMPLATE): cv.template,
+    vol.Inclusive(CONF_DOCKED_TEMPLATE, 'docked'): cv.template,
-    vol.Optional(CONF_DOCKED_TOPIC): mqtt.valid_publish_topic,
+    vol.Inclusive(CONF_DOCKED_TOPIC, 'docked'): mqtt.valid_publish_topic,
-    vol.Optional(CONF_ERROR_TEMPLATE): cv.template,
+    vol.Inclusive(CONF_ERROR_TEMPLATE, 'error'): cv.template,
-    vol.Optional(CONF_ERROR_TOPIC): mqtt.valid_publish_topic,
+    vol.Inclusive(CONF_ERROR_TOPIC, 'error'): mqtt.valid_publish_topic,
     vol.Optional(CONF_FAN_SPEED_LIST, default=[]):
         vol.All(cv.ensure_list, [cv.string]),
-    vol.Optional(CONF_FAN_SPEED_TEMPLATE): cv.template,
+    vol.Inclusive(CONF_FAN_SPEED_TEMPLATE, 'fan_speed'): cv.template,
-    vol.Optional(CONF_FAN_SPEED_TOPIC): mqtt.valid_publish_topic,
+    vol.Inclusive(CONF_FAN_SPEED_TOPIC, 'fan_speed'): mqtt.valid_publish_topic,
     vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
     vol.Optional(CONF_PAYLOAD_CLEAN_SPOT,
                  default=DEFAULT_PAYLOAD_CLEAN_SPOT): cv.string,
@@ -131,8 +130,6 @@ PLATFORM_SCHEMA = mqtt.MQTT_BASE_PLATFORM_SCHEMA.extend({
                  default=DEFAULT_PAYLOAD_TURN_ON): cv.string,
     vol.Optional(CONF_SEND_COMMAND_TOPIC): mqtt.valid_publish_topic,
     vol.Optional(CONF_SET_FAN_SPEED_TOPIC): mqtt.valid_publish_topic,
-    vol.Optional(CONF_STATE_TEMPLATE): cv.template,
-    vol.Optional(CONF_STATE_TOPIC): mqtt.valid_publish_topic,
     vol.Optional(CONF_SUPPORTED_FEATURES, default=DEFAULT_SERVICE_STRINGS):
         vol.All(cv.ensure_list, [vol.In(STRING_TO_SERVICE.keys())]),
     vol.Optional(CONF_UNIQUE_ID): cv.string,
@@ -283,7 +280,7 @@ class MqttVacuum(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
                 battery_level = self._templates[CONF_BATTERY_LEVEL_TEMPLATE]\
                     .async_render_with_possible_json_value(
                         msg.payload, error_value=None)
-                if battery_level is not None:
+                if battery_level:
                     self._battery_level = int(battery_level)
 
             if msg.topic == self._state_topics[CONF_CHARGING_TOPIC] and \
@@ -291,7 +288,7 @@ class MqttVacuum(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
                 charging = self._templates[CONF_CHARGING_TEMPLATE]\
                     .async_render_with_possible_json_value(
                         msg.payload, error_value=None)
-                if charging is not None:
+                if charging:
                     self._charging = cv.boolean(charging)
 
             if msg.topic == self._state_topics[CONF_CLEANING_TOPIC] and \
@@ -299,7 +296,7 @@ class MqttVacuum(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
                 cleaning = self._templates[CONF_CLEANING_TEMPLATE]\
                     .async_render_with_possible_json_value(
                         msg.payload, error_value=None)
-                if cleaning is not None:
+                if cleaning:
                     self._cleaning = cv.boolean(cleaning)
 
             if msg.topic == self._state_topics[CONF_DOCKED_TOPIC] and \
@@ -307,7 +304,7 @@ class MqttVacuum(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
                 docked = self._templates[CONF_DOCKED_TEMPLATE]\
                     .async_render_with_possible_json_value(
                         msg.payload, error_value=None)
-                if docked is not None:
+                if docked:
                     self._docked = cv.boolean(docked)
 
             if msg.topic == self._state_topics[CONF_ERROR_TOPIC] and \
@@ -315,7 +312,7 @@ class MqttVacuum(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
                 error = self._templates[CONF_ERROR_TEMPLATE]\
                     .async_render_with_possible_json_value(
                         msg.payload, error_value=None)
-                if error is not None:
+                if error:
                     self._error = cv.string(error)
 
             if self._docked:
@@ -325,7 +322,7 @@ class MqttVacuum(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
                     self._status = "Docked"
             elif self._cleaning:
                 self._status = "Cleaning"
-            elif self._error is not None and not self._error:
+            elif self._error:
                 self._status = "Error: {}".format(self._error)
             else:
                 self._status = "Stopped"
@@ -335,7 +332,7 @@ class MqttVacuum(MqttAttributes, MqttAvailability, MqttDiscoveryUpdate,
                 fan_speed = self._templates[CONF_FAN_SPEED_TEMPLATE]\
                     .async_render_with_possible_json_value(
                         msg.payload, error_value=None)
-                if fan_speed is not None:
+                if fan_speed:
                     self._fan_speed = fan_speed
 
             self.async_write_ha_state()

tests/components/mqtt/test_vacuum.py

@@ -1,6 +1,6 @@
 """The tests for the Mqtt vacuum platform."""
+import copy
 import json
-
 import pytest
 
 from homeassistant.components import mqtt, vacuum
@@ -31,8 +31,8 @@ default_config = {
     mqttvacuum.CONF_CLEANING_TEMPLATE: '{{ value_json.cleaning }}',
     mqttvacuum.CONF_DOCKED_TOPIC: 'vacuum/state',
     mqttvacuum.CONF_DOCKED_TEMPLATE: '{{ value_json.docked }}',
-    mqttvacuum.CONF_STATE_TOPIC: 'vacuum/state',
+    mqttvacuum.CONF_ERROR_TOPIC: 'vacuum/state',
-    mqttvacuum.CONF_STATE_TEMPLATE: '{{ value_json.state }}',
+    mqttvacuum.CONF_ERROR_TEMPLATE: '{{ value_json.error }}',
     mqttvacuum.CONF_FAN_SPEED_TOPIC: 'vacuum/state',
     mqttvacuum.CONF_FAN_SPEED_TEMPLATE: '{{ value_json.fan_speed }}',
     mqttvacuum.CONF_SET_FAN_SPEED_TOPIC: 'vacuum/set_fan_speed',
@@ -177,6 +177,122 @@ async def test_status(hass, mock_publish):
     assert 'min' == state.attributes.get(ATTR_FAN_SPEED)
 
 
+async def test_status_battery(hass, mock_publish):
+    """Test status updates from the vacuum."""
+    default_config[mqttvacuum.CONF_SUPPORTED_FEATURES] = \
+        mqttvacuum.services_to_strings(mqttvacuum.ALL_SERVICES)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: default_config,
+    })
+
+    message = """{
+        "battery_level": 54
+    }"""
+    async_fire_mqtt_message(hass, 'vacuum/state', message)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('vacuum.mqtttest')
+    assert 'mdi:battery-50' == \
+        state.attributes.get(ATTR_BATTERY_ICON)
+
+
+async def test_status_cleaning(hass, mock_publish):
+    """Test status updates from the vacuum."""
+    default_config[mqttvacuum.CONF_SUPPORTED_FEATURES] = \
+        mqttvacuum.services_to_strings(mqttvacuum.ALL_SERVICES)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: default_config,
+    })
+
+    message = """{
+        "cleaning": true
+    }"""
+    async_fire_mqtt_message(hass, 'vacuum/state', message)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('vacuum.mqtttest')
+    assert STATE_ON == state.state
+
+
+async def test_status_docked(hass, mock_publish):
+    """Test status updates from the vacuum."""
+    default_config[mqttvacuum.CONF_SUPPORTED_FEATURES] = \
+        mqttvacuum.services_to_strings(mqttvacuum.ALL_SERVICES)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: default_config,
+    })
+
+    message = """{
+        "docked": true
+    }"""
+    async_fire_mqtt_message(hass, 'vacuum/state', message)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('vacuum.mqtttest')
+    assert STATE_OFF == state.state
+
+
+async def test_status_charging(hass, mock_publish):
+    """Test status updates from the vacuum."""
+    default_config[mqttvacuum.CONF_SUPPORTED_FEATURES] = \
+        mqttvacuum.services_to_strings(mqttvacuum.ALL_SERVICES)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: default_config,
+    })
+
+    message = """{
+        "charging": true
+    }"""
+    async_fire_mqtt_message(hass, 'vacuum/state', message)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('vacuum.mqtttest')
+    assert 'mdi:battery-outline' == \
+        state.attributes.get(ATTR_BATTERY_ICON)
+
+
+async def test_status_fan_speed(hass, mock_publish):
+    """Test status updates from the vacuum."""
+    default_config[mqttvacuum.CONF_SUPPORTED_FEATURES] = \
+        mqttvacuum.services_to_strings(mqttvacuum.ALL_SERVICES)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: default_config,
+    })
+
+    message = """{
+        "fan_speed": "max"
+    }"""
+    async_fire_mqtt_message(hass, 'vacuum/state', message)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('vacuum.mqtttest')
+    assert 'max' == state.attributes.get(ATTR_FAN_SPEED)
+
+
+async def test_status_error(hass, mock_publish):
+    """Test status updates from the vacuum."""
+    default_config[mqttvacuum.CONF_SUPPORTED_FEATURES] = \
+        mqttvacuum.services_to_strings(mqttvacuum.ALL_SERVICES)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: default_config,
+    })
+
+    message = """{
+        "error": "Error1"
+    }"""
+    async_fire_mqtt_message(hass, 'vacuum/state', message)
+    await hass.async_block_till_done()
+    await hass.async_block_till_done()
+    state = hass.states.get('vacuum.mqtttest')
+    assert 'Error: Error1' == state.attributes.get(ATTR_STATUS)
+
+
 async def test_battery_template(hass, mock_publish):
     """Test that you can use non-default templates for battery_level."""
     default_config.update({
@@ -214,6 +330,84 @@ async def test_status_invalid_json(hass, mock_publish):
     assert "Stopped" == state.attributes.get(ATTR_STATUS)
 
 
+async def test_missing_battery_template(hass, mock_publish):
+    """Test to make sure missing template is not allowed."""
+    config = copy.deepcopy(default_config)
+    config.pop(mqttvacuum.CONF_BATTERY_LEVEL_TEMPLATE)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: config,
+    })
+
+    state = hass.states.get('vacuum.mqtttest')
+    assert state is None
+
+
+async def test_missing_charging_template(hass, mock_publish):
+    """Test to make sure missing template is not allowed."""
+    config = copy.deepcopy(default_config)
+    config.pop(mqttvacuum.CONF_CHARGING_TEMPLATE)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: config,
+    })
+
+    state = hass.states.get('vacuum.mqtttest')
+    assert state is None
+
+
+async def test_missing_cleaning_template(hass, mock_publish):
+    """Test to make sure missing template is not allowed."""
+    config = copy.deepcopy(default_config)
+    config.pop(mqttvacuum.CONF_CLEANING_TEMPLATE)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: config,
+    })
+
+    state = hass.states.get('vacuum.mqtttest')
+    assert state is None
+
+
+async def test_missing_docked_template(hass, mock_publish):
+    """Test to make sure missing template is not allowed."""
+    config = copy.deepcopy(default_config)
+    config.pop(mqttvacuum.CONF_DOCKED_TEMPLATE)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: config,
+    })
+
+    state = hass.states.get('vacuum.mqtttest')
+    assert state is None
+
+
+async def test_missing_error_template(hass, mock_publish):
+    """Test to make sure missing template is not allowed."""
+    config = copy.deepcopy(default_config)
+    config.pop(mqttvacuum.CONF_ERROR_TEMPLATE)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: config,
+    })
+
+    state = hass.states.get('vacuum.mqtttest')
+    assert state is None
+
+
+async def test_missing_fan_speed_template(hass, mock_publish):
+    """Test to make sure missing template is not allowed."""
+    config = copy.deepcopy(default_config)
+    config.pop(mqttvacuum.CONF_FAN_SPEED_TEMPLATE)
+
+    assert await async_setup_component(hass, vacuum.DOMAIN, {
+        vacuum.DOMAIN: config,
+    })
+
+    state = hass.states.get('vacuum.mqtttest')
+    assert state is None
+
+
 async def test_default_availability_payload(hass, mock_publish):
     """Test availability by default payload with defined topic."""
     default_config.update({