2013-06-21 14:17:31

by Lucas De Marchi

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

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



2013-06-25 18:50:00

by Lucas De Marchi

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

On Fri, Jun 21, 2013 at 7:47 PM, Marcel Holtmann <[email protected]> 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

2013-06-24 08:14:22

by Johan Hedberg

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

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

2013-06-21 22:47:37

by Marcel Holtmann

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

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


2013-06-21 22:37:15

by Lucas De Marchi

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

On Fri, Jun 21, 2013 at 7:02 PM, Marcel Holtmann <[email protected]> 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

2013-06-21 22:02:23

by Marcel Holtmann

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

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


2013-06-21 14:17:35

by Lucas De Marchi

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

From: Lucas De Marchi <[email protected]>

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


2013-06-21 14:17:34

by Lucas De Marchi

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

From: Lucas De Marchi <[email protected]>

---
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


2013-06-21 14:17:33

by Lucas De Marchi

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

From: Lucas De Marchi <[email protected]>

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


2013-06-21 14:17:32

by Lucas De Marchi

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

From: Lucas De Marchi <[email protected]>

---
.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