mapping is a class now, some simplifications

bin/key-mapper-gtk

@@ -32,7 +32,7 @@ from gi.repository import Gtk, Gdk, GLib
 
 from keymapper.data import get_data_path
 from keymapper.X import create_setxkbmap_config, apply_preset, \
-    create_preset, get_mappings
+    create_preset, Mapping
 from keymapper.presets import get_presets, find_newest_preset, \
     delete_preset, rename_preset
 from keymapper.logger import logger, update_verbosity, log_info
@@ -43,6 +43,8 @@ window = None
 
 
 # TODO check for sudo rights
+# TODO NUM1 doesnt work anymore
+# TODO write some tests?
 
 
 def gtk_iteration():
@@ -51,35 +53,54 @@ def gtk_iteration():
         Gtk.main_iteration()
 
 
+keycode_reader = KeycodeReader()
+
+mapping = Mapping()
+
+
 class SingleKeyMapping:
     """A single, configurable key mapping."""
-    def __init__(
-        self, device, delete_callback, keycode_reader,
-        key_code=None, character=None,
-    ):
-        """Construct a row and add it to the list in the GUI."""
+    def __init__(self, device, delete_callback, keycode=None, character=None):
+        """Construct a row widget."""
         self.widget = None
         self.device = device
         self.delete_callback = delete_callback
-        self.keycode_reader = keycode_reader
-        self.put_together(key_code, character)
+        self.put_together(keycode, character)
 
     def get_widget(self):
         """Return the widget that wraps all the widgets of the row."""
         return self.widget
 
-    def start_watching_key_codes(self, *args):
-        print('start_watching_key_codes')
-        self.keycode_reader.clear()
-        GLib.timeout_add(100, self.get_newest_keycode)
+    def get_keycode(self):
+        keycode = self.keycode.get_label()
+        return int(keycode) if keycode else None
+
+    def get_character(self):
+        character = self.character_input.get_text()
+        return character if character else None
+
+    def start_watching_keycodes(self, *args):
+        print('start_watching_keycodes')
+        keycode_reader.clear()
+        GLib.timeout_add(1000 / 30, self.get_newest_keycode)
 
     def get_newest_keycode(self):
-        code = self.keycode_reader.read()
-        if code is not None:
-            self.key_code.set_label(str(code))
-        return self.key_code.is_focus()
+        new_keycode = keycode_reader.read()
+        previous_keycode = self.get_keycode()
+        character = self.get_character()
 
-    def put_together(self, key_code, character):
+        # TODO this is broken af
+        if new_keycode is not None:
+            try:
+                mapping.change(previous_keycode, new_keycode, character)
+                self.keycode.set_label(str(new_keycode))
+            except KeyError as e:
+                # Show error in status bar
+                logger.info(str(e)[1:-1])
+
+        return self.keycode.is_focus()
+
+    def put_together(self, keycode, character):
         """Create all GTK widgets."""
         delete_button = Gtk.EventBox()
         delete_button.add(Gtk.Image.new_from_icon_name(
@@ -93,18 +114,18 @@ class SingleKeyMapping:
         delete_button.set_margin_start(5)
         delete_button.set_margin_end(5)
 
-        key_code_input = Gtk.ToggleButton()
-        if key_code is not None:
-            key_code_input.set_label(key_code)
-        key_code_input.connect(
+        keycode_input = Gtk.ToggleButton()
+        if keycode is not None:
+            keycode_input.set_label(str(keycode))
+        keycode_input.connect(
             'focus-in-event',
-            self.start_watching_key_codes
+            self.start_watching_keycodes
         )
         # make the togglebutton go back to its normal state when doing
         # something else in the UI
-        key_code_input.connect(
+        keycode_input.connect(
             'focus-out-event',
-            lambda *args: key_code_input.set_active(False)
+            lambda *args: keycode_input.set_active(False)
         )
 
         character_input = Gtk.Entry()
@@ -116,7 +137,8 @@ class SingleKeyMapping:
 
         row = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)
         row.set_homogeneous(True)
-        row.pack_start(key_code_input, expand=True, fill=True, padding=0)
+        row.set_spacing(2)
+        row.pack_start(keycode_input, expand=True, fill=True, padding=0)
         row.pack_start(character_input, expand=True, fill=True, padding=0)
         row.pack_start(delete_button, expand=True, fill=False, padding=0)
 
@@ -126,10 +148,11 @@ class SingleKeyMapping:
 
         self.widget = row
         self.character_input = character_input
-        self.key_code = key_code_input
+        self.keycode = keycode_input
 
     def on_delete_button_clicked(self, *args):
         """Destroy the row and remove it from the config."""
+        mapping.clear()
         self.delete_callback(self)
 
 
@@ -138,9 +161,6 @@ class Window:
     def __init__(self):
         self.selected_device = None
         self.selected_preset = None
-        self.mappings = []
-
-        self.keycode_reader = KeycodeReader(gtk_iteration)
 
         gladefile = get_data_path('key-mapper.glade')
         builder = Gtk.Builder()
@@ -173,7 +193,6 @@ class Window:
 
     def on_close(self, *_):
         """Safely close the application."""
-        self.keycode_reader.stop_reading()
         Gtk.main_quit()
 
     def select_newest_preset(self):
@@ -217,35 +236,6 @@ class Window:
         key_list = self.get('key_list')
         key_list.forall(key_list.remove)
 
-    def update_mappings(self):
-        """Construct the mapping from the inputs without saving or applying."""
-        key_list = self.get('key_list')
-        mappings = []
-
-        def read_mapping(row):
-            box = row.get_children()[0]
-            columns = box.get_children()
-            code = columns[0].get_label()
-
-            # validate
-            character = columns[1].get_text()
-            if code == '' or character == '':
-                return
-            try:
-                code = int(code)
-            except ValueError:
-                return
-
-            # add to mapping
-            mappings.append((
-                int(columns[0].get_label()),
-                columns[1].get_text()
-            ))
-
-        key_list.forall(read_mapping)
-        logger.debug('Constructed mappings: %s', mappings)
-        self.mappings = mappings
-
     def on_save_preset_clicked(self, button):
         """Save changes to a preset to the file system."""
         new_name = self.get('preset_name_input').get_text()
@@ -279,11 +269,10 @@ class Window:
 
         self.selected_device = device
         self.selected_preset = None
-        self.mappings = []
 
         self.populate_presets()
         GLib.idle_add(
-            lambda: self.keycode_reader.start_reading(self.selected_device)
+            lambda: keycode_reader.start_reading(self.selected_device)
         )
 
     def on_create_preset_clicked(self, button):
@@ -301,45 +290,41 @@ class Window:
         logger.debug('Selecting preset "%s"', preset)
 
         self.selected_preset = preset
-        mappings = get_mappings(
+        mapping.load(
             self.selected_device,
             self.selected_preset
         )
 
         key_list = self.get('key_list')
-        for mapping in mappings:
-            mapping = SingleKeyMapping(
-                self.selected_device,
-                self.on_row_removed,
-                self.keycode_reader,
-                mapping[0],
-                mapping[1]
+        for keycode, character in mapping:
+            single_key_mapping = SingleKeyMapping(
+                device=self.selected_device,
+                delete_callback=self.on_row_removed,
+                keycode=keycode,
+                character=character
             )
-            key_list.insert(mapping.get_widget(), -1)
-
-        self.mappings = mappings
+            key_list.insert(single_key_mapping.get_widget(), -1)
 
         self.add_empty()
 
     def add_empty(self):
         empty = SingleKeyMapping(
-            self.selected_device,
-            self.on_row_removed,
-            self.keycode_reader
+            device=self.selected_device,
+            delete_callback=self.on_row_removed
         )
         key_list = self.get('key_list')
         key_list.insert(empty.get_widget(), -1)
 
-    def on_row_removed(self, mapping):
+    def on_row_removed(self, single_key_mapping):
         """Stuff to do when a row was removed
 
         Parameters
         ----------
-        mapping : SingleKeyMapping
+        single_key_mapping : SingleKeyMapping
         """
         key_list = self.get('key_list')
         # https://stackoverflow.com/a/30329591/4417769
-        key_list.remove(mapping.get_widget().get_parent())
+        key_list.remove(single_key_mapping.get_widget().get_parent())
         # shrink the window down as much as possible, otherwise it
         # will increase with each added mapping but won't go back when they
         # are removed.
@@ -360,7 +345,7 @@ class Window:
         create_setxkbmap_config(
             self.selected_device,
             self.selected_preset,
-            self.mappings
+            mapping
         )
 
 

keymapper/X.py

@@ -43,6 +43,62 @@ from keymapper.presets import get_presets
 from keymapper.linux import get_devices, can_grab
 
 
+class Mapping:
+    """Contains and manages mappings.
+
+    The keycode is always unique, multiple keycodes may map to the same
+    character.
+    """
+    def __init__(self):
+        self._mapping = {}
+
+    def __iter__(self):
+        """Iterate over tuples of unique keycodes and their character."""
+        return iter(self._mapping.items())
+
+    def load(self, device, preset):
+        """Parse the X config to replace the current mapping with that one."""
+        with open(get_home_path(device, preset), 'r') as f:
+            # from "key <12> { [ 1 ] };" extract 12 and 1,
+            # avoid lines that start with special characters
+            # (might be comments)
+            result = re.findall(r'\n\s+?key <(.+?)>.+?\[\s+(\w+)', f.read())
+            logger.debug('Found %d mappings in this preset', len(result))
+            self._mapping = {
+                int(keycode): character
+                for keycode, character
+                in result
+            }
+
+    def change(self, previous_keycode, new_keycode, character):
+        """Change a mapping. Return True on success."""
+        if new_keycode and character and new_keycode != previous_keycode:
+            self.add(new_keycode, character)
+            # clear previous mapping of that code, because the line
+            # representing that one will now represent a different one.
+            self.clear(previous_keycode)
+            return True
+        return False
+
+    def clear(self, keycode):
+        """Remove a keycode from the mapping."""
+        print('clear', id(self), keycode, self._mapping.get(keycode))
+        if self._mapping.get(keycode) is not None:
+            del self._mapping[keycode]
+
+    def add(self, keycode, character):
+        """Add a mapping."""
+        if self._mapping.get(keycode) is not None:
+            raise KeyError(
+                f'Keycode {keycode} is already mapped '
+                f'to {self._mapping.get(keycode)}'
+            )
+        self._mapping[keycode] = character
+
+    def get(self, keycode):
+        return self._mapping.get(keycode)
+
+
 def ensure_symlink():
     """Make sure the symlink exists.
 
@@ -181,15 +237,14 @@ def create_identity_mapping():
         keycodes.write(result)
 
 
-def generate_symbols_file_content(device, preset, mappings):
+def generate_symbols_file_content(device, preset, mapping):
     """Create config contents to be placed in /usr/share/X11/xkb/symbols.
 
     Parameters
     ----------
     device : string
     preset : string
-    mappings : array
-        tuples of code, character
+    mapping : Mapping
     """
     system_default = 'us'  # TODO get the system default
 
@@ -201,12 +256,12 @@ def generate_symbols_file_content(device, preset, mappings):
         keycodes = re.findall(r'<.+?>', f.read())
 
     xkb_symbols = []
-    for code, character in mappings:
-        if f'<{code}>' not in keycodes:
-            logger.error(f'Unknown keycode <{code}> for "{character}"')
+    for keycode, character in mapping.iter():
+        if f'<{keycode}>' not in keycodes:
+            logger.error(f'Unknown keycode <{keycode}> for "{character}"')
             # continue, otherwise X would crash when loading
             continue
-        xkb_symbols.append(f'key <{code}> {{ [ {character} ] }};')
+        xkb_symbols.append(f'key <{keycode}> {{ [ {character} ] }};')
     if len(xkb_symbols) == 0:
         logger.error('Failed to populate xkb_symbols')
         return None
@@ -239,17 +294,4 @@ def get_xinput_id_mapping():
 
     names = [name for name in names if name != '']
     ids = [int(id) for id in ids if id != '']
-    return zip(names, ids)
-
-
-def get_mappings(device, preset):
-    """Parse the X config to get a current mapping.
-
-    Returns tuples of (keycode, character)
-    """
-    with open(get_home_path(device, preset), 'r') as f:
-        # from "key <12> { [ 1 ] };" extract 12 and 1,
-        # avoid lines that start with special characters (might be comments)
-        result = re.findall(r'\n\s+?key <(.+?)>.+?\[\s+(\w+)', f.read())
-        logger.debug('Found %d mappings in this preset', len(result))
-        return result
+    return zip(names, ids)

keymapper/linux.py

@@ -46,15 +46,24 @@ def can_grab(path):
 
 
 class KeycodeReader:
-    def __init__(self, iterate):
-        self.iterate = iterate
-        self.keep_reading = False
-        self.currently_reading = False
-        self.newest_keycode = None
+    """Keeps reading keycodes in the background for the UI to use.
+
+    A new arriving keycode indicates that a button was pressed, so the
+    UI can keep checking for a new keycode on this object and act like the
+    keycode went right to the input box.
+
+    GTK inputs cannot listen on keys that don't write a character, so
+    they have to repeatedly ask for new data (in this solution).
+    """
+    def __init__(self):
+        self.virtual_devices = []
 
     def clear(self):
         """Next time when reading don't return the previous keycode."""
-        self.newest_keycode = None
+        # read all of them to clear the buffer or whatever
+        for virtual_device in self.virtual_devices:
+            while virtual_device.read_one():
+                pass
 
     def start_reading(self, device):
         """Start a loop that keeps reading keycodes.
@@ -63,46 +72,30 @@ class KeycodeReader:
         function that calls this until stop_reading is called from somewhere
         else.
         """
-        # stop the current loop
-        if self.currently_reading:
-            self.stop_reading()
-            while self.currently_reading:
-                self.iterate()
-
         # start the next one
         logger.debug('Starting reading keycodes for %s', device)
-        self.keep_reading = True
-        self.currently_reading = True
 
-        # all the virtual devices of the hardware.
-        # Watch over each one of them
+        # Watch over each one of the potentially multiple devices per hardware
         paths = _devices[device]['paths']
-        virtual_devices = [
+        self.virtual_devices = [
             evdev.InputDevice(path)
             for path in paths[:1]
         ]
 
-        while self.keep_reading:
-            for virtual_device in virtual_devices:
+    def read(self):
+        """Get the newest key or None if none was pressed."""
+        newest_keycode = None
+        for virtual_device in self.virtual_devices:
+            while True:
                 event = virtual_device.read_one()
-                if event is not None and event.type == evdev.ecodes.EV_KEY:
+                if event is None:
+                    break
+                elif event.type == evdev.ecodes.EV_KEY and event.value == 1:
+                    # value: 1 for down, 0 for up, 2 for hold.
                     # this happens to report key codes that are 8 lower
                     # than the ones reported by xev
-                    self.newest_keycode = event.code + 8
-            self.iterate()
-
-        # done
-        logger.debug('Stopped reading keycodes for %s', device)
-        self.currently_reading = False
-
-    def stop_reading(self):
-        """Stop the loop that keeps reading keycodes."""
-        self.keep_reading = False
-        self.newest_keycode = None
-
-    def read(self):
-        """Get the newest key."""
-        return self.newest_keycode
+                    newest_keycode = event.code + 8
+        return newest_keycode
 
 
 def get_devices():