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.
[PATCH 1/6] Implement SetProperty D-Bus method
[PATCH 2/6] Confiure C.C.C descriptor during the thermometer configuration
[PATCH 3/6] Process measurements interval indication
[PATCH 4/6] Fix possible null pointer deference
[PATCH 5/6] Fix access to an invalid memory sector if time stamp is supported
[PATCH 6/6] 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 e37254c..97a8ea0 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;
@@ -388,7 +388,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) {
@@ -593,11 +593,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);
@@ -627,7 +627,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);
@@ -659,7 +659,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);
@@ -673,7 +673,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;
@@ -703,7 +703,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;
@@ -733,7 +733,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;
@@ -763,7 +763,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;
@@ -814,8 +814,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;
@@ -836,10 +836,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))
@@ -869,10 +869,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))
@@ -898,10 +898,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);
@@ -930,10 +930,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))
@@ -1020,7 +1020,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;
@@ -1042,7 +1042,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 */
@@ -1067,7 +1067,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;
@@ -1105,7 +1105,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.7.4
---
thermometer/thermometer.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 1746e60..9ad7e59 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -1084,7 +1084,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.7.4
---
thermometer/thermometer.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 4faf900..e37254c 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -1073,12 +1073,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.7.4
---
thermometer/thermometer.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 9ad7e59..4faf900 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -599,6 +599,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);
@@ -672,6 +675,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");
@@ -699,6 +705,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");
@@ -726,6 +735,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");
@@ -753,6 +765,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.7.4
---
thermometer/thermometer.c | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/thermometer/thermometer.c b/thermometer/thermometer.c
index 2a0fb0f..1746e60 100644
--- a/thermometer/thermometer.c
+++ b/thermometer/thermometer.c
@@ -340,21 +340,31 @@ 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];
+
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;
+
+ atval[0] = 0x02;
+ atval[1] = 0x00;
} 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;
+
+ atval[0] = 0x01;
+ atval[1] = 0x00;
} else if (g_strcmp0(ch->attr.uuid,
MEASUREMENT_INTERVAL_UUID) == 0) {
- /* TODO: Enable indications */
- DBG("C.C.C in Measurement Interval");
+ atval[0] = 0x02;
+ atval[1] = 0x00;
} else
goto done;
+ gatt_write_char(ch->t->attrib, desc->handle, atval, 2,
+ NULL, NULL);
return;
}
--
1.7.7.4
---
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.7.4
Hi Santiago,
On Fri, Dec 09, 2011, Santiago Carot-Nemesio wrote:
> 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.
All patches have been applied. I'd prefer if you use the prefix
"thermometer" instead of "thermometer.c" in the future (I fixed it
manually this time). Also pay attention to your commit message line
lengths. git log indents the message by 4 characters and git shortlog by
6, so to be on the safe side on 80 character wide terminals keep the
lines less than 74 characters (e.g. patch 2/6 had a too long summary
line).
Johan
Hi Johan,
2011/12/7 Johan Hedberg <[email protected]>:
> Hi Santiago,
>
> On Tue, Nov 22, 2011, Santiago Carot-Nemesio wrote:
>> ---
>> ?thermometer/thermometer.c | ?118 +++++++++++++++++++++++++++++++++++++-------
>> ?1 files changed, 99 insertions(+), 19 deletions(-)
>
> Is this patch set still valid? If so, could you resend it with proper
> commit messages. "Implement SetProperty D-Bus method" is much too
> generic. It'd be nice if you could start using the convention we've
> tried to adopt where we add context dependent prefixes to the message
> summaries. In this case "thermometer: Add SetProperty D-Bus method"
> would have already given enough of context to not have to look at the
> patch content to know what it does.
>
Discard this set of patches for now, I'll prepare a new set after
fixing the magic number issue in GATT.
Regards.
Hi Santiago,
On Tue, Nov 22, 2011, Santiago Carot-Nemesio wrote:
> ---
> thermometer/thermometer.c | 118 +++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 99 insertions(+), 19 deletions(-)
Is this patch set still valid? If so, could you resend it with proper
commit messages. "Implement SetProperty D-Bus method" is much too
generic. It'd be nice if you could start using the convention we've
tried to adopt where we add context dependent prefixes to the message
summaries. In this case "thermometer: Add SetProperty D-Bus method"
would have already given enough of context to not have to look at the
patch content to know what it does.
Johan
Hi Santiago,
On Fri, Dec 2, 2011 at 7:52 AM, Santiago Carot <[email protected]> wrote:
> These are GATT flags to enable/disable indications/notifications in
> client characteristic configuration descriptor, I thought about this
> issue, I haven't seen any macros mapping those values in GATT though.
> I was thinking in sending a separate patch to deal with this issue but
> I saw some Claudio's commits to deal with notification/indications so
> I guess he is already working on it, so I decided no to ?interfere and
> wait to see how he is thinking to deal with this stuff.
I think you can create these macros and then begin using it for
thermometer code (feel free to modify generic GATT code to use them as
well).
The commit you are referring to is on one of Claudio's development
branches, right? That commit will be probably discarded (it is
incomplete, and was just an idea), so feel free to send your own
version. Be sure to take into account that these values are bit masks,
and that maybe we should define all the possible macros for them now.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
Hi Johan,
2011/12/2 Johan Hedberg <[email protected]>:
> Hi Santiago,
>
> On Tue, Nov 22, 2011, Santiago Carot-Nemesio wrote:
>> + ? ? ? ? ? ? ? ? ? ? atval[0] = 0x02;
>> + ? ? ? ? ? ? ? ? ? ? atval[1] = 0x00;
>
> What are these magic values? Could we have some defines for them or
> something else more readable?
These are GATT flags to enable/disable indications/notifications in
client characteristic configuration descriptor, I thought about this
issue, I haven't seen any macros mapping those values in GATT though.
I was thinking in sending a separate patch to deal with this issue but
I saw some Claudio's commits to deal with notification/indications so
I guess he is already working on it, so I decided no to interfere and
wait to see how he is thinking to deal with this stuff.
Regards.
Hi Santiago,
On Tue, Nov 22, 2011, Santiago Carot-Nemesio wrote:
> + atval[0] = 0x02;
> + atval[1] = 0x00;
What are these magic values? Could we have some defines for them or
something else more readable?
Johan