[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