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

chore(build): remove tier4_autoware_utils.hpp evaluator/ simulator/ #4839

Conversation

soblin
Copy link
Contributor

@soblin soblin commented Sep 1, 2023

Description

#4821

Tests performed

Not applicable.

Effects on system behavior

Not applicable.

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.

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.

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

Summary by CodeRabbit

Release Notes:

  • Refactor: Removed the inclusion of tier4_autoware_utils/tier4_autoware_utils.hpp header file and replaced it with specific headers in multiple files related to kinematic, localization, planning, and metrics calculations. This change improves modularity and reduces unnecessary dependencies.
  • Bug Fix: Replaced the inclusion of tier4_autoware_utils/tier4_autoware_utils.hpp with appropriate headers in several files, ensuring correct functionality and compatibility with the tier4_autoware_utils library.
  • Documentation: Updated the include statements in various files to reflect the changes made for better code organization and maintainability.

@github-actions github-actions bot added component:planning Route planning, decision-making, and navigation. (auto-assigned) component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) component:common Common packages from the autoware-common repository. (auto-assigned) component:simulation Virtual environment setups and simulations. (auto-assigned) component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) labels Sep 1, 2023
@soblin soblin changed the title chore(build): remove tier4 autoware utils hpp evaluator/ simulator/ chore(build): remove tier4_autoware_utils.hpp evaluator/ simulator/ Sep 1, 2023
Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
@soblin soblin force-pushed the build-fast2/component/evaluator-simulator/remove-tier4_autoware_utils_hpp branch from 28d1719 to 10d1f8c Compare September 1, 2023 04:27
@github-actions github-actions bot removed component:planning Route planning, decision-making, and navigation. (auto-assigned) component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) component:common Common packages from the autoware-common repository. (auto-assigned) labels Sep 1, 2023
@soblin soblin marked this pull request as ready for review September 1, 2023 06:19
@soblin soblin added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Sep 1, 2023
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Image description CodeRabbit

Walkthrough

The changes involve removing the inclusion of tier4_autoware_utils/tier4_autoware_utils.hpp header file in multiple files and replacing it with different headers from the tier4_autoware_utils library. The new headers include geometry.hpp, pose_deviation.hpp, msg_covariance.hpp, and update_param.hpp. These changes suggest a shift in dependencies and functionality, potentially impacting the code's behavior and external interface.

Changes

Files Summary
evaluator/kinematic_evaluator/src/metrics/kinematic_metrics.cpp Removes inclusion of tier4_autoware_utils/tier4_autoware_utils.hpp header file.
evaluator/localization_evaluator/src/metrics/localization_metrics.cpp Removes inclusion of tier4_autoware_utils/tier4_autoware_utils.hpp header file and replaces it with <cmath> header file.
evaluator/planning_evaluator/src/metrics/deviation_metrics.cpp
evaluator/planning_evaluator/src/metrics/metrics_utils.cpp
evaluator/planning_evaluator/src/metrics/obstacle_metrics.cpp
evaluator/planning_evaluator/src/metrics/stability_metrics.cpp
evaluator/planning_evaluator/src/metrics/trajectory_metrics.cpp
evaluator/planning_evaluator/src/metrics_calculator.cpp
Removes inclusion of tier4_autoware_utils.hpp and adds new includes from the tier4_autoware_utils library.
simulator/dummy_perception_publisher/src/node.cpp Removes inclusion of tier4_autoware_utils/tier4_autoware_utils.hpp and replaces it with tier4_autoware_utils/geometry/geometry.hpp.
simulator/simple_planning_simulator/src/simple_planning_simulator/simple_planning_simulator_core.cpp Includes changes to #include statements, removing tier4_autoware_utils.hpp and adding geometry.hpp, msg_covariance.hpp, and update_param.hpp.

"Code changes like rabbits hop,
Dependencies shift, headers swap.
With tier4_autoware_utils, a new dawn,
Geometry, pose deviation, they spawn.
The code evolves, its path unwinds,
Embracing change, as progress finds."


Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between ade1c41 and 10d1f8c commits.
Files selected (10)
  • evaluator/kinematic_evaluator/src/metrics/kinematic_metrics.cpp (1)
  • evaluator/localization_evaluator/src/metrics/localization_metrics.cpp (1)
  • evaluator/planning_evaluator/src/metrics/deviation_metrics.cpp (1)
  • evaluator/planning_evaluator/src/metrics/metrics_utils.cpp (1)
  • evaluator/planning_evaluator/src/metrics/obstacle_metrics.cpp (1)
  • evaluator/planning_evaluator/src/metrics/stability_metrics.cpp (1)
  • evaluator/planning_evaluator/src/metrics/trajectory_metrics.cpp (1)
  • evaluator/planning_evaluator/src/metrics_calculator.cpp (1)
  • simulator/dummy_perception_publisher/src/node.cpp (1)
  • simulator/simple_planning_simulator/src/simple_planning_simulator/simple_planning_simulator_core.cpp (1)
Files skipped from review due to trivial changes (5)
  • evaluator/kinematic_evaluator/src/metrics/kinematic_metrics.cpp
  • evaluator/localization_evaluator/src/metrics/localization_metrics.cpp
  • evaluator/planning_evaluator/src/metrics/deviation_metrics.cpp
  • evaluator/planning_evaluator/src/metrics/obstacle_metrics.cpp
  • evaluator/planning_evaluator/src/metrics/trajectory_metrics.cpp
Review comments generated (6)
  • Review: 2
  • LGTM: 4

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

@@ -14,13 +14,12 @@

#include "planning_evaluator/metrics_calculator.hpp"

#include "motion_utils/motion_utils.hpp"
#include "motion_utils/trajectory/trajectory.hpp"
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The inclusion of the motion_utils/trajectory/trajectory.hpp header file seems to be missing in the new code. Please ensure that it is included.

+ #include "motion_utils/trajectory/trajectory.hpp"

#include "planning_evaluator/metrics/deviation_metrics.hpp"
#include "planning_evaluator/metrics/obstacle_metrics.hpp"
#include "planning_evaluator/metrics/stability_metrics.hpp"
#include "planning_evaluator/metrics/trajectory_metrics.hpp"
#include "tier4_autoware_utils/tier4_autoware_utils.hpp"

#include "tier4_autoware_utils/geometry/geometry.hpp"
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The inclusion of the tier4_autoware_utils/geometry/geometry.hpp header file seems to be missing in the new code. Please ensure that it is included.

+ #include "tier4_autoware_utils/geometry/geometry.hpp"

@soblin soblin removed the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Sep 1, 2023
Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
Copy link
Contributor

@kminoda kminoda left a comment

Choose a reason for hiding this comment

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

LGTM

@soblin soblin added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Sep 1, 2023
@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (ade1c41) 15.12% compared to head (bb1b41e) 15.12%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4839   +/-   ##
=======================================
  Coverage   15.12%   15.12%           
=======================================
  Files        1573     1573           
  Lines      108415   108415           
  Branches    33306    33306           
=======================================
+ Hits        16395    16396    +1     
  Misses      74148    74148           
+ Partials    17872    17871    -1     
Flag Coverage Δ *Carryforward flag
differential 39.38% <ø> (?)
total 15.12% <ø> (ø) Carriedforward from ade1c41

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

Files Changed Coverage Δ
...ematic_evaluator/src/metrics/kinematic_metrics.cpp 100.00% <ø> (ø)
...ion_evaluator/src/metrics/localization_metrics.cpp 80.00% <ø> (ø)
...anning_evaluator/src/metrics/deviation_metrics.cpp 90.90% <ø> (ø)
...r/planning_evaluator/src/metrics/metrics_utils.cpp 100.00% <ø> (ø)
...lanning_evaluator/src/metrics/obstacle_metrics.cpp 86.36% <ø> (ø)
...anning_evaluator/src/metrics/stability_metrics.cpp 92.85% <ø> (ø)
...nning_evaluator/src/metrics/trajectory_metrics.cpp 76.81% <ø> (ø)
...ator/planning_evaluator/src/metrics_calculator.cpp 87.83% <ø> (ø)
simulator/dummy_perception_publisher/src/node.cpp 0.00% <ø> (ø)
...nning_simulator/simple_planning_simulator_core.cpp 39.37% <ø> (ø)

... and 1 file with indirect coverage changes

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

@soblin
Copy link
Contributor Author

soblin commented Sep 2, 2023

@yukkysaito @tkimura4 @TakaHoribe Can you approve this PR ?

Copy link
Contributor

@yukkysaito yukkysaito left a comment

Choose a reason for hiding this comment

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

LGTM

@soblin soblin merged commit 887722c into autowarefoundation:main Sep 2, 2023
@soblin soblin deleted the build-fast2/component/evaluator-simulator/remove-tier4_autoware_utils_hpp branch September 2, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) component:simulation Virtual environment setups and simulations. (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.

6 participants