[PATCH BlueZ 0/2] shared: recover from failed input initialization

[PATCH BlueZ 0/2] shared: recover from failed input initialization

[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

[PATCH BlueZ 1/2] zsh: amend completions

[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

RE: shared: recover from failed input initialization

[bluez.test.bot]

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

Re: [PATCH BlueZ 1/2] zsh: amend completions

[Luiz Augusto von Dentz]

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

[PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization

[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

Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization

[Luiz Augusto von Dentz]

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

Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization

[Ronan Pigott]

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

Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization

[Luiz Augusto von Dentz]

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

Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization

[Bastien Nocera]

[-- 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()

Integration testing for BlueZ

[Pauli Virtanen]

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

Re: Integration testing for BlueZ

[Bastien Nocera]

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

Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization

[Bastien Nocera]

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.

Re: [PATCH BlueZ 2/2] shared/shell: gracefully recover from failed input initialization

[Ronan Pigott]

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!