Lirc codec and starting "space" event

Lirc codec and starting "space" event

[Austin Lund]

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

Hi,

I've been having a problem with a GPIO ir device in an i.mx6 arm
system that I have (cubox-i).

It seems to all work ok, except the output on /dev/lirc0 is not quite
what lircd seems to expect.  Lircd wants a long space before the
starting pulse before processing any output. However, no long space is
sent when I check the output (doing "mode2" and a plain hexdump
/dev/lirc0).

This causes problems in detecting button presses on remotes.
Sometimes it works if you press the buttons quick enough, but after
waiting a while it doesn't work.

I have been looking at the code for a while now, and it seems that it
has something to do with the lirc codec ignoring reset events (just
returns 0).

I've made up this patch, but I'm travelling at the moment and haven't
had a chance to actually test it.

What I'm wondering is if this issue is known, and if my approach is
going down the right path.

The only alternative I could see is to change the way the gpio ir
driver handles events.  It seems to just call ir_raw_event_store_edge
which put a zeroed reset event into the queue.  I'm assuming there are
other users of these functions and that it's probably best not to
fiddle with that if possible.

Thanks.

PS Please CC me as I'm not subscribed.

[-- Attachment #2: 0001-media-rc-Send-sync-space-information-on-the-lirc-dev.patch --]
[-- Type: text/x-patch, Size: 1796 bytes --]

From 49c041e6ab7a9d5fcbe87817d5e819c2aef6b3ac Mon Sep 17 00:00:00 2001
From: Austin Lund <austin.lund@gmail.com>
Date: Mon, 31 Mar 2014 14:52:47 +1000
Subject: [PATCH] media/rc: Send sync space information on the lirc device.

Userspace expects to see a long space before the first pulse is sent on
the lirc device.  Currently, if a long time has passed and a new packet
is started, the lirc codec just returns and doesn't send anything.  This
makes lircd ignore many perfectly valid signals unless they are sent in
quick sucession.  When a reset event is delivered, we cannot know
anything about the duration of the space.  But it should be safe to
assume it has been a long time and we just set the duration to maximum.
---
 drivers/media/rc/ir-lirc-codec.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index e456126..a895ed0 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -42,11 +42,17 @@ static int ir_lirc_decode(struct rc_dev *dev, struct ir_raw_event ev)
 		return -EINVAL;
 
 	/* Packet start */
-	if (ev.reset)
-		return 0;
+	if (ev.reset) {
+		/* Userspace expects a long space event before the start of
+		 * the signal to use as a sync.  This may be done with repeat
+		 * packets and normal samples.  But if a reset has been sent
+		 * then we assume that a long time has passed, so we send a
+		 * space with the maximum time value. */
+		sample = LIRC_SPACE(LIRC_VALUE_MASK);
+		IR_dprintk(2, "delivering reset sync space to lirc_dev\n");
 
 	/* Carrier reports */
-	if (ev.carrier_report) {
+	} else if (ev.carrier_report) {
 		sample = LIRC_FREQUENCY(ev.carrier);
 		IR_dprintk(2, "carrier report (freq: %d)\n", sample);
 
-- 
1.9.1

Re: Lirc codec and starting "space" event

[Austin Lund]

On 1 April 2014 10:04, Austin Lund <austin.lund@gmail.com> wrote:
> Hi,
>
> I've been having a problem with a GPIO ir device in an i.mx6 arm
> system that I have (cubox-i).
>
> It seems to all work ok, except the output on /dev/lirc0 is not quite
> what lircd seems to expect.  Lircd wants a long space before the
> starting pulse before processing any output. However, no long space is
> sent when I check the output (doing "mode2" and a plain hexdump
> /dev/lirc0).
>
> This causes problems in detecting button presses on remotes.
> Sometimes it works if you press the buttons quick enough, but after
> waiting a while it doesn't work.
>
> I have been looking at the code for a while now, and it seems that it
> has something to do with the lirc codec ignoring reset events (just
> returns 0).
>
> I've made up this patch, but I'm travelling at the moment and haven't
> had a chance to actually test it.
>
> What I'm wondering is if this issue is known, and if my approach is
> going down the right path.
>
> The only alternative I could see is to change the way the gpio ir
> driver handles events.  It seems to just call ir_raw_event_store_edge
> which put a zeroed reset event into the queue.  I'm assuming there are
> other users of these functions and that it's probably best not to
> fiddle with that if possible.
>
> Thanks.
>
> PS Please CC me as I'm not subscribed.

Just a note that I have tested this patch now and it works.  No idea
what impact it might have on other users.

Re: Lirc codec and starting "space" event

[Mauro Carvalho Chehab]

Em Tue, 1 Apr 2014 10:04:16 +1000
Austin Lund <austin.lund@gmail.com> escreveu:

> Hi,
> 
> I've been having a problem with a GPIO ir device in an i.mx6 arm
> system that I have (cubox-i).
> 
> It seems to all work ok, except the output on /dev/lirc0 is not quite
> what lircd seems to expect.  Lircd wants a long space before the
> starting pulse before processing any output. However, no long space is
> sent when I check the output (doing "mode2" and a plain hexdump
> /dev/lirc0).
> 
> This causes problems in detecting button presses on remotes.
> Sometimes it works if you press the buttons quick enough, but after
> waiting a while it doesn't work.
> 
> I have been looking at the code for a while now, and it seems that it
> has something to do with the lirc codec ignoring reset events (just
> returns 0).
> 
> I've made up this patch, but I'm travelling at the moment and haven't
> had a chance to actually test it.
> 
> What I'm wondering is if this issue is known, and if my approach is
> going down the right path.
> 
> The only alternative I could see is to change the way the gpio ir
> driver handles events.  It seems to just call ir_raw_event_store_edge
> which put a zeroed reset event into the queue.  I'm assuming there are
> other users of these functions and that it's probably best not to
> fiddle with that if possible.
> 
> Thanks.
> 
> PS Please CC me as I'm not subscribed.

You forgot to add a:

Signed-off-by: your name <your@email>

on your patch. Please add it and resend.

Thanks!
Mauro

[PATCH] media/rc: Send sync space information on the lirc device.

[Austin Lund]

Userspace expects to see a long space before the first pulse is sent on
the lirc device.  Currently, if a long time has passed and a new packet
is started, the lirc codec just returns and doesn't send anything.  This
makes lircd ignore many perfectly valid signals unless they are sent in
quick sucession.  When a reset event is delivered, we cannot know
anything about the duration of the space.  But it should be safe to
assume it has been a long time and we just set the duration to maximum.

Signed-off-by: Austin Lund <austin.lund@gmail.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/rc/ir-lirc-codec.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index d731da6..2af2326 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -42,11 +42,17 @@ static int ir_lirc_decode(struct rc_dev *dev, struct ir_raw_event ev)
 		return -EINVAL;
 
 	/* Packet start */
-	if (ev.reset)
-		return 0;
+	if (ev.reset) {
+		/* Userspace expects a long space event before the start of
+		 * the signal to use as a sync.  This may be done with repeat
+		 * packets and normal samples.  But if a reset has been sent
+		 * then we assume that a long time has passed, so we send a
+		 * space with the maximum time value. */
+		sample = LIRC_SPACE(LIRC_VALUE_MASK);
+		IR_dprintk(2, "delivering reset sync space to lirc_dev\n");
 
 	/* Carrier reports */
-	if (ev.carrier_report) {
+	} else if (ev.carrier_report) {
 		sample = LIRC_FREQUENCY(ev.carrier);
 		IR_dprintk(2, "carrier report (freq: %d)\n", sample);
 
-- 
2.0.2