[PATCH BlueZ v3 1/5] build-sys: Don't build gdbus several times

[PATCH BlueZ v3 1/5] build-sys: Don't build gdbus several times

[Lucas De Marchi]

We are currently compiling gdbus 3 times: one for tests, one for
bluetoothd and another for obexd.

$ ls gdbus/*watch.o
gdbus/bluetoothd-watch.o  gdbus/obexd-watch.o  gdbus/watch.o

Instead of prepending $(gdbus_sources) to several _SOURCES variable, use
a convenience library.
---
 Makefile.am    | 16 +++++++++++-----
 Makefile.obexd |  4 ++--
 Makefile.tools | 25 ++++++++++++-------------
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 80edafd..82efba2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -112,6 +112,11 @@ builtin_nodist =
 
 include Makefile.plugins
 
+noinst_LTLIBRARIES += gdbus/libgdbus.la
+gdbus_libgdbus_la_SOURCES = $(gdbus_sources)
+gdbus_libgdbus_la_LDFLAGS = $(AM_LDFLAGS) @DBUS_LIBS@ @GLIB_LIBS@
+gdbus_libgdbus_la_CFLAGS = $(AM_CFLAGS) @DBUS_CFLAGS@ @GLIB_CFLAGS@
+
 if MAINTAINER_MODE
 plugin_LTLIBRARIES += plugins/external-dummy.la
 plugins_external_dummy_la_SOURCES = plugins/external-dummy.c
@@ -122,7 +127,7 @@ endif
 
 libexec_PROGRAMS += src/bluetoothd
 
-src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
+src_bluetoothd_SOURCES = $(builtin_sources) \
 			$(attrib_sources) $(btio_sources) \
 			src/bluetooth.ver \
 			src/main.c src/log.h src/log.c \
@@ -148,8 +153,9 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/eir.h src/eir.c \
 			src/shared/util.h src/shared/util.c \
 			src/shared/mgmt.h src/shared/mgmt.c
-src_bluetoothd_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@ @DBUS_LIBS@ \
-								-ldl -lrt
+src_bluetoothd_LDADD = lib/libbluetooth-private.la gdbus/libgdbus.la \
+			@GLIB_LIBS@ @DBUS_LIBS@ \
+			-ldl -lrt
 src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
 				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
@@ -245,8 +251,8 @@ unit_test_sdp_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
 
 unit_tests += unit/test-gdbus-client
 
-unit_test_gdbus_client_SOURCES = $(gdbus_sources) unit/test-gdbus-client.c
-unit_test_gdbus_client_LDADD = @GLIB_LIBS@ @DBUS_LIBS@
+unit_test_gdbus_client_SOURCES = unit/test-gdbus-client.c
+unit_test_gdbus_client_LDADD = gdbus/libgdbus.la @GLIB_LIBS@ @DBUS_LIBS@
 
 unit_tests += unit/test-gobex-header unit/test-gobex-packet unit/test-gobex \
 			unit/test-gobex-transfer unit/test-gobex-apparam
diff --git a/Makefile.obexd b/Makefile.obexd
index 5824e0a..37aa2d9 100644
--- a/Makefile.obexd
+++ b/Makefile.obexd
@@ -54,7 +54,7 @@ obexd_builtin_sources += obexd/client/mns.c obexd/src/map_ap.h \
 
 libexec_PROGRAMS += obexd/src/obexd
 
-obexd_src_obexd_SOURCES = $(gdbus_sources) $(btio_sources) $(gobex_sources) \
+obexd_src_obexd_SOURCES = $(btio_sources) $(gobex_sources) \
 			$(obexd_builtin_sources) \
 			obexd/src/main.c obexd/src/obexd.h \
 			obexd/src/plugin.h obexd/src/plugin.c \
@@ -78,7 +78,7 @@ obexd_src_obexd_SOURCES = $(gdbus_sources) $(btio_sources) $(gobex_sources) \
 			obexd/client/dbus.h obexd/client/dbus.c \
 			obexd/client/driver.h obexd/client/driver.c \
 			obexd/src/map_ap.h
-obexd_src_obexd_LDADD = lib/libbluetooth-private.la \
+obexd_src_obexd_LDADD = gdbus/libgdbus.la lib/libbluetooth-private.la \
 				@ICAL_LIBS@ @DBUS_LIBS@ @GLIB_LIBS@ -ldl
 
 obexd_src_obexd_LDFLAGS = -Wl,--export-dynamic
diff --git a/Makefile.tools b/Makefile.tools
index 8f245e0..cc0d8ee 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -2,11 +2,12 @@
 if CLIENT
 bin_PROGRAMS += client/bluetoothctl
 
-client_bluetoothctl_SOURCES = $(gdbus_sources) client/main.c \
+client_bluetoothctl_SOURCES = client/main.c \
 					client/display.h client/display.c \
 					client/agent.h client/agent.c \
 					monitor/uuid.h monitor/uuid.c
-client_bluetoothctl_LDADD = @GLIB_LIBS@ @DBUS_LIBS@ -lreadline
+client_bluetoothctl_LDADD = gdbus/libgdbus.la @GLIB_LIBS@ @DBUS_LIBS@ \
+				-lreadline
 endif
 
 if MONITOR
@@ -57,14 +58,12 @@ tools_l2cap_tester_SOURCES = tools/l2cap-tester.c monitor/bt.h \
 				src/shared/tester.h src/shared/tester.c
 tools_l2cap_tester_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
 
-tools_gap_tester_SOURCES = $(gdbus_sources) \
-				tools/gap-tester.c monitor/bt.h \
+tools_gap_tester_SOURCES = tools/gap-tester.c monitor/bt.h \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				src/shared/hciemu.h src/shared/hciemu.c \
 				src/shared/tester.h src/shared/tester.c
-
-tools_gap_tester_LDADD = @GLIB_LIBS@ @DBUS_LIBS@
+tools_gap_tester_LDADD = gdbus/libgdbus.la @GLIB_LIBS@ @DBUS_LIBS@
 endif
 
 if TOOLS
@@ -188,8 +187,8 @@ tools_btmgmt_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
 tools_btiotest_SOURCES = tools/btiotest.c btio/btio.h btio/btio.c
 tools_btiotest_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
 
-tools_mpris_player_SOURCES = $(gdbus_sources) tools/mpris-player.c
-tools_mpris_player_LDADD = @GLIB_LIBS@ @DBUS_LIBS@
+tools_mpris_player_SOURCES = tools/mpris-player.c
+tools_mpris_player_LDADD = gdbus/libgdbus.la @GLIB_LIBS@ @DBUS_LIBS@
 
 tools_bluetooth_player_SOURCES = $(gdbus_sources) tools/bluetooth-player.c \
 				client/display.h client/display.c
@@ -222,8 +221,8 @@ endif
 if EXPERIMENTAL
 noinst_PROGRAMS += profiles/iap/iapd
 
-profiles_iap_iapd_SOURCES = $(gdbus_sources) profiles/iap/main.c
-profiles_iap_iapd_LDADD = @GLIB_LIBS@ @DBUS_LIBS@
+profiles_iap_iapd_SOURCES = profiles/iap/main.c
+profiles_iap_iapd_LDADD = gdbus/libgdbus.la @GLIB_LIBS@ @DBUS_LIBS@
 endif
 
 if CUPS
@@ -231,14 +230,14 @@ cupsdir = $(libdir)/cups/backend
 
 cups_PROGRAMS = profiles/cups/bluetooth
 
-profiles_cups_bluetooth_SOURCES = $(gdbus_sources) profiles/cups/main.c \
+profiles_cups_bluetooth_SOURCES = profiles/cups/main.c \
 					profiles/cups/cups.h \
 					profiles/cups/sdp.c \
 					profiles/cups/spp.c \
 					profiles/cups/hcrp.c
 
-profiles_cups_bluetooth_LDADD = @GLIB_LIBS@ @DBUS_LIBS@ \
-				lib/libbluetooth-private.la
+profiles_cups_bluetooth_LDADD = gdbus/libgdbus.la lib/libbluetooth-private.la \
+				@GLIB_LIBS@ @DBUS_LIBS@
 endif
 
 test_scripts += test/sap_client.py test/bluezutils.py \
-- 
1.8.3.1

[PATCH BlueZ v3 2/5] gitignore: Add missing tools

[Lucas De Marchi]

From: Lucas De Marchi <lucas.demarchi@intel.com>

---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index c6e0ae2..550b350 100644
--- a/.gitignore
+++ b/.gitignore
@@ -59,6 +59,8 @@ tools/avtest
 tools/bdaddr
 tools/btiotest
 tools/mpris-player
+tools/bluetooth-player
+tools/l2cap-tester
 test/sap_client.pyc
 test/bluezutils.pyc
 unit/test-eir
-- 
1.8.3.1

[PATCH BlueZ v3 3/5] adapter: Remove never used parameter

[Lucas De Marchi]

From: Lucas De Marchi <lucas.demarchi@intel.com>

adapter_remove_device() is always called with remove_storage set to
TRUE, so remove it.
---
 src/adapter.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 9e817f2..17f5508 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1019,8 +1019,7 @@ static void service_auth_cancel(struct service_auth *auth)
 }
 
 static void adapter_remove_device(struct btd_adapter *adapter,
-						struct btd_device *dev,
-						gboolean remove_storage)
+						struct btd_device *dev)
 {
 	GList *l;
 
@@ -1052,7 +1051,7 @@ static void adapter_remove_device(struct btd_adapter *adapter,
 		service_auth_cancel(auth);
 	}
 
-	device_remove(dev, remove_storage);
+	device_remove(dev, TRUE);
 }
 
 struct btd_device *adapter_get_device(struct btd_adapter *adapter,
@@ -1514,7 +1513,7 @@ static gboolean remove_temp_devices(gpointer user_data)
 		next = g_slist_next(l);
 
 		if (device_is_temporary(dev))
-			adapter_remove_device(adapter, dev, TRUE);
+			adapter_remove_device(adapter, dev);
 	}
 
 	return FALSE;
@@ -2141,7 +2140,7 @@ static DBusMessage *remove_device(DBusConnection *conn,
 	device_set_temporary(device, TRUE);
 
 	if (!device_is_connected(device)) {
-		adapter_remove_device(adapter, device, TRUE);
+		adapter_remove_device(adapter, device);
 		return dbus_message_new_method_return(msg);
 	}
 
@@ -4252,7 +4251,7 @@ static void adapter_remove_connection(struct btd_adapter *adapter,
 		const char *path = device_get_path(device);
 
 		DBG("Removing temporary device %s", path);
-		adapter_remove_device(adapter, device, TRUE);
+		adapter_remove_device(adapter, device);
 	}
 }
 
@@ -5808,7 +5807,7 @@ static void connect_failed_callback(uint16_t index, uint16_t length,
 	 * when it is temporary. */
 	if (device && !device_is_bonding(device, NULL)
 						&& device_is_temporary(device))
-		adapter_remove_device(adapter, device, TRUE);
+		adapter_remove_device(adapter, device);
 }
 
 static void unpaired_callback(uint16_t index, uint16_t length,
@@ -5839,7 +5838,7 @@ static void unpaired_callback(uint16_t index, uint16_t length,
 	if (device_is_connected(device))
 		device_request_disconnect(device, NULL);
 	else
-		adapter_remove_device(adapter, device, TRUE);
+		adapter_remove_device(adapter, device);
 }
 
 static void read_info_complete(uint8_t status, uint16_t length,
-- 
1.8.3.1

[PATCH BlueZ v3 4/5] client: Add description for default-agent command

[Lucas De Marchi]

From: Lucas De Marchi <lucas.demarchi@intel.com>

---
 client/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/client/main.c b/client/main.c
index b0a66d8..ff2afc6 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1057,7 +1057,8 @@ static const struct {
 	{ "agent",        "<on/off/capability>", cmd_agent,
 				"Enable/disable agent with given capability",
 							capability_generator},
-	{ "default-agent",NULL,       cmd_default_agent },
+	{ "default-agent",NULL,       cmd_default_agent,
+				"Set agent as the default one" },
 	{ "scan",         "<on/off>", cmd_scan, "Scan for devices" },
 	{ "info",         "<dev>",    cmd_info, "Device information",
 							dev_generator },
-- 
1.8.3.1

[PATCH BlueZ v3 5/5] client: prettify help table

[Lucas De Marchi]

From: Lucas De Marchi <lucas.demarchi@intel.com>

cmd + options have a maximum length of 25. Align the description
after this value. In order not to reach 80 chars so easily change the
first \t to 2 spaces, like is done in udev, kmod, systemd, etc.
---
 client/main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/client/main.c b/client/main.c
index ff2afc6..0dd1510 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1184,9 +1184,10 @@ static void rl_handler(char *input)
 
 	for (i = 0; cmd_table[i].cmd; i++) {
 		if (cmd_table[i].desc)
-			printf("\t%s %s\t%s\n", cmd_table[i].cmd,
-						cmd_table[i].arg ? : "    ",
-						cmd_table[i].desc);
+			printf("  %s %-*s %s\n", cmd_table[i].cmd,
+					(int)(25 - strlen(cmd_table[i].cmd)),
+					cmd_table[i].arg ? : "",
+					cmd_table[i].desc ? : "");
 	}
 
 done:
-- 
1.8.3.1

Re: [PATCH BlueZ v3 1/5] build-sys: Don't build gdbus several times

[Marcel Holtmann]

Hi Lucas,

> We are currently compiling gdbus 3 times: one for tests, one for
> bluetoothd and another for obexd.

we recompile because of the different CFLAGS for these binaries.

> $ ls gdbus/*watch.o
> gdbus/bluetoothd-watch.o  gdbus/obexd-watch.o  gdbus/watch.o
> 
> Instead of prepending $(gdbus_sources) to several _SOURCES variable, use
> a convenience library.

When we switched to non-recursive build, we stopped building interim libraries. What is the benefit of going back to building them. I am personally fine with building gdbus several times.

Regards

Marcel

Re: [PATCH BlueZ v3 1/5] build-sys: Don't build gdbus several times

[Lucas De Marchi]

On Fri, Jun 21, 2013 at 7:02 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Lucas,
>
>> We are currently compiling gdbus 3 times: one for tests, one for
>> bluetoothd and another for obexd.
>
> we recompile because of the different CFLAGS for these binaries.

But they shouldn=B4t be affecting gdbus.

>
>> $ ls gdbus/*watch.o
>> gdbus/bluetoothd-watch.o  gdbus/obexd-watch.o  gdbus/watch.o
>>
>> Instead of prepending $(gdbus_sources) to several _SOURCES variable, use
>> a convenience library.
>
> When we switched to non-recursive build, we stopped building interim libr=
aries. What is the benefit of going back to building them. I am personally =
fine with building gdbus several times.

It=B4s more that there=B4s no advantage doing otherwise. The small benefit
is building less files and letting it simpler.


Lucas De Marchi

Re: [PATCH BlueZ v3 1/5] build-sys: Don't build gdbus several times

[Marcel Holtmann]

Hi Lucas,

>>> We are currently compiling gdbus 3 times: one for tests, one for
>>> bluetoothd and another for obexd.
>> 
>> we recompile because of the different CFLAGS for these binaries.
> 
> But they shouldn´t be affecting gdbus.
> 
>> 
>>> $ ls gdbus/*watch.o
>>> gdbus/bluetoothd-watch.o  gdbus/obexd-watch.o  gdbus/watch.o
>>> 
>>> Instead of prepending $(gdbus_sources) to several _SOURCES variable, use
>>> a convenience library.
>> 
>> When we switched to non-recursive build, we stopped building interim libraries. What is the benefit of going back to building them. I am personally fine with building gdbus several times.
> 
> It´s more that there´s no advantage doing otherwise. The small benefit
> is building less files and letting it simpler.

then at least get into a bit of a standard habit. We are building libbluetooth into a public and private version.

Currently we name it libbluetooth-private, but maybe following the systemd "standard", we should start calling that libbluetooth-internal. And then libgdbus-internal.

You also do realize that I will make you do this for all other projects using gdbus.

Regards

Marcel

Re: [PATCH BlueZ v3 5/5] client: prettify help table

[Johan Hedberg]

Hi Lucas,

On Fri, Jun 21, 2013, Lucas De Marchi wrote:
> cmd + options have a maximum length of 25. Align the description
> after this value. In order not to reach 80 chars so easily change the
> first \t to 2 spaces, like is done in udev, kmod, systemd, etc.
> ---
>  client/main.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Patches 2-5 have been applied. Thanks.

Johan

Re: [PATCH BlueZ v3 1/5] build-sys: Don't build gdbus several times

[Lucas De Marchi]

On Fri, Jun 21, 2013 at 7:47 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Lucas,
>
>>>> We are currently compiling gdbus 3 times: one for tests, one for
>>>> bluetoothd and another for obexd.
>>>
>>> we recompile because of the different CFLAGS for these binaries.
>>
>> But they shouldn=B4t be affecting gdbus.
>>
>>>
>>>> $ ls gdbus/*watch.o
>>>> gdbus/bluetoothd-watch.o  gdbus/obexd-watch.o  gdbus/watch.o
>>>>
>>>> Instead of prepending $(gdbus_sources) to several _SOURCES variable, u=
se
>>>> a convenience library.
>>>
>>> When we switched to non-recursive build, we stopped building interim li=
braries. What is the benefit of going back to building them. I am personall=
y fine with building gdbus several times.
>>
>> It=B4s more that there=B4s no advantage doing otherwise. The small benef=
it
>> is building less files and letting it simpler.
>
> then at least get into a bit of a standard habit. We are building libblue=
tooth into a public and private version.
>
> Currently we name it libbluetooth-private, but maybe following the system=
d "standard", we should start calling that libbluetooth-internal. And then =
libgdbus-internal.

ok

>
> You also do realize that I will make you do this for all other projects u=
sing gdbus.

hahaha... ok

Lucas De Marchi