* [PATCH BlueZ 0/4] android fixes and gattrib shim
@ 2014-11-12 20:22 Michael Janssen
2014-11-12 20:22 ` [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs Michael Janssen
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Michael Janssen @ 2014-11-12 20:22 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Michael Janssen
This patchset fixes some issues with Android GATT testing, a minor
issue with Android GATT exposed by the shim, and provides a
GAttrib shim using bt_att.
Passes all android/android-tester, unit/test-gattrib
Michael Janssen (4):
android/tester-gatt: DRY/bugfix search single PDUs
android/tester-gatt: deduplicate read-by-type PDUs
android/gatt: dummy callback for indications
GATT shim to src/shared bt_att
android/gatt.c | 15 +-
android/tester-gatt.c | 167 +++---------
attrib/gattrib.c | 733 +++++++++++---------------------------------------
3 files changed, 208 insertions(+), 707 deletions(-)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs 2014-11-12 20:22 [PATCH BlueZ 0/4] android fixes and gattrib shim Michael Janssen @ 2014-11-12 20:22 ` Michael Janssen 2014-11-12 20:51 ` Szymon Janc 2014-11-12 20:22 ` [PATCH BlueZ 2/4] android/tester-gatt: deduplicate read-by-type PDUs Michael Janssen ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Michael Janssen @ 2014-11-12 20:22 UTC (permalink / raw) To: linux-bluetooth; +Cc: Michael Janssen The search service with a single service response PDUs are repeated multiple times throughout the testing data. Use a macro instead. Also, the final PDU was malformed on many of these (had 0x11 instead of 0x10 in the "request opcode in error" slot) which this fixes. --- android/tester-gatt.c | 106 +++++++++++++------------------------------------- 1 file changed, 26 insertions(+), 80 deletions(-) diff --git a/android/tester-gatt.c b/android/tester-gatt.c index 13e096f..b88eeff 100644 --- a/android/tester-gatt.c +++ b/android/tester-gatt.c @@ -832,11 +832,14 @@ static struct send_resp_data send_resp_data_2 = { .response = &response_2, }; +#define SEARCH_SERVICE_SINGLE_SUCCESS_PDUS \ + raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), \ + raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), \ + raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), \ + raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a) \ + static struct iovec search_service[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, end_pdu }; @@ -857,10 +860,7 @@ static struct iovec search_service_3[] = { }; static struct iovec get_characteristic_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -869,10 +869,7 @@ static struct iovec get_characteristic_1[] = { }; static struct iovec get_descriptor_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -885,10 +882,7 @@ static struct iovec get_descriptor_1[] = { }; static struct iovec get_descriptor_2[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -901,10 +895,7 @@ static struct iovec get_descriptor_2[] = { }; static struct iovec get_descriptor_3[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -915,10 +906,7 @@ static struct iovec get_descriptor_3[] = { }; static struct iovec get_included_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), raw_pdu(0x09, 0x08, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00, 0xff, 0xfe), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x02, 0x28), @@ -927,10 +915,7 @@ static struct iovec get_included_1[] = { }; static struct iovec get_included_2[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), raw_pdu(0x09, 0x06, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00), raw_pdu(0x0a, 0x15, 0x00), @@ -942,20 +927,14 @@ static struct iovec get_included_2[] = { }; static struct iovec get_included_3[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a), end_pdu }; static struct iovec read_characteristic_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -966,10 +945,7 @@ static struct iovec read_characteristic_1[] = { }; static struct iovec read_characteristic_2[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -980,10 +956,7 @@ static struct iovec read_characteristic_2[] = { }; static struct iovec read_descriptor_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -998,10 +971,7 @@ static struct iovec read_descriptor_1[] = { }; static struct iovec read_descriptor_2[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1016,10 +986,7 @@ static struct iovec read_descriptor_2[] = { }; static struct iovec write_characteristic_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1029,10 +996,7 @@ static struct iovec write_characteristic_1[] = { }; static struct iovec write_characteristic_2[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1043,10 +1007,7 @@ static struct iovec write_characteristic_2[] = { }; static struct iovec write_characteristic_3[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1057,10 +1018,7 @@ static struct iovec write_characteristic_3[] = { }; static struct iovec write_descriptor_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1075,10 +1033,7 @@ static struct iovec write_descriptor_1[] = { }; static struct iovec write_descriptor_2[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1093,10 +1048,7 @@ static struct iovec write_descriptor_2[] = { }; static struct iovec notification_1[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1105,10 +1057,7 @@ static struct iovec notification_1[] = { }; static struct iovec notification_2[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), @@ -1119,10 +1068,7 @@ static struct iovec notification_2[] = { }; static struct iovec notification_3[] = { - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs 2014-11-12 20:22 ` [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs Michael Janssen @ 2014-11-12 20:51 ` Szymon Janc 2014-11-13 15:30 ` Michael Janssen 0 siblings, 1 reply; 16+ messages in thread From: Szymon Janc @ 2014-11-12 20:51 UTC (permalink / raw) To: Michael Janssen; +Cc: linux-bluetooth HI Michael, On Wednesday 12 of November 2014 12:22:56 Michael Janssen wrote: This "DRY/" in commit subject is intended? > The search service with a single service response PDUs are repeated > multiple times throughout the testing data. Use a macro instead. > > Also, the final PDU was malformed on many of these (had 0x11 instead of > 0x10 in the "request opcode in error" slot) which this fixes. > --- > android/tester-gatt.c | 106 > +++++++++++++------------------------------------- 1 file changed, 26 > insertions(+), 80 deletions(-) > > diff --git a/android/tester-gatt.c b/android/tester-gatt.c > index 13e096f..b88eeff 100644 > --- a/android/tester-gatt.c > +++ b/android/tester-gatt.c > @@ -832,11 +832,14 @@ static struct send_resp_data send_resp_data_2 = { > .response = &response_2, > }; > > +#define SEARCH_SERVICE_SINGLE_SUCCESS_PDUS \ > + raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), \ > + raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), \ > + raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), \ > + raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a) \ > + > static struct iovec search_service[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > end_pdu > }; > > @@ -857,10 +860,7 @@ static struct iovec search_service_3[] = { > }; > > static struct iovec get_characteristic_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -869,10 +869,7 @@ static struct iovec get_characteristic_1[] = { > }; > > static struct iovec get_descriptor_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -885,10 +882,7 @@ static struct iovec get_descriptor_1[] = { > }; > > static struct iovec get_descriptor_2[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -901,10 +895,7 @@ static struct iovec get_descriptor_2[] = { > }; > > static struct iovec get_descriptor_3[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -915,10 +906,7 @@ static struct iovec get_descriptor_3[] = { > }; > > static struct iovec get_included_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), > raw_pdu(0x09, 0x08, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00, 0xff, 0xfe), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x02, 0x28), > @@ -927,10 +915,7 @@ static struct iovec get_included_1[] = { > }; > > static struct iovec get_included_2[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), > raw_pdu(0x09, 0x06, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00), > raw_pdu(0x0a, 0x15, 0x00), > @@ -942,20 +927,14 @@ static struct iovec get_included_2[] = { > }; > > static struct iovec get_included_3[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), > raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a), > end_pdu > }; > > static struct iovec read_characteristic_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -966,10 +945,7 @@ static struct iovec read_characteristic_1[] = { > }; > > static struct iovec read_characteristic_2[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -980,10 +956,7 @@ static struct iovec read_characteristic_2[] = { > }; > > static struct iovec read_descriptor_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -998,10 +971,7 @@ static struct iovec read_descriptor_1[] = { > }; > > static struct iovec read_descriptor_2[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1016,10 +986,7 @@ static struct iovec read_descriptor_2[] = { > }; > > static struct iovec write_characteristic_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1029,10 +996,7 @@ static struct iovec write_characteristic_1[] = { > }; > > static struct iovec write_characteristic_2[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1043,10 +1007,7 @@ static struct iovec write_characteristic_2[] = { > }; > > static struct iovec write_characteristic_3[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1057,10 +1018,7 @@ static struct iovec write_characteristic_3[] = { > }; > > static struct iovec write_descriptor_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1075,10 +1033,7 @@ static struct iovec write_descriptor_1[] = { > }; > > static struct iovec write_descriptor_2[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1093,10 +1048,7 @@ static struct iovec write_descriptor_2[] = { > }; > > static struct iovec notification_1[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1105,10 +1057,7 @@ static struct iovec notification_1[] = { > }; > > static struct iovec notification_2[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > @@ -1119,10 +1068,7 @@ static struct iovec notification_2[] = { > }; > > static struct iovec notification_3[] = { > - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), > - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), > - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), > + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, > raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), > raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), > raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), -- BR Szymon Janc ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs 2014-11-12 20:51 ` Szymon Janc @ 2014-11-13 15:30 ` Michael Janssen 2014-11-13 16:16 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 16+ messages in thread From: Michael Janssen @ 2014-11-13 15:30 UTC (permalink / raw) To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org Hi Szymon, Yes, DRY stands for "Don't Repeat Yourself"[1]. It's referring to the deduplication of these PDUs in the testing iovecs. [1]: http://c2.com/cgi/wiki?DontRepeatYourself On Wed, Nov 12, 2014 at 12:51 PM, Szymon Janc <szymon.janc@tieto.com> wrote: > HI Michael, > > On Wednesday 12 of November 2014 12:22:56 Michael Janssen wrote: > > This "DRY/" in commit subject is intended? > >> The search service with a single service response PDUs are repeated >> multiple times throughout the testing data. Use a macro instead. >> >> Also, the final PDU was malformed on many of these (had 0x11 instead of >> 0x10 in the "request opcode in error" slot) which this fixes. >> --- >> android/tester-gatt.c | 106 >> +++++++++++++------------------------------------- 1 file changed, 26 >> insertions(+), 80 deletions(-) >> >> diff --git a/android/tester-gatt.c b/android/tester-gatt.c >> index 13e096f..b88eeff 100644 >> --- a/android/tester-gatt.c >> +++ b/android/tester-gatt.c >> @@ -832,11 +832,14 @@ static struct send_resp_data send_resp_data_2 = { >> .response = &response_2, >> }; >> >> +#define SEARCH_SERVICE_SINGLE_SUCCESS_PDUS \ >> + raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), \ >> + raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), \ >> + raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), \ >> + raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a) \ >> + >> static struct iovec search_service[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> end_pdu >> }; >> >> @@ -857,10 +860,7 @@ static struct iovec search_service_3[] = { >> }; >> >> static struct iovec get_characteristic_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -869,10 +869,7 @@ static struct iovec get_characteristic_1[] = { >> }; >> >> static struct iovec get_descriptor_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -885,10 +882,7 @@ static struct iovec get_descriptor_1[] = { >> }; >> >> static struct iovec get_descriptor_2[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -901,10 +895,7 @@ static struct iovec get_descriptor_2[] = { >> }; >> >> static struct iovec get_descriptor_3[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -915,10 +906,7 @@ static struct iovec get_descriptor_3[] = { >> }; >> >> static struct iovec get_included_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), >> raw_pdu(0x09, 0x08, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00, 0xff, 0xfe), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x02, 0x28), >> @@ -927,10 +915,7 @@ static struct iovec get_included_1[] = { >> }; >> >> static struct iovec get_included_2[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), >> raw_pdu(0x09, 0x06, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00), >> raw_pdu(0x0a, 0x15, 0x00), >> @@ -942,20 +927,14 @@ static struct iovec get_included_2[] = { >> }; >> >> static struct iovec get_included_3[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), >> raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a), >> end_pdu >> }; >> >> static struct iovec read_characteristic_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -966,10 +945,7 @@ static struct iovec read_characteristic_1[] = { >> }; >> >> static struct iovec read_characteristic_2[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -980,10 +956,7 @@ static struct iovec read_characteristic_2[] = { >> }; >> >> static struct iovec read_descriptor_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -998,10 +971,7 @@ static struct iovec read_descriptor_1[] = { >> }; >> >> static struct iovec read_descriptor_2[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1016,10 +986,7 @@ static struct iovec read_descriptor_2[] = { >> }; >> >> static struct iovec write_characteristic_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1029,10 +996,7 @@ static struct iovec write_characteristic_1[] = { >> }; >> >> static struct iovec write_characteristic_2[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1043,10 +1007,7 @@ static struct iovec write_characteristic_2[] = { >> }; >> >> static struct iovec write_characteristic_3[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1057,10 +1018,7 @@ static struct iovec write_characteristic_3[] = { >> }; >> >> static struct iovec write_descriptor_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1075,10 +1033,7 @@ static struct iovec write_descriptor_1[] = { >> }; >> >> static struct iovec write_descriptor_2[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1093,10 +1048,7 @@ static struct iovec write_descriptor_2[] = { >> }; >> >> static struct iovec notification_1[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1105,10 +1057,7 @@ static struct iovec notification_1[] = { >> }; >> >> static struct iovec notification_2[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> @@ -1119,10 +1068,7 @@ static struct iovec notification_2[] = { >> }; >> >> static struct iovec notification_3[] = { >> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), > > -- > BR > Szymon Janc Michael Janssen ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs 2014-11-13 15:30 ` Michael Janssen @ 2014-11-13 16:16 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 16+ messages in thread From: Luiz Augusto von Dentz @ 2014-11-13 16:16 UTC (permalink / raw) To: Michael Janssen; +Cc: Szymon Janc, linux-bluetooth@vger.kernel.org Hi Michael, On Thu, Nov 13, 2014 at 5:30 PM, Michael Janssen <jamuraa@chromium.org> wrote: > Hi Szymon, > > Yes, DRY stands for "Don't Repeat Yourself"[1]. It's referring to the > deduplication of these PDUs in the testing iovecs. > > [1]: http://c2.com/cgi/wiki?DontRepeatYourself Well it is always easier if you just spell out everything, btw when it is a bugfix make sure it start with <prefix>: Fix... > On Wed, Nov 12, 2014 at 12:51 PM, Szymon Janc <szymon.janc@tieto.com> wrote: >> HI Michael, >> >> On Wednesday 12 of November 2014 12:22:56 Michael Janssen wrote: >> >> This "DRY/" in commit subject is intended? >> >>> The search service with a single service response PDUs are repeated >>> multiple times throughout the testing data. Use a macro instead. >>> >>> Also, the final PDU was malformed on many of these (had 0x11 instead of >>> 0x10 in the "request opcode in error" slot) which this fixes. >>> --- >>> android/tester-gatt.c | 106 >>> +++++++++++++------------------------------------- 1 file changed, 26 >>> insertions(+), 80 deletions(-) >>> >>> diff --git a/android/tester-gatt.c b/android/tester-gatt.c >>> index 13e096f..b88eeff 100644 >>> --- a/android/tester-gatt.c >>> +++ b/android/tester-gatt.c >>> @@ -832,11 +832,14 @@ static struct send_resp_data send_resp_data_2 = { >>> .response = &response_2, >>> }; >>> >>> +#define SEARCH_SERVICE_SINGLE_SUCCESS_PDUS \ >>> + raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), \ >>> + raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), \ >>> + raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), \ >>> + raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a) \ >>> + >>> static struct iovec search_service[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> end_pdu >>> }; >>> >>> @@ -857,10 +860,7 @@ static struct iovec search_service_3[] = { >>> }; >>> >>> static struct iovec get_characteristic_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -869,10 +869,7 @@ static struct iovec get_characteristic_1[] = { >>> }; >>> >>> static struct iovec get_descriptor_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -885,10 +882,7 @@ static struct iovec get_descriptor_1[] = { >>> }; >>> >>> static struct iovec get_descriptor_2[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -901,10 +895,7 @@ static struct iovec get_descriptor_2[] = { >>> }; >>> >>> static struct iovec get_descriptor_3[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -915,10 +906,7 @@ static struct iovec get_descriptor_3[] = { >>> }; >>> >>> static struct iovec get_included_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), >>> raw_pdu(0x09, 0x08, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00, 0xff, 0xfe), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x02, 0x28), >>> @@ -927,10 +915,7 @@ static struct iovec get_included_1[] = { >>> }; >>> >>> static struct iovec get_included_2[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), >>> raw_pdu(0x09, 0x06, 0x02, 0x00, 0x15, 0x00, 0x19, 0x00), >>> raw_pdu(0x0a, 0x15, 0x00), >>> @@ -942,20 +927,14 @@ static struct iovec get_included_2[] = { >>> }; >>> >>> static struct iovec get_included_3[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), >>> raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a), >>> end_pdu >>> }; >>> >>> static struct iovec read_characteristic_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -966,10 +945,7 @@ static struct iovec read_characteristic_1[] = { >>> }; >>> >>> static struct iovec read_characteristic_2[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -980,10 +956,7 @@ static struct iovec read_characteristic_2[] = { >>> }; >>> >>> static struct iovec read_descriptor_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -998,10 +971,7 @@ static struct iovec read_descriptor_1[] = { >>> }; >>> >>> static struct iovec read_descriptor_2[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1016,10 +986,7 @@ static struct iovec read_descriptor_2[] = { >>> }; >>> >>> static struct iovec write_characteristic_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1029,10 +996,7 @@ static struct iovec write_characteristic_1[] = { >>> }; >>> >>> static struct iovec write_characteristic_2[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1043,10 +1007,7 @@ static struct iovec write_characteristic_2[] = { >>> }; >>> >>> static struct iovec write_characteristic_3[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x03, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1057,10 +1018,7 @@ static struct iovec write_characteristic_3[] = { >>> }; >>> >>> static struct iovec write_descriptor_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1075,10 +1033,7 @@ static struct iovec write_descriptor_1[] = { >>> }; >>> >>> static struct iovec write_descriptor_2[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1093,10 +1048,7 @@ static struct iovec write_descriptor_2[] = { >>> }; >>> >>> static struct iovec notification_1[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1105,10 +1057,7 @@ static struct iovec notification_1[] = { >>> }; >>> >>> static struct iovec notification_2[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >>> @@ -1119,10 +1068,7 @@ static struct iovec notification_2[] = { >>> }; >>> >>> static struct iovec notification_3[] = { >>> - raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), >>> - raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), >>> - raw_pdu(0x01, 0x11, 0x11, 0x00, 0x0a), >>> + SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, >>> raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), >>> raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), >>> raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), >> >> -- >> BR >> Szymon Janc > > Michael Janssen > -- > 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 -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH BlueZ 2/4] android/tester-gatt: deduplicate read-by-type PDUs 2014-11-12 20:22 [PATCH BlueZ 0/4] android fixes and gattrib shim Michael Janssen 2014-11-12 20:22 ` [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs Michael Janssen @ 2014-11-12 20:22 ` Michael Janssen 2014-11-12 20:22 ` [PATCH BlueZ 3/4] android/gatt: dummy callback for indications Michael Janssen 2014-11-12 20:22 ` [PATCH BlueZ 4/4] GATT shim to src/shared bt_att Michael Janssen 3 siblings, 0 replies; 16+ messages in thread From: Michael Janssen @ 2014-11-12 20:22 UTC (permalink / raw) To: linux-bluetooth; +Cc: Michael Janssen These read-by-type PDUs are repeated multiple times throughout the code. --- android/tester-gatt.c | 63 +++++++++++++++------------------------------------ 1 file changed, 18 insertions(+), 45 deletions(-) diff --git a/android/tester-gatt.c b/android/tester-gatt.c index b88eeff..ebfa005 100644 --- a/android/tester-gatt.c +++ b/android/tester-gatt.c @@ -836,7 +836,13 @@ static struct send_resp_data send_resp_data_2 = { raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), \ raw_pdu(0x11, 0x06, 0x01, 0x00, 0x10, 0x00, 0x00, 0x18), \ raw_pdu(0x10, 0x11, 0x00, 0xff, 0xff, 0x00, 0x28), \ - raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a) \ + raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a) + +#define READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS \ + raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), \ + raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), \ + raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), \ + raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a) static struct iovec search_service[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, @@ -861,19 +867,13 @@ static struct iovec search_service_3[] = { static struct iovec get_characteristic_1[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, end_pdu }; static struct iovec get_descriptor_1[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x04, 0x01, 0x00, 0x10, 0x00), raw_pdu(0x05, 0x01, 0x04, 0x00, 0x00, 0x29), raw_pdu(0x04, 0x05, 0x00, 0x10, 0x00), @@ -883,10 +883,7 @@ static struct iovec get_descriptor_1[] = { static struct iovec get_descriptor_2[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x04, 0x01, 0x00, 0x10, 0x00), raw_pdu(0x05, 0x01, 0x04, 0x00, 0x00, 0x29, 0x05, 0x00, 0x01, 0x29), raw_pdu(0x04, 0x06, 0x00, 0x10, 0x00), @@ -896,10 +893,7 @@ static struct iovec get_descriptor_2[] = { static struct iovec get_descriptor_3[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x04, 0x01, 0x00, 0x10, 0x00), raw_pdu(0x01, 0x04, 0x01, 0x00, 0x0a), end_pdu @@ -957,10 +951,7 @@ static struct iovec read_characteristic_2[] = { static struct iovec read_descriptor_1[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x04, 0x01, 0x00, 0x10, 0x00), raw_pdu(0x05, 0x01, 0x04, 0x00, 0x00, 0x29), raw_pdu(0x04, 0x05, 0x00, 0x10, 0x00), @@ -972,10 +963,7 @@ static struct iovec read_descriptor_1[] = { static struct iovec read_descriptor_2[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x04, 0x01, 0x00, 0x10, 0x00), raw_pdu(0x05, 0x01, 0x04, 0x00, 0x00, 0x29), raw_pdu(0x04, 0x05, 0x00, 0x10, 0x00), @@ -1019,10 +1007,7 @@ static struct iovec write_characteristic_3[] = { static struct iovec write_descriptor_1[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x04, 0x01, 0x00, 0x10, 0x00), raw_pdu(0x05, 0x01, 0x04, 0x00, 0x00, 0x29), raw_pdu(0x04, 0x05, 0x00, 0x10, 0x00), @@ -1034,10 +1019,7 @@ static struct iovec write_descriptor_1[] = { static struct iovec write_descriptor_2[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x04, 0x01, 0x00, 0x10, 0x00), raw_pdu(0x05, 0x01, 0x04, 0x00, 0x00, 0x29), raw_pdu(0x04, 0x05, 0x00, 0x10, 0x00), @@ -1049,19 +1031,13 @@ static struct iovec write_descriptor_2[] = { static struct iovec notification_1[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, end_pdu }; static struct iovec notification_2[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x1d, 0x03, 0x00, 0x01), raw_pdu(0x1e), end_pdu @@ -1069,10 +1045,7 @@ static struct iovec notification_2[] = { static struct iovec notification_3[] = { SEARCH_SERVICE_SINGLE_SUCCESS_PDUS, - raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x09, 0x07, 0x02, 0x00, 0x04, 0x00, 0x00, 0x19, 0x00), - raw_pdu(0x08, 0x03, 0x00, 0x10, 0x00, 0x03, 0x28), - raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a), + READ_BY_TYPE_SINGLE_CHARACTERISTIC_PDUS, raw_pdu(0x1b, 0x03, 0x00, 0x01), end_pdu }; -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH BlueZ 3/4] android/gatt: dummy callback for indications 2014-11-12 20:22 [PATCH BlueZ 0/4] android fixes and gattrib shim Michael Janssen 2014-11-12 20:22 ` [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs Michael Janssen 2014-11-12 20:22 ` [PATCH BlueZ 2/4] android/tester-gatt: deduplicate read-by-type PDUs Michael Janssen @ 2014-11-12 20:22 ` Michael Janssen 2014-11-12 20:56 ` Szymon Janc 2014-11-12 20:22 ` [PATCH BlueZ 4/4] GATT shim to src/shared bt_att Michael Janssen 3 siblings, 1 reply; 16+ messages in thread From: Michael Janssen @ 2014-11-12 20:22 UTC (permalink / raw) To: linux-bluetooth; +Cc: Michael Janssen Indications require a confirmation reply, and newer APIs require that a callback is provided. Add a dummy callback which ignores this confirmation to ensure future compatability. --- android/gatt.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/android/gatt.c b/android/gatt.c index 086bb94..9c48f97 100644 --- a/android/gatt.c +++ b/android/gatt.c @@ -5414,6 +5414,11 @@ failed: HAL_OP_GATT_SERVER_DELETE_SERVICE, status); } +static void ignore_confirmation_cb(guint8 status, const guint8 *pdu, + guint16 len, gpointer user_data) { + // Ignored. +} + static void handle_server_send_indication(const void *buf, uint16_t len) { const struct hal_cmd_gatt_server_send_indication *cmd = buf; @@ -5422,6 +5427,7 @@ static void handle_server_send_indication(const void *buf, uint16_t len) uint16_t length; uint8_t *pdu; size_t mtu; + GAttribResultFunc confirmation_cb = NULL; DBG(""); @@ -5434,12 +5440,13 @@ static void handle_server_send_indication(const void *buf, uint16_t len) pdu = g_attrib_get_buffer(conn->device->attrib, &mtu); - if (cmd->confirm) + if (cmd->confirm) { /* TODO: Add data to track confirmation for this request */ length = enc_indication(cmd->attribute_handle, (uint8_t *)cmd->value, cmd->len, pdu, mtu); - else + confirmation_cb = ignore_confirmation_cb; + } else length = enc_notification(cmd->attribute_handle, (uint8_t *)cmd->value, cmd->len, pdu, mtu); @@ -5448,8 +5455,8 @@ static void handle_server_send_indication(const void *buf, uint16_t len) error("gatt: Failed to encode indication"); status = HAL_STATUS_FAILED; } else { - g_attrib_send(conn->device->attrib, 0, pdu, length, NULL, NULL, - NULL); + g_attrib_send(conn->device->attrib, 0, pdu, length, + confirmation_cb, NULL, NULL); status = HAL_STATUS_SUCCESS; } -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 3/4] android/gatt: dummy callback for indications 2014-11-12 20:22 ` [PATCH BlueZ 3/4] android/gatt: dummy callback for indications Michael Janssen @ 2014-11-12 20:56 ` Szymon Janc 0 siblings, 0 replies; 16+ messages in thread From: Szymon Janc @ 2014-11-12 20:56 UTC (permalink / raw) To: Michael Janssen; +Cc: linux-bluetooth Hi Michael, On Wednesday 12 of November 2014 12:22:58 Michael Janssen wrote: > Indications require a confirmation reply, and newer APIs require that a > callback is provided. Add a dummy callback which ignores this > confirmation to ensure future compatability. > --- There are some coding style issues in this patch. > android/gatt.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/android/gatt.c b/android/gatt.c > index 086bb94..9c48f97 100644 > --- a/android/gatt.c > +++ b/android/gatt.c > @@ -5414,6 +5414,11 @@ failed: > HAL_OP_GATT_SERVER_DELETE_SERVICE, status); > } > > +static void ignore_confirmation_cb(guint8 status, const guint8 *pdu, > + guint16 len, gpointer user_data) { > + // Ignored. > +} Please use C-style /* foo */ comments. Also function braces should be open in new line. > + > static void handle_server_send_indication(const void *buf, uint16_t len) > { > const struct hal_cmd_gatt_server_send_indication *cmd = buf; > @@ -5422,6 +5427,7 @@ static void handle_server_send_indication(const void > *buf, uint16_t len) uint16_t length; > uint8_t *pdu; > size_t mtu; > + GAttribResultFunc confirmation_cb = NULL; > > DBG(""); > > @@ -5434,12 +5440,13 @@ static void handle_server_send_indication(const void > *buf, uint16_t len) > > pdu = g_attrib_get_buffer(conn->device->attrib, &mtu); > > - if (cmd->confirm) > + if (cmd->confirm) { > /* TODO: Add data to track confirmation for this request */ > length = enc_indication(cmd->attribute_handle, > (uint8_t *)cmd->value, cmd->len, pdu, > mtu); > - else > + confirmation_cb = ignore_confirmation_cb; > + } else > length = enc_notification(cmd->attribute_handle, > (uint8_t *)cmd->value, cmd->len, > pdu, mtu); If single branch has braces every branch should have those. > @@ -5448,8 +5455,8 @@ static void handle_server_send_indication(const void > *buf, uint16_t len) error("gatt: Failed to encode indication"); > status = HAL_STATUS_FAILED; > } else { > - g_attrib_send(conn->device->attrib, 0, pdu, length, NULL, NULL, > - NULL); > + g_attrib_send(conn->device->attrib, 0, pdu, length, > + confirmation_cb, NULL, NULL); > status = HAL_STATUS_SUCCESS; > } Other than code style issues this looks fine for me. -- BR Szymon Janc ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-12 20:22 [PATCH BlueZ 0/4] android fixes and gattrib shim Michael Janssen ` (2 preceding siblings ...) 2014-11-12 20:22 ` [PATCH BlueZ 3/4] android/gatt: dummy callback for indications Michael Janssen @ 2014-11-12 20:22 ` Michael Janssen 2014-11-12 21:14 ` Szymon Janc 3 siblings, 1 reply; 16+ messages in thread From: Michael Janssen @ 2014-11-12 20:22 UTC (permalink / raw) To: linux-bluetooth; +Cc: Michael Janssen This patch implements a version of GAttrib which is backed by bt_att, which enables the simultaneous use of GAttrib and bt_att. This should enable smooth transition of profiles from the GAttrib API to the src/shared bt_att API. --- attrib/gattrib.c | 733 ++++++++++++------------------------------------------- 1 file changed, 154 insertions(+), 579 deletions(-) diff --git a/attrib/gattrib.c b/attrib/gattrib.c index fa51b6d..b55e437 100644 --- a/attrib/gattrib.c +++ b/attrib/gattrib.c @@ -36,227 +36,124 @@ #include <bluetooth/bluetooth.h> #include "btio/btio.h" -#include "lib/uuid.h" -#include "src/shared/util.h" #include "src/log.h" -#include "attrib/att.h" +#include "src/shared/util.h" +#include "src/shared/att.h" #include "attrib/gattrib.h" -#define GATT_TIMEOUT 30 - struct _GAttrib { + int ref_count; + struct bt_att *att; GIOChannel *io; - int refs; - uint8_t *buf; - size_t buflen; - guint read_watch; - guint write_watch; - guint timeout_watch; - GQueue *requests; - GQueue *responses; - GSList *events; - guint next_cmd_id; GDestroyNotify destroy; gpointer destroy_user_data; - bool stale; + GQueue *callbacks; + uint8_t *buf; + int buflen; }; -struct command { - guint id; - guint8 opcode; - guint8 *pdu; - guint16 len; - guint8 expected; - bool sent; - GAttribResultFunc func; - gpointer user_data; - GDestroyNotify notify; -}; -struct event { - guint id; - guint8 expected; - guint16 handle; - GAttribNotifyFunc func; +struct attrib_callbacks { + GAttribResultFunc result_func; + GAttribNotifyFunc notify_func; + GDestroyNotify destroy_func; gpointer user_data; - GDestroyNotify notify; + GAttrib *parent; + uint16_t notify_handle; }; -static guint8 opcode2expected(guint8 opcode) +GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) { - switch (opcode) { - case ATT_OP_MTU_REQ: - return ATT_OP_MTU_RESP; - - case ATT_OP_FIND_INFO_REQ: - return ATT_OP_FIND_INFO_RESP; - - case ATT_OP_FIND_BY_TYPE_REQ: - return ATT_OP_FIND_BY_TYPE_RESP; - - case ATT_OP_READ_BY_TYPE_REQ: - return ATT_OP_READ_BY_TYPE_RESP; - - case ATT_OP_READ_REQ: - return ATT_OP_READ_RESP; - - case ATT_OP_READ_BLOB_REQ: - return ATT_OP_READ_BLOB_RESP; - - case ATT_OP_READ_MULTI_REQ: - return ATT_OP_READ_MULTI_RESP; + gint fd; + GAttrib *attr; - case ATT_OP_READ_BY_GROUP_REQ: - return ATT_OP_READ_BY_GROUP_RESP; + if (!io) + return NULL; - case ATT_OP_WRITE_REQ: - return ATT_OP_WRITE_RESP; + fd = g_io_channel_unix_get_fd(io); + attr = new0(GAttrib, 1); + if (!attr) + return NULL; - case ATT_OP_PREP_WRITE_REQ: - return ATT_OP_PREP_WRITE_RESP; + g_io_channel_ref(io); + attr->io = io; - case ATT_OP_EXEC_WRITE_REQ: - return ATT_OP_EXEC_WRITE_RESP; + attr->att = bt_att_new(fd); + if (!attr->att) + goto fail; - case ATT_OP_HANDLE_IND: - return ATT_OP_HANDLE_CNF; - } + attr->buf = g_malloc0(mtu); + attr->buflen = mtu; + if (!attr->buf) + goto fail; - return 0; -} + attr->callbacks = g_queue_new(); + if (!attr->callbacks) + goto fail; -static bool is_response(guint8 opcode) -{ - switch (opcode) { - case ATT_OP_ERROR: - case ATT_OP_MTU_RESP: - case ATT_OP_FIND_INFO_RESP: - case ATT_OP_FIND_BY_TYPE_RESP: - case ATT_OP_READ_BY_TYPE_RESP: - case ATT_OP_READ_RESP: - case ATT_OP_READ_BLOB_RESP: - case ATT_OP_READ_MULTI_RESP: - case ATT_OP_READ_BY_GROUP_RESP: - case ATT_OP_WRITE_RESP: - case ATT_OP_PREP_WRITE_RESP: - case ATT_OP_EXEC_WRITE_RESP: - case ATT_OP_HANDLE_CNF: - return true; - } + return g_attrib_ref(attr); - return false; -} - -static bool is_request(guint8 opcode) -{ - switch (opcode) { - case ATT_OP_MTU_REQ: - case ATT_OP_FIND_INFO_REQ: - case ATT_OP_FIND_BY_TYPE_REQ: - case ATT_OP_READ_BY_TYPE_REQ: - case ATT_OP_READ_REQ: - case ATT_OP_READ_BLOB_REQ: - case ATT_OP_READ_MULTI_REQ: - case ATT_OP_READ_BY_GROUP_REQ: - case ATT_OP_WRITE_REQ: - case ATT_OP_WRITE_CMD: - case ATT_OP_PREP_WRITE_REQ: - case ATT_OP_EXEC_WRITE_REQ: - return true; - } - - return false; +fail: + free(attr->buf); + bt_att_unref(attr->att); + g_io_channel_unref(io); + free(attr); + return NULL; } GAttrib *g_attrib_ref(GAttrib *attrib) { - int refs; - if (!attrib) return NULL; - refs = __sync_add_and_fetch(&attrib->refs, 1); + __sync_fetch_and_add(&attrib->ref_count, 1); - DBG("%p: ref=%d", attrib, refs); + DBG("%p: g_attrib_ref=%d ", attrib, attrib->ref_count); return attrib; } -static void command_destroy(struct command *cmd) +static void attrib_callbacks_destroy(void *user_data) { - if (cmd->notify) - cmd->notify(cmd->user_data); + struct attrib_callbacks *cb; - g_free(cmd->pdu); - g_free(cmd); -} + cb = (struct attrib_callbacks *)user_data; + if (!user_data || !g_queue_remove(cb->parent->callbacks, user_data)) + return; -static void event_destroy(struct event *evt) -{ - if (evt->notify) - evt->notify(evt->user_data); + if (cb->destroy_func) + cb->destroy_func(cb->user_data); - g_free(evt); + free(user_data); } -static void attrib_destroy(GAttrib *attrib) +void g_attrib_unref(GAttrib *attrib) { - GSList *l; - struct command *c; - - while ((c = g_queue_pop_head(attrib->requests))) - command_destroy(c); - - while ((c = g_queue_pop_head(attrib->responses))) - command_destroy(c); - - g_queue_free(attrib->requests); - attrib->requests = NULL; - - g_queue_free(attrib->responses); - attrib->responses = NULL; + struct attrib_callbacks *cb; - for (l = attrib->events; l; l = l->next) - event_destroy(l->data); - - g_slist_free(attrib->events); - attrib->events = NULL; - - if (attrib->timeout_watch > 0) - g_source_remove(attrib->timeout_watch); - - if (attrib->write_watch > 0) - g_source_remove(attrib->write_watch); - - if (attrib->read_watch > 0) - g_source_remove(attrib->read_watch); + if (!attrib) + return; - if (attrib->io) - g_io_channel_unref(attrib->io); + DBG("%p: g_attrib_unref=%d ", attrib, attrib->ref_count-1); - g_free(attrib->buf); + if (__sync_sub_and_fetch(&attrib->ref_count, 1)) + return; if (attrib->destroy) attrib->destroy(attrib->destroy_user_data); - g_free(attrib); -} + while ((cb = g_queue_peek_head(attrib->callbacks))) + attrib_callbacks_destroy(cb); -void g_attrib_unref(GAttrib *attrib) -{ - int refs; + g_queue_free(attrib->callbacks); - if (!attrib) - return; - - refs = __sync_sub_and_fetch(&attrib->refs, 1); + g_free(attrib->buf); - DBG("%p: ref=%d", attrib, refs); + bt_att_unref(attrib->att); - if (refs > 0) - return; + g_io_channel_unref(attrib->io); - attrib_destroy(attrib); + g_free(attrib); } GIOChannel *g_attrib_get_channel(GAttrib *attrib) @@ -270,7 +167,7 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib) gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy, gpointer user_data) { - if (attrib == NULL) + if (!attrib) return FALSE; attrib->destroy = destroy; @@ -279,380 +176,130 @@ gboolean g_attrib_set_destroy_function(GAttrib *attrib, return TRUE; } -static gboolean disconnect_timeout(gpointer data) -{ - struct _GAttrib *attrib = data; - struct command *c; - - g_attrib_ref(attrib); - - c = g_queue_pop_head(attrib->requests); - if (c == NULL) - goto done; - - if (c->func) - c->func(ATT_ECODE_TIMEOUT, NULL, 0, c->user_data); - - command_destroy(c); - - while ((c = g_queue_pop_head(attrib->requests))) { - if (c->func) - c->func(ATT_ECODE_ABORTED, NULL, 0, c->user_data); - command_destroy(c); - } -done: - attrib->stale = true; - - g_attrib_unref(attrib); - - return FALSE; -} - -static gboolean can_write_data(GIOChannel *io, GIOCondition cond, - gpointer data) +static void attrib_callback_result(uint8_t opcode, const void *pdu, + uint16_t length, void *user_data) { - struct _GAttrib *attrib = data; - struct command *cmd; - GError *gerr = NULL; - gsize len; - GIOStatus iostat; - GQueue *queue; - - if (attrib->stale) - return FALSE; - - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) - return FALSE; - - queue = attrib->responses; - cmd = g_queue_peek_head(queue); - if (cmd == NULL) { - queue = attrib->requests; - cmd = g_queue_peek_head(queue); - } - if (cmd == NULL) - return FALSE; - - /* - * Verify that we didn't already send this command. This can only - * happen with elementes from attrib->requests. - */ - if (cmd->sent) - return FALSE; + uint8_t *buf; + struct attrib_callbacks *cb = user_data; + guint8 status = 0; - iostat = g_io_channel_write_chars(io, (char *) cmd->pdu, cmd->len, - &len, &gerr); - if (iostat != G_IO_STATUS_NORMAL) { - if (gerr) { - error("%s", gerr->message); - g_error_free(gerr); - } + if (!cb) + return; - return FALSE; - } + buf = g_malloc0(length+1); + if (!buf) + return; - if (cmd->expected == 0) { - g_queue_pop_head(queue); - command_destroy(cmd); + buf[0] = opcode; + memcpy(buf+1, pdu, length); - return TRUE; + if (opcode == BT_ATT_OP_ERROR_RSP) { + if (length < 4) + status = BT_ATT_ERROR_UNLIKELY; + else + status = buf[4]; } - cmd->sent = true; - - if (attrib->timeout_watch == 0) - attrib->timeout_watch = g_timeout_add_seconds(GATT_TIMEOUT, - disconnect_timeout, attrib); + if (cb->result_func) + cb->result_func(status, buf, length+1, cb->user_data); - return FALSE; + g_free(buf); } -static void destroy_sender(gpointer data) -{ - struct _GAttrib *attrib = data; - attrib->write_watch = 0; - g_attrib_unref(attrib); -} - -static void wake_up_sender(struct _GAttrib *attrib) -{ - if (attrib->write_watch > 0) - return; - - attrib = g_attrib_ref(attrib); - attrib->write_watch = g_io_add_watch_full(attrib->io, - G_PRIORITY_DEFAULT, G_IO_OUT, - can_write_data, attrib, destroy_sender); -} - -static bool match_event(struct event *evt, const uint8_t *pdu, gsize len) -{ - guint16 handle; - - if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS) - return true; - - if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES) - return true; - - if (len < 3) - return false; - - handle = get_le16(&pdu[1]); - - if (evt->expected == pdu[0] && evt->handle == handle) - return true; - - return false; -} - -static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data) +static void attrib_callback_notify(uint8_t opcode, const void *pdu, + uint16_t length, void *user_data) { - struct _GAttrib *attrib = data; - struct command *cmd = NULL; - GSList *l; - uint8_t buf[512], status; - gsize len; - GIOStatus iostat; - - if (attrib->stale) - return FALSE; - - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) { - struct command *c; - - while ((c = g_queue_pop_head(attrib->requests))) { - if (c->func) - c->func(ATT_ECODE_IO, NULL, 0, c->user_data); - command_destroy(c); - } - - attrib->read_watch = 0; - - return FALSE; - } - - memset(buf, 0, sizeof(buf)); - - iostat = g_io_channel_read_chars(io, (char *) buf, sizeof(buf), - &len, NULL); - if (iostat != G_IO_STATUS_NORMAL) { - status = ATT_ECODE_IO; - goto done; - } - - for (l = attrib->events; l; l = l->next) { - struct event *evt = l->data; - - if (match_event(evt, buf, len)) - evt->func(buf, len, evt->user_data); - } - - if (!is_response(buf[0])) - return TRUE; - - if (attrib->timeout_watch > 0) { - g_source_remove(attrib->timeout_watch); - attrib->timeout_watch = 0; - } - - cmd = g_queue_pop_head(attrib->requests); - if (cmd == NULL) { - /* Keep the watch if we have events to report */ - return attrib->events != NULL; - } - - if (buf[0] == ATT_OP_ERROR) { - status = buf[4]; - goto done; - } - - if (cmd->expected != buf[0]) { - status = ATT_ECODE_IO; - goto done; - } - - status = 0; - -done: - if (!g_queue_is_empty(attrib->requests) || - !g_queue_is_empty(attrib->responses)) - wake_up_sender(attrib); - - if (cmd) { - if (cmd->func) - cmd->func(status, buf, len, cmd->user_data); - - command_destroy(cmd); - } + uint8_t *buf; + struct attrib_callbacks *cb = user_data; - return TRUE; -} + if (!cb) + return; -GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) -{ - struct _GAttrib *attrib; + if (cb->notify_func == NULL) + return; - g_io_channel_set_encoding(io, NULL, NULL); - g_io_channel_set_buffered(io, FALSE); + if (cb->notify_handle != GATTRIB_ALL_HANDLES && length < 2) + return; - attrib = g_try_new0(struct _GAttrib, 1); - if (attrib == NULL) - return NULL; + if (cb->notify_handle != GATTRIB_ALL_HANDLES && + cb->notify_handle != get_le16(pdu)) + return; - attrib->buf = g_malloc0(mtu); - attrib->buflen = mtu; + buf = g_malloc0(length+1); + if (!buf) + return; - attrib->io = g_io_channel_ref(io); - attrib->requests = g_queue_new(); - attrib->responses = g_queue_new(); + buf[0] = opcode; + memcpy(buf+1, pdu, length); - attrib->read_watch = g_io_add_watch(attrib->io, - G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL, - received_data, attrib); + cb->notify_func(buf, length+1, cb->user_data); - return g_attrib_ref(attrib); + g_free(buf); } guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len, GAttribResultFunc func, gpointer user_data, GDestroyNotify notify) { - struct command *c; - GQueue *queue; - uint8_t opcode; - - if (attrib->stale) - return 0; + struct attrib_callbacks *cb = NULL; + bt_att_response_func_t response_cb = NULL; + bt_att_destroy_func_t destroy_cb = NULL; - c = g_try_new0(struct command, 1); - if (c == NULL) + if (!pdu || !len) return 0; - opcode = pdu[0]; - - c->opcode = opcode; - c->expected = opcode2expected(opcode); - c->pdu = g_malloc(len); - memcpy(c->pdu, pdu, len); - c->len = len; - c->func = func; - c->user_data = user_data; - c->notify = notify; - - if (is_response(opcode)) - queue = attrib->responses; - else - queue = attrib->requests; - - if (id) { - c->id = id; - if (!is_response(opcode)) - g_queue_push_head(queue, c); - else - /* Don't re-order responses even if an ID is given */ - g_queue_push_tail(queue, c); - } else { - c->id = ++attrib->next_cmd_id; - g_queue_push_tail(queue, c); + if (func || notify) { + cb = new0(struct attrib_callbacks, 1); + if (cb == 0) + return 0; + cb->result_func = func; + cb->user_data = user_data; + cb->destroy_func = notify; + cb->parent = attrib; + g_queue_push_head(attrib->callbacks, cb); + response_cb = attrib_callback_result; + destroy_cb = attrib_callbacks_destroy; } - /* - * If a command was added to the queue and it was empty before, wake up - * the sender. If the sender was already woken up by the second queue, - * wake_up_sender will just return. - */ - if (g_queue_get_length(queue) == 1) - wake_up_sender(attrib); - - return c->id; -} - -static int command_cmp_by_id(gconstpointer a, gconstpointer b) -{ - const struct command *cmd = a; - guint id = GPOINTER_TO_UINT(b); - - return cmd->id - id; + return bt_att_send(attrib->att, pdu[0], (void *)pdu+1, len-1, + response_cb, cb, destroy_cb); } gboolean g_attrib_cancel(GAttrib *attrib, guint id) { - GList *l = NULL; - struct command *cmd; - GQueue *queue; - - if (attrib == NULL) - return FALSE; - - queue = attrib->requests; - if (queue) - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), - command_cmp_by_id); - if (l == NULL) { - queue = attrib->responses; - if (!queue) - return FALSE; - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), - command_cmp_by_id); - } - - if (l == NULL) - return FALSE; - - cmd = l->data; - - if (cmd == g_queue_peek_head(queue) && cmd->sent) - cmd->func = NULL; - else { - g_queue_remove(queue, cmd); - command_destroy(cmd); - } - - return TRUE; + return bt_att_cancel(attrib->att, id); } -static gboolean cancel_all_per_queue(GQueue *queue) +gboolean g_attrib_cancel_all(GAttrib *attrib) { - struct command *c, *head = NULL; - gboolean first = TRUE; - - if (queue == NULL) - return FALSE; - - while ((c = g_queue_pop_head(queue))) { - if (first && c->sent) { - /* If the command was sent ignore its callback ... */ - c->func = NULL; - head = c; - continue; - } - - first = FALSE; - command_destroy(c); - } - - if (head) { - /* ... and put it back in the queue */ - g_queue_push_head(queue, head); - } - - return TRUE; + return bt_att_cancel_all(attrib->att); } -gboolean g_attrib_cancel_all(GAttrib *attrib) +guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, + GAttribNotifyFunc func, gpointer user_data, + GDestroyNotify notify) { - gboolean ret; - - if (attrib == NULL) - return FALSE; + struct attrib_callbacks *cb = NULL; + + if (func || notify) { + cb = new0(struct attrib_callbacks, 1); + if (cb == 0) + return 0; + cb->notify_func = func; + cb->notify_handle = handle; + cb->user_data = user_data; + cb->destroy_func = notify; + cb->parent = attrib; + g_queue_push_head(attrib->callbacks, cb); + } - ret = cancel_all_per_queue(attrib->requests); - ret = cancel_all_per_queue(attrib->responses) && ret; + if (opcode == GATTRIB_ALL_REQS) + opcode = BT_ATT_ALL_REQUESTS; - return ret; + return bt_att_register(attrib->att, opcode, attrib_callback_notify, + cb, attrib_callbacks_destroy); } uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) @@ -661,98 +308,26 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) return NULL; *len = attrib->buflen; - return attrib->buf; } gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu) { - if (mtu < ATT_DEFAULT_LE_MTU) - return FALSE; - - attrib->buf = g_realloc(attrib->buf, mtu); - - attrib->buflen = mtu; - - return TRUE; -} - -guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, - GAttribNotifyFunc func, gpointer user_data, - GDestroyNotify notify) -{ - static guint next_evt_id = 0; - struct event *event; - - event = g_try_new0(struct event, 1); - if (event == NULL) - return 0; - - event->expected = opcode; - event->handle = handle; - event->func = func; - event->user_data = user_data; - event->notify = notify; - event->id = ++next_evt_id; - - attrib->events = g_slist_append(attrib->events, event); - - return event->id; -} - -static int event_cmp_by_id(gconstpointer a, gconstpointer b) -{ - const struct event *evt = a; - guint id = GPOINTER_TO_UINT(b); + /* Clients of this expect a buffer to use. */ + if (mtu > attrib->buflen) { + attrib->buf = g_realloc(attrib->buf, mtu); + attrib->buflen = mtu; + } - return evt->id - id; + return bt_att_set_mtu(attrib->att, mtu); } gboolean g_attrib_unregister(GAttrib *attrib, guint id) { - struct event *evt; - GSList *l; - - if (id == 0) { - warn("%s: invalid id", __func__); - return FALSE; - } - - l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id), - event_cmp_by_id); - if (l == NULL) - return FALSE; - - evt = l->data; - - attrib->events = g_slist_remove(attrib->events, evt); - - if (evt->notify) - evt->notify(evt->user_data); - - g_free(evt); - - return TRUE; + return bt_att_unregister(attrib->att, id); } gboolean g_attrib_unregister_all(GAttrib *attrib) { - GSList *l; - - if (attrib->events == NULL) - return FALSE; - - for (l = attrib->events; l; l = l->next) { - struct event *evt = l->data; - - if (evt->notify) - evt->notify(evt->user_data); - - g_free(evt); - } - - g_slist_free(attrib->events); - attrib->events = NULL; - - return TRUE; + return bt_att_unregister_all(attrib->att); } -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-12 20:22 ` [PATCH BlueZ 4/4] GATT shim to src/shared bt_att Michael Janssen @ 2014-11-12 21:14 ` Szymon Janc 2014-11-13 0:49 ` Arman Uguray 0 siblings, 1 reply; 16+ messages in thread From: Szymon Janc @ 2014-11-12 21:14 UTC (permalink / raw) To: Michael Janssen; +Cc: linux-bluetooth Hi Michael, On Wednesday 12 of November 2014 12:22:59 Michael Janssen wrote: > This patch implements a version of GAttrib which is backed by > bt_att, which enables the simultaneous use of GAttrib and bt_att. > > This should enable smooth transition of profiles from the GAttrib > API to the src/shared bt_att API. > --- > attrib/gattrib.c | 733 > ++++++++++++------------------------------------------- 1 file changed, 154 > insertions(+), 579 deletions(-) > > diff --git a/attrib/gattrib.c b/attrib/gattrib.c > index fa51b6d..b55e437 100644 > --- a/attrib/gattrib.c > +++ b/attrib/gattrib.c > @@ -36,227 +36,124 @@ > #include <bluetooth/bluetooth.h> > > #include "btio/btio.h" > -#include "lib/uuid.h" > -#include "src/shared/util.h" > #include "src/log.h" > -#include "attrib/att.h" > +#include "src/shared/util.h" > +#include "src/shared/att.h" > #include "attrib/gattrib.h" > > -#define GATT_TIMEOUT 30 > - > struct _GAttrib { > + int ref_count; > + struct bt_att *att; > GIOChannel *io; > - int refs; > - uint8_t *buf; > - size_t buflen; > - guint read_watch; > - guint write_watch; > - guint timeout_watch; > - GQueue *requests; > - GQueue *responses; > - GSList *events; > - guint next_cmd_id; > GDestroyNotify destroy; > gpointer destroy_user_data; > - bool stale; > + GQueue *callbacks; > + uint8_t *buf; > + int buflen; > }; > > -struct command { > - guint id; > - guint8 opcode; > - guint8 *pdu; > - guint16 len; > - guint8 expected; > - bool sent; > - GAttribResultFunc func; > - gpointer user_data; > - GDestroyNotify notify; > -}; > > -struct event { > - guint id; > - guint8 expected; > - guint16 handle; > - GAttribNotifyFunc func; > +struct attrib_callbacks { > + GAttribResultFunc result_func; > + GAttribNotifyFunc notify_func; > + GDestroyNotify destroy_func; > gpointer user_data; > - GDestroyNotify notify; > + GAttrib *parent; > + uint16_t notify_handle; > }; > > -static guint8 opcode2expected(guint8 opcode) > +GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) > { > - switch (opcode) { > - case ATT_OP_MTU_REQ: > - return ATT_OP_MTU_RESP; > - > - case ATT_OP_FIND_INFO_REQ: > - return ATT_OP_FIND_INFO_RESP; > - > - case ATT_OP_FIND_BY_TYPE_REQ: > - return ATT_OP_FIND_BY_TYPE_RESP; > - > - case ATT_OP_READ_BY_TYPE_REQ: > - return ATT_OP_READ_BY_TYPE_RESP; > - > - case ATT_OP_READ_REQ: > - return ATT_OP_READ_RESP; > - > - case ATT_OP_READ_BLOB_REQ: > - return ATT_OP_READ_BLOB_RESP; > - > - case ATT_OP_READ_MULTI_REQ: > - return ATT_OP_READ_MULTI_RESP; > + gint fd; > + GAttrib *attr; > > - case ATT_OP_READ_BY_GROUP_REQ: > - return ATT_OP_READ_BY_GROUP_RESP; > + if (!io) > + return NULL; > > - case ATT_OP_WRITE_REQ: > - return ATT_OP_WRITE_RESP; > + fd = g_io_channel_unix_get_fd(io); > + attr = new0(GAttrib, 1); > + if (!attr) > + return NULL; > > - case ATT_OP_PREP_WRITE_REQ: > - return ATT_OP_PREP_WRITE_RESP; > + g_io_channel_ref(io); > + attr->io = io; > > - case ATT_OP_EXEC_WRITE_REQ: > - return ATT_OP_EXEC_WRITE_RESP; > + attr->att = bt_att_new(fd); > + if (!attr->att) > + goto fail; > > - case ATT_OP_HANDLE_IND: > - return ATT_OP_HANDLE_CNF; > - } > + attr->buf = g_malloc0(mtu); > + attr->buflen = mtu; > + if (!attr->buf) > + goto fail; > > - return 0; > -} > + attr->callbacks = g_queue_new(); > + if (!attr->callbacks) > + goto fail; > > -static bool is_response(guint8 opcode) > -{ > - switch (opcode) { > - case ATT_OP_ERROR: > - case ATT_OP_MTU_RESP: > - case ATT_OP_FIND_INFO_RESP: > - case ATT_OP_FIND_BY_TYPE_RESP: > - case ATT_OP_READ_BY_TYPE_RESP: > - case ATT_OP_READ_RESP: > - case ATT_OP_READ_BLOB_RESP: > - case ATT_OP_READ_MULTI_RESP: > - case ATT_OP_READ_BY_GROUP_RESP: > - case ATT_OP_WRITE_RESP: > - case ATT_OP_PREP_WRITE_RESP: > - case ATT_OP_EXEC_WRITE_RESP: > - case ATT_OP_HANDLE_CNF: > - return true; > - } > + return g_attrib_ref(attr); > > - return false; > -} > - > -static bool is_request(guint8 opcode) > -{ > - switch (opcode) { > - case ATT_OP_MTU_REQ: > - case ATT_OP_FIND_INFO_REQ: > - case ATT_OP_FIND_BY_TYPE_REQ: > - case ATT_OP_READ_BY_TYPE_REQ: > - case ATT_OP_READ_REQ: > - case ATT_OP_READ_BLOB_REQ: > - case ATT_OP_READ_MULTI_REQ: > - case ATT_OP_READ_BY_GROUP_REQ: > - case ATT_OP_WRITE_REQ: > - case ATT_OP_WRITE_CMD: > - case ATT_OP_PREP_WRITE_REQ: > - case ATT_OP_EXEC_WRITE_REQ: > - return true; > - } > - > - return false; > +fail: > + free(attr->buf); > + bt_att_unref(attr->att); > + g_io_channel_unref(io); > + free(attr); > + return NULL; > } > > GAttrib *g_attrib_ref(GAttrib *attrib) > { > - int refs; > - > if (!attrib) > return NULL; > > - refs = __sync_add_and_fetch(&attrib->refs, 1); > + __sync_fetch_and_add(&attrib->ref_count, 1); > > - DBG("%p: ref=%d", attrib, refs); > + DBG("%p: g_attrib_ref=%d ", attrib, attrib->ref_count); > > return attrib; > } > > -static void command_destroy(struct command *cmd) > +static void attrib_callbacks_destroy(void *user_data) > { > - if (cmd->notify) > - cmd->notify(cmd->user_data); > + struct attrib_callbacks *cb; > > - g_free(cmd->pdu); > - g_free(cmd); > -} > + cb = (struct attrib_callbacks *)user_data; This cast shouldn't be needed, user_data is void*. > + if (!user_data || !g_queue_remove(cb->parent->callbacks, user_data)) > + return; > > -static void event_destroy(struct event *evt) > -{ > - if (evt->notify) > - evt->notify(evt->user_data); > + if (cb->destroy_func) > + cb->destroy_func(cb->user_data); > > - g_free(evt); > + free(user_data); > } > > -static void attrib_destroy(GAttrib *attrib) > +void g_attrib_unref(GAttrib *attrib) > { > - GSList *l; > - struct command *c; > - > - while ((c = g_queue_pop_head(attrib->requests))) > - command_destroy(c); > - > - while ((c = g_queue_pop_head(attrib->responses))) > - command_destroy(c); > - > - g_queue_free(attrib->requests); > - attrib->requests = NULL; > - > - g_queue_free(attrib->responses); > - attrib->responses = NULL; > + struct attrib_callbacks *cb; > > - for (l = attrib->events; l; l = l->next) > - event_destroy(l->data); > - > - g_slist_free(attrib->events); > - attrib->events = NULL; > - > - if (attrib->timeout_watch > 0) > - g_source_remove(attrib->timeout_watch); > - > - if (attrib->write_watch > 0) > - g_source_remove(attrib->write_watch); > - > - if (attrib->read_watch > 0) > - g_source_remove(attrib->read_watch); > + if (!attrib) > + return; > > - if (attrib->io) > - g_io_channel_unref(attrib->io); > + DBG("%p: g_attrib_unref=%d ", attrib, attrib->ref_count-1); > > - g_free(attrib->buf); > + if (__sync_sub_and_fetch(&attrib->ref_count, 1)) > + return; > > if (attrib->destroy) > attrib->destroy(attrib->destroy_user_data); > > - g_free(attrib); > -} > + while ((cb = g_queue_peek_head(attrib->callbacks))) > + attrib_callbacks_destroy(cb); > > -void g_attrib_unref(GAttrib *attrib) > -{ > - int refs; > + g_queue_free(attrib->callbacks); > > - if (!attrib) > - return; > - > - refs = __sync_sub_and_fetch(&attrib->refs, 1); > + g_free(attrib->buf); > > - DBG("%p: ref=%d", attrib, refs); > + bt_att_unref(attrib->att); > > - if (refs > 0) > - return; > + g_io_channel_unref(attrib->io); > > - attrib_destroy(attrib); > + g_free(attrib); > } > > GIOChannel *g_attrib_get_channel(GAttrib *attrib) > @@ -270,7 +167,7 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib) > gboolean g_attrib_set_destroy_function(GAttrib *attrib, > GDestroyNotify destroy, gpointer user_data) > { > - if (attrib == NULL) > + if (!attrib) > return FALSE; > > attrib->destroy = destroy; > @@ -279,380 +176,130 @@ gboolean g_attrib_set_destroy_function(GAttrib > *attrib, return TRUE; > } > > -static gboolean disconnect_timeout(gpointer data) > -{ > - struct _GAttrib *attrib = data; > - struct command *c; > - > - g_attrib_ref(attrib); > - > - c = g_queue_pop_head(attrib->requests); > - if (c == NULL) > - goto done; > - > - if (c->func) > - c->func(ATT_ECODE_TIMEOUT, NULL, 0, c->user_data); > - > - command_destroy(c); > - > - while ((c = g_queue_pop_head(attrib->requests))) { > - if (c->func) > - c->func(ATT_ECODE_ABORTED, NULL, 0, c->user_data); > - command_destroy(c); > - } > > -done: > - attrib->stale = true; > - > - g_attrib_unref(attrib); > - > - return FALSE; > -} > - > -static gboolean can_write_data(GIOChannel *io, GIOCondition cond, > - gpointer data) > +static void attrib_callback_result(uint8_t opcode, const void *pdu, > + uint16_t length, void *user_data) > { > - struct _GAttrib *attrib = data; > - struct command *cmd; > - GError *gerr = NULL; > - gsize len; > - GIOStatus iostat; > - GQueue *queue; > - > - if (attrib->stale) > - return FALSE; > - > - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) > - return FALSE; > - > - queue = attrib->responses; > - cmd = g_queue_peek_head(queue); > - if (cmd == NULL) { > - queue = attrib->requests; > - cmd = g_queue_peek_head(queue); > - } > - if (cmd == NULL) > - return FALSE; > - > - /* > - * Verify that we didn't already send this command. This can only > - * happen with elementes from attrib->requests. > - */ > - if (cmd->sent) > - return FALSE; > + uint8_t *buf; > + struct attrib_callbacks *cb = user_data; > + guint8 status = 0; > > - iostat = g_io_channel_write_chars(io, (char *) cmd->pdu, cmd->len, > - &len, &gerr); > - if (iostat != G_IO_STATUS_NORMAL) { > - if (gerr) { > - error("%s", gerr->message); > - g_error_free(gerr); > - } > + if (!cb) > + return; > > - return FALSE; > - } > + buf = g_malloc0(length+1); > + if (!buf) > + return; g_malloc0() will abort if memory allocation failed, should this be g_malloc0_try() ? Also if this is not due to glib API requirement we should be using malloc0(). Also there should be spaces around +. (this is affecting multiple places) > > - if (cmd->expected == 0) { > - g_queue_pop_head(queue); > - command_destroy(cmd); > + buf[0] = opcode; > + memcpy(buf+1, pdu, length); > > - return TRUE; > + if (opcode == BT_ATT_OP_ERROR_RSP) { > + if (length < 4) Should be < 5 if we are accessing buf[4]. > + status = BT_ATT_ERROR_UNLIKELY; > + else > + status = buf[4]; > } > > - cmd->sent = true; > - > - if (attrib->timeout_watch == 0) > - attrib->timeout_watch = g_timeout_add_seconds(GATT_TIMEOUT, > - disconnect_timeout, attrib); > + if (cb->result_func) > + cb->result_func(status, buf, length+1, cb->user_data); > > - return FALSE; > + g_free(buf); > } > > -static void destroy_sender(gpointer data) > -{ > - struct _GAttrib *attrib = data; > > - attrib->write_watch = 0; > - g_attrib_unref(attrib); > -} > - > -static void wake_up_sender(struct _GAttrib *attrib) > -{ > - if (attrib->write_watch > 0) > - return; > - > - attrib = g_attrib_ref(attrib); > - attrib->write_watch = g_io_add_watch_full(attrib->io, > - G_PRIORITY_DEFAULT, G_IO_OUT, > - can_write_data, attrib, destroy_sender); > -} > - > -static bool match_event(struct event *evt, const uint8_t *pdu, gsize len) > -{ > - guint16 handle; > - > - if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS) > - return true; > - > - if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES) > - return true; > - > - if (len < 3) > - return false; > - > - handle = get_le16(&pdu[1]); > - > - if (evt->expected == pdu[0] && evt->handle == handle) > - return true; > - > - return false; > -} > - > -static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer > data) +static void attrib_callback_notify(uint8_t opcode, const void *pdu, > + uint16_t length, void *user_data) > { > - struct _GAttrib *attrib = data; > - struct command *cmd = NULL; > - GSList *l; > - uint8_t buf[512], status; > - gsize len; > - GIOStatus iostat; > - > - if (attrib->stale) > - return FALSE; > - > - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) { > - struct command *c; > - > - while ((c = g_queue_pop_head(attrib->requests))) { > - if (c->func) > - c->func(ATT_ECODE_IO, NULL, 0, c->user_data); > - command_destroy(c); > - } > - > - attrib->read_watch = 0; > - > - return FALSE; > - } > - > - memset(buf, 0, sizeof(buf)); > - > - iostat = g_io_channel_read_chars(io, (char *) buf, sizeof(buf), > - &len, NULL); > - if (iostat != G_IO_STATUS_NORMAL) { > - status = ATT_ECODE_IO; > - goto done; > - } > - > - for (l = attrib->events; l; l = l->next) { > - struct event *evt = l->data; > - > - if (match_event(evt, buf, len)) > - evt->func(buf, len, evt->user_data); > - } > - > - if (!is_response(buf[0])) > - return TRUE; > - > - if (attrib->timeout_watch > 0) { > - g_source_remove(attrib->timeout_watch); > - attrib->timeout_watch = 0; > - } > - > - cmd = g_queue_pop_head(attrib->requests); > - if (cmd == NULL) { > - /* Keep the watch if we have events to report */ > - return attrib->events != NULL; > - } > - > - if (buf[0] == ATT_OP_ERROR) { > - status = buf[4]; > - goto done; > - } > - > - if (cmd->expected != buf[0]) { > - status = ATT_ECODE_IO; > - goto done; > - } > - > - status = 0; > - > -done: > - if (!g_queue_is_empty(attrib->requests) || > - !g_queue_is_empty(attrib->responses)) > - wake_up_sender(attrib); > - > - if (cmd) { > - if (cmd->func) > - cmd->func(status, buf, len, cmd->user_data); > - > - command_destroy(cmd); > - } > + uint8_t *buf; > + struct attrib_callbacks *cb = user_data; > > - return TRUE; > -} > + if (!cb) > + return; > > -GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) > -{ > - struct _GAttrib *attrib; > + if (cb->notify_func == NULL) Use ! for pointer checking. I know that we are not fully following this in full code base but lets be consistent in new code. > + return; > > - g_io_channel_set_encoding(io, NULL, NULL); > - g_io_channel_set_buffered(io, FALSE); > + if (cb->notify_handle != GATTRIB_ALL_HANDLES && length < 2) > + return; > > - attrib = g_try_new0(struct _GAttrib, 1); > - if (attrib == NULL) > - return NULL; > + if (cb->notify_handle != GATTRIB_ALL_HANDLES && > + cb->notify_handle != get_le16(pdu)) > + return; > > - attrib->buf = g_malloc0(mtu); > - attrib->buflen = mtu; > + buf = g_malloc0(length+1); > + if (!buf) > + return; Same here: use malloc0() or g_mallo0_try(). > > - attrib->io = g_io_channel_ref(io); > - attrib->requests = g_queue_new(); > - attrib->responses = g_queue_new(); > + buf[0] = opcode; > + memcpy(buf+1, pdu, length); > > - attrib->read_watch = g_io_add_watch(attrib->io, > - G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL, > - received_data, attrib); > + cb->notify_func(buf, length+1, cb->user_data); > > - return g_attrib_ref(attrib); > + g_free(buf); > } > > guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 > len, GAttribResultFunc func, gpointer user_data, > GDestroyNotify notify) > { > - struct command *c; > - GQueue *queue; > - uint8_t opcode; > - > - if (attrib->stale) > - return 0; > + struct attrib_callbacks *cb = NULL; > + bt_att_response_func_t response_cb = NULL; > + bt_att_destroy_func_t destroy_cb = NULL; > > - c = g_try_new0(struct command, 1); > - if (c == NULL) > + if (!pdu || !len) > return 0; > > - opcode = pdu[0]; > - > - c->opcode = opcode; > - c->expected = opcode2expected(opcode); > - c->pdu = g_malloc(len); > - memcpy(c->pdu, pdu, len); > - c->len = len; > - c->func = func; > - c->user_data = user_data; > - c->notify = notify; > - > - if (is_response(opcode)) > - queue = attrib->responses; > - else > - queue = attrib->requests; > - > - if (id) { > - c->id = id; > - if (!is_response(opcode)) > - g_queue_push_head(queue, c); > - else > - /* Don't re-order responses even if an ID is given */ > - g_queue_push_tail(queue, c); > - } else { > - c->id = ++attrib->next_cmd_id; > - g_queue_push_tail(queue, c); > + if (func || notify) { > + cb = new0(struct attrib_callbacks, 1); > + if (cb == 0) if (!cb) ... (this is affecting multiple places) > + return 0; > + cb->result_func = func; > + cb->user_data = user_data; > + cb->destroy_func = notify; > + cb->parent = attrib; > + g_queue_push_head(attrib->callbacks, cb); > + response_cb = attrib_callback_result; > + destroy_cb = attrib_callbacks_destroy; > } > > - /* > - * If a command was added to the queue and it was empty before, wake up > - * the sender. If the sender was already woken up by the second queue, > - * wake_up_sender will just return. > - */ > - if (g_queue_get_length(queue) == 1) > - wake_up_sender(attrib); > - > - return c->id; > -} > - > -static int command_cmp_by_id(gconstpointer a, gconstpointer b) > -{ > - const struct command *cmd = a; > - guint id = GPOINTER_TO_UINT(b); > - > - return cmd->id - id; > + return bt_att_send(attrib->att, pdu[0], (void *)pdu+1, len-1, > + response_cb, cb, destroy_cb); > } > > gboolean g_attrib_cancel(GAttrib *attrib, guint id) > { > - GList *l = NULL; > - struct command *cmd; > - GQueue *queue; > - > - if (attrib == NULL) > - return FALSE; > - > - queue = attrib->requests; > - if (queue) > - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), > - command_cmp_by_id); > - if (l == NULL) { > - queue = attrib->responses; > - if (!queue) > - return FALSE; > - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), > - command_cmp_by_id); > - } > - > - if (l == NULL) > - return FALSE; > - > - cmd = l->data; > - > - if (cmd == g_queue_peek_head(queue) && cmd->sent) > - cmd->func = NULL; > - else { > - g_queue_remove(queue, cmd); > - command_destroy(cmd); > - } > - > - return TRUE; > + return bt_att_cancel(attrib->att, id); > } > > -static gboolean cancel_all_per_queue(GQueue *queue) > +gboolean g_attrib_cancel_all(GAttrib *attrib) > { > - struct command *c, *head = NULL; > - gboolean first = TRUE; > - > - if (queue == NULL) > - return FALSE; > - > - while ((c = g_queue_pop_head(queue))) { > - if (first && c->sent) { > - /* If the command was sent ignore its callback ... */ > - c->func = NULL; > - head = c; > - continue; > - } > - > - first = FALSE; > - command_destroy(c); > - } > - > - if (head) { > - /* ... and put it back in the queue */ > - g_queue_push_head(queue, head); > - } > - > - return TRUE; > + return bt_att_cancel_all(attrib->att); > } > > -gboolean g_attrib_cancel_all(GAttrib *attrib) > +guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, > + GAttribNotifyFunc func, gpointer user_data, > + GDestroyNotify notify) > { > - gboolean ret; > - > - if (attrib == NULL) > - return FALSE; > + struct attrib_callbacks *cb = NULL; > + > + if (func || notify) { > + cb = new0(struct attrib_callbacks, 1); > + if (cb == 0) > + return 0; > + cb->notify_func = func; > + cb->notify_handle = handle; > + cb->user_data = user_data; > + cb->destroy_func = notify; > + cb->parent = attrib; > + g_queue_push_head(attrib->callbacks, cb); > + } > > - ret = cancel_all_per_queue(attrib->requests); > - ret = cancel_all_per_queue(attrib->responses) && ret; > + if (opcode == GATTRIB_ALL_REQS) > + opcode = BT_ATT_ALL_REQUESTS; > > - return ret; > + return bt_att_register(attrib->att, opcode, attrib_callback_notify, > + cb, attrib_callbacks_destroy); > } > > uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) > @@ -661,98 +308,26 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t > *len) return NULL; > > *len = attrib->buflen; > - > return attrib->buf; > } > > gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu) > { > - if (mtu < ATT_DEFAULT_LE_MTU) > - return FALSE; > - > - attrib->buf = g_realloc(attrib->buf, mtu); > - > - attrib->buflen = mtu; > - > - return TRUE; > -} > - > -guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, > - GAttribNotifyFunc func, gpointer user_data, > - GDestroyNotify notify) > -{ > - static guint next_evt_id = 0; > - struct event *event; > - > - event = g_try_new0(struct event, 1); > - if (event == NULL) > - return 0; > - > - event->expected = opcode; > - event->handle = handle; > - event->func = func; > - event->user_data = user_data; > - event->notify = notify; > - event->id = ++next_evt_id; > - > - attrib->events = g_slist_append(attrib->events, event); > - > - return event->id; > -} > - > -static int event_cmp_by_id(gconstpointer a, gconstpointer b) > -{ > - const struct event *evt = a; > - guint id = GPOINTER_TO_UINT(b); > + /* Clients of this expect a buffer to use. */ > + if (mtu > attrib->buflen) { > + attrib->buf = g_realloc(attrib->buf, mtu); > + attrib->buflen = mtu; > + } > > - return evt->id - id; > + return bt_att_set_mtu(attrib->att, mtu); > } > > gboolean g_attrib_unregister(GAttrib *attrib, guint id) > { > - struct event *evt; > - GSList *l; > - > - if (id == 0) { > - warn("%s: invalid id", __func__); > - return FALSE; > - } > - > - l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id), > - event_cmp_by_id); > - if (l == NULL) > - return FALSE; > - > - evt = l->data; > - > - attrib->events = g_slist_remove(attrib->events, evt); > - > - if (evt->notify) > - evt->notify(evt->user_data); > - > - g_free(evt); > - > - return TRUE; > + return bt_att_unregister(attrib->att, id); > } > > gboolean g_attrib_unregister_all(GAttrib *attrib) > { > - GSList *l; > - > - if (attrib->events == NULL) > - return FALSE; > - > - for (l = attrib->events; l; l = l->next) { > - struct event *evt = l->data; > - > - if (evt->notify) > - evt->notify(evt->user_data); > - > - g_free(evt); > - } > - > - g_slist_free(attrib->events); > - attrib->events = NULL; > - > - return TRUE; > + return bt_att_unregister_all(attrib->att); > } -- BR Szymon Janc ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-12 21:14 ` Szymon Janc @ 2014-11-13 0:49 ` Arman Uguray 2014-11-13 14:05 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 16+ messages in thread From: Arman Uguray @ 2014-11-13 0:49 UTC (permalink / raw) To: Szymon Janc; +Cc: BlueZ development Hi Michael & Szymon, > On Wed, Nov 12, 2014 at 1:14 PM, Szymon Janc <szymon.janc@tieto.com> wrote: > Hi Michael, > > On Wednesday 12 of November 2014 12:22:59 Michael Janssen wrote: >> This patch implements a version of GAttrib which is backed by >> bt_att, which enables the simultaneous use of GAttrib and bt_att. >> >> This should enable smooth transition of profiles from the GAttrib >> API to the src/shared bt_att API. >> --- >> attrib/gattrib.c | 733 >> ++++++++++++------------------------------------------- 1 file changed, 154 >> insertions(+), 579 deletions(-) >> >> diff --git a/attrib/gattrib.c b/attrib/gattrib.c >> index fa51b6d..b55e437 100644 >> --- a/attrib/gattrib.c >> +++ b/attrib/gattrib.c >> @@ -36,227 +36,124 @@ >> #include <bluetooth/bluetooth.h> >> >> #include "btio/btio.h" >> -#include "lib/uuid.h" >> -#include "src/shared/util.h" >> #include "src/log.h" >> -#include "attrib/att.h" >> +#include "src/shared/util.h" >> +#include "src/shared/att.h" >> #include "attrib/gattrib.h" >> >> -#define GATT_TIMEOUT 30 >> - >> struct _GAttrib { >> + int ref_count; >> + struct bt_att *att; >> GIOChannel *io; >> - int refs; >> - uint8_t *buf; >> - size_t buflen; >> - guint read_watch; >> - guint write_watch; >> - guint timeout_watch; >> - GQueue *requests; >> - GQueue *responses; >> - GSList *events; >> - guint next_cmd_id; >> GDestroyNotify destroy; >> gpointer destroy_user_data; >> - bool stale; >> + GQueue *callbacks; >> + uint8_t *buf; >> + int buflen; >> }; >> >> -struct command { >> - guint id; >> - guint8 opcode; >> - guint8 *pdu; >> - guint16 len; >> - guint8 expected; >> - bool sent; >> - GAttribResultFunc func; >> - gpointer user_data; >> - GDestroyNotify notify; >> -}; >> >> -struct event { >> - guint id; >> - guint8 expected; >> - guint16 handle; >> - GAttribNotifyFunc func; >> +struct attrib_callbacks { >> + GAttribResultFunc result_func; >> + GAttribNotifyFunc notify_func; >> + GDestroyNotify destroy_func; >> gpointer user_data; >> - GDestroyNotify notify; >> + GAttrib *parent; >> + uint16_t notify_handle; >> }; >> >> -static guint8 opcode2expected(guint8 opcode) >> +GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) >> { >> - switch (opcode) { >> - case ATT_OP_MTU_REQ: >> - return ATT_OP_MTU_RESP; >> - >> - case ATT_OP_FIND_INFO_REQ: >> - return ATT_OP_FIND_INFO_RESP; >> - >> - case ATT_OP_FIND_BY_TYPE_REQ: >> - return ATT_OP_FIND_BY_TYPE_RESP; >> - >> - case ATT_OP_READ_BY_TYPE_REQ: >> - return ATT_OP_READ_BY_TYPE_RESP; >> - >> - case ATT_OP_READ_REQ: >> - return ATT_OP_READ_RESP; >> - >> - case ATT_OP_READ_BLOB_REQ: >> - return ATT_OP_READ_BLOB_RESP; >> - >> - case ATT_OP_READ_MULTI_REQ: >> - return ATT_OP_READ_MULTI_RESP; >> + gint fd; >> + GAttrib *attr; >> >> - case ATT_OP_READ_BY_GROUP_REQ: >> - return ATT_OP_READ_BY_GROUP_RESP; >> + if (!io) >> + return NULL; >> >> - case ATT_OP_WRITE_REQ: >> - return ATT_OP_WRITE_RESP; >> + fd = g_io_channel_unix_get_fd(io); >> + attr = new0(GAttrib, 1); >> + if (!attr) >> + return NULL; >> >> - case ATT_OP_PREP_WRITE_REQ: >> - return ATT_OP_PREP_WRITE_RESP; >> + g_io_channel_ref(io); >> + attr->io = io; >> >> - case ATT_OP_EXEC_WRITE_REQ: >> - return ATT_OP_EXEC_WRITE_RESP; >> + attr->att = bt_att_new(fd); >> + if (!attr->att) >> + goto fail; >> >> - case ATT_OP_HANDLE_IND: >> - return ATT_OP_HANDLE_CNF; >> - } >> + attr->buf = g_malloc0(mtu); >> + attr->buflen = mtu; >> + if (!attr->buf) >> + goto fail; >> >> - return 0; >> -} >> + attr->callbacks = g_queue_new(); >> + if (!attr->callbacks) >> + goto fail; >> >> -static bool is_response(guint8 opcode) >> -{ >> - switch (opcode) { >> - case ATT_OP_ERROR: >> - case ATT_OP_MTU_RESP: >> - case ATT_OP_FIND_INFO_RESP: >> - case ATT_OP_FIND_BY_TYPE_RESP: >> - case ATT_OP_READ_BY_TYPE_RESP: >> - case ATT_OP_READ_RESP: >> - case ATT_OP_READ_BLOB_RESP: >> - case ATT_OP_READ_MULTI_RESP: >> - case ATT_OP_READ_BY_GROUP_RESP: >> - case ATT_OP_WRITE_RESP: >> - case ATT_OP_PREP_WRITE_RESP: >> - case ATT_OP_EXEC_WRITE_RESP: >> - case ATT_OP_HANDLE_CNF: >> - return true; >> - } >> + return g_attrib_ref(attr); >> >> - return false; >> -} >> - >> -static bool is_request(guint8 opcode) >> -{ >> - switch (opcode) { >> - case ATT_OP_MTU_REQ: >> - case ATT_OP_FIND_INFO_REQ: >> - case ATT_OP_FIND_BY_TYPE_REQ: >> - case ATT_OP_READ_BY_TYPE_REQ: >> - case ATT_OP_READ_REQ: >> - case ATT_OP_READ_BLOB_REQ: >> - case ATT_OP_READ_MULTI_REQ: >> - case ATT_OP_READ_BY_GROUP_REQ: >> - case ATT_OP_WRITE_REQ: >> - case ATT_OP_WRITE_CMD: >> - case ATT_OP_PREP_WRITE_REQ: >> - case ATT_OP_EXEC_WRITE_REQ: >> - return true; >> - } >> - >> - return false; >> +fail: >> + free(attr->buf); >> + bt_att_unref(attr->att); >> + g_io_channel_unref(io); >> + free(attr); >> + return NULL; >> } >> >> GAttrib *g_attrib_ref(GAttrib *attrib) >> { >> - int refs; >> - >> if (!attrib) >> return NULL; >> >> - refs = __sync_add_and_fetch(&attrib->refs, 1); >> + __sync_fetch_and_add(&attrib->ref_count, 1); >> >> - DBG("%p: ref=%d", attrib, refs); >> + DBG("%p: g_attrib_ref=%d ", attrib, attrib->ref_count); >> >> return attrib; >> } >> >> -static void command_destroy(struct command *cmd) >> +static void attrib_callbacks_destroy(void *user_data) >> { >> - if (cmd->notify) >> - cmd->notify(cmd->user_data); >> + struct attrib_callbacks *cb; >> >> - g_free(cmd->pdu); >> - g_free(cmd); >> -} >> + cb = (struct attrib_callbacks *)user_data; > > This cast shouldn't be needed, user_data is void*. > >> + if (!user_data || !g_queue_remove(cb->parent->callbacks, user_data)) >> + return; >> >> -static void event_destroy(struct event *evt) >> -{ >> - if (evt->notify) >> - evt->notify(evt->user_data); >> + if (cb->destroy_func) >> + cb->destroy_func(cb->user_data); >> >> - g_free(evt); >> + free(user_data); >> } >> >> -static void attrib_destroy(GAttrib *attrib) >> +void g_attrib_unref(GAttrib *attrib) >> { >> - GSList *l; >> - struct command *c; >> - >> - while ((c = g_queue_pop_head(attrib->requests))) >> - command_destroy(c); >> - >> - while ((c = g_queue_pop_head(attrib->responses))) >> - command_destroy(c); >> - >> - g_queue_free(attrib->requests); >> - attrib->requests = NULL; >> - >> - g_queue_free(attrib->responses); >> - attrib->responses = NULL; >> + struct attrib_callbacks *cb; >> >> - for (l = attrib->events; l; l = l->next) >> - event_destroy(l->data); >> - >> - g_slist_free(attrib->events); >> - attrib->events = NULL; >> - >> - if (attrib->timeout_watch > 0) >> - g_source_remove(attrib->timeout_watch); >> - >> - if (attrib->write_watch > 0) >> - g_source_remove(attrib->write_watch); >> - >> - if (attrib->read_watch > 0) >> - g_source_remove(attrib->read_watch); >> + if (!attrib) >> + return; >> >> - if (attrib->io) >> - g_io_channel_unref(attrib->io); >> + DBG("%p: g_attrib_unref=%d ", attrib, attrib->ref_count-1); >> >> - g_free(attrib->buf); >> + if (__sync_sub_and_fetch(&attrib->ref_count, 1)) >> + return; >> >> if (attrib->destroy) >> attrib->destroy(attrib->destroy_user_data); >> >> - g_free(attrib); >> -} >> + while ((cb = g_queue_peek_head(attrib->callbacks))) >> + attrib_callbacks_destroy(cb); >> >> -void g_attrib_unref(GAttrib *attrib) >> -{ >> - int refs; >> + g_queue_free(attrib->callbacks); >> >> - if (!attrib) >> - return; >> - >> - refs = __sync_sub_and_fetch(&attrib->refs, 1); >> + g_free(attrib->buf); >> >> - DBG("%p: ref=%d", attrib, refs); >> + bt_att_unref(attrib->att); >> >> - if (refs > 0) >> - return; >> + g_io_channel_unref(attrib->io); >> >> - attrib_destroy(attrib); >> + g_free(attrib); >> } >> >> GIOChannel *g_attrib_get_channel(GAttrib *attrib) >> @@ -270,7 +167,7 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib) >> gboolean g_attrib_set_destroy_function(GAttrib *attrib, >> GDestroyNotify destroy, gpointer user_data) >> { >> - if (attrib == NULL) >> + if (!attrib) >> return FALSE; >> >> attrib->destroy = destroy; >> @@ -279,380 +176,130 @@ gboolean g_attrib_set_destroy_function(GAttrib >> *attrib, return TRUE; >> } >> >> -static gboolean disconnect_timeout(gpointer data) >> -{ >> - struct _GAttrib *attrib = data; >> - struct command *c; >> - >> - g_attrib_ref(attrib); >> - >> - c = g_queue_pop_head(attrib->requests); >> - if (c == NULL) >> - goto done; >> - >> - if (c->func) >> - c->func(ATT_ECODE_TIMEOUT, NULL, 0, c->user_data); >> - >> - command_destroy(c); >> - >> - while ((c = g_queue_pop_head(attrib->requests))) { >> - if (c->func) >> - c->func(ATT_ECODE_ABORTED, NULL, 0, c->user_data); >> - command_destroy(c); >> - } >> >> -done: >> - attrib->stale = true; >> - >> - g_attrib_unref(attrib); >> - >> - return FALSE; >> -} >> - >> -static gboolean can_write_data(GIOChannel *io, GIOCondition cond, >> - gpointer data) >> +static void attrib_callback_result(uint8_t opcode, const void *pdu, >> + uint16_t length, void *user_data) >> { >> - struct _GAttrib *attrib = data; >> - struct command *cmd; >> - GError *gerr = NULL; >> - gsize len; >> - GIOStatus iostat; >> - GQueue *queue; >> - >> - if (attrib->stale) >> - return FALSE; >> - >> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) >> - return FALSE; >> - >> - queue = attrib->responses; >> - cmd = g_queue_peek_head(queue); >> - if (cmd == NULL) { >> - queue = attrib->requests; >> - cmd = g_queue_peek_head(queue); >> - } >> - if (cmd == NULL) >> - return FALSE; >> - >> - /* >> - * Verify that we didn't already send this command. This can only >> - * happen with elementes from attrib->requests. >> - */ >> - if (cmd->sent) >> - return FALSE; >> + uint8_t *buf; >> + struct attrib_callbacks *cb = user_data; >> + guint8 status = 0; >> >> - iostat = g_io_channel_write_chars(io, (char *) cmd->pdu, cmd->len, >> - &len, &gerr); >> - if (iostat != G_IO_STATUS_NORMAL) { >> - if (gerr) { >> - error("%s", gerr->message); >> - g_error_free(gerr); >> - } >> + if (!cb) >> + return; >> >> - return FALSE; >> - } >> + buf = g_malloc0(length+1); >> + if (!buf) >> + return; > > g_malloc0() will abort if memory allocation failed, should this be > g_malloc0_try() ? Also if this is not due to glib API requirement we should > be using malloc0(). > > Also there should be spaces around +. (this is affecting multiple places) > >> >> - if (cmd->expected == 0) { >> - g_queue_pop_head(queue); >> - command_destroy(cmd); >> + buf[0] = opcode; >> + memcpy(buf+1, pdu, length); >> >> - return TRUE; >> + if (opcode == BT_ATT_OP_ERROR_RSP) { >> + if (length < 4) > > Should be < 5 if we are accessing buf[4]. > The PDU returned by bt_att doesn't include the opcode. To convert it into the format expected by GAttrib, he copied "pdu" in to "buf" which is one byte larger. So, the code is technically correct if not very readable. Michael, please add a comment here to explain why this is OK. If you're checking "length", then maybe assign from "pdu[3]" instead of "buf" for consistency, and explain above why you're doing a memcpy to "buf + 1". >> + status = BT_ATT_ERROR_UNLIKELY; >> + else >> + status = buf[4]; >> } >> >> - cmd->sent = true; >> - >> - if (attrib->timeout_watch == 0) >> - attrib->timeout_watch = g_timeout_add_seconds(GATT_TIMEOUT, >> - disconnect_timeout, attrib); >> + if (cb->result_func) >> + cb->result_func(status, buf, length+1, cb->user_data); >> >> - return FALSE; >> + g_free(buf); >> } >> >> -static void destroy_sender(gpointer data) >> -{ >> - struct _GAttrib *attrib = data; >> >> - attrib->write_watch = 0; >> - g_attrib_unref(attrib); >> -} >> - >> -static void wake_up_sender(struct _GAttrib *attrib) >> -{ >> - if (attrib->write_watch > 0) >> - return; >> - >> - attrib = g_attrib_ref(attrib); >> - attrib->write_watch = g_io_add_watch_full(attrib->io, >> - G_PRIORITY_DEFAULT, G_IO_OUT, >> - can_write_data, attrib, destroy_sender); >> -} >> - >> -static bool match_event(struct event *evt, const uint8_t *pdu, gsize len) >> -{ >> - guint16 handle; >> - >> - if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS) >> - return true; >> - >> - if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES) >> - return true; >> - >> - if (len < 3) >> - return false; >> - >> - handle = get_le16(&pdu[1]); >> - >> - if (evt->expected == pdu[0] && evt->handle == handle) >> - return true; >> - >> - return false; >> -} >> - >> -static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer >> data) +static void attrib_callback_notify(uint8_t opcode, const void *pdu, >> + uint16_t length, void *user_data) >> { >> - struct _GAttrib *attrib = data; >> - struct command *cmd = NULL; >> - GSList *l; >> - uint8_t buf[512], status; >> - gsize len; >> - GIOStatus iostat; >> - >> - if (attrib->stale) >> - return FALSE; >> - >> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) { >> - struct command *c; >> - >> - while ((c = g_queue_pop_head(attrib->requests))) { >> - if (c->func) >> - c->func(ATT_ECODE_IO, NULL, 0, c->user_data); >> - command_destroy(c); >> - } >> - >> - attrib->read_watch = 0; >> - >> - return FALSE; >> - } >> - >> - memset(buf, 0, sizeof(buf)); >> - >> - iostat = g_io_channel_read_chars(io, (char *) buf, sizeof(buf), >> - &len, NULL); >> - if (iostat != G_IO_STATUS_NORMAL) { >> - status = ATT_ECODE_IO; >> - goto done; >> - } >> - >> - for (l = attrib->events; l; l = l->next) { >> - struct event *evt = l->data; >> - >> - if (match_event(evt, buf, len)) >> - evt->func(buf, len, evt->user_data); >> - } >> - >> - if (!is_response(buf[0])) >> - return TRUE; >> - >> - if (attrib->timeout_watch > 0) { >> - g_source_remove(attrib->timeout_watch); >> - attrib->timeout_watch = 0; >> - } >> - >> - cmd = g_queue_pop_head(attrib->requests); >> - if (cmd == NULL) { >> - /* Keep the watch if we have events to report */ >> - return attrib->events != NULL; >> - } >> - >> - if (buf[0] == ATT_OP_ERROR) { >> - status = buf[4]; >> - goto done; >> - } >> - >> - if (cmd->expected != buf[0]) { >> - status = ATT_ECODE_IO; >> - goto done; >> - } >> - >> - status = 0; >> - >> -done: >> - if (!g_queue_is_empty(attrib->requests) || >> - !g_queue_is_empty(attrib->responses)) >> - wake_up_sender(attrib); >> - >> - if (cmd) { >> - if (cmd->func) >> - cmd->func(status, buf, len, cmd->user_data); >> - >> - command_destroy(cmd); >> - } >> + uint8_t *buf; >> + struct attrib_callbacks *cb = user_data; >> >> - return TRUE; >> -} >> + if (!cb) >> + return; >> >> -GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) >> -{ >> - struct _GAttrib *attrib; >> + if (cb->notify_func == NULL) > > Use ! for pointer checking. I know that we are not fully following this in > full code base but lets be consistent in new code. > >> + return; >> >> - g_io_channel_set_encoding(io, NULL, NULL); >> - g_io_channel_set_buffered(io, FALSE); >> + if (cb->notify_handle != GATTRIB_ALL_HANDLES && length < 2) >> + return; >> >> - attrib = g_try_new0(struct _GAttrib, 1); >> - if (attrib == NULL) >> - return NULL; >> + if (cb->notify_handle != GATTRIB_ALL_HANDLES && >> + cb->notify_handle != get_le16(pdu)) >> + return; >> >> - attrib->buf = g_malloc0(mtu); >> - attrib->buflen = mtu; >> + buf = g_malloc0(length+1); >> + if (!buf) >> + return; > > Same here: use malloc0() or g_mallo0_try(). > >> >> - attrib->io = g_io_channel_ref(io); >> - attrib->requests = g_queue_new(); >> - attrib->responses = g_queue_new(); >> + buf[0] = opcode; >> + memcpy(buf+1, pdu, length); >> >> - attrib->read_watch = g_io_add_watch(attrib->io, >> - G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL, >> - received_data, attrib); >> + cb->notify_func(buf, length+1, cb->user_data); >> >> - return g_attrib_ref(attrib); >> + g_free(buf); >> } >> >> guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 >> len, GAttribResultFunc func, gpointer user_data, >> GDestroyNotify notify) >> { >> - struct command *c; >> - GQueue *queue; >> - uint8_t opcode; >> - >> - if (attrib->stale) >> - return 0; >> + struct attrib_callbacks *cb = NULL; >> + bt_att_response_func_t response_cb = NULL; >> + bt_att_destroy_func_t destroy_cb = NULL; >> >> - c = g_try_new0(struct command, 1); >> - if (c == NULL) >> + if (!pdu || !len) >> return 0; >> >> - opcode = pdu[0]; >> - >> - c->opcode = opcode; >> - c->expected = opcode2expected(opcode); >> - c->pdu = g_malloc(len); >> - memcpy(c->pdu, pdu, len); >> - c->len = len; >> - c->func = func; >> - c->user_data = user_data; >> - c->notify = notify; >> - >> - if (is_response(opcode)) >> - queue = attrib->responses; >> - else >> - queue = attrib->requests; >> - >> - if (id) { >> - c->id = id; >> - if (!is_response(opcode)) >> - g_queue_push_head(queue, c); >> - else >> - /* Don't re-order responses even if an ID is given */ >> - g_queue_push_tail(queue, c); >> - } else { >> - c->id = ++attrib->next_cmd_id; >> - g_queue_push_tail(queue, c); >> + if (func || notify) { >> + cb = new0(struct attrib_callbacks, 1); >> + if (cb == 0) > > if (!cb) ... (this is affecting multiple places) > >> + return 0; >> + cb->result_func = func; >> + cb->user_data = user_data; >> + cb->destroy_func = notify; >> + cb->parent = attrib; >> + g_queue_push_head(attrib->callbacks, cb); >> + response_cb = attrib_callback_result; >> + destroy_cb = attrib_callbacks_destroy; >> } >> >> - /* >> - * If a command was added to the queue and it was empty before, wake up >> - * the sender. If the sender was already woken up by the second queue, >> - * wake_up_sender will just return. >> - */ >> - if (g_queue_get_length(queue) == 1) >> - wake_up_sender(attrib); >> - >> - return c->id; >> -} >> - >> -static int command_cmp_by_id(gconstpointer a, gconstpointer b) >> -{ >> - const struct command *cmd = a; >> - guint id = GPOINTER_TO_UINT(b); >> - >> - return cmd->id - id; >> + return bt_att_send(attrib->att, pdu[0], (void *)pdu+1, len-1, >> + response_cb, cb, destroy_cb); >> } >> >> gboolean g_attrib_cancel(GAttrib *attrib, guint id) >> { >> - GList *l = NULL; >> - struct command *cmd; >> - GQueue *queue; >> - >> - if (attrib == NULL) >> - return FALSE; >> - >> - queue = attrib->requests; >> - if (queue) >> - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), >> - command_cmp_by_id); >> - if (l == NULL) { >> - queue = attrib->responses; >> - if (!queue) >> - return FALSE; >> - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), >> - command_cmp_by_id); >> - } >> - >> - if (l == NULL) >> - return FALSE; >> - >> - cmd = l->data; >> - >> - if (cmd == g_queue_peek_head(queue) && cmd->sent) >> - cmd->func = NULL; >> - else { >> - g_queue_remove(queue, cmd); >> - command_destroy(cmd); >> - } >> - >> - return TRUE; >> + return bt_att_cancel(attrib->att, id); >> } >> >> -static gboolean cancel_all_per_queue(GQueue *queue) >> +gboolean g_attrib_cancel_all(GAttrib *attrib) >> { >> - struct command *c, *head = NULL; >> - gboolean first = TRUE; >> - >> - if (queue == NULL) >> - return FALSE; >> - >> - while ((c = g_queue_pop_head(queue))) { >> - if (first && c->sent) { >> - /* If the command was sent ignore its callback ... */ >> - c->func = NULL; >> - head = c; >> - continue; >> - } >> - >> - first = FALSE; >> - command_destroy(c); >> - } >> - >> - if (head) { >> - /* ... and put it back in the queue */ >> - g_queue_push_head(queue, head); >> - } >> - >> - return TRUE; >> + return bt_att_cancel_all(attrib->att); >> } >> >> -gboolean g_attrib_cancel_all(GAttrib *attrib) >> +guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, >> + GAttribNotifyFunc func, gpointer user_data, >> + GDestroyNotify notify) >> { >> - gboolean ret; >> - >> - if (attrib == NULL) >> - return FALSE; >> + struct attrib_callbacks *cb = NULL; >> + >> + if (func || notify) { >> + cb = new0(struct attrib_callbacks, 1); >> + if (cb == 0) >> + return 0; >> + cb->notify_func = func; >> + cb->notify_handle = handle; >> + cb->user_data = user_data; >> + cb->destroy_func = notify; >> + cb->parent = attrib; >> + g_queue_push_head(attrib->callbacks, cb); >> + } >> >> - ret = cancel_all_per_queue(attrib->requests); >> - ret = cancel_all_per_queue(attrib->responses) && ret; >> + if (opcode == GATTRIB_ALL_REQS) >> + opcode = BT_ATT_ALL_REQUESTS; >> >> - return ret; >> + return bt_att_register(attrib->att, opcode, attrib_callback_notify, >> + cb, attrib_callbacks_destroy); >> } >> >> uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) >> @@ -661,98 +308,26 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t >> *len) return NULL; >> >> *len = attrib->buflen; >> - >> return attrib->buf; >> } >> >> gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu) >> { >> - if (mtu < ATT_DEFAULT_LE_MTU) >> - return FALSE; >> - >> - attrib->buf = g_realloc(attrib->buf, mtu); >> - >> - attrib->buflen = mtu; >> - >> - return TRUE; >> -} >> - >> -guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, >> - GAttribNotifyFunc func, gpointer user_data, >> - GDestroyNotify notify) >> -{ >> - static guint next_evt_id = 0; >> - struct event *event; >> - >> - event = g_try_new0(struct event, 1); >> - if (event == NULL) >> - return 0; >> - >> - event->expected = opcode; >> - event->handle = handle; >> - event->func = func; >> - event->user_data = user_data; >> - event->notify = notify; >> - event->id = ++next_evt_id; >> - >> - attrib->events = g_slist_append(attrib->events, event); >> - >> - return event->id; >> -} >> - >> -static int event_cmp_by_id(gconstpointer a, gconstpointer b) >> -{ >> - const struct event *evt = a; >> - guint id = GPOINTER_TO_UINT(b); >> + /* Clients of this expect a buffer to use. */ >> + if (mtu > attrib->buflen) { >> + attrib->buf = g_realloc(attrib->buf, mtu); >> + attrib->buflen = mtu; >> + } >> >> - return evt->id - id; >> + return bt_att_set_mtu(attrib->att, mtu); >> } >> >> gboolean g_attrib_unregister(GAttrib *attrib, guint id) >> { >> - struct event *evt; >> - GSList *l; >> - >> - if (id == 0) { >> - warn("%s: invalid id", __func__); >> - return FALSE; >> - } >> - >> - l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id), >> - event_cmp_by_id); >> - if (l == NULL) >> - return FALSE; >> - >> - evt = l->data; >> - >> - attrib->events = g_slist_remove(attrib->events, evt); >> - >> - if (evt->notify) >> - evt->notify(evt->user_data); >> - >> - g_free(evt); >> - >> - return TRUE; >> + return bt_att_unregister(attrib->att, id); >> } >> >> gboolean g_attrib_unregister_all(GAttrib *attrib) >> { >> - GSList *l; >> - >> - if (attrib->events == NULL) >> - return FALSE; >> - >> - for (l = attrib->events; l; l = l->next) { >> - struct event *evt = l->data; >> - >> - if (evt->notify) >> - evt->notify(evt->user_data); >> - >> - g_free(evt); >> - } >> - >> - g_slist_free(attrib->events); >> - attrib->events = NULL; >> - >> - return TRUE; >> + return bt_att_unregister_all(attrib->att); >> } > > -- > BR > Szymon Janc > -- > 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 [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-13 0:49 ` Arman Uguray @ 2014-11-13 14:05 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 16+ messages in thread From: Luiz Augusto von Dentz @ 2014-11-13 14:05 UTC (permalink / raw) To: Arman Uguray; +Cc: Szymon Janc, BlueZ development Hi Michael, On Thu, Nov 13, 2014 at 2:49 AM, Arman Uguray <armansito@chromium.org> wrote: > Hi Michael & Szymon, > >> On Wed, Nov 12, 2014 at 1:14 PM, Szymon Janc <szymon.janc@tieto.com> wrote: >> Hi Michael, >> >> On Wednesday 12 of November 2014 12:22:59 Michael Janssen wrote: >>> This patch implements a version of GAttrib which is backed by >>> bt_att, which enables the simultaneous use of GAttrib and bt_att. >>> >>> This should enable smooth transition of profiles from the GAttrib >>> API to the src/shared bt_att API. >>> --- >>> attrib/gattrib.c | 733 >>> ++++++++++++------------------------------------------- 1 file changed, 154 >>> insertions(+), 579 deletions(-) I guess you forgot about the format flag this time so this changes should show up as complete rewrite. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH BlueZ 0/4] GAttrib cleanup and switch to bt_att @ 2014-11-04 21:19 Michael Janssen 2014-11-04 21:19 ` [PATCH BlueZ 4/4] GATT shim to src/shared bt_att Michael Janssen 0 siblings, 1 reply; 16+ messages in thread From: Michael Janssen @ 2014-11-04 21:19 UTC (permalink / raw) To: linux-bluetooth; +Cc: Michael Janssen This patch set cleans up gattrib and adds some missing functionality to switch GAttrib to a shim on top of bt_att. After this is merged, profiles using GAttrib can be transitioned to bt_att. Michael Janssen (4): unit-gattrib: Fix uninitialized variable gattrib: Remove unused GATTRIB_ALL_EVENTS shared/att: Add support for BT_ATT_ALL_REQUESTS GATT shim to src/shared bt_att attrib/gattrib.c | 737 ++++++++++--------------------------------------- attrib/gattrib.h | 1 - src/shared/att-types.h | 3 + src/shared/att.c | 11 +- unit/test-gattrib.c | 18 +- 5 files changed, 161 insertions(+), 609 deletions(-) -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-04 21:19 [PATCH BlueZ 0/4] GAttrib cleanup and switch to bt_att Michael Janssen @ 2014-11-04 21:19 ` Michael Janssen 2014-11-05 14:49 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 16+ messages in thread From: Michael Janssen @ 2014-11-04 21:19 UTC (permalink / raw) To: linux-bluetooth; +Cc: Michael Janssen This patch implements a version of GAttrib which is backed by bt_att, which enables the simultaneous use of GAttrib and bt_att. This should enable smooth transition of profiles from the GAttrib API to the src/shared bt_att API. --- attrib/gattrib.c | 734 +++++++++++-------------------------------------------- 1 file changed, 147 insertions(+), 587 deletions(-) diff --git a/attrib/gattrib.c b/attrib/gattrib.c index fa51b6d..63f219d 100644 --- a/attrib/gattrib.c +++ b/attrib/gattrib.c @@ -2,8 +2,7 @@ * * BlueZ - Bluetooth protocol stack for Linux * - * Copyright (C) 2010 Nokia Corporation - * Copyright (C) 2010 Marcel Holtmann <marcel@holtmann.org> + * Copyright (C) 2014 Google, Inc. * * * This program is free software; you can redistribute it and/or modify @@ -36,227 +35,124 @@ #include <bluetooth/bluetooth.h> #include "btio/btio.h" -#include "lib/uuid.h" -#include "src/shared/util.h" #include "src/log.h" -#include "attrib/att.h" +#include "src/shared/util.h" +#include "src/shared/att.h" #include "attrib/gattrib.h" -#define GATT_TIMEOUT 30 - struct _GAttrib { + int ref_count; + struct bt_att *att; GIOChannel *io; - int refs; - uint8_t *buf; - size_t buflen; - guint read_watch; - guint write_watch; - guint timeout_watch; - GQueue *requests; - GQueue *responses; - GSList *events; - guint next_cmd_id; GDestroyNotify destroy; gpointer destroy_user_data; - bool stale; + GQueue *callbacks; + uint8_t *buf; + int buflen; }; -struct command { - guint id; - guint8 opcode; - guint8 *pdu; - guint16 len; - guint8 expected; - bool sent; - GAttribResultFunc func; - gpointer user_data; - GDestroyNotify notify; -}; -struct event { - guint id; - guint8 expected; - guint16 handle; - GAttribNotifyFunc func; +struct attrib_callbacks { + GAttribResultFunc result_func; + GAttribNotifyFunc notify_func; + GDestroyNotify destroy_func; gpointer user_data; - GDestroyNotify notify; + GAttrib *parent; + uint16_t notify_handle; }; -static guint8 opcode2expected(guint8 opcode) +GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) { - switch (opcode) { - case ATT_OP_MTU_REQ: - return ATT_OP_MTU_RESP; - - case ATT_OP_FIND_INFO_REQ: - return ATT_OP_FIND_INFO_RESP; - - case ATT_OP_FIND_BY_TYPE_REQ: - return ATT_OP_FIND_BY_TYPE_RESP; - - case ATT_OP_READ_BY_TYPE_REQ: - return ATT_OP_READ_BY_TYPE_RESP; - - case ATT_OP_READ_REQ: - return ATT_OP_READ_RESP; - - case ATT_OP_READ_BLOB_REQ: - return ATT_OP_READ_BLOB_RESP; - - case ATT_OP_READ_MULTI_REQ: - return ATT_OP_READ_MULTI_RESP; + gint fd; + GAttrib *attr; - case ATT_OP_READ_BY_GROUP_REQ: - return ATT_OP_READ_BY_GROUP_RESP; + if (!io) + return NULL; - case ATT_OP_WRITE_REQ: - return ATT_OP_WRITE_RESP; + fd = g_io_channel_unix_get_fd(io); + attr = new0(GAttrib, 1); + if (!attr) + return NULL; - case ATT_OP_PREP_WRITE_REQ: - return ATT_OP_PREP_WRITE_RESP; + g_io_channel_ref(io); + attr->io = io; - case ATT_OP_EXEC_WRITE_REQ: - return ATT_OP_EXEC_WRITE_RESP; + attr->att = bt_att_new(fd); + if (!attr->att) + goto fail; - case ATT_OP_HANDLE_IND: - return ATT_OP_HANDLE_CNF; - } + attr->buf = g_malloc0(mtu); + attr->buflen = mtu; + if (!attr->buf) + goto fail; - return 0; -} + attr->callbacks = g_queue_new(); + if (!attr->callbacks) + goto fail; -static bool is_response(guint8 opcode) -{ - switch (opcode) { - case ATT_OP_ERROR: - case ATT_OP_MTU_RESP: - case ATT_OP_FIND_INFO_RESP: - case ATT_OP_FIND_BY_TYPE_RESP: - case ATT_OP_READ_BY_TYPE_RESP: - case ATT_OP_READ_RESP: - case ATT_OP_READ_BLOB_RESP: - case ATT_OP_READ_MULTI_RESP: - case ATT_OP_READ_BY_GROUP_RESP: - case ATT_OP_WRITE_RESP: - case ATT_OP_PREP_WRITE_RESP: - case ATT_OP_EXEC_WRITE_RESP: - case ATT_OP_HANDLE_CNF: - return true; - } + return g_attrib_ref(attr); - return false; -} - -static bool is_request(guint8 opcode) -{ - switch (opcode) { - case ATT_OP_MTU_REQ: - case ATT_OP_FIND_INFO_REQ: - case ATT_OP_FIND_BY_TYPE_REQ: - case ATT_OP_READ_BY_TYPE_REQ: - case ATT_OP_READ_REQ: - case ATT_OP_READ_BLOB_REQ: - case ATT_OP_READ_MULTI_REQ: - case ATT_OP_READ_BY_GROUP_REQ: - case ATT_OP_WRITE_REQ: - case ATT_OP_WRITE_CMD: - case ATT_OP_PREP_WRITE_REQ: - case ATT_OP_EXEC_WRITE_REQ: - return true; - } - - return false; +fail: + free(attr->buf); + bt_att_unref(attr->att); + g_io_channel_unref(io); + free(attr); + return NULL; } GAttrib *g_attrib_ref(GAttrib *attrib) { - int refs; - if (!attrib) return NULL; - refs = __sync_add_and_fetch(&attrib->refs, 1); + __sync_fetch_and_add(&attrib->ref_count, 1); - DBG("%p: ref=%d", attrib, refs); + DBG("%p: g_attrib_ref=%d ", attrib, attrib->ref_count); return attrib; } -static void command_destroy(struct command *cmd) +static void attrib_callbacks_destroy(void *user_data) { - if (cmd->notify) - cmd->notify(cmd->user_data); + struct attrib_callbacks *cb; - g_free(cmd->pdu); - g_free(cmd); -} + cb = (struct attrib_callbacks *)user_data; + if (!user_data || !g_queue_remove(cb->parent->callbacks, user_data)) + return; -static void event_destroy(struct event *evt) -{ - if (evt->notify) - evt->notify(evt->user_data); + if (cb->destroy_func) + cb->destroy_func(cb->user_data); - g_free(evt); + free(user_data); } -static void attrib_destroy(GAttrib *attrib) +void g_attrib_unref(GAttrib *attrib) { - GSList *l; - struct command *c; - - while ((c = g_queue_pop_head(attrib->requests))) - command_destroy(c); - - while ((c = g_queue_pop_head(attrib->responses))) - command_destroy(c); - - g_queue_free(attrib->requests); - attrib->requests = NULL; - - g_queue_free(attrib->responses); - attrib->responses = NULL; + struct attrib_callbacks *cb; - for (l = attrib->events; l; l = l->next) - event_destroy(l->data); - - g_slist_free(attrib->events); - attrib->events = NULL; - - if (attrib->timeout_watch > 0) - g_source_remove(attrib->timeout_watch); - - if (attrib->write_watch > 0) - g_source_remove(attrib->write_watch); - - if (attrib->read_watch > 0) - g_source_remove(attrib->read_watch); + if (!attrib) + return; - if (attrib->io) - g_io_channel_unref(attrib->io); + DBG("%p: g_attrib_unref=%d ", attrib, attrib->ref_count-1); - g_free(attrib->buf); + if (__sync_sub_and_fetch(&attrib->ref_count, 1)) + return; if (attrib->destroy) attrib->destroy(attrib->destroy_user_data); - g_free(attrib); -} + while ((cb = g_queue_peek_head(attrib->callbacks))) + attrib_callbacks_destroy(cb); -void g_attrib_unref(GAttrib *attrib) -{ - int refs; + g_queue_free(attrib->callbacks); - if (!attrib) - return; - - refs = __sync_sub_and_fetch(&attrib->refs, 1); + g_free(attrib->buf); - DBG("%p: ref=%d", attrib, refs); + bt_att_unref(attrib->att); - if (refs > 0) - return; + g_io_channel_unref(attrib->io); - attrib_destroy(attrib); + g_free(attrib); } GIOChannel *g_attrib_get_channel(GAttrib *attrib) @@ -270,7 +166,7 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib) gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy, gpointer user_data) { - if (attrib == NULL) + if (!attrib) return FALSE; attrib->destroy = destroy; @@ -279,380 +175,116 @@ gboolean g_attrib_set_destroy_function(GAttrib *attrib, return TRUE; } -static gboolean disconnect_timeout(gpointer data) -{ - struct _GAttrib *attrib = data; - struct command *c; - - g_attrib_ref(attrib); - - c = g_queue_pop_head(attrib->requests); - if (c == NULL) - goto done; - - if (c->func) - c->func(ATT_ECODE_TIMEOUT, NULL, 0, c->user_data); - - command_destroy(c); - - while ((c = g_queue_pop_head(attrib->requests))) { - if (c->func) - c->func(ATT_ECODE_ABORTED, NULL, 0, c->user_data); - command_destroy(c); - } - -done: - attrib->stale = true; - - g_attrib_unref(attrib); - - return FALSE; -} - -static gboolean can_write_data(GIOChannel *io, GIOCondition cond, - gpointer data) -{ - struct _GAttrib *attrib = data; - struct command *cmd; - GError *gerr = NULL; - gsize len; - GIOStatus iostat; - GQueue *queue; - - if (attrib->stale) - return FALSE; - - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) - return FALSE; - - queue = attrib->responses; - cmd = g_queue_peek_head(queue); - if (cmd == NULL) { - queue = attrib->requests; - cmd = g_queue_peek_head(queue); - } - if (cmd == NULL) - return FALSE; - - /* - * Verify that we didn't already send this command. This can only - * happen with elementes from attrib->requests. - */ - if (cmd->sent) - return FALSE; - - iostat = g_io_channel_write_chars(io, (char *) cmd->pdu, cmd->len, - &len, &gerr); - if (iostat != G_IO_STATUS_NORMAL) { - if (gerr) { - error("%s", gerr->message); - g_error_free(gerr); - } - - return FALSE; - } - - if (cmd->expected == 0) { - g_queue_pop_head(queue); - command_destroy(cmd); - return TRUE; - } - - cmd->sent = true; - - if (attrib->timeout_watch == 0) - attrib->timeout_watch = g_timeout_add_seconds(GATT_TIMEOUT, - disconnect_timeout, attrib); - - return FALSE; -} - -static void destroy_sender(gpointer data) +static void attrib_callback_result(uint8_t opcode, const void *pdu, + uint16_t length, void *user_data) { - struct _GAttrib *attrib = data; - - attrib->write_watch = 0; - g_attrib_unref(attrib); -} + uint8_t *buf; + struct attrib_callbacks *cb = user_data; -static void wake_up_sender(struct _GAttrib *attrib) -{ - if (attrib->write_watch > 0) + if (!cb) return; - attrib = g_attrib_ref(attrib); - attrib->write_watch = g_io_add_watch_full(attrib->io, - G_PRIORITY_DEFAULT, G_IO_OUT, - can_write_data, attrib, destroy_sender); -} - -static bool match_event(struct event *evt, const uint8_t *pdu, gsize len) -{ - guint16 handle; - - if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS) - return true; - - if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES) - return true; - - if (len < 3) - return false; + buf = g_malloc0(length+1); + if (!buf) + return; - handle = get_le16(&pdu[1]); + buf[0] = opcode; + memcpy(buf+1, pdu, length); - if (evt->expected == pdu[0] && evt->handle == handle) - return true; + if (cb->result_func) + cb->result_func(0, buf, length+1, cb->user_data); - return false; + g_free(buf); } -static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data) -{ - struct _GAttrib *attrib = data; - struct command *cmd = NULL; - GSList *l; - uint8_t buf[512], status; - gsize len; - GIOStatus iostat; - - if (attrib->stale) - return FALSE; - - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) { - struct command *c; - - while ((c = g_queue_pop_head(attrib->requests))) { - if (c->func) - c->func(ATT_ECODE_IO, NULL, 0, c->user_data); - command_destroy(c); - } - - attrib->read_watch = 0; - - return FALSE; - } - - memset(buf, 0, sizeof(buf)); - iostat = g_io_channel_read_chars(io, (char *) buf, sizeof(buf), - &len, NULL); - if (iostat != G_IO_STATUS_NORMAL) { - status = ATT_ECODE_IO; - goto done; - } - - for (l = attrib->events; l; l = l->next) { - struct event *evt = l->data; - - if (match_event(evt, buf, len)) - evt->func(buf, len, evt->user_data); - } - - if (!is_response(buf[0])) - return TRUE; - - if (attrib->timeout_watch > 0) { - g_source_remove(attrib->timeout_watch); - attrib->timeout_watch = 0; - } - - cmd = g_queue_pop_head(attrib->requests); - if (cmd == NULL) { - /* Keep the watch if we have events to report */ - return attrib->events != NULL; - } - - if (buf[0] == ATT_OP_ERROR) { - status = buf[4]; - goto done; - } - - if (cmd->expected != buf[0]) { - status = ATT_ECODE_IO; - goto done; - } - - status = 0; - -done: - if (!g_queue_is_empty(attrib->requests) || - !g_queue_is_empty(attrib->responses)) - wake_up_sender(attrib); - - if (cmd) { - if (cmd->func) - cmd->func(status, buf, len, cmd->user_data); - - command_destroy(cmd); - } +static void attrib_callback_notify(uint8_t opcode, const void *pdu, + uint16_t length, void *user_data) +{ + uint8_t *buf; + struct attrib_callbacks *cb = user_data; - return TRUE; -} + if (!cb) + return; -GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) -{ - struct _GAttrib *attrib; + if (cb->notify_func == NULL) + return; - g_io_channel_set_encoding(io, NULL, NULL); - g_io_channel_set_buffered(io, FALSE); + if (cb->notify_handle != GATTRIB_ALL_HANDLES && length < 2) + return; - attrib = g_try_new0(struct _GAttrib, 1); - if (attrib == NULL) - return NULL; + if (cb->notify_handle != GATTRIB_ALL_HANDLES && + cb->notify_handle != get_le16(pdu)) + return; - attrib->buf = g_malloc0(mtu); - attrib->buflen = mtu; + buf = g_malloc0(length+1); + if (!buf) + return; - attrib->io = g_io_channel_ref(io); - attrib->requests = g_queue_new(); - attrib->responses = g_queue_new(); + buf[0] = opcode; + memcpy(buf+1, pdu, length); - attrib->read_watch = g_io_add_watch(attrib->io, - G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL, - received_data, attrib); + cb->notify_func(buf, length+1, cb->user_data); - return g_attrib_ref(attrib); + g_free(buf); } guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len, GAttribResultFunc func, gpointer user_data, GDestroyNotify notify) { - struct command *c; - GQueue *queue; - uint8_t opcode; - - if (attrib->stale) - return 0; - - c = g_try_new0(struct command, 1); - if (c == NULL) - return 0; - - opcode = pdu[0]; - - c->opcode = opcode; - c->expected = opcode2expected(opcode); - c->pdu = g_malloc(len); - memcpy(c->pdu, pdu, len); - c->len = len; - c->func = func; - c->user_data = user_data; - c->notify = notify; - - if (is_response(opcode)) - queue = attrib->responses; - else - queue = attrib->requests; - - if (id) { - c->id = id; - if (!is_response(opcode)) - g_queue_push_head(queue, c); - else - /* Don't re-order responses even if an ID is given */ - g_queue_push_tail(queue, c); - } else { - c->id = ++attrib->next_cmd_id; - g_queue_push_tail(queue, c); + struct attrib_callbacks *cb = NULL; + bt_att_response_func_t response_cb = NULL; + bt_att_destroy_func_t destroy_cb = NULL; + + if (func || notify) { + cb = new0(struct attrib_callbacks, 1); + if (cb == 0) + return 0; + cb->result_func = func; + cb->user_data = user_data; + cb->destroy_func = notify; + cb->parent = attrib; + g_queue_push_head(attrib->callbacks, cb); + response_cb = attrib_callback_result; + destroy_cb = attrib_callbacks_destroy; } - /* - * If a command was added to the queue and it was empty before, wake up - * the sender. If the sender was already woken up by the second queue, - * wake_up_sender will just return. - */ - if (g_queue_get_length(queue) == 1) - wake_up_sender(attrib); - - return c->id; -} - -static int command_cmp_by_id(gconstpointer a, gconstpointer b) -{ - const struct command *cmd = a; - guint id = GPOINTER_TO_UINT(b); - - return cmd->id - id; + return bt_att_send(attrib->att, pdu[0], (void *)pdu+1, len-1, + response_cb, cb, destroy_cb); } gboolean g_attrib_cancel(GAttrib *attrib, guint id) { - GList *l = NULL; - struct command *cmd; - GQueue *queue; - - if (attrib == NULL) - return FALSE; - - queue = attrib->requests; - if (queue) - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), - command_cmp_by_id); - if (l == NULL) { - queue = attrib->responses; - if (!queue) - return FALSE; - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), - command_cmp_by_id); - } - - if (l == NULL) - return FALSE; - - cmd = l->data; - - if (cmd == g_queue_peek_head(queue) && cmd->sent) - cmd->func = NULL; - else { - g_queue_remove(queue, cmd); - command_destroy(cmd); - } - - return TRUE; + return bt_att_cancel(attrib->att, id); } -static gboolean cancel_all_per_queue(GQueue *queue) +gboolean g_attrib_cancel_all(GAttrib *attrib) { - struct command *c, *head = NULL; - gboolean first = TRUE; - - if (queue == NULL) - return FALSE; - - while ((c = g_queue_pop_head(queue))) { - if (first && c->sent) { - /* If the command was sent ignore its callback ... */ - c->func = NULL; - head = c; - continue; - } - - first = FALSE; - command_destroy(c); - } - - if (head) { - /* ... and put it back in the queue */ - g_queue_push_head(queue, head); - } - - return TRUE; + return bt_att_cancel_all(attrib->att); } -gboolean g_attrib_cancel_all(GAttrib *attrib) +guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, + GAttribNotifyFunc func, gpointer user_data, + GDestroyNotify notify) { - gboolean ret; - - if (attrib == NULL) - return FALSE; - - ret = cancel_all_per_queue(attrib->requests); - ret = cancel_all_per_queue(attrib->responses) && ret; + struct attrib_callbacks *cb = NULL; + + if (func || notify) { + cb = new0(struct attrib_callbacks, 1); + if (cb == 0) + return 0; + cb->notify_func = func; + cb->notify_handle = handle; + cb->user_data = user_data; + cb->destroy_func = notify; + cb->parent = attrib; + g_queue_push_head(attrib->callbacks, cb); + } - return ret; + return bt_att_register(attrib->att, opcode, attrib_callback_notify, + cb, attrib_callbacks_destroy); } uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) @@ -661,98 +293,26 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) return NULL; *len = attrib->buflen; - return attrib->buf; } gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu) { - if (mtu < ATT_DEFAULT_LE_MTU) - return FALSE; - - attrib->buf = g_realloc(attrib->buf, mtu); - - attrib->buflen = mtu; - - return TRUE; -} - -guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, - GAttribNotifyFunc func, gpointer user_data, - GDestroyNotify notify) -{ - static guint next_evt_id = 0; - struct event *event; - - event = g_try_new0(struct event, 1); - if (event == NULL) - return 0; - - event->expected = opcode; - event->handle = handle; - event->func = func; - event->user_data = user_data; - event->notify = notify; - event->id = ++next_evt_id; - - attrib->events = g_slist_append(attrib->events, event); - - return event->id; -} - -static int event_cmp_by_id(gconstpointer a, gconstpointer b) -{ - const struct event *evt = a; - guint id = GPOINTER_TO_UINT(b); + /* Clients of this expect a buffer to use. */ + if (mtu > attrib->buflen) { + attrib->buf = g_realloc(attrib->buf, mtu); + attrib->buflen = mtu; + } - return evt->id - id; + return bt_att_set_mtu(attrib->att, mtu); } gboolean g_attrib_unregister(GAttrib *attrib, guint id) { - struct event *evt; - GSList *l; - - if (id == 0) { - warn("%s: invalid id", __func__); - return FALSE; - } - - l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id), - event_cmp_by_id); - if (l == NULL) - return FALSE; - - evt = l->data; - - attrib->events = g_slist_remove(attrib->events, evt); - - if (evt->notify) - evt->notify(evt->user_data); - - g_free(evt); - - return TRUE; + return bt_att_unregister(attrib->att, id); } gboolean g_attrib_unregister_all(GAttrib *attrib) { - GSList *l; - - if (attrib->events == NULL) - return FALSE; - - for (l = attrib->events; l; l = l->next) { - struct event *evt = l->data; - - if (evt->notify) - evt->notify(evt->user_data); - - g_free(evt); - } - - g_slist_free(attrib->events); - attrib->events = NULL; - - return TRUE; + return bt_att_unregister_all(attrib->att); } -- 2.1.0.rc2.206.gedb03e5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-04 21:19 ` [PATCH BlueZ 4/4] GATT shim to src/shared bt_att Michael Janssen @ 2014-11-05 14:49 ` Luiz Augusto von Dentz 2014-11-05 15:17 ` Michael Janssen 0 siblings, 1 reply; 16+ messages in thread From: Luiz Augusto von Dentz @ 2014-11-05 14:49 UTC (permalink / raw) To: Michael Janssen; +Cc: linux-bluetooth@vger.kernel.org Hi Michael, On Tue, Nov 4, 2014 at 11:19 PM, Michael Janssen <jamuraa@chromium.org> wrote: > This patch implements a version of GAttrib which is backed by > bt_att, which enables the simultaneous use of GAttrib and bt_att. > > This should enable smooth transition of profiles from the GAttrib > API to the src/shared bt_att API. > --- > attrib/gattrib.c | 734 +++++++++++-------------------------------------------- > 1 file changed, 147 insertions(+), 587 deletions(-) > > diff --git a/attrib/gattrib.c b/attrib/gattrib.c > index fa51b6d..63f219d 100644 > --- a/attrib/gattrib.c > +++ b/attrib/gattrib.c > @@ -2,8 +2,7 @@ > * > * BlueZ - Bluetooth protocol stack for Linux > * > - * Copyright (C) 2010 Nokia Corporation > - * Copyright (C) 2010 Marcel Holtmann <marcel@holtmann.org> > + * Copyright (C) 2014 Google, Inc. You got to be super careful with copyright changes, because of that we usually request this to be in a separate patch, except for newly created files of course. To speed up this I would suggest you leave this for later. > * > * > * This program is free software; you can redistribute it and/or modify > @@ -36,227 +35,124 @@ > #include <bluetooth/bluetooth.h> > > #include "btio/btio.h" > -#include "lib/uuid.h" > -#include "src/shared/util.h" > #include "src/log.h" > -#include "attrib/att.h" > +#include "src/shared/util.h" > +#include "src/shared/att.h" > #include "attrib/gattrib.h" > > -#define GATT_TIMEOUT 30 > - > struct _GAttrib { > + int ref_count; > + struct bt_att *att; > GIOChannel *io; > - int refs; > - uint8_t *buf; > - size_t buflen; > - guint read_watch; > - guint write_watch; > - guint timeout_watch; > - GQueue *requests; > - GQueue *responses; > - GSList *events; > - guint next_cmd_id; > GDestroyNotify destroy; > gpointer destroy_user_data; > - bool stale; > + GQueue *callbacks; > + uint8_t *buf; > + int buflen; > }; > > -struct command { > - guint id; > - guint8 opcode; > - guint8 *pdu; > - guint16 len; > - guint8 expected; > - bool sent; > - GAttribResultFunc func; > - gpointer user_data; > - GDestroyNotify notify; > -}; > > -struct event { > - guint id; > - guint8 expected; > - guint16 handle; > - GAttribNotifyFunc func; > +struct attrib_callbacks { > + GAttribResultFunc result_func; > + GAttribNotifyFunc notify_func; > + GDestroyNotify destroy_func; > gpointer user_data; > - GDestroyNotify notify; > + GAttrib *parent; > + uint16_t notify_handle; > }; > > -static guint8 opcode2expected(guint8 opcode) > +GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) > { > - switch (opcode) { > - case ATT_OP_MTU_REQ: > - return ATT_OP_MTU_RESP; > - > - case ATT_OP_FIND_INFO_REQ: > - return ATT_OP_FIND_INFO_RESP; > - > - case ATT_OP_FIND_BY_TYPE_REQ: > - return ATT_OP_FIND_BY_TYPE_RESP; > - > - case ATT_OP_READ_BY_TYPE_REQ: > - return ATT_OP_READ_BY_TYPE_RESP; > - > - case ATT_OP_READ_REQ: > - return ATT_OP_READ_RESP; > - > - case ATT_OP_READ_BLOB_REQ: > - return ATT_OP_READ_BLOB_RESP; > - > - case ATT_OP_READ_MULTI_REQ: > - return ATT_OP_READ_MULTI_RESP; > + gint fd; > + GAttrib *attr; > > - case ATT_OP_READ_BY_GROUP_REQ: > - return ATT_OP_READ_BY_GROUP_RESP; > + if (!io) > + return NULL; > > - case ATT_OP_WRITE_REQ: > - return ATT_OP_WRITE_RESP; > + fd = g_io_channel_unix_get_fd(io); > + attr = new0(GAttrib, 1); > + if (!attr) > + return NULL; > > - case ATT_OP_PREP_WRITE_REQ: > - return ATT_OP_PREP_WRITE_RESP; > + g_io_channel_ref(io); > + attr->io = io; > > - case ATT_OP_EXEC_WRITE_REQ: > - return ATT_OP_EXEC_WRITE_RESP; > + attr->att = bt_att_new(fd); > + if (!attr->att) > + goto fail; > > - case ATT_OP_HANDLE_IND: > - return ATT_OP_HANDLE_CNF; > - } > + attr->buf = g_malloc0(mtu); > + attr->buflen = mtu; > + if (!attr->buf) > + goto fail; > > - return 0; > -} > + attr->callbacks = g_queue_new(); > + if (!attr->callbacks) > + goto fail; > > -static bool is_response(guint8 opcode) > -{ > - switch (opcode) { > - case ATT_OP_ERROR: > - case ATT_OP_MTU_RESP: > - case ATT_OP_FIND_INFO_RESP: > - case ATT_OP_FIND_BY_TYPE_RESP: > - case ATT_OP_READ_BY_TYPE_RESP: > - case ATT_OP_READ_RESP: > - case ATT_OP_READ_BLOB_RESP: > - case ATT_OP_READ_MULTI_RESP: > - case ATT_OP_READ_BY_GROUP_RESP: > - case ATT_OP_WRITE_RESP: > - case ATT_OP_PREP_WRITE_RESP: > - case ATT_OP_EXEC_WRITE_RESP: > - case ATT_OP_HANDLE_CNF: > - return true; > - } > + return g_attrib_ref(attr); > > - return false; > -} > - > -static bool is_request(guint8 opcode) > -{ > - switch (opcode) { > - case ATT_OP_MTU_REQ: > - case ATT_OP_FIND_INFO_REQ: > - case ATT_OP_FIND_BY_TYPE_REQ: > - case ATT_OP_READ_BY_TYPE_REQ: > - case ATT_OP_READ_REQ: > - case ATT_OP_READ_BLOB_REQ: > - case ATT_OP_READ_MULTI_REQ: > - case ATT_OP_READ_BY_GROUP_REQ: > - case ATT_OP_WRITE_REQ: > - case ATT_OP_WRITE_CMD: > - case ATT_OP_PREP_WRITE_REQ: > - case ATT_OP_EXEC_WRITE_REQ: > - return true; > - } > - > - return false; > +fail: > + free(attr->buf); > + bt_att_unref(attr->att); > + g_io_channel_unref(io); > + free(attr); > + return NULL; > } > > GAttrib *g_attrib_ref(GAttrib *attrib) > { > - int refs; > - > if (!attrib) > return NULL; > > - refs = __sync_add_and_fetch(&attrib->refs, 1); > + __sync_fetch_and_add(&attrib->ref_count, 1); > > - DBG("%p: ref=%d", attrib, refs); > + DBG("%p: g_attrib_ref=%d ", attrib, attrib->ref_count); > > return attrib; > } > > -static void command_destroy(struct command *cmd) > +static void attrib_callbacks_destroy(void *user_data) > { > - if (cmd->notify) > - cmd->notify(cmd->user_data); > + struct attrib_callbacks *cb; > > - g_free(cmd->pdu); > - g_free(cmd); > -} > + cb = (struct attrib_callbacks *)user_data; > + if (!user_data || !g_queue_remove(cb->parent->callbacks, user_data)) > + return; > > -static void event_destroy(struct event *evt) > -{ > - if (evt->notify) > - evt->notify(evt->user_data); > + if (cb->destroy_func) > + cb->destroy_func(cb->user_data); > > - g_free(evt); > + free(user_data); > } > > -static void attrib_destroy(GAttrib *attrib) > +void g_attrib_unref(GAttrib *attrib) > { > - GSList *l; > - struct command *c; > - > - while ((c = g_queue_pop_head(attrib->requests))) > - command_destroy(c); > - > - while ((c = g_queue_pop_head(attrib->responses))) > - command_destroy(c); > - > - g_queue_free(attrib->requests); > - attrib->requests = NULL; > - > - g_queue_free(attrib->responses); > - attrib->responses = NULL; > + struct attrib_callbacks *cb; > > - for (l = attrib->events; l; l = l->next) > - event_destroy(l->data); > - > - g_slist_free(attrib->events); > - attrib->events = NULL; > - > - if (attrib->timeout_watch > 0) > - g_source_remove(attrib->timeout_watch); > - > - if (attrib->write_watch > 0) > - g_source_remove(attrib->write_watch); > - > - if (attrib->read_watch > 0) > - g_source_remove(attrib->read_watch); > + if (!attrib) > + return; > > - if (attrib->io) > - g_io_channel_unref(attrib->io); > + DBG("%p: g_attrib_unref=%d ", attrib, attrib->ref_count-1); > > - g_free(attrib->buf); > + if (__sync_sub_and_fetch(&attrib->ref_count, 1)) > + return; > > if (attrib->destroy) > attrib->destroy(attrib->destroy_user_data); > > - g_free(attrib); > -} > + while ((cb = g_queue_peek_head(attrib->callbacks))) > + attrib_callbacks_destroy(cb); > > -void g_attrib_unref(GAttrib *attrib) > -{ > - int refs; > + g_queue_free(attrib->callbacks); > > - if (!attrib) > - return; > - > - refs = __sync_sub_and_fetch(&attrib->refs, 1); > + g_free(attrib->buf); > > - DBG("%p: ref=%d", attrib, refs); > + bt_att_unref(attrib->att); > > - if (refs > 0) > - return; > + g_io_channel_unref(attrib->io); > > - attrib_destroy(attrib); > + g_free(attrib); > } > > GIOChannel *g_attrib_get_channel(GAttrib *attrib) > @@ -270,7 +166,7 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib) > gboolean g_attrib_set_destroy_function(GAttrib *attrib, > GDestroyNotify destroy, gpointer user_data) > { > - if (attrib == NULL) > + if (!attrib) > return FALSE; > > attrib->destroy = destroy; > @@ -279,380 +175,116 @@ gboolean g_attrib_set_destroy_function(GAttrib *attrib, > return TRUE; > } > > -static gboolean disconnect_timeout(gpointer data) > -{ > - struct _GAttrib *attrib = data; > - struct command *c; > - > - g_attrib_ref(attrib); > - > - c = g_queue_pop_head(attrib->requests); > - if (c == NULL) > - goto done; > - > - if (c->func) > - c->func(ATT_ECODE_TIMEOUT, NULL, 0, c->user_data); > - > - command_destroy(c); > - > - while ((c = g_queue_pop_head(attrib->requests))) { > - if (c->func) > - c->func(ATT_ECODE_ABORTED, NULL, 0, c->user_data); > - command_destroy(c); > - } > - > -done: > - attrib->stale = true; > - > - g_attrib_unref(attrib); > - > - return FALSE; > -} > - > -static gboolean can_write_data(GIOChannel *io, GIOCondition cond, > - gpointer data) > -{ > - struct _GAttrib *attrib = data; > - struct command *cmd; > - GError *gerr = NULL; > - gsize len; > - GIOStatus iostat; > - GQueue *queue; > - > - if (attrib->stale) > - return FALSE; > - > - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) > - return FALSE; > - > - queue = attrib->responses; > - cmd = g_queue_peek_head(queue); > - if (cmd == NULL) { > - queue = attrib->requests; > - cmd = g_queue_peek_head(queue); > - } > - if (cmd == NULL) > - return FALSE; > - > - /* > - * Verify that we didn't already send this command. This can only > - * happen with elementes from attrib->requests. > - */ > - if (cmd->sent) > - return FALSE; > - > - iostat = g_io_channel_write_chars(io, (char *) cmd->pdu, cmd->len, > - &len, &gerr); > - if (iostat != G_IO_STATUS_NORMAL) { > - if (gerr) { > - error("%s", gerr->message); > - g_error_free(gerr); > - } > - > - return FALSE; > - } > - > - if (cmd->expected == 0) { > - g_queue_pop_head(queue); > - command_destroy(cmd); > > - return TRUE; > - } > - > - cmd->sent = true; > - > - if (attrib->timeout_watch == 0) > - attrib->timeout_watch = g_timeout_add_seconds(GATT_TIMEOUT, > - disconnect_timeout, attrib); > - > - return FALSE; > -} > - > -static void destroy_sender(gpointer data) > +static void attrib_callback_result(uint8_t opcode, const void *pdu, > + uint16_t length, void *user_data) > { > - struct _GAttrib *attrib = data; > - > - attrib->write_watch = 0; > - g_attrib_unref(attrib); > -} > + uint8_t *buf; > + struct attrib_callbacks *cb = user_data; > > -static void wake_up_sender(struct _GAttrib *attrib) > -{ > - if (attrib->write_watch > 0) > + if (!cb) > return; > > - attrib = g_attrib_ref(attrib); > - attrib->write_watch = g_io_add_watch_full(attrib->io, > - G_PRIORITY_DEFAULT, G_IO_OUT, > - can_write_data, attrib, destroy_sender); > -} > - > -static bool match_event(struct event *evt, const uint8_t *pdu, gsize len) > -{ > - guint16 handle; > - > - if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS) > - return true; > - > - if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES) > - return true; > - > - if (len < 3) > - return false; > + buf = g_malloc0(length+1); > + if (!buf) > + return; > > - handle = get_le16(&pdu[1]); > + buf[0] = opcode; > + memcpy(buf+1, pdu, length); > > - if (evt->expected == pdu[0] && evt->handle == handle) > - return true; > + if (cb->result_func) > + cb->result_func(0, buf, length+1, cb->user_data); > > - return false; > + g_free(buf); > } > > -static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data) > -{ > - struct _GAttrib *attrib = data; > - struct command *cmd = NULL; > - GSList *l; > - uint8_t buf[512], status; > - gsize len; > - GIOStatus iostat; > - > - if (attrib->stale) > - return FALSE; > - > - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) { > - struct command *c; > - > - while ((c = g_queue_pop_head(attrib->requests))) { > - if (c->func) > - c->func(ATT_ECODE_IO, NULL, 0, c->user_data); > - command_destroy(c); > - } > - > - attrib->read_watch = 0; > - > - return FALSE; > - } > - > - memset(buf, 0, sizeof(buf)); > > - iostat = g_io_channel_read_chars(io, (char *) buf, sizeof(buf), > - &len, NULL); > - if (iostat != G_IO_STATUS_NORMAL) { > - status = ATT_ECODE_IO; > - goto done; > - } > - > - for (l = attrib->events; l; l = l->next) { > - struct event *evt = l->data; > - > - if (match_event(evt, buf, len)) > - evt->func(buf, len, evt->user_data); > - } > - > - if (!is_response(buf[0])) > - return TRUE; > - > - if (attrib->timeout_watch > 0) { > - g_source_remove(attrib->timeout_watch); > - attrib->timeout_watch = 0; > - } > - > - cmd = g_queue_pop_head(attrib->requests); > - if (cmd == NULL) { > - /* Keep the watch if we have events to report */ > - return attrib->events != NULL; > - } > - > - if (buf[0] == ATT_OP_ERROR) { > - status = buf[4]; > - goto done; > - } > - > - if (cmd->expected != buf[0]) { > - status = ATT_ECODE_IO; > - goto done; > - } > - > - status = 0; > - > -done: > - if (!g_queue_is_empty(attrib->requests) || > - !g_queue_is_empty(attrib->responses)) > - wake_up_sender(attrib); > - > - if (cmd) { > - if (cmd->func) > - cmd->func(status, buf, len, cmd->user_data); > - > - command_destroy(cmd); > - } > +static void attrib_callback_notify(uint8_t opcode, const void *pdu, > + uint16_t length, void *user_data) > +{ > + uint8_t *buf; > + struct attrib_callbacks *cb = user_data; > > - return TRUE; > -} > + if (!cb) > + return; > > -GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) > -{ > - struct _GAttrib *attrib; > + if (cb->notify_func == NULL) > + return; > > - g_io_channel_set_encoding(io, NULL, NULL); > - g_io_channel_set_buffered(io, FALSE); > + if (cb->notify_handle != GATTRIB_ALL_HANDLES && length < 2) > + return; > > - attrib = g_try_new0(struct _GAttrib, 1); > - if (attrib == NULL) > - return NULL; > + if (cb->notify_handle != GATTRIB_ALL_HANDLES && > + cb->notify_handle != get_le16(pdu)) > + return; > > - attrib->buf = g_malloc0(mtu); > - attrib->buflen = mtu; > + buf = g_malloc0(length+1); > + if (!buf) > + return; > > - attrib->io = g_io_channel_ref(io); > - attrib->requests = g_queue_new(); > - attrib->responses = g_queue_new(); > + buf[0] = opcode; > + memcpy(buf+1, pdu, length); > > - attrib->read_watch = g_io_add_watch(attrib->io, > - G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL, > - received_data, attrib); > + cb->notify_func(buf, length+1, cb->user_data); > > - return g_attrib_ref(attrib); > + g_free(buf); > } > > guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len, > GAttribResultFunc func, gpointer user_data, > GDestroyNotify notify) > { > - struct command *c; > - GQueue *queue; > - uint8_t opcode; > - > - if (attrib->stale) > - return 0; > - > - c = g_try_new0(struct command, 1); > - if (c == NULL) > - return 0; > - > - opcode = pdu[0]; > - > - c->opcode = opcode; > - c->expected = opcode2expected(opcode); > - c->pdu = g_malloc(len); > - memcpy(c->pdu, pdu, len); > - c->len = len; > - c->func = func; > - c->user_data = user_data; > - c->notify = notify; > - > - if (is_response(opcode)) > - queue = attrib->responses; > - else > - queue = attrib->requests; > - > - if (id) { > - c->id = id; > - if (!is_response(opcode)) > - g_queue_push_head(queue, c); > - else > - /* Don't re-order responses even if an ID is given */ > - g_queue_push_tail(queue, c); > - } else { > - c->id = ++attrib->next_cmd_id; > - g_queue_push_tail(queue, c); > + struct attrib_callbacks *cb = NULL; > + bt_att_response_func_t response_cb = NULL; > + bt_att_destroy_func_t destroy_cb = NULL; > + > + if (func || notify) { > + cb = new0(struct attrib_callbacks, 1); > + if (cb == 0) > + return 0; > + cb->result_func = func; > + cb->user_data = user_data; > + cb->destroy_func = notify; > + cb->parent = attrib; > + g_queue_push_head(attrib->callbacks, cb); > + response_cb = attrib_callback_result; > + destroy_cb = attrib_callbacks_destroy; > } > > - /* > - * If a command was added to the queue and it was empty before, wake up > - * the sender. If the sender was already woken up by the second queue, > - * wake_up_sender will just return. > - */ > - if (g_queue_get_length(queue) == 1) > - wake_up_sender(attrib); > - > - return c->id; > -} > - > -static int command_cmp_by_id(gconstpointer a, gconstpointer b) > -{ > - const struct command *cmd = a; > - guint id = GPOINTER_TO_UINT(b); > - > - return cmd->id - id; > + return bt_att_send(attrib->att, pdu[0], (void *)pdu+1, len-1, > + response_cb, cb, destroy_cb); > } > > gboolean g_attrib_cancel(GAttrib *attrib, guint id) > { > - GList *l = NULL; > - struct command *cmd; > - GQueue *queue; > - > - if (attrib == NULL) > - return FALSE; > - > - queue = attrib->requests; > - if (queue) > - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), > - command_cmp_by_id); > - if (l == NULL) { > - queue = attrib->responses; > - if (!queue) > - return FALSE; > - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), > - command_cmp_by_id); > - } > - > - if (l == NULL) > - return FALSE; > - > - cmd = l->data; > - > - if (cmd == g_queue_peek_head(queue) && cmd->sent) > - cmd->func = NULL; > - else { > - g_queue_remove(queue, cmd); > - command_destroy(cmd); > - } > - > - return TRUE; > + return bt_att_cancel(attrib->att, id); > } > > -static gboolean cancel_all_per_queue(GQueue *queue) > +gboolean g_attrib_cancel_all(GAttrib *attrib) > { > - struct command *c, *head = NULL; > - gboolean first = TRUE; > - > - if (queue == NULL) > - return FALSE; > - > - while ((c = g_queue_pop_head(queue))) { > - if (first && c->sent) { > - /* If the command was sent ignore its callback ... */ > - c->func = NULL; > - head = c; > - continue; > - } > - > - first = FALSE; > - command_destroy(c); > - } > - > - if (head) { > - /* ... and put it back in the queue */ > - g_queue_push_head(queue, head); > - } > - > - return TRUE; > + return bt_att_cancel_all(attrib->att); > } > > -gboolean g_attrib_cancel_all(GAttrib *attrib) > +guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, > + GAttribNotifyFunc func, gpointer user_data, > + GDestroyNotify notify) > { > - gboolean ret; > - > - if (attrib == NULL) > - return FALSE; > - > - ret = cancel_all_per_queue(attrib->requests); > - ret = cancel_all_per_queue(attrib->responses) && ret; > + struct attrib_callbacks *cb = NULL; > + > + if (func || notify) { > + cb = new0(struct attrib_callbacks, 1); > + if (cb == 0) > + return 0; > + cb->notify_func = func; > + cb->notify_handle = handle; > + cb->user_data = user_data; > + cb->destroy_func = notify; > + cb->parent = attrib; > + g_queue_push_head(attrib->callbacks, cb); > + } > > - return ret; > + return bt_att_register(attrib->att, opcode, attrib_callback_notify, > + cb, attrib_callbacks_destroy); > } > > uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) > @@ -661,98 +293,26 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) > return NULL; > > *len = attrib->buflen; > - > return attrib->buf; > } > > gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu) > { > - if (mtu < ATT_DEFAULT_LE_MTU) > - return FALSE; > - > - attrib->buf = g_realloc(attrib->buf, mtu); > - > - attrib->buflen = mtu; > - > - return TRUE; > -} > - > -guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, > - GAttribNotifyFunc func, gpointer user_data, > - GDestroyNotify notify) > -{ > - static guint next_evt_id = 0; > - struct event *event; > - > - event = g_try_new0(struct event, 1); > - if (event == NULL) > - return 0; > - > - event->expected = opcode; > - event->handle = handle; > - event->func = func; > - event->user_data = user_data; > - event->notify = notify; > - event->id = ++next_evt_id; > - > - attrib->events = g_slist_append(attrib->events, event); > - > - return event->id; > -} > - > -static int event_cmp_by_id(gconstpointer a, gconstpointer b) > -{ > - const struct event *evt = a; > - guint id = GPOINTER_TO_UINT(b); > + /* Clients of this expect a buffer to use. */ > + if (mtu > attrib->buflen) { > + attrib->buf = g_realloc(attrib->buf, mtu); > + attrib->buflen = mtu; > + } > > - return evt->id - id; > + return bt_att_set_mtu(attrib->att, mtu); > } > > gboolean g_attrib_unregister(GAttrib *attrib, guint id) > { > - struct event *evt; > - GSList *l; > - > - if (id == 0) { > - warn("%s: invalid id", __func__); > - return FALSE; > - } > - > - l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id), > - event_cmp_by_id); > - if (l == NULL) > - return FALSE; > - > - evt = l->data; > - > - attrib->events = g_slist_remove(attrib->events, evt); > - > - if (evt->notify) > - evt->notify(evt->user_data); > - > - g_free(evt); > - > - return TRUE; > + return bt_att_unregister(attrib->att, id); > } > > gboolean g_attrib_unregister_all(GAttrib *attrib) > { > - GSList *l; > - > - if (attrib->events == NULL) > - return FALSE; > - > - for (l = attrib->events; l; l = l->next) { > - struct event *evt = l->data; > - > - if (evt->notify) > - evt->notify(evt->user_data); > - > - g_free(evt); > - } > - > - g_slist_free(attrib->events); > - attrib->events = NULL; > - > - return TRUE; > + return bt_att_unregister_all(attrib->att); > } > -- > 2.1.0.rc2.206.gedb03e5 > > -- > 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 -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-05 14:49 ` Luiz Augusto von Dentz @ 2014-11-05 15:17 ` Michael Janssen 2014-11-05 20:30 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 16+ messages in thread From: Michael Janssen @ 2014-11-05 15:17 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org Hi Luiz, On Wed, Nov 5, 2014 at 6:49 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > Hi Michael, > > On Tue, Nov 4, 2014 at 11:19 PM, Michael Janssen <jamuraa@chromium.org> wrote: >> This patch implements a version of GAttrib which is backed by >> bt_att, which enables the simultaneous use of GAttrib and bt_att. >> >> This should enable smooth transition of profiles from the GAttrib >> API to the src/shared bt_att API. >> --- >> attrib/gattrib.c | 734 +++++++++++-------------------------------------------- >> 1 file changed, 147 insertions(+), 587 deletions(-) >> >> diff --git a/attrib/gattrib.c b/attrib/gattrib.c >> index fa51b6d..63f219d 100644 >> --- a/attrib/gattrib.c >> +++ b/attrib/gattrib.c >> @@ -2,8 +2,7 @@ >> * >> * BlueZ - Bluetooth protocol stack for Linux >> * >> - * Copyright (C) 2010 Nokia Corporation >> - * Copyright (C) 2010 Marcel Holtmann <marcel@holtmann.org> >> + * Copyright (C) 2014 Google, Inc. > > You got to be super careful with copyright changes, because of that we > usually request this to be in a separate patch, except for newly > created files of course. To speed up this I would suggest you leave > this for later. To be clear: this looking like a change of a bunch of lines of a single file is an artifact of the patch process - I created this from an empty file by copying the headers from gattrib.h and implementing them. It was originally a new file called gattrib-shared.c. Normally I wouldn't change this copyright stuff and have no issue keeping the changes for v2. >> * >> * >> * This program is free software; you can redistribute it and/or modify >> @@ -36,227 +35,124 @@ >> #include <bluetooth/bluetooth.h> >> >> #include "btio/btio.h" >> -#include "lib/uuid.h" >> -#include "src/shared/util.h" >> #include "src/log.h" >> -#include "attrib/att.h" >> +#include "src/shared/util.h" >> +#include "src/shared/att.h" >> #include "attrib/gattrib.h" >> >> -#define GATT_TIMEOUT 30 >> - >> struct _GAttrib { >> + int ref_count; >> + struct bt_att *att; >> GIOChannel *io; >> - int refs; >> - uint8_t *buf; >> - size_t buflen; >> - guint read_watch; >> - guint write_watch; >> - guint timeout_watch; >> - GQueue *requests; >> - GQueue *responses; >> - GSList *events; >> - guint next_cmd_id; >> GDestroyNotify destroy; >> gpointer destroy_user_data; >> - bool stale; >> + GQueue *callbacks; >> + uint8_t *buf; >> + int buflen; >> }; >> >> -struct command { >> - guint id; >> - guint8 opcode; >> - guint8 *pdu; >> - guint16 len; >> - guint8 expected; >> - bool sent; >> - GAttribResultFunc func; >> - gpointer user_data; >> - GDestroyNotify notify; >> -}; >> >> -struct event { >> - guint id; >> - guint8 expected; >> - guint16 handle; >> - GAttribNotifyFunc func; >> +struct attrib_callbacks { >> + GAttribResultFunc result_func; >> + GAttribNotifyFunc notify_func; >> + GDestroyNotify destroy_func; >> gpointer user_data; >> - GDestroyNotify notify; >> + GAttrib *parent; >> + uint16_t notify_handle; >> }; >> >> -static guint8 opcode2expected(guint8 opcode) >> +GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) >> { >> - switch (opcode) { >> - case ATT_OP_MTU_REQ: >> - return ATT_OP_MTU_RESP; >> - >> - case ATT_OP_FIND_INFO_REQ: >> - return ATT_OP_FIND_INFO_RESP; >> - >> - case ATT_OP_FIND_BY_TYPE_REQ: >> - return ATT_OP_FIND_BY_TYPE_RESP; >> - >> - case ATT_OP_READ_BY_TYPE_REQ: >> - return ATT_OP_READ_BY_TYPE_RESP; >> - >> - case ATT_OP_READ_REQ: >> - return ATT_OP_READ_RESP; >> - >> - case ATT_OP_READ_BLOB_REQ: >> - return ATT_OP_READ_BLOB_RESP; >> - >> - case ATT_OP_READ_MULTI_REQ: >> - return ATT_OP_READ_MULTI_RESP; >> + gint fd; >> + GAttrib *attr; >> >> - case ATT_OP_READ_BY_GROUP_REQ: >> - return ATT_OP_READ_BY_GROUP_RESP; >> + if (!io) >> + return NULL; >> >> - case ATT_OP_WRITE_REQ: >> - return ATT_OP_WRITE_RESP; >> + fd = g_io_channel_unix_get_fd(io); >> + attr = new0(GAttrib, 1); >> + if (!attr) >> + return NULL; >> >> - case ATT_OP_PREP_WRITE_REQ: >> - return ATT_OP_PREP_WRITE_RESP; >> + g_io_channel_ref(io); >> + attr->io = io; >> >> - case ATT_OP_EXEC_WRITE_REQ: >> - return ATT_OP_EXEC_WRITE_RESP; >> + attr->att = bt_att_new(fd); >> + if (!attr->att) >> + goto fail; >> >> - case ATT_OP_HANDLE_IND: >> - return ATT_OP_HANDLE_CNF; >> - } >> + attr->buf = g_malloc0(mtu); >> + attr->buflen = mtu; >> + if (!attr->buf) >> + goto fail; >> >> - return 0; >> -} >> + attr->callbacks = g_queue_new(); >> + if (!attr->callbacks) >> + goto fail; >> >> -static bool is_response(guint8 opcode) >> -{ >> - switch (opcode) { >> - case ATT_OP_ERROR: >> - case ATT_OP_MTU_RESP: >> - case ATT_OP_FIND_INFO_RESP: >> - case ATT_OP_FIND_BY_TYPE_RESP: >> - case ATT_OP_READ_BY_TYPE_RESP: >> - case ATT_OP_READ_RESP: >> - case ATT_OP_READ_BLOB_RESP: >> - case ATT_OP_READ_MULTI_RESP: >> - case ATT_OP_READ_BY_GROUP_RESP: >> - case ATT_OP_WRITE_RESP: >> - case ATT_OP_PREP_WRITE_RESP: >> - case ATT_OP_EXEC_WRITE_RESP: >> - case ATT_OP_HANDLE_CNF: >> - return true; >> - } >> + return g_attrib_ref(attr); >> >> - return false; >> -} >> - >> -static bool is_request(guint8 opcode) >> -{ >> - switch (opcode) { >> - case ATT_OP_MTU_REQ: >> - case ATT_OP_FIND_INFO_REQ: >> - case ATT_OP_FIND_BY_TYPE_REQ: >> - case ATT_OP_READ_BY_TYPE_REQ: >> - case ATT_OP_READ_REQ: >> - case ATT_OP_READ_BLOB_REQ: >> - case ATT_OP_READ_MULTI_REQ: >> - case ATT_OP_READ_BY_GROUP_REQ: >> - case ATT_OP_WRITE_REQ: >> - case ATT_OP_WRITE_CMD: >> - case ATT_OP_PREP_WRITE_REQ: >> - case ATT_OP_EXEC_WRITE_REQ: >> - return true; >> - } >> - >> - return false; >> +fail: >> + free(attr->buf); >> + bt_att_unref(attr->att); >> + g_io_channel_unref(io); >> + free(attr); >> + return NULL; >> } >> >> GAttrib *g_attrib_ref(GAttrib *attrib) >> { >> - int refs; >> - >> if (!attrib) >> return NULL; >> >> - refs = __sync_add_and_fetch(&attrib->refs, 1); >> + __sync_fetch_and_add(&attrib->ref_count, 1); >> >> - DBG("%p: ref=%d", attrib, refs); >> + DBG("%p: g_attrib_ref=%d ", attrib, attrib->ref_count); >> >> return attrib; >> } >> >> -static void command_destroy(struct command *cmd) >> +static void attrib_callbacks_destroy(void *user_data) >> { >> - if (cmd->notify) >> - cmd->notify(cmd->user_data); >> + struct attrib_callbacks *cb; >> >> - g_free(cmd->pdu); >> - g_free(cmd); >> -} >> + cb = (struct attrib_callbacks *)user_data; >> + if (!user_data || !g_queue_remove(cb->parent->callbacks, user_data)) >> + return; >> >> -static void event_destroy(struct event *evt) >> -{ >> - if (evt->notify) >> - evt->notify(evt->user_data); >> + if (cb->destroy_func) >> + cb->destroy_func(cb->user_data); >> >> - g_free(evt); >> + free(user_data); >> } >> >> -static void attrib_destroy(GAttrib *attrib) >> +void g_attrib_unref(GAttrib *attrib) >> { >> - GSList *l; >> - struct command *c; >> - >> - while ((c = g_queue_pop_head(attrib->requests))) >> - command_destroy(c); >> - >> - while ((c = g_queue_pop_head(attrib->responses))) >> - command_destroy(c); >> - >> - g_queue_free(attrib->requests); >> - attrib->requests = NULL; >> - >> - g_queue_free(attrib->responses); >> - attrib->responses = NULL; >> + struct attrib_callbacks *cb; >> >> - for (l = attrib->events; l; l = l->next) >> - event_destroy(l->data); >> - >> - g_slist_free(attrib->events); >> - attrib->events = NULL; >> - >> - if (attrib->timeout_watch > 0) >> - g_source_remove(attrib->timeout_watch); >> - >> - if (attrib->write_watch > 0) >> - g_source_remove(attrib->write_watch); >> - >> - if (attrib->read_watch > 0) >> - g_source_remove(attrib->read_watch); >> + if (!attrib) >> + return; >> >> - if (attrib->io) >> - g_io_channel_unref(attrib->io); >> + DBG("%p: g_attrib_unref=%d ", attrib, attrib->ref_count-1); >> >> - g_free(attrib->buf); >> + if (__sync_sub_and_fetch(&attrib->ref_count, 1)) >> + return; >> >> if (attrib->destroy) >> attrib->destroy(attrib->destroy_user_data); >> >> - g_free(attrib); >> -} >> + while ((cb = g_queue_peek_head(attrib->callbacks))) >> + attrib_callbacks_destroy(cb); >> >> -void g_attrib_unref(GAttrib *attrib) >> -{ >> - int refs; >> + g_queue_free(attrib->callbacks); >> >> - if (!attrib) >> - return; >> - >> - refs = __sync_sub_and_fetch(&attrib->refs, 1); >> + g_free(attrib->buf); >> >> - DBG("%p: ref=%d", attrib, refs); >> + bt_att_unref(attrib->att); >> >> - if (refs > 0) >> - return; >> + g_io_channel_unref(attrib->io); >> >> - attrib_destroy(attrib); >> + g_free(attrib); >> } >> >> GIOChannel *g_attrib_get_channel(GAttrib *attrib) >> @@ -270,7 +166,7 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib) >> gboolean g_attrib_set_destroy_function(GAttrib *attrib, >> GDestroyNotify destroy, gpointer user_data) >> { >> - if (attrib == NULL) >> + if (!attrib) >> return FALSE; >> >> attrib->destroy = destroy; >> @@ -279,380 +175,116 @@ gboolean g_attrib_set_destroy_function(GAttrib *attrib, >> return TRUE; >> } >> >> -static gboolean disconnect_timeout(gpointer data) >> -{ >> - struct _GAttrib *attrib = data; >> - struct command *c; >> - >> - g_attrib_ref(attrib); >> - >> - c = g_queue_pop_head(attrib->requests); >> - if (c == NULL) >> - goto done; >> - >> - if (c->func) >> - c->func(ATT_ECODE_TIMEOUT, NULL, 0, c->user_data); >> - >> - command_destroy(c); >> - >> - while ((c = g_queue_pop_head(attrib->requests))) { >> - if (c->func) >> - c->func(ATT_ECODE_ABORTED, NULL, 0, c->user_data); >> - command_destroy(c); >> - } >> - >> -done: >> - attrib->stale = true; >> - >> - g_attrib_unref(attrib); >> - >> - return FALSE; >> -} >> - >> -static gboolean can_write_data(GIOChannel *io, GIOCondition cond, >> - gpointer data) >> -{ >> - struct _GAttrib *attrib = data; >> - struct command *cmd; >> - GError *gerr = NULL; >> - gsize len; >> - GIOStatus iostat; >> - GQueue *queue; >> - >> - if (attrib->stale) >> - return FALSE; >> - >> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) >> - return FALSE; >> - >> - queue = attrib->responses; >> - cmd = g_queue_peek_head(queue); >> - if (cmd == NULL) { >> - queue = attrib->requests; >> - cmd = g_queue_peek_head(queue); >> - } >> - if (cmd == NULL) >> - return FALSE; >> - >> - /* >> - * Verify that we didn't already send this command. This can only >> - * happen with elementes from attrib->requests. >> - */ >> - if (cmd->sent) >> - return FALSE; >> - >> - iostat = g_io_channel_write_chars(io, (char *) cmd->pdu, cmd->len, >> - &len, &gerr); >> - if (iostat != G_IO_STATUS_NORMAL) { >> - if (gerr) { >> - error("%s", gerr->message); >> - g_error_free(gerr); >> - } >> - >> - return FALSE; >> - } >> - >> - if (cmd->expected == 0) { >> - g_queue_pop_head(queue); >> - command_destroy(cmd); >> >> - return TRUE; >> - } >> - >> - cmd->sent = true; >> - >> - if (attrib->timeout_watch == 0) >> - attrib->timeout_watch = g_timeout_add_seconds(GATT_TIMEOUT, >> - disconnect_timeout, attrib); >> - >> - return FALSE; >> -} >> - >> -static void destroy_sender(gpointer data) >> +static void attrib_callback_result(uint8_t opcode, const void *pdu, >> + uint16_t length, void *user_data) >> { >> - struct _GAttrib *attrib = data; >> - >> - attrib->write_watch = 0; >> - g_attrib_unref(attrib); >> -} >> + uint8_t *buf; >> + struct attrib_callbacks *cb = user_data; >> >> -static void wake_up_sender(struct _GAttrib *attrib) >> -{ >> - if (attrib->write_watch > 0) >> + if (!cb) >> return; >> >> - attrib = g_attrib_ref(attrib); >> - attrib->write_watch = g_io_add_watch_full(attrib->io, >> - G_PRIORITY_DEFAULT, G_IO_OUT, >> - can_write_data, attrib, destroy_sender); >> -} >> - >> -static bool match_event(struct event *evt, const uint8_t *pdu, gsize len) >> -{ >> - guint16 handle; >> - >> - if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS) >> - return true; >> - >> - if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES) >> - return true; >> - >> - if (len < 3) >> - return false; >> + buf = g_malloc0(length+1); >> + if (!buf) >> + return; >> >> - handle = get_le16(&pdu[1]); >> + buf[0] = opcode; >> + memcpy(buf+1, pdu, length); >> >> - if (evt->expected == pdu[0] && evt->handle == handle) >> - return true; >> + if (cb->result_func) >> + cb->result_func(0, buf, length+1, cb->user_data); >> >> - return false; >> + g_free(buf); >> } >> >> -static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data) >> -{ >> - struct _GAttrib *attrib = data; >> - struct command *cmd = NULL; >> - GSList *l; >> - uint8_t buf[512], status; >> - gsize len; >> - GIOStatus iostat; >> - >> - if (attrib->stale) >> - return FALSE; >> - >> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) { >> - struct command *c; >> - >> - while ((c = g_queue_pop_head(attrib->requests))) { >> - if (c->func) >> - c->func(ATT_ECODE_IO, NULL, 0, c->user_data); >> - command_destroy(c); >> - } >> - >> - attrib->read_watch = 0; >> - >> - return FALSE; >> - } >> - >> - memset(buf, 0, sizeof(buf)); >> >> - iostat = g_io_channel_read_chars(io, (char *) buf, sizeof(buf), >> - &len, NULL); >> - if (iostat != G_IO_STATUS_NORMAL) { >> - status = ATT_ECODE_IO; >> - goto done; >> - } >> - >> - for (l = attrib->events; l; l = l->next) { >> - struct event *evt = l->data; >> - >> - if (match_event(evt, buf, len)) >> - evt->func(buf, len, evt->user_data); >> - } >> - >> - if (!is_response(buf[0])) >> - return TRUE; >> - >> - if (attrib->timeout_watch > 0) { >> - g_source_remove(attrib->timeout_watch); >> - attrib->timeout_watch = 0; >> - } >> - >> - cmd = g_queue_pop_head(attrib->requests); >> - if (cmd == NULL) { >> - /* Keep the watch if we have events to report */ >> - return attrib->events != NULL; >> - } >> - >> - if (buf[0] == ATT_OP_ERROR) { >> - status = buf[4]; >> - goto done; >> - } >> - >> - if (cmd->expected != buf[0]) { >> - status = ATT_ECODE_IO; >> - goto done; >> - } >> - >> - status = 0; >> - >> -done: >> - if (!g_queue_is_empty(attrib->requests) || >> - !g_queue_is_empty(attrib->responses)) >> - wake_up_sender(attrib); >> - >> - if (cmd) { >> - if (cmd->func) >> - cmd->func(status, buf, len, cmd->user_data); >> - >> - command_destroy(cmd); >> - } >> +static void attrib_callback_notify(uint8_t opcode, const void *pdu, >> + uint16_t length, void *user_data) >> +{ >> + uint8_t *buf; >> + struct attrib_callbacks *cb = user_data; >> >> - return TRUE; >> -} >> + if (!cb) >> + return; >> >> -GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) >> -{ >> - struct _GAttrib *attrib; >> + if (cb->notify_func == NULL) >> + return; >> >> - g_io_channel_set_encoding(io, NULL, NULL); >> - g_io_channel_set_buffered(io, FALSE); >> + if (cb->notify_handle != GATTRIB_ALL_HANDLES && length < 2) >> + return; >> >> - attrib = g_try_new0(struct _GAttrib, 1); >> - if (attrib == NULL) >> - return NULL; >> + if (cb->notify_handle != GATTRIB_ALL_HANDLES && >> + cb->notify_handle != get_le16(pdu)) >> + return; >> >> - attrib->buf = g_malloc0(mtu); >> - attrib->buflen = mtu; >> + buf = g_malloc0(length+1); >> + if (!buf) >> + return; >> >> - attrib->io = g_io_channel_ref(io); >> - attrib->requests = g_queue_new(); >> - attrib->responses = g_queue_new(); >> + buf[0] = opcode; >> + memcpy(buf+1, pdu, length); >> >> - attrib->read_watch = g_io_add_watch(attrib->io, >> - G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL, >> - received_data, attrib); >> + cb->notify_func(buf, length+1, cb->user_data); >> >> - return g_attrib_ref(attrib); >> + g_free(buf); >> } >> >> guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len, >> GAttribResultFunc func, gpointer user_data, >> GDestroyNotify notify) >> { >> - struct command *c; >> - GQueue *queue; >> - uint8_t opcode; >> - >> - if (attrib->stale) >> - return 0; >> - >> - c = g_try_new0(struct command, 1); >> - if (c == NULL) >> - return 0; >> - >> - opcode = pdu[0]; >> - >> - c->opcode = opcode; >> - c->expected = opcode2expected(opcode); >> - c->pdu = g_malloc(len); >> - memcpy(c->pdu, pdu, len); >> - c->len = len; >> - c->func = func; >> - c->user_data = user_data; >> - c->notify = notify; >> - >> - if (is_response(opcode)) >> - queue = attrib->responses; >> - else >> - queue = attrib->requests; >> - >> - if (id) { >> - c->id = id; >> - if (!is_response(opcode)) >> - g_queue_push_head(queue, c); >> - else >> - /* Don't re-order responses even if an ID is given */ >> - g_queue_push_tail(queue, c); >> - } else { >> - c->id = ++attrib->next_cmd_id; >> - g_queue_push_tail(queue, c); >> + struct attrib_callbacks *cb = NULL; >> + bt_att_response_func_t response_cb = NULL; >> + bt_att_destroy_func_t destroy_cb = NULL; >> + >> + if (func || notify) { >> + cb = new0(struct attrib_callbacks, 1); >> + if (cb == 0) >> + return 0; >> + cb->result_func = func; >> + cb->user_data = user_data; >> + cb->destroy_func = notify; >> + cb->parent = attrib; >> + g_queue_push_head(attrib->callbacks, cb); >> + response_cb = attrib_callback_result; >> + destroy_cb = attrib_callbacks_destroy; >> } >> >> - /* >> - * If a command was added to the queue and it was empty before, wake up >> - * the sender. If the sender was already woken up by the second queue, >> - * wake_up_sender will just return. >> - */ >> - if (g_queue_get_length(queue) == 1) >> - wake_up_sender(attrib); >> - >> - return c->id; >> -} >> - >> -static int command_cmp_by_id(gconstpointer a, gconstpointer b) >> -{ >> - const struct command *cmd = a; >> - guint id = GPOINTER_TO_UINT(b); >> - >> - return cmd->id - id; >> + return bt_att_send(attrib->att, pdu[0], (void *)pdu+1, len-1, >> + response_cb, cb, destroy_cb); >> } >> >> gboolean g_attrib_cancel(GAttrib *attrib, guint id) >> { >> - GList *l = NULL; >> - struct command *cmd; >> - GQueue *queue; >> - >> - if (attrib == NULL) >> - return FALSE; >> - >> - queue = attrib->requests; >> - if (queue) >> - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), >> - command_cmp_by_id); >> - if (l == NULL) { >> - queue = attrib->responses; >> - if (!queue) >> - return FALSE; >> - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), >> - command_cmp_by_id); >> - } >> - >> - if (l == NULL) >> - return FALSE; >> - >> - cmd = l->data; >> - >> - if (cmd == g_queue_peek_head(queue) && cmd->sent) >> - cmd->func = NULL; >> - else { >> - g_queue_remove(queue, cmd); >> - command_destroy(cmd); >> - } >> - >> - return TRUE; >> + return bt_att_cancel(attrib->att, id); >> } >> >> -static gboolean cancel_all_per_queue(GQueue *queue) >> +gboolean g_attrib_cancel_all(GAttrib *attrib) >> { >> - struct command *c, *head = NULL; >> - gboolean first = TRUE; >> - >> - if (queue == NULL) >> - return FALSE; >> - >> - while ((c = g_queue_pop_head(queue))) { >> - if (first && c->sent) { >> - /* If the command was sent ignore its callback ... */ >> - c->func = NULL; >> - head = c; >> - continue; >> - } >> - >> - first = FALSE; >> - command_destroy(c); >> - } >> - >> - if (head) { >> - /* ... and put it back in the queue */ >> - g_queue_push_head(queue, head); >> - } >> - >> - return TRUE; >> + return bt_att_cancel_all(attrib->att); >> } >> >> -gboolean g_attrib_cancel_all(GAttrib *attrib) >> +guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, >> + GAttribNotifyFunc func, gpointer user_data, >> + GDestroyNotify notify) >> { >> - gboolean ret; >> - >> - if (attrib == NULL) >> - return FALSE; >> - >> - ret = cancel_all_per_queue(attrib->requests); >> - ret = cancel_all_per_queue(attrib->responses) && ret; >> + struct attrib_callbacks *cb = NULL; >> + >> + if (func || notify) { >> + cb = new0(struct attrib_callbacks, 1); >> + if (cb == 0) >> + return 0; >> + cb->notify_func = func; >> + cb->notify_handle = handle; >> + cb->user_data = user_data; >> + cb->destroy_func = notify; >> + cb->parent = attrib; >> + g_queue_push_head(attrib->callbacks, cb); >> + } >> >> - return ret; >> + return bt_att_register(attrib->att, opcode, attrib_callback_notify, >> + cb, attrib_callbacks_destroy); >> } >> >> uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) >> @@ -661,98 +293,26 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) >> return NULL; >> >> *len = attrib->buflen; >> - >> return attrib->buf; >> } >> >> gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu) >> { >> - if (mtu < ATT_DEFAULT_LE_MTU) >> - return FALSE; >> - >> - attrib->buf = g_realloc(attrib->buf, mtu); >> - >> - attrib->buflen = mtu; >> - >> - return TRUE; >> -} >> - >> -guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, >> - GAttribNotifyFunc func, gpointer user_data, >> - GDestroyNotify notify) >> -{ >> - static guint next_evt_id = 0; >> - struct event *event; >> - >> - event = g_try_new0(struct event, 1); >> - if (event == NULL) >> - return 0; >> - >> - event->expected = opcode; >> - event->handle = handle; >> - event->func = func; >> - event->user_data = user_data; >> - event->notify = notify; >> - event->id = ++next_evt_id; >> - >> - attrib->events = g_slist_append(attrib->events, event); >> - >> - return event->id; >> -} >> - >> -static int event_cmp_by_id(gconstpointer a, gconstpointer b) >> -{ >> - const struct event *evt = a; >> - guint id = GPOINTER_TO_UINT(b); >> + /* Clients of this expect a buffer to use. */ >> + if (mtu > attrib->buflen) { >> + attrib->buf = g_realloc(attrib->buf, mtu); >> + attrib->buflen = mtu; >> + } >> >> - return evt->id - id; >> + return bt_att_set_mtu(attrib->att, mtu); >> } >> >> gboolean g_attrib_unregister(GAttrib *attrib, guint id) >> { >> - struct event *evt; >> - GSList *l; >> - >> - if (id == 0) { >> - warn("%s: invalid id", __func__); >> - return FALSE; >> - } >> - >> - l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id), >> - event_cmp_by_id); >> - if (l == NULL) >> - return FALSE; >> - >> - evt = l->data; >> - >> - attrib->events = g_slist_remove(attrib->events, evt); >> - >> - if (evt->notify) >> - evt->notify(evt->user_data); >> - >> - g_free(evt); >> - >> - return TRUE; >> + return bt_att_unregister(attrib->att, id); >> } >> >> gboolean g_attrib_unregister_all(GAttrib *attrib) >> { >> - GSList *l; >> - >> - if (attrib->events == NULL) >> - return FALSE; >> - >> - for (l = attrib->events; l; l = l->next) { >> - struct event *evt = l->data; >> - >> - if (evt->notify) >> - evt->notify(evt->user_data); >> - >> - g_free(evt); >> - } >> - >> - g_slist_free(attrib->events); >> - attrib->events = NULL; >> - >> - return TRUE; >> + return bt_att_unregister_all(attrib->att); >> } >> -- >> 2.1.0.rc2.206.gedb03e5 >> >> -- >> 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 > > > > -- > Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att 2014-11-05 15:17 ` Michael Janssen @ 2014-11-05 20:30 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 16+ messages in thread From: Luiz Augusto von Dentz @ 2014-11-05 20:30 UTC (permalink / raw) To: Michael Janssen; +Cc: linux-bluetooth@vger.kernel.org Hi Michael, On Wed, Nov 5, 2014 at 5:17 PM, Michael Janssen <jamuraa@chromium.org> wrote: > Hi Luiz, > > On Wed, Nov 5, 2014 at 6:49 AM, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: >> Hi Michael, >> >> On Tue, Nov 4, 2014 at 11:19 PM, Michael Janssen <jamuraa@chromium.org> wrote: >>> This patch implements a version of GAttrib which is backed by >>> bt_att, which enables the simultaneous use of GAttrib and bt_att. >>> >>> This should enable smooth transition of profiles from the GAttrib >>> API to the src/shared bt_att API. >>> --- >>> attrib/gattrib.c | 734 +++++++++++-------------------------------------------- >>> 1 file changed, 147 insertions(+), 587 deletions(-) >>> >>> diff --git a/attrib/gattrib.c b/attrib/gattrib.c >>> index fa51b6d..63f219d 100644 >>> --- a/attrib/gattrib.c >>> +++ b/attrib/gattrib.c >>> @@ -2,8 +2,7 @@ >>> * >>> * BlueZ - Bluetooth protocol stack for Linux >>> * >>> - * Copyright (C) 2010 Nokia Corporation >>> - * Copyright (C) 2010 Marcel Holtmann <marcel@holtmann.org> >>> + * Copyright (C) 2014 Google, Inc. >> >> You got to be super careful with copyright changes, because of that we >> usually request this to be in a separate patch, except for newly >> created files of course. To speed up this I would suggest you leave >> this for later. > > To be clear: this looking like a change of a bunch of lines of a > single file is an artifact of the patch process - I created this from > an empty file by copying the headers from gattrib.h and implementing > them. It was originally a new file called gattrib-shared.c. Normally > I wouldn't change this copyright stuff and have no issue keeping the > changes for v2. You can try -B option of format patch, that should show up as a complete rewrite. >>> * >>> * >>> * This program is free software; you can redistribute it and/or modify >>> @@ -36,227 +35,124 @@ >>> #include <bluetooth/bluetooth.h> >>> >>> #include "btio/btio.h" >>> -#include "lib/uuid.h" >>> -#include "src/shared/util.h" >>> #include "src/log.h" >>> -#include "attrib/att.h" >>> +#include "src/shared/util.h" >>> +#include "src/shared/att.h" >>> #include "attrib/gattrib.h" >>> >>> -#define GATT_TIMEOUT 30 >>> - >>> struct _GAttrib { >>> + int ref_count; >>> + struct bt_att *att; >>> GIOChannel *io; >>> - int refs; >>> - uint8_t *buf; >>> - size_t buflen; >>> - guint read_watch; >>> - guint write_watch; >>> - guint timeout_watch; >>> - GQueue *requests; >>> - GQueue *responses; >>> - GSList *events; >>> - guint next_cmd_id; >>> GDestroyNotify destroy; >>> gpointer destroy_user_data; >>> - bool stale; >>> + GQueue *callbacks; >>> + uint8_t *buf; >>> + int buflen; >>> }; >>> >>> -struct command { >>> - guint id; >>> - guint8 opcode; >>> - guint8 *pdu; >>> - guint16 len; >>> - guint8 expected; >>> - bool sent; >>> - GAttribResultFunc func; >>> - gpointer user_data; >>> - GDestroyNotify notify; >>> -}; >>> >>> -struct event { >>> - guint id; >>> - guint8 expected; >>> - guint16 handle; >>> - GAttribNotifyFunc func; >>> +struct attrib_callbacks { >>> + GAttribResultFunc result_func; >>> + GAttribNotifyFunc notify_func; >>> + GDestroyNotify destroy_func; >>> gpointer user_data; >>> - GDestroyNotify notify; >>> + GAttrib *parent; >>> + uint16_t notify_handle; >>> }; >>> >>> -static guint8 opcode2expected(guint8 opcode) >>> +GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) >>> { >>> - switch (opcode) { >>> - case ATT_OP_MTU_REQ: >>> - return ATT_OP_MTU_RESP; >>> - >>> - case ATT_OP_FIND_INFO_REQ: >>> - return ATT_OP_FIND_INFO_RESP; >>> - >>> - case ATT_OP_FIND_BY_TYPE_REQ: >>> - return ATT_OP_FIND_BY_TYPE_RESP; >>> - >>> - case ATT_OP_READ_BY_TYPE_REQ: >>> - return ATT_OP_READ_BY_TYPE_RESP; >>> - >>> - case ATT_OP_READ_REQ: >>> - return ATT_OP_READ_RESP; >>> - >>> - case ATT_OP_READ_BLOB_REQ: >>> - return ATT_OP_READ_BLOB_RESP; >>> - >>> - case ATT_OP_READ_MULTI_REQ: >>> - return ATT_OP_READ_MULTI_RESP; >>> + gint fd; >>> + GAttrib *attr; >>> >>> - case ATT_OP_READ_BY_GROUP_REQ: >>> - return ATT_OP_READ_BY_GROUP_RESP; >>> + if (!io) >>> + return NULL; >>> >>> - case ATT_OP_WRITE_REQ: >>> - return ATT_OP_WRITE_RESP; >>> + fd = g_io_channel_unix_get_fd(io); >>> + attr = new0(GAttrib, 1); >>> + if (!attr) >>> + return NULL; >>> >>> - case ATT_OP_PREP_WRITE_REQ: >>> - return ATT_OP_PREP_WRITE_RESP; >>> + g_io_channel_ref(io); >>> + attr->io = io; >>> >>> - case ATT_OP_EXEC_WRITE_REQ: >>> - return ATT_OP_EXEC_WRITE_RESP; >>> + attr->att = bt_att_new(fd); >>> + if (!attr->att) >>> + goto fail; >>> >>> - case ATT_OP_HANDLE_IND: >>> - return ATT_OP_HANDLE_CNF; >>> - } >>> + attr->buf = g_malloc0(mtu); >>> + attr->buflen = mtu; >>> + if (!attr->buf) >>> + goto fail; >>> >>> - return 0; >>> -} >>> + attr->callbacks = g_queue_new(); >>> + if (!attr->callbacks) >>> + goto fail; >>> >>> -static bool is_response(guint8 opcode) >>> -{ >>> - switch (opcode) { >>> - case ATT_OP_ERROR: >>> - case ATT_OP_MTU_RESP: >>> - case ATT_OP_FIND_INFO_RESP: >>> - case ATT_OP_FIND_BY_TYPE_RESP: >>> - case ATT_OP_READ_BY_TYPE_RESP: >>> - case ATT_OP_READ_RESP: >>> - case ATT_OP_READ_BLOB_RESP: >>> - case ATT_OP_READ_MULTI_RESP: >>> - case ATT_OP_READ_BY_GROUP_RESP: >>> - case ATT_OP_WRITE_RESP: >>> - case ATT_OP_PREP_WRITE_RESP: >>> - case ATT_OP_EXEC_WRITE_RESP: >>> - case ATT_OP_HANDLE_CNF: >>> - return true; >>> - } >>> + return g_attrib_ref(attr); >>> >>> - return false; >>> -} >>> - >>> -static bool is_request(guint8 opcode) >>> -{ >>> - switch (opcode) { >>> - case ATT_OP_MTU_REQ: >>> - case ATT_OP_FIND_INFO_REQ: >>> - case ATT_OP_FIND_BY_TYPE_REQ: >>> - case ATT_OP_READ_BY_TYPE_REQ: >>> - case ATT_OP_READ_REQ: >>> - case ATT_OP_READ_BLOB_REQ: >>> - case ATT_OP_READ_MULTI_REQ: >>> - case ATT_OP_READ_BY_GROUP_REQ: >>> - case ATT_OP_WRITE_REQ: >>> - case ATT_OP_WRITE_CMD: >>> - case ATT_OP_PREP_WRITE_REQ: >>> - case ATT_OP_EXEC_WRITE_REQ: >>> - return true; >>> - } >>> - >>> - return false; >>> +fail: >>> + free(attr->buf); >>> + bt_att_unref(attr->att); >>> + g_io_channel_unref(io); >>> + free(attr); >>> + return NULL; >>> } >>> >>> GAttrib *g_attrib_ref(GAttrib *attrib) >>> { >>> - int refs; >>> - >>> if (!attrib) >>> return NULL; >>> >>> - refs = __sync_add_and_fetch(&attrib->refs, 1); >>> + __sync_fetch_and_add(&attrib->ref_count, 1); >>> >>> - DBG("%p: ref=%d", attrib, refs); >>> + DBG("%p: g_attrib_ref=%d ", attrib, attrib->ref_count); >>> >>> return attrib; >>> } >>> >>> -static void command_destroy(struct command *cmd) >>> +static void attrib_callbacks_destroy(void *user_data) >>> { >>> - if (cmd->notify) >>> - cmd->notify(cmd->user_data); >>> + struct attrib_callbacks *cb; >>> >>> - g_free(cmd->pdu); >>> - g_free(cmd); >>> -} >>> + cb = (struct attrib_callbacks *)user_data; >>> + if (!user_data || !g_queue_remove(cb->parent->callbacks, user_data)) >>> + return; >>> >>> -static void event_destroy(struct event *evt) >>> -{ >>> - if (evt->notify) >>> - evt->notify(evt->user_data); >>> + if (cb->destroy_func) >>> + cb->destroy_func(cb->user_data); >>> >>> - g_free(evt); >>> + free(user_data); >>> } >>> >>> -static void attrib_destroy(GAttrib *attrib) >>> +void g_attrib_unref(GAttrib *attrib) >>> { >>> - GSList *l; >>> - struct command *c; >>> - >>> - while ((c = g_queue_pop_head(attrib->requests))) >>> - command_destroy(c); >>> - >>> - while ((c = g_queue_pop_head(attrib->responses))) >>> - command_destroy(c); >>> - >>> - g_queue_free(attrib->requests); >>> - attrib->requests = NULL; >>> - >>> - g_queue_free(attrib->responses); >>> - attrib->responses = NULL; >>> + struct attrib_callbacks *cb; >>> >>> - for (l = attrib->events; l; l = l->next) >>> - event_destroy(l->data); >>> - >>> - g_slist_free(attrib->events); >>> - attrib->events = NULL; >>> - >>> - if (attrib->timeout_watch > 0) >>> - g_source_remove(attrib->timeout_watch); >>> - >>> - if (attrib->write_watch > 0) >>> - g_source_remove(attrib->write_watch); >>> - >>> - if (attrib->read_watch > 0) >>> - g_source_remove(attrib->read_watch); >>> + if (!attrib) >>> + return; >>> >>> - if (attrib->io) >>> - g_io_channel_unref(attrib->io); >>> + DBG("%p: g_attrib_unref=%d ", attrib, attrib->ref_count-1); >>> >>> - g_free(attrib->buf); >>> + if (__sync_sub_and_fetch(&attrib->ref_count, 1)) >>> + return; >>> >>> if (attrib->destroy) >>> attrib->destroy(attrib->destroy_user_data); >>> >>> - g_free(attrib); >>> -} >>> + while ((cb = g_queue_peek_head(attrib->callbacks))) >>> + attrib_callbacks_destroy(cb); >>> >>> -void g_attrib_unref(GAttrib *attrib) >>> -{ >>> - int refs; >>> + g_queue_free(attrib->callbacks); >>> >>> - if (!attrib) >>> - return; >>> - >>> - refs = __sync_sub_and_fetch(&attrib->refs, 1); >>> + g_free(attrib->buf); >>> >>> - DBG("%p: ref=%d", attrib, refs); >>> + bt_att_unref(attrib->att); >>> >>> - if (refs > 0) >>> - return; >>> + g_io_channel_unref(attrib->io); >>> >>> - attrib_destroy(attrib); >>> + g_free(attrib); >>> } >>> >>> GIOChannel *g_attrib_get_channel(GAttrib *attrib) >>> @@ -270,7 +166,7 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib) >>> gboolean g_attrib_set_destroy_function(GAttrib *attrib, >>> GDestroyNotify destroy, gpointer user_data) >>> { >>> - if (attrib == NULL) >>> + if (!attrib) >>> return FALSE; >>> >>> attrib->destroy = destroy; >>> @@ -279,380 +175,116 @@ gboolean g_attrib_set_destroy_function(GAttrib *attrib, >>> return TRUE; >>> } >>> >>> -static gboolean disconnect_timeout(gpointer data) >>> -{ >>> - struct _GAttrib *attrib = data; >>> - struct command *c; >>> - >>> - g_attrib_ref(attrib); >>> - >>> - c = g_queue_pop_head(attrib->requests); >>> - if (c == NULL) >>> - goto done; >>> - >>> - if (c->func) >>> - c->func(ATT_ECODE_TIMEOUT, NULL, 0, c->user_data); >>> - >>> - command_destroy(c); >>> - >>> - while ((c = g_queue_pop_head(attrib->requests))) { >>> - if (c->func) >>> - c->func(ATT_ECODE_ABORTED, NULL, 0, c->user_data); >>> - command_destroy(c); >>> - } >>> - >>> -done: >>> - attrib->stale = true; >>> - >>> - g_attrib_unref(attrib); >>> - >>> - return FALSE; >>> -} >>> - >>> -static gboolean can_write_data(GIOChannel *io, GIOCondition cond, >>> - gpointer data) >>> -{ >>> - struct _GAttrib *attrib = data; >>> - struct command *cmd; >>> - GError *gerr = NULL; >>> - gsize len; >>> - GIOStatus iostat; >>> - GQueue *queue; >>> - >>> - if (attrib->stale) >>> - return FALSE; >>> - >>> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) >>> - return FALSE; >>> - >>> - queue = attrib->responses; >>> - cmd = g_queue_peek_head(queue); >>> - if (cmd == NULL) { >>> - queue = attrib->requests; >>> - cmd = g_queue_peek_head(queue); >>> - } >>> - if (cmd == NULL) >>> - return FALSE; >>> - >>> - /* >>> - * Verify that we didn't already send this command. This can only >>> - * happen with elementes from attrib->requests. >>> - */ >>> - if (cmd->sent) >>> - return FALSE; >>> - >>> - iostat = g_io_channel_write_chars(io, (char *) cmd->pdu, cmd->len, >>> - &len, &gerr); >>> - if (iostat != G_IO_STATUS_NORMAL) { >>> - if (gerr) { >>> - error("%s", gerr->message); >>> - g_error_free(gerr); >>> - } >>> - >>> - return FALSE; >>> - } >>> - >>> - if (cmd->expected == 0) { >>> - g_queue_pop_head(queue); >>> - command_destroy(cmd); >>> >>> - return TRUE; >>> - } >>> - >>> - cmd->sent = true; >>> - >>> - if (attrib->timeout_watch == 0) >>> - attrib->timeout_watch = g_timeout_add_seconds(GATT_TIMEOUT, >>> - disconnect_timeout, attrib); >>> - >>> - return FALSE; >>> -} >>> - >>> -static void destroy_sender(gpointer data) >>> +static void attrib_callback_result(uint8_t opcode, const void *pdu, >>> + uint16_t length, void *user_data) >>> { >>> - struct _GAttrib *attrib = data; >>> - >>> - attrib->write_watch = 0; >>> - g_attrib_unref(attrib); >>> -} >>> + uint8_t *buf; >>> + struct attrib_callbacks *cb = user_data; >>> >>> -static void wake_up_sender(struct _GAttrib *attrib) >>> -{ >>> - if (attrib->write_watch > 0) >>> + if (!cb) >>> return; >>> >>> - attrib = g_attrib_ref(attrib); >>> - attrib->write_watch = g_io_add_watch_full(attrib->io, >>> - G_PRIORITY_DEFAULT, G_IO_OUT, >>> - can_write_data, attrib, destroy_sender); >>> -} >>> - >>> -static bool match_event(struct event *evt, const uint8_t *pdu, gsize len) >>> -{ >>> - guint16 handle; >>> - >>> - if (is_request(pdu[0]) && evt->expected == GATTRIB_ALL_REQS) >>> - return true; >>> - >>> - if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES) >>> - return true; >>> - >>> - if (len < 3) >>> - return false; >>> + buf = g_malloc0(length+1); >>> + if (!buf) >>> + return; >>> >>> - handle = get_le16(&pdu[1]); >>> + buf[0] = opcode; >>> + memcpy(buf+1, pdu, length); >>> >>> - if (evt->expected == pdu[0] && evt->handle == handle) >>> - return true; >>> + if (cb->result_func) >>> + cb->result_func(0, buf, length+1, cb->user_data); >>> >>> - return false; >>> + g_free(buf); >>> } >>> >>> -static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data) >>> -{ >>> - struct _GAttrib *attrib = data; >>> - struct command *cmd = NULL; >>> - GSList *l; >>> - uint8_t buf[512], status; >>> - gsize len; >>> - GIOStatus iostat; >>> - >>> - if (attrib->stale) >>> - return FALSE; >>> - >>> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) { >>> - struct command *c; >>> - >>> - while ((c = g_queue_pop_head(attrib->requests))) { >>> - if (c->func) >>> - c->func(ATT_ECODE_IO, NULL, 0, c->user_data); >>> - command_destroy(c); >>> - } >>> - >>> - attrib->read_watch = 0; >>> - >>> - return FALSE; >>> - } >>> - >>> - memset(buf, 0, sizeof(buf)); >>> >>> - iostat = g_io_channel_read_chars(io, (char *) buf, sizeof(buf), >>> - &len, NULL); >>> - if (iostat != G_IO_STATUS_NORMAL) { >>> - status = ATT_ECODE_IO; >>> - goto done; >>> - } >>> - >>> - for (l = attrib->events; l; l = l->next) { >>> - struct event *evt = l->data; >>> - >>> - if (match_event(evt, buf, len)) >>> - evt->func(buf, len, evt->user_data); >>> - } >>> - >>> - if (!is_response(buf[0])) >>> - return TRUE; >>> - >>> - if (attrib->timeout_watch > 0) { >>> - g_source_remove(attrib->timeout_watch); >>> - attrib->timeout_watch = 0; >>> - } >>> - >>> - cmd = g_queue_pop_head(attrib->requests); >>> - if (cmd == NULL) { >>> - /* Keep the watch if we have events to report */ >>> - return attrib->events != NULL; >>> - } >>> - >>> - if (buf[0] == ATT_OP_ERROR) { >>> - status = buf[4]; >>> - goto done; >>> - } >>> - >>> - if (cmd->expected != buf[0]) { >>> - status = ATT_ECODE_IO; >>> - goto done; >>> - } >>> - >>> - status = 0; >>> - >>> -done: >>> - if (!g_queue_is_empty(attrib->requests) || >>> - !g_queue_is_empty(attrib->responses)) >>> - wake_up_sender(attrib); >>> - >>> - if (cmd) { >>> - if (cmd->func) >>> - cmd->func(status, buf, len, cmd->user_data); >>> - >>> - command_destroy(cmd); >>> - } >>> +static void attrib_callback_notify(uint8_t opcode, const void *pdu, >>> + uint16_t length, void *user_data) >>> +{ >>> + uint8_t *buf; >>> + struct attrib_callbacks *cb = user_data; >>> >>> - return TRUE; >>> -} >>> + if (!cb) >>> + return; >>> >>> -GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu) >>> -{ >>> - struct _GAttrib *attrib; >>> + if (cb->notify_func == NULL) >>> + return; >>> >>> - g_io_channel_set_encoding(io, NULL, NULL); >>> - g_io_channel_set_buffered(io, FALSE); >>> + if (cb->notify_handle != GATTRIB_ALL_HANDLES && length < 2) >>> + return; >>> >>> - attrib = g_try_new0(struct _GAttrib, 1); >>> - if (attrib == NULL) >>> - return NULL; >>> + if (cb->notify_handle != GATTRIB_ALL_HANDLES && >>> + cb->notify_handle != get_le16(pdu)) >>> + return; >>> >>> - attrib->buf = g_malloc0(mtu); >>> - attrib->buflen = mtu; >>> + buf = g_malloc0(length+1); >>> + if (!buf) >>> + return; >>> >>> - attrib->io = g_io_channel_ref(io); >>> - attrib->requests = g_queue_new(); >>> - attrib->responses = g_queue_new(); >>> + buf[0] = opcode; >>> + memcpy(buf+1, pdu, length); >>> >>> - attrib->read_watch = g_io_add_watch(attrib->io, >>> - G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL, >>> - received_data, attrib); >>> + cb->notify_func(buf, length+1, cb->user_data); >>> >>> - return g_attrib_ref(attrib); >>> + g_free(buf); >>> } >>> >>> guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len, >>> GAttribResultFunc func, gpointer user_data, >>> GDestroyNotify notify) >>> { >>> - struct command *c; >>> - GQueue *queue; >>> - uint8_t opcode; >>> - >>> - if (attrib->stale) >>> - return 0; >>> - >>> - c = g_try_new0(struct command, 1); >>> - if (c == NULL) >>> - return 0; >>> - >>> - opcode = pdu[0]; >>> - >>> - c->opcode = opcode; >>> - c->expected = opcode2expected(opcode); >>> - c->pdu = g_malloc(len); >>> - memcpy(c->pdu, pdu, len); >>> - c->len = len; >>> - c->func = func; >>> - c->user_data = user_data; >>> - c->notify = notify; >>> - >>> - if (is_response(opcode)) >>> - queue = attrib->responses; >>> - else >>> - queue = attrib->requests; >>> - >>> - if (id) { >>> - c->id = id; >>> - if (!is_response(opcode)) >>> - g_queue_push_head(queue, c); >>> - else >>> - /* Don't re-order responses even if an ID is given */ >>> - g_queue_push_tail(queue, c); >>> - } else { >>> - c->id = ++attrib->next_cmd_id; >>> - g_queue_push_tail(queue, c); >>> + struct attrib_callbacks *cb = NULL; >>> + bt_att_response_func_t response_cb = NULL; >>> + bt_att_destroy_func_t destroy_cb = NULL; >>> + >>> + if (func || notify) { >>> + cb = new0(struct attrib_callbacks, 1); >>> + if (cb == 0) >>> + return 0; >>> + cb->result_func = func; >>> + cb->user_data = user_data; >>> + cb->destroy_func = notify; >>> + cb->parent = attrib; >>> + g_queue_push_head(attrib->callbacks, cb); >>> + response_cb = attrib_callback_result; >>> + destroy_cb = attrib_callbacks_destroy; >>> } >>> >>> - /* >>> - * If a command was added to the queue and it was empty before, wake up >>> - * the sender. If the sender was already woken up by the second queue, >>> - * wake_up_sender will just return. >>> - */ >>> - if (g_queue_get_length(queue) == 1) >>> - wake_up_sender(attrib); >>> - >>> - return c->id; >>> -} >>> - >>> -static int command_cmp_by_id(gconstpointer a, gconstpointer b) >>> -{ >>> - const struct command *cmd = a; >>> - guint id = GPOINTER_TO_UINT(b); >>> - >>> - return cmd->id - id; >>> + return bt_att_send(attrib->att, pdu[0], (void *)pdu+1, len-1, >>> + response_cb, cb, destroy_cb); >>> } >>> >>> gboolean g_attrib_cancel(GAttrib *attrib, guint id) >>> { >>> - GList *l = NULL; >>> - struct command *cmd; >>> - GQueue *queue; >>> - >>> - if (attrib == NULL) >>> - return FALSE; >>> - >>> - queue = attrib->requests; >>> - if (queue) >>> - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), >>> - command_cmp_by_id); >>> - if (l == NULL) { >>> - queue = attrib->responses; >>> - if (!queue) >>> - return FALSE; >>> - l = g_queue_find_custom(queue, GUINT_TO_POINTER(id), >>> - command_cmp_by_id); >>> - } >>> - >>> - if (l == NULL) >>> - return FALSE; >>> - >>> - cmd = l->data; >>> - >>> - if (cmd == g_queue_peek_head(queue) && cmd->sent) >>> - cmd->func = NULL; >>> - else { >>> - g_queue_remove(queue, cmd); >>> - command_destroy(cmd); >>> - } >>> - >>> - return TRUE; >>> + return bt_att_cancel(attrib->att, id); >>> } >>> >>> -static gboolean cancel_all_per_queue(GQueue *queue) >>> +gboolean g_attrib_cancel_all(GAttrib *attrib) >>> { >>> - struct command *c, *head = NULL; >>> - gboolean first = TRUE; >>> - >>> - if (queue == NULL) >>> - return FALSE; >>> - >>> - while ((c = g_queue_pop_head(queue))) { >>> - if (first && c->sent) { >>> - /* If the command was sent ignore its callback ... */ >>> - c->func = NULL; >>> - head = c; >>> - continue; >>> - } >>> - >>> - first = FALSE; >>> - command_destroy(c); >>> - } >>> - >>> - if (head) { >>> - /* ... and put it back in the queue */ >>> - g_queue_push_head(queue, head); >>> - } >>> - >>> - return TRUE; >>> + return bt_att_cancel_all(attrib->att); >>> } >>> >>> -gboolean g_attrib_cancel_all(GAttrib *attrib) >>> +guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, >>> + GAttribNotifyFunc func, gpointer user_data, >>> + GDestroyNotify notify) >>> { >>> - gboolean ret; >>> - >>> - if (attrib == NULL) >>> - return FALSE; >>> - >>> - ret = cancel_all_per_queue(attrib->requests); >>> - ret = cancel_all_per_queue(attrib->responses) && ret; >>> + struct attrib_callbacks *cb = NULL; >>> + >>> + if (func || notify) { >>> + cb = new0(struct attrib_callbacks, 1); >>> + if (cb == 0) >>> + return 0; >>> + cb->notify_func = func; >>> + cb->notify_handle = handle; >>> + cb->user_data = user_data; >>> + cb->destroy_func = notify; >>> + cb->parent = attrib; >>> + g_queue_push_head(attrib->callbacks, cb); >>> + } >>> >>> - return ret; >>> + return bt_att_register(attrib->att, opcode, attrib_callback_notify, >>> + cb, attrib_callbacks_destroy); >>> } >>> >>> uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) >>> @@ -661,98 +293,26 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len) >>> return NULL; >>> >>> *len = attrib->buflen; >>> - >>> return attrib->buf; >>> } >>> >>> gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu) >>> { >>> - if (mtu < ATT_DEFAULT_LE_MTU) >>> - return FALSE; >>> - >>> - attrib->buf = g_realloc(attrib->buf, mtu); >>> - >>> - attrib->buflen = mtu; >>> - >>> - return TRUE; >>> -} >>> - >>> -guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle, >>> - GAttribNotifyFunc func, gpointer user_data, >>> - GDestroyNotify notify) >>> -{ >>> - static guint next_evt_id = 0; >>> - struct event *event; >>> - >>> - event = g_try_new0(struct event, 1); >>> - if (event == NULL) >>> - return 0; >>> - >>> - event->expected = opcode; >>> - event->handle = handle; >>> - event->func = func; >>> - event->user_data = user_data; >>> - event->notify = notify; >>> - event->id = ++next_evt_id; >>> - >>> - attrib->events = g_slist_append(attrib->events, event); >>> - >>> - return event->id; >>> -} >>> - >>> -static int event_cmp_by_id(gconstpointer a, gconstpointer b) >>> -{ >>> - const struct event *evt = a; >>> - guint id = GPOINTER_TO_UINT(b); >>> + /* Clients of this expect a buffer to use. */ >>> + if (mtu > attrib->buflen) { >>> + attrib->buf = g_realloc(attrib->buf, mtu); >>> + attrib->buflen = mtu; >>> + } >>> >>> - return evt->id - id; >>> + return bt_att_set_mtu(attrib->att, mtu); >>> } >>> >>> gboolean g_attrib_unregister(GAttrib *attrib, guint id) >>> { >>> - struct event *evt; >>> - GSList *l; >>> - >>> - if (id == 0) { >>> - warn("%s: invalid id", __func__); >>> - return FALSE; >>> - } >>> - >>> - l = g_slist_find_custom(attrib->events, GUINT_TO_POINTER(id), >>> - event_cmp_by_id); >>> - if (l == NULL) >>> - return FALSE; >>> - >>> - evt = l->data; >>> - >>> - attrib->events = g_slist_remove(attrib->events, evt); >>> - >>> - if (evt->notify) >>> - evt->notify(evt->user_data); >>> - >>> - g_free(evt); >>> - >>> - return TRUE; >>> + return bt_att_unregister(attrib->att, id); >>> } >>> >>> gboolean g_attrib_unregister_all(GAttrib *attrib) >>> { >>> - GSList *l; >>> - >>> - if (attrib->events == NULL) >>> - return FALSE; >>> - >>> - for (l = attrib->events; l; l = l->next) { >>> - struct event *evt = l->data; >>> - >>> - if (evt->notify) >>> - evt->notify(evt->user_data); >>> - >>> - g_free(evt); >>> - } >>> - >>> - g_slist_free(attrib->events); >>> - attrib->events = NULL; >>> - >>> - return TRUE; >>> + return bt_att_unregister_all(attrib->att); >>> } >>> -- >>> 2.1.0.rc2.206.gedb03e5 >>> >>> -- >>> 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 >> >> >> >> -- >> Luiz Augusto von Dentz -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-11-13 16:16 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-12 20:22 [PATCH BlueZ 0/4] android fixes and gattrib shim Michael Janssen 2014-11-12 20:22 ` [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs Michael Janssen 2014-11-12 20:51 ` Szymon Janc 2014-11-13 15:30 ` Michael Janssen 2014-11-13 16:16 ` Luiz Augusto von Dentz 2014-11-12 20:22 ` [PATCH BlueZ 2/4] android/tester-gatt: deduplicate read-by-type PDUs Michael Janssen 2014-11-12 20:22 ` [PATCH BlueZ 3/4] android/gatt: dummy callback for indications Michael Janssen 2014-11-12 20:56 ` Szymon Janc 2014-11-12 20:22 ` [PATCH BlueZ 4/4] GATT shim to src/shared bt_att Michael Janssen 2014-11-12 21:14 ` Szymon Janc 2014-11-13 0:49 ` Arman Uguray 2014-11-13 14:05 ` Luiz Augusto von Dentz -- strict thread matches above, loose matches on Subject: below -- 2014-11-04 21:19 [PATCH BlueZ 0/4] GAttrib cleanup and switch to bt_att Michael Janssen 2014-11-04 21:19 ` [PATCH BlueZ 4/4] GATT shim to src/shared bt_att Michael Janssen 2014-11-05 14:49 ` Luiz Augusto von Dentz 2014-11-05 15:17 ` Michael Janssen 2014-11-05 20:30 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox