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

sync-with-eclipse-ecal-2024-10-18 #79

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

rex-schilasky
Copy link
Contributor

Description

sync-with-eclipse-ecal-2024-10-18

Copy link

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

@@ -32,7 +32,7 @@
namespace
{
// After switchting to c++17, this can be replaced by an inline constexpr
static const eCAL_Logging_Filter log_level_default = log_level_info | log_level_warning | log_level_error | log_level_fatal;
static const eCAL_Logging_Filter log_filter_default = log_level_info | log_level_warning | log_level_error | log_level_fatal;

Choose a reason for hiding this comment

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

warning: variable 'log_filter_default' defined in a header file; variable definitions in header files can lead to ODR violations [misc-definitions-in-headers]

  static const eCAL_Logging_Filter log_filter_default = log_level_info | log_level_warning | log_level_error | log_level_fatal;
                                   ^

@@ -32,7 +32,7 @@
namespace
{
// After switchting to c++17, this can be replaced by an inline constexpr
static const eCAL_Logging_Filter log_level_default = log_level_info | log_level_warning | log_level_error | log_level_fatal;
static const eCAL_Logging_Filter log_filter_default = log_level_info | log_level_warning | log_level_error | log_level_fatal;

Choose a reason for hiding this comment

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

warning: 'log_filter_default' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]

Suggested change
static const eCAL_Logging_Filter log_filter_default = log_level_info | log_level_warning | log_level_error | log_level_fatal;
const eCAL_Logging_Filter log_filter_default = log_level_info | log_level_warning | log_level_error | log_level_fatal;

typedef unsigned char eCAL_Logging_Filter; //!< This type is to be used as a bitmask for the activated logging levels

Choose a reason for hiding this comment

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

warning: use 'using' instead of 'typedef' [modernize-use-using]

Suggested change
typedef unsigned char eCAL_Logging_Filter; //!< This type is to be used as a bitmask for the activated logging levels
using eCAL_Logging_Filter = unsigned char; //!< This type is to be used as a bitmask for the activated logging levels

@@ -63,7 +63,7 @@ namespace eCAL
* @param data_type_info_ Topic data type information (encoding, type, descriptor).
* @param config_ Optional configuration parameters.
**/
CMsgPublisher(const std::string& topic_name_, const struct SDataTypeInformation& data_type_info_, const Publisher::Configuration& config_ = {}) : CPublisher(topic_name_, data_type_info_, config_)
CMsgPublisher(const std::string& topic_name_, const struct SDataTypeInformation& data_type_info_, const Publisher::Configuration& config_ = GetPublisherConfiguration()) : CPublisher(topic_name_, data_type_info_, config_)

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& data_type_info_, const Publisher::Configuration& config_ = GetPublisherConfiguration()) : CPublisher(topic_name_, data_type_info_, config_)
    ^

@@ -74,7 +74,7 @@
* @param topic_name_ Unique topic name.
* @param config_ Optional configuration parameters.
**/
explicit CMsgPublisher(const std::string& topic_name_, const Publisher::Configuration& config_ = {}) : CMsgPublisher(topic_name_, GetDataTypeInformation(), config_)
explicit CMsgPublisher(const std::string& topic_name_, const Publisher::Configuration& config_ = GetPublisherConfiguration()) : CMsgPublisher(topic_name_, GetDataTypeInformation(), config_)

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_, const Publisher::Configuration& config_ = GetPublisherConfiguration()) : CMsgPublisher(topic_name_, GetDataTypeInformation(), config_)
    ^

@@ -125,15 +126,18 @@ namespace eCAL
////////////////
// LAYER
////////////////
CTCPReaderLayer::CTCPReaderLayer() : m_initialized(false) {}
CTCPReaderLayer::CTCPReaderLayer()

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_attributes [cppcoreguidelines-pro-type-member-init]

ecal/core/src/readwrite/tcp/ecal_reader_tcp.h:85:

-     eCAL::eCALReader::TCPLayer::SAttributes m_attributes;
+     eCAL::eCALReader::TCPLayer::SAttributes m_attributes{};

@@ -36,13 +37,13 @@ namespace eCAL
////////////////
// LAYER
////////////////
class CUDPReaderLayer : public CReaderLayer<CUDPReaderLayer>
class CUDPReaderLayer : public CReaderLayer<CUDPReaderLayer, eCAL::eCALReader::UDP::SAttributes>

Choose a reason for hiding this comment

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

warning: class 'CUDPReaderLayer' defines a non-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 CUDPReaderLayer : public CReaderLayer<CUDPReaderLayer, eCAL::eCALReader::UDP::SAttributes>
        ^

}
#endif

// start cyclic registration thread
m_reg_sample_snd_thread = std::make_shared<CCallbackThread>(std::bind(&CRegistrationProvider::RegisterSendThread, this));

Choose a reason for hiding this comment

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

warning: misleading indentation: statement is indented too deeply [readability-misleading-indentation]

    m_reg_sample_snd_thread = std::make_shared<CCallbackThread>(std::bind(&CRegistrationProvider::RegisterSendThread, this));
    ^
Additional context

ecal/core/src/registration/ecal_registration_provider.cpp:70: did you mean this line to be inside this 'if'

    if (m_attributes.shm_enabled)
    ^

#include <memory>
#include <mutex>

#include "util/ecal_thread.h"

Choose a reason for hiding this comment

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

warning: duplicate include [readability-duplicate-include]

ecal/core/src/registration/ecal_registration_provider.h:38:

- 
- #include "util/ecal_thread.h"


for (const auto& registration_sample : expired_samples)
{
Sample unregistration_sample = CreateUnregisterSample(registration_sample.second);

Choose a reason for hiding this comment

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

warning: variable 'unregistration_sample' of type 'Sample' can be declared 'const' [misc-const-correctness]

Suggested change
Sample unregistration_sample = CreateUnregisterSample(registration_sample.second);
Sample const unregistration_sample = CreateUnregisterSample(registration_sample.second);

Copy link

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

@@ -59,20 +59,20 @@ namespace eCAL
return reinterpret_cast<void *>(static_cast<char *>(address) + GetMemfileHeader(address)->message_queue_offset);
}

CMemoryFileBroadcast::CMemoryFileBroadcast(): m_created(false), m_max_queue_size(0), m_broadcast_memfile(std::make_unique<eCAL::CMemoryFile>()), m_event_queue(), m_last_timestamp(0)
CMemoryFileBroadcast::CMemoryFileBroadcast(): m_created(false), m_broadcast_memfile(std::make_unique<eCAL::CMemoryFile>()), m_event_queue(), m_last_timestamp(0)

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_event_queue' is redundant [readability-redundant-member-init]

Suggested change
CMemoryFileBroadcast::CMemoryFileBroadcast(): m_created(false), m_broadcast_memfile(std::make_unique<eCAL::CMemoryFile>()), m_event_queue(), m_last_timestamp(0)
CMemoryFileBroadcast::CMemoryFileBroadcast(): m_created(false), m_broadcast_memfile(std::make_unique<eCAL::CMemoryFile>()), , m_last_timestamp(0)


using namespace eCAL;

eCAL::CRegistrationReceiverUDP::CRegistrationReceiverUDP(RegistrationApplySampleCallbackT apply_sample_callback)
eCAL::CRegistrationReceiverUDP::CRegistrationReceiverUDP(RegistrationApplySampleCallbackT apply_sample_callback, const Registration::UDP::SReceiverAttributes& attr_)

Choose a reason for hiding this comment

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

warning: the parameter 'apply_sample_callback' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
eCAL::CRegistrationReceiverUDP::CRegistrationReceiverUDP(RegistrationApplySampleCallbackT apply_sample_callback, const Registration::UDP::SReceiverAttributes& attr_)
eCAL::CRegistrationReceiverUDP::CRegistrationReceiverUDP(const RegistrationApplySampleCallbackT& apply_sample_callback, const Registration::UDP::SReceiverAttributes& attr_)

ecal/core/src/registration/udp/ecal_registration_receiver_udp.h:40:

-     CRegistrationReceiverUDP(RegistrationApplySampleCallbackT apply_sample_callback, const Registration::UDP::SReceiverAttributes& attr_);
+     CRegistrationReceiverUDP(const RegistrationApplySampleCallbackT& apply_sample_callback, const Registration::UDP::SReceiverAttributes& attr_);

bool operator==(eCAL::Types::IpAddressV4 lhs, const std::string& ip_string_) { return lhs.Get() == ip_string_; };
bool operator==(const std::string& ip_string_, eCAL::Types::IpAddressV4 rhs) { return rhs == ip_string_; };
bool IpAddressV4::operator==(const eCAL::Types::IpAddressV4& rhs) const { return m_ip_address == rhs.Get(); }
bool operator==(eCAL::Types::IpAddressV4 lhs, const char* ip_string_) { return lhs.Get() == std::string(ip_string_); }

Choose a reason for hiding this comment

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

warning: the parameter 'lhs' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
bool operator==(eCAL::Types::IpAddressV4 lhs, const char* ip_string_) { return lhs.Get() == std::string(ip_string_); }
bool operator==(const eCAL::Types::IpAddressV4& lhs, const char* ip_string_) { return lhs.Get() == std::string(ip_string_); }

ecal/core/include/ecal/types/ecal_custom_data_types.h:56:

-       ECAL_API friend bool operator==(eCAL::Types::IpAddressV4 lhs, const char* ip_string_);
+       ECAL_API friend bool operator==(const eCAL::Types::IpAddressV4& lhs, const char* ip_string_);

bool operator==(const std::string& ip_string_, eCAL::Types::IpAddressV4 rhs) { return rhs == ip_string_; };
bool IpAddressV4::operator==(const eCAL::Types::IpAddressV4& rhs) const { return m_ip_address == rhs.Get(); }
bool operator==(eCAL::Types::IpAddressV4 lhs, const char* ip_string_) { return lhs.Get() == std::string(ip_string_); }
bool operator==(const char* ip_string_, eCAL::Types::IpAddressV4 rhs) { return rhs == ip_string_; }

Choose a reason for hiding this comment

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

warning: parameter 'rhs' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]

ecal/core/src/types/ecal_custom_data_types.cpp:29:

- #include <ecal_def.h>
+ #include <utility>
+ #include <ecal_def.h>
Suggested change
bool operator==(const char* ip_string_, eCAL::Types::IpAddressV4 rhs) { return rhs == ip_string_; }
bool operator==(const char* ip_string_, eCAL::Types::IpAddressV4 rhs) { return std::move(rhs) == ip_string_; }

bool IpAddressV4::operator==(const eCAL::Types::IpAddressV4& rhs) const { return m_ip_address == rhs.Get(); }
bool operator==(eCAL::Types::IpAddressV4 lhs, const char* ip_string_) { return lhs.Get() == std::string(ip_string_); }
bool operator==(const char* ip_string_, eCAL::Types::IpAddressV4 rhs) { return rhs == ip_string_; }
bool operator==(eCAL::Types::IpAddressV4 lhs, const std::string& ip_string_) { return lhs.Get() == ip_string_; }

Choose a reason for hiding this comment

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

warning: the parameter 'lhs' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
bool operator==(eCAL::Types::IpAddressV4 lhs, const std::string& ip_string_) { return lhs.Get() == ip_string_; }
bool operator==(const eCAL::Types::IpAddressV4& lhs, const std::string& ip_string_) { return lhs.Get() == ip_string_; }

ecal/core/include/ecal/types/ecal_custom_data_types.h:58:

-       ECAL_API friend bool operator==(eCAL::Types::IpAddressV4 lhs, const std::string& ip_string_);
+       ECAL_API friend bool operator==(const eCAL::Types::IpAddressV4& lhs, const std::string& ip_string_);

#include "serialization/ecal_struct_sample_registration.h"

eCAL::Registration::Sample pub_foo_process_a_unregister;
eCAL::Registration::Sample pub_foo_process_a_register_1;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample pub_foo_process_a_register_1;
                           ^


eCAL::Registration::Sample pub_foo_process_a_unregister;
eCAL::Registration::Sample pub_foo_process_a_register_1;
eCAL::Registration::Sample pub_foo_process_a_register_2;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample pub_foo_process_a_register_2;
                           ^

eCAL::Registration::Sample pub_foo_process_a_register_1;
eCAL::Registration::Sample pub_foo_process_a_register_2;

eCAL::Registration::Sample sub_foo_process_a_unregister;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample sub_foo_process_a_unregister;
                           ^

eCAL::Registration::Sample pub_foo_process_a_register_2;

eCAL::Registration::Sample sub_foo_process_a_unregister;
eCAL::Registration::Sample sub_foo_process_a_register_1;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample sub_foo_process_a_register_1;
                           ^


eCAL::Registration::Sample sub_foo_process_a_unregister;
eCAL::Registration::Sample sub_foo_process_a_register_1;
eCAL::Registration::Sample sub_foo_process_a_register_2;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample sub_foo_process_a_register_2;
                           ^

Copy link

@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::Registration::Sample sub_foo_process_a_register_1;
eCAL::Registration::Sample sub_foo_process_a_register_2;

eCAL::Registration::Sample sub_foo_process_b_unregister;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample sub_foo_process_b_unregister;
                           ^

eCAL::Registration::Sample sub_foo_process_a_register_2;

eCAL::Registration::Sample sub_foo_process_b_unregister;
eCAL::Registration::Sample sub_foo_process_b_register_1;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample sub_foo_process_b_register_1;
                           ^


eCAL::Registration::Sample sub_foo_process_b_unregister;
eCAL::Registration::Sample sub_foo_process_b_register_1;
eCAL::Registration::Sample sub_foo_process_b_register_2;

Choose a reason for hiding this comment

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

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

eCAL::Registration::Sample sub_foo_process_b_register_2;
                           ^

}

private:
static duration current_time;

Choose a reason for hiding this comment

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

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

  static duration current_time;
                  ^

};

// Initialize the static member
TestingClock::duration TestingClock::current_time{ 0 };

Choose a reason for hiding this comment

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

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

TestingClock::duration TestingClock::current_time{ 0 };
                                     ^

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.

1 participant