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.

Sign up for GitHub

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

Jump to bottom

Add publishMesh example #154

Open
wants to merge 2 commits into
base: eloquent-devel
Choose a base branch
from
Open

Add publishMesh example #154

wants to merge 2 commits into from

Conversation

nbbrooks
Copy link
Member

@nbbrooks nbbrooks commented Jun 2, 2020

  • Add .gitattributes and chess piece .stl files

@nbbrooks
Add publishMesh example
ba9ff88 
- Add .gitattributes and chess piece .stl files
@auto-assign auto-assign bot requested a review from tylerjw June 2, 2020 23:13
@AndyZe
Copy link
Contributor

AndyZe commented Jun 8, 2020 •
edited

I actually need this for one of our projects right now! Let's try to get it merged.

Edit - oh, I need it for ROS1. Well, this is still a nice example to have

AndyZe
AndyZe approved these changes Jun 8, 2020
View reviewed changes
CMakeLists.txt
@@ -198,6 +198,13 @@ install(
RUNTIME DESTINATION lib/${PROJECT_NAME}
)

# Install shared resources
install(DIRECTORY launch DESTINATION share/${PROJECT_NAME})
Copy link
Contributor

Choose a reason for hiding this comment

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

Too many spaces between launch and DESTINATION

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the best thing would be to put DESTINATION on a new line, like at L205

src/rviz_visual_tools_demo.cpp
step = 0.2;
const std::array<double, 6> thetas = { 0.0, 0.0, 0.0, 0.0, 0.0, 0.0 };
const std::array<std::string, 6> meshes = {
"pawn", "knight", "bishop", "rook", "queen", "king"
Copy link
Contributor

@AndyZe AndyZe Jun 8, 2020 •
edited

Choose a reason for hiding this comment

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

I'm not a huge fan of uploading so many STL's, I think one would have been fine. But I guess it's OK

src/rviz_visual_tools_demo.cpp
visual_tools_->publishMesh(pose1 * Eigen::AngleAxisd(thetas[i], Eigen::Vector3d::UnitZ()),
"package://rviz_visual_tools/meshes/" + meshes[i] + ".stl",
mesh_colors[i], scales[i]);
if (i == 0.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose you could use (!i) here

@AndyZe
Copy link
Contributor

AndyZe commented Jun 8, 2020

Also, delete the comment at L596 of rviz_visual_tools_demo.cpp

tylerjw
tylerjw reviewed Nov 16, 2020
View reviewed changes
Copy link
Contributor

@tylerjw tylerjw left a comment

Choose a reason for hiding this comment

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

This is failing for clang-tidy reasons. Please look at the output in travis and apply that diff. If those failures are unrelated to this PR, please make a new PR with those changes and then rebase this one after that one is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tylerjw tylerjw tylerjw left review comments

@AndyZe AndyZe AndyZe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nbbrooks @AndyZe @tylerjw