From 414d25c870de3cfdd01cd2bf864f483628c441d6 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Wed, 5 Jul 2017 19:01:37 +0200 Subject: [PATCH] fix time events for rules old code would compare QTime == QTime() every second which might not trigger if e.g. milliseconds don't match. New code checks the interval between the last check and the current one. --- server/ruleengine.cpp | 8 +++++--- server/ruleengine.h | 2 ++ server/time/timedescriptor.cpp | 4 ++-- server/time/timedescriptor.h | 2 +- server/time/timeeventitem.cpp | 26 +++++++++++++++++--------- server/time/timeeventitem.h | 2 +- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/server/ruleengine.cpp b/server/ruleengine.cpp index ededa4aa..28415bd0 100644 --- a/server/ruleengine.cpp +++ b/server/ruleengine.cpp @@ -122,7 +122,8 @@ namespace guhserver { instance available from \l{GuhCore}. This one should be used instead of creating multiple ones. */ RuleEngine::RuleEngine(QObject *parent) : - QObject(parent) + QObject(parent), + m_lastEvaluationTime(QDateTime::currentDateTime()) { GuhSettings settings(GuhSettings::SettingsRoleRules); qCDebug(dcRuleEngine) << "loading rules from" << settings.fileName(); @@ -396,7 +397,7 @@ QList RuleEngine::evaluateTime(const QDateTime &dateTime) // check if this rule is based on calendarItems if (!rule.timeDescriptor().calendarItems().isEmpty()) { //qCDebug(dcRuleEngine()) << "Evaluate CalendarItem against" << dateTime.toString("dd:MM:yyyy hh:mm") << "for rule" << rule.id().toString(); - bool active = rule.timeDescriptor().evaluate(dateTime); + bool active = rule.timeDescriptor().evaluate(m_lastEvaluationTime, dateTime); if (active) { if (!m_activeRules.contains(rule.id())) { qCDebug(dcRuleEngine) << "Rule" << rule.id().toString() << "active."; @@ -418,7 +419,7 @@ QList RuleEngine::evaluateTime(const QDateTime &dateTime) // check if this rule is based on timeEventItems if (!rule.timeDescriptor().timeEventItems().isEmpty()) { - bool valid = rule.timeDescriptor().evaluate(dateTime); + bool valid = rule.timeDescriptor().evaluate(m_lastEvaluationTime, dateTime); if (valid) { rules.append(rule); } @@ -426,6 +427,7 @@ QList RuleEngine::evaluateTime(const QDateTime &dateTime) } } + m_lastEvaluationTime = QDateTime::currentDateTime(); return rules; } diff --git a/server/ruleengine.h b/server/ruleengine.h index e3b9d3fb..b840592e 100644 --- a/server/ruleengine.h +++ b/server/ruleengine.h @@ -112,6 +112,8 @@ private: QList m_ruleIds; // Keeping a list of RuleIds to keep sorting order... QHash m_rules; // ...but use a Hash for faster finding QList m_activeRules; + + QDateTime m_lastEvaluationTime; }; } diff --git a/server/time/timedescriptor.cpp b/server/time/timedescriptor.cpp index f40e7c22..d50039b9 100644 --- a/server/time/timedescriptor.cpp +++ b/server/time/timedescriptor.cpp @@ -82,7 +82,7 @@ bool TimeDescriptor::isEmpty() const valid if the \l{TimeEventItem}{TimeEventItems} or \l{CalendarItem}{CalendarItems} match the given \a dateTime. */ -bool TimeDescriptor::evaluate(const QDateTime &dateTime) const +bool TimeDescriptor::evaluate(const QDateTime &lastEvaluationTime, const QDateTime &dateTime) const { // If there are calendarItems (always OR connected) if (!m_calendarItems.isEmpty()) { @@ -96,7 +96,7 @@ bool TimeDescriptor::evaluate(const QDateTime &dateTime) const // If there are timeEventItems (always OR connected) if (!m_timeEventItems.isEmpty()) { foreach (const TimeEventItem &timeEventItem, m_timeEventItems) { - if (timeEventItem.evaluate(dateTime)) { + if (timeEventItem.evaluate(lastEvaluationTime, dateTime)) { return true; } } diff --git a/server/time/timedescriptor.h b/server/time/timedescriptor.h index b23d4cf9..ca46cb5b 100644 --- a/server/time/timedescriptor.h +++ b/server/time/timedescriptor.h @@ -41,7 +41,7 @@ public: bool isValid() const; bool isEmpty() const; - bool evaluate(const QDateTime &dateTime) const; + bool evaluate(const QDateTime &lastEvaluationTime, const QDateTime &dateTime) const; // void dumpToSettings(GuhSettings &settings, const QString &groupName) const; // static TimeDescriptor loadFromSettings(GuhSettings &settings, const QString &groupPrefix); diff --git a/server/time/timeeventitem.cpp b/server/time/timeeventitem.cpp index 5ce9a2c5..e22f5aea 100644 --- a/server/time/timeeventitem.cpp +++ b/server/time/timeeventitem.cpp @@ -90,22 +90,30 @@ bool TimeEventItem::isValid() const } /*! Returns true, if the given \a dateTime matches this \l{TimeEventItem}. */ -bool TimeEventItem::evaluate(const QDateTime &dateTime) const +bool TimeEventItem::evaluate(const QDateTime &lastEvaluationTime, const QDateTime &dateTime) const { // Check time matches if (m_time.isValid()) { switch (m_repeatingOption.mode()) { - case RepeatingOption::RepeatingModeNone: // If there is no repeating option, we assume it is meant daily. - return m_time == dateTime.time();; - case RepeatingOption::RepeatingModeHourly: - return m_time.minute() == dateTime.time().minute(); + case RepeatingOption::RepeatingModeNone: case RepeatingOption::RepeatingModeDaily: - return m_time == dateTime.time();; + return lastEvaluationTime.time() < m_time && m_time <= dateTime.time(); + case RepeatingOption::RepeatingModeHourly: { + QTime begin, vut, end; + begin.setHMS(0, lastEvaluationTime.time().minute(), lastEvaluationTime.time().second()); + end.setHMS(0, dateTime.time().minute(), dateTime.time().second()); + vut.setHMS(0, m_time.minute(), m_time.second()); + return begin < vut && vut < end; + } case RepeatingOption::RepeatingModeWeekly: - return m_repeatingOption.evaluateWeekDay(dateTime) && m_time == dateTime.time(); + return m_repeatingOption.evaluateWeekDay(dateTime) && + lastEvaluationTime.time() < m_time && + m_time <= dateTime.time(); case RepeatingOption::RepeatingModeMonthly: - return m_repeatingOption.evaluateMonthDay(dateTime) && m_time == dateTime.time(); + return m_repeatingOption.evaluateMonthDay(dateTime) && + lastEvaluationTime.time() < m_time && + m_time <= dateTime.time(); case RepeatingOption::RepeatingModeYearly: return false; } @@ -115,7 +123,7 @@ bool TimeEventItem::evaluate(const QDateTime &dateTime) const if (m_repeatingOption.mode() == RepeatingOption::RepeatingModeYearly) return m_dateTime.date().month() == dateTime.date().month() && m_dateTime.date().day() == dateTime.date().day() && - m_dateTime.time() == dateTime.time(); + lastEvaluationTime.time() < m_dateTime.time() && m_dateTime.time() <= dateTime.time(); // Check dateTime matches return dateTime == m_dateTime; diff --git a/server/time/timeeventitem.h b/server/time/timeeventitem.h index c0746739..70a6405f 100644 --- a/server/time/timeeventitem.h +++ b/server/time/timeeventitem.h @@ -45,7 +45,7 @@ public: bool isValid() const; - bool evaluate(const QDateTime &dateTime) const; + bool evaluate(const QDateTime &lastEvaluationTime, const QDateTime &dateTime) const; private: QDateTime m_dateTime;