extmod/network_cyw43: Allow configuring active AP interface.

Configuring the AP for cyw43 writes to some buffers that are only sent to
the modem when the interface is brought up. This means you can't configure
the AP after calling active(True), the new settings seem to be accepted but
the radio doesn't change.

This is different to the WLAN behaviour on other ports. The esp8266 port
requires calling active(True) on the AP before configuring, even.

Fix this by bouncing the AP interface after a config change, if it's
active. Configuring with active(False) still works the same as before.

Adds a static variable to track interface active state, rather than relying
on the LWIP interface state. This is because the interface state is updated
by a driver callback and there's a race: if code calls active(True) and
then config(a=b) then the driver doesn't know it's active yet and the
changes aren't correctly applied.

It is possible this pattern will cause the AP to come up briefly with the
default "PICOabcd" SSID before being reconfigured, however (due to the
aforementioned race condition) it seems like this may not happen at all
before the new config is applied.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>

1 files changed, 27 insertions, 3 deletions

diff --git a/extmod/network_cyw43.c b/extmod/network_cyw43.c
index cebc8deb0..02b022cb8 100644
--- a/extmod/network_cyw43.c
+++ b/extmod/network_cyw43.c
@@ -60,6 +60,10 @@ typedef struct _network_cyw43_obj_t {
 static const network_cyw43_obj_t network_cyw43_wl_sta = { { &mp_network_cyw43_type }, &cyw43_state, CYW43_ITF_STA };
 static const network_cyw43_obj_t network_cyw43_wl_ap = { { &mp_network_cyw43_type }, &cyw43_state, CYW43_ITF_AP };
 
+// Avoid race conditions with callbacks by tracking the last up or down request
+// we have made for each interface.
+static bool if_active[2];
+
 static void network_cyw43_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) {
     network_cyw43_obj_t *self = MP_OBJ_TO_PTR(self_in);
     struct netif *netif = &self->cyw->netif[self->itf];
@@ -122,6 +126,10 @@ static MP_DEFINE_CONST_FUN_OBJ_3(network_cyw43_ioctl_obj, network_cyw43_ioctl);
 /*******************************************************************************/
 // network API
 
+static uint32_t get_country_code(void) {
+    return CYW43_COUNTRY(mod_network_country_code[0], mod_network_country_code[1], 0);
+}
+
 static mp_obj_t network_cyw43_deinit(mp_obj_t self_in) {
     network_cyw43_obj_t *self = MP_OBJ_TO_PTR(self_in);
     cyw43_deinit(self->cyw);
@@ -132,10 +140,11 @@ static MP_DEFINE_CONST_FUN_OBJ_1(network_cyw43_deinit_obj, network_cyw43_deinit)
 static mp_obj_t network_cyw43_active(size_t n_args, const mp_obj_t *args) {
     network_cyw43_obj_t *self = MP_OBJ_TO_PTR(args[0]);
     if (n_args == 1) {
-        return mp_obj_new_bool(cyw43_tcpip_link_status(self->cyw, self->itf));
+        return mp_obj_new_bool(if_active[self->itf]);
     } else {
-        uint32_t country = CYW43_COUNTRY(mod_network_country_code[0], mod_network_country_code[1], 0);
-        cyw43_wifi_set_up(self->cyw, self->itf, mp_obj_is_true(args[1]), country);
+        bool value = mp_obj_is_true(args[1]);
+        cyw43_wifi_set_up(self->cyw, self->itf, value, get_country_code());
+        if_active[self->itf] = value;
         return mp_const_none;
     }
 }
@@ -457,6 +466,10 @@ static mp_obj_t network_cyw43_config(size_t n_args, const mp_obj_t *args, mp_map
             mp_raise_TypeError(MP_ERROR_TEXT("can't specify pos and kw args"));
         }
 
+        // A number of these options only update buffers in memory, and
+        // won't do anything until the interface is cycled down and back up
+        bool cycle_active = false;
+
         for (size_t i = 0; i < kwargs->alloc; ++i) {
             if (MP_MAP_SLOT_IS_FILLED(kwargs, i)) {
                 mp_map_elem_t *e = &kwargs->table[i];
@@ -469,6 +482,7 @@ static mp_obj_t network_cyw43_config(size_t n_args, const mp_obj_t *args, mp_map
                     }
                     case MP_QSTR_channel: {
                         cyw43_wifi_ap_set_channel(self->cyw, mp_obj_get_int(e->value));
+                        cycle_active = true;
                         break;
                     }
                     case MP_QSTR_ssid:
@@ -476,6 +490,7 @@ static mp_obj_t network_cyw43_config(size_t n_args, const mp_obj_t *args, mp_map
                         size_t len;
                         const char *str = mp_obj_str_get_data(e->value, &len);
                         cyw43_wifi_ap_set_ssid(self->cyw, len, (const uint8_t *)str);
+                        cycle_active = true;
                         break;
                     }
                     case MP_QSTR_monitor: {
@@ -495,6 +510,7 @@ static mp_obj_t network_cyw43_config(size_t n_args, const mp_obj_t *args, mp_map
                     }
                     case MP_QSTR_security: {
                         cyw43_wifi_ap_set_auth(self->cyw, mp_obj_get_int(e->value));
+                        cycle_active = true;
                         break;
                     }
                     case MP_QSTR_key:
@@ -502,6 +518,7 @@ static mp_obj_t network_cyw43_config(size_t n_args, const mp_obj_t *args, mp_map
                         size_t len;
                         const char *str = mp_obj_str_get_data(e->value, &len);
                         cyw43_wifi_ap_set_password(self->cyw, len, (const uint8_t *)str);
+                        cycle_active = true;
                         break;
                     }
                     case MP_QSTR_pm: {
@@ -531,6 +548,13 @@ static mp_obj_t network_cyw43_config(size_t n_args, const mp_obj_t *args, mp_map
             }
         }
 
+        // If the interface is already active, cycle it down and up
+        if (cycle_active && if_active[self->itf]) {
+            uint32_t country = get_country_code();
+            cyw43_wifi_set_up(self->cyw, self->itf, false, country);
+            cyw43_wifi_set_up(self->cyw, self->itf, true, country);
+        }
+
         return mp_const_none;
     }
 }