Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Address review comments from William
Browse files Browse the repository at this point in the history
Signed-off-by: Barry Xu <[email protected]>
Barry-Xu-2018 committed Jan 12, 2024
1 parent da86048 commit 69145c4
Showing 7 changed files with 708 additions and 684 deletions.
41 changes: 28 additions & 13 deletions rclcpp/include/rclcpp/client.hpp
Original file line number Diff line number Diff line change
@@ -115,6 +115,29 @@ struct FutureAndRequestId
/// Destructor.
~FutureAndRequestId() = default;
};

template<typename PendingRequestsT, typename AllocatorT = std::allocator<int64_t>>
size_t
prune_requests_older_than_impl(
PendingRequestsT & pending_requests,
std::mutex & pending_requests_mutex,
std::chrono::time_point<std::chrono::system_clock> time_point,
std::vector<int64_t, AllocatorT> * pruned_requests = nullptr)
{
std::lock_guard guard(pending_requests_mutex);
auto old_size = pending_requests.size();
for (auto it = pending_requests.begin(), last = pending_requests.end(); it != last; ) {
if (it->second.first < time_point) {
if (pruned_requests) {
pruned_requests->push_back(it->first);
}
it = pending_requests.erase(it);
} else {
++it;
}
}
return old_size - pending_requests.size();
}
} // namespace detail

namespace node_interfaces
@@ -771,19 +794,11 @@ class Client : public ClientBase
std::chrono::time_point<std::chrono::system_clock> time_point,
std::vector<int64_t, AllocatorT> * pruned_requests = nullptr)
{
std::lock_guard guard(pending_requests_mutex_);
auto old_size = pending_requests_.size();
for (auto it = pending_requests_.begin(), last = pending_requests_.end(); it != last; ) {
if (it->second.first < time_point) {
if (pruned_requests) {
pruned_requests->push_back(it->first);
}
it = pending_requests_.erase(it);
} else {
++it;
}
}
return old_size - pending_requests_.size();
return detail::prune_requests_older_than_impl(
pending_requests_,
pending_requests_mutex_,
time_point,
pruned_requests);
}

/// Configure client introspection.
22 changes: 22 additions & 0 deletions rclcpp/include/rclcpp/create_generic_client.hpp
Original file line number Diff line number Diff line change
@@ -19,6 +19,9 @@
#include <string>

#include "rclcpp/generic_client.hpp"
#include "rclcpp/node_interfaces/get_node_base_interface.hpp"
#include "rclcpp/node_interfaces/get_node_graph_interface.hpp"
#include "rclcpp/node_interfaces/get_node_services_interface.hpp"
#include "rclcpp/node_interfaces/node_base_interface.hpp"
#include "rclcpp/node_interfaces/node_graph_interface.hpp"
#include "rclcpp/node_interfaces/node_services_interface.hpp"
@@ -51,6 +54,25 @@ create_generic_client(
const rclcpp::QoS & qos = rclcpp::ServicesQoS(),
rclcpp::CallbackGroup::SharedPtr group = nullptr);

template<typename NodeT>
rclcpp::GenericClient::SharedPtr
create_generic_client(
NodeT node,
const std::string & service_name,
const std::string & service_type,
const rclcpp::QoS & qos = rclcpp::ServicesQoS(),
rclcpp::CallbackGroup::SharedPtr group = nullptr)
{
return create_generic_client(
rclcpp::node_interfaces::get_node_base_interface(node),
rclcpp::node_interfaces::get_node_graph_interface(node),
rclcpp::node_interfaces::get_node_services_interface(node),
service_name,
service_type,
qos,
group
);
}
} // namespace rclcpp

#endif // RCLCPP__CREATE_GENERIC_CLIENT_HPP_
47 changes: 25 additions & 22 deletions rclcpp/include/rclcpp/generic_client.hpp
Original file line number Diff line number Diff line change
@@ -84,13 +84,16 @@ class GenericClient : public ClientBase
rcl_client_options_t & client_options);

RCLCPP_PUBLIC
SharedResponse create_response() override;
SharedResponse
create_response() override;

RCLCPP_PUBLIC
std::shared_ptr<rmw_request_id_t> create_request_header() override;
std::shared_ptr<rmw_request_id_t>
create_request_header() override;

RCLCPP_PUBLIC
void handle_response(
void
handle_response(
std::shared_ptr<rmw_request_id_t> request_header,
std::shared_ptr<void> response) override;

@@ -123,7 +126,8 @@ class GenericClient : public ClientBase
* \return a FutureAndRequestId instance.
*/
RCLCPP_PUBLIC
FutureAndRequestId async_send_request(const Request request);
FutureAndRequestId
async_send_request(const Request request);

/// Clean all pending requests older than a time_point.
/**
@@ -138,26 +142,21 @@ class GenericClient : public ClientBase
std::chrono::time_point<std::chrono::system_clock> time_point,
std::vector<int64_t, AllocatorT> * pruned_requests = nullptr)
{
std::lock_guard guard(pending_requests_mutex_);
auto old_size = pending_requests_.size();
for (auto it = pending_requests_.begin(), last = pending_requests_.end(); it != last; ) {
if (it->second.first < time_point) {
if (pruned_requests) {
pruned_requests->push_back(it->first);
}
it = pending_requests_.erase(it);
} else {
++it;
}
}
return old_size - pending_requests_.size();
return detail::prune_requests_older_than_impl(
pending_requests_,
pending_requests_mutex_,
time_point,
pruned_requests);
}

RCLCPP_PUBLIC
size_t prune_pending_requests();
size_t
prune_pending_requests();

RCLCPP_PUBLIC
bool remove_pending_request(int64_t request_id);
bool
remove_pending_request(
int64_t request_id);

/// Take the next response for this client.
/**
@@ -173,7 +172,8 @@ class GenericClient : public ClientBase
* rcl function fail.
*/
RCLCPP_PUBLIC
bool take_response(Response response_out, rmw_request_id_t & request_header_out)
bool
take_response(Response response_out, rmw_request_id_t & request_header_out)
{
return this->take_type_erased_response(response_out, request_header_out);
}
@@ -183,10 +183,13 @@ class GenericClient : public ClientBase
std::promise<SharedResponse>>; // Use variant for extension

int64_t
async_send_request_impl(const Request request, CallbackInfoVariant value);
async_send_request_impl(
const Request request,
CallbackInfoVariant value);

std::optional<CallbackInfoVariant>
get_and_erase_pending_request(int64_t request_number);
get_and_erase_pending_request(
int64_t request_number);

RCLCPP_DISABLE_COPY(GenericClient)

11 changes: 11 additions & 0 deletions rclcpp/test/rclcpp/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -74,6 +74,17 @@ if(TARGET test_generic_client)
${test_msgs_TARGETS}
)
endif()
ament_add_gtest(test_client_common test_client_common.cpp)
if(TARGET test_client_common)
target_link_libraries(test_client_common ${PROJECT_NAME}
mimick
${rcl_interfaces_TARGETS}
rmw::rmw
rosidl_runtime_cpp::rosidl_runtime_cpp
rosidl_typesupport_cpp::rosidl_typesupport_cpp
${test_msgs_TARGETS}
)
endif()
ament_add_gtest(test_create_subscription test_create_subscription.cpp)
if(TARGET test_create_subscription)
target_link_libraries(test_create_subscription ${PROJECT_NAME} ${test_msgs_TARGETS})
Loading

0 comments on commit 69145c4

Please sign in to comment.