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

[new core alignment] integrate new core #1370

Merged
merged 27 commits into from
Feb 29, 2024

Conversation

rex-schilasky
Copy link
Contributor

@rex-schilasky rex-schilasky commented Feb 14, 2024

Description

This is the first PR to redesign the ecal core. You can find the motivation behind it and the current status here

https://github.com/eclipse-ecal/ecal-core

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 265. Check the log or trigger a new build to see more.


if (eCAL::Monitoring::GetLogging(logging_string))
if (eCAL::Logging::GetLogging(logging_string))
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]

app/mon/mon_gui/src/widgets/log_widget/log_widget.cpp:193:

- )
+  != 0)

inline CDynamicJSONSubscriber::CDynamicJSONSubscriber() :
m_created(false),
m_msg_decoder(nullptr),
m_msg_string()
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: initializer for member 'm_msg_string' is redundant [readability-redundant-member-init]

Suggested change
m_msg_string()

inline CDynamicJSONSubscriber::CDynamicJSONSubscriber(const std::string& topic_name_) :
m_created(false),
m_msg_decoder(nullptr),
m_msg_string()
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: initializer for member 'm_msg_string' is redundant [readability-redundant-member-init]

Suggested change
m_msg_string()

inline bool CDynamicJSONSubscriber::AddReceiveCallback(ReceiveCallbackT callback_)
{
if (!m_created) return false;
m_msg_callback = callback_;
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 'callback_' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]

ecal/core/include/ecal/msg/protobuf/dynamic_json_subscriber.h:35:

+ #include <utility>
Suggested change
m_msg_callback = callback_;
m_msg_callback = std::move(callback_);

options.always_print_primitive_fields = true;

std::string binary_input;
binary_input.assign((char*)data_->buf, static_cast<size_t>(data_->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: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

        binary_input.assign((char*)data_->buf, static_cast<size_t>(data_->size));
                            ^


std::string Get() { return (m_ss.str()); }

void AddError(
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: annotate this function with 'override' or (rarely) 'final' [cppcoreguidelines-explicit-virtual-functions]

ecal/core/include/ecal/msg/protobuf/ecal_proto_dyn.h:226:

-       )
+       ) override

Add(filename, element_name, descriptor, location, "ERROR: " + message);
}

void AddWarning(
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: annotate this function with 'override' or (rarely) 'final' [cppcoreguidelines-explicit-virtual-functions]

ecal/core/include/ecal/msg/protobuf/ecal_proto_dyn.h:237:

-       )
+       ) override


// create message object
google::protobuf::Message* proto_msg = GetProtoMessageFromDescriptorSet(pset, msg_type_, error_s_);
if (!proto_msg)
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 'google::protobuf::Message *' -> bool [readability-implicit-bool-conversion]

Suggested change
if (!proto_msg)
if (proto_msg == nullptr)

std::stringstream ss;
ss << fs.rdbuf();

std::string proto_str = ss.str();
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 'proto_str' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

Suggested change
std::string proto_str = ss.str();
std::string const proto_str = ss.str();

return (GetFileDescriptorFromString(proto_str, file_desc_proto_, error_s_));
}

inline bool CProtoDynDecoder::GetFileDescriptorFromString(const std::string& proto_string_, google::protobuf::FileDescriptorProto* file_desc_proto_, std::string& error_s_)
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 'GetFileDescriptorFromString' can be made static [readability-convert-member-functions-to-static]

ecal/core/include/ecal/msg/protobuf/ecal_proto_dyn.h:172:

-       bool GetFileDescriptorFromString(const std::string& proto_string_, google::protobuf::FileDescriptorProto* file_desc_proto_, std::string& error_s_);
+       static bool GetFileDescriptorFromString(const std::string& proto_string_, google::protobuf::FileDescriptorProto* file_desc_proto_, std::string& error_s_);

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 244. Check the log or trigger a new build to see more.

auto count = descriptor->field_count();
for (int i = 0; i < count; ++i)
{
auto field = descriptor->field(i);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'auto field' can be declared as 'const auto *field' [readability-qualified-auto]

Suggested change
auto field = descriptor->field(i);
const auto *field = descriptor->field(i);

@@ -25,8 +25,9 @@
#pragma once

#include <ecal/ecal_server.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: 'ecal/ecal_server.h' file not found [clang-diagnostic-error]

#include <ecal/ecal_server.h>
         ^

**/
CMsgPublisher(const std::string& topic_name_, const SDataTypeInformation& topic_info_) : CPublisher(topic_name_, topic_info_)
**/
CMsgPublisher(const std::string& topic_name_, const struct SDataTypeInformation& topic_info_) : CPublisher(topic_name_, 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: constructor does not initialize these fields: m_buffer [cppcoreguidelines-pro-type-member-init]

    CMsgPublisher(const std::string& topic_name_, const struct SDataTypeInformation& topic_info_) : CPublisher(topic_name_, topic_info_)
    ^

**/
CMsgPublisher(const std::string& topic_name_) : CMsgPublisher(topic_name_, GetDataTypeInformation())
**/
explicit CMsgPublisher(const std::string& topic_name_) : CMsgPublisher(topic_name_, GetDataTypeInformation())
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: constructor does not initialize these fields: m_buffer [cppcoreguidelines-pro-type-member-init]

    explicit CMsgPublisher(const std::string& topic_name_) : CMsgPublisher(topic_name_, GetDataTypeInformation())
    ^

@@ -24,6 +24,7 @@

#pragma once

#include <cstddef>
#include <ecal/ecal_deprecate.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: 'ecal/ecal_deprecate.h' file not found [clang-diagnostic-error]

#include <ecal/ecal_deprecate.h>
         ^

@@ -168,23 +177,25 @@
{
SServerMon()
{
rclock = 0;
pid = 0;
rclock = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'rclock' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
rclock = 0;

ecal/core/include/ecal/types/monitoring.h:186:

-       int32_t                  rclock;                          //<! registration clock    
+       int32_t                  rclock{0};                          //<! registration clock    

rclock = 0;
pid = 0;
rclock = 0;
pid = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'pid' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
pid = 0;

ecal/core/include/ecal/types/monitoring.h:190:

-       int32_t                  pid;                             //<! process id
+       int32_t                  pid{0};                             //<! process id

pid = 0;
rclock = 0;
pid = 0;
version = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'version' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
version = 0;

ecal/core/include/ecal/types/monitoring.h:195:

-       uint32_t                 version;                         //<! service protocol version
+       uint32_t                 version{0};                         //<! service protocol version

@@ -193,23 +204,26 @@
{
SClientMon()
{
rclock = 0;
pid = 0;
rclock = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'rclock' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
rclock = 0;

ecal/core/include/ecal/types/monitoring.h:211:

-       int32_t      rclock;                                      //<! registration clock
+       int32_t      rclock{0};                                      //<! registration clock

rclock = 0;
pid = 0;
rclock = 0;
pid = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'pid' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
pid = 0;

ecal/core/include/ecal/types/monitoring.h:215:

-       int32_t      pid;                                         //<! process id
+       int32_t      pid{0};                                         //<! process id

@rex-schilasky rex-schilasky marked this pull request as draft February 14, 2024 15:19
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 224. Check the log or trigger a new build to see more.

pid = 0;
rclock = 0;
pid = 0;
version = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'version' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
version = 0;

ecal/core/include/ecal/types/monitoring.h:220:

-       uint32_t     version;                                     //<! client protocol version
+       uint32_t     version{0};                                     //<! client protocol version

if(ret)
{
std::string topic_type_s = topic_info.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_type_s' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

Suggested change
std::string topic_type_s = topic_info.name;
std::string const topic_type_s = topic_info.name;

bool ret = eCAL::Util::GetTopicDataTypeInformation(topic_name_, topic_info);
if (ret)
{
std::string topic_encoding_s = topic_info.encoding;
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_encoding_s' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

Suggested change
std::string topic_encoding_s = topic_info.encoding;
std::string const topic_encoding_s = topic_info.encoding;

if(ret)
{
std::string topic_desc_s = topic_info.descriptor;
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_desc_s' of type 'std::string' (aka 'basic_string') can be declared 'const' [misc-const-correctness]

Suggested change
std::string topic_desc_s = topic_info.descriptor;
std::string const topic_desc_s = topic_info.descriptor;

if (!pub->Create(topic_name_, topic_type_))
eCAL::SDataTypeInformation topic_info;
topic_info.name = topic_type_;
auto* pub = new eCAL::CPublisher;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: initializing non-owner 'eCAL::CPublisher *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

  auto* pub = new eCAL::CPublisher;
  ^

@@ -738,11 +634,10 @@
/****************************************/
ECAL_API bool client_destroy(ECAL_HANDLE handle_)
{
eCAL::CServiceClient* client = static_cast<eCAL::CServiceClient*>(handle_);
auto* client = static_cast<eCAL::CServiceClient*>(handle_);
if (client != nullptr)
{
delete client;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete client;
    ^
Additional context

ecal/core/src/ecal_clang.cpp:636: variable declared here

  auto* client = static_cast<eCAL::CServiceClient*>(handle_);
  ^

#define NET_UDP_MULTICAST_PORT_SAMPLE_OFF 2
#define NET_UDP_MULTICAST_PORT_LOG_OFF 4
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 'NET_UDP_MULTICAST_PORT_LOG_OFF' defines an integral constant; prefer an enum instead [modernize-macro-to-enum]

#define NET_UDP_MULTICAST_PORT_LOG_OFF             4
        ^

#endif
#if ECAL_CORE_SUBSCRIBER
sstream << "------------------------- SUBSCRIPTION LAYER DEFAULTS ------------" << std::endl;
sstream << "Layer Mode UDP MC : " << LayerMode(Config::IsUdpMulticastRecEnabled()) << std::endl;
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 bool -> 'int' [readability-implicit-bool-conversion]

Suggested change
sstream << "Layer Mode UDP MC : " << LayerMode(Config::IsUdpMulticastRecEnabled()) << std::endl;
sstream << "Layer Mode UDP MC : " << LayerMode(static_cast<int>(Config::IsUdpMulticastRecEnabled())) << std::endl;

@@ -17,10 +17,11 @@
* ========================= eCAL LICENSE =================================
*/

#include <errno.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: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead [modernize-deprecated-headers]

Suggested change
#include <errno.h>
#include <cerrno>

// take monitoring snapshot
static eCAL::pb::Monitoring GetMonitoring()
static Monitoring::SMonitoring GetMonitoring()
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 'GetMonitoring' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]

    static Monitoring::SMonitoring GetMonitoring()
                                   ^

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 204. Check the log or trigger a new build to see more.

{
if (!eCAL::IsInitialized(eCAL::Init::Monitoring))
if (!IsInitialized(Init::Monitoring))
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
if (!IsInitialized(Init::Monitoring))
if (IsInitialized(Init::Monitoring) == 0)

@@ -35,6 +35,7 @@
#define WIN32_LEAN_AND_CLEAN
#endif // !defined(WIN32_LEAN_AND_CLEAN)

#include <winsock2.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: 'winsock2.h' file not found [clang-diagnostic-error]

#include <winsock2.h>
         ^

{
return(eCAL_Util_GetTopicTypeName(topic_name_, topic_type_, topic_type_len_));
if (!topic_name_) return(0);
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 'const char *' -> bool [readability-implicit-bool-conversion]

Suggested change
if (!topic_name_) return(0);
if (topic_name_ == nullptr) return(0);

{
return(eCAL_Util_GetTopicTypeName(topic_name_, topic_type_, topic_type_len_));
if (!topic_name_) return(0);
if (!topic_encoding_) return(0);
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 'void *' -> bool [readability-implicit-bool-conversion]

Suggested change
if (!topic_encoding_) return(0);
if (topic_encoding_ == nullptr) return(0);

callback_(topic_name_, &data, par_);
}

extern "C"
{
ECALC_API ECAL_HANDLE eCAL_Pub_New()
{
eCAL::CPublisher* pub = new eCAL::CPublisher;
auto* pub = new eCAL::CPublisher;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: initializing non-owner 'eCAL::CPublisher *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

    auto* pub = new eCAL::CPublisher;
    ^

@@ -1072,35 +834,35 @@
return ret_state;
}

static std::recursive_mutex g_server_event_callback_mtx;
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 'g_server_event_callback_mtx' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

  static std::recursive_mutex g_server_event_callback_mtx;
                              ^

@@ -1072,35 +834,35 @@
return ret_state;
}

static std::recursive_mutex g_server_event_callback_mtx;
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 'g_server_event_callback_mtx' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]

  static std::recursive_mutex g_server_event_callback_mtx;
                              ^

if (handle_ == NULL) return(0);
eCAL::CServiceServer* server = static_cast<eCAL::CServiceServer*>(handle_);
if (handle_ == nullptr) return(0);
auto* server = static_cast<eCAL::CServiceServer*>(handle_);
delete server;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete server;
    ^
Additional context

ecal/core/src/ecalc.cpp:856: variable declared here

    auto* server = static_cast<eCAL::CServiceServer*>(handle_);
    ^

service_response.response_len = static_cast<int>(service_response_.response.size());
callback_(&service_response, par_);
}

static std::recursive_mutex g_client_event_callback_mtx;
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 'g_client_event_callback_mtx' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

  static std::recursive_mutex g_client_event_callback_mtx;
                              ^

service_response.response_len = static_cast<int>(service_response_.response.size());
callback_(&service_response, par_);
}

static std::recursive_mutex g_client_event_callback_mtx;
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 'g_client_event_callback_mtx' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]

  static std::recursive_mutex g_client_event_callback_mtx;
                              ^

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 184. Check the log or trigger a new build to see more.

if(handle_ == NULL) return(0);
eCAL::CServiceClient* client = static_cast<eCAL::CServiceClient*>(handle_);
if (handle_ == nullptr) return(0);
auto* client = static_cast<eCAL::CServiceClient*>(handle_);
delete client;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete client;
    ^
Additional context

ecal/core/src/ecalc.cpp:962: variable declared here

    auto* client = static_cast<eCAL::CServiceClient*>(handle_);
    ^

@@ -40,7 +42,7 @@ namespace eCAL
const std::lock_guard<std::mutex> lock(m_memfile_map_mtx);

// erase memory files from memory map
for (MemFileMapT::iterator iter = m_memfile_map.begin(); iter != m_memfile_map.end(); ++iter)
for (auto iter = m_memfile_map.begin(); iter != m_memfile_map.end(); ++iter)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use range-based for loop instead [modernize-loop-convert]

Suggested change
for (auto iter = m_memfile_map.begin(); iter != m_memfile_map.end(); ++iter)
for (auto & iter : m_memfile_map)

ecal/core/src/io/shm/ecal_memfile_db.cpp:46:

-       auto& memfile_info = iter->second;
+       auto& memfile_info = iter.second;

#include <sys/types.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <errno.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: inclusion of deprecated C++ header 'errno.h'; consider using 'cerrno' instead [modernize-deprecated-headers]

Suggested change
#include <errno.h>
#include <cerrno>

@@ -62,7 +62,7 @@ namespace eCAL
flProtect = PAGE_READONLY;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use of undeclared identifier 'PAGE_READONLY' [clang-diagnostic-error]

            flProtect = PAGE_READONLY;
                        ^

@@ -62,7 +62,7 @@
flProtect = PAGE_READONLY;
}
mem_file_info_.map_region = CreateFileMapping(INVALID_HANDLE_VALUE, nullptr, flProtect, 0, (DWORD)mem_file_info_.size, mem_file_info_.name.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use of undeclared identifier 'INVALID_HANDLE_VALUE' [clang-diagnostic-error]

          mem_file_info_.map_region = CreateFileMapping(INVALID_HANDLE_VALUE, nullptr, flProtect, 0, (DWORD)mem_file_info_.size, mem_file_info_.name.c_str());
                                                        ^

}
}

void CMonitoringImpl::MonitorTopics(STopicMonMap& map_, eCAL::pb::Monitoring& monitoring_, const std::string& direction_)
void CMonitoringImpl::MonitorTopics(STopicMonMap& map_, Monitoring::SMonitoring& monitoring_, const std::string& direction_)
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 'MonitorTopics' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/monitoring/ecal_monitoring_impl.h:146:

-     void MonitorTopics(STopicMonMap& map_, Monitoring::SMonitoring& monitoring_, const std::string& direction_);
+     static void MonitorTopics(STopicMonMap& map_, Monitoring::SMonitoring& monitoring_, const std::string& direction_);

bool ApplyTopicToDescGate(const std::string& topic_name_, const SDataTypeInformation& topic_info_);

static std::atomic<bool> m_created;
static std::atomic<bool> m_created;
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 'm_created' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

    static std::atomic<bool> m_created;
                             ^

@@ -104,9 +108,9 @@ namespace eCAL
return(g_subgate()->HasSample(sample_name_));
}

bool CUDPReaderLayer::ApplySample(const eCAL::pb::Sample& ecal_sample_)
bool CUDPReaderLayer::ApplySample(const char* serialized_sample_data_, size_t serialized_sample_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: method 'ApplySample' can be made static [readability-convert-member-functions-to-static]

ecal/core/src/readwrite/udp/ecal_reader_udp_mc.h:53:

-     bool ApplySample(const char* serialized_sample_data_, size_t serialized_sample_size_);
+     static bool ApplySample(const char* serialized_sample_data_, size_t serialized_sample_size_);


bool return_value {true};
const std::lock_guard<std::mutex> lock(m_topics_map_sync);
for(SampleMapT::const_iterator iter = m_topics_map.begin(); iter != m_topics_map.end(); ++iter)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use range-based for loop instead [modernize-loop-convert]

Suggested change
for(SampleMapT::const_iterator iter = m_topics_map.begin(); iter != m_topics_map.end(); ++iter)
for(const auto & iter : m_topics_map)

ecal/core/src/registration/ecal_registration_provider.cpp:407:

-       const std::string topic_name(iter->second.topic.tname);
-       const bool topic_is_a_publisher(iter->second.cmd_type == eCAL::bct_reg_publisher);
+       const std::string topic_name(iter.second.topic.tname);
+       const bool topic_is_a_publisher(iter.second.cmd_type == eCAL::bct_reg_publisher);

ecal/core/src/registration/ecal_registration_provider.cpp:411:

-       const auto& topic_datatype = iter->second.topic.tdatatype;
+       const auto& topic_datatype = iter.second.topic.tdatatype;

ecal/core/src/registration/ecal_registration_provider.cpp:421:

-       return_value &= ApplySample(iter->second.topic.tname, iter->second);
+       return_value &= ApplySample(iter.second.topic.tname, iter.second);


bool return_value {true};
const std::lock_guard<std::mutex> lock(m_server_map_sync);
for(SampleMapT::const_iterator iter = m_server_map.begin(); iter != m_server_map.end(); ++iter)
for (SampleMapT::const_iterator iter = m_server_map.begin(); iter != m_server_map.end(); ++iter)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use range-based for loop instead [modernize-loop-convert]

Suggested change
for (SampleMapT::const_iterator iter = m_server_map.begin(); iter != m_server_map.end(); ++iter)
for (const auto & iter : m_server_map)

ecal/core/src/registration/ecal_registration_provider.cpp:439:

-       const auto& ecal_sample_service = iter->second.service;
+       const auto& ecal_sample_service = iter.second.service;

ecal/core/src/registration/ecal_registration_provider.cpp:456:

-       return_value &= ApplySample(iter->second.service.sname, iter->second);
+       return_value &= ApplySample(iter.second.service.sname, iter.second);

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 172. Check the log or trigger a new build to see more.

@@ -168,23 +172,25 @@ namespace eCAL
{
SServerMon()
{
rclock = 0;
pid = 0;
rclock = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'rclock' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
rclock = 0;

ecal/core/include/ecal/types/monitoring.h:181:

-       int32_t                  rclock;                          //<! registration clock    
+       int32_t                  rclock{0};                          //<! registration clock    

rclock = 0;
pid = 0;
rclock = 0;
pid = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'pid' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
pid = 0;

ecal/core/include/ecal/types/monitoring.h:185:

-       int32_t                  pid;                             //<! process id
+       int32_t                  pid{0};                             //<! process id

pid = 0;
rclock = 0;
pid = 0;
version = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'version' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
version = 0;

ecal/core/include/ecal/types/monitoring.h:190:

-       uint32_t                 version;                         //<! service protocol version
+       uint32_t                 version{0};                         //<! service protocol version

@@ -193,23 +199,26 @@
{
SClientMon()
{
rclock = 0;
pid = 0;
rclock = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'rclock' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
rclock = 0;

ecal/core/include/ecal/types/monitoring.h:206:

-       int32_t      rclock;                                      //<! registration clock
+       int32_t      rclock{0};                                      //<! registration clock

rclock = 0;
pid = 0;
rclock = 0;
pid = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'pid' should be initialized in an in-class default member initializer [cppcoreguidelines-prefer-member-initializer]

Suggested change
pid = 0;

ecal/core/include/ecal/types/monitoring.h:210:

-       int32_t      pid;                                         //<! process id
+       int32_t      pid{0};                                         //<! process id

///////////////////////////////////////////////
// prepare sample for decoding
///////////////////////////////////////////////
pb_log_message_list.log_messages.funcs.decode = &decode_log_message_list_field;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]

    pb_log_message_list.log_messages.funcs.decode = &decode_log_message_list_field;
                                           ^

// decode it
///////////////////////////////////////////////
pb_istream_t pb_istream;
pb_istream = pb_istream_from_buffer((pb_byte_t*)data_, 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: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    pb_istream = pb_istream_from_buffer((pb_byte_t*)data_, size_);
                                        ^

if (arg == nullptr) return false;
if (*arg == nullptr) return false;

auto* process_vec = (std::vector<eCAL::Monitoring::SProcessMon>*)(*arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    auto* process_vec = (std::vector<eCAL::Monitoring::SProcessMon>*)(*arg);
                        ^

if (arg == nullptr) return false;
if (*arg == nullptr) return false;

auto* layer_vec = (std::vector<eCAL::Monitoring::TLayer>*)(*arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    auto* layer_vec = (std::vector<eCAL::Monitoring::TLayer>*)(*arg);
                      ^


void encode_mon_registration_layer(pb_callback_t& pb_callback, const std::vector<eCAL::Monitoring::TLayer>& layer_vec)
{
pb_callback.funcs.encode = &encode_mon_registration_layer_field;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]

    pb_callback.funcs.encode = &encode_mon_registration_layer_field;
                      ^

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 157. Check the log or trigger a new build to see more.

@@ -35,7 +35,7 @@ namespace eCAL
{
public:
DynamicReflectionException(const std::string& message) : message_(message) {}
virtual const char* what() const throw() { return message_.c_str(); }
virtual const char* what() const noexcept { return message_.c_str(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]

Suggested change
virtual const char* what() const noexcept { return message_.c_str(); }
const char* what() const noexcept override { return message_.c_str(); }

google::protobuf::DynamicMessageFactory m_message_factory;
};

class ParserErrorCollector : public google::protobuf::io::ErrorCollector
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: class 'ParserErrorCollector' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

    class ParserErrorCollector : public google::protobuf::io::ErrorCollector
          ^

{
public:
DescriptorErrorCollector() = default;
~DescriptorErrorCollector() override {}
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use '= default' to define a trivial destructor [modernize-use-equals-default]

Suggested change
~DescriptorErrorCollector() override {}
~DescriptorErrorCollector() override = default;

@@ -147,7 +149,8 @@ namespace eCAL
std::string output_type_name = method_descriptor->output_type()->name();

// get message type descriptors
std::string input_type_desc, output_type_desc;
std::string input_type_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 'input_type_desc' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::string input_type_desc;
std::string input_type_desc = 0;

@@ -147,7 +149,8 @@
std::string output_type_name = method_descriptor->output_type()->name();

// get message type descriptors
std::string input_type_desc, output_type_desc;
std::string input_type_desc;
std::string output_type_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 'output_type_desc' is not initialized [cppcoreguidelines-init-variables]

Suggested change
std::string output_type_desc;
std::string output_type_desc = 0;

AssignValues(pb_process, process);

// add sample to vector
auto* processes_vec = (std::vector<eCAL::Monitoring::SProcessMon>*)(*arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    auto* processes_vec = (std::vector<eCAL::Monitoring::SProcessMon>*)(*arg);
                          ^

layer.confirmed = pb_layer.confirmed;

// add layer
auto tgt_vector = (std::vector<eCAL::Monitoring::TLayer>*)(*arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 'auto tgt_vector' can be declared as 'auto *tgt_vector' [readability-qualified-auto]

Suggested change
auto tgt_vector = (std::vector<eCAL::Monitoring::TLayer>*)(*arg);
auto *tgt_vector = (std::vector<eCAL::Monitoring::TLayer>*)(*arg);

layer.confirmed = pb_layer.confirmed;

// add layer
auto tgt_vector = (std::vector<eCAL::Monitoring::TLayer>*)(*arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    auto tgt_vector = (std::vector<eCAL::Monitoring::TLayer>*)(*arg);
                      ^


void decode_mon_registration_layer(pb_callback_t& pb_callback, std::vector<eCAL::Monitoring::TLayer>& layer_vec)
{
pb_callback.funcs.decode = &decode_mon_registration_layer_field;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]

    pb_callback.funcs.decode = &decode_mon_registration_layer_field;
                      ^

AssignValues(pb_topic, topic);

// add sample to vector
auto* monitoring = (eCAL::Monitoring::SMonitoring*)(*arg);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    auto* monitoring = (eCAL::Monitoring::SMonitoring*)(*arg);
                       ^

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 126. Check the log or trigger a new build to see more.

// prepare request for encoding
///////////////////////////////////////////////
eCAL_pb_Request pb_request = eCAL_pb_Request_init_default;
size_t target_size = RequestStruct2PbRequest(request_, pb_request);
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 'target_size' of type 'size_t' (aka 'unsigned long') can be declared 'const' [misc-const-correctness]

Suggested change
size_t target_size = RequestStruct2PbRequest(request_, pb_request);
size_t const target_size = RequestStruct2PbRequest(request_, pb_request);

// prepare response for encoding
///////////////////////////////////////////////
eCAL_pb_Response pb_response = eCAL_pb_Response_init_default;
size_t target_size = ResponseStruct2PbResponse(response_, pb_response);
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 'target_size' of type 'size_t' (aka 'unsigned long') can be declared 'const' [misc-const-correctness]

Suggested change
size_t target_size = ResponseStruct2PbResponse(response_, pb_response);
size_t const target_size = ResponseStruct2PbResponse(response_, pb_response);

#endif

/* Enum definitions */
typedef enum _eCAL_pb_eCmdType {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_eCmdType', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
typedef enum _eCAL_pb_eCmdType {
typedef enum eCAL_pb_eCmdType {

} eCAL_pb_eCmdType;

/* Struct definitions */
typedef struct _eCAL_pb_Content {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_Content', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
typedef struct _eCAL_pb_Content {
typedef struct eCAL_pb_Content {

int64_t hash; /* unique hash for that sample */
} eCAL_pb_Content;

typedef struct _eCAL_pb_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: declaration uses identifier '_eCAL_pb_Sample', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
typedef struct _eCAL_pb_Sample {
typedef struct eCAL_pb_Sample {

pb_callback_t memory_file_list; /* list of memory file names */
} eCAL_pb_LayerParShm;

typedef struct _eCAL_pb_LayerParTcp {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_LayerParTcp', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
typedef struct _eCAL_pb_LayerParTcp {
typedef struct eCAL_pb_LayerParTcp {

int32_t port; /* tcp writers port number */
} eCAL_pb_LayerParTcp;

typedef struct _eCAL_pb_ConnnectionPar {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_ConnnectionPar', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
typedef struct _eCAL_pb_ConnnectionPar {
typedef struct eCAL_pb_ConnnectionPar {

eCAL_pb_LayerParTcp layer_par_tcp; /* parameter for ecal tcp */
} eCAL_pb_ConnnectionPar;

typedef struct _eCAL_pb_TLayer {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_TLayer', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
typedef struct _eCAL_pb_TLayer {
typedef struct eCAL_pb_TLayer {

#endif

/* Helper constants for enums */
#define _eCAL_pb_eTLayerType_MIN eCAL_pb_eTLayerType_tl_none
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_eTLayerType_MIN', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
#define _eCAL_pb_eTLayerType_MIN eCAL_pb_eTLayerType_tl_none
#define eCAL_pb_eTLayerType_MIN eCAL_pb_eTLayerType_tl_none


/* Helper constants for enums */
#define _eCAL_pb_eTLayerType_MIN eCAL_pb_eTLayerType_tl_none
#define _eCAL_pb_eTLayerType_MAX eCAL_pb_eTLayerType_tl_all
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_eTLayerType_MAX', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
#define _eCAL_pb_eTLayerType_MAX eCAL_pb_eTLayerType_tl_all
#define eCAL_pb_eTLayerType_MAX eCAL_pb_eTLayerType_tl_all

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 110. Check the log or trigger a new build to see more.

#endif
#if ECAL_CORE_SUBSCRIBER
sstream << "------------------------- SUBSCRIPTION LAYER DEFAULTS ------------" << '\n';
sstream << "Layer Mode UDP MC : " << LayerMode(Config::IsUdpMulticastRecEnabled()) << '\n';
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 bool -> 'int' [readability-implicit-bool-conversion]

Suggested change
sstream << "Layer Mode UDP MC : " << LayerMode(Config::IsUdpMulticastRecEnabled()) << '\n';
sstream << "Layer Mode UDP MC : " << LayerMode(static_cast<int>(Config::IsUdpMulticastRecEnabled())) << '\n';


bool return_value {true};
const std::lock_guard<std::mutex> lock(m_topics_map_sync);
for(SampleMapT::const_iterator iter = m_topics_map.begin(); iter != m_topics_map.end(); ++iter)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use range-based for loop instead [modernize-loop-convert]

Suggested change
for(SampleMapT::const_iterator iter = m_topics_map.begin(); iter != m_topics_map.end(); ++iter)
for(const auto & iter : m_topics_map)

ecal/core/src/registration/ecal_registration_provider.cpp:402:

-       const std::string topic_name(iter->second.topic.tname);
-       const bool topic_is_a_publisher(iter->second.cmd_type == eCAL::bct_reg_publisher);
+       const std::string topic_name(iter.second.topic.tname);
+       const bool topic_is_a_publisher(iter.second.cmd_type == eCAL::bct_reg_publisher);

ecal/core/src/registration/ecal_registration_provider.cpp:406:

-       const auto& topic_datatype = iter->second.topic.tdatatype;
+       const auto& topic_datatype = iter.second.topic.tdatatype;

ecal/core/src/registration/ecal_registration_provider.cpp:416:

-       return_value &= ApplySample(iter->second.topic.tname, iter->second);
+       return_value &= ApplySample(iter.second.topic.tname, iter.second);


bool return_value {true};
const std::lock_guard<std::mutex> lock(m_server_map_sync);
for(SampleMapT::const_iterator iter = m_server_map.begin(); iter != m_server_map.end(); ++iter)
for (SampleMapT::const_iterator iter = m_server_map.begin(); iter != m_server_map.end(); ++iter)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use range-based for loop instead [modernize-loop-convert]

Suggested change
for (SampleMapT::const_iterator iter = m_server_map.begin(); iter != m_server_map.end(); ++iter)
for (const auto & iter : m_server_map)

ecal/core/src/registration/ecal_registration_provider.cpp:434:

-       const auto& ecal_sample_service = iter->second.service;
+       const auto& ecal_sample_service = iter.second.service;

ecal/core/src/registration/ecal_registration_provider.cpp:451:

-       return_value &= ApplySample(iter->second.service.sname, iter->second);
+       return_value &= ApplySample(iter.second.service.sname, iter.second);

/* Helper constants for enums */
#define _eCAL_pb_eTLayerType_MIN eCAL_pb_eTLayerType_tl_none
#define _eCAL_pb_eTLayerType_MAX eCAL_pb_eTLayerType_tl_all
#define _eCAL_pb_eTLayerType_ARRAYSIZE ((eCAL_pb_eTLayerType)(eCAL_pb_eTLayerType_tl_all+1))
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_eTLayerType_ARRAYSIZE', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
#define _eCAL_pb_eTLayerType_ARRAYSIZE ((eCAL_pb_eTLayerType)(eCAL_pb_eTLayerType_tl_all+1))
#define eCAL_pb_eTLayerType_ARRAYSIZE ((eCAL_pb_eTLayerType)(eCAL_pb_eTLayerType_tl_all+1))

#endif

/* Struct definitions */
typedef struct _eCAL_pb_LogMessage {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: declaration uses identifier '_eCAL_pb_LogMessage', which is reserved in the global namespace [bugprone-reserved-identifier]

Suggested change
typedef struct _eCAL_pb_LogMessage {
typedef struct eCAL_pb_LogMessage {

#define PB_DS_PB_HTYPE_REPEATED(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PB_HTYPE_FIXARRAY(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_REQUIRED(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_SINGULAR(structname, fieldname) pb_membersize(structname, fieldname[0])
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 argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change
#define PB_DS_PTR_PB_HTYPE_SINGULAR(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_SINGULAR(structname, fieldname) pb_membersize(structname, (fieldname)[0])

#define PB_DS_PB_HTYPE_FIXARRAY(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_REQUIRED(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_SINGULAR(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_OPTIONAL(structname, fieldname) pb_membersize(structname, fieldname[0])
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 argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change
#define PB_DS_PTR_PB_HTYPE_OPTIONAL(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_OPTIONAL(structname, fieldname) pb_membersize(structname, (fieldname)[0])

#define PB_DS_PTR_PB_HTYPE_SINGULAR(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_OPTIONAL(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_ONEOF(structname, fieldname) pb_membersize(structname, PB_ONEOF_NAME(FULL, fieldname)[0])
#define PB_DS_PTR_PB_HTYPE_REPEATED(structname, fieldname) pb_membersize(structname, fieldname[0])
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 argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change
#define PB_DS_PTR_PB_HTYPE_REPEATED(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_REPEATED(structname, fieldname) pb_membersize(structname, (fieldname)[0])

#define PB_DS_PTR_PB_HTYPE_OPTIONAL(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_ONEOF(structname, fieldname) pb_membersize(structname, PB_ONEOF_NAME(FULL, fieldname)[0])
#define PB_DS_PTR_PB_HTYPE_REPEATED(structname, fieldname) pb_membersize(structname, fieldname[0])
#define PB_DS_PTR_PB_HTYPE_FIXARRAY(structname, fieldname) pb_membersize(structname, fieldname[0][0])
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 argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change
#define PB_DS_PTR_PB_HTYPE_FIXARRAY(structname, fieldname) pb_membersize(structname, fieldname[0][0])
#define PB_DS_PTR_PB_HTYPE_FIXARRAY(structname, fieldname) pb_membersize(structname, (fieldname)[0][0])

* you can increase the descriptor width by defining PB_FIELDINFO_WIDTH or by setting
* descriptorsize option in .options file.
*/
#define PB_FITS(value,bits) ((uint32_t)(value) < ((uint32_t)1<<bits))
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 argument should be enclosed in parentheses [bugprone-macro-parentheses]

Suggested change
#define PB_FITS(value,bits) ((uint32_t)(value) < ((uint32_t)1<<bits))
#define PB_FITS(value,bits) ((uint32_t)(value) < ((uint32_t)1<<(bits)))

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

// that, but we don't want their values, any more.
*block_modifying_responses = true;

return responses;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: constness of 'responses' prevents automatic move [performance-no-automatic-move]

      return responses;
             ^

std::shared_ptr<std::vector<std::pair<bool, eCAL::SServiceResponse>>> CallBlocking(const std::string& method_name_, const std::string& request_, std::chrono::nanoseconds timeout_);

static void fromSerializedProtobuf(const std::string& response_pb_, eCAL::SServiceResponse& response_);
static void fromStruct(const Service::Response& response_struct_, eCAL::SServiceResponse& response_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use of undeclared identifier 'Service'; did you mean 'service'? [clang-diagnostic-error]

Suggested change
static void fromStruct(const Service::Response& response_struct_, eCAL::SServiceResponse& response_);
static void fromStruct(const service::Response& response_struct_, eCAL::SServiceResponse& response_);
Additional context

ecal/service/ecal_service/include/ecal/service/client_session.h:41: 'service' declared here

  namespace service
            ^

std::shared_ptr<std::vector<std::pair<bool, eCAL::SServiceResponse>>> CallBlocking(const std::string& method_name_, const std::string& request_, std::chrono::nanoseconds timeout_);

static void fromSerializedProtobuf(const std::string& response_pb_, eCAL::SServiceResponse& response_);
static void fromStruct(const Service::Response& response_struct_, eCAL::SServiceResponse& response_);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no type named 'Response' in namespace 'eCAL::service' [clang-diagnostic-error]

    static void fromStruct(const Service::Response& response_struct_, eCAL::SServiceResponse& response_);
                                          ^

@@ -35,15 +36,15 @@ namespace eCAL
public:
CTimerImpl() : m_stop(false), m_running(false), m_last_error(0) {}

CTimerImpl(const int timeout_, TimerCallbackT callback_, const int delay_) : m_stop(false), m_running(false) { Start(timeout_, callback_, delay_); }
CTimerImpl(const int timeout_, const TimerCallbackT& callback_, const int delay_) : m_stop(false), m_running(false) { Start(timeout_, callback_, delay_); }
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: constructor does not initialize these fields: m_last_error [cppcoreguidelines-pro-type-member-init]

ecal/core/src/time/ecal_timer.cpp:132:

-     std::chrono::nanoseconds m_last_error;
+     std::chrono::nanoseconds m_last_error{};

{
std::vector<TCLAP::Arg*> arg_list;
for (size_t j = 0; j < xor_list[i].size(); j++)
for (size_t j = 0; j < i.size(); j++)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use range-based for loop instead [modernize-loop-convert]

Suggested change
for (size_t j = 0; j < i.size(); j++)
for (auto j : i)

ecal/core/src/util/advanced_tclap_output.cpp:213:

-         if (hidden_arguments_.find(i[j]) == hidden_arguments_.end())
+         if (hidden_arguments_.find(j) == hidden_arguments_.end())

ecal/core/src/util/advanced_tclap_output.cpp:215:

-           arg_list.push_back(i[j]);
+           arg_list.push_back(j);

@@ -24,7 +24,6 @@

#pragma once
#include <ecal/ecal.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: 'ecal/ecal.h' file not found [clang-diagnostic-error]

#include <ecal/ecal.h>
         ^

@@ -1426,7 +1215,7 @@ PyObject* mon_logging(PyObject* /*self*/, PyObject* /*args*/)
PyObject* retList = PyList_New(0);

std::string logging_s;
if (eCAL::Monitoring::GetLogging(logging_s))
if (eCAL::Logging::GetLogging(logging_s))
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
if (eCAL::Logging::GetLogging(logging_s))
if (eCAL::Logging::GetLogging(logging_s) != 0)

@rex-schilasky rex-schilasky marked this pull request as ready for review February 27, 2024 13:30
// We cannot make it pure virtual, as it would break a bunch of implementations, who are not (yet) implementing this function
virtual SDataTypeInformation GetDataTypeInformation() const { return SDataTypeInformation{}; }
virtual struct SDataTypeInformation GetDataTypeInformation() const { return SDataTypeInformation{}; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need the sttruct keyword here? This should not be necessary? Is it not known or are we maybe missing an include?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rechecked it and it's not needed for sure. Not sure how this change happened.

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

if(handle_ == NULL) return(0);
eCAL::CSubscriber* sub = static_cast<eCAL::CSubscriber*>(handle_);
if(handle_ == nullptr) return(0);
auto* sub = static_cast<eCAL::CSubscriber*>(handle_);
delete sub;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete sub;
    ^
Additional context

ecal/core/src/ecalc.cpp:524: variable declared here

    auto* sub = static_cast<eCAL::CSubscriber*>(handle_);
    ^

if(handle_ == NULL) return(0);
eCAL::CTimer* timer = static_cast<eCAL::CTimer*>(handle_);
if (handle_ == nullptr) return(0);
auto* timer = static_cast<eCAL::CTimer*>(handle_);
delete timer;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete timer;
    ^
Additional context

ecal/core/src/ecalc.cpp:794: variable declared here

    auto* timer = static_cast<eCAL::CTimer*>(handle_);
    ^

if (handle_ == NULL) return(0);
eCAL::CServiceServer* server = static_cast<eCAL::CServiceServer*>(handle_);
if (handle_ == nullptr) return(0);
auto* server = static_cast<eCAL::CServiceServer*>(handle_);
delete server;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete server;
    ^
Additional context

ecal/core/src/ecalc.cpp:857: variable declared here

    auto* server = static_cast<eCAL::CServiceServer*>(handle_);
    ^

if(handle_ == NULL) return(0);
eCAL::CServiceClient* client = static_cast<eCAL::CServiceClient*>(handle_);
if (handle_ == nullptr) return(0);
auto* client = static_cast<eCAL::CServiceClient*>(handle_);
delete client;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete client;
    ^
Additional context

ecal/core/src/ecalc.cpp:963: variable declared here

    auto* client = static_cast<eCAL::CServiceClient*>(handle_);
    ^

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

if(handle_ == NULL) return(0);
eCAL::CServiceClient* client = static_cast<eCAL::CServiceClient*>(handle_);
if (handle_ == nullptr) return(0);
auto* client = static_cast<eCAL::CServiceClient*>(handle_);
delete client;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

    delete client;
    ^
Additional context

ecal/core/src/ecalc.cpp:953: variable declared here

    auto* client = static_cast<eCAL::CServiceClient*>(handle_);
    ^

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.

I wI will approve it even though i have not been able to review all files. we need to make sure to test things appropriately before an eCAL6 release.

# core internal feature configuration - end
# ------------------------------------------------------------------------------------------------------------------------------------------------------------------

cmake_dependent_option(ECAL_CORE_NPCAP_SUPPORT "Enable the eCAL Npcap Receiver (Win10 performance fix for UDP communication)" ON "ECAL_NPCAP_SUPPORT" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer that the main CMakeLists.txt file sets these options, this file needs no knowledge of options from its parent.
in CMakeLists.txt

if (ECAL_NPCA_SUPPORT)
  set(ECAL_CORE_NPCAP_SUPPORT ON)
endif()


#if ECAL_CORE_PUBLISHER
Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly disapprove of all these #ifdef . However this is a discussion for a different time.

namespace eCAL
{

inline SDataTypeInformation eCALSampleToTopicInformation(const eCAL::pb::Sample& sample)
inline SDataTypeInformation eCALSampleToTopicInformation(const eCAL::Registration::Sample& sample)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a question here. Now you actually write these struct types, they are not autogenerated.
Why do we have to map the types? Can't we use the SDatatypeInformation as a Member of the Sample in the first place?

return topic;
}

// This function can be removed in eCAL6. For the time being we need to enrich incoming samples with additional topic information.
inline void ModifyIncomingSampleForBackwardsCompatibility(const eCAL::pb::Sample& sample, eCAL::pb::Sample& modified_sample)
inline void ModifyIncomingSampleForBackwardsCompatibility(const eCAL::Registration::Sample& sample, eCAL::Registration::Sample& modified_sample)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function can actually be removed, like the above comment says.

}

void PubShareType(bool state_)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

We should discuss these for eCAL6. At least they need to be part of Publisher Konstructor options.

~CNamedMutexStubImpl()
{
}
~CNamedMutexStubImpl() override = default;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the constructor can be removed altogether - otherwise rule of 5.

@rex-schilasky rex-schilasky merged commit ac314fa into master Feb 29, 2024
14 checks passed
@rex-schilasky rex-schilasky deleted the new-core-alignment/integrate-new-core branch February 29, 2024 13:44
FlorianReimold added a commit that referenced this pull request Mar 11, 2024
The Qt6 support was removed by accident by #1370

Co-authored-by: Peter Guenschmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants