Don't skip empty transfers. In isochronous transfers, make sure (#72)

Don't skip empty transfers.
Update the fake libusb to honor the endpoint max packet sizes.
Update tests that were taking advantage of the fact that libusb
allowed unlimited amount of data in any transfer packet.
This commit is contained in:
Sebastian Zagrodzki
2019-08-12 21:38:32 +02:00
committed by GitHub
parent 2dc560e6be
commit 18f4c1d8a7
6 changed files with 68 additions and 28 deletions

View File

@@ -86,10 +86,6 @@ func (e *endpoint) String() string {
}
func (e *endpoint) transfer(ctx context.Context, buf []byte) (int, error) {
if len(buf) == 0 {
return 0, nil
}
t, err := newUSBTransfer(e.ctx, e.h, &e.Desc, len(buf))
if err != nil {
return 0, err

View File

@@ -70,7 +70,7 @@ func TestEndpointReadStream(t *testing.T) {
}
defer stream.Close()
var got int
want := goodTransfers * 1024
want := goodTransfers * 512 // EP max packet size is 512
buf := make([]byte, 1024)
for got <= want {
num, err := stream.Read(buf)
@@ -130,31 +130,30 @@ func TestEndpointWriteStream(t *testing.T) {
if err != nil {
t.Fatalf("%s.Endpoint(1): %v", intf, err)
}
pktSize := 1024
stream, err := ep.NewStream(pktSize, 5)
bufSize := 1024
stream, err := ep.NewStream(bufSize, 5)
if err != nil {
t.Fatalf("%s.NewStream(%d, 5): %v", ep, pktSize, err)
t.Fatalf("%s.NewStream(%d, 5): %v", ep, bufSize, err)
}
defer stream.Close()
for i := 0; i < 5; i++ {
if n, err := stream.Write(make([]byte, pktSize*2)); err != nil {
if n, err := stream.Write(make([]byte, bufSize*2)); err != nil {
t.Fatalf("stream.Write: got error %v", err)
} else if n != pktSize*2 {
t.Fatalf("stream.Write: %d, want %d", n, pktSize*2)
} else if n != bufSize*2 {
t.Fatalf("stream.Write: %d, want %d", n, bufSize*2)
}
}
want := pktSize * 10
if err := stream.Close(); err != nil {
t.Fatalf("stream.Close: got error %v", err)
}
if got := stream.Written(); got != want {
if got, want := stream.Written(), 10240; got != want { // 5 stream.Writes, each with 2048 bytes
t.Errorf("stream.Written: got %d, want %d", got, want)
}
done <- struct{}{}
if num != 10 {
t.Errorf("received transfers: got %d, want %d", num, 10)
if w := stream.Written(); total != w {
t.Errorf("received data: got %d, but stream.Written returned %d, results should be identical", total, w)
}
if total != want {
t.Errorf("received data: got %d, want %d", total, want)
if wantXfers := 20; num != wantXfers { // transferred 10240 bytes, device max packet size is 512
t.Errorf("received transfers: got %d, want %d", num, wantXfers)
}
}

View File

@@ -76,9 +76,9 @@ func TestEndpoint(t *testing.T) {
}{
{
desc: "empty buffer",
buf: nil,
ret: 10,
wantSubmit: false,
buf: []byte{},
ret: 0,
wantSubmit: true,
want: 0,
},
{

View File

@@ -38,6 +38,12 @@ type fakeTransfer struct {
// length is the number of bytes used from the buffer (write) or available
// in the buffer (read).
length int
// ep is the endpoint that this transfer was created for.
ep *EndpointDesc
// isoPackets is the number of isochronous transfers performed in a single libusb transfer
isoPackets int
// maxLength is the maximum number of bytes this transfer could contain
maxLength int
}
func (t *fakeTransfer) setData(d []byte) {
@@ -199,11 +205,27 @@ func (f *fakeLibusb) setAlt(d *libusbDevHandle, intf, alt uint8) error {
return nil
}
func (f *fakeLibusb) alloc(_ *libusbDevHandle, _ *EndpointDesc, _ int, bufLen int, done chan struct{}) (*libusbTransfer, error) {
func (f *fakeLibusb) alloc(_ *libusbDevHandle, ep *EndpointDesc, isoPackets int, bufLen int, done chan struct{}) (*libusbTransfer, error) {
f.mu.Lock()
defer f.mu.Unlock()
maxLen := ep.MaxPacketSize
if isoPackets > 0 {
if ep.TransferType != TransferTypeIsochronous {
return nil, fmt.Errorf("alloc(..., ep: %s, isoPackets: %d, ...): endpoint is not an isochronous type endpoint, iso packets must be 0", ep, isoPackets)
}
maxLen = isoPackets * ep.MaxPacketSize
}
if bufLen > maxLen {
bufLen = maxLen
}
t := newFakeTransferPointer()
f.ts[t] = &fakeTransfer{buf: make([]byte, bufLen), done: done}
f.ts[t] = &fakeTransfer{
buf: make([]byte, bufLen),
ep: ep,
isoPackets: isoPackets,
maxLength: maxLen,
done: done,
}
return t, nil
}
func (f *fakeLibusb) cancel(t *libusbTransfer) error {
@@ -225,17 +247,28 @@ func (f *fakeLibusb) buffer(t *libusbTransfer) []byte { return f.ts[t].buf }
func (f *fakeLibusb) data(t *libusbTransfer) (int, TransferStatus) {
f.mu.Lock()
defer f.mu.Unlock()
return f.ts[t].length, f.ts[t].status
ret := f.ts[t].length
if maxRet := f.ts[t].maxLength; ret > maxRet {
ret = maxRet
}
return ret, f.ts[t].status
}
func (f *fakeLibusb) free(t *libusbTransfer) {
f.mu.Lock()
defer f.mu.Unlock()
delete(f.ts, t)
}
func (f *fakeLibusb) setIsoPacketLengths(*libusbTransfer, uint32) {}
func (f *fakeLibusb) setIsoPacketLengths(t *libusbTransfer, length uint32) {
f.mu.Lock()
defer f.mu.Unlock()
maxLen := f.ts[t].isoPackets * int(length)
if bufLen := len(f.ts[t].buf); maxLen > bufLen {
maxLen = bufLen
}
f.ts[t].maxLength = maxLen
}
// waitForSubmitted can be used by tests to define custom behavior of the transfers submitted on the USB bus.
// TODO(sebek): add fields in fakeTransfer to differentiate between different devices/endpoints used concurrently.
func (f *fakeLibusb) waitForSubmitted(done <-chan struct{}) *fakeTransfer {
select {
case t, ok := <-f.submitted:

View File

@@ -131,7 +131,11 @@ func newUSBTransfer(ctx *Context, dev *libusbDevHandle, ei *EndpointDesc, bufLen
if bufLen < isoPktSize {
isoPktSize = bufLen
}
isoPackets = bufLen / isoPktSize
if isoPktSize > 0 {
isoPackets = bufLen / isoPktSize
} else {
isoPackets = 1
}
debug.Printf("New isochronous transfer - buffer length %d, using %d packets of %d bytes each", bufLen, isoPackets, isoPktSize)
}

View File

@@ -44,7 +44,7 @@ func TestNewTransfer(t *testing.T) {
tt: TransferTypeBulk,
maxPkt: 512,
buf: 1024,
wantLength: 1024,
wantLength: 512,
},
{
desc: "iso out transfer, 3 * 1024B packets",
@@ -52,7 +52,15 @@ func TestNewTransfer(t *testing.T) {
tt: TransferTypeIsochronous,
maxPkt: 3 * 1024,
buf: 10000,
wantLength: 10000,
wantLength: 9216,
},
{
desc: "iso out transfer, 512B packets",
dir: EndpointDirectionOut,
tt: TransferTypeIsochronous,
maxPkt: 512,
buf: 2048,
wantLength: 2048,
},
} {
xfer, err := newUSBTransfer(ctx, nil, &EndpointDesc{