feat(map_based_prediction): add time_keeper #8176
Conversation
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
ebdf5cc to
824ea47
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8176 +/- ##
==========================================
- Coverage 29.24% 29.22% -0.02%
==========================================
Files 1596 1598 +2
Lines 117660 117725 +65
Branches 50653 50711 +58
==========================================
Hits 34409 34409
- Misses 74053 74117 +64
- Partials 9198 9199 +1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Sorry, testing for:
|
|
/review |
PR Reviewer Guide 🔍
|
|
/improve |
| @@ -922,6 +928,8 @@ | |||
|
|
|||
| void MapBasedPredictionNode::mapCallback(const LaneletMapBin::ConstSharedPtr msg) | |||
| { | |||
| autoware::universe_utils::ScopedTimeTrack st(__func__, *time_keeper_); | |||
There was a problem hiding this comment.
Suggestion: Consider checking if time_keeper_ is not null before dereferencing it in the ScopedTimeTrack constructor to avoid potential null pointer dereference. [possible issue, importance: 9]
| autoware::universe_utils::ScopedTimeTrack st(__func__, *time_keeper_); | |
| if (time_keeper_) { | |
| autoware::universe_utils::ScopedTimeTrack st(__func__, *time_keeper_); | |
| } |
| @@ -922,6 +928,8 @@ | |||
|
|
|||
| void MapBasedPredictionNode::mapCallback(const LaneletMapBin::ConstSharedPtr msg) | |||
| { | |||
| autoware::universe_utils::ScopedTimeTrack st(__func__, *time_keeper_); | |||
There was a problem hiding this comment.
Suggestion: To improve readability and maintainability, consider creating a helper function for the repeated pattern of creating a ScopedTimeTrack object. [maintainability, importance: 7]
| autoware::universe_utils::ScopedTimeTrack st(__func__, *time_keeper_); | |
| auto createScopedTimeTrack = [this](const char* func_name) { | |
| if (time_keeper_) { | |
| return autoware::universe_utils::ScopedTimeTrack(func_name, *time_keeper_); | |
| } | |
| return std::optional<autoware::universe_utils::ScopedTimeTrack>{}; | |
| }; | |
| auto st = createScopedTimeTrack(__func__); |
| @@ -222,6 +223,9 @@ class MapBasedPredictionNode : public rclcpp::Node | |||
| bool remember_lost_crosswalk_users_; | |||
|
|
|||
| std::unique_ptr<autoware::universe_utils::PublishedTimePublisher> published_time_publisher_; | |||
| rclcpp::Publisher<autoware::universe_utils::ProcessingTimeDetail>::SharedPtr | |||
| detailed_processing_time_publisher_; | |||
| mutable std::shared_ptr<autoware::universe_utils::TimeKeeper> time_keeper_{nullptr}; | |||
There was a problem hiding this comment.
Suggestion: Consider initializing time_keeper_ directly in the member initializer list to ensure it is properly initialized when the object is created. [best practice, importance: 6]
| mutable std::shared_ptr<autoware::universe_utils::TimeKeeper> time_keeper_{nullptr}; | |
| mutable std::shared_ptr<autoware::universe_utils::TimeKeeper> time_keeper_; |
|
@soblin -san, you can use or ignore the suggestions, I hope they were useful. My test is finished. You can check https://autowarefoundation.github.io/autoware-documentation/main/contributing/pull-request-guidelines/ai-pr-review/#how-to-use if you find it useful. |
Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
…#1426) Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
Signed-off-by: Mamoru Sobue <mamoru.sobue@tier4.jp>
Description
Related links
Parent Issue:
How was this PR tested?
see the pic
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.