From ae5fcde2456318bfdfa89ee948f093f63cf162ca Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Fri, 31 Jan 2025 09:57:30 +0100 Subject: [PATCH] timeout argument fixed for CClientInstance as well --- ecal/core/include/ecal/service/client.h | 8 ++++---- ecal/core/include/ecal/service/client_instance.h | 11 +++++++---- ecal/core/src/service/ecal_service_client.cpp | 4 ++-- ecal/core/src/service/ecal_service_client_impl.cpp | 5 +++-- ecal/core/src/service/ecal_service_client_impl.h | 2 +- .../core/src/service/ecal_service_client_instance.cpp | 6 +++--- ecal/core/src/v5/service/ecal_service_client_impl.cpp | 2 +- .../services/minimal_client/src/minimal_client.cpp | 2 +- 8 files changed, 22 insertions(+), 18 deletions(-) diff --git a/ecal/core/include/ecal/service/client.h b/ecal/core/include/ecal/service/client.h index 809e709326..c7bb454ba7 100644 --- a/ecal/core/include/ecal/service/client.h +++ b/ecal/core/include/ecal/service/client.h @@ -100,12 +100,12 @@ namespace eCAL * @param method_name_ Method name. * @param request_ Request string. * @param [out] service_response_vec_ Response vector containing service responses from every called service (null pointer == no response). - * @param timeout_ Maximum time before operation returns (in milliseconds, DEFAULT_TIME_ARGUMENT means infinite). + * @param timeout_ms_ Maximum time before operation returns (in milliseconds, DEFAULT_TIME_ARGUMENT means infinite). * * @return True if all calls were successful. **/ ECAL_API_EXPORTED_MEMBER - bool CallWithResponse(const std::string& method_name_, const std::string& request_, ServiceResponseVecT& service_response_vec_, int timeout_ = DEFAULT_TIME_ARGUMENT) const; + bool CallWithResponse(const std::string& method_name_, const std::string& request_, ServiceResponseVecT& service_response_vec_, int timeout_ms_ = DEFAULT_TIME_ARGUMENT) const; /** * @brief Blocking call (with timeout) of a service method for all existing service instances, using callback @@ -113,12 +113,12 @@ namespace eCAL * @param method_name_ Method name. * @param request_ Request string. * @param response_callback_ Callback function for the service method response. - * @param timeout_ Maximum time before operation returns (in milliseconds, DEFAULT_TIME_ARGUMENT means infinite). + * @param timeout_ms_ Maximum time before operation returns (in milliseconds, DEFAULT_TIME_ARGUMENT means infinite). * * @return True if all calls were successful. **/ ECAL_API_EXPORTED_MEMBER - bool CallWithCallback(const std::string& method_name_, const std::string& request_, const ResponseCallbackT& response_callback_, int timeout_ = DEFAULT_TIME_ARGUMENT) const; + bool CallWithCallback(const std::string& method_name_, const std::string& request_, const ResponseCallbackT& response_callback_, int timeout_ms_ = DEFAULT_TIME_ARGUMENT) const; /** * @brief Asynchronous call of a service method for all existing service instances, using callback diff --git a/ecal/core/include/ecal/service/client_instance.h b/ecal/core/include/ecal/service/client_instance.h index 17130f3545..f4bec9696b 100644 --- a/ecal/core/include/ecal/service/client_instance.h +++ b/ecal/core/include/ecal/service/client_instance.h @@ -40,6 +40,9 @@ namespace eCAL class ECAL_API_CLASS CClientInstance final { public: + ECAL_API_EXPORTED_MEMBER + static constexpr long long DEFAULT_TIME_ARGUMENT = -1; /*!< Use DEFAULT_TIME_ARGUMENT in the `CallWithResponse()` and `CallWithCallback()` functions for blocking calls */ + // Constructor ECAL_API_EXPORTED_MEMBER CClientInstance(const SEntityId& entity_id_, const std::shared_ptr& service_client_id_impl_); @@ -60,25 +63,25 @@ namespace eCAL * * @param method_name_ Method name. * @param request_ Request string. - * @param timeout_ Maximum time before operation returns (in milliseconds, -1 means infinite). + * @param timeout_ms_ Maximum time before operation returns (in milliseconds, DEFAULT_TIME_ARGUMENT means infinite). * * @return success state and service response **/ ECAL_API_EXPORTED_MEMBER - std::pair CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_ = -1); + std::pair CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_ms_ = DEFAULT_TIME_ARGUMENT); /** * @brief Blocking call of a service method, using callback * * @param method_name_ Method name. * @param request_ Request string. - * @param timeout_ Maximum time before operation returns (in milliseconds, -1 means infinite). * @param response_callback_ Callback function for the service method response. + * @param timeout_ms_ Maximum time before operation returns (in milliseconds, DEFAULT_TIME_ARGUMENT means infinite). * * @return True if successful. **/ ECAL_API_EXPORTED_MEMBER - bool CallWithCallback(const std::string& method_name_, const std::string& request_, int timeout_, const ResponseCallbackT& response_callback_); + bool CallWithCallback(const std::string& method_name_, const std::string& request_, const ResponseCallbackT& response_callback_, int timeout_ms_ = DEFAULT_TIME_ARGUMENT); /** * @brief Asynchronous call of a service method, using callback diff --git a/ecal/core/src/service/ecal_service_client.cpp b/ecal/core/src/service/ecal_service_client.cpp index 26e52af07f..2440fc01fc 100644 --- a/ecal/core/src/service/ecal_service_client.cpp +++ b/ecal/core/src/service/ecal_service_client.cpp @@ -145,9 +145,9 @@ namespace eCAL for (auto& instance : instances) { futures.emplace_back(std::async(std::launch::async, - [&instance, method_name_ = method_name_, request_ = request_, timeout_, response_callback_]() + [&instance, method_name_ = method_name_, request_ = request_, response_callback_, timeout_]() { - return instance.CallWithCallback(method_name_, request_, timeout_, response_callback_); + return instance.CallWithCallback(method_name_, request_, response_callback_, timeout_); })); } diff --git a/ecal/core/src/service/ecal_service_client_impl.cpp b/ecal/core/src/service/ecal_service_client_impl.cpp index 1fde23c5c1..172099602e 100644 --- a/ecal/core/src/service/ecal_service_client_impl.cpp +++ b/ecal/core/src/service/ecal_service_client_impl.cpp @@ -162,10 +162,11 @@ namespace eCAL return entity_vector; } + // TODO: We need to reimplment this function. It makes no sense to call a service with response callback and to return a pair // Calls a service method synchronously, blocking until a response is received or timeout occurs std::pair CServiceClientImpl::CallWithCallback( - const SEntityId & entity_id_, const std::string & method_name_, - const std::string & request_, int timeout_ms_, const ResponseCallbackT & response_callback_) + const SEntityId& entity_id_, const std::string& method_name_, + const std::string& request_, const ResponseCallbackT& response_callback_, int timeout_ms_) { #ifndef NDEBUG eCAL::Logging::Log(eCAL::Logging::log_level_debug1, "CServiceClientImpl::CallWithCallback: Performing synchronous call for service: " + m_service_name + ", method: " + method_name_); diff --git a/ecal/core/src/service/ecal_service_client_impl.h b/ecal/core/src/service/ecal_service_client_impl.h index 803bd1ff32..ba8f8ad1a1 100644 --- a/ecal/core/src/service/ecal_service_client_impl.h +++ b/ecal/core/src/service/ecal_service_client_impl.h @@ -62,7 +62,7 @@ namespace eCAL // if a callback is provided call the callback as well std::pair CallWithCallback( const SEntityId& entity_id_, const std::string& method_name_, - const std::string& request_, int timeout_ms_, const ResponseCallbackT& response_callback_ = nullptr); + const std::string& request_, const ResponseCallbackT& response_callback_, int timeout_ms_); // Asynchronous call to a specific service using callback bool CallWithCallbackAsync( diff --git a/ecal/core/src/service/ecal_service_client_instance.cpp b/ecal/core/src/service/ecal_service_client_instance.cpp index a069e9d8c0..827fe6cd77 100644 --- a/ecal/core/src/service/ecal_service_client_instance.cpp +++ b/ecal/core/src/service/ecal_service_client_instance.cpp @@ -34,12 +34,12 @@ namespace eCAL std::pair CClientInstance::CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_) { - return m_service_client_impl->CallWithCallback(m_entity_id, method_name_, request_, timeout_); + return m_service_client_impl->CallWithCallback(m_entity_id, method_name_, request_, nullptr, timeout_); } - bool CClientInstance::CallWithCallback(const std::string& method_name_, const std::string& request_, int timeout_, const ResponseCallbackT& response_callback_) + bool CClientInstance::CallWithCallback(const std::string& method_name_, const std::string& request_, const ResponseCallbackT& response_callback_, int timeout_) { - auto response = m_service_client_impl->CallWithCallback(m_entity_id, method_name_, request_, timeout_, response_callback_); + auto response = m_service_client_impl->CallWithCallback(m_entity_id, method_name_, request_, response_callback_, timeout_); return response.first; } diff --git a/ecal/core/src/v5/service/ecal_service_client_impl.cpp b/ecal/core/src/v5/service/ecal_service_client_impl.cpp index 11767031ed..73e5410073 100644 --- a/ecal/core/src/v5/service/ecal_service_client_impl.cpp +++ b/ecal/core/src/v5/service/ecal_service_client_impl.cpp @@ -204,7 +204,7 @@ namespace eCAL { if (instance.GetClientID().host_name == m_host_name || m_host_name.empty()) { - success |= instance.CallWithCallback(method_name_, request_, timeout_, callback); + success |= instance.CallWithCallback(method_name_, request_, callback, timeout_); } } // Call the method using the new API diff --git a/ecal/samples/cpp/services/minimal_client/src/minimal_client.cpp b/ecal/samples/cpp/services/minimal_client/src/minimal_client.cpp index ae8a457a1c..fd78fad525 100644 --- a/ecal/samples/cpp/services/minimal_client/src/minimal_client.cpp +++ b/ecal/samples/cpp/services/minimal_client/src/minimal_client.cpp @@ -101,7 +101,7 @@ int main() ////////////////////////////////////// // Service call (with callback) ////////////////////////////////////// - if (client_instance.CallWithCallback(method_name, request, -1, service_response_callback)) + if (client_instance.CallWithCallback(method_name, request, service_response_callback)) { std::cout << std::endl << "Method 'echo' called with message : " << request << std::endl; }