All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/bluez5_utils: expose more disable options
@ 2021-07-26 20:12 Michael Nosthoff via buildroot
  2021-07-26 21:07 ` Thomas Petazzoni
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Nosthoff via buildroot @ 2021-07-26 20:12 UTC (permalink / raw)
  To: buildroot; +Cc: Michael Nosthoff, Marcin Bis

BlueZ builds a lot of Classic BT profiles by default but allows
to disable them. This is especially handy when only BLE is needed
and enabled in the kernel.

Otherwise this yields warnings like this on bootup:

 profiles/network/bnep.c:bnep_init() kernel lacks bnep-protocol support
 src/plugin.c:plugin_init() System does not support network plugin

Also it allows to disable btmon which should not be needed on
production systems and is ~800KB in size.

Expose those options but default to 'y' to no break existing
configurations.

Signed-off-by: Michael Nosthoff <buildroot@heine.tech>
---
 package/bluez5_utils/Config.in       | 36 ++++++++++++++++++++++++
 package/bluez5_utils/bluez5_utils.mk | 41 ++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/package/bluez5_utils/Config.in b/package/bluez5_utils/Config.in
index 2c8625a66d..9f20bf83ac 100644
--- a/package/bluez5_utils/Config.in
+++ b/package/bluez5_utils/Config.in
@@ -37,6 +37,12 @@ config BR2_PACKAGE_BLUEZ5_UTILS_CLIENT
 	help
 	  Build the command line client "bluetoothctl".
 
+config BR2_PACKAGE_BLUEZ5_UTILS_MONITOR
+	bool "build monitor utility"
+	default y
+	help
+	  Build monitor utility btmon.
+
 config BR2_PACKAGE_BLUEZ5_UTILS_DEPRECATED
 	bool "install deprecated tools"
 	depends on BR2_PACKAGE_BLUEZ5_UTILS_CLIENT
@@ -52,11 +58,35 @@ config BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL
 	  "Nokia OBEX PC Suite tool". So, only if OBEX support is
 	  enabled this option has an effect.
 
+config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_A2DP
+	bool "build a2dp plugin"
+	default y
+	help
+	  Build plugin for A2DP (audio) profiles.
+
+config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_AVRCP
+	bool "build avrcp plugin"
+	default y
+	help
+	  Build plugin for AVRCP (audio) profiles.
+
 config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_HEALTH
 	bool "build health plugin"
 	help
 	  Build plugin for health profiles.
 
+config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_HID
+	bool "build hid plugin"
+	default y
+	help
+	  Build plugin for HID (input) profiles.
+
+config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_HOG
+	bool "build hog plugin"
+	default y
+	help
+	  Build plugin for HoG (input) profiles.
+
 config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_MESH
 	bool "build mesh plugin"
 	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_12 # ell
@@ -76,6 +106,12 @@ config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_MIDI
 	help
 	  Build MIDI support via ALSA sequencer.
 
+config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_NETWORK
+	bool "build network plugin"
+	default y
+	help
+	  Build plugin for PANU, NAP, GN profiles.
+
 config BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_NFC
 	bool "build nfc plugin"
 	help
diff --git a/package/bluez5_utils/bluez5_utils.mk b/package/bluez5_utils/bluez5_utils.mk
index 3bd3f5490d..7b7e31500f 100644
--- a/package/bluez5_utils/bluez5_utils.mk
+++ b/package/bluez5_utils/bluez5_utils.mk
@@ -40,6 +40,12 @@ else
 BLUEZ5_UTILS_CONF_OPTS += --disable-client
 endif
 
+ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_MONITOR),y)
+BLUEZ5_UTILS_CONF_OPTS += --enable-monitor
+else
+BLUEZ5_UTILS_CONF_OPTS += --disable-monitor
+endif
+
 # experimental plugins
 ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL),y)
 BLUEZ5_UTILS_CONF_OPTS += --enable-experimental
@@ -47,6 +53,20 @@ else
 BLUEZ5_UTILS_CONF_OPTS += --disable-experimental
 endif
 
+# enable a2dp plugin
+ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_A2DP),y)
+BLUEZ5_UTILS_CONF_OPTS += --enable-a2dp
+else
+BLUEZ5_UTILS_CONF_OPTS += --disable-a2dp
+endif
+
+#enable avrcp plugin
+ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_AVRCP),y)
+BLUEZ5_UTILS_CONF_OPTS += --enable-avrcp
+else
+BLUEZ5_UTILS_CONF_OPTS += --disable-avrcp
+endif
+
 # enable health plugin
 ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_HEALTH),y)
 BLUEZ5_UTILS_CONF_OPTS += --enable-health
@@ -54,6 +74,20 @@ else
 BLUEZ5_UTILS_CONF_OPTS += --disable-health
 endif
 
+# enable hid plugin
+ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_HID),y)
+BLUEZ5_UTILS_CONF_OPTS += --enable-hid
+else
+BLUEZ5_UTILS_CONF_OPTS += --disable-hid
+endif
+
+# enable hog plugin
+ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_HOG),y)
+BLUEZ5_UTILS_CONF_OPTS += --enable-hog
+else
+BLUEZ5_UTILS_CONF_OPTS += --disable-hog
+endif
+
 # enable mesh profile
 ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_MESH),y)
 BLUEZ5_UTILS_CONF_OPTS += --enable-external-ell --enable-mesh
@@ -70,6 +104,13 @@ else
 BLUEZ5_UTILS_CONF_OPTS += --disable-midi
 endif
 
+# enable network plugin
+ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_NETWORK),y)
+BLUEZ5_UTILS_CONF_OPTS += --enable-network
+else
+BLUEZ5_UTILS_CONF_OPTS += --disable-network
+endif
+
 # enable nfc plugin
 ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_PLUGINS_NFC),y)
 BLUEZ5_UTILS_CONF_OPTS += --enable-nfc
-- 
2.32.0

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH] package/bluez5_utils: expose more disable options
  2021-07-26 20:12 [Buildroot] [PATCH] package/bluez5_utils: expose more disable options Michael Nosthoff via buildroot
@ 2021-07-26 21:07 ` Thomas Petazzoni
  2021-07-27 19:53   ` Arnout Vandecappelle
  2021-07-28 17:00   ` Yann E. MORIN
  0 siblings, 2 replies; 5+ messages in thread
From: Thomas Petazzoni @ 2021-07-26 21:07 UTC (permalink / raw)
  To: Michael Nosthoff; +Cc: Marcin Bis, Yann E. MORIN, buildroot

Hello,

(Yann, Arnout, Peter: question for you below.)

On Mon, 26 Jul 2021 22:12:42 +0200
Michael Nosthoff via buildroot <buildroot@busybox.net> wrote:

> BlueZ builds a lot of Classic BT profiles by default but allows
> to disable them. This is especially handy when only BLE is needed
> and enabled in the kernel.
> 
> Otherwise this yields warnings like this on bootup:
> 
>  profiles/network/bnep.c:bnep_init() kernel lacks bnep-protocol support
>  src/plugin.c:plugin_init() System does not support network plugin
> 
> Also it allows to disable btmon which should not be needed on
> production systems and is ~800KB in size.
> 
> Expose those options but default to 'y' to no break existing
> configurations.
> 
> Signed-off-by: Michael Nosthoff <buildroot@heine.tech>
> ---
>  package/bluez5_utils/Config.in       | 36 ++++++++++++++++++++++++
>  package/bluez5_utils/bluez5_utils.mk | 41 ++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+)

Applied to master, thanks.

Arnout, Peter, Yann: in order to preserve backward compatibility,
Michael has created those new options with a "default y". However,
while it keeps backward compatibility, it also means that all new users
will get a more bloated bluez_utils installation than is probably
necessary. Should we break our backward compatibility rule here and
drop the "default y" on those new options ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH] package/bluez5_utils: expose more disable options
  2021-07-26 21:07 ` Thomas Petazzoni
@ 2021-07-27 19:53   ` Arnout Vandecappelle
  2021-07-27 20:19     ` Michael Nosthoff via buildroot
  2021-07-28 17:00   ` Yann E. MORIN
  1 sibling, 1 reply; 5+ messages in thread
From: Arnout Vandecappelle @ 2021-07-27 19:53 UTC (permalink / raw)
  To: Thomas Petazzoni, Michael Nosthoff; +Cc: Marcin Bis, Yann E. MORIN, buildroot



On 26/07/2021 23:07, Thomas Petazzoni wrote:
> Hello,
> 
> (Yann, Arnout, Peter: question for you below.)
> 
> On Mon, 26 Jul 2021 22:12:42 +0200
> Michael Nosthoff via buildroot <buildroot@busybox.net> wrote:
> 
>> BlueZ builds a lot of Classic BT profiles by default but allows
>> to disable them. This is especially handy when only BLE is needed
>> and enabled in the kernel.
>>
>> Otherwise this yields warnings like this on bootup:
>>
>>  profiles/network/bnep.c:bnep_init() kernel lacks bnep-protocol support
>>  src/plugin.c:plugin_init() System does not support network plugin
>>
>> Also it allows to disable btmon which should not be needed on
>> production systems and is ~800KB in size.
>>
>> Expose those options but default to 'y' to no break existing
>> configurations.
>>
>> Signed-off-by: Michael Nosthoff <buildroot@heine.tech>
>> ---
>>  package/bluez5_utils/Config.in       | 36 ++++++++++++++++++++++++
>>  package/bluez5_utils/bluez5_utils.mk | 41 ++++++++++++++++++++++++++++
>>  2 files changed, 77 insertions(+)
> 
> Applied to master, thanks.
> 
> Arnout, Peter, Yann: in order to preserve backward compatibility,
> Michael has created those new options with a "default y". However,
> while it keeps backward compatibility, it also means that all new users
> will get a more bloated bluez_utils installation than is probably
> necessary. Should we break our backward compatibility rule here and
> drop the "default y" on those new options ?

 I know from experience that that's the kind of tricky changes that can make it
annoying to update Buildroot.

 Assuming you select bluez5_utils from menuconfig, having a default y is not
much of a problem in practice. However, bluez5_utils is also selected
automatically by a couple of packages [*] - in that case, the user doesn't see
the suboptions so isn't aware that they can disable them.

 So yes, there are good reasons to prefer default n. But then I think we should
think of a way to smoothen things for people who want to update buildroot. Here
are a few ideas:

- Just mention it in CHANGES.

- Extend the documentation in docs/manual/migrating.txt with this kind of
information. That would make that chapter quite a bit bigger over time, and
perhaps looking over the notes for each 3-month update is going to be quite a
bit of work.

- Write a tool that detects relevant changes. You'd run it in the newer
buildroot tree on the old .config. It would detect for example that the .config
has BR2_PACKAGE_BLUEZ5_UTILS set but BR2_PACKAGE_BLUEZ5_UTILS_MONITOR is not
mentioned at all in the .config, and write something about it. This tool could
also replace Config.in.legacy.

- Like above, but implemented directly in kconfig.

- Independent of this, a feature that I sometimes miss is a tool that tells me
which packages (and kernel and bootloader) are enabled but not selected by
another package (like an interactive defconfig, but showing everything that is
'yes' instead of everything that is default). Indeed, when you're developing,
you tend to enable a bunch of packages that you're later going to remove again.
Removing the package itself isn't much of a bother, but they often pull i
dependencies that you don't actually need. The way I solve this is that I edit
the defconfig instead of using menuconfig - not exactly user friendly. Anyway,
if such a tool existed, then it would be much more obvious that a bunch of
bluez5_utils plugins are enabled so the default y is less harmful.


 Given that all of these ideas except the first two are a lot of work, we'll
probably end up with the first two :-)


[*] BTW, did someone check if those packages that select bluez5_utils now should
be updated to select the appropriate plugin? Probably not - I imagine cwiid (for
the wiimote) for example needs the HID plugin, and the gstreamer plugin needs
A2DP and/or AVDP. Runtime dependencies, of course, so we'll never know until
someone actually complains about it.


 Regards,
 Arnout
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH] package/bluez5_utils: expose more disable options
  2021-07-27 19:53   ` Arnout Vandecappelle
@ 2021-07-27 20:19     ` Michael Nosthoff via buildroot
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Nosthoff via buildroot @ 2021-07-27 20:19 UTC (permalink / raw)
  To: Arnout Vandecappelle, Thomas Petazzoni
  Cc: buildroot, Yann E. MORIN, Marcin Bis

Hi,

On 27.07.21 21:53, Arnout Vandecappelle wrote:
> On 26/07/2021 23:07, Thomas Petazzoni wrote:
>> Arnout, Peter, Yann: in order to preserve backward compatibility,
>> Michael has created those new options with a "default y". However,
>> while it keeps backward compatibility, it also means that all new users
>> will get a more bloated bluez_utils installation than is probably
>> necessary. Should we break our backward compatibility rule here and
>> drop the "default y" on those new options ?

If we go for a compatibility break here I would then also submit a patch 
to make
"--disable-tools" configurable and default to no. Those tools are 
usually not
required when using d-bus and/or bluetoothctl and add up to 1-2MB.

I was hesitant on this one because the relation with 
--disable-deprecated, which
buildroot already exposes, is a bit weird but I think i figured it out.

>   I know from experience that that's the kind of tricky changes that can make it
> annoying to update Buildroot.
>
>   Assuming you select bluez5_utils from menuconfig, having a default y is not
> much of a problem in practice. However, bluez5_utils is also selected
> automatically by a couple of packages [*] - in that case, the user doesn't see
> the suboptions so isn't aware that they can disable them.
>
>   So yes, there are good reasons to prefer default n. But then I think we should
> think of a way to smoothen things for people who want to update buildroot. Here
> are a few ideas:
>
> - Just mention it in CHANGES.
 From a user perspective I think this should mention this, even if we go 
with the migrating.txt
or another approach. Because this is where I look on upgrading before 
trying to build.

> [*] BTW, did someone check if those packages that select bluez5_utils now should
> be updated to select the appropriate plugin? Probably not - I imagine cwiid (for
> the wiimote) for example needs the HID plugin, and the gstreamer plugin needs
> A2DP and/or AVDP. Runtime dependencies, of course, so we'll never know until
> someone actually complains about it.

I have some spare time in the next days. I'll check the dependencies 
where possible.

Regards,
Michael

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Buildroot] [PATCH] package/bluez5_utils: expose more disable options
  2021-07-26 21:07 ` Thomas Petazzoni
  2021-07-27 19:53   ` Arnout Vandecappelle
@ 2021-07-28 17:00   ` Yann E. MORIN
  1 sibling, 0 replies; 5+ messages in thread
From: Yann E. MORIN @ 2021-07-28 17:00 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: Michael Nosthoff, Marcin Bis, buildroot

Thomas, All,

On 2021-07-26 23:07 +0200, Thomas Petazzoni spake thusly:
> On Mon, 26 Jul 2021 22:12:42 +0200
> Michael Nosthoff via buildroot <buildroot@busybox.net> wrote:
> > BlueZ builds a lot of Classic BT profiles by default but allows
> > to disable them. This is especially handy when only BLE is needed
> > and enabled in the kernel.
> > 
> > Otherwise this yields warnings like this on bootup:
> > 
> >  profiles/network/bnep.c:bnep_init() kernel lacks bnep-protocol support
> >  src/plugin.c:plugin_init() System does not support network plugin
> > 
> > Also it allows to disable btmon which should not be needed on
> > production systems and is ~800KB in size.
> > 
> > Expose those options but default to 'y' to no break existing
> > configurations.
> > 
> > Signed-off-by: Michael Nosthoff <buildroot@heine.tech>
> > ---
> >  package/bluez5_utils/Config.in       | 36 ++++++++++++++++++++++++
> >  package/bluez5_utils/bluez5_utils.mk | 41 ++++++++++++++++++++++++++++
> >  2 files changed, 77 insertions(+)
> 
> Applied to master, thanks.
> 
> Arnout, Peter, Yann: in order to preserve backward compatibility,
> Michael has created those new options with a "default y". However,
> while it keeps backward compatibility, it also means that all new users
> will get a more bloated bluez_utils installation than is probably
> necessary. Should we break our backward compatibility rule here and
> drop the "default y" on those new options ?

My opinion is to just drop the 'default y', yes.

We always say that this backward compatibility is being nice to users.
But users who upgrade *must* pay attention to what they do, and I am of
the opinion that this should not be made too easy. Easy enough, yes; too
easy, no.

There are so many things that have to be checked anyway, that we can not
provide for. For example, if the licensing terms of a package changed,
we have no way to propagate that info to the user. Yes, we provide them
with the new terms, but we are not telling them they changed.

Also, the "core" settings, like optimisation, hardening, et al. did
change, but we don't tell users; they have to verify that by themselves.

And when a package brings in a new dependency, we don't tell them
either; they just get that new library that maybe they previously were
fine not having, while now they no longer have the choice...

And changes to the package options are most probably the easiest to
discover: a config that used to build no longer builds because of a
library that is now optionally built, or does not run as expected
because a program is now missing...

And in three years time, we will have a set of Config.in files crippled
because of the excuse that some user rarely upgrade and we just want to
support them... No, thanks. ;-)

That's the same for legacy: how much sensible is it for people to
upgrade in one big leap from a more than 5-year old version to the
latest one? That's is going to be so disruptive that legacy is the
least of the annoyance they will have to face... Upgrading should be
done in small leaps. Even upgrading from one LTS to the next is just a
daunting task... For example, I always upgrade to each intermediate
release and, at the very least, review the configuraton there, resorting
to actual builds and full-CI tests to double check the new version did
not break anything (it always do break some thing or some other)...

For legacy, we do have a central location, so it gets easy to remove
after a while, while for the 'default y', they are scattered everywhere
around the code that it will be impossible to track down...

And then, when we eventually decide to remove them, we won't be able to
do so anyway, because of the excude that existing defconfig files would
now break... :-/

Enough of my ranting, then: "say no to default y". ;-]

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-07-28 17:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26 20:12 [Buildroot] [PATCH] package/bluez5_utils: expose more disable options Michael Nosthoff via buildroot
2021-07-26 21:07 ` Thomas Petazzoni
2021-07-27 19:53   ` Arnout Vandecappelle
2021-07-27 20:19     ` Michael Nosthoff via buildroot
2021-07-28 17:00   ` Yann E. MORIN

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.