timeout argument fixed for CClientInstance as well

eclipse-ecal · Jan 31, 2025 · ae5fcde · ae5fcde
1 parent 67fe0e5
commit ae5fcde
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 18 deletions.
diff --git a/ecal/core/include/ecal/service/client.h b/ecal/core/include/ecal/service/client.h
@@ -100,25 +100,25 @@ 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
      *
      * @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
@@ -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<CServiceClientImpl>& 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<bool, SServiceResponse> CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_ = -1);
+      std::pair<bool, SServiceResponse> 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
@@ -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
@@ -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<bool, SServiceResponse>
   // Calls a service method synchronously, blocking until a response is received or timeout occurs
   std::pair<bool, SServiceResponse> 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
@@ -62,7 +62,7 @@ namespace eCAL
       // if a callback is provided call the callback as well
       std::pair<bool, SServiceResponse> 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
@@ -34,12 +34,12 @@ namespace eCAL
 
   std::pair<bool, SServiceResponse> 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
@@ -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
@@ -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;
       }