[PATCH] Add proper messages for Android network manager

[Dmitry Shmidt]

On Fri, Feb 25, 2011 at 10:24 AM, Dmitry Shmidt <dimitrysh at google.com> wrote:
> On Fri, Feb 25, 2011 at 6:14 AM, Jouni Malinen <j at w1.fi> wrote:
>> On Thu, Feb 24, 2011 at 06:01:26PM -0800, Dmitry Shmidt wrote:
>>> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
>>> @@ -530,7 +530,14 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
>>> +#ifdef ANDROID
>>> + ? ? int network_id = -1;
>>> +
>>> + ? ? if (wpa_s && wpa_s->current_ssid)
>>> + ? ? ? ? ? ? network_id = wpa_s->current_ssid->id;
>>> + ? ? wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_STATE_CHANGE "id=%d state=%d
>>> BSSID=" MACSTR,
>>> + ? ? ? ? ? ? network_id, state, MAC2STR(wpa_s->pending_bssid));
>>> +#endif
>>
>> I would prefer to add this into wpas_notify_state_changed() instead of
>> here. Do you want this to show up in debug log, too, or would
>> wpa_msg_ctrl() be more appropriate here? In addition, I'm not completely
>> sure about the use of pending_bssid here.. What is that BSSID value used
>> for? Please note that pending_bssid may not be the current BSSID.
>>
>
> You are right, it fits much better to wpas_notify_state_changed().
> And yes, I realize that pending_bssid is not the current one, but
> Network manager is using
> it sometimes. I am pretty sure we can use your message.

We do need pending_bssid. It has a correct value when state is changed
to COMPLETED
and network manager is tracking it for its purposes.

>
>>
>>> @@ -2501,6 +2508,9 @@ void wpa_supplicant_deinit(struct wpa_global *global)
>>> ? ? ? if (global == NULL)
>>> ? ? ? ? ? ? ? return;
>>>
>>> +#ifdef ANDROID
>>> + ? ? wpa_supplicant_terminate(0, global);
>>> +#endif

wpa_supplicant_deinit() is called when supplicant is ending "by
itself" for some reason.
We are calling wpa_supplicant_terminate(0, global); mostly to provide
WPA_EVENT_TERMINATING message to the wifi manger.

>>
>> Surely this does not belong to this patch. What is the goal of this
>> change?
>>
>> As far as the WPA_EVENT_STATE_CHANGE notification is concerned, I would
>> use something like this:
>>
>>
>> diff --git a/src/common/wpa_ctrl.h b/src/common/wpa_ctrl.h
>> index 86653a2..528cc16 100644
>> --- a/src/common/wpa_ctrl.h
>> +++ b/src/common/wpa_ctrl.h
>> @@ -56,6 +56,8 @@ extern "C" {
>> ?#define WPA_EVENT_EAP_FAILURE "CTRL-EVENT-EAP-FAILURE "
>> ?/** New scan results available */
>> ?#define WPA_EVENT_SCAN_RESULTS "CTRL-EVENT-SCAN-RESULTS "
>> +/** wpa_supplicant state change */
>> +#define WPA_EVENT_STATE_CHANGE "CTRL-EVENT-STATE-CHANGE "
>> ?/** A new BSS entry was added (followed by BSS entry id and BSSID) */
>> ?#define WPA_EVENT_BSS_ADDED "CTRL-EVENT-BSS-ADDED "
>> ?/** A BSS entry was removed (followed by BSS entry id and BSSID) */
>> diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
>> index 4daffd8..5e3aaa0 100644
>> --- a/wpa_supplicant/notify.c
>> +++ b/wpa_supplicant/notify.c
>> @@ -92,6 +92,13 @@ void wpas_notify_state_changed(struct wpa_supplicant *wpa_s,
>> ?#endif /* CONFIG_P2P */
>>
>> ? ? ? ?sme_state_changed(wpa_s);
>> +
>> +#ifdef ANDROID
>> + ? ? ? wpa_msg_ctrl(wpa_s, MSG_INFO, WPA_EVENT_STATE_CHANGE
>> + ? ? ? ? ? ? ? ? ? ?"id=%d state=%d BSSID=" MACSTR,
>> + ? ? ? ? ? ? ? ? ? ?wpa_s->current_ssid ? wpa_s->current_ssid->id : -1,
>> + ? ? ? ? ? ? ? ? ? ?new_state, MAC2STR(wpa_s->pending_bssid));
>> +#endif /* ANDROID */
>> ?}
>
> Works for me. Thanks !
>
>>
>> --
>> Jouni Malinen ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?PGP id EFC895FA
>> _______________________________________________
>> HostAP mailing list
>> HostAP at lists.shmoo.com
>> http://lists.shmoo.com/mailman/listinfo/hostap
>>
>