Skip to content
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

Checks and installs pyusb and confirms compatibility with Latitude 7270 #19

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kalpo
Copy link

@kalpo kalpo commented Mar 12, 2022

This pull would:

  • Add a check for pyusb before importing it to handle gracefully failure to load the lib
  • Allows the user to install the library from the script (using pip)
  • Redirects the user to the official documentation for pyusb if it fails to install it
  • Adds a link to the pyusb site on pypi to the readme.md.
  • Adds to the readme.md file that the script has now been tested on latitude 7270

Feel free to edit all you want. Thanks again for taking the time to make this project happen

Gon Asla added 2 commits March 12, 2022 16:46
Also:
- Added a link to pip pyusb site
- Added Latitude 7270 to the list of tested laptops
@kalpo
Copy link
Author

kalpo commented Mar 31, 2022

Thank you for taking the time to give such good feedbac. I'll have some time to make those little changes soon.

	- usb device funtions now on a separate file
	- nfc main function catches import exception
		by importing separate file
	- no longer attempts to install pyusb
Copy link
Author

@kalpo kalpo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done the requested changes. It still works great on my latitude 7270.
Thanks again for taking the time to make this project!

nfc.py Outdated
from usbhandler import UsbDeviceFinder
except ImportError as e:
if e.name == 'usb':
print("'usb' module note found!.\nThe 'pyusb' library is required.")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Use logger (level - error) or print to stderr (print('...', file=sys.stderr)).
  2. If module other than usb cannot be imported, you'd not print anything - please add else clause for such case, i.e.:
else:
    raise

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Darn, you are right! Will do that now.

@jacekkow
Copy link
Owner

One thing left and I'm ready to merge it.

@kalpo
Copy link
Author

kalpo commented Apr 2, 2022

This last commit prints to sderr for any inport error. And only specifically tells the user that pyusb is required when it is that library that we they are missing. Happy to delete that bit if you think it is a bit too much info. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants