full path to the daemon for loading presets, some TODOs, .json file extension, nested mapping in .json config

bin/key-mapper-gtk

@@ -50,12 +50,12 @@ if __name__ == '__main__':
     update_verbosity(options.debug)
     log_info()
 
-    if getpass.getuser() != 'root' and 'unittest' not in sys.modules.keys():
+    """if getpass.getuser() != 'root' and 'unittest' not in sys.modules.keys():
         ErrorDialog(
             'Error',
             'Sudo is required to talk to the daemon and discover devices'
         )
-        raise SystemExit(1)
+        raise SystemExit(1)"""
 
     window = Window()
 

keymapper/config.py

@@ -80,7 +80,7 @@ class _Config:
         with open(CONFIG_PATH, 'w') as file:
             json.dump(self._config, file, indent=4)
             logger.info('Saved config to %s', CONFIG_PATH)
-            shutil.chown(CONFIG_PATH, os.getlogin())
+            shutil.chown(CONFIG_PATH, os.getlogin(), os.getlogin())
             file.write('\n')
 
 

keymapper/daemon.py

@@ -31,9 +31,16 @@ from keymapper.logger import logger
 from keymapper.config import config
 from keymapper.injector import KeycodeInjector
 from keymapper.mapping import Mapping
+from keymapper.paths import get_config_path
 
 
 # TODO service file in data for a root daemon
+#  - start service as root with .service https://wiki.archlinux.org/index.php/Systemd#Writing_unit_files # noqa
+#  - starting service (root), running key-mapper-gtk (non-root), will the dbus
+#    be found? (Error says dbus not provided by .service files)
+
+
+# TODO the daemon creates an initial config in my home dir. it shouldn't.
 
 
 def is_service_running():
@@ -63,7 +70,8 @@ def get_dbus_interface():
     except Exception as error:
         logger.error(
             'Could not connect to the dbus of "key-mapper-service", mapping '
-            'keys only works as long as the window is open.'
+            'keys only works as long as the window is open. Is one of your '
+            'key-mapper processes not running as root?'
         )
         logger.error(error)
         return Daemon(autoload=False)
@@ -82,7 +90,7 @@ class Daemon(service.Object):
         if autoload:
             for device, preset in config.iterate_autoload_presets():
                 mapping = Mapping()
-                mapping.load(device, preset)
+                mapping.load(get_config_path(device, preset))
                 self.injectors[device] = KeycodeInjector(device, mapping)
 
         super().__init__(*args, **kwargs)
@@ -108,16 +116,25 @@ class Daemon(service.Object):
         'com.keymapper.Interface',
         in_signature='ss'
     )
-    def start_injecting(self, device, preset):
+    def start_injecting(self, device, path):
-        """Start injecting the preset for the device.
+        """Start injecting the preset on the path for the device.
 
         Returns True on success.
+
+        Parameters
+        ----------
+        device : string
+            The name of the device
+        path : string
+            Path to the json file that describes the preset
         """
+        # TODO the integration tests don't seem to test that path actually
+        #  exists
         if self.injectors.get(device) is not None:
             self.injectors[device].stop_injecting()
 
         mapping = Mapping()
-        mapping.load(device, preset)
+        mapping.load(path)
         try:
             self.injectors[device] = KeycodeInjector(device, mapping)
         except OSError:

keymapper/gtk/window.py

@@ -37,6 +37,7 @@ from keymapper.gtk.row import Row
 from keymapper.gtk.unsaved import unsaved_changes_dialog, GO_BACK
 from keymapper.reader import keycode_reader
 from keymapper.daemon import get_dbus_interface
+from keymapper.paths import get_config_path
 
 
 def gtk_iteration():
@@ -275,7 +276,7 @@ class Window:
         )
         success = self.dbus.start_injecting(
             self.selected_device,
-            self.selected_preset
+            get_config_path(self.selected_device, self.selected_preset)
         )
 
         if not success:
@@ -343,7 +344,8 @@ class Window:
         logger.debug('Selecting preset "%s"', preset)
 
         self.selected_preset = preset
-        custom_mapping.load(self.selected_device, self.selected_preset)
+        path = get_config_path(self.selected_device, self.selected_preset)
+        custom_mapping.load(path)
 
         key_list = self.get('key_list')
         for keycode, output in custom_mapping:

keymapper/mapping.py

@@ -129,9 +129,10 @@ class Mapping:
         self.changed = True
 
     @update_reverse_mapping
-    def load(self, device, preset):
+    def load(self, path):
-        """Load a dumped JSON from home to overwrite the mappings."""
+        """Load a dumped JSON from a path to overwrite the mappings."""
-        path = get_config_path(device, preset)
+        # since the daemon needs to load presets and runs as root, it
+        # needs the full path.
         logger.info('Loading preset from "%s"', path)
 
         if not os.path.exists(path):
@@ -140,7 +141,11 @@ class Mapping:
 
         with open(path, 'r') as file:
             mapping = json.load(file)
-            for keycode, character in mapping.items():
+            if mapping.get('mapping') is None:
+                logger.error('Invalid preset config at "%s"', path)
+                return
+
+            for keycode, character in mapping['mapping'].items():
                 try:
                     keycode = int(keycode)
                 except ValueError:
@@ -160,10 +165,14 @@ class Mapping:
             os.makedirs(os.path.dirname(path), exist_ok=True)
             os.mknod(path)
             # if this is done with sudo rights, give the file to the user
-            shutil.chown(path, os.getlogin())
+            shutil.chown(path, os.getlogin(), os.getlogin())
 
         with open(path, 'w') as file:
-            json.dump(self._mapping, file, indent=4)
+            # make sure to keep the option to add metadata if ever needed,
+            # so put the mapping into a special key
+            json.dump({
+                'mapping': self._mapping
+            }, file, indent=4)
             file.write('\n')
 
         self.changed = False

keymapper/paths.py

@@ -32,6 +32,14 @@ def get_config_path(device=None, preset=None):
     """Get a path to the stored preset, or to store a preset to."""
     if device is None:
         return CONFIG
+
+    if preset is not None:
+        # the extension of the preset should not be shown in the ui.
+        # currently only .json files are used.
+        assert not preset.endswith('.json')
+        preset = f'{preset}.json'
+
     if preset is None:
         return os.path.join(CONFIG, device)
+
     return os.path.join(CONFIG, device, preset)

keymapper/presets.py

@@ -46,7 +46,7 @@ def get_available_preset_name(device, preset='new preset'):
 
 
 def get_presets(device):
-    """Get all configured presets for the device, sorted by modification date.
+    """Get all presets for the device and user, starting with the newest.
 
     Parameters
     ----------
@@ -56,7 +56,7 @@ def get_presets(device):
     if not os.path.exists(device_folder):
         os.makedirs(device_folder)
     presets = [
-        os.path.basename(path)
+        os.path.splitext(os.path.basename(path))[0]
         for path in sorted(
             glob.glob(os.path.join(device_folder, '*')),
             key=os.path.getmtime
@@ -78,7 +78,8 @@ def get_any_preset():
 
 
 def find_newest_preset(device=None):
-    """Get a tuple of (device, preset) that was most recently modified.
+    """Get a tuple of (device, preset) that was most recently modified
+    in the users home directory.
 
     If no device has been configured yet, return an arbitrary device.
 
@@ -119,13 +120,14 @@ def find_newest_preset(device=None):
         logger.debug('None of the configured devices is currently online')
         return get_any_preset()
 
+    preset = os.path.splitext(preset)[0]
     logger.debug('The newest preset is "%s", "%s"', device, preset)
 
     return device, preset
 
 
 def delete_preset(device, preset):
-    """Delete a preset from the file system."""
+    """Delete one of the users presets."""
     preset_path = get_config_path(device, preset)
     if not os.path.exists(preset_path):
         logger.debug('Cannot remove non existing path "%s"', preset_path)
@@ -141,7 +143,7 @@ def delete_preset(device, preset):
 
 
 def rename_preset(device, old_preset_name, new_preset_name):
-    """Rename a preset while avoiding name conflicts."""
+    """Rename one of the users presets while avoiding name conflicts."""
     if new_preset_name == old_preset_name:
         return
 

tests/testcases/integration.py

@@ -190,7 +190,7 @@ class Integration(unittest.TestCase):
         self.window.get('preset_name_input').set_text('asdf')
         self.window.on_save_preset_clicked(None)
         self.assertEqual(self.window.selected_preset, 'asdf')
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/asdf'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/asdf.json'))
         self.assertEqual(custom_mapping.get_character(14), 'b')
 
     def test_select_device_and_preset(self):
@@ -206,15 +206,15 @@ class Integration(unittest.TestCase):
 
         # created on start because the first device is selected and some empty
         # preset prepared.
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset.json'))
         self.assertEqual(self.window.selected_device, 'device 1')
         self.assertEqual(self.window.selected_preset, 'new preset')
 
         # create another one
         self.window.on_create_preset_clicked(None)
         gtk_iteration()
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset.json'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset 2'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset 2.json'))
         self.assertEqual(self.window.selected_preset, 'new preset 2')
 
         self.window.on_select_preset(FakeDropdown('new preset'))
@@ -223,26 +223,26 @@ class Integration(unittest.TestCase):
 
         self.assertListEqual(
             sorted(os.listdir(f'{CONFIG}/device 1')),
-            ['new preset', 'new preset 2']
+            sorted(['new preset.json', 'new preset 2.json'])
         )
 
         # now try to change the name
         self.window.get('preset_name_input').set_text('abc 123')
         gtk_iteration()
         self.assertEqual(self.window.selected_preset, 'new preset')
-        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/abc 123'))
+        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/abc 123.json'))
         custom_mapping.change(10, '1', None)
         self.window.on_save_preset_clicked(None)
         gtk_iteration()
         self.assertEqual(self.window.selected_preset, 'abc 123')
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/abc 123'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/abc 123.json'))
         self.assertListEqual(
             sorted(os.listdir(CONFIG)),
-            ['device 1']
+            sorted(['device 1'])
         )
         self.assertListEqual(
             sorted(os.listdir(f'{CONFIG}/device 1')),
-            ['abc 123', 'new preset 2']
+            sorted(['abc 123.json', 'new preset 2.json'])
         )
 
     def test_start_injecting(self):

tests/testcases/mapping.py

@@ -23,6 +23,7 @@ import unittest
 
 from keymapper.mapping import Mapping
 from keymapper.state import parse_xmodmap
+from keymapper.paths import get_config_path
 
 
 class TestMapping(unittest.TestCase):
@@ -44,7 +45,7 @@ class TestMapping(unittest.TestCase):
         self.mapping.save('device 1', 'test')
         loaded = Mapping()
         self.assertEqual(len(loaded), 0)
-        loaded.load('device 1', 'test')
+        loaded.load(get_config_path('device 1', 'test'))
         self.assertEqual(len(loaded), 3)
         self.assertEqual(loaded.get_character(10), '1')
         self.assertEqual(loaded.get_character(11), '2')

tests/testcases/presets.py

@@ -47,21 +47,21 @@ class TestCreatePreset(unittest.TestCase):
         self.assertEqual(get_any_preset(), ('device 1', None))
         create_preset('device 1')
         self.assertEqual(get_any_preset(), ('device 1', 'new preset'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset.json'))
 
     def test_create_preset_2(self):
         create_preset('device 1')
         create_preset('device 1')
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset.json'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset 2'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset 2.json'))
 
     def test_create_preset_3(self):
         create_preset('device 1', 'pre set')
         create_preset('device 1', 'pre set')
         create_preset('device 1', 'pre set')
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set.json'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set 2'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set 2.json'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set 3'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set 3.json'))
 
 
 class TestDeletePreset(unittest.TestCase):
@@ -72,13 +72,13 @@ class TestDeletePreset(unittest.TestCase):
     def test_delete_preset(self):
         create_preset('device 1')
         create_preset('device 1')
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset.json'))
         delete_preset('device 1', 'new preset')
-        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/new preset'))
+        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/new preset.json'))
         self.assertTrue(os.path.exists(f'{CONFIG}/device 1'))
         delete_preset('device 1', 'new preset 2')
-        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/new preset'))
+        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/new preset.json'))
-        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/new preset 2'))
+        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/new preset 2.json'))
         # if no preset in the directory, remove the directory
         self.assertFalse(os.path.exists(f'{CONFIG}/device 1'))
 
@@ -94,10 +94,10 @@ class TestRenamePreset(unittest.TestCase):
         create_preset('device 1', 'foobar')
         rename_preset('device 1', 'preset 1', 'foobar')
         rename_preset('device 1', 'preset 2', 'foobar')
-        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/preset 1'))
+        self.assertFalse(os.path.exists(f'{CONFIG}/device 1/preset 1.json'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar.json'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 2'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 2.json'))
-        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 3'))
+        self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 3.json'))
 
 
 class TestFindPresets(unittest.TestCase):