Hi Mark, On Mon, Aug 24, 2020 at 1:53 AM Mark Marshall <mark.marshall@xxxxxxxxxxxxxxxxx> wrote: > > On Thu, 2020-08-20 at 09:33 -0700, Luiz Augusto von Dentz wrote: > > 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. > > The problem is that NewConnection is too late. This only gets called > after the remote side has connected. The reason that I don't think it > can be a property, is that the PSM is technically a "per adapter" > number, it's possible (I think) for BlueZ to auto-allocate a > different PSM for each adapter. Right, though I think we would allocate the same PSM and bind to BDADDR_ANY. > If I do choose a fixed PSM, are there chances of a collision? > Would such a collision matter? I have "complete" control over all > software on the server, but the client devices are not mine. > What is the best advice about picking a fixed PSM? If you have full control then assigning a fixed PSM is probably the best thing you can do here as you can guarantee no one else will be attempting to register a service for the same PSM. > > > > > 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
