Hi,
On Thu, Aug 24, 2017 at 7:33 PM, Jonah Petri <jonah(a)sense.com> wrote:
> The active scan is useful in order to perform a fast scan looking
for the WiFi networks we have got connected in the past. I think this is particularly
helpful to speed up auto-connect procedure at the start-up of the system. Instead, when
user asks for scanning I think it should always be a passive scan because users want to
see all WiFi networks available in range. Therefore, I propose to make ConnMan ask
wpa_supplicant for an active scan only at star-up of the system and passive scan when it
is directly asked from users through a Technology.Scan() D-Bus call. Both things no matter
BackgroundScanning's value.
>
> What do all you think?
>
(Now it was my turn to be on vacation! Sorry for the delay.)
Don't worry, welcome back!
Great idea. This is fine for my use case, and the behaviour you
propose is what I expected given what Technology.Scan() claims to do.
Well, the scan algorithm is quite complex but I think I finally
understood how it works and there is something we should change from
this idea.
We can resume our scenario as following:
* WiFi Driver support active scanning, it means "MaxScanSSID" is greater than
1.
* User has successfully connected to at least one WiFi network in the
past so the service's information is present in
{STOREDIR}/connman/wifi_* but none of those networks are currently in
range, let's call them "known-services".
Under these assumptions, when user asks for scanning the algorithm
will perform an active scan looking for those known-services in their
specific frequencies, this kind of scanning is generally faster than a
passive one which allows to speed up auto-connection procedure.
However, we have seen that some drivers reply with all the networks in
the specified frequencies and some other does not reply any network.
From this point forward, the behaviour changes based on
"BackgroundScanning" value:
* Enabled: A simple backoff mechanism starting from 10s up to 5
minutes will run. Of course, using passive scan thus at the end driver
will reply with all networks in range, as user expects after scanning.
* Disabled: Nothing else is done. Any additional scan will be
executed, resulting in a possible empty scan results forever, Johan's
case.
In my initial idea I proposed to perform passive scan, no matter the
BackgroundScanning value, when user performs a Scan() D-Bus call to
ensure all networks are found. However, while trying to implement it
and due to the reasons I explained before, I realized that it could
also increase the auto-connection procedure with the default ConnMan's
configuration, i.e. BackgroundScanning enabled. Therefore, I am now
limiting this change to users who manually disabled
BackgroundScanning.
Patrik, Daniel, what do you think?
Jonah, please test this patch:
diff --git a/include/device.h b/include/device.h
index 9ac800a..5375a3a 100644
--- a/include/device.h
+++ b/include/device.h
@@ -120,7 +120,7 @@ struct connman_device_driver {
int (*enable) (struct connman_device *device);
int (*disable) (struct connman_device *device);
int (*scan)(enum connman_service_type type,
- struct connman_device *device,
+ struct connman_device *device, bool requested,
const char *ssid, unsigned int ssid_len,
const char *identity, const char* passphrase,
const char *security, void *user_data);
diff --git a/plugins/wifi.c b/plugins/wifi.c
index 34c16df..0b3ee28 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -1784,7 +1784,7 @@ static int p2p_find(struct connman_device *device)
* hidden AP first time. It is not used when system autoconnects to hidden AP.
*/
static int wifi_scan(enum connman_service_type type,
- struct connman_device *device,
+ struct connman_device *device, bool requested,
const char *ssid, unsigned int ssid_len,
const char *identity, const char* passphrase,
const char *security, void *user_data)
@@ -1876,7 +1876,8 @@ static int wifi_scan(enum connman_service_type type,
return 0;
}
- } else if (wifi->connected) {
+ } else if ((requested &&
!connman_setting_get_bool("BackgroundScanning")) ||
+ wifi->connected) {
g_supplicant_free_scan_params(scan_params);
return wifi_scan_simple(device);
} else {
diff --git a/src/device.c b/src/device.c
index a563f46..8bdf828 100644
--- a/src/device.c
+++ b/src/device.c
@@ -591,7 +591,7 @@ int connman_device_set_powered(struct
connman_device *device,
if (device->driver && device->driver->scan)
device->driver->scan(CONNMAN_SERVICE_TYPE_UNKNOWN, device,
- NULL, 0, NULL, NULL, NULL, NULL);
+ false, NULL, 0, NULL, NULL, NULL, NULL);
return 0;
}
@@ -610,7 +610,7 @@ static int device_scan(enum connman_service_type type,
if (!device->powered)
return -ENOLINK;
- return device->driver->scan(type, device, NULL, 0,
+ return device->driver->scan(type, device, true, NULL, 0,
NULL, NULL, NULL, NULL);
}
@@ -1104,7 +1104,7 @@ int __connman_device_request_hidden_scan(struct
connman_device *device,
return -EINVAL;
return device->driver->scan(CONNMAN_SERVICE_TYPE_UNKNOWN,
- device, ssid, ssid_len, identity,
+ device, false, ssid, ssid_len, identity,
passphrase, security, user_data);
}
Best regards,
Jose Blanquicet