Skip to content

Commit 17be447

Browse files
committed
correct Radio.connect() and .start_ap() signatures; clean up some code
1 parent df0150f commit 17be447

File tree

7 files changed

+69
-60
lines changed

7 files changed

+69
-60
lines changed

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,31 +55,31 @@ mp_obj_t common_hal_wifi_network_get_country(wifi_network_obj_t *self) {
5555
}
5656

5757
mp_obj_t common_hal_wifi_network_get_authmode(wifi_network_obj_t *self) {
58-
uint8_t authmode_mask = 0;
58+
uint32_t authmode_mask = 0;
5959
switch (self->record.authmode) {
6060
case WIFI_AUTH_OPEN:
61-
authmode_mask = (1 << AUTHMODE_OPEN);
61+
authmode_mask = AUTHMODE_OPEN;
6262
break;
6363
case WIFI_AUTH_WEP:
64-
authmode_mask = (1 << AUTHMODE_WEP);
64+
authmode_mask = AUTHMODE_WEP;
6565
break;
6666
case WIFI_AUTH_WPA_PSK:
67-
authmode_mask = (1 << AUTHMODE_WPA) | (1 << AUTHMODE_PSK);
67+
authmode_mask = AUTHMODE_WPA | AUTHMODE_PSK;
6868
break;
6969
case WIFI_AUTH_WPA2_PSK:
70-
authmode_mask = (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK);
70+
authmode_mask = AUTHMODE_WPA2 | AUTHMODE_PSK;
7171
break;
7272
case WIFI_AUTH_WPA_WPA2_PSK:
73-
authmode_mask = (1 << AUTHMODE_WPA) | (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK);
73+
authmode_mask = AUTHMODE_WPA | AUTHMODE_WPA2 | AUTHMODE_PSK;
7474
break;
7575
case WIFI_AUTH_WPA2_ENTERPRISE:
76-
authmode_mask = (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_ENTERPRISE);
76+
authmode_mask = AUTHMODE_WPA2 | AUTHMODE_ENTERPRISE;
7777
break;
7878
case WIFI_AUTH_WPA3_PSK:
79-
authmode_mask = (1 << AUTHMODE_WPA3) | (1 << AUTHMODE_PSK);
79+
authmode_mask = AUTHMODE_WPA3 | AUTHMODE_PSK;
8080
break;
8181
case WIFI_AUTH_WPA2_WPA3_PSK:
82-
authmode_mask = (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_WPA3) | (1 << AUTHMODE_PSK);
82+
authmode_mask = AUTHMODE_WPA2 | AUTHMODE_WPA3 | AUTHMODE_PSK;
8383
break;
8484
default:
8585
break;

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -205,20 +205,21 @@ void common_hal_wifi_radio_stop_station(wifi_radio_obj_t *self) {
205205
set_mode_station(self, false);
206206
}
207207

208-
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) {
208+
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) {
209209
set_mode_ap(self, true);
210210

211-
switch (authmode) {
212-
case (1 << AUTHMODE_OPEN):
211+
uint8_t authmode = 0;
212+
switch (authmodes) {
213+
case AUTHMODE_OPEN:
213214
authmode = WIFI_AUTH_OPEN;
214215
break;
215-
case ((1 << AUTHMODE_WPA) | (1 << AUTHMODE_PSK)):
216+
case AUTHMODE_WPA | AUTHMODE_PSK:
216217
authmode = WIFI_AUTH_WPA_PSK;
217218
break;
218-
case ((1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK)):
219+
case AUTHMODE_WPA2 | AUTHMODE_PSK:
219220
authmode = WIFI_AUTH_WPA2_PSK;
220221
break;
221-
case ((1 << AUTHMODE_WPA) | (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK)):
222+
case AUTHMODE_WPA | AUTHMODE_WPA2 | AUTHMODE_PSK:
222223
authmode = WIFI_AUTH_WPA_WPA2_PSK;
223224
break;
224225
default:

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,18 @@ mp_obj_t common_hal_wifi_network_get_country(wifi_network_obj_t *self) {
5555
mp_obj_t common_hal_wifi_network_get_authmode(wifi_network_obj_t *self) {
5656
uint8_t authmode_mask = 0;
5757
if (self->record.auth_mode == 0) {
58-
authmode_mask = (1 << AUTHMODE_OPEN);
58+
authmode_mask = AUTHMODE_OPEN;
5959
}
6060
if (self->record.auth_mode & 1) {
61-
authmode_mask |= (1 << AUTHMODE_PSK);
61+
authmode_mask |= AUTHMODE_PSK;
6262
}
6363
;
6464
if (self->record.auth_mode & 2) {
65-
authmode_mask |= (1 << AUTHMODE_WPA);
65+
authmode_mask |= AUTHMODE_WPA;
6666
}
6767
;
6868
if (self->record.auth_mode & 4) {
69-
authmode_mask |= (1 << AUTHMODE_WPA2);
69+
authmode_mask |= AUTHMODE_WPA2;
7070
}
7171
;
7272
mp_obj_t authmode_list = mp_obj_new_list(0, NULL);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ void common_hal_wifi_radio_stop_station(wifi_radio_obj_t *self) {
164164
bindings_cyw43_wifi_enforce_pm();
165165
}
166166

167-
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) {
167+
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) {
168168
mp_raise_NotImplementedError(NULL);
169169
bindings_cyw43_wifi_enforce_pm();
170170
}

shared-bindings/wifi/AuthMode.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@
3030
#include "py/enum.h"
3131

3232
typedef enum {
33-
AUTHMODE_OPEN,
34-
AUTHMODE_WEP,
35-
AUTHMODE_WPA,
36-
AUTHMODE_WPA2,
37-
AUTHMODE_WPA3,
38-
AUTHMODE_PSK,
39-
AUTHMODE_ENTERPRISE
33+
AUTHMODE_OPEN = 1 << 0,
34+
AUTHMODE_WEP = 1 << 1,
35+
AUTHMODE_WPA = 1 << 2,
36+
AUTHMODE_WPA2 = 1 << 3,
37+
AUTHMODE_WPA3 = 1 << 4,
38+
AUTHMODE_PSK = 1 << 5,
39+
AUTHMODE_ENTERPRISE = 1 << 6,
4040
} wifi_authmode_t;
4141

4242
extern const mp_obj_type_t wifi_authmode_type;

shared-bindings/wifi/Radio.c

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -282,21 +282,22 @@ MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_stop_station_obj, wifi_radio_stop_station);
282282
//| def start_ap(
283283
//| self,
284284
//| ssid: Union[str | ReadableBuffer],
285-
//| password: Union[str | ReadableBuffer] = "",
285+
//| password: Union[str | ReadableBuffer] = b"",
286286
//| *,
287-
//| channel: Optional[int] = 1,
288-
//| authmode: Optional[AuthMode],
287+
//| channel: int = 1,
288+
//| authmode: Optional[AuthMode] = None,
289289
//| max_connections: Optional[int] = 4
290290
//| ) -> None:
291291
//| """Starts an Access Point with the specified ssid and password.
292292
//|
293293
//| If ``channel`` is given, the access point will use that channel unless
294294
//| a station is already operating on a different channel.
295295
//|
296-
//| If ``authmode`` is given, the access point will use that Authentication
297-
//| mode. If a password is given, ``authmode`` must not be ``OPEN``.
298-
//| If ``authmode`` isn't given, ``OPEN`` will be used when password isn't provided,
299-
//| otherwise ``WPA_WPA2_PSK``.
296+
//| If ``authmode`` is not None, the access point will use that Authentication
297+
//| mode. If a non-empty password is given, ``authmode`` must not be ``OPEN``.
298+
//| If ``authmode`` is not given or is None,
299+
//| ``OPEN`` will be used when the password is the empty string,
300+
//| otherwise ``authmode`` will be ``WPA_WPA2_PSK``.
300301
//|
301302
//| If ``max_connections`` is given, the access point will allow up to
302303
//| that number of stations to connect."""
@@ -305,22 +306,23 @@ STATIC mp_obj_t wifi_radio_start_ap(size_t n_args, const mp_obj_t *pos_args, mp_
305306
enum { ARG_ssid, ARG_password, ARG_channel, ARG_authmode, ARG_max_connections };
306307
static const mp_arg_t allowed_args[] = {
307308
{ MP_QSTR_ssid, MP_ARG_REQUIRED | MP_ARG_OBJ },
308-
{ MP_QSTR_password, MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
309+
{ MP_QSTR_password, MP_ARG_OBJ, {.u_obj = mp_const_empty_bytes} },
309310
{ MP_QSTR_channel, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 1} },
310-
{ MP_QSTR_authmode, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
311+
{ MP_QSTR_authmode, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none } },
311312
{ MP_QSTR_max_connections, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 4} },
312313
};
313314

314315
wifi_radio_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
315316
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
316317
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
317318

318-
uint8_t authmode = 0;
319-
if (args[ARG_authmode].u_obj != MP_OBJ_NULL) {
319+
// 0 indicates mode wasn't given.
320+
uint32_t authmodes = 0;
321+
if (args[ARG_authmode].u_obj != mp_const_none) {
320322
mp_obj_iter_buf_t iter_buf;
321323
mp_obj_t item, iterable = mp_getiter(args[ARG_authmode].u_obj, &iter_buf);
322324
while ((item = mp_iternext(iterable)) != MP_OBJ_STOP_ITERATION) {
323-
authmode |= (1 << (wifi_authmode_t)cp_enum_value(&wifi_authmode_type, item));
325+
authmodes |= cp_enum_value(&wifi_authmode_type, item);
324326
}
325327
}
326328

@@ -329,20 +331,24 @@ STATIC mp_obj_t wifi_radio_start_ap(size_t n_args, const mp_obj_t *pos_args, mp_
329331
mp_arg_validate_length_range(ssid.len, 1, 32, MP_QSTR_ssid);
330332

331333
mp_buffer_info_t password;
332-
password.len = 0;
333-
if (args[ARG_password].u_obj != MP_OBJ_NULL) {
334-
if (authmode == 1) {
335-
mp_raise_ValueError(translate("AuthMode.OPEN is not used with password"));
336-
} else if (authmode == 0) {
337-
authmode = (1 << AUTHMODE_WPA) | (1 << AUTHMODE_WPA2) | (1 << AUTHMODE_PSK);
334+
mp_get_buffer_raise(args[ARG_password].u_obj, &password, MP_BUFFER_READ);
335+
if (authmodes == 0) {
336+
if (password.len == 0) {
337+
authmodes = AUTHMODE_OPEN;
338+
} else {
339+
authmodes = AUTHMODE_WPA | AUTHMODE_WPA2 | AUTHMODE_PSK;
338340
}
339-
mp_get_buffer_raise(args[ARG_password].u_obj, &password, MP_BUFFER_READ);
341+
}
342+
343+
if (authmodes == AUTHMODE_OPEN && password.len > 0) {
344+
mp_raise_ValueError(translate("AuthMode.OPEN is not used with password"));
345+
}
346+
347+
if (authmodes != AUTHMODE_OPEN) {
340348
mp_arg_validate_length_range(password.len, 8, 63, MP_QSTR_password);
341-
} else {
342-
authmode = 1;
343349
}
344350

345-
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);
351+
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);
346352
return mp_const_none;
347353
}
348354
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);
359365
//| def connect(
360366
//| self,
361367
//| ssid: Union[str | ReadableBuffer],
362-
//| password: Union[str | ReadableBuffer] = "",
368+
//| password: Union[str | ReadableBuffer] = b"",
363369
//| *,
364-
//| channel: Optional[int] = 0,
365-
//| bssid: Optional[Union[str | ReadableBuffer]] = "",
370+
//| channel: int = 0,
371+
//| bssid: Optional[Union[str | ReadableBuffer]] = None,
366372
//| timeout: Optional[float] = None
367373
//| ) -> None:
368374
//| """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);
371377
//| By default, this will scan all channels and connect to the access point (AP) with the
372378
//| given ``ssid`` and greatest signal strength (rssi).
373379
//|
374-
//| If ``channel`` is given, the scan will begin with the given channel and connect to
380+
//| If ``channel`` is non-zero, the scan will begin with the given channel and connect to
375381
//| the first AP with the given ``ssid``. This can speed up the connection time
376382
//| significantly because a full scan doesn't occur.
377383
//|
378-
//| If ``bssid`` is given, the scan will start at the first channel or the one given and
384+
//| If ``bssid`` is given and not None, the scan will start at the first channel or the one given and
379385
//| connect to the AP with the given ``bssid`` and ``ssid``."""
380386
//| ...
381387
STATIC mp_obj_t wifi_radio_connect(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
382388
enum { ARG_ssid, ARG_password, ARG_channel, ARG_bssid, ARG_timeout };
383389
static const mp_arg_t allowed_args[] = {
384390
{ MP_QSTR_ssid, MP_ARG_REQUIRED | MP_ARG_OBJ },
385-
{ MP_QSTR_password, MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
391+
{ MP_QSTR_password, MP_ARG_OBJ, {.u_obj = mp_const_empty_bytes} },
386392
{ MP_QSTR_channel, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0} },
387-
{ MP_QSTR_bssid, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
393+
{ MP_QSTR_bssid, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} },
388394
{ MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} },
389395
};
390396

@@ -404,17 +410,19 @@ STATIC mp_obj_t wifi_radio_connect(size_t n_args, const mp_obj_t *pos_args, mp_m
404410

405411
mp_buffer_info_t password;
406412
password.len = 0;
407-
if (args[ARG_password].u_obj != MP_OBJ_NULL) {
413+
if (args[ARG_password].u_obj != mp_const_none) {
408414
mp_get_buffer_raise(args[ARG_password].u_obj, &password, MP_BUFFER_READ);
409-
mp_arg_validate_length_range(password.len, 8, 63, MP_QSTR_password);
415+
if (password.len != 0) {
416+
mp_arg_validate_length_range(password.len, 8, 63, MP_QSTR_password);
417+
}
410418
}
411419

412420
#define MAC_ADDRESS_LENGTH 6
413421

414422
mp_buffer_info_t bssid;
415423
bssid.len = 0;
416424
// Should probably make sure bssid is just bytes and not something else too
417-
if (args[ARG_bssid].u_obj != MP_OBJ_NULL) {
425+
if (args[ARG_bssid].u_obj != mp_const_none) {
418426
mp_get_buffer_raise(args[ARG_bssid].u_obj, &bssid, MP_BUFFER_READ);
419427
if (bssid.len != MAC_ADDRESS_LENGTH) {
420428
mp_raise_ValueError(translate("Invalid BSSID"));

shared-bindings/wifi/Radio.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ extern void common_hal_wifi_radio_stop_scanning_networks(wifi_radio_obj_t *self)
9191
extern void common_hal_wifi_radio_start_station(wifi_radio_obj_t *self);
9292
extern void common_hal_wifi_radio_stop_station(wifi_radio_obj_t *self);
9393

94-
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);
94+
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);
9595
extern void common_hal_wifi_radio_stop_ap(wifi_radio_obj_t *self);
9696

9797
extern void common_hal_wifi_radio_start_dhcp_client(wifi_radio_obj_t *self);

0 commit comments

Comments
 (0)