Lib extration #33
Merged
Lib extration #33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Extracted common scan registration logic into a ScanRegistration base class. This simplifies concrete implementations for different types of lidar sensors. * Extracted velodyne specific scan registration logic into VelodyneScanRegistration class. * Extracted remeining parts of main function into velodyne_scan_registration_node.cpp file. * Added struct for scan registration parameters. * Added struct for (accumulated) IMU states. * Created CircularBuffer class for holding the IMU state data history. * Moved some math functions from common.h to math_util.h. * Changed rotation functions to override input vector / point. Almost all rotations were performed on temporary data structures and then directly stored in their target data structure. Changes to the logic: * Calculate imuShiftFromStart and imuVelocityFromStart values only at the end of a sweep, as they are not used (more than once) inbetween. * Changed calculation of scan start and end indices, by using the individual ring cloud sizes. Bugfixes: * Fixed a copy & paste error in transformToStartIMU. New features: * All buffers within the scan registration components used during feature extration are now dynamic and automatically adapted during processing. Also, the buffer scopes - and with that their typical sizes - are reduced to their required minimum, to reduce the memory footprint. However, the number of scans per sweep in the VelodyneScanRegistration is still required to be 16, due to the scan ring mapping. * There exist two different modi: active and passtive. In active mode, the component works just like before. Passive mode is intended for offline processing of bag files and does not listen to topics nor publish any messages.
* Let transformToEnd transform a whole cloud at once instead of only one point, since it was only called in loops over whole clouds. * Reduced scopes of variables where possible. Changes to the logic: * Do not publish corner and surface clouds during initialization, as they are anyhow rejected without the corresponding full resolution cloud (which was not sent during initialization). New Features: * All buffers used during optimization are now dynamic and automatically adapted to the currently processed cloud sizes. * Again, there are two modis: active and passive. However, passive mode requires some further work with respect to result management.
No changes to the logic. Active / passive modi added, but passive mode needs further work to be usable.
...as they may be confusing and it's not clear yet how the potential offline version is supposed to work.
* Moved code for pose optimization into own function. * Reduced scopes of variables where possible. No changes to the logic. This commit completes the extraction of the code into an object oriented library.
* Moved all class header files into include directory. * Moved Vector3, Angle and Twist class definitions from math_utils.h into own loam_types.h in include directory. * Added publish cloud message function to common.h to simplify publishing of cloud messages. * Made all point calculations / transformations in math_utils.h generic with respect to the pcl point type.
* Replaced scan start and end indices vectors with one std::pair vector. * Removed _nScans variable from ScanRegistration class and replaced its usage with the size of the scan indices vector. This way feature extration will automatically run for all available scans. * Extracted neighbor marking on feature selection into own function. * Removed some duplicate code for IMU interpolation. * Split interpolation of IMU state and point transformation into two functions and store all relevant information needed for the transformation. This way, multi-laser lidars can use the same IMU interpolation for registering all laser measurements to a given point in time. * Removed point cloud getters. * Automatically generate IMU transformation information cloud when publishing the result. * Removed unnecessary shared pointer declaration of internal cloud buffers. * Some minor simplifications.
* Moved Angle, Vector3 and Twist classes into separate header files and removed loam_types.h again.
|
Very nice changes !!! |
|
Thanks!! All ROS pkgs hould be structured like this: a generic library (ideally not using ROS types at all!) + the node wrapper... Just my view ;-) |
|
Thanks for your feedback! However, I have to say that Martin Velas went way further with his branch and introduced significant simplifications. I feel kind of sad that his effort is not honored much and with all the changes I introduced, the chance of merging his branch is even lower than before. I guess the best way of acknowledging his work is to keep up working on LOAM and integrating his improvements, too. |
|
I agree, but the only one that can really take some actions is Martin Velas himself |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Extracted all components into separate classes and adapted project structure.
This pull request is conflicting with the master branch, due to the previous hotfix that I committed. However, this pull request basically contains the previous hotfix (as it further simplified processing of rotations) and merging by overriding the changes of the hotfix should be fine.