[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Arn R]

These options can now be disabled individually.
Previously, these options were simply enabled if the package they depended on was also enabled.
By default they will still be enabled if the packages they depend on are enabled.

Signed-off-by: Arn R <arnerro@gmail.com>
---
 package/gpsd/Config.in | 20 ++++++++++++++++++++
 package/gpsd/gpsd.mk   |  6 +++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/package/gpsd/Config.in b/package/gpsd/Config.in
index 337f8e7..c38d52b 100644
--- a/package/gpsd/Config.in
+++ b/package/gpsd/Config.in
@@ -90,6 +90,26 @@ config BR2_PACKAGE_GPSD_CONTROLSEND
 config BR2_PACKAGE_GPSD_SQUELCH
 	bool "squelch gpsd_report and gpsd_hexdump to save cpu"
 
+config BR2_PACKAGE_GPSD_BLUETOOTH_DEV
+	bool "allow GPS devices via Bluetooth"
+	default y
+	depends on BR2_PACKAGE_BLUEZ_UTILS
+	help
+	  Support Bluetooth devices with BlueZ.
+
+config BR2_PACKAGE_GPSD_USB_DEV
+	bool "allow GPS devices via USB"
+	default y
+	depends on BR2_PACKAGE_LIBUSB
+	help
+	  Support USB devices with libusb.
+
+config BR2_PACKAGE_GPSD_QTBINDING
+	bool "enable the Qt bindings"
+	default y
+	depends on BR2_PACKAGE_QT_NETWORK
+	help
+	  Builds the library libQgpsmm to provide Qt bindings.
 endmenu
 
 menu "Protocols"
diff --git a/package/gpsd/gpsd.mk b/package/gpsd/gpsd.mk
index 384726d..be4046f 100644
--- a/package/gpsd/gpsd.mk
+++ b/package/gpsd/gpsd.mk
@@ -44,7 +44,7 @@ else
 endif
 
 # Enable or disable Qt binding
-ifeq ($(BR2_PACKAGE_QT_NETWORK),y)
+ifeq ($(BR2_PACKAGE_GPSD_QTBINDING),y)
 	GPSD_SCONS_ENV += QMAKE="$(QT_QMAKE)"
 	GPSD_DEPENDENCIES += qt host-pkgconf
 else
@@ -52,14 +52,14 @@ else
 endif
 
 # If libusb is available build it before so the package can use it
-ifeq ($(BR2_PACKAGE_LIBUSB),y)
+ifeq ($(BR2_PACKAGE_GPSD_USB_DEV),y)
 	GPSD_DEPENDENCIES += libusb
 else
 	GPSD_SCONS_OPTS += usb=no
 endif
 
 # If bluetooth is available build it before so the package can use it
-ifeq ($(BR2_PACKAGE_BLUEZ_UTILS),y)
+ifeq ($(BR2_PACKAGE_GPSD_BLUETOOTH_DEV),y)
 	GPSD_DEPENDENCIES += bluez_utils
 else
 	GPSD_SCONS_OPTS += bluez=no
-- 
1.8.1.2

[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Thomas Petazzoni]

Dear Arn R,

On Fri, 24 May 2013 00:43:01 +1000, Arn R wrote:
> These options can now be disabled individually.
> Previously, these options were simply enabled if the package they
> depended on was also enabled. By default they will still be enabled
> if the packages they depend on are enabled.

Well, we have a fuzzy boundary on how to handle this, we've discussed
that not long ago with Arnout. Which problem are you seeing with the
current situation (i.e, without your patch) ?

When possible and when it makes we generally try to auto-detect when we
can enable a particular feature, as you've seen in gpsd.mk.

Anyway, some comments below:

> +config BR2_PACKAGE_GPSD_BLUETOOTH_DEV
> +	bool "allow GPS devices via Bluetooth"
> +	default y

This should not be enabled by default.

> +	depends on BR2_PACKAGE_BLUEZ_UTILS

This should be a select.

And ditto for the other options.

I see what you wanted to achieve: something that's automatically
enabled when for example bluez-utils is enabled, so that the existing
behavior is preserved. But for package sub-options, they are generally
disabled by default and they select whichever dependency they need.

Let's wait the opinion of others to see how to handle this.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Arn R]

Hi Thomas,

On Fri, May 24, 2013 at 12:56 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Well, we have a fuzzy boundary on how to handle this, we've discussed
> that not long ago with Arnout.

Could you point me to the thread where you discussed it?

> Which problem are you seeing with the
> current situation (i.e, without your patch) ?

There is no serious problem, this is more a matter of micro-management :)
For example, I don't use the Qt binding (even though Qt is present in
my build) so I simply do not want it present in the filesystem. My gps
module is connected to a plain UART port (but I use BlueZ for another
bluetooth device), so I have no need for the Bluetooth or USB gps
support. Since GPSd provided explicit options to enable/disable these
very features I thought I may as well provide access to those options.


> When possible and when it makes we generally try to auto-detect when we
> can enable a particular feature, as you've seen in gpsd.mk.
>
> Anyway, some comments below:
>
> > +config BR2_PACKAGE_GPSD_BLUETOOTH_DEV
> > +     bool "allow GPS devices via Bluetooth"
> > +     default y
>
> This should not be enabled by default.
>
> > +     depends on BR2_PACKAGE_BLUEZ_UTILS
>
> This should be a select.
>
> And ditto for the other options.
>
> I see what you wanted to achieve: something that's automatically
> enabled when for example bluez-utils is enabled, so that the existing
> behavior is preserved. But for package sub-options, they are generally
> disabled by default and they select whichever dependency they need.
>
> Let's wait the opinion of others to see how to handle this.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Thomas Petazzoni]

Dear Arn R,

On Fri, 24 May 2013 01:30:14 +1000, Arn R wrote:
> Hi Thomas,
> 
> On Fri, May 24, 2013 at 12:56 AM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > Well, we have a fuzzy boundary on how to handle this, we've
> > discussed that not long ago with Arnout.
> 
> Could you point me to the thread where you discussed it?

See http://lists.busybox.net/pipermail/buildroot/2013-May/072318.html
from Arnout, and my reply at
http://lists.busybox.net/pipermail/buildroot/2013-May/072335.html.

> > Which problem are you seeing with the
> > current situation (i.e, without your patch) ?
> 
> There is no serious problem, this is more a matter of
> micro-management :) For example, I don't use the Qt binding (even
> though Qt is present in my build) so I simply do not want it present
> in the filesystem. My gps module is connected to a plain UART port
> (but I use BlueZ for another bluetooth device), so I have no need for
> the Bluetooth or USB gps support. Since GPSd provided explicit
> options to enable/disable these very features I thought I may as well
> provide access to those options.

I see, thanks for your feedback on this use case.

We have two options here:

 1) Either declare that adding such options would add too many options
    to all the packages, and that such special modifications should be
    left to local modifications.

 2) Or add many more configuration options to packages.

Maybe (2) is what makes sense here. The cost of new options in terms of
maintenance is not so high, so maybe that's what we should do. I don't
know.

Peter, Arnout?

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Arnout Vandecappelle]

On 23/05/13 17:46, Thomas Petazzoni wrote:
> Dear Arn R,
>
> On Fri, 24 May 2013 01:30:14 +1000, Arn R wrote:
>> Hi Thomas,
>>
>> On Fri, May 24, 2013 at 12:56 AM, Thomas Petazzoni
>> <thomas.petazzoni@free-electrons.com> wrote:
>>> Well, we have a fuzzy boundary on how to handle this, we've
>>> discussed that not long ago with Arnout.
[snip]
>>> Which problem are you seeing with the
>>> current situation (i.e, without your patch) ?
>>
>> There is no serious problem, this is more a matter of
>> micro-management :) For example, I don't use the Qt binding (even
>> though Qt is present in my build) so I simply do not want it present
>> in the filesystem. My gps module is connected to a plain UART port
>> (but I use BlueZ for another bluetooth device), so I have no need for
>> the Bluetooth or USB gps support. Since GPSd provided explicit
>> options to enable/disable these very features I thought I may as well
>> provide access to those options.

  If enabling the option adds a significant amount of overhead (typically 
rootfs size overhead but it could also be performance overhead), then it 
is definitely worthwhile to make it an option. So for the qt bindings 
it's probably not worthwhile (qt itself is much larger). But for the 
Bluetooth or USB support, it could make a difference.

>
> I see, thanks for your feedback on this use case.
>
> We have two options here:
>
>   1) Either declare that adding such options would add too many options
>      to all the packages, and that such special modifications should be
>      left to local modifications.
>
>   2) Or add many more configuration options to packages.
>
> Maybe (2) is what makes sense here. The cost of new options in terms of
> maintenance is not so high,

  You forgot about updating the 'depends on' statements when a suboption 
wants to select e.g. glib...

> so maybe that's what we should do. I don't know.

  My dream is that the whole Config.in could be generated from the .mk 
file... but I'm not sure if that is really realistic. OpenWRT does it but 
it doesn't look that great either.

  So I think we'll stick to

    3) Decide on a case-by-case basis whether to go for suboptions or 
automatic enable/disable.


  Regards,
  Arnout

> Peter, Arnout?
>
> Best regards,
>
> Thomas
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Thomas Petazzoni]

Dear Arnout Vandecappelle,

On Thu, 23 May 2013 21:03:22 +0200, Arnout Vandecappelle wrote:

>   If enabling the option adds a significant amount of overhead (typically 
> rootfs size overhead but it could also be performance overhead), then it 
> is definitely worthwhile to make it an option. So for the qt bindings 
> it's probably not worthwhile (qt itself is much larger). But for the 
> Bluetooth or USB support, it could make a difference.

Regarding the Qt binding, if it installs additional files for example,
I wouldn't mind having a separate sub-option to disable it.

> > Maybe (2) is what makes sense here. The cost of new options in terms of
> > maintenance is not so high,
> 
>   You forgot about updating the 'depends on' statements when a suboption 
> wants to select e.g. glib...

Hum, not sure to understand what you meant here. Did you mean
"propagate the 'depends on'" from the selected package into the
selecting package, for dependencies on toolchain features and things
like that?

> > so maybe that's what we should do. I don't know.
> 
>   My dream is that the whole Config.in could be generated from the .mk 
> file... but I'm not sure if that is really realistic. OpenWRT does it but 
> it doesn't look that great either.
> 
>   So I think we'll stick to
> 
>     3) Decide on a case-by-case basis whether to go for suboptions or 
> automatic enable/disable.

I agree, as you don't see much other reasonable solutions.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 1/1] gpsd: Add config options for USB, Bluetooth, Qt.

[Arnout Vandecappelle]

On 23/05/13 21:12, Thomas Petazzoni wrote:
>>> > >Maybe (2) is what makes sense here. The cost of new options in terms of
>>> > >maintenance is not so high,
>> >
>> >   You forgot about updating the 'depends on' statements when a suboption
>> >wants to select e.g. glib...
> Hum, not sure to understand what you meant here. Did you mean
> "propagate the 'depends on'" from the selected package into the
> selecting package, for dependencies on toolchain features and things
> like that?

  Yes indeed. And the maintenance effort is to keep that up-to-date when 
heavily used libraries grow new dependencies.

  Regards,
  Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F