Repository: XKNX/xknx

Issue:
Cover update callback stops in some special case
Hello!

My intention is to use `device_updated_cb` callback for Cover to process some logic during its movement etc.. 
I faced the problem when callback stops working in some special case, for example:

Pretend the blind initial position is 0% and we need to move it to 90%
1) start moving the blind to some position `await cover.set_position(90)`
2) send Stop during movement `cover.stop()` 
3) then continue to move it to the previously given position `await cover.set_position(90)`
Here the `device_updated_cb` stops 
4) if then or instead to send the blind to any other position but except 90%, the callback gets back to work, for example, sending to any of 0-89 or 91-100% .

Not sure if I'm doing something not correctly.
Here is the reduced example:

`
async def cover_update_cb(cover: Cover):
    print('Something inside cover update callback')
    
cover: Cover = None

async def test():
    global cover
    await asyncio.sleep(1)
    await cover.set_position(90)
    await asyncio.sleep(4)
    print('STOP command')
    await cover.stop()
    await asyncio.sleep(2)
    print(f'Continue to move to 90')
    await cover.set_position(90) 
    #  here cover_update_cb stops. If instead set any different position from a position that the blind started moving from at the beginning (in our case 90) then callback will work. 
     
async def main():
    global cover
    xknx = XKNX(daemon_mode=True)     

    cover = Cover(
        xknx,
        'Cover',
        group_address_stop = "1/1/2",
        group_address_position = "1/1/3",
        group_address_position_state = "1/1/4",
        device_updated_cb=cover_update_cb
    )
    
    loop = asyncio.get_running_loop()
    loop.create_task(test())
        
    await xknx.start()
    await xknx.stop()    
    
asyncio.run(main())

`



 


Discussion:
mralessio: Don't judge me too much please. This is my first pull request :see_no_evil: Hope I did it right
farmio: Thank you very much! 👍

Could you add (or maybe adjust an existing) test for that? 
codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/1190?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#1190](https://codecov.io/gh/XKNX/xknx/pull/1190?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (732b18f) into [main](https://codecov.io/gh/XKNX/xknx/commit/ada1d85718d3406b4de69338ba3aa23119fb1556?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (ada1d85) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/XKNX/xknx/pull/1190/graphs/tree.svg?width=650&height=150&src=pr&token=irWbIygS84&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)](https://codecov.io/gh/XKNX/xknx/pull/1190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@           Coverage Diff           @@
##             main    #1190   +/-   ##
=======================================
  Coverage   96.45%   96.45%           
=======================================
  Files         143      143           
  Lines        8944     8944           
=======================================
  Hits         8627     8627           
  Misses        317      317           
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/1190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/devices/cover.py](https://codecov.io/gh/XKNX/xknx/pull/1190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2NvdmVyLnB5) | `99.46% <100.00%> (ø)` | |

farmio: Allright! Have a look here: https://github.com/XKNX/xknx/blob/88fc1ead4cdb10bdacee6824905540365588bf9d/test/devices_tests/cover_test.py#L810
farmio: I did add the test myself since I want to do a release today and would like to have this fix merged by then.

Thank you very much for your contribution! 🙂

Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 03b50890..5834d7d3 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -17,6 +17,7 @@ nav_order: 2
 
 - Parse Data Secure credentials form Keyring from non-IP-Secure interfaces.
 - Parse Data Secure credentials from Keyrings exported for specific interfaces.
+- Fix callback for Cover target position when called with same value consecutively.
 - Fix Windows TCP transport bug when using IP Secure Tunnelling.
 
 ### Protocol
diff --git a/xknx/devices/cover.py b/xknx/devices/cover.py
index 750913e1..489c1364 100644
--- a/xknx/devices/cover.py
+++ b/xknx/devices/cover.py
@@ -360,7 +360,7 @@ class Cover(Device):
                 await self._stop_position_update()
 
         await self.position_current.process(telegram, always_callback=True)
-        await self.position_target.process(telegram)
+        await self.position_target.process(telegram, always_callback=True)
         await self.angle.process(telegram)
         await self.locked.process(telegram)
 


Test patch:
diff --git a/test/devices_tests/cover_test.py b/test/devices_tests/cover_test.py
index 2617622e..9dbe67ad 100644
--- a/test/devices_tests/cover_test.py
+++ b/test/devices_tests/cover_test.py
@@ -830,6 +830,9 @@ class TestCover:
             ("1/2/1", DPTBinary(1), "long"),
             ("1/2/2", DPTBinary(1), "short"),
             ("1/2/4", DPTArray(42), "position"),
+            ("1/2/3", DPTBinary(1), "stop"),
+            # call position with same value again to make sure `always_callback` is set for target position
+            ("1/2/4", DPTArray(42), "position"),
             ("1/2/5", DPTArray(42), "position state"),
             ("1/2/6", DPTArray(42), "angle"),
             ("1/2/7", DPTArray(51), "angle state"),