Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] introduced default timeout parameter for client service method calls #1984

Merged
merged 2 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading