From a6e284610b9937b8d71036a0ca786f8ca64b8fcc Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Sun, 5 Feb 2017 04:16:57 +0100 Subject: [PATCH 1/8] Use the calculated max iso packet size. --- usb/iso.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/usb/iso.go b/usb/iso.go index 3be4152..414ee9b 100644 --- a/usb/iso.go +++ b/usb/iso.go @@ -37,10 +37,8 @@ func iso_callback(cptr unsafe.Pointer) { } func (end *endpoint) allocTransfer() *Transfer { - // Use libusb_get_max_iso_packet_size ? const ( - iso_packets = 8 // 128 // 242 - packet_size = 2 * 960 // 1760 + iso_packets = 8 // 128 // 242 ) xfer := C.libusb_alloc_transfer(C.int(iso_packets)) @@ -48,11 +46,22 @@ func (end *endpoint) allocTransfer() *Transfer { log.Printf("usb: transfer allocation failed?!") return nil } + handle := end.Device.handle + dev := C.libusb_get_device(handle) + packet_size := C.libusb_get_max_iso_packet_size(dev, C.uchar(end.Address)) + switch packet_size { + case C.LIBUSB_ERROR_NOT_FOUND: + log.Printf("usb: get_max_iso_packet_size: endpoint does not exist") + return nil + case C.LIBUSB_ERROR_OTHER: + log.Printf("usb: get_max_iso_packet_size: unknown failure") + return nil + } buf := make([]byte, iso_packets*packet_size) done := make(chan struct{}, 1) - xfer.dev_handle = end.Device.handle + xfer.dev_handle = handle xfer.endpoint = C.uchar(end.Address) xfer._type = C.LIBUSB_TRANSFER_TYPE_ISOCHRONOUS @@ -60,7 +69,7 @@ func (end *endpoint) allocTransfer() *Transfer { xfer.length = C.int(len(buf)) xfer.num_iso_packets = iso_packets - C.libusb_set_iso_packet_lengths(xfer, packet_size) + C.libusb_set_iso_packet_lengths(xfer, C.uint(packet_size)) /* pkts := *(*[]C.struct_libusb_packet_descriptor)(unsafe.Pointer(&reflect.SliceHeader{ Data: uintptr(unsafe.Pointer(&xfer.iso_packet_desc)), From 4319ef2cc2e2aab110da70e564dac833531b7cad Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Sun, 5 Feb 2017 16:00:53 +0100 Subject: [PATCH 2/8] Revert "Use the calculated max iso packet size." The libusb_get_max_iso_packet_size ignores the endpoint information and will return the same size for all endpoints, even if the current alternative configuration does not support given size. This reverts commit a6e284610b9937b8d71036a0ca786f8ca64b8fcc. --- usb/iso.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/usb/iso.go b/usb/iso.go index 414ee9b..3be4152 100644 --- a/usb/iso.go +++ b/usb/iso.go @@ -37,8 +37,10 @@ func iso_callback(cptr unsafe.Pointer) { } func (end *endpoint) allocTransfer() *Transfer { + // Use libusb_get_max_iso_packet_size ? const ( - iso_packets = 8 // 128 // 242 + iso_packets = 8 // 128 // 242 + packet_size = 2 * 960 // 1760 ) xfer := C.libusb_alloc_transfer(C.int(iso_packets)) @@ -46,22 +48,11 @@ func (end *endpoint) allocTransfer() *Transfer { log.Printf("usb: transfer allocation failed?!") return nil } - handle := end.Device.handle - dev := C.libusb_get_device(handle) - packet_size := C.libusb_get_max_iso_packet_size(dev, C.uchar(end.Address)) - switch packet_size { - case C.LIBUSB_ERROR_NOT_FOUND: - log.Printf("usb: get_max_iso_packet_size: endpoint does not exist") - return nil - case C.LIBUSB_ERROR_OTHER: - log.Printf("usb: get_max_iso_packet_size: unknown failure") - return nil - } buf := make([]byte, iso_packets*packet_size) done := make(chan struct{}, 1) - xfer.dev_handle = handle + xfer.dev_handle = end.Device.handle xfer.endpoint = C.uchar(end.Address) xfer._type = C.LIBUSB_TRANSFER_TYPE_ISOCHRONOUS @@ -69,7 +60,7 @@ func (end *endpoint) allocTransfer() *Transfer { xfer.length = C.int(len(buf)) xfer.num_iso_packets = iso_packets - C.libusb_set_iso_packet_lengths(xfer, C.uint(packet_size)) + C.libusb_set_iso_packet_lengths(xfer, packet_size) /* pkts := *(*[]C.struct_libusb_packet_descriptor)(unsafe.Pointer(&reflect.SliceHeader{ Data: uintptr(unsafe.Pointer(&xfer.iso_packet_desc)), From f13728c6e156f79f3b9286bcca9f3dcb4a22a285 Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Sun, 5 Feb 2017 16:20:58 +0100 Subject: [PATCH 3/8] Set the MaxIsoPacket field in the endpoint info if the endpoint is an isochronous endpoint. Use MaxIsoPacket as the iso packet size when preparing the iso transfer. --- usb/config.go | 19 +++++++++++++------ usb/iso.go | 8 +++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/usb/config.go b/usb/config.go index 9571174..62177e0 100644 --- a/usb/config.go +++ b/usb/config.go @@ -126,15 +126,22 @@ func newConfig(dev *C.libusb_device, cfg *C.struct_libusb_config_descriptor) Con } i.Endpoints = make([]EndpointInfo, 0, len(ends)) for _, end := range ends { - i.Endpoints = append(i.Endpoints, EndpointInfo{ + ei := EndpointInfo{ Address: uint8(end.bEndpointAddress), Attributes: uint8(end.bmAttributes), MaxPacketSize: uint16(end.wMaxPacketSize), - //MaxIsoPacket: uint32(C.libusb_get_max_iso_packet_size(dev, C.uchar(end.bEndpointAddress))), - PollInterval: uint8(end.bInterval), - RefreshRate: uint8(end.bRefresh), - SynchAddress: uint8(end.bSynchAddress), - }) + PollInterval: uint8(end.bInterval), + RefreshRate: uint8(end.bRefresh), + SynchAddress: uint8(end.bSynchAddress), + } + if TransferType(ei.Attributes)&TRANSFER_TYPE_MASK == TRANSFER_TYPE_ISOCHRONOUS { + // bits 0-10 identify the packet size, bits 11-12 are the number of additional transactions per microframe. + // Don't use libusb_get_max_iso_packet_size, as it has a bug where it returns the same value + // regardless of alternative setting used, where different alternative settings might define different + // max packet sizes. + ei.MaxIsoPacket = uint32(end.wMaxPacketSize) & 0x07ff * (uint32(end.wMaxPacketSize)>>11&3 + 1) + } + i.Endpoints = append(i.Endpoints, ei) } descs = append(descs, i) } diff --git a/usb/iso.go b/usb/iso.go index 3be4152..1044de3 100644 --- a/usb/iso.go +++ b/usb/iso.go @@ -37,10 +37,8 @@ func iso_callback(cptr unsafe.Pointer) { } func (end *endpoint) allocTransfer() *Transfer { - // Use libusb_get_max_iso_packet_size ? const ( - iso_packets = 8 // 128 // 242 - packet_size = 2 * 960 // 1760 + iso_packets = 8 // 128 // 242 ) xfer := C.libusb_alloc_transfer(C.int(iso_packets)) @@ -49,7 +47,7 @@ func (end *endpoint) allocTransfer() *Transfer { return nil } - buf := make([]byte, iso_packets*packet_size) + buf := make([]byte, iso_packets*end.EndpointInfo.MaxIsoPacket) done := make(chan struct{}, 1) xfer.dev_handle = end.Device.handle @@ -60,7 +58,7 @@ func (end *endpoint) allocTransfer() *Transfer { xfer.length = C.int(len(buf)) xfer.num_iso_packets = iso_packets - C.libusb_set_iso_packet_lengths(xfer, packet_size) + C.libusb_set_iso_packet_lengths(xfer, C.uint(end.EndpointInfo.MaxIsoPacket)) /* pkts := *(*[]C.struct_libusb_packet_descriptor)(unsafe.Pointer(&reflect.SliceHeader{ Data: uintptr(unsafe.Pointer(&xfer.iso_packet_desc)), From 4d60ebb06557d412615e521ef3872b664645c8bb Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Sun, 5 Feb 2017 16:29:22 +0100 Subject: [PATCH 4/8] Move usb_test to a separate package, in line with opensource Go expectations. Multiple packages per directory are not supported in this realm ;) And with a separate package, the dot import seems unnecessary. --- {usb => usb_test}/usb_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) rename {usb => usb_test}/usb_test.go (87%) diff --git a/usb/usb_test.go b/usb_test/usb_test.go similarity index 87% rename from usb/usb_test.go rename to usb_test/usb_test.go index cc83b9f..0b678fe 100644 --- a/usb/usb_test.go +++ b/usb_test/usb_test.go @@ -20,22 +20,22 @@ import ( "os" "testing" - . "github.com/kylelemons/gousb/usb" + "github.com/kylelemons/gousb/usb" "github.com/kylelemons/gousb/usbid" ) func TestNoop(t *testing.T) { - c := NewContext() + c := usb.NewContext() defer c.Close() c.Debug(0) } func TestEnum(t *testing.T) { - c := NewContext() + c := usb.NewContext() defer c.Close() c.Debug(0) - logDevice := func(t *testing.T, desc *Descriptor) { + logDevice := func(t *testing.T, desc *usb.Descriptor) { t.Logf("%03d.%03d %s", desc.Bus, desc.Address, usbid.Describe(desc)) t.Logf("- Protocol: %s", usbid.Classify(desc)) @@ -55,8 +55,8 @@ func TestEnum(t *testing.T) { } } - descs := []*Descriptor{} - devs, err := c.ListDevices(func(desc *Descriptor) bool { + descs := []*usb.Descriptor{} + devs, err := c.ListDevices(func(desc *usb.Descriptor) bool { logDevice(t, desc) descs = append(descs, desc) return true @@ -82,12 +82,12 @@ func TestEnum(t *testing.T) { } func TestOpenDeviceWithVidPid(t *testing.T) { - c := NewContext() + c := usb.NewContext() defer c.Close() c.Debug(0) // Accept for all device - devs, err := c.ListDevices(func(desc *Descriptor) bool { + devs, err := c.ListDevices(func(desc *usb.Descriptor) bool { return true }) defer func() { @@ -125,8 +125,8 @@ func TestMultipleContexts(t *testing.T) { var buf bytes.Buffer log.SetOutput(&buf) for i := 0; i < 2; i++ { - ctx := NewContext() - _, err := ctx.ListDevices(func(desc *Descriptor) bool { + ctx := usb.NewContext() + _, err := ctx.ListDevices(func(desc *usb.Descriptor) bool { return false }) if err != nil { From c1b87403fa9c85df9f1605fde0a3dea017cc409c Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Sun, 5 Feb 2017 16:46:56 +0100 Subject: [PATCH 5/8] Ditto for device_test, move to usb_test package. --- {usb => usb_test}/device_test.go | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {usb => usb_test}/device_test.go (100%) diff --git a/usb/device_test.go b/usb_test/device_test.go similarity index 100% rename from usb/device_test.go rename to usb_test/device_test.go From 9849c8088e55314c3be919de73ac7b84c669554a Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Sun, 5 Feb 2017 16:49:02 +0100 Subject: [PATCH 6/8] Add a reference to libusb ticket on incorrect max iso packet size. --- usb/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/usb/config.go b/usb/config.go index 62177e0..72e3795 100644 --- a/usb/config.go +++ b/usb/config.go @@ -139,6 +139,7 @@ func newConfig(dev *C.libusb_device, cfg *C.struct_libusb_config_descriptor) Con // Don't use libusb_get_max_iso_packet_size, as it has a bug where it returns the same value // regardless of alternative setting used, where different alternative settings might define different // max packet sizes. + // See http://libusb.org/ticket/77 for more background. ei.MaxIsoPacket = uint32(end.wMaxPacketSize) & 0x07ff * (uint32(end.wMaxPacketSize)>>11&3 + 1) } i.Endpoints = append(i.Endpoints, ei) From aafad620e6201b41f94d7cfa16df810f98dcb680 Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Mon, 6 Feb 2017 20:26:23 +0100 Subject: [PATCH 7/8] Revert "Move usb_test to a separate package, in line with opensource Go" This reverts commit 4d60ebb06557d412615e521ef3872b664645c8bb. --- {usb_test => usb}/usb_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) rename {usb_test => usb}/usb_test.go (87%) diff --git a/usb_test/usb_test.go b/usb/usb_test.go similarity index 87% rename from usb_test/usb_test.go rename to usb/usb_test.go index 0b678fe..cc83b9f 100644 --- a/usb_test/usb_test.go +++ b/usb/usb_test.go @@ -20,22 +20,22 @@ import ( "os" "testing" - "github.com/kylelemons/gousb/usb" + . "github.com/kylelemons/gousb/usb" "github.com/kylelemons/gousb/usbid" ) func TestNoop(t *testing.T) { - c := usb.NewContext() + c := NewContext() defer c.Close() c.Debug(0) } func TestEnum(t *testing.T) { - c := usb.NewContext() + c := NewContext() defer c.Close() c.Debug(0) - logDevice := func(t *testing.T, desc *usb.Descriptor) { + logDevice := func(t *testing.T, desc *Descriptor) { t.Logf("%03d.%03d %s", desc.Bus, desc.Address, usbid.Describe(desc)) t.Logf("- Protocol: %s", usbid.Classify(desc)) @@ -55,8 +55,8 @@ func TestEnum(t *testing.T) { } } - descs := []*usb.Descriptor{} - devs, err := c.ListDevices(func(desc *usb.Descriptor) bool { + descs := []*Descriptor{} + devs, err := c.ListDevices(func(desc *Descriptor) bool { logDevice(t, desc) descs = append(descs, desc) return true @@ -82,12 +82,12 @@ func TestEnum(t *testing.T) { } func TestOpenDeviceWithVidPid(t *testing.T) { - c := usb.NewContext() + c := NewContext() defer c.Close() c.Debug(0) // Accept for all device - devs, err := c.ListDevices(func(desc *usb.Descriptor) bool { + devs, err := c.ListDevices(func(desc *Descriptor) bool { return true }) defer func() { @@ -125,8 +125,8 @@ func TestMultipleContexts(t *testing.T) { var buf bytes.Buffer log.SetOutput(&buf) for i := 0; i < 2; i++ { - ctx := usb.NewContext() - _, err := ctx.ListDevices(func(desc *usb.Descriptor) bool { + ctx := NewContext() + _, err := ctx.ListDevices(func(desc *Descriptor) bool { return false }) if err != nil { From 397a363de106781e235c5f76d79966f7dd1d9bff Mon Sep 17 00:00:00 2001 From: Sebastian Zagrodzki Date: Mon, 6 Feb 2017 20:26:32 +0100 Subject: [PATCH 8/8] Revert "Ditto for device_test, move to usb_test package." This reverts commit c1b87403fa9c85df9f1605fde0a3dea017cc409c. --- {usb_test => usb}/device_test.go | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {usb_test => usb}/device_test.go (100%) diff --git a/usb_test/device_test.go b/usb/device_test.go similarity index 100% rename from usb_test/device_test.go rename to usb/device_test.go