Skip to content

Commit

Permalink
std::map replaced by CExpMap in description gate (#995)
Browse files Browse the repository at this point in the history 
Bugfix: CDescGate (map entries for topics and services never expired)
Enhancement: 4 new API functions in eCAL::Util namespace (GetTopics, GetTopicNames, GetServices, GetServiceNames) added
  • Loading branch information
@rex-schilasky
rex-schilasky authored Mar 6, 2023
1 parent 4cbd401 commit 4c6acef
Show file tree
Hide file tree
Showing 10 changed files with 362 additions and 109 deletions.
18 changes: 17 additions & 1 deletion ecal/core/include/ecal/ecal_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
#include <ecal/ecal_os.h>

#include <map>
#include <unordered_map>
#include <string>
#include <vector>

namespace eCAL
{
Expand Down Expand Up @@ -148,7 +150,14 @@ namespace eCAL
* @param topic_info_map_ Map to store the topic informations.
* Map containing { TopicName -> (Type, Description) } mapping of all topics that are currently known.
**/
ECAL_API void GetTopics(std::map<std::string, STopicInfo>& topic_info_map_);
ECAL_API void GetTopics(std::unordered_map<std::string, STopicInfo>& topic_info_map_);

/**
* @brief Get all topic names.
*
* @param topic_names_ Vector to store the topic names.
**/
ECAL_API void GetTopicNames(std::vector<std::string>& topic_names_);

/**
* @brief Gets type name of the specified topic.
Expand Down Expand Up @@ -203,6 +212,13 @@ namespace eCAL
**/
ECAL_API void GetServices(std::map<std::tuple<std::string, std::string>, Util::SServiceMethodInfo>& service_info_map_);

/**
* @brief Get all service/method names.
*
* @param service_method_names_ Vector to store the service/method tuples (Vector { (ServiceName, MethodName) }).
**/
ECAL_API void GetServiceNames(std::vector<std::tuple<std::string, std::string>>& service_method_names_);

/**
* @brief Gets service method request and response type names.
*
Expand Down
169 changes: 106 additions & 63 deletions ecal/core/src/ecal_descgate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,20 @@
**/

#include <ecal/ecal_log.h>
#include <ecal/ecal_config.h>

#include "ecal_descgate.h"
#include <assert.h>
#include <algorithm>
#include <mutex>

namespace eCAL
{
CDescGate::CDescGate() = default;
CDescGate::CDescGate() :
m_topic_info_map (std::chrono::milliseconds(Config::GetMonitoringTimeoutMs())),
m_service_info_map(std::chrono::milliseconds(Config::GetMonitoringTimeoutMs()))
{
}
CDescGate::~CDescGate() = default;

void CDescGate::Create()
Expand All @@ -42,30 +48,33 @@ namespace eCAL

void CDescGate::ApplyTopicDescription(const std::string& topic_name_, const std::string& topic_type_, const std::string& topic_desc_, const QualityFlags description_quality_)
{
std::unique_lock<std::shared_timed_mutex> lock(m_topic_info_map_mutex);
const auto topic_info_it = m_topic_info_map.find(topic_name_);
std::unique_lock<std::shared_timed_mutex> lock(m_topic_info_map.sync);
m_topic_info_map.map->remove_deprecated();

const auto topic_info_it = m_topic_info_map.map->find(topic_name_);

// new element (no need to check anything, just add it)
if(topic_info_it == m_topic_info_map.end())
if(topic_info_it == m_topic_info_map.map->end())
{
// TODO: Maybe we should copy the description from another topic with the same type, if it is empty?
STopicInfoQuality topic_info;
STopicInfoQuality& topic_info = (*m_topic_info_map.map)[topic_name_];
topic_info.info.type_name = topic_type_;
topic_info.info.type_description = topic_desc_;
topic_info.description_quality = description_quality_;
m_topic_info_map.emplace(topic_name_, std::move(topic_info));
}
else
{
// existing type for the same topic name should be equal !!
// we log the error only one time
if( !topic_info_it->second.type_missmatch_logged
STopicInfoQuality& topic_info = (*m_topic_info_map.map)[topic_name_];

if( !topic_info.type_missmatch_logged
&& !topic_type_.empty()
&& !topic_info_it->second.info.type_name.empty()
&& (topic_info_it->second.info.type_name != topic_type_)
&& !topic_info.info.type_name.empty()
&& (topic_info.info.type_name != topic_type_)
)
{
std::string ttype1 = topic_info_it->second.info.type_name;
std::string ttype1 = topic_info.info.type_name;
std::string ttype2 = topic_type_;
std::replace(ttype1.begin(), ttype1.end(), '\0', '?');
std::replace(ttype1.begin(), ttype1.end(), '\t', '?');
Expand All @@ -80,66 +89,85 @@ namespace eCAL
msg += "\')";
eCAL::Logging::Log(log_level_warning, msg);

topic_info_it->second.type_missmatch_logged = true;
topic_info.type_missmatch_logged = true;
}

// existing description for the same topic name should be equal !!
// we log the warning only one time
if ( !topic_info_it->second.type_missmatch_logged
if( !topic_info.type_missmatch_logged
&& !topic_desc_.empty()
&& !topic_info_it->second.info.type_description.empty()
&& (topic_info_it->second.info.type_description != topic_desc_)
&& !topic_info.info.type_description.empty()
&& (topic_info.info.type_description != topic_desc_)
)
{
std::string msg = "eCAL Pub/Sub description mismatch for topic ";
msg += topic_name_;
eCAL::Logging::Log(log_level_warning, msg);

topic_info_it->second.type_missmatch_logged = true;
topic_info.type_missmatch_logged = true;
}

// Now let's check whether the current information has a higher quality.
// If it has a higher quality, we overwrite it.
if (description_quality_ > topic_info_it->second.description_quality)
if (description_quality_ > topic_info.description_quality)
{
topic_info_it->second.info.type_name = topic_type_;
topic_info_it->second.info.type_description = topic_desc_;
topic_info_it->second.description_quality = description_quality_;
topic_info.info.type_name = topic_type_;
topic_info.info.type_description = topic_desc_;
topic_info.description_quality = description_quality_;
}
}
}

void CDescGate::GetTopics(std::map<std::string, Util::STopicInfo>& topic_info_map_)
void CDescGate::GetTopics(std::unordered_map<std::string, Util::STopicInfo>& topic_info_map_)
{
topic_info_map_.clear();
std::shared_lock<std::shared_timed_mutex> lock(m_topic_info_map_mutex);
for (auto& topic_info : m_topic_info_map)
std::unordered_map<std::string, Util::STopicInfo> map;

std::shared_lock<std::shared_timed_mutex> lock(m_topic_info_map.sync);
m_topic_info_map.map->remove_deprecated();
map.reserve(m_topic_info_map.map->size());

for (const auto& topic_info : (*m_topic_info_map.map))
{
topic_info_map_[topic_info.first] = topic_info.second.info;
map.emplace(topic_info.first, topic_info.second.info);
}
topic_info_map_.swap(map);
}

void CDescGate::GetTopicNames(std::vector<std::string>& topic_names_)
{
topic_names_.clear();

std::shared_lock<std::shared_timed_mutex> lock(m_topic_info_map.sync);
m_topic_info_map.map->remove_deprecated();
topic_names_.reserve(m_topic_info_map.map->size());

for (const auto& topic_info : (*m_topic_info_map.map))
{
topic_names_.emplace_back(topic_info.first);
}
}

bool CDescGate::GetTopicTypeName(const std::string& topic_name_, std::string& topic_type_)
{
if(topic_name_.empty()) return(false);

std::shared_lock<std::shared_timed_mutex> lock(m_topic_info_map_mutex);
const auto topic_info_it = m_topic_info_map.find(topic_name_);
std::shared_lock<std::shared_timed_mutex> lock(m_topic_info_map.sync);
const auto topic_info_it = m_topic_info_map.map->find(topic_name_);

if(topic_info_it == m_topic_info_map.end()) return(false);
topic_type_ = topic_info_it->second.info.type_name;
if(topic_info_it == m_topic_info_map.map->end()) return(false);
topic_type_ = (*topic_info_it).second.info.type_name;
return(true);
}

bool CDescGate::GetTopicDescription(const std::string& topic_name_, std::string& topic_desc_)
{
if(topic_name_.empty()) return(false);
if (topic_name_.empty()) return(false);

std::shared_lock<std::shared_timed_mutex> lock(m_topic_info_map_mutex);
const auto topic_info_it = m_topic_info_map.find(topic_name_);
std::shared_lock<std::shared_timed_mutex> lock(m_topic_info_map.sync);
const auto topic_info_it = m_topic_info_map.map->find(topic_name_);

if(topic_info_it == m_topic_info_map.end()) return(false);
topic_desc_ = topic_info_it->second.info.type_description;
if (topic_info_it == m_topic_info_map.map->end()) return(false);
topic_desc_ = (*topic_info_it).second.info.type_description;
return(true);
}

Expand All @@ -153,70 +181,85 @@ namespace eCAL
{
std::tuple<std::string, std::string> service_method_tuple = std::make_tuple(service_name_, method_name_);

std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map_mutex);
auto service_info_map_it = m_service_info_map.find(service_method_tuple);
if (service_info_map_it == m_service_info_map.end())
std::unique_lock<std::shared_timed_mutex> lock(m_service_info_map.sync);
m_service_info_map.map->remove_deprecated();

auto service_info_map_it = m_service_info_map.map->find(service_method_tuple);
if (service_info_map_it == m_service_info_map.map->end())
{
SServiceMethodInfoQuality service_info;
SServiceMethodInfoQuality& service_info = (*m_service_info_map.map)[service_method_tuple];
service_info.info.request_type_name = req_type_name_;
service_info.info.request_type_description = req_type_desc_;
service_info.info.response_type_name = resp_type_name_;
service_info.info.response_type_description = resp_type_desc_;
service_info.info_quality = info_quality_;

m_service_info_map[service_method_tuple] = std::move(service_info);
}
else
{
// do we need to check consistency ?
if (info_quality_ > service_info_map_it->second.info_quality)
SServiceMethodInfoQuality& service_info = (*m_service_info_map.map)[service_method_tuple];
if (info_quality_ > service_info.info_quality)
{
service_info_map_it->second.info.request_type_name = req_type_name_;
service_info_map_it->second.info.request_type_description = req_type_desc_;
service_info_map_it->second.info.response_type_name = resp_type_name_;
service_info_map_it->second.info.response_type_description = resp_type_desc_;
service_info_map_it->second.info_quality = info_quality_;
service_info.info.request_type_name = req_type_name_;
service_info.info.request_type_description = req_type_desc_;
service_info.info.response_type_name = resp_type_name_;
service_info.info.response_type_description = resp_type_desc_;
service_info.info_quality = info_quality_;
}
}
}

void CDescGate::GetServices(std::map<std::tuple<std::string, std::string>, Util::SServiceMethodInfo>& service_info_map_)
{
service_info_map_.clear();
std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map_mutex);
for (auto& service_info : m_service_info_map)
std::map<std::tuple<std::string, std::string>, Util::SServiceMethodInfo> map;

std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map.sync);
m_service_info_map.map->remove_deprecated();

for (const auto& service_info : (*m_service_info_map.map))
{
service_info_map_[service_info.first] = service_info.second.info;
map.emplace(service_info.first, service_info.second.info);
}
service_info_map_.swap(map);
}

bool CDescGate::GetServiceTypeNames(const std::string& service_name_, const std::string& method_name_, std::string& req_type_name_, std::string& resp_type_name_)
void CDescGate::GetServiceNames(std::vector<std::tuple<std::string, std::string>>& service_method_names_)
{
std::tuple<std::string, std::string> service_method_tuple = std::make_tuple(service_name_, method_name_);
service_method_names_.clear();

std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map_mutex);
auto service_info_map_it = m_service_info_map.find(service_method_tuple);
std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map.sync);
m_service_info_map.map->remove_deprecated();
service_method_names_.reserve(m_service_info_map.map->size());

if (service_info_map_it == m_service_info_map.end()) return false;
for (const auto& service_info : (*m_service_info_map.map))
{
service_method_names_.emplace_back(service_info.first);
}
}

req_type_name_ = service_info_map_it->second.info.request_type_name;
resp_type_name_ = service_info_map_it->second.info.response_type_name;
bool CDescGate::GetServiceTypeNames(const std::string& service_name_, const std::string& method_name_, std::string& req_type_name_, std::string& resp_type_name_)
{
std::tuple<std::string, std::string> service_method_tuple = std::make_tuple(service_name_, method_name_);

std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map.sync);
auto service_info_map_it = m_service_info_map.map->find(service_method_tuple);

if (service_info_map_it == m_service_info_map.map->end()) return false;
req_type_name_ = (*service_info_map_it).second.info.request_type_name;
resp_type_name_ = (*service_info_map_it).second.info.response_type_name;
return true;
}

bool CDescGate::GetServiceDescription(const std::string& service_name_, const std::string& method_name_, std::string& req_type_desc_, std::string& resp_type_desc_)
{
std::tuple<std::string, std::string> service_method_tuple = std::make_tuple(service_name_, method_name_);

std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map_mutex);
auto service_info_map_it = m_service_info_map.find(service_method_tuple);

if (service_info_map_it == m_service_info_map.end()) return false;

req_type_desc_ = service_info_map_it->second.info.request_type_description;
resp_type_desc_ = service_info_map_it->second.info.response_type_description;
std::shared_lock<std::shared_timed_mutex> lock(m_service_info_map.sync);
auto service_info_map_it = m_service_info_map.map->find(service_method_tuple);

if (service_info_map_it == m_service_info_map.map->end()) return false;
req_type_desc_ = (*service_info_map_it).second.info.request_type_description;
resp_type_desc_ = (*service_info_map_it).second.info.response_type_description;
return true;
}
};
Loading

1 comment on commit 4c6acef

@FlorianReimold
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit consists of 2 Parts:

The Behavioral change

  • Problem: The descriptor gate never invalidated deprecated topic information. Now, after 5seconds (depending on ecal.ini) of not seeing a topic, the information is removed.

The API enhancement

  • Changed functions signatures of the API functions introduced in the last commit (4cbd401)

Please sign in to comment.