search

ros-drivers / flir_ptu

ROS driver for FLIR pan-tilt units
http://wiki.ros.org/flir_ptu_driver
14 stars 35 forks source link

Remove the flir_ptu_viz package #49

Closed civerachb-cpr closed 2 years ago

civerachb-cpr commented 2 years ago

The flir_ptu_viz package fails tests when building on noetic and isn't necessary; when the URDF is added to an existing robot that robot's view_robot/view_model launch files can be used, or rviz can be launched manually and the robot model added.

tonybaltovski commented 2 years ago

@civerachb-cpr what fails? Can we try to fix the issue rather than removing it?

civerachb-cpr commented 2 years ago

@tonybaltovski the issue is that the roslaunch test consistently fail on noetic:

CMake Error at /opt/ros/noetic/share/catkin/cmake/test/tests.cmake:174 (add_dependencies):
  The dependency target "rviz" of target
  "run_tests_flir_ptu_viz_roslaunch-check_launch_view_ptu.launch" does not
  exist.
Call Stack (most recent call first):
  /opt/ros/noetic/share/roslaunch/cmake/roslaunch-extras.cmake:66 (catkin_run_tests_target)
  flir_ptu/flir_ptu_viz/CMakeLists.txt:9 (roslaunch_add_file_check)

CMake Error at /opt/ros/noetic/share/catkin/cmake/test/tests.cmake:190 (add_dependencies):
  The dependency target "robot_state_publisher" of target
  "_run_tests_flir_ptu_viz_roslaunch-check_launch_view_model.launch" does not
  exist.
Call Stack (most recent call first):
  /opt/ros/noetic/share/roslaunch/cmake/roslaunch-extras.cmake:66 (catkin_run_tests_target)
  flir_ptu/flir_ptu_viz/CMakeLists.txt:7 (roslaunch_add_file_check)

CMake Error at /opt/ros/noetic/share/catkin/cmake/test/tests.cmake:174 (add_dependencies):
  The dependency target "robot_state_publisher" of target
  "run_tests_flir_ptu_viz_roslaunch-check_launch_view_model.launch" does not
  exist.
Call Stack (most recent call first):
  /opt/ros/noetic/share/roslaunch/cmake/roslaunch-extras.cmake:66 (catkin_run_tests_target)
  flir_ptu/flir_ptu_viz/CMakeLists.txt:7 (roslaunch_add_file_check)

CMake Error at /opt/ros/noetic/share/catkin/cmake/test/tests.cmake:190 (add_dependencies):
  The dependency target "rviz" of target
  "_run_tests_flir_ptu_viz_roslaunch-check_launch_view_ptu.launch" does not
  exist.
Call Stack (most recent call first):
  /opt/ros/noetic/share/roslaunch/cmake/roslaunch-extras.cmake:66 (catkin_run_tests_target)
  flir_ptu/flir_ptu_viz/CMakeLists.txt:9 (roslaunch_add_file_check)

The dependencies themselves do exist, and I've been scratching my head trying to figure out why these tests are failing. But given the _viz package's redundancy with other similar packages, it seemed reasonable (to me at least) to simply remove this package completely. It doesn't do anything that isn't already done by other robot visualization packages.

A lateral benefit to removing that package from this repo is that anyone cloning onto a headless robot won't accidentally pull in Rviz + its dependencies with rosdep if they forget/don't realize they should CATKIN_IGNORE the _viz directory.

civerachb-cpr commented 2 years ago

Closing this; there are some additional changes I want to make to enable better simulation support that also fix the problems with the _viz package.