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.

Sign up for GitHub

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

Jump to bottom

feat(map_based_prediction): add diagnostic handler to warn the processing time excess #10219

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(map_based_prediction): add diagnostic handler to warn the processing time excess #10219

wants to merge 3 commits into from
+41 −7

Conversation

ktro2828
Copy link
Contributor

@ktro2828 ktro2828 commented Mar 3, 2025 •
edited
Loading

Description

This PR introduces diagnostics handler to warn when the processing time exceed a threshold value.

Related links

How was this PR tested?

Screencast.from.03-18-2025.04.30.02.PM.webm

TIER IV INTERNAL LINK

Notes for reviewers

None.

Interface changes

ROS Parameter Changes

Additions and removals

Change type Parameter Name Type Default Value Description
Added processing_time_tolerance_ms double 500.0 If the processing exceeds this value, diagnostics publishes warning.

Effects on system behavior

None.

@github-actions github-actions bot added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Mar 3, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 3, 2025 •
edited
Loading

Thank you for contributing to the Autoware project!

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

Please ensure:

@ktro2828 ktro2828 force-pushed the feat/map_based_prediction/diagnostic branch from 468156f to 283d13d Compare March 3, 2025 16:10
@ktro2828 ktro2828 added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 3, 2025
@codecov Codecov
Copy link

codecov bot commented Mar 3, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes missing coverage. Please review.

Project coverage is 26.02%. Comparing base (837db39) to head (508a449).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...based_prediction/src/map_based_prediction_node.cpp 0.00% 21 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #10219      +/-   ##
==========================================
- Coverage   26.04%   26.02%   -0.03%     
==========================================
  Files        1375     1375              
  Lines      106433   106522      +89     
  Branches    40872    40953      +81     
==========================================
- Hits        27725    27723       -2     
- Misses      75710    76094     +384     
+ Partials     2998     2705     -293
Flag Coverage Δ *Carryforward flag
differential 3.45% <0.00%> (?)
differential-cuda 3.45% <0.00%> (?)
total 26.05% <ø> (+<0.01%) ⬆️ Carriedforward from ace935d

*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.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ktro2828 ktro2828 force-pushed the feat/map_based_prediction/diagnostic branch from 283d13d to 5b0c060 Compare March 11, 2025 05:18
@ktro2828 ktro2828 changed the title feat: add diagnostic handler to warn the processing time excess feat(map_based_prediction): add diagnostic handler to warn the processing time excess Mar 11, 2025
@ktro2828 ktro2828 marked this pull request as ready for review March 11, 2025 06:38
technolojin
technolojin requested changes Mar 11, 2025
View reviewed changes
Copy link
Contributor

@technolojin technolojin left a comment

Choose a reason for hiding this comment

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

I would like to request to reflect changes, then show test result by showing

  • test procedure
  • screenshot of ros2 run rqt_runtime_monitor rqt_runtime_monitor
  • how faulty situation is produced and the diagnostics showed the warn/error status (for example, set the tolerance very short)

@technolojin
Copy link
Contributor

@ktro2828
This PR also need to update autoware_launch configuration.
Can you create PR for the launcher repo and link it to this PR please?

technolojin
technolojin reviewed Mar 18, 2025
View reviewed changes
Copy link
Contributor

@technolojin technolojin left a comment

Choose a reason for hiding this comment

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

similar to https://github.com/autowarefoundation/autoware.universe/pull/10257/files#, it would be better to set tolerance duration of is_processing_time_ok=false

@ktro2828 ktro2828 mentioned this pull request Mar 18, 2025
Open
@ktro2828 ktro2828 marked this pull request as draft March 18, 2025 05:44
@ktro2828
feat: add diagnostic handler to warn when the processing time exceed …
40f652c 
…a threshold

Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
@ktro2828 ktro2828 force-pushed the feat/map_based_prediction/diagnostic branch from b9c98ca to 080a5a1 Compare March 18, 2025 05:45
@ktro2828
feat: always measure processing time with stopwatch
ace935d 
Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
@ktro2828 ktro2828 force-pushed the feat/map_based_prediction/diagnostic branch from 080a5a1 to ace935d Compare March 18, 2025 05:50
@ktro2828
feat: update to publish processing time to diagnostics
508a449 
Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
@ktro2828 ktro2828 marked this pull request as ready for review March 18, 2025 07:37
@ktro2828
Copy link
Contributor Author

@technolojin I added some updates and confirmed it works as shown the video in PR description.

technolojin
technolojin requested changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@technolojin technolojin left a comment

Choose a reason for hiding this comment

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

similar to https://github.com/autowarefoundation/autoware.universe/pull/10257/files#, it would be better to set tolerance duration of is_processing_time_ok=false

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@technolojin technolojin technolojin requested changes

@kyoichi-sugahara kyoichi-sugahara Awaiting requested review from kyoichi-sugahara kyoichi-sugahara is a code owner

@soblin soblin Awaiting requested review from soblin soblin is a code owner

@takayuki5168 takayuki5168 Awaiting requested review from takayuki5168 takayuki5168 is a code owner

@tkimura4 tkimura4 Awaiting requested review from tkimura4 tkimura4 is a code owner

@YoshiRi YoshiRi Awaiting requested review from YoshiRi YoshiRi is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@technolojin technolojin

Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned) run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
Software Working Group
Status: To Triage
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ktro2828 @technolojin