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

Migration to libusb1 and changes to make it compatible with Python on Windows #274

Closed

Conversation

juanmitaboada
Copy link

@juanmitaboada juanmitaboada commented Dec 4, 2017

… Windows

Contributor checklist

  • I have read the CONTRIBUTING.rst
  • I have tested my contribution on these devices:
  • e.g. Epson TM-T88II
  • My contribution is ready to be merged as is

Description

@patkan
Copy link
Member

patkan commented Dec 4, 2017

Hi, thank you for your contribution :-) I'm going to have a look at it later.
In order to make the AUTHORS work like you added it, you could use a mailmap line like this:

Juanmi Taboada // br0th3r <juanmi@juanmitaboada.com> Juanmi Taboada <juanmi@centrologic.com>

@juanmitaboada
Copy link
Author

Thank you, AUTHORS corrected. :-)

Copy link
Member

@patkan patkan 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 added some questions in the code.
All in all the changes look good to me, however I don't have a USB-device so maybe somebody could check it in person?

@br0th3r could you please explain the difference between pyusb and libusb1? As far as I see it, pyusb>=1 also uses libusb1.x as backend and should also work on Windows.

if check_driver is None or check_driver:
try:
self.device.detach_kernel_driver(0)
except usb.core.USBError as e:
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to also check for this exception in your code?

Copy link
Author

Choose a reason for hiding this comment

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

I am not sure if this is necesary with libusb1.

@@ -107,7 +107,7 @@ def run_tests(self):
'Topic :: Office/Business :: Financial :: Point-Of-Sale',
],
install_requires=[
'pyusb>=1.0.0',
'libusb1',
Copy link
Member

Choose a reason for hiding this comment

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

Are all versions of this lib OK?

Copy link
Author

Choose a reason for hiding this comment

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

I haven't checked other versions of libusb1.

@juanmitaboada
Copy link
Author

Pyusb didn't work on Windows for me after 10h trying on several ways. Reading on Internet I got to know that there several differences between the API v0.1 and v1.x of libusb1 and Pyusb is not compatible with the latest which happens to work properly on Windows.

I have checked this library on Linux and Windows 7 for an EPSON TM-T20II (MODEL M267D) and it worked like a charm. :-)

@patkan
Copy link
Member

patkan commented Dec 12, 2017

So if nobody opposes I would merge this in the coming days.

@belono
Copy link
Contributor

belono commented Dec 12, 2017

Is possible to extend support for libusb1 without dropping pyusb?

@patkan
Copy link
Member

patkan commented Dec 13, 2017

I will research how to set optional dependencies for a Python package. This way we could use both libraries.

What bothers me a bit now is that I think I remember that pyusb allegedly supports libusbv1.
I will also research this.

patkan added a commit that referenced this pull request May 13, 2018
cherry-picked from 8494cca in #274
@patkan
Copy link
Member

patkan commented May 15, 2018

It seems possible to specify platform-dependant requirements. https://stackoverflow.com/a/32955538/4244236 https://stackoverflow.com/a/42946495/4244236
This change should keep the old library an allow to switch to the new dependency. Ideally the proper package is preselected for the platform and can be switched. (In the module this can be realized by trying to import the first one, then falling back to the other if the import does not succeed.)

@om26er
Copy link
Contributor

om26er commented May 22, 2019

@patkan @belono I made an attempt to make things work on Windows here #330

@patkan patkan mentioned this pull request Jun 4, 2019
3 tasks
@patkan patkan closed this in #330 Jun 4, 2019
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.

None yet

4 participants