[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
>>
>