[feature] debounce for groups

[sjorge]

Bug Report

What happened

I noticed I get a attReport from 2 bulbs in a group usually within a second of eachother, the group update get published twice to mqtt with the sam payload.

What did you expect to happen

I expected to be able to set 'debounce' for a group, but that had no effect.
Looking at the code, I don't think that is currently support. So I guess this is a feature request.

How to reproduce it (minimal and precise)

Add 2 trådfri bulbs to a group, then change the state via the group.

• both bulbs change state
• both bulbs withing < 1 sec publish an attReport for genOnOff with the new state
• group gets published twice more (I think 3 times as once gets published form setting the state via the group too)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sjorge]

Still wanted

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sjorge]

Unstale

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

[sjorge]

Unstale

[Koenkk]

I'm wondering that instead of debounce, we can also not publish a new group state when there is nothing different to be published. This has to advantage of not having to wait the debounce time before a new state gets published.

[sjorge]

Should we not try to keep the behavior consistent between groups and devices?

Although both would work for my use case.

[Koenkk]

Both yes and no, the no part is not to add features/settings for things that are not used. Besides that not republishing the "optimistic" state for groups when it didn't change makes a lot of sense, I can't imagine why you don't want this.

[sjorge]

True but should this not be done for groups and devices then? (although, I guess if the elapse or last_seen are in the payload we then should...)

Edit, attempt at better phrasing

Yes, not updating when there is no change makes more sense and would make debounce for groups not necessary.
But if we do this publish on change only, we should also do it for devices, as it would also make sense there.

Although for devices there are some exceptions I think like when the elapsed or last_seen payload is present.

[Koenkk]

I only meant the optimistic state for groups based on device changes. So a /set on a group will still publish a state update, even when identical. But when e.g. 2 bulbs in a group report they have been turned off, the group will only publish it's updated state once.

Please try the groups.js changes from https://github.com/Koenkk/zigbee2mqtt/pull/4623/files

[sjorge]

Seems to behave as expected.

Both triggered via remote or via mqtt will result in 1 publish

[Koenkk]

Good, does it solve your OP issue? Because this is exactly what you described in "How to reproduce"

[sjorge]

Yeah this is fine for my use case. my 6 bulb groups now no longer spam mqtt messages 1-3 sec after toggling them via de group.

[Koenkk]

Great, merged I assume we can close this now 😄

[sjorge]

Something is still broken after restarting z2m, I need to investigate more but it looks like it does not republish retained messages for groups at startup now.

[sjorge]

I've just verified this is indeed the case! :(

How I verified this:

• start the following mqtt subs
  • mosquitto_sub -t 'zigbee2mqtt/office/floor_light' | jq .
  • mosquitto_sub -t 'zigbee2mqtt/office/floor_light/bulb' | jq .
• restart zigbee2mqtt

Only the sub that was for a device got the payload published again as expected, the sub for the group did not.
This breaks most of my node-red integration, I assume it's a side effect of checking if the current payload changed vs the cached payload. Which after restart, would be the case and also be skipped.

I guess the one that publishes at startup should get called with a 'force' flag or something that will always (re)publish.

[Koenkk]

@sjorge fixed

[sjorge]

Looks good now after grabbing the latest dev.