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: <20200817212554.3844965-1-yudiliu@google.com> <20200817142545.RFC.v2.1.Ibaa1dfd49179a141c19a651f3c2132a28b71b344@changeid>
 <CABBYNZLRqhB6d5Hm7_RqvUqCJo11D4+pgYLCeWj0j=8dZk0Bcg@mail.gmail.com>
 <CAHC-ybxshy5zmkAku6suicvjO0Ngofp0+gRyYLX_RXr9YhU94Q@mail.gmail.com>
 <CAHC-ybxFY_3HWrvnwF5DS-i-YJtYpUuR9K3-MSWJWo=C-CKQHA@mail.gmail.com>
 <CALWDO_Wsyx3s3SwBejAFdc6SFX=V29DnvPKmo48hd-yy9SqHSg@mail.gmail.com>
 <CABBYNZJxjYUvui7e-S4TGx21W2U9QvmXhyKNOaWg3RLR4jWg2A@mail.gmail.com> <CALWDO_XV+Hn-U+SgAe_tCcpST0NAo+UjRCBbdS8dzBQZuUCXEg@mail.gmail.com>
In-Reply-To: <CALWDO_XV+Hn-U+SgAe_tCcpST0NAo+UjRCBbdS8dzBQZuUCXEg@mail.gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 1 Sep 2020 13:47:48 -0700
Message-ID: <CABBYNZJfCMxAKkNvnEUs3vH+XWQLYtyM6-Q9nrB1HPE+tDxKCw@mail.gmail.com>
Subject: Re: [RFC PATCH v2 1/1] adapter - D-Bus API for querying the adapter's capabilities
To:     Alain Michaud <alainmichaud@google.com>
Cc:     Yu Liu <yudiliu@google.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@chromium.org>,
        Archie Pusaka <apusaka@chromium.org>,
        Sonny Sasaka <sonnysasaka@chromium.org>,
        Marcel Holtmann <marcel@holtmann.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Alain,

On Tue, Sep 1, 2020 at 12:00 PM Alain Michaud <alainmichaud@google.com> wrote:
>
> Hi Luiz,
>
>
> On Tue, Sep 1, 2020 at 1:47 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Alain,
> >
> > On Tue, Sep 1, 2020 at 8:43 AM Alain Michaud <alainmichaud@google.com> wrote:
> > >
> > > Hi Marcel/Luiz,
> > >
> > > I'd be particularly interested in seeing your opinion on whether this could be better described as some form of SCO socket property... even if this is indeed an adapter property.
> >
> > Yep, but wasn't that supposed to be what BT_PKT_STATUS is for? I mean
> > one can just attempt to read it with getsockopt and in case of returns
> > an error it means the controller does not support it, that said it
> > doesn't look like we do check the adapter features when using
> > BT_PKT_STATUS, should that be checking if HCI_WIDEBAND_SPEECH_ENABLED
> > is set?
> The problem here is that this will be after the connection is created
> and a packet is exchanged.  In this context, we'd like to know if the
> controller supports it ahead of creating a sco connection so we can
> choose to use the headset at all.  For example, there are devices and
> circumstances where using the device's built-in mic and A2DP will
> yield a better experience for the user so the platform may choose that
> as a default for the user rather than degrading down to narrow-band.

You can use get/setsockopt after bind, so I wonder if that is really a
problem here, in fact BT_VOICE wouldn't work if we couldn't use socket
options before the connection is made see how it is used in ofono for
instance:

https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/src/handsfree-audio.c#n105

Also in order to use WBS one will need to use BT_VOICE anyway,
actually that all you need for WBS with mSBC while BT_PKT_STATUS will
indicate the possibility to do PLC, not sure why we are mixing these
things together or does HFP mandates mSBC to implement PLC nowadays?

> >
> > Also it doesn't seem we have updated userspace to support
> > BT_PKT_STATUS, we should probably have something to test it via
> > isotest and perhaps create a iso-tester.c to validate all the options.
> >
> > > Thanks,
> > > Alain
> > >
> > > On Mon, Aug 31, 2020 at 5:44 PM Yu Liu <yudiliu@google.com> wrote:
> > >>
> > >> +Alain Michaud
> > >>
> > >> Hi Marcel,
> > >>
> > >> Can you please comment on the cl as well as Luiz's suggestion? Thanks.
> > >>
> > >> On Thu, Aug 20, 2020 at 10:20 AM Yu Liu <yudiliu@google.com> wrote:
> > >> >
> > >> > Friendly ping for comments from Marcel. Thanks.
> > >> >
> > >> >
> > >> > On Mon, Aug 17, 2020 at 4:17 PM Luiz Augusto von Dentz
> > >> > <luiz.dentz@gmail.com> wrote:
> > >> > >
> > >> > > Hi Marcel,
> > >> > >
> > >> > > On Mon, Aug 17, 2020 at 4:07 PM Yu Liu <yudiliu@google.com> wrote:
> > >> > > >
> > >> > > > From: Archie Pusaka <apusaka@chromium.org>
> > >> > > >
> > >> > > > Initially this is introduced to query whether WBS is supported by the adapter,
> > >> > > > the API is generic enough to be extended to support querying others in
> > >> > > > the future.
> > >> > > >
> > >> > > > Reviewed-by: sonnysasaka@chromium.org
> > >> > > >
> > >> > > > Signed-off-by: Yu Liu <yudiliu@google.com>
> > >> > > > ---
> > >> > > >
> > >> > > > Changes in v2:
> > >> > > > - Return an array of strings instead of a dict
> > >> > > >
> > >> > > > Changes in v1:
> > >> > > > - Initial change
> > >> > > >
> > >> > > >  doc/adapter-api.txt | 12 ++++++++++++
> > >> > > >  1 file changed, 12 insertions(+)
> > >> > > >
> > >> > > > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> > >> > > > index 1a7255750..8fbcadb54 100644
> > >> > > > --- a/doc/adapter-api.txt
> > >> > > > +++ b/doc/adapter-api.txt
> > >> > > > @@ -204,6 +204,18 @@ Methods            void StartDiscovery()
> > >> > > >                                          org.bluez.Error.NotReady
> > >> > > >                                          org.bluez.Error.Failed
> > >> > > >
> > >> > > > +               array{string} GetCapabilities()
> > >> > > > +
> > >> > > > +                       This method returns a list of supported
> > >> > > > +                       capabilities that is populated when the adapter
> > >> > > > +                       initiated.
> > >> > > > +
> > >> > > > +                       Possible values:
> > >> > > > +                               "wbs" - Wide band speech
> > >> > >
> > >> > > Btw, should we stick to use wbs terminology here, or we should
> > >> > > actually use the HCI feature/command, because wbs has actually to be
> > >> > > implemented by the HFP afaik this is only indicating that the
> > >> > > controller is able to notify packets drops, etc, with use of erroneous
> > >> > > command. Perhaps we should actually use the term PLC (Packet Loss
> > >> > > Concealment) instead since that seems to be the real capability here,
> > >> > > afaik WBS does not actually require PLC.
> > >> > >
> > >> > > > +
> > >> > > > +                       Possible errors: org.bluez.Error.NotReady
> > >> > > > +                                        org.bluez.Error.Failed
> > >> > > > +
> > >> > > >  Properties     string Address [readonly]
> > >> > > >
> > >> > > >                         The Bluetooth device address.
> > >> > > > --
> > >> > > > 2.28.0.220.ged08abb693-goog
> > >> > > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz


-- 
Luiz Augusto von Dentz