Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200401221434.12185-1-sonnysasaka@chromium.org>
 <ACB5D50F-BAD5-435A-AD1C-58A16878C310@holtmann.org> <CAOxioNnBq3J_F63RNHZfQzKOcwSmVy2igJ6zCfgBH+zcL1kaCw@mail.gmail.com>
In-Reply-To: <CAOxioNnBq3J_F63RNHZfQzKOcwSmVy2igJ6zCfgBH+zcL1kaCw@mail.gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 9 Apr 2020 11:35:14 -0700
Message-ID: <CABBYNZKnoUoA=01o1fZL25=Pvmq-L8VZehPB80p9fErm=GF+Gg@mail.gmail.com>
Subject: Re: [PATCH 1/5] D-Bus API changes for managing SDP records
To:     Sonny Sasaka <sonnysasaka@chromium.org>
Cc:     Marcel Holtmann <marcel@holtmann.org>,
        Bluez mailing list <linux-bluetooth@vger.kernel.org>,
        Rahul Chaturvedi <rkc@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Sonny,

On Thu, Apr 9, 2020 at 11:28 AM Sonny Sasaka <sonnysasaka@chromium.org> wro=
te:
>
> Hi Marcel,
>
> This was needed to support emulation of Android apps's Bluetooth API
> on top of BlueZ. Android expects to be able to access the SDP database
> of a remote device via its HAL interface
> (https://android.googlesource.com/platform/system/bt/+/master/include/har=
dware/bt_sdp.h#38).

I suppose you could read the cache directly, though there is no
guarantee that we won't change the format of that, btw I suppose
services handled by the daemon should not be exposed to the Android
side otherwise you may have conflicts so going with Profile interface
is probably the way to go, but you will need to know the UUIDs that
would be handled by Android.

> Thanks for the feedback, we will revisit this and see if a redesign
> based on your suggestion is possible.
>
> On Thu, Apr 9, 2020 at 11:05 AM Marcel Holtmann <marcel@holtmann.org> wro=
te:
> >
> > Hi Sonny,
> >
> > > This defines the DBus API that we'll use with BlueZ to create, remove
> > > and get service records.
> > > ---
> > > doc/adapter-api.txt | 46 ++++++++++++++++++++++++++++++++++++++++++++=
+
> > > doc/device-api.txt  | 37 ++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 83 insertions(+)
> > >
> > > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> > > index acae032d9..6e4c37fc9 100644
> > > --- a/doc/adapter-api.txt
> > > +++ b/doc/adapter-api.txt
> > > @@ -204,6 +204,52 @@ Methods          void StartDiscovery()
> > >                                        org.bluez.Error.NotReady
> > >                                        org.bluez.Error.Failed
> > >
> > > +             uint32 CreateServiceRecord(dict record)
> > > +
> > > +                     This method creates an entry with the local SDP=
 server
> > > +                     for this adapter for the specified record. This=
 method
> > > +                     will only create the SDP record and not start l=
istening
> > > +                     on any ports. It is up to the caller of the met=
hod to
> > > +                     ensure the validity of the service record. This=
 record
> > > +                     will not be parsed for any validation but will =
instead
> > > +                     directly be inserted into the local SDP server=
=E2=80=99s
> > > +                     records.
> > > +
> > > +                     The return value from this method will be the 3=
2 bit
> > > +                     handle for the created service record.
> > > +
> > > +                     The record dictionary will have dictionary entr=
ies of
> > > +                     the format: {id : (type, size, value)}, where,
> > > +
> > > +                     uint16 id:      The 16 bit attribute ID for an
> > > +                                     attribute.
> > > +                     uint8 type:     This will contain the type of t=
he
> > > +                                     attribute value. Attribute type=
 values
> > > +                                     are defined in the Bluetooth sp=
ec in
> > > +                                     Volume 3, Part B, 3.2.
> > > +                     uint32 size:    This is the size of the attribu=
te
> > > +                                     value.
> > > +                     variant value:  This will contain the attribute=
 value
> > > +                                     for a given attribute_id. This =
variant
> > > +                                     can either contain a primitive =
type, or
> > > +                                     if type is SEQUENCE, an array o=
f struct
> > > +                                     of the signature (type, size, v=
alue).
> > > +
> > > +                     Possible errors: org.bluez.Error.NotReady
> > > +                                      org.bluez.Error.AlreadyExists
> > > +                                      org.bluez.Error.Failed
> > > +                                      org.bluez.Error.InvalidArgumen=
ts
> > > +
> > > +             void RemoveServiceRecord(uint32 handle)
> > > +
> > > +                     This method removes the SDP record with the giv=
en
> > > +                     handle from the local SDP server.
> > > +
> > > +                     Possible errors: org.bluez.Error.NotReady
> > > +                                      org.bluez.Error.DoesNotExist
> > > +                                      org.bluez.Error.Failed
> > > +                                      org.bluez.Error.InvalidArgumen=
ts
> > > +
> >
> > so when design the BlueZ 5.x APIs, we on purpose didn=E2=80=99t do this=
. You are suppose to use doc/profile-api.txt for these kind of things.
> >
> > I am not in favor of dangling SDP records where we have no lifetime gua=
rantee of the service behind it.
> >
> > If you look at profiles/iap/main.c then you see how you could write a v=
endor profile just as easily. That one is the skeleton for iOS accessory pr=
otocol over Bluetooth.
> >
> > > Properties    string Address [readonly]
> > >
> > >                       The Bluetooth device address.
> > > diff --git a/doc/device-api.txt b/doc/device-api.txt
> > > index ceb68d2f6..e8f2c670d 100644
> > > --- a/doc/device-api.txt
> > > +++ b/doc/device-api.txt
> > > @@ -120,6 +120,43 @@ Methods          void Connect()
> > >                       Possible errors: org.bluez.Error.DoesNotExist
> > >                                        org.bluez.Error.Failed
> > >
> > > +             array{array{dict}} GetServiceRecords()
> > > +
> > > +                     This method returns the complete service record=
s of all
> > > +                     discovered BR/EDR services of the connected dev=
ice till
> > > +                     now. The return value will be an array of an ar=
ray of
> > > +                     dictionary entries. Each nested array of dictio=
nary
> > > +                     entries will contain one service record. Each p=
air in
> > > +                     the returned dictionary entries will represent =
an
> > > +                     attribute in the service record.
> > > +
> > > +                     The dictionary entries in the returned array of=
 entries
> > > +                     will be of the format: {id : (type, size, value=
)} where,
> > > +
> > > +                     uint16 id:      The 16 bit attribute ID for an
> > > +                                     attribute.
> > > +                     uint8 type:     This will contain the type of t=
he
> > > +                                     attribute value. Attribute type=
 values
> > > +                                     are defined in the Bluetooth sp=
ec in
> > > +                                     Volume 3, Part B, 3.2.
> > > +                     uint32 size:    This is the size of the attribu=
te
> > > +                                     value.
> > > +                     variant value:  This will contain the attribute=
 value
> > > +                                     for a given attribute_id. This =
variant
> > > +                                     can either contain a primitive =
type, or
> > > +                                     if type is SEQUENCE, an array o=
f struct
> > > +                                     of the signature (type, size, v=
alue).
> > > +
> > > +                     Since the service record contains the service U=
UID and
> > > +                     name, these fields do not need to be given sepa=
rately.
> > > +                     This method should be called after the ServiceR=
esolved
> > > +                     property for this object has been set to true o=
therwise
> > > +                     it may return a partial or stale cached list.
> > > +
> > > +                     Possible errors: org.bluez.Error.NotReady
> > > +                                      org.bluez.Error.NotConnected
> > > +                                      org.bluez.Error.Failed
> > > +
> >
> > These two things are not related and do not belong in the same patch. H=
owever what is your goal behind this API since even here we tried not to gi=
ve raw SDP to the application. It is all baked into the Profile API.
> >
> > Regards
> >
> > Marcel
> >


--=20
Luiz Augusto von Dentz