| Message ID | 2303f692-bb7c-4851-86fa-befde45b4b32@EXC04-ATKLA.omicron.at (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | Add support for LE profiles (LE-L2CAP) | expand |
Hi Mark, On Wed, Aug 19, 2020 at 8:13 AM Mark Marshall <mark.marshall@omicronenergy.com> 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. > 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 >
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@omicronenergy.com> 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). 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? 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? (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 > > > >
Hi Mark, On Thu, Aug 20, 2020 at 3:10 AM Mark Marshall <mark.marshall@omicronenergy.com> 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@omicronenergy.com> 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 > > > > > > > >
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@omicronenergy.com> 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@omicronenergy.com> 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. 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? > > > 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 > > > > > > > > > > > >
Hi Mark, On Mon, Aug 24, 2020 at 1:53 AM Mark Marshall <mark.marshall@omicronenergy.com> 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@omicronenergy.com> 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@omicronenergy.com> 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 > > > > > > > > > > > > > > > > > >
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 + 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) }, { } };