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

Feature event naming #1898

Merged
merged 2 commits into from
Jan 14, 2025
Merged

Feature event naming #1898

merged 2 commits into from
Jan 14, 2025

Conversation

KerstinKeller
Copy link
Contributor

Description

All v6 callbacks / events have ID removed from their names.

@KerstinKeller KerstinKeller added the cherry-pick-to-NONE Don't cherry-pick these changes label Jan 14, 2025
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

@@ -47,11 +47,11 @@ namespace eCAL
if(g_pubgate() != nullptr) g_pubgate()->Register(topic_name_, m_publisher_impl);
}

CPublisher::CPublisher(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const PubEventIDCallbackT event_callback_, const Publisher::Configuration& config_) :
CPublisher::CPublisher(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const PubEventCallbackT event_callback_, const Publisher::Configuration& config_) :
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: the const qualified parameter 'event_callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

ecal/core/include/ecal/ecal_publisher.h:71:

-         CPublisher(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const PubEventCallbackT event_callback_, const Publisher::Configuration& config_ = GetPublisherConfiguration());
+         CPublisher(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const PubEventCallbackT& event_callback_, const Publisher::Configuration& config_ = GetPublisherConfiguration());
Suggested change
CPublisher::CPublisher(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const PubEventCallbackT event_callback_, const Publisher::Configuration& config_) :
CPublisher::CPublisher(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const PubEventCallbackT& event_callback_, const Publisher::Configuration& config_) :

@@ -400,12 +400,12 @@ namespace eCAL
return(true);
}

bool CPublisherImpl::SetEventIDCallback(const PubEventIDCallbackT callback_)
bool CPublisherImpl::SetEventCallback(const PubEventCallbackT 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: the const qualified parameter 'callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

ecal/core/src/pubsub/ecal_publisher_impl.h:89:

-     bool SetEventCallback(const PubEventCallbackT callback_);
+     bool SetEventCallback(const PubEventCallbackT& callback_);
Suggested change
bool CPublisherImpl::SetEventCallback(const PubEventCallbackT callback_)
bool CPublisherImpl::SetEventCallback(const PubEventCallbackT& callback_)

@@ -40,7 +40,7 @@ namespace eCAL
if (g_subgate() != nullptr) g_subgate()->Register(topic_name_, m_subscriber_impl);
}

CSubscriber::CSubscriber(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const SubEventIDCallbackT event_callback_, const Subscriber::Configuration& config_) :
CSubscriber::CSubscriber(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const SubEventCallbackT event_callback_, const Subscriber::Configuration& config_) :
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: the const qualified parameter 'event_callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

ecal/core/include/ecal/ecal_subscriber.h:67:

-         CSubscriber(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const SubEventCallbackT event_callback_, const Subscriber::Configuration& config_ = GetSubscriberConfiguration());
+         CSubscriber(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const SubEventCallbackT& event_callback_, const Subscriber::Configuration& config_ = GetSubscriberConfiguration());
Suggested change
CSubscriber::CSubscriber(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const SubEventCallbackT event_callback_, const Subscriber::Configuration& config_) :
CSubscriber::CSubscriber(const std::string& topic_name_, const SDataTypeInformation& data_type_info_, const SubEventCallbackT& event_callback_, const Subscriber::Configuration& config_) :

@@ -231,7 +231,7 @@ namespace eCAL
return(true);
}

bool CSubscriberImpl::SetEventIDCallback(const SubEventIDCallbackT callback_)
bool CSubscriberImpl::SetEventIDCallback(const SubEventCallbackT 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: the const qualified parameter 'callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

Suggested change
bool CSubscriberImpl::SetEventIDCallback(const SubEventCallbackT callback_)
bool CSubscriberImpl::SetEventIDCallback(const SubEventCallbackT& callback_)

ecal/core/src/pubsub/ecal_subscriber_impl.h:80:

-     bool SetEventIDCallback(const SubEventCallbackT callback_);
+     bool SetEventIDCallback(const SubEventCallbackT& callback_);

@@ -31,7 +31,7 @@

namespace eCAL
{
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_, const ClientEventIDCallbackT event_callback_)
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_, const ClientEventCallbackT event_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: the const qualified parameter 'event_callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

ecal/core/include/ecal/ecal_client.h:58:

-         CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_ = ServiceMethodInformationMapT(), const ClientEventCallbackT event_callback_ = ClientEventCallbackT());
+         CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_ = ServiceMethodInformationMapT(), const ClientEventCallbackT& event_callback_ = ClientEventCallbackT());
Suggested change
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_, const ClientEventCallbackT event_callback_)
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_, const ClientEventCallbackT& event_callback_)

@@ -31,7 +31,7 @@

namespace eCAL
{
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_, const ClientEventIDCallbackT event_callback_)
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_, const ClientEventCallbackT event_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: the const qualified parameter 'method_information_map_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

ecal/core/include/ecal/ecal_client.h:58:

-         CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_ = ServiceMethodInformationMapT(), const ClientEventCallbackT event_callback_ = ClientEventCallbackT());
+         CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT& method_information_map_ = ServiceMethodInformationMapT(), const ClientEventCallbackT event_callback_ = ClientEventCallbackT());
Suggested change
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT method_information_map_, const ClientEventCallbackT event_callback_)
CServiceClient::CServiceClient(const std::string& service_name_, const ServiceMethodInformationMapT& method_information_map_, const ClientEventCallbackT event_callback_)

@@ -94,7 +94,7 @@ namespace eCAL
Logging::Log(log_level_debug1, "v5::CServiceClientImpl: Creating service client with name: " + service_name_);

// Define the event callback to pass to CServiceClient
ClientEventIDCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const struct SClientEventIDCallbackData& data_)
v6::ClientEventCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SClientEventCallbackData& data_)
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 'event_callback' of type 'v6::ClientEventCallbackT' (aka 'function<void (const Registration::SServiceMethodId &, const SClientEventCallbackData &)>') can be declared 'const' [misc-const-correctness]

Suggested change
v6::ClientEventCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SClientEventCallbackData& data_)
v6::ClientEventCallbackT const event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SClientEventCallbackData& data_)

@@ -29,7 +29,7 @@

namespace eCAL
{
CServiceServer::CServiceServer(const std::string& service_name_, const ServerEventIDCallbackT event_callback_)
CServiceServer::CServiceServer(const std::string& service_name_, const ServerEventCallbackT event_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: the const qualified parameter 'event_callback_' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]

Suggested change
CServiceServer::CServiceServer(const std::string& service_name_, const ServerEventCallbackT event_callback_)
CServiceServer::CServiceServer(const std::string& service_name_, const ServerEventCallbackT& event_callback_)

ecal/core/include/ecal/ecal_server.h:55:

-         explicit CServiceServer(const std::string& service_name_, const ServerEventCallbackT event_callback_ = ServerEventCallbackT());
+         explicit CServiceServer(const std::string& service_name_, const ServerEventCallbackT& event_callback_ = ServerEventCallbackT());

@@ -49,7 +49,7 @@ namespace eCAL
}

// Constructor
CServiceServerImpl::CServiceServerImpl(const std::string& service_name_, const ServerEventIDCallbackT& event_callback_)
CServiceServerImpl::CServiceServerImpl(const std::string& service_name_, const ServerEventCallbackT& event_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: constructor does not initialize these fields: m_service_id [cppcoreguidelines-pro-type-member-init]

ecal/core/src/service/ecal_service_server_impl.h:100:

-     Registration::EntityIdT                m_service_id;
+     Registration::EntityIdT                m_service_id{};

@@ -57,7 +57,7 @@ namespace eCAL
}

// Define the event callback to pass to CServiceClient
ServerEventIDCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const struct SServerEventIDCallbackData& data_)
v6::ServerEventCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SServerEventCallbackData& data_)
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 'event_callback' of type 'v6::ServerEventCallbackT' (aka 'function<void (const Registration::SServiceMethodId &, const struct SServerEventCallbackData &)>') can be declared 'const' [misc-const-correctness]

Suggested change
v6::ServerEventCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SServerEventCallbackData& data_)
v6::ServerEventCallbackT const event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SServerEventCallbackData& data_)

Copy link
Contributor

@Peguen Peguen left a comment

Choose a reason for hiding this comment

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

✍️

@KerstinKeller KerstinKeller merged commit 5a90554 into master Jan 14, 2025
22 checks passed
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.

2 participants