fix(usb_host_hid): Fixed opening HID device with multiple non-sequent…

…ial interfaces.

Closes espressif/esp-idf#13031

host/class/hid/usb_host_hid/CHANGELOG.md

@@ -1,7 +1,11 @@
+## 1.0.2
+<! Unreleased >
+- Fixed device open procedure for HID devices with multiple non-sequential interfaces.
+
 ## 1.0.1
 
 - Fixed a bug where configuring the driver with `create_background_task = false` did not properly initialize the driver. This lead to `the hid_host_uninstall()` hang-up.
-- Fixed bug where `hid_host_uninstall()` would cause a crash during the call while USB device has not been removed.
+- Fixed a bug where `hid_host_uninstall()` would cause a crash during the call while USB device has not been removed.
 - Added `hid_host_get_device_info()` to get the basic information of a connected USB HID device.
 
 ## 1.0.0

host/class/hid/usb_host_hid/hid_host.c

@@ -52,6 +52,19 @@ static portMUX_TYPE hid_lock = portMUX_INITIALIZER_UNLOCKED;
 
 #define HID_RETURN_ON_INVALID_ARG(exp) ESP_RETURN_ON_FALSE((exp) != NULL, ESP_ERR_INVALID_ARG, TAG, "Argument error")
 
+// USB Descriptor parsing helping macroses
+#define GET_NEXT_INTERFACE_DESC(p, max_len, offs)                                                \
+    ((const usb_intf_desc_t *)usb_parse_next_descriptor_of_type((const usb_standard_desc_t *)p,  \
+                                                                max_len,                         \
+                                                                USB_B_DESCRIPTOR_TYPE_INTERFACE, \
+                                                                &(offs)))
+
+#define GET_NEXT_HID_DESC(p, max_len, offs)                                                      \
+    ((const hid_descriptor_t *)usb_parse_next_descriptor_of_type((const usb_standard_desc_t *)p, \
+                                                                max_len,                         \
+                                                                HID_CLASS_DESCRIPTOR_TYPE_HID,   \
+                                                                &(offs)))
+
 static const char *TAG = "hid-host";
 
 #define DEFAULT_TIMEOUT_MS  (5000)
@@ -254,6 +267,30 @@ static hid_iface_t *get_iface_by_handle(hid_host_device_handle_t hid_dev_handle)
     return hid_iface;
 }
 
+/**
+ * @brief Returns pointer to first IN Endpoint descriptor
+ *
+ * @param[in] iface_desc    Pointer to Interface Descriptor
+ * @param[in] total_length  Total length of configuration descriptor
+ * @return usb_ep_desc_t Pointer to EP IN Descriptor
+ */
+static inline const usb_ep_desc_t *get_iface_ep_in(const usb_intf_desc_t *iface_desc,
+        const size_t total_length)
+{
+    assert(iface_desc);
+    const usb_ep_desc_t *ep_desc = NULL;
+    for (int i = 0; i < iface_desc->bNumEndpoints; i++) {
+        int ep_offset = 0;
+        ep_desc = usb_parse_endpoint_descriptor_by_index(iface_desc, i, total_length, &ep_offset);
+        if (ep_desc) {
+            if (USB_EP_DESC_GET_EP_DIR(ep_desc)) {
+                return ep_desc;
+            }
+        }
+    }
+    return NULL;
+}
+
 /**
  * @brief Check HID interface descriptor present
  *
@@ -424,50 +461,39 @@ static void hid_host_notify_interface_connected(hid_device_t *hid_device)
 static esp_err_t hid_host_interface_list_create(hid_device_t *hid_device,
         const usb_config_desc_t *config_desc)
 {
+    assert(hid_device);
+    assert(config_desc);
     size_t total_length = config_desc->wTotalLength;
     const usb_intf_desc_t *iface_desc = NULL;
     const hid_descriptor_t *hid_desc = NULL;
-    const usb_ep_desc_t *ep_desc = NULL;
     const usb_ep_desc_t *ep_in_desc = NULL;
-    int offset = 0;
+    int iface_offset = 0;
+    int hid_desc_offset = 0;
 
+    // Get first Interface descriptor
+    iface_desc = GET_NEXT_INTERFACE_DESC(config_desc, total_length, iface_offset);
     // For every Interface
-    for (int i = 0; i < config_desc->bNumInterfaces; i++) {
-        iface_desc = usb_parse_interface_descriptor(config_desc, i, 0, &offset);
+    while (iface_desc != NULL) {
+
         hid_desc = NULL;
+        hid_desc_offset = iface_offset;
         ep_in_desc = NULL;
 
         if (USB_CLASS_HID == iface_desc->bInterfaceClass) {
-            // HID descriptor
-            hid_desc = (const hid_descriptor_t *) usb_parse_next_descriptor_of_type((const usb_standard_desc_t *) iface_desc,
-                       total_length,
-                       HID_CLASS_DESCRIPTOR_TYPE_HID,
-                       &offset);
-            if (!hid_desc) {
-                return ESP_ERR_NOT_FOUND;
-            }
-
-            // EP descriptors for Interface
-            for (int i = 0; i < iface_desc->bNumEndpoints; i++) {
-                int ep_offset = 0;
-                ep_desc = usb_parse_endpoint_descriptor_by_index(iface_desc, i, total_length, &ep_offset);
-
-                if (ep_desc) {
-                    if (USB_EP_DESC_GET_EP_DIR(ep_desc)) {
-                        ep_in_desc = ep_desc;
-                    }
-                } else {
-                    return ESP_ERR_NOT_FOUND;
+            ESP_LOGD(TAG, "Found HID, bInterfaceNumber=%d", iface_desc->bInterfaceNumber);
+            hid_desc = GET_NEXT_HID_DESC(iface_desc, total_length, hid_desc_offset);
+            if (hid_desc) {
+                ep_in_desc = get_iface_ep_in(iface_desc, total_length);
+                if (ep_in_desc) {
+                    HID_RETURN_ON_ERROR( hid_host_add_interface(hid_device,
+                                         iface_desc,
+                                         hid_desc,
+                                         ep_in_desc),
+                                         "Unable to add HID Interface to the RAM list");
                 }
-            } // for every EP within Interface
-
-            // Add Interface to the list
-            HID_RETURN_ON_ERROR( hid_host_add_interface(hid_device,
-                                 iface_desc,
-                                 hid_desc,
-                                 ep_in_desc),
-                                 "Unable to add HID Interface to the RAM list");
-        } // USB_CLASS_HID
+            }
+        } // HID Interface
+        iface_desc = GET_NEXT_INTERFACE_DESC(iface_desc, total_length, iface_offset);
     }
 
     hid_host_notify_interface_connected(hid_device);

host/class/hid/usb_host_hid/test/hid_mock_device.c

@@ -68,7 +68,7 @@ static const uint8_t hid_configuration_descriptor_two_ifaces[] = {
 
     // Interface number, string index, boot protocol, report descriptor len, EP In address, size & polling interval
     TUD_HID_DESCRIPTOR(0, 4, HID_ITF_PROTOCOL_KEYBOARD, sizeof(hid_keyboard_report_descriptor), 0x81, 16, 10),
-    TUD_HID_DESCRIPTOR(1, 4, HID_ITF_PROTOCOL_MOUSE, sizeof(hid_mouse_report_descriptor), 0x82, 16, 10),
+    TUD_HID_DESCRIPTOR(2, 4, HID_ITF_PROTOCOL_MOUSE, sizeof(hid_mouse_report_descriptor), 0x82, 16, 10),
 };
 
 static const uint8_t *hid_configuration_descriptor_list[TUSB_IFACE_COUNT_MAX] = {