google / gousb

gousb provides low-level interface for accessing USB devices
Apache License 2.0
813 stars 121 forks source link

Hot Plug Support #8

Open bruce opened 6 years ago

bruce commented 6 years ago

Any interest in the participants from kylelemons/gousb#9 (@zagrodzki , @nkovacs, et al) reviving the work on the hotplug event support in this version?

(My own golang/c skills are woefully inadequate to help in anything much besides testing and moral support, but I would be very interested in this feature for a project I'm flailing around with.)

nkovacs commented 6 years ago

I'm sorry, I haven't had time for this, and my old fork works well enough for the project where I needed this. I'll probably take another stab at it when I have to update that project, but no promises.

nkovacs commented 6 years ago

I've started working on this: https://github.com/nkovacs/gousb/tree/hotplug

Because the libusb function has a bunch of optional parameters, I've decided to use a fluent api:

cb := ctx.Hotplug().Enumerate().ProductID(42).Register(func(event gousb.HotplugEvent) {
    ...
})

Another remaining issue is how to handle deregistering a callback inside the callback. This might be useful if you want to have a one-off event. Although you could call the returned object's Deregister function in the callback (cb.Deregister()), there's a race condition there, because the cb object might not exist when the callback is first called. Plus you would have to write your code like this:

var cb gousb.HotplugCallback

cb = ctx.Hotplug().Enumerate().ProductID(42).Register(func(event gousb.HotplugEvent) {
    cb.Deregister()
})

Which is unwieldy (and, of course, racy). Putting a method onto HotplugEvent could work, though the other methods must not be called after the callback function returns (I think that's how libusb works, you're probably not supposed to hang on to the parameters passed in to the callback function), and implementing it correctly is a bit hard. The libusb callback can return true to deregister the event, and that's how I implemented it before, but that's also confusing, and I don't like forcing you to write a return in the callback when that's not a common use case (see https://github.com/kylelemons/gousb/pull/9#discussion_r99775808). Also, it doesn't work correctly, because I have to clean up the map, or it'll leak memory (the previous version was also racy).

zagrodzki commented 6 years ago

Thanks. I've added some minor comments at the first pass, but to be honest, I already completely forgot how all this works.

What would be the purpose of the callback? apart from deregistering the handler. If there's no other use for it, then perhaps we could have something like this:

h := ctx.Hotplug(func(sameH *gousb.Hotplug, ev gousb.HotplugEvent) {
  ...
  sameH.Cancel()
}, ...<options>...)
...
h.Cancel()

Ideally, to get me back on track, would you mind preparing a few examples of how the new API would be used for different cases?

nkovacs commented 6 years ago

My use case is to listen forever for any device, both leave and arrive, so it would look like this:

ctx.Hotplug.Enumerate().Register(func() {
...
})
// ctx.Close will clean up everything

If you're only interested when one device leaves (you've opened it, but you want to know when it goes away):

id := uint16(desc.Bus) + 255 + uint16(desc.Address)
ctx.Hotplug.Left().Register(func(event gousb.HotplugEvent) {
    desc, _ := event.DeviceDesc()
    leftID := uint16(desc.Bus) + 255 + uint16(desc.Address)
    if id == leftID {
        // do some cleanup now that the device has left
        // deregister the hotplug callback somehow
    }
})

Though it would probably be better to register a single "left" callback instead of a per-device one.

If you only care about a specific type of device:

ctx.Hotplug.ProductID(0x1234).VendorID(0x1234).Enumerate().Register(func() {
...
})

Now that I thought about it, maybe productID and vendorID filtering is not necessary. It doesn't allow you to register the same callback for multiple products, for example: PID: 18d1, VID: 2d00 is Android in accessory mode, but if you have ADB enabled, the VID changes to 2d01, so you would have to register the same function as two separate callbacks. And you can just use an if inside the callback. The same goes for device class. So maybe the whole thing can be simplified to something like this:

RegisterHotplug(cb func(HotplugEvent), enumerate bool)
RegisterHotplugArrived(cb func(HotplugEvent), enumerate bool)
// enumerate doesn't make sense here
RegisterHotplugLeft(cb func(HotplugEvent))

Also, I found this in libusb's source code: https://github.com/libusb/libusb/blob/6402a10c145d1a38964fc43009d115fe7a86c8c7/libusb/hotplug.c#L62 This wasn't documented in the api doc.

kylelemons commented 6 years ago

Just as a drive-by comment, I tend to prefer the variadic options approach rather than the fluent/builder approach in Go.

You can kinda combine them a bit and let people find the balance that works for them too.

I would be kinda tempted to make it so that you enable hotplug and the Go library receives all events so that you can build a more useful and powerful API on top of it, like:

hp := ctx.NewHotplug().Match(hotplug.Enter())
for _, product := range products {
  hp.MatchAll(hotplug.Vendor(product.VendorID), hotplug.Product(product.ProductID))
}
hp.MatchAny(hotplug.Class(usb.HID), hotplug.Class(usb.MSC), hotplug.Class(usb.UMS))
hp.Callback(func(ev *hotplug.Event) { ... })
hp.CallbackOnce(func(ev *hotplug.Event) { ... })
nkovacs commented 6 years ago

What is hotplug in your example? A separate package?

Also, the MatchAny/MatchAll functions wouldn't work like that, libusb doesn't support matching like that. We'd have to reimplement matching in gousb.

kylelemons commented 6 years ago

Yes, hotplug was a separate package (to avoid usb.HotplugFoo for everything) and yeah, that was the point of the illustration. It might also very naturally solve the unregistration problem if you only have to worry about a single call back that is enabled and disabled via a top level function and that call back hands off to a dispatcher that understands a more natural set of filters and Go idioms.

zagrodzki commented 6 years ago

I'm not actually sure if we need that complex logic. Thinking some more about it, why do we need to do some special filter classes in the callback, if we could just let the user match any event directly, using Go instead of our invented matcher syntax. I generally agree with Kyle's sentiment that perhaps we should just pass all the hotplug events, I'd be up for that.

So I could imagine something as simple as this:

func (Context) RegisterHotplug(func(ev *HotplugEvent)) cancel func()

type HotplugEvent struct {
  Type HotplugEventType
  DeviceDesc *DeviceDesc
}

func (HotplugEvent) Open() (*Device, error)
func (HotplugEvent) Cancel()

HotplugEvent.Cancel does the same thing as the cancel closure returned by RegisterHotplug.

Everything related to matching can be done by the user based on the ev.DeviceDesc.

The cases mentioned by @nkovacs above would then be expressed as:

listen for all enumerated, arriving and leaving devices:

ctx.RegisterHotplug(func(ev *gousb.HotplugEvent) {
  ...
})

One particular device:

bus, addr := desc.Bus, desc.Address
ctx.RegisterHotplug(func(ev *gosusb.HotplugEvent)) {
  if ev.DeviceDesc.Bus != bus || ev.DeviceDesc.Addr != addr || ev.Type != gousb.HotplugLeft {
    return
  }
  // do some cleanup now that the device has left
  ev.Cancel()
})

WDYT?

nkovacs commented 6 years ago

@zagrodzki What about enumerate?

I don't know if it's a use case we should care about, but with your suggestion, we cannot leave out already plugged in devices, since the callback doesn't receive this information.

That's why I suggested this:

RegisterHotplug(cb func(HotplugEvent), enumerate bool)
RegisterHotplugArrived(cb func(HotplugEvent), enumerate bool)
// enumerate doesn't make sense here
RegisterHotplugLeft(cb func(HotplugEvent))
zagrodzki commented 6 years ago

I think not supporting it is a better option. I think the enumerate flag is poorly designed. Looking at the code, the only thing it does is an equivalent of OpenDevices (libusb_get_device_list underneath) after the callback has been registered. I don't think it makes sense to reuse that part of the implementation.

So I think it might be enough to first use RegisterHotplug, then call OpenDevices in the code. If it later turns out the enumerate behavior is valuable, I'd be up for reimplementing it on top of libusb, with a dedicated gousb event type "enumerate", instead of ambiguous "arrived" for both new arrival and enumerated.

nkovacs commented 6 years ago

While I agree with you that it's bad that the two overlap, I disagree with removing it. For my use case, enumerate is very convenient because I don't care if the device was plugged in before my program started, and I think this is the more common use case.

It also avoids a race condition (device gets plugged in after RegisterHotplug but before OpenDevices).

zagrodzki commented 6 years ago

The enumerate documentation states explicitly that it doesn't avoid the race:

This means that when using LIBUSB_HOTPLUG_ENUMERATE, your callback may be called twice for the arrival of the same device, once from libusb_hotplug_register_callback() and once from libusb_handle_events(); and/or your callback may be called for the removal of a device for which an arrived call was never made.

http://libusb.sourceforge.net/api-1.0/group__hotplug.html#gae6c5f1add6cc754005549c7259dc35ea

I can agree on the convenience factor if you think that's useful. I'd be fine with adding an equivalent of enumerate controlled by Go code. I think then it would make sense to have three event types (arrive/leave/enumerate) to let the users decide if they want to use enumeration or not (with a simple if in the callback). I.e. have enumerate always enabled. We could then also properly handle the "stop callback" signal regardless of whether it arrives from enumeration or hotplug event, since we'd do that in Go code anyway.

nkovacs commented 6 years ago

Oh, that's unfortunate.

I'm not sure about making enumerate a separate event type. It's just an arrive event, your code is probably going to handle it the same way. And I'm struggling to come up with a use case where you would not want to use enumerate.

zagrodzki commented 6 years ago

I'm not sure. libusb defaults to "not enumerate", but I don't know if that's just because legacy API was not convenient enough and they needed to preserve compatibility, or because there are cases where you don't care about enumeration. Still, making a distinction between arrive and enumerate seems trivial for us and easy for the users to understand and use, while libusb's behavior makes it impossible to make a distinction. There's also the "cancel registration" which in libusb works differently depending on whether it's a callback or enumeration, which I find frustrating.

I'd rather preserve the capability to differentiate than to loose it irreversibly up front. So let's make separate event types for "arrived" and separate for "enumerated" and let's mention explicitly that the libusb restriction holds - you might get "arrived" and "enumerated" for the same device, and you may get "left" for a device that was never "arrived" or "enumerated".

nkovacs commented 6 years ago

If we implement the enumerate in Go code, we can handle cancellation correctly. We could also deal with the race condition, but that feels like it's too much work for something that's easy enough to handle on the callback's side (i.e. check if you already have the device, ignore the duplicate "arrived"/"enumerated" and don't do anything for "left" if you don't have the device).

There's an old issue about refactoring libusb's hotplug API: https://github.com/libusb/libusb/issues/29 It makes a good point about your callback being split into two with an if.

With the enumerate event, your callback will almost always look like this:

if (event.Type == gousb.HotplugEventDeviceArrived || event.Type == gousb.HotplugEventDeviceEnumerated) {
    // handle device arrived
} else {
    // handle device left
}

Maybe separate callbacks, one for left and one for arrived (with enumerate being a bool on the event object for the "arrived" event, so you can ignore it if you don't care, which I think is the common use case) would be better?

Another thing I noticed now that I looked at libusb's source: libusb_handle_events runs in a separate goroutine, and that will trigger the callback, but enumerate can trigger the callback on the goroutine calling register. Although libusb has mutexes for the callbacks, the callback functions themselves can be called from two separate goroutines, so you would have to use locking in your callbacks. That's probably another thing we should fix, but I'm not sure how.

zagrodzki commented 6 years ago

Re race conditions: agreed, I don't think it's worth addressing.

Re two distinct callbacks vs one function: unsure. I suppose we have four options, really:

I would be fine with an interface, although I'm not sure if that doesn't make the setup unnecessarily convoluted, too much java-factory-style. So in the end I think I would still opt for a single function.

@kylelemons thoughts?

Re threads: that's not a problem, the callbacks should be thread-safe and ok to call concurrently. FWIW, I would not want to use the enumerate function, instead I'd enumerate in Go code, but that still has the same property - the callback might be called while the Go enumeration is calling the same callback. I don't think this needs to be "fixed" in any way, just mentioned in the docs that the callbacks may be called concurrently for the same device.

nkovacs commented 6 years ago

two registrations, each for one purpose

The problem with that is that is that you can't register just one libusb callback, and if you register arrive first, you might get an arrive event but miss a leave event.

Instead of an interface, you could also use the builder pattern (chaining optional):

hotplug := gousb.NewHotplug()
hotplug.OnArrive(func(e HotplugArriveEvent) {
})
hotplug.OnEnumerate(func(e HotplugArriveEvent) {
})
// or use hotplug.OnArriveEnumerate if you want the same callback for both
hotplug.OnLeave(func(e HotplugLeaveEvent) {
})
hotplug.Register()

The reason for the different type of event is that HotplugArriveEvent can open the device, HotplugLeaveEvent can't. The only bad thing about this is that you have to call hotplug.Register(), otherwise nothing will happen and you won't get any error, it just won't work.

Re threads: that's not a problem, the callbacks should be thread-safe and ok to call concurrently. FWIW, I would not want to use the enumerate function, instead I'd enumerate in Go code, but that still has the same property - the callback might be called while the Go enumeration is calling the same callback. I don't think this needs to be "fixed" in any way, just mentioned in the docs that the callbacks may be called concurrently for the same device.

All the normal event callbacks get called from the same goroutine (the one running libusbImpl.handleEvents), enumerate is the only one getting called from a different goroutine. But you're right, in practice you need to make your callbacks thread-safe anyway, so it doesn't matter.

zagrodzki commented 6 years ago

FWIW, I think we could safely use only a single libusb callback for the entire gousb. There seems to be little purpose in having multiple, considering that we're not going to benefit a lot from native integration.

I really dislike this builder pattern. It already seems cleaner to write:

hotplug := &gousb.Hotplug{}
hotplug.OnArrive = func(....)
hotplug.OnLeave = func(...)
hotplug.Register()

and then you can of course boil it down to:

hotplug := &gousb.Hotplug{
  OnArrive: func(...),
  OnLeave: func(...),
}
hotplug.Register()

and at that point we're back to "we're cramming multiple functions with similar signatures inline into one body". None of that seems elegant.

I think I like this more:

type listener struct {... whatever data needed between arrive and leave ...}

func (l *listener) Enumerate(d *gousb.DeviceDesc, opener func() *gousb.Device, error) { ... }
func (l *listener) Arrive(d *gousb.DeviceDesc, opener func() *gousb.Device, error) { ... }
func (l *listener) Leave(d *gousb.DeviceDesc) { ... }

l := &listener{...}
gousb.NewHotplug(l)

No event types, simple registration, local data struct to hold references to devices. The signature of the opener is not super friendly, but it's similar to OpenDevices opener signature. The device matching logic needs to be repeated between methods, but any complex logic can be refactored to a separate method. Has some problems with one-shot scenarios, where you want to deregister after handling an event, that would require passing something (cancellation func, hotplug event, something) in the signature.

And compare to single function:

hotplug.Register(func(e HotplugEvent) {
  if e.Desc.Vendor != ... || e.Desc.Product != ... { return }
  switch e.Type {
  case HotplugArrive, HotplugEnumerate:
    ....
  case HotplugLeave:
    ....
   }
}

It's not that bad. I think I still like it the most...

kylelemons commented 6 years ago

I think I like the interface the best. You can provide a simple one-function wrapper object if you wanted, ala http.HandlerFunc. I would also suggest lumping enumerate and arrive together and simply not distinguish between the two. You can document in the API that callers who wish to enumerate devices can simply call OpenDevices with a noop callback (and probably provide an example to this effect).

nkovacs commented 6 years ago

The problem is callers who do not want to enumerate devices.

kylelemons commented 6 years ago

They don't have to call OpenDevices after registering the callback.

nkovacs commented 6 years ago

But if we lump arrive and enumerate together, they'll receive events for devices already plugged in. What you're saying requires that we remove enumerate, which means you lose the convenience. And if you do want to filter, you have to filter once in the callback and you have to write a separate opener function for OpenDevices.

zagrodzki commented 6 years ago

FTR, I don't particularly care either way in regards to including or leaving out enumeration. But I think it's better to distinguish between arrived and enumerated if we do include enumeration. Lumping them together is easy (just delegate between both methods).

nkovacs commented 6 years ago

I'm not saying we shouldn't be able to distinguish between the two, I just disagree with forcing this distinction on users who don't care about it. I only have my use case of course, and I can't really think of a case when you need to make a distinction except to ignore enumerated devices.

So I would prefer something like this:

type listener struct {... whatever data needed between arrive and leave ...}

func (l *listener) Arrive(e HotplugArriveEvent) {
    if e.Enumerated {
        return
    }
    device, err := e.Open()
    ...
}
func (l *listener) Leave(e HotplugLeaveEvent) { ... }

l := &listener{...}
gousb.NewHotplug(l)
bsakweson commented 5 years ago

Did anything ever come out of this? I happen to need this same feature on a project I am currently working. I definitely would not want to reinvent the will here and would rather use something out of this gousb package, especially because my current go code relies heavily on it. Any pointers on how to capture "hotplug" events will be highly appreciated. I notice this thread has been stale for some time now but I am still to get something reasonable out of it.

maitredede commented 3 years ago

Hi, Did someone reached a testable point in usb device hotplug ?

AkashicSeer commented 1 year ago

I was so looking forward to using Golang in my current project. One of the main requirements is USB support so I can connect to a 3d printer. However all of the packages/modules WTF ever go calls them require libusb or GCO and if I use GCO then I will have problems cross compiling my code. Cross compilation is requirement #2 I need my app to run on modern mobile devices as well as linux and windows. I read that the use of libusb means that the end user has to also install the library. That is requirement #3 down the shitter. Requirement #3 states that the user must be able to "just install" the app. We do not want to piss our end users off by having them have to perform some ludicrous ceremony of installing BS like you go through when trying to use anything Python.

This makes me question how many of the other libraries/frameworks etc. would also cause such issues making the end user have to install complicated BS. Requirement #5 is a web interface, so if we pick a framework that uses some C library then our whole project becomes a damn funhouse of working with C and Go. So why the hell don't we just use C then?

I couldn't believe Golang had no native support of USB devices. That makes no sense.

Gustavomurta commented 1 year ago

I was so looking forward to using Golang in my current project. One of the main requirements is USB support so I can connect to a 3d printer. However all of the packages/modules WTF ever go calls them require libusb or GCO and if I use GCO then I will have problems cross compiling my code. Cross compilation is requirement https://github.com/google/gousb/pull/2 I need my app to run on modern mobile devices as well as linux and windows. I read that the use of libusb means that the end user has to also install the library. That is requirement https://github.com/google/gousb/issues/3 down the shitter. Requirement https://github.com/google/gousb/issues/3 states that the user must be able to "just install" the app. We do not want to piss our end users off by having them have to perform some ludicrous ceremony of installing BS like you go through when trying to use anything Python.

This makes me question how many of the other libraries/frameworks etc. would also cause such issues making the end user have to install complicated BS. Requirement https://github.com/google/gousb/issues/5 is a web interface, so if we pick a framework that uses some C library then our whole project becomes a damn funhouse of working with C and Go. So why the hell don't we just use C then?

I couldn't believe Golang had no native support of USB devices. That makes no sense.

Hi AkashiSerr,

I'm not a programming expert, but I've been developing projects with Golang USB communication with Linux and Windows. It should be clarified that Golang programs are compiled and not interpreted like Python. As far as I know, you will need the libusb C++ Library during compilation. When running the compiled program, will it need Libusb too? I suggest you run tests.

AkashicSeer commented 1 year ago

Thanks. I am going by what I read in the documentation for the USB modules. I couldn't find any that didn't require the end user to have libusb installed on their system. I refuse to put my users through that. The very reason I am creating the app I am is because the existing apps are in python and require me to become an expert in Python to get them running. I don't want that to happen to my users.

From what I read in the modules, you have to have libusb installed to compile and then the end user needs it installed on their system to be able to use your code. And I would have to use GCO to compile it and then I'd have to deal with Go and C errors etc. It was all in the modules info. That isn't acceptable to me. I want a language that just works. C/C++ seem to be the only thing that will be portable without the user needing to install anything.

Maybe in a few more years Golang will have this worked out so that no matter what everything is 100% compiled in and the end user needs to do nothing else but install my program. I mean that is how it should work anyways. It is an issue with C's static linked vs dynamic linked libraries I believe if I remember correctly.

On Wed, Feb 22, 2023 at 6:25 AM Jose Gustavo Abreu Murta < @.***> wrote:

Eu estava ansioso para usar Golang em meu projeto atual. Um dos principais requisitos é o suporte USB para que eu possa me conectar a uma impressora 3D. No entanto, todos os pacotes/módulos que o WTF já chamou exigem libusb ou GCO e, se eu usar o GCO, terei problemas para compilar meu código. A compilação cruzada é o requisito nº 2. https://github.com/google/gousb/pull/2 Preciso que meu aplicativo seja executado em dispositivos móveis modernos, bem como Linux e Windows. Eu li que o uso de libusb significa que o usuário final também deve instalar a biblioteca. Esse é o requisito nº 3 https://github.com/google/gousb/issues/3 na merda. Requisito nº 3 https://github.com/google/gousb/issues/3afirma que o usuário deve ser capaz de "apenas instalar" o aplicativo. Não queremos irritar nossos usuários finais fazendo com que eles tenham que realizar alguma cerimônia ridícula de instalação de BS como você faz ao tentar usar qualquer coisa Python.

Isso me faz questionar quantas das outras bibliotecas/frameworks etc. também causariam tais problemas, fazendo com que o usuário final tivesse que instalar BS complicado. O requisito nº 5 https://github.com/google/gousb/issues/5 é uma interface da Web, portanto, se escolhermos uma estrutura que use alguma biblioteca C, todo o nosso projeto se tornará uma diversão para trabalhar com C e Go. Então, por que diabos não usamos apenas C?

Eu não podia acreditar que Golang não tinha suporte nativo para dispositivos USB. Isso não faz sentido.

Hi AkashiSerr,

I'm not a programming expert, but I've been developing projects with Golang USB communication with Linux and Windows. It should be clarified that Golang programs are compiled and not interpreted like Python. As far as I know, you will need the libusb C++ Library during compilation. When running the compiled program, will it need Libusb too? I suggest you run tests.

— Reply to this email directly, view it on GitHub https://github.com/google/gousb/issues/8#issuecomment-1439853997, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKYQZAXRLUSAOYHC2WXUU7LWYXZQZANCNFSM4DV2JHKQ . You are receiving this because you commented.Message ID: @.***>