From fecfd3879a33ea34bbab17ab215437379954f914 Mon Sep 17 00:00:00 2001 From: mundschenk_at Date: Mon, 22 Jan 2024 22:27:36 +0100 Subject: [PATCH] Republish correctly on scene changes --- lib/eventBus.ts | 6 +++--- lib/extension/homeassistant.ts | 25 +++++++++---------------- lib/extension/publish.ts | 2 +- lib/types/types.d.ts | 1 + test/homeassistant.test.js | 34 ++++++++++++++++++++++++---------- 5 files changed, 38 insertions(+), 30 deletions(-) diff --git a/lib/eventBus.ts b/lib/eventBus.ts index e36996732d..71716d9b30 100644 --- 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 index 119a68f73f..1498d8ceff 100644 --- a/lib/extension/homeassistant.ts +++ b/lib/extension/homeassistant.ts @@ -1667,20 +1667,15 @@ export default class HomeAssistant extends Extension { this.discover(data.device); } - @bind async onScenesChanged(): Promise { - // 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(); + @bind async onScenesChanged(data: eventdata.ScenesChanged): Promise { + // 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 @@ -1688,11 +1683,9 @@ export default class HomeAssistant extends Extension { 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 index bc3338953d..872e21957b 100644 --- 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 index c5b2e5ba32..b7982d77a5 100644 --- a/lib/types/types.d.ts +++ b/lib/types/types.d.ts @@ -107,6 +107,7 @@ declare global { data: KeyValue | Array; 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 index ffc9dca19e..72f9e8b0c5 100644 --- 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 () => {