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 <koenkanters94@gmail.com>

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;

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):
+        // - <base_topic>/device_name/set (endpoint and attribute is defined in the payload)
+        // - <base_topic>/device_name/set/attribute (default endpoint used)
+        // - <base_topic>/device_name/endpoint/set (attribute is defined in the payload)
+        // - <base_topic>/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;
             }
 

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;}

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));
         }

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']);