From 8ffa7db2ae8af27ab862b901c1f147735b81af4d Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 16:16:14 +0100 Subject: [PATCH 01/10] pbio/drv/usb/stm32_usbd: Use usb_ch9.h structs for descriptors This should be less error-prone compared to manually declaring each byte. --- lib/pbio/drv/usb/stm32_usbd/usbd_desc.c | 49 +++++------ lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c | 97 +++++++++++---------- lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h | 2 - lib/pbio/drv/usb/usb_ch9.h | 8 +- lib/pbio/drv/usb/usb_ev3.c | 8 +- 5 files changed, 84 insertions(+), 80 deletions(-) diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c b/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c index c03afa3ad..b890c01d9 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c +++ b/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c @@ -54,6 +54,8 @@ #include "usbd_conf.h" #include "usbd_pybricks.h" +#include "../usb_ch9.h" + /* Private typedef -----------------------------------------------------------*/ /* Private define ------------------------------------------------------------*/ #define USBD_LANGID_STRING 0x409 @@ -74,29 +76,28 @@ #define USB_SIZ_BOS_DESC (5 + 28 + 24) /* USB Standard Device Descriptor */ -__ALIGN_BEGIN static +static #if !defined(PBDRV_CONFIG_USB_STM32F4_HUB_VARIANT_ADDR) const #endif -uint8_t USBD_DeviceDesc[] __ALIGN_END = { - 0x12, /* bLength */ - USB_DESC_TYPE_DEVICE, /* bDescriptorType */ - 0x10, 0x02, /* bcdUSB = 2.1.0 (for BOS support) */ - PBIO_PYBRICKS_USB_DEVICE_CLASS, /* bDeviceClass */ - PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, /* bDeviceSubClass */ - PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, /* bDeviceProtocol */ - USB_MAX_EP0_SIZE, /* bMaxPacketSize */ - LOBYTE(PBDRV_CONFIG_USB_VID), /* idVendor */ - HIBYTE(PBDRV_CONFIG_USB_VID), /* idVendor */ - LOBYTE(PBDRV_CONFIG_USB_PID), /* idProduct */ - HIBYTE(PBDRV_CONFIG_USB_PID), /* idProduct */ - 0x00, 0x02, /* bcdDevice rel. 2.0.0 */ - USBD_IDX_MFC_STR, /* Index of manufacturer string */ - USBD_IDX_PRODUCT_STR, /* Index of product string */ - USBD_IDX_SERIAL_STR, /* Index of serial number string */ - USBD_MAX_NUM_CONFIGURATION /* bNumConfigurations */ +pbdrv_usb_dev_desc_union_t USBD_DeviceDesc = { + .s = { + .bLength = sizeof(pbdrv_usb_dev_desc_t), + .bDescriptorType = DESC_TYPE_DEVICE, + .bcdUSB = 0x0210, /* 2.1.0 (for BOS support) */ + .bDeviceClass = PBIO_PYBRICKS_USB_DEVICE_CLASS, + .bDeviceSubClass = PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, + .bDeviceProtocol = PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, + .bMaxPacketSize0 = USB_MAX_EP0_SIZE, + .idVendor = PBDRV_CONFIG_USB_VID, + .idProduct = PBDRV_CONFIG_USB_PID, + .bcdDevice = 0x0200, /* rel. 2.0.0 */ + .iManufacturer = USBD_IDX_MFC_STR, + .iProduct = USBD_IDX_PRODUCT_STR, + .iSerialNumber = USBD_IDX_SERIAL_STR, + .bNumConfigurations = USBD_MAX_NUM_CONFIGURATION, + } }; /* USB_DeviceDescriptor */ -_Static_assert(USB_LEN_DEV_DESC == sizeof(USBD_DeviceDesc)); /** BOS descriptor. */ __ALIGN_BEGIN static const uint8_t USBD_BOSDesc[] __ALIGN_END = @@ -316,8 +317,8 @@ static uint8_t *USBD_Pybricks_DeviceDescriptor(USBD_SpeedTypeDef speed, uint16_t /* Prevent unused argument(s) compilation warning */ UNUSED(speed); - *length = sizeof(USBD_DeviceDesc); - return (uint8_t *)USBD_DeviceDesc; + *length = sizeof(USBD_DeviceDesc.s); + return (uint8_t *)&USBD_DeviceDesc; } /** @@ -424,11 +425,9 @@ void USBD_Pybricks_Desc_Init(void) { #ifdef PBDRV_CONFIG_USB_STM32F4_HUB_VARIANT_ADDR #define VARIANT (*(uint32_t *)PBDRV_CONFIG_USB_STM32F4_HUB_VARIANT_ADDR) if (VARIANT == 0) { - USBD_DeviceDesc[10] = LOBYTE(PBDRV_CONFIG_USB_PID_0); - USBD_DeviceDesc[11] = HIBYTE(PBDRV_CONFIG_USB_PID_0); + USBD_DeviceDesc.s.idProduct = PBDRV_CONFIG_USB_PID_0; } else if (VARIANT == 1) { - USBD_DeviceDesc[10] = LOBYTE(PBDRV_CONFIG_USB_PID_1); - USBD_DeviceDesc[11] = HIBYTE(PBDRV_CONFIG_USB_PID_1); + USBD_DeviceDesc.s.idProduct = PBDRV_CONFIG_USB_PID_1; } #endif } diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c index df65c6bf5..a3897328b 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c +++ b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c @@ -42,6 +42,8 @@ #include "usbd_ctlreq.h" #include "usbd_pybricks.h" +#include "../usb_ch9.h" + /** @addtogroup STM32_USB_DEVICE_LIBRARY * @{ @@ -111,49 +113,54 @@ USBD_ClassTypeDef USBD_Pybricks_ClassDriver = }; /* USB Pybricks device Configuration Descriptor */ -__ALIGN_BEGIN static uint8_t USBD_Pybricks_CfgDesc[USBD_PYBRICKS_CONFIG_DESC_SIZ] __ALIGN_END = -{ - /* Configuration Descriptor */ - 0x09, /* bLength: Configuration Descriptor size */ - USB_DESC_TYPE_CONFIGURATION, /* bDescriptorType: Configuration */ - LOBYTE(USBD_PYBRICKS_CONFIG_DESC_SIZ), /* wTotalLength:no of returned bytes */ - HIBYTE(USBD_PYBRICKS_CONFIG_DESC_SIZ), - 0x01, /* bNumInterfaces: 1 interface */ - 0x01, /* bConfigurationValue: Configuration value */ - 0x00, /* iConfiguration: Index of string descriptor describing the configuration */ - 0x80, /* bmAttributes */ - 250, /* MaxPower 500mA (number of 2mA units) */ - - /*---------------------------------------------------------------------------*/ - - /* Data class interface descriptor */ - 0x09, /* bLength: Endpoint Descriptor size */ - USB_DESC_TYPE_INTERFACE, /* bDescriptorType: */ - 0x00, /* bInterfaceNumber: Number of Interface */ - 0x00, /* bAlternateSetting: Alternate setting */ - 0x02, /* bNumEndpoints: Two endpoints used */ - PBIO_PYBRICKS_USB_DEVICE_CLASS, /* bInterfaceClass */ - PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, /* bInterfaceSubClass */ - PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, /* bInterfaceProtocol */ - 0x00, /* iInterface: */ - - /* Endpoint OUT Descriptor */ - 0x07, /* bLength: Endpoint Descriptor size */ - USB_DESC_TYPE_ENDPOINT, /* bDescriptorType: Endpoint */ - USBD_PYBRICKS_OUT_EP, /* bEndpointAddress */ - USBD_EP_TYPE_BULK, /* bmAttributes */ - LOBYTE(USBD_PYBRICKS_MAX_PACKET_SIZE), /* wMaxPacketSize: */ - HIBYTE(USBD_PYBRICKS_MAX_PACKET_SIZE), - 0x00, /* bInterval: ignore for Bulk transfer */ - - /* Endpoint IN Descriptor */ - 0x07, /* bLength: Endpoint Descriptor size */ - USB_DESC_TYPE_ENDPOINT, /* bDescriptorType: Endpoint */ - USBD_PYBRICKS_IN_EP, /* bEndpointAddress */ - USBD_EP_TYPE_BULK, /* bmAttributes */ - LOBYTE(USBD_PYBRICKS_MAX_PACKET_SIZE), /* wMaxPacketSize: */ - HIBYTE(USBD_PYBRICKS_MAX_PACKET_SIZE), - 0x00 /* bInterval */ +typedef struct PBDRV_PACKED { + pbdrv_usb_conf_desc_t conf_desc; + pbdrv_usb_iface_desc_t iface_desc; + pbdrv_usb_ep_desc_t ep_out; + pbdrv_usb_ep_desc_t ep_in; +} pbdrv_usb_stm32_conf_t; +PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_stm32_conf); + +static pbdrv_usb_stm32_conf_union_t USBD_Pybricks_CfgDesc = { + .s = { + .conf_desc = { + .bLength = sizeof(pbdrv_usb_conf_desc_t), + .bDescriptorType = DESC_TYPE_CONFIGURATION, + .wTotalLength = sizeof(pbdrv_usb_stm32_conf_t), + .bNumInterfaces = 1, + .bConfigurationValue = 1, + .iConfiguration = 0, + .bmAttributes = USB_CONF_DESC_BM_ATTR_MUST_BE_SET, + .bMaxPower = 250, /* 500mA (number of 2mA units) */ + }, + .iface_desc = { + .bLength = sizeof(pbdrv_usb_iface_desc_t), + .bDescriptorType = DESC_TYPE_INTERFACE, + .bInterfaceNumber = 0, + .bAlternateSetting = 0, + .bNumEndpoints = 2, + .bInterfaceClass = PBIO_PYBRICKS_USB_DEVICE_CLASS, + .bInterfaceSubClass = PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, + .bInterfaceProtocol = PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, + .iInterface = 0, + }, + .ep_out = { + .bLength = sizeof(pbdrv_usb_ep_desc_t), + .bDescriptorType = DESC_TYPE_ENDPOINT, + .bEndpointAddress = USBD_PYBRICKS_OUT_EP, + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, + .wMaxPacketSize = USBD_PYBRICKS_MAX_PACKET_SIZE, + .bInterval = 0, /* ignore for Bulk transfer */ + }, + .ep_in = { + .bLength = sizeof(pbdrv_usb_ep_desc_t), + .bDescriptorType = DESC_TYPE_ENDPOINT, + .bEndpointAddress = USBD_PYBRICKS_IN_EP, + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, + .wMaxPacketSize = USBD_PYBRICKS_MAX_PACKET_SIZE, + .bInterval = 0, /* ignore for Bulk transfer */ + }, + } }; __ALIGN_BEGIN static const uint8_t WebUSB_DescSet[] __ALIGN_END = @@ -336,8 +343,8 @@ static USBD_StatusTypeDef USBD_Pybricks_Setup(USBD_HandleTypeDef *pdev, * @retval pointer to descriptor buffer */ static uint8_t *USBD_Pybricks_GetCfgDesc(uint16_t *length) { - *length = (uint16_t)sizeof(USBD_Pybricks_CfgDesc); - return USBD_Pybricks_CfgDesc; + *length = (uint16_t)sizeof(USBD_Pybricks_CfgDesc.s); + return (uint8_t *)&USBD_Pybricks_CfgDesc; } /** diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h index c9619cea2..663faf288 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h +++ b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h @@ -47,8 +47,6 @@ enum { #define USBD_WEBUSB_LANDING_PAGE_IDX 1 -#define USBD_PYBRICKS_CONFIG_DESC_SIZ (9 + 9 + 7 + 7) - #define USBD_PYBRICKS_IN_EP 0x81U /* EP1 for data IN */ #define USBD_PYBRICKS_OUT_EP 0x01U /* EP1 for data OUT */ diff --git a/lib/pbio/drv/usb/usb_ch9.h b/lib/pbio/drv/usb/usb_ch9.h index eda7e8ad5..91e928107 100644 --- a/lib/pbio/drv/usb/usb_ch9.h +++ b/lib/pbio/drv/usb/usb_ch9.h @@ -141,10 +141,10 @@ typedef struct PBDRV_PACKED { } pbdrv_usb_ep_desc_t; // Endpoint types for bmAttributes -#define EP_TYPE_CTRL 0 -#define EP_TYPE_ISOC 1 -#define EP_TYPE_BULK 2 -#define EP_TYPE_INTR 3 +#define PBDRV_USB_EP_TYPE_CTRL 0 +#define PBDRV_USB_EP_TYPE_ISOC 1 +#define PBDRV_USB_EP_TYPE_BULK 2 +#define PBDRV_USB_EP_TYPE_INTR 3 // Device qualifier descriptor typedef struct PBDRV_PACKED { diff --git a/lib/pbio/drv/usb/usb_ev3.c b/lib/pbio/drv/usb/usb_ev3.c index d9789ce3d..740068a12 100644 --- a/lib/pbio/drv/usb/usb_ev3.c +++ b/lib/pbio/drv/usb/usb_ev3.c @@ -122,7 +122,7 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_hs = { .bLength = sizeof(pbdrv_usb_ep_desc_t), .bDescriptorType = DESC_TYPE_ENDPOINT, .bEndpointAddress = 0x01, - .bmAttributes = EP_TYPE_BULK, + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, .wMaxPacketSize = PYBRICKS_EP_PKT_SZ_HS, .bInterval = 0, }, @@ -130,7 +130,7 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_hs = { .bLength = sizeof(pbdrv_usb_ep_desc_t), .bDescriptorType = DESC_TYPE_ENDPOINT, .bEndpointAddress = 0x81, - .bmAttributes = EP_TYPE_BULK, + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, .wMaxPacketSize = PYBRICKS_EP_PKT_SZ_HS, .bInterval = 0, }, @@ -164,7 +164,7 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_fs = { .bLength = sizeof(pbdrv_usb_ep_desc_t), .bDescriptorType = DESC_TYPE_ENDPOINT, .bEndpointAddress = 0x01, - .bmAttributes = EP_TYPE_BULK, + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, .wMaxPacketSize = PYBRICKS_EP_PKT_SZ_FS, .bInterval = 0, }, @@ -172,7 +172,7 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_fs = { .bLength = sizeof(pbdrv_usb_ep_desc_t), .bDescriptorType = DESC_TYPE_ENDPOINT, .bEndpointAddress = 0x81, - .bmAttributes = EP_TYPE_BULK, + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, .wMaxPacketSize = PYBRICKS_EP_PKT_SZ_FS, .bInterval = 0, }, From e451d18e9d7ee1c42dc4ba98f5fe0891618269c2 Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 16:29:09 +0100 Subject: [PATCH 02/10] pbio/drv/usb/usb_nxt: Also use usb_ch9.h for descriptors --- lib/pbio/drv/usb/usb_nxt.c | 155 ++++++++++++++++++++----------------- 1 file changed, 83 insertions(+), 72 deletions(-) diff --git a/lib/pbio/drv/usb/usb_nxt.c b/lib/pbio/drv/usb/usb_nxt.c index 5c973fbc0..cbe285af0 100644 --- a/lib/pbio/drv/usb/usb_nxt.c +++ b/lib/pbio/drv/usb/usb_nxt.c @@ -27,6 +27,8 @@ #include "nxos/drivers/aic.h" #include "nxos/util.h" +#include "usb_ch9.h" + /* The USB controller supports up to 4 endpoints. */ #define PBDRV_USB_NXT_N_ENDPOINTS 4 @@ -96,31 +98,33 @@ * don't have a vendor ID to use. Therefore, we are currently * piggybacking on Lego's device space, using an unused product ID. */ -static const uint8_t pbdrv_usb_nxt_device_descriptor[] = { - 18, USB_DESC_TYPE_DEVICE, /* Packet size and type. */ - 0x10, 0x02, /* This packet is USB 2.1 (needed for BOS descriptors). */ - PBIO_PYBRICKS_USB_DEVICE_CLASS, /* Class code. */ - PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, /* Sub class code. */ - PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, /* Device protocol. */ - MAX_EP0_SIZE, /* Maximum packet size for EP0 (control endpoint). */ - 0x94, 0x06, /* Vendor ID : LEGO */ - 0x02, 0x00, /* Product ID : NXT */ - 0x00, 0x02, /* Product revision: 2.0.0. */ - 1, /* Index of the vendor string. */ - 2, /* Index of the product string. */ - 0, /* Index of the serial number (none for us). */ - 1, /* The number of possible configurations. */ +static const pbdrv_usb_dev_desc_t pbdrv_usb_nxt_device_descriptor = { + .bLength = sizeof(pbdrv_usb_dev_desc_t), + .bDescriptorType = DESC_TYPE_DEVICE, + .bcdUSB = 0x0210, /* This packet is USB 2.1 (needed for BOS descriptors). */ + .bDeviceClass = PBIO_PYBRICKS_USB_DEVICE_CLASS, + .bDeviceSubClass = PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, + .bDeviceProtocol = PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, + .bMaxPacketSize0 = MAX_EP0_SIZE, + .idVendor = 0x0694, /* Vendor ID : LEGO */ + .idProduct = 0x0002, /* Product ID : NXT */ + .bcdDevice = 0x0200, /* Product revision: 2.0.0. */ + .iManufacturer = 1, + .iProduct = 2, + .iSerialNumber = 0, + .bNumConfigurations = 1, }; -static const uint8_t pbdrv_usb_nxt_dev_qualifier_desc[] = { - 10, USB_DESC_TYPE_DEVICE_QUALIFIER, /* Packet size and type. */ - 0x10, 0x02, /* This packet is USB 2.1. */ - PBIO_PYBRICKS_USB_DEVICE_CLASS, /* Class code */ - PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, /* Sub class code */ - PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, /* Device protocol */ - MAX_EP0_SIZE, /* Maximum packet size for EP0. */ - 1, /* The number of possible configurations. */ - 0, /* Reserved for future use, must be zero. */ +static const pbdrv_usb_dev_qualifier_desc_t pbdrv_usb_nxt_dev_qualifier_desc = { + .bLength = sizeof(pbdrv_usb_dev_qualifier_desc_t), + .bDescriptorType = DESC_TYPE_DEVICE_QUALIFIER, + .bcdUSB = 0x0210, /* This packet is USB 2.1. */ + .bDeviceClass = PBIO_PYBRICKS_USB_DEVICE_CLASS, + .bDeviceSubClass = PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, + .bDeviceProtocol = PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, + .bMaxPacketSize0 = MAX_EP0_SIZE, + .bNumConfigurations = 1, + .bReserved = 0, }; // These enumerations are specific to the configuration of this device. @@ -181,54 +185,61 @@ static const uint8_t pbdrv_usb_nxt_bos_desc[] = { 0x00, /* bAltEnumCode = Does not support alternate enumeration */ }; -static const uint8_t pbdrv_usb_nxt_full_config[] = { - 0x09, USB_DESC_TYPE_CONFIG, /* Descriptor size and type. */ - 0x20, 0x00, /* Total length of the configuration, interface - * description included. - */ - 1, /* The number of interfaces declared by this configuration. */ - 1, /* The ID for this configuration. */ - 0, /* Index of the configuration description string (none). */ - - /* Configuration attributes bitmap. Bit 7 (MSB) must be 1, bit 6 is - * 1 because the NXT is self-powered, bit 5 is 0 because the NXT - * doesn't support remote wakeup, and bits 0-4 are 0 (reserved). - */ - 0xC0, - 0, /* Device power consumption, for non self-powered devices. */ - - /* - * This is the descriptor for the interface associated with the - * configuration. - */ - 0x09, USB_DESC_TYPE_INT, /* Descriptor size and type. */ - 0x00, /* Interface index. */ - 0x00, /* ID for this interface configuration. */ - 0x02, /* The number of endpoints defined by this interface - * (excluding EP0). - */ - PBIO_PYBRICKS_USB_DEVICE_CLASS, /* Interface class ("Vendor specific"). */ - PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, /* Interface subclass (see above). */ - PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, /* Interface protocol (see above). */ - 0x00, /* Index of the string descriptor for this interface (none). */ - +typedef struct PBDRV_PACKED { + pbdrv_usb_conf_desc_t conf_desc; + pbdrv_usb_iface_desc_t iface_desc; + pbdrv_usb_ep_desc_t ep_out; + pbdrv_usb_ep_desc_t ep_in; +} pbdrv_usb_nxt_conf_t; + +static const pbdrv_usb_nxt_conf_t pbdrv_usb_nxt_full_config = { + .conf_desc = { + .bLength = sizeof(pbdrv_usb_conf_desc_t), + .bDescriptorType = DESC_TYPE_CONFIGURATION, + .wTotalLength = sizeof(pbdrv_usb_nxt_conf_t), + .bNumInterfaces = 1, + .bConfigurationValue = 1, + .iConfiguration = 0, + /* Configuration attributes bitmap. Bit 7 (MSB) must be 1, bit 6 is + * 1 because the NXT is self-powered, bit 5 is 0 because the NXT + * doesn't support remote wakeup, and bits 0-4 are 0 (reserved). + */ + .bmAttributes = USB_CONF_DESC_BM_ATTR_MUST_BE_SET | USB_CONF_DESC_BM_ATTR_SELF_POWERED, + .bMaxPower = 0, + }, + .iface_desc = { + .bLength = sizeof(pbdrv_usb_iface_desc_t), + .bDescriptorType = DESC_TYPE_INTERFACE, + .bInterfaceNumber = 0, + .bAlternateSetting = 0, + .bNumEndpoints = 2, + .bInterfaceClass = PBIO_PYBRICKS_USB_DEVICE_CLASS, + .bInterfaceSubClass = PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, + .bInterfaceProtocol = PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, + .iInterface = 0, + }, /* * Descriptor for EP1. */ - 7, USB_DESC_TYPE_ENDPT, /* Descriptor length and type. */ - 0x1, /* Endpoint number. MSB is zero, meaning this is an OUT EP. */ - 0x2, /* Endpoint type (bulk). */ - MAX_RCV_SIZE, 0x00, /* Maximum packet size (64). */ - 0, /* EP maximum NAK rate (device never NAKs). */ - + .ep_out = { + .bLength = sizeof(pbdrv_usb_ep_desc_t), + .bDescriptorType = DESC_TYPE_ENDPOINT, + .bEndpointAddress = 0x01, /* Endpoint number. MSB is zero, meaning this is an OUT EP. */ + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, + .wMaxPacketSize = MAX_RCV_SIZE, + .bInterval = 0, + }, /* * Descriptor for EP2. */ - 7, USB_DESC_TYPE_ENDPT, /* Descriptor length and type. */ - 0x82, /* Endpoint number. MSB is one, meaning this is an IN EP. */ - 0x2, /* Endpoint type (bulk). */ - MAX_RCV_SIZE, 0x00, /* Maximum packet size (64). */ - 0, /* EP maximum NAK rate (device never NAKs). */ + .ep_in = { + .bLength = sizeof(pbdrv_usb_ep_desc_t), + .bDescriptorType = DESC_TYPE_ENDPOINT, + .bEndpointAddress = 0x82, /* Endpoint number. MSB is one, meaning this is an IN EP. */ + .bmAttributes = PBDRV_USB_EP_TYPE_BULK, + .wMaxPacketSize = MAX_SND_SIZE, + .bInterval = 0, + }, }; static const uint8_t pbdrv_usb_nxt_string_desc[] = { @@ -625,17 +636,17 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { index = (packet->value & USB_WVALUE_INDEX); switch ((packet->value & USB_WVALUE_TYPE) >> 8) { case USB_DESC_TYPE_DEVICE: /* Device descriptor */ - size = pbdrv_usb_nxt_device_descriptor[0]; - pbdrv_usb_nxt_write_data(0, pbdrv_usb_nxt_device_descriptor, + size = sizeof(pbdrv_usb_nxt_device_descriptor); + pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_nxt_device_descriptor, MIN(size, packet->length)); break; case USB_DESC_TYPE_CONFIG: /* Configuration descriptor */ - pbdrv_usb_nxt_write_data(0, pbdrv_usb_nxt_full_config, - MIN(pbdrv_usb_nxt_full_config[2], packet->length)); + pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_nxt_full_config, + MIN(sizeof(pbdrv_usb_nxt_full_config), packet->length)); /* TODO: Why? This is not specified in the USB specs. */ - if (pbdrv_usb_nxt_full_config[2] < packet->length) { + if (pbdrv_usb_nxt_full_config.conf_desc.wTotalLength < packet->length) { pbdrv_usb_nxt_send_null(); } break; @@ -654,8 +665,8 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { break; case USB_DESC_TYPE_DEVICE_QUALIFIER: /* Device qualifier descriptor. */ - size = pbdrv_usb_nxt_dev_qualifier_desc[0]; - pbdrv_usb_nxt_write_data(0, pbdrv_usb_nxt_dev_qualifier_desc, + size = pbdrv_usb_nxt_dev_qualifier_desc.bLength; + pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_nxt_dev_qualifier_desc, MIN(size, packet->length)); break; From 709dff897c61e73f10935ac99e25b64f7df36947 Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 18:27:59 +0100 Subject: [PATCH 03/10] pbio/drv/usb/usb_ch9.h: Add WebUSB/Microsoft descriptors Although these are not standard USB descriptors, Pybricks does require them, and they logically fit in with the rest of the descriptor definitions in this file. --- lib/pbio/drv/usb/usb_ch9.h | 127 +++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/lib/pbio/drv/usb/usb_ch9.h b/lib/pbio/drv/usb/usb_ch9.h index 91e928107..0323adf97 100644 --- a/lib/pbio/drv/usb/usb_ch9.h +++ b/lib/pbio/drv/usb/usb_ch9.h @@ -80,6 +80,12 @@ PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_setup_packet) #define DESC_TYPE_DEVICE_QUALIFIER 6 #define DESC_TYPE_OTHER_SPEED_CONFIGURATION 7 #define DESC_TYPE_INTERFACE_POWER 8 +// Binary Object Store, originally from the Wireless USB / USB 3.0 spec +// These descriptors have been backported to USB 2 and are available to +// devices which indicate a bcdDevice of at least 2.1. +// They are used for WebUSB and Microsoft WinUSB driver installation. +#define DESC_TYPE_BOS 15 +#define DESC_TYPE_DEVICE_CAPABILITY 16 // Device descriptor typedef struct PBDRV_PACKED { @@ -160,4 +166,125 @@ typedef struct PBDRV_PACKED { } pbdrv_usb_dev_qualifier_desc_t; PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_dev_qualifier_desc); +// Binary Object Store +typedef struct PBDRV_PACKED { + uint8_t bLength; + uint8_t bDescriptorType; + uint16_t wTotalLength; + uint8_t bNumDeviceCaps; +} pbdrv_usb_bos_desc_t; + +typedef struct PBDRV_PACKED { + uint32_t u0; + uint16_t u1; + uint16_t u2; + uint8_t u3[8]; +} pbdrv_usb_uuid_t; + +// Platform-specific capabilities +#define USB_DEVICE_CAPABILITY_TYPE_PLATFORM 5 + +typedef struct PBDRV_PACKED { + uint8_t bLength; + uint8_t bDescriptorType; + uint8_t bDevCapabilityType; + uint8_t bReserved; + pbdrv_usb_uuid_t uuid; +} pbdrv_usb_platform_caps_common_t; + +typedef struct PBDRV_PACKED { + pbdrv_usb_platform_caps_common_t hdr; + uint16_t bcdVersion; + uint8_t bVendorCode; + uint8_t iLandingPage; +} pbdrv_usb_webusb_capability_t; + +// {3408b638-09a9-47a0-8bfd-a0768815b665} +#define USB_PLATFORM_CAP_GUID_WEBUSB { \ + .u0 = 0x3408b638, \ + .u1 = 0x09a9, \ + .u2 = 0x47a0, \ + .u3 = {0x8b, 0xfd, 0xa0, 0x76, 0x88, 0x15, 0xb6, 0x65}, \ +} + +typedef struct PBDRV_PACKED { + pbdrv_usb_platform_caps_common_t hdr; + uint32_t dwWindowsVersion; + uint16_t wMSOSDescriptorSetTotalLength; + uint8_t bMS_VendorCode; + uint8_t bAltEnumCode; +} pbdrv_usb_microsoft_20_capability_t; + +// {D8DD60DF-4589-4CC7-9CD2-659D9E648A9F} +#define USB_PLATFORM_CAP_GUID_MS_20 { \ + .u0 = 0xD8DD60DF, \ + .u1 = 0x4589, \ + .u2 = 0x4CC7, \ + .u3 = {0x9C, 0xD2, 0x65, 0x9D, 0x9E, 0x64, 0x8A, 0x9F}, \ +} + +// These are not technically part of USB Chapter 9, but +// we need them and they logically fit with the rest of the structs. + +#define WEBUSB_REQ_GET_URL 2 +#define WEBUSB_DESC_TYPE_URL 3 +#define WEBUSB_URL_SCHEME_HTTP 0 +#define WEBUSB_URL_SCHEME_HTTPS 1 + +typedef struct PBDRV_PACKED { + uint8_t bLength; + uint8_t bDescriptorType; + uint8_t bScheme; + uint8_t url[]; +} pbdrv_usb_webusb_url_desc_t; +PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_webusb_url_desc); + +// Microsoft wIndex values +#define MS_OS_20_DESCRIPTOR_INDEX 7 +#define MS_OS_20_SET_ALT_ENUMERATION 8 + +// Microsoft descriptor types +#define MS_OS_20_SET_HEADER_DESCRIPTOR 0 +#define MS_OS_20_SUBSET_HEADER_CONFIGURATION 1 +#define MS_OS_20_SUBSET_HEADER_FUNCTION 2 +#define MS_OS_20_FEATURE_COMPATBLE_ID 3 +#define MS_OS_20_FEATURE_REG_PROPERTY 4 +#define MS_OS_20_FEATURE_MIN_RESUME_TIME 5 +#define MS_OS_20_FEATURE_MODEL_ID 6 +#define MS_OS_20_FEATURE_CCGP_DEVICE 7 +#define MS_OS_20_FEATURE_VENDOR_REVISION 8 + +// We only need (and thus define) the following descriptor types + +typedef struct PBDRV_PACKED { + uint16_t wLength; + uint16_t wDescriptorType; + uint32_t dwWindowsVersion; + uint16_t wTotalLength; +} pbdrv_usb_ms_20_desc_set_header_t; + +typedef struct PBDRV_PACKED { + uint16_t wLength; + uint16_t wDescriptorType; + uint8_t CompatibleID[8]; + uint8_t SubCompatibleID[8]; +} pbdrv_usb_ms_20_compatible_t; + +typedef struct PBDRV_PACKED { + uint16_t wLength; + uint16_t wDescriptorType; + uint16_t wPropertyDataType; +} pbdrv_usb_ms_20_reg_prop_hdr_t; + +#define MS_OS_20_REG_PROP_TYPE_REG_SZ 1 +#define MS_OS_20_REG_PROP_TYPE_REG_EXPAND_SZ 2 +#define MS_OS_20_REG_PROP_TYPE_REG_BINARY 3 +#define MS_OS_20_REG_PROP_TYPE_REG_DWORD_LE 4 +#define MS_OS_20_REG_PROP_TYPE_REG_DWORD_BE 5 +#define MS_OS_20_REG_PROP_TYPE_REG_LINK 6 +#define MS_OS_20_REG_PROP_TYPE_REG_MULTI_SZ 7 + +// This is the minimum Windows version needed to support these descriptors +#define MS_WINDOWS_VERSION_81 0x06030000 + #endif // _INTERNAL_PBDRV_USB_CH9_H_ From ab5de202b71d9cd9c48ddab5ba079e4c805e15c5 Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 18:29:33 +0100 Subject: [PATCH 04/10] pbio/drv/usb/usb_ev3.c: Add WebUSB and Microsoft descriptors This allows for more friendly device setup when connecting to a computer. --- lib/pbio/drv/usb/usb_ev3.c | 321 +++++++++++++++++++++++++++---------- 1 file changed, 232 insertions(+), 89 deletions(-) diff --git a/lib/pbio/drv/usb/usb_ev3.c b/lib/pbio/drv/usb/usb_ev3.c index 740068a12..8835abdc4 100644 --- a/lib/pbio/drv/usb/usb_ev3.c +++ b/lib/pbio/drv/usb/usb_ev3.c @@ -51,6 +51,16 @@ enum { STRING_DESC_SERIAL, }; +/** + * Indices for vendor requests, which are used for platform descriptors + */ +enum { + VENDOR_REQ_WEBUSB, + VENDOR_REQ_MS_20, +}; + +#define WEBUSB_LANDING_PAGE_URL_IDX 1 + // Begin hardcoded USB descriptors static const pbdrv_usb_dev_desc_union_t dev_desc = { @@ -179,6 +189,113 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_fs = { } }; +#if PBIO_VERSION_LEVEL_HEX == 0xA +#define PYBRICKS_CODE_URL "alpha.pybricks.com" +#elif PBIO_VERSION_LEVEL_HEX == 0xB +#define PYBRICKS_CODE_URL "beta.pybricks.com" +#else +#define PYBRICKS_CODE_URL "code.pybricks.com" +#endif + +static const pbdrv_usb_webusb_url_desc_union_t webusb_landing_page = { + .s = { + .bLength = sizeof(pbdrv_usb_webusb_url_desc_t) + sizeof(PYBRICKS_CODE_URL) - 1, + .bDescriptorType = WEBUSB_DESC_TYPE_URL, + .bScheme = WEBUSB_URL_SCHEME_HTTPS, + .url = PYBRICKS_CODE_URL, + }, +}; + +#define PBDRV_USB_MS_20_REG_PROPERTY_NAME u"DeviceInterfaceGUIDs" +#define PBDRV_USB_MS_20_REG_PROPERTY_VAL u"{A5C44A4C-53D4-4389-9821-AE95051908A1}" + +#define MS_20_REGISTRY_DATA_EXTRA_SZ \ + 2 + sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME) + \ + 2 + sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL) + \ + 2 + +typedef struct PBDRV_PACKED { + pbdrv_usb_ms_20_desc_set_header_t desc_set_hdr; + pbdrv_usb_ms_20_compatible_t compatible; + pbdrv_usb_ms_20_reg_prop_hdr_t reg_prop_hdr; + uint16_t property_name_len; + uint16_t device_interface_guids[sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME) / 2]; + uint16_t property_val_len; + uint16_t device_interface_guid_val[sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL) / 2]; + uint16_t _multi_sz_end; /* Terminates a REG_MULTI_SZ list */ +} pbdrv_usb_ev3_ms_20_desc_set_t; +PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_ev3_ms_20_desc_set); + +static const pbdrv_usb_ev3_ms_20_desc_set_union_t ms_20_desc_set = { + .s = { + .desc_set_hdr = { + .wLength = sizeof(pbdrv_usb_ms_20_desc_set_header_t), + .wDescriptorType = MS_OS_20_SET_HEADER_DESCRIPTOR, + .dwWindowsVersion = MS_WINDOWS_VERSION_81, + .wTotalLength = sizeof(pbdrv_usb_ev3_ms_20_desc_set_t), + }, + .compatible = { + .wLength = sizeof(pbdrv_usb_ms_20_compatible_t), + .wDescriptorType = MS_OS_20_FEATURE_COMPATBLE_ID, + .CompatibleID = "WINUSB", + .SubCompatibleID = "", + }, + .reg_prop_hdr = { + .wLength = sizeof(pbdrv_usb_ms_20_reg_prop_hdr_t) + MS_20_REGISTRY_DATA_EXTRA_SZ, + .wDescriptorType = MS_OS_20_FEATURE_REG_PROPERTY, + .wPropertyDataType = MS_OS_20_REG_PROP_TYPE_REG_MULTI_SZ, + }, + .property_name_len = sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME), + .device_interface_guids = PBDRV_USB_MS_20_REG_PROPERTY_NAME, + .property_val_len = sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL), + .device_interface_guid_val = PBDRV_USB_MS_20_REG_PROPERTY_VAL, + } +}; + +typedef struct PBDRV_PACKED { + pbdrv_usb_bos_desc_t bos; + // WebUSB must occur before Microsoft OS descriptors + pbdrv_usb_webusb_capability_t webusb; + pbdrv_usb_microsoft_20_capability_t ms_20; +} pbdrv_usb_ev3_bos_desc_set_t; +PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_ev3_bos_desc_set); + +static const pbdrv_usb_ev3_bos_desc_set_union_t bos_desc_set = { + .s = { + .bos = { + .bLength = sizeof(pbdrv_usb_bos_desc_t), + .bDescriptorType = DESC_TYPE_BOS, + .wTotalLength = sizeof(pbdrv_usb_ev3_bos_desc_set_t), + .bNumDeviceCaps = 2, + }, + .webusb = { + .hdr = { + .bLength = sizeof(pbdrv_usb_webusb_capability_t), + .bDescriptorType = DESC_TYPE_DEVICE_CAPABILITY, + .bDevCapabilityType = USB_DEVICE_CAPABILITY_TYPE_PLATFORM, + .bReserved = 0, + .uuid = USB_PLATFORM_CAP_GUID_WEBUSB, + }, + .bcdVersion = 0x0100, + .bVendorCode = VENDOR_REQ_WEBUSB, + .iLandingPage = WEBUSB_LANDING_PAGE_URL_IDX, + }, + .ms_20 = { + .hdr = { + .bLength = sizeof(pbdrv_usb_microsoft_20_capability_t), + .bDescriptorType = DESC_TYPE_DEVICE_CAPABILITY, + .bDevCapabilityType = USB_DEVICE_CAPABILITY_TYPE_PLATFORM, + .bReserved = 0, + .uuid = USB_PLATFORM_CAP_GUID_MS_20, + }, + .dwWindowsVersion = MS_WINDOWS_VERSION_81, + .wMSOSDescriptorSetTotalLength = sizeof(pbdrv_usb_ev3_ms_20_desc_set_t), + .bMS_VendorCode = VENDOR_REQ_MS_20, + .bAltEnumCode = 0, + } + } +}; + typedef struct PBDRV_PACKED { uint8_t bLength; uint8_t bDescriptorType; @@ -502,6 +619,11 @@ static bool usb_get_descriptor(uint16_t wValue) { return true; } break; + + case DESC_TYPE_BOS: + pbdrv_usb_setup_data_to_send = bos_desc_set.u; + pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_ev3_bos_desc_set_t); + return true; } return false; @@ -563,121 +685,142 @@ static void usb_device_intr(void) { setup_pkt.u[1] = HWREG(USB0_BASE + USB_O_FIFO0); HWREGH(USB0_BASE + USB_O_CSRL0) = USB_CSRL0_RXRDYC; - if ((setup_pkt.s.bmRequestType & BM_REQ_TYPE_MASK) == BM_REQ_TYPE_STANDARD) { - switch (setup_pkt.s.bmRequestType & BM_REQ_RECIP_MASK) { - case BM_REQ_RECIP_DEV: - switch (setup_pkt.s.bRequest) { - case SET_ADDRESS: - pbdrv_usb_addr = setup_pkt.s.wValue; - pbdrv_usb_addr_needs_setting = true; - handled = true; - break; + switch (setup_pkt.s.bmRequestType & BM_REQ_TYPE_MASK) { + case BM_REQ_TYPE_STANDARD: + switch (setup_pkt.s.bmRequestType & BM_REQ_RECIP_MASK) { + case BM_REQ_RECIP_DEV: + switch (setup_pkt.s.bRequest) { + case SET_ADDRESS: + pbdrv_usb_addr = setup_pkt.s.wValue; + pbdrv_usb_addr_needs_setting = true; + handled = true; + break; - case SET_CONFIGURATION: - if (setup_pkt.s.wValue <= 1) { - pbdrv_usb_config = setup_pkt.s.wValue; + case SET_CONFIGURATION: + if (setup_pkt.s.wValue <= 1) { + pbdrv_usb_config = setup_pkt.s.wValue; - if (pbdrv_usb_config == 1) { - // configuring + if (pbdrv_usb_config == 1) { + // configuring - // Reset data toggle, clear stall, flush fifo - HWREGB(USB0_BASE + USB_O_TXCSRL1) = USB_TXCSRL1_CLRDT | USB_TXCSRL1_FLUSH; - HWREGB(USB0_BASE + USB_O_RXCSRL1) = USB_RXCSRL1_CLRDT | USB_RXCSRL1_FLUSH; - } else { - // deconfiguring + // Reset data toggle, clear stall, flush fifo + HWREGB(USB0_BASE + USB_O_TXCSRL1) = USB_TXCSRL1_CLRDT | USB_TXCSRL1_FLUSH; + HWREGB(USB0_BASE + USB_O_RXCSRL1) = USB_RXCSRL1_CLRDT | USB_RXCSRL1_FLUSH; + } else { + // deconfiguring - // Set stall condition - HWREGB(USB0_BASE + USB_O_TXCSRL1) = USB_TXCSRL1_STALL; - HWREGB(USB0_BASE + USB_O_RXCSRL1) = USB_RXCSRL1_STALL; + // Set stall condition + HWREGB(USB0_BASE + USB_O_TXCSRL1) = USB_TXCSRL1_STALL; + HWREGB(USB0_BASE + USB_O_RXCSRL1) = USB_RXCSRL1_STALL; + } + handled = true; } - handled = true; - } - break; - - case GET_CONFIGURATION: - pbdrv_usb_setup_misc_tx_byte = pbdrv_usb_config; - pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; - pbdrv_usb_setup_data_to_send_sz = 1; - handled = true; - break; - - case GET_STATUS: - pbdrv_usb_setup_misc_tx_byte = 1; // self-powered - pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; - pbdrv_usb_setup_data_to_send_sz = 2; - handled = true; - break; - - case GET_DESCRIPTOR: - if (usb_get_descriptor(setup_pkt.s.wValue)) { - handled = true; - } - break; - } - break; + break; - case BM_REQ_RECIP_IF: - if (setup_pkt.s.wIndex == 0) { - switch (setup_pkt.s.bRequest) { - case GET_INTERFACE: - pbdrv_usb_setup_misc_tx_byte = 0; + case GET_CONFIGURATION: + pbdrv_usb_setup_misc_tx_byte = pbdrv_usb_config; pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; pbdrv_usb_setup_data_to_send_sz = 1; handled = true; break; case GET_STATUS: - pbdrv_usb_setup_misc_tx_byte = 0; + pbdrv_usb_setup_misc_tx_byte = 1; // self-powered pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; pbdrv_usb_setup_data_to_send_sz = 2; handled = true; break; - } - } - break; - - case BM_REQ_RECIP_EP: - switch (setup_pkt.s.bRequest) { - case GET_STATUS: - if (setup_pkt.s.wIndex == 1) { - pbdrv_usb_setup_misc_tx_byte = !!(HWREGB(USB0_BASE + USB_O_RXCSRL1) & USB_RXCSRL1_STALL); - pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; - pbdrv_usb_setup_data_to_send_sz = 2; - handled = true; - } else if (setup_pkt.s.wIndex == 0x81) { - pbdrv_usb_setup_misc_tx_byte = !!(HWREGB(USB0_BASE + USB_O_TXCSRL1) & USB_TXCSRL1_STALL); - pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; - pbdrv_usb_setup_data_to_send_sz = 2; - handled = true; - } - break; - case CLEAR_FEATURE: - if (setup_pkt.s.wValue == 0) { - if (setup_pkt.s.wIndex == 1) { - HWREGB(USB0_BASE + USB_O_RXCSRL1) &= ~USB_RXCSRL1_STALL; - handled = true; - } else if (setup_pkt.s.wIndex == 0x81) { - HWREGB(USB0_BASE + USB_O_TXCSRL1) &= ~USB_TXCSRL1_STALL; + case GET_DESCRIPTOR: + if (usb_get_descriptor(setup_pkt.s.wValue)) { handled = true; } + break; + } + break; + + case BM_REQ_RECIP_IF: + if (setup_pkt.s.wIndex == 0) { + switch (setup_pkt.s.bRequest) { + case GET_INTERFACE: + pbdrv_usb_setup_misc_tx_byte = 0; + pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; + pbdrv_usb_setup_data_to_send_sz = 1; + handled = true; + break; + + case GET_STATUS: + pbdrv_usb_setup_misc_tx_byte = 0; + pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; + pbdrv_usb_setup_data_to_send_sz = 2; + handled = true; + break; } - break; + } + break; - case SET_FEATURE: - if (setup_pkt.s.wValue == 0) { + case BM_REQ_RECIP_EP: + switch (setup_pkt.s.bRequest) { + case GET_STATUS: if (setup_pkt.s.wIndex == 1) { - HWREGB(USB0_BASE + USB_O_RXCSRL1) |= USB_RXCSRL1_STALL; + pbdrv_usb_setup_misc_tx_byte = !!(HWREGB(USB0_BASE + USB_O_RXCSRL1) & USB_RXCSRL1_STALL); + pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; + pbdrv_usb_setup_data_to_send_sz = 2; handled = true; } else if (setup_pkt.s.wIndex == 0x81) { - HWREGB(USB0_BASE + USB_O_TXCSRL1) |= USB_TXCSRL1_STALL; + pbdrv_usb_setup_misc_tx_byte = !!(HWREGB(USB0_BASE + USB_O_TXCSRL1) & USB_TXCSRL1_STALL); + pbdrv_usb_setup_data_to_send = &pbdrv_usb_setup_misc_tx_byte; + pbdrv_usb_setup_data_to_send_sz = 2; handled = true; } - } - break; - } - break; - } + break; + + case CLEAR_FEATURE: + if (setup_pkt.s.wValue == 0) { + if (setup_pkt.s.wIndex == 1) { + HWREGB(USB0_BASE + USB_O_RXCSRL1) &= ~USB_RXCSRL1_STALL; + handled = true; + } else if (setup_pkt.s.wIndex == 0x81) { + HWREGB(USB0_BASE + USB_O_TXCSRL1) &= ~USB_TXCSRL1_STALL; + handled = true; + } + } + break; + + case SET_FEATURE: + if (setup_pkt.s.wValue == 0) { + if (setup_pkt.s.wIndex == 1) { + HWREGB(USB0_BASE + USB_O_RXCSRL1) |= USB_RXCSRL1_STALL; + handled = true; + } else if (setup_pkt.s.wIndex == 0x81) { + HWREGB(USB0_BASE + USB_O_TXCSRL1) |= USB_TXCSRL1_STALL; + handled = true; + } + } + break; + } + break; + } + break; + + case BM_REQ_TYPE_VENDOR: + switch (setup_pkt.s.bRequest) { + case VENDOR_REQ_WEBUSB: + if (setup_pkt.s.wIndex == WEBUSB_REQ_GET_URL && setup_pkt.s.wValue == WEBUSB_LANDING_PAGE_URL_IDX) { + pbdrv_usb_setup_data_to_send = webusb_landing_page.u; + pbdrv_usb_setup_data_to_send_sz = webusb_landing_page.s.bLength; + handled = true; + } + break; + + case VENDOR_REQ_MS_20: + if (setup_pkt.s.wIndex == MS_OS_20_DESCRIPTOR_INDEX) { + pbdrv_usb_setup_data_to_send = ms_20_desc_set.u; + pbdrv_usb_setup_data_to_send_sz = ms_20_desc_set.s.desc_set_hdr.wTotalLength; + handled = true; + } + break; + } } if (!handled) { From b12bd2aa81a790d18cfca41e0ed7f055c0556a13 Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 18:57:52 +0100 Subject: [PATCH 05/10] pbio/drv/usb: Deduplicate common descriptors These descriptors are expected to be the same on all devices, so moving them to a new file reduces duplicate code. --- bricks/_common/sources.mk | 1 + lib/pbio/drv/usb/stm32_usbd/usbd_desc.c | 155 +---------------- lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c | 36 +--- lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h | 14 -- lib/pbio/drv/usb/usb_common_desc.c | 96 +++++++++++ lib/pbio/drv/usb/usb_common_desc.h | 59 +++++++ lib/pbio/drv/usb/usb_ev3.c | 136 ++------------- lib/pbio/drv/usb/usb_nxt.c | 177 +------------------- 8 files changed, 186 insertions(+), 488 deletions(-) create mode 100644 lib/pbio/drv/usb/usb_common_desc.c create mode 100644 lib/pbio/drv/usb/usb_common_desc.h diff --git a/bricks/_common/sources.mk b/bricks/_common/sources.mk index 6503a9f09..421abfde7 100644 --- a/bricks/_common/sources.mk +++ b/bricks/_common/sources.mk @@ -186,6 +186,7 @@ PBIO_SRC_C = $(addprefix lib/pbio/,\ drv/uart/uart_stm32f0.c \ drv/uart/uart_stm32f4_ll_irq.c \ drv/uart/uart_stm32l4_ll_dma.c \ + drv/usb/usb_common_desc.c \ drv/usb/usb_ev3.c \ drv/usb/usb_nxt.c \ drv/usb/usb_stm32.c \ diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c b/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c index b890c01d9..a636c29c3 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c +++ b/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c @@ -55,6 +55,7 @@ #include "usbd_pybricks.h" #include "../usb_ch9.h" +#include "../usb_common_desc.h" /* Private typedef -----------------------------------------------------------*/ /* Private define ------------------------------------------------------------*/ @@ -73,7 +74,6 @@ // descriptor sizes #define USB_SIZ_STRING_SERIAL 26 -#define USB_SIZ_BOS_DESC (5 + 28 + 24) /* USB Standard Device Descriptor */ static @@ -99,155 +99,6 @@ pbdrv_usb_dev_desc_union_t USBD_DeviceDesc = { } }; /* USB_DeviceDescriptor */ -/** BOS descriptor. */ -__ALIGN_BEGIN static const uint8_t USBD_BOSDesc[] __ALIGN_END = -{ - 5, /* bLength */ - USB_DESC_TYPE_BOS, /* bDescriptorType = BOS */ - LOBYTE(USB_SIZ_BOS_DESC), /* wTotalLength */ - HIBYTE(USB_SIZ_BOS_DESC), /* wTotalLength */ - 2, /* bNumDeviceCaps */ - - // IMPORTANT: The WebUSB descriptor must be first to make Chromium happy. - - 24, /* bLength */ - USB_DEVICE_CAPABITY_TYPE, /* bDescriptorType = Device Capability */ - USB_DEV_CAP_TYPE_PLATFORM, /* bDevCapabilityType */ - 0x00, /* bReserved */ - - /* - * PlatformCapabilityUUID - * WebUSB Platform Capability descriptor - * 3408B638-09A9-47A0-8BFD-A0768815B665 - * RFC 4122 explains the correct byte ordering - */ - 0x38, 0xB6, 0x08, 0x34, /* 32-bit value */ - 0xA9, 0x09, /* 16-bit value */ - 0xA0, 0x47, /* 16-bit value */ - 0x8B, 0xFD, - 0xA0, 0x76, 0x88, 0x15, 0xB6, 0x65, - - LOBYTE(0x0100), /* bcdVersion */ - HIBYTE(0x0100), /* bcdVersion */ - USBD_VENDOR_CODE_WEBUSB, /* bVendorCode */ - USBD_WEBUSB_LANDING_PAGE_IDX, /* iLandingPage */ - - // IMPORTANT: The MS OS 2.0 descriptor must be last to make Chromium happy. - - 28, /* bLength */ - USB_DEVICE_CAPABITY_TYPE, /* bDescriptorType = Device Capability */ - USB_DEV_CAP_TYPE_PLATFORM, /* bDevCapabilityType */ - 0x00, /* bReserved */ - - /* - * PlatformCapabilityUUID - * Microsoft OS 2.0 descriptor platform capability ID - * D8DD60DF-4589-4CC7-9CD2-659D9E648A9F - * RFC 4122 explains the correct byte ordering - */ - 0xDF, 0x60, 0xDD, 0xD8, /* 32-bit value */ - 0x89, 0x45, /* 16-bit value */ - 0xC7, 0x4C, /* 16-bit value */ - 0x9C, 0xD2, - 0x65, 0x9D, 0x9E, 0x64, 0x8A, 0x9F, - - 0x00, 0x00, 0x03, 0x06, /* dwWindowsVersion = 0x06030000 for Windows 8.1 Build */ - LOBYTE(USBD_SIZ_MS_OS_DSCRPTR_SET), /* wMSOSDescriptorSetTotalLength */ - HIBYTE(USBD_SIZ_MS_OS_DSCRPTR_SET), /* wMSOSDescriptorSetTotalLength */ - USBD_VENDOR_CODE_MS, /* bMS_VendorCode */ - 0x00, /* bAltEnumCode = Does not support alternate enumeration */ -}; -_Static_assert(USB_SIZ_BOS_DESC == sizeof(USBD_BOSDesc)); - -__ALIGN_BEGIN const uint8_t USBD_OSDescSet[] __ALIGN_END = -{ - 0x0A, 0x00, /* wLength = 10 */ - 0x00, 0x00, /* wDescriptorType = MS_OS_20_SET_HEADER_DESCRIPTOR */ - 0x00, 0x00, 0x03, 0x06, /* dwWindowsVersion = 0x06030000 for Windows 8.1 Build */ - LOBYTE(USBD_SIZ_MS_OS_DSCRPTR_SET), /* wTotalLength */ - HIBYTE(USBD_SIZ_MS_OS_DSCRPTR_SET), /* wTotalLength (cont.) */ - - 0x14, 0x00, /* wLength = 20 */ - 0x03, 0x00, /* wDescriptorType = MS_OS_20_FEATURE_COMPATBLE_ID */ - 'W', 'I', 'N', 'U', 'S', 'B', /* CompatibleID */ - 0x00, 0x00, /* CompatibleID (cont.) */ - 0x00, 0x00, 0x00, 0x00, /* SubCompatibleID */ - 0x00, 0x00, 0x00, 0x00, /* SubCompatibleID (cont.) */ - - 0x84, 0x00, /* wLength = 132 */ - 0x04, 0x00, /* wDescriptorType = MS_OS_20_FEATURE_REG_PROPERTY */ - 0x07, 0x00, /* wStringType = REG_MULTI_SZ */ - /* wPropertyNameLength = 42 */ - 0x2A, 0x00, - /* PropertyName = DeviceInterfaceGUIDs */ - 'D', '\0', - 'e', '\0', - 'v', '\0', - 'i', '\0', - 'c', '\0', - 'e', '\0', - 'I', '\0', - 'n', '\0', - 't', '\0', - 'e', '\0', - 'r', '\0', - 'f', '\0', - 'a', '\0', - 'c', '\0', - 'e', '\0', - 'G', '\0', - 'U', '\0', - 'I', '\0', - 'D', '\0', - 's', '\0', - '\0', '\0', - - /* wPropertyDataLength = 80 */ - 0x50, 0x00, - /* PropertyData = {A5C44A4C-53D4-4389-9821-AE95051908A1} */ - '{', '\0', - 'A', '\0', - '5', '\0', - 'C', '\0', - '4', '\0', - '4', '\0', - 'A', '\0', - '4', '\0', - 'C', '\0', - '-', '\0', - '5', '\0', - '3', '\0', - 'D', '\0', - '4', '\0', - '-', '\0', - '4', '\0', - '3', '\0', - '8', '\0', - '9', '\0', - '-', '\0', - '9', '\0', - '8', '\0', - '2', '\0', - '1', '\0', - '-', '\0', - 'A', '\0', - 'E', '\0', - '9', '\0', - '5', '\0', - '0', '\0', - '5', '\0', - '1', '\0', - '9', '\0', - '0', '\0', - '8', '\0', - 'A', '\0', - '1', '\0', - '}', '\0', - '\0', '\0', - '\0', '\0' -}; -_Static_assert(USBD_SIZ_MS_OS_DSCRPTR_SET == sizeof(USBD_OSDescSet)); - /* USB Standard Device Descriptor */ __ALIGN_BEGIN static const uint8_t USBD_LangIDDesc[USB_LEN_LANGID_STR_DESC] __ALIGN_END = { USB_LEN_LANGID_STR_DESC, @@ -404,8 +255,8 @@ static uint8_t *USBD_Pybricks_BOSDescriptor(USBD_SpeedTypeDef speed, uint16_t *l /* Prevent unused argument(s) compilation warning */ UNUSED(speed); - *length = USB_SIZ_BOS_DESC; - return (uint8_t *)USBD_BOSDesc; + *length = sizeof(pbdrv_usb_bos_desc_set.s); + return (uint8_t *)&pbdrv_usb_bos_desc_set; } USBD_DescriptorsTypeDef USBD_Pybricks_Desc = { diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c index a3897328b..f550db0ec 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c +++ b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.c @@ -43,6 +43,7 @@ #include "usbd_pybricks.h" #include "../usb_ch9.h" +#include "../usb_common_desc.h" /** @addtogroup STM32_USB_DEVICE_LIBRARY @@ -163,27 +164,6 @@ static pbdrv_usb_stm32_conf_union_t USBD_Pybricks_CfgDesc = { } }; -__ALIGN_BEGIN static const uint8_t WebUSB_DescSet[] __ALIGN_END = -{ - #if PBIO_VERSION_LEVEL_HEX == 0xA - 21, /* bLength */ - #else - 20, /* bLength */ - #endif - 0x03, /* bDescriptorType = URL */ - 0x01, /* bScheme = https:// */ - - /* URL */ - #if PBIO_VERSION_LEVEL_HEX == 0xA - 'a', 'l', 'p', 'h', 'a', - #elif PBIO_VERSION_LEVEL_HEX == 0xB - 'b', 'e', 't', 'a', - #else - 'c', 'o', 'd', 'e', - #endif - '.', 'p', 'y', 'b', 'r', 'i', 'c', 'k', 's', '.', 'c', 'o', 'm' -}; - /** * @} */ @@ -268,17 +248,17 @@ static USBD_StatusTypeDef USBD_Pybricks_Setup(USBD_HandleTypeDef *pdev, case USB_REQ_TYPE_VENDOR: switch (req->bRequest) { - case USBD_VENDOR_CODE_MS: + case PBDRV_USB_VENDOR_REQ_MS_20: (void)USBD_CtlSendData(pdev, - (uint8_t *)USBD_OSDescSet, - MIN(sizeof(USBD_OSDescSet), req->wLength)); + (uint8_t *)&pbdrv_usb_ms_20_desc_set, + MIN(sizeof(pbdrv_usb_ms_20_desc_set.s), req->wLength)); break; - case USBD_VENDOR_CODE_WEBUSB: - if ((req->wValue == USBD_WEBUSB_LANDING_PAGE_IDX) && (req->wIndex == 0x02)) { + case PBDRV_USB_VENDOR_REQ_WEBUSB: + if ((req->wValue == PBDRV_USB_WEBUSB_LANDING_PAGE_URL_IDX) && (req->wIndex == WEBUSB_REQ_GET_URL)) { (void)USBD_CtlSendData(pdev, - (uint8_t *)WebUSB_DescSet, - MIN(sizeof(WebUSB_DescSet), req->wLength)); + (uint8_t *)&pbdrv_usb_webusb_landing_page, + MIN(pbdrv_usb_webusb_landing_page.s.bLength, req->wLength)); } break; diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h index 663faf288..042e89c84 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h +++ b/lib/pbio/drv/usb/stm32_usbd/usbd_pybricks.h @@ -36,17 +36,6 @@ extern "C" { * @{ */ - -/** @defgroup USBD_Pybricks_Exported_Defines - * @{ - */ -enum { - USBD_VENDOR_CODE_WEBUSB, - USBD_VENDOR_CODE_MS -}; - -#define USBD_WEBUSB_LANDING_PAGE_IDX 1 - #define USBD_PYBRICKS_IN_EP 0x81U /* EP1 for data IN */ #define USBD_PYBRICKS_OUT_EP 0x01U /* EP1 for data OUT */ @@ -120,9 +109,6 @@ typedef struct extern USBD_ClassTypeDef USBD_Pybricks_ClassDriver; -#define USBD_SIZ_MS_OS_DSCRPTR_SET (10 + 20 + 132) -extern const uint8_t USBD_OSDescSet[USBD_SIZ_MS_OS_DSCRPTR_SET]; - /** * @} */ diff --git a/lib/pbio/drv/usb/usb_common_desc.c b/lib/pbio/drv/usb/usb_common_desc.c new file mode 100644 index 000000000..f4e862131 --- /dev/null +++ b/lib/pbio/drv/usb/usb_common_desc.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2025 The Pybricks Authors + +// Descriptors which are shared across devices +// (i.e. WebUSB, Microsoft OS descriptors) + +#include + +#if PBDRV_CONFIG_USB + +#include "usb_common_desc.h" + +#if PBIO_VERSION_LEVEL_HEX == 0xA +#define PYBRICKS_CODE_URL "labs.pybricks.com" +#elif PBIO_VERSION_LEVEL_HEX == 0xB +#define PYBRICKS_CODE_URL "beta.pybricks.com" +#else +#define PYBRICKS_CODE_URL "code.pybricks.com" +#endif + +const pbdrv_usb_webusb_url_desc_union_t pbdrv_usb_webusb_landing_page = { + .s = { + .bLength = sizeof(pbdrv_usb_webusb_url_desc_t) + sizeof(PYBRICKS_CODE_URL) - 1, + .bDescriptorType = WEBUSB_DESC_TYPE_URL, + .bScheme = WEBUSB_URL_SCHEME_HTTPS, + .url = PYBRICKS_CODE_URL, + }, +}; + +#define MS_20_REGISTRY_DATA_EXTRA_SZ \ + 2 + sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME) + \ + 2 + sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL) + +const pbdrv_usb_ms_20_desc_set_union_t pbdrv_usb_ms_20_desc_set = { + .s = { + .desc_set_hdr = { + .wLength = sizeof(pbdrv_usb_ms_20_desc_set_header_t), + .wDescriptorType = MS_OS_20_SET_HEADER_DESCRIPTOR, + .dwWindowsVersion = MS_WINDOWS_VERSION_81, + .wTotalLength = sizeof(pbdrv_usb_ms_20_desc_set_t), + }, + .compatible = { + .wLength = sizeof(pbdrv_usb_ms_20_compatible_t), + .wDescriptorType = MS_OS_20_FEATURE_COMPATBLE_ID, + .CompatibleID = "WINUSB", + .SubCompatibleID = "", + }, + .reg_prop_hdr = { + .wLength = sizeof(pbdrv_usb_ms_20_reg_prop_hdr_t) + MS_20_REGISTRY_DATA_EXTRA_SZ, + .wDescriptorType = MS_OS_20_FEATURE_REG_PROPERTY, + .wPropertyDataType = MS_OS_20_REG_PROP_TYPE_REG_MULTI_SZ, + }, + .property_name_len = sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME), + .device_interface_guids = PBDRV_USB_MS_20_REG_PROPERTY_NAME, + .property_val_len = sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL), + .device_interface_guid_val = PBDRV_USB_MS_20_REG_PROPERTY_VAL, + } +}; + +const pbdrv_usb_bos_desc_set_union_t pbdrv_usb_bos_desc_set = { + .s = { + .bos = { + .bLength = sizeof(pbdrv_usb_bos_desc_t), + .bDescriptorType = DESC_TYPE_BOS, + .wTotalLength = sizeof(pbdrv_usb_bos_desc_set_t), + .bNumDeviceCaps = 2, + }, + .webusb = { + .hdr = { + .bLength = sizeof(pbdrv_usb_webusb_capability_t), + .bDescriptorType = DESC_TYPE_DEVICE_CAPABILITY, + .bDevCapabilityType = USB_DEVICE_CAPABILITY_TYPE_PLATFORM, + .bReserved = 0, + .uuid = USB_PLATFORM_CAP_GUID_WEBUSB, + }, + .bcdVersion = 0x0100, + .bVendorCode = PBDRV_USB_VENDOR_REQ_WEBUSB, + .iLandingPage = PBDRV_USB_WEBUSB_LANDING_PAGE_URL_IDX, + }, + .ms_20 = { + .hdr = { + .bLength = sizeof(pbdrv_usb_microsoft_20_capability_t), + .bDescriptorType = DESC_TYPE_DEVICE_CAPABILITY, + .bDevCapabilityType = USB_DEVICE_CAPABILITY_TYPE_PLATFORM, + .bReserved = 0, + .uuid = USB_PLATFORM_CAP_GUID_MS_20, + }, + .dwWindowsVersion = MS_WINDOWS_VERSION_81, + .wMSOSDescriptorSetTotalLength = sizeof(pbdrv_usb_ms_20_desc_set_t), + .bMS_VendorCode = PBDRV_USB_VENDOR_REQ_MS_20, + .bAltEnumCode = 0, + } + } +}; + +#endif // PBDRV_CONFIG_USB diff --git a/lib/pbio/drv/usb/usb_common_desc.h b/lib/pbio/drv/usb/usb_common_desc.h new file mode 100644 index 000000000..15e4c7cde --- /dev/null +++ b/lib/pbio/drv/usb/usb_common_desc.h @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2025 The Pybricks Authors + +// Descriptors which are shared across devices +// (i.e. WebUSB, Microsoft OS descriptors) + +#ifndef _INTERNAL_PBDRV_USB_COMMON_DESC_H_ +#define _INTERNAL_PBDRV_USB_COMMON_DESC_H_ + +#include + +#include "usb_ch9.h" + +/** + * USB vendor requests which will be used to retrieve WebUSB and Microsoft descriptors + */ +enum { + PBDRV_USB_VENDOR_REQ_WEBUSB, + PBDRV_USB_VENDOR_REQ_MS_20, +}; + +// The descriptor index for the WebUSB landing page +#define PBDRV_USB_WEBUSB_LANDING_PAGE_URL_IDX 1 + +#define PBDRV_USB_MS_20_REG_PROPERTY_NAME u"DeviceInterfaceGUIDs" +// This UUID has been generated by Pybricks and identifies specifically +// our devices which use the Pybricks protocol. Windows APIs can match on this. +// An extra null terminator is required to terminate a REG_MULTI_SZ +#define PBDRV_USB_MS_20_REG_PROPERTY_VAL u"{A5C44A4C-53D4-4389-9821-AE95051908A1}\x00" + +// Complete set of Microsoft USB descriptors +typedef struct PBDRV_PACKED { + pbdrv_usb_ms_20_desc_set_header_t desc_set_hdr; + pbdrv_usb_ms_20_compatible_t compatible; + pbdrv_usb_ms_20_reg_prop_hdr_t reg_prop_hdr; + uint16_t property_name_len; + uint16_t device_interface_guids[PBIO_ARRAY_SIZE(PBDRV_USB_MS_20_REG_PROPERTY_NAME)]; + uint16_t property_val_len; + uint16_t device_interface_guid_val[PBIO_ARRAY_SIZE(PBDRV_USB_MS_20_REG_PROPERTY_VAL)]; +} pbdrv_usb_ms_20_desc_set_t; +PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_ms_20_desc_set); + +extern const pbdrv_usb_ms_20_desc_set_union_t pbdrv_usb_ms_20_desc_set; + +// WebUSB descriptor +extern const pbdrv_usb_webusb_url_desc_union_t pbdrv_usb_webusb_landing_page; + +// Complete set of USB BOS descriptors +typedef struct PBDRV_PACKED { + pbdrv_usb_bos_desc_t bos; + // WebUSB must occur before Microsoft OS descriptors + pbdrv_usb_webusb_capability_t webusb; + pbdrv_usb_microsoft_20_capability_t ms_20; +} pbdrv_usb_bos_desc_set_t; +PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_bos_desc_set); + +extern const pbdrv_usb_bos_desc_set_union_t pbdrv_usb_bos_desc_set; + +#endif // _INTERNAL_PBDRV_USB_COMMON_DESC_H_ diff --git a/lib/pbio/drv/usb/usb_ev3.c b/lib/pbio/drv/usb/usb_ev3.c index 8835abdc4..a9d7b27a8 100644 --- a/lib/pbio/drv/usb/usb_ev3.c +++ b/lib/pbio/drv/usb/usb_ev3.c @@ -35,6 +35,7 @@ #include #include "usb_ch9.h" +#include "usb_common_desc.h" // Maximum packet sizes for the USB pipes #define EP0_BUF_SZ 64 @@ -51,16 +52,6 @@ enum { STRING_DESC_SERIAL, }; -/** - * Indices for vendor requests, which are used for platform descriptors - */ -enum { - VENDOR_REQ_WEBUSB, - VENDOR_REQ_MS_20, -}; - -#define WEBUSB_LANDING_PAGE_URL_IDX 1 - // Begin hardcoded USB descriptors static const pbdrv_usb_dev_desc_union_t dev_desc = { @@ -189,113 +180,6 @@ static const pbdrv_usb_ev3_conf_1_union_t configuration_1_desc_fs = { } }; -#if PBIO_VERSION_LEVEL_HEX == 0xA -#define PYBRICKS_CODE_URL "alpha.pybricks.com" -#elif PBIO_VERSION_LEVEL_HEX == 0xB -#define PYBRICKS_CODE_URL "beta.pybricks.com" -#else -#define PYBRICKS_CODE_URL "code.pybricks.com" -#endif - -static const pbdrv_usb_webusb_url_desc_union_t webusb_landing_page = { - .s = { - .bLength = sizeof(pbdrv_usb_webusb_url_desc_t) + sizeof(PYBRICKS_CODE_URL) - 1, - .bDescriptorType = WEBUSB_DESC_TYPE_URL, - .bScheme = WEBUSB_URL_SCHEME_HTTPS, - .url = PYBRICKS_CODE_URL, - }, -}; - -#define PBDRV_USB_MS_20_REG_PROPERTY_NAME u"DeviceInterfaceGUIDs" -#define PBDRV_USB_MS_20_REG_PROPERTY_VAL u"{A5C44A4C-53D4-4389-9821-AE95051908A1}" - -#define MS_20_REGISTRY_DATA_EXTRA_SZ \ - 2 + sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME) + \ - 2 + sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL) + \ - 2 - -typedef struct PBDRV_PACKED { - pbdrv_usb_ms_20_desc_set_header_t desc_set_hdr; - pbdrv_usb_ms_20_compatible_t compatible; - pbdrv_usb_ms_20_reg_prop_hdr_t reg_prop_hdr; - uint16_t property_name_len; - uint16_t device_interface_guids[sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME) / 2]; - uint16_t property_val_len; - uint16_t device_interface_guid_val[sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL) / 2]; - uint16_t _multi_sz_end; /* Terminates a REG_MULTI_SZ list */ -} pbdrv_usb_ev3_ms_20_desc_set_t; -PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_ev3_ms_20_desc_set); - -static const pbdrv_usb_ev3_ms_20_desc_set_union_t ms_20_desc_set = { - .s = { - .desc_set_hdr = { - .wLength = sizeof(pbdrv_usb_ms_20_desc_set_header_t), - .wDescriptorType = MS_OS_20_SET_HEADER_DESCRIPTOR, - .dwWindowsVersion = MS_WINDOWS_VERSION_81, - .wTotalLength = sizeof(pbdrv_usb_ev3_ms_20_desc_set_t), - }, - .compatible = { - .wLength = sizeof(pbdrv_usb_ms_20_compatible_t), - .wDescriptorType = MS_OS_20_FEATURE_COMPATBLE_ID, - .CompatibleID = "WINUSB", - .SubCompatibleID = "", - }, - .reg_prop_hdr = { - .wLength = sizeof(pbdrv_usb_ms_20_reg_prop_hdr_t) + MS_20_REGISTRY_DATA_EXTRA_SZ, - .wDescriptorType = MS_OS_20_FEATURE_REG_PROPERTY, - .wPropertyDataType = MS_OS_20_REG_PROP_TYPE_REG_MULTI_SZ, - }, - .property_name_len = sizeof(PBDRV_USB_MS_20_REG_PROPERTY_NAME), - .device_interface_guids = PBDRV_USB_MS_20_REG_PROPERTY_NAME, - .property_val_len = sizeof(PBDRV_USB_MS_20_REG_PROPERTY_VAL), - .device_interface_guid_val = PBDRV_USB_MS_20_REG_PROPERTY_VAL, - } -}; - -typedef struct PBDRV_PACKED { - pbdrv_usb_bos_desc_t bos; - // WebUSB must occur before Microsoft OS descriptors - pbdrv_usb_webusb_capability_t webusb; - pbdrv_usb_microsoft_20_capability_t ms_20; -} pbdrv_usb_ev3_bos_desc_set_t; -PBDRV_USB_TYPE_PUNNING_HELPER(pbdrv_usb_ev3_bos_desc_set); - -static const pbdrv_usb_ev3_bos_desc_set_union_t bos_desc_set = { - .s = { - .bos = { - .bLength = sizeof(pbdrv_usb_bos_desc_t), - .bDescriptorType = DESC_TYPE_BOS, - .wTotalLength = sizeof(pbdrv_usb_ev3_bos_desc_set_t), - .bNumDeviceCaps = 2, - }, - .webusb = { - .hdr = { - .bLength = sizeof(pbdrv_usb_webusb_capability_t), - .bDescriptorType = DESC_TYPE_DEVICE_CAPABILITY, - .bDevCapabilityType = USB_DEVICE_CAPABILITY_TYPE_PLATFORM, - .bReserved = 0, - .uuid = USB_PLATFORM_CAP_GUID_WEBUSB, - }, - .bcdVersion = 0x0100, - .bVendorCode = VENDOR_REQ_WEBUSB, - .iLandingPage = WEBUSB_LANDING_PAGE_URL_IDX, - }, - .ms_20 = { - .hdr = { - .bLength = sizeof(pbdrv_usb_microsoft_20_capability_t), - .bDescriptorType = DESC_TYPE_DEVICE_CAPABILITY, - .bDevCapabilityType = USB_DEVICE_CAPABILITY_TYPE_PLATFORM, - .bReserved = 0, - .uuid = USB_PLATFORM_CAP_GUID_MS_20, - }, - .dwWindowsVersion = MS_WINDOWS_VERSION_81, - .wMSOSDescriptorSetTotalLength = sizeof(pbdrv_usb_ev3_ms_20_desc_set_t), - .bMS_VendorCode = VENDOR_REQ_MS_20, - .bAltEnumCode = 0, - } - } -}; - typedef struct PBDRV_PACKED { uint8_t bLength; uint8_t bDescriptorType; @@ -621,8 +505,8 @@ static bool usb_get_descriptor(uint16_t wValue) { break; case DESC_TYPE_BOS: - pbdrv_usb_setup_data_to_send = bos_desc_set.u; - pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_ev3_bos_desc_set_t); + pbdrv_usb_setup_data_to_send = pbdrv_usb_bos_desc_set.u; + pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_bos_desc_set.s); return true; } @@ -805,18 +689,18 @@ static void usb_device_intr(void) { case BM_REQ_TYPE_VENDOR: switch (setup_pkt.s.bRequest) { - case VENDOR_REQ_WEBUSB: - if (setup_pkt.s.wIndex == WEBUSB_REQ_GET_URL && setup_pkt.s.wValue == WEBUSB_LANDING_PAGE_URL_IDX) { - pbdrv_usb_setup_data_to_send = webusb_landing_page.u; - pbdrv_usb_setup_data_to_send_sz = webusb_landing_page.s.bLength; + case PBDRV_USB_VENDOR_REQ_WEBUSB: + if (setup_pkt.s.wIndex == WEBUSB_REQ_GET_URL && setup_pkt.s.wValue == PBDRV_USB_WEBUSB_LANDING_PAGE_URL_IDX) { + pbdrv_usb_setup_data_to_send = pbdrv_usb_webusb_landing_page.u; + pbdrv_usb_setup_data_to_send_sz = pbdrv_usb_webusb_landing_page.s.bLength; handled = true; } break; - case VENDOR_REQ_MS_20: + case PBDRV_USB_VENDOR_REQ_MS_20: if (setup_pkt.s.wIndex == MS_OS_20_DESCRIPTOR_INDEX) { - pbdrv_usb_setup_data_to_send = ms_20_desc_set.u; - pbdrv_usb_setup_data_to_send_sz = ms_20_desc_set.s.desc_set_hdr.wTotalLength; + pbdrv_usb_setup_data_to_send = pbdrv_usb_ms_20_desc_set.u; + pbdrv_usb_setup_data_to_send_sz = sizeof(pbdrv_usb_ms_20_desc_set.s); handled = true; } break; diff --git a/lib/pbio/drv/usb/usb_nxt.c b/lib/pbio/drv/usb/usb_nxt.c index cbe285af0..e5f2b1642 100644 --- a/lib/pbio/drv/usb/usb_nxt.c +++ b/lib/pbio/drv/usb/usb_nxt.c @@ -28,6 +28,7 @@ #include "nxos/util.h" #include "usb_ch9.h" +#include "usb_common_desc.h" /* The USB controller supports up to 4 endpoints. */ #define PBDRV_USB_NXT_N_ENDPOINTS 4 @@ -127,64 +128,6 @@ static const pbdrv_usb_dev_qualifier_desc_t pbdrv_usb_nxt_dev_qualifier_desc = { .bReserved = 0, }; -// These enumerations are specific to the configuration of this device. - -enum { - PBDRV_USB_NXT_VENDOR_CODE_WEBUSB, - PBDRV_USB_NXT_VENDOR_CODE_MS, -}; - -// NB: Chromium seems quite particular about the order of these descriptors. -// The WebUSB descriptor must come first and the MS OS 2.0 descriptor be last. -static const uint8_t pbdrv_usb_nxt_bos_desc[] = { - 5, USB_DESC_TYPE_BOS, /* Descriptor length and type. */ - 0x39, 0x00, /* Total length of the descriptor = 57. */ - 2, /* Number of device capabilities. */ - - 24, /* bLength */ - USB_DEVICE_CAPABILITY_TYPE, /* bDescriptorType = Device Capability */ - USB_DEV_CAP_TYPE_PLATFORM, /* bDevCapabilityType */ - 0x00, /* bReserved */ - - /* - * PlatformCapabilityUUID - * WebUSB Platform Capability descriptor - * 3408B638-09A9-47A0-8BFD-A0768815B665 - * RFC 4122 explains the correct byte ordering - */ - 0x38, 0xB6, 0x08, 0x34, /* 32-bit value */ - 0xA9, 0x09, /* 16-bit value */ - 0xA0, 0x47, /* 16-bit value */ - 0x8B, 0xFD, - 0xA0, 0x76, 0x88, 0x15, 0xB6, 0x65, - - 0x00, 0x01, /* bcdVersion = 1.00 */ - PBDRV_USB_NXT_VENDOR_CODE_WEBUSB, /* bVendorCode */ - 1, /* iLandingPage */ - - 28, /* bLength */ - USB_DEVICE_CAPABILITY_TYPE, /* bDescriptorType = Device Capability */ - USB_DEV_CAP_TYPE_PLATFORM, /* bDevCapabilityType */ - 0x00, /* bReserved */ - - /* - * PlatformCapabilityUUID - * Microsoft OS 2.0 descriptor platform capability ID - * D8DD60DF-4589-4CC7-9CD2-659D9E648A9F - * RFC 4122 explains the correct byte ordering - */ - 0xDF, 0x60, 0xDD, 0xD8, /* 32-bit value */ - 0x89, 0x45, /* 16-bit value */ - 0xC7, 0x4C, /* 16-bit value */ - 0x9C, 0xD2, - 0x65, 0x9D, 0x9E, 0x64, 0x8A, 0x9F, - - 0x00, 0x00, 0x03, 0x06, /* dwWindowsVersion = 0x06030000 for Windows 8.1 Build */ - 0xA2, 0x00, /* wMSOSDescriptorSetTotalLength = 162 */ - PBDRV_USB_NXT_VENDOR_CODE_MS, /* bMS_VendorCode */ - 0x00, /* bAltEnumCode = Does not support alternate enumeration */ -}; - typedef struct PBDRV_PACKED { pbdrv_usb_conf_desc_t conf_desc; pbdrv_usb_iface_desc_t iface_desc; @@ -464,108 +407,6 @@ static void pbdrv_usb_nxt_send_null(void) { pbdrv_usb_nxt_write_data(0, NULL, 0); } -static const uint8_t pbdrv_usb_desc_set_ms_os[] = { - 0x0A, 0x00, /* wLength = 10 */ - 0x00, 0x00, /* wDescriptorType = MS_OS_20_SET_HEADER_DESCRIPTOR */ - 0x00, 0x00, 0x03, 0x06, /* dwWindowsVersion = 0x06030000 for Windows 8.1 Build */ - 0xA2, 0x00, /* wTotalLength = 162 */ - - 0x14, 0x00, /* wLength = 20 */ - 0x03, 0x00, /* wDescriptorType = MS_OS_20_FEATURE_COMPATBLE_ID */ - 'W', 'I', 'N', 'U', 'S', 'B', /* CompatibleID */ - 0x00, 0x00, /* CompatibleID (cont.) */ - 0x00, 0x00, 0x00, 0x00, /* SubCompatibleID */ - 0x00, 0x00, 0x00, 0x00, /* SubCompatibleID (cont.) */ - - 0x84, 0x00, /* wLength = 132 */ - 0x04, 0x00, /* wDescriptorType = MS_OS_20_FEATURE_REG_PROPERTY */ - 0x07, 0x00, /* wStringType = REG_MULTI_SZ */ - /* wPropertyNameLength = 42 */ - 0x2A, 0x00, - /* PropertyName = DeviceInterfaceGUIDs */ - 'D', '\0', - 'e', '\0', - 'v', '\0', - 'i', '\0', - 'c', '\0', - 'e', '\0', - 'I', '\0', - 'n', '\0', - 't', '\0', - 'e', '\0', - 'r', '\0', - 'f', '\0', - 'a', '\0', - 'c', '\0', - 'e', '\0', - 'G', '\0', - 'U', '\0', - 'I', '\0', - 'D', '\0', - 's', '\0', - '\0', '\0', - - /* wPropertyDataLength = 80 */ - 0x50, 0x00, - /* PropertyData = {A5C44A4C-53D4-4389-9821-AE95051908A1} */ - '{', '\0', - 'A', '\0', - '5', '\0', - 'C', '\0', - '4', '\0', - '4', '\0', - 'A', '\0', - '4', '\0', - 'C', '\0', - '-', '\0', - '5', '\0', - '3', '\0', - 'D', '\0', - '4', '\0', - '-', '\0', - '4', '\0', - '3', '\0', - '8', '\0', - '9', '\0', - '-', '\0', - '9', '\0', - '8', '\0', - '2', '\0', - '1', '\0', - '-', '\0', - 'A', '\0', - 'E', '\0', - '9', '\0', - '5', '\0', - '0', '\0', - '5', '\0', - '1', '\0', - '9', '\0', - '0', '\0', - '8', '\0', - 'A', '\0', - '1', '\0', - '}', '\0', - '\0', '\0', - '\0', '\0', -}; - -static const uint8_t pbdrv_usb_desc_set_webusb[] = { - 20, /* bLength */ - 0x03, /* bDescriptorType = URL */ - 0x01, /* bScheme = https:// */ - - /* URL */ - #if PBIO_VERSION_LEVEL_HEX == 0xA - 'a', 'l', 'p', 'h', 'a', - #elif PBIO_VERSION_LEVEL_HEX == 0xB - 'b', 'e', 't', 'a', - #else - 'c', 'o', 'd', 'e', - #endif - '.', 'p', 'y', 'b', 'r', 'i', 'c', 'k', 's', '.', 'c', 'o', 'm', -}; - typedef struct { uint8_t request_attrs; /* Request characteristics. */ uint8_t request; /* Request type. */ @@ -671,8 +512,8 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { break; case USB_DESC_TYPE_BOS: /* BOS descriptor */ - size = pbdrv_usb_nxt_bos_desc[2]; - pbdrv_usb_nxt_write_data(0, pbdrv_usb_nxt_bos_desc, MIN(size, packet->length)); + size = sizeof(pbdrv_usb_bos_desc_set.s); + pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_bos_desc_set, MIN(size, packet->length)); break; default: /* Unknown descriptor, tell the host by stalling. */ @@ -817,15 +658,15 @@ static uint32_t pbdrv_usb_nxt_manage_setup_packet(void) { break; case USB_BMREQUEST_TYPE_VENDOR: switch (packet.request) { - case PBDRV_USB_NXT_VENDOR_CODE_WEBUSB: + case PBDRV_USB_VENDOR_REQ_WEBUSB: // Since there is only one WebUSB descriptor, we ignore the index. - pbdrv_usb_nxt_write_data(0, pbdrv_usb_desc_set_webusb, - MIN(sizeof(pbdrv_usb_desc_set_webusb), packet.length)); + pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_webusb_landing_page, + MIN(pbdrv_usb_webusb_landing_page.s.bLength, packet.length)); break; - case PBDRV_USB_NXT_VENDOR_CODE_MS: + case PBDRV_USB_VENDOR_REQ_MS_20: // Since there is only one MS descriptor, we ignore the index. - pbdrv_usb_nxt_write_data(0, pbdrv_usb_desc_set_ms_os, - MIN(sizeof(pbdrv_usb_desc_set_ms_os), packet.length)); + pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_ms_20_desc_set, + MIN(sizeof(pbdrv_usb_ms_20_desc_set.s), packet.length)); break; default: pbdrv_usb_nxt_send_stall(0); From ac1b2637dc282f32b8d97540f485174464a2b8ba Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 22:30:36 +0100 Subject: [PATCH 06/10] pbio/drv/usb/usb_nxt.c: Minor cleanups This changes pbdrv_usb_nxt_write_data to accept a void *, which removes a lot of ugly casting. It also adds a note that a serial number should be implemented in the future --- lib/pbio/drv/usb/usb_nxt.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/pbio/drv/usb/usb_nxt.c b/lib/pbio/drv/usb/usb_nxt.c index e5f2b1642..718665c8e 100644 --- a/lib/pbio/drv/usb/usb_nxt.c +++ b/lib/pbio/drv/usb/usb_nxt.c @@ -112,7 +112,7 @@ static const pbdrv_usb_dev_desc_t pbdrv_usb_nxt_device_descriptor = { .bcdDevice = 0x0200, /* Product revision: 2.0.0. */ .iManufacturer = 1, .iProduct = 2, - .iSerialNumber = 0, + .iSerialNumber = 0, // TODO: implement a serial number .bNumConfigurations = 1, }; @@ -303,7 +303,8 @@ static void pbdrv_usb_nxt_csr_set_flag(uint8_t endpoint, uint32_t flags) { * single USB packet, the data is split and scheduled to be sent in * several packets. */ -static void pbdrv_usb_nxt_write_data(int endpoint, const uint8_t *ptr, uint32_t length) { +static void pbdrv_usb_nxt_write_data(int endpoint, const void *ptr_, uint32_t length) { + const uint8_t *ptr = ptr_; uint32_t packet_size; int tx; @@ -438,7 +439,7 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { response = 0; } - pbdrv_usb_nxt_write_data(0, (uint8_t *)&response, 2); + pbdrv_usb_nxt_write_data(0, &response, 2); } break; @@ -478,13 +479,14 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { switch ((packet->value & USB_WVALUE_TYPE) >> 8) { case USB_DESC_TYPE_DEVICE: /* Device descriptor */ size = sizeof(pbdrv_usb_nxt_device_descriptor); - pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_nxt_device_descriptor, + pbdrv_usb_nxt_write_data(0, &pbdrv_usb_nxt_device_descriptor, MIN(size, packet->length)); break; case USB_DESC_TYPE_CONFIG: /* Configuration descriptor */ - pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_nxt_full_config, - MIN(sizeof(pbdrv_usb_nxt_full_config), packet->length)); + size = sizeof(pbdrv_usb_nxt_full_config); + pbdrv_usb_nxt_write_data(0, &pbdrv_usb_nxt_full_config, + MIN(size, packet->length)); /* TODO: Why? This is not specified in the USB specs. */ if (pbdrv_usb_nxt_full_config.conf_desc.wTotalLength < packet->length) { @@ -507,13 +509,13 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { case USB_DESC_TYPE_DEVICE_QUALIFIER: /* Device qualifier descriptor. */ size = pbdrv_usb_nxt_dev_qualifier_desc.bLength; - pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_nxt_dev_qualifier_desc, + pbdrv_usb_nxt_write_data(0, &pbdrv_usb_nxt_dev_qualifier_desc, MIN(size, packet->length)); break; case USB_DESC_TYPE_BOS: /* BOS descriptor */ size = sizeof(pbdrv_usb_bos_desc_set.s); - pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_bos_desc_set, MIN(size, packet->length)); + pbdrv_usb_nxt_write_data(0, &pbdrv_usb_bos_desc_set, MIN(size, packet->length)); break; default: /* Unknown descriptor, tell the host by stalling. */ @@ -582,19 +584,19 @@ static void pbdrv_usb_nxt_handle_class_request(pbdrv_usb_nxt_setup_packet_t *pac switch (packet->value) { case 0x2A00: { // device name const char *name = pbdrv_bluetooth_get_hub_name(); - pbdrv_usb_nxt_write_data(0, (const uint8_t *)name, + pbdrv_usb_nxt_write_data(0, name, MIN(strlen(name), packet->length)); break; } case 0x2A26: { // firmware revision const char *fw = PBIO_VERSION_STR; - pbdrv_usb_nxt_write_data(0, (const uint8_t *)fw, + pbdrv_usb_nxt_write_data(0, fw, MIN(strlen(fw), packet->length)); break; } case 0x2A28: { // software revision const char *sw = PBIO_PROTOCOL_VERSION_STR; - pbdrv_usb_nxt_write_data(0, (const uint8_t *)sw, + pbdrv_usb_nxt_write_data(0, sw, MIN(strlen(sw), packet->length)); break; } @@ -660,12 +662,12 @@ static uint32_t pbdrv_usb_nxt_manage_setup_packet(void) { switch (packet.request) { case PBDRV_USB_VENDOR_REQ_WEBUSB: // Since there is only one WebUSB descriptor, we ignore the index. - pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_webusb_landing_page, + pbdrv_usb_nxt_write_data(0, &pbdrv_usb_webusb_landing_page, MIN(pbdrv_usb_webusb_landing_page.s.bLength, packet.length)); break; case PBDRV_USB_VENDOR_REQ_MS_20: // Since there is only one MS descriptor, we ignore the index. - pbdrv_usb_nxt_write_data(0, (const uint8_t *)&pbdrv_usb_ms_20_desc_set, + pbdrv_usb_nxt_write_data(0, &pbdrv_usb_ms_20_desc_set, MIN(sizeof(pbdrv_usb_ms_20_desc_set.s), packet.length)); break; default: From 7e378249e61f43840f43e5464c64226d1aaea2e4 Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 22:31:57 +0100 Subject: [PATCH 07/10] pbio/drv/usb/stm32_usbd: Delete unused string descriptors These descriptors are not referenced and thus can never be retrieved. --- lib/pbio/drv/usb/stm32_usbd/usbd_desc.c | 26 ------------------------- 1 file changed, 26 deletions(-) diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c b/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c index a636c29c3..67bfd926a 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c +++ b/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c @@ -60,8 +60,6 @@ /* Private typedef -----------------------------------------------------------*/ /* Private define ------------------------------------------------------------*/ #define USBD_LANGID_STRING 0x409 -#define USBD_CONFIGURATION_FS_STRING "Pybricks Config" -#define USBD_INTERFACE_FS_STRING "Pybricks Interface" // STM32 MCU Device ID register addresses // REVISIT: make pbdrv_xxx_get_serial_number() and use that instead @@ -229,28 +227,6 @@ static uint8_t *USBD_Pybricks_SerialStrDescriptor(USBD_SpeedTypeDef speed, uint1 return (uint8_t *)USBD_StringSerial; } -/** - * @brief Returns the configuration string descriptor. - * @param speed: Current device speed - * @param length: Pointer to data length variable - * @retval Pointer to descriptor buffer - */ -static uint8_t *USBD_Pybricks_ConfigStrDescriptor(USBD_SpeedTypeDef speed, uint16_t *length) { - USBD_GetString((uint8_t *)USBD_CONFIGURATION_FS_STRING, USBD_StrDesc, length); - return USBD_StrDesc; -} - -/** - * @brief Returns the interface string descriptor. - * @param speed: Current device speed - * @param length: Pointer to data length variable - * @retval Pointer to descriptor buffer - */ -static uint8_t *USBD_Pybricks_InterfaceStrDescriptor(USBD_SpeedTypeDef speed, uint16_t *length) { - USBD_GetString((uint8_t *)USBD_INTERFACE_FS_STRING, USBD_StrDesc, length); - return USBD_StrDesc; -} - static uint8_t *USBD_Pybricks_BOSDescriptor(USBD_SpeedTypeDef speed, uint16_t *length) { /* Prevent unused argument(s) compilation warning */ UNUSED(speed); @@ -265,8 +241,6 @@ USBD_DescriptorsTypeDef USBD_Pybricks_Desc = { .GetManufacturerStrDescriptor = USBD_Pybricks_ManufacturerStrDescriptor, .GetProductStrDescriptor = USBD_Pybricks_ProductStrDescriptor, .GetSerialStrDescriptor = USBD_Pybricks_SerialStrDescriptor, - .GetConfigurationStrDescriptor = USBD_Pybricks_ConfigStrDescriptor, - .GetInterfaceStrDescriptor = USBD_Pybricks_InterfaceStrDescriptor, .GetBOSDescriptor = USBD_Pybricks_BOSDescriptor, }; From 677f81cf275e3a2155e68d7ffae47d94ce6b8426 Mon Sep 17 00:00:00 2001 From: R Date: Wed, 23 Jul 2025 23:15:43 +0100 Subject: [PATCH 08/10] 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. --- lib/lego/lego/usb.h | 12 ++-- lib/pbio/drv/usb/stm32_usbd/usbd_desc.c | 24 +++---- lib/pbio/drv/usb/usb_ch9.h | 4 ++ lib/pbio/drv/usb/usb_common_desc.c | 24 +++++++ lib/pbio/drv/usb/usb_common_desc.h | 31 +++++++++ lib/pbio/drv/usb/usb_ev3.c | 49 +++----------- lib/pbio/drv/usb/usb_nxt.c | 85 ++++++++++--------------- lib/pbio/platform/nxt/pbdrvconfig.h | 4 ++ 8 files changed, 123 insertions(+), 110 deletions(-) diff --git a/lib/lego/lego/usb.h b/lib/lego/lego/usb.h index 5c57cbf00..9510119be 100644 --- a/lib/lego/lego/usb.h +++ b/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_ diff --git a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c b/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c index 67bfd926a..e3355d9aa 100644 --- a/lib/pbio/drv/usb/stm32_usbd/usbd_desc.c +++ b/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; } /** diff --git a/lib/pbio/drv/usb/usb_ch9.h b/lib/pbio/drv/usb/usb_ch9.h index 0323adf97..dcc50f8de 100644 --- a/lib/pbio/drv/usb/usb_ch9.h +++ b/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; diff --git a/lib/pbio/drv/usb/usb_common_desc.c b/lib/pbio/drv/usb/usb_common_desc.c index f4e862131..bacbe97d2 100644 --- a/lib/pbio/drv/usb/usb_common_desc.c +++ b/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 diff --git a/lib/pbio/drv/usb/usb_common_desc.h b/lib/pbio/drv/usb/usb_common_desc.h index 15e4c7cde..ff77c17e2 100644 --- a/lib/pbio/drv/usb/usb_common_desc.h +++ b/lib/pbio/drv/usb/usb_common_desc.h @@ -9,6 +9,9 @@ #include +#include +#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_ diff --git a/lib/pbio/drv/usb/usb_ev3.c b/lib/pbio/drv/usb/usb_ev3.c index a9d7b27a8..27d7926ff 100644 --- a/lib/pbio/drv/usb/usb_ev3.c +++ b/lib/pbio/drv/usb/usb_ev3.c @@ -18,7 +18,6 @@ #include #include -#include "pbdrvconfig.h" #include #include @@ -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: diff --git a/lib/pbio/drv/usb/usb_nxt.c b/lib/pbio/drv/usb/usb_nxt.c index 718665c8e..80ef4c0bd 100644 --- a/lib/pbio/drv/usb/usb_nxt.c +++ b/lib/pbio/drv/usb/usb_nxt.c @@ -27,6 +27,8 @@ #include "nxos/drivers/aic.h" #include "nxos/util.h" +#include + #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; diff --git a/lib/pbio/platform/nxt/pbdrvconfig.h b/lib/pbio/platform/nxt/pbdrvconfig.h index 17f1a36f5..04d0e78ed 100644 --- a/lib/pbio/platform/nxt/pbdrvconfig.h +++ b/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" From 1c9049d56f4838b3532a1ccb8ecc1c4030b0b47a Mon Sep 17 00:00:00 2001 From: R Date: Thu, 24 Jul 2025 17:05:12 +0100 Subject: [PATCH 09/10] pbio/drv/usb/usb_nxt.c: Remove device qualifier descriptor The device qualifier descriptor is used to indicate that a USB 2.0 HS device performs differently from when it is functioning as a FS device. The NXT is not capable of high speed, so it should not implement this. --- lib/pbio/drv/usb/usb_nxt.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/lib/pbio/drv/usb/usb_nxt.c b/lib/pbio/drv/usb/usb_nxt.c index 80ef4c0bd..235be1371 100644 --- a/lib/pbio/drv/usb/usb_nxt.c +++ b/lib/pbio/drv/usb/usb_nxt.c @@ -127,18 +127,6 @@ static const pbdrv_usb_dev_desc_t pbdrv_usb_nxt_device_descriptor = { .bNumConfigurations = 1, }; -static const pbdrv_usb_dev_qualifier_desc_t pbdrv_usb_nxt_dev_qualifier_desc = { - .bLength = sizeof(pbdrv_usb_dev_qualifier_desc_t), - .bDescriptorType = DESC_TYPE_DEVICE_QUALIFIER, - .bcdUSB = 0x0210, /* This packet is USB 2.1. */ - .bDeviceClass = PBIO_PYBRICKS_USB_DEVICE_CLASS, - .bDeviceSubClass = PBIO_PYBRICKS_USB_DEVICE_SUBCLASS, - .bDeviceProtocol = PBIO_PYBRICKS_USB_DEVICE_PROTOCOL, - .bMaxPacketSize0 = MAX_EP0_SIZE, - .bNumConfigurations = 1, - .bReserved = 0, -}; - typedef struct PBDRV_PACKED { pbdrv_usb_conf_desc_t conf_desc; pbdrv_usb_iface_desc_t iface_desc; @@ -492,12 +480,6 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { } break; - case USB_DESC_TYPE_DEVICE_QUALIFIER: /* Device qualifier descriptor. */ - size = pbdrv_usb_nxt_dev_qualifier_desc.bLength; - pbdrv_usb_nxt_write_data(0, &pbdrv_usb_nxt_dev_qualifier_desc, - MIN(size, packet->length)); - break; - case USB_DESC_TYPE_BOS: /* BOS descriptor */ size = sizeof(pbdrv_usb_bos_desc_set.s); pbdrv_usb_nxt_write_data(0, &pbdrv_usb_bos_desc_set, MIN(size, packet->length)); From 318f21d51b2d4c8d0a0e967ec580633559ca3ba7 Mon Sep 17 00:00:00 2001 From: R Date: Thu, 24 Jul 2025 17:13:54 +0100 Subject: [PATCH 10/10] pbio/drv/usb/usb_nxt.c: Fix ZLPs on the control pipe This is needed to correctly indicate end of descriptor when the descriptor size is an exact multiple of the endpoint packet size. This fixes enumeration issues on Windows. Removes the former TODO in the configuration descriptor handling. It is not needed and was masking this issue. --- lib/pbio/drv/usb/usb_nxt.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/pbio/drv/usb/usb_nxt.c b/lib/pbio/drv/usb/usb_nxt.c index 235be1371..bc81e5b0e 100644 --- a/lib/pbio/drv/usb/usb_nxt.c +++ b/lib/pbio/drv/usb/usb_nxt.c @@ -291,7 +291,14 @@ static void pbdrv_usb_nxt_write_data(int endpoint, const void *ptr_, uint32_t le pbdrv_usb_nxt_state.tx_data[tx] = (uint8_t *)(ptr + packet_size); pbdrv_usb_nxt_state.tx_len[tx] = length; } else { - pbdrv_usb_nxt_state.tx_data[tx] = NULL; + if (length == packet_size && endpoint == 0) { + // If we are sending data to the control pipe, we must terminate the data + // with a ZLP. In order to do so, we set the data pointer to non-NULL + // but the length to 0. We do not want to send ZLPs on the Pybricks bulk pipe. + pbdrv_usb_nxt_state.tx_data[tx] = (uint8_t *)(ptr); + } else { + pbdrv_usb_nxt_state.tx_data[tx] = NULL; + } pbdrv_usb_nxt_state.tx_len[tx] = 0; } @@ -447,11 +454,6 @@ static void pbdrv_usb_handle_std_request(pbdrv_usb_nxt_setup_packet_t *packet) { size = sizeof(pbdrv_usb_nxt_full_config); pbdrv_usb_nxt_write_data(0, &pbdrv_usb_nxt_full_config, MIN(size, packet->length)); - - /* TODO: Why? This is not specified in the USB specs. */ - if (pbdrv_usb_nxt_full_config.conf_desc.wTotalLength < packet->length) { - pbdrv_usb_nxt_send_null(); - } break; case USB_DESC_TYPE_STR: /* String or language info. */ @@ -776,8 +778,7 @@ static void pbdrv_usb_nxt_isr(void) { } /* and we will send the following data */ - if (pbdrv_usb_nxt_state.tx_len[endpoint] > 0 - && pbdrv_usb_nxt_state.tx_data[endpoint] != NULL) { + if (pbdrv_usb_nxt_state.tx_data[endpoint] != NULL) { pbdrv_usb_nxt_write_data(endpoint, pbdrv_usb_nxt_state.tx_data[endpoint], pbdrv_usb_nxt_state.tx_len[endpoint]); } else {