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

Implement ROS2 actions #60

Open
RobertoRoos opened this issue Nov 28, 2023 · 7 comments · May be fixed by #61
Open

Implement ROS2 actions #60

RobertoRoos opened this issue Nov 28, 2023 · 7 comments · May be fixed by #61

Comments

@RobertoRoos
Copy link

RobertoRoos commented Nov 28, 2023

Title says it all: it would be cool to support ROS2 actions.

Potentially duplicates RobotecAI/ros2-for-unity#48

I'm currently trying to write this, PR will hopefully follow.

An overview on how the different APIs look so far:

Langague Publisher Service Action
Python
from std_msgs.msg import String

self.create_publisher(String, 'topic', 10)
from example_interfaces.srv import AddTwoInts

self.create_service(AddTwoInts, 'add_two_ints', self.add_two_ints_callback)
from action_tutorials_interfaces.action import Fibonacci

ActionServer(
            self,
            Fibonacci,
            'fibonacci',
            self.execute_callback)
C++
#include "std_msgs/msg/string.hpp"

this->create_publisher<std_msgs::msg::String>("topic", 10);
#include "example_interfaces/srv/add_two_ints.hpp"

this->create_service<example_interfaces::srv::AddTwoInts>("add_two_ints", &add);
#include "action_tutorials_interfaces/action/fibonacci.hpp"

rclcpp_action::create_server<Fibonacci>(
      this,
      "fibonacci",
      std::bind(&FibonacciActionServer::handle_goal, this, _1, _2),
      std::bind(&FibonacciActionServer::handle_cancel, this, _1),
      std::bind(&FibonacciActionServer::handle_accepted, this, _1));
C#
using std_msgs.msg;

node.CreatePublisher<String>("chatter");
using example_interfaces.srv;

node.CreateService<AddTwoInts_Request, AddTwoInts_Response>("add_two_ints", recv_callback);
Missing
@RobertoRoos
Copy link
Author

I'm first looking at building the interface generator for actions.

Is there some way to quickly test the generator? I am currently running colcon build --packages-select rosidl_generator_cs example_interfaces ... all the time to test the generated code, but it's quite slow.

@RobertoRoos
Copy link
Author

RobertoRoos commented Nov 28, 2023

Since actions are really two Services and a Publisher (link), another big question is where to 'split'' it.
We could define an action from the ground up, or try to define it through Services etc. as soon as possible.

Thoughts?

EDIT:
See the table in the original comment - the templating is not entirely consistent. Python and C++ use the main Service type as a single template, whereas our C# version splits the request and the response.
For the new action, should be stick to a single template (like Python and C++) or split each message (like C# so far)?

@Deric-W
Copy link
Contributor

Deric-W commented Nov 29, 2023

Since I contributed to the service implementation I can share some thoughts:
I think the splitting into multiple messages is necessary since the different pieces of information like goal, feedback and result each require a class to represent them.

The splitting of Services (but not Actions since they just use normal subscriptions and services) into independent messages is probably a little bit dangerous since it allows using the messages with publishers and subscriptions which I am not sure the rcl C library likes.
I tried refactoring it but failed to find detailed documentation on how the ROS type support system and the C type support (which is used by the C# one) work, if you found some I would be thankful if you could send me a link.
My plan was to generate type support classes alongside the messages which would be required for creating publishers, subscriptions and so on, preventing them from being used for different purposes since there would be no matching type support available.

@RobertoRoos
Copy link
Author

I think the splitting into multiple messages is necessary since the different pieces of information like goal, feedback and result each require a class to represent them.

Right. Python for example does rely on a single Service and Action object, which includes (references to) the request/response and goal/feedback/result messages. Couldn't we also easily create containers for the underlying objects?
The gain would be some convenience for application code.

I tried refactoring it but failed to find detailed documentation on how the ROS type support system and the C type support (which is used by the C# one) work, if you found some I would be thankful if you could send me a link.
My plan was to generate type support classes alongside the messages which would be required for creating publishers, subscriptions and so on, preventing them from being used for different purposes since there would be no matching type support available.

Myeah, for sure I haven't found out more than you. All of this feels rather undocumented indeed.

Are we using the C/C++ code generation that already exists in ROS2 literally?

@Deric-W
Copy link
Contributor

Deric-W commented Nov 30, 2023

Yes, to prevent having to implement a type support for every supported DDS implementation the C# type support is more or less just a wrapper around the C type support (the Python type support does this too I think).

@RobertoRoos
Copy link
Author

Right. Do you know why there are also {msg,srv,idl}_c.em templates then? I was wandering if I'd also need for the Actions.

@Deric-W
Copy link
Contributor

Deric-W commented Nov 30, 2023

They are used to provide convenient getter, setter and memory management code which we can call from C# to prevent having to redefine every generated C struct in C#.

@RobertoRoos RobertoRoos linked a pull request Dec 1, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants