Port planning_scene to ROS2 by vmayoral · Pull Request #43 · moveit/moveit2

vmayoral · 2019-03-10T17:16:46Z

@anasarrak

mlautman · 2019-03-11T22:31:28Z

moveit_core/planning_scene/CMakeLists.txt

 add_library(${MOVEIT_LIB_NAME} src/planning_scene.cpp)
-set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}")
+#TODO: Fix the versioning
+# set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}")


This was added in moveit/moveit#273. Maybe @v4hn can confirm this but perhaps we can take the same approach as the urdf_parser

davetcoleman · 2019-03-15T01:17:24Z

moveit_core/planning_scene/CMakeLists.txt

-  target_link_libraries(test_planning_scene ${MOVEIT_LIB_NAME})
-endif()
+#TODO: Fix this so it can be built with colcon
+# if(BUILD_TESTING)


re-enable tests please

Tests failing. Let's try to solve AcutronicRobotics#13 first please. We should be able to re-enable all of them afterwards.

You all fixed this somewhere in AcutronicRobotics#71, is that correct?

Yes however they should be submitted here (and we should rebase this code as well). @ahcorde started doing it iteratively while we review the changes.

+1 to restoring tests before merge

moveit_core/planning_scene/src/planning_scene.cpp

moveit_core/planning_scene/include/moveit/planning_scene/planning_scene.h

moveit_core/planning_scene/src/planning_scene.cpp

mlautman · 2019-05-21T20:57:26Z

I applied the clang formatting to this PR. Once we have the tests enabled, and we resolved why world is being made non-const +1

moveit_core/planning_scene/src/planning_scene.cpp

mlautman

Some small changes, enable and pass tests then +1.

mlautman · 2019-05-21T22:05:07Z

moveit_core/planning_scene/CMakeLists.txt

-  target_link_libraries(test_planning_scene ${MOVEIT_LIB_NAME})
-endif()
+#TODO: Fix this so it can be built with colcon
+# if(BUILD_TESTING)


+1 to restoring tests before merge

moveit_core/planning_scene/src/planning_scene.cpp

…s_planning_scene.cpp

ahcorde · 2019-05-22T08:09:01Z

@mlautman updated

henningkayser

+1 from me after this point is addressed

henningkayser · 2019-05-22T15:34:41Z

moveit_core/planning_scene/CMakeLists.txt


-add_library(${MOVEIT_LIB_NAME} src/planning_scene.cpp)
+if(WIN32)
+  # set(append_library_dirs "$<TARGET_FILE_DIR:${PROJECT_NAME}>;$<TARGET_FILE_DIR:${PROJECT_NAME}_TestPlugins1>")


This should be fixed, commented or removed.

mlautman · 2019-05-29T19:35:22Z

moveit_core/planning_scene/CMakeLists.txt

  include_directories(${moveit_resources_INCLUDE_DIRS})

+  if(UNIX OR APPLE)
+    set(append_library_dirs "${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_CURRENT_BINARY_DIR}/../utils:${CMAKE_CURRENT_BINARY_DIR}/../collision_detection_fcl:${CMAKE_CURRENT_BINARY_DIR}/../collision_detection")


Hardcoding relative paths is a bad idea.

I would recommend that for every library in this package that is used else ware you set a <lib_name>_lib_dir environment variable that can then be used by the tests

A good example is the rcl_lib_dir I have taken the time to show how it is used below:

/home/mike/ws_ros2/src/ros2/rcl/rcl/CMakeLists.txt: 78 79: # rcl_lib_dir is passed as APPEND_LIBRARY_DIRS for each ament_add_gtest call so 80 # the librcl that they link against is on the library path. 81 # This is especially important on Windows. 82 # This is overwritten each loop, but which one it points to doesn't really matter. 83: set(rcl_lib_dir "$<TARGET_FILE_DIR:${PROJECT_NAME}>") 84 /home/mike/ws_ros2/src/ros2/rcl/rcl/test/CMakeLists.txt: 16 17: set(extra_lib_dirs "${rcl_lib_dir}") 18 /home/mike/ws_ros2/src/ros2/rcl/rcl/test/CMakeLists.txt: 34 rcl_add_custom_gtest(test_client${target_suffix} 35 SRCS rcl/test_client.cpp 36: INCLUDE_DIRS ${osrf_testing_tools_cpp_INCLUDE_DIRS} 37 ENV ${rmw_implementation_env_var} 38 APPEND_LIBRARY_DIRS ${extra_lib_dirs}

The example uses rcl_add_custom_gtest but it should work just as well with ament_add_gtest

If you don't mind I prefer to merge this. And then I will fix them all together. Because we have other merged that use this style. I can open an issue to avoid forgetting it.

ahcorde · 2019-05-31T15:26:25Z

Ready to merge @henningkayser

ahcorde · 2019-06-03T08:24:51Z

@henningkayser or @davetcoleman can we merge this PR?

anasarrak · 2019-06-06T10:11:35Z

Ping @mlautman @davetcoleman @henningkayser

See comment

Update CI to moveit_ci

* clang formatting * adding clang-format notes to README * fixing build warnings

* updated cmake to colcon/ament * file cleanup and commented out unconfigured build_depend in package.xml * more cleanup * removed subdirectory moveit_cpp * Switch to travis-ci.com in readme links * fix to travis status in readme * Update conf.py to generate CNAME * updated to meta.keys() (moveit#45) * fix travis for custom domain (moveit#47) * updated cmake with moveit_package, removed empty cmake, added vcs file * package.xml cleanup * updated readme with source build instructions * updated travis to foxy * updated repos to include moveit2.repos * fixed typo * ament format * more ament formatting * removed code coverage ci test added moveit.repos * updated cmake to colcon/ament * file cleanup and commented out unconfigured build_depend in package.xml * more cleanup * removed subdirectory moveit_cpp * updated cmake with moveit_package, removed empty cmake, added vcs file * package.xml cleanup * updated readme with source build instructions * updated travis to foxy * updated repos to include moveit2.repos * fixed typo * ament format * more ament formatting * removed code coverage ci test added moveit.repos * fixed travis and repose files, rebased on main * re-added fix travis for custom domain Co-authored-by: Alex Goldman <alex@picknik.ai>

Port planning_scene to ROS2

1445fe7

Authored by @anasarrak

mlautman reviewed Mar 11, 2019

View reviewed changes

davetcoleman reviewed Mar 15, 2019

View reviewed changes

mlautman mentioned this pull request Mar 20, 2019

Convert to ROS2 Logging #21

Closed

planning_scene, adapt logger to moveit#21

3181e29

vmayoral mentioned this pull request Apr 8, 2019

Planning interface, port to ROS 2 #61

Merged

henningkayser previously requested changes Apr 9, 2019

View reviewed changes

moveit_core/planning_scene/include/moveit/planning_scene/planning_scene.h Show resolved Hide resolved

moveit_core/planning_scene/src/planning_scene.cpp Outdated Show resolved Hide resolved

moveit_core/planning_scene/src/planning_scene.cpp Outdated Show resolved Hide resolved

ahcorde added 2 commits May 17, 2019 11:12

planning_scene - applying clang format 3.9

e6f6082

planning_scene smart pointer world passed by value

d5bd6ca

ahcorde force-pushed the planning-scene branch from 0cebbc1 to d5bd6ca Compare May 17, 2019 09:31

Alejandro Hernández Cordero and others added 2 commits May 17, 2019 11:33

Merge branch 'master' into planning-scene

ba1e600

clang

ca02302

mlautman reviewed May 21, 2019

View reviewed changes

moveit_core/planning_scene/src/planning_scene.cpp Outdated Show resolved Hide resolved

moveit_core planning_scene - restoring constructor headers

acd617d

mlautman reviewed May 21, 2019

View reviewed changes

ahcorde added 2 commits May 22, 2019 09:57

moveit_core planning_scene - Fixing Cmakelists.txt and including test…

6d35b17

…s_planning_scene.cpp

move_core planning_scene - Logger inside the namespace

15ba426

henningkayser reviewed May 22, 2019

View reviewed changes

moveit_core planning_scene - fixing CMakeLists.txt

9fa297f

ahcorde mentioned this pull request May 24, 2019

Moveit core CMakeLists.txt #90

Closed

14 tasks

mlautman reviewed May 29, 2019

View reviewed changes

clang and whitespace

01f9e4f

mlautman approved these changes Jun 1, 2019

View reviewed changes

mlautman approved these changes Jun 12, 2019

View reviewed changes

mlautman merged commit 670372e into moveit:master Jun 12, 2019

ahcorde deleted the planning-scene branch June 12, 2019 21:38

JafarAbdi pushed a commit to JafarAbdi/moveit2 that referenced this pull request Oct 28, 2019

Merge pull request moveit#43 from AcutronicRobotics/ci

044a257

Update CI to moveit_ci

MikeWrock pushed a commit to MikeWrock/moveit2 that referenced this pull request Aug 15, 2022

Clang format (moveit#43)

4fda0f6

* clang formatting * adding clang-format notes to README * fixing build warnings

Conversation

vmayoral commented Mar 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mlautman commented May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mlautman left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahcorde commented May 22, 2019

Uh oh!

henningkayser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahcorde commented May 31, 2019

Uh oh!

ahcorde commented Jun 3, 2019

Uh oh!

anasarrak commented Jun 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mlautman commented May 21, 2019 •

edited

Loading

mlautman left a comment •

edited

Loading