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

net: wifi: Changes needed to run Zephyr net samples with Wi-Fi #11803

Merged
merged 8 commits into from
Aug 3, 2023

Conversation

rlubos
Copy link
Contributor

@rlubos rlubos commented Jul 13, 2023

This PR introduces a set of changes needed to run upstream Zephyr networking samples with nRF7002. For a list of samples, see nrfconnect/sdk-zephyr#1242 - samples with an overlay added have been successfully tested with nrf7002dk_nrf5340_cpuapp.

This PR comprises mostly of changes needed to automatically enable features essential for Wi-Fi operation when an appropriate board is selected and networking is enabled. The current feature auto-enablement is pretty aggressive so in most cases, the only change needed on the sample side is to configure SSID/password.

Originally, I was hoping we could rely on automatic configuration only, and avoid overlays in sdk-zephyr, but it soon turned out that in some cases an overlay config is unavoidable (for example when the sample overwritten the default POSIX_MAX_FDS). Since we'd need to carry downstream patch anyway because of this, I've decided to add an overlay to all of the required samples, to clearly indicate which samples have been tested and are supported. This means we can relax the current approach to automatically enable certain features a bit and move configuration to the overlay files. If you think something is too much (like for example enabling Connection Manager automatically is a bit of a gray area for me) please speak up, this PR is, among others, to gather feedback.

As for the remaining patches:

  • ae933961e5abd4e228fa9d72f31be69562a508ca - upstream networking samples rely on the NET_CONFIG library (and its initial boot delay) heavily, therefore we should not skip that part. I propose to revert that change, and set timeout to 0 in sdk-nrf samples only (not sure if it's really needed though)
  • b92a6745741f9d83c836ec60950ccee6ebdaaf8e - in case static SSID/password configuration is used, we don't really need a backend to store credentials, it only adds overall configuration complexity (you need to enable settings/nvs/flash etc. for each and every sample). Therefore I suggest to introduce a dummy variant of the backend, enabled automatically when static credential configuration is used.

@rlubos rlubos added the DNM label Jul 13, 2023
@github-actions github-actions bot added changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. manifest labels Jul 13, 2023
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jul 13, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
zephyr nrfconnect/sdk-zephyr@6041f17 nrfconnect/sdk-zephyr@4579f02 (main) nrfconnect/sdk-zephyr@6041f17c..4579f028

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@@ -71,6 +71,25 @@ config FLASH_LOAD_SIZE

endif # BOARD_NRF7002DK_NRF5340_CPUAPP_NS

if NETWORKING
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do this for nRF7000/nRF7001 also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krish2718 Now looking at this, I would add a similar defconfig for nR7001 DK/shield, but what about nRF7000 shield? WPA_SUPP is not needed for sure, but do NETWORKING/NET_L2_ETHERNET need to be enabled for scan only? Or is WIFI/WIFI_NRF700X enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WIFI_NRF700X and NET_L2_ETHERNET both are needed.

drivers/wifi/nrf700x/Kconfig.nrf700x Outdated Show resolved Hide resolved
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jul 13, 2023

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite
desktop52_verification X
test-fw-nrfconnect-ble X
test-fw-nrfconnect-ble_samples X
test-fw-nrfconnect-chip X
test-fw-nrfconnect-fem X
test-fw-nrfconnect-nfc X
test-fw-nrfconnect-nrf-iot_cloud X
test-fw-nrfconnect-nrf-iot_lwm2m X
test-fw-nrfconnect-nrf-iot_mosh X
test-fw-nrfconnect-nrf-iot_positioning X
test-fw-nrfconnect-nrf-iot_thingy91 X
test-fw-nrfconnect-nrf-iot_zephyr_lwm2m X
test-fw-nrfconnect-nrf_crypto X
test-fw-nrfconnect-rpc X
test-fw-nrfconnect-rs X
test-fw-nrfconnect-tfm X
test-fw-nrfconnect-thread X
test-sdk-find-my X
test-sdk-wifi X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch 4 times, most recently from cc42928 to aaa684e Compare July 13, 2023 19:20
@rlubos rlubos requested a review from a team as a code owner July 13, 2023 19:20
@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch 3 times, most recently from 2fa897f to 155d233 Compare July 20, 2023 08:56
@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch from d59dc5b to 0b55be6 Compare July 25, 2023 08:33
@rlubos
Copy link
Contributor Author

rlubos commented Jul 25, 2023

Thanks @richabp, I've applied your comments, and added "Configuration" section.

@richabp The doc page content/location is just my proposal, I'm fully open for even a complete redesign, whatever you suggest.

@rlubos Shouldn't it be in the Wi-Fi folder?

Did you mean samples/wifi here? I wasn't sure if we can put documentation there, as we'd be adding doc file in the sample folder structure w/o a corresponding sample, but if that's fine I can move it there. Unless you meant something else.

@rlubos As discussed offline, we keep it as is for now.

Copy link
Contributor

@richabp richabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes for consistency.

doc/nrf/samples/wifi_zephyr.rst Show resolved Hide resolved
doc/nrf/samples/wifi_zephyr.rst Show resolved Hide resolved
doc/nrf/samples/wifi_zephyr.rst Outdated Show resolved Hide resolved
doc/nrf/samples/wifi_zephyr.rst Outdated Show resolved Hide resolved
doc/nrf/samples/wifi_zephyr.rst Show resolved Hide resolved
@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch 3 times, most recently from f4d00bc to e295696 Compare July 25, 2023 14:01
@rlubos
Copy link
Contributor Author

rlubos commented Jul 25, 2023

Thanks @richabp, I've applied your suggestions

@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch from e295696 to dbbbccc Compare July 25, 2023 15:27
@github-actions github-actions bot removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jul 25, 2023
@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch from dbbbccc to c7f646b Compare July 25, 2023 15:30
@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch from c7f646b to 79bfe2e Compare July 26, 2023 13:08
@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch 2 times, most recently from 990b69e to e9f5150 Compare August 1, 2023 13:15
@rlubos
Copy link
Contributor Author

rlubos commented Aug 1, 2023

As the Zephyr part of the supplicant was moved to sdk-nrf, the PR towards sdk-hostap was no longer applicable. Instead, I've moved the two commits to this PR.

@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch 2 times, most recently from f4222be to b11ae86 Compare August 2, 2023 07:26
Using the default timeout value is beneficial in terms of support for
upstream sample, which often rely on network interface being up and
running before the sample code starts. Using the default timeout gives
the conn_mgr/supplicant a chance to connect to a Wi-Fi network before
the sample runs.

Therefore avoid modifying the default value of the timeout in the
driver. Instead, set the timeout explicitly to 0 for the NCS samples
that use the net config module.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Fine tune defaults for certain system components (heap/stacks/fd
entries) to better match driver requirements. This allows to run
upstream networking samples without modifying the configuration files.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
In case of simple samples, no Wi-Fi credentials storage backend is
really needed if static credentials configuration is used. Therefore add
an option to use no storage backend.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Enable Wi-Fi components essential for automatic connection establishment
by default, if Wi-Fi is enabled.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Pull changes needed to run Zephyr samples with Wi-Fi

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Add a documentation page which specifies which Zephyr networking samples
are currently supported in terms of nRF700x and how to build them for
that platform.

Add changelog entry that mentions adding nRF700x support for upstream
Zephyr networking samples.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Supplicant won't work w/o sockets, so just enable them, instead of
relying on dependency. This is more consistent with other modules like
NET_SOCKETS_PACKET which are selected by the supplicant.

It also seems that supplicant requires newlib to work, so enable it by
default.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Statically created threads with K_THREAD_DEFINE() are launched after the
SYS_INIT phase. This does not play well wth NET_CONFIG library, which
may block during SYS_INIT until network interface is UP and RUNNING.

In order to be able to connect to Wi-Fi network and thus mark the
network interface as running, we need to be able to run supplicant
during SYS_INIT. This can be achieved, by starting the thread
dynamically during SYS_INIT phase, instead of relying on static thread
creation.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
@rlubos rlubos force-pushed the upstream-net-samples-nrf7002 branch from b11ae86 to 0055cc9 Compare August 2, 2023 12:01
@NordicBuilder NordicBuilder removed the DNM label Aug 2, 2023
@rlubos rlubos merged commit d02e160 into nrfconnect:main Aug 3, 2023
29 of 32 checks passed
@rlubos rlubos deleted the upstream-net-samples-nrf7002 branch August 31, 2023 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-required PR must not be merged without tech writer approval. manifest manifest-hostap manifest-zephyr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants