I deleted most of the code related to finding include services. This was only used
in situation when HID service as primary service was not found. However according to
SPEC HID device shall implement HID service as primary. Further HOG is called
from hidhost's hog_conn_cb when HID service was found and connection was established using LE.
Now searching for include services is performed because TC_HGDR_RH_BV_01_I requires that.
Finding battery services is performed like before, during primary services discovery.
Compared to v3 I used queue instead of GSList for bas.
Add a queue of battery services because according to HOGP SPEC there could be more
than one battery services like in TC_HGDC_HH_BV_14_I where there is only one HID service,
and two battery services. So the first found bas was overwritten by the second one and
after reconnection only one bas was attached.
For test purposes requiring read of battery level read_value_cb was added.
Mariusz Skamra (3):
android/bas: Read battery level characteristic
android/hog: Fix find included battery services
android/pts: Update test results for HOGP
android/bas.c | 13 ++++++--
android/hog.c | 84 +++++++++++++++++-----------------------------------
android/pts-hogp.txt | 6 ++--
3 files changed, 41 insertions(+), 62 deletions(-)
--
1.9.1
Hi Mariusz,
On Wednesday 25 of February 2015 13:22:08 Mariusz Skamra wrote:
> I deleted most of the code related to finding include services. This was only used
> in situation when HID service as primary service was not found. However according to
> SPEC HID device shall implement HID service as primary. Further HOG is called
> from hidhost's hog_conn_cb when HID service was found and connection was established using LE.
> Now searching for include services is performed because TC_HGDR_RH_BV_01_I requires that.
> Finding battery services is performed like before, during primary services discovery.
> Compared to v3 I used queue instead of GSList for bas.
>
> Add a queue of battery services because according to HOGP SPEC there could be more
> than one battery services like in TC_HGDC_HH_BV_14_I where there is only one HID service,
> and two battery services. So the first found bas was overwritten by the second one and
> after reconnection only one bas was attached.
>
> For test purposes requiring read of battery level read_value_cb was added.
>
> Mariusz Skamra (3):
> android/bas: Read battery level characteristic
> android/hog: Fix find included battery services
> android/pts: Update test results for HOGP
>
> android/bas.c | 13 ++++++--
> android/hog.c | 84 +++++++++++++++++-----------------------------------
> android/pts-hogp.txt | 6 ++--
> 3 files changed, 41 insertions(+), 62 deletions(-)
>
All patches applied (after fixing some minor style issues), thanks.
--
Best regards,
Szymon Janc
---
android/pts-hogp.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/android/pts-hogp.txt b/android/pts-hogp.txt
index 488d4dd..827e9b4 100644
--- a/android/pts-hogp.txt
+++ b/android/pts-hogp.txt
@@ -1,7 +1,7 @@
PTS test results for HoG
PTS version: 6.0
-Tested: 16-February-2015
+Tested: 24-February-2015
Android version: 5.0
Results:
@@ -18,7 +18,7 @@ TC_HGDS_HH_BV_02_I PASS
TC_HGDS_HH_BV_03_I PASS
TC_HGDS_HD_BV_01_I N/A
TC_HGDS_HD_BV_02_I N/A
-TC_HGDR_RH_BV_01_I INC JIRA #BA-340
+TC_HGDR_RH_BV_01_I PASS
TC_HGDC_RH_BV_01_I PASS
TC_HGDC_RH_BV_02_I PASS
TC_HGDC_RH_BV_03_I PASS
@@ -93,7 +93,7 @@ TC_HGCF_BH_BV_03_I N/A
TC_HGCF_BH_BV_04_I N/A
TC_HGCF_BH_BV_05_I N/A
TC_HGCF_BH_BV_06_I N/A
-TC_HGNF_RH_BV_01_I PASS
+TC_HGNF_RH_BV_01_I PASS PTS issue #12878
TC_HGNF_RH_BI_01_I PASS
TC_HGNF_RH_BI_01_I PASS
TC_HGNF_BH_BV_02_I N/A
--
1.9.1
I deleted most of the code related to finding include services. This method was used
only in situation when HID service as primary service was not found. However according to
SPEC HID device shall implement HID service as primary. Further HOG is called
from hidhost's hog_conn_cb when HID service was found and connection was established using LE.
Now searching for include services is performed because TC_HGDR_RH_BV_01_I requires that.
I also added a queue of battery services because according to HOGP SPEC there could be more
than one battery services like in TC_HGDC_HH_BV_14_I where there is only one HID service,
and two battery services. So the first found bas was overwritten by the second one and
after reconnection only one bas was attached.
---
android/hog.c | 84 +++++++++++++++++++----------------------------------------
1 file changed, 27 insertions(+), 57 deletions(-)
diff --git a/android/hog.c b/android/hog.c
index 7f441f1..c844893 100644
--- a/android/hog.c
+++ b/android/hog.c
@@ -99,7 +99,7 @@ struct bt_hog {
uint16_t setrep_id;
struct bt_scpp *scpp;
struct bt_dis *dis;
- struct bt_bas *bas;
+ struct queue *bas;
GSList *instances;
struct queue *gatt_op;
};
@@ -1157,11 +1157,11 @@ static void hog_free(void *data)
bt_hog_detach(hog);
+ queue_destroy(hog->bas, (void *) bt_bas_unref);
g_slist_free_full(hog->instances, hog_free);
bt_scpp_unref(hog->scpp);
bt_dis_unref(hog->dis);
- bt_bas_unref(hog->bas);
bt_uhid_unref(hog->uhid);
g_slist_free_full(hog->reports, report_free);
g_free(hog->name);
@@ -1185,10 +1185,18 @@ struct bt_hog *bt_hog_new(const char *name, uint16_t vendor, uint16_t product,
return NULL;
}
+ hog->bas = queue_new();
+ if (!hog->bas) {
+ queue_destroy(hog->gatt_op, NULL);
+ hog_free(hog);
+ return NULL;
+ }
+
hog->uhid = bt_uhid_new_default();
if (!hog->uhid) {
hog_free(hog);
queue_destroy(hog->gatt_op, NULL);
+ queue_destroy(hog->bas, NULL);
return NULL;
}
@@ -1227,54 +1235,23 @@ void bt_hog_unref(struct bt_hog *hog)
static void find_included_cb(uint8_t status, GSList *services, void *user_data)
{
struct gatt_request *req = user_data;
- struct bt_hog *hog = req->user_data;
- struct gatt_included *include;
GSList *l;
DBG("");
destroy_gatt_req(req);
- if (hog->primary)
- return;
-
if (status) {
const char *str = att_ecode2str(status);
DBG("Find included failed: %s", str);
return;
}
- if (!services) {
- DBG("No included service found");
- return;
- }
-
for (l = services; l; l = l->next) {
- include = l->data;
-
- if (strcmp(include->uuid, HOG_UUID) == 0)
- break;
- }
-
- if (!l) {
- for (l = services; l; l = l->next) {
- include = l->data;
-
- find_included(hog, hog->attrib,
- include->range.start,
- include->range.end, find_included_cb,
- hog);
- }
- return;
+ struct gatt_included *include = l->data;
+ DBG("included: handle %x, uuid %s",
+ include->handle, include->uuid);
}
-
- hog->primary = g_new0(struct gatt_primary, 1);
- memcpy(hog->primary->uuid, include->uuid, sizeof(include->uuid));
- memcpy(&hog->primary->range, &include->range, sizeof(include->range));
-
- discover_char(hog, hog->attrib, hog->primary->range.start,
- hog->primary->range.end, NULL,
- char_discovered_cb, hog);
}
static void hog_attach_scpp(struct bt_hog *hog, struct gatt_primary *primary)
@@ -1315,14 +1292,14 @@ static void hog_attach_dis(struct bt_hog *hog, struct gatt_primary *primary)
static void hog_attach_bas(struct bt_hog *hog, struct gatt_primary *primary)
{
- if (hog->bas) {
- bt_bas_attach(hog->bas, hog->attrib);
+ struct bt_bas *instance;
+
+ instance = bt_bas_new(primary);
+ if (!instance)
return;
- }
- hog->bas = bt_bas_new(primary);
- if (hog->bas)
- bt_bas_attach(hog->bas, hog->attrib);
+ bt_bas_attach(instance, hog->attrib);
+ queue_push_head(hog->bas, instance);
}
static void hog_attach_hog(struct bt_hog *hog, struct gatt_primary *primary)
@@ -1334,6 +1311,8 @@ static void hog_attach_hog(struct bt_hog *hog, struct gatt_primary *primary)
discover_char(hog, hog->attrib, primary->range.start,
primary->range.end, NULL,
char_discovered_cb, hog);
+ find_included(hog, hog->attrib, primary->range.start,
+ primary->range.end, find_included_cb, hog);
return;
}
@@ -1342,6 +1321,9 @@ static void hog_attach_hog(struct bt_hog *hog, struct gatt_primary *primary)
if (!instance)
return;
+ find_included(instance, hog->attrib, primary->range.start,
+ primary->range.end, find_included_cb, instance);
+
bt_hog_attach(instance, hog->attrib);
hog->instances = g_slist_append(hog->instances, instance);
}
@@ -1389,16 +1371,6 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
if (strcmp(primary->uuid, HOG_UUID) == 0)
hog_attach_hog(hog, primary);
}
-
- if (hog->primary)
- return;
-
- for (l = services; l; l = l->next) {
- primary = l->data;
-
- find_included(hog, hog->attrib, primary->range.start,
- primary->range.end, find_included_cb, hog);
- }
}
bool bt_hog_attach(struct bt_hog *hog, void *gatt)
@@ -1422,8 +1394,7 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
if (hog->dis)
bt_dis_attach(hog->dis, gatt);
- if (hog->bas)
- bt_bas_attach(hog->bas, gatt);
+ queue_foreach(hog->bas, (void *) bt_bas_attach, gatt);
for (l = hog->instances; l; l = l->next) {
struct bt_hog *instance = l->data;
@@ -1457,6 +1428,8 @@ void bt_hog_detach(struct bt_hog *hog)
if (!hog->attrib)
return;
+ queue_foreach(hog->bas, (void *) bt_bas_detach, NULL);
+
for (l = hog->instances; l; l = l->next) {
struct bt_hog *instance = l->data;
@@ -1478,9 +1451,6 @@ void bt_hog_detach(struct bt_hog *hog)
if (hog->dis)
bt_dis_detach(hog->dis);
- if (hog->bas)
- bt_bas_detach(hog->bas);
-
queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL);
g_attrib_unref(hog->attrib);
hog->attrib = NULL;
--
1.9.1
For TC_HGRF_HH_BV_10_I test purposes, apart from registration for
notification, read request should be sent to get battery level.
---
android/bas.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/android/bas.c b/android/bas.c
index 3d4bee0..b3fb41f 100644
--- a/android/bas.c
+++ b/android/bas.c
@@ -44,6 +44,7 @@
#include "android/bas.h"
#define ATT_NOTIFICATION_HEADER_SIZE 3
+#define ATT_READ_RESPONSE_HEADER_SIZE 1
struct bt_bas {
int ref_count;
@@ -226,11 +227,17 @@ static void discover_desc(struct bt_bas *bas, GAttrib *attrib,
free(req);
}
-static void value_cb(const guint8 *pdu, guint16 len, gpointer user_data)
+static void notification_cb(const guint8 *pdu, guint16 len, gpointer user_data)
{
DBG("Battery Level at %u", pdu[ATT_NOTIFICATION_HEADER_SIZE]);
}
+static void read_value_cb(guint8 status, const guint8 *pdu, guint16 len,
+ gpointer user_data)
+{
+ DBG("Battery Level at %u", pdu[ATT_READ_RESPONSE_HEADER_SIZE]);
+}
+
static void ccc_written_cb(guint8 status, const guint8 *pdu,
guint16 plen, gpointer user_data)
{
@@ -248,7 +255,7 @@ static void ccc_written_cb(guint8 status, const guint8 *pdu,
DBG("Battery Level: notification enabled");
bas->id = g_attrib_register(bas->attrib, ATT_OP_HANDLE_NOTIFY,
- bas->handle, value_cb, bas, NULL);
+ bas->handle, notification_cb, bas, NULL);
}
static void write_ccc(struct bt_bas *bas, GAttrib *attrib, uint16_t handle,
@@ -319,6 +326,8 @@ static void bas_discovered_cb(uint8_t status, GSList *chars, void *user_data)
DBG("Battery handle: 0x%04x", bas->handle);
+ read_char(bas, bas->attrib, bas->handle, read_value_cb, bas);
+
start = chr->value_handle + 1;
end = bas->primary->range.end;
--
1.9.1