zephyr: Update SoftAP event functions of Zephyr with hostapd enabled #72
Conversation
nxf58150
force-pushed
the
hostapd_split
branch
from
fdd8641 to
b7f65d9
Compare
nxf58150
force-pushed
the
hostapd_split
branch
from
b7f65d9 to
536ef15
Compare
| supplicant_send_wifi_mgmt_ap_status(iface, | ||
| NET_EVENT_WIFI_CMD_AP_ENABLE_RESULT, | ||
| WIFI_STATUS_AP_SUCCESS); | ||
| #endif /* __ZEPHYR__ && CONFIG_WIFI_NM_HOSTAPD_AP */ | ||
| #endif /* CONFIG_WIFI_NM_HOSTAPD_AP */ | ||
| #endif /* __ZEPHYR__ */ |
There was a problem hiding this comment.
There is something nasty here. iface is a struct hostapd_iface *. However, supplicant_send_wifi_mgmt_ap_status() cast it in struct wpa_supplicant *.
In addition, this compile time condition prevent to choose AP or supplicant mode during runtime (even if it not yet supported, we probably want to do it in the future).
There was a problem hiding this comment.
This is for softAP with hostapd only so I removed function call of supplicant_send_wifi_mgmt_ap_status() here.
I think this PR is targeting on hostapd support split. For choosing mode at runtime, more changes could be made in the future once the feature is required and have detailed plan.
Thanks!
| #else | ||
| supplicant_send_wifi_mgmt_ap_sta_event(hapd->iface->owner, | ||
| event, | ||
| sta); | ||
| #endif | ||
| #endif /* CONFIG_WIFI_NM_HOSTAPD_AP */ | ||
| #endif /* __ZEPHYR__ */ |
There was a problem hiding this comment.
Ditto. I believe we need to evaluate this condition during runtime:
if (IS_ENABLED(CONFIG_WIFI_NM_HOSTAPD_AP) && is_softap_running) {
hostapd_send_wifi_mgmt_ap_sta_event(...);
} else if (IS_ENABLED(CONFIG_WIFI_NM_WPA_SUPPLICANT) && !is_softap_running) {
supplicant_send_wifi_mgmt_ap_sta_event(...);
} else {
/* Panic! */
}
BTW, I feel this code should be relocated in zephyr.
There was a problem hiding this comment.
Same as previous reply, for choosing mode at runtime, more changes could be made in the future once the feature is required and have detailed plan.
Thanks!
There was a problem hiding this comment.
I believe my comment is still relevant even if is_softap_running does not exist:
if (IS_ENABLED(CONFIG_WIFI_NM_HOSTAPD_AP)) {
hostapd_send_wifi_mgmt_ap_sta_event(...);
} else {
supplicant_send_wifi_mgmt_ap_sta_event(...);
}
(In order to compile this, you have to include hapd_events.h and supp_events.h unconditionally).
BTW, rather than duplicating this pattern everywhere in hostapd, isn't it possible to locate it in one/two functions in the Zephyr side?
|
ping @nxf58150 could you address the comments from @jerome-pouiller |
Just back to office from Chinese New Year Holiday. Will update PR soon. Thanks! |
Update SoftAP event functions of Zephyr with hostapd enabled to support hostapd split in glue layer. Signed-off-by: Hui Bai <[email protected]>
nxf58150
force-pushed
the
hostapd_split
branch
from
536ef15 to
1fd1cad
Compare
| #include <supp_events.h> | ||
| #endif /* CONFIG_WIFI_NM_HOSTAPD_AP */ |
There was a problem hiding this comment.
Is supp_events.h still required?
| #else | ||
| supplicant_send_wifi_mgmt_ap_sta_event(hapd->iface->owner, | ||
| event, | ||
| sta); | ||
| #endif | ||
| #endif /* CONFIG_WIFI_NM_HOSTAPD_AP */ | ||
| #endif /* __ZEPHYR__ */ |
There was a problem hiding this comment.
I believe my comment is still relevant even if is_softap_running does not exist:
if (IS_ENABLED(CONFIG_WIFI_NM_HOSTAPD_AP)) {
hostapd_send_wifi_mgmt_ap_sta_event(...);
} else {
supplicant_send_wifi_mgmt_ap_sta_event(...);
}
(In order to compile this, you have to include hapd_events.h and supp_events.h unconditionally).
BTW, rather than duplicating this pattern everywhere in hostapd, isn't it possible to locate it in one/two functions in the Zephyr side?
Update SoftAP event functions of Zephyr with hostapd enabled.