From cb9ef18bf5b8f9e33ceabfe270056ccc28a7552f Mon Sep 17 00:00:00 2001 From: Kerstin Keller Date: Fri, 17 Jan 2025 11:17:30 +0100 Subject: [PATCH] [core] Combine TopicID in Pubs/Sub-Event Callback struct, and SMethodInfoID in Server/Client Callback struct. --- ecal/core/include/ecal/ecal_callback.h | 38 +++++++++++-------- ecal/core/src/pubsub/ecal_publisher_impl.cpp | 4 +- ecal/core/src/pubsub/ecal_subscriber_impl.cpp | 9 +++-- .../src/service/ecal_service_client_impl.cpp | 6 ++- .../service/ecal_service_client_v5_impl.cpp | 12 +++--- .../src/service/ecal_service_server_impl.cpp | 5 ++- .../service/ecal_service_server_v5_impl.cpp | 4 +- .../src/clientserver_test.cpp | 6 +-- 8 files changed, 49 insertions(+), 35 deletions(-) diff --git a/ecal/core/include/ecal/ecal_callback.h b/ecal/core/include/ecal/ecal_callback.h index a00ec21811..3a0bf0242a 100644 --- a/ecal/core/include/ecal/ecal_callback.h +++ b/ecal/core/include/ecal/ecal_callback.h @@ -153,10 +153,11 @@ namespace eCAL **/ struct SPubEventCallbackData { - ePublisherEvent type{ ePublisherEvent::none }; //!< publisher event type - long long time{ 0 }; //!< publisher event time in µs - long long clock{ 0 }; //!< publisher event clock - SDataTypeInformation tdatatype; //!< datatype description of the connected subscriber (for pub_event_update_connection only) + Registration::STopicId publisher_id; //!< ID of the publisher triggering the event + ePublisherEvent type{ ePublisherEvent::none }; //!< publisher event type + long long time{ 0 }; //!< publisher event time in µs + long long clock{ 0 }; //!< publisher event clock + SDataTypeInformation tdatatype; //!< datatype description of the connected subscriber (for pub_event_update_connection only) }; /** @@ -165,17 +166,18 @@ namespace eCAL * @param topic_id_ The topic id struct of the received message. * @param data_ Event callback data structure with the event specific information. **/ - using PubEventCallbackT = std::function; + using PubEventCallbackT = std::function; /** * @brief eCAL subscriber event callback struct. **/ struct SSubEventCallbackData { - eSubscriberEvent type{ eSubscriberEvent::none }; //!< subscriber event type - long long time{ 0 }; //!< subscriber event time in µs - long long clock{ 0 }; //!< subscriber event clock - SDataTypeInformation tdatatype; //!< topic information of the connected subscriber (for sub_event_update_connection only) + Registration::STopicId subscriber_id; //!< ID of the publisher triggering the event + eSubscriberEvent type{ eSubscriberEvent::none }; //!< subscriber event type + long long time{ 0 }; //!< subscriber event time in µs + long long clock{ 0 }; //!< subscriber event clock + SDataTypeInformation tdatatype; //!< topic information of the connected subscriber (for sub_event_update_connection only) }; /** @@ -184,15 +186,17 @@ namespace eCAL * @param topic_id_ The topic id struct of the received message. * @param data_ Event callback data structure with the event specific information. **/ - using SubEventCallbackT = std::function; + using SubEventCallbackT = std::function; /** * @brief eCAL client event callback struct. **/ struct SClientEventCallbackData { - eClientEvent type{ eClientEvent::none }; //!< event type - long long time = 0; //!< event time in µs + // TODO: shouldn't this be a SServiceId? No methods involved? + Registration::SServiceMethodId client_id; //!< ID of the client triggering the event + eClientEvent type{ eClientEvent::none }; //!< event type + long long time = 0; //!< event time in µs }; /** @@ -201,15 +205,17 @@ namespace eCAL * @param service_id_ The service id struct of the connection that triggered the event. * @param data_ Event callback data structure with the event specific information. **/ - using ClientEventCallbackT = std::function; + using ClientEventCallbackT = std::function; /** * @brief eCAL server event callback struct. **/ struct SServerEventCallbackData { - eServerEvent type{ eServerEvent::none }; //!< event type - long long time = 0; //!< event time in µs + // TODO: shouldn't this be a SServiceId? No methods involved? + Registration::SServiceMethodId server_id; //!< ID of the server triggering the event + eServerEvent type{ eServerEvent::none }; //!< event type + long long time = 0; //!< event time in µs }; /** @@ -218,6 +224,6 @@ namespace eCAL * @param service_id_ The service id struct of the connection that triggered the event. * @param data_ Event callback data structure with the event specific information. **/ - using ServerEventCallbackT = std::function; + using ServerEventCallbackT = std::function; } } diff --git a/ecal/core/src/pubsub/ecal_publisher_impl.cpp b/ecal/core/src/pubsub/ecal_publisher_impl.cpp index cf4b34cee7..01a8e0afd0 100644 --- a/ecal/core/src/pubsub/ecal_publisher_impl.cpp +++ b/ecal/core/src/pubsub/ecal_publisher_impl.cpp @@ -744,7 +744,7 @@ namespace eCAL data.clock = 0; data.tdatatype = data_type_info_; - Registration::STopicId topic_id; + Registration::STopicId& topic_id = data.publisher_id; topic_id.topic_id.entity_id = subscription_info_.entity_id; topic_id.topic_id.process_id = subscription_info_.process_id; topic_id.topic_id.host_name = subscription_info_.host_name; @@ -752,7 +752,7 @@ namespace eCAL const std::lock_guard lock(m_event_id_callback_mutex); // call event callback - m_event_id_callback(topic_id, data); + m_event_id_callback(data); } // deprecated event handling with topic name diff --git a/ecal/core/src/pubsub/ecal_subscriber_impl.cpp b/ecal/core/src/pubsub/ecal_subscriber_impl.cpp index 019d62dce6..d9945100ea 100644 --- a/ecal/core/src/pubsub/ecal_subscriber_impl.cpp +++ b/ecal/core/src/pubsub/ecal_subscriber_impl.cpp @@ -793,7 +793,7 @@ namespace eCAL data.clock = 0; data.tdatatype = data_type_info_; - Registration::STopicId topic_id; + Registration::STopicId& topic_id = data.subscriber_id; topic_id.topic_id.entity_id = publication_info_.entity_id; topic_id.topic_id.process_id = publication_info_.process_id; topic_id.topic_id.host_name = publication_info_.host_name; @@ -801,7 +801,7 @@ namespace eCAL const std::lock_guard lock(m_event_id_callback_mutex); // call event callback - m_event_id_callback(topic_id, data); + m_event_id_callback(data); } // deprecated event handling with topic name @@ -914,12 +914,13 @@ namespace eCAL data.time = std::chrono::duration_cast(std::chrono::steady_clock::now().time_since_epoch()).count(); data.clock = current_clock_; - Registration::STopicId topic_id; + // TODO: Event data not filled correctly here! We should have information about the TopicID of the connection where data was dropped. + Registration::STopicId& topic_id = data.subscriber_id; topic_id.topic_name = m_attributes.topic_name; const std::lock_guard lock(m_event_id_callback_mutex); // call event callback - m_event_id_callback(topic_id, data); + m_event_id_callback(data); } // deprecated event handling with topic name diff --git a/ecal/core/src/service/ecal_service_client_impl.cpp b/ecal/core/src/service/ecal_service_client_impl.cpp index b6b9b180e9..55678ed32e 100644 --- a/ecal/core/src/service/ecal_service_client_impl.cpp +++ b/ecal/core/src/service/ecal_service_client_impl.cpp @@ -526,8 +526,12 @@ namespace eCAL SClientEventCallbackData callback_data; callback_data.type = event_type_; + // TODO: is it correct to use the system time here? do we not need some kind of eCAL time??? callback_data.time = std::chrono::duration_cast(std::chrono::steady_clock::now().time_since_epoch()).count(); - m_event_callback(service_id_, callback_data); + + // TODO: We would like to avoid copying here! + callback_data.client_id = service_id_; + m_event_callback(callback_data); } std::shared_ptr CServiceClientImpl::PrepareInitialResponse(const SClient& client_, const std::string& method_name_) diff --git a/ecal/core/src/service/ecal_service_client_v5_impl.cpp b/ecal/core/src/service/ecal_service_client_v5_impl.cpp index 5005509cc3..b7586f59c5 100644 --- a/ecal/core/src/service/ecal_service_client_v5_impl.cpp +++ b/ecal/core/src/service/ecal_service_client_v5_impl.cpp @@ -94,7 +94,7 @@ namespace eCAL Logging::Log(Logging::log_level_debug1, "v5::CServiceClientImpl: Creating service client with name: " + service_name_); // Define the event callback to pass to CServiceClient - v6::ClientEventCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SClientEventCallbackData& data_) + v6::ClientEventCallbackT event_callback = [this](const v6::SClientEventCallbackData& data_) { Logging::Log(Logging::log_level_debug2, "v5::CServiceClientImpl: Event callback triggered for event type: " + to_string(data_.type)); @@ -110,11 +110,11 @@ namespace eCAL SClientEventCallbackData event_data; event_data.type = data_.type; event_data.time = data_.time; - event_data.attr.hname = service_id_.service_id.host_name; - event_data.attr.sname = service_id_.service_name; - event_data.attr.pid = service_id_.service_id.process_id; - event_data.attr.sid = service_id_.service_id.entity_id; - callback->second(service_id_.service_name.c_str(), &event_data); + event_data.attr.hname = data_.client_id.service_id.host_name; + event_data.attr.sname = data_.client_id.service_name; + event_data.attr.pid = data_.client_id.service_id.process_id; + event_data.attr.sid = data_.client_id.service_id.entity_id; + callback->second(data_.client_id.service_name.c_str(), &event_data); } }; diff --git a/ecal/core/src/service/ecal_service_server_impl.cpp b/ecal/core/src/service/ecal_service_server_impl.cpp index e50335ade5..f9d2240527 100644 --- a/ecal/core/src/service/ecal_service_server_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_impl.cpp @@ -505,6 +505,9 @@ namespace eCAL SServerEventCallbackData callback_data; callback_data.type = event_type_; callback_data.time = std::chrono::duration_cast(std::chrono::steady_clock::now().time_since_epoch()).count(); - m_event_callback(service_id_, callback_data); + + // TODO: we should try to avoid a copy here + callback_data.server_id = service_id_; + m_event_callback(callback_data); } } diff --git a/ecal/core/src/service/ecal_service_server_v5_impl.cpp b/ecal/core/src/service/ecal_service_server_v5_impl.cpp index e465a81b06..e5291dfda9 100644 --- a/ecal/core/src/service/ecal_service_server_v5_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_v5_impl.cpp @@ -57,7 +57,7 @@ namespace eCAL } // Define the event callback to pass to CServiceClient - v6::ServerEventCallbackT event_callback = [this](const Registration::SServiceMethodId& service_id_, const v6::SServerEventCallbackData& data_) + v6::ServerEventCallbackT event_callback = [this](const v6::SServerEventCallbackData& data_) { Logging::Log(Logging::log_level_debug2, "v5::CServiceServerImpl: Event callback triggered for event type: " + to_string(data_.type)); @@ -73,7 +73,7 @@ namespace eCAL SServerEventCallbackData event_data; event_data.type = data_.type; event_data.time = data_.time; - callback->second(service_id_.service_name.c_str(), &event_data); + callback->second(data_.server_id.service_name.c_str(), &event_data); } }; diff --git a/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp b/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp index 2e7004d994..867c0e1c4d 100644 --- a/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp +++ b/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp @@ -116,7 +116,7 @@ TEST(core_cpp_clientserver, ClientConnectEvent) // client event callback for connect events atomic_signalable event_connected_fired (0); atomic_signalable event_disconnected_fired(0); - auto event_callback = [&](const eCAL::Registration::SServiceMethodId& /*service_id_*/, const struct eCAL::SClientEventCallbackData& data_) + auto event_callback = [&](const struct eCAL::SClientEventCallbackData& data_) { switch (data_.type) { @@ -181,7 +181,7 @@ TEST(core_cpp_clientserver, ServerConnectEvent) // server event callback for connect events atomic_signalable event_connected_fired (0); atomic_signalable event_disconnected_fired(0); - auto event_callback = [&](const eCAL::Registration::SServiceMethodId& /*service_id_*/, const struct eCAL::SServerEventCallbackData& data_) -> void + auto event_callback = [&](const struct eCAL::SServerEventCallbackData& data_) -> void { switch (data_.type) { @@ -384,7 +384,7 @@ TEST(core_cpp_clientserver, ClientServerBaseCallbackTimeout) // event callback for timeout event std::atomic timeout_fired(0); - auto event_callback = [&](const eCAL::Registration::SServiceMethodId& /*service_id_*/, const struct eCAL::SClientEventCallbackData& data_) -> void + auto event_callback = [&](const struct eCAL::SClientEventCallbackData& data_) -> void { if (data_.type == eCAL::eClientEvent::timeout) {