From a90ebce67724ae78f3b5d449e54426b8173d3a11 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Sat, 13 Jun 2020 23:28:06 +0200 Subject: [PATCH] Change device options. #3181 --- lib/extension/bridge.js | 50 ++++++++++++++++++++++------ lib/extension/legacy/bridgeLegacy.js | 2 +- lib/util/settings.js | 16 ++++++--- test/bridge.test.js | 37 ++++++++++++++++++++ test/settings.test.js | 8 +++++ 5 files changed, 97 insertions(+), 16 deletions(-) diff --git a/lib/extension/bridge.js b/lib/extension/bridge.js index 8662901f1..f6717fe2d 100644 --- a/lib/extension/bridge.js +++ b/lib/extension/bridge.js @@ -13,11 +13,13 @@ class Bridge extends Extension { this.lastJoinedDeviceIeeeAddr = null; this.requestLookup = { - 'permitjoin': this.permitJoin.bind(this), 'device/remove': this.deviceRemove.bind(this), - 'group/remove': this.groupRemove.bind(this), 'device/rename': this.deviceRename.bind(this), + 'device/changeoptions': this.deviceChangeOptions.bind(this), + 'group/changeoptions': this.groupChangeOptions.bind(this), + 'group/remove': this.groupRemove.bind(this), 'group/rename': this.groupRename.bind(this), + 'permitjoin': this.permitJoin.bind(this), }; } @@ -80,6 +82,14 @@ class Bridge extends Extension { * Requests */ + async deviceChangeOptions(message) { + return this.changeEntityOptions('device', message); + } + + async groupChangeOptions(message) { + return this.changeEntityOptions('group', message); + } + async deviceRemove(message) { return this.removeEntity('device', message); } @@ -107,6 +117,23 @@ class Bridge extends Extension { * Utils */ + changeEntityOptions(entityType, message) { + if (typeof message !== 'object' || !message.hasOwnProperty('ID') || !message.hasOwnProperty('options')) { + throw new Error(`Invalid payload`); + } + + const ID = message.ID; + const entity = this.getEntity(entityType, ID); + settings.changeEntityOptions(ID, message.options); + const cleanup = (o) => { + delete o.friendlyName; delete o.friendly_name; delete o.ID; delete o.type; delete o.devices; + return o; + } + const oldOptions = cleanup(entity.settings); + const newOptions = cleanup(settings.getEntity(ID)); + return utils.getResponse(message, {from: oldOptions, to: newOptions, ID}, null); + } + renameEntity(entityType, message) { const deviceAndHasLast = entityType === 'device' && typeof message === 'object' && message.last === true; if (typeof message !== 'object' || (!message.hasOwnProperty('from') && !deviceAndHasLast) || !message.hasOwnProperty('to')) { @@ -119,11 +146,7 @@ class Bridge extends Extension { const from = deviceAndHasLast ? this.lastJoinedDeviceIeeeAddr : message.from; const to = message.to; - const entity = this.zigbee.resolveEntity(from); - if (!entity || entity.type !== entityType) { - throw new Error(`${utils.capitalize(entityType)} '${from}' does not exist`); - } - + const entity = this.getEntity(entityType, from); settings.changeFriendlyName(from, to); @@ -140,10 +163,7 @@ class Bridge extends Extension { async removeEntity(entityType, message) { const ID = typeof message === 'object' ? message.ID : message.trim(); - const entity = this.zigbee.resolveEntity(ID); - if (!entity || entity.type !== entityType) { - throw new Error(`${utils.capitalize(entityType)} '${ID}' does not exist`); - } + const entity = this.getEntity(entityType, ID); let ban = false; let force = false; @@ -200,6 +220,14 @@ class Bridge extends Extension { } } + getEntity(type, ID) { + const entity = this.zigbee.resolveEntity(ID); + if (!entity || entity.type !== type) { + throw new Error(`${utils.capitalize(type)} '${ID}' does not exist`); + } + return entity; + } + async publishInfo() { const payload = { version: this.zigbee2mqttVersion.version, diff --git a/lib/extension/legacy/bridgeLegacy.js b/lib/extension/legacy/bridgeLegacy.js index c9615a54f..866e38bd6 100644 --- a/lib/extension/legacy/bridgeLegacy.js +++ b/lib/extension/legacy/bridgeLegacy.js @@ -88,7 +88,7 @@ class BridgeLegacy extends Extension { const entity = settings.getEntity(json.friendly_name); assert(entity, `Entity '${json.friendly_name}' does not exist`); - settings.changeDeviceOptions(entity.ID, json.options); + settings.changeEntityOptions(entity.ID, json.options); logger.info(`Changed device specific options of '${json.friendly_name}' (${JSON.stringify(json.options)})`); } diff --git a/lib/util/settings.js b/lib/util/settings.js index b6f1c72e8..978b231a6 100644 --- a/lib/util/settings.js +++ b/lib/util/settings.js @@ -638,10 +638,18 @@ function removeGroup(groupIDorName) { write(); } -function changeDeviceOptions(IDorName, newOptions) { - const device = getDeviceThrowIfNotExists(IDorName); +function changeEntityOptions(IDorName, newOptions) { const settings = get(); - objectAssignDeep(settings.devices[device.ID], newOptions); + delete newOptions.friendly_name; + delete newOptions.devices; + if (getDevice(IDorName)) { + objectAssignDeep(settings.devices[getDevice(IDorName).ID], newOptions); + } else if (getGroup(IDorName)) { + objectAssignDeep(settings.groups[getGroup(IDorName).ID], newOptions); + } else { + throw new Error(`Device or group '${IDorName}' does not exist`); + } + write(); } @@ -678,7 +686,7 @@ module.exports = { removeGroup, addDeviceToGroup, removeDeviceFromGroup, - changeDeviceOptions, + changeEntityOptions, changeFriendlyName, // For tests only diff --git a/test/bridge.test.js b/test/bridge.test.js index 74f61ef21..54d156fd0 100644 --- a/test/bridge.test.js +++ b/test/bridge.test.js @@ -391,4 +391,41 @@ describe('Bridge', () => { {retain: false, qos: 0}, expect.any(Function) ); }); + + it('Should allow change device options', async () => { + MQTT.publish.mockClear(); + expect(settings.getDevice('bulb')).toStrictEqual({"ID": "0x000b57fffec6a5b2", "friendlyName": "bulb", "friendly_name": "bulb", "retain": true}); + MQTT.events.message('zigbee2mqtt/bridge/request/device/changeOptions', JSON.stringify({options: {retain: false, transition: 1}, ID: 'bulb'})); + await flushPromises(); + expect(settings.getDevice('bulb')).toStrictEqual({"ID": "0x000b57fffec6a5b2", "friendlyName": "bulb", "friendly_name": "bulb", "retain": false, "transition": 1}); + expect(MQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/device/changeOptions', + JSON.stringify({"data":{"from":{"retain": true},"to":{"retain": false,"transition":1}, "ID":"bulb"},"status":"ok"}), + {retain: false, qos: 0}, expect.any(Function) + ); + }); + + 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, "friendlyName": "group_1", "optimistic": true}); + MQTT.events.message('zigbee2mqtt/bridge/request/group/changeOptions', JSON.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, "friendlyName": "group_1", "optimistic": true, "transition": 1}); + expect(MQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/group/changeOptions', + JSON.stringify({"data":{"from":{"optimistic": true,"retain": false},"to":{"optimistic": true,"retain": true,"transition":1}, "ID":"group_1"},"status":"ok"}), + {retain: false, qos: 0}, expect.any(Function) + ); + }); + + it('Should throw error on invalid device change options payload', async () => { + MQTT.publish.mockClear(); + MQTT.events.message('zigbee2mqtt/bridge/request/device/changeOptions', JSON.stringify({options_: {retain: true, transition: 1}, ID: 'bulb'})); + await flushPromises(); + expect(MQTT.publish).toHaveBeenCalledWith( + 'zigbee2mqtt/bridge/response/device/changeOptions', + JSON.stringify({"data":{},"status":"error","error":"Invalid payload"}), + {retain: false, qos: 0}, expect.any(Function) + ); + }); }); diff --git a/test/settings.test.js b/test/settings.test.js index f4bac8d55..8a7447a6b 100644 --- a/test/settings.test.js +++ b/test/settings.test.js @@ -359,6 +359,14 @@ describe('Settings', () => { expect(settings.get().groups).toStrictEqual(expected); }); + it('Should throw error when changing entity options of non-existing device', () => { + write(configurationFile, {}); + + expect(() => { + settings.changeEntityOptions('not_existing_123', {}); + }).toThrow(new Error("Device or group 'not_existing_123' does not exist")); + }); + it('Should not add duplicate groups', () => { write(configurationFile, {});