From 17be447c4bae7aeb63d3ad617c87fcdc2b0cacf5 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 22 Nov 2022 16:09:47 -0500 Subject: [PATCH] correct Radio.connect() and .start_ap() signatures; clean up some code --- ports/espressif/common-hal/wifi/Network.c | 18 ++--- ports/espressif/common-hal/wifi/Radio.c | 13 ++-- ports/raspberrypi/common-hal/wifi/Network.c | 8 +-- ports/raspberrypi/common-hal/wifi/Radio.c | 2 +- shared-bindings/wifi/AuthMode.h | 14 ++-- shared-bindings/wifi/Radio.c | 74 ++++++++++++--------- shared-bindings/wifi/Radio.h | 2 +- 7 files changed, 70 insertions(+), 61 deletions(-) diff --git a/ports/espressif/common-hal/wifi/Network.c b/ports/espressif/common-hal/wifi/Network.c index 34cb15d603..5c9852a1f7 100644 --- a/ports/espressif/common-hal/wifi/Network.c +++ b/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; diff --git a/ports/espressif/common-hal/wifi/Radio.c b/ports/espressif/common-hal/wifi/Radio.c index 27e2c8c610..acd6ae53bc 100644 --- a/ports/espressif/common-hal/wifi/Radio.c +++ b/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) { - case (1 << AUTHMODE_OPEN): + uint8_t authmode = 0; + 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: diff --git a/ports/raspberrypi/common-hal/wifi/Network.c b/ports/raspberrypi/common-hal/wifi/Network.c index 8db42e962c..e9b64ffd2c 100644 --- a/ports/raspberrypi/common-hal/wifi/Network.c +++ b/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); diff --git a/ports/raspberrypi/common-hal/wifi/Radio.c b/ports/raspberrypi/common-hal/wifi/Radio.c index 2a74c16e0a..a4df1d22d6 100644 --- a/ports/raspberrypi/common-hal/wifi/Radio.c +++ b/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(); } diff --git a/shared-bindings/wifi/AuthMode.h b/shared-bindings/wifi/AuthMode.h index a1016d6c8a..c514fb2a32 100644 --- a/shared-bindings/wifi/AuthMode.h +++ b/shared-bindings/wifi/AuthMode.h @@ -30,13 +30,13 @@ #include "py/enum.h" typedef enum { - AUTHMODE_OPEN, - AUTHMODE_WEP, - AUTHMODE_WPA, - AUTHMODE_WPA2, - AUTHMODE_WPA3, - AUTHMODE_PSK, - AUTHMODE_ENTERPRISE + AUTHMODE_OPEN = 1 << 0, + AUTHMODE_WEP = 1 << 1, + AUTHMODE_WPA = 1 << 2, + AUTHMODE_WPA2 = 1 << 3, + AUTHMODE_WPA3 = 1 << 4, + AUTHMODE_PSK = 1 << 5, + AUTHMODE_ENTERPRISE = 1 << 6, } wifi_authmode_t; extern const mp_obj_type_t wifi_authmode_type; diff --git a/shared-bindings/wifi/Radio.c b/shared-bindings/wifi/Radio.c index 57e953ec68..37123dbb44 100644 --- a/shared-bindings/wifi/Radio.c +++ b/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, -//| authmode: Optional[AuthMode], +//| channel: int = 1, +//| 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 -//| mode. If a password is given, ``authmode`` must not be ``OPEN``. -//| If ``authmode`` isn't given, ``OPEN`` will be used when password isn't provided, -//| otherwise ``WPA_WPA2_PSK``. +//| If ``authmode`` is not None, the access point will use that Authentication +//| mode. If a non-empty password is given, ``authmode`` must not be ``OPEN``. +//| If ``authmode`` is not given or is None, +//| ``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; - if (args[ARG_authmode].u_obj != MP_OBJ_NULL) { + // 0 indicates mode wasn't given. + 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; - if (args[ARG_password].u_obj != MP_OBJ_NULL) { - if (authmode == 1) { - mp_raise_ValueError(translate("AuthMode.OPEN is not used with password")); - } else if (authmode == 0) { - authmode = (1 << AUTHMODE_WPA) | (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK); + mp_get_buffer_raise(args[ARG_password].u_obj, &password, MP_BUFFER_READ); + if (authmodes == 0) { + if (password.len == 0) { + authmodes = AUTHMODE_OPEN; + } else { + 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, -//| bssid: Optional[Union[str | ReadableBuffer]] = "", +//| channel: int = 0, +//| 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")); diff --git a/shared-bindings/wifi/Radio.h b/shared-bindings/wifi/Radio.h index 370bdbd917..407443c77e 100644 --- a/shared-bindings/wifi/Radio.h +++ b/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);