feat: add multi lanelet parser

StepTurtle commented 4 months ago

Description

Move https://github.com/autowarefoundation/autoware_common/pull/234 to new autoware_lanelet2_extension repository.

This PR add a new library class to facilitate the loading of multiple OSM files in lanelet2_extension. These changes were made to support dynamic lanelet loading.

Tests performed

In this video, the map in background loaded with current approach and the white map load new class and cannot see any difference. Also the maps which loaded with new class tested with mission and behavior planner and cannot see any problem.

Video Link

Notes for reviewers

Interface changes

Effects on system behavior

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

[X] I've confirmed the contribution guidelines.
[X] The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

[ ] The PR follows the pull request guidelines.
[ ] The PR has been properly tested.
[ ] The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

[ ] There are no open discussions or they are tracked via tickets.
[ ] The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

mitsudome-r commented 4 months ago

I was wondering if we should create another parser for loading multiple files. Are there any reasons for not merging the Lanelets after loading each files one by one?

StepTurtle commented 4 months ago

I was wondering if we should create another parser for loading multiple files. Are there any reasons for not merging the Lanelets after loading each files one by one?

After call OsmParser::parse and create map object, I did not find a good way to merge map objects. Is there any way to merge map objects?

YamatoAndo commented 2 months ago

@reviewers @kosuke55 @soblin @Motsu-san @mitsudome-r @takayuki5168 @youtalk The review of the related PRs is almost finished. I would appreciate it if you could proceed with the review of this PR.

Motsu-san commented 2 months ago

@soblin Could you review and test this PR?

Motsu-san commented 2 months ago

@soblin Could you review and test this PR?

@soblin I misunderstood. In the parent PR, Maxim-san already has tested the related PR including this PR. So please ignore my request to test.

Motsu-san commented 2 months ago

I was wondering if we should create another parser for loading multiple files. Are there any reasons for not merging the Lanelets after loading each files one by one?

After call OsmParser::parse and create map object, I did not find a good way to merge map objects. Is there any way to merge map objects?

@mitsudome-r Could you respond to this message?

YamatoAndo commented 1 month ago

@kosuke55 @soblin @Motsu-san @mitsudome-r @takayuki5168 @youtalk Review this PR!!

soblin commented 1 month ago

@StepTurtle

After call OsmParser::parse and create map object, I did not find a good way to merge map objects. Is there any way to merge map objects?

I think my comments answers your question. I believe it is possible to merge/delete map objects in sliding window manner in the context of differential map loading.

Here is my suggestion:

the MuptileOsmParser just reads the file and store the list of element_ids for each layer for each file -- you know there are 6 primitive layers in LaneletMapPtr (see https://github.com/fzi-forschungszentrum-informatik/Lanelet2/blob/507033b82d9915f086f2539d56c3b62e71802438/lanelet2_core/include/lanelet2_core/LaneletMap.h#L353-L358) -- , just managing the file name and element ids
the DifferetialMapLoader decides which file to load next and which file to drop next, and MultipleOsmParser just tells which element_ids to load and drop for the loading.

mitsudome-r commented 1 month ago

@StepTurtle

After call OsmParser::parse and create map object, I did not find a good way to merge map objects. Is there any way to merge map objects?

You can create a function to merge each layer of Lanelet map as mentioned by @soblin's comment. Something like the following:

// merge map2 to map1
void mergeMap( lanelet::LaneletMapPtr map1, lanelet::LaneletMapPtr map2)
{
  for(auto llt : map2->laneletLayer){
    map1.add(llt);
  }
  for(auto area : map2->areaLayer){
    map1.add(area);
  }
  for(auto reg_elem : map2->regulatoryElementLayer){
    map1.add(reg_elem);
  }
  for(auto poly : map2->polygonLayer){
    map1.add(poly);
  }
  for(auto ls : map2->lineStringLayer){
    map1.add(ls);
  }
  for(auto pt : map2->pointLayer){
    map1.add(pt);
  }
}

(Note that above code is just an image and I haven't tested the code.

YamatoAndo commented 1 month ago

@soblin @mitsudome-r cc: @StepTurtle Thank you for stating review. Please create a new release tag after merging.

StepTurtle commented 1 month ago

Hey @YamatoAndo , sorry about this, but I tried a few things and couldn't quite figure out the points from the last comments. I also haven’t had enough time to work on the lanelet reviews. Could I ask for your help with the this PR? Apologies again for the trouble. 🙏🏽

YamatoAndo commented 1 month ago

@StepTurtle Hi,

I tried a few things and couldn't quite figure out the points from the last comments

After merging this PR, please create a PR similar to https://github.com/autowarefoundation/autoware_lanelet2_extension/pull/30. Once that PR is merged, we will create a new release tag. Since I haven't done this task before, I would like you to collaborate with @soblin and @mitsudome-r to move forward with the process.

YamatoAndo commented 1 month ago

@kosuke55 @soblin @Motsu-san @mitsudome-r @takayuki5168 @youtalk

I also haven’t had enough time to work on the lanelet reviews. Could I ask for your help with the this PR?

Could you support this PR instead?

soblin commented 1 month ago

@StepTurtle All right, then I'd like to propose moving your implementation in autoware.universe's common package for now. The reason for my suggestion is that this "feature" is experimental and has room for future additional requirements and better implementation. Also it should not be placed under map module because it renders planning dependent on map (relates to this initiative https://github.com/orgs/autowarefoundation/discussions/4661#discussioncomment-9995806 cc : @youtalk ). I am Ok with current implementation !

YamatoAndo commented 5 hours ago

@kosuke55 @soblin @Motsu-san @mitsudome-r @takayuki5168 @youtalk Does it seem like you can support this?

autowarefoundation / autoware_lanelet2_extension