-
Notifications
You must be signed in to change notification settings - Fork 118
[WIP] ROS2 Port #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] ROS2 Port #101
Changes from 13 commits
2e6b0af
47b88ca
8384400
1c764fa
1c63c60
e78726c
ed75892
772e87d
7ba71c5
7a99b24
f36358d
bcd3b4d
a6cca4d
668929b
3f70f54
8b1d0c0
1a81445
968a750
e91de73
1f64994
76727a2
294ec4c
b1d24cb
b8f2408
010310f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,88 +1,99 @@ | ||||||
| cmake_minimum_required(VERSION 3.1.3) | ||||||
| cmake_minimum_required(VERSION 3.16.3) | ||||||
| project(moveit_calibration_gui) | ||||||
|
|
||||||
| set(CMAKE_CXX_STANDARD 14) | ||||||
| set(CMAKE_CXX_EXTENSIONS OFF) | ||||||
| # Common cmake code applied to all moveit packages | ||||||
| find_package(moveit_common REQUIRED) | ||||||
| moveit_package() | ||||||
|
|
||||||
| if(NOT CMAKE_CONFIGURATION_TYPES AND NOT CMAKE_BUILD_TYPE) | ||||||
| set(CMAKE_BUILD_TYPE Release) | ||||||
| endif() | ||||||
| find_package(ament_cmake REQUIRED) | ||||||
| find_package(ament_index_cpp REQUIRED) | ||||||
| find_package(class_loader REQUIRED) | ||||||
| find_package(geometric_shapes REQUIRED) | ||||||
| find_package(moveit_calibration_plugins REQUIRED) | ||||||
| find_package(moveit_core REQUIRED) | ||||||
| find_package(moveit_ros_perception REQUIRED) | ||||||
| find_package(moveit_ros_planning REQUIRED) | ||||||
| find_package(moveit_ros_planning_interface REQUIRED) | ||||||
| find_package(moveit_ros_visualization REQUIRED) | ||||||
| find_package(moveit_visual_tools REQUIRED) | ||||||
| find_package(pluginlib REQUIRED) | ||||||
| find_package(tf2_eigen REQUIRED) | ||||||
|
|
||||||
| find_package(catkin REQUIRED COMPONENTS | ||||||
| class_loader | ||||||
| cv_bridge | ||||||
| geometric_shapes | ||||||
| image_geometry | ||||||
| moveit_calibration_plugins | ||||||
| moveit_core | ||||||
| moveit_ros_perception | ||||||
| moveit_ros_planning | ||||||
| moveit_ros_planning_interface | ||||||
| moveit_ros_visualization | ||||||
| moveit_visual_tools | ||||||
| pluginlib | ||||||
| rosconsole | ||||||
| roscpp | ||||||
| rviz | ||||||
| rviz_visual_tools | ||||||
| tf2_eigen | ||||||
| ) | ||||||
| find_package(rviz2 REQUIRED) | ||||||
| find_package(rviz_common REQUIRED) | ||||||
| find_package(rviz_default_plugins REQUIRED) | ||||||
| find_package(rviz_rendering REQUIRED) | ||||||
| find_package(rviz_visual_tools REQUIRED) | ||||||
| find_package(rviz_ogre_vendor REQUIRED) | ||||||
|
|
||||||
| find_package(OpenCV REQUIRED) | ||||||
| find_package(eigen3_cmake_module REQUIRED) | ||||||
| find_package(Eigen3 REQUIRED) | ||||||
| find_package(OpenGL REQUIRED) | ||||||
| find_package(GLUT REQUIRED) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we directly depending on GLUT or is this only added because of |
||||||
|
|
||||||
| # Qt Stuff | ||||||
| if(rviz_QT_VERSION VERSION_LESS "5") | ||||||
| find_package(Qt4 ${rviz_QT_VERSION} REQUIRED QtCore QtGui) | ||||||
| include(${QT_USE_FILE}) | ||||||
| macro(qt_wrap_ui) | ||||||
| qt4_wrap_ui(${ARGN}) | ||||||
| endmacro() | ||||||
| else() | ||||||
| find_package(Qt5 ${rviz_QT_VERSION} REQUIRED Core Widgets) | ||||||
| set(QT_LIBRARIES Qt5::Widgets) | ||||||
| macro(qt_wrap_ui) | ||||||
| qt5_wrap_ui(${ARGN}) | ||||||
| endmacro() | ||||||
| endif() | ||||||
| find_package(Qt5 ${rviz_QT_VERSION} REQUIRED Core Widgets) | ||||||
| set(QT_LIBRARIES Qt5::Widgets) | ||||||
| macro(qt_wrap_ui) | ||||||
| qt5_wrap_ui(${ARGN}) | ||||||
| endmacro() | ||||||
|
|
||||||
| set(CMAKE_INCLUDE_CURRENT_DIR ON) | ||||||
| set(CMAKE_AUTOMOC ON) | ||||||
| set(CMAKE_AUTOUIC ON) | ||||||
| set(CMAKE_AUTORCC ON) | ||||||
|
|
||||||
| set(CMAKE_INCLUDE_CURRENT_DIR ON) | ||||||
| add_definitions(-DQT_NO_KEYWORDS) | ||||||
|
|
||||||
| catkin_package( | ||||||
| LIBRARIES | ||||||
| moveit_handeye_calibration_rviz_plugin_core | ||||||
| INCLUDE_DIRS | ||||||
| handeye_calibration_rviz_plugin/include | ||||||
| CATKIN_DEPENDS | ||||||
| moveit_calibration_plugins | ||||||
| set(THIS_PACKAGE_INCLUDE_DEPENDS | ||||||
| ament_index_cpp | ||||||
| Eigen3 | ||||||
| eigen3_cmake_module | ||||||
| moveit_core | ||||||
| moveit_calibration_plugins | ||||||
| moveit_ros_perception | ||||||
| moveit_ros_planning | ||||||
| moveit_ros_planning_interface | ||||||
| moveit_ros_visualization | ||||||
| moveit_visual_tools | ||||||
| roscpp | ||||||
| rviz | ||||||
| rclcpp | ||||||
| rviz_common | ||||||
| rviz_default_plugins | ||||||
| rviz_visual_tools | ||||||
| DEPENDS | ||||||
| EIGEN3 | ||||||
| rviz_rendering | ||||||
| rviz_ogre_vendor | ||||||
| pluginlib | ||||||
| tf2_eigen | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| ) | ||||||
|
|
||||||
| include_directories( | ||||||
| handeye_calibration_rviz_plugin/include | ||||||
| ${Boost_INCLUDE_DIRS} | ||||||
| ${catkin_INCLUDE_DIRS}) | ||||||
| set(THIS_PACKAGE_LIBRARIES | ||||||
| moveit_handeye_calibration_rviz_plugin_core | ||||||
| moveit_handeye_calibration_rviz_plugin | ||||||
| ) | ||||||
|
|
||||||
| include_directories(SYSTEM | ||||||
| ${EIGEN3_INCLUDE_DIRS} | ||||||
| ${QT_INCLUDE_DIR}) | ||||||
| include_directories( | ||||||
| handeye_calibration_rviz_plugin/include | ||||||
| ) | ||||||
| include_directories("${OGRE_PREFIX_DIR}/include") | ||||||
|
|
||||||
| add_subdirectory(handeye_calibration_rviz_plugin) | ||||||
|
|
||||||
| install(FILES | ||||||
| handeye_calibration_rviz_plugin_description.xml | ||||||
| DESTINATION ${CATKIN_PACKAGE_SHARE_DESTINATION}) | ||||||
| pluginlib_export_plugin_description_file(rviz_common handeye_calibration_rviz_plugin_description.xml) | ||||||
|
|
||||||
| install( | ||||||
| TARGETS ${THIS_PACKAGE_LIBRARIES} | ||||||
| EXPORT export_${PROJECT_NAME} | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was recently advised by @tylerjw that this is more standard cmake than the export_ prefix
Suggested change
|
||||||
| LIBRARY DESTINATION lib | ||||||
| ARCHIVE DESTINATION lib | ||||||
| RUNTIME DESTINATION bin | ||||||
| INCLUDES DESTINATION include | ||||||
| ) | ||||||
|
|
||||||
| # if (CATKIN_ENABLE_TESTING) | ||||||
| # find_package(rostest REQUIRED) | ||||||
| # endif() | ||||||
|
|
||||||
| if (CATKIN_ENABLE_TESTING) | ||||||
| find_package(rostest REQUIRED) | ||||||
| endif() | ||||||
| ament_export_targets(export_${PROJECT_NAME} HAS_LIBRARY_TARGET) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: There is also an |
||||||
| ament_export_dependencies(${THIS_PACKAGE_INCLUDE_DEPENDS}) | ||||||
| ament_package() | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,3 @@ | ||
| find_package(OpenCV REQUIRED) | ||
|
|
||
| set(HEADERS | ||
| include/moveit/handeye_calibration_rviz_plugin/handeye_calibration_display.h | ||
| include/moveit/handeye_calibration_rviz_plugin/handeye_calibration_frame.h | ||
|
|
@@ -21,17 +19,26 @@ set(SOURCE_FILES | |
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can remove the comment above that is related to catkin_lint |
||
|
|
||
| set(MOVEIT_LIB_NAME moveit_handeye_calibration_rviz_plugin) | ||
| add_library(${MOVEIT_LIB_NAME}_core ${SOURCE_FILES} ${HEADERS}) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Do you know why this is built with headers and source files instead of using only the source files and including + exporting the headers? |
||
|
|
||
| add_library(${MOVEIT_LIB_NAME}_core SHARED ${SOURCE_FILES} ${HEADERS}) | ||
| set_target_properties(${MOVEIT_LIB_NAME}_core PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}") | ||
| target_link_libraries(${MOVEIT_LIB_NAME}_core | ||
| ${catkin_LIBRARIES} ${OpenCV_LIBS} ${rviz_DEFAULT_PLUGIN_LIBRARIES} ${OGRE_LIBRARIES} ${QT_LIBRARIES} ${Boost_LIBRARIES}) | ||
| target_link_libraries(${MOVEIT_LIB_NAME}_core ${OpenCV_LIBS}) | ||
| ament_target_dependencies(${MOVEIT_LIB_NAME}_core | ||
| rviz2 | ||
| rviz_ogre_vendor | ||
| Qt5 | ||
| pluginlib | ||
| rviz_visual_tools | ||
| moveit_visual_tools | ||
| moveit_core | ||
| moveit_calibration_plugins | ||
| moveit_ros_planning_interface) | ||
| target_include_directories(${MOVEIT_LIB_NAME}_core PRIVATE "${OGRE_PREFIX_DIR}/include") | ||
|
|
||
| add_library(${MOVEIT_LIB_NAME} src/plugin_init.cpp) | ||
| add_library(${MOVEIT_LIB_NAME} SHARED src/plugin_init.cpp) | ||
| set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}") | ||
| target_link_libraries(${MOVEIT_LIB_NAME} ${MOVEIT_LIB_NAME}_core ${catkin_LIBRARIES} ${Boost_LIBRARIES}) | ||
|
|
||
| install(DIRECTORY include/ DESTINATION ${CATKIN_GLOBAL_INCLUDE_DESTINATION}) | ||
| target_link_libraries(${MOVEIT_LIB_NAME} ${MOVEIT_LIB_NAME}_core) | ||
| ament_target_dependencies(${MOVEIT_LIB_NAME} pluginlib rviz_ogre_vendor rviz_visual_tools) | ||
| target_include_directories(${MOVEIT_LIB_NAME} PRIVATE "${OGRE_PREFIX_DIR}/include") | ||
|
|
||
| install(TARGETS ${MOVEIT_LIB_NAME} ${MOVEIT_LIB_NAME}_core | ||
| ARCHIVE DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION} | ||
| LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}) | ||
| install(DIRECTORY include/ DESTINATION include) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ | |
| // qt | ||
|
|
||
| // ros | ||
| #include <rviz_visual_tools/tf_visual_tools.h> | ||
| #include <rviz_visual_tools/tf_visual_tools.hpp> | ||
|
|
||
| // local | ||
| #include <moveit/handeye_calibration_rviz_plugin/handeye_calibration_display.h> | ||
|
|
@@ -48,9 +48,9 @@ | |
| #include <moveit/handeye_calibration_rviz_plugin/handeye_control_widget.h> | ||
|
|
||
| #ifndef Q_MOC_RUN | ||
| #include <ros/ros.h> | ||
| #include <rviz/display_factory.h> | ||
| #include <rviz/display_context.h> | ||
| #include <rclcpp/rclcpp.hpp> | ||
| // #include <rviz_common/display_factory.hpp> Do we need this? | ||
|
Abishalini marked this conversation as resolved.
|
||
| #include <rviz_common/display_context.hpp> | ||
| #endif | ||
|
|
||
| namespace moveit_rviz_plugin | ||
|
|
@@ -66,12 +66,12 @@ class HandEyeCalibrationFrame : public QWidget | |
| Q_OBJECT | ||
|
|
||
| public: | ||
| explicit HandEyeCalibrationFrame(HandEyeCalibrationDisplay* pdisplay, rviz::DisplayContext* context, | ||
| explicit HandEyeCalibrationFrame(HandEyeCalibrationDisplay* pdisplay, rviz_common::DisplayContext* context, | ||
| QWidget* parent = 0); | ||
| ~HandEyeCalibrationFrame() override; | ||
|
|
||
| virtual void loadWidget(const rviz::Config& config); | ||
| virtual void saveWidget(rviz::Config config) const; | ||
| virtual void loadWidget(const rviz_common::Config& config); | ||
| virtual void saveWidget(rviz_common::Config& config) const; | ||
|
|
||
| protected: | ||
| // ****************************************************************************************** | ||
|
|
@@ -83,7 +83,7 @@ class HandEyeCalibrationFrame : public QWidget | |
| ControlTabWidget* tab_control_; | ||
|
|
||
| private: | ||
| rviz::DisplayContext* context_; | ||
| rviz_common::DisplayContext* context_; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a high priority until things are working, but I would strongly advocate replacing all unnecessary raw pointers with smart pointers if possible |
||
| HandEyeCalibrationDisplay* calibration_display_; | ||
|
|
||
| rviz_visual_tools::TFVisualToolsPtr tf_tools_; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure if this is still required (I think it is), but we used to depend on
eigen3_cmake_modulefor Eigen3 to work. See https://github.com/ros2/eigen3_cmake_module