Hi Iulia, On Thu, Jan 23, 2025 at 8:23 AM Iulia Tanasescu <iulia.tanasescu@xxxxxxx> wrote: > > Since the transport.select command should also work for transports > created by audio servers, the transport should not be required to > be associated with a local bluetoothctl endpoint, to avoid errors > like below: We may want to expand the documentation though to state the above in the Select documentation: https://github.com/bluez/bluez/blob/master/doc/org.bluez.MediaTransport.rst > [bluetoothctl]> scan on > [bluetoothctl]> [NEW] Device 1C:F1:FA:E7:B0:3F 1C-F1-FA-E7-B0-3F > [1C-F1-FA-E7-B0-3F]> [NEW] Transport > /org/bluez/hci0/dev_1C_F1_FA_E7_B0_3F/bis1/fd0 > [1C-F1-FA-E7-B0-3F]> [NEW] Transport > /org/bluez/hci0/dev_1C_F1_FA_E7_B0_3F/bis2/fd1 > [1C-F1-FA-E7-B0-3F]> transport.select > /org/bluez/hci0/dev_1C_F1_FA_E7_B0_3F/bis1/fd0 > /org/bluez/hci0/dev_1C_F1_FA_E7_B0_3F/bis2/fd > Local endpoint not found > > This reworks transport.select to use a dedicated structure to hold > information about the transport and its links, instead of using the > local endpoint. > --- > client/player.c | 160 +++++++++++++++++++++++------------------------- > 1 file changed, 77 insertions(+), 83 deletions(-) > > diff --git a/client/player.c b/client/player.c > index 464a9cc14..e58b42bec 100644 > --- a/client/player.c > +++ b/client/player.c > @@ -4,7 +4,7 @@ > * BlueZ - Bluetooth protocol stack for Linux > * > * Copyright (C) 2020 Intel Corporation. All rights reserved. > - * Copyright 2023-2024 NXP > + * Copyright 2023-2025 NXP > * > * > */ > @@ -115,8 +115,6 @@ struct endpoint { > uint8_t iso_group; > uint8_t iso_stream; > struct queue *acquiring; > - struct queue *links; > - struct queue *selecting; > struct queue *transports; > DBusMessage *msg; > struct preset *preset; > @@ -150,8 +148,14 @@ struct transport { > int num; > }; > > -static void transport_set_links(struct endpoint *ep, GDBusProxy *proxy); > -static void transport_select(GDBusProxy *proxy); > +struct transport_select_args { > + GDBusProxy *proxy; > + struct queue *links; > + struct queue *selecting; > +}; > + > +static void transport_set_links(struct transport_select_args *args); > +static void transport_select(struct transport_select_args *args); > > static void endpoint_unregister(void *data) > { > @@ -2923,8 +2927,6 @@ static void endpoint_free(void *data) > free(ep->preset); > > queue_destroy(ep->acquiring, NULL); > - queue_destroy(ep->links, NULL); > - queue_destroy(ep->selecting, NULL); > queue_destroy(ep->transports, free); > > g_free(ep->path); > @@ -4891,28 +4893,45 @@ static void acquire_reply(DBusMessage *message, void *user_data) > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > +static void free_transport_select_args(struct transport_select_args *args) > +{ > + queue_destroy(args->links, NULL); > + queue_destroy(args->selecting, NULL); > + g_free(args); > +} > + > static void select_reply(DBusMessage *message, void *user_data) > { > DBusError error; > - struct endpoint *ep = user_data; > + struct transport_select_args *args = user_data; > + GDBusProxy *link; > > dbus_error_init(&error); > > if (dbus_set_error_from_message(&error, message) == TRUE) { > bt_shell_printf("Failed to select: %s\n", error.name); > dbus_error_free(&error); > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > bt_shell_printf("Select successful\n"); > > - if (queue_isempty(ep->selecting)) { > + if (queue_isempty(args->selecting)) { > /* All links have been selected */ > - queue_destroy(ep->selecting, NULL); > - ep->selecting = NULL; > - > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_SUCCESS); > } > + > + /* Select next link */ > + link = queue_pop_head(args->selecting); > + if (link) { > + args->proxy = link; > + transport_select(args); > + } else { > + free_transport_select_args(args); > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > + } Seems like you could in fact remove the queue_isempty and only leave queue_pop_head since if that is empty it would result in NULL link anyway. > } > > static void unselect_reply(DBusMessage *message, void *user_data) > @@ -5174,22 +5193,23 @@ static void cmd_acquire_transport(int argc, char *argv[]) > > static void set_bcode_cb(const DBusError *error, void *user_data) > { > - GDBusProxy *proxy = user_data; > + struct transport_select_args *args = user_data; > > if (dbus_error_is_set(error)) { > bt_shell_printf("Failed to set broadcast code: %s\n", > error->name); > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > bt_shell_printf("Setting broadcast code succeeded\n"); > > - transport_select(proxy); > + transport_select(args); > } > > static void set_bcode(const char *input, void *user_data) > { > - GDBusProxy *proxy = user_data; > + struct transport_select_args *args = user_data; > char *bcode; > > if (!strcasecmp(input, "n") || !strcasecmp(input, "no")) > @@ -5197,47 +5217,39 @@ static void set_bcode(const char *input, void *user_data) > else > bcode = g_strdup(input); > > - if (g_dbus_proxy_set_property_dict(proxy, "QoS", > + if (g_dbus_proxy_set_property_dict(args->proxy, "QoS", > set_bcode_cb, user_data, > NULL, "BCode", DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, > strlen(bcode), bcode, NULL) == FALSE) { > bt_shell_printf("Setting broadcast code failed\n"); > g_free(bcode); > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > g_free(bcode); > } > > -static void transport_select(GDBusProxy *proxy) > +static void transport_select(struct transport_select_args *args) > { > - struct endpoint *ep; > - GDBusProxy *link; > - > - ep = find_ep_by_transport(g_dbus_proxy_get_path(proxy)); > - if (!ep) > - return bt_shell_noninteractive_quit(EXIT_FAILURE); > - > - if (!g_dbus_proxy_method_call(proxy, "Select", NULL, > - select_reply, ep, NULL)) { > + if (!g_dbus_proxy_method_call(args->proxy, "Select", NULL, > + select_reply, args, NULL)) { > bt_shell_printf("Failed select transport\n"); > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > - > - /* Select next link */ > - link = queue_pop_head(ep->selecting); > - if (link) > - transport_select(link); > } > > -static void transport_set_bcode(GDBusProxy *proxy) > +static void transport_set_bcode(struct transport_select_args *args) > { > DBusMessageIter iter, array, entry, value; > unsigned char encryption; > const char *key; > > - if (g_dbus_proxy_get_property(proxy, "QoS", &iter) == FALSE) > + if (g_dbus_proxy_get_property(args->proxy, "QoS", &iter) == FALSE) { > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > + } > > dbus_message_iter_recurse(&iter, &array); > > @@ -5253,7 +5265,7 @@ static void transport_set_bcode(GDBusProxy *proxy) > if (encryption == 1) { > bt_shell_prompt_input("", > "Enter brocast code[value/no]:", > - set_bcode, proxy); > + set_bcode, args); > return; > } > break; > @@ -5264,7 +5276,7 @@ static void transport_set_bcode(GDBusProxy *proxy) > /* Go straight to selecting transport, if Broadcast Code > * is not required. > */ > - transport_select(proxy); > + transport_select(args); > } > > static void transport_unselect(GDBusProxy *proxy, bool prompt) > @@ -5278,58 +5290,52 @@ static void transport_unselect(GDBusProxy *proxy, bool prompt) > > static void set_links_cb(const DBusError *error, void *user_data) > { > - GDBusProxy *proxy = user_data; > - const char *path = g_dbus_proxy_get_path(proxy); > - struct endpoint *ep; > + struct transport_select_args *args = user_data; > GDBusProxy *link; > > - ep = find_ep_by_transport(path); > - if (!ep) { > - bt_shell_printf("Local endpoint not found\n"); > - return bt_shell_noninteractive_quit(EXIT_FAILURE); > - } > - > - link = queue_pop_head(ep->links); > + link = queue_pop_head(args->links); > > - if (queue_isempty(ep->links)) { > - queue_destroy(ep->links, NULL); > - ep->links = NULL; > + if (queue_isempty(args->links)) { > + queue_destroy(args->links, NULL); > + args->links = NULL; > } > > if (dbus_error_is_set(error)) { > bt_shell_printf("Failed to set link %s: %s\n", > g_dbus_proxy_get_path(link), > error->name); > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > bt_shell_printf("Successfully linked transport %s\n", > g_dbus_proxy_get_path(link)); > > - if (!ep->selecting) > - ep->selecting = queue_new(); > + if (!args->selecting) > + args->selecting = queue_new(); > > /* Enqueue link to mark that it is ready to be selected */ > - queue_push_tail(ep->selecting, link); > + queue_push_tail(args->selecting, link); > > /* Continue setting the remanining links */ > - transport_set_links(ep, proxy); > + transport_set_links(args); > } > > -static void transport_set_links(struct endpoint *ep, GDBusProxy *proxy) > +static void transport_set_links(struct transport_select_args *args) > { > GDBusProxy *link; > const char *path; > > - link = queue_peek_head(ep->links); > + link = queue_peek_head(args->links); > if (link) { > path = g_dbus_proxy_get_path(link); > > - if (g_dbus_proxy_set_property_array(proxy, "Links", > + if (g_dbus_proxy_set_property_array(args->proxy, "Links", > DBUS_TYPE_OBJECT_PATH, > &path, 1, set_links_cb, > - proxy, NULL) == FALSE) { > + args, NULL) == FALSE) { > bt_shell_printf("Linking transport %s failed\n", path); > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > @@ -5339,28 +5345,17 @@ static void transport_set_links(struct endpoint *ep, GDBusProxy *proxy) > /* If all links have been set, check is transport requires the > * user to provide a Broadcast Code. > */ > - transport_set_bcode(proxy); > -} > - > -static void endpoint_set_links(struct endpoint *ep) > -{ > - GDBusProxy *proxy = queue_pop_head(ep->links); > - > - if (!proxy) { > - bt_shell_printf("No transport to set links for\n"); > - return bt_shell_noninteractive_quit(EXIT_FAILURE); > - } > - > - transport_set_links(ep, proxy); > + transport_set_bcode(args); > } > > static void cmd_select_transport(int argc, char *argv[]) > { > GDBusProxy *link = NULL; > - struct queue *links = queue_new(); > - struct endpoint *ep; > + struct transport_select_args *args; > int i; > > + args = g_new0(struct transport_select_args, 1); > + > for (i = 1; i < argc; i++) { > link = g_dbus_proxy_lookup(transports, NULL, argv[i], > BLUEZ_MEDIA_TRANSPORT_INTERFACE); > @@ -5375,26 +5370,25 @@ static void cmd_select_transport(int argc, char *argv[]) > goto fail; > } > > - /* Enqueue all links */ > - queue_push_tail(links, link); > - } > + if (!args->proxy) { > + args->proxy = link; > + continue; > + } > > - /* Get reference to local endpoint */ > - ep = find_ep_by_transport(g_dbus_proxy_get_path(link)); > - if (!ep) { > - bt_shell_printf("Local endpoint not found\n"); > - goto fail; > - } > + if (!args->links) > + args->links = queue_new(); > > - ep->links = links; > + /* Enqueue all links */ > + queue_push_tail(args->links, link); > + } > > /* Link streams before selecting one by one */ > - endpoint_set_links(ep); > + transport_set_links(args); > > return; > > fail: > - queue_destroy(links, NULL); > + free_transport_select_args(args); > return bt_shell_noninteractive_quit(EXIT_FAILURE); > } > > -- > 2.43.0 > -- Luiz Augusto von Dentz
