[Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Juha Yrjölä]

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

Hi,

Attached is a patch which fixes the problem of keys repeating when too many
keys pressed at the same time (e.g. when typing quickly).  This often
results in "mount" becoming "mouount". =)

It seems that the BT keyboard sends a HID report with the keys all set to
0x01 if too many keys were pressed at the same time.  This confused the
previous report handling logic.

Verified working on a Logitech diNovo keyboard.

Signed-off-by: Juha Yrjölä <juha.yrjola@iki.fi>

Oh yeah, the patch is against 2.6.10-mh1.

Cheers,
Juha

[-- Attachment #2: too-many-keys-patch.diff --]
[-- Type: text/plain, Size: 600 bytes --]

--- net/bluetooth/hidp/core.c.old	2005-01-20 00:08:05.000000000 +0200
+++ net/bluetooth/hidp/core.c	2005-01-20 00:32:18.000000000 +0200
@@ -175,6 +175,15 @@
 		for (i = 0; i < 8; i++)
 			input_report_key(dev, hidp_keycode[i + 224], (udata[0] >> i) & 1);
 
+		/* If all the key codes have been set to 0x01, it means
+		 * too many keys were pressed at the same time */
+		for (i = 2; i < 8; i++) {
+			if (udata[i] != 0x01)
+				break;
+		}
+		if (i == 8)
+			break;
+
 		for (i = 2; i < 8; i++) {
 			if (keys[i] > 3 && memscan(udata + 2, keys[i], 6) == udata + 8) {
 				if (hidp_keycode[keys[i]])

Re: [Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Marcel Holtmann]

Hi Juha,

> Attached is a patch which fixes the problem of keys repeating when too many
> keys pressed at the same time (e.g. when typing quickly).  This often
> results in "mount" becoming "mouount". =)
> 
> It seems that the BT keyboard sends a HID report with the keys all set to
> 0x01 if too many keys were pressed at the same time.  This confused the
> previous report handling logic.

can you point me to the part of the HID specification that defines this
behaviour? And show me an extract from hcidump when this happens.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Juha Yrjölä]

Hi Marcel,

On Thu, Jan 20, 2005 at 01:19:38AM +0100, Marcel Holtmann wrote:

> can you point me to the part of the HID specification that defines this
> behaviour? And show me an extract from hcidump when this happens.

I don't know if this is in the spec, or if it is just a quirk of some
Logitech keyboards.  However, it shouldn't cause any trouble to check
for it as a special case, since that kind of data packet is clearly
invalid, no?

Here's the dump of me pressing 'qwert', and then releasing 't':

1106180982.868085 > ACL data: handle 0x0029 flags 0x02 dlen 14
    L2CAP(d): cid 0x0041 len 10 [psm 0]
      A1 01 00 00 14 00 00 00 00 00 
1106180982.927077 > ACL data: handle 0x0029 flags 0x02 dlen 14
    L2CAP(d): cid 0x0041 len 10 [psm 0]
      A1 01 00 00 14 1A 00 00 00 00 
1106180982.967071 > ACL data: handle 0x0029 flags 0x02 dlen 14
    L2CAP(d): cid 0x0041 len 10 [psm 0]
      A1 01 00 00 14 1A 08 00 00 00 
1106180983.027061 > ACL data: handle 0x0029 flags 0x02 dlen 14
    L2CAP(d): cid 0x0041 len 10 [psm 0]
      A1 01 00 00 14 1A 08 15 00 00 
1106180983.047059 > ACL data: handle 0x0029 flags 0x02 dlen 14
    L2CAP(d): cid 0x0041 len 10 [psm 0]
      A1 01 00 00 01 01 01 01 01 01 
1106180983.267020 > ACL data: handle 0x0029 flags 0x02 dlen 14
    L2CAP(d): cid 0x0041 len 10 [psm 0]
      A1 01 00 00 14 1A 08 15 00 00 

Cheers,
Juha


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Marcel Holtmann]

Hi Juha,

> > can you point me to the part of the HID specification that defines this
> > behaviour? And show me an extract from hcidump when this happens.
> 
> I don't know if this is in the spec, or if it is just a quirk of some
> Logitech keyboards.  However, it shouldn't cause any trouble to check
> for it as a special case, since that kind of data packet is clearly
> invalid, no?
> 
> Here's the dump of me pressing 'qwert', and then releasing 't':
> 
> 1106180982.868085 > ACL data: handle 0x0029 flags 0x02 dlen 14
>     L2CAP(d): cid 0x0041 len 10 [psm 0]
>       A1 01 00 00 14 00 00 00 00 00 
> 1106180982.927077 > ACL data: handle 0x0029 flags 0x02 dlen 14
>     L2CAP(d): cid 0x0041 len 10 [psm 0]
>       A1 01 00 00 14 1A 00 00 00 00 
> 1106180982.967071 > ACL data: handle 0x0029 flags 0x02 dlen 14
>     L2CAP(d): cid 0x0041 len 10 [psm 0]
>       A1 01 00 00 14 1A 08 00 00 00 
> 1106180983.027061 > ACL data: handle 0x0029 flags 0x02 dlen 14
>     L2CAP(d): cid 0x0041 len 10 [psm 0]
>       A1 01 00 00 14 1A 08 15 00 00 
> 1106180983.047059 > ACL data: handle 0x0029 flags 0x02 dlen 14
>     L2CAP(d): cid 0x0041 len 10 [psm 0]
>       A1 01 00 00 01 01 01 01 01 01 
> 1106180983.267020 > ACL data: handle 0x0029 flags 0x02 dlen 14
>     L2CAP(d): cid 0x0041 len 10 [psm 0]
>       A1 01 00 00 14 1A 08 15 00 00 

this looks weird and even the 't' is still missing in that dump. Is this
a general behaviour? Do we see the same on other keyboard?

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Juha Yrjölä]

Hi,

On Thu, Jan 20, 2005 at 02:01:13AM +0100, Marcel Holtmann wrote:

> > 1106180983.027061 > ACL data: handle 0x0029 flags 0x02 dlen 14
> >     L2CAP(d): cid 0x0041 len 10 [psm 0]
> >       A1 01 00 00 14 1A 08 15 00 00 
> > 1106180983.047059 > ACL data: handle 0x0029 flags 0x02 dlen 14
> >     L2CAP(d): cid 0x0041 len 10 [psm 0]
> >       A1 01 00 00 01 01 01 01 01 01 
> > 1106180983.267020 > ACL data: handle 0x0029 flags 0x02 dlen 14
> >     L2CAP(d): cid 0x0041 len 10 [psm 0]
> >       A1 01 00 00 14 1A 08 15 00 00 
> 
> this looks weird and even the 't' is still missing in that dump.

Yes, weird it is.  The 't' is missing because the keyboard is not able to
detect it anymore, because four other keys are being pressed (due to the
design of the keyboard, which allows only certain keys being detected at the
same time).  The keyboard only detects the condition "too many keys pressed",
hence the 0x01 packet.

> Is this a general behaviour? Do we see the same on other keyboard?

I have only the diNovo, and it does this every time.  A brief search through
the ML archives revealed:

http://sourceforge.net/mailarchive/message.php?msg_id=8762071

It seems that at least other Logitech keyboards express similiar behaviour.

I believe my patch is the least intrusive fix for handling the odd packet.

Cheers,
Juha


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Marcel Holtmann]

Hi Juha,

> > > 1106180983.027061 > ACL data: handle 0x0029 flags 0x02 dlen 14
> > >     L2CAP(d): cid 0x0041 len 10 [psm 0]
> > >       A1 01 00 00 14 1A 08 15 00 00 
> > > 1106180983.047059 > ACL data: handle 0x0029 flags 0x02 dlen 14
> > >     L2CAP(d): cid 0x0041 len 10 [psm 0]
> > >       A1 01 00 00 01 01 01 01 01 01 
> > > 1106180983.267020 > ACL data: handle 0x0029 flags 0x02 dlen 14
> > >     L2CAP(d): cid 0x0041 len 10 [psm 0]
> > >       A1 01 00 00 14 1A 08 15 00 00 
> > 
> > this looks weird and even the 't' is still missing in that dump.
> 
> Yes, weird it is.  The 't' is missing because the keyboard is not able to
> detect it anymore, because four other keys are being pressed (due to the
> design of the keyboard, which allows only certain keys being detected at the
> same time).  The keyboard only detects the condition "too many keys pressed",
> hence the 0x01 packet.
> 
> > Is this a general behaviour? Do we see the same on other keyboard?
> 
> I have only the diNovo, and it does this every time.  A brief search through
> the ML archives revealed:
> 
> http://sourceforge.net/mailarchive/message.php?msg_id=8762071
> 
> It seems that at least other Logitech keyboards express similiar behaviour.

I tested it with my Microsoft and Apple keyboards. It seems that both of
them take 5 keys before they result in too many keys are pressed, but
this event still occurs.

Since Apple uses the Broadcom chip and Logitech/Microsoft uses CSR this
seems not to be unique to the Bluetooth software on the keyboard. So it
must be inside the specification or all of these keyboards simple share
the same keyboard chip. Has anyone some detailed information for me
about this?

> I believe my patch is the least intrusive fix for handling the odd packet.

I think we should add a patch for this, but I don't like the way you did
it. Checking for the end value of a for-loop is not a good programming
practice. Using memcmp() here should be a lot better and cleaner.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Juha Yrjölä]

Hi Marcel,

On Sun, Jan 23, 2005 at 09:46:48AM +0100, Marcel Holtmann wrote:

> I think we should add a patch for this, but I don't like the way you did
> it. Checking for the end value of a for-loop is not a good programming
> practice. Using memcmp() here should be a lot better and cleaner.

It depends on how you think about the problem.  If you see the data as just
an array of bytes, memcmp() is the way to go.  If you think of it as 6
consecutive key codes, with each one set to 1, for loop is conceptually the            
right thing to use.
        
Cheers,
Juha


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Fix too-many-keys-pressed error in hidp/core.c

[Marcel Holtmann]

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

Hi Juha,

> > I think we should add a patch for this, but I don't like the way you did
> > it. Checking for the end value of a for-loop is not a good programming
> > practice. Using memcmp() here should be a lot better and cleaner.
> 
> It depends on how you think about the problem.  If you see the data as just
> an array of bytes, memcmp() is the way to go.  If you think of it as 6
> consecutive key codes, with each one set to 1, for loop is conceptually the            
> right thing to use.

I don't see your difference. We have to match a specific case and we
have to do it with clean code. And as I said, checking the end value of
a for-loop is not good code for me. So I am using memcmp() for it and my
patch is attached.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 860 bytes --]

===== net/bluetooth/hidp/core.c 1.3 vs edited =====
--- 1.3/net/bluetooth/hidp/core.c	2004-12-26 19:19:32 +01:00
+++ edited/net/bluetooth/hidp/core.c	2005-01-23 10:01:48 +01:00
@@ -74,6 +74,8 @@
 	150,158,159,128,136,177,178,176,142,152,173,140
 };
 
+static unsigned char hidp_mkeyspat[] = { 0x01, 0x01, 0x01, 0x01, 0x01, 0x01 };
+
 static struct hidp_session *__hidp_get_session(bdaddr_t *bdaddr)
 {
 	struct hidp_session *session;
@@ -174,6 +176,11 @@
 	case 0x01:	/* Keyboard report */
 		for (i = 0; i < 8; i++)
 			input_report_key(dev, hidp_keycode[i + 224], (udata[0] >> i) & 1);
+
+		/* If all the key codes have been set to 0x01, it means
+		 * too many keys were pressed at the same time. */
+		if (!memcmp(udata + 2, hidp_mkeyspat, 6))
+			break;
 
 		for (i = 2; i < 8; i++) {
 			if (keys[i] > 3 && memscan(udata + 2, keys[i], 6) == udata + 8) {