* [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 10:51 Health Thermometer Profile (HTP) Santiago Carot-Nemesio
@ 2011-11-09 10:51 ` Santiago Carot-Nemesio
2011-11-09 12:00 ` Anderson Lizardo
0 siblings, 1 reply; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-09 10:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
thermometer/thermometer.c | 57 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 56 insertions(+), 1 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 5b0e30a..0928f15 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -163,6 +163,17 @@ static gint cmp_char_uuid(gconstpointer a, gconstpointer b)
return g_strcmp0(ch->attr.uuid, uuid);
}
+static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
+{
+ const struct characteristic *ch = a;
+ const uint16_t *handle = b;
+
+ if (ch->attr.value_handle == *handle)
+ return 0;
+
+ return -1;
+}
+
static gint cmp_descriptor(gconstpointer a, gconstpointer b)
{
const struct descriptor *desc = a;
@@ -703,9 +714,53 @@ static GDBusSignalTable thermometer_signals[] = {
{ }
};
+static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
+ uint16_t len, gboolean final)
+{
+ DBG("TODO: Process measurement indication");
+}
+
+static void proc_measurement_interval(struct thermometer *t, const uint8_t *pdu,
+ uint16_t len)
+{
+ DBG("TODO: Process measurements interval indication");
+}
+
static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
{
- /* TODO: Process indication */
+ struct thermometer *t = user_data;
+ const struct characteristic *ch;
+ uint8_t opdu[ATT_MAX_MTU];
+ uint16_t handle, olen;
+ GSList *l;
+
+ if (len < 3) {
+ DBG("Bad pdu received");
+ goto done;
+ }
+
+ if (pdu[0] != ATT_OP_HANDLE_IND) {
+ DBG("Not indication received");
+ return;
+ }
+
+ handle = att_get_u16(&pdu[1]);
+ l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
+ if (l == NULL)
+ goto done;
+
+ ch = l->data;
+ if (g_strcmp0(ch->attr.uuid, TEMPERATURE_MEASUREMENT_UUID) == 0)
+ proc_measurement(t, pdu, len, TRUE);
+ else if (g_strcmp0(ch->attr.uuid, MEASUREMENT_INTERVAL_UUID) == 0)
+ proc_measurement_interval(t, pdu, len);
+
+done:
+ olen = enc_confirmation(opdu, sizeof(opdu));
+
+ if (olen > 0)
+ g_attrib_send(t->attrib, 0, opdu[0], opdu, olen, NULL, NULL,
+ NULL);
}
static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
--
1.7.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 10:51 ` [PATCH 1/6] Manage GATT attribute indications in handle callback Santiago Carot-Nemesio
@ 2011-11-09 12:00 ` Anderson Lizardo
2011-11-09 12:54 ` Hendrik Sattler
2011-11-09 14:44 ` Santiago Carot
0 siblings, 2 replies; 20+ messages in thread
From: Anderson Lizardo @ 2011-11-09 12:00 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
Hi Santiago,
On Wed, Nov 9, 2011 at 6:51 AM, Santiago Carot-Nemesio
<sancane@gmail.com> wrote:
> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
> +{
> + const struct characteristic *ch = a;
> + const uint16_t *handle = b;
> +
> + if (ch->attr.value_handle == *handle)
> + return 0;
> +
> + return -1;
> +}
Usually we implement the function above as:
return ch->attr.value_handle - *handle;
It will work exactly as your code.
> static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
> {
> - /* TODO: Process indication */
> + struct thermometer *t = user_data;
> + const struct characteristic *ch;
> + uint8_t opdu[ATT_MAX_MTU];
> + uint16_t handle, olen;
> + GSList *l;
> +
> + if (len < 3) {
> + DBG("Bad pdu received");
> + goto done;
> + }
Are you sure we should send a confirmation even if the PDU received is
invalid? What if it is not even an indication (you only check below)?
> +
> + if (pdu[0] != ATT_OP_HANDLE_IND) {
> + DBG("Not indication received");
> + return;
> + }
I suggest a more descriptive debug message here, something like:
DBG("Unexpected ATT opcode: 0x%02x", pdu[0]);
> +
> + handle = att_get_u16(&pdu[1]);
> + l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
> + if (l == NULL)
> + goto done;
Again, should we send a confirmation to the thermometer even if the
indication's handle does not match any characteristic value?
(note I didn't read the thermometer profile spec carefully yet. Not
sure if it covers invalid PDUs.)
> +
> + ch = l->data;
> + if (g_strcmp0(ch->attr.uuid, TEMPERATURE_MEASUREMENT_UUID) == 0)
> + proc_measurement(t, pdu, len, TRUE);
> + else if (g_strcmp0(ch->attr.uuid, MEASUREMENT_INTERVAL_UUID) == 0)
> + proc_measurement_interval(t, pdu, len);
> +
> +done:
> + olen = enc_confirmation(opdu, sizeof(opdu));
> +
> + if (olen > 0)
> + g_attrib_send(t->attrib, 0, opdu[0], opdu, olen, NULL, NULL,
> + NULL);
> }
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 12:00 ` Anderson Lizardo
@ 2011-11-09 12:54 ` Hendrik Sattler
2011-11-09 13:30 ` Anderson Lizardo
2011-11-09 14:44 ` Santiago Carot
1 sibling, 1 reply; 20+ messages in thread
From: Hendrik Sattler @ 2011-11-09 12:54 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Santiago Carot-Nemesio, linux-bluetooth
Am 09.11.2011 13:00, schrieb Anderson Lizardo:
> Hi Santiago,
>
> On Wed, Nov 9, 2011 at 6:51 AM, Santiago Carot-Nemesio
> <sancane@gmail.com> wrote:
>> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
>> +{
>> + const struct characteristic *ch = a;
>> + const uint16_t *handle = b;
>> +
>> + if (ch->attr.value_handle == *handle)
>> + return 0;
>> +
>> + return -1;
>> +}
>
> Usually we implement the function above as:
>
> return ch->attr.value_handle - *handle;
>
> It will work exactly as your code.
You can do it this way with signed integers but not with unsigned
integers, unless you cast both to signed first.
Still, the above code completely misses the +1 case.
HS
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 12:54 ` Hendrik Sattler
@ 2011-11-09 13:30 ` Anderson Lizardo
2011-11-09 13:53 ` Hendrik Sattler
0 siblings, 1 reply; 20+ messages in thread
From: Anderson Lizardo @ 2011-11-09 13:30 UTC (permalink / raw)
To: Hendrik Sattler; +Cc: Santiago Carot-Nemesio, linux-bluetooth
Hi Hendrik,
On Wed, Nov 9, 2011 at 8:54 AM, Hendrik Sattler <post@hendrik-sattler.de> wrote:
> Am 09.11.2011 13:00, schrieb Anderson Lizardo:
>>
>> Hi Santiago,
>>
>> On Wed, Nov 9, 2011 at 6:51 AM, Santiago Carot-Nemesio
>> <sancane@gmail.com> wrote:
>>>
>>> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
>>> +{
>>> + const struct characteristic *ch = a;
>>> + const uint16_t *handle = b;
>>> +
>>> + if (ch->attr.value_handle == *handle)
>>> + return 0;
>>> +
>>> + return -1;
>>> +}
>>
>> Usually we implement the function above as:
>>
>> return ch->attr.value_handle - *handle;
>>
>> It will work exactly as your code.
>
> You can do it this way with signed integers but not with unsigned integers,
> unless you cast both to signed first.
> Still, the above code completely misses the +1 case.
The idea here is to return 0 if they are same or non zero if they are
not same. The actual sign is not relevant because the comparison is
for finding items, not sorting them (in this case).
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 13:30 ` Anderson Lizardo
@ 2011-11-09 13:53 ` Hendrik Sattler
2011-11-09 14:17 ` Anderson Lizardo
0 siblings, 1 reply; 20+ messages in thread
From: Hendrik Sattler @ 2011-11-09 13:53 UTC (permalink / raw)
To: linux-bluetooth
Am 09.11.2011 14:30, schrieb Anderson Lizardo:
> Hi Hendrik,
>
> On Wed, Nov 9, 2011 at 8:54 AM, Hendrik Sattler
> <post@hendrik-sattler.de> wrote:
>> Am 09.11.2011 13:00, schrieb Anderson Lizardo:
>>>
>>> Hi Santiago,
>>>
>>> On Wed, Nov 9, 2011 at 6:51 AM, Santiago Carot-Nemesio
>>> <sancane@gmail.com> wrote:
>>>>
>>>> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
>>>> +{
>>>> + const struct characteristic *ch = a;
>>>> + const uint16_t *handle = b;
>>>> +
>>>> + if (ch->attr.value_handle == *handle)
>>>> + return 0;
>>>> +
>>>> + return -1;
>>>> +}
>>>
>>> Usually we implement the function above as:
>>>
>>> return ch->attr.value_handle - *handle;
>>>
>>> It will work exactly as your code.
>>
>> You can do it this way with signed integers but not with unsigned
>> integers,
>> unless you cast both to signed first.
>> Still, the above code completely misses the +1 case.
>
> The idea here is to return 0 if they are same or non zero if they are
> not same. The actual sign is not relevant because the comparison is
> for finding items, not sorting them (in this case).
Kind of a lazy approach, no?
Ah well...
HS
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 13:53 ` Hendrik Sattler
@ 2011-11-09 14:17 ` Anderson Lizardo
0 siblings, 0 replies; 20+ messages in thread
From: Anderson Lizardo @ 2011-11-09 14:17 UTC (permalink / raw)
To: Hendrik Sattler; +Cc: linux-bluetooth
Hi Hendrik,
On Wed, Nov 9, 2011 at 9:53 AM, Hendrik Sattler <post@hendrik-sattler.de> wrote:
>> The idea here is to return 0 if they are same or non zero if they are
>> not same. The actual sign is not relevant because the comparison is
>> for finding items, not sorting them (in this case).
>
> Kind of a lazy approach, no?
> Ah well...
I agree with you, and maybe it is even risky if soneone reuses these
functions for list sorting as well without extending them, but this is
the current procedure on BlueZ as a whole:
$ grep -R 'return.*handle - ' src/ attrib/
src/sdpd-database.c: return rec1->handle - rec2->handle;
src/sdpd-database.c: return rec1->handle - rec2->handle;
src/device.c: return r1->handle - r2->handle;
src/attrib-server.c: return attrib->handle - handle;
src/attrib-server.c: return attrib1->handle - attrib2->handle;
attrib/client.c: return chr->handle - handle;
(all these handles are unsigned.)
Note that I'm not defending the current usage, but then it would need
be "fixed" in a separate commit to fix all cases and maintain
consistency.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 12:00 ` Anderson Lizardo
2011-11-09 12:54 ` Hendrik Sattler
@ 2011-11-09 14:44 ` Santiago Carot
2011-11-09 15:40 ` Anderson Lizardo
1 sibling, 1 reply; 20+ messages in thread
From: Santiago Carot @ 2011-11-09 14:44 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
Hi Anderson,
Please, see coments below,
2011/11/9 Anderson Lizardo <anderson.lizardo@openbossa.org>:
> Hi Santiago,
>
> On Wed, Nov 9, 2011 at 6:51 AM, Santiago Carot-Nemesio
> <sancane@gmail.com> wrote:
>> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
>> +{
>> + const struct characteristic *ch = a;
>> + const uint16_t *handle = b;
>> +
>> + if (ch->attr.value_handle == *handle)
>> + return 0;
>> +
>> + return -1;
>> +}
>
> Usually we implement the function above as:
>
> return ch->attr.value_handle - *handle;
>
> It will work exactly as your code.
>
ok, that's only a nitpick.
>> static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
>> {
>> - /* TODO: Process indication */
>> + struct thermometer *t = user_data;
>> + const struct characteristic *ch;
>> + uint8_t opdu[ATT_MAX_MTU];
>> + uint16_t handle, olen;
>> + GSList *l;
>> +
>> + if (len < 3) {
>> + DBG("Bad pdu received");
>> + goto done;
>> + }
>
> Are you sure we should send a confirmation even if the PDU received is
> invalid? What if it is not even an indication (you only check below)?
This callback is supposed only to manage indications not notifications:
in the code:
...
g_attrib_register(t->attrib, ATT_OP_HANDLE_IND,
ind_handler, t, NULL);
Shall I expect another kind of notifications even if I have not
indicated it when I registered the callback?
>
>> +
>> + if (pdu[0] != ATT_OP_HANDLE_IND) {
>> + DBG("Not indication received");
>> + return;
>> + }
>
> I suggest a more descriptive debug message here, something like:
>
> DBG("Unexpected ATT opcode: 0x%02x", pdu[0]);
>
OK, no problem.
>> +
>> + handle = att_get_u16(&pdu[1]);
>> + l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
>> + if (l == NULL)
>> + goto done;
>
> Again, should we send a confirmation to the thermometer even if the
> indication's handle does not match any characteristic value?
>
> (note I didn't read the thermometer profile spec carefully yet. Not
> sure if it covers invalid PDUs.)
>
As far as I know, the thermometer spec is not focused in such kind of
GATT details, so I guess it will depend of the implementation. If we
don't send a reply to that indication the remote side will close the
connection when the timeout expired. So here is the question: what
would we rather do, to keep the connection alive or to let the remote
side to close it?
Waiting comments.
Regards
Santiago
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 14:44 ` Santiago Carot
@ 2011-11-09 15:40 ` Anderson Lizardo
2011-11-09 16:50 ` Santiago Carot
0 siblings, 1 reply; 20+ messages in thread
From: Anderson Lizardo @ 2011-11-09 15:40 UTC (permalink / raw)
To: Santiago Carot; +Cc: linux-bluetooth
Hi Santiago,
On Wed, Nov 9, 2011 at 10:44 AM, Santiago Carot <sancane@gmail.com> wrote:
>>> static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
>>> {
>>> - /* TODO: Process indication */
>>> + struct thermometer *t = user_data;
>>> + const struct characteristic *ch;
>>> + uint8_t opdu[ATT_MAX_MTU];
>>> + uint16_t handle, olen;
>>> + GSList *l;
>>> +
>>> + if (len < 3) {
>>> + DBG("Bad pdu received");
>>> + goto done;
>>> + }
>>
>> Are you sure we should send a confirmation even if the PDU received is
>> invalid? What if it is not even an indication (you only check below)?
>
> This callback is supposed only to manage indications not notifications:
> in the code:
> ...
> g_attrib_register(t->attrib, ATT_OP_HANDLE_IND,
> ind_handler, t, NULL);
>
> Shall I expect another kind of notifications even if I have not
> indicated it when I registered the callback?
I only made this comment because you do this check on the function:
+ if (pdu[0] != ATT_OP_HANDLE_IND) {
+ DBG("Not indication received");
+ return;
+ }
I think you can simply drop this check, as you mentioned it will only
be called for indications.
About the PDU size check, I think it is important to have it (to avoid
memory corruption on invalid PDU), but then the peer device is bogus,
so I think you should not send a confirmation for it, because you did
not receive a valid indication in the first place.
>>> +
>>> + handle = att_get_u16(&pdu[1]);
>>> + l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
>>> + if (l == NULL)
>>> + goto done;
>>
>> Again, should we send a confirmation to the thermometer even if the
>> indication's handle does not match any characteristic value?
>>
>> (note I didn't read the thermometer profile spec carefully yet. Not
>> sure if it covers invalid PDUs.)
>>
>
> As far as I know, the thermometer spec is not focused in such kind of
> GATT details, so I guess it will depend of the implementation. If we
> don't send a reply to that indication the remote side will close the
> connection when the timeout expired. So here is the question: what
> would we rather do, to keep the connection alive or to let the remote
> side to close it?
I believe that in general, if the peer device sends a bogus indication
(or if the communication channel was corrupted somehow), we should
*not* confirm it. I see confirmations as an "ack" that the indication
was received, processed and validated by BlueZ. If it is somehow
invalid (PDU size incorrect, invalid handle), we should not confirm
it.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-09 15:40 ` Anderson Lizardo
@ 2011-11-09 16:50 ` Santiago Carot
0 siblings, 0 replies; 20+ messages in thread
From: Santiago Carot @ 2011-11-09 16:50 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
Hi Anderson,
2011/11/9 Anderson Lizardo <anderson.lizardo@openbossa.org>:
> Hi Santiago,
>
> On Wed, Nov 9, 2011 at 10:44 AM, Santiago Carot <sancane@gmail.com> wrote:
>>>> static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
>>>> {
>>>> - /* TODO: Process indication */
>>>> + struct thermometer *t = user_data;
>>>> + const struct characteristic *ch;
>>>> + uint8_t opdu[ATT_MAX_MTU];
>>>> + uint16_t handle, olen;
>>>> + GSList *l;
>>>> +
>>>> + if (len < 3) {
>>>> + DBG("Bad pdu received");
>>>> + goto done;
>>>> + }
>>>
>>> Are you sure we should send a confirmation even if the PDU received is
>>> invalid? What if it is not even an indication (you only check below)?
>>
>> This callback is supposed only to manage indications not notifications:
>> in the code:
>> ...
>> g_attrib_register(t->attrib, ATT_OP_HANDLE_IND,
>> ind_handler, t, NULL);
>>
>> Shall I expect another kind of notifications even if I have not
>> indicated it when I registered the callback?
>
> I only made this comment because you do this check on the function:
>
> + if (pdu[0] != ATT_OP_HANDLE_IND) {
> + DBG("Not indication received");
> + return;
> + }
>
> I think you can simply drop this check, as you mentioned it will only
> be called for indications.
>
> About the PDU size check, I think it is important to have it (to avoid
> memory corruption on invalid PDU), but then the peer device is bogus,
> so I think you should not send a confirmation for it, because you did
> not receive a valid indication in the first place.
>
>>>> +
>>>> + handle = att_get_u16(&pdu[1]);
>>>> + l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
>>>> + if (l == NULL)
>>>> + goto done;
>>>
>>> Again, should we send a confirmation to the thermometer even if the
>>> indication's handle does not match any characteristic value?
>>>
>>> (note I didn't read the thermometer profile spec carefully yet. Not
>>> sure if it covers invalid PDUs.)
>>>
>>
>> As far as I know, the thermometer spec is not focused in such kind of
>> GATT details, so I guess it will depend of the implementation. If we
>> don't send a reply to that indication the remote side will close the
>> connection when the timeout expired. So here is the question: what
>> would we rather do, to keep the connection alive or to let the remote
>> side to close it?
>
> I believe that in general, if the peer device sends a bogus indication
> (or if the communication channel was corrupted somehow), we should
> *not* confirm it. I see confirmations as an "ack" that the indication
> was received, processed and validated by BlueZ. If it is somehow
> invalid (PDU size incorrect, invalid handle), we should not confirm
> it.
It makes sense for me, I'll send a new set of patches with the changes
you have suggested.
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-10 11:36 Santiago Carot-Nemesio
@ 2011-11-10 11:36 ` Santiago Carot-Nemesio
0 siblings, 0 replies; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-10 11:36 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
thermometer/thermometer.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 50 insertions(+), 1 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 5b0e30a..69ae708 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -163,6 +163,14 @@ static gint cmp_char_uuid(gconstpointer a, gconstpointer b)
return g_strcmp0(ch->attr.uuid, uuid);
}
+static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
+{
+ const struct characteristic *ch = a;
+ const uint16_t *handle = b;
+
+ return ch->attr.value_handle - *handle;
+}
+
static gint cmp_descriptor(gconstpointer a, gconstpointer b)
{
const struct descriptor *desc = a;
@@ -703,9 +711,50 @@ static GDBusSignalTable thermometer_signals[] = {
{ }
};
+static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
+ uint16_t len, gboolean final)
+{
+ DBG("TODO: Process measurement indication");
+}
+
+static void proc_measurement_interval(struct thermometer *t, const uint8_t *pdu,
+ uint16_t len)
+{
+ DBG("TODO: Process measurements interval indication");
+}
+
static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
{
- /* TODO: Process indication */
+ struct thermometer *t = user_data;
+ const struct characteristic *ch;
+ uint8_t opdu[ATT_MAX_MTU];
+ uint16_t handle, olen;
+ GSList *l;
+
+ if (len < 3) {
+ DBG("Bad pdu received");
+ return;
+ }
+
+ handle = att_get_u16(&pdu[1]);
+ l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
+ if (l == NULL) {
+ DBG("Unexpected handle: 0x%04x", handle);
+ return;
+ }
+
+ ch = l->data;
+
+ if (g_strcmp0(ch->attr.uuid, TEMPERATURE_MEASUREMENT_UUID) == 0)
+ proc_measurement(t, pdu, len, TRUE);
+ else if (g_strcmp0(ch->attr.uuid, MEASUREMENT_INTERVAL_UUID) == 0)
+ proc_measurement_interval(t, pdu, len);
+
+ olen = enc_confirmation(opdu, sizeof(opdu));
+
+ if (olen > 0)
+ g_attrib_send(t->attrib, 0, opdu[0], opdu, olen, NULL, NULL,
+ NULL);
}
static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
--
1.7.7.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Health Thermometer Profile (HTP)
@ 2011-11-14 12:11 Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 1/6] Manage GATT attribute indications in handle callback Santiago Carot-Nemesio
2011-11-18 11:15 ` Health Thermometer Profile (HTP) Johan Hedberg
0 siblings, 2 replies; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-14 12:11 UTC (permalink / raw)
To: linux-bluetooth
This is a new set of patches to enhance thermometer plugin fuctionality.
These patches enable watchers to receiave final/intermediate measurements
from thermometers devices. They also includes latest changes suggested
in the list.
[PATCH 1/6] Manage GATT attribute indications in handle callback.
[PATCH 2/6] Parse final measurement indication
[PATCH 3/6] Add org.bluez.ThermometerWatcher interface to default
[PATCH 4/6] Implement EnableIntermediateMeasurement D-Bus method
[PATCH 5/6] Implement DisableIntermediateMeasurement D-Bus method
[PATCH 6/6] Notify intermediate measurements
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/6] Manage GATT attribute indications in handle callback.
2011-11-14 12:11 Health Thermometer Profile (HTP) Santiago Carot-Nemesio
@ 2011-11-14 12:11 ` Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 2/6] Parse final measurement indication Santiago Carot-Nemesio
2011-11-18 11:15 ` Health Thermometer Profile (HTP) Johan Hedberg
1 sibling, 1 reply; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-14 12:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
thermometer/thermometer.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 50 insertions(+), 1 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 5b0e30a..69ae708 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -163,6 +163,14 @@ static gint cmp_char_uuid(gconstpointer a, gconstpointer b)
return g_strcmp0(ch->attr.uuid, uuid);
}
+static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
+{
+ const struct characteristic *ch = a;
+ const uint16_t *handle = b;
+
+ return ch->attr.value_handle - *handle;
+}
+
static gint cmp_descriptor(gconstpointer a, gconstpointer b)
{
const struct descriptor *desc = a;
@@ -703,9 +711,50 @@ static GDBusSignalTable thermometer_signals[] = {
{ }
};
+static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
+ uint16_t len, gboolean final)
+{
+ DBG("TODO: Process measurement indication");
+}
+
+static void proc_measurement_interval(struct thermometer *t, const uint8_t *pdu,
+ uint16_t len)
+{
+ DBG("TODO: Process measurements interval indication");
+}
+
static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
{
- /* TODO: Process indication */
+ struct thermometer *t = user_data;
+ const struct characteristic *ch;
+ uint8_t opdu[ATT_MAX_MTU];
+ uint16_t handle, olen;
+ GSList *l;
+
+ if (len < 3) {
+ DBG("Bad pdu received");
+ return;
+ }
+
+ handle = att_get_u16(&pdu[1]);
+ l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
+ if (l == NULL) {
+ DBG("Unexpected handle: 0x%04x", handle);
+ return;
+ }
+
+ ch = l->data;
+
+ if (g_strcmp0(ch->attr.uuid, TEMPERATURE_MEASUREMENT_UUID) == 0)
+ proc_measurement(t, pdu, len, TRUE);
+ else if (g_strcmp0(ch->attr.uuid, MEASUREMENT_INTERVAL_UUID) == 0)
+ proc_measurement_interval(t, pdu, len);
+
+ olen = enc_confirmation(opdu, sizeof(opdu));
+
+ if (olen > 0)
+ g_attrib_send(t->attrib, 0, opdu[0], opdu, olen, NULL, NULL,
+ NULL);
}
static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
--
1.7.7.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/6] Parse final measurement indication
2011-11-14 12:11 ` [PATCH 1/6] Manage GATT attribute indications in handle callback Santiago Carot-Nemesio
@ 2011-11-14 12:11 ` Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 3/6] Add org.bluez.ThermometerWatcher interface to default policy Santiago Carot-Nemesio
2011-11-14 21:52 ` [PATCH 2/6] Parse final measurement indication Johan Hedberg
0 siblings, 2 replies; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-14 12:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
thermometer/thermometer.c | 163 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 162 insertions(+), 1 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 69ae708..b35c3aa 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -47,6 +47,13 @@
#define INTERMEDIATE_TEMPERATURE_UUID "00002a1e-0000-1000-8000-00805f9b34fb"
#define MEASUREMENT_INTERVAL_UUID "00002a21-0000-1000-8000-00805f9b34fb"
+/* Temperature measurement flag fields */
+#define TEMP_UNITS 0x01
+#define TEMP_TIME_STAMP 0x02
+#define TEMP_TYPE 0x04
+
+#define FLOAT_MAX_MANTISSA 16777216 /* 2^24 */
+
struct thermometer {
DBusConnection *conn; /* The connection to the bus */
struct btd_device *dev; /* Device reference */
@@ -84,8 +91,40 @@ struct watcher {
gchar *path;
};
+struct measurement {
+ gint16 exp;
+ gint32 mant;
+ guint64 time;
+ gboolean suptime;
+ gchar *unit;
+ gchar *type;
+ gchar *value;
+};
+
static GSList *thermometers = NULL;
+const char *temp_type[] = {
+ "<reserved>",
+ "Armpit",
+ "Body",
+ "Ear",
+ "Finger",
+ "Intestines",
+ "Mouth",
+ "Rectum",
+ "Toe",
+ "Tympanum"
+};
+
+static const gchar *temptype2str(uint8_t value)
+{
+ if (value > 0 && value < G_N_ELEMENTS(temp_type))
+ return temp_type[value];
+
+ error("Temperature type %d reserved for future use", value);
+ return NULL;
+}
+
static void destroy_watcher(gpointer user_data)
{
struct watcher *watcher = user_data;
@@ -711,10 +750,132 @@ static GDBusSignalTable thermometer_signals[] = {
{ }
};
+static void update_watcher(gpointer data, gpointer user_data)
+{
+ struct watcher *w = data;
+ struct measurement *m = user_data;
+ DBusConnection *conn = w->t->conn;
+ DBusMessageIter iter;
+ DBusMessageIter dict;
+ DBusMessage *msg;
+
+ msg = dbus_message_new_method_call(w->srv, w->path,
+ "org.bluez.ThermometerWatcher",
+ "MeasurementReceived");
+ if (msg == NULL)
+ return;
+
+ dbus_message_iter_init_append(msg, &iter);
+
+ dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+ dict_append_entry(&dict, "Exponent", DBUS_TYPE_INT16, &m->exp);
+ dict_append_entry(&dict, "Mantissa", DBUS_TYPE_INT32, &m->mant);
+ dict_append_entry(&dict, "Unit", DBUS_TYPE_STRING, &m->unit);
+
+ if (m->suptime)
+ dict_append_entry(&dict, "Time", DBUS_TYPE_UINT64, &m->time);
+
+ dict_append_entry(&dict, "Type", DBUS_TYPE_STRING, &m->type);
+ dict_append_entry(&dict, "Measurement", DBUS_TYPE_STRING, &m->value);
+
+ dbus_message_iter_close_container(&iter, &dict);
+
+ dbus_message_set_no_reply(msg, TRUE);
+ g_dbus_send_message(conn, msg);
+}
+
+static void recv_measurement(struct thermometer *t, struct measurement *m)
+{
+ if (g_strcmp0(m->value, "Intermediate") == 0) {
+ DBG("Notification of intermediate measurement not implemented");
+ return;
+ }
+
+ g_slist_foreach(t->fwatchers, update_watcher, m);
+}
+
static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
uint16_t len, gboolean final)
{
- DBG("TODO: Process measurement indication");
+ struct measurement m;
+ const gchar *type;
+ uint8_t flags;
+ uint32_t raw;
+
+ if (len < 4) {
+ DBG("Mandatory flags are not provided");
+ return;
+ }
+
+ flags = pdu[3];
+ if (flags & TEMP_UNITS)
+ m.unit = "Fahrenheit";
+ else
+ m.unit = "Celsius";
+
+ if (len < 8) {
+ DBG("Temperature measurement value is not provided");
+ return;
+ }
+
+ raw = att_get_u32(&pdu[4]);
+ m.mant = raw & 0x00FFFFFF;
+ m.exp = ((gint32) raw) >> 24;
+
+ if (m.mant & 0x00800000) {
+ /* convert to C2 negative value */
+ m.mant = m.mant - FLOAT_MAX_MANTISSA;
+ }
+
+ if (flags & TEMP_TIME_STAMP) {
+ struct tm ts;
+ time_t time;
+
+ if (len < 15) {
+ DBG("Can't get time stamp value");
+ return;
+ }
+
+ ts.tm_year = att_get_u16(&pdu[8]) - 1900;
+ ts.tm_mon = pdu[10];
+ ts.tm_mday = pdu[11];
+ ts.tm_hour = pdu[12];
+ ts.tm_min = pdu[13];
+ ts.tm_sec = pdu[14];
+ ts.tm_isdst = -1;
+
+ time = mktime(&ts);
+ m.time = (guint64) time;
+ m.suptime = TRUE;
+ } else
+ m.suptime = FALSE;
+
+ if (flags & TEMP_TYPE) {
+ if (len < 16) {
+ DBG("Can't get temperature type");
+ return;
+ }
+
+ type = temptype2str(pdu[15]);
+ } else if (t->has_type)
+ type = temptype2str(t->type);
+ else {
+ DBG("Can't get temperature type");
+ return;
+ }
+
+ if (type == NULL)
+ return;
+
+ m.type = g_strdup(type);
+ m.value = final ? "Final" : "Intermediate";
+
+ recv_measurement(t, &m);
+ g_free(m.type);
}
static void proc_measurement_interval(struct thermometer *t, const uint8_t *pdu,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/6] Add org.bluez.ThermometerWatcher interface to default policy
2011-11-14 12:11 ` [PATCH 2/6] Parse final measurement indication Santiago Carot-Nemesio
@ 2011-11-14 12:11 ` Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 4/6] Implement EnableIntermediateMeasurement D-Bus method Santiago Carot-Nemesio
2011-11-14 21:52 ` [PATCH 2/6] Parse final measurement indication Johan Hedberg
1 sibling, 1 reply; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-14 12:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
src/bluetooth.conf | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/bluetooth.conf b/src/bluetooth.conf
index b5a6af3..664dbd9 100644
--- a/src/bluetooth.conf
+++ b/src/bluetooth.conf
@@ -15,6 +15,7 @@
<allow send_interface="org.bluez.MediaEndpoint"/>
<allow send_interface="org.bluez.MediaPlayer"/>
<allow send_interface="org.bluez.Watcher"/>
+ <allow send_interface="org.bluez.ThermometerWatcher"/>
</policy>
<policy at_console="true">
--
1.7.7.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/6] Implement EnableIntermediateMeasurement D-Bus method
2011-11-14 12:11 ` [PATCH 3/6] Add org.bluez.ThermometerWatcher interface to default policy Santiago Carot-Nemesio
@ 2011-11-14 12:11 ` Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 5/6] Implement DisableIntermediateMeasurement " Santiago Carot-Nemesio
0 siblings, 1 reply; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-14 12:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
thermometer/thermometer.c | 74 +++++++++++++++++++++++++++++++++++++-------
1 files changed, 62 insertions(+), 12 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index b35c3aa..64b6aa6 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -63,6 +63,7 @@ struct thermometer {
guint attindid; /* Att incications id */
GSList *chars; /* Characteristics */
GSList *fwatchers; /* Final measurements */
+ GSList *iwatchers; /* Intermediate measurements */
gboolean intermediate;
guint8 type;
guint16 interval;
@@ -558,7 +559,7 @@ static struct descriptor *get_descriptor(struct characteristic *ch,
return l->data;
}
-static void final_measurement_cb(guint8 status, const guint8 *pdu,
+static void measurement_cb(guint8 status, const guint8 *pdu,
guint16 len, gpointer user_data)
{
gchar *msg = user_data;
@@ -593,8 +594,34 @@ static void enable_final_measurement(struct thermometer *t)
atval[0] = 0x02;
atval[1] = 0x00;
msg = g_strdup("Enable final measurement");
- gatt_write_char(t->attrib, desc->handle, atval, 2,
- final_measurement_cb, msg);
+ gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
+}
+
+static void enable_intermediate_measurement(struct thermometer *t)
+{
+ struct characteristic *ch;
+ struct descriptor *desc;
+ bt_uuid_t btuuid;
+ uint8_t atval[2];
+ gchar *msg;
+
+ ch = get_characteristic(t, INTERMEDIATE_TEMPERATURE_UUID);
+ if (ch == NULL) {
+ DBG("Intermediate measurement characteristic not found");
+ return;
+ }
+
+ bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
+ desc = get_descriptor(ch, &btuuid);
+ if (desc == NULL) {
+ DBG("Client characteristic configuration descriptor not found");
+ return;
+ }
+
+ atval[0] = 0x01;
+ atval[1] = 0x00;
+ msg = g_strdup("Enable intermediate measurement");
+ gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
}
static void disable_final_measurement(struct thermometer *t)
@@ -621,8 +648,7 @@ static void disable_final_measurement(struct thermometer *t)
atval[0] = 0x00;
atval[1] = 0x00;
msg = g_strdup("Disable final measurement");
- gatt_write_char(t->attrib, desc->handle, atval, 2,
- final_measurement_cb, msg);
+ gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
}
static void watcher_exit(DBusConnection *conn, void *user_data)
@@ -639,7 +665,7 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
disable_final_measurement(t);
}
-static struct watcher *find_watcher(struct thermometer *t, const gchar *sender,
+static struct watcher *find_watcher(GSList *list, const gchar *sender,
const gchar *path)
{
struct watcher *match;
@@ -649,7 +675,7 @@ static struct watcher *find_watcher(struct thermometer *t, const gchar *sender,
match->srv = g_strdup(sender);
match->path = g_strdup(path);
- l = g_slist_find_custom(t->fwatchers, match, cmp_watcher);
+ l = g_slist_find_custom(list, match, cmp_watcher);
destroy_watcher(match);
if (l != NULL)
@@ -670,7 +696,7 @@ static DBusMessage *register_watcher(DBusConnection *conn, DBusMessage *msg,
DBUS_TYPE_INVALID))
return btd_error_invalid_args(msg);
- watcher = find_watcher(t, sender, path);
+ watcher = find_watcher(t->fwatchers, sender, path);
if (watcher != NULL)
return btd_error_already_exists(msg);
@@ -703,7 +729,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
DBUS_TYPE_INVALID))
return btd_error_invalid_args(msg);
- watcher = find_watcher(t, sender, path);
+ watcher = find_watcher(t->fwatchers, sender, path);
if (watcher == NULL)
return btd_error_does_not_exist(msg);
@@ -721,9 +747,33 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
static DBusMessage *enable_intermediate(DBusConnection *conn, DBusMessage *msg,
void *data)
{
- /* TODO: */
- return g_dbus_create_error(msg, ERROR_INTERFACE ".ThermometerError",
- "Function not implemented.");
+ const gchar *sender = dbus_message_get_sender(msg);
+ struct thermometer *t = data;
+ struct watcher *watcher;
+ gchar *path;
+
+ if (!t->intermediate)
+ return btd_error_not_supported(msg);
+
+ if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID))
+ return btd_error_invalid_args(msg);
+
+ watcher = find_watcher(t->fwatchers, sender, path);
+ if (watcher == NULL)
+ return btd_error_does_not_exist(msg);
+
+ if (find_watcher(t->iwatchers, sender, path))
+ return btd_error_already_exists(msg);
+
+ DBG("Intermediate measurement watcher %s registered", path);
+
+ if (g_slist_length(t->iwatchers) == 0)
+ enable_intermediate_measurement(t);
+
+ t->iwatchers = g_slist_prepend(t->iwatchers, watcher);
+
+ return dbus_message_new_method_return(msg);
}
static DBusMessage *disable_intermediate(DBusConnection *conn, DBusMessage *msg,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/6] Implement DisableIntermediateMeasurement D-Bus method
2011-11-14 12:11 ` [PATCH 4/6] Implement EnableIntermediateMeasurement D-Bus method Santiago Carot-Nemesio
@ 2011-11-14 12:11 ` Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 6/6] Notify intermediate measurements Santiago Carot-Nemesio
0 siblings, 1 reply; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-14 12:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
thermometer/thermometer.c | 63 ++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 64b6aa6..0e38b19 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -651,6 +651,44 @@ static void disable_final_measurement(struct thermometer *t)
gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
}
+static void disable_intermediate_measurement(struct thermometer *t)
+{
+ struct characteristic *ch;
+ struct descriptor *desc;
+ bt_uuid_t btuuid;
+ uint8_t atval[2];
+ gchar *msg;
+
+ ch = get_characteristic(t, INTERMEDIATE_TEMPERATURE_UUID);
+ if (ch == NULL) {
+ DBG("Intermediate measurement characteristic not found");
+ return;
+ }
+
+ bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
+ desc = get_descriptor(ch, &btuuid);
+ if (desc == NULL) {
+ DBG("Client characteristic configuration descriptor not found");
+ return;
+ }
+
+ atval[0] = 0x00;
+ atval[1] = 0x00;
+ msg = g_strdup("Disable intermediate measurement");
+ gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
+}
+
+static void remove_int_watcher(struct thermometer *t, struct watcher *w)
+{
+ if (!g_slist_find(t->iwatchers, w))
+ return;
+
+ t->iwatchers = g_slist_remove(t->iwatchers, w);
+
+ if (g_slist_length(t->iwatchers) == 0)
+ disable_intermediate_measurement(t);
+}
+
static void watcher_exit(DBusConnection *conn, void *user_data)
{
struct watcher *watcher = user_data;
@@ -658,6 +696,8 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
DBG("Thermometer watcher %s disconnected", watcher->path);
+ remove_int_watcher(t, watcher);
+
t->fwatchers = g_slist_remove(t->fwatchers, watcher);
watcher->id = 0;
@@ -735,6 +775,8 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
DBG("Thermometer watcher %s unregistered", path);
+ remove_int_watcher(t, watcher);
+
t->fwatchers = g_slist_remove(t->fwatchers, watcher);
destroy_watcher(watcher);
@@ -779,9 +821,24 @@ static DBusMessage *enable_intermediate(DBusConnection *conn, DBusMessage *msg,
static DBusMessage *disable_intermediate(DBusConnection *conn, DBusMessage *msg,
void *data)
{
- /* TODO: */
- return g_dbus_create_error(msg, ERROR_INTERFACE ".ThermometerError",
- "Function not implemented.");
+ const gchar *sender = dbus_message_get_sender(msg);
+ struct thermometer *t = data;
+ struct watcher *watcher;
+ gchar *path;
+
+ if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID))
+ return btd_error_invalid_args(msg);
+
+ watcher = find_watcher(t->iwatchers, sender, path);
+ if (watcher == NULL)
+ return btd_error_does_not_exist(msg);
+
+ DBG("Intermediate measurement %s unregistered", path);
+
+ remove_int_watcher(t, watcher);
+
+ return dbus_message_new_method_return(msg);
}
static GDBusMethodTable thermometer_methods[] = {
--
1.7.7.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/6] Notify intermediate measurements
2011-11-14 12:11 ` [PATCH 5/6] Implement DisableIntermediateMeasurement " Santiago Carot-Nemesio
@ 2011-11-14 12:11 ` Santiago Carot-Nemesio
0 siblings, 0 replies; 20+ messages in thread
From: Santiago Carot-Nemesio @ 2011-11-14 12:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
thermometer/thermometer.c | 47 ++++++++++++++++++++++++++++++++++++++++----
1 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 0e38b19..db33cdb 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -61,6 +61,7 @@ struct thermometer {
struct att_range *svc_range; /* Thermometer range */
guint attioid; /* Att watcher id */
guint attindid; /* Att incications id */
+ guint attnotid; /* Att notifications id */
GSList *chars; /* Characteristics */
GSList *fwatchers; /* Final measurements */
GSList *iwatchers; /* Intermediate measurements */
@@ -156,6 +157,9 @@ static void destroy_thermometer(gpointer user_data)
if (t->attindid > 0)
g_attrib_unregister(t->attrib, t->attindid);
+ if (t->attnotid > 0)
+ g_attrib_unregister(t->attrib, t->attnotid);
+
if (t->attrib != NULL)
g_attrib_unref(t->attrib);
@@ -897,12 +901,14 @@ static void update_watcher(gpointer data, gpointer user_data)
static void recv_measurement(struct thermometer *t, struct measurement *m)
{
- if (g_strcmp0(m->value, "Intermediate") == 0) {
- DBG("Notification of intermediate measurement not implemented");
- return;
- }
+ GSList *wlist;
- g_slist_foreach(t->fwatchers, update_watcher, m);
+ if (g_strcmp0(m->value, "Intermediate") == 0)
+ wlist = t->iwatchers;
+ else
+ wlist = t->fwatchers;
+
+ g_slist_foreach(wlist, update_watcher, m);
}
static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
@@ -1025,6 +1031,30 @@ static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
NULL);
}
+static void notif_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
+{
+ struct thermometer *t = user_data;
+ const struct characteristic *ch;
+ uint16_t handle;
+ GSList *l;
+
+ if (len < 3) {
+ DBG("Bad pdu received");
+ return;
+ }
+
+ handle = att_get_u16(&pdu[1]);
+ l = g_slist_find_custom(t->chars, &handle, cmp_char_val_handle);
+ if (l == NULL) {
+ DBG("Unexpected handle: 0x%04x", handle);
+ return;
+ }
+
+ ch = l->data;
+ if (g_strcmp0(ch->attr.uuid, INTERMEDIATE_TEMPERATURE_UUID) == 0)
+ proc_measurement(t, pdu, len, FALSE);
+}
+
static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
{
struct thermometer *t = user_data;
@@ -1033,6 +1063,8 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
t->attindid = g_attrib_register(t->attrib, ATT_OP_HANDLE_IND,
ind_handler, t, NULL);
+ t->attnotid = g_attrib_register(t->attrib, ATT_OP_HANDLE_NOTIFY,
+ notif_handler, t, NULL);
gatt_discover_char(t->attrib, t->svc_range->start, t->svc_range->end,
NULL, configure_thermometer_cb, t);
}
@@ -1048,6 +1080,11 @@ static void attio_disconnected_cb(gpointer user_data)
t->attindid = 0;
}
+ if (t->attnotid > 0) {
+ g_attrib_unregister(t->attrib, t->attnotid);
+ t->attnotid = 0;
+ }
+
g_attrib_unref(t->attrib);
t->attrib = NULL;
}
--
1.7.7.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/6] Parse final measurement indication
2011-11-14 12:11 ` [PATCH 2/6] Parse final measurement indication Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 3/6] Add org.bluez.ThermometerWatcher interface to default policy Santiago Carot-Nemesio
@ 2011-11-14 21:52 ` Johan Hedberg
2011-11-15 10:06 ` Santiago Carot
1 sibling, 1 reply; 20+ messages in thread
From: Johan Hedberg @ 2011-11-14 21:52 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
Hi Santiago,
On Mon, Nov 14, 2011, Santiago Carot-Nemesio wrote:
> + dict_append_entry(&dict, "Exponent", DBUS_TYPE_INT16, &m->exp);
> + dict_append_entry(&dict, "Mantissa", DBUS_TYPE_INT32, &m->mant);
Could you remind me why we have this kind of encoding in the D-Bus
interface instead of using the native DBUS_TYPE_DOUBLE? Using
DBUS_TYPE_DOUBLE seems like the obvious & clean approach from an API
standpoint so there should be good reasons for not using it.
Johan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/6] Parse final measurement indication
2011-11-14 21:52 ` [PATCH 2/6] Parse final measurement indication Johan Hedberg
@ 2011-11-15 10:06 ` Santiago Carot
0 siblings, 0 replies; 20+ messages in thread
From: Santiago Carot @ 2011-11-15 10:06 UTC (permalink / raw)
To: Santiago Carot-Nemesio, linux-bluetooth
Hi Johan,
2011/11/14 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi Santiago,
>
> On Mon, Nov 14, 2011, Santiago Carot-Nemesio wrote:
>> + dict_append_entry(&dict, "Exponent", DBUS_TYPE_INT16, &m->exp);
>> + dict_append_entry(&dict, "Mantissa", DBUS_TYPE_INT32, &m->mant);
>
> Could you remind me why we have this kind of encoding in the D-Bus
> interface instead of using the native DBUS_TYPE_DOUBLE? Using
> DBUS_TYPE_DOUBLE seems like the obvious & clean approach from an API
> standpoint so there should be good reasons for not using it.
>
As far as I remember, the reason why we did that in this way is
because health devices through GATT use IEEE-11073 32-bit FLOAT types,
so in order to keep the thermometer transcodable[1] through the API we
thought the esier way could be by providing its value representation
in their exponent/magnitude components. IEEE-11073 32-bit FLOAT type
has special values to indicate special circumstances so we thought
that it could be better for applications to provide the exp/mantissa
values to make easier for them to detect this kind of issues instead
of comparing floating point numbers directly, wich is more tricky.
Furthermomre, most of the transcoder applications will need to know
expnonet and mantissa values to generate APDUs in an
ISO/IEEE-11073-20601 eco-system, so we thought that it would be easier
for both non-IEEE11073 and transcoder applications to get the FLOAT
value through their exp/mantissa represenation rather than to get
exp/mantissa values from the FLOAT value directly.
Those are some of the reason we were talking about... of course,
suggestion are always welcome.
[1] Personal Health Devices Transcoding Whitepaper
https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=238300
Regards.
Santiago
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Health Thermometer Profile (HTP)
2011-11-14 12:11 Health Thermometer Profile (HTP) Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 1/6] Manage GATT attribute indications in handle callback Santiago Carot-Nemesio
@ 2011-11-18 11:15 ` Johan Hedberg
1 sibling, 0 replies; 20+ messages in thread
From: Johan Hedberg @ 2011-11-18 11:15 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
Hi Santiago,
On Mon, Nov 14, 2011, Santiago Carot-Nemesio wrote:
> This is a new set of patches to enhance thermometer plugin fuctionality.
> These patches enable watchers to receiave final/intermediate measurements
> from thermometers devices. They also includes latest changes suggested
> in the list.
>
> [PATCH 1/6] Manage GATT attribute indications in handle callback.
> [PATCH 2/6] Parse final measurement indication
> [PATCH 3/6] Add org.bluez.ThermometerWatcher interface to default
> [PATCH 4/6] Implement EnableIntermediateMeasurement D-Bus method
> [PATCH 5/6] Implement DisableIntermediateMeasurement D-Bus method
> [PATCH 6/6] Notify intermediate measurements
All of these patches have been pushed upstream. Thanks.
Johan
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-11-18 11:15 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-14 12:11 Health Thermometer Profile (HTP) Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 1/6] Manage GATT attribute indications in handle callback Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 2/6] Parse final measurement indication Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 3/6] Add org.bluez.ThermometerWatcher interface to default policy Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 4/6] Implement EnableIntermediateMeasurement D-Bus method Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 5/6] Implement DisableIntermediateMeasurement " Santiago Carot-Nemesio
2011-11-14 12:11 ` [PATCH 6/6] Notify intermediate measurements Santiago Carot-Nemesio
2011-11-14 21:52 ` [PATCH 2/6] Parse final measurement indication Johan Hedberg
2011-11-15 10:06 ` Santiago Carot
2011-11-18 11:15 ` Health Thermometer Profile (HTP) Johan Hedberg
-- strict thread matches above, loose matches on Subject: below --
2011-11-10 11:36 Santiago Carot-Nemesio
2011-11-10 11:36 ` [PATCH 1/6] Manage GATT attribute indications in handle callback Santiago Carot-Nemesio
2011-11-09 10:51 Health Thermometer Profile (HTP) Santiago Carot-Nemesio
2011-11-09 10:51 ` [PATCH 1/6] Manage GATT attribute indications in handle callback Santiago Carot-Nemesio
2011-11-09 12:00 ` Anderson Lizardo
2011-11-09 12:54 ` Hendrik Sattler
2011-11-09 13:30 ` Anderson Lizardo
2011-11-09 13:53 ` Hendrik Sattler
2011-11-09 14:17 ` Anderson Lizardo
2011-11-09 14:44 ` Santiago Carot
2011-11-09 15:40 ` Anderson Lizardo
2011-11-09 16:50 ` Santiago Carot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).