-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Get OnNetwork commissioning working #34978
Get OnNetwork commissioning working #34978
Conversation
Review changes with SemanticDiff. |
PR #34978: Size comparison from 0ea43c9 to 2538710 Full report (3 builds for cc32xx, stm32)
|
PR #34978: Size comparison from 0ea43c9 to 831ef5a Full report (78 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, tizen)
|
PR #34978: Size comparison from b823a29 to 97e103b Full report (71 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, tizen)
|
PR #34978: Size comparison from 9a90d8b to 29417c8 Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34978: Size comparison from ff6863e to 3156be2 Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34978: Size comparison from 8b97b28 to 81dfa8b Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Hey ! I don't know why Darwin / Run framework tests (tsan, -enableThreadSanitizer YES) (pull_request) always failed ? @Damian-Nordic @andy31415 any clue ? Edit : Seems to be good |
PR #34978: Size comparison from 0851382 to 49590a8 Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34978: Size comparison from a296b66 to d9de09a Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
PR #34978: Size comparison from c305e3f to c336d2f Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
@Damian-Nordic @andy31415 Is there someone who can review this PR ? My internship finish in a week and half, thanks ! |
Hi @Damian-Nordic ! Is there something else missing or everything is good ? |
|
@Damian-Nordic Okay thanks ! I asked because it isn't merge actually |
PR #34978: Size comparison from 9bbf5b9 to 86134c7 Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't merged because PRs need two reviews from different companies to merge, and because it had a random CI failure. CI job retriggered.
Rubber-stamping; the changes I understand look reasonable, and I trust @Damian-Nordic checked the other bits carefully.
Hey @bzbarsky-apple, sorry to rush you a bit with this PR. I understand that revisions can take time, and there's also the vacations, but my internship ends in a week (and my account with it), so that's why I was in a bit of a hurry! Thank you for your understanding. |
PR #34978: Size comparison from 0e3434a to 25e14f7 Full report (88 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
* Update Zephyr platform to get working commissioning for onnetwork purposes * Restyled by whitespace * Remove unwanted and unnecessary comments Signed-off-by: Caldeira, Quentin <QuentinCaldeira@eaton.com> * Bring Openthread default config back * Bring back udp and tcp configurations --------- Signed-off-by: Caldeira, Quentin <QuentinCaldeira@eaton.com> Co-authored-by: Restyled.io <commits@restyled.io>
As discussed in many others issues, Zephyr platform doesn't handle well the OnNetwork Commissioning. This PR add some missing things like the Kconfig MBEDTLS_X509, and the patch for Multicast Join made by nRF (I guess). It seems that Zephyr platform doesn't required an Ethernet Driver, but it should be nice to have one which perform DHCP or static ipv6 address.
Now, the OnNetwork commissioning is working. I've a full application working with ethernet only. I don't have a Thread or WiFi board to develop it more. Should be nice to have a real Zephyr SDK based application.
The application and this PR were tested on a Nucleo h563zi, with the zephyr SDK, and Home Assistant on a Raspberry as a controller. No issues so far. Factory Reset is working, Zephyr handle the Ethernet disconnection, and you can power off, then power on the device with no connection lost.
I could provide the application in another PR if you want. I think we maybe should have a work on include specific Kconfig only when Ethernet or wifi is selected, to reduce the application size.
Also, I put OpenThread to false by default, because like Bluetooth the user shouldn't have to expect that theses components are enables, as this is not the only way to communicate in Matter. If it's a problem, I can put it back on