* [PATCH BlueZ 0/2] shared: recover from failed input initialization @ 2026-02-18 2:32 Ronan Pigott 2026-02-18 2:32 ` [PATCH BlueZ 1/2] zsh: amend completions Ronan Pigott 2026-02-18 2:33 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Ronan Pigott 0 siblings, 2 replies; 17+ messages in thread From: Ronan Pigott @ 2026-02-18 2:32 UTC (permalink / raw) To: linux-bluetooth; +Cc: Ronan Pigott This fixes an issue where bluetoothctl stopped printing values outside of the shell. I reported this in [1] and bisected the error to commit e73bf582dae60356641a32fc27ae03d359ec4c47. My motivation is to keep the zsh completions functional, so I have also included a few small fixes for the zsh completions. I saw that another patch was submitted which unfortunately is not a satisfactory resolution, since it causes bluetoothctl to print all the extraneous lines present in the interactive session for non-interactive sessions as well. So I have submitted this suggestion instead. [1] https://github.com/bluez/bluez/issues/1896 Ronan Pigott (2): zsh: amend completions shared/shell: gracefully recover from failed input initialization completion/zsh/_bluetoothctl | 133 ++++++++++++++++------------------- src/shared/shell.c | 23 ++++-- 2 files changed, 79 insertions(+), 77 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH BlueZ 1/2] zsh: amend completions 2026-02-18 2:32 [PATCH BlueZ 0/2] shared: recover from failed input initialization Ronan Pigott @ 2026-02-18 2:32 ` Ronan Pigott 2026-02-18 3:37 ` shared: recover from failed input initialization bluez.test.bot 2026-02-18 14:56 ` [PATCH BlueZ 1/2] zsh: amend completions Luiz Augusto von Dentz 2026-02-18 2:33 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Ronan Pigott 1 sibling, 2 replies; 17+ messages in thread From: Ronan Pigott @ 2026-02-18 2:32 UTC (permalink / raw) To: linux-bluetooth; +Cc: Ronan Pigott First, use the correct completion return value. The return value of a completion function is significant, if we fail to return success additional completers may be invoked when they otherwise should not be. Also cleanup up the zsh completion, removing the redundant definition of _bluetoothctl and using the _call_program helper where appropriate. Finally, update the bluetoothctl command invocations to account for the media lines printed after some of the non-interactive commands. --- completion/zsh/_bluetoothctl | 133 ++++++++++++++++------------------- src/shared/shell.c | 4 ++ 2 files changed, 66 insertions(+), 71 deletions(-) diff --git a/completion/zsh/_bluetoothctl b/completion/zsh/_bluetoothctl index 610ca2b8d59c..b6f513376532 100644 --- a/completion/zsh/_bluetoothctl +++ b/completion/zsh/_bluetoothctl @@ -1,97 +1,88 @@ #compdef bluetoothctl -__bluetoothctl() { - bluetoothctl "$@" 2>/dev/null -} - _bluezcomp_controller() { local -a controllers - bluetoothctl list | - while read _ MAC NAME; do - controllers+="${MAC//:/\\:}:${NAME//:/\\:}" + _call_program bluez-controller bluetoothctl list | + while read KIND MAC NAME FLAG; do + [[ $KIND == Controller ]] && + controllers+=("${MAC//:/\\:}:${NAME//:/\\:}") done _describe -t controllers 'controller' controllers } _bluezcomp_device() { local -a devices - bluetoothctl devices | - while read _ MAC NAME; do - devices+="${MAC//:/\\:}:${NAME//:/\\:}" + _call_program bluez-device bluetoothctl devices | + while read KIND MAC NAME; do + [[ $KIND == Device ]] && + devices+=("${MAC//:/\\:}:${NAME//:/\\:}") done _describe -t devices 'device' devices } _bluezcomp_agentcap() { - local -a agent_options=(${(f)"$(__bluetoothctl agent help)"}) - agent_options=( "${(@)agent_options:#(on|off)}" ) - compadd -a agent_options + local -a agent_options=(${${(@f)"$(_call_program bluez-agent bluetoothctl agent help)"}:#(on|off)}) + compadd "$@" - -a agent_options } _bluetoothctl_agent() { - local -a agent_options=(${(f)"$(__bluetoothctl agent help)"}) - agent_options+=help - compadd -a agent_options + local -a agent_options=(help ${(@f)"$(_call_program bluez-agent bluetoothctl agent help)"}) + compadd "$@" - -a agent_options } -_bluetoothctl_advertise() { - local -a ad_options=(${(f)"$(__bluetoothctl advertise help)"}) - ad_options+=help - compadd -a ad_options -} +local -a toggle_commands=( + "discoverable" "pairable" "power" "scan" +) -_bluetoothctl() { - local -a toggle_commands=( - "discoverable" "pairable" "power" "scan" - ) +local -a controller_commands=( + "select" "show" +) - local -a controller_commands=( - "select" "show" - ) +local -a device_commands=( + "block" "connect" "disconnect" "info" + "pair" "remove" "trust" "unblock" "untrust" +) - local -a device_commands=( - "block" "connect" "disconnect" "info" - "pair" "remove" "trust" "unblock" "untrust" - ) +# Other commands may be handled by _bluetoothctl_$command +local -a all_commands=( "${(@f)$(_call_program bluetoothctl bluetoothctl --zsh-complete help)}" ) - # Other commands may be handled by _bluetoothctl_$command - local -a all_commands=( "${(@f)$(__bluetoothctl --zsh-complete help)}" ) +local curcontext=$curcontext state line ret=1 +_arguments -C \ + + '(info)' \ + {-h,--help}'[Show help message and exit]' \ + {-v,--version}'--version[Show version info and exit]' \ + + 'mod' \ + '(info)'{-a+,--agent=}'[Register agent handler]:agent:_bluezcomp_agentcap' \ + '(info)'{-t,--timeout}'[Timeout in seconds for non-interactive mode]' \ + '(info)'{-m,--monitor}'[Enable monitor output]' \ + + 'command' \ + '(info):command:->command' \ + '(info):: :->argument' - local curcontext=$curcontext state line ret=1 - _arguments -C \ - + '(info)' \ - {-h,--help}'[Show help message and exit]' \ - {-v,--version}'--version[Show version info and exit]' \ - + 'mod' \ - '(info)'{-a+,--agent=}'[Register agent handler]:agent:_bluezcomp_agentcap' \ - '(info)'{-t,--timeout}'[Timeout in seconds for non-interactive mode]' \ - '(info)'{-m,--monitor}'[Enable monitor output]' \ - + 'command' \ - '(info):command:->command' \ - '(info):: :->argument' - - if [[ $state == "command" ]]; then - _describe -t commands 'command' all_commands - elif [[ $state == "argument" ]]; then - if (( ! ${"${(@)all_commands%%:*}"[(I)${line[1]}]} )); then - _message "Unknown bluetoothctl command: $line[1]" - return 1; - fi - - curcontext="${curcontext%:*:*}:bluetoothctl-$line[1]:" - if ! _call_function ret _bluetoothctl_$line[1]; then - case $line[1] in - (${(~j.|.)toggle_commands}) - compadd on off - ;; - (${(~j.|.)device_commands}) - _bluezcomp_device - ;; - (${(~j.|.)controller_commands}) - _bluezcomp_controller - ;; - esac - fi - return ret +if [[ $state == "command" ]]; then + _describe -t commands 'command' all_commands +elif [[ $state == "argument" ]]; then + if (( ! ${"${(@)all_commands%%:*}"[(I)${line[1]}]} )); then + _message "Unknown bluetoothctl command: $line[1]" + return 1; fi -} && _bluetoothctl + + curcontext="${curcontext%:*:*}:bluetoothctl-$line[1]:" + if ! _call_function ret _bluetoothctl_$line[1]; then + case $line[1] in + (advertise) + compadd - help on off type && ret=0 + ;; + (${(~j.|.)toggle_commands}) + compadd on off && ret=0 + ;; + (${(~j.|.)device_commands}) + _bluezcomp_device && ret=0 + ;; + (${(~j.|.)controller_commands}) + _bluezcomp_controller && ret=0 + ;; + esac + fi + return ret +fi diff --git a/src/shared/shell.c b/src/shared/shell.c index f014d8f7c2b2..873a14176af9 100644 --- a/src/shared/shell.c +++ b/src/shared/shell.c @@ -1737,6 +1737,10 @@ int bt_shell_get_timeout(void) void bt_shell_handle_non_interactive_help(void) { + if (data.zsh) { + shell_print_menu_zsh_complete(); + exit(EXIT_SUCCESS); + } if (!data.mode) return; if (data.argv[0] != cmplt) -- 2.53.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: shared: recover from failed input initialization 2026-02-18 2:32 ` [PATCH BlueZ 1/2] zsh: amend completions Ronan Pigott @ 2026-02-18 3:37 ` bluez.test.bot 2026-02-18 14:56 ` [PATCH BlueZ 1/2] zsh: amend completions Luiz Augusto von Dentz 1 sibling, 0 replies; 17+ messages in thread From: bluez.test.bot @ 2026-02-18 3:37 UTC (permalink / raw) To: linux-bluetooth, ronan [-- Attachment #1: Type: text/plain, Size: 1880 bytes --] 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=1055060 ---Test result--- Test Summary: CheckPatch PENDING 0.38 seconds GitLint PENDING 0.37 seconds BuildEll PASS 20.74 seconds BluezMake PASS 644.05 seconds MakeCheck PASS 18.38 seconds MakeDistcheck PASS 244.93 seconds CheckValgrind PASS 293.77 seconds CheckSmatch WARNING 359.08 seconds bluezmakeextell PASS 183.92 seconds IncrementalBuild PENDING 0.41 seconds ScanBuild PASS 1014.70 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: CheckSmatch - WARNING Desc: Run smatch tool with source Output: src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h): ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH BlueZ 1/2] zsh: amend completions 2026-02-18 2:32 ` [PATCH BlueZ 1/2] zsh: amend completions Ronan Pigott 2026-02-18 3:37 ` shared: recover from failed input initialization bluez.test.bot @ 2026-02-18 14:56 ` Luiz Augusto von Dentz 1 sibling, 0 replies; 17+ messages in thread From: Luiz Augusto von Dentz @ 2026-02-18 14:56 UTC (permalink / raw) To: Ronan Pigott; +Cc: linux-bluetooth Hi Ronan, On Tue, Feb 17, 2026 at 9:46 PM Ronan Pigott <ronan@rjp.ie> wrote: > > First, use the correct completion return value. > > The return value of a completion function is significant, if we fail to > return success additional completers may be invoked when they otherwise > should not be. > > Also cleanup up the zsh completion, removing the redundant definition of > _bluetoothctl and using the _call_program helper where appropriate. > > Finally, update the bluetoothctl command invocations to account for the > media lines printed after some of the non-interactive commands. > --- > completion/zsh/_bluetoothctl | 133 ++++++++++++++++------------------- > src/shared/shell.c | 4 ++ > 2 files changed, 66 insertions(+), 71 deletions(-) > > diff --git a/completion/zsh/_bluetoothctl b/completion/zsh/_bluetoothctl > index 610ca2b8d59c..b6f513376532 100644 > --- a/completion/zsh/_bluetoothctl > +++ b/completion/zsh/_bluetoothctl > @@ -1,97 +1,88 @@ > #compdef bluetoothctl > > -__bluetoothctl() { > - bluetoothctl "$@" 2>/dev/null > -} > - > _bluezcomp_controller() { > local -a controllers > - bluetoothctl list | > - while read _ MAC NAME; do > - controllers+="${MAC//:/\\:}:${NAME//:/\\:}" > + _call_program bluez-controller bluetoothctl list | > + while read KIND MAC NAME FLAG; do > + [[ $KIND == Controller ]] && > + controllers+=("${MAC//:/\\:}:${NAME//:/\\:}") > done > _describe -t controllers 'controller' controllers > } > > _bluezcomp_device() { > local -a devices > - bluetoothctl devices | > - while read _ MAC NAME; do > - devices+="${MAC//:/\\:}:${NAME//:/\\:}" > + _call_program bluez-device bluetoothctl devices | > + while read KIND MAC NAME; do > + [[ $KIND == Device ]] && > + devices+=("${MAC//:/\\:}:${NAME//:/\\:}") > done > _describe -t devices 'device' devices > } > > _bluezcomp_agentcap() { > - local -a agent_options=(${(f)"$(__bluetoothctl agent help)"}) > - agent_options=( "${(@)agent_options:#(on|off)}" ) > - compadd -a agent_options > + local -a agent_options=(${${(@f)"$(_call_program bluez-agent bluetoothctl agent help)"}:#(on|off)}) > + compadd "$@" - -a agent_options > } > > _bluetoothctl_agent() { > - local -a agent_options=(${(f)"$(__bluetoothctl agent help)"}) > - agent_options+=help > - compadd -a agent_options > + local -a agent_options=(help ${(@f)"$(_call_program bluez-agent bluetoothctl agent help)"}) > + compadd "$@" - -a agent_options > } > > -_bluetoothctl_advertise() { > - local -a ad_options=(${(f)"$(__bluetoothctl advertise help)"}) > - ad_options+=help > - compadd -a ad_options > -} > +local -a toggle_commands=( > + "discoverable" "pairable" "power" "scan" > +) > > -_bluetoothctl() { > - local -a toggle_commands=( > - "discoverable" "pairable" "power" "scan" > - ) > +local -a controller_commands=( > + "select" "show" > +) > > - local -a controller_commands=( > - "select" "show" > - ) > +local -a device_commands=( > + "block" "connect" "disconnect" "info" > + "pair" "remove" "trust" "unblock" "untrust" > +) > > - local -a device_commands=( > - "block" "connect" "disconnect" "info" > - "pair" "remove" "trust" "unblock" "untrust" > - ) > +# Other commands may be handled by _bluetoothctl_$command > +local -a all_commands=( "${(@f)$(_call_program bluetoothctl bluetoothctl --zsh-complete help)}" ) > > - # Other commands may be handled by _bluetoothctl_$command > - local -a all_commands=( "${(@f)$(__bluetoothctl --zsh-complete help)}" ) > +local curcontext=$curcontext state line ret=1 > +_arguments -C \ > + + '(info)' \ > + {-h,--help}'[Show help message and exit]' \ > + {-v,--version}'--version[Show version info and exit]' \ > + + 'mod' \ > + '(info)'{-a+,--agent=}'[Register agent handler]:agent:_bluezcomp_agentcap' \ > + '(info)'{-t,--timeout}'[Timeout in seconds for non-interactive mode]' \ > + '(info)'{-m,--monitor}'[Enable monitor output]' \ > + + 'command' \ > + '(info):command:->command' \ > + '(info):: :->argument' > > - local curcontext=$curcontext state line ret=1 > - _arguments -C \ > - + '(info)' \ > - {-h,--help}'[Show help message and exit]' \ > - {-v,--version}'--version[Show version info and exit]' \ > - + 'mod' \ > - '(info)'{-a+,--agent=}'[Register agent handler]:agent:_bluezcomp_agentcap' \ > - '(info)'{-t,--timeout}'[Timeout in seconds for non-interactive mode]' \ > - '(info)'{-m,--monitor}'[Enable monitor output]' \ > - + 'command' \ > - '(info):command:->command' \ > - '(info):: :->argument' > - > - if [[ $state == "command" ]]; then > - _describe -t commands 'command' all_commands > - elif [[ $state == "argument" ]]; then > - if (( ! ${"${(@)all_commands%%:*}"[(I)${line[1]}]} )); then > - _message "Unknown bluetoothctl command: $line[1]" > - return 1; > - fi > - > - curcontext="${curcontext%:*:*}:bluetoothctl-$line[1]:" > - if ! _call_function ret _bluetoothctl_$line[1]; then > - case $line[1] in > - (${(~j.|.)toggle_commands}) > - compadd on off > - ;; > - (${(~j.|.)device_commands}) > - _bluezcomp_device > - ;; > - (${(~j.|.)controller_commands}) > - _bluezcomp_controller > - ;; > - esac > - fi > - return ret > +if [[ $state == "command" ]]; then > + _describe -t commands 'command' all_commands > +elif [[ $state == "argument" ]]; then > + if (( ! ${"${(@)all_commands%%:*}"[(I)${line[1]}]} )); then > + _message "Unknown bluetoothctl command: $line[1]" > + return 1; > fi > -} && _bluetoothctl > + > + curcontext="${curcontext%:*:*}:bluetoothctl-$line[1]:" > + if ! _call_function ret _bluetoothctl_$line[1]; then > + case $line[1] in > + (advertise) > + compadd - help on off type && ret=0 > + ;; > + (${(~j.|.)toggle_commands}) > + compadd on off && ret=0 > + ;; > + (${(~j.|.)device_commands}) > + _bluezcomp_device && ret=0 > + ;; > + (${(~j.|.)controller_commands}) > + _bluezcomp_controller && ret=0 > + ;; > + esac > + fi > + return ret > +fi > diff --git a/src/shared/shell.c b/src/shared/shell.c > index f014d8f7c2b2..873a14176af9 100644 > --- a/src/shared/shell.c > +++ b/src/shared/shell.c > @@ -1737,6 +1737,10 @@ int bt_shell_get_timeout(void) > > void bt_shell_handle_non_interactive_help(void) > { > + if (data.zsh) { > + shell_print_menu_zsh_complete(); > + exit(EXIT_SUCCESS); > + } I suggest splitting these changes, so shared should go in its own patch with a proper explanation why it has to be done this way. > if (!data.mode) > return; > if (data.argv[0] != cmplt) > -- > 2.53.0 > > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization 2026-02-18 2:32 [PATCH BlueZ 0/2] shared: recover from failed input initialization Ronan Pigott 2026-02-18 2:32 ` [PATCH BlueZ 1/2] zsh: amend completions Ronan Pigott @ 2026-02-18 2:33 ` Ronan Pigott 2026-02-18 14:58 ` Luiz Augusto von Dentz 1 sibling, 1 reply; 17+ messages in thread From: Ronan Pigott @ 2026-02-18 2:33 UTC (permalink / raw) To: linux-bluetooth; +Cc: Ronan Pigott If input initialization fails, back out before waiting on events that will not come. In turn, we can go back to initializing inputs for non-interactive invocations, which fixes printing for non-interactive commands and the zsh completions that rely on it. This effectively reverts commit e73bf582dae60356641a32fc27ae03d359ec4c47. --- src/shared/shell.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/shared/shell.c b/src/shared/shell.c index 873a14176af9..b9fcc665fcae 100644 --- a/src/shared/shell.c +++ b/src/shared/shell.c @@ -1640,15 +1640,22 @@ static bool shell_quit(void *data) bool bt_shell_attach(int fd) { + struct input *input; + + input = input_new(fd); + if (!input) + return false; + if (data.mode == MODE_INTERACTIVE) { - struct input *input; - - input = input_new(fd); - if (!input) + if (!io_set_read_handler(input->io, input_read, input, NULL)) { + input_destroy(input->io); return false; + } - io_set_read_handler(input->io, input_read, input, NULL); - io_set_disconnect_handler(input->io, input_hup, input, NULL); + if (!io_set_disconnect_handler(input->io, input_hup, input, NULL)) { + input_destroy(input->io); + return false; + } if (data.init_fd >= 0) { int fd = data.init_fd; -- 2.53.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization 2026-02-18 2:33 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Ronan Pigott @ 2026-02-18 14:58 ` Luiz Augusto von Dentz 2026-02-18 17:45 ` Ronan Pigott 0 siblings, 1 reply; 17+ messages in thread From: Luiz Augusto von Dentz @ 2026-02-18 14:58 UTC (permalink / raw) To: Ronan Pigott; +Cc: linux-bluetooth Hi Ronan, On Tue, Feb 17, 2026 at 9:46 PM Ronan Pigott <ronan@rjp.ie> wrote: > > If input initialization fails, back out before waiting on events that > will not come. In turn, we can go back to initializing inputs for > non-interactive invocations, which fixes printing for non-interactive > commands and the zsh completions that rely on it. > > This effectively reverts commit e73bf582dae60356641a32fc27ae03d359ec4c47. Yeah, the revert part doesn't really fly since e73bf582dae60356641a32fc27ae03d359ec4c47 does fixes a valid issue, that said we do have another proposal under discussion: https://patchwork.kernel.org/project/bluetooth/patch/20260217222954.432676-1-larsch@belunktum.dk/ > --- > src/shared/shell.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/src/shared/shell.c b/src/shared/shell.c > index 873a14176af9..b9fcc665fcae 100644 > --- a/src/shared/shell.c > +++ b/src/shared/shell.c > @@ -1640,15 +1640,22 @@ static bool shell_quit(void *data) > > bool bt_shell_attach(int fd) > { > + struct input *input; > + > + input = input_new(fd); > + if (!input) > + return false; > + > if (data.mode == MODE_INTERACTIVE) { > - struct input *input; > - > - input = input_new(fd); > - if (!input) > + if (!io_set_read_handler(input->io, input_read, input, NULL)) { > + input_destroy(input->io); > return false; > + } > > - io_set_read_handler(input->io, input_read, input, NULL); > - io_set_disconnect_handler(input->io, input_hup, input, NULL); > + if (!io_set_disconnect_handler(input->io, input_hup, input, NULL)) { > + input_destroy(input->io); > + return false; > + } > > if (data.init_fd >= 0) { > int fd = data.init_fd; > -- > 2.53.0 > > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization 2026-02-18 14:58 ` Luiz Augusto von Dentz @ 2026-02-18 17:45 ` Ronan Pigott 2026-02-18 18:45 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 17+ messages in thread From: Ronan Pigott @ 2026-02-18 17:45 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth February 18, 2026 at 7:58 AM, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote: > Yeah, the revert part doesn't really fly since > e73bf582dae60356641a32fc27ae03d359ec4c47 does fixes a valid issue, > that said we do have another proposal under discussion: > > https://patchwork.kernel.org/project/bluetooth/patch/20260217222954.432676-1-larsch@belunktum.dk/ Hi Luiz, I'm aware it fixes a valid issue. My hope with the patch is to fix that same issue in another way. The problem adressed IIUC is that once the input is initialized, a subsequent epoll_wait would hang since the epoll_ctl call had failed to actually add the new event source. The reverted patch had fixed this by trying to avoid initializing stdin in all cases, which inadvertently broke bluetoothctl. My suggestion is to revert that change, and instead check the return value of io_set_read_handler so that we can avoid digging ourselves into a deeper hole. Even if bt_shell_printf is refactored so that it doesn't require the input fd, which sounds prudent, I think it probably makes sense to check the return values here, so I believe the reverted patch would no longer be necessary. I have seen Lars's patch, and that is actually what prompted me to submit this one. Unfortunately, it isn't correct since it causes non-interactive bluetoothctl invocations to also print several extraneous lines relating to the interactive shell. I will submit the zsh changes separately as suggested. Cheers, Ronan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization 2026-02-18 17:45 ` Ronan Pigott @ 2026-02-18 18:45 ` Luiz Augusto von Dentz 2026-02-25 16:01 ` Bastien Nocera 0 siblings, 1 reply; 17+ messages in thread From: Luiz Augusto von Dentz @ 2026-02-18 18:45 UTC (permalink / raw) To: Ronan Pigott; +Cc: linux-bluetooth Hi Ronan, On Wed, Feb 18, 2026 at 12:45 PM Ronan Pigott <ronan@rjp.ie> wrote: > > February 18, 2026 at 7:58 AM, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote: > > > Yeah, the revert part doesn't really fly since > > e73bf582dae60356641a32fc27ae03d359ec4c47 does fixes a valid issue, > > that said we do have another proposal under discussion: > > > > https://patchwork.kernel.org/project/bluetooth/patch/20260217222954.432676-1-larsch@belunktum.dk/ > > Hi Luiz, > > I'm aware it fixes a valid issue. My hope with the patch is to fix that same issue in another way. > > The problem adressed IIUC is that once the input is initialized, a subsequent epoll_wait would hang > since the epoll_ctl call had failed to actually add the new event source. The reverted patch had > fixed this by trying to avoid initializing stdin in all cases, which inadvertently broke > bluetoothctl. My suggestion is to revert that change, and instead check the return value of > io_set_read_handler so that we can avoid digging ourselves into a deeper hole. Even if bt_shell_printf > is refactored so that it doesn't require the input fd, which sounds prudent, I think it probably makes > sense to check the return values here, so I believe the reverted patch would no longer be necessary. > > I have seen Lars's patch, and that is actually what prompted me to submit this one. Unfortunately, > it isn't correct since it causes non-interactive bluetoothctl invocations to also print several > extraneous lines relating to the interactive shell. Feel free to git a review to Lars's patch; hopefully, that will help us get this resolved quicker. We might as well create a unit test for shell to try to emulate different modes and environments. That way, we prevent introducing regressions like this in the future. > I will submit the zsh changes separately as suggested. > > Cheers, > > Ronan -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization 2026-02-18 18:45 ` Luiz Augusto von Dentz @ 2026-02-25 16:01 ` Bastien Nocera 2026-02-25 18:58 ` Integration testing for BlueZ Pauli Virtanen 2026-02-27 11:31 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Bastien Nocera 0 siblings, 2 replies; 17+ messages in thread From: Bastien Nocera @ 2026-02-25 16:01 UTC (permalink / raw) To: Luiz Augusto von Dentz, Ronan Pigott; +Cc: linux-bluetooth [-- Attachment #1: Type: text/plain, Size: 3122 bytes --] On Wed, 2026-02-18 at 13:45 -0500, Luiz Augusto von Dentz wrote: > Hi Ronan, > > On Wed, Feb 18, 2026 at 12:45 PM Ronan Pigott <ronan@rjp.ie> wrote: > > > > February 18, 2026 at 7:58 AM, "Luiz Augusto von Dentz" > > <luiz.dentz@gmail.com> wrote: > > > > > Yeah, the revert part doesn't really fly since > > > e73bf582dae60356641a32fc27ae03d359ec4c47 does fixes a valid > > > issue, > > > that said we do have another proposal under discussion: > > > > > > https://patchwork.kernel.org/project/bluetooth/patch/20260217222954.432676-1-larsch@belunktum.dk/ > > > > Hi Luiz, > > > > I'm aware it fixes a valid issue. My hope with the patch is to fix > > that same issue in another way. > > > > The problem adressed IIUC is that once the input is initialized, a > > subsequent epoll_wait would hang > > since the epoll_ctl call had failed to actually add the new event > > source. The reverted patch had > > fixed this by trying to avoid initializing stdin in all cases, > > which inadvertently broke > > bluetoothctl. My suggestion is to revert that change, and instead > > check the return value of > > io_set_read_handler so that we can avoid digging ourselves into a > > deeper hole. Even if bt_shell_printf > > is refactored so that it doesn't require the input fd, which sounds > > prudent, I think it probably makes > > sense to check the return values here, so I believe the reverted > > patch would no longer be necessary. > > > > I have seen Lars's patch, and that is actually what prompted me to > > submit this one. Unfortunately, > > it isn't correct since it causes non-interactive bluetoothctl > > invocations to also print several > > extraneous lines relating to the interactive shell. > > Feel free to git a review to Lars's patch; hopefully, that will help > us get this resolved quicker. We might as well create a unit test for > shell to try to emulate different modes and environments. That way, > we > prevent introducing regressions like this in the future. I wrote one, which I can integrate into meson. This starts "btvirt" (so requires root), launches bluetoothd if there isn't a daemon already running, and launches "bluetoothctl list". You can run it manually with: $ sudo top_builddir=/path/to/bluez/builddir/ ./integration-test.py If "bluetoothctl list" doesn't output any text, the test errors out. I've tested that pointing the bluetoothctl_path() output at an old version of bluetoothctl worked. This pattern of using Python to create test suites using python- dbusmock is something I've already used in PolicyKit, upower, power- profiles-daemon, gnome-bluetooth and many other places. This test is pretty heavy-handed if we just want to test whether bluetoothctl outputs things correctly, but it has the benefit of testing the emulator as well as bluetoothd. We could start adding more tests to the mix, such as creating more adapters, pairing them, etc. Hopefully, this is a useful test to run on CIs, although I'm probably missing spawning a system bus on top of everything else. What do you think? [-- Attachment #2: bluez-btvirt-integration-test.py --] [-- Type: text/x-python3, Size: 4574 bytes --] #!/usr/bin/python3 # Copyright: (C) 2025 Bastien Nocera <hadess@hadess.net> # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by # the Free Software Foundation; either version 2 of the License, or # (at your option) any later version. # # This program is distributed in the hope that it will be useful, # but WITHOUT ANY WARRANTY; without even the implied warranty of # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. import os import sys import subprocess import unittest import time try: import gi from gi.repository import GLib except ImportError as e: sys.stderr.write('Skipping tests, PyGobject not available for Python 3, or missing GI typelibs: %s\n' % str(e)) sys.exit(77) builddir = os.getenv('top_builddir', '.') try: import dbusmock except ImportError: sys.stderr.write('Skipping tests, python-dbusmock not available (http://pypi.python.org/pypi/python-dbusmock).\n') sys.exit(77) class Tests(dbusmock.DBusTestCase): @classmethod def setUpClass(cls): super().setUpClass() cls.start_system_bus() cls.dbus_con = cls.get_dbus(True) @classmethod def tearDownClass(cls): super().tearDownClass() def setUp(self): super().setUp() self.log = None def stop_virt(self): if self.virt: try: self.virt.terminate() except OSError: pass self.assertEqual(self.virt.wait(timeout=3000), 0) self.virt = None def start_virt(self): self.virt = subprocess.Popen( [ self.btvirt_path(), '-l1' ], stdout=self.log, stderr=sys.stderr ) self.addCleanup(self.stop_virt) # time.sleep(1) def stop_daemon(self): if self.daemon: try: self.daemon.terminate() except OSError: pass self.assertEqual(self.daemon.wait(timeout=3000), 0) self.daemon = None def daemon_check(self): daemon_check = subprocess.run(['gdbus', 'introspect', '--system', '--dest', 'org.bluez', '--object-path', '/' ], stdout = subprocess.DEVNULL, check=False, capture_output=False) return daemon_check.returncode == 0 def start_daemon(self): # Only start daemon if one is not already available if not self.daemon_check(): self.daemon = subprocess.Popen( [ self.bluetoothd_path(), '--nodetach' ], stdout=self.log, stderr=sys.stderr ) self.addCleanup(self.stop_daemon) self.assert_eventually(lambda: self.daemon_check()) def bluetoothctl_path(self): builddir = os.getenv("top_builddir", ".") return os.path.join(builddir, "client", "bluetoothctl") def btvirt_path(self): builddir = os.getenv("top_builddir", ".") return os.path.join(builddir, "emulator", "btvirt") def bluetoothd_path(self): builddir = os.getenv("top_builddir", ".") return os.path.join(builddir, "src", "bluetoothd") def assert_eventually(self, condition, message=None, timeout=5000, keep_checking=0): """Assert that condition function eventually returns True. Timeout is in milliseconds, defaulting to 5000 (5 seconds). message is printed on failure. """ if not keep_checking: if condition(): return done = False def on_timeout_reached(): nonlocal done done = True source = GLib.timeout_add(timeout, on_timeout_reached) while not done: if condition(): GLib.source_remove(source) if keep_checking > 0: self.assert_condition_persists( condition, message, timeout=keep_checking ) return GLib.MainContext.default().iteration(False) self.fail(message() if message else f"timed out waiting for {condition}") def test_bluetoothctl(self): self.start_virt() self.start_daemon() cmd = subprocess.run([ self.bluetoothctl_path(), 'list' ], check=False, capture_output=True) self.assertRegex(cmd.stdout.decode('UTF-8'), '.*\\[default\\]') if __name__ == '__main__': unittest.main() ^ permalink raw reply [flat|nested] 17+ messages in thread
* Integration testing for BlueZ 2026-02-25 16:01 ` Bastien Nocera @ 2026-02-25 18:58 ` Pauli Virtanen 2026-02-27 11:37 ` Bastien Nocera 2026-03-30 12:38 ` Bastien Nocera 2026-02-27 11:31 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Bastien Nocera 1 sibling, 2 replies; 17+ messages in thread From: Pauli Virtanen @ 2026-02-25 18:58 UTC (permalink / raw) To: Bastien Nocera, Luiz Augusto von Dentz; +Cc: linux-bluetooth Hi, ke, 2026-02-25 kello 17:01 +0100, Bastien Nocera kirjoitti: [clip] > > Feel free to git a review to Lars's patch; hopefully, that will help > > us get this resolved quicker. We might as well create a unit test for > > shell to try to emulate different modes and environments. That way, > > we > > prevent introducing regressions like this in the future. > > I wrote one, which I can integrate into meson. > > This starts "btvirt" (so requires root), launches bluetoothd if there > isn't a daemon already running, and launches "bluetoothctl list". > > You can run it manually with: > $ sudo top_builddir=/path/to/bluez/builddir/ ./integration-test.py > > If "bluetoothctl list" doesn't output any text, the test errors out. > I've tested that pointing the bluetoothctl_path() output at an old > version of bluetoothctl worked. > > This pattern of using Python to create test suites using python- > dbusmock is something I've already used in PolicyKit, upower, power- > profiles-daemon, gnome-bluetooth and many other places. > > This test is pretty heavy-handed if we just want to test whether > bluetoothctl outputs things correctly, but it has the benefit of > testing the emulator as well as bluetoothd. We could start adding more > tests to the mix, such as creating more adapters, pairing them, etc. > > Hopefully, this is a useful test to run on CIs, although I'm probably > missing spawning a system bus on top of everything else. > > What do you think? Adding some testing for this is a good idea, and I'd think Python is the way to go. I was planning on pushing this a bit further, and automate also end-to- end integration testing. That is, QEmu instances connected to each other via btvirt, so we can have repeatable tests in a "real" environment. This is maybe overkill for simple bluetoothctl command line tests, but it allows things like automated testing of Pipewire <-> Bluez <-> btvirt <-> BlueZ <-> Pipewire audio setup. This involves lots of subsystems, and it's currently 100% relying on manual testing and sometimes things are missed, like breaking A2DP in some setups in 5.86, or breaking BAP in 5.85. Here's a working prototype, needs some more work though so details may change but the general shape is probably going to stay: https://github.com/pv/bluez/blob/func-test/unit/func_test/test_cli_simple.py https://github.com/pv/bluez/blob/func-test/doc/test-functional.rst -- Pauli Virtanen ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Integration testing for BlueZ 2026-02-25 18:58 ` Integration testing for BlueZ Pauli Virtanen @ 2026-02-27 11:37 ` Bastien Nocera 2026-03-30 12:38 ` Bastien Nocera 1 sibling, 0 replies; 17+ messages in thread From: Bastien Nocera @ 2026-02-27 11:37 UTC (permalink / raw) To: Pauli Virtanen, Luiz Augusto von Dentz; +Cc: linux-bluetooth On Wed, 2026-02-25 at 20:58 +0200, Pauli Virtanen wrote: > Hi, > > ke, 2026-02-25 kello 17:01 +0100, Bastien Nocera kirjoitti: > [clip] > > > Feel free to git a review to Lars's patch; hopefully, that will > > > help > > > us get this resolved quicker. We might as well create a unit test > > > for > > > shell to try to emulate different modes and environments. That > > > way, > > > we > > > prevent introducing regressions like this in the future. > > > > I wrote one, which I can integrate into meson. > > > > This starts "btvirt" (so requires root), launches bluetoothd if > > there > > isn't a daemon already running, and launches "bluetoothctl list". > > > > You can run it manually with: > > $ sudo top_builddir=/path/to/bluez/builddir/ ./integration-test.py > > > > If "bluetoothctl list" doesn't output any text, the test errors > > out. > > I've tested that pointing the bluetoothctl_path() output at an old > > version of bluetoothctl worked. > > > > This pattern of using Python to create test suites using python- > > dbusmock is something I've already used in PolicyKit, upower, > > power- > > profiles-daemon, gnome-bluetooth and many other places. > > > > This test is pretty heavy-handed if we just want to test whether > > bluetoothctl outputs things correctly, but it has the benefit of > > testing the emulator as well as bluetoothd. We could start adding > > more > > tests to the mix, such as creating more adapters, pairing them, > > etc. > > > > Hopefully, this is a useful test to run on CIs, although I'm > > probably > > missing spawning a system bus on top of everything else. > > > > What do you think? > > Adding some testing for this is a good idea, and I'd think Python is > the way to go. > > I was planning on pushing this a bit further, and automate also end- > to- > end integration testing. That is, QEmu instances connected to each > other via btvirt, so we can have repeatable tests in a "real" > environment. This is something I've never actually done, but I would definitely be interested in trying out. > This is maybe overkill for simple bluetoothctl command line tests, > but > it allows things like automated testing of Pipewire <-> Bluez <-> > btvirt <-> BlueZ <-> Pipewire audio setup. > > This involves lots of subsystems, and it's currently 100% relying on > manual testing and sometimes things are missed, like breaking A2DP in > some setups in 5.86, or breaking BAP in 5.85. > > Here's a working prototype, needs some more work though so details > may > change but the general shape is probably going to stay: > > https://github.com/pv/bluez/blob/func-test/unit/func_test/test_cli_simple.py > > https://github.com/pv/bluez/blob/func-test/doc/test-functional.rst > I'll probably steal some of those into the tests I worked on, as I've, so far, made it so that one doesn't need root to run any tests if there's a Bluetooth adapter plugged in and bluetoothd is running, although I'm probably going to look into using python-dbusmock's bluetoothd mocking for more bluetoothctl automated testing. Cheers ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Integration testing for BlueZ 2026-02-25 18:58 ` Integration testing for BlueZ Pauli Virtanen 2026-02-27 11:37 ` Bastien Nocera @ 2026-03-30 12:38 ` Bastien Nocera 2026-03-30 12:59 ` Pauli Virtanen 1 sibling, 1 reply; 17+ messages in thread From: Bastien Nocera @ 2026-03-30 12:38 UTC (permalink / raw) To: Pauli Virtanen, Luiz Augusto von Dentz; +Cc: linux-bluetooth On Wed, 2026-02-25 at 20:58 +0200, Pauli Virtanen wrote: > Hi, > > ke, 2026-02-25 kello 17:01 +0100, Bastien Nocera kirjoitti: > [clip] > > > Feel free to git a review to Lars's patch; hopefully, that will > > > help > > > us get this resolved quicker. We might as well create a unit test > > > for > > > shell to try to emulate different modes and environments. That > > > way, > > > we > > > prevent introducing regressions like this in the future. > > > > I wrote one, which I can integrate into meson. > > > > This starts "btvirt" (so requires root), launches bluetoothd if > > there > > isn't a daemon already running, and launches "bluetoothctl list". > > > > You can run it manually with: > > $ sudo top_builddir=/path/to/bluez/builddir/ ./integration-test.py > > > > If "bluetoothctl list" doesn't output any text, the test errors > > out. > > I've tested that pointing the bluetoothctl_path() output at an old > > version of bluetoothctl worked. > > > > This pattern of using Python to create test suites using python- > > dbusmock is something I've already used in PolicyKit, upower, > > power- > > profiles-daemon, gnome-bluetooth and many other places. > > > > This test is pretty heavy-handed if we just want to test whether > > bluetoothctl outputs things correctly, but it has the benefit of > > testing the emulator as well as bluetoothd. We could start adding > > more > > tests to the mix, such as creating more adapters, pairing them, > > etc. > > > > Hopefully, this is a useful test to run on CIs, although I'm > > probably > > missing spawning a system bus on top of everything else. > > > > What do you think? > > Adding some testing for this is a good idea, and I'd think Python is > the way to go. > > I was planning on pushing this a bit further, and automate also end- > to- > end integration testing. That is, QEmu instances connected to each > other via btvirt, so we can have repeatable tests in a "real" > environment. > > This is maybe overkill for simple bluetoothctl command line tests, > but > it allows things like automated testing of Pipewire <-> Bluez <-> > btvirt <-> BlueZ <-> Pipewire audio setup. > > This involves lots of subsystems, and it's currently 100% relying on > manual testing and sometimes things are missed, like breaking A2DP in > some setups in 5.86, or breaking BAP in 5.85. > > Here's a working prototype, needs some more work though so details > may > change but the general shape is probably going to stay: > > https://github.com/pv/bluez/blob/func-test/unit/func_test/test_cli_simple.py Do you still have a copy of the above file somewhere? I wanted to finish porting them into my own integration tests which I'm hoping to add to the regression/unit tests that are usually run for every commit/MR. > > https://github.com/pv/bluez/blob/func-test/doc/test-functional.rst > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Integration testing for BlueZ 2026-03-30 12:38 ` Bastien Nocera @ 2026-03-30 12:59 ` Pauli Virtanen 2026-03-30 15:03 ` Bastien Nocera 0 siblings, 1 reply; 17+ messages in thread From: Pauli Virtanen @ 2026-03-30 12:59 UTC (permalink / raw) To: Bastien Nocera; +Cc: linux-bluetooth Hi, 30. maaliskuuta 2026 12.38.49 UTC Bastien Nocera <hadess@hadess.net> kirjoitti: >On Wed, 2026-02-25 at 20:58 +0200, Pauli Virtanen wrote: >> Hi, >> >> ke, 2026-02-25 kello 17:01 +0100, Bastien Nocera kirjoitti: >> [clip] >> > > Feel free to git a review to Lars's patch; hopefully, that will >> > > help >> > > us get this resolved quicker. We might as well create a unit test >> > > for >> > > shell to try to emulate different modes and environments. That >> > > way, >> > > we >> > > prevent introducing regressions like this in the future. >> > >> > I wrote one, which I can integrate into meson. >> > >> > This starts "btvirt" (so requires root), launches bluetoothd if >> > there >> > isn't a daemon already running, and launches "bluetoothctl list". >> > >> > You can run it manually with: >> > $ sudo top_builddir=/path/to/bluez/builddir/ ./integration-test.py >> > >> > If "bluetoothctl list" doesn't output any text, the test errors >> > out. >> > I've tested that pointing the bluetoothctl_path() output at an old >> > version of bluetoothctl worked. >> > >> > This pattern of using Python to create test suites using python- >> > dbusmock is something I've already used in PolicyKit, upower, >> > power- >> > profiles-daemon, gnome-bluetooth and many other places. >> > >> > This test is pretty heavy-handed if we just want to test whether >> > bluetoothctl outputs things correctly, but it has the benefit of >> > testing the emulator as well as bluetoothd. We could start adding >> > more >> > tests to the mix, such as creating more adapters, pairing them, >> > etc. >> > >> > Hopefully, this is a useful test to run on CIs, although I'm >> > probably >> > missing spawning a system bus on top of everything else. >> > >> > What do you think? >> >> Adding some testing for this is a good idea, and I'd think Python is >> the way to go. >> >> I was planning on pushing this a bit further, and automate also end- >> to- >> end integration testing. That is, QEmu instances connected to each >> other via btvirt, so we can have repeatable tests in a "real" >> environment. >> >> This is maybe overkill for simple bluetoothctl command line tests, >> but >> it allows things like automated testing of Pipewire <-> Bluez <-> >> btvirt <-> BlueZ <-> Pipewire audio setup. >> >> This involves lots of subsystems, and it's currently 100% relying on >> manual testing and sometimes things are missed, like breaking A2DP in >> some setups in 5.86, or breaking BAP in 5.85. >> >> Here's a working prototype, needs some more work though so details >> may >> change but the general shape is probably going to stay: >> >> https://github.com/pv/bluez/blob/func-test/unit/func_test/test_cli_simple.py > >Do you still have a copy of the above file somewhere? I wanted to >finish porting them into my own integration tests which I'm hoping to >add to the regression/unit tests that are usually run for every >commit/MR. https://github.com/pv/bluez/blob/func-test-v3/test/functional/test_bluetoothctl_vm.py and test_btmgmt_vm.py Although these don't currently contain much, but could be extended. FWIW I'd maybe consider using pytest also for Python regression & unit tests, at least if you can make them run without root. There's interesting bluetoothd mock dbus library by the bluez-alsa developer, which looks like it could be useful for bluetoothctl/etc utility testing <https://github.com/Samsung/bluezoo> > >> >> https://github.com/pv/bluez/blob/func-test/doc/test-functional.rst >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Integration testing for BlueZ 2026-03-30 12:59 ` Pauli Virtanen @ 2026-03-30 15:03 ` Bastien Nocera 2026-04-05 12:37 ` Pauli Virtanen 0 siblings, 1 reply; 17+ messages in thread From: Bastien Nocera @ 2026-03-30 15:03 UTC (permalink / raw) To: Pauli Virtanen; +Cc: linux-bluetooth On Mon, 2026-03-30 at 12:59 +0000, Pauli Virtanen wrote: > > Do you still have a copy of the above file somewhere? I wanted to > > finish porting them into my own integration tests which I'm hoping > > to > > add to the regression/unit tests that are usually run for every > > commit/MR. > > https://github.com/pv/bluez/blob/func-test-v3/test/functional/test_bluetoothctl_vm.py > > and test_btmgmt_vm.py > > Although these don't currently contain much, but could be extended. > > FWIW I'd maybe consider using pytest also for Python regression & > unit tests, at least if you can make them run without root. > > There's interesting bluetoothd mock dbus library by the bluez-alsa > developer, which looks like it could be useful for bluetoothctl/etc > utility testing > > <https://github.com/Samsung/bluezoo> I'm pretty familiar with python-dbusmock which includes a bluetoothd mock server which can be used as non-root: https://github.com/martinpitt/python-dbusmock It's used extensively in the gnome-bluetooth test suite: https://gitlab.gnome.org/GNOME/gnome-bluetooth/-/blob/master/tests/integration-test.py?ref_type=heads along with its mock upower implementation. bluezoo looks more like a complete replacement including some behaviour when running certain commands. The python-dbusmock implementation is voluntarily simpler. The questions at the end of the day boils down to *what* we want to test: 1) Just like the gnome-bluetooth test suite, we could only care for our own code, bluetoothd and upower are mocked, no specific hardware or permissions is required. This usually requires tweaking the mock implementation to better match the behaviour of the real thing. 2) As I've implemented in this integration test: https://github.com/hadess/bluez/blob/wip/hadess/add-meson-ell-wrap/unit/integration-test.py we test command-line tools against the real bluetoothd which we compile, it requires either root (to run btvirt if needed, and run bluetoothd), or a machine with a real/mocked hardware and bluetoothd already running. Ideally, with root access, you're testing the "integration" of the command-line tools and bluetoothd. Do we want to run those against a mocked bluetoothd that could get out of sync with reality? This wouldn't allow us to test bluetoothd itself as I've done in one test, but maybe that's good to allow us to run those tests in more circumstances? My goal was being able to run "meson test" on my local development machine without worrying about a layer of possibly outdated mocking code. I'd be fine running those tests that could be against a mocked bluetoothd, if that means more tests being run. I'd expect both those first options to be run for every merge request or patch set, possibly even for every commit if we run into errors. 3) We have your variant of the test suite that does what 2) does but with many more moving parts, and pretty high requirements in terms of setup (and possibly resources). The only thing that's mocked is the Bluetooth adapters, and not even for all tests. In short, I think I might, before sending a patch out, re-focus the basic tools tests to run against a mocked dbusmock bluez, so they always run, whatever the environment. Next, we can try to figure out whether some tests like the bluetoothd startup warning one can be shared between the "run on every merge request" (option 2) vs. "run once a week or once a day" heavier duty tests (option 3). ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Integration testing for BlueZ 2026-03-30 15:03 ` Bastien Nocera @ 2026-04-05 12:37 ` Pauli Virtanen 0 siblings, 0 replies; 17+ messages in thread From: Pauli Virtanen @ 2026-04-05 12:37 UTC (permalink / raw) To: Bastien Nocera; +Cc: linux-bluetooth ma, 2026-03-30 kello 17:03 +0200, Bastien Nocera kirjoitti: > On Mon, 2026-03-30 at 12:59 +0000, Pauli Virtanen wrote: > > > Do you still have a copy of the above file somewhere? I wanted to > > > finish porting them into my own integration tests which I'm hoping > > > to > > > add to the regression/unit tests that are usually run for every > > > commit/MR. > > > > https://github.com/pv/bluez/blob/func-test-v3/test/functional/test_bluetoothctl_vm.py > > > > and test_btmgmt_vm.py > > > > Although these don't currently contain much, but could be extended. > > > > FWIW I'd maybe consider using pytest also for Python regression & > > unit tests, at least if you can make them run without root. > > > > There's interesting bluetoothd mock dbus library by the bluez-alsa > > developer, which looks like it could be useful for bluetoothctl/etc > > utility testing > > > > <https://github.com/Samsung/bluezoo> > > I'm pretty familiar with python-dbusmock which includes a bluetoothd > mock server which can be used as non-root: > https://github.com/martinpitt/python-dbusmock > > It's used extensively in the gnome-bluetooth test suite: > https://gitlab.gnome.org/GNOME/gnome-bluetooth/-/blob/master/tests/integration-test.py?ref_type=heads > along with its mock upower implementation. > > bluezoo looks more like a complete replacement including some behaviour > when running certain commands. The python-dbusmock implementation is > voluntarily simpler. > > The questions at the end of the day boils down to *what* we want to > test: > > 1) Just like the gnome-bluetooth test suite, we could only care for our > own code, bluetoothd and upower are mocked, no specific hardware or > permissions is required. > > This usually requires tweaking the mock implementation to better match > the behaviour of the real thing. > > 2) As I've implemented in this integration test: > https://github.com/hadess/bluez/blob/wip/hadess/add-meson-ell-wrap/unit/integration-test.py > we test command-line tools against the real bluetoothd which we > compile, it requires either root (to run btvirt if needed, and run > bluetoothd), or a machine with a real/mocked hardware and bluetoothd > already running. Ideally, with root access, you're testing the > "integration" of the command-line tools and bluetoothd. > Do we want to run those against a mocked bluetoothd that could get out > of sync with reality? > > This wouldn't allow us to test bluetoothd itself as I've done in one > test, but maybe that's good to allow us to run those tests in more > circumstances? > > My goal was being able to run "meson test" on my local development > machine without worrying about a layer of possibly outdated mocking > code. > > I'd be fine running those tests that could be against a mocked > bluetoothd, if that means more tests being run. > > I'd expect both those first options to be run for every merge request > or patch set, possibly even for every commit if we run into errors. > > 3) We have your variant of the test suite that does what 2) does but > with many more moving parts, and pretty high requirements in terms of > setup (and possibly resources). The only thing that's mocked is the > Bluetooth adapters, and not even for all tests. > > In short, I think I might, before sending a patch out, re-focus the > basic tools tests to run against a mocked dbusmock bluez, so they > always run, whatever the environment. > > Next, we can try to figure out whether some tests like the bluetoothd > startup warning one can be shared between the "run on every merge > request" (option 2) vs. "run once a week or once a day" heavier duty > tests (option 3). Yes, mocked tests make sense only for subset of tests, here I was thinking about the regression tests that check that "--help" works and "bluetoothctl" prints things correctly. Didn't remember dbusmock had stubs for some bluetoothd services. The VM-based tests have some advantages in that the environment is more reproducible, you are able to test against bluetooth-next kernel, and they are more convenient to run on dev workstation as they don't need root or interfere with system services. In principle several of the tests could run both on host kernel with btvirt, and with some extra work you could define a compatible test fixture that can reuse the exact same test code, or just write the tests in a way that the test core can be reused. Although the bluetoothd + kernel stack is written in a way that controllers are independent, the single-host test setup is not exactly the same as multiple hosts, so there's probably some value in end-to-end tests. Performance-wise there is some room for improving the VM test speed, eg. not compiling kernel with lock debugging etc. slow debug options on, and using virtiofs instead of 9p as it appears the emulated 9p disk access is somewhat slow. The VM boot time is already amortized by reusing the VM instances, and bluetoothd etc environment restarts can be avoided where possible, so you could write multiple tests like the - -help one with VM adding mostly a fixed cost. The tests that pair devices first spend time on waiting on the btvirt page timeout which is adding many seconds per tests. So there is also a non-negligible part of test time that is not limited by VM execution speed. For any tests running outside VM, it's probably simplest to write them in such a way that the test core could be reused. E.g. the bluetoothctl completion crash for example like would be def check_bluetoothctl_completion_crash(): child = pexpect.spawn(bluetoothctl) child._decoder = AsciiDecoder() child.expect(r"\[bluetoothctl\]> ") child.sendline(" \t") child.expect(r"\[bluetoothctl\]> ") time.sleep(0.5) assert child.isalive() @bluetoothd_reuse_config def test_bluetoothctl_completion_crash(hosts): (host,) = hosts host.call(check_bluetoothctl_completion_crash) This adds ~0.8 sec to the total test duration in VM (compared to ~0.7 outside VM with system bluetoothd), as the test does not require bluetoothd or VM restart. I'd also write all tests properly as Python package that can be imported elsewhere as needed, and follow the pytest convention that tests are in files named `test_*.py`, so that they and any helper functions are more reusable. If you use pytest for them, you also don't need a runner script. For deciding what to run on CI, probably real runtime numbers are what matters. If the VM tests are fast enough, I'd think it would be OK to run them for all patches, as is done with the kernel VM testers. -- Pauli Virtanen ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization 2026-02-25 16:01 ` Bastien Nocera 2026-02-25 18:58 ` Integration testing for BlueZ Pauli Virtanen @ 2026-02-27 11:31 ` Bastien Nocera 2026-02-28 17:32 ` Ronan Pigott 1 sibling, 1 reply; 17+ messages in thread From: Bastien Nocera @ 2026-02-27 11:31 UTC (permalink / raw) To: Luiz Augusto von Dentz, Ronan Pigott; +Cc: linux-bluetooth On Wed, 2026-02-25 at 17:01 +0100, Bastien Nocera wrote: > On Wed, 2026-02-18 at 13:45 -0500, Luiz Augusto von Dentz wrote: > > Hi Ronan, > > > > On Wed, Feb 18, 2026 at 12:45 PM Ronan Pigott <ronan@rjp.ie> wrote: > > > > > > February 18, 2026 at 7:58 AM, "Luiz Augusto von Dentz" > > > <luiz.dentz@gmail.com> wrote: > > > > > > > Yeah, the revert part doesn't really fly since > > > > e73bf582dae60356641a32fc27ae03d359ec4c47 does fixes a valid > > > > issue, > > > > that said we do have another proposal under discussion: > > > > > > > > https://patchwork.kernel.org/project/bluetooth/patch/20260217222954.432676-1-larsch@belunktum.dk/ > > > > > > Hi Luiz, > > > > > > I'm aware it fixes a valid issue. My hope with the patch is to > > > fix > > > that same issue in another way. > > > > > > The problem adressed IIUC is that once the input is initialized, > > > a > > > subsequent epoll_wait would hang > > > since the epoll_ctl call had failed to actually add the new event > > > source. The reverted patch had > > > fixed this by trying to avoid initializing stdin in all cases, > > > which inadvertently broke > > > bluetoothctl. My suggestion is to revert that change, and instead > > > check the return value of > > > io_set_read_handler so that we can avoid digging ourselves into a > > > deeper hole. Even if bt_shell_printf > > > is refactored so that it doesn't require the input fd, which > > > sounds > > > prudent, I think it probably makes > > > sense to check the return values here, so I believe the reverted > > > patch would no longer be necessary. > > > > > > I have seen Lars's patch, and that is actually what prompted me > > > to > > > submit this one. Unfortunately, > > > it isn't correct since it causes non-interactive bluetoothctl > > > invocations to also print several > > > extraneous lines relating to the interactive shell. > > > > Feel free to git a review to Lars's patch; hopefully, that will > > help > > us get this resolved quicker. We might as well create a unit test > > for > > shell to try to emulate different modes and environments. That way, > > we > > prevent introducing regressions like this in the future. > > I wrote one, which I can integrate into meson. > > This starts "btvirt" (so requires root), launches bluetoothd if there > isn't a daemon already running, and launches "bluetoothctl list". > > You can run it manually with: > $ sudo top_builddir=/path/to/bluez/builddir/ ./integration-test.py > > If "bluetoothctl list" doesn't output any text, the test errors out. > I've tested that pointing the bluetoothctl_path() output at an old > version of bluetoothctl worked. > > This pattern of using Python to create test suites using python- > dbusmock is something I've already used in PolicyKit, upower, power- > profiles-daemon, gnome-bluetooth and many other places. > > This test is pretty heavy-handed if we just want to test whether > bluetoothctl outputs things correctly, but it has the benefit of > testing the emulator as well as bluetoothd. We could start adding > more > tests to the mix, such as creating more adapters, pairing them, etc. > > Hopefully, this is a useful test to run on CIs, although I'm probably > missing spawning a system bus on top of everything else. I've integrated tests for both bugs (no output in 5.86 and btmgmt hanging) into the integration tests at: https://github.com/hadess/bluez/commits/wip/hadess/add-integration-tests/ and I've independently tested that both problems were fixed by the patches I sent to the list this instant. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization 2026-02-27 11:31 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Bastien Nocera @ 2026-02-28 17:32 ` Ronan Pigott 0 siblings, 0 replies; 17+ messages in thread From: Ronan Pigott @ 2026-02-28 17:32 UTC (permalink / raw) To: Bastien Nocera, Luiz Augusto von Dentz; +Cc: linux-bluetooth February 27, 2026 at 4:31 AM, "Bastien Nocera" wrote: Hi Bastien, > I've integrated tests for both bugs (no output in 5.86 and btmgmt > hanging) into the integration tests at: > https://github.com/hadess/bluez/commits/wip/hadess/add-integration-tests/ > > and I've independently tested that both problems were fixed by the > patches I sent to the list this instant. Looks good! Thanks! ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-04-05 12:46 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-18 2:32 [PATCH BlueZ 0/2] shared: recover from failed input initialization Ronan Pigott 2026-02-18 2:32 ` [PATCH BlueZ 1/2] zsh: amend completions Ronan Pigott 2026-02-18 3:37 ` shared: recover from failed input initialization bluez.test.bot 2026-02-18 14:56 ` [PATCH BlueZ 1/2] zsh: amend completions Luiz Augusto von Dentz 2026-02-18 2:33 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Ronan Pigott 2026-02-18 14:58 ` Luiz Augusto von Dentz 2026-02-18 17:45 ` Ronan Pigott 2026-02-18 18:45 ` Luiz Augusto von Dentz 2026-02-25 16:01 ` Bastien Nocera 2026-02-25 18:58 ` Integration testing for BlueZ Pauli Virtanen 2026-02-27 11:37 ` Bastien Nocera 2026-03-30 12:38 ` Bastien Nocera 2026-03-30 12:59 ` Pauli Virtanen 2026-03-30 15:03 ` Bastien Nocera 2026-04-05 12:37 ` Pauli Virtanen 2026-02-27 11:31 ` [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization Bastien Nocera 2026-02-28 17:32 ` Ronan Pigott
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox