2011-03-10 14:01:30

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: [PATCH v4] Implement ATT handle indications

This patch takes advantage of g_attrib_send() implicit guarantee of
queueing i.e. indication is only sent if the previous one has been
confirmed, so we don't need to maintain an explicit queue.
---
src/attrib-server.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index b980b28..f157424 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -875,6 +875,8 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
length = find_by_type(start, end, &uuid, value, vlen,
opdu, channel->mtu);
break;
+ case ATT_OP_HANDLE_CNF:
+ return;
case ATT_OP_READ_MULTI_REQ:
case ATT_OP_PREP_WRITE_REQ:
case ATT_OP_EXEC_WRITE_REQ:
@@ -972,6 +974,20 @@ static void attrib_notify_clients(struct attribute *attr)
g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
NULL, NULL, NULL);
}
+
+ /* Indication */
+ if (g_slist_find_custom(channel->indicate,
+ GUINT_TO_POINTER(handle), handle_cmp)) {
+ uint8_t pdu[ATT_MAX_MTU];
+ uint16_t len;
+
+ len = enc_indication(attr, pdu, channel->mtu);
+ if (len == 0)
+ return;
+
+ g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
+ NULL, NULL, NULL);
+ }
}
}

--
1.7.1



2011-03-10 14:48:02

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH v4] Implement ATT handle indications

Hi Claudio,

On Thu, Mar 10, 2011 at 10:16 AM, Claudio Takahasi
<[email protected]> wrote:
> Hi Elvis,
>
> Is it allowed to have notification and indication bits set to the same
> characteristic?
> If it is not allowed you could simplify the logic inside the "for".

The spec does not forbid both indication and notification to be
enabled, as Elvis mentioned.
>> + ? ? ? ? ? ? ? /* Indication */
>> + ? ? ? ? ? ? ? if (g_slist_find_custom(channel->indicate,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? GUINT_TO_POINTER(handle), handle_cmp)) {
>> + ? ? ? ? ? ? ? ? ? ? ? uint8_t pdu[ATT_MAX_MTU];
>> + ? ? ? ? ? ? ? ? ? ? ? uint16_t len;
>
> The same declarations(pdu and len) exist ?for Notification. I suggest
> move to the outer scope.

Actually, it was a suggestion from Johan to move these declarations to
inner scope. But at the time there was only one if() statement, which
justified this.

Anyway, patch is applied, so this can be made into a simple refactor
if you find it better to put in outer scope.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-03-10 14:35:26

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: Re: [PATCH v4] Implement ATT handle indications


On 10 Mar 2011, at 11:16 , Claudio Takahasi wrote:

> Hi Elvis,
>
> Is it allowed to have notification and indication bits set to the same
> characteristic?

Client Characteristic Configuration is a bitmap, and in theory it is possible
for a client to set both bits (configuration and indication), even though it
looks silly to do so. BlueZ currently allows this and I didn't find any
mention in the docs preventing it.

Andr? had mentioned something about mutual exclusion of indicaitons and
notifications some days ago. Do you have anything to add, Andre?

2011-03-10 14:31:01

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v4] Implement ATT handle indications

Hi Elvis,

On Thu, Mar 10, 2011, Elvis Pf??tzenreuter wrote:
> This patch takes advantage of g_attrib_send() implicit guarantee of
> queueing i.e. indication is only sent if the previous one has been
> confirmed, so we don't need to maintain an explicit queue.
> ---
> src/attrib-server.c | 16 ++++++++++++++++
> 1 files changed, 16 insertions(+), 0 deletions(-)

Pushed upstream. Thanks.

Johan

2011-03-10 14:16:55

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [PATCH v4] Implement ATT handle indications

SGkgRWx2aXMsCgpJcyBpdCBhbGxvd2VkIHRvIGhhdmUgbm90aWZpY2F0aW9uIGFuZCBpbmRpY2F0
aW9uIGJpdHMgc2V0IHRvIHRoZSBzYW1lCmNoYXJhY3RlcmlzdGljPwpJZiBpdCBpcyBub3QgYWxs
b3dlZCB5b3UgY291bGQgc2ltcGxpZnkgdGhlIGxvZ2ljIGluc2lkZSB0aGUgImZvciIuCgpPbiBU
aHUsIE1hciAxMCwgMjAxMSBhdCAyOjAxIFBNLCBFbHZpcyBQZsO8dHplbnJldXRlciA8ZXB4QHNp
Z25vdmUuY29tPiB3cm90ZToKPiBUaGlzIHBhdGNoIHRha2VzIGFkdmFudGFnZSBvZiBnX2F0dHJp
Yl9zZW5kKCkgaW1wbGljaXQgZ3VhcmFudGVlIG9mCj4gcXVldWVpbmcgaS5lLiBpbmRpY2F0aW9u
IGlzIG9ubHkgc2VudCBpZiB0aGUgcHJldmlvdXMgb25lIGhhcyBiZWVuCj4gY29uZmlybWVkLCBz
byB3ZSBkb24ndCBuZWVkIHRvIG1haW50YWluIGFuIGV4cGxpY2l0IHF1ZXVlLgo+IC0tLQo+IMKg
c3JjL2F0dHJpYi1zZXJ2ZXIuYyB8IMKgIDE2ICsrKysrKysrKysrKysrKysKPiDCoDEgZmlsZXMg
Y2hhbmdlZCwgMTYgaW5zZXJ0aW9ucygrKSwgMCBkZWxldGlvbnMoLSkKPgo+IGRpZmYgLS1naXQg
YS9zcmMvYXR0cmliLXNlcnZlci5jIGIvc3JjL2F0dHJpYi1zZXJ2ZXIuYwo+IGluZGV4IGI5ODBi
MjguLmYxNTc0MjQgMTAwNjQ0Cj4gLS0tIGEvc3JjL2F0dHJpYi1zZXJ2ZXIuYwo+ICsrKyBiL3Ny
Yy9hdHRyaWItc2VydmVyLmMKPiBAQCAtODc1LDYgKzg3NSw4IEBAIHN0YXRpYyB2b2lkIGNoYW5u
ZWxfaGFuZGxlcihjb25zdCB1aW50OF90ICppcGR1LCB1aW50MTZfdCBsZW4sCj4gwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqBsZW5ndGggPSBmaW5kX2J5X3R5cGUoc3RhcnQsIGVuZCwgJnV1aWQsIHZh
bHVlLCB2bGVuLAo+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgb3BkdSwgY2hhbm5lbC0+bXR1
KTsKPiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoGJyZWFrOwo+ICsgwqAgwqAgwqAgY2FzZSBBVFRf
T1BfSEFORExFX0NORjoKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHJldHVybjsKPiDCoCDCoCDC
oCDCoGNhc2UgQVRUX09QX1JFQURfTVVMVElfUkVROgo+IMKgIMKgIMKgIMKgY2FzZSBBVFRfT1Bf
UFJFUF9XUklURV9SRVE6Cj4gwqAgwqAgwqAgwqBjYXNlIEFUVF9PUF9FWEVDX1dSSVRFX1JFUToK
PiBAQCAtOTcyLDYgKzk3NCwyMCBAQCBzdGF0aWMgdm9pZCBhdHRyaWJfbm90aWZ5X2NsaWVudHMo
c3RydWN0IGF0dHJpYnV0ZSAqYXR0cikKPiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoGdfYXR0cmliX3NlbmQoY2hhbm5lbC0+YXR0cmliLCAwLCBwZHVbMF0sIHBkdSwgbGVuLAo+
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgTlVMTCwgTlVMTCwgTlVMTCk7Cj4gwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqB9Cj4gKwo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgLyogSW5kaWNhdGlv
biAqLwo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgaWYgKGdfc2xpc3RfZmluZF9jdXN0b20oY2hh
bm5lbC0+aW5kaWNhdGUsCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCBHVUlOVF9UT19QT0lOVEVSKGhhbmRsZSksIGhhbmRsZV9jbXAp
KSB7Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB1aW50OF90IHBkdVtBVFRf
TUFYX01UVV07Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB1aW50MTZfdCBs
ZW47CgpUaGUgc2FtZSBkZWNsYXJhdGlvbnMocGR1IGFuZCBsZW4pIGV4aXN0ICBmb3IgTm90aWZp
Y2F0aW9uLiBJIHN1Z2dlc3QKbW92ZSB0byB0aGUgb3V0ZXIgc2NvcGUuCgpCUiwKQ2xhdWRpbwoK
PiArCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBsZW4gPSBlbmNfaW5kaWNh
dGlvbihhdHRyLCBwZHUsIGNoYW5uZWwtPm10dSk7Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCBpZiAobGVuID09IDApCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCByZXR1cm47Cj4gKwo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgZ19hdHRyaWJfc2VuZChjaGFubmVsLT5hdHRyaWIsIDAsIHBkdVswXSwgcGR1LCBs
ZW4sCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBOVUxMLCBOVUxMLCBOVUxMKTsKPiArIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIH0KPiDCoCDCoCDCoCDCoH0KPiDCoH0KPgo+IC0tCj4gMS43LjEK
Pgo+IC0tCj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVu
c3Vic2NyaWJlIGxpbnV4LWJsdWV0b290aCIgaW4KPiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8g
bWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZwo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgwqBodHRw
Oi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwKPgo=