Opened 7 months ago
Closed 5 months ago
#18641 closed bug (fixed)
USB HID presents control codes +1 offset for consumer page inputs
| Reported by: | kallisti5 | Owned by: | nobody |
|---|---|---|---|
| Priority: | normal | Milestone: | R1/beta5 |
| Component: | Drivers/Input/HID/USB | Version: | R1/beta4 |
| Keywords: | Cc: | ||
| Blocked By: | Blocking: | ||
| Platform: | All |
Description (last modified by )
Haiku sees control codes +1 for consumer page (and other non-bitmap encoded hid pages)
The Keyboard is a Keychron Q3 Pro SE running the open source VIA firmware.
Example (looking at the volume controls)...
- Haiku:
- c00e3 mute, c00ea increase, c00eb decrease
- Spec:
- c00e2 mute, c00e9 increase, c00ea decrease
- usbhid-dump on Linux:
- c00e2 mute, c00e9 increase, c00ea decrease
This is based on puck_'s analysis. I'm a dog in a lab coat.
Get the HID descriptor...
usbhid-dump --model=3434:0630
Take the HID descriptor..
001:002:002:DEC 1698190821.432852 05 01 09 02 A1 01 85 02 09 01 A1 00 05 09 19 01 29 08 15 00 25 01 95 08 75 01 81 02 05 01 09 30 09 31 15 81 25 7F 95 02 75 08 81 06 09 38 15 81 25 7F 95 01 75 08 81 06 05 0C 0A 38 02 15 81 25 7F 95 01 75 08 81 06 C0 C0 05 01 09 80 A1 01 85 03 19 01 2A B7 00 15 01 26 B7 00 95 01 75 10 81 00 C0 05 0C 09 01 A1 01 85 04 19 01 2A A0 02 15 01 26 A0 02 95 01 75 10 81 00 C0 05 01 09 06 A1 01 85 06 05 07 19 E0 29 E7 15 00 25 01 95 08 75 01 81 02 05 07 19 00 29 EF 15 00 25 01 95 F0 75 01 81 02 05 08 19 01 29 05 95 05 75 01 91 02 95 01 75 03 91 01 C0
And plug it into https://eleccelerator.com/usbdescreqparser/
This decodes to...
0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x02, // Usage (Mouse) 0xA1, 0x01, // Collection (Application) 0x85, 0x02, // Report ID (2) 0x09, 0x01, // Usage (0x01) 0xA1, 0x00, // Collection (Physical) 0x05, 0x09, // Usage Page (Button) 0x19, 0x01, // Usage Minimum (0x01) 0x29, 0x08, // Usage Maximum (0x08) 0x15, 0x00, // Logical Minimum (0) 0x25, 0x01, // Logical Maximum (1) 0x95, 0x08, // Report Count (8) 0x75, 0x01, // Report Size (1) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x30, // Usage (X) 0x09, 0x31, // Usage (Y) 0x15, 0x81, // Logical Minimum (-127) 0x25, 0x7F, // Logical Maximum (127) 0x95, 0x02, // Report Count (2) 0x75, 0x08, // Report Size (8) 0x81, 0x06, // Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position) 0x09, 0x38, // Usage (Wheel) 0x15, 0x81, // Logical Minimum (-127) 0x25, 0x7F, // Logical Maximum (127) 0x95, 0x01, // Report Count (1) 0x75, 0x08, // Report Size (8) 0x81, 0x06, // Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position) 0x05, 0x0C, // Usage Page (Consumer) 0x0A, 0x38, 0x02, // Usage (AC Pan) 0x15, 0x81, // Logical Minimum (-127) 0x25, 0x7F, // Logical Maximum (127) 0x95, 0x01, // Report Count (1) 0x75, 0x08, // Report Size (8) 0x81, 0x06, // Input (Data,Var,Rel,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection 0xC0, // End Collection 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x80, // Usage (Sys Control) 0xA1, 0x01, // Collection (Application) 0x85, 0x03, // Report ID (3) 0x19, 0x01, // Usage Minimum (Pointer) 0x2A, 0xB7, 0x00, // Usage Maximum (Sys Display LCD Autoscale) 0x15, 0x01, // Logical Minimum (1) 0x26, 0xB7, 0x00, // Logical Maximum (183) 0x95, 0x01, // Report Count (1) 0x75, 0x10, // Report Size (16) 0x81, 0x00, // Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection 0x05, 0x0C, // Usage Page (Consumer) 0x09, 0x01, // Usage (Consumer Control) 0xA1, 0x01, // Collection (Application) 0x85, 0x04, // Report ID (4) 0x19, 0x01, // Usage Minimum (Consumer Control) 0x2A, 0xA0, 0x02, // Usage Maximum (0x02A0) 0x15, 0x01, // Logical Minimum (1) 0x26, 0xA0, 0x02, // Logical Maximum (672) 0x95, 0x01, // Report Count (1) 0x75, 0x10, // Report Size (16) 0x81, 0x00, // Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) 0x09, 0x06, // Usage (Keyboard) 0xA1, 0x01, // Collection (Application) 0x85, 0x06, // Report ID (6) 0x05, 0x07, // Usage Page (Kbrd/Keypad) 0x19, 0xE0, // Usage Minimum (0xE0) 0x29, 0xE7, // Usage Maximum (0xE7) 0x15, 0x00, // Logical Minimum (0) 0x25, 0x01, // Logical Maximum (1) 0x95, 0x08, // Report Count (8) 0x75, 0x01, // Report Size (1) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0x05, 0x07, // Usage Page (Kbrd/Keypad) 0x19, 0x00, // Usage Minimum (0x00) 0x29, 0xEF, // Usage Maximum (0xEF) 0x15, 0x00, // Logical Minimum (0) 0x25, 0x01, // Logical Maximum (1) 0x95, 0xF0, // Report Count (-16) 0x75, 0x01, // Report Size (1) 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) 0x05, 0x08, // Usage Page (LEDs) 0x19, 0x01, // Usage Minimum (Num Lock) 0x29, 0x05, // Usage Maximum (Kana) 0x95, 0x05, // Report Count (5) 0x75, 0x01, // Report Size (1) 0x91, 0x02, // Output (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) 0x95, 0x01, // Report Count (1) 0x75, 0x03, // Report Size (3) 0x91, 0x01, // Output (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) 0xC0, // End Collection // 185 bytes
Change History (10)
comment:1 by , 7 months ago
| Description: | modified (diff) |
|---|
comment:2 by , 7 months ago
| Description: | modified (diff) |
|---|
comment:3 by , 7 months ago
| Description: | modified (diff) |
|---|
comment:4 by , 7 months ago
It would seem the usage should be computed so: uint32 usageOffset = val - h.logical_minimum; usage = h.usage_minimum + usageOffset;
comment:5 by , 7 months ago
The relevant part of the descriptor is this:
0x05, 0x0C, // Usage Page (Consumer) 0x09, 0x01, // Usage (Consumer Control) 0xA1, 0x01, // Collection (Application) 0x85, 0x04, // Report ID (4) 0x19, 0x01, // Usage Minimum (Consumer Control) 0x2A, 0xA0, 0x02, // Usage Maximum (0x02A0) 0x15, 0x01, // Logical Minimum (1) 0x26, 0xA0, 0x02, // Logical Maximum (672) 0x95, 0x01, // Report Count (1) 0x75, 0x10, // Report Size (16) 0x81, 0x00, // Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) 0xC0, // End Collection
This says the keyboard is going to send 1 (Report Count) value of 16 (Report Size) bits representing a "Consumer Control" key, in the range 1 to 672 == 1 to 0x02A0.
The value 0 is not included in the range because it is used for key releases (a value of 0 means no keys pressed).
As far as I can see, the logical minimum is 1 and the usage minimum is also 1. So there shouldn't be any conversion needed here? Or at least the one you suggest wouldn't change anything. Am I missing something?
I also looked at the code and I don't see where we would apply either of these offsets to the key. We just take the Data() from the HID report item (KeyboardProtocolHandler.cpp line 552).
comment:6 by , 7 months ago
usage_minimum looks like already applied at line 814 so I would substract the logical_minimum at line 552
diff --git a/src/add-ons/kernel/drivers/input/hid_shared/HIDReportItem.h b/src/add-ons/kernel/drivers/input/hid_shared/HIDReportItem.h
index 926f6551c0..b77f4be524 100644
--- a/src/add-ons/kernel/drivers/input/hid_shared/HIDReportItem.h
+++ b/src/add-ons/kernel/drivers/input/hid_shared/HIDReportItem.h
@@ -23,6 +23,7 @@ public:
bool Relative() { return fRelative; };
bool Array() { return fArray; };
bool Signed() { return fMinimum > fMaximum; };
+ uint32 Minimum() { return fMinimum; }
uint16 UsagePage();
uint16 UsageID();
diff --git a/src/add-ons/kernel/drivers/input/hid_shared/KeyboardProtocolHandler.cpp b/src/add-ons/kernel/drivers/input/hid_shared/KeyboardProtocolHandler.cpp
index da6619f109..5e7fd41b45 100644
--- a/src/add-ons/kernel/drivers/input/hid_shared/KeyboardProtocolHandler.cpp
+++ b/src/add-ons/kernel/drivers/input/hid_shared/KeyboardProtocolHandler.cpp
@@ -549,7 +549,7 @@ KeyboardProtocolHandler::_ReadReport(bigtime_t timeout, uint32 *cookie)
if (key->Extract() == B_OK && key->Valid()) {
// handle both array and bitmap based keyboard reports
if (key->Array()) {
- fCurrentKeys[i] = key->Data();
+ fCurrentKeys[i] = key->Data() - key->Minimum();
} else {
if (key->Data() == 1)
fCurrentKeys[i] = key->UsageID();
comment:7 by , 7 months ago
The HID spec doesn't make this very clear, this is my interpretation:
This is an Array input, which means that the logical minimum/maximum are used to indicate the valid range of button indexes that may be enabled (so array index 0 is invalid, then 1-672 are valid). This means that no scaling of the value should happen in this case.
The specification also explicitly calls out this example, on page 32 (of the v1.11 HID spec.)
As an aside, I originally thought the issue was that Haiku wasn't processing the physical minimum, but that's possibly another issue altogether.
The combination of Usage and Logical Minimum here does need to be kept in mind, though.
comment:8 by , 7 months ago
comment:10 by , 5 months ago
| Milestone: | Unscheduled → R1/beta5 |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
Resolved. All the extended keycodes are expected values :-)
(I'm fixing the decoded descriptor, I think you copied the first line in the decoder and it found some extra bytes there that shouldn't be decoded)