feat(diagnostic_converter): add converter to use planning_evaluator's output for scenario's condition #2514

kyoichi-sugahara · 2022-12-16T06:01:55Z

Description

background

Planning_evaluator node is designed to evaluate the quality of planning and control.
We would like to make use of the evaluation result for the scenairo simulation's pass/fail judgment.
Planing evaluator's output diagnostic_msgs::msg::DiagnosticArray can't be understood by scenario_simulator_v2 directly.
So in this PR, converter node which convert from diagnostic_msgs::msg::DiagnosticArray to tier4_simulator_msgs::msg::UserDefinedValue which can be understood by scenario_simulator_v2 is implemented.

discussuion

UserDefinedValue msg is originally included in scenario_simulator_v2 repository
scenario_simulator_v2 side want to keep their compability with other system not only autoware. And autoware-dependent implementation (converter node in this case) shouldn't be included in the scenario_simulator_v2 repository.

node diagram and interface between with `scenario_simulator_v2`

Proposal in related PRs

Take UserDefinedValue msg out from scenario_simulator_v2 and tried to make repository for the msg. We noticed this is not the only option with the comment and we decided to add this msg in tier4_autoware_msgs. You can check discussion in this PR
implement converter node in autoware side not in scenario_simulator_v2 repositpry

Related PRs

tier4/tier4_autoware_msgs#73: Adding new message type UserDefinedValue and UserDefinedValueCondition to package tier4_simulation_msgs
autowarefoundation/autoware_launch#219: Adding argument so that planning_evaluator node will be launched when scenario_simulator_v2 is running.
tier4/scenario_simulator_v2#874: modify scenario_simulator_v2 so that the output from convertor node cen be understood by scenario_simulator_v2.

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.
The PR follows the pull request guidelines.

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.

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

…tion

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

maxime-clem · 2023-01-05T06:32:02Z

I am unable to build the new package due to a missing message:

Starting >>> diagnostic_converter
--- stderr: diagnostic_converter                               
In file included from /home/mclement/autoware/autoware/src/universe/autoware.universe/planning/diagnostic_converter/src/converter_node.cpp:15:
/home/mclement/autoware/autoware/src/universe/autoware.universe/planning/diagnostic_converter/include/converter_node.hpp:21:10: fatal error: scenario_simulator_v2_msgs/msg/user_defined_value.hpp: No such file or directory
   21 | #include <scenario_simulator_v2_msgs/msg/user_defined_value.hpp>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In the CI this also causes an issue:

diagnostic_converter: Cannot locate rosdep definition for [scenario_simulator_v2_msgs]

kyoichi-sugahara · 2023-01-11T00:36:13Z

@maxime-clem
sorry please use this branch
https://github.com/tier4/pilot-auto/tree/feature/evaluator_integration

…tion

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

…tion

maxime-clem

Tested and it works well.

kyoichi-sugahara · 2023-03-07T01:55:37Z

@mitsudome-r
Could you review this PR?
I need approve from autoware-global-codeowners.

mitsudome-r · 2023-03-07T07:35:41Z

launch/tier4_simulator_launch/launch/simulator.launch.xml

+  <group if="$(var launch_planning_evaluator)">
+    <include file="$(find-pkg-share planning_evaluator)/launch/planning_evaluator.launch.xml"/>
+  </group>


@TakaHoribe I remember you saying that planning_evaluator nodes are expected to launch regardless of simulation or real vehicle driving (e.g., to publish diagnostics for MRM triggering). Is it correct?
If that's true, then I think this should be included from planning.launch, not simulator.launch

@kyoichi-sugahara @mitsudome-r Thank you for the mention. Right, the planning evaluator should be launched in both sim and real, and thus be called from the planning.launch

@mitsudome-r @TakaHoribe
I have modified code so that the planning_evaluator node is always laucnhed from planning.launch and confirmed its behavior with this new PR.

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

evaluator/diagnostic_converter/CMakeLists.txt

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

KeisukeShima

LGTM

… output for scenario's condition (autowarefoundation#2514) * add original diagnostic_convertor Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * add test Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * fix typo Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * delete file Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * change include Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * temp Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * delete comments Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * made launch for converter Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * ci(pre-commit): autofix * ci(pre-commit): autofix * add diagnostic convertor in launch Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * ci(pre-commit): autofix * change debug from info Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * change arg name to launch diagnostic convertor * add planning_evaluator launcher in simulator.launch.xml Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * fix arg wrong setting Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * style(pre-commit): autofix * use simulation msg in tier4_autoware_msgs Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * style(pre-commit): autofix * fix README Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * style(pre-commit): autofix * refactoring Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * style(pre-commit): autofix * remove unnecessary dependency Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * remove unnecessary dependency Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * move folder Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * reformat Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * style(pre-commit): autofix * Update evaluator/diagnostic_converter/include/converter_node.hpp Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com> * Update evaluator/diagnostic_converter/README.md Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com> * Update evaluator/diagnostic_converter/src/converter_node.cpp Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com> * Update evaluator/diagnostic_converter/test/test_converter_node.cpp Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com> * define diagnostic_topics as parameter Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * fix include way Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * fix include way Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * delete ament_cmake_clang_format from package.xml Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * fix test_depend Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> * Update evaluator/diagnostic_converter/test/test_converter_node.cpp Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com> * style(pre-commit): autofix * Update launch/tier4_simulator_launch/launch/simulator.launch.xml Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com> --------- Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com>

kyoichi-sugahara added 8 commits December 16, 2022 14:56

add original diagnostic_convertor

758cc2a

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

add test

8b9d18a

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

fix typo

7a73248

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

delete file

628d02a

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

change include

0290994

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

temp

8be0906

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

delete comments

015806d

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

made launch for converter

53a2456

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Dec 16, 2022

ci(pre-commit): autofix

bb98e48

kyoichi-sugahara changed the title ~~Feature/evaluator integration~~ feat(diagnostic_converter): add converter to use planning_evaluator's output for scenario's condition Dec 16, 2022

kyoichi-sugahara marked this pull request as ready for review December 21, 2022 06:02

kyoichi-sugahara requested review from maxime-clem and a team as code owners December 21, 2022 06:02

kyoichi-sugahara and others added 6 commits December 22, 2022 14:47

Merge branch 'autowarefoundation:main' into feature/evaluator_integra…

bab02f1

…tion

ci(pre-commit): autofix

826c484

add diagnostic convertor in launch

1abce95

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

ci(pre-commit): autofix

de108fd

change debug from info

3d78f70

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

Merge branch 'main' into feature/evaluator_integration

a9be8ae

kyoichi-sugahara mentioned this pull request Dec 22, 2022

feat(planning_evaluator): add planning_evaluator in planning_simulator.launch.xml tier4/autoware_launch-old#645

Closed

8 tasks

kyoichi-sugahara marked this pull request as draft January 18, 2023 06:57

kyoichi-sugahara added 3 commits January 27, 2023 14:56

change arg name to launch diagnostic convertor

0b90bf2

Merge branch 'autowarefoundation:main' into feature/evaluator_integra…

3199d3c

…tion

add planning_evaluator launcher in simulator.launch.xml

b5209b1

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

github-actions bot added the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Jan 27, 2023

Merge branch 'autowarefoundation:main' into feature/evaluator_integra…

f6658a0

…tion

maxime-clem approved these changes Mar 7, 2023

View reviewed changes

kyoichi-sugahara enabled auto-merge (squash) March 7, 2023 01:00

mitsudome-r reviewed Mar 7, 2023

View reviewed changes

launch planning_evaluator from planning.launch

a910ae1

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

kyoichi-sugahara requested review from rej55, mkuri, soblin, satoshi-ota, shmpwk, taikitanaka3, takayuki5168, TomohitoAndo, tkimura4, purewater0901 and zulfaqar-azmi-t4 as code owners March 8, 2023 04:37

Merge branch 'main' into feature/evaluator_integration

8c216ac

kyoichi-sugahara mentioned this pull request Mar 8, 2023

feat(planning_evaluator): delete_launch_planning_evaluator_arg autowarefoundation/autoware_launch#240

Merged

4 tasks

Merge branch 'main' into feature/evaluator_integration

a4a21a2

mitsudome-r approved these changes Mar 9, 2023

View reviewed changes

zulfaqar-azmi-t4 approved these changes Mar 9, 2023

View reviewed changes

KeisukeShima reviewed Mar 10, 2023

View reviewed changes

evaluator/diagnostic_converter/CMakeLists.txt Show resolved Hide resolved

add pluginlib package

c391c37

Signed-off-by: kyoichi-sugahara <kyoichi.sugahara@tier4.jp>

KeisukeShima approved these changes Mar 10, 2023

View reviewed changes

kyoichi-sugahara merged commit fa9b018 into autowarefoundation:main Mar 10, 2023

tier4-autoware-public-bot bot mentioned this pull request Mar 11, 2023

chore: sync upstream tier4/autoware.universe#311

Merged

kosuke55 mentioned this pull request Mar 16, 2023

feat(planing_evaluator): add dianostic_converter and modfied_goal_deviation tier4/autoware.universe#322

Merged

7 tasks

kyoichi-sugahara deleted the feature/evaluator_integration branch April 15, 2023 03:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(diagnostic_converter): add converter to use planning_evaluator's output for scenario's condition #2514

feat(diagnostic_converter): add converter to use planning_evaluator's output for scenario's condition #2514

kyoichi-sugahara commented Dec 16, 2022 •

edited

Loading

maxime-clem commented Jan 5, 2023

kyoichi-sugahara commented Jan 11, 2023

maxime-clem left a comment

kyoichi-sugahara commented Mar 7, 2023

mitsudome-r Mar 7, 2023

TakaHoribe Mar 8, 2023

kyoichi-sugahara Mar 8, 2023

KeisukeShima left a comment

feat(diagnostic_converter): add converter to use planning_evaluator's output for scenario's condition #2514

feat(diagnostic_converter): add converter to use planning_evaluator's output for scenario's condition #2514

Conversation

kyoichi-sugahara commented Dec 16, 2022 • edited Loading

Description

background

discussuion

node diagram and interface between with scenario_simulator_v2

Proposal in related PRs

Related PRs

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

maxime-clem commented Jan 5, 2023

kyoichi-sugahara commented Jan 11, 2023

maxime-clem left a comment

Choose a reason for hiding this comment

kyoichi-sugahara commented Mar 7, 2023

mitsudome-r Mar 7, 2023

Choose a reason for hiding this comment

TakaHoribe Mar 8, 2023

Choose a reason for hiding this comment

kyoichi-sugahara Mar 8, 2023

Choose a reason for hiding this comment

KeisukeShima left a comment

Choose a reason for hiding this comment

kyoichi-sugahara commented Dec 16, 2022 •

edited

Loading

node diagram and interface between with `scenario_simulator_v2`