Message ID | 20250123132331.27435-2-iulia.tanasescu@nxp.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | client/player: Rework transport.select | expand |
Context | Check | Description |
---|---|---|
tedd_an/pre-ci_am | success | Success |
tedd_an/BuildEll | success | Build ELL PASS |
tedd_an/BluezMake | success | Bluez Make PASS |
tedd_an/MakeCheck | success | Bluez Make Check PASS |
tedd_an/MakeDistcheck | success | Make Distcheck PASS |
tedd_an/CheckValgrind | success | Check Valgrind PASS |
tedd_an/CheckSmatch | success | CheckSparse PASS |
tedd_an/bluezmakeextell | success | Make External ELL PASS |
tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=927816 ---Test result--- Test Summary: CheckPatch PENDING 0.28 seconds GitLint PENDING 0.19 seconds BuildEll PASS 20.51 seconds BluezMake PASS 1480.72 seconds MakeCheck PASS 14.62 seconds MakeDistcheck PASS 162.65 seconds CheckValgrind PASS 220.38 seconds CheckSmatch PASS 278.74 seconds bluezmakeextell PASS 100.87 seconds IncrementalBuild PENDING 0.29 seconds ScanBuild PASS 899.07 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
Hi Iulia, On Thu, Jan 23, 2025 at 8:23 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> 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 >
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); + } } 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); }