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

Update msg for support in ROS Iron #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update msg for support in ROS Iron #2

wants to merge 1 commit into from

Conversation

Apurv354
Copy link

@Apurv354 Apurv354 commented Feb 3, 2024

No description provided.

@Apurv354 Apurv354 requested a review from fabid February 3, 2024 03:54
@Apurv354 Apurv354 self-assigned this Feb 3, 2024
@Apurv354
Copy link
Author

Apurv354 commented Feb 7, 2024

@fabid I have confirmed that we don't need to add any if condition here.
This change applies to all the versions of ros.

@fabid
Copy link

fabid commented Feb 8, 2024

actually I think I made a mistake yesterday to talk about a condition based on ros version, but there might be a need to make a change base on MoveIt version, right?
I think the change on this PR works with moveit rolling and probably the latest moveit on iron, but not the default moveit-humble. Or does it work with moveit-humble?

@Apurv354
Copy link
Author

Apurv354 commented Feb 8, 2024

actually I think I made a mistake yesterday to talk about a condition based on ros version, but there might be a need to make a change base on MoveIt version, right? I think the change on this PR works with moveit rolling and probably the latest moveit on iron, but not the default moveit-humble. Or does it work with moveit-humble?

Except humble this change works with every ros version.

@fabid
Copy link

fabid commented Feb 8, 2024

Except humble this change works with every ros version.

Do you mean every ROS version newer than Humble? (I suspect it won't work with Foxy if it doesn't with Humble).

And so can we modify the PR to make it work including on Humble, with some conditional, so that it can be merged in main?

I also realized that on top of providing latest ros-humble-moveit-* they also have version numbers like 2.6 and 2.7. It would be great to track down which version change is the PR change related to

@Apurv354
Copy link
Author

Apurv354 commented Feb 9, 2024

Except humble this change works with every ros version.

Do you mean every ROS version newer than Humble? (I suspect it won't work with Foxy if it doesn't with Humble).

And so can we modify the PR to make it work including on Humble, with some conditional, so that it can be merged in main?

I also realized that on top of providing latest ros-humble-moveit-* they also have version numbers like 2.6 and 2.7. It would be great to track down which version change is the PR change related to

I will track it down

@fabid
Copy link

fabid commented Feb 14, 2024

Option1: find a way to find moveit2 version and add a conditional:
Option2: PR to a branch of upstream => not sure how to do it

@fabid
Copy link

fabid commented Feb 19, 2024

@Apurv354 any update on this one?

@Apurv354
Copy link
Author

@Apurv354 any update on this one?

@fabid I went through the git history of moveit_ros_msgs. It is a bit complicated but I saw they have a master branch which has the most up-to-date code and there are several ros branches.

They merged this master to Iron 7 months ago which brought the change - moveit/moveit_msgs@1fe0082

There is no update to the humble branch that's why the code fails in ros_moveit_msgs - humble.

@fabid
Copy link

fabid commented Feb 21, 2024

thank you @Apurv354 for digging. Could you double check your link? it is directing me to a CI workflow change

@fabid
Copy link

fabid commented Feb 21, 2024

this PR? moveit/moveit_msgs#130

@fabid
Copy link

fabid commented Feb 21, 2024

Changelog of moveit_msgs: https://github.com/ros-planning/moveit_msgs/blob/9756797dcbc293c7cae66541a7bc0c7acec85892/CHANGELOG.rst#L12
change since version 0.11.3 (2022-09-13)

I cannot find a clear reference of the matching moveit2 version, even the ubuntu package directory does not display matching versions https://index.ros.org/p/moveit_msgs/#rolling

So I would say that based on our tests, the change in this PR should target iron and rolling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants