From d225e45a670b3670d45708806a4ac29740001229 Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Tue, 7 Jan 2025 11:02:48 +0100 Subject: [PATCH 1/6] use SDataTypeInfo for service/client registration --- .../src/service/ecal_service_client_impl.cpp | 9 +++++- .../src/service/ecal_service_server_impl.cpp | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/ecal/core/src/service/ecal_service_client_impl.cpp b/ecal/core/src/service/ecal_service_client_impl.cpp index 961a495e43..1dcc94ed19 100644 --- a/ecal/core/src/service/ecal_service_client_impl.cpp +++ b/ecal/core/src/service/ecal_service_client_impl.cpp @@ -396,11 +396,18 @@ namespace eCAL const auto& method_information = method_information_pair.second; Service::Method method; - method.mname = method_name; + method.mname = method_name; + + // old type and descriptor fields method.req_type = method_information.request_type.name; method.req_desc = method_information.request_type.descriptor; method.resp_type = method_information.response_type.name; method.resp_desc = method_information.response_type.descriptor; + + // new type and descriptor fields + method.req_datatype = method_information.request_type; + method.resp_datatype = method_information.response_type; + { const auto& call_count_iter = m_method_call_count_map.find(method_name); if (call_count_iter != m_method_call_count_map.end()) diff --git a/ecal/core/src/service/ecal_service_server_impl.cpp b/ecal/core/src/service/ecal_service_server_impl.cpp index 6be955148f..d74ef12eeb 100644 --- a/ecal/core/src/service/ecal_service_server_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_impl.cpp @@ -77,10 +77,18 @@ namespace eCAL if (iter != m_method_map.end()) { Logging::Log(log_level_warning, "CServiceServerImpl::AddMethodCallback: Method already exists, updating callback: " + method_); + + // old type and descriptor fields iter->second.method.req_type = method_info_.request_type.name; iter->second.method.req_desc = method_info_.request_type.descriptor; iter->second.method.resp_type = method_info_.response_type.name; iter->second.method.resp_desc = method_info_.response_type.descriptor; + + // new type and descriptor fields + iter->second.method.req_datatype = method_info_.request_type; + iter->second.method.resp_datatype = method_info_.response_type; + + // callback iter->second.callback = callback_; } else @@ -89,12 +97,21 @@ namespace eCAL Logging::Log(log_level_debug1, "CServiceServerImpl::AddMethodCallback: Registering new method: " + method_); #endif SMethod method; - method.method.mname = method_; + // method name + method.method.mname = method_; + + // old type and descriptor fields method.method.req_type = method_info_.request_type.name; method.method.req_desc = method_info_.request_type.descriptor; method.method.resp_type = method_info_.response_type.name; method.method.resp_desc = method_info_.response_type.descriptor; - method.callback = callback_; + + // new type and descriptor fields + method.method.req_datatype = method_info_.request_type; + method.method.resp_datatype = method_info_.response_type; + + // callback + method.callback = callback_; m_method_map[method_] = method; } @@ -295,11 +312,18 @@ namespace eCAL for (const auto& iter : m_method_map) { Service::Method method; - method.mname = iter.first; + method.mname = iter.first; + + // old type and descriptor fields method.req_type = iter.second.method.req_type; method.req_desc = iter.second.method.req_desc; method.resp_type = iter.second.method.resp_type; method.resp_desc = iter.second.method.resp_desc; + + // new type and descriptor fields + method.req_datatype = iter.second.method.req_datatype; + method.resp_datatype = iter.second.method.resp_datatype; + method.call_count = iter.second.method.call_count; service.methods.push_back(method); } From e49ec46dbdc76a9c7f49909d20fec5fe55da2317 Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Tue, 7 Jan 2025 11:14:56 +0100 Subject: [PATCH 2/6] introduce new MethodInfoCallbackT using SDataTypeInformation for request and response type --- ecal/core/include/ecal/ecal_server.h | 5 +--- ecal/core/include/ecal/ecal_service_info.h | 13 ++++++++- ecal/core/src/service/ecal_service_server.cpp | 2 +- .../src/service/ecal_service_server_impl.cpp | 4 +-- .../src/service/ecal_service_server_impl.h | 6 ++--- .../service/ecal_service_server_v5_impl.cpp | 13 ++++++++- .../latency_server/src/latency_server.cpp | 2 +- .../minimal_server/src/minimal_server.cpp | 2 +- .../src/clientserver_test.cpp | 27 ++++++++++--------- .../src/registration_getservices.cpp | 4 +-- 10 files changed, 49 insertions(+), 29 deletions(-) diff --git a/ecal/core/include/ecal/ecal_server.h b/ecal/core/include/ecal/ecal_server.h index 25a95edada..d569c4b52f 100644 --- a/ecal/core/include/ecal/ecal_server.h +++ b/ecal/core/include/ecal/ecal_server.h @@ -92,11 +92,8 @@ namespace eCAL * * @return True if succeeded, false if not. **/ - - // TODO: Provide new MethodCallbackT type using SServiceMethodInformation instead "MethodCallbackT = std::function" - ECAL_API_EXPORTED_MEMBER - bool SetMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodCallbackT& callback_); + bool SetMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_); /** * @brief Remove method callback. diff --git a/ecal/core/include/ecal/ecal_service_info.h b/ecal/core/include/ecal/ecal_service_info.h index 61b77e0b7d..fa3e762a27 100644 --- a/ecal/core/include/ecal/ecal_service_info.h +++ b/ecal/core/include/ecal/ecal_service_info.h @@ -56,7 +56,7 @@ namespace eCAL using ServiceResponseVecT = std::vector; //!< vector of multiple service responses (deprecated) /** - * @brief Service method callback function type (low level server interface). + * @brief Service method callback function type (low level server interface). (deprecated) * * @param method_ The method name. * @param req_type_ The type of the method request. @@ -66,6 +66,17 @@ namespace eCAL **/ using MethodCallbackT = std::function; + /** + * @brief Service method callback function type (low level server interface). + * + * @param method_ The method name. + * @param req_type_ The type of the method request. + * @param resp_type_ The type of the method response. + * @param request_ The request. + * @param response_ The response returned from the method call. + **/ + using MethodInfoCallbackT = std::function; + /** * @brief Service response callback function type (low level client interface). (deprecated) * diff --git a/ecal/core/src/service/ecal_service_server.cpp b/ecal/core/src/service/ecal_service_server.cpp index fda2e33d92..e3fe517b43 100644 --- a/ecal/core/src/service/ecal_service_server.cpp +++ b/ecal/core/src/service/ecal_service_server.cpp @@ -62,7 +62,7 @@ namespace eCAL return *this; } - bool CServiceServer::SetMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodCallbackT& callback_) + bool CServiceServer::SetMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_) { if (m_service_server_impl == nullptr) return false; return m_service_server_impl->AddMethodCallback(method_, method_info_, callback_); diff --git a/ecal/core/src/service/ecal_service_server_impl.cpp b/ecal/core/src/service/ecal_service_server_impl.cpp index d74ef12eeb..d123b2787c 100644 --- a/ecal/core/src/service/ecal_service_server_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_impl.cpp @@ -66,7 +66,7 @@ namespace eCAL Stop(); } - bool CServiceServerImpl::AddMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodCallbackT& callback_) + bool CServiceServerImpl::AddMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_) { #ifndef NDEBUG Logging::Log(log_level_debug1, "CServiceServerImpl::AddMethodCallback: Adding method callback for method: " + method_); @@ -416,7 +416,7 @@ namespace eCAL // execute method (outside lock guard) const std::string& request_s = request.request; std::string response_s; - const int service_return_state = method.callback(method.method.mname, method.method.req_type, method.method.resp_type, request_s, response_s); + const int service_return_state = method.callback(method.method.mname, method.method.req_datatype, method.method.resp_datatype, request_s, response_s); // set method call state 'executed' response_header.state = Service::eMethodCallState::executed; diff --git a/ecal/core/src/service/ecal_service_server_impl.h b/ecal/core/src/service/ecal_service_server_impl.h index be5ad0c44d..5d7e07edff 100644 --- a/ecal/core/src/service/ecal_service_server_impl.h +++ b/ecal/core/src/service/ecal_service_server_impl.h @@ -55,7 +55,7 @@ namespace eCAL public: ~CServiceServerImpl(); - bool AddMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodCallbackT& callback_); + bool AddMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_); bool RemoveMethodCallback(const std::string& method_); // Check connection state of a specific service @@ -104,8 +104,8 @@ namespace eCAL // Server method map and synchronization struct SMethod { - Service::Method method; - MethodCallbackT callback; + Service::Method method; + MethodInfoCallbackT callback; }; using MethodMapT = std::map; 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 d715455f02..0a54fb51c5 100644 --- a/ecal/core/src/service/ecal_service_server_v5_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_v5_impl.cpp @@ -122,7 +122,18 @@ namespace eCAL method_info.request_type.name = req_type_; method_info.response_type.name = resp_type_; - return m_service_server_impl->SetMethodCallback(method_, method_info, callback_); + MethodInfoCallbackT callback = + [req_type_, resp_type_, callback_]( + const std::string& method, + const SDataTypeInformation& req_type_info, + const SDataTypeInformation& resp_type_info, + const std::string& request, + std::string& response) -> int + { + return callback_(method, req_type_info.name, resp_type_info.name, request, response); + }; + + return m_service_server_impl->SetMethodCallback(method_, method_info, callback); } bool CServiceServerImpl::RemMethodCallback(const std::string& method_) diff --git a/ecal/samples/cpp/services/latency_server/src/latency_server.cpp b/ecal/samples/cpp/services/latency_server/src/latency_server.cpp index 2003da72a3..71f89eb718 100644 --- a/ecal/samples/cpp/services/latency_server/src/latency_server.cpp +++ b/ecal/samples/cpp/services/latency_server/src/latency_server.cpp @@ -22,7 +22,7 @@ #include #include -int OnHello(const std::string& /*method_*/, const std::string& /*req_type_*/, const std::string& /*resp_type_*/, const std::string& /*request_*/, std::string& /*response_*/) +int OnHello(const std::string& /*method_*/, const eCAL::SDataTypeInformation& /*req_type_*/, const eCAL::SDataTypeInformation& /*resp_type_*/, const std::string& /*request_*/, std::string& /*response_*/) { return 0; } diff --git a/ecal/samples/cpp/services/minimal_server/src/minimal_server.cpp b/ecal/samples/cpp/services/minimal_server/src/minimal_server.cpp index 6c8a1e8100..9dc490bc57 100644 --- a/ecal/samples/cpp/services/minimal_server/src/minimal_server.cpp +++ b/ecal/samples/cpp/services/minimal_server/src/minimal_server.cpp @@ -24,7 +24,7 @@ #include // method callback -int OnMethodCallback(const std::string& method_, const std::string& /*req_type_*/, const std::string& /*resp_type_*/, const std::string& request_, std::string& response_) +int OnMethodCallback(const std::string& method_, const eCAL::SDataTypeInformation& /*req_type_*/, const eCAL::SDataTypeInformation& /*resp_type_*/, const std::string& request_, std::string& response_) { std::cout << "Method called : " << method_ << std::endl; std::cout << "Request : " << request_ << std::endl << std::endl; diff --git a/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp b/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp index 0e4be483bc..fed22681eb 100644 --- a/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp +++ b/ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp @@ -56,18 +56,19 @@ namespace typedef std::vector> ClientVecT; #if DO_LOGGING - void PrintRequest(const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_) + void PrintRequest(const std::string& method_, const eCAL::SDataTypeInformation& req_type_, const eCAL::SDataTypeInformation& resp_type_, const std::string& request_) { std::cout << "------ REQUEST -------" << std::endl; - std::cout << "Method name : " << method_ << std::endl; - std::cout << "Method request type : " << req_type_ << std::endl; - std::cout << "Method response type : " << resp_type_ << std::endl; - std::cout << "Method request type : " << resp_type_ << std::endl; - std::cout << "Method request : " << request_ << std::endl; + std::cout << "Method name : " << method_ << std::endl; + std::cout << "Method request type name : " << req_type_.name << std::endl; + std::cout << "Method request type encoding : " << req_type_.encoding << std::endl; + std::cout << "Method response type name : " << resp_type_.name << std::endl; + std::cout << "Method response type encoding : " << resp_type_.encoding << std::endl; + std::cout << "Method request : " << request_ << std::endl; std::cout << std::endl; } #else - void PrintRequest(const std::string& /*method_*/, const std::string& /*req_type_*/, const std::string& /*resp_type_*/, const std::string& /*request_*/) + void PrintRequest(const std::string& /*method_*/, const eCAL::SDataTypeInformation& /*req_type_*/, const eCAL::SDataTypeInformation& /*resp_type_*/, const std::string& /*request_*/) { } #endif @@ -258,7 +259,7 @@ TEST(core_cpp_clientserver, ClientServerBaseCallback) // method callback function std::atomic methods_executed(0); std::atomic method_process_time(0); - auto method_callback = [&](const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_, std::string& response_) -> int + auto method_callback = [&](const std::string& method_, const eCAL::SDataTypeInformation& req_type_, const eCAL::SDataTypeInformation& resp_type_, const std::string& request_, std::string& response_) -> int { eCAL::Process::SleepMS(method_process_time); PrintRequest(method_, req_type_, resp_type_, request_); @@ -363,7 +364,7 @@ TEST(core_cpp_clientserver, ClientServerBaseCallbackTimeout) // method callback function std::atomic methods_executed(0); std::atomic method_process_time(0); - auto method_callback = [&](const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_, std::string& response_) -> int + auto method_callback = [&](const std::string& method_, const eCAL::SDataTypeInformation& req_type_, const eCAL::SDataTypeInformation& resp_type_, const std::string& request_, std::string& response_) -> int { eCAL::Process::SleepMS(method_process_time); PrintRequest(method_, req_type_, resp_type_, request_); @@ -519,7 +520,7 @@ TEST(core_cpp_clientserver, ClientServerBaseAsyncCallback) // method callback function std::atomic methods_executed(0); - auto method_callback = [&](const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_, std::string& response_) -> int + auto method_callback = [&](const std::string& method_, const eCAL::SDataTypeInformation& req_type_, const eCAL::SDataTypeInformation& resp_type_, const std::string& request_, std::string& response_) -> int { PrintRequest(method_, req_type_, resp_type_, request_); response_ = "I answered on " + request_; @@ -593,7 +594,7 @@ TEST(core_cpp_clientserver, ClientServerBaseAsync) atomic_signalable num_service_callbacks_finished(0); int service_callback_time_ms(0); - auto method_callback = [&](const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_, std::string& response_) -> int + auto method_callback = [&](const std::string& method_, const eCAL::SDataTypeInformation& req_type_, const eCAL::SDataTypeInformation& resp_type_, const std::string& request_, std::string& response_) -> int { eCAL::Process::SleepMS(service_callback_time_ms); PrintRequest(method_, req_type_, resp_type_, request_); @@ -703,7 +704,7 @@ TEST(core_cpp_clientserver, ClientServerBaseBlocking) // method callback function std::atomic methods_executed(0); - auto method_callback = [&](const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_, std::string& response_) -> int + auto method_callback = [&](const std::string& method_, const eCAL::SDataTypeInformation& req_type_, const eCAL::SDataTypeInformation& resp_type_, const std::string& request_, std::string& response_) -> int { PrintRequest(method_, req_type_, resp_type_, request_); response_ = "I answer on " + request_; @@ -800,7 +801,7 @@ TEST(core_cpp_clientserver, NestedRPCCall) // request callback function std::atomic methods_executed(0); - auto method_callback = [&](const std::string& method_, const std::string& req_type_, const std::string& resp_type_, const std::string& request_, std::string& response_) -> int + auto method_callback = [&](const std::string& method_, const eCAL::SDataTypeInformation& req_type_, const eCAL::SDataTypeInformation& resp_type_, const std::string& request_, std::string& response_) -> int { PrintRequest(method_, req_type_, resp_type_, request_); response_ = "I answer on " + request_; diff --git a/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp b/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp index 1e80539523..0543fcaf34 100644 --- a/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp +++ b/ecal/tests/cpp/registration_test_public/src/registration_getservices.cpp @@ -61,7 +61,7 @@ TEST_P(ServicesTestFixture, ServiceExpiration) { // create service eCAL::CServiceServer service("foo::service"); - service.SetMethodCallback("foo::method", { { "foo::req_type", "foo::req_desc" }, { "foo::resp_type", "foo::resp_desc" } }, eCAL::MethodCallbackT()); + service.SetMethodCallback("foo::method", { { "foo::req_type", "foo::req_desc" }, { "foo::resp_type", "foo::resp_desc" } }, eCAL::MethodInfoCallbackT()); // let's register eCAL::Process::SleepMS(2 * CMN_REGISTRATION_REFRESH_MS); @@ -116,7 +116,7 @@ TEST_P(ServicesTestFixture, GetServiceIDs) service_method_info.request_type.descriptor = "foo::req_desc"; service_method_info.response_type.name = "foo::resp_type"; service_method_info.response_type.descriptor = "foo::resp_desc"; - service.SetMethodCallback("method", service_method_info, eCAL::MethodCallbackT()); + service.SetMethodCallback("method", service_method_info, eCAL::MethodInfoCallbackT()); // let's register eCAL::Process::SleepMS(2 * CMN_REGISTRATION_REFRESH_MS); From 06b19e0da34d81e51bd26d168d9b37a2417ee473 Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:43:55 +0100 Subject: [PATCH 3/6] clang tidy --- ecal/core/src/service/ecal_service_server_v5_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0a54fb51c5..05fc2115c0 100644 --- a/ecal/core/src/service/ecal_service_server_v5_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_v5_impl.cpp @@ -122,7 +122,7 @@ namespace eCAL method_info.request_type.name = req_type_; method_info.response_type.name = resp_type_; - MethodInfoCallbackT callback = + const MethodInfoCallbackT callback = [req_type_, resp_type_, callback_]( const std::string& method, const SDataTypeInformation& req_type_info, From ccdfdfde84b7ee92ccee7a1dfb61d64888dbd6ff Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Thu, 9 Jan 2025 08:53:53 +0100 Subject: [PATCH 4/6] apply/convert new request/response datatype from old single field if needed (downward compatibility to v5) --- ecal/core/src/ecal_descgate.cpp | 68 +++++++++++++++---- .../src/monitoring/ecal_monitoring_impl.cpp | 17 +++-- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/ecal/core/src/ecal_descgate.cpp b/ecal/core/src/ecal_descgate.cpp index 0a143f0643..12a830a05d 100644 --- a/ecal/core/src/ecal_descgate.cpp +++ b/ecal/core/src/ecal_descgate.cpp @@ -178,15 +178,34 @@ namespace eCAL { for (const auto& method : sample_.service.methods) { - SDataTypeInformation request_type{}; - request_type.name = method.req_type; - request_type.descriptor = method.req_desc; + SDataTypeInformation request_datatype{}; + SDataTypeInformation response_datatype{}; - SDataTypeInformation response_type{}; - response_type.name = method.resp_type; - response_type.descriptor = method.resp_desc; + // if the new req_datatype field is set, use it + if (!method.req_datatype.name.empty()) + { + request_datatype = method.req_datatype; + } + // otherwise use the old req_type and req_desc fields + else + { + request_datatype.name = method.req_type; + request_datatype.descriptor = method.req_desc; + } + + // if the new resp_datatype field is set, use it + if (!method.resp_datatype.name.empty()) + { + response_datatype = method.resp_datatype; + } + // otherwise use the old resp_type and resp_desc fields + else + { + response_datatype.name = method.resp_type; + response_datatype.descriptor = method.resp_desc; + } - ApplyServiceDescription(m_service_info_map, sample_.identifier, sample_.service.sname, method.mname, request_type, response_type); + ApplyServiceDescription(m_service_info_map, sample_.identifier, sample_.service.sname, method.mname, request_datatype, response_datatype); } } break; @@ -196,16 +215,35 @@ namespace eCAL case bct_reg_client: for (const auto& method : sample_.client.methods) { - SDataTypeInformation request_type; - request_type.name = method.req_type; - request_type.descriptor = method.req_desc; + SDataTypeInformation request_datatype{}; + SDataTypeInformation response_datatype{}; - SDataTypeInformation response_type{}; - response_type.name = method.resp_type; - response_type.descriptor = method.resp_desc; + // if the new req_datatype field is set, use it + if (!method.req_datatype.name.empty()) + { + request_datatype = method.req_datatype; + } + // otherwise use the old req_type and req_desc fields + else + { + request_datatype.name = method.req_type; + request_datatype.descriptor = method.req_desc; + } - ApplyServiceDescription(m_client_info_map, sample_.identifier, sample_.client.sname, method.mname, request_type, response_type); - } + // if the new resp_datatype field is set, use it + if (!method.resp_datatype.name.empty()) + { + response_datatype = method.resp_datatype; + } + // otherwise use the old resp_type and resp_desc fields + else + { + response_datatype.name = method.resp_type; + response_datatype.descriptor = method.resp_desc; + } + + ApplyServiceDescription(m_client_info_map, sample_.identifier, sample_.client.sname, method.mname, request_datatype, response_datatype); + } break; case bct_unreg_client: RemServiceDescription(m_client_info_map, sample_.identifier, sample_.client.sname); diff --git a/ecal/core/src/monitoring/ecal_monitoring_impl.cpp b/ecal/core/src/monitoring/ecal_monitoring_impl.cpp index aee83cfafe..bc62bd0d7e 100644 --- a/ecal/core/src/monitoring/ecal_monitoring_impl.cpp +++ b/ecal/core/src/monitoring/ecal_monitoring_impl.cpp @@ -419,12 +419,17 @@ namespace eCAL for (const auto& sample_service_method : sample_.service.methods) { struct Monitoring::SMethodMon method; - method.mname = sample_service_method.mname; - method.req_type = sample_service_method.req_type; - method.req_desc = sample_service_method.req_desc; - method.resp_type = sample_service_method.resp_type; - method.resp_desc = sample_service_method.resp_desc; - method.call_count = sample_service_method.call_count; + method.mname = sample_service_method.mname; + + method.req_type = sample_service_method.req_type; + method.req_desc = sample_service_method.req_desc; + method.resp_type = sample_service_method.resp_type; + method.resp_desc = sample_service_method.resp_desc; + + method.req_datatype = sample_service_method.req_datatype; + method.resp_datatype = sample_service_method.resp_datatype; + + method.call_count = sample_service_method.call_count; ServerInfo.methods.push_back(method); } From b1c89e517b75fc2e19f28d83b1df5a1e866a2e3d Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Thu, 9 Jan 2025 08:54:51 +0100 Subject: [PATCH 5/6] rename AddMethodCallback to SetMethodCallback (to align internal implementation naming to public API) --- ecal/core/src/service/ecal_service_server.cpp | 2 +- ecal/core/src/service/ecal_service_server_impl.cpp | 8 ++++---- ecal/core/src/service/ecal_service_server_impl.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ecal/core/src/service/ecal_service_server.cpp b/ecal/core/src/service/ecal_service_server.cpp index e3fe517b43..e952b9032d 100644 --- a/ecal/core/src/service/ecal_service_server.cpp +++ b/ecal/core/src/service/ecal_service_server.cpp @@ -65,7 +65,7 @@ namespace eCAL bool CServiceServer::SetMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_) { if (m_service_server_impl == nullptr) return false; - return m_service_server_impl->AddMethodCallback(method_, method_info_, callback_); + return m_service_server_impl->SetMethodCallback(method_, method_info_, callback_); } bool CServiceServer::RemoveMethodCallback(const std::string& method_) diff --git a/ecal/core/src/service/ecal_service_server_impl.cpp b/ecal/core/src/service/ecal_service_server_impl.cpp index d123b2787c..efc1586199 100644 --- a/ecal/core/src/service/ecal_service_server_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_impl.cpp @@ -66,17 +66,17 @@ namespace eCAL Stop(); } - bool CServiceServerImpl::AddMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_) + bool CServiceServerImpl::SetMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_) { #ifndef NDEBUG - Logging::Log(log_level_debug1, "CServiceServerImpl::AddMethodCallback: Adding method callback for method: " + method_); + Logging::Log(log_level_debug1, "CServiceServerImpl::SetMethodCallback: Adding method callback for method: " + method_); #endif const std::lock_guard lock(m_method_map_mutex); auto iter = m_method_map.find(method_); if (iter != m_method_map.end()) { - Logging::Log(log_level_warning, "CServiceServerImpl::AddMethodCallback: Method already exists, updating callback: " + method_); + Logging::Log(log_level_warning, "CServiceServerImpl::SetMethodCallback: Method already exists, updating callback: " + method_); // old type and descriptor fields iter->second.method.req_type = method_info_.request_type.name; @@ -94,7 +94,7 @@ namespace eCAL else { #ifndef NDEBUG - Logging::Log(log_level_debug1, "CServiceServerImpl::AddMethodCallback: Registering new method: " + method_); + Logging::Log(log_level_debug1, "CServiceServerImpl::SetMethodCallback: Registering new method: " + method_); #endif SMethod method; // method name diff --git a/ecal/core/src/service/ecal_service_server_impl.h b/ecal/core/src/service/ecal_service_server_impl.h index 5d7e07edff..6c800c837d 100644 --- a/ecal/core/src/service/ecal_service_server_impl.h +++ b/ecal/core/src/service/ecal_service_server_impl.h @@ -55,7 +55,7 @@ namespace eCAL public: ~CServiceServerImpl(); - bool AddMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_); + bool SetMethodCallback(const std::string& method_, const SServiceMethodInformation& method_info_, const MethodInfoCallbackT& callback_); bool RemoveMethodCallback(const std::string& method_); // Check connection state of a specific service From 7194274c67f7bfff5838d9581211a861698beabe Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:29:07 +0100 Subject: [PATCH 6/6] single fields req_type, req_desc, resp_type and resp_desc removed from SMethodMon ugly logic added to make new service server implementation compatible to old v5 API (where AddDescription may be called any time separately) python API extended with datatype information for pub/sub and server/client --- ecal/core/include/ecal/types/monitoring.h | 5 -- .../src/monitoring/ecal_monitoring_impl.cpp | 11 +-- .../ecal_serialize_monitoring.cpp | 9 -- .../src/service/ecal_service_server_impl.cpp | 88 ++++++++++++++----- .../src/monitoring_compare.cpp | 8 -- .../src/monitoring_generate.cpp | 7 +- lang/python/core/src/ecal_wrap.cxx | 43 ++++++--- 7 files changed, 103 insertions(+), 68 deletions(-) diff --git a/ecal/core/include/ecal/types/monitoring.h b/ecal/core/include/ecal/types/monitoring.h index ba7051c8a5..d90d33d049 100644 --- a/ecal/core/include/ecal/types/monitoring.h +++ b/ecal/core/include/ecal/types/monitoring.h @@ -135,11 +135,6 @@ namespace eCAL { std::string mname; //second.method.req_type = method_info_.request_type.name; - iter->second.method.req_desc = method_info_.request_type.descriptor; - iter->second.method.resp_type = method_info_.response_type.name; - iter->second.method.resp_desc = method_info_.response_type.descriptor; - - // new type and descriptor fields + Logging::Log(log_level_warning, "CServiceServerImpl::SetMethodCallback: Method already exists, updating attributes and callback: " + method_); + +#if 0 // this is how it should look like if we do not use the old type and descriptor fields + // update data type and callback iter->second.method.req_datatype = method_info_.request_type; iter->second.method.resp_datatype = method_info_.response_type; + iter->second.callback = callback_; +#else + ///////////////////////////////////////////// + // old types and descriptors + ///////////////////////////////////////////// + iter->second.method.req_type = method_info_.request_type.name; + iter->second.method.resp_type = method_info_.response_type.name; + + // we need to check these fields, because the v5 implementation is using SetMethodCallback with partially filled fields + if (!method_info_.request_type.descriptor.empty()) iter->second.method.req_desc = method_info_.request_type.descriptor; + if (!method_info_.response_type.descriptor.empty()) iter->second.method.resp_desc = method_info_.response_type.descriptor; - // callback - iter->second.callback = callback_; + ///////////////////////////////////////////// + // new types, encodings and descriptors + ///////////////////////////////////////////// + iter->second.method.req_datatype.name = method_info_.request_type.name; + iter->second.method.resp_datatype.name = method_info_.response_type.name; + + // we need to check these fields, because the v5 implementation is using SetMethodCallback with partially filled fields + if (!method_info_.request_type.encoding.empty()) iter->second.method.req_datatype.encoding = method_info_.request_type.encoding; + if (!method_info_.response_type.encoding.empty()) iter->second.method.resp_datatype.encoding = method_info_.response_type.encoding; + if (!method_info_.request_type.descriptor.empty()) iter->second.method.req_datatype.descriptor = method_info_.request_type.descriptor; + if (!method_info_.response_type.descriptor.empty()) iter->second.method.resp_datatype.descriptor = method_info_.response_type.descriptor; + + // we need to do this ugly hack here, because the v5 implementation is using SetMethodCallback with nullptr to update descriptions (AddDescription) + if (callback_ != nullptr) + { + iter->second.callback = callback_; + } +#endif } else { @@ -99,19 +121,43 @@ namespace eCAL SMethod method; // method name method.method.mname = method_; - - // old type and descriptor fields - method.method.req_type = method_info_.request_type.name; - method.method.req_desc = method_info_.request_type.descriptor; - method.method.resp_type = method_info_.response_type.name; - method.method.resp_desc = method_info_.response_type.descriptor; - // new type and descriptor fields +#if 0 // this is how it should look like if we do not use the old type and descriptor fields + // set data type and callback method.method.req_datatype = method_info_.request_type; method.method.resp_datatype = method_info_.response_type; - - // callback - method.callback = callback_; + method.callback = callback_; +#else +#endif + ///////////////////////////////////////////// + // old types and descriptors + ///////////////////////////////////////////// + method.method.req_type = method_info_.request_type.name; + method.method.resp_type = method_info_.response_type.name; + + // we need to check these fields, because the v5 implementation is using SetMethodCallback with partially filled fields + if (!method_info_.request_type.descriptor.empty()) method.method.req_desc = method_info_.request_type.descriptor; + if (!method_info_.response_type.descriptor.empty()) method.method.resp_desc = method_info_.response_type.descriptor; + + ///////////////////////////////////////////// + // new types, encodings and descriptors + ///////////////////////////////////////////// + method.method.req_datatype.name = method_info_.request_type.name; + method.method.resp_datatype.name = method_info_.response_type.name; + + // we need to check these fields, because the v5 implementation is using SetMethodCallback with partially filled fields + if (!method_info_.request_type.encoding.empty()) method.method.req_datatype.encoding = method_info_.request_type.encoding; + if (!method_info_.response_type.encoding.empty()) method.method.resp_datatype.encoding = method_info_.response_type.encoding; + if (!method_info_.request_type.descriptor.empty()) method.method.req_datatype.descriptor = method_info_.request_type.descriptor; + if (!method_info_.response_type.descriptor.empty()) method.method.resp_datatype.descriptor = method_info_.response_type.descriptor; + + // we need to do this ugly hack here, because the v5 implementation is using SetMethodCallback with nullptr to update descriptions (AddDescription) + if (callback_ != nullptr) + { + method.callback = callback_; + } + + // apply new method m_method_map[method_] = method; } diff --git a/ecal/tests/cpp/serialization_test/src/monitoring_compare.cpp b/ecal/tests/cpp/serialization_test/src/monitoring_compare.cpp index c744f447a1..21913181b3 100644 --- a/ecal/tests/cpp/serialization_test/src/monitoring_compare.cpp +++ b/ecal/tests/cpp/serialization_test/src/monitoring_compare.cpp @@ -147,10 +147,6 @@ namespace eCAL for (size_t j = 0; j < monitoring1.server[i].methods.size(); ++j) { if (monitoring1.server[i].methods[j].mname != monitoring2.server[i].methods[j].mname || - monitoring1.server[i].methods[j].req_type != monitoring2.server[i].methods[j].req_type || - monitoring1.server[i].methods[j].req_desc != monitoring2.server[i].methods[j].req_desc || - monitoring1.server[i].methods[j].resp_type != monitoring2.server[i].methods[j].resp_type || - monitoring1.server[i].methods[j].resp_desc != monitoring2.server[i].methods[j].resp_desc || monitoring1.server[i].methods[j].req_datatype != monitoring2.server[i].methods[j].req_datatype || monitoring1.server[i].methods[j].resp_datatype != monitoring2.server[i].methods[j].resp_datatype || monitoring1.server[i].methods[j].call_count != monitoring2.server[i].methods[j].call_count) @@ -184,10 +180,6 @@ namespace eCAL for (size_t j = 0; j < monitoring1.clients[i].methods.size(); ++j) { if (monitoring1.clients[i].methods[j].mname != monitoring2.clients[i].methods[j].mname || - monitoring1.clients[i].methods[j].req_type != monitoring2.clients[i].methods[j].req_type || - monitoring1.clients[i].methods[j].req_desc != monitoring2.clients[i].methods[j].req_desc || - monitoring1.clients[i].methods[j].resp_type != monitoring2.clients[i].methods[j].resp_type || - monitoring1.clients[i].methods[j].resp_desc != monitoring2.clients[i].methods[j].resp_desc || monitoring1.clients[i].methods[j].req_datatype != monitoring2.clients[i].methods[j].req_datatype || monitoring1.clients[i].methods[j].resp_datatype != monitoring2.clients[i].methods[j].resp_datatype || monitoring1.clients[i].methods[j].call_count != monitoring2.clients[i].methods[j].call_count) diff --git a/ecal/tests/cpp/serialization_test/src/monitoring_generate.cpp b/ecal/tests/cpp/serialization_test/src/monitoring_generate.cpp index 0d50929924..c0590e8817 100644 --- a/ecal/tests/cpp/serialization_test/src/monitoring_generate.cpp +++ b/ecal/tests/cpp/serialization_test/src/monitoring_generate.cpp @@ -79,12 +79,7 @@ namespace eCAL SMethodMon GenerateServiceMethod() { SMethodMon method; - method.mname = GenerateString(8); - - method.req_type = GenerateString(8); // deprecated - method.req_desc = GenerateString(10); // deprecated - method.resp_type = GenerateString(8); // deprecated - method.resp_desc = GenerateString(10); // deprecated + method.mname = GenerateString(8); method.req_datatype = eCAL::Registration::GenerateDataTypeInformation(); method.resp_datatype = eCAL::Registration::GenerateDataTypeInformation(); diff --git a/lang/python/core/src/ecal_wrap.cxx b/lang/python/core/src/ecal_wrap.cxx index 999cd232c6..a269be22b5 100644 --- a/lang/python/core/src/ecal_wrap.cxx +++ b/lang/python/core/src/ecal_wrap.cxx @@ -1021,7 +1021,14 @@ namespace val = Py_BuildValue("s", topic.direction.c_str()); PyDict_SetItemString(topicDict, "direction", val); Py_DECREF(val); - // TODO: SDataTypeInformation tdatatype + val = Py_BuildValue("s", topic.tdatatype.name.c_str()); + PyDict_SetItemString(topicDict, "tdatatype_name", val); Py_DECREF(val); + + val = Py_BuildValue("s", topic.tdatatype.encoding.c_str()); + PyDict_SetItemString(topicDict, "tdatatype_encoding", val); Py_DECREF(val); + + val = Py_BuildValue("y#", topic.tdatatype.descriptor.c_str(), topic.tdatatype.descriptor.length()); + PyDict_SetItemString(topicDict, "tdatatype_descriptor", val); Py_DECREF(val); // TODO: std::vector tlayer @@ -1165,15 +1172,23 @@ PyObject* mon_monitoring(PyObject* /*self*/, PyObject* /*args*/) val = Py_BuildValue("s", method.mname.c_str()); PyDict_SetItemString(methodsDict, "mname", val); Py_DECREF(val); - val = Py_BuildValue("s", method.req_type.c_str()); + val = Py_BuildValue("s", method.req_datatype.name.c_str()); PyDict_SetItemString(methodsDict, "req_type", val); Py_DECREF(val); - // TODO: std::string req_desc + val = Py_BuildValue("s", method.req_datatype.encoding.c_str()); + PyDict_SetItemString(methodsDict, "req_encoding", val); Py_DECREF(val); - val = Py_BuildValue("s", method.resp_type.c_str()); + val = Py_BuildValue("y#", method.req_datatype.descriptor.c_str(), method.req_datatype.descriptor.length()); + PyDict_SetItemString(methodsDict, "req_descriptor", val); Py_DECREF(val); + + val = Py_BuildValue("s", method.resp_datatype.name.c_str()); PyDict_SetItemString(methodsDict, "resp_type", val); Py_DECREF(val); - // TODO: std::string resp_desc + val = Py_BuildValue("s", method.resp_datatype.encoding.c_str()); + PyDict_SetItemString(methodsDict, "resp_encoding", val); Py_DECREF(val); + + val = Py_BuildValue("y#", method.resp_datatype.descriptor.c_str(), method.resp_datatype.descriptor.length()); + PyDict_SetItemString(methodsDict, "resp_descriptor", val); Py_DECREF(val); val = Py_BuildValue("i", method.call_count); PyDict_SetItemString(methodsDict, "call_count", val); Py_DECREF(val); @@ -1220,15 +1235,23 @@ PyObject* mon_monitoring(PyObject* /*self*/, PyObject* /*args*/) val = Py_BuildValue("s", method.mname.c_str()); PyDict_SetItemString(methodsDict, "mname", val); Py_DECREF(val); - val = Py_BuildValue("s", method.req_type.c_str()); + val = Py_BuildValue("s", method.req_datatype.name.c_str()); PyDict_SetItemString(methodsDict, "req_type", val); Py_DECREF(val); - // TODO: std::string req_desc - - val = Py_BuildValue("s", method.resp_type.c_str()); + val = Py_BuildValue("s", method.req_datatype.encoding.c_str()); + PyDict_SetItemString(methodsDict, "req_encoding", val); Py_DECREF(val); + + val = Py_BuildValue("y#", method.req_datatype.descriptor.c_str(), method.req_datatype.descriptor.length()); + PyDict_SetItemString(methodsDict, "req_descriptor", val); Py_DECREF(val); + + val = Py_BuildValue("s", method.resp_datatype.name.c_str()); PyDict_SetItemString(methodsDict, "resp_type", val); Py_DECREF(val); - // TODO: std::string resp_desc + val = Py_BuildValue("s", method.resp_datatype.encoding.c_str()); + PyDict_SetItemString(methodsDict, "resp_encoding", val); Py_DECREF(val); + + val = Py_BuildValue("y#", method.resp_datatype.descriptor.c_str(), method.resp_datatype.descriptor.length()); + PyDict_SetItemString(methodsDict, "resp_descriptor", val); Py_DECREF(val); val = Py_BuildValue("i", method.call_count); PyDict_SetItemString(methodsDict, "call_count", val); Py_DECREF(val);