-
Notifications
You must be signed in to change notification settings - Fork 11
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
multiple lidar support #3
Conversation
Jakub-Krakowiak
commented
Nov 2, 2022
- implemented gazebo system (server) plugin interface - Configure, PreUpdate and PostUpdate functions
- connected RGL using FetchContent
- added mesh detection from gazebo internals (both primitive and external meshes)
- added the meshes to rgl scene
- implemented rgl lidar entity in gazebo
- pose updates from gazebo to rgl for both lidar and other entities
- rgl ray tracing hits visible in GUI if RGLGuiPlugin is loaded
- divided RGLGazeboPlugin into three parts: RGLServerPluginManager, RGLServerPluginInstance and RGLGuiPlugin (since the visualization does not need rgl)
- RGLServerPluginManager manages adding meshes and entities to the rgl scene, while RGLServerPluginInstance takes care of raycasting and publishing PointCloudPacked messages. You can create as many RGLServerPluginInstances as you like - there is multiple lidar support
- added raycasting while simulation is paused
…t modules to FetchContent
…rder to ignore its model in RGL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part 1 of the review: RGLServerPluginManager and its dependencies.
- Most of the code looks good, especially RGLServerPluginManager.cc is very clean and readable.
- Some parts would benefit from a refactor, e.g. the code that deals with mesh & entity handling.
- Please consider using some
using
directives in .cc files to make types shorter. - Please make sure errors are well reported.
- Function names could be more precise to a new reader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some final thoughts; to be addressed on v11-support
branch. This one can be merged to main
.