ethz-asl / segmap

A map representation based on 3D segments
BSD 3-Clause "New" or "Revised" License
1.06k stars 393 forks source link

Encapsulate the functionalities of laser_mapper in a separate library #4

Closed rdube closed 7 years ago

rdube commented 7 years ago

See #2.

gawela commented 7 years ago

Do we have a timeplan for this? As to make sure TRADR content stays in sync, or is this going to live in private laser_slam for the time being?

rdube commented 7 years ago

I started to look a bit into this. First impression is that it might be hard to separate a significant part or I just do not yet see how. If segmatch_worker_params_.localize and segmatch_worker_params_.close_loops are both false, segmatch will be completely out of the loop. However the laser_mapper will still depend on segmatch and on PCL an so on. I will continue looking.

rdube commented 7 years ago

I'll start by moving the parameters loading as a first step.

HannesSommer commented 7 years ago

This search https://github.com/ethz-asl/segmatch/search?utf8=%E2%9C%93&q=segmatch_worker_ seems to indicate that the interaction between laser_mapper and segmatch is minimal. Is that wrong? If not I'd recommend creating an interface for this interaction in laser_mapper and implement it with the segment_worker. The conceptual and naming basis for the new interface should be the role segmatch is playing for laser_mapper. What is it for laser_mapper? A loop-closer? A localizer? Or both?

HannesSommer commented 7 years ago

Why is depending on pcl a problem? They can both depend on pcl, no?

rdube commented 7 years ago

Alright I created a laser_slam_ros and moved only parameters loading there for now. See #2 and #9.

For PCL, we removed everything related to PCL in the laser_mapper we were using at TRADR evaluation as we were having conflicts with other packages. That should be fixed one day but in the short term it would be nice to keep a laser_mapper which does not depend on PCL but uses octomap from volumetric_mapping for filtering.

@HannesSommer the current laser_mapper does lidar odometry with ICP and can localize or find loops using segmatch and feed the loop closure constraints back to laser_slam for trajectory estimation.

We could maybe sit together next week and brainstorm about a clean way of doing this?

HannesSommer commented 7 years ago

Yes, brainstorming together sounds good. Based on what you wrote, I'm quite optimistic :).

rdube commented 7 years ago

Hi @HannesSommer @gawela! I moved the laser_slam functionalities to the laser_slam_ros library and trimmed down our laser_mapper. See #10 and https://github.com/ethz-asl/laser_slam/pull/3. The laser_mapper.cpp file now has only 156 lines! :-)

I'll do a bit more of testing / cleaning and we should be good to go!

gawela commented 7 years ago

This is wonderful! :)

HannesSommer commented 7 years ago

Awesome!

rdube commented 7 years ago

Done with #10!