Rewrite CYW43 WiFi connection scheme (#3160)

Fixes #3157
Fixes #1837

The SDK's CYW43 WiFi connection scheme combined both link layer (WiFi net
connection) and IP (getting an IP address from DHCP) into a single step.
We worked around it with a hack wrapped function, but it resulted in
the WiFi.begin() timeout being 2x as long as specified.

Remove the SDK's cyw43_arch_wifi_connect_timeout_ms and replace with
our own custom CYW43::begin which will only look at the link layer.
Note that the SDK docs are hokey and the cyw43_link_status is set to 1
*while network connection attempts are underway.*  This means we can't
actually just look at the link state, it will be in JOIN even if it's
not really joined.  So, we peek at the state internal CYW43 join FSM
to see we're *really* connected.

This exposed a corner case which @ledereyes seems to have run into in
his own app where the LwipIntfDev could potentially try and register
the same netif twice because the underlying lower-level CYW43 link
had failed.  Ensure we netif_remove() in that case to avoid this issue.

On WiFi.end(), just disconnect the network and don't deinitialize the
chip in case Bluetooth is active.

Author: earlephilhower

cores/rp2040/freertos/freertos-lwip.cpp

@@ -381,10 +381,19 @@ static void lwipThread(void *params) {
             }
             case __ethernet_input: {
                 __ethernet_input_req *r = (__ethernet_input_req *)w.req;
-                printf("__real_ethernet_input\n");
                 *(r->ret) = __real_ethernet_input(r->p, r->netif);
                 break;
             }
+            case __cyw43_wifi_join: {
+                __cyw43_wifi_join_req *r = (__cyw43_wifi_join_req *)w.req;
+                *(r->ret) = __real_cyw43_wifi_join(r->self, r->ssid_len, r->ssid, r->key_len, r->key, r->auth_type, r->bssid, r->channel);
+                break;
+            }
+            case __cyw43_wifi_leave: {
+                __cyw43_wifi_leave_req *r = (__cyw43_wifi_leave_req*)w.req;
+                *(r->ret) = __real_cyw43_wifi_leave(r->self, r->itf);
+                break;
+            }
             case __callback: {
                 __callback_req *r = (__callback_req *)w.req;
                 r->cb(r->cbData);

cores/rp2040/lwip_wrap.cpp

@@ -832,6 +832,34 @@ extern "C" {
         return __real_ethernet_input(p, netif);
     }
 
+    int __real_cyw43_wifi_join(cyw43_t *self, size_t ssid_len, const uint8_t *ssid, size_t key_len, const uint8_t *key, uint32_t auth_type, const uint8_t *bssid, uint32_t channel);
+    int __wrap_cyw43_wifi_join(cyw43_t *self, size_t ssid_len, const uint8_t *ssid, size_t key_len, const uint8_t *key, uint32_t auth_type, const uint8_t *bssid, uint32_t channel) {
+#ifdef __FREERTOS
+        if (!__isLWIPThread()) {
+            int ret;
+            __cyw43_wifi_join_req req = { self, ssid_len, ssid, key_len, key, auth_type, bssid, channel, &ret };
+            __lwip(__cyw43_wifi_join, &req);
+            return ret;
+        }
+#endif
+        return __real_cyw43_wifi_join(self, ssid_len, ssid, key_len, key, auth_type, bssid, channel);
+    }
+
+    int __real_cyw43_wifi_leave(cyw43_t* self, int itf);
+    int __wrap_cyw43_wifi_leave(cyw43_t* self, int itf) {
+#ifdef __FREERTOS
+        if (!__isLWIPThread()) {
+            int ret;
+            __cyw43_wifi_leave_req req = { self, itf, &ret };
+            __lwip(__cyw43_wifi_leave, &req);
+            return ret;
+        }
+#endif
+        return __real_cyw43_wifi_leave(self, itf);
+    }
+
+
+
     void lwip_callback(void (*cb)(void *), void *cbData, __callback_req *buffer) {
 #ifdef __FREERTOS
         if (buffer) {
@@ -895,14 +923,6 @@ extern "C" {
     void __wrap_cyw43_schedule_internal_poll_dispatch(void (*func)()) {
         __real_cyw43_schedule_internal_poll_dispatch(func);
     }
-    extern int __real_cyw43_arch_wifi_connect_bssid_timeout_ms(const char *ssid, const uint8_t *bssid, const char *pw, uint32_t auth, uint32_t timeout_ms);
-    int __wrap_cyw43_arch_wifi_connect_bssid_timeout_ms(const char *ssid, const uint8_t *bssid, const char *pw, uint32_t auth, uint32_t timeout_ms) {
-        return __real_cyw43_arch_wifi_connect_bssid_timeout_ms(ssid, bssid, pw, auth, timeout_ms);
-    }
-    extern int __real_cyw43_arch_wifi_connect_timeout_ms(const char *ssid, const char *pw, uint32_t auth, uint32_t timeout_ms);
-    int __wrap_cyw43_arch_wifi_connect_timeout_ms(const char *ssid, const char *pw, uint32_t auth, uint32_t timeout_ms) {
-        return __real_cyw43_arch_wifi_connect_timeout_ms(ssid, pw, auth, timeout_ms);
-    }
     extern void __real_cyw43_arch_gpio_put(uint wl_gpio, bool value);
     void __wrap_cyw43_arch_gpio_put(uint wl_gpio, bool value) {
         __real_cyw43_arch_gpio_put(wl_gpio, value);

cores/rp2040/lwip_wrap.h

@@ -167,6 +167,9 @@ typedef enum {
 
     __ethernet_input = 9000,
 
+    __cyw43_wifi_join = 9500,
+    __cyw43_wifi_leave,
+
     __callback = 10000,
 } __lwip_op;
 
@@ -230,6 +233,8 @@ extern void __real_raw_remove(struct raw_pcb *pcb);
 extern struct netif *__real_netif_add(struct netif *netif, const ip4_addr_t *ipaddr, const ip4_addr_t *netmask, const ip4_addr_t *gw, void *state, netif_init_fn init, netif_input_fn input);
 extern void __real_netif_remove(struct netif *netif);
 extern err_t __real_ethernet_input(struct pbuf *p, struct netif *netif);
+extern int __real_cyw43_wifi_join(cyw43_t *self, size_t ssid_len, const uint8_t *ssid, size_t key_len, const uint8_t *key, uint32_t auth_type, const uint8_t *bssid, uint32_t channel);
+extern int __real_cyw43_wifi_leave(cyw43_t *self, int itf);
 
 
 typedef struct {
@@ -568,6 +573,24 @@ typedef struct {
     err_t *ret;
 } __ethernet_input_req;
 
+typedef struct {
+    cyw43_t *self;
+    size_t ssid_len;
+    const uint8_t *ssid;
+    size_t key_len;
+    const uint8_t *key;
+    uint32_t auth_type;
+    const uint8_t *bssid;
+    uint32_t channel;
+    int *ret;
+} __cyw43_wifi_join_req;
+
+typedef struct {
+    cyw43_t *self;
+    int itf;
+    int *ret;
+} __cyw43_wifi_leave_req;
+
 // Run a callback in the LWIP thread (i.e. for Ethernet device polling and packet reception)
 // When in an interrupt, need to pass in a heap-allocated buffer
 typedef struct {

cores/rp2040/sdkoverride/cyw43_driver_freertos.cpp

@@ -178,54 +178,6 @@ extern "C" void __wrap_cyw43_delay_us(uint32_t us) {
     delayMicroseconds(us);
 }
 
-
-
-static int this_cyw43_arch_wifi_connect_bssid_until(const char *ssid, const uint8_t *bssid, const char *pw, uint32_t auth, uint32_t timeout_ms) {
-    uint32_t start = millis();
-    int err = cyw43_arch_wifi_connect_bssid_async(ssid, bssid, pw, auth);
-    if (err) {
-        return err;
-    }
-    int status = CYW43_LINK_UP + 1;
-    while (status >= 0 && status != CYW43_LINK_UP) {
-        int new_status = cyw43_tcpip_link_status(&cyw43_state, CYW43_ITF_STA);
-        // If there was no network, keep trying
-        if (new_status == CYW43_LINK_NONET) {
-            new_status = CYW43_LINK_JOIN;
-            err = cyw43_arch_wifi_connect_bssid_async(ssid, bssid, pw, auth);
-            if (err) {
-                return err;
-            }
-        }
-        if (new_status != status) {
-            status = new_status;
-        }
-        uint32_t delta = millis() - start;
-        if (delta > timeout_ms) {
-            return PICO_ERROR_TIMEOUT;
-        }
-        // Do polling
-        //cyw43_arch_poll();
-        __wrap_cyw43_await_background_or_timeout_us((timeout_ms - delta) * 1000); //cyw43_arch_wait_for_work_until(until);
-    }
-    // Turn status into a pico_error_codes, CYW43_LINK_NONET shouldn't happen as we fail with PICO_ERROR_TIMEOUT instead
-    assert(status == CYW43_LINK_UP || status == CYW43_LINK_BADAUTH || status == CYW43_LINK_FAIL);
-    if (status == CYW43_LINK_UP) {
-        return PICO_OK; // success
-    } else if (status == CYW43_LINK_BADAUTH) {
-        return PICO_ERROR_BADAUTH;
-    } else {
-        return PICO_ERROR_CONNECT_FAILED;
-    }
-}
-extern "C" int __wrap_cyw43_arch_wifi_connect_bssid_timeout_ms(const char *ssid, const uint8_t *bssid, const char *pw, uint32_t auth, uint32_t timeout_ms) {
-    return this_cyw43_arch_wifi_connect_bssid_until(ssid, bssid, pw, auth, timeout_ms); //make_timeout_time_ms(timeout_ms));
-}
-
-extern "C" int __wrap_cyw43_arch_wifi_connect_timeout_ms(const char *ssid, const char *pw, uint32_t auth, uint32_t timeout_ms) {
-    return __wrap_cyw43_arch_wifi_connect_bssid_timeout_ms(ssid, nullptr, pw, auth, timeout_ms);
-}
-
 extern "C" void __real_cyw43_arch_gpio_put(uint wl_gpio, bool value);
 void do_cyw43_arch_gpio_put(void *data) {
     uint32_t d = (uint32_t)data;

lib/core_wrap.txt

@@ -74,7 +74,6 @@
 -Wl,--wrap=cyw43_cb_process_ethernet
 -Wl,--wrap=cyw43_cb_tcpip_set_link_up
 -Wl,--wrap=cyw43_cb_tcpip_set_link_down
--Wl,--wrap=cyw43_tcpip_link_status
 -Wl,--wrap=cyw43_cb_tcpip_init
 -Wl,--wrap=cyw43_cb_tcpip_deinit
 
@@ -90,7 +89,7 @@
 -Wl,--wrap=cyw43_delay_us
 -Wl,--wrap=cyw43_post_poll_hook
 -Wl,--wrap=cyw43_schedule_internal_poll_dispatch
--Wl,--wrap=cyw43_arch_wifi_connect_bssid_timeout_ms
--Wl,--wrap=cyw43_arch_wifi_connect_timeout_ms
+-Wl,--wrap=cyw43_wifi_join
+-Wl,--wrap=cyw43_wifi_leave
 -Wl,--wrap=cyw43_arch_gpio_put
 -Wl,--wrap=btstack_run_loop_async_context_get_instance

libraries/lwIP_CYW43/src/utility/CYW43shim.cpp

@@ -79,19 +79,38 @@ bool CYW43::begin(const uint8_t* address, netif* netif) {
         cyw43_wifi_update_multicast_filter(&cyw43_state, mdnsV6, true);
 #endif
 
-        if (_bssid[0] | _bssid[1] | _bssid[2] | _bssid[3] | _bssid[4] | _bssid[5]) {
-            if (cyw43_arch_wifi_connect_bssid_timeout_ms(_ssid, _bssid, _password, authmode, _timeout)) {
-                return false;
-            } else {
+        cyw43_wifi_join(&cyw43_state, strlen(_ssid), (const uint8_t *)_ssid, strlen(_password), (const uint8_t *)_password, authmode, (_bssid[0] | _bssid[1] | _bssid[2] | _bssid[3] | _bssid[4] | _bssid[5]) ? _bssid : nullptr, CYW43_CHANNEL_NONE);
+        uint32_t start = millis();
+        while (millis() - start < (long unsigned int)_timeout) {
+            auto link = cyw43_wifi_link_status(&cyw43_state, 0);
+            if (link != 1) {
+                // Some error, we can retry
+                cyw43_wifi_join(&cyw43_state, strlen(_ssid), (const uint8_t *)_ssid, strlen(_password), (const uint8_t *)_password, authmode, (_bssid[0] | _bssid[1] | _bssid[2] | _bssid[3] | _bssid[4] | _bssid[5]) ? _bssid : nullptr, CYW43_CHANNEL_NONE);
+            } else if (cyw43_state.wifi_join_state == 1) {
+                // Link state can == 1 even if the negotiation is ongoing.  Check the internal negotiaion state.  When it hits 1 we are actually connected
                 return true;
+            } else {
+                delay(50);
             }
-        } else {
-            if (cyw43_arch_wifi_connect_timeout_ms(_ssid, _password, authmode, _timeout)) {
-                return false;
+        }
+        if (cyw43_wifi_link_status(&cyw43_state, 0) == 1) {
+            // Now we may have just started a retry and the work is still in progress, or we're connected
+            if (cyw43_state.wifi_join_state == 1) {
+                return true; // We succeeded
             } else {
-                return true;
+                // We're still trying on our last try.  Wait for things to change
+                while (true) {
+                    if (cyw43_wifi_link_status(&cyw43_state, 0) != 1) {
+                        return false;
+                    } else if (cyw43_state.wifi_join_state == 1) {
+                        return true;
+                    } else {
+                        delay(50);
+                    }
+                }
             }
         }
+        return false; // We ended above with a failure
     } else {
         _itf = 1;
         cyw43_arch_enable_ap_mode(_ssid, _password, _password ? CYW43_AUTH_WPA2_AES_PSK : CYW43_AUTH_OPEN);
@@ -102,16 +121,13 @@ bool CYW43::begin(const uint8_t* address, netif* netif) {
 
 void CYW43::end() {
     _netif = nullptr;
-    cyw43_deinit(&cyw43_state);
+    cyw43_wifi_leave(&cyw43_state, _itf);
 }
-int fails = 0;
-int calls = 0;
+
 uint16_t CYW43::sendFrame(const uint8_t* data, uint16_t datalen) {
-    calls++;
     if (0 == cyw43_send_ethernet(_self, _itf, datalen, data, false)) {
         return datalen;
     }
-    fails++;
     return 0;
 }
 

libraries/lwIP_Ethernet/src/LwipIntfDev.h

@@ -402,6 +402,7 @@ bool LwipIntfDev<RawDev>::begin(const uint8_t* macAddress, const uint16_t mtu) {
     }
 
     if (!RawDev::begin(_macAddress, &_netif)) {
+        netif_remove(&_netif);
         return false;
     }
 
@@ -420,6 +421,7 @@ bool LwipIntfDev<RawDev>::begin(const uint8_t* macAddress, const uint16_t mtu) {
             break;
 
         case ERR_IF:
+            netif_remove(&_netif);
             return false;
 
         default: