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

Robot: Report mesh loading issues #744

Merged
merged 3 commits into from
Sep 17, 2021
Merged

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Aug 17, 2021

Signed-off-by: ahcorde ahcorde@gmail.com

This PR is related to this PR ros-visualization/rviz#1629 in RVIZ 1.

This PR will allow to visualize the errors of an invalid URDF in the RVIZ 2 GUI.

Selección_108

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde self-assigned this Aug 17, 2021
@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 13, 2021

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@@ -179,6 +179,8 @@ void RobotLinkSelectionHandler::postRenderPass(uint32_t pass)
}
}

static std::map<const RobotLink *, std::string> errors;
Copy link
Contributor

Choose a reason for hiding this comment

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

I have to say that this seems like a weird way to do this. Can't we just have std::string error as a member variable in the RobotLink class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

One nit inline, but I'll approve anyway.

vsnprintf(buffer, sizeof(buffer), format, args);
va_end(args);

std::string & err = const_cast<std::string &>(getGeometryErrors());
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Since this is all in the same class, can we just use the error member variable instead (rather than calling a method and doing the cast)?

Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Looks good to me with green CI.

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 17, 2021

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit e667cc1 into ros2 Sep 17, 2021
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/report_mesh_loading_issues branch September 17, 2021 16:59
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2021-9-16/22372/1

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

3 participants