diff --git a/include/hnz.h b/include/hnz.h index 74fc3e4..93f041c 100644 --- a/include/hnz.h +++ b/include/hnz.h @@ -74,13 +74,15 @@ class HNZ { /** * Start the HZN south plugin + * @param requestedStart tells if start was requested by the Fledge API */ - void start(); + void start(bool requestedStart = false); /** * Stop the HZN south plugin + * @param requestedStop tells if stop was requested by the Fledge API */ - void stop(); + void stop(bool requestedStop = false); /** * Save the callback function and its data @@ -170,7 +172,11 @@ class HNZ { void sendInitialGI(); private: + // Tells if the plugin is currently running std::atomic m_is_running{false}; + // Tells if the plugin should be running (eg: if you should be restarted after a configuration change) + std::atomic m_should_run{false}; + // Receiving threads std::unique_ptr m_receiving_thread_A; std::unique_ptr m_receiving_thread_B; diff --git a/src/hnz.cpp b/src/hnz.cpp index dbf6b10..bb4c7d4 100644 --- a/src/hnz.cpp +++ b/src/hnz.cpp @@ -21,14 +21,23 @@ HNZ::HNZ() {} HNZ::~HNZ() { if (m_is_running) { - stop(); + stop(true); } } -void HNZ::start() { +void HNZ::start(bool requestedStart /*= false*/) { std::lock_guard guard(m_configMutex); std::string beforeLog = HnzUtility::NamePlugin + " - HNZ::start -"; + if (requestedStart) { + m_should_run = true; + } + + if (m_is_running) { + HnzUtility::log_info("%s HNZ south plugin already started", beforeLog.c_str()); + return; + } + if (!m_hnz_conf->is_complete()) { HnzUtility::log_info("%s HNZ south plugin can't start because configuration is incorrect.", beforeLog.c_str()); return; @@ -53,11 +62,15 @@ void HNZ::start() { m_hnz_connection->start(); } -void HNZ::stop() { +void HNZ::stop(bool requestedStop /*= false*/) { std::string beforeLog = HnzUtility::NamePlugin + " - HNZ::stop -"; HnzUtility::log_info("%s Starting shutdown of HNZ plugin", beforeLog.c_str()); m_is_running = false; + if (requestedStop) { + m_should_run = false; + } + // Connection must be stopped before management threads of both path // or join on both receive threads will hang forever if (m_hnz_connection != nullptr) { @@ -104,7 +117,7 @@ void HNZ::setJsonConfig(const string& protocol_conf_json, const string& msg_conf HnzUtility::log_info("%s No new configuration provided to reconfigure, skipping", beforeLog.c_str()); return; } - bool was_running = m_is_running; + if (m_is_running) { HnzUtility::log_info("%s Configuration change requested, stopping the plugin", beforeLog.c_str()); stop(); @@ -129,7 +142,7 @@ void HNZ::setJsonConfig(const string& protocol_conf_json, const string& msg_conf m_test_msg_receive = m_hnz_conf->get_test_msg_receive(); m_hnz_connection = make_unique(m_hnz_conf, this); - if (was_running) { + if (m_should_run) { HnzUtility::log_warn("%s Restarting the plugin...", beforeLog.c_str()); start(); } diff --git a/src/plugin.cpp b/src/plugin.cpp index d1fce1d..016f896 100644 --- a/src/plugin.cpp +++ b/src/plugin.cpp @@ -190,7 +190,7 @@ void plugin_start(PLUGIN_HANDLE *handle) { std::string beforeLog = HnzUtility::NamePlugin + " - plugin_start -"; HnzUtility::log_info("%s Starting the plugin...", beforeLog.c_str()); auto hnz = reinterpret_cast(handle); - hnz->start(); + hnz->start(true); HnzUtility::log_info("%s Plugin started", beforeLog.c_str()); } diff --git a/tests/test_hnz.cpp b/tests/test_hnz.cpp index 25828fe..813cc2f 100644 --- a/tests/test_hnz.cpp +++ b/tests/test_hnz.cpp @@ -204,13 +204,13 @@ class HNZTest : public testing::Test { const std::string& protocol_stack = protocol_stack_generator(port, port2); std::string configure = std::regex_replace(configureTemplate, std::regex(""), protocol_stack); configure = std::regex_replace(configure, std::regex(""), exchanged_data_def); - auto config = new ConfigCategory("newConfig", configure); - hnz->reconfigure(*config); + ConfigCategory config("newConfig", configure); + hnz->reconfigure(config); } static void startHNZ(int port, int port2) { initConfig(port, port2); - hnz->start(); + hnz->start(true); } template @@ -661,13 +661,13 @@ class ServersWrapper { } std::shared_ptr server1() { if (m_server1 && !m_server1->HNZServerIsReady()) { - m_server1 = nullptr; + return nullptr; } return m_server1; } std::shared_ptr server2() { if (m_server2 && !m_server2->HNZServerIsReady()) { - m_server2 = nullptr; + return nullptr; } return m_server2; } @@ -2155,6 +2155,107 @@ TEST_F(HNZTest, ReconfigureWhileConnectionActive) { if(HasFatalFailure()) return; } +TEST_F(HNZTest, ReconfigureBadConfig) { + ServersWrapper wrapper(0x05, getNextPort()); + BasicHNZServer* server = wrapper.server1().get(); + ASSERT_NE(server, nullptr) << "Something went wrong. Connection is not established in 10s..."; + validateAllTIQualityUpdate(true, false); + if(HasFatalFailure()) return; + + debug_print("[HNZ south plugin] Send bad plugin configuration"); + clearReadings(); + static const std::string badConfig = QUOTE({ + "exchanged_data" : { + "value": 42 + } + }); + ConfigCategory config("newConfig", badConfig); + hnz->reconfigure(config); + + // Check that connection was lost + waitUntil(southEventsReceived, 1, 1000); + // Check that ingestCallback had been called only one time + ASSERT_EQ(southEventsReceived, 1); + // Validate new connection state + std::shared_ptr currentReading = popFrontReadingsUntil("TEST_STATUS"); + validateSouthEvent(currentReading, "TEST_STATUS", { + {"connx_status", "not connected"}, + }); + if(HasFatalFailure()) return; + + // No quality update message as new config contains no TI + ASSERT_EQ(dataObjectsReceived, 0); + + // Also stop the server as it is unable to reconnect on the fly + debug_print("[HNZ server] Request server stop..."); + server->stopHNZServer(); + debug_print("[HNZ server] Request server start..."); + server->startHNZServer(); + + // Check that connection cannot be established as client is no longer running due to invalid configuration + BasicHNZServer* deadServer = wrapper.server1().get(); + ASSERT_EQ(deadServer, nullptr) << "Something went wrong. Server should not be able to reconnect, but it did!"; + + // This calls HNZ::reconfigure() again, causing a reconnect of the client + debug_print("[HNZ south plugin] Send good plugin configuration"); + clearReadings(); + wrapper.initHNZPlugin(); + + // Check that connection attempt to reopen on client side + validateAllTIQualityUpdate(true, false, true); + if(HasFatalFailure()) return; + + // Restart the server + debug_print("[HNZ server] Request server stop 2..."); + server->stopHNZServer(); + debug_print("[HNZ server] Request server start 2..."); + server->startHNZServer(); + + // Check that the server is reconnected after reconfigure + server = wrapper.server1().get(); + ASSERT_NE(server, nullptr) << "Something went wrong. Connection 2 is not established in 10s..."; + // Wait for initial CG request + waitUntil(southEventsReceived, 2, 1000); + // Check that ingestCallback had been called only for two GI status updates + ASSERT_EQ(southEventsReceived, 2); + resetCounters(); + // Validate reconnection + currentReading = popFrontReadingsUntil("TEST_STATUS"); + validateSouthEvent(currentReading, "TEST_STATUS", { + {"connx_status", "started"}, + }); + if(HasFatalFailure()) return; + // Validate new GI state + currentReading = popFrontReadingsUntil("TEST_STATUS"); + validateSouthEvent(currentReading, "TEST_STATUS", { + {"gi_status", "started"}, + }); + if(HasFatalFailure()) return; + + // Complete CG request by sending all expected TS + server->sendFrame({0x16, 0x33, 0x00, 0x00, 0x00, 0x00}, false); + server->sendFrame({0x16, 0x39, 0x00, 0x02, 0x00, 0x00}, false); + debug_print("[HNZ Server] TSCG sent"); + waitUntil(southEventsReceived, 2, 1000); + // Check that ingestCallback had been called only for two GI status updates + ASSERT_EQ(southEventsReceived, 2); + resetCounters(); + currentReading = popFrontReadingsUntil("TEST_STATUS"); + validateSouthEvent(currentReading, "TEST_STATUS", { + {"gi_status", "in progress"}, + }); + if(HasFatalFailure()) return; + currentReading = popFrontReadingsUntil("TEST_STATUS"); + validateSouthEvent(currentReading, "TEST_STATUS", { + {"gi_status", "finished"}, + }); + if(HasFatalFailure()) return; + + // Manually stop the server here or we may end up in a deadlock in the HNZServer + debug_print("[HNZ server] Request server stop 3..."); + server->stopHNZServer(); +} + TEST_F(HNZTest, UnknownMessage) { ServersWrapper wrapper(0x05, getNextPort()); BasicHNZServer* server = wrapper.server1().get();