diff --git a/bin/key-mapper-gtk b/bin/key-mapper-gtk index b72e1ec4..ab37daaa 100755 --- a/bin/key-mapper-gtk +++ b/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() diff --git a/keymapper/config.py b/keymapper/config.py index 4756397b..35463a66 100644 --- a/keymapper/config.py +++ b/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') diff --git a/keymapper/daemon.py b/keymapper/daemon.py index d97815dd..c743d8ac 100644 --- a/keymapper/daemon.py +++ b/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): - """Start injecting the preset for the device. + def start_injecting(self, device, path): + """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: diff --git a/keymapper/gtk/window.py b/keymapper/gtk/window.py index 0c333cd8..22c35cdd 100755 --- a/keymapper/gtk/window.py +++ b/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: diff --git a/keymapper/mapping.py b/keymapper/mapping.py index e4aa3476..3a69a336 100644 --- a/keymapper/mapping.py +++ b/keymapper/mapping.py @@ -129,9 +129,10 @@ class Mapping: self.changed = True @update_reverse_mapping - def load(self, device, preset): - """Load a dumped JSON from home to overwrite the mappings.""" - path = get_config_path(device, preset) + def load(self, path): + """Load a dumped JSON from a path to overwrite the mappings.""" + # 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 diff --git a/keymapper/paths.py b/keymapper/paths.py index f2dd176c..c97c5283 100644 --- a/keymapper/paths.py +++ b/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) diff --git a/keymapper/presets.py b/keymapper/presets.py index 775a70f9..628c9b02 100644 --- a/keymapper/presets.py +++ b/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 diff --git a/tests/testcases/integration.py b/tests/testcases/integration.py index 120e5024..45f8cfa8 100644 --- a/tests/testcases/integration.py +++ b/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 2')) + self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset.json')) + 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): diff --git a/tests/testcases/mapping.py b/tests/testcases/mapping.py index 7c4d629c..34ed6059 100644 --- a/tests/testcases/mapping.py +++ b/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') diff --git a/tests/testcases/presets.py b/tests/testcases/presets.py index 8eabf73c..1990f86d 100644 --- a/tests/testcases/presets.py +++ b/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 2')) + self.assertTrue(os.path.exists(f'{CONFIG}/device 1/new preset.json')) + 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 2')) - self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set 3')) + self.assertTrue(os.path.exists(f'{CONFIG}/device 1/pre set.json')) + 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.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 2')) + self.assertFalse(os.path.exists(f'{CONFIG}/device 1/new preset.json')) + 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.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar')) - self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 2')) - self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 3')) + self.assertFalse(os.path.exists(f'{CONFIG}/device 1/preset 1.json')) + self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar.json')) + self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 2.json')) + self.assertTrue(os.path.exists(f'{CONFIG}/device 1/foobar 3.json')) class TestFindPresets(unittest.TestCase):