-
Notifications
You must be signed in to change notification settings - Fork 49
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
Filter by suction voxel overlap in parallel #81
base: master
Are you sure you want to change the base?
Filter by suction voxel overlap in parallel #81
Conversation
@@ -413,20 +430,19 @@ class GraspPipelineDemo | |||
double& y_width, double& z_height) | |||
{ | |||
// Generate random cuboid |
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 this be updated? It seems only the polar angle is random now
@@ -111,7 +112,8 @@ struct IkThreadStruct | |||
const planning_scene::PlanningScenePtr& planning_scene, Eigen::Isometry3d& link_transform, | |||
std::size_t grasp_id, kinematics::KinematicsBaseConstPtr kin_solver, | |||
const robot_state::RobotStatePtr& robot_state, double timeout, bool filter_pregrasp, bool visual_debug, | |||
std::size_t thread_id, const std::string& grasp_target_object_id) | |||
std::size_t thread_id, const std::string& grasp_target_object_id, | |||
const moveit_visual_tools::MoveItVisualToolsPtr& visual_tools, double animation_speed) |
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.
animation_speed
I find it a bit confusing when code that is, as far as I can tell, purely used for debugging, is mandatory. Would prefer at least a default value that is obviously invalid, together with documenting how to use it.
@@ -250,6 +264,7 @@ class GraspPipelineDemo | |||
moveit_grasps::GraspCandidatePtr selected_grasp_candidate; | |||
moveit_msgs::MotionPlanResponse pre_approach_plan; | |||
|
|||
// return false; |
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.
Remove?
Seems like lots of dead code in this file overall.
Various small improvements separated by commits.
Biggest change is to filter suction voxel by overlap in parallel.
Some small logging improvements.
Removing ACM entries after they are no longer "allowed"
and adding an error code to string method for filtering