ENH: Rework mounting mechanism to increase chances of successful mount

.vscode/settings.json

@@ -10,6 +10,9 @@
         "combi",
         "datetimepicker",
         "htmx",
+        "lsusb",
+        "pmount",
+        "pyudev",
         "timepicker",
         "unmounting",
         "wizer",

CHANGELOG.md

@@ -5,7 +5,21 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-...
+### Changed
+* Changed mounting mechanism to be triggered early on by `udev` whenever a Garmin device
+  is connected, even before knowing the exact product id and type. Thus workoutizer
+  takes care of waiting for the device being mountable and than mounts it, once it is
+  ready. This fixes [#9](https://github.com/fgebhart/workoutizer/issues/9). Mind the
+  following:
+  - In case your device gets mounted via the manually added `udev` rule and the new
+    mounting mechanism, you no longer need to provide the `path_to_garmin_device` setting
+    parameter, since the path gets detected automatically during mounting. Just leave the
+    setting empty to disable the device watchdog in this case. Your device will still be
+    detected automatically and fit files would be collected accordingly.
+  - In case your device is mounted automatically on your system (without specifying a
+    custom `udev` rule), you would still need to configure the path to your device within
+    the setting page aka `path_to_garmin_device` in order for workoutizer to regularly
+    check for your device and and collect  fit files.
 
 ## [0.22.0](https://github.com/fgebhart/workoutizer/releases/tag/v0.22.0) - 2021-09-20
 ### Fixed

setup/Readme.md

@@ -46,11 +46,10 @@ pip install workoutizer
 To configure your Raspberry Pi to automatically detect and mount your garmin watch you'll need to follow these steps:
 
 ### 1. Create a udev rule
-Create a file at `/etc/udev/rules.d/device_mount.rules` with the following content:
+Create a file at `/etc/udev/rules.d/99-mount_device.rules` with the following content:
 
 ```
-ACTION=="add", SUBSYSTEM=="block", ATTRS{idVendor}=="091e", TAG+="systemd", ENV{SYSTEMD_WANTS}="wkz_mount"
-ACTION=="add", SUBSYSTEM=="usb", ATTRS{idVendor}=="091e", ATTRS{ID_MTP_DEVICE}="1", TAG+="systemd", ENV{SYSTEMD_WANTS}="wkz_mount"
+ACTION=="add", ATTRS{idVendor}=="091e", TAG+="systemd", ENV{SYSTEMD_WANTS}="wkz_mount"
 ```
 
 ### 2. Create the wkz mount service
@@ -110,10 +109,14 @@ wkz run
 
 ## Background
 
-Whenever you connect your Garmin device to your Raspberry Pi, workoutizer will automatically mount the device. Workoutizer currently supports devices with MTP (e.g. FR645) and Block (e.g. FR920XT) modes. Some devices support both modes, some only one. 
+Whenever you connect your Garmin device to your Raspberry Pi, workoutizer will automatically mount the device.
+Workoutizer currently supports devices with MTP (e.g. FR645) and Block (e.g. FR920XT) modes. Some devices support both
+modes, some only one. 
 
-All devices are mounted using `udev` which is used to define the type of device. MTP devices are mounted as a [gvfs](https://en.wikipedia.org/wiki/GVfs) device, the file system of your device will
-be mounted at `/run/user/1000/gvfs/...`. This is the default location for Raspbian and workoutizer will look for your
-device in this location as default.
+All devices are mounted using `udev` which is used to define the type of device. MTP devices are mounted as a
+[gvfs](https://en.wikipedia.org/wiki/GVfs) device, the file system of your device will be mounted at
+`/run/user/1000/gvfs/...`. This is the default location for Raspbian and workoutizer will look for your device in this
+location as default.
 
-The Block devices are mounted using `pmount`, the file system is found at `/media/garmin`. You should configure this location under `Fetch Files from Device` under the setting from workoutizer.
+The Block devices are mounted using `pmount`, the file system is found at `/media/garmin`. You should configure this
+location under `Fetch Files from Device` under the setting from workoutizer.

setup/other/debugging.md

@@ -4,22 +4,23 @@
 * what is /run/user/: https://askubuntu.com/questions/842409/what-is-run-user-1000-gvfs
 
 ## useful commands
-display usb devices `lsusb`
-show attaching of new devices `dmesg`
-get attributes of usb device `udevadm info --attribute-walk --name=/dev/bus/usb/001/010`
-monitor changes when plugging devices `udevadm monitor --environment --udev`
-
-mount device via gio `gio mount -d /dev/bus/usb/001/004`
-unmount device `gio mount -u /run/user/1000/gvfs/mtp:host=091e_4b48_0000c4fa0516`
+* display usb devices `lsusb`
+* show attaching of new devices `dmesg`
+* get attributes of usb device `udevadm info --attribute-walk --name=/dev/bus/usb/001/010`
+* monitor changes when plugging devices `udevadm monitor --environment --udev`
+* mount device via gio `gio mount -d /dev/bus/usb/001/004`
+* unmount device `gio mount -u /run/user/1000/gvfs/mtp:host=091e_4b48_0000c4fa0516`
+* reload udev rules without reboot `sudo udevadm control --reload-rules && sudo udevadm trigger`
 
 ## my device
-idVendor=091e
-idProduct=4b48
-ATTRS{serial}=="0000c4fa0516"
+* idVendor=091e
+* idProduct=4b48
+* ATTRS{serial}=="0000c4fa0516"
+
 
-get properties of udev device `udevadm info --name=/dev/bus/usb/$BUS_NUMBER/$DEV_NUMBER --query=property`
-show info on partitions `cat /proc/partitions`
-get info on mount process `cat /proc/11/mountinfo`
+* get properties of udev device `udevadm info --name=/dev/bus/usb/$BUS_NUMBER/$DEV_NUMBER --query=property`
+* show info on partitions `cat /proc/partitions`
+* get info on mount process `cat /proc/11/mountinfo`
 
 
 ## short instructions

tests/conftest.py

@@ -1,4 +1,5 @@
 import os
+import subprocess
 
 import pytest
 
@@ -42,3 +43,28 @@ def fit_file_a():
 @pytest.fixture
 def fit_file_b():
     return "2019-09-25-16-15-53.fit"
+
+
+MOCKED_WAIT = 0.1
+MOCKED_RETRY = 3
+
+
+@pytest.fixture
+def mock_mount_waiting_time(monkeypatch):
+    from wkz.device import mount
+
+    # mock number of retries and waiting time to speed up test execution
+    monkeypatch.setattr(mount, "WAIT", MOCKED_WAIT)
+    monkeypatch.setattr(mount, "RETRIES", MOCKED_RETRY)
+
+
+@pytest.fixture
+def _mock_lsusb(monkeypatch) -> None:
+    # mocking the subprocess call to `lsusb` to get the desired outout
+    def mock(output: str) -> None:
+        def lsusb_output(foo) -> bytes:
+            return bytes(output, "utf8")
+
+        return monkeypatch.setattr(subprocess, "check_output", lsusb_output)
+
+    return mock

tests/db_tests/browser_tests/test_settings_page.py

@@ -1,4 +1,5 @@
 import datetime
+import logging
 import operator
 
 import pytest
@@ -91,11 +92,16 @@ def test_settings_page__demo_activity_present__delete_it(import_demo_data, live_
         webdriver.find_element(By.ID, "delete-demo-data")
 
 
-def test_settings_page__edit_and_submit_form(live_server, webdriver):
+def test_settings_page__edit_and_submit_form(db, live_server, webdriver_firefox, caplog, tmp_path):
+    # Note, running this test with firefox only, since chrome seemed to fail in github actions
+    # note sure why exactly, though. Test passed locally in docker environment.
+    webdriver = webdriver_firefox()
+    caplog.set_level(logging.DEBUG, logger="wkz.tools.sse")
+
     # get settings and check that all values are at their default configuration
     settings = models.get_settings()
     assert settings.path_to_trace_dir == django_settings.TRACKS_DIR
-    assert settings.path_to_garmin_device == "/run/user/1000/gvfs/"
+    assert settings.path_to_garmin_device == ""
     assert settings.delete_files_after_import is False
     assert settings.number_of_days == 30
 
@@ -109,26 +115,47 @@ def test_settings_page__edit_and_submit_form(live_server, webdriver):
     WebDriverWait(webdriver, 3).until(EC.element_to_be_clickable((By.ID, "id_path_to_trace_dir")))
     trace_dir_input_field = webdriver.find_element(By.ID, "id_path_to_trace_dir")
     trace_dir_input_field.clear()
-    trace_dir_input_field.send_keys("some/dummy/path")
+    # test invalid path
+    invalid_path = "invalid/dummy/path"
+    trace_dir_input_field.send_keys(invalid_path)
     # basically clicking somewhere else to trigger submitting the change
     webdriver.find_element(By.ID, "navigation").click()
 
     # wait until loading image shows up and disappears again
     WebDriverWait(webdriver, 3).until(EC.presence_of_element_located((By.ID, "loading-bar")))
     WebDriverWait(webdriver, 3).until(EC.invisibility_of_element_located((By.ID, "loading-bar")))
 
-    delayed_assertion(lambda: models.get_settings().path_to_trace_dir, operator.eq, "some/dummy/path")
+    delayed_assertion(lambda: models.get_settings().path_to_trace_dir, operator.eq, invalid_path)
+    assert f"{invalid_path} is not a valid path." in caplog.text
+
+    # test valid path
+    valid_path = tmp_path / "foo"
+    valid_path.mkdir()
+    assert valid_path.is_dir()
+    WebDriverWait(webdriver, 3).until(EC.element_to_be_clickable((By.ID, "id_path_to_garmin_device")))
+    garmin_device_input_field = webdriver.find_element(By.ID, "id_path_to_garmin_device")
+    garmin_device_input_field.clear()
+    garmin_device_input_field.send_keys(str(valid_path))
+    # click somewhere to trigger updating settings
+    webdriver.find_element(By.ID, "navigation").click()
+    # wait until loading image shows up and disappears again
+    WebDriverWait(webdriver, 3).until(EC.presence_of_element_located((By.ID, "loading-bar")))
+    WebDriverWait(webdriver, 3).until(EC.invisibility_of_element_located((By.ID, "loading-bar")))
+    delayed_assertion(lambda: models.get_settings().path_to_garmin_device, operator.eq, str(valid_path))
+    assert f"Device watchdog now monitors {valid_path}" in caplog.text
 
+    # test disabling by passing an empty string
+    empty_string = ""
     WebDriverWait(webdriver, 3).until(EC.element_to_be_clickable((By.ID, "id_path_to_garmin_device")))
     garmin_device_input_field = webdriver.find_element(By.ID, "id_path_to_garmin_device")
     garmin_device_input_field.clear()
-    garmin_device_input_field.send_keys("garmin/dummy/path")
     # click somewhere to trigger updating settings
     webdriver.find_element(By.ID, "navigation").click()
     # wait until loading image shows up and disappears again
     WebDriverWait(webdriver, 3).until(EC.presence_of_element_located((By.ID, "loading-bar")))
     WebDriverWait(webdriver, 3).until(EC.invisibility_of_element_located((By.ID, "loading-bar")))
-    delayed_assertion(lambda: models.get_settings().path_to_garmin_device, operator.eq, "garmin/dummy/path")
+    delayed_assertion(lambda: models.get_settings().path_to_garmin_device, operator.eq, empty_string)
+    assert "Disabled device watchdog." in caplog.text
 
     # got removed, should not be accessible
     with pytest.raises(NoSuchElementException):
@@ -139,8 +166,8 @@ def test_settings_page__edit_and_submit_form(live_server, webdriver):
 
     # again get settings and check that the values are the once entered above
     settings = models.get_settings()
-    assert settings.path_to_trace_dir == "some/dummy/path"
-    assert settings.path_to_garmin_device == "garmin/dummy/path"
+    assert settings.path_to_trace_dir == invalid_path
+    assert settings.path_to_garmin_device == ""
 
     # did not get changed, since selenium is not able to click into checkbox (?)
     assert settings.delete_files_after_import is False

tests/db_tests/conftest.py

@@ -10,7 +10,7 @@
 
 from wkz import models
 from wkz.demo import copy_demo_fit_files_to_track_dir, prepare_import_of_demo_activities
-from wkz.file_importer import run_importer__dask
+from wkz.io.file_importer import run_importer__dask
 from workoutizer import settings as django_settings
 
 

tests/db_tests/test_api.py

@@ -1,11 +1,11 @@
-import os
+import logging
 import subprocess
 
 import pytest
 from rest_framework.test import APIClient
 
-from wkz import models
-from wkz.file_helper import fit_collector
+from tests.unit_tests.device.test_mount import lsusb_ready_to_be_mounted_device
+from wkz.device import mount
 
 
 @pytest.fixture
@@ -25,55 +25,75 @@ def test_stop(client):
         client.post("/stop/")
 
 
-def test_mount_device__failure(db, monkeypatch, client):
+def test_mount_device__no_device_connected(db, monkeypatch, client):
     # mock output of subprocess to prevent function from failing
     def dummy_output(dummy):
-        return "dummy-string"
+        return b"dummy-string"
 
     monkeypatch.setattr(subprocess, "check_output", dummy_output)
     res = client.post("/mount-device/")
 
     # mounting a device is barely possible in testing, thus at least assert that the endpoint returns 500
-    assert res.status_code == 500
+    assert res.status_code == 200
+    assert res.content.decode("utf8") == '"No Garmin device connected."'
+
+
+def test_mount_device__device_connected(db, monkeypatch, client, mock_mount_waiting_time):
+    from workoutizer import settings as django_settings
+
+    def task(func):
+        return func
+
+    # first mock decorator HUEY.task with dummy function
+    monkeypatch.setattr(django_settings.HUEY, "task", task)
+
+    # mock output of subprocess to prevent function from failing
+    def dummy_output(dummy):
+        return b"dummy-string-containing-Garmin"
+
+    monkeypatch.setattr(subprocess, "check_output", dummy_output)
+    res = client.post("/mount-device/")
+
+    # mounting a device is barely possible in testing, thus at least assert that the endpoint returns 500
+    assert res.status_code == 200
+    assert res.content.decode("utf8") == '"Found device, will mount and collect fit files."'
 
 
 @pytest.mark.parametrize("mock_dev", ["BLOCK", "MTP"])
-def test_mount_device__success(db, monkeypatch, tmpdir, client, mock_dev, caplog):
-    # prepare settings
-    target_dir = tmpdir.mkdir("tracks")
-    settings = models.get_settings()
-    settings.path_to_garmin_device = tmpdir  # source
-    settings.path_to_trace_dir = target_dir  # target
-    settings.save()
+def test_mount_device__success(db, monkeypatch, tmpdir, client, mock_dev, caplog, _mock_lsusb):
+    caplog.set_level(logging.DEBUG, logger="wkz.api")
+    from workoutizer import settings as django_settings
 
-    # mock output of subprocess ("lsusb") to prevent function from failing
-    def check_output(dummy):
-        return "dummy\nstring\nsome\ncontent\ncontaining\nGarmin"
+    def task(func):
+        return func
+
+    # first mock decorator HUEY.task with dummy function
+    monkeypatch.setattr(django_settings.HUEY, "task", task)
 
-    monkeypatch.setattr(subprocess, "check_output", check_output)
+    # mock output of subprocess ("lsusb") to prevent function from failing
+    _mock_lsusb(lsusb_ready_to_be_mounted_device)
 
     # mock output of actual mounting command (with actual gio output text)
-    path_to_device = "/some/dummy/path/to/device"
+    path_to_device = tmpdir
 
-    def mount(path):
+    def mount_cmd(path):
         return f"Mounted /dev/bus/usb/001/004 at {path_to_device}"
 
-    monkeypatch.setattr(fit_collector, "_mount_device_using_gio", mount)
-    monkeypatch.setattr(fit_collector, "_mount_device_using_pmount", mount)
+    monkeypatch.setattr(mount, "_mount_device_using_gio", mount_cmd)
+    monkeypatch.setattr(mount, "_mount_device_using_pmount", mount_cmd)
 
-    # mock output of _find_device_type
-    def _find_device_type(bus, dev):
+    # mock output of _determine_device_type
+    def _determine_device_type(path):
         if mock_dev == "MTP":
-            return ("MTP", "/dev/bus/usb/001/002")
+            return "MTP"
         elif mock_dev == "BLOCK":
-            return ("BLOCK", "/dev/sda")
-
-    monkeypatch.setattr(fit_collector, "_find_device_type", _find_device_type)
+            return "BLOCK"
 
-    # create directory to import the fit files from
-    fake_device_dir = os.path.join(tmpdir, "mtp:host", "Primary", "GARMIN", "Activity")
-    os.makedirs(fake_device_dir)
+    monkeypatch.setattr(mount, "_determine_device_type", _determine_device_type)
 
+    # mount device (no new fit files collected)
     res = client.post("/mount-device/")
+    assert "received POST request for mounting garmin device" in caplog.text
     assert f"successfully mounted device at: {path_to_device}" in caplog.text
     assert res.status_code == 200
+    assert res.content.decode("utf8") == '"Found device, will mount and collect fit files."'

tests/db_tests/test_export_gpx_file.py

@@ -2,7 +2,7 @@
 
 from lxml import etree
 
-from wkz.file_helper.gpx_exporter import save_activity_to_gpx_file
+from wkz.io.gpx_exporter import save_activity_to_gpx_file
 
 
 def test_save_activity_to_gpx_file(activity):

tests/db_tests/test_import_activities.py

@@ -8,14 +8,14 @@
 from wkz import models
 from wkz.best_sections.generic import activity_suitable_for_awards
 from wkz.demo import copy_demo_fit_files_to_track_dir
-from wkz.file_importer import run_importer__dask
+from wkz.io.file_importer import run_importer__dask
 from wkz.tools.utils import calc_md5
 from workoutizer import settings as django_settings
 
 
 def test_activity_data_in_db_after_import(import_one_activity):
     """
-    This test corresponds to tests.unit_tests.file_helper.test_fit_parser.test__parse_records
+    This test corresponds to tests.unit_tests.io.test_fit_parser.test__parse_records
     but checks the integration with the database and the django model fields.
     """