Skip to content

Fixes for #1275 and #1365 #1423

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

Merged
merged 2 commits into from
Sep 20, 2019

Conversation

rhaschke
Copy link
Contributor

@rhaschke rhaschke commented Sep 8, 2019

This fixes some more issues of #1275:

  • invalid quaternion: issue warning only
  • triangle list accepts points.size()/3 colors as well (face colors)
  • don't create (empty) text marker for interactive marker control with empty description
    (to avoid warning)

as well as #1365:

  • use default material for .stl meshes

@v4hn and @simonschmeisser, could you please have a look?

@simonschmeisser
Copy link
Contributor

Thanks for implementing the special handling for STL, I was going to do it the same way. 👍

Generally speaking while this is perfectly compatible with the previous stl loader I wonder about one thing:

  • urdf spec does not specify what to do with materials/colors when the mesh file already supports/includes them. I'd assume the info in URDF overrides the info from the mesh (like we do with stl files containing colors [1]) but my understanding is that we currently just ignore the info from urdf. Which is sufficient I guess.

As I'm at home with a cold I have only slightly tested this code with publicly available data. I'm fine with merging as is but would like to wait with the release a few more days to get better testing.

[1] assimp supports stl files with color data but I have not yet encountered one in the wild ...

@rhaschke rhaschke force-pushed the melodic-devel branch 3 times, most recently from 0467a29 to e0a7cf4 Compare September 10, 2019 06:18
@simonschmeisser
Copy link
Contributor

I did some testing on our actual software and so far it looks good 👍

- invalid quaternion: issue warning only
- triangle list accepts points.size()/3 colors as well (face colors)
- don't create text marker for interactive marker control with empty description
- allow more fine-grained log levels
  - distinguish all ros::console::levels
  - issue rviz status warning for any message, but avoid cluttering the console
  - use logger namespace to allow disabling console output
@rhaschke
Copy link
Contributor Author

I added another fixup for #1275 according to #1120 (comment).

@rhaschke rhaschke merged commit 06539cb into ros-visualization:melodic-devel Sep 20, 2019
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.

3 participants