Merge branch 'fix/pgroen/error-handling-improvement' into 'master'

Fix/pgroen/error handling improvement Added logging and sensible return points if a problem occurs. After merge : * it will be tagged as version 0.9.4 * it will be integrated into the Main IPZC SDK ( Aarch64 ) @ Priva See merge request !7

Merge branch 'fix/pgroen/error-handling-improvement' into 'master'
Fix/pgroen/error handling improvement Added logging and sensible return points if a problem occurs. After merge : * it will be tagged as version 0.9.4 * it will be integrated into the Main IPZC SDK ( Aarch64 ) @ Priva See merge request !7
Peter M. Groen
2 parents d4d38789 1cfa5165
Showing 7 changed files with 76 additions and 92 deletions
CMakeLists.txt
examples/pub/CMakeLists.txt
examples/sub/CMakeLists.txt
scripts/setup_submodules
scripts/submodules.list
src/CMakeLists.txt
src/mqttclient.cpp
 cmake_minimum_required(VERSION 3.0)
 project(osdev_mqtt)
-# ==============================================================================
-# Check to see if we're a submodule or top-repo.
-if(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/cmake)
-    message( STATUS "Looks like we're a single module" )
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake)
-elseif(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-    message( STATUS "Looks like we're a submodule" )
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-else()
-    message( FATAL_ERROR "No cmake directory found. Did you run the submodules script?" )
-endif()
-
-# ==============================================================================
-# Check to see if there is versioning information available
-if(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/versioning)
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/versioning/cmake)
-elseif(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../versioning)
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../versioning/cmake)
-else()
-    message( FATAL_ERROR "No ${CURRENT_SOURCE_DIR}/osdev_versioning directory found. Did you run the submodules script?" )
-endif()
+LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/submodules/cmake)
 # ==============================================================================
 # = Include build information
-include(osdevversion)
 include(projectheader)
-
 project_header(osdev_mqtt)
+add_subdirectory(submodules/logger)
 add_subdirectory(src)
 add_subdirectory(examples/pub)
 add_subdirectory(examples/sub)
 cmake_minimum_required(VERSION 3.0)
-LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../cmake)
+LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/submodules/cmake)
 include(projectheader)
 project_header(test_mqtt_pub)
@@ -27,7 +27,7 @@ target_link_libraries(
 set_target_properties( ${PROJECT_NAME} PROPERTIES
     RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin
     LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib
-    ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/archive
+    ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib
 )
 include(installation)
 cmake_minimum_required(VERSION 3.0)
-LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../cmake)
+LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/submodules/cmake)
 include(projectheader)
 project_header(test_mqtt_sub)
@@ -3,7 +3,9 @@
 # ===============================================
 # == Setting some environment variables
 # ===============================================
-GIT_URL_SUBS="http://gitlab.osdev.nl/open_source"
+GIT_URL_OPEN="http://gitlab.osdev.nl/open_source"
+GIT_URL_CLOSED="git@gitlab.osdev.nl:closed_source"
+
 FUNC_RESULT="-1"
 # Name          : print_usage_exit()
@@ -98,12 +100,19 @@ function read_submodules()
 function add_submodules() 
 {
     echo -e "Adding SubModule(s)."
-    for SUB_MODULE in ${SUB_MODULES}
+    for SUB_MODULE in ${SUB_MODULES_OPEN}
     do
-        echo -e "< ${SUB_MODULE} >"
-        git submodule add -f ${GIT_URL_SUBS}/${SUB_MODULE}.git ${SUB_MODULE}
-        git config submodule.${SUB_MODULE}.url ${GIT_URL_SUBS}/${SUB_MODULE}.git
+        git submodule add -f ${GIT_URL_OPEN}/${SUB_MODULE}.git submodules/${SUB_MODULE}
+        git config submodule.${SUB_MODULE}.url ${GIT_URL_OPEN}/${SUB_MODULE}.git
+    done
+    
+    for SUB_MODULE in ${SUB_MODULES_CLOSED}
+    do    
+        echo {GIT_URL_CLOSED}/${SUB_MODULE}.git
+        git submodule add -f ${GIT_URL_CLOSED}/${SUB_MODULE}.git submodules/${SUB_MODULE}
+        git config submodule.${SUB_MODULE}.url ${GIT_URL_CLOSED}/${SUB_MODULE}.git
     done
+    
 }
 # Name          : get_submodules
-SUB_MODULES="versioning
-cmake"
+SUB_MODULES_OPEN="cmake
+logger"
+
+SUB_MODULES_CLOSED=""
 cmake_minimum_required(VERSION 3.12)
-# ==============================================================================
-# Check to see if we're a submodule or top-repo.
-if(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-    message( STATUS "Looks like we're a single module" )
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-elseif(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../../cmake)
-    message( STATUS "Looks like we're a submodule" )
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../cmake)
-else()
-    message( FATAL_ERROR "No cmake directory found. Did you run the submodules script?" )
-endif()
+LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/submodules/cmake)
-# ==============================================================================
-# Check to see if there is versioning information available
-if(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../versioning)
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../versioning/cmake)
-elseif(IS_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/../../versioning)
-    LIST(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../versioning/cmake)
-else()
-    message( FATAL_ERROR "No ${CURRENT_SOURCE_DIR}/osdev_versioning directory found. Did you run the submodules script?" )
-endif()
-# ==============================================================================
 include(projectheader)
-
 project_header(mqtt-cpp)
 find_package( Boost REQUIRED COMPONENTS regex )
@@ -31,6 +10,7 @@ include(compiler)
 include_directories(
     ${CMAKE_SOURCE_DIR}/include
+    ${CMAKE_SOURCE_DIR}/submodules/logger/src
 )
 set(SRC_LIST
@@ -67,6 +47,7 @@ add_libraries(
         Boost::boost
         Boost::regex
         paho-mqtt3a
+        ${CMAKE_SOURCE_DIR}/build/lib/liblogger.a
 )
 include(installation)
@@ -21,21 +21,24 @@
  * ***************************************************************************/
 #include "mqttclient.h"
+// osdev::components::logger
+#include "log.h"
+ 
 // osdev::components::mqtt
 #include "clientpaho.h"
 #include "mqttutil.h"
 #include "mqttidgenerator.h"
 #include "mqtttypeconverter.h"
-
-// mlogic::common
 #include "lockguard.h"
 #include "uriparser.h"
 // std
 #include <numeric>
 #include <iostream>
+#include <string>
 using namespace osdev::components::mqtt;
+using namespace osdev::components::log;
 namespace {
 /**
@@ -63,28 +66,30 @@ MqttClient::MqttClient(const std::string&amp; _clientId, const std::function&lt;void(co
     , m_principalClient()
     , m_additionalClients()
     , m_eventQueue(_clientId)
-    , m_workerThread(std::thread(&MqttClient::eventHandler, this))
+    , m_workerThread( std::thread( &MqttClient::eventHandler, this ) )
 {
+    Log::init( "mqtt-library" );
+    LogInfo( "MQTT Client started", "[MqttClient::MqttClient]");
 }
 MqttClient::~MqttClient()
 {
-    // DebugLogToFIle ("MqttClient", "%1 - dtor", m_clientId);
     {
-        // DebugLogToFIle ("MqttClient", "%1 - disconnect", m_clientId);
+        // LogDebug( "MqttClient", std::string( m_clientId + " - disconnect" ) );
         this->disconnect();
         decltype(m_principalClient) principalClient{};
         OSDEV_COMPONENTS_LOCKGUARD(m_internalMutex);
-        // DebugLogToFIle ("MqttClient", "%1 - cleanup principal client", m_clientId);
+        LogDebug( "MqttClient", std::string( m_clientId + " - cleanup principal client" ) );
         m_principalClient.swap(principalClient);
     }
-    // DebugLogToFIle ("MqttClient", "%1 - dtor stop queue", m_clientId);
+
+    LogDebug( "MqttClient", std::string( m_clientId + " - dtor stop queue" ) );
     m_eventQueue.stop();
     if (m_workerThread.joinable()) {
         m_workerThread.join();
     }
-    // DebugLogToFIle ("MqttClient", "%1 - dtor ready", m_clientId);
+    LogDebug( "MqttClient", std::string( m_clientId + " - dtor ready" ) );
 }
 std::string MqttClient::clientId() const
@@ -135,7 +140,8 @@ void MqttClient::connect(const std::string&amp; host, int port, const Credentials&amp; c
 void MqttClient::connect(const std::string& _endpoint)
 {
-    // InfoLogToFIle ("MqttClient", "%1 - Request connect", m_clientId);
+    LogInfo( "MqttClient", std::string( m_clientId + " - Request connect" ) );
+
     OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex);
     IMqttClientImpl* client(nullptr);
     {
@@ -148,8 +154,8 @@ void MqttClient::connect(const std::string&amp; _endpoint)
             }
             else
             {
-                // ErrorLogToFIle ("MqttClient", "%1 - Cannot connect to different endpoint. Disconnect first.", m_clientId);
-                // Normally a throw (Yuck!) (MqttException, "Cannot connect while already connected");
+                LogError( "MqttClient", std::string( m_clientId + " - Cannot connect to different endpoint. Disconnect first." ) );
+                return;
             }
         }
         m_endpoint = _endpoint;
@@ -168,26 +174,31 @@ void MqttClient::connect(const std::string&amp; _endpoint)
 void MqttClient::disconnect()
 {
-    // InfoLogToFIle ("MqttClient", "%1 - Request disconnect", m_clientId);
+    LogInfo( "MqttClient", std::string( m_clientId + " - Request disconnect" ) );
     OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex);
     decltype(m_additionalClients) additionalClients{};
     std::vector<IMqttClientImpl*> clients{};
     {
         OSDEV_COMPONENTS_LOCKGUARD(m_internalMutex);
-        if (!m_principalClient || m_principalClient->connectionStatus() == ConnectionStatus::Disconnected || m_principalClient->connectionStatus() == ConnectionStatus::DisconnectInProgress) {
+        if (!m_principalClient || m_principalClient->connectionStatus() == ConnectionStatus::Disconnected || m_principalClient->connectionStatus() == ConnectionStatus::DisconnectInProgress)
+        {
+            LogDebug( "MqttClient", std::string( m_clientId + " - Principal client not connected" ) );
             return;
         }
-        m_additionalClients.swap(additionalClients);
+        m_additionalClients.swap( additionalClients );
-        for (const auto& c : additionalClients) {
-            clients.push_back(c.get());
+        for (const auto& c : additionalClients)
+        {
+            clients.push_back( c.get() );
         }
-        clients.push_back(m_principalClient.get());
+        clients.push_back( m_principalClient.get() );
     }
-    // DebugLogToFIle ("MqttClient", "%1 - Unsubscribe and disconnect clients", m_clientId);
-    for (auto& cl : clients) {
+
+    LogDebug( "MqttClient", std::string( m_clientId +  " - Unsubscribe and disconnect clients" ) );
+    for ( auto& cl : clients )
+    {
         cl->unsubscribeAll();
     }
     this->waitForCompletionInternal(clients, std::chrono::milliseconds(2000), std::set<Token>{});
@@ -211,15 +222,16 @@ Token MqttClient::publish(const MqttMessage&amp; message, int qos)
         {
             if( !m_principalClient )
             {
-                std::cout << "Principal client not initialized" << std::endl;
+                LogInfo( "[MqttClient::publish]", std::string( "Principal client not initialized") );
             }
             if( m_principalClient->connectionStatus() == ConnectionStatus::Disconnected )
             {
                 std::cout << "Unable to publish, not connected.." << std::endl;
             }
-            // ErrorLogToFIle ("MqttClient", "%1 - Unable to publish, not connected", m_clientId);
-            // Throw (MqttException, "Not connected");
+            LogError("MqttClient", std::string( m_clientId + " - Unable to publish, not connected" ) );
+
+            return Token(m_clientId, -1);
         }
         client = m_principalClient.get();
     }
@@ -228,7 +240,7 @@ Token MqttClient::publish(const MqttMessage&amp; message, int qos)
 Token MqttClient::subscribe(const std::string& topic, int qos, const std::function<void(MqttMessage)>& cb)
 {
-    // DebugLogToFIle ("MqttClient", "%1 - Subscribe to topic %2 with qos %3", m_clientId, topic, qos);
+    LogDebug( "[MqttClient::subscribe]", std::string( m_clientId + " - Subscribe to topic " + topic ) );
     // OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex);
     bool clientFound = false;
     IMqttClientImpl* client(nullptr);
@@ -236,8 +248,9 @@ Token MqttClient::subscribe(const std::string&amp; topic, int qos, const std::functi
         // OSDEV_COMPONENTS_LOCKGUARD(m_internalMutex);
         if (!m_principalClient || m_principalClient->connectionStatus() == ConnectionStatus::Disconnected)
         {
-            // ErrorLogToFIle ("MqttClient", "%1 - Unable to subscribe, not connected", m_clientId);
+            LogError("MqttClient", std::string( m_clientId + " - Unable to subscribe, not connected" ) );
             // throw (?)(MqttException, "Not connected");
+            return Token(m_clientId, -1);
         }
         if (!m_principalClient->isOverlapping(topic)) {
             client = m_principalClient.get();
@@ -253,7 +266,7 @@ Token MqttClient::subscribe(const std::string&amp; topic, int qos, const std::functi
             }
         }
         if (!clientFound) {
-            // DebugLogToFIle ("MqttClient", "%1 - Creating new ClientPaho instance for subscription on topic %2", m_clientId, topic);
+            LogDebug("[MqttClient::subscribe]", std::string( m_clientId + " - Creating new ClientPaho instance for subscription on topic " + topic ) );
             std::string derivedClientId(generateUniqueClientId(m_clientId, m_additionalClients.size() + 2)); // principal client is nr 1.
             m_additionalClients.emplace_back(std::make_unique<ClientPaho>(
                 m_endpoint,
@@ -271,14 +284,15 @@ Token MqttClient::subscribe(const std::string&amp; topic, int qos, const std::functi
 std::set<Token> MqttClient::unsubscribe(const std::string& topic, int qos)
 {
-    // DebugLogToFIle ("MqttClient", "%1 - Unsubscribe from topic %2 with qos %3", m_clientId, topic, qos);
+    LogDebug("[MqttClient::unsubscribe]", std::string( m_clientId + " - Unsubscribe from topic " + topic ) );
     OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex);
     std::vector<IMqttClientImpl*> clients{};
     {
         OSDEV_COMPONENTS_LOCKGUARD(m_internalMutex);
         if (!m_principalClient || m_principalClient->connectionStatus() == ConnectionStatus::Disconnected) {
-            // ErrorLogToFIle ("MqttClient", "%1 - Unable to unsubscribe, not connected", m_clientId);
+            LogError("[MqttClient::unsubscribe]", std::string( m_clientId + " - Unable to unsubscribe, not connected" ) );
             // Throw (MqttException, "Not connected");
+            return std::set<Token>();
         }
         clients.push_back(m_principalClient.get());
         for (const auto& c : m_additionalClients) {
@@ -359,9 +373,7 @@ std::string MqttClient::endpoint() const
 void MqttClient::connectionStatusChanged(const std::string& id, ConnectionStatus cs)
 {
-    (void)id;
-    (void)cs;
-    // DebugLogToFIle ("MqttClient", "%1 - connection status of wrapped client %2 changed to %3", m_clientId, id, cs);
+    LogDebug("[MqttClient::connectionStatusChanged]", std::string( m_clientId + " - connection status of wrapped client " + id + " changed to " + std::to_string( static_cast<int>(cs) ) ) );
     IMqttClientImpl* principalClient{ nullptr };
     std::vector<IMqttClientImpl*> clients{};
     std::vector<ConnectionStatus> connectionStates{};
@@ -392,7 +404,7 @@ void MqttClient::connectionStatusChanged(const std::string&amp; id, ConnectionStatus
                 cl->resubscribe();
             }
             catch (const std::exception& e) {
-                // ErrorLogToFIle ("MqttClient", "%1 - resubscribe on wrapped client %2 in context of connection status change in wrapped client %3 failed : %4", m_clientId, cl->clientId(), id, e.what());
+                LogError("[MqttClient::connectionStatusChanged]", std::string( m_clientId + " - resubscribe on wrapped client " + cl->clientId() + " in context of connection status change in wrapped client : " + id + " => FAILED : " + e.what() ) );
             }
         }
         m_activeTokensCV.notify_all();
@@ -407,7 +419,7 @@ void MqttClient::connectionStatusChanged(const std::string&amp; id, ConnectionStatus
             auto waitFor = std::chrono::milliseconds(1000);
             if (!waitForCompletionInternalClients(clients, waitFor, std::set<Token>{})) {
                 if (std::accumulate(clients.begin(), clients.end(), false, [](bool hasPending, IMqttClientImpl* client) { return hasPending || client->hasPendingSubscriptions(); })) {
-                    // WarnLogToFIle ("MqttClient", "%1 - subscriptions are not recovered within timeout.", m_clientId)
+                    LogWarning("[MqttClient::connectionStatusChanged]", std::string( m_clientId + " - subscriptions are not recovered within timeout." ) );
                 }
             }
             if (principalClient) {
@@ -415,7 +427,7 @@ void MqttClient::connectionStatusChanged(const std::string&amp; id, ConnectionStatus
                     principalClient->publishPending();
                 }
                 catch (const std::exception& e) {
-                    // ErrorLogToFIle ("MqttClient", "%1 - publishPending on wrapped client %2 failed : %3", m_clientId, principalClient->clientId(), e.what());
+                    LogError( "[MqttClient::connectionStatusChanged]", std::string( m_clientId + " - publishPending on wrapped client " + principalClient->clientId() + " => FAILED " + e.what() ) );
                 }
             }
         }
@@ -434,7 +446,7 @@ void MqttClient::deliveryComplete(const std::string&amp; _clientId, std::int32_t tok
         OSDEV_COMPONENTS_LOCKGUARD(m_internalMutex);
         if (!m_activeTokens.insert(t).second) {
             // This should not happen. This means that some callback on the wrapper never came.
-            // ErrorLogToFIle ("MqttClient", "%1 -deliveryComplete, token %1 is already active", m_clientId, t);
+            LogDebug("[MqttClient::deliveryComplete]", std::string( m_clientId + " - deliveryComplete, token is already active" ) );
         }
     }
     this->pushEvent([this, t]() {
@@ -531,8 +543,9 @@ void MqttClient::pushEvent(std::function&lt;void()&gt; ev)
 void MqttClient::eventHandler()
 {
-    // InfoLogToFIle ("MqttClient", "%1 - starting event handler", m_clientId);
-    for (;;) {
+    LogInfo("[MqttClient::eventHandler]", std::string( m_clientId + " - starting event handler." ) );
+    for (;;)
+    {
         std::vector<std::function<void()>> events;
         if (!m_eventQueue.pop(events))
         {
@@ -543,5 +556,5 @@ void MqttClient::eventHandler()
             ev();
         }
     }
-    // InfoLogToFIle ("MqttClient", "%1 - leaving event handler", m_clientId);
+    LogInfo("[MqttClient::eventHandler]", std::string( m_clientId + " - leaving event handler." ) );
 }