[BUG] ir-ctl: error sending file with multiple scancodes

[BUG] ir-ctl: error sending file with multiple scancodes

[Matthias Reichl]

Hi Sean!

According to the ir-ctl manpage it should be possible to send a file
containing multiple scancodes, but when trying to do this I get
a warning and an error message.

I initially noticed that on version 1.12.3 but 1.12.5 and master
(rev 85f8e5a99) give the same error.

Sending a file with a single scancode or using the -S option
to specify the scancode on the command line both work fine.

I've tested with the following file:

scancode sony12:0x100015
space 25000
scancode sony12:0x100015

Trying to send it gives this:
$ ./utils/ir-ctl/ir-ctl -s ../sony-test.irctl
warning: ../sony-test.irctl:2: trailing space ignored
/dev/lirc0: failed to send: Invalid argument

Checking with the -v option gives some interesting output - it
looks like the the second half of the buffer hadn't been filled in:

$ ./utils/ir-ctl/ir-ctl -v -s ../sony-test.irctl
warning: ../sony-test.irctl:2: trailing space ignored
Sending:
pulse 2400
space 600
pulse 1200
space 600
pulse 600
space 600
pulse 1200
space 600
pulse 600
space 600
pulse 1200
space 600
pulse 600
space 600
pulse 600
space 600
pulse 600
space 600
pulse 600
space 600
pulse 600
space 600
pulse 600
space 600
pulse 1200
space 600
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
space 0
pulse 0
/dev/lirc0: failed to send: Invalid argument

The goal I'm trying to achieve is to send a repeated signal with ir-ctl
(a user reported his sony receiver needs this to actually power up).

Using the -S option multiple times comes rather close, but the 125ms
delay between signals is a bit long for the sony protocol - would be
nice if that would be adjustable :)

so long,

Hias

Re: [BUG] ir-ctl: error sending file with multiple scancodes

[Sean Young]

Hi Matthias,

On Wed, Nov 29, 2017 at 03:44:00PM +0100, Matthias Reichl wrote:
> Hi Sean!
> 
> According to the ir-ctl manpage it should be possible to send a file
> containing multiple scancodes, but when trying to do this I get
> a warning and an error message.
> 
> I initially noticed that on version 1.12.3 but 1.12.5 and master
> (rev 85f8e5a99) give the same error.
> 
> Sending a file with a single scancode or using the -S option
> to specify the scancode on the command line both work fine.
> 
> I've tested with the following file:
> 
> scancode sony12:0x100015
> space 25000
> scancode sony12:0x100015
> 
> Trying to send it gives this:
> $ ./utils/ir-ctl/ir-ctl -s ../sony-test.irctl
> warning: ../sony-test.irctl:2: trailing space ignored
> /dev/lirc0: failed to send: Invalid argument
> 
> Checking with the -v option gives some interesting output - it
> looks like the the second half of the buffer hadn't been filled in:
> 
> $ ./utils/ir-ctl/ir-ctl -v -s ../sony-test.irctl
> warning: ../sony-test.irctl:2: trailing space ignored
> Sending:
> pulse 2400
> space 600
> pulse 1200
> space 600
> pulse 600
> space 600
> pulse 1200
> space 600
> pulse 600
> space 600
> pulse 1200
> space 600
> pulse 600
> space 600
> pulse 600
> space 600
> pulse 600
> space 600
> pulse 600
> space 600
> pulse 600
> space 600
> pulse 600
> space 600
> pulse 1200
> space 600
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> space 0
> pulse 0
> /dev/lirc0: failed to send: Invalid argument

Oh dear, that looks very broken! Looks like I did not test multiple
scancodes in one file.

> The goal I'm trying to achieve is to send a repeated signal with ir-ctl
> (a user reported his sony receiver needs this to actually power up).

That's interesting.

> Using the -S option multiple times comes rather close, but the 125ms
> delay between signals is a bit long for the sony protocol - would be
> nice if that would be adjustable :)

Yes, that would be a useful feature.

I've got some patches for this, I'll send them as a reply to this. Please
let me know what you think.

Thanks,

Sean

[PATCH 1/2] ir-ctl: fix multiple scancodes in one file

[Sean Young]

A file with contents:

scancode sony12:0x100015
space 25000
scancode sony12:0x100015

Will produce bogus results.

Reported-by: Matthias Reichl <hias@horus.com>
Signed-off-by: Sean Young <sean@mess.org>
---
 utils/ir-ctl/ir-ctl.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/utils/ir-ctl/ir-ctl.c b/utils/ir-ctl/ir-ctl.c
index 544ad341..8538ec5d 100644
--- a/utils/ir-ctl/ir-ctl.c
+++ b/utils/ir-ctl/ir-ctl.c
@@ -230,8 +230,8 @@ static struct file *read_file(const char *fname)
 			char *scancodestr;
 
 			if (!expect_pulse) {
-				fprintf(stderr, _("error: %s:%d: space must precede scancode\n"), fname, lineno);
-				return NULL;
+				f->buf[len++] = IR_DEFAULT_TIMEOUT;
+				expect_pulse = true;
 			}
 
 			scancodestr = strchr(p, ':');
@@ -268,7 +268,8 @@ static struct file *read_file(const char *fname)
 			else
 				f->carrier = carrier;
 
-			len += protocol_encode(proto, scancode, f->buf);
+			len += protocol_encode(proto, scancode, f->buf + len);
+			expect_pulse = false;
 			continue;
 		}
 
-- 
2.14.3

[PATCH 2/2] ir-ctl: specify the gap between scancodes or files

[Sean Young]

When sending multiple scancodes, or pulse space files, by default there
is 125ms gap between them. Allow this to be specified.

Signed-off-by: Sean Young <sean@mess.org>
---
 utils/ir-ctl/ir-ctl.1.in |  5 +++++
 utils/ir-ctl/ir-ctl.c    | 18 +++++++++++++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/utils/ir-ctl/ir-ctl.1.in b/utils/ir-ctl/ir-ctl.1.in
index 05550fb1..89aa281f 100644
--- a/utils/ir-ctl/ir-ctl.1.in
+++ b/utils/ir-ctl/ir-ctl.1.in
@@ -93,6 +93,11 @@ Comma separated list of emitters to use for sending. The first emitter is
 number 1. Some devices only support enabling one emitter (the winbond-cir
 driver).
 .TP
+\fB\-g\fR, \fB\-\-gap\fR=\fIGAP\fR
+Set the gap time between scancodes, or the gap between files when
+multiple are specified on the command line. By default this is 125000
+microseconds.
+.TP
 \fB\-?\fR, \fB\-\-help\fR
 Prints the help message
 .TP
diff --git a/utils/ir-ctl/ir-ctl.c b/utils/ir-ctl/ir-ctl.c
index 8538ec5d..6fb05b1a 100644
--- a/utils/ir-ctl/ir-ctl.c
+++ b/utils/ir-ctl/ir-ctl.c
@@ -82,6 +82,7 @@ struct arguments {
 	int wideband;
 	unsigned carrier_low, carrier_high;
 	unsigned timeout;
+	unsigned gap;
 	int carrier_reports;
 	int timeout_reports;
 	unsigned carrier;
@@ -111,6 +112,7 @@ static const struct argp_option options[] = {
 	{ "carrier",	'c',	N_("CARRIER"),	0,	N_("set send carrier") },
 	{ "duty-cycle",	'D',	N_("DUTY"),	0,	N_("set duty cycle") },
 	{ "emitters",	'e',	N_("EMITTERS"),	0,	N_("set send emitters") },
+	{ "gap",	'g',	N_("GAP"),	0,	N_("set gap between files or scancodes") },
 	{ }
 };
 
@@ -130,6 +132,7 @@ static const char doc[] = N_(
 	"  CARRIER  - the carrier frequency to use for sending\n"
 	"  DUTY     - the duty cycle to use for sending\n"
 	"  EMITTERS - comma separated list of emitters to use for sending, e.g. 1,2\n"
+	"  GAP      - gap between pulse and files or scancodes in microseconds\n"
 	"  RANGE    - set range of accepted carrier frequencies, e.g. 20000-40000\n"
 	"  TIMEOUT  - set length of space before recording stops in microseconds\n"
 	"  SCANCODE - protocol:scancode, e.g. nec:0xa814\n\n"
@@ -185,7 +188,7 @@ static unsigned parse_emitters(char *p)
 	return emit;
 }
 
-static struct file *read_file(const char *fname)
+static struct file *read_file(struct arguments *args, const char *fname)
 {
 	bool expect_pulse = true;
 	int lineno = 0, lastspace = 0;
@@ -230,7 +233,7 @@ static struct file *read_file(const char *fname)
 			char *scancodestr;
 
 			if (!expect_pulse) {
-				f->buf[len++] = IR_DEFAULT_TIMEOUT;
+				f->buf[len++] = args->gap;
 				expect_pulse = true;
 			}
 
@@ -486,6 +489,11 @@ static error_t parse_opt(int k, char *arg, struct argp_state *state)
 		if (arguments->emitters == 0)
 			argp_error(state, _("cannot parse emitters `%s'"), arg);
 		break;
+	case 'g':
+		arguments->gap = strtoint(arg, "");
+		if (arguments->gap == 0)
+			argp_error(state, _("cannot parse gap `%s'"), arg);
+		break;
 	case 'D':
 		arguments->duty = strtoint(arg, "%");
 		if (arguments->duty == 0 || arguments->duty >= 100)
@@ -494,7 +502,7 @@ static error_t parse_opt(int k, char *arg, struct argp_state *state)
 	case 's':
 		if (arguments->record || arguments->features)
 			argp_error(state, _("send can not be combined with record or features option"));
-		s = read_file(arg);
+		s = read_file(arguments, arg);
 		if (s == NULL)
 			exit(EX_DATAERR);
 
@@ -884,7 +892,7 @@ err:
 
 int main(int argc, char *argv[])
 {
-	struct arguments args = {};
+	struct arguments args = { .gap = IR_DEFAULT_TIMEOUT };
 
 #ifdef ENABLE_NLS
         setlocale (LC_ALL, "");
@@ -912,7 +920,7 @@ int main(int argc, char *argv[])
 	while (s) {
 		struct file *next = s->next;
 		if (s != args.send)
-			usleep(IR_DEFAULT_TIMEOUT);
+			usleep(args.gap);
 
 		rc = lirc_send(&args, fd, features, s);
 		if (rc) {
-- 
2.14.3

Re: [BUG] ir-ctl: error sending file with multiple scancodes

[Matthias Reichl]

Hi Sean,

On Wed, Nov 29, 2017 at 08:05:21PM +0000, Sean Young wrote:
> On Wed, Nov 29, 2017 at 03:44:00PM +0100, Matthias Reichl wrote:
> > The goal I'm trying to achieve is to send a repeated signal with ir-ctl
> > (a user reported his sony receiver needs this to actually power up).
> 
> That's interesting.

I'm not sure how common that is. I've got a Panasonic TV here
that needs a 0.5-1sec button press to power up from standby,
but it could well be that this is a rather nieche issue.

I had a look at what it would take to add proper repeat handling
to ir-ctl (similar to the --count <NUMBER> option in lirc's irsend)
but that looks like a larger endeavour: implement automatic
variable length gaps to get fixed repeat times, handle toggle
bits in some protocols, send special repeat codes eg in NEC etc.

As I'm not sure if all of this is even needed I think it's best
to leave it for maybe some time later. For now the current
functionality in ir-ctl looks sufficient.

> > Using the -S option multiple times comes rather close, but the 125ms
> > delay between signals is a bit long for the sony protocol - would be
> > nice if that would be adjustable :)
> 
> Yes, that would be a useful feature.
> 
> I've got some patches for this, I'll send them as a reply to this. Please
> let me know what you think.

[PATCH 1/2] ir-ctl: fix multiple scancodes in one file 01-multiple-scancodes.patch
[PATCH 2/2] ir-ctl: specify the gap between scancodes or files

Tested-by: Matthias Reichl <hias@horus.com>

Thanks, the patches look and tested fine!

I've tested multiple scancodes in a file with and without explicit
space in between and the gap option with multiple -S scancodes on
the command line. I also tested rc5 protocol in addition to sony12.

So I'd like to say a big thank you for fixing the issue so quickly!

so long,

Hias

Re: [BUG] ir-ctl: error sending file with multiple scancodes

[Sean Young]

Hi Matthias,

On Thu, Nov 30, 2017 at 04:34:33PM +0100, Matthias Reichl wrote:
> Hi Sean,
> 
> On Wed, Nov 29, 2017 at 08:05:21PM +0000, Sean Young wrote:
> > On Wed, Nov 29, 2017 at 03:44:00PM +0100, Matthias Reichl wrote:
> > > The goal I'm trying to achieve is to send a repeated signal with ir-ctl
> > > (a user reported his sony receiver needs this to actually power up).
> > 
> > That's interesting.
> 
> I'm not sure how common that is. I've got a Panasonic TV here
> that needs a 0.5-1sec button press to power up from standby,
> but it could well be that this is a rather nieche issue.
> 
> I had a look at what it would take to add proper repeat handling
> to ir-ctl (similar to the --count <NUMBER> option in lirc's irsend)
> but that looks like a larger endeavour: implement automatic
> variable length gaps to get fixed repeat times, handle toggle
> bits in some protocols, send special repeat codes eg in NEC etc.

Yes, I've thought about that too. I'm not sure what the user inferface
should look like (and how useful it is).

> As I'm not sure if all of this is even needed I think it's best
> to leave it for maybe some time later. For now the current
> functionality in ir-ctl looks sufficient.

If you have any suggestions. :-)

> > > Using the -S option multiple times comes rather close, but the 125ms
> > > delay between signals is a bit long for the sony protocol - would be
> > > nice if that would be adjustable :)
> > 
> > Yes, that would be a useful feature.
> > 
> > I've got some patches for this, I'll send them as a reply to this. Please
> > let me know what you think.
> 
> [PATCH 1/2] ir-ctl: fix multiple scancodes in one file 01-multiple-scancodes.patch
> [PATCH 2/2] ir-ctl: specify the gap between scancodes or files
> 
> Tested-by: Matthias Reichl <hias@horus.com>
> 
> Thanks, the patches look and tested fine!
> 
> I've tested multiple scancodes in a file with and without explicit
> space in between and the gap option with multiple -S scancodes on
> the command line. I also tested rc5 protocol in addition to sony12.
> 
> So I'd like to say a big thank you for fixing the issue so quickly!

Thanks for making ir-ctl a better tool :)


Sean