From e9ff0d8520e95221d66865a7e810d2253df75198 Mon Sep 17 00:00:00 2001 From: Lomtev Dmytro Date: Wed, 11 Oct 2017 13:40:37 +0300 Subject: [PATCH] Fixed nmdaemon restart sequence. Problem: nmdaemon hang up when exception during signIn occured. Solve: signIn local unique_lock forced to unlock before exception is thrown. HubClient refactored. Problem: it is difficult to test functions which use network connection to remote server. Solve: IotResourceClient added as base for HubClient. --- device_core/agent_lib/src/agentpolicyadapter.cpp | 4 +- device_core/ctrl_app_lib/inc/hub_client.h | 34 ++--------- device_core/ctrl_app_lib/src/hub_client.cpp | 55 +++-------------- .../ctrl_app_lib/src/iotchilddevice_impl.cpp | 2 +- device_core/iotivity_lib/inc/iot_resource.h | 44 ++++++++++++++ device_core/iotivity_lib/src/iot_resource.cpp | 69 +++++++++++++++++++--- device_core/iotivity_lib/src/iotivity_impl.cpp | 10 +++- device_core/nmdaemon/main.cpp | 12 ++-- device_core/utest/test_hubclient.cpp | 2 +- 9 files changed, 134 insertions(+), 98 deletions(-) diff --git a/device_core/agent_lib/src/agentpolicyadapter.cpp b/device_core/agent_lib/src/agentpolicyadapter.cpp index 473e3bf..25e41db 100644 --- a/device_core/agent_lib/src/agentpolicyadapter.cpp +++ b/device_core/agent_lib/src/agentpolicyadapter.cpp @@ -38,7 +38,5 @@ AgentPolicyAdapter::~AgentPolicyAdapter() void AgentPolicyAdapter::RmiClientDeleter::operator()(rmi::Client* p) { - if (p != nullptr) { - delete p; - } + delete p; } diff --git a/device_core/ctrl_app_lib/inc/hub_client.h b/device_core/ctrl_app_lib/inc/hub_client.h index 142a1b2..a6a86a6 100644 --- a/device_core/ctrl_app_lib/inc/hub_client.h +++ b/device_core/ctrl_app_lib/inc/hub_client.h @@ -11,19 +11,16 @@ #define HUB_CLIENT_H #include -#include -#include -#include -#include +#include "iotdevice.h" +#include "iot_resource.h" namespace NetworkManager { /** * @brief The HubClient class provide connection to hub resource */ -class HubClient: public std::enable_shared_from_this +class HubClient: public std::enable_shared_from_this, public IotResourceClient { - std::shared_ptr resource; public: static const std::string RESOURCE_TYPE; @@ -33,19 +30,13 @@ public: * @brief Constructor * @param hub_resource [in] resource tied wiith hub device */ - HubClient(std::shared_ptr hub_resource) : resource(hub_resource) { + HubClient(std::shared_ptr hub_resource) : IotResourceClient(hub_resource) { } /** * @brief Copy constructor */ - HubClient(const HubClient& host) : resource(host.resource) { - } - - /** - * @brief true if resource found and false otherwise - */ - operator bool() const; + HubClient(const HubClient& host) = default; /** * @brief getUnownedDevices get list of unowned primitive devices @@ -78,21 +69,6 @@ public: ownPrimitiveDevice(uid, false); } - /** - * @brief getUUID return id of the device hosted resource - * @return device id - */ - std::string getUUID() const { - return resource->sid(); - } - - /** - * @brief host return host url - * @return - */ - std::string host() const { - return resource->host(); - } private: /** * @brief getPrimitiveDevices get list of primitive devices diff --git a/device_core/ctrl_app_lib/src/hub_client.cpp b/device_core/ctrl_app_lib/src/hub_client.cpp index 4811e1a..f24949a 100644 --- a/device_core/ctrl_app_lib/src/hub_client.cpp +++ b/device_core/ctrl_app_lib/src/hub_client.cpp @@ -28,37 +28,10 @@ namespace NetworkManager const std::string HubClient::RESOURCE_TYPE{"device.hub"}; -HubClient::operator bool() const -{ - return (bool)resource; -} - void HubClient::getPrimitiveDevices(const OC::QueryParamsMap& query, IoTDevicesMap& device_map) { FN_VISIT - std::string devices; - - if (resource) { - LOG_D(TAG, "Resource ready"); - GetResourceCallback::Sptr callback = std::make_shared(); - - auto result = resource->get(query, bind_callback(callback, PH::_1, PH::_2, PH::_3)); - - if (OC_STACK_OK != result) { - throw IoTInternalError("HubClient::getPrimitiveDevices error", result); - } - - if (!callback->wait()) { - throw IoTInternalError("HubClient::getPrimitiveDevices callback not called", EC_UNAUTHORIZED); - } - - if (callback->errorCode > OC_STACK_RESOURCE_CHANGED) { - throw IoTInternalError("HubClient::getPrimitiveDevices error", callback->errorCode); - } - - callback->representation.getValue("devices", devices); - } - + std::string devices = get(query).getValue("devices"); Json::Reader reader; Json::Value root; reader.parse(devices, root); @@ -72,6 +45,7 @@ void HubClient::getPrimitiveDevices(const OC::QueryParamsMap& query, IoTDevicesM std::string type = dev["type"].asString(); bool state = dev["state"].asBool(); auto it = device_map.find(did); + if (it == device_map.end()) { device_map.emplace(did, std::make_shared(this_shared, did, name, model, type, state)); } else { @@ -82,26 +56,13 @@ void HubClient::getPrimitiveDevices(const OC::QueryParamsMap& query, IoTDevicesM void HubClient::ownPrimitiveDevice(const std::string& uid, bool state) { - if (resource) { - QueryParamsMap query; - query["uid"] = uid; - query["state"] = state ? "own" : "unown"; + QueryParamsMap query; + query["uid"] = uid; + query["state"] = state ? "own" : "unown"; + auto result = post(OCRepresentation{}, query); - PostResourceCallback::Sptr callback = std::make_shared(); - - auto result = resource->post(RESOURCE_TYPE, - DEFAULT_INTERFACE, - OCRepresentation{}, - query, - bind_callback(callback, PH::_1, PH::_2, PH::_3)); - - if (OC_STACK_OK != result) { - throw IoTInternalError("HubClient::ownPrimitiveDevice error", result); - } - - if (!callback->wait()) { - throw IoTInternalError("HubClient::ownPrimitiveDevice callback not called", EC_UNAUTHORIZED); - } + if (OC_STACK_OK != result) { + throw IoTInternalError("HubClient::ownPrimitiveDevice error", result); } } diff --git a/device_core/ctrl_app_lib/src/iotchilddevice_impl.cpp b/device_core/ctrl_app_lib/src/iotchilddevice_impl.cpp index 5c967d3..1985023 100644 --- a/device_core/ctrl_app_lib/src/iotchilddevice_impl.cpp +++ b/device_core/ctrl_app_lib/src/iotchilddevice_impl.cpp @@ -29,7 +29,7 @@ IoTChildDevice_impl::IoTChildDevice_impl(std::shared_ptr hub_resource dev(hub_resource), uuid(uid), name(name), model(model), type(type), online(connected) { host = dev->host(); - hub_uuid = dev->getUUID(); + hub_uuid = dev->sid(); } IoTChildDevice_impl::~IoTChildDevice_impl() diff --git a/device_core/iotivity_lib/inc/iot_resource.h b/device_core/iotivity_lib/inc/iot_resource.h index 12e21bb..06f3f6b 100644 --- a/device_core/iotivity_lib/inc/iot_resource.h +++ b/device_core/iotivity_lib/inc/iot_resource.h @@ -18,9 +18,22 @@ public: * @brief Constructor * @param resource [in] pointer to OCResource instance */ + IotResourceClient(OC::OCResource::Ptr resource); + + /** + * @brief Constructor + * @param iotivity [in] poiter to iotivity instance + * @param resource_type [in] type of the resource + * @param sid [in] optional resource server identifier + */ IotResourceClient(IoTivity* iotivity, const std::string& resource_type, const std::string& sid = ""); /** + * @brief Copy constructor + */ + IotResourceClient(const IotResourceClient&) = default; + + /** * @brief Destructor */ virtual ~IotResourceClient() @@ -29,6 +42,13 @@ public: /** * @brief Assignment operator + * @param IotResourceClient [in] another IotResourceClient object + * @return + */ + IotResourceClient& operator=(IotResourceClient& resource) = default; + + /** + * @brief Assignment operator from resource * @param resource [in] pointer to OCResource instance * @return */ @@ -46,6 +66,7 @@ public: /** * @brief Try to find resource * @param on_cloud [in] if true search in IoT cloud account + * @param retry [in] if true undertakes another try to find the resource after resignIn * @return true if resource found */ bool findResource(bool on_cloud = true, bool retry = true); @@ -56,6 +77,13 @@ public: operator bool() const; /** + * @brief get - perform GET request to the resource + * @param query_params [in] additional query parameters + * @return representation or throws on error + */ + virtual OC::OCRepresentation get(const OC::QueryParamsMap& query_params); + + /** * @brief post - perform POST request to the resource * @param representation [in] representation which have to be posted * @param query_params [in] additional query parameters @@ -63,6 +91,22 @@ public: */ virtual OCStackResult post(const OC::OCRepresentation& representation, const OC::QueryParamsMap& query_params); + /** + * @brief Return id the resource server + * @return server id id + */ + std::string sid() const { + return m_resource->sid(); + } + + /** + * @brief Return resource server host url + * @return + */ + std::string host() const { + return m_resource->host(); + } + protected: OC::OCResource::Ptr m_resource; IoTivity* m_iotivity; diff --git a/device_core/iotivity_lib/src/iot_resource.cpp b/device_core/iotivity_lib/src/iot_resource.cpp index d61a228..7105fe0 100644 --- a/device_core/iotivity_lib/src/iot_resource.cpp +++ b/device_core/iotivity_lib/src/iot_resource.cpp @@ -6,6 +6,7 @@ #include "resource_callbacks.h" #include "logging.h" #include "network_manager_tag.h" +#include "nmlib.h" using namespace OC; namespace PH = std::placeholders; @@ -20,6 +21,22 @@ const std::chrono::seconds DEFAULT_TIMEOUT{5}; namespace NetworkManager { +IotResourceClient::IotResourceClient(OC::OCResource::Ptr resource) + : m_resource(resource) + , m_type{} + , m_sid{} +{ + if (m_resource) { + auto types = m_resource->getResourceTypes(); + + if (!types.empty()) { + m_type = types[0]; + } + + m_sid = resource->sid(); + } +} + IotResourceClient::IotResourceClient(IoTivity* iotivity, const std::string& resource_type, const std::string& sid) : m_resource(nullptr), m_iotivity(iotivity), m_type(resource_type), m_sid(sid) { @@ -28,6 +45,17 @@ IotResourceClient::IotResourceClient(IoTivity* iotivity, const std::string& reso IotResourceClient& IotResourceClient::operator=(OC::OCResource::Ptr resource) { m_resource = resource; + + if (m_resource) { + auto types = m_resource->getResourceTypes(); + + if (!types.empty()) { + m_type = types[0]; + } + + m_sid = resource->sid(); + } + return *this; } @@ -35,8 +63,8 @@ bool IotResourceClient::findResource(bool on_cloud, bool retry) { LOG_D(TAG, "IotResourceClient: search for resource of type '%s'", m_type.c_str()); - if (nullptr == m_iotivity) { - LOG_E(TAG, "IotResourceClient: iotivity not initialized"); + if (nullptr == m_iotivity || m_type.empty()) { + LOG_E(TAG, "IotResourceClient: failed to find, resource incorrectly initialized"); return false; } @@ -65,15 +93,36 @@ IotResourceClient::operator bool() const return bool(m_resource); } +OCRepresentation IotResourceClient::get(const OC::QueryParamsMap& query_params) +{ + if (m_resource) { + GetResourceCallback::Sptr callback = std::make_shared(); + + auto result = m_resource->get(query_params, bind_callback(callback, PH::_1, PH::_2, PH::_3)); + + if (OC_STACK_OK != result) { + throw IoTInternalError("Resource get request error", result); + } + + if (!callback->wait()) { + throw IoTInternalError("Resource get callback not called", EC_UNAUTHORIZED); + } + + if (callback->errorCode > OC_STACK_RESOURCE_CHANGED) { + throw IoTInternalError("Resource get callback error", callback->errorCode); + } + + return callback->representation; + } + + return OCRepresentation{}; +} + OCStackResult IotResourceClient::post(const OCRepresentation& representation, const QueryParamsMap& query_params) { OCStackResult res = OC_STACK_ERROR; if (m_resource) { - std::mutex mtx; - std::unique_lock lck(mtx); - std::condition_variable cvar; - bool fired = false; auto callback = std::make_shared(); res = m_resource->post( @@ -81,8 +130,12 @@ OCStackResult IotResourceClient::post(const OCRepresentation& representation, co query_params, bind_callback(callback, PH::_1, PH::_2, PH::_3)); - if (callback->wait()) { - res = static_cast(callback->errorCode); + if (res == OC_STACK_OK) { + if (callback->wait()) { + res = static_cast(callback->errorCode); + } else { + res = OC_STACK_ERROR; + } } } diff --git a/device_core/iotivity_lib/src/iotivity_impl.cpp b/device_core/iotivity_lib/src/iotivity_impl.cpp index 0c55a11..83c95e3 100644 --- a/device_core/iotivity_lib/src/iotivity_impl.cpp +++ b/device_core/iotivity_lib/src/iotivity_impl.cpp @@ -328,9 +328,8 @@ void IoTivity_impl::signIn() throw IoTInternalError("No account manager", EC_IOTIVITY_ERROR); } - std::unique_lock lock(instance_mutex); - shared_ptr signInCb = make_shared(); + std::unique_lock lock(instance_mutex); guardErrorCode(account_mgr->signIn( cloud_uid, @@ -338,15 +337,22 @@ void IoTivity_impl::signIn() std::bind(SignInCallback::call, weak_ptr(signInCb), PH::_1, PH::_2, PH::_3)), SIGNIN); signInCb->condVar.wait_for(lock, std::chrono::seconds(DEFAULT_TIMEOUT)); + lock.unlock(); + guardTimeout(signInCb->timeout, SIGNIN); guardPostErrorCode(signInCb->resultCode, SIGNIN); + lock.lock(); + if (cloud_sid.empty()) { cloud_sid = signInCb->sid; } + signed_in = true; connected = true; + lock.unlock(); + mq_handler = std::make_shared(cloud_host); } diff --git a/device_core/nmdaemon/main.cpp b/device_core/nmdaemon/main.cpp index 471c34b..c333708 100644 --- a/device_core/nmdaemon/main.cpp +++ b/device_core/nmdaemon/main.cpp @@ -29,6 +29,8 @@ static std::string g_device_type = "iotdevice"; static MainThread g_main_thread; bool volatile g_running = true; +// Default exit code is 1 to enable systemd to restart the service +bool volatile g_ret_code = 1; void sig_handler(int sig); static void kill_handler(int _sig); @@ -125,13 +127,7 @@ int main(int argc, char** argv) write_log(TAG "stopped\n"); -#ifndef __BUILD_UBUNTU__ - while (1) { - sleep(1); - } -#endif - - return 0; + return g_ret_code; } void sig_handler(int sig) @@ -161,6 +157,8 @@ void sig_handler(int sig) void kill_handler(int _sig) { + // Request from systemd to stop the daemon + g_ret_code = 0; write_log(TAG "should stopped\n"); g_running = false; threads_done(); diff --git a/device_core/utest/test_hubclient.cpp b/device_core/utest/test_hubclient.cpp index 1f9ae42..bd12e24 100644 --- a/device_core/utest/test_hubclient.cpp +++ b/device_core/utest/test_hubclient.cpp @@ -32,7 +32,7 @@ TEST(Test_HubClient, construction) EXPECT_FALSE(!hub_copy); EXPECT_EQ(RESOURCE_HOST, hub.host()); - EXPECT_EQ("", hub.getUUID()); + EXPECT_EQ("", hub.sid()); IoTDevicesMap devs; EXPECT_ANY_THROW(hub.getOwnedDevices(devs)); -- 2.7.4