From f9aba6fab508ab8a5299471690457868a492eff7 Mon Sep 17 00:00:00 2001 From: zagrodzki Date: Mon, 4 Sep 2017 14:17:34 +0200 Subject: [PATCH] Config and interface description (#16) * Add APIs for config and interface descriptors. Split out the common parts of selecting a config descriptor from device desc and selecting a setting descriptor from a config desc. * Parallelize the few tests that actually can be parallelized safely. Add comments where they can't. Note to self: it would be beneficial to restructure the fakelibusb to index all properties of the lib with the used context. That way a libusb implementation wouldn't need to be referred via a shared variable. --- config.go | 25 +++++++++---- device.go | 63 ++++++++++++++++++++++++------- device_test.go | 83 +++++++++++++++++++++++++++++++++-------- endpoint_stream_test.go | 1 + endpoint_test.go | 8 ++-- fakelibusb_devices.go | 18 ++++++--- interface.go | 2 + libusb.go | 20 +++++----- misc_test.go | 1 + transfer_stream_test.go | 3 ++ transfer_test.go | 2 + usb_test.go | 2 + 12 files changed, 175 insertions(+), 53 deletions(-) diff --git a/config.go b/config.go index 076b5bc..febc3f1 100644 --- a/config.go +++ b/config.go @@ -40,6 +40,8 @@ type ConfigDesc struct { MaxPower Milliamperes // Interfaces has a list of USB interfaces available in this configuration. Interfaces []InterfaceDesc + + iConfiguration int // index of a string descriptor describing this configuration } // String returns the human-readable description of the configuration descriptor. @@ -47,6 +49,17 @@ func (c ConfigDesc) String() string { return fmt.Sprintf("Configuration %d", c.Number) } +func (c ConfigDesc) intfDesc(num, alt int) (*InterfaceSetting, error) { + if num < 0 || num >= len(c.Interfaces) { + return nil, fmt.Errorf("interface %d not found, available interfaces 0..%d", num, len(c.Interfaces)-1) + } + ifInfo := c.Interfaces[num] + if alt < 0 || alt >= len(ifInfo.AltSettings) { + return nil, fmt.Errorf("alternate setting %d not found for %s, available alt settings 0..%d", alt, ifInfo, len(ifInfo.AltSettings)-1) + } + return &ifInfo.AltSettings[alt], nil +} + // Config represents a USB device set to use a particular configuration. // Only one Config of a particular device can be used at any one time. // To access device endpoints, claim an interface and it's alternate @@ -94,12 +107,10 @@ func (c *Config) Interface(num, alt int) (*Interface, error) { if c.dev == nil { return nil, fmt.Errorf("Interface(%d, %d) called on %s after Close", num, alt, c) } - if num < 0 || num >= len(c.Desc.Interfaces) { - return nil, fmt.Errorf("interface %d not found in %s, available interfaces 0..%d", num, c, len(c.Desc.Interfaces)-1) - } - ifInfo := c.Desc.Interfaces[num] - if alt < 0 || alt >= len(ifInfo.AltSettings) { - return nil, fmt.Errorf("alternate setting %d not found for %s in %s, available alt settings 0..%d", alt, ifInfo, c, len(ifInfo.AltSettings)-1) + + altInfo, err := c.Desc.intfDesc(num, alt) + if err != nil { + return nil, fmt.Errorf("in %s: %v", err) } c.mu.Lock() @@ -120,7 +131,7 @@ func (c *Config) Interface(num, alt int) (*Interface, error) { c.claimed[num] = true return &Interface{ - Setting: ifInfo.AltSettings[alt], + Setting: *altInfo, config: c, }, nil } diff --git a/device.go b/device.go index 2cb3fce..1e70f67 100644 --- a/device.go +++ b/device.go @@ -56,6 +56,23 @@ func (d *DeviceDesc) String() string { return fmt.Sprintf("%d.%d: %s:%s (available configs: %v)", d.Bus, d.Address, d.Vendor, d.Product, d.sortedConfigIds()) } +func (d *DeviceDesc) sortedConfigIds() []int { + var cfgs []int + for c := range d.Configs { + cfgs = append(cfgs, c) + } + sort.Ints(cfgs) + return cfgs +} + +func (d *DeviceDesc) cfgDesc(cfgNum int) (*ConfigDesc, error) { + desc, ok := d.Configs[cfgNum] + if !ok { + return nil, fmt.Errorf("configuration id %d not found in the descriptor of the device. Available config ids: %v", cfgNum, d.sortedConfigIds()) + } + return &desc, nil +} + // Device represents an opened USB device. // Device allows sending USB control commands through the Command() method. // For data transfers select a device configuration through a call to @@ -77,15 +94,6 @@ type Device struct { autodetach bool } -func (d *DeviceDesc) sortedConfigIds() []int { - var cfgs []int - for c := range d.Configs { - cfgs = append(cfgs, c) - } - sort.Ints(cfgs) - return cfgs -} - // String represents a human readable representation of the device. func (d *Device) String() string { return fmt.Sprintf("vid=%s,pid=%s,bus=%d,addr=%d", d.Desc.Vendor, d.Desc.Product, d.Desc.Bus, d.Desc.Address) @@ -126,12 +134,12 @@ func (d *Device) Config(cfgNum int) (*Config, error) { if d.handle == nil { return nil, fmt.Errorf("Config(%d) called on %s after Close", cfgNum, d) } - desc, ok := d.Desc.Configs[cfgNum] - if !ok { - return nil, fmt.Errorf("configuration id %d not found in the descriptor of the device %s. Available config ids: %v", cfgNum, d, d.Desc.sortedConfigIds()) + desc, err := d.Desc.cfgDesc(cfgNum) + if err != nil { + return nil, fmt.Errorf("device %s: %v", d, err) } cfg := &Config{ - Desc: desc, + Desc: *desc, dev: d, claimed: make(map[int]bool), } @@ -212,6 +220,10 @@ func (d *Device) GetStringDescriptor(descIndex int) (string, error) { if d.handle == nil { return "", fmt.Errorf("GetStringDescriptor(%d) called on %s after Close", descIndex, d) } + // string descriptor index value of 0 indicates no string descriptor. + if descIndex == 0 { + return "", nil + } return libusb.getStringDesc(d.handle, descIndex) } @@ -233,6 +245,31 @@ func (d *Device) SerialNumber() (string, error) { return d.GetStringDescriptor(d.Desc.iSerialNumber) } +// ConfigDescription returns the description of the selected device +// configuration. GetStringDescriptor's string conversion rules apply. +func (d *Device) ConfigDescription(cfg int) (string, error) { + c, err := d.Desc.cfgDesc(cfg) + if err != nil { + return "", fmt.Errorf("%s: %v", d, err) + } + return d.GetStringDescriptor(c.iConfiguration) +} + +// InterfaceDescription returns the description of the selected interface and +// its alternate setting in a selected configuration. GetStringDescriptor's +// string conversion rules apply. +func (d *Device) InterfaceDescription(cfgNum, intfNum, altNum int) (string, error) { + cfg, err := d.Desc.cfgDesc(cfgNum) + if err != nil { + return "", fmt.Errorf("%s: %v", d, err) + } + alt, err := cfg.intfDesc(intfNum, altNum) + if err != nil { + return "", fmt.Errorf("%s, configuration %d: %v", d, cfgNum, err) + } + return d.GetStringDescriptor(alt.iInterface) +} + // SetAutoDetach enables/disables automatic kernel driver detachment. // When autodetach is enabled gousb will automatically detach the kernel driver // on the interface and reattach it when releasing the interface. diff --git a/device_test.go b/device_test.go index 7e1d856..d4aed33 100644 --- a/device_test.go +++ b/device_test.go @@ -21,6 +21,7 @@ import ( ) func TestClaimAndRelease(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. _, done := newFakeLibusb() defer done() @@ -44,26 +45,45 @@ func TestClaimAndRelease(t *testing.T) { t.Fatalf("OpenDeviceWithVIDPID(0x8888, 0x0002): %v", err) } - mfg, err := dev.Manufacturer() - if err != nil { - t.Errorf("%s.Manufacturer(): %v", dev, err) + if mfg, err := dev.Manufacturer(); err != nil { + t.Errorf("%s.Manufacturer(): error %v", dev, err) + } else if want := "ACME Industries"; mfg != want { + t.Errorf("%s.Manufacturer(): %q, want %q", dev, mfg, want) } - if mfg != "ACME Industries" { - t.Errorf("%s.Manufacturer(): %q", dev, mfg) + if prod, err := dev.Product(); err != nil { + t.Errorf("%s.Product(): error %v", dev, err) + } else if want := "Fidgety Gadget"; prod != want { + t.Errorf("%s.Product(): %q, want %q", dev, prod, want) } - prod, err := dev.Product() - if err != nil { - t.Errorf("%s.Product(): %v", dev, err) + if sn, err := dev.SerialNumber(); err != nil { + t.Errorf("%s.SerialNumber(): error %v", dev, err) + } else if want := "01234567"; sn != want { + t.Errorf("%s.SerialNumber(): %q, want %q", dev, sn, want) } - if prod != "Fidgety Gadget" { - t.Errorf("%s.Product(): %q", dev, prod) + + if got, err := dev.ConfigDescription(1); err != nil { + t.Errorf("%s.ConfigDescription(1): %v", dev, err) + } else if want := "Weird configuration"; got != want { + t.Errorf("%s.ConfigDescription(1): %q, want %q", dev, got, want) } - sn, err := dev.SerialNumber() - if err != nil { - t.Errorf("%s.SerialNumber(): %v", dev, err) + if got, err := dev.ConfigDescription(2); err == nil { + t.Errorf("%s.ConfigDescription(2): %q, want error", dev, got) } - if sn != "01234567" { - t.Errorf("%s.SerialNumber(): %q", dev, sn) + + for _, tc := range []struct { + intf, alt int + want string + }{ + {0, 0, "Boring setting"}, + {1, 0, "Fast streaming"}, + {1, 1, "Slower streaming"}, + {1, 2, ""}, + } { + if got, err := dev.InterfaceDescription(1, tc.intf, tc.alt); err != nil { + t.Errorf("%s.InterfaceDescription(1, %d, %d): %v", dev, tc.intf, tc.alt, err) + } else if got != tc.want { + t.Errorf("%s.InterfaceDescription(1, %d, %d): %q, want %q", dev, tc.intf, tc.alt, got, tc.want) + } } if err = dev.SetAutoDetach(true); err != nil { @@ -156,6 +176,38 @@ func TestClaimAndRelease(t *testing.T) { } } +func TestInterfaceDescriptionError(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. + _, done := newFakeLibusb() + defer done() + + for _, tc := range []struct { + name string + cfg, intf, alt int + }{ + {"no config", 2, 1, 1}, + {"no interface", 1, 3, 1}, + {"no alt setting", 1, 1, 5}, + } { + t.Run(tc.name, func(t *testing.T) { + // Can't be parallelized, depends on the shared global state set before the loop. + c := NewContext() + defer c.Close() + dev, err := c.OpenDeviceWithVIDPID(0x8888, 0x0002) + if dev == nil { + t.Fatal("OpenDeviceWithVIDPID(0x8888, 0x0002): got nil device, need non-nil") + } + defer dev.Close() + if err != nil { + t.Fatalf("OpenDeviceWithVIDPID(0x8888, 0x0002): %v", err) + } + if desc, err := dev.InterfaceDescription(tc.cfg, tc.intf, tc.alt); err == nil { + t.Errorf("%s.InterfaceDescriptor(%d, %d, %d): %q, want error", dev, tc.cfg, tc.intf, tc.alt, desc) + } + }) + } +} + type failDetachLib struct { *fakeLibusb } @@ -168,6 +220,7 @@ func (*failDetachLib) detachKernelDriver(h *libusbDevHandle, i uint8) error { } func TestAutoDetachFailure(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. fake, done := newFakeLibusb() defer done() libusb = &failDetachLib{fake} diff --git a/endpoint_stream_test.go b/endpoint_stream_test.go index ad11c8c..7f21a55 100644 --- a/endpoint_stream_test.go +++ b/endpoint_stream_test.go @@ -17,6 +17,7 @@ package gousb import "testing" func TestEndpointReadStream(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. lib, done := newFakeLibusb() defer done() diff --git a/endpoint_test.go b/endpoint_test.go index 1dfcaf0..909be62 100644 --- a/endpoint_test.go +++ b/endpoint_test.go @@ -20,6 +20,7 @@ import ( ) func TestEndpoint(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. lib, done := newFakeLibusb() defer done() @@ -116,6 +117,7 @@ func TestEndpoint(t *testing.T) { } func TestEndpointInfo(t *testing.T) { + t.Parallel() for _, tc := range []struct { ep EndpointDesc want string @@ -161,8 +163,7 @@ func TestEndpointInfo(t *testing.T) { } func TestEndpointInOut(t *testing.T) { - defer func(i libusbIntf) { libusb = i }(libusb) - + // Can't be parallelized, newFakeLibusb modifies a shared global state. lib, done := newFakeLibusb() defer done() @@ -242,8 +243,7 @@ func TestEndpointInOut(t *testing.T) { } func TestSameEndpointNumberInOut(t *testing.T) { - defer func(i libusbIntf) { libusb = i }(libusb) - + // Can't be parallelized, newFakeLibusb modifies a shared global state. _, done := newFakeLibusb() defer done() diff --git a/fakelibusb_devices.go b/fakelibusb_devices.go index a99ebe0..7154290 100644 --- a/fakelibusb_devices.go +++ b/fakelibusb_devices.go @@ -78,14 +78,16 @@ var fakeDevices = []fakeDevice{ Product: ID(0x0002), Protocol: 255, Configs: map[int]ConfigDesc{1: { - Number: 1, - MaxPower: Milliamperes(100), + Number: 1, + MaxPower: Milliamperes(100), + iConfiguration: 5, Interfaces: []InterfaceDesc{{ Number: 0, AltSettings: []InterfaceSetting{{ - Number: 0, - Alternate: 0, - Class: ClassVendorSpec, + Number: 0, + Alternate: 0, + Class: ClassVendorSpec, + iInterface: 6, }}, }, { Number: 1, @@ -111,6 +113,7 @@ var fakeDevices = []fakeDevice{ UsageType: IsoUsageTypeData, }, }, + iInterface: 7, }, { Number: 1, Alternate: 1, @@ -131,6 +134,7 @@ var fakeDevices = []fakeDevice{ TransferType: TransferTypeIsochronous, }, }, + iInterface: 8, }, { Number: 1, Alternate: 2, @@ -162,6 +166,10 @@ var fakeDevices = []fakeDevice{ 1: "ACME Industries", 2: "Fidgety Gadget", 3: "01234567", + 5: "Weird configuration", + 6: "Boring setting", + 7: "Fast streaming", + 8: "Slower streaming", }, }, // Bus 001 Device 003: ID 9999:0002 diff --git a/interface.go b/interface.go index 96f1790..ec062c5 100644 --- a/interface.go +++ b/interface.go @@ -52,6 +52,8 @@ type InterfaceSetting struct { // Endpoints enumerates the endpoints available on this interface with // this alternate setting. Endpoints map[EndpointAddress]EndpointDesc + + iInterface int // index of a string descriptor describing this interface. } func (a InterfaceSetting) sortedEndpointIds() []string { diff --git a/libusb.go b/libusb.go index 5b16be6..2633376 100644 --- a/libusb.go +++ b/libusb.go @@ -250,10 +250,11 @@ func (libusbImpl) getDeviceDesc(d *libusbDevice) (*DeviceDesc, error) { return nil, err } c := ConfigDesc{ - Number: int(cfg.bConfigurationValue), - SelfPowered: (cfg.bmAttributes & selfPoweredMask) != 0, - RemoteWakeup: (cfg.bmAttributes & remoteWakeupMask) != 0, - MaxPower: 2 * Milliamperes(cfg.MaxPower), + Number: int(cfg.bConfigurationValue), + SelfPowered: (cfg.bmAttributes & selfPoweredMask) != 0, + RemoteWakeup: (cfg.bmAttributes & remoteWakeupMask) != 0, + MaxPower: 2 * Milliamperes(cfg.MaxPower), + iConfiguration: int(cfg.iConfiguration), } // at GenX speeds MaxPower is expressed in units of 8mA, not 2mA. if dev.Speed == SpeedSuper { @@ -281,11 +282,12 @@ func (libusbImpl) getDeviceDesc(d *libusbDevice) (*DeviceDesc, error) { descs := make([]InterfaceSetting, 0, len(alts)) for altNum, alt := range alts { i := InterfaceSetting{ - Number: int(alt.bInterfaceNumber), - Alternate: int(alt.bAlternateSetting), - Class: Class(alt.bInterfaceClass), - SubClass: Class(alt.bInterfaceSubClass), - Protocol: Protocol(alt.bInterfaceProtocol), + Number: int(alt.bInterfaceNumber), + Alternate: int(alt.bAlternateSetting), + Class: Class(alt.bInterfaceClass), + SubClass: Class(alt.bInterfaceSubClass), + Protocol: Protocol(alt.bInterfaceProtocol), + iInterface: int(alt.iInterface), } if ifNum != i.Number { return nil, fmt.Errorf("config %d interface at index %d has number %d, USB standard states they should be identical", c.Number, ifNum, i.Number) diff --git a/misc_test.go b/misc_test.go index a85d176..a5bca67 100644 --- a/misc_test.go +++ b/misc_test.go @@ -20,6 +20,7 @@ import ( ) func TestBCD(t *testing.T) { + t.Parallel() tests := []struct { major, minor uint8 bcd BCD diff --git a/transfer_stream_test.go b/transfer_stream_test.go index b14fe5d..b159ca8 100644 --- a/transfer_stream_test.go +++ b/transfer_stream_test.go @@ -106,6 +106,7 @@ func (r readRes) String() string { } func TestTransferReadStream(t *testing.T) { + t.Parallel() for tcNum, tc := range []struct { desc string closeBefore int @@ -211,7 +212,9 @@ func TestTransferReadStream(t *testing.T) { }, }, } { + tcNum, tc := tcNum, tc // t.Parallel will delay the execution of the test, save the iteration values. t.Run(strconv.Itoa(tcNum), func(t *testing.T) { + t.Parallel() t.Logf("Case %d: %s", tcNum, tc.desc) ftt := make([]*fakeStreamTransfer, len(tc.transfers)) tt := make([]transferIntf, len(tc.transfers)) diff --git a/transfer_test.go b/transfer_test.go index 9ce0fcc..0c98b99 100644 --- a/transfer_test.go +++ b/transfer_test.go @@ -20,6 +20,7 @@ import ( ) func TestNewTransfer(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. _, done := newFakeLibusb() defer done() @@ -70,6 +71,7 @@ func TestNewTransfer(t *testing.T) { } func TestTransferProtocol(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. f, done := newFakeLibusb() defer done() diff --git a/usb_test.go b/usb_test.go index 9f68050..687e2bd 100644 --- a/usb_test.go +++ b/usb_test.go @@ -18,6 +18,7 @@ package gousb import "testing" func TestOPenDevices(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. _, done := newFakeLibusb() defer done() @@ -54,6 +55,7 @@ func TestOPenDevices(t *testing.T) { } func TestOpenDeviceWithVIDPID(t *testing.T) { + // Can't be parallelized, newFakeLibusb modifies a shared global state. _, done := newFakeLibusb() defer done()