diff mbox series

[BlueZ] policy: Change AutoEnable default to true

Message ID 20220413222425.187199-1-luiz.dentz@gmail.com (mailing list archive)
State Accepted
Commit 180cf09933b2d8eb03972c8638063429fe5fece5
Headers show
Series [BlueZ] policy: Change AutoEnable default to true | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/setupell success Setup ELL PASS
tedd_an/buildprep success Build Prep PASS
tedd_an/build success Build Configuration PASS
tedd_an/makecheck success Make Check PASS
tedd_an/makecheckvalgrind success Make Check PASS
tedd_an/makedistcheck success Make Distcheck PASS
tedd_an/build_extell success Build External ELL PASS
tedd_an/build_extell_make success Build Make with External ELL PASS
tedd_an/incremental_build success Pass

Commit Message

Luiz Augusto von Dentz April 13, 2022, 10:24 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This changes the default of AutoEnable to true so controllers are power
up by default.

Fixes: https://github.com/bluez/bluez/issues/328
---
 plugins/policy.c | 6 +++++-
 src/main.conf    | 4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

Comments

bluez.test.bot@gmail.com April 15, 2022, 6:45 a.m. UTC | #1
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=632044

---Test result---

Test Summary:
CheckPatch                    PASS      1.38 seconds
GitLint                       PASS      1.02 seconds
Prep - Setup ELL              PASS      42.71 seconds
Build - Prep                  PASS      0.68 seconds
Build - Configure             PASS      8.60 seconds
Build - Make                  PASS      1282.51 seconds
Make Check                    PASS      11.48 seconds
Make Check w/Valgrind         PASS      438.79 seconds
Make Distcheck                PASS      226.86 seconds
Build w/ext ELL - Configure   PASS      8.56 seconds
Build w/ext ELL - Make        PASS      1249.86 seconds
Incremental Build with patchesPASS      1279.41 seconds



---
Regards,
Linux Bluetooth
patchwork-bot+bluetooth@kernel.org April 19, 2022, midnight UTC | #2
Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 13 Apr 2022 15:24:25 -0700 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This changes the default of AutoEnable to true so controllers are power
> up by default.
> 
> Fixes: https://github.com/bluez/bluez/issues/328
> 
> [...]

Here is the summary with links:
  - [BlueZ] policy: Change AutoEnable default to true
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=180cf09933b2

You are awesome, thank you!
Marcel Holtmann April 19, 2022, 8:48 a.m. UTC | #3
Hi Luiz,

> This changes the default of AutoEnable to true so controllers are power
> up by default.
> 
> Fixes: https://github.com/bluez/bluez/issues/328
> ---
> plugins/policy.c | 6 +++++-
> src/main.conf    | 4 ++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/plugins/policy.c b/plugins/policy.c
> index 48f5db7d3..0bbdbfc88 100644
> --- a/plugins/policy.c
> +++ b/plugins/policy.c
> @@ -892,7 +892,11 @@ static int policy_init(void)
> 	}
> 
> 	auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
> -									NULL);
> +								&gerr);
> +	if (gerr) {
> +		g_clear_error(&gerr);
> +		auto_enable = true;
> +	}
> 
> 	resume_delay = g_key_file_get_integer(
> 			conf, "Policy", "ResumeDelay", &gerr);
> diff --git a/src/main.conf b/src/main.conf
> index 401796235..91b98b8c4 100644
> --- a/src/main.conf
> +++ b/src/main.conf
> @@ -276,8 +276,8 @@
> 
> # AutoEnable defines option to enable all controllers when they are found.
> # This includes adapters present on start as well as adapters that are plugged
> -# in later on. Defaults to 'false'.
> -#AutoEnable=false
> +# in later on. Defaults to 'true'.
> +#AutoEnable=true

so this is problematic since all wireless technology daemon where design to not automatically activate their hardware. It needed a system above like ConnMan, NetworkManager or even the UI to start the operation.

It is especially tricky since bluetoothd doesn’t remember the last state. So if you power off, then restart, it is on again the next time around. Not something you might have wanted. Using false here is the right choice as a default.

If you install systemd-rfkill and want to auto-power once soft-rfkill is released and systemd-rfkill remembers the state persistently, then you need to install your main.conf with AutoEnable=true. That is a distro choice.

Regards

Marcel
Luiz Augusto von Dentz April 19, 2022, 5:52 p.m. UTC | #4
Hi Marcel,

On Tue, Apr 19, 2022 at 1:48 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > This changes the default of AutoEnable to true so controllers are power
> > up by default.
> >
> > Fixes: https://github.com/bluez/bluez/issues/328
> > ---
> > plugins/policy.c | 6 +++++-
> > src/main.conf    | 4 ++--
> > 2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/plugins/policy.c b/plugins/policy.c
> > index 48f5db7d3..0bbdbfc88 100644
> > --- a/plugins/policy.c
> > +++ b/plugins/policy.c
> > @@ -892,7 +892,11 @@ static int policy_init(void)
> >       }
> >
> >       auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
> > -                                                                     NULL);
> > +                                                             &gerr);
> > +     if (gerr) {
> > +             g_clear_error(&gerr);
> > +             auto_enable = true;
> > +     }
> >
> >       resume_delay = g_key_file_get_integer(
> >                       conf, "Policy", "ResumeDelay", &gerr);
> > diff --git a/src/main.conf b/src/main.conf
> > index 401796235..91b98b8c4 100644
> > --- a/src/main.conf
> > +++ b/src/main.conf
> > @@ -276,8 +276,8 @@
> >
> > # AutoEnable defines option to enable all controllers when they are found.
> > # This includes adapters present on start as well as adapters that are plugged
> > -# in later on. Defaults to 'false'.
> > -#AutoEnable=false
> > +# in later on. Defaults to 'true'.
> > +#AutoEnable=true
>
> so this is problematic since all wireless technology daemon where design to not automatically activate their hardware. It needed a system above like ConnMan, NetworkManager or even the UI to start the operation.
>
> It is especially tricky since bluetoothd doesn’t remember the last state. So if you power off, then restart, it is on again the next time around. Not something you might have wanted. Using false here is the right choice as a default.
>
> If you install systemd-rfkill and want to auto-power once soft-rfkill is released and systemd-rfkill remembers the state persistently, then you need to install your main.conf with AutoEnable=true. That is a distro choice.

The argument was that nowadays most systems ship with systemd-rfkill
so having AutoEnable=true is much more common than the old default,
perhaps we could detect if the system has systemd-rfkill than we
default to AutoEnable=true automatically, does this sound better to
you?

> Regards
>
> Marcel
>
diff mbox series

Patch

diff --git a/plugins/policy.c b/plugins/policy.c
index 48f5db7d3..0bbdbfc88 100644
--- a/plugins/policy.c
+++ b/plugins/policy.c
@@ -892,7 +892,11 @@  static int policy_init(void)
 	}
 
 	auto_enable = g_key_file_get_boolean(conf, "Policy", "AutoEnable",
-									NULL);
+								&gerr);
+	if (gerr) {
+		g_clear_error(&gerr);
+		auto_enable = true;
+	}
 
 	resume_delay = g_key_file_get_integer(
 			conf, "Policy", "ResumeDelay", &gerr);
diff --git a/src/main.conf b/src/main.conf
index 401796235..91b98b8c4 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -276,8 +276,8 @@ 
 
 # AutoEnable defines option to enable all controllers when they are found.
 # This includes adapters present on start as well as adapters that are plugged
-# in later on. Defaults to 'false'.
-#AutoEnable=false
+# in later on. Defaults to 'true'.
+#AutoEnable=true
 
 # Audio devices that were disconnected due to suspend will be reconnected on
 # resume. ResumeDelay determines the delay between when the controller