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. Ideally non hidden AP should not be autoconnected when hidden AP connection is in progress. This patch blocks wifi autoconnection when hidden connection is in progress and unblocks autoconnection after hidden connection is started or hidden wifi scan is completed and hidden AP is not found in the scan list. --- src/connman.h | 2 ++ src/network.c | 4 ++++ src/service.c | 13 ++++++++++++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/connman.h b/src/connman.h index ce3ef8d..24461b1 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_set_wifi_autoconnect(bool autoconnect); +

#include <connman/peer.h> int __connman_peer_init(void); diff --git a/src/network.c b/src/network.c index 2e423bc..eccb2ef 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_set_wifi_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_set_wifi_autoconnect(true); + if (!service) return -EINVAL; diff --git a/src/service.c b/src/service.c index f6a76f6..5fdff4b 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 wifi_autoconnect = true; struct connman_stats { bool valid; @@ -3764,6 +3765,9 @@ static bool auto_connect_service(GList *services, ignore[CONNMAN_SERVICE_TYPE_VPN] = true; + if (!wifi_autoconnect) + ignore[CONNMAN_SERVICE_TYPE_WIFI] = true;

+ for (list = services; list; list = list->next) { service = list->data; @@ -5921,12 +5925,19 @@ static void prepare_8021x(struct connman_service *service) service->phase2); } +void __connman_set_wifi_autoconnect(bool autoconnect) +{ + wifi_autoconnect = autoconnect; +} + static int service_connect(struct connman_service *service) { int err; - if (service->hidden) + if (service->hidden) { + wifi_autoconnect = false; return -EPERM; + } switch (service->type) { case CONNMAN_SERVICE_TYPE_UNKNOWN:

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;

> 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);

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?

> 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. > Ideally non hidden AP should not be autoconnected when hidden AP connection > is in progress. > This patch blocks wifi autoconnection when hidden connection is in > progress and unblocks autoconnection after hidden connection is started > or hidden wifi scan is completed and hidden AP is not found in the scan list. > --- > src/connman.h | 2 ++ > src/network.c | 4 ++++ > src/service.c | 13 ++++++++++++- > 3 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/src/connman.h b/src/connman.h > index ce3ef8d..24461b1 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_set_wifi_autoconnect(bool autoconnect); > +

Since this function belongs to the service internal API it should have the prefix __connman_service. Furthermore, I rather have a textual hint what it does. Usually a plain bool is easy to get wrong. So what about

__connman_service_{enable|disable}_autoconnect()

?

Something sligthly more hacky would be to overload __connman_service_auto_connect() with a reason code like CONNMAN_SERVICE_CONNECT_REASON_BLOCK. But that is also a bit ugly. Maybe the above is better.

> #include <connman/peer.h> > > int __connman_peer_init(void); > diff --git a/src/network.c b/src/network.c > index 2e423bc..eccb2ef 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_set_wifi_autoconnect(true); > +

Shouldn't that be a false?

> /* > * 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_set_wifi_autoconnect(true); > + > if (!service) > return -EINVAL; > > diff --git a/src/service.c b/src/service.c > index f6a76f6..5fdff4b 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 wifi_autoconnect = true; > > struct connman_stats { > bool valid; > @@ -3764,6 +3765,9 @@ static bool auto_connect_service(GList *services, > > ignore[CONNMAN_SERVICE_TYPE_VPN] = true; > > + if (!wifi_autoconnect) > + ignore[CONNMAN_SERVICE_TYPE_WIFI] = true;

What about the other service types. With your patch we would still try to auto connect anything besides wifi. I think you need to be very careful here. That is pretty tricky code and this looks like it is going to do funny things.

Maybe you should just block the auto connect requests and unblock it later. So don't through them away. Why? Because it might be that in the meantime when you try to connect a hidden wifi network we have a different source for calling auto connect. If you just block it and unblock you don't regress anything.

Yeah, better would be to this not in auto_connect_service() instead in __connman_service_auto_connect().

> + > for (list = services; list; list = list->next) { > service = list->data; > > @@ -5921,12 +5925,19 @@ static void prepare_8021x(struct connman_service *service) > service->phase2); > } > > +void __connman_set_wifi_autoconnect(bool autoconnect) > +{ > + wifi_autoconnect = autoconnect; > +} > + > static int service_connect(struct connman_service *service) > { > int err; > > - if (service->hidden) > + if (service->hidden) { > + wifi_autoconnect = false; > return -EPERM; > + } > > switch (service->type) { > case CONNMAN_SERVICE_TYPE_UNKNOWN: >