Repository: XKNX/xknx
Pull request #800 (2021-11-12 05:56:47)

Debounce individual colors callback
<!--
  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.
-->
Debounce individual colors callback

Fixes # (issue)

## 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.
-->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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
- [ ] 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


codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/800?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#800](https://codecov.io/gh/XKNX/xknx/pull/800?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (3802eac) into [main](https://codecov.io/gh/XKNX/xknx/commit/981751f7a321dce743c23fe8b205962cb94a98a7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (981751f) will **decrease** coverage by `0.01%`.
> The diff coverage is `92.30%`.

[![Impacted file tree graph](https://codecov.io/gh/XKNX/xknx/pull/800/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/800?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@            Coverage Diff             @@
##             main     #800      +/-   ##
==========================================
- Coverage   94.75%   94.74%   -0.02%     
==========================================
  Files         109      109              
  Lines        6812     6837      +25     
==========================================
+ Hits         6455     6478      +23     
- Misses        357      359       +2     
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/800?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/devices/light.py](https://codecov.io/gh/XKNX/xknx/pull/800/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2xpZ2h0LnB5) | `98.83% <92.30%> (-0.74%)` | :arrow_down: |


Patch:
diff --git a/changelog.md b/changelog.md
index 4d463709..ee25294e 100644
--- a/changelog.md
+++ b/changelog.md
@@ -5,6 +5,7 @@
 ### Devices
 
 - Light: Only send to global switch or brightness address if individual colors are also configured
+- Light: Debounce individual colors callback to mitigate color flicker in visualizations
 
 ## 0.18.12 Add always callback to NumericValue and RawValue 2021-11-01
 
diff --git a/xknx/devices/light.py b/xknx/devices/light.py
index 0e46773f..0fa3664c 100644
--- a/xknx/devices/light.py
+++ b/xknx/devices/light.py
@@ -12,7 +12,9 @@ It provides functionality for
 """
 from __future__ import annotations
 
+import asyncio
 from enum import Enum
+from itertools import chain
 import logging
 from typing import TYPE_CHECKING, Any, Awaitable, Callable, Iterator, Tuple, cast
 
@@ -114,6 +116,7 @@ class _SwitchAndBrightness:
 class Light(Device):
     """Class for managing a light."""
 
+    DEBOUNCE_TIMEOUT = 0.2
     DEFAULT_MIN_KELVIN = 2700  # 370 mireds
     DEFAULT_MAX_KELVIN = 6000  # 166 mireds
 
@@ -266,7 +269,7 @@ class Light(Device):
             group_address_brightness_red,
             group_address_brightness_red_state,
             sync_state=sync_state,
-            after_update_cb=self.after_update,
+            after_update_cb=self._individual_color_callback_debounce,
         )
 
         self.green = _SwitchAndBrightness(
@@ -278,7 +281,7 @@ class Light(Device):
             group_address_brightness_green,
             group_address_brightness_green_state,
             sync_state=sync_state,
-            after_update_cb=self.after_update,
+            after_update_cb=self._individual_color_callback_debounce,
         )
 
         self.blue = _SwitchAndBrightness(
@@ -290,7 +293,7 @@ class Light(Device):
             group_address_brightness_blue,
             group_address_brightness_blue_state,
             sync_state=sync_state,
-            after_update_cb=self.after_update,
+            after_update_cb=self._individual_color_callback_debounce,
         )
 
         self.white = _SwitchAndBrightness(
@@ -302,14 +305,26 @@ class Light(Device):
             group_address_brightness_white,
             group_address_brightness_white_state,
             sync_state=sync_state,
-            after_update_cb=self.after_update,
+            after_update_cb=self._individual_color_callback_debounce,
         )
 
         self.min_kelvin = min_kelvin
         self.max_kelvin = max_kelvin
+        self._individual_color_debounce_task_name = (
+            f"{id(self)}_individual_color_debounce"
+        )
+        self._individual_color_debounce_telegram_counter: int
+        self._reset_individual_color_debounce_telegrams()
 
     def _iter_remote_values(self) -> Iterator[RemoteValue[Any, Any]]:
         """Iterate the devices RemoteValue classes."""
+        return chain(
+            self._iter_instant_remote_values(),
+            self._iter_debounce_remote_values(),
+        )
+
+    def _iter_instant_remote_values(self) -> Iterator[RemoteValue[Any, Any]]:
+        """Iterate the devices RemoteValue classes calling after_update_cb immediately."""
         yield self.switch
         yield self.brightness
         yield self.color
@@ -319,6 +334,9 @@ class Light(Device):
         yield self.xyy_color
         yield self.tunable_white
         yield self.color_temperature
+
+    def _iter_debounce_remote_values(self) -> Iterator[RemoteValue[Any, Any]]:
+        """Iterate the devices RemoteValue classes debouncing after_update_cb."""
         for color in self._iter_individual_colors():
             yield color.switch
             yield color.brightness
@@ -327,6 +345,36 @@ class Light(Device):
         """Iterate the devices individual colors."""
         yield from (self.red, self.green, self.blue, self.white)
 
+    def _reset_individual_color_debounce_telegrams(self) -> None:
+        """Reset individual color debounce telegram counter."""
+        self._individual_color_debounce_telegram_counter = sum(
+            (
+                self.red.switch.initialized or self.red.brightness.initialized,
+                self.green.switch.initialized or self.green.brightness.initialized,
+                self.blue.switch.initialized or self.blue.brightness.initialized,
+                self.white.switch.initialized or self.white.brightness.initialized,
+            )
+        )
+
+    async def _individual_color_callback_debounce(self) -> None:
+        """Run callback after all individual colors were updated or timeout passed."""
+
+        async def debouncer() -> None:
+            await asyncio.sleep(Light.DEBOUNCE_TIMEOUT)
+            self._reset_individual_color_debounce_telegrams()
+            await asyncio.shield(self.after_update())
+
+        self._individual_color_debounce_telegram_counter -= 1
+        if self._individual_color_debounce_telegram_counter > 0:
+            # task registry cancels existing task
+            self.xknx.task_registry.register(
+                self._individual_color_debounce_task_name, debouncer()
+            ).start()
+            return
+        self.xknx.task_registry.unregister(self._individual_color_debounce_task_name)
+        self._reset_individual_color_debounce_telegrams()
+        await self.after_update()
+
     @property
     def supports_brightness(self) -> bool:
         """Return if light supports brightness."""
@@ -549,8 +597,10 @@ class Light(Device):
 
     async def process_group_write(self, telegram: "Telegram") -> None:
         """Process incoming and outgoing GROUP WRITE telegram."""
-        for remote_value in self._iter_remote_values():
+        for remote_value in self._iter_instant_remote_values():
             await remote_value.process(telegram)
+        for remote_value in self._iter_debounce_remote_values():
+            await remote_value.process(telegram, always_callback=True)
 
     def __str__(self) -> str:
         """Return object as readable string."""


Test patch:
diff --git a/test/devices_tests/light_test.py b/test/devices_tests/light_test.py
index bd9fad9a..f1fa7b9b 100644
--- a/test/devices_tests/light_test.py
+++ b/test/devices_tests/light_test.py
@@ -1414,6 +1414,98 @@ class TestLight:
         await light.process(telegram)
         assert light.current_color == (None, 42)
 
+    async def test_process_individual_color_debouncer(self, time_travel):
+        """Test the debouncer for individual color lights."""
+        xknx = XKNX()
+        rgb_callback = AsyncMock()
+        rgbw_callback = AsyncMock()
+
+        rgb_light = Light(
+            xknx,
+            "TestRGBLight",
+            group_address_switch_red="1/1/1",
+            group_address_switch_red_state="1/1/2",
+            group_address_brightness_red="1/1/3",
+            group_address_brightness_red_state="1/1/4",
+            group_address_switch_green="1/1/5",
+            group_address_switch_green_state="1/1/6",
+            group_address_brightness_green="1/1/7",
+            group_address_brightness_green_state="1/1/8",
+            group_address_switch_blue="1/1/9",
+            group_address_switch_blue_state="1/1/10",
+            group_address_brightness_blue="1/1/11",
+            group_address_brightness_blue_state="1/1/12",
+            device_updated_cb=rgb_callback,
+        )
+        rgbw_light = Light(
+            xknx,
+            "TestRGBWLight",
+            group_address_switch="1/1/0",
+            group_address_brightness_red="1/1/3",
+            group_address_brightness_red_state="1/1/4",
+            group_address_brightness_green="1/1/7",
+            group_address_brightness_green_state="1/1/8",
+            group_address_brightness_blue="1/1/11",
+            group_address_brightness_blue_state="1/1/12",
+            group_address_brightness_white="1/1/15",
+            group_address_brightness_white_state="1/1/16",
+            device_updated_cb=rgbw_callback,
+        )
+
+        assert rgb_light.current_color == (None, None)
+        assert rgbw_light.current_color == (None, None)
+
+        red = Telegram(
+            destination_address=GroupAddress("1/1/4"),
+            payload=GroupValueWrite(DPTArray(42)),
+        )
+        green = Telegram(
+            destination_address=GroupAddress("1/1/8"),
+            payload=GroupValueWrite(DPTArray(43)),
+        )
+        blue = Telegram(
+            destination_address=GroupAddress("1/1/12"),
+            payload=GroupValueWrite(DPTArray(44)),
+        )
+        white = Telegram(
+            destination_address=GroupAddress("1/1/16"),
+            payload=GroupValueWrite(DPTArray(50)),
+        )
+
+        await xknx.devices.process(red)
+        rgb_callback.assert_not_called()
+        rgbw_callback.assert_not_called()
+        await xknx.devices.process(green)
+        rgb_callback.assert_not_called()
+        rgbw_callback.assert_not_called()
+        await xknx.devices.process(blue)
+        rgb_callback.assert_called_once()
+        rgbw_callback.assert_not_called()
+        await xknx.devices.process(white)
+        rgbw_callback.assert_called_once()
+
+        assert rgb_light.current_color == ((42, 43, 44), None)
+        assert rgbw_light.current_color == ((42, 43, 44), 50)
+
+        rgb_callback.reset_mock()
+        rgbw_callback.reset_mock()
+        # second time with only 2 telegrams
+        await xknx.devices.process(red)
+        rgb_callback.assert_not_called()
+        rgbw_callback.assert_not_called()
+        await xknx.devices.process(green)
+        rgb_callback.assert_not_called()
+        rgbw_callback.assert_not_called()
+        await time_travel(Light.DEBOUNCE_TIMEOUT / 2)
+        rgb_callback.assert_not_called()
+        rgbw_callback.assert_not_called()
+        await time_travel(Light.DEBOUNCE_TIMEOUT / 2)
+        rgb_callback.assert_called_once()
+        rgbw_callback.assert_called_once()
+
+        assert rgb_light.current_color == ((42, 43, 44), None)
+        assert rgbw_light.current_color == ((42, 43, 44), 50)
+
     async def test_process_hs_color(self):
         """Test process / reading telegrams from telegram queue. Test if color is processed."""
         xknx = XKNX()