Skip to content

Commit

Permalink
code smell
Browse files Browse the repository at this point in the history
  • Loading branch information
dan-du-car committed Nov 13, 2023
1 parent 74e5b8f commit 2ea0021
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 65 deletions.
59 changes: 25 additions & 34 deletions src/tmx/TmxUtils/src/SNMPClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace tmx::utils
: ip_(ip), port_(port), community_(community), snmp_version_(snmp_version), timeout_(timeout)
{

PLOG(logDEBUG1) << "Starting SNMP Client. Target device IP address: " << ip_<< ", Target device SNMP port: " << port_;
PLOG(logDEBUG1) << "Starting SNMP Client. Target device IP address: " << ip_ << ", Target device SNMP port: " << port_;

// Bring the IP address and port of the target SNMP device in the required form, which is "IPADDRESS:PORT":
std::string ip_port_string = ip_ + ":" + std::to_string(port_);
Expand All @@ -27,11 +27,7 @@ namespace tmx::utils
// Fallback behavior to setup a community for SNMP V1/V2
if (snmp_version_ != SNMP_VERSION_3)
{
char community_char[community_.length()];
std::copy(community_.begin(), community_.end(), community_char);
unsigned char *comm = reinterpret_cast<unsigned char *>(community_char);

session.community = comm;
session.community = (unsigned char *)community.c_str();
session.community_len = community_.length();
}

Expand All @@ -54,40 +50,35 @@ namespace tmx::utils
auto phrase = (u_char *)authPassPhrase.c_str();

// Defining and generating auth config with SHA1
session.securityAuthProto = snmp_duplicate_objid(usmHMACSHA1AuthProtocol, USM_AUTH_PROTO_SHA_LEN);;
session.securityAuthProto = snmp_duplicate_objid(usmHMACSHA1AuthProtocol, USM_AUTH_PROTO_SHA_LEN);
session.securityAuthProtoLen = USM_AUTH_PROTO_SHA_LEN;
session.securityAuthKeyLen = USM_AUTH_KU_LEN;
if(session.securityLevel != SNMP_SEC_LEVEL_NOAUTH)
if (session.securityLevel != SNMP_SEC_LEVEL_NOAUTH && generate_Ku(session.securityAuthProto,
session.securityAuthProtoLen,
phrase, phrase_len,
session.securityAuthKey,
&session.securityAuthKeyLen) != SNMPERR_SUCCESS)
{
if (generate_Ku(session.securityAuthProto,
session.securityAuthProtoLen,
phrase, phrase_len,
session.securityAuthKey,
&session.securityAuthKeyLen) != SNMPERR_SUCCESS)
{
std::string errMsg = "Error generating Ku from authentication pass phrase. \n";
throw snmp_client_exception(errMsg);
}
std::string errMsg = "Error generating Ku from authentication pass phrase. \n";
throw snmp_client_exception(errMsg);
}

// Defining and generating priv config with AES (since using SHA1)
if(session.securityLevel == SNMP_SEC_LEVEL_AUTHPRIV)
session.securityPrivKeyLen = USM_PRIV_KU_LEN;
session.securityPrivProto =
snmp_duplicate_objid(usmAESPrivProtocol,
OID_LENGTH(usmAESPrivProtocol));
session.securityPrivProtoLen = OID_LENGTH(usmAESPrivProtocol);

if (session.securityLevel == SNMP_SEC_LEVEL_AUTHPRIV && generate_Ku(session.securityAuthProto,
session.securityAuthProtoLen,
phrase, phrase_len,
session.securityPrivKey,
&session.securityPrivKeyLen) != SNMPERR_SUCCESS)
{
session.securityPrivKeyLen = USM_PRIV_KU_LEN;
session.securityPrivProto =
snmp_duplicate_objid(usmAESPrivProtocol,
OID_LENGTH(usmAESPrivProtocol));
session.securityPrivProtoLen = OID_LENGTH(usmAESPrivProtocol);
if (generate_Ku(session.securityAuthProto,
session.securityAuthProtoLen,
phrase, phrase_len,
session.securityPrivKey,
&session.securityPrivKeyLen) != SNMPERR_SUCCESS)
{
std::string errMsg = "Error generating Ku from privacy pass phrase. \n";
throw snmp_client_exception(errMsg);
}
}
std::string errMsg = "Error generating Ku from privacy pass phrase. \n";
throw snmp_client_exception(errMsg);
}

session.timeout = timeout_;

Expand Down Expand Up @@ -316,7 +307,7 @@ namespace tmx::utils
return port_;
}

void snmp_client::log_error(const int &status, const request_type &request_type, snmp_pdu *response) const
void snmp_client::log_error(const int &status, const request_type &request_type, const snmp_pdu *response) const
{

if (status == STAT_SUCCESS)
Expand Down
7 changes: 5 additions & 2 deletions src/tmx/TmxUtils/src/SNMPClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ class snmp_client
snmp_client(const std::string& ip, const int& port, const std::string& community, const std::string &snmp_user, const std::string &securityLevel, const std::string &authPassPhrase, int snmp_version = 0, int timeout = 100);

/* Disable default copy constructor*/
snmp_client() = delete;
snmp_client(snmp_client& sc) = delete;

/* Disable default move constructor*/
snmp_client(snmp_client&& sc) = delete;


/** @brief Returns true or false depending on whether the request could be processed for given input OID at the Traffic Signal Controller.
Expand All @@ -106,7 +109,7 @@ class snmp_client

virtual int get_port() const; // Returns the current port (should always be 161 or 162)

void log_error(const int& status, const request_type& request_type, snmp_pdu *response) const;
void log_error(const int& status, const request_type& request_type, const snmp_pdu *response) const;

/** @brief Destructor for client. Closes the snmp session**/
virtual ~snmp_client();
Expand Down
45 changes: 19 additions & 26 deletions src/v2i-hub/RSUHealthMonitorPlugin/src/RSUHealthMonitorPlugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ namespace RSUHealthMonitor
_rsuStatusTimer->AddPeriodicTick([this]()
{
// Periodic SNMP call to get RSU status based on RSU MIB version 4.1
auto rsuStatusJson = _rsuWorker->getRSUStatus(_rsuMibVersion, _rsuIp, _snmpPort, _authPassPhrase, _securityUser, _securityLevel, SEC_TO_MICRO);

auto rsuStatusJson = _rsuWorker->getRSUStatus(_rsuMibVersion, _rsuIp, _snmpPort, _securityUser, _authPassPhrase, _securityLevel, SEC_TO_MICRO);
PLOG(logINFO) << "Updating _interval: " << _interval;
//Broadcast RSU status periodically at _interval
BroadcastRSUStatus(rsuStatusJson); },
chrono::seconds(_interval));
std::chrono::milliseconds(_interval * SEC_TO_MILLI));
_rsuStatusTimer->Start();
}

Expand Down Expand Up @@ -58,32 +58,25 @@ namespace RSUHealthMonitor

void RSUHealthMonitorPlugin::BroadcastRSUStatus(const Json::Value &rsuStatusJson)
{
try
// Broadcast the RSU status info when there are RSU responses.
if (!rsuStatusJson.empty())
{
// Broadcast the RSU status info when there are RSU responses.
if (!rsuStatusJson.empty())
vector<string> rsuStatusFields;
for (auto const &field : rsuStatusJson.getMemberNames())
{
vector<string> rsuStatusFields;
for (auto const &field : rsuStatusJson.getMemberNames())
{
rsuStatusFields.push_back(field);
}
// Only broadcast RSU status when all required fields are present.
if (_rsuWorker && _rsuWorker->isAllRequiredFieldsPresent(_rsuMibVersion, rsuStatusFields))
{
Json::FastWriter fasterWirter;
string json_str = fasterWirter.write(rsuStatusJson);
tmx::messages::RSUStatusMessage sendRsuStatusMsg;
sendRsuStatusMsg.set_contents(json_str);
string source = RSUHealthMonitorPlugin::GetName();
BroadcastMessage(sendRsuStatusMsg, source);
PLOG(logINFO) << "Broadcast RSU status: " << json_str;
}
rsuStatusFields.push_back(field);
}
// Only broadcast RSU status when all required fields are present.
if (_rsuWorker && _rsuWorker->isAllRequiredFieldsPresent(_rsuMibVersion, rsuStatusFields))
{
Json::FastWriter fasterWirter;
string json_str = fasterWirter.write(rsuStatusJson);
tmx::messages::RSUStatusMessage sendRsuStatusMsg;
sendRsuStatusMsg.set_contents(json_str);
string source = RSUHealthMonitorPlugin::GetName();
BroadcastMessage(sendRsuStatusMsg, source);
PLOG(logINFO) << "Broadcast RSU status: " << json_str;
}
}
catch (const std::exception &ex)
{
PLOG(logERROR) << ex.what();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace RSUHealthMonitor
shared_ptr<RSUHealthMonitorWorker> _rsuWorker;
unique_ptr<ThreadTimer> _rsuStatusTimer;
const long SEC_TO_MICRO = 1000000;
const long SEC_TO_MILLI= 1000;
/**
* @brief Update RSU OID configuration map with input JSON string.
* @param JSON string with RSU OID configuration.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ namespace RSUHealthMonitor
return isAllPresent;
}

RSUStatusConfigTable RSUHealthMonitorWorker::GetRSUStatusConfig(const RSUMibVersion &mibVersion)
RSUStatusConfigTable RSUHealthMonitorWorker::GetRSUStatusConfig(const RSUMibVersion &mibVersion) const
{
RSUStatusConfigTable result;
try
Expand All @@ -86,7 +86,6 @@ namespace RSUHealthMonitor
catch (const out_of_range &ex)
{
PLOG(logERROR) << "Unknown MIB version! " << ex.what();
;
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ namespace RSUHealthMonitor
RSUHealthMonitorWorker();

// Access to the RSU status table based in the RSU MIB version provided
RSUStatusConfigTable GetRSUStatusConfig(const RSUMibVersion &mibVersion);
RSUStatusConfigTable GetRSUStatusConfig(const RSUMibVersion &mibVersion) const;

/**
* @brief determine if all required fields in the RSU config map _RSUSTATUSConfigMapPtr present in the input fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,11 @@ namespace RSUHealthMonitor
auto gps_map_invalid = _rsuWorker->ParseRSUGPS(invalid_gps_nmea_data);
ASSERT_EQ(0, gps_map_invalid.size());
}

TEST_F(test_RSUHealthMonitorWorker, getRSUStatus)
{
uint16_t port = 161;
auto json = _rsuWorker->getRSUStatus(RSUMibVersion::RSUMIB_V_4_1, "127.0.0.1", port, "test", "testtesttest", "authPriv", 1000);
ASSERT_TRUE(json.empty());
}
}

0 comments on commit 2ea0021

Please sign in to comment.