bloguetronica / cp2130-conf

CP2130 Configurator (cp2130-conf) is an application that can be used to configure CP2130 devices, including VID, PID, as well as other descriptors. Most importantly, the application allows you to configure pin functions and states.
GNU General Public License v3.0
1 stars 1 forks source link

In "mainwindow.cpp" and "mainwindow.h", on_comboBoxDevices_currentIndexChanged() should be renamed to on_comboBoxDevices_activated() #14

Closed samuelfmlourenco closed 2 years ago

samuelfmlourenco commented 2 years ago

The slot "on_comboBoxDevices_currentIndexChanged()" is triggered when the change is done either by the user or programmatically. However, since this combo box is only changed by the user, and does need to be changed programmatically, or its action triggered that way, the slot should be renamed to "on_comboBoxDevices_activated()".

For a reference, please visit https://doc.qt.io/qt-5/qcombobox.html#activated.

samuelfmlourenco commented 2 years ago

This is not a bug. After all, the combo box is changed programmatically when refresh() is called. In fact, this is currently mentioned in the source code, in line 120. For reference:

// Checks for valid user input, enabling or disabling the combo box and the "Refresh" button, accordingly
void MainWindow::validateInput()
{
    QString vidstr = ui->lineEditVID->text();
    QString pidstr = ui->lineEditPID->text();
    if (vidstr.size() == 4 && pidstr.size() == 4) {
        vid_ = static_cast<quint16>(vidstr.toInt(nullptr, 16));
        pid_ = static_cast<quint16>(pidstr.toInt(nullptr, 16));
        ui->comboBoxDevices->setEnabled(true);
        ui->pushButtonRefresh->setEnabled(true);
    } else {
        ui->comboBoxDevices->setEnabled(false);
        ui->pushButtonRefresh->setEnabled(false);
    }
    refresh();  // This also disables the "Open" button - Note that this is the intended behavior!
}

In fact, renaming the slot as proposed would have the undesirable effect of not disabling the "Open" button if the user decided to clear either the "Vendor ID" or the "Product ID" fields, after filling those and after selecting a device. So, the programmatic change does need to be acknowledged as well, in order to disable the "Open" button in such event.

This will not be implemented.