-
Notifications
You must be signed in to change notification settings - Fork 40
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
Catch ament_index_cpp::PackageNotFoundError #32
Conversation
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
What does it do if you give it a |
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.
lgtm
Just logs a different message
|
But in that case the package does exist? |
In that case the package doesn't exist because I didn't source my tb3 workspace. Here's the collada case again with a different package name.
|
What happens if the |
Weird, maybe it could use more investigation, but it seems like the current behavior is ok for both, even if we don't understand it at the moment. |
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.
lgtm, with CI
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
This fixes a crash in rviz2. It should probably be backported to dashing and eloquent, though I've only tested on the latest ROS 2 sources.
To reproduce:
Save the following as
pkg_dne.urdf
Publish it using
robot_state_publisher
ros2 run robot_state_publisher robot_state_publisher pkg_dne.urdf
Launch rviz2 and add a
RobotModel
display. Tell the display to use the description topic/robot_description
. RViz will immediately crash.With this PR, RViz will log errors but continue to run
Side note, RViz only crashes if the mesh type is
stl
. If I change it todae
then RViz does not crash, even without this PR. I don't know the reason for the difference. @wjwwood is this difference unexpected or something that should be looked at?