Fixing release events when keys are released in order of pressing

inputremapper/injection/mapping_handlers/combination_handler.py

@@ -18,7 +18,7 @@
 # along with input-remapper.  If not, see <https://www.gnu.org/licenses/>.
 
 from __future__ import annotations  # needed for the TYPE_CHECKING import
-from typing import TYPE_CHECKING, Dict, Hashable
+from typing import TYPE_CHECKING, Dict, Hashable, Tuple
 
 import evdev
 from evdev.ecodes import EV_ABS, EV_REL
@@ -45,6 +45,7 @@ class CombinationHandler(MappingHandler):
     _output_state: bool  # the last update we sent to a sub-handler
     _sub_handler: InputEventHandler
     _handled_input_hashes: list[Hashable]
+    _notify_results: Dict[Tuple[int, int], bool]
 
     def __init__(
         self,
@@ -58,6 +59,7 @@ def __init__(
         self._pressed_keys = {}
         self._output_state = False
         self._context = context
+        self._notify_results = {}
 
         # prepare a key map for all events with non-zero value
         for input_config in combination:
@@ -93,6 +95,27 @@ def notify(
         event: InputEvent,
         source: evdev.InputDevice,
         suppress: bool = False,
+    ) -> bool:
+        result = self._notify(event, source, suppress)
+
+        if event.type_and_code in self._notify_results:
+            # The return value is always the same as for the key-down event.
+            # If a key-up event arrives that will inactivate the combination, but
+            # for which previously a key-down event was injected (because it was
+            # an earlier key in the combination chain), then we need to ensure that its
+            # release is injected as well.
+            result = self._notify_results[event.type_and_code]
+            del self._notify_results[event.type_and_code]
+            return result
+
+        self._notify_results[event.type_and_code] = result
+        return result
+
+    def _notify(
+        self,
+        event: InputEvent,
+        source: evdev.InputDevice,
+        suppress: bool = False,
     ) -> bool:
         if event.input_match_hash not in self._handled_input_hashes:
             # we are not responsible for the event

inputremapper/injection/mapping_handlers/key_handler.py

@@ -68,7 +68,6 @@ def child(self):  # used for logging
 
     def notify(self, event: InputEvent, *_, **__) -> bool:
         """Inject event.value to the target key."""
-
         event_tuple = (*self._maps_to, event.value)
         try:
             global_uinputs.write(event_tuple, self.mapping.target_uinput)

tests/unit/test_event_pipeline/test_event_pipeline.py

@@ -17,6 +17,9 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with input-remapper.  If not, see <https://www.gnu.org/licenses/>.
+
+from tests import test
+
 import asyncio
 import unittest
 from typing import Iterable
@@ -108,7 +111,7 @@ def create_event_reader(
         return reader
 
 
-class TestIdk(EventPipelineTestBase):
+class TestCombination(EventPipelineTestBase):
     async def test_any_event_as_button(self):
         """As long as there is an event handler and a mapping we should be able
         to map anything to a button"""
@@ -416,6 +419,10 @@ async def test_combination(self):
             output_symbol="c",
         )
 
+        assert mapping_1.release_combination_keys
+        assert mapping_2.release_combination_keys
+        assert mapping_3.release_combination_keys
+
         preset = Preset()
         preset.add(mapping_1)
         preset.add(mapping_2)
@@ -440,6 +447,8 @@ async def test_combination(self):
 
         forwarded_history = self.forward_uinput.write_history
 
+        # I don't remember the specifics. I guess if there is a combination ongoing,
+        # it shouldn't trigger ABS_X -> a?
         self.assertNotIn((EV_KEY, a, 1), keyboard_history)
 
         # c and b should have been written, because the input from send_events
@@ -465,6 +474,70 @@ async def test_combination(self):
         self.assertEqual(keyboard_history.count((EV_KEY, c, 0)), 1)
         self.assertEqual(keyboard_history.count((EV_KEY, b, 0)), 1)
 
+    async def test_combination_manual_release_in_press_order(self):
+        """Test if combinations map to keys properly."""
+        # release_combination_keys is off
+        # press 5, then 6, then release 5, then release 6
+        in_1 = system_mapping.get("7")
+        in_2 = system_mapping.get("8")
+        out = system_mapping.get("a")
+
+        origin = fixtures.foo_device_2_keyboard
+        origin_hash = origin.get_device_hash()
+
+        input_combination = InputCombination(
+            [
+                InputConfig(
+                    type=EV_KEY,
+                    code=in_1,
+                    origin_hash=origin_hash,
+                ),
+                InputConfig(
+                    type=EV_KEY,
+                    code=in_2,
+                    origin_hash=origin_hash,
+                ),
+            ]
+        )
+
+        mapping = Mapping(
+            input_combination=input_combination.to_config(),
+            target_uinput="keyboard",
+            output_symbol="a",
+            release_combination_keys=False,
+        )
+
+        assert not mapping.release_combination_keys
+
+        preset = Preset()
+        preset.add(mapping)
+
+        event_reader = self.create_event_reader(preset, origin)
+
+        keyboard_history = global_uinputs.get_uinput("keyboard").write_history
+        forwarded_history = self.forward_uinput.write_history
+
+        # press the first key of the combination
+        await self.send_events([InputEvent.key(in_1, 1, origin_hash)], event_reader)
+        self.assertListEqual(forwarded_history, [(EV_KEY, in_1, 1)])
+
+        # then the second, it should trigger the combination
+        await self.send_events([InputEvent.key(in_2, 1, origin_hash)], event_reader)
+        self.assertListEqual(forwarded_history, [(EV_KEY, in_1, 1)])
+        self.assertListEqual(keyboard_history, [(EV_KEY, out, 1)])
+
+        # release the first key. A key-down event was injected for it previously, so
+        # now we find a key-up event here as well.
+        await self.send_events([InputEvent.key(in_1, 0, origin_hash)], event_reader)
+        self.assertListEqual(forwarded_history, [(EV_KEY, in_1, 1), (EV_KEY, in_1, 0)])
+        self.assertListEqual(keyboard_history, [(EV_KEY, out, 1), (EV_KEY, out, 0)])
+
+        # release the second key. No key-down event was injected, so we don't have a
+        # key-up event here either.
+        await self.send_events([InputEvent.key(in_2, 0, origin_hash)], event_reader)
+        self.assertListEqual(forwarded_history, [(EV_KEY, in_1, 1), (EV_KEY, in_1, 0)])
+        self.assertListEqual(keyboard_history, [(EV_KEY, out, 1), (EV_KEY, out, 0)])
+
     async def test_ignore_hold(self):
         # hold as in event-value 2, not in macro-hold.
         # linux will generate events with value 2 after input-remapper injected

tests/unit/test_event_pipeline/test_mapping_handlers.py

@@ -21,6 +21,7 @@
 
 """See TestEventPipeline for more tests."""
 
+from tests import test
 
 import asyncio
 import unittest
@@ -59,13 +60,16 @@
 from inputremapper.injection.mapping_handlers.hierarchy_handler import HierarchyHandler
 from inputremapper.injection.mapping_handlers.key_handler import KeyHandler
 from inputremapper.injection.mapping_handlers.macro_handler import MacroHandler
-from inputremapper.injection.mapping_handlers.mapping_handler import MappingHandler
+from inputremapper.injection.mapping_handlers.mapping_handler import (
+    MappingHandler,
+    InputEventHandler,
+)
 from inputremapper.injection.mapping_handlers.rel_to_abs_handler import RelToAbsHandler
 from inputremapper.input_event import InputEvent, EventActions
 
 from tests.lib.cleanup import cleanup
 from tests.lib.logger import logger
-from tests.lib.patches import InputDevice
+from tests.lib.patches import InputDevice, UInput
 from tests.lib.constants import MAX_ABS
 from tests.lib.fixtures import fixtures
 
@@ -286,26 +290,29 @@ def setUp(self):
                 input_combination=input_combination.to_config(),
                 target_uinput="mouse",
                 output_symbol="BTN_LEFT",
+                release_combination_keys=True,
             ),
             self.context_mock,
         )
 
+        sub_handler_mock = MagicMock(InputEventHandler)
+        self.handler.set_sub_handler(sub_handler_mock)
+
+        # insert our own test-uinput to see what is being written to it
+        self.uinputs = {
+            self.mouse_hash: UInput(),
+            self.keyboard_hash: UInput(),
+            self.gamepad_hash: UInput(),
+        }
+        self.context_mock.get_forward_uinput = lambda origin_hash: self.uinputs[
+            origin_hash
+        ]
+
     def test_forward_correctly(self):
         # In the past, if a mapping has inputs from two different sub devices, it
         # always failed to send the release events to the correct one.
         # Nowadays, self._context.get_forward_uinput(origin_hash) is used to
         # release them correctly.
-        mock = MagicMock()
-        self.handler.set_sub_handler(mock)
-
-        # insert our own test-uinput to see what is being written to it
-        uinputs = {
-            self.mouse_hash: evdev.UInput(),
-            self.keyboard_hash: evdev.UInput(),
-            self.gamepad_hash: evdev.UInput(),
-        }
-        self.context_mock.get_forward_uinput = lambda origin_hash: uinputs[origin_hash]
-
         # 1. trigger the combination
         self.handler.notify(
             InputEvent.rel(
@@ -335,30 +342,20 @@ def test_forward_correctly(self):
         # 2. expect release events to be written to the correct devices, as indicated
         # by the origin_hash of the InputConfigs
         self.assertListEqual(
-            uinputs[self.mouse_hash].write_history,
+            self.uinputs[self.mouse_hash].write_history,
             [InputEvent.rel(self.input_combination[0].code, 0)],
         )
         self.assertListEqual(
-            uinputs[self.keyboard_hash].write_history,
+            self.uinputs[self.keyboard_hash].write_history,
             [InputEvent.key(self.input_combination[1].code, 0)],
         )
         self.assertListEqual(
-            uinputs[self.gamepad_hash].write_history,
+            self.uinputs[self.gamepad_hash].write_history,
             [InputEvent.key(self.input_combination[2].code, 0)],
         )
 
     def test_no_forwards(self):
         # if a combination is not triggered, nothing is released
-        mock = MagicMock()
-        self.handler.set_sub_handler(mock)
-
-        # insert our own test-uinput to see what is being written to it
-        uinputs = {
-            self.mouse_hash: evdev.UInput(),
-            self.keyboard_hash: evdev.UInput(),
-        }
-        self.context_mock.get_forward_uinput = lambda origin_hash: uinputs[origin_hash]
-
         # 1. inject any two events
         self.handler.notify(
             InputEvent.rel(
@@ -378,8 +375,8 @@ def test_no_forwards(self):
         )
 
         # 2. expect no release events to be written
-        self.assertListEqual(uinputs[self.mouse_hash].write_history, [])
-        self.assertListEqual(uinputs[self.keyboard_hash].write_history, [])
+        self.assertListEqual(self.uinputs[self.mouse_hash].write_history, [])
+        self.assertListEqual(self.uinputs[self.keyboard_hash].write_history, [])
 
 
 class TestHierarchyHandler(BaseTests, unittest.IsolatedAsyncioTestCase):