Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0334e0e3 authored by Josh's avatar Josh
Browse files

Fixed mixup with keycode / modifier mapping. 

Keycode mapping and modifier mapping were done through the same function
and same map, and this caused a conflict where KEYCODE_1 was being
mapped as META_FN_ON. I've separated these two methods

Test: ShortcutHelperCategoriesRepositoryTest
Flag: com.android.systemui.keyboard_shortcut_helper_rewrite
Fix: 372820099
Change-Id: If67ec7919d8dd24c519d5e4d64bd988cb9a07cf6
parent e6634fe6

packages/SystemUI/multivalentTests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt

+53 −24
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.systemui.keyboard.shortcut.data.repository



import android.hardware.input.fakeInputManager

import android.view.KeyEvent.KEYCODE_1

import android.view.KeyEvent.KEYCODE_A

import android.view.KeyEvent.KEYCODE_B

import android.view.KeyEvent.KEYCODE_C
@@ -24,6 +25,7 @@ import android.view.KeyEvent.KEYCODE_D 
import android.view.KeyEvent.KEYCODE_E

import android.view.KeyEvent.KEYCODE_F

import android.view.KeyEvent.KEYCODE_G

import android.view.KeyEvent.META_FUNCTION_ON

import android.view.KeyboardShortcutGroup

import android.view.KeyboardShortcutInfo

import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -86,6 +88,48 @@ class ShortcutHelperCategoriesRepositoryTest : SysuiTestCase() { 
        fakeMultiTaskingSource.setGroups(TestShortcuts.multitaskingGroups)

    }



    @Test

    fun categories_keycodeAndModifiersAreMappedSeparatelyWhenIdentical() =

        testScope.runTest {

            fakeSystemSource.setGroups(simpleGroup(simpleShortcutInfo(KEYCODE_1)))



            helper.toggle(deviceId = 123)

            val categories by collectLastValue(repo.categories)



            val systemCategory = categories?.firstOrNull { it.type == ShortcutCategoryType.System }



            // Keycode 0x8 should be translated to the Key 1 instead of modifier FN

            // which has the same keycode.

            val expectedCategory =

                ShortcutCategory(

                    type = ShortcutCategoryType.System,

                    simpleSubCategory(simpleShortcut("1")),

                )



            assertThat(systemCategory).isEqualTo(expectedCategory)

        }



    @Test

    fun categories_keyCodeAndModifierHaveSameCode_codesAreMappedCorrectly() =

        testScope.runTest {

            fakeSystemSource.setGroups(simpleGroup(simpleShortcutInfo(KEYCODE_1, META_FUNCTION_ON)))



            helper.toggle(deviceId = 123)

            val categories by collectLastValue(repo.categories)



            val systemCategory = categories?.firstOrNull { it.type == ShortcutCategoryType.System }



            // Keycode 0x8 should be translated to the Key 1 instead of modifier FN

            // which has the same keycode. while modifier mask 0x8 should be translated to FN.

            val expectedCategory =

                ShortcutCategory(

                    type = ShortcutCategoryType.System,

                    simpleSubCategory(simpleShortcut("Fn", "1")),

                )



            assertThat(systemCategory).isEqualTo(expectedCategory)

        }



    @Test

    fun categories_multipleSubscribers_replaysExistingValueToNewSubscribers() =

        testScope.runTest {
@@ -111,24 +155,14 @@ class ShortcutHelperCategoriesRepositoryTest : SysuiTestCase() { 
        testScope.runTest {

            fakeSystemSource.setGroups(

                listOf(

                    simpleGroup(

                        simpleShortcutInfo(KEYCODE_A),

                        simpleShortcutInfo(KEYCODE_B),

                    ),

                    simpleGroup(

                        simpleShortcutInfo(KEYCODE_C),

                    ),

                    simpleGroup(simpleShortcutInfo(KEYCODE_A), simpleShortcutInfo(KEYCODE_B)),

                    simpleGroup(simpleShortcutInfo(KEYCODE_C)),

                )

            )

            fakeMultiTaskingSource.setGroups(

                listOf(

                    simpleGroup(

                        simpleShortcutInfo(KEYCODE_D),

                    ),

                    simpleGroup(

                        simpleShortcutInfo(KEYCODE_E),

                        simpleShortcutInfo(KEYCODE_F),

                    ),

                    simpleGroup(simpleShortcutInfo(KEYCODE_D)),

                    simpleGroup(simpleShortcutInfo(KEYCODE_E), simpleShortcutInfo(KEYCODE_F)),

                )

            )

            fakeAppCategoriesSource.setGroups(listOf(simpleGroup(simpleShortcutInfo(KEYCODE_G))))
@@ -144,16 +178,11 @@ class ShortcutHelperCategoriesRepositoryTest : SysuiTestCase() { 
                        listOf(

                            simpleSubCategory(simpleShortcut("B")),

                            simpleSubCategory(simpleShortcut("C")),

                        )

                        ),

                    ),

                    ShortcutCategory(

                        ShortcutCategoryType.MultiTasking,

                        listOf(

                            simpleSubCategory(

                                simpleShortcut("E"),

                                simpleShortcut("F"),

                            ),

                        )

                        listOf(simpleSubCategory(simpleShortcut("E"), simpleShortcut("F"))),

                    ),

                )

        }
@@ -164,14 +193,14 @@ class ShortcutHelperCategoriesRepositoryTest : SysuiTestCase() { 
    private fun simpleShortcut(vararg keys: String) =

        Shortcut(

            label = simpleShortcutLabel,

            commands = listOf(ShortcutCommand(keys.map { ShortcutKey.Text(it) }))

            commands = listOf(ShortcutCommand(keys.map { ShortcutKey.Text(it) })),

        )



    private fun simpleGroup(vararg shortcuts: KeyboardShortcutInfo) =

        KeyboardShortcutGroup(simpleGroupLabel, shortcuts.asList())



    private fun simpleShortcutInfo(keyCode: Int = 0) =

        KeyboardShortcutInfo(simpleShortcutLabel, keyCode, /* modifiers= */ 0)

    private fun simpleShortcutInfo(keyCode: Int = 0, modifiers: Int = 0) =

        KeyboardShortcutInfo(simpleShortcutLabel, keyCode, modifiers)



    private val simpleShortcutLabel = "shortcut label"

    private val simpleGroupLabel = "group label"

packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt

+27 −13
Original line number Diff line number Diff line
@@ -68,7 +68,7 @@ constructor( 
    @InputShortcuts private val inputShortcutsSource: KeyboardShortcutGroupsSource,

    @CurrentAppShortcuts private val currentAppShortcutsSource: KeyboardShortcutGroupsSource,

    private val inputManager: InputManager,

    stateRepository: ShortcutHelperStateRepository

    stateRepository: ShortcutHelperStateRepository,

) {



    private val sources =
@@ -76,27 +76,27 @@ constructor( 
            InternalGroupsSource(

                source = systemShortcutsSource,

                isTrusted = true,

                typeProvider = { System }

                typeProvider = { System },

            ),

            InternalGroupsSource(

                source = multitaskingShortcutsSource,

                isTrusted = true,

                typeProvider = { MultiTasking }

                typeProvider = { MultiTasking },

            ),

            InternalGroupsSource(

                source = appCategoriesShortcutsSource,

                isTrusted = true,

                typeProvider = { AppCategories }

                typeProvider = { AppCategories },

            ),

            InternalGroupsSource(

                source = inputShortcutsSource,

                isTrusted = false,

                typeProvider = { InputMethodEditor }

                typeProvider = { InputMethodEditor },

            ),

            InternalGroupsSource(

                source = currentAppShortcutsSource,

                isTrusted = false,

                typeProvider = { groups -> getCurrentAppShortcutCategoryType(groups) }

                typeProvider = { groups -> getCurrentAppShortcutCategoryType(groups) },

            ),

        )
@@ -179,7 +179,7 @@ constructor( 
                            shortcutGroup.items,

                            keepIcons,

                            supportedKeyCodes,

                        )

                        ),

                    )

                }

                .filter { it.shortcuts.isNotEmpty() }
@@ -214,13 +214,13 @@ constructor( 
        return Shortcut(

            label = shortcutInfo.label!!.toString(),

            icon = toShortcutIcon(keepIcon, shortcutInfo),

            commands = listOf(shortcutCommand)

            commands = listOf(shortcutCommand),

        )

    }



    private fun toShortcutIcon(

        keepIcon: Boolean,

        shortcutInfo: KeyboardShortcutInfo

        shortcutInfo: KeyboardShortcutInfo,

    ): ShortcutIcon? {

        if (!keepIcon) {

            return null
@@ -236,13 +236,13 @@ constructor( 


    private fun toShortcutCommand(

        keyCharacterMap: KeyCharacterMap,

        info: KeyboardShortcutInfo

        info: KeyboardShortcutInfo,

    ): ShortcutCommand? {

        val keys = mutableListOf<ShortcutKey>()

        var remainingModifiers = info.modifiers

        SUPPORTED_MODIFIERS.forEach { supportedModifier ->

            if ((supportedModifier and remainingModifiers) != 0) {

                keys += toShortcutKey(keyCharacterMap, supportedModifier) ?: return null

                keys += toShortcutModifierKey(supportedModifier) ?: return null

                // "Remove" the modifier from the remaining modifiers

                remainingModifiers = remainingModifiers and supportedModifier.inv()

            }
@@ -262,6 +262,20 @@ constructor( 
        return ShortcutCommand(keys)

    }



    private fun toShortcutModifierKey(modifierMask: Int): ShortcutKey? {

        val iconResId = ShortcutHelperKeys.keyIcons[modifierMask]

        if (iconResId != null) {

            return ShortcutKey.Icon(iconResId)

        }



        val modifierLabel = ShortcutHelperKeys.modifierLabels[modifierMask]

        if (modifierLabel != null) {

            return ShortcutKey.Text(modifierLabel(context))

        }

        Log.wtf("TAG", "Couldn't find label or icon for modifier $modifierMask")

        return null

    }



    private fun toShortcutKey(

        keyCharacterMap: KeyCharacterMap,

        keyCode: Int,
@@ -289,7 +303,7 @@ constructor( 


    private suspend fun fetchSupportedKeyCodes(

        deviceId: Int,

        groupsFromAllSources: List<List<KeyboardShortcutGroup>>

        groupsFromAllSources: List<List<KeyboardShortcutGroup>>,

    ): Set<Int> =

        withContext(backgroundDispatcher) {

            val allUsedKeyCodes =
@@ -320,7 +334,7 @@ constructor( 
                KeyEvent.META_ALT_ON,

                KeyEvent.META_SHIFT_ON,

                KeyEvent.META_SYM_ON,

                KeyEvent.META_FUNCTION_ON

                KeyEvent.META_FUNCTION_ON,

            )

    }

}

packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperKeys.kt

+12 −9
Original line number Diff line number Diff line
@@ -124,6 +124,17 @@ object ShortcutHelperKeys { 
            KEYCODE_RECENT_APPS to R.drawable.ic_check_box_outline_blank,

        )



    val modifierLabels =

        mapOf<Int, (Context) -> String>(

            // Modifiers

            META_META_ON to { "Meta" },

            META_CTRL_ON to { "Ctrl" },

            META_ALT_ON to { "Alt" },

            META_SHIFT_ON to { "Shift" },

            META_SYM_ON to { "Sym" },

            META_FUNCTION_ON to { "Fn" },

        )



    val specialKeyLabels =

        mapOf<Int, (Context) -> String>(

            KEYCODE_HOME to { context -> context.getString(R.string.keyboard_key_home) },
@@ -317,7 +328,7 @@ object ShortcutHelperKeys { 
                { context ->

                    context.getString(

                        R.string.keyboard_key_numpad_template,

                        context.getString(R.string.keyboard_key_enter)

                        context.getString(R.string.keyboard_key_enter),

                    )

                },

            KEYCODE_NUMPAD_EQUALS to
@@ -343,13 +354,5 @@ object ShortcutHelperKeys { 
            KEYCODE_CTRL_RIGHT to { "Ctrl" },

            KEYCODE_SHIFT_LEFT to { "Shift" },

            KEYCODE_SHIFT_RIGHT to { "Shift" },



            // Modifiers

            META_META_ON to { "Meta" },

            META_CTRL_ON to { "Ctrl" },

            META_ALT_ON to { "Alt" },

            META_SHIFT_ON to { "Shift" },

            META_SYM_ON to { "Sym" },

            META_FUNCTION_ON to { "Fn" },

        )

}