Add stop support to openzwave (mqtt) cover (#44622)

* feat: add stop to openzwave (mqtt) cover

* Fix isort and black linter

* Remove supported_features for cover.

As suggested by @MartinHjelmare, not needed anymore because base class
implementation is sufficient.

https://github.com/home-assistant/core/pull/44622#discussion_r549854542

* Make a simpler version depending on idempotency

qt-openzwave already implements idempotency, see:
77e414217f/qt-openzwave/source/qtozwvalueidmodel.cpp (L180)

We can use it and trigger button release anywhen.

* Clean up

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

homeassistant/components/ozw/cover.py

@@ -7,7 +7,6 @@ from homeassistant.components.cover import (
     DOMAIN as COVER_DOMAIN,
     SUPPORT_CLOSE,
     SUPPORT_OPEN,
-    SUPPORT_SET_POSITION,
     CoverEntity,
 )
 from homeassistant.core import callback
@@ -16,9 +15,10 @@ from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from .const import DATA_UNSUBSCRIBE, DOMAIN
 from .entity import ZWaveDeviceEntity
 
-SUPPORTED_FEATURES_POSITION = SUPPORT_OPEN | SUPPORT_CLOSE | SUPPORT_SET_POSITION
 SUPPORT_GARAGE = SUPPORT_OPEN | SUPPORT_CLOSE
 VALUE_SELECTED_ID = "Selected_id"
+PRESS_BUTTON = True
+RELEASE_BUTTON = False
 
 
 async def async_setup_entry(hass, config_entry, async_add_entities):
@@ -52,11 +52,6 @@ def percent_to_zwave_position(value):
 class ZWaveCoverEntity(ZWaveDeviceEntity, CoverEntity):
     """Representation of a Z-Wave Cover device."""
 
-    @property
-    def supported_features(self):
-        """Flag supported features."""
-        return SUPPORTED_FEATURES_POSITION
-
     @property
     def is_closed(self):
         """Return true if cover is closed."""
@@ -73,11 +68,20 @@ class ZWaveCoverEntity(ZWaveDeviceEntity, CoverEntity):
 
     async def async_open_cover(self, **kwargs):
         """Open the cover."""
-        self.values.primary.send_value(99)
+        self.values.open.send_value(PRESS_BUTTON)
 
     async def async_close_cover(self, **kwargs):
         """Close cover."""
-        self.values.primary.send_value(0)
+        self.values.close.send_value(PRESS_BUTTON)
+
+    async def async_stop_cover(self, **kwargs):
+        """Stop cover."""
+        # Need to issue both buttons release since qt-openzwave implements idempotency
+        # keeping internal state of model to trigger actual updates. We could also keep
+        # another state in Home Assistant to know which button to release,
+        # but this implementation is simpler.
+        self.values.open.send_value(RELEASE_BUTTON)
+        self.values.close.send_value(RELEASE_BUTTON)
 
 
 class ZwaveGarageDoorBarrier(ZWaveDeviceEntity, CoverEntity):

tests/components/ozw/test_cover.py

@@ -16,6 +16,25 @@ async def test_cover(hass, cover_data, sent_messages, cover_msg):
     assert state.state == "closed"
     assert state.attributes[ATTR_CURRENT_POSITION] == 0
 
+    # Test setting position
+    await hass.services.async_call(
+        "cover",
+        "set_cover_position",
+        {"entity_id": "cover.roller_shutter_3_instance_1_level", "position": 50},
+        blocking=True,
+    )
+    assert len(sent_messages) == 1
+    msg = sent_messages[0]
+    assert msg["topic"] == "OpenZWave/1/command/setvalue/"
+    assert msg["payload"] == {"Value": 50, "ValueIDKey": 625573905}
+
+    # Feedback on state
+    cover_msg.decode()
+    cover_msg.payload["Value"] = 50
+    cover_msg.encode()
+    receive_message(cover_msg)
+    await hass.async_block_till_done()
+
     # Test opening
     await hass.services.async_call(
         "cover",
@@ -23,22 +42,26 @@ async def test_cover(hass, cover_data, sent_messages, cover_msg):
         {"entity_id": "cover.roller_shutter_3_instance_1_level"},
         blocking=True,
     )
-    assert len(sent_messages) == 1
+    assert len(sent_messages) == 2
-    msg = sent_messages[0]
+    msg = sent_messages[1]
     assert msg["topic"] == "OpenZWave/1/command/setvalue/"
-    assert msg["payload"] == {"Value": 99, "ValueIDKey": 625573905}
+    assert msg["payload"] == {"Value": True, "ValueIDKey": 281475602284568}
 
-    # Feedback on state
+    # Test stopping after opening
-    cover_msg.decode()
+    await hass.services.async_call(
-    cover_msg.payload["Value"] = 99
+        "cover",
-    cover_msg.encode()
+        "stop_cover",
-    receive_message(cover_msg)
+        {"entity_id": "cover.roller_shutter_3_instance_1_level"},
-    await hass.async_block_till_done()
+        blocking=True,
+    )
+    assert len(sent_messages) == 4
+    msg = sent_messages[2]
+    assert msg["topic"] == "OpenZWave/1/command/setvalue/"
+    assert msg["payload"] == {"Value": False, "ValueIDKey": 281475602284568}
 
-    state = hass.states.get("cover.roller_shutter_3_instance_1_level")
+    msg = sent_messages[3]
-    assert state is not None
+    assert msg["topic"] == "OpenZWave/1/command/setvalue/"
-    assert state.state == "open"
+    assert msg["payload"] == {"Value": False, "ValueIDKey": 562950578995224}
-    assert state.attributes[ATTR_CURRENT_POSITION] == 100
 
     # Test closing
     await hass.services.async_call(
@@ -47,22 +70,43 @@ async def test_cover(hass, cover_data, sent_messages, cover_msg):
         {"entity_id": "cover.roller_shutter_3_instance_1_level"},
         blocking=True,
     )
-    assert len(sent_messages) == 2
+    assert len(sent_messages) == 5
-    msg = sent_messages[1]
+    msg = sent_messages[4]
     assert msg["topic"] == "OpenZWave/1/command/setvalue/"
-    assert msg["payload"] == {"Value": 0, "ValueIDKey": 625573905}
+    assert msg["payload"] == {"Value": True, "ValueIDKey": 562950578995224}
 
-    # Test setting position
+    # Test stopping after closing
     await hass.services.async_call(
         "cover",
-        "set_cover_position",
+        "stop_cover",
-        {"entity_id": "cover.roller_shutter_3_instance_1_level", "position": 50},
+        {"entity_id": "cover.roller_shutter_3_instance_1_level"},
         blocking=True,
     )
-    assert len(sent_messages) == 3
+    assert len(sent_messages) == 7
-    msg = sent_messages[2]
+    msg = sent_messages[5]
     assert msg["topic"] == "OpenZWave/1/command/setvalue/"
-    assert msg["payload"] == {"Value": 50, "ValueIDKey": 625573905}
+    assert msg["payload"] == {"Value": False, "ValueIDKey": 281475602284568}
+
+    msg = sent_messages[6]
+    assert msg["topic"] == "OpenZWave/1/command/setvalue/"
+    assert msg["payload"] == {"Value": False, "ValueIDKey": 562950578995224}
+
+    # Test stopping after no open/close
+    await hass.services.async_call(
+        "cover",
+        "stop_cover",
+        {"entity_id": "cover.roller_shutter_3_instance_1_level"},
+        blocking=True,
+    )
+    # both stop open/close messages sent
+    assert len(sent_messages) == 9
+    msg = sent_messages[7]
+    assert msg["topic"] == "OpenZWave/1/command/setvalue/"
+    assert msg["payload"] == {"Value": False, "ValueIDKey": 281475602284568}
+
+    msg = sent_messages[8]
+    assert msg["topic"] == "OpenZWave/1/command/setvalue/"
+    assert msg["payload"] == {"Value": False, "ValueIDKey": 562950578995224}
 
     # Test converting position to zwave range for position > 0
     await hass.services.async_call(
@@ -71,8 +115,8 @@ async def test_cover(hass, cover_data, sent_messages, cover_msg):
         {"entity_id": "cover.roller_shutter_3_instance_1_level", "position": 100},
         blocking=True,
     )
-    assert len(sent_messages) == 4
+    assert len(sent_messages) == 10
-    msg = sent_messages[3]
+    msg = sent_messages[9]
     assert msg["topic"] == "OpenZWave/1/command/setvalue/"
     assert msg["payload"] == {"Value": 99, "ValueIDKey": 625573905}
 
@@ -83,8 +127,8 @@ async def test_cover(hass, cover_data, sent_messages, cover_msg):
         {"entity_id": "cover.roller_shutter_3_instance_1_level", "position": 0},
         blocking=True,
     )
-    assert len(sent_messages) == 5
+    assert len(sent_messages) == 11
-    msg = sent_messages[4]
+    msg = sent_messages[10]
     assert msg["topic"] == "OpenZWave/1/command/setvalue/"
     assert msg["payload"] == {"Value": 0, "ValueIDKey": 625573905}