Skip to content

Commit 645ac16

Browse files
committed
pbio/drv/usb: Migrate USB string descriptors to common implementation
Now that we have C11 enabled, we can declare these directly as UTF-16 strings and embed them as constant data. This deduplicates code for manually laying out the string descriptors.
1 parent f77a2ec commit 645ac16

File tree

8 files changed

+97
-94
lines changed

8 files changed

+97
-94
lines changed

lib/lego/lego/usb.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
/** Official LEGO USB Vendor ID. */
1212
#define LEGO_USB_VID 0x0694
13+
/** Official LEGO USB Product ID for MINDSTORMS NXT. */
14+
#define LEGO_USB_PID_NXT 0x0002
1315
/** Official LEGO USB Product ID for MINDSTORMS EV3. */
1416
#define LEGO_USB_PID_EV3 0x0005
1517
/** Official LEGO USB Product ID for MINDSTORMS EV3. */
@@ -28,12 +30,14 @@
2830
#define LEGO_USB_PID_ROBOT_INVENTOR_DFU 0x0011
2931

3032
/** Official LEGO USB Manufacturer String. */
31-
#define LEGO_USB_MFG_STR "LEGO System A/S"
33+
#define LEGO_USB_MFG_STR u"LEGO System A/S"
34+
/** NXT does not officially come with a product string */
35+
#define LEGO_USB_PROD_STR_NXT u"NXT"
3236
/** Official LEGO USB Product String for MINDSTORMS EV3. */
33-
#define LEGO_USB_PROD_STR_EV3 "LEGO MINDSTORMS EV3"
37+
#define LEGO_USB_PROD_STR_EV3 u"LEGO MINDSTORMS EV3"
3438
/** Official LEGO USB Product String for SPIKE Prime and MINDSTORMS Robot Inventor. */
35-
#define LEGO_USB_PROD_STR_TECHNIC_LARGE_HUB "LEGO Technic Large Hub"
39+
#define LEGO_USB_PROD_STR_TECHNIC_LARGE_HUB u"LEGO Technic Large Hub"
3640
/** Official LEGO USB Product String for SPIKE Essential. */
37-
#define LEGO_USB_PROD_STR_TECHNIC_SMALL_HUB "LEGO Technic Small Hub"
41+
#define LEGO_USB_PROD_STR_TECHNIC_SMALL_HUB u"LEGO Technic Small Hub"
3842

3943
#endif // _LEGO_USB_H_

lib/pbio/drv/usb/stm32_usbd/usbd_desc.c

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -98,20 +98,11 @@ pbdrv_usb_dev_desc_union_t USBD_DeviceDesc = {
9898
}; /* USB_DeviceDescriptor */
9999

100100
/* USB Standard Device Descriptor */
101-
__ALIGN_BEGIN static const uint8_t USBD_LangIDDesc[USB_LEN_LANGID_STR_DESC] __ALIGN_END = {
102-
USB_LEN_LANGID_STR_DESC,
103-
USB_DESC_TYPE_STRING,
104-
LOBYTE(USBD_LANGID_STRING),
105-
HIBYTE(USBD_LANGID_STRING),
106-
};
107-
108101
__ALIGN_BEGIN static uint8_t USBD_StringSerial[USB_SIZ_STRING_SERIAL] __ALIGN_END = {
109102
USB_SIZ_STRING_SERIAL,
110103
USB_DESC_TYPE_STRING,
111104
};
112105

113-
__ALIGN_BEGIN static uint8_t USBD_StrDesc[USBD_MAX_STR_DESC_SIZ] __ALIGN_END;
114-
115106

116107
/**
117108
* @brief Convert Hex 32Bits value into char
@@ -180,8 +171,8 @@ static uint8_t *USBD_Pybricks_LangIDStrDescriptor(USBD_SpeedTypeDef speed, uint1
180171
/* Prevent unused argument(s) compilation warning */
181172
UNUSED(speed);
182173

183-
*length = sizeof(USBD_LangIDDesc);
184-
return (uint8_t *)USBD_LangIDDesc;
174+
*length = pbdrv_usb_str_desc_langid.s.bLength;
175+
return (uint8_t *)&pbdrv_usb_str_desc_langid;
185176
}
186177

187178
/**
@@ -191,8 +182,11 @@ static uint8_t *USBD_Pybricks_LangIDStrDescriptor(USBD_SpeedTypeDef speed, uint1
191182
* @retval Pointer to descriptor buffer
192183
*/
193184
static uint8_t *USBD_Pybricks_ProductStrDescriptor(USBD_SpeedTypeDef speed, uint16_t *length) {
194-
USBD_GetString((uint8_t *)PBDRV_CONFIG_USB_PROD_STR, USBD_StrDesc, length);
195-
return USBD_StrDesc;
185+
/* Prevent unused argument(s) compilation warning */
186+
UNUSED(speed);
187+
188+
*length = pbdrv_usb_str_desc_prod.s.bLength;
189+
return (uint8_t *)&pbdrv_usb_str_desc_prod;
196190
}
197191

198192
/**
@@ -205,8 +199,8 @@ static uint8_t *USBD_Pybricks_ManufacturerStrDescriptor(USBD_SpeedTypeDef speed,
205199
/* Prevent unused argument(s) compilation warning */
206200
UNUSED(speed);
207201

208-
USBD_GetString((uint8_t *)PBDRV_CONFIG_USB_MFG_STR, USBD_StrDesc, length);
209-
return USBD_StrDesc;
202+
*length = pbdrv_usb_str_desc_mfg.s.bLength;
203+
return (uint8_t *)&pbdrv_usb_str_desc_mfg;
210204
}
211205

212206
/**

lib/pbio/drv/usb/usb_ch9.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ typedef struct PBDRV_PACKED {
136136
uint8_t iInterface;
137137
} pbdrv_usb_iface_desc_t;
138138

139+
// This LangID is used for string descriptors
140+
// English (United States)
141+
#define PBDRV_USB_STRING_LANGID_EN_US 0x0409
142+
139143
// Endpoint descriptor
140144
typedef struct PBDRV_PACKED {
141145
uint8_t bLength;

lib/pbio/drv/usb/usb_common_desc.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,30 @@ const pbdrv_usb_bos_desc_set_union_t pbdrv_usb_bos_desc_set = {
9494
}
9595
};
9696

97+
const pbdrv_usb_langid_union_t pbdrv_usb_str_desc_langid = {
98+
.s = {
99+
.bLength = 4,
100+
.bDescriptorType = DESC_TYPE_STRING,
101+
.langID = {PBDRV_USB_STRING_LANGID_EN_US},
102+
}
103+
};
104+
105+
const pbdrv_usb_str_mfg_union_t pbdrv_usb_str_desc_mfg = {
106+
.s = {
107+
// Note: strip trailing null terminator
108+
.bLength = sizeof(pbdrv_usb_str_mfg_t) - 2,
109+
.bDescriptorType = DESC_TYPE_STRING,
110+
.str = PBDRV_CONFIG_USB_MFG_STR,
111+
}
112+
};
113+
114+
const pbdrv_usb_str_prod_union_t pbdrv_usb_str_desc_prod = {
115+
.s = {
116+
// Note: strip trailing null terminator
117+
.bLength = sizeof(pbdrv_usb_str_prod_t) - 2,
118+
.bDescriptorType = DESC_TYPE_STRING,
119+
.str = PBDRV_CONFIG_USB_PROD_STR,
120+
}
121+
};
122+
97123
#endif // PBDRV_CONFIG_USB

lib/pbio/drv/usb/usb_common_desc.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
#ifndef _INTERNAL_PBDRV_USB_COMMON_DESC_H_
88
#define _INTERNAL_PBDRV_USB_COMMON_DESC_H_
99

10+
#include <lego/usb.h>
11+
#include "pbdrvconfig.h"
12+
1013
#include "usb_ch9.h"
1114

1215
/**
@@ -52,4 +55,32 @@ PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_bos_desc_set);
5255

5356
extern const pbdrv_usb_bos_desc_set_union_t pbdrv_usb_bos_desc_set;
5457

58+
// (Human-readable) string descriptors
59+
typedef struct PBDRV_PACKED {
60+
uint8_t bLength;
61+
uint8_t bDescriptorType;
62+
uint16_t langID[1];
63+
} pbdrv_usb_langid_t;
64+
PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_langid);
65+
66+
extern const pbdrv_usb_langid_union_t pbdrv_usb_str_desc_langid;
67+
68+
typedef struct PBDRV_PACKED {
69+
uint8_t bLength;
70+
uint8_t bDescriptorType;
71+
uint16_t str[sizeof(PBDRV_CONFIG_USB_MFG_STR) / 2];
72+
} pbdrv_usb_str_mfg_t;
73+
PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_str_mfg);
74+
75+
extern const pbdrv_usb_str_mfg_union_t pbdrv_usb_str_desc_mfg;
76+
77+
typedef struct PBDRV_PACKED {
78+
uint8_t bLength;
79+
uint8_t bDescriptorType;
80+
uint16_t str[sizeof(PBDRV_CONFIG_USB_PROD_STR) / 2];
81+
} pbdrv_usb_str_prod_t;
82+
PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_str_prod);
83+
84+
extern const pbdrv_usb_str_prod_union_t pbdrv_usb_str_desc_prod;
85+
5586
#endif // _INTERNAL_PBDRV_USB_COMMON_DESC_H_

lib/pbio/drv/usb/usb_ev3.c

Lines changed: 8 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -180,22 +180,8 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_fs = {
180180
}
181181
};
182182

183-
typedef struct PBDRV_PACKED {
184-
uint8_t bLength;
185-
uint8_t bDescriptorType;
186-
uint16_t langID[1];
187-
} pbdrv_usb_langid_t;
188-
PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_langid);
189-
190-
pbdrv_usb_langid_union_t usb_str_desc_langid = {
191-
.s = {
192-
.bLength = 4,
193-
.bDescriptorType = DESC_TYPE_STRING,
194-
.langID = {0x0409}, // English (United States)
195-
}
196-
};
197-
198-
// We generate string descriptors at runtime, so this dynamic buffer is needed
183+
// We generate a serial number string descriptors at runtime
184+
// so this dynamic buffer is needed
199185
#define STRING_DESC_MAX_SZ 64
200186
static union {
201187
uint8_t b[STRING_DESC_MAX_SZ];
@@ -456,36 +442,18 @@ static bool usb_get_descriptor(uint16_t wValue) {
456442
case DESC_TYPE_STRING:
457443
switch (desc_idx) {
458444
case STRING_DESC_LANGID:
459-
pbdrv_usb_setup_data_to_send = usb_str_desc_langid.u;
460-
pbdrv_usb_setup_data_to_send_sz = sizeof(usb_str_desc_langid);
445+
pbdrv_usb_setup_data_to_send = pbdrv_usb_str_desc_langid.u;
446+
pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_str_desc_langid.s);
461447
return true;
462448

463449
case STRING_DESC_MFG:
464-
usb_string_desc_buffer.b[1] = DESC_TYPE_STRING;
465-
i = 0;
466-
while (PBDRV_CONFIG_USB_MFG_STR[i]) {
467-
usb_string_desc_buffer.b[2 + 2 * i] = PBDRV_CONFIG_USB_MFG_STR[i];
468-
usb_string_desc_buffer.b[2 + 2 * i + 1] = 0;
469-
i++;
470-
}
471-
usb_string_desc_buffer.b[0] = 2 * i + 2;
472-
473-
pbdrv_usb_setup_data_to_send = usb_string_desc_buffer.u;
474-
pbdrv_usb_setup_data_to_send_sz = usb_string_desc_buffer.b[0];
450+
pbdrv_usb_setup_data_to_send = pbdrv_usb_str_desc_mfg.u;
451+
pbdrv_usb_setup_data_to_send_sz = pbdrv_usb_str_desc_mfg.s.bLength;
475452
return true;
476453

477454
case STRING_DESC_PRODUCT:
478-
usb_string_desc_buffer.b[1] = DESC_TYPE_STRING;
479-
i = 0;
480-
while (PBDRV_CONFIG_USB_PROD_STR[i]) {
481-
usb_string_desc_buffer.b[2 + 2 * i] = PBDRV_CONFIG_USB_PROD_STR[i];
482-
usb_string_desc_buffer.b[2 + 2 * i + 1] = 0;
483-
i++;
484-
}
485-
usb_string_desc_buffer.b[0] = 2 * i + 2;
486-
487-
pbdrv_usb_setup_data_to_send = usb_string_desc_buffer.u;
488-
pbdrv_usb_setup_data_to_send_sz = usb_string_desc_buffer.b[0];
455+
pbdrv_usb_setup_data_to_send = pbdrv_usb_str_desc_prod.u;
456+
pbdrv_usb_setup_data_to_send_sz = pbdrv_usb_str_desc_prod.s.bLength;
489457
return true;
490458

491459
case STRING_DESC_SERIAL:

lib/pbio/drv/usb/usb_nxt.c

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
#include "nxos/drivers/aic.h"
2828
#include "nxos/util.h"
2929

30+
#include <lego/usb.h>
31+
#include "pbdrvconfig.h"
32+
3033
#include "usb_ch9.h"
3134
#include "usb_common_desc.h"
3235

@@ -185,43 +188,12 @@ static const pbdrv_usb_nxt_conf_t pbdrv_usb_nxt_full_config = {
185188
},
186189
};
187190

188-
static const uint8_t pbdrv_usb_nxt_string_desc[] = {
189-
4, USB_DESC_TYPE_STR, /* Descriptor length and type. */
190-
0x09, 0x04, /* Supported language ID (US English). */
191-
};
192-
193-
static const uint8_t pbdrv_usb_lego_str[] = {
194-
10, USB_DESC_TYPE_STR,
195-
'L', 0,
196-
'E', 0,
197-
'G', 0,
198-
'O', 0,
199-
};
200-
201-
static const uint8_t pbdrv_usb_nxt_str[] = {
202-
30, USB_DESC_TYPE_STR,
203-
'N', 0,
204-
'X', 0,
205-
'T', 0,
206-
' ', 0,
207-
'+', 0,
208-
' ', 0,
209-
'P', 0,
210-
'y', 0,
211-
'b', 0,
212-
'r', 0,
213-
'i', 0,
214-
'c', 0,
215-
'k', 0,
216-
's', 0,
217-
};
218-
219191
/* Internal lookup table mapping string descriptors to their indices
220192
* in the USB string descriptor table.
221193
*/
222194
static const uint8_t *pbdrv_usb_nxt_strings[] = {
223-
pbdrv_usb_lego_str,
224-
pbdrv_usb_nxt_str,
195+
(const uint8_t *)&pbdrv_usb_str_desc_mfg,
196+
(const uint8_t *)&pbdrv_usb_str_desc_prod,
225197
};
226198

227199
typedef enum {
@@ -495,8 +467,8 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) {
495467

496468
case USB_DESC_TYPE_STR: /* String or language info. */
497469
if ((packet->value & USB_WVALUE_INDEX) == 0) {
498-
pbdrv_usb_nxt_write_data(0, pbdrv_usb_nxt_string_desc,
499-
MIN(pbdrv_usb_nxt_string_desc[0], packet->length));
470+
pbdrv_usb_nxt_write_data(0, &pbdrv_usb_str_desc_langid,
471+
MIN(pbdrv_usb_str_desc_langid.s.bLength, packet->length));
500472
} else {
501473
/* The host wants a specific string. */
502474
/* TODO: This should check if the requested string exists. */

lib/pbio/platform/nxt/pbdrvconfig.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,7 @@
5050

5151
#define PBDRV_CONFIG_USB (1)
5252
#define PBDRV_CONFIG_USB_NXT (1)
53+
#define PBDRV_CONFIG_USB_VID LEGO_USB_VID
54+
#define PBDRV_CONFIG_USB_PID LEGO_USB_PID_NXT
55+
#define PBDRV_CONFIG_USB_MFG_STR LEGO_USB_MFG_STR
56+
#define PBDRV_CONFIG_USB_PROD_STR LEGO_USB_PROD_STR_NXT " + Pybricks"

0 commit comments

Comments
 (0)