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
Physical keyboard to android support #6097
Conversation
/claim #3556 |
Cargo.toml
Outdated
@@ -64,7 +64,7 @@ default-net = "0.14" | |||
wol-rs = "1.0" | |||
flutter_rust_bridge = { version = "1.75", features = ["uuid"], optional = true} | |||
errno = "0.3" | |||
rdev = { git = "https://github.com/fufesou/rdev" } | |||
rdev = { git = "https://github.com/mcfans/rdev" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please merge to https://github.com/fufesou/rdev
@fufesou please review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will open a PR for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finish this first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RequiresApi(Build.VERSION_CODES.N) | ||
fun onKeyEvent(data: ByteArray) { | ||
val keyEvent = KeyEvent.parseFrom(data); | ||
val handler = Handler(Looper.getMainLooper()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can not parse it in rust?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's serialized on rust side by purpose, because I want to use generated KeyEvent class on Kotlin side instead of pass the fields of it on rust side through ffi that would be a lot of parameters and less convenient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree
@fufesou please have more review. |
Have you tested different languages? e.g. German, French. And combination key, e.g. alt + key, will generate special character for different language. |
Not yet. I will try it later. |
👍 👍 👋Win -> Android , en->en, the charaters are all correct for both "Map mode" and "Translate mode". android.input.mp4Some possible improvements
avc permission issuesI'm not familiar with avc issues in android. They're not related to the keyboard issue. But if you're familiar with it, it's good. ======================= update ============================= I see the secure context is I'm trying to find out:
|
I found a new way to do this. Accessibility service has a method called getInputMethod, we could then send key event directly to it.This could solve previous problem.
Old way can only set text and text selection separately, if application doesn't do some special text handling, this is fine.But if app would change text after text set, then our text selection data would be wrong. So I ignored text selection data for correctness. In this new way I leave this to system to handle, arrow key works fine.
This is because findFocus gave a wrong input widget, send event to IME also fix this. |
I tested combination key, I could enter exclamation mark(shift + 1). I couldn't test different languages for now because I found out my android device doesn't support other languages. Need some help on that. |
@RequiresApi(Build.VERSION_CODES.N) | ||
fun onKeyEvent(data: ByteArray) { | ||
val keyEvent = KeyEvent.parseFrom(data); | ||
getInputMethod()?.let { inputMethod -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getInputMethod added in API level 33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may have some modification to make it compatible with lower level target device. @mcfans
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the wrong field, I thought we were using API 33. My bad. So I just keep this for higher version and add the old code back for lower API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with kotlin about how to call higher API on high target device but still can run on low target device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My android device may be the lower version. The input does not work now, and it reports:
D/ffi ( 3011): librustdesk::server::connection: call_main_service_key_event fail:Java exception was thrown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you try it again? I added old code back with a version check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keyboard input works now.
The "Translate mode" works fine. For the "Map mode" (fr -> fr), the the characters are different. It seems the default en-use keyboard layout is used. Maybe it's better hide the "Legacy mode" and "Map mode" if local is desktop and remote is Android. ======================== update ============================= Maybe you need to use the https://developer.android.com/reference/android/view/KeyEvent#KeyEvent "Map mode" means using the "position code" (scancode) to control the key inputs. With "Map mode", people can control the peer as the physical keyboard is connected directly to the peer. |
👍 |
@mcfans 👍 Do you have a plan to implement the "Map mode"? |
I am trying to implement Map mode these days.But there are two problems. First we can only send a keycode that matches US keyboard layout, Android keycode only has those keys. Set scancode of a keyevent is ineffective base on my experiments. Second I only found ways to send key events to application, not send to input method. On Android a hardware keyboard event is sent to input method, then the input method would figure out correct text to insert into application. I failed to find a way to do the same thing. However I have not tested what text would be inserted if the OS language matches input method, because my android device couldn't do that. If that could work, we could add map mode back. |
I fixed translated mode and legacy mode in those commits. |
The scancode should be handled by the input method. It should have nothing to do with the OS language.
I'm not familiar with Android. But it seems you are right, the event should be sent to the system or input method. |
If we can send key event to the system, everything would be perfect. |
So should we merge this? |
AnyDesk's map mode work? |
I tested it. It doesn't work correctly with 1:1 mode on and local input method is fr, its output is US layout keys. Same behavior with my implementation. |
I've tried the ID edit in RustDesk. |
src/ui_session_interface.rs
Outdated
@@ -216,6 +216,11 @@ impl<T: InvokeUiSession> Session<T> { | |||
|
|||
pub fn get_keyboard_mode(&self) -> String { | |||
let mode = self.lc.read().unwrap().keyboard_mode.clone(); | |||
if self.peer_platform() == crate::PLATFORM_ANDROID { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the logic here is the same as in flutter_ffi.rs. Maybe it's better to merge the code.
if session.peer_platform() == "Android" && mode == KeyboardMode::Map {
return SyncReturn(false);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic in flutter_ffi.rs is used to hide map mode in UI, ui_session_interface.rs is used to override keyboard mode saved in local storage. It’s two part of the same problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s two part of the same problem.
Yes, but if something changed, you have to change to two places, which is error prone.
They are doing almost the same filtering, maybe you can extract the filter code, or some func like should_replace_mode_to(conds, mode) -> Option<mode>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, that's better. Working on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
# Conflicts: # src/server/connection.rs
I made the cursor works for API level lower than 33 just like AnyDesk. |
@mcfans Hi, what architecture did you use for testing? It's strange that I can't run x86_64 on my emulator. |
With the latest commit, I can't enter "Backspace" "Enter" "Left/Up/Right/Down". |
For API level 34, I am using arm64 apk on x86 emulator. It’s okay for Android 11 and above. For API level 28, I am using arm64 apk on arm64 emulator. I have a M2 MacBook, it’s arm64, so it’s fine. You probably need |
Which API level are you using? |
c2703d2 |
@mcfans please fix the conflicts. I will merge then. |
# Conflicts: # Cargo.lock # src/server/connection.rs
src/server/connection.rs
Outdated
log::debug!("call_main_service_key_event fail:{}", e); | ||
} | ||
} else { | ||
log::debug!("encode key event fail:{}", encode_result.err().unwrap()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this unwrap
? BTW, space between :
and {
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's safe here, but I changed it to a match
block anyway.
Thanks for so awaited feature! :) |
#6417 big regression failure. |
I don't know if here's the right place for reporting it but I have a problem with using keyboard in Windows 10 (R. ver.1.2.4)-> Android 11 (R. ver. 1.2.4) scenario. Am I doing something incorrectly or it's a bug? |
This feature is ready in 1.2.4, current night build, https://github.com/rustdesk/rustdesk/releases/tag/nightly |
Description
Support KeyEvent input in android side. Fix issue(#3556)
/claim #3556
Detailed Design
Android doesn't provide an API for send key event to the whole system, since most scenario that needs keyboard input are text input. I used android accessibility service to find current focused text input widget and set text to it. I create an invisible text input widget in InputService, and set current focusing accessibility node info's text and selection to it, send the key event to it then retrieve it's new state and set the new text back to the current focusing node.In this way, we don't need to handle what different key will do to text on our side, just use system default implementation.Use getInputMethod() API, then send key event to it.
Other Approach
Register an IME to system then use that to send key stroke to system. WIFIKeyboard use this approach. But Android system can only have one IME at a time, receive side will not able to use their on screen IME while using RustDesk, that would make features like chatting not work.
Changes
Test
Tested on:
iOS -> Android,
macOS -> Android(map mode),
windows -> Android(legacy mode)
Compatibility
Android receive side needs update to this version to work.
iOS control side needs update to this version to show a soft keyboard.
Desktop version doesn't need update for legacy keyboard mode, need update for map keyboard mode.
A Video
IMG_1634.MOV