Handling of USB "Programmable button" controls as KEY_MACRO# events

Hi everybody,

Would it make sense to map the "Programmable Buttons" control from the
USB HID Consumer page [0] to the linux event codes KEY_MACRO1 ... KEY_MACRO# ?

Those controls are documented in the USB spec as:

"The user defines the function of these
buttons to control software applications or GUI objects."

The KEY_MACRO event codes are documented with:

"Some keyboards have keys which do not have a defined meaning, these keys
are intended to be programmed / bound to macros by the user."

My usecase is the passing of custom keycodes from a programmable keypad
(via QMK[1]) to Linux.
(This would also need new functionality in QMK itself)

Alternatives:

* Send Raw HID from QMK
  * Con: needs a dedicated, nonstandard driver on the host
* Use F-Keys
  * Con: only F13-F19 are usable (F1-F12 are used by normal keyboards, F20-F23
    are repurposed with other keys for X11 compat)

Possible problems:

* There are 65k programmable keys defined by USB but only 30 macro keys are
  supported by Linux.

Thanks,
Thomas

[0] https://www.usb.org/sites/default/files/hut1_22.pdf#section.15.14
[1] https://qmk.fm/

Hi,

On 5/18/21 3:21 PM, Thomas Weißschuh wrote:

Hi everybody,

Would it make sense to map the "Programmable Buttons" control from the
USB HID Consumer page [0] to the linux event codes KEY_MACRO1 ... KEY_MACRO# ?

Those controls are documented in the USB spec as:

"The user defines the function of these
buttons to control software applications or GUI objects."

The KEY_MACRO event codes are documented with:

"Some keyboards have keys which do not have a defined meaning, these keys
are intended to be programmed / bound to macros by the user."

My usecase is the passing of custom keycodes from a programmable keypad
(via QMK[1]) to Linux.
(This would also need new functionality in QMK itself)

I think the idea is good, but AFAICT the HUT does not actually assign
any usage codes in the consumer-page for this. It simply points to the
Button usage-page, which means things conflict with e.g. mouse and joystick
buttons and I do not see any dedicated codes in the table
"Table 15.1: Consumer Page" so I'm not sure how to interpret the spec. here ...

I guess there is something which we can do with the report's application here,
since the code dealing with HID_UP_BUTTON is already doing a switch-case
on field->application to differentiate between mouse and gaming buttons.

I guess interpreting an application of HID_CP_CONSUMER_CONTROL in combination
with using the buttons usage-page as wat the HUT is trying to specify and
thus map that the first 30 codes in that combination to KEY_MACRO1 - 30
might make sense.

Regards,

Hans

Alternatives:

* Send Raw HID from QMK
  * Con: needs a dedicated, nonstandard driver on the host
* Use F-Keys
  * Con: only F13-F19 are usable (F1-F12 are used by normal keyboards, F20-F23
    are repurposed with other keys for X11 compat)

Possible problems:

* There are 65k programmable keys defined by USB but only 30 macro keys are
  supported by Linux.

Thanks,
Thomas

[0] https://www.usb.org/sites/default/files/hut1_22.pdf#section.15.14
[1] https://qmk.fm/

From: Thomas Weißschuh <redacted>

Map them to KEY_MACRO# event codes.

These buttons are defined by HID as follows:
"The user defines the function of these buttons to control software
applications or GUI objects."

This matches the semantics of the KEY_MACRO# input event codes that
Linux supports.

Signed-off-by: Thomas Weißschuh <redacted>
---
 drivers/hid/hid-debug.c | 11 +++++++++++
 drivers/hid/hid-input.c |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 59f8d716d78f..0e76d9b4530a 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c

@@ -122,6 +122,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
   {  9, 0, "Button" },
   { 10, 0, "Ordinal" },
   { 12, 0, "Consumer" },
+      {0, 0x003, "ProgrammableButtons"},
       {0, 0x238, "HorizontalWheel"},
   { 13, 0, "Digitizers" },
     {0, 0x01, "Digitizer"},

@@ -939,6 +940,16 @@ static const char *keys[KEY_MAX + 1] = {
 	[KEY_KBDINPUTASSIST_NEXTGROUP] = "KbdInputAssistNextGroup",
 	[KEY_KBDINPUTASSIST_ACCEPT] = "KbdInputAssistAccept",
 	[KEY_KBDINPUTASSIST_CANCEL] = "KbdInputAssistCancel",
+	[KEY_MACRO1] = "Macro1", [KEY_MACRO2] = "Macro2", [KEY_MACRO3] = "Macro3",
+	[KEY_MACRO4] = "Macro4", [KEY_MACRO5] = "Macro5", [KEY_MACRO6] = "Macro6",
+	[KEY_MACRO7] = "Macro7", [KEY_MACRO8] = "Macro8", [KEY_MACRO9] = "Macro9",
+	[KEY_MACRO10] = "Macro10", [KEY_MACRO11] = "Macro11", [KEY_MACRO12] = "Macro12",
+	[KEY_MACRO13] = "Macro13", [KEY_MACRO14] = "Macro14", [KEY_MACRO15] = "Macro15",
+	[KEY_MACRO16] = "Macro16", [KEY_MACRO17] = "Macro17", [KEY_MACRO18] = "Macro18",
+	[KEY_MACRO19] = "Macro19", [KEY_MACRO20] = "Macro20", [KEY_MACRO21] = "Macro21",
+	[KEY_MACRO22] = "Macro22", [KEY_MACRO23] = "Macro23", [KEY_MACRO24] = "Macro24",
+	[KEY_MACRO25] = "Macro25", [KEY_MACRO26] = "Macro26", [KEY_MACRO27] = "Macro27",
+	[KEY_MACRO28] = "Macro28", [KEY_MACRO29] = "Macro29", [KEY_MACRO30] = "Macro30",
 };
 
 static const char *relatives[REL_MAX + 1] = {

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 18f5e28d475c..7d4dee58d869 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

@@ -632,6 +632,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 				else
 					code += BTN_TRIGGER_HAPPY - 0x10;
 				break;
+		case HID_CP_CONSUMER_CONTROL: code += KEY_MACRO1; break;
 		default:
 			switch (field->physical) {
 			case HID_GD_MOUSE:

base-commit: efd8929b9eec1cde120abb36d76dd00ff6711023
-- 
2.31.1

Hi,

On 5/19/21 6:03 PM, Thomas Weißschuh wrote:

15 more quoted lines

Shouldn't there be a check here to ensure that we don't map things above KEY_MACRO30 ?
if we do that then we start hitting other codes like KEY_MACRO_RECORD_START and eventually
BTN_TRIGGER_HAPPY and after the BTN_TRIGGER_HAPPY range we go over KEY_MAX which I think
is not supported ?

Regards,

Hans

 		default:
 			switch (field->physical) {
 			case HID_GD_MOUSE:

base-commit: efd8929b9eec1cde120abb36d76dd00ff6711023

Hi,

On Mi, 2021-05-19T18:13+0200, Hans de Goede wrote:

Hi,

On 5/19/21 6:03 PM, Thomas Weißschuh wrote:

quoted

From: Thomas Weißschuh <redacted>

Map them to KEY_MACRO# event codes.

These buttons are defined by HID as follows:
"The user defines the function of these buttons to control software
applications or GUI objects."

This matches the semantics of the KEY_MACRO# input event codes that
Linux supports.

Signed-off-by: Thomas Weißschuh <redacted>
---
 drivers/hid/hid-debug.c | 11 +++++++++++
 drivers/hid/hid-input.c |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 18f5e28d475c..7d4dee58d869 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

@@ -632,6 +632,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 				else
 					code += BTN_TRIGGER_HAPPY - 0x10;
 				break;
+		case HID_CP_CONSUMER_CONTROL: code += KEY_MACRO1; break;

Shouldn't there be a check here to ensure that we don't map things above KEY_MACRO30 ?
if we do that then we start hitting other codes like KEY_MACRO_RECORD_START and eventually
BTN_TRIGGER_HAPPY and after the BTN_TRIGGER_HAPPY range we go over KEY_MAX which I think
is not supported ?

Regards,

Hans

I thought all the other chunks of logic around this one would be affected by
this issue, too.
But actually it seems all the overflowing keys get first assigned to the
BTN_TRIGGER_HAPPY range and after that will be clipped directly by
map_key()/hid_map_usage().

I'll resend the patch.

Thanks,
Thomas

Map them to KEY_MACRO# event codes.

These buttons are defined by HID as follows:
"The user defines the function of these buttons to control software applications or GUI objects."

This matches the semantics of the KEY_MACRO# input event codes that Linux supports.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---

v1 -> v2: Only handle the 30 keys known

 drivers/hid/hid-debug.c | 11 +++++++++++
 drivers/hid/hid-input.c |  6 ++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 59f8d716d78f..0e76d9b4530a 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c

@@ -122,6 +122,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
   {  9, 0, "Button" },
   { 10, 0, "Ordinal" },
   { 12, 0, "Consumer" },
+      {0, 0x003, "ProgrammableButtons"},
       {0, 0x238, "HorizontalWheel"},
   { 13, 0, "Digitizers" },
     {0, 0x01, "Digitizer"},

@@ -939,6 +940,16 @@ static const char *keys[KEY_MAX + 1] = {
 	[KEY_KBDINPUTASSIST_NEXTGROUP] = "KbdInputAssistNextGroup",
 	[KEY_KBDINPUTASSIST_ACCEPT] = "KbdInputAssistAccept",
 	[KEY_KBDINPUTASSIST_CANCEL] = "KbdInputAssistCancel",
+	[KEY_MACRO1] = "Macro1", [KEY_MACRO2] = "Macro2", [KEY_MACRO3] = "Macro3",
+	[KEY_MACRO4] = "Macro4", [KEY_MACRO5] = "Macro5", [KEY_MACRO6] = "Macro6",
+	[KEY_MACRO7] = "Macro7", [KEY_MACRO8] = "Macro8", [KEY_MACRO9] = "Macro9",
+	[KEY_MACRO10] = "Macro10", [KEY_MACRO11] = "Macro11", [KEY_MACRO12] = "Macro12",
+	[KEY_MACRO13] = "Macro13", [KEY_MACRO14] = "Macro14", [KEY_MACRO15] = "Macro15",
+	[KEY_MACRO16] = "Macro16", [KEY_MACRO17] = "Macro17", [KEY_MACRO18] = "Macro18",
+	[KEY_MACRO19] = "Macro19", [KEY_MACRO20] = "Macro20", [KEY_MACRO21] = "Macro21",
+	[KEY_MACRO22] = "Macro22", [KEY_MACRO23] = "Macro23", [KEY_MACRO24] = "Macro24",
+	[KEY_MACRO25] = "Macro25", [KEY_MACRO26] = "Macro26", [KEY_MACRO27] = "Macro27",
+	[KEY_MACRO28] = "Macro28", [KEY_MACRO29] = "Macro29", [KEY_MACRO30] = "Macro30",
 };
 
 static const char *relatives[REL_MAX + 1] = {

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 18f5e28d475c..32962772cb42 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

@@ -632,6 +632,12 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 				else
 					code += BTN_TRIGGER_HAPPY - 0x10;
 				break;
+		case HID_CP_CONSUMER_CONTROL:
+				if (code <= 29)
+					code += KEY_MACRO1;
+				else
+					code += BTN_TRIGGER_HAPPY - 30;
+				break;
 		default:
 			switch (field->physical) {
 			case HID_GD_MOUSE:

base-commit: efd8929b9eec1cde120abb36d76dd00ff6711023
-- 
2.31.1

Hi,

On 5/19/21 7:43 PM, Thomas Weißschuh wrote:

Map them to KEY_MACRO# event codes.

These buttons are defined by HID as follows:
"The user defines the function of these buttons to control software applications or GUI objects."

This matches the semantics of the KEY_MACRO# input event codes that Linux supports.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---

v1 -> v2: Only handle the 30 keys known

I'm no expert in this part of HID, so I'm not 100% sure this is correct,
with that said this looks good to me:

Reviewed-by: Hans de Goede <redacted>

Regards,

Hans

14 more quoted lines