diff --git a/keymapper/gtk/row.py b/keymapper/gtk/row.py index c40b7e17..d6fa8437 100644 --- a/keymapper/gtk/row.py +++ b/keymapper/gtk/row.py @@ -91,6 +91,7 @@ class Row(Gtk.ListBoxRow): # the newest_keycode is populated since the ui regularly polls it # in order to display it in the status bar. key = self.get_keycode() + previous_type = key[0] if key else None previous_keycode = key[1] if key else None character = self.get_character() @@ -104,8 +105,10 @@ class Row(Gtk.ListBoxRow): return # keycode is already set by some other row - if custom_mapping.get_character(ev_type, new_keycode) is not None: - msg = f'Keycode {new_keycode} is already mapped' + existing = custom_mapping.get_character(ev_type, new_keycode) + if existing is not None: + name = to_string(ev_type, new_keycode) + msg = f'"{name}" already mapped to {existing}"' logger.info(msg) self.window.get('status_bar').push(CTX_KEYCODE, msg) return @@ -129,10 +132,9 @@ class Row(Gtk.ListBoxRow): # else, the keycode has changed, the character is set, all good custom_mapping.change( - ev_type=ev_type, - new_keycode=new_keycode, + new=(ev_type, new_keycode), character=character, - previous_keycode=previous_keycode + previous=(previous_type, previous_keycode) ) def highlight(self): @@ -153,10 +155,9 @@ class Row(Gtk.ListBoxRow): if key is not None: ev_type, keycode = key custom_mapping.change( - ev_type=ev_type, - new_keycode=keycode, + new=(ev_type, keycode), character=character, - previous_keycode=None + previous=(None, None) ) def put_together(self, character): diff --git a/keymapper/gtk/window.py b/keymapper/gtk/window.py index dbfd866f..7ff13b19 100755 --- a/keymapper/gtk/window.py +++ b/keymapper/gtk/window.py @@ -140,11 +140,13 @@ class Window: """Ensure that one empty row is available at all times.""" num_rows = len(self.get('key_list').get_children()) - # verify that all mappings are displayed - if num_rows < len(custom_mapping): + # verify that all mappings are displayed. One of them + # is possible the empty row + num_maps = len(custom_mapping) + if num_rows < num_maps or num_rows > num_maps + 1: raise AssertionError( f'custom_mapping contains {len(custom_mapping)} rows, ' - f'but only {num_rows} are displayed' + f'but {num_rows} are displayed' ) if num_rows == len(custom_mapping): diff --git a/keymapper/mapping.py b/keymapper/mapping.py index b0ae81ae..40050545 100644 --- a/keymapper/mapping.py +++ b/keymapper/mapping.py @@ -25,7 +25,6 @@ import os import json import copy -import evdev from keymapper.logger import logger from keymapper.paths import get_config_path, touch @@ -51,41 +50,60 @@ class Mapping: def __len__(self): return len(self._mapping) - def change(self, ev_type, new_keycode, character, previous_keycode=None): + def change(self, new, character, previous=(None, None)): """Replace the mapping of a keycode with a different one. Return True on success. Parameters ---------- - ev_type : int - one of evdev.events, taken from the original source event. - Everything will be mapped to EV_KEY. - new_keycode : int - The source keycode, what the mouse would report without any + new : int, int + 0: type, one of evdev.events, taken from the original source + event. Everything will be mapped to EV_KEY. + 1: The source keycode, what the mouse would report without any modification. character : string or string[] A single character known to xkb or linux. Examples: KP_1, Shift_L, a, B, BTN_LEFT. - previous_keycode : int or None + previous : int, int If None, will not remove any previous mapping. If you recently used 10 for new_keycode and want to overwrite that with 11, provide 5 here. """ + new_type, new_keycode = new + prev_type, prev_keycode = previous + + # either both of them are None, or both integers + if prev_keycode is None and prev_keycode != prev_type: + logger.error('Got (%s, %s) for previous', prev_type, prev_keycode) + if new_keycode is None and prev_keycode != new_type: + logger.error('Got (%s, %s) for new', new_type, new_keycode) + try: new_keycode = int(new_keycode) - if previous_keycode is not None: - previous_keycode = int(previous_keycode) + new_type = int(new_type) + if prev_keycode is not None: + prev_keycode = int(prev_keycode) + if prev_type is not None: + prev_type = int(prev_type) except ValueError: - logger.error('Can only use numbers as keycodes') + logger.error('Can only use numbers in the tuples') return False if new_keycode and character: - self._mapping[(ev_type, new_keycode)] = character - if new_keycode != previous_keycode: + logger.debug( + 'type:%s, code:%s will map to %s, replacing type:%s, code:%s', + new_type, new_keycode, character, prev_type, prev_keycode + ) + self._mapping[(new_type, new_keycode)] = character + code_changed = new_keycode != prev_keycode + if code_changed and prev_keycode is not None: # clear previous mapping of that code, because the line # representing that one will now represent a different one. - self.clear(ev_type, previous_keycode) + # TODO test that None won't reach the clear function. + # TODO test that overwriting an entry with a different type + # works + self.clear(prev_type, prev_keycode) self.changed = True return True @@ -100,7 +118,16 @@ class Mapping: ev_type : int one of evdev.events """ + assert keycode is not None + assert ev_type is not None + assert isinstance(ev_type, int) + assert isinstance(keycode, int) + if self._mapping.get((ev_type, keycode)) is not None: + logger.debug( + 'type:%s, code:%s will be cleared', + ev_type, keycode + ) del self._mapping[(ev_type, keycode)] self.changed = True diff --git a/tests/testcases/test_daemon.py b/tests/testcases/test_daemon.py index c5a74909..7ff6b9d0 100644 --- a/tests/testcases/test_daemon.py +++ b/tests/testcases/test_daemon.py @@ -85,8 +85,8 @@ class TestDaemon(unittest.TestCase): keycode_from_2 = 12 keycode_to_2 = 100 - custom_mapping.change(EV_KEY, keycode_from_1, 'a') - custom_mapping.change(EV_KEY, keycode_from_2, 'b') + custom_mapping.change((EV_KEY, keycode_from_1), 'a') + custom_mapping.change((EV_KEY, keycode_from_2), 'b') clear_system_mapping() system_mapping['a'] = keycode_to_1 system_mapping['b'] = keycode_to_2 diff --git a/tests/testcases/test_injector.py b/tests/testcases/test_injector.py index 66950ce9..515cf58d 100644 --- a/tests/testcases/test_injector.py +++ b/tests/testcases/test_injector.py @@ -76,10 +76,7 @@ class TestInjector(unittest.TestCase): } mapping = Mapping() - mapping.change(EV_KEY, - new_keycode=80, - character='a' - ) + mapping.change((EV_KEY, 80), 'a') maps_to = system_mapping['a'] @@ -99,7 +96,7 @@ class TestInjector(unittest.TestCase): def test_grab(self): # path is from the fixtures - custom_mapping.change(EV_KEY, 10, 'a') + custom_mapping.change((EV_KEY, 10), 'a') self.injector = KeycodeInjector('device 1', custom_mapping) path = '/dev/input/event10' @@ -124,7 +121,7 @@ class TestInjector(unittest.TestCase): def test_skip_unused_device(self): # skips a device because its capabilities are not used in the mapping - custom_mapping.change(EV_KEY, 10, 'a') + custom_mapping.change((EV_KEY, 10), 'a') self.injector = KeycodeInjector('device 1', custom_mapping) path = '/dev/input/event11' device, abs_to_rel = self.injector._prepare_device(path) @@ -284,11 +281,11 @@ class TestInjector(unittest.TestCase): self.assertIn(('b', 0), history) def test_injector(self): - custom_mapping.change(EV_KEY, 8, 'k(KEY_Q).k(w)') - custom_mapping.change(EV_KEY, 9, 'a') + custom_mapping.change((EV_KEY, 8), 'k(KEY_Q).k(w)') + custom_mapping.change((EV_KEY, 9), 'a') # one mapping that is unknown in the system_mapping on purpose input_b = 10 - custom_mapping.change(EV_KEY, input_b, 'b') + custom_mapping.change((EV_KEY, input_b), 'b') clear_system_mapping() code_a = 100 diff --git a/tests/testcases/test_integration.py b/tests/testcases/test_integration.py index baedc5b8..196d880d 100644 --- a/tests/testcases/test_integration.py +++ b/tests/testcases/test_integration.py @@ -157,9 +157,9 @@ class TestIntegration(unittest.TestCase): def test_select_device(self): # creates a new empty preset when no preset exists for the device self.window.on_select_device(FakeDropdown('device 1')) - custom_mapping.change(EV_KEY, 50, 'q') - custom_mapping.change(EV_KEY, 51, 'u') - custom_mapping.change(EV_KEY, 52, 'x') + custom_mapping.change((EV_KEY, 50), 'q') + custom_mapping.change((EV_KEY, 51), 'u') + custom_mapping.change((EV_KEY, 52), 'x') self.assertEqual(len(custom_mapping), 3) self.window.on_select_device(FakeDropdown('device 2')) self.assertEqual(len(custom_mapping), 0) @@ -345,12 +345,12 @@ class TestIntegration(unittest.TestCase): remove(row_3, None, 'c', 1) def test_rename_and_save(self): - custom_mapping.change(EV_KEY, 14, 'a', None) + custom_mapping.change((EV_KEY, 14), 'a', (None, None)) self.assertEqual(self.window.selected_preset, 'new preset') self.window.on_save_preset_clicked(None) self.assertEqual(custom_mapping.get_character(EV_KEY, 14), 'a') - custom_mapping.change(EV_KEY, 14, 'b', None) + custom_mapping.change((EV_KEY, 14), 'b', (None, None)) self.window.get('preset_name_input').set_text('asdf') self.window.on_save_preset_clicked(None) self.assertEqual(self.window.selected_preset, 'asdf') @@ -385,7 +385,7 @@ class TestIntegration(unittest.TestCase): gtk_iteration() self.assertEqual(self.window.selected_preset, 'new preset') self.assertFalse(os.path.exists(f'{CONFIG}/device 1/abc 123.json')) - custom_mapping.change(EV_KEY, 10, '1', None) + custom_mapping.change((EV_KEY, 10), '1', (None, None)) self.window.on_save_preset_clicked(None) gtk_iteration() self.assertEqual(self.window.selected_preset, 'abc 123') diff --git a/tests/testcases/test_mapping.py b/tests/testcases/test_mapping.py index 4c5a55a5..4245aebd 100644 --- a/tests/testcases/test_mapping.py +++ b/tests/testcases/test_mapping.py @@ -41,9 +41,9 @@ class TestMapping(unittest.TestCase): def test_clone(self): mapping1 = Mapping() - mapping1.change(EV_KEY, 1, 'a') + mapping1.change((EV_KEY, 1), 'a') mapping2 = mapping1.clone() - mapping1.change(EV_KEY, 2, 'b') + mapping1.change((EV_KEY, 2), 'b') self.assertEqual(mapping1.get_character(EV_KEY, 1), 'a') self.assertEqual(mapping1.get_character(EV_KEY, 2), 'b') @@ -52,9 +52,9 @@ class TestMapping(unittest.TestCase): self.assertIsNone(mapping2.get_character(EV_KEY, 2)) def test_save_load(self): - self.mapping.change(EV_KEY, 10, '1') - self.mapping.change(EV_KEY, 11, '2') - self.mapping.change(EV_KEY, 12, '3') + self.mapping.change((EV_KEY, 10), '1') + self.mapping.change((EV_KEY, 11), '2') + self.mapping.change((EV_KEY, 12), '3') self.mapping.config['foo'] = 'bar' self.mapping.save('device 1', 'test') @@ -70,41 +70,42 @@ class TestMapping(unittest.TestCase): def test_change(self): # 1 is not assigned yet, ignore it - self.mapping.change(EV_KEY, 2, 'a', 1) + self.mapping.change((EV_KEY, 2), 'a', (EV_KEY, 1)) self.assertTrue(self.mapping.changed) self.assertIsNone(self.mapping.get_character(EV_KEY, 1)) self.assertEqual(self.mapping.get_character(EV_KEY, 2), 'a') self.assertEqual(len(self.mapping), 1) # change 2 to 3 and change a to b - self.mapping.change(EV_KEY, 3, 'b', 2) + self.mapping.change((EV_KEY, 3), 'b', (EV_KEY, 2)) self.assertIsNone(self.mapping.get_character(EV_KEY, 2)) self.assertEqual(self.mapping.get_character(EV_KEY, 3), 'b') self.assertEqual(len(self.mapping), 1) # add 4 - self.mapping.change(EV_KEY, 4, 'c', None) + self.mapping.change((EV_KEY, 4), 'c', (None, None)) self.assertEqual(self.mapping.get_character(EV_KEY, 3), 'b') self.assertEqual(self.mapping.get_character(EV_KEY, 4), 'c') self.assertEqual(len(self.mapping), 2) # change the mapping of 4 to d - self.mapping.change(EV_KEY, 4, 'd', None) + self.mapping.change((EV_KEY, 4), 'd', (None, None)) self.assertEqual(self.mapping.get_character(EV_KEY, 4), 'd') self.assertEqual(len(self.mapping), 2) # this also works in the same way - self.mapping.change(EV_KEY, 4, 'e', 4) + self.mapping.change((EV_KEY, 4), 'e', (EV_KEY, 4)) self.assertEqual(self.mapping.get_character(EV_KEY, 4), 'e') self.assertEqual(len(self.mapping), 2) # and this - self.mapping.change(EV_KEY, '4', 'f', '4') + self.mapping.change((EV_KEY, '4'), 'f', (str(EV_KEY), '4')) self.assertEqual(self.mapping.get_character(EV_KEY, 4), 'f') self.assertEqual(len(self.mapping), 2) # non-int keycodes are ignored - self.mapping.change(EV_KEY, 'b', 'c', 'a') + # TODO what about non-int types? + self.mapping.change((EV_KEY, 'b'), 'c', (EV_KEY, 'a')) self.assertEqual(len(self.mapping), 2) def test_clear(self): @@ -119,10 +120,10 @@ class TestMapping(unittest.TestCase): self.assertEqual(len(self.mapping), 0) self.assertTrue(self.mapping.changed) - self.mapping.change(EV_KEY, 10, 'KP_1', None) + self.mapping.change((EV_KEY, 10), 'KP_1', (None, None)) self.assertTrue(self.mapping.changed) - self.mapping.change(EV_KEY, 20, 'KP_2', None) - self.mapping.change(EV_KEY, 30, 'KP_3', None) + self.mapping.change((EV_KEY, 20), 'KP_2', (None, None)) + self.mapping.change((EV_KEY, 30), 'KP_3', (None, None)) self.assertEqual(len(self.mapping), 3) self.mapping.clear(EV_KEY, 20) self.assertEqual(len(self.mapping), 2) @@ -131,9 +132,9 @@ class TestMapping(unittest.TestCase): self.assertEqual(self.mapping.get_character(EV_KEY, 30), 'KP_3') def test_empty(self): - self.mapping.change(EV_KEY, 10, '1') - self.mapping.change(EV_KEY, 11, '2') - self.mapping.change(EV_KEY, 12, '3') + self.mapping.change((EV_KEY, 10), '1') + self.mapping.change((EV_KEY, 11), '2') + self.mapping.change((EV_KEY, 12), '3') self.assertEqual(len(self.mapping), 3) self.mapping.empty() self.assertEqual(len(self.mapping), 0)