From 509ff084cdd8adcdbd3ebc2dfc2e6950aa36adee Mon Sep 17 00:00:00 2001 From: Nav Date: Wed, 7 Apr 2021 23:30:01 +0100 Subject: [PATCH] Refactored some of the USB device (libusb wrapper) & interface code --- .../Microchip/AtmelICE/AtmelIce.cpp | 9 ++- .../Microchip/PowerDebugger/PowerDebugger.cpp | 7 +- .../Microchip/PowerDebugger/PowerDebugger.hpp | 5 +- src/DebugToolDrivers/USB/HID/HidInterface.cpp | 71 ++++++++--------- src/DebugToolDrivers/USB/HID/HidInterface.hpp | 22 ++---- src/DebugToolDrivers/USB/Interface.cpp | 79 +++++-------------- src/DebugToolDrivers/USB/Interface.hpp | 44 ++++------- src/DebugToolDrivers/USB/UsbDevice.cpp | 50 +++++++++--- src/DebugToolDrivers/USB/UsbDevice.hpp | 11 ++- 9 files changed, 133 insertions(+), 165 deletions(-) diff --git a/src/DebugToolDrivers/Microchip/AtmelICE/AtmelIce.cpp b/src/DebugToolDrivers/Microchip/AtmelICE/AtmelIce.cpp index d5ccae52..8a885cbe 100644 --- a/src/DebugToolDrivers/Microchip/AtmelICE/AtmelIce.cpp +++ b/src/DebugToolDrivers/Microchip/AtmelICE/AtmelIce.cpp @@ -11,11 +11,14 @@ void AtmelIce::init() { // TODO: Move away from hard-coding the CMSIS-DAP/EDBG interface number auto& usbHidInterface = this->getEdbgInterface().getUsbHidInterface(); usbHidInterface.setNumber(0); - usbHidInterface.setUSBDevice(this->getLibUsbDevice()); - usbHidInterface.setVendorId(this->getVendorId()); - usbHidInterface.setProductId(this->getProductId()); + usbHidInterface.setLibUsbDevice(this->libUsbDevice); + usbHidInterface.setLibUsbDeviceHandle(this->libUsbDeviceHandle); + usbHidInterface.setVendorId(this->vendorId); + usbHidInterface.setProductId(this->productId); if (!usbHidInterface.isInitialised()) { + usbHidInterface.detachKernelDriver(); + UsbDevice::setConfiguration(0); usbHidInterface.init(); } diff --git a/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.cpp b/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.cpp index 4db4039b..166d885f 100644 --- a/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.cpp +++ b/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.cpp @@ -11,9 +11,10 @@ void PowerDebugger::init() { // TODO: Move away from hard-coding the CMSIS-DAP/EDBG interface number auto& usbHidInterface = this->getEdbgInterface().getUsbHidInterface(); usbHidInterface.setNumber(0); - usbHidInterface.setUSBDevice(this->getLibUsbDevice()); - usbHidInterface.setVendorId(this->getVendorId()); - usbHidInterface.setProductId(this->getProductId()); + usbHidInterface.setLibUsbDevice(this->libUsbDevice); + usbHidInterface.setLibUsbDeviceHandle(this->libUsbDeviceHandle); + usbHidInterface.setVendorId(this->vendorId); + usbHidInterface.setProductId(this->productId); if (!usbHidInterface.isInitialised()) { usbHidInterface.init(); diff --git a/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.hpp b/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.hpp index 594aa239..27d32c49 100644 --- a/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.hpp +++ b/src/DebugToolDrivers/Microchip/PowerDebugger/PowerDebugger.hpp @@ -26,10 +26,7 @@ namespace Bloom::DebugToolDrivers * as an Atmel Data Gateway Interface (DGI). * * Communication: - * Like the Atmel-ICE, using the Power Debugger device for AVR debugging/programming requires the AVR - * communication protocol, which is a sub-protocol of the CMSIS-DAP. - * - * For more information on the communication protocol, see the DebugToolDrivers::AtmelIce class. + * Uses the same EDBG protocol as described in the AtmelIce driver. See the AtmelIce debug tool class for more. * * USB Setup: * Vendor ID: 0x03eb (1003) diff --git a/src/DebugToolDrivers/USB/HID/HidInterface.cpp b/src/DebugToolDrivers/USB/HID/HidInterface.cpp index 509153ed..d7c95ae8 100644 --- a/src/DebugToolDrivers/USB/HID/HidInterface.cpp +++ b/src/DebugToolDrivers/USB/HID/HidInterface.cpp @@ -1,6 +1,5 @@ #include #include -#include #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(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(timeout))) == -1 + if ((transferred = hid_read_timeout( + this->hidDevice, + buffer, + maxLength, + timeout == 0 ? -1 : static_cast(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(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 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 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 HidInterface::read(unsigned int timeout) { diff --git a/src/DebugToolDrivers/USB/HID/HidInterface.hpp b/src/DebugToolDrivers/USB/HID/HidInterface.hpp index d11b3c77..1919d400 100644 --- a/src/DebugToolDrivers/USB/HID/HidInterface.hpp +++ b/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. 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. - * @see write(std::vector 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 */ diff --git a/src/DebugToolDrivers/USB/Interface.cpp b/src/DebugToolDrivers/USB/Interface.cpp index b0345d94..94215fac 100644 --- a/src/DebugToolDrivers/USB/Interface.cpp +++ b/src/DebugToolDrivers/USB/Interface.cpp @@ -11,82 +11,45 @@ using namespace Bloom::Exceptions; void Interface::init() { - libusb_device_descriptor deviceDescriptor = {}; - auto deviceHandle = this->getLibUsbDeviceHandle(); - auto libUsbDevice = this->getUSBDevice(); - int libUsbStatusCode; - - if (libUsbDevice == nullptr) { - throw Exception("Cannot open USB device without libusb device pointer."); + if (this->libUsbDevice == nullptr) { + throw Exception("Cannot initialise interface without libusb device pointer."); } - if (deviceHandle == nullptr) { - // Obtain a device handle from libusb - if ((libUsbStatusCode = libusb_open(libUsbDevice, &deviceHandle)) < 0) { - throw Exception("Failed to open USB device - error code " - + std::to_string(libUsbStatusCode) + " returned."); - } + if (this->libUsbDeviceHandle == nullptr) { + throw Exception("Cannot initialise interface without libusb device handle."); } - if ((libUsbStatusCode = libusb_get_device_descriptor(libUsbDevice, &deviceDescriptor))) { - throw Exception("Failed to obtain USB device descriptor - error code " - + std::to_string(libUsbStatusCode) + " returned."); - } - - this->setLibUsbDeviceHandle(deviceHandle); - this->setInitialised(true); -} - -void Interface::setConfiguration(int configIndex) { - libusb_config_descriptor* configDescriptor = {}; - int libUsbStatusCode; - - if ((libUsbStatusCode = libusb_get_config_descriptor(this->getUSBDevice(), 0, &configDescriptor))) { - throw Exception("Failed to obtain USB configuration descriptor - error code " - + std::to_string(libUsbStatusCode) + " returned."); - } - - if ((libUsbStatusCode = libusb_set_configuration(this->getLibUsbDeviceHandle(), configDescriptor->bConfigurationValue))) { - throw Exception("Failed to set USB configuration - error code " - + std::to_string(libUsbStatusCode) + " returned."); - } - - libusb_free_config_descriptor(configDescriptor); + this->initialised = true; } void Interface::close() { - auto deviceHandle = this->getLibUsbDeviceHandle(); - this->release(); - - if (deviceHandle != nullptr) { - libusb_close(deviceHandle); - this->setLibUsbDeviceHandle(nullptr); + if (this->libUsbDeviceHandle != nullptr) { + this->release(); } - this->setInitialised(false); + this->initialised = false; } void Interface::claim() { int interfaceNumber = this->getNumber(); - int libUsbStatusCode = 0; this->detachKernelDriver(); - if (libusb_claim_interface(this->getLibUsbDeviceHandle(), interfaceNumber) != 0) { + if (libusb_claim_interface(this->libUsbDeviceHandle, interfaceNumber) != 0) { throw Exception("Failed to claim interface {" + std::to_string(interfaceNumber) + "} on USB device\n"); } - this->setClaimed(true); + this->claimed = true; } void Interface::detachKernelDriver() { int interfaceNumber = this->getNumber(); int libUsbStatusCode; - if ((libUsbStatusCode = libusb_kernel_driver_active(this->getLibUsbDeviceHandle(), interfaceNumber)) != 0) { + if ((libUsbStatusCode = libusb_kernel_driver_active(this->libUsbDeviceHandle, interfaceNumber)) != 0) { if (libUsbStatusCode == 1) { // A kernel driver is active on this interface. Attempt to detach it - if (libusb_detach_kernel_driver(this->getLibUsbDeviceHandle(), interfaceNumber) != 0) { + if (libusb_detach_kernel_driver(this->libUsbDeviceHandle, interfaceNumber) != 0) { throw Exception("Failed to detach kernel driver from interface " + std::to_string(interfaceNumber) + "\n"); } @@ -98,11 +61,11 @@ void Interface::detachKernelDriver() { void Interface::release() { if (this->isClaimed()) { - if (libusb_release_interface(this->getLibUsbDeviceHandle(), this->getNumber()) != 0) { + if (libusb_release_interface(this->libUsbDeviceHandle, this->getNumber()) != 0) { throw Exception("Failed to release interface {" + std::to_string(this->getNumber()) + "} on USB device\n"); } - this->setClaimed(false); + this->claimed = false; } } @@ -113,12 +76,12 @@ int Interface::read(unsigned char* buffer, unsigned char endPoint, size_t length while (length > totalTransferred) { libUsbStatusCode = libusb_interrupt_transfer( - this->getLibUsbDeviceHandle(), - endPoint, - buffer, - static_cast(length), - &transferred, - static_cast(timeout) + this->libUsbDeviceHandle, + endPoint, + buffer, + static_cast(length), + &transferred, + static_cast(timeout) ); if (libUsbStatusCode != 0 && libUsbStatusCode != -7) { @@ -136,7 +99,7 @@ void Interface::write(unsigned char* buffer, unsigned char endPoint, int length) int libUsbStatusCode = 0; libUsbStatusCode = libusb_interrupt_transfer( - this->getLibUsbDeviceHandle(), + this->libUsbDeviceHandle, endPoint, buffer, length, diff --git a/src/DebugToolDrivers/USB/Interface.hpp b/src/DebugToolDrivers/USB/Interface.hpp index 2cd3f043..2a3fe95f 100644 --- a/src/DebugToolDrivers/USB/Interface.hpp +++ b/src/DebugToolDrivers/USB/Interface.hpp @@ -10,46 +10,26 @@ namespace Bloom::Usb { class Interface { - private: - libusb_device* USBDevice = nullptr; + protected: + libusb_device* libUsbDevice = nullptr; + libusb_device_handle* libUsbDeviceHandle = nullptr; + std::uint16_t vendorId = 0; std::uint16_t productId = 0; - /** - * With libusb, we can only claim a single USB interface per device handle. For this reason, - * device handles are stored against the interface. Each interface must obtain a new handle before - * claiming. - */ - libusb_device_handle* libUsbDeviceHandle = nullptr; std::uint8_t number = 0; std::string name = ""; + bool initialised = false; bool claimed = false; - void setInitialised(bool initialised) { - this->initialised = initialised; - } - - protected: - void setClaimed(bool claimed) { - this->claimed = claimed; - } - public: explicit Interface(const std::uint8_t& interfaceNumber = 0) { this->setNumber(interfaceNumber); } - libusb_device* getUSBDevice() const { - return this->USBDevice; - } - - void setUSBDevice(libusb_device* USBDevice) { - this->USBDevice = USBDevice; - } - - libusb_device_handle* getLibUsbDeviceHandle() const { - return this->libUsbDeviceHandle; + void setLibUsbDevice(libusb_device* libUsbDevice) { + this->libUsbDevice = libUsbDevice; } void setLibUsbDeviceHandle(libusb_device_handle* libUsbDeviceHandle) { @@ -101,8 +81,6 @@ namespace Bloom::Usb */ virtual void init(); - virtual void setConfiguration(int configIndex); - /** * Releases the interface and closes the device descriptor. */ @@ -112,7 +90,15 @@ namespace Bloom::Usb * Attempts to claim the interface */ void claim(); + + /** + * If a kernel driver is attached to the interface, this method will detach it. + */ void detachKernelDriver(); + + /** + * Releases the interface, if claimed. + */ void release(); /** diff --git a/src/DebugToolDrivers/USB/UsbDevice.cpp b/src/DebugToolDrivers/USB/UsbDevice.cpp index 08184bf3..c8657ad5 100644 --- a/src/DebugToolDrivers/USB/UsbDevice.cpp +++ b/src/DebugToolDrivers/USB/UsbDevice.cpp @@ -1,9 +1,4 @@ #include -#include -#include -#include -#include -#include #include #include "UsbDevice.hpp" @@ -48,8 +43,7 @@ std::vector UsbDevice::findMatchingDevices( return matchedDevices; } -void UsbDevice::init() -{ +void UsbDevice::init() { libusb_init(&this->libUsbContext); // libusb_set_option(this->libUsbContext, LIBUSB_OPTION_LOG_LEVEL, LIBUSB_LOG_LEVEL_NONE); auto devices = this->findMatchingDevices(); @@ -60,18 +54,48 @@ void UsbDevice::init() } else if (devices.size() > 1) { // TODO: implement support for multiple devices (maybe via serial number?) throw Exception("Multiple devices of matching vendor & product ID found.\n" - "Yes, as a program I really am too stupid to figure out what to do " - "here, so I'm just going to quit.\n Please ensure that only one debug tool " - "is connected and then try again."); + "Yes, as a program I really am too stupid to figure out what to do " + "here, so I'm just going to quit.\n Please ensure that only one debug tool " + "is connected and then try again."); } // For now, just use the first device found. - this->setLibUsbDevice(devices.front()); + auto device = devices.front(); + this->setLibUsbDevice(device); + + int libUsbStatusCode; + + // Obtain a device handle from libusb + if ((libUsbStatusCode = libusb_open(libUsbDevice, &this->libUsbDeviceHandle)) < 0) { + throw Exception("Failed to open USB device - error code " + std::to_string(libUsbStatusCode) + + " returned."); + } } -void UsbDevice::close() -{ +void UsbDevice::close() { + if (this->libUsbDeviceHandle != nullptr) { + libusb_close(this->libUsbDeviceHandle); + this->libUsbDeviceHandle = nullptr; + } + if (this->libUsbContext != nullptr) { libusb_exit(this->libUsbContext); } } + +void UsbDevice::setConfiguration(int configIndex) { + libusb_config_descriptor* configDescriptor = {}; + int libUsbStatusCode; + + if ((libUsbStatusCode = libusb_get_config_descriptor(this->libUsbDevice, 0, &configDescriptor))) { + throw Exception("Failed to obtain USB configuration descriptor - error code " + + std::to_string(libUsbStatusCode) + " returned."); + } + + if ((libUsbStatusCode = libusb_set_configuration(this->libUsbDeviceHandle, configDescriptor->bConfigurationValue))) { + throw Exception("Failed to set USB configuration - error code " + + std::to_string(libUsbStatusCode) + " returned."); + } + + libusb_free_config_descriptor(configDescriptor); +} diff --git a/src/DebugToolDrivers/USB/UsbDevice.hpp b/src/DebugToolDrivers/USB/UsbDevice.hpp index 22519c18..ea8d5956 100644 --- a/src/DebugToolDrivers/USB/UsbDevice.hpp +++ b/src/DebugToolDrivers/USB/UsbDevice.hpp @@ -12,9 +12,10 @@ namespace Bloom::Usb { class UsbDevice { - private: + protected: libusb_context* libUsbContext = nullptr; libusb_device* libUsbDevice = nullptr; + libusb_device_handle* libUsbDeviceHandle = nullptr; std::uint16_t vendorId; std::uint16_t productId; @@ -22,7 +23,6 @@ namespace Bloom::Usb std::optional vendorId = std::nullopt, std::optional productId = std::nullopt ); - protected: void close(); public: @@ -50,5 +50,12 @@ namespace Bloom::Usb std::uint16_t getProductId() const { return this->productId; } + + /** + * Selects a specific configuration on the device, using the configuration index. + * + * @param configIndex + */ + virtual void setConfiguration(int configIndex); }; }