fix: Remove dependency on predefined list of endpoints (`parseEntityI…

…D()` function) (#21264)

* Add function resolveEntityByID() function as an improved alternative for parseEntityID()

* Port deviceGroupMembership.ts from parseEntityID() to resolveEntityByID()

* Migrade groups.ts from parseEntityID() to resolveEntityByID()

* Improve test coverage on groups functionality

* Migrate from parseEntityID() to resolveEntityByID() for bridge functionality

* Migrate from parseEntityID() to resolveEntityByID() for bind functionality

* Finally get rid of parseEntityID() function

* Fix linter issues

* Move resolveEntityAndEndpoint() function to zigbee.ts

---------

Co-authored-by: Koen Kanters <koenkanters94@gmail.com>

lib/extension/bind.ts

@@ -222,25 +222,28 @@ export default class Bind extends Extension {
         const message = utils.parseJSON(data.message, data.message);
 
         let error = null;
-        const parsedSource = utils.parseEntityID(sourceKey);
-        const parsedTarget = utils.parseEntityID(targetKey);
-        const source = this.zigbee.resolveEntity(parsedSource.ID);
-        const target = targetKey === 'default_bind_group' ?
-            defaultBindGroup : this.zigbee.resolveEntity(parsedTarget.ID);
+        const parsedSource = this.zigbee.resolveEntityAndEndpoint(sourceKey);
+        const parsedTarget = this.zigbee.resolveEntityAndEndpoint(targetKey);
+        const source = parsedSource.entity;
+        const target = targetKey === 'default_bind_group' ? defaultBindGroup : parsedTarget.entity;
         const responseData: KeyValue = {from: sourceKey, to: targetKey};
 
         if (!source || !(source instanceof Device)) {
             error = `Source device '${sourceKey}' does not exist`;
+        } else if (parsedSource.endpointID && !parsedSource.endpoint) {
+            error = `Source device '${parsedSource.ID}' does not have endpoint '${parsedSource.endpointID}'`;
         } else if (!target) {
             error = `Target device or group '${targetKey}' does not exist`;
+        } else if (target instanceof Device && parsedTarget.endpointID && !parsedTarget.endpoint) {
+            error = `Target device '${parsedTarget.ID}' does not have endpoint '${parsedTarget.endpointID}'`;
         } else {
             const successfulClusters: string[] = [];
             const failedClusters = [];
             const attemptedClusters = [];
 
-            const bindSource: zh.Endpoint = source.endpoint(parsedSource.endpoint);
+            const bindSource: zh.Endpoint = parsedSource.endpoint;
             let bindTarget: number | zh.Group | zh.Endpoint = null;
-            if (target instanceof Device) bindTarget = target.endpoint(parsedTarget.endpoint);
+            if (target instanceof Device) bindTarget = parsedTarget.endpoint;
             else if (target instanceof Group) bindTarget = target.zh;
             else bindTarget = Number(target.ID);
 

lib/extension/bridge.ts

@@ -430,8 +430,13 @@ export default class Bridge extends Extension {
             throw new Error(`Invalid payload`);
         }
 
-        const parsedID = utils.parseEntityID(message.id);
-        const endpoint = (this.getEntity('device', parsedID.ID) as Device).endpoint(parsedID.endpoint);
+        const device = this.zigbee.resolveEntityAndEndpoint(message.id);
+        if (!device.entity) throw new Error(`Device '${message.id}' does not exist`);
+
+        const endpoint = device.endpoint;
+        if (device.endpointID && !endpoint) {
+            throw new Error(`Device '${device.ID}' does not have endpoint '${device.endpointID}'`);
+        }
 
         const coordinatorEndpoint = this.zigbee.firstCoordinatorEndpoint();
         await endpoint.bind(message.cluster, coordinatorEndpoint);
@@ -457,8 +462,9 @@ export default class Bridge extends Extension {
             throw new Error(`Invalid payload`);
         }
 
-        const parsedID = utils.parseEntityID(message.id);
-        const device = this.getEntity('device', parsedID.ID) as Device;
+        const device = this.zigbee.resolveEntityAndEndpoint(message.id).entity as Device;
+        if (!device) throw new Error(`Device '${message.id}' does not exist`);
+
         const source = await zhc.generateExternalDefinitionSource(device.zh);
 
         return utils.getResponse(message, {id: message.id, source}, null);

lib/extension/groups.ts

@@ -68,10 +68,13 @@ export default class Groups extends Extension {
             const groupID = settingGroup.ID;
             const zigbeeGroup = zigbeeGroups.find((g) => g.ID === groupID) || this.zigbee.createGroup(groupID);
             const settingsEndpoint = settingGroup.devices.map((d) => {
-                const parsed = utils.parseEntityID(d);
-                const entity = this.zigbee.resolveEntity(parsed.ID) as Device;
+                const parsed = this.zigbee.resolveEntityAndEndpoint(d);
+                const entity = parsed.entity as Device;
                 if (!entity) logger.error(`Cannot find '${d}' of group '${settingGroup.friendly_name}'`);
-                return {'endpoint': entity?.endpoint(parsed.endpoint), 'name': entity?.name};
+                if (parsed.endpointID && !parsed.endpoint) {
+                    logger.error(`Cannot find endpoint '${parsed.endpointID}' of device '${parsed.ID}'`);
+                }
+                return {'endpoint': parsed.endpoint, 'name': entity?.name};
             }).filter((e) => e.endpoint != null);
 
             // In settings but not in zigbee
@@ -238,8 +241,8 @@ export default class Groups extends Extension {
                 type = 'remove_all';
             }
 
-            const parsedEntity = utils.parseEntityID(data.message);
-            resolvedEntityDevice = this.zigbee.resolveEntity(parsedEntity.ID) as Device;
+            const parsedEntity = this.zigbee.resolveEntityAndEndpoint(data.message);
+            resolvedEntityDevice = parsedEntity.entity as Device;
             if (!resolvedEntityDevice || !(resolvedEntityDevice instanceof Device)) {
                 logger.error(`Device '${data.message}' does not exist`);
 
@@ -256,7 +259,12 @@ export default class Groups extends Extension {
 
                 return null;
             }
-            resolvedEntityEndpoint = resolvedEntityDevice.endpoint(parsedEntity.endpoint);
+
+            resolvedEntityEndpoint = parsedEntity.endpoint;
+            if (parsedEntity.endpointID && !resolvedEntityEndpoint) {
+                logger.error(`Device '${parsedEntity.ID}' does not have endpoint '${parsedEntity.endpointID}'`);
+                return null;
+            }
         } else if (topicRegexMatch) {
             type = topicRegexMatch[1] as 'remove' | 'add' | 'remove_all';
             const message = JSON.parse(data.message);
@@ -271,13 +279,16 @@ export default class Groups extends Extension {
                 }
             }
 
-            const parsed = utils.parseEntityID(message.device);
-            resolvedEntityDevice = this.zigbee.resolveEntity(parsed.ID) as Device;
+            const parsed = this.zigbee.resolveEntityAndEndpoint(message.device);
+            resolvedEntityDevice = parsed?.entity as Device;
             if (!error && (!resolvedEntityDevice || !(resolvedEntityDevice instanceof Device))) {
                 error = `Device '${message.device}' does not exist`;
             }
             if (!error) {
-                resolvedEntityEndpoint = resolvedEntityDevice.endpoint(parsed.endpoint);
+                resolvedEntityEndpoint = parsed.endpoint;
+                if (parsed.endpointID && !resolvedEntityEndpoint) {
+                    error = `Device '${parsed.ID}' does not have endpoint '${parsed.endpointID}'`;
+                }
             }
         }
 

lib/extension/legacy/deviceGroupMembership.ts

@@ -1,7 +1,6 @@
 /* istanbul ignore file */
 import * as settings from '../../util/settings';
 import logger from '../../util/logger';
-import utils from '../../util/utils';
 import Extension from '../extension';
 import bind from 'bind-decorator';
 import Device from '../../model/device';
@@ -19,13 +18,19 @@ export default class DeviceGroupMembership extends Extension {
             return null;
         }
 
-        const parsed = utils.parseEntityID(match[1]);
-        const device = this.zigbee.resolveEntity(parsed.ID) as Device;
+        const parsed = this.zigbee.resolveEntityAndEndpoint(match[1]);
+        const device = parsed?.entity as Device;
         if (!device || !(device instanceof Device)) {
             logger.error(`Device '${match[1]}' does not exist`);
             return;
         }
-        const endpoint = device.endpoint(parsed.endpoint);
+
+        const endpoint = parsed.endpoint;
+        if (parsed.endpointID && !endpoint) {
+            logger.error(`Device '${parsed.ID}' does not have endpoint '${parsed.endpointID}'`);
+            return;
+        }
+
         const response = await endpoint.command(
             `genGroups`, 'getMembership', {groupcount: 0, grouplist: []}, {},
         );

lib/util/utils.ts

@@ -309,12 +309,6 @@ function isAvailabilityEnabledForEntity(entity: Device | Group, settings: Settin
     return !blocklist.includes(entity.name) && !blocklist.includes(entity.ieeeAddr);
 }
 
-const entityIDRegex = new RegExp(`^(.+?)(?:/(${endpointNames.join('|')}|\\d+))?$`);
-function parseEntityID(ID: string): {ID: string, endpoint: string} {
-    const match = ID.match(entityIDRegex);
-    return match && {ID: match[1], endpoint: match[2]};
-}
-
 function isEndpoint(obj: unknown): obj is zh.Endpoint {
     return obj.constructor.name.toLowerCase() === 'endpoint';
 }
@@ -436,7 +430,7 @@ export default {
     endpointNames, capitalize, getZigbee2MQTTVersion, getDependencyVersion, formatDate, objectHasProperties,
     equalsPartial, getObjectProperty, getResponse, parseJSON, loadModuleFromText, loadModuleFromFile,
     removeNullPropertiesFromObject, toNetworkAddressHex, toSnakeCase,
-    parseEntityID, isEndpoint, isZHGroup, hours, minutes, seconds, validateFriendlyName, sleep,
+    isEndpoint, isZHGroup, hours, minutes, seconds, validateFriendlyName, sleep,
     sanitizeImageParameter, isAvailabilityEnabledForEntity, publishLastSeen, availabilityPayload,
     getAllFiles, filterProperties, flatten, arrayUnique, clone, computeSettingsToChange, getScenes,
 };

lib/zigbee.ts

@@ -11,6 +11,8 @@ import * as ZHEvents from 'zigbee-herdsman/dist/controller/events';
 import bind from 'bind-decorator';
 import {randomInt} from 'crypto';
 
+const entityIDRegex = new RegExp(`^(.+?)(?:/(.+))?$`);
+
 export default class Zigbee {
     private herdsman: Controller;
     private eventBus: EventBus;
@@ -283,6 +285,37 @@ export default class Zigbee {
         }
     }
 
+    resolveEntityAndEndpoint(ID: string)
+        : {ID: string, entity: Device | Group, endpointID: string, endpoint: zh.Endpoint} {
+        // This function matches the following entity formats:
+        // device_name          (just device name)
+        // device_name/ep_name  (device name and endpoint numeric ID or name)
+        // device/name          (device name with slashes)
+        // device/name/ep_name  (device name with slashes, and endpoint numeric ID or name)
+
+        // First split the input token by the latest slash
+        const match = ID.match(entityIDRegex);
+
+        // Try to match 'device_name/endpoint' pattern
+        let entityName = match[1];
+        let deviceOrGroup = this.resolveEntity(match[1]);
+        let endpointNameOrID = match[2];
+
+        // If 'device_name/endpoint' pattern does not match, perhaps this is device name with slashes
+        if (!deviceOrGroup) {
+            entityName = ID;
+            deviceOrGroup = this.resolveEntity(ID);
+            endpointNameOrID = null;
+        }
+
+        // If the function returns non-null endpoint name, but the endpoint field is null, then
+        // it means that endpoint was not matched because there is no such endpoint on the device
+        // (or the entity is a group)
+        const endpoint = deviceOrGroup?.isDevice() ? deviceOrGroup.endpoint(endpointNameOrID) : null;
+
+        return {ID: entityName, entity: deviceOrGroup, endpointID: endpointNameOrID, endpoint: endpoint};
+    }
+
     firstCoordinatorEndpoint(): zh.Endpoint {
         return this.herdsman.getDevicesByType('Coordinator')[0].endpoints[0];
     }

test/bind.test.js

@@ -387,7 +387,7 @@ describe('Bind', () => {
         );
     });
 
-    it('Error bind fails when source not existing', async () => {
+    it('Error bind fails when source device does not exist', async () => {
         const device = zigbeeHerdsman.devices.remote;
         const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
         const endpoint = device.getEndpoint(1);
@@ -401,7 +401,21 @@ describe('Bind', () => {
         );
     });
 
-    it('Error bind fails when target not existing', async () => {
+    it("Error bind fails when source device's endpoint does not exist", async () => {
+        const device = zigbeeHerdsman.devices.remote;
+        const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
+        const endpoint = device.getEndpoint(1);
+        mockClear(device);
+        MQTT.events.message('zigbee2mqtt/bridge/request/device/bind', stringify({from: 'remote/not_existing_endpoint', to: 'bulb_color'}));
+        await flushPromises();
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            'zigbee2mqtt/bridge/response/device/bind',
+            stringify({"data":{"from":"remote/not_existing_endpoint","to":"bulb_color"},"status":"error","error":"Source device 'remote' does not have endpoint 'not_existing_endpoint'"}),
+            {retain: false, qos: 0}, expect.any(Function)
+        );
+    });
+
+    it('Error bind fails when target device or group does not exist', async () => {
         const device = zigbeeHerdsman.devices.remote;
         const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
         const endpoint = device.getEndpoint(1);
@@ -415,6 +429,20 @@ describe('Bind', () => {
         );
     });
 
+    it("Error bind fails when target device's endpoint does not exist", async () => {
+        const device = zigbeeHerdsman.devices.remote;
+        const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
+        const endpoint = device.getEndpoint(1);
+        mockClear(device);
+        MQTT.events.message('zigbee2mqtt/bridge/request/device/bind', stringify({from: 'remote', to: 'bulb_color/not_existing_endpoint'}));
+        await flushPromises();
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            'zigbee2mqtt/bridge/response/device/bind',
+            stringify({"data":{"from":"remote","to":"bulb_color/not_existing_endpoint"},"status":"error","error":"Target device 'bulb_color' does not have endpoint 'not_existing_endpoint'"}),
+            {retain: false, qos: 0}, expect.any(Function)
+        );
+    });
+
     it('Legacy api: Should bind', async () => {
         const device = zigbeeHerdsman.devices.remote;
         const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);

test/bridge.test.js

@@ -681,6 +681,17 @@ describe('Bridge', () => {
         );
     });
 
+    it('Should error when generate_external_definition requested for unknown device', async () => {
+        MQTT.publish.mockClear();
+        MQTT.events.message('zigbee2mqtt/bridge/request/device/generate_external_definition', stringify({id: 'non_existing_device'}));
+        await flushPromises();
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            'zigbee2mqtt/bridge/response/device/generate_external_definition',
+            stringify({ data: {}, error: "Device 'non_existing_device' does not exist", status: 'error' }),
+            {retain: false, qos: 0}, expect.any(Function)
+        );
+    });
+
     it('Should allow to generate device definition', async () => {
         MQTT.publish.mockClear();
         MQTT.events.message('zigbee2mqtt/bridge/request/device/generate_external_definition', stringify({id: ZNCZ02LM.ieeeAddr}));
@@ -1132,6 +1143,38 @@ describe('Bridge', () => {
         );
     });
 
+    it('Should throw error when configure reporting is called for non-existing device', async () => {
+        const device = zigbeeHerdsman.devices.bulb;
+        const endpoint = device.getEndpoint(1);
+        endpoint.configureReporting.mockClear();
+        zigbeeHerdsman.permitJoin.mockClear();
+        MQTT.publish.mockClear();
+        MQTT.events.message('zigbee2mqtt/bridge/request/device/configure_reporting', stringify({id: 'non_existing_device', cluster: 'genLevelCtrl', attribute: 'currentLevel', maximum_report_interval: 10, minimum_report_interval: 1, reportable_change: 1}));
+        await flushPromises();
+        expect(endpoint.configureReporting).toHaveBeenCalledTimes(0);
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            'zigbee2mqtt/bridge/response/device/configure_reporting',
+            stringify({"data":{},"status":"error","error":"Device 'non_existing_device' does not exist"}),
+            {retain: false, qos: 0}, expect.any(Function)
+        );
+    });
+
+    it('Should throw error when configure reporting is called for non-existing endpoint', async () => {
+        const device = zigbeeHerdsman.devices.bulb;
+        const endpoint = device.getEndpoint(1);
+        endpoint.configureReporting.mockClear();
+        zigbeeHerdsman.permitJoin.mockClear();
+        MQTT.publish.mockClear();
+        MQTT.events.message('zigbee2mqtt/bridge/request/device/configure_reporting', stringify({id: '0x000b57fffec6a5b2/non_existing_endpoint', cluster: 'genLevelCtrl', attribute: 'currentLevel', maximum_report_interval: 10, minimum_report_interval: 1, reportable_change: 1}));
+        await flushPromises();
+        expect(endpoint.configureReporting).toHaveBeenCalledTimes(0);
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            'zigbee2mqtt/bridge/response/device/configure_reporting',
+            stringify({"data":{},"status":"error","error":"Device '0x000b57fffec6a5b2' does not have endpoint 'non_existing_endpoint'"}),
+            {retain: false, qos: 0}, expect.any(Function)
+        );
+    });
+
     it('Should allow to create a backup', async () => {
         fs.mkdirSync(path.join(data.mockDir, 'ext_converters'));
         fs.writeFileSync(path.join(data.mockDir, 'ext_converters', 'afile.js'), 'test123')

test/group.test.js

@@ -273,14 +273,22 @@ describe('Groups', () => {
         expect(logger.error).toHaveBeenCalledWith("Group 'group_1_not_existing' does not exist");
     });
 
-    it('Legacy api: Log when adding to non-existing device', async () => {
+    it('Legacy api: Log when adding a non-existing device', async () => {
         await resetExtension();
         logger.error.mockClear();
         MQTT.events.message('zigbee2mqtt/bridge/group/group_1/add', 'bulb_color_not_existing');
         await flushPromises();
         expect(logger.error).toHaveBeenCalledWith("Device 'bulb_color_not_existing' does not exist");
     });
 
+    it('Legacy api: Log when adding a non-existing endpoint', async () => {
+        await resetExtension();
+        logger.error.mockClear();
+        MQTT.events.message('zigbee2mqtt/bridge/group/group_1/add', 'bulb_color/not_existing_endpoint');
+        await flushPromises();
+        expect(logger.error).toHaveBeenCalledWith("Device 'bulb_color' does not have endpoint 'not_existing_endpoint'");
+    });
+
     it('Should publish group state change when a device in it changes state', async () => {
         const device = zigbeeHerdsman.devices.bulb_color;
         const endpoint = device.getEndpoint(1);
@@ -869,7 +877,7 @@ describe('Groups', () => {
         );
     });
 
-    it('Error when adding to non-existing device', async () => {
+    it('Error when adding a non-existing device', async () => {
         await resetExtension();
         logger.error.mockClear();
         MQTT.publish.mockClear();
@@ -883,6 +891,20 @@ describe('Groups', () => {
         );
     });
 
+    it('Error when adding a non-existing endpoint', async () => {
+        await resetExtension();
+        logger.error.mockClear();
+        MQTT.publish.mockClear();
+        MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group_1', device: 'bulb_color/not_existing_endpoint'}));
+        await flushPromises();
+        expect(MQTT.publish).not.toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function));
+        expect(MQTT.publish).toHaveBeenCalledWith(
+            'zigbee2mqtt/bridge/response/group/members/add',
+            stringify({"data":{"device":"bulb_color/not_existing_endpoint","group":"group_1"},"status":"error","error":"Device 'bulb_color' does not have endpoint 'not_existing_endpoint'"}),
+            {retain: false, qos: 0}, expect.any(Function)
+        );
+    });
+
     it('Should only include relevant properties when publishing member states', async () => {
         const bulbColor = zigbeeHerdsman.devices.bulb_color;
         const bulbColorTemp = zigbeeHerdsman.devices.bulb;