From 1acccc002a623f5251d7f51cb2828fb3fe7d26d3 Mon Sep 17 00:00:00 2001
From: Koen Kanters <koenkanters94@gmail.com>
Date: Wed, 17 Mar 2021 21:44:29 +0100
Subject: [PATCH] Only publish changed properties for groups and its members.
 #6718

---
 lib/extension/groups.js |  4 ++--
 lib/state.js            | 11 +++++++----
 test/group.test.js      | 28 ++++++++++++++++++++++++++++
 test/publish.test.js    |  4 ++--
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/lib/extension/groups.js b/lib/extension/groups.js
index fbc4b99bc..3012fd4bc 100644
--- a/lib/extension/groups.js
+++ b/lib/extension/groups.js
@@ -86,8 +86,8 @@ class Groups extends Extension {
         const payload = {};
 
         properties.forEach((prop) => {
-            if (data.to.hasOwnProperty(prop)) {
-                payload[prop] = data.to[prop];
+            if (data.changed.hasOwnProperty(prop)) {
+                payload[prop] = data.changed[prop];
             }
         });
 
diff --git a/lib/state.js b/lib/state.js
index 5524c3fa3..022df262e 100644
--- a/lib/state.js
+++ b/lib/state.js
@@ -77,18 +77,21 @@ class State {
 
     set(ID, state, reason=null) {
         const toState = objectAssignDeep.noMutate(state);
+        const fromState = this.state[ID];
+        const changed = {};
 
         for (const property of Object.keys(toState)) {
             if (dontCacheProperties.find((p) => property.match(p))) {
                 delete toState[property];
             }
+
+            if (!fromState || toState[property] !== fromState[property]) {
+                changed[property] = toState[property];
+            }
         }
 
-        const fromState = this.state[ID];
-
         this.state[ID] = toState;
-
-        this.eventBus.emit('stateChange', {ID, from: fromState, to: state, reason});
+        this.eventBus.emit('stateChange', {ID, from: fromState, to: state, reason, changed});
     }
 
     removeKey(ID, path) {
diff --git a/test/group.test.js b/test/group.test.js
index c122a5263..b9a988876 100644
--- a/test/group.test.js
+++ b/test/group.test.js
@@ -505,6 +505,34 @@ describe('Groups', () => {
         expect(MQTT.publish).toHaveBeenCalledWith("zigbee2mqtt/group_1", stringify({"state":"OFF"}), {"retain": false, qos: 0}, expect.any(Function));
     });
 
+    it('Should only update group state with changed properties', async () => {
+        const device_1 = zigbeeHerdsman.devices.bulb_color;
+        const device_2 = zigbeeHerdsman.devices.bulb;
+        const endpoint_1 = device_1.getEndpoint(1);
+        const endpoint_2 = device_2.getEndpoint(1);
+        const group = zigbeeHerdsman.groups.group_1;
+        group.members.push(endpoint_1);
+        group.members.push(endpoint_2);
+        settings.set(['groups'], {
+            '1': {friendly_name: 'group_1', devices: [device_1.ieeeAddr, device_2.ieeeAddr], retain: false}
+        });
+        await controller.start();
+        await flushPromises();
+        MQTT.publish.mockClear();
+
+        await MQTT.events.message('zigbee2mqtt/bulb_color/set', stringify({state: 'OFF', color_temp: 200}));
+        await MQTT.events.message('zigbee2mqtt/bulb/set', stringify({state: 'ON', color_temp: 250}));
+        await flushPromises();
+        MQTT.publish.mockClear();
+
+        await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({color_temp: 300}));
+        await flushPromises();
+        expect(MQTT.publish).toHaveBeenCalledTimes(3);
+        expect(MQTT.publish).toHaveBeenCalledWith("zigbee2mqtt/bulb_color", stringify({"color":{"x":0.415211980162654,"y":0.395434886759171},"color_temp":300,"state":"OFF"}), {"retain": false, qos: 0}, expect.any(Function));
+        expect(MQTT.publish).toHaveBeenCalledWith("zigbee2mqtt/bulb", stringify({"color":{"x":0.415211980162654,"y":0.395434886759171},"color_temp":300,"state":"ON"}), {"retain": true, qos: 0}, expect.any(Function));
+        expect(MQTT.publish).toHaveBeenCalledWith("zigbee2mqtt/group_1", stringify({"color":{"x":0.415211980162654,"y":0.395434886759171},"color_temp":300,"state":"ON"}), {"retain": false, qos: 0}, expect.any(Function));
+    });
+
     it('Should publish state change off even when missing current state', async () => {
         const device_1 = zigbeeHerdsman.devices.bulb_color;
         const device_2 = zigbeeHerdsman.devices.bulb;
diff --git a/test/publish.test.js b/test/publish.test.js
index f565123f8..c8902bab6 100644
--- a/test/publish.test.js
+++ b/test/publish.test.js
@@ -1377,12 +1377,12 @@ describe('Publish', () => {
         );
         expect(MQTT.publish).toHaveBeenNthCalledWith(4,
             'zigbee2mqtt/bulb_2',
-            stringify({"brightness":100,"color":{"x":0.408707336668894,"y":0.39239142575868},"color_temp":290,"state":"ON"}),
+            stringify({"brightness":100,"color":{"x":0.408707336668894,"y":0.39239142575868},"state":"ON"}),
             {retain: false, qos: 0}, expect.any(Function)
         );
         expect(MQTT.publish).toHaveBeenNthCalledWith(5,
             'zigbee2mqtt/group_with_tradfri',
-            stringify({"brightness":100,"color":{"x":0.408707336668894,"y":0.39239142575868},"color_temp":290,"state":"ON"}),
+            stringify({"brightness":100,"color":{"x":0.408707336668894,"y":0.39239142575868},"state":"ON"}),
             {retain: false, qos: 0}, expect.any(Function)
         );
     });