fix #283

fix #303

small bug fixes and add logging for rule enable/disable and rule action execution

guh.pri

@@ -2,7 +2,7 @@
 GUH_VERSION_STRING=$$system('dpkg-parsechangelog | sed -n -e "s/^Version: //p"')
 
 # define protocol versions
-JSON_PROTOCOL_VERSION=44
+JSON_PROTOCOL_VERSION=45
 REST_API_VERSION=1
 
 DEFINES += GUH_VERSION_STRING=\\\"$${GUH_VERSION_STRING}\\\" \

libguh/devicemanager.cpp

@@ -856,14 +856,46 @@ DeviceManager::DeviceError DeviceManager::verifyParam(const ParamType &paramType
             return DeviceErrorInvalidParameter;
         }
 
-        if (paramType.maxValue().isValid() && param.value() > paramType.maxValue()) {
+        if (paramType.type() == QVariant::Int) {
-            qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Max:" << paramType.maxValue();
+            if (paramType.maxValue().isValid() && param.value().toInt() > paramType.maxValue().toInt()) {
-            return DeviceErrorInvalidParameter;
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Max:" << paramType.maxValue();
-        }
+                return DeviceErrorInvalidParameter;
+            }
 
-        if (paramType.minValue().isValid() && param.value() < paramType.minValue()) {
+            if (paramType.minValue().isValid() && param.value().toInt() < paramType.minValue().toInt()) {
-            qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Min:" << paramType.minValue();
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Min:" << paramType.minValue();
-            return DeviceErrorInvalidParameter;
+                return DeviceErrorInvalidParameter;
+            }
+        } else if (paramType.type() == QVariant::UInt) {
+            if (paramType.maxValue().isValid() && param.value().toUInt() > paramType.maxValue().toUInt()) {
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Max:" << paramType.maxValue();
+                return DeviceErrorInvalidParameter;
+            }
+
+            if (paramType.minValue().isValid() && param.value().toUInt() < paramType.minValue().toUInt()) {
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Min:" << paramType.minValue();
+                return DeviceErrorInvalidParameter;
+            }
+        } else if (paramType.type() == QVariant::Double) {
+            if (paramType.maxValue().isValid() && param.value().toDouble() > paramType.maxValue().toDouble()) {
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Max:" << paramType.maxValue();
+                return DeviceErrorInvalidParameter;
+            }
+
+            if (paramType.minValue().isValid() && param.value().toDouble() < paramType.minValue().toDouble()) {
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Min:" << paramType.minValue();
+                return DeviceErrorInvalidParameter;
+            }
+        } else {
+            if (paramType.maxValue().isValid() && param.value() > paramType.maxValue()) {
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Max:" << paramType.maxValue();
+                return DeviceErrorInvalidParameter;
+            }
+
+            if (paramType.minValue().isValid() && param.value() < paramType.minValue()) {
+                qCWarning(dcDeviceManager) << "Value out of range for param" << param.name() << " Got:" << param.value() << " Min:" << paramType.minValue();
+                return DeviceErrorInvalidParameter;
+            }
         }
 
         if (!paramType.allowedValues().isEmpty() && !paramType.allowedValues().contains(param.value())) {

server/guhapplication.cpp

@@ -163,8 +163,6 @@ static void catchUnixSignals(const std::vector<int>& quitSignals, const std::vec
 }
 
 
-
-
 /*! Constructs a GuhApplication with the given argument count \a argc and argument vector \a argv. */
 GuhApplication::GuhApplication(int &argc, char **argv) :
     QCoreApplication(argc, argv)

server/guhcore.cpp

@@ -99,7 +99,6 @@
 #include "loggingcategories.h"
 #include "jsonrpcserver.h"
 #include "ruleengine.h"
-#include "logging/logengine.h"
 
 #include "devicemanager.h"
 #include "plugin/device.h"
@@ -327,8 +326,8 @@ void GuhCore::executeRuleActions(const QList<RuleAction> ruleActions)
             qCWarning(dcRuleEngine) << "Error executing action:" << status;
         }
 
-        if (status != DeviceManager::DeviceErrorAsync)
+//        if (status != DeviceManager::DeviceErrorAsync)
-            m_logger->logAction(action, status == DeviceManager::DeviceErrorNoError ? Logging::LoggingLevelInfo : Logging::LoggingLevelAlert, status);
+//            m_logger->logAction(action, status == DeviceManager::DeviceErrorNoError ? Logging::LoggingLevelInfo : Logging::LoggingLevelAlert, status);
     }
 }
 
@@ -337,7 +336,7 @@ void GuhCore::executeRuleActions(const QList<RuleAction> ruleActions)
 RuleEngine::RuleError GuhCore::removeRule(const RuleId &id)
 {
     RuleEngine::RuleError removeError = m_ruleEngine->removeRule(id);
-    if (removeError != RuleEngine::RuleErrorNoError)
+    if (removeError == RuleEngine::RuleErrorNoError)
         m_logger->removeRuleLogs(id);
 
     return removeError;

server/guhcore.h

@@ -28,6 +28,8 @@
 #include "plugin/deviceclass.h"
 #include "plugin/devicedescriptor.h"
 
+#include "logging/logengine.h"
+#include "guhconfiguration.h"
 #include "devicemanager.h"
 #include "ruleengine.h"
 #include "servermanager.h"

server/jsonrpc/jsontypes.cpp

@@ -804,6 +804,10 @@ QVariantMap JsonTypes::packLogEntry(const LogEntry &logEntry)
         logEntryMap.insert("active", logEntry.active());
     }
 
+    if (logEntry.eventType() == Logging::LoggingEventTypeEnabledChange) {
+        logEntryMap.insert("active", logEntry.active());
+    }
+
     if (logEntry.level() == Logging::LoggingLevelAlert) {
         switch (logEntry.source()) {
         case Logging::LoggingSourceRules:

server/logging/logengine.cpp

@@ -194,10 +194,10 @@ QList<LogEntry> LogEngine::logEntries(const LogFilter &filter) const
         entry.setTypeId(query.value("typeId").toUuid());
         entry.setDeviceId(DeviceId(query.value("deviceId").toString()));
         entry.setValue(query.value("value").toString());
-        if ((Logging::LoggingEventType)query.value("loggingEventType").toInt() == Logging::LoggingEventTypeActiveChange) {
+        entry.setEventType((Logging::LoggingEventType)query.value("loggingEventType").toInt());
-            entry.setActive(query.value("active").toBool());
+        entry.setActive(query.value("active").toBool());
-        }
+        entry.setActive(query.value("active").toBool());
-        //qCDebug(dcLogEngine) << entry;
+        qCDebug(dcLogEngine) << entry;
         results.append(entry);
     }
     qCDebug(dcLogEngine) << "Fetched" << results.count() << "entries for db query:" << queryCall;
@@ -222,6 +222,7 @@ void LogEngine::clearDatabase()
 void LogEngine::logSystemEvent(const QDateTime &dateTime, bool active, Logging::LoggingLevel level)
 {
     LogEntry entry(dateTime, level, Logging::LoggingSourceSystem);
+    entry.setEventType(Logging::LoggingEventTypeActiveChange);
     entry.setActive(active);
     appendLogEntry(entry);
 }
@@ -263,6 +264,7 @@ void LogEngine::logRuleTriggered(const Rule &rule)
 {
     LogEntry entry(Logging::LoggingSourceRules);
     entry.setTypeId(rule.id());
+    entry.setEventType(Logging::LoggingEventTypeTrigger);
     appendLogEntry(entry);
 }
 
@@ -271,6 +273,32 @@ void LogEngine::logRuleActiveChanged(const Rule &rule)
     LogEntry entry(Logging::LoggingSourceRules);
     entry.setTypeId(rule.id());
     entry.setActive(rule.active());
+    entry.setEventType(Logging::LoggingEventTypeActiveChange);
+    appendLogEntry(entry);
+}
+
+void LogEngine::logRuleEnabledChanged(const Rule &rule, const bool &enabled)
+{
+    LogEntry entry(Logging::LoggingSourceRules);
+    entry.setTypeId(rule.id());
+    entry.setEventType(Logging::LoggingEventTypeEnabledChange);
+    entry.setActive(enabled);
+    appendLogEntry(entry);
+}
+
+void LogEngine::logRuleActionsExecuted(const Rule &rule)
+{
+    LogEntry entry(Logging::LoggingSourceRules);
+    entry.setTypeId(rule.id());
+    entry.setEventType(Logging::LoggingEventTypeActionsExecuted);
+    appendLogEntry(entry);
+}
+
+void LogEngine::logRuleExitActionsExecuted(const Rule &rule)
+{
+    LogEntry entry(Logging::LoggingSourceRules);
+    entry.setTypeId(rule.id());
+    entry.setEventType(Logging::LoggingEventTypeExitActionsExecuted);
     appendLogEntry(entry);
 }
 
@@ -304,6 +332,8 @@ void LogEngine::appendLogEntry(const LogEntry &entry)
 {
     checkDBSize();
 
+    qCDebug(dcLogEngine()) << "Add log" << entry;
+
     QString queryString = QString("INSERT INTO entries (timestamp, loggingEventType, loggingLevel, sourceType, typeId, deviceId, value, active, errorCode) values ('%1', '%2', '%3', '%4', '%5', '%6', '%7', '%8', '%9');")
             .arg(entry.timestamp().toTime_t())
             .arg(entry.eventType())
@@ -410,11 +440,15 @@ void LogEngine::initDB()
                    "FOREIGN KEY(sourceType) REFERENCES sourceTypes(id),"
                    "FOREIGN KEY(loggingEventType) REFERENCES loggingEventTypes(id)"
                    ");");
-        if (query.lastError().isValid()) {
+
+        if (query.lastError().isValid())
             qCWarning(dcLogEngine) << "Error creating log table in database. Driver error:" << query.lastError().driverText() << "Database error:" << query.lastError().databaseText();
-        }
+
+
     }
+
     qCDebug(dcLogEngine) << "Initialized logging DB successfully. (maximum DB size:" << m_dbMaxSize << ")";
+
 }
 
 }

server/logging/logengine.h

@@ -57,13 +57,18 @@ private:
     void checkDBSize();
 
 private:
-    // Only GuhCore is allowed to log events.
+    // Only GuhCore and ruleEngine are allowed to log events.
     friend class GuhCore;
+    friend class RuleEngine;
+
     void logSystemEvent(const QDateTime &dateTime, bool active, Logging::LoggingLevel level = Logging::LoggingLevelInfo);
     void logEvent(const Event &event);
     void logAction(const Action &action, Logging::LoggingLevel level = Logging::LoggingLevelInfo, int errorCode = 0);
     void logRuleTriggered(const Rule &rule);
     void logRuleActiveChanged(const Rule &rule);
+    void logRuleEnabledChanged(const Rule &rule, const bool &enabled);
+    void logRuleActionsExecuted(const Rule &rule);
+    void logRuleExitActionsExecuted(const Rule &rule);
     void removeDeviceLogs(const DeviceId &deviceId);
     void removeRuleLogs(const RuleId &ruleId);
 

server/logging/logentry.cpp

@@ -129,6 +129,12 @@ Logging::LoggingEventType LogEntry::eventType() const
     return m_eventType;
 }
 
+/*! Sets the \a eventType of this \l{LogEntry}. */
+void LogEntry::setEventType(const Logging::LoggingEventType &eventType)
+{
+    m_eventType = eventType;
+}
+
 /*! Returns true if this \l{LogEntry} is a system active type. */
 bool LogEntry::active() const
 {
@@ -138,7 +144,6 @@ bool LogEntry::active() const
 /*! Sets this \l{LogEntry} to \a active. */
 void LogEntry::setActive(bool active)
 {
-    m_eventType = Logging::LoggingEventTypeActiveChange;
     m_active = active;
 }
 
@@ -151,7 +156,7 @@ int LogEntry::errorCode() const
 
 QDebug operator<<(QDebug dbg, const LogEntry &entry)
 {
-    dbg.nospace() << "LogEntry (count:" << entry.timestamp().toString() << endl;
+    dbg.nospace() << "LogEntry (" << entry.timestamp().toString() << ")" << endl;
     dbg.nospace() << " time stamp: " << entry.timestamp().toTime_t() << endl;
     dbg.nospace() << "   DeviceId: " << entry.deviceId().toString() << endl;
     dbg.nospace() << "    type id: " << entry.typeId().toString() << endl;

server/logging/logentry.h

@@ -43,7 +43,9 @@ public:
     QDateTime timestamp() const;
     Logging::LoggingLevel level() const;
     Logging::LoggingSource source() const;
+
     Logging::LoggingEventType eventType() const;
+    void setEventType(const Logging::LoggingEventType &eventType);
 
     // Valid for LoggingSourceStates, LoggingSourceEvents, LoggingSourceActions, LoggingSourceRules
     QUuid typeId() const;
@@ -69,7 +71,7 @@ private:
     Logging::LoggingLevel m_level;
     Logging::LoggingSource m_source;
 
-    // RuleSource specifiv properties.
+    // RuleSource specific properties.
     // FIXME: If it turns out we need many more of those, we should subclass LogEntry with specific ones.
     QUuid m_typeId;
     DeviceId m_deviceId;
@@ -78,6 +80,7 @@ private:
     bool m_active;
     int m_errorCode;
 };
+
 QDebug operator<<(QDebug dbg, const LogEntry &entry);
 
 }

server/logging/logging.h

@@ -58,12 +58,12 @@ public:
     enum LoggingEventType {
         LoggingEventTypeTrigger,
         LoggingEventTypeActiveChange,
+        LoggingEventTypeEnabledChange,
         LoggingEventTypeActionsExecuted,
         LoggingEventTypeExitActionsExecuted
     };
 
     Logging(QObject *parent = 0);
-
 };
 
 }

server/ruleengine.cpp

@@ -719,13 +719,18 @@ RuleEngine::RuleError RuleEngine::enableRule(const RuleId &ruleId)
         qCWarning(dcRuleEngine) << "Rule not found. Can't enable it";
         return RuleErrorRuleNotFound;
     }
+
     Rule rule = m_rules.value(ruleId);
+    if (rule.enabled())
+        return RuleErrorNoError;
+
     rule.setEnabled(true);
     m_rules[ruleId] = rule;
-
     saveRule(rule);
     emit ruleConfigurationChanged(rule);
 
+    GuhCore::instance()->logEngine()->logRuleEnabledChanged(rule, true);
+
     return RuleErrorNoError;
 }
 
@@ -741,10 +746,16 @@ RuleEngine::RuleError RuleEngine::disableRule(const RuleId &ruleId)
     }
 
     Rule rule = m_rules.value(ruleId);
+    if (!rule.enabled())
+        return RuleErrorNoError;
+
     rule.setEnabled(false);
     m_rules[ruleId] = rule;
     saveRule(rule);
     emit ruleConfigurationChanged(rule);
+
+    GuhCore::instance()->logEngine()->logRuleEnabledChanged(rule, false);
+
     return RuleErrorNoError;
 }
 
@@ -778,6 +789,7 @@ RuleEngine::RuleError RuleEngine::executeActions(const RuleId &ruleId)
     }
 
     qCDebug(dcRuleEngine) << "Executing rule actions of rule" << rule.name() << rule.id();
+    GuhCore::instance()->logEngine()->logRuleActionsExecuted(rule);
     GuhCore::instance()->executeRuleActions(rule.actions());
     return RuleErrorNoError;
 }
@@ -809,6 +821,7 @@ RuleEngine::RuleError RuleEngine::executeExitActions(const RuleId &ruleId)
     }
 
     qCDebug(dcRuleEngine) << "Executing rule exit actions of rule" << rule.name() << rule.id();
+    GuhCore::instance()->logEngine()->logRuleExitActionsExecuted(rule);
     GuhCore::instance()->executeRuleActions(rule.exitActions());
     return RuleErrorNoError;
 }

tests/auto/api.json

@@ -1,4 +1,4 @@
-44
+45
 {
     "methods": {
         "Actions.ExecuteAction": {
@@ -941,6 +941,7 @@
         "LoggingEventType": [
             "LoggingEventTypeTrigger",
             "LoggingEventTypeActiveChange",
+            "LoggingEventTypeEnabledChange",
             "LoggingEventTypeActionsExecuted",
             "LoggingEventTypeExitActionsExecuted"
         ],