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.

Sign up for GitHub

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

Jump to bottom

Sorting of scanned wifi networks #661

Open

tyeth wants to merge 6 commits into adafruit:main from tyeth:wireless-rssi-sorting

+241 −36

Contributor

tyeth commented Nov 13, 2024 •

edited

Loading

Addresses #659

tyeth force-pushed the wireless-rssi-sorting branch from 7d2aebe to f234c27 Compare

November 13, 2024 18:35

Contributor Author

tyeth commented Nov 14, 2024 •

edited

Loading

Works on Pico, which is what I was after as it was affecting my testing of other things.

No known network sorted response details

---- Reopened serial port COM40 ----
Found single wifi network in secrets.json
Adafruit.io WipperSnapper
-------Device Information-------
Firmware Version: 1.0.0-beta.92
Board ID: rpi-pico-w
Adafruit.io User: tyeth
WiFi Network: free4all_2G
MAC Address: 28:CD:C1:0E:17:E8
-------------------------------
Generating device's MQTT topics...
Running Network FSM...
Establishing network connection...
Performing a WiFi scan for SSID...ERROR: Your requested WiFi network was not found!
WipperSnapper found these WiFi networks: 
free4all -50dB
free4all -52dB
free4all -55dB
Tenda_60C06C -79dB
free4all -82dB
ERROR [WDT RESET]: ERROR: Unable to find WiFi network, rebooting soon...
ERROR [WDT RESET]: ERROR: Unable to find WiFi network, rebooting soon...

And success (failing on connect attempt #0, but succeeding on # 1 aka 2nd attempt):

Details

---- Opened the serial port COM40 ----
Connecting to WiFi (attempt #1)
Connected to WiFi!
Connecting to AIO MQTT (attempt #0)
WiFi Status: 20
Registering hardware with WipperSnapper...
Registering hardware with IO...
Encoding registration request...Encoding registration msg...Published!
Polling for registration message response...2
GOT Registration Response Message:
Hardware Response Msg:
        GPIO Pins: 31
        Analog Pins: 4
        Reference voltage: 3.30v
Completed registration process, configuration next!
Polling for message containing hardware configuration...
Polling for message containing hardware configuration...
Polling for message containing hardware configuration...
cbSignalTopic: New Msg on Signal Topic
2 bytes.
decodeSignalMsg
cbSignalMsg
Sub-messages found: 1
Signal Msg Tag: Pin Configuration
Initial Pin Configuration Complete!
Publishing to pin config complete...
Hardware configured successfully!
Registration and configuration complete!
Running application...
Sending MQTT PING: SUCCESS!
WiFi RSSI: -36
Sending MQTT PING: SUCCESS!
WiFi RSSI: -44
Sending MQTT PING: SUCCESS!
WiFi RSSI: -53
Sending MQTT PING: SUCCESS!
WiFi RSSI: -43
Sending MQTT PING: SUCCESS!

tyeth marked this pull request as ready for review

November 14, 2024 20:51

tyeth changed the title ~~DRAFT CI Assets - Initial sorting of scanned wifi networks~~ Sorting of scanned wifi networks

tyeth requested a review from brentru

November 15, 2024 15:16

Contributor Author

tyeth commented Nov 15, 2024 •

edited

Loading

@brentru this is ready for review. I should retest on the other platforms, especially samd.

brentru requested changes

View reviewed changes

Member

brentru left a comment

@tyeth I have a few review notes below and also some questions:

Do we take the network with the highest RSSI?
If we fail to connect to the defined network, do we try again with the next network in our list of networks?
What is the purpose of sorting by RSSI if we are not going to take the network with the largest RSSI or if its not defined?

src/network_interfaces/Wippersnapper_AIRLIFT.h Outdated

+                /****************************************************************/
+                struct WiFiNetwork {
+                  char ssid[33]; /*!< SSID (Max 32 characters + null terminator */
+                  int rssi;      /*!< Received Signal Strength Indicator */

Member

brentru Nov 15, 2024

RSSI should be type int32_t to match the WiFi library

src/network_interfaces/Wippersnapper_AIRLIFT.h Outdated

@@ @@ -119,22 +145,38 @@ class Wippersnapper_AIRLIFT : public Wippersnapper { @@
                     return false;
                   }
-                  // Was the network within secrets.json found?
+                  // Dynamically allocate memory for the network list
+                  std::vector<WiFiNetwork> networks(n);

Member

brentru Nov 15, 2024

I am not sure about using a vector/dynamic alloc. here, allowing too many possible networks will increase sort time.

Instead, could you define a global MAX_WIFI_NETWORK macro of 15 possible network options?

src/network_interfaces/Wippersnapper_AIRLIFT.h Outdated

+                */
+                /****************************************************************/
+                struct WiFiNetwork {
+                  char ssid[33]; /*!< SSID (Max 32 characters + null terminator */

Member

brentru Nov 15, 2024

Looks like SSID is a string in the WiFi library, so we should match the type
https://github.com/espressif/arduino-esp32/blob/master/libraries/WiFi/src/WiFiScan.h#L48

src/network_interfaces/Wippersnapper_AIRLIFT.h Outdated

-                      WS_DEBUG_PRINT("SSID found! RSSI: ");
-                      WS_DEBUG_PRINTLN(WiFi.RSSI(i));
+                    strncpy(networks[i].ssid, WiFi.SSID(i), sizeof(networks[i].ssid) - 1);
+                    networks[i].ssid[sizeof(networks[i].ssid) - 1] =

Member

brentru Nov 15, 2024

If you switch SSID to a String type, it will be null-terminated during compilation.

src/network_interfaces/Wippersnapper_AIRLIFT.h

@@ @@ -26,6 +26,8 @@ @@
               #include "SPI.h"
               #include "WiFiNINA.h"
               #include "Wippersnapper.h"
+              #include <algorithm>

Member

brentru Nov 15, 2024

Do we want this here, or moved to be included by WipperSnapper.h or another file (networking.h?) so it's included by all the drivers?

src/network_interfaces/Wippersnapper_AIRLIFT.h Outdated

+                  std::sort(networks.begin(), networks.end(), compareByRSSI);
+                  // Was the network within secrets.json found?
+                  for (const auto &network : networks) {

Member

brentru Nov 15, 2024

Why are we using autotype assignment here? Instead of this overhead, we could iterate until the end of the array

tyeth added 5 commits

November 21, 2024 18:39


          Update network scan to sort by RSSI

0e77e3c


          correct ssid access for some platforms

c6216b1


          clang format "sort networks by RSSI"

cdb96c7


          Doxygen comments for compareByRSSI

bec682e


          Document wifi network struct for sorting

a6b68cd

tyeth force-pushed the wireless-rssi-sorting branch from 806690f to a6b68cd Compare

November 21, 2024 18:39

Contributor Author

tyeth commented Nov 21, 2024 •

edited

Loading

@tyeth I have a few review notes below and also some questions:

Do we take the network with the highest RSSI?

If we fail to connect to the defined network, do we try again with the next network in our list of networks?

What is the purpose of sorting by RSSI if we are not going to take the network with the largest RSSI or if its not defined?

Yep, the least negative, or closest to 0, being the strongest / highest RSSI. (Unless you mean more negative highest)
-Edit- It appears on wikipedia that different hardware may return positive RSSI values, so we may need to switch the sort in those rare cases.
I believe we only make an attempt to the strongest known network then do a reboot on third connect failure.
It would be a nice extension to continue connecting to the other known networks if present. Don't think we'd need to keep a list of BSSIDs matching the SSIDs, to know which network we've failed on, instead we just accept if the default network fails and move on to the first alternative network entry, repeating until no remaining networks to try, then reboot. I guess we keep a position in the list, and the initial connect and reconnect logic goes through one by one.
I believe the RSSI cannot be undefined, and we do take the first (closest to zero) network with the smallest/largest/strongest RSSI. I tested that we do and it correctly picks between 3 meshes when running the new code (it prints the RSSI on ping which is helpful).

tyeth force-pushed the wireless-rssi-sorting branch from 7b75b42 to ee9b0ba Compare

November 21, 2024 19:52


          Swap dynamic memory for static in wifi rssi sorting

a1ea262

tyeth force-pushed the wireless-rssi-sorting branch from ee9b0ba to a1ea262 Compare

November 21, 2024 19:58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet