From: Luiz Augusto von Dentz <[email protected]>
This enables the client to set its discoverable setting while
discovering which is very typical situation as usually the setings
application would allow incoming pairing request while scanning, so
this would reduce the number of calls setting Discoverable and
DiscoverableTimeout and restoring after done with discovery.
---
doc/adapter-api.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index d14d0ca50..4791af2c7 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -113,6 +113,12 @@ Methods void StartDiscovery()
generated for either ManufacturerData and
ServiceData everytime they are discovered.
+ bool Discoverable (Default: false)
+
+ Make adapter discoverable while discovering,
+ if the adapter is already discoverable this
+ setting this filter won't do anything.
+
When discovery filter is set, Device objects will be
created as new devices with matching criteria are
discovered regardless of they are connectable or
--
2.17.1
Hi Marcel,
On Mon, Jul 30, 2018 at 2:09 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Luiz,
>
>> This enables the client to set its discoverable setting while
>> discovering which is very typical situation as usually the setings
>> application would allow incoming pairing request while scanning, so
>> this would reduce the number of calls setting Discoverable and
>> DiscoverableTimeout and restoring after done with discovery.
>> ---
>> doc/adapter-api.txt | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
>> index d14d0ca50..4791af2c7 100644
>> --- a/doc/adapter-api.txt
>> +++ b/doc/adapter-api.txt
>> @@ -113,6 +113,12 @@ Methods void StartDiscovery()
>> generated for either ManufacturerData and
>> ServiceData everytime they are discovered.
>>
>> + bool Discoverable (Default: false)
>> +
>> + Make adapter discoverable while discoverin=
g,
>> + if the adapter is already discoverable thi=
s
>> + setting this filter won't do anything.
>> +
>> When discovery filter is set, Device objects will =
be
>> created as new devices with matching criteria are
>> discovered regardless of they are connectable or
>
> I am now regretting that we didn=E2=80=99t add a uint32 Flags parameter t=
o Start Service Discovery mgmt command if this is something that is often d=
one. Letting the kernel coordinate this properly for the lifetime of the di=
scovery would be best. Otherwise things start to get racy eventually. Meani=
ng that when implementing this in bluetoothd, we need to be careful to hand=
le other mgmt commands correctly that might be issued from non-bluetoothd a=
pplications.
Yep, though StartDiscovery does continuous discovery while the mgmt
don't have that so I suppose if we do want to extend the mgmt
interface to add such flags we could perhaps add a flag to do it
continuously otherwise we would be toggling the setting on every
discovery window which is imo not very nice. Also should this perhaps
be extended to support add advertising into the picture, I guess it
should since discovery filters are for both LE and BR/EDR.
We anyway need a new command for extended scanning so we can have a
PHY filter, or we do intended to have the PHY as address_type? I guess
not so that means we could adds this sort of filter when do add PHY
filtering, though we still need to support old kernels so Im afraid we
will have do something in bluetoothd no matter what.
> Regards
>
> Marcel
>
--=20
Luiz Augusto von Dentz
Hi Luiz,
> This enables the client to set its discoverable setting while
> discovering which is very typical situation as usually the setings
> application would allow incoming pairing request while scanning, so
> this would reduce the number of calls setting Discoverable and
> DiscoverableTimeout and restoring after done with discovery.
> ---
> doc/adapter-api.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index d14d0ca50..4791af2c7 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -113,6 +113,12 @@ Methods void StartDiscovery()
> generated for either ManufacturerData and
> ServiceData everytime they are discovered.
>
> + bool Discoverable (Default: false)
> +
> + Make adapter discoverable while discovering,
> + if the adapter is already discoverable this
> + setting this filter won't do anything.
> +
> When discovery filter is set, Device objects will be
> created as new devices with matching criteria are
> discovered regardless of they are connectable or
I am now regretting that we didn’t add a uint32 Flags parameter to Start Service Discovery mgmt command if this is something that is often done. Letting the kernel coordinate this properly for the lifetime of the discovery would be best. Otherwise things start to get racy eventually. Meaning that when implementing this in bluetoothd, we need to be careful to handle other mgmt commands correctly that might be issued from non-bluetoothd applications.
Regards
Marcel
Hi Bastien,
On Mon, Jul 30, 2018 at 1:21 PM, Bastien Nocera <[email protected]> wrote:
> On Mon, 2018-07-30 at 09:44 +0300, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> This adds discoverable command to scan menu which can be used to set
>> if adapter should become discoverable while scanning:
>>
>> [bluetooth]# scan.discoverable on
>> [bluetooth]# scan on
>> SetDiscoveryFilter success
>> [CHG] Controller XX:XX:XX:XX:XX:XX Discoverable: yes
>> Discovery started
>> [CHG] Controller XX:XX:XX:XX:XX:XX Discovering: yes
>> [bluetooth]# scan off
>> Discovery stopped
>> [CHG] Controller XX:XX:XX:XX:XX:XX Discoverable: no
>
> This doesn't call SetDiscoverFilter() when the scan as already started,
> meaning that my earlier test still fails.
>
>> As mentioned on IRC, this does not work if the Discoverable filter is
>> set after the discovery has started. So:
>> discoverable off
>> scan on
>> scan.discoverable on
>> show <- this will show "Discoverable" still off.
>
> Doing a quick "scan off" / "scan on" will show "SetDiscoveryFilter
> success" appearing, this should also appear when the filter is changed
> while scanning is on-going.
>
> If you don't want to change the currently filter, it might be good for
> the client to show a "Filter change queued" or similar message that
> would show that the setting hasn't been applied, but would be for the
> next run.
Yep, I can add a message that it would be set when the scan if
enabled, in fact I don't think it is a good practice to change things
like discoverable in the middle of the discovery since may interfere
with what the controller is scheduling over the air, but since we
allow thing like UUID and RSSI it makes sense to allow everything to
be changed.
--
Luiz Augusto von Dentz
On Mon, 2018-07-30 at 09:44 +0300, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds discoverable command to scan menu which can be used to set
> if adapter should become discoverable while scanning:
>
> [bluetooth]# scan.discoverable on
> [bluetooth]# scan on
> SetDiscoveryFilter success
> [CHG] Controller XX:XX:XX:XX:XX:XX Discoverable: yes
> Discovery started
> [CHG] Controller XX:XX:XX:XX:XX:XX Discovering: yes
> [bluetooth]# scan off
> Discovery stopped
> [CHG] Controller XX:XX:XX:XX:XX:XX Discoverable: no
This doesn't call SetDiscoverFilter() when the scan as already started,
meaning that my earlier test still fails.
> As mentioned on IRC, this does not work if the Discoverable filter is
> set after the discovery has started. So:
> discoverable off
> scan on
> scan.discoverable on
> show <- this will show "Discoverable" still off.
Doing a quick "scan off" / "scan on" will show "SetDiscoveryFilter
success" appearing, this should also appear when the filter is changed
while scanning is on-going.
If you don't want to change the currently filter, it might be good for
the client to show a "Filter change queued" or similar message that
would show that the setting hasn't been applied, but would be for the
next run.
Cheers
On Mon, 2018-07-30 at 09:44 +0300, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This enables the client to set its discoverable setting while
> discovering which is very typical situation as usually the setings
> application would allow incoming pairing request while scanning, so
> this would reduce the number of calls setting Discoverable and
> DiscoverableTimeout and restoring after done with discovery.
This seems to take into account the changes in filters (though
SetDiscoveryFilter) that happen while discovery is running, so the
gnome-bluetooth branch works as expected:
https://gitlab.gnome.org/GNOME/gnome-bluetooth/merge_requests/1
There still might be a problem in a later patch for bluetoothctl.
From: Luiz Augusto von Dentz <[email protected]>
If the discovery has been stopped and the client has set filters those
should be put back into filter list since the client may still be
interested in using them the next time it start a scanning.
---
src/adapter.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 4227718b9..92708b4f4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1858,7 +1858,7 @@ static bool set_discovery_discoverable(struct btd_adapter *adapter, bool enable)
return set_discoverable(adapter, enable, 0);
}
-static void discovery_remove(struct watch_client *client)
+static void discovery_remove(struct watch_client *client, bool exit)
{
struct btd_adapter *adapter = client->adapter;
@@ -1870,7 +1870,11 @@ static void discovery_remove(struct watch_client *client)
adapter->discovery_list = g_slist_remove(adapter->discovery_list,
client);
- discovery_free(client);
+ if (!exit && client->discovery_filter)
+ adapter->set_filter_list = g_slist_prepend(
+ adapter->set_filter_list, client);
+ else
+ discovery_free(client);
/*
* If there are other client discoveries in progress, then leave
@@ -1899,8 +1903,11 @@ static void stop_discovery_complete(uint8_t status, uint16_t length,
goto done;
}
- if (client->msg)
+ if (client->msg) {
g_dbus_send_reply(dbus_conn, client->msg, DBUS_TYPE_INVALID);
+ dbus_message_unref(client->msg);
+ client->msg = NULL;
+ }
adapter->discovery_type = 0x00;
adapter->discovery_enable = 0x00;
@@ -1913,7 +1920,7 @@ static void stop_discovery_complete(uint8_t status, uint16_t length,
trigger_passive_scanning(adapter);
done:
- discovery_remove(client);
+ discovery_remove(client, false);
}
static int compare_sender(gconstpointer a, gconstpointer b)
@@ -2148,14 +2155,14 @@ static int update_discovery_filter(struct btd_adapter *adapter)
return -EINPROGRESS;
}
-static int discovery_stop(struct watch_client *client)
+static int discovery_stop(struct watch_client *client, bool exit)
{
struct btd_adapter *adapter = client->adapter;
struct mgmt_cp_stop_discovery cp;
/* Check if there are more client discovering */
if (g_slist_next(adapter->discovery_list)) {
- discovery_remove(client);
+ discovery_remove(client, exit);
update_discovery_filter(adapter);
return 0;
}
@@ -2168,7 +2175,7 @@ static int discovery_stop(struct watch_client *client)
* and so it is enough to send out the signal and just return.
*/
if (adapter->discovery_enable == 0x00) {
- discovery_remove(client);
+ discovery_remove(client, exit);
adapter->discovering = false;
g_dbus_emit_property_changed(dbus_conn, adapter->path,
ADAPTER_INTERFACE, "Discovering");
@@ -2193,7 +2200,7 @@ static void discovery_disconnect(DBusConnection *conn, void *user_data)
DBG("owner %s", client->owner);
- discovery_stop(client);
+ discovery_stop(client, true);
}
/*
@@ -2583,7 +2590,7 @@ static DBusMessage *stop_discovery(DBusConnection *conn,
if (client->msg)
return btd_error_busy(msg);
- err = discovery_stop(client);
+ err = discovery_stop(client, false);
switch (err) {
case 0:
return dbus_message_new_method_return(msg);
--
2.17.1
From: Luiz Augusto von Dentz <[email protected]>
This implements scan.clear for discoverable filter.
---
client/main.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/client/main.c b/client/main.c
index 6e6f6d2fb..1a66a3ab4 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1416,6 +1416,11 @@ static void filter_clear_duplicate(void)
filter.duplicate = false;
}
+static void filter_clear_discoverable(void)
+{
+ filter.discoverable = false;
+}
+
struct clear_entry {
const char *name;
void (*clear) (void);
@@ -1427,6 +1432,7 @@ static const struct clear_entry filter_clear[] = {
{ "pathloss", filter_clear_pathloss },
{ "transport", filter_clear_transport },
{ "duplicate-data", filter_clear_duplicate },
+ { "discoverable", filter_clear_discoverable },
{}
};
@@ -2539,7 +2545,8 @@ static const struct bt_shell_menu scan_menu = {
{ "discoverable", "[on/off]", cmd_scan_filter_discoverable,
"Set/Get discoverable filter",
NULL },
- { "clear", "[uuids/rssi/pathloss/transport/duplicate-data]",
+ { "clear",
+ "[uuids/rssi/pathloss/transport/duplicate-data/discoverable]",
cmd_scan_filter_clear,
"Clears discovery filter.",
filter_clear_generator },
--
2.17.1
From: Luiz Augusto von Dentz <[email protected]>
This adds discoverable command to scan menu which can be used to set
if adapter should become discoverable while scanning:
[bluetooth]# scan.discoverable on
[bluetooth]# scan on
SetDiscoveryFilter success
[CHG] Controller XX:XX:XX:XX:XX:XX Discoverable: yes
Discovery started
[CHG] Controller XX:XX:XX:XX:XX:XX Discovering: yes
[bluetooth]# scan off
Discovery stopped
[CHG] Controller XX:XX:XX:XX:XX:XX Discoverable: no
---
client/main.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/client/main.c b/client/main.c
index 6f472d050..6e6f6d2fb 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1166,6 +1166,7 @@ static struct set_discovery_filter_args {
char **uuids;
size_t uuids_len;
dbus_bool_t duplicate;
+ dbus_bool_t discoverable;
bool set;
} filter = {
.rssi = DISTANCE_VAL_INVALID,
@@ -1205,6 +1206,11 @@ static void set_discovery_filter_setup(DBusMessageIter *iter, void *user_data)
DBUS_TYPE_BOOLEAN,
&args->duplicate);
+ if (args->discoverable)
+ g_dbus_dict_append_entry(&dict, "Discoverable",
+ DBUS_TYPE_BOOLEAN,
+ &args->discoverable);
+
dbus_message_iter_close_container(iter, &dict);
}
@@ -1362,6 +1368,26 @@ static void cmd_scan_filter_duplicate_data(int argc, char *argv[])
filter.set = false;
}
+static void cmd_scan_filter_discoverable(int argc, char *argv[])
+{
+ if (argc < 2 || !strlen(argv[1])) {
+ bt_shell_printf("Discoverable: %s\n",
+ filter.discoverable ? "on" : "off");
+ return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+ }
+
+ if (!strcmp(argv[1], "on"))
+ filter.discoverable = true;
+ else if (!strcmp(argv[1], "off"))
+ filter.discoverable = false;
+ else {
+ bt_shell_printf("Invalid option: %s\n", argv[1]);
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }
+
+ filter.set = false;
+}
+
static void filter_clear_uuids(void)
{
g_strfreev(filter.uuids);
@@ -2510,6 +2536,9 @@ static const struct bt_shell_menu scan_menu = {
{ "duplicate-data", "[on/off]", cmd_scan_filter_duplicate_data,
"Set/Get duplicate data filter",
NULL },
+ { "discoverable", "[on/off]", cmd_scan_filter_discoverable,
+ "Set/Get discoverable filter",
+ NULL },
{ "clear", "[uuids/rssi/pathloss/transport/duplicate-data]",
cmd_scan_filter_clear,
"Clears discovery filter.",
--
2.17.1
From: Luiz Augusto von Dentz <[email protected]>
This implements the discovery filter discoverable and tracks which
clients had enabled it and restores the settings when the last client
enabling it exits.
---
src/adapter.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index f92c897c7..4227718b9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -157,6 +157,7 @@ struct discovery_filter {
int16_t rssi;
GSList *uuids;
bool duplicate;
+ bool discoverable;
};
struct watch_client {
@@ -219,6 +220,7 @@ struct btd_adapter {
uint8_t discovery_type; /* current active discovery type */
uint8_t discovery_enable; /* discovery enabled/disabled */
bool discovery_suspended; /* discovery has been suspended */
+ bool discovery_discoverable; /* discoverable while discovering */
GSList *discovery_list; /* list of discovery clients */
GSList *set_filter_list; /* list of clients that specified
* filter, but don't scan yet
@@ -1842,6 +1844,20 @@ static void discovery_free(void *user_data)
g_free(client);
}
+static bool set_discovery_discoverable(struct btd_adapter *adapter, bool enable)
+{
+ if (adapter->discovery_discoverable == enable)
+ return true;
+
+ /* Reset discoverable filter if already set */
+ if (enable && (adapter->current_settings & MGMT_OP_SET_DISCOVERABLE))
+ return true;
+
+ adapter->discovery_discoverable = enable;
+
+ return set_discoverable(adapter, enable, 0);
+}
+
static void discovery_remove(struct watch_client *client)
{
struct btd_adapter *adapter = client->adapter;
@@ -2090,6 +2106,8 @@ static bool filters_equal(struct mgmt_cp_start_service_discovery *a,
static int update_discovery_filter(struct btd_adapter *adapter)
{
struct mgmt_cp_start_service_discovery *sd_cp;
+ GSList *l;
+
DBG("");
@@ -2099,6 +2117,18 @@ static int update_discovery_filter(struct btd_adapter *adapter)
return -ENOMEM;
}
+ for (l = adapter->discovery_list; l; l = g_slist_next(l)) {
+ struct watch_client *client = l->data;
+
+ if (!client->discovery_filter)
+ continue;
+
+ if (client->discovery_filter->discoverable)
+ break;
+ }
+
+ set_discovery_discoverable(adapter, l ? true : false);
+
/*
* If filters are equal, then don't update scan, except for when
* starting discovery.
@@ -2130,6 +2160,9 @@ static int discovery_stop(struct watch_client *client)
return 0;
}
+ if (adapter->discovery_discoverable)
+ set_discovery_discoverable(adapter, false);
+
/*
* In the idle phase of a discovery, there is no need to stop it
* and so it is enough to send out the signal and just return.
@@ -2224,6 +2257,7 @@ static DBusMessage *start_discovery(DBusConnection *conn,
adapter->set_filter_list, client);
adapter->discovery_list = g_slist_prepend(
adapter->discovery_list, client);
+
goto done;
}
@@ -2348,6 +2382,17 @@ static bool parse_duplicate_data(DBusMessageIter *value,
return true;
}
+static bool parse_discoverable(DBusMessageIter *value,
+ struct discovery_filter *filter)
+{
+ if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN)
+ return false;
+
+ dbus_message_iter_get_basic(value, &filter->discoverable);
+
+ return true;
+}
+
struct filter_parser {
const char *name;
bool (*func)(DBusMessageIter *iter, struct discovery_filter *filter);
@@ -2357,6 +2402,7 @@ struct filter_parser {
{ "Pathloss", parse_pathloss },
{ "Transport", parse_transport },
{ "DuplicateData", parse_duplicate_data },
+ { "Discoverable", parse_discoverable },
{ }
};
@@ -2396,6 +2442,7 @@ static bool parse_discovery_filter_dict(struct btd_adapter *adapter,
(*filter)->rssi = DISTANCE_VAL_INVALID;
(*filter)->type = get_scan_type(adapter);
(*filter)->duplicate = false;
+ (*filter)->discoverable = false;
dbus_message_iter_init(msg, &iter);
if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY ||
@@ -2441,8 +2488,10 @@ static bool parse_discovery_filter_dict(struct btd_adapter *adapter,
goto invalid_args;
DBG("filtered discovery params: transport: %d rssi: %d pathloss: %d "
- " duplicate data: %s ", (*filter)->type, (*filter)->rssi,
- (*filter)->pathloss, (*filter)->duplicate ? "true" : "false");
+ " duplicate data: %s discoverable %s", (*filter)->type,
+ (*filter)->rssi, (*filter)->pathloss,
+ (*filter)->duplicate ? "true" : "false",
+ (*filter)->discoverable ? "true" : "false");
return true;
@@ -2880,6 +2929,9 @@ static void property_set_discoverable(const GDBusPropertyTable *property,
return;
}
+ /* Reset discovery_discoverable as Discoverable takes precedence */
+ adapter->discovery_discoverable = false;
+
property_set_mode(adapter, MGMT_SETTING_DISCOVERABLE, iter, id);
}
--
2.17.1