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

Fixed compile error when using the latest version of rviz #220

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

svwilliams
Copy link
Contributor

When using the latest melodic version of rviz (1.13.15), I get the following compile error:

from fuse_viz/include/fuse_viz/moc_serialized_graph_display.cpp:9:
/usr/include/c++/7/bits/stl_pair.h: In instantiation of ‘struct std::pair<const std::__cxx11::basic_string<char>, Ogre::ColourValue>’:
...
/usr/include/c++/7/bits/stl_pair.h:215:11: error: ‘std::pair<_T1, _T2>::second’ has incomplete type
       _T2 second;                /// @c second is a copy of the second object
           ^~~~~~
note: forward declaration of ‘class Ogre::ColourValue’
     class ColourValue;
           ^~~~~~~~~~~

The problem seems to be the forward declarations here:
https://github.com/locusrobotics/fuse/blob/devel/fuse_viz/include/fuse_viz/serialized_graph_display.h#L51-L57

But the header needs the full class definitions here:
https://github.com/locusrobotics/fuse/blob/devel/fuse_viz/include/fuse_viz/serialized_graph_display.h#L100

I suspect that the previous version of rviz was including the required headers somewhere, and this file was compiling due to this indirect include. Something in rviz subsequently changed, exposing the error here.

Technically I could leave the SceneNode as a forward declaration, as we only use a pointer to a SceneNode in this header. If that is preferred, or some other solution to this issue is desired, just let me know. I'm not exactly sure why there were forward declared to begin with.

Copy link
Member

@paulbovbel paulbovbel left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Collaborator

@efernandez efernandez left a comment

Choose a reason for hiding this comment

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

I'm using an older version, but this builds fine for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants