-
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ros2] Add JointTrajectory message conversion #121
[ros2] Add JointTrajectory message conversion #121
Conversation
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.
I haven't tried running it, but I took a high-level look and it looks good to me. I only noticed the small style issues pointed out below.
CI fails because it is not using fork of ign-msgs with ignition.msgs.JointTrajectory
The ros2
branch supports Citadel (ign-msgs5
) and Dome (ign-msgs6
). So before merging this PR, gazebosim/gz-msgs#106 needs to be:
- merged against
ign-msgs6
- backported to
ign-msgs5
- released into
ign-msgs 5.X
- released into
ign-msgs 6.X
From your end, you just need to iterate on that PR until it gets merged, then the maintainers can take care of the rest.
Also, I am currently not using ROS 1 and therefore I unfortunately won't be looking into adding this conversion there.
No problem, we can ticket an issue to give it visibility in case someone else wants to pick it up.
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.
Looking good to me! Can I just ask that you sign all the commits on the PR for the DCO checker?
Then we just need to wait for the ign-msgs
release and check CI.
Conversion between - ignition::msgs::JointTrajectory - trajectory_msgs::msg::JointTrajectory Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
- UPPER_CASE to lower_case Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
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.
The necessary ign-msgs
version is upstream and CI is passing. Merging!
This PR adds conversion between
trajectory_msgs/msg/JointTrajectory
andignition.msgs.JointTrajectory
(PR - gazebosim/gz-msgs#106).Direction
ROS 2 -> Ignition Transport
is tested withign_moveit2
. However, I did NOT explicitly tryIgnition Transport -> ROS 2
(besides running the automated tests).CI fails because it is not using fork of
ign-msgs
withignition.msgs.JointTrajectory
, hence it cannot be found. I am unsure about the best way to resolve this problem. Any help/suggestions are appreciated 馃檪.Also, I am currently not using ROS 1 and therefore I unfortunately won't be looking into adding this conversion there.