Don't rely on any initial state data from group (#133)

* Don't rely on any initial state data from group

* Fix typing

pydeconz/gateway.py

@@ -185,9 +185,6 @@ def update_group_color(self, lights: list) -> None:
         state of the lights in the group. This method updates the color
         properties of the group to the current color of the lights in the
         group.
-
-        For groups where the lights have different colors the group color will
-        only reflect the color of the latest changed light in the group.
         """
         for group in self.groups.values():  # type: ignore
             # Skip group if there are no common light ids.
@@ -198,11 +195,13 @@ def update_group_color(self, lights: list) -> None:
             if len(light_ids := lights) > 1:
                 light_ids = group.lights
 
+            first = True
             for light_id in light_ids:
                 light = self.lights[light_id]  # type: ignore
 
                 if light.ZHATYPE == Light.ZHATYPE and light.reachable:
-                    group.update_color_state(light)
+                    group.update_color_state(light, update_all_attributes=first)
+                    first = False
 
     def update_scenes(self) -> None:
         """Update scenes to hold all known scenes from existing groups."""

pydeconz/group.py

@@ -14,6 +14,16 @@
 RESOURCE_TYPE_SCENE = "scenes"
 URL = "/groups"
 
+group_to_light_attributes = {
+    "bri": "brightness",
+    "ct": "ct",
+    "hue": "hue",
+    "sat": "sat",
+    "xy": "xy",
+    "colormode": "colormode",
+    "effect": "effect",
+}
+
 
 class Groups(APIItems):
     """Represent deCONZ groups."""
@@ -191,24 +201,24 @@ def multideviceids(self) -> Optional[list]:
         """
         return self.raw.get("multideviceids")
 
-    def update_color_state(self, light: Light) -> None:
-        """Sync color state with light."""
-        data: Dict[str, Union[float, int, str, tuple]] = {}
-
-        if light.brightness is not None:
-            data["bri"] = light.brightness
-        if light.hue is not None:
-            data["hue"] = light.hue
-        if light.sat is not None:
-            data["sat"] = light.sat
-        if light.ct is not None:
-            data["ct"] = light.ct
-        if light.xy is not None:
-            data["xy"] = light.xy
-        if light.colormode is not None:
-            data["colormode"] = light.colormode
-        if light.effect is not None:
-            data["effect"] = light.effect
+    def update_color_state(self, light: Light, update_all_attributes=False) -> None:
+        """Sync color state with light.
+
+          update_all_attributes is used to control whether or not to
+        write light attributes with the value None to the group.
+        This is used to not keep any bad values from the group.
+        """
+        data: Dict[str, Union[float, int, str, tuple, None]] = {}
+
+        for group_key, light_attribute_key in group_to_light_attributes.items():
+            light_attribute = getattr(light, light_attribute_key)
+
+            if light_attribute is not None:
+                data[group_key] = light_attribute
+                continue
+
+            if update_all_attributes:
+                data[group_key] = None if group_key != "xy" else (None, None)
 
         self.update({"action": data})
 

tests/test_gateway.py

@@ -3,7 +3,7 @@
 pytest --cov-report term-missing --cov=pydeconz.gateway tests/test_gateway.py
 """
 
-from unittest.mock import AsyncMock, Mock, patch
+from unittest.mock import Mock, patch
 import pytest
 
 from pydeconz import (
@@ -394,7 +394,60 @@ async def test_event_handler(mock_aioresponse):
     await session.session.close()
 
 
-async def test_update_group_color(mock_aioresponse):
+@pytest.mark.parametrize(
+    "light_ids,expected_group_state",
+    [
+        (
+            ["l1", "l2", "l3", "l4"],
+            {
+                "brightness": 3,
+                "ct": 2,
+                "hue": 1,
+                "sat": 1,
+                "xy": (0.1, 0.1),
+                "colormode": "ct",
+                "effect": None,
+            },
+        ),
+        (
+            ["l1"],
+            {
+                "brightness": 1,
+                "ct": 1,
+                "hue": 1,
+                "sat": 1,
+                "xy": (0.1, 0.1),
+                "colormode": "xy",
+                "effect": None,
+            },
+        ),
+        (
+            ["l2"],
+            {
+                "brightness": 2,
+                "ct": 2,
+                "hue": None,
+                "sat": None,
+                "xy": None,
+                "colormode": "ct",
+                "effect": None,
+            },
+        ),
+        (
+            ["l3"],
+            {
+                "brightness": 3,
+                "ct": None,
+                "hue": None,
+                "sat": None,
+                "xy": None,
+                "colormode": None,
+                "effect": None,
+            },
+        ),
+    ],
+)
+async def test_update_group_color(mock_aioresponse, light_ids, expected_group_state):
     """Test update_group_color works as expected."""
     session = DeconzSession(aiohttp.ClientSession(), HOST, PORT, API_KEY)
     init_response = {
@@ -410,7 +463,7 @@ async def test_update_group_color(mock_aioresponse):
                     "colormode": "hs",
                 },
                 "id": "gid",
-                "lights": ["l1", "l2", "l3", "l4"],
+                "lights": light_ids,
                 "scenes": [],
             }
         },
@@ -473,11 +526,12 @@ async def test_update_group_color(mock_aioresponse):
 
     await session.initialize()
 
-    assert session.groups["g1"].brightness == 3
-    assert session.groups["g1"].hue == 1
-    assert session.groups["g1"].sat == 1
-    assert session.groups["g1"].xy == (0.1, 0.1)
-    assert session.groups["g1"].colormode == "ct"
-    assert session.groups["g1"].ct == 2
+    assert session.groups["g1"].brightness == expected_group_state["brightness"]
+    assert session.groups["g1"].ct == expected_group_state["ct"]
+    assert session.groups["g1"].hue == expected_group_state["hue"]
+    assert session.groups["g1"].sat == expected_group_state["sat"]
+    assert session.groups["g1"].xy == expected_group_state["xy"]
+    assert session.groups["g1"].colormode == expected_group_state["colormode"]
+    assert session.groups["g1"].effect == expected_group_state["effect"]
 
     await session.session.close()

tests/test_groups.py

@@ -4,7 +4,10 @@
 """
 from unittest.mock import AsyncMock, Mock
 
+import pytest
+
 from pydeconz.group import Groups
+from pydeconz.light import Light
 
 
 async def test_create_group():
@@ -107,3 +110,90 @@ async def test_create_group():
 
     assert scene.name == "coldlight"
     assert scene.full_name == "Hall coldlight"
+
+
+@pytest.mark.parametrize(
+    "light_state,update_all,expected_group_state",
+    [
+        (
+            {
+                "bri": 1,
+                "ct": 1,
+                "hue": 1,
+                "sat": 1,
+                "xy": (0.1, 0.1),
+                "colormode": "xy",
+                "reachable": True,
+            },
+            False,
+            {
+                "brightness": 1,
+                "ct": 1,
+                "hue": 1,
+                "sat": 1,
+                "xy": (0.1, 0.1),
+                "colormode": "xy",
+                "effect": "none",
+            },
+        ),
+        (
+            {
+                "bri": 1,
+                "ct": 1,
+                "colormode": "ct",
+                "reachable": True,
+            },
+            True,
+            {
+                "brightness": 1,
+                "ct": 1,
+                "hue": None,
+                "sat": None,
+                "xy": None,
+                "colormode": "ct",
+                "effect": None,
+            },
+        ),
+    ],
+)
+async def test_update_color_state(light_state, update_all, expected_group_state):
+    """Verify that groups works."""
+    groups = Groups(
+        {
+            "0": {
+                "action": {
+                    "bri": 132,
+                    "colormode": "hs",
+                    "ct": 0,
+                    "effect": "none",
+                    "hue": 0,
+                    "on": False,
+                    "sat": 127,
+                    "scene": None,
+                    "xy": [0, 0],
+                },
+                "devicemembership": [],
+                "etag": "e31c23b3bd9ece918f23ee17ef430304",
+                "id": "11",
+                "lights": ["14", "15", "12"],
+                "name": "Hall",
+                "scenes": [{"id": "1", "name": "warmlight"}],
+                "state": {"all_on": False, "any_on": True},
+                "type": "LightGroup",
+            }
+        },
+        AsyncMock(),
+    )
+    group = groups["0"]
+
+    light = Light("0", {"type": "light", "state": light_state}, AsyncMock())
+
+    group.update_color_state(light, update_all_attributes=update_all)
+
+    assert group.brightness == expected_group_state["brightness"]
+    assert group.ct == expected_group_state["ct"]
+    assert group.hue == expected_group_state["hue"]
+    assert group.sat == expected_group_state["sat"]
+    assert group.xy == expected_group_state["xy"]
+    assert group.colormode == expected_group_state["colormode"]
+    assert group.effect == expected_group_state["effect"]