Implement #7354 by adding skip_disable_reporting to unbind group/remove

lib/extension/bind.js

@@ -127,6 +127,7 @@ class Bind extends Extension {
         let sourceKey = null;
         let targetKey = null;
         let clusters = null;
+        let skip_disable_reporting = false;
 
         if (this.legacyApi && topic.match(legacyTopicRegex)) {
             topic = topic.replace(`${settings.get().mqtt.base_topic}/bridge/`, '');
@@ -139,13 +140,14 @@ class Bind extends Extension {
             sourceKey = message.from;
             targetKey = message.to;
             clusters = message.clusters;
+            skip_disable_reporting = message.hasOwnProperty('skip_disable_reporting') ? message.skip_disable_reporting : false;
         }
 
-        return {type, sourceKey, targetKey, clusters};
+        return {type, sourceKey, targetKey, clusters, skip_disable_reporting};
     }
 
     async onMQTTMessage(topic, message) {
-        const {type, sourceKey, targetKey, clusters} = this.parseMQTTMessage(topic, message);
+        const {type, sourceKey, targetKey, clusters, skip_disable_reporting} = this.parseMQTTMessage(topic, message);
         if (!type) return null;
         message = utils.parseJSON(message, message);
 
@@ -255,7 +257,7 @@ class Bind extends Extension {
                 if (type === 'bind') {
                     await this.setupReporting(source.endpoint.binds.filter((b) =>
                         successfulClusters.includes(b.cluster.name) && b.target === bindTarget));
-                } else if (target.type !== 'group_number') {
+                } else if ((target.type !== 'group_number') && !skip_disable_reporting) {
                     await this.disableUnnecessaryReportings(bindTarget);
                 }
             }
@@ -281,7 +283,9 @@ class Bind extends Extension {
         if (data.action === 'add') {
             await this.setupReporting(bindsToGroup);
         } else { // action === remove/remove_all
-            await this.disableUnnecessaryReportings(data.endpoint);
+            if (!data.skip_disable_reporting) {
+                await this.disableUnnecessaryReportings(data.endpoint);
+            }
         }
     }
 

lib/extension/groups.js

@@ -169,6 +169,7 @@ class Groups extends Extension {
         let groupKey = null;
         let deviceKey = null;
         let triggeredViaLegacyApi = false;
+        let skip_disable_reporting = false;
 
         /* istanbul ignore else */
         const topicRegexMatch = topic.match(topicRegex);
@@ -224,6 +225,7 @@ class Groups extends Extension {
             type = topicRegexMatch[1];
             message = JSON.parse(message);
             deviceKey = message.device;
+            skip_disable_reporting = message.hasOwnProperty('skip_disable_reporting') ? message.skip_disable_reporting : false;
 
             if (type !== 'remove_all') {
                 groupKey = message.group;
@@ -243,14 +245,14 @@ class Groups extends Extension {
 
         return {
             resolvedEntityGroup, resolvedEntityDevice, type, hasEndpointName, error, groupKey, deviceKey,
-            triggeredViaLegacyApi,
+            triggeredViaLegacyApi, skip_disable_reporting,
         };
     }
 
     async onMQTTMessage(topic, message) {
         let {
             resolvedEntityGroup, resolvedEntityDevice, type, hasEndpointName, error, triggeredViaLegacyApi,
-            groupKey, deviceKey,
+            groupKey, deviceKey, skip_disable_reporting,
         } = this.parseMQTTMessage(topic, message);
         if (!type) return;
         message = utils.parseJSON(message, message);
@@ -339,7 +341,7 @@ class Groups extends Extension {
             logger.error(error);
         } else {
             this.eventBus.emit('groupMembersChanged',
-                {group: resolvedEntityGroup, action: type, endpoint: resolvedEntityDevice.endpoint});
+                {group: resolvedEntityGroup, action: type, endpoint: resolvedEntityDevice.endpoint, skip_disable_reporting: skip_disable_reporting});
         }
     }
 }

test/bind.test.js

@@ -229,6 +229,36 @@ describe('Bind', () => {
         );
     });
 
+    it('Should unbind from group with skip_disable_reporting=true', async () => {
+        const device = zigbeeHerdsman.devices.remote;
+        const target = zigbeeHerdsman.groups.group_1;
+        const target1Member = zigbeeHerdsman.devices.bulb.getEndpoint(1);
+        const endpoint = device.getEndpoint(1);
+        target.members.push(target1Member);
+        target1Member.configureReporting.mockClear();
+        mockClear(device);
+        MQTT.events.message('zigbee2mqtt/bridge/request/device/unbind', stringify({from: 'remote', to: 'group_1', skip_disable_reporting: true}));
+        await flushPromises();
+        expect(endpoint.unbind).toHaveBeenCalledTimes(3);
+        // with skip_disable_reporting set, we expect it to not reconfigure reporting
+        expect(target1Member.configureReporting).toHaveBeenCalledTimes(0);
+    });
+
+    it('Should unbind from group with skip_disable_reporting=false', async () => {
+        const device = zigbeeHerdsman.devices.remote;
+        const target = zigbeeHerdsman.groups.group_1;
+        const target1Member = zigbeeHerdsman.devices.bulb.getEndpoint(1);
+        const endpoint = device.getEndpoint(1);
+        target.members.push(target1Member);
+        target1Member.configureReporting.mockClear();
+        mockClear(device);
+        MQTT.events.message('zigbee2mqtt/bridge/request/device/unbind', stringify({from: 'remote', to: 'group_1', skip_disable_reporting: false}));
+        await flushPromises();
+        expect(endpoint.unbind).toHaveBeenCalledTimes(3);
+        // with skip_disable_reporting set, we expect it to reconfigure reporting
+        expect(target1Member.configureReporting).toHaveBeenCalledTimes(2);
+    });
+
     it('Should bind to group by number', async () => {
         const device = zigbeeHerdsman.devices.remote;
         const target = zigbeeHerdsman.groups.group_1;

test/group.test.js

@@ -718,6 +718,27 @@ describe('Groups', () => {
         );
     });
 
+    it('Remove from group via MQTT keeping device reporting', async () => {
+        const device = zigbeeHerdsman.devices.bulb_color;
+        const endpoint = device.getEndpoint(1);
+        const group = zigbeeHerdsman.groups.group_1;
+        group.members.push(endpoint);
+        settings.set(['groups'], {'1': {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}});
+        await controller.start();
+        await flushPromises();
+        MQTT.publish.mockClear();
+        MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1', device: 'bulb_color', skip_disable_reporting: true}));
+        await flushPromises();
+        expect(group.members).toStrictEqual([]);
+        expect(settings.getGroup('group_1').devices).toStrictEqual([]);
+        expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function));
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            'zigbee2mqtt/bridge/response/group/members/remove',
+            stringify({"data":{"device":"bulb_color","group":"group_1"},"status":"ok"}),
+            {retain: false, qos: 0}, expect.any(Function)
+        );
+    });
+
     it('Remove from group via MQTT when in zigbee but not in settings', async () => {
         const device = zigbeeHerdsman.devices.bulb_color;
         const endpoint = device.getEndpoint(1);