* Re: [PATCH] android/hal-audio: Workaround AudioFlinger issues
From: Szymon Janc @ 2014-01-24 10:09 UTC (permalink / raw)
To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1390490712-25124-1-git-send-email-andrzej.kaczmarek@tieto.com>
Hi Andrzej,
On Thursday 23 of January 2014 16:25:12 Andrzej Kaczmarek wrote:
> Audio HAL code calculates accurate input stream buffer size which
> allows to fill media packets with as much data as possible. However,
> in most cases calculated buffer size does not work well with Android
> audio code which causes glitches when playing simultaneously to
> different audio output (like notification) or crashes mediaserver
> when disconnecting with headset.
>
> This patch changes input buffer size to fixed magic value 20*512 which
> is used in Bluedroid Audio HAL. Such change requires that we need to
> drop assumption that each input buffer can be used to fill exactly one
> media packet and need to use it to fill multiple media packets. To
> avoid buffering in Audio HAL, we allow that last media packet can be
> filled in non-optimal way, i.e. has less data that can fit.
> ---
> android/hal-audio.c | 86 ++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 56 insertions(+), 30 deletions(-)
>
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index 4326ccd..f4a4ee1 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -413,6 +413,42 @@ static void sbc_resume(void *codec_data)
> sbc_data->frames_sent = 0;
> }
>
> +static void write_media_packet(int fd, struct sbc_data *sbc_data,
> + struct media_packet *mp, size_t data_len)
> +{
> + struct timespec cur;
> + struct timespec diff;
> + unsigned expected_frames;
> + int ret;
> +
> + ret = write(fd, mp, sizeof(*mp) + data_len);
> + if (ret < 0) {
> + int err = errno;
> + DBG("error writing data: %d (%s)", err,
> + strerror(err));
> + }
> +
> + sbc_data->frames_sent += mp->payload.frame_count;
> +
> + clock_gettime(CLOCK_MONOTONIC, &cur);
> + timespec_diff(&cur, &sbc_data->start, &diff);
> + expected_frames = (diff.tv_sec * 1000000 + diff.tv_nsec / 1000) /
> + sbc_data->frame_duration;
> +
> + /* AudioFlinger does not seem to provide any *working*
> + * API to provide data in some interval and will just
> + * send another buffer as soon as we process current
> + * one. To prevent overflowing L2CAP socket, we need to
> + * introduce some artificial delay here base on how many
> + * audio frames were sent so far, i.e. if we're not
> + * lagging behind audio stream, we can sleep for
> + * duration of single media packet.
> + */
> + if (sbc_data->frames_sent >= expected_frames)
> + usleep(sbc_data->frame_duration *
> + mp->payload.frame_count);
> +}
> +
> static ssize_t sbc_write_data(void *codec_data, const void *buffer,
> size_t bytes, int fd)
> {
> @@ -421,9 +457,6 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
> size_t encoded = 0;
> struct media_packet *mp = (struct media_packet *) sbc_data->out_buf;
> size_t free_space = sbc_data->out_buf_size - sizeof(*mp);
> - struct timespec cur;
> - struct timespec diff;
> - unsigned expected_frames;
> int ret;
>
> mp->hdr.v = 2;
> @@ -450,39 +483,28 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
> consumed += ret;
> encoded += written;
> free_space -= written;
> - }
>
> - ret = write(fd, mp, sizeof(*mp) + encoded);
> - if (ret < 0) {
> - int err = errno;
> - DBG("error writing data: %d (%s)", err, strerror(err));
> + /* write data if we either filled media packed or encoded all
> + * input data
> + */
> + if (mp->payload.frame_count == sbc_data->frames_per_packet ||
> + bytes == consumed) {
> + write_media_packet(fd, sbc_data, mp, encoded);
> +
> + encoded = 0;
> + free_space = sbc_data->out_buf_size - sizeof(*mp);
> + mp->payload.frame_count = 0;
> + }
> }
>
> - if (consumed != bytes || free_space != 0) {
> - /* we should encode all input data and fill output buffer
> + if (consumed != bytes) {
> + /* we should encode all input data
> * if we did not, something went wrong but we can't really
> * handle this so this is just sanity check
> */
> DBG("some data were not encoded");
> }
>
> - sbc_data->frames_sent += mp->payload.frame_count;
> -
> - clock_gettime(CLOCK_MONOTONIC, &cur);
> - timespec_diff(&cur, &sbc_data->start, &diff);
> - expected_frames = (diff.tv_sec * 1000000 + diff.tv_nsec / 1000) /
> - sbc_data->frame_duration;
> -
> - /* AudioFlinger does not seem to provide any *working* API to provide
> - * data in some interval and will just send another buffer as soon as
> - * we process current one. To prevent overflowing L2CAP socket, we need
> - * to introduce some artificial delay here base on how many audio frames
> - * were sent so far, i.e. if we're not lagging behind audio stream, we
> - * can sleep for duration of single media packet.
> - */
> - if (sbc_data->frames_sent >= expected_frames)
> - usleep(sbc_data->frame_duration * mp->payload.frame_count);
> -
> /* we always assume that all data was processed and sent */
> return bytes;
> }
> @@ -853,11 +875,15 @@ static int out_set_sample_rate(struct audio_stream *stream, uint32_t rate)
>
> static size_t out_get_buffer_size(const struct audio_stream *stream)
> {
> - struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
> -
> DBG("");
>
> - return out->ep->codec->get_buffer_size(out->ep->codec_data);
> + /* We should return proper buffer size calculated by codec (so each
> + * input buffer is encoded into single media packed) but this does not
> + * work well with AudioFlinger and causes problems. For this reason we
> + * use magic value here and out_write code takes care of splitting
> + * input buffer into multiple media packets.
> + */
> + return 20 * 512;
> }
>
> static uint32_t out_get_channels(const struct audio_stream *stream)
>
Pushed, thanks.
--
Best regards,
Szymon Janc
^ permalink raw reply
* Re: [PATCH v2 2/2] Bluetooth: Switch ATT channels to use L2CAP_CHAN_FIXED
From: Marcel Holtmann @ 2014-01-24 8:46 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <1390552541-11729-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> ATT channels are not connection oriented so having them use
> L2CAP_CHAN_CONN_ORIENTED is quite confusing. Instead, use the new
> L2CAP_CHAN_FIXED type and ensure that the MTU and CID values get
> properly set.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/l2cap_core.c | 21 +++++++++------------
> net/bluetooth/l2cap_sock.c | 9 +++++++++
> 2 files changed, 18 insertions(+), 12 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2 1/2] Bluetooth: Rename L2CAP_CHAN_CONN_FIX_A2MP to L2CAP_CHAN_FIXED
From: Marcel Holtmann @ 2014-01-24 8:45 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <1390552541-11729-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> There's no reason why A2MP should need or deserve its on channel type.
> Instead we should be able to group all fixed CID users under a single
> channel type and reuse as much code as possible for them. Where CID
> specific exceptions are needed the chan-scid value can be used.
>
> This patch renames the current A2MP channel type to a generic one and
> thereby paves the way to allow converting ATT and SMP (and any future
> fixed channel protocols) to use the new channel type.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/l2cap.h | 2 +-
> net/bluetooth/a2mp.c | 8 ++++++--
> net/bluetooth/l2cap_core.c | 15 ++++++---------
> 3 files changed, 13 insertions(+), 12 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* [PATCH v2 2/2] Bluetooth: Switch ATT channels to use L2CAP_CHAN_FIXED
From: johan.hedberg @ 2014-01-24 8:35 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1390552541-11729-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
ATT channels are not connection oriented so having them use
L2CAP_CHAN_CONN_ORIENTED is quite confusing. Instead, use the new
L2CAP_CHAN_FIXED type and ensure that the MTU and CID values get
properly set.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/l2cap_core.c | 21 +++++++++------------
net/bluetooth/l2cap_sock.c | 9 +++++++++
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index cd28057d2903..e5c5c7427c41 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -498,18 +498,10 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
switch (chan->chan_type) {
case L2CAP_CHAN_CONN_ORIENTED:
- if (conn->hcon->type == LE_LINK) {
- if (chan->dcid == L2CAP_CID_ATT) {
- chan->omtu = L2CAP_DEFAULT_MTU;
- chan->scid = L2CAP_CID_ATT;
- } else {
- chan->scid = l2cap_alloc_cid(conn);
- }
- } else {
- /* Alloc CID for connection-oriented socket */
- chan->scid = l2cap_alloc_cid(conn);
+ /* Alloc CID for connection-oriented socket */
+ chan->scid = l2cap_alloc_cid(conn);
+ if (conn->hcon->type == ACL_LINK)
chan->omtu = L2CAP_DEFAULT_MTU;
- }
break;
case L2CAP_CHAN_CONN_LESS:
@@ -7025,7 +7017,12 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
goto done;
}
- if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED && !(psm || cid)) {
+ if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED && !psm) {
+ err = -EINVAL;
+ goto done;
+ }
+
+ if (chan->chan_type == L2CAP_CHAN_FIXED && !cid) {
err = -EINVAL;
goto done;
}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 4aa6704f0b33..3f8e2a223474 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -101,6 +101,15 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
if (!bdaddr_type_is_valid(la.l2_bdaddr_type))
return -EINVAL;
+ if (la.l2_cid) {
+ /* When the socket gets created it defaults to
+ * CHAN_CONN_ORIENTED, so we need to overwrite the
+ * default here.
+ */
+ chan->chan_type = L2CAP_CHAN_FIXED;
+ chan->omtu = L2CAP_DEFAULT_MTU;
+ }
+
if (bdaddr_type_is_le(la.l2_bdaddr_type)) {
if (!enable_lecoc && la.l2_psm)
return -EINVAL;
--
1.8.5.3
^ permalink raw reply related
* [PATCH v2 1/2] Bluetooth: Rename L2CAP_CHAN_CONN_FIX_A2MP to L2CAP_CHAN_FIXED
From: johan.hedberg @ 2014-01-24 8:35 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@intel.com>
There's no reason why A2MP should need or deserve its on channel type.
Instead we should be able to group all fixed CID users under a single
channel type and reuse as much code as possible for them. Where CID
specific exceptions are needed the chan-scid value can be used.
This patch renames the current A2MP channel type to a generic one and
thereby paves the way to allow converting ATT and SMP (and any future
fixed channel protocols) to use the new channel type.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/l2cap.h | 2 +-
net/bluetooth/a2mp.c | 8 ++++++--
net/bluetooth/l2cap_core.c | 15 ++++++---------
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 85cf40acc47e..ae482f41594a 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -651,7 +651,7 @@ struct l2cap_user {
#define L2CAP_CHAN_RAW 1
#define L2CAP_CHAN_CONN_LESS 2
#define L2CAP_CHAN_CONN_ORIENTED 3
-#define L2CAP_CHAN_CONN_FIX_A2MP 4
+#define L2CAP_CHAN_FIXED 4
/* ----- L2CAP socket info ----- */
#define l2cap_pi(sk) ((struct l2cap_pinfo *) sk)
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index efcd108822c4..f986b9968bdb 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -235,7 +235,7 @@ static int a2mp_discover_rsp(struct amp_mgr *mgr, struct sk_buff *skb,
BT_DBG("chan %p state %s", chan,
state_to_string(chan->state));
- if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP)
+ if (chan->scid == L2CAP_CID_A2MP)
continue;
l2cap_chan_lock(chan);
@@ -726,7 +726,11 @@ static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn, bool locked)
BT_DBG("chan %p", chan);
- chan->chan_type = L2CAP_CHAN_CONN_FIX_A2MP;
+ chan->chan_type = L2CAP_CHAN_FIXED;
+ chan->scid = L2CAP_CID_A2MP;
+ chan->dcid = L2CAP_CID_A2MP;
+ chan->omtu = L2CAP_A2MP_DEFAULT_MTU;
+ chan->imtu = L2CAP_A2MP_DEFAULT_MTU;
chan->flush_to = L2CAP_DEFAULT_FLUSH_TO;
chan->ops = &a2mp_chan_ops;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 138394ad3e51..cd28057d2903 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -519,11 +519,8 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
chan->omtu = L2CAP_DEFAULT_MTU;
break;
- case L2CAP_CHAN_CONN_FIX_A2MP:
- chan->scid = L2CAP_CID_A2MP;
- chan->dcid = L2CAP_CID_A2MP;
- chan->omtu = L2CAP_A2MP_DEFAULT_MTU;
- chan->imtu = L2CAP_A2MP_DEFAULT_MTU;
+ case L2CAP_CHAN_FIXED:
+ /* Caller will set CID and CID specific MTU values */
break;
default:
@@ -571,7 +568,7 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
chan->conn = NULL;
- if (chan->chan_type != L2CAP_CHAN_CONN_FIX_A2MP)
+ if (chan->scid != L2CAP_CID_A2MP)
hci_conn_drop(conn->hcon);
if (mgr && mgr->bredr_chan == chan)
@@ -1310,7 +1307,7 @@ static void l2cap_send_disconn_req(struct l2cap_chan *chan, int err)
__clear_ack_timer(chan);
}
- if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP) {
+ if (chan->scid == L2CAP_CID_A2MP) {
l2cap_state_change(chan, BT_DISCONN);
return;
}
@@ -1508,7 +1505,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
l2cap_chan_lock(chan);
- if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP) {
+ if (chan->scid == L2CAP_CID_A2MP) {
l2cap_chan_unlock(chan);
continue;
}
@@ -7245,7 +7242,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
BT_DBG("chan %p scid 0x%4.4x state %s", chan, chan->scid,
state_to_string(chan->state));
- if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP) {
+ if (chan->scid == L2CAP_CID_A2MP) {
l2cap_chan_unlock(chan);
continue;
}
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH 1/2] Bluetooth: Rename L2CAP_CHAN_CONN_FIX_A2MP to L2CAP_CHAN_FIXED_CID
From: Johan Hedberg @ 2014-01-24 8:29 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development
In-Reply-To: <9D839443-6517-49AA-81E4-F4D744F82635@holtmann.org>
Hi Marcel,
On Thu, Jan 23, 2014, Marcel Holtmann wrote:
> > There's no reason why A2DP should need or deserve its on channel type.
>
> I assume you mean A2MP here?
>
> > Instead we should be able to group all fixed CID users under a single
> > channel type and reuse as much code as possible for them. Where CID
> > specific exceptions are needed the chan-scid value can be used.
> >
> > This patch renames the current A2DP channel type to a generic one and
>
> Same here.
Yep. Seems like typing A2DP is somehow hardwired into my brain after so
many years.
> > -#define L2CAP_CHAN_CONN_FIX_A2MP 4
> > +#define L2CAP_CHAN_FIXED_CID 4
>
> You want to use FIXED_CID instead of just FIXED. I am fine with
> FIXED_CID, just double checking here.
I was thinking of it as "channel with a fixed CID", however now that I
re-read the Core Spec it talks about "Fixed Channels", so I'll change
this simply to CHAN_FIXED.
Johan
^ permalink raw reply
* Paired LE HoG devices are unable to reconnect to BlueZ host when discovery is active
From: Petri Gynther @ 2014-01-24 5:21 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org development
With BlueZ 5.13, I've noticed that previously paired LE HoG devices
are unable to reconnect to the host when device discovery is active on
the BT interface.
Steps to reproduce:
1. Pair LE HoG device with BlueZ 5.13 host.
2. Verify that keypresses from HoG device are delivered to uHID input pipeline.
3. Let HoG device disconnect from host (due to inactivity timeout).
4. Press key on HoG device. At this point, it reconnects to host fine.
5. Let HoG device disconnect from host again.
6. Start "test-discovery" script.
7. Press key on HoG device => it does not reconnect to host.
8. Kill "test-discovery" script.
9. Press key on HoG device => it now reconnects to host successfully.
It looks to me that the root cause is in src/adapter.c:update_found_devices()
/*
* Only if at least one client has requested discovery, maintain
* list of found devices and name confirming for legacy devices.
* Otherwise, this is an event from passive discovery and we
* should check if the device needs connecting to.
*/
if (!adapter->discovery_list)
goto connect_le;
if (g_slist_find(adapter->discovery_found, dev))
return;
When (active) discovery is on, adapter->discovery_list != NULL, and
g_slist_find(adapter->discovery_found, dev) will obviously find this
device since it is already paired. So, we end up returning from the
function without an attempt to reconnect to the previously paired LE
device.
What would be the right fix here? Stop discovery before attempting to
reconnect to the LE device?
^ permalink raw reply
* Re: [PATCH 2/2] Bluetooth: Switch ATT channels to use L2CAP_CHAN_FIXED_CID
From: Marcel Holtmann @ 2014-01-24 0:52 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <1390483368-21321-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> ATT channels are not connection oriented so having them use
> L2CAP_CHAN_CONN_ORIENTED is quite confusing. Instead, use the new
> L2CAP_CHAN_FIXED_CID type and ensure that the MTU and CID values get
> properly set.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/l2cap_core.c | 21 +++++++++------------
> net/bluetooth/l2cap_sock.c | 6 ++++++
> 2 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 84714541b48c..c5c47667bfe0 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -498,18 +498,10 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
>
> switch (chan->chan_type) {
> case L2CAP_CHAN_CONN_ORIENTED:
> - if (conn->hcon->type == LE_LINK) {
> - if (chan->dcid == L2CAP_CID_ATT) {
> - chan->omtu = L2CAP_DEFAULT_MTU;
> - chan->scid = L2CAP_CID_ATT;
> - } else {
> - chan->scid = l2cap_alloc_cid(conn);
> - }
> - } else {
> - /* Alloc CID for connection-oriented socket */
> - chan->scid = l2cap_alloc_cid(conn);
> + /* Alloc CID for connection-oriented socket */
> + chan->scid = l2cap_alloc_cid(conn);
> + if (conn->hcon->type == ACL_LINK)
> chan->omtu = L2CAP_DEFAULT_MTU;
> - }
> break;
>
> case L2CAP_CHAN_CONN_LESS:
> @@ -7025,7 +7017,12 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
> goto done;
> }
>
> - if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED && !(psm || cid)) {
> + if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED && !psm) {
> + err = -EINVAL;
> + goto done;
> + }
> +
> + if (chan->chan_type == L2CAP_CHAN_FIXED_CID && !cid) {
> err = -EINVAL;
> goto done;
> }
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 4aa6704f0b33..45064d130308 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -101,6 +101,12 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> if (!bdaddr_type_is_valid(la.l2_bdaddr_type))
> return -EINVAL;
>
> + if (la.l2_cid) {
> + /* Update type from the default CONN_ORIENTED */
I think that I understand your comment intention here, but we might want to make it less cryptic and actually spell out that this changed the channel type to overwrite the default.
> + chan->chan_type = L2CAP_CHAN_FIXED_CID;
> + chan->omtu = L2CAP_DEFAULT_MTU;
> + }
> +
> if (bdaddr_type_is_le(la.l2_bdaddr_type)) {
> if (!enable_lecoc && la.l2_psm)
> return -EINVAL;
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: Rename L2CAP_CHAN_CONN_FIX_A2MP to L2CAP_CHAN_FIXED_CID
From: Marcel Holtmann @ 2014-01-24 0:50 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <1390483368-21321-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> There's no reason why A2DP should need or deserve its on channel type.
I assume you mean A2MP here?
> Instead we should be able to group all fixed CID users under a single
> channel type and reuse as much code as possible for them. Where CID
> specific exceptions are needed the chan-scid value can be used.
>
> This patch renames the current A2DP channel type to a generic one and
Same here.
> thereby paves the way to allow converting ATT and SMP (and any future
> fixed channel protocols) to use the new channel type.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/l2cap.h | 2 +-
> net/bluetooth/a2mp.c | 8 ++++++--
> net/bluetooth/l2cap_core.c | 15 ++++++---------
> 3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 85cf40acc47e..eeb02f414255 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -651,7 +651,7 @@ struct l2cap_user {
> #define L2CAP_CHAN_RAW 1
> #define L2CAP_CHAN_CONN_LESS 2
> #define L2CAP_CHAN_CONN_ORIENTED 3
> -#define L2CAP_CHAN_CONN_FIX_A2MP 4
> +#define L2CAP_CHAN_FIXED_CID 4
You want to use FIXED_CID instead of just FIXED. I am fine with FIXED_CID, just double checking here.
> /* ----- L2CAP socket info ----- */
> #define l2cap_pi(sk) ((struct l2cap_pinfo *) sk)
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index efcd108822c4..677b970cede6 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -235,7 +235,7 @@ static int a2mp_discover_rsp(struct amp_mgr *mgr, struct sk_buff *skb,
> BT_DBG("chan %p state %s", chan,
> state_to_string(chan->state));
>
> - if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP)
> + if (chan->scid == L2CAP_CID_A2MP)
> continue;
>
> l2cap_chan_lock(chan);
> @@ -726,7 +726,11 @@ static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn, bool locked)
>
> BT_DBG("chan %p", chan);
>
> - chan->chan_type = L2CAP_CHAN_CONN_FIX_A2MP;
> + chan->chan_type = L2CAP_CHAN_FIXED_CID;
> + chan->scid = L2CAP_CID_A2MP;
> + chan->dcid = L2CAP_CID_A2MP;
> + chan->omtu = L2CAP_A2MP_DEFAULT_MTU;
> + chan->imtu = L2CAP_A2MP_DEFAULT_MTU;
> chan->flush_to = L2CAP_DEFAULT_FLUSH_TO;
>
> chan->ops = &a2mp_chan_ops;
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 138394ad3e51..84714541b48c 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -519,11 +519,8 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
> chan->omtu = L2CAP_DEFAULT_MTU;
> break;
>
> - case L2CAP_CHAN_CONN_FIX_A2MP:
> - chan->scid = L2CAP_CID_A2MP;
> - chan->dcid = L2CAP_CID_A2MP;
> - chan->omtu = L2CAP_A2MP_DEFAULT_MTU;
> - chan->imtu = L2CAP_A2MP_DEFAULT_MTU;
> + case L2CAP_CHAN_FIXED_CID:
> + /* Caller will set CID and CID specific MTU values */
> break;
>
> default:
> @@ -571,7 +568,7 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
>
> chan->conn = NULL;
>
> - if (chan->chan_type != L2CAP_CHAN_CONN_FIX_A2MP)
> + if (chan->scid != L2CAP_CID_A2MP)
> hci_conn_drop(conn->hcon);
>
> if (mgr && mgr->bredr_chan == chan)
> @@ -1310,7 +1307,7 @@ static void l2cap_send_disconn_req(struct l2cap_chan *chan, int err)
> __clear_ack_timer(chan);
> }
>
> - if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP) {
> + if (chan->scid == L2CAP_CID_A2MP) {
> l2cap_state_change(chan, BT_DISCONN);
> return;
> }
> @@ -1508,7 +1505,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
>
> l2cap_chan_lock(chan);
>
> - if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP) {
> + if (chan->scid == L2CAP_CID_A2MP) {
> l2cap_chan_unlock(chan);
> continue;
> }
> @@ -7245,7 +7242,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> BT_DBG("chan %p scid 0x%4.4x state %s", chan, chan->scid,
> state_to_string(chan->state));
>
> - if (chan->chan_type == L2CAP_CHAN_CONN_FIX_A2MP) {
> + if (chan->scid == L2CAP_CID_A2MP) {
> l2cap_chan_unlock(chan);
> continue;
> }
Rest look fine to me.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/9] android/ipc-tester: Add cases for SOCK msg size
From: Szymon Janc @ 2014-01-23 23:25 UTC (permalink / raw)
To: Jakub Tyszkowski; +Cc: linux-bluetooth
In-Reply-To: <1390379124-16426-1-git-send-email-jakub.tyszkowski@tieto.com>
Hi Jakub,
On Wednesday 22 January 2014 09:25:16 Jakub Tyszkowski wrote:
> Add cases testing message size verification for SOCK opcodes.
> ---
> android/ipc-tester.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/android/ipc-tester.c b/android/ipc-tester.c
> index ed0dd10..088c324 100644
> --- a/android/ipc-tester.c
> +++ b/android/ipc-tester.c
> @@ -864,5 +864,23 @@ int main(int argc, char *argv[])
> sizeof(struct hal_cmd_le_test_mode), -1,
> HAL_SERVICE_ID_BLUETOOTH);
>
> + /* check for valid data size for SOCK */
> + test_datasize_valid("SOCK Listen+", HAL_SERVICE_ID_SOCK,
> + HAL_OP_SOCK_LISTEN,
> + sizeof(struct hal_cmd_sock_listen), 1,
> + HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_SOCK);
> + test_datasize_valid("SOCK Listen-", HAL_SERVICE_ID_SOCK,
> + HAL_OP_SOCK_LISTEN,
> + sizeof(struct hal_cmd_sock_listen), -1,
> + HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_SOCK);
> + test_datasize_valid("SOCK Connect+", HAL_SERVICE_ID_SOCK,
> + HAL_OP_SOCK_CONNECT,
> + sizeof(struct hal_cmd_sock_connect), 1,
> + HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_SOCK);
> + test_datasize_valid("SOCK Connect-", HAL_SERVICE_ID_SOCK,
> + HAL_OP_SOCK_CONNECT,
> + sizeof(struct hal_cmd_sock_connect), -1,
> + HAL_SERVICE_ID_BLUETOOTH, HAL_SERVICE_ID_SOCK);
> +
> return tester_run();
> }
All patches in this set have been applied, thanks.
--
Szymon K. Janc
szymon.janc@gmail.com
^ permalink raw reply
* [PATCH v3 5/5] android/bluetooth: Rename devices list to cached_devices
From: Szymon Janc @ 2014-01-23 22:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1390515490-29538-1-git-send-email-szymon.janc@gmail.com>
This makes it clear what is the purpose of this list.
---
android/bluetooth.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 126b26a..a9e529f 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -140,7 +140,7 @@ static uint16_t option_index = MGMT_INDEX_NONE;
static struct mgmt *mgmt_if = NULL;
static GSList *bonded_devices = NULL;
-static GSList *devices = NULL;
+static GSList *cached_devices = NULL;
/* This list contains addresses which are asked for records */
static GSList *browse_reqs;
@@ -300,7 +300,7 @@ static struct device *find_device(const bdaddr_t *bdaddr)
if (l)
return l->data;
- l = g_slist_find_custom(devices, bdaddr, device_match);
+ l = g_slist_find_custom(cached_devices, bdaddr, device_match);
if (l)
return l->data;
@@ -320,24 +320,24 @@ static void cache_device(struct device *new_dev)
struct device *dev;
GSList *l;
- l = g_slist_find(devices, new_dev);
+ l = g_slist_find(cached_devices, new_dev);
if (l) {
- devices = g_slist_remove(devices, new_dev);
+ cached_devices = g_slist_remove(cached_devices, new_dev);
goto cache;
}
- if (g_slist_length(devices) < DEVICES_CACHE_MAX)
+ if (g_slist_length(cached_devices) < DEVICES_CACHE_MAX)
goto cache;
- l = g_slist_last(devices);
+ l = g_slist_last(cached_devices);
dev = l->data;
- devices = g_slist_remove(devices, dev);
+ cached_devices = g_slist_remove(cached_devices, dev);
remove_device_info(dev, CACHE_FILE);
free_device(dev);
cache:
- devices = g_slist_prepend(devices, new_dev);
+ cached_devices = g_slist_prepend(cached_devices, new_dev);
new_dev->timestamp = time(NULL);
store_device_info(new_dev, CACHE_FILE);
}
@@ -617,7 +617,7 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
}
break;
case HAL_BOND_STATE_BONDED:
- devices = g_slist_remove(devices, dev);
+ cached_devices = g_slist_remove(cached_devices, dev);
bonded_devices = g_slist_prepend(bonded_devices, dev);
remove_device_info(dev, CACHE_FILE);
store_device_info(dev, DEVICES_FILE);
@@ -1004,7 +1004,7 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
cp.state = HAL_DISCOVERY_STATE_STARTED;
} else {
g_slist_foreach(bonded_devices, clear_device_found, NULL);
- g_slist_foreach(devices, clear_device_found, NULL);
+ g_slist_foreach(cached_devices, clear_device_found, NULL);
cp.state = HAL_DISCOVERY_STATE_STOPPED;
}
@@ -1794,10 +1794,10 @@ static void load_devices_cache(void)
struct device *dev;
dev = create_device_from_info(key_file, devs[i]);
- devices = g_slist_prepend(devices, dev);
+ cached_devices = g_slist_prepend(cached_devices, dev);
}
- devices = g_slist_sort(devices, device_timestamp_cmp);
+ cached_devices = g_slist_sort(cached_devices, device_timestamp_cmp);
g_strfreev(devs);
g_key_file_free(key_file);
@@ -3213,8 +3213,8 @@ void bt_bluetooth_unregister(void)
g_slist_free_full(bonded_devices, (GDestroyNotify) free_device);
bonded_devices = NULL;
- g_slist_free_full(devices, (GDestroyNotify) free_device);
- devices = NULL;
+ g_slist_free_full(cached_devices, (GDestroyNotify) free_device);
+ cached_devices = NULL;
ipc_unregister(HAL_SERVICE_ID_CORE);
}
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 4/5] android/bluetooth: Add support for loading caches devices from storage
From: Szymon Janc @ 2014-01-23 22:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1390515490-29538-1-git-send-email-szymon.janc@gmail.com>
From: Szymon Janc <szymon.janc@tieto.com>
Info is now stored for all devices and bond state depends on file.
Based on that devices loaded from storage are put either to cache
or to bonded_devices list.
---
android/bluetooth.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 54 insertions(+), 5 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index cf2dc83..126b26a 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1679,7 +1679,8 @@ static void clear_uuids(void)
sizeof(cp), &cp, NULL, NULL, NULL);
}
-static void create_device_from_info(GKeyFile *key_file, const char *peer)
+static struct device *create_device_from_info(GKeyFile *key_file,
+ const char *peer)
{
struct device *dev;
uint8_t type;
@@ -1694,7 +1695,11 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)
str2ba(peer, &bdaddr);
dev = create_device(&bdaddr, type);
- dev->bond_state = HAL_BOND_STATE_BONDED;
+ str = g_key_file_get_string(key_file, peer, "LinkKey", NULL);
+ if (str) {
+ g_free(str);
+ dev->bond_state = HAL_BOND_STATE_BONDED;
+ }
str = g_key_file_get_string(key_file, peer, "Name", NULL);
if (str) {
@@ -1730,6 +1735,8 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)
g_strfreev(uuids);
}
+
+ return dev;
}
static struct mgmt_link_key_info *get_key_info(GKeyFile *key_file, const char *peer)
@@ -1762,6 +1769,40 @@ failed:
return info;
}
+static int device_timestamp_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct device *deva = a;
+ const struct device *devb = b;
+
+ return deva->timestamp < devb->timestamp;
+}
+
+static void load_devices_cache(void)
+{
+ GKeyFile *key_file;
+ gchar **devs;
+ gsize len = 0;
+ unsigned int i;
+
+ key_file = g_key_file_new();
+
+ g_key_file_load_from_file(key_file, CACHE_FILE, 0, NULL);
+
+ devs = g_key_file_get_groups(key_file, &len);
+
+ for (i = 0; i < len; i++) {
+ struct device *dev;
+
+ dev = create_device_from_info(key_file, devs[i]);
+ devices = g_slist_prepend(devices, dev);
+ }
+
+ devices = g_slist_sort(devices, device_timestamp_cmp);
+
+ g_strfreev(devs);
+ g_key_file_free(key_file);
+}
+
static void load_devices_info(bt_bluetooth_ready cb)
{
GKeyFile *key_file;
@@ -1778,14 +1819,21 @@ static void load_devices_info(bt_bluetooth_ready cb)
for (i = 0; i < len; i++) {
struct mgmt_link_key_info *key_info;
+ struct device *dev;
- create_device_from_info(key_file, devs[i]);
+ dev = create_device_from_info(key_file, devs[i]);
key_info = get_key_info(key_file, devs[i]);
- if (key_info)
- keys = g_slist_prepend(keys, key_info);
+ if (!key_info) {
+ error("Failed to load linkkey for %s, skipping",
+ devs[i]);
+ continue;
+ }
/* TODO ltk */
+
+ keys = g_slist_prepend(keys, key_info);
+ bonded_devices = g_slist_prepend(bonded_devices, dev);
}
load_link_keys(keys, cb);
@@ -1873,6 +1921,7 @@ static void read_info_complete(uint8_t status, uint16_t length,
clear_uuids();
load_devices_info(cb);
+ load_devices_cache();
set_io_capability();
set_device_id();
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 3/5] android/bluetooth: Add support for caching remote device info
From: Szymon Janc @ 2014-01-23 22:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1390515490-29538-1-git-send-email-szymon.janc@gmail.com>
From: Szymon Janc <szymon.janc@tieto.com>
Cache is limited to DEVICES_CACHE_MAX. Devices are sorted with
timestamp so if cache is full olderst device is removed.
---
android/bluetooth.c | 115 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 82 insertions(+), 33 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index f5ac0e4..cf2dc83 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -56,6 +56,7 @@
#define SETTINGS_FILE ANDROID_STORAGEDIR"/settings"
#define DEVICES_FILE ANDROID_STORAGEDIR"/devices"
+#define CACHE_FILE ANDROID_STORAGEDIR"/cache"
#define DEVICE_ID_SOURCE 0x0002 /* USB */
#define DEVICE_ID_VENDOR 0x1d6b /* Linux Foundation */
@@ -70,6 +71,8 @@
/* Default discoverable timeout 120sec as in Android */
#define DEFAULT_DISCOVERABLE_TIMEOUT 120
+#define DEVICES_CACHE_MAX 300
+
#define BASELEN_PROP_CHANGED (sizeof(struct hal_ev_adapter_props_changed) \
+ (sizeof(struct hal_property)))
@@ -199,7 +202,7 @@ static void load_adapter_config(void)
g_key_file_free(key_file);
}
-static void store_device_info(struct device *dev)
+static void store_device_info(struct device *dev, const char *path)
{
GKeyFile *key_file;
char addr[18];
@@ -207,22 +210,10 @@ static void store_device_info(struct device *dev)
char **uuids = NULL;
char *str;
- /* We only store bonded devices and need to modify the storage
- * if the state is either NONE or BONDED.
- */
- if (dev->bond_state != HAL_BOND_STATE_BONDED &&
- dev->bond_state != HAL_BOND_STATE_NONE)
- return;
-
ba2str(&dev->bdaddr, addr);
key_file = g_key_file_new();
- g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL);
-
- if (dev->bond_state == HAL_BOND_STATE_NONE) {
- g_key_file_remove_group(key_file, addr, NULL);
- goto done;
- }
+ g_key_file_load_from_file(key_file, path, 0, NULL);
g_key_file_set_integer(key_file, addr, "Type", dev->bdaddr_type);
@@ -264,15 +255,35 @@ static void store_device_info(struct device *dev)
g_key_file_remove_key(key_file, addr, "Services", NULL);
}
-done:
str = g_key_file_to_data(key_file, &length, NULL);
- g_file_set_contents(DEVICES_FILE, str, length, NULL);
+ g_file_set_contents(path, str, length, NULL);
g_free(str);
g_key_file_free(key_file);
g_strfreev(uuids);
}
+static void remove_device_info(struct device *dev, const char *path)
+{
+ GKeyFile *key_file;
+ gsize length = 0;
+ char addr[18];
+ char *str;
+
+ ba2str(&dev->bdaddr, addr);
+
+ key_file = g_key_file_new();
+ g_key_file_load_from_file(key_file, path, 0, NULL);
+
+ g_key_file_remove_group(key_file, addr, NULL);
+
+ str = g_key_file_to_data(key_file, &length, NULL);
+ g_file_set_contents(path, str, length, NULL);
+ g_free(str);
+
+ g_key_file_free(key_file);
+}
+
static int device_match(gconstpointer a, gconstpointer b)
{
const struct device *dev = a;
@@ -296,6 +307,41 @@ static struct device *find_device(const bdaddr_t *bdaddr)
return NULL;
}
+static void free_device(struct device *dev)
+{
+ g_free(dev->name);
+ g_free(dev->friendly_name);
+ g_slist_free_full(dev->uuids, g_free);
+ g_free(dev);
+}
+
+static void cache_device(struct device *new_dev)
+{
+ struct device *dev;
+ GSList *l;
+
+ l = g_slist_find(devices, new_dev);
+ if (l) {
+ devices = g_slist_remove(devices, new_dev);
+ goto cache;
+ }
+
+ if (g_slist_length(devices) < DEVICES_CACHE_MAX)
+ goto cache;
+
+ l = g_slist_last(devices);
+ dev = l->data;
+
+ devices = g_slist_remove(devices, dev);
+ remove_device_info(dev, CACHE_FILE);
+ free_device(dev);
+
+cache:
+ devices = g_slist_prepend(devices, new_dev);
+ new_dev->timestamp = time(NULL);
+ store_device_info(new_dev, CACHE_FILE);
+}
+
static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
{
struct device *dev;
@@ -314,19 +360,10 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
/* use address for name, will be change if one is present
* eg. in EIR or set by set_property. */
dev->name = g_strdup(addr);
- devices = g_slist_prepend(devices, dev);
return dev;
}
-static void free_device(struct device *dev)
-{
- g_free(dev->name);
- g_free(dev->friendly_name);
- g_slist_free_full(dev->uuids, g_free);
- g_free(dev);
-}
-
static struct device *get_device(const bdaddr_t *bdaddr, uint8_t type)
{
struct device *dev;
@@ -335,7 +372,11 @@ static struct device *get_device(const bdaddr_t *bdaddr, uint8_t type)
if (dev)
return dev;
- return create_device(bdaddr, type);
+ dev = create_device(bdaddr, type);
+
+ cache_device(dev);
+
+ return dev;
}
static void send_adapter_property(uint8_t type, uint16_t len, const void *val)
@@ -571,12 +612,15 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
case HAL_BOND_STATE_NONE:
if (dev->bond_state == HAL_BOND_STATE_BONDED) {
bonded_devices = g_slist_remove(bonded_devices, dev);
- devices = g_slist_prepend(devices, dev);
+ remove_device_info(dev, DEVICES_FILE);
+ cache_device(dev);
}
break;
case HAL_BOND_STATE_BONDED:
devices = g_slist_remove(devices, dev);
bonded_devices = g_slist_prepend(bonded_devices, dev);
+ remove_device_info(dev, CACHE_FILE);
+ store_device_info(dev, DEVICES_FILE);
break;
case HAL_BOND_STATE_BONDING:
default:
@@ -585,8 +629,6 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
dev->bond_state = state;
- store_device_info(dev);
-
send_bond_state_change(&dev->bdaddr, status, state);
}
@@ -627,7 +669,10 @@ static void set_device_uuids(struct device *dev, GSList *uuids)
g_slist_free_full(dev->uuids, g_free);
dev->uuids = uuids;
- store_device_info(dev);
+ if (dev->bond_state == HAL_BOND_STATE_BONDED)
+ store_device_info(dev, DEVICES_FILE);
+ else
+ store_device_info(dev, CACHE_FILE);
send_device_uuids_notif(dev);
}
@@ -1058,8 +1103,6 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
ev->status = HAL_STATUS_SUCCESS;
bdaddr2android(bdaddr, ev->bdaddr);
-
- dev->timestamp = time(NULL);
}
if (eir.class) {
@@ -1087,6 +1130,9 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
(*num_prop)++;
}
+ if (dev->bond_state != HAL_BOND_STATE_BONDED)
+ cache_device(dev);
+
if (*num_prop)
ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH, opcode, size, buf);
@@ -2860,7 +2906,10 @@ static uint8_t set_device_friendly_name(struct device *dev, const uint8_t *val,
g_free(dev->friendly_name);
dev->friendly_name = g_strndup((const char *) val, len);
- store_device_info(dev);
+ if (dev->bond_state == HAL_BOND_STATE_BONDED)
+ store_device_info(dev, DEVICES_FILE);
+ else
+ store_device_info(dev, CACHE_FILE);
send_device_property(&dev->bdaddr, HAL_PROP_DEVICE_FRIENDLY_NAME,
strlen(dev->friendly_name), dev->friendly_name);
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 2/5] android/bluetooth: Use defines for settings and devices files paths
From: Szymon Janc @ 2014-01-23 22:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1390515490-29538-1-git-send-email-szymon.janc@gmail.com>
---
android/bluetooth.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 5d222e1..f5ac0e4 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -54,6 +54,9 @@
#define DUT_MODE_FILE "/sys/kernel/debug/bluetooth/hci%u/dut_mode"
+#define SETTINGS_FILE ANDROID_STORAGEDIR"/settings"
+#define DEVICES_FILE ANDROID_STORAGEDIR"/devices"
+
#define DEVICE_ID_SOURCE 0x0002 /* USB */
#define DEVICE_ID_VENDOR 0x1d6b /* Linux Foundation */
#define DEVICE_ID_PRODUCT 0x0247 /* BlueZ for Android */
@@ -148,8 +151,7 @@ static void store_adapter_config(void)
key_file = g_key_file_new();
- g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/settings", 0,
- NULL);
+ g_key_file_load_from_file(key_file, SETTINGS_FILE, 0, NULL);
ba2str(&adapter.bdaddr, addr);
@@ -160,7 +162,7 @@ static void store_adapter_config(void)
data = g_key_file_to_data(key_file, &length, NULL);
- g_file_set_contents(ANDROID_STORAGEDIR"/settings", data, length, NULL);
+ g_file_set_contents(SETTINGS_FILE, data, length, NULL);
g_free(data);
g_key_file_free(key_file);
@@ -173,8 +175,7 @@ static void load_adapter_config(void)
char *str;
key_file = g_key_file_new();
- g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/settings", 0,
- NULL);
+ g_key_file_load_from_file(key_file, SETTINGS_FILE, 0, NULL);
str = g_key_file_get_string(key_file, "General", "Address", NULL);
if (!str) {
@@ -216,8 +217,7 @@ static void store_device_info(struct device *dev)
ba2str(&dev->bdaddr, addr);
key_file = g_key_file_new();
- g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices", 0,
- NULL);
+ g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL);
if (dev->bond_state == HAL_BOND_STATE_NONE) {
g_key_file_remove_group(key_file, addr, NULL);
@@ -266,7 +266,7 @@ static void store_device_info(struct device *dev)
done:
str = g_key_file_to_data(key_file, &length, NULL);
- g_file_set_contents(ANDROID_STORAGEDIR"/devices", str, length, NULL);
+ g_file_set_contents(DEVICES_FILE, str, length, NULL);
g_free(str);
g_key_file_free(key_file);
@@ -521,8 +521,7 @@ static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
key_file = g_key_file_new();
- if (!g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices",
- 0, NULL))
+ if (!g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL))
return;
ba2str(dst, addr);
@@ -537,7 +536,7 @@ static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
g_key_file_set_integer(key_file, addr, "LinkKeyPinLength", pin_length);
data = g_key_file_to_data(key_file, &length, NULL);
- g_file_set_contents(ANDROID_STORAGEDIR"/devices", data, length, NULL);
+ g_file_set_contents(DEVICES_FILE, data, length, NULL);
g_free(data);
g_key_file_free(key_file);
@@ -1727,8 +1726,7 @@ static void load_devices_info(bt_bluetooth_ready cb)
key_file = g_key_file_new();
- g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices", 0,
- NULL);
+ g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL);
devs = g_key_file_get_groups(key_file, &len);
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 1/5] android/bluetooth: Split devices list to devices and bonded_devices
From: Szymon Janc @ 2014-01-23 22:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1390515490-29538-1-git-send-email-szymon.janc@gmail.com>
Bonded devices are permament until unbondedn. Non-bonded devices will
be held in (size limited) cache based on timestamp property so split
list to ease separation.
---
android/bluetooth.c | 48 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 36 insertions(+), 12 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 4849dab..5d222e1 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -132,6 +132,8 @@ static const uint16_t uuid_list[] = {
static uint16_t option_index = MGMT_INDEX_NONE;
static struct mgmt *mgmt_if = NULL;
+
+static GSList *bonded_devices = NULL;
static GSList *devices = NULL;
/* This list contains addresses which are asked for records */
@@ -283,6 +285,10 @@ static struct device *find_device(const bdaddr_t *bdaddr)
{
GSList *l;
+ l = g_slist_find_custom(bonded_devices, bdaddr, device_match);
+ if (l)
+ return l->data;
+
l = g_slist_find_custom(devices, bdaddr, device_match);
if (l)
return l->data;
@@ -559,12 +565,30 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
if (!dev)
return;
- if (dev->bond_state != state) {
- dev->bond_state = state;
- send_bond_state_change(&dev->bdaddr, status, state);
+ if (dev->bond_state == state)
+ return;
- store_device_info(dev);
+ switch (state) {
+ case HAL_BOND_STATE_NONE:
+ if (dev->bond_state == HAL_BOND_STATE_BONDED) {
+ bonded_devices = g_slist_remove(bonded_devices, dev);
+ devices = g_slist_prepend(devices, dev);
+ }
+ break;
+ case HAL_BOND_STATE_BONDED:
+ devices = g_slist_remove(devices, dev);
+ bonded_devices = g_slist_prepend(bonded_devices, dev);
+ break;
+ case HAL_BOND_STATE_BONDING:
+ default:
+ break;
}
+
+ dev->bond_state = state;
+
+ store_device_info(dev);
+
+ send_bond_state_change(&dev->bdaddr, status, state);
}
static void send_device_property(const bdaddr_t *bdaddr, uint8_t type,
@@ -935,6 +959,7 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
if (adapter.discovering) {
cp.state = HAL_DISCOVERY_STATE_STARTED;
} else {
+ g_slist_foreach(bonded_devices, clear_device_found, NULL);
g_slist_foreach(devices, clear_device_found, NULL);
cp.state = HAL_DISCOVERY_STATE_STOPPED;
}
@@ -2128,18 +2153,15 @@ static uint8_t get_adapter_scan_mode(void)
static uint8_t get_adapter_bonded_devices(void)
{
- uint8_t buf[sizeof(bdaddr_t) * g_slist_length(devices)];
+ uint8_t buf[sizeof(bdaddr_t) * g_slist_length(bonded_devices)];
int i = 0;
GSList *l;
DBG("");
- for (l = devices; l; l = g_slist_next(l)) {
+ for (l = bonded_devices; l; l = g_slist_next(l)) {
struct device *dev = l->data;
- if (dev->bond_state != HAL_BOND_STATE_BONDED)
- continue;
-
bdaddr2android(&dev->bdaddr, buf + (i * sizeof(bdaddr_t)));
i++;
}
@@ -2691,11 +2713,10 @@ static void send_bonded_devices_props(void)
{
GSList *l;
- for (l = devices; l; l = g_slist_next(l)) {
+ for (l = bonded_devices; l; l = g_slist_next(l)) {
struct device *dev = l->data;
- if (dev->bond_state == HAL_BOND_STATE_BONDED)
- get_remote_device_props(dev);
+ get_remote_device_props(dev);
}
}
@@ -3093,6 +3114,9 @@ void bt_bluetooth_unregister(void)
{
DBG("");
+ g_slist_free_full(bonded_devices, (GDestroyNotify) free_device);
+ bonded_devices = NULL;
+
g_slist_free_full(devices, (GDestroyNotify) free_device);
devices = NULL;
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 0/5] Remote device cache support
From: Szymon Janc @ 2014-01-23 22:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
V3:
- bugfixes in cache handling
V2:
- keep cache in separate file
- patch 3/5 and 4/5 from V1 squashed to 3/4
V1:
- set cache limit to 300
- update timestamp of cached device
- rebased to master
- other minor fixes
Szymon Janc (5):
android/bluetooth: Split devices list to devices and bonded_devices
android/bluetooth: Use defines for settings and devices files paths
android/bluetooth: Add support for caching remote device info
android/bluetooth: Add support for loading caches devices from storage
android/bluetooth: Rename devices list to cached_devices
android/bluetooth.c | 246 ++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 183 insertions(+), 63 deletions(-)
--
1.8.5.3
^ permalink raw reply
* [PATCH BlueZ 4/4] emulator: Fix crash if socket(AF_ALG) is not support by the kernel
From: Anderson Lizardo @ 2014-01-23 21:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1390512281-26541-1-git-send-email-anderson.lizardo@openbossa.org>
On this situation, smp_stop() was being called with NULL pointer.
Crash detected by valgrind:
==7925== Invalid read of size 4
==7925== at 0x8052F18: smp_stop (smp.c:480)
==7925== by 0x8052542: bthost_stop (bthost.c:2073)
==7925== by 0x805521D: hciemu_unref (hciemu.c:372)
==7925== by 0x8058C65: test_post_teardown (android-tester.c:464)
==7925== by 0x8055DE7: tester_teardown_complete (tester.c:533)
==7925== by 0x8055501: teardown_callback (tester.c:312)
==7925== by 0x408348F: g_idle_dispatch (gmain.c:5250)
==7925== by 0x4086A75: g_main_context_dispatch (gmain.c:3065)
==7925== by 0x4086E14: g_main_context_iterate.isra.23 (gmain.c:3712)
==7925== by 0x40872FA: g_main_loop_run (gmain.c:3906)
==7925== by 0x41744D2: (below main) (libc-start.c:226)
==7925== Address 0x8 is not stack'd, malloc'd or (recently) free'd
---
emulator/bthost.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/emulator/bthost.c b/emulator/bthost.c
index c4603ae..3ff2a36 100644
--- a/emulator/bthost.c
+++ b/emulator/bthost.c
@@ -2070,6 +2070,8 @@ bool bthost_connect_rfcomm(struct bthost *bthost, uint16_t handle,
void bthost_stop(struct bthost *bthost)
{
- smp_stop(bthost->smp_data);
- bthost->smp_data = NULL;
+ if (bthost->smp_data) {
+ smp_stop(bthost->smp_data);
+ bthost->smp_data = NULL;
+ }
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 3/4] android: Trivial replacement of tabs where spaces are expected
From: Anderson Lizardo @ 2014-01-23 21:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1390512281-26541-1-git-send-email-anderson.lizardo@openbossa.org>
---
android/android-tester.c | 2 +-
android/hidhost.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/android-tester.c b/android/android-tester.c
index ba9137a..870ad8d 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3459,7 +3459,7 @@ static void setup_hidhost_connect(const void *test_data)
bthost_write_scan_enable(bthost, 0x03);
}
-static void hid_discon_cb(bt_bdaddr_t *bd_addr, bthh_connection_state_t state)
+static void hid_discon_cb(bt_bdaddr_t *bd_addr, bthh_connection_state_t state)
{
if (state == BTHH_CONN_STATE_DISCONNECTED)
tester_test_passed();
diff --git a/android/hidhost.c b/android/hidhost.c
index c01c563..fd70a1c 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -375,7 +375,7 @@ static void bt_hid_notify_get_report(struct hid_device *dev, uint8_t *buf,
if (!((buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_INPUT)) ||
(buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_OUTPUT)) ||
- (buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_FEATURE)))) {
+ (buf[0] == (HID_MSG_DATA | HID_DATA_TYPE_FEATURE)))) {
ev = g_malloc0(ev_len);
ev->status = buf[0];
bdaddr2android(&dev->dst, ev->bdaddr);
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 2/4] android: Remove useless extra parenthesis
From: Anderson Lizardo @ 2014-01-23 21:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1390512281-26541-1-git-send-email-anderson.lizardo@openbossa.org>
---
android/android-tester.c | 4 ++--
android/bluetooth.c | 2 +-
android/ipc-tester.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/android/android-tester.c b/android/android-tester.c
index e4f95ce..ba9137a 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -518,7 +518,7 @@ static void emulator(int pipe, int hci_index)
memset(buf, 0, sizeof(buf));
len = read(fd, buf, sizeof(buf));
- if (len <= 0 || (strcmp(buf, "bluetooth.start=daemon")))
+ if (len <= 0 || strcmp(buf, "bluetooth.start=daemon"))
goto failed;
close(pipe);
@@ -1985,7 +1985,7 @@ static bool setup(struct test_data *data)
data->bluetoothd_pid = pid;
len = read(signal_fd[0], buf, sizeof(buf));
- if (len <= 0 || (strcmp(buf, EMULATOR_SIGNAL))) {
+ if (len <= 0 || strcmp(buf, EMULATOR_SIGNAL)) {
close(signal_fd[0]);
return false;
}
diff --git a/android/bluetooth.c b/android/bluetooth.c
index 4849dab..e2bf668 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -68,7 +68,7 @@
#define DEFAULT_DISCOVERABLE_TIMEOUT 120
#define BASELEN_PROP_CHANGED (sizeof(struct hal_ev_adapter_props_changed) \
- + (sizeof(struct hal_property)))
+ + sizeof(struct hal_property))
#define BASELEN_REMOTE_DEV_PROP (sizeof(struct hal_ev_remote_device_props) \
+ sizeof(struct hal_property))
diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index ed0dd10..8d3e44d 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -264,7 +264,7 @@ static void emulator(int pipe, int hci_index)
memset(buf, 0, sizeof(buf));
len = read(fd, buf, sizeof(buf));
- if (len <= 0 || (strcmp(buf, "ctl.start=bluetoothd")))
+ if (len <= 0 || strcmp(buf, "ctl.start=bluetoothd"))
goto failed;
close(pipe);
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 1/4] android/tester: Fix crash on failure inside setup()
From: Anderson Lizardo @ 2014-01-23 21:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1390512281-26541-1-git-send-email-anderson.lizardo@openbossa.org>
The various setup_* functions were still continuing even though setup()
failed and did not initialize data->if_bluetooth properly.
Also do a little refactoring by moving tester_setup_failed() calls to
the setup() callers, so they stay close to the other failure points and
not hidden deep into a helper function.
Crash detected by Valgrind:
==4959== Invalid read of size 4
==4959== at 0x805967A: setup_base (android-tester.c:2029)
==4959== by 0x8055541: setup_callback (tester.c:373)
==4959== by 0x408348F: g_idle_dispatch (gmain.c:5250)
==4959== by 0x4086A75: g_main_context_dispatch (gmain.c:3065)
==4959== by 0x4086E14: g_main_context_iterate.isra.23 (gmain.c:3712)
==4959== by 0x40872FA: g_main_loop_run (gmain.c:3906)
==4959== by 0x41744D2: (below main) (libc-start.c:226)
==4959== Address 0x4 is not stack'd, malloc'd or (recently) free'd
---
android/android-tester.c | 85 +++++++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 39 deletions(-)
diff --git a/android/android-tester.c b/android/android-tester.c
index aa953bf..e4f95ce 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -1951,8 +1951,7 @@ static bt_callbacks_t bt_callbacks = {
.le_test_mode_cb = NULL
};
-
-static void setup(struct test_data *data)
+static bool setup(struct test_data *data)
{
const hw_module_t *module;
hw_device_t *device;
@@ -1962,18 +1961,15 @@ static void setup(struct test_data *data)
int len;
int err;
- if (pipe(signal_fd)) {
- tester_setup_failed();
- return;
- }
+ if (pipe(signal_fd))
+ return false;
pid = fork();
if (pid < 0) {
close(signal_fd[0]);
close(signal_fd[1]);
- tester_setup_failed();
- return;
+ return false;
}
if (pid == 0) {
@@ -1991,32 +1987,27 @@ static void setup(struct test_data *data)
len = read(signal_fd[0], buf, sizeof(buf));
if (len <= 0 || (strcmp(buf, EMULATOR_SIGNAL))) {
close(signal_fd[0]);
- tester_setup_failed();
- return;
+ return false;
}
close(signal_fd[0]);
err = hw_get_module(BT_HARDWARE_MODULE_ID, &module);
- if (err) {
- tester_setup_failed();
- return;
- }
+ if (err)
+ return false;
err = module->methods->open(module, BT_HARDWARE_MODULE_ID, &device);
- if (err) {
- tester_setup_failed();
- return;
- }
+ if (err)
+ return false;
data->device = device;
data->if_bluetooth = ((bluetooth_device_t *)
device)->get_bluetooth_interface();
- if (!data->if_bluetooth) {
- tester_setup_failed();
- return;
- }
+ if (!data->if_bluetooth)
+ return false;
+
+ return true;
}
static void setup_base(const void *test_data)
@@ -2024,7 +2015,10 @@ static void setup_base(const void *test_data)
struct test_data *data = tester_get_data();
bt_status_t status;
- setup(data);
+ if (!setup(data)) {
+ tester_setup_failed();
+ return;
+ }
status = data->if_bluetooth->init(&bt_callbacks);
if (status != BT_STATUS_SUCCESS) {
@@ -2040,7 +2034,10 @@ static void setup_enabled_adapter(const void *test_data)
struct test_data *data = tester_get_data();
bt_status_t status;
- setup(data);
+ if (!setup(data)) {
+ tester_setup_failed();
+ return;
+ }
status = data->if_bluetooth->init(&bt_callbacks);
if (status != BT_STATUS_SUCCESS) {
@@ -2786,7 +2783,10 @@ static void setup_socket_interface(const void *test_data)
bt_status_t status;
const void *sock;
- setup(data);
+ if (!setup(data)) {
+ tester_setup_failed();
+ return;
+ }
status = data->if_bluetooth->init(&bt_socket_callbacks);
if (status != BT_STATUS_SUCCESS) {
@@ -2812,7 +2812,10 @@ static void setup_socket_interface_enabled(const void *test_data)
bt_status_t status;
const void *sock;
- setup(data);
+ if (!setup(data)) {
+ tester_setup_failed();
+ return;
+ }
status = data->if_bluetooth->init(&bt_socket_callbacks);
if (status != BT_STATUS_SUCCESS) {
@@ -3158,41 +3161,42 @@ static bthh_callbacks_t bthh_callbacks = {
.virtual_unplug_cb = hidhost_virual_unplug_cb
};
-static void setup_hidhost(const void *test_data)
+static bool setup_hidhost(const void *test_data)
{
struct test_data *data = tester_get_data();
bt_status_t status;
const void *hid;
- setup(data);
+ if (!setup(data))
+ return false;
status = data->if_bluetooth->init(&bt_callbacks);
if (status != BT_STATUS_SUCCESS) {
data->if_bluetooth = NULL;
- tester_setup_failed();
- return;
+ return false;
}
hid = data->if_bluetooth->get_profile_interface(BT_PROFILE_HIDHOST_ID);
- if (!hid) {
- tester_setup_failed();
- return;
- }
+ if (!hid)
+ return false;
data->if_hid = hid;
status = data->if_hid->init(&bthh_callbacks);
if (status != BT_STATUS_SUCCESS) {
data->if_hid = NULL;
- tester_setup_failed();
- return;
+ return false;
}
+
+ return true;
}
static void setup_hidhost_interface(const void *test_data)
{
- setup_hidhost(test_data);
- tester_setup_complete();
+ if (setup_hidhost(test_data))
+ tester_setup_complete();
+ else
+ tester_setup_failed();
}
#define HID_GET_REPORT_PROTOCOL 0x60
@@ -3437,7 +3441,10 @@ static void setup_hidhost_connect(const void *test_data)
struct test_data *data = tester_get_data();
struct bthost *bthost;
- setup_hidhost(test_data);
+ if (!setup_hidhost(test_data)) {
+ tester_setup_failed();
+ return;
+ }
bthost = hciemu_client_get_host(data->hciemu);
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 0/4] android: Minor fixes
From: Anderson Lizardo @ 2014-01-23 21:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Hi,
This patch set contains 2 patches fixing problems found while trying to run
android-tester on a system with some features disabled on the kernel. Two other
patches are simple cleanups.
Best Regards
Anderson Lizardo
Anderson Lizardo (4):
android/tester: Fix crash on failure inside setup()
android: Remove useless extra parenthesis
android: Trivial replacement of tabs where spaces are expected
emulator: Fix crash if socket(AF_ALG) is not support by the kernel
android/android-tester.c | 91 +++++++++++++++++++++++++---------------------
android/bluetooth.c | 2 +-
android/hidhost.c | 2 +-
android/ipc-tester.c | 2 +-
emulator/bthost.c | 6 ++-
5 files changed, 56 insertions(+), 47 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH] android/audio: Fix for loading audio lib
From: Luiz Augusto von Dentz @ 2014-01-23 18:09 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth@vger.kernel.org, Szymon Janc
In-Reply-To: <1390493791-23043-1-git-send-email-lukasz.rymanowski@tieto.com>
Hi Lukasz,
On Thu, Jan 23, 2014 at 6:16 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> Issue visible when haltest loads audio.a2dp.default.so
>
>> HAL E: load:
>> module=/home/rymanluk/devel/kitkat/external/bluetooth/bluez/android/.libs/audio.a2dp.default.so
> /home/rymanluk/devel/kitkat/external/bluetooth/bluez/android/.libs/audio.a2dp.default.so:
> undefined symbol: clock_gettime
> hw_get_module_by_class returned -22
>
> ---
> android/Makefile.am | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/android/Makefile.am b/android/Makefile.am
> index e9d9db1..f85de20 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -152,7 +152,7 @@ android_audio_a2dp_default_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/android
> android_audio_a2dp_default_la_LIBADD = @SBC_LIBS@
>
> android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
> - -no-undefined -pthread
> + -no-undefined -pthread -lrt
>
> endif
>
> --
> 1.8.4
Applied, just had to fix a little but the description to state why
-lrt is needed.
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v2 1/2] android/hal-audio: Simplify and fix locking
From: Luiz Augusto von Dentz @ 2014-01-23 18:08 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1390490548-19697-1-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Thu, Jan 23, 2014 at 5:22 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> This fix various issues with locking like missing unlock on
> audio_ipc_cmd() return or accesing audio_sk without holding lock.
> close_thread is removed to simplify code and shutdown on listen_sk is
> used to indicate that that handler thread should stop.
> ---
> v2: fix missing unregister_endpoints() on audio_close
>
> android/hal-audio.c | 70 +++++++++++++++++++++++++++++------------------------
> 1 file changed, 38 insertions(+), 32 deletions(-)
>
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index 4326ccd..52f8894 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -44,10 +44,8 @@ static const uint8_t a2dp_src_uuid[] = {
>
> static int listen_sk = -1;
> static int audio_sk = -1;
> -static bool close_thread = false;
>
> static pthread_t ipc_th = 0;
> -static pthread_mutex_t close_mutex = PTHREAD_MUTEX_INITIALIZER;
> static pthread_mutex_t sk_mutex = PTHREAD_MUTEX_INITIALIZER;
>
> #if __BYTE_ORDER == __LITTLE_ENDIAN
> @@ -487,14 +485,6 @@ static ssize_t sbc_write_data(void *codec_data, const void *buffer,
> return bytes;
> }
>
> -static void audio_ipc_cleanup(void)
> -{
> - if (audio_sk >= 0) {
> - shutdown(audio_sk, SHUT_RDWR);
> - audio_sk = -1;
> - }
> -}
> -
> static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
> void *param, size_t *rsp_len, void *rsp, int *fd)
> {
> @@ -506,6 +496,8 @@ static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
> struct hal_status s;
> size_t s_len = sizeof(s);
>
> + pthread_mutex_lock(&sk_mutex);
> +
> if (audio_sk < 0) {
> error("audio: Invalid cmd socket passed to audio_ipc_cmd");
> goto failed;
> @@ -533,12 +525,9 @@ static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
> msg.msg_iov = iv;
> msg.msg_iovlen = 2;
>
> - pthread_mutex_lock(&sk_mutex);
> -
> ret = sendmsg(audio_sk, &msg, 0);
> if (ret < 0) {
> error("audio: Sending command failed:%s", strerror(errno));
> - pthread_mutex_unlock(&sk_mutex);
> goto failed;
> }
>
> @@ -570,12 +559,9 @@ static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
> if (ret < 0) {
> error("audio: Receiving command response failed:%s",
> strerror(errno));
> - pthread_mutex_unlock(&sk_mutex);
> goto failed;
> }
>
> - pthread_mutex_unlock(&sk_mutex);
> -
> if (ret < (ssize_t) sizeof(cmd)) {
> error("audio: Too small response received(%zd bytes)", ret);
> goto failed;
> @@ -611,9 +597,13 @@ static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
> goto failed;
> }
>
> + pthread_mutex_unlock(&sk_mutex);
> +
> return s->code;
> }
>
> + pthread_mutex_unlock(&sk_mutex);
> +
> /* Receive auxiliary data in msg */
> if (fd) {
> struct cmsghdr *cmsg;
> @@ -638,7 +628,8 @@ static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
> failed:
> /* Some serious issue happen on IPC - recover */
> shutdown(audio_sk, SHUT_RDWR);
> - audio_sk = -1;
> + pthread_mutex_unlock(&sk_mutex);
> +
> return AUDIO_STATUS_FAILED;
> }
>
> @@ -1311,10 +1302,10 @@ static int audio_close(hw_device_t *device)
>
> DBG("");
>
> - pthread_mutex_lock(&close_mutex);
> - audio_ipc_cleanup();
> - close_thread = true;
> - pthread_mutex_unlock(&close_mutex);
> + unregister_endpoints();
> +
> + shutdown(listen_sk, SHUT_RDWR);
> + shutdown(audio_sk, SHUT_RDWR);
>
> pthread_join(ipc_th, NULL);
>
> @@ -1329,19 +1320,31 @@ static void *ipc_handler(void *data)
> {
> bool done = false;
> struct pollfd pfd;
> + int sk;
>
> DBG("");
>
> while (!done) {
> DBG("Waiting for connection ...");
> - audio_sk = accept(listen_sk, NULL, NULL);
> - if (audio_sk < 0) {
> +
> + sk = accept(listen_sk, NULL, NULL);
> + if (sk < 0) {
> int err = errno;
> - error("audio: Failed to accept socket: %d (%s)", err,
> - strerror(err));
> - continue;
> +
> + if (err == EINTR)
> + continue;
> +
> + if (err != ECONNABORTED && err != EINVAL)
> + error("audio: Failed to accept socket: %d (%s)",
> + err, strerror(err));
> +
> + break;
> }
>
> + pthread_mutex_lock(&sk_mutex);
> + audio_sk = sk;
> + pthread_mutex_unlock(&sk_mutex);
> +
> DBG("Audio IPC: Connected");
>
> if (register_endpoints() != AUDIO_STATUS_SUCCESS) {
> @@ -1349,7 +1352,12 @@ static void *ipc_handler(void *data)
>
> unregister_endpoints();
>
> + pthread_mutex_lock(&sk_mutex);
> shutdown(audio_sk, SHUT_RDWR);
> + close(audio_sk);
> + audio_sk = -1;
> + pthread_mutex_unlock(&sk_mutex);
> +
> continue;
> }
>
> @@ -1362,14 +1370,12 @@ static void *ipc_handler(void *data)
>
> if (pfd.revents & (POLLHUP | POLLERR | POLLNVAL)) {
> info("Audio HAL: Socket closed");
> +
> + pthread_mutex_lock(&sk_mutex);
> + close(audio_sk);
> audio_sk = -1;
> + pthread_mutex_unlock(&sk_mutex);
> }
> -
> - /*Check if audio_dev is closed */
> - pthread_mutex_lock(&close_mutex);
> - done = close_thread;
> - close_thread = false;
> - pthread_mutex_unlock(&close_mutex);
> }
>
> unregister_endpoints();
> --
> 1.8.3.2
Pushed, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply
* [RFC] android: Add support for Valgrind in debug variants
From: Andrzej Kaczmarek @ 2014-01-23 17:54 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1390499644-8027-1-git-send-email-andrzej.kaczmarek@tieto.com>
This patch automatically builds BlueZ in a way it can be easily run
with Valgrind which is available in AOSP tree.
For userdebug and eng variant, bluetoothd will have additional
dependency to necessary Valgrind modules and bluetoothd binary will not
be stripped. Special version of init.bluetooth.rc is also provided
which defines bluetoothd service to be run using Valgrind with proper
environment and it will be installed automatically as well.
---
android/Android.mk | 13 +++++++++++++
android/init.bluetooth.rc-debug | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
create mode 100644 android/init.bluetooth.rc-debug
diff --git a/android/Android.mk b/android/Android.mk
index c274295..55093d2 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -80,6 +80,15 @@ $(foreach file,$(lib_headers), $(shell ln -sf ../$(file) $(LOCAL_PATH)/bluez/lib
LOCAL_MODULE_TAGS := optional
LOCAL_MODULE := bluetoothd
+ifneq (,$(filter userdebug eng,$(TARGET_BUILD_VARIANT)))
+LOCAL_STRIP_MODULE := false
+LOCAL_REQUIRED_MODULES := valgrind \
+ memcheck-$(TARGET_ARCH)-linux \
+ vgpreload_core-$(TARGET_ARCH)-linux \
+ vgpreload_memcheck-$(TARGET_ARCH)-linux \
+ default.supp
+endif
+
include $(BUILD_EXECUTABLE)
#
@@ -294,7 +303,11 @@ include $(CLEAR_VARS)
LOCAL_MODULE := init.bluetooth.rc
LOCAL_MODULE_CLASS := ETC
+ifneq (,$(filter userdebug eng,$(TARGET_BUILD_VARIANT)))
+LOCAL_SRC_FILES := bluez/android/$(LOCAL_MODULE)-debug
+else
LOCAL_SRC_FILES := bluez/android/$(LOCAL_MODULE)
+endif
LOCAL_MODULE_TAGS := optional
LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
diff --git a/android/init.bluetooth.rc-debug b/android/init.bluetooth.rc-debug
new file mode 100644
index 0000000..c6c382c
--- /dev/null
+++ b/android/init.bluetooth.rc-debug
@@ -0,0 +1,39 @@
+# required permissions
+on boot
+ chown bluetooth bluetooth /data/misc/bluetooth
+ chown bluetooth bluetooth /dev/uhid
+
+# services
+on property:bluetooth.start=daemon
+ setprop bluetooth.start none
+ start bluetoothd
+
+on property:bluetooth.stop=daemon
+ setprop bluetooth.stop none
+ stop bluetoothd
+
+on property:bluetooth.start=snoop
+ setprop bluetooth.start none
+ start bluetoothd-snoop
+
+on property:bluetooth.stop=snoop
+ setprop bluetooth.stop none
+ stop bluetoothd-snoop
+
+service bluetoothd /system/bin/logwrapper /system/bin/valgrind --leak-check=full /system/bin/bluetoothd
+ setenv G_SLICE always-malloc
+ setenv G_DEBUG gc-friendly
+ class main
+ # init does not yet support setting capabilities so run as root,
+ # bluetoothd drop uid to bluetooth with the right linux capabilities
+ group bluetooth
+ disabled
+ oneshot
+
+service bluetoothd-snoop /system/bin/logwrapper /system/bin/bluetoothd-snoop
+ class main
+ # init does not yet support setting capabilities so run as root,
+ # bluetoothd-snoop drops unneeded linux capabilities
+ group nobody
+ disabled
+ oneshot
--
1.8.5.2
^ permalink raw reply related
* [RFC] Valgrind on Android
From: Andrzej Kaczmarek @ 2014-01-23 17:54 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Andrzej Kaczmarek
Hi,
This is patch I've been using for some time now to build BlueZ packages
in a way that bluetooth starts automatically via Valgrind, so no need
to hack this manually.
It's enabled automatically for userdebug and eng build which may be not
exactly what everybody wants, so other option could be to rely on some
fancy variable (VALGRIND_FOR_BLUEZ or sth) so those who want it enabled
may just define this in environment.
Andrzej Kaczmarek (1):
android: Add support for Valgrind in debug variants
android/Android.mk | 13 +++++++++++++
android/init.bluetooth.rc-debug | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
create mode 100644 android/init.bluetooth.rc-debug
--
1.8.5.2
^ permalink raw reply
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