drivers/atwinc15x0: support dynamic scanning and connection to AP #19387
Conversation
fabian18
requested review from
leandrolanzieri,
jia200x,
MrKevinWeiss,
smlng,
aabadie,
MichelRottleuthner,
fjmolinas,
miri64,
haukepetersen,
PeterKietzmann and
cgundogan
as code owners
github-actions
bot
added
Area: build system
|
Thanks for contribution this feature. Some additional remarks:
|
|
BTW, we should have this feature also in our prime WiFi platform ESP32x 😉 |
|
Your eMail address looks a bit mangled - you want to run on that machine. |
| #define ATWINC15X0_FLAG_SCANNING (((atwinc15x0_flags_t)1) << 0) | ||
|
|
||
| /** | ||
| * @brief Flag to indicate that the transceiver is trying to connect to an AP | ||
| * at the moment and is thus not able to process another connect | ||
| * request concurrently | ||
| */ | ||
| #define ATWINC1510_FLAG_CONNECTING (((atwinc15x0_flags_t)1) << 1) | ||
|
|
||
| /** | ||
| * @brief Flag to indicate that the transceiver is connected to an AP | ||
| */ | ||
| #define ATWINC15X0_FLAG_CONNECTED (((atwinc15x0_flags_t)1) << 2) |
There was a problem hiding this comment.
Can those be set at the same time?
From a glance it feels like distinct states like
typedef enum {
ATWINC1510_STATE_DISCONNECTED,
ATWINC1510_STATE_DISCONNECTED_SCANNING,
ATWINC1510_STATE_CONNECTING,
ATWINC1510_STATE_CONNECTED,
ATWINC1510_STATE_CONNECTED_SCANNING,
} atwinc15x0_state_t;would be easier to handle.
There was a problem hiding this comment.
Since
if (IS_USED(MODULE_ATWINC15X0_STATIC_CONNECT)) {
/* try to connect and return */
res = _atwinc15x0_static_connect();
}
if (IS_USED(MODULE_ATWINC15X0_DYNAMIC_SCAN)) {
wifi_scan_request_t request = WIFI_SCAN_REQUEST_INITIALIZER(NETOPT_SCAN_REQ_ALL_CH,
_wifi_scan_cb, 0);
_atwinc15x0_scan(&request);
}did not work when both modules were enabled, ATWINC1510_STATE_CONNECTING_SCANNING would not be a valid state, so I think your state analysis is right. Maybe it requires one more is_connected check to distinguish between ATWINC1510_STATE_DISCONNECTED_SCANNING and ATWINC1510_STATE_CONNECTED_SCANNING, but I can try to reformulate it with states.
There was a problem hiding this comment.
I am trying to turn netopt_state_t state into atwinc15x0_state_t state in atwinc15x0_t. It is a bit more effort than I thought, to think about what operation can be performed in which state.
There was a problem hiding this comment.
I must still test a bit with the state handling
| */ | ||
| typedef struct netopt_connect_request { | ||
| netopt_on_connect_result_t conn_cb; /**< On connect callback */ | ||
| netopt_on_disconnect_result_t disconn_cb; /**< On disconnect callback */ |
There was a problem hiding this comment.
I see some redundancy with NETDEV_EVENT_LINK_UP/NETDEV_EVENT_LINK_DOWN with those - could #17902 be a generic alternative here?
On the other hand the only use case seems to be to make the shell commands block until completion.
There was a problem hiding this comment.
An alternative maybe, but not sure.
An improvement for interested threads who did not request the action definately!
Giving a callback is not enforced, and also I would not exclude the case to send a message in the callback.
If #17902 gets merged before, I would also try with this, but I´m not sure if both ways are mutually exclusive.
Thank you for the hint. I applied the command with my new email. |
In typedef enum {
M2M_WIFI_SEC_INVALID = 0,
/*!< Invalid security type.
*/
M2M_WIFI_SEC_OPEN,
/*!< Wi-Fi network is not secured.
*/
M2M_WIFI_SEC_WPA_PSK,
/*!< Wi-Fi network is secured with WPA/WPA2 personal(PSK).
*/
M2M_WIFI_SEC_WEP,
/*!< Security type WEP (40 or 104) OPEN OR SHARED.
*/
M2M_WIFI_SEC_802_1X
/*!< Wi-Fi network is secured with WPA/WPA2 Enterprise.IEEE802.1x user-name/password authentication.
*/
}tenuM2mSecType;#define M2M_1X_USR_NAME_MAX 21
/*!< The maximum size of the user name including the NULL termination.
It is used for RADIUS authentication in case of connecting the device to
an AP secured with WPA-Enterprise.
*/
#define M2M_1X_PWD_MAX 41
/*!< The maximum size of the password including the NULL termination.
It is used for RADIUS authentication in case of connecting the device to
an AP secured with WPA-Enterprise.
*//*!
@struct \
tstr1xAuthCredentials
@brief
Credentials for the user to authenticate with the AAA server (WPA-Enterprise Mode IEEE802.1x).
*/
typedef struct{
uint8 au8UserName[M2M_1X_USR_NAME_MAX];
/*!< User Name. It must be Null terminated string.
*/
uint8 au8Passwd[M2M_1X_PWD_MAX];
/*!< Password corresponding to the user name. It must be Null terminated string.
*/
}tstr1xAuthCredentials;/*!
@union \
tuniM2MWifiAuth
@brief
Wi-Fi Security Parameters for all supported security modes.
*/
typedef union{
uint8 au8PSK[M2M_MAX_PSK_LEN];
/*!< Pre-Shared Key in case of WPA-Personal security.
*/
tstr1xAuthCredentials strCred1x;
/*!< Credentials for RADIUS server authentication in case of WPA-Enterprise security.
*/
tstrM2mWifiWepParams strWepInfo;
/*!< WEP key parameters in case of WEP security.
*/
}tuniM2MWifiAuth;I am not so much familiar with WPA2 Enterprise but I have read that it can also deal with certificates and in your original PR you said you were having problems to connect to an Enterprise AP. I put the latest FW 19.7.7 on my module so maybe it works. I have not tried to connect to an Enterprise AP so far, but I think we have one in the office. |
|
@gschorcht I think I addressed your comments.
@benpicco I will try to change the flags to states and have a look at #17902 |
fabian18
force-pushed
the
drivers_atwinc1510_scan_result
branch
from
613499e to
c6ba3fe
Compare
|
Please squash. Use a shorter commit message for commit f932a3b, it exhausts the maximum of 72 characters. |
fabian18
force-pushed
the
drivers_atwinc1510_scan_result
branch
from
c6ba3fe to
283c206
Compare
|
Awesome, thanks! |
fabian18
added
the
CI: ready for build
fabian18
force-pushed
the
drivers_atwinc1510_scan_result
branch
from
283c206 to
63497a8
Compare
There are new pseudomodules for this driver: - atwinc15x0_static_connect: Should behave as before, by trying to connect to an AP by specified WIFI_SSIS and WIFI_PASS - atwinc15x0_dynamic_connect: takes connection request via NETOPT_CONNECT and provides the connection result via callback - atwinc15x0_dynamic_scan: takes network scan requests via NETOPT_SCAN and provides the scan result as a sorted list via callback
fabian18
force-pushed
the
drivers_atwinc1510_scan_result
branch
from
63497a8 to
8e122e9
Compare
|
@gschorcht are you fine with the force pushed changes which showed to be necessary from the Murdock run? |
Yes, I'm fine with squashing changes directly to make Murdock happy. |
gschorcht
added
Type: new feature
|
Maybe @benpicco can merge it together with other PRs that are already approved. I don't know whether setting the label |
|
The merge process is still very manual, re-triggering #19253 gives some time to write bors merge beneath all PRs that should be merged. The label is just to ease filtering for PRs to include in the merge train. |
19387: drivers/atwinc15x0: support dynamic scanning and connection to AP r=benpicco a=fabian18 19874: coap: add missing option numbers r=benpicco a=JKRhb 19875: coap: add missing Content-Format definitions r=benpicco a=JKRhb 19876: sys/net/ipv4/addr: fix typos r=benpicco a=Enoch247 ### Contribution description This patch fixes some typos in the doxygen doc. ### Testing procedure Nothing to test. No change to code. ### Issues/PRs references - None known 19878: makefiles/usb_board_reset.mk: declare term-delay target with test target r=benpicco a=aabadie 19886: cpu/efm32: fix DAC configuration r=benpicco a=gschorcht ### Contribution description The EFM32 MCU allows the reference voltage to be configured per DAC device, not per DAC channel. Also, the DAC reference voltage was defined in the configuration but not used anywhere. At the moment we have only defined one board (`stwstk6220a`) that uses the DAC, so changing the configuration interface shouldn't be critical. ### Testing procedure `tests/periph/dac` should still work for the `stwstk6220a` ``` BOARD=slwstk6220a make -j8 -C tests/periph/dac flash ``` I don't have a `stwstk6220a` board (EFM32 Series 0) so that I can't test it. I could only test it for the `sltb009a` board (EFM32 Series 1) with the change for VDAC in PR #19887. ### Issues/PRs references 19888: boards/sltb009a: complete and fix documentation r=benpicco a=gschorcht ### Contribution description This PR completes and fixes the documentation which was still in the state as generated automatically by `efm2riot`. The PR also includes a fix of the configuration of the second UART device that was find out while completing the documentation. ### Testing procedure Green CI ### Issues/PRs references Co-authored-by: Fabian Hüßler <[email protected]@MLPA-NB119.(none)> Co-authored-by: Fabian Hüßler <[email protected]> Co-authored-by: Jan Romann <[email protected]> Co-authored-by: Joshua DeWeese <[email protected]> Co-authored-by: Alexandre Abadie <[email protected]> Co-authored-by: Gunar Schorcht <[email protected]>
|
Build failed (retrying...): |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Contribution description
We currently only support static Wi-Fi credentials using
WIFI_SSIDandWIFI_PASS.This PR implements a way of dynamic scanning and connecting to Access Points using new netopts
NETOPT_SCAN,NETOPT_CONNECT, andNETOPT_DISCONNECT.atwinc15x0driver is adapted to support these options.atwinc15x0_static_connectcan be activated to show the old behaviouratwinc15x0_dynamic_connectcan be activated to support dynamic connection requestsatwinc15x0_dynamic_scancan be activated to support dynamic scanningYou can enable whatever pseudomodules you want or even none.
A
l2scan_listdata structure is added to deliver scan results in a list, sorted by their RSSI.A new shell command
iwis added as an interface to interact with Wi-Fi interfacesTesting procedure
Test
l2scan_list:Test:
gnrc_networking:My wrapping Makefile:
Makefile_same54_xpro.mk
Try out
iwcommand.Test:
gnrc_border routerMy wrapping Makefile:
Makefile_same54_xpro.mk
Somehow the uplink does not configure a link-local address.
I will investigate on this problem.
Issues/PRs references