Improve tests - use offsetof to find the position of the iso packet

descriptor in the transfer struct.

usb/endpoint.go

@@ -62,7 +62,7 @@ func (e *endpoint) transfer(buf []byte, timeout time.Duration) (int, error) {
 	}
 
 	tt := e.TransferType()
-	t, err := newUSBTransfer(e.Device.handle, e.EndpointInfo, buf, timeout)
+	t, err := newUSBTransfer((*deviceHandle)(e.Device.handle), e.EndpointInfo, buf, timeout)
 	if err != nil {
 		return 0, err
 	}

usb/transfer.go

@@ -23,11 +23,35 @@ int submit(struct libusb_transfer *xfer);
 import "C"
 
 import (
+	"errors"
 	"fmt"
+	"runtime"
+	"sync"
 	"time"
 	"unsafe"
 )
 
+// libusb hooks used as injection points for tests.
+var (
+	cCancel = func(t *libusbTransfer) usbError {
+		return usbError(C.libusb_cancel_transfer((*C.struct_libusb_transfer)(t)))
+	}
+	cSubmit = func(t *libusbTransfer) usbError {
+		return usbError(C.submit((*C.struct_libusb_transfer)(t)))
+	}
+)
+
+// because of a limitation of cgo, tests cannot import C.
+type deviceHandle C.libusb_device_handle
+type libusbTransfer C.struct_libusb_transfer
+type libusbIso C.struct_libusb_iso_packet_descriptor
+
+// also for tests
+var (
+	libusbIsoSize   = C.sizeof_struct_libusb_iso_packet_descriptor
+	libusbIsoOffset = unsafe.Offsetof(C.struct_libusb_transfer{}.iso_packet_desc)
+)
+
 //export xfer_callback
 func xfer_callback(cptr unsafe.Pointer) {
 	ch := *(*chan struct{})(cptr)
@@ -35,25 +59,35 @@ func xfer_callback(cptr unsafe.Pointer) {
 }
 
 type usbTransfer struct {
+	// mu protects the transfer state.
+	mu sync.Mutex
 	// xfer is the allocated libusb_transfer.
-	xfer *C.struct_libusb_transfer
+	xfer *libusbTransfer
 	// buf is the buffer allocated for the transfer. Both buf and xfer.buffer
 	// point to the same piece of memory.
 	buf []byte
 	// done is blocking until the transfer is complete and data and transfer
 	// status are available.
 	done chan struct{}
+	// submitted is true if this transfer was passed to libusb through submit()
+	submitted bool
 }
 
 // submits the transfer. After submit() the transfer is in flight and is owned by libusb.
 // It's not safe to access the contents of the transfer until wait() returns.
 // Once wait() returns, it's ok to re-use the same transfer structure by calling submit() again.
 func (t *usbTransfer) submit() error {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	if t.submitted {
+		return errors.New("transfer was already submitted and is not finished yet.")
+	}
 	t.done = make(chan struct{})
 	t.xfer.user_data = (unsafe.Pointer)(&t.done)
-	if errno := C.submit(t.xfer); errno < 0 {
-		return usbError(errno)
+	if err := cSubmit(t.xfer); err != SUCCESS {
+		return err
 	}
+	t.submitted = true
 	return nil
 }
 
@@ -63,29 +97,59 @@ func (t *usbTransfer) submit() error {
 // of the buffer were read or written by libusb, and it can be
 // smaller than the length of t.buf.
 func (t *usbTransfer) wait() (n int, err error) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	if !t.submitted {
+		return 0, nil
+	}
 	select {
 	case <-time.After(10 * time.Second):
 		return 0, fmt.Errorf("wait timed out after 10s")
 	case <-t.done:
 	}
+	t.submitted = false
 	var status TransferStatus
 	switch TransferType(t.xfer._type) {
 	case TRANSFER_TYPE_ISOCHRONOUS:
 		n = int(C.compact_iso_data(t.xfer, (*C.uchar)(unsafe.Pointer(&status))))
 	default:
-		n = int(t.xfer.length)
+		n = int(t.xfer.actual_length)
 		status = TransferStatus(t.xfer.status)
 	}
 	if status != LIBUSB_TRANSFER_COMPLETED {
-		return 0, status
+		return n, status
 	}
 	return n, err
 }
 
+// cancel aborts a submitted transfer. The transfer is cancelled
+// asynchronously and the user still needs to wait() to return.
+func (t *usbTransfer) cancel() error {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	if !t.submitted {
+		return nil
+	}
+	err := usbError(cCancel(t.xfer))
+	if err == ERROR_NOT_FOUND {
+		// transfer already completed
+		err = SUCCESS
+	}
+	if err != SUCCESS {
+		return err
+	}
+	return nil
+}
+
 // free releases the memory allocated for the transfer.
 // free should be called only if the transfer is not used by libusb,
 // i.e. it should not be called after submit() and before wait() returns.
 func (t *usbTransfer) free() error {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	if t.submitted {
+		return errors.New("free() cannot be called on a submitted transfer until wait() returns")
+	}
 	C.libusb_free_transfer(t.xfer)
 	t.xfer = nil
 	t.buf = nil
@@ -93,15 +157,16 @@ func (t *usbTransfer) free() error {
 	return nil
 }
 
-type deviceHandle *C.libusb_device_handle
-
 // newUSBTransfer allocates a new transfer structure for communication with a
 // given device/endpoint, with buf as the underlying transfer buffer.
-func newUSBTransfer(dev deviceHandle, ei EndpointInfo, buf []byte, timeout time.Duration) (*usbTransfer, error) {
+func newUSBTransfer(dev *deviceHandle, ei EndpointInfo, buf []byte, timeout time.Duration) (*usbTransfer, error) {
 	var isoPackets int
 	tt := ei.TransferType()
 	if tt == TRANSFER_TYPE_ISOCHRONOUS {
 		isoPackets = len(buf) / int(ei.MaxIsoPacket)
+		if int(ei.MaxIsoPacket)*isoPackets < len(buf) {
+			isoPackets++
+		}
 	}
 
 	xfer := C.libusb_alloc_transfer(C.int(isoPackets))
@@ -109,7 +174,7 @@ func newUSBTransfer(dev deviceHandle, ei EndpointInfo, buf []byte, timeout time.
 		return nil, fmt.Errorf("libusb_alloc_transfer(%d) failed", isoPackets)
 	}
 
-	xfer.dev_handle = dev
+	xfer.dev_handle = (*C.struct_libusb_device_handle)(dev)
 	xfer.timeout = C.uint(timeout / time.Millisecond)
 	xfer.endpoint = C.uchar(ei.Address)
 	xfer._type = C.uchar(tt)
@@ -122,8 +187,14 @@ func newUSBTransfer(dev deviceHandle, ei EndpointInfo, buf []byte, timeout time.
 		C.libusb_set_iso_packet_lengths(xfer, C.uint(ei.MaxIsoPacket))
 	}
 
-	return &usbTransfer{
-		xfer: xfer,
+	t := &usbTransfer{
+		xfer: (*libusbTransfer)(xfer),
 		buf:  buf,
-	}, nil
+	}
+	runtime.SetFinalizer(t, func(t *usbTransfer) {
+		t.cancel()
+		t.wait()
+		t.free()
+	})
+	return t, nil
 }

usb/transfer_fakelibusb_test.go

@@ -0,0 +1,56 @@
+// Copyright 2017 the gousb Authors.  All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package usb
+
+import "sync"
+
+type fakeLibusb struct {
+	sync.Mutex
+	ts map[*libusbTransfer]chan struct{}
+}
+
+func (f *fakeLibusb) submit(t *libusbTransfer) usbError {
+	f.Lock()
+	defer f.Unlock()
+	if f.ts[t] == nil {
+		f.ts[t] = make(chan struct{})
+	}
+	close(f.ts[t])
+	return SUCCESS
+}
+
+func (f *fakeLibusb) cancel(t *libusbTransfer) usbError { return SUCCESS }
+
+func (f *fakeLibusb) waitForSubmit(t *usbTransfer) {
+	f.Lock()
+	if f.ts[t.xfer] == nil {
+		f.ts[t.xfer] = make(chan struct{})
+	}
+	ch := f.ts[t.xfer]
+	f.Unlock()
+	<-ch
+}
+
+func (f *fakeLibusb) runCallback(t *usbTransfer, cb func(*usbTransfer)) {
+	f.Lock()
+	defer f.Unlock()
+	delete(f.ts, t.xfer)
+	cb(t)
+	close(t.done)
+}
+
+func newFakeLibusb() *fakeLibusb {
+	return &fakeLibusb{ts: make(map[*libusbTransfer]chan struct{})}
+}

usb/transfer_test.go

@@ -0,0 +1,230 @@
+// Copyright 2017 the gousb Authors.  All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package usb
+
+import (
+	"testing"
+	"time"
+	"unsafe"
+)
+
+func TestNewTransfer(t *testing.T) {
+	for _, tc := range []struct {
+		desc        string
+		dir         EndpointDirection
+		tt          TransferType
+		maxPkt      uint16
+		maxIso      uint32
+		buf         int
+		timeout     time.Duration
+		wantIso     int
+		wantLength  int
+		wantTimeout int
+	}{
+		{
+			desc:        "bulk in transfer, 512B packets",
+			dir:         ENDPOINT_DIR_IN,
+			tt:          TRANSFER_TYPE_BULK,
+			maxPkt:      512,
+			buf:         1024,
+			timeout:     time.Second,
+			wantIso:     0,
+			wantLength:  1024,
+			wantTimeout: 1000,
+		},
+		{
+			desc:        "iso out transfer, 3 * 1024B packets",
+			dir:         ENDPOINT_DIR_OUT,
+			tt:          TRANSFER_TYPE_ISOCHRONOUS,
+			maxPkt:      2<<11 + 1024,
+			maxIso:      3 * 1024,
+			buf:         10000,
+			timeout:     500 * time.Millisecond,
+			wantIso:     4,
+			wantLength:  10000,
+			wantTimeout: 500,
+		},
+	} {
+		xfer, err := newUSBTransfer(nil, EndpointInfo{
+			Address:       uint8(tc.dir) | 0x02,
+			Attributes:    uint8(tc.tt),
+			MaxPacketSize: tc.maxPkt,
+			MaxIsoPacket:  tc.maxIso,
+			PollInterval:  1,
+		}, make([]byte, tc.buf), tc.timeout)
+
+		if err != nil {
+			t.Fatalf("newUSBTransfer(): %v", err)
+		}
+		if got, want := len(xfer.buf), tc.buf; got != want {
+			t.Errorf("xfer.buf: got %d bytes, want %d", got, want)
+		}
+		if got, want := int(xfer.xfer.length), tc.buf; got != want {
+			t.Errorf("xfer.length: got %d, want %d", got, want)
+		}
+		if got, want := int(xfer.xfer.num_iso_packets), tc.wantIso; got != want {
+			t.Errorf("xfer.num_iso_packets: got %d, want %d", got, want)
+		}
+		if got, want := int(xfer.xfer.timeout), tc.wantTimeout; got != want {
+			t.Errorf("xfer.timeout: got %d ms, want %d", got, want)
+		}
+		if got, want := TransferType(xfer.xfer._type), tc.tt; got != want {
+			t.Errorf("xfer._type: got %s, want %s", got, want)
+		}
+	}
+}
+
+func TestTransferProtocol(t *testing.T) {
+	origSubmit, origCancel := cSubmit, cCancel
+	defer func() { cSubmit, cCancel = origSubmit, origCancel }()
+
+	f := newFakeLibusb()
+	cSubmit = f.submit
+	cCancel = f.cancel
+
+	xfers := make([]*usbTransfer, 2)
+	var err error
+	for i := 0; i < 2; i++ {
+		xfers[i], err = newUSBTransfer(nil, EndpointInfo{
+			Address:       0x86,
+			Attributes:    uint8(TRANSFER_TYPE_BULK),
+			MaxPacketSize: 512,
+			PollInterval:  1,
+		}, make([]byte, 10240), time.Second)
+		if err != nil {
+			t.Fatalf("newUSBTransfer: %v", err)
+		}
+	}
+
+	go func() {
+		f.waitForSubmit(xfers[0])
+		f.runCallback(xfers[0], func(t *usbTransfer) {
+			t.xfer.actual_length = 5
+			t.xfer.status = uint32(SUCCESS)
+			copy(t.buf, []byte{1, 2, 3, 4, 5})
+		})
+	}()
+	go func() {
+		f.waitForSubmit(xfers[1])
+		f.runCallback(xfers[1], func(t *usbTransfer) {
+			t.xfer.actual_length = 99
+			t.xfer.status = uint32(SUCCESS)
+			copy(t.buf, []byte{12, 12, 12, 12, 12})
+		})
+	}()
+
+	xfers[1].submit()
+	xfers[0].submit()
+	got, err := xfers[0].wait()
+	if err != nil {
+		t.Errorf("xfer#0.wait returned error %v, want nil", err)
+	}
+	if want := 5; got != want {
+		t.Errorf("xfer#0.wait returned %d bytes, want %d", got, want)
+	}
+	got, err = xfers[1].wait()
+	if err != nil {
+		t.Errorf("xfer#0.wait returned error %v, want nil", err)
+	}
+	if want := 99; got != want {
+		t.Errorf("xfer#0.wait returned %d bytes, want %d", got, want)
+	}
+
+	go func() {
+		f.waitForSubmit(xfers[1])
+		f.runCallback(xfers[1], func(t *usbTransfer) {
+			t.xfer.actual_length = 123
+			t.xfer.status = uint32(LIBUSB_TRANSFER_CANCELLED)
+		})
+	}()
+	xfers[1].submit()
+	xfers[1].cancel()
+	got, err = xfers[1].wait()
+	if err == nil {
+		t.Error("xfer#1(resubmitted).wait returned error nil, want non-nil")
+	}
+	if want := 123; got != want {
+		t.Errorf("xfer#1(resubmitted).wait returned %d bytes, want %d", got, want)
+	}
+
+	for i := 0; i < 2; i++ {
+		xfers[i].cancel()
+		xfers[i].wait()
+		xfers[i].free()
+	}
+}
+
+func TestIsoPackets(t *testing.T) {
+	origSubmit, origCancel := cSubmit, cCancel
+	defer func() { cSubmit, cCancel = origSubmit, origCancel }()
+
+	f := newFakeLibusb()
+	cSubmit = f.submit
+	cCancel = f.cancel
+
+	xfer, err := newUSBTransfer(nil, EndpointInfo{
+		Address:       0x82,
+		Attributes:    uint8(TRANSFER_TYPE_ISOCHRONOUS),
+		MaxPacketSize: 3<<11 + 1024,
+		MaxIsoPacket:  3 * 1024,
+		PollInterval:  1,
+	}, make([]byte, 15000), time.Second)
+	if err != nil {
+		t.Fatalf("newUSBTransfer: %v", err)
+	}
+
+	// 15000 / (3*1024) = 4.something, rounded up to 5
+	if got, want := int(xfer.xfer.num_iso_packets), 5; got != want {
+		t.Fatalf("newUSBTransfer: got %d iso packets, want %d", got, want)
+	}
+
+	go func() {
+		f.waitForSubmit(xfer)
+		f.runCallback(xfer, func(x *usbTransfer) {
+			x.xfer.actual_length = 1234 // meaningless for iso transfers
+			x.xfer.status = uint32(LIBUSB_TRANSFER_TIMED_OUT)
+			for i := 0; i < int(xfer.xfer.num_iso_packets); i++ {
+				// this is a horrible calculation.
+				// libusb_transfer uses a flexible array for the iso packet
+				// descriptors at the end of the transfer struct.
+				// The only way to get access to the elements of that array
+				// is to use pointer arithmetic.
+				// Calculate the offset of the first descriptor in the struct,
+				// then move by sizeof(iso descriptor) for num_iso_packets.
+				desc := (*libusbIso)(unsafe.Pointer(uintptr(unsafe.Pointer(x.xfer)) + libusbIsoOffset + uintptr(i*libusbIsoSize)))
+				// max iso packet = 3 * 1024
+				if desc.length != 3*1024 {
+					t.Errorf("iso pkt length: got %d, want %d", desc.length, 3*1024)
+				}
+				desc.actual_length = 100
+				// packets 0..2 are successful, packet 3 is timed out
+				if i != 4 {
+					desc.status = uint32(LIBUSB_TRANSFER_COMPLETED)
+				} else {
+					desc.status = uint32(LIBUSB_TRANSFER_TIMED_OUT)
+				}
+			}
+		})
+	}()
+
+	xfer.submit()
+	got, err := xfer.wait()
+	if err == nil {
+		t.Error("Iso transfer: got nil error, want non-nil")
+	}
+	if want := 4 * 100; got != want {
+		t.Errorf("Iso transfer: got %d bytes, want %d", got, want)
+	}
+}