Skip to content

Adapt repository for new moveit_resources layout#2199

Merged
rhaschke merged 4 commits intomasterfrom
pr-master-rework-resources
Aug 12, 2020
Merged

Adapt repository for new moveit_resources layout#2199
rhaschke merged 4 commits intomasterfrom
pr-master-rework-resources

Conversation

@v4hn
Copy link
Copy Markdown
Contributor

@v4hn v4hn commented Jul 7, 2020

Related to moveit/moveit_resources#36 .

There is no reason to include them from cmake anymore.
The resources are not in a single path anymore and there is no header exposed anymore.

@v4hn v4hn changed the title [WIP] remove moveit_resources from CMakeLists.txt [WIP] adapt repository for new moveit_resources layout Jul 7, 2020
@v4hn v4hn added the help wanted please consider improving this request! label Jul 7, 2020
This was referenced Jul 7, 2020
Merged
Merged
@v4hn v4hn requested a review from henningkayser as a code owner July 7, 2020 14:04
@rhaschke rhaschke force-pushed the pr-master-rework-resources branch from d74aa07 to c74199b Compare July 7, 2020 14:23
@rhaschke
Copy link
Copy Markdown
Contributor

rhaschke commented Jul 7, 2020

I'm looking into the remaining (test) build issues right now.

@rhaschke rhaschke force-pushed the pr-master-rework-resources branch from 6fde021 to a3691eb Compare July 7, 2020 22:26
@rhaschke
Copy link
Copy Markdown
Contributor

rhaschke commented Jul 7, 2020

I hope I fixed most of the numerous issues. Will finish for now.

@rhaschke rhaschke force-pushed the pr-master-rework-resources branch 3 times, most recently from 7d35659 to 2633e23 Compare July 8, 2020 08:15
@JeroenDM JeroenDM mentioned this pull request Jul 23, 2020
Closed
jschleicher
jschleicher reviewed Jul 28, 2020
View reviewed changes
moveit.rosinstall
version: master
version: pr-master-rework-resources
- git:
local-name: panda_moveit_config
Copy link
Copy Markdown
Contributor

@jschleicher jschleicher Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

panda_moveit_config is now used from moveit_resources and could be dropped here?!

Copy link
Copy Markdown
Contributor Author

@v4hn v4hn Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't used in MoveIt before either, but it's required for the tutorials and thus it's listed in this file.

@v4hn v4hn force-pushed the pr-master-rework-resources branch from 2633e23 to 38dfc57 Compare July 28, 2020 16:04
@v4hn
Copy link
Copy Markdown
Contributor Author

v4hn commented Jul 28, 2020

rebased to resolve conflicts.

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 28, 2020 •
edited
Loading

Codecov Report

Merging #2199 into master will increase coverage by 0.37%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2199      +/-   ##
==========================================
+ Coverage   57.73%   58.10%   +0.37%     
==========================================
  Files         326      326              
  Lines       25621    25861     +240     
==========================================
+ Hits        14792    15027     +235     
- Misses      10829    10834       +5
Impacted Files Coverage Δ
...tils/include/moveit/utils/robot_model_test_utils.h 100.00% <ø> (ø)
.../collision_detection/test_collision_common_panda.h 99.31% <100.00%> (ø)
...it/collision_detection/test_collision_common_pr2.h 100.00% <100.00%> (ø)
moveit_core/utils/src/robot_model_test_utils.cpp 73.07% <100.00%> (ø)
...ics_plugin_loader/src/kinematics_plugin_loader.cpp 60.54% <0.00%> (-2.05%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 69.59% <0.00%> (-0.30%) ⬇️
...os/moveit_servo/include/moveit_servo/servo_calcs.h 100.00% <0.00%> (ø)
moveit_core/robot_state/src/robot_state.cpp 47.30% <0.00%> (+0.29%) ⬆️
...meterization/work_space/pose_model_state_space.cpp 82.99% <0.00%> (+0.68%) ⬆️
...ipulation/pick_place/src/manipulation_pipeline.cpp 77.35% <0.00%> (+0.94%) ⬆️
... and 13 more

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 7ad2bc7...f0e75b5. Read the comment docs.

jschleicher
jschleicher approved these changes Aug 5, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jschleicher jschleicher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thanks for the clarification. Than the only issue is the failing mesh_filter test(s) #2228 - obviously independent from this refactoring.

This was referenced Aug 5, 2020
Closed
Closed
@SansoneG
Copy link
Copy Markdown
Contributor

SansoneG commented Aug 7, 2020

@v4hn Is there anything still holding this PR back from getting merged?

@rhaschke rhaschke force-pushed the pr-master-rework-resources branch from 38dfc57 to f37be19 Compare August 12, 2020 09:30
@rhaschke rhaschke changed the title [WIP] adapt repository for new moveit_resources layout Adapt repository for new moveit_resources layout Aug 12, 2020
rhaschke
rhaschke approved these changes Aug 12, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Travis succeeds, I will go to merge this and the related PRs moveit/moveit_resources#36 and moveit/moveit_tutorials#507.

v4hn and others added 4 commits August 12, 2020 11:34
@v4hn @rhaschke
Remove moveit_resources from CMakeLists.txt
9f28498 
There is no reason to include them from cmake anymore.
The resources are not in a single path anymore and there is no C header exposed anymore.
@rhaschke
Replace moveit_resources/<folder> with moveit_resources_<package>
21ff2e2
@rhaschke
Fix loading of test resources
e8728c4 
- Convert code to use robot_model_test_utils
- Fixup loading in robot_model_test_utils
@rhaschke
Adapt expected self-collision distance
f0e75b5
@rhaschke rhaschke force-pushed the pr-master-rework-resources branch 2 times, most recently from 3ebbaef to f0e75b5 Compare August 12, 2020 22:16
@rhaschke rhaschke merged commit 4611219 into master Aug 12, 2020
@rhaschke rhaschke deleted the pr-master-rework-resources branch August 12, 2020 22:18
@rhaschke rhaschke mentioned this pull request Aug 12, 2020
Closed
rhaschke added a commit that referenced this pull request Aug 13, 2020
@rhaschke
Merge PR #2199: Adapt repository for splitted moveit_resources layout
395af8e
@rhaschke rhaschke mentioned this pull request Aug 13, 2020
Merged
rhaschke added a commit that referenced this pull request Aug 13, 2020
@rhaschke
Merge PR #2199: Adapt repository for splitted moveit_resources layout
ad067ac
@JafarAbdi JafarAbdi mentioned this pull request Aug 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhaschke rhaschke rhaschke approved these changes

@gavanderhoorn gavanderhoorn Awaiting requested review from gavanderhoorn gavanderhoorn is a code owner

@IanTheEngineer IanTheEngineer Awaiting requested review from IanTheEngineer

@jrgnicho jrgnicho Awaiting requested review from jrgnicho

@mamoll mamoll Awaiting requested review from mamoll mamoll is a code owner

@mlautman mlautman Awaiting requested review from mlautman

@tylerjw tylerjw Awaiting requested review from tylerjw tylerjw is a code owner

@henningkayser henningkayser Awaiting requested review from henningkayser henningkayser is a code owner

+1 more reviewer

@jschleicher jschleicher jschleicher approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

help wanted please consider improving this request!

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@v4hn @rhaschke @SansoneG @jschleicher