-
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
Remove DeviceNetworkProvisioningDelegate interface to force vendor to use NetworkCommissioning interface… #23391
Remove DeviceNetworkProvisioningDelegate interface to force vendor to use NetworkCommissioning interface… #23391
Conversation
PR #23391: Size comparison from 09acc29 to 0fb9eb7 Increases (3 builds for bl602, bl702, k32w)
Decreases (2 builds for bl602, k32w)
Full report (11 builds for bl602, bl702, k32w, linux, qpg)
|
PR #23391: Size comparison from 09acc29 to 17e421f Increases (8 builds for bl602, bl702, cyw30739, esp32, k32w, psoc6)
Decreases (6 builds for esp32, psoc6, qpg, telink)
Full report (40 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, psoc6, qpg, telink)
|
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.
Is this class used at all? I thought I switched to using NetworkCommissioning drivers. The only usage of ProvisionWifi
I could find is some shell command and ProvisionThread
is not used at all. Shouldn't we rather remove the class?
Yes, this API is not used in any example app, but one of my internal product team has asked question about this, if we have alternative solution in NetworkCommissioning drivers, then we should remove this redundant APIs |
@Damian-Nordic I found the NetworkCommissioning drivers provides ScanNetworks/AddOrUpdateNetwork APIs which can do the similar stuff, but only ProvisionWiFi within DeviceNetworkProvisioningDelegate could trigger the ProvisionWiFiNetwork API in ConnectivityMgr directly. This is higher level API used to connect to an access point in one shot. We also have example in chip_shell demonstrate how to use this API. |
After second thought, if we provide two ways to do the similar thing, it will confuse the platform vendors, maybe you are right, by removing DeviceNetworkProvisioningDelegate API, we can force vendors to use NetworkCommissioning drivers to provision WiFi. |
PR #23391: Size comparison from f88f7bb to 8f14737 Increases (10 builds for cc13x2_26x2, k32w, nrfconnect, psoc6, qpg, telink)
Decreases (16 builds for cc13x2_26x2, esp32, linux, nrfconnect, psoc6, telink)
Full report (61 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
Yes, if the NetworkCommissioning API is suitable for other needs, too, I think it would be better to remove DeviceNetworkProvisioningDelegate. Mostly because it is currently not tested by any CI tests. |
… use NetworkCommissioning interface… (project-chip#23391) * Remove intermediate CRTP layer from DeviceNetworkProvisioningDelegate * Remove DeviceNetworkProvisioningDelegate interface
… use NetworkCommissioning interface… (project-chip#23391) * Remove intermediate CRTP layer from DeviceNetworkProvisioningDelegate * Remove DeviceNetworkProvisioningDelegate interface
Currently, we have two interfaces to provision WiFi, ScanNetworks/AddOrUpdateNetwork APIs in NetworkCommissioning drivers and ProvisionWiFi within DeviceNetworkProvisioningDelegate. DeviceNetworkProvisioningDelegate interfaces are not used except example shell command.
To prevent confusion, we should remove DeviceNetworkProvisioningDelegate interface
Solution:
Remove DeviceNetworkProvisioningDelegate interface to force vendor to use NetworkCommissioning interface.