samples: nrf_cloud_multi_service: Wi-Fi scan config #12143

junqingzou · 2023-08-28T04:13:11Z

CONFIG_NRF700X_SCAN_LIMIT has been removed in PR #12008.
Replace with CONFIG_WIFI_MGMT_SCAN_MAX_BSS_CNT.

NordicBuilder · 2023-08-28T04:14:15Z

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-fw-nrfconnect-nrf-iot_cloud	X

Detailed information of selected test modules

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

glarsennordic · 2023-08-30T18:26:30Z

samples/cellular/nrf_cloud_multi_service/overlay_nrf7002ek_wifi_no_lte.conf

@@ -124,3 +126,6 @@ CONFIG_NET_SOCKETS_POLL_MAX=8
 # this could be set as low as 130000. Add 256 bytes for each additional scanning result, assuming
 # sane SSID lengths. We set the heap much higher in case of long SSIDs.
 CONFIG_HEAP_MEM_POOL_SIZE=130000
+
+## Configure device provision certificate
+CONFIG_NRF_CLOUD_PROVISION_CERTIFICATES=y


This is not required by CMakeLists.txt -- you mention it is needed for a test, can you elaborate on that? I think this should be added to the config of that test, rather than in the nrf7002 overlay

the directions in the MSS readme require additional parameters for the build command:

sdk-nrf/samples/cellular/nrf_cloud_multi_service/README.rst

Line 598 in e8babf4

west build --board your_board -p always -- -DCONFIG_NRF_CLOUD_PROVISION_CERTIFICATES=y -DCONFIG_NRF_CLOUD_CLIENT_ID_SRC_COMPILE_TIME=y -DCONFIG_NRF_CLOUD_CLIENT_ID="698d4c11-0ccc-4f04-89cd-6882724e3f6f"

so this need isn't really specific to zou's "test".

related: #11863 (comment)

Okay. As we discussed over there, requiring -DCONFIG_NRF_CLOUD_PROVISION_CERTIFICATES to be enabled manually was a deliberate decision, but this is not a hill I want to die on

i was just providing the history for Zou and also highlighting that this a general need, not test specific.

if we agree to keep this addition to the .conf, then the README should be updated, as that extra parameter is not needed.

Sorry I'm completely not aware of the doc.

west build --board your_board -p always -- -DCONFIG_NRF_CLOUD_PROVISION_CERTIFICATES=y -DCONFIG_NRF_CLOUD_CLIENT_ID_SRC_COMPILE_TIME=y -DCONFIG_NRF_CLOUD_CLIENT_ID="698d4c11-0ccc-4f04-89cd-6882724e3f6f"

In my test, I only added one -DOVERLAY_CONFIG=overlay_nrf7002ek_wifi_no_lte.conf

This is not required by CMakeLists.txt -- you mention it is needed for a test, can you elaborate on that? I think this should be added to the config of that test, rather than in the nrf7002 overlay

I mean this line:
# Include certs files if enabled. zephyr_include_directories_ifdef(CONFIG_NRF_CLOUD_PROVISION_CERTIFICATES certs)

As far as I know this is the only way to supply the credentials to lib_nrf_cloud for Wi-Fi devices as the TF-M PS storage is not ready yet.

glarsennordic · 2023-08-30T18:27:43Z

samples/cellular/nrf_cloud_multi_service/overlay_nrf7002ek_wifi_no_lte.conf

+CONFIG_NRF_CLOUD_CLIENT_ID_SRC_HW_ID=y
+CONFIG_HW_ID_LIBRARY=y
+CONFIG_HW_ID_LIBRARY_SOURCE_NET_MAC=y


Similarly, you could enable this in your test, rather than in the overlay. I would rather not prescribe a specific means of getting ID in this overlay, it's meant to be compatible with whatever the user configures themselves

Also we probably shouldn't encourage using the Wi-Fi MAC address as the nrf cloud client id?
But I do see why it is useful in this case, since all the modem stuff is disabled by the overlay, there is no way to get the nrf91 device UUID.

i didn't mean to approve. i think this should be removed?

samples/cellular/nrf_cloud_multi_service/src/cloud_connection.c

jayteemo · 2023-08-30T19:00:55Z

samples/cellular/nrf_cloud_multi_service/src/cloud_connection.c

+ LOG_ERR("nRF Cloud client_id is not initialized, error: %d", err);
+ return err;
+ } else {
+ LOG_INF("nRF Cloud client_id is %s", client_id);


Suggested change

LOG_INF("nRF Cloud client_id is %s", client_id);

LOG_INF("nRF Cloud client ID: %s", client_id);

samples/cellular/nrf_cloud_multi_service/src/cloud_connection.c

jayteemo · 2023-08-30T19:05:18Z

samples/cellular/nrf_cloud_multi_service/overlay_nrf7002ek_wifi_no_lte.conf

@@ -124,3 +126,6 @@ CONFIG_NET_SOCKETS_POLL_MAX=8
 # this could be set as low as 130000. Add 256 bytes for each additional scanning result, assuming
 # sane SSID lengths. We set the heap much higher in case of long SSIDs.
 CONFIG_HEAP_MEM_POOL_SIZE=130000
+
+## Configure device provision certificate
+CONFIG_NRF_CLOUD_PROVISION_CERTIFICATES=y


the directions in the MSS readme require additional parameters for the build command:

sdk-nrf/samples/cellular/nrf_cloud_multi_service/README.rst

Line 598 in e8babf4

west build --board your_board -p always -- -DCONFIG_NRF_CLOUD_PROVISION_CERTIFICATES=y -DCONFIG_NRF_CLOUD_CLIENT_ID_SRC_COMPILE_TIME=y -DCONFIG_NRF_CLOUD_CLIENT_ID="698d4c11-0ccc-4f04-89cd-6882724e3f6f"

so this need isn't really specific to zou's "test".

related: #11863 (comment)

jayteemo · 2023-08-30T19:09:49Z

samples/cellular/nrf_cloud_multi_service/overlay_nrf7002ek_wifi_no_lte.conf

+CONFIG_NRF_CLOUD_CLIENT_ID_SRC_HW_ID=y
+CONFIG_HW_ID_LIBRARY=y
+CONFIG_HW_ID_LIBRARY_SOURCE_NET_MAC=y


Also we probably shouldn't encourage using the Wi-Fi MAC address as the nrf cloud client id?
But I do see why it is useful in this case, since all the modem stuff is disabled by the overlay, there is no way to get the nrf91 device UUID.

jayteemo · 2023-08-30T19:25:19Z

samples/cellular/nrf_cloud_multi_service/overlay_nrf7002ek_wifi_no_lte.conf

+CONFIG_NRF_CLOUD_CLIENT_ID_SRC_HW_ID=y
+CONFIG_HW_ID_LIBRARY=y
+CONFIG_HW_ID_LIBRARY_SOURCE_NET_MAC=y


i didn't mean to approve. i think this should be removed?

junqingzou · 2023-08-30T23:53:22Z

Thanks for all the comments, I think that I'll remove the second commit as of now.

For the long term, if device credentials could be saved somewhere for lib_nrf_cloud to fetch, I think it makes sense to use Wi-Fi MAC address as nRF Cloud client_id, which makes one build of the sample to run on any devices.

Furthermore, this sample works greatly on nRF7002DK (nRF5340 + nRF7002) so maybe the document could be updated to reflect that.

CONFIG_NRF700X_SCAN_LIMIT has been removed in PR#12008. Replace with CONFIG_WIFI_MGMT_SCAN_MAX_BSS_CNT. Signed-off-by: Jun Qing Zou <jun.qing.zou@nordicsemi.no>

jayteemo · 2023-08-30T19:34:13Z

samples/cellular/nrf_cloud_multi_service/overlay_nrf7002ek_wifi_no_lte.conf

@@ -124,3 +126,6 @@ CONFIG_NET_SOCKETS_POLL_MAX=8
 # this could be set as low as 130000. Add 256 bytes for each additional scanning result, assuming
 # sane SSID lengths. We set the heap much higher in case of long SSIDs.
 CONFIG_HEAP_MEM_POOL_SIZE=130000
+
+## Configure device provision certificate
+CONFIG_NRF_CLOUD_PROVISION_CERTIFICATES=y


i was just providing the history for Zou and also highlighting that this a general need, not test specific.

if we agree to keep this addition to the .conf, then the README should be updated, as that extra parameter is not needed.

junqingzou requested review from plskeggs, jayteemo and glarsennordic as code owners August 28, 2023 04:13

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Aug 28, 2023

junqingzou requested a review from sachinthegreen August 28, 2023 04:13

jayteemo approved these changes Aug 28, 2023

View reviewed changes

plskeggs approved these changes Aug 28, 2023

View reviewed changes

junqingzou force-pushed the pr_wifi_scan_limit branch from e48ce96 to d5961e7 Compare August 29, 2023 02:30

junqingzou requested review from plskeggs and jayteemo August 30, 2023 09:03

sachinthegreen approved these changes Aug 30, 2023

View reviewed changes

glarsennordic requested changes Aug 30, 2023

View reviewed changes

jayteemo reviewed Aug 30, 2023

View reviewed changes

glarsennordic self-requested a review August 30, 2023 19:23

glarsennordic approved these changes Aug 30, 2023

View reviewed changes

jayteemo requested changes Aug 30, 2023

View reviewed changes

samples: nrf_cloud_multi_service: Wi-Fi scan config

90ad247

CONFIG_NRF700X_SCAN_LIMIT has been removed in PR#12008. Replace with CONFIG_WIFI_MGMT_SCAN_MAX_BSS_CNT. Signed-off-by: Jun Qing Zou <jun.qing.zou@nordicsemi.no>

junqingzou force-pushed the pr_wifi_scan_limit branch from 59d5ec2 to 90ad247 Compare August 31, 2023 00:01

junqingzou requested a review from jayteemo August 31, 2023 00:04

jayteemo approved these changes Aug 31, 2023

View reviewed changes

nordicjm merged commit 26e0fc9 into nrfconnect:main Aug 31, 2023
12 checks passed

junqingzou deleted the pr_wifi_scan_limit branch August 31, 2023 06:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

samples: nrf_cloud_multi_service: Wi-Fi scan config #12143

samples: nrf_cloud_multi_service: Wi-Fi scan config #12143

junqingzou commented Aug 28, 2023 •

edited

Loading

NordicBuilder commented Aug 28, 2023 •

edited

Loading

glarsennordic Aug 30, 2023

jayteemo Aug 30, 2023

glarsennordic Aug 30, 2023

jayteemo Aug 30, 2023

junqingzou Aug 30, 2023

junqingzou Aug 30, 2023

glarsennordic Aug 30, 2023

jayteemo Aug 30, 2023

jayteemo Aug 30, 2023

jayteemo Aug 30, 2023

jayteemo Aug 30, 2023

jayteemo Aug 30, 2023

jayteemo Aug 30, 2023

junqingzou commented Aug 30, 2023

jayteemo Aug 30, 2023

	LOG_INF("nRF Cloud client_id is %s", client_id);
	LOG_INF("nRF Cloud client ID: %s", client_id);

samples: nrf_cloud_multi_service: Wi-Fi scan config #12143

samples: nrf_cloud_multi_service: Wi-Fi scan config #12143

Conversation

junqingzou commented Aug 28, 2023 • edited Loading

NordicBuilder commented Aug 28, 2023 • edited Loading

Test specification

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junqingzou commented Aug 30, 2023

Choose a reason for hiding this comment

junqingzou commented Aug 28, 2023 •

edited

Loading

NordicBuilder commented Aug 28, 2023 •

edited

Loading