diff --git a/lib/extension/groups.js b/lib/extension/groups.js index 4cec86e40..2b9990b14 100644 --- a/lib/extension/groups.js +++ b/lib/extension/groups.js @@ -102,7 +102,7 @@ class Groups extends Extension { const resolvedEntity = this.zigbee.resolveEntity(`${data.ID}${endpointName ? `/${endpointName}` : ''}`); const zigbeeGroups = this.zigbee.getGroups().filter((zigbeeGroup) => { const settingsGroup = settings.getGroup(zigbeeGroup.groupID); - return settingsGroup && settingsGroup.optimistic; + return settingsGroup && (!settingsGroup.hasOwnProperty('optimistic') || settingsGroup.optimistic); }); if (resolvedEntity.type === 'device') { diff --git a/lib/extension/publish.js b/lib/extension/publish.js index 254bc405d..512d0deee 100644 --- a/lib/extension/publish.js +++ b/lib/extension/publish.js @@ -224,10 +224,8 @@ class EntityPublish extends Extension { if (topic.type === 'set' && converter.convertSet) { logger.debug(`Publishing '${topic.type}' '${key}' to '${resolvedEntity.name}'`); const result = await converter.convertSet(actualTarget, key, value, meta); - const isGroup = actualTarget.constructor.name == 'Group'; - // Optimistic behaviour of groups and devices is different. - const publishOptimistic = isGroup || (!options.hasOwnProperty('optimistic') || options.optimistic); - if (result && result.state && publishOptimistic) { + const optimistic = !options.hasOwnProperty('optimistic') || options.optimistic; + if (result && result.state && optimistic) { const msg = result.state; if (endpointName) { @@ -242,7 +240,7 @@ class EntityPublish extends Extension { this.publishEntityState(resolvedEntity.settings.ID, msg); } - if (result && result.membersState && publishOptimistic) { + if (result && result.membersState && optimistic) { for (const [ieeeAddr, state] of Object.entries(result.membersState)) { this.publishEntityState(ieeeAddr, state); } diff --git a/lib/util/settings.js b/lib/util/settings.js index eb3f27a47..107863648 100644 --- a/lib/util/settings.js +++ b/lib/util/settings.js @@ -426,12 +426,12 @@ function getGroup(IDorName) { const settings = getWithDefaults(); const byID = settings.groups[IDorName]; if (byID) { - return {optimistic: true, devices: [], ...byID, ID: Number(IDorName), friendlyName: byID.friendly_name}; + return {devices: [], ...byID, ID: Number(IDorName), friendlyName: byID.friendly_name}; } for (const [ID, group] of Object.entries(settings.groups)) { if (group.friendly_name === IDorName) { - return {optimistic: true, devices: [], ...group, ID: Number(ID), friendlyName: group.friendly_name}; + return {devices: [], ...group, ID: Number(ID), friendlyName: group.friendly_name}; } } @@ -441,7 +441,7 @@ function getGroup(IDorName) { function getGroups() { const settings = getWithDefaults(); return Object.entries(settings.groups).map(([ID, group]) => { - return {optimistic: true, devices: [], ...group, ID: Number(ID), friendlyName: group.friendly_name}; + return {devices: [], ...group, ID: Number(ID), friendlyName: group.friendly_name}; }); } diff --git a/test/bridge.test.js b/test/bridge.test.js index 547000ba9..ab29fe095 100644 --- a/test/bridge.test.js +++ b/test/bridge.test.js @@ -513,7 +513,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')).toBeNull(); - expect(settings.getGroup('group_new_name')).toStrictEqual({"ID": 1, "devices": [], "friendly_name": "group_new_name", "friendlyName": "group_new_name", "optimistic": true, "retain": false}); + expect(settings.getGroup('group_new_name')).toStrictEqual({"ID": 1, "devices": [], "friendly_name": "group_new_name", "friendlyName": "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', @@ -610,13 +610,13 @@ 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, "friendlyName": "group_1", "optimistic": true}); + expect(settings.getGroup('group_1')).toStrictEqual({"ID": 1, "devices": [], "friendly_name": "group_1", "retain": false, "friendlyName": "group_1"}); 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, "friendlyName": "group_1", "optimistic": true, "transition": 1}); + expect(settings.getGroup('group_1')).toStrictEqual({"ID": 1, "devices": [], "friendly_name": "group_1", "retain": true, "friendlyName": "group_1", "transition": 1}); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/response/group/options', - stringify({"data":{"from":{"optimistic": true,"retain": false},"to":{"optimistic": true,"retain": true,"transition":1}, "id":"group_1"},"status":"ok"}), + stringify({"data":{"from":{"retain": false},"to":{"retain": true,"transition":1}, "id":"group_1"},"status":"ok"}), {retain: false, qos: 0}, expect.any(Function) ); }); @@ -636,7 +636,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", "friendlyName": "group_193", "optimistic": true}); + expect(settings.getGroup('group_193')).toStrictEqual({"ID": 3, "devices": [], "friendly_name": "group_193", "friendlyName": "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', @@ -649,7 +649,7 @@ describe('Bridge', () => { MQTT.publish.mockClear(); MQTT.events.message('zigbee2mqtt/bridge/request/group/add', stringify({friendly_name: "group_193", id: 9})); await flushPromises(); - expect(settings.getGroup('group_193')).toStrictEqual({"ID": 9, "devices": [], "friendly_name": "group_193", "friendlyName": "group_193", "optimistic": true}); + expect(settings.getGroup('group_193')).toStrictEqual({"ID": 9, "devices": [], "friendly_name": "group_193", "friendlyName": "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/legacy/bridgeLegacy.test.js b/test/legacy/bridgeLegacy.test.js index c33df6d85..b87ec2750 100644 --- a/test/legacy/bridgeLegacy.test.js +++ b/test/legacy/bridgeLegacy.test.js @@ -188,7 +188,7 @@ describe('Bridge legacy', () => { await flushPromises(); expect(MQTT.publish.mock.calls[0][0]).toStrictEqual('zigbee2mqtt/bridge/log'); const payload = JSON.parse(MQTT.publish.mock.calls[0][1]); - expect(payload).toStrictEqual({"message":[{"ID":1,"devices":[],"friendly_name":"group_1","optimistic":true,"retain":false},{"ID":2,"devices":[],"friendly_name":"group_2","optimistic":true,"retain":false},{"ID":11,"devices":["bulb_2"],"friendly_name":"group_with_tradfri","optimistic":true,"retain":false},{"ID":12,"devices":["TS0601_thermostat"],"friendly_name":"thermostat_group","optimistic":true,"retain":false},{"ID":14,"devices":["power_plug"],"friendly_name":"switch_group","optimistic":true,"retain":false},{"ID":21,"devices":["GLEDOPTO_2ID/cct"],"friendly_name":"gledopto_group","optimistic":true},{"ID":15071,"devices":["bulb_color_2","bulb_2"],"friendly_name":"group_tradfri_remote","optimistic":true,"retain":false}],"type":"groups"}); + expect(payload).toStrictEqual({"message":[{"ID":1,"devices":[],"friendly_name":"group_1","retain":false},{"ID":2,"devices":[],"friendly_name":"group_2","retain":false},{"ID":11,"devices":["bulb_2"],"friendly_name":"group_with_tradfri","retain":false},{"ID":12,"devices":["TS0601_thermostat"],"friendly_name":"thermostat_group","retain":false},{"ID":14,"devices":["power_plug"],"friendly_name":"switch_group","retain":false},{"ID":21,"devices":["GLEDOPTO_2ID/cct"],"friendly_name":"gledopto_group"},{"ID":15071,"devices":["bulb_color_2","bulb_2"],"friendly_name":"group_tradfri_remote","retain":false}],"type":"groups"}); }); it('Should allow rename devices', async () => { @@ -221,10 +221,10 @@ describe('Bridge legacy', () => { it('Should allow rename groups', async () => { MQTT.publish.mockClear(); - expect(settings.getGroup(1)).toStrictEqual({"ID": 1, devices: [], friendlyName: "group_1", "friendly_name": "group_1", optimistic: true, retain: false}); + expect(settings.getGroup(1)).toStrictEqual({"ID": 1, devices: [], friendlyName: "group_1", "friendly_name": "group_1", retain: false}); MQTT.events.message('zigbee2mqtt/bridge/config/rename', stringify({old: 'group_1', new: 'group_1_renamed'})); await flushPromises(); - expect(settings.getGroup(1)).toStrictEqual({"ID": 1, devices: [], friendlyName: "group_1_renamed", "friendly_name": "group_1_renamed", optimistic: true, retain: false}); + expect(settings.getGroup(1)).toStrictEqual({"ID": 1, devices: [], friendlyName: "group_1_renamed", "friendly_name": "group_1_renamed", retain: false}); expect(MQTT.publish).toHaveBeenCalledWith( 'zigbee2mqtt/bridge/log', stringify({type: 'group_renamed', message: {from: 'group_1', to: 'group_1_renamed'}}), @@ -270,7 +270,7 @@ describe('Bridge legacy', () => { {qos: 0, retain: false}, expect.any(Function) ); - 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", devices: []}); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(3); }); @@ -279,7 +279,7 @@ describe('Bridge legacy', () => { 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", devices: [], optimistic: true}); + expect(settings.getGroup('new_group')).toStrictEqual({"ID": 3, "friendlyName": "new_group", "friendly_name": "new_group", devices: []}); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(3); expect(MQTT.publish).toHaveBeenCalledWith( @@ -294,7 +294,7 @@ describe('Bridge legacy', () => { 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", devices: [], optimistic: true}); + expect(settings.getGroup('new_group')).toStrictEqual({"ID": 42, "friendlyName": "new_group", "friendly_name": "new_group", devices: []}); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(42); expect(MQTT.publish).toHaveBeenCalledWith( @@ -309,7 +309,7 @@ describe('Bridge legacy', () => { 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", devices: [], optimistic: true}); + expect(settings.getGroup('group_42')).toStrictEqual({"ID": 42, "friendlyName": "group_42", "friendly_name": "group_42", devices: []}); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledTimes(1); expect(zigbeeHerdsman.createGroup).toHaveBeenCalledWith(42) }); diff --git a/test/publish.test.js b/test/publish.test.js index 91b447760..716d3e6ec 100644 --- a/test/publish.test.js +++ b/test/publish.test.js @@ -422,6 +422,13 @@ describe('Publish', () => { expect(MQTT.publish).toHaveBeenCalledTimes(0); }); + it('Shouldnt publish new state when optimistic = false for group', async () => { + settings.set(['groups', '2', 'optimistic'], false); + await MQTT.events.message('zigbee2mqtt/group_2/set', stringify({brightness: '200'})); + await flushPromises(); + expect(MQTT.publish).toHaveBeenCalledTimes(0); + }); + it('Should handle non-valid topics', async () => { await MQTT.events.message('zigbee2mqtt1/bulb_color/set', stringify({state: 'ON'})); await flushPromises(); diff --git a/test/settings.test.js b/test/settings.test.js index 8b131303d..3ca026017 100644 --- a/test/settings.test.js +++ b/test/settings.test.js @@ -352,7 +352,6 @@ describe('Settings', () => { friendlyName: '123', friendly_name: '123', devices: [], - optimistic: true, }; expect(group).toStrictEqual(expected); @@ -378,7 +377,6 @@ describe('Settings', () => { friendlyName: '123', friendly_name: '123', devices: [], - optimistic: true, }; expect(group).toStrictEqual(expected); @@ -420,7 +418,6 @@ describe('Settings', () => { const group = settings.getGroup('1'); const expectedGroup = { - optimistic: true, ID: 1, friendlyName: '123', friendly_name: '123',