Arian04 / android-hid-client

Android app that allows you to use your phone as a keyboard and mouse WITHOUT any software on the other end (Requires root)
GNU General Public License v3.0
109 stars 4 forks source link

Improve the performance of the sending of each HID report #3

Closed Arian04 closed 1 year ago

Arian04 commented 2 years ago

The current implementation uses su -c with echo to send the report, however, su causes a significant performance hit, and when sending many keys back to back, it really shows.

Possible solutions:

Arian04 commented 2 years ago

a92dbbf4382007faa198018be25dd1e372243a58 partially solves this issue. It adds a performance mode so users can choose between having the keys sent using the current stable method that correctly detects timeouts and is overall more resistant to failure and having the keys sent using the more performant, but less resistant, code. This is a temporary fix until I can figure out how to give the app access to /dev/hidgX without having to run the echo commands in a root shell. Leaving this issue open until I properly fix it.

Overview of both implementations

Old code (Default) - uses su -c to echo the HID report to the character device. It escalates privileges using su -c every single time it sends a key. This significantly increases the time it takes to send each key. When pressing keys somewhat quickly, this is very noticeable. However, since each key is sent in its own process, it's simple to detect timeouts and all other keystrokes will continue as normal.

New code (Performance mode) - Opens a root shell on startup and just basically types the echo command into it each time a key needs to be sent. Avoids having to send each key through a shell command, but I haven't found a way to handle timeouts as gracefully as the other method while doing this. However, the vast majority of timeouts that have occurred in my testing were due to temporary bugs or not having the phone connected to a computer. If the Android device is not connected to a computer when sending keys (and performance mode is enabled), then the shell will hang until it is connected to one. After it is connected to a computer, it recovers pretty fast. However, if the shell ever hangs for some other reason, there currently isn't any code in the app to detect that and it will likely just have to be restarted to fix it. Because of this (and the fact that I just haven't tested it much), performance mode is currently not the default, and I think I'd rather figure out how to write to /dev/hidgX without using su -c for every HID report than make it the default.

Arian04 commented 2 years ago

I've discovered that this is an SELinux-related issue. 19825e2 makes it so, before every key is sent, it checks if SELinux is disabled. If SELinux is disabled, it forgoes using shell commands and instead just writes directly to the character device. This is another temporary fix until I can learn enough about SELinux (or figure out if there's some permission that I overlooked that gives an app access to /dev/*) to allow my app write permissions to /dev/hidgX without insecurely disabling SELinux altogether.

Arian04 commented 2 years ago

b0a0662 addresses part of this issue by doing the actual sending of the HID report on a separate thread and only adding the keys to a queue on the main thread. Currently, this is the status of the three methods of sending keys that I'm messing with:

  1. Sending each key in its own shell command (thus opening a new shell for each HID report) - Horribly slow, even more noticeable when sending a large string through manual input
  2. Sending each key by writing to a singular open shell - Fast enough to where the delay in sending a key isn't noticeable.
  3. Sending each key by directly writing to the character device (ex: /dev/hidg0) - Should be more than fast enough, I'm assuming it'll be similar in performance to the 2nd method, however, as stated previously in this issue, I haven't figured out enough about Android's usage of SELinux policies to get it to work without setenforce 0, which I feel is a terrible solution, so I haven't done much testing with it.

However, in implementing this, I have found a new performance issue that doesn't involve the actual sending of keys. Currently, in order to minimize the TextView's consumption of KeyEvents, I clear it every single time text is entered into it. Without this, certain things like left/right arrow keys can break, because the user would be moving through the text in the TextView, causing it to be consumed there and not processed by the onKeyDown listener. This is definitely a hacky solution, but now it's also insufficient because the setText(null) call is by far the slowest part of the process of adding a key to the queue. I would just run this code in another thread to avoid it causing issues with the processing of keys on the main thread, but a UI element cannot be accessed from a thread other than the main one. I aimed to address this in b0a0662 along with the other performance issues by just removing the setText(null) and rewriting the code to just process the character at the end of the string every time a key is pressed, but, due to how the TextWatcher works, this code would break when certain characters like space were used. bd40c0c fixed up the code to clear the EditText when necessary, but not often enough to cause very noticeable performance issues. Further testing needs to be done, but it seems to work decently well. Ideally, I'll eventually replace the janky EditText and TextWatcher with a custom View (or an existing one that I just haven't found yet) that'll let the onKeyDown listener process all KeyEvents and avoid this problem altogether, but I haven't learned how to do that yet, or even if it's the best solution.

Arian04 commented 2 years ago

In 458c302, I improved the logic behind when to clear the EditText. Currently, the only known issue is that left arrow presses are broken, because the EditText consumes it by moving the cursor left.

Arian04 commented 1 year ago

Found another issue with this. After hitting the enter key, hitting the up and down arrows gets weird because they only work once you go all the way to the top or bottom of the EditText. I had an idea though, maybe check the position of the cursor and clear based on that?

Arian04 commented 1 year ago

After looking back at this issue, I've realized that the last 2 comments should be put in a separate issue (#4)