2014-11-12 20:22:55

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ 0/4] android fixes and gattrib shim

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



2014-11-13 16:16:41

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs

Hi Michael,

On Thu, Nov 13, 2014 at 5:30 PM, Michael Janssen <[email protected]> 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 <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Luiz Augusto von Dentz

2014-11-13 15:30:43

by Marie Janssen

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs

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 <[email protected]> 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

2014-11-13 14:05:52

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att

Hi Michael,

On Thu, Nov 13, 2014 at 2:49 AM, Arman Uguray <[email protected]> wrote:
> Hi Michael & Szymon,
>
>> On Wed, Nov 12, 2014 at 1:14 PM, Szymon Janc <[email protected]> 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

2014-11-13 00:49:28

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att

Hi Michael & Szymon,

> On Wed, Nov 12, 2014 at 1:14 PM, Szymon Janc <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Cheers,
Arman

2014-11-12 21:14:05

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att

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

2014-11-12 20:56:57

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH BlueZ 3/4] android/gatt: dummy callback for indications

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

2014-11-12 20:51:37

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs

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

2014-11-12 20:22:56

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ 1/4] android/tester-gatt: DRY/bugfix search single PDUs

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


2014-11-12 20:22:57

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ 2/4] android/tester-gatt: deduplicate read-by-type PDUs

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


2014-11-12 20:22:59

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att

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


2014-11-12 20:22:58

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ 3/4] android/gatt: dummy callback for indications

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


2014-11-05 20:30:32

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att

Hi Michael,

On Wed, Nov 5, 2014 at 5:17 PM, Michael Janssen <[email protected]> wrote:
> Hi Luiz,
>
> On Wed, Nov 5, 2014 at 6:49 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Michael,
>>
>> On Tue, Nov 4, 2014 at 11:19 PM, Michael Janssen <[email protected]> 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 <[email protected]>
>>> + * 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 [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz



--
Luiz Augusto von Dentz

2014-11-05 15:17:05

by Marie Janssen

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att

Hi Luiz,

On Wed, Nov 5, 2014 at 6:49 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Michael,
>
> On Tue, Nov 4, 2014 at 11:19 PM, Michael Janssen <[email protected]> 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 <[email protected]>
>> + * 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz

2014-11-05 14:49:49

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/4] GATT shim to src/shared bt_att

Hi Michael,

On Tue, Nov 4, 2014 at 11:19 PM, Michael Janssen <[email protected]> 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 <[email protected]>
> + * 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Luiz Augusto von Dentz