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

fix(virtual traffic light): suppress lauch #1290

Merged
merged 5 commits into from
Jun 7, 2024

Conversation

yuki-takagi-66
Copy link

@yuki-takagi-66 yuki-takagi-66 commented May 13, 2024

Description

https://tier4.atlassian.net/browse/AEAP-1145 の修正PRです。

発生している現象

走行中に、End Line通過済みのVTLが反応して、Stop Lineが表示される事がある

現象による影響

  • End Line通過済みVTLに対して、VTLの停止線が表示される問題があります。
  • この問題により、意図しない停車や交通設備連携モジュールが誤動作する可能性があります。

本件の原因

behavior_planning/pathの後端がVTLのend_lineを含むlaneletを通過後、
経路計画の再計画により、behavior_planning/pathの後端が通過済みVTLのend_lineを含むlaneletに再度侵入した場合、通過済みのVTLが再度起動するため。

対策

通過し終わったVTLに対して、同じVTLが再度起動しないようにする修正

対策として、起動条件を狭めるべく、pathがstop_lineと交点を持つことを起動条件に加えました。
そのために、stop_lineが空でないことと、pathの型変換もあわせて行っております。
この修正により問題が再現しなくなることを確認しました。

VTLのstop lineが出力されるロジック

virtual traffice lightモジュールが起動した時点で、stop lineを通過していないという初期値が設定される事で、
stop lineが表示される仕組みとなっている。

  • virtual traffice lightモジュールが起動する条件
    /planning/scenario_planning/lane_driving/behavior_planning/path_with_lane_id(後退含む)がend_lineを含むlaneletの交点を持つ場合

VTLのstop lineが出力されるロジック上の問題

Pathの後端がend_lineを含むlaneletから一旦出た後に、Pathの後端が再び入った場合に、
virtual traffic lightが再起動してしまう問題がある。

VTLのstop lineが出力されるロジックに対するアプローチ

Pathの後端がend_lineを含むlaneletから一旦出た後に、Pathの後端が再び入った場合に、
virtual traffic lightが不必要な再起動をしないように修正。

  • virtual traffice lightモジュールが起動する条件を変更
    /planning/scenario_planning/lane_driving/behavior_planning/path_with_lane_id`(後退含む)がend_lineを含むlaneletの交点を持つかつstop_lineと交点をもつ場合

解決策の妥当性

  • 特定条件下において、意図しないVTLのstop lineが出力する問題を解決しました。
    • 特定条件 :
      • /planning/scenario_planning/lane_driving/behavior_planning/path_with_lane_idの後端がend_lineを含むlaneletから一旦出た後に再び入った場合に、stop_lineと交点を持たない場合は、VTLのstop lineが出力されない。
    • 意図しないVTLが出力されるケース
      • /planning/scenario_planning/lane_driving/behavior_planning/path_with_lane_id(後退含む)がstop_lineと交点を持つ時には、end line通過後でもVTLのstop lineが出力される。
        • stop lineとend lineを含むlaneletの後端が極端に近い場合

Tests performed

・本PRに対する既存動作へ影響
地図上のvirtual_traffic_lightに停止線が設定されていない場合は、変更後は内部的にモジュールの起動が行われず、停止線の埋め込みも行われません。変更前は、内部的にモジュールの起動はしますが、停止線は埋め込まれませんでした。
そのため、モジュール起動有無の違いはありますが、停止線の有無に変更がないため、外部的な挙動に変化はありません。

・テスト観点と実施条件・内容
手元のpsimで問題の再現方法と同様の環境でテストをし、virtual wallがでないことを確認しました。また、計算時間が既存処理と比べて十分に短いことを確認しました。

・修正に対する不具合確認に対して、リグレッションテストの有無と内容
P/Cでは、本変更により、無印pilot-autoとして他のモジュールに大きな悪影響がないことを確認しました。

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.

Note

  • 今回の課題対策については、今回起きた現象に対する課題を改善するだけで、本質的な改善する訳ではない事を
    eveに共有する必要がある。

Signed-off-by: Yuki Takagi <[email protected]>
@yuki-takagi-66 yuki-takagi-66 changed the title fix(crosswalk): suppress lauch fix(virtual traffic light): suppress lauch May 13, 2024
@sfukuta
Copy link

sfukuta commented May 14, 2024

@yuki-takagi-66
pre-commit.ciがfailする問題を解消しました。update branchを実施する事で、本PRも問題がなくなると思います。

@yuki-takagi-66 yuki-takagi-66 marked this pull request as ready for review May 14, 2024 09:08
@sfukuta
Copy link

sfukuta commented May 14, 2024

@yuki-takagi-66
対応ありがとうございます。
PRの記載が曖昧なため、下記について、JIRAへ記載をお願いします。

  • 問題に対する原因と対策
  • 本PRに対する既存動作へ影響
  • テスト内容の妥当性が不明のため、テスト観点と実施条件・内容
  • 修正に対する不具合確認に対して、リグレッションテストの有無と内容

@sfukuta
Copy link

sfukuta commented May 31, 2024

@yuki-takagi-66 approveしましたが、まだマージは待って下さい。

Copy link

@asa-naki asa-naki left a comment

Choose a reason for hiding this comment

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

既存のVTLのmodule読み込みに対しての影響がないように思われるので、approveします。

@yuki-takagi-66
Copy link
Author

eve系のみなさんのタイミングでマージしていただけるとありがたいです。
私としては即マージでもOKです。

@yn-mrse
Copy link

yn-mrse commented Jun 5, 2024

@yuki-takagi-66
eve projで品質を管理するために、PR上で妥当性確認が可能な情報を網羅的に埋め込んでいただく必要があります。
JIRAに記載があると思うので、転記いただけますと幸いです。

@yn-mrse
Copy link

yn-mrse commented Jun 5, 2024

File changedとしての対策内容と、Descriptionにおける記載内容の対応関係がわかるように記載ください。

@yuki-takagi-66
Copy link
Author

@yn-mrse
多少ですが追記をさせていただきました。より追加の情報が必要でしたら教えていただけたらと思います。

@sfukuta
Copy link

sfukuta commented Jun 6, 2024

@yuki-takagi-66

Pathの後端がend_lineを含むlaneletから一旦出た後に再び入っており、この一連の流れによりvirtual traffic lightが再起動してしまっているのが原因でした。

上記について、原因の表現を変更します。以下の表現で誤解はないか確認をお願いします。

behavior_planning/pathの後端がVTLのend_lineを含むlaneletを通過後、
経路計画の再計画により、behavior_planning/pathの後端が通過済みVTLのend_lineを含むlaneletに再度侵入した場合、通過済みのVTLが再度起動する事が原因となります。

@sfukuta
Copy link

sfukuta commented Jun 6, 2024

@yuki-takagi-66

pathがstop_lineと交点を持つことを起動条件に加えました。

上記、pathが、どのtopicのpathを指しているか分かるように、記載をして欲しいです。

@sfukuta
Copy link

sfukuta commented Jun 6, 2024

@yuki-takagi-66

・本PRに対する既存動作へ影響
 地図上のvirtual_traffic_lightに停止線が設定されていないとモジュールが起動しなくなります。

→ JIRAより、外部的ない影響がない旨の回答をもらっているため、既存動作への影響は以下で、問題ないでしょうか?

地図上のvirtual_traffic_lightに停止線が設定されていない場合は、変更後は内部的にモジュールの起動が行われず、停止線の埋め込みも行われません。変更前は、内部的にモジュールの起動はしますが、停止線は埋め込まれませんでした。
そのため、モジュール起動有無の違いはありますが、停止線の有無に変更がないため、外部的な挙動に変化はありません。

@sfukuta
Copy link

sfukuta commented Jun 7, 2024

Descriptionについて、マージ後に引き続き見直しが必要。

Copy link

sonarcloud bot commented Jun 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@sfukuta sfukuta merged commit 35e1dde into beta/v0.3.18.1 Jun 7, 2024
10 of 11 checks passed
@sfukuta sfukuta deleted the fix/virtual_traffic_light/suppress_launch branch June 7, 2024 06:23
kimurariku pushed a commit that referenced this pull request Sep 12, 2024
* suppress launch

Signed-off-by: Yuki Takagi <[email protected]>

* add existence check

Signed-off-by: Yuki Takagi <[email protected]>

---------

Signed-off-by: Yuki Takagi <[email protected]>
Co-authored-by: Shigekazu Fukuta <[email protected]>
asa-naki pushed a commit that referenced this pull request Sep 17, 2024
* suppress launch

Signed-off-by: Yuki Takagi <[email protected]>

* add existence check

Signed-off-by: Yuki Takagi <[email protected]>

---------

Signed-off-by: Yuki Takagi <[email protected]>
Co-authored-by: Shigekazu Fukuta <[email protected]>
@asa-naki asa-naki mentioned this pull request Sep 18, 2024
asa-naki added a commit that referenced this pull request Sep 18, 2024
* fix(system_monitor): extend command line to display (backport autowarefoundation#4553) (#768)

fix(system_monitor): extend command line to display (autowarefoundation#4553)

Signed-off-by: ito-san <[email protected]>
Co-authored-by: ito-san <[email protected]>

* feat(behavior_path_planner): resample output path (backport #1604) (#782)

feat(behavior_path_planner): resample output path (#1604)

* feat(behavior_path_planner): resample output path



* update param

Signed-off-by: Takayuki Murooka <[email protected]>
Co-authored-by: Takayuki Murooka <[email protected]>

* ci: add dispatch-push-event workflow  (#803)

* ci: add dispatch-push-event workflow

Signed-off-by: Keisuke Shima <[email protected]>

* fix: change APP KEY

Signed-off-by: Keisuke Shima <[email protected]>

* chore: use strategy

Signed-off-by: Keisuke Shima <[email protected]>

* chore: change trigger

Signed-off-by: Keisuke Shima <[email protected]>

* pre-commit fixes

Signed-off-by: Keisuke Shima <[email protected]>

* Update .github/workflows/dispatch-push-event.yaml

* Update .github/workflows/dispatch-push-event.yaml

* style(pre-commit): autofix

* Update .github/workflows/dispatch-push-event.yaml

---------

Signed-off-by: Keisuke Shima <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(behavior_path): only apply spline interpolation for its output, not for turn_signal processing (#909)

* fix(behavior_path): only apply spline interpolate for output, not for turn_signal processing

* fix implementation

* ci(pre-commit): autofix

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(behavior_path): fix base points vanishing and inconsistent lane_ids on the spline interpolated path (#929)

* add base points to resampled path in behavior_path

* Revert "fix(behavior_path): only apply spline interpolation for its output, not for turn_signal processing (#909)"

This reverts commit c80c986.

* ci(pre-commit): autofix

* fix insert

* fix: not interpolate behavior velocity path

* Revert "Revert "fix(behavior_path): only apply spline interpolation for its output, not for turn_signal processing (#909)""

This reverts commit e6dd540.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(system_monitor): fix program command line reading (backport autowarefoundation#5191, autowarefoundation#5430) (#995)

* perf(system_monitor): fix program command line reading (autowarefoundation#5191)

* Fix program command line reading

Signed-off-by: Owen-Liuyuxuan <[email protected]>

* style(pre-commit): autofix

* fix spelling commandline->command_line

Signed-off-by: Owen-Liuyuxuan <[email protected]>

---------

Signed-off-by: Owen-Liuyuxuan <[email protected]>
Co-authored-by: Owen-Liuyuxuan <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(system_monitor): output command line (autowarefoundation#5430)

* fix(system_monitor): output command line

Signed-off-by: takeshi.iwanari <[email protected]>

* style(pre-commit): autofix

---------

Signed-off-by: takeshi.iwanari <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

---------

Signed-off-by: Owen-Liuyuxuan <[email protected]>
Signed-off-by: takeshi.iwanari <[email protected]>
Co-authored-by: Yuxuan Liu <[email protected]>
Co-authored-by: Owen-Liuyuxuan <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: takeshi-iwanari <[email protected]>
Co-authored-by: Akihisa Nagata <[email protected]>

* feat(obstacle_stop): upd obstacle hunting (#1068)

* Adapted from PR #1458

Signed-off-by: Shigekazu Fukuta <[email protected]>

* Adapted from PR #1627

Signed-off-by: Shigekazu Fukuta <[email protected]>

* fix parameter name

Signed-off-by: Shigekazu Fukuta <[email protected]>

* Adapted from PR autowarefoundation#2647

Signed-off-by: Shigekazu Fukuta <[email protected]>

* ci(pre-commit): autofix

* fix build error

* ci(pre-commit): autofix

* remove comment line

* remove logic

* Delete parameters other than those added this time

* ci(pre-commit): autofix

* add stop vehicle check

* ci(pre-commit): autofix

* fix stop velocity threshold

* fix engage obstacle clear and stop threshold

* ci(pre-commit): autofix

---------

Signed-off-by: Shigekazu Fukuta <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(obstacle_avoidance_planner): add empty check (#1285)

* fix(obstacle_avoidance_planner): add empty check

* ci(pre-commit): autofix

* add invalid_argument

* delete empty check

* return code moved to end

* add warning log

* update rclcpp_debug

* delete debug log

* Delete unnecessary blank lines

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix(virtual traffic light): suppress lauch (#1290)

* suppress launch

Signed-off-by: Yuki Takagi <[email protected]>

* add existence check

Signed-off-by: Yuki Takagi <[email protected]>

---------

Signed-off-by: Yuki Takagi <[email protected]>
Co-authored-by: Shigekazu Fukuta <[email protected]>

* revert: #929,#909,#782

* revert: #1068
- d56c191.

---------

Signed-off-by: ito-san <[email protected]>
Signed-off-by: Takayuki Murooka <[email protected]>
Signed-off-by: Keisuke Shima <[email protected]>
Signed-off-by: Owen-Liuyuxuan <[email protected]>
Signed-off-by: takeshi.iwanari <[email protected]>
Signed-off-by: Shigekazu Fukuta <[email protected]>
Signed-off-by: Yuki Takagi <[email protected]>
Co-authored-by: Hiroki OTA <[email protected]>
Co-authored-by: ito-san <[email protected]>
Co-authored-by: Takayuki Murooka <[email protected]>
Co-authored-by: Keisuke Shima <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Yuxuan Liu <[email protected]>
Co-authored-by: Owen-Liuyuxuan <[email protected]>
Co-authored-by: takeshi-iwanari <[email protected]>
Co-authored-by: Shigekazu Fukuta <[email protected]>
Co-authored-by: Yuki TAKAGI <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants