Port to ROS 2 by vmayoral · Pull Request #96 · moveit/geometric_shapes

vmayoral · 2019-02-10T15:53:04Z

Note to maintainers: Please do not merge in this branch, create a ros2 branch and modify this PR.

With the changes provided, package compiles in a ROS 2 workspace.

Pending items:

Review warnings
Port testing

For completeness, warnings pending of review:

In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shapes.cpp:39:
In file included from /usr/local/include/octomap/octomap.h:37:
In file included from /usr/local/include/octomap/OcTree.h:38:
In file included from /usr/local/include/octomap/OccupancyOcTreeBase.h:506:
/usr/local/include/octomap/OccupancyOcTreeBase.hxx:116:108: warning: unused parameter 'maxrange' [-Wunused-parameter]
  void OccupancyOcTreeBase<NODE>::insertPointCloudRays(const Pointcloud& pc, const point3d& origin, double maxrange, bool lazy_eval) {
                                                                                                           ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shapes.cpp:40:
/usr/local/include/console_bridge/console.h:70:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                       ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shapes.cpp:265:34: warning: unused parameter 'scale' [-Wunused-parameter]
void OcTree::scaleAndPadd(double scale, double padd)
                                 ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shapes.cpp:265:48: warning: unused parameter 'padd' [-Wunused-parameter]
void OcTree::scaleAndPadd(double scale, double padd)
                                               ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shapes.cpp:40:
/usr/local/include/console_bridge/console.h:70:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                       ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shapes.cpp:270:33: warning: unused parameter 'scale' [-Wunused-parameter]
void Plane::scaleAndPadd(double scale, double padding)
                                ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shapes.cpp:270:47: warning: unused parameter 'padding' [-Wunused-parameter]
void Plane::scaleAndPadd(double scale, double padding)
                                              ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/mesh_operations.cpp:49:
/usr/local/include/assimp/scene.h:240:1: warning: 'aiScene' defined as a struct here but previously declared as a class [-Wmismatched-tags]
struct aiScene
^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/include/geometric_shapes/mesh_operations.h:45:1: note: did you mean struct here?
class aiScene;
^~~~~
struct
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/body_operations.cpp:39:
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                         ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shape_operations.cpp:45:
/usr/local/include/console_bridge/console.h:70:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                       ^
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                         ^
/usr/local/include/console_bridge/console.h:67:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                       ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shape_operations.cpp:158:49: warning: unused parameter 'shape_msg' [-Wunused-parameter]
  void operator()(const shape_msgs::msg::Plane& shape_msg) const
                                                ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shape_operations.cpp:45:
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                         ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shape_operations.cpp:205:60: warning: unused parameter 'shape_msg' [-Wunused-parameter]
  Eigen::Vector3d operator()(const shape_msgs::msg::Plane& shape_msg) const
                                                           ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/shape_operations.cpp:45:
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                         ^
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
7 warnings generated.
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/bodies.cpp:131:67: warning: unused parameter 'verbose' [-Wunused-parameter]
bool bodies::Sphere::containsPoint(const Eigen::Vector3d& p, bool verbose) const
                                                                  ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/mesh_operations.cpp:46:
/usr/local/include/console_bridge/console.h:67:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                       ^
/usr/local/include/console_bridge/console.h:70:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                       ^
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                         ^
/usr/local/include/console_bridge/console.h:70:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                         ^
/usr/local/include/console_bridge/console.h:70:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
/usr/local/include/console_bridge/console.h:70:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                       ^
/usr/local/include/console_bridge/console.h:70:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                         ^
/usr/local/include/console_bridge/console.h:70:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
/usr/local/include/console_bridge/console.h:70:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                         ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/bodies.cpp:264:69: warning: unused parameter 'verbose' [-Wunused-parameter]
bool bodies::Cylinder::containsPoint(const Eigen::Vector3d& p, bool verbose) const
                                                                    ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/bodies.cpp:317:99: warning: unused parameter 'max_attempts' [-Wunused-parameter]
bool bodies::Cylinder::samplePointInside(random_numbers::RandomNumberGenerator& rng, unsigned int max_attempts,
                                                                                                  ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/bodies.cpp:477:64: warning: unused parameter 'verbose' [-Wunused-parameter]
bool bodies::Box::containsPoint(const Eigen::Vector3d& p, bool verbose) const
                                                               ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/bodies.cpp:665:71: warning: unused parameter 'verbose' [-Wunused-parameter]
bool bodies::ConvexMesh::containsPoint(const Eigen::Vector3d& p, bool verbose) const
                                                                      ^
In file included from /Users/victor/ros2_moveit_ws/src/geometric_shapes/src/bodies.cpp:40:
/usr/local/include/console_bridge/console.h:70:88: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_WARN,  fmt, ##__VA_ARGS__)
                                                                                       ^
/Users/victor/ros2_moveit_ws/src/geometric_shapes/src/mesh_operations.cpp:633:13: warning: unused function 'writeFloatToSTL' [-Wunused-function]
inline void writeFloatToSTL(char*& ptr, float data)
            ^
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  console_bridge::log(__FILE__, __LINE__, console_bridge::CONSOLE_BRIDGE_LOG_ERROR, fmt, ##__VA_ARGS__)
                                                                                         ^
/usr/local/include/console_bridge/console.h:67:90: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
2 warnings generated.
8 warnings generated.
12 warnings generated.
8 warnings generated.

Replaced by shape_msgs::msg::SolidPrimitive

rhaschke · 2019-02-10T15:56:02Z

Directed at ros2 branch.

Requires further work

…_shapes into ros2

vmayoral · 2019-02-24T13:08:57Z

Ported a few bits of the build testing infrastructure but it requires further work. Help is welcome @rhaschke, @davetcoleman, @gavanderhoorn and others.

Change bodies Eigen::Issometry calls to Eigen::Affine3d

gavanderhoorn · 2019-02-28T17:34:16Z

include/geometric_shapes/bodies.h


  /** \brief Construct a body vector from a vector of shapes, a vector of poses and a padding */
-  BodyVector(const std::vector<shapes::Shape*>& shapes, const EigenSTL::vector_Isometry3d& poses, double padding = 0.0);
+  BodyVector(const std::vector<shapes::Shape*>& shapes, const EigenSTL::vector_Affine3d& poses, double padding = 0.0);


I understand that this is currently blocking you in your porting efforts, but it seems more efficient and forward thinking to me to assume that the Isometry3d migration will happen in all ROS pkgs that use Eigen::Affine3d.

Instead of reverting all those changes in MoveIt and related pkgs, perhaps forward-porting the Isometry3d changes to the pkgs that should have it would be more efficient in the long run.

+1 we do not want to revert back to affine3d

davetcoleman · 2019-03-01T07:50:43Z

CMakeLists.txt

+#   find_package(ament_cmake_gtest REQUIRED)
+#   # Unit tests
+#   add_subdirectory(test)
+# endif()


enable tests

davetcoleman · 2019-03-01T07:51:44Z

package.xml

+<package format="3">
  <name>geometric_shapes</name>
-  <version>0.6.1</version>
+  <version>0.6.2</version>


is it necessary to bump the version here? my understanding is that only bloom does this during the release process @130s ?

It's ok to modify version number along with other code change although that sounds unusual. IMO it's typically done during release flow, e.g. in Catkin's world, catkin_prepare_release nicely takes care of it automatically. Using a toolchain like that is preferable as it allows release flow to be more systematic and organized.

And the change suggested in this PR seems non-trivial, so bumping minor or even major number makes more sense to me (if the repo follows common major-minor-patch version semantics).

We should keep ROS and ROS2 versions separate. In the RQT port we changed all of the ROS2 versions to 1.0.0 although I am not sure that is ideal

Return back to Isometry3d

mlautman · 2019-03-07T22:16:31Z

CMakeLists.txt

+#   find_package(ament_cmake_gtest REQUIRED)
+#   # Unit tests
+#   add_subdirectory(test)
+# endif()


mlautman · 2019-03-07T22:17:15Z

CMakeLists.txt

-        ARCHIVE DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
-        LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION})
+        ARCHIVE DESTINATION lib
+      LIBRARY DESTINATION lib)


fix alignment

mlautman · 2019-03-07T22:18:30Z

CMakeLists.txt

+        ARCHIVE DESTINATION lib
+      LIBRARY DESTINATION lib)

 install(DIRECTORY include/${PROJECT_NAME}/


you can use include_directories(include) no?

mlautman · 2019-03-07T22:19:25Z

include/geometric_shapes/bodies.h


  /** \brief Get a clone of this body, but one that is located at the pose \e pose */
-  BodyPtr cloneAt(const Eigen::Isometry3d& pose) const
+  BodyPtr cloneAt(const Eigen::Isometry3d& pose)


Why are we mixing code changes with porting changes? Is this actually necessary for ROS2?

mlautman · 2019-03-07T23:40:51Z

include/geometric_shapes/bodies.h

      padding and \e scaling. This function is useful to implement
      thread safety, when bodies need to be moved around. */
-  virtual BodyPtr cloneAt(const Eigen::Isometry3d& pose, double padding, double scaling) const = 0;
+  virtual BodyPtr cloneAt(const Eigen::Isometry3d& pose, double padding, double scaling);


Same as above.

mlautman · 2019-03-07T23:41:15Z

include/geometric_shapes/shape_extents.h


-#include <shape_msgs/SolidPrimitive.h>
-#include <shape_msgs/Mesh.h>
+#include "shape_msgs/msg/solid_primitive.hpp"


Is there a reason to switch from <> to ""?

mlautman · 2019-03-07T23:43:26Z

package.xml

+<package format="3">
  <name>geometric_shapes</name>
-  <version>0.6.1</version>
+  <version>0.6.2</version>


We should keep ROS and ROS2 versions separate. In the RQT port we changed all of the ROS2 versions to 1.0.0 although I am not sure that is ideal

…t#101) * consolidate finding absolute paths for ASSIMP_LIBRARIES * Add comments. Co-Authored-By: seanyen <seanyen@microsoft.com>

Fix cmakelists to compile against other executables and fixing tests

reformat it slightly as well. This commit allows linking in OS X but requires console_bridge to be installed from sources. Note that console_bridge_vendor is a shim over console_bridge and still requires the installation of the later through its sources.

Fix packages.xml

…#104)

rhaschke · 2019-11-17T16:09:37Z

Closing in favor of #118 / #122.

vmayoral · 2019-11-17T16:54:51Z

@rhaschke tons of changes at #122 that removed authorship. E.g. @ahcorde contributed with 25255f3. This is wrong.

vmayoral added 7 commits February 10, 2019 16:24

Port headers to ROS 2

264c3e5

Adapt shape_msgs::SolidPrimitive to ROS 2

1066642

Replaced by shape_msgs::msg::SolidPrimitive

Adapt shape_msgs::Mesh to ROS 2

a57c4ac

Adapt visualization_msgs::Marker to ROS 2

cc16b0b

Adapt geometry_msgs usage to ROS 2

264a092

Adapt a variety of primitives to ROS 2

bf68b13

Port CMakeLists.txt and package.xml

c326d54

rhaschke changed the base branch from melodic-devel to ros2 February 10, 2019 15:55

anasarrak and others added 3 commits February 22, 2019 10:48

Update setpose to be used with Affine3d

19a1137

Few bits for porting build testing to ROS 2

67cd903

Requires further work

Merge branch 'ros2' of https://github.com/AcutronicRobotics/geometric…

3420b4e

…_shapes into ros2

anasarrak and others added 2 commits February 28, 2019 15:55

Change bodies Eigen::Issometry calls to Eigen::Affine3d

9424d11

Merge pull request #1 from anasarrak/ros2

c498e9d

Change bodies Eigen::Issometry calls to Eigen::Affine3d

gavanderhoorn reviewed Feb 28, 2019

View reviewed changes

davetcoleman requested changes Mar 1, 2019

View reviewed changes

anasarrak and others added 2 commits March 5, 2019 10:10

Return back to Isometry3d

678d3bd

Merge pull request #2 from anasarrak/ros2

751f114

Return back to Isometry3d

davetcoleman requested a review from mlautman March 6, 2019 05:25

mlautman changed the base branch from ros2 to crystal-devel March 7, 2019 22:14

mlautman reviewed Mar 7, 2019

View reviewed changes

seanyen and others added 8 commits April 2, 2019 21:43

[Windows][melodic] Fix ASSIMP libraries path for Windows build (movei…

b1bcd7c

…t#101) * consolidate finding absolute paths for ASSIMP_LIBRARIES * Add comments. Co-Authored-By: seanyen <seanyen@microsoft.com>

Fix cmakelists to compile against other executables and fixing tests

5232ad0

Converting into a SHARED library

25255f3

Merge pull request #3 from AcutronicRobotics/fix-cmake

ff1bf18

Fix cmakelists to compile against other executables and fixing tests

Add ASSIMP and console_bridge

e02b97d

reformat it slightly as well. This commit allows linking in OS X but requires console_bridge to be installed from sources. Note that console_bridge_vendor is a shim over console_bridge and still requires the installation of the later through its sources.

Fix packages.xml to fit with CMakeLists.txt.

b1bd9bb

Merge pull request #4 from kazuki0824/ros2

a7c6287

Fix packages.xml

Added bodies::Body::computeBoundingBox (aligned box version). (moveit…

f8ca209

…#104)

Merge branch 'melodic-devel' into ros2

341196e

henningkayser mentioned this pull request Nov 15, 2019

Port to ROS 2 dashing (new branch) #118

Closed

rhaschke closed this Nov 17, 2019

rhaschke mentioned this pull request Nov 17, 2019

Port to ROS2 #122

Merged

Conversation

vmayoral commented Feb 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhaschke commented Feb 10, 2019

Uh oh!

vmayoral commented Feb 24, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhaschke commented Nov 17, 2019

Uh oh!

vmayoral commented Nov 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

vmayoral commented Feb 10, 2019 •

edited

Loading