search

fgebhart / workoutizer

:weight_lifting: Browser based Sport and Workout Organizer :running_woman:
MIT License
62 stars 11 forks source link

ENH: Mount block devices #174

Closed gotiniens closed 3 years ago

gotiniens commented 3 years ago

As promised this is the changes needed for mounting block devices, as discussed in #167.

This is sadly not all needed for supporting the 920xt, because the fit collector did not seem to pick up the fit files. Thats for an other time.

The _find_device_type function tries to find the device type based on data published by udev. I added the pyudev module for this, in the future it maybe can be used to to monitor device plugins. You can identify MTP devices with an ID_MTP_DEVICE, block devices do not have such an identifier I could find, so that is a bit more unclear. The function returns the device type BLOCK or MTP and a device path needed to mount. This function does not work in the docker image on my machine, because the udev info is missing some critical values, I don't know why.

_mount_device_using_pmount can be used to mount the block devices, it has no output so I had to define the variable myself. pmount mounts at /media/{label} I hard coded the label to garmin. The pmount command needs to be installed on Raspbian.

I have not written tests because it looks hard to test such functions with physical interactions. Maybe you have to write fake pyudev functions to mimic the responses?

Be harsh on me when reviewing, I do not pretend to be an good programmer ;)

fgebhart commented 3 years ago

A few words on the fit collector:

The FitCollector class currently expects to find a folder called Activity within the path in which your device is mounted. It is assumed that the Activity folder contains the .fit files to be copied to the local path_to_trace_dir. My watch currently gets mounted at /run/user/1000/gvfs/, i.e. the activity dir would be /run/user/1000/gvfs/GARMIN/Primary/Activity/*.fit. If this is not the case for your watch, we could of course try to come up with a more general/flexible approach. Feel free to open a issue for this.

gotiniens commented 3 years ago

A few words on the fit collector:

The FitCollector class currently expects to find a folder called Activity within the path in which your device is mounted. It is assumed that the Activity folder contains the .fit files to be copied to the local path_to_trace_dir. My watch currently gets mounted at /run/user/1000/gvfs/, i.e. the activity dir would be /run/user/1000/gvfs/GARMIN/Primary/Activity/*.fit. If this is not the case for your watch, we could of course try to come up with a more general/flexible approach. Feel free to open a issue for this.

I think the capitalization of the dir names is the main culprit of not yet importing a bit like in #151 . But I first wanted to focus on the mounting of the device. I will create an issue/Pull request for that fix when I figured the exact problem.

fgebhart commented 3 years ago

I was able to verify that mounting of MTP devices still works 👍

fgebhart commented 3 years ago

Looking at your commit messages, are you aware of the pre-commit hooks? You might want to activate them, to automatically apply/check black, isort, flake8 formatting prior to each git commit. Enable the hooks with pre-commit install or manually run the hook with pre-commit run.

Could be worth auto activating them withing the run_docker.sh script, what do you think?

gotiniens commented 3 years ago

Added your suggestions.

Those pre-commit hooks are neat, I saw them mentioned but I thought they where run serverside, granted that was based on my experience with svn.

Adding the pre-commit hooks to run_docker.sh is not the best solution I think. But I do not know an better solution.

p.s. like mentioned above, there are still are some changes needed for my device. watchdogs._watch_for_device needs an small change, PR coming tommorow.

fgebhart commented 3 years ago

resolved conflict within changelog, will merge this once build passes.