Skip to content
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

Merged
merged 2 commits into from May 17, 2017

Conversation

Jntzko
Copy link
Contributor

@Jntzko Jntzko commented Apr 25, 2017

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.

Copy link
Contributor

@v4hn v4hn left a 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.

Copy link
Contributor

@jrgnicho jrgnicho left a 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() &&
Copy link
Contributor

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);
Copy link
Contributor

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?

@jrgnicho
Copy link
Contributor

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.

@v4hn
Copy link
Contributor

v4hn commented Apr 26, 2017 via email

@davetcoleman
Copy link
Member

I love this! Having it off by default seems like a good idea, at least until the next ROS release.

Thoughts from the video:

  • Can the title of the panel be "MoveIt! Trajectory Slider"
  • If we change that title, perhaps we could append "MoveIt!" to the regular "MotionPlanning" panel as well
  • Please capitalize "play" to match other format
  • What is the behavior if there are more waypoints, e.g. >100?

}
else
{
button_->setText("pause");
Copy link
Member

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));
Copy link
Member

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.

Copy link
Contributor Author

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;
Copy link
Member

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.
@Jntzko
Copy link
Contributor Author

Jntzko commented Apr 27, 2017

What is the behavior if there are more waypoints, e.g. >100?

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.

Can the title of the panel be "MoveIt! Trajectory Slider"
If we change that title, perhaps we could append "MoveIt!" to the regular "MotionPlanning" panel as 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.
@v4hn
Copy link
Contributor

v4hn commented May 5, 2017

@davetcoleman @jrgnicho Your feedback has been addressed. Could you have another look please?
I would really like to have this in the next release :)

Copy link
Member

@davetcoleman davetcoleman left a 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!

@davetcoleman davetcoleman merged commit 572566f into moveit:indigo-devel May 17, 2017
davetcoleman pushed a commit to davetcoleman/moveit that referenced this pull request May 17, 2017
)

* 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.
davetcoleman pushed a commit to davetcoleman/moveit that referenced this pull request May 17, 2017
)

* 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.
v4hn pushed a commit that referenced this pull request May 17, 2017
…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.
v4hn pushed a commit that referenced this pull request May 17, 2017
…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.
130s added a commit to 130s/moveit_tutorials that referenced this pull request Jun 13, 2017
@130s
Copy link
Contributor

130s commented Jun 13, 2017

Usage for this is suggested moveit/moveit_tutorials#79

130s added a commit to 130s/moveit_tutorials that referenced this pull request Jun 14, 2017
@130s 130s mentioned this pull request Jun 17, 2017
v4hn pushed a commit to moveit/moveit_tutorials that referenced this pull request Jun 19, 2017
130s added a commit to moveit/moveit_tutorials that referenced this pull request Jun 22, 2017
davetcoleman pushed a commit to davetcoleman/moveit_tutorials that referenced this pull request Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants