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

[WIP] UHID mouse/keyboard input #4473

Closed
wants to merge 2 commits into from
Closed

Conversation

yume-chan
Copy link
Contributor

@yume-chan yume-chan commented Nov 28, 2023

Fixes #4034
Fixes #3110
Fixes #3913

Currently only the keyboard part is implemented to get the overall architecture reviewed.

As described in #4034 (comment), the client is responsible for creating and parsing UHID messages, and the server only do the reads and writes to UHID file descriptors. This allows reusing existing AoA mouse/keyboard code to do the HID device emulation, and use native C code to handle UHID messages (which is also defined in C headers).

The new option is named --keyboard-input-mode, after its internal name. Available values are disable, inject, aoa and uhid. aoa deprecates the -K/--hid-keyboard option. When --otg is not specified, all values are accepted, except aoa doesn't work on Windows. When --otg is specified, only disable and aoa are accepted. When --keyboard-input-mode is not speficied, it defaults to inject in normal mode and aoa in OTG mode.

Three new control messages are added: UHID_OPEN, UHID_WRITE and UHID_CLOSE. Each UHID device is a separate file descriptor, so all messages have an int id field, generated by client, maps to a file descriptor on server.

Currently UHID_OPEN and UHID_WRITE both have a byte[] data field (containing a uhid_event structure), so opening file and registering device can be done in one message. But this design is a little bit strange. Maybe we don't need UHID_OPEN at all.

One new device messages are added: UHID_DATA. It contains a uhid_event structure read from the file descriptor. In theory the client should wait for a UHID_DATA messages with START and OPEN UHID message types before starting to send inputs, but in practice these two arrives pretty quickly, so when user starts typing, UHID already finished initializing.

UHID_DATA can also contain OUTPUT UHID messages, these are HID output reports defined in the device's report descriptor, for example LED status for keyboards and rumbles for gamepads. The code to handle these messages haven't been added to the client.

@rom1v
Copy link
Collaborator

rom1v commented Jan 11, 2024

Thank you very much 👍

Sorry for the delay, I don't have much time currently. After a quick glance, it looks good 👍, but I need to review in more details to better understand how everything works.

One problem I encounter however is that it does not use the correct keyboard mapping.

If I use AOA, I can select the physical keyboard mapping once (doc | #2632 (comment)), and it works fine. But with UHID it is not used, so when I press é (French keyboard) it injects 2 (US keyboard). Do you know a solution for this problem?

@yume-chan
Copy link
Contributor Author

Keyboard layout setting is also available for UHID virtual keyboards.

image

I think Android system can distinguish different keyboards and save different layout settings for each device.

The display name is set here: https://github.com/Genymobile/scrcpy/pull/4473/files#diff-ee5bd2180bba9b212ca2eb2799d4272e64029b26751a4e305beac776124aee93R25

@rom1v
Copy link
Collaborator

rom1v commented Jan 12, 2024

Awesome 👌

@yume-chan
Copy link
Contributor Author

yume-chan commented Jan 12, 2024

I added keyboard output report parsing. After the virtual keyboard is registered, or when keyboard modifier status changes, the device will send an output report for keyboard LED status. The client can combine this information with computer keyboard modifier key status to make sure they are in sync.

For example, if the user toggles caps lock outside Scrcpy, on the next key event, the client can detect the local modifier status is different from what is on device, and send an extra caps lock key press event to correct that. Or, if caps lock is on on device but off on PC when Scrcpy starts, the client can turn off caps lock on device. This might resolve #4350.

@rom1v
Copy link
Collaborator

rom1v commented Jan 15, 2024

As a preliminary work, I started to extract the replacement of -K/--hid-keyboard and -M/--hid-mouse by --keyboard-input-mode=… and --mouse-input-mode=… to a separate commit (before the introduction of uhid).

But I think that --keyboard-input-mode= and --mouse-input-mode= are too long as command line parameter names.

Maybe --keyboard-im= and --mouse-im=? Or even --keyboard= and --mouse= (but probably not explicit enough)? What do you think?

@yume-chan
Copy link
Contributor Author

I think only --keyboard and --mouse is fine. Maybe they would block us from adding some unrelated --keyboard-XX options in the future, but I can't think of any concrete examples.

@rom1v
Copy link
Collaborator

rom1v commented Jan 18, 2024

One concrete example could be an option to install and switch to a specific keyboard on the device (AdbKeyboard, NullKeyboard…). Even if such an option is not planned, it could be named --keyboard=….

@rom1v
Copy link
Collaborator

rom1v commented Jan 20, 2024

I like --keyboard and --mouse, so as preliminary commits I added these options with 3 possible values: disabled, sdk and aoa.

Branch uhid.3 (3 last commits).

Copy link
Collaborator

@rom1v rom1v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the AOA/HID keyboard handles two different things:

  1. create the HID events
  2. send them over AOA

To reuse the HID part but using another protocol, your PR abstracts (2) (how to send them):

  • introduce a new trait sc_hid_interface, implemented by sc_aoa and sc_uhid
  • an instance is passed to construct a sc_hid_keyboard

This works, but it brings some complexity:

  • two levels of interfaces implementations (something implementing a sc_key_processor, with one implementation receiving something implementing a sc_hid_interface)
  • the need for an async_message flag (because not all implementations behave exactly the same)
  • all implementation have to create()/destroy() on the client-side (for UHID, I would prefer to create the UHID struct on the server side, the client should not have to handle all these details, like vendor_id/product_id, and the UHID handle should probably be closed on exit from the server side, without any message from the client)

I think "abstracting" the other way around ((1) extracting how to create the HID events) leads to a simpler solution (with less coupling).

My idea is to extract a component to create HID events (regardless of how they will be sent), so that separate keyboard implementations could reuse that same HID tools (without interfaces/traits) when they need to.

That way, we could get 3 separate keyboard/mouse "independant" implementations:

  • sc_keyboard_sdk (previously `sc_keyboard_inject")
  • sc_keyboard_aoa (previously sc_hid_keyboard)
  • sc_keyboard_uhid

Concretly:

  • app/src/hid would contain HID tools
  • app/src/usb would contain the AOA keyboard/mouse implementations
  • app/src/uhid would contain the UHID keyboard/mouse implementations

I implemented preliminary commits which perform all these refactors with the current features: uhid.10

From this branch, you could more easily add a UHID implementation (independently of the AOA keyboard).

What do you think?

app/src/hid/uhid.h Show resolved Hide resolved
@yume-chan
Copy link
Contributor Author

I think "abstracting" the other way around ((1) extracting how to create the HID events) leads to a simpler solution (with less coupling).

I agree with that. In fact, my app is using this architecture.

@castillofrancodamian
Copy link

How do I change the keyboard language? I already tried changing the language of the physical keyboard on Android but it still doesn't work.

@rom1v
Copy link
Collaborator

rom1v commented Feb 24, 2024

Here is my current branch: uhid.20.

UHID and mod synchronization work. Please review and test. :)

I also added a new shortcut MOD+k to open physical keyboard settings easily.

Next steps:

  • documentation
  • HID mouse

@rom1v
Copy link
Collaborator

rom1v commented Feb 25, 2024

I fixed several issues, and implemented mouse UHID: uhid.24

@rom1v
Copy link
Collaborator

rom1v commented Feb 25, 2024

Note: on a Nexus 5 with Android 6, I get a permission denied when opening /dev/uhid (branch uhid.26):

java.io.IOException: android.system.ErrnoException: open failed: EACCES (Permission denied)
$ adb shell ls /dev/uhid
/dev/uhid: Permission denied

@anotheruserofgithub
Copy link

Hello @rom1v, I'm encountering an issue when trying to build uhid.27:

app/meson.build:177:0: ERROR: File src/uhid/uhid_output.c does not exist.

It looks like you forgot to add uhid_output.c in commit e4e6ad5, or am I mistaken?

@rom1v
Copy link
Collaborator

rom1v commented Feb 26, 2024

Oops, indeed some files were not committed.

I forced pushed uhid.27 with the missing files.

@anotheruserofgithub
Copy link

A few minor things I saw by looking quickly at the mouse implementation:

diff1
diff --git a/app/src/uhid/mouse_uhid.c b/app/src/uhid/mouse_uhid.c
index c3f78fd1..77446f9e 100644
--- a/app/src/uhid/mouse_uhid.c
+++ b/app/src/uhid/mouse_uhid.c
@@ -10,7 +10,7 @@
 #define UHID_MOUSE_ID 2
 
 static void
-sc_mouse_uhid_send_input(struct sc_mouse_uhid *kb,
+sc_mouse_uhid_send_input(struct sc_mouse_uhid *mouse,
                          const struct sc_hid_event *event, const char *name) {
     struct sc_control_msg msg;
     msg.type = SC_CONTROL_MSG_TYPE_UHID_INPUT;
@@ -20,7 +20,7 @@ sc_mouse_uhid_send_input(struct sc_mouse_uhid *kb,
     memcpy(msg.uhid_input.data, event->data, event->size);
     msg.uhid_input.size = event->size;
 
-    if (!sc_controller_push_msg(kb->controller, &msg)) {
+    if (!sc_controller_push_msg(mouse->controller, &msg)) {
         LOGE("Could not send UHID_INPUT message (%s)", name);
     }
 }
diff2
diff --git a/app/src/mouse_sdk.c b/app/src/mouse_sdk.c
index 620fb52c..37ba0736 100644
--- a/app/src/mouse_sdk.c
+++ b/app/src/mouse_sdk.c
@@ -57,7 +57,7 @@ convert_touch_action(enum sc_touch_action action) {
 
 static void
 sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp,
-                                    const struct sc_mouse_motion_event *event) {
+                                        const struct sc_mouse_motion_event *event) {
     if (!event->buttons_state) {
         // Do not send motion events when no click is pressed
         return;
@@ -83,7 +83,7 @@ sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp,
 
 static void
 sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp,
-                                    const struct sc_mouse_click_event *event) {
+                                       const struct sc_mouse_click_event *event) {
     struct sc_mouse_sdk *m = DOWNCAST(mp);
 
     struct sc_control_msg msg = {
@@ -105,7 +105,7 @@ sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp,
 
 static void
 sc_mouse_processor_process_mouse_scroll(struct sc_mouse_processor *mp,
-                                   const struct sc_mouse_scroll_event *event) {
+                                        const struct sc_mouse_scroll_event *event) {
     struct sc_mouse_sdk *m = DOWNCAST(mp);
 
     struct sc_control_msg msg = {
diff --git a/app/src/uhid/mouse_uhid.c b/app/src/uhid/mouse_uhid.c
index c3f78fd1..a4aa289b 100644
--- a/app/src/uhid/mouse_uhid.c
+++ b/app/src/uhid/mouse_uhid.c
@@ -27,7 +27,7 @@ sc_mouse_uhid_send_input(struct sc_mouse_uhid *kb,
 
 static void
 sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp,
-                                    const struct sc_mouse_motion_event *event) {
+                                        const struct sc_mouse_motion_event *event) {
     struct sc_mouse_uhid *mouse = DOWNCAST(mp);
 
     struct sc_hid_event hid_event;
@@ -38,7 +38,7 @@ sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp,
 
 static void
 sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp,
-                                   const struct sc_mouse_click_event *event) {
+                                       const struct sc_mouse_click_event *event) {
     struct sc_mouse_uhid *mouse = DOWNCAST(mp);
 
     struct sc_hid_event hid_event;
@@ -49,7 +49,7 @@ sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp,
 
 static void
 sc_mouse_processor_process_mouse_scroll(struct sc_mouse_processor *mp,
-                                    const struct sc_mouse_scroll_event *event) {
+                                        const struct sc_mouse_scroll_event *event) {
     struct sc_mouse_uhid *mouse = DOWNCAST(mp);
 
     struct sc_hid_event hid_event;
diff --git a/app/src/usb/mouse_aoa.c b/app/src/usb/mouse_aoa.c
index 93b32328..adb0cfce 100644
--- a/app/src/usb/mouse_aoa.c
+++ b/app/src/usb/mouse_aoa.c
@@ -13,7 +13,7 @@
 
 static void
 sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp,
-                                    const struct sc_mouse_motion_event *event) {
+                                        const struct sc_mouse_motion_event *event) {
     struct sc_mouse_aoa *mouse = DOWNCAST(mp);
 
     struct sc_hid_event hid_event;
@@ -27,7 +27,7 @@ sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp,
 
 static void
 sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp,
-                                   const struct sc_mouse_click_event *event) {
+                                       const struct sc_mouse_click_event *event) {
     struct sc_mouse_aoa *mouse = DOWNCAST(mp);
 
     struct sc_hid_event hid_event;
@@ -41,7 +41,7 @@ sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp,
 
 static void
 sc_mouse_processor_process_mouse_scroll(struct sc_mouse_processor *mp,
-                                    const struct sc_mouse_scroll_event *event) {
+                                        const struct sc_mouse_scroll_event *event) {
     struct sc_mouse_aoa *mouse = DOWNCAST(mp);
 
     struct sc_hid_event hid_event;

@anotheruserofgithub
Copy link

Great job! This is a very nice solution. ;)

A few years ago I implemented uinput & uhid injection on top of scrcpy for showing the mouse pointer, but I used JNI and didn't know that the mouse could be captured by SDL thus I used absolute motion on the client side and had to calibrate the pointer offset at the beginning on the server side (force the mouse outside the screen because its position gets clamped, then you can work with increments) so that both the pointers on the device and on the computer are aligned. I'm probably wrong but I'm wondering, maybe this could allow you to also handle touch events even though the mouse is in relative coordinates? I can clean up my code and share it with you if you wish to have a look.

@rom1v
Copy link
Collaborator

rom1v commented Feb 26, 2024

diff1

Thank you, fixed locally.

diff2

This is to not exceed 80 chars (that's maybe a stupid rule, but for consistency I stick to it).

I'm probably wrong but I'm wondering, maybe this could allow you to also handle touch events even though the mouse is in relative coordinates?

I'm pretty sure it should be possible to simulate a HID touch screen (with absolute coordinates). To be investigated…

@anotheruserofgithub
Copy link

This is due to not exceed 80 chars (that's maybe a stupid rule, but for consistency I stick to it).

Oh, sorry, that makes sense!

@rom1v
Copy link
Collaborator

rom1v commented Feb 29, 2024

#4473 (comment)

So apparently it fails on Android 6. It works on Android 11.

Could you please test on Android 7, 8, 9 and 10 to determine from which Android version it works?

@yume-chan
Copy link
Contributor Author

I can only confirm it works on Android 10.

uhid.32 branch LGTM, I only corrected some typos: yume-chan@503d0f7

rom1v added a commit that referenced this pull request Feb 29, 2024
An event contained several fields:
 - the accessory id
 - the HID event data
 - a field ack_to_wait specific to the AOA implementation.

Extract the HID event part to prepare the factorization of HID event
creation.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Split the keyboard implementation using AOA and the code handling HID
events, so that HID events can be reused for another protocol (UHID).

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Split the mouse implementation using AOA and the code handling HID
events, so that HID events can be reused for another protocol (UHID).

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Rename {keyboard,mouse}_inject to {keyboard,mouse}_sdk.

All implementations "inject" key events and mouse events, what differs
is the mechanism. For these implementations, the Android SDK API is used
to inject events.

Note that the input mode enum variants were already renamed
(SC_KEYBOARD_INPUT_MODE_SDK and SC_MOUSE_INPUT_MODE_SDK).

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
rom1v added a commit that referenced this pull request Feb 29, 2024
There is a dependency cycle in the initialization order:
 - keyboard depends on controller
 - controller depends on acksync
 - acksync depends on keyboard initialization

To break this cycle, bind the async instance to the controller in a
second step.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
The UHID keyboard initializer will need the controller.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
On close, the controller is expected to throw an IOException because the
socket is closed, so the exception was ignored.

However, message handling actions may also throw IOException, and they
must not be silently ignored.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Use the following command:

    scrcpy --keyboard=uhid

PR #4473 <#4473>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
rom1v added a commit that referenced this pull request Feb 29, 2024
Refactor DeviceMessage as a queue of message. This will allow to add
other message types.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Use UHID output reports to synchronize CapsLock and VerrNum states.

PR #4473 <#4473>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
rom1v added a commit that referenced this pull request Feb 29, 2024
There is no need to create a UhidManager instance (with its thread) if
no UHID is used.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Call the older startActivityAsUser() instead of
startActivityAsUserWithFeature() so that it also works on older Android
versions.

Fixes #4704 <#4704>
PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
The keyboard settings can be opened by:

    adb shell am start -a android.settings.HARD_KEYBOARD_SETTINGS

Add a shortcut (MOD+k) for convenience if the current keyboard is HID.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Use the following command:

    scrcpy --mouse=uhid

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
The options were deprecated, but for convenience, reassign them to
aliases for --keyboard=uhid and --mouse=uhid respectively.

Their long version (--hid-keyboard and --hid-mouse) remain deprecated.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Initially, if AOA initialization failed, default injection method was
used, in order to use the same command/shortcut when the device is
connected via USB or via TCP/IP, without changing the arguments.

Now that there are 3 keyboard modes, it seems unexpected to switch to
another specific mode if AOA fails (and it is inconsistent). If the user
explicitly requests AOA, then use AOA or fail.

Refs #2632 comment <#2632 (comment)>
PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Fail if an option specific to --keyboard=sdk is passed with another
keyboard input mode.

PR #4473 <#4473>
rom1v added a commit that referenced this pull request Feb 29, 2024
Rework the documentation to present the keyboard and mouse input modes.

PR #4473 <#4473>
@rom1v
Copy link
Collaborator

rom1v commented Feb 29, 2024

uhid.38 merged into dev 🚀

@rom1v rom1v closed this Feb 29, 2024
@rom1v rom1v mentioned this pull request Mar 3, 2024
@anotheruserofgithub
Copy link

anotheruserofgithub commented Mar 3, 2024

Could you please test on Android 7, 8, 9 and 10 to determine from which Android version it works?

@rom1v See these commits in the history of system/core/rootdir/ueventd.rc:

You can see when the permission changed from system/bluetooth to uhid/uhid and you can then find in which API level it first came out.

Does that mean that uinput is also available since Android 10? Maybe some ROMs still disallow it (see table in OP of #2130)? I remember it's a bit simpler to use (no need to figure out a descriptor) but I don't know if it's faster than uhid.

@chldbwnstm
Copy link

Hi @yume-chan ,

Thanks for the update. I was just wondering if I could send a software message to scrcpy server like
click(300,300) and it should click the coordinates using the scrcpy server's mouse input structure.
Can this be achieved with the new UHID mouse feature?

@rom1v
Copy link
Collaborator

rom1v commented Mar 6, 2024

@chldbwnstm No, to factorize the code between AOA and UHID, the client sends the HID payloads, which are created by the client (but the UHID structure is created on the device side in the merged version).

@anotheruserofgithub
Copy link

In response to #4835 (comment), following up #4473 (comment):

I am quite confident this has been changed in Android 8.1.0 (API level 27), look for this commit in the history:

And permission for /dev/uinput changed in Android 10.0.0 (API level 29), look for this commit in the history:

But some vendors might have disabled it, or uhid permission is not granted to normal users.
Or I am just completely mistaken. ;)

@anotheruserofgithub
Copy link

#4473 (comment) plus kernel version should be >= 3.15, see #4751 (comment).

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

Successfully merging this pull request may close these issues.

None yet

5 participants