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

Added services examples #3

Merged
merged 8 commits into from
Jan 17, 2015
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
11 changes: 9 additions & 2 deletions userland/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ find_package(ament_cmake REQUIRED)
find_package(rclcpp REQUIRED)
find_package(ros_middleware_implementation REQUIRED)
find_package(simple_msgs REQUIRED)
find_package(userland_msgs REQUIRED)

ament_package()

Expand All @@ -27,7 +28,8 @@ function(build_executable executable)
ament_target_dependencies(${executable}
"rclcpp"
"ros_middleware_implementation"
"simple_msgs")
"simple_msgs"
"userland_msgs")

install(TARGETS ${executable} DESTINATION bin)

Expand All @@ -37,7 +39,8 @@ function(build_executable executable)
ament_target_dependencies(${executable}__${middleware_impl_tmp}
"rclcpp"
"${middleware_impl_tmp}"
"simple_msgs")
"simple_msgs"
"userland_msgs")

install(TARGETS ${executable}__${middleware_impl_tmp} DESTINATION bin)
endforeach()
Expand Down Expand Up @@ -110,3 +113,7 @@ build_executable(listener src/ros1_like/listener.cpp)
build_executable(different_groups src/explicit/different_groups.cpp)
build_executable(two_nodes src/explicit/two_nodes.cpp)
# build_executable(two_nodes_no_intra src/explicit/two_nodes_no_intra.cpp)

# Build services examples
build_executable(add_two_ints_client src/add_two_ints_client.cpp)
build_executable(add_two_ints_server src/add_two_ints_server.cpp)
6 changes: 6 additions & 0 deletions userland/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,14 @@
<build_depend>rclcpp</build_depend>
<build_depend>ros_middleware_implementation</build_depend>
<build_depend>simple_msgs</build_depend>
<build_depend>userland_msgs</build_depend>

<buildtool_depend>rosidl_default_generators</buildtool_depend>

<exec_depend>rclcpp</exec_depend>
<exec_depend>ros_middleware_implementation</exec_depend>
<exec_depend>simple_msgs</exec_depend>
<exec_depend>userland_msgs</exec_depend>

<exec_depend>rosidl_default_runtime</exec_depend>
</package>
49 changes: 49 additions & 0 deletions userland/src/add_two_ints_client.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#include <algorithm>
#include <chrono>
#include <cmath>
#include <iostream>
#include <string>
#include <sys/time.h>
#include <thread>

#include <rclcpp/rclcpp.hpp>

#include <simple_msgs/AllBuiltinTypes.h>
#include <simple_msgs/AllDynamicArrayTypes.h>
#include <simple_msgs/AllPrimitiveTypes.h>
#include <simple_msgs/AllStaticArrayTypes.h>
#include <simple_msgs/Nested.h>
#include <simple_msgs/String.h>
#include <simple_msgs/Uint32.h>

#include <userland_msgs/AddTwoInts.h>

int main(int argc, char** argv)
{
rclcpp::init(argc, argv);

auto node = rclcpp::Node::make_shared("add_two_ints_client");

auto client = node->create_client<userland_msgs::AddTwoInts>("add_two_ints");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an asynchronous example (beside this synchronous one) would be good in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the example to work asynchronously. Unfortunately, I can't hide the do { ... } loop because I can't spin the node from inside the client.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting something like this for the asynchronous case:

void handle_response(const std::shared_ptr<userland_msgs::AddTwoInts::Response> &response)
{
  std::cout << "Got response: " << response->sum << std::endl;
}

...
client->async_send_request(request, handle_response);  // I guess you pass a pre-allocated response too
rclcpp::spin();  // This could be here or in another thread and would cause the callback to get called

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the future method supports other use cases as well though, we may need something similar to both.

Even in the future method as you have here I would have expected the spin to be elsewhere, though not necessarily.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expected to be able to spin inside the client, but alas the API doesn't allow that, or I couldn't figure out how. Where else did you expect to spin?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about the issues of spinning inside of a client, you'll need a recursive lock and a way to deal with multithreaded execution...

You can spin in different threads, for example you could create thread to call spin in or you could create the async request in a callback for subscription or you could spin in the main thread and then do the async request in a user created thread. There are lots of options.

The callback pattern that I described above has the advantage that you don't have to poll the future, so its more like request it and forget it. The future pattern has the advantage that you can use one thread to do work while periodically checking the future. But in both cases you need to be spinning in the mean time. In your example you just happen to use the single thread for polling the future and spinning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way this looks like a good proof of concept, we can certainly implement what I described above with the existing functionality. I think we should get a consensus on the API before we spend more time iterating on more API features. To that end I think we should get these branches merged and then we should have a meeting to sync up on the API's, I'll try to setup a meeting today.

auto request = std::make_shared<userland_msgs::AddTwoInts::Request>();
auto response = std::make_shared<userland_msgs::AddTwoInts::Response>();
request->a = 2;
request->b = 3;

auto f = client->async_send_request(request, response);

std::future_status status;
do
{
rclcpp::spin_some(node);
status = f.wait_for(std::chrono::milliseconds(100));
} while(status != std::future_status::ready && rclcpp::ok());

if(std::future_status::ready == status)
{
std::cout << "FUTURE READY" << std::endl;
std::cout << f.get()->sum << std::endl;
}

return 0;
}
40 changes: 40 additions & 0 deletions userland/src/add_two_ints_server.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#include <algorithm>
#include <chrono>
#include <cmath>
#include <iostream>
#include <string>
#include <sys/time.h>
#include <thread>

#include <rclcpp/rclcpp.hpp>

#include <simple_msgs/AllBuiltinTypes.h>
#include <simple_msgs/AllDynamicArrayTypes.h>
#include <simple_msgs/AllPrimitiveTypes.h>
#include <simple_msgs/AllStaticArrayTypes.h>
#include <simple_msgs/Nested.h>
#include <simple_msgs/String.h>
#include <simple_msgs/Uint32.h>

#include <userland_msgs/AddTwoInts.h>

void add(const std::shared_ptr<userland_msgs::AddTwoInts::Request> req,
std::shared_ptr<userland_msgs::AddTwoInts::Response> res)
{
std::cout << "Incoming request" << std::endl;
std::cout << "a: " << req->a << " b: " << req->b << std::endl;
res->sum = req->a + req->b;
}

int main(int argc, char** argv)
{
rclcpp::init(argc, argv);

auto node = rclcpp::Node::make_shared("add_two_ints_server");

node->create_service<userland_msgs::AddTwoInts>("add_two_ints", add);

rclcpp::spin(node);

return 0;
}
18 changes: 18 additions & 0 deletions userland_msgs/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
cmake_minimum_required(VERSION 2.8.3)

project(userland_msgs)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")

find_package(ament_cmake REQUIRED)
find_package(builtin_msgs REQUIRED)
find_package(rosidl_default_generators REQUIRED)

include_directories(${rosidl_default_generators_INCLUDE_DIRS})

rosidl_generate_interfaces(${PROJECT_NAME}
"srv/AddTwoInts.srv"
DEPENDENCIES builtin_msgs
)

ament_package()
20 changes: 20 additions & 0 deletions userland_msgs/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0"?>
<package format="2">
<name>userland_msgs</name>
<version>0.0.0</version>
<description>A package containing simple message definitions.</description>
<maintainer email="[email protected]">Dirk Thomas</maintainer>
<license>Apache License 2.0</license>

<buildtool_depend>ament_cmake</buildtool_depend>

<buildtool_depend>rosidl_default_generators</buildtool_depend>

<build_depend>builtin_msgs</build_depend>
<build_depend>simple_msgs</build_depend>

<exec_depend>builtin_msgs</exec_depend>
<exec_depend>simple_msgs</exec_depend>

<exec_depend>rosidl_default_runtime</exec_depend>
</package>
4 changes: 4 additions & 0 deletions userland_msgs/srv/AddTwoInts.srv
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
int64 a
int64 b
---
int64 sum