diff --git a/lib/extension/bridgeConfig.js b/lib/extension/bridgeConfig.js index c122b735b..b4d5899cd 100644 --- a/lib/extension/bridgeConfig.js +++ b/lib/extension/bridgeConfig.js @@ -204,8 +204,29 @@ class BridgeConfig extends BaseExtension { } addGroup(topic, message) { - const name = message; - const group = settings.addGroup(name); + let id = null; + let name = null; + try { + // json payload with id and friendly_name + const json = JSON.parse(message); + if (json.hasOwnProperty('id')) { + id = json.id; + name = `group_${id}`; + } + if (json.hasOwnProperty('friendly_name')) { + name = json.friendly_name; + } + } catch (e) { + // just friendly_name + name = message; + } + + if (name == null) { + logger.error('Failed to add group, missing friendly_name!'); + return; + } + + const group = settings.addGroup(name, id); this.zigbee.createGroup(group.ID); logger.info(`Added group '${name}'`); } diff --git a/lib/util/settings.js b/lib/util/settings.js index 1f64507b3..b6810a1c3 100644 --- a/lib/util/settings.js +++ b/lib/util/settings.js @@ -483,7 +483,7 @@ function removeDevice(IDorName) { write(); } -function addGroup(name) { +function addGroup(name, ID=null) { if (getGroup(name) || getDevice(name)) { throw new Error(`friendly_name '${name}' is already in use`); } @@ -493,12 +493,21 @@ function addGroup(name) { settings.groups = {}; } - let ID = '1'; - while (settings.groups.hasOwnProperty(ID)) { - ID = (Number.parseInt(ID) + 1).toString(); + if (ID == null) { + // look for free ID + ID = '1'; + while (settings.groups.hasOwnProperty(ID)) { + ID = (Number.parseInt(ID) + 1).toString(); + } + } else { + // ensure provided ID is not in use + ID = ID.toString(); + if (settings.groups.hasOwnProperty(ID)) { + throw new Error(`group id '${ID}' is already in use`); + } } - settings.groups[ID] = {friendly_name: name}; + settings.groups[ID] = {friendly_name: name, retain: false}; write(); return getGroup(ID); diff --git a/test/bridgeConfig.test.js b/test/bridgeConfig.test.js index c40f8419c..5fcc44745 100644 --- a/test/bridgeConfig.test.js +++ b/test/bridgeConfig.test.js @@ -205,11 +205,38 @@ describe('Bridge config', () => { zigbeeHerdsman.createGroup.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/config/add_group', 'new_group'); await flushPromises(); - expect(settings.getGroup('new_group')).toStrictEqual({"ID": 3, "friendlyName": "new_group", "friendly_name": "new_group", devices: [], optimistic: true}); + expect(settings.getGroup('new_group')).toStrictEqual({"ID": 3, "friendlyName": "new_group", "friendly_name": "new_group", "retain": false, devices: [], optimistic: true}); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(3) }); + it('Should allow to add groups with json', async () => { + zigbeeHerdsman.createGroup.mockClear(); + MQTT.events.message('zigbee2mqtt/bridge/config/add_group', '{"friendly_name": "new_group"}'); + await flushPromises(); + expect(settings.getGroup('new_group')).toStrictEqual({"ID": 3, "friendlyName": "new_group", "friendly_name": "new_group", "retain": false, devices: [], optimistic: true}); + expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); + expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(3) + }); + + it('Should allow to add groups with json specifying id', async () => { + zigbeeHerdsman.createGroup.mockClear(); + MQTT.events.message('zigbee2mqtt/bridge/config/add_group', '{"friendly_name": "new_group", "id": 42}'); + await flushPromises(); + expect(settings.getGroup('new_group')).toStrictEqual({"ID": 42, "friendlyName": "new_group", "friendly_name": "new_group", "retain": false, devices: [], optimistic: true}); + expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); + expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(42) + }); + + it('Should allow to add groups with json specifying only id', async () => { + zigbeeHerdsman.createGroup.mockClear(); + MQTT.events.message('zigbee2mqtt/bridge/config/add_group', '{"id": 42}'); + await flushPromises(); + expect(settings.getGroup('group_42')).toStrictEqual({"ID": 42, "friendlyName": "group_42", "friendly_name": "group_42", "retain": false, devices: [], optimistic: true}); + expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); + expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(42) + }); + it('Should allow to remove groups', async () => { settings.addGroup('to_be_removed') MQTT.events.message('zigbee2mqtt/bridge/config/remove_group', 'to_be_removed'); @@ -217,6 +244,13 @@ describe('Bridge config', () => { expect(settings.getGroup('to_be_removed')).toStrictEqual(null); }); + it('Shouldnt allow add groups without id or friendly_name in json', async () => { + zigbeeHerdsman.createGroup.mockClear(); + MQTT.events.message('zigbee2mqtt/bridge/config/add_group', '{}'); + await flushPromises(); + expect(logger.error).toHaveBeenCalledWith('Failed to add group, missing friendly_name!'); + }); + it('Shouldnt do anything on unsupported topic', async () => { await flushPromises(); MQTT.publish.mockClear(); diff --git a/test/settings.test.js b/test/settings.test.js index 427477174..6765e6c54 100644 --- a/test/settings.test.js +++ b/test/settings.test.js @@ -338,6 +338,21 @@ describe('Settings', () => { const expected = { '1': { friendly_name: 'test123', + retain: false, + }, + }; + + expect(settings.get().groups).toStrictEqual(expected); + }); + + it('Should add groups with specific ID', () => { + write(configurationFile, {}); + + const added = settings.addGroup('test123', 123); + const expected = { + '123': { + friendly_name: 'test123', + retain: false, }, }; @@ -354,6 +369,24 @@ describe('Settings', () => { const expected = { '1': { friendly_name: 'test123', + retain: false, + }, + }; + + expect(settings.get().groups).toStrictEqual(expected); + }); + + it('Should not add duplicate groups with specific ID', () => { + write(configurationFile, {}); + + settings.addGroup('test123', 123); + expect(() => { + settings.addGroup('test_id_123', 123); + }).toThrow(new Error("group id '123' is already in use")); + const expected = { + '123': { + friendly_name: 'test123', + retain: false, }, }; @@ -375,6 +408,7 @@ describe('Settings', () => { const expected = { '1': { friendly_name: 'test123', + retain: false, devices: ['0x123'], }, };