-
Couldn't load subscription status.
- Fork 51
snagrecover: improve logging on USB access errors #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| from dataclasses import astuple | ||
| import struct | ||
| import logging | ||
| import errno | ||
|
|
||
| logger = logging.getLogger("snagrecover") | ||
|
|
||
|
|
@@ -37,10 +38,12 @@ def is_usb_path(usb_addr) -> bool: | |
| return isinstance(usb_addr, tuple) and isinstance(usb_addr[1], tuple) | ||
|
|
||
|
|
||
| def access_error(dev_type: str, dev_addr: str): | ||
| logger.error( | ||
| f"Device access error: failed to access {dev_type} device {dev_addr}, please check its presence and access rights" | ||
| ) | ||
| def access_error(dev_type: str, dev_addr: str, log_err: bool = True): | ||
| if log_err: | ||
| logger.error( | ||
| f"Device access error: failed to access {dev_type} device {dev_addr}, please check its presence and access rights" | ||
| ) | ||
|
|
||
| sys.exit(-1) | ||
|
|
||
|
|
||
|
|
@@ -141,7 +144,10 @@ def active_cfg_check(dev: usb.core.Device): | |
| dev.get_active_configuration() | ||
| except NotImplementedError: | ||
| return True | ||
| except usb.core.USBError: | ||
| except usb.core.USBError as e: | ||
| if e.errno == errno.EACCES: # permission error | ||
| raise e | ||
|
|
||
| logger.warning( | ||
| f"Failed to get configuration descriptor for device at {prettify_usb_addr((dev.bus, dev.port_numbers))}!" | ||
| ) | ||
|
|
@@ -170,8 +176,35 @@ def get_usb( | |
| if nb_devs == 1: | ||
| dev = dev_list[0] | ||
|
|
||
| if ready_check(dev): | ||
| return dev | ||
| try: | ||
| if ready_check(dev): | ||
| return dev | ||
| except usb.USBError as e: | ||
| if e.errno == errno.EACCES: | ||
| logger.error( | ||
| f"USB Device has been found at address {pretty_addr} but can't be accessed because of permission issue." | ||
| ) | ||
| if sys.platform == "linux": | ||
| logger.error( | ||
| "Please check your udev config (refer to README.md#Installation on Linux)." | ||
| ) | ||
| logger.error( | ||
| "The following udev rule allow access to the USB device:" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/allow access/grants access/ |
||
| ) | ||
| logger.error( | ||
| f'SUBSYSTEM=="usb", ATTRS{{idVendor}}=="{dev.idProduct:x}", ATTRS{{idProduct}}=="{dev.idVendor:x}", MODE="0660", TAG+="uaccess"' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. USB vendor/device ID pairs should be formatted with 4 hexadecimal digits and leading zeros, so: |
||
| ) | ||
| elif sys.platform == "win32": | ||
| logger.error( | ||
| "Please check your installation (refer to README.md#Installation on Windows 10 or 11)." | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be more specific, a USB access rights issue on Windows would likely be due to no appropriate libusb driver being bound to the device. So: "Please check that the 'libusb-win32' driver is bound to this USB device ID: {dev.idProduct:04x}:{dev.idVendor:04x}" |
||
| ) | ||
|
|
||
| if error_on_fail: | ||
| # Don't retry if it is a permission issue | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will still allow retrying if Moreover, I'm wondering if not retrying is correct or not. After all, the udev rule could take some time to kick in... Therefore, I think we should only run the access rights check on the last retry. |
||
| access_error("USB", pretty_addr, log_err=False) | ||
| else: | ||
| # Don't silence other USBErrors | ||
| raise e | ||
|
|
||
| elif nb_devs > 1: | ||
| logger.info( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/has been found/was found/
s/of permission issue/of a device file access rights issue/