* [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
* [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: 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
* 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: [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: 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: [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
* 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
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