Modernize Hid/Bulk Lists

src/controllers/bulk/bulkcontroller.cpp

@@ -2,6 +2,8 @@
 
 #include <libusb.h>
 
+#include <algorithm>
+
 #include "controllers/bulk/bulksupported.h"
 #include "controllers/defs_controllers.h"
 #include "moc_bulkcontroller.cpp"
@@ -130,12 +132,10 @@ bool BulkController::matchProductInfo(const ProductInfo& product) {
         return false;
     }
 
-#if defined(__WINDOWS__) || defined(__APPLE__)
     value = product.interface_number.toInt(&ok, 16);
-    if (!ok || m_interfaceNumber != static_cast<unsigned int>(value)) {
+    if (!ok || m_interfaceNumber != value) {
         return false;
     }
-#endif
 
     // Match found
     return true;
@@ -148,23 +148,18 @@ int BulkController::open() {
     }
 
     /* Look up endpoint addresses in supported database */
-    int i;
-    for (i = 0; bulk_supported[i].vendor_id; ++i) {
-        if ((bulk_supported[i].vendor_id == m_vendorId) &&
-                (bulk_supported[i].product_id == m_productId)) {
-            m_inEndpointAddr = bulk_supported[i].in_epaddr;
-            m_outEndpointAddr = bulk_supported[i].out_epaddr;
-#if defined(__WINDOWS__) || defined(__APPLE__)
-            m_interfaceNumber = bulk_supported[i].interface_number;
-#endif
-            break;
-        }
-    }
 
-    if (bulk_supported[i].vendor_id == 0) {
+    const bulk_support_lookup* pDevice = std::find_if(
+            std::cbegin(bulk_supported), std::cend(bulk_supported), [this](const auto& dev) {
+                return dev.key.vendor_id == m_vendorId && dev.key.product_id == m_productId;
+            });
+    if (pDevice == std::cend(bulk_supported)) {
         qCWarning(m_logBase) << "USB Bulk device" << getName() << "unsupported";
         return -1;
     }
+    m_inEndpointAddr = pDevice->endpoints.in_epaddr;
+    m_outEndpointAddr = pDevice->endpoints.out_epaddr;
+    m_interfaceNumber = pDevice->endpoints.interface_number;
 
     // XXX: we should enumerate devices and match vendor, product, and serial
     if (m_phandle == nullptr) {
@@ -176,31 +171,21 @@ int BulkController::open() {
         qCWarning(m_logBase) << "Unable to open USB Bulk device" << getName();
         return -1;
     }
-
-#if defined(__WINDOWS__) || defined(__APPLE__)
-    if (m_interfaceNumber && libusb_kernel_driver_active(m_phandle, m_interfaceNumber) == 1) {
-        qCDebug(m_logBase) << "Found a driver active for" << getName();
-        if (libusb_detach_kernel_driver(m_phandle, 0) == 0)
-            qCDebug(m_logBase) << "Kernel driver detached for" << getName();
-        else {
-            qCWarning(m_logBase) << "Couldn't detach kernel driver for" << getName();
-            libusb_close(m_phandle);
-            return -1;
-        }
+    if (libusb_set_auto_detach_kernel_driver(m_phandle, true) == LIBUSB_ERROR_NOT_SUPPORTED) {
+        qCDebug(m_logBase) << "unable to automatically detach kernel driver for" << getName();
     }
 
-    if (m_interfaceNumber) {
-        int ret = libusb_claim_interface(m_phandle, m_interfaceNumber);
-        if (ret < 0) {
+    if (m_interfaceNumber.has_value()) {
+        int error = libusb_claim_interface(m_phandle, *m_interfaceNumber);
+        if (error < 0) {
             qCWarning(m_logBase) << "Cannot claim interface for" << getName()
-                                 << ":" << libusb_error_name(ret);
+                                 << ":" << libusb_error_name(error);
             libusb_close(m_phandle);
             return -1;
         } else {
             qCDebug(m_logBase) << "Claimed interface for" << getName();
         }
     }
-#endif
 
     setOpen(true);
     startEngine();
@@ -255,15 +240,13 @@ int BulkController::close() {
     stopEngine();
 
     // Close device
-#if defined(__WINDOWS__) || defined(__APPLE__)
-    if (m_interfaceNumber) {
-        int ret = libusb_release_interface(m_phandle, m_interfaceNumber);
-        if (ret < 0) {
+    if (m_interfaceNumber.has_value()) {
+        int error = libusb_release_interface(m_phandle, *m_interfaceNumber);
+        if (error < 0) {
             qCWarning(m_logBase) << "Cannot release interface for" << getName()
-                                 << ":" << libusb_error_name(ret);
+                                 << ":" << libusb_error_name(error);
         }
     }
-#endif
     qCInfo(m_logBase) << "  Closing device";
     libusb_close(m_phandle);
     m_phandle = nullptr;

src/controllers/bulk/bulkcontroller.h

@@ -2,6 +2,7 @@
 
 #include <QAtomicInt>
 #include <QThread>
+#include <optional>
 
 #include "controllers/controller.h"
 #include "controllers/hid/legacyhidcontrollermapping.h"
@@ -72,13 +73,12 @@ class BulkController : public Controller {
 
     // Local copies of things we need from desc
 
-    unsigned short m_vendorId;
-    unsigned short m_productId;
-    unsigned char m_inEndpointAddr;
-    unsigned char m_outEndpointAddr;
-#if defined(__WINDOWS__) || defined(__APPLE__)
-    unsigned int m_interfaceNumber;
-#endif
+    std::uint16_t m_vendorId;
+    std::uint16_t m_productId;
+    std::uint8_t m_inEndpointAddr;
+    std::uint8_t m_outEndpointAddr;
+    std::optional<std::uint8_t> m_interfaceNumber;
+
     QString m_manufacturer;
     QString m_product;
 

src/controllers/bulk/bulkenumerator.cpp

@@ -21,14 +21,12 @@ BulkEnumerator::~BulkEnumerator() {
     libusb_exit(m_context);
 }
 
-static bool is_interesting(struct libusb_device_descriptor *desc) {
-    for (int i = 0; bulk_supported[i].vendor_id; ++i) {
-        if ((bulk_supported[i].vendor_id == desc->idVendor) &&
-            (bulk_supported[i].product_id == desc->idProduct)) {
-            return true;
-        }
-    }
-    return false;
+static bool is_interesting(const libusb_device_descriptor& desc) {
+    return std::any_of(std::cbegin(bulk_supported),
+            std::cend(bulk_supported),
+            [&](const auto& dev) {
+                return dev.key.vendor_id == desc.idVendor && dev.key.product_id == desc.idProduct;
+            });
 }
 
 QList<Controller*> BulkEnumerator::queryDevices() {
@@ -43,7 +41,7 @@ QList<Controller*> BulkEnumerator::queryDevices() {
         struct libusb_device_descriptor desc;
 
         libusb_get_device_descriptor(device, &desc);
-        if (is_interesting(&desc)) {
+        if (is_interesting(desc)) {
             struct libusb_device_handle* handle = nullptr;
             err = libusb_open(device, &handle);
             if (err) {

src/controllers/bulk/bulksupported.h

@@ -1,18 +1,31 @@
+#pragma once
+
+#include <cstdint>
+#include <optional>
+
+struct bulk_device_id {
+    std::uint16_t vendor_id;
+    std::uint16_t product_id;
+};
+
 // A list of supported USB bulk devices
 
-#pragma once
+struct bulk_device_endpoints {
+    std::uint8_t in_epaddr;
+    std::uint8_t out_epaddr;
+    // we may not know the interface, in which case we should not try to claim it.
+    // these devices are likely unusable on windows without claiming the correct interface.
+    std::optional<std::uint8_t> interface_number;
+};
 
-typedef struct bulk_supported {
-    unsigned short vendor_id;
-    unsigned short product_id;
-    unsigned char in_epaddr;
-    unsigned char out_epaddr;
-    unsigned int interface_number;
-} bulk_supported_t;
+struct bulk_support_lookup {
+    bulk_device_id key;
+    bulk_device_endpoints endpoints;
+};
 
-static bulk_supported_t bulk_supported[] = {
-        {0x06f8, 0xb105, 0x82, 0x03, 0}, // Hercules MP3e2
-        {0x06f8, 0xb107, 0x83, 0x03, 0}, // Hercules Mk4
-        {0x06f8, 0xb100, 0x86, 0x06, 0}, // Hercules Mk2
-        {0x06f8, 0xb120, 0x82, 0x03, 0}, // Hercules MP3 LE / Glow
-        {0, 0, 0, 0, 0}};
+constexpr static bulk_support_lookup bulk_supported[] = {
+        {{0x06f8, 0xb105}, {0x82, 0x03, std::nullopt}}, // Hercules MP3e2
+        {{0x06f8, 0xb107}, {0x83, 0x03, std::nullopt}}, // Hercules Mk4
+        {{0x06f8, 0xb100}, {0x86, 0x06, std::nullopt}}, // Hercules Mk2
+        {{0x06f8, 0xb120}, {0x82, 0x03, std::nullopt}}, // Hercules MP3 LE / Glow
+};

src/controllers/hid/hiddenylist.h

@@ -1,21 +1,35 @@
 #pragma once
 
-typedef struct hid_denylist {
+// TODO: unify this with the invalid interfacenumber from the bulkenumerator
+constexpr static int kInvalidInterfaceNumber = -1;
+constexpr static unsigned short kAnyValue = 0x0;
+
+struct hid_denylist_t {
     unsigned short vendor_id;
     unsigned short product_id;
     unsigned short usage_page;
     unsigned short usage;
-    int interface_number;
-} hid_denylist_t;
+    int interface_number = kInvalidInterfaceNumber;
+};
 
 /// USB HID device that should not be recognized as controllers
-constexpr hid_denylist_t hid_denylisted[] = {
-        {0x1157, 0x300, 0x1, 0x2, -1},  // EKS Otus mouse pad (OS/X,windows)
-        {0x1157, 0x300, 0x0, 0x0, 0x3}, // EKS Otus mouse pad (linux)
-        {0x04f3, 0x2d26, 0x0, 0x0, -1}, // ELAN2D26:00 Touch screen
-        {0x046d, 0xc539, 0x0, 0x0, -1}, // Logitech G Pro Wireless
+constexpr static hid_denylist_t hid_denylisted[] = {
+        {0x1157, 0x300, 0x1, 0x2},                  // EKS Otus mouse pad (OS/X,windows)
+        {0x1157, 0x300, kAnyValue, kAnyValue, 0x3}, // EKS Otus mouse pad (linux)
+        {0x04f3, 0x2d26, kAnyValue, kAnyValue},     // ELAN2D26:00 Touch screen
+        {0x046d, 0xc539, kAnyValue, kAnyValue},     // Logitech G Pro Wireless
         // The following rules have been created using the official USB HID page
         // spec as specified at https://usb.org/sites/default/files/hut1_4.pdf
-        {0x0, 0x0, 0x0D, 0x04, -1}, // Touch Screen
-        {0x0, 0x0, 0x0D, 0x22, -1}, // Finger
+        {
+                kAnyValue,
+                kAnyValue,
+                0x0D,
+                0x04,
+        }, // Touch Screen
+        {
+                kAnyValue,
+                kAnyValue,
+                0x0D,
+                0x22,
+        }, // Finger
 };

src/controllers/hid/hidenumerator.cpp

@@ -33,36 +33,26 @@ bool recognizeDevice(const hid_device_info& device_info) {
     }
 
     // Exclude specific devices from the denylist.
-    bool interface_number_valid = device_info.interface_number != -1;
-    const int denylist_len = sizeof(hid_denylisted) / sizeof(hid_denylisted[0]);
-    for (int bl_index = 0; bl_index < denylist_len; bl_index++) {
-        hid_denylist_t denylisted = hid_denylisted[bl_index];
+    for (const hid_denylist_t& denylisted : hid_denylisted) {
         // If vendor ids are specified and do not match, skip.
-        if (denylisted.vendor_id && device_info.vendor_id != denylisted.vendor_id) {
+        if (denylisted.vendor_id != kAnyValue &&
+                device_info.vendor_id != denylisted.vendor_id) {
             continue;
         }
         // If product IDs are specified and do not match, skip.
-        if (denylisted.product_id && device_info.product_id != denylisted.product_id) {
+        if (denylisted.product_id != kAnyValue &&
+                device_info.product_id != denylisted.product_id) {
             continue;
         }
         // Denylist entry based on interface number
-        if (denylisted.interface_number != -1) {
-            // Skip matching for devices without usage info.
-            if (!interface_number_valid) {
-                continue;
-            }
-            // If interface number is present and the interface numbers do not
-            // match, skip.
-            if (device_info.interface_number != denylisted.interface_number) {
-                continue;
-            }
+        // If interface number is present and the interface numbers do not
+        // match, skip.
+        if (denylisted.interface_number != kInvalidInterfaceNumber &&
+                device_info.interface_number != denylisted.interface_number) {
+            continue;
         }
         // Denylist entry based on usage_page and usage (both required)
-        if (denylisted.usage_page != 0 && denylisted.usage != 0) {
-            // Skip matching for devices with no usage_page/usage info.
-            if (device_info.usage_page == 0 && device_info.usage == 0) {
-                continue;
-            }
+        if (denylisted.usage_page != kAnyValue && denylisted.usage != kAnyValue) {
             // If usage_page is different, skip.
             if (device_info.usage_page != denylisted.usage_page) {
                 continue;