From 5a699dd28e9bf520a131d423f19535827a770ec4 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Mon, 14 Oct 2024 22:30:58 +0200 Subject: [PATCH 1/6] fix!: Remove configuring group members through `configuration.yaml` --- lib/extension/groups.ts | 73 ----------------------------------- lib/util/settings.schema.json | 6 --- 2 files changed, 79 deletions(-) diff --git a/lib/extension/groups.ts b/lib/extension/groups.ts index 114774c35a..6d435e588d 100644 --- a/lib/extension/groups.ts +++ b/lib/extension/groups.ts @@ -45,79 +45,6 @@ export default class Groups extends Extension { override async start(): Promise { this.eventBus.onStateChange(this, this.onStateChange); this.eventBus.onMQTTMessage(this, this.onMQTTMessage); - await this.syncGroupsWithSettings(); - } - - private async syncGroupsWithSettings(): Promise { - const settingsGroups = settings.getGroups(); - - const addRemoveFromGroup = async ( - action: 'add' | 'remove', - deviceName: string, - groupName: string | number, - endpoint: zh.Endpoint, - group: Group, - ): Promise => { - try { - logger.info(`${action === 'add' ? 'Adding' : 'Removing'} '${deviceName}' to group '${groupName}'`); - - if (action === 'remove') { - await endpoint.removeFromGroup(group.zh); - } else { - await endpoint.addToGroup(group.zh); - } - } catch (error) { - logger.error(`Failed to ${action} '${deviceName}' from '${groupName}'`); - logger.debug((error as Error).stack!); - } - }; - - for (const settingGroup of settingsGroups) { - const groupID = settingGroup.ID; - 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) { - const parsed = this.zigbee.resolveEntityAndEndpoint(d); - const device = parsed.entity as Device; - - if (!device) { - logger.error(`Cannot find '${d}' of group '${settingGroup.friendly_name}'`); - } - - if (!parsed.endpoint) { - if (parsed.endpointID) { - logger.error(`Cannot find endpoint '${parsed.endpointID}' of device '${parsed.ID}'`); - } - - continue; - } - - // In settings but not in zigbee - if (!zigbeeGroup.zh.hasMember(parsed.endpoint)) { - await addRemoveFromGroup('add', device?.name, settingGroup.friendly_name, parsed.endpoint, zigbeeGroup); - } - - settingsEndpoints.push(parsed.endpoint); - } - - // In zigbee but not in settings - for (const endpoint of zigbeeGroup.zh.members) { - if (!settingsEndpoints.includes(endpoint)) { - const deviceName = settings.getDevice(endpoint.getDevice().ieeeAddr)!.friendly_name; - - await addRemoveFromGroup('remove', deviceName, settingGroup.friendly_name, endpoint, zigbeeGroup); - } - } - } - - 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); - } - } } @bind async onStateChange(data: eventdata.StateChange): Promise { diff --git a/lib/util/settings.schema.json b/lib/util/settings.schema.json index 0717afbf4d..c359ecf729 100644 --- a/lib/util/settings.schema.json +++ b/lib/util/settings.schema.json @@ -909,12 +909,6 @@ "retain": { "type": "boolean" }, - "devices": { - "type": "array", - "items": { - "type": "string" - } - }, "optimistic": { "type": "boolean" }, From 866e30a1da6f3dfc4d6faac20157594fe1d63f24 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Tue, 15 Oct 2024 22:28:09 +0200 Subject: [PATCH 2/6] Updates --- lib/extension/groups.ts | 6 --- lib/types/types.d.ts | 3 +- lib/util/settings.ts | 56 ++-------------------- test/bridge.test.js | 71 ++++++++++++++-------------- test/group.test.js | 100 ---------------------------------------- test/settings.test.js | 91 ------------------------------------ test/stub/data.js | 6 --- 7 files changed, 39 insertions(+), 294 deletions(-) diff --git a/lib/extension/groups.ts b/lib/extension/groups.ts index 6d435e588d..08023c232c 100644 --- a/lib/extension/groups.ts +++ b/lib/extension/groups.ts @@ -267,13 +267,11 @@ export default class Groups extends Extension { assert(resolvedEntityGroup, '`resolvedEntityGroup` is missing'); logger.info(`Adding '${resolvedEntityDevice.name}' to '${resolvedEntityGroup.name}'`); await resolvedEntityEndpoint.addToGroup(resolvedEntityGroup.zh); - settings.addDeviceToGroup(resolvedEntityGroup.ID.toString(), keys); changedGroups.push(resolvedEntityGroup); } else if (type === 'remove') { assert(resolvedEntityGroup, '`resolvedEntityGroup` is missing'); logger.info(`Removing '${resolvedEntityDevice.name}' from '${resolvedEntityGroup.name}'`); await resolvedEntityEndpoint.removeFromGroup(resolvedEntityGroup.zh); - settings.removeDeviceFromGroup(resolvedEntityGroup.ID.toString(), keys); changedGroups.push(resolvedEntityGroup); } else { // remove_all @@ -284,10 +282,6 @@ export default class Groups extends Extension { } await resolvedEntityEndpoint.removeFromAllGroups(); - - for (const settingsGroup of settings.getGroups()) { - settings.removeDeviceFromGroup(settingsGroup.ID.toString(), keys); - } } } catch (e) { error = `Failed to ${type} from group (${(e as Error).message})`; diff --git a/lib/types/types.d.ts b/lib/types/types.d.ts index e658060564..30463f57f7 100644 --- a/lib/types/types.d.ts +++ b/lib/types/types.d.ts @@ -185,7 +185,7 @@ declare global { ssl_key?: string; }; devices: {[s: string]: DeviceOptions}; - groups: {[s: string]: OptionalProps, 'devices'>}; + groups: {[s: string]: Omit}; device_options: KeyValue; advanced: { log_rotation: boolean; @@ -239,7 +239,6 @@ declare global { } interface GroupOptions { - devices: string[]; ID: number; optimistic?: boolean; off_state?: 'all_members_off' | 'last_member_state'; diff --git a/lib/util/settings.ts b/lib/util/settings.ts index 97a53d2999..16167366dd 100644 --- a/lib/util/settings.ts +++ b/lib/util/settings.ts @@ -528,12 +528,12 @@ export function getGroup(IDorName: string | number): GroupOptions | undefined { const byID = settings.groups[IDorName]; if (byID) { - return {devices: [], ...byID, ID: Number(IDorName)}; + return {...byID, ID: Number(IDorName)}; } for (const [ID, group] of Object.entries(settings.groups)) { if (group.friendly_name === IDorName) { - return {devices: [], ...group, ID: Number(ID)}; + return {...group, ID: Number(ID)}; } } @@ -544,7 +544,7 @@ export function getGroups(): GroupOptions[] { const settings = get(); return Object.entries(settings.groups).map(([ID, group]) => { - return {devices: [], ...group, ID: Number(ID)}; + return {...group, ID: Number(ID)}; }); } @@ -615,16 +615,6 @@ export function removeDevice(IDorName: string): void { const device = getDeviceThrowIfNotExists(IDorName); const settings = getInternalSettings(); delete settings.devices?.[device.ID]; - - // Remove device from groups - if (settings.groups) { - const regex = 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)); - } - } - write(); } @@ -662,46 +652,6 @@ export function addGroup(name: string, ID?: string): GroupOptions { return getGroup(ID)!; // valid from creation above } -function groupGetDevice(group: {devices?: string[]}, keys: string[]): string | undefined { - for (const device of group.devices ?? []) { - if (keys.includes(device)) { - return device; - } - } - - return undefined; -} - -export function addDeviceToGroup(IDorName: string, keys: string[]): void { - const groupID = getGroupThrowIfNotExists(IDorName).ID!; - const settings = getInternalSettings(); - - const group = settings.groups![groupID]; - - if (!groupGetDevice(group, keys)) { - if (!group.devices) group.devices = []; - group.devices.push(keys[0]); - write(); - } -} - -export function removeDeviceFromGroup(IDorName: string, keys: string[]): void { - const groupID = getGroupThrowIfNotExists(IDorName).ID!; - const settings = getInternalSettings(); - const group = settings.groups![groupID]; - - if (!group.devices) { - return; - } - - const key = groupGetDevice(group, keys); - - if (key) { - group.devices = group.devices.filter((d) => d != key); - write(); - } -} - export function removeGroup(IDorName: string | number): void { const groupID = getGroupThrowIfNotExists(IDorName.toString()).ID!; const settings = getInternalSettings(); diff --git a/test/bridge.test.js b/test/bridge.test.js index 40feccbd8c..ed6a5a4599 100644 --- a/test/bridge.test.js +++ b/test/bridge.test.js @@ -182,13 +182,13 @@ describe('Bridge', () => { external_converters: [], groups: { 1: {friendly_name: 'group_1', retain: false}, - 11: {devices: ['bulb_2'], friendly_name: 'group_with_tradfri', retain: false}, - 12: {devices: ['TS0601_thermostat'], friendly_name: 'thermostat_group', retain: false}, - 14: {devices: ['power_plug', 'bulb_2'], friendly_name: 'switch_group', retain: false}, - 15071: {devices: ['bulb_color_2', 'bulb_2'], friendly_name: 'group_tradfri_remote', retain: false}, + 11: {friendly_name: 'group_with_tradfri', retain: false}, + 12: {friendly_name: 'thermostat_group', retain: false}, + 14: {friendly_name: 'switch_group', retain: false}, + 15071: {friendly_name: 'group_tradfri_remote', retain: false}, 2: {friendly_name: 'group_2', retain: false}, - 21: {devices: ['GLEDOPTO_2ID/cct'], friendly_name: 'gledopto_group'}, - 9: {devices: ['bulb_color_2', 'bulb_2', 'wall_switch_double/right'], friendly_name: 'ha_discovery_group'}, + 21: {friendly_name: 'gledopto_group'}, + 9: {friendly_name: 'ha_discovery_group'}, }, homeassistant: false, map_options: { @@ -2154,27 +2154,44 @@ describe('Bridge', () => { it('Should publish groups on startup', async () => { await resetExtension(); logger.setTransportsEnabled(true); + console.log(MQTT.publish.mock.calls.filter((c) => c[0] === 'zigbee2mqtt/bridge/groups')); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/groups', stringify([ {friendly_name: 'group_1', id: 1, members: [], scenes: []}, - {friendly_name: 'group_tradfri_remote', id: 15071, members: [{endpoint: 1, ieee_address: '0x000b57fffec6a5b4'}], scenes: []}, + { + friendly_name: 'group_tradfri_remote', + id: 15071, + members: [ + {endpoint: 1, ieee_address: '0x000b57fffec6a5b4'}, + {endpoint: 1, ieee_address: '0x000b57fffec6a5b7'}, + ], + scenes: [], + }, {friendly_name: '99', id: 99, members: [], scenes: []}, - {friendly_name: 'group_with_tradfri', id: 11, members: [], scenes: []}, - {friendly_name: 'thermostat_group', id: 12, members: [], scenes: []}, - {friendly_name: 'switch_group', id: 14, members: [{endpoint: 1, ieee_address: '0x0017880104e45524'}], scenes: []}, - {friendly_name: 'gledopto_group', id: 21, members: [], scenes: []}, + {friendly_name: 'group_with_tradfri', id: 11, members: [{endpoint: 1, ieee_address: '0x000b57fffec6a5b7'}], scenes: []}, + {friendly_name: 'thermostat_group', id: 12, members: [{endpoint: 1, ieee_address: '0x0017882104a44559'}], scenes: []}, + { + friendly_name: 'switch_group', + id: 14, + members: [ + {endpoint: 1, ieee_address: '0x0017880104e45524'}, + {endpoint: 1, ieee_address: '0x000b57fffec6a5b7'}, + ], + scenes: [], + }, + {friendly_name: 'gledopto_group', id: 21, members: [{endpoint: 15, ieee_address: '0x0017880104e45724'}], scenes: []}, {friendly_name: 'default_bind_group', id: 901, members: [], scenes: []}, { friendly_name: 'ha_discovery_group', id: 9, members: [ {endpoint: 1, ieee_address: '0x000b57fffec6a5b4'}, + {endpoint: 1, ieee_address: '0x000b57fffec6a5b7'}, {endpoint: 2, ieee_address: '0x0017880104e45542'}, ], scenes: [{id: 4, name: 'Scene 4'}], }, - {friendly_name: 'group_2', id: 2, members: [], scenes: []}, ]), {retain: true, qos: 0}, expect.any(Function), @@ -2778,22 +2795,6 @@ describe('Bridge', () => { it('Should allow to remove device by string', async () => { const device = zigbeeHerdsman.devices.bulb; - settings.set(['groups'], { - 1: { - friendly_name: 'group_1', - retain: false, - devices: [ - '0x999b57fffec6a5b9/1', - '0x000b57fffec6a5b2/1', - 'bulb', - 'bulb/right', - 'other_bulb', - 'bulb_1', - '0x000b57fffec6a5b2', - 'bulb/room/2', - ], - }, - }); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/device/remove', 'bulb'); await flushPromises(); @@ -2810,7 +2811,6 @@ describe('Bridge', () => { expect.any(Function), ); expect(settings.get().blocklist).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual(['0x999b57fffec6a5b9/1', 'other_bulb', 'bulb_1', 'bulb/room/2']); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/devices', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); }); @@ -2977,7 +2977,7 @@ describe('Bridge', () => { MQTT.events.message('zigbee2mqtt/bridge/request/group/rename', stringify({from: 'group_1', to: 'group_new_name'})); await flushPromises(); expect(settings.getGroup('group_1')).toBeUndefined(); - expect(settings.getGroup('group_new_name')).toStrictEqual({ID: 1, devices: [], friendly_name: 'group_new_name', retain: false}); + expect(settings.getGroup('group_new_name')).toStrictEqual({ID: 1, friendly_name: 'group_new_name', retain: false}); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/rename', @@ -3312,10 +3312,10 @@ describe('Bridge', () => { it('Should allow change group options', async () => { MQTT.publish.mockClear(); - expect(settings.getGroup('group_1')).toStrictEqual({ID: 1, devices: [], friendly_name: 'group_1', retain: false}); + expect(settings.getGroup('group_1')).toStrictEqual({ID: 1, friendly_name: 'group_1', retain: false}); MQTT.events.message('zigbee2mqtt/bridge/request/group/options', stringify({options: {retain: true, transition: 1}, id: 'group_1'})); await flushPromises(); - expect(settings.getGroup('group_1')).toStrictEqual({ID: 1, devices: [], friendly_name: 'group_1', retain: true, transition: 1}); + expect(settings.getGroup('group_1')).toStrictEqual({ID: 1, friendly_name: 'group_1', retain: true, transition: 1}); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/options', stringify({data: {from: {retain: false}, to: {retain: true, transition: 1}, restart_required: false, id: 'group_1'}, status: 'ok'}), @@ -3326,12 +3326,11 @@ describe('Bridge', () => { it('Should allow change group options with restart required', async () => { MQTT.publish.mockClear(); - expect(settings.getGroup('group_1')).toStrictEqual({ID: 1, devices: [], friendly_name: 'group_1', retain: false}); + expect(settings.getGroup('group_1')).toStrictEqual({ID: 1, friendly_name: 'group_1', retain: false}); MQTT.events.message('zigbee2mqtt/bridge/request/group/options', stringify({options: {off_state: 'all_members_off'}, id: 'group_1'})); await flushPromises(); expect(settings.getGroup('group_1')).toStrictEqual({ ID: 1, - devices: [], friendly_name: 'group_1', retain: false, off_state: 'all_members_off', @@ -3363,7 +3362,7 @@ describe('Bridge', () => { MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/add', 'group_193'); await flushPromises(); - expect(settings.getGroup('group_193')).toStrictEqual({ID: 3, devices: [], friendly_name: 'group_193'}); + expect(settings.getGroup('group_193')).toStrictEqual({ID: 3, friendly_name: 'group_193'}); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/add', @@ -3377,7 +3376,7 @@ describe('Bridge', () => { MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/add', stringify({friendly_name: 'group_193', id: 92})); await flushPromises(); - expect(settings.getGroup('group_193')).toStrictEqual({ID: 92, devices: [], friendly_name: 'group_193'}); + expect(settings.getGroup('group_193')).toStrictEqual({ID: 92, friendly_name: 'group_193'}); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/add', diff --git a/test/group.test.js b/test/group.test.js index 4c160bb416..550948fd31 100644 --- a/test/group.test.js +++ b/test/group.test.js @@ -45,81 +45,6 @@ describe('Groups', () => { controller.state.state = {}; }); - it('Apply group updates add', async () => { - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: ['bulb', 'bulb_color']}}); - zigbeeHerdsman.groups.group_1.members.push(zigbeeHerdsman.devices.bulb.getEndpoint(1)); - await resetExtension(); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([ - zigbeeHerdsman.devices.bulb.getEndpoint(1), - zigbeeHerdsman.devices.bulb_color.getEndpoint(1), - ]); - }); - - it('Apply group updates remove', async () => { - const endpoint = zigbeeHerdsman.devices.bulb_color.getEndpoint(1); - const group = zigbeeHerdsman.groups.group_1; - group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false}}); - await resetExtension(); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([]); - }); - - it('Apply group updates remove handle fail', async () => { - const endpoint = zigbeeHerdsman.devices.bulb_color.getEndpoint(1); - endpoint.removeFromGroup.mockImplementationOnce(() => { - throw new Error('failed!'); - }); - const group = zigbeeHerdsman.groups.group_1; - group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false}}); - logger.error.mockClear(); - await resetExtension(); - expect(logger.error).toHaveBeenCalledWith(`Failed to remove 'bulb_color' from 'group_1'`); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([endpoint]); - }); - - it('Move to non existing group', async () => { - const device = zigbeeHerdsman.devices.bulb_color; - const endpoint = device.getEndpoint(1); - const group = zigbeeHerdsman.groups.group_1; - group.members.push(endpoint); - settings.set(['groups'], {3: {friendly_name: 'group_3', retain: false, devices: [device.ieeeAddr]}}); - await resetExtension(); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([]); - }); - - it('Add non standard endpoint to group with name', async () => { - const QBKG03LM = zigbeeHerdsman.devices.QBKG03LM; - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: ['0x0017880104e45542/right']}}); - await resetExtension(); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([QBKG03LM.getEndpoint(3)]); - }); - - it('Add non standard endpoint to group with number', async () => { - const QBKG03LM = zigbeeHerdsman.devices.QBKG03LM; - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: ['wall_switch_double/2']}}); - await resetExtension(); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([QBKG03LM.getEndpoint(2)]); - }); - - it('Shouldnt crash on non-existing devices', async () => { - logger.error.mockClear(); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: ['not_existing_bla']}}); - await resetExtension(); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([]); - expect(logger.error).toHaveBeenCalledWith("Cannot find 'not_existing_bla' of group 'group_1'"); - }); - - it('Should resolve device friendly names', async () => { - settings.set(['devices', zigbeeHerdsman.devices.bulb.ieeeAddr, 'friendly_name'], 'bulb_friendly_name'); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: ['bulb_friendly_name', 'bulb_color']}}); - await resetExtension(); - expect(zigbeeHerdsman.groups.group_1.members).toStrictEqual([ - zigbeeHerdsman.devices.bulb.getEndpoint(1), - zigbeeHerdsman.devices.bulb_color.getEndpoint(1), - ]); - }); - 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); @@ -678,7 +603,6 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}}); await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message( @@ -687,28 +611,6 @@ describe('Groups', () => { ); await flushPromises(); expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual([]); - expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); - expect(MQTT.publish).toHaveBeenCalledWith( - 'zigbee2mqtt/bridge/response/group/members/remove', - stringify({data: {device: 'bulb_color', group: 'group_1'}, status: 'ok'}), - {retain: false, qos: 0}, - expect.any(Function), - ); - }); - - it('Remove from group via MQTT when in zigbee but not in settings', async () => { - const device = zigbeeHerdsman.devices.bulb_color; - const endpoint = device.getEndpoint(1); - const group = zigbeeHerdsman.groups.group_1; - group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: ['dummy']}}); - await resetExtension(); - MQTT.publish.mockClear(); - MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1', device: 'bulb_color'})); - await flushPromises(); - expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual(['dummy']); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/remove', @@ -723,13 +625,11 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [`wall_switch_double/right`]}}); await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1', device: '0x0017880104e45542/3'})); await flushPromises(); expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual([]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/remove', diff --git a/test/settings.test.js b/test/settings.test.js index fb707ceb18..0c372d72b4 100644 --- a/test/settings.test.js +++ b/test/settings.test.js @@ -434,7 +434,6 @@ describe('Settings', () => { const expected = { ID: 1, friendly_name: '123', - devices: [], }; expect(group).toStrictEqual(expected); @@ -458,7 +457,6 @@ describe('Settings', () => { const expected = { ID: 1, friendly_name: '123', - devices: [], }; expect(group).toStrictEqual(expected); @@ -591,95 +589,6 @@ describe('Settings', () => { expect(settings.get().groups).toStrictEqual(expected); }); - it('Should add devices to groups', () => { - write(configurationFile, { - devices: { - '0x123': { - friendly_name: 'bulb', - retain: true, - }, - }, - }); - - settings.addGroup('test123'); - settings.addDeviceToGroup('test123', ['0x123']); - const expected = { - 1: { - friendly_name: 'test123', - devices: ['0x123'], - }, - }; - - expect(settings.get().groups).toStrictEqual(expected); - }); - - it('Should remove devices from groups', () => { - write(configurationFile, { - devices: { - '0x123': { - friendly_name: 'bulb', - retain: true, - }, - }, - groups: { - 1: { - friendly_name: 'test123', - devices: ['0x123'], - }, - }, - }); - - settings.removeDeviceFromGroup('test123', ['0x123']); - const expected = { - 1: { - friendly_name: 'test123', - devices: [], - }, - }; - - expect(settings.get().groups).toStrictEqual(expected); - }); - - it('Shouldnt crash when removing device from group when group has no devices', () => { - write(configurationFile, { - devices: { - '0x123': { - friendly_name: 'bulb', - retain: true, - }, - }, - groups: { - 1: { - friendly_name: 'test123', - }, - }, - }); - - settings.removeDeviceFromGroup('test123', ['0x123']); - const expected = { - 1: { - friendly_name: 'test123', - }, - }; - - expect(settings.get().groups).toStrictEqual(expected); - }); - - it('Should throw when adding device to non-existing group', () => { - write(configurationFile, { - devices: { - '0x123': { - friendly_name: 'bulb', - retain: true, - }, - }, - }); - - expect(() => { - settings.removeDeviceFromGroup('test123', 'bulb'); - }).toThrow(new Error("Group 'test123' does not exist")); - }); - it('Should throw when adding device which already exists', () => { write(configurationFile, { devices: { diff --git a/test/stub/data.js b/test/stub/data.js index 66a9f80e49..b4cca26553 100644 --- a/test/stub/data.js +++ b/test/stub/data.js @@ -213,30 +213,24 @@ function writeDefaultConfiguration() { 15071: { friendly_name: 'group_tradfri_remote', retain: false, - devices: ['bulb_color_2', 'bulb_2'], }, 11: { friendly_name: 'group_with_tradfri', retain: false, - devices: ['bulb_2'], }, 12: { friendly_name: 'thermostat_group', retain: false, - devices: ['TS0601_thermostat'], }, 14: { friendly_name: 'switch_group', retain: false, - devices: ['power_plug', 'bulb_2'], }, 21: { friendly_name: 'gledopto_group', - devices: ['GLEDOPTO_2ID/cct'], }, 9: { friendly_name: 'ha_discovery_group', - devices: ['bulb_color_2', 'bulb_2', 'wall_switch_double/right'], }, }, external_converters: [], From ee66c06e82ff66c1c3b0ef09ffcd23f9b0b109c4 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Sun, 20 Oct 2024 22:07:17 +0200 Subject: [PATCH 3/6] Updates --- test/bridge.test.js | 5 +- test/group.test.js | 114 ++++++++---------------------------- test/stub/zigbeeHerdsman.js | 17 ++++-- 3 files changed, 40 insertions(+), 96 deletions(-) diff --git a/test/bridge.test.js b/test/bridge.test.js index ed6a5a4599..cca0c67b6f 100644 --- a/test/bridge.test.js +++ b/test/bridge.test.js @@ -2154,11 +2154,12 @@ describe('Bridge', () => { it('Should publish groups on startup', async () => { await resetExtension(); logger.setTransportsEnabled(true); - console.log(MQTT.publish.mock.calls.filter((c) => c[0] === 'zigbee2mqtt/bridge/groups')); + // console.log(MQTT.publish.mock.calls.filter((c) => c[0] === 'zigbee2mqtt/bridge/groups')); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/groups', stringify([ {friendly_name: 'group_1', id: 1, members: [], scenes: []}, + {friendly_name: 'group_2', id: 2, members: [], scenes: []}, { friendly_name: 'group_tradfri_remote', id: 15071, @@ -2188,7 +2189,7 @@ describe('Bridge', () => { members: [ {endpoint: 1, ieee_address: '0x000b57fffec6a5b4'}, {endpoint: 1, ieee_address: '0x000b57fffec6a5b7'}, - {endpoint: 2, ieee_address: '0x0017880104e45542'}, + {endpoint: 3, ieee_address: '0x0017880104e45542'}, ], scenes: [{id: 4, name: 'Scene 4'}], }, diff --git a/test/group.test.js b/test/group.test.js index 550948fd31..444231c81a 100644 --- a/test/group.test.js +++ b/test/group.test.js @@ -19,11 +19,6 @@ const settings = require('../lib/util/settings'); describe('Groups', () => { let controller; - let resetExtension = async () => { - await controller.enableDisableExtension(false, 'Groups'); - await controller.enableDisableExtension(true, 'Groups'); - }; - beforeAll(async () => { jest.useFakeTimers(); controller = new Controller(jest.fn(), jest.fn()); @@ -36,7 +31,7 @@ describe('Groups', () => { }); beforeEach(() => { - Object.values(zigbeeHerdsman.groups).forEach((g) => (g.members = [])); + zigbeeHerdsman.resetGroupMembers(); data.writeDefaultConfiguration(); settings.reRead(); MQTT.publish.mockClear(); @@ -50,8 +45,6 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}}); - await resetExtension(); MQTT.publish.mockClear(); const payload = {data: {onOff: 1}, cluster: 'genOnOff', device, endpoint, type: 'attributeReport', linkquality: 10}; @@ -66,8 +59,6 @@ describe('Groups', () => { it('Should not republish identical optimistic group states', async () => { const device1 = zigbeeHerdsman.devices.bulb_2; const device2 = zigbeeHerdsman.devices.bulb_color_2; - const group = zigbeeHerdsman.groups.group_tradfri_remote; - await resetExtension(); MQTT.publish.mockClear(); await zigbeeHerdsman.events.message({ @@ -126,8 +117,6 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}}); - await resetExtension(); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); @@ -143,8 +132,6 @@ describe('Groups', () => { const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); settings.set(['devices', device.ieeeAddr, 'disabled'], true); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}}); - await resetExtension(); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); @@ -159,8 +146,6 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}}); - await resetExtension(); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/bulb_color/set', stringify({state: 'ON'})); @@ -186,7 +171,6 @@ describe('Groups', () => { it('Should publish state of device with endpoint name', async () => { const group = zigbeeHerdsman.groups.gledopto_group; - await resetExtension(); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/gledopto_group/set', stringify({state: 'ON'})); @@ -210,7 +194,6 @@ describe('Groups', () => { it('Should publish state of group when specific state of specific endpoint is changed', async () => { const group = zigbeeHerdsman.groups.gledopto_group; - await resetExtension(); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/GLEDOPTO_2ID/set', stringify({state_cct: 'ON'})); @@ -236,8 +219,7 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, filtered_attributes: ['brightness'], devices: [device.ieeeAddr]}}); - await resetExtension(); + settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, filtered_attributes: ['brightness']}}); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON', brightness: 100})); @@ -257,8 +239,7 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', devices: [device.ieeeAddr], optimistic: false, retain: false}}); - await resetExtension(); + settings.set(['groups'], {1: {friendly_name: 'group_1', optimistic: false, retain: false}}); MQTT.publish.mockClear(); const payload = {data: {onOff: 1}, cluster: 'genOnOff', device, endpoint, type: 'attributeReport', linkquality: 10}; @@ -272,14 +253,8 @@ describe('Groups', () => { it('Should publish state change of another group with shared device when a group changes its state', async () => { const device = zigbeeHerdsman.devices.bulb_color; const endpoint = device.getEndpoint(1); - const group = zigbeeHerdsman.groups.group_1; - group.members.push(endpoint); - settings.set(['groups'], { - 1: {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}, - 2: {friendly_name: 'group_2', retain: false, devices: [device.ieeeAddr]}, - 3: {friendly_name: 'group_3', retain: false, devices: []}, - }); - await resetExtension(); + zigbeeHerdsman.groups.group_1.members.push(endpoint); + zigbeeHerdsman.groups.group_2.members.push(endpoint); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); @@ -298,10 +273,6 @@ describe('Groups', () => { const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint_1); group.members.push(endpoint_2); - settings.set(['groups'], { - 1: {friendly_name: 'group_1', devices: [device_1.ieeeAddr, device_2.ieeeAddr], retain: false}, - }); - await resetExtension(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); await flushPromises(); @@ -322,9 +293,8 @@ describe('Groups', () => { group.members.push(endpoint_1); group.members.push(endpoint_2); settings.set(['groups'], { - 1: {friendly_name: 'group_1', devices: [device_1.ieeeAddr, device_2.ieeeAddr], retain: false, off_state: 'last_member_state'}, + 1: {friendly_name: 'group_1', retain: false, off_state: 'last_member_state'}, }); - await resetExtension(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); await flushPromises(); @@ -354,14 +324,9 @@ describe('Groups', () => { const device_2 = zigbeeHerdsman.devices.bulb; const endpoint_1 = device_1.getEndpoint(1); const endpoint_2 = device_2.getEndpoint(1); - const group = zigbeeHerdsman.groups.group_1; - group.members.push(endpoint_1); - group.members.push(endpoint_2); - settings.set(['groups'], { - 1: {friendly_name: 'group_1', devices: [device_1.ieeeAddr, device_2.ieeeAddr], retain: false}, - 2: {friendly_name: 'group_2', retain: false, devices: [device_1.ieeeAddr]}, - }); - await resetExtension(); + zigbeeHerdsman.groups.group_1.members.push(endpoint_1); + zigbeeHerdsman.groups.group_1.members.push(endpoint_2); + zigbeeHerdsman.groups.group_2.members.push(endpoint_1); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); await flushPromises(); @@ -383,9 +348,8 @@ describe('Groups', () => { group.members.push(endpoint_1); group.members.push(endpoint_2); settings.set(['groups'], { - 1: {friendly_name: 'group_1', devices: [device_1.ieeeAddr, device_2.ieeeAddr], retain: false}, + 1: {friendly_name: 'group_1', retain: false}, }); - await resetExtension(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); await flushPromises(); @@ -409,9 +373,8 @@ describe('Groups', () => { group.members.push(endpoint_1); group.members.push(endpoint_2); settings.set(['groups'], { - 1: {friendly_name: 'group_1', devices: [device_1.ieeeAddr, device_2.ieeeAddr], retain: false}, + 1: {friendly_name: 'group_1', retain: false}, }); - await resetExtension(); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/bulb_color/set', stringify({state: 'OFF', color_temp: 200})); @@ -451,9 +414,8 @@ describe('Groups', () => { group.members.push(endpoint_1); group.members.push(endpoint_2); settings.set(['groups'], { - 1: {friendly_name: 'group_1', devices: [device_1.ieeeAddr, device_2.ieeeAddr], retain: false}, + 1: {friendly_name: 'group_1', retain: false}, }); - await resetExtension(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({state: 'ON'})); await flushPromises(); @@ -472,14 +434,12 @@ describe('Groups', () => { const device = zigbeeHerdsman.devices.bulb_color; const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: []}}); + settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false}}); expect(group.members.length).toBe(0); - await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({transaction: '123', group: 'group_1', device: 'bulb_color'})); await flushPromises(); expect(group.members).toStrictEqual([endpoint]); - expect(settings.getGroup('group_1').devices).toStrictEqual([`${device.ieeeAddr}/1`]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/add', @@ -493,9 +453,8 @@ describe('Groups', () => { const device = zigbeeHerdsman.devices.bulb_color; const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: []}}); + settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false}}); expect(group.members.length).toBe(0); - await resetExtension(); endpoint.addToGroup.mockImplementationOnce(() => { throw new Error('timeout'); }); @@ -504,7 +463,6 @@ describe('Groups', () => { MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group_1', device: 'bulb_color'})); await flushPromises(); expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual([]); 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', @@ -518,14 +476,12 @@ describe('Groups', () => { const device = zigbeeHerdsman.devices.bulb_color; const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups['group/with/slashes']; - settings.set(['groups'], {99: {friendly_name: 'group/with/slashes', retain: false, devices: []}}); + settings.set(['groups'], {99: {friendly_name: 'group/with/slashes', retain: false}}); expect(group.members.length).toBe(0); - await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group/with/slashes', device: 'bulb_color'})); await flushPromises(); expect(group.members).toStrictEqual([endpoint]); - expect(settings.getGroup('group/with/slashes').devices).toStrictEqual([`${device.ieeeAddr}/1`]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/add', @@ -540,12 +496,10 @@ describe('Groups', () => { const endpoint = device.getEndpoint(3); const group = zigbeeHerdsman.groups.group_1; expect(group.members.length).toBe(0); - await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group_1', device: 'wall_switch_double/right'})); await flushPromises(); expect(group.members).toStrictEqual([endpoint]); - expect(settings.getGroup('group_1').devices).toStrictEqual([`${device.ieeeAddr}/${endpoint.ID}`]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/add', @@ -560,14 +514,12 @@ describe('Groups', () => { const endpoint = device.getEndpoint(3); const group = zigbeeHerdsman.groups.group_1; expect(group.members.length).toBe(0); - await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group_1', device: 'wall_switch_double/right'})); await flushPromises(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group_1', device: '0x0017880104e45542/3'})); await flushPromises(); expect(group.members).toStrictEqual([endpoint]); - expect(settings.getGroup('group_1').devices).toStrictEqual([`${device.ieeeAddr}/${endpoint.ID}`]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/add', @@ -582,13 +534,11 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [device.ieeeAddr]}}); - await resetExtension(); + settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false}}); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1', device: 'bulb_color'})); await flushPromises(); expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual([]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/remove', @@ -603,7 +553,6 @@ describe('Groups', () => { const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message( 'zigbee2mqtt/bridge/request/group/members/remove', @@ -621,11 +570,10 @@ describe('Groups', () => { }); it('Remove from group via MQTT with postfix variant 1', async () => { - const device = zigbeeHerdsman.devices.bulb_color; - const endpoint = device.getEndpoint(1); + const device = zigbeeHerdsman.devices.QBKG03LM; + const endpoint = device.getEndpoint(3); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1', device: '0x0017880104e45542/3'})); await flushPromises(); @@ -640,17 +588,15 @@ describe('Groups', () => { }); it('Remove from group via MQTT with postfix variant 2', async () => { - const device = zigbeeHerdsman.devices.bulb_color; - const endpoint = device.getEndpoint(1); + const device = zigbeeHerdsman.devices.QBKG03LM; + const endpoint = device.getEndpoint(3); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [`0x0017880104e45542/right`]}}); - await resetExtension(); + settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false}}); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1', device: 'wall_switch_double/3'})); await flushPromises(); expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual([]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/remove', @@ -661,17 +607,14 @@ describe('Groups', () => { }); it('Remove from group via MQTT with postfix variant 3', async () => { - const device = zigbeeHerdsman.devices.bulb_color; - const endpoint = device.getEndpoint(1); + const device = zigbeeHerdsman.devices.QBKG03LM; + const endpoint = device.getEndpoint(3); const group = zigbeeHerdsman.groups.group_1; group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [`wall_switch_double/3`]}}); - await resetExtension(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1', device: '0x0017880104e45542/right'})); await flushPromises(); expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual([]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/remove', @@ -685,14 +628,11 @@ describe('Groups', () => { const device = zigbeeHerdsman.devices.bulb_color; const endpoint = device.getEndpoint(1); const group = zigbeeHerdsman.groups.group_1; - group.members.push(endpoint); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [`wall_switch_double/3`]}}); - await resetExtension(); + group.members.push(zigbeeHerdsman.devices.QBKG03LM.endpoints[2]); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove_all', stringify({device: '0x0017880104e45542/right'})); await flushPromises(); expect(group.members).toStrictEqual([]); - expect(settings.getGroup('group_1').devices).toStrictEqual([]); expect(MQTT.publish).toHaveBeenCalledWith('zigbee2mqtt/bridge/groups', expect.any(String), expect.any(Object), expect.any(Function)); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/members/remove_all', @@ -703,7 +643,6 @@ describe('Groups', () => { }); it('Error when adding to non-existing group', async () => { - await resetExtension(); logger.error.mockClear(); MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/members/remove', stringify({group: 'group_1_not_existing', device: 'bulb_color'})); @@ -722,7 +661,6 @@ describe('Groups', () => { }); it('Error when adding a non-existing device', 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'})); @@ -741,7 +679,6 @@ describe('Groups', () => { }); it('Error when adding a non-existing endpoint', async () => { - await resetExtension(); logger.error.mockClear(); MQTT.publish.mockClear(); MQTT.events.message( @@ -768,8 +705,7 @@ describe('Groups', () => { const group = zigbeeHerdsman.groups.group_1; group.members.push(bulbColor.getEndpoint(1)); group.members.push(bulbColorTemp.getEndpoint(1)); - settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false, devices: [bulbColor.ieeeAddr, bulbColorTemp.ieeeAddr]}}); - await resetExtension(); + settings.set(['groups'], {1: {friendly_name: 'group_1', retain: false}}); MQTT.publish.mockClear(); await MQTT.events.message('zigbee2mqtt/group_1/set', stringify({color_temp: 50})); diff --git a/test/stub/zigbeeHerdsman.js b/test/stub/zigbeeHerdsman.js index 5cdfe3c563..82eac42c92 100644 --- a/test/stub/zigbeeHerdsman.js +++ b/test/stub/zigbeeHerdsman.js @@ -141,10 +141,7 @@ class Endpoint { this.removeFromGroup = jest.fn(); this.removeFromGroup.mockImplementation((group) => { - const index = group.members.indexOf(this); - if (index != -1) { - group.members.splice(index, 1); - } + group.members = group.members.filter((e) => e !== this); }); this.removeFromAllGroups = () => { @@ -352,6 +349,7 @@ const zigfred_plus = new Device( const groups = { group_1: new Group(1, []), + group_2: new Group(2, []), group_tradfri_remote: new Group(15071, [bulb_color_2.endpoints[0], bulb_2.endpoints[0]]), 'group/with/slashes': new Group(99, []), group_with_tradfri: new Group(11, [bulb_2.endpoints[0]]), @@ -359,9 +357,17 @@ const groups = { group_with_switch: new Group(14, [ZNCZ02LM.endpoints[0], bulb_2.endpoints[0]]), gledopto_group: new Group(21, [GLEDOPTO_2ID.endpoints[3]]), default_bind_group: new Group(901, []), - ha_discovery_group: new Group(9, [bulb_color_2.endpoints[0], bulb_2.endpoints[0], QBKG03LM.endpoints[1]]), + ha_discovery_group: new Group(9, [bulb_color_2.endpoints[0], bulb_2.endpoints[0], QBKG03LM.endpoints[2]]), }; +const groupMembersBackup = Object.fromEntries(Object.entries(groups).map((v) => [v[0], [...v[1].members]])); + +function resetGroupMembers() { + for (const key in groupMembersBackup) { + groups[key].members = [...groupMembersBackup[key]]; + } +} + const devices = { coordinator: new Device('Coordinator', '0x00124b00120144ae', 0, 0, [new Endpoint(1, [], [], '0x00124b00120144ae')], false), bulb: new Device( @@ -919,5 +925,6 @@ module.exports = { devices, groups, returnDevices, + resetGroupMembers, custom_clusters, }; From 453b1c2e27af90d28982c003565523b850d27286 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Sun, 20 Oct 2024 22:35:28 +0200 Subject: [PATCH 4/6] fix --- test/extensions/bridge.test.ts | 13 +++++++------ test/extensions/groups.test.ts | 12 +++++++----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/test/extensions/bridge.test.ts b/test/extensions/bridge.test.ts index 8e08002f4e..b33ded8a7d 100644 --- a/test/extensions/bridge.test.ts +++ b/test/extensions/bridge.test.ts @@ -1,3 +1,10 @@ +import * as data from '../mocks/data'; +import {mockJSZipFile, mockJSZipGenerateAsync} from '../mocks/jszip'; +import {mockLogger} from '../mocks/logger'; +import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; +import {flushPromises, JestMockAny} from '../mocks/utils'; +import {CUSTOM_CLUSTERS, devices, groups, mockController as mockZHController, events as mockZHEvents, returnDevices} from '../mocks/zigbeeHerdsman'; + import type Bridge from '../../lib/extension/bridge'; import assert from 'assert'; @@ -9,12 +16,6 @@ import stringify from 'json-stable-stringify-without-jsonify'; import {Controller} from '../../lib/controller'; import * as settings from '../../lib/util/settings'; import utils from '../../lib/util/utils'; -import * as data from '../mocks/data'; -import {mockJSZipFile, mockJSZipGenerateAsync} from '../mocks/jszip'; -import {mockLogger} from '../mocks/logger'; -import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; -import {flushPromises, JestMockAny} from '../mocks/utils'; -import {CUSTOM_CLUSTERS, devices, groups, mockController as mockZHController, events as mockZHEvents, returnDevices} from '../mocks/zigbeeHerdsman'; returnDevices.push(devices.coordinator.ieeeAddr); returnDevices.push(devices.bulb.ieeeAddr); diff --git a/test/extensions/groups.test.ts b/test/extensions/groups.test.ts index eaffb57df3..9c1f638983 100644 --- a/test/extensions/groups.test.ts +++ b/test/extensions/groups.test.ts @@ -1,14 +1,15 @@ +import * as data from '../mocks/data'; +import {mockLogger} from '../mocks/logger'; +import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; +import {flushPromises} from '../mocks/utils'; +import {devices, groups, events as mockZHEvents, resetGroupMembers, returnDevices} from '../mocks/zigbeeHerdsman'; + import stringify from 'json-stable-stringify-without-jsonify'; import {toZigbee as zhcToZigbee} from 'zigbee-herdsman-converters'; import {Controller} from '../../lib/controller'; import * as settings from '../../lib/util/settings'; -import * as data from '../mocks/data'; -import {mockLogger} from '../mocks/logger'; -import {mockMQTT, events as mockMQTTEvents} from '../mocks/mqtt'; -import {flushPromises} from '../mocks/utils'; -import {devices, groups, events as mockZHEvents, resetGroupMembers, returnDevices} from '../mocks/zigbeeHerdsman'; returnDevices.push( devices.coordinator.ieeeAddr, @@ -558,6 +559,7 @@ describe('Extension: Groups', () => { const device = devices.bulb_color; const endpoint = device.getEndpoint(1); const group = groups['group/with/slashes']; + settings.set(['groups'], {99: {friendly_name: 'group/with/slashes', retain: false}}); expect(group.members.length).toBe(0); mockMQTT.publish.mockClear(); mockMQTTEvents.message('zigbee2mqtt/bridge/request/group/members/add', stringify({group: 'group/with/slashes', device: 'bulb_color'})); From ac1a8609cc5bd2036183bb5415b8066c0d389370 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Sun, 20 Oct 2024 22:43:08 +0200 Subject: [PATCH 5/6] Updates --- lib/util/settings.ts | 8 -------- test/settings.test.ts | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/util/settings.ts b/lib/util/settings.ts index 277f320493..ccfc402329 100644 --- a/lib/util/settings.ts +++ b/lib/util/settings.ts @@ -539,14 +539,6 @@ export function getGroup(IDorName: string | number): GroupOptions | undefined { return undefined; } -export function getGroups(): GroupOptions[] { - const settings = get(); - - return Object.entries(settings.groups).map(([ID, group]) => { - return {...group, ID: Number(ID)}; - }); -} - function getGroupThrowIfNotExists(IDorName: string): GroupOptions { const group = getGroup(IDorName); diff --git a/test/settings.test.ts b/test/settings.test.ts index 784a6c61bb..9e1330cfb9 100644 --- a/test/settings.test.ts +++ b/test/settings.test.ts @@ -447,6 +447,20 @@ describe('Settings', () => { expect(group).toStrictEqual(expected); }); + it('Throw if removing non-existing group', () => { + const content = { + groups: { + 1: { + friendly_name: '123', + }, + }, + }; + + write(configurationFile, content); + + expect(() => settings.removeGroup('2')).toThrow(`Group '2' does not exist`); + }); + it('Should read groups from a separate file', () => { const contentConfiguration = { groups: 'groups.yaml', From 3c0855ac306ccba9ebc5eb95c5de224ce909df39 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Mon, 21 Oct 2024 21:35:12 +0200 Subject: [PATCH 6/6] Feedback --- lib/extension/groups.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/lib/extension/groups.ts b/lib/extension/groups.ts index 08023c232c..eb5b0e6ed7 100644 --- a/lib/extension/groups.ts +++ b/lib/extension/groups.ts @@ -247,22 +247,6 @@ export default class Groups extends Extension { if (!error) { assert(resolvedEntityEndpoint, '`resolvedEntityEndpoint` is missing'); try { - const keys = [ - `${resolvedEntityDevice.ieeeAddr}/${resolvedEntityEndpoint.ID}`, - `${resolvedEntityDevice.name}/${resolvedEntityEndpoint.ID}`, - ]; - const endpointNameLocal = resolvedEntityDevice.endpointName(resolvedEntityEndpoint); - - if (endpointNameLocal) { - keys.push(`${resolvedEntityDevice.ieeeAddr}/${endpointNameLocal}`); - keys.push(`${resolvedEntityDevice.name}/${endpointNameLocal}`); - } - - if (!endpointNameLocal) { - keys.push(resolvedEntityDevice.name); - keys.push(resolvedEntityDevice.ieeeAddr); - } - if (type === 'add') { assert(resolvedEntityGroup, '`resolvedEntityGroup` is missing'); logger.info(`Adding '${resolvedEntityDevice.name}' to '${resolvedEntityGroup.name}'`);