2020-11-11 01:19:50

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

This patch add the documentation of the Battery Provider which lets
external clients feed battery information to BlueZ if they are able to
decode battery reporting via any profile. BlueZ UI clients can then use
the org.bluez.Battery1 API as a single source of battery information
coming from many different profiles.

Reviewed-by: Miao-chen Chou <[email protected]>

---
doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/doc/battery-api.txt b/doc/battery-api.txt
index dc7dbeda2..058bf0c6e 100644
--- a/doc/battery-api.txt
+++ b/doc/battery-api.txt
@@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
Properties byte Percentage [readonly]

The percentage of battery left as an unsigned 8-bit integer.
+
+ string Source [readonly, optional, experimental]
+
+ Describes where the battery information comes from
+ (e.g. "HFP 1.7", "HID").
+ This property is informational only and may be useful
+ for debugging purposes.
+
+
+Battery Provider Manager hierarchy
+==================================
+A battery provider starts by registering itself as a battery provider with the
+RegisterBatteryProvider method passing an object path as the provider ID. Then,
+it can start exposing org.bluez.BatteryProvider1 objects having the path
+starting with the given provider ID. It can also remove objects at any time.
+BlueZ will stop monitoring these exposed and removed objects after
+UnregisterBatteryProvider is called for that provider ID.
+
+Service org.bluez
+Interface org.bluez.BatteryProviderManager1 [experimental]
+Object path /org/bluez/{hci0,hci1,...}
+
+Methods void RegisterBatteryProvider(object provider)
+
+ This registers a battery provider. A registered
+ battery provider can then expose objects with
+ org.bluez.BatteryProvider1 interface described below.
+
+ void UnregisterBatteryProvider(object provider)
+
+ This unregisters a battery provider. After
+ unregistration, the BatteryProvider1 objects provided
+ by this client are ignored by BlueZ.
+
+
+Battery Provider hierarchy
+==========================
+
+Service <client D-Bus address>
+Interface org.bluez.BatteryProvider1 [experimental]
+Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+Properties byte Percentage [readonly]
+
+ The percentage of battery left as an unsigned 8-bit
+ integer.
+
+ string Source [readonly, optional]
+
+ Describes where the battery information comes from
+ (e.g. "HFP 1.7", "HID").
+ This property is informational only and may be useful
+ for debugging purposes. The content of this property is
+ a free string, but it is recommended to include the
+ profile name and version to be useful.
--
2.26.2


2020-11-17 00:19:08

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

Hi Sonny,

On Tue, Nov 10, 2020 at 5:22 PM Sonny Sasaka <[email protected]> wrote:
>
> This patch add the documentation of the Battery Provider which lets
> external clients feed battery information to BlueZ if they are able to
> decode battery reporting via any profile. BlueZ UI clients can then use
> the org.bluez.Battery1 API as a single source of battery information
> coming from many different profiles.
>
> Reviewed-by: Miao-chen Chou <[email protected]>
>
> ---
> doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> index dc7dbeda2..058bf0c6e 100644
> --- a/doc/battery-api.txt
> +++ b/doc/battery-api.txt
> @@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> Properties byte Percentage [readonly]
>
> The percentage of battery left as an unsigned 8-bit integer.
> +
> + string Source [readonly, optional, experimental]
> +
> + Describes where the battery information comes from
> + (e.g. "HFP 1.7", "HID").
> + This property is informational only and may be useful
> + for debugging purposes.

We should probably tight this to UUID instead.

> +
> +
> +Battery Provider Manager hierarchy
> +==================================
> +A battery provider starts by registering itself as a battery provider with the
> +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> +it can start exposing org.bluez.BatteryProvider1 objects having the path
> +starting with the given provider ID. It can also remove objects at any time.
> +BlueZ will stop monitoring these exposed and removed objects after
> +UnregisterBatteryProvider is called for that provider ID.
> +
> +Service org.bluez
> +Interface org.bluez.BatteryProviderManager1 [experimental]
> +Object path /org/bluez/{hci0,hci1,...}
> +
> +Methods void RegisterBatteryProvider(object provider)
> +
> + This registers a battery provider. A registered
> + battery provider can then expose objects with
> + org.bluez.BatteryProvider1 interface described below.
> +
> + void UnregisterBatteryProvider(object provider)
> +
> + This unregisters a battery provider. After
> + unregistration, the BatteryProvider1 objects provided
> + by this client are ignored by BlueZ.

Not sure if you were followinging the monitor patches, for registering
objects we do prefer the ObjectManager style so multiple provider can
be registered at once, also we may want to tight the control of
battery provider with Profile API so we guarantee that the same entity
that handles the profile connection is the one providing the battery
status.

> +
> +
> +Battery Provider hierarchy
> +==========================
> +
> +Service <client D-Bus address>
> +Interface org.bluez.BatteryProvider1 [experimental]
> +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> +
> +Properties byte Percentage [readonly]
> +
> + The percentage of battery left as an unsigned 8-bit
> + integer.
> +
> + string Source [readonly, optional]
> +
> + Describes where the battery information comes from
> + (e.g. "HFP 1.7", "HID").
> + This property is informational only and may be useful
> + for debugging purposes. The content of this property is
> + a free string, but it is recommended to include the
> + profile name and version to be useful.
> --
> 2.26.2

Perhaps we should make this use the same interface as we use for the
daemon itself (Battery1) and the Source there as well. Last but not
least, have you explored the idea of exporting the battery status over
uHID? If I got this right this would aggregate the status of different
sources and then make the daemon expose them, which while it works for
now it means that upper layer have different interfaces for handling a
battery status of something plugged over USB and over Bluetooth.


--
Luiz Augusto von Dentz

2020-11-17 22:39:59

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

Hi Luiz,

Thanks for the feedback. Please find my responses below.

On Mon, Nov 16, 2020 at 4:17 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Tue, Nov 10, 2020 at 5:22 PM Sonny Sasaka <[email protected]> wrote:
> >
> > This patch add the documentation of the Battery Provider which lets
> > external clients feed battery information to BlueZ if they are able to
> > decode battery reporting via any profile. BlueZ UI clients can then use
> > the org.bluez.Battery1 API as a single source of battery information
> > coming from many different profiles.
> >
> > Reviewed-by: Miao-chen Chou <[email protected]>
> >
> > ---
> > doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > index dc7dbeda2..058bf0c6e 100644
> > --- a/doc/battery-api.txt
> > +++ b/doc/battery-api.txt
> > @@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > Properties byte Percentage [readonly]
> >
> > The percentage of battery left as an unsigned 8-bit integer.
> > +
> > + string Source [readonly, optional, experimental]
> > +
> > + Describes where the battery information comes from
> > + (e.g. "HFP 1.7", "HID").
> > + This property is informational only and may be useful
> > + for debugging purposes.
>
> We should probably tight this to UUID instead.
Ack. Will update the doc to suggest UUID in this source field.

>
> > +
> > +
> > +Battery Provider Manager hierarchy
> > +==================================
> > +A battery provider starts by registering itself as a battery provider with the
> > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > +starting with the given provider ID. It can also remove objects at any time.
> > +BlueZ will stop monitoring these exposed and removed objects after
> > +UnregisterBatteryProvider is called for that provider ID.
> > +
> > +Service org.bluez
> > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > +Object path /org/bluez/{hci0,hci1,...}
> > +
> > +Methods void RegisterBatteryProvider(object provider)
> > +
> > + This registers a battery provider. A registered
> > + battery provider can then expose objects with
> > + org.bluez.BatteryProvider1 interface described below.
> > +
> > + void UnregisterBatteryProvider(object provider)
> > +
> > + This unregisters a battery provider. After
> > + unregistration, the BatteryProvider1 objects provided
> > + by this client are ignored by BlueZ.
>
> Not sure if you were followinging the monitor patches, for registering
> objects we do prefer the ObjectManager style so multiple provider can
> be registered at once, also we may want to tight the control of
> battery provider with Profile API so we guarantee that the same entity
> that handles the profile connection is the one providing the battery
> status.
It is actually already in ObjectManager style. After the "root path"
is registered, the provider can expose many D-Bus objects at once and
bluetoothd can detect it.
About tying it with the Profile API, I will take a look at how it could be done.

>
> > +
> > +
> > +Battery Provider hierarchy
> > +==========================
> > +
> > +Service <client D-Bus address>
> > +Interface org.bluez.BatteryProvider1 [experimental]
> > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > +
> > +Properties byte Percentage [readonly]
> > +
> > + The percentage of battery left as an unsigned 8-bit
> > + integer.
> > +
> > + string Source [readonly, optional]
> > +
> > + Describes where the battery information comes from
> > + (e.g. "HFP 1.7", "HID").
> > + This property is informational only and may be useful
> > + for debugging purposes. The content of this property is
> > + a free string, but it is recommended to include the
> > + profile name and version to be useful.
> > --
> > 2.26.2
>
> Perhaps we should make this use the same interface as we use for the
> daemon itself (Battery1) and the Source there as well. Last but not
> least, have you explored the idea of exporting the battery status over
> uHID? If I got this right this would aggregate the status of different
> sources and then make the daemon expose them, which while it works for
> now it means that upper layer have different interfaces for handling a
> battery status of something plugged over USB and over Bluetooth.
I am okay with naming the interface Battery1, the same as the daemon.
Will make an update.
About the exporting battery status via UHID, it is possible to be
done, but it is an orthogonal problem that we plan to tackle
separately. Right now, this API is general enough for Chrome OS to
allow both HFP and HID batteries to be consolidated in BlueZ. Chrome
OS's powerd feeds only Bluetooth battery levels from
/sys/class/power_supply and filters out USB devices so the UI layer
does not need to worry about it: everything from BlueZ is tied to a
Bluetooth device.

>
>
> --
> Luiz Augusto von Dentz

2020-11-17 23:03:10

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

Hi Sonny,

On Tue, Nov 17, 2020 at 2:37 PM Sonny Sasaka <[email protected]> wrote:
>
> Hi Luiz,
>
> Thanks for the feedback. Please find my responses below.
>
> On Mon, Nov 16, 2020 at 4:17 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Sonny,
> >
> > On Tue, Nov 10, 2020 at 5:22 PM Sonny Sasaka <[email protected]> wrote:
> > >
> > > This patch add the documentation of the Battery Provider which lets
> > > external clients feed battery information to BlueZ if they are able to
> > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > the org.bluez.Battery1 API as a single source of battery information
> > > coming from many different profiles.
> > >
> > > Reviewed-by: Miao-chen Chou <[email protected]>
> > >
> > > ---
> > > doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 55 insertions(+)
> > >
> > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > index dc7dbeda2..058bf0c6e 100644
> > > --- a/doc/battery-api.txt
> > > +++ b/doc/battery-api.txt
> > > @@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > Properties byte Percentage [readonly]
> > >
> > > The percentage of battery left as an unsigned 8-bit integer.
> > > +
> > > + string Source [readonly, optional, experimental]
> > > +
> > > + Describes where the battery information comes from
> > > + (e.g. "HFP 1.7", "HID").
> > > + This property is informational only and may be useful
> > > + for debugging purposes.
> >
> > We should probably tight this to UUID instead.
> Ack. Will update the doc to suggest UUID in this source field.
>
> >
> > > +
> > > +
> > > +Battery Provider Manager hierarchy
> > > +==================================
> > > +A battery provider starts by registering itself as a battery provider with the
> > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > +starting with the given provider ID. It can also remove objects at any time.
> > > +BlueZ will stop monitoring these exposed and removed objects after
> > > +UnregisterBatteryProvider is called for that provider ID.
> > > +
> > > +Service org.bluez
> > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > +Object path /org/bluez/{hci0,hci1,...}
> > > +
> > > +Methods void RegisterBatteryProvider(object provider)
> > > +
> > > + This registers a battery provider. A registered
> > > + battery provider can then expose objects with
> > > + org.bluez.BatteryProvider1 interface described below.
> > > +
> > > + void UnregisterBatteryProvider(object provider)
> > > +
> > > + This unregisters a battery provider. After
> > > + unregistration, the BatteryProvider1 objects provided
> > > + by this client are ignored by BlueZ.
> >
> > Not sure if you were followinging the monitor patches, for registering
> > objects we do prefer the ObjectManager style so multiple provider can
> > be registered at once, also we may want to tight the control of
> > battery provider with Profile API so we guarantee that the same entity
> > that handles the profile connection is the one providing the battery
> > status.
> It is actually already in ObjectManager style. After the "root path"
> is registered, the provider can expose many D-Bus objects at once and
> bluetoothd can detect it.
> About tying it with the Profile API, I will take a look at how it could be done.
>
> >
> > > +
> > > +
> > > +Battery Provider hierarchy
> > > +==========================
> > > +
> > > +Service <client D-Bus address>
> > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > +
> > > +Properties byte Percentage [readonly]
> > > +
> > > + The percentage of battery left as an unsigned 8-bit
> > > + integer.
> > > +
> > > + string Source [readonly, optional]
> > > +
> > > + Describes where the battery information comes from
> > > + (e.g. "HFP 1.7", "HID").
> > > + This property is informational only and may be useful
> > > + for debugging purposes. The content of this property is
> > > + a free string, but it is recommended to include the
> > > + profile name and version to be useful.
> > > --
> > > 2.26.2
> >
> > Perhaps we should make this use the same interface as we use for the
> > daemon itself (Battery1) and the Source there as well. Last but not
> > least, have you explored the idea of exporting the battery status over
> > uHID? If I got this right this would aggregate the status of different
> > sources and then make the daemon expose them, which while it works for
> > now it means that upper layer have different interfaces for handling a
> > battery status of something plugged over USB and over Bluetooth.
> I am okay with naming the interface Battery1, the same as the daemon.
> Will make an update.
> About the exporting battery status via UHID, it is possible to be
> done, but it is an orthogonal problem that we plan to tackle
> separately. Right now, this API is general enough for Chrome OS to
> allow both HFP and HID batteries to be consolidated in BlueZ. Chrome
> OS's powerd feeds only Bluetooth battery levels from
> /sys/class/power_supply and filters out USB devices so the UI layer
> does not need to worry about it: everything from BlueZ is tied to a
> Bluetooth device.

But how about devices pushing their battery status over HID reports
instead of GATT? Afaik this is possible since the HID device may have
support for USB (wired) as well where it exposes its battery status
over HID it may just choose to continue doing so while connected over
Bluetooth.

> >
> >
> > --
> > Luiz Augusto von Dentz



--
Luiz Augusto von Dentz

2020-11-17 23:17:58

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

Hi Luiz,

On Tue, Nov 17, 2020 at 3:01 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Tue, Nov 17, 2020 at 2:37 PM Sonny Sasaka <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > Thanks for the feedback. Please find my responses below.
> >
> > On Mon, Nov 16, 2020 at 4:17 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Tue, Nov 10, 2020 at 5:22 PM Sonny Sasaka <[email protected]> wrote:
> > > >
> > > > This patch add the documentation of the Battery Provider which lets
> > > > external clients feed battery information to BlueZ if they are able to
> > > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > > the org.bluez.Battery1 API as a single source of battery information
> > > > coming from many different profiles.
> > > >
> > > > Reviewed-by: Miao-chen Chou <[email protected]>
> > > >
> > > > ---
> > > > doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 55 insertions(+)
> > > >
> > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > index dc7dbeda2..058bf0c6e 100644
> > > > --- a/doc/battery-api.txt
> > > > +++ b/doc/battery-api.txt
> > > > @@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > Properties byte Percentage [readonly]
> > > >
> > > > The percentage of battery left as an unsigned 8-bit integer.
> > > > +
> > > > + string Source [readonly, optional, experimental]
> > > > +
> > > > + Describes where the battery information comes from
> > > > + (e.g. "HFP 1.7", "HID").
> > > > + This property is informational only and may be useful
> > > > + for debugging purposes.
> > >
> > > We should probably tight this to UUID instead.
> > Ack. Will update the doc to suggest UUID in this source field.
> >
> > >
> > > > +
> > > > +
> > > > +Battery Provider Manager hierarchy
> > > > +==================================
> > > > +A battery provider starts by registering itself as a battery provider with the
> > > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > > +starting with the given provider ID. It can also remove objects at any time.
> > > > +BlueZ will stop monitoring these exposed and removed objects after
> > > > +UnregisterBatteryProvider is called for that provider ID.
> > > > +
> > > > +Service org.bluez
> > > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > > +Object path /org/bluez/{hci0,hci1,...}
> > > > +
> > > > +Methods void RegisterBatteryProvider(object provider)
> > > > +
> > > > + This registers a battery provider. A registered
> > > > + battery provider can then expose objects with
> > > > + org.bluez.BatteryProvider1 interface described below.
> > > > +
> > > > + void UnregisterBatteryProvider(object provider)
> > > > +
> > > > + This unregisters a battery provider. After
> > > > + unregistration, the BatteryProvider1 objects provided
> > > > + by this client are ignored by BlueZ.
> > >
> > > Not sure if you were followinging the monitor patches, for registering
> > > objects we do prefer the ObjectManager style so multiple provider can
> > > be registered at once, also we may want to tight the control of
> > > battery provider with Profile API so we guarantee that the same entity
> > > that handles the profile connection is the one providing the battery
> > > status.
> > It is actually already in ObjectManager style. After the "root path"
> > is registered, the provider can expose many D-Bus objects at once and
> > bluetoothd can detect it.
> > About tying it with the Profile API, I will take a look at how it could be done.
> >
> > >
> > > > +
> > > > +
> > > > +Battery Provider hierarchy
> > > > +==========================
> > > > +
> > > > +Service <client D-Bus address>
> > > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > +
> > > > +Properties byte Percentage [readonly]
> > > > +
> > > > + The percentage of battery left as an unsigned 8-bit
> > > > + integer.
> > > > +
> > > > + string Source [readonly, optional]
> > > > +
> > > > + Describes where the battery information comes from
> > > > + (e.g. "HFP 1.7", "HID").
> > > > + This property is informational only and may be useful
> > > > + for debugging purposes. The content of this property is
> > > > + a free string, but it is recommended to include the
> > > > + profile name and version to be useful.
> > > > --
> > > > 2.26.2
> > >
> > > Perhaps we should make this use the same interface as we use for the
> > > daemon itself (Battery1) and the Source there as well. Last but not
> > > least, have you explored the idea of exporting the battery status over
> > > uHID? If I got this right this would aggregate the status of different
> > > sources and then make the daemon expose them, which while it works for
> > > now it means that upper layer have different interfaces for handling a
> > > battery status of something plugged over USB and over Bluetooth.
> > I am okay with naming the interface Battery1, the same as the daemon.
> > Will make an update.
> > About the exporting battery status via UHID, it is possible to be
> > done, but it is an orthogonal problem that we plan to tackle
> > separately. Right now, this API is general enough for Chrome OS to
> > allow both HFP and HID batteries to be consolidated in BlueZ. Chrome
> > OS's powerd feeds only Bluetooth battery levels from
> > /sys/class/power_supply and filters out USB devices so the UI layer
> > does not need to worry about it: everything from BlueZ is tied to a
> > Bluetooth device.
>
> But how about devices pushing their battery status over HID reports
> instead of GATT? Afaik this is possible since the HID device may have
> support for USB (wired) as well where it exposes its battery status
> over HID it may just choose to continue doing so while connected over
> Bluetooth.
If the Bluetooth device reports battery status via HID, it will go
into sys/class/power_supply. In Chrome OS, powerd sends this back to
BlueZ because it knows the Bluetooth device address from
/sys/class/power_supply/ file name. This way the UI can get the
battery value from BlueZ since it's tied to a Bluetooth device. In
most Linux desktops, upower already exposes /sys/class/power_supply to
the UI directly so it's fine also.
>
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

2020-11-17 23:34:37

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

Hi Sonny,

On Tue, Nov 17, 2020 at 3:17 PM Sonny Sasaka <[email protected]> wrote:
>
> Hi Luiz,
>
> On Tue, Nov 17, 2020 at 3:01 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Sonny,
> >
> > On Tue, Nov 17, 2020 at 2:37 PM Sonny Sasaka <[email protected]> wrote:
> > >
> > > Hi Luiz,
> > >
> > > Thanks for the feedback. Please find my responses below.
> > >
> > > On Mon, Nov 16, 2020 at 4:17 PM Luiz Augusto von Dentz
> > > <[email protected]> wrote:
> > > >
> > > > Hi Sonny,
> > > >
> > > > On Tue, Nov 10, 2020 at 5:22 PM Sonny Sasaka <[email protected]> wrote:
> > > > >
> > > > > This patch add the documentation of the Battery Provider which lets
> > > > > external clients feed battery information to BlueZ if they are able to
> > > > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > > > the org.bluez.Battery1 API as a single source of battery information
> > > > > coming from many different profiles.
> > > > >
> > > > > Reviewed-by: Miao-chen Chou <[email protected]>
> > > > >
> > > > > ---
> > > > > doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 55 insertions(+)
> > > > >
> > > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > > index dc7dbeda2..058bf0c6e 100644
> > > > > --- a/doc/battery-api.txt
> > > > > +++ b/doc/battery-api.txt
> > > > > @@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > Properties byte Percentage [readonly]
> > > > >
> > > > > The percentage of battery left as an unsigned 8-bit integer.
> > > > > +
> > > > > + string Source [readonly, optional, experimental]
> > > > > +
> > > > > + Describes where the battery information comes from
> > > > > + (e.g. "HFP 1.7", "HID").
> > > > > + This property is informational only and may be useful
> > > > > + for debugging purposes.
> > > >
> > > > We should probably tight this to UUID instead.
> > > Ack. Will update the doc to suggest UUID in this source field.
> > >
> > > >
> > > > > +
> > > > > +
> > > > > +Battery Provider Manager hierarchy
> > > > > +==================================
> > > > > +A battery provider starts by registering itself as a battery provider with the
> > > > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > > > +starting with the given provider ID. It can also remove objects at any time.
> > > > > +BlueZ will stop monitoring these exposed and removed objects after
> > > > > +UnregisterBatteryProvider is called for that provider ID.
> > > > > +
> > > > > +Service org.bluez
> > > > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > > > +Object path /org/bluez/{hci0,hci1,...}
> > > > > +
> > > > > +Methods void RegisterBatteryProvider(object provider)
> > > > > +
> > > > > + This registers a battery provider. A registered
> > > > > + battery provider can then expose objects with
> > > > > + org.bluez.BatteryProvider1 interface described below.
> > > > > +
> > > > > + void UnregisterBatteryProvider(object provider)
> > > > > +
> > > > > + This unregisters a battery provider. After
> > > > > + unregistration, the BatteryProvider1 objects provided
> > > > > + by this client are ignored by BlueZ.
> > > >
> > > > Not sure if you were followinging the monitor patches, for registering
> > > > objects we do prefer the ObjectManager style so multiple provider can
> > > > be registered at once, also we may want to tight the control of
> > > > battery provider with Profile API so we guarantee that the same entity
> > > > that handles the profile connection is the one providing the battery
> > > > status.
> > > It is actually already in ObjectManager style. After the "root path"
> > > is registered, the provider can expose many D-Bus objects at once and
> > > bluetoothd can detect it.
> > > About tying it with the Profile API, I will take a look at how it could be done.
> > >
> > > >
> > > > > +
> > > > > +
> > > > > +Battery Provider hierarchy
> > > > > +==========================
> > > > > +
> > > > > +Service <client D-Bus address>
> > > > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > +
> > > > > +Properties byte Percentage [readonly]
> > > > > +
> > > > > + The percentage of battery left as an unsigned 8-bit
> > > > > + integer.
> > > > > +
> > > > > + string Source [readonly, optional]
> > > > > +
> > > > > + Describes where the battery information comes from
> > > > > + (e.g. "HFP 1.7", "HID").
> > > > > + This property is informational only and may be useful
> > > > > + for debugging purposes. The content of this property is
> > > > > + a free string, but it is recommended to include the
> > > > > + profile name and version to be useful.
> > > > > --
> > > > > 2.26.2
> > > >
> > > > Perhaps we should make this use the same interface as we use for the
> > > > daemon itself (Battery1) and the Source there as well. Last but not
> > > > least, have you explored the idea of exporting the battery status over
> > > > uHID? If I got this right this would aggregate the status of different
> > > > sources and then make the daemon expose them, which while it works for
> > > > now it means that upper layer have different interfaces for handling a
> > > > battery status of something plugged over USB and over Bluetooth.
> > > I am okay with naming the interface Battery1, the same as the daemon.
> > > Will make an update.
> > > About the exporting battery status via UHID, it is possible to be
> > > done, but it is an orthogonal problem that we plan to tackle
> > > separately. Right now, this API is general enough for Chrome OS to
> > > allow both HFP and HID batteries to be consolidated in BlueZ. Chrome
> > > OS's powerd feeds only Bluetooth battery levels from
> > > /sys/class/power_supply and filters out USB devices so the UI layer
> > > does not need to worry about it: everything from BlueZ is tied to a
> > > Bluetooth device.
> >
> > But how about devices pushing their battery status over HID reports
> > instead of GATT? Afaik this is possible since the HID device may have
> > support for USB (wired) as well where it exposes its battery status
> > over HID it may just choose to continue doing so while connected over
> > Bluetooth.
> If the Bluetooth device reports battery status via HID, it will go
> into sys/class/power_supply. In Chrome OS, powerd sends this back to
> BlueZ because it knows the Bluetooth device address from
> /sys/class/power_supply/ file name.

Well that is something Id like to avoid since if we do in the future
create an HID report this sort of logic could cause duplications as it
either the daemon or the kernel may start parsing these so we will
need some way to identify each source (perhaps by UUID) to avoid
duplications. Also having differences in upower and powerd really
doesn't help us unifying the handling here, I hope at least for
Bluetooth we end up with something similar when either daemons are
used, otherwise we may need plugins for each separately.

> >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz



--
Luiz Augusto von Dentz

2020-11-17 23:57:34

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

Hi Luiz,


On Tue, Nov 17, 2020 at 3:33 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Tue, Nov 17, 2020 at 3:17 PM Sonny Sasaka <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > On Tue, Nov 17, 2020 at 3:01 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Tue, Nov 17, 2020 at 2:37 PM Sonny Sasaka <[email protected]> wrote:
> > > >
> > > > Hi Luiz,
> > > >
> > > > Thanks for the feedback. Please find my responses below.
> > > >
> > > > On Mon, Nov 16, 2020 at 4:17 PM Luiz Augusto von Dentz
> > > > <[email protected]> wrote:
> > > > >
> > > > > Hi Sonny,
> > > > >
> > > > > On Tue, Nov 10, 2020 at 5:22 PM Sonny Sasaka <[email protected]> wrote:
> > > > > >
> > > > > > This patch add the documentation of the Battery Provider which lets
> > > > > > external clients feed battery information to BlueZ if they are able to
> > > > > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > > > > the org.bluez.Battery1 API as a single source of battery information
> > > > > > coming from many different profiles.
> > > > > >
> > > > > > Reviewed-by: Miao-chen Chou <[email protected]>
> > > > > >
> > > > > > ---
> > > > > > doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 55 insertions(+)
> > > > > >
> > > > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > > > index dc7dbeda2..058bf0c6e 100644
> > > > > > --- a/doc/battery-api.txt
> > > > > > +++ b/doc/battery-api.txt
> > > > > > @@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > > Properties byte Percentage [readonly]
> > > > > >
> > > > > > The percentage of battery left as an unsigned 8-bit integer.
> > > > > > +
> > > > > > + string Source [readonly, optional, experimental]
> > > > > > +
> > > > > > + Describes where the battery information comes from
> > > > > > + (e.g. "HFP 1.7", "HID").
> > > > > > + This property is informational only and may be useful
> > > > > > + for debugging purposes.
> > > > >
> > > > > We should probably tight this to UUID instead.
> > > > Ack. Will update the doc to suggest UUID in this source field.
> > > >
> > > > >
> > > > > > +
> > > > > > +
> > > > > > +Battery Provider Manager hierarchy
> > > > > > +==================================
> > > > > > +A battery provider starts by registering itself as a battery provider with the
> > > > > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > > > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > > > > +starting with the given provider ID. It can also remove objects at any time.
> > > > > > +BlueZ will stop monitoring these exposed and removed objects after
> > > > > > +UnregisterBatteryProvider is called for that provider ID.
> > > > > > +
> > > > > > +Service org.bluez
> > > > > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > > > > +Object path /org/bluez/{hci0,hci1,...}
> > > > > > +
> > > > > > +Methods void RegisterBatteryProvider(object provider)
> > > > > > +
> > > > > > + This registers a battery provider. A registered
> > > > > > + battery provider can then expose objects with
> > > > > > + org.bluez.BatteryProvider1 interface described below.
> > > > > > +
> > > > > > + void UnregisterBatteryProvider(object provider)
> > > > > > +
> > > > > > + This unregisters a battery provider. After
> > > > > > + unregistration, the BatteryProvider1 objects provided
> > > > > > + by this client are ignored by BlueZ.
> > > > >
> > > > > Not sure if you were followinging the monitor patches, for registering
> > > > > objects we do prefer the ObjectManager style so multiple provider can
> > > > > be registered at once, also we may want to tight the control of
> > > > > battery provider with Profile API so we guarantee that the same entity
> > > > > that handles the profile connection is the one providing the battery
> > > > > status.
> > > > It is actually already in ObjectManager style. After the "root path"
> > > > is registered, the provider can expose many D-Bus objects at once and
> > > > bluetoothd can detect it.
> > > > About tying it with the Profile API, I will take a look at how it could be done.
> > > >
> > > > >
> > > > > > +
> > > > > > +
> > > > > > +Battery Provider hierarchy
> > > > > > +==========================
> > > > > > +
> > > > > > +Service <client D-Bus address>
> > > > > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > > > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > > +
> > > > > > +Properties byte Percentage [readonly]
> > > > > > +
> > > > > > + The percentage of battery left as an unsigned 8-bit
> > > > > > + integer.
> > > > > > +
> > > > > > + string Source [readonly, optional]
> > > > > > +
> > > > > > + Describes where the battery information comes from
> > > > > > + (e.g. "HFP 1.7", "HID").
> > > > > > + This property is informational only and may be useful
> > > > > > + for debugging purposes. The content of this property is
> > > > > > + a free string, but it is recommended to include the
> > > > > > + profile name and version to be useful.
> > > > > > --
> > > > > > 2.26.2
> > > > >
> > > > > Perhaps we should make this use the same interface as we use for the
> > > > > daemon itself (Battery1) and the Source there as well. Last but not
> > > > > least, have you explored the idea of exporting the battery status over
> > > > > uHID? If I got this right this would aggregate the status of different
> > > > > sources and then make the daemon expose them, which while it works for
> > > > > now it means that upper layer have different interfaces for handling a
> > > > > battery status of something plugged over USB and over Bluetooth.
> > > > I am okay with naming the interface Battery1, the same as the daemon.
> > > > Will make an update.
> > > > About the exporting battery status via UHID, it is possible to be
> > > > done, but it is an orthogonal problem that we plan to tackle
> > > > separately. Right now, this API is general enough for Chrome OS to
> > > > allow both HFP and HID batteries to be consolidated in BlueZ. Chrome
> > > > OS's powerd feeds only Bluetooth battery levels from
> > > > /sys/class/power_supply and filters out USB devices so the UI layer
> > > > does not need to worry about it: everything from BlueZ is tied to a
> > > > Bluetooth device.
> > >
> > > But how about devices pushing their battery status over HID reports
> > > instead of GATT? Afaik this is possible since the HID device may have
> > > support for USB (wired) as well where it exposes its battery status
> > > over HID it may just choose to continue doing so while connected over
> > > Bluetooth.
> > If the Bluetooth device reports battery status via HID, it will go
> > into sys/class/power_supply. In Chrome OS, powerd sends this back to
> > BlueZ because it knows the Bluetooth device address from
> > /sys/class/power_supply/ file name.
>
> Well that is something Id like to avoid since if we do in the future
> create an HID report this sort of logic could cause duplications as it
> either the daemon or the kernel may start parsing these so we will
> need some way to identify each source (perhaps by UUID) to avoid
> duplications. Also having differences in upower and powerd really
> doesn't help us unifying the handling here, I hope at least for
> Bluetooth we end up with something similar when either daemons are
> used, otherwise we may need plugins for each separately.
Duplication is something that we have to deal with, regardless whether
it's about HID or anything else. As I originally proposed and
reflected in the code, we start with the simplest duplicate
resolution: accepting the first one who registered and ignores the
rest. From BlueZ point of view, clients can feed battery information
and it handles the duplication. From external clients point of view,
they do not know whether there is already another source (provider
from HFP does not know that there is already another provider from
FastPair, for example) so what they do is feed what they have and let
BlueZ handle the duplication. With this behavior defined, I don't
understand why it becomes necessary for BlueZ to extract HID battery
information directly? I *do* think it is a good idea, but I don't
understand why it (extracting HID battery directly by BlueZ) is a
prerequisite to have the battery provider API in place.

>
> > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

2020-11-20 21:01:43

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc

Hi Luiz,

As agreed off this thread, we will proceed with the first iteration of
the Battery Provider API without profile integration and HID internal
battery decoder. Please take another look at v3 patches I just sent.
Thanks for reviewing!

On Tue, Nov 17, 2020 at 3:55 PM Sonny Sasaka <[email protected]> wrote:
>
> Hi Luiz,
>
>
> On Tue, Nov 17, 2020 at 3:33 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Sonny,
> >
> > On Tue, Nov 17, 2020 at 3:17 PM Sonny Sasaka <[email protected]> wrote:
> > >
> > > Hi Luiz,
> > >
> > > On Tue, Nov 17, 2020 at 3:01 PM Luiz Augusto von Dentz
> > > <[email protected]> wrote:
> > > >
> > > > Hi Sonny,
> > > >
> > > > On Tue, Nov 17, 2020 at 2:37 PM Sonny Sasaka <[email protected]> wrote:
> > > > >
> > > > > Hi Luiz,
> > > > >
> > > > > Thanks for the feedback. Please find my responses below.
> > > > >
> > > > > On Mon, Nov 16, 2020 at 4:17 PM Luiz Augusto von Dentz
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > Hi Sonny,
> > > > > >
> > > > > > On Tue, Nov 10, 2020 at 5:22 PM Sonny Sasaka <[email protected]> wrote:
> > > > > > >
> > > > > > > This patch add the documentation of the Battery Provider which lets
> > > > > > > external clients feed battery information to BlueZ if they are able to
> > > > > > > decode battery reporting via any profile. BlueZ UI clients can then use
> > > > > > > the org.bluez.Battery1 API as a single source of battery information
> > > > > > > coming from many different profiles.
> > > > > > >
> > > > > > > Reviewed-by: Miao-chen Chou <[email protected]>
> > > > > > >
> > > > > > > ---
> > > > > > > doc/battery-api.txt | 55 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 55 insertions(+)
> > > > > > >
> > > > > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > > > > index dc7dbeda2..058bf0c6e 100644
> > > > > > > --- a/doc/battery-api.txt
> > > > > > > +++ b/doc/battery-api.txt
> > > > > > > @@ -12,3 +12,58 @@ Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > > > Properties byte Percentage [readonly]
> > > > > > >
> > > > > > > The percentage of battery left as an unsigned 8-bit integer.
> > > > > > > +
> > > > > > > + string Source [readonly, optional, experimental]
> > > > > > > +
> > > > > > > + Describes where the battery information comes from
> > > > > > > + (e.g. "HFP 1.7", "HID").
> > > > > > > + This property is informational only and may be useful
> > > > > > > + for debugging purposes.
> > > > > >
> > > > > > We should probably tight this to UUID instead.
> > > > > Ack. Will update the doc to suggest UUID in this source field.
> > > > >
> > > > > >
> > > > > > > +
> > > > > > > +
> > > > > > > +Battery Provider Manager hierarchy
> > > > > > > +==================================
> > > > > > > +A battery provider starts by registering itself as a battery provider with the
> > > > > > > +RegisterBatteryProvider method passing an object path as the provider ID. Then,
> > > > > > > +it can start exposing org.bluez.BatteryProvider1 objects having the path
> > > > > > > +starting with the given provider ID. It can also remove objects at any time.
> > > > > > > +BlueZ will stop monitoring these exposed and removed objects after
> > > > > > > +UnregisterBatteryProvider is called for that provider ID.
> > > > > > > +
> > > > > > > +Service org.bluez
> > > > > > > +Interface org.bluez.BatteryProviderManager1 [experimental]
> > > > > > > +Object path /org/bluez/{hci0,hci1,...}
> > > > > > > +
> > > > > > > +Methods void RegisterBatteryProvider(object provider)
> > > > > > > +
> > > > > > > + This registers a battery provider. A registered
> > > > > > > + battery provider can then expose objects with
> > > > > > > + org.bluez.BatteryProvider1 interface described below.
> > > > > > > +
> > > > > > > + void UnregisterBatteryProvider(object provider)
> > > > > > > +
> > > > > > > + This unregisters a battery provider. After
> > > > > > > + unregistration, the BatteryProvider1 objects provided
> > > > > > > + by this client are ignored by BlueZ.
> > > > > >
> > > > > > Not sure if you were followinging the monitor patches, for registering
> > > > > > objects we do prefer the ObjectManager style so multiple provider can
> > > > > > be registered at once, also we may want to tight the control of
> > > > > > battery provider with Profile API so we guarantee that the same entity
> > > > > > that handles the profile connection is the one providing the battery
> > > > > > status.
> > > > > It is actually already in ObjectManager style. After the "root path"
> > > > > is registered, the provider can expose many D-Bus objects at once and
> > > > > bluetoothd can detect it.
> > > > > About tying it with the Profile API, I will take a look at how it could be done.
> > > > >
> > > > > >
> > > > > > > +
> > > > > > > +
> > > > > > > +Battery Provider hierarchy
> > > > > > > +==========================
> > > > > > > +
> > > > > > > +Service <client D-Bus address>
> > > > > > > +Interface org.bluez.BatteryProvider1 [experimental]
> > > > > > > +Object path {provider_root}/org/bluez/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > > > > > > +
> > > > > > > +Properties byte Percentage [readonly]
> > > > > > > +
> > > > > > > + The percentage of battery left as an unsigned 8-bit
> > > > > > > + integer.
> > > > > > > +
> > > > > > > + string Source [readonly, optional]
> > > > > > > +
> > > > > > > + Describes where the battery information comes from
> > > > > > > + (e.g. "HFP 1.7", "HID").
> > > > > > > + This property is informational only and may be useful
> > > > > > > + for debugging purposes. The content of this property is
> > > > > > > + a free string, but it is recommended to include the
> > > > > > > + profile name and version to be useful.
> > > > > > > --
> > > > > > > 2.26.2
> > > > > >
> > > > > > Perhaps we should make this use the same interface as we use for the
> > > > > > daemon itself (Battery1) and the Source there as well. Last but not
> > > > > > least, have you explored the idea of exporting the battery status over
> > > > > > uHID? If I got this right this would aggregate the status of different
> > > > > > sources and then make the daemon expose them, which while it works for
> > > > > > now it means that upper layer have different interfaces for handling a
> > > > > > battery status of something plugged over USB and over Bluetooth.
> > > > > I am okay with naming the interface Battery1, the same as the daemon.
> > > > > Will make an update.
> > > > > About the exporting battery status via UHID, it is possible to be
> > > > > done, but it is an orthogonal problem that we plan to tackle
> > > > > separately. Right now, this API is general enough for Chrome OS to
> > > > > allow both HFP and HID batteries to be consolidated in BlueZ. Chrome
> > > > > OS's powerd feeds only Bluetooth battery levels from
> > > > > /sys/class/power_supply and filters out USB devices so the UI layer
> > > > > does not need to worry about it: everything from BlueZ is tied to a
> > > > > Bluetooth device.
> > > >
> > > > But how about devices pushing their battery status over HID reports
> > > > instead of GATT? Afaik this is possible since the HID device may have
> > > > support for USB (wired) as well where it exposes its battery status
> > > > over HID it may just choose to continue doing so while connected over
> > > > Bluetooth.
> > > If the Bluetooth device reports battery status via HID, it will go
> > > into sys/class/power_supply. In Chrome OS, powerd sends this back to
> > > BlueZ because it knows the Bluetooth device address from
> > > /sys/class/power_supply/ file name.
> >
> > Well that is something Id like to avoid since if we do in the future
> > create an HID report this sort of logic could cause duplications as it
> > either the daemon or the kernel may start parsing these so we will
> > need some way to identify each source (perhaps by UUID) to avoid
> > duplications. Also having differences in upower and powerd really
> > doesn't help us unifying the handling here, I hope at least for
> > Bluetooth we end up with something similar when either daemons are
> > used, otherwise we may need plugins for each separately.
> Duplication is something that we have to deal with, regardless whether
> it's about HID or anything else. As I originally proposed and
> reflected in the code, we start with the simplest duplicate
> resolution: accepting the first one who registered and ignores the
> rest. From BlueZ point of view, clients can feed battery information
> and it handles the duplication. From external clients point of view,
> they do not know whether there is already another source (provider
> from HFP does not know that there is already another provider from
> FastPair, for example) so what they do is feed what they have and let
> BlueZ handle the duplication. With this behavior defined, I don't
> understand why it becomes necessary for BlueZ to extract HID battery
> information directly? I *do* think it is a good idea, but I don't
> understand why it (extracting HID battery directly by BlueZ) is a
> prerequisite to have the battery provider API in place.
>
> >
> > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz