Skip to content

Commit

Permalink
refactoring and minor bug fix in clientgate (unique_lock)
Browse files Browse the repository at this point in the history
  • Loading branch information
rex-schilasky committed Nov 28, 2024
1 parent 2a73825 commit 18f1da3
Show file tree
Hide file tree
Showing 10 changed files with 338 additions and 270 deletions.
7 changes: 4 additions & 3 deletions ecal/core/include/ecal/ecal_client_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,20 @@ namespace eCAL
* @return Vector of client instances
**/
ECAL_API_EXPORTED_MEMBER
std::vector<CServiceClientInstance> GetServiceClientInstances() const;
std::vector<CClientInstance> GetClientInstances() const;

/**
* @brief Blocking call of a service method for all existing service instances, response will be returned as vector<pair<bool, SServiceReponse>>
*
* @param method_name_ Method name.
* @param request_ Request string.
* @param timeout_ Maximum time before operation returns (in milliseconds, -1 means infinite).
* @param [out] service_response_vec_ Response vector containing service responses from every called service (null pointer == no response).
*
* @return vector of success states and service responses
* @return True if all calls were successful.
**/
ECAL_API_EXPORTED_MEMBER
std::vector<std::pair<bool, SServiceResponse>> CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_ = -1);
bool CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_, ServiceResponseVecT& service_response_vec_) const;

/**
* @brief Blocking call (with timeout) of a service method for all existing service instances, using callback
Expand Down
14 changes: 7 additions & 7 deletions ecal/core/include/ecal/ecal_client_instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,23 @@ namespace eCAL
{
class CServiceClientIDImpl;

class ECAL_API_CLASS CServiceClientInstance final
class ECAL_API_CLASS CClientInstance final
{
public:
// Constructor
ECAL_API_EXPORTED_MEMBER
CServiceClientInstance(const Registration::SEntityId& entity_id_, const std::shared_ptr<CServiceClientIDImpl>& service_client_id_impl_);
CClientInstance(const Registration::SEntityId& entity_id_, const std::shared_ptr<CServiceClientIDImpl>& service_client_id_impl_);

// Defaulted destructor
~CServiceClientInstance() = default;
~CClientInstance() = default;

// Deleted copy constructor and copy assignment operator
CServiceClientInstance(const CServiceClientInstance&) = delete;
CServiceClientInstance& operator=(const CServiceClientInstance&) = delete;
CClientInstance(const CClientInstance&) = delete;
CClientInstance& operator=(const CClientInstance&) = delete;

// Defaulted move constructor and move assignment operator
ECAL_API_EXPORTED_MEMBER CServiceClientInstance(CServiceClientInstance&& rhs) noexcept = default;
ECAL_API_EXPORTED_MEMBER CServiceClientInstance& operator=(CServiceClientInstance&& rhs) noexcept = default;
ECAL_API_EXPORTED_MEMBER CClientInstance(CClientInstance&& rhs) noexcept = default;
ECAL_API_EXPORTED_MEMBER CClientInstance& operator=(CClientInstance&& rhs) noexcept = default;

/**
* @brief Blocking call of a service method, response will be returned as pair<bool, SServiceReponse>
Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/service/ecal_clientgate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ namespace eCAL
if (!m_created) return;

// destroy all remaining clients
const std::shared_lock<std::shared_timed_mutex> lock(m_service_client_id_map_sync);
const std::unique_lock<std::shared_timed_mutex> lock(m_service_client_id_map_sync);
m_service_client_id_map.clear();

m_created = false;
Expand Down
74 changes: 34 additions & 40 deletions ecal/core/src/service/ecal_service_client_id.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ namespace eCAL
*
* @return Vector of client instances
**/
std::vector<CServiceClientInstance> CServiceClientID::GetServiceClientInstances() const
std::vector<CClientInstance> CServiceClientID::GetClientInstances() const
{
std::vector<CServiceClientInstance> instances;
std::vector<CClientInstance> instances;
if (!m_service_client_impl) return instances;

auto entity_ids = m_service_client_impl->GetServiceIDs();
Expand All @@ -122,31 +122,20 @@ namespace eCAL
* @param method_name_ Method name.
* @param request_ Request string.
* @param timeout_ Maximum time before operation returns (in milliseconds, -1 means infinite).
* @param [out] service_response_vec_ Response vector containing service responses from every called service (null pointer == no response).
*
* @return vector of success states and service responses
* @return True if all calls were successful.
**/
std::vector<std::pair<bool, SServiceResponse>> CServiceClientID::CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_)
bool CServiceClientID::CallWithResponse(const std::string& method_name_, const std::string& request_, int timeout_, ServiceResponseVecT& service_response_vec_) const
{
#if 0
std::vector<std::pair<bool, SServiceResponse>> responses;
auto instances = GetServiceClientInstances();
for (auto& instance : instances)
{
responses.emplace_back(instance.CallWithResponse(method_name_, request_, timeout_));
}
return responses;
#else
std::vector<std::pair<bool, SServiceResponse>> responses;

auto instances = GetServiceClientInstances();
auto instances = GetClientInstances();
size_t num_instances = instances.size();
responses.reserve(num_instances);

// vector to hold futures
// Vector to hold futures for the return values and responses
std::vector<std::future<std::pair<bool, SServiceResponse>>> futures;
futures.reserve(num_instances);

// launch asynchronous tasks for each instance
// Launch asynchronous calls for each instance
for (auto& instance : instances)
{
futures.emplace_back(std::async(std::launch::async,
Expand All @@ -156,22 +145,37 @@ namespace eCAL
}));
}

// collect results
bool overall_success = true;
service_response_vec_.clear(); // Ensure the response vector is empty before populating it

// Collect responses
for (auto& future : futures)
{
try
{
responses.emplace_back(future.get());
// Explicitly unpack the pair
std::pair<bool, SServiceResponse> result = future.get();
bool success = result.first;
SServiceResponse response = result.second;

// Add response to the vector
service_response_vec_.emplace_back(response);

// Aggregate success states
overall_success &= success;
}
catch (const std::exception& /*e*/)
catch (const std::exception& e)
{
// handle exceptions
responses.emplace_back(std::make_pair(false, SServiceResponse{}));
// Handle exceptions and add an error response
SServiceResponse error_response;
error_response.error_msg = e.what();
error_response.call_state = call_state_failed;
service_response_vec_.emplace_back(error_response);
overall_success = false; // Mark overall success as false if any call fails
}
}

return responses;
#endif
return overall_success;
}

/**
Expand All @@ -186,16 +190,7 @@ namespace eCAL
**/
bool CServiceClientID::CallWithCallback(const std::string& method_name_, const std::string& request_, int timeout_, const ResponseIDCallbackT& response_callback_) const
{
#if 0
bool return_state = true;
auto instances = GetServiceClientInstances();
for (auto& instance : instances)
{
return_state &= instance.CallWithCallback(method_name_, request_, timeout_, response_callback_);
}
return return_state;
#else
auto instances = GetServiceClientInstances();
auto instances = GetClientInstances();
size_t num_instances = instances.size();

// Vector to hold futures for the return values
Expand All @@ -218,15 +213,14 @@ namespace eCAL
{
return_state &= future.get();
}
catch (const std::exception& e)
catch (const std::exception& /*e*/)
{
// Handle exceptions
return_state = false;
}
}

return return_state;
#endif
}

/**
Expand All @@ -241,7 +235,7 @@ namespace eCAL
bool CServiceClientID::CallWithCallbackAsync(const std::string& method_name_, const std::string& request_, const ResponseIDCallbackT& response_callback_) const
{
bool return_state = true;
auto instances = GetServiceClientInstances();
auto instances = GetClientInstances();
for (auto& instance : instances)
{
return_state &= instance.CallWithCallbackAsync(method_name_, request_, response_callback_);
Expand All @@ -266,7 +260,7 @@ namespace eCAL
**/
bool CServiceClientID::IsConnected() const
{
const auto instances = GetServiceClientInstances();
const auto instances = GetClientInstances();
for (const auto& instance : instances)
{
if (instance.IsConnected()) return true;
Expand Down
Loading

0 comments on commit 18f1da3

Please sign in to comment.