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: shell: only process scan events during requested scan #84546

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EricNRS
Copy link
Contributor

@EricNRS EricNRS commented Jan 25, 2025

The scan events are always enabled which means if another software component requests a scan, then the WiFi shell scan printouts will also be sent to the shell. This change only enables the network management scan events when a user has requested a scan.

Without the change when my application does a scan to find the best channel:

uart> config wifi_channel
Scanning for best channels
SSID IoT, Channel 11, RSSI -46 dBm
SSID IoT, Channel 11, RSSI -63 dBm
SSID IoT, Channel 6, RSSI -77 dBm
SSID IoT, Channel 1, RSSI -81 dBm
Scan completed
Best Channel: 11, Max RSSI: -46 dBm

# These are printed from the WiFi Shell even though it wasn't requested:
Num  | SSID                             (len) | Chan (Band)   | RSSI | Security        | BSSID             | MFP     
1    | IoT                              3     | 11   (2.4GHz) | -46  | WPA2-PSK        | -------------------- | Disable 
2    | IoT                              3     | 11   (2.4GHz) | -63  | WPA2-PSK        | --------------------  | Disable 
3    | IoT                              3     | 6    (2.4GHz) | -77  | WPA2-PSK        | --------------------  | Disable 
4    | IoT                              3     | 1    (2.4GHz) | -81  | WPA2-PSK        | --------------------  | Disable 
Scan request done

The scan events are always enabled which means if another software
component requests a scan, then the WiFi shell scan printouts will
also be sent to the shell.

Only enable the network management scan events when a user has
requested a scan.

Signed-off-by: Eric Holmberg <eric.holmberg@northriversystems.co.nz>
@EricNRS
Copy link
Contributor Author

EricNRS commented Jan 25, 2025

The change is ready to merge, but marking as a draft since I have not discussed it with anyone yet.

@MaochenWang1
Copy link
Collaborator

In you case, how your software component triggers scan? Call the net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params))?
The wifi shell print log contains more detailed information than your component log, why not just use the wifi shell log directly?

@EricNRS
Copy link
Contributor Author

EricNRS commented Jan 27, 2025

In you case, how your software component triggers scan? Call the net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params))? The wifi shell print log contains more detailed information than your component log, why not just use the wifi shell log directly?

Correct, I use net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params)) to trigger the scan every 30 seconds which is then sorted to determine the best channel and perform a reconnect operation. This is all done automatically in software, so the logging output is just a nuisance. In addition, logging is kept to a minimum due to OTA costs.

@krish2718
Copy link
Collaborator

In you case, how your software component triggers scan? Call the net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params))? The wifi shell print log contains more detailed information than your component log, why not just use the wifi shell log directly?

Correct, I use net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params)) to trigger the scan every 30 seconds which is then sorted to determine the best channel and perform a reconnect operation. This is all done automatically in software, so the logging output is just a nuisance. In addition, logging is kept to a minimum due to OTA costs.

The change itself it fine, but can't you just disable Wi-Fi shell CONFIG_NET_L2_WIFI_SHELL=n? Shell is only for development.

@EricNRS
Copy link
Contributor Author

EricNRS commented Jan 27, 2025

In you case, how your software component triggers scan? Call the net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params))? The wifi shell print log contains more detailed information than your component log, why not just use the wifi shell log directly?

Correct, I use net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params)) to trigger the scan every 30 seconds which is then sorted to determine the best channel and perform a reconnect operation. This is all done automatically in software, so the logging output is just a nuisance. In addition, logging is kept to a minimum due to OTA costs.

The change itself it fine, but can't you just disable Wi-Fi shell CONFIG_NET_L2_WIFI_SHELL=n? Shell is only for development.

Yes, I am in field trials at the moment, so just wanted to keep some of the shell commands such as wifi connect and wifi disconnect, but I can reimplement them and disable the WiFi shell entirely. If nobody else sees any value in this change, then I am happy to abandon it.

@krish2718
Copy link
Collaborator

In you case, how your software component triggers scan? Call the net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params))? The wifi shell print log contains more detailed information than your component log, why not just use the wifi shell log directly?

Correct, I use net_mgmt(NET_REQUEST_WIFI_SCAN, iface, &params, sizeof(params)) to trigger the scan every 30 seconds which is then sorted to determine the best channel and perform a reconnect operation. This is all done automatically in software, so the logging output is just a nuisance. In addition, logging is kept to a minimum due to OTA costs.

The change itself it fine, but can't you just disable Wi-Fi shell CONFIG_NET_L2_WIFI_SHELL=n? Shell is only for development.

Yes, I am in field trials at the moment, so just wanted to keep some of the shell commands such as wifi connect and wifi disconnect, but I can reimplement them and disable the WiFi shell entirely. If nobody else sees any value in this change, then I am happy to abandon it.

The change itself LGTM, only impact to shell is that (re)adding callback for every scan, but IMHO that should be fine.

@EricNRS EricNRS marked this pull request as ready for review January 27, 2025 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants