Skip to content
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

feat(autoware_map_msgs): add ALL_AREA mode #53

Closed

Conversation

Shin-kyoto
Copy link
Contributor

@Shin-kyoto Shin-kyoto commented Mar 27, 2023

Description

Please see autowarefoundation/autoware.universe#3162 for details
This PR changes service for PR above in autoware universe.

Related links

Tests performed

Notes for reviewers

Pre-review checklist for the PR author

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

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.

Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com>
Signed-off-by: Shin-kyoto <58775300+Shin-kyoto@users.noreply.github.com>
@yukkysaito
Copy link
Collaborator

Changing interface has a significant impact on each component. The interface must also be the minimum amount of information necessary for scalable Autoware development.

Therefore, we need to discuss the need for this change a bit more fully.
cc @mitsudome-r @xmfcx @kminoda

@Shin-kyoto
Copy link
Contributor Author

Changing interface has a significant impact on each component. The interface must also be the minimum amount of information necessary for scalable Autoware development.

Therefore, we need to discuss the need for this change a bit more fully. cc @mitsudome-r @xmfcx @kminoda

@yukkysaito @mitsudome-r @xmfcx @kminoda
Thank you so much for your review!
I want to add ALL_AREA mode to AreaInfo.msg.

  • Now
    • The differential_map_loader sends the map by dividing it into shapes of the SPHERE. Center coordinates and radius are required.
  • In this PR
    • I want to add ALL_AREA mode, which tries to send the entire map. This mode does not require center coordinates or radius.
    • ALL_AREA mode is useful to send large size maps.
    • To add the ALL_AREA mode, I want to add a "type" to the AreaInfo.msg, with "SPHERE" as the type for the conventional mode and "ALL_AREA" as the type for the mode I want to add in this PR.
    • This "type" will also be useful in the future for expanding the map division shape into various shapes, not just the SPHERE. (Currently, the area load only supports spherical area)

@Shin-kyoto
Copy link
Contributor Author

We discuss the implementation of autoware_map_msgs in
https://github.com/orgs/autowarefoundation/discussions/2812#discussioncomment-5540316, and we chose another interface. So I close this PR.

#57 is the final implementation. Please check this.

@Shin-kyoto Shin-kyoto closed this Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants