Merge pull request #7249 from dhalbert/wifi-connect-ap-api

correct Radio.connect() and .start_ap() signatures; clean up some code

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

@@ -55,31 +55,31 @@ mp_obj_t common_hal_wifi_network_get_country(wifi_network_obj_t *self) {
 }
 
 mp_obj_t common_hal_wifi_network_get_authmode(wifi_network_obj_t *self) {
-    uint8_t authmode_mask = 0;
+    uint32_t authmode_mask = 0;
     switch (self->record.authmode) {
         case WIFI_AUTH_OPEN:
-            authmode_mask = (1 << AUTHMODE_OPEN);
+            authmode_mask = AUTHMODE_OPEN;
             break;
         case WIFI_AUTH_WEP:
-            authmode_mask = (1 << AUTHMODE_WEP);
+            authmode_mask = AUTHMODE_WEP;
             break;
         case WIFI_AUTH_WPA_PSK:
-            authmode_mask = (1 << AUTHMODE_WPA) | (1 << AUTHMODE_PSK);
+            authmode_mask = AUTHMODE_WPA | AUTHMODE_PSK;
             break;
         case WIFI_AUTH_WPA2_PSK:
-            authmode_mask = (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK);
+            authmode_mask = AUTHMODE_WPA2 | AUTHMODE_PSK;
             break;
         case WIFI_AUTH_WPA_WPA2_PSK:
-            authmode_mask = (1 << AUTHMODE_WPA) | (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK);
+            authmode_mask = AUTHMODE_WPA | AUTHMODE_WPA2 | AUTHMODE_PSK;
             break;
         case WIFI_AUTH_WPA2_ENTERPRISE:
-            authmode_mask = (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_ENTERPRISE);
+            authmode_mask = AUTHMODE_WPA2 | AUTHMODE_ENTERPRISE;
             break;
         case WIFI_AUTH_WPA3_PSK:
-            authmode_mask = (1 << AUTHMODE_WPA3) | (1 << AUTHMODE_PSK);
+            authmode_mask = AUTHMODE_WPA3 | AUTHMODE_PSK;
             break;
         case WIFI_AUTH_WPA2_WPA3_PSK:
-            authmode_mask = (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_WPA3) | (1 << AUTHMODE_PSK);
+            authmode_mask = AUTHMODE_WPA2 | AUTHMODE_WPA3 | AUTHMODE_PSK;
             break;
         default:
             break;

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

@@ -205,20 +205,21 @@ void common_hal_wifi_radio_stop_station(wifi_radio_obj_t *self) {
     set_mode_station(self, false);
 }
 
-void common_hal_wifi_radio_start_ap(wifi_radio_obj_t *self, uint8_t *ssid, size_t ssid_len, uint8_t *password, size_t password_len, uint8_t channel, uint8_t authmode, uint8_t max_connections) {
+void common_hal_wifi_radio_start_ap(wifi_radio_obj_t *self, uint8_t *ssid, size_t ssid_len, uint8_t *password, size_t password_len, uint8_t channel, uint32_t authmodes, uint8_t max_connections) {
     set_mode_ap(self, true);
 
-    switch (authmode) {
+    uint8_t authmode = 0;
-        case (1 << AUTHMODE_OPEN):
+    switch (authmodes) {
+        case AUTHMODE_OPEN:
             authmode = WIFI_AUTH_OPEN;
             break;
-        case ((1 << AUTHMODE_WPA) | (1 << AUTHMODE_PSK)):
+        case AUTHMODE_WPA | AUTHMODE_PSK:
             authmode = WIFI_AUTH_WPA_PSK;
             break;
-        case ((1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK)):
+        case AUTHMODE_WPA2 | AUTHMODE_PSK:
             authmode = WIFI_AUTH_WPA2_PSK;
             break;
-        case ((1 << AUTHMODE_WPA) | (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK)):
+        case AUTHMODE_WPA | AUTHMODE_WPA2 | AUTHMODE_PSK:
             authmode = WIFI_AUTH_WPA_WPA2_PSK;
             break;
         default:

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

@@ -55,18 +55,18 @@ mp_obj_t common_hal_wifi_network_get_country(wifi_network_obj_t *self) {
 mp_obj_t common_hal_wifi_network_get_authmode(wifi_network_obj_t *self) {
     uint8_t authmode_mask = 0;
     if (self->record.auth_mode == 0) {
-        authmode_mask = (1 << AUTHMODE_OPEN);
+        authmode_mask = AUTHMODE_OPEN;
     }
     if (self->record.auth_mode & 1) {
-        authmode_mask |= (1 << AUTHMODE_PSK);
+        authmode_mask |= AUTHMODE_PSK;
     }
     ;
     if (self->record.auth_mode & 2) {
-        authmode_mask |= (1 << AUTHMODE_WPA);
+        authmode_mask |= AUTHMODE_WPA;
     }
     ;
     if (self->record.auth_mode & 4) {
-        authmode_mask |= (1 << AUTHMODE_WPA2);
+        authmode_mask |= AUTHMODE_WPA2;
     }
     ;
     mp_obj_t authmode_list = mp_obj_new_list(0, NULL);

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

@@ -164,7 +164,7 @@ void common_hal_wifi_radio_stop_station(wifi_radio_obj_t *self) {
     bindings_cyw43_wifi_enforce_pm();
 }
 
-void common_hal_wifi_radio_start_ap(wifi_radio_obj_t *self, uint8_t *ssid, size_t ssid_len, uint8_t *password, size_t password_len, uint8_t channel, uint8_t authmode, uint8_t max_connections) {
+void common_hal_wifi_radio_start_ap(wifi_radio_obj_t *self, uint8_t *ssid, size_t ssid_len, uint8_t *password, size_t password_len, uint8_t channel, uint32_t authmodes, uint8_t max_connections) {
     mp_raise_NotImplementedError(NULL);
     bindings_cyw43_wifi_enforce_pm();
 }

shared-bindings/wifi/AuthMode.h

@@ -30,13 +30,13 @@
 #include "py/enum.h"
 
 typedef enum {
-    AUTHMODE_OPEN,
+    AUTHMODE_OPEN = 1 << 0,
-    AUTHMODE_WEP,
+    AUTHMODE_WEP = 1 << 1,
-    AUTHMODE_WPA,
+    AUTHMODE_WPA = 1 << 2,
-    AUTHMODE_WPA2,
+    AUTHMODE_WPA2 = 1 << 3,
-    AUTHMODE_WPA3,
+    AUTHMODE_WPA3 = 1 << 4,
-    AUTHMODE_PSK,
+    AUTHMODE_PSK = 1 << 5,
-    AUTHMODE_ENTERPRISE
+    AUTHMODE_ENTERPRISE = 1 << 6,
 } wifi_authmode_t;
 
 extern const mp_obj_type_t wifi_authmode_type;

shared-bindings/wifi/Radio.c

@@ -282,10 +282,10 @@ MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_stop_station_obj, wifi_radio_stop_station);
 //|     def start_ap(
 //|         self,
 //|         ssid: Union[str | ReadableBuffer],
-//|         password: Union[str | ReadableBuffer] = "",
+//|         password: Union[str | ReadableBuffer] = b"",
 //|         *,
-//|         channel: Optional[int] = 1,
+//|         channel: int = 1,
-//|         authmode: Optional[AuthMode],
+//|         authmode: Optional[AuthMode] = None,
 //|         max_connections: Optional[int] = 4
 //|     ) -> None:
 //|         """Starts an Access Point with the specified ssid and password.
@@ -293,10 +293,11 @@ MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_stop_station_obj, wifi_radio_stop_station);
 //|         If ``channel`` is given, the access point will use that channel unless
 //|         a station is already operating on a different channel.
 //|
-//|         If ``authmode`` is given, the access point will use that Authentication
+//|         If ``authmode`` is not None, the access point will use that Authentication
-//|         mode. If a password is given, ``authmode`` must not be ``OPEN``.
+//|         mode. If a non-empty password is given, ``authmode`` must not be ``OPEN``.
-//|         If ``authmode`` isn't given, ``OPEN`` will be used when password isn't provided,
+//|         If ``authmode`` is not given or is None,
-//|         otherwise ``WPA_WPA2_PSK``.
+//|         ``OPEN`` will be used when the password is the empty string,
+//|         otherwise ``authmode`` will be ``WPA_WPA2_PSK``.
 //|
 //|         If ``max_connections`` is given, the access point will allow up to
 //|         that number of stations to connect."""
@@ -305,9 +306,9 @@ STATIC mp_obj_t wifi_radio_start_ap(size_t n_args, const mp_obj_t *pos_args, mp_
     enum { ARG_ssid, ARG_password, ARG_channel, ARG_authmode, ARG_max_connections };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_ssid, MP_ARG_REQUIRED | MP_ARG_OBJ },
-        { MP_QSTR_password,  MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
+        { MP_QSTR_password,  MP_ARG_OBJ, {.u_obj = mp_const_empty_bytes} },
         { MP_QSTR_channel, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 1} },
-        { MP_QSTR_authmode, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
+        { MP_QSTR_authmode, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none } },
         { MP_QSTR_max_connections, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 4} },
     };
 
@@ -315,12 +316,13 @@ STATIC mp_obj_t wifi_radio_start_ap(size_t n_args, const mp_obj_t *pos_args, mp_
     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 authmode = 0;
+    // 0 indicates mode wasn't given.
-    if (args[ARG_authmode].u_obj != MP_OBJ_NULL) {
+    uint32_t authmodes = 0;
+    if (args[ARG_authmode].u_obj != mp_const_none) {
         mp_obj_iter_buf_t iter_buf;
         mp_obj_t item, iterable = mp_getiter(args[ARG_authmode].u_obj, &iter_buf);
         while ((item = mp_iternext(iterable)) != MP_OBJ_STOP_ITERATION) {
-            authmode |= (1 << (wifi_authmode_t)cp_enum_value(&wifi_authmode_type, item));
+            authmodes |= cp_enum_value(&wifi_authmode_type, item);
         }
     }
 
@@ -329,20 +331,24 @@ STATIC mp_obj_t wifi_radio_start_ap(size_t n_args, const mp_obj_t *pos_args, mp_
     mp_arg_validate_length_range(ssid.len, 1, 32, MP_QSTR_ssid);
 
     mp_buffer_info_t password;
-    password.len = 0;
+    mp_get_buffer_raise(args[ARG_password].u_obj, &password, MP_BUFFER_READ);
-    if (args[ARG_password].u_obj != MP_OBJ_NULL) {
+    if (authmodes == 0) {
-        if (authmode == 1) {
+        if (password.len == 0) {
-            mp_raise_ValueError(translate("AuthMode.OPEN is not used with password"));
+            authmodes = AUTHMODE_OPEN;
-        } else if (authmode == 0) {
+        } else {
-            authmode = (1 << AUTHMODE_WPA) | (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK);
+            authmodes = AUTHMODE_WPA | AUTHMODE_WPA2 | AUTHMODE_PSK;
         }
-        mp_get_buffer_raise(args[ARG_password].u_obj, &password, MP_BUFFER_READ);
-        mp_arg_validate_length_range(password.len, 8, 63, MP_QSTR_password);
-    } else {
-        authmode = 1;
     }
 
-    common_hal_wifi_radio_start_ap(self, ssid.buf, ssid.len, password.buf, password.len, args[ARG_channel].u_int, authmode, args[ARG_max_connections].u_int);
+    if (authmodes == AUTHMODE_OPEN && password.len > 0) {
+        mp_raise_ValueError(translate("AuthMode.OPEN is not used with password"));
+    }
+
+    if (authmodes != AUTHMODE_OPEN) {
+        mp_arg_validate_length_range(password.len, 8, 63, MP_QSTR_password);
+    }
+
+    common_hal_wifi_radio_start_ap(self, ssid.buf, ssid.len, password.buf, password.len, args[ARG_channel].u_int, authmodes, args[ARG_max_connections].u_int);
     return mp_const_none;
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_KW(wifi_radio_start_ap_obj, 1, wifi_radio_start_ap);
@@ -359,10 +365,10 @@ MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_stop_ap_obj, wifi_radio_stop_ap);
 //|     def connect(
 //|         self,
 //|         ssid: Union[str | ReadableBuffer],
-//|         password: Union[str | ReadableBuffer] = "",
+//|         password: Union[str | ReadableBuffer] = b"",
 //|         *,
-//|         channel: Optional[int] = 0,
+//|         channel: int = 0,
-//|         bssid: Optional[Union[str | ReadableBuffer]] = "",
+//|         bssid: Optional[Union[str | ReadableBuffer]] = None,
 //|         timeout: Optional[float] = None
 //|     ) -> None:
 //|         """Connects to the given ssid and waits for an ip address. Reconnections are handled
@@ -371,20 +377,20 @@ MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_stop_ap_obj, wifi_radio_stop_ap);
 //|         By default, this will scan all channels and connect to the access point (AP) with the
 //|         given ``ssid`` and greatest signal strength (rssi).
 //|
-//|         If ``channel`` is given, the scan will begin with the given channel and connect to
+//|         If ``channel`` is non-zero, the scan will begin with the given channel and connect to
 //|         the first AP with the given ``ssid``. This can speed up the connection time
 //|         significantly because a full scan doesn't occur.
 //|
-//|         If ``bssid`` is given, the scan will start at the first channel or the one given and
+//|         If ``bssid`` is given and not None, the scan will start at the first channel or the one given and
 //|         connect to the AP with the given ``bssid`` and ``ssid``."""
 //|         ...
 STATIC mp_obj_t wifi_radio_connect(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     enum { ARG_ssid, ARG_password, ARG_channel, ARG_bssid, ARG_timeout };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_ssid, MP_ARG_REQUIRED | MP_ARG_OBJ },
-        { MP_QSTR_password,  MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
+        { MP_QSTR_password,  MP_ARG_OBJ, {.u_obj = mp_const_empty_bytes} },
         { MP_QSTR_channel, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0} },
-        { MP_QSTR_bssid, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
+        { MP_QSTR_bssid, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} },
         { MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} },
     };
 
@@ -404,9 +410,11 @@ STATIC mp_obj_t wifi_radio_connect(size_t n_args, const mp_obj_t *pos_args, mp_m
 
     mp_buffer_info_t password;
     password.len = 0;
-    if (args[ARG_password].u_obj != MP_OBJ_NULL) {
+    if (args[ARG_password].u_obj != mp_const_none) {
         mp_get_buffer_raise(args[ARG_password].u_obj, &password, MP_BUFFER_READ);
-        mp_arg_validate_length_range(password.len, 8, 63, MP_QSTR_password);
+        if (password.len != 0) {
+            mp_arg_validate_length_range(password.len, 8, 63, MP_QSTR_password);
+        }
     }
 
     #define MAC_ADDRESS_LENGTH 6
@@ -414,7 +422,7 @@ STATIC mp_obj_t wifi_radio_connect(size_t n_args, const mp_obj_t *pos_args, mp_m
     mp_buffer_info_t bssid;
     bssid.len = 0;
     // Should probably make sure bssid is just bytes and not something else too
-    if (args[ARG_bssid].u_obj != MP_OBJ_NULL) {
+    if (args[ARG_bssid].u_obj != mp_const_none) {
         mp_get_buffer_raise(args[ARG_bssid].u_obj, &bssid, MP_BUFFER_READ);
         if (bssid.len != MAC_ADDRESS_LENGTH) {
             mp_raise_ValueError(translate("Invalid BSSID"));

shared-bindings/wifi/Radio.h

@@ -91,7 +91,7 @@ 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);
 extern void common_hal_wifi_radio_stop_station(wifi_radio_obj_t *self);
 
-extern void common_hal_wifi_radio_start_ap(wifi_radio_obj_t *self, uint8_t *ssid, size_t ssid_len, uint8_t *password, size_t password_len, uint8_t channel, uint8_t authmode, uint8_t max_connections);
+extern void common_hal_wifi_radio_start_ap(wifi_radio_obj_t *self, uint8_t *ssid, size_t ssid_len, uint8_t *password, size_t password_len, uint8_t channel, uint32_t authmodes, uint8_t max_connections);
 extern void common_hal_wifi_radio_stop_ap(wifi_radio_obj_t *self);
 
 extern void common_hal_wifi_radio_start_dhcp_client(wifi_radio_obj_t *self);