Hi Patrik,
Resending my observation, please suggest
> In the following scenario:
> 1. Device is connected to AP in non hidden mode.
> 2. Connection is initated to AP in hidden mode.
> connman disconnects old service and tries to connect to new service.
> But
> for connecting hidden service it first scans the hidden AP and when
> network is added for hidden AP then connection is tried. In the
> meantime
> normal AP got disconnected and was tried to autoconnect even before
> hidden AP was scanned successfully.
> This patch blocks auto connect when any service connection is in
> progress and unblocks autoconnection after connection is started
> or hidden wifi scan is completed and hidden AP is not found in the
> scan list.
> ---
>
> v3: Defer auto connect of services when one service is connecting
> Allow auto connect of services when service connection fails with
> error
>
>
> src/connman.h | 2 ++
> src/network.c | 4 ++++
> src/service.c | 22 ++++++++++++++++++++--
> 3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/src/connman.h b/src/connman.h
> index ce3ef8d..945518b 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -796,6 +796,8 @@ void __connman_service_notify(struct
> connman_service *service,
> int __connman_service_counter_register(const char *counter);
> void __connman_service_counter_unregister(const char *counter);
>
> +void __connman_service_enable_autoconnect(bool autoconnect);
> +
> #include <connman/peer.h>
>
> int __connman_peer_init(void);
> diff --git a/src/network.c b/src/network.c
> index 2e423bc..e24970d 100644
> --- a/src/network.c
> +++ b/src/network.c
> @@ -1370,6 +1370,8 @@ void connman_network_clear_hidden(void
> *user_data)
>
> DBG("user_data %p", user_data);
>
> + __connman_service_enable_autoconnect(true);
> +
> /*
> * Hidden service does not have a connect timeout so
> * we do not need to remove it. We can just return
> @@ -1389,6 +1391,8 @@ int connman_network_connect_hidden(struct
> connman_network *network,
>
> DBG("network %p service %p user_data %p", network, service,
> user_data);
>
> + __connman_service_enable_autoconnect(true);
> +
> if (!service)
> return -EINVAL;
>
> diff --git a/src/service.c b/src/service.c
> index f6a76f6..316581a 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -48,6 +48,7 @@ static unsigned int autoconnect_timeout = 0;
> static unsigned int vpn_autoconnect_timeout = 0;
> static struct connman_service *current_default = NULL;
> static bool services_dirty = false;
> +static bool allow_autoconnect = true;
>
> struct connman_stats {
> bool valid;
> @@ -3849,6 +3850,9 @@ void __connman_service_auto_connect(enum
> connman_service_connect_reason reason)
> if (autoconnect_timeout != 0)
> return;
>
> + if (!allow_autoconnect)
> + return;
> +
> if (!__connman_session_policy_autoconnect(reason))
> return;
>
> @@ -4025,6 +4029,8 @@ static DBusMessage
> *connect_service(DBusConnection *conn,
>
> index = __connman_service_get_index(service);
>
> + allow_autoconnect = false;
> +
> for (list = service_list; list; list = list->next) {
> struct connman_service *temp = list->data;
>
> @@ -4042,8 +4048,10 @@ static DBusMessage
> *connect_service(DBusConnection *conn,
> err = -EINPROGRESS;
>
> }
> - if (err == -EINPROGRESS)
> + if (err == -EINPROGRESS) {
> + allow_autoconnect = true;
> return __connman_error_operation_timeout(msg);
> + }
>
> service->ignore = false;
>
> @@ -4060,8 +4068,10 @@ static DBusMessage
> *connect_service(DBusConnection *conn,
> service->pending = NULL;
> }
>
> - if (err < 0)
> + if (err < 0) {
> + allow_autoconnect = true;
> return __connman_error_failed(msg, -err);
> + }
>
> return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> }
> @@ -5231,6 +5241,9 @@ static void request_input_cb(struct
> connman_service *service,
> service_complete(service);
> __connman_connection_update_gateway();
> }
> +
> + if (!service->hidden)
> + allow_autoconnect = true;
> }
>
> static void downgrade_connected_services(void)
> @@ -5921,6 +5934,11 @@ static void prepare_8021x(struct
> connman_service *service)
> service-
> >phase2);
> }
>
> +void __connman_service_enable_autoconnect(bool autoconnect)
> +{
> + allow_autoconnect = autoconnect;
> +}
> +
> static int service_connect(struct connman_service *service)
> {
> int err;
This enabling and disabling of autoconnect looks a bit complicated.
When the previous service is being disconnected, does the disconnection
go like this:
- src/service.c, 6132, err = service_connect(service);
- src/service.c, 6072, err = __connman_network_connect(service->network);
- src/network.c, 1446, __connman_device_disconnect(network->device);
- src/device.c, 645, __connman_network_disconnect(network);
Currently disconnection goes like this:
- src/service.c, 4093, if (__connman_service_get_index(temp) == index &&
__connman_service_disconnect(temp) == -EINPROGRESS)
- src/service.c, 6198, err = __connman_network_disconnect(service->network);
So in this case it should be the plugin itself that needs to decide
whether and when another network needs to be disconnected? That way it
can keep the network connected until it finds the hidden network it is
looking for?
With that the call to __connman_network_disconnect() could then be
removed, but needs to move into the plugins? Or only look out for
hidden WiFis?
I agree it is a good approach to allow plugins to disconnect
network when connection is started. This will require to remove
both the disconnect call by src/service.c when connection
is started.
We can add a check in network_connect() function of
plugins/wifi.c like below:
if (wifi->connected)
g_supplicant_interface_disconnect(wifi->interface, disconnect_callback, wifi);
In case of hidden WiFis also above change would disconnect current WiFi only
when hidden WiFi is scanned and its connection is initiated.
Thanks,
Saurav