Honour optimisitic false for groups (#7615)

* Honour optimisitic false for groups. https://github.com/Koenkk/zigbee2mqtt/issues/7571

* Remove default optimistic true for groups. https://github.com/Koenkk/zigbee2mqtt/issues/7571

* Updates
This commit is contained in:
Koen Kanters 2021-05-28 20:23:00 +02:00 committed by GitHub
parent 7185741881
commit 9941be1d1a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 27 additions and 25 deletions

View File

@ -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') {

View File

@ -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);
}

View File

@ -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};
});
}

View File

@ -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',

View File

@ -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)
});

View File

@ -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();

View File

@ -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',