From ac62f76348540d444213ec42a4f84b182b20d088 Mon Sep 17 00:00:00 2001 From: sds100 Date: Tue, 18 Feb 2025 10:26:20 -0600 Subject: [PATCH 1/2] #1386 write tests --- .../mappings/keymaps/KeyMapControllerTest.kt | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt b/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt index debed73113..9b28be85b3 100644 --- a/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt +++ b/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt @@ -36,6 +36,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.currentTime import kotlinx.coroutines.test.runTest @@ -181,6 +182,103 @@ class KeyMapControllerTest { ) } + /** + * Issue #1386 + */ + @Test + fun `Wait for sequence trigger timeout before triggering overlapping parallel triggers 1`() = runTest(testDispatcher) { + // GIVEN + val copyTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_J)) + val copyAction = KeyMapAction(data = ActionData.CopyText) + + val pasteTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_K)) + val pasteAction = KeyMapAction(data = ActionData.PasteText) + + val sequenceTrigger = + sequenceTrigger(triggerKey(KeyEvent.KEYCODE_J), triggerKey(KeyEvent.KEYCODE_K)) + val enterAction = KeyMapAction(data = ActionData.InputKeyEvent(KeyEvent.KEYCODE_ENTER)) + + keyMapListFlow.value = listOf( + KeyMap(0, trigger = copyTrigger, actionList = listOf(copyAction)), + KeyMap(1, trigger = pasteTrigger, actionList = listOf(pasteAction)), + KeyMap(2, trigger = sequenceTrigger, actionList = listOf(enterAction)), + ) + + // WHEN + mockTriggerKeyInput(copyTrigger.keys[0]) + advanceTimeBy(SEQUENCE_TRIGGER_TIMEOUT) + + // THEN + verify(performActionsUseCase, times(1)).perform(copyAction.data) + verify(performActionsUseCase, never()).perform(pasteAction.data) + verify(performActionsUseCase, never()).perform(enterAction.data) + } + + /** + * Issue #1386 + */ + @Test + fun `Wait for sequence trigger timeout before triggering overlapping parallel triggers 2`() = runTest(testDispatcher) { + // GIVEN + val copyTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_J)) + val copyAction = KeyMapAction(data = ActionData.CopyText) + + val pasteTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_K)) + val pasteAction = KeyMapAction(data = ActionData.PasteText) + + val sequenceTrigger = + sequenceTrigger(triggerKey(KeyEvent.KEYCODE_J), triggerKey(KeyEvent.KEYCODE_K)) + val enterAction = KeyMapAction(data = ActionData.InputKeyEvent(KeyEvent.KEYCODE_ENTER)) + + keyMapListFlow.value = listOf( + KeyMap(0, trigger = copyTrigger, actionList = listOf(copyAction)), + KeyMap(1, trigger = pasteTrigger, actionList = listOf(pasteAction)), + KeyMap(2, trigger = sequenceTrigger, actionList = listOf(enterAction)), + ) + + // WHEN + mockTriggerKeyInput(pasteTrigger.keys[0]) + advanceTimeBy(SEQUENCE_TRIGGER_TIMEOUT) + + // THEN + verify(performActionsUseCase, never()).perform(copyAction.data) + verify(performActionsUseCase, times(1)).perform(pasteAction.data) + verify(performActionsUseCase, never()).perform(enterAction.data) + } + + /** + * Issue #1386 + */ + @Test + fun `Do not trigger parallel trigger if a sequence trigger contains the same keys`() = runTest(testDispatcher) { + // GIVEN + val copyTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_J)) + val copyAction = KeyMapAction(data = ActionData.CopyText) + + val pasteTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_K)) + val pasteAction = KeyMapAction(data = ActionData.PasteText) + + val sequenceTrigger = + sequenceTrigger(triggerKey(KeyEvent.KEYCODE_J), triggerKey(KeyEvent.KEYCODE_K)) + val enterAction = KeyMapAction(data = ActionData.InputKeyEvent(KeyEvent.KEYCODE_ENTER)) + + keyMapListFlow.value = listOf( + KeyMap(0, trigger = copyTrigger, actionList = listOf(copyAction)), + KeyMap(1, trigger = pasteTrigger, actionList = listOf(pasteAction)), + KeyMap(2, trigger = sequenceTrigger, actionList = listOf(enterAction)), + ) + + // WHEN + mockTriggerKeyInput(sequenceTrigger.keys[0]) + mockTriggerKeyInput(sequenceTrigger.keys[1]) + + // THEN + + verify(performActionsUseCase, never()).perform(copyAction.data) + verify(performActionsUseCase, never()).perform(pasteAction.data) + verify(performActionsUseCase, times(1)).perform(enterAction.data) + } + @Test fun `Hold down key event action while DPAD button is held down via motion events`() = runTest(testDispatcher) { val trigger = singleKeyTrigger( From 71044e43d0ed3c2341b08d41269677628488ce0b Mon Sep 17 00:00:00 2001 From: sds100 Date: Tue, 18 Feb 2025 14:46:17 -0600 Subject: [PATCH 2/2] #1386 fix: wait for sequence trigger timeout before triggering overlapping triggers --- .../keymaps/detection/KeyMapController.kt | 130 ++++++++++++++---- .../mappings/keymaps/KeyMapControllerTest.kt | 103 +++++++++++++- 2 files changed, 197 insertions(+), 36 deletions(-) diff --git a/app/src/free/java/io/github/sds100/keymapper/mappings/keymaps/detection/KeyMapController.kt b/app/src/free/java/io/github/sds100/keymapper/mappings/keymaps/detection/KeyMapController.kt index 7eaab4142d..06abe5b525 100644 --- a/app/src/free/java/io/github/sds100/keymapper/mappings/keymaps/detection/KeyMapController.kt +++ b/app/src/free/java/io/github/sds100/keymapper/mappings/keymaps/detection/KeyMapController.kt @@ -249,7 +249,7 @@ class KeyMapController( MutableList(triggers.size) { mutableSetOf() } for (triggerIndex in parallelTriggers) { - val trigger = triggers[triggerIndex] + val parallelTrigger = triggers[triggerIndex] otherTriggerLoop@ for (otherTriggerIndex in sequenceTriggers) { val otherTrigger = triggers[otherTriggerIndex] @@ -259,7 +259,7 @@ class KeyMapController( continue@otherTriggerLoop } - for ((keyIndex, key) in trigger.keys.withIndex()) { + for ((keyIndex, key) in parallelTrigger.keys.withIndex()) { var lastMatchedIndex: Int? = null for ((otherKeyIndex, otherKey) in otherTrigger.keys.withIndex()) { @@ -271,7 +271,7 @@ class KeyMapController( continue@otherTriggerLoop } - if (keyIndex == trigger.keys.lastIndex) { + if (keyIndex == parallelTrigger.keys.lastIndex) { sequenceTriggersOverlappingParallelTriggers[triggerIndex].add( otherTriggerIndex, ) @@ -489,30 +489,36 @@ class KeyMapController( private var modifierKeyEventActions: Boolean = false private var notModifierKeyEventActions: Boolean = false - private var keyCodesToImitateUpAction: MutableSet = mutableSetOf() + private var keyCodesToImitateUpAction: MutableSet = mutableSetOf() private var metaStateFromActions: Int = 0 private var metaStateFromKeyEvent: Int = 0 - private val eventDownTimeMap: MutableMap = mutableMapOf() + private val eventDownTimeMap: MutableMap = mutableMapOf() + + /** + * This solves issue #1386. This stores the jobs that will wait until the sequence trigger + * times out and check whether the overlapping sequence trigger was indeed triggered. + */ + private val performActionsAfterSequenceTriggerTimeout: MutableMap = mutableMapOf() /** * The indexes of parallel triggers that didn't have their actions performed because there is a matching trigger but * for a long-press. These actions should only be performed if the long-press fails, otherwise when the user * holds down the trigger keys for the long-press trigger, actions from both triggers will be performed. */ - private val performActionsOnFailedLongPress: MutableSet = mutableSetOf() + private val performActionsOnFailedLongPress: MutableSet = mutableSetOf() /** * The indexes of parallel triggers that didn't have their actions performed because there is a matching trigger but * for a double-press. These actions should only be performed if the double-press fails, otherwise each time the user * presses the keys for the double press, actions from both triggers will be performed. */ - private val performActionsOnFailedDoublePress: MutableSet = mutableSetOf() + private val performActionsOnFailedDoublePress: MutableSet = mutableSetOf() /** * Maps jobs to perform an action after a long press to their corresponding parallel trigger index */ - private val parallelTriggerLongPressJobs: SparseArrayCompat = SparseArrayCompat() + private val parallelTriggerLongPressJobs: SparseArrayCompat = SparseArrayCompat() /** * Keys that are detected through an input method will potentially send multiple DOWN key events @@ -849,31 +855,63 @@ class KeyMapController( mappedToParallelTriggerAction = true parallelTriggersAwaitingReleaseAfterBeingTriggered[triggerIndex] = true - val actionKeys = triggerActions[triggerIndex] + // See issue #1386. + val overlappingSequenceTrigger = + sequenceTriggersOverlappingParallelTriggers[triggerIndex] + // Only consider the sequence triggers where this + // short press trigger has been already pressed + // or will be pressed next. + .filter { + for (i in 0..(lastMatchedEventIndices[it] + 1)) { + val matchingEvent = triggers[it].matchingEventAtIndex( + event.withShortPress, + i, + ) + + if (matchingEvent) { + return@filter true + } + } + + return@filter false + } + .maxByOrNull { sequenceTriggerTimeout(triggers[it]) } + + if (overlappingSequenceTrigger == null) { + val actionKeys = triggerActions[triggerIndex] - actionKeys.forEach { actionKey -> - val action = actionMap[actionKey] ?: return@forEach + actionKeys.forEach { actionKey -> + val action = actionMap[actionKey] ?: return@forEach - if (action.data is ActionData.InputKeyEvent) { - val actionKeyCode = action.data.keyCode + if (action.data is ActionData.InputKeyEvent) { + val actionKeyCode = action.data.keyCode - if (isModifierKey(actionKeyCode)) { - val actionMetaState = - InputEventUtils.modifierKeycodeToMetaState(actionKeyCode) - metaStateFromActions = - metaStateFromActions.withFlag(actionMetaState) + if (isModifierKey(actionKeyCode)) { + val actionMetaState = + InputEventUtils.modifierKeycodeToMetaState(actionKeyCode) + metaStateFromActions = + metaStateFromActions.withFlag(actionMetaState) + } } - } - detectedShortPressTriggers.add(triggerIndex) + detectedShortPressTriggers.add(triggerIndex) - val vibrateDuration = when { - trigger.vibrate -> vibrateDuration(trigger) - forceVibrate.value -> defaultVibrateDuration.value - else -> -1L + val vibrateDuration = when { + trigger.vibrate -> vibrateDuration(trigger) + forceVibrate.value -> defaultVibrateDuration.value + else -> -1L + } + + vibrateDurations.add(vibrateDuration) } + } else { + performActionsAfterSequenceTriggerTimeout[triggerIndex]?.cancel() - vibrateDurations.add(vibrateDuration) + performActionsAfterSequenceTriggerTimeout[triggerIndex] = + performActionsAfterSequenceTriggerTimeout( + triggerIndex, + overlappingSequenceTrigger, + ) } } } @@ -1443,14 +1481,14 @@ class KeyMapController( metaStateFromKeyEvent = 0 keyCodesToImitateUpAction = mutableSetOf() - parallelTriggerLongPressJobs.valueIterator().forEach { - it.cancel() - } - + parallelTriggerLongPressJobs.valueIterator().forEach { it.cancel() } parallelTriggerLongPressJobs.clear() parallelTriggerActionPerformers.values.forEach { it.reset() } sequenceTriggerActionPerformers.values.forEach { it.reset() } + + performActionsAfterSequenceTriggerTimeout.forEach { (_, job) -> job.cancel() } + performActionsAfterSequenceTriggerTimeout.clear() } /** @@ -1499,6 +1537,40 @@ class KeyMapController( return detectedTriggerIndexes.isNotEmpty() } + /** + * For parallel triggers only. + */ + private fun performActionsAfterSequenceTriggerTimeout( + triggerIndex: Int, + sequenceTriggerIndex: Int, + ) = coroutineScope.launch { + val timeout = sequenceTriggerTimeout(triggers[sequenceTriggerIndex]) + + delay(timeout) + + // If it equals -1 then it means the sequence trigger was triggered + // and it reset the counter. + if (lastMatchedEventIndices[sequenceTriggerIndex] == -1) { + return@launch + } + + parallelTriggerActionPerformers[triggerIndex]?.onTriggered( + calledOnTriggerRelease = true, + metaState = metaStateFromActions.withFlag(metaStateFromKeyEvent), + ) + + if (triggers[triggerIndex].vibrate || + forceVibrate.value || + triggers[triggerIndex].longPressDoubleVibration + ) { + useCase.vibrate(vibrateDuration(triggers[triggerIndex])) + } + + if (triggers[triggerIndex].showToast) { + useCase.showTriggeredToast() + } + } + private fun encodeActionList(actions: List): IntArray = actions.map { getActionKey(it) }.toIntArray() /** diff --git a/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt b/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt index 9b28be85b3..4a2a288d73 100644 --- a/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt +++ b/app/src/test/java/io/github/sds100/keymapper/mappings/keymaps/KeyMapControllerTest.kt @@ -182,6 +182,95 @@ class KeyMapControllerTest { ) } + /** + * Issue #1386 + */ + @Test + fun `Do not trigger both triggers if sequence trigger is triggered while waiting`() = runTest(testDispatcher) { + // GIVEN + val copyTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_J)) + val copyAction = KeyMapAction(data = ActionData.CopyText) + + val sequenceTrigger = + sequenceTrigger(triggerKey(KeyEvent.KEYCODE_J), triggerKey(KeyEvent.KEYCODE_K)) + val enterAction = KeyMapAction(data = ActionData.InputKeyEvent(KeyEvent.KEYCODE_ENTER)) + + keyMapListFlow.value = listOf( + KeyMap(0, trigger = copyTrigger, actionList = listOf(copyAction)), + KeyMap(1, trigger = sequenceTrigger, actionList = listOf(enterAction)), + ) + + // WHEN + mockTriggerKeyInput(copyTrigger.keys[0]) + mockTriggerKeyInput(sequenceTrigger.keys[0]) + mockTriggerKeyInput(sequenceTrigger.keys[1]) + advanceTimeBy(SEQUENCE_TRIGGER_TIMEOUT) + + // THEN + verify(performActionsUseCase, never()).perform(copyAction.data) + verify(performActionsUseCase, times(1)).perform(enterAction.data) + } + + /** + * Issue #1386 + */ + @Test + fun `Wait for longest sequence trigger timeout before triggering overlapping parallel triggers if multiple sequence triggers`() = runTest(testDispatcher) { + // GIVEN + val copyTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_J)) + val copyAction = KeyMapAction(data = ActionData.CopyText) + + val sequenceTrigger1 = + Trigger( + keys = listOf( + triggerKey(KeyEvent.KEYCODE_J), + triggerKey(KeyEvent.KEYCODE_K), + ), + mode = TriggerMode.Sequence, + sequenceTriggerTimeout = 500, + ) + val sequenceTriggerAction1 = + KeyMapAction(data = ActionData.InputKeyEvent(KeyEvent.KEYCODE_ENTER)) + + // This has a different second key. + val sequenceTrigger2 = + Trigger( + keys = listOf( + triggerKey(KeyEvent.KEYCODE_J), + triggerKey(KeyEvent.KEYCODE_A), + ), + mode = TriggerMode.Sequence, + sequenceTriggerTimeout = 1000, + ) + val sequenceTriggerAction2 = + KeyMapAction(data = ActionData.InputKeyEvent(KeyEvent.KEYCODE_ENTER)) + + keyMapListFlow.value = listOf( + KeyMap(0, trigger = copyTrigger, actionList = listOf(copyAction)), + KeyMap(1, trigger = sequenceTrigger1, actionList = listOf(sequenceTriggerAction1)), + KeyMap(2, trigger = sequenceTrigger2, actionList = listOf(sequenceTriggerAction2)), + ) + + // WHEN + mockTriggerKeyInput(copyTrigger.keys[0]) + + // THEN + inOrder(performActionsUseCase) { + // The single key trigger should not be executed straight away. Wait for + // the longer sequence trigger delay. + verify(performActionsUseCase, never()).perform(copyAction.data) + + // It still shouldn't be executed after the first sequence trigger delay. + advanceTimeBy(500) + verify(performActionsUseCase, never()).perform(copyAction.data) + + advanceTimeBy(1000) + verify(performActionsUseCase, times(1)).perform(copyAction.data) + verify(performActionsUseCase, never()).perform(sequenceTriggerAction1.data) + verify(performActionsUseCase, never()).perform(sequenceTriggerAction2.data) + } + } + /** * Issue #1386 */ @@ -218,13 +307,14 @@ class KeyMapControllerTest { * Issue #1386 */ @Test - fun `Wait for sequence trigger timeout before triggering overlapping parallel triggers 2`() = runTest(testDispatcher) { + fun `Immediately trigger a key that is the 2nd key in a sequence trigger`() = + runTest(testDispatcher) { // GIVEN val copyTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_J)) val copyAction = KeyMapAction(data = ActionData.CopyText) val pasteTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_K)) - val pasteAction = KeyMapAction(data = ActionData.PasteText) + val pasteAction = KeyMapAction(data = ActionData.PasteText) val sequenceTrigger = sequenceTrigger(triggerKey(KeyEvent.KEYCODE_J), triggerKey(KeyEvent.KEYCODE_K)) @@ -238,7 +328,6 @@ class KeyMapControllerTest { // WHEN mockTriggerKeyInput(pasteTrigger.keys[0]) - advanceTimeBy(SEQUENCE_TRIGGER_TIMEOUT) // THEN verify(performActionsUseCase, never()).perform(copyAction.data) @@ -250,13 +339,14 @@ class KeyMapControllerTest { * Issue #1386 */ @Test - fun `Do not trigger parallel trigger if a sequence trigger contains the same keys`() = runTest(testDispatcher) { + fun `Do not trigger parallel trigger if a sequence trigger with the same keys is triggered`() = + runTest(testDispatcher) { // GIVEN val copyTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_J)) val copyAction = KeyMapAction(data = ActionData.CopyText) - val pasteTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_K)) - val pasteAction = KeyMapAction(data = ActionData.PasteText) + val pasteTrigger = singleKeyTrigger(triggerKey(KeyEvent.KEYCODE_K)) + val pasteAction = KeyMapAction(data = ActionData.PasteText) val sequenceTrigger = sequenceTrigger(triggerKey(KeyEvent.KEYCODE_J), triggerKey(KeyEvent.KEYCODE_K)) @@ -273,7 +363,6 @@ class KeyMapControllerTest { mockTriggerKeyInput(sequenceTrigger.keys[1]) // THEN - verify(performActionsUseCase, never()).perform(copyAction.data) verify(performActionsUseCase, never()).perform(pasteAction.data) verify(performActionsUseCase, times(1)).perform(enterAction.data)