fix: Optimize Home Assistant discovery (#22701)

* fix: Optimize Home Assistant discovery

* update

* u

* u

* u

* fix
This commit is contained in:
Koen Kanters
2024-05-21 20:53:37 +02:00
committed by GitHub
parent 30f6c8e60c
commit ef68cc328e
5 changed files with 169 additions and 81 deletions
+1
View File
@@ -152,6 +152,7 @@ export default class Frontend extends Extension {
}
@bind private onMQTTPublishMessage(data: eventdata.MQTTMessagePublished): void {
/* istanbul ignore else */
if (data.topic.startsWith(`${this.mqttBaseTopic}/`)) {
// Send topic without base_topic
const topic = data.topic.substring(this.mqttBaseTopic.length + 1);
+95 -74
View File
@@ -23,6 +23,13 @@ const sensorClick: DiscoveryEntry = {
},
};
interface Discovered {
mockProperties: Set<MockProperty>,
messages: {[s: string]: {payload: string, published: boolean}},
triggers: Set<string>,
discovered: boolean,
}
const ACCESS_STATE = 0b001;
const ACCESS_SET = 0b010;
const groupSupportedTypes = ['light', 'switch', 'lock', 'cover'];
@@ -106,10 +113,10 @@ class Bridge {
* This extensions handles integration with HomeAssistant
*/
export default class HomeAssistant extends Extension {
private discovered: {[s: string]:
{topics: Set<string>, mockProperties: Set<MockProperty>, objectIDs: Set<string>}} = {};
private discoveredTriggers : {[s: string]: Set<string>}= {};
private discovered: {[s: string]: Discovered} = {};
private discoveryTopic = settings.get().homeassistant.discovery_topic;
private discoveryRegex = new RegExp(`${settings.get().homeassistant.discovery_topic}/(.*)/(.*)/(.*)/config`);
private discoveryRegexWoTopic = new RegExp(`(.*)/(.*)/(.*)/config`);
private statusTopic = settings.get().homeassistant.status_topic;
private entityAttributes = settings.get().homeassistant.legacy_entity_attributes;
private zigbee2MQTTVersion: string;
@@ -145,22 +152,41 @@ export default class HomeAssistant extends Extension {
this.eventBus.onDeviceInterview(this, this.onZigbeeEvent);
this.eventBus.onDeviceMessage(this, this.onZigbeeEvent);
this.eventBus.onScenesChanged(this, this.onScenesChanged);
this.eventBus.onEntityOptionsChanged(this, (data) => this.discover(data.entity, true));
this.eventBus.onExposesChanged(this, (data) => this.discover(data.device, true));
this.eventBus.onEntityOptionsChanged(this, (data) => this.discover(data.entity));
this.eventBus.onExposesChanged(this, (data) => this.discover(data.device));
this.mqtt.subscribe(this.statusTopic);
this.mqtt.subscribe(defaultStatusTopic);
this.mqtt.subscribe(`${this.discoveryTopic}/#`);
// MQTT discovery of all paired devices on startup.
for (const entity of [this.bridge, ...this.zigbee.devices(false), ...this.zigbee.groups()]) {
this.discover(entity, true);
}
/**
* Prevent unecessary re-discovery of entities by waiting 5 seconds for retained discovery messages to come in.
* Any received discovery messages will not be published again.
* Unsubscribe from the discoveryTopic to prevent receiving our own messages.
*/
const discoverWait = 5;
// Discover with `published = false`, this will populate `this.discovered` without publishing the discoveries.
// This is needed for clearing outdated entries in `this.onMQTTMessage()`
[this.bridge, ...this.zigbee.devices(false), ...this.zigbee.groups()].forEach((e) => this.discover(e, false));
logger.debug(`Discovering entities to Home Assistant in ${discoverWait}s`);
this.mqtt.subscribe(`${this.discoveryTopic}/#`);
setTimeout(() => {
this.mqtt.unsubscribe(`${this.discoveryTopic}/#`);
logger.debug(`Discovering entities to Home Assistant`);
[this.bridge, ...this.zigbee.devices(false), ...this.zigbee.groups()].forEach((e) => this.discover(e));
}, utils.seconds(discoverWait));
// Send availability messages, this is required if the legacy_availability_payload option has been changed.
this.eventBus.emitPublishAvailability();
}
private getDiscovered(entity: Device | Group | Bridge | string): Discovered {
const ID = typeof entity === 'string' ? entity : entity.ID;
if (!(ID in this.discovered)) {
this.discovered[ID] = {messages: {}, triggers: new Set(), mockProperties: new Set(), discovered: false};
}
return this.discovered[ID];
}
private exposeToConfig(exposes: zhc.Expose[], entityType: 'device' | 'group',
allExposes: zhc.Expose[], definition?: zhc.Definition): DiscoveryEntry[] {
// For groups an array of exposes (of the same type) is passed, this is to determine e.g. what features
@@ -1109,8 +1135,9 @@ export default class HomeAssistant extends Extension {
}
@bind onDeviceRemoved(data: eventdata.DeviceRemoved): void {
logger.debug(`Clearing Home Assistant discovery topic for '${data.name}'`);
this.discovered[data.ieeeAddr]?.topics.forEach((topic) => {
logger.debug(`Clearing Home Assistant discovery for '${data.name}'`);
const discovered = this.getDiscovered(data.ieeeAddr);
Object.keys(discovered.messages).forEach((topic) => {
this.mqtt.publish(topic, null, {retain: true, qos: 1}, this.discoveryTopic, false, false);
});
@@ -1118,7 +1145,7 @@ export default class HomeAssistant extends Extension {
}
@bind onGroupMembersChanged(data: eventdata.GroupMembersChanged): void {
this.discover(data.group, true);
this.discover(data.group);
}
@bind async onPublishEntityState(data: eventdata.PublishEntityState): Promise<void> {
@@ -1131,8 +1158,9 @@ export default class HomeAssistant extends Extension {
* zigbee2mqtt/mydevice/l1.
*/
const entity = this.zigbee.resolveEntity(data.entity.name);
if (entity.isDevice() && this.discovered[entity.ieeeAddr]) {
for (const objectID of this.discovered[entity.ieeeAddr].objectIDs) {
if (entity.isDevice()) {
Object.keys(this.getDiscovered(entity).messages).forEach((topic) => {
const objectID = topic.match(this.discoveryRegexWoTopic)?.[3];
const lightMatch = /^light_(.*)/.exec(objectID);
const coverMatch = /^cover_(.*)/.exec(objectID);
@@ -1149,11 +1177,9 @@ export default class HomeAssistant extends Extension {
}
}
await this.mqtt.publish(
`${data.entity.name}/${endpoint}`, stringify(payload), {},
);
this.mqtt.publish(`${data.entity.name}/${endpoint}`, stringify(payload), {});
}
}
});
}
/**
@@ -1190,20 +1216,21 @@ export default class HomeAssistant extends Extension {
// Clear before rename so Home Assistant uses new friendly_name
// https://github.com/Koenkk/zigbee2mqtt/issues/4096#issuecomment-674044916
if (data.homeAssisantRename) {
for (const config of this.getConfigs(data.entity)) {
const topic = this.getDiscoveryTopic(config, data.entity);
const discovered = this.getDiscovered(data.entity);
for (const topic of Object.keys(discovered.messages)) {
this.mqtt.publish(topic, null, {retain: true, qos: 1}, this.discoveryTopic, false, false);
}
discovered.messages = {};
// Make sure Home Assistant deletes the old entity first otherwise another one (_2) is created
// https://github.com/Koenkk/zigbee2mqtt/issues/12610
await utils.sleep(2);
}
this.discover(data.entity, true);
this.discover(data.entity);
if (data.entity.isDevice() && this.discoveredTriggers[data.entity.ieeeAddr]) {
for (const config of this.discoveredTriggers[data.entity.ieeeAddr]) {
if (data.entity.isDevice()) {
for (const config of this.getDiscovered(data.entity).triggers) {
const key = config.substring(0, config.indexOf('_'));
const value = config.substring(config.indexOf('_') + 1);
this.publishDeviceTriggerDiscover(data.entity, key, value, true);
@@ -1387,28 +1414,22 @@ export default class HomeAssistant extends Extension {
return configs;
}
private getDiscoverKey(entity: Device | Group | Bridge): string | number {
return entity.ID;
}
private discover(entity: Device | Group | Bridge, force=false): void {
// Check if already discovered and check if there are configs.
const discoverKey = this.getDiscoverKey(entity);
const discover = force || !this.discovered[discoverKey];
private discover(entity: Device | Group | Bridge, publish: boolean = true): void {
// Handle type differences.
const isDevice = entity.isDevice();
const isGroup = entity.isGroup();
if (isGroup && (!discover || entity.zh.members.length === 0)) {
if (isGroup && entity.zh.members.length === 0) {
return;
} else if (isDevice && (!discover || !entity.definition || entity.zh.interviewing ||
} else if (isDevice && (!entity.definition || entity.zh.interviewing ||
(entity.options.hasOwnProperty('homeassistant') && !entity.options.homeassistant))) {
return;
}
const lastDiscovered = this.discovered[discoverKey];
this.discovered[discoverKey] = {topics: new Set(), mockProperties: new Set(), objectIDs: new Set()};
const discovered = this.getDiscovered(entity);
discovered.discovered = true;
const lastDiscoverdTopics = Object.keys(discovered.messages);
const newDiscoveredTopics: Set<string> = new Set();
this.getConfigs(entity).forEach((config) => {
const payload = {...config.discovery_payload};
const baseTopic = `${settings.get().mqtt.base_topic}/${entity.name}`;
@@ -1611,22 +1632,30 @@ export default class HomeAssistant extends Extension {
}
const topic = this.getDiscoveryTopic(config, entity);
this.mqtt.publish(topic, stringify(payload), {retain: true, qos: 1}, this.discoveryTopic, false, false);
this.discovered[discoverKey].topics.add(topic);
this.discovered[discoverKey].objectIDs.add(config.object_id);
config.mockProperties?.forEach((mockProperty) =>
this.discovered[discoverKey].mockProperties.add(mockProperty));
const payloadStr = stringify(payload);
newDiscoveredTopics.add(topic);
// Only discover when not discovered yet
const discoveredMessage = discovered.messages[topic];
if (!discoveredMessage || discoveredMessage.payload !== payloadStr || !discoveredMessage.published) {
discovered.messages[topic] = {payload: payloadStr, published: publish};
if (publish) {
this.mqtt.publish(topic, payloadStr, {retain: true, qos: 1}, this.discoveryTopic, false, false);
}
} else {
logger.debug(`Skipping discovery of '${topic}', already discovered`);
}
config.mockProperties?.forEach((mockProperty) => discovered.mockProperties.add(mockProperty));
});
lastDiscovered?.topics?.forEach((topic) => {
if (!this.discovered[discoverKey].topics.has(topic)) {
lastDiscoverdTopics.forEach((topic) => {
if (!newDiscoveredTopics.has(topic)) {
this.mqtt.publish(topic, null, {retain: true, qos: 1}, this.discoveryTopic, false, false);
}
});
}
@bind private onMQTTMessage(data: eventdata.MQTTMessage): void {
const discoveryRegex = new RegExp(`${this.discoveryTopic}/(.*)/(.*)/(.*)/config`);
const discoveryMatch = data.topic.match(discoveryRegex);
const discoveryMatch = data.topic.match(this.discoveryRegex);
const isDeviceAutomation = discoveryMatch && discoveryMatch[1] === 'device_automation';
if (discoveryMatch) {
// Clear outdated discovery configs and remember already discovered device_automations
@@ -1656,27 +1685,23 @@ export default class HomeAssistant extends Extension {
const key = `${discoveryMatch[3].substring(0, discoveryMatch[3].indexOf('_'))}`;
const triggerTopic = `${settings.get().mqtt.base_topic}/${entity.name}/${key}`;
if (isDeviceAutomation && message.topic === triggerTopic) {
if (!this.discoveredTriggers[ID]) {
this.discoveredTriggers[ID] = new Set();
}
this.discoveredTriggers[ID].add(discoveryMatch[3]);
this.getDiscovered(ID).triggers.add(discoveryMatch[3]);
}
}
if (!clear && !isDeviceAutomation) {
const type = discoveryMatch[1];
const objectID = discoveryMatch[3];
clear = !this.getConfigs(entity)
.find((c) => c.type === type && c.object_id === objectID &&
`${this.discoveryTopic}/${this.getDiscoveryTopic(c, entity)}` === data.topic);
const topic = data.topic.substring(this.discoveryTopic.length + 1);
if (!clear && !isDeviceAutomation && !(topic in this.getDiscovered(entity).messages)) {
clear = true;
}
// Device was flagged to be excluded from homeassistant discovery
clear = clear || (entity.options.hasOwnProperty('homeassistant') && !entity.options.homeassistant);
if (clear) {
logger.debug(`Clearing Home Assistant config '${data.topic}'`);
const topic = data.topic.substring(this.discoveryTopic.length + 1);
logger.debug(`Clearing outdated Home Assistant config '${data.topic}'`);
this.mqtt.publish(topic, null, {retain: true, qos: 1}, this.discoveryTopic, false, false);
} else {
this.getDiscovered(entity).messages[topic] = {payload: stringify(message), published: true};
}
} else if ((data.topic === this.statusTopic || data.topic === defaultStatusTopic) &&
data.message.toLowerCase() === 'online') {
@@ -1694,17 +1719,21 @@ export default class HomeAssistant extends Extension {
}
@bind onZigbeeEvent(data: {device: Device}): void {
this.discover(data.device);
if (!this.getDiscovered(data.device).discovered) {
this.discover(data.device);
}
}
@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
logger.debug(`Clearing Home Assistant scene discovery topics for '${data.entity.name}'`);
this.discovered[this.getDiscoverKey(data.entity)]?.topics.forEach((topic) => {
logger.debug(`Clearing Home Assistant scene discovery for '${data.entity.name}'`);
const discovered = this.getDiscovered(data.entity);
Object.keys(discovered.messages).forEach((topic) => {
if (topic.startsWith('scene')) {
this.mqtt.publish(topic, null, {retain: true, qos: 1}, this.discoveryTopic, false, false);
delete discovered.messages[topic];
}
});
@@ -1715,7 +1744,7 @@ export default class HomeAssistant extends Extension {
// Re-discover entity (including any new scenes).
logger.debug(`Re-discovering entities with their scenes.`);
this.discover(data.entity, true);
this.discover(data.entity);
}
private getDevicePayload(entity: Device | Group | Bridge): KeyValue {
@@ -1765,8 +1794,7 @@ export default class HomeAssistant extends Extension {
}
override adjustMessageBeforePublish(entity: Device | Group | Bridge, message: KeyValue): void {
const discoverKey = this.getDiscoverKey(entity);
this.discovered[discoverKey]?.mockProperties?.forEach((mockProperty) => {
this.getDiscovered(entity).mockProperties.forEach((mockProperty) => {
if (!message.hasOwnProperty(mockProperty.property)) {
message[mockProperty.property] = mockProperty.value;
}
@@ -1804,12 +1832,9 @@ export default class HomeAssistant extends Extension {
return;
}
if (!this.discoveredTriggers[device.ieeeAddr]) {
this.discoveredTriggers[device.ieeeAddr] = new Set();
}
const discovered = this.getDiscovered(device);
const discoveredKey = `${key}_${value}`;
if (this.discoveredTriggers[device.ieeeAddr].has(discoveredKey) && !force) {
if (discovered.triggers.has(discoveredKey) && !force) {
return;
}
@@ -1834,11 +1859,7 @@ export default class HomeAssistant extends Extension {
};
await this.mqtt.publish(topic, stringify(payload), {retain: true, qos: 1}, this.discoveryTopic, false, false);
this.discoveredTriggers[device.ieeeAddr].add(discoveredKey);
}
_clearDiscoveredTrigger(): void {
this.discoveredTriggers = {};
discovered.triggers.add(discoveredKey);
}
private getBridgeEntity(coordinatorVersion: zh.CoordinatorVersion): Bridge {
+4
View File
@@ -136,6 +136,10 @@ export default class MQTT {
this.client.subscribe(topic);
}
unsubscribe(topic: string): void {
this.client.unsubscribe(topic);
}
@bind public onMessage(topic: string, message: Buffer): void {
// Since we subscribe to zigbee2mqtt/# we also receive the message we send ourselves, skip these.
if (!this.publishedTopics.has(topic)) {
+68 -7
View File
@@ -15,11 +15,23 @@ describe('HomeAssistant extension', () => {
let extension;
let origin;
let resetExtension = async () => {
let resetExtension = async (runTimers=true) => {
await controller.enableDisableExtension(false, 'HomeAssistant');
MQTT.publish.mockClear();
await controller.enableDisableExtension(true, 'HomeAssistant');
extension = controller.extensions.find((e) => e.constructor.name === 'HomeAssistant');
if (runTimers) {
jest.runOnlyPendingTimers();
}
}
let resetDiscoveryPayloads = (id) => {
// Change discovered payload, otherwise it's not re-published because it's the same.
Object.values(extension.discovered[id].messages).forEach((m) => m.payload = 'changed');
}
let clearDiscoveredTrigger = (id) => {
extension.discovered[id].triggers = new Set();
}
beforeEach(async () => {
@@ -29,6 +41,7 @@ describe('HomeAssistant extension', () => {
data.writeEmptyState();
controller.state.load();
await resetExtension();
await flushPromises();
});
beforeAll(async () => {
@@ -77,7 +90,6 @@ describe('HomeAssistant extension', () => {
it('Should discover devices and groups', async () => {
let payload;
await flushPromises();
payload = {
"availability":[{"topic":"zigbee2mqtt/bridge/state"}],
@@ -410,6 +422,51 @@ describe('HomeAssistant extension', () => {
);
});
it('Should not discovery devices which are already discovered', async() => {
await resetExtension(false);
const topic = 'homeassistant/sensor/0x0017880104e45522/humidity/config';
const payload = stringify({
'unit_of_measurement': '%',
'device_class': 'humidity',
'state_class': 'measurement',
'value_template': '{{ value_json.humidity }}',
'state_topic': 'zigbee2mqtt/weather_sensor',
'json_attributes_topic': 'zigbee2mqtt/weather_sensor',
'object_id': 'weather_sensor_humidity',
'unique_id': '0x0017880104e45522_humidity_zigbee2mqtt',
'origin': origin,
'enabled_by_default': true,
'device': {
'identifiers': ['zigbee2mqtt_0x0017880104e45522'],
'name': 'weather_sensor',
'sw_version': null,
'model': 'Temperature and humidity sensor (WSDCGQ11LM)',
'manufacturer': 'Aqara',
'via_device': 'zigbee2mqtt_bridge_0x00124b00120144ae',
},
'availability': [{topic: 'zigbee2mqtt/bridge/state'}],
});
// Should subscribe to `homeassistant/#` to find out what devices are already discovered.
expect(MQTT.subscribe).toHaveBeenCalledWith(`homeassistant/#`);
// Retained Home Assistant discovery message arrives
await MQTT.events.message(topic, payload);
jest.runOnlyPendingTimers();
// Should unsubscribe to not receive all messages that are going to be published to `homeassistant/#` again.
expect(MQTT.unsubscribe).toHaveBeenCalledWith(`homeassistant/#`);
expect(MQTT.publish).not.toHaveBeenCalledWith(
'homeassistant/sensor/0x0017880104e45522/humidity/config',
expect.any(String),
expect.any(Object),
expect.any(Function),
);
expect(logger.debug).toHaveBeenCalledWith(`Skipping discovery of 'sensor/0x0017880104e45522/humidity/config', already discovered`)
});
it('Should discover devices with precision', async () => {
settings.set(['devices', '0x0017880104e45522'], {
humidity_precision: 0,
@@ -1073,7 +1130,7 @@ describe('HomeAssistant extension', () => {
});
it('Should discover when not discovered yet', async () => {
controller.extensions.find((e) => e.constructor.name === 'HomeAssistant').discovered = {};
extension.discovered = {};
const device = zigbeeHerdsman.devices.WSDCGQ11LM;
const data = {measuredValue: -85}
const payload = {data, cluster: 'msTemperatureMeasurement', device, endpoint: device.getEndpoint(1), type: 'attributeReport', linkquality: 10};
@@ -1111,7 +1168,7 @@ describe('HomeAssistant extension', () => {
});
it('Shouldnt discover when device leaves', async () => {
controller.extensions.find((e) => e.constructor.name === 'HomeAssistant').discovered = {};
extension.discovered = {};
const device = zigbeeHerdsman.devices.bulb;
const payload = {ieeeAddr: device.ieeeAddr};
MQTT.publish.mockClear();
@@ -1121,6 +1178,7 @@ describe('HomeAssistant extension', () => {
it('Should discover when options change', async () => {
const device = controller.zigbee.resolveEntity(zigbeeHerdsman.devices.bulb);
resetDiscoveryPayloads(device.ieeeAddr);
MQTT.publish.mockClear();
controller.eventBus.emitEntityOptionsChanged({entity: device, from: {}, to: {'test': 123}});
await flushPromises();
@@ -1629,7 +1687,7 @@ describe('HomeAssistant extension', () => {
);
// Shouldn't rediscover when already discovered in previous session
controller.extensions.find((e) => e.constructor.name === 'HomeAssistant')._clearDiscoveredTrigger();
clearDiscoveredTrigger('0x0017880104e45520');
await MQTT.events.message('homeassistant/device_automation/0x0017880104e45520/action_double/config', stringify({topic: 'zigbee2mqtt/button/action'}));
await MQTT.events.message('homeassistant/device_automation/0x0017880104e45520/action_double/config', stringify({topic: 'zigbee2mqtt/button/action'}));
await flushPromises();
@@ -1640,7 +1698,7 @@ describe('HomeAssistant extension', () => {
expect(MQTT.publish).not.toHaveBeenCalledWith('homeassistant/device_automation/0x0017880104e45520/action_double/config', expect.any(String), expect.any(Object), expect.any(Function));
// Should rediscover when already discovered in previous session but with different name
controller.extensions.find((e) => e.constructor.name === 'HomeAssistant')._clearDiscoveredTrigger();
clearDiscoveredTrigger('0x0017880104e45520');
await MQTT.events.message('homeassistant/device_automation/0x0017880104e45520/action_double/config', stringify({topic: 'zigbee2mqtt/button_other_name/action'}));
await flushPromises();
MQTT.publish.mockClear();
@@ -1905,6 +1963,7 @@ describe('HomeAssistant extension', () => {
});
it('Should rediscover group when device is added to it', async () => {
resetDiscoveryPayloads(9);
MQTT.publish.mockClear();
MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'ha_discovery_group', device: 'wall_switch_double/left'}));
await flushPromises();
@@ -2148,9 +2207,9 @@ 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);
resetDiscoveryPayloads(device.ieeeAddr);
MQTT.publish.mockClear();
controller.eventBus.emitScenesChanged({entity: device});
@@ -2194,6 +2253,7 @@ describe('HomeAssistant extension', () => {
// Group scenes.
const group = controller.zigbee.resolveEntity('ha_discovery_group');
resetDiscoveryPayloads(9);
MQTT.publish.mockClear();
controller.eventBus.emitScenesChanged({entity: group});
@@ -2503,6 +2563,7 @@ describe('HomeAssistant extension', () => {
const device = zigbeeHerdsman.devices['BMCT-SLZ'];
const data = {deviceMode: 0}
const msg = {data, cluster: 'manuSpecificBosch10', device, endpoint: device.getEndpoint(1), type: 'attributeReport', linkquality: 10};
resetDiscoveryPayloads('0x18fc26000000cafe');
await zigbeeHerdsman.events.message(msg);
const payload = {
'availability':[{'topic':'zigbee2mqtt/bridge/state'}],
+1
View File
@@ -4,6 +4,7 @@ const mock = {
publish: jest.fn().mockImplementation((topic, payload, options, cb) => cb()),
end: jest.fn(),
subscribe: jest.fn(),
unsubscribe: jest.fn(),
reconnecting: false,
on: jest.fn(),
stream: {setMaxListeners: jest.fn()}