Jump to content

Recommended Posts

I've implemented a new feature in Lineage OS Pro¹'s keyboard driver to be able to use Fn+Tab as Alt+Tab as this is most convenient for multitasking with two thumbs.
This thread is to track that issue specifically so please stay on topic.

I'm trying to contribute this back to official Lineage repository.

Here is my change request on Gerrit:

https://review.lineageos.org/c/LineageOS/android_kernel_fxtec_msm8998/+/315155

There was also a pull request on GitHub but that's just going to be closed as the GitHub repositories are just mirrors I guess:

https://github.com/LineageOS/android_kernel_fxtec_msm8998/pull/1

Here is Lineage OS Pro¹ Linux Kernel:

https://github.com/LineageOS/android_kernel_fxtec_msm8998

Here is my private fork of the Pro¹ Linux Kernel:

https://github.com/Slion/android_kernel_fxtec_msm8998

Related threads:

https://community.fxtec.com/topic/3347-how-to-customize-the-keyboard-layout-on-lineageos-181/?do=findComment&comment=58949
https://community.fxtec.com/topic/3346-lineageos-181-official-release-for-pro1/

Edited by Slion
  • Like 1
  • Thanks 3
Link to post
Share on other sites
29 minutes ago, Slion said:

I've implemented a new feature in Lineage OS Pro¹'s keyboard driver to be able to use Fn+Tab as Alt+Tab as this is most convenient for multitasking with two thumbs.

That would make a lot of sense, indeed.

If the patch gets accepted in the kernel branch, would it be in effect for all LOS branches (16.0, 17.1, and 18.1), or just for 18.1?

I could test against 16.0 by applying the patch to my local copy of 16.0. Should I?

Edited by claude0001
  • Like 1
Link to post
Share on other sites
2 minutes ago, claude0001 said:

If the patch gets accepted in the kernel branch, would it be in effect for all LOS branches (16.0, 17.1, and 18.1), or just for 18.1?

No, I've only submitted to 18.1 not sure what's the process is to propagate it to 16.0. As it is if my change is merged it only goes to 18.1 branch.

5 minutes ago, claude0001 said:

I could test against 16.0 by applying the patch to my local copy of 16.0. Should I?

That would be great. If the driver code is still the same then you can just copy the file.

Link to post
Share on other sites
1 minute ago, Slion said:

No, I've only submitted to 18.1 not sure what's the process is to propagate it to 16.0. As it is if my change is merged it only goes to 18.1 branch.

That would be great. If the driver code is still the same then you can just copy the file.

We'll see. I think the proposed behaviour is definitely useful.

If the patch does not apply cleanly, I'll try to backport it. If all goes well, I'll start a LOS 16 build with your new driver code in the evening and will report tomorrow.

  • Thanks 1
Link to post
Share on other sites
14 minutes ago, EskeRahn said:

Alt+Sh+Tab is also useful, so would be nice if Arrow+Sh+Tab could mirror that as well, as this could be done with thumb+index left side.
(Your change might do that already, have not looked at the code)

Good point, Fn+Shit+Tab works as Alt+Shift+Tab would with that patch already.

Edited by Slion
  • Thanks 1
Link to post
Share on other sites
6 hours ago, Slion said:

I've implemented a new feature in Lineage OS Pro¹'s keyboard driver to be able to use Fn+Tab as Alt+Tab as this is most convenient for multitasking with two thumbs

Nothing against it, but just to be clear, what do you mean with Fn? The key with the F stylized after the Fxtec logo?

  • Like 1
Link to post
Share on other sites

@Slion, I think you introduced a typo in the latest revision of the patch. Build fails with

../../../../../../kernel/fxtec/msm8998/drivers/input/keyboard/qx1000.c:1709:41: error: expected ';' after expression
        size = sizeof(struct gpio_keys_drvdata)
                                               ^

I think that line should stay like in the original code:

size = sizeof(struct gpio_keys_drvdata) +
			pdata->nbuttons * sizeof(struct gpio_button_data);

You probably deleted the "+" by accident in the latest edit.

  • Like 1
  • Thanks 1
Link to post
Share on other sites

This is a great idea, but there is a much simpler way to get most of the functionality you want. See 

https://review.lineageos.org/c/LineageOS/android_kernel_fxtec_msm8998/+/315172

and

https://review.lineageos.org/c/LineageOS/android_device_fxtec_pro1/+/315173

With these two small changes Fn+TAB will bring up the task switcher. The behavior is a little different as you must press the key combo twice to switch to the last app (vs. automatically switching when the keys are released). Also, these patches introduce a bug that disables Alt+TAB :-( That's certainly unexpected behavior, but I'll see if I can find a way to fix it.

  • Thanks 1
Link to post
Share on other sites
1 hour ago, claude0001 said:

I think you introduced a typo in the latest revision of the patch. Build fails with

Could be, I was running some script to clean-up the file, sorry about, that was sloppy.

Edited by Slion
Link to post
Share on other sites
36 minutes ago, Sean McCreary said:

This is a great idea, but there is a much simpler way to get most of the functionality you want. See 

https://review.lineageos.org/c/LineageOS/android_kernel_fxtec_msm8998/+/315172

and

https://review.lineageos.org/c/LineageOS/android_device_fxtec_pro1/+/315173

With these two small changes Fn+TAB will bring up the task switcher. The behavior is a little different as you must press the key combo twice to switch to the last app (vs. automatically switching when the keys are released). Also, these patches introduce a bug that disables Alt+TAB 😞 That's certainly unexpected behavior, but I'll see if I can find a way to fix it.

I vaguely recall playing with that functionality using KCM on stock but you just don't get proper Alt+Tab and Alt+Shift+Tab behaviour…

Edited by Slion
Link to post
Share on other sites
30 minutes ago, Slion said:

I vaguely recall playing with that functionality use KCM on stock but you just don't get proper Alt+Tab and Alt+Shift+Tab…

Yes, the behavior is a bit different. Instead of cycling through the list of apps by holding ALT (or Shift+ALT) and pressing TAB multiple times, it only toggles between the two most recent apps when TAB is pressed multiple times. But you can still select which app you want using the touchscreen.

The code paths for these two methods of activating the task switcher are different. Your changes to the kernel driver are triggering this code:

https://github.com/LineageOS/android_frameworks_base/blob/lineage-18.1/services/core/java/com/android/server/policy/PhoneWindowManager.java#L3421

The behavior triggered by the single keycode event is handled differently, starting here:

https://github.com/LineageOS/android_frameworks_base/blob/lineage-18.1/services/core/java/com/android/server/policy/PhoneWindowManager.java#L3163

  • Thanks 1
Link to post
Share on other sites
1 hour ago, Sean McCreary said:

his is a great idea, but there is a much simpler way to get most of the functionality you want.

I don't think we want most of the functionality. We want the actual Alt+Tab functionality. that the point of those changes.

1 hour ago, Sean McCreary said:

The behavior is a little different as you must press the key combo twice to switch to the last app

Yeah we don't want that when we can have the real thing. Reminds me of a bug they introduced lately in Vivaldi Web Browser where you have to hit Ctrl+Tab twice to get to your previous web page, that pissed the hell out of me 🤣
 

I'm open to alternative solution but I doubt there any while you can't remap the right Fn key from KCM to actual Alt.
I was floating the idea of having a new keymap that would not mask the right Fn key from the system but that's not my favourite solution ATM.

The proposed change does not affect the rest of the driver functionalities.

  • Like 1
Link to post
Share on other sites
3 minutes ago, Slion said:

I don't think we want most of the functionality. We want the actual Alt+Tab functionality.

I agree. The point is to comfortably switch apps while keeping your thumbs on both sides of the keyboard and not having to use the touch screen.

3 minutes ago, Slion said:

I think fixed it in the last patch. Did a build and testing it again.

Yes. I had added back the "+" manually and it is building now ... 60% and going strong. I expect a flashable LOS 16.0 with Fn-Tab functionality in 2 hours or so ... 🙂

  • Thanks 2
Link to post
Share on other sites

I have one more bit of information that might help you simplify your patch. The keycode definitions in qx1000.c support modified keycodes, e.g.

KEY_TAB | KF_ALT

While this does the expected thing, triggering the code path you want, it does not support multiple keyup events expected by PhoneWindowManager.java. Consequently if you make these changes:

diff --git a/drivers/input/keyboard/qx1000.c b/drivers/input/keyboard/qx1000.c
index db637ff17455..d53cbeb4ec7a 100755
--- a/drivers/input/keyboard/qx1000.c
+++ b/drivers/input/keyboard/qx1000.c
@@ -304,7 +304,7 @@ static const u16 qwerty_fn_keys[AW9523_NR_KEYS] = {
        KEY_U,                          KEY_8 | KF_SHIFT,               KEY_R,                          KEY_5 | KF_SHIFT,
        /* 56..63 */
        KEY_BACK,                       KEY_1 | KF_SHIFT,               KEY_RESERVED,                   KEY_RESERVED,
-       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB,                        KEY_RESERVED,
+       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB | KF_ALT,               KEY_RESERVED,
 };
 
 static const u16 qwertz_keys[AW9523_NR_KEYS] = {
@@ -357,7 +357,7 @@ static const u16 qwertz_fn_keys[AW9523_NR_KEYS] = {
        KEY_8 | KF_ALTGR,               KEY_8 | KF_SHIFT,               KEY_T,                          KEY_5 | KF_SHIFT,
        /* 56..63 */
        KEY_BACK,                       KEY_1 | KF_SHIFT,               KEY_RESERVED,                   KEY_RESERVED,
-       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB,                        KEY_RESERVED,
+       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB | KF_ALT,               KEY_RESERVED,
 };
 
 static int aw9523b_i2c_read(struct i2c_client *client, char *writebuf,

then you can bring up the task switcher with Fn+TAB, but there's no way to cycle the list other than using the touchscreen.

  • Like 1
Link to post
Share on other sites
6 minutes ago, Sean McCreary said:

I have one more bit of information that might help you simplify your patch. The keycode definitions in qx1000.c support modified keycodes, e.g.

KEY_TAB | KF_ALT

While this does the expected thing, triggering the code path you want, it does not support multiple keyup events expected by PhoneWindowManager.java. Consequently if you make these changes:

diff --git a/drivers/input/keyboard/qx1000.c b/drivers/input/keyboard/qx1000.c
index db637ff17455..d53cbeb4ec7a 100755
--- a/drivers/input/keyboard/qx1000.c
+++ b/drivers/input/keyboard/qx1000.c
@@ -304,7 +304,7 @@ static const u16 qwerty_fn_keys[AW9523_NR_KEYS] = {
        KEY_U,                          KEY_8 | KF_SHIFT,               KEY_R,                          KEY_5 | KF_SHIFT,
        /* 56..63 */
        KEY_BACK,                       KEY_1 | KF_SHIFT,               KEY_RESERVED,                   KEY_RESERVED,
-       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB,                        KEY_RESERVED,
+       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB | KF_ALT,               KEY_RESERVED,
 };
 
 static const u16 qwertz_keys[AW9523_NR_KEYS] = {
@@ -357,7 +357,7 @@ static const u16 qwertz_fn_keys[AW9523_NR_KEYS] = {
        KEY_8 | KF_ALTGR,               KEY_8 | KF_SHIFT,               KEY_T,                          KEY_5 | KF_SHIFT,
        /* 56..63 */
        KEY_BACK,                       KEY_1 | KF_SHIFT,               KEY_RESERVED,                   KEY_RESERVED,
-       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB,                        KEY_RESERVED,
+       KEY_2 | KF_SHIFT,               KEY_4 | KF_SHIFT,               KEY_TAB | KF_ALT,               KEY_RESERVED,
 };
 
 static int aw9523b_i2c_read(struct i2c_client *client, char *writebuf,

then you can bring up the task switcher with Fn+TAB, but there's no way to cycle the list other than using the touchscreen.

Yes, I did try that using custom keymap before going out of my way to modify the driver as it did look like the only way to implement actual alt+tab with fn+tab.
Also since the driver already implements forced modifiers it was quite easy to get it done.

Link to post
Share on other sites
3 hours ago, Slion said:

I think I fixed it in the last patch. Did a build and testing it again.

It built alright in the LOS 16 tree. Flashed it some minutes ago.

Fn-Tab and Fn-Shift-Tab work as expected. I could not spot any regressions up to now. I will keep this build for the weekend and report.

  • Thanks 1
Link to post
Share on other sites
  • 2 months later...

After a very long delay, this feature has finally been merged in LineageOS 18.1. Beginning with tomorrow's build there will be a new option in Settings to make 'fn_r' (the right-hand diagonal yellow arrow key) send ralt/AltGr. This will enable full Windows-style task switching behavior, as well as assist entering accented characters when typing languages other than English. Enabling this feature will disable the use of this key to enter the yellow glyphs printed on each key, but all of them can be generated by chording with 'shift' instead, with the exception of '/' and '?'. Since these two keys are on the same side of the keyboard as fn_r, we are hoping everyone has already been using fn_l to enter these two characters. 

The key remapping API has also been extended, allowing a custom keymap to change the keycodes generated by the six keys with dedicated GPIO lines. These are 'home' (the F logo key), 'shift', 'ctrl', 'alt', 'fn_l' (the left-hand diagonal yellow arrow key), and 'fn_r'. These keys can be specified in a custom keymap using the following key numbers:

64:home
65:shift
66:ctrl
67:alt
68:fn_l
69:fn_r

The new toggle in Settings simply writes a small custom map containing "69:0064:0064" to change the keycode generated by fn_r. Note that the second value in this string is syntactically required, but is ignored by the keyboard driver for these six keys. By convention, both keycode values should always be the same in a custom keymap.

  • Like 5
  • Thanks 2
Link to post
Share on other sites
On 11/27/2021 at 10:22 PM, Sean McCreary said:

After a very long delay, this feature has finally been merged in LineageOS 18.1.

Thank you - I didn't have chance for it to test (on other device) before but now (as part of the last update) it was installed on my daily driver.
Naturally, it works. 🙂

Maybe one option what would be good to have is a setting to pick a custom layout file then copy it to its appropriate folder, that would eliminate the need of any kind of root mechanisms. However, it is an Android programming question and not kernel-related.

Edited by VaZso
  • Thanks 1
Link to post
Share on other sites
23 hours ago, VaZso said:

Thank you - I didn't have chance for it to test (on other device) before but now (as part of the last update) it was installed on my daily driver.
Naturally, it works. 🙂

Maybe one option what would be good to have is a setting to pick a custom layout file then copy it to its appropriate folder, that would eliminate the need of any kind of root mechanisms. However, it is an Android programming question and not kernel-related.

I'm hoping to improve the existing Settings app as part of the changes to support the Pro1X. An option to install a custom keymap from within Settings seems like a good idea, and I want to add support for a full custom keymap generator app in the driver. This would include a way to ask the driver what default keymaps are supported, and what keys are in each map (i.e. what is printed on each key). Since the default QWERTY map on the Pro1X will be different from the Pro1, the standard keyboard layout names aren't sufficient specify the correct 'default' keymap.

  • Thanks 3
Link to post
Share on other sites
52 minutes ago, Sean McCreary said:

I'm hoping to improve the existing Settings app as part of the changes to support the Pro1X. An option to install a custom keymap from within Settings seems like a good idea, and I want to add support for a full custom keymap generator app in the driver. This would include a way to ask the driver what default keymaps are supported, and what keys are in each map (i.e. what is printed on each key). Since the default QWERTY map on the Pro1X will be different from the Pro1, the standard keyboard layout names aren't sufficient specify the correct 'default' keymap.

Right, pick up of a file is the simplest but working solution, a full-featured GUI is a more complicated but much better solution.
A custom keymap generator is a very good idea, especially if it also has an import/export function, that would be the non plus ultra. 🙂

Also you are right about the different QWERTY variation Pro1-X will have.
Although it could be differentiated based on device names (Pro1 and Pro1-X), it is really a much better solution if related codes of these devices could be the same, so if there is a GUI generator where user may pick the real physical keymap (s)he has, that would be a perfect way to do.

Especially if F(x)tec will also sell replacement keypads or AZERTY / other keymaps separately, old users of Pro1 may want to replace their keymaps using the new variants, so it is extremely good to have an option to simply set the replaced layout at GUI interface even if it was non-existent when original Pro1 was launched.

So again, it is a perfect idea.

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

Terms