Prepare to move specific ZDO requests out of Adapter #1187

Nerivec · 2024-09-14T23:34:05Z

Final goal is to remove the following functions from Adapter in favor of a generic sendZdo with requests built and sent directly from Controller/Device/Endpoint:

changeChannel
permitJoin
lqi
routingTable
nodeDescriptor
activeEndpoints
simpleDescriptor
bind
unbind
removeDevice

This should make the API far more robust against changes (by not requiring updates in every adapter) and offer full payloads to the upstream code (instead of the current limited set of properties), which could lead to a few new features.
Due to the sizeable overhaul required in some stacks, this PR will focus first on introducing the sendZdo function while keeping above functions in place until they can safely be removed (all adapters are required to have support before that can happen).

A working example (albeit using an older implementation) of the full update for ember (as well as some tentative partial implementations in other stacks) can be found in this branch.

TODO:

TODO separate PR:

Move ZDO requests to upstream code (done but not yet added to the PR for compat.)
Remove deviceAnnounce and networkAddress event from Adapter
Remove above functions from Adapter & impl.
Fix tests
- Controller
- ember
- zstack

CC: @kirovilya @ChrisHae @G1K @devbis contributions/ideas on specific stacks welcome!

src/controller/controller.ts

Nerivec · 2024-09-18T21:42:26Z

@kirovilya Could you test out zboss?
@devbis @fairecasoimeme Could you test out zigate?

Lots of moving parts, need to confirm nothing broke. If you can report back issues.
A round of permit join + join device (interview) + network map + remove device should pretty much trigger all involved ZDOs.
Change channel requires changing the channel in configuration.yaml then starting Z2M again. (Though depending on the start logic of the adapter, it might not trigger at all.)

src/adapter/zboss/frame.ts

src/adapter/zboss/adapter/zbossAdapter.ts

kirovilya · 2024-09-19T18:47:18Z

src/adapter/zboss/adapter/zbossAdapter.ts

+                // `authentication`: TC significance always 1 (zb specs)
+                const zdoPayload = Zdo.Buffalo.buildRequest(this.hasZdoMessageOverhead, clusterId, seconds, 1, []);
+
+                await this.sendZdo(ZSpec.BLANK_EUI64, ZSpec.BroadcastAddress.DEFAULT, clusterId, zdoPayload, true);


there is a special feature for the ZBOSS - you need to send 2 commands: to the coordinator and the broadcast.
await this.sendZdo(ZSpec.BLANK_EUI64, networkAddress, clusterId, zdoPayload, true);

So proper logic is like this?

if (networkAddress) { // device-only when network > 0 await this.sendZdo(ZSpec.BLANK_EUI64, networkAddress, clusterId, zdoPayload, true); } else { // coordinator-only when networkAddress === 0 or undefined (all) await this.sendZdo(ZSpec.BLANK_EUI64, ZSpec.COORDINATOR_ADDRESS, clusterId, zdoPayload, true); if (networkAddress === undefined) { // all await this.sendZdo(ZSpec.BLANK_EUI64, ZSpec.BroadcastAddress.DEFAULT, clusterId, zdoPayload, true); } }

yes. or combine in one block with 2 commands sendZdo - always send a broadcast, even if we connect it to some router

Not separating it would break what Z2M expects (from the dropdown in frontend).

123 => device-only

0 => coordinator-only

undefined => all (coordinator+broadcast)

So I'll push like that, let me know if that doesn't work out in zboss.

to join to the coordinator, I have to send 2 commands: one with the coordinator's address (0), and the second with the broadcast. only in this case, the joining of devices begins.
to join to the router, I sent only one command (without broadcast), but the device did not connect to it (the concentrator did not give permission to connect) and I haven't been able to beat it yet. maybe we should send another broadcast team, but I haven't checked yet.

I'm seeing a zb_nlme_permit_joining_request(duration) in espressif zboss library. Could it be similar to emberznet, where there's a special function to open the network on the coordinator-only without going through a ZDO?

Koenkk · 2024-09-19T19:58:37Z

src/adapter/z-stack/adapter/zStackAdapter.ts

@@ -221,14 +214,25 @@ class ZStackAdapter extends Adapter {
    }

    public async permitJoin(seconds: number, networkAddress?: number): Promise<void> {
-        const addrmode = networkAddress === undefined ? 0x0f : 0x02;
-        const dstaddr = networkAddress || 0xfffc;
+        const clusterId = Zdo.ClusterId.PERMIT_JOINING_REQUEST;


Note to self: check if this doesn't break https://github.com/Koenkk/Z-Stack-firmware/blob/7398d834eb3a790876c280293c4181da96cc7114/coordinator/Z-Stack_3.x.0/firmware.patch#L274 and https://github.com/Koenkk/Z-Stack-firmware/blob/7398d834eb3a790876c280293c4181da96cc7114/coordinator/Z-Stack_3.x.0/firmware.patch#L579

Koenkk · 2024-09-19T20:01:29Z

src/adapter/z-stack/adapter/zStackAdapter.ts

-                return result;
-            } catch (error) {
-                logger.debug(`Node descriptor request for '${networkAddress}' failed (${error}), retry`, NS);
-                // Doing a route discovery after simple descriptor request fails makes it succeed sometimes.


This logic is lost now, I guess we need to move the logic up.

The discovery bit? I made a note of that.

I'm thinking maybe adding a function to Adapter for failed requests, something like this: onFailedRequest(ieeeAddress, networkAddress, profileId, clusterId), which might also allow to make the other various retry logic more generic in the future (upstream in Controller/Device instead of Adapter).
Thoughts?
I guess we could also have a discoverRoute function in Adapter (which may not do anything if not needed). This is quite stack-specific, emberznet does this automatically for any frame via APS option for example.
I guess we could temporarily add it back in sendZdo, I expect this would more or less do the same?

if (clusterId === Zdo.ClusterId.NODE_DESCRIPTOR_REQUEST) { await this.discoverRoute(networkAddress); } await this.znp.requestZdo(clusterId, payload, waiter?.ID);

Koenkk · 2024-09-19T20:03:45Z

src/adapter/z-stack/adapter/zStackAdapter.ts

+                case Zdo.ClusterId.PERMIT_JOINING_REQUEST: {
+                    const finalPayload = Buffer.alloc(payload.length + 3);
+                    finalPayload.writeUInt8(ZSpec.BroadcastAddress[networkAddress] ? AddressMode.ADDR_BROADCAST : AddressMode.ADDR_16BIT, 0);
+                    // TODO: confirm zstack uses AddressMode.ADDR_16BIT + ZSpec.BroadcastAddress.DEFAULT to signal "coordinator-only" (assumed from previous code)


Note to self; check this

Koenkk · 2024-09-19T20:08:30Z

src/adapter/z-stack/znp/znp.ts

@@ -308,6 +312,30 @@ class Znp extends events.EventEmitter {
        });
    }

+    public requestZdo(clusterId: ZdoClusterId, payload: Buffer, waiterID?: number): Promise<void> {


My expectation was that we don't use the ZNP specific ZDO commands but just AF_DATA_REQUEST, wouldn't it be easier to use that?

Don't know if the ZDO rsp payloads will match the same? Responses would still come in as AREQ/ZDO?
Probably have to rework the req payloads (looks like the network address would no longer be prefixing at least?).
Does it work for NETWORK_ADDRESS_REQUEST too (just wondering since it has to use the ieee from the zdo payload and not dstaddr from data req)?
Something like this would work out-of-the-box?

// await this.znp.requestZdo(clusterId, payload, waiter?.ID); await this.dataRequest(networkAddress, Zdo.ZDO_ENDPOINT, Zdo.ZDO_ENDPOINT, clusterId, Constants.AF.DEFAULT_RADIUS, payload, 15000);

Nerivec force-pushed the zdo-move branch from e815187 to 6d64d18 Compare September 15, 2024 09:48

Koenkk mentioned this pull request Sep 16, 2024

Check for a changed network address when routing fails in zstack #1183

Draft

Nerivec force-pushed the zdo-move branch from d88f7c2 to cbe1b44 Compare September 17, 2024 20:55

Koenkk reviewed Sep 18, 2024

View reviewed changes

src/controller/controller.ts Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

kirovilya reviewed Sep 19, 2024

View reviewed changes

src/adapter/zboss/frame.ts Outdated Show resolved Hide resolved

kirovilya reviewed Sep 19, 2024

View reviewed changes

src/adapter/zboss/adapter/zbossAdapter.ts Outdated Show resolved Hide resolved

kirovilya reviewed Sep 19, 2024

View reviewed changes

Koenkk reviewed Sep 19, 2024

View reviewed changes

Nerivec added 17 commits September 19, 2024 23:20

Prepare to move specific ZDO requests out of Adapter

c0ec83c

Fix prettier.

80521fe

Fix zstack tests.

7f8b47c

Fix ember tests.

2ec6c6d

More coverage.

d9bceb9

Fix prettier.

05e6fd8

Add tests for zstack ZDO request payload alteration logic.

6380fbb

Cleanup zstack ZDO response handling & more robust tests.

cd3f1a7

Extra zstack edge-case coverage.

c517e12

Update zstack to use sendZdo.

1c1e4ee

zstack: Match AREQ/SREQ logic, cleanup and optimize. More coverage.

46706e9

Better condition for IEEE vs nwk matching (avoid possible edge-cases).

f7d9b0b

deconz

cd4153d

ezsp

2cd42bf

Fix prettier.

5ad8540

ezsp: fix not compatible with 8.x.x.

4fb5471

zboss

3501f77

Nerivec added 9 commits September 19, 2024 23:20

zboss: fix.

ddafff5

deconz: fix deviceJoined event

d3b418c

zigate

75567f4

zigate: better zdo buffalo patch

8dbf0a1

zboss: fix ignore response status in driver (handled upstream)

8c68efb

zboss: fix non-standard ZDO response payloads

59b7c92

zboss: fix leftover

49ab0cc

zboss: fix permit join.

824a801

zboss: fix INDICATION payload

f5bc52b

Nerivec force-pushed the zdo-move branch from 4655079 to f5bc52b Compare September 19, 2024 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare to move specific ZDO requests out of Adapter #1187

Prepare to move specific ZDO requests out of Adapter #1187

Nerivec commented Sep 14, 2024 •

edited

Loading

Nerivec commented Sep 18, 2024

This comment was marked as resolved.

kirovilya Sep 19, 2024

Nerivec Sep 19, 2024

kirovilya Sep 19, 2024

Nerivec Sep 19, 2024 •

edited

Loading

kirovilya Sep 19, 2024

Nerivec Sep 19, 2024

Koenkk Sep 19, 2024 •

edited

Loading

Koenkk Sep 19, 2024

Nerivec Sep 19, 2024

Koenkk Sep 19, 2024

Koenkk Sep 19, 2024

Nerivec Sep 19, 2024 •

edited

Loading

Prepare to move specific ZDO requests out of Adapter #1187

Are you sure you want to change the base?

Prepare to move specific ZDO requests out of Adapter #1187

Conversation

Nerivec commented Sep 14, 2024 • edited Loading

Nerivec commented Sep 18, 2024

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nerivec Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Koenkk Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nerivec Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Nerivec commented Sep 14, 2024 •

edited

Loading

Nerivec Sep 19, 2024 •

edited

Loading

Koenkk Sep 19, 2024 •

edited

Loading

Nerivec Sep 19, 2024 •

edited

Loading