cleanup and augment moveit test resources #7
Conversation
For future reference would prefer this in a series of smaller PRs |
| install(DIRECTORY test DESTINATION ${CATKIN_PACKAGE_SHARE_DESTINATION}) | ||
| install(FILES "${CMAKE_CURRENT_BINARY_DIR}/installconfig/config_install.h" | ||
| DESTINATION "include/moveit/test_resources" | ||
| install(DIRECTORY pr2_description DESTINATION ${CATKIN_PACKAGE_SHARE_DESTINATION}) |
There was a problem hiding this comment.
could this conflict with actual pr2_description package?
There was a problem hiding this comment.
I don't think so, because the package here is named moveit_resources. Hence, the full name is $(find moveit_resources)/pr2_description.
|
I'm still uneasy about adding all the cruft from fanuc_moveit_config into here. the pr2 did not do that, but then again there isn't an integration test for the pr2. i think i would be ok with this if you created a parent folder for Fanuc's |
|
ping - if you moved the fanuc config and launch folders into a parent fanuc_moveit_config folder, then I think i'd be ready to merge these two PRs in |
|
@davetcoleman Sorry, I haven't seen your reply before.
I agree and actually I started along that line. However, for some reason, I thought that the launch folder of a ROS package needs to reside in the main package folder and for that reason I splitted the moveit_config up. Obviously, that is not required. Will fix this. |
|
Done. @davetcoleman Could you have another look? |
|
Attention: This PR is tied to moveit/moveit#83. |
Files generated by setup_assistant and modified to work within existing package moveit_resources.
|
@davetcoleman You should maybe enable travis testing for this repo at: https://travis-ci.org/profile/ros-planning? |
| matrix: | ||
| - ROS_DISTRO="kinetic" ROS_REPO=ros | ||
| - ROS_DISTRO="jade" ROS_REPO=ros | ||
| - ROS_DISTRO="indigo" ROS_REPO=ros |
There was a problem hiding this comment.
You didn't include:
UPSTREAM_WORKSPACE=https://raw.githubusercontent.com/ros-planning/moveit/kinetic-devel/moveit.rosinstall
So when moveit_ci tests this, it will only install the dependencies of moveit_resource (almost nothing) and not test changes against the rest of moveit. Is this what you intended? It would be a better test if we added that line
There was a problem hiding this comment.
Here, indeed, I just intended to test whether moveit_resources correctly builds.
There was a problem hiding this comment.
but moveit_resources is tightly coupled (only used by) moveit-consolidated, so why not run the whole test setup?
|
i enabled travis ci for this repo everything looks good to me except my travis comment. this must be merged at the exact same time as moveit/moveit#83 |
* adapted paths to moveit_resources (renamed folder moveit_resources/test to moveit_resources/pr2_description) * fetch moveit_resources path at compile time using variable MOVEIT_TEST_RESOURCES_DIR provided by config.h instead of calling ros::package::getPath()
This is the first part for moveit/moveit#17
Now, this package can be used for integration tests.