From b162c45e0ad4c376c12bb5a132940460d618035c Mon Sep 17 00:00:00 2001 From: mtl010957 Date: Thu, 11 Mar 2021 07:49:10 -0500 Subject: [PATCH] Cover Tilt Position Bugfix (#47682) * Report tilt position properly when inverting using tilt_max < tilt_min * Add warning per review comment * Add test for inverted tilt position configuration * Separate non-numeric and out of range warnings per review comment * Fix out of range message and add tests for not numeric and out of range messages --- homeassistant/components/mqtt/cover.py | 15 +++- tests/components/mqtt/test_cover.py | 106 +++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/mqtt/cover.py b/homeassistant/components/mqtt/cover.py index 5c7b0c10cf6..7f810501e1a 100644 --- a/homeassistant/components/mqtt/cover.py +++ b/homeassistant/components/mqtt/cover.py @@ -267,15 +267,26 @@ class MqttCover(MqttEntity, CoverEntity): payload ) - if payload.isnumeric() and ( + if not payload.isnumeric(): + _LOGGER.warning("Payload '%s' is not numeric", payload) + elif ( self._config[CONF_TILT_MIN] <= int(payload) <= self._config[CONF_TILT_MAX] + or self._config[CONF_TILT_MAX] + <= int(payload) + <= self._config[CONF_TILT_MIN] ): - level = self.find_percentage_in_range(float(payload)) self._tilt_value = level self.async_write_ha_state() + else: + _LOGGER.warning( + "Payload '%s' is out of range, must be between '%s' and '%s' inclusive", + payload, + self._config[CONF_TILT_MIN], + self._config[CONF_TILT_MAX], + ) @callback @log_messages(self.hass, self.entity_id) diff --git a/tests/components/mqtt/test_cover.py b/tests/components/mqtt/test_cover.py index 44144642f40..d6899d5149a 100644 --- a/tests/components/mqtt/test_cover.py +++ b/tests/components/mqtt/test_cover.py @@ -1315,6 +1315,112 @@ async def test_tilt_via_topic_altered_range(hass, mqtt_mock): assert current_cover_tilt_position == 50 +async def test_tilt_status_out_of_range_warning(hass, caplog, mqtt_mock): + """Test tilt status via MQTT tilt out of range warning message.""" + 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_min": 0, + "tilt_max": 50, + } + }, + ) + await hass.async_block_till_done() + + async_fire_mqtt_message(hass, "tilt-status-topic", "60") + + assert ( + "Payload '60' is out of range, must be between '0' and '50' inclusive" + ) in caplog.text + + +async def test_tilt_status_not_numeric_warning(hass, caplog, mqtt_mock): + """Test tilt status via MQTT tilt not numeric warning message.""" + 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_min": 0, + "tilt_max": 50, + } + }, + ) + await hass.async_block_till_done() + + async_fire_mqtt_message(hass, "tilt-status-topic", "abc") + + assert ("Payload 'abc' is not numeric") in caplog.text + + +async def test_tilt_via_topic_altered_range_inverted(hass, mqtt_mock): + """Test tilt status via MQTT with altered tilt range and inverted tilt position.""" + 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_min": 50, + "tilt_max": 0, + } + }, + ) + await hass.async_block_till_done() + + async_fire_mqtt_message(hass, "tilt-status-topic", "0") + + current_cover_tilt_position = hass.states.get("cover.test").attributes[ + ATTR_CURRENT_TILT_POSITION + ] + assert current_cover_tilt_position == 100 + + async_fire_mqtt_message(hass, "tilt-status-topic", "50") + + current_cover_tilt_position = hass.states.get("cover.test").attributes[ + ATTR_CURRENT_TILT_POSITION + ] + assert current_cover_tilt_position == 0 + + async_fire_mqtt_message(hass, "tilt-status-topic", "25") + + current_cover_tilt_position = hass.states.get("cover.test").attributes[ + ATTR_CURRENT_TILT_POSITION + ] + assert current_cover_tilt_position == 50 + + async def test_tilt_via_topic_template_altered_range(hass, mqtt_mock): """Test tilt status via MQTT and template with altered tilt range.""" assert await async_setup_component(