[RFC 0/8] HoG Plugin

[RFC 0/8] HoG Plugin

[João Paulo Rechi Vita]

This is a first draft of the HoG plugin. This RFC is mainly to give feedback
to other people working on HoG-relate stuff, like DIS, UHID, etc.

There are quite a few "magic" hardcoded values around the code, which will go
away as soon as we implement the discovery of all characteristics from the
device. It has been tested only with one specific keyboard model and support
for output reports is not implemented yet.

Comments are welcome.

Claudio Takahasi (1):
  HoG: Register HID over GATT device driver

João Paulo Rechi Vita (7):
  HoG: register ATTIO callbacks
  HoG: enable report characteristic notification
  HoG: load primary service handle
  HoG: discover report characteristic
  HoG: add report notification handler
  HoG: print the report map on debug output
  HoG: HID I/O driver

 Makefile.am        |    5 +-
 input/hog_device.c |  324 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 input/hog_device.h |   30 +++++
 input/main.c       |   13 ++
 input/manager.c    |   36 ++++++
 input/manager.h    |    3 +
 6 files changed, 409 insertions(+), 2 deletions(-)
 create mode 100644 input/hog_device.c
 create mode 100644 input/hog_device.h

-- 
1.7.7.6

[RFC 1/8] HoG: Register HID over GATT device driver

[João Paulo Rechi Vita]

From: Claudio Takahasi <claudio.takahasi@openbossa.org>

---
 Makefile.am        |    5 +++--
 input/hog_device.h |   25 +++++++++++++++++++++++++
 input/main.c       |   13 +++++++++++++
 input/manager.c    |   34 ++++++++++++++++++++++++++++++++++
 input/manager.h    |    3 +++
 5 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100644 input/hog_device.h

diff --git a/Makefile.am b/Makefile.am
index ddc28d2..32f98b2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -174,12 +174,13 @@ sap_libsap_a_SOURCES = sap/sap.h sap/sap-dummy.c sap/sap-u8500.c
 endif
 
 if INPUTPLUGIN
-builtin_modules += input
+builtin_modules += input hog
 builtin_sources += input/main.c \
 			input/manager.h input/manager.c \
 			input/server.h input/server.c \
 			input/device.h input/device.c \
-			input/fakehid.c input/fakehid.h
+			input/fakehid.c input/fakehid.h \
+			input/hog_device.h
 endif
 
 if SERIALPLUGIN
diff --git a/input/hog_device.h b/input/hog_device.h
new file mode 100644
index 0000000..a0158ea
--- /dev/null
+++ b/input/hog_device.h
@@ -0,0 +1,25 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012  Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#define HOG_UUID		"00001812-0000-1000-8000-00805f9b34fb"
diff --git a/input/main.c b/input/main.c
index e165ab4..960cf05 100644
--- a/input/main.c
+++ b/input/main.c
@@ -84,3 +84,16 @@ static void input_exit(void)
 
 BLUETOOTH_PLUGIN_DEFINE(input, VERSION,
 			BLUETOOTH_PLUGIN_PRIORITY_DEFAULT, input_init, input_exit)
+
+static int hog_init(void)
+{
+	return hog_manager_init();
+}
+
+static void hog_exit(void)
+{
+	hog_manager_exit();
+}
+
+BLUETOOTH_PLUGIN_DEFINE(hog, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+							hog_init, hog_exit)
diff --git a/input/manager.c b/input/manager.c
index 9bcab10..6025295 100644
--- a/input/manager.c
+++ b/input/manager.c
@@ -39,6 +39,7 @@
 #include "../src/device.h"
 
 #include "device.h"
+#include "hog_device.h"
 #include "server.h"
 #include "manager.h"
 
@@ -194,3 +195,36 @@ void input_manager_exit(void)
 
 	connection = NULL;
 }
+
+static int hog_device_probe(struct btd_device *device, GSList *uuids)
+{
+	const char *path = device_get_path(device);
+
+	DBG("path %s", path);
+
+	return 0;
+}
+
+static void hog_device_remove(struct btd_device *device)
+{
+	const gchar *path = device_get_path(device);
+
+	DBG("path %s", path);
+}
+
+static struct btd_device_driver hog_driver = {
+	.name	= "input-hog",
+	.uuids	= BTD_UUIDS(HOG_UUID),
+	.probe	= hog_device_probe,
+	.remove	= hog_device_remove,
+};
+
+int hog_manager_init(void)
+{
+	return btd_register_device_driver(&hog_driver);
+}
+
+void hog_manager_exit(void)
+{
+	btd_unregister_device_driver(&hog_driver);
+}
diff --git a/input/manager.h b/input/manager.h
index 7b93c5b..468de64 100644
--- a/input/manager.h
+++ b/input/manager.h
@@ -23,3 +23,6 @@
 
 int input_manager_init(DBusConnection *conn, GKeyFile *config);
 void input_manager_exit(void);
+
+int hog_manager_init(void);
+void hog_manager_exit(void);
-- 
1.7.7.6

[RFC 2/8] HoG: register ATTIO callbacks

[João Paulo Rechi Vita]

This way the LE connection is kept up.
---
 Makefile.am        |    2 +-
 input/hog_device.c |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 input/hog_device.h |    5 ++
 input/manager.c    |    4 +-
 4 files changed, 149 insertions(+), 2 deletions(-)
 create mode 100644 input/hog_device.c

diff --git a/Makefile.am b/Makefile.am
index 32f98b2..fb8c8df 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -180,7 +180,7 @@ builtin_sources += input/main.c \
 			input/server.h input/server.c \
 			input/device.h input/device.c \
 			input/fakehid.c input/fakehid.h \
-			input/hog_device.h
+			input/hog_device.c input/hog_device.h
 endif
 
 if SERIALPLUGIN
diff --git a/input/hog_device.c b/input/hog_device.c
new file mode 100644
index 0000000..2228a76
--- /dev/null
+++ b/input/hog_device.c
@@ -0,0 +1,140 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012  Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <errno.h>
+#include <unistd.h>
+
+#include <bluetooth/bluetooth.h>
+
+#include <glib.h>
+
+#include "log.h"
+
+#include "../src/adapter.h"
+#include "../src/device.h"
+
+#include "hog_device.h"
+
+#include "gattrib.h"
+#include "attio.h"
+
+struct hog_device {
+	char			*path;
+	struct btd_device	*device;
+	GAttrib			*attrib;
+	guint			attioid;
+};
+
+static GSList *devices = NULL;
+
+static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
+{
+	struct hog_device *hogdev = user_data;
+
+	hogdev->attrib = g_attrib_ref(attrib);
+}
+
+static void attio_disconnected_cb(gpointer user_data)
+{
+	struct hog_device *hogdev = user_data;
+
+	g_attrib_unref(hogdev->attrib);
+	hogdev->attrib = NULL;
+}
+
+static struct hog_device *find_device_by_path(GSList *list, const char *path)
+{
+	for (; list; list = list->next) {
+		struct hog_device *hogdev = list->data;
+
+		if (!strcmp(hogdev->path, path))
+			return hogdev;
+	}
+
+	return NULL;
+}
+
+static struct hog_device *hog_device_new(struct btd_device *device,
+							const char *path)
+{
+	struct hog_device *hogdev;
+
+	hogdev = g_new0(struct hog_device, 1);
+	if (!hogdev)
+		return NULL;
+
+	hogdev->path = g_strdup(path);
+	hogdev->device = btd_device_ref(device);
+
+	return hogdev;
+}
+
+int hog_device_register(struct btd_device *device, const char *path)
+{
+	struct hog_device *hogdev;
+
+	hogdev = find_device_by_path(devices, path);
+	if (hogdev)
+		return -EALREADY;
+
+	hogdev = hog_device_new(device, path);
+	if (!hogdev)
+		return -ENOMEM;
+
+	hogdev->attioid = btd_device_add_attio_callback(device,
+							attio_connected_cb,
+							attio_disconnected_cb,
+							hogdev);
+
+	devices = g_slist_append(devices, hogdev);
+
+	return 0;
+}
+
+static void hog_device_free(struct hog_device *hogdev)
+{
+	btd_device_unref(hogdev->device);
+	g_free(hogdev->path);
+	g_free(hogdev);
+}
+
+int hog_device_unregister(const char *path)
+{
+	struct hog_device *hogdev;
+
+	hogdev = find_device_by_path(devices, path);
+	if (hogdev == NULL)
+		return -EINVAL;
+
+	btd_device_remove_attio_callback(hogdev->device, hogdev->attioid);
+	devices = g_slist_remove(devices, hogdev);
+	hog_device_free(hogdev);
+
+	return 0;
+}
diff --git a/input/hog_device.h b/input/hog_device.h
index a0158ea..585e3d5 100644
--- a/input/hog_device.h
+++ b/input/hog_device.h
@@ -23,3 +23,8 @@
  */
 
 #define HOG_UUID		"00001812-0000-1000-8000-00805f9b34fb"
+
+struct hog_device;
+
+int hog_device_register(struct btd_device *device, const char *path);
+int hog_device_unregister(const char *path);
diff --git a/input/manager.c b/input/manager.c
index 6025295..a38edd4 100644
--- a/input/manager.c
+++ b/input/manager.c
@@ -202,7 +202,7 @@ static int hog_device_probe(struct btd_device *device, GSList *uuids)
 
 	DBG("path %s", path);
 
-	return 0;
+	return hog_device_register(device, path);
 }
 
 static void hog_device_remove(struct btd_device *device)
@@ -210,6 +210,8 @@ static void hog_device_remove(struct btd_device *device)
 	const gchar *path = device_get_path(device);
 
 	DBG("path %s", path);
+
+	hog_device_unregister(path);
 }
 
 static struct btd_device_driver hog_driver = {
-- 
1.7.7.6

[RFC 3/8] HoG: enable report characteristic notification

[João Paulo Rechi Vita]

From: João Paulo Rechi Vita <jprvita@gmail.com>

---
 input/hog_device.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/input/hog_device.c b/input/hog_device.c
index 2228a76..80df978 100644
--- a/input/hog_device.c
+++ b/input/hog_device.c
@@ -31,6 +31,7 @@
 #include <unistd.h>
 
 #include <bluetooth/bluetooth.h>
+#include <bluetooth/uuid.h>
 
 #include <glib.h>
 
@@ -41,8 +42,10 @@
 
 #include "hog_device.h"
 
+#include "att.h"
 #include "gattrib.h"
 #include "attio.h"
+#include "gatt.h"
 
 struct hog_device {
 	char			*path;
@@ -53,11 +56,27 @@ struct hog_device {
 
 static GSList *devices = NULL;
 
+static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
+					guint16 plen, gpointer user_data)
+{
+	if (status != 0) {
+		error("Write report characteristic descriptor failed: %s",
+							att_ecode2str(status));
+		return;
+	}
+
+	DBG("Report characteristic descriptor written: notification enabled");
+}
+
 static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 {
 	struct hog_device *hogdev = user_data;
+	uint8_t value[] = { 0x01, 0x00 };
 
 	hogdev->attrib = g_attrib_ref(attrib);
+
+	gatt_write_char(hogdev->attrib, 0x000c, value, 2,
+						report_ccc_written_cb, hogdev);
 }
 
 static void attio_disconnected_cb(gpointer user_data)
-- 
1.7.7.6

[RFC 4/8] HoG: load primary service handle

[João Paulo Rechi Vita]

---
 input/hog_device.c |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/input/hog_device.c b/input/hog_device.c
index 80df978..5ec2d09 100644
--- a/input/hog_device.c
+++ b/input/hog_device.c
@@ -52,6 +52,7 @@ struct hog_device {
 	struct btd_device	*device;
 	GAttrib			*attrib;
 	guint			attioid;
+	struct gatt_primary	*hog_primary;
 };
 
 static GSList *devices = NULL;
@@ -114,17 +115,45 @@ static struct hog_device *hog_device_new(struct btd_device *device,
 	return hogdev;
 }
 
+static gint primary_uuid_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct gatt_primary *prim = a;
+	const char *uuid = b;
+
+	return g_strcmp0(prim->uuid, uuid);
+}
+
+static struct gatt_primary *load_hog_primary(struct btd_device *device)
+{
+	GSList *primaries, *l;
+
+	primaries = btd_device_get_primaries(device);
+
+	l = g_slist_find_custom(primaries, HOG_UUID, primary_uuid_cmp);
+
+	return (l ? l->data : NULL);
+}
+
 int hog_device_register(struct btd_device *device, const char *path)
 {
 	struct hog_device *hogdev;
+	struct gatt_primary *prim;
 
 	hogdev = find_device_by_path(devices, path);
 	if (hogdev)
 		return -EALREADY;
 
+	prim = load_hog_primary(device);
+	if (!prim)
+		return -EINVAL;
+
 	hogdev = hog_device_new(device, path);
-	if (!hogdev)
+	if (!hogdev) {
+		g_free(prim);
 		return -ENOMEM;
+	}
+
+	hogdev->hog_primary = prim;
 
 	hogdev->attioid = btd_device_add_attio_callback(device,
 							attio_connected_cb,
@@ -140,6 +169,7 @@ static void hog_device_free(struct hog_device *hogdev)
 {
 	btd_device_unref(hogdev->device);
 	g_free(hogdev->path);
+	g_free(hogdev->hog_primary);
 	g_free(hogdev);
 }
 
-- 
1.7.7.6

[RFC 5/8] HoG: discover report characteristic

[João Paulo Rechi Vita]

---
 input/hog_device.c |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/input/hog_device.c b/input/hog_device.c
index 5ec2d09..b06f898 100644
--- a/input/hog_device.c
+++ b/input/hog_device.c
@@ -47,12 +47,15 @@
 #include "attio.h"
 #include "gatt.h"
 
+#define HOG_REPORT_UUID		0x2A4D
+
 struct hog_device {
 	char			*path;
 	struct btd_device	*device;
 	GAttrib			*attrib;
 	guint			attioid;
 	struct gatt_primary	*hog_primary;
+	struct gatt_char	*report;
 };
 
 static GSList *devices = NULL;
@@ -69,15 +72,48 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
 	DBG("Report characteristic descriptor written: notification enabled");
 }
 
+static void char_discovered_cb(GSList *chars, guint8 status, gpointer user_data)
+{
+	struct hog_device *hogdev = user_data;
+	bt_uuid_t report_uuid;
+	GSList *l;
+
+	if (status != 0) {
+		const char *str = att_ecode2str(status);
+		DBG("Discover all characteristics failed: %s", str);
+		return;
+	}
+
+	bt_uuid16_create(&report_uuid, HOG_REPORT_UUID);
+
+	for (l = chars; l; l = g_slist_next(l)) {
+		struct gatt_char *chr = l->data;
+		bt_uuid_t uuid;
+
+		bt_string_to_uuid(&uuid, chr->uuid);
+
+		if (bt_uuid_cmp(&uuid, &report_uuid) != 0)
+			continue;
+
+		hogdev->report = chr;
+		DBG("Report 0x%04x UUID: %s properties: %02x",
+				chr->handle, chr->uuid, chr->properties);
+	}
+}
+
 static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 {
 	struct hog_device *hogdev = user_data;
+	struct gatt_primary *prim = hogdev->hog_primary;
 	uint8_t value[] = { 0x01, 0x00 };
 
 	hogdev->attrib = g_attrib_ref(attrib);
 
 	gatt_write_char(hogdev->attrib, 0x000c, value, 2,
 						report_ccc_written_cb, hogdev);
+
+	gatt_discover_char(hogdev->attrib, prim->range.start, prim->range.end,
+					NULL, char_discovered_cb, hogdev);
 }
 
 static void attio_disconnected_cb(gpointer user_data)
-- 
1.7.7.6

[RFC 6/8] HoG: add report notification handler

[João Paulo Rechi Vita]

---
 input/hog_device.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/input/hog_device.c b/input/hog_device.c
index b06f898..3a767cd 100644
--- a/input/hog_device.c
+++ b/input/hog_device.c
@@ -60,9 +60,27 @@ struct hog_device {
 
 static GSList *devices = NULL;
 
+static void report_value_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
+{
+	uint16_t handle;
+
+	if (len < 3) {
+		error("Malformed ATT notification");
+		return;
+	}
+
+	handle = att_get_u16(&pdu[1]);
+
+	DBG("Report(0x%04x): 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x "
+				"0x%02x", handle, pdu[2], pdu[3], pdu[4],
+				pdu[5], pdu[6], pdu[7], pdu[8], pdu[9]);
+}
+
 static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
 					guint16 plen, gpointer user_data)
 {
+	struct hog_device *hogdev = user_data;
+
 	if (status != 0) {
 		error("Write report characteristic descriptor failed: %s",
 							att_ecode2str(status));
@@ -70,6 +88,9 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
 	}
 
 	DBG("Report characteristic descriptor written: notification enabled");
+
+	g_attrib_register(hogdev->attrib, ATT_OP_HANDLE_NOTIFY, report_value_cb,
+								hogdev, NULL);
 }
 
 static void char_discovered_cb(GSList *chars, guint8 status, gpointer user_data)
-- 
1.7.7.6

[RFC 7/8] HoG: print the report map on debug output

[João Paulo Rechi Vita]

---
 input/hog_device.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/input/hog_device.c b/input/hog_device.c
index 3a767cd..7a757f4 100644
--- a/input/hog_device.c
+++ b/input/hog_device.c
@@ -93,6 +93,31 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
 								hogdev, NULL);
 }
 
+static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
+							gpointer user_data)
+{
+	uint8_t value[ATT_MAX_MTU];
+	int vlen, i;
+
+	if (status != 0) {
+		error("Report Map read failed: %s", att_ecode2str(status));
+		return;
+	}
+
+	if (!dec_read_resp(pdu, plen, value, &vlen)) {
+		error("ATT protocol error");
+		return;
+	}
+
+	DBG("Report MAP:");
+	for (i = 0; i < vlen; i += 2) {
+		if (i + 1 == vlen)
+			DBG("\t %02x", value[i]);
+		else
+			DBG("\t %02x %02x", value[i], value[i + 1]);
+	}
+}
+
 static void char_discovered_cb(GSList *chars, guint8 status, gpointer user_data)
 {
 	struct hog_device *hogdev = user_data;
@@ -133,6 +158,8 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	gatt_write_char(hogdev->attrib, 0x000c, value, 2,
 						report_ccc_written_cb, hogdev);
 
+	gatt_read_char(hogdev->attrib, 0x0015, 0, report_map_read_cb, hogdev);
+
 	gatt_discover_char(hogdev->attrib, prim->range.start, prim->range.end,
 					NULL, char_discovered_cb, hogdev);
 }
-- 
1.7.7.6

[RFC 8/8] HoG: HID I/O driver

[João Paulo Rechi Vita]

UHID is HID I/O driver that makes possible to implement HID I/O drivers in
user-space. It works similar to the uinput but it is initialized with a HID
descriptor and deals with raw HID reports.

This commit uses UHID to create a HID device for the remote HoG device and
to tranfers HID reports to HID subsystem.
---
 input/hog_device.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/input/hog_device.c b/input/hog_device.c
index 7a757f4..183da22 100644
--- a/input/hog_device.c
+++ b/input/hog_device.c
@@ -29,6 +29,10 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <linux/uhid.h>
 
 #include <bluetooth/bluetooth.h>
 #include <bluetooth/uuid.h>
@@ -48,6 +52,7 @@
 #include "gatt.h"
 
 #define HOG_REPORT_UUID		0x2A4D
+#define UHID_DEVICE_FILE	"/dev/uhid"
 
 struct hog_device {
 	char			*path;
@@ -56,13 +61,17 @@ struct hog_device {
 	guint			attioid;
 	struct gatt_primary	*hog_primary;
 	struct gatt_char	*report;
+	int			uhid_fd;
 };
 
 static GSList *devices = NULL;
 
 static void report_value_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
 {
+	struct hog_device *hogdev = user_data;
+	struct uhid_event ev;
 	uint16_t handle;
+	uint16_t report_size = len - 2;
 
 	if (len < 3) {
 		error("Malformed ATT notification");
@@ -74,6 +83,14 @@ static void report_value_cb(const uint8_t *pdu, uint16_t len, gpointer user_data
 	DBG("Report(0x%04x): 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x "
 				"0x%02x", handle, pdu[2], pdu[3], pdu[4],
 				pdu[5], pdu[6], pdu[7], pdu[8], pdu[9]);
+
+	memset(&ev, 0, sizeof(ev));
+	ev.type = UHID_INPUT;
+	ev.u.input.size = report_size;
+	memcpy(ev.u.input.data, &pdu[2], report_size);
+
+	if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
+		error("UHID write failed: %s", strerror(errno));
 }
 
 static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
@@ -96,6 +113,8 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
 static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
 							gpointer user_data)
 {
+	struct hog_device *hogdev = user_data;
+	struct uhid_event ev;
 	uint8_t value[ATT_MAX_MTU];
 	int vlen, i;
 
@@ -116,6 +135,22 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
 		else
 			DBG("\t %02x %02x", value[i], value[i + 1]);
 	}
+
+	/* create UHID device */
+	memset(&ev, 0, sizeof(ev));
+	ev.type = UHID_CREATE;
+	/* TODO: get info from DIS */
+	strcpy((char*)ev.u.create.name, "bluez-hog-device");
+	ev.u.create.vendor = 0xBEBA;
+	ev.u.create.product = 0xCAFE;
+	ev.u.create.version = 0;
+	ev.u.create.country = 0;
+	ev.u.create.bus = BUS_USB; /* BUS_BLUETOOTH doesn't work here */
+	ev.u.create.rd_data = value;
+	ev.u.create.rd_size = vlen;
+
+	if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
+		error("Failed to create UHID device: %s", strerror(errno));
 }
 
 static void char_discovered_cb(GSList *chars, guint8 status, gpointer user_data)
@@ -162,11 +197,27 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 
 	gatt_discover_char(hogdev->attrib, prim->range.start, prim->range.end,
 					NULL, char_discovered_cb, hogdev);
+
+	if (hogdev->uhid_fd > 0)
+		return;
+
+	hogdev->uhid_fd = open(UHID_DEVICE_FILE, O_RDWR, 0666);
+	if (hogdev->uhid_fd < 0)
+		error("Failed to open UHID device: %s", strerror(errno));
 }
 
 static void attio_disconnected_cb(gpointer user_data)
 {
 	struct hog_device *hogdev = user_data;
+	struct uhid_event ev;
+
+	memset(&ev, 0, sizeof(ev));
+	ev.type = UHID_DESTROY;
+	if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
+		error("Failed to destroy UHID device: %s", strerror(errno));
+
+	close(hogdev->uhid_fd);
+	hogdev->uhid_fd = -1;
 
 	g_attrib_unref(hogdev->attrib);
 	hogdev->attrib = NULL;
-- 
1.7.7.6

Re: [RFC 2/8] HoG: register ATTIO callbacks

[Andrei Emeltchenko]

Hi João,

Minor comments:

On Tue, Mar 27, 2012 at 07:31:21PM -0300, João Paulo Rechi Vita wrote:
...
> +static struct hog_device *hog_device_new(struct btd_device *device,
> +							const char *path)
> +{
...
> +
> +	hogdev->attioid = btd_device_add_attio_callback(device,
> +							attio_connected_cb,
> +							attio_disconnected_cb,
> +							hogdev);

What is the coding style for the userspace? Shall it be at least the same?

Best regards 
Andrei Emeltchenko 

Re: [RFC 8/8] HoG: HID I/O driver

[David Herrmann]

Hi Joao

On Wed, Mar 28, 2012 at 12:31 AM, João Paulo Rechi Vita
<jprvita@openbossa.org> wrote:
> UHID is HID I/O driver that makes possible to implement HID I/O drivers in
> user-space. It works similar to the uinput but it is initialized with a HID
> descriptor and deals with raw HID reports.
>
> This commit uses UHID to create a HID device for the remote HoG device and
> to tranfers HID reports to HID subsystem.
> ---
>  input/hog_device.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/input/hog_device.c b/input/hog_device.c
> index 7a757f4..183da22 100644
> --- a/input/hog_device.c
> +++ b/input/hog_device.c
> @@ -29,6 +29,10 @@
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <linux/uhid.h>
>
>  #include <bluetooth/bluetooth.h>
>  #include <bluetooth/uuid.h>
> @@ -48,6 +52,7 @@
>  #include "gatt.h"
>
>  #define HOG_REPORT_UUID                0x2A4D
> +#define UHID_DEVICE_FILE       "/dev/uhid"
>
>  struct hog_device {
>        char                    *path;
> @@ -56,13 +61,17 @@ struct hog_device {
>        guint                   attioid;
>        struct gatt_primary     *hog_primary;
>        struct gatt_char        *report;
> +       int                     uhid_fd;
>  };
>
>  static GSList *devices = NULL;
>
>  static void report_value_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
>  {
> +       struct hog_device *hogdev = user_data;
> +       struct uhid_event ev;
>        uint16_t handle;
> +       uint16_t report_size = len - 2;
>
>        if (len < 3) {
>                error("Malformed ATT notification");
> @@ -74,6 +83,14 @@ static void report_value_cb(const uint8_t *pdu, uint16_t len, gpointer user_data
>        DBG("Report(0x%04x): 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x "
>                                "0x%02x", handle, pdu[2], pdu[3], pdu[4],
>                                pdu[5], pdu[6], pdu[7], pdu[8], pdu[9]);
> +
> +       memset(&ev, 0, sizeof(ev));
> +       ev.type = UHID_INPUT;
> +       ev.u.input.size = report_size;
> +       memcpy(ev.u.input.data, &pdu[2], report_size);

Please use min(report_size, UHID_DATA_MAX) here.

> +
> +       if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
> +               error("UHID write failed: %s", strerror(errno));
>  }
>
>  static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
> @@ -96,6 +113,8 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
>  static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
>                                                        gpointer user_data)
>  {
> +       struct hog_device *hogdev = user_data;
> +       struct uhid_event ev;
>        uint8_t value[ATT_MAX_MTU];
>        int vlen, i;
>
> @@ -116,6 +135,22 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
>                else
>                        DBG("\t %02x %02x", value[i], value[i + 1]);
>        }
> +
> +       /* create UHID device */
> +       memset(&ev, 0, sizeof(ev));
> +       ev.type = UHID_CREATE;
> +       /* TODO: get info from DIS */
> +       strcpy((char*)ev.u.create.name, "bluez-hog-device");
> +       ev.u.create.vendor = 0xBEBA;
> +       ev.u.create.product = 0xCAFE;
> +       ev.u.create.version = 0;
> +       ev.u.create.country = 0;
> +       ev.u.create.bus = BUS_USB; /* BUS_BLUETOOTH doesn't work here */

I've seen the other mail. I will have a look at this. It's probably
because generic-bluetooth doesn't get loaded for uhid devices but only
generic-usb.

> +       ev.u.create.rd_data = value;
> +       ev.u.create.rd_size = vlen;
> +
> +       if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
> +               error("Failed to create UHID device: %s", strerror(errno));
>  }
>
>  static void char_discovered_cb(GSList *chars, guint8 status, gpointer user_data)
> @@ -162,11 +197,27 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>
>        gatt_discover_char(hogdev->attrib, prim->range.start, prim->range.end,
>                                        NULL, char_discovered_cb, hogdev);
> +
> +       if (hogdev->uhid_fd > 0)
> +               return;
> +
> +       hogdev->uhid_fd = open(UHID_DEVICE_FILE, O_RDWR, 0666);

You probably want O_RDWR | O_CLOEXEC here. And please remove the third
parameter 0666. It doesn't make sense if you do not use O_CREATE.

> +       if (hogdev->uhid_fd < 0)
> +               error("Failed to open UHID device: %s", strerror(errno));

I don't know what side-effects the "error()" call has, but I think we
need better cleanup at all the error() calls in this file. But I
haven't checked it thoroughly, yet.

Also this is a good place to add uhid_fd to the event-loop and wait
for READABLE events. The callback should then simply read from uhid_fd
and write the raw data prefix with the att header. There is no need to
validate the data you read from uhid_fd.

>  }
>
>  static void attio_disconnected_cb(gpointer user_data)
>  {
>        struct hog_device *hogdev = user_data;
> +       struct uhid_event ev;
> +
> +       memset(&ev, 0, sizeof(ev));
> +       ev.type = UHID_DESTROY;
> +       if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
> +               error("Failed to destroy UHID device: %s", strerror(errno));
> +
> +       close(hogdev->uhid_fd);
> +       hogdev->uhid_fd = -1;
>
>        g_attrib_unref(hogdev->attrib);
>        hogdev->attrib = NULL;
> --
> 1.7.7.6
>

Looks good overall, regards
David

Re: [RFC 2/8] HoG: register ATTIO callbacks

[Joao Paulo Rechi Vita]

SGVsbG8gQW5kcmVpLAoKMjAxMi8zLzI4IEFuZHJlaSBFbWVsdGNoZW5rbyA8YW5kcmVpLmVtZWx0
Y2hlbmtvLm5ld3NAZ21haWwuY29tPjoKPiBIaSBKb8OjbywKPgo+IE1pbm9yIGNvbW1lbnRzOgo+
Cj4gT24gVHVlLCBNYXIgMjcsIDIwMTIgYXQgMDc6MzE6MjFQTSAtMDMwMCwgSm/Do28gUGF1bG8g
UmVjaGkgVml0YSB3cm90ZToKPiAuLi4KPj4gK3N0YXRpYyBzdHJ1Y3QgaG9nX2RldmljZSAqaG9n
X2RldmljZV9uZXcoc3RydWN0IGJ0ZF9kZXZpY2UgKmRldmljZSwKPj4gKyDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCBjb25zdCBjaGFyICpwYXRoKQo+PiArewo+IC4uLgo+PiArCj4+ICsgwqAgwqAgaG9n
ZGV2LT5hdHRpb2lkID0gYnRkX2RldmljZV9hZGRfYXR0aW9fY2FsbGJhY2soZGV2aWNlLAo+PiAr
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIGF0dGlvX2Nvbm5lY3RlZF9jYiwKPj4gKyDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCBhdHRpb19kaXNjb25uZWN0ZWRfY2IsCj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgaG9nZGV2KTsKPgo+IFdoYXQgaXMgdGhlIGNvZGluZyBzdHlsZSBmb3IgdGhlIHVzZXJzcGFj
ZT8gU2hhbGwgaXQgYmUgYXQgbGVhc3QgdGhlIHNhbWU/Cj4KClVubGVzcyBzb21ldGhpbmcgaGFz
IGNoYW5nZWQgbGF0ZWx5IHdlIHVzZSB0aGUga2VybmVsIGNvZGluZyBzdHlsZS4gSQpkb24ndCBz
ZWUgd2hlcmUgSSBkaWRuJ3QgZm9sbG93IGl0IGhlcmUsIGJ1dCBJIG1heSBiZSBtaXNzaW5nCnNv
bWV0aGluZy4gQ2FuIHlvdSBwbGVhc2UgcG9pbnQgbWUgd2hhdCBJJ20gbWlzc2luZyBpZiB0aGF0
IGlzIHRoZQpjYXNlPwoKLS0gCkpvw6NvIFBhdWxvIFJlY2hpIFZpdGEKT3BlbmJvc3NhIExhYnMg
LSBJTmRUCg==

Re: [RFC 2/8] HoG: register ATTIO callbacks

[Andrei Emeltchenko]

Hi João,

On Wed, Mar 28, 2012 at 10:55:18AM -0300, Joao Paulo Rechi Vita wrote:
> Hello Andrei,
> 
> 2012/3/28 Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com>:
> > Hi João,
> >
> > Minor comments:
> >
> > On Tue, Mar 27, 2012 at 07:31:21PM -0300, João Paulo Rechi Vita wrote:
> > ...
> >> +static struct hog_device *hog_device_new(struct btd_device *device,
> >> +                                                     const char *path)
> >> +{
> > ...
> >> +
> >> +     hogdev->attioid = btd_device_add_attio_callback(device,
> >> +                                                     attio_connected_cb,
> >> +                                                     attio_disconnected_cb,
> >> +                                                     hogdev);
> >
> > What is the coding style for the userspace? Shall it be at least the same?
> >
> 
> Unless something has changed lately we use the kernel coding style. I
> don't see where I didn't follow it here, but I may be missing
> something. Can you please point me what I'm missing if that is the
> case?

I mean are those parameters aligned to the right or at "(".

Best regards 
Andrei Emeltchenko 

Re: [RFC 8/8] HoG: HID I/O driver

[Joao Paulo Rechi Vita]

Hello David,

2012/3/28 David Herrmann <dh.herrmann@googlemail.com>:
> Hi Joao
>
> On Wed, Mar 28, 2012 at 12:31 AM, João Paulo Rechi Vita
> <jprvita@openbossa.org> wrote:
>> UHID is HID I/O driver that makes possible to implement HID I/O drivers in
>> user-space. It works similar to the uinput but it is initialized with a HID
>> descriptor and deals with raw HID reports.
>>
>> This commit uses UHID to create a HID device for the remote HoG device and
>> to tranfers HID reports to HID subsystem.
>> ---
>>  input/hog_device.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 51 insertions(+), 0 deletions(-)
>>
>> diff --git a/input/hog_device.c b/input/hog_device.c
>> index 7a757f4..183da22 100644
>> --- a/input/hog_device.c
>> +++ b/input/hog_device.c
>> @@ -29,6 +29,10 @@
>>  #include <stdlib.h>
>>  #include <errno.h>
>>  #include <unistd.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <fcntl.h>
>> +#include <linux/uhid.h>
>>
>>  #include <bluetooth/bluetooth.h>
>>  #include <bluetooth/uuid.h>
>> @@ -48,6 +52,7 @@
>>  #include "gatt.h"
>>
>>  #define HOG_REPORT_UUID                0x2A4D
>> +#define UHID_DEVICE_FILE       "/dev/uhid"
>>
>>  struct hog_device {
>>        char                    *path;
>> @@ -56,13 +61,17 @@ struct hog_device {
>>        guint                   attioid;
>>        struct gatt_primary     *hog_primary;
>>        struct gatt_char        *report;
>> +       int                     uhid_fd;
>>  };
>>
>>  static GSList *devices = NULL;
>>
>>  static void report_value_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
>>  {
>> +       struct hog_device *hogdev = user_data;
>> +       struct uhid_event ev;
>>        uint16_t handle;
>> +       uint16_t report_size = len - 2;
>>
>>        if (len < 3) {
>>                error("Malformed ATT notification");
>> @@ -74,6 +83,14 @@ static void report_value_cb(const uint8_t *pdu, uint16_t len, gpointer user_data
>>        DBG("Report(0x%04x): 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x "
>>                                "0x%02x", handle, pdu[2], pdu[3], pdu[4],
>>                                pdu[5], pdu[6], pdu[7], pdu[8], pdu[9]);
>> +
>> +       memset(&ev, 0, sizeof(ev));
>> +       ev.type = UHID_INPUT;
>> +       ev.u.input.size = report_size;
>> +       memcpy(ev.u.input.data, &pdu[2], report_size);
>
> Please use min(report_size, UHID_DATA_MAX) here.
>

Ok.

>> +
>> +       if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
>> +               error("UHID write failed: %s", strerror(errno));
>>  }
>>
>>  static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
>> @@ -96,6 +113,8 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
>>  static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
>>                                                        gpointer user_data)
>>  {
>> +       struct hog_device *hogdev = user_data;
>> +       struct uhid_event ev;
>>        uint8_t value[ATT_MAX_MTU];
>>        int vlen, i;
>>
>> @@ -116,6 +135,22 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
>>                else
>>                        DBG("\t %02x %02x", value[i], value[i + 1]);
>>        }
>> +
>> +       /* create UHID device */
>> +       memset(&ev, 0, sizeof(ev));
>> +       ev.type = UHID_CREATE;
>> +       /* TODO: get info from DIS */
>> +       strcpy((char*)ev.u.create.name, "bluez-hog-device");
>> +       ev.u.create.vendor = 0xBEBA;
>> +       ev.u.create.product = 0xCAFE;
>> +       ev.u.create.version = 0;
>> +       ev.u.create.country = 0;
>> +       ev.u.create.bus = BUS_USB; /* BUS_BLUETOOTH doesn't work here */
>
> I've seen the other mail. I will have a look at this. It's probably
> because generic-bluetooth doesn't get loaded for uhid devices but only
> generic-usb.
>

All right, I'll leave it like this and wait for any updates.

>> +       ev.u.create.rd_data = value;
>> +       ev.u.create.rd_size = vlen;
>> +
>> +       if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
>> +               error("Failed to create UHID device: %s", strerror(errno));
>>  }
>>
>>  static void char_discovered_cb(GSList *chars, guint8 status, gpointer user_data)
>> @@ -162,11 +197,27 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>>
>>        gatt_discover_char(hogdev->attrib, prim->range.start, prim->range.end,
>>                                        NULL, char_discovered_cb, hogdev);
>> +
>> +       if (hogdev->uhid_fd > 0)
>> +               return;
>> +
>> +       hogdev->uhid_fd = open(UHID_DEVICE_FILE, O_RDWR, 0666);
>
> You probably want O_RDWR | O_CLOEXEC here. And please remove the third
> parameter 0666. It doesn't make sense if you do not use O_CREATE.
>

What's is the point of using CLOEXEC here? AFAIK it only makes any
difference for programs that calls any syscall from the exec family,
and bluetoothd doesn't do so.

>> +       if (hogdev->uhid_fd < 0)
>> +               error("Failed to open UHID device: %s", strerror(errno));
>
> I don't know what side-effects the "error()" call has, but I think we
> need better cleanup at all the error() calls in this file. But I
> haven't checked it thoroughly, yet.
>

error() here is not from error.h, but from bluez own log.h. It doesn't
have any side effects, just prints the strings to stderr. I don't
think there are cleanups missing from them, tho.

> Also this is a good place to add uhid_fd to the event-loop and wait
> for READABLE events. The callback should then simply read from uhid_fd
> and write the raw data prefix with the att header. There is no need to
> validate the data you read from uhid_fd.
>

Support for readable events will come in a separate patch.

>>  }
>>
>>  static void attio_disconnected_cb(gpointer user_data)
>>  {
>>        struct hog_device *hogdev = user_data;
>> +       struct uhid_event ev;
>> +
>> +       memset(&ev, 0, sizeof(ev));
>> +       ev.type = UHID_DESTROY;
>> +       if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
>> +               error("Failed to destroy UHID device: %s", strerror(errno));
>> +
>> +       close(hogdev->uhid_fd);
>> +       hogdev->uhid_fd = -1;
>>
>>        g_attrib_unref(hogdev->attrib);
>>        hogdev->attrib = NULL;
>> --
>> 1.7.7.6
>>
>
> Looks good overall, regards
> David

Thanks for the comments.

-- 
João Paulo Rechi Vita
Openbossa Labs - INdT

Re: [RFC 2/8] HoG: register ATTIO callbacks

[Joao Paulo Rechi Vita]

On Wed, Mar 28, 2012 at 11:14 AM, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi João,
>
> On Wed, Mar 28, 2012 at 10:55:18AM -0300, Joao Paulo Rechi Vita wrote:
>> Hello Andrei,
>>
>> 2012/3/28 Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com>:
>> > Hi João,
>> >
>> > Minor comments:
>> >
>> > On Tue, Mar 27, 2012 at 07:31:21PM -0300, João Paulo Rechi Vita wrote:
>> > ...
>> >> +static struct hog_device *hog_device_new(struct btd_device *device,
>> >> +                                                     const char *path)
>> >> +{
>> > ...
>> >> +
>> >> +     hogdev->attioid = btd_device_add_attio_callback(device,
>> >> +                                                     attio_connected_cb,
>> >> +                                                     attio_disconnected_cb,
>> >> +                                                     hogdev);
>> >
>> > What is the coding style for the userspace? Shall it be at least the same?
>> >
>>
>> Unless something has changed lately we use the kernel coding style. I
>> don't see where I didn't follow it here, but I may be missing
>> something. Can you please point me what I'm missing if that is the
>> case?
>
> I mean are those parameters aligned to the right or at "(".
>

The parameters are aligned to the right and after the opening brace.
Isn't that right?

-- 
João Paulo Rechi Vita
Openbossa Labs - INdT