-
Notifications
You must be signed in to change notification settings - Fork 125
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
rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake: Make ament free #709
rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake: Make ament free #709
Conversation
The module FindPython3 already provides the interpreter executable path via Python3_EXECUTABLE. No need to use ament. Less loaded module means faster execution. And easier for out of tree build system to reuse this function. Signed-off-by: Yasushi SHOJI <yashi@spacecubics.com>
Hi, thanks for raising this PR. Can it be merged and backported to humble? We are having issues with building ROS2 on top of a custom python installation because of this line |
Hi, I'm not clear what I'm asked to do. The PR is obviously not merged. But are you trying to use it on Humble? Am I asked to test it or port it on Humble? How can I reproduce the problem you are seeing here? |
I think I just wanted to ask in general how to get this merged. Do you know the maintainers for this repo at all? Maybe we could tag them? |
@clalancette could you help us figure out what we need to do in order to get this PR merged? It would help us in our project |
@aditya2592 This PR is getting the path to the Python interpreter from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with full CI
@yashi if you're looking to convert Here's a quick manual example: sloretz@rolling-jammy:~$ cd /opt/ros/rolling/share/
sloretz@rolling-jammy:/opt/ros/rolling/share$ rosidl translate --to idl sensor_msgs std_msgs/msg/Header.msg -o /tmp/
Reading input file: /opt/ros/rolling/share/std_msgs/msg/Header.msg
Writing output file: /tmp/std_msgs/msg/Header.idl
sloretz@rolling-jammy:/opt/ros/rolling/share$ cat /tmp/std_msgs/msg/Header.idl
// generated from rosidl_adapter/resource/msg.idl.em
// with input from sensor_msgs/std_msgs/msg/Header.msg
// generated code does not contain a copyright notice
#include "builtin_interfaces/msg/Time.idl"
module sensor_msgs {
module msg {
@verbatim (language="comment", text=
"Standard metadata for higher-level stamped data types." "\n"
"This is generally used to communicate timestamped data" "\n"
"in a particular coordinate frame.")
struct Header {
@verbatim (language="comment", text=
"Two-integer timestamp that is expressed as seconds and nanoseconds.")
builtin_interfaces::msg::Time stamp;
@verbatim (language="comment", text=
"Transform frame with which this data is associated.")
string frame_id;
};
};
}; For a real example, see how it's used to build ROS 2 interfaces with bazel: There's an example that uses it for building interfaces with bazel here: https://github.com/RobotLocomotion/drake-ros/blob/7c1429806547dadc7209f3784eecf3cf5106c45f/bazel_ros2_rules/ros2/rosidl.bzl#L117 |
…b.com:yashi/rosidl into yashi-rosidl_adapt_interfaces-cmake-make-ament-free Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
48cb9ba
to
377007a
Compare
CI (repos file build: everything test: everything) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI can't find service_msgs
on jammy it seems ?
For the Rpr job, yeah, that is going to be the case currently. We need to do a round of releases on Rolling (hopefully this week). |
The module FindPython3 already provides the interpreter executable path via Python3_EXECUTABLE. No need to use ament.
Less loaded module means faster execution. And easier for out of tree build system to reuse this function.
Signed-off-by: Yasushi SHOJI yashi@spacecubics.com