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

Rename cartesian_limits.yaml #1422

Merged
merged 3 commits into from
Jul 12, 2022

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Jul 8, 2022

I think it makes sense to specify this is only for Pilz. Users were getting confused.

Merge with moveit/moveit_resources#146.

Fixes #1421

@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #1422 (2dd1633) into main (4597319) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1422   +/-   ##
=======================================
  Coverage   50.85%   50.85%           
=======================================
  Files         381      381           
  Lines       31735    31735           
=======================================
  Hits        16135    16135           
  Misses      15600    15600           
Impacted Files Coverage Δ
...nclude/moveit_setup_framework/data/srdf_config.hpp 63.83% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4597319...2dd1633. Read the comment docs.

@AndyZe AndyZe force-pushed the andyz/cartesian_limits_renaming branch from 66e05c9 to dac30d3 Compare July 8, 2022 18:53
@AndyZe
Copy link
Member Author

AndyZe commented Jul 8, 2022

I think CI is failing because the moveit_resources change hasn't propagated yet (moveit/moveit_resources#146).

"File /opt/ros/rolling/share/moveit_resources_panda_moveit_config/config/pilz_cartesian_limits.yaml doesn't exist"

Somebody with higher-level permissions will probably have to merge it (or just wait). @henningkayser @mamoll @nbbrooks

@mikeferguson
Copy link
Contributor

@AndyZe I think we'll actually need a new release of moveit_resources for this to pass - looks like @vatanaksoytezer has been the one releasing moveit_resources recently?

@tylerjw tylerjw force-pushed the andyz/cartesian_limits_renaming branch from dac30d3 to 6e44c85 Compare July 12, 2022 16:46
Signed-off-by: Tyler Weaver <tylerjw@gmail.com>
@tylerjw
Copy link
Member

tylerjw commented Jul 12, 2022

I pushed my change here to update the repos file for moveit_resources and add the fix for the breaking change in ros2_control that was just released. That way I can merge both of these changes (there was no way to make either pass CI without each-other).

@mikeferguson
Copy link
Contributor

@tylerjw do we actually want to be merging things that won't build without people adding packages to their local workspace? Or should we be holding until things propagate through proper release/build?

@tylerjw
Copy link
Member

tylerjw commented Jul 12, 2022

@mikeferguson we have always done this for changes to repos we control (like moveit_resources) and ros2_control on the main branch. We obviously can't cut a release in this state though.

Also, the current state of the main branch will not pass tests in CI because of the breaking change in ros2_control that is now in the testing repos.

At the same time, this fix to get the main branch to pass tests we need to make a change to moveit_resources because of the change to ros2_control that was just released and is in testing.

The breaking change in ros2_control without a tick-tock that was released has sort of forced our hand here. I don't see another path where the main branch of moveit2 will build on rolling with the testing repos (and soon the main rolling and humble repos).

@tylerjw
Copy link
Member

tylerjw commented Jul 12, 2022

We do plan on cutting a release of moveit_resources quickly to make this very temporary. Then once that and the ros2_control change has propagated to the main ros apt repos we can remove the entries from the repos file I added here.

@tylerjw
Copy link
Member

tylerjw commented Jul 12, 2022

I reran the humble-ci-testing job because the failure there is the flaky test in the rdf loader: #1334

@tylerjw tylerjw merged commit 47c52f2 into moveit:main Jul 12, 2022
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 this pull request may close these issues.

Rename cartesian_limits.yaml --> pilz_cartesian_limits.yaml
4 participants