2019-10-01 17:04:24

by Bruno Randolf

[permalink] [raw]
Subject: bluetoothd: Please don't filter UUIDs

Hello All,

I am a developer using bluez via D-Bus to access GATT services and
characteristics [1] and I would like to argue that bluez should not
filter UUIDs of characteristics it handles by internal plugins.

I recently came across this with the battery service (BAS). Our BLE
device exports it, I can read it on Android, iOS and from other devices,
but reading it from Linux is impossible. The UUID doesn't even show up
in the list of characteristics! It took me a while to figure out, that
it is filtered out and that you are supposed to read it via a different
API on D-Bus [2].

It surely makes sense to provide this more generic API, but I'd argue
that all services and characteristics should be available via the normal
GATT-based API using org.bluez.GattCharacteristic1 as well.

One of my clients, for example, uses Linux/bluez as an interface for
Server-based reading and writing of GATT characteristics of several
managed devices. So I can read all those UUIDs, but why not the battery
level? What happens when Bluez learns other GATT services, will their
characteristics then also disappear? I think there is a strong argument
for maintaining a generic API for GATT reading/writing characteristics
independently.

I made the following change to the bluetoothd code to get access again
to all UUIDs, and I would like you to consider to include it upstream to
enable us to access all characteristics via the normal GATT API:

--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -2006,9 +2006,6 @@ static void export_service(struct
gatt_db_attribute *attr, void *user_data)
struct btd_gatt_client *client = user_data;
struct service *service;

- if (gatt_db_service_get_claimed(attr))
- return;
-
service = service_create(attr, client);
if (!service)
return;

Thank you,
bruno


[1] I published parts of that as an open source library:
https://github.com/infsoft-locaware/blzlib

[2]
https://stackoverflow.com/questions/49078659/check-battery-level-of-connected-bluetooth-device-on-linux



2019-10-03 14:30:44

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: bluetoothd: Please don't filter UUIDs

Hi Bruno,

On Tue, Oct 1, 2019 at 8:06 PM Bruno Randolf <[email protected]> wrote:
>
> Hello All,
>
> I am a developer using bluez via D-Bus to access GATT services and
> characteristics [1] and I would like to argue that bluez should not
> filter UUIDs of characteristics it handles by internal plugins.

Well I guess you are forgetting that other users of the GATT may
interfere with plugin which is why we do the claim APIs in the first
place.

> I recently came across this with the battery service (BAS). Our BLE
> device exports it, I can read it on Android, iOS and from other devices,
> but reading it from Linux is impossible. The UUID doesn't even show up
> in the list of characteristics! It took me a while to figure out, that
> it is filtered out and that you are supposed to read it via a different
> API on D-Bus [2].

Reading is done automatically if the plugin is enabled, you can
disable the plugin if you don want it to be automatically read, having
both will not gonna happen since it will incur in more traffic in this
case and may have security implication in services such as HoG.

> It surely makes sense to provide this more generic API, but I'd argue
> that all services and characteristics should be available via the normal
> GATT-based API using org.bluez.GattCharacteristic1 as well.

Not if the service has security implication, for instance we don't
want application to be able to access the keys presses coming from a
HoG device, or other things like changing the settings bluetoothd has
configured.

> One of my clients, for example, uses Linux/bluez as an interface for
> Server-based reading and writing of GATT characteristics of several
> managed devices. So I can read all those UUIDs, but why not the battery
> level? What happens when Bluez learns other GATT services, will their
> characteristics then also disappear? I think there is a strong argument
> for maintaining a generic API for GATT reading/writing characteristics
> independently.

But there is even a stronger argument if something breaks because the
app access something it shouldn't, even if there are no conflicts
between the plugin the very least it would cause is duplicating the
traffic.

> I made the following change to the bluetoothd code to get access again
> to all UUIDs, and I would like you to consider to include it upstream to
> enable us to access all characteristics via the normal GATT API:
>
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -2006,9 +2006,6 @@ static void export_service(struct
> gatt_db_attribute *attr, void *user_data)
> struct btd_gatt_client *client = user_data;
> struct service *service;
>
> - if (gatt_db_service_get_claimed(attr))
> - return;
> -
> service = service_create(attr, client);
> if (!service)
> return;
>
> Thank you,
> bruno
>
>
> [1] I published parts of that as an open source library:
> https://github.com/infsoft-locaware/blzlib
>
> [2]
> https://stackoverflow.com/questions/49078659/check-battery-level-of-connected-bluetooth-device-on-linux
>
>


--
Luiz Augusto von Dentz

2019-10-09 13:10:35

by Bruno Randolf

[permalink] [raw]
Subject: Re: bluetoothd: Please don't filter UUIDs

On 03/10/2019 14:04, Luiz Augusto von Dentz wrote:
> Well I guess you are forgetting that other users of the GATT may
> interfere with plugin which is why we do the claim APIs in the first
> place.

Okay, I understand your arguments from your perspective, which seems to
focus on specific use cases with Desktop Linux.

But how do you suppose should application programmers or solution
providers access GATT characteristics under Linux in a predictable way
then? Not using BlueZ does not seem like a realistic option. Building
patched versions of bluetoothd isn't practical either.

At the very least there should be a build option to deactivate all
plugins, but I think something more flexible would be better. Could it
be a user choice? E.g. only claim services which the User has actively
connected to and activated that service?

In general I don't need to poll the battery status of every device I
happened to connect to, even though it might export a Battery service...
so I'd consider it more traffic if BAS is polled even though I didn't
ask for it. On the other hand on a Bluetooth device I have paired with,
something like a Headphone or Keyboard it obviously is the right choice.

I hope there is a way to support both use cases.

Regards,
bruno

>> It surely makes sense to provide this more generic API, but I'd argue
>> that all services and characteristics should be available via the normal
>> GATT-based API using org.bluez.GattCharacteristic1 as well.
>
> Not if the service has security implication, for instance we don't
> want application to be able to access the keys presses coming from a
> HoG device, or other things like changing the settings bluetoothd has
> configured.
>
>> One of my clients, for example, uses Linux/bluez as an interface for
>> Server-based reading and writing of GATT characteristics of several
>> managed devices. So I can read all those UUIDs, but why not the battery
>> level? What happens when Bluez learns other GATT services, will their
>> characteristics then also disappear? I think there is a strong argument
>> for maintaining a generic API for GATT reading/writing characteristics
>> independently.
>
> But there is even a stronger argument if something breaks because the
> app access something it shouldn't, even if there are no conflicts
> between the plugin the very least it would cause is duplicating the
> traffic.
>
>> I made the following change to the bluetoothd code to get access again
>> to all UUIDs, and I would like you to consider to include it upstream to
>> enable us to access all characteristics via the normal GATT API:
>>
>> --- a/src/gatt-client.c
>> +++ b/src/gatt-client.c
>> @@ -2006,9 +2006,6 @@ static void export_service(struct
>> gatt_db_attribute *attr, void *user_data)
>> struct btd_gatt_client *client = user_data;
>> struct service *service;
>>
>> - if (gatt_db_service_get_claimed(attr))
>> - return;
>> -
>> service = service_create(attr, client);
>> if (!service)
>> return;
>>
>> Thank you,
>> bruno
>>
>>
>> [1] I published parts of that as an open source library:
>> https://github.com/infsoft-locaware/blzlib
>>
>> [2]
>> https://stackoverflow.com/questions/49078659/check-battery-level-of-connected-bluetooth-device-on-linux
>>
>>
>
>

2019-10-09 13:35:46

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: bluetoothd: Please don't filter UUIDs

Hi Bruno,

On Wed, Oct 9, 2019 at 4:09 PM Bruno Randolf <[email protected]> wrote:
>
> On 03/10/2019 14:04, Luiz Augusto von Dentz wrote:
> > Well I guess you are forgetting that other users of the GATT may
> > interfere with plugin which is why we do the claim APIs in the first
> > place.
>
> Okay, I understand your arguments from your perspective, which seems to
> focus on specific use cases with Desktop Linux.
>
> But how do you suppose should application programmers or solution
> providers access GATT characteristics under Linux in a predictable way
> then? Not using BlueZ does not seem like a realistic option. Building
> patched versions of bluetoothd isn't practical either.

Right, so I assume then every application interested on battery would
then attempt to subscribe for the battery level on the own instead of
using org.bluez.Battery1? Either way that would be done via D-Bus but
dealing with a characteristic seem quite a bit more expensive not to
mention is another object the daemon has to maintain.

> At the very least there should be a build option to deactivate all
> plugins, but I think something more flexible would be better. Could it
> be a user choice? E.g. only claim services which the User has actively
> connected to and activated that service?

There are runtime switches to disable plugins i.e.: bluetoothd -P
battery and we can add build time switches as well, btw patches are
welcome if you want to disable battery plugin.

> In general I don't need to poll the battery status of every device I
> happened to connect to, even though it might export a Battery service...
> so I'd consider it more traffic if BAS is polled even though I didn't
> ask for it. On the other hand on a Bluetooth device I have paired with,
> something like a Headphone or Keyboard it obviously is the right choice.

If the system don't have any need for battery it can disable it just
like I said, but depending on the system it is probably better to
leave the daemon to subscribe since there could be several
applications interested on the battery status, simply reading
org.bluez.Battery1 is a _lot_ simpler than enumerating all attributes
and then subscribing to one characteristic.

> I hope there is a way to support both use cases.

There should be but it up for the system to decide if it wants a
dedicated plugin to deal with the battery status or not, for the
desktop usecase it is most likely simpler to support.

> Regards,
> bruno
>
> >> It surely makes sense to provide this more generic API, but I'd argue
> >> that all services and characteristics should be available via the normal
> >> GATT-based API using org.bluez.GattCharacteristic1 as well.
> >
> > Not if the service has security implication, for instance we don't
> > want application to be able to access the keys presses coming from a
> > HoG device, or other things like changing the settings bluetoothd has
> > configured.
> >
> >> One of my clients, for example, uses Linux/bluez as an interface for
> >> Server-based reading and writing of GATT characteristics of several
> >> managed devices. So I can read all those UUIDs, but why not the battery
> >> level? What happens when Bluez learns other GATT services, will their
> >> characteristics then also disappear? I think there is a strong argument
> >> for maintaining a generic API for GATT reading/writing characteristics
> >> independently.
> >
> > But there is even a stronger argument if something breaks because the
> > app access something it shouldn't, even if there are no conflicts
> > between the plugin the very least it would cause is duplicating the
> > traffic.
> >
> >> I made the following change to the bluetoothd code to get access again
> >> to all UUIDs, and I would like you to consider to include it upstream to
> >> enable us to access all characteristics via the normal GATT API:
> >>
> >> --- a/src/gatt-client.c
> >> +++ b/src/gatt-client.c
> >> @@ -2006,9 +2006,6 @@ static void export_service(struct
> >> gatt_db_attribute *attr, void *user_data)
> >> struct btd_gatt_client *client = user_data;
> >> struct service *service;
> >>
> >> - if (gatt_db_service_get_claimed(attr))
> >> - return;
> >> -
> >> service = service_create(attr, client);
> >> if (!service)
> >> return;
> >>
> >> Thank you,
> >> bruno
> >>
> >>
> >> [1] I published parts of that as an open source library:
> >> https://github.com/infsoft-locaware/blzlib
> >>
> >> [2]
> >> https://stackoverflow.com/questions/49078659/check-battery-level-of-connected-bluetooth-device-on-linux
> >>
> >>
> >
> >



--
Luiz Augusto von Dentz

2019-10-11 10:14:48

by Pavel Nikulin

[permalink] [raw]
Subject: Re: bluetoothd: Please don't filter UUIDs

Hello

On Wed, Oct 9, 2019 at 7:35 PM Luiz Augusto von Dentz
<[email protected]> wrote:
> Right, so I assume then every application interested on battery would
> then attempt to subscribe for the battery level on the own instead of
> using org.bluez.Battery1? Either way that would be done via D-Bus but
> dealing with a characteristic seem quite a bit more expensive not to
> mention is another object the daemon has to maintain.

There is an issue with org.bluez.Battery1 currently that I wrote to
linux-bluetooth about a week ago. The quickest solution other than
leaving it to client programs would be to fix the battery plugin. As
of now, it doesn't seem to support a use case where you have battery
service, but no battery attribute. Also, HFP/HSP own battery level
indication mechanism doesn't seem to be implemented at all.

I contacted the original contributor of LE code for
org.bluez.Battery1, but he said he is no longer involved with that.

2019-10-19 08:05:17

by Bruno Randolf

[permalink] [raw]
Subject: Re: bluetoothd: Please don't filter UUIDs

Hi!

On 09/10/2019 14:34, Luiz Augusto von Dentz wrote:
> There are runtime switches to disable plugins i.e.: bluetoothd -P
> battery and we can add build time switches as well, btw patches are
> welcome if you want to disable battery plugin.

That option to disable the plugin is good enough for me at the moment.
Sorry for not having found it on my own.

Regards,
bruno