bloguetronica / cp2130

A C++ class that uses libusb to interface with CP2130 devices. It can be used to configure the OTP ROM of such devices, as well as to control them. This class was originally derived from the corresponding class for Qt. Please refer to https://github.com/bloguetronica/cp2130-qt if you wish to use the original class instead.
2 stars 0 forks source link

Function open() should be rewritten for clarity #20

Closed samuelfmlourenco closed 2 years ago

samuelfmlourenco commented 2 years ago

As it is currently implemented, it is not very clear what value open() will return in case of success, because success is always assumed until a failure occurs, if such happens for any reason. Thus, open() should be rewritten for clarity:

// Opens the device having the given VID, PID and, optionally, the given serial number, and assigns its handle
// Since version 1.1.0, it is not required to specify a serial number
int CP2130::open(uint16_t vid, uint16_t pid, const std::string &serial)
{
    int retval;
    if (isOpen()) {  // Just in case the calling algorithm tries to open a device that was already sucessfully open, or tries to open different devices concurrently, all while using (or referencing to) the same object
        retval = SUCCESS;
    } else if (libusb_init(&context_) != 0) {  // Initialize libusb. In case of failure
        retval = ERROR_INIT;
    } else {  // If libusb is initialized
        if (serial.empty()) {  // Note that serial, by omission, is an empty string
            handle_ = libusb_open_device_with_vid_pid(context_, vid, pid);  // If no serial number is specified, this will open the first device found with matching VID and PID
        } else {
            char *serialcstr = new char[serial.size() + 1];  // Allocated dynamically since version 1.1.0
            std::strcpy(serialcstr, serial.c_str());
            handle_ = libusb_open_device_with_vid_pid_serial(context_, vid, pid, reinterpret_cast<unsigned char *>(serialcstr));
            delete[] serialcstr;
        }
        if (handle_ == nullptr) {  // If the previous operation fails to get a device handle
            libusb_exit(context_);  // Deinitialize libusb
            retval = ERROR_NOT_FOUND;
        } else {  // If the device is successfully opened and a handle obtained
            if (libusb_kernel_driver_active(handle_, 0) == 1) {  // If a kernel driver is active on the interface
                libusb_detach_kernel_driver(handle_, 0);  // Detach the kernel driver
                kernelWasAttached_ = true;  // Flag that the kernel driver was attached
            } else {
                kernelWasAttached_ = false;  // The kernel driver was not attached
            }
            if (libusb_claim_interface(handle_, 0) != 0) {  // Claim the interface. In case of failure
                if (kernelWasAttached_) {  // If a kernel driver was attached to the interface before
                    libusb_attach_kernel_driver(handle_, 0);  // Reattach the kernel driver
                }
                libusb_close(handle_);  // Close the device
                libusb_exit(context_);  // Deinitialize libusb
                handle_ = nullptr;  // Required to mark the device as closed
                retval = ERROR_BUSY;
            } else {
                disconnected_ = false;  // Note that this flag is never assumed to be true for a device that was never opened - See constructor for details!
                retval = SUCCESS;
            }
        }
    }
    return retval;
}
samuelfmlourenco commented 2 years ago

Implemented in version 1.2.2.