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

Enable minimal changes with improved template logic #18

Closed
tfoote opened this issue Jan 16, 2024 · 4 comments
Closed

Enable minimal changes with improved template logic #18

tfoote opened this issue Jan 16, 2024 · 4 comments
Assignees

Comments

@tfoote
Copy link
Owner

tfoote commented Jan 16, 2024

The design of the TypeAdaptor shouldn't need this line changed.

publisher_ = this->create_publisher<std_msgs::msg::typesupport_protobuf_cpp::StringTypeAdapter>("topic", 10);

You should be able to use the

publisher_ = this->create_publisher<std_msgs::msg::String>("topic", 10);

And the compiler template logic should be able to let you publish with either data type if you've included the right headers. So I think that we're slightly off on how we have the headers/templates setup to enable this.

@gonzodepedro
Copy link
Collaborator

I think the way to do this is using: RCLCPP_USING_CUSTOM_TYPE_AS_ROS_MESSAGE_TYPE.

@gonzodepedro
Copy link
Collaborator

I hacked a little bit our demo to use RCLCPP_USING_CUSTOM_TYPE_AS_ROS_MESSAGE_TYPE(std_msgs::msg::pb::String, std_msgs::msg::String);, by doing that I was able to construct the publisher with the custom type: publisher_ = this->create_publisher<std_msgs::msg::pb::String>("topic", 10);

I couldn't find a way to construct the publisher with the ROS_Type and make it automatically use the adapter.
Here is the publisher constructor documentation for reference. https://github.com/ros2/rclcpp/blob/3594381e04ae882b073875963c843c656fbf24f3/rclcpp/include/rclcpp/publisher.hpp#L57

@tfoote
Copy link
Owner Author

tfoote commented Jan 18, 2024

Yeah, I think you're right reading: https://github.com/ros2/rclcpp/blob/6f6b5f2e3608eafeadb908aa4f6eded5b38c3043/rclcpp/include/rclcpp/type_adapter.hpp#L86C50-L95

And https://ros.org/reps/rep-2007.html#defining-a-type-adapter

We can change it to allow declaring publish and subscribe on the adapted type if we inject the RCLCPP_USING_CUSTOM_TYPE_AS_ROS_MESSAGE_TYPE into the generated code. But we don't have the template metaprogramming to allow it to accept both types. Lets add that to the generation to allow people the simpler syntax with just declaring the adapted type and we can call that good enough.

@gonzodepedro
Copy link
Collaborator

Merged via eclipse-ecal#33

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

No branches or pull requests

2 participants