* Re: [PATCH] Bluetooth: Remove unneeded zero init
From: Marcel Holtmann @ 2012-08-30 15:06 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1346317467-20903-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> hdev is allocated with kzalloc so zero initialization is not needed.
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 36720f0..05bcea6 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -443,10 +443,8 @@ static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> static inline void hci_conn_hash_init(struct hci_dev *hdev)
> {
> struct hci_conn_hash *h = &hdev->conn_hash;
> +
> INIT_LIST_HEAD(&h->list);
> - h->acl_num = 0;
> - h->sco_num = 0;
> - h->le_num = 0;
> }
since there is only one user of hci_conn_hash_init, we should just
replace that call with a direct INIT_LIST_HEAD.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions
From: Marcel Holtmann @ 2012-08-30 15:08 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1346332740-29319-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
> Using this macro in configure.ac enables certain extensions that BlueZ
> currently depends on. The macro is recommended instead of defining
> _GNU_SOURCE on each C file.
what is the advantage of this. I am actually fine with using _GNU_SOURCE
in the C files. It is according to the man pages.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH obexd] client: Add parameters to get message in map module
From: Luiz Augusto von Dentz @ 2012-08-30 15:21 UTC (permalink / raw)
To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1346233190-6238-1-git-send-email-frederic.danis@linux.intel.com>
Hi Frédéric,
On Wed, Aug 29, 2012 at 12:39 PM, Frédéric Danis
<frederic.danis@linux.intel.com> wrote:
> GetMessage function needs attachment and charset application params
> ---
> client/map.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/client/map.c b/client/map.c
> index 4f07fcb..fa27c21 100644
> --- a/client/map.c
> +++ b/client/map.c
> @@ -29,6 +29,8 @@
> #include <glib.h>
> #include <gdbus.h>
>
> +#include <gobex/gobex-apparam.h>
> +
> #include "dbus.h"
> #include "log.h"
>
> @@ -36,6 +38,7 @@
> #include "transfer.h"
> #include "session.h"
> #include "driver.h"
> +#include "map_ap.h"
>
> #define OBEX_MAS_UUID \
> "\xBB\x58\x2B\x40\x42\x0C\x11\xDB\xB0\xDE\x08\x00\x20\x0C\x9A\x66"
> @@ -258,6 +261,9 @@ static DBusMessage *map_msg_get(DBusConnection *connection,
> struct map_msg *msg = user_data;
> struct obc_transfer *transfer;
> const char *target_file;
> + GObexApparam *apparam;
> + guint8 buf[6];
> + gsize len;
> GError *err = NULL;
> DBusMessage *reply;
>
> @@ -272,6 +278,14 @@ static DBusMessage *map_msg_get(DBusConnection *connection,
> if (transfer == NULL)
> goto fail;
>
> + apparam = g_obex_apparam_set_uint8(NULL, MAP_AP_ATTACHMENT, 0);
> + apparam = g_obex_apparam_set_uint8(apparam, MAP_AP_CHARSET, 1);
> + len = g_obex_apparam_encode(apparam, buf, sizeof(buf));
> +
> + obc_transfer_set_params(transfer, buf, len);
> +
> + g_obex_apparam_free(apparam);
> +
> if (!obc_session_queue(msg->data->session, transfer, NULL, NULL, &err))
> goto fail;
>
> --
> 1.7.9.5
I have a pending patchset to address this, basically adding filters
parameters so the application can enable what this parameters should
be. Btw, Im confused why are you setting attachment as type uint8, the
spec says it should assume "On" or "Off", which normally means it is
of string type.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions
From: Anderson Lizardo @ 2012-08-30 15:26 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1346339333.23377.10.camel@aeonflux>
Hi Marcel,
On Thu, Aug 30, 2012 at 11:08 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Anderson,
>
>> Using this macro in configure.ac enables certain extensions that BlueZ
>> currently depends on. The macro is recommended instead of defining
>> _GNU_SOURCE on each C file.
>
> what is the advantage of this. I am actually fine with using _GNU_SOURCE
> in the C files. It is according to the man pages.
The only advantage I see is that we don't have to worry about
reviewing these defines as the symbols get incorporated in newer
standards. For instance, this patch was brought up because O_CLOEXEC
does not exist on POSIX.1-2001, but was incorporated POSIX.1-2008, so
for newer systems _GNU_SOURCE is not necessary for it anymore, but for
some (still maintained) distros it is.
Sometimes we remove code that used these extensions, and simply forget
the _GNU_SOURCE there as well.
It is just a minor build system simplification, if you think it is not
worthy, please reconsider my original patch that adds _GNU_SOURCE just
where it is necessary.
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 3/4] Bluetooth: Avoid rfcomm_session_timeout using freed pointer
From: Dean Jenkins @ 2012-08-30 15:36 UTC (permalink / raw)
To: Gustavo Padovan, Dean Jenkins, linux-bluetooth
In-Reply-To: <20120821185606.GF17005@joana>
Hi Gustavo,
On 21 August 2012 19:56, Gustavo Padovan <gustavo@padovan.org> wrote:
> Hi Dean,
>
> * Dean Jenkins <djenkins@mvista.com> [2012-08-11 19:47:09 +0100]:
>
>> rfcomm_session_timeout() protects the scenario of the remote
>> Bluetooth device failing to send a DISC on the rfcomm control
>> channel after the last data DLC channel has been closed.
>>
>> There is a race condition between the timer expiring causing
>> rfcomm_session_timeout() to run and the rfcomm session being
>> deleted. If the rfcomm session is deleted then
>> rfcomm_session_timeout() would use a freed rfcomm session
>> pointer resulting in a potential kernel crash or memory corruption.
>> Note the timer is cleared before the rfcomm session is deleted
>> by del_timer() so the circumstances for a failure to occur are
>> as follows:
>>
>> rfcomm_session_timeout() needs to be executing before the
>> del_timer() is called to clear the timer but
>> rfcomm_session_timeout() needs to be delayed from using the
>> rfcomm session pointer until after the session has been deleted.
>> Therefore, there is a very small window of opportunity for failure.
>>
>> The solution is to use del_timer_sync() instead of del_timer()
>> as this ensures that rfcomm_session_timeout() is not running
>> when del_timer_sync() returns. This means that it is not
>> possible for rfcomm_session_timeout() to run after the rfcomm
>> session has been deleted.
>
>>
>> Signed-off-by: Dean Jenkins <djenkins@mvista.com>
>> ---
>> net/bluetooth/rfcomm/core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
>> index 24d4d3c..c7921fd 100644
>> --- a/net/bluetooth/rfcomm/core.c
>> +++ b/net/bluetooth/rfcomm/core.c
>> @@ -264,8 +264,8 @@ static void rfcomm_session_clear_timer(struct rfcomm_session *s)
>> {
>> BT_DBG("session %p state %ld", s, s->state);
>>
>> - if (timer_pending(&s->timer))
>> - del_timer(&s->timer);
>> + /* ensure rfcomm_session_timeout() is not running past this point */
>> + del_timer_sync(&s->timer);
>
> I'm not happy of the idea of let the stack broken between patches 2 and 3
> (this one). As you said if we use del_timer_sync() we don't need rfcnt here,
> can you add this as a first patch, maybe? and after this continue to remove
> the rest of the refcount code?
>
> Gustavo
Thanks for your feedback. Sorry for the delay in getting back to you.
My gmail failed to filter out your reply.
OK, I'll start with a patch for del_timer_sync(). This can be a
standalone patch.
For the removal of the refcnt, do you propose 1 patch to remove the
refcnt AND to manage the rfcomm session pointer ? I have doubts now
because you don't wish the stack to be broken between patches. Perhaps
it is possible to add a patch to manage the rfcomm session pointer
with the refcnt in place so we have "belt and braces" then have a
patch to remove the refcnt as the last thing to do.
I am open to suggestions.
Thanks,
Regards,
Dean
--
Dean Jenkins
Embedded Software Engineer
Professional Services UK/EMEA
MontaVista Software, LLC
^ permalink raw reply
* Re: [PATCH 4/4] Bluetooth: On socket shutdown check rfcomm session and DLC exists
From: Dean Jenkins @ 2012-08-30 15:40 UTC (permalink / raw)
To: Gustavo Padovan, Dean Jenkins, linux-bluetooth
In-Reply-To: <20120821185029.GE17005@joana>
Hi Gustavo,
On 21 August 2012 19:50, Gustavo Padovan <gustavo@padovan.org> wrote:
> Hi Dean,
>
> * Dean Jenkins <djenkins@mvista.com> [2012-08-11 19:47:10 +0100]:
>
>> A race condition exists between near simultaneous asynchronous
>> DLC data channel disconnection requests from the host and remote device.
>> This causes the socket layer to request a socket shutdown at the same time
>> the rfcomm core is processing the disconnect request from the remote
>> device.
>>
>> The socket layer retains a copy of a struct rfcomm_dlc d pointer.
>> The d pointer refers to a copy of a struct rfcomm_session.
>> When the socket layer thread performs a socket shutdown, the thread
>> may wait on a rfcomm lock in rfcomm_dlc_close(). This means that
>> whilst the thread waits, the rfcomm_session and/or rfcomm_dlc structures
>> pointed to by d maybe freed due to rfcomm core handling. Consequently,
>> when the rfcomm lock becomes available and the thread runs, a
>> malfunction could occur as a freed rfcomm_session pointer and/or a
>> freed d pointer will be erroneously reused.
>>
>> Therefore, after the rfcomm lock is acquired, check that the struct
>> rfcomm_session is still valid by searching the rfcomm session list.
>> If the session is valid then validate the d pointer by searching the
>> rfcomm session list of active DLCs for the rfcomm_dlc structure
>> pointed by d.
>>
>> If active DLCs exist when the rfcomm session is terminating,
>> avoid a memory leak of rfcomm_dlc structures by ensuring that
>> rfcomm_session_close() is used instead of rfcomm_session_del().
>>
>> Signed-off-by: Dean Jenkins <djenkins@mvista.com>
>> ---
>> net/bluetooth/rfcomm/core.c | 29 ++++++++++++++++++++++++++---
>> 1 file changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
>> index c7921fd..1a7db34 100644
>> --- a/net/bluetooth/rfcomm/core.c
>> +++ b/net/bluetooth/rfcomm/core.c
>> @@ -496,11 +496,34 @@ static int __rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
>>
>> int rfcomm_dlc_close(struct rfcomm_dlc *d, int err)
>> {
>> - int r;
>> + int r = 0;
>> + struct rfcomm_dlc *d_list;
>> + struct rfcomm_session *s, *s_list;
>> + struct list_head *p, *n, *p2, *n2;
>>
>> rfcomm_lock();
>>
>> - r = __rfcomm_dlc_close(d, err);
>> + s = d->session;
>> + if (s) {
>
> Please invert the check here, and add a goto unlock;. There too many
> indentation levels here.
>
>> + /* check the session still exists after waiting on the mutex */
>> + list_for_each_safe(p, n, &session_list) {
>> + s_list = list_entry(p, struct rfcomm_session, list);
>
> please use list_for_each_entry(), the _safe version seems to not be needed.
>
>> + if (s == s_list) {
>> + /* check the dlc still exists */
>> + /* after waiting on the mutex */
>> + list_for_each_safe(p2, n2, &s->dlcs) {
>> + d_list = list_entry(p2,
>> + struct rfcomm_dlc,
>> + list);
>
>
> and here you can use rfcomm_dlc_get()
>
> Gustavo
Thanks for your feedback. Sorry for the delay getting back to you. I
blame gmail.
OK, I will take a look at redoing the patch as per your suggestion.
Thanks,
Regards,
Dean
--
Dean Jenkins
Embedded Software Engineer
Professional Services UK/EMEA
MontaVista Software, LLC
^ permalink raw reply
* [PATCH BlueZ v0] hcitool: Add LE scan based on White List
From: Claudio Takahasi @ 2012-08-30 16:16 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
This patch extends the LE scanning command allowing the user to inform
an option to enable LE scanning detecting advertises sent from devices
in the White List Only. This option can be used combined with others.
---
tools/hcitool.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/hcitool.c b/tools/hcitool.c
index 66e5c20..aefbd68 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -2487,6 +2487,7 @@ static struct option lescan_options[] = {
{ "help", 0, 0, 'h' },
{ "privacy", 0, 0, 'p' },
{ "passive", 0, 0, 'P' },
+ { "whitelist", 0, 0, 'w' },
{ "discovery", 1, 0, 'd' },
{ "duplicates", 0, 0, 'D' },
{ 0, 0, 0, 0 }
@@ -2496,6 +2497,7 @@ static const char *lescan_help =
"Usage:\n"
"\tlescan [--privacy] enable privacy\n"
"\tlescan [--passive] set scan type passive (default active)\n"
+ "\tlescan [--whitelist] scan for address in the whitelist only\n"
"\tlescan [--discovery=g|l] enable general or limited discovery"
"procedure\n"
"\tlescan [--duplicates] don't filter duplicates\n";
@@ -2506,6 +2508,7 @@ static void cmd_lescan(int dev_id, int argc, char **argv)
uint8_t own_type = 0x00;
uint8_t scan_type = 0x01;
uint8_t filter_type = 0;
+ uint8_t filter_policy = 0x00;
uint16_t interval = htobs(0x0010);
uint16_t window = htobs(0x0010);
uint8_t filter_dup = 1;
@@ -2518,6 +2521,9 @@ static void cmd_lescan(int dev_id, int argc, char **argv)
case 'P':
scan_type = 0x00; /* Passive */
break;
+ case 'w':
+ filter_policy = 0x01; /* Whitelist */
+ break;
case 'd':
filter_type = optarg[0];
if (filter_type != 'g' && filter_type != 'l') {
@@ -2548,7 +2554,7 @@ static void cmd_lescan(int dev_id, int argc, char **argv)
}
err = hci_le_set_scan_parameters(dd, scan_type, interval, window,
- own_type, 0x00, 1000);
+ own_type, filter_policy, 1000);
if (err < 0) {
perror("Set scan parameters failed");
exit(1);
--
1.7.12
^ permalink raw reply related
* [PATCH 1/2] rctest: add automated test
From: Gustavo Padovan @ 2012-08-30 18:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo Padovan
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
adds -a option to enable automated tests. We use the -i option to define
the receiving side and the -a define the sending side:
./rctest -i hci0 -a hci1
---
test/rctest.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/test/rctest.c b/test/rctest.c
index f82d2cc..0c645b7 100644
--- a/test/rctest.c
+++ b/test/rctest.c
@@ -55,7 +55,8 @@ enum {
DUMP,
CONNECT,
CRECV,
- LSEND
+ LSEND,
+ AUTO,
};
static unsigned char *buf;
@@ -72,6 +73,7 @@ static unsigned long delay = 0;
/* Default addr and channel */
static bdaddr_t bdaddr;
+static bdaddr_t auto_bdaddr;
static uint16_t uuid = 0x0000;
static uint8_t channel = 10;
@@ -162,7 +164,11 @@ static int do_connect(const char *svr)
/* Bind to local address */
memset(&addr, 0, sizeof(addr));
addr.rc_family = AF_BLUETOOTH;
- bacpy(&addr.rc_bdaddr, &bdaddr);
+
+ if (bacmp(&auto_bdaddr, BDADDR_ANY))
+ bacpy(&addr.rc_bdaddr, &auto_bdaddr);
+ else
+ bacpy(&addr.rc_bdaddr, &bdaddr);
if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
syslog(LOG_ERR, "Can't bind socket: %s (%d)",
@@ -591,6 +597,23 @@ static void multi_connect_mode(int argc, char *argv[])
}
}
+static void automated_send_recv()
+{
+ int sk;
+ char device[18];
+
+ if (fork()) {
+ do_listen(recv_mode);
+ } else {
+ ba2str(&bdaddr, device);
+
+ sk = do_connect(device);
+ if (sk < 0)
+ exit(1);
+ send_mode(sk);
+ }
+}
+
static void usage(void)
{
printf("rctest - RFCOMM testing\n"
@@ -604,7 +627,8 @@ static void usage(void)
"\t-u connect and receive\n"
"\t-n connect and be silent\n"
"\t-c connect, disconnect, connect, ...\n"
- "\t-m multiple connects\n");
+ "\t-m multiple connects\n"
+ "\t-a automated test (receive hcix as parameter)\n");
printf("Options:\n"
"\t[-b bytes] [-i device] [-P channel] [-U uuid]\n"
@@ -628,8 +652,9 @@ int main(int argc, char *argv[])
int opt, sk, mode = RECV, need_addr = 0;
bacpy(&bdaddr, BDADDR_ANY);
+ bacpy(&auto_bdaddr, BDADDR_ANY);
- while ((opt=getopt(argc,argv,"rdscuwmnb:i:P:U:B:N:MAESL:W:C:D:Y:T")) != EOF) {
+ while ((opt=getopt(argc,argv,"rdscuwmna:b:i:P:U:B:N:MAESL:W:C:D:Y:T")) != EOF) {
switch (opt) {
case 'r':
mode = RECV;
@@ -668,6 +693,15 @@ int main(int argc, char *argv[])
need_addr = 1;
break;
+ case 'a':
+ mode = AUTO;
+
+ if (!strncasecmp(optarg, "hci", 3))
+ hci_devba(atoi(optarg + 3), &auto_bdaddr);
+ else
+ str2ba(optarg, &auto_bdaddr);
+ break;
+
case 'b':
data_size = atoi(optarg);
break;
@@ -804,6 +838,10 @@ int main(int argc, char *argv[])
exit(1);
dump_mode(sk);
break;
+
+ case AUTO:
+ automated_send_recv();
+ break;
}
syslog(LOG_INFO, "Exit");
--
1.7.11.2
^ permalink raw reply related
* [PATCH 2/2] rctest: add option to save received data to file
From: Gustavo Padovan @ 2012-08-30 18:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo Padovan
In-Reply-To: <1346351548-5056-1-git-send-email-gustavo@padovan.org>
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
works only for automated test option for now
---
test/rctest.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 52 insertions(+), 3 deletions(-)
diff --git a/test/rctest.c b/test/rctest.c
index 0c645b7..f60c8f9 100644
--- a/test/rctest.c
+++ b/test/rctest.c
@@ -78,6 +78,8 @@ static uint16_t uuid = 0x0000;
static uint8_t channel = 10;
static char *filename = NULL;
+static char *savefile = NULL;
+static int save_fd = 0;
static int master = 0;
static int auth = 0;
@@ -448,6 +450,23 @@ static void dump_mode(int sk)
syslog(LOG_INFO, "Received %d bytes", len);
}
+static void save_mode(int sk)
+{
+ int len, ret;
+ char *b;
+
+ b = malloc(data_size);
+ if (!b)
+ syslog(LOG_ERR, "Failed to open file to save recv data");
+
+ syslog(LOG_INFO, "Receiving ...");
+ while ((len = read(sk, b, data_size)) > 0) {
+ ret = write(save_fd, b, len);
+ if (ret < 0)
+ return;
+ }
+}
+
static void recv_mode(int sk)
{
struct timeval tv_beg, tv_end, tv_diff;
@@ -603,7 +622,20 @@ static void automated_send_recv()
char device[18];
if (fork()) {
- do_listen(recv_mode);
+ if (!savefile) {
+ do_listen(recv_mode);
+ return;
+ }
+
+ save_fd = open(savefile, O_CREAT | O_WRONLY,
+ S_IRUSR | S_IWUSR);
+ if (!save_fd)
+ syslog(LOG_ERR, "Failed to open file to save "
+ "recv data");
+
+ do_listen(save_mode);
+
+ close(save_fd);
} else {
ba2str(&bdaddr, device);
@@ -614,6 +646,15 @@ static void automated_send_recv()
}
}
+static void sig_child_exit(int code)
+{
+ syslog(LOG_INFO, "Exit");
+ exit(0);
+
+ if (save_fd)
+ close(save_fd);
+}
+
static void usage(void)
{
printf("rctest - RFCOMM testing\n"
@@ -635,6 +676,7 @@ static void usage(void)
"\t[-L seconds] enabled SO_LINGER option\n"
"\t[-W seconds] enable deferred setup\n"
"\t[-B filename] use data packets from file\n"
+ "\t[-O filename] save received data to file\n"
"\t[-N num] number of frames to send\n"
"\t[-C num] send num frames before delay (default = 1)\n"
"\t[-D milliseconds] delay after sending num frames (default = 0)\n"
@@ -654,7 +696,7 @@ int main(int argc, char *argv[])
bacpy(&bdaddr, BDADDR_ANY);
bacpy(&auto_bdaddr, BDADDR_ANY);
- while ((opt=getopt(argc,argv,"rdscuwmna:b:i:P:U:B:N:MAESL:W:C:D:Y:T")) != EOF) {
+ while ((opt=getopt(argc,argv,"rdscuwmna:b:i:P:U:B:O:N:MAESL:W:C:D:Y:T")) != EOF) {
switch (opt) {
case 'r':
mode = RECV;
@@ -754,6 +796,10 @@ int main(int argc, char *argv[])
filename = strdup(optarg);
break;
+ case 'O':
+ savefile = strdup(optarg);
+ break;
+
case 'N':
num_frames = atoi(optarg);
break;
@@ -791,7 +837,10 @@ int main(int argc, char *argv[])
}
memset(&sa, 0, sizeof(sa));
- sa.sa_handler = SIG_IGN;
+ if (mode == AUTO)
+ sa.sa_handler = sig_child_exit;
+ else
+ sa.sa_handler = SIG_IGN;
sa.sa_flags = SA_NOCLDSTOP;
sigaction(SIGCHLD, &sa, NULL);
--
1.7.11.2
^ permalink raw reply related
* Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions
From: Lucas De Marchi @ 2012-08-30 20:02 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <CAJdJm_NXvcfYeCT5gaYpDD0Oshk89ohUaj5BJ66CL29YwD7UgQ@mail.gmail.com>
Hi Marcel,
On Thu, Aug 30, 2012 at 12:26 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Marcel,
>
> On Thu, Aug 30, 2012 at 11:08 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Anderson,
>>
>>> Using this macro in configure.ac enables certain extensions that BlueZ
>>> currently depends on. The macro is recommended instead of defining
>>> _GNU_SOURCE on each C file.
>>
>> what is the advantage of this. I am actually fine with using _GNU_SOURCE
>> in the C files. It is according to the man pages.
>
> The only advantage I see is that we don't have to worry about
> reviewing these defines as the symbols get incorporated in newer
> standards. For instance, this patch was brought up because O_CLOEXEC
> does not exist on POSIX.1-2001, but was incorporated POSIX.1-2008, so
> for newer systems _GNU_SOURCE is not necessary for it anymore, but for
> some (still maintained) distros it is.
>
> Sometimes we remove code that used these extensions, and simply forget
> the _GNU_SOURCE there as well.
And also forget to add it. Then 3 months later comes a patch to fix it
by adding the definition.
Using the autofoo macro we stop the build-fix patches for things like this.
That is: it removes code, it's more future-proof and it has no
downsides. So, what's the point of not using it?
Some months ago I sent a patch to also remove a lot of dumb "#include
config.h". It has the same reasoning behind it: remove code and be
more future-proof. It was for connman, but if you are interested I can
send it to bluez/ofono/etc too -
http://permalink.gmane.org/gmane.linux.network.connman/7310
Lucas De Marchi
^ permalink raw reply
* Re: [PATCH BlueZ v3 06/14] core: mutually exclude concurrent connections
From: Joao Paulo Rechi Vita @ 2012-08-30 22:08 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20120817075117.GC30060@x220>
On Fri, Aug 17, 2012 at 4:51 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi,
>
> On Thu, Aug 16, 2012, João Paulo Rechi Vita wrote:
>> @@ -133,8 +133,10 @@ struct btd_adapter {
>> + GSList *connecting_list; /* Pending connects */
>> + gboolean connecting; /* Connect active */
>
> What's the relationship with the above two? Isn't connecting always true
> when g_slist_length(connecting_list) > 0 and false when the list is
> empty?
>
The connecting_list keeps track of devices that have been found but
for which a connect has not been issued yet. This was mainly used for
1) not trying to connect twice to the same device (on two subsequent
device_found events), and 2) to know if there are still any device
waiting for a connect when one connect finishes, to device if we
should restart scanning or not.
Reviewing this logic I've been able to simplify it a little bit, using
a counter (adapter->waiting_to_connect) to address problem #2, and #1
could be handled only using the adapter->connect_list.
>> static gboolean connect_pending_cb(gpointer user_data)
>> {
>> struct btd_device *device = user_data;
>> struct btd_adapter *adapter = device_get_adapter(device);
>> + GIOChannel *io;
>>
>> /* in the future we may want to check here if the controller supports
>> * scanning and connecting at the same time */
>> if (adapter->discovering)
>> return TRUE;
>>
>> - device_att_connect(device);
>> + if (adapter->connecting)
>> + return TRUE;
>> +
>> + adapter->connecting = TRUE;
>> + io = device_att_connect(device);
>> + g_io_add_watch(io, G_IO_OUT | G_IO_ERR, clean_connecting_state,
>> + btd_device_ref(device));
>>
>> return FALSE;
>> }
>
> Since you're not storing "io" after the function returns you should be
> doing a g_io_channel_unref() after calling g_io_add_watch(). Looking at
> device_att_connect it seems like it's missing a g_io_channel_ref (either
> when assigning to device->att_io or when returning from the function).
>
>> + adapter->connecting_list = g_slist_append(
>> + adapter->connecting_list, device);
>>
>
> Missing btd_device_unref()?
>
The whole ref counting has been reviewed as well, with the new logic
modifications mentioned above.
--
João Paulo Rechi Vita
Openbossa Labs - INdT
^ permalink raw reply
* [PATCH BlueZ v4 00/15] LE General Connection Establishment procedure
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
This series implement the LE General Connection Establishment procedure
for LE connections.
If there are LE bonded devices marked for auto connection they are added
to a connect_list on the adapter. When there is any device on this list
scan is performed continuously. When a device is found the connect_list
is checked. If that device is on the list scan is stopped and a
connection attempt is made to that device.
If any client tries to perform discovery and the scan for the General
Connection Establishment procedure is active, the discovery request is
queued and performed right after the GCEP scan session finishes.
This series changes quite a lot the LE connection logic, but we've been
testing and using this code at INdT for about 4 weeks. Any comments and
more testing are appreciated.
Claudio Takahasi (6):
core: Control connections based on adapter state
mgmt: Add LE scanning callback
core: Replace interleaved by LE scanning
core: Start LE scanning when a device requests
core: Queue discovery if scanning is active
core: Re-connect for ECONNRESET or ECONNABORTED
João Paulo Rechi Vita (8):
core: Fix missing g_io_channel_ref
mgmt: Print error message when start_discovery fails
core: Add compare function for bdaddr in a struct btd_device
core: Add a list of LE devices to connect
core: Use adapter connect list for LE connections
core: Mutually exclude concurrent connections
mgmt: Add address type to bonding debug message
core: Suspend scanning before connect on pairing
Paulo Alcantara (1):
core: Disable unnecessary auto connections
src/adapter.c | 193 ++++++++++++++++++++++++++++++++++++++++++++-------
src/adapter.h | 5 ++
src/device.c | 219 +++++++++++++++++++++++++++++-----------------------------
src/device.h | 2 +
src/mgmt.c | 44 +++++++++++-
src/mgmt.h | 1 +
6 files changed, 326 insertions(+), 138 deletions(-)
--
1.7.11.4
^ permalink raw reply
* [PATCH BlueZ v4 01/15] core: Fix missing g_io_channel_ref
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
---
src/device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/device.c b/src/device.c
index 3b44d9b..2c40ec2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2027,7 +2027,7 @@ static gboolean att_connect(gpointer user_data)
return FALSE;
}
- device->att_io = io;
+ device->att_io = g_io_channel_ref(io);
return FALSE;
}
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 02/15] core: Control connections based on adapter state
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
From: Claudio Takahasi <claudio.takahasi@openbossa.org>
This patch disable automatic ATTIO connections when the adapter is
powered down and enable automatic connection when the adapter is powered
on.
---
src/adapter.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 50779fd..3df46fe 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2098,6 +2098,14 @@ static int get_pairable_timeout(const char *src)
return main_opts.pairto;
}
+static void set_auto_connect(gpointer data, gpointer user_data)
+{
+ struct btd_device *device = data;
+ gboolean enable = GPOINTER_TO_INT(user_data);
+
+ device_set_auto_connect(device, enable);
+}
+
static void call_adapter_powered_callbacks(struct btd_adapter *adapter,
gboolean powered)
{
@@ -2107,7 +2115,10 @@ static void call_adapter_powered_callbacks(struct btd_adapter *adapter,
btd_adapter_powered_cb cb = l->data;
cb(adapter, powered);
- }
+ }
+
+ g_slist_foreach(adapter->devices, set_auto_connect,
+ GINT_TO_POINTER(powered));
}
static void emit_device_disappeared(gpointer data, gpointer user_data)
@@ -3316,15 +3327,10 @@ static gboolean disable_auto(gpointer user_data)
return FALSE;
}
-static void set_auto_connect(gpointer data, gpointer user_data)
-{
- struct btd_device *device = data;
-
- device_set_auto_connect(device, TRUE);
-}
-
void btd_adapter_enable_auto_connect(struct btd_adapter *adapter)
{
+ gboolean enable = TRUE;
+
if (!adapter->up)
return;
@@ -3333,7 +3339,8 @@ void btd_adapter_enable_auto_connect(struct btd_adapter *adapter)
if (adapter->auto_timeout_id)
return;
- g_slist_foreach(adapter->devices, set_auto_connect, NULL);
+ g_slist_foreach(adapter->devices, set_auto_connect,
+ GINT_TO_POINTER(enable));
adapter->auto_timeout_id = g_timeout_add_seconds(main_opts.autoto,
disable_auto, adapter);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 03/15] mgmt: Print error message when start_discovery fails
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
If we fail to communicate with the MGMT socket is better to print the
error message on the mgmtops plugin, where it really happened, instead
of leaving this job to its users.
---
src/adapter.c | 6 +-----
src/mgmt.c | 7 +++++--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 3df46fe..3b3f0ad 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1002,13 +1002,9 @@ struct btd_device *adapter_get_device(DBusConnection *conn,
static gboolean discovery_cb(gpointer user_data)
{
struct btd_adapter *adapter = user_data;
- int err;
adapter->discov_id = 0;
-
- err = mgmt_start_discovery(adapter->dev_id);
- if (err < 0)
- error("start_discovery: %s (%d)", strerror(-err), -err);
+ mgmt_start_discovery(adapter->dev_id);
return FALSE;
}
diff --git a/src/mgmt.c b/src/mgmt.c
index 58aab2d..651c940 100644
--- a/src/mgmt.c
+++ b/src/mgmt.c
@@ -1996,8 +1996,11 @@ int mgmt_start_discovery(int index)
cp->type = info->discov_type;
- if (write(mgmt_sock, buf, sizeof(buf)) < 0)
- return -errno;
+ if (write(mgmt_sock, buf, sizeof(buf)) < 0) {
+ int err = -errno;
+ error("failed to write to MGMT socket: %s", strerror(-err));
+ return err;
+ }
return 0;
}
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 04/15] core: Add compare function for bdaddr in a struct btd_device
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
This is a utility function similar to device_address_cmp but comparing
bdaddr instead of the string representing the address. This way is
possible to avoid allocating two buffers to temporarily hold the
strings, two sprintf() calls to generate the strings from the bdaddr
arrays, and a string comparison, substituting all of it for one memcmp()
call.
---
src/device.c | 5 +++++
src/device.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/src/device.c b/src/device.c
index 2c40ec2..2abc9dc 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1226,6 +1226,11 @@ gint device_address_cmp(struct btd_device *device, const gchar *address)
return strcasecmp(addr, address);
}
+gint device_bdaddr_cmp(struct btd_device *device, bdaddr_t *bdaddr)
+{
+ return bacmp(&device->bdaddr, bdaddr);
+}
+
static gboolean record_has_uuid(const sdp_record_t *rec,
const char *profile_uuid)
{
diff --git a/src/device.h b/src/device.h
index a65de26..9157465 100644
--- a/src/device.h
+++ b/src/device.h
@@ -45,6 +45,7 @@ uint16_t btd_device_get_product(struct btd_device *device);
uint16_t btd_device_get_version(struct btd_device *device);
void device_remove(struct btd_device *device, gboolean remove_stored);
gint device_address_cmp(struct btd_device *device, const gchar *address);
+gint device_bdaddr_cmp(struct btd_device *device, bdaddr_t *bdaddr);
int device_browse_primary(struct btd_device *device, DBusConnection *conn,
DBusMessage *msg, gboolean secure);
int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 05/15] core: Add a list of LE devices to connect
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
This commit creates a per-adapter list of LE devices to connect when a
advertising from them is seen during a scan.
---
src/adapter.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
src/adapter.h | 5 +++++
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/src/adapter.c b/src/adapter.c
index 3b3f0ad..5613056 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -132,6 +132,7 @@ struct btd_adapter {
GSList *devices; /* Devices structure pointers */
GSList *mode_sessions; /* Request Mode sessions */
GSList *disc_sessions; /* Discovery sessions */
+ GSList *connect_list; /* Devices to connect when found */
guint discov_id; /* Discovery timer */
gboolean discovering; /* Discovery active */
gboolean discov_suspended; /* Discovery suspended */
@@ -2181,6 +2182,36 @@ const char *btd_adapter_get_name(struct btd_adapter *adapter)
return adapter->name;
}
+void adapter_connect_list_add(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ if (g_slist_find(adapter->connect_list, device)) {
+ DBG("ignoring already added device %s",
+ device_get_path(device));
+ return;
+ }
+
+ adapter->connect_list = g_slist_append(adapter->connect_list,
+ btd_device_ref(device));
+ DBG("%s added to %s's connect_list", device_get_path(device),
+ adapter->name);
+}
+
+void adapter_connect_list_remove(struct btd_adapter *adapter,
+ struct btd_device *device)
+{
+ if (!g_slist_find(adapter->connect_list, device)) {
+ DBG("device %s is not on the list, ignoring",
+ device_get_path(device));
+ return;
+ }
+
+ adapter->connect_list = g_slist_remove(adapter->connect_list, device);
+ DBG("%s removed from %s's connect_list", device_get_path(device),
+ adapter->name);
+ btd_device_unref(device);
+}
+
void btd_adapter_start(struct btd_adapter *adapter)
{
char address[18];
@@ -2843,6 +2874,21 @@ static char *read_stored_data(bdaddr_t *local, bdaddr_t *peer,
return textfile_get(filename, key);
}
+static gboolean connect_pending_cb(gpointer user_data)
+{
+ struct btd_device *device = user_data;
+ struct btd_adapter *adapter = device_get_adapter(device);
+
+ /* in the future we may want to check here if the controller supports
+ * scanning and connecting at the same time */
+ if (adapter->discovering)
+ return TRUE;
+
+ /* TODO: call device connect callback */
+
+ return FALSE;
+}
+
void adapter_update_found_devices(struct btd_adapter *adapter,
bdaddr_t *bdaddr, uint8_t bdaddr_type,
int8_t rssi, uint8_t confirm_name,
@@ -2854,6 +2900,7 @@ void adapter_update_found_devices(struct btd_adapter *adapter,
gboolean legacy, name_known;
uint32_t dev_class;
int err;
+ GSList *l;
memset(&eir_data, 0, sizeof(eir_data));
err = eir_parse(&eir_data, data, data_len);
@@ -2876,7 +2923,7 @@ void adapter_update_found_devices(struct btd_adapter *adapter,
eir_data.name);
dev = adapter_search_found_devices(adapter, bdaddr);
- if (dev) {
+ if (dev && dev->bdaddr_type == BDADDR_BREDR) {
adapter->oor_devices = g_slist_remove(adapter->oor_devices,
dev);
@@ -2928,6 +2975,16 @@ void adapter_update_found_devices(struct btd_adapter *adapter,
adapter->found_devices = g_slist_prepend(adapter->found_devices, dev);
+ if (bdaddr_type == BDADDR_LE_PUBLIC ||
+ bdaddr_type == BDADDR_LE_RANDOM) {
+ l = g_slist_find_custom(adapter->connect_list, bdaddr,
+ (GCompareFunc) device_bdaddr_cmp);
+ if (l) {
+ g_idle_add(connect_pending_cb, l->data);
+ stop_discovery(adapter);
+ }
+ }
+
done:
dev->rssi = rssi;
diff --git a/src/adapter.h b/src/adapter.h
index 5a0247e..cd37b15 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -232,3 +232,8 @@ int btd_adapter_gatt_server_start(struct btd_adapter *adapter);
void btd_adapter_gatt_server_stop(struct btd_adapter *adapter);
int btd_adapter_ssp_enabled(struct btd_adapter *adapter);
+
+void adapter_connect_list_add(struct btd_adapter *adapter,
+ struct btd_device *device);
+void adapter_connect_list_remove(struct btd_adapter *adapter,
+ struct btd_device *device);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 06/15] core: Use adapter connect list for LE connections
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
When a connection is needed for a LE device it is added to the adapter
connect list instead of directly connecting the ATT io channel.
---
src/adapter.c | 2 +-
src/device.c | 45 +++++++++++----------------------------------
src/device.h | 1 +
3 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 5613056..d7c1cea 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2884,7 +2884,7 @@ static gboolean connect_pending_cb(gpointer user_data)
if (adapter->discovering)
return TRUE;
- /* TODO: call device connect callback */
+ device_att_connect(device);
return FALSE;
}
diff --git a/src/device.c b/src/device.c
index 2abc9dc..56090f1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -67,8 +67,6 @@
#define DISCONNECT_TIMER 2
#define DISCOVERY_TIMER 2
-#define AUTO_CONNECTION_INTERVAL 5 /* Next connection attempt */
-
struct btd_disconnect_data {
guint id;
disconnect_watch watch;
@@ -1836,15 +1834,6 @@ static void attio_disconnected(gpointer data, gpointer user_data)
attio->dcfunc(attio->user_data);
}
-static void att_connect_dispatched(gpointer user_data)
-{
- struct btd_device *device = user_data;
-
- device->auto_id = 0;
-}
-
-static gboolean att_connect(gpointer user_data);
-
static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
@@ -1864,10 +1853,7 @@ static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
if (device->auto_connect == FALSE || err != ETIMEDOUT)
goto done;
- device->auto_id = g_timeout_add_seconds_full(G_PRIORITY_DEFAULT_IDLE,
- AUTO_CONNECTION_INTERVAL,
- att_connect, device,
- att_connect_dispatched);
+ adapter_connect_list_add(device_get_adapter(device), device);
done:
attio_cleanup(device);
@@ -1967,11 +1953,7 @@ static void att_error_cb(const GError *gerr, gpointer user_data)
if (device->auto_connect == FALSE)
return;
- device->auto_id = g_timeout_add_seconds_full(G_PRIORITY_DEFAULT_IDLE,
- AUTO_CONNECTION_INTERVAL,
- att_connect, device,
- att_connect_dispatched);
-
+ adapter_connect_list_add(device_get_adapter(device), device);
DBG("Enabling automatic connections");
}
@@ -1986,7 +1968,7 @@ static void att_success_cb(gpointer user_data)
g_slist_foreach(device->attios, attio_connected, device->attrib);
}
-static gboolean att_connect(gpointer user_data)
+gboolean device_att_connect(gpointer user_data)
{
struct btd_device *device = user_data;
struct btd_adapter *adapter = device->adapter;
@@ -2248,6 +2230,9 @@ void device_set_temporary(struct btd_device *device, gboolean temporary)
DBG("temporary %d", temporary);
+ if (temporary)
+ adapter_connect_list_remove(device_get_adapter(device), device);
+
device->temporary = temporary;
}
@@ -2263,6 +2248,7 @@ void device_set_bonded(struct btd_device *device, gboolean bonded)
void device_set_auto_connect(struct btd_device *device, gboolean enable)
{
+ struct btd_adapter *adapter = device_get_adapter(device);
char addr[18];
if (!device)
@@ -2276,15 +2262,10 @@ void device_set_auto_connect(struct btd_device *device, gboolean enable)
/* Disabling auto connect */
if (enable == FALSE) {
- if (device->auto_id)
- g_source_remove(device->auto_id);
+ adapter_connect_list_remove(adapter, device);
return;
}
- /* Enabling auto connect */
- if (device->auto_id != 0)
- return;
-
if (device->attrib) {
DBG("Already connected");
return;
@@ -2293,9 +2274,8 @@ void device_set_auto_connect(struct btd_device *device, gboolean enable)
if (device->attios == NULL && device->attios_offline == NULL)
return;
- device->auto_id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE,
- att_connect, device,
- att_connect_dispatched);
+ /* Enabling auto connect */
+ adapter_connect_list_add(adapter, device);
}
static gboolean start_discovery(gpointer user_data)
@@ -3134,10 +3114,7 @@ guint btd_device_add_attio_callback(struct btd_device *device,
device->attios = g_slist_append(device->attios, attio);
- if (device->auto_id == 0)
- device->auto_id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE,
- att_connect, device,
- att_connect_dispatched);
+ adapter_connect_list_add(device_get_adapter(device), device);
return attio->id;
}
diff --git a/src/device.h b/src/device.h
index 9157465..06ebc8a 100644
--- a/src/device.h
+++ b/src/device.h
@@ -131,3 +131,4 @@ int device_unblock(DBusConnection *conn, struct btd_device *device,
void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
uint16_t vendor_id, uint16_t product_id,
uint16_t product_ver);
+gboolean device_att_connect(gpointer user_data);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 07/15] core: Mutually exclude concurrent connections
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
Since controllers don't support more than one ongoing connection
procedure at the same time, new connection attempts needs to yield if
there is an ongoing connection procedure already.
---
src/adapter.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
src/device.c | 6 +++---
src/device.h | 2 +-
3 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index d7c1cea..fdd9cf9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -135,6 +135,8 @@ struct btd_adapter {
GSList *connect_list; /* Devices to connect when found */
guint discov_id; /* Discovery timer */
gboolean discovering; /* Discovery active */
+ gboolean connecting; /* Connect active */
+ guint waiting_to_connect; /* # of devices waiting to connect */
gboolean discov_suspended; /* Discovery suspended */
guint auto_timeout_id; /* Automatic connections timeout */
sdp_list_t *services; /* Services associated to adapter */
@@ -2239,6 +2241,9 @@ void btd_adapter_start(struct btd_adapter *adapter)
call_adapter_powered_callbacks(adapter, TRUE);
info("Adapter %s has been enabled", adapter->path);
+
+ if (g_slist_length(adapter->connect_list))
+ mgmt_start_discovery(adapter->dev_id);
}
static void reply_pending_requests(struct btd_adapter *adapter)
@@ -2548,6 +2553,7 @@ void adapter_set_discovering(struct btd_adapter *adapter,
gboolean discovering)
{
const char *path = adapter->path;
+ guint connect_list_size;
adapter->discovering = discovering;
@@ -2562,11 +2568,17 @@ void adapter_set_discovering(struct btd_adapter *adapter,
g_slist_free_full(adapter->oor_devices, dev_info_free);
adapter->oor_devices = g_slist_copy(adapter->found_devices);
- if (!adapter_has_discov_sessions(adapter) || adapter->discov_suspended)
+ if (adapter->discov_suspended)
+ return;
+
+ connect_list_size = g_slist_length(adapter->connect_list);
+
+ if (!adapter_has_discov_sessions(adapter) && !connect_list_size)
return;
- DBG("hci%u restarting discovery, disc_sessions %u", adapter->dev_id,
- g_slist_length(adapter->disc_sessions));
+ DBG("hci%u restarting discovery: disc_sessions %u, connect_list size "
+ "%u", adapter->dev_id, g_slist_length(adapter->disc_sessions),
+ connect_list_size);
adapter->discov_id = g_idle_add(discovery_cb, adapter);
}
@@ -2874,17 +2886,43 @@ static char *read_stored_data(bdaddr_t *local, bdaddr_t *peer,
return textfile_get(filename, key);
}
+static gboolean clean_connecting_state(GIOChannel *io, GIOCondition cond, gpointer user_data)
+{
+ struct btd_device *device = user_data;
+ struct btd_adapter *adapter = device_get_adapter(device);
+
+ adapter->connecting = FALSE;
+
+ if (adapter->waiting_to_connect == 0 &&
+ g_slist_length(adapter->connect_list))
+ mgmt_start_discovery(adapter->dev_id);
+
+ btd_device_unref(device);
+
+ return FALSE;
+}
+
static gboolean connect_pending_cb(gpointer user_data)
{
struct btd_device *device = user_data;
struct btd_adapter *adapter = device_get_adapter(device);
+ GIOChannel *io;
/* in the future we may want to check here if the controller supports
* scanning and connecting at the same time */
if (adapter->discovering)
return TRUE;
- device_att_connect(device);
+ if (adapter->connecting)
+ return TRUE;
+
+ adapter->connecting = TRUE;
+ adapter->waiting_to_connect--;
+
+ io = device_att_connect(device);
+ g_io_add_watch(io, G_IO_OUT | G_IO_ERR, clean_connecting_state,
+ btd_device_ref(device));
+ g_io_channel_unref(io);
return FALSE;
}
@@ -2977,12 +3015,19 @@ void adapter_update_found_devices(struct btd_adapter *adapter,
if (bdaddr_type == BDADDR_LE_PUBLIC ||
bdaddr_type == BDADDR_LE_RANDOM) {
+ struct btd_device *device;
+
l = g_slist_find_custom(adapter->connect_list, bdaddr,
(GCompareFunc) device_bdaddr_cmp);
- if (l) {
- g_idle_add(connect_pending_cb, l->data);
- stop_discovery(adapter);
- }
+ if (!l)
+ goto done;
+
+ device = l->data;
+ adapter_connect_list_remove(adapter, device);
+
+ g_idle_add(connect_pending_cb, btd_device_ref(device));
+ stop_discovery(adapter);
+ adapter->waiting_to_connect++;
}
done:
diff --git a/src/device.c b/src/device.c
index 56090f1..1093237 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1968,7 +1968,7 @@ static void att_success_cb(gpointer user_data)
g_slist_foreach(device->attios, attio_connected, device->attrib);
}
-gboolean device_att_connect(gpointer user_data)
+GIOChannel *device_att_connect(gpointer user_data)
{
struct btd_device *device = user_data;
struct btd_adapter *adapter = device->adapter;
@@ -2011,12 +2011,12 @@ gboolean device_att_connect(gpointer user_data)
error("ATT bt_io_connect(%s): %s", addr, gerr->message);
g_error_free(gerr);
g_free(attcb);
- return FALSE;
+ return NULL;
}
device->att_io = g_io_channel_ref(io);
- return FALSE;
+ return io;
}
static void att_browse_error_cb(const GError *gerr, gpointer user_data)
diff --git a/src/device.h b/src/device.h
index 06ebc8a..95d9215 100644
--- a/src/device.h
+++ b/src/device.h
@@ -131,4 +131,4 @@ int device_unblock(DBusConnection *conn, struct btd_device *device,
void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
uint16_t vendor_id, uint16_t product_id,
uint16_t product_ver);
-gboolean device_att_connect(gpointer user_data);
+GIOChannel *device_att_connect(gpointer user_data);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 08/15] mgmt: Add LE scanning callback
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
From: Claudio Takahasi <claudio.takahasi@openbossa.org>
This patch adds a new callback to allow the adapter to control LE
scanning. The current approach uses the active scanning with default
windows and intervals defined by the core spec without any filtering.
---
src/mgmt.c | 34 ++++++++++++++++++++++++++++++++++
src/mgmt.h | 1 +
2 files changed, 35 insertions(+)
diff --git a/src/mgmt.c b/src/mgmt.c
index 651c940..662d70b 100644
--- a/src/mgmt.c
+++ b/src/mgmt.c
@@ -2005,6 +2005,40 @@ int mgmt_start_discovery(int index)
return 0;
}
+int mgmt_start_scanning(int index)
+{
+ char buf[MGMT_HDR_SIZE + sizeof(struct mgmt_cp_start_discovery)];
+ struct mgmt_hdr *hdr = (void *) buf;
+ struct mgmt_cp_start_discovery *cp = (void *) &buf[sizeof(*hdr)];
+ struct controller_info *info = &controllers[index];
+
+ DBG("index %d", index);
+
+ if (!mgmt_low_energy(info->current_settings)) {
+ error("scanning failed: Low Energy not enabled/supported");
+ return -ENOTSUP;
+ }
+
+ info->discov_type = 0;
+ hci_set_bit(BDADDR_LE_PUBLIC, &info->discov_type);
+ hci_set_bit(BDADDR_LE_RANDOM, &info->discov_type);
+
+ memset(buf, 0, sizeof(buf));
+ hdr->opcode = htobs(MGMT_OP_START_DISCOVERY);
+ hdr->len = htobs(sizeof(*cp));
+ hdr->index = htobs(index);
+
+ cp->type = info->discov_type;
+
+ if (write(mgmt_sock, buf, sizeof(buf)) < 0) {
+ int err = -errno;
+ error("failed to write to MGMT socket: %s", strerror(-err));
+ return err;
+ }
+
+ return 0;
+}
+
int mgmt_stop_discovery(int index)
{
char buf[MGMT_HDR_SIZE + sizeof(struct mgmt_cp_start_discovery)];
diff --git a/src/mgmt.h b/src/mgmt.h
index 95245d2..eb7434a 100644
--- a/src/mgmt.h
+++ b/src/mgmt.h
@@ -33,6 +33,7 @@ int mgmt_set_dev_class(int index, uint8_t major, uint8_t minor);
int mgmt_set_fast_connectable(int index, gboolean enable);
int mgmt_start_discovery(int index);
+int mgmt_start_scanning(int index);
int mgmt_stop_discovery(int index);
int mgmt_read_clock(int index, bdaddr_t *bdaddr, int which, int timeout,
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 09/15] core: Replace interleaved by LE scanning
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
From: Claudio Takahasi <claudio.takahasi@openbossa.org>
This patches replaces the interleaved discovery by LE scanning when LE
re-connection is required.
---
src/adapter.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index fdd9cf9..c30dd83 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2243,7 +2243,7 @@ void btd_adapter_start(struct btd_adapter *adapter)
info("Adapter %s has been enabled", adapter->path);
if (g_slist_length(adapter->connect_list))
- mgmt_start_discovery(adapter->dev_id);
+ mgmt_start_scanning(adapter->dev_id);
}
static void reply_pending_requests(struct btd_adapter *adapter)
@@ -2573,14 +2573,22 @@ void adapter_set_discovering(struct btd_adapter *adapter,
connect_list_size = g_slist_length(adapter->connect_list);
- if (!adapter_has_discov_sessions(adapter) && !connect_list_size)
+ if (adapter_has_discov_sessions(adapter)) {
+ adapter->discov_id = g_idle_add(discovery_cb, adapter);
+
+ DBG("hci%u restarting discovery: disc_sessions %u",
+ adapter->dev_id,
+ g_slist_length(adapter->disc_sessions));
return;
+ }
- DBG("hci%u restarting discovery: disc_sessions %u, connect_list size "
- "%u", adapter->dev_id, g_slist_length(adapter->disc_sessions),
- connect_list_size);
+ if (connect_list_size) {
+ mgmt_start_scanning(adapter->dev_id);
- adapter->discov_id = g_idle_add(discovery_cb, adapter);
+ DBG("hci%u restarting scanning connect_list size %u",
+ adapter->dev_id, connect_list_size);
+ return;
+ }
}
static void suspend_discovery(struct btd_adapter *adapter)
@@ -2895,7 +2903,7 @@ static gboolean clean_connecting_state(GIOChannel *io, GIOCondition cond, gpoint
if (adapter->waiting_to_connect == 0 &&
g_slist_length(adapter->connect_list))
- mgmt_start_discovery(adapter->dev_id);
+ mgmt_start_scanning(adapter->dev_id);
btd_device_unref(device);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 10/15] core: Start LE scanning when a device requests
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
From: Claudio Takahasi <claudio.takahasi@openbossa.org>
This patch enables the LE scanning when a device requires connection and
there isn't discovery sessions, triggering the General Connection
Establishment Procedure.
---
src/adapter.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/adapter.c b/src/adapter.c
index c30dd83..9e8667d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2197,6 +2197,11 @@ void adapter_connect_list_add(struct btd_adapter *adapter,
btd_device_ref(device));
DBG("%s added to %s's connect_list", device_get_path(device),
adapter->name);
+
+ if (adapter->disc_sessions)
+ return;
+
+ mgmt_start_scanning(adapter->dev_id);
}
void adapter_connect_list_remove(struct btd_adapter *adapter,
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 11/15] core: Queue discovery if scanning is active
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
From: Claudio Takahasi <claudio.takahasi@openbossa.org>
This patch manages BR/EDR inquiry and BLE scanning discovery sessions.
A scanning session is added in the discovery session list when there is
a bonded device which requires re-connection.
bluetoothd decides if interleaved or scanning needs to be executed based
on the queued discovery sessions. Interleaved discovery has higher
priority, scanning only is executed when there is only a scanning
session active.
---
src/adapter.c | 67 +++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 47 insertions(+), 20 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 9e8667d..3131dc6 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -132,6 +132,7 @@ struct btd_adapter {
GSList *devices; /* Devices structure pointers */
GSList *mode_sessions; /* Request Mode sessions */
GSList *disc_sessions; /* Discovery sessions */
+ struct session_req *scanning_session;
GSList *connect_list; /* Devices to connect when found */
guint discov_id; /* Discovery timer */
gboolean discovering; /* Discovery active */
@@ -220,18 +221,19 @@ static struct session_req *create_session(struct btd_adapter *adapter,
DBusConnection *conn, DBusMessage *msg,
uint8_t mode, GDBusWatchFunction cb)
{
- const char *sender = dbus_message_get_sender(msg);
+ const char *sender;
struct session_req *req;
req = g_new0(struct session_req, 1);
req->adapter = adapter;
- req->conn = dbus_connection_ref(conn);
- req->msg = dbus_message_ref(msg);
req->mode = mode;
- if (cb == NULL)
+ if (conn == NULL || cb == NULL || msg == NULL)
return session_ref(req);
+ req->conn = dbus_connection_ref(conn);
+ req->msg = dbus_message_ref(msg);
+ sender = dbus_message_get_sender(msg);
req->owner = g_strdup(sender);
req->id = g_dbus_add_disconnect_watch(conn, sender, cb, req, NULL);
@@ -443,7 +445,9 @@ static struct session_req *find_session(GSList *list, const char *sender)
for (; list; list = list->next) {
struct session_req *req = list->data;
- if (g_str_equal(req->owner, sender))
+ /* req->owner may be NULL if the session has been added by the
+ * daemon itself, so we use g_strcmp0 instead of g_str_equal */
+ if (g_strcmp0(req->owner, sender) == 0)
return req;
}
@@ -518,7 +522,7 @@ static void session_remove(struct session_req *req)
struct btd_adapter *adapter = req->adapter;
/* Ignore set_mode session */
- if (req->owner == NULL)
+ if (req->owner == NULL && adapter->pending_mode)
return;
DBG("%s session %p with %s deactivated",
@@ -1007,7 +1011,12 @@ static gboolean discovery_cb(gpointer user_data)
struct btd_adapter *adapter = user_data;
adapter->discov_id = 0;
- mgmt_start_discovery(adapter->dev_id);
+
+ if (adapter->scanning_session &&
+ (g_slist_length(adapter->disc_sessions) == 1))
+ mgmt_start_scanning(adapter->dev_id);
+ else
+ mgmt_start_discovery(adapter->dev_id);
return FALSE;
}
@@ -2187,6 +2196,8 @@ const char *btd_adapter_get_name(struct btd_adapter *adapter)
void adapter_connect_list_add(struct btd_adapter *adapter,
struct btd_device *device)
{
+ struct session_req *req;
+
if (g_slist_find(adapter->connect_list, device)) {
DBG("ignoring already added device %s",
device_get_path(device));
@@ -2198,10 +2209,21 @@ void adapter_connect_list_add(struct btd_adapter *adapter,
DBG("%s added to %s's connect_list", device_get_path(device),
adapter->name);
- if (adapter->disc_sessions)
+ if (!adapter->up)
+ return;
+
+ if (adapter->off_requested)
+ return;
+
+ if (adapter->scanning_session)
return;
- mgmt_start_scanning(adapter->dev_id);
+ if (adapter->disc_sessions == NULL)
+ adapter->discov_id = g_idle_add(discovery_cb, adapter);
+
+ req = create_session(adapter, NULL, NULL, 0, NULL);
+ adapter->disc_sessions = g_slist_append(adapter->disc_sessions, req);
+ adapter->scanning_session = req;
}
void adapter_connect_list_remove(struct btd_adapter *adapter,
@@ -2221,6 +2243,7 @@ void adapter_connect_list_remove(struct btd_adapter *adapter,
void btd_adapter_start(struct btd_adapter *adapter)
{
+ struct session_req *req;
char address[18];
gboolean powered;
@@ -2247,8 +2270,15 @@ void btd_adapter_start(struct btd_adapter *adapter)
info("Adapter %s has been enabled", adapter->path);
- if (g_slist_length(adapter->connect_list))
- mgmt_start_scanning(adapter->dev_id);
+ if (g_slist_length(adapter->connect_list) == 0 ||
+ adapter->disc_sessions)
+ return;
+
+ req = create_session(adapter, NULL, NULL, 0, NULL);
+ adapter->disc_sessions = g_slist_append(adapter->disc_sessions, req);
+ adapter->scanning_session = req;
+
+ adapter->discov_id = g_idle_add(discovery_cb, adapter);
}
static void reply_pending_requests(struct btd_adapter *adapter)
@@ -2578,6 +2608,11 @@ void adapter_set_discovering(struct btd_adapter *adapter,
connect_list_size = g_slist_length(adapter->connect_list);
+ if (connect_list_size == 0 && adapter->scanning_session) {
+ session_unref(adapter->scanning_session);
+ adapter->scanning_session = NULL;
+ }
+
if (adapter_has_discov_sessions(adapter)) {
adapter->discov_id = g_idle_add(discovery_cb, adapter);
@@ -2586,14 +2621,6 @@ void adapter_set_discovering(struct btd_adapter *adapter,
g_slist_length(adapter->disc_sessions));
return;
}
-
- if (connect_list_size) {
- mgmt_start_scanning(adapter->dev_id);
-
- DBG("hci%u restarting scanning connect_list size %u",
- adapter->dev_id, connect_list_size);
- return;
- }
}
static void suspend_discovery(struct btd_adapter *adapter)
@@ -2908,7 +2935,7 @@ static gboolean clean_connecting_state(GIOChannel *io, GIOCondition cond, gpoint
if (adapter->waiting_to_connect == 0 &&
g_slist_length(adapter->connect_list))
- mgmt_start_scanning(adapter->dev_id);
+ adapter->discov_id = g_idle_add(discovery_cb, adapter);
btd_device_unref(device);
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 12/15] core: Disable unnecessary auto connections
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Paulo Alcantara
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
From: Paulo Alcantara <paulo.alcantara@openbossa.org>
BlueZ host disconnects the link when encryption fails. ECONNABORTED
error is returned by the kernel when the connection is terminated by the
local host. This scenario commonly happens when authentication fails due
PIN or Key Missing.
---
src/device.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/device.c b/src/device.c
index 1093237..b0c4b12 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1950,6 +1950,9 @@ static void att_error_cb(const GError *gerr, gpointer user_data)
struct att_callbacks *attcb = user_data;
struct btd_device *device = attcb->user_data;
+ if (g_error_matches(gerr, BT_IO_ERROR, ECONNABORTED))
+ return;
+
if (device->auto_connect == FALSE)
return;
--
1.7.11.4
^ permalink raw reply related
* [PATCH BlueZ v4 13/15] core: Re-connect for ECONNRESET or ECONNABORTED
From: João Paulo Rechi Vita @ 2012-08-30 22:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1346364960-7699-1-git-send-email-jprvita@openbossa.org>
From: Claudio Takahasi <claudio.takahasi@openbossa.org>
This patch keeps scanning and re-connections active if the disconnection
reason is ECONNRESET(Remote Initiated Disconnection).
Re-connection is a behaviour determined by Profiles or by the upper
layer(user actions). For instance, HoG requires re-connection always
active, no matter if the previous disconnection reason was page timeout
or remote initiated disconnection (ECONNRESET). Some devices disconnects
after some idle time, connectable advertises are sent by the peripheral
when commanded by the user(eg: key pressed). Disconnection can be also
triggered by the local host (ECONNABORTED) using command line tools or
Disconnect method in the Device interface.
The peripheral dictates the re-connection controlling the connectable
advertises, BlueZ(central) needs to keep the scanning always active to
able to detect the advertises and trigger the connection.
---
src/device.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/device.c b/src/device.c
index b0c4b12..3789c6c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1850,10 +1850,18 @@ static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
g_slist_foreach(device->attios, attio_disconnected, NULL);
- if (device->auto_connect == FALSE || err != ETIMEDOUT)
+ if (device->auto_connect == FALSE) {
+ DBG("Automatic connection disabled");
goto done;
+ }
- adapter_connect_list_add(device_get_adapter(device), device);
+ /*
+ * Keep scanning/re-connection active if disconnection reason
+ * is page timeout, remote user terminated connection or local
+ * initiated disconnection.
+ */
+ if (err == ETIMEDOUT || err == ECONNRESET || err == ECONNABORTED)
+ adapter_connect_list_add(device_get_adapter(device), device);
done:
attio_cleanup(device);
--
1.7.11.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox