Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCHv6 02/14] shared/gatt: Add initial implementation of discover_included_services
From: Marcin Kraglak @ 2014-10-23 10:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1414059337-12040-1-git-send-email-marcin.kraglak@tieto.com>

Current implementation allow to discover included services with
16 bit UUID only.
---
 src/shared/gatt-helpers.c | 118 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 4dc787f..4cc483a 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -692,6 +692,92 @@ bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
 								destroy, false);
 }
 
+static void discover_included_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct bt_gatt_result *final_result = NULL;
+	struct discovery_op *op = user_data;
+	struct bt_gatt_result *cur_result;
+	uint8_t att_ecode = 0;
+	uint16_t last_handle;
+	size_t data_length;
+	bool success;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		att_ecode = process_error(pdu, length);
+
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+							op->result_head)
+			goto done;
+
+		success = false;
+		goto failed;
+	}
+
+	if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 6) {
+		success = false;
+		goto failed;
+	}
+
+	data_length = ((const uint8_t *) pdu)[0];
+
+	/*
+	 * Check if PDU contains data sets with length declared in the beginning
+	 * of frame and if this length is correct.
+	 * Data set length may be 6 or 8 octets:
+	 * 2 octets - include service handle
+	 * 2 octets - start handle of included service
+	 * 2 octets - end handle of included service
+	 * optional 2 octets - Bluetooth UUID of included service
+	 */
+	if (data_length != 8 || (length - 1) % data_length) {
+		success = false;
+		goto failed;
+	}
+
+	cur_result = result_create(opcode, pdu + 1, length - 1,
+							data_length, op);
+	if (!cur_result) {
+		success = false;
+		goto failed;
+	}
+
+	if (!op->result_head) {
+		op->result_head = op->result_tail = cur_result;
+	} else {
+		op->result_tail->next = cur_result;
+		op->result_tail = cur_result;
+	}
+
+	last_handle = get_le16(pdu + length - data_length);
+	if (last_handle != op->end_handle) {
+		uint8_t pdu[6];
+
+		put_le16(last_handle + 1, pdu);
+		put_le16(op->end_handle, pdu + 2);
+		put_le16(GATT_INCLUDE_UUID, pdu + 4);
+
+		if (bt_att_send(op->att, BT_ATT_OP_READ_BY_TYPE_REQ,
+							pdu, sizeof(pdu),
+							discover_included_cb,
+							discovery_op_ref(op),
+							discovery_op_unref))
+			return;
+
+		discovery_op_unref(op);
+		success = false;
+		goto failed;
+	}
+
+done:
+	success = true;
+	final_result = op->result_head;
+
+failed:
+	if (op->callback)
+		op->callback(success, att_ecode, final_result, op->user_data);
+}
+
 bool bt_gatt_discover_included_services(struct bt_att *att,
 					uint16_t start, uint16_t end,
 					bt_uuid_t *uuid,
@@ -699,8 +785,36 @@ bool bt_gatt_discover_included_services(struct bt_att *att,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct discovery_op *op;
+	uint8_t pdu[6];
+
+	if (!att)
+		return false;
+
+	op = new0(struct discovery_op, 1);
+	if (!op)
+		return false;
+
+	op->att = att;
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+	op->end_handle = end;
+
+	put_le16(start, pdu);
+	put_le16(end, pdu + 2);
+	put_le16(GATT_INCLUDE_UUID, pdu + 4);
+
+	if (!bt_att_send(att, BT_ATT_OP_READ_BY_TYPE_REQ,
+					pdu, sizeof(pdu),
+					discover_included_cb,
+					discovery_op_ref(op),
+					discovery_op_unref)) {
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 static void discover_chrcs_cb(uint8_t opcode, const void *pdu,
-- 
1.9.3


^ permalink raw reply related

* [PATCHv6 01/14] shared/gatt: Add discover_secondary_services()
From: Marcin Kraglak @ 2014-10-23 10:15 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1414059337-12040-1-git-send-email-marcin.kraglak@tieto.com>

This pacth implements searching Secondary Services in given range.
---
 src/shared/gatt-helpers.c | 56 +++++++++++++++++++++++++++++++++--------------
 src/shared/gatt-helpers.h |  5 +++++
 2 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 55e6992..4dc787f 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -175,6 +175,7 @@ struct discovery_op {
 	uint16_t end_handle;
 	int ref_count;
 	bt_uuid_t uuid;
+	uint16_t service_type;
 	struct bt_gatt_result *result_head;
 	struct bt_gatt_result *result_tail;
 	bt_gatt_discovery_callback_t callback;
@@ -487,7 +488,7 @@ static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
 
 		put_le16(last_end + 1, pdu);
 		put_le16(op->end_handle, pdu + 2);
-		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+		put_le16(op->service_type, pdu + 4);
 
 		if (bt_att_send(op->att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
 							pdu, sizeof(pdu),
@@ -569,7 +570,7 @@ static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
 
 		put_le16(last_end + 1, pdu);
 		put_le16(op->end_handle, pdu + 2);
-		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+		put_le16(op->service_type, pdu + 4);
 		put_uuid_le(&op->uuid, pdu + 6);
 
 		if (bt_att_send(op->att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
@@ -594,21 +595,12 @@ done:
 		op->callback(success, att_ecode, final_result, op->user_data);
 }
 
-bool bt_gatt_discover_all_primary_services(struct bt_att *att, bt_uuid_t *uuid,
-					bt_gatt_discovery_callback_t callback,
-					void *user_data,
-					bt_gatt_destroy_func_t destroy)
-{
-	return bt_gatt_discover_primary_services(att, uuid, 0x0001, 0xffff,
-							callback, user_data,
-							destroy);
-}
-
-bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
+static bool discover_services(struct bt_att *att, bt_uuid_t *uuid,
 					uint16_t start, uint16_t end,
 					bt_gatt_discovery_callback_t callback,
 					void *user_data,
-					bt_gatt_destroy_func_t destroy)
+					bt_gatt_destroy_func_t destroy,
+					bool primary)
 {
 	struct discovery_op *op;
 	bool result;
@@ -625,6 +617,8 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
 	op->callback = callback;
 	op->user_data = user_data;
 	op->destroy = destroy;
+	/* set service uuid to primary or secondary */
+	op->service_type = primary ? GATT_PRIM_SVC_UUID : GATT_SND_SVC_UUID;
 
 	/* If UUID is NULL, then discover all primary services */
 	if (!uuid) {
@@ -632,7 +626,7 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
 
 		put_le16(start, pdu);
 		put_le16(end, pdu + 2);
-		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+		put_le16(op->service_type, pdu + 4);
 
 		result = bt_att_send(att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
 							pdu, sizeof(pdu),
@@ -652,7 +646,7 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
 
 		put_le16(start, pdu);
 		put_le16(end, pdu + 2);
-		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+		put_le16(op->service_type, pdu + 4);
 		put_uuid_le(&op->uuid, pdu + 6);
 
 		result = bt_att_send(att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
@@ -668,6 +662,36 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
 	return result;
 }
 
+bool bt_gatt_discover_all_primary_services(struct bt_att *att, bt_uuid_t *uuid,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	return bt_gatt_discover_primary_services(att, uuid, 0x0001, 0xffff,
+							callback, user_data,
+							destroy);
+}
+
+bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
+					uint16_t start, uint16_t end,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	return discover_services(att, uuid, start, end, callback, user_data,
+								destroy, true);
+}
+
+bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
+					uint16_t start, uint16_t end,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	return discover_services(att, uuid, start, end, callback, user_data,
+								destroy, false);
+}
+
 bool bt_gatt_discover_included_services(struct bt_att *att,
 					uint16_t start, uint16_t end,
 					bt_uuid_t *uuid,
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index f6f4b62..8a25dea 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -72,6 +72,11 @@ bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
 					bt_gatt_discovery_callback_t callback,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy);
+bool bt_gatt_discover_secondary_services(struct bt_att *att, bt_uuid_t *uuid,
+					uint16_t start, uint16_t end,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
 bool bt_gatt_discover_included_services(struct bt_att *att,
 					uint16_t start, uint16_t end,
 					bt_uuid_t *uuid,
-- 
1.9.3


^ permalink raw reply related

* [PATCHv6 00/14] Included service discovery
From: Marcin Kraglak @ 2014-10-23 10:15 UTC (permalink / raw)
  To: linux-bluetooth

v3:
In this version after primary service discovery,
secondary services are discovered. Next included
services are resolved. With this approach we
don't have recursively search for included service,
like it was TODO in previous proposal.
There is also small coding style fix suggested by Arman.

v4:
If no secondary services found, continue include services search (fixed
in gatt-client.c).
Fixed wrong debug logs (primary->secondary).
Fixed searching descriptors

v5:
Ignore Unsupported Group Type Error in response to secondary service
discovery and continue included services discovery.

v6
* Added comments to specific calculations and numbers.
* Fixed goto labels as suggested by Szymon
* Changed printing type of service.
* Typo fixes pointed by Szymon

Marcin Kraglak (14):
  shared/gatt: Add discover_secondary_services()
  shared/gatt: Add initial implementation of discover_included_services
  shared/gatt: Discover included services 128 bit UUIDS
  shared/gatt: Add extra check in characteristic iterator
  shared/gatt: Add included service iterator
  shared/gatt: Add function bt_gatt_result_included_count()
  shared/gatt: Remove not needed function parameter
  shared/gatt: Distinguish Primary from Secondary services
  tools/btgatt-client: Print type of service
  shared/gatt: Discover secondary services
  shared/gatt: Discover included services
  shared/gatt: Add gatt-client include service iterator
  tools/btgatt-client: Print found include services
  shared/gatt: Fix searching descriptors

 src/shared/gatt-client.c  | 252 +++++++++++++++++++++++--
 src/shared/gatt-client.h  |  18 ++
 src/shared/gatt-helpers.c | 462 ++++++++++++++++++++++++++++++++++++++++++++--
 src/shared/gatt-helpers.h |  10 +-
 tools/btgatt-client.c     |  20 +-
 5 files changed, 725 insertions(+), 37 deletions(-)

-- 
1.9.3


^ permalink raw reply

* Re: [PATCH v3 bluetooth-next 0/4] 6lowpan: Move skb delivery out of IPHC
From: Martin Townsend @ 2014-10-23  9:51 UTC (permalink / raw)
  To: Alexander Aring, Martin Townsend
  Cc: linux-bluetooth, linux-wpan, marcel, jukka.rissanen
In-Reply-To: <20141023094355.GA11768@x61s.guest.pengutronix.de>

Hi Alex,

Looks like the SKB is now non-linear which causes the skb_put to assert.  Maybe skb_linearlize will help[0]

I'll also take a look.

- Martin.

[0] http://lxr.free-electrons.com/source/include/linux/skbuff.h#L2399


On 23/10/14 10:43, Alexander Aring wrote:
> Hi Martin,
>
> On Wed, Oct 22, 2014 at 09:39:46AM +0100, Martin Townsend wrote:
>> This series moves skb delivery out of IPHC and into the receive routines of
>> both bluetooth and 802.15.4.  The reason is that we need to support more
>> (de)compression schemes in the future.  It also means that calling 
>> lowpan_process_data now only returns error codes or 0 for success so 
>> this has been cleaned up.  The final patch then renames occurences of
>> lowpan_process_data and process_data to something more meaningful.
>>
> With this series and running fragmented 6LoWPAN packet on 802.15.4 a BUG()
> occurs.
>
> I can't bisect this issue and take a deeper look into this now. I have at
> weekend more time for this. Can you try a simple fragmented ping like "ping6
> $IP -s 127"? Do you get a similar issue?
>
> [   73.250963] ------------[ cut here ]------------
> [   73.255840] kernel BUG at net/core/skbuff.c:1275!
> [   73.260770] Internal error: Oops - BUG: 0 [#1] ARM
> [   73.265791] Modules linked in: autofs4
> [   73.269753] CPU: 0 PID: 39 Comm: kworker/u2:1 Tainted: G        W      3.17.0-rc1-118898-g89f5047 #5
> [   73.279347] Workqueue: wpan-phy0 mac802154_rx_worker
> [   73.284557] task: cf20e2c0 ti: cf2d8000 task.ti: cf2d8000
> [   73.290223] PC is at skb_put+0x18/0x50
> [   73.294153] LR is at 0x7fffff00
> [   73.297448] pc : [<c041000c>]    lr : [<7fffff00>]    psr: 20000013
> [   73.297448] sp : cf2d9c48  ip : cf50e71a  fp : cf5aba00
> [   73.309467] r10: cf2d9c84  r9 : cf2d9c83  r8 : cf4ab000
> [   73.314939] r7 : 80000000  r6 : cf5aba00  r5 : cf5aba00  r4 : cf5aba00
> [   73.321774] r3 : 00000000  r2 : c0410700  r1 : 80000000  r0 : cf5aba00
> [   73.328612] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> [   73.336267] Control: 10c5387d  Table: 8f408019  DAC: 00000015
> [   73.342283] Process kworker/u2:1 (pid: 39, stack limit = 0xcf2d8240)
> [   73.348937] Stack: (0xcf2d9c48 to 0xcf2da000)
> [   73.353510] 9c40:                   00000000 cf5aba00 cf5aba00 c0410700 cf2d9c84 cf5aba00
> [   73.362081] 9c60: cf596bd0 cf5aba00 00000000 cf4ab000 cf5aba00 c0000dc0 cf5aba00 c058bc78
> [   73.370651] 9c80: 0020e2c0 00000000 cf2d9c90 cf2d9ca0 abcdf303 000000f3 aaaaaaaa aaaaaaaa
> [   73.379222] 9ca0: abcd0003 00000000 aaaaaabb aaaaaaaa cf5aba00 cf5aba00 cf4ab000 cf5aba00
> [   73.387792] 9cc0: 00000000 0000f600 cf4ab000 c0821ba8 00000000 c058b208 00000000 c005eb18
> [   73.396363] 9ce0: c08645fc 60000013 00000000 cf2d8000 c0f8cc61 00000002 abcd0003 00000000
> [   73.404932] 9d00: aaaaaaaa aaaaaaaa abcd0003 00000000 aaaaaabb aaaaaaaa 00000000 00000000
> [   73.413503] 9d20: c0865c40 cf5abb80 c0821b68 c08646e0 c08644fc cf5abb80 00000000 c041ac5c
> [   73.422074] 9d40: 00000000 00000000 c041a86c 00000002 00000003 00000040 c0865c40 cf5abb80
> [   73.430644] 9d60: cf20e2c0 c0864510 00000001 c0865c40 cf5abb80 00000000 c0865c14 c0865bd4
> [   73.439215] 9d80: 00000040 c0865c40 c0865bc8 c041c1dc cf20e2c0 c0865bc0 0000012c 00000040
> [   73.447787] 9da0: c0865bc0 c0865bc8 ffffa76e c041bb20 c086714c ffffa76e c041ba6c 00000008
> [   73.456357] 9dc0: cf2d8020 cf2d8000 c0867140 00000100 c0867100 c086714c 00000004 c00366b0
> [   73.464927] 9de0: cf20e2c0 cf2d8038 00000003 40000003 0000000a 04208060 00000000 ffffa76d
> [   73.473497] 9e00: 20000013 60000013 aaaaaaaa cf5abb80 cf4ab520 cf4ab538 cf2d8030 00000001
> [   73.482067] 9e20: 00000000 c00368a0 00000000 c041b518 00000000 c058e14c 00000000 00000000
> [   73.490638] 9e40: c058df74 c005eb18 cf320a9c cf4ab520 cff8cc61 cf320a9c abcdf303 000000f3
> [   73.499208] 9e60: aaaaaaaa aaaaaaaa abcd0003 00000000 aaaaaabb aaaaaaaa 00000001 cf320a9c
> [   73.507780] 9e80: 00000000 cf5abb80 cf5abb80 cf320a60 cd9e1200 cf2d9ec8 cf03ea00 c058c7ec
> [   73.516351] 9ea0: cf2d4d40 cd9e1204 cf036400 c0045fc4 00000001 00000000 c0045f4c 60000093
> [   73.524921] 9ec0: 00000000 00000000 c1082b28 c0bb9980 00000000 c0765727 cf20e2c0 cf2d4d40
> [   73.533492] 9ee0: cf036400 cf036400 c0832920 cf036430 cf2d8000 cf2d4d58 00000000 c0046b64
> [   73.542063] 9f00: cf2d4d40 cf20e2c0 00000000 cf2d2400 00000000 cf2d4d40 c004688c 00000000
> [   73.550633] 9f20: 00000000 00000000 00000000 c004a0a0 c0831550 00000000 cf20e2c0 cf2d4d40
> [   73.559202] 9f40: 00000000 00000001 dead4ead ffffffff ffffffff c086769c 00000000 00000000
> [   73.567772] 9f60: c07038f4 cf2d9f64 cf2d9f64 00000000 00000001 dead4ead ffffffff ffffffff
> [   73.576342] 9f80: c086769c 00000000 00000000 c07038f4 cf2d9f90 cf2d9f90 cf2d9fac cf2d2400
> [   73.584910] 9fa0: c0049fcc 00000000 00000000 c000db88 00000000 00000000 00000000 00000000
> [   73.593479] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [   73.602048] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 fefffbae bf3baaae
> [   73.610626] [<c041000c>] (skb_put) from [<c0410700>] (skb_try_coalesce+0x6c/0x32c)
> [   73.618564] [<c0410700>] (skb_try_coalesce) from [<c058bc78>] (lowpan_frag_rcv+0x668/0x7a8)
> [   73.627318] [<c058bc78>] (lowpan_frag_rcv) from [<c058b208>] (lowpan_rcv+0xc8/0x1f0)
> [   73.635441] [<c058b208>] (lowpan_rcv) from [<c041ac5c>] (__netif_receive_skb_core+0x43c/0x56c)
> [   73.644470] [<c041ac5c>] (__netif_receive_skb_core) from [<c041c1dc>] (process_backlog+0x78/0x110)
> [   73.653860] [<c041c1dc>] (process_backlog) from [<c041bb20>] (net_rx_action+0xb4/0x18c)
> [   73.662267] [<c041bb20>] (net_rx_action) from [<c00366b0>] (__do_softirq+0x104/0x264)
> [   73.670478] [<c00366b0>] (__do_softirq) from [<c00368a0>] (do_softirq+0x44/0x6c)
> [   73.678233] [<c00368a0>] (do_softirq) from [<c041b518>] (netif_rx_ni+0x20/0x2c)
> [   73.685900] [<c041b518>] (netif_rx_ni) from [<c058e14c>] (mac802154_wpans_rx+0x208/0x260)
> [   73.694472] [<c058e14c>] (mac802154_wpans_rx) from [<c058c7ec>] (mac802154_rx_worker+0x7c/0x94)
> [   73.703594] [<c058c7ec>] (mac802154_rx_worker) from [<c0045fc4>] (process_one_work+0x23c/0x3cc)
> [   73.712712] [<c0045fc4>] (process_one_work) from [<c0046b64>] (worker_thread+0x2d8/0x438)
>
> - Alex

^ permalink raw reply

* Re: [PATCH v3 bluetooth-next 0/4] 6lowpan: Move skb delivery out of IPHC
From: Alexander Aring @ 2014-10-23  9:43 UTC (permalink / raw)
  To: Martin Townsend
  Cc: linux-bluetooth, linux-wpan, marcel, jukka.rissanen,
	Martin Townsend
In-Reply-To: <1413967190-28890-1-git-send-email-martin.townsend@xsilon.com>

Hi Martin,

On Wed, Oct 22, 2014 at 09:39:46AM +0100, Martin Townsend wrote:
> This series moves skb delivery out of IPHC and into the receive routines of
> both bluetooth and 802.15.4.  The reason is that we need to support more
> (de)compression schemes in the future.  It also means that calling 
> lowpan_process_data now only returns error codes or 0 for success so 
> this has been cleaned up.  The final patch then renames occurences of
> lowpan_process_data and process_data to something more meaningful.
> 
With this series and running fragmented 6LoWPAN packet on 802.15.4 a BUG()
occurs.

I can't bisect this issue and take a deeper look into this now. I have at
weekend more time for this. Can you try a simple fragmented ping like "ping6
$IP -s 127"? Do you get a similar issue?

[   73.250963] ------------[ cut here ]------------
[   73.255840] kernel BUG at net/core/skbuff.c:1275!
[   73.260770] Internal error: Oops - BUG: 0 [#1] ARM
[   73.265791] Modules linked in: autofs4
[   73.269753] CPU: 0 PID: 39 Comm: kworker/u2:1 Tainted: G        W      3.17.0-rc1-118898-g89f5047 #5
[   73.279347] Workqueue: wpan-phy0 mac802154_rx_worker
[   73.284557] task: cf20e2c0 ti: cf2d8000 task.ti: cf2d8000
[   73.290223] PC is at skb_put+0x18/0x50
[   73.294153] LR is at 0x7fffff00
[   73.297448] pc : [<c041000c>]    lr : [<7fffff00>]    psr: 20000013
[   73.297448] sp : cf2d9c48  ip : cf50e71a  fp : cf5aba00
[   73.309467] r10: cf2d9c84  r9 : cf2d9c83  r8 : cf4ab000
[   73.314939] r7 : 80000000  r6 : cf5aba00  r5 : cf5aba00  r4 : cf5aba00
[   73.321774] r3 : 00000000  r2 : c0410700  r1 : 80000000  r0 : cf5aba00
[   73.328612] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[   73.336267] Control: 10c5387d  Table: 8f408019  DAC: 00000015
[   73.342283] Process kworker/u2:1 (pid: 39, stack limit = 0xcf2d8240)
[   73.348937] Stack: (0xcf2d9c48 to 0xcf2da000)
[   73.353510] 9c40:                   00000000 cf5aba00 cf5aba00 c0410700 cf2d9c84 cf5aba00
[   73.362081] 9c60: cf596bd0 cf5aba00 00000000 cf4ab000 cf5aba00 c0000dc0 cf5aba00 c058bc78
[   73.370651] 9c80: 0020e2c0 00000000 cf2d9c90 cf2d9ca0 abcdf303 000000f3 aaaaaaaa aaaaaaaa
[   73.379222] 9ca0: abcd0003 00000000 aaaaaabb aaaaaaaa cf5aba00 cf5aba00 cf4ab000 cf5aba00
[   73.387792] 9cc0: 00000000 0000f600 cf4ab000 c0821ba8 00000000 c058b208 00000000 c005eb18
[   73.396363] 9ce0: c08645fc 60000013 00000000 cf2d8000 c0f8cc61 00000002 abcd0003 00000000
[   73.404932] 9d00: aaaaaaaa aaaaaaaa abcd0003 00000000 aaaaaabb aaaaaaaa 00000000 00000000
[   73.413503] 9d20: c0865c40 cf5abb80 c0821b68 c08646e0 c08644fc cf5abb80 00000000 c041ac5c
[   73.422074] 9d40: 00000000 00000000 c041a86c 00000002 00000003 00000040 c0865c40 cf5abb80
[   73.430644] 9d60: cf20e2c0 c0864510 00000001 c0865c40 cf5abb80 00000000 c0865c14 c0865bd4
[   73.439215] 9d80: 00000040 c0865c40 c0865bc8 c041c1dc cf20e2c0 c0865bc0 0000012c 00000040
[   73.447787] 9da0: c0865bc0 c0865bc8 ffffa76e c041bb20 c086714c ffffa76e c041ba6c 00000008
[   73.456357] 9dc0: cf2d8020 cf2d8000 c0867140 00000100 c0867100 c086714c 00000004 c00366b0
[   73.464927] 9de0: cf20e2c0 cf2d8038 00000003 40000003 0000000a 04208060 00000000 ffffa76d
[   73.473497] 9e00: 20000013 60000013 aaaaaaaa cf5abb80 cf4ab520 cf4ab538 cf2d8030 00000001
[   73.482067] 9e20: 00000000 c00368a0 00000000 c041b518 00000000 c058e14c 00000000 00000000
[   73.490638] 9e40: c058df74 c005eb18 cf320a9c cf4ab520 cff8cc61 cf320a9c abcdf303 000000f3
[   73.499208] 9e60: aaaaaaaa aaaaaaaa abcd0003 00000000 aaaaaabb aaaaaaaa 00000001 cf320a9c
[   73.507780] 9e80: 00000000 cf5abb80 cf5abb80 cf320a60 cd9e1200 cf2d9ec8 cf03ea00 c058c7ec
[   73.516351] 9ea0: cf2d4d40 cd9e1204 cf036400 c0045fc4 00000001 00000000 c0045f4c 60000093
[   73.524921] 9ec0: 00000000 00000000 c1082b28 c0bb9980 00000000 c0765727 cf20e2c0 cf2d4d40
[   73.533492] 9ee0: cf036400 cf036400 c0832920 cf036430 cf2d8000 cf2d4d58 00000000 c0046b64
[   73.542063] 9f00: cf2d4d40 cf20e2c0 00000000 cf2d2400 00000000 cf2d4d40 c004688c 00000000
[   73.550633] 9f20: 00000000 00000000 00000000 c004a0a0 c0831550 00000000 cf20e2c0 cf2d4d40
[   73.559202] 9f40: 00000000 00000001 dead4ead ffffffff ffffffff c086769c 00000000 00000000
[   73.567772] 9f60: c07038f4 cf2d9f64 cf2d9f64 00000000 00000001 dead4ead ffffffff ffffffff
[   73.576342] 9f80: c086769c 00000000 00000000 c07038f4 cf2d9f90 cf2d9f90 cf2d9fac cf2d2400
[   73.584910] 9fa0: c0049fcc 00000000 00000000 c000db88 00000000 00000000 00000000 00000000
[   73.593479] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   73.602048] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 fefffbae bf3baaae
[   73.610626] [<c041000c>] (skb_put) from [<c0410700>] (skb_try_coalesce+0x6c/0x32c)
[   73.618564] [<c0410700>] (skb_try_coalesce) from [<c058bc78>] (lowpan_frag_rcv+0x668/0x7a8)
[   73.627318] [<c058bc78>] (lowpan_frag_rcv) from [<c058b208>] (lowpan_rcv+0xc8/0x1f0)
[   73.635441] [<c058b208>] (lowpan_rcv) from [<c041ac5c>] (__netif_receive_skb_core+0x43c/0x56c)
[   73.644470] [<c041ac5c>] (__netif_receive_skb_core) from [<c041c1dc>] (process_backlog+0x78/0x110)
[   73.653860] [<c041c1dc>] (process_backlog) from [<c041bb20>] (net_rx_action+0xb4/0x18c)
[   73.662267] [<c041bb20>] (net_rx_action) from [<c00366b0>] (__do_softirq+0x104/0x264)
[   73.670478] [<c00366b0>] (__do_softirq) from [<c00368a0>] (do_softirq+0x44/0x6c)
[   73.678233] [<c00368a0>] (do_softirq) from [<c041b518>] (netif_rx_ni+0x20/0x2c)
[   73.685900] [<c041b518>] (netif_rx_ni) from [<c058e14c>] (mac802154_wpans_rx+0x208/0x260)
[   73.694472] [<c058e14c>] (mac802154_wpans_rx) from [<c058c7ec>] (mac802154_rx_worker+0x7c/0x94)
[   73.703594] [<c058c7ec>] (mac802154_rx_worker) from [<c0045fc4>] (process_one_work+0x23c/0x3cc)
[   73.712712] [<c0045fc4>] (process_one_work) from [<c0046b64>] (worker_thread+0x2d8/0x438)

- Alex

^ permalink raw reply

* [bluetooth]  btusb issues
From: Patrick Shirkey @ 2014-10-23  8:59 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

I have now got backports to run with bluez instead of bluedroid and the
btusb driver instead of rtk_btusb.

However the android bluetooth system does not want to run for me.


/proc/kmsg:

<6>[   19.428823] Loading modules backported from Linux version
next-20140905-0-g92d88cb
<6>[   19.437576] Backport generated by backports.git
backports-20140905-0-ged7bd06
<6>[   19.813396] Bluetooth: Core ver 2.19
<6>[   19.818542] NET: Registered protocol family 31
<6>[   19.823621] Bluetooth: HCI device and connection manager initialized
<6>[   19.831138] Bluetooth: HCI socket layer initialized
<6>[   19.837130] Bluetooth: L2CAP socket layer initialized
<6>[   19.842883] Bluetooth: SCO socket layer initialized
<6>[   19.948096] usbcore: registered new interface driver btusb
<4>[   19.955936] [rfkill]: rfkill set power 1
<4>[   19.975853] [rtl8723au]: rtl8723au is powered down by bt


logcat:

D/BluetoothManagerService( 1613): Message: 201
D/BluetoothManagerService( 1613): MESSAGE_SAVE_NAME_AND_ADDRESS
D/BluetoothAdapterState( 2109): CURRENT_STATE=OFF, MESSAGE = USER_TURN_ON
D/BluetoothAdapterProperties( 2109): Setting state to 11
I/BluetoothAdapterState( 2109): Bluetooth adapter state changed: 10-> 11
D/BluetoothAdapterService( 2109): Broadcasting updateAdapterState() to 0
receivers.
D/BluetoothBondStateMachine( 2109): make
I/BluetoothAdapterState( 2109): Entering PendingCommandState State:
isTurningOn()=true, isTurningOff()=false
D/BluetoothAdapterState( 2109): CURRENT_STATE=PENDING, MESSAGE = STARTED,
isTurningOn=true, isTurningOff=false
E/BluetoothAdapterState( 2109): Error while turning Bluetooth On
D/BluetoothAdapterProperties( 2109): Setting state to 10
I/BluetoothAdapterState( 2109): Bluetooth adapter state changed: 11-> 10
D/BluetoothAdapterService( 2109): Broadcasting updateAdapterState() to 0
receivers.
I/BluetoothAdapterState( 2109): Entering OffState
I/BluetoothBondStateMachine( 2109): StableState(): Entering Off State


Start bluetooth with UI:


D/BluetoothManagerService( 1613): enable():  mBluetooth =null mBinding =
false
D/BluetoothManagerService( 1613): Message: 1
D/BluetoothManagerService( 1613): MESSAGE_ENABLE: mBluetooth = null
D/BluetoothAdapterService( 2109): REFCOUNT: CREATED. INSTANCE_COUNT2
D/BluetoothAdapterState( 2109): make
I/BluetoothAdapterState( 2109): Entering OffState
D/BluetoothManagerService( 1613): BluetoothServiceConnection:
com.android.bluetooth.btservice.AdapterService
D/BluetoothManagerService( 1613): Message: 40
D/BluetoothManagerService( 1613): MESSAGE_BLUETOOTH_SERVICE_CONNECTED: 1
E/BluetoothManagerService( 1613): IBluetooth.configHciSnoopLog return false
D/BluetoothManagerService( 1613): Calling onBluetoothServiceUp callbacks
D/BluetoothManagerService( 1613): Broadcasting onBluetoothServiceUp() to 4
receivers.
D/BluetoothAdapterState( 2109): CURRENT_STATE=OFF, MESSAGE = USER_TURN_ON
D/BluetoothAdapterProperties( 2109): Setting state to 11
I/BluetoothAdapterState( 2109): Bluetooth adapter state changed: 10-> 11
D/BluetoothAdapterService( 2109): Broadcasting updateAdapterState() to 1
receivers.
D/BluetoothManagerService( 1613): Message: 60
D/BluetoothManagerService( 1613): MESSAGE_BLUETOOTH_STATE_CHANGE:
prevState = 10, newState=11
D/BluetoothManagerService( 1613): Bluetooth State Change Intent: 10 -> 11
D/BluetoothBondStateMachine( 2109): make
D/BluetoothMapService( 2109): onReceive
D/HeadsetService( 2109): Received start request. Starting profile...
D/HeadsetStateMachine( 2109): make
I/BluetoothAdapterState( 2109): Entering PendingCommandState State:
isTurningOn()=true, isTurningOff()=false
I/BluetoothBondStateMachine( 2109): StableState(): Entering Off State
E/HeadsetStateMachine( 2109): Could not bind to Bluetooth Headset Phone
Service
E/BluetoothHeadsetServiceJni( 2109): Bluetooth module is not loaded
D/A2dpService( 2109): Received start request. Starting profile...
D/A2dpStateMachine( 2109): make
E/BluetoothA2dpServiceJni( 2109): Bluetooth module is not loaded
V/Avrcp   ( 2109): make
E/BluetoothAvrcpServiceJni( 2109): Bluetooth module is not loaded
D/A2dpStateMachine( 2109): Enter Disconnected: -2
D/HidService( 2109): Received start request. Starting profile...
E/BluetoothHidServiceJni( 2109): Bluetooth module is not loaded
D/HealthService( 2109): Received start request. Starting profile...
E/BluetoothHealthServiceJni( 2109): Bluetooth module is not loaded
D/PanService( 2109): Received start request. Starting profile...
D/BluetoothPanServiceJni( 2109): initializeNative(L110): pan
E/BluetoothPanServiceJni( 2109): ## ERROR : initializeNative(L115):
Bluetooth module is not loaded##
D/BtGatt.DebugUtils( 2109): handleDebugAction() action=null
D/BtGatt.GattService( 2109): Received start request. Starting profile...
D/BtGatt.GattService( 2109): start()
E/BtGatt.JNI( 2109): ERROR: initializeNative(L695): Bluetooth module is
not loaded##
D/BluetoothMapService( 2109): Received start request. Starting profile...
D/BluetoothMapService( 2109): start()
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.hdp.HealthService
D/HeadsetPhoneState( 2109): sendDeviceStateChanged. mService=0 mSignal=0
mRoam=0 mBatteryCharge=5
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.hdp.HealthService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.hdp.HealthService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.map.BluetoothMapService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.map.BluetoothMapService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.map.BluetoothMapService
D/BluetoothAdapterState( 2109): CURRENT_STATE=PENDING, MESSAGE = STARTED,
isTurningOn=true, isTurningOff=false
E/BluetoothAdapterState( 2109): Error while turning Bluetooth On
D/BluetoothAdapterProperties( 2109): Setting state to 10
I/BluetoothAdapterState( 2109): Bluetooth adapter state changed: 11-> 10
D/BluetoothAdapterService( 2109): Broadcasting updateAdapterState() to 1
receivers.
D/BluetoothManagerService( 1613): Message: 60
D/BluetoothManagerService( 1613): MESSAGE_BLUETOOTH_STATE_CHANGE:
prevState = 11, newState=10
D/BluetoothManagerService( 1613): Broadcasting
onBluetoothStateChange(false) to 6 receivers.
I/BluetoothAdapterState( 2109): Entering OffState
D/BluetoothMap( 1837): onBluetoothStateChange: up=false
D/BluetoothInputDevice( 1837): onBluetoothStateChange: up=false
D/BluetoothHeadset( 1613): onBluetoothStateChange: up=false
D/BluetoothA2dp( 1613): onBluetoothStateChange: up=false
D/BluetoothPbap( 1837): onBluetoothStateChange: up=false
D/BluetoothManagerService( 1613): Bluetooth State Change Intent: 11 -> 10
D/BluetoothMapService( 2109): onReceive
E/BluetoothManagerService( 1613): recoverBluetoothServiceFromError
W/ContextImpl( 1837): Calling a method in the system process without a
qualified user: android.app.ContextImpl.startService:1479
android.content.ContextWrapper.startService:494
android.content.ContextWrapper.startService:494
com.android.settings.bluetooth.DockEventReceiver.beginStartingService:134
com.android.settings.bluetooth.DockEventReceiver.onReceive:115
D/DockEventReceiver( 1837): finishStartingService: stopping service
D/BluetoothManagerService( 1613): Sending off request.
D/BluetoothAdapterState( 2109): CURRENT_STATE=OFF, MESSAGE = USER_TURN_OFF
D/BluetoothManagerService( 1613): Calling onBluetoothServiceDown callbacks
D/BluetoothManagerService( 1613): Broadcasting onBluetoothServiceDown() to
4 receivers.
D/BluetoothAdapterService( 2109): Cleaning up adapter native....
D/BluetoothAdapterService( 2109): Done cleaning up adapter native....
D/BluetoothAdapterService(1102208296)( 2109): ****onDestroy()********
D/BluetoothManagerService( 1613): Message: 42
D/BluetoothManagerService( 1613): MESSAGE_RESTART_BLUETOOTH_SERVICE:
Restart IBluetooth service
D/BluetoothAdapterService( 2109): REFCOUNT: CREATED. INSTANCE_COUNT3
D/BluetoothAdapterState( 2109): make
D/BluetoothManagerService( 1613): BluetoothServiceConnection:
com.android.bluetooth.btservice.AdapterService
D/BluetoothManagerService( 1613): Message: 40
D/BluetoothManagerService( 1613): MESSAGE_BLUETOOTH_SERVICE_CONNECTED: 1
E/BluetoothManagerService( 1613): IBluetooth.configHciSnoopLog return false
D/BluetoothManagerService( 1613): Calling onBluetoothServiceUp callbacks
I/BluetoothAdapterState( 2109): Entering OffState
D/BluetoothManagerService( 1613): Broadcasting onBluetoothServiceUp() to 4
receivers.
D/BluetoothAdapterState( 2109): CURRENT_STATE=OFF, MESSAGE = USER_TURN_ON
D/BluetoothAdapterProperties( 2109): Setting state to 11
I/BluetoothAdapterState( 2109): Bluetooth adapter state changed: 10-> 11
D/BluetoothAdapterService( 2109): Broadcasting updateAdapterState() to 1
receivers.
D/BluetoothBondStateMachine( 2109): make
D/BluetoothManagerService( 1613): Message: 60
D/BluetoothManagerService( 1613): MESSAGE_BLUETOOTH_STATE_CHANGE:
prevState = 10, newState=11
D/BluetoothManagerService( 1613): Bluetooth State Change Intent: 10 -> 11
D/BluetoothMapService( 2109): onReceive
D/HeadsetService( 2109): Received start request. Starting profile...
D/HeadsetStateMachine( 2109): make
I/BluetoothBondStateMachine( 2109): StableState(): Entering Off State
I/BluetoothAdapterState( 2109): Entering PendingCommandState State:
isTurningOn()=true, isTurningOff()=false
E/HeadsetStateMachine( 2109): Could not bind to Bluetooth Headset Phone
Service
E/BluetoothHeadsetServiceJni( 2109): Bluetooth module is not loaded
D/A2dpService( 2109): Received start request. Starting profile...
D/A2dpStateMachine( 2109): make
E/BluetoothA2dpServiceJni( 2109): Bluetooth module is not loaded
V/Avrcp   ( 2109): make
E/BluetoothAvrcpServiceJni( 2109): Bluetooth module is not loaded
D/A2dpStateMachine( 2109): Enter Disconnected: -2
D/HidService( 2109): Received start request. Starting profile...
E/BluetoothHidServiceJni( 2109): Bluetooth module is not loaded
D/HealthService( 2109): Received start request. Starting profile...
E/BluetoothHealthServiceJni( 2109): Bluetooth module is not loaded
D/PanService( 2109): Received start request. Starting profile...
D/BluetoothPanServiceJni( 2109): initializeNative(L110): pan
E/BluetoothPanServiceJni( 2109): ## ERROR : initializeNative(L115):
Bluetooth module is not loaded##
D/BtGatt.DebugUtils( 2109): handleDebugAction() action=null
D/BtGatt.GattService( 2109): Received start request. Starting profile...
D/BtGatt.GattService( 2109): start()
E/BtGatt.JNI( 2109): ERROR: initializeNative(L695): Bluetooth module is
not loaded##
D/BluetoothMapService( 2109): Received start request. Starting profile...
D/BluetoothMapService( 2109): start()
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.hdp.HealthService
D/HeadsetPhoneState( 2109): sendDeviceStateChanged. mService=0 mSignal=0
mRoam=0 mBatteryCharge=5
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.hdp.HealthService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.hdp.HealthService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.map.BluetoothMapService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.map.BluetoothMapService
D/BluetoothAdapterService( 2109): Profile still not
running:com.android.bluetooth.map.BluetoothMapService
D/BluetoothAdapterState( 2109): CURRENT_STATE=PENDING, MESSAGE = STARTED,
isTurningOn=true, isTurningOff=false
E/BluetoothAdapterState( 2109): Error while turning Bluetooth On
D/BluetoothAdapterProperties( 2109): Setting state to 10
I/BluetoothAdapterState( 2109): Bluetooth adapter state changed: 11-> 10
D/BluetoothAdapterService( 2109): Broadcasting updateAdapterState() to 1
receivers.
D/BluetoothManagerService( 1613): Message: 60
D/BluetoothManagerService( 1613): MESSAGE_BLUETOOTH_STATE_CHANGE:
prevState = 11, newState=10
D/BluetoothManagerService( 1613): Broadcasting
onBluetoothStateChange(false) to 6 receivers.
E/BluetoothPan( 1837):
E/BluetoothPan( 1837): java.lang.IllegalArgumentException: Service not
registered: android.bluetooth.BluetoothPan$2@41b0dd10
E/BluetoothPan( 1837): 	at
android.app.LoadedApk.forgetServiceDispatcher(LoadedApk.java:931)
E/BluetoothPan( 1837): 	at
android.app.ContextImpl.unbindService(ContextImpl.java:1595)
E/BluetoothPan( 1837): 	at
android.content.ContextWrapper.unbindService(ContextWrapper.java:529)
E/BluetoothPan( 1837): 	at
android.bluetooth.BluetoothPan$1.onBluetoothStateChange(BluetoothPan.java:199)
E/BluetoothPan( 1837): 	at
android.bluetooth.IBluetoothStateChangeCallback$Stub.onTransact(IBluetoothStateChangeCallback.java:55)
E/BluetoothPan( 1837): 	at android.os.Binder.execTransact(Binder.java:404)
E/BluetoothPan( 1837): 	at dalvik.system.NativeStart.run(Native Method)
I/BluetoothAdapterState( 2109): Entering OffState
D/BluetoothMap( 1837): onBluetoothStateChange: up=false
E/BluetoothMap( 1837):
E/BluetoothMap( 1837): java.lang.IllegalArgumentException: Service not
registered: android.bluetooth.BluetoothMap$2@41b13478
E/BluetoothMap( 1837): 	at
android.app.LoadedApk.forgetServiceDispatcher(LoadedApk.java:931)
E/BluetoothMap( 1837): 	at
android.app.ContextImpl.unbindService(ContextImpl.java:1595)
E/BluetoothMap( 1837): 	at
android.content.ContextWrapper.unbindService(ContextWrapper.java:529)
E/BluetoothMap( 1837): 	at
android.bluetooth.BluetoothMap$1.onBluetoothStateChange(BluetoothMap.java:66)
E/BluetoothMap( 1837): 	at
android.bluetooth.IBluetoothStateChangeCallback$Stub.onTransact(IBluetoothStateChangeCallback.java:55)
E/BluetoothMap( 1837): 	at android.os.Binder.execTransact(Binder.java:404)
E/BluetoothMap( 1837): 	at dalvik.system.NativeStart.run(Native Method)
D/BluetoothInputDevice( 1837): onBluetoothStateChange: up=false
E/BluetoothInputDevice( 1837):
E/BluetoothInputDevice( 1837): java.lang.IllegalArgumentException: Service
not registered: android.bluetooth.BluetoothInputDevice$2@41b08180
E/BluetoothInputDevice( 1837): 	at
android.app.LoadedApk.forgetServiceDispatcher(LoadedApk.java:931)
E/BluetoothInputDevice( 1837): 	at
android.app.ContextImpl.unbindService(ContextImpl.java:1595)
E/BluetoothInputDevice( 1837): 	at
android.content.ContextWrapper.unbindService(ContextWrapper.java:529)
E/BluetoothInputDevice( 1837): 	at
android.bluetooth.BluetoothInputDevice$1.onBluetoothStateChange(BluetoothInputDevice.java:199)
E/BluetoothInputDevice( 1837): 	at
android.bluetooth.IBluetoothStateChangeCallback$Stub.onTransact(IBluetoothStateChangeCallback.java:55)
E/BluetoothInputDevice( 1837): 	at
android.os.Binder.execTransact(Binder.java:404)
E/BluetoothInputDevice( 1837): 	at dalvik.system.NativeStart.run(Native
Method)
D/BluetoothHeadset( 1613): onBluetoothStateChange: up=false
E/BluetoothHeadset( 1613):
E/BluetoothHeadset( 1613): java.lang.IllegalArgumentException: Service not
registered: android.bluetooth.BluetoothHeadset$2@41e17290
E/BluetoothHeadset( 1613): 	at
android.app.LoadedApk.forgetServiceDispatcher(LoadedApk.java:931)
E/BluetoothHeadset( 1613): 	at
android.app.ContextImpl.unbindService(ContextImpl.java:1595)
E/BluetoothHeadset( 1613): 	at
android.bluetooth.BluetoothHeadset$1.onBluetoothStateChange(BluetoothHeadset.java:239)
E/BluetoothHeadset( 1613): 	at
com.android.server.BluetoothManagerService.sendBluetoothStateCallback(BluetoothManagerService.java:484)
E/BluetoothHeadset( 1613): 	at
com.android.server.BluetoothManagerService.bluetoothStateChangeHandler(BluetoothManagerService.java:1129)
E/BluetoothHeadset( 1613): 	at
com.android.server.BluetoothManagerService.access$2800(BluetoothManagerService.java:47)
E/BluetoothHeadset( 1613): 	at
com.android.server.BluetoothManagerService$BluetoothHandler.handleMessage(BluetoothManagerService.java:864)
E/BluetoothHeadset( 1613): 	at
android.os.Handler.dispatchMessage(Handler.java:102)
E/BluetoothHeadset( 1613): 	at android.os.Looper.loop(Looper.java:137)
E/BluetoothHeadset( 1613): 	at
android.os.HandlerThread.run(HandlerThread.java:61)
D/BluetoothA2dp( 1613): onBluetoothStateChange: up=false
E/BluetoothA2dp( 1613):
E/BluetoothA2dp( 1613): java.lang.IllegalArgumentException: Service not
registered: android.bluetooth.BluetoothA2dp$2@41e4c018
E/BluetoothA2dp( 1613): 	at
android.app.LoadedApk.forgetServiceDispatcher(LoadedApk.java:931)
E/BluetoothA2dp( 1613): 	at
android.app.ContextImpl.unbindService(ContextImpl.java:1595)
E/BluetoothA2dp( 1613): 	at
android.bluetooth.BluetoothA2dp$1.onBluetoothStateChange(BluetoothA2dp.java:121)
E/BluetoothA2dp( 1613): 	at
com.android.server.BluetoothManagerService.sendBluetoothStateCallback(BluetoothManagerService.java:484)
E/BluetoothA2dp( 1613): 	at
com.android.server.BluetoothManagerService.bluetoothStateChangeHandler(BluetoothManagerService.java:1129)
E/BluetoothA2dp( 1613): 	at
com.android.server.BluetoothManagerService.access$2800(BluetoothManagerService.java:47)
E/BluetoothA2dp( 1613): 	at
com.android.server.BluetoothManagerService$BluetoothHandler.handleMessage(BluetoothManagerService.java:864)
E/BluetoothA2dp( 1613): 	at
android.os.Handler.dispatchMessage(Handler.java:102)
E/BluetoothA2dp( 1613): 	at android.os.Looper.loop(Looper.java:137)
E/BluetoothA2dp( 1613): 	at
android.os.HandlerThread.run(HandlerThread.java:61)
D/BluetoothPbap( 1837): onBluetoothStateChange: up=false
E/BluetoothPbap( 1837):
E/BluetoothPbap( 1837): java.lang.IllegalArgumentException: Service not
registered: android.bluetooth.BluetoothPbap$2@41b18c50
E/BluetoothPbap( 1837): 	at
android.app.LoadedApk.forgetServiceDispatcher(LoadedApk.java:931)
E/BluetoothPbap( 1837): 	at
android.app.ContextImpl.unbindService(ContextImpl.java:1595)
E/BluetoothPbap( 1837): 	at
android.content.ContextWrapper.unbindService(ContextWrapper.java:529)
E/BluetoothPbap( 1837): 	at
android.bluetooth.BluetoothPbap$1.onBluetoothStateChange(BluetoothPbap.java:122)
E/BluetoothPbap( 1837): 	at
android.bluetooth.IBluetoothStateChangeCallback$Stub.onTransact(IBluetoothStateChangeCallback.java:55)
E/BluetoothPbap( 1837): 	at android.os.Binder.execTransact(Binder.java:404)
E/BluetoothPbap( 1837): 	at dalvik.system.NativeStart.run(Native Method)
D/BluetoothManagerService( 1613): Bluetooth State Change Intent: 11 -> 10
D/BluetoothMapService( 2109): onReceive
E/BluetoothManagerService( 1613): recoverBluetoothServiceFromError
W/ContextImpl( 1837): Calling a method in the system process without a
qualified user: android.app.ContextImpl.startService:1479
android.content.ContextWrapper.startService:494
android.content.ContextWrapper.startService:494
com.android.settings.bluetooth.DockEventReceiver.beginStartingService:134
com.android.settings.bluetooth.DockEventReceiver.onReceive:115
D/DockEventReceiver( 1837): finishStartingService: stopping service
D/BluetoothManagerService( 1613): Sending off request.
D/BluetoothAdapterState( 2109): CURRENT_STATE=OFF, MESSAGE = USER_TURN_OFF
D/BluetoothManagerService( 1613): Calling onBluetoothServiceDown callbacks
D/BluetoothManagerService( 1613): Broadcasting onBluetoothServiceDown() to
4 receivers.
D/BluetoothAdapterService( 2109): Cleaning up adapter native....
D/BluetoothAdapterService( 2109): Done cleaning up adapter native....
D/BluetoothAdapterService(1102260480)( 2109): ****onDestroy()********
D/BluetoothManagerService( 1613): Message: 42
D/BluetoothManagerService( 1613): MESSAGE_RESTART_BLUETOOTH_SERVICE:
Restart IBluetooth service
D/BluetoothAdapterService( 2109): REFCOUNT: CREATED. INSTANCE_COUNT4


Testing with bluez cli tools:


# haltest

audio_hw_device_open returned -98
hw_get_module_by_class returned -2
if_bluetooth->init: BT_STATUS_FAIL
get_profile_interface(handsfree) : 0x0
get_profile_interface(a2dp) : 0x0
get_profile_interface(avrcp) : 0x0
get_profile_interface(health) : 0x0
get_profile_interface(hidhost) : 0x0
get_profile_interface(pan) : 0x0
get_profile_interface(gatt) : 0x0
get_profile_interface(socket) : 0x0
if_bluetooth->init: BT_STATUS_FAIL
if_av is NULL
if_rc is NULL
if_gatt is NULL
if_hf is NULL
if_hh is NULL
if_pan is NULL
if_hl is NULL
>

logcat output from haltest


D/BlueZ   ( 2567): external/bluetooth/bluez/android/hal-audio.c:audio_open()
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-audio.c:audio_ipc_init()
E/BlueZ   ( 2567): audio: Failed to bind socket: 98 (Address already in use)
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:open_bluetooth()
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_bluetooth_interface()
D/BlueZ   ( 2567): external/bluetooth/bluez/android/hal-bluetooth.c:init()
I/bluetoothd( 2568): bluetoothd[2569]: Bluetooth daemon 5.23
I/bluetoothd( 2568): bluetoothd[2569]: Starting SDP server
I/bluetoothd( 2568): bluetoothd[2569]: Bluetooth management interface 1.7
initialized
I/bluetoothd( 2568): bluetoothd[2569]: Kernel connection control will be used
I/bluetoothd( 2568): bluetoothd[2569]: Stopping SDP server
I/bluetoothd( 2568): bluetoothd[2569]: Exit
E/BlueZ   ( 2567): bluetoothd connect timeout
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
handsfree
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
a2dp
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
avrcp
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
health
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
hidhost
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
pan
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
gatt
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_profile_interface()
socket
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:open_bluetooth()
D/BlueZ   ( 2567):
external/bluetooth/bluez/android/hal-bluetooth.c:get_bluetooth_interface()
D/BlueZ   ( 2567): external/bluetooth/bluez/android/hal-bluetooth.c:init()
I/bluetoothd( 2570): bluetoothd[2571]: Bluetooth daemon 5.23
I/bluetoothd( 2570): bluetoothd[2571]: Starting SDP server
I/bluetoothd( 2570): bluetoothd[2571]: Bluetooth management interface 1.7
initialized
I/bluetoothd( 2570): bluetoothd[2571]: Kernel connection control will be used
I/bluetoothd( 2570): bluetoothd[2571]: Stopping SDP server
I/bluetoothd( 2570): bluetoothd[2571]: Exit
E/BlueZ   ( 2567): bluetoothd connect timeout


# l2test
l2test[2591]: Waiting for connection on psm 4113 ...

# rctest
rctest[2593]: Can't create socket: Protocol not supported (93)

# btmgmt le on
Set Low Energy for hci0 failed with status 0x11 (Invalid Index)
# btmgmt info
# btmgmt power on
Set Powered for hci0 failed with status 0x11 (Invalid Index)


--
Patrick Shirkey
Boost Hardware Ltd

^ permalink raw reply

* Re: [PATCHv5 00/14] Included service discovery
From: Luiz Augusto von Dentz @ 2014-10-23  7:55 UTC (permalink / raw)
  To: Szymon Janc
  Cc: Arman Uguray, Marcin Kraglak,
	linux-bluetooth@vger.kernel.org development
In-Reply-To: <2326994.RrINgRSL6X@athlon>

Hi Szymon,

On Wed, Oct 22, 2014 at 9:39 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi,
>
> On Wednesday 22 October 2014 08:35:05 Arman Uguray wrote:
>> Hi Luiz,
>>
>> On Wed, Oct 22, 2014 at 7:54 AM, Luiz Augusto von Dentz
>>
>> <luiz.dentz@gmail.com> wrote:
>> > Hi Marcin,
>> >
>> > On Wed, Oct 22, 2014 at 9:25 AM, Marcin Kraglak
>> >
>> > <marcin.kraglak@tieto.com> wrote:
>> >> On 16 October 2014 12:17, Marcin Kraglak <marcin.kraglak@tieto.com>
> wrote:
>> >>> v3:
>> >>> In this version after primary service discovery,
>> >>> secondary services are discovered. Next included
>> >>> services are resolved. With this approach we
>> >>> don't have recursively search for included service,
>> >>> like it was TODO in previous proposal.
>> >>> There is also small coding style fix suggested by Arman.
>> >>>
>> >>> v4:
>> >>> If no secondary services found, continue include services search (fixed
>> >>> in gatt-client.c).
>> >>> Fixed wrong debug logs (primary->secondary).
>> >>> Fixed searching descriptors
>> >>>
>> >>> v5:
>> >>> Ignore Unsupported Group Type Error in response to secondary service
>> >>> discovery and continue included services discovery.
>> >>>
>> >>> Marcin Kraglak (14):
>> >>>   shared/gatt: Add discover_secondary_services()
>> >>>   shared/gatt: Add initial implementation of discover_included_services
>> >>>   shared/gatt: Discover included services 128 bit UUIDS
>> >>>   shared/gatt: Add extra check in characteristic iterator
>> >>>   shared/gatt: Add included service iterator
>> >>>   shared/gatt: Remove not needed function parameter
>> >>>   shared/gatt: Distinguish Primary from Secondary services
>> >>>   tools/btgatt-client: Print type of service
>> >>>   shared/gatt: Discover secondary services
>> >>>   shared/gatt: Discover included services
>> >>>   shared/gatt: Add gatt-client include service iterator
>> >>>   tools/btgatt-client: Print found include services
>> >>>   shared/gatt: Fix searching descriptors
>> >>>   shared/gatt: Add function bt_gatt_result_included_count()
>> >>>
>> >>>  src/shared/gatt-client.c  | 263 +++++++++++++++++++++++++++--
>> >>>  src/shared/gatt-client.h  |  18 ++
>> >>>  src/shared/gatt-helpers.c | 418
>> >>>  +++++++++++++++++++++++++++++++++++++++++++---
>> >>>  src/shared/gatt-helpers.h |  10 +-
>> >>>  tools/btgatt-client.c     |  17 +-
>> >>>  5 files changed, 690 insertions(+), 36 deletions(-)
>> >>>
>> >>> --
>> >>> 1.9.3
>> >
>> > Patches looks fine to me, but I would like Arman to ack before applying.
>>
>> I'm fine with the patches as they are, though I saw that Szymon has
>> left comments on some of them (I'm guessing he's a doing a pass of the
>> patch set now). I'm good with it as long as his comments are
>> addressed.
>
> I really think we should have more comments and less magic numbers there.
> Otherwise this code will be hard to maintain in long term.

Yep, we could actually start creating packed structs for PDU as it was
done for AVRCP so we could do sizeof etc, it makes the code much
easier to understand and maintain.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: Attribute security permissions and CCC descriptors in shared/gatt-server.
From: Luiz Augusto von Dentz @ 2014-10-23  7:51 UTC (permalink / raw)
  To: Arman Uguray; +Cc: BlueZ development, Marcin Kraglak, Marcel Holtmann
In-Reply-To: <CAHrH25S3h=LjaXSFkJiz5X41JD9-1zNJiD1R7hQNfA3aSbO0sg@mail.gmail.com>

Hi Arman,

On Wed, Oct 22, 2014 at 11:12 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi all,
>
> I have some unresolved questions about two aspects of
> shared/gatt-server and I thought it might be better to discuss them
> before moving on with the related parts of the implementation.
>
> *** 1. Who will be responsible for Attribute Permission checks? ***
>
> These mainly come in play when the remote end wants to read or write
> the value of a characteristic/descriptor. We currently have read &
> write callbacks that get assigned by the upper-layer to a
> characteristic/descriptor definition in shared/gatt-db. Since these
> callbacks already handle the read/write it initially made sense to me
> that this is where encryption/authentication/authorization permission
> checks can be performed. I then realized that there are cases where we
> may want to perform this check before we invoke the read/write
> callback. For instance, the Prepare Write request requires attribute
> permission checks to be performed, however (in the current design) we
> wouldn't actually call the write callback until a subsequent Execute
> Write request is received.

I believe this should be handle by the db, perhaps with a dedicated
prepare function that should probably call a callback so the
implementation can really tell when a write is about to happen. The
weird thing here is that the gatt_db_add_characteristic take
permissions but never really use it for anything except in
gatt_db_get_attribute_permissions, I thought the idea would be to do
the permission checks with it or Im missing something.

> The main problem here is the fact that shared/att is designed to be
> transport agnostic: it doesn't know whether the underlying connection
> is AF_BLUETOOTH or something else, so it can't make assumptions on the
> nature of the connection to obtain security level information. We
> could add some sort of delegate callback to this end, perhaps
> something like:

Then we should probably have some functions to set and get the
security level, but Im sure it needs to be a callback as it does not
looks like it gonna change without us noticing it, then we can
automatically check with security level against the permissions
provided by the profile when registering the characteristic.

> static uint8_t my_sec_handler(void *user_data)
> {
>         ....
>         return sec;
> }
> ...
> bt_att_register_security_handler(att, my_sec_handler);
>
> And then bt_gatt_server can obtain the current security level like this:
>
> uint8_t sec = bt_att_get_security_level(att);
>
> where bt_att_get_security_level is implemented as:
>
> static uint8_t bt_att_get_security_level(struct bt_att *att)
> {
>       if (!att || !att->security_callback)
>               return 0;
>
>       return att->security_callback(att->security_data);
> }
>
>
> I'm mostly thinking out loud here; would something like this make sense?
>
>
> *** 2. Is shared/gatt-server responsible for keeping track of Client
> Characteristic Configuration descriptor states for each bonded client?
> ***
>
> Currently, the descriptor read/write operations need to be handled by
> the upper layer via the read/write callbacks anyway, since the
> side-effects of a writing to a particular CCC descriptor need to be
> implemented by the related profile. In the bonding case, the
> read/write callbacks could determine what value to return and how to
> cache the value on a per-device basis without shared/gatt-db having a
> notion of "per-client CCC descriptors".

Well making gatt_db have the notion of CCC would make things a little
bit simpler for storing and reloading but it would probably force us
to rethink how the db access the values, perhaps the db could cache
CCC values per client so it could detect changes and send
notifications automatically whenever the profiles tell it values has
changed or when a write succeeds.

> Does this make sense? If so, is there a point of keeping the "bdaddr_t
> *bdaddr" arguments of gatt_db_read_t, gatt_db_write_t, gatt_db_read,
> and gatt_db_write? Note that I'm currently passing NULL to all of
> these calls for this parameter in shared/gatt-server, since bt_att
> doesn't have a notion of a BD_ADDR.

I guess this was inspired by Android, I would agree that if you
register a characteristic and set the permission properly then it does
not matter who is attempting to read/write as long as they are fulfill
the requirements it should be okay but perhaps Android have pushed the
notion of CCC up in the stack which force it to expose the remote
address. Anyway for unit test I will have to address it since that
will be using a socket pair, perhaps passing NULL is fine if we have
CCC caching but in case of Android I don't think we can do much about
it.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Attribute security permissions and CCC descriptors in shared/gatt-server.
From: Arman Uguray @ 2014-10-22 20:12 UTC (permalink / raw)
  To: BlueZ development, Marcin Kraglak, Luiz Augusto von Dentz,
	Marcel Holtmann

Hi all,

I have some unresolved questions about two aspects of
shared/gatt-server and I thought it might be better to discuss them
before moving on with the related parts of the implementation.

*** 1. Who will be responsible for Attribute Permission checks? ***

These mainly come in play when the remote end wants to read or write
the value of a characteristic/descriptor. We currently have read &
write callbacks that get assigned by the upper-layer to a
characteristic/descriptor definition in shared/gatt-db. Since these
callbacks already handle the read/write it initially made sense to me
that this is where encryption/authentication/authorization permission
checks can be performed. I then realized that there are cases where we
may want to perform this check before we invoke the read/write
callback. For instance, the Prepare Write request requires attribute
permission checks to be performed, however (in the current design) we
wouldn't actually call the write callback until a subsequent Execute
Write request is received.

The main problem here is the fact that shared/att is designed to be
transport agnostic: it doesn't know whether the underlying connection
is AF_BLUETOOTH or something else, so it can't make assumptions on the
nature of the connection to obtain security level information. We
could add some sort of delegate callback to this end, perhaps
something like:

static uint8_t my_sec_handler(void *user_data)
{
        ....
        return sec;
}
...
bt_att_register_security_handler(att, my_sec_handler);

And then bt_gatt_server can obtain the current security level like this:

uint8_t sec = bt_att_get_security_level(att);

where bt_att_get_security_level is implemented as:

static uint8_t bt_att_get_security_level(struct bt_att *att)
{
      if (!att || !att->security_callback)
              return 0;

      return att->security_callback(att->security_data);
}


I'm mostly thinking out loud here; would something like this make sense?


*** 2. Is shared/gatt-server responsible for keeping track of Client
Characteristic Configuration descriptor states for each bonded client?
***

Currently, the descriptor read/write operations need to be handled by
the upper layer via the read/write callbacks anyway, since the
side-effects of a writing to a particular CCC descriptor need to be
implemented by the related profile. In the bonding case, the
read/write callbacks could determine what value to return and how to
cache the value on a per-device basis without shared/gatt-db having a
notion of "per-client CCC descriptors".

Does this make sense? If so, is there a point of keeping the "bdaddr_t
*bdaddr" arguments of gatt_db_read_t, gatt_db_write_t, gatt_db_read,
and gatt_db_write? Note that I'm currently passing NULL to all of
these calls for this parameter in shared/gatt-server, since bt_att
doesn't have a notion of a BD_ADDR.

Thanks,
Arman

^ permalink raw reply

* Re: [PATCHv5 05/14] shared/gatt: Add included service iterator
From: Szymon Janc @ 2014-10-22 18:43 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-6-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Thursday 16 October 2014 12:17:17 Marcin Kraglak wrote:
> It will fetch included services from result.
> ---
>  src/shared/gatt-helpers.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++++ src/shared/gatt-helpers.h |
>  3 +++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 58104d9..fd8d06c 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -186,6 +186,64 @@ struct discovery_op {
>  	bt_gatt_destroy_func_t destroy;
>  };
> 
> +bool bt_gatt_iter_next_included_service(struct bt_gatt_iter *iter,
> +				uint16_t *handle, uint16_t *start_handle,
> +				uint16_t *end_handle, uint8_t uuid[16])
> +{
> +	struct bt_gatt_result *read_result;
> +	const void *pdu_ptr;
> +	int i = 0;
> +
> +	if (!iter || !iter->result || !handle || !start_handle || !end_handle
> +								|| !uuid)
> +		return false;
> +
> +	if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
> +		return false;
> +
> +	if (iter->result->data_len != 8 && iter->result->data_len != 6)
> +		return false;
> +
> +	pdu_ptr = iter->result->pdu + iter->pos;
> +
> +	if (iter->result->data_len == 8) {
> +		*handle = get_le16(pdu_ptr);
> +		*start_handle = get_le16(pdu_ptr + 2);
> +		*end_handle = get_le16(pdu_ptr + 4);
> +		convert_uuid_le(pdu_ptr + 6, 2, uuid);
> +
> +		iter->pos += iter->result->data_len;
> +
> +		if (iter->pos == iter->result->pdu_len) {
> +			iter->result = iter->result->next;
> +			iter->pos = 0;
> +		}
> +
> +		return true;
> +	}
> +
> +	*handle = get_le16(pdu_ptr);
> +	*start_handle = get_le16(pdu_ptr + 2);
> +	*end_handle = get_le16(pdu_ptr + 4);
> +	read_result = iter->result;
> +
> +	do {
> +		read_result = read_result->next;
> +	} while (read_result && i++ < (iter->pos / iter->result->data_len));

This loop looks quite unusual. Maybe use for and break inside?
Also please put some comment about those calculations.

> +
> +	if (!read_result)
> +		return false;
> +
> +	convert_uuid_le(read_result->pdu, read_result->data_len, uuid);
> +	iter->pos += iter->result->data_len;
> +	if (iter->pos == iter->result->pdu_len) {
> +		iter->result = read_result->next;
> +		iter->pos = 0;
> +	}
> +
> +	return true;
> +}
> +
>  bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
>  				uint16_t *start_handle, uint16_t *end_handle,
>  				uint8_t uuid[16])
> diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
> index 8a25dea..8c434c1 100644
> --- a/src/shared/gatt-helpers.h
> +++ b/src/shared/gatt-helpers.h
> @@ -49,6 +49,9 @@ bool bt_gatt_iter_next_characteristic(struct bt_gatt_iter
> *iter, uint8_t uuid[16]);
>  bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter, uint16_t
> *handle, uint8_t uuid[16]);
> +bool bt_gatt_iter_next_included_service(struct bt_gatt_iter *iter,



> +				uint16_t *handle, uint16_t *start_handle,
> +				uint16_t *end_handle, uint8_t uuid[16]);
> 
>  typedef void (*bt_gatt_destroy_func_t)(void *user_data);

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCHv5 03/14] shared/gatt: Discover included services 128 bit UUIDS
From: Szymon Janc @ 2014-10-22 18:43 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-4-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Thursday 16 October 2014 12:17:15 Marcin Kraglak wrote:
> If included services has 128 bit UUID, it won't be returned in
> READ_BY_TYPE_RSP. To get UUID READ_REQUEST is used.
> This procedure is described in CORE SPEC 4.5.1 "Find Included Services".
> ---
>  src/shared/gatt-helpers.c | 170
> +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 169
> insertions(+), 1 deletion(-)
> 
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index dcb2a2f..c18f738 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -692,6 +692,160 @@ bool bt_gatt_discover_secondary_services(struct bt_att
> *att, bt_uuid_t *uuid, destroy, false);
>  }
> 
> +struct read_incl_data {
> +	struct discovery_op *op;
> +	struct bt_gatt_result *result;
> +	int pos;
> +	int ref_count;
> +};
> +
> +static struct read_incl_data *new_read_included(struct bt_gatt_result *res)
> +{
> +	struct read_incl_data *data;
> +
> +	data = new0(struct read_incl_data, 1);
> +	if (!data)
> +		return NULL;
> +
> +	data->op = discovery_op_ref(res->op);
> +	data->result = res;
> +
> +	return data;
> +};
> +
> +static struct read_incl_data *read_included_ref(struct read_incl_data
> *data) +{
> +	__sync_fetch_and_add(&data->ref_count, 1);
> +
> +	return data;
> +}
> +
> +static void read_included_unref(void *data)
> +{
> +	struct read_incl_data *read_data = data;
> +
> +	if (__sync_sub_and_fetch(&read_data->ref_count, 1))
> +		return;
> +
> +	discovery_op_unref(read_data->op);
> +
> +	free(read_data);
> +}
> +
> +static void discover_included_cb(uint8_t opcode, const void *pdu,
> +					uint16_t length, void *user_data);
> +
> +static void read_included_cb(uint8_t opcode, const void *pdu,
> +					uint16_t length, void *user_data)
> +{
> +	struct read_incl_data *data = user_data;
> +	struct bt_gatt_result *final_result = NULL;
> +	struct discovery_op *op = data->op;
> +	struct bt_gatt_result *cur_result;
> +	uint8_t att_ecode = 0;
> +	uint16_t handle;
> +	uint8_t read_pdu[2];
> +	bool success;
> +
> +	if (opcode == BT_ATT_OP_ERROR_RSP) {
> +		success = false;
> +		att_ecode = process_error(pdu, length);
> +		goto done;
> +	}
> +
> +	if (opcode != BT_ATT_OP_READ_RSP || (!pdu && length)) {
> +		success = false;
> +		goto done;
> +	}
> +
> +	if (length != 16) {

I think we should have those magic numbers defined (this is more general 
comment since more code here uses magic values). It might be clear for you now 
why this must be 16 but won't be in 2 months :)

> +		success = false;
> +		goto done;
> +	}



Empty line here.

> +	cur_result = result_create(opcode, pdu, length, length, op);
> +	if (!cur_result) {
> +		success = false;
> +		goto done;
> +	}
> +
> +	if (!op->result_head)
> +		op->result_head = op->result_tail = cur_result;

Braces on both branches.

> +	else {
> +		op->result_tail->next = cur_result;
> +		op->result_tail = cur_result;
> +	}
> +
> +	if (data->pos == data->result->pdu_len) {
> +		uint16_t last_handle, data_len;
> +		uint8_t pdu[6];
> +
> +		data_len = data->result->data_len;
> +		last_handle = get_le16(data->result->pdu + data->pos -
> +								data_len);

This data_len variable doesn't make code clearer.

> +		if (last_handle == op->end_handle) {
> +			final_result = op->result_head;
> +			success = true;
> +			goto done;
> +		}
> +
> +		put_le16(last_handle + 1, pdu);
> +		put_le16(op->end_handle, pdu + 2);
> +		put_le16(GATT_INCLUDE_UUID, pdu + 4);
> +
> +		if (bt_att_send(op->att, BT_ATT_OP_READ_BY_TYPE_REQ,
> +						pdu, sizeof(pdu),
> +						discover_included_cb,
> +						discovery_op_ref(op),
> +						discovery_op_unref))

This should fit just in 2 lines, instead of 4.

> +			return;
> +
> +		discovery_op_unref(op);
> +		success = false;
> +		goto done;
> +	}
> +
> +	handle = get_le16(data->result->pdu + data->pos + 2);
> +	put_le16(handle, read_pdu);

So if both are LE value why not just memcpy() here?

> +
> +	data->pos += data->result->data_len;
> +
> +	if (bt_att_send(op->att, BT_ATT_OP_READ_REQ, read_pdu, sizeof(read_pdu),
> +							read_included_cb,
> +							read_included_ref(data),
> +							read_included_unref))

Ditto.

> +		return;
> +
> +	read_included_unref(data);
> +	success = false;
> +
> +done:
> +	if (op->callback)
> +		op->callback(success, att_ecode, final_result, op->user_data);
> +}
> +
> +static void read_included(struct read_incl_data *data)
> +{
> +	struct discovery_op *op = data->op;
> +	uint16_t handle;
> +	uint8_t pdu[2];
> +
> +	handle = get_le16(data->result->pdu + 2);
> +	put_le16(handle, pdu);

memcpy

> +
> +	data->pos += data->result->data_len;
> +
> +	if (bt_att_send(op->att, BT_ATT_OP_READ_REQ, pdu, sizeof(pdu),
> +							read_included_cb,
> +							read_included_ref(data),
> +							read_included_unref))
> +		return;
> +
> +	read_included_unref(data);
> +
> +	if (op->callback)
> +		op->callback(false, 0, NULL, data->op->user_data);
> +}
> +
>  static void discover_included_cb(uint8_t opcode, const void *pdu,
>  					uint16_t length, void *user_data)
>  {
> @@ -721,7 +875,8 @@ static void discover_included_cb(uint8_t opcode, const
> void *pdu,
> 
>  	data_length = ((uint8_t *) pdu)[0];
> 
> -	if ((length - 1) % data_length || data_length != 8) {
> +	if (((length - 1) % data_length) ||
> +				(data_length != 8 && data_length != 6)) {
>  		success = false;
>  		goto done;
>  	}
> @@ -740,6 +895,19 @@ static void discover_included_cb(uint8_t opcode, const
> void *pdu, op->result_tail = cur_result;
>  	}
> 
> +	if (data_length == 6) {
> +		struct read_incl_data *data;
> +
> +		data = new_read_included(cur_result);
> +		if (!data) {
> +			success = false;
> +			goto done;
> +		}
> +
> +		read_included(data);
> +		return;
> +	}
> +
>  	last_handle = get_le16(pdu + length - data_length);
>  	if (last_handle != op->end_handle) {
>  		uint8_t pdu[6];

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCHv5 00/14] Included service discovery
From: Szymon Janc @ 2014-10-22 18:39 UTC (permalink / raw)
  To: Arman Uguray
  Cc: Luiz Augusto von Dentz, Marcin Kraglak,
	linux-bluetooth@vger.kernel.org development
In-Reply-To: <CAHrH25T_OGAuC2aS3FvV2ZNffv0Ep2PxxOk4+HDn7YXuvfJqbQ@mail.gmail.com>

Hi,

On Wednesday 22 October 2014 08:35:05 Arman Uguray wrote:
> Hi Luiz,
> 
> On Wed, Oct 22, 2014 at 7:54 AM, Luiz Augusto von Dentz
> 
> <luiz.dentz@gmail.com> wrote:
> > Hi Marcin,
> > 
> > On Wed, Oct 22, 2014 at 9:25 AM, Marcin Kraglak
> > 
> > <marcin.kraglak@tieto.com> wrote:
> >> On 16 October 2014 12:17, Marcin Kraglak <marcin.kraglak@tieto.com> 
wrote:
> >>> v3:
> >>> In this version after primary service discovery,
> >>> secondary services are discovered. Next included
> >>> services are resolved. With this approach we
> >>> don't have recursively search for included service,
> >>> like it was TODO in previous proposal.
> >>> There is also small coding style fix suggested by Arman.
> >>> 
> >>> v4:
> >>> If no secondary services found, continue include services search (fixed
> >>> in gatt-client.c).
> >>> Fixed wrong debug logs (primary->secondary).
> >>> Fixed searching descriptors
> >>> 
> >>> v5:
> >>> Ignore Unsupported Group Type Error in response to secondary service
> >>> discovery and continue included services discovery.
> >>> 
> >>> Marcin Kraglak (14):
> >>>   shared/gatt: Add discover_secondary_services()
> >>>   shared/gatt: Add initial implementation of discover_included_services
> >>>   shared/gatt: Discover included services 128 bit UUIDS
> >>>   shared/gatt: Add extra check in characteristic iterator
> >>>   shared/gatt: Add included service iterator
> >>>   shared/gatt: Remove not needed function parameter
> >>>   shared/gatt: Distinguish Primary from Secondary services
> >>>   tools/btgatt-client: Print type of service
> >>>   shared/gatt: Discover secondary services
> >>>   shared/gatt: Discover included services
> >>>   shared/gatt: Add gatt-client include service iterator
> >>>   tools/btgatt-client: Print found include services
> >>>   shared/gatt: Fix searching descriptors
> >>>   shared/gatt: Add function bt_gatt_result_included_count()
> >>>  
> >>>  src/shared/gatt-client.c  | 263 +++++++++++++++++++++++++++--
> >>>  src/shared/gatt-client.h  |  18 ++
> >>>  src/shared/gatt-helpers.c | 418
> >>>  +++++++++++++++++++++++++++++++++++++++++++---
> >>>  src/shared/gatt-helpers.h |  10 +-
> >>>  tools/btgatt-client.c     |  17 +-
> >>>  5 files changed, 690 insertions(+), 36 deletions(-)
> >>> 
> >>> --
> >>> 1.9.3
> > 
> > Patches looks fine to me, but I would like Arman to ack before applying.
> 
> I'm fine with the patches as they are, though I saw that Szymon has
> left comments on some of them (I'm guessing he's a doing a pass of the
> patch set now). I'm good with it as long as his comments are
> addressed.

I really think we should have more comments and less magic numbers there.
Otherwise this code will be hard to maintain in long term.

> 
> > Btw, there is one thing I would like to see addressed, right now
> > whenever we disconnect the cache is lost which doesn't use the CCC
> > ability to persist across connections, this can cause us to loose
> > notifications when reconnecting because the code don't remember any
> > handles and this is specially bad for profiles such as HoG since we
> > may loose some input events while rediscovering, in fact we should
> > probably store the cache whenever the remote supports service changed
> > and be able to reload them so we only have to do the discover again if
> > something has changed.
> 
> There's an item in the top-level TODO for long-term caching of
> attributes, so I guess this falls under that?
> 
> > --
> > Luiz Augusto von Dentz
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> > in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Cheers,
> -Arman
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCHv5 11/14] shared/gatt: Add gatt-client include service iterator
From: Szymon Janc @ 2014-10-22 18:29 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-12-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Thursday 16 October 2014 12:17:23 Marcin Kraglak wrote:
> It will allow user to take value, handle, start and end handle
> of included service.
> ---
>  src/shared/gatt-client.c | 30 ++++++++++++++++++++++++++++++
>  src/shared/gatt-client.h | 10 ++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 971788c..28865da 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -1674,6 +1674,36 @@ bool bt_gatt_characteristic_iter_next(struct
> bt_gatt_characteristic_iter *iter, return true;
>  }
> 
> +bool bt_gatt_include_service_iter_init(struct bt_gatt_incl_service_iter
> *iter, +					const bt_gatt_service_t *service)
> +{
> +	if (!iter || !service)
> +		return false;
> +
> +	memset(iter, 0, sizeof(*iter));
> +	iter->service = (struct service_list *) service;


I know that this is based on existing code but I'll comment on it here :)

This cast is really confusing. Either it should be casted to void* as service 
field inside iter or service should be const in iter if we are passing const 
data there. Or it should be of type service_list and named services?

This list interface is kinda hard to read and I don't really like it without 
proper macros (like in kernel code)...

> +
> +	return true;
> +}
> +
> +bool bt_gatt_include_service_iter_next(struct bt_gatt_incl_service_iter
> *iter, +					const bt_gatt_included_service_t **incl)
> +{
> +	struct service_list *service;
> +
> +	if (!iter || !incl)
> +		return false;
> +
> +	service = iter->service;
> +
> +	if (iter->pos >= service->num_includes)
> +		return false;
> +
> +	*incl = &service->includes[iter->pos++];
> +
> +	return true;
> +}
> +
>  struct read_op {
>  	bt_gatt_client_read_callback_t callback;
>  	void *user_data;
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index 05b4838..bf4e7bb 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -113,6 +113,11 @@ struct bt_gatt_characteristic_iter {
>  	size_t pos;
>  };
> 
> +struct bt_gatt_incl_service_iter {
> +	void *service;
> +	size_t pos;
> +};
> +
>  bool bt_gatt_service_iter_init(struct bt_gatt_service_iter *iter,
>  						struct bt_gatt_client *client);
>  bool bt_gatt_service_iter_next(struct bt_gatt_service_iter *iter,
> @@ -129,6 +134,11 @@ bool bt_gatt_characteristic_iter_init(struct
> bt_gatt_characteristic_iter *iter, bool
> bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
> const bt_gatt_characteristic_t **chrc);
> 
> +bool bt_gatt_include_service_iter_init(struct bt_gatt_incl_service_iter
> *iter, +					const bt_gatt_service_t *service);
> +bool bt_gatt_include_service_iter_next(struct bt_gatt_incl_service_iter
> *iter, +					const bt_gatt_included_service_t **inc);
> +
>  typedef void (*bt_gatt_client_read_callback_t)(bool success, uint8_t
> att_ecode, const uint8_t *value, uint16_t length,
>  					void *user_data);

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCHv5 10/14] shared/gatt: Discover included services
From: Szymon Janc @ 2014-10-22 18:20 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-11-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Thursday 16 October 2014 12:17:22 Marcin Kraglak wrote:
> ---
>  src/shared/gatt-client.c | 114
> +++++++++++++++++++++++++++++++++++++++++++++-- src/shared/gatt-client.h | 
>  7 +++
>  2 files changed, 117 insertions(+), 4 deletions(-)
> 
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index e04724c..971788c 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -69,6 +69,8 @@ struct service_list {
>  	bt_gatt_service_t service;
>  	struct chrc_data *chrcs;
>  	size_t num_chrcs;
> +	bt_gatt_included_service_t *includes;
> +	size_t num_includes;
>  	struct service_list *next;
>  };
> 
> @@ -253,6 +255,14 @@ static void service_destroy_characteristics(struct
> service_list *service) free(service->chrcs);
>  }
> 
> +static void service_destroy_includes(struct service_list *service)
> +{
> +	free(service->includes);
> +
> +	service->includes = NULL;
> +	service->num_includes = 0;
> +}
> +
>  static void service_list_clear(struct service_list **head,
>  						struct service_list **tail)
>  {
> @@ -265,6 +275,7 @@ static void service_list_clear(struct service_list
> **head,
> 
>  	while (l) {
>  		service_destroy_characteristics(l);
> +		service_destroy_includes(l);
>  		tmp = l;
>  		l = tmp->next;
>  		free(tmp);
> @@ -293,6 +304,7 @@ static void service_list_clear_range(struct service_list
> **head, }
> 
>  		service_destroy_characteristics(cur);
> +		service_destroy_includes(cur);
> 
>  		if (!prev)
>  			*head = cur->next;
> @@ -428,6 +440,99 @@ static int uuid_cmp(const uint8_t uuid128[16], uint16_t
> uuid16) return memcmp(uuid128, rhs_uuid, sizeof(rhs_uuid));
>  }
> 
> +static void discover_incl_cb(bool success, uint8_t att_ecode,
> +		struct bt_gatt_result *result,
> +		void *user_data)

Indentation is broken here and those likely fit in single line.

> +{
> +	struct discovery_op *op = user_data;
> +	struct bt_gatt_client *client = op->client;
> +	struct bt_gatt_iter iter;
> +	char uuid_str[MAX_LEN_UUID_STR];
> +	bt_gatt_included_service_t *includes;
> +	unsigned int includes_count, i;
> +
> +	if (!success) {
> +		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
> +			success = true;
> +			goto next;
> +		}
> +
> +		goto done;
> +	}
> +
> +	if (!result || !bt_gatt_iter_init(&iter, result)) {
> +		success = false;
> +		goto done;
> +	}
> +
> +	includes_count = bt_gatt_result_included_count(result);
> +	if (includes_count == 0) {
> +		success = false;
> +		goto done;
> +	}
> +
> +	includes = new0(bt_gatt_included_service_t, includes_count);
> +	if (!includes) {
> +		success = false;
> +		goto done;
> +	}
> +
> +	util_debug(client->debug_callback, client->debug_data,
> +						"Included services found: %u",
> +						includes_count);
> +
> +	i = 0;
> +	while (bt_gatt_iter_next_included_service(&iter, &includes[i].handle,
> +						&includes[i].start_handle,
> +						&includes[i].end_handle,
> +						includes[i].uuid)) {
> +		uuid_to_string(includes[i].uuid, uuid_str);
> +		util_debug(client->debug_callback, client->debug_data,
> +				"handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
> +				"uuid: %s", includes[i].handle,
> +				includes[i].start_handle,
> +				includes[i].end_handle, uuid_str);
> +		i++;
> +	}

I'd use for(;;) + break here. Also should we verify if this loop iterated 
includes_count times?

> +
> +	op->cur_service->includes = includes;
> +	op->cur_service->num_includes = includes_count;
> +
> +next:
> +	if (!op->cur_service->next) {
> +		op->cur_service = op->result_head;
> +		if (bt_gatt_discover_characteristics(client->att,
> +					op->cur_service->service.start_handle,
> +					op->cur_service->service.end_handle,
> +					discover_chrcs_cb,
> +					discovery_op_ref(op),
> +					discovery_op_unref))
> +			return;
> +
> +		util_debug(client->debug_callback, client->debug_data,
> +				"Failed to start characteristic discovery");
> +		discovery_op_unref(op);
> +		success = false;
> +		goto done;
> +	}
> +
> +	op->cur_service = op->cur_service->next;
> +	if (bt_gatt_discover_included_services(client->att,
> +				op->cur_service->service.start_handle,
> +				op->cur_service->service.end_handle,
> +				discover_incl_cb,
> +				discovery_op_ref(op),
> +				discovery_op_unref))
> +		return;

Empty line here.

> +	util_debug(client->debug_callback, client->debug_data,
> +					"Failed to start included discovery");
> +	discovery_op_unref(op);
> +	success = false;
> +
> +done:
> +	op->complete_func(op, success, att_ecode);

This is always called with success == false so maybe label should be called 
failed and called with hardcoded false?  Or there is some codepath missing for 
success case?

> +}
> +
>  static void discover_descs_cb(bool success, uint8_t att_ecode,
>  						struct bt_gatt_result *result,
>  						void *user_data)
> @@ -532,6 +637,7 @@ done:
>  	op->complete_func(op, success, att_ecode);
>  }
> 
> +
>  static void discover_chrcs_cb(bool success, uint8_t att_ecode,
>  						struct bt_gatt_result *result,
>  						void *user_data)
> @@ -703,18 +809,18 @@ static void discover_secondary_cb(bool success,
> uint8_t att_ecode, }
> 
>  next:
> -	/* Sequentially discover the characteristics of all services */
> +	/* Sequentially discover included services */
>  	op->cur_service = op->result_head;
> -	if (bt_gatt_discover_characteristics(client->att,
> +	if (bt_gatt_discover_included_services(client->att,
>  					op->cur_service->service.start_handle,
>  					op->cur_service->service.end_handle,
> -					discover_chrcs_cb,
> +					discover_incl_cb,
>  					discovery_op_ref(op),
>  					discovery_op_unref))
>  		return;
> 
>  	util_debug(client->debug_callback, client->debug_data,
> -				"Failed to start characteristic discovery");
> +				"Failed to start included services discovery");
>  	discovery_op_unref(op);
>  	success = false;
> 
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index 22d4dc0..05b4838 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -96,6 +96,13 @@ typedef struct {
>  	size_t num_descs;
>  } bt_gatt_characteristic_t;
> 
> +typedef struct {
> +	uint16_t handle;
> +	uint16_t start_handle;
> +	uint16_t end_handle;
> +	uint8_t uuid[BT_GATT_UUID_SIZE];
> +} bt_gatt_included_service_t;
> +
>  struct bt_gatt_service_iter {
>  	struct bt_gatt_client *client;
>  	void *ptr;

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCHv5 08/14] tools/btgatt-client: Print type of service
From: Szymon Janc @ 2014-10-22 18:00 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-9-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Thursday 16 October 2014 12:17:20 Marcin Kraglak wrote:
> ---
>  tools/btgatt-client.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
> index d900e08..5c692ff 100644
> --- a/tools/btgatt-client.c
> +++ b/tools/btgatt-client.c
> @@ -182,8 +182,9 @@ static void print_service(const bt_gatt_service_t
> *service) return;
>  	}
> 
> -	printf(COLOR_RED "service" COLOR_OFF " - start: 0x%04x, "
> +	printf(COLOR_RED "service %s" COLOR_OFF " - start: 0x%04x, "
>  				"end: 0x%04x, uuid: ",
> +				service->primary ? "primary" : "second.",
>  				service->start_handle, service->end_handle);

As we discussed offline, maybe having "type: foo" would be nicer instead of 
"serivce second." ?

>  	print_uuid(service->uuid);

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCHv5 04/14] shared/gatt: Add extra check in characteristic iterator
From: Szymon Janc @ 2014-10-22 17:49 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1413454646-23076-5-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Thursday 16 October 2014 12:17:16 Marcin Kraglak wrote:
> Avoid incorrect reading of included service discovery results.
> ---
>  src/shared/gatt-helpers.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index c18f738..58104d9 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -123,6 +123,9 @@ unsigned int bt_gatt_result_characteristic_count(struct
> bt_gatt_result *result) if (result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
>  		return 0;
> 
> +	if (result->data_len != 21 && result->data_len != 7)
> +		return 0;

This really needs to be commented (or better use meaningful defines instead of 
magic values).

> +
>  	return result_element_count(result);
>  }
> 
> @@ -239,6 +242,9 @@ bool bt_gatt_iter_next_characteristic(struct
> bt_gatt_iter *iter, if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
>  		return false;
> 
> +	if (iter->result->data_len != 21 && iter->result->data_len != 7)
> +		return false;

Ditto.

> +
>  	op = iter->result->op;
>  	pdu_ptr = iter->result->pdu + iter->pos;

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ permalink raw reply

* Re: [PATCH BlueZ 1/8] shared/att: bt_att_cancel should not cancel pending requests/indications.
From: Arman Uguray @ 2014-10-22 15:40 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Arman Uguray, linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZJ6cR80Y0pmPyW_kgVCdssz-JHcLAij8X3+xx4DQb2+6w@mail.gmail.com>

Hi Luiz,


On Wed, Oct 22, 2014 at 7:39 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <armansito@chromium.org> wrote:
>> bt_att_cancel and bt_att_cancel_all no longer destroy any pending
>> request or indication, since this would cause a later bt_att_send to
>> erroneously send out a request or indication before a
>> response/confirmation for a pending one is received. Instead, they now
>> keep the pending operations but clear their response and destroy
>> callbacks since the upper layer is no longer interested in them.
>> ---
>>  src/shared/att.c | 24 ++++++++++++++----------
>>  1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/shared/att.c b/src/shared/att.c
>> index 503e06c..c70d396 100644
>> --- a/src/shared/att.c
>> +++ b/src/shared/att.c
>> @@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
>>                 return false;
>>
>>         if (att->pending_req && att->pending_req->id == id) {
>> -               op = att->pending_req;
>> -               att->pending_req = NULL;
>> -               goto done;
>> +               /* Don't cancel the pending request; remove it's handlers */
>> +               att->pending_req->callback = NULL;
>> +               att->pending_req->destroy = NULL;
>> +               return true;
>>         }
>>
>>         if (att->pending_ind && att->pending_ind->id == id) {
>> -               op = att->pending_ind;
>> -               att->pending_ind = NULL;
>> -               goto done;
>> +               /* Don't cancel the pending indication; remove it's handlers */
>> +               att->pending_ind->callback = NULL;
>> +               att->pending_ind->destroy = NULL;
>> +               return true;
>>         }
>>
>>         op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
>> @@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
>>         queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
>>
>>         if (att->pending_req) {
>> -               destroy_att_send_op(att->pending_req);
>> -               att->pending_req = NULL;
>> +               /* Don't cancel the pending request; remove it's handlers */
>> +               att->pending_req->callback = NULL;
>> +               att->pending_req->destroy = NULL;
>>         }
>>
>>         if (att->pending_ind) {
>> -               destroy_att_send_op(att->pending_ind);
>> -               att->pending_ind = NULL;
>> +               /* Don't cancel the pending indication; remove it's handlers */
>> +               att->pending_ind->callback = NULL;
>> +               att->pending_ind->destroy = NULL;
>>         }
>>
>>         return true;
>> --
>> 2.1.0.rc2.206.gedb03e5
>
> I would like to first finish with Marcin's patches, are you okay with
> v5? Btw, please make sure you follow the HACKING document when
> submitting patches so we can be consistent from now on.
>
>
> --
> Luiz Augusto von Dentz

I'm fine with Marcin's v5, though we may have to wait for v6 since I
saw some comments fly by from Szymon.

Yeah I'm following HACKING from now on. So far I used the code around
me (shared/mgmt, etc) as the style guide and might have repeated
mistakes that already existed in the code, so it might makes sense to
do a general style fix pass within src/shared at some point.

-Arman

^ permalink raw reply

* Re: [PATCHv5 00/14] Included service discovery
From: Arman Uguray @ 2014-10-22 15:35 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcin Kraglak, linux-bluetooth@vger.kernel.org development
In-Reply-To: <CABBYNZKQ-1s9p_18HOnKW3Sew+cMrW+6SnPJHwp-s5m5nYF3QQ@mail.gmail.com>

Hi Luiz,

On Wed, Oct 22, 2014 at 7:54 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Marcin,
>
> On Wed, Oct 22, 2014 at 9:25 AM, Marcin Kraglak
> <marcin.kraglak@tieto.com> wrote:
>> On 16 October 2014 12:17, Marcin Kraglak <marcin.kraglak@tieto.com> wrote:
>>> v3:
>>> In this version after primary service discovery,
>>> secondary services are discovered. Next included
>>> services are resolved. With this approach we
>>> don't have recursively search for included service,
>>> like it was TODO in previous proposal.
>>> There is also small coding style fix suggested by Arman.
>>>
>>> v4:
>>> If no secondary services found, continue include services search (fixed
>>> in gatt-client.c).
>>> Fixed wrong debug logs (primary->secondary).
>>> Fixed searching descriptors
>>>
>>> v5:
>>> Ignore Unsupported Group Type Error in response to secondary service
>>> discovery and continue included services discovery.
>>>
>>> Marcin Kraglak (14):
>>>   shared/gatt: Add discover_secondary_services()
>>>   shared/gatt: Add initial implementation of discover_included_services
>>>   shared/gatt: Discover included services 128 bit UUIDS
>>>   shared/gatt: Add extra check in characteristic iterator
>>>   shared/gatt: Add included service iterator
>>>   shared/gatt: Remove not needed function parameter
>>>   shared/gatt: Distinguish Primary from Secondary services
>>>   tools/btgatt-client: Print type of service
>>>   shared/gatt: Discover secondary services
>>>   shared/gatt: Discover included services
>>>   shared/gatt: Add gatt-client include service iterator
>>>   tools/btgatt-client: Print found include services
>>>   shared/gatt: Fix searching descriptors
>>>   shared/gatt: Add function bt_gatt_result_included_count()
>>>
>>>  src/shared/gatt-client.c  | 263 +++++++++++++++++++++++++++--
>>>  src/shared/gatt-client.h  |  18 ++
>>>  src/shared/gatt-helpers.c | 418 +++++++++++++++++++++++++++++++++++++++++++---
>>>  src/shared/gatt-helpers.h |  10 +-
>>>  tools/btgatt-client.c     |  17 +-
>>>  5 files changed, 690 insertions(+), 36 deletions(-)
>>>
>>> --
>>> 1.9.3
>>>
>
> Patches looks fine to me, but I would like Arman to ack before applying.
>

I'm fine with the patches as they are, though I saw that Szymon has
left comments on some of them (I'm guessing he's a doing a pass of the
patch set now). I'm good with it as long as his comments are
addressed.


> Btw, there is one thing I would like to see addressed, right now
> whenever we disconnect the cache is lost which doesn't use the CCC
> ability to persist across connections, this can cause us to loose
> notifications when reconnecting because the code don't remember any
> handles and this is specially bad for profiles such as HoG since we
> may loose some input events while rediscovering, in fact we should
> probably store the cache whenever the remote supports service changed
> and be able to reload them so we only have to do the discover again if
> something has changed.
>

There's an item in the top-level TODO for long-term caching of
attributes, so I guess this falls under that?


>
> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Cheers,
-Arman

^ permalink raw reply

* [PATCH] hciattach: bcm43xx: Use final baudrate to download the firmware
From: Loic Poulain @ 2014-10-22 15:08 UTC (permalink / raw)
  To: johan.hedberg, marcel; +Cc: linux-bluetooth, Loic Poulain

Speed up bcm43xx init by setting the final baudrate before firmware
download.
---
 tools/hciattach.c         |  2 +-
 tools/hciattach.h         |  3 ++-
 tools/hciattach_bcm43xx.c | 63 +++++++++++++++++++++++++++++------------------
 3 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/tools/hciattach.c b/tools/hciattach.c
index bf927fc..14d3511 100644
--- a/tools/hciattach.c
+++ b/tools/hciattach.c
@@ -329,7 +329,7 @@ static int intel(int fd, struct uart_t *u, struct termios *ti)
 
 static int bcm43xx(int fd, struct uart_t *u, struct termios *ti)
 {
-	return bcm43xx_init(fd, u->speed, ti, u->bdaddr);
+	return bcm43xx_init(fd, u->init_speed, u->speed, ti, u->bdaddr);
 }
 
 static int read_check(int fd, void *buf, int count)
diff --git a/tools/hciattach.h b/tools/hciattach.h
index 4810a09..2aaf075 100644
--- a/tools/hciattach.h
+++ b/tools/hciattach.h
@@ -58,4 +58,5 @@ int ath3k_init(int fd, int speed, int init_speed, char *bdaddr,
 int ath3k_post(int fd, int pm);
 int qualcomm_init(int fd, int speed, struct termios *ti, const char *bdaddr);
 int intel_init(int fd, int init_speed, int *speed, struct termios *ti);
-int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr);
+int bcm43xx_init(int fd, int def_speed, int speed, struct termios *ti,
+		const char *bdaddr);
diff --git a/tools/hciattach_bcm43xx.c b/tools/hciattach_bcm43xx.c
index cb4bfb9..3d36c20 100644
--- a/tools/hciattach_bcm43xx.c
+++ b/tools/hciattach_bcm43xx.c
@@ -154,63 +154,71 @@ static int bcm43xx_set_bdaddr(int fd, const char *bdaddr)
 	return 0;
 }
 
-static int bcm43xx_set_speed(int fd, uint32_t speed)
+static int bcm43xx_set_clock(int fd, unsigned char clock)
 {
-	unsigned char cmd[] =
-		{ HCI_COMMAND_PKT, 0x18, 0xfc, 0x06, 0x00, 0x00,
-				0x00, 0x00, 0x00, 0x00 };
+	unsigned char cmd[] = { HCI_COMMAND_PKT, 0x45, 0xfc, 0x01, 0x00 };
 	unsigned char resp[CC_MIN_SIZE];
 
-	printf("Set Controller UART speed to %d bit/s\n", speed);
+	printf("Set Controller clock (%d)\n", clock);
 
-	cmd[6] = (uint8_t) (speed);
-	cmd[7] = (uint8_t) (speed >> 8);
-	cmd[8] = (uint8_t) (speed >> 16);
-	cmd[9] = (uint8_t) (speed >> 24);
+	cmd[4] = clock;
 
 	tcflush(fd, TCIOFLUSH);
 
 	if (write(fd, cmd, sizeof(cmd)) != sizeof(cmd)) {
-		fprintf(stderr, "Failed to write update baudrate command\n");
+		fprintf(stderr, "Failed to write update clock command\n");
 		return -1;
 	}
 
 	if (read_hci_event(fd, resp, sizeof(resp)) < CC_MIN_SIZE) {
-		fprintf(stderr, "Failed to update baudrate, invalid HCI event\n");
+		fprintf(stderr, "Failed to update clock, invalid HCI event\n");
 		return -1;
 	}
 
 	if (resp[4] != cmd[1] || resp[5] != cmd[2] || resp[6] != CMD_SUCCESS) {
-		fprintf(stderr, "Failed to update baudrate, command failure\n");
+		fprintf(stderr, "Failed to update clock, command failure\n");
 		return -1;
 	}
 
 	return 0;
 }
 
-static int bcm43xx_set_clock(int fd, unsigned char clock)
+static int bcm43xx_set_speed(int fd, struct termios *ti, uint32_t speed)
 {
-	unsigned char cmd[] = { HCI_COMMAND_PKT, 0x45, 0xfc, 0x01, 0x00 };
+	unsigned char cmd[] =
+		{ HCI_COMMAND_PKT, 0x18, 0xfc, 0x06, 0x00, 0x00,
+				0x00, 0x00, 0x00, 0x00 };
 	unsigned char resp[CC_MIN_SIZE];
 
-	printf("Set Controller clock (%d)\n", clock);
+	if (speed > 3000000 && bcm43xx_set_clock(fd, BCM43XX_CLOCK_48))
+		return -1;
 
-	cmd[4] = clock;
+	printf("Set Controller UART speed to %d bit/s\n", speed);
+
+	cmd[6] = (uint8_t) (speed);
+	cmd[7] = (uint8_t) (speed >> 8);
+	cmd[8] = (uint8_t) (speed >> 16);
+	cmd[9] = (uint8_t) (speed >> 24);
 
 	tcflush(fd, TCIOFLUSH);
 
 	if (write(fd, cmd, sizeof(cmd)) != sizeof(cmd)) {
-		fprintf(stderr, "Failed to write update clock command\n");
+		fprintf(stderr, "Failed to write update baudrate command\n");
 		return -1;
 	}
 
 	if (read_hci_event(fd, resp, sizeof(resp)) < CC_MIN_SIZE) {
-		fprintf(stderr, "Failed to update clock, invalid HCI event\n");
+		fprintf(stderr, "Failed to update baudrate, invalid HCI event\n");
 		return -1;
 	}
 
 	if (resp[4] != cmd[1] || resp[5] != cmd[2] || resp[6] != CMD_SUCCESS) {
-		fprintf(stderr, "Failed to update clock, command failure\n");
+		fprintf(stderr, "Failed to update baudrate, command failure\n");
+		return -1;
+	}
+
+	if (set_speed(fd, ti, speed) < 0) {
+		perror("Can't set host baud rate");
 		return -1;
 	}
 
@@ -343,7 +351,8 @@ static int bcm43xx_locate_patch(const char *dir_name,
 	return ret;
 }
 
-int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
+int bcm43xx_init(int fd, int def_speed, int speed, struct termios *ti,
+		const char *bdaddr)
 {
 	char chip_name[20];
 	char fw_path[PATH_MAX];
@@ -359,9 +368,18 @@ int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
 	if (bcm43xx_locate_patch(FIRMWARE_DIR, chip_name, fw_path)) {
 		fprintf(stderr, "Patch not found, continue anyway\n");
 	} else {
+		if (bcm43xx_set_speed(fd, ti, speed))
+			return -1;
+
 		if (bcm43xx_load_firmware(fd, fw_path))
 			return -1;
 
+		/* Controller speed has been reset to def speed */
+		if (set_speed(fd, ti, def_speed) < 0) {
+			perror("Can't set host baud rate");
+			return -1;
+		}
+
 		if (bcm43xx_reset(fd))
 			return -1;
 	}
@@ -369,10 +387,7 @@ int bcm43xx_init(int fd, int speed, struct termios *ti, const char *bdaddr)
 	if (bdaddr)
 		bcm43xx_set_bdaddr(fd, bdaddr);
 
-	if (speed > 3000000 && bcm43xx_set_clock(fd, BCM43XX_CLOCK_48))
-		return -1;
-
-	if (bcm43xx_set_speed(fd, speed))
+	if (bcm43xx_set_speed(fd, ti, speed))
 		return -1;
 
 	return 0;
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCHv5 00/14] Included service discovery
From: Luiz Augusto von Dentz @ 2014-10-22 14:54 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <CABD6X-+URkwy0eEVMoUusc3ymFb6fdnFZ0z+jfPdVNq+ibXRQQ@mail.gmail.com>

Hi Marcin,

On Wed, Oct 22, 2014 at 9:25 AM, Marcin Kraglak
<marcin.kraglak@tieto.com> wrote:
> On 16 October 2014 12:17, Marcin Kraglak <marcin.kraglak@tieto.com> wrote:
>> v3:
>> In this version after primary service discovery,
>> secondary services are discovered. Next included
>> services are resolved. With this approach we
>> don't have recursively search for included service,
>> like it was TODO in previous proposal.
>> There is also small coding style fix suggested by Arman.
>>
>> v4:
>> If no secondary services found, continue include services search (fixed
>> in gatt-client.c).
>> Fixed wrong debug logs (primary->secondary).
>> Fixed searching descriptors
>>
>> v5:
>> Ignore Unsupported Group Type Error in response to secondary service
>> discovery and continue included services discovery.
>>
>> Marcin Kraglak (14):
>>   shared/gatt: Add discover_secondary_services()
>>   shared/gatt: Add initial implementation of discover_included_services
>>   shared/gatt: Discover included services 128 bit UUIDS
>>   shared/gatt: Add extra check in characteristic iterator
>>   shared/gatt: Add included service iterator
>>   shared/gatt: Remove not needed function parameter
>>   shared/gatt: Distinguish Primary from Secondary services
>>   tools/btgatt-client: Print type of service
>>   shared/gatt: Discover secondary services
>>   shared/gatt: Discover included services
>>   shared/gatt: Add gatt-client include service iterator
>>   tools/btgatt-client: Print found include services
>>   shared/gatt: Fix searching descriptors
>>   shared/gatt: Add function bt_gatt_result_included_count()
>>
>>  src/shared/gatt-client.c  | 263 +++++++++++++++++++++++++++--
>>  src/shared/gatt-client.h  |  18 ++
>>  src/shared/gatt-helpers.c | 418 +++++++++++++++++++++++++++++++++++++++++++---
>>  src/shared/gatt-helpers.h |  10 +-
>>  tools/btgatt-client.c     |  17 +-
>>  5 files changed, 690 insertions(+), 36 deletions(-)
>>
>> --
>> 1.9.3
>>

Patches looks fine to me, but I would like Arman to ack before applying.

Btw, there is one thing I would like to see addressed, right now
whenever we disconnect the cache is lost which doesn't use the CCC
ability to persist across connections, this can cause us to loose
notifications when reconnecting because the code don't remember any
handles and this is specially bad for profiles such as HoG since we
may loose some input events while rediscovering, in fact we should
probably store the cache whenever the remote supports service changed
and be able to reload them so we only have to do the discover again if
something has changed.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH BlueZ 1/8] shared/att: bt_att_cancel should not cancel pending requests/indications.
From: Luiz Augusto von Dentz @ 2014-10-22 14:39 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1413838861-29956-2-git-send-email-armansito@chromium.org>

Hi Arman,

On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <armansito@chromium.org> wrote:
> bt_att_cancel and bt_att_cancel_all no longer destroy any pending
> request or indication, since this would cause a later bt_att_send to
> erroneously send out a request or indication before a
> response/confirmation for a pending one is received. Instead, they now
> keep the pending operations but clear their response and destroy
> callbacks since the upper layer is no longer interested in them.
> ---
>  src/shared/att.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 503e06c..c70d396 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
>                 return false;
>
>         if (att->pending_req && att->pending_req->id == id) {
> -               op = att->pending_req;
> -               att->pending_req = NULL;
> -               goto done;
> +               /* Don't cancel the pending request; remove it's handlers */
> +               att->pending_req->callback = NULL;
> +               att->pending_req->destroy = NULL;
> +               return true;
>         }
>
>         if (att->pending_ind && att->pending_ind->id == id) {
> -               op = att->pending_ind;
> -               att->pending_ind = NULL;
> -               goto done;
> +               /* Don't cancel the pending indication; remove it's handlers */
> +               att->pending_ind->callback = NULL;
> +               att->pending_ind->destroy = NULL;
> +               return true;
>         }
>
>         op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
> @@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
>         queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
>
>         if (att->pending_req) {
> -               destroy_att_send_op(att->pending_req);
> -               att->pending_req = NULL;
> +               /* Don't cancel the pending request; remove it's handlers */
> +               att->pending_req->callback = NULL;
> +               att->pending_req->destroy = NULL;
>         }
>
>         if (att->pending_ind) {
> -               destroy_att_send_op(att->pending_ind);
> -               att->pending_ind = NULL;
> +               /* Don't cancel the pending indication; remove it's handlers */
> +               att->pending_ind->callback = NULL;
> +               att->pending_ind->destroy = NULL;
>         }
>
>         return true;
> --
> 2.1.0.rc2.206.gedb03e5

I would like to first finish with Marcin's patches, are you okay with
v5? Btw, please make sure you follow the HACKING document when
submitting patches so we can be consistent from now on.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH v3] obexd/mas: Add Support for MSETime filter
From: Luiz Augusto von Dentz @ 2014-10-22 14:12 UTC (permalink / raw)
  To: Bharat Panda; +Cc: linux-bluetooth@vger.kernel.org, cpgs
In-Reply-To: <1413896899-29802-1-git-send-email-bharat.panda@samsung.com>

Hi,

On Tue, Oct 21, 2014 at 4:08 PM, Bharat Panda <bharat.panda@samsung.com> wrote:
> Changes made to add support for MSE local time and timezone offset
> parameter along with GetMessageListing response.
> ---
>  obexd/plugins/mas.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/obexd/plugins/mas.c b/obexd/plugins/mas.c
> index fb97fe3..5b95b3a 100644
> --- a/obexd/plugins/mas.c
> +++ b/obexd/plugins/mas.c
> @@ -30,6 +30,7 @@
>  #include <glib.h>
>  #include <fcntl.h>
>  #include <inttypes.h>
> +#include <sys/time.h>
>
>  #include <gobex/gobex.h>
>  #include <gobex/gobex-apparam.h>
> @@ -228,6 +229,34 @@ static void g_string_append_escaped_printf(GString *string,
>         va_end(ap);
>  }
>
> +static gchar *get_mse_timestamp(void)
> +{
> +       struct timeval time_val;
> +       struct tm ltime;
> +       gchar *local_ts;
> +       char sign;
> +
> +       gettimeofday(&time_val, NULL);
> +
> +       if (!localtime_r(&time_val.tv_sec, &ltime))
> +               return NULL;
> +
> +       if (difftime(mktime(localtime(&time_val.tv_sec)),
> +                               mktime(gmtime(&time_val.tv_sec))) < 0)
> +               sign = '+';
> +       else
> +               sign = '-';
> +
> +       local_ts = g_strdup_printf("%04d%02d%02dT%02d%02d%02d%c%2ld%2ld",
> +                                       ltime.tm_year + 1900, ltime.tm_mon + 1,
> +                                       ltime.tm_mday, ltime.tm_hour,
> +                                       ltime.tm_min, ltime.tm_sec, sign,
> +                                       ltime.tm_gmtoff/3600,
> +                                       (ltime.tm_gmtoff%3600)/60);
> +
> +       return local_ts;
> +}
> +
>  static const char *yesorno(gboolean a)
>  {
>         if (a)
> @@ -243,6 +272,7 @@ static void get_messages_listing_cb(void *session, int err, uint16_t size,
>  {
>         struct mas_session *mas = user_data;
>         uint16_t max = 1024;
> +       gchar *mse_time;
>
>         if (err < 0 && err != -EAGAIN) {
>                 obex_object_set_io_flags(mas, G_IO_ERR, err);
> @@ -358,6 +388,13 @@ proceed:
>                 mas->outparams = g_obex_apparam_set_uint8(mas->outparams,
>                                                 MAP_AP_NEWMESSAGE,
>                                                 newmsg ? 1 : 0);
> +               /* Response to report the local time of MSE */
> +               mse_time = get_mse_timestamp();
> +               if (mse_time) {
> +                       g_obex_apparam_set_string(mas->outparams,
> +                                               MAP_AP_MSETIME, mse_time);
> +                       g_free(mse_time);
> +               }
>         }
>
>         if (err != -EAGAIN)
> --
> 1.9.1

Applied, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [RFC] Bluetooth : Fix hci_sync miss wakeup interrupt
From: 박찬열 @ 2014-10-22 13:48 UTC (permalink / raw)
  To: linux-bluetooth

DQpfX2hjaV9jbWRfc3luY19ldigpLCBfX2hjaV9yZXFfc3luYygpIGNvdWxkIG1pc3Mgd2FrZV91
cF9pbnRlcnJ1cHQgZnJvbQ0KaGNpX3JlcV9zeW5jX2NvbXBsZXRlKCkgYmVjYXVzZSBoY2lfY21k
X3dvcmsoKSB3b3JrcXVlZSBhbmQgaXRzIHJlcG9uc2UNCmNvdWxkIGJlIGNvbXBsZXRlZCBiZWZv
cmUgdGhleSBhcmUgcmVhZHkgdG8gZ2V0IHRoZSBzaWduYWwgdGhyb3VnaA0KYWRkX3dhaXRfcXVl
dWUoKSwgc2V0X2N1cnJlbnRfc3RhdGUoVEFTS19JTlRFUlJVUFRJQkxFKS4NCg0KU2lnbmVkLW9m
Zi1ieTogQ2hhbi15ZW9sIFBhcmsgPGNoYW55ZW9sLnBhcmtAc2Ftc3VuZy5jb20+DQpTaWduZWQt
b2ZmLWJ5OiBLeXVuZ21pbiBQYXJrIDxreXVuZ21pbi5wYXJrQHNhbXN1bmcuY29tPg0KLS0tDQog
bmV0L2JsdWV0b290aC9oY2lfY29yZS5jIHwgMTggKysrKysrKysrKystLS0tLS0tDQogMSBmaWxl
IGNoYW5nZWQsIDExIGluc2VydGlvbnMoKyksIDcgZGVsZXRpb25zKC0pDQoNCmRpZmYgLS1naXQg
YS9uZXQvYmx1ZXRvb3RoL2hjaV9jb3JlLmMgYi9uZXQvYmx1ZXRvb3RoL2hjaV9jb3JlLmMNCmlu
ZGV4IGNiMDVkN2YuLmMwMDhmMWYgMTAwNjQ0DQotLS0gYS9uZXQvYmx1ZXRvb3RoL2hjaV9jb3Jl
LmMNCisrKyBiL25ldC9ibHVldG9vdGgvaGNpX2NvcmUuYw0KQEAgLTExNDcsMTMgKzExNDcsMTUg
QEAgc3RydWN0IHNrX2J1ZmYgKl9faGNpX2NtZF9zeW5jX2V2KHN0cnVjdCBoY2lfZGV2ICpoZGV2
LCB1MTYgb3Bjb2RlLCB1MzIgcGxlbiwNCg0KICAgICAgICBoZGV2LT5yZXFfc3RhdHVzID0gSENJ
X1JFUV9QRU5EOw0KDQotICAgICAgIGVyciA9IGhjaV9yZXFfcnVuKCZyZXEsIGhjaV9yZXFfc3lu
Y19jb21wbGV0ZSk7DQotICAgICAgIGlmIChlcnIgPCAwKQ0KLSAgICAgICAgICAgICAgIHJldHVy
biBFUlJfUFRSKGVycik7DQotDQogICAgICAgIGFkZF93YWl0X3F1ZXVlKCZoZGV2LT5yZXFfd2Fp
dF9xLCAmd2FpdCk7DQogICAgICAgIHNldF9jdXJyZW50X3N0YXRlKFRBU0tfSU5URVJSVVBUSUJM
RSk7DQoNCisgICAgICAgZXJyID0gaGNpX3JlcV9ydW4oJnJlcSwgaGNpX3JlcV9zeW5jX2NvbXBs
ZXRlKTsNCisgICAgICAgaWYgKGVyciA8IDApIHsNCisgICAgICAgICAgICAgICByZW1vdmVfd2Fp
dF9xdWV1ZSgmaGRldi0+cmVxX3dhaXRfcSwgJndhaXQpOw0KKyAgICAgICAgICAgICAgIHJldHVy
biBFUlJfUFRSKGVycik7DQorICAgICAgIH0NCisNCiAgICAgICAgc2NoZWR1bGVfdGltZW91dCh0
aW1lb3V0KTsNCg0KICAgICAgICByZW1vdmVfd2FpdF9xdWV1ZSgmaGRldi0+cmVxX3dhaXRfcSwg
JndhaXQpOw0KQEAgLTEyMTEsMTAgKzEyMTMsMTUgQEAgc3RhdGljIGludCBfX2hjaV9yZXFfc3lu
YyhzdHJ1Y3QgaGNpX2RldiAqaGRldiwNCg0KICAgICAgICBmdW5jKCZyZXEsIG9wdCk7DQoNCisg
ICAgICAgYWRkX3dhaXRfcXVldWUoJmhkZXYtPnJlcV93YWl0X3EsICZ3YWl0KTsNCisgICAgICAg
c2V0X2N1cnJlbnRfc3RhdGUoVEFTS19JTlRFUlJVUFRJQkxFKTsNCisNCiAgICAgICAgZXJyID0g
aGNpX3JlcV9ydW4oJnJlcSwgaGNpX3JlcV9zeW5jX2NvbXBsZXRlKTsNCiAgICAgICAgaWYgKGVy
ciA8IDApIHsNCiAgICAgICAgICAgICAgICBoZGV2LT5yZXFfc3RhdHVzID0gMDsNCg0KKyAgICAg
ICAgICAgICAgIHJlbW92ZV93YWl0X3F1ZXVlKCZoZGV2LT5yZXFfd2FpdF9xLCAmd2FpdCk7DQor
DQogICAgICAgICAgICAgICAgLyogRU5PREFUQSBtZWFucyB0aGUgSENJIHJlcXVlc3QgY29tbWFu
ZCBxdWV1ZSBpcyBlbXB0eS4NCiAgICAgICAgICAgICAgICAgKiBUaGlzIGNhbiBoYXBwZW4gd2hl
biBhIHJlcXVlc3Qgd2l0aCBjb25kaXRpb25hbHMgZG9lc24ndA0KICAgICAgICAgICAgICAgICAq
IHRyaWdnZXIgYW55IGNvbW1hbmRzIHRvIGJlIHNlbnQuIFRoaXMgaXMgbm9ybWFsIGJlaGF2aW9y
DQpAQCAtMTIyNiw5ICsxMjMzLDYgQEAgc3RhdGljIGludCBfX2hjaV9yZXFfc3luYyhzdHJ1Y3Qg
aGNpX2RldiAqaGRldiwNCiAgICAgICAgICAgICAgICByZXR1cm4gZXJyOw0KICAgICAgICB9DQoN
Ci0gICAgICAgYWRkX3dhaXRfcXVldWUoJmhkZXYtPnJlcV93YWl0X3EsICZ3YWl0KTsNCi0gICAg
ICAgc2V0X2N1cnJlbnRfc3RhdGUoVEFTS19JTlRFUlJVUFRJQkxFKTsNCi0NCiAgICAgICAgc2No
ZWR1bGVfdGltZW91dCh0aW1lb3V0KTsNCg0KICAgICAgICByZW1vdmVfd2FpdF9xdWV1ZSgmaGRl
di0+cmVxX3dhaXRfcSwgJndhaXQpOw0KLS0NCjEuOS4xIA0KICAgICAgIA0KDQogIA0KDQogICAg
ICAgICANCiAgDQo=



^ permalink raw reply

* Re: rtk_btusb issues
From: Patrick Shirkey @ 2014-10-22 13:42 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <7FC77530-F48C-436C-8954-195E9CD08E97@holtmann.org>


On Thu, October 23, 2014 12:05 am, Marcel Holtmann wrote:
> Hi Patrick,
>
>> I have an android device running the rtk_btusb driver. The original
>> driver
>> was a couple of years old and there was a problem with stability with
>> BLE.
>> In addition the localname was not found in the scan_response data.
>>
>> I upgraded the bluetooth stack by manually backporting from a recent
>> kernel. That fixed the issue wit the scan_repsonse data but there were
>> still stability issues with BLE devices (although slightly improved
>> compared to the older bluetooth stack).
>>
>> I have tried upgrading the driver to the latest version of the rtk_btusb
>> driver from the git repo and ported the hooks for bluedroid from the old
>> driver however I get issues with hci send command when loading the
>> driver.
>>
>> http://pastebin.com/hXALmXBr
>>
>> I traced the command but I am not sure where the send value is
>> generated.
>> I can see the function definition in the hci_dev struct but not the
>> location where the value is actually set.
>>
>> I thought it might be an issue specific to bluedroid so I installed
>> bluez
>> but the issue is still there so it appears to be a bug in my version of
>> the driver or something wrong with the bluetooth kernel layer.
>>
>> I went back to the older version of the driver running against  bluez
>> instead of bluedroid. That gets me further along but I still cannot
>> initialise the bluetooth system on the device.
>>
>> http://pastebin.com/HcSZXiMu
>>
>> Does anyone have any suggestions for how to go about resolving this
>> situation I have got myself into?
>
> the rtk_btusb driver is not an upstream driver and that is the main
> problem. There has been recently an effort to get btusb support the
> Realtek devices, but then it went silent again.
>
> What you need is having proper Realtek support in btusb and nothing else.
> Out of tree hacked up vendor drivers are not something any of us will
> likely have a look at and trying to fix.
>

My quest is to fix the stability issue not the rtk_btusb driver.  I
spotted the "new" branch in the git tree. I am testing that driver now.

Do you know if there are known issues with BLE stability on the 8723au
chipset?




--
Patrick Shirkey
Boost Hardware Ltd

^ permalink raw reply

* Re: [PATCH 3/3] bnep: Return errno instead of -1 and print error
From: Andrei Emeltchenko @ 2014-10-22 13:37 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
In-Reply-To: <CAMv5Gbck+=b9kY-s+gd3shJsLUDAEZib5dKGM09ZBWkP==ufFw@mail.gmail.com>

Hi Grzegorz,

On Wed, Oct 22, 2014 at 02:55:54PM +0200, Grzegorz Kolodziejczyk wrote:
> Hi Andrei,
> 
> On 22 October 2014 13:10, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Make code consistent with the rest returning -errno and printing error
> > message.
> > ---
> >  profiles/network/bnep.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> > index 4cf38d9..09d4b65 100644
> > --- a/profiles/network/bnep.c
> > +++ b/profiles/network/bnep.c
> > @@ -231,7 +231,7 @@ static int bnep_if_down(const char *devname)
> >         if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
> >                 err = -errno;
> >                 error("bnep: Could not bring down %s: %s(%d)",
> > -                               devname, strerror(-err), -err);
> > +                                               devname, strerror(-err), -err);
> 
> Why don't you fix this indention in first patch ? (this line is added
> in previous patch)

Sorry, resend the series.

Best regards 
Andrei Emeltchenko 


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox