Skip to content
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

Implement new zigpy network state #445

Merged
merged 52 commits into from
Jun 27, 2022
Merged

Conversation

puddly
Copy link
Contributor

@puddly puddly commented Dec 12, 2021

zigpy/zigpy#848

At the moment this PR is very WIP and I'm not very familiar with EZSP so there are likely mistakes with the hashed TCLK stuff. No unit tests pass and I've only tested it with zigpy-cli, though it does generate a valid backup for me.

Test coverage is 100% 🎉

@SylvainPer
Copy link
Contributor

@puddly in the __init __() of ControllerApplication could you call SCHEMA(config) in the super().__init __() like other libs ?

@puddly
Copy link
Contributor Author

puddly commented Mar 6, 2022

This has been fixed inside of zigpy itself: puddly/zigpy@4c78afc

@SylvainPer
Copy link
Contributor

@puddly I tried the new load_network_info function and I don't know what to think about the result:

  • all getChildData requests answer are in EMBER_NOT_JOINED state
  • all getAddressTableRemoteNodeId return 'ffff'
  • all getAddressTableRemoteEui64 return 'ffffffffffffffff' for the first one and then '0000000000000000'

do you have the same with your controller ?

@puddly
Copy link
Contributor Author

puddly commented Mar 8, 2022

That mostly matches what I see with my coordinator but since I don't normally use a SiLabs coordinator, I'm not sure if it's "normal" or not. None of that information is critical, it was just included to make the ZNP backup complete.

@SylvainPer
Copy link
Contributor

SylvainPer commented Mar 9, 2022

It's not critical but it's something we use in the initialization part to check if the couple IEEE/shortId is still up-to-date.

Maybe the networkInit() reset those values, I saw that you check the network state before calling it, I'll give a try.
Edit: Misread your modification

@pipiche38
Copy link
Contributor

@puddly

That mostly matches what I see with my coordinator but since I don't normally use a SiLabs coordinator, I'm not sure if it's "normal" or not. None of that information is critical, it was just included to make the ZNP backup complete.

It is not critical, but it is quiet important. The main reason is that if you have your coordinator working, but you have your application/box/plugin off ( maintenance, upgrade ....), when restarting the plugin it is a really great things to be able to load Ieee/Nwkid and check against the last plugin state and eventually update it. In zigpy I believe device database is updated accordingly.
Lacking this part, will make receiving zigbee messages from unknown device (if that one has changed during the time the plugin/library was off), and will required some effort to retreive the IEEE/NwkId.

@SylvainPer
Copy link
Contributor

Just made some test live:

  • even after driving a plug, nothing change in the tables
  • after a pairing, the device appears in getAddressTableRemoteNodeId and getAddressTableRemoteEui64 (index 16), nothing in getChildData
  • after a restart (startup), the device disappeared from the tables

@SylvainPer
Copy link
Contributor

@puddly could you take #452 into your radio-api branch please ?

@puddly puddly added this to In progress in New radio API Mar 23, 2022
@puddly puddly moved this from In progress to Done in New radio API Mar 23, 2022
puddly and others added 15 commits May 5, 2022 13:28
When a RSTACK message is processed right after the UART has been opened,
it causes EZSP.enter_failed_state() getting called at a point where the
application callbacks are not registered yet. In that case the UART
will get closed and it won't get opened again. Bellows is stuck with a
closed transport.

Avoid this issue by not closing the port in case there is no application
callback registered yet.

Typically, it is unlikely that a RSTACK message arrives right when
the port gets opened (the race window is very narrow). However, with
hardware flow control opening the port leads to RTS signal to get
asserted which causes the radio to send pending messages, e.g. resets
caused by EmberZNet watchdog.

Note: With hardware flow control this is only the case if the tty "hupcl"
option is set. The option is set by default, but cleared by tools like
GNU screen. This option makes sure that the RTS signal is deasserted
while the port is closed. Pyserial/bellows does not change the state
of that option.
@puddly puddly marked this pull request as ready for review June 16, 2022 18:53
@puddly puddly requested a review from Adminiuga June 19, 2022 21:48
@pipiche38
Copy link
Contributor

@puddly , I'm trying to test the bellows new-radio-API ,and I'm current getting issue when importing.

I'm using your branch puddly:puddly/zigpy-radio-api

Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:     from Classes.ZigpyTransport.AppBellows import App_bellows
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:   File "/home/domoticz/domoticz/plugins/Domoticz-Zigbee/Classes/ZigpyTransport/AppBellows.py", line 15, in <module>
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:     import bellows.ezsp as ezsp
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:   File "/home/domoticz/domoticz/plugins/Domoticz-Zigbee/bellows/ezsp/__init__.py", line 21, in <module>
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:     import bellows.types as t
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:   File "/home/domoticz/domoticz/plugins/Domoticz-Zigbee/bellows/types/__init__.py", line 3, in <module>
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:     from .struct import *  # noqa: F401,F403
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:   File "/home/domoticz/domoticz/plugins/Domoticz-Zigbee/bellows/types/struct.py", line 258, in <module>
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:     class EmberNetworkParameters(EzspStruct):
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:   File "/home/domoticz/domoticz/plugins/Domoticz-Zigbee/bellows/types/struct.py", line 286, in EmberNetworkParameters
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab:     def zigpy_network_information(self) -> app_state.NetworkInformation:
Jun 20 21:09:20 pi4 domoticz[674]: zigpy-Silicon Lab: AttributeError: module 'zigpy.state' has no attribute 'NetworkInformation'

@puddly
Copy link
Contributor Author

puddly commented Jun 20, 2022

I don't think you're using the correct branch. NetworkInformation does not appear in the source code.

@zigbeefordomoticz
Copy link

zigbeefordomoticz commented Jun 20, 2022 via email

@zigbeefordomoticz
Copy link

would it be possible to get this PR approved and merged ? We are waiting for it in order to integrate the full new-radio-API ( zigpy , zigpy-znp, zigpy-deconz ) have already been merged

thanks in advance and thanks to @puddly for making it, this really make the usage across radio lib simpler

@pipiche38
Copy link
Contributor

gentlemen @Hedda, @puddly , @Adminiuga any chance to get this approve and merged ? Thanks in advance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants