Skip to content

Commit 81335e5

Browse files
grafalex82Koenkk
grafalex82
and
Koenkk
authored
fix: Remove dependency on predefined list of endpoints (parseEntityID() 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 <[email protected]>
1 parent 55653c9 commit 81335e5

File tree

9 files changed

+180
-35
lines changed

9 files changed

+180
-35
lines changed

‎lib/extension/bind.ts

+10-7
Original file line numberDiff line numberDiff line change
@@ -222,25 +222,28 @@ export default class Bind extends Extension {
222222
const message = utils.parseJSON(data.message, data.message);
223223

224224
let error = null;
225-
const parsedSource = utils.parseEntityID(sourceKey);
226-
const parsedTarget = utils.parseEntityID(targetKey);
227-
const source = this.zigbee.resolveEntity(parsedSource.ID);
228-
const target = targetKey === 'default_bind_group' ?
229-
defaultBindGroup : this.zigbee.resolveEntity(parsedTarget.ID);
225+
const parsedSource = this.zigbee.resolveEntityAndEndpoint(sourceKey);
226+
const parsedTarget = this.zigbee.resolveEntityAndEndpoint(targetKey);
227+
const source = parsedSource.entity;
228+
const target = targetKey === 'default_bind_group' ? defaultBindGroup : parsedTarget.entity;
230229
const responseData: KeyValue = {from: sourceKey, to: targetKey};
231230

232231
if (!source || !(source instanceof Device)) {
233232
error = `Source device '${sourceKey}' does not exist`;
233+
} else if (parsedSource.endpointID && !parsedSource.endpoint) {
234+
error = `Source device '${parsedSource.ID}' does not have endpoint '${parsedSource.endpointID}'`;
234235
} else if (!target) {
235236
error = `Target device or group '${targetKey}' does not exist`;
237+
} else if (target instanceof Device && parsedTarget.endpointID && !parsedTarget.endpoint) {
238+
error = `Target device '${parsedTarget.ID}' does not have endpoint '${parsedTarget.endpointID}'`;
236239
} else {
237240
const successfulClusters: string[] = [];
238241
const failedClusters = [];
239242
const attemptedClusters = [];
240243

241-
const bindSource: zh.Endpoint = source.endpoint(parsedSource.endpoint);
244+
const bindSource: zh.Endpoint = parsedSource.endpoint;
242245
let bindTarget: number | zh.Group | zh.Endpoint = null;
243-
if (target instanceof Device) bindTarget = target.endpoint(parsedTarget.endpoint);
246+
if (target instanceof Device) bindTarget = parsedTarget.endpoint;
244247
else if (target instanceof Group) bindTarget = target.zh;
245248
else bindTarget = Number(target.ID);
246249

‎lib/extension/bridge.ts

+10-4
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,13 @@ export default class Bridge extends Extension {
430430
throw new Error(`Invalid payload`);
431431
}
432432

433-
const parsedID = utils.parseEntityID(message.id);
434-
const endpoint = (this.getEntity('device', parsedID.ID) as Device).endpoint(parsedID.endpoint);
433+
const device = this.zigbee.resolveEntityAndEndpoint(message.id);
434+
if (!device.entity) throw new Error(`Device '${message.id}' does not exist`);
435+
436+
const endpoint = device.endpoint;
437+
if (device.endpointID && !endpoint) {
438+
throw new Error(`Device '${device.ID}' does not have endpoint '${device.endpointID}'`);
439+
}
435440

436441
const coordinatorEndpoint = this.zigbee.firstCoordinatorEndpoint();
437442
await endpoint.bind(message.cluster, coordinatorEndpoint);
@@ -457,8 +462,9 @@ export default class Bridge extends Extension {
457462
throw new Error(`Invalid payload`);
458463
}
459464

460-
const parsedID = utils.parseEntityID(message.id);
461-
const device = this.getEntity('device', parsedID.ID) as Device;
465+
const device = this.zigbee.resolveEntityAndEndpoint(message.id).entity as Device;
466+
if (!device) throw new Error(`Device '${message.id}' does not exist`);
467+
462468
const source = await zhc.generateExternalDefinitionSource(device.zh);
463469

464470
return utils.getResponse(message, {id: message.id, source}, null);

‎lib/extension/groups.ts

+20-9
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,13 @@ export default class Groups extends Extension {
6868
const groupID = settingGroup.ID;
6969
const zigbeeGroup = zigbeeGroups.find((g) => g.ID === groupID) || this.zigbee.createGroup(groupID);
7070
const settingsEndpoint = settingGroup.devices.map((d) => {
71-
const parsed = utils.parseEntityID(d);
72-
const entity = this.zigbee.resolveEntity(parsed.ID) as Device;
71+
const parsed = this.zigbee.resolveEntityAndEndpoint(d);
72+
const entity = parsed.entity as Device;
7373
if (!entity) logger.error(`Cannot find '${d}' of group '${settingGroup.friendly_name}'`);
74-
return {'endpoint': entity?.endpoint(parsed.endpoint), 'name': entity?.name};
74+
if (parsed.endpointID && !parsed.endpoint) {
75+
logger.error(`Cannot find endpoint '${parsed.endpointID}' of device '${parsed.ID}'`);
76+
}
77+
return {'endpoint': parsed.endpoint, 'name': entity?.name};
7578
}).filter((e) => e.endpoint != null);
7679

7780
// In settings but not in zigbee
@@ -238,8 +241,8 @@ export default class Groups extends Extension {
238241
type = 'remove_all';
239242
}
240243

241-
const parsedEntity = utils.parseEntityID(data.message);
242-
resolvedEntityDevice = this.zigbee.resolveEntity(parsedEntity.ID) as Device;
244+
const parsedEntity = this.zigbee.resolveEntityAndEndpoint(data.message);
245+
resolvedEntityDevice = parsedEntity.entity as Device;
243246
if (!resolvedEntityDevice || !(resolvedEntityDevice instanceof Device)) {
244247
logger.error(`Device '${data.message}' does not exist`);
245248

@@ -256,7 +259,12 @@ export default class Groups extends Extension {
256259

257260
return null;
258261
}
259-
resolvedEntityEndpoint = resolvedEntityDevice.endpoint(parsedEntity.endpoint);
262+
263+
resolvedEntityEndpoint = parsedEntity.endpoint;
264+
if (parsedEntity.endpointID && !resolvedEntityEndpoint) {
265+
logger.error(`Device '${parsedEntity.ID}' does not have endpoint '${parsedEntity.endpointID}'`);
266+
return null;
267+
}
260268
} else if (topicRegexMatch) {
261269
type = topicRegexMatch[1] as 'remove' | 'add' | 'remove_all';
262270
const message = JSON.parse(data.message);
@@ -271,13 +279,16 @@ export default class Groups extends Extension {
271279
}
272280
}
273281

274-
const parsed = utils.parseEntityID(message.device);
275-
resolvedEntityDevice = this.zigbee.resolveEntity(parsed.ID) as Device;
282+
const parsed = this.zigbee.resolveEntityAndEndpoint(message.device);
283+
resolvedEntityDevice = parsed?.entity as Device;
276284
if (!error && (!resolvedEntityDevice || !(resolvedEntityDevice instanceof Device))) {
277285
error = `Device '${message.device}' does not exist`;
278286
}
279287
if (!error) {
280-
resolvedEntityEndpoint = resolvedEntityDevice.endpoint(parsed.endpoint);
288+
resolvedEntityEndpoint = parsed.endpoint;
289+
if (parsed.endpointID && !resolvedEntityEndpoint) {
290+
error = `Device '${parsed.ID}' does not have endpoint '${parsed.endpointID}'`;
291+
}
281292
}
282293
}
283294

‎lib/extension/legacy/deviceGroupMembership.ts

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* istanbul ignore file */
22
import * as settings from '../../util/settings';
33
import logger from '../../util/logger';
4-
import utils from '../../util/utils';
54
import Extension from '../extension';
65
import bind from 'bind-decorator';
76
import Device from '../../model/device';
@@ -19,13 +18,19 @@ export default class DeviceGroupMembership extends Extension {
1918
return null;
2019
}
2120

22-
const parsed = utils.parseEntityID(match[1]);
23-
const device = this.zigbee.resolveEntity(parsed.ID) as Device;
21+
const parsed = this.zigbee.resolveEntityAndEndpoint(match[1]);
22+
const device = parsed?.entity as Device;
2423
if (!device || !(device instanceof Device)) {
2524
logger.error(`Device '${match[1]}' does not exist`);
2625
return;
2726
}
28-
const endpoint = device.endpoint(parsed.endpoint);
27+
28+
const endpoint = parsed.endpoint;
29+
if (parsed.endpointID && !endpoint) {
30+
logger.error(`Device '${parsed.ID}' does not have endpoint '${parsed.endpointID}'`);
31+
return;
32+
}
33+
2934
const response = await endpoint.command(
3035
`genGroups`, 'getMembership', {groupcount: 0, grouplist: []}, {},
3136
);

‎lib/util/utils.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -309,12 +309,6 @@ function isAvailabilityEnabledForEntity(entity: Device | Group, settings: Settin
309309
return !blocklist.includes(entity.name) && !blocklist.includes(entity.ieeeAddr);
310310
}
311311

312-
const entityIDRegex = new RegExp(`^(.+?)(?:/(${endpointNames.join('|')}|\\d+))?$`);
313-
function parseEntityID(ID: string): {ID: string, endpoint: string} {
314-
const match = ID.match(entityIDRegex);
315-
return match && {ID: match[1], endpoint: match[2]};
316-
}
317-
318312
function isEndpoint(obj: unknown): obj is zh.Endpoint {
319313
return obj.constructor.name.toLowerCase() === 'endpoint';
320314
}
@@ -436,7 +430,7 @@ export default {
436430
endpointNames, capitalize, getZigbee2MQTTVersion, getDependencyVersion, formatDate, objectHasProperties,
437431
equalsPartial, getObjectProperty, getResponse, parseJSON, loadModuleFromText, loadModuleFromFile,
438432
removeNullPropertiesFromObject, toNetworkAddressHex, toSnakeCase,
439-
parseEntityID, isEndpoint, isZHGroup, hours, minutes, seconds, validateFriendlyName, sleep,
433+
isEndpoint, isZHGroup, hours, minutes, seconds, validateFriendlyName, sleep,
440434
sanitizeImageParameter, isAvailabilityEnabledForEntity, publishLastSeen, availabilityPayload,
441435
getAllFiles, filterProperties, flatten, arrayUnique, clone, computeSettingsToChange, getScenes,
442436
};

‎lib/zigbee.ts

+33
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import * as ZHEvents from 'zigbee-herdsman/dist/controller/events';
1111
import bind from 'bind-decorator';
1212
import {randomInt} from 'crypto';
1313

14+
const entityIDRegex = new RegExp(`^(.+?)(?:/(.+))?$`);
15+
1416
export default class Zigbee {
1517
private herdsman: Controller;
1618
private eventBus: EventBus;
@@ -283,6 +285,37 @@ export default class Zigbee {
283285
}
284286
}
285287

288+
resolveEntityAndEndpoint(ID: string)
289+
: {ID: string, entity: Device | Group, endpointID: string, endpoint: zh.Endpoint} {
290+
// This function matches the following entity formats:
291+
// device_name (just device name)
292+
// device_name/ep_name (device name and endpoint numeric ID or name)
293+
// device/name (device name with slashes)
294+
// device/name/ep_name (device name with slashes, and endpoint numeric ID or name)
295+
296+
// First split the input token by the latest slash
297+
const match = ID.match(entityIDRegex);
298+
299+
// Try to match 'device_name/endpoint' pattern
300+
let entityName = match[1];
301+
let deviceOrGroup = this.resolveEntity(match[1]);
302+
let endpointNameOrID = match[2];
303+
304+
// If 'device_name/endpoint' pattern does not match, perhaps this is device name with slashes
305+
if (!deviceOrGroup) {
306+
entityName = ID;
307+
deviceOrGroup = this.resolveEntity(ID);
308+
endpointNameOrID = null;
309+
}
310+
311+
// If the function returns non-null endpoint name, but the endpoint field is null, then
312+
// it means that endpoint was not matched because there is no such endpoint on the device
313+
// (or the entity is a group)
314+
const endpoint = deviceOrGroup?.isDevice() ? deviceOrGroup.endpoint(endpointNameOrID) : null;
315+
316+
return {ID: entityName, entity: deviceOrGroup, endpointID: endpointNameOrID, endpoint: endpoint};
317+
}
318+
286319
firstCoordinatorEndpoint(): zh.Endpoint {
287320
return this.herdsman.getDevicesByType('Coordinator')[0].endpoints[0];
288321
}

‎test/bind.test.js

+30-2
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ describe('Bind', () => {
387387
);
388388
});
389389

390-
it('Error bind fails when source not existing', async () => {
390+
it('Error bind fails when source device does not exist', async () => {
391391
const device = zigbeeHerdsman.devices.remote;
392392
const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
393393
const endpoint = device.getEndpoint(1);
@@ -401,7 +401,21 @@ describe('Bind', () => {
401401
);
402402
});
403403

404-
it('Error bind fails when target not existing', async () => {
404+
it("Error bind fails when source device's endpoint does not exist", async () => {
405+
const device = zigbeeHerdsman.devices.remote;
406+
const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
407+
const endpoint = device.getEndpoint(1);
408+
mockClear(device);
409+
MQTT.events.message('zigbee2mqtt/bridge/request/device/bind', stringify({from: 'remote/not_existing_endpoint', to: 'bulb_color'}));
410+
await flushPromises();
411+
expect(MQTT.publish).toHaveBeenCalledWith(
412+
'zigbee2mqtt/bridge/response/device/bind',
413+
stringify({"data":{"from":"remote/not_existing_endpoint","to":"bulb_color"},"status":"error","error":"Source device 'remote' does not have endpoint 'not_existing_endpoint'"}),
414+
{retain: false, qos: 0}, expect.any(Function)
415+
);
416+
});
417+
418+
it('Error bind fails when target device or group does not exist', async () => {
405419
const device = zigbeeHerdsman.devices.remote;
406420
const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
407421
const endpoint = device.getEndpoint(1);
@@ -415,6 +429,20 @@ describe('Bind', () => {
415429
);
416430
});
417431

432+
it("Error bind fails when target device's endpoint does not exist", async () => {
433+
const device = zigbeeHerdsman.devices.remote;
434+
const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);
435+
const endpoint = device.getEndpoint(1);
436+
mockClear(device);
437+
MQTT.events.message('zigbee2mqtt/bridge/request/device/bind', stringify({from: 'remote', to: 'bulb_color/not_existing_endpoint'}));
438+
await flushPromises();
439+
expect(MQTT.publish).toHaveBeenCalledWith(
440+
'zigbee2mqtt/bridge/response/device/bind',
441+
stringify({"data":{"from":"remote","to":"bulb_color/not_existing_endpoint"},"status":"error","error":"Target device 'bulb_color' does not have endpoint 'not_existing_endpoint'"}),
442+
{retain: false, qos: 0}, expect.any(Function)
443+
);
444+
});
445+
418446
it('Legacy api: Should bind', async () => {
419447
const device = zigbeeHerdsman.devices.remote;
420448
const target = zigbeeHerdsman.devices.bulb_color.getEndpoint(1);

‎test/bridge.test.js

+43
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,17 @@ describe('Bridge', () => {
681681
);
682682
});
683683

684+
it('Should error when generate_external_definition requested for unknown device', async () => {
685+
MQTT.publish.mockClear();
686+
MQTT.events.message('zigbee2mqtt/bridge/request/device/generate_external_definition', stringify({id: 'non_existing_device'}));
687+
await flushPromises();
688+
expect(MQTT.publish).toHaveBeenCalledWith(
689+
'zigbee2mqtt/bridge/response/device/generate_external_definition',
690+
stringify({ data: {}, error: "Device 'non_existing_device' does not exist", status: 'error' }),
691+
{retain: false, qos: 0}, expect.any(Function)
692+
);
693+
});
694+
684695
it('Should allow to generate device definition', async () => {
685696
MQTT.publish.mockClear();
686697
MQTT.events.message('zigbee2mqtt/bridge/request/device/generate_external_definition', stringify({id: ZNCZ02LM.ieeeAddr}));
@@ -1132,6 +1143,38 @@ describe('Bridge', () => {
11321143
);
11331144
});
11341145

1146+
it('Should throw error when configure reporting is called for non-existing device', async () => {
1147+
const device = zigbeeHerdsman.devices.bulb;
1148+
const endpoint = device.getEndpoint(1);
1149+
endpoint.configureReporting.mockClear();
1150+
zigbeeHerdsman.permitJoin.mockClear();
1151+
MQTT.publish.mockClear();
1152+
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}));
1153+
await flushPromises();
1154+
expect(endpoint.configureReporting).toHaveBeenCalledTimes(0);
1155+
expect(MQTT.publish).toHaveBeenCalledWith(
1156+
'zigbee2mqtt/bridge/response/device/configure_reporting',
1157+
stringify({"data":{},"status":"error","error":"Device 'non_existing_device' does not exist"}),
1158+
{retain: false, qos: 0}, expect.any(Function)
1159+
);
1160+
});
1161+
1162+
it('Should throw error when configure reporting is called for non-existing endpoint', async () => {
1163+
const device = zigbeeHerdsman.devices.bulb;
1164+
const endpoint = device.getEndpoint(1);
1165+
endpoint.configureReporting.mockClear();
1166+
zigbeeHerdsman.permitJoin.mockClear();
1167+
MQTT.publish.mockClear();
1168+
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}));
1169+
await flushPromises();
1170+
expect(endpoint.configureReporting).toHaveBeenCalledTimes(0);
1171+
expect(MQTT.publish).toHaveBeenCalledWith(
1172+
'zigbee2mqtt/bridge/response/device/configure_reporting',
1173+
stringify({"data":{},"status":"error","error":"Device '0x000b57fffec6a5b2' does not have endpoint 'non_existing_endpoint'"}),
1174+
{retain: false, qos: 0}, expect.any(Function)
1175+
);
1176+
});
1177+
11351178
it('Should allow to create a backup', async () => {
11361179
fs.mkdirSync(path.join(data.mockDir, 'ext_converters'));
11371180
fs.writeFileSync(path.join(data.mockDir, 'ext_converters', 'afile.js'), 'test123')

‎test/group.test.js

+24-2
Original file line numberDiff line numberDiff line change
@@ -273,14 +273,22 @@ describe('Groups', () => {
273273
expect(logger.error).toHaveBeenCalledWith("Group 'group_1_not_existing' does not exist");
274274
});
275275

276-
it('Legacy api: Log when adding to non-existing device', async () => {
276+
it('Legacy api: Log when adding a non-existing device', async () => {
277277
await resetExtension();
278278
logger.error.mockClear();
279279
MQTT.events.message('zigbee2mqtt/bridge/group/group_1/add', 'bulb_color_not_existing');
280280
await flushPromises();
281281
expect(logger.error).toHaveBeenCalledWith("Device 'bulb_color_not_existing' does not exist");
282282
});
283283

284+
it('Legacy api: Log when adding a non-existing endpoint', async () => {
285+
await resetExtension();
286+
logger.error.mockClear();
287+
MQTT.events.message('zigbee2mqtt/bridge/group/group_1/add', 'bulb_color/not_existing_endpoint');
288+
await flushPromises();
289+
expect(logger.error).toHaveBeenCalledWith("Device 'bulb_color' does not have endpoint 'not_existing_endpoint'");
290+
});
291+
284292
it('Should publish group state change when a device in it changes state', async () => {
285293
const device = zigbeeHerdsman.devices.bulb_color;
286294
const endpoint = device.getEndpoint(1);
@@ -869,7 +877,7 @@ describe('Groups', () => {
869877
);
870878
});
871879

872-
it('Error when adding to non-existing device', async () => {
880+
it('Error when adding a non-existing device', async () => {
873881
await resetExtension();
874882
logger.error.mockClear();
875883
MQTT.publish.mockClear();
@@ -883,6 +891,20 @@ describe('Groups', () => {
883891
);
884892
});
885893

894+
it('Error when adding a non-existing endpoint', async () => {
895+
await resetExtension();
896+
logger.error.mockClear();
897+
MQTT.publish.mockClear();
898+
MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group_1', device: 'bulb_color/not_existing_endpoint'}));
899+
await flushPromises();
900+
expect(MQTT.publish).not.toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function));
901+
expect(MQTT.publish).toHaveBeenCalledWith(
902+
'zigbee2mqtt/bridge/response/group/members/add',
903+
stringify({"data":{"device":"bulb_color/not_existing_endpoint","group":"group_1"},"status":"error","error":"Device 'bulb_color' does not have endpoint 'not_existing_endpoint'"}),
904+
{retain: false, qos: 0}, expect.any(Function)
905+
);
906+
});
907+
886908
it('Should only include relevant properties when publishing member states', async () => {
887909
const bulbColor = zigbeeHerdsman.devices.bulb_color;
888910
const bulbColorTemp = zigbeeHerdsman.devices.bulb;

0 commit comments

Comments
 (0)