ENH: Rework mounting mechanism to increase chances of successful mount #214
Conversation
wkz/device/mount.py
Outdated
| return False | ||
|
|
||
|
|
||
| def _is_of_type_block(device_tree: pyudev.core.Context, path: str) -> bool: |
There was a problem hiding this comment.
@gotiniens I changed the implementation of the mount mechanism quite a bit. However, I tried to stick to what we used to have in place for determining the device type as far as possible. As we have discussed earlier testing this piece of code is of course not trivial. So I would appreciate if you could spend a moment for testing this implementation with your device, before I merge it.
You should be able to directly install the implementation of this PR using
pip install git+https://github.com/fgebhart/workoutizer.git@refs/pull/214/head#egg=workoutizer
Let me know if you encounter any issues.
There was a problem hiding this comment.
Note, that these changes also require you to use the simplified version of the udev rule as given here: https://github.com/fgebhart/workoutizer/pull/214/files#diff-aa7a481338e2c1cee33238469338ef1d478d3912ced41db1c35146b9e6521b1bR52
There was a problem hiding this comment.
@fgebhart I just tested the pull request, first result is that it is not working for me, I get a general error:
10-10 14:42:25 - DEBUG - wkz.api - received POST request for mounting garmin device
10-10 14:42:25 - DEBUG - wkz.api - found connected garmin device
10-10 14:42:25 - ERROR - wkz.views - Error while loading /mount-device/
I need some extra logging to be sure of the exact problem, will try to add it locally later today
wkz/api.py
Outdated
| huey_task = HUEY.task(mount_device_and_collect_files) | ||
| # schedule huey task | ||
| huey_task() |
There was a problem hiding this comment.
I found this solution to starting the huey task correctly, see also suggestion for line 12
| huey_task = HUEY.task(mount_device_and_collect_files) | |
| # schedule huey task | |
| huey_task() | |
| mount_device_and_collect_files_task() |
You also need to add the following lines to wkz.tasks
from wkz.device.mount import mount_device_and_collect_files
@task()
def mount_device_and_collect_files_task():
mount_device_and_collect_files()
This tried to mount the device, but mis identified my device as an MTP device, resulting in using the wrong mount method. Still researching that problem.
There was a problem hiding this comment.
@gotiniens Thanks for looking into it - you are right.
It seems during development and testing my huey instance somehow hold a correct reference to the function which I intended to run as huey.task in its db. Now I removed the db (rm /tmp/huey.db) to start from scratch and ran into the exact same problem you described.
Your proposed solution also solves the issue for me. Will implement it, adapt the tests accordingly and update this PR asap. Thanks 👍
FYI: when debugging you can set the DJANGO_DEBUG env var to True which will avoid these generic error messages and output the entire traceback.
There was a problem hiding this comment.
Just double checked and everything seems to be working as expected now on my side. I assume this to be working for you as well, since you essentially suggested the fix.
However, will wait a few days more before merging to give you another chance to comment, @gotiniens.
wkz/device/mount.py
Outdated
|
|
||
| def _is_of_type_mtp(device_tree: pyudev.core.Context, path: str) -> bool: | ||
| devices = ( | ||
| device_tree.list_devices(subsystem="usb").match_property("DEVNAME", path).match_property("ID_MTP_DEVICE", "1") |
There was a problem hiding this comment.
Multiple match_propery are 'OR'ed and not 'AND'ed like you probably expect here. That's why the old _find_device_type looped through all found devices, and did an check on every found device.
My device is seen as MTP because of this rule.
There was a problem hiding this comment.
Yea, seems like I assumed the wrong thing. Let me remove .match_property("DEVNAME", path) - this should prevent your device from being seen as MTP.
|
|
||
|
|
||
| def _mount_device_using_pmount(path: str) -> str: | ||
| subprocess.check_output(["pmount", path, "garmin"]).decode("utf-8") |
There was a problem hiding this comment.
path is here /dev/bus/usb/001/004, but pmount needs /dev/sdb, The old _find_device_type has some extra magic to make this translation. I'm not really sure how to implement this in this setup.
There was a problem hiding this comment.
I will refactor this to actually use the approach you implemented to achieve this translation to the correct path.
| return True | ||
| else: | ||
| return False | ||
| def _get_block_device_path(device_tree: pyudev.core.Context, path: str) -> str: |
There was a problem hiding this comment.
@gotiniens I reverted this part back to almost reflect your original implementation. I hope this allows mounting block devices again (MTP still works with this implementation).
|
Those latest changes seem to work for me,Plugging the device in it is mounted automatically after it fails a few times (due to it not having settled yet), so it looks pretty robust. Then the files are copied automatically and processed. p.s. I got an e-mail from an comment you posted on thursday, could not find the comment anymore? |
Perfect. Thanks again for testing and providing feedback. Will merge this PR then.
yea... I deleted it a few minutes after I submitted it, because I realized I was out of my mind and sensed and bug where there actually was none 😆 I hoped you didn't get an email.. well just ignore it - all good 👍 |
This PR changes the mounting mechanism in the following way:
lsusbcommandFrom my tests this did not mount the device every time when I connected it, but at least most of the time. So I would consider this to fix #9.