fix: Republish correctly on scene changes to Home Assistant (#20952)

Koenkk · Jan 23, 2024 · 6372d84 · 6372d84
1 parent 45579dc
commit 6372d84
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 30 deletions.
diff --git a/lib/eventBus.ts b/lib/eventBus.ts
@@ -146,10 +146,10 @@ export default class EventBus {
         this.on('devicesChanged', callback, key);
     }
 
-    public emitScenesChanged(): void {
-        this.emitter.emit('scenesChanged');
+    public emitScenesChanged(data: eventdata.ScenesChanged): void {
+        this.emitter.emit('scenesChanged', data);
     }
-    public onScenesChanged(key: ListenerKey, callback: () => void): void {
+    public onScenesChanged(key: ListenerKey, callback: (data: eventdata.ScenesChanged) => void): void {
         this.on('scenesChanged', callback, key);
     }
 

diff --git a/lib/extension/homeassistant.ts b/lib/extension/homeassistant.ts
@@ -1667,32 +1667,25 @@ export default class HomeAssistant extends Extension {
         this.discover(data.device);
     }
 
-    @bind async onScenesChanged(): Promise<void> {
-        // Re-trigger MQTT discovery of all devices and groups, similar to bridge.ts
-        const entities = [...this.zigbee.devices(), ...this.zigbee.groups()];
-        const clearedEntities = new Set<Device|Group>();
+    @bind async onScenesChanged(data: eventdata.ScenesChanged): Promise<void> {
+        // Re-trigger MQTT discovery of changed devices and groups, similar to bridge.ts
 
         // First, clear existing scene discovery topics
-        entities.forEach((entity) => {
-            logger.debug(`Clearing Home Assistant scene discovery topics for '${entity.name}'`);
-            this.discovered[this.getDiscoverKey(entity)]?.topics.forEach((topic) => {
-                if (topic.startsWith('scene')) {
-                    this.mqtt.publish(topic, null, {retain: true, qos: 1}, this.discoveryTopic, false, false);
-                    clearedEntities.add(entity);
-                }
-            });
+        logger.debug(`Clearing Home Assistant scene discovery topics for '${data.entity.name}'`);
+        this.discovered[this.getDiscoverKey(data.entity)]?.topics.forEach((topic) => {
+            if (topic.startsWith('scene')) {
+                this.mqtt.publish(topic, null, {retain: true, qos: 1}, this.discoveryTopic, false, false);
+            }
         });
 
         // Make sure Home Assistant deletes the old entity first otherwise another one (_2) is created
         // https://github.com/Koenkk/zigbee2mqtt/issues/12610
         logger.debug(`Finished clearing scene discovery topics, waiting for Home Assistant.`);
         await utils.sleep(2);
 
-        // Re-discover all entities (including their new scenes).
+        // Re-discover entity (including any new scenes).
         logger.debug(`Re-discovering entities with their scenes.`);
-        clearedEntities.forEach((entity) => {
-            this.discover(entity, true);
-        });
+        this.discover(data.entity, true);
     }
 
     private getDevicePayload(entity: Device | Group | Bridge): KeyValue {

diff --git a/lib/extension/publish.ts b/lib/extension/publish.ts
@@ -298,7 +298,7 @@ export default class Publish extends Extension {
         const scenesChanged = Object.values(usedConverters)
             .some((cl) => cl.some((c) => c.key.some((k) => sceneConverterKeys.includes(k))));
         if (scenesChanged) {
-            this.eventBus.emitScenesChanged();
+            this.eventBus.emitScenesChanged({entity: re});
         }
     }
 }
diff --git a/lib/types/types.d.ts b/lib/types/types.d.ts
@@ -107,6 +107,7 @@ declare global {
             data: KeyValue | Array<string | number>;
             meta: {zclTransactionSequenceNumber?: number; manufacturerCode?: number; frameControl?: ZHFrameControl;};
         };
+        type ScenesChanged = { entity: Device | Group };
     }
 
     // Settings

diff --git a/test/homeassistant.test.js b/test/homeassistant.test.js
@@ -2156,8 +2156,12 @@ describe('HomeAssistant extension', () => {
     });
 
     it('Should rediscover scenes when a scene is changed', async () => {
+
+        // Device/endpoint scenes.
+        const device = controller.zigbee.resolveEntity(zigbeeHerdsman.devices.bulb_color_2);
+
         MQTT.publish.mockClear();
-        controller.eventBus.emitScenesChanged();
+        controller.eventBus.emitScenesChanged({entity: device});
         await flushPromises();
 
         // Discovery messages for scenes have been purged.
@@ -2167,12 +2171,6 @@ describe('HomeAssistant extension', () => {
             {retain: true, qos: 1},
             expect.any(Function),
         );
-        expect(MQTT.publish).toHaveBeenCalledWith(
-            `homeassistant/scene/1221051039810110150109113116116_9/scene_4/config`,
-            null,
-            {retain: true, qos: 1},
-            expect.any(Function),
-        );
         jest.runOnlyPendingTimers();
         await flushPromises();
 
@@ -2200,6 +2198,24 @@ describe('HomeAssistant extension', () => {
             {retain: true, qos: 1},
             expect.any(Function),
         );
+        expect(MQTT.publish).toHaveBeenCalledTimes( 12 );
+
+        // Group scenes.
+        const group = controller.zigbee.resolveEntity('ha_discovery_group');
+
+        MQTT.publish.mockClear();
+        controller.eventBus.emitScenesChanged({entity: group});
+        await flushPromises();
+
+        // Discovery messages for scenes have been purged.
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            `homeassistant/scene/1221051039810110150109113116116_9/scene_4/config`,
+            null,
+            {retain: true, qos: 1},
+            expect.any(Function),
+        );
+        jest.runOnlyPendingTimers();
+        await flushPromises();
 
         payload = {
             'name': 'Scene 4',
@@ -2225,9 +2241,7 @@ describe('HomeAssistant extension', () => {
             {retain: true, qos: 1},
             expect.any(Function),
         );
-
-        // Only discovery entries for entities with scenes need to be republished.
-        expect(MQTT.publish).toHaveBeenCalledTimes( 16 );
+        expect(MQTT.publish).toHaveBeenCalledTimes( 6 );
     });
 
     it('Should not clear bridge entities unnecessarily', async () => {