Refactored some of the USB device (libusb wrapper) & interface code

src/DebugToolDrivers/USB/HID/HidInterface.cpp

@@ -1,6 +1,5 @@
 #include <cstdint>
 #include <string>
-#include <stdexcept>
 
 #include "HidInterface.hpp"
 #include "hidapi.hpp"
@@ -12,45 +11,37 @@ using namespace Bloom::Usb;
 using namespace Bloom::Exceptions;
 
 std::string HidInterface::getDevicePathByInterfaceNumber(const std::uint16_t& interfaceNumber) {
-    hid_device_info* HIDDeviceInfoList = hid_enumerate(this->getVendorId(), this->getProductId());
+    hid_device_info* hidDeviceInfoList = hid_enumerate(this->getVendorId(), this->getProductId());
 
-    while (HIDDeviceInfoList != nullptr) {
-        if (HIDDeviceInfoList->interface_number == interfaceNumber) {
+    while (hidDeviceInfoList != nullptr) {
+        if (hidDeviceInfoList->interface_number == interfaceNumber) {
             break;
         }
 
-        HIDDeviceInfoList = HIDDeviceInfoList->next;
+        hidDeviceInfoList = hidDeviceInfoList->next;
     }
 
-    if (HIDDeviceInfoList == nullptr) {
-        throw std::runtime_error("Failed to match interface number with HID interface.");
+    if (hidDeviceInfoList == nullptr) {
+        throw Exception("Failed to match interface number with HID interface.");
     }
 
-    auto path = std::string(HIDDeviceInfoList->path);
-    hid_free_enumeration(HIDDeviceInfoList);
+    auto path = std::string(hidDeviceInfoList->path);
+    hid_free_enumeration(hidDeviceInfoList);
     return path;
 }
 
 void HidInterface::init() {
-    /*
-     * Because we use the HIDAPI with libusb for our HID interfaces, we must allow the HIDAPI to own the device
-     * resources (device handle and the interface). However, the HIDAPI does not provide any means to ensure that a
-     * specific configuration is set against the device. This is why we first open the device via libusb (by calling
-     * the generic init() method), so that we can set the correct configuration. We then close the device to allow
-     * the HIDAPI to take ownership.
-     */
-    Interface::init();
-    Interface::detachKernelDriver();
-    Interface::setConfiguration(0);
-    Interface::close();
+    if (this->libUsbDevice == nullptr) {
+        throw Exception("Cannot initialise interface without libusb device pointer.");
+    }
 
     hid_init();
     hid_device* hidDevice;
 
-    std::string HIDInterfacePath = this->getDevicePathByInterfaceNumber(this->getNumber());
-    Logger::debug("HID device path: " + HIDInterfacePath);
+    std::string hidInterfacePath = this->getDevicePathByInterfaceNumber(this->getNumber());
+    Logger::debug("HID device path: " + hidInterfacePath);
 
-    if ((hidDevice = hid_open_path(HIDInterfacePath.c_str())) == nullptr) {
+    if ((hidDevice = hid_open_path(hidInterfacePath.c_str())) == nullptr) {
         throw Exception("Failed to open HID device via hidapi.");
     }
 
@@ -60,17 +51,18 @@ void HidInterface::init() {
 
     this->setHidDevice(hidDevice);
     this->setInputReportSize(static_cast<std::size_t>(hidDevice->input_ep_max_packet_size));
-    this->setLibUsbDeviceHandle(hidDevice->device_handle);
+    this->libUsbDeviceHandle = hidDevice->device_handle;
+    this->initialised = true;
 }
 
 void HidInterface::close() {
     auto hidDevice = this->getHidDevice();
 
     if (hidDevice != nullptr) {
-        this->setLibUsbDeviceHandle(nullptr);
+        this->libUsbDeviceHandle = nullptr;
         hid_close(hidDevice);
         // hid_close() releases the interface
-        Interface::setClaimed(false);
+        this->claimed = false;
     }
 
     hid_exit();
@@ -80,8 +72,12 @@ void HidInterface::close() {
 std::size_t HidInterface::read(unsigned char* buffer, std::size_t maxLength, unsigned int timeout) {
     int transferred;
 
-    if ((transferred = hid_read_timeout(this->getHidDevice(), buffer, maxLength,
-        timeout == 0 ? -1 : static_cast<int>(timeout))) == -1
+    if ((transferred = hid_read_timeout(
+            this->hidDevice,
+            buffer,
+            maxLength,
+            timeout == 0 ? -1 : static_cast<int>(timeout))
+        ) == -1
     ) {
         throw DeviceCommunicationFailure("Failed to read from HID device.");
     }
@@ -89,16 +85,6 @@ std::size_t HidInterface::read(unsigned char* buffer, std::size_t maxLength, uns
     return static_cast<std::size_t>(transferred);
 }
 
-void HidInterface::write(unsigned char* buffer, std::size_t length) {
-    int transferred;
-
-    if ((transferred = hid_write(this->getHidDevice(), buffer, length)) != length) {
-        Logger::debug("Attempted to write " + std::to_string(length)
-        + " bytes to HID interface. Bytes written: " + std::to_string(transferred));
-        throw DeviceCommunicationFailure("Failed to write data to HID interface.");
-    }
-}
-
 void HidInterface::write(std::vector<unsigned char> buffer) {
     if (buffer.size() > this->getInputReportSize()) {
         throw Exception("Cannot send data via HID interface - data exceeds maximum packet size.");
@@ -111,7 +97,14 @@ void HidInterface::write(std::vector<unsigned char> buffer) {
         buffer.resize(this->getInputReportSize(), 0);
     }
 
-    this->write(buffer.data(), buffer.size());
+    int transferred;
+    auto length = buffer.size();
+
+    if ((transferred = hid_write(this->getHidDevice(), buffer.data(), length)) != length) {
+        Logger::debug("Attempted to write " + std::to_string(length)
+            + " bytes to HID interface. Bytes written: " + std::to_string(transferred));
+        throw DeviceCommunicationFailure("Failed to write data to HID interface.");
+    }
 }
 
 std::vector<unsigned char> HidInterface::read(unsigned int timeout) {

src/DebugToolDrivers/USB/HID/HidInterface.hpp

@@ -10,7 +10,7 @@
 namespace Bloom::Usb
 {
     /**
-     * The HIDInterface uses the HIDAPI library to implement communication with HID endpoints.
+     * The HidInterface uses the HIDAPI library to implement communication with HID endpoints.
      *
      * Currently, this interface only supports single-report HID implementations. HID interfaces with
      * multiple reports will be supported as-and-when we need it.
@@ -48,6 +48,8 @@ namespace Bloom::Usb
          * Keeping this in the private scope to enforce use of vector<unsigned char>. See read()
          * method in public scope.
          *
+         * @TODO: Do we really need this? Why not just have the one that accepts the vector. Review
+         *
          * @param buffer
          * @param maxLength
          * @param timeout
@@ -57,17 +59,6 @@ namespace Bloom::Usb
          */
         std::size_t read(unsigned char* buffer, std::size_t maxLength, unsigned int timeout);
 
-        /**
-         * Writes `length` bytes from `buffer` to HID output endpoint.
-         *
-         * Keeping this in the private scope to enforce use of vector<unsigned char>.
-         * @see write(std::vector<unsigned char> buffer);
-         *
-         * @param buffer
-         * @param length
-         */
-        void write(unsigned char* buffer, std::size_t length);
-
     protected:
         hid_device* getHidDevice() const {
             return this->hidDevice;
@@ -79,14 +70,17 @@ namespace Bloom::Usb
         }
 
         /**
-         * Claims the USB HID interface, obtains a hid_device instance and configures
+         * Claims the USB HID interface and obtains a hid_device instance
          */
         void init() override;
 
+        /**
+         * Closes the hid_device and releases any claimed interfaces (via hid_close())
+         */
         void close() override;
 
         /**
-         * Wrapper for write(unsigned char *buffer, int length)
+         * Writes buffer to HID output endpoint.
          *
          * @param buffer
          */