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.

Author: ArcaneNibble

lib/lego/lego/usb.h

@@ -10,6 +10,8 @@
 
 /** Official LEGO USB Vendor ID. */
 #define LEGO_USB_VID 0x0694
+/** Official LEGO USB Product ID for MINDSTORMS NXT. */
+#define LEGO_USB_PID_NXT 0x0002
 /** Official LEGO USB Product ID for MINDSTORMS EV3. */
 #define LEGO_USB_PID_EV3 0x0005
 /** Official LEGO USB Product ID for MINDSTORMS EV3. */
@@ -28,12 +30,14 @@
 #define LEGO_USB_PID_ROBOT_INVENTOR_DFU 0x0011
 
 /** Official LEGO USB Manufacturer String. */
-#define LEGO_USB_MFG_STR "LEGO System A/S"
+#define LEGO_USB_MFG_STR u"LEGO System A/S"
+/** NXT does not officially come with a product string */
+#define LEGO_USB_PROD_STR_NXT u"NXT"
 /** Official LEGO USB Product String for MINDSTORMS EV3. */
-#define LEGO_USB_PROD_STR_EV3 "LEGO MINDSTORMS EV3"
+#define LEGO_USB_PROD_STR_EV3 u"LEGO MINDSTORMS EV3"
 /** Official LEGO USB Product String for SPIKE Prime and MINDSTORMS Robot Inventor. */
-#define LEGO_USB_PROD_STR_TECHNIC_LARGE_HUB "LEGO Technic Large Hub"
+#define LEGO_USB_PROD_STR_TECHNIC_LARGE_HUB u"LEGO Technic Large Hub"
 /** Official LEGO USB Product String for SPIKE Essential. */
-#define LEGO_USB_PROD_STR_TECHNIC_SMALL_HUB "LEGO Technic Small Hub"
+#define LEGO_USB_PROD_STR_TECHNIC_SMALL_HUB u"LEGO Technic Small Hub"
 
 #endif // _LEGO_USB_H_

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

@@ -98,20 +98,11 @@ pbdrv_usb_dev_desc_union_t USBD_DeviceDesc = {
 }; /* USB_DeviceDescriptor */
 
 /* USB Standard Device Descriptor */
-__ALIGN_BEGIN static const uint8_t USBD_LangIDDesc[USB_LEN_LANGID_STR_DESC] __ALIGN_END = {
-    USB_LEN_LANGID_STR_DESC,
-    USB_DESC_TYPE_STRING,
-    LOBYTE(USBD_LANGID_STRING),
-    HIBYTE(USBD_LANGID_STRING),
-};
-
 __ALIGN_BEGIN static uint8_t USBD_StringSerial[USB_SIZ_STRING_SERIAL] __ALIGN_END = {
     USB_SIZ_STRING_SERIAL,
     USB_DESC_TYPE_STRING,
 };
 
-__ALIGN_BEGIN static uint8_t USBD_StrDesc[USBD_MAX_STR_DESC_SIZ] __ALIGN_END;
-
 
 /**
   * @brief  Convert Hex 32Bits value into char
@@ -180,8 +171,8 @@ static uint8_t *USBD_Pybricks_LangIDStrDescriptor(USBD_SpeedTypeDef speed, uint1
     /* Prevent unused argument(s) compilation warning */
     UNUSED(speed);
 
-    *length = sizeof(USBD_LangIDDesc);
-    return (uint8_t *)USBD_LangIDDesc;
+    *length = sizeof(pbdrv_usb_str_desc_langid.s);
+    return (uint8_t *)&pbdrv_usb_str_desc_langid;
 }
 
 /**
@@ -191,8 +182,11 @@ static uint8_t *USBD_Pybricks_LangIDStrDescriptor(USBD_SpeedTypeDef speed, uint1
   * @retval Pointer to descriptor buffer
   */
 static uint8_t *USBD_Pybricks_ProductStrDescriptor(USBD_SpeedTypeDef speed, uint16_t *length) {
-    USBD_GetString((uint8_t *)PBDRV_CONFIG_USB_PROD_STR, USBD_StrDesc, length);
-    return USBD_StrDesc;
+    /* Prevent unused argument(s) compilation warning */
+    UNUSED(speed);
+
+    *length = sizeof(pbdrv_usb_str_desc_prod.s);
+    return (uint8_t *)&pbdrv_usb_str_desc_prod;
 }
 
 /**
@@ -205,8 +199,8 @@ static uint8_t *USBD_Pybricks_ManufacturerStrDescriptor(USBD_SpeedTypeDef speed,
     /* Prevent unused argument(s) compilation warning */
     UNUSED(speed);
 
-    USBD_GetString((uint8_t *)PBDRV_CONFIG_USB_MFG_STR, USBD_StrDesc, length);
-    return USBD_StrDesc;
+    *length = sizeof(pbdrv_usb_str_desc_mfg.s);
+    return (uint8_t *)&pbdrv_usb_str_desc_mfg;
 }
 
 /**

lib/pbio/drv/usb/usb_ch9.h

@@ -136,6 +136,10 @@ typedef struct PBDRV_PACKED {
     uint8_t iInterface;
 } pbdrv_usb_iface_desc_t;
 
+// This LangID is used for string descriptors
+// English (United States)
+#define PBDRV_USB_STRING_LANGID_EN_US   0x0409
+
 // Endpoint descriptor
 typedef struct PBDRV_PACKED {
     uint8_t bLength;

lib/pbio/drv/usb/usb_common_desc.c

@@ -93,4 +93,28 @@ const pbdrv_usb_bos_desc_set_union_t pbdrv_usb_bos_desc_set = {
     }
 };
 
+const pbdrv_usb_langid_union_t pbdrv_usb_str_desc_langid = {
+    .s = {
+        .bLength = 4,
+        .bDescriptorType = DESC_TYPE_STRING,
+        .langID = {PBDRV_USB_STRING_LANGID_EN_US},
+    }
+};
+
+const pbdrv_usb_str_mfg_union_t pbdrv_usb_str_desc_mfg = {
+    .s = {
+        .bLength = sizeof(pbdrv_usb_str_mfg_t),
+        .bDescriptorType = DESC_TYPE_STRING,
+        .str = PBDRV_CONFIG_USB_MFG_STR,
+    }
+};
+
+const pbdrv_usb_str_prod_union_t pbdrv_usb_str_desc_prod = {
+    .s = {
+        .bLength = sizeof(pbdrv_usb_str_prod_t),
+        .bDescriptorType = DESC_TYPE_STRING,
+        .str = PBDRV_CONFIG_USB_PROD_STR,
+    }
+};
+
 #endif // PBDRV_CONFIG_USB

lib/pbio/drv/usb/usb_common_desc.h

@@ -9,6 +9,9 @@
 
 #include <pbio/util.h>
 
+#include <lego/usb.h>
+#include "pbdrvconfig.h"
+
 #include "usb_ch9.h"
 
 /**
@@ -56,4 +59,32 @@ PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_bos_desc_set);
 
 extern const pbdrv_usb_bos_desc_set_union_t pbdrv_usb_bos_desc_set;
 
+// (Human-readable) string descriptors
+typedef struct PBDRV_PACKED {
+    uint8_t bLength;
+    uint8_t bDescriptorType;
+    uint16_t langID[1];
+} pbdrv_usb_langid_t;
+PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_langid);
+
+extern const pbdrv_usb_langid_union_t pbdrv_usb_str_desc_langid;
+
+typedef struct PBDRV_PACKED {
+    uint8_t bLength;
+    uint8_t bDescriptorType;
+    uint16_t str[PBIO_ARRAY_SIZE(PBDRV_CONFIG_USB_MFG_STR) - 1];
+} pbdrv_usb_str_mfg_t;
+PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_str_mfg);
+
+extern const pbdrv_usb_str_mfg_union_t pbdrv_usb_str_desc_mfg;
+
+typedef struct PBDRV_PACKED {
+    uint8_t bLength;
+    uint8_t bDescriptorType;
+    uint16_t str[PBIO_ARRAY_SIZE(PBDRV_CONFIG_USB_PROD_STR) - 1];
+} pbdrv_usb_str_prod_t;
+PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_str_prod);
+
+extern const pbdrv_usb_str_prod_union_t pbdrv_usb_str_desc_prod;
+
 #endif // _INTERNAL_PBDRV_USB_COMMON_DESC_H_

lib/pbio/drv/usb/usb_ev3.c

@@ -18,7 +18,6 @@
 #include <pbio/util.h>
 
 #include <lego/usb.h>
-#include "pbdrvconfig.h"
 
 #include <tiam1808/armv5/am1808/interrupt.h>
 #include <tiam1808/cppi41dma.h>
@@ -180,22 +179,8 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_fs = {
     }
 };
 
-typedef struct PBDRV_PACKED {
-    uint8_t bLength;
-    uint8_t bDescriptorType;
-    uint16_t langID[1];
-} pbdrv_usb_langid_t;
-PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_langid);
-
-pbdrv_usb_langid_union_t usb_str_desc_langid = {
-    .s = {
-        .bLength = 4,
-        .bDescriptorType = DESC_TYPE_STRING,
-        .langID = {0x0409},     // English (United States)
-    }
-};
-
-// We generate string descriptors at runtime, so this dynamic buffer is needed
+// We generate a serial number string descriptors at runtime
+// so this dynamic buffer is needed
 #define STRING_DESC_MAX_SZ      64
 static union {
     uint8_t b[STRING_DESC_MAX_SZ];
@@ -456,36 +441,18 @@ static bool usb_get_descriptor(uint16_t wValue) {
         case DESC_TYPE_STRING:
             switch (desc_idx) {
                 case STRING_DESC_LANGID:
-                    pbdrv_usb_setup_data_to_send = usb_str_desc_langid.u;
-                    pbdrv_usb_setup_data_to_send_sz = sizeof(usb_str_desc_langid);
+                    pbdrv_usb_setup_data_to_send = pbdrv_usb_str_desc_langid.u;
+                    pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_str_desc_langid.s);
                     return true;
 
                 case STRING_DESC_MFG:
-                    usb_string_desc_buffer.b[1] = DESC_TYPE_STRING;
-                    i = 0;
-                    while (PBDRV_CONFIG_USB_MFG_STR[i]) {
-                        usb_string_desc_buffer.b[2 + 2 * i] = PBDRV_CONFIG_USB_MFG_STR[i];
-                        usb_string_desc_buffer.b[2 + 2 * i + 1] = 0;
-                        i++;
-                    }
-                    usb_string_desc_buffer.b[0] = 2 * i + 2;
-
-                    pbdrv_usb_setup_data_to_send = usb_string_desc_buffer.u;
-                    pbdrv_usb_setup_data_to_send_sz = usb_string_desc_buffer.b[0];
+                    pbdrv_usb_setup_data_to_send = pbdrv_usb_str_desc_mfg.u;
+                    pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_str_desc_mfg.s);
                     return true;
 
                 case STRING_DESC_PRODUCT:
-                    usb_string_desc_buffer.b[1] = DESC_TYPE_STRING;
-                    i = 0;
-                    while (PBDRV_CONFIG_USB_PROD_STR[i]) {
-                        usb_string_desc_buffer.b[2 + 2 * i] = PBDRV_CONFIG_USB_PROD_STR[i];
-                        usb_string_desc_buffer.b[2 + 2 * i + 1] = 0;
-                        i++;
-                    }
-                    usb_string_desc_buffer.b[0] = 2 * i + 2;
-
-                    pbdrv_usb_setup_data_to_send = usb_string_desc_buffer.u;
-                    pbdrv_usb_setup_data_to_send_sz = usb_string_desc_buffer.b[0];
+                    pbdrv_usb_setup_data_to_send = pbdrv_usb_str_desc_prod.u;
+                    pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_str_desc_prod.s);
                     return true;
 
                 case STRING_DESC_SERIAL:

lib/pbio/drv/usb/usb_nxt.c

@@ -27,6 +27,8 @@
 #include "nxos/drivers/aic.h"
 #include "nxos/util.h"
 
+#include <lego/usb.h>
+
 #include "usb_ch9.h"
 #include "usb_common_desc.h"
 
@@ -86,6 +88,15 @@
 
 #define USB_WVALUE_INDEX       0xFF
 
+/**
+ * Indices for string descriptors
+ */
+enum {
+    STRING_DESC_LANGID,
+    STRING_DESC_MFG,
+    STRING_DESC_PRODUCT,
+};
+
 /* The following definitions are 'raw' USB setup packets. They are all
  * standard responses to various setup requests by the USB host. These
  * packets are all constant, and mostly boilerplate. Don't be too
@@ -110,8 +121,8 @@ static const pbdrv_usb_dev_desc_t pbdrv_usb_nxt_device_descriptor = {
     .idVendor = 0x0694,     /* Vendor ID : LEGO */
     .idProduct = 0x0002,    /* Product ID : NXT */
     .bcdDevice = 0x0200,    /* Product revision: 2.0.0. */
-    .iManufacturer = 1,
-    .iProduct = 2,
+    .iManufacturer = STRING_DESC_MFG,
+    .iProduct = STRING_DESC_PRODUCT,
     .iSerialNumber = 0,     // TODO: implement a serial number
     .bNumConfigurations = 1,
 };
@@ -185,45 +196,6 @@ static const pbdrv_usb_nxt_conf_t pbdrv_usb_nxt_full_config = {
     },
 };
 
-static const uint8_t pbdrv_usb_nxt_string_desc[] = {
-    4, USB_DESC_TYPE_STR, /* Descriptor length and type. */
-    0x09, 0x04, /* Supported language ID (US English). */
-};
-
-static const uint8_t pbdrv_usb_lego_str[] = {
-    10, USB_DESC_TYPE_STR,
-    'L', 0,
-    'E', 0,
-    'G', 0,
-    'O', 0,
-};
-
-static const uint8_t pbdrv_usb_nxt_str[] = {
-    30, USB_DESC_TYPE_STR,
-    'N', 0,
-    'X', 0,
-    'T', 0,
-    ' ', 0,
-    '+', 0,
-    ' ', 0,
-    'P', 0,
-    'y', 0,
-    'b', 0,
-    'r', 0,
-    'i', 0,
-    'c', 0,
-    'k', 0,
-    's', 0,
-};
-
-/* Internal lookup table mapping string descriptors to their indices
- * in the USB string descriptor table.
- */
-static const uint8_t *pbdrv_usb_nxt_strings[] = {
-    pbdrv_usb_lego_str,
-    pbdrv_usb_nxt_str,
-};
-
 typedef enum {
     USB_UNINITIALIZED,
     USB_READY,
@@ -495,17 +467,30 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) {
                     break;
 
                 case USB_DESC_TYPE_STR: /* String or language info. */
-                    if ((packet->value & USB_WVALUE_INDEX) == 0) {
-                        pbdrv_usb_nxt_write_data(0, pbdrv_usb_nxt_string_desc,
-                            MIN(pbdrv_usb_nxt_string_desc[0], packet->length));
+                {
+                    const void *desc = 0;
+                    switch (index) {
+                        case STRING_DESC_LANGID:
+                            desc = &pbdrv_usb_str_desc_langid;
+                            size = sizeof(pbdrv_usb_str_desc_langid.s);
+                            break;
+                        case STRING_DESC_MFG:
+                            desc = &pbdrv_usb_str_desc_mfg;
+                            size = sizeof(pbdrv_usb_str_desc_mfg.s);
+                            break;
+                        case STRING_DESC_PRODUCT:
+                            desc = &pbdrv_usb_str_desc_prod;
+                            size = sizeof(pbdrv_usb_str_desc_prod.s);
+                            break;
+                    }
+
+                    if (desc) {
+                        pbdrv_usb_nxt_write_data(0, desc, MIN(size, packet->length));
                     } else {
-                        /* The host wants a specific string. */
-                        /* TODO: This should check if the requested string exists. */
-                        pbdrv_usb_nxt_write_data(0, pbdrv_usb_nxt_strings[index - 1],
-                            MIN(pbdrv_usb_nxt_strings[index - 1][0],
-                                packet->length));
+                        pbdrv_usb_nxt_send_stall(0);
                     }
-                    break;
+                }
+                break;
 
                 case USB_DESC_TYPE_DEVICE_QUALIFIER: /* Device qualifier descriptor. */
                     size = pbdrv_usb_nxt_dev_qualifier_desc.bLength;

lib/pbio/platform/nxt/pbdrvconfig.h

@@ -50,3 +50,7 @@
 
 #define PBDRV_CONFIG_USB                            (1)
 #define PBDRV_CONFIG_USB_NXT                        (1)
+#define PBDRV_CONFIG_USB_VID                        LEGO_USB_VID
+#define PBDRV_CONFIG_USB_PID                        LEGO_USB_PID_NXT
+#define PBDRV_CONFIG_USB_MFG_STR                    LEGO_USB_MFG_STR
+#define PBDRV_CONFIG_USB_PROD_STR                   LEGO_USB_PROD_STR_NXT " + Pybricks"