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

refactor(localization): remove unnecessary dependency in localization packages #8202

Conversation

KYabuuchi
Copy link
Contributor

@KYabuuchi KYabuuchi commented Jul 26, 2024

Description

To minimize package dependencies, I removed unnecessary package dependencies from the package.xml.

The selection of packages to remove was based on autowarefoundation/autoware#3468 (comment)

Related links

https://github.com/orgs/autowarefoundation/discussions/5007
In this discussion, it is mentioned that "slimming down the container image sizes is the next urgent task."

How was this PR tested?

I deleted the build/ and install/ directories and confirmed that the build was successful.

Notes for reviewers

None.

Interface changes

Does not change.

Effects on system behavior

Does not change.

Signed-off-by: Kento Yabuuchi <kento.yabuuchi.2@tier4.jp>
Signed-off-by: Kento Yabuuchi <kento.yabuuchi.2@tier4.jp>
@github-actions github-actions bot added the component:localization Vehicle's position determination in its environment. (auto-assigned) label Jul 26, 2024
Copy link

github-actions bot commented Jul 26, 2024

Thank you for contributing to the Autoware project!

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

Please ensure:

@KYabuuchi KYabuuchi added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jul 26, 2024
Copy link

codecov bot commented Jul 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.22%. Comparing base (5093495) to head (5cf71ef).
Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8202      +/-   ##
==========================================
- Coverage   29.24%   29.22%   -0.02%     
==========================================
  Files        1600     1602       +2     
  Lines      117738   117441     -297     
  Branches    50720    50629      -91     
==========================================
- Hits        34427    34317     -110     
+ Misses      74120    73923     -197     
- Partials     9191     9201      +10     
Flag Coverage Δ *Carryforward flag
differential 41.72% <ø> (?)
total 29.21% <ø> (-0.03%) ⬇️ Carriedforward from 7dc3939

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KYabuuchi
Copy link
Contributor Author

@a-maumau maybe our works are conflicted. sorry 😭

@KYabuuchi
Copy link
Contributor Author

To the maintainers,
since @a-maumau has conducted more detailed testing, I have asked him to review this and share his detailed analysis.
Please wait your reviews until he has completed his review.

@a-maumau
Copy link
Contributor

a-maumau commented Jul 26, 2024

@KYabuuchi
I have updated the script for checking some autoware_* dependency and the result is following:

updated result
--- Checking src/universe/autoware.universe/localization/autoware_landmark_based_localizer/autoware_landmark_manager/package.xml ---

--- Checking src/universe/autoware.universe/localization/autoware_landmark_based_localizer/autoware_ar_tag_based_localizer/package.xml ---
ament_index_cpp should be rather be marked as either <build_depend> or <test_depend>, not <depend>
autoware_lanelet2_extension seems not to be used

--- Checking src/universe/autoware.universe/localization/pose_initializer/package.xml ---

--- Checking src/universe/autoware.universe/localization/ekf_localizer/package.xml ---

--- Checking src/universe/autoware.universe/localization/pose_estimator_arbiter/package.xml ---
pcl_ros seems not to be used
pluginlib seems not to be used
ublox_msgs seems not to be used
yabloc_particle_filter seems not to be used

--- Checking src/universe/autoware.universe/localization/yabloc/yabloc_pose_initializer/package.xml ---

--- Checking src/universe/autoware.universe/localization/yabloc/yabloc_image_processing/package.xml ---

--- Checking src/universe/autoware.universe/localization/yabloc/yabloc_monitor/package.xml ---

--- Checking src/universe/autoware.universe/localization/yabloc/yabloc_common/package.xml ---

--- Checking src/universe/autoware.universe/localization/yabloc/yabloc_particle_filter/package.xml ---

--- Checking src/universe/autoware.universe/localization/autoware_pose_covariance_modifier/package.xml ---

--- Checking src/universe/autoware.universe/localization/twist2accel/package.xml ---

--- Checking src/universe/autoware.universe/localization/autoware_stop_filter/package.xml ---

--- Checking src/universe/autoware.universe/localization/gyro_odometer/package.xml ---

--- Checking src/universe/autoware.universe/localization/localization_util/package.xml ---
autoware_universe_utils seems not to be used
fmt seems not to be used
nav_msgs seems not to be used
sensor_msgs seems not to be used
std_srvs seems not to be used
tf2_ros seems not to be used
tf2_sensor_msgs seems not to be used
tier4_debug_msgs seems not to be used
tier4_localization_msgs seems not to be used

--- Checking src/universe/autoware.universe/localization/ndt_scan_matcher/package.xml ---
std_msgs seems not to be used

--- Checking src/universe/autoware.universe/localization/pose_instability_detector/package.xml ---
ament_index_cpp should be rather be marked as either <build_depend> or <test_depend>, not <depend>
tier4_debug_msgs seems not to be used

--- Checking src/universe/autoware.universe/localization/localization_error_monitor/package.xml ---
diagnostic_updater seems not to be used

--- Checking src/universe/autoware.universe/localization/geo_pose_projector/package.xml ---

--- Checking src/universe/autoware.universe/localization/pose2twist/package.xml ---

At least for compiling, we can follow the above result for removing the unused dependency except for autoware_universe_utils in localization_util.

  • Remove the autoware_universe_utils and add diagnostic_msgs to remove the autoware_universe_utils dependency in localization_util.
  • add the autoware_universe_utils in autoware_landmark_manager.

@KYabuuchi KYabuuchi marked this pull request as draft July 26, 2024 07:13
KYabuuchi and others added 4 commits July 26, 2024 16:51
Signed-off-by: Kento Yabuuchi <kento.yabuuchi.2@tier4.jp>
Signed-off-by: Kento Yabuuchi <kento.yabuuchi.2@tier4.jp>
…Yabuuchi/autoware.universe into refactor/remove_unnecessary_dependency
@KYabuuchi KYabuuchi marked this pull request as ready for review July 26, 2024 08:11
@KYabuuchi
Copy link
Contributor Author

I have updated the dependencies based on a-maumau's review. 🙏

And, I have reconfirmed that all packages build successfully again.

Now, this PR is ready for review 🙇

Copy link
Contributor

@YamatoAndo YamatoAndo 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 all packages build successfully and the Lsim works with sample rosbag.

@KYabuuchi KYabuuchi merged commit d5e8e85 into autowarefoundation:main Jul 31, 2024
30 checks passed
@KYabuuchi KYabuuchi deleted the refactor/remove_unnecessary_dependency branch July 31, 2024 00:31
kyoichi-sugahara pushed a commit to kyoichi-sugahara/autoware.universe that referenced this pull request Aug 5, 2024
… packages (autowarefoundation#8202)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Yamato Ando <yamato.ando@gmail.com>
esteve pushed a commit to esteve/autoware.universe that referenced this pull request Aug 13, 2024
… packages (autowarefoundation#8202)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Yamato Ando <yamato.ando@gmail.com>
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) tag: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.

3 participants