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: [],