2015-09-04 18:29:59

by Petri Gynther

[permalink] [raw]
Subject: [PATCH 1/2] hog: add more debugging to HoG device init

Add more debugging to HoG device init, so that it is easier to see
whether the HoG device init completes successfully.
---
profiles/input/hog.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index 4be9fd2..bd35830 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -633,6 +633,8 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
int i, err;
GSList *l;

+ DBG("HoG inspecting report map");
+
if (status != 0) {
error("Report Map read failed: %s", att_ecode2str(status));
return;
@@ -703,6 +705,7 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
bt_uhid_register(hogdev->uhid, UHID_GET_REPORT, get_report, hogdev);

hogdev->uhid_created = TRUE;
+ DBG("HoG created uHID device");

for (l = hogdev->reports; l; l = l->next) {
struct report *r = l->data;
@@ -780,6 +783,8 @@ static void char_discovered_cb(uint8_t status, GSList *chars, void *user_data)
GSList *l;
uint16_t info_handle = 0, proto_mode_handle = 0;

+ DBG("HoG inspecting characteristics");
+
if (status != 0) {
const char *str = att_ecode2str(status);
DBG("Discover all characteristics failed: %s", str);
@@ -816,6 +821,7 @@ static void char_discovered_cb(uint8_t status, GSList *chars, void *user_data)
report);
discover_descriptor(hogdev->attrib, start, end, report);
} else if (bt_uuid_cmp(&uuid, &report_map_uuid) == 0) {
+ DBG("HoG discovering report map");
gatt_read_char(hogdev->attrib, chr->value_handle,
report_map_read_cb, hogdev);
discover_descriptor(hogdev->attrib, start, end, hogdev);
@@ -849,6 +855,7 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
hogdev->attrib = g_attrib_ref(attrib);

if (hogdev->reports == NULL) {
+ DBG("HoG discovering characteristics");
gatt_discover_char(hogdev->attrib, prim->range.start,
prim->range.end, NULL,
char_discovered_cb, hogdev);
--
2.5.0.457.gab17608



2015-09-07 14:16:33

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/2] hog: add more debugging to HoG device init

Hi Petri,

On Fri, Sep 4, 2015 at 9:29 PM, Petri Gynther <[email protected]> wrote:
> Add more debugging to HoG device init, so that it is easier to see
> whether the HoG device init completes successfully.
> ---
> profiles/input/hog.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
> index 4be9fd2..bd35830 100644
> --- a/profiles/input/hog.c
> +++ b/profiles/input/hog.c
> @@ -633,6 +633,8 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
> int i, err;
> GSList *l;
>
> + DBG("HoG inspecting report map");
> +
> if (status != 0) {
> error("Report Map read failed: %s", att_ecode2str(status));
> return;
> @@ -703,6 +705,7 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
> bt_uhid_register(hogdev->uhid, UHID_GET_REPORT, get_report, hogdev);
>
> hogdev->uhid_created = TRUE;
> + DBG("HoG created uHID device");
>
> for (l = hogdev->reports; l; l = l->next) {
> struct report *r = l->data;
> @@ -780,6 +783,8 @@ static void char_discovered_cb(uint8_t status, GSList *chars, void *user_data)
> GSList *l;
> uint16_t info_handle = 0, proto_mode_handle = 0;
>
> + DBG("HoG inspecting characteristics");
> +
> if (status != 0) {
> const char *str = att_ecode2str(status);
> DBG("Discover all characteristics failed: %s", str);
> @@ -816,6 +821,7 @@ static void char_discovered_cb(uint8_t status, GSList *chars, void *user_data)
> report);
> discover_descriptor(hogdev->attrib, start, end, report);
> } else if (bt_uuid_cmp(&uuid, &report_map_uuid) == 0) {
> + DBG("HoG discovering report map");
> gatt_read_char(hogdev->attrib, chr->value_handle,
> report_map_read_cb, hogdev);
> discover_descriptor(hogdev->attrib, start, end, hogdev);
> @@ -849,6 +855,7 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
> hogdev->attrib = g_attrib_ref(attrib);
>
> if (hogdev->reports == NULL) {
> + DBG("HoG discovering characteristics");
> gatt_discover_char(hogdev->attrib, prim->range.start,
> prim->range.end, NULL,
> char_discovered_cb, hogdev);
> --
> 2.5.0.457.gab17608

Applied, thanks.

Note: We had the plan to move to android/hog.c at some point but
things are starting to divert too much between the 2 files, not to
mention we still need to port it to use bt_gatt_client and gatt_db
APIs perhaps you can help getting this done?


--
Luiz Augusto von Dentz

2015-09-04 18:30:00

by Petri Gynther

[permalink] [raw]
Subject: [PATCH 2/2] hog: handle HoG init failures correctly

When attio_connected_cb() is called for a HoG device, BlueZ should
be in one of the two states:

1) hogdev->uhid_created == FALSE && hogdev->reports == NULL
* initial connection to HoG device, or first reconnect after
BlueZ has been restarted
* BlueZ needs to discover all HoG device characteristics
(including report map) and create uHID device for HID input

2) hogdev->uhid_created == TRUE && hogdev->reports != NULL
* second or subsequent reconnect
* all HoG device characteristics (including report map) have
been successfully discovered previously, and uHID device
has been created

However, it is possible that the connection between BlueZ and
HoG device is abruptly terminated amid HoG device characteristics
discovery. Or, HoG report map discovery might intermittently fail.
This can leave BlueZ in inconsistent state such that it knows about
some of the characteristics, but the report map was never received
and uHID device not created, i.e.:
hogdev->uhid_created == FALSE && hogdev->reports != NULL

attio_connected_cb() needs to detect this condition, clean up
hogdev->reports, and re-discover HoG device characteristics.
---
profiles/input/hog.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index bd35830..e006add 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -844,6 +844,18 @@ static void char_discovered_cb(uint8_t status, GSList *chars, void *user_data)
hogdev);
}

+static void report_free(void *data)
+{
+ struct report *report = data;
+ struct hog_device *hogdev = report->hogdev;
+
+ if (hogdev->attrib)
+ g_attrib_unregister(hogdev->attrib, report->notifyid);
+
+ g_free(report->decl);
+ g_free(report);
+}
+
static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
{
struct hog_device *hogdev = user_data;
@@ -852,6 +864,12 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)

DBG("HoG connected");

+ if (!hogdev->uhid_created && hogdev->reports) {
+ DBG("HoG init failed previously, preparing for re-init");
+ g_slist_free_full(hogdev->reports, report_free);
+ hogdev->reports = NULL;
+ }
+
hogdev->attrib = g_attrib_ref(attrib);

if (hogdev->reports == NULL) {
@@ -901,18 +919,6 @@ static struct hog_device *hog_new_device(struct btd_device *device,
return hogdev;
}

-static void report_free(void *data)
-{
- struct report *report = data;
- struct hog_device *hogdev = report->hogdev;
-
- if (hogdev->attrib)
- g_attrib_unregister(hogdev->attrib, report->notifyid);
-
- g_free(report->decl);
- g_free(report);
-}
-
static void hog_free_device(struct hog_device *hogdev)
{
btd_device_unref(hogdev->device);
--
2.5.0.457.gab17608