-
Notifications
You must be signed in to change notification settings - Fork 191
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
Fix configuration of jsk_pcl_ros #1828
Changes from all commits
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 |
---|---|---|
|
@@ -7,21 +7,47 @@ project(jsk_pcl_ros) | |
set(PCL_MSGS pcl_msgs) ## hydro and later | ||
|
||
find_package(catkin REQUIRED COMPONENTS | ||
jsk_recognition_utils jsk_pcl_ros_utils | ||
dynamic_reconfigure pcl_ros nodelet message_generation genmsg | ||
${PCL_MSGS} sensor_msgs geometry_msgs jsk_recognition_msgs | ||
eigen_conversions tf_conversions tf2_ros tf | ||
image_transport nodelet cv_bridge | ||
jsk_topic_tools | ||
cv_bridge | ||
diagnostic_msgs | ||
diagnostic_updater | ||
dynamic_reconfigure | ||
eigen_conversions | ||
genmsg | ||
geometry_msgs | ||
image_geometry | ||
jsk_footstep_msgs | ||
image_transport | ||
image_view2 | ||
interactive_markers | ||
jsk_data | ||
jsk_footstep_msgs | ||
jsk_pcl_ros_utils | ||
jsk_recognition_msgs | ||
jsk_recognition_utils | ||
jsk_topic_tools | ||
kdl_conversions | ||
kdl_parser | ||
laser_assembler | ||
octomap_server | ||
octomap_ros | ||
message_generation | ||
ml_classifiers | ||
moveit_core | ||
nav_msgs | ||
nodelet | ||
octomap_msgs | ||
kdl_parser | ||
kdl_conversions | ||
octomap_ros | ||
octomap_server | ||
pcl_conversions | ||
${PCL_MSGS} | ||
pcl_ros nodelet | ||
rosboost_cfg | ||
roscpp_tutorials | ||
sensor_msgs | ||
std_msgs | ||
std_srvs | ||
stereo_msgs | ||
tf | ||
tf2_ros | ||
tf_conversions | ||
visualization_msgs | ||
) | ||
find_package(PkgConfig) | ||
pkg_check_modules(ml_classifieres ml_classifiers QUIET) | ||
|
@@ -115,6 +141,36 @@ generate_dynamic_reconfigure_options( | |
cfg/PointcloudDatabaseServer.cfg | ||
) | ||
|
||
generate_messages(DEPENDENCIES | ||
${PCL_MSGS} sensor_msgs geometry_msgs jsk_recognition_msgs jsk_footstep_msgs) | ||
|
||
catkin_package( | ||
DEPENDS PCL | ||
# | ||
CATKIN_DEPENDS | ||
diagnostic_msgs | ||
geometry_msgs | ||
pcl_ros | ||
message_runtime | ||
nav_msgs | ||
octomap_msgs | ||
${PCL_MSGS} | ||
sensor_msgs | ||
std_msgs | ||
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. if other message depends on std_msgs, you do not have to add this again. |
||
stereo_msgs | ||
jsk_footstep_msgs | ||
jsk_recognition_msgs | ||
jsk_recognition_utils | ||
visualization_msgs | ||
# | ||
INCLUDE_DIRS include | ||
# | ||
LIBRARIES | ||
jsk_pcl_ros | ||
jsk_pcl_ros_base | ||
jsk_pcl_ros_moveit | ||
) | ||
|
||
find_package(OpenCV REQUIRED core imgproc) | ||
|
||
include_directories(include ${catkin_INCLUDE_DIRS} ${OpenCV_INCLUDE_DIRS} ${PCL_INCLUDE_DIRS}) | ||
|
@@ -409,26 +465,20 @@ target_link_libraries(range_sensor_error_visualization jsk_pcl_ros) | |
add_executable(depth_camera_error_visualization tool/depth_camera_error_visualization.cpp) | ||
target_link_libraries(depth_camera_error_visualization jsk_pcl_ros) | ||
|
||
generate_messages(DEPENDENCIES | ||
${PCL_MSGS} sensor_msgs geometry_msgs jsk_recognition_msgs jsk_footstep_msgs) | ||
|
||
get_property(dirs DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES) | ||
message("flags: ${CMAKE_CXX_FLAGS}") | ||
|
||
catkin_package( | ||
DEPENDS pcl | ||
CATKIN_DEPENDS pcl_ros message_runtime ${PCL_MSGS} sensor_msgs geometry_msgs jsk_recognition_msgs jsk_recognition_utils | ||
INCLUDE_DIRS include | ||
LIBRARIES jsk_pcl_ros jsk_pcl_ros_base jsk_pcl_ros_moveit | ||
) | ||
|
||
# download and install sample data | ||
add_custom_target(install_sample_data ALL COMMAND ${PROJECT_SOURCE_DIR}/scripts/install_sample_data.py) | ||
|
||
install(DIRECTORY include/${PROJECT_NAME}/ | ||
DESTINATION ${CATKIN_PACKAGE_INCLUDE_DESTINATION}) | ||
install(TARGETS jsk_pcl_ros jsk_pcl_ros_base jsk_pcl_ros_moveit | ||
${jsk_pcl_nodelet_executables} | ||
bench_ransac_plane_estimation | ||
depth_camera_error_visualization | ||
range_sensor_error_visualization | ||
sample_cube_nearest_point | ||
RUNTIME DESTINATION ${CATKIN_PACKAGE_BIN_DESTINATION} | ||
ARCHIVE DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION} | ||
LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,7 @@ | |
<build_depend>octomap_ros</build_depend> | ||
<build_depend>jsk_pcl_ros_utils</build_depend> | ||
<build_depend>jsk_data</build_depend> | ||
<build_depend>genmsg</build_depend> | ||
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. do you need to add 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. Removed from build_depend and find_package. |
||
|
||
<!-- <run_depend>pcl</run_depend> --> | ||
<run_depend>eigen_conversions</run_depend> | ||
|
@@ -113,6 +114,7 @@ | |
<run_depend>jsk_pcl_ros_utils</run_depend> | ||
<run_depend>jsk_data</run_depend> | ||
<test_depend>jsk_tools</test_depend> | ||
<test_depend>rostest</test_depend> | ||
<export> | ||
<nodelet plugin="${prefix}/jsk_pcl_nodelets.xml"/> | ||
<cpp lflags="-Wl,-rpath,${prefix}/lib -L${prefix}/lib -ljsk_pcl_ros" cflags="-I${prefix}/include"/> | ||
|
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.
do you need to add these lines? if other package depends on std_msgs, it think we do not have to do that again.
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.
I seems that
jsk_pcl_ros
explicitly depends onstd_msgs
, and this is necessary.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.
i don't think explicit depends does not requires
find_package
norcatkin_package
for example
https://github.com/ros-controls/ros_controllers/blob/kinetic-devel/velocity_controllers/include/velocity_controllers/joint_position_controller.h#L78
uses
#include <std_msgs/Float64.h
butstd_msgs
is not included inhttps://github.com/ros-controls/ros_controllers/blob/kinetic-devel/velocity_controllers/CMakeLists.txt#L5-L27
◉ Kei Okada
On Sun, Aug 7, 2016 at 11:08 PM, Kentaro Wada [email protected]
wrote:
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.
It is just a bug, don't you think?
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.
Any comments, please?
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.
ok, let's if we really need std_msgs
#1846
◉ Kei Okada
On Tue, Aug 23, 2016 at 1:03 PM, Kentaro Wada [email protected]
wrote:
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.
I know currently the std_msgs is not necessary because roscpp depends on it.
jsk_pcl_ros -> roscpp_tutorials -> roscpp -> std_msgs
But if they removed the dependency, the build will fail, right?
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.
Any comments?
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.
In #1846, the commit ( 641cf00) has exactly same packages as original one, just change the order and format. and test (
https://travis-ci.org/jsk-ros-pkg/jsk_recognition/builds/154928142 ) seems Ok, though I haven't restarted 2 failed test,
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.
it seems both tests passed build test, but failed in run_tests
https://travis-ci.org/jsk-ros-pkg/jsk_recognition/jobs/154928148, https://travis-ci.org/jsk-ros-pkg/jsk_recognition/jobs/154928146