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

Device information functionality does not acknowledge a device disconnection #6

Closed samuelfmlourenco closed 2 years ago

samuelfmlourenco commented 2 years ago

After disconnecting a device, selecting Device > Information on its window does not show a message reporting that the device was disconnected, or any other error. It does nothing instead.

samuelfmlourenco commented 2 years ago

Confirmed that the error is not being reported because opCheck no longer displays error messages directly, as implemented in version 1.1. This is necessary to prevent the application from freezing. Instead, error messages are being printed elsewhere. However, on_actionInformation_triggered() was not changed to conform to the new implementation.

void ConfiguratorWindow::on_actionInformation_triggered()
{
    int errcnt = 0;
    QString errstr;
    InformationDialog info;
    CP2130::SiliconVersion siversion = cp2130_.getSiliconVersion(errcnt, errstr);
    info.setSiliconVersionLabelText(siversion.maj, siversion.min);
    if (opCheck(tr("device-information-retrieval-op"), errcnt, errstr)) {  // If error check passes (the string "device-information-retrieval-op" should be translated to "Device information retrieval")
        info.exec();
    }
}

The previous function should print the messages relayed by opCheck(), instead of relying on the latter to do so. It should check configerr and errmsg to do so.

Opened a new milestone for this particular fix.

samuelfmlourenco commented 2 years ago

Bug fixed in version 1.2. Here is the on_actionInformation_triggered() implementation:

void ConfiguratorWindow::on_actionInformation_triggered()
{
    int errcnt = 0;
    QString errstr;
    InformationDialog info;
    CP2130::SiliconVersion siversion = cp2130_.getSiliconVersion(errcnt, errstr);
    info.setSiliconVersionLabelText(siversion.maj, siversion.min);
    opCheck(tr("device-information-retrieval-op"), errcnt, errstr);  // The string "device-information-retrieval-op" should be translated to "Device information retrieval"
    if (err_) {  // Fix implemented in version 1.2
        handleError();
    } else {  // If error check passes
        info.exec();
    }
}

For reference:

// Determines the type of error and acts accordingly, always showing a message
void ConfiguratorWindow::handleError()
{
    QMessageBox::critical(this, tr("Error"), errmsg_);
    if (cp2130_.disconnected() || !cp2130_.isOpen()) {
        disableView();  // Disable configurator window
        cp2130_.close();  // If the device is already closed, this will have no effect
    }
}

// Checks for errors and validates device operations
// Void since version 1.2, since the return value was found to be redundant
void ConfiguratorWindow::opCheck(const QString &op, int errcnt, QString errstr)
{
    if (errcnt > 0) {
        err_ = true;
        if (cp2130_.disconnected()) {
            errmsg_ = tr("Device disconnected.\n\nThe corresponding window will be disabled.");
        } else {
            errstr.chop(1);  // Remove the last character, which is always a newline
            errmsg_ = tr("%1 operation returned the following error(s):\n– %2", "", errcnt).arg(op, errstr.replace("\n", "\n– "));
        }
    }
}

Note that the boolean configerr is now simply named err, because it has a broader meaning.