8
\$\begingroup\$

I want to use libusb library in my C++ application. I have created classes which wraps libusb functions. You can see that libusb API is split into 8 sections:

http://libusb.sourceforge.net/api-1.0/modules.html

For me the most necessary sections are:

  • Device handling and enumeration
  • Synchronous device I/O
  • Library initialization/deinitialization
  • Miscellaneous

I have also split "Device handling and enumeration" section to "Device Info" and "Device Handling". To use libusb functions it is need to initialize libusb library (call libusb_init function), store somewhere its initialized libusb_context *, next - call functions (libusb_get_device_list, libusb_open_device_with_vid_pid) to obtain and store somewhere "device pointer" (in libusb known as libusb_device* or libusb_device_handle*), and then finally call functions for obtaining device info or performing some USB transfers (for instance: libusb_set_configuration, libusb_get_bus_number). Note, that these functions require "device pointer": libusb_device_handle* or libusb_device_* as first parameter. You can see whole process of initializing/opening/controlling device here:

http://www.dreamincode.net/forums/topic/148707-introduction-to-using-libusb-10/

Currenlty my solution consists of following classes:

  • LibusbInfoAPI: provides functions to obtain device info
  • LibusbControlAPI: provides functions to control device
  • LibusbHandlingAPI: creates devices, destroys, manages them, and stores libusb_context *, libusb_device *, libusb_device_handle *

LibusbControlAPI header:

class LibusbControlAPI
{
public:
    int GetConfiguration(DevHandlePtr deviceHandle, int& activeConfiguration);
    int SetConfiguration(DevHandlePtr deviceHandle, int targetConfiguration);

    int ClaimInterface(DevHandlePtr deviceHandle, int interfaceNumber);
    int ReleaseInterface(DevHandlePtr deviceHandle, int interfaceNumber);
    int SetInterfaceAlternateSetting(DevHandlePtr deviceHandle, int interfaceNumber, int alternateSetting);

    int ClearHalt(DevHandlePtr deviceHandle, USBEndpoint endpoint);

    int SendControlTransfer(DevHandlePtr deviceHandle,
            uint8_t bmRequestType,
            uint8_t bRequest,
            uint16_t wValue,
            uint16_t wIndex,
            unsigned char * buffer,
            uint16_t wLength,
            unsigned int timeout);

    int SendInterruptTransfer(DevHandlePtr deviceHandle,
            USBEndpoint endpoint,
            unsigned char * buffer,
            int length,
            int & transferred,
            unsigned int timeout);

    int SendBlukTransfer(DevHandlePtr deviceHandle,
            USBEndpoint endpoint,
            unsigned char * buffer,
            int length,
            int & transferred,
            unsigned int timeout);
};

Part of cpp file:

int LibusbControlAPI::GetConfiguration(DevHandlePtr deviceHandle, int & activeConfiguration)
{
    int result;
    int * configuration = nullptr;

    if (!deviceHandle->m_opened)
        return 1;

    result = libusb_get_configuration(deviceHandle->m_libusbData->m_libusbDeviceHandle, configuration);
    if (result != 0)
        return 1;

    activeConfiguration = *configuration;
    return 0;
}

Similary for LibusbInfoAPI header i have:

class LibusbInfoAPI
{
public:
    int GetBusNumber(DevHandlePtr deviceHandle, uint8_t& busNumber);
    int GetPortNumber(DevHandlePtr deviceHandle, uint8_t& portNumber);

    int GetDeviceAddress(DevHandlePtr deviceHandle, uint8_t& deviceAddress);
    int GetDeviceSpeed(DevHandlePtr deviceHandle, USBSpeed& usbSpeed);

    int GetMaxPacketSize(DevHandlePtr deviceHandle, USBEndpoint endpoint, int& maxPacketSize);
    int GetMaxIsoPacketSize(DevHandlePtr deviceHandle, USBEndpoint endpoint, int& isoMaxPacketSize);
};

...and LibusbHandlingAPI (in this class I store libusb structures, in CreateDevice function I return a pointer which I use for LibusbControlAPI and LibusbInfoAPI):

class LibusbHandlingAPI
{
private:
    typedef std::vector<DevHandlePtr> DevHandlePtrVect;

    DevHandlePtrVect m_deviceHandleVect;
    LibusbContextPtr m_libusbContext;

public:
    DevHandlePtr CreateDevice(uint16_t vendorId, uint16_t productId);
    DevHandlePtr CreateDevice(uint16_t vendorId, uint16_t productId, uint8_t busNumber, uint8_t * portNumbers, int portNumbersLength);
    void DestroyDevice(DevHandlePtr deviceHandle);

    int OpenDevice(DevHandlePtr deviceHandle);
    int CloseDevice(DevHandlePtr deviceHandle);

    int InitializeLibrary(SystemLibraryLogLevel logLevel);
    void DestroyLibrary(void);
};

So, my question is:

  • Is it good solution/class design? What do you think?
  • What are yours propositions/suggestions?
  • How can I improve it, what should i change?
  • What classes should I create/remove?
\$\endgroup\$
  • 1
    \$\begingroup\$ Your object don't contain any state. Yet all the methods in LibusbControlAPI and LibusbInfoAPI take a DevHandlePtr as a parameter. You would think there would be a USB object that abstracted away the need to keep and store this handle. As it stands this is not a wrapper (it is simply a re-packaging). \$\endgroup\$ – Martin York Mar 2 '17 at 19:52
  • \$\begingroup\$ Ok, I think I know why I am wrong. In my code i still use libusb_device* and libusb_device_handle* as keys to libusb API (packed into DevHandlePtr), and this code what I wrote only repeats/rewrites libusb API. So I think I need to create libusb API class (such thing like LibusbInfoAPI or LibusbControlAPI but the first parameter for methods shouldn't be DevHandlePtr). In brief as I understand it, my wrapper code shouldn't remind or associate with libusb API. \$\endgroup\$ – shjeff Mar 3 '17 at 8:51
  • \$\begingroup\$ I think, the thing what I need to do is to create class like LibusbControlAPI (as I mentioned in comment above, without DevHadnlePtr) and prepare constructor for injecting external object. Then I could call methods from such class without providing DevHadnlePtr as first parameter. But the object I would pass to construtor needs to store reference to libusb_device* and libusb_device_handle*, such object would be some kind of facade. \$\endgroup\$ – shjeff Mar 3 '17 at 8:59
  • 1
    \$\begingroup\$ Something like this: gist.github.com/Loki-Astari/e5a905597af6a4f61e438dc6bcde72f2 \$\endgroup\$ – Martin York Mar 3 '17 at 16:45
6
\$\begingroup\$

It is kind of hard to say without seeing the whole code but at a glance the whole thing seems kind of pointless to me. You're not making it object oriented but rather just adding your own "flavor" API over libusb.

I can tell this by the fact that I see a DevHandlePtr deviceHandle being passed around just about everywhere.

If you want to actually make a useful C++ API then you need to find usable objects in the existing API and structure your code around these objects instead. You'll know you have done it right when you stop passing "handles" around in your API.

Without reading the libusb API fully, I'll make an example to show what I mean:

class UniversalSerialBus{
public: 
    UniversalSerialBus(){
       // Initialize libusb
    }
    ~UniversalSerialBus(){
       // Shutdown libusb
    }

    std::vector<USBDeviceIdentifier> enumerateDevices() const;

    USBDeviceContext createDevice(USBDeviceIdentifier id);
}

class USBDeviceIdentifier{
public:
    int getVendorID();
    int getProductID();
    int getBusNumber();
    int getPorNumber();
    //...

}

class USBDeviceContext{
public:
    USBDeviceContext(){ /* Connect to the device */ }
    ~USBDeviceContext(){ /* Disconnect the device */ }

    bool SendInterrupt(int, int)
    bool SendBlockTransfer(int, int, Iterator dataBegin, Iterator dataEnd);
}

I hope this helps.

\$\endgroup\$
  • \$\begingroup\$ It looks pretty and clear, I will take this into account \$\endgroup\$ – shjeff Mar 3 '17 at 9:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.