2015-04-10 16:57:42

by Alexander Holler

[permalink] [raw]
Subject: [PATCH] bluetoothd: add option to automatically power on the first adapter found

You want this option if you're using e.g. a bt-keyboard as the only
input device. Other solutions to power on an adapter at startup are
either unreliable or too complicated.

E.g. I had a

sleep 4
echo 'power on' | bluetoothctl

in some startup-script. And with an update of bluez from 5.23 to
5.29 Murphy visited me and I had a box without a usable keyboard
because those 4 second haven't been enough anymore.
---
src/adapter.c | 10 ++++++++--
src/adapter.h | 2 +-
src/main.c | 7 ++++++-
3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 7ffd302..10de028 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -117,6 +117,8 @@ static GSList *adapter_drivers = NULL;
static GSList *disconnect_list = NULL;
static GSList *conn_fail_list = NULL;

+static bool power_on_first_adapter;
+
struct link_key_info {
bdaddr_t bdaddr;
unsigned char key[16];
@@ -7315,8 +7317,11 @@ static int adapter_register(struct btd_adapter *adapter)
return -EINVAL;
}

- if (adapters == NULL)
+ if (adapters == NULL) {
adapter->is_default = true;
+ if (power_on_first_adapter)
+ set_mode(adapter, MGMT_OP_SET_POWERED, 0x01);
+ }

adapters = g_slist_append(adapters, adapter);

@@ -8132,8 +8137,9 @@ static void mgmt_debug(const char *str, void *user_data)
info("%s%s", prefix, str);
}

-int adapter_init(void)
+int adapter_init(bool _power_on_first_adapter)
{
+ power_on_first_adapter = _power_on_first_adapter;
dbus_conn = btd_get_dbus_connection();

mgmt_master = mgmt_new_default();
diff --git a/src/adapter.h b/src/adapter.h
index 0c95f5d..96fef5f 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -55,7 +55,7 @@ struct oob_handler {
void *user_data;
};

-int adapter_init(void);
+int adapter_init(bool _power_on_first_adapter);
void adapter_cleanup(void);
void adapter_shutdown(void);

diff --git a/src/main.c b/src/main.c
index 4c94a69..d0a301e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -457,6 +457,7 @@ static gboolean option_compat = FALSE;
static gboolean option_detach = TRUE;
static gboolean option_version = FALSE;
static gboolean option_experimental = FALSE;
+static gboolean option_power_on_first_adapter;

static void free_options(void)
{
@@ -542,6 +543,7 @@ static GOptionEntry options[] = {
"Specify plugins not to load", "NAME,..." },
{ "compat", 'C', 0, G_OPTION_ARG_NONE, &option_compat,
"Provide deprecated command line interfaces" },
+
{ "experimental", 'E', 0, G_OPTION_ARG_NONE, &option_experimental,
"Enable experimental interfaces" },
{ "nodetach", 'n', G_OPTION_FLAG_REVERSE,
@@ -549,6 +551,9 @@ static GOptionEntry options[] = {
"Run with logging in foreground" },
{ "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
"Show version information and exit" },
+ { "power_on_first_adapter", 'o', 0, G_OPTION_ARG_NONE,
+ &option_power_on_first_adapter,
+ "Automatically power on the first adapter found" },
{ NULL },
};

@@ -607,7 +612,7 @@ int main(int argc, char *argv[])

g_dbus_set_flags(gdbus_flags);

- if (adapter_init() < 0) {
+ if (adapter_init(option_power_on_first_adapter) < 0) {
error("Adapter handling initialization failed");
exit(1);
}
--
2.1.0



2015-04-27 20:36:39

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 27.04.2015 um 20:53 schrieb Marcel Holtmann:
> Hi Alexander,
>
>>>>>> Your race power on vs keys and known devices programmed into the
>>>>>> kernel. It needs to be done in the right order.
>>>>>
>>>>> Hmm, I still seem to function, no sign of races, besides that I'm
>>>>> happily working without a Linux kernel.
>>>>>
>>>>> But in regard to the patch. Maybe. No idea. It works for me (tm) and is
>>>>> faster and more reliable here than other stuff.
>>>>
>>>> To pick up that thread again, if you want that I remove a race in that patch I don't see by looking at it, you have to be a bit more verbose about the race.
>>>
>>> it is pretty simple, you want to configure the list of known devices and its keys before you turn on your device for general operation. If you don't do that, you have a race condition.
>>>
>>>> For me it currently reads like you're more talking about an existing problem in the kernel. If the kernel doesn't like it that a bt-device will be turned on before something like keys and known devices have been setup, then the kernel should throw an error back to userspace.
>>>
>>> The kernel is not enforcing policy. We can not require to have keys loaded first. There might be no keys or you want to use the Bluetooth subsystem for something else. The kernel mgmt interface provides a flexible way into the general procedures that Bluetooth requires. With bluetoothd we enforce profiles and other behaviors to be consistent. That is not the kernel's job.
>>>
>>> And even if we wanted to restrict this heavily, we can not easily do that without potentially breaking existing userspace.
>>>
>>>> But that's just an assumption from me, I haven't read through all the source in bluetoothd or the kernel, but instead just turned on a knob like it would be turned on by issuing "power on" in bluetoothctl.
>>>
>>> With bluetoothctl you talk over the D-Bus interface to bluetoothd. So when you turn power on there, then it is ensured that everything happens in the right order. That is what bluetoothd enforces. It is not the kernels job to do that.
>>
>> For me this reads like kernel <-> userspace API is very fragile.
>>
>> Also I haven't looked at the source for that, I would assume that it should be no problem to submit an empty list of keys and devices to the kernel. So a probably stable design of that part of the API would be:
>>
>> - Require that userspace sets up various stuff
>> - Require that userspace sends a list of keys (maybe empty)
>> - Require that userspace sends a list of devices (maybe empty)
>> - Turn on devices which are requested to be turned on and start operation.
>>
>> I'm aware that this might involve problems with older userspace, but just trying to discuss a problem away (e.g. by murdering the messenger) is usually a bad solution.
>>
>> Right now it looks like userspace can confuse the kernel quiet a lot which it should not be able to do.
>>
>> For example you are calling it a race if my patch for bluetoothd is used. Which bad things might happen? Do I have to fear my machine blows up and will erase my storage? Or, as we are talking about wireless stuff, do I have to fear that security is absent and everyone might be able to connect to my machine?
>
> the kernel is different from bluetoothd. You are changing code in bluetoothd and thus lets do it correctly. I really have no idea what you are arguing here for.

You are saying that it's a race to power on the dongle before keys and
devices are setup. And I'm assuming the keys and/or devices are managed,
at least in part, by the kernel.

If not, then it's a of course just an userspace only problem. Also I'm
still interested what might go wrong, it's unlikely I will get an answer
as I seem to have to look up that myself.

>> Sorry that I don't understand your reasoning, but if I would translate that, it would mean the same like if it's a race to call unlink(2) on a non-existing file. Of course, it would be much easier for the kernel to not have to handle all the possible error conditions, but that's the dream of any sw-dev and will never become reality. Handling all the possible error conditions just has to be part of almost any sw, regardless if it's a kernel or not.
>>
>> Anyway, to come back to the problem of easy and reliable turning on bt-devices at startup, I will look if I find the place in bluetoothd where the keys and devices are submitted to the kernel in order to put the requested power-on after that. But as I'm still happy with my current solution, I don't make any promises on a timeline. ;)
>
> If you want the patch to get upstream, I clearly gave you instructions on what has to change to make it upstream acceptable. The work here is on you.

Sorry, but as said several times before, I don't care much for any Linux
upstream anymore. It's just too cumbersome to discuss about maintainer
specific styles and other maintainer specific preferences and I've
already wasted much too much time in making patches ready for submission
which never ended up anywhere (but for which I still receive
mails/requests from other users). I now just care for other users and
post a patch if I really think it might help several people or if I'm
annoyed because I've read too much complaints about a problem. So no
work is on me.

Regards,

Alexander Holler

2015-04-27 18:53:37

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

>>>>> Your race power on vs keys and known devices programmed into the
>>>>> kernel. It needs to be done in the right order.
>>>>
>>>> Hmm, I still seem to function, no sign of races, besides that I'm
>>>> happily working without a Linux kernel.
>>>>
>>>> But in regard to the patch. Maybe. No idea. It works for me (tm) and is
>>>> faster and more reliable here than other stuff.
>>>
>>> To pick up that thread again, if you want that I remove a race in that patch I don't see by looking at it, you have to be a bit more verbose about the race.
>>
>> it is pretty simple, you want to configure the list of known devices and its keys before you turn on your device for general operation. If you don't do that, you have a race condition.
>>
>>> For me it currently reads like you're more talking about an existing problem in the kernel. If the kernel doesn't like it that a bt-device will be turned on before something like keys and known devices have been setup, then the kernel should throw an error back to userspace.
>>
>> The kernel is not enforcing policy. We can not require to have keys loaded first. There might be no keys or you want to use the Bluetooth subsystem for something else. The kernel mgmt interface provides a flexible way into the general procedures that Bluetooth requires. With bluetoothd we enforce profiles and other behaviors to be consistent. That is not the kernel's job.
>>
>> And even if we wanted to restrict this heavily, we can not easily do that without potentially breaking existing userspace.
>>
>>> But that's just an assumption from me, I haven't read through all the source in bluetoothd or the kernel, but instead just turned on a knob like it would be turned on by issuing "power on" in bluetoothctl.
>>
>> With bluetoothctl you talk over the D-Bus interface to bluetoothd. So when you turn power on there, then it is ensured that everything happens in the right order. That is what bluetoothd enforces. It is not the kernels job to do that.
>
> For me this reads like kernel <-> userspace API is very fragile.
>
> Also I haven't looked at the source for that, I would assume that it should be no problem to submit an empty list of keys and devices to the kernel. So a probably stable design of that part of the API would be:
>
> - Require that userspace sets up various stuff
> - Require that userspace sends a list of keys (maybe empty)
> - Require that userspace sends a list of devices (maybe empty)
> - Turn on devices which are requested to be turned on and start operation.
>
> I'm aware that this might involve problems with older userspace, but just trying to discuss a problem away (e.g. by murdering the messenger) is usually a bad solution.
>
> Right now it looks like userspace can confuse the kernel quiet a lot which it should not be able to do.
>
> For example you are calling it a race if my patch for bluetoothd is used. Which bad things might happen? Do I have to fear my machine blows up and will erase my storage? Or, as we are talking about wireless stuff, do I have to fear that security is absent and everyone might be able to connect to my machine?

the kernel is different from bluetoothd. You are changing code in bluetoothd and thus lets do it correctly. I really have no idea what you are arguing here for.

> Sorry that I don't understand your reasoning, but if I would translate that, it would mean the same like if it's a race to call unlink(2) on a non-existing file. Of course, it would be much easier for the kernel to not have to handle all the possible error conditions, but that's the dream of any sw-dev and will never become reality. Handling all the possible error conditions just has to be part of almost any sw, regardless if it's a kernel or not.
>
> Anyway, to come back to the problem of easy and reliable turning on bt-devices at startup, I will look if I find the place in bluetoothd where the keys and devices are submitted to the kernel in order to put the requested power-on after that. But as I'm still happy with my current solution, I don't make any promises on a timeline. ;)

If you want the patch to get upstream, I clearly gave you instructions on what has to change to make it upstream acceptable. The work here is on you.

Regards

Marcel


2015-04-27 09:11:36

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 27.04.2015 um 06:40 schrieb Marcel Holtmann:
> Hi Alexander,
>
>>>> Your race power on vs keys and known devices programmed into the
>>>> kernel. It needs to be done in the right order.
>>>
>>> Hmm, I still seem to function, no sign of races, besides that I'm
>>> happily working without a Linux kernel.
>>>
>>> But in regard to the patch. Maybe. No idea. It works for me (tm) and is
>>> faster and more reliable here than other stuff.
>>
>> To pick up that thread again, if you want that I remove a race in that patch I don't see by looking at it, you have to be a bit more verbose about the race.
>
> it is pretty simple, you want to configure the list of known devices and its keys before you turn on your device for general operation. If you don't do that, you have a race condition.
>
>> For me it currently reads like you're more talking about an existing problem in the kernel. If the kernel doesn't like it that a bt-device will be turned on before something like keys and known devices have been setup, then the kernel should throw an error back to userspace.
>
> The kernel is not enforcing policy. We can not require to have keys loaded first. There might be no keys or you want to use the Bluetooth subsystem for something else. The kernel mgmt interface provides a flexible way into the general procedures that Bluetooth requires. With bluetoothd we enforce profiles and other behaviors to be consistent. That is not the kernel's job.
>
> And even if we wanted to restrict this heavily, we can not easily do that without potentially breaking existing userspace.
>
>> But that's just an assumption from me, I haven't read through all the source in bluetoothd or the kernel, but instead just turned on a knob like it would be turned on by issuing "power on" in bluetoothctl.
>
> With bluetoothctl you talk over the D-Bus interface to bluetoothd. So when you turn power on there, then it is ensured that everything happens in the right order. That is what bluetoothd enforces. It is not the kernels job to do that.

For me this reads like kernel <-> userspace API is very fragile.

Also I haven't looked at the source for that, I would assume that it
should be no problem to submit an empty list of keys and devices to the
kernel. So a probably stable design of that part of the API would be:

- Require that userspace sets up various stuff
- Require that userspace sends a list of keys (maybe empty)
- Require that userspace sends a list of devices (maybe empty)
- Turn on devices which are requested to be turned on and start operation.

I'm aware that this might involve problems with older userspace, but
just trying to discuss a problem away (e.g. by murdering the messenger)
is usually a bad solution.

Right now it looks like userspace can confuse the kernel quiet a lot
which it should not be able to do.

For example you are calling it a race if my patch for bluetoothd is
used. Which bad things might happen? Do I have to fear my machine blows
up and will erase my storage? Or, as we are talking about wireless
stuff, do I have to fear that security is absent and everyone might be
able to connect to my machine?

Sorry that I don't understand your reasoning, but if I would translate
that, it would mean the same like if it's a race to call unlink(2) on a
non-existing file. Of course, it would be much easier for the kernel to
not have to handle all the possible error conditions, but that's the
dream of any sw-dev and will never become reality. Handling all the
possible error conditions just has to be part of almost any sw,
regardless if it's a kernel or not.

Anyway, to come back to the problem of easy and reliable turning on
bt-devices at startup, I will look if I find the place in bluetoothd
where the keys and devices are submitted to the kernel in order to put
the requested power-on after that. But as I'm still happy with my
current solution, I don't make any promises on a timeline. ;)

Regards,

Alexander Holler

2015-04-27 04:40:29

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

>>> Your race power on vs keys and known devices programmed into the
>>> kernel. It needs to be done in the right order.
>>
>> Hmm, I still seem to function, no sign of races, besides that I'm
>> happily working without a Linux kernel.
>>
>> But in regard to the patch. Maybe. No idea. It works for me (tm) and is
>> faster and more reliable here than other stuff.
>
> To pick up that thread again, if you want that I remove a race in that patch I don't see by looking at it, you have to be a bit more verbose about the race.

it is pretty simple, you want to configure the list of known devices and its keys before you turn on your device for general operation. If you don't do that, you have a race condition.

> For me it currently reads like you're more talking about an existing problem in the kernel. If the kernel doesn't like it that a bt-device will be turned on before something like keys and known devices have been setup, then the kernel should throw an error back to userspace.

The kernel is not enforcing policy. We can not require to have keys loaded first. There might be no keys or you want to use the Bluetooth subsystem for something else. The kernel mgmt interface provides a flexible way into the general procedures that Bluetooth requires. With bluetoothd we enforce profiles and other behaviors to be consistent. That is not the kernel's job.

And even if we wanted to restrict this heavily, we can not easily do that without potentially breaking existing userspace.

> But that's just an assumption from me, I haven't read through all the source in bluetoothd or the kernel, but instead just turned on a knob like it would be turned on by issuing "power on" in bluetoothctl.

With bluetoothctl you talk over the D-Bus interface to bluetoothd. So when you turn power on there, then it is ensured that everything happens in the right order. That is what bluetoothd enforces. It is not the kernels job to do that.

Regards

Marcel


2015-04-25 10:26:13

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 15.04.2015 um 19:59 schrieb Alexander Holler:
> Am 14.04.2015 um 15:50 schrieb Marcel Holtmann:
>
>> Your race power on vs keys and known devices programmed into the
>> kernel. It needs to be done in the right order.
>
> Hmm, I still seem to function, no sign of races, besides that I'm
> happily working without a Linux kernel.
>
> But in regard to the patch. Maybe. No idea. It works for me (tm) and is
> faster and more reliable here than other stuff.

To pick up that thread again, if you want that I remove a race in that
patch I don't see by looking at it, you have to be a bit more verbose
about the race.

For me it currently reads like you're more talking about an existing
problem in the kernel. If the kernel doesn't like it that a bt-device
will be turned on before something like keys and known devices have been
setup, then the kernel should throw an error back to userspace.

But that's just an assumption from me, I haven't read through all the
source in bluetoothd or the kernel, but instead just turned on a knob
like it would be turned on by issuing "power on" in bluetoothctl.

Regards,

Alexander Holler

2015-04-15 17:59:43

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 14.04.2015 um 15:50 schrieb Marcel Holtmann:

> Your race power on vs keys and known devices programmed into the kernel. It needs to be done in the right order.

Hmm, I still seem to function, no sign of races, besides that I'm
happily working without a Linux kernel.

But in regard to the patch. Maybe. No idea. It works for me (tm) and is
faster and more reliable here than other stuff.

Regards,

Alexander Holler

2015-04-14 15:56:30

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi,

On Tuesday 14 of April 2015 16:14:42 Szymon Janc wrote:
> Hi,
>
> On Tuesday 14 of April 2015 06:50:11 Marcel Holtmann wrote:
> > Hi Alexander,
> >
> > >>>>>>> As said, this (currently) needs that dbus is ready and
> > >>>>>>> bluetoothctl
> > >>>>>>> can communicate with bluetoothd. And on a reasonable fast machine
> > >>>>>>> (4 core x86_64 2ghz+) this needs more than 4 seconds after startup
> > >>>>>>> here (invoked through /etc/rc.d/rc.local on f21).>>>>>>
> > >>>>>>
> > >>>>>> Szymon send an example on how to add wait-for-adapter in
> > >>>>>> bluetoothctl. Together with that and a proper systemd unit, I bet
> > >>>>>> you that this does not take 4 seconds.
> > >>>>>>
> > >>>>>> If you insist on doing old fashion rc.local stuff, then I can not
> > >>>>>> help you. You can look into systemd target definitions, but I am
> > >>>>>> pretty sure it ends up being started after something else
>
> happened.>>>>>
>
> > >>>>> At least that old fashioned stuff worked. I even still have
> > >>>>> bt-alsa-devices on systems where I stick to bluez 4.0.
> > >>>>>
> > >>>>> And it would make me extremly wonder, if it would be any faster when
> > >>>>> using a systemd unit. Remember, I did in rc.local:
> > >>>>>
> > >>>>> sleep 4
> > >>>>> echo "power on" | /bin/bluetoothctl
> > >>>>>
> > >>>>> And if that fails, then how could a systemd unit which calls
> > >>>>> bluetoothctl be faster? There is absolutely nothing in those 2 lines
> > >>>>> which slows down the necessary prerequisits for bluetoothctl.>>>>
> > >>>>
> > >>>> except the sleep 4 that you put in. So you are going on for rant this
> > >>>> takes 4 seconds and the reason it takes 4 seconds since you put a
> > >>>> sleep 4 in there. Szymon gave you a simple solution for removing the
> > >>>> sleep and we are still discussing this.>>>
> > >>>
> > >>> I've run into the problem, because those 4 second weren't enough after
> > >>> an update of bluez. I then had to use 6 seconds, otherwise that
> > >>> construct failed. And because that ugly sleep was just an extremly
> > >>> ugly
> > >>> workaround for the missing easy-to-use knob to reliable turn on bt on
> > >>> Linux with bluez 5.x, I've now written that patch.
> > >>>
> > >>> But it's senseless trying to discuss further. If you think people have
> > >>> to write udev-rules or systemd units in order to use bluetooth on
> > >>> Linux, there is just nothing I could add to such a discussion.>>
> > >>
> > >> you did read my responses here? I mean there are so many possible
> > >> options. Szymon provided an addition for bluetoothctl command where you
> > >> could just have replaced sleep 4 with the new command. And you could
> > >> have it working perfectly all the time.>>
> > >>
> > >>> And you might want to decide if you want a configuration less bluez
> > >>> (something I think I've read several times and which is likely the
> > >>> reason that make install doesn't install at least an example for
> > >>> configuration), or if you still want to support some configuration as
> > >>> mentioned in the comment in regard to a [policy] section.>>
> > >>
> > >> Upstream is not responsible for personal or distribution specific
> > >> customization. We provide configuration via main.conf and I am
> > >> repeating
> > >> myself now for the 3rd time here, I am fine with a policy setting for
> > >> auto-power on found controllers. The distribution has to do the work to
> > >> put a proper main.conf in place if they choose to. It is really up to
> > >> them. This is not something upstream should do in the first place.
> > >>
> > >> bluetoothd has to work properly with default values if the
> > >> configuration
> > >> is present or not. Failing because a configuration file is not present
> > >> and you can not run a proper sane default is plain stupid. That is not
> > >> how we write software. We are doing proper defaults.>
> > >
> > > Sorry, I'm sick of having to teach others about the usability problems
> > > of
> > > bluez.
> > >
> > > E.g. since the configuration knob for hid2hci disappeared (I think that
> > > happened with bluez 4.x), it's still done wrong by many people and
> > > distributions. E.g. on Ubuntu the udev-rule for hid2hci is still wrong
> > > because nobody explained them that the udev-rule is not the problem but
> > > the non-existing knob to turn off hid2hci if it is installed. Not to
> > > mention that it should be turned off by default (until the user turned
> > > it
> > > on, hoping he is aware of the implications).
> > >
> > > Just the same problem which happened on Fedora and Gentoo too (until
> > > I've
> > > explained the problem to the maintainers and now hid2hci isnt installed
> > > by default there). It's just to hard to understand (and to explain) that
> > > installing a command-line utility (by default) might render a system
> > > unusable. And users aren't the problem here, if even maintainers get it
> > > wrong.
> > >
> > > And almost anyone runs into the problem that bt-adapters are now powered
> > > off by default and people have not idea how to change that policy or
> > > even
> > > how to manually turn on the adapter. And that now happens since 5.x was
> > > thrown at users, which is already 30 releases old.>
> > >
> > >> Please respect the upstream decision not to install main.conf in
> > >> /etc/bluetooth/ and leave this up to the distribution and packagers. In
> > >> case you wonder this is important for stateless installations and
> > >> systems that support factory reset.
> > >>
> > >> I am happy to take a patch that implements the auto-power on properly
> > >> and
> > >> without race conditions and makes it configurable based on the
> > >> main.conf
> > >> policy section. And I said this many times now. The amount of emails
> > >> exchanged on this topic where I agreed to take a patch if it would be
> > >> done race free and in sync with how upstream operates is amazing.>
> > >
> > > Sorry, but I've no idea about which races you are talking. Of course, I
> > > haven't fixed the already existing race(s) in bluetoothd with the
> > > missing
> > > lock for the adapter list (at least I haven't seen or found any lock for
> > > that list, otherwise I would have used it). But that still might be my
> > > problem, as I'm not very familiar with bluetoothd and especially it's
> > > locking strategy (if there is any, which I hope).
> >
> > it is a single threaded process. It does not need a lock for the adapter
> > list.
> >
> > Your race power on vs keys and known devices programmed into the kernel.
> > It
> > needs to be done in the right order.
> >
> > > I also have noticed that bluetoothd and bluetoothctl seem to have a
> > > different idea about the default adapter (at least that is_default
> > > variable in bluetoothd doesn't seem to be in sync with what bluetoothctl
> > > displayed as default adapter when I've tried the stuff with two
> > > adapters). Or I just have missed how that is_default was changed during
> > > my few tests. Otherwise I would have named that option I've added
> > > different ("default adapter" instead of "first adapter").
> >
> > That sounds like a bug to me that should be fixed. I wonder why bluetoothd
> > actually has to carry the concept of the default adapter at all anymore.
>
> There is no default adapter on D-Bus level. All are equal.
>
> This (is_default) just an internal implementation detail used only by
> plugins (sixaxis, neard and hostname) ie to pick up any adapter when there
> is no other context. One exception would be hostname plugin which use this
> to not add number to 'default' adapter name. But all those could be simply
> replaced by picking first adapter on list (or one with lowest id, as this
> is pretty much the same as what is_default is indicating).

Correct that. Default adapter is one that was first reported by kernel (last
one plugged), if that one is removed new default is pick up as last plugged
but powered up adapter.

Making is dynamic (eg always report last plugged but powered adapter) would
actually make sense for plugins like sixaxis or neard where this is most
likely what user would expect.

--
BR
Szymon Janc

2015-04-14 14:14:42

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi,

On Tuesday 14 of April 2015 06:50:11 Marcel Holtmann wrote:
> Hi Alexander,
>
> >>>>>>> As said, this (currently) needs that dbus is ready and bluetoothctl
> >>>>>>> can communicate with bluetoothd. And on a reasonable fast machine
> >>>>>>> (4 core x86_64 2ghz+) this needs more than 4 seconds after startup
> >>>>>>> here (invoked through /etc/rc.d/rc.local on f21).>>>>>>
> >>>>>> Szymon send an example on how to add wait-for-adapter in
> >>>>>> bluetoothctl. Together with that and a proper systemd unit, I bet
> >>>>>> you that this does not take 4 seconds.
> >>>>>>
> >>>>>> If you insist on doing old fashion rc.local stuff, then I can not
> >>>>>> help you. You can look into systemd target definitions, but I am
> >>>>>> pretty sure it ends up being started after something else
happened.>>>>>
> >>>>> At least that old fashioned stuff worked. I even still have
> >>>>> bt-alsa-devices on systems where I stick to bluez 4.0.
> >>>>>
> >>>>> And it would make me extremly wonder, if it would be any faster when
> >>>>> using a systemd unit. Remember, I did in rc.local:
> >>>>>
> >>>>> sleep 4
> >>>>> echo "power on" | /bin/bluetoothctl
> >>>>>
> >>>>> And if that fails, then how could a systemd unit which calls
> >>>>> bluetoothctl be faster? There is absolutely nothing in those 2 lines
> >>>>> which slows down the necessary prerequisits for bluetoothctl.>>>>
> >>>> except the sleep 4 that you put in. So you are going on for rant this
> >>>> takes 4 seconds and the reason it takes 4 seconds since you put a
> >>>> sleep 4 in there. Szymon gave you a simple solution for removing the
> >>>> sleep and we are still discussing this.>>>
> >>> I've run into the problem, because those 4 second weren't enough after
> >>> an update of bluez. I then had to use 6 seconds, otherwise that
> >>> construct failed. And because that ugly sleep was just an extremly ugly
> >>> workaround for the missing easy-to-use knob to reliable turn on bt on
> >>> Linux with bluez 5.x, I've now written that patch.
> >>>
> >>> But it's senseless trying to discuss further. If you think people have
> >>> to write udev-rules or systemd units in order to use bluetooth on
> >>> Linux, there is just nothing I could add to such a discussion.>>
> >> you did read my responses here? I mean there are so many possible
> >> options. Szymon provided an addition for bluetoothctl command where you
> >> could just have replaced sleep 4 with the new command. And you could
> >> have it working perfectly all the time.>>
> >>> And you might want to decide if you want a configuration less bluez
> >>> (something I think I've read several times and which is likely the
> >>> reason that make install doesn't install at least an example for
> >>> configuration), or if you still want to support some configuration as
> >>> mentioned in the comment in regard to a [policy] section.>>
> >> Upstream is not responsible for personal or distribution specific
> >> customization. We provide configuration via main.conf and I am repeating
> >> myself now for the 3rd time here, I am fine with a policy setting for
> >> auto-power on found controllers. The distribution has to do the work to
> >> put a proper main.conf in place if they choose to. It is really up to
> >> them. This is not something upstream should do in the first place.
> >>
> >> bluetoothd has to work properly with default values if the configuration
> >> is present or not. Failing because a configuration file is not present
> >> and you can not run a proper sane default is plain stupid. That is not
> >> how we write software. We are doing proper defaults.>
> > Sorry, I'm sick of having to teach others about the usability problems of
> > bluez.
> >
> > E.g. since the configuration knob for hid2hci disappeared (I think that
> > happened with bluez 4.x), it's still done wrong by many people and
> > distributions. E.g. on Ubuntu the udev-rule for hid2hci is still wrong
> > because nobody explained them that the udev-rule is not the problem but
> > the non-existing knob to turn off hid2hci if it is installed. Not to
> > mention that it should be turned off by default (until the user turned it
> > on, hoping he is aware of the implications).
> >
> > Just the same problem which happened on Fedora and Gentoo too (until I've
> > explained the problem to the maintainers and now hid2hci isnt installed
> > by default there). It's just to hard to understand (and to explain) that
> > installing a command-line utility (by default) might render a system
> > unusable. And users aren't the problem here, if even maintainers get it
> > wrong.
> >
> > And almost anyone runs into the problem that bt-adapters are now powered
> > off by default and people have not idea how to change that policy or even
> > how to manually turn on the adapter. And that now happens since 5.x was
> > thrown at users, which is already 30 releases old.>
> >> Please respect the upstream decision not to install main.conf in
> >> /etc/bluetooth/ and leave this up to the distribution and packagers. In
> >> case you wonder this is important for stateless installations and
> >> systems that support factory reset.
> >>
> >> I am happy to take a patch that implements the auto-power on properly and
> >> without race conditions and makes it configurable based on the main.conf
> >> policy section. And I said this many times now. The amount of emails
> >> exchanged on this topic where I agreed to take a patch if it would be
> >> done race free and in sync with how upstream operates is amazing.>
> > Sorry, but I've no idea about which races you are talking. Of course, I
> > haven't fixed the already existing race(s) in bluetoothd with the missing
> > lock for the adapter list (at least I haven't seen or found any lock for
> > that list, otherwise I would have used it). But that still might be my
> > problem, as I'm not very familiar with bluetoothd and especially it's
> > locking strategy (if there is any, which I hope).
> it is a single threaded process. It does not need a lock for the adapter
> list.
>
> Your race power on vs keys and known devices programmed into the kernel. It
> needs to be done in the right order.
> > I also have noticed that bluetoothd and bluetoothctl seem to have a
> > different idea about the default adapter (at least that is_default
> > variable in bluetoothd doesn't seem to be in sync with what bluetoothctl
> > displayed as default adapter when I've tried the stuff with two
> > adapters). Or I just have missed how that is_default was changed during
> > my few tests. Otherwise I would have named that option I've added
> > different ("default adapter" instead of "first adapter").

> That sounds like a bug to me that should be fixed. I wonder why bluetoothd
> actually has to carry the concept of the default adapter at all anymore.

There is no default adapter on D-Bus level. All are equal.

This (is_default) just an internal implementation detail used only by plugins
(sixaxis, neard and hostname) ie to pick up any adapter when there is no other
context. One exception would be hostname plugin which use this to not add
number to 'default' adapter name. But all those could be simply replaced by
picking first adapter on list (or one with lowest id, as this is pretty much
the same as what is_default is indicating).

--
BR
Szymon Janc

2015-04-14 13:50:11

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

>>>>>>> As said, this (currently) needs that dbus is ready and bluetoothctl can communicate with bluetoothd. And on a reasonable fast machine (4 core x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked through /etc/rc.d/rc.local on f21).
>>>>>>
>>>>>> Szymon send an example on how to add wait-for-adapter in bluetoothctl. Together with that and a proper systemd unit, I bet you that this does not take 4 seconds.
>>>>>>
>>>>>> If you insist on doing old fashion rc.local stuff, then I can not help you. You can look into systemd target definitions, but I am pretty sure it ends up being started after something else happened.
>>>>>
>>>>> At least that old fashioned stuff worked. I even still have bt-alsa-devices on systems where I stick to bluez 4.0.
>>>>>
>>>>> And it would make me extremly wonder, if it would be any faster when using a systemd unit. Remember, I did in rc.local:
>>>>>
>>>>> sleep 4
>>>>> echo "power on" | /bin/bluetoothctl
>>>>>
>>>>> And if that fails, then how could a systemd unit which calls bluetoothctl be faster? There is absolutely nothing in those 2 lines which slows down the necessary prerequisits for bluetoothctl.
>>>>
>>>> except the sleep 4 that you put in. So you are going on for rant this takes 4 seconds and the reason it takes 4 seconds since you put a sleep 4 in there. Szymon gave you a simple solution for removing the sleep and we are still discussing this.
>>>
>>> I've run into the problem, because those 4 second weren't enough after an update of bluez. I then had to use 6 seconds, otherwise that construct failed. And because that ugly sleep was just an extremly ugly workaround for the missing easy-to-use knob to reliable turn on bt on Linux with bluez 5.x, I've now written that patch.
>>>
>>> But it's senseless trying to discuss further. If you think people have to write udev-rules or systemd units in order to use bluetooth on Linux, there is just nothing I could add to such a discussion.
>>
>> you did read my responses here? I mean there are so many possible options. Szymon provided an addition for bluetoothctl command where you could just have replaced sleep 4 with the new command. And you could have it working perfectly all the time.
>>
>>> And you might want to decide if you want a configuration less bluez (something I think I've read several times and which is likely the reason that make install doesn't install at least an example for configuration), or if you still want to support some configuration as mentioned in the comment in regard to a [policy] section.
>>
>> Upstream is not responsible for personal or distribution specific customization. We provide configuration via main.conf and I am repeating myself now for the 3rd time here, I am fine with a policy setting for auto-power on found controllers. The distribution has to do the work to put a proper main.conf in place if they choose to. It is really up to them. This is not something upstream should do in the first place.
>>
>> bluetoothd has to work properly with default values if the configuration is present or not. Failing because a configuration file is not present and you can not run a proper sane default is plain stupid. That is not how we write software. We are doing proper defaults.
>
> Sorry, I'm sick of having to teach others about the usability problems of bluez.
>
> E.g. since the configuration knob for hid2hci disappeared (I think that
> happened with bluez 4.x), it's still done wrong by many people and
> distributions. E.g. on Ubuntu the udev-rule for hid2hci is still wrong because nobody explained them that the udev-rule is not the problem but the non-existing knob to turn off hid2hci if it is installed. Not to mention that it should be turned off by default (until the user turned it on, hoping he is aware of the implications).
>
> Just the same problem which happened on Fedora and Gentoo too (until I've explained the problem to the maintainers and now hid2hci isnt installed by default there). It's just to hard to understand (and to explain) that installing a command-line utility (by default) might render a system unusable. And users aren't the problem here, if even maintainers get it wrong.
>
> And almost anyone runs into the problem that bt-adapters are now powered
> off by default and people have not idea how to change that policy or even how to manually turn on the adapter. And that now happens since 5.x was thrown at users, which is already 30 releases old.
>
>>
>> Please respect the upstream decision not to install main.conf in /etc/bluetooth/ and leave this up to the distribution and packagers. In case you wonder this is important for stateless installations and systems that support factory reset.
>>
>> I am happy to take a patch that implements the auto-power on properly and without race conditions and makes it configurable based on the main.conf policy section. And I said this many times now. The amount of emails exchanged on this topic where I agreed to take a patch if it would be done race free and in sync with how upstream operates is amazing.
>>
>
> Sorry, but I've no idea about which races you are talking. Of course, I haven't fixed the already existing race(s) in bluetoothd with the missing lock for the adapter list (at least I haven't seen or found any lock for that list, otherwise I would have used it). But that still might be my problem, as I'm not very familiar with bluetoothd and especially it's locking strategy (if there is any, which I hope).

it is a single threaded process. It does not need a lock for the adapter list.

Your race power on vs keys and known devices programmed into the kernel. It needs to be done in the right order.

> I also have noticed that bluetoothd and bluetoothctl seem to have a different idea about the default adapter (at least that is_default variable in bluetoothd doesn't seem to be in sync with what bluetoothctl displayed as default adapter when I've tried the stuff with two adapters). Or I just have missed how that is_default was changed during my few tests. Otherwise I would have named that option I've added different ("default adapter" instead of "first adapter").

That sounds like a bug to me that should be fixed. I wonder why bluetoothd actually has to carry the concept of the default adapter at all anymore.

Regards

Marcel


2015-04-14 08:33:54

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 13.04.2015 um 22:22 schrieb Marcel Holtmann:
> Hi Alexander,
>
>>>>>> As said, this (currently) needs that dbus is ready and bluetoothctl can communicate with bluetoothd. And on a reasonable fast machine (4 core x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked through /etc/rc.d/rc.local on f21).
>>>>>
>>>>> Szymon send an example on how to add wait-for-adapter in bluetoothctl. Together with that and a proper systemd unit, I bet you that this does not take 4 seconds.
>>>>>
>>>>> If you insist on doing old fashion rc.local stuff, then I can not help you. You can look into systemd target definitions, but I am pretty sure it ends up being started after something else happened.
>>>>
>>>> At least that old fashioned stuff worked. I even still have bt-alsa-devices on systems where I stick to bluez 4.0.
>>>>
>>>> And it would make me extremly wonder, if it would be any faster when using a systemd unit. Remember, I did in rc.local:
>>>>
>>>> sleep 4
>>>> echo "power on" | /bin/bluetoothctl
>>>>
>>>> And if that fails, then how could a systemd unit which calls bluetoothctl be faster? There is absolutely nothing in those 2 lines which slows down the necessary prerequisits for bluetoothctl.
>>>
>>> except the sleep 4 that you put in. So you are going on for rant this takes 4 seconds and the reason it takes 4 seconds since you put a sleep 4 in there. Szymon gave you a simple solution for removing the sleep and we are still discussing this.
>>
>> I've run into the problem, because those 4 second weren't enough after an update of bluez. I then had to use 6 seconds, otherwise that construct failed. And because that ugly sleep was just an extremly ugly workaround for the missing easy-to-use knob to reliable turn on bt on Linux with bluez 5.x, I've now written that patch.
>>
>> But it's senseless trying to discuss further. If you think people have to write udev-rules or systemd units in order to use bluetooth on Linux, there is just nothing I could add to such a discussion.
>
> you did read my responses here? I mean there are so many possible options. Szymon provided an addition for bluetoothctl command where you could just have replaced sleep 4 with the new command. And you could have it working perfectly all the time.
>
>> And you might want to decide if you want a configuration less bluez (something I think I've read several times and which is likely the reason that make install doesn't install at least an example for configuration), or if you still want to support some configuration as mentioned in the comment in regard to a [policy] section.
>
> Upstream is not responsible for personal or distribution specific customization. We provide configuration via main.conf and I am repeating myself now for the 3rd time here, I am fine with a policy setting for auto-power on found controllers. The distribution has to do the work to put a proper main.conf in place if they choose to. It is really up to them. This is not something upstream should do in the first place.
>
> bluetoothd has to work properly with default values if the configuration is present or not. Failing because a configuration file is not present and you can not run a proper sane default is plain stupid. That is not how we write software. We are doing proper defaults.

Sorry, I'm sick of having to teach others about the usability problems
of bluez.

E.g. since the configuration knob for hid2hci disappeared (I think that
happened with bluez 4.x), it's still done wrong by many people and
distributions. E.g. on Ubuntu the udev-rule for hid2hci is still wrong
because nobody explained them that the udev-rule is not the problem but
the non-existing knob to turn off hid2hci if it is installed. Not to
mention that it should be turned off by default (until the user turned
it on, hoping he is aware of the implications).

Just the same problem which happened on Fedora and Gentoo too (until
I've explained the problem to the maintainers and now hid2hci isnt
installed by default there). It's just to hard to understand (and to
explain) that installing a command-line utility (by default) might
render a system unusable. And users aren't the problem here, if even
maintainers get it wrong.

And almost anyone runs into the problem that bt-adapters are now powered
off by default and people have not idea how to change that policy or
even how to manually turn on the adapter. And that now happens since 5.x
was thrown at users, which is already 30 releases old.

>
> Please respect the upstream decision not to install main.conf in /etc/bluetooth/ and leave this up to the distribution and packagers. In case you wonder this is important for stateless installations and systems that support factory reset.
>
> I am happy to take a patch that implements the auto-power on properly and without race conditions and makes it configurable based on the main.conf policy section. And I said this many times now. The amount of emails exchanged on this topic where I agreed to take a patch if it would be done race free and in sync with how upstream operates is amazing.
>

Sorry, but I've no idea about which races you are talking. Of course, I
haven't fixed the already existing race(s) in bluetoothd with the
missing lock for the adapter list (at least I haven't seen or found any
lock for that list, otherwise I would have used it). But that still
might be my problem, as I'm not very familiar with bluetoothd and
especially it's locking strategy (if there is any, which I hope).

I also have noticed that bluetoothd and bluetoothctl seem to have a
different idea about the default adapter (at least that is_default
variable in bluetoothd doesn't seem to be in sync with what bluetoothctl
displayed as default adapter when I've tried the stuff with two
adapters). Or I just have missed how that is_default was changed during
my few tests. Otherwise I would have named that option I've added
different ("default adapter" instead of "first adapter").

Regards,

Alexander Holler

2015-04-13 20:22:48

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

>>>>> As said, this (currently) needs that dbus is ready and bluetoothctl can communicate with bluetoothd. And on a reasonable fast machine (4 core x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked through /etc/rc.d/rc.local on f21).
>>>>
>>>> Szymon send an example on how to add wait-for-adapter in bluetoothctl. Together with that and a proper systemd unit, I bet you that this does not take 4 seconds.
>>>>
>>>> If you insist on doing old fashion rc.local stuff, then I can not help you. You can look into systemd target definitions, but I am pretty sure it ends up being started after something else happened.
>>>
>>> At least that old fashioned stuff worked. I even still have bt-alsa-devices on systems where I stick to bluez 4.0.
>>>
>>> And it would make me extremly wonder, if it would be any faster when using a systemd unit. Remember, I did in rc.local:
>>>
>>> sleep 4
>>> echo "power on" | /bin/bluetoothctl
>>>
>>> And if that fails, then how could a systemd unit which calls bluetoothctl be faster? There is absolutely nothing in those 2 lines which slows down the necessary prerequisits for bluetoothctl.
>>
>> except the sleep 4 that you put in. So you are going on for rant this takes 4 seconds and the reason it takes 4 seconds since you put a sleep 4 in there. Szymon gave you a simple solution for removing the sleep and we are still discussing this.
>
> I've run into the problem, because those 4 second weren't enough after an update of bluez. I then had to use 6 seconds, otherwise that construct failed. And because that ugly sleep was just an extremly ugly workaround for the missing easy-to-use knob to reliable turn on bt on Linux with bluez 5.x, I've now written that patch.
>
> But it's senseless trying to discuss further. If you think people have to write udev-rules or systemd units in order to use bluetooth on Linux, there is just nothing I could add to such a discussion.

you did read my responses here? I mean there are so many possible options. Szymon provided an addition for bluetoothctl command where you could just have replaced sleep 4 with the new command. And you could have it working perfectly all the time.

> And you might want to decide if you want a configuration less bluez (something I think I've read several times and which is likely the reason that make install doesn't install at least an example for configuration), or if you still want to support some configuration as mentioned in the comment in regard to a [policy] section.

Upstream is not responsible for personal or distribution specific customization. We provide configuration via main.conf and I am repeating myself now for the 3rd time here, I am fine with a policy setting for auto-power on found controllers. The distribution has to do the work to put a proper main.conf in place if they choose to. It is really up to them. This is not something upstream should do in the first place.

bluetoothd has to work properly with default values if the configuration is present or not. Failing because a configuration file is not present and you can not run a proper sane default is plain stupid. That is not how we write software. We are doing proper defaults.

Please respect the upstream decision not to install main.conf in /etc/bluetooth/ and leave this up to the distribution and packagers. In case you wonder this is important for stateless installations and systems that support factory reset.

I am happy to take a patch that implements the auto-power on properly and without race conditions and makes it configurable based on the main.conf policy section. And I said this many times now. The amount of emails exchanged on this topic where I agreed to take a patch if it would be done race free and in sync with how upstream operates is amazing.

So the choice is really yours here. However upstream has been pretty much inviting here to handle your use case. Fun part is just you seem to disagree with upstream's proposal. And that is your right, but I am honestly not spending any more time on this. The direction is crystal clear. And with Szymon's patch for bluetoothctl you would even have a temporary solution that is race free and dead simple to deploy. Then again, you seemed to prefer argumenting for a sleep vs a proper command. And honestly with that dismissal of people trying to help, nobody is going to bother in the end.

Regards

Marcel


2015-04-13 19:08:03

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 13.04.2015 um 16:32 schrieb Marcel Holtmann:

>>>> As said, this (currently) needs that dbus is ready and bluetoothctl can communicate with bluetoothd. And on a reasonable fast machine (4 core x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked through /etc/rc.d/rc.local on f21).
>>>
>>> Szymon send an example on how to add wait-for-adapter in bluetoothctl. Together with that and a proper systemd unit, I bet you that this does not take 4 seconds.
>>>
>>> If you insist on doing old fashion rc.local stuff, then I can not help you. You can look into systemd target definitions, but I am pretty sure it ends up being started after something else happened.
>>
>> At least that old fashioned stuff worked. I even still have bt-alsa-devices on systems where I stick to bluez 4.0.
>>
>> And it would make me extremly wonder, if it would be any faster when using a systemd unit. Remember, I did in rc.local:
>>
>> sleep 4
>> echo "power on" | /bin/bluetoothctl
>>
>> And if that fails, then how could a systemd unit which calls bluetoothctl be faster? There is absolutely nothing in those 2 lines which slows down the necessary prerequisits for bluetoothctl.
>
> except the sleep 4 that you put in. So you are going on for rant this takes 4 seconds and the reason it takes 4 seconds since you put a sleep 4 in there. Szymon gave you a simple solution for removing the sleep and we are still discussing this.

I've run into the problem, because those 4 second weren't enough after
an update of bluez. I then had to use 6 seconds, otherwise that
construct failed. And because that ugly sleep was just an extremly ugly
workaround for the missing easy-to-use knob to reliable turn on bt on
Linux with bluez 5.x, I've now written that patch.

But it's senseless trying to discuss further. If you think people have
to write udev-rules or systemd units in order to use bluetooth on Linux,
there is just nothing I could add to such a discussion.

And you might want to decide if you want a configuration less bluez
(something I think I've read several times and which is likely the
reason that make install doesn't install at least an example for
configuration), or if you still want to support some configuration as
mentioned in the comment in regard to a [policy] section.

Alexander Holler

2015-04-13 14:32:56

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

>>>>>>>>> We took the power on policy out of BlueZ 5 on purpose.
>>>>>>>>
>>>>>>>> Nobody has to use the option which is by default off.
>>>>>>>
>>>>>>> Sorry, but I just have to ask that in order to keep my reputation as
>>>>>>> troll:
>>>>>>>
>>>>>>> What's that purpose?
>>>>>>>
>>>>>>> Defeating usability? ;)
>>>>>>
>>>>>> Just to clarify my patch. Its purpose is not to reimplement a "power on
>>>>>> by default" policy because I understand that it's a good idea to not
>>>>>> power on bt-adapters by default.
>>>>>>
>>>>>> The purpose of my patch is to offer users a simple, fast and reliable
>>>>>> way to power on an adapter on startup, without the need to wait until
>>>>>> dbus and udev are ready and without the need for additional tools (or
>>>>>> complications) like udev-rules, hciconfig or bluetootctl.
>>>>>
>>>>>
>>>>> And just in case some people don't understand why I'm calling a simple looking udev-rule a complication, here is an incomplete list what all might fail when using such error-prone mechanisms:
>>>>>
>>>>> As example I use the rule
>>>>>
>>>>> ACTION=="add", KERNEL=="hci0", RUN+="/usr/bin/hciconfig hci0 up"
>>>>>
>>>>> which might work as a bad replacement for the patch I've posted.
>>>>>
>>>>> - There is the devicename (hci0). That might change.
>>>>
>>>> that is actually not true. The device name stays the same as long as the hardware is plugged in.
>>>
>>> Yes, but the initial name might change. We just (a year or 2 ago) had such for most network devices and since that change I have e.g. only a 70% chance that a bt-network device is named bnep0 when I'm starting a bnep-connection. The other 30% it's named en* (on one of my systems where I regulary do so). Up to now I still haven't searched where that randomness does come from and I'm using it here just as an example for an existing uncertainty in regard to device names. There's no need to discuss this problem further in this thread.
>>
>> I you start intermixing network interface names with Bluetooth HCI device names, then there is really nothing much more to add. They are two different things. Plain and simple.
>
> I've used it as an example that device names might change, in general, to explain one of the many possible failures if there is a need for an udev-rule which calls another executable. Nothing else. Try to abstract.
>
>>>>> - There is the need for a working hciconfig-executable.
>>>>> - The path to the executable might change (think at /usr/bin -> /bin)
>>>>> - The call to hciconfig (like options) might change without notice
>>>>
>>>> hciconfig is a deprecated tool and should not be used anymore. That might be your biggest problem that the binary might go away in BlueZ 6.
>>>>
>>>>> - The partition the unnecessarily needed third executable (besides bluetoothd and dbus) might be mounted to late (maybe through a change in some mount order)
>>>>
>>>> To be fair, we could just built this into bluetoothctl as a command.
>>>
>>> As said, this (currently) needs that dbus is ready and bluetoothctl can communicate with bluetoothd. And on a reasonable fast machine (4 core x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked through /etc/rc.d/rc.local on f21).
>>
>> Szymon send an example on how to add wait-for-adapter in bluetoothctl. Together with that and a proper systemd unit, I bet you that this does not take 4 seconds.
>>
>> If you insist on doing old fashion rc.local stuff, then I can not help you. You can look into systemd target definitions, but I am pretty sure it ends up being started after something else happened.
>
> At least that old fashioned stuff worked. I even still have bt-alsa-devices on systems where I stick to bluez 4.0.
>
> And it would make me extremly wonder, if it would be any faster when using a systemd unit. Remember, I did in rc.local:
>
> sleep 4
> echo "power on" | /bin/bluetoothctl
>
> And if that fails, then how could a systemd unit which calls bluetoothctl be faster? There is absolutely nothing in those 2 lines which slows down the necessary prerequisits for bluetoothctl.

except the sleep 4 that you put in. So you are going on for rant this takes 4 seconds and the reason it takes 4 seconds since you put a sleep 4 in there. Szymon gave you a simple solution for removing the sleep and we are still discussing this.

> So, now were there, people are called old fashioned, if they even use systemd but still stick to some easy to use script. Nice new Linux world where nothing works. No wonder Linux on desktops (which aren't Android or Chrome*) is dead ...
>
>
>>>>> - the udev-rule syntax might change
>>>>
>>>> That would be a udev / systemd regression that one should highly object against.
>>>>
>>>>> - udev might not be available at all (there are still some alternatives)
>>>>
>>>> And now we are getting into the funny cases where it is a home baked distro. Then someone might also just compile their own version of BlueZ with all sorts of patches. Upstream can not support every corner cases. So this is something I would not entertain at all.
>>>>
>>>>> - ...
>>>>>
>>>>> And please don't try to tell me that all of the above possiblities for errors are pure fiction which only might be found inside my brain. I've experienced almost all of them (not in regard to bluez or hciconfig) and most of them not just once or on systems I (mis)configured myself.
>>>>
>>>> What we can do is add an option AutomaticEnabling=true,false into the [Policy] section that would equally just power on all controllers when they are found.
>>>
>>> Then people will ask which [Policy] section? E.g. on Fedora the configuration file disappeared completely. There's none in /etc anymore and no example in /usr/share/doc (likely because make install doesn't install such).
>>
>> That is a documentation problem. Send patches for it.
>
>
> Why should I send any further patches if I already fail with such simple ones?

I am not taking patches that are just hacks only because they work for you. It is plain and simple like that. I offered a way for adding this in a generic fashion that will be actually useful for others and would work for you, but it seems nobody cares.

> Anyway. It's ok. I accept that my patch is an unbelievable piece of shit which renders bluetoothd into a big bug and kills almost any Linux system.
>
> But maybe it could help other victims. That was the only reason I've posted this patch and even tried to explain the motivations to not have the unnecessary need for further complications besides a simple additional command line option for bluetoothd.

Regards

Marcel


2015-04-13 09:10:06

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 12.04.2015 um 20:50 schrieb Marcel Holtmann:
> Hi Alexander,
>
>>>>>>>> We took the power on policy out of BlueZ 5 on purpose.
>>>>>>>
>>>>>>> Nobody has to use the option which is by default off.
>>>>>>
>>>>>> Sorry, but I just have to ask that in order to keep my reputation as
>>>>>> troll:
>>>>>>
>>>>>> What's that purpose?
>>>>>>
>>>>>> Defeating usability? ;)
>>>>>
>>>>> Just to clarify my patch. Its purpose is not to reimplement a "power on
>>>>> by default" policy because I understand that it's a good idea to not
>>>>> power on bt-adapters by default.
>>>>>
>>>>> The purpose of my patch is to offer users a simple, fast and reliable
>>>>> way to power on an adapter on startup, without the need to wait until
>>>>> dbus and udev are ready and without the need for additional tools (or
>>>>> complications) like udev-rules, hciconfig or bluetootctl.
>>>>
>>>>
>>>> And just in case some people don't understand why I'm calling a simple looking udev-rule a complication, here is an incomplete list what all might fail when using such error-prone mechanisms:
>>>>
>>>> As example I use the rule
>>>>
>>>> ACTION=="add", KERNEL=="hci0", RUN+="/usr/bin/hciconfig hci0 up"
>>>>
>>>> which might work as a bad replacement for the patch I've posted.
>>>>
>>>> - There is the devicename (hci0). That might change.
>>>
>>> that is actually not true. The device name stays the same as long as the hardware is plugged in.
>>
>> Yes, but the initial name might change. We just (a year or 2 ago) had such for most network devices and since that change I have e.g. only a 70% chance that a bt-network device is named bnep0 when I'm starting a bnep-connection. The other 30% it's named en* (on one of my systems where I regulary do so). Up to now I still haven't searched where that randomness does come from and I'm using it here just as an example for an existing uncertainty in regard to device names. There's no need to discuss this problem further in this thread.
>
> I you start intermixing network interface names with Bluetooth HCI device names, then there is really nothing much more to add. They are two different things. Plain and simple.

I've used it as an example that device names might change, in general,
to explain one of the many possible failures if there is a need for an
udev-rule which calls another executable. Nothing else. Try to abstract.

>>>> - There is the need for a working hciconfig-executable.
>>>> - The path to the executable might change (think at /usr/bin -> /bin)
>>>> - The call to hciconfig (like options) might change without notice
>>>
>>> hciconfig is a deprecated tool and should not be used anymore. That might be your biggest problem that the binary might go away in BlueZ 6.
>>>
>>>> - The partition the unnecessarily needed third executable (besides bluetoothd and dbus) might be mounted to late (maybe through a change in some mount order)
>>>
>>> To be fair, we could just built this into bluetoothctl as a command.
>>
>> As said, this (currently) needs that dbus is ready and bluetoothctl can communicate with bluetoothd. And on a reasonable fast machine (4 core x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked through /etc/rc.d/rc.local on f21).
>
> Szymon send an example on how to add wait-for-adapter in bluetoothctl. Together with that and a proper systemd unit, I bet you that this does not take 4 seconds.
>
> If you insist on doing old fashion rc.local stuff, then I can not help you. You can look into systemd target definitions, but I am pretty sure it ends up being started after something else happened.

At least that old fashioned stuff worked. I even still have
bt-alsa-devices on systems where I stick to bluez 4.0.

And it would make me extremly wonder, if it would be any faster when
using a systemd unit. Remember, I did in rc.local:

sleep 4
echo "power on" | /bin/bluetoothctl

And if that fails, then how could a systemd unit which calls
bluetoothctl be faster? There is absolutely nothing in those 2 lines
which slows down the necessary prerequisits for bluetoothctl.

So, now were there, people are called old fashioned, if they even use
systemd but still stick to some easy to use script. Nice new Linux world
where nothing works. No wonder Linux on desktops (which aren't Android
or Chrome*) is dead ...


>>>> - the udev-rule syntax might change
>>>
>>> That would be a udev / systemd regression that one should highly object against.
>>>
>>>> - udev might not be available at all (there are still some alternatives)
>>>
>>> And now we are getting into the funny cases where it is a home baked distro. Then someone might also just compile their own version of BlueZ with all sorts of patches. Upstream can not support every corner cases. So this is something I would not entertain at all.
>>>
>>>> - ...
>>>>
>>>> And please don't try to tell me that all of the above possiblities for errors are pure fiction which only might be found inside my brain. I've experienced almost all of them (not in regard to bluez or hciconfig) and most of them not just once or on systems I (mis)configured myself.
>>>
>>> What we can do is add an option AutomaticEnabling=true,false into the [Policy] section that would equally just power on all controllers when they are found.
>>
>> Then people will ask which [Policy] section? E.g. on Fedora the configuration file disappeared completely. There's none in /etc anymore and no example in /usr/share/doc (likely because make install doesn't install such).
>
> That is a documentation problem. Send patches for it.


Why should I send any further patches if I already fail with such simple
ones?

Anyway. It's ok. I accept that my patch is an unbelievable piece of shit
which renders bluetoothd into a big bug and kills almost any Linux system.

But maybe it could help other victims. That was the only reason I've
posted this patch and even tried to explain the motivations to not have
the unnecessary need for further complications besides a simple
additional command line option for bluetoothd.

Alexander Holler

2015-04-12 18:50:20

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

>>>>>>> We took the power on policy out of BlueZ 5 on purpose.
>>>>>>
>>>>>> Nobody has to use the option which is by default off.
>>>>>
>>>>> Sorry, but I just have to ask that in order to keep my reputation as
>>>>> troll:
>>>>>
>>>>> What's that purpose?
>>>>>
>>>>> Defeating usability? ;)
>>>>
>>>> Just to clarify my patch. Its purpose is not to reimplement a "power on
>>>> by default" policy because I understand that it's a good idea to not
>>>> power on bt-adapters by default.
>>>>
>>>> The purpose of my patch is to offer users a simple, fast and reliable
>>>> way to power on an adapter on startup, without the need to wait until
>>>> dbus and udev are ready and without the need for additional tools (or
>>>> complications) like udev-rules, hciconfig or bluetootctl.
>>>
>>>
>>> And just in case some people don't understand why I'm calling a simple looking udev-rule a complication, here is an incomplete list what all might fail when using such error-prone mechanisms:
>>>
>>> As example I use the rule
>>>
>>> ACTION=="add", KERNEL=="hci0", RUN+="/usr/bin/hciconfig hci0 up"
>>>
>>> which might work as a bad replacement for the patch I've posted.
>>>
>>> - There is the devicename (hci0). That might change.
>>
>> that is actually not true. The device name stays the same as long as the hardware is plugged in.
>
> Yes, but the initial name might change. We just (a year or 2 ago) had such for most network devices and since that change I have e.g. only a 70% chance that a bt-network device is named bnep0 when I'm starting a bnep-connection. The other 30% it's named en* (on one of my systems where I regulary do so). Up to now I still haven't searched where that randomness does come from and I'm using it here just as an example for an existing uncertainty in regard to device names. There's no need to discuss this problem further in this thread.

I you start intermixing network interface names with Bluetooth HCI device names, then there is really nothing much more to add. They are two different things. Plain and simple.

>>> - There is the need for a working hciconfig-executable.
>>> - The path to the executable might change (think at /usr/bin -> /bin)
>>> - The call to hciconfig (like options) might change without notice
>>
>> hciconfig is a deprecated tool and should not be used anymore. That might be your biggest problem that the binary might go away in BlueZ 6.
>>
>>> - The partition the unnecessarily needed third executable (besides bluetoothd and dbus) might be mounted to late (maybe through a change in some mount order)
>>
>> To be fair, we could just built this into bluetoothctl as a command.
>
> As said, this (currently) needs that dbus is ready and bluetoothctl can communicate with bluetoothd. And on a reasonable fast machine (4 core x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked through /etc/rc.d/rc.local on f21).

Szymon send an example on how to add wait-for-adapter in bluetoothctl. Together with that and a proper systemd unit, I bet you that this does not take 4 seconds.

If you insist on doing old fashion rc.local stuff, then I can not help you. You can look into systemd target definitions, but I am pretty sure it ends up being started after something else happened.

>>> - the udev-rule syntax might change
>>
>> That would be a udev / systemd regression that one should highly object against.
>>
>>> - udev might not be available at all (there are still some alternatives)
>>
>> And now we are getting into the funny cases where it is a home baked distro. Then someone might also just compile their own version of BlueZ with all sorts of patches. Upstream can not support every corner cases. So this is something I would not entertain at all.
>>
>>> - ...
>>>
>>> And please don't try to tell me that all of the above possiblities for errors are pure fiction which only might be found inside my brain. I've experienced almost all of them (not in regard to bluez or hciconfig) and most of them not just once or on systems I (mis)configured myself.
>>
>> What we can do is add an option AutomaticEnabling=true,false into the [Policy] section that would equally just power on all controllers when they are found.
>
> Then people will ask which [Policy] section? E.g. on Fedora the configuration file disappeared completely. There's none in /etc anymore and no example in /usr/share/doc (likely because make install doesn't install such).

That is a documentation problem. Send patches for it.

>> This needs to be clearly marked as policy since that is what it is. And it also needs to be implemented correctly. Since you need to program all the keys before you power on. Otherwise you might have tons of race conditions when it comes to re-connection of the mouse/keyboard and the key is missing.
>>
>> One more complicated option would to do the Apple way in the sense, when there is not keyboard attached, they start the HID pairing UI to let you pair one. However that should be better done in the UI.
>
> I'm not just talking about BT HID devices, also these are likely the main use cases. It's about automatically turning on a bt-adapter I either plug in or which is fixed, if the user wants such a behaviour.

Let me repeat this. This is a policy decision. I proposed a way for doing that with BlueZ in an upstream compatible fashion that fits into the framework.

> And I don't want to end up in a discussion about the necessary initial pairing processs for bt-keyboards. On Linux, since ever, you need either a second keyboard or an ssh connection to do so. That's a complete different problem and isn't easy to solve without some special sw in order to do this without the need for a different input device (or network login or similiar).
>
> What I just don't understand is, what's wrong with that simple command line option done with a simple patch (3 files changed, 15 insertions(+), 4 deletions(-))?

Your patch is wrong. I explained that in my initial response. Either you believe me or you don't. Saying that you hacked up something and insisting is not going to make your case here.

> Why should there be a need to use bluetoothctl for that purpose? I don't care if it's my patch with names I've choosen for the option and would be happy with any patch from any author which would let me do this with a simple command line option for bluetoothd.

You did read my response. Either it become an option in the [Policy] section of main.conf or you do it with bluetoothctl. You can make your choice, but it is one of these and not some freaky command line switch.

Regards

Marcel


2015-04-12 09:23:09

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 11.04.2015 um 19:55 schrieb Marcel Holtmann:
> Hi Alexander,
>
>>>>>> We took the power on policy out of BlueZ 5 on purpose.
>>>>>
>>>>> Nobody has to use the option which is by default off.
>>>>
>>>> Sorry, but I just have to ask that in order to keep my reputation as
>>>> troll:
>>>>
>>>> What's that purpose?
>>>>
>>>> Defeating usability? ;)
>>>
>>> Just to clarify my patch. Its purpose is not to reimplement a "power on
>>> by default" policy because I understand that it's a good idea to not
>>> power on bt-adapters by default.
>>>
>>> The purpose of my patch is to offer users a simple, fast and reliable
>>> way to power on an adapter on startup, without the need to wait until
>>> dbus and udev are ready and without the need for additional tools (or
>>> complications) like udev-rules, hciconfig or bluetootctl.
>>
>>
>> And just in case some people don't understand why I'm calling a simple looking udev-rule a complication, here is an incomplete list what all might fail when using such error-prone mechanisms:
>>
>> As example I use the rule
>>
>> ACTION=="add", KERNEL=="hci0", RUN+="/usr/bin/hciconfig hci0 up"
>>
>> which might work as a bad replacement for the patch I've posted.
>>
>> - There is the devicename (hci0). That might change.
>
> that is actually not true. The device name stays the same as long as the hardware is plugged in.

Yes, but the initial name might change. We just (a year or 2 ago) had
such for most network devices and since that change I have e.g. only a
70% chance that a bt-network device is named bnep0 when I'm starting a
bnep-connection. The other 30% it's named en* (on one of my systems
where I regulary do so). Up to now I still haven't searched where that
randomness does come from and I'm using it here just as an example for
an existing uncertainty in regard to device names. There's no need to
discuss this problem further in this thread.

>> - There is the need for a working hciconfig-executable.
>> - The path to the executable might change (think at /usr/bin -> /bin)
>> - The call to hciconfig (like options) might change without notice
>
> hciconfig is a deprecated tool and should not be used anymore. That might be your biggest problem that the binary might go away in BlueZ 6.
>
>> - The partition the unnecessarily needed third executable (besides bluetoothd and dbus) might be mounted to late (maybe through a change in some mount order)
>
> To be fair, we could just built this into bluetoothctl as a command.

As said, this (currently) needs that dbus is ready and bluetoothctl can
communicate with bluetoothd. And on a reasonable fast machine (4 core
x86_64 2ghz+) this needs more than 4 seconds after startup here (invoked
through /etc/rc.d/rc.local on f21).

>
>> - the udev-rule syntax might change
>
> That would be a udev / systemd regression that one should highly object against.
>
>> - udev might not be available at all (there are still some alternatives)
>
> And now we are getting into the funny cases where it is a home baked distro. Then someone might also just compile their own version of BlueZ with all sorts of patches. Upstream can not support every corner cases. So this is something I would not entertain at all.
>
>> - ...
>>
>> And please don't try to tell me that all of the above possiblities for errors are pure fiction which only might be found inside my brain. I've experienced almost all of them (not in regard to bluez or hciconfig) and most of them not just once or on systems I (mis)configured myself.
>
> What we can do is add an option AutomaticEnabling=true,false into the [Policy] section that would equally just power on all controllers when they are found.

Then people will ask which [Policy] section? E.g. on Fedora the
configuration file disappeared completely. There's none in /etc anymore
and no example in /usr/share/doc (likely because make install doesn't
install such).

>
> This needs to be clearly marked as policy since that is what it is. And it also needs to be implemented correctly. Since you need to program all the keys before you power on. Otherwise you might have tons of race conditions when it comes to re-connection of the mouse/keyboard and the key is missing.
>
> One more complicated option would to do the Apple way in the sense, when there is not keyboard attached, they start the HID pairing UI to let you pair one. However that should be better done in the UI.

I'm not just talking about BT HID devices, also these are likely the
main use cases. It's about automatically turning on a bt-adapter I
either plug in or which is fixed, if the user wants such a behaviour.

And I don't want to end up in a discussion about the necessary initial
pairing processs for bt-keyboards. On Linux, since ever, you need either
a second keyboard or an ssh connection to do so. That's a complete
different problem and isn't easy to solve without some special sw in
order to do this without the need for a different input device (or
network login or similiar).

What I just don't understand is, what's wrong with that simple command
line option done with a simple patch (3 files changed, 15 insertions(+),
4 deletions(-))?

Why should there be a need to use bluetoothctl for that purpose? I don't
care if it's my patch with names I've choosen for the option and would
be happy with any patch from any author which would let me do this with
a simple command line option for bluetoothd.

Regards,

Alexander Holler

2015-04-11 17:55:09

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

>>>>> We took the power on policy out of BlueZ 5 on purpose.
>>>>
>>>> Nobody has to use the option which is by default off.
>>>
>>> Sorry, but I just have to ask that in order to keep my reputation as
>>> troll:
>>>
>>> What's that purpose?
>>>
>>> Defeating usability? ;)
>>
>> Just to clarify my patch. Its purpose is not to reimplement a "power on
>> by default" policy because I understand that it's a good idea to not
>> power on bt-adapters by default.
>>
>> The purpose of my patch is to offer users a simple, fast and reliable
>> way to power on an adapter on startup, without the need to wait until
>> dbus and udev are ready and without the need for additional tools (or
>> complications) like udev-rules, hciconfig or bluetootctl.
>
>
> And just in case some people don't understand why I'm calling a simple looking udev-rule a complication, here is an incomplete list what all might fail when using such error-prone mechanisms:
>
> As example I use the rule
>
> ACTION=="add", KERNEL=="hci0", RUN+="/usr/bin/hciconfig hci0 up"
>
> which might work as a bad replacement for the patch I've posted.
>
> - There is the devicename (hci0). That might change.

that is actually not true. The device name stays the same as long as the hardware is plugged in.

> - There is the need for a working hciconfig-executable.
> - The path to the executable might change (think at /usr/bin -> /bin)
> - The call to hciconfig (like options) might change without notice

hciconfig is a deprecated tool and should not be used anymore. That might be your biggest problem that the binary might go away in BlueZ 6.

> - The partition the unnecessarily needed third executable (besides bluetoothd and dbus) might be mounted to late (maybe through a change in some mount order)

To be fair, we could just built this into bluetoothctl as a command.

> - the udev-rule syntax might change

That would be a udev / systemd regression that one should highly object against.

> - udev might not be available at all (there are still some alternatives)

And now we are getting into the funny cases where it is a home baked distro. Then someone might also just compile their own version of BlueZ with all sorts of patches. Upstream can not support every corner cases. So this is something I would not entertain at all.

> - ...
>
> And please don't try to tell me that all of the above possiblities for errors are pure fiction which only might be found inside my brain. I've experienced almost all of them (not in regard to bluez or hciconfig) and most of them not just once or on systems I (mis)configured myself.

What we can do is add an option AutomaticEnabling=true,false into the [Policy] section that would equally just power on all controllers when they are found.

This needs to be clearly marked as policy since that is what it is. And it also needs to be implemented correctly. Since you need to program all the keys before you power on. Otherwise you might have tons of race conditions when it comes to re-connection of the mouse/keyboard and the key is missing.

One more complicated option would to do the Apple way in the sense, when there is not keyboard attached, they start the HID pairing UI to let you pair one. However that should be better done in the UI.

Regards

Marcel


2015-04-11 16:48:35

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 11.04.2015 um 07:07 schrieb Alexander Holler:
> Am 11.04.2015 um 06:06 schrieb Alexander Holler:
>> Am 10.04.2015 um 19:15 schrieb Alexander Holler:
>>> Am 10.04.2015 um 19:10 schrieb Marcel Holtmann:
>>
>>>> We took the power on policy out of BlueZ 5 on purpose.
>>>
>>> Nobody has to use the option which is by default off.
>>
>> Sorry, but I just have to ask that in order to keep my reputation as
>> troll:
>>
>> What's that purpose?
>>
>> Defeating usability? ;)
>
> Just to clarify my patch. Its purpose is not to reimplement a "power on
> by default" policy because I understand that it's a good idea to not
> power on bt-adapters by default.
>
> The purpose of my patch is to offer users a simple, fast and reliable
> way to power on an adapter on startup, without the need to wait until
> dbus and udev are ready and without the need for additional tools (or
> complications) like udev-rules, hciconfig or bluetootctl.


And just in case some people don't understand why I'm calling a simple
looking udev-rule a complication, here is an incomplete list what all
might fail when using such error-prone mechanisms:

As example I use the rule

ACTION=="add", KERNEL=="hci0", RUN+="/usr/bin/hciconfig hci0 up"

which might work as a bad replacement for the patch I've posted.

- There is the devicename (hci0). That might change.
- There is the need for a working hciconfig-executable.
- The path to the executable might change (think at /usr/bin -> /bin)
- The call to hciconfig (like options) might change without notice
- The partition the unnecessarily needed third executable (besides
bluetoothd and dbus) might be mounted to late (maybe through a change in
some mount order)
- the udev-rule syntax might change
- udev might not be available at all (there are still some alternatives)
- ...

And please don't try to tell me that all of the above possiblities for
errors are pure fiction which only might be found inside my brain. I've
experienced almost all of them (not in regard to bluez or hciconfig) and
most of them not just once or on systems I (mis)configured myself.


> E.g. one of my use-cases is to be able to use a bt-keyboard right after
> the system shows me a login. Without that patch waiting 4 seconds
> haven't been enough, and with the patch the keyboard is now usable
> almost immediately.
>
> And there are many other use cases where people do want that an adapter
> is turned on after it's found. E.g. I assume on most systems which don't
> have a fixed bt-adapter but rely on usb-dongles, people might prefer
> that their dongle will be automatically turned on when it's plugged in
> (otherwise they likely wouldn't plug it in). And the patch offers just a
> very simple, clean and easy to use way to reach that goal and is not a
> try to change the default policy.
>
> Regards,
>
> Alexander Holler

2015-04-11 05:07:13

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 11.04.2015 um 06:06 schrieb Alexander Holler:
> Am 10.04.2015 um 19:15 schrieb Alexander Holler:
>> Am 10.04.2015 um 19:10 schrieb Marcel Holtmann:
>
>>> We took the power on policy out of BlueZ 5 on purpose.
>>
>> Nobody has to use the option which is by default off.
>
> Sorry, but I just have to ask that in order to keep my reputation as troll:
>
> What's that purpose?
>
> Defeating usability? ;)

Just to clarify my patch. Its purpose is not to reimplement a "power on
by default" policy because I understand that it's a good idea to not
power on bt-adapters by default.

The purpose of my patch is to offer users a simple, fast and reliable
way to power on an adapter on startup, without the need to wait until
dbus and udev are ready and without the need for additional tools (or
complications) like udev-rules, hciconfig or bluetootctl.

E.g. one of my use-cases is to be able to use a bt-keyboard right after
the system shows me a login. Without that patch waiting 4 seconds
haven't been enough, and with the patch the keyboard is now usable
almost immediately.

And there are many other use cases where people do want that an adapter
is turned on after it's found. E.g. I assume on most systems which don't
have a fixed bt-adapter but rely on usb-dongles, people might prefer
that their dongle will be automatically turned on when it's plugged in
(otherwise they likely wouldn't plug it in). And the patch offers just a
very simple, clean and easy to use way to reach that goal and is not a
try to change the default policy.

Regards,

Alexander Holler

2015-04-11 04:06:43

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 10.04.2015 um 19:15 schrieb Alexander Holler:
> Am 10.04.2015 um 19:10 schrieb Marcel Holtmann:

>> We took the power on policy out of BlueZ 5 on purpose.
>
> Nobody has to use the option which is by default off.

Sorry, but I just have to ask that in order to keep my reputation as troll:

What's that purpose?

Defeating usability? ;)

Regards,

Alexander Holler

2015-04-10 17:15:17

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Am 10.04.2015 um 19:10 schrieb Marcel Holtmann:
> Hi Alexander,
>
>> You want this option if you're using e.g. a bt-keyboard as the only
>> input device. Other solutions to power on an adapter at startup are
>> either unreliable or too complicated.
>>
>> E.g. I had a
>>
>> sleep 4
>> echo 'power on' | bluetoothctl
>>
>> in some startup-script. And with an update of bluez from 5.23 to
>> 5.29 Murphy visited me and I had a box without a usable keyboard
>> because those 4 second haven't been enough anymore.
>
> I think you better have a special script with something like wait-for-adapter or something similar instead of trying to force policy into the daemon.

Sorry, this won't happen and as I'm already used to have to patch big
parts of any Linux system which should be usable I don't care much if
this patch ends up in bluez. ;)

> We took the power on policy out of BlueZ 5 on purpose.

Nobody has to use the option which is by default off.

Thanks for the fast response.

Regards,

Alexander Holler

2015-04-10 17:15:17

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi,

On Friday 10 of April 2015 10:10:18 Marcel Holtmann wrote:
> Hi Alexander,
>
> > You want this option if you're using e.g. a bt-keyboard as the only
> > input device. Other solutions to power on an adapter at startup are
> > either unreliable or too complicated.
> >
> > E.g. I had a
> >
> > sleep 4
> > echo 'power on' | bluetoothctl
> >
> > in some startup-script. And with an update of bluez from 5.23 to
> > 5.29 Murphy visited me and I had a box without a usable keyboard
> > because those 4 second haven't been enough anymore.
>
> I think you better have a special script with something like
> wait-for-adapter or something similar instead of trying to force policy
> into the daemon.

I've send RFC patch that adds 'wait for adapter to show up' as an option to
bluetoothctl before it process stdin. This should allow to script things like
echo 'power on' | bluetoothctl -w
without need for extra dbus scripting.

> We took the power on policy out of BlueZ 5 on purpose.
>
> > ---
> > src/adapter.c | 10 ++++++++--
> > src/adapter.h | 2 +-
> > src/main.c | 7 ++++++-
> > 3 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index 7ffd302..10de028 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -117,6 +117,8 @@ static GSList *adapter_drivers = NULL;
> > static GSList *disconnect_list = NULL;
> > static GSList *conn_fail_list = NULL;
> >
> > +static bool power_on_first_adapter;
> > +
> > struct link_key_info {
> >
> > bdaddr_t bdaddr;
> > unsigned char key[16];
> >
> > @@ -7315,8 +7317,11 @@ static int adapter_register(struct btd_adapter
> > *adapter)>
> > return -EINVAL;
> >
> > }
> >
> > - if (adapters == NULL)
> > + if (adapters == NULL) {
> >
> > adapter->is_default = true;
> >
> > + if (power_on_first_adapter)
> > + set_mode(adapter, MGMT_OP_SET_POWERED, 0x01);
> > + }
>
> And this is pretty much the wrong location. You would power on the
> controller with old keys and device list programmed into it. That is not a
> good idea.
> > adapters = g_slist_append(adapters, adapter);
> >
> > @@ -8132,8 +8137,9 @@ static void mgmt_debug(const char *str, void
> > *user_data)>
> > info("%s%s", prefix, str);
> >
> > }
>
> Regards
>
> Marcel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
BR
Szymon Janc

2015-04-10 17:10:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

Hi Alexander,

> You want this option if you're using e.g. a bt-keyboard as the only
> input device. Other solutions to power on an adapter at startup are
> either unreliable or too complicated.
>
> E.g. I had a
>
> sleep 4
> echo 'power on' | bluetoothctl
>
> in some startup-script. And with an update of bluez from 5.23 to
> 5.29 Murphy visited me and I had a box without a usable keyboard
> because those 4 second haven't been enough anymore.

I think you better have a special script with something like wait-for-adapter or something similar instead of trying to force policy into the daemon.

We took the power on policy out of BlueZ 5 on purpose.

> ---
> src/adapter.c | 10 ++++++++--
> src/adapter.h | 2 +-
> src/main.c | 7 ++++++-
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 7ffd302..10de028 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -117,6 +117,8 @@ static GSList *adapter_drivers = NULL;
> static GSList *disconnect_list = NULL;
> static GSList *conn_fail_list = NULL;
>
> +static bool power_on_first_adapter;
> +
> struct link_key_info {
> bdaddr_t bdaddr;
> unsigned char key[16];
> @@ -7315,8 +7317,11 @@ static int adapter_register(struct btd_adapter *adapter)
> return -EINVAL;
> }
>
> - if (adapters == NULL)
> + if (adapters == NULL) {
> adapter->is_default = true;
> + if (power_on_first_adapter)
> + set_mode(adapter, MGMT_OP_SET_POWERED, 0x01);
> + }

And this is pretty much the wrong location. You would power on the controller with old keys and device list programmed into it. That is not a good idea.

> adapters = g_slist_append(adapters, adapter);
>
> @@ -8132,8 +8137,9 @@ static void mgmt_debug(const char *str, void *user_data)
> info("%s%s", prefix, str);
> }

Regards

Marcel