Handle exception in RobotStateDisplay and print warning to not crash RViz #1267
Conversation
|
Thanks for helping in improving MoveIt! |
moveit_ros/visualization/robot_state_rviz_plugin/src/robot_state_display.cpp
Outdated
Show resolved
Hide resolved
moveit_ros/visualization/robot_state_rviz_plugin/src/robot_state_display.cpp
Outdated
Show resolved
Hide resolved
|
@v4hn Done. |
There was a problem hiding this comment.
After being approved by @v4hn, this PR had several remaining issues:
- the exception was printed twice on the console, from here and here
- the
robot_state_was left in an inconsistent state on an exception - the error status message wasn't reset upon a successful retrieval of the RobotState
- the same exception crash occured for wrong
DisplayTrajectorymsgs
I fixed these issues, pushing directly to @christian-rauch's repo. @v4hn, please approve again.
|
@rhaschke Thanks for pointing to the additional |
| robot_state::robotStateMsgToRobotState(state_msg->state, *robot_state_); | ||
| auto robot_state = std::make_shared<robot_state::RobotState>(robot_model_); | ||
| robot_state::robotStateMsgToRobotState(state_msg->state, *robot_state); | ||
| robot_state_.swap(robot_state); |
There was a problem hiding this comment.
Yes, this makes sense. However, I preferred to skip the allocation because the failure case is very uncommon but the allocation is required on every update.
How about using setToDefaultValues to enforce a safe state in case of error?
|
@v4hn Can you have a final look, please? If you approve, I would like to squash some of the commits for cleanup before finally merging. |
|
Looks good now. |
|
Merged in 47ff403 after some cleanup squashing. |
Description
The RobotStateDisplay plugin crashes RVIz if a robot state with a non-existing joint is provided. See: #1266
This PR simply catches the exception and shows a warning instead of forwarding the exception an have RViz crash.