[Merge] lp:~jonas-drange/ubuntu-ui-extras/consolidated-devices into lp:ubuntu-ui-extras/staging

Andrew Hayzen andrew.hayzen at canonical.com
Thu Apr 6 10:02:36 UTC 2017


Review: Needs Fixing

The changes look good.

One inline comment:
1) Looks like this is missing
proc.start(program, arguments);
?

Also I think ubuntu-ui-extras should now depends or at least recommend the package hplip.

Diff comments:

> 
> === modified file 'modules/Ubuntu/Components/Extras/Printers/cups/devicesearcher.cpp'
> --- modules/Ubuntu/Components/Extras/Printers/cups/devicesearcher.cpp	2017-03-08 14:47:16 +0000
> +++ modules/Ubuntu/Components/Extras/Printers/cups/devicesearcher.cpp	2017-04-05 15:19:56 +0000
> @@ -68,3 +89,123 @@
>  {
>      Q_EMIT loaded(device);
>  }
> +
> +QList<Device> DeviceSearcher::createHpDevices(const Device &device)
> +{
> +    QList<Device> ret;
> +
> +    auto deviceHost = device.host();
> +    auto makeModelLower = device.makeModel.toLower();
> +    bool isHpType = device.type() == PrinterEnum::DeviceType::HPType;
> +    bool isNetwork = device.cls == QStringLiteral("network");
> +    bool haveMakeModel = !device.makeModel.isEmpty();
> +    bool unknownMake = device.makeModel == QStringLiteral("Unknown");
> +    bool startsWithHp = (makeModelLower.startsWith(QStringLiteral("hp")) ||
> +                         makeModelLower.startsWith(QStringLiteral("hewlett")));
> +
> +    if (!isHpType && isNetwork && (!haveMakeModel || unknownMake ||
> +                                   startsWithHp)) {
> +        auto uri = hplipUri(deviceHost, HpPrinterMode::HpPrinter);
> +
> +        // We couldn't get anything useful from the hplip tools.
> +        if (uri.isEmpty()) {
> +            return ret;
> +        }
> +
> +        // We now have a hp printer device we want to add to the list.
> +        Device printerDev(device);
> +        printerDev.uri = uri;
> +        ret << printerDev;
> +
> +        // TODO: Check if it's able to scan.
> +
> +        auto faxUri = hplipUri(deviceHost, HpPrinterMode::HpFax);
> +        if (!faxUri.isEmpty()) {
> +            Device faxDev(device);
> +            faxDev.uri = faxUri;
> +            faxDev.id = hpfaxDeviceId(faxUri);
> +            faxDev.info = __("Fax");
> +            ret << faxDev;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
> +QString DeviceSearcher::hplipUri(const QString &host,
> +                                 const HpPrinterMode &mode)
> +{
> +    QString program = "hp-makeuri";
> +    QStringList arguments;
> +    QString ret;
> +
> +    switch (mode) {
> +    default:
> +    case HpPrinterMode::HpPrinter:
> +        arguments << "-c";
> +        break;
> +    case HpPrinterMode::HpFax:
> +        arguments << "-f";
> +        break;
> +    }
> +
> +    arguments << host;
> +
> +    QProcess proc;
> +    proc.start(program, arguments);
> +
> +    if (proc.waitForFinished() && proc.exitStatus() == QProcess::NormalExit) {
> +        ret = QString(proc.readAllStandardOutput()).trimmed();
> +    }
> +    return ret;
> +}
> +
> +QString DeviceSearcher::hpfaxDeviceId(const QString &faxUri)
> +{
> +    auto env = QProcessEnvironment(QProcessEnvironment::systemEnvironment());
> +    env.insert("LC_ALL", "C");
> +    env.insert("DISPLAY", "");
> +
> +    QString ret;
> +    QString program = "hp-info";
> +    QStringList arguments;
> +    arguments << "-x" << "-i" << "-d" << faxUri;
> +
> +    QProcess proc;
> +    proc.setProcessEnvironment(env);

Looks like this is missing
proc.start(program, arguments);
?

> +
> +    QRegularExpression faxTypeRe("(\\d+)");
> +    int faxType = -1;
> +
> +    if (proc.waitForFinished() && proc.exitStatus() == QProcess::NormalExit) {
> +        QStringList lines = QString(proc.readAllStandardOutput()).split("\n");
> +        Q_FOREACH(auto line, lines) {
> +            if (!line.contains(QStringLiteral("fax-type"))) {
> +                continue;
> +            }
> +
> +            auto match = faxTypeRe.match(line);
> +            if (match.hasMatch()) {
> +                auto capturedFaxType = match.captured(0);
> +                bool ok;
> +                int candidateFaxType = capturedFaxType.toInt(&ok);
> +                if (ok) {
> +                    faxType = candidateFaxType;
> +                }
> +            }
> +
> +            if (faxType >= 0) {
> +                break;
> +            }
> +        }
> +
> +        if (faxType <= 0) {
> +            return QString::null;
> +        } else if (faxType == 4) {
> +            return QStringLiteral("MFG:HP;MDL:Fax 2;DES:HP Fax 2;");
> +        } else {
> +            return QStringLiteral("MFG:HP;MDL:Fax;DES:HP Fax;");
> +        }
> +    }
> +    return ret;
> +}


-- 
https://code.launchpad.net/~jonas-drange/ubuntu-ui-extras/consolidated-devices/+merge/321497
Your team Ubuntu Phablet Team is subscribed to branch lp:ubuntu-ui-extras/staging.



More information about the Ubuntu-reviews mailing list