[Buildroot] [PATCH 1/1] bluez5_utils: Add config option to install "bdaddr"

[Buildroot] [PATCH 1/1] bluez5_utils: Add config option to install "bdaddr"

[Vishal Thanki]

"bdaddr" utility allows to change the Bluetooth device
address. This patch adds a config option to enable
installation of bdaddr utility.

Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
 package/bluez5_utils/Config.in       | 6 ++++++
 package/bluez5_utils/bluez5_utils.mk | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/package/bluez5_utils/Config.in b/package/bluez5_utils/Config.in
index a7c0ee4..00f8ec2 100644
--- a/package/bluez5_utils/Config.in
+++ b/package/bluez5_utils/Config.in
@@ -64,6 +64,12 @@ config BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL
 	help
 	  Build BlueZ 5.x experimental plugins (SAP, NFC, ...).
 
+config BR2_PACKAGE_BLUEZ5_TOOLS_BDADDR
+	bool "install bdaddr tool"
+	depends on BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL
+	help
+	  The "bdaddr" tool is used for changing the Bluetooth device address.
+
 config BR2_PACKAGE_BLUEZ5_PLUGINS_SIXAXIS
 	bool "build sixaxis plugin"
 	depends on BR2_PACKAGE_HAS_UDEV
diff --git a/package/bluez5_utils/bluez5_utils.mk b/package/bluez5_utils/bluez5_utils.mk
index 66c3eab..f9e73c2 100644
--- a/package/bluez5_utils/bluez5_utils.mk
+++ b/package/bluez5_utils/bluez5_utils.mk
@@ -38,6 +38,13 @@ else
 BLUEZ5_UTILS_CONF_OPTS += --disable-experimental
 endif
 
+ifeq ($(BR2_PACKAGE_BLUEZ5_TOOLS_BDADDR),y)
+define BLUEZ5_UTILS_INSTALL_BDADDR
+	$(INSTALL) -D -m 0755 $(@D)/tools/bdaddr $(TARGET_DIR)/usr/bin/bdaddr
+endef
+BLUEZ5_UTILS_POST_INSTALL_TARGET_HOOKS += BLUEZ5_UTILS_INSTALL_BDADDR
+endif
+
 # enable sixaxis plugin
 ifeq ($(BR2_PACKAGE_BLUEZ5_PLUGINS_SIXAXIS),y)
 BLUEZ5_UTILS_CONF_OPTS += --enable-sixaxis
-- 
2.4.11

[Buildroot] [PATCH 1/1] bluez5_utils: Add config option to install "bdaddr"

[Baruch Siach]

Hi Vishal Thanki,

On Tue, Feb 14, 2017 at 04:03:52PM +0100, Vishal Thanki wrote:
> "bdaddr" utility allows to change the Bluetooth device
> address. This patch adds a config option to enable
> installation of bdaddr utility.
> 
> Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
> ---
>  package/bluez5_utils/Config.in       | 6 ++++++
>  package/bluez5_utils/bluez5_utils.mk | 7 +++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/package/bluez5_utils/Config.in b/package/bluez5_utils/Config.in
> index a7c0ee4..00f8ec2 100644
> --- a/package/bluez5_utils/Config.in
> +++ b/package/bluez5_utils/Config.in
> @@ -64,6 +64,12 @@ config BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL
>  	help
>  	  Build BlueZ 5.x experimental plugins (SAP, NFC, ...).
>  
> +config BR2_PACKAGE_BLUEZ5_TOOLS_BDADDR
> +	bool "install bdaddr tool"
> +	depends on BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL
> +	help
> +	  The "bdaddr" tool is used for changing the Bluetooth device address.

bdaddr is less than 100KB. I don't think it's worth another config option. 
100KB is negligible when you already have glib and dbus. I'd suggested to 
either install bdaddr unconditionally, or have a single option for all tools/ 
executables that are not covered by another config option already.

> +
>  config BR2_PACKAGE_BLUEZ5_PLUGINS_SIXAXIS
>  	bool "build sixaxis plugin"
>  	depends on BR2_PACKAGE_HAS_UDEV
> diff --git a/package/bluez5_utils/bluez5_utils.mk b/package/bluez5_utils/bluez5_utils.mk
> index 66c3eab..f9e73c2 100644
> --- a/package/bluez5_utils/bluez5_utils.mk
> +++ b/package/bluez5_utils/bluez5_utils.mk
> @@ -38,6 +38,13 @@ else
>  BLUEZ5_UTILS_CONF_OPTS += --disable-experimental
>  endif
>  
> +ifeq ($(BR2_PACKAGE_BLUEZ5_TOOLS_BDADDR),y)
> +define BLUEZ5_UTILS_INSTALL_BDADDR
> +	$(INSTALL) -D -m 0755 $(@D)/tools/bdaddr $(TARGET_DIR)/usr/bin/bdaddr
> +endef
> +BLUEZ5_UTILS_POST_INSTALL_TARGET_HOOKS += BLUEZ5_UTILS_INSTALL_BDADDR
> +endif
> +
>  # enable sixaxis plugin
>  ifeq ($(BR2_PACKAGE_BLUEZ5_PLUGINS_SIXAXIS),y)
>  BLUEZ5_UTILS_CONF_OPTS += --enable-sixaxis

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] bluez5_utils: Add config option to install "bdaddr"

[Thomas Petazzoni]

Hello,

On Tue, 14 Feb 2017 18:08:44 +0200, Baruch Siach wrote:

> > +config BR2_PACKAGE_BLUEZ5_TOOLS_BDADDR
> > +	bool "install bdaddr tool"
> > +	depends on BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL
> > +	help
> > +	  The "bdaddr" tool is used for changing the Bluetooth device address.  
> 
> bdaddr is less than 100KB. I don't think it's worth another config option. 
> 100KB is negligible when you already have glib and dbus. I'd suggested to 
> either install bdaddr unconditionally, or have a single option for all tools/ 
> executables that are not covered by another config option already.

I would tend to agree, but we already have options like
BR2_PACKAGE_BLUEZ5_UTILS_GATTTOOL to install a single tool.

How many tools are provided by bluez5_utils? If it's just a very small
set of tools, it's OK to have one option for each. But if there are
many small tools, we definitely don't want to have one option for each.

In any case, we need to provide a better guideline to Vishal, because
right now, we are not explaining how the patch should be fixed. If we
introduce something like BR2_PACKAGE_BLUEZ5_UTILS_TOOLS, then what
should be done with BR2_PACKAGE_BLUEZ5_UTILS_CLIENT or
BR2_PACKAGE_BLUEZ5_UTILS_GATTTOOL for example ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] bluez5_utils: Add config option to install "bdaddr"

[Baruch Siach]

Hi Thomas,

On Tue, Feb 14, 2017 at 09:14:43PM +0100, Thomas Petazzoni wrote:
> On Tue, 14 Feb 2017 18:08:44 +0200, Baruch Siach wrote:
> > > +config BR2_PACKAGE_BLUEZ5_TOOLS_BDADDR
> > > +	bool "install bdaddr tool"
> > > +	depends on BR2_PACKAGE_BLUEZ5_UTILS_EXPERIMENTAL
> > > +	help
> > > +	  The "bdaddr" tool is used for changing the Bluetooth device address.  
> > 
> > bdaddr is less than 100KB. I don't think it's worth another config option. 
> > 100KB is negligible when you already have glib and dbus. I'd suggested to 
> > either install bdaddr unconditionally, or have a single option for all tools/ 
> > executables that are not covered by another config option already.
> 
> I would tend to agree, but we already have options like
> BR2_PACKAGE_BLUEZ5_UTILS_GATTTOOL to install a single tool.
> 
> How many tools are provided by bluez5_utils? If it's just a very small
> set of tools, it's OK to have one option for each. But if there are
> many small tools, we definitely don't want to have one option for each.

The bluez5_utils-5.43 tools/ subdirectory has 56 executables, totaling 5.7MB. 
Some are covered by covered by --enable-tools which we enable unconditionally, 
some other are installed by --enable-experimental, and yet others are only 
built by --enable-experimental. bdaddr falls in the last category.

> In any case, we need to provide a better guideline to Vishal, because
> right now, we are not explaining how the patch should be fixed. If we
> introduce something like BR2_PACKAGE_BLUEZ5_UTILS_TOOLS, then what
> should be done with BR2_PACKAGE_BLUEZ5_UTILS_CLIENT or
> BR2_PACKAGE_BLUEZ5_UTILS_GATTTOOL for example ?

All should go under BR2_PACKAGE_BLUEZ5_UTILS_TOOLS, as well as --enable-tools, 
IMO.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] bluez5_utils: Add config option to install "bdaddr"

[Thomas Petazzoni]

Hello,

On Wed, 15 Feb 2017 08:39:11 +0200, Baruch Siach wrote:

> > How many tools are provided by bluez5_utils? If it's just a very small
> > set of tools, it's OK to have one option for each. But if there are
> > many small tools, we definitely don't want to have one option for each.  
> 
> The bluez5_utils-5.43 tools/ subdirectory has 56 executables, totaling 5.7MB. 
> Some are covered by covered by --enable-tools which we enable unconditionally, 
> some other are installed by --enable-experimental, and yet others are only 
> built by --enable-experimental. bdaddr falls in the last category.
> 
> > In any case, we need to provide a better guideline to Vishal, because
> > right now, we are not explaining how the patch should be fixed. If we
> > introduce something like BR2_PACKAGE_BLUEZ5_UTILS_TOOLS, then what
> > should be done with BR2_PACKAGE_BLUEZ5_UTILS_CLIENT or
> > BR2_PACKAGE_BLUEZ5_UTILS_GATTTOOL for example ?  
> 
> All should go under BR2_PACKAGE_BLUEZ5_UTILS_TOOLS, as well as --enable-tools, 
> IMO.

Well, 5.7 MB and 56 executables is definitely not a small amount, so I
would tend to instead accept a more fine-grained selection of the tools.

However, if indeed --enable-tools is optional, we shouldn't enable it
unconditional, but have an option for that.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] bluez5_utils: Add config option to install "bdaddr"

[Vishal Thanki]

Hi Thomas, Baruch,

Thanks for your comments.

>> All should go under BR2_PACKAGE_BLUEZ5_UTILS_TOOLS, as well as --enable-tools,
>> IMO.
>
> Well, 5.7 MB and 56 executables is definitely not a small amount, so I
> would tend to instead accept a more fine-grained selection of the tools.
>

Does this mean that "bdaddr" should be enabled with a separate option
as done in this patch? OR this patch should also provide options for
all other tools which do not get installed by default?

> However, if indeed --enable-tools is optional, we shouldn't enable it
> unconditional, but have an option for that.
>

Should "--enable-tools" option in buildroot be handled in this patch
or in a separate patch?

Thanks,
Vishal