2013-06-18 20:13:43

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 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 9d570fb..19782ce 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-19 15:02:59

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/5] client: Display command even with no description

On Wed, Jun 19, 2013 at 11:54 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Lucas,
>
>>>>>> Having no description doesn't mean it's not an available command. So add
>>>>>> command to the output of "help" command, even if it has no description.
>>>>>> ---
>>>>>> client/main.c | 5 ++---
>>>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/client/main.c b/client/main.c
>>>>>> index b0a66d8..54af807 100644
>>>>>> --- a/client/main.c
>>>>>> +++ b/client/main.c
>>>>>> @@ -1182,10 +1182,9 @@ static void rl_handler(char *input)
>>>>>> printf("Available commands:\n");
>>>>>>
>>>>>> for (i = 0; cmd_table[i].cmd; i++) {
>>>>>> - if (cmd_table[i].desc)
>>>>>> - printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>>>>> + printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>>>>> cmd_table[i].arg ? : " ",
>>>>>> - cmd_table[i].desc);
>>>>>> + cmd_table[i].desc ? : "");
>>>>>
>>>>>
>>>>> if I remember this correctly, then I was doing it to be able to hide
>>>>> commands from the help output. Mainly commands that are on purpose
>>>>> duplicates.
>>>>
>>>> So the right fix is to add the missing descriptions then? Lucas, can you
>>>> provide an updated patch that does this?
>>>
>>> actually I wanted to hide these commands on purpose.
>>
>> So you can't pair a device starting from remote using bluetoothctl on
>> purpose? Or is adding a description to at least "default-agent" ok?
>
> is this for the "default-agent" command? I forgot to add a description for that one?

Yes. I went with "show even if not description is available" because
it doesn't do any harm. I can add a description to default-agent
though

Lucas De Marchi

2013-06-19 14:54:35

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/5] client: Display command even with no description

Hi Lucas,

>>>>> Having no description doesn't mean it's not an available command. So add
>>>>> command to the output of "help" command, even if it has no description.
>>>>> ---
>>>>> client/main.c | 5 ++---
>>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/client/main.c b/client/main.c
>>>>> index b0a66d8..54af807 100644
>>>>> --- a/client/main.c
>>>>> +++ b/client/main.c
>>>>> @@ -1182,10 +1182,9 @@ static void rl_handler(char *input)
>>>>> printf("Available commands:\n");
>>>>>
>>>>> for (i = 0; cmd_table[i].cmd; i++) {
>>>>> - if (cmd_table[i].desc)
>>>>> - printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>>>> + printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>>>> cmd_table[i].arg ? : " ",
>>>>> - cmd_table[i].desc);
>>>>> + cmd_table[i].desc ? : "");
>>>>
>>>>
>>>> if I remember this correctly, then I was doing it to be able to hide
>>>> commands from the help output. Mainly commands that are on purpose
>>>> duplicates.
>>>
>>> So the right fix is to add the missing descriptions then? Lucas, can you
>>> provide an updated patch that does this?
>>
>> actually I wanted to hide these commands on purpose.
>
> So you can't pair a device starting from remote using bluetoothctl on
> purpose? Or is adding a description to at least "default-agent" ok?

is this for the "default-agent" command? I forgot to add a description for that one?

Regards

Marcel


2013-06-19 13:40:52

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/5] client: Display command even with no description

On Wed, Jun 19, 2013 at 7:31 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Johan,
>
>>>> Having no description doesn't mean it's not an available command. So add
>>>> command to the output of "help" command, even if it has no description.
>>>> ---
>>>> client/main.c | 5 ++---
>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/client/main.c b/client/main.c
>>>> index b0a66d8..54af807 100644
>>>> --- a/client/main.c
>>>> +++ b/client/main.c
>>>> @@ -1182,10 +1182,9 @@ static void rl_handler(char *input)
>>>> printf("Available commands:\n");
>>>>
>>>> for (i = 0; cmd_table[i].cmd; i++) {
>>>> - if (cmd_table[i].desc)
>>>> - printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>>> + printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>>> cmd_table[i].arg ? : " ",
>>>> - cmd_table[i].desc);
>>>> + cmd_table[i].desc ? : "");
>>>
>>>
>>> if I remember this correctly, then I was doing it to be able to hide
>>> commands from the help output. Mainly commands that are on purpose
>>> duplicates.
>>
>> So the right fix is to add the missing descriptions then? Lucas, can you
>> provide an updated patch that does this?
>
> actually I wanted to hide these commands on purpose.

So you can't pair a device starting from remote using bluetoothctl on
purpose? Or is adding a description to at least "default-agent" ok?

Lucas De Marchi

2013-06-19 10:31:42

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/5] client: Display command even with no description

Hi Johan,

>>> Having no description doesn't mean it's not an available command. So add
>>> command to the output of "help" command, even if it has no description.
>>> ---
>>> client/main.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/client/main.c b/client/main.c
>>> index b0a66d8..54af807 100644
>>> --- a/client/main.c
>>> +++ b/client/main.c
>>> @@ -1182,10 +1182,9 @@ static void rl_handler(char *input)
>>> printf("Available commands:\n");
>>>
>>> for (i = 0; cmd_table[i].cmd; i++) {
>>> - if (cmd_table[i].desc)
>>> - printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>> + printf("\t%s %s\t%s\n", cmd_table[i].cmd,
>>> cmd_table[i].arg ? : " ",
>>> - cmd_table[i].desc);
>>> + cmd_table[i].desc ? : "");
>>
>>
>> if I remember this correctly, then I was doing it to be able to hide
>> commands from the help output. Mainly commands that are on purpose
>> duplicates.
>
> So the right fix is to add the missing descriptions then? Lucas, can you
> provide an updated patch that does this?

actually I wanted to hide these commands on purpose.

Regards

Marcel


2013-06-19 09:08:50

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/5] client: Display command even with no description

Hi Marcel, Lucas,

On Wed, Jun 19, 2013, Marcel Holtmann wrote:
> Hi Lucas,
>
> > Having no description doesn't mean it's not an available command. So add
> > command to the output of "help" command, even if it has no description.
> > ---
> > client/main.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/client/main.c b/client/main.c
> > index b0a66d8..54af807 100644
> > --- a/client/main.c
> > +++ b/client/main.c
> > @@ -1182,10 +1182,9 @@ static void rl_handler(char *input)
> > printf("Available commands:\n");
> >
> > for (i = 0; cmd_table[i].cmd; i++) {
> > - if (cmd_table[i].desc)
> > - printf("\t%s %s\t%s\n", cmd_table[i].cmd,
> > + printf("\t%s %s\t%s\n", cmd_table[i].cmd,
> > cmd_table[i].arg ? : " ",
> > - cmd_table[i].desc);
> > + cmd_table[i].desc ? : "");
>
>
> if I remember this correctly, then I was doing it to be able to hide
> commands from the help output. Mainly commands that are on purpose
> duplicates.

So the right fix is to add the missing descriptions then? Lucas, can you
provide an updated patch that does this?

Johan

2013-06-19 05:08:33

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/5] client: Display command even with no description

Hi Lucas,

> Having no description doesn't mean it's not an available command. So add
> command to the output of "help" command, even if it has no description.
> ---
> client/main.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/client/main.c b/client/main.c
> index b0a66d8..54af807 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -1182,10 +1182,9 @@ static void rl_handler(char *input)
> printf("Available commands:\n");
>
> for (i = 0; cmd_table[i].cmd; i++) {
> - if (cmd_table[i].desc)
> - printf("\t%s %s\t%s\n", cmd_table[i].cmd,
> + printf("\t%s %s\t%s\n", cmd_table[i].cmd,
> cmd_table[i].arg ? : " ",
> - cmd_table[i].desc);
> + cmd_table[i].desc ? : "");


if I remember this correctly, then I was doing it to be able to hide commands from the help output. Mainly commands that are on purpose duplicates.

Regards

Marcel


2013-06-18 20:13:46

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 4/5] client: Display command even with no description

From: Lucas De Marchi <[email protected]>

Having no description doesn't mean it's not an available command. So add
command to the output of "help" command, even if it has no description.
---
client/main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/client/main.c b/client/main.c
index b0a66d8..54af807 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1182,10 +1182,9 @@ static void rl_handler(char *input)
printf("Available commands:\n");

for (i = 0; cmd_table[i].cmd; i++) {
- if (cmd_table[i].desc)
- printf("\t%s %s\t%s\n", cmd_table[i].cmd,
+ printf("\t%s %s\t%s\n", cmd_table[i].cmd,
cmd_table[i].arg ? : " ",
- cmd_table[i].desc);
+ cmd_table[i].desc ? : "");
}

done:
--
1.8.3.1


2013-06-18 20:13:45

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 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-18 20:13:47

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 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 54af807..db6705f 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1182,9 +1182,10 @@ static void rl_handler(char *input)
printf("Available commands:\n");

for (i = 0; cmd_table[i].cmd; i++) {
- 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-18 20:13:44

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ 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