[PATCH v2 0/3] Optionally destroy uHID devices on disconnect

[PATCH v2 0/3] Optionally destroy uHID devices on disconnect

[Juha Kuikka]

Changes since v1:
 * Add UHIDDestroy configuration option and make it control uHID lifetime

Juha Kuikka (3):
  input: Add UHIDDestroy configuration option
  input: Add UHIDDestroy option into input.conf
  hog: Optionally destroy uHID device when disconnected

 profiles/input/hog-lib.c  | 29 +++++++++++++++++++++++++++++
 profiles/input/hog-lib.h  |  1 +
 profiles/input/input.conf |  4 ++++
 profiles/input/manager.c  | 13 ++++++++++++-
 4 files changed, 46 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v2 1/3] input: Add UHIDDestroy configuration option

[Juha Kuikka]

Add a configuration option UHIDDestroy to control whether HoG uHID devices
persist or are destroyed on disconnect.
---
 profiles/input/manager.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/profiles/input/manager.c b/profiles/input/manager.c
index 1d31b06..b36e77e 100644
--- a/profiles/input/manager.c
+++ b/profiles/input/manager.c
@@ -43,6 +43,8 @@
 #include "device.h"
 #include "server.h"
 
+#include "profiles/input/hog-lib.h"
+
 static int hid_server_probe(struct btd_profile *p, struct btd_adapter *adapter)
 {
 	return server_start(btd_adapter_get_address(adapter));
@@ -96,7 +98,7 @@ static int input_init(void)
 	config = load_config_file(CONFIGDIR "/input.conf");
 	if (config) {
 		int idle_timeout;
-		gboolean uhid_enabled;
+		gboolean uhid_enabled, uhid_destroy;
 
 		idle_timeout = g_key_file_get_integer(config, "General",
 							"IdleTimeout", &err);
@@ -114,6 +116,15 @@ static int input_init(void)
 			input_enable_userspace_hid(uhid_enabled);
 		} else
 			g_clear_error(&err);
+
+		uhid_destroy = g_key_file_get_boolean(config, "General",
+							"UHIDDestroy", &err);
+		if (!err) {
+			DBG("input.conf: UHIDDestroy=%s", uhid_destroy ?
+							"true" : "false");
+			bt_hog_set_uhid_destroy(uhid_destroy);
+		} else
+			g_clear_error(&err);
 	}
 
 	btd_profile_register(&input_profile);
-- 
2.7.4

[PATCH v2 2/3] input: Add UHIDDestroy option into input.conf

[Juha Kuikka]

UHIDDestroy controls whether uHID device is destroyed on HID/HoG disconnect.
Defaults to false.
---
 profiles/input/input.conf | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/profiles/input/input.conf b/profiles/input/input.conf
index 3e1d65a..4a93e42 100644
--- a/profiles/input/input.conf
+++ b/profiles/input/input.conf
@@ -11,3 +11,7 @@
 # Enable HID protocol handling in userspace input profile
 # Defaults to false (HIDP handled in HIDP kernel module)
 #UserspaceHID=true
+
+# Destroy uHID device on HID/HoG disconnect
+# Defaults to false (uHID device is persistent across reconnects)
+#UHIDDestroy=true
-- 
2.7.4

[PATCH v2 3/3] hog: Optionally destroy uHID device when disconnected

[Juha Kuikka]

Unlink bluetooth classic HID, HoG persists the uHID device after across
connections. This is fine for stateless devices that require fast reconnections
but some device utilize hidraw to configure them upon connection and with the
persisting uHID device, reconnections are hidden from them.

Similarly, applications such as game emulators may be using the presence of the
HID or the accompanying input event devices to indicate that it is available,
thus changing behavior.

This patch introduces a configuration option that, when set, destroys the uHID
device upon disconnection, it gets re-created upon reconnection, just like on
initial connection.
---
 profiles/input/hog-lib.c | 29 +++++++++++++++++++++++++++++
 profiles/input/hog-lib.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
index dab385f..b2a371e 100644
--- a/profiles/input/hog-lib.c
+++ b/profiles/input/hog-lib.c
@@ -129,6 +129,13 @@ struct gatt_request {
 	void *user_data;
 };
 
+static bool uhid_destroy = false;
+
+void bt_hog_set_uhid_destroy(bool state)
+{
+	uhid_destroy = state;
+}
+
 static struct gatt_request *create_request(struct bt_hog *hog,
 							void *user_data)
 {
@@ -1565,6 +1572,25 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
 	}
 }
 
+static void destroy_uhid(struct bt_hog *hog)
+{
+	struct uhid_event ev = {
+		.type = UHID_DESTROY
+	};
+	int err;
+
+	if (hog->uhid_created) {
+		err = bt_uhid_send(hog->uhid, &ev);
+		if (err < 0) {
+			error("bt_uhid_send: %s", strerror(-err));
+			return;
+		}
+		hog->uhid_created = false;
+
+		DBG("HoG destroyed uHID device");
+	}
+}
+
 bool bt_hog_attach(struct bt_hog *hog, void *gatt)
 {
 	GSList *l;
@@ -1651,6 +1677,9 @@ void bt_hog_detach(struct bt_hog *hog)
 	queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL);
 	g_attrib_unref(hog->attrib);
 	hog->attrib = NULL;
+
+	if (uhid_destroy)
+		destroy_uhid(hog);
 }
 
 int bt_hog_set_control_point(struct bt_hog *hog, bool suspend)
diff --git a/profiles/input/hog-lib.h b/profiles/input/hog-lib.h
index 415dc63..de8c42f 100644
--- a/profiles/input/hog-lib.h
+++ b/profiles/input/hog-lib.h
@@ -39,3 +39,4 @@ void bt_hog_detach(struct bt_hog *hog);
 
 int bt_hog_set_control_point(struct bt_hog *hog, bool suspend);
 int bt_hog_send_report(struct bt_hog *hog, void *data, size_t size, int type);
+void bt_hog_set_uhid_destroy(bool state);
-- 
2.7.4

Re: [PATCH v2 2/3] input: Add UHIDDestroy option into input.conf

[Luiz Augusto von Dentz]

Hi Juha,

On Wed, Mar 1, 2017 at 3:06 AM, Juha Kuikka <juha.kuikka@synapse.com> wrote:
> UHIDDestroy controls whether uHID device is destroyed on HID/HoG disconnect.
> Defaults to false.
> ---
>  profiles/input/input.conf | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/profiles/input/input.conf b/profiles/input/input.conf
> index 3e1d65a..4a93e42 100644
> --- a/profiles/input/input.conf
> +++ b/profiles/input/input.conf
> @@ -11,3 +11,7 @@
>  # Enable HID protocol handling in userspace input profile
>  # Defaults to false (HIDP handled in HIDP kernel module)
>  #UserspaceHID=true
> +
> +# Destroy uHID device on HID/HoG disconnect
> +# Defaults to false (uHID device is persistent across reconnects)
> +#UHIDDestroy=true
> --
> 2.7.4

Id follow the prefix of UserspaceHID here since it should apply to any
uHID instance including classic and please merge this into the first
patch.


-- 
Luiz Augusto von Dentz

Re: [PATCH v2 3/3] hog: Optionally destroy uHID device when disconnected

[Luiz Augusto von Dentz]

Hi Juha,

On Wed, Mar 1, 2017 at 3:06 AM, Juha Kuikka <juha.kuikka@synapse.com> wrote:
> Unlink bluetooth classic HID, HoG persists the uHID device after across
> connections. This is fine for stateless devices that require fast reconnections
> but some device utilize hidraw to configure them upon connection and with the
> persisting uHID device, reconnections are hidden from them.
>
> Similarly, applications such as game emulators may be using the presence of the
> HID or the accompanying input event devices to indicate that it is available,
> thus changing behavior.
>
> This patch introduces a configuration option that, when set, destroys the uHID
> device upon disconnection, it gets re-created upon reconnection, just like on
> initial connection.
> ---
>  profiles/input/hog-lib.c | 29 +++++++++++++++++++++++++++++
>  profiles/input/hog-lib.h |  1 +
>  2 files changed, 30 insertions(+)
>
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index dab385f..b2a371e 100644
> --- a/profiles/input/hog-lib.c
> +++ b/profiles/input/hog-lib.c
> @@ -129,6 +129,13 @@ struct gatt_request {
>         void *user_data;
>  };
>
> +static bool uhid_destroy = false;
> +
> +void bt_hog_set_uhid_destroy(bool state)
> +{
> +       uhid_destroy = state;
> +}
> +
>  static struct gatt_request *create_request(struct bt_hog *hog,
>                                                         void *user_data)
>  {
> @@ -1565,6 +1572,25 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
>         }
>  }
>
> +static void destroy_uhid(struct bt_hog *hog)
> +{
> +       struct uhid_event ev = {
> +               .type = UHID_DESTROY
> +       };
> +       int err;
> +
> +       if (hog->uhid_created) {
> +               err = bt_uhid_send(hog->uhid, &ev);
> +               if (err < 0) {
> +                       error("bt_uhid_send: %s", strerror(-err));
> +                       return;
> +               }
> +               hog->uhid_created = false;
> +
> +               DBG("HoG destroyed uHID device");
> +       }
> +}
> +
>  bool bt_hog_attach(struct bt_hog *hog, void *gatt)
>  {
>         GSList *l;
> @@ -1651,6 +1677,9 @@ void bt_hog_detach(struct bt_hog *hog)
>         queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL);
>         g_attrib_unref(hog->attrib);
>         hog->attrib = NULL;
> +
> +       if (uhid_destroy)
> +               destroy_uhid(hog);

It probably make sense to check if hog->uhid_created.

>  }
>
>  int bt_hog_set_control_point(struct bt_hog *hog, bool suspend)
> diff --git a/profiles/input/hog-lib.h b/profiles/input/hog-lib.h
> index 415dc63..de8c42f 100644
> --- a/profiles/input/hog-lib.h
> +++ b/profiles/input/hog-lib.h
> @@ -39,3 +39,4 @@ void bt_hog_detach(struct bt_hog *hog);
>
>  int bt_hog_set_control_point(struct bt_hog *hog, bool suspend);
>  int bt_hog_send_report(struct bt_hog *hog, void *data, size_t size, int type);
> +void bt_hog_set_uhid_destroy(bool state);

Ive made this per instance.

> --
> 2.7.4

I would consider doing this in the bt_uhid instance



-- 
Luiz Augusto von Dentz

Re: [PATCH v2 2/3] input: Add UHIDDestroy option into input.conf

[Juha Kuikka]

Hi Luiz

On Wed, Mar 1, 2017 at 2:21 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Juha,
>
> On Wed, Mar 1, 2017 at 3:06 AM, Juha Kuikka <juha.kuikka@synapse.com> wrote:
>> UHIDDestroy controls whether uHID device is destroyed on HID/HoG disconnect.
>> Defaults to false.
>> ---
>>  profiles/input/input.conf | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/profiles/input/input.conf b/profiles/input/input.conf
>> index 3e1d65a..4a93e42 100644
>> --- a/profiles/input/input.conf
>> +++ b/profiles/input/input.conf
>> @@ -11,3 +11,7 @@
>>  # Enable HID protocol handling in userspace input profile
>>  # Defaults to false (HIDP handled in HIDP kernel module)
>>  #UserspaceHID=true
>> +
>> +# Destroy uHID device on HID/HoG disconnect
>> +# Defaults to false (uHID device is persistent across reconnects)
>> +#UHIDDestroy=true
>> --
>> 2.7.4
>
> Id follow the prefix of UserspaceHID here since it should apply to any
> uHID instance including classic and please merge this into the first
> patch.

Thanks, will do!

 - Juha

Re: [PATCH v2 3/3] hog: Optionally destroy uHID device when disconnected

[Juha Kuikka]

On Wed, Mar 1, 2017 at 2:58 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Juha,
>
> On Wed, Mar 1, 2017 at 3:06 AM, Juha Kuikka <juha.kuikka@synapse.com> wrote:
>> Unlink bluetooth classic HID, HoG persists the uHID device after across
>> connections. This is fine for stateless devices that require fast reconnections
>> but some device utilize hidraw to configure them upon connection and with the
>> persisting uHID device, reconnections are hidden from them.
>>
>> Similarly, applications such as game emulators may be using the presence of the
>> HID or the accompanying input event devices to indicate that it is available,
>> thus changing behavior.
>>
>> This patch introduces a configuration option that, when set, destroys the uHID
>> device upon disconnection, it gets re-created upon reconnection, just like on
>> initial connection.
>> ---
>>  profiles/input/hog-lib.c | 29 +++++++++++++++++++++++++++++
>>  profiles/input/hog-lib.h |  1 +
>>  2 files changed, 30 insertions(+)
>>
>> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
>> index dab385f..b2a371e 100644
>> --- a/profiles/input/hog-lib.c
>> +++ b/profiles/input/hog-lib.c
>> @@ -129,6 +129,13 @@ struct gatt_request {
>>         void *user_data;
>>  };
>>
>> +static bool uhid_destroy = false;
>> +
>> +void bt_hog_set_uhid_destroy(bool state)
>> +{
>> +       uhid_destroy = state;
>> +}
>> +
>>  static struct gatt_request *create_request(struct bt_hog *hog,
>>                                                         void *user_data)
>>  {
>> @@ -1565,6 +1572,25 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
>>         }
>>  }
>>
>> +static void destroy_uhid(struct bt_hog *hog)
>> +{
>> +       struct uhid_event ev = {
>> +               .type = UHID_DESTROY
>> +       };
>> +       int err;
>> +
>> +       if (hog->uhid_created) {
>> +               err = bt_uhid_send(hog->uhid, &ev);
>> +               if (err < 0) {
>> +                       error("bt_uhid_send: %s", strerror(-err));
>> +                       return;
>> +               }
>> +               hog->uhid_created = false;
>> +
>> +               DBG("HoG destroyed uHID device");
>> +       }
>> +}
>> +
>>  bool bt_hog_attach(struct bt_hog *hog, void *gatt)
>>  {
>>         GSList *l;
>> @@ -1651,6 +1677,9 @@ void bt_hog_detach(struct bt_hog *hog)
>>         queue_foreach(hog->gatt_op, (void *) cancel_gatt_req, NULL);
>>         g_attrib_unref(hog->attrib);
>>         hog->attrib = NULL;
>> +
>> +       if (uhid_destroy)
>> +               destroy_uhid(hog);
>
> It probably make sense to check if hog->uhid_created.

That is checked within destroy_uhid() but I can move it here so the
checks are all in one place.

>
>>  }
>>
>>  int bt_hog_set_control_point(struct bt_hog *hog, bool suspend)
>> diff --git a/profiles/input/hog-lib.h b/profiles/input/hog-lib.h
>> index 415dc63..de8c42f 100644
>> --- a/profiles/input/hog-lib.h
>> +++ b/profiles/input/hog-lib.h
>> @@ -39,3 +39,4 @@ void bt_hog_detach(struct bt_hog *hog);
>>
>>  int bt_hog_set_control_point(struct bt_hog *hog, bool suspend);
>>  int bt_hog_send_report(struct bt_hog *hog, void *data, size_t size, int type);
>> +void bt_hog_set_uhid_destroy(bool state);
>
> Ive made this per instance.
>
>> --
>> 2.7.4
>
> I would consider doing this in the bt_uhid instance

Can you elaborate? Moving the uhid_destroy flag into struct bt_uhid?
Or just into the src/shared/uhid.c? Since the configuration file
option is global and read before anything else, no instances of bt_hog
or bt_uhid are present. Are you thinking of adding a runtime
configuration mechanism?

The user of bt_uhid is responsible for creating the uhid device so I
made the destruction mirror that.

 - Juha