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

Additional visualization #31

Merged
merged 3 commits into from Aug 23, 2018

Conversation

Projects
None yet
3 participants
@mlautman
Copy link
Member

commented Aug 21, 2018

I also ran clang formatting

@mlautman mlautman requested a review from davetcoleman Aug 21, 2018

@davetcoleman

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

please add better PR title in the future

if (!collision_obj.primitives[0].dimensions[shape_msgs::SolidPrimitive::BOX_Z])
collision_obj.primitives[0].dimensions[shape_msgs::SolidPrimitive::BOX_Z] = rviz_visual_tools::SMALL_SCALE;

// ROS_INFO_STREAM_NAMED(name_,"CollisionObject: \n " << collision_obj);

This comment has been minimized.

Copy link
@davetcoleman
@@ -347,6 +347,12 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
const std::string& name,
const rviz_visual_tools::colors& color = rviz_visual_tools::GREEN);

bool publishCollisionCuboid(const Eigen::Affine3d& pose, double x, double y, double z, const std::string& name,

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Aug 22, 2018

Member

please add function documentation

* \param pose - position of the centroid of the cube
* \param x - width
* \param y - depth
* \param z - height

This comment has been minimized.

Copy link
@davetcoleman

davetcoleman Aug 22, 2018

Member

the standard in this file is to call these variables width, depth, height - not x,y,z which imply position

This comment has been minimized.

Copy link
@mlautman

mlautman Aug 23, 2018

Author Member

done

@mlautman mlautman force-pushed the mlautman:additional-vis branch from 1a05bba to d16e3cf Aug 23, 2018

@davetcoleman davetcoleman merged commit 884dae7 into ros-planning:kinetic-devel Aug 23, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

davetcoleman added a commit that referenced this pull request Aug 23, 2018

Additional visualization (#31)
* adding a visualization for publishing a box with width, height, and depth

* adding additional publishCollisionCuboid method overloads

* changing x,y,z to width, depth, height
@davetcoleman

This comment has been minimized.

Copy link
Member

commented Aug 23, 2018

cherry-picked to M

@rhaschke

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2018

@davetcoleman @mlautman Cherry-picking this to Melodic broke both this repo's melodic-branch as well as MoveIt's Travis (which is pulling in moveit_visual_tools via rosinstall file):
https://travis-ci.org/ros-planning/moveit/builds/421224125

Please fix.

@mlautman mlautman deleted the mlautman:additional-vis branch Aug 27, 2018

@mlautman

This comment has been minimized.

Copy link
Member Author

commented Aug 27, 2018

@rhaschke Working on it

@davetcoleman

This comment has been minimized.

Copy link
Member

commented Sep 6, 2018

Just did a post-mortem with @mlautman and the takeaway is when doing direct cherry-picks between distros, check for anything TF related and if that is mentioned, do a full PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.