Replace /group/0 group cast handing with broadcast

[manup]

Currently /group/0 is treated as a Zigbee group with id 0xfff0. This causes various problems like devices sending unwanted reports to this group and other devices reacting to these. The group also occupied an addition group slot on each device.

The PR makes the following changes:

• Sends all commands directed to /group/0 as a broadcast to all routers where rxOnWhenIdle = true;
• Removes Zigbee group 0xfff0 which represented /group/0 from devices if it is found via Get Group Membership Request;
• The internal name of /group/0 was changed from "All" to "__All" so that users are able to create a group named "All".

It should be noted that the usage of /group/0 in general is discouraged since for example in moderate complex networks with lots of different devices the reception of on/off command might cause devices other than lights to turn on/off. Creating explicit groups via the REST API provides finer control on which devices should receive a command.

Replaces PR: https://github.com/dresden-elektronik/deconz-rest-plugin/pull/3966

Issue: https://github.com/dresden-elektronik/deconz-rest-plugin/issues/3744

[ebaauw]

I think this a step backwards, see my comment on the previous PR: https://github.com/dresden-elektronik/deconz-rest-plugin/pull/3966#issuecomment-770698996.

[manup]

Ah I see, didn't have this in my mind when swing the bold removal hammer :)

Imho /groups/0 is not about broadcast vs groupcast, but about (eventually) offering a compatible /scenes API endpoint. I think:

• Only device endpoints with a server _Groups _ cluster should be exposed as /lights endpoints;
• The group corresponding to /groups/0 should be added to all these endpoints (or: all /lights resources should be added to /groups/0);
• state.on and action.on should be used only for the server On/Off cluster. Other devices (like Window Covering) should use different attributes (state.lift, action.lift) that result in proper commands being sent to groups, thus enabling a group of e.g. Window Covering or Warning Device devices.
• Whenever we expose non-light endpoints under a different API endpoint (/plugs, /windowCoverings, /warningDevices), these resources should be in /groups/0.

So in retrospect here are some thoughts:

• By default consider group0 membership if the Server Groups Cluster exist on the resource main endpoint, perhaps with further filters if needed since we always have some exceptions to the rule;
• Don't rely on device type;
• Mark a Resource when it should be part of group0. To keep it generic and not tied to LightNodes perhaps as non-public ResourceItem "attr/flags" carrying configuration/behavior flags like R_FLAG_GROUP0 or R_FLAG_LIGHT_SLEEPER, etc.;
• Since this can be specified in DDF files it doesn't need to be hard coded in C++ (later on);
• Only Resources which have the flag can invoke add to group0. For example in the case that a device has LightNode and a Sensor only the light Resource can trigger that the group0 is added. All other Resources ignore group0 handling;

bool R_HasFlags(const Resource *r, qint64 flags)
{
    const auto *item = r ? r->item(RAttrFlags) : nullptr;
    return item && (item(RAttrFlags)->toNumber() & flags) == flags;
}

if (R_HasFlags(lightNode, R_FLAG_GROUP0))
{
    // verify / add group0
}

And in the Get Group Membership Response group0 will only be handled if (R_HasFlags(..., R_FLAG_GROUP0)) so that the group isn't removed accidentally.

I just remembered that reporting was disabled for IKEA lights SwBuildId via https://github.com/dresden-elektronik/deconz-rest-plugin/pull/3446 this might have already solved the prior seen issues with Basic Cluster group casts originating from IKEA lights and disturbing Xiaomi mains powered switches. For some reason IKEA lights create a group binding from Basic Cluster when added to a group.

[SwoopX]

Well, I guess we all have our own perspective on this here. I basically agree with all the points made above, however, there's one thing which troubles me while only occurring eventually: Buttons triggering the All group while not explicitly being configured for that.

I had my own story around that. To keep it short: new switch paired to my production, pressing the "off" button of one row, whole place shut down, literally, me being pi**ed, no joy at 1AM. There's other stories around, mainly for the Opple, where it triggers everything and o/c we have the Ikea blinds...

So this "connection" must be broken. I have hoped the PR could somehow have addressed and I was sure (bad memory) the other PR being out there for quite some time had a solution for that... So I stuck my nose in the code and guess finally found the reason for that being an inconvenient order of code and defaults.

Function getGroupForId() defaults to All group when it is called and the group IDs associated to a switch are still in their initialization state (0). It probably plays a role when the group item is added, but if the initialized group ID 0 is not found resulting in the gid of 0xfff0 in checkSensorBindingsForClientClusters prior to binding the whole thing starts.

Over time, this all has probably become so damn twisted that it could use some decent overhauling. Meanwhile, the solution to what I'm after is probably simply to disallow switch bindings against 0xfff0 in checkSensorBindingsForAttributeReporting 🤷‍♂️

[manup]

Function getGroupForId() defaults to All group when it is called and the group IDs associated to a switch are still in their initialization state (0).

Over time, this all has probably become so damn twisted that it could use some decent overhauling. Meanwhile, the solution to what I'm after is probably simply to disallow switch bindings against 0xfff0 in checkSensorBindingsForAttributeReporting :man_shrugging:

Indeed this sounds like a solid cause for trouble. Skipping gwGroup0 in checkSensorBindingsForAttributeReporting() should solve this, albeit I think it might perhaps be better to fix this where the initialization state (0) of the switch group is handled? The 0 group id imho should only be accessed from clients via REST-API /groups/0 and not conflict switch initialization, or perhaps it should be initialization state (0xffff — non existing group id) to resolve the conflict?

[SwoopX]

Indeed this sounds like a solid cause for trouble. Skipping gwGroup0 in checkSensorBindingsForAttributeReporting() should solve this, albeit I think it might perhaps be better to fix this where the initialization state (0) of the switch group is handled? The 0 group id imho should only be accessed from clients via REST-API /groups/0 and not conflict switch initialization, or perhaps it should be initialization state (0xffff — non existing group id) to resolve the conflict?

I assume the topic is twofold: The specs define valid groups from 0x0001 - 0xfff0, in that sense all groups are initialized with a disallowed group, so that shouldn't necessarily pose an issue. However, for a Group object, the ID (decimal) always equals the address (hex). While iterating through all available groups, 0 cannot be found as it will never exist and getGroupForId() demands to default to gwGroup0 (currently 0xfff0) in those cases.

So changing the "initialization group" wouldn't have any effect, as it will not be found and the binding will default again to 0xfff0 Another interesting aspect here is that this binding will override the group determined by the device itself. Apparently, disallowing any bindings against 0xfff0 would ease the observed effects, but also explicitly forbid using the All group in case that's intentionally desired.

How about mising the thoughts here? If we would default to either group 0x0000 or 0xffff in getGroupForId() and then disallow those for binding in checkSensorBindingsForClientClusters (refered to the wrong function in the post above btw.), I guess we can elegantly dance around the issue with minimal effort. Maybe even better to create that group. That should also preserve the groups the device has chosen during pairing and allow deconz to eavesdrop the used groups.

[justme-1968]

as there are events send for the special groups maybe 'artificial' groups should appear in the api call for /groups/ ? or if calls to group0 are routed to 0xfff0 events for 0xfff0 should be mapped back to group0. or there should one no events generated for such groups that have not been created by and are visible to the enduser.