Add functions to achieve android support #126
Conversation
usb.go
Outdated
| @@ -210,6 +211,41 @@ func (c *Context) OpenDevices(opener func(desc *DeviceDesc) bool) ([]*Device, er | |||
| return ret, reterr | |||
| } | |||
|
|
|||
| func (c *Context) OpenDeviceWithFileDescriptor(fileDescriptor string) (*Device, error) { | |||
There was a problem hiding this comment.
it's not a descriptor, it's a device path, right?
OpenDevicePath(path string)
may be better.
There was a problem hiding this comment.
It's actually a file descriptor! It's a bit 'funny', but what you get from the Android API is this: https://developer.android.com/reference/android/hardware/usb/UsbDeviceConnection#getFileDescriptor()
usb.go
Outdated
| func (c *Context) OpenDeviceWithFileDescriptor(fileDescriptor string) (*Device, error) { | ||
| fd, err := syscall.Open(fileDescriptor, syscall.O_RDWR, 0) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
annotate this error to indicate that it came from Open(path)
| if device == nil { | ||
| return nil, fmt.Errorf("libusb_get_device failed") | ||
| } |
There was a problem hiding this comment.
I don't think you need to handle this case here. As far as I can tell, the only thing libusb_get_device does is return the device that is already stored in the handle. I don't think it can be nil, if obtaining the handle didn't return an error.
| return (*libusbDevHandle)(handle), nil | ||
| } | ||
|
|
||
| func (libusbImpl) getDevice(d *libusbDevHandle) (*libusbDevice, error) { |
There was a problem hiding this comment.
libusb_get_device doesn't return errors, so this should probably be just a trivial wrapper:
func (...) getDevice(*d libusbDevHandle) *libusbDevice {
return C.lbusb_get_device(...)
}
| @@ -133,7 +140,7 @@ func (ep libusbEndpoint) endpointDesc(dev *DeviceDesc) EndpointDesc { | |||
| // and occasionally on convenience data types (like TransferType or DeviceDesc). | |||
| type libusbIntf interface { | |||
| // context | |||
| init() (*libusbContext, error) | |||
| init(flags ...libusbOpt) (*libusbContext, error) | |||
There was a problem hiding this comment.
I don't think this is the best approach. Keep the init() function as is. Instead, store the options inside the libusbImpl struct.
| for _, flag := range flags { | ||
| switch flag { | ||
| case LIBUSB_OPTION_NO_DEVICE_DISCOVERY: | ||
| C.gousb_disable_device_discovery(ctx) |
There was a problem hiding this comment.
this call can return an error, you need to add fromErrNo(...) wrapper around it and check for nil
| type libusbOpt int | ||
|
|
||
| const ( | ||
| LIBUSB_OPTION_NO_DEVICE_DISCOVERY libusbOpt = iota |
There was a problem hiding this comment.
I don't believe you've addressed the user-visible portion of the API, essentially passing the options to NewContext()
I'd suggest:
type ContextOptions struct {
DisableDeviceDiscovery bool
}
func (o *ContextOptions) NewContext() (*Context, error) { ... }
| } | ||
|
|
||
| func (f *fakeLibusb) getDevice(d *libusbDevHandle) (*libusbDevice, error) { | ||
| //TODO should we do something for this |
| @@ -210,6 +210,36 @@ func (c *Context) OpenDevices(opener func(desc *DeviceDesc) bool) ([]*Device, er | |||
| return ret, reterr | |||
| } | |||
|
|
|||
| func (c *Context) OpenDeviceWithFileDescriptor(fd uintptr) (*Device, error) { | |||
There was a problem hiding this comment.
wouldn't it be better to open a device from a system path? Open() can happen inside the function (note that Open will need to become a method on the libusb interface, so that it can be faked).
Unless you know of an example where this wouldn't be sufficient?
| @@ -108,6 +110,16 @@ func (f *fakeLibusb) getDevices(*libusbContext) ([]*libusbDevice, error) { | |||
| return ret, nil | |||
| } | |||
|
|
|||
| func (f *fakeLibusb) wrapSysDevice(ctx *libusbContext, systemDeviceHandle uintptr) (*libusbDevHandle, error) { | |||
| //TODO should we do something for this | |||
There was a problem hiding this comment.
add a new field to the fakelibusb, "fileDescriptors", which should map int descriptor numbers to *libusbDevice (then the device can be pulled from f.fakeDevices map) or to *fakeDevice directly.
You can populate the keys of fileDescriptors by reusing slice indices from fakeDevices (from fakelibusb_devices.go), pointing to the same list of devices.
…recommended libusb context creation method
|
I've adjusted the changes according to the recommendations and fixed the test to use the fake implementation. Thx! |
A new PR is the best way to go, I think. |
|
Super! See here: #130 |
|
closing this in favor of #130 |
Since Android has limitations for enumerating devices (see), it would be useful to have other ways of opening devices.
This PR adds a new function to open a device provided its file descriptor, along with it also allows the user to set the
LIBUSB_OPTION_NO_DEVICE_DISCOVERY, again, to try to overcome the Android limitations.This PR is missing some work on the tests, plus I'm not sure if the way I modified the API follows your standards, please let me know.