DVB: TechnoTrend CT-3650 IR support

From 705adeab4da152cf24cede069b724765b8a07d55 Mon Sep 17 00:00:00 2001

Hi Linux-media,

As a spare time project I bought myself a TT CT-3650, to see if I could get it working. Waling Dijkstra did write a IR & CI patch for this model half a year ago, so I was hopeful. (Reference: http://www.mail-archive.com/[email protected]/msg19860.html )

Having tested the patch, the IR is working (tested all keys via the "evtest" tool), however descrambling is NOT working.

Waling's patch was reviewed but never merged. So I have taken the IR part of the patch, cleaned it up a little, and hopefully this part is ready for merging now. Patch is against linux-2.6.git.

Hi David,

Em 26-12-2010 07:14, David Henningsson escreveu:

Quoted 2 lines

Hi Linux-media,

Quoted 5 lines

As a spare time project I bought myself a TT CT-3650, to see if I could get it working. Waling Dijkstra did write a IR& CI patch for this model half a year ago, so I was hopeful. (Reference: http://www.mail-archive.com/[email protected]/msg19860.html )

Having tested the patch, the IR is working (tested all keys via the "evtest" tool), however descrambling is NOT working.

Waling's patch was reviewed but never merged. So I have taken the IR part of the patch, cleaned it up a little, and hopefully this part is ready for merging now. Patch is against linux-2.6.git.

Could you please rebase it to work with the rc_core support? I suspect that you based it on a kernel older than .36, as the dvb_usb rc struct changed.

The better is to base it over the latest V4L/DVB/RC development git, available at:
http://git.linuxtv.org/media_tree.git

From 8c42121a08c5dabbd1a943cc1e5726ed99f0d957 Mon Sep 17 00:00:00 2001

Quoted 5 lines

Hi David,

Em 26-12-2010 07:14, David Henningsson escreveu:

Quoted 2 lines

Hi Linux-media,

Quoted 8 lines

Quoted 5 lines

As a spare time project I bought myself a TT CT-3650, to see if I could get it working. Waling Dijkstra did write a IR& CI patch for this model half a year ago, so I was hopeful. (Reference: http://www.mail-archive.com/[email protected]/msg19860.html )

Having tested the patch, the IR is working (tested all keys via the "evtest" tool), however descrambling is NOT working.

Waling's patch was reviewed but never merged. So I have taken the IR part of the patch, cleaned it up a little, and hopefully this part is ready for merging now. Patch is against linux-2.6.git.

Could you please rebase it to work with the rc_core support? I suspect that you based it on a kernel older than .36, as the dvb_usb rc struct changed.

Ok, I have now done this, but I'm not completely satisfied, perhaps you can help out a little? I'm new to IR/RC stuff, but I feel I'm missing correct "repeat" functionality, i e, if you keep a key pressed it appears as separate key presses with whatever interval set as .rc_interval. (This was probably a problem with the old patch as well.) Is there any support for this is rc_core?

From your decode logic, I suspect that the IR hardware decoder has its own logic for repeat.

I'm attaching a temporary patch (just for review) so you know what I'm talking about.

Quoted 2 lines

The better is to base it over the latest V4L/DVB/RC development git, available at:
http://git.linuxtv.org/media_tree.git

Ok. I was probably confused with this entry: http://linuxtv.org/news.php?entry=2010-01-19.mchehab
telling me to base it on v4l-dvb.git, which is not updated for four months. However, http://git.linuxtv.org/ correctly lists the media_tree.git as the repository of choice.

Thanks for the review!

From 8c42121a08c5dabbd1a943cc1e5726ed99f0d957 Mon Sep 17 00:00:00 2001
From: David Henningsson <[email protected]>
Date: Sun, 26 Dec 2010 14:23:58 +0100
Subject: [PATCH] DVB: IR support for CT-3650

Signed-off-by David Henningsson <[email protected]>

---
 drivers/media/dvb/dvb-usb/ttusb2.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 debian/rules

debian/rules +0 -0

old mode 100644
new mode 100755

drivers/media/dvb/dvb-usb/ttusb2.c +3 -0

index a6de489..ded8a4b 100644
@@ -128,6 +128,27 @@ static struct i2c_algorithm ttusb2_i2c_algo = {
 	.functionality = ttusb2_i2c_func,
 };
 
+/* command to poll IR receiver (copied from pctv452e.c) */
+#define CMD_GET_IR_CODE     0x1b
+

drivers/media/dvb/dvb-usb/ttusb2.c +4 -0

+	return 0;
+}
+
+
 /* Callbacks for DVB USB */
 static int ttusb2_identify_state (struct usb_device *udev, struct
 		dvb_usb_device_properties *props, struct dvb_usb_device_description **desc,

Em 26-12-2010 17:38, David Henningsson escreveu:

Quoted 12 lines · Mauro Carvalho Chehab

Quoted 11 lines

Hi David,

Em 26-12-2010 07:14, David Henningsson escreveu:

Quoted 7 lines

Hi Linux-media,

As a spare time project I bought myself a TT CT-3650, to see if I could get it working. Waling Dijkstra did write a IR& CI patch for this model half a year ago, so I was hopeful. (Reference: http://www.mail-archive.com/[email protected]/msg19860.html )

Having tested the patch, the IR is working (tested all keys via the "evtest" tool), however descrambling is NOT working.

Waling's patch was reviewed but never merged. So I have taken the IR part of the patch, cleaned it up a little, and hopefully this part is ready for merging now. Patch is against linux-2.6.git.

Quoted 7 lines

Quoted 2 lines

Could you please rebase it to work with the rc_core support? I suspect that you based it on a kernel older than .36, as the dvb_usb rc struct changed.

Ok, I have now done this, but I'm not completely satisfied, perhaps you can help out a little? I'm new to IR/RC stuff, but I feel I'm missing correct "repeat" functionality, i e, if you keep a key pressed it appears as separate key presses with whatever interval set as .rc_interval. (This was probably a problem with the old patch as well.) Is there any support for this is rc_core?

From your decode logic, I suspect that the IR hardware decoder has its own logic for repeat.
In this case, there's not much you can do, as it probably uses a very high time for repeat.

Quoted 7 lines

I'm attaching a temporary patch (just for review) so you know what I'm talking about.

Quoted 2 lines

The better is to base it over the latest V4L/DVB/RC development git, available at:
http://git.linuxtv.org/media_tree.git

Ok. I was probably confused with this entry: http://linuxtv.org/news.php?entry=2010-01-19.mchehab
telling me to base it on v4l-dvb.git, which is not updated for four months. However, http://git.linuxtv.org/ correctly lists the media_tree.git as the repository of choice.

Ah... yeah, old news;)

Quoted 2 lines

Thanks for the review!

Em 26-12-2010 17:38, David Henningsson escreveu:

Quoted 22 lines

From 8c42121a08c5dabbd1a943cc1e5726ed99f0d957 Mon Sep 17 00:00:00 2001
From: David Henningsson<[email protected]>
Date: Sun, 26 Dec 2010 14:23:58 +0100
Subject: [PATCH] DVB: IR support for CT-3650

Signed-off-by David Henningsson<[email protected]>

---
  drivers/media/dvb/dvb-usb/ttusb2.c |   28 ++++++++++++++++++++++++++++
  1 files changed, 28 insertions(+), 0 deletions(-)

mode change 100644 => 100755 debian/rules

debian/rules +0 -0

old mode 100644
new mode 100755

drivers/media/dvb/dvb-usb/ttusb2.c +0 -0

index a6de489..ded8a4b 100644
@@ -128,6 +128,27 @@ static struct i2c_algorithm ttusb2_i2c_algo = {
  	.functionality = ttusb2_i2c_func,
  };
 

drivers/media/dvb/dvb-usb/ttusb2.c static struct i2c_algorithm ttusb2_i2c_algo = {

Quoted 13 lines

drivers/media/dvb/dvb-usb/ttusb2.c +13 -0

+/* command to poll IR receiver (copied from pctv452e.c) */
+#define CMD_GET_IR_CODE     0x1b
+
+/* IR */
+static int tt3650_rc_query(struct dvb_usb_device *d)
+{
+	int ret;
+	u8 rx[9]; /* A CMD_GET_IR_CODE reply is 9 bytes long */
+	ret = ttusb2_msg(d, CMD_GET_IR_CODE, NULL, 0, rx, sizeof(rx));
+	if (ret != 0)
+		return ret;
+
+	if (rx[8]&  0x01) {

Maybe (rx[8]& 0x01) == 0 indicates a keyup event. If so, if you map both keydown and keyup events, the in-kernel repeat logic will work.

drivers/media/dvb/dvb-usb/ttusb2.c static struct i2c_algorithm ttusb2_i2c_algo = { · line 147

Quoted 5 lines

drivers/media/dvb/dvb-usb/ttusb2.c +4 -0

+		/* got a "press" event */
+		deb_info("%s: cmd=0x%02x sys=0x%02x\n", __func__, rx[2], rx[3]);
+		rc_keydown(d->rc_dev, rx[2], 0);
+	}

As you're receiving both command+address, please use the complete code:

	rc_keydown(d->rc_dev, (rx[3]<<  8) | rx[2], 0);

Also as it is receiving 8 bytes from the device, maybe the IR decoding logic is capable of decoding more than just one protocol. Such feature is nice, as it allows replacing the original keycode table by a more complete one.

One of the most interesting features of the new RC code is that it offers a sysfs class and some additional logic to allow dynamically change/replace the keymaps and keycodes via userspace. The idea is to remove all in-kernel keymaps in the future, using, instead, the userspace way, via ir-keytable tool, available at:
http://git.linuxtv.org/v4l-utils.git

The tool already supports auto-loading the keymap via udev.

For IR's where we don't know the protocol or that we don't have the full scancode, loading the keymap via userspace will not bring any new feature. But, for those devices where we can be sure about the protocol and for those that also allow using other protocols, users can just replace the device-provided IR with a more powerful remote controller with more keys.

So, it would be wonderful if you could identify what's the supported protocol(s) instead of using RC_TYPE_UNKNOWN. You can double check the protocol if you have with you another RC device that supports raw decoding. The rc-core internal decoders will tell you what protocol was used to decode a keycode, if you enable debug.

Quoted 19 lines

Em 26-12-2010 17:38, David Henningsson escreveu:

Quoted 5 lines · Mauro Carvalho Chehab

drivers/media/dvb/dvb-usb/ttusb2.c +3 -0

+/* command to poll IR receiver (copied from pctv452e.c) */
+#define CMD_GET_IR_CODE     0x1b
+

drivers/media/dvb/dvb-usb/ttusb2.c static struct i2c_algorithm ttusb2_i2c_algo = {

Quoted 10 lines

drivers/media/dvb/dvb-usb/ttusb2.c +10 -0

+/* IR */
+static int tt3650_rc_query(struct dvb_usb_device *d)
+{
+    int ret;
+    u8 rx[9]; /* A CMD_GET_IR_CODE reply is 9 bytes long */
+    ret = ttusb2_msg(d, CMD_GET_IR_CODE, NULL, 0, rx, sizeof(rx));
+    if (ret != 0)
+        return ret;
+
+    if (rx[8]&  0x01) {

Maybe (rx[8]& 0x01) == 0 indicates a keyup event. If so, if you map both keydown and keyup events, the in-kernel repeat logic will work.

Hmm. If I should fix keyup events, the most reliable version would probably be something like:

if (rx[8] & 0x01) {
int currentkey = rx[2]; // or (rx[3]<< 8) | rx[2]; if (currentkey == lastkey)

    rc_repeat(lastkey);
  else {
    if (lastkey)
      rc_keyup(lastkey);
    lastkey = currentkey;
    rc_keydown(currentkey);

}

}
else if (lastkey) {
rc_keyup(lastkey);
lastkey = 0;
}

Does this sound reasonable to you?

Quoted 6 lines

Quoted 5 lines

drivers/media/dvb/dvb-usb/ttusb2.c +4 -0

+        /* got a "press" event */
+        deb_info("%s: cmd=0x%02x sys=0x%02x\n", __func__, rx[2], rx[3]);
+        rc_keydown(d->rc_dev, rx[2], 0);
+    }

Quoted 2 lines

As you're receiving both command+address, please use the complete code:

    rc_keydown(d->rc_dev, (rx[3]<<  8) | rx[2], 0);

I've tried this, but it stops working. evtest shows only scancode events, so my guess is that this makes it incompatible with RC_MAP_TT_1500, which lists only the lower byte.

Quoted 3 lines

Also as it is receiving 8 bytes from the device, maybe the IR decoding logic is capable of decoding more than just one protocol. Such feature is nice, as it allows replacing the original keycode table by a more complete one.

I've tried dumping all nine bytes but I can't make much out of it as I'm unfamiliar with RC protocols and decoders.

Typical reply is (no key pressed):

cc 35 0b 15 00 03 00 00 00

Does this tell you anything?

Quoted 7 lines

One of the most interesting features of the new RC code is that it offers a sysfs class and some additional logic to allow dynamically change/replace the keymaps and keycodes via userspace. The idea is to remove all in-kernel keymaps in the future, using, instead, the userspace way, via ir-keytable tool, available at:
http://git.linuxtv.org/v4l-utils.git

Quoted 7 lines

The tool already supports auto-loading the keymap via udev.

For IR's where we don't know the protocol or that we don't have the full scancode, loading the keymap via userspace will not bring any new feature. But, for those devices where we can be sure about the protocol and for those that also allow using other protocols, users can just replace the device-provided IR with a more powerful remote controller with more keys.

Yeah, that sounds like a really nice feature.

Quoted 4 lines

So, it would be wonderful if you could identify what's the supported protocol(s) instead of using RC_TYPE_UNKNOWN. You can double check the protocol if you have with you another RC device that supports raw decoding. The rc-core internal decoders will tell you what protocol was used to decode a keycode, if you enable debug.

I don't have any such RC receiver device. I do have a Logitech Harmony 525, so I tried pointing that one towards the CT 3650, but CMD_GET_IR_CODE didn't change for any of the devices I've currently told my Harmony to emulate.

So I don't really see how I can help further in this case?

Em 27-12-2010 13:54, David Henningsson escreveu:

Quoted 23 lines · Mauro Carvalho Chehab

Quoted 19 lines

Em 26-12-2010 17:38, David Henningsson escreveu:

Quoted 5 lines · Mauro Carvalho Chehab

(quoted diff) +3 -0

+/* command to poll IR receiver (copied from pctv452e.c) */
+#define CMD_GET_IR_CODE     0x1b
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+/* IR */
+static int tt3650_rc_query(struct dvb_usb_device *d)
+{
+    int ret;
+    u8 rx[9]; /* A CMD_GET_IR_CODE reply is 9 bytes long */
+    ret = ttusb2_msg(d, CMD_GET_IR_CODE, NULL, 0, rx, sizeof(rx));
+    if (ret != 0)
+        return ret;
+
+    if (rx[8]&   0x01) {

Maybe (rx[8]& 0x01) == 0 indicates a keyup event. If so, if you map both keydown
and keyup events, the in-kernel repeat logic will work.

Hmm. If I should fix keyup events, the most reliable version would probably be something like:

Quoted 10 lines

if (rx[8]&  0x01) {
   int currentkey = rx[2]; // or (rx[3]<<   8) | rx[2];
   if (currentkey == lastkey)
     rc_repeat(lastkey);
   else {
     if (lastkey)
       rc_keyup(lastkey);
     lastkey = currentkey;
     rc_keydown(currentkey);
   }

rc_keydown() already handles repeat events (see ir_do_keydown and rc_keydown, at rc-main.c), so, you don't need it.

Quoted 5 lines

}

else if (lastkey) {
   rc_keyup(lastkey);
   lastkey = 0;
}

Yeah, this makes sense, if bit 1 of rx[8] indicates keyup/keydown or repeat.

You need to double check if you are not receiving any packet with this bit unset, when you press and hold a key, as some devices use a bit to just indicate that the info there is valid or not (a "done" bit).

Quoted 10 lines

Does this sound reasonable to you?

Quoted 6 lines

Quoted 5 lines

(quoted diff) +4 -0

+        /* got a "press" event */
+        deb_info("%s: cmd=0x%02x sys=0x%02x\n", __func__, rx[2], rx[3]);
+        rc_keydown(d->rc_dev, rx[2], 0);
+    }

Quoted 4 lines

Quoted 2 lines

As you're receiving both command+address, please use the complete code:

     rc_keydown(d->rc_dev, (rx[3]<<   8) | rx[2], 0);

I've tried this, but it stops working. evtest shows only scancode events, so my guess is that this makes it incompatible with RC_MAP_TT_1500, which lists only the lower byte.

yeah, you'll need either to create another table or to fix it. The better is to fix the table and to use .scanmask = 0xff at the old drivers. This way, the same table will work for both the legacy/incomplete get_scancode function and for the new one.

Quoted 10 lines

Quoted 3 lines

Also as it is receiving 8 bytes from the device, maybe the IR decoding logic is capable of decoding more than just one protocol. Such feature is nice, as it allows replacing the original keycode table by a more complete one.

I've tried dumping all nine bytes but I can't make much out of it as I'm unfamiliar with RC protocols and decoders.

Typical reply is (no key pressed):

cc 35 0b 15 00 03 00 00 00

Quoted 1 line

Does this tell you anything?

This means nothing to me, but the only way to double check is to test the device with other remote controllers. On several hardware, it is possible to use RC5 remote controllers as well. As there are some empty (zero) fields, maybe this device also supports RC6 protocols (that have more than 16 bits) and NEC extended (24 bits or 32 bits, on a few variants).

Quoted 22 lines

Quoted 7 lines

One of the most interesting features of the new RC code is that it offers a sysfs class and some additional logic to allow dynamically change/replace the keymaps and keycodes via userspace. The idea is to remove all in-kernel keymaps in the future, using, instead, the userspace way, via ir-keytable tool, available at:
http://git.linuxtv.org/v4l-utils.git

Quoted 7 lines

The tool already supports auto-loading the keymap via udev.

For IR's where we don't know the protocol or that we don't have the full scancode, loading the keymap via userspace will not bring any new feature. But, for those devices where we can be sure about the protocol and for those that also allow using other protocols, users can just replace the device-provided IR with a more powerful remote controller with more keys.

Yeah, that sounds like a really nice feature.

Quoted 4 lines

So, it would be wonderful if you could identify what's the supported protocol(s) instead of using RC_TYPE_UNKNOWN. You can double check the protocol if you have with you another RC device that supports raw decoding. The rc-core internal decoders will tell you what protocol was used to decode a keycode, if you enable debug.

Quoted 3 lines

I don't have any such RC receiver device. I do have a Logitech Harmony 525, so I tried pointing that one towards the CT 3650, but CMD_GET_IR_CODE didn't change for any of the devices I've currently told my Harmony to emulate.

So I don't really see how I can help further in this case?

I don't have a Logitech Harmony, so I'm not sure about it. Maybe Jarod may have some info about it.

From 7435de9c183fa2b62c03311ae8e08ddd436cb287 Mon Sep 17 00:00:00 2001

From 404319567f67d5c63001239ca4f960aaf775a075 Mon Sep 17 00:00:00 2001

Quoted 44 lines

Em 27-12-2010 13:54, David Henningsson escreveu:

Quoted 23 lines · Mauro Carvalho Chehab

Quoted 19 lines

Em 26-12-2010 17:38, David Henningsson escreveu:

Quoted 5 lines · Mauro Carvalho Chehab

(quoted diff) +3 -0

+/* command to poll IR receiver (copied from pctv452e.c) */
+#define CMD_GET_IR_CODE     0x1b
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+/* IR */
+static int tt3650_rc_query(struct dvb_usb_device *d)
+{
+    int ret;
+    u8 rx[9]; /* A CMD_GET_IR_CODE reply is 9 bytes long */
+    ret = ttusb2_msg(d, CMD_GET_IR_CODE, NULL, 0, rx, sizeof(rx));
+    if (ret != 0)
+        return ret;
+
+    if (rx[8]&   0x01) {

Maybe (rx[8]& 0x01) == 0 indicates a keyup event. If so, if you map both keydown
and keyup events, the in-kernel repeat logic will work.

Hmm. If I should fix keyup events, the most reliable version would probably be something like:

Quoted 10 lines

if (rx[8]&  0x01) {
   int currentkey = rx[2]; // or (rx[3]<<   8) | rx[2];
   if (currentkey == lastkey)
     rc_repeat(lastkey);
   else {
     if (lastkey)
       rc_keyup(lastkey);
     lastkey = currentkey;
     rc_keydown(currentkey);
   }

rc_keydown() already handles repeat events (see ir_do_keydown and rc_keydown, at rc-main.c), so, you don't need it.

Quoted 5 lines

}

else if (lastkey) {
   rc_keyup(lastkey);
   lastkey = 0;
}

Quoted 5 lines

Yeah, this makes sense, if bit 1 of rx[8] indicates keyup/keydown or repeat.

You need to double check if you are not receiving any packet with this bit unset, when you press and hold a key, as some devices use a bit to just indicate that the info there is valid or not (a "done" bit).

As far as I can understand, a value of "1" indicates that a key is currently pressed, and a value of "0" indicates that no key is pressed.

Quoted 14 lines

Quoted 14 lines

Does this sound reasonable to you?

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +4 -0

+        /* got a "press" event */
+        deb_info("%s: cmd=0x%02x sys=0x%02x\n", __func__, rx[2], rx[3]);
+        rc_keydown(d->rc_dev, rx[2], 0);
+    }

As you're receiving both command+address, please use the complete code:

     rc_keydown(d->rc_dev, (rx[3]<<   8) | rx[2], 0);

Quoted 5 lines

Quoted 1 line

I've tried this, but it stops working. evtest shows only scancode events, so my guess is that this makes it incompatible with RC_MAP_TT_1500, which lists only the lower byte.

yeah, you'll need either to create another table or to fix it. The better is to fix the table and to use .scanmask = 0xff at the old drivers. This way, the same table will work for both the legacy/incomplete get_scancode function and for the new one.

Ok. I did a grep for RC_MAP_TT_1500 and found one place only, so I'm attaching two patches that should fix this, feel free to commit them if they look good to you.

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Also as it is receiving 8 bytes from the device, maybe the IR decoding logic is capable of decoding more than just one protocol. Such feature is nice, as it allows replacing the original keycode table by a more complete one.

I've tried dumping all nine bytes but I can't make much out of it as I'm unfamiliar with RC protocols and decoders.

Typical reply is (no key pressed):

cc 35 0b 15 00 03 00 00 00

Quoted 7 lines

Quoted 1 line

Does this tell you anything?

This means nothing to me, but the only way to double check is to test the device with other remote controllers. On several hardware, it is possible to use RC5 remote controllers as well. As there are some empty (zero) fields, maybe this device also supports RC6 protocols (that have more than 16 bits) and NEC extended (24 bits or 32 bits, on a few variants).

Ok.

Quoted 22 lines

Quoted 22 lines

Quoted 7 lines

One of the most interesting features of the new RC code is that it offers a sysfs class and some additional logic to allow dynamically change/replace the keymaps and keycodes via userspace. The idea is to remove all in-kernel keymaps in the future, using, instead, the userspace way, via ir-keytable tool, available at:
http://git.linuxtv.org/v4l-utils.git

Quoted 7 lines

The tool already supports auto-loading the keymap via udev.

For IR's where we don't know the protocol or that we don't have the full scancode, loading the keymap via userspace will not bring any new feature. But, for those devices where we can be sure about the protocol and for those that also allow using other protocols, users can just replace the device-provided IR with a more powerful remote controller with more keys.

Yeah, that sounds like a really nice feature.

Quoted 4 lines

So, it would be wonderful if you could identify what's the supported protocol(s) instead of using RC_TYPE_UNKNOWN. You can double check the protocol if you have with you another RC device that supports raw decoding. The rc-core internal decoders will tell you what protocol was used to decode a keycode, if you enable debug.

Quoted 6 lines

Quoted 3 lines

I don't have any such RC receiver device. I do have a Logitech Harmony 525, so I tried pointing that one towards the CT 3650, but CMD_GET_IR_CODE didn't change for any of the devices I've currently told my Harmony to emulate.

So I don't really see how I can help further in this case?

I don't have a Logitech Harmony, so I'm not sure about it. Maybe Jarod may have some info about it.

Would you like me to provide a patch with RC_TYPE_UNKNOWN at this point (i e what I showed you earlier + your review comments), and when I or somebody else can provide more complete information, we make an additional patch with better protocol support? Does that make sense to you?

Em 27-12-2010 17:02, David Henningsson escreveu:

Quoted 45 lines · Mauro Carvalho Chehab

Quoted 44 lines

Em 27-12-2010 13:54, David Henningsson escreveu:

Quoted 23 lines · Mauro Carvalho Chehab

Quoted 19 lines

Em 26-12-2010 17:38, David Henningsson escreveu:

Quoted 5 lines · Mauro Carvalho Chehab

(quoted diff) +3 -0

+/* command to poll IR receiver (copied from pctv452e.c) */
+#define CMD_GET_IR_CODE     0x1b
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+/* IR */
+static int tt3650_rc_query(struct dvb_usb_device *d)
+{
+    int ret;
+    u8 rx[9]; /* A CMD_GET_IR_CODE reply is 9 bytes long */
+    ret = ttusb2_msg(d, CMD_GET_IR_CODE, NULL, 0, rx, sizeof(rx));
+    if (ret != 0)
+        return ret;
+
+    if (rx[8]&    0x01) {

Maybe (rx[8]& 0x01) == 0 indicates a keyup event. If so, if you map both keydown
and keyup events, the in-kernel repeat logic will work.

Hmm. If I should fix keyup events, the most reliable version would probably be something like:

Quoted 10 lines

if (rx[8]&   0x01) {
    int currentkey = rx[2]; // or (rx[3]<<    8) | rx[2];
    if (currentkey == lastkey)
      rc_repeat(lastkey);
    else {
      if (lastkey)
        rc_keyup(lastkey);
      lastkey = currentkey;
      rc_keydown(currentkey);
    }

rc_keydown() already handles repeat events (see ir_do_keydown and rc_keydown, at rc-main.c), so, you don't need it.

Quoted 5 lines

}

else if (lastkey) {
    rc_keyup(lastkey);
    lastkey = 0;
}

Quoted 7 lines

Quoted 5 lines

Yeah, this makes sense, if bit 1 of rx[8] indicates keyup/keydown or repeat.

You need to double check if you are not receiving any packet with this bit unset, when you press and hold a key, as some devices use a bit to just indicate that the info there is valid or not (a "done" bit).

As far as I can understand, a value of "1" indicates that a key is currently pressed, and a value of "0" indicates that no key is pressed.

Ok.

Quoted 14 lines

Quoted 14 lines

Quoted 14 lines

Does this sound reasonable to you?

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +4 -0

+        /* got a "press" event */
+        deb_info("%s: cmd=0x%02x sys=0x%02x\n", __func__, rx[2], rx[3]);
+        rc_keydown(d->rc_dev, rx[2], 0);
+    }

As you're receiving both command+address, please use the complete code:

      rc_keydown(d->rc_dev, (rx[3]<<    8) | rx[2], 0);

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

I've tried this, but it stops working. evtest shows only scancode events, so my guess is that this makes it incompatible with RC_MAP_TT_1500, which lists only the lower byte.

yeah, you'll need either to create another table or to fix it. The better is to fix the table and to use .scanmask = 0xff at the old drivers. This way, the same table will work for both the legacy/incomplete get_scancode function and for the new one.

Ok. I did a grep for RC_MAP_TT_1500 and found one place only, so I'm attaching two patches that should fix this, feel free to commit them if they look good to you.

They are good. Applied, thanks!

Quoted 45 lines

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Also as it is receiving 8 bytes from the device, maybe the IR decoding logic is capable of decoding more than just one protocol. Such feature is nice, as it allows replacing the original keycode table by a more complete one.

I've tried dumping all nine bytes but I can't make much out of it as I'm unfamiliar with RC protocols and decoders.

Typical reply is (no key pressed):

cc 35 0b 15 00 03 00 00 00

Quoted 7 lines

Quoted 1 line

Does this tell you anything?

This means nothing to me, but the only way to double check is to test the device with other remote controllers. On several hardware, it is possible to use RC5 remote controllers as well. As there are some empty (zero) fields, maybe this device also supports RC6 protocols (that have more than 16 bits) and NEC extended (24 bits or 32 bits, on a few variants).

Ok.

Quoted 24 lines

Quoted 24 lines

Quoted 7 lines

One of the most interesting features of the new RC code is that it offers a sysfs class and some additional logic to allow dynamically change/replace the keymaps and keycodes via userspace. The idea is to remove all in-kernel keymaps in the future, using, instead, the userspace way, via ir-keytable tool, available at:
http://git.linuxtv.org/v4l-utils.git

Quoted 7 lines

The tool already supports auto-loading the keymap via udev.

For IR's where we don't know the protocol or that we don't have the full scancode, loading the keymap via userspace will not bring any new feature. But, for those devices where we can be sure about the protocol and for those that also allow using other protocols, users can just replace the device-provided IR with a more powerful remote controller with more keys.

Yeah, that sounds like a really nice feature.

Quoted 4 lines

So, it would be wonderful if you could identify what's the supported protocol(s) instead of using RC_TYPE_UNKNOWN. You can double check the protocol if you have with you another RC device that supports raw decoding. The rc-core internal decoders will tell you what protocol was used to decode a keycode, if you enable debug.

I don't have any such RC receiver device. I do have a Logitech Harmony 525, so I tried pointing that one towards the CT 3650, but CMD_GET_IR_CODE didn't change for any of the devices I've currently told my Harmony to emulate.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

So I don't really see how I can help further in this case?

I don't have a Logitech Harmony, so I'm not sure about it. Maybe Jarod may have some info about it.

Would you like me to provide a patch with RC_TYPE_UNKNOWN at this point (i e what I showed you earlier + your review comments), and when I or somebody else can provide more complete information, we make an additional patch with better protocol support? Does that make sense to you?

Yeah, this should be OK for me.

From 44e2a8503f2db4f5316ec739b804b6a3498111e3 Mon Sep 17 00:00:00 2001

Ok, here comes the patch. It seems to be working sufficiently well after I discovered I needed a poll interval less than IR_KEYPRESS_TIMEOUT. As a side note, grepping for rc_interval seems to reveal a few intervals >= 250, could we have suboptimal results from these as well?