Merge pull request #7023 from dhalbert/wifi-scanning-fixes

update esp-idf; allow start/stop channels in wifi scanning

ports/espressif/common-hal/wifi/Radio.c

@@ -165,7 +165,7 @@ void common_hal_wifi_radio_set_mac_address_ap(wifi_radio_obj_t *self, const uint
     esp_wifi_set_mac(ESP_IF_WIFI_AP, mac);
 }
 
-mp_obj_t common_hal_wifi_radio_start_scanning_networks(wifi_radio_obj_t *self) {
+mp_obj_t common_hal_wifi_radio_start_scanning_networks(wifi_radio_obj_t *self, uint8_t start_channel, uint8_t stop_channel) {
     if (self->current_scan != NULL) {
         mp_raise_RuntimeError(translate("Already scanning for wifi networks"));
     }
@@ -177,9 +177,12 @@ mp_obj_t common_hal_wifi_radio_start_scanning_networks(wifi_radio_obj_t *self) {
     wifi_scannednetworks_obj_t *scan = m_new_obj(wifi_scannednetworks_obj_t);
     scan->base.type = &wifi_scannednetworks_type;
     self->current_scan = scan;
-    scan->start_channel = 1;
+    scan->current_channel_index = 0;
-    scan->end_channel = 11;
+    scan->start_channel = start_channel;
+    scan->end_channel = stop_channel;
     scan->radio_event_group = self->event_group_handle;
+    scan->done = false;
+    scan->channel_scan_in_progress = false;
     wifi_scannednetworks_scan_next_channel(scan);
     return scan;
 }

ports/espressif/common-hal/wifi/ScannedNetworks.c

@@ -72,7 +72,7 @@ mp_obj_t common_hal_wifi_scannednetworks_next(wifi_scannednetworks_obj_t *self)
         return mp_const_none;
     }
     // If we are scanning, wait and then load them.
-    if (self->scanning) {
+    if (self->channel_scan_in_progress) {
         // We may have to scan more than one channel to get a result.
         while (!self->done) {
             if (!wifi_scannednetworks_wait_for_scan(self)) {
@@ -81,7 +81,7 @@ mp_obj_t common_hal_wifi_scannednetworks_next(wifi_scannednetworks_obj_t *self)
             }
 
             esp_wifi_scan_get_ap_num(&self->total_results);
-            self->scanning = false;
+            self->channel_scan_in_progress = false;
             if (self->total_results > 0) {
                 break;
             }
@@ -112,7 +112,7 @@ mp_obj_t common_hal_wifi_scannednetworks_next(wifi_scannednetworks_obj_t *self)
             }
         }
         esp_wifi_scan_get_ap_records(&self->total_results, self->results);
-        self->scanning = false;
+        self->channel_scan_in_progress = false;
     }
 
     wifi_network_obj_t *entry = m_new_obj(wifi_network_obj_t);
@@ -132,40 +132,49 @@ mp_obj_t common_hal_wifi_scannednetworks_next(wifi_scannednetworks_obj_t *self)
 }
 
 // We don't do a linear scan so that we look at a variety of spectrum up front.
-static uint8_t scan_pattern[] = {6, 1, 11, 3, 9, 13, 2, 4, 8, 12, 5, 7, 10, 14};
+static uint8_t scan_pattern[] = {6, 1, 11, 3, 9, 13, 2, 4, 8, 12, 5, 7, 10, 14, 0};
 
 void wifi_scannednetworks_scan_next_channel(wifi_scannednetworks_obj_t *self) {
-    uint8_t next_channel = sizeof(scan_pattern);
+    // There is no channel 0, so use that as a flag to indicate we've run out of channels to scan.
+    uint8_t next_channel = 0;
     while (self->current_channel_index < sizeof(scan_pattern)) {
         next_channel = scan_pattern[self->current_channel_index];
         self->current_channel_index++;
+        // Scan only channels that are in the specified range.
         if (self->start_channel <= next_channel && next_channel <= self->end_channel) {
             break;
         }
     }
     wifi_scan_config_t config = { 0 };
     config.channel = next_channel;
-    if (next_channel == sizeof(scan_pattern)) {
+    if (next_channel == 0) {
         wifi_scannednetworks_done(self);
     } else {
         esp_err_t result = esp_wifi_scan_start(&config, false);
         if (result != ESP_OK) {
             wifi_scannednetworks_done(self);
         } else {
-            self->scanning = true;
+            self->channel_scan_in_progress = true;
         }
     }
 }
 
 void wifi_scannednetworks_deinit(wifi_scannednetworks_obj_t *self) {
     // if a scan is active, make sure and clean up the idf's buffer of results.
-    if (self->scanning) {
+    if (self->channel_scan_in_progress) {
         esp_wifi_scan_stop();
         if (wifi_scannednetworks_wait_for_scan(self)) {
-            // Ignore the number of records since we're throwing them away.
+            // Discard the scanned records, one at a time, to avoid memory leaks.
-            uint16_t number = 0;
+            uint16_t number;
-            esp_wifi_scan_get_ap_records(&number, NULL);
+            do {
-            self->scanning = false;
+                number = 1;
+                wifi_ap_record_t record;
+                esp_wifi_scan_get_ap_records(&number, &record);
+            } while (number > 0);
+            // TODO: available in ESP-IDF v5.0; do instead of the above.
+            // Discard scan results.
+            // esp_wifi_clear_ap_list();
+            self->channel_scan_in_progress = false;
         }
     }
     wifi_scannednetworks_done(self);

ports/espressif/common-hal/wifi/ScannedNetworks.h

@@ -53,7 +53,7 @@ typedef struct {
     uint8_t end_channel; // Inclusive
 
     bool done;
-    bool scanning;
+    bool channel_scan_in_progress;
 } wifi_scannednetworks_obj_t;
 
 void wifi_scannednetworks_scan_next_channel(wifi_scannednetworks_obj_t *self);

ports/espressif/common-hal/wifi/__init__.c

@@ -172,7 +172,7 @@ void common_hal_wifi_init(bool user_initiated) {
     // Even though we just called esp_netif_create_default_wifi_sta,
     //   station mode isn't actually ready for use until esp_wifi_set_mode()
     //   is called and the configuration is loaded via esp_wifi_set_config().
-    // Set both convienence flags to false so it's not forgotten.
+    // Set both convenience flags to false so it's not forgotten.
     self->sta_mode = 0;
     self->ap_mode = 0;
 

ports/espressif/esp-idf

@@ -1 +1 @@
-Subproject commit d51f7d882187afa4b39c2613fd0fe2ac2fea1145
+Subproject commit 716d8531d71b122975e2966a24ec7613b87eb7b0

ports/raspberrypi/common-hal/wifi/Radio.c

@@ -131,7 +131,8 @@ void common_hal_wifi_radio_set_mac_address_ap(wifi_radio_obj_t *self, const uint
     ro_attribute(MP_QSTR_mac_address_ap);
 }
 
-mp_obj_t common_hal_wifi_radio_start_scanning_networks(wifi_radio_obj_t *self) {
+mp_obj_t common_hal_wifi_radio_start_scanning_networks(wifi_radio_obj_t *self, uint8_t start_channel, uint8_t stop_channel) {
+    // channel bounds are ignored; not implemented in driver
     if (self->current_scan) {
         mp_raise_RuntimeError(translate("Already scanning for wifi networks"));
     }

shared-bindings/wifi/Radio.c

@@ -216,14 +216,38 @@ MP_PROPERTY_GETSET(wifi_radio_mac_address_ap_obj,
 //|     def start_scanning_networks(
 //|         self, *, start_channel: int = 1, stop_channel: int = 11
 //|     ) -> Iterable[Network]:
-//|         """Scans for available wifi networks over the given channel range. Make sure the channels are allowed in your country."""
+//|         """Scans for available wifi networks over the given channel range. Make sure the channels are allowed in your country.
+//|
+//|         .. note::
+//|
+//|             In the raspberrypi port (RP2040 CYW43), ``start_channel`` and ``stop_channel`` are ignored.
+//|         """
 //|         ...
-STATIC mp_obj_t wifi_radio_start_scanning_networks(mp_obj_t self_in) {
+STATIC mp_obj_t wifi_radio_start_scanning_networks(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
-    wifi_radio_obj_t *self = MP_OBJ_TO_PTR(self_in);
+    enum { ARG_start_channel, ARG_stop_channel };
+    static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_start_channel, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 1} },
+        { MP_QSTR_stop_channel, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 11} },
+    };
 
-    return common_hal_wifi_radio_start_scanning_networks(self);
+    wifi_radio_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+    mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
+    mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
+
+    uint8_t start_channel =
+        (uint8_t)mp_arg_validate_int_range(args[ARG_start_channel].u_int, 1, 14, MP_QSTR_start_channel);
+    uint8_t stop_channel =
+        (uint8_t)mp_arg_validate_int_range(args[ARG_stop_channel].u_int, 1, 14, MP_QSTR_stop_channel);
+    // Swap if in reverse order, without complaining.
+    if (start_channel > stop_channel) {
+        uint8_t temp = stop_channel;
+        stop_channel = start_channel;
+        start_channel = temp;
     }
-STATIC MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_start_scanning_networks_obj, wifi_radio_start_scanning_networks);
+
+    return common_hal_wifi_radio_start_scanning_networks(self, start_channel, stop_channel);
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_KW(wifi_radio_start_scanning_networks_obj, 1, wifi_radio_start_scanning_networks);
 
 //|     def stop_scanning_networks(self) -> None:
 //|         """Stop scanning for Wifi networks and free any resources used to do it."""

shared-bindings/wifi/Radio.h

@@ -85,7 +85,7 @@ extern void common_hal_wifi_radio_set_mac_address_ap(wifi_radio_obj_t *self, con
 extern mp_float_t common_hal_wifi_radio_get_tx_power(wifi_radio_obj_t *self);
 extern void common_hal_wifi_radio_set_tx_power(wifi_radio_obj_t *self, const mp_float_t power);
 
-extern mp_obj_t common_hal_wifi_radio_start_scanning_networks(wifi_radio_obj_t *self);
+extern mp_obj_t common_hal_wifi_radio_start_scanning_networks(wifi_radio_obj_t *self, uint8_t start_channel, uint8_t stop_channel);
 extern void common_hal_wifi_radio_stop_scanning_networks(wifi_radio_obj_t *self);
 
 extern void common_hal_wifi_radio_start_station(wifi_radio_obj_t *self);