Bug fixing and refactor

1. AccessoriesSwitchProvider is not processing hardware back button
2. Memory faults on empty switches removed
3. Keys unmapped to switches can stop previous switch interaction
4. SwitchConfigurationItem enable additional constructor for lambda functions

Change-Id: I2085c7e0926188b5f4315af19b2765205e698833

diff --git a/src/AccessoriesSwitchProvider.cpp b/src/AccessoriesSwitchProvider.cpp
index cfda9ac4fe55e6e71ffbbcfbac48e0374b26c29b..4545cc9cebc81e545aa6b3fbb8a0bc29501e28d3 100644 (file)
--- a/src/AccessoriesSwitchProvider.cpp
+++ b/src/AccessoriesSwitchProvider.cpp
@@ -704,26 +704,50 @@ std::shared_ptr<Switch> AccessoriesSwitchProvider::findSwitchById(const std::str
        return nullptr;
 }
 
-void AccessoriesSwitchProvider::processKeyDownEvent(const std::string &keyId)
+Eina_Bool AccessoriesSwitchProvider::processKeyDownEvent(const std::string &keyId)
 {
        if (currentKeyId == keyId)
-               return;
+               return ECORE_CALLBACK_DONE;
 
-       currentKeyId = keyId;
-       auto switchId = SwitchId(currentKeyId, getId());
+       DEBUG("key pressed: %s", keyId.c_str());
+
+       auto switchId = SwitchId(keyId, getId());
        auto sw = findSwitchById(switchId.getGlobalId());
+       if (!sw) {
+               auto id = SwitchId(prevKeyId, getId());
+               interactionManager.stopInteraction(findSwitchById(id.getGlobalId()));
+               return ECORE_CALLBACK_PASS_ON;
+       }
+
+       currentKeyId = keyId;
+       prevKeyId = keyId;
+
        interactionManager.startInteraction([ = ]() {
                this->notify(sw);
        }, sw);
+
+       return ECORE_CALLBACK_DONE;
 }
 
-void AccessoriesSwitchProvider::processKeyUpEvent(const std::string &keyId)
+Eina_Bool AccessoriesSwitchProvider::processKeyUpEvent(const std::string &keyId)
 {
-       if (currentKeyId == keyId) {
-               auto switchId = SwitchId(currentKeyId, getId());
-               interactionManager.stopInteraction(findSwitchById(switchId.getGlobalId()));
-               currentKeyId.clear();
-       }
+       if (keys.find(keyId) == keys.end())
+               return ECORE_CALLBACK_PASS_ON;
+
+       if (currentKeyId != keyId)
+               return ECORE_CALLBACK_DONE;
+
+       DEBUG("key released: %s", keyId.c_str());
+
+       auto switchId = SwitchId(currentKeyId, getId());
+       auto sw = findSwitchById(switchId.getGlobalId());
+       currentKeyId.clear();
+       if (!sw)
+               return ECORE_CALLBACK_DONE;
+
+       interactionManager.stopInteraction(sw);
+
+       return ECORE_CALLBACK_DONE;
 }
 
 Eina_Bool AccessoriesSwitchProvider::keyDownCb(void *data, int type, void *ev)
@@ -732,9 +756,8 @@ Eina_Bool AccessoriesSwitchProvider::keyDownCb(void *data, int type, void *ev)
        std::string keyId(event->key);
 
        auto asp = static_cast<AccessoriesSwitchProvider *>(data);
-       asp->processKeyDownEvent(keyId);
 
-       return ECORE_CALLBACK_DONE;
+       return asp->processKeyDownEvent(keyId);
 }
 
 Eina_Bool AccessoriesSwitchProvider::keyUpCb(void *data, int type, void *ev)
@@ -743,7 +766,6 @@ Eina_Bool AccessoriesSwitchProvider::keyUpCb(void *data, int type, void *ev)
        std::string keyId(event->key);
 
        auto asp = static_cast<AccessoriesSwitchProvider *>(data);
-       asp->processKeyUpEvent(keyId);
 
-       return ECORE_CALLBACK_DONE;
+       return asp->processKeyUpEvent(keyId);
 }

diff --git a/src/AccessoriesSwitchProvider.hpp b/src/AccessoriesSwitchProvider.hpp
index fe0a7669dd8a7073d71050eb74a5dc766665b10c..4650720771f083edca5d0c2e3626c3c236d43efa 100644 (file)
--- a/src/AccessoriesSwitchProvider.hpp
+++ b/src/AccessoriesSwitchProvider.hpp
@@ -6,7 +6,7 @@
 #include <Elementary.h>
 #include <Ecore.h>
 
-#include <vector>
+#include <set>
 #include <string>
 #include <memory>
 
@@ -20,17 +20,18 @@ public:
        std::shared_ptr<Switch> findSwitchById(const std::string &switchId) const override;
 
 private:
-       void processKeyDownEvent(const std::string &keyId);
-       void processKeyUpEvent(const std::string &keyId);
+       Eina_Bool processKeyDownEvent(const std::string &keyId);
+       Eina_Bool processKeyUpEvent(const std::string &keyId);
 
        static Eina_Bool keyDownCb(void *data, int type, void *ev);
        static Eina_Bool keyUpCb(void *data, int type, void *ev);
 
-       const std::vector<std::string> keys;
+       const std::set<std::string> keys;
        SwitchCollection switches;
        Ecore_Event_Handler *keyDownHandler;
        Ecore_Event_Handler *keyUpHandler;
        std::string currentKeyId;
+       std::string prevKeyId;
 };
 
 #endif

diff --git a/src/SQLiteConfiguration.cpp b/src/SQLiteConfiguration.cpp
index 07e1403d4acae41ffccbd919a03c2cc0f3a2a0a0..7daa8c42d034d9bd9120cfb43df040f9733e06be 100644 (file)
--- a/src/SQLiteConfiguration.cpp
+++ b/src/SQLiteConfiguration.cpp
@@ -4,6 +4,13 @@
 
 #include "QueryBuilder.hpp"
 
+enum SwitchConfigurationItemFields {
+       SWITCH_ID,
+       USER_NAME,
+       ACTIVITY_TYPE,
+       Count
+};
+
 SQLiteConfiguration::SQLiteConfiguration(bool createInMemory)
        : password("UniversalSwitchPassword"), dbName(createInMemory ? ":memory:" : "UniversalSwitch.db"), tableName("`UNIVERSAL_SWITCH`"),
          switchIdCol("`SWITCH_ID`"), userNameCol("`USER_NAME`"),
@@ -92,10 +99,10 @@ std::shared_ptr<SwitchConfigurationItem> SQLiteConfiguration::findBySwitchId(con
        auto query = QueryBuilder().select({"*"}).from(tableName).where(switchIdCol, "=", switchId).build();
        INFO("query: %s", query.c_str());
 
-       auto item = std::make_shared<SwitchConfigurationItem>();
+       std::shared_ptr<SwitchConfigurationItem> item;
 
-       if (!executeQuery(query, [ = ](int argc, char **argv, char **azColName) {
-       item->construct(argc, argv, azColName);
+       if (!executeQuery(query, [&item](int argc, char **argv, char **azColName) {
+       item = constructItem(argc, argv, azColName);
        }))
        return nullptr;
 
@@ -110,10 +117,8 @@ std::vector<std::shared_ptr<SwitchConfigurationItem>> SQLiteConfiguration::findA
 
        auto collection = std::vector<std::shared_ptr<SwitchConfigurationItem>>();
 
-       if (!executeQuery(query, [&](int argc, char **argv, char **azColName) {
-       auto item = std::make_shared<SwitchConfigurationItem>();
-               item->construct(argc, argv, azColName);
-               collection.emplace_back(item);
+       if (!executeQuery(query, [&collection](int argc, char **argv, char **azColName) {
+       collection.emplace_back(constructItem(argc, argv, azColName));
        }))
        return {};
 
@@ -169,6 +174,21 @@ void SQLiteConfiguration::createTable()
        executeQuery(query);
 }
 
+std::shared_ptr<SwitchConfigurationItem> SQLiteConfiguration::constructItem(int argc, char **argv, char **azColName)
+{
+       ASSERT(argc == SwitchConfigurationItemFields::Count, "Fields number does not match the one in SwitchConfigurationItem");
+
+       auto item = std::make_shared<SwitchConfigurationItem>();
+
+       item->setSwitchId(argv[SwitchConfigurationItemFields::SWITCH_ID]);
+       item->setUserName(argv[SwitchConfigurationItemFields::USER_NAME]);
+       item->setActivityType(argv[SwitchConfigurationItemFields::ACTIVITY_TYPE]);
+
+       INFO("switchId: %s, userName: %s, activityType: %s", item->getSwitchId().c_str(), item->getUserName().c_str(), item->getActivityType().c_str());
+
+       return item;
+}
+
 void SQLiteConfiguration::clearTable()
 {
        auto query = "DELETE FROM " + tableName ;

diff --git a/src/SQLiteConfiguration.hpp b/src/SQLiteConfiguration.hpp
index 2cb0ce1bebe25da8d7c9de4d6ee42b2c9ef54db4..3929944a9a415bb819c0f7424419245a2d47f092 100644 (file)
--- a/src/SQLiteConfiguration.hpp
+++ b/src/SQLiteConfiguration.hpp
@@ -33,6 +33,7 @@ private:
        bool executeQuery(const std::string &query, const execCallback_t &cb) const;
        bool executeQuery(const std::string &query) const;
        void createTable();
+       static std::shared_ptr<SwitchConfigurationItem> constructItem(int argc, char **argv, char **azColName);
 
        static void update_callback(void *data, int operation, char const *database, char const *table, sqlite_int64 rowid);
 

diff --git a/src/Subject.hpp b/src/Subject.hpp
index 5c068c5953891b2e29f258461d41c783f8a167de..6861138ed969a7efa9efd4f75b272788a9bfe4c1 100644 (file)
--- a/src/Subject.hpp
+++ b/src/Subject.hpp
@@ -31,6 +31,7 @@ public:
 protected:
        void notify(std::shared_ptr<T> item)
        {
+               ASSERT(item, "Notification of NULL item requested");
                for (auto &it : observers) {
                        auto observer = it.first.lock();
                        if (observer && it.second->matching(item.get()))

diff --git a/src/SwitchConfigurationItem.cpp b/src/SwitchConfigurationItem.cpp
index cb7a18cc067eb4b7a97e75e9ef754e6bac7ae129..40a98597a94c7bd401eb4bacf9f88d369c9c90dc 100644 (file)
--- a/src/SwitchConfigurationItem.cpp
+++ b/src/SwitchConfigurationItem.cpp
@@ -2,18 +2,10 @@
 
 #include <dlog.h>
 
-SwitchConfigurationItem::SwitchConfigurationItem(ChangeType type)
-       : changeType(type)
-{}
-
 SwitchConfigurationItem::SwitchConfigurationItem(const std::string &switchId, const std::string &activityType)
        : switchId(switchId), activityType(activityType)
 {}
 
-SwitchConfigurationItem::SwitchConfigurationItem(const std::string &switchId, const std::string &activityType, ChangeType type)
-       : switchId(switchId), activityType(activityType), changeType(type)
-{}
-
 bool SwitchConfigurationItem::operator==(const SwitchConfigurationItem &item) const
 {
        return switchId == item.getSwitchId() && activityType == item.getActivityType() && changeType == item.getChangeType();
@@ -58,19 +50,3 @@ void SwitchConfigurationItem::setChangeType(ChangeType type)
 {
        changeType = type;
 }
-
-void SwitchConfigurationItem::construct(int argc, char **argv, char **azColName)
-{
-       INFO("Function invokation");
-
-       if (argc != static_cast<int>(SwitchConfigurationItemFields::Count)) {
-               ERROR("Fields number does not match the one in SwitchConfigurationItem");
-               return;
-       }
-
-       setSwitchId(argv[static_cast<int>(SwitchConfigurationItemFields::SWITCH_ID)]);
-       setUserName(argv[static_cast<int>(SwitchConfigurationItemFields::USER_NAME)]);
-       setActivityType(argv[static_cast<int>(SwitchConfigurationItemFields::ACTIVITY_TYPE)]);
-
-       INFO("switchId: %s, userName: %s, activityType: %s", switchId.c_str(), userName.c_str(), activityType.c_str());
-}

diff --git a/src/SwitchConfigurationItem.hpp b/src/SwitchConfigurationItem.hpp
index 7eb97bf2b10e6ae57bb0fb35474aaa3d9167dbc7..97e56ed040eba52aff96e232782b50b61aa91791 100644 (file)
--- a/src/SwitchConfigurationItem.hpp
+++ b/src/SwitchConfigurationItem.hpp
@@ -12,20 +12,11 @@ enum ChangeType {
        NONE
 };
 
-enum class SwitchConfigurationItemFields {
-       SWITCH_ID,
-       USER_NAME,
-       ACTIVITY_TYPE,
-       Count
-};
-
 class SwitchConfigurationItem
 {
 public:
        SwitchConfigurationItem() = default;
-       SwitchConfigurationItem(ChangeType type);
        SwitchConfigurationItem(const std::string &switchId, const std::string &activityType);
-       SwitchConfigurationItem(const std::string &switchId,  const std::string &activityType, ChangeType type);
 
        bool operator==(const SwitchConfigurationItem &item) const;
 
@@ -38,8 +29,6 @@ public:
        ChangeType getChangeType() const;
        void setChangeType(ChangeType type);
 
-       void construct(int argc, char **argv, char **azColName);
-
 private:
        std::string switchId;
        std::string userName;

diff --git a/src/SwitchInteractionManager.cpp b/src/SwitchInteractionManager.cpp
index 3db8bf102a9a273fec9f5d85edcbbde103480a02..0f03efe6085eb3cbc525c28a7c6572e75d9fbbee 100644 (file)
--- a/src/SwitchInteractionManager.cpp
+++ b/src/SwitchInteractionManager.cpp
@@ -18,28 +18,31 @@ void SwitchInteractionManager::startInteraction(const NotifyMethod &method, cons
        deleteAndClearTimer(minimalSwitchInteractionTimer);
        deleteAndClearTimer(notificationRepetitionIntervalTimer);
 
-       notifyMethod = method;
-       currentSwitch = sw;
-       if (!currentSwitch) {
+       if (!sw) {
                ERROR("Switch has not been created by provider");
                return;
        }
 
+       currentSwitch = sw;
+       notifyMethod = method;
+
        DEBUG("switch: %s", sw->getId()->getGlobalId().c_str());
        minimalSwitchInteractionTimer = ecore_timer_add(minimalSwitchInteractionTime, minimalSwitchInteractionTimeCb, this);
 }
 
 void SwitchInteractionManager::stopInteraction(const std::shared_ptr<Switch> &sw)
 {
-       if (currentSwitch->getId()->getGlobalId() != sw->getId()->getGlobalId())
+       bool isActiveInteraction = minimalSwitchInteractionTimer || notificationRepetitionIntervalTimer;
+       if (!isActiveInteraction)
                return;
 
-       bool isActiveInteraction = minimalSwitchInteractionTimer || notificationRepetitionIntervalTimer;
+       if (!currentSwitch || !sw || currentSwitch->getId()->getGlobalId() != sw->getId()->getGlobalId())
+               return;
 
        deleteAndClearTimer(minimalSwitchInteractionTimer);
        deleteAndClearTimer(notificationRepetitionIntervalTimer);
 
-       if (isActiveInteraction && !switchInteractionIntervalTimer) {
+       if (!switchInteractionIntervalTimer) {
                DEBUG("interval timer set");
                switchInteractionIntervalTimer = ecore_timer_add(switchInteractionInterval, switchInteractionIntervalCb, this);
        }
@@ -61,6 +64,8 @@ double SwitchInteractionManager::getNotificationRepetitionInterval() const
 }
 
 SwitchInteractionManager::SwitchInteractionManager()
+       : minimalSwitchInteractionTime(0.0), switchInteractionInterval(0.0), notificationRepetitionInterval(0.0),
+         minimalSwitchInteractionTimer(nullptr), switchInteractionIntervalTimer(nullptr), notificationRepetitionIntervalTimer(nullptr)
 {
        minimalSwitchInteractionTimeHandler = VConfSingleton::instance().registerAndGet<double>(VCONF_PREFIX "TAP_DURATION_VALUE", 1.0, [this](int x) {
                minimalSwitchInteractionTime = x;
@@ -93,35 +98,49 @@ void SwitchInteractionManager::deleteAndClearTimer(Ecore_Timer *&timer)
 
 Eina_Bool SwitchInteractionManager::minimalSwitchInteractionTimeCb(void *data)
 {
-       DEBUG("invoked");
-
        auto im = static_cast<SwitchInteractionManager *>(data);
-
-       im->minimalSwitchInteractionTimer = nullptr;
-       im->notifyMethod();
-
-       if (im->notificationRepetitionInterval != 0.0)
-               im->notificationRepetitionIntervalTimer = ecore_timer_add(im->notificationRepetitionInterval, notificationRepetitionIntervalCb, im);
-       else
-               ecore_timer_add(im->switchInteractionInterval, switchInteractionIntervalCb, im);
+       im->afterMinimalInteractionTime();
 
        return ECORE_CALLBACK_CANCEL;
 }
 
 Eina_Bool SwitchInteractionManager::switchInteractionIntervalCb(void *data)
 {
-       DEBUG("invoked");
        auto im = static_cast<SwitchInteractionManager *>(data);
-       im->switchInteractionIntervalTimer = nullptr;
+       im->afterInteractionInterval();
 
        return ECORE_CALLBACK_CANCEL;
 }
 
 Eina_Bool SwitchInteractionManager::notificationRepetitionIntervalCb(void *data)
 {
-       DEBUG("invoked");
        auto im = static_cast<SwitchInteractionManager *>(data);
-       im->notifyMethod();
+       im->afterRepetitionInterval();
 
        return ECORE_CALLBACK_RENEW;
 }
+
+void SwitchInteractionManager::afterMinimalInteractionTime()
+{
+       DEBUG("invoked: %s", currentSwitch->getId()->getGlobalId().c_str());
+
+       minimalSwitchInteractionTimer = nullptr;
+       notifyMethod();
+
+       if (notificationRepetitionInterval != 0.0)
+               notificationRepetitionIntervalTimer = ecore_timer_add(notificationRepetitionInterval, notificationRepetitionIntervalCb, this);
+       else
+               ecore_timer_add(switchInteractionInterval, switchInteractionIntervalCb, this);
+}
+
+void SwitchInteractionManager::afterInteractionInterval()
+{
+       DEBUG("invoked");
+       switchInteractionIntervalTimer = nullptr;
+}
+
+void SwitchInteractionManager::afterRepetitionInterval()
+{
+       DEBUG("invoked: %s", currentSwitch->getId()->getGlobalId().c_str());
+       notifyMethod();
+}

diff --git a/src/SwitchInteractionManager.hpp b/src/SwitchInteractionManager.hpp
index 43a41d1d8f6ada318e4b229c29806c9d5d175da8..dde5749900feec4c908d5b11d3902f12f4e00572 100644 (file)
--- a/src/SwitchInteractionManager.hpp
+++ b/src/SwitchInteractionManager.hpp
@@ -34,6 +34,10 @@ private:
        static Eina_Bool switchInteractionIntervalCb(void *data);
        static Eina_Bool notificationRepetitionIntervalCb(void *data);
 
+       void afterMinimalInteractionTime();
+       void afterInteractionInterval();
+       void afterRepetitionInterval();
+
        std::shared_ptr<Switch> currentSwitch;
        double minimalSwitchInteractionTime;
        double switchInteractionInterval;