-
Notifications
You must be signed in to change notification settings - Fork 41
Allow sending and receiving raw ZDO commands #109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
01e773f
to
141b737
Compare
0f958de
to
201f4ea
Compare
af7d149
to
13781ea
Compare
Codecov Report
@@ Coverage Diff @@
## dev #109 +/- ##
==========================================
- Coverage 98.76% 98.66% -0.10%
==========================================
Files 44 44
Lines 3882 3907 +25
==========================================
+ Hits 3834 3855 +21
- Misses 48 52 +4
Continue to review full report at Codecov.
|
@Adminiuga I'm trying to find a clean zigpy equivalent to the following ZNP-specific code that discovers the IEEE address of an unknown device: nwk = 0x1234 # The NWK address of an unknown device
ieee_addr_rsp = await self._znp.request_callback_rsp(
request=c.ZDO.IEEEAddrReq.Req(
NWK=nwk,
RequestType=c.zdo.AddrRequestType.SINGLE,
StartIndex=0,
),
RspStatus=t.Status.SUCCESS,
callback=c.ZDO.IEEEAddrRsp.Callback(partial=True, NWK=nwk),
timeout=5,
) Since we don't have a Similar code in Any thoughts on how to implement this in zigpy (if it even belongs there)? Maybe a module-level function like |
Once the required changes to zigpy are made, `_get_or_discover_device` can be shared across other libraries.
@puddly , to discovers the IEEE address of unknown device, don't you think that the best is to rely on the Neighbour table from all routers ? From my end this is what I came to a conclusion as this is the only way for end device (which are not listen on idle) |
Good idea, I did not think to use Once I figure out a way to make the device discovery code independent of zigpy-znp, I think this would be a good way to avoid sending |
Basically here is the way I'm doing. If I get an unknown shortId, then I look to the latest topology report that I have in house. If I'm not able to get an IEEE (basically the shortId is not found), then I trigger a new report and drop that message. This mean that the message is drop for now, but when I'll get an other one from the same device, I might be able to reconnect and get the device back known. |
@puddly, I did some test with the broadcast, and while the broadcast is going through, it looks like it is not a correct format This is the working paquet with the mainstream version (original)
This is the non-working paquet with your zdo-refactor version (this current PR)
|
I think this may be a bug with your code because the only change is that now zigpy-znp is sending exactly the bytes that you provide it. The below zigpy code: await zigpy.zdo.broadcast(
app,
zdo_t.ZDOCmd.Mgmt_NWK_Update_req,
0x0000, # group id (ignore)
0, # radius
zdo_t.NwkUpdate(
ScanChannels=t.Channels.from_channel_list([20]),
ScanDuration=zdo_t.NwkUpdate.CHANNEL_CHANGE_REQ,
nwkUpdateId=1,
),
broadcast_address=t.BroadcastAddress.ALL_ROUTERS_AND_COORDINATOR,
) Produces the following packet: AF.DataRequestExt.Req(
DstAddrModeAddress=AddrModeAddress(mode=<AddrMode.Broadcast: 15>, address=0xFFFC),
DstEndpoint=0,
DstPanId=0x0000,
SrcEndpoint=0,
ClusterId=56,
TSN=6,
Options=<TransmitOptions.NONE: 0>,
Radius=0,
Data=b'\x06\x00\x00\x10\x00\xFE\x01'
) I'm not sure where those two trailing |
Yeah, there's no clean way to do it. And currently zigpy relies on existing and initialized
I think this was one of the approaches tried in the past and yes it is hacky 🤷 So we can't store temporary device or devices with an unknown IEEE or NWK directly in zigpy (well, we could store devices with unknown NWK until NWK gets discovered, but that would may break or return an incorrect device if we Should we have a list in zigpy of "ephemeral" devices, for which we currently don't know IEEE or NWK and extend |
@puddly you are totally right, this was on my end. sorry for that. Will test today against your branch and will let you know, but I'm quiet optimistic |
@puddly I was trying to send the Channel update to the controller. My idea was when we do the broadcast most-likely the controller doesn't see.
|
This message is fine, as zigpy does not handle those requests. |
Z-Stack doesn't seem to process some ZDO commands when they're sent this way. It doesn't open joins when you send a @Adminiuga You think this is something that should be fixed by the radio library via zigpy/zigpy#855? Or should we just recommend using the radio API after sending the ZDO broadcast to all other devices on the network? await self.load_network_info()
await self.write_network_info(node_info=self.node_info, network_info=self.network_info.replace(channel=25)) Possibly with a helper method that can be re-implemented by radio libraries that take 30s to form a network from scratch (i.e. ZNP)? await self.update_network_info(network_info={"channel": 25}) |
No the controller didn't change its channel. The broadcast is done , but unfortunately the controller stay. I guess the answer from @puddly is that we must go through the native ZDO feature for each Controller |
When sending a broadcast out, do you get the broadcast loopback in? If not, send the broadcast, send another one unicast to coordinator and add a listener handler for nwk update request (zigpy does those now in pr zigpy#855) to translate that request into native stack api request |
yep, that what I did an the Unicast to coordinator is producing this error message No handler for ZDO
So I might need the #855 , but I'm also using the PR from @puddly, so will wait for you guys to merge |
It looks like it. Unfortunately, Z-Stack makes things interesting and also sends the same ZDO request/response indications in response to its own internal commands. This is a problem because creating a Do other coordinators respond to these ZDO commands properly? The Conbee worked with my energy scan test but EZSP did not. No clue about ZiGate or XBee. I suppose I can unsubscribe from all ZDO requests (i.e. any command below |
I meant for the
|
That's indeed my implementation. Here's a concrete example for # I send a Z-Stack internal request to permit joins (since raw ZDO doesn't do anything)
===> ZDO.MgmtPermitJoinReq.Req(...)
# Z-Stack confirms it
<=== ZDO.MgmtPermitJoinReq.Rsp(Status=<Status.SUCCESS: 0>)
# But also indicates that it received a ZDO `Mgmt_Permit_Joining_req`
<=== ZDO.MsgCbIncoming.Callback(Src=0x0000, IsBroadcast=<Bool.false: 0>, ClusterId=54, SecurityUse=0, TSN=0, MacDst=0x0000, Data=b'\x00\x01')
# Then sends back its internal response
<=== ZDO.MgmtPermitJoinRsp.Callback(Src=0x0000, Status=<Status.SUCCESS: 0>)
# And also that it sent back a ZDO `Mgmt_Permit_Joining_rsp`
<=== ZDO.MsgCbIncoming.Callback(Src=0x0000, IsBroadcast=<Bool.false: 0>, ClusterId=32822, SecurityUse=0, TSN=0, MacDst=0x0000, Data=b'\x00')
# Zigpy receives both, triggering the coordinator `Mgmt_Permit_Joining_req` handler again
[0x0000:zdo] ZDO request ZDOCmd.Mgmt_Permit_Joining_req: [0, <Bool.true: 1>]
[0x0000:zdo] ZDO request ZDOCmd.Mgmt_Permit_Joining_rsp: [<Status.SUCCESS: 0>] There's no way to distinguish someone calling The only real purpose of subscribing to ZDO requests is because Z-Stack forwards broadcasts as well. Maybe I can do that internally... |
Interesting. And it is not a broadcast. So when ZStack API is called directly it also creates a callback, like if the message was received directly over the air? |
I was mistaken about broadcasts, they don't get sent back to the coordinator. Since there's no real benefit to subscribing to ZDO request events, I can instead subscribe only to responses (to forward them to zigpy) and then trigger |
@puddly , I took your last commit and indeed I do confirm that it works. |
@puddly can it be that cluster 0x0013 is filtered and I'm not receiving it ? for instance when pairing a new device, I'm getting those logs, but never get the handle_message() with the 0x0013 Device Announcement message
|
You are right, I am filtering all clusters below 0x8000. I think there needs to be a list of exceptions. |
So either a list of exception, or a way to enable them during the startup for instance. So any user of zigpy-znp can indeed select what they want to filter and what they don't want. As I rely on it , I was not able to discover any new devices ;-) |
Thanks for the change, it works now. Just a question, why filtering some of the ZDO events ? Why not leaving to the upper level to handle them and eventually drop them ? |
If you have a use case that requires being notified of ZDO requests (Z-Stack will automatically answer 99% of them so you won't be able to reply), feel free to submit a PR later. For now I'd like to merge this change with the minimal feature set that keeps ZHA functional. |
For instance is the Node Descriptor Request not filtered ? |
Z-Stack sends its own response that's unfortunately hard-coded in the firmware. You'd be sending a duplicate response. These are just indications. Z-Stack automatically responds to all but a select few. In fact, the only one I'm aware of is |
Ok. so I agree with you feel free to merge your PR . I think we are fine as well |
Unhandled commands will be logged completely in the preceding line
Z-Stack has a
ZDO.MsgCallbackRegister
command that allows an application to register callbacks for specific ZDO command IDs. While Z-Stack will still send its normal, pre-parsed callbacks to zigpy-initiated ZDO requests, it will now also send a callback containing the raw ZDO response.This greatly reduces the amount of ZNP-specific code and will allow for some features to be moved into zigpy. For example:
This codebase is a WIP but I'm currently able to get new devices to join and all of the initialization-specific ZDO requests are handled properly by zigpy, with no intermediate translation done by zigpy-znp.