Add mappings for media keys #3022
Conversation
sasha0552
force-pushed
the
devel
branch
8 times, most recently
from
e7fe4b6
to
e889e80
Compare
|
Hi @sasha0552 Thanks for this, particularly the references. I'm going to run out of time to review this before I have to spend some family time next week I'm afraid. This isn't an area I'm that familiar with so it may take me a while. @jsorg71 - do you have some time to look at this? One immediate question I have is your scancodes, particularly that for
The same document only allows a CARD8 for the keycode in the on-wire protocol, even though the Where are you getting these values from? |
|
Hi @matt335672.
It's okay, there's no need to rush.
I got the evdev scancodes from the I believe Xorg can handle scancodes up to unsigned int. However, there just aren't that many scancodes, so I decided to use |
A single keycode can have multiple meanings, I guess, but scancode is different. Scancode comes from the linux kernel. For example: |
|
|
|
Again, I've not spent any significant time on this yet, but regarding the key code limit of 255, this is very much not a solved problem. Here are a couple of links:-
Also, if you have a closer look at and:- I think I've got a lot of mugging up to do on the existing code before I can give your PR the attention it deserves. I'll try to write it up on the wiki. |
|
I think xrdp is passing scancodes, not keycodes. The keycode (
In addition, this key is fully functional with the current code as listed in the table (I tested each key with |
|
I mean, xrdp doesn't even passing scancodes. The process looks like this:
Maybe we can eliminate the internal value and directly map the rdp codes to keysyms & chars in keymap file? |
sasha0552
force-pushed
the
devel
branch
2 times, most recently
from
616adc3
to
c11cb69
Compare
|
I ever worked on around keyboard. I can look in to this but it takes time to remember. |
|
I've had a bit of time today to do some reading up on the code, and I've got a better idea of what's going on within xrdp.
I'm hoping to be able to get a proper review of this done next week. |
|
Right, I think I'm up to speed on what's going on in this area. Here's a summary of the current architecture:- https://github.com/neutrinolabs/xrdp/wiki/Keyboard-mappings First of all; @sasha0552 - thanks for your patience and for the changes you've made to your initial proposal. Things look a lot better now. I'm going to make a couple of other suggestions based on the above link. Please consider these and get back to me with any comments. This patch removes unmapped keys from the keymap files, which makes the diffs easier to follow:- diff --git a/genkeymap/genkeymap.c b/genkeymap/genkeymap.c
index bc5ddba7..51c05b11 100644
--- a/genkeymap/genkeymap.c
+++ b/genkeymap/genkeymap.c
@@ -148,6 +148,10 @@ int main(int argc, char **argv)
e.keycode = i;
}
nbytes = XLookupString(&e, text, 255, &ks, NULL);
+ if (ks == NoSymbol)
+ {
+ continue;
+ }
text[nbytes] = 0;
char_count = mbstowcs(wtext, text, 255);
unicode = 0;
diff --git a/xrdp/lang.c b/xrdp/lang.c
index a998805d..edb03b68 100644
--- a/xrdp/lang.c
+++ b/xrdp/lang.c
@@ -189,6 +189,7 @@ km_read_section(int fd, const char *section_name, struct xrdp_key_info *keymap)
values = list_create();
values->auto_free = 1;
+ g_memset(keymap, '\0', sizeof(*keymap));
if (file_read_section(fd, section_name, names, values) == 0)
{
for (index = names->count - 1; index >= 0; index--)This one explicitly uses --- a/genkeymap/dump-keymaps.sh
+++ b/genkeymap/dump-keymaps.sh
@@ -6,6 +6,11 @@ then
exit 1
fi
+OLD_SETTINGS=$(setxkbmap -query -verbose 4 | sed "s/^\([a-z]\+\):\s*\(.*\)$/-\1 \2/;s/^-options/-option \"\" -option/;s/,/ -option /g" | xargs -d \\n)
+
+# Use evdev rules for these mappings
+setxkbmap -rules evdev
+
# English - US 'en-us' 0x00000409
setxkbmap -model pc104 -layout us
./xrdp-genkeymap ../instfiles/km-00000409.ini
@@ -15,7 +20,6 @@ setxkbmap -model pc104 -layout dvorak
./xrdp-genkeymap ../instfiles/km-00010409.ini
# English - US 'dvp' 0x19360409
-OLD_SETTINGS=$(setxkbmap -query -verbose 4 | sed "s/^\([a-z]\+\):\s*\(.*\)$/-\1 \2/;s/^-options/-option \"\" -option/;s/,/ -option /g" | xargs -d \\n)
setxkbmap -rules xfree86 -model pc105 -layout us -variant dvp -option "" -option compose:102 -option caps:shift -option numpad:sg -option numpad:shift3 -option keypad:hex -option keypad:atm -option kpdl:semi -option lv3:ralt_alt
./xrdp-genkeymap ../instfiles/km-19360409.ini
setxkbmap ${OLD_SETTINGS}
@@ -52,5 +56,5 @@ setxkbmap -model pc104 -layout se
setxkbmap -model pc104 -layout pt
./xrdp-genkeymap ../instfiles/km-00000816.ini
-# set back to en-us
-setxkbmap -model pc104 -layout us
+# set back to entry settings
+setxkbmap ${OLD_SETTINGS} |
|
@matt335672 Thanks for the patches, they look good to me. I have applied them. |
|
@sasha0552 - If I check out your PR, and run How are you running the script on your end? We need to understand why there's a difference, or it's difficult to have any confidence in the script itself. I get the following from As far as neutrinolabs/xorgxrdp#303 goes, I think if we can get evdev keycode support working reliably here, we can probably do a lot of the mapping work in xrdp itself. Let's come back to that when we've bottomed this one out. |
|
@matt335672 setxkbmap -v 10 |
|
That's useful - thanks. I'll update the script to incorporate that. Patch on the way... |
|
I think we are using the base rules not evdev. I've seen these defined in evdev but not all in base. We can just add them in Xorg that is not a problem. The problem I've seen is mstsc does not send them. @sasha0552 when you say 'tested/works', what client is that. |
|
@jsorg71 I'm using FreeRDP (xfreerdp). |
|
@jsorg71 - I'm thinking that moving to evdev-based keycodes would be a good idea in any case. They're the default on Linux and FreeBSD, and it should make a lot of use-cases simpler to support. What are your thoughts on that? You've got a lot more experience in this area than I have. |
|
We can implement this even if it's not standard but it would be nice to confirm these freerdp rdp scancodes are correct either with a MS client for some MSDN documentation. |
|
@sasha0552 - here's a patch which should make things a bit more consistent:- --- a/genkeymap/dump-keymaps.sh
+++ b/genkeymap/dump-keymaps.sh
@@ -1,9 +1,22 @@
#!/bin/sh
-if ! command -v setxkbmap >/dev/null
-then
- echo "error, setxkbmap not found"
- exit 1
+# Run the keymap extraction in a clean X session
+if [ "$1" != _IN_XVFB_SESSION_ ]; then
+ # All commands available?
+ ok=1
+ for cmd in setxkbmap xvfb-run xauth; do
+ if ! command -v $cmd >/dev/null
+ then
+ echo "Error. $cmd not found" >&2
+ ok=
+ fi
+ done
+ if [ -z "$ok" ]; then
+ exit 1
+ fi
+
+ xvfb-run --auto-servernum $0 _IN_XVFB_SESSION_
+ exit $?
fi
OLD_SETTINGS=$(setxkbmap -query -verbose 4 | sed "s/^\([a-z]\+\):\s*\(.*\)$/-\1 \2/;s/^-options/-option \"\" -option/;s/,/ -option /g" | xargs -d \\n)Do you get the same keymaps for it? @jsorg71 - I can look at mstsc.exe tomorrow, UK time. I've got a Windows laptop and a USB keyboard with Audio Up/Down/Mute keys on it. That should give us a clue whether we're looking in the right area or not. |
|
@matt335672 |
|
I did test mstsc with browser back and forward and got no message to xrdp. X11rdp did use evdev but there was problem. Then I noticed that FreeBSD and Xvnc were using base rules. In fact the only place Xorg used evdev was on Linux console.. Then I noticed it was called evdev because that is the Linux input driver name. It was heavily tied to the evdev Linux driver. This may not be true today and it might be worth another try especially if Xvnc and Xorg on other platforms moved to evdev. |
|
I've done a bit of testing this morning with a UK laptop manufactured by Acer. It's running Windows 10 and mstsc.exe First I applied this patch so I could see what was coming over from the RDP client:- --- a/xrdp/xrdp_wm.c
+++ b/xrdp/xrdp_wm.c
@@ -1573,7 +1573,9 @@ xrdp_wm_key(struct xrdp_wm *self, int device_flags, int scan_code)
int msg;
struct xrdp_key_info *ki;
- /*g_printf("count %d\n", self->key_down_list->count);*/
+ LOG_DEVEL(LOG_LEVEL_DEBUG,
+ "xrdp_wm_key: RDP scancode:0x%04x, flags: 0x%04x",
+ scan_code, device_flags);
scan_code = scan_code % 128;
if (self->popup_wnd != 0)mstsc.exe was configured as follows:-
The laptop keyboard is compact, but has media keys accessed via a key marked '[Fn]':- The useful keys appear to be raise volume, lower volume, play, stop, previous, next. The '[Fn]' key is invisible to the RDP protocol and is probably handled by the hardware, or by the keyboard driver. I got the following results from this keyboard:-
Next I plugged in a USB keyboard. This is a cheap Dell keyboard. At a previous job we used to get crates of these when we took delivery of a bunch of Optiplex machines. We didn't need them. This keyboard also has media keys. Some of these are accessed via an Fn key and some aren't. Again the Fn key is not visible at an RDP protocol level:-
Useful keys on this keyboard are search, previous, play, next, mute, lower volume, raise volume. I got the following results from this keyboard:-
Both of these are consistent. Also, putting them together, I can match them up with @sasha0552's table above:-
In conclusion there appears to be some sort of standard at work here. |
|
There's a table of scancodes with references at AIUI the Fn key is special and not sent to the OS by itself, only in combination with other "functional" keys. |
|
I get minor differences on Ubuntu 22.04 when I run the dump script, but nothing significant. FreeBSD 14 doesn't have As regards evdev vs base keymaps, there are three places which need to be aware of the RDP scancode to keycode map currently:-
That's two too many. I think we should have one place where the scancode to keycode map is stored. This can be hard-coded, or externally located in a file. Both are good. Having one location addresses the issue of which mapping to use as we can easily change it. That said, I'd be happy to merge this as-is and then work on unifying the maps. Thoughts? |
|
I see now why I've never seen these keys work with mstsc. You need to be full screen or you need to set 'Keyboard | Apply Windows key combinations' to 'On the remote computer'. Strange that these keys are considered like Alt Tab. |
|
That's possibly it. I had a slight 'duh' moment yesterday with the laptop keyboard when it took me a couple of minutes to realise why the brightness keys weren't being passed through to xrdp! In my defence, it was before my normal morning coffee. My current thoughts on where we go from this are:-
If we then need to use base mappings on platforms without evdev we just need to update 2) Thoughts/objections anyone? I know this is a complicated area However, if we're going to get these maps sorted out I need to really be on top of it as we'll potentially have a few user issues to triage when it goes live. |
|
I was going to add a review comment about some of the keyboards not being auto-generated, but having looked into this there seems to be a significant issue here which I don't think we can address in this PR. The following keyboards in the instfiles directory are not currently autogenerated:-
Having tried to add these, I'm getting significant differences around some of the keys, in particular AE11, AE12, AD11 and AD12. It's possible these files were generated on a different X server, or the users weren't bothered about these keys as they were only using the keymaps to log in - I can't tell which. However, at the moment it seems these keymap files are unmaintainable. I'll sweep these up when I regenerate the files to map directly from RDP scancodes. |
|
@sasha0552 - sorry it's taking a while to merge this, but I'm still exploring this area in general. I've done a bit more testing with the generated keymaps, and I've found another potential snag for the future which is related to a change in the delivered XKB config. When I re-run the https://gitlab.freedesktop.org/xkeyboard-config/xkeyboard-config/-/merge_requests/229/diffs Before that patch, Shift and NumLock had the same effect on the keypad. After the patch, Shift no longer produces the same results. Why does this matter? There's some code in Lines 87 to 98 in 0a0a393 In the fragment above,
If I take (for example) the '[noshift]' and '[shift]' sections from You can see that these are different, and the code above generates different results when NumLock is pressed. On an Ubuntu 22.04 system, if I run It seems likely that moving forward we will need to generate a separate NumLock section in the keymaps. |
|
Any progress on this? The media keys are quite important to me |
|
@tim-gromeyer - I'm working through a bit of a backlog at the moment but this is on the list. Be aware that this PR and #3039 will get media keys working for VNC-based sessions. For xorgxrdp-based sessions there's a bit more work to do. #3039 centralises the RDP scancode -> X11 keycode mappings in xrdp itself, and makes these available to xorgxrdp. We also need to modify xorgxrdp to make use of these. |
|
Thanks @sasha0552, and sorry about the delay. I've rebased #3039, and after running it through CI checks will merge that too. That should let us get on with finishing off neutrinolabs/xorgxrdp#303 as well. |
Fixes #1725, at least for this repository. The three media keys available on my keyboard now work (using Xvnc session type).
Related: neutrinolabs/xorgxrdp#303
TODO:
List of rdp codes
List of xorg keycodes
xmodmap -pkList of keysyms
16173XF86AudioPrev25171XF86AudioNext32121XF86AudioMute34172XF86AudioPlay36174XF86AudioStop46122XF86AudioLowerVolume48123XF86AudioRaiseVolume50180XF86HomePage101225XF86Search102164XF86Favorites103181XF86Reload104136Cancel105167XF86Forward106166XF86Back108163XF86Mail109234XF86AudioMedia110156XF86Launch1111157XF86Launch2