Repository: XKNX/xknx (python)

Issue:
 cover.set_cover_position not stopping with no position_address
**Description of problem:**

Dear xknx team,

covers with **no position_address** do not longer stop when moving the slider in the ha frontpanel to a position e.g. 30%. I attached my according HA config as well as some debug log from HA. This was working in precious versions of Home Assistant for me ( version 0.111).

The **service cover.set_cover_position ist not stopping at this value**. The services cover.close_cover;  service cover.open_cover;  cover.stop_cover are working currently.

Maybe a bug to the reworked travelcalculator / cover.py? It seems to me, that there is no stop movement send to actor when the calculated position is reached.

Remark: My covers do not have a positional feedback. But this applies not only to my knx config, this reproducible to other knx hardware setup with HA as well (a friend of mine reproduces this bug with his knx covers (which have a position_address). When deleting the position address in his config, this bug occurs as well. It would be great, to have this feature back / working again! Let me know, if i can assist you reproducing this bug. 

All the best 
Jan


- [x ] using Home-Assistant knx plugin

**Version information:**
- xknx / Home-Assistant release with the issue:  0.116.4
- last working xknx / Home-Assistant release (if known): 0.111


**KNX installation:**
<!--
- Manufacturer and model of relevant actors, sensors or interfaces.

-->

**Problem-relevant `xknx.yaml` or `configuration.yaml` entries (fill out even if it seems unimportant):**
[LOG_ha.txt](https://github.com/XKNX/xknx/files/5434327/LOG_ha.txt)
[knx_cover.yaml.txt](https://github.com/XKNX/xknx/files/5434337/knx_cover.yaml.txt)
[knx.yaml.txt](https://github.com/XKNX/xknx/files/5434338/knx.yaml.txt)
![Log](https://user-images.githubusercontent.com/19536666/97102778-98466880-16a8-11eb-93fd-f1ca839db2ed.JPG)


Pull request:
Cover: Fix auto-stopping without position state
<!--
  You are awesome! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template!.
-->
## Description
<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
-->

Fixes auto-stopping covers without position state. Due to processing every (outgoing) telegram in every device we did overwrite the target position with the end position. 

Eg. when we want from closed to 50% an UP-Telegram is sent to the bus (because we have no position address) and when reaching 50% a stop() shall be called. When processing this (outgoing) UP-Telegram we were treating it like any UP so target position became 0%.

Now we check if the cover is already moving in the received direction and ignore the telegram if it is. 
This implies that also signals from the bus are ignored when the cover is currently travelling in the same direction, to a distinct position from xknx - which is a very rare scenario imho.

Fixes #481 

## Type of change
<!--
Please tick the applicable options.
NOTE: Ticking multiple options most likely indicates
that your change is to big and it is suggested to split it into several smaller PRs.
-->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update

## Checklist:

- [x] I have performed a self-review of my own code
- [x] The documentation has been adjusted accordingly
- [x] The changes generate no new warnings
- [x] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog
- [ ] The Homeassistant plugin has been adjusted in case of new config options


Patch:
diff --git a/changelog.md b/changelog.md
index b3ef0a15..5ed77445 100644
--- a/changelog.md
+++ b/changelog.md
@@ -12,6 +12,7 @@
 ### Bugfixes
 
 - HA Switch entity: keep state without state_address
+- Cover: fix `set_position` without writable position / auto_stop_if_necessary
 
 ## 0.15.2 Winter is coming
 
diff --git a/xknx/devices/cover.py b/xknx/devices/cover.py
index 4ff53c1a..7596c1e5 100644
--- a/xknx/devices/cover.py
+++ b/xknx/devices/cover.py
@@ -270,13 +270,20 @@ class Cover(Device):
 
     async def process_group_write(self, telegram):
         """Process incoming and outgoing GROUP WRITE telegram."""
+        # call after_update to account for travelcalculator changes
         if await self.updown.process(telegram):
-            if self.updown.value == RemoteValueUpDown.Direction.UP:
+            if (
+                not self.is_opening()
+                and self.updown.value == RemoteValueUpDown.Direction.UP
+            ):
                 self.travelcalculator.start_travel_up()
-            else:
+                await self.after_update()
+            elif (
+                not self.is_closing()
+                and self.updown.value == RemoteValueUpDown.Direction.DOWN
+            ):
                 self.travelcalculator.start_travel_down()
-            # call after_update to account for travelcalculator changes
-            await self.after_update()
+                await self.after_update()
         # stop from bus
         if await self.stop_.process(telegram) or await self.step.process(telegram):
             if self.is_traveling():


Test patch:
diff --git a/test/devices_tests/cover_test.py b/test/devices_tests/cover_test.py
index 16e4547c..0865454c 100644
--- a/test/devices_tests/cover_test.py
+++ b/test/devices_tests/cover_test.py
@@ -397,6 +397,10 @@ class TestCover(unittest.TestCase):
             telegram, Telegram(GroupAddress("1/2/1"), payload=DPTBinary(0))
         )
         self.assertEqual(cover.travelcalculator.travel_to_position, 50)
+        self.assertTrue(cover.is_opening())
+        # process the outgoing telegram to make sure it doesn't overwrite the target position
+        self.loop.run_until_complete(cover.process(telegram))
+        self.assertEqual(cover.travelcalculator.travel_to_position, 50)
 
     def test_position_without_position_address_down(self):
         """Test moving cover down - with no absolute positioning supported."""
@@ -416,6 +420,10 @@ class TestCover(unittest.TestCase):
             telegram, Telegram(GroupAddress("1/2/1"), payload=DPTBinary(1))
         )
         self.assertEqual(cover.travelcalculator.travel_to_position, 80)
+        self.assertTrue(cover.is_closing())
+        # process the outgoing telegram to make sure it doesn't overwrite the target position
+        self.loop.run_until_complete(cover.process(telegram))
+        self.assertEqual(cover.travelcalculator.travel_to_position, 80)
 
     def test_position_without_position_address_uninitialized_up(self):
         """Test moving uninitialized cover to absolute position - with no absolute positioning supported."""