Port kinematic_constraints to ROS2 #42

vmayoral · 2019-03-10T17:13:29Z

@anasarrak

mlautman · 2019-03-11T22:36:56Z

moveit_core/kinematic_constraints/include/moveit/kinematic_constraints/utils.h

@@ -200,7 +200,7 @@ moveit_msgs::msg::Constraints constructGoalConstraints(const std::string& link_n
 *
 * @return was the construction successful?
 */
-bool constructConstraints(XmlRpc::XmlRpcValue& params, moveit_msgs::msg::Constraints& constraints);
+// bool constructConstraints(XmlRpc::XmlRpcValue& params, moveit_msgs::msg::Constraints& constraints);


Add TODO with missing dependency. Was this removed because xmlrpc is not available? If so, it looks like it is used in the ros1_bridge

We can actually use the rclcpp parameters to fix these kind of things, or at least, the way that I'm following.

@v4hn spent a lot of time on this feature. Perhaps you and he can collaborate on the best path for porting this. In the mean time, any time you comment out a feature, please add comments explaining why the feature is being removed and what steps would be necessary before re-enabling the feature

moveit_core/kinematic_constraints/src/utils.cpp

mlautman · 2019-03-11T22:42:25Z

moveit_core/kinematic_constraints/src/utils.cpp

-  constraints.name = static_cast<std::string>(params["name"]);
-  return collectConstraints(params["constraints"], constraints);
-}
+// TODO: Is not getting called from anywhere, check if is necessary


This is intended to be used by application code. It enables the user to define a set of constraints in a config file and use it for both the generation of an approximated constraint manifold and the sampling of that manifold. Without this code those parameters have to be defined multiple times and may get out of sync. The tutorial on approximated constraint manifolds has some more information: https://ros-planning.github.io/moveit_tutorials/doc/planning_with_approximated_constraint_manifolds/planning_with_approximated_constraint_manifolds_tutorial.html

I believe @v4hn is working on documenting this feature better

henningkayser · 2019-03-18T16:41:04Z

moveit_core/kinematic_constraints/src/kinematic_constraint.cpp

  delete m;
  mk.header.frame_id = robot_model_->getModelFrame();
-  mk.header.stamp = ros::Time::now();
+  mk.header.stamp = stamp;


Why not use clock_ros_.now() directly? I don't see stamp being used anywhere else.

Changes were made on the following commit b3855c3

henningkayser · 2019-03-18T16:51:52Z

moveit_core/kinematic_constraints/src/kinematic_constraint.cpp

@@ -848,25 +851,26 @@ shapes::Mesh* VisibilityConstraint::getVisibilityCone(const robot_state::RobotSt
 }

 void VisibilityConstraint::getMarkers(const robot_state::RobotState& state,
-                                      visualization_msgs::MarkerArray& markers) const
+                                      visualization_msgs::msg::MarkerArray& markers)


getMarkers() should be const. Did you change it because of clock_ros_? In this case I would suggest creating a local instance.

Even better, just use rclcpp::Time::now(RCL_ROS_TIME);

Reverted back here AcutronicRobotics@beb3bca

henningkayser · 2019-03-18T16:52:50Z

moveit_core/kinematic_constraints/src/kinematic_constraint.cpp

@@ -991,7 +995,8 @@ ConstraintEvaluationResult VisibilityConstraint::decide(const robot_state::Robot
  collision_detection::CollisionRequest req;
  collision_detection::CollisionResult res;
  collision_detection::AllowedCollisionMatrix acm;
-  acm.setDefaultEntry("cone", boost::bind(&VisibilityConstraint::decideContact, this, _1));
+  acm.setDefaultEntry("cone", (boost::function<bool(collision_detection::Contact&)>)


Why is this function cast necessary?

@ahcorde ping

I can't explain you exactly why we have this cast. Maybe @anasarrak can jump here and add some light. But I was trying today to change this to the current version but it was not working. My guess is because we are using C++14. I have tried to change this to std using std::function and std::bind because most of the packages are moving from boost to std but again I was not able to do it.

Anyhow help is wellcome.

I fixed this with this commit b3855c3

What do you think?

It was causing some compilation problems, the cast prevented the problem.

Still not building but fixes ROS 2 APIs

mlautman · 2019-05-29T19:20:56Z

moveit_core/kinematic_constraints/CMakeLists.txt

+if(WIN32)
+	# set(append_library_dirs "$<TARGET_FILE_DIR:${PROJECT_NAME}>;$<TARGET_FILE_DIR:${PROJECT_NAME}_TestPlugins1>")
+else()
+	set(append_library_dirs "${CMAKE_CURRENT_BINARY_DIR};${CMAKE_CURRENT_BINARY_DIR}/../utils;${CMAKE_CURRENT_BINARY_DIR}/../robot_model;${CMAKE_CURRENT_BINARY_DIR}/../robot_state;${CMAKE_CURRENT_BINARY_DIR}/../collision_detection_fcl")


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 works 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.

moveit_core/kinematic_constraints/CMakeLists.txt

moveit_core/kinematic_constraints/include/moveit/kinematic_constraints/kinematic_constraint.h

moveit_core/kinematic_constraints/src/kinematic_constraint.cpp

…LOGGER_KINEMATIC_CONSTRAINTS

ahcorde · 2019-06-03T08:54:15Z

friendly ping @mlautman @henningkayser @davetcoleman

mlautman · 2019-06-03T22:57:28Z

@ahcorde Please address all feedback. I see some open questions from @henningkayser

anasarrak · 2019-06-06T10:06:50Z

Ping @mlautman @davetcoleman @henningkayser

mlautman · 2019-06-12T21:28:47Z

@henningkayser can you re-review this

ahcorde · 2019-06-24T07:36:19Z

friendly ping @mlautman @henningkayser @davetcoleman

Can we merge this?

stale

* visualization * adding moveit_visual_tools depend * install panda from src instead of deb * minor grammar fix * fixing maintainer list * minor typeo

Port kinematic_constraints to ROS2

7cdb756

Authored by @anasarrak

mlautman reviewed Mar 11, 2019

View reviewed changes

henningkayser previously requested changes Mar 18, 2019

View reviewed changes

vmayoral added 2 commits March 30, 2019 21:50

Merge branch 'moveit2' into kinematic-constraints

0f079c9

kinematic-constraints, first steps towards unit tests

5ace7e8

Still not building but fixes ROS 2 APIs

vmayoral mentioned this pull request Apr 8, 2019

Planning interface, port to ROS 2 #61

Merged

ahcorde added 4 commits May 24, 2019 13:14

Fixing kinematic constraints tests

8d49463

kinematic_constraints adding library dependencies

d70f888

moveit_core kinematics_constraints - addressing some issues

beb3bca

Merge branch 'master' into kinematic-constraints

61cdfba

ahcorde mentioned this pull request May 24, 2019

Moveit core CMakeLists.txt #90

Closed

14 tasks

mlautman reviewed May 29, 2019

View reviewed changes

moveit_core/kinematic_constraints/CMakeLists.txt Outdated Show resolved Hide resolved

mlautman reviewed May 29, 2019

View reviewed changes

moveit_core/kinematic_constraints/include/moveit/kinematic_constraints/kinematic_constraint.h Outdated Show resolved Hide resolved

mlautman reviewed May 29, 2019

View reviewed changes

moveit_core/kinematic_constraints/src/kinematic_constraint.cpp Outdated Show resolved Hide resolved

ahcorde added 4 commits May 29, 2019 23:32

moveit_core kinematics_constraints - logger_kinematic_constraints to …

6b1dd61

…LOGGER_KINEMATIC_CONSTRAINTS

moveit_core kinematics_constraints - remove duplicated getMarkers

57f271e

moveit_core kinematics_constraints - fix whitespaces in CMakeLists.txt

c6aa8e6

moveit_core kinematics_constraints - fix whitespaces in CMakeLists.txt

df09d3a

ahcorde force-pushed the kinematic-constraints branch from c4e2f69 to df09d3a Compare May 29, 2019 21:46

moveit_core kinematic_constraints - Fix getMarkers header

da78630

ahcorde and others added 3 commits June 4, 2019 17:19

moveit_core kinematics constraints - converting boost to std

b3855c3

Merge branch 'master' into kinematic-constraints

4ff5652

Remove not necessary stamp

f4a7b2d

mlautman approved these changes Jun 12, 2019

View reviewed changes

mlautman merged commit 3405196 into moveit:master Jun 25, 2019

anasarrak mentioned this pull request Jun 26, 2019

Update master AcutronicRobotics/moveit2#123

Merged

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

Runthrough (moveit#42)

7c66e21

* visualization * adding moveit_visual_tools depend * install panda from src instead of deb * minor grammar fix * fixing maintainer list * minor typeo

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port kinematic_constraints to ROS2 #42

Port kinematic_constraints to ROS2 #42

vmayoral commented Mar 10, 2019

mlautman Mar 11, 2019

anasarrak Mar 18, 2019

mlautman Mar 18, 2019

mlautman Mar 11, 2019

henningkayser Mar 18, 2019

anasarrak Jun 6, 2019

henningkayser Mar 18, 2019 •

edited

Loading

henningkayser Mar 18, 2019

anasarrak Jun 6, 2019

henningkayser Mar 18, 2019 •

edited

Loading

mlautman Jun 3, 2019

ahcorde Jun 4, 2019 •

edited

Loading

ahcorde Jun 4, 2019

anasarrak Jun 6, 2019

mlautman May 29, 2019

ahcorde May 29, 2019

ahcorde commented Jun 3, 2019

mlautman commented Jun 3, 2019

anasarrak commented Jun 6, 2019

mlautman commented Jun 12, 2019

ahcorde commented Jun 24, 2019

Port kinematic_constraints to ROS2 #42

Port kinematic_constraints to ROS2 #42

Conversation

vmayoral commented Mar 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henningkayser Mar 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henningkayser Mar 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahcorde Jun 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahcorde commented Jun 3, 2019

mlautman commented Jun 3, 2019

anasarrak commented Jun 6, 2019

mlautman commented Jun 12, 2019

ahcorde commented Jun 24, 2019

henningkayser Mar 18, 2019 •

edited

Loading

henningkayser Mar 18, 2019 •

edited

Loading

ahcorde Jun 4, 2019 •

edited

Loading