Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Home and Back button not forwarded when using hid-remapper #120

Open
bjlaur opened this issue Mar 14, 2024 · 9 comments
Open

Home and Back button not forwarded when using hid-remapper #120

bjlaur opened this issue Mar 14, 2024 · 9 comments

Comments

@bjlaur
Copy link

bjlaur commented Mar 14, 2024

I am using a Rii i4 with a hid-remapper. (Rii i4 is using the 2.4ghz usb dongle)

Everything works as expected except for the fact that two buttons (Home and Back) no longer operate when used in conjunction with the Rii i4.

In the monitor tab, these two 'keys' show up as:

  • 0x000c0223 - Home
  • 0x000c0224 - Back

Is there a section of the code where we are only allowing certain keys to be forwarded?

image

Steps to reproduce: Use a keyboard with Home and Back buttons. Press these buttons when plugged in through an hid-remapper

Expected result: Unless these keys are otherwise mapped, the hid-remapper should forward their events to the host device

Actual result: Buttons no longer operate when connection in conjunction with an hid-remapper.

@jfedor2
Copy link
Owner

jfedor2 commented Mar 14, 2024

HID Remapper's report descriptor is fixed, it doesn't change based on what devices are plugged into it. It just doesn't have these keys so it can't pass them through. You can map them to something else if you want.

@bjlaur
Copy link
Author

bjlaur commented Mar 14, 2024

How can one change the report descriptor to enable these keys?

@jfedor2
Copy link
Owner

jfedor2 commented Mar 14, 2024

It's in our_descriptor.cc.

@bjlaur
Copy link
Author

bjlaur commented Mar 18, 2024

I pulled the descriptor from the keyboard, and extracted the relevant portions. I added these to the hid-remapper descriptor.
(This was after a few iterations of trying to do it manually... but the documentation for how to register these AC commands is rather spare.)

The result is the hid-remapper can no longer forward (any) events to the host.

@jfedor2 do you have any hints for me? Do you think there is a problem with the descriptor?
Or is there some other fault/incompatibility with the hid-remapper code that is causing it to fault?

How would one typically go about debugging this?

diff --git a/firmware/src/our_descriptor.cc b/firmware/src/our_descriptor.cc
index 464d716..c17a66d 100644
--- a/firmware/src/our_descriptor.cc
+++ b/firmware/src/our_descriptor.cc
@@ -135,6 +135,13 @@ const uint8_t our_report_descriptor_kb_mouse[] = {
     0x09, 0x2F,                //   Usage (Phone Mute)
     0x95, 0x01,                //   Report Count (1)
     0x81, 0x02,                //   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
+       0x15, 0x01,        // Logical Minimum (1)
+       0x26, 0x8C, 0x02,  // Logical Maximum (652)
+       0x19, 0x01,        // Usage Minimum (Consumer Control)
+       0x2A, 0x8C, 0x02,  // Usage Maximum (AC Send)
+       0x75, 0x10,        // Report Size (16)
+       0x95, 0x02,        // Report Count (2)
+       0x81, 0x00,        // Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position)
     0xC0,                      // End Collection
 };

@jfedor2
Copy link
Owner

jfedor2 commented Mar 18, 2024

It's because HID Remapper doesn't know how to output "array" usages (its default descriptors don't have them).

But you don't have to replicate the report descriptor exactly, the descriptor just has to have the right usages.

For example you might add something like this (in the same place):

0x05, 0x0C,        // Usage Page (Consumer)
0x1A, 0x20, 0x02,  // Usage Minimum (AC Find and Replace)
0x2A, 0x27, 0x02,  // Usage Maximum (AC Refresh)
0x75, 0x01,        // Report Size (1)
0x95, 0x08,        // Report Count (8)
0x81, 0x02,        // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)

(edited to add Usage Page)

@bjlaur
Copy link
Author

bjlaur commented Mar 18, 2024

Thanks @jfedor2! I really appreciate the feedback.

I gave that a try (with and without the usage page), but I have the same behavior. The hid remapper web tool and monitor the device, but hid-remapper isn't forwarding anything to the host.

I would really appreciate any additional thoughts and/or some guidance on how to troubleshoot the issue.

diff --git a/firmware/src/our_descriptor.cc b/firmware/src/our_descriptor.cc
index 464d716..3b4ddb1 100644
--- a/firmware/src/our_descriptor.cc
+++ b/firmware/src/our_descriptor.cc
@@ -135,6 +135,12 @@ const uint8_t our_report_descriptor_kb_mouse[] = {
     0x09, 0x2F,                //   Usage (Phone Mute)
     0x95, 0x01,                //   Report Count (1)
     0x81, 0x02,                //   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
+0x05, 0x0C,        // Usage Page (Consumer)
+0x1A, 0x20, 0x02,  // Usage Minimum (AC Find and Replace)
+0x2A, 0x27, 0x02,  // Usage Maximum (AC Refresh)
+0x75, 0x01,        // Report Size (1)
+0x95, 0x08,        // Report Count (8)
+0x81, 0x02,        // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
     0xC0,                      // End Collection
 };

@jfedor2
Copy link
Owner

jfedor2 commented Mar 18, 2024

Ah, there's a bug when the descriptor length is above 255 bytes, which we're just hitting when adding this.

The fix would look something like this:

diff --git a/firmware/src/tinyusb_stuff.cc b/firmware/src/tinyusb_stuff.cc
index 22e4951..590e596 100644
--- a/firmware/src/tinyusb_stuff.cc
+++ b/firmware/src/tinyusb_stuff.cc
@@ -85,7 +85,8 @@ uint8_t const* tud_descriptor_device_cb() {
 // Application return pointer to descriptor
 // Descriptor contents must exist long enough for transfer to complete
 uint8_t const* tud_descriptor_configuration_cb(uint8_t index) {
-    desc_configuration[25] = our_descriptor->descriptor_length;  // XXX there has to be a better way
+    desc_configuration[25] = TU_U16_LOW(our_descriptor->descriptor_length);
+    desc_configuration[26] = TU_U16_HIGH(our_descriptor->descriptor_length);
     return desc_configuration;
 }

@bjlaur
Copy link
Author

bjlaur commented Mar 18, 2024

It's working now. Thanks for your help @jfedor2 !

Can I ask why we don't just add all of the 'AC' keys to the descriptor so that all of these will be supported if a users keyboard happens to have them?

@bjlaur
Copy link
Author

bjlaur commented Mar 18, 2024

Thanks again! I really appreciate the assistance.

For future reference, I've commit the fix here:
86e2195
a6eb4e4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants