Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] active entity unregistration in descgate #1520

Merged
merged 31 commits into from
Apr 24, 2024

Conversation

rex-schilasky
Copy link
Contributor

@rex-schilasky rex-schilasky commented Apr 10, 2024

Description

DescGate is currently collecting descriptions (type encodings, type names, type descriptors) for publisher, subscriber and services.

This PR:

  • extends it to collect descriptions for clients too
  • introduces active removal of descriptions triggered by unregistration events, this will lead to faster removal
  • improves quality logic by separate quality for service request and response data info types
  • utilize the pub/sub/server/client id's to not merge/overwrite description in the DescGate but keep them separately

@rex-schilasky rex-schilasky added the cherry-pick-to-NONE Don't cherry-pick these changes label Apr 10, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 26. Check the log or trigger a new build to see more.

const auto topic_info_it = m_topic_info_map.map->find(topic_name_);
if (topic_info_it != m_topic_info_map.map->end())
{
STopicInfoQuality& topic_info = (*m_topic_info_map.map)[topic_name_];
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'topic_info' of type 'STopicInfoQuality &' can be declared 'const' [misc-const-correctness]

Suggested change
STopicInfoQuality& topic_info = (*m_topic_info_map.map)[topic_name_];
STopicInfoQuality const& topic_info = (*m_topic_info_map.map)[topic_name_];

return false;
}

bool CDescGate::ApplyServiceDescription(SServiceMethodInfoMap& service_method_map_
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'ApplyServiceDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:136:

-     bool ApplyServiceDescription(SServiceMethodInfoMap& service_method_map_,
+     static bool ApplyServiceDescription(SServiceMethodInfoMap& service_method_map_,

return false;
}

bool CDescGate::RemServiceDescription(SServiceMethodInfoMap& service_method_map_, const std::string& service_name_, const std::string& service_id_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'RemServiceDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:144:

-     bool RemServiceDescription(SServiceMethodInfoMap& service_method_map_,
+     static bool RemServiceDescription(SServiceMethodInfoMap& service_method_map_,

return success;
}

void CDescGate::GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_, const SServiceMethodInfoMap& service_method_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetServices' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:148:

-     void GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_, const SServiceMethodInfoMap& service_method_map_);
+     static void GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_, const SServiceMethodInfoMap& service_method_map_);

service_info_map_.swap(map);
}

void CDescGate::GetServiceMethodNames(std::vector<std::tuple<std::string, std::string>>& method_names_, const SServiceMethodInfoMap& service_method_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetServiceMethodNames' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:149:

-     void GetServiceMethodNames(std::vector<std::tuple<std::string, std::string>>& method_names_, const SServiceMethodInfoMap& service_method_map_);
+     static void GetServiceMethodNames(std::vector<std::tuple<std::string, std::string>>& method_names_, const SServiceMethodInfoMap& service_method_map_);

@@ -125,7 +125,7 @@
}

// This test assures that find can be called on a const CExpMap and returns an CExpMap::const_iterator
TEST(core_cpp_util, ExpMap_FindConst)
TEST(core_cpp_core, ExpMap_FindConst)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_core, ExpMap_FindConst)
TEST(core_cpp_core /*unused*/, ExpMap_FindConst /*unused*/)

@@ -146,7 +146,7 @@
EXPECT_EQ(0, expmap.size());
}

TEST(core_cpp_util, ExpMap_Iterate)
TEST(core_cpp_core, ExpMap_Iterate)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_core, ExpMap_Iterate)
TEST(core_cpp_core /*unused*/, ExpMap_Iterate /*unused*/)

@@ -180,7 +180,7 @@
EXPECT_EQ(1, value);
}

TEST(core_cpp_util, ExpMap_ConstExpMapIterate)
TEST(core_cpp_core, ExpMap_ConstExpMapIterate)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_core, ExpMap_ConstExpMapIterate)
TEST(core_cpp_core /*unused*/, ExpMap_ConstExpMapIterate /*unused*/)

@@ -189,23 +189,23 @@
ConstRefIterate(expmap);
}

TEST(core_cpp_util, ExpMap_Empty)
TEST(core_cpp_core, ExpMap_Empty)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_core, ExpMap_Empty)
TEST(core_cpp_core /*unused*/, ExpMap_Empty /*unused*/)

{
eCAL::Util::CExpMap<std::string, int> expmap(std::chrono::milliseconds(200));
EXPECT_EQ(true, expmap.empty());
expmap["A"] = 1;
EXPECT_EQ(false, expmap.empty());
}

TEST(core_cpp_util, ExpMap_Size)
TEST(core_cpp_core, ExpMap_Size)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_core, ExpMap_Size)
TEST(core_cpp_core /*unused*/, ExpMap_Size /*unused*/)

@rex-schilasky rex-schilasky marked this pull request as draft April 11, 2024 15:11
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 22. Check the log or trigger a new build to see more.

*
* @param topic_info_map_ Map to store the qualified datatype information.
**/
ECAL_API void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualfied_data_type_info_map_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::Util::GetTopics' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

    ECAL_API void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualfied_data_type_info_map_);
                  ^
Additional context

ecal/core/src/ecal_util.cpp:330: the definition seen here

    void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualified_data_type_info_map_)
         ^

ecal/core/include/ecal/ecal_util.h:226: differing parameters are named here: ('qualfied_data_type_info_map_'), in definition: ('qualified_data_type_info_map_')

    ECAL_API void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualfied_data_type_info_map_);
                  ^

}

void CDescGate::GetServiceNames(std::vector<std::tuple<std::string, std::string>>& service_method_names_)
CDescGate::TopicNameIdMap CDescGate::GetTopics(const STopicInfoMap& topic_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetTopics' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:98:

-     TopicNameIdMap GetTopics(const STopicInfoMap& topic_map_);
+     static TopicNameIdMap GetTopics(const STopicInfoMap& topic_map_);

}

bool CDescGate::GetServiceDescription(const std::string& service_name_, const std::string& method_name_, std::string& req_type_desc_, std::string& resp_type_desc_)
CDescGate::ServiceMethodNameIdMap CDescGate::GetServices(const SServiceMethodInfoMap& service_method_info_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetServices' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:113:

-     ServiceMethodNameIdMap GetServices(const SServiceMethodInfoMap& service_method_info_map_);
+     static ServiceMethodNameIdMap GetServices(const SServiceMethodInfoMap& service_method_info_map_);

}

bool CDescGate::ApplyTopicDescription(const std::string& topic_name_, const SDataTypeInformation& topic_info_, const QualityFlags description_quality_)
void CDescGate::ApplyTopicDescription(STopicInfoMap& topic_info_map_,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'ApplyTopicDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:117:

-     void ApplyTopicDescription(STopicInfoMap& topic_info_map_,
+     static void ApplyTopicDescription(STopicInfoMap& topic_info_map_,

// topic type name or topic description differ and this is not logged yet
// so we log the error and update the entry one time
bool update_topic_info(false);
const auto topic_info_it = topic_info_map_.map->find(topic_info_key);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: Value stored to 'topic_info_it' during its initialization is never read [clang-analyzer-deadcode.DeadStores]

    const auto topic_info_it = topic_info_map_.map->find(topic_info_key);
               ^
Additional context

ecal/core/src/ecal_descgate.cpp:228: Value stored to 'topic_info_it' during its initialization is never read

    const auto topic_info_it = topic_info_map_.map->find(topic_info_key);
               ^

{
std::vector<SQualityServiceMethodInformation> service_method_info_vec = GetServiceTypeInformation(client_name_, method_name_, GetClients());

SServiceMethodInformation service_method_info = GetHighestQualifiedServiceMethodInformation(service_method_info_vec);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_info' of type 'SServiceMethodInformation' can be declared 'const' [misc-const-correctness]

Suggested change
SServiceMethodInformation service_method_info = GetHighestQualifiedServiceMethodInformation(service_method_info_vec);
SServiceMethodInformation const service_method_info = GetHighestQualifiedServiceMethodInformation(service_method_info_vec);

{
eCAL::Util::CExpMap<std::string, int> expmap(std::chrono::milliseconds(200));
EXPECT_EQ(0, expmap.size());
expmap["A"] = 1;
EXPECT_EQ(1, expmap.size());
}

TEST(core_cpp_util, ExpMap_Remove)
TEST(core_cpp_core, ExpMap_Remove)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_core, ExpMap_Remove)
TEST(core_cpp_core /*unused*/, ExpMap_Remove /*unused*/)

#include <ecal/ecal.h>
#include <ecal/ecal_types.h>

#include <gtest/gtest.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'gtest/gtest.h' file not found [clang-diagnostic-error]

#include <gtest/gtest.h>
         ^

Comment on lines 25 to 27
#define CMN_REGISTRATION_REFRESH 1000
#define CMN_MONITORING_TIMEOUT 5000

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: replace macro with enum [modernize-macro-to-enum]

Suggested change
#define CMN_REGISTRATION_REFRESH 1000
#define CMN_MONITORING_TIMEOUT 5000
enum {
CMN_REGISTRATION_REFRESH = 1000,
CMN_MONITORING_TIMEOUT = 5000};


#include <gtest/gtest.h>

#define CMN_REGISTRATION_REFRESH 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: macro 'CMN_REGISTRATION_REFRESH' defines an integral constant; prefer an enum instead [modernize-macro-to-enum]

#define CMN_REGISTRATION_REFRESH 1000
        ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

#include <gtest/gtest.h>

#define CMN_REGISTRATION_REFRESH 1000
#define CMN_MONITORING_TIMEOUT 5000
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: macro 'CMN_MONITORING_TIMEOUT' defines an integral constant; prefer an enum instead [modernize-macro-to-enum]

#define CMN_MONITORING_TIMEOUT   5000
        ^

#define CMN_REGISTRATION_REFRESH 1000
#define CMN_MONITORING_TIMEOUT 5000

TEST(core_cpp_util, GetClients)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'TEST' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

TEST(core_cpp_util, GetClients)
^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

* @param qualfied_data_type_info_map_ Map to store the qualified datatype information.
* Map { TopicName -> SQualityDataTypeInformation } mapping of all currently known publisher/subscriber.
**/
ECAL_API void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualfied_data_type_info_map_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::Util::GetTopics' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

    ECAL_API void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualfied_data_type_info_map_);
                  ^
Additional context

ecal/core/src/ecal_util.cpp:332: the definition seen here

    void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualified_data_type_info_map_)
         ^

ecal/core/include/ecal/ecal_util.h:228: differing parameters are named here: ('qualfied_data_type_info_map_'), in definition: ('qualified_data_type_info_map_')

    ECAL_API void GetTopics(std::unordered_map<std::string, SQualityDataTypeInformation>& qualfied_data_type_info_map_);
                  ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 21. Check the log or trigger a new build to see more.

*
* @return Highest quality data type information.
**/
ECAL_API SDataTypeInformation GetHighestQualityDataTypeInformation(const std::vector<SQualityTopicInfo>& data_type_info_vec_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::Util::GetHighestQualityDataTypeInformation' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

    ECAL_API SDataTypeInformation GetHighestQualityDataTypeInformation(const std::vector<SQualityTopicInfo>& data_type_info_vec_);
                                  ^
Additional context

ecal/core/src/ecal_util.cpp:327: the definition seen here

    SDataTypeInformation GetHighestQualityDataTypeInformation(const std::vector<SQualityTopicInfo>& quality_topic_info_vec_)
                         ^

ecal/core/include/ecal/ecal_util.h:188: differing parameters are named here: ('data_type_info_vec_'), in definition: ('quality_topic_info_vec_')

    ECAL_API SDataTypeInformation GetHighestQualityDataTypeInformation(const std::vector<SQualityTopicInfo>& data_type_info_vec_);
                                  ^

*
* @return Highest quality service method information.
**/
ECAL_API SServiceMethodInformation GetHighestQualityServiceMethodInformation(const std::vector<SQualityServiceInfo>& service_method_info_vec_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::Util::GetHighestQualityServiceMethodInformation' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

    ECAL_API SServiceMethodInformation GetHighestQualityServiceMethodInformation(const std::vector<SQualityServiceInfo>& service_method_info_vec_);
                                       ^
Additional context

ecal/core/src/ecal_util.cpp:367: the definition seen here

    SServiceMethodInformation GetHighestQualityServiceMethodInformation(const std::vector<SQualityServiceInfo>& quality_service_info_vec_)
                              ^

ecal/core/include/ecal/ecal_util.h:211: differing parameters are named here: ('service_method_info_vec_'), in definition: ('quality_service_info_vec_')

    ECAL_API SServiceMethodInformation GetHighestQualityServiceMethodInformation(const std::vector<SQualityServiceInfo>& service_method_info_vec_);
                                       ^

* @param quality_data_type_info_map_ Map to store the quality datatype information.
* Map { TopicName -> SQualityDataTypeInformation } mapping of all currently known publisher/subscriber.
**/
ECAL_API void GetTopics(std::map<std::string, SQualityTopicInfo>& quality_data_type_info_map_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::Util::GetTopics' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

    ECAL_API void GetTopics(std::map<std::string, SQualityTopicInfo>& quality_data_type_info_map_);
                  ^
Additional context

ecal/core/src/ecal_util.cpp:394: the definition seen here

    void GetTopics(std::map<std::string, SQualityTopicInfo>& quality_topic_info_map_)
         ^

ecal/core/include/ecal/ecal_util.h:227: differing parameters are named here: ('quality_data_type_info_map_'), in definition: ('quality_topic_info_map_')

    ECAL_API void GetTopics(std::map<std::string, SQualityTopicInfo>& quality_data_type_info_map_);
                  ^

@@ -149,19 +245,27 @@
ECAL_API bool GetTopicDataTypeInformation(const std::string& topic_name_, SDataTypeInformation& topic_info_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::Util::GetTopicDataTypeInformation' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

    ECAL_API bool GetTopicDataTypeInformation(const std::string& topic_name_, SDataTypeInformation& topic_info_);
                  ^
Additional context

ecal/core/src/ecal_util.cpp:430: the definition seen here

    bool GetTopicDataTypeInformation(const std::string& topic_name_, SDataTypeInformation& data_type_info_)
         ^

ecal/core/include/ecal/ecal_util.h:244: differing parameters are named here: ('topic_info_'), in definition: ('data_type_info_')

    ECAL_API bool GetTopicDataTypeInformation(const std::string& topic_name_, SDataTypeInformation& topic_info_);
                  ^

* @param service_info_map_ Map to store the service/method descriptions.
* Map { (ServiceName, MethodName) -> SServiceMethodInformation } mapping of all currently known services.
**/
ECAL_API void GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'eCAL::Util::GetServices' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

      ECAL_API void GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_);
                    ^
Additional context

ecal/core/src/ecal_util.cpp:441: the definition seen here

    void GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_method_info_map_)
         ^

ecal/core/include/ecal/ecal_util.h:252: differing parameters are named here: ('service_info_map_'), in definition: ('service_method_info_map_')

      ECAL_API void GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_);
                    ^

}

bool GetServiceDescription(const std::string& service_name_, const std::string& method_name_, std::string& req_desc_, std::string& resp_desc_)
{
if (g_descgate() == nullptr) return(false);
return(g_descgate()->GetServiceDescription(service_name_, method_name_, req_desc_, resp_desc_));
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(service_name_, method_name_, GetServices());
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_info_vec' of type 'std::vector' can be declared 'const' [misc-const-correctness]

Suggested change
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(service_name_, method_name_, GetServices());
std::vector<SQualityServiceInfo> const service_method_info_vec = GetQualityServiceInfoVec(service_name_, method_name_, GetServices());

return(g_descgate()->GetServiceDescription(service_name_, method_name_, req_desc_, resp_desc_));
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(service_name_, method_name_, GetServices());

SServiceMethodInformation service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_info' of type 'SServiceMethodInformation' can be declared 'const' [misc-const-correctness]

Suggested change
SServiceMethodInformation service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);
SServiceMethodInformation const service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);


bool GetClientTypeNames(const std::string& client_name_, const std::string& method_name_, std::string& req_type_, std::string& resp_type_)
{
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_info_vec' of type 'std::vector' can be declared 'const' [misc-const-correctness]

Suggested change
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());
std::vector<SQualityServiceInfo> const service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());

{
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());

SServiceMethodInformation service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_info' of type 'SServiceMethodInformation' can be declared 'const' [misc-const-correctness]

Suggested change
SServiceMethodInformation service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);
SServiceMethodInformation const service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);


bool GetClientDescription(const std::string& client_name_, const std::string& method_name_, std::string& req_desc_, std::string& resp_desc_)
{
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_info_vec' of type 'std::vector' can be declared 'const' [misc-const-correctness]

Suggested change
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());
std::vector<SQualityServiceInfo> const service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

{
std::vector<SQualityServiceInfo> service_method_info_vec = GetQualityServiceInfoVec(client_name_, method_name_, GetClients());

SServiceMethodInformation service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_info' of type 'SServiceMethodInformation' can be declared 'const' [misc-const-correctness]

Suggested change
SServiceMethodInformation service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);
SServiceMethodInformation const service_method_info = GetHighestQualityServiceMethodInformation(service_method_info_vec);

util test, getservices, getclients started to refactor
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 39. Check the log or trigger a new build to see more.

, const SDataTypeInformation& request_type_information_
, const SDataTypeInformation& response_type_information_
, const QualityFlags description_quality_)
void CDescGate::RemServiceDescription(SQualityServiceIdMap& service_method_info_map_, const std::string& service_name_, const std::string& service_id_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'RemServiceDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:145:

-     void RemServiceDescription(SQualityServiceIdMap& service_method_info_map_,
+     static void RemServiceDescription(SQualityServiceIdMap& service_method_info_map_,

const std::string& topic_name_,
const std::string& topic_id_,
const SDataTypeInformation& topic_info_,
const Util::DescQualityFlags topic_quality_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'topic_quality_' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const Util::DescQualityFlags topic_quality_);
Util::DescQualityFlags topic_quality_);

const std::string& method_name_,
const SDataTypeInformation& request_type_information_,
const SDataTypeInformation& response_type_information_,
const Util::DescQualityFlags request_type_quality_,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'request_type_quality_' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const Util::DescQualityFlags request_type_quality_,
Util::DescQualityFlags request_type_quality_,

const SDataTypeInformation& request_type_information_,
const SDataTypeInformation& response_type_information_,
const Util::DescQualityFlags request_type_quality_,
const Util::DescQualityFlags response_type_quality_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'response_type_quality_' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
const Util::DescQualityFlags response_type_quality_);
Util::DescQualityFlags response_type_quality_);

Comment on lines 25 to 26
#define CMN_MONITORING_TIMEOUT 5000

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: replace macro with enum [modernize-macro-to-enum]

Suggested change
#define CMN_MONITORING_TIMEOUT 5000
enum {
CMN_MONITORING_TIMEOUT = 5000};

eCAL::Finalize();
}

TEST(core_cpp_util, ClientDifferentQualities)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_util, ClientDifferentQualities)
TEST(core_cpp_util /*unused*/, ClientDifferentQualities /*unused*/)

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> client_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'client_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> client_info_map;
std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> client_info_map = 0;

EXPECT_EQ(client_info_map.size(), 1);

// check attributes
std::string req_type, resp_type;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'req_type' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

    std::string req_type, resp_type;
    ^

EXPECT_EQ(client_info_map.size(), 1);

// check attributes
std::string req_type, resp_type;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
std::string req_type, resp_type;
std::string req_type;
std::string resp_type;


// check attributes
std::string req_type, resp_type;
std::string req_desc, resp_desc;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'req_desc' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

    std::string req_desc, resp_desc;
    ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions


// check attributes
std::string req_type, resp_type;
std::string req_desc, resp_desc;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
std::string req_desc, resp_desc;
std::string req_desc;
std::string resp_desc;

#include <ecal/ecal.h>
#include <ecal/ecal_types.h>

#include <gtest/gtest.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'gtest/gtest.h' file not found [clang-diagnostic-error]

#include <gtest/gtest.h>
         ^


enum { CMN_MONITORING_TIMEOUT = 5000 };

TEST(core_cpp_util, ServiceExpiration)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_util, ServiceExpiration)
TEST(core_cpp_util /*unused*/, ServiceExpiration /*unused*/)

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> service_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> service_info_map;
std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> service_info_map = 0;

EXPECT_EQ(service_info_map.size(), 1);

// check service/method names
std::vector<std::tuple<std::string, std::string>> service_method_names;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_names' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::vector<std::tuple<std::string, std::string>> service_method_names;
std::vector<std::tuple<std::string, std::string>> service_method_names = 0;

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> service_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> service_info_map;
std::map<std::tuple<std::string, std::string>, eCAL::SServiceMethodInformation> service_info_map = 0;

EXPECT_EQ(service_info_map.size(), 1);

// check attributes
std::string req_type, resp_type;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'req_type' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

    std::string req_type, resp_type;
    ^

EXPECT_EQ(service_info_map.size(), 1);

// check attributes
std::string req_type, resp_type;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
std::string req_type, resp_type;
std::string req_type;
std::string resp_type;


// check attributes
std::string req_type, resp_type;
std::string req_desc, resp_desc;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'req_desc' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

    std::string req_desc, resp_desc;
    ^


// check attributes
std::string req_type, resp_type;
std::string req_desc, resp_desc;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
std::string req_desc, resp_desc;
std::string req_desc;
std::string resp_desc;

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

}

void CDescGate::GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_)
QualityTopicIdMap CDescGate::GetTopics(const SQualityTopicIdMap& topic_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetTopics' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:123:

-     QualityTopicIdMap   GetTopics(const SQualityTopicIdMap& topic_map_);
+     static QualityTopicIdMap   GetTopics(const SQualityTopicIdMap& topic_map_);

}

void CDescGate::GetServiceNames(std::vector<std::tuple<std::string, std::string>>& service_method_names_)
QualityServiceIdMap CDescGate::GetServices(const SQualityServiceIdMap& service_method_info_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetServices' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:124:

-     QualityServiceIdMap GetServices(const SQualityServiceIdMap& service_method_info_map_);
+     static QualityServiceIdMap GetServices(const SQualityServiceIdMap& service_method_info_map_);

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions


#include <gtest/gtest.h>

TEST(core_cpp_descgate, GetPublisher)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_descgate, GetPublisher)
TEST(core_cpp_descgate /*unused*/, GetPublisher /*unused*/)

auto pub_map = desc_gate->GetPublisher();
EXPECT_EQ(0, pub_map.size());

eCAL::Registration::Sample reg_sample;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'reg_sample' is not initialized [cppcoreguidelines-init-variables]

Suggested change
eCAL::Registration::Sample reg_sample;
eCAL::Registration::Sample reg_sample = 0;

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 21. Check the log or trigger a new build to see more.

Comment on lines +27 to +28
#define DESCGATE_EXPIRATION_MS 500

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: replace macro with enum [modernize-macro-to-enum]

Suggested change
#define DESCGATE_EXPIRATION_MS 500
enum {
DESCGATE_EXPIRATION_MS = 500};

#include <chrono>
#include <thread>

#define DESCGATE_EXPIRATION_MS 500
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: macro 'DESCGATE_EXPIRATION_MS' defines an integral constant; prefer an enum instead [modernize-macro-to-enum]

#define DESCGATE_EXPIRATION_MS 500
        ^

{
eCAL::Registration::Sample CreatePublisher(const std::string& topic_name_)
{
eCAL::Registration::Sample reg_sample;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'reg_sample' is not initialized [cppcoreguidelines-init-variables]

Suggested change
eCAL::Registration::Sample reg_sample;
eCAL::Registration::Sample reg_sample = 0;


eCAL::Registration::Sample CreateSubscriber(const std::string& topic_name_)
{
eCAL::Registration::Sample reg_sample;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'reg_sample' is not initialized [cppcoreguidelines-init-variables]

Suggested change
eCAL::Registration::Sample reg_sample;
eCAL::Registration::Sample reg_sample = 0;


eCAL::Registration::Sample CreateService(const std::string& service_name_)
{
eCAL::Registration::Sample reg_sample;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'reg_sample' is not initialized [cppcoreguidelines-init-variables]

Suggested change
eCAL::Registration::Sample reg_sample;
eCAL::Registration::Sample reg_sample = 0;

EXPECT_EQ(0, desc_gate.GetSubscriber().size());
}

TEST(core_cpp_descgate, ServiceExpiration)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_descgate, ServiceExpiration)
TEST(core_cpp_descgate /*unused*/, ServiceExpiration /*unused*/)


// apply sample 5 times, sample should not expire
auto runs(5);
while (runs--)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion]

Suggested change
while (runs--)
while ((runs--) != 0)

EXPECT_EQ(0, desc_gate.GetServices().size());
}

TEST(core_cpp_descgate, ManyService)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_descgate, ManyService)
TEST(core_cpp_descgate /*unused*/, ManyService /*unused*/)

EXPECT_EQ(0, desc_gate.GetServices().size());
}

TEST(core_cpp_descgate, ClientExpiration)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_descgate, ClientExpiration)
TEST(core_cpp_descgate /*unused*/, ClientExpiration /*unused*/)


// apply sample 5 times, sample should not expire
auto runs(5);
while (runs--)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion]

Suggested change
while (runs--)
while ((runs--) != 0)

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

ecal/core/include/ecal/ecal_util.h Show resolved Hide resolved
ecal/core/include/ecal/ecal_util.h Show resolved Hide resolved
std::string msg = "eCAL Pub/Sub description mismatch for topic ";
msg += topic_name_;
eCAL::Logging::Log(log_level_warning, msg);
void CDescGate::RemTopicDescription(SQualityTopicIdMap& topic_info_map_, const std::string& topic_name_, const std::uint64_t& topic_id_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'RemTopicDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:132:

-     void RemTopicDescription(SQualityTopicIdMap& topic_info_map_,
+     static void RemTopicDescription(SQualityTopicIdMap& topic_info_map_,

, const SDataTypeInformation& request_type_information_
, const SDataTypeInformation& response_type_information_
, const QualityFlags description_quality_)
void CDescGate::RemServiceDescription(SQualityServiceIdMap& service_method_info_map_, const std::string& service_name_, const std::uint64_t& service_id_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'RemServiceDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:145:

-     void RemServiceDescription(SQualityServiceIdMap& service_method_info_map_,
+     static void RemServiceDescription(SQualityServiceIdMap& service_method_info_map_,

EXPECT_EQ(0, desc_gate.GetClients().size());
}

TEST(core_cpp_descgate, ManyClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_descgate, ManyClient)
TEST(core_cpp_descgate /*unused*/, ManyClient /*unused*/)

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

EXPECT_EQ(0, desc_gate.GetSubscriber().size());
}

TEST(core_cpp_descgate, SubscriberQualities)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: all parameters should be named in a function [readability-named-parameter]

Suggested change
TEST(core_cpp_descgate, SubscriberQualities)
TEST(core_cpp_descgate /*unused*/, SubscriberQualities /*unused*/)

@rex-schilasky rex-schilasky marked this pull request as ready for review April 17, 2024 06:05
types introduced for TopicId and ServiceId
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

ecal/core/include/ecal/ecal_util.h Show resolved Hide resolved
ecal/core/include/ecal/ecal_util.h Show resolved Hide resolved
std::string msg = "eCAL Pub/Sub description mismatch for topic ";
msg += topic_name_;
eCAL::Logging::Log(log_level_warning, msg);
void CDescGate::RemTopicDescription(SQualityTopicIdMap& topic_info_map_, const std::string& topic_name_, const Util::TopicId& topic_id_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'RemTopicDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:132:

-     void RemTopicDescription(SQualityTopicIdMap& topic_info_map_,
+     static void RemTopicDescription(SQualityTopicIdMap& topic_info_map_,

, const SDataTypeInformation& request_type_information_
, const SDataTypeInformation& response_type_information_
, const QualityFlags description_quality_)
void CDescGate::RemServiceDescription(SQualityServiceIdMap& service_method_info_map_, const std::string& service_name_, const Util::ServiceId& service_id_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'RemServiceDescription' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:145:

-     void RemServiceDescription(SQualityServiceIdMap& service_method_info_map_,
+     static void RemServiceDescription(SQualityServiceIdMap& service_method_info_map_,

jitter of 100 ms added to util test timeouts
…bscriber -> CDescGate::GetSubscribers

static functions ApplyTopicDescription, RemTopicDescription, ApplyServiceDescription, RemServiceDescription
unique_ptr<map) -> map
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

}

void CDescGate::GetServices(std::map<std::tuple<std::string, std::string>, SServiceMethodInformation>& service_info_map_)
QualityTopicIdMap CDescGate::GetTopics(SQualityTopicIdMap& topic_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetTopics' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:117:

-     QualityTopicIdMap   GetTopics(SQualityTopicIdMap& topic_map_);
+     static QualityTopicIdMap   GetTopics(SQualityTopicIdMap& topic_map_);

}

void CDescGate::GetServiceNames(std::vector<std::tuple<std::string, std::string>>& service_method_names_)
QualityServiceIdMap CDescGate::GetServices(SQualityServiceIdMap& service_method_info_map_)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'GetServices' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/ecal_descgate.h:118:

-     QualityServiceIdMap GetServices(SQualityServiceIdMap& service_method_info_map_);
+     static QualityServiceIdMap GetServices(SQualityServiceIdMap& service_method_info_map_);

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'client_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map;
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map = 0;

EXPECT_EQ(client_info_map.size(), 1);

// check client/method names
std::set<eCAL::Util::SServiceMethod> client_method_names;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'client_method_names' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::set<eCAL::Util::SServiceMethod> client_method_names;
std::set<eCAL::Util::SServiceMethod> client_method_names = 0;

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'client_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map;
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map = 0;

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'client_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map;
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> client_info_map = 0;

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map;
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map = 0;

EXPECT_EQ(service_info_map.size(), 1);

// check service/method names
std::set<eCAL::Util::SServiceMethod> service_method_names;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_method_names' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::set<eCAL::Util::SServiceMethod> service_method_names;
std::set<eCAL::Util::SServiceMethod> service_method_names = 0;

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map;
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map = 0;

// initialize eCAL API
eCAL::Initialize(0, nullptr, "core_cpp_util");

std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'service_info_map' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map;
std::map<eCAL::Util::SServiceMethod, eCAL::SServiceMethodInformation> service_info_map = 0;

Copy link
Contributor

@KerstinKeller KerstinKeller left a comment

Choose a reason for hiding this comment

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

Sorry I have some more remarks. some might be from yesterday, and outdated, but others are from today.

ecal/core/include/ecal/ecal_util.h Outdated Show resolved Hide resolved
ecal/core/include/ecal/ecal_util.h Outdated Show resolved Hide resolved
ecal/core/include/ecal/ecal_util.h Show resolved Hide resolved
ecal/core/include/ecal/ecal_util.h Outdated Show resolved Hide resolved
ecal/core/src/cimpl/ecal_util_cimpl.cpp Show resolved Hide resolved
ecal/core/src/ecal_descgate.h Outdated Show resolved Hide resolved
ecal/core/src/ecal_util.cpp Outdated Show resolved Hide resolved
ecal/core/include/ecal/ecal_util.h Show resolved Hide resolved
ecal/core/src/ecal_util.cpp Show resolved Hide resolved
ecal/core/src/ecal_util.cpp Outdated Show resolved Hide resolved

bool operator<(const SServiceIdKey& other) const
{
return std::tie(service_name, service_id) < std::tie(other.service_name, other.service_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you "forget" the method_name in the comparison, or is this by intent?

Copy link
Contributor

@KerstinKeller KerstinKeller left a comment

Choose a reason for hiding this comment

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

Looks good, now.

@rex-schilasky rex-schilasky merged commit 1acd2ee into master Apr 24, 2024
15 checks passed
@rex-schilasky rex-schilasky deleted the feature/active-descgate-unregistration branch April 24, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-to-NONE Don't cherry-pick these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants