-
Notifications
You must be signed in to change notification settings - Fork 34
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
fix(concatedate_data): add error handling when sensor points is empty (backport #3814) #526
Conversation
…autowarefoundation#3814) fix(concatedate_date): add error handling when sensor points are empty
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## beta/v0.3.16 #526 +/- ##
==============================================
Coverage ? 0.00%
==============================================
Files ? 36
Lines ? 2825
Branches ? 0
==============================================
Hits ? 0
Misses ? 2825
Partials ? 0
☔ View full report in Codecov by Sentry. |
sensing/pointcloud_preprocessor/src/concatenate_data/concatenate_data_nodelet.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- sensor points is emptyな状況は正常系で発生する理解でしょうか?(この事象は車両を停車させる必要はないか?)
もしシステム異常として発生し、かつそれが危険事象をもたらすものとすると、このノードないし後段のノードでシステムエラーとして検知する必要があると思われます。
そうでないのであれば特に問題ないかと思います。 - fix(concatedate_date): add error handling when sensor points is empty autowarefoundation/autoware.universe#3814 では単体テストは実施されているように読み取れますが、システムレイヤでのテストが実施されているかどうかが不明です。こちらはテストされていますでしょうか?(あるいはテストの計画がありますでしょうか?)
後段のノードでも同様のエラーが発生してしまう懸念があると思われますので、その可能性があるのか否かを明確にしたいです。
正常系では発生しない理解です。発生する場合はセンサ故障が疑われる事象かと思います。
システムレイヤとしてのテストがどこまでを指すかが不明確なので回答が難しいのですが、sensing全体を通した動作確認を指すのであれば、そちらは一度確認しています。 |
ありがとうございます! |
これは今回のPRについて、でしょうか? |
@yn-mrse 記載をしてみたので確認お願いします |
少なからず変更前については、(意図していないにせよ)センサ故障により発生するリスクに対してMRMで対処できていたと思われます。 |
こちらの記載については問題ありません! |
懸念が残るため導入を見送ります |
@yn-mrse
こういった場合に正常なのにノードが落ちてしまうのは問題なので、導入する方向で進めたいです |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
そもそも故障を一意に特定できるエラー事象ではなく、これによる可用性損失が生じているため、本機能を入れることは妥当と判断します。
本機能を入れることによる安全性への影響は気になりますが、そもそもFMEAを実施のうえ各種監視モジュールを動作させている背景があるため、監視機能としては既存の監視モジュールで賄われているものと考えます。
Description
autowarefoundation#3814 のbackport
Tests performed
同じ内容の修正になっていること
このPRにより点群のwidthが0の場合にsensingにおいてノードが落ちないことを確認
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.
After all checkboxes are checked, anyone who has write access can merge the PR.