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

3 files changed, 92 insertions, 46 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt
index 6e883c24b5d2..9e20e7d0f98c 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt
@@ -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
@@ -87,6 +89,48 @@ class ShortcutHelperCategoriesRepositoryTest : SysuiTestCase() {
     }
 
     @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 {
             fakeSystemSource.setGroups(TestShortcuts.systemGroups)
@@ -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"
diff --git a/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt b/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt
index 85bd0b0b1968..a08588750f85 100644
--- a/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt
@@ -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,
             )
     }
 }
diff --git a/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperKeys.kt b/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperKeys.kt
index 8db16fa9a06e..288efa275219 100644
--- a/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperKeys.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperKeys.kt
@@ -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" },
         )
 }