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

Use pluginlib to reenable displays panel and load basic plugins #123

Merged
merged 41 commits into from
Dec 5, 2017

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Nov 27, 2017

This pr does a bunch of stuff:

  • sets up the rviz_default_plugins package
  • uses the new ros2 branch for pluginlib:
  • finishes migrating files that used pluginlib and that were skipped over in previous refactorings
  • reeanbled the displays plugin
  • changed some of the libraries to SHARED and changed other linking settings to address issue with multiple Ogre instances due to multiple copies of the Ogre static libraries
  • routes logging of the application through the new ros logging infrastructure (fixes [rviz_rendering] Unnecessary information in ogre log messages #62, or at least works around it)
  • migrates move tool and orbit view controller to rviz_default_plugins

What I still need to do:

  • add pluginlib to instructions in readme
  • (re) migrate a few of the display plugins, at least grid
  • do an uncrustify and cpplint pass
  • test on
    • Linux
    • Windows (disabled for now 😢)
  • run tests
  • update instructions or fix CMake to make it build on Linux
  • resolve issues with filesystem experimental on Linux (linking and API issues)
  • figure out what is wrong with the orbit view controller

This still doesn't get a display panel to show at first, that needs the configuration loading to be fixed, but you can add one from the add panel dialog.

I also need to test out the add display by topic more, since I just ported it without trying it out (just made it compile).

@wjwwood wjwwood added the in progress Actively being worked on (Kanban column) label Nov 27, 2017
@greimela-si
Copy link
Contributor

Nice work!

I built the branch on Ubuntu 16.04, here are some findings:

  • The default tinyxml2 APT package libtinyxml2-dev is not sufficient for CMake builds, as it does not provide a TinyXML2Config.cmake. I had to add the tinyxml2 repo to the rviz workspace and build it from source.
  • When building tinyxml2 from source it creates a tinyxml2Config.cmake (lowercase), but the build expects a TinyXML2Config.cmake (uppercase). I had to replace the name in find_package in the pluginlibs CMakeLists.txt with the lowercase variant.
  • When using the default GCC (5.4.0), has_include(<experimental/filesystem>) in the filesystem_helper returns true but the API seems to miss some methods (e.g. fs::path::parent_path()). So the fact that <experimental/filesystem> is available may not be enough to use it.
  • both the displays panel and the view controller panel can be added fine
  • the orbit view controller behaves strangely right now

@wjwwood
Copy link
Member Author

wjwwood commented Nov 28, 2017

I updated the OP with more todo check boxes based on your feedback. I'm going to try and get this done today, but I have some things to do this afternoon which might get in the way of that.

@jeising
Copy link
Contributor

jeising commented Dec 3, 2017

Maybe we could take up these three fixes and tests for next week, @greimela-si and @wjwwood? So they would not be part of this PR:

  • (re) migrate the display plugins
  • figure out what is wrong with the orbit view controller
  • test out the add display by topic more

@wjwwood
Copy link
Member Author

wjwwood commented Dec 3, 2017

@jeising that's the plan, but currently I'm hung up on an issue with the Qt Automoc step, but only on windows. I've spent the better part of a day on this one thing and haven't found a workaround just yet. I wanted it to be at least compiling before pushing this for merge (and then subsequently opening issues for the other problems).

I'll push what I have shortly and describe the problem before picking up on it again.

@greimela-si
Copy link
Contributor

greimela-si commented Dec 4, 2017

Some more findings:

  • The resource_retriever is currently missing in the readme.

On Ubuntu:

  • The library libassimpd.4.so can't be found when starting rviz2_app.
    It works after adding install/opt/rviz_assimp_vendor/lib folder to the LD_LIBRARY_PATH.

On Windows:

@greimela-si
Copy link
Contributor

I fixed the orbit view controller in #129.

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.

[rviz_rendering] Unnecessary information in ogre log messages
3 participants