feat: emberAdapter: add SEND_MULTICASTS_TO_SLEEPY_ADDRESS stack config.#1729
feat: emberAdapter: add SEND_MULTICASTS_TO_SLEEPY_ADDRESS stack config.#1729adriweb wants to merge 1 commit into
Conversation
b101b01 to
94577b3
Compare
Note: several commercial Zigbee stacks/coordinators use this option. P.S. This happens to be a "solution" to this zigbee2mqtt issue: Koenkk/zigbee2mqtt#30247
94577b3 to
0216bb7
Compare
|
Per Silabs source:
Also, this is hardcoded in a lot of converters (since this is spec compliance), even with this, the error in the linked issue would still be thrown. |
|
Yep, I noted the non-compliance thing in the commit I have locally for the zigbee2mqtt doc if this ends up being merged. Anyway, I mean, I've been testing it with zigbee2mqtt with several brands of routers and end devices, so far so good. It seemed fine to me as a custom optional user option only toggleable in a json config file. Do you know of specific issues that this would trigger on the z2m side so I could look into it? |
|
Every converter in ZHC with a hardcoded group reject would not work, even with this, since it rejects long before it reaches the adapter. The whole codebase is treating this as impossible, so, bits that won't work are going to be spread all over. It has the potential to break entire networks (takes just one router discarding these messages to make a mess), so, very tricky in my opinion. Can't rely on what a brand is doing to judge if it's safe, way too many are doing custom stuff, as long as it works with their hub... Anyhow, without compatibility overhaul on Z2M side, it wouldn't work. We can't even be sure if it is properly wired "everywhere" in the Silabs stack, since that's a custom that would always be disabled in internal testing. 🥵 |
|
Oh, well, yes it would definitely be much more manageable if this was done some other more standard-friendly way, I'm just not aware of anything else now. In fact, I wonder if this stack option is toggleable on the fly (i.e. what if we try to enable it right before sending out a command to such a special flagged group, then disable it back afterwards), I haven't checked... Note for context: several people (me included) tried to group window-covering devices (some of them battery-powered, so those are definitely not routers), and we see that the battery ones don't get group commands (or multicast in general, as expected) but they are still kind of always listening anyway (very frequent poll) since they are controllable individually just fine. With this option turned on, it does work fine for group too. Anyway, I'm sure you understood where this came from. Anyway, is there any hope of having this merged if we insist that "this is an option that might at best not work, at worst break stuff, and it shouldn't be used unless you know what you're doing and assume all responsibility yourself for whatever happens" or something like that? Or should the PR just be closed for now (until a zigbee2mqtt overhaul to better handle the consequences of that - in which case, we should probably know a bit more about what that involves)? |
|
This pull request is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Any word from a(nother?) maintainer, or should we just close it now since it's going to expire anyway? |
|
I understand it's not safe for main. In this case I'd like to see it added as an extension (if possible) |
|
As mentioned before, it's not just about it being not safe. It's that it won't work in a lot of cases, even with this. This is only stack-level (the extent of support being unknown, since untested/not recommended), but Z2M is also respecting the spec (save for device workarounds), and plenty of places will reject even without going to the adapter. It would only work for the subset that isn't actually properly validating (which hopefully, they aren't too many... despite this). |
|
Yep, this doesn't change anything for normal commands sent by any frontend or normal Z2m high-level MQTT commands. It only makes it possible for raw {
action: 'raw',
params: {
network_address: broadcastAddress, // 0xFFFF for instance for all devices, even if sleepy
dst_endpoint: 0xFF, // broadcast endpoint
src_endpoint: 1,
cluster_key: rawCommand.cluster_key,
disable_response: true,
zcl: {
frame_type: 1,
direction: 0,
disable_default_response: true,
command_key: rawCommand.command_key,
payload: rawCommand.payload,
}
}
}You'd have to construct your ( that's what I added in my app anyway, use it similarly at your own risks :) ) |
Note: several commercial Zigbee stacks/coordinators use this option.
P.S. This happens to be a "solution" to Koenkk/zigbee2mqtt#30247.
Tested successfully on a SONOFF Dongle-M coordinator.