From a5a87a79a80aaa261a34a8fb541f47347c4ca729 Mon Sep 17 00:00:00 2001 From: Oleksandr Masliuchenko Date: Wed, 7 Feb 2024 21:12:48 +0200 Subject: [PATCH] fix: Match endpoint name in the MQTT topic based on endpoints listed in the definition (#21214) * Add endpoint matching based on the endpoint names list in the device definition * Cleanup debug prints * Correct creating endpoint names list, filter out null elements * Correct test due to slightly changed behavior * Fix endpoint name lookup when converter does not provide full name-to-id mapping * Simplify topic name matching * Remove obsolete branches, improve code coverage * Make linter happy * Make linter happy * Remove dependency on endpoint names while removing device from all groups * Get rid of predefined list of endpoints when processing group state update --------- Co-authored-by: Koen Kanters --- lib/extension/groups.ts | 3 ++- lib/extension/publish.ts | 49 +++++++++++++++++++++++++++------------- lib/model/device.ts | 17 +++++++++++--- lib/util/settings.ts | 2 +- test/publish.test.js | 2 +- 5 files changed, 51 insertions(+), 22 deletions(-) diff --git a/lib/extension/groups.ts b/lib/extension/groups.ts index 3b7166ccb..d107eecdb 100644 --- a/lib/extension/groups.ts +++ b/lib/extension/groups.ts @@ -109,8 +109,9 @@ export default class Groups extends Extension { const payload: KeyValue = {}; let endpointName: string = null; + const endpointNames: string[] = data.entity instanceof Device ? data.entity.getEndpointNames() : []; for (let [prop, value] of Object.entries(data.update)) { - const endpointNameMatch = utils.endpointNames.find((n) => prop.endsWith(`_${n}`)); + const endpointNameMatch = endpointNames.find((n) => prop.endsWith(`_${n}`)); if (endpointNameMatch) { prop = prop.substring(0, prop.length - endpointNameMatch.length - 1); endpointName = endpointNameMatch; diff --git a/lib/extension/publish.ts b/lib/extension/publish.ts index 872e21957..064a1c608 100644 --- a/lib/extension/publish.ts +++ b/lib/extension/publish.ts @@ -10,8 +10,7 @@ import Group from '../model/group'; import Device from '../model/device'; import bind from 'bind-decorator'; -const topicRegex = new RegExp(`^(.+?)(?:/(${utils.endpointNames.join('|')}|\\d+))?/(get|set)(?:/(.+))?`); -const propertyEndpointRegex = new RegExp(`^(.*?)_(${utils.endpointNames.join('|')})$`); +const topicGetSetRegex = new RegExp(`^(.+?)(?:/([^/]+))?/(get|set)(?:/(.+))?`); const stateValues = ['on', 'off', 'toggle', 'open', 'close', 'stop', 'lock', 'unlock']; const sceneConverterKeys = ['scene_store', 'scene_add', 'scene_remove', 'scene_remove_all', 'scene_rename']; @@ -40,18 +39,37 @@ export default class Publish extends Extension { } parseTopic(topic: string): ParsedTopic | null { - const match = topic.match(topicRegex); - if (!match) { - return null; + // The function supports the following topic formats (below are for 'set'. 'get' will look the same): + // - /device_name/set (endpoint and attribute is defined in the payload) + // - /device_name/set/attribute (default endpoint used) + // - /device_name/endpoint/set (attribute is defined in the payload) + // - /device_name/endpoint/set/attribute (payload is the value) + + // The first step is to get rid of base topic part + topic = topic.replace(`${settings.get().mqtt.base_topic}/`, ''); + + // Also bridge requests are something we don't care about + if (topic.match(/bridge/)) return null; + + // Make the rough split on get/set keyword. + // Before the get/set is the device name and optional endpoint name. + // After it there will be an optional attribute name. + const match = topic.match(topicGetSetRegex); + if (!match) return null; + let deviceName = match[1]; + let endpointName = match[2]; + const attribute = match[4]; + + // There can be some ambiguoty between 'device_name/endpoint' and 'device/name/with/slashes' with no endpoint + // Try to ensure the device with one of these names exist + const re = this.zigbee.resolveEntity(deviceName); + if (re == null) { + // Possibly the last before get/set is just a continuation of the device name + deviceName = `${deviceName}/${endpointName}`; + endpointName = null; } - const ID = match[1].replace(`${settings.get().mqtt.base_topic}/`, ''); - // If we didn't replace base_topic we received something we don't care about - if (ID === match[1] || ID.match(/bridge/)) { - return null; - } - - return {ID: ID, endpoint: match[2], type: match[3] as 'get' | 'set', attribute: match[4]}; + return {ID: deviceName, endpoint: endpointName, type: match[3] as 'get' | 'set', attribute: attribute}; } parseMessage(parsedTopic: ParsedTopic, data: eventdata.MQTTMessage): KeyValue | null { @@ -185,6 +203,9 @@ export default class Publish extends Extension { toPublish[ID] = {...toPublish[ID], ...payload}; }; + const endpointNames = re instanceof Device ? re.getEndpointNames() : []; + const propertyEndpointRegex = new RegExp(`^(.*?)_(${endpointNames.join('|')})$`); + for (let [key, value] of entries) { let endpointName = parsedTopic.endpoint; let localTarget = target; @@ -196,10 +217,6 @@ export default class Publish extends Extension { endpointName = propertyEndpointMatch[2]; key = propertyEndpointMatch[1]; localTarget = re.endpoint(endpointName); - if (localTarget == null) { - logger.error(`Device '${re.name}' has no endpoint '${endpointName}'`); - continue; - } endpointOrGroupID = localTarget.ID; } diff --git a/lib/model/device.ts b/lib/model/device.ts index 260e7e4ef..2488badad 100644 --- a/lib/model/device.ts +++ b/lib/model/device.ts @@ -65,12 +65,23 @@ export default class Device { } endpointName(endpoint: zh.Endpoint): string { - let name = null; + let epName = null; if (this.definition?.endpoint) { - name = Object.entries(this.definition?.endpoint(this.zh)).find((e) => e[1] == endpoint.ID)[0]; + const mapping = this.definition?.endpoint(this.zh); + for (const [name, id] of Object.entries(mapping)) { + if (id == endpoint.ID) { + epName = name; + } + } } /* istanbul ignore next */ - return name === 'default' ? null : name; + return epName === 'default' ? null : epName; + } + + getEndpointNames(): string[] { + return this.zh.endpoints + .map((ep) => this.endpointName(ep)) + .filter((name) => name !== null); } isIkeaTradfri(): boolean {return this.zh.manufacturerID === 4476;} diff --git a/lib/util/settings.ts b/lib/util/settings.ts index bb5c6cb43..0d6868ac9 100644 --- a/lib/util/settings.ts +++ b/lib/util/settings.ts @@ -593,7 +593,7 @@ export function removeDevice(IDorName: string): void { // Remove device from groups if (settings.groups) { const regex = - new RegExp(`^(${device.friendly_name}|${device.ID})(/(\\d|${utils.endpointNames.join('|')}))?$`); + new RegExp(`^(${device.friendly_name}|${device.ID})(/[^/]+)?$`); for (const group of Object.values(settings.groups).filter((g) => g.devices)) { group.devices = group.devices.filter((device) => !device.match(regex)); } diff --git a/test/publish.test.js b/test/publish.test.js index c548ebb25..333c3ca00 100644 --- a/test/publish.test.js +++ b/test/publish.test.js @@ -534,7 +534,7 @@ describe('Publish', () => { logger.error.mockClear(); await MQTT.events.message('zigbee2mqtt/0x0017880104e45542/get', stringify({state_center: '', state_right: ''})); await flushPromises(); - expect(logger.error).toHaveBeenCalledWith(`Device 'wall_switch_double' has no endpoint 'center'`); + expect(logger.error).toHaveBeenCalledWith(`No converter available for 'state_center' ("")`); expect(endpoint2.read).toHaveBeenCalledTimes(0); expect(endpoint3.read).toHaveBeenCalledTimes(1); expect(endpoint3.read).toHaveBeenCalledWith('genOnOff', ['onOff']);