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

adding trigger call to animations if (batch_publishing_enabled_) #36

Merged

Conversation

mlautman
Copy link
Contributor

@mlautman mlautman commented Sep 4, 2018

No description provided.

@@ -438,7 +438,8 @@ bool MoveItVisualTools::publishAnimatedGrasps(const std::vector<moveit_msgs::Gra
break;

publishAnimatedGrasp(possible_grasps[i], ee_jmg, animate_speed);

if (batch_publishing_enabled_)
trigger();
ros::Duration(0.1).sleep();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side note: this sleep should be customizable - e.g. pause_between_grasps

Copy link
Contributor Author

@mlautman mlautman Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reuse animate_sleep?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

@@ -438,7 +438,8 @@ bool MoveItVisualTools::publishAnimatedGrasps(const std::vector<moveit_msgs::Gra
break;

publishAnimatedGrasp(possible_grasps[i], ee_jmg, animate_speed);

if (batch_publishing_enabled_)
trigger();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting. it seems to me this trigger actually belongs inside publishAnimatedGrasp() because it has its own for-loop that publishes the motion of the end effector. thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will change accordingly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@davetcoleman
Copy link
Member

@mlautman Did you notice the " // Display Grasp Score" feature inside animate grasps?

@mlautman
Copy link
Contributor Author

@davetcoleman ready for merge?

@davetcoleman
Copy link
Member

i don't see any pushed updates to the code

@mlautman
Copy link
Contributor Author

mlautman commented Sep 13, 2018

My bad. I hadn't pushed

@davetcoleman davetcoleman merged commit deecd1a into moveit:kinetic-devel Sep 14, 2018
@davetcoleman davetcoleman deleted the trigger-animations branch September 14, 2018 01:04
@davetcoleman
Copy link
Member

cherry picked

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

2 participants