From 13ac8a0f539ece1ccc9f67ef4a8e5dd9bc8de09f Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Thu, 8 Aug 2024 20:21:40 +0200 Subject: [PATCH] fix: Improve looping performance (#23541) * Use herdsman improved looping. * Fix lint. * Fix tests. * Feedback --- lib/controller.ts | 12 +- lib/extension/availability.ts | 6 +- lib/extension/bind.ts | 4 +- lib/extension/bridge.ts | 57 ++++--- lib/extension/configure.ts | 5 +- lib/extension/frontend.ts | 2 +- lib/extension/groups.ts | 29 ++-- lib/extension/homeassistant.ts | 10 +- lib/extension/legacy/bridgeLegacy.ts | 8 +- lib/extension/legacy/report.ts | 3 +- lib/extension/networkMap.ts | 70 +++++--- lib/extension/onEvent.ts | 6 +- lib/extension/otaUpdate.ts | 6 +- lib/extension/publish.ts | 2 +- lib/extension/receive.ts | 2 +- lib/util/utils.ts | 12 ++ lib/zigbee.ts | 44 +++-- test/controller.test.js | 2 +- test/networkMap.test.js | 246 +++++++++++++++++++++------ test/stub/zigbeeHerdsman.js | 20 ++- test/utils.test.js | 5 + 21 files changed, 387 insertions(+), 164 deletions(-) diff --git a/lib/controller.ts b/lib/controller.ts index 547f4cf30..4d203b0bc 100644 --- a/lib/controller.ts +++ b/lib/controller.ts @@ -157,15 +157,19 @@ export class Controller { } // Log zigbee clients on startup - const devices = this.zigbee.devices(false); - logger.info(`Currently ${devices.length} devices are joined:`); - for (const device of devices) { + let deviceCount = 0; + + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { const model = device.isSupported ? `${device.definition.model} - ${device.definition.vendor} ${device.definition.description}` : 'Not supported'; logger.info(`${device.name} (${device.ieeeAddr}): ${model} (${device.zh.type})`); + + deviceCount++; } + logger.info(`Currently ${deviceCount} devices are joined.`); + // Enable zigbee join try { if (settings.get().permit_join) { @@ -193,7 +197,7 @@ export class Controller { // Send all cached states. if (settings.get().advanced.cache_state_send_on_startup && settings.get().advanced.cache_state) { - for (const entity of [...devices, ...this.zigbee.groups()]) { + for (const entity of this.zigbee.devicesAndGroupsIterator()) { if (this.state.exists(entity)) { await this.publishEntityState(entity, this.state.get(entity), 'publishCached'); } diff --git a/lib/extension/availability.ts b/lib/extension/availability.ts index 22a7c0ebf..07c0046da 100644 --- a/lib/extension/availability.ts +++ b/lib/extension/availability.ts @@ -140,7 +140,7 @@ export default class Availability extends Extension { await this.publishAvailabilityForAllEntities(); // Start availability for the devices - for (const device of this.zigbee.devices(false)) { + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { if (utils.isAvailabilityEnabledForEntity(device, settings.get())) { this.resetTimer(device); @@ -153,7 +153,7 @@ export default class Availability extends Extension { } @bind private async publishAvailabilityForAllEntities(): Promise { - for (const entity of [...this.zigbee.devices(false), ...this.zigbee.groups()]) { + for (const entity of this.zigbee.devicesAndGroupsIterator(utils.deviceNotCoordinator)) { if (utils.isAvailabilityEnabledForEntity(entity, settings.get())) { await this.publishAvailability(entity, true, false, true); } @@ -187,7 +187,7 @@ export default class Availability extends Extension { await this.mqtt.publish(topic, payload, {retain: true, qos: 1}); if (!skipGroups && entity.isDevice()) { - for (const group of this.zigbee.groups()) { + for (const group of this.zigbee.groupsIterator()) { if (group.hasMember(entity) && utils.isAvailabilityEnabledForEntity(group, settings.get())) { await this.publishAvailability(group, false, forcePublish); } diff --git a/lib/extension/bind.ts b/lib/extension/bind.ts index 751dfe20d..d8ac3dadc 100755 --- a/lib/extension/bind.ts +++ b/lib/extension/bind.ts @@ -363,7 +363,7 @@ export default class Bind extends Extension { if (data.action === 'add') { const bindsToGroup: zh.Bind[] = []; - for (const device of this.zigbee.devices(false)) { + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { for (const endpoint of device.zh.endpoints) { for (const bind of endpoint.binds) { if (bind.target === data.group.zh) { @@ -443,7 +443,7 @@ export default class Bind extends Extension { const endpoints = utils.isEndpoint(target) ? [target] : target.members; const allBinds: zh.Bind[] = []; - for (const device of this.zigbee.devices(false)) { + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { for (const endpoint of device.zh.endpoints) { for (const bind of endpoint.binds) { allBinds.push(bind); diff --git a/lib/extension/bridge.ts b/lib/extension/bridge.ts index d8d841e93..4c33cffdd 100644 --- a/lib/extension/bridge.ts +++ b/lib/extension/bridge.ts @@ -725,8 +725,12 @@ export default class Bridge extends Extension { scenes: Scene[]; } - const devices = this.zigbee.devices().map((device) => { + // XXX: definition<>DefinitionPayload don't match to use `Device[]` type here + const devices: KeyValue[] = []; + + for (const device of this.zigbee.devicesIterator()) { const endpoints: {[s: number]: Data} = {}; + for (const endpoint of device.zh.endpoints) { const data: Data = { scenes: utils.getScenes(endpoint), @@ -758,7 +762,7 @@ export default class Bridge extends Extension { endpoints[endpoint.ID] = data; } - return { + devices.push({ ieee_address: device.ieeeAddr, type: device.zh.type, network_address: device.zh.networkAddress, @@ -775,24 +779,32 @@ export default class Bridge extends Extension { interview_completed: device.zh.interviewCompleted, manufacturer: device.zh.manufacturerName, endpoints, - }; - }); + }); + } await this.mqtt.publish('bridge/devices', stringify(devices), {retain: true, qos: 0}, settings.get().mqtt.base_topic, true); } async publishGroups(): Promise { - const groups = this.zigbee.groups().map((g) => { - return { - id: g.ID, - friendly_name: g.ID === 901 ? 'default_bind_group' : g.name, - description: g.options.description, - scenes: utils.getScenes(g.zh), - members: g.zh.members.map((e) => { - return {ieee_address: e.getDevice().ieeeAddr, endpoint: e.ID}; - }), - }; - }); + // XXX: id<>ID can't use `Group[]` type + const groups: KeyValue[] = []; + + for (const group of this.zigbee.groupsIterator()) { + const members = []; + + for (const member of group.zh.members) { + members.push({ieee_address: member.getDevice().ieeeAddr, endpoint: member.ID}); + } + + groups.push({ + id: group.ID, + friendly_name: group.ID === 901 ? 'default_bind_group' : group.name, + description: group.options.description, + scenes: utils.getScenes(group.zh), + members, + }); + } + await this.mqtt.publish('bridge/groups', stringify(groups), {retain: true, qos: 0}, settings.get().mqtt.base_topic, true); } @@ -807,20 +819,23 @@ export default class Bridge extends Extension { custom_clusters: {}, }; - for (const device of this.zigbee.devices()) { - if (Object.keys(device.customClusters).length !== 0) { - data.custom_clusters[device.ieeeAddr] = device.customClusters; - } + for (const device of this.zigbee.devicesIterator((d) => !utils.objectIsEmpty(d.customClusters))) { + data.custom_clusters[device.ieeeAddr] = device.customClusters; } await this.mqtt.publish('bridge/definitions', stringify(data), {retain: true, qos: 0}, settings.get().mqtt.base_topic, true); } - getDefinitionPayload(device: Device): DefinitionPayload { - if (!device.definition) return null; + getDefinitionPayload(device: Device): DefinitionPayload | null { + if (!device.definition) { + return null; + } + + // TODO: better typing to avoid @ts-expect-error // @ts-expect-error icon is valid for external definitions const definitionIcon = device.definition.icon; let icon = device.options.icon ?? definitionIcon; + if (icon) { icon = icon.replace('${zigbeeModel}', utils.sanitizeImageParameter(device.zh.modelID)); icon = icon.replace('${model}', utils.sanitizeImageParameter(device.definition.model)); diff --git a/lib/extension/configure.ts b/lib/extension/configure.ts index 2f1e0c995..f5257a3fb 100644 --- a/lib/extension/configure.ts +++ b/lib/extension/configure.ts @@ -68,9 +68,8 @@ export default class Configure extends Extension { setImmediate(async () => { // Only configure routers on startup, end devices are likely sleeping and // will reconfigure once they send a message - for (const device of this.zigbee.devices(false).filter((d) => d.zh.type === 'Router')) { - // Sleep 10 seconds between configuring on startup to not DDoS the coordinator - // when many devices have to be configured. + for (const device of this.zigbee.devicesIterator((d) => d.type === 'Router')) { + // Sleep 10 seconds between configuring on startup to not DDoS the coordinator when many devices have to be configured. await utils.sleep(10); await this.configure(device, 'started'); } diff --git a/lib/extension/frontend.ts b/lib/extension/frontend.ts index 643abba3b..e46d35b70 100644 --- a/lib/extension/frontend.ts +++ b/lib/extension/frontend.ts @@ -139,7 +139,7 @@ export default class Frontend extends Extension { } } - for (const device of this.zigbee.devices(false)) { + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { const payload = this.state.get(device); const lastSeen = settings.get().advanced.last_seen; /* istanbul ignore if */ diff --git a/lib/extension/groups.ts b/lib/extension/groups.ts index ea60ad825..21126bfca 100644 --- a/lib/extension/groups.ts +++ b/lib/extension/groups.ts @@ -51,7 +51,6 @@ export default class Groups extends Extension { private async syncGroupsWithSettings(): Promise { const settingsGroups = settings.getGroups(); - const zigbeeGroups = this.zigbee.groups(); const addRemoveFromGroup = async ( action: 'add' | 'remove', @@ -76,7 +75,7 @@ export default class Groups extends Extension { for (const settingGroup of settingsGroups) { const groupID = settingGroup.ID; - const zigbeeGroup = zigbeeGroups.find((g) => g.ID === groupID) || this.zigbee.createGroup(groupID); + const zigbeeGroup = this.zigbee.groupsIterator((g) => g.groupID === groupID).next().value || this.zigbee.createGroup(groupID); const settingsEndpoints: zh.Endpoint[] = []; for (const d of settingGroup.devices) { @@ -113,13 +112,11 @@ export default class Groups extends Extension { } } - for (const zigbeeGroup of zigbeeGroups) { - if (!settingsGroups.some((g) => g.ID === zigbeeGroup.ID)) { - for (const endpoint of zigbeeGroup.zh.members) { - const deviceName = settings.getDevice(endpoint.getDevice().ieeeAddr).friendly_name; + for (const zigbeeGroup of this.zigbee.groupsIterator((zg) => !settingsGroups.some((sg) => sg.ID === zg.groupID))) { + for (const endpoint of zigbeeGroup.zh.members) { + const deviceName = settings.getDevice(endpoint.getDevice().ieeeAddr).friendly_name; - await addRemoveFromGroup('remove', deviceName, zigbeeGroup.ID, endpoint, zigbeeGroup); - } + await addRemoveFromGroup('remove', deviceName, zigbeeGroup.ID, endpoint, zigbeeGroup); } } } @@ -153,7 +150,13 @@ export default class Groups extends Extension { if (payloadKeys.length) { const entity = data.entity; - const groups = this.zigbee.groups().filter((g) => g.options && (g.options.optimistic == undefined || g.options.optimistic)); + const groups = []; + + for (const group of this.zigbee.groupsIterator()) { + if (group.options && (group.options.optimistic == undefined || group.options.optimistic)) { + groups.push(group); + } + } if (entity instanceof Device) { for (const group of groups) { @@ -359,7 +362,7 @@ export default class Groups extends Extension { resolvedEntityEndpoint, } = parsed; let error = parsed.error; - let changedGroups: Group[] = []; + const changedGroups: Group[] = []; if (!error) { try { @@ -406,7 +409,11 @@ export default class Groups extends Extension { } else { // remove_all logger.info(`Removing '${resolvedEntityDevice.name}' from all groups`); - changedGroups = this.zigbee.groups().filter((g) => g.zh.members.includes(resolvedEntityEndpoint)); + + for (const group of this.zigbee.groupsIterator((g) => g.members.includes(resolvedEntityEndpoint))) { + changedGroups.push(group); + } + await resolvedEntityEndpoint.removeFromAllGroups(); for (const settingsGroup of settings.getGroups()) { diff --git a/lib/extension/homeassistant.ts b/lib/extension/homeassistant.ts index 17da4524f..14a73238e 100644 --- a/lib/extension/homeassistant.ts +++ b/lib/extension/homeassistant.ts @@ -225,7 +225,9 @@ export default class HomeAssistant extends Extension { 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()` - for (const e of [this.bridge, ...this.zigbee.devices(false), ...this.zigbee.groups()]) { + await this.discover(this.bridge, false); + + for (const e of this.zigbee.devicesAndGroupsIterator(utils.deviceNotCoordinator)) { await this.discover(e, false); } @@ -235,7 +237,9 @@ export default class HomeAssistant extends Extension { this.mqtt.unsubscribe(`${this.discoveryTopic}/#`); logger.debug(`Discovering entities to Home Assistant`); - for (const e of [this.bridge, ...this.zigbee.devices(false), ...this.zigbee.groups()]) { + await this.discover(this.bridge); + + for (const e of this.zigbee.devicesAndGroupsIterator(utils.deviceNotCoordinator)) { await this.discover(e); } }, utils.seconds(discoverWait)); @@ -1780,7 +1784,7 @@ export default class HomeAssistant extends Extension { } else if ((data.topic === this.statusTopic || data.topic === defaultStatusTopic) && data.message.toLowerCase() === 'online') { const timer = setTimeout(async () => { // Publish all device states. - for (const entity of [...this.zigbee.devices(false), ...this.zigbee.groups()]) { + for (const entity of this.zigbee.devicesAndGroupsIterator(utils.deviceNotCoordinator)) { if (this.state.exists(entity)) { await this.publishEntityState(entity, this.state.get(entity), 'publishCached'); } diff --git a/lib/extension/legacy/bridgeLegacy.ts b/lib/extension/legacy/bridgeLegacy.ts index 41e061b41..b3b8d5b84 100644 --- a/lib/extension/legacy/bridgeLegacy.ts +++ b/lib/extension/legacy/bridgeLegacy.ts @@ -127,7 +127,9 @@ export default class BridgeLegacy extends Extension { @bind async devices(topic: string): Promise { const coordinator = await this.zigbee.getCoordinatorVersion(); - const devices = this.zigbee.devices().map((device) => { + const devices: KeyValue[] = []; + + for (const device of this.zigbee.devicesIterator()) { const payload: KeyValue = { ieeeAddr: device.ieeeAddr, type: device.zh.type, @@ -155,8 +157,8 @@ export default class BridgeLegacy extends Extension { payload.lastSeen = Date.now(); } - return payload; - }); + devices.push(payload); + } if (topic.split('/').pop() == 'get') { await this.mqtt.publish(`bridge/config/devices`, stringify(devices), {}, settings.get().mqtt.base_topic, false, false); diff --git a/lib/extension/legacy/report.ts b/lib/extension/legacy/report.ts index f6b05b827..51289da44 100644 --- a/lib/extension/legacy/report.ts +++ b/lib/extension/legacy/report.ts @@ -2,6 +2,7 @@ import * as zhc from 'zigbee-herdsman-converters'; import logger from '../../util/logger'; import * as settings from '../../util/settings'; +import utils from '../../util/utils'; import Extension from '../extension'; const defaultConfiguration = { @@ -178,7 +179,7 @@ export default class Report extends Extension { } override async start(): Promise { - for (const device of this.zigbee.devices(false)) { + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { if (this.shouldSetupReporting(device, null)) { await this.setupReporting(device); } diff --git a/lib/extension/networkMap.ts b/lib/extension/networkMap.ts index a0a29ff3f..81545f9fa 100644 --- a/lib/extension/networkMap.ts +++ b/lib/extension/networkMap.ts @@ -210,31 +210,31 @@ export default class NetworkMap extends Extension { async networkScan(includeRoutes: boolean): Promise { logger.info(`Starting network scan (includeRoutes '${includeRoutes}')`); - const devices = this.zigbee.devices().filter((d) => d.zh.type !== 'GreenPower' && !d.options.disabled); const lqis: Map = new Map(); const routingTables: Map = new Map(); const failed: Map = new Map(); + const requestWithRetry = async (request: () => Promise): Promise => { + try { + const result = await request(); + + return result; + } catch { + // Network is possibly congested, sleep 5 seconds to let the network settle. + await utils.sleep(5); + return request(); + } + }; + + for (const device of this.zigbee.devicesIterator((d) => d.type !== 'GreenPower' && d.type !== 'EndDevice')) { + if (device.options.disabled) { + continue; + } - for (const device of devices.filter((d) => d.zh.type != 'EndDevice')) { failed.set(device, []); await utils.sleep(1); // sleep 1 second between each scan to reduce stress on network. - const doRequest = async (request: () => Promise, firstAttempt = true): Promise => { - try { - return await request(); - } catch (error) { - if (!firstAttempt) { - throw error; - } else { - // Network is possibly congested, sleep 5 seconds to let the network settle. - await utils.sleep(5); - return doRequest(request, false); - } - } - }; - try { - const result = await doRequest(async () => device.zh.lqi()); + const result = await requestWithRetry(async () => device.zh.lqi()); lqis.set(device, result); logger.debug(`LQI succeeded for '${device.name}'`); } catch (error) { @@ -245,12 +245,13 @@ export default class NetworkMap extends Extension { if (includeRoutes) { try { - const result = await doRequest(async () => device.zh.routingTable()); + const result = await requestWithRetry(async () => device.zh.routingTable()); routingTables.set(device, result); logger.debug(`Routing table succeeded for '${device.name}'`); } catch (error) { failed.get(device).push('routingTable'); - logger.error(`Failed to execute routing table for '${device.name}' (${error.message})`); + logger.error(`Failed to execute routing table for '${device.name}'`); + logger.debug(error.stack); } } } @@ -258,8 +259,14 @@ export default class NetworkMap extends Extension { logger.info(`Network scan finished`); const topology: Topology = {nodes: [], links: []}; - // Add nodes - for (const device of devices) { + + // XXX: display GP/disabled devices in the map, better feedback than just hiding them? + for (const device of this.zigbee.devicesIterator((d) => d.type !== 'GreenPower')) { + if (device.options.disabled) { + continue; + } + + // Add nodes const definition = device.definition ? { model: device.definition.model, @@ -289,7 +296,7 @@ export default class NetworkMap extends Extension { } // Add links - lqis.forEach((lqi, device) => { + for (const [device, lqi] of lqis) { for (const neighbor of lqi.neighbors) { if (neighbor.relationship > 3) { // Relationship is not active, skip it @@ -298,9 +305,13 @@ export default class NetworkMap extends Extension { // Some Xiaomi devices return 0x00 as the neighbor ieeeAddr (obviously not correct). // Determine the correct ieeeAddr based on the networkAddress. - const neighborDevice = this.zigbee.deviceByNetworkAddress(neighbor.networkAddress); - if (neighbor.ieeeAddr === '0x0000000000000000' && neighborDevice) { - neighbor.ieeeAddr = neighborDevice.ieeeAddr; + if (neighbor.ieeeAddr === '0x0000000000000000') { + const neighborDevice = this.zigbee.deviceByNetworkAddress(neighbor.networkAddress); + + /* istanbul ignore else */ + if (neighborDevice) { + neighbor.ieeeAddr = neighborDevice.ieeeAddr; + } } const link: Link = { @@ -318,13 +329,18 @@ export default class NetworkMap extends Extension { }; const routingTable = routingTables.get(device); + if (routingTable) { - link.routes = routingTable.table.filter((t) => t.status === 'ACTIVE' && t.nextHop === neighbor.networkAddress); + for (const entry of routingTable.table) { + if (entry.status === 'ACTIVE' && entry.nextHop === neighbor.networkAddress) { + link.routes.push(entry); + } + } } topology.links.push(link); } - }); + } return topology; } diff --git a/lib/extension/onEvent.ts b/lib/extension/onEvent.ts index f8d100714..f666195d9 100644 --- a/lib/extension/onEvent.ts +++ b/lib/extension/onEvent.ts @@ -1,5 +1,6 @@ import * as zhc from 'zigbee-herdsman-converters'; +import utils from '../util/utils'; import Extension from './extension'; /** @@ -7,7 +8,7 @@ import Extension from './extension'; */ export default class OnEvent extends Extension { override async start(): Promise { - for (const device of this.zigbee.devices(false)) { + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { await this.callOnEvent(device, 'start', {}); } @@ -31,7 +32,8 @@ export default class OnEvent extends Extension { override async stop(): Promise { await super.stop(); - for (const device of this.zigbee.devices(false)) { + + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { await this.callOnEvent(device, 'stop', {}); } } diff --git a/lib/extension/otaUpdate.ts b/lib/extension/otaUpdate.ts index a2a7b3d8a..0c41575be 100644 --- a/lib/extension/otaUpdate.ts +++ b/lib/extension/otaUpdate.ts @@ -65,10 +65,10 @@ export default class OTAUpdate extends Extension { // In order to support local firmware files we need to let zigbeeOTA know where the data directory is zhc.ota.setDataDir(dataDir.getPath()); - // In case Zigbee2MQTT is restared during an update, progress and remaining values are still in state. - // remove them. - for (const device of this.zigbee.devices(false)) { + // In case Zigbee2MQTT is restared during an update, progress and remaining values are still in state, remove them. + for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { this.removeProgressAndRemainingFromState(device); + // Reset update state, e.g. when Z2M restarted during update. if (this.state.get(device).update?.state === 'updating') { this.state.get(device).update.state = 'available'; diff --git a/lib/extension/publish.ts b/lib/extension/publish.ts index dfc082a20..b8899def1 100644 --- a/lib/extension/publish.ts +++ b/lib/extension/publish.ts @@ -317,7 +317,7 @@ export default class Publish extends Extension { } for (const [ID, payload] of Object.entries(toPublish)) { - if (Object.keys(payload).length != 0) { + if (!utils.objectIsEmpty(payload)) { await this.publishEntityState(toPublishEntity[ID], payload); } } diff --git a/lib/extension/receive.ts b/lib/extension/receive.ts index 9e65bf00d..2292eb991 100755 --- a/lib/extension/receive.ts +++ b/lib/extension/receive.ts @@ -163,7 +163,7 @@ export default class Receive extends Extension { } } - if (Object.keys(payload).length) { + if (!utils.objectIsEmpty(payload)) { await publish(payload); } else { await utils.publishLastSeen({device: data.device, reason: 'messageEmitted'}, settings.get(), true, this.publishEntityState); diff --git a/lib/util/utils.ts b/lib/util/utils.ts index 786a16853..1dc0966ca 100644 --- a/lib/util/utils.ts +++ b/lib/util/utils.ts @@ -92,6 +92,12 @@ function formatDate(time: number, type: 'ISO_8601' | 'ISO_8601_local' | 'epoch' } } +function objectIsEmpty(object: object): boolean { + // much faster than checking `Object.keys(object).length` + for (const k in object) return false; + return true; +} + function objectHasProperties(object: {[s: string]: unknown}, properties: string[]): boolean { for (const property of properties) { if (!object.hasOwnProperty(property)) { @@ -397,6 +403,10 @@ function getScenes(entity: zh.Endpoint | zh.Group): Scene[] { return Object.values(scenes); } +function deviceNotCoordinator(device: zh.Device): boolean { + return device.type !== 'Coordinator'; +} + /* istanbul ignore next */ const noop = (): void => {}; @@ -405,6 +415,7 @@ export default { getZigbee2MQTTVersion, getDependencyVersion, formatDate, + objectIsEmpty, objectHasProperties, equalsPartial, getObjectProperty, @@ -431,5 +442,6 @@ export default { flatten, arrayUnique, getScenes, + deviceNotCoordinator, noop, }; diff --git a/lib/zigbee.ts b/lib/zigbee.ts index 5182d6ba1..30bf1ff41 100644 --- a/lib/zigbee.ts +++ b/lib/zigbee.ts @@ -66,7 +66,7 @@ export default class Zigbee { throw error; } - for (const device of this.devices(false)) { + for (const device of this.devicesIterator(utils.deviceNotCoordinator)) { await device.resolveDefinition(); } @@ -124,7 +124,7 @@ export default class Zigbee { logger.info(`Coordinator firmware version: '${stringify(await this.getCoordinatorVersion())}'`); logger.debug(`Zigbee network parameters: ${stringify(await this.herdsman.getNetworkParameters())}`); - for (const device of this.devices(false)) { + for (const device of this.devicesIterator(utils.deviceNotCoordinator)) { // If a passlist is used, all other device will be removed from the network. const passlist = settings.get().passlist; const blocklist = settings.get().blocklist; @@ -135,19 +135,20 @@ export default class Zigbee { logger.error(`Failed to remove '${device.ieeeAddr}' (${error.message})`); } }; + if (passlist.length > 0) { if (!passlist.includes(device.ieeeAddr)) { - logger.warning(`Device which is not on passlist connected (${device.ieeeAddr}), removing...`); + logger.warning(`Device not on passlist currently connected (${device.ieeeAddr}), removing...`); await remove(device); } } else if (blocklist.includes(device.ieeeAddr)) { - logger.warning(`Device on blocklist is connected (${device.ieeeAddr}), removing...`); + logger.warning(`Device on blocklist currently connected (${device.ieeeAddr}), removing...`); await remove(device); } } // Check if we have to set a transmit power - if (settings.get().advanced.hasOwnProperty('transmit_power')) { + if (settings.get().advanced.transmit_power != null) { const transmitPower = settings.get().advanced.transmit_power; await this.herdsman.setTransmitPower(transmitPower); logger.info(`Set transmit power to '${transmitPower}'`); @@ -327,20 +328,29 @@ export default class Zigbee { return this.herdsman.getDevicesByType('Coordinator')[0].endpoints[0]; } - groups(): Group[] { - return this.herdsman.getGroups().map((g) => this.resolveGroup(g.groupID)); - } - - devices(includeCoordinator = true): Device[] { - const devices: Device[] = []; - - for (const device of this.herdsman.getDevices()) { - if (includeCoordinator || device.type !== 'Coordinator') { - devices.push(this.resolveDevice(device.ieeeAddr)); - } + *devicesAndGroupsIterator( + devicePredicate?: (value: zh.Device) => boolean, + groupPredicate?: (value: zh.Group) => boolean, + ): Generator { + for (const device of this.herdsman.getDevicesIterator(devicePredicate)) { + yield this.resolveDevice(device.ieeeAddr); } - return devices; + for (const group of this.herdsman.getGroupsIterator(groupPredicate)) { + yield this.resolveGroup(group.groupID); + } + } + + *groupsIterator(predicate?: (value: zh.Group) => boolean): Generator { + for (const group of this.herdsman.getGroupsIterator(predicate)) { + yield this.resolveGroup(group.groupID); + } + } + + *devicesIterator(predicate?: (value: zh.Device) => boolean): Generator { + for (const device of this.herdsman.getDevicesIterator(predicate)) { + yield this.resolveDevice(device.ieeeAddr); + } } @bind private async acceptJoiningDeviceHandler(ieeeAddr: string): Promise { diff --git a/test/controller.test.js b/test/controller.test.js index 2d04c1e73..7e46295f7 100644 --- a/test/controller.test.js +++ b/test/controller.test.js @@ -85,7 +85,7 @@ describe('Controller', () => { expect(zigbeeHerdsman.setTransmitPower).toHaveBeenCalledTimes(0); expect(zigbeeHerdsman.permitJoin).toHaveBeenCalledTimes(1); expect(zigbeeHerdsman.permitJoin).toHaveBeenCalledWith(true, undefined, undefined); - expect(logger.info).toHaveBeenCalledWith(`Currently ${Object.values(zigbeeHerdsman.devices).length - 1} devices are joined:`); + expect(logger.info).toHaveBeenCalledWith(`Currently ${Object.values(zigbeeHerdsman.devices).length - 1} devices are joined.`); expect(logger.info).toHaveBeenCalledWith( 'bulb (0x000b57fffec6a5b2): LED1545G12 - IKEA TRADFRI bulb E26/E27, white spectrum, globe, opal, 980 lm (Router)', ); diff --git a/test/networkMap.test.js b/test/networkMap.test.js index 912f02912..ca40b4b29 100644 --- a/test/networkMap.test.js +++ b/test/networkMap.test.js @@ -66,62 +66,40 @@ describe('Networkmap', () => { * | -> CC2530_ROUTER -> WXKG02LM_rev1 * */ - coordinator.lqi = () => { - return { - neighbors: [ - {ieeeAddr: bulb_color.ieeeAddr, networkAddress: bulb_color.networkAddress, relationship: 2, depth: 1, linkquality: 120}, - {ieeeAddr: bulb.ieeeAddr, networkAddress: bulb.networkAddress, relationship: 2, depth: 1, linkquality: 92}, - { - ieeeAddr: external_converter_device.ieeeAddr, - networkAddress: external_converter_device.networkAddress, - relationship: 2, - depth: 1, - linkquality: 92, - }, - ], - }; - }; - coordinator.routingTable = () => { - return {table: [{destinationAddress: CC2530_ROUTER.networkAddress, status: 'ACTIVE', nextHop: bulb.networkAddress}]}; - }; - - bulb.lqi = () => { - return { - neighbors: [ - {ieeeAddr: bulb_color.ieeeAddr, networkAddress: bulb_color.networkAddress, relationship: 1, depth: 2, linkquality: 110}, - {ieeeAddr: CC2530_ROUTER.ieeeAddr, networkAddress: CC2530_ROUTER.networkAddress, relationship: 1, depth: 2, linkquality: 100}, - ], - }; - }; - bulb.routingTable = () => { - return {table: []}; - }; - - bulb_color.lqi = () => { - return {neighbors: []}; - }; - bulb_color.routingTable = () => { - return {table: []}; - }; - - CC2530_ROUTER.lqi = () => { - return { - neighbors: [ - {ieeeAddr: '0x0000000000000000', networkAddress: WXKG02LM_rev1.networkAddress, relationship: 1, depth: 2, linkquality: 130}, - {ieeeAddr: bulb_color.ieeeAddr, networkAddress: bulb_color.networkAddress, relationship: 4, depth: 2, linkquality: 130}, - ], - }; - }; - CC2530_ROUTER.routingTable = () => { - return {table: []}; - }; - - unsupported_router.lqi = () => { - throw new Error('failed'); - }; - unsupported_router.routingTable = () => { - throw new Error('failed'); - }; + coordinator.lqi = jest.fn().mockResolvedValue({ + neighbors: [ + {ieeeAddr: bulb_color.ieeeAddr, networkAddress: bulb_color.networkAddress, relationship: 2, depth: 1, linkquality: 120}, + {ieeeAddr: bulb.ieeeAddr, networkAddress: bulb.networkAddress, relationship: 2, depth: 1, linkquality: 92}, + { + ieeeAddr: external_converter_device.ieeeAddr, + networkAddress: external_converter_device.networkAddress, + relationship: 2, + depth: 1, + linkquality: 92, + }, + ], + }); + coordinator.routingTable = jest.fn().mockResolvedValue({ + table: [{destinationAddress: CC2530_ROUTER.networkAddress, status: 'ACTIVE', nextHop: bulb.networkAddress}], + }); + bulb.lqi = jest.fn().mockResolvedValue({ + neighbors: [ + {ieeeAddr: bulb_color.ieeeAddr, networkAddress: bulb_color.networkAddress, relationship: 1, depth: 2, linkquality: 110}, + {ieeeAddr: CC2530_ROUTER.ieeeAddr, networkAddress: CC2530_ROUTER.networkAddress, relationship: 1, depth: 2, linkquality: 100}, + ], + }); + bulb.routingTable = jest.fn().mockResolvedValue({table: []}); + bulb_color.lqi = jest.fn().mockResolvedValue({neighbors: []}); + bulb_color.routingTable = jest.fn().mockResolvedValue({table: []}); + CC2530_ROUTER.lqi = jest.fn().mockResolvedValue({ + neighbors: [ + {ieeeAddr: '0x0000000000000000', networkAddress: WXKG02LM_rev1.networkAddress, relationship: 1, depth: 2, linkquality: 130}, + {ieeeAddr: bulb_color.ieeeAddr, networkAddress: bulb_color.networkAddress, relationship: 4, depth: 2, linkquality: 130}, + ], + }); + CC2530_ROUTER.routingTable = jest.fn().mockResolvedValue({table: []}); + unsupported_router.lqi = jest.fn().mockRejectedValue(new Error('failed')); + unsupported_router.routingTable = jest.fn().mockRejectedValue(new Error('failed')); } it('Output raw networkmap legacy api', async () => { @@ -826,4 +804,160 @@ describe('Networkmap', () => { const actual = JSON.parse(call[1]); expect(actual).toStrictEqual(expected); }); + + it('Handles retrying request when first attempt fails', async () => { + settings.set(['devices', '0x000b57fffec6a5b2', 'disabled'], true); + mock(); + bulb.lqi.mockRejectedValueOnce(new Error('failed')); + MQTT.publish.mockClear(); + MQTT.events.message('zigbee2mqtt/bridge/request/networkmap', stringify({type: 'raw', routes: true})); + await flushPromises(); + expect(MQTT.publish).toHaveBeenCalledTimes(1); + let call = MQTT.publish.mock.calls[0]; + expect(call[0]).toStrictEqual('zigbee2mqtt/bridge/response/networkmap'); + + const expected = { + data: { + routes: true, + type: 'raw', + value: { + links: [ + { + depth: 1, + linkquality: 120, + lqi: 120, + relationship: 2, + routes: [], + source: {ieeeAddr: '0x000b57fffec6a5b3', networkAddress: 40399}, + sourceIeeeAddr: '0x000b57fffec6a5b3', + sourceNwkAddr: 40399, + target: {ieeeAddr: '0x00124b00120144ae', networkAddress: 0}, + targetIeeeAddr: '0x00124b00120144ae', + }, + { + depth: 1, + linkquality: 92, + lqi: 92, + relationship: 2, + routes: [{destinationAddress: 6540, nextHop: 40369, status: 'ACTIVE'}], + source: {ieeeAddr: '0x000b57fffec6a5b2', networkAddress: 40369}, + sourceIeeeAddr: '0x000b57fffec6a5b2', + sourceNwkAddr: 40369, + target: {ieeeAddr: '0x00124b00120144ae', networkAddress: 0}, + targetIeeeAddr: '0x00124b00120144ae', + }, + { + depth: 1, + linkquality: 92, + lqi: 92, + relationship: 2, + routes: [], + source: {ieeeAddr: '0x0017880104e45511', networkAddress: 1114}, + sourceIeeeAddr: '0x0017880104e45511', + sourceNwkAddr: 1114, + target: {ieeeAddr: '0x00124b00120144ae', networkAddress: 0}, + targetIeeeAddr: '0x00124b00120144ae', + }, + { + depth: 2, + linkquality: 130, + lqi: 130, + relationship: 1, + routes: [], + source: {ieeeAddr: '0x0017880104e45521', networkAddress: 6538}, + sourceIeeeAddr: '0x0017880104e45521', + sourceNwkAddr: 6538, + target: {ieeeAddr: '0x0017880104e45559', networkAddress: 6540}, + targetIeeeAddr: '0x0017880104e45559', + }, + ], + nodes: [ + { + definition: null, + failed: [], + friendlyName: 'Coordinator', + ieeeAddr: '0x00124b00120144ae', + lastSeen: 1000, + modelID: null, + networkAddress: 0, + type: 'Coordinator', + }, + { + definition: { + description: 'Hue Go', + model: '7146060PH', + supports: + 'light (state, brightness, color_temp, color_temp_startup, color_xy, color_hs), power_on_behavior, effect, linkquality', + vendor: 'Philips', + }, + failed: [], + friendlyName: 'bulb_color', + ieeeAddr: '0x000b57fffec6a5b3', + lastSeen: 1000, + modelID: 'LLC020', + networkAddress: 40399, + type: 'Router', + }, + { + definition: { + description: 'Wireless remote switch (double rocker), 2016 model', + model: 'WXKG02LM_rev1', + supports: 'battery, voltage, power_outage_count, action, linkquality', + vendor: 'Aqara', + }, + friendlyName: 'button_double_key', + ieeeAddr: '0x0017880104e45521', + lastSeen: 1000, + modelID: 'lumi.sensor_86sw2.es1', + networkAddress: 6538, + type: 'EndDevice', + }, + { + definition: { + description: 'Automatically generated definition', + model: 'notSupportedModelID', + supports: 'action, linkquality', + vendor: 'Boef', + }, + failed: ['lqi', 'routingTable'], + friendlyName: '0x0017880104e45525', + ieeeAddr: '0x0017880104e45525', + lastSeen: 1000, + manufacturerName: 'Boef', + modelID: 'notSupportedModelID', + networkAddress: 6536, + type: 'Router', + }, + { + definition: { + description: 'CC2530 router', + model: 'CC2530.ROUTER', + supports: 'led, linkquality', + vendor: 'Custom devices (DiY)', + }, + failed: [], + friendlyName: 'cc2530_router', + ieeeAddr: '0x0017880104e45559', + lastSeen: 1000, + modelID: 'lumi.router', + networkAddress: 6540, + type: 'Router', + }, + { + definition: {description: 'external', model: 'external_converter_device', supports: 'linkquality', vendor: 'external'}, + friendlyName: '0x0017880104e45511', + ieeeAddr: '0x0017880104e45511', + lastSeen: 1000, + modelID: 'external_converter_device', + networkAddress: 1114, + type: 'EndDevice', + }, + ], + }, + }, + status: 'ok', + }; + const actual = JSON.parse(call[1]); + expect(actual).toStrictEqual(expected); + }); }); diff --git a/test/stub/zigbeeHerdsman.js b/test/stub/zigbeeHerdsman.js index fc65ba205..f8f54f931 100644 --- a/test/stub/zigbeeHerdsman.js +++ b/test/stub/zigbeeHerdsman.js @@ -857,8 +857,14 @@ const mock = { events[type] = handler; }, stop: jest.fn(), - getDevices: jest.fn().mockImplementation(() => { - return Object.values(devices).filter((d) => returnDevices.length === 0 || returnDevices.includes(d.ieeeAddr)); + getDevicesIterator: jest.fn().mockImplementation(function* (predicate) { + for (const key in devices) { + const device = devices[key]; + + if ((returnDevices.length === 0 || returnDevices.includes(device.ieeeAddr)) && (!predicate || predicate(device))) { + yield device; + } + } }), getDevicesByType: jest.fn().mockImplementation((type) => { return Object.values(devices) @@ -875,8 +881,14 @@ const mock = { .filter((d) => returnDevices.length === 0 || returnDevices.includes(d.ieeeAddr)) .find((d) => d.networkAddress === networkAddress); }), - getGroups: jest.fn().mockImplementation((query) => { - return Object.values(groups); + getGroupsIterator: jest.fn().mockImplementation(function* (predicate) { + for (const key in groups) { + const group = groups[key]; + + if (!predicate || predicate(group)) { + yield group; + } + } }), getGroupByID: jest.fn().mockImplementation((groupID) => { return Object.values(groups).find((d) => d.groupID === groupID); diff --git a/test/utils.test.js b/test/utils.test.js index 053e004ae..76e5bd569 100644 --- a/test/utils.test.js +++ b/test/utils.test.js @@ -4,6 +4,11 @@ const versionHerdsman = require('../node_modules/zigbee-herdsman/package.json'). const versionHerdsmanConverters = require('../node_modules/zigbee-herdsman-converters/package.json').version; describe('Utils', () => { + it('Object is empty', () => { + expect(utils.objectIsEmpty({})).toBeTruthy(); + expect(utils.objectIsEmpty({a: 1})).toBeFalsy(); + }); + it('Object has properties', () => { expect(utils.objectHasProperties({a: 1, b: 2, c: 3}, ['a', 'b'])).toBeTruthy(); expect(utils.objectHasProperties({a: 1, b: 2, c: 3}, ['a', 'b', 'd'])).toBeFalsy();