-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add laser decimation #225
Add laser decimation #225
Conversation
@@ -35,6 +36,17 @@ const Submaps* GlobalTrajectoryBuilder::submaps() const { | |||
void GlobalTrajectoryBuilder::AddRangefinderData( | |||
const common::Time time, const Eigen::Vector3f& origin, | |||
const sensor::PointCloud& ranges) { | |||
// TODO average laser scans instead of discarding them? |
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.
Not sure what you mean by averaging, but would it also help in your case if you combine multiple scans using the predicted poses in the LocalTrajectoryBuilder
introducing a parameter scans_per_accumulation
? This would be similar to the 3D case if you look at the mapping_3d/kalman_local_trajectory_builder.cc
for example.
Otherwise we now add a stop gap solution that we will remove again in the future, instead of moving a smaller step in the direction we would like to go.
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.
I agree, this is indeed a stop gap solution, I was not sure if I should submit a pull request for this at all. I decided to go for it because it really made a big improvement on my bag from #193.
I will take a look at the 3D case and try to do it that way. If it also gives good results, I will update the pull request and replace scan dropping with scan accumulation.
The TODO was just an idea I thought would make sense - by "averaging" I meant averaging the rangefinder data. E.g. the distance in some direction theta is the average of the distances in the same direction in these sequential scans which would otherwise be dropped. Perhaps it's the same thing as the scan accumulation from 3D that you have mentioned - at this point I can't say because I haven't looked into it yet.
Also inform the user about slow performance if they do this.
Controlled by the laser_decimation_factor setting in the trajectory builder lua dictionary.
I will close the pull request for now, until I take a look at the 3D local trajectory builders. I will make separate pull requests for those two misc commits. |
…pher-project#225) FindThreads.cmake (e.g. of trusty) is used which adds the "-lpthead" dependency that is added to cartographer's dependencies. Catkin then fails to resolve this in the cartographer_rosConfig.cmake file it generates to be used by all dependencies of cartographer_ros. This means for now, users of cartographer_ros also have to directly depend on cartographer.
This turned out to be the best mitigation of #193 for me.
I attempted to do the discarding of laser messages as early in the call stack of processing them as I could, while being able to access the trajectory builder options, where I think this option should be located. This turned out to be in the global trajectory builder class for the 2D case. I expect that you might have a better idea where this should go.
Also included are two completely miscellaneous things that I decided to add in the pull request so I don't have to keep them stashed the whole time. :)