Hi Luiz,
We discussed the persistence of the uHID object some time ago. I went ahead
and implemented this chance, making the uHID object follow the life time of the
connection rather than the device.
Like you said, this may not be something you want to merge for other reasons,
but since we need it, I figured I'd post it anyway since it might be useful
for someone else as well.
Thanks,
- Juha
Hi Juha,
On Tue, Feb 28, 2017 at 4:01 PM, Juha Kuikka <[email protected]> wrote:
> Hi Petry,
>
> On Mon, Feb 27, 2017 at 6:17 PM, Petri Gynther <[email protected]> wrote:
>> Hi Juha,
>>
>> On Mon, Feb 27, 2017 at 2:40 PM, Juha Kuikka <[email protected]> wrote:
>>>
>>> Unlink bluetooth classic HID, HoG persists the uHID device after across
>>> connections. This is fine for stateless devices 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 destroys the uHID device upon disconnection, it gets re-created upon
>>> reconnection, just like on initial connection.
>>
>> For HID and HoG remote controls, the reconnect latency is important
>> for user experience. Thus, it is desirable that the uHID device is
>> persistent across reconnects until next reboot.
>
> I see. That makes sense, this way the whole pipeline is guaranteed to
> be already open when the connection forms, leading to less dropped
> button presses I suppose.
>
Correct.
>> How about adding a new config item to profiles/input/input.conf to
>> control this (default = persistent uHID devices)?
>
> Works for me. Something like this maybe? Excuse the quick patch, I
> will send a revision to the set.
>
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index f41055b..2b954b3 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 persist_uhid = true;
> +
> +void bt_hog_set_persist_uhid(bool state)
> +{
> + persist_uhid = state;
> +}
> +
> static struct gatt_request *create_request(struct bt_hog *hog,
> void *user_data)
> {
> diff --git a/profiles/input/hog-lib.h b/profiles/input/hog-lib.h
> index 415dc63..7e81547 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_persist_uhid(bool state);
> diff --git a/profiles/input/manager.c b/profiles/input/manager.c
> index 1d31b06..129b7b5 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, persist_uhid;
>
> 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);
> +
> + persist_uhid = g_key_file_get_boolean(config, "General",
> + "PersistUHID", &err);
> + if (!err) {
> + DBG("input.conf: PersistUHID=%s", persist_uhid ?
> + "true" : "false");
> + bt_hog_set_persist_uhid(persist_uhid);
> + } else
> + g_clear_error(&err);
> }
>
> btd_profile_register(&input_profile);
>
> Thanks,
> - Juha
I would prefer to use UHIDDestroy (which will be false by default).
Please add this to your patch:
diff --git a/profiles/input/input.conf b/profiles/input/input.conf
index 3e1d65aae..4a93e4220 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
Hi Petry,
On Mon, Feb 27, 2017 at 6:17 PM, Petri Gynther <[email protected]> wrote:
> Hi Juha,
>
> On Mon, Feb 27, 2017 at 2:40 PM, Juha Kuikka <[email protected]> wrote:
>>
>> Unlink bluetooth classic HID, HoG persists the uHID device after across
>> connections. This is fine for stateless devices 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 destroys the uHID device upon disconnection, it gets re-created upon
>> reconnection, just like on initial connection.
>
> For HID and HoG remote controls, the reconnect latency is important
> for user experience. Thus, it is desirable that the uHID device is
> persistent across reconnects until next reboot.
I see. That makes sense, this way the whole pipeline is guaranteed to
be already open when the connection forms, leading to less dropped
button presses I suppose.
> How about adding a new config item to profiles/input/input.conf to
> control this (default = persistent uHID devices)?
Works for me. Something like this maybe? Excuse the quick patch, I
will send a revision to the set.
diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
index f41055b..2b954b3 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 persist_uhid = true;
+
+void bt_hog_set_persist_uhid(bool state)
+{
+ persist_uhid = state;
+}
+
static struct gatt_request *create_request(struct bt_hog *hog,
void *user_data)
{
diff --git a/profiles/input/hog-lib.h b/profiles/input/hog-lib.h
index 415dc63..7e81547 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_persist_uhid(bool state);
diff --git a/profiles/input/manager.c b/profiles/input/manager.c
index 1d31b06..129b7b5 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, persist_uhid;
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);
+
+ persist_uhid = g_key_file_get_boolean(config, "General",
+ "PersistUHID", &err);
+ if (!err) {
+ DBG("input.conf: PersistUHID=%s", persist_uhid ?
+ "true" : "false");
+ bt_hog_set_persist_uhid(persist_uhid);
+ } else
+ g_clear_error(&err);
}
btd_profile_register(&input_profile);
Thanks,
- Juha
Hi Juha,
On Mon, Feb 27, 2017 at 2:40 PM, Juha Kuikka <[email protected]> wrote:
>
> Unlink bluetooth classic HID, HoG persists the uHID device after across
> connections. This is fine for stateless devices 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 destroys the uHID device upon disconnection, it gets re-created upon
> reconnection, just like on initial connection.
For HID and HoG remote controls, the reconnect latency is important
for user experience. Thus, it is desirable that the uHID device is
persistent across reconnects until next reboot.
How about adding a new config item to profiles/input/input.conf to
control this (default = persistent uHID devices)?
-- Petri
>
> ---
> profiles/input/hog-lib.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index dab385f..f41055b 100644
> --- a/profiles/input/hog-lib.c
> +++ b/profiles/input/hog-lib.c
> @@ -1565,6 +1565,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 +1670,8 @@ 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;
> +
> + destroy_uhid(hog);
> }
>
> int bt_hog_set_control_point(struct bt_hog *hog, bool suspend)
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Unlink bluetooth classic HID, HoG persists the uHID device after across
connections. This is fine for stateless devices 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 destroys the uHID device upon disconnection, it gets re-created upon
reconnection, just like on initial connection.
---
profiles/input/hog-lib.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
index dab385f..f41055b 100644
--- a/profiles/input/hog-lib.c
+++ b/profiles/input/hog-lib.c
@@ -1565,6 +1565,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 +1670,8 @@ 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;
+
+ destroy_uhid(hog);
}
int bt_hog_set_control_point(struct bt_hog *hog, bool suspend)
--
2.7.4