improved error replies in helper

helper/CMakeLists.txt

@@ -1,4 +1,5 @@
-set(LIBRARIES KF5::Auth)
+set(LIBRARIES KF5::Auth
+              KF5::I18n)
 
 if(NOT NO_SYSTEMD)
 

helper/src/helper.cpp

@@ -24,6 +24,7 @@
 #include <QtCore/QProcess>
 
 #include <KAuth/KAuthHelperSupport>
+#include <KI18n/KLocalizedString>
 
 #ifndef NO_SYSTEMD
 #include <QtDBus/QDBusInterface>
@@ -72,7 +73,7 @@ ActionReply Helper::action(const QVariantMap &arguments)
         if (!file.open(QIODevice::ReadOnly))
         {
            reply = ActionReply::HelperErrorType;
-           reply.setErrorCode(ActionReply::AuthorizationDeniedError);
+           reply.setErrorDescription(file.errorString());
 
            return reply;
         }
@@ -94,7 +95,7 @@ ActionReply Helper::action(const QVariantMap &arguments)
         if (!file.open(QIODevice::WriteOnly))
         {
            reply = ActionReply::HelperErrorType;
-           reply.addData("errorDescription", file.errorString());
+           reply.setErrorDescription(file.errorString());
 
            return reply;
         }
@@ -114,7 +115,7 @@ ActionReply Helper::action(const QVariantMap &arguments)
         if (!process.waitForStarted(10000))
         {
             reply = ActionReply::HelperErrorType;
-            reply.addData("errorDescription", process.errorString());
+            reply.setErrorDescription(process.errorString());
             
             return reply;
         }
@@ -122,11 +123,17 @@ ActionReply Helper::action(const QVariantMap &arguments)
         if (!process.waitForFinished(10000))
         {
             reply = ActionReply::HelperErrorType;
-            reply.addData("errorDescription", process.errorString());
+            reply.setErrorDescription(process.errorString());
             
             return reply;
         }
     }
+    else
+    {
+        reply.setType(ActionReply::HelperErrorType);
+        reply.setErrorCode(ActionReply::NoSuchActionError);
+        reply.setErrorDescription(i18n("This action does not exist!"));
+    }
     
     return reply;
 }

lib/src/pwmfan.cpp

@@ -172,7 +172,7 @@ bool PwmFan::setPwm(int pwm, bool write)
                 action.setHelperId("fancontrol.gui.helper");
                 if (!action.isValid())
                 {
-                    qCritical() << "setPwm action is invalid";
+                    emit errorChanged("setPwm action is invalid");
                     return false;
                 }
                 
@@ -181,19 +181,25 @@ bool PwmFan::setPwm(int pwm, bool write)
                 map["filename"] = qobject_cast<QFile *>(m_pwmStream->device())->fileName();
                 map["content"] = QString::number(pwm);
                 action.setArguments(map);
-                KAuth::ExecuteJob *reply = action.execute();
-
-                if (!reply->exec())
-                {
-                    qCritical() << "setPwm error:" << reply->errorString() << reply->errorText();
-                    return false;
-                }
+                KAuth::ExecuteJob *job = action.execute();
+                connect(job, SIGNAL(result(KJob*)), this, SLOT(handleSetPwmResult(KJob*)));
+                job->start();
             }
         }
     }
     return true;
 }
 
+void PwmFan::handleSetPwmResult(KJob *job)
+{
+    if (job->error())
+    {
+        emit errorChanged(QString("setPwm error:" + job->errorString() + job->errorText()));
+        return;
+    }
+    update();
+}
+
 bool PwmFan::setPwmMode(int pwmMode, bool write)
 {
     if (m_pwmMode != pwmMode)
@@ -212,7 +218,7 @@ bool PwmFan::setPwmMode(int pwmMode, bool write)
                 action.setHelperId("fancontrol.gui.helper");
                 if (!action.isValid())
                 {
-                    qCritical() << "setPwmMode action is invalid";
+                    emit errorChanged("setPwmMode action is invalid");
                     return false;
                 }
                 
@@ -221,47 +227,77 @@ bool PwmFan::setPwmMode(int pwmMode, bool write)
                 map["filename"] = qobject_cast<QFile *>(m_modeStream->device())->fileName();
                 map["content"] = QString::number(pwmMode);
                 action.setArguments(map);
-                KAuth::ExecuteJob *reply = action.execute();
-
-                if (!reply->exec())
-                {
-                    qCritical() << "setPwmMode error:" << reply->errorString() << reply->errorText();
-                    return false;
-                }
+                KAuth::ExecuteJob *job = action.execute();
+                connect(job, SIGNAL(result(KJob*)), this, SLOT(handleSetPwmModeResult(KJob*)));
+                job->start();
             }
         }
     }
     return true;
 }
 
-bool PwmFan::test()
+void PwmFan::handleSetPwmModeResult(KJob *job)
 {
-    if (setPwmMode(1) && setPwm(255))
+    if (job->error())
     {
-        m_testStatus = FindingStop1;
-        emit testingChanged();
-        
-        QTimer::singleShot(500, this, SLOT(continueTest()));
-        qDebug() << "Start testing...";
+        emit errorChanged(QString("setPwmMode error:" + job->errorString() + job->errorText()));
+        return;
     }
-    else
+    update();
+}
+
+bool PwmFan::test()
+{
+    KAuth::Action action("fancontrol.gui.helper.action");
+    action.setHelperId("fancontrol.gui.helper");
+    if (!action.isValid())
     {
-        qCritical() << "Testing failed";
+        emit errorChanged("Test action is invalid");
         return false;
     }
+    KAuth::ExecuteJob *job = action.execute(KAuth::Action::AuthorizeOnlyMode);
+    connect(job, SIGNAL(statusChanged(int)), this, SLOT(handleTestAuthReply(int)));
+    job->start();
     
     return true;
 }
 
+void PwmFan::handleTestAuthReply(int authStatus)
+{
+    switch (authStatus)
+    {
+        case KAuth::Action::AuthorizedStatus:
+        {
+            setPwmMode(1);
+            setPwm(255);
+            
+            m_testStatus = FindingStop1;
+            emit testingChanged();
+            
+            QTimer::singleShot(500, this, SLOT(continueTest()));
+            qDebug() << "Start testing...";
+            break;
+        }
+        case KAuth::Action::ErrorStatus:
+        {
+            emit errorChanged("Test action error");
+            break;
+        }
+    }
+}
+
 void PwmFan::abortTest()
 {
-    setPwm(255);
-    disconnect(0, 0, this, SLOT(continueTest()));
+    if (m_testStatus >= FindingStop1 && m_testStatus <= FindingStart)
+    {
+        setPwm(255);
+        disconnect(0, 0, this, SLOT(continueTest()));
 
-    m_testStatus = Cancelled;
-    emit testingChanged();
+        m_testStatus = Cancelled;
+        emit testingChanged();
 
-    setPwm(255);
+        setPwm(255);
+    }
 }
 
 void PwmFan::continueTest()

lib/src/pwmfan.h

@@ -30,6 +30,7 @@
 
 
 class QTextStream;
+class KJob;
 
 namespace Fancontrol
 {
@@ -103,6 +104,9 @@ protected slots:
 
     void update() Q_DECL_OVERRIDE;
     void continueTest();
+    void handleSetPwmResult(KJob *job);
+    void handleSetPwmModeResult(KJob *job);
+    void handleTestAuthReply(int authStatus);
 
 
 private:

lib/src/systemdcommunicator.cpp

@@ -265,7 +265,7 @@ bool SystemdCommunicator::dbusAction(const QString &method, const QVariantList &
     return true;
 }
 
-void SystemdCommunicator::handleDbusActionReply(KJob *job)
+void SystemdCommunicator::handleDbusActionResult(KJob *job)
 {
     if (job->error())
         setError(job->errorString() + job->errorText());

lib/src/systemdcommunicator.h

@@ -66,7 +66,7 @@ signals:
 protected slots:
     
     void updateServiceProperties(QString, QVariantMap, QStringList);
-    void handleDbusActionReply(KJob *job);
+    void handleDbusActionResult(KJob *job);
     
     
 protected: