[PATCH] input: Implement idle timeout for fakehid.

[PATCH] input: Implement idle timeout for fakehid.

[Jeff Hansen]

The batteries on my BD remote would die in a few days without this patch.
Now they've been going for weeks (hopefully months).
---
 profiles/input/device.c  |    6 ++++++
 profiles/input/device.h  |    4 ++++
 profiles/input/fakehid.c |   18 ++++++++++++++++++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index 48d397a..31c970d 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -638,6 +638,7 @@ static int hidp_add_connection(const struct input_device *idev,
 		fake->disconnect = fake_hid_disconnect;
 		fake->priv = fake_hid;
 		fake->idev = idev;
+		fake->timeout = iconn->timeout * 1000;
 		fake = fake_hid_connadd(fake, iconn->intr_io, fake_hid);
 		if (fake == NULL)
 			err = -ENOMEM;
@@ -1307,3 +1308,8 @@ int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst)
 
 	return 0;
 }
+
+void input_device_request_disconnect(const struct input_device *idev)
+{
+	device_request_disconnect(idev->device, NULL);
+}
diff --git a/profiles/input/device.h b/profiles/input/device.h
index 509a353..1b490b4 100644
--- a/profiles/input/device.h
+++ b/profiles/input/device.h
@@ -37,6 +37,8 @@ struct fake_input {
 	int		(*disconnect) (struct input_conn *iconn);
 	void		*priv;
 	const struct input_device *idev;
+	guint		idle_timeout;
+	uint32_t	timeout;
 };
 
 int fake_input_register(DBusConnection *conn, struct btd_device *device,
@@ -49,3 +51,5 @@ int input_device_unregister(const char *path, const char *uuid);
 int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm,
 							GIOChannel *io);
 int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst);
+
+void input_device_request_disconnect(const struct input_device *idev);
diff --git a/profiles/input/fakehid.c b/profiles/input/fakehid.c
index 3be1489..a92622f 100644
--- a/profiles/input/fakehid.c
+++ b/profiles/input/fakehid.c
@@ -199,6 +199,13 @@ error:
 	return -1;
 }
 
+static gboolean ps3remote_idle(gpointer data)
+{
+	struct fake_input *fake = data;
+	input_device_request_disconnect(fake->idev);
+	return FALSE;
+}
+
 static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 				gpointer data)
 {
@@ -209,6 +216,13 @@ static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 	char buff[50];
 	int fd;
 
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
+	if (fake->timeout)
+		fake->idle_timeout = g_timeout_add(fake->timeout, ps3remote_idle, fake);
+
 	if (cond & G_IO_NVAL)
 		return FALSE;
 
@@ -328,6 +342,10 @@ static gboolean fake_hid_common_connect(struct fake_input *fake, GError **err)
 
 static int fake_hid_common_disconnect(struct fake_input *fake)
 {
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
 	return 0;
 }
 
-- 
1.7.8.6

Re: [PATCH] input: Implement idle timeout for fakehid.

[Johan Hedberg]

Hi Jeff,

On Mon, Aug 27, 2012, Jeff Hansen wrote:
> +		fake->timeout = iconn->timeout * 1000;

Why not use the original iconn->timeout value and then use
g_timeout_add_seconds instead of g_timeout_add?

> +	guint		idle_timeout;
> +	uint32_t	timeout;

Why uint32_t instead of just int that iconn->timeout is?

> +static gboolean ps3remote_idle(gpointer data)
> +{
> +	struct fake_input *fake = data;
> +	input_device_request_disconnect(fake->idev);
> +	return FALSE;
> +}

Could you add an empty line before and after
input_device_request_disconnect for slightly better readability.

> +	if (fake->idle_timeout) {

For consistency with the rest of the code base please use > 0 for
checking for valid GSource IDs. I'd also just call it idle_id to avoid
confusion with "timeout" which is not a GSource ID.

> +		g_source_remove(fake->idle_timeout);
> +		fake->idle_timeout = 0;
> +	}
> +	if (fake->timeout)
> +		fake->idle_timeout = g_timeout_add(fake->timeout, ps3remote_idle, fake);

Please add an empty line before this second if statement and also do > 0
here.

> +	if (fake->idle_timeout) {

And > 0 again here.

Johan

[PATCH] input: Implement idle timeout for fakehid.

[Jeff Hansen]

The batteries on my BD remote would die in a few days without this patch.
Now they've been going for weeks (hopefully months).
---
 profiles/input/device.c  |    6 ++++++
 profiles/input/device.h  |    4 ++++
 profiles/input/fakehid.c |   18 ++++++++++++++++++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index 48d397a..31c970d 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -638,6 +638,7 @@ static int hidp_add_connection(const struct input_device *idev,
 		fake->disconnect = fake_hid_disconnect;
 		fake->priv = fake_hid;
 		fake->idev = idev;
+		fake->timeout = iconn->timeout * 1000;
 		fake = fake_hid_connadd(fake, iconn->intr_io, fake_hid);
 		if (fake == NULL)
 			err = -ENOMEM;
@@ -1307,3 +1308,8 @@ int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst)
 
 	return 0;
 }
+
+void input_device_request_disconnect(const struct input_device *idev)
+{
+	device_request_disconnect(idev->device, NULL);
+}
diff --git a/profiles/input/device.h b/profiles/input/device.h
index 509a353..1b490b4 100644
--- a/profiles/input/device.h
+++ b/profiles/input/device.h
@@ -37,6 +37,8 @@ struct fake_input {
 	int		(*disconnect) (struct input_conn *iconn);
 	void		*priv;
 	const struct input_device *idev;
+	guint		idle_timeout;
+	uint32_t	timeout;
 };
 
 int fake_input_register(DBusConnection *conn, struct btd_device *device,
@@ -49,3 +51,5 @@ int input_device_unregister(const char *path, const char *uuid);
 int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm,
 							GIOChannel *io);
 int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst);
+
+void input_device_request_disconnect(const struct input_device *idev);
diff --git a/profiles/input/fakehid.c b/profiles/input/fakehid.c
index 3be1489..a92622f 100644
--- a/profiles/input/fakehid.c
+++ b/profiles/input/fakehid.c
@@ -199,6 +199,13 @@ error:
 	return -1;
 }
 
+static gboolean ps3remote_idle(gpointer data)
+{
+	struct fake_input *fake = data;
+	input_device_request_disconnect(fake->idev);
+	return FALSE;
+}
+
 static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 				gpointer data)
 {
@@ -209,6 +216,13 @@ static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 	char buff[50];
 	int fd;
 
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
+	if (fake->timeout)
+		fake->idle_timeout = g_timeout_add(fake->timeout, ps3remote_idle, fake);
+
 	if (cond & G_IO_NVAL)
 		return FALSE;
 
@@ -328,6 +342,10 @@ static gboolean fake_hid_common_connect(struct fake_input *fake, GError **err)
 
 static int fake_hid_common_disconnect(struct fake_input *fake)
 {
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
 	return 0;
 }
 
-- 
1.7.8.6

[PATCH] input: Implement idle timeout for fakehid

[Jeff Hansen]

[-- Attachment #1: Type: text/plain, Size: 174 bytes --]

(bump)

The batteries on my BD remote would die in a few days without this
patch.  The idle timeout needs to be enforced for fakehid devices as
well as normal HID devices.



[-- Attachment #2: 0001-input-Implement-idle-timeout-for-fakehid.patch --]
[-- Type: text/x-patch, Size: 2950 bytes --]

>From a96f67cb6268e34e677e5d288f2c3b89acfca7fe Mon Sep 17 00:00:00 2001
From: Jeff Hansen <x@jeffhansen.com>
Date: Tue, 31 Jul 2012 22:45:23 -0600
Subject: [PATCH] input: Implement idle timeout for fakehid.

The batteries on my BD remote would die in a few days without this patch.

Signed-off-by: Jeff Hansen <x@jeffhansen.com>
---
 input/device.c  |    6 ++++++
 input/device.h  |    4 ++++
 input/fakehid.c |   18 ++++++++++++++++++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/input/device.c b/input/device.c
index 0ab63c0..ae11a13 100644
--- a/input/device.c
+++ b/input/device.c
@@ -609,6 +609,7 @@ static int hidp_add_connection(const struct input_device *idev,
 		fake->disconnect = fake_hid_disconnect;
 		fake->priv = fake_hid;
 		fake->idev = idev;
+		fake->timeout = iconn->timeout * 1000;
 		fake = fake_hid_connadd(fake, iconn->intr_io, fake_hid);
 		if (fake == NULL)
 			err = -ENOMEM;
@@ -1257,3 +1258,8 @@ int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst)
 
 	return 0;
 }
+
+void input_device_request_disconnect(const struct input_device *idev)
+{
+	device_request_disconnect(idev->device, NULL);
+}
diff --git a/input/device.h b/input/device.h
index 14c0f97..416cc9c 100644
--- a/input/device.h
+++ b/input/device.h
@@ -40,6 +40,8 @@ struct fake_input {
 	int		(*disconnect) (struct input_conn *iconn);
 	void		*priv;
 	const struct input_device *idev;
+	guint		idle_timeout;
+	uint32_t	timeout;
 };
 
 int fake_input_register(DBusConnection *conn, struct btd_device *device,
@@ -54,3 +56,5 @@ int input_device_unregister(const char *path, const char *uuid);
 int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm,
 							GIOChannel *io);
 int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst);
+
+void input_device_request_disconnect(const struct input_device *idev);
diff --git a/input/fakehid.c b/input/fakehid.c
index b809870..0407ae3 100644
--- a/input/fakehid.c
+++ b/input/fakehid.c
@@ -208,6 +208,13 @@ error:
 	return -1;
 }
 
+static gboolean ps3remote_idle(gpointer data)
+{
+	struct fake_input *fake = data;
+	input_device_request_disconnect(fake->idev);
+	return FALSE;
+}
+
 static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 				gpointer data)
 {
@@ -218,6 +225,13 @@ static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 	char buff[50];
 	int fd;
 
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
+	if (fake->timeout)
+		fake->idle_timeout = g_timeout_add(fake->timeout, ps3remote_idle, fake);
+
 	if (cond & G_IO_NVAL)
 		return FALSE;
 
@@ -337,6 +351,10 @@ static gboolean fake_hid_common_connect(struct fake_input *fake, GError **err)
 
 static int fake_hid_common_disconnect(struct fake_input *fake)
 {
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
 	return 0;
 }
 
-- 
1.7.8.6

Re: [PATCH] input: Implement idle timeout for fakehid

[Bastien Nocera]

On Mon, 2012-08-13 at 11:40 -0600, Jeff Hansen wrote:
> (bump)
> 
> The batteries on my BD remote would die in a few days without this
> patch.  The idle timeout needs to be enforced for fakehid devices as
> well as normal HID devices.

Except that the timeout is disabled by default for HID devices. I posted
a patch that requires a little bit of work to get pushed last month or
so.

Re: [PATCH] input: Implement idle timeout for fakehid

[Jeff Hansen]

On 08/14/12 03:50, Bastien Nocera wrote:
> On Mon, 2012-08-13 at 11:40 -0600, Jeff Hansen wrote:
>> (bump)
>>
>> The batteries on my BD remote would die in a few days without this
>> patch.  The idle timeout needs to be enforced for fakehid devices as
>> well as normal HID devices.
> Except that the timeout is disabled by default for HID devices. I posted
> a patch that requires a little bit of work to get pushed last month or
> so.
>
Yes, of course it is disabled by default (which I don't understand, but 
that's a separate issue).  That is fine, but if a general timeout *is* 
specified by the user, it should carry into fakehid devices as well as 
normal HID devices.

Are there any bluez maintainers that can take a look at this patch?  
Anything that needs to change?

-Jeff

-- 
---------------------------------------------------
"If someone's gotta do it, it might as well be me."
                 x@jeffhansen.com

[PATCH] input: Implement idle timeout for fakehid

[Jeff Hansen]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

The batteries on my BD remote would die in a few days without this patch.


[-- Attachment #2: 0001-input-Implement-idle-timeout-for-fakehid.patch --]
[-- Type: text/x-patch, Size: 2949 bytes --]

>From a96f67cb6268e34e677e5d288f2c3b89acfca7fe Mon Sep 17 00:00:00 2001
From: Jeff Hansen <x@jeffhansen.com>
Date: Tue, 31 Jul 2012 22:45:23 -0600
Subject: [PATCH] input: Implement idle timeout for fakehid.

The batteries on my BD remote would die in a few days without this patch.

Signed-off-by: Jeff Hansen <x@jeffhansen.com>
---
 input/device.c  |    6 ++++++
 input/device.h  |    4 ++++
 input/fakehid.c |   18 ++++++++++++++++++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/input/device.c b/input/device.c
index 0ab63c0..ae11a13 100644
--- a/input/device.c
+++ b/input/device.c
@@ -609,6 +609,7 @@ static int hidp_add_connection(const struct input_device *idev,
 		fake->disconnect = fake_hid_disconnect;
 		fake->priv = fake_hid;
 		fake->idev = idev;
+		fake->timeout = iconn->timeout * 1000;
 		fake = fake_hid_connadd(fake, iconn->intr_io, fake_hid);
 		if (fake == NULL)
 			err = -ENOMEM;
@@ -1257,3 +1258,8 @@ int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst)
 
 	return 0;
 }
+
+void input_device_request_disconnect(const struct input_device *idev)
+{
+	device_request_disconnect(idev->device, NULL);
+}
diff --git a/input/device.h b/input/device.h
index 14c0f97..416cc9c 100644
--- a/input/device.h
+++ b/input/device.h
@@ -40,6 +40,8 @@ struct fake_input {
 	int		(*disconnect) (struct input_conn *iconn);
 	void		*priv;
 	const struct input_device *idev;
+	guint		idle_timeout;
+	uint32_t	timeout;
 };
 
 int fake_input_register(DBusConnection *conn, struct btd_device *device,
@@ -54,3 +56,5 @@ int input_device_unregister(const char *path, const char *uuid);
 int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm,
 							GIOChannel *io);
 int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst);
+
+void input_device_request_disconnect(const struct input_device *idev);
diff --git a/input/fakehid.c b/input/fakehid.c
index b809870..0407ae3 100644
--- a/input/fakehid.c
+++ b/input/fakehid.c
@@ -208,6 +208,13 @@ error:
 	return -1;
 }
 
+static gboolean ps3remote_idle(gpointer data)
+{
+	struct fake_input *fake = data;
+	input_device_request_disconnect(fake->idev);
+	return FALSE;
+}
+
 static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 				gpointer data)
 {
@@ -218,6 +225,13 @@ static gboolean ps3remote_event(GIOChannel *chan, GIOCondition cond,
 	char buff[50];
 	int fd;
 
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
+	if (fake->timeout)
+		fake->idle_timeout = g_timeout_add(fake->timeout, ps3remote_idle, fake);
+
 	if (cond & G_IO_NVAL)
 		return FALSE;
 
@@ -337,6 +351,10 @@ static gboolean fake_hid_common_connect(struct fake_input *fake, GError **err)
 
 static int fake_hid_common_disconnect(struct fake_input *fake)
 {
+	if (fake->idle_timeout) {
+		g_source_remove(fake->idle_timeout);
+		fake->idle_timeout = 0;
+	}
 	return 0;
 }
 
-- 
1.7.8.6