Improve broadcasting via sendZclFrameToAll

[burmistrzak]

Just adds an optional parameter to the sendZclFrameToAll method so sleepy end devices (i.e. 0xFFFF) can be included in broadcast messages. 😊

Seems to be required for this to work properly.

[Nerivec]

I'd say no boolean, just a variable for the address:

/** Broadcast to all routers. */
export const BROADCAST_ADDRESS = 0xFFFC;
/** Broadcast to all non-sleepy devices. */
export const BROADCAST_RX_ON_WHEN_IDLE_ADDRESS = 0xFFFD;
/** Broadcast to all devices, including sleepy end devices. */
export const BROADCAST_SLEEPY_ADDRESS = 0xFFFF;

These should be with Zigbee spec types, usable everywhere.

And the adapter function becomes

public abstract sendZclFrameToAll(endpoint: number, zclFrame: ZclFrame, sourceEndpoint: number, address: number = BROADCAST_ADDRESS): Promise<void>;

Note:

ZigBee specifies three different broadcast addresses that reach different collections of nodes. Broadcasts are normally sent only to routers. Broadcasts can also be forwarded to end devices, either all of them or only those that do not sleep. Broadcasting to end devices is both significantly more resource-intensive and significantly less reliable than broadcasting to routers.

[burmistrzak]

I'd say no boolean, just a variable for the address:

Funnily enough, that was my initial idea. 😊

These should be with Zigbee spec types, usable everywhere.

Sure, is adapter.ts the right place for such definitions?

And the adapter function becomes

public abstract sendZclFrameToAll(endpoint: number, zclFrame: ZclFrame, sourceEndpoint: number, address: number = BROADCAST_ADDRESS): Promise<void>;

We'll have to keep the current behavior, so BROADCAST_RX_ON_WHEN_IDLE_ADDRESS should be default.

ZigBee specifies three different broadcast addresses that reach different collections of nodes. Broadcasts are normally sent only to routers. Broadcasts can also be forwarded to end devices, either all of them or only those that do not sleep. Broadcasting to end devices is both significantly more resource-intensive and significantly less reliable than broadcasting to routers.

For the mentioned use.case, reaching every sleeping ZED seems kinda important. 😉

[Nerivec]

Only 6 use cases of sendZclFrameToAll currently, all are for GP, so not complicated to refactor. Better to use the proper default broadcast (FFFC) and change calls as needed to FFFD or FFFF by specifying it.

[burmistrzak]

Only 6 use cases of sendZclFrameToAll currently, all are for GP, so not complicated to refactor. Better to use the proper default broadcast (FFFC) and change calls as needed to FFFD or FFFF by specifying it.

Sounds reasonable.
Will do that next, when I'm done here.

[burmistrzak]

These should be with Zigbee spec types, usable everywhere.

@Nerivec Not entirely sure what you mean by that. Can you please elaborate?

Edit: Do you mean something similar to status.ts, e.g. Zcl.Broadcast.NON_SLEEPY_DEVICES? 🤔

[Nerivec]

@Koenkk Not sure what convention we want to use for naming, but since we're gonna introduce the zdo spec too, I figure we can put global stuff at the root and zcl/zdo as subfolders. Feel free to rename! (I didn't move the content of the zcl folder here, since that's a bigger refactor. I also didn't add the million or so consts that can be introduced to remove magic numbers/strings throughout the code 😁)

I added a test that mirrors the requirements of the Bosch in Koenkk/zigbee-herdsman-converters#7427. I think it makes more sense that way?
ZHC can just do

 await meta.device.triggerBroadcast(255, 1, BroadcastAddress.SLEEPY, Zcl.Direction.CLIENT_TO_SERVER, 'boschSmokeDetectorSiren', Zcl.Clusters.ssIasZone.ID, {data: index});

[burmistrzak]

Not sure what convention we want to use for naming, but since we're gonna introduce the zdo spec too, I figure we can put global stuff at the root and zcl/zdo as subfolders.

Ah, I see! So I wasn't that far off with my guess. 😅

ZHC can just do

 await meta.device.triggerBroadcast(255, 1, BroadcastAddress.SLEEPY, Zcl.Direction.CLIENT_TO_SERVER, 'boschSmokeDetectorSiren', Zcl.Clusters.ssIasZone.ID, {data: index});

Very elegant solution!
Makes it generally more useful for converter developers. Excellent job! 🤝

[burmistrzak]

Can we export BroadcastAddress (or better zspec) at the root, so using it from ZHC is more convenient?

[Nerivec]

Waiting for Koenkk's input on naming & co, so we can export this properly. The spec folder structure will affect a lot of stuff, better to start with a solid base.

[Nerivec]

I figure, might as well keep entirely in line with the rest, so zclCommandToAll seems the best option?

@Koenkk Can you confirm if this.customClusters should be passed in this scenario (to getCluster and to ZclFrame.create)? Same question for this.manufacturerID to getCluster (that's what endpoint is using)? Seems a bit conflicting between using device (this) and using options.

[burmistrzak]

I figure, might as well keep entirely in line with the rest, so zclCommandToAll seems the best option?

Agree. 👍
And thanks for cleaning up after me, looking good so far. 🤝

[burmistrzak]

Waiting for Koenkk's input on naming & co, so we can export this properly. The spec folder structure will affect a lot of stuff, better to start with a solid base.

@Koenkk The proposed folder structure seems sensible, but we'll eventually have to move ZCL to the Zspec directory.

• src/zspec/
• src/zspec/zcl/
• src/zspec/zdo/

Ultimately, it's your decision. 😉

[Nerivec]

The move will be in a separate PR. It was just the right time to create the base for it since we needed a couple of "global zigbee" consts for this.

[burmistrzak]

The move will be in a separate PR. It was just the right time to create the base for it since we needed a couple of "global zigbee" consts for this.

Sounds good.
Should we already export BroadcastAddress (e.g. as Zspec.BroadcastAddress.SLEEPY), before the "big move" PR comes around? Or do we want to keep these things in separate PRs?

[Koenkk]

@burmistrzak let's do this in a separate PR such that we can also merge Koenkk/zigbee-herdsman-converters#7427 before the new release.