2011-12-09 18:18:58

by Santiago Carot

[permalink] [raw]
Subject: Health Thermometer Profile patches

This is a new set of patches wich implement most of the features in
HTP. GATT reconnections are the only missing feature up to now.

These patches depend on the patch:
att.h: Add client characteristic configuration bit fields

Changes include next patches:
[PATCH 1/6] thermometer.c: Implement SetProperty D-Bus method
[PATCH 2/6] thermometer.c: Confiure C.C.C descriptor during the
[PATCH 3/6] thermometer.c: Process measurement interval indications
[PATCH 4/6] thermometer.c: Fix possible null pointer deference
[PATCH 5/6] thermometer.c: Fix bad read operation when time stamp isn't provided
[PATCH 6/6] thermometer.c: Use system types instead of GLIB ones.


2011-12-09 18:18:59

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 1/6] thermometer.c: Implement SetProperty D-Bus method

---
thermometer/thermometer.c | 118 +++++++++++++++++++++++++++++++++++++-------
1 files changed, 99 insertions(+), 19 deletions(-)

diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index db33cdb..2a0fb0f 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -103,6 +103,11 @@ struct measurement {
gchar *value;
};

+struct tmp_interval_data {
+ struct thermometer *thermometer;
+ guint16 interval;
+};
+
static GSList *thermometers = NULL;

const char *temp_type[] = {
@@ -223,6 +228,30 @@ static gint cmp_descriptor(gconstpointer a, gconstpointer b)
return bt_uuid_cmp(&desc->uuid, uuid);
}

+static struct characteristic *get_characteristic(struct thermometer *t,
+ const gchar *uuid)
+{
+ GSList *l;
+
+ l = g_slist_find_custom(t->chars, uuid, cmp_char_uuid);
+ if (l == NULL)
+ return NULL;
+
+ return l->data;
+}
+
+static struct descriptor *get_descriptor(struct characteristic *ch,
+ const bt_uuid_t *uuid)
+{
+ GSList *l;
+
+ l = g_slist_find_custom(ch->desc, uuid, cmp_descriptor);
+ if (l == NULL)
+ return NULL;
+
+ return l->data;
+}
+
static void change_property(struct thermometer *t, const gchar *name,
gpointer value) {
if (g_strcmp0(name, "Intermediate") == 0) {
@@ -531,36 +560,87 @@ static DBusMessage *get_properties(DBusConnection *conn, DBusMessage *msg,
return reply;
}

-static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
- void *data)
+static void write_interval_cb (guint8 status, const guint8 *pdu, guint16 len,
+ gpointer user_data)
{
- /* TODO: */
- return g_dbus_create_error(msg, ERROR_INTERFACE ".ThermometerError",
- "Function not implemented.");
+ struct tmp_interval_data *data = user_data;
+
+ if (status != 0) {
+ error("Interval Write Request failed %s",
+ att_ecode2str(status));
+ goto done;
+ }
+
+ if (!dec_write_resp(pdu, len)) {
+ error("Interval Write Request: protocol error");
+ goto done;
+ }
+
+ change_property(data->thermometer, "Interval", &data->interval);
+
+done:
+ g_free(user_data);
}

-static struct characteristic *get_characteristic(struct thermometer *t,
- const gchar *uuid)
+static DBusMessage *write_attr_interval(struct thermometer *t, DBusMessage *msg,
+ guint16 value)
{
- GSList *l;
+ struct tmp_interval_data *data;
+ struct characteristic *ch;
+ guint8 atval[2];

- l = g_slist_find_custom(t->chars, uuid, cmp_char_uuid);
- if (l == NULL)
- return NULL;
+ ch = get_characteristic(t, MEASUREMENT_INTERVAL_UUID);
+ if (ch == NULL)
+ return btd_error_not_available(msg);

- return l->data;
+ if (value < t->min || value > t->max)
+ return btd_error_invalid_args(msg);
+
+ att_put_u16(value, &atval[0]);
+
+ data = g_new0(struct tmp_interval_data, 1);
+ data->thermometer = t;
+ data->interval = value;
+ gatt_write_char(t->attrib, ch->attr.value_handle, atval, 2,
+ write_interval_cb, data);
+
+ return dbus_message_new_method_return(msg);
}

-static struct descriptor *get_descriptor(struct characteristic *ch,
- const bt_uuid_t *uuid)
+static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
+ void *data)
{
- GSList *l;
+ struct thermometer *t = data;
+ const char *property;
+ DBusMessageIter iter;
+ DBusMessageIter sub;
+ guint16 value;

- l = g_slist_find_custom(ch->desc, uuid, cmp_descriptor);
- if (l == NULL)
- return NULL;
+ if (!dbus_message_iter_init(msg, &iter))
+ return btd_error_invalid_args(msg);

- return l->data;
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&iter, &property);
+ if (g_strcmp0("Interval", property) != 0)
+ return btd_error_invalid_args(msg);
+
+ if (!t->has_interval)
+ return btd_error_not_available(msg);
+
+ dbus_message_iter_next(&iter);
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_recurse(&iter, &sub);
+
+ if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
+ return btd_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&sub, &value);
+
+ return write_attr_interval(t, msg, value);
}

static void measurement_cb(guint8 status, const guint8 *pdu,
--
1.7.8


2011-12-09 18:19:02

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 4/6] thermometer.c: Fix possible null pointer deference

This patch check the GATT server is connected before doing any
GATT transaction. If it isn't, we abort the operation to avoid the
NULL pointer deference problem.
---
thermometer/thermometer.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index da793a6..ad1569b 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -598,6 +598,9 @@ static DBusMessage *write_attr_interval(struct thermometer *t, DBusMessage *msg,
struct characteristic *ch;
guint8 atval[2];

+ if (t->attrib == NULL)
+ return btd_error_not_connected(msg);
+
ch = get_characteristic(t, MEASUREMENT_INTERVAL_UUID);
if (ch == NULL)
return btd_error_not_available(msg);
@@ -671,6 +674,9 @@ static void enable_final_measurement(struct thermometer *t)
uint8_t atval[2];
gchar *msg;

+ if (t->attrib == NULL)
+ return;
+
ch = get_characteristic(t, TEMPERATURE_MEASUREMENT_UUID);
if (ch == NULL) {
DBG("Temperature measurement characteristic not found");
@@ -698,6 +704,9 @@ static void enable_intermediate_measurement(struct thermometer *t)
uint8_t atval[2];
gchar *msg;

+ if (t->attrib == NULL)
+ return;
+
ch = get_characteristic(t, INTERMEDIATE_TEMPERATURE_UUID);
if (ch == NULL) {
DBG("Intermediate measurement characteristic not found");
@@ -725,6 +734,9 @@ static void disable_final_measurement(struct thermometer *t)
uint8_t atval[2];
gchar *msg;

+ if (t->attrib == NULL)
+ return;
+
ch = get_characteristic(t, TEMPERATURE_MEASUREMENT_UUID);
if (ch == NULL) {
DBG("Temperature measurement characteristic not found");
@@ -752,6 +764,9 @@ static void disable_intermediate_measurement(struct thermometer *t)
uint8_t atval[2];
gchar *msg;

+ if (t->attrib == NULL)
+ return;
+
ch = get_characteristic(t, INTERMEDIATE_TEMPERATURE_UUID);
if (ch == NULL) {
DBG("Intermediate measurement characteristic not found");
--
1.7.8


2011-12-09 18:19:03

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 5/6] thermometer.c: Fix bad read operation when time stamp is not provided.

Time stamp value is an optional field provided in the measure, so next
value in the array will start in a different index depending if the
time stamp was provided or not. This patch check this case and update
the index to a proper value in the byte array before reading the
temperature type value.
---
thermometer/thermometer.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index ad1569b..b112f04 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -1072,12 +1072,18 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
m.suptime = FALSE;

if (flags & TEMP_TYPE) {
- if (len < 16) {
+ uint8_t index;
+
+ if (m.suptime && len >= 16)
+ index = 15;
+ else if (!m.suptime && len >= 9)
+ index = 9;
+ else {
DBG("Can't get temperature type");
return;
}

- type = temptype2str(pdu[15]);
+ type = temptype2str(pdu[index]);
} else if (t->has_type)
type = temptype2str(t->type);
else {
--
1.7.8


2011-12-09 18:19:04

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 6/6] thermometer.c: Use system types instead of GLIB ones.

---
thermometer/thermometer.c | 78 ++++++++++++++++++++++----------------------
1 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index b112f04..d494bd5 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -66,10 +66,10 @@ struct thermometer {
GSList *fwatchers; /* Final measurements */
GSList *iwatchers; /* Intermediate measurements */
gboolean intermediate;
- guint8 type;
- guint16 interval;
- guint16 max;
- guint16 min;
+ uint8_t type;
+ uint16_t interval;
+ uint16_t max;
+ uint16_t min;
gboolean has_type;
gboolean has_interval;
};
@@ -89,23 +89,23 @@ struct descriptor {
struct watcher {
struct thermometer *t;
guint id;
- gchar *srv;
- gchar *path;
+ char *srv;
+ char *path;
};

struct measurement {
- gint16 exp;
- gint32 mant;
- guint64 time;
+ int16_t exp;
+ int32_t mant;
+ uint64_t time;
gboolean suptime;
- gchar *unit;
- gchar *type;
- gchar *value;
+ char *unit;
+ char *type;
+ char *value;
};

struct tmp_interval_data {
struct thermometer *thermometer;
- guint16 interval;
+ uint16_t interval;
};

static GSList *thermometers = NULL;
@@ -207,7 +207,7 @@ static gint cmp_watcher(gconstpointer a, gconstpointer b)
static gint cmp_char_uuid(gconstpointer a, gconstpointer b)
{
const struct characteristic *ch = a;
- const gchar *uuid = b;
+ const char *uuid = b;

return g_strcmp0(ch->attr.uuid, uuid);
}
@@ -229,7 +229,7 @@ static gint cmp_descriptor(gconstpointer a, gconstpointer b)
}

static struct characteristic *get_characteristic(struct thermometer *t,
- const gchar *uuid)
+ const char *uuid)
{
GSList *l;

@@ -252,7 +252,7 @@ static struct descriptor *get_descriptor(struct characteristic *ch,
return l->data;
}

-static void change_property(struct thermometer *t, const gchar *name,
+static void change_property(struct thermometer *t, const char *name,
gpointer value) {
if (g_strcmp0(name, "Intermediate") == 0) {
gboolean *intermediate = value;
@@ -387,7 +387,7 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
{
struct characteristic *ch = user_data;
struct att_data_list *list;
- guint8 format;
+ uint8_t format;
int i;

if (status != 0) {
@@ -592,11 +592,11 @@ done:
}

static DBusMessage *write_attr_interval(struct thermometer *t, DBusMessage *msg,
- guint16 value)
+ uint16_t value)
{
struct tmp_interval_data *data;
struct characteristic *ch;
- guint8 atval[2];
+ uint8_t atval[2];

if (t->attrib == NULL)
return btd_error_not_connected(msg);
@@ -626,7 +626,7 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
const char *property;
DBusMessageIter iter;
DBusMessageIter sub;
- guint16 value;
+ uint16_t value;

if (!dbus_message_iter_init(msg, &iter))
return btd_error_invalid_args(msg);
@@ -658,7 +658,7 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
static void measurement_cb(guint8 status, const guint8 *pdu,
guint16 len, gpointer user_data)
{
- gchar *msg = user_data;
+ char *msg = user_data;

if (status != 0)
error("%s failed", msg);
@@ -672,7 +672,7 @@ static void enable_final_measurement(struct thermometer *t)
struct descriptor *desc;
bt_uuid_t btuuid;
uint8_t atval[2];
- gchar *msg;
+ char *msg;

if (t->attrib == NULL)
return;
@@ -702,7 +702,7 @@ static void enable_intermediate_measurement(struct thermometer *t)
struct descriptor *desc;
bt_uuid_t btuuid;
uint8_t atval[2];
- gchar *msg;
+ char *msg;

if (t->attrib == NULL)
return;
@@ -732,7 +732,7 @@ static void disable_final_measurement(struct thermometer *t)
struct descriptor *desc;
bt_uuid_t btuuid;
uint8_t atval[2];
- gchar *msg;
+ char *msg;

if (t->attrib == NULL)
return;
@@ -762,7 +762,7 @@ static void disable_intermediate_measurement(struct thermometer *t)
struct descriptor *desc;
bt_uuid_t btuuid;
uint8_t atval[2];
- gchar *msg;
+ char *msg;

if (t->attrib == NULL)
return;
@@ -813,8 +813,8 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
disable_final_measurement(t);
}

-static struct watcher *find_watcher(GSList *list, const gchar *sender,
- const gchar *path)
+static struct watcher *find_watcher(GSList *list, const char *sender,
+ const char *path)
{
struct watcher *match;
GSList *l;
@@ -835,10 +835,10 @@ static struct watcher *find_watcher(GSList *list, const gchar *sender,
static DBusMessage *register_watcher(DBusConnection *conn, DBusMessage *msg,
void *data)
{
- const gchar *sender = dbus_message_get_sender(msg);
+ const char *sender = dbus_message_get_sender(msg);
struct thermometer *t = data;
struct watcher *watcher;
- gchar *path;
+ char *path;

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
@@ -868,10 +868,10 @@ static DBusMessage *register_watcher(DBusConnection *conn, DBusMessage *msg,
static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
void *data)
{
- const gchar *sender = dbus_message_get_sender(msg);
+ const char *sender = dbus_message_get_sender(msg);
struct thermometer *t = data;
struct watcher *watcher;
- gchar *path;
+ char *path;

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
@@ -897,10 +897,10 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
static DBusMessage *enable_intermediate(DBusConnection *conn, DBusMessage *msg,
void *data)
{
- const gchar *sender = dbus_message_get_sender(msg);
+ const char *sender = dbus_message_get_sender(msg);
struct thermometer *t = data;
struct watcher *watcher;
- gchar *path;
+ char *path;

if (!t->intermediate)
return btd_error_not_supported(msg);
@@ -929,10 +929,10 @@ static DBusMessage *enable_intermediate(DBusConnection *conn, DBusMessage *msg,
static DBusMessage *disable_intermediate(DBusConnection *conn, DBusMessage *msg,
void *data)
{
- const gchar *sender = dbus_message_get_sender(msg);
+ const char *sender = dbus_message_get_sender(msg);
struct thermometer *t = data;
struct watcher *watcher;
- gchar *path;
+ char *path;

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
@@ -1019,7 +1019,7 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
uint16_t len, gboolean final)
{
struct measurement m;
- const gchar *type;
+ const char *type;
uint8_t flags;
uint32_t raw;

@@ -1041,7 +1041,7 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,

raw = att_get_u32(&pdu[4]);
m.mant = raw & 0x00FFFFFF;
- m.exp = ((gint32) raw) >> 24;
+ m.exp = ((int32_t) raw) >> 24;

if (m.mant & 0x00800000) {
/* convert to C2 negative value */
@@ -1066,7 +1066,7 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
ts.tm_isdst = -1;

time = mktime(&ts);
- m.time = (guint64) time;
+ m.time = (uint64_t) time;
m.suptime = TRUE;
} else
m.suptime = FALSE;
@@ -1104,7 +1104,7 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
static void proc_measurement_interval(struct thermometer *t, const uint8_t *pdu,
uint16_t len)
{
- guint16 interval;
+ uint16_t interval;

if (len < 5) {
DBG("Measurement interval value is not provided");
--
1.7.8


2011-12-09 18:19:01

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 3/6] thermometer.c: Process measurement interval indications

This patch emits PropertyChange signal whenever a new value is
set in the measurement interval characteristic.
---
thermometer/thermometer.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index d175800..da793a6 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -1083,7 +1083,16 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
static void proc_measurement_interval(struct thermometer *t, const uint8_t *pdu,
uint16_t len)
{
- DBG("TODO: Process measurements interval indication");
+ guint16 interval;
+
+ if (len < 5) {
+ DBG("Measurement interval value is not provided");
+ return;
+ }
+
+ interval = att_get_u16(&pdu[3]);
+
+ change_property(t, "Interval", &interval);
}

static void ind_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
--
1.7.8


2011-12-09 18:19:00

by Santiago Carot

[permalink] [raw]
Subject: [PATCH 2/6] thermometer.c: Confiure C.C.C descriptor during the thermometer configuration

This patch enables notification/indication in GATT server if there
are any watcher regitered for measurements when the thermometer is
configured.
---
thermometer/thermometer.c | 25 +++++++++++++++++--------
1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 2a0fb0f..d175800 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -340,21 +340,30 @@ static void process_thermometer_desc(struct descriptor *desc)
bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);

if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
+ uint8_t atval[2];
+ uint16_t val;
+
if (g_strcmp0(ch->attr.uuid,
TEMPERATURE_MEASUREMENT_UUID) == 0) {
- /* TODO: Check if we have to enable it */
- DBG("C.C.C in Temperature Measurement");
+ if (g_slist_length(ch->t->fwatchers) == 0)
+ return;
+
+ val = ATT_CLIENT_CHAR_CONF_INDICATION;
} else if (g_strcmp0(ch->attr.uuid,
INTERMEDIATE_TEMPERATURE_UUID) == 0) {
- /* TODO: Check if we have to enable it */
- DBG("C.C.C in Intermediate Temperature");
+ if (g_slist_length(ch->t->iwatchers) == 0)
+ return;
+
+ val = ATT_CLIENT_CHAR_CONF_NOTIFICATION;
} else if (g_strcmp0(ch->attr.uuid,
- MEASUREMENT_INTERVAL_UUID) == 0) {
- /* TODO: Enable indications */
- DBG("C.C.C in Measurement Interval");
- } else
+ MEASUREMENT_INTERVAL_UUID) == 0)
+ val = ATT_CLIENT_CHAR_CONF_INDICATION;
+ else
goto done;

+ att_put_u16(val, atval);
+ gatt_write_char(ch->t->attrib, desc->handle, atval, 2,
+ NULL, NULL);
return;
}

--
1.7.8