Re: [PATCH BlueZ 2/2] src/profile.c: Add a GetProfileInfo method

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mark,

On Thu, Aug 20, 2020 at 3:10 AM Mark Marshall
<mark.marshall@xxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, 2020-08-19 at 11:37 -0700, Luiz Augusto von Dentz wrote:
> > Hi Mark,
> >
> > On Wed, Aug 19, 2020 at 8:13 AM Mark Marshall
> > <mark.marshall@xxxxxxxxxxxxxxxxx> wrote:
> > >
> > > Add a GetProfileInfo method to org.bluez.ProfileManager1
> > > ---
> > >  doc/profile-api.txt | 13 +++++++
> > >  src/profile.c       | 93 +++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 106 insertions(+)
> > >
> > > diff --git a/doc/profile-api.txt b/doc/profile-api.txt
> > > index 8c7d0a06d..d13703ab4 100644
> > > --- a/doc/profile-api.txt
> > > +++ b/doc/profile-api.txt
> > > @@ -133,6 +133,19 @@ Object path        /org/bluez
> > >
> > >                         Possible errors: org.bluez.Error.DoesNotExist
> > >
> > > +               options GetProfileInfo(object profile, object adapter)
> > > +
> > > +                       This returns a dictionary of options for the
> > > +                       profile.  Values returned are: UUID, Name,
> > > +                       Path, Service, Mode and AddressType.  The
> > > +                       adapter parameter is optional - if it is
> > > +                       non-empty, then two additional values might be
> > > +                       returned, if the profile is active on the
> > > +                       specified adapter: PSM and Channel.
> > > +
> > > +                       Possible errors: org.bluez.Error.InvalidArguments
> > > +                                        org.bluez.Error.DoesNotExist
> > > +
> >
> > If this is really required I would be willing to merge something like this:
> >
> > https://github.com/Vudentz/BlueZ/commit/9e196f8830511a4102e990d82d06c2e0487b3ad9
> >
> > It exposes service objects so you can control exactly what gets
> > connect, though now given a second look at this seem to return details
> > that the client can query directly on the socket itself like the
> > Channel, PSM, Mode, etc, also not sure what is the point on retrieving
> > things like UUID, Name, Path if the application is already in control
> > of these when registering.
> >
>
> I only returned a full dictionary of items here as I was trying to
> match RegisterProfile, I don't actually need all of this info.  (I
> also thought that the more verbose structure was more useful, but
> it is wasteful).
>
> The information that I really need is the PSM (or Channel) number.
> This information is needed on the server side, and the socket used
> is not exposed to anything outside of bluetoothd, as far as I can
> tell.  (This is the socket returned from bt_io_listen, in profile.c).

The file descriptor passed on NewConnection is the same socket the
daemon uses so you can query things like PSM, etc, using getsockopt
just like the daemon does, or do you need it before a connection is
made? In which case it might be preferable to write the PSM back to
the Property.Set method, that said perhaps it is better to have a PSM
set instead of leaving that to be auto allocated since you may not
want to change if you need to expose over GATT.

> I have my profile auto-select a free PSM, which I thought was the
> preferred method.  In the BR/EDR case, this number gets passed to the
> peer (I assume through SDP), but in the LE case, where I want to use
> LE-L2CAP, there is no defined mechanism to transfer the PSM.  My plan
> was to expose it as a GATT attribute, but this mechanism is not
> standardised, so I didn't think BlueZ would want to be involved?

BlueZ don't need to get involved directly but you can have vendor
specific GATT service that expose the PSM so the remote side can learn
about it and connect.

> Would a simpler interface that just returned the PSM or Channel number
> be better?  Is there another way to find out the PSM of a listening
> socket, on the server, before a connection is made?

For auto allocation I would make actually make the properties
writable, though it seems we don't have any properties on Profile1 so
we would need to create then in order to reflect the options that can
be passed over on RegisterProfile.

> (From my reading of the above patch, the service object is created
> as a link between a "profile" and a "device".  In my case, I think
> there is no service object yet, as I have no device connection?)



>
> > >  Profile hierarchy
> > >  =================
> > > diff --git a/src/profile.c b/src/profile.c
> > > index 10850f305..e287a66d7 100644
> > > --- a/src/profile.c
> > > +++ b/src/profile.c
> > > @@ -2509,6 +2509,96 @@ static DBusMessage *unregister_profile(DBusConnection *conn,
> > >         return dbus_message_new_method_return(msg);
> > >  }
> > >
> > > +static DBusMessage *get_profile_info(DBusConnection *conn,
> > > +                                       DBusMessage *msg, void *user_data)
> > > +{
> > > +       DBusMessage *reply;
> > > +       DBusMessageIter iter, dict;
> > > +       const char *path, *adapter, *sender;
> > > +       struct ext_profile *ext;
> > > +       uint16_t u16;
> > > +       GSList *l, *next;
> > > +
> > > +       sender = dbus_message_get_sender(msg);
> > > +
> > > +       DBG("sender %s", sender);
> > > +
> > > +       if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> > > +                                  DBUS_TYPE_OBJECT_PATH, &adapter,
> > > +                                  DBUS_TYPE_INVALID)) {
> > > +               return btd_error_invalid_args(msg);
> > > +       }
> > > +
> > > +       if (adapter && !*adapter)
> > > +               adapter = NULL;
> > > +
> > > +       ext = find_ext_profile(sender, path);
> > > +       if (!ext)
> > > +               return btd_error_does_not_exist(msg);
> > > +
> > > +       reply = dbus_message_new_method_return(msg);
> > > +
> > > +       dbus_message_iter_init_append(reply, &iter);
> > > +
> > > +       dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> > > +                                        "{sv}", &dict);
> > > +
> > > +       g_dbus_dict_append_entry(&dict, "UUID", DBUS_TYPE_STRING,
> > > +                                &ext->uuid);
> > > +       if (ext->name) {
> > > +               g_dbus_dict_append_entry(&dict, "Name", DBUS_TYPE_STRING,
> > > +                                        &ext->name);
> > > +       }
> > > +       if (ext->path) {
> > > +               g_dbus_dict_append_entry(&dict, "Path", DBUS_TYPE_STRING,
> > > +                                        &ext->path);
> > > +       }
> > > +       if (ext->service) {
> > > +               g_dbus_dict_append_entry(&dict, "Service", DBUS_TYPE_STRING,
> > > +                                        &ext->service);
> > > +       }
> > > +
> > > +       u16 = ext->mode;
> > > +       g_dbus_dict_append_entry(&dict, "Mode", DBUS_TYPE_UINT16,
> > > +                                &u16);
> > > +
> > > +       u16 = ext->addr_type;
> > > +       g_dbus_dict_append_entry(&dict, "AddressType", DBUS_TYPE_UINT16,
> > > +                                &u16);
> > > +
> > > +       if (adapter) {
> > > +               for (l = ext->servers; l != NULL; l = next) {
> > > +                       struct ext_io *server = l->data;
> > > +                       const char *ctype;
> > > +
> > > +                       DBG("server:%p  %d %d psm:%d chan:%d",
> > > +                           server, server->resolving, server->connected,
> > > +                           server->psm, server->chan);
> > > +
> > > +                       next = g_slist_next(l);
> > > +
> > > +                       if (strcmp(adapter, adapter_get_path(server->adapter)))
> > > +                               continue;
> > > +
> > > +                       if (server->proto == BTPROTO_L2CAP) {
> > > +                               ctype = "PSM";
> > > +                               u16 = server->psm;
> > > +                       } else if (server->proto == BTPROTO_RFCOMM) {
> > > +                               ctype = "Channel";
> > > +                               u16 = server->chan;
> > > +                       } else {
> > > +                               continue;
> > > +                       }
> > > +                       g_dbus_dict_append_entry(
> > > +                               &dict, ctype, DBUS_TYPE_UINT16, &u16);
> > > +               }
> > > +       }
> > > +
> > > +       dbus_message_iter_close_container(&iter, &dict);
> > > +
> > > +       return reply;
> > > +}
> > > +
> > >  static const GDBusMethodTable methods[] = {
> > >         { GDBUS_METHOD("RegisterProfile",
> > >                         GDBUS_ARGS({ "profile", "o"}, { "UUID", "s" },
> > > @@ -2516,6 +2606,9 @@ static const GDBusMethodTable methods[] = {
> > >                         NULL, register_profile) },
> > >         { GDBUS_METHOD("UnregisterProfile", GDBUS_ARGS({ "profile", "o" }),
> > >                         NULL, unregister_profile) },
> > > +       { GDBUS_METHOD("GetProfileInfo",
> > > +                       GDBUS_ARGS({ "profile", "o" }, { "adapter", "o" }),
> > > +                       GDBUS_ARGS({ "options", "a{sv}" }), get_profile_info) },
> > >         { }
> > >  };
> > >
> > > --
> > > 2.17.1
> > >
> >
> >
>


-- 
Luiz Augusto von Dentz



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux