Skip to content

Commit a159b85

Browse files
committed
Reorder composite device interfaces; fix report length bug
1 parent beb6ad2 commit a159b85

4 files changed

Lines changed: 124 additions & 84 deletions

File tree

ports/atmel-samd/common-hal/usb_hid/Device.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,16 @@ static uint32_t usb_hid_send_report(usb_hid_device_obj_t *self, uint8_t* report,
6060
// buffer load gets zero'd out when transaction completes, so if
6161
// you copy before it's ready, only zeros will get sent.
6262

63-
// Prefix with a report id if one is supplied
63+
// Prefix with a report id if one is supplied.
6464
if (self->report_id > 0) {
6565
self->report_buffer[0] = self->report_id;
6666
memcpy(&(self->report_buffer[1]), report, len);
67+
return hiddf_generic_write(self->report_buffer, len + 1);
6768
} else {
6869
memcpy(self->report_buffer, report, len);
70+
return hiddf_generic_write(self->report_buffer, len);
6971
}
7072

71-
return hiddf_generic_write(self->report_buffer, self->report_length);
7273
}
7374

7475
void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t* report, uint8_t len) {

ports/atmel-samd/common-hal/usb_hid/__init__.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
#include "genhdr/autogen_usb_descriptor.h"
3636

37-
// Buffers are report size + 1 to include the Report ID prefix byte
37+
// Buffers are report size + 1 to include the Report ID prefix byte if needed.
3838
static uint8_t keyboard_report_buffer[USB_HID_REPORT_LENGTH_KEYBOARD + 1];
3939
static uint8_t mouse_report_buffer[USB_HID_REPORT_LENGTH_MOUSE + 1];
4040
static uint8_t consumer_report_buffer[USB_HID_REPORT_LENGTH_CONSUMER + 1];

ports/atmel-samd/tools/gen_usb_descriptor.py

Lines changed: 119 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -24,82 +24,100 @@
2424

2525
args = parser.parse_args()
2626

27-
langid = standard.StringDescriptor("\u0409")
28-
manufacturer = standard.StringDescriptor(args.manufacturer)
29-
product = standard.StringDescriptor(args.product)
30-
serial_number = standard.StringDescriptor("serial number. you should fill in a unique serial number here."[:args.serial_number_length])
31-
strings = [langid, manufacturer, product, serial_number]
27+
class StringIndex:
28+
"""Assign a monotonically increasing index to each unique string. Start with 0."""
29+
string_to_index = {}
30+
strings = []
3231

33-
# vid = 0x239A
34-
# pid = 0x8021
32+
@classmethod
33+
def index(cls, string):
34+
if string in cls.string_to_index:
35+
return cls.string_to_index[string]
36+
else:
37+
idx = len(cls.strings)
38+
cls.string_to_index[string] = idx
39+
cls.strings.append(string)
40+
return idx
41+
42+
@classmethod
43+
def strings_in_order(cls):
44+
return cls.strings
45+
46+
47+
48+
# langid must be the 0th string descriptor
49+
LANGID_INDEX = StringIndex.index("\u0409")
50+
assert LANGID_INDEX == 0
51+
SERIAL_NUMBER_INDEX = StringIndex.index("S" * args.serial_number_length)
3552

3653
device = standard.DeviceDescriptor(
3754
description="top",
3855
idVendor=args.vid,
3956
idProduct=args.pid,
40-
iManufacturer=strings.index(manufacturer),
41-
iProduct=strings.index(product),
42-
iSerialNumber=strings.index(serial_number))
43-
44-
# Interface numbers are interface set local and endpoints are interface local
45-
# until core.join_interfaces renumbers them.
46-
cdc_interfaces = [
47-
standard.InterfaceDescriptor(
48-
description="CDC comm",
49-
bInterfaceClass=cdc.CDC_CLASS_COMM, # Communications Device Class
50-
bInterfaceSubClass=cdc.CDC_SUBCLASS_ACM, # Abstract control model
51-
bInterfaceProtocol=cdc.CDC_PROTOCOL_V25TER, # Common AT Commands
52-
subdescriptors=[
53-
# Working 2.x
54-
# radix: hexadecimal
55-
# 05 24 00 10 01 header
56-
# 05 24 01 03 01 call manage
57-
# 04 24 02 06 acm
58-
# 05 24 06 00 01 union
59-
cdc.Header(
60-
description="CDC comm",
61-
bcdCDC=0x0110),
62-
cdc.CallManagement(
63-
description="CDC comm",
64-
bmCapabilities=0x03,
65-
bDataInterface=0x01),
66-
cdc.AbstractControlManagement(
67-
description="CDC comm",
68-
bmCapabilities=0x02),
69-
cdc.Union(
70-
description="CDC comm",
71-
bMasterInterface=0x00,
72-
bSlaveInterface_list=[0x01]),
73-
standard.EndpointDescriptor(
74-
description="CDC comm in",
75-
bEndpointAddress=0x0 | standard.EndpointDescriptor.DIRECTION_IN,
76-
bmAttributes=standard.EndpointDescriptor.TYPE_INTERRUPT,
77-
wMaxPacketSize=0x0040,
78-
bInterval=0x10)
79-
]
80-
),
81-
standard.InterfaceDescriptor(
82-
description="CDC data",
83-
bInterfaceClass=cdc.CDC_CLASS_DATA,
84-
subdescriptors=[
85-
standard.EndpointDescriptor(
86-
description="CDC data in",
87-
bEndpointAddress=0x0 | standard.EndpointDescriptor.DIRECTION_IN,
88-
bmAttributes=standard.EndpointDescriptor.TYPE_BULK),
89-
standard.EndpointDescriptor(
90-
description="CDC data out",
91-
bEndpointAddress=0x0 | standard.EndpointDescriptor.DIRECTION_OUT,
92-
bmAttributes=standard.EndpointDescriptor.TYPE_BULK)
93-
]
94-
)
95-
]
57+
iManufacturer=StringIndex.index(args.manufacturer),
58+
iProduct=StringIndex.index(args.product),
59+
iSerialNumber=SERIAL_NUMBER_INDEX)
60+
61+
# Interface numbers are interface-set local and endpoints are interface local
62+
# until util.join_interfaces renumbers them.
63+
64+
cdc_union = cdc.Union(
65+
description="CDC comm",
66+
bMasterInterface=0x00, # Adjust this after interfaces are renumbered.
67+
bSlaveInterface_list=[0x01]) # Adjust this after interfaces are renumbered.
68+
69+
cdc_call_management = cdc.CallManagement(
70+
description="CDC comm",
71+
bmCapabilities=0x01,
72+
bDataInterface=0x01) # Adjust this after interfaces are renumbered.
73+
74+
cdc_comm_interface = standard.InterfaceDescriptor(
75+
description="CDC comm",
76+
bInterfaceClass=cdc.CDC_CLASS_COMM, # Communications Device Class
77+
bInterfaceSubClass=cdc.CDC_SUBCLASS_ACM, # Abstract control model
78+
bInterfaceProtocol=cdc.CDC_PROTOCOL_NONE,
79+
iInterface=StringIndex.index("CircuitPython CDC control"),
80+
subdescriptors=[
81+
cdc.Header(
82+
description="CDC comm",
83+
bcdCDC=0x0110),
84+
cdc_call_management,
85+
cdc.AbstractControlManagement(
86+
description="CDC comm",
87+
bmCapabilities=0x02),
88+
cdc_union,
89+
standard.EndpointDescriptor(
90+
description="CDC comm in",
91+
bEndpointAddress=0x0 | standard.EndpointDescriptor.DIRECTION_IN,
92+
bmAttributes=standard.EndpointDescriptor.TYPE_INTERRUPT,
93+
wMaxPacketSize=0x0040,
94+
bInterval=0x10)
95+
])
96+
97+
cdc_data_interface = standard.InterfaceDescriptor(
98+
description="CDC data",
99+
bInterfaceClass=cdc.CDC_CLASS_DATA,
100+
iInterface=StringIndex.index("CircuitPython CDC data"),
101+
subdescriptors=[
102+
standard.EndpointDescriptor(
103+
description="CDC data out",
104+
bEndpointAddress=0x0 | standard.EndpointDescriptor.DIRECTION_OUT,
105+
bmAttributes=standard.EndpointDescriptor.TYPE_BULK),
106+
standard.EndpointDescriptor(
107+
description="CDC data in",
108+
bEndpointAddress=0x0 | standard.EndpointDescriptor.DIRECTION_IN,
109+
bmAttributes=standard.EndpointDescriptor.TYPE_BULK),
110+
])
111+
112+
cdc_interfaces = [cdc_comm_interface, cdc_data_interface]
96113

97114
msc_interfaces = [
98115
standard.InterfaceDescriptor(
99116
description="MSC",
100117
bInterfaceClass=msc.MSC_CLASS,
101118
bInterfaceSubClass=msc.MSC_SUBCLASS_TRANSPARENT,
102119
bInterfaceProtocol=msc.MSC_PROTOCOL_BULK,
120+
iInterface=StringIndex.index("CircuitPython Mass Storage"),
103121
subdescriptors=[
104122
standard.EndpointDescriptor(
105123
description="MSC in",
@@ -124,7 +142,6 @@
124142
description="HID in",
125143
bEndpointAddress=0x0 | standard.EndpointDescriptor.DIRECTION_IN,
126144
bmAttributes=standard.EndpointDescriptor.TYPE_INTERRUPT,
127-
wMaxPacketSize=hid_max_report_length + 1, # +1 for the Report ID
128145
bInterval=0x02)
129146

130147
hid_endpoint_out_descriptor = standard.EndpointDescriptor(
@@ -134,41 +151,63 @@
134151

135152
hid_interfaces = [
136153
standard.InterfaceDescriptor(
137-
description="HID Keyboard",
154+
description="HID Multiple Devices",
138155
bInterfaceClass=hid.HID_CLASS,
139156
bInterfaceSubClass=hid.HID_SUBCLASS_NOBOOT,
140-
bInterfaceProtocol=hid.HID_PROTOCOL_KEYBOARD,
157+
bInterfaceProtocol=hid.HID_PROTOCOL_NONE,
158+
iInterface=StringIndex.index("CircuitPython HID"),
141159
subdescriptors=[
142-
hid.HIDDescriptor(wDescriptorLength=len(bytes(hid_report_descriptor))),
160+
hid.HIDDescriptor(
161+
description="HID",
162+
wDescriptorLength=len(bytes(hid_report_descriptor))),
143163
hid_endpoint_in_descriptor,
144164
hid_endpoint_out_descriptor,
145165
]
146166
),
147167
]
148168

149-
# This will renumber the endpoints to make them unique across descriptors.
150-
interfaces = util.join_interfaces(cdc_interfaces, msc_interfaces, hid_interfaces)
151-
#interfaces = util.join_interfaces(cdc_interfaces, hid_interfaces, msc_interfaces)
152-
#interfaces = util.join_interfaces(cdc_interfaces, hid_interfaces)
169+
# This will renumber the endpoints to make them unique across descriptors,
170+
# and renumber the interfaces in order. But we still need to fix up certain
171+
# interface cross-references.
172+
interfaces = util.join_interfaces(hid_interfaces, msc_interfaces, cdc_interfaces)
173+
174+
# Now adjust the CDC interface cross-references.
175+
176+
cdc_union.bMasterInterface = cdc_comm_interface.bInterfaceNumber
177+
cdc_union.bSlaveInterface_list = [cdc_data_interface.bInterfaceNumber]
178+
179+
cdc_call_management.bDataInterface = cdc_data_interface.bInterfaceNumber
153180

154-
cdc_function = standard.InterfaceAssociationDescriptor(
155-
description="CDC function",
156-
bFirstInterface=interfaces.index(cdc_interfaces[0]),
181+
cdc_iad = standard.InterfaceAssociationDescriptor(
182+
description="CDC IAD",
183+
bFirstInterface=cdc_comm_interface.bInterfaceNumber,
157184
bInterfaceCount=len(cdc_interfaces),
158185
bFunctionClass=0x2, # Communications Device Class
159186
bFunctionSubClass=0x2, # Abstract control model
160-
bFunctionProtocol=0x1) # Common AT Commands
187+
bFunctionProtocol=0x1)
161188

162189
configuration = standard.ConfigurationDescriptor(
163190
description="Composite configuration",
164191
wTotalLength=(standard.ConfigurationDescriptor.bLength +
165-
cdc_function.bLength +
192+
cdc_iad.bLength +
166193
sum([len(bytes(x)) for x in interfaces])),
167194
bNumInterfaces=len(interfaces))
168195

169-
descriptor_list = [device, configuration, cdc_function]
170-
descriptor_list.extend(interfaces)
171-
descriptor_list.extend(strings)
196+
descriptor_list = []
197+
descriptor_list.append(device)
198+
descriptor_list.append(configuration)
199+
descriptor_list.extend(hid_interfaces)
200+
descriptor_list.extend(msc_interfaces)
201+
# Put the CDC IAD just before the CDC interfaces.
202+
# There appears to be a bug in the Windows composite USB driver that requests the
203+
# HID report descriptor with the wrong interface number if the HID interface is not given
204+
# first. However, it still fetches the descriptor anyway.
205+
descriptor_list.append(cdc_iad)
206+
descriptor_list.extend(cdc_interfaces)
207+
208+
string_descriptors = [standard.StringDescriptor(string) for string in StringIndex.strings_in_order()]
209+
serial_number_descriptor = string_descriptors[SERIAL_NUMBER_INDEX]
210+
descriptor_list.extend(string_descriptors)
172211

173212
c_file = args.output_c_file
174213
h_file = args.output_h_file
@@ -199,7 +238,7 @@
199238
b = bytes(descriptor)
200239
i = 0
201240

202-
if descriptor == serial_number:
241+
if descriptor == serial_number_descriptor:
203242
# Add two for bLength and bDescriptorType.
204243
serial_number_offset = descriptor_length + 2
205244

0 commit comments

Comments
 (0)