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(map_projection_loader): add scale_factor and remove altitude #340

Merged

Conversation

YamatoAndo
Copy link
Contributor

@YamatoAndo YamatoAndo commented Mar 28, 2025

Description

add scale_factor and remove altitude.

Related links

https://github.com/orgs/autowarefoundation/discussions/5765

Parent Issue:

  • Link

How was this PR tested?

Check scale factor

  1. Build with feat(autoware_msg_msgs): add scale factor remove altitude autoware_msgs#121 and feat(projection): add_scale_factor autoware_lanelet2_extension#54.
  2. Run Autoware
  • Use sample rosbag map(MGRS map)
    Confirmed that it works correctly as usual.
  • Use sample rosbag map(TransverseMercator map)
    I have prepared a sample map converted to the Transverse Mercator coordinate system with different scale factor.
    I have prepared the PCD map and lanelet2 map in each folder so that their scale factors match. If you open map_projector_info.yaml, change the scale_factor, and then run Autoware, only the lanelet2 map will be converted according to the modified scale factor.
    However, position estimation will fail with any value other than the original scale factor (0.9996).
    Please verify that the map changes according to the scale factor.

examples


pcd map: original scale factor (0.9996)
lanelet2 map: original scale factor (0.9996)
Screenshot from 2025-03-28 18-34-19


pcd map: original scale factor (0.9996)
lanelet2 map: scale factor 1.5
Screenshot from 2025-03-28 18-38-13


Check altitude

Since this change has no impact, PR test is not required.

Notes for reviewers

Please merge this PR after autowarefoundation/autoware_msgs#121 and autowarefoundation/autoware_lanelet2_extension#54.

Interface changes

None.

Effects on system behavior

None.

Signed-off-by: Yamato Ando <yamato.ando@tier4.jp>
Signed-off-by: Yamato Ando <yamato.ando@tier4.jp>
Copy link

github-actions bot commented Mar 28, 2025

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

YamatoAndo and others added 5 commits April 2, 2025 17:47
Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yamato Ando <yamato.ando@tier4.jp>
Signed-off-by: Yamato Ando <yamato.ando@tier4.jp>
@YamatoAndo YamatoAndo added run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) component:localization Vehicle's position determination in its environment. (auto-assigned) labels Apr 2, 2025
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@RyuYamamoto RyuYamamoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have confirmed that the logging_simulator work well.

LGTM

@youtalk youtalk enabled auto-merge (squash) April 6, 2025 01:56
@youtalk
Copy link
Member

youtalk commented Apr 6, 2025

The dependencies are ready.

@YamatoAndo
Copy link
Contributor Author

There was an error in the test of autoware_geograhpy_utils.
Screenshot from 2025-04-07 18-28-07

Fixed in b20c158

@youtalk
Copy link
Member

youtalk commented Apr 9, 2025

I've manually removed the GitHub Actions' cache and hope it would be resolved.

@youtalk youtalk disabled auto-merge April 9, 2025 07:55
youtalk added 2 commits April 9, 2025 17:17
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk youtalk enabled auto-merge (squash) April 9, 2025 08:43
@youtalk
Copy link
Member

youtalk commented Apr 9, 2025

I investigated various aspects but couldn’t find the issue. I added a one-line comment to ignore the clang-tidy error and a TODO comment. 1fd7a35
Let’s resolve it in a separate PR.

youtalk and others added 4 commits April 9, 2025 17:51
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
This reverts commit e9af822.
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk
Copy link
Member

youtalk commented Apr 9, 2025

youtalk added 2 commits April 9, 2025 22:28
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk
Copy link
Member

youtalk commented Apr 9, 2025

I gave up for today. I don’t understand why NOLINT is being ignored. 1fd7a35 a0592e9

clang-apply-replacements version 14.0.0
clang-tidy-14 --use-color -export-fixes /tmp/tmps3jvw3af/tmpo3ytlsiw.yaml -p=build/ /__w/autoware_core/autoware_core/common/autoware_geography_utils/src/lanelet2_projector.cpp
�[1m/__w/autoware_core/autoware_core/common/autoware_geography_utils/src/lanelet2_projector.cpp:53:60: �[0m�[0;1;31merror: �[0m�[1mno matching constructor for initialization of 'const lanelet::projection::TransverseMercatorProjector' [clang-diagnostic-error]�[0m
    const lanelet::projection::TransverseMercatorProjector projector{
�[0;1;32m                                                           ^        ~

Kindly ping @veqcc @mitsudome-r

@xmfcx
Copy link
Contributor

xmfcx commented Apr 9, 2025

I've found the issue.

https://github.com/autowarefoundation/autoware_core/actions/runs/14358958246/job/40255411996?pr=340#step:9:551

image

Here the clang-tidy job is installing dependency ros-humble-autoware-lanelet2-extension from the apt repository. And it uses that one instead of the one in the local folder.

Why is it doing that?

https://github.com/autowarefoundation/autoware-github-actions/blob/d3b80b41bcc9d68bf93d117407995bcc1b47c5c9/clang-tidy/action.yaml#L67-L84

Here it tries to pull the build-depend.repos but we removed them, so it cannot find them.

And underlay-workspace is not sourced.

How to fix?

Add underlay-workspace arg to the clang-tidy/action.yaml and source it before performing the rosdep install. And before running the critical clang-tidy command.

@youtalk
Copy link
Member

youtalk commented Apr 9, 2025

@xmfcx Great job!!!!

youtalk added 3 commits April 10, 2025 07:10
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Signed-off-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
@youtalk
Copy link
Member

youtalk commented Apr 9, 2025

After merging autowarefoundation/autoware-github-actions#341 and reverting d520a70, we will finally merge this.

@veqcc
Copy link

veqcc commented Apr 10, 2025

@youtalk
As for clang-tidy, clang-diagnostic-error is NOT a warning but a parse error, which means we cannot suppress it with NOLINT notations.

Possible general (not specific to this case) solutions are

  • fix the codebase (recommended)
  • ignore the entire file or package

@youtalk youtalk enabled auto-merge (squash) April 10, 2025 01:52
@youtalk youtalk merged commit c6ec615 into autowarefoundation:main Apr 10, 2025
25 of 29 checks passed
@youtalk
Copy link
Member

youtalk commented Apr 10, 2025

Merged finally. Thank you for your all support!

@YamatoAndo
Copy link
Contributor Author

Thank you all!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:localization Vehicle's position determination in its environment. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants