-
Notifications
You must be signed in to change notification settings - Fork 947
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
New panel with a slider to control the visualized trajectory #491
Conversation
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.
@Jntzko worked on that for quite a while and spent a bit of work on keeping the current behavior as is.
It is a really nice tool that integrates well with the current interface.
I wholeheartedly approve.
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.
But a few comments this looks really good, I'll build and run it just to confirm that it works well.
animating_path_ = true; | ||
else if (displaying_trajectory_message_) | ||
{ | ||
if (trajectory_slider_panel_->isVisible() && !loop_display_property_->getBool() && |
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 feel that this if statement could be untangled a bit more for the sake of clarity. Both conditions check for trajectory_slider_panel_->isVisible()
, perhaps that could be moved into its own if statement.
} | ||
|
||
private Q_SLOTS: | ||
void sliderValueChanged(int); |
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.
Should there be an argument name here?
I have tested it with the MotionPlanning Display and it works well. I think the Slider panel should be added automatically whenever the MotionPlanning Display is added, otherwise the user may never know that it's there. |
On Wed, Apr 26, 2017 at 01:10:53PM -0700, Jorge Nicho wrote:
I think the Slider panel should be added automatically whenever
the MotionPlanning Display is added, otherwise the user may never know that it's there.
There was some internal debate on this over here..
It is yet another item clobbering RViz when MoveIt is used and in many cases you don't need it.
People didn't miss it too much in everyday usage up to now. They only miss it, when they really *need* to look at the trajectory in that much detail.
Also, for each trajectory display you add (the MotionPlanningDisplay includes one too), you get one of these panels.
Based on these arguments my personal opinion is that it should not be active by default.
My plan was to make advertisement for it with the next release and maybe add a short tutorial.
If more maintainers disagree, this can of course be changed.
|
I love this! Having it off by default seems like a good idea, at least until the next ROS release. Thoughts from the video:
|
} | ||
else | ||
{ | ||
button_->setText("pause"); |
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.
Please capitalize these buttons
trajectory_slider_panel_->getSliderPosition() == displaying_trajectory_message_->getWayPointCount() - 1) | ||
{ // show the last waypoint if the slider is enabled | ||
display_path_robot_->update( | ||
displaying_trajectory_message_->getWayPointPtr(displaying_trajectory_message_->getWayPointCount() - 1)); |
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.
are there proper checks elsewhere for zero-length trajectories being published to this display? If not, this would segfault.
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 we can get a slider position of -1, so the update function will never be called with a zero-length trajectory. Please correct me if I'm wrong.
|
||
void TrajectoryPanel::update(int way_point_count) | ||
{ | ||
last_way_point_ = way_point_count - 1; |
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.
check if way_point_count is zero?
This panel is integrated in the trajectory visualization, that means for the Trajectory display and the MotionPlanning display appears a new panel. This is an extension to the displayed planned path, currently the planned path is shown once after it is planned, if the loop mode is enabled the animation will be repeated. These behaviors will be kept. This panel includes a slider which controls this animated path, it is possible now to display the single waypoints of the trajectory. There is also a play/pause button which starts the animation from the current waypoint to the end and to pause it again. Pressing the play button after the animation is finished replays the trajectory. This can also be used to pause the loop mode.
The slider has one step for every waypoint. The performance is still the same as it was before, we tested it with around 600 waypoints and it still performed well.
The problem is that the trajectory display has also a slider panel, if we set the name to "displayname" + "Trajectory Slider" we get a "Trajectory Trajectory Slider" as default. |
the changed logic clarify the seperate cases we have with the loop mode and the slider.
@davetcoleman @jrgnicho Your feedback has been addressed. Could you have another look 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.
Thanks for addressing the feedback!
) * New panel with a slider controlling the planned path This panel is integrated in the trajectory visualization, that means for the Trajectory display and the MotionPlanning display appears a new panel. This is an extension to the displayed planned path, currently the planned path is shown once after it is planned, if the loop mode is enabled the animation will be repeated. These behaviors will be kept. This panel includes a slider which controls this animated path, it is possible now to display the single waypoints of the trajectory. There is also a play/pause button which starts the animation from the current waypoint to the end and to pause it again. Pressing the play button after the animation is finished replays the trajectory. This can also be used to pause the loop mode. * Simplified logic of the animated planned path with the slider the changed logic clarify the seperate cases we have with the loop mode and the slider.
) * New panel with a slider controlling the planned path This panel is integrated in the trajectory visualization, that means for the Trajectory display and the MotionPlanning display appears a new panel. This is an extension to the displayed planned path, currently the planned path is shown once after it is planned, if the loop mode is enabled the animation will be repeated. These behaviors will be kept. This panel includes a slider which controls this animated path, it is possible now to display the single waypoints of the trajectory. There is also a play/pause button which starts the animation from the current waypoint to the end and to pause it again. Pressing the play button after the animation is finished replays the trajectory. This can also be used to pause the loop mode. * Simplified logic of the animated planned path with the slider the changed logic clarify the seperate cases we have with the loop mode and the slider.
…507) * New panel with a slider controlling the planned path This panel is integrated in the trajectory visualization, that means for the Trajectory display and the MotionPlanning display appears a new panel. This is an extension to the displayed planned path, currently the planned path is shown once after it is planned, if the loop mode is enabled the animation will be repeated. These behaviors will be kept. This panel includes a slider which controls this animated path, it is possible now to display the single waypoints of the trajectory. There is also a play/pause button which starts the animation from the current waypoint to the end and to pause it again. Pressing the play button after the animation is finished replays the trajectory. This can also be used to pause the loop mode. * Simplified logic of the animated planned path with the slider the changed logic clarify the seperate cases we have with the loop mode and the slider.
…508) * New panel with a slider controlling the planned path This panel is integrated in the trajectory visualization, that means for the Trajectory display and the MotionPlanning display appears a new panel. This is an extension to the displayed planned path, currently the planned path is shown once after it is planned, if the loop mode is enabled the animation will be repeated. These behaviors will be kept. This panel includes a slider which controls this animated path, it is possible now to display the single waypoints of the trajectory. There is also a play/pause button which starts the animation from the current waypoint to the end and to pause it again. Pressing the play button after the animation is finished replays the trajectory. This can also be used to pause the loop mode. * Simplified logic of the animated planned path with the slider the changed logic clarify the seperate cases we have with the loop mode and the slider.
Usage for this is suggested moveit/moveit_tutorials#79 |
Add trajectory slider (moveit/moveit#491).
Add trajectory slider (moveit/moveit#491).
Add trajectory slider (moveit/moveit#491).
The new panel includes a slider to control the animated planned path.
It allows to display single waypoints of the trajectory.
There is a play/pause button which starts or pauses the animation at the current waypoint.
Pressing the play button after the animation is finished replays the trajectory.
This can also be used to pause the loop mode.
The panel is integrated into the trajectory visualization and is switched off by default.
That means the Trajectory display and the MotionPlanning display support this new panel and it can be activated in rviz' "Panels" menu entry.
The current behavior of loop and trail is retained.
This video shows the features of the slider.