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: <20201111011745.2016-1-sonnysasaka@chromium.org> <20201111011745.2016-4-sonnysasaka@chromium.org>
In-Reply-To: <20201111011745.2016-4-sonnysasaka@chromium.org>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Mon, 16 Nov 2020 16:16:47 -0800
Message-ID: <CABBYNZJkyOBT63aAYEc4CtCJtqaQEW3bLyyrJ=XJQ_EpeDDmuQ@mail.gmail.com>
Subject: Re: [PATCH BlueZ v2 4/7] doc: Add Battery Provider API doc
To:     Sonny Sasaka <sonnysasaka@chromium.org>
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 Sonny,

On Tue, Nov 10, 2020 at 5:22 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>
>
> ---
>  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