-
Notifications
You must be signed in to change notification settings - Fork 669
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
Fixes #64 - First implementation for libusb1-based hotplug #160
base: master
Are you sure you want to change the base?
Conversation
@roberthartung sorry, I didn't forget your PR, crazy weeks... |
Do you have any idea when you are going to look at this? I would love to use hotplugging. |
@ijager I lost my test hardware, I am working on a solution for that, that's why I stalled more serious patches for the project. |
@ijager You can add my repo as an upstream in the meantime and cherry pick the commit! |
I just uploaded a bugfix for user_data in the hotplug handle |
Oh yes, please consider merging this pull request! |
cool this works. Maybe maintainers can consider merging this PR or discuss what issues are remaining to fix? |
@walac Any news? |
I am not the maintainer of PyUSB anymore, 303 @SimplicityGuy |
Fixed another bug where userdata was pointing to the wrong object, if a dict was passed directly to the register function. |
Is there an example on how to use this in practice? |
The following example seems to work but also causes an exception import usb.hotplug
def on_plug(dev, event, user_data):
print(dev, event, user_data)
handle = usb.hotplug.register_callback(0x01 | 0x02, 0, -1, -1, -1, on_plug, None)
for event in usb.hotplug.loop():
print(event) Traceback (most recent call last):
File "_ctypes/callbacks.c", line 257, in 'converting callback result'
TypeError: an integer is required (got type NoneType)
Exception ignored in: <bound method _HotplugHandle.callback of <usb.backend.libusb1._HotplugHandle object at 0x7f676c5ab0d0>> |
Please help to rebase. Thanks. |
Conflict was trivial. Solved it through the web UI, sorry for the resulting ugly merge commit. Still needs to be tested, and possibly a fix for the issue mentioned by @raetiacorvus. |
I will try to rebase and test tomorrow! I will force push to get rid of the merge commit as well. |
Can one of the maintainers kindly take a look at the pull request and provide feedback. This is quite a useful feature that other libraries like https://github.com/python-escpos/python-escpos use to actively monitor connected usb printers. |
@@ -575,6 +582,16 @@ def libusb_fill_iso_transfer(_libusb_transfer_p, dev_handle, endpoint, buffer, l | |||
#int libusb_handle_events(libusb_context *ctx); | |||
lib.libusb_handle_events.argtypes = [c_void_p] | |||
|
|||
try: | |||
lib.libusb_hotplug_register_callback.argtypes = [c_void_p, c_int, c_int, c_int, c_int, c_int, _libusb_hotplug_callback_fn, py_object, POINTER(_libusb_hotplug_callback_handle)] | |||
except AttributeError: |
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.
Not really doing a review, just putting a comment: A mention about why exception handling was needed would be helpful
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.
I have no idea. :-D
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.
Looking at the code: I am not sure, the code existing code above mine also catched the exception - sometimes. Too bad I didn't document this. But I guess I just did the same way as the existing code.
Might have something to do with POINTER
c_void_p? not sure.
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.
In the other places it is used to deal with older versions libusb 1.0 that lack certain APIs.¹
All hotplug APIs are marked with
Since version 1.0.16, LIBUSB_API_VERSION >= 0x01000102
so the try-catch blocks are appropriate here.
¹ Though in some of these existing places it might not be necessary after all (e.g. libusb_get_port_number
).
It's on my queue, but there a few bugs ahead of it still. I'm also hoping that @roberthartung can take a first look at the possible issue pointed out by @raetiacorvus, since right now he knows a lot more about USB hotplug than me. |
…userdata was pointing to a wrong object
f8380b6
to
4ed0cd6
Compare
Force-pushed with latest rebase. |
@jonasmalacofilho @raetiacorvus You code is just missing a line of code, it should be:
The callback function requires a return value ( see Documentation ):
|
Quick update: I started to look over this pull request, but there are some things I still don't quite understand (so no review yet). I'm also thinking about whether the public API should be that tied to libusb1. Sure, libusb1 is the only backend that matters (in the foreseeable future); it's also hard to build a general API based on a single real world implementation. But maybe we could avoid depending/exposing/directly manipulating libusb1 constants, and also be a bit closer to |
This need to become a higher priority. |
Hm yeah it makes sense I guess. I will try to implement some abstracted API or at least provide generalized constants! |
Has this been abandoned? would be a really useful feature |
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.
I think my previous comment still applies.
I'm also thinking about whether the public API should be that tied to libusb1. Sure, libusb1 is the only backend that matters (in the foreseeable future); it's also hard to build a general API based on a single real world implementation. But maybe we could avoid depending/exposing/directly manipulating libusb1 constants, and also be a bit closer to find when it comes to specifying vendor/product/class filters.
While this appears to be a functional implementation, we have the problem that the advertised goal of PyUSB still is "an API rich, backend neutral Python USB module easy to use." This is a bit weird in a time where, essentially, only libusb1 is maintained, but it may still be useful if new backends become available.
I honestly don't know how I feel about dropping the "backend neutral" requirement, nor do I think I should get to decide about that alone. For the time being, I think it's better to keep as much neutrality as possible, which in this case would mean at least exploring an API that leaks a lot less of the underlying libusb1 backend.
I extracted the changes from this PR in to a monkey patch. Seems to work great - but happy to hear about any troubles I may find doing this. Hope to see this PR get resolved someday! Thanks. Patchimport usb
from usb.core import Device
from usb.backend.libusb1 import (
CFUNCTYPE, POINTER, c_int, c_void_p, _libusb_device_handle, c_uint, py_object,
_objfinalizer, byref, _Device
)
# # # # # # PATCH pyusb for hotplug # # # # # #
# Extracted from: https://github.com/pyusb/pyusb/pull/160
backend = usb.backend.libusb1.get_backend()
_libusb_hotplug_callback_handle = c_int
_libusb_hotplug_callback_fn = CFUNCTYPE(c_int, c_void_p, _libusb_device_handle, c_uint, py_object)
backend.lib.libusb_hotplug_register_callback.argtypes = [
c_void_p, c_int, c_int, c_int, c_int, c_int,
_libusb_hotplug_callback_fn, py_object,
POINTER(_libusb_hotplug_callback_handle)
]
backend.lib.libusb_hotplug_deregister_callback.argtypes = [
c_void_p, _libusb_hotplug_callback_handle
]
class _HotplugHandle(_objfinalizer.AutoFinalizedObject):
def __init__(self, backend, events, flags, vendor_id, product_id, dev_class, user_callback, user_data):
self.user_callback = user_callback
self.__callback = _libusb_hotplug_callback_fn(self.callback)
self.__backend = backend
# If an object is passed directly and never stored anywhere,
# the pointer will remain and user data might be wrong!
# therefore we have to keep a refernce here
self.__user_data = py_object(user_data)
handle = _libusb_hotplug_callback_handle()
backend.lib.libusb_hotplug_register_callback(backend.ctx, events, flags, vendor_id, product_id, dev_class,
self.__callback, self.__user_data, byref(handle))
def callback(self, ctx, dev, evnt, user_data):
dev = Device(_Device(dev), self.__backend)
return self.user_callback(dev, evnt, user_data)
def register_callback(callback, *,
user_data=None,
events=0x01 | 0x02, # arrive and exit
flags=0, # do not emumerate on register
vendor_id=-1, # match any
product_id=-1, # match any
dev_class=-1 # match any
):
return _HotplugHandle(backend, events, flags, vendor_id, product_id, dev_class, callback, user_data)
def deregister_callback(handle):
backend.lib.libusb_hotplug_deregister_callback(backend.ctx, handle.handle)
def loop():
while True:
yield backend.lib.libusb_handle_events(backend.ctx) Usedef on_plug(dev, event, user_data):
print(dev, event, user_data)
return 0
handle = register_callback(on_plug)
for event in loop():
print(event) |
Thank you a lot @williamkapke, works like a sharm ! |
|
FYI: there is a pending but working PR for libusb Windows hotplug implementation. |
No description provided.