Skip to content

Commit

Permalink
refs #60 Sonarqube code quality fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
Florent Peyrusse committed Aug 28, 2023
1 parent f7066af commit b93554f
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 69 deletions.
24 changes: 11 additions & 13 deletions include/hnz.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@

#include "hnzconf.h"

using namespace std;
using namespace std::chrono;

class HNZConnection;
class HNZPath;

Expand Down Expand Up @@ -50,7 +47,7 @@ class HNZ {
HNZ();
~HNZ();

void setAssetName(const string& asset) { m_asset = asset; }
void setAssetName(const std::string& asset) { m_asset = asset; }

/**
* Start the HZN south plugin
Expand All @@ -68,8 +65,8 @@ class HNZ {
* @param protocol_conf_json Contain value to configure the protocol
* @param msg_conf_json Describe the messages that the plugin can received
*/
bool setJsonConfig(const string& protocol_conf_json,
const string& msg_configuration);
bool setJsonConfig(const std::string& protocol_conf_json,
const std::string& msg_configuration);

/**
* Save the callback function and its data
Expand Down Expand Up @@ -150,16 +147,17 @@ class HNZ {
void sendInitialGI();

private:
string m_asset; // Plugin name in fledge
atomic<bool> m_is_running;
thread *m_receiving_thread_A,
*m_receiving_thread_B = nullptr; // Receiving threads
std::string m_asset; // Plugin name in fledge
std::atomic<bool> m_is_running;
// Receiving threads
std::unique_ptr<std::thread> m_receiving_thread_A;
std::unique_ptr<std::thread> m_receiving_thread_B;
// Contains all addressed of the TS received in response to a GI request
vector<unsigned int> m_gi_addresses_received;
std::vector<unsigned int> m_gi_addresses_received;

// Others HNZ related class
HNZConf* m_hnz_conf = nullptr; // HNZ Configuration
HNZConnection* m_hnz_connection = nullptr; // HNZ Connection handling
std::shared_ptr<HNZConf> m_hnz_conf; // HNZ Configuration
std::unique_ptr<HNZConnection> m_hnz_connection; // HNZ Connection handling

// Fledge related
INGEST_CB m_ingest; // Callback function used to send data to south service
Expand Down
7 changes: 7 additions & 0 deletions include/hnzconf.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@
#define HNZConf_H

#include <map>
#include <memory>
#include "rapidjson/document.h"

// Local definition of make_unique as it is only available since C++14 and right now fledge-south-hnz is built with C++11
template<typename T, typename... Args>
std::unique_ptr<T> make_unique(Args&&... args) {
return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
}

constexpr char JSON_CONF_NAME[] = "protocol_stack";
constexpr char NAME[] = "name";
constexpr char JSON_VERSION[] = "version";
Expand Down
6 changes: 3 additions & 3 deletions include/hnzconnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ enum class GiStatus;
*/
class HNZConnection {
public:
HNZConnection(HNZConf* hnz_conf, HNZ* hnz_fledge);
HNZConnection(std::shared_ptr<HNZConf> hnz_conf, HNZ* hnz_fledge);
~HNZConnection();

/**
Expand Down Expand Up @@ -172,8 +172,8 @@ class HNZConnection {
*/
void m_update_quality_update_timer();

HNZConf* m_hnz_conf = nullptr; // HNZ Configuration
HNZ* m_hnz_fledge = nullptr; // HNZ Fledge
std::shared_ptr<HNZConf> m_hnz_conf; // HNZ Configuration
HNZ* m_hnz_fledge = nullptr; // HNZ Fledge
};

#endif
16 changes: 5 additions & 11 deletions include/hnzpath.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ class HNZPath {
// Give access to HNZPath private members for HNZConnection
friend class HNZConnection;
public:
HNZPath(const HNZConf* hnz_conf, HNZConnection* hnz_connection, bool secondary);
HNZPath(const std::shared_ptr<HNZConf> hnz_conf, HNZConnection* hnz_connection, bool secondary);
~HNZPath();

string getName() { return m_name_log; };
string getName() const { return m_name_log; };

/**
* Connect (or re-connect) to the HNZ PA (TCP connection and HNZ connection
Expand Down Expand Up @@ -100,7 +100,7 @@ class HNZPath {
/**
* Returns the number of times the last message was sent.
*/
int getRepeat() { return m_repeat; };
int getRepeat() const { return m_repeat; };

/**
* Resend a message that has already been sent but not acknowledged.
Expand Down Expand Up @@ -153,13 +153,13 @@ class HNZPath {
* Gets the state of the path
* @return true if active, false if passive
*/
bool isActivePath() { return m_is_active_path; }
bool isActivePath() const { return m_is_active_path; }

/**
* Gets the state of the HNZ protocol (CONNECTION, CONNECTED)
* @return CONNECTION if SARM/UA step is not complete, CONNECTED after that
*/
int getProtocolState() { return m_protocol_state; }
int getProtocolState() const { return m_protocol_state; }

private:
std::unique_ptr<HNZClient> m_hnz_client; // HNZ Client that manage TCP connection
Expand Down Expand Up @@ -325,12 +325,6 @@ class HNZPath {
void m_send_time_setting();

void m_go_to_connected();

// Local definition of make_unique as it is only available since C++14 and right now fledge-south-hnz is built with C++11
template<typename T, typename... Args>
std::unique_ptr<T> make_unique(Args&&... args) {
return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
}
};

#endif
27 changes: 11 additions & 16 deletions src/hnz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@
#include "hnzconnection.h"
#include "hnzpath.h"

HNZ::HNZ() : m_hnz_conf(new HNZConf), m_is_running(false) {}
HNZ::HNZ() : m_hnz_conf(std::make_shared<HNZConf>()), m_is_running(false) {}

HNZ::~HNZ() {
if (m_is_running) {
stop();
}
if (m_hnz_connection != nullptr) delete m_hnz_connection;
if (m_hnz_conf != nullptr) delete m_hnz_conf;
}

void HNZ::start() {
Expand All @@ -44,12 +42,12 @@ void HNZ::start() {
m_sendAllTSQualityReadings(true, false);

auto pathPair = m_hnz_connection->getBothPath();
m_receiving_thread_A = new thread(&HNZ::receive, this, pathPair.first);
m_receiving_thread_A = make_unique<thread>(&HNZ::receive, this, pathPair.first);
if (pathPair.second != nullptr) {
// Wait after getting the passive path pointer as connection init of active path may swap path
this_thread::sleep_for(milliseconds(1000));
// Path B is defined in the configuration
m_receiving_thread_B = new thread(&HNZ::receive, this, pathPair.second);
m_receiving_thread_B = make_unique<thread>(&HNZ::receive, this, pathPair.second);
}

m_hnz_connection->start();
Expand All @@ -61,20 +59,17 @@ void HNZ::stop() {

if (m_hnz_connection != nullptr) {
m_hnz_connection->stop();
delete m_hnz_connection;
m_hnz_connection = nullptr;
}

if (m_receiving_thread_A != nullptr) {
HnzUtility::log_debug("Waiting for the receiving thread (path A)");
m_receiving_thread_A->join();
delete m_receiving_thread_A;
m_receiving_thread_A = nullptr;
}
if (m_receiving_thread_B != nullptr) {
HnzUtility::log_debug("Waiting for the receiving thread (path B)");
m_receiving_thread_B->join();
delete m_receiving_thread_B;
m_receiving_thread_B = nullptr;
}
HnzUtility::log_info("Plugin stopped !");
Expand Down Expand Up @@ -103,7 +98,7 @@ bool HNZ::setJsonConfig(const string &protocol_conf_json,

m_remote_address = m_hnz_conf->get_remote_station_addr();
m_test_msg_receive = m_hnz_conf->get_test_msg_receive();
m_hnz_connection = new HNZConnection(m_hnz_conf, this);
m_hnz_connection = make_unique<HNZConnection>(m_hnz_conf, this);

if (was_running) {
HnzUtility::log_warn("Restarting the plugin...");
Expand Down Expand Up @@ -137,10 +132,10 @@ void HNZ::receive(HNZPath *hnz_path_in_use) {
// Try to reconnect, unless thread is stopping
if (m_is_running) {
hnz_path_in_use->disconnect();
// Shutdown request may happen while disconnecting, if it does cancel reconnection
if (m_is_running) {
hnz_path_in_use->connect();
}
}
// Shutdown request may happen while disconnecting, if it does cancel reconnection
if (m_is_running) {
hnz_path_in_use->connect();
}
} else {
// Push each message to fledge
Expand Down Expand Up @@ -535,9 +530,9 @@ unsigned long HNZ::getEpochMsTimestamp(std::chrono::time_point<std::chrono::syst
static const auto oneDay = std::chrono::hours{24};
// Get the date of the start of the day in epoch milliseconds
auto days = dateTime - (dateTime.time_since_epoch() % oneDay);
unsigned long epochMs = duration_cast<std::chrono::milliseconds>(days.time_since_epoch()).count();
unsigned long epochMs = std::chrono::duration_cast<std::chrono::milliseconds>(days.time_since_epoch()).count();
// Add or remove one day if we are at edge of day and day section is on the other day
long int ms_today = duration_cast<std::chrono::milliseconds>(dateTime.time_since_epoch()).count() % oneDayMs;
long int ms_today = std::chrono::duration_cast<std::chrono::milliseconds>(dateTime.time_since_epoch()).count() % oneDayMs;
long int hours = (ms_today / oneHourMs) % 24;
// Remote section of day is after midnight but local clock is before midnight: add one day
if ((daySection == 0) && (hours == 23)) {
Expand Down Expand Up @@ -698,7 +693,7 @@ void HNZ::m_sendAllTMQualityReadings(bool invalid, bool outdated, const vector<u

void HNZ::m_sendAllTSQualityReadings(bool invalid, bool outdated, const vector<unsigned int>& rejectFilter /*= {}*/) {
ReadingParameters paramsTemplate;
unsigned long epochMs = duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
unsigned long epochMs = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
paramsTemplate.msg_code = "TS";
paramsTemplate.station_addr = m_remote_address;
paramsTemplate.valid = static_cast<unsigned int>(invalid);
Expand Down
4 changes: 2 additions & 2 deletions src/hnzconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "hnzpath.h"
#include "hnzconnection.h"

HNZConnection::HNZConnection(HNZConf* hnz_conf, HNZ* hnz_fledge) {
HNZConnection::HNZConnection(std::shared_ptr<HNZConf> hnz_conf, HNZ* hnz_fledge) {
this->m_hnz_conf = hnz_conf;
this->m_hnz_fledge = hnz_fledge;

Expand Down Expand Up @@ -223,7 +223,7 @@ void HNZConnection::m_check_command_timer() {
void HNZConnection::m_update_current_time() {
uint64_t prevTimeMs = m_current;
m_current =
duration_cast<milliseconds>(system_clock::now().time_since_epoch())
std::chrono::duration_cast<milliseconds>(system_clock::now().time_since_epoch())
.count();
m_elapsedTimeMs = m_current - prevTimeMs;
}
Expand Down
16 changes: 8 additions & 8 deletions src/hnzpath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "hnz.h"
#include "hnzpath.h"

HNZPath::HNZPath(const HNZConf* hnz_conf, HNZConnection* hnz_connection, bool secondary):
HNZPath::HNZPath(const std::shared_ptr<HNZConf> hnz_conf, HNZConnection* hnz_connection, bool secondary):
// Path settings
m_hnz_client(make_unique<HNZClient>()),
m_hnz_connection(hnz_connection),
Expand Down Expand Up @@ -443,7 +443,7 @@ void HNZPath::m_receivedRR(int nr, bool repetition) {

// Waiting for other RR, set timer
if (!msg_sent.empty())
last_sent_time = duration_cast<milliseconds>(
last_sent_time = std::chrono::duration_cast<milliseconds>(
system_clock::now().time_since_epoch())
.count();

Expand Down Expand Up @@ -547,7 +547,7 @@ void HNZPath::m_sendInfoImmediately(Message message) {
// Set timer if there is not other message sent waiting for confirmation
if (msg_sent.empty())
last_sent_time =
duration_cast<milliseconds>(system_clock::now().time_since_epoch())
std::chrono::duration_cast<milliseconds>(system_clock::now().time_since_epoch())
.count();

message.ns = m_ns;
Expand All @@ -569,7 +569,7 @@ void HNZPath::sendBackInfo(Message& message) {
sizeof(msgWithNrNs));

last_sent_time =
duration_cast<milliseconds>(system_clock::now().time_since_epoch())
std::chrono::duration_cast<milliseconds>(system_clock::now().time_since_epoch())
.count();
}

Expand All @@ -590,7 +590,7 @@ void HNZPath::m_send_date_setting() {
void HNZPath::m_send_time_setting() {
unsigned char msg[5];
long int ms_since_epoch, mod10m, frac;
ms_since_epoch = duration_cast<milliseconds>(
ms_since_epoch = std::chrono::duration_cast<milliseconds>(
high_resolution_clock::now().time_since_epoch())
.count();
long int ms_today = ms_since_epoch % 86400000;
Expand Down Expand Up @@ -618,7 +618,7 @@ void HNZPath::sendGeneralInterrogation() {
m_hnz_connection->updateGiStatus(GiStatus::STARTED);
}
gi_repeat++;
gi_start_time = duration_cast<milliseconds>(
gi_start_time = std::chrono::duration_cast<milliseconds>(
high_resolution_clock::now().time_since_epoch())
.count();
}
Expand All @@ -637,7 +637,7 @@ bool HNZPath::sendTVCCommand(unsigned char address, int value) {

// Add the command in the list of commend sent (to check ACK later)
Command_message cmd;
cmd.timestamp_max = duration_cast<milliseconds>(
cmd.timestamp_max = std::chrono::duration_cast<milliseconds>(
high_resolution_clock::now().time_since_epoch())
.count() +
c_ack_time_max;
Expand All @@ -663,7 +663,7 @@ bool HNZPath::sendTCCommand(unsigned char address, unsigned char value) {

// Add the command in the list of commend sent (to check ACK later)
Command_message cmd;
cmd.timestamp_max = duration_cast<milliseconds>(
cmd.timestamp_max = std::chrono::duration_cast<milliseconds>(
high_resolution_clock::now().time_since_epoch())
.count() +
c_ack_time_max;
Expand Down
4 changes: 2 additions & 2 deletions tests/test_hnz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ class HNZTest : public testing::Test {
waitUntil(dataObjectsReceived, expectedMessages, maxWaitTimeMs);
ASSERT_EQ(dataObjectsReceived, expectedMessages);
resetCounters();
unsigned long epochMs = duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
unsigned long epochMs = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
// First 7 messages are from init
// Those messages are expected to be sent before the CG time frame
std::string timeRangeStr(to_string(epochMs - (maxWaitTimeMs + 10000)) + ";" + to_string(epochMs - maxWaitTimeMs));
Expand Down Expand Up @@ -482,7 +482,7 @@ class HNZTest : public testing::Test {
ASSERT_EQ(dataObjectsReceived, labels.size());
resetCounters();
}
unsigned long epochMs = duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
unsigned long epochMs = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
std::string timeRangeStr(to_string(epochMs - 1000) + ";" + to_string(epochMs));
std::shared_ptr<Reading> currentReading = nullptr;
for(const auto& label: labels) {
Expand Down
20 changes: 6 additions & 14 deletions tests/test_hnzconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,33 +58,25 @@ static string exchanged_data_def = QUOTE({
});

TEST(HNZConnection, OnlyOnePathConfigured) {
HNZConf* conf = new HNZConf();
std::shared_ptr<HNZConf> conf = std::make_shared<HNZConf>();
conf->importConfigJson(protocol_stack_def_one_path);
conf->importExchangedDataJson(exchanged_data_def);

HNZ* hnz = new HNZ();
HNZConnection* hnz_connection = new HNZConnection(conf, hnz);
std::unique_ptr<HNZ> hnz = make_unique<HNZ>();
std::unique_ptr<HNZConnection> hnz_connection = make_unique<HNZConnection>(conf, hnz.get());

ASSERT_NE(nullptr, hnz_connection->getActivePath());
ASSERT_EQ(nullptr, hnz_connection->getPassivePath());

delete hnz_connection;
delete conf;
delete hnz;
}

TEST(HNZConnection, TwoPathConfigured) {
HNZConf* conf = new HNZConf();
std::shared_ptr<HNZConf> conf = std::make_shared<HNZConf>();
conf->importConfigJson(protocol_stack_def_two_path);
conf->importExchangedDataJson(exchanged_data_def);

HNZ* hnz = new HNZ();
HNZConnection* hnz_connection = new HNZConnection(conf, hnz);
std::unique_ptr<HNZ> hnz = make_unique<HNZ>();
std::unique_ptr<HNZConnection> hnz_connection = make_unique<HNZConnection>(conf, hnz.get());

ASSERT_NE(nullptr, hnz_connection->getActivePath());
ASSERT_NE(nullptr, hnz_connection->getPassivePath());

delete hnz_connection;
delete conf;
delete hnz;
}

0 comments on commit b93554f

Please sign in to comment.