Skip to content

Commit

Permalink
[core] introduced default timeout parameter for client service method…
Browse files Browse the repository at this point in the history
… calls (#1984)
  • Loading branch information
rex-schilasky authored Jan 31, 2025
1 parent d9faad3 commit b7dc7ce
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 38 deletions.
15 changes: 9 additions & 6 deletions ecal/core/include/ecal/service/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ namespace eCAL
class ECAL_API_CLASS CServiceClient
{
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 */

/**
* @brief Constructor.
*
Expand Down Expand Up @@ -94,28 +97,28 @@ namespace eCAL
/**
* @brief Blocking call of a service method for all existing service instances, response will be returned as ServiceResponseVecT
*
* @param method_name_ Method name.
* @param request_ Request string.
* @param timeout_ Maximum time before operation returns (in milliseconds, -1 means infinite).
* @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_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_, int timeout_, ServiceResponseVecT& service_response_vec_) 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 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 all calls were successful.
**/
ECAL_API_EXPORTED_MEMBER
bool CallWithCallback(const std::string& method_name_, const std::string& request_, int timeout_, const ResponseCallbackT& response_callback_) 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
Expand Down
11 changes: 7 additions & 4 deletions ecal/core/include/ecal/service/client_instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions ecal/core/src/service/ecal_service_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ namespace eCAL
return instances;
}

bool CServiceClient::CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_, ServiceResponseVecT& service_response_vec_) const
bool CServiceClient::CallWithResponse(const std::string& method_name_, const std::string& request_, ServiceResponseVecT& service_response_vec_, int timeout_) const
{
auto instances = GetClientInstances();
size_t num_instances = instances.size();
Expand Down Expand Up @@ -133,7 +133,7 @@ namespace eCAL
return overall_success;
}

bool CServiceClient::CallWithCallback(const std::string& method_name_, const std::string& request_, int timeout_, const ResponseCallbackT& response_callback_) const
bool CServiceClient::CallWithCallback(const std::string& method_name_, const std::string& request_, const ResponseCallbackT& response_callback_, int timeout_) const
{
auto instances = GetClientInstances();
size_t num_instances = instances.size();
Expand All @@ -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_);
}));
}

Expand Down
5 changes: 3 additions & 2 deletions ecal/core/src/service/ecal_service_client_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/service/ecal_service_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions ecal/core/src/service/ecal_service_client_instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/v5/service/ecal_service_client_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ int main()
// call service method "hello" subcalls times (for better time accuracy)
for (auto i = 0; i < subcalls; ++i)
{
latency_client.CallWithCallback("hello", "", -1, eCAL::ResponseCallbackT());
latency_client.CallWithCallback("hello", "", eCAL::ResponseCallbackT());
}

// take return time and store it into the latency array
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
28 changes: 14 additions & 14 deletions ecal/tests/cpp/clientserver_test/src/clientserver_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,11 @@ TEST(core_cpp_clientserver, ClientServerBaseCallback)
for (const auto& client : client_vec)
{
// call method 1
success &= client->CallWithCallback("foo::method1", "my request for method 1", -1, response_callback);
success &= client->CallWithCallback("foo::method1", "my request for method 1", response_callback);
methods_called++;

// call method 2
success &= client->CallWithCallback("foo::method2", "my request for method 2", -1, response_callback);
success &= client->CallWithCallback("foo::method2", "my request for method 2", response_callback);
methods_called++;
}
}
Expand All @@ -323,11 +323,11 @@ TEST(core_cpp_clientserver, ClientServerBaseCallback)
for (const auto& client : client_vec)
{
// call method 1
success &= client->CallWithCallback("foo::method1", "my request for method 1", -1, response_callback);
success &= client->CallWithCallback("foo::method1", "my request for method 1", response_callback);
methods_called++;

// call method 2
success &= client->CallWithCallback("foo::method2", "my request for method 2", -1, response_callback);
success &= client->CallWithCallback("foo::method2", "my request for method 2", response_callback);
methods_called++;
}
}
Expand Down Expand Up @@ -425,11 +425,11 @@ TEST(core_cpp_clientserver, ClientServerBaseCallbackTimeout)
for (const auto& client : client_vec)
{
// call method 1
success &= client->CallWithCallback("foo::method1", "my request for method 1", -1, response_callback);
success &= client->CallWithCallback("foo::method1", "my request for method 1", response_callback);
methods_called++;

// call method 2
success &= client->CallWithCallback("foo::method2", "my request for method 2", -1, response_callback);
success &= client->CallWithCallback("foo::method2", "my request for method 2", response_callback);
methods_called++;
}
}
Expand All @@ -454,11 +454,11 @@ TEST(core_cpp_clientserver, ClientServerBaseCallbackTimeout)
for (const auto& client : client_vec)
{
// call method 1
success &= client->CallWithCallback("foo::method1", "my request for method 1", method_process_time * 4, response_callback);
success &= client->CallWithCallback("foo::method1", "my request for method 1", response_callback, method_process_time * 4);
methods_called++;

// call method 2
success &= client->CallWithCallback("foo::method2", "my request for method 2", method_process_time * 4, response_callback);
success &= client->CallWithCallback("foo::method2", "my request for method 2", response_callback, method_process_time * 4);
methods_called++;
}
}
Expand All @@ -483,12 +483,12 @@ TEST(core_cpp_clientserver, ClientServerBaseCallbackTimeout)
for (const auto& client : client_vec)
{
// call method 1
success &= client->CallWithCallback("foo::method1", "my request for method 1", method_process_time / 10, response_callback);
success &= client->CallWithCallback("foo::method1", "my request for method 1", response_callback, method_process_time / 10);
eCAL::Process::SleepMS(method_process_time * 4);
methods_called++;

// call method 2
success &= client->CallWithCallback("foo::method2", "my request for method 2", method_process_time / 10, response_callback);
success &= client->CallWithCallback("foo::method2", "my request for method 2", response_callback, method_process_time / 10);
eCAL::Process::SleepMS(method_process_time * 4);
methods_called++;
}
Expand Down Expand Up @@ -741,7 +741,7 @@ TEST(core_cpp_clientserver, ClientServerBaseBlocking)
for (const auto& client : client_vec)
{
// call method 1
if (client->CallWithResponse("foo::method1", "my request for method 1", -1, service_response_vec))
if (client->CallWithResponse("foo::method1", "my request for method 1", service_response_vec))
{
ASSERT_EQ(2, service_response_vec.size());

Expand All @@ -755,7 +755,7 @@ TEST(core_cpp_clientserver, ClientServerBaseBlocking)
}

// call method 2
if (client->CallWithResponse("foo::method2", "my request for method 2", -1, service_response_vec))
if (client->CallWithResponse("foo::method2", "my request for method 2", service_response_vec))
{
ASSERT_EQ(2, service_response_vec.size());

Expand Down Expand Up @@ -831,7 +831,7 @@ TEST(core_cpp_clientserver, NestedRPCCall)
auto response_callback1 = [&](const struct eCAL::SServiceResponse& service_response_)
{
PrintResponse(service_response_);
success &= client2.CallWithCallback("foo::method2", "my request for method 2", -1, response_callback2);
success &= client2.CallWithCallback("foo::method2", "my request for method 2", response_callback2);
methods_called++;
responses_executed++;
};
Expand All @@ -843,7 +843,7 @@ TEST(core_cpp_clientserver, NestedRPCCall)
for (auto i = 0; i < calls; ++i)
{
// call method 1
success &= client1.CallWithCallback("foo::method1", "my request for method 1", -1, response_callback1);
success &= client1.CallWithCallback("foo::method1", "my request for method 1", response_callback1);
eCAL::Process::SleepMS(sleep);
methods_called++;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ int main()
{
// call Ping service method
eCAL::ServiceResponseVecT service_response_vec;
if (ping_client.CallWithResponse("Ping", ping_request, -1, service_response_vec))
if (ping_client.CallWithResponse("Ping", ping_request, service_response_vec))
{
std::cout << '\n' << "PingService::Ping method called with message (JSON) : " << req_json << '\n';

Expand Down

0 comments on commit b7dc7ce

Please sign in to comment.