Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201120205728.339325-1-sonnysasaka@chromium.org>
 <20201120205728.339325-4-sonnysasaka@chromium.org> <CABBYNZLsGs5KDC8kGY4ahfA1Bmdhz1XaR9kNRBKNxN7dTZoFHw@mail.gmail.com>
 <CAO271m=Bsz8NXtHzjAn5iYxMs0hxd17DW9QCUnXZACywTDVcvg@mail.gmail.com> <CABBYNZ+ZtmfNxQRd1Zai__htG=xhg+iz-LZw=ktn3b49PwP1xg@mail.gmail.com>
In-Reply-To: <CABBYNZ+ZtmfNxQRd1Zai__htG=xhg+iz-LZw=ktn3b49PwP1xg@mail.gmail.com>
From:   Sonny Sasaka <sonnysasaka@chromium.org>
Date:   Tue, 24 Nov 2020 17:20:39 -0800
Message-ID: <CAO271mmNvaFuuG9Sv8EHZ6+giYvom8FfWCmQJvJTUfgmsJ3GjA@mail.gmail.com>
Subject: Re: [PATCH BlueZ v3 4/7] doc: Add Battery Provider API doc
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        Miao-chen Chou <mcchou@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Luiz,

On Tue, Nov 24, 2020 at 4:23 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sonny,
>
> On Tue, Nov 24, 2020 at 1:30 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
> >
> > Hi Luiz,
> >
> >
> > On Tue, Nov 24, 2020 at 1:21 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Fri, Nov 20, 2020 at 1:00 PM Sonny Sasaka <sonnysasaka@chromium.org> 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 <mcchou@chromium.org>
> > > >
> > > > ---
> > > > Changes in v3:
> > > > * Remove doc duplication in BatteryProvider1 and mention that it's the
> > > >   same as Battery1 instead.
> > > > * Suggest profile UUID in Source property.
> > > >
> > > >  doc/battery-api.txt | 49 +++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 49 insertions(+)
> > > >
> > > > diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> > > > index dc7dbeda2..b5c9a7be1 100644
> > > > --- a/doc/battery-api.txt
> > > > +++ b/doc/battery-api.txt
> > > > @@ -12,3 +12,52 @@ 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
> > > > +                       This property is informational only and may be useful
> > > > +                       for debugging purposes.
> > > > +                       Providers from BatteryProvider1 may make use of this
> > > > +                       property to indicate where the battery report comes from
> > > > +                       (e.g. "HFP 1.7", "HID", or the profile UUID).
> > >
> > > We might need to remove the version number here since there is no
> > > equivalent on UUID, in fact friendly names may be a bad idea after all
> > > since for new profiles we may not have a friendly name to do the
> > > translation and since this is property that would be hard to notify
> > > the provider that we don't understand what is the Source while UUIDs,
> > > if well formatted, should not have this problem so Id just get rid of
> > > the use of friendly names altogether and expect the Source to be a
> > > 128bits UUID in string format.
> > Ack. Will change to 128bit UUID format.
> > >
> > > > +
> > > > +
> > > > +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.
> > > > +The objects and their properties exposed by battery providers will be reflected
> > > > +on org.bluez.Battery1 interface.
> > > > +
> > > > +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.
> > >
> > > We should probably mention this expects an object implementing
> > > ObjectManaged in order to list the Battery1 provider.
> > Ack. Will add more explanation.
> > >
> > > > +               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
> > >
> > > If this is on the client the object path does not necessarily need to
> > > follow our object hierarchy.
> > We need a convention to match the exposed object by the battery
> > provider and BlueZ's device. I am suggesting that the simplest
> > convention is to use the same path of the BlueZ's device object, which
> > is easy to follow and implement by providers. Otherwise, we would
> > still need another convention to match them, but I think any other
> > convention is likely more complex to implement by battery providers.
> > Can you suggest an alternative convention to match the battery and the
> > device?
>
> I thought the objects would be registered directly on the device
> object itself but it looks like it is on the adapter thus why you need
> the device in the first place, but if you are using the object path it
> is just a matter of moving BatteryProviderManager1 to the device
> object.
If we move BatteryProviderManager1 to the device object, that means we
can't use the object manager style and providers have to register each
battery once rather than registering once in the beginning and expose
several objects afterwards, so this would lose your suggestion to use
object manager in the first place. I prefer we stick to using object
manager style, it is simple, easy to understand and implement for
providers (refer to my python test app in one of the patches in this
series).

>
> > >
> > > > +
> > > > +Properties     Objects provided on this interface contain the same properties
> > > > +               as org.bluez.Battery1 interface.
> > > > --
> > > > 2.26.2
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz