feat(raw_vehicle_cmd_converter): validate map at first before run time #2152

taikitanaka3 · 2022-10-25T09:06:40Z

Signed-off-by: taikitanaka3 taiki.tanaka@tier4.jp

resolve: #2151

Description

1: [==========] Running 5 tests from 1 test suite.
1: [----------] Global test environment set-up.
1: [----------] 5 tests from ConverterTests
1: [ RUN      ] ConverterTests.LoadExampleMap
1: [       OK ] ConverterTests.LoadExampleMap (1 ms)
1: [ RUN      ] ConverterTests.LoadValidPath
1: Cannot open invalid.csv
1: Cannot open invalid.csv
1: Cannot open invalid.csv
1: Cannot read /home/ros2-galatic/workspace/Universe.proj/install/raw_vehicle_cmd_converter/share/raw_vehicle_cmd_converter/test/map_data/test_1col_map.csv CSV file should have at least 2 column
1: Cannot read /home/ros2-galatic/workspace/Universe.proj/install/raw_vehicle_cmd_converter/share/raw_vehicle_cmd_converter/test/map_data/test_inconsistent_rows_map.csv. Each row should have a same number of columns
1: index around (i,j) is invalid ( 2, 1 )
1: [       OK ] ConverterTests.LoadValidPath (0 ms)
1: [ RUN      ] ConverterTests.AccelMapCalculation
1: Input throttle: vel: 20 is out of range. use closest value.
1: Input throttle: acc: 2 is out of range. use closest value.
1: [       OK ] ConverterTests.AccelMapCalculation (0 ms)
1: [ RUN      ] ConverterTests.BrakeMapCalculation
1: Input brake: vel: 11 is out of range. use closest value.
1: Input brake: acc: 1.1 is out of range. use closest value.
1: Input brake: vel: 12 is out of range. use closest value.
1: Input brake: acc: 1.1 is out of range. use closest value.
1: [       OK ] ConverterTests.BrakeMapCalculation (0 ms)
1: [ RUN      ] ConverterTests.SteerMapCalculation
1: [       OK ] ConverterTests.SteerMapCalculation (0 ms)
1: [----------] 5 tests from ConverterTests (1 ms total)
1: 
1: [----------] Global test environment tear-down
1: [==========] 5 tests from 1 test suite ran. (1 ms total)
1: [  PASSED  ] 5 tests.
1: -- run_test.py: return code 0

Tests performed

validation with existing acc brake sterr map
validate with more realistic params
add unit test for invalid map case

checked external cmd converter runs successful with psim

Notes for reviewers

see if unit tests are successful

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.
The PR has been properly tested.
The PR has been reviewed by the code owners.

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.
The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp>

vehicle/raw_vehicle_cmd_converter/src/node.cpp

codecov · 2022-10-25T09:25:10Z

Codecov Report

Base: 10.76% // Head: 10.82% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (06db32f) compared to base (641b84b).
Patch coverage: 43.24% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2152      +/-   ##
==========================================
+ Coverage   10.76%   10.82%   +0.05%     
==========================================
  Files        1186     1186              
  Lines       84826    84965     +139     
  Branches    19889    19978      +89     
==========================================
+ Hits         9133     9195      +62     
- Misses      66008    66041      +33     
- Partials     9685     9729      +44

Flag	Coverage Δ		*Carryforward flag
differential	`7.35% <22.91%> (?)`
total	`10.74% <50.98%> (ø)`		Carriedforward from 01edb12

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
...r/include/raw_vehicle_cmd_converter/csv_loader.hpp	`100.00% <ø> (ø)`
...ehicle/raw_vehicle_cmd_converter/src/brake_map.cpp	`43.18% <0.00%> (+9.84%)`	⬆️
vehicle/raw_vehicle_cmd_converter/src/node.cpp	`0.00% <0.00%> (ø)`
..._converter/test/test_raw_vehicle_cmd_converter.cpp	`37.39% <25.71%> (+9.61%)`	⬆️
...hicle/raw_vehicle_cmd_converter/src/csv_loader.cpp	`67.85% <70.37%> (-2.36%)`	⬇️
...ehicle/raw_vehicle_cmd_converter/src/accel_map.cpp	`39.39% <100.00%> (-1.35%)`	⬇️
...ehicle/raw_vehicle_cmd_converter/src/steer_map.cpp	`50.00% <100.00%> (+2.94%)`	⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp>

taikitanaka3 · 2022-10-26T00:58:32Z

vehicle/raw_vehicle_cmd_converter/src/csv_loader.cpp

+      }
+    }
+  }
+  if (is_invalid) {


Note: add output where is invalid in map

taikitanaka3 · 2022-10-26T01:00:26Z

vehicle/raw_vehicle_cmd_converter/test/map_data/test_accel_map.csv

-0,      1.0,  11.0,  21.0
-0.5,    2.0,  22.0,  42.0
-1.0,    3.0,  33.0,  46.0
+default,0.0,  5.0,  10.0


Note: use more realistic params for test because it's hard to tell if case "calcThrottle" should use accel map or brake map for single unit test

taikitanaka3 · 2022-10-26T01:03:28Z

vehicle/raw_vehicle_cmd_converter/test/test_raw_vehicle_cmd_converter.cpp

+  const auto data_path =
+    ament_index_cpp::get_package_share_directory("raw_vehicle_cmd_converter") + "/data/default/";
+  // for invalid path
+  EXPECT_TRUE(accel_map.readAccelMapFromCSV(data_path + "accel_map.csv"));


Note: add test with existing accel / brake / steer map for unit test

taikitanaka3 · 2022-10-26T01:11:29Z

vehicle/raw_vehicle_cmd_converter/src/csv_loader.cpp

@@ -100,10 +142,10 @@ std::vector<double> CSVLoader::getColumnIndex(const Table & table)
 double CSVLoader::clampValue(
  const double val, const std::vector<double> & ranges, const std::string & name)
 {
-  const double max_value = ranges.back();
-  const double min_value = ranges.front();
+  const double max_value = *std::max_element(ranges.begin(), ranges.end());


Note: max = vec.back() min = vec.front() is common for current map format but I want this change for safe

min ----> max min | max

taikitanaka3 · 2022-10-26T01:19:10Z

vehicle/raw_vehicle_cmd_converter/src/node.cpp

@@ -148,25 +144,17 @@ double RawVehicleCommandConverterNode::calculateSteer(
  double dt = (current_time - prev_time_steer_calculation_).seconds();
  if (std::abs(dt) > 1.0) {
    RCLCPP_WARN_EXPRESSION(get_logger(), is_debugging_, "ignore old topic");
-    dt = 0.0;
+    dt = 0.1;  // set ordinaray delta time instead


Note: add ordinary dt for delay case

TakaHoribe

LGTM!

Signed-off-by: tanaka3 <ttatcoder@outlook.jp>

This reverts commit 0350724.

autowarefoundation#2152) * feat(raw_vehicle_cmd_converter): validate map at first before run time Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> * chore: remove debug * fix: clamp method and add more unit tests Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> * feat: add more realistic params Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> * chore: revert to simple map Signed-off-by: tanaka3 <ttatcoder@outlook.jp> * Revert "chore: revert to simple map" This reverts commit 0350724. Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> Signed-off-by: tanaka3 <ttatcoder@outlook.jp>

autowarefoundation#2152) * feat(raw_vehicle_cmd_converter): validate map at first before run time Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> * chore: remove debug * fix: clamp method and add more unit tests Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> * feat: add more realistic params Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> * chore: revert to simple map Signed-off-by: tanaka3 <ttatcoder@outlook.jp> * Revert "chore: revert to simple map" This reverts commit 0350724. Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp> Signed-off-by: tanaka3 <ttatcoder@outlook.jp> Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>

feat(raw_vehicle_cmd_converter): validate map at first before run time

7d469cb

Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp>

taikitanaka3 commented Oct 25, 2022

View reviewed changes

vehicle/raw_vehicle_cmd_converter/src/node.cpp Outdated Show resolved Hide resolved

chore: remove debug

23a00a4

taikitanaka3 requested a review from tkimura4 October 25, 2022 09:08

taikitanaka3 added 2 commits October 25, 2022 19:38

fix: clamp method and add more unit tests

2137f7f

Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp>

feat: add more realistic params

01edb12

Signed-off-by: taikitanaka3 <taiki.tanaka@tier4.jp>

taikitanaka3 marked this pull request as ready for review October 25, 2022 15:15

taikitanaka3 requested a review from TakaHoribe as a code owner October 25, 2022 15:15

taikitanaka3 requested a review from mkuri October 25, 2022 15:17

taikitanaka3 commented Oct 26, 2022

View reviewed changes

TakaHoribe approved these changes Oct 26, 2022

View reviewed changes

taikitanaka3 added 2 commits October 26, 2022 11:17

chore: revert to simple map

0350724

Signed-off-by: tanaka3 <ttatcoder@outlook.jp>

Revert "chore: revert to simple map"

06db32f

This reverts commit 0350724.

taikitanaka3 enabled auto-merge (squash) October 26, 2022 02:55

taikitanaka3 merged commit 897794a into autowarefoundation:main Oct 26, 2022

taikitanaka3 deleted the feature/validate_acc_brake_map branch October 26, 2022 03:01

tier4-autoware-public-bot bot mentioned this pull request Oct 27, 2022

chore: sync upstream tier4/autoware.universe#162

Merged

tkimura4 mentioned this pull request Oct 27, 2022

fix: avoid same value in accel and brake map #2160

Merged

4 tasks

taikitanaka3 mentioned this pull request Dec 5, 2022

fix(raw_vehicle_cmd_converter): fix unnecessary row check #2446

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(raw_vehicle_cmd_converter): validate map at first before run time #2152

feat(raw_vehicle_cmd_converter): validate map at first before run time #2152

taikitanaka3 commented Oct 25, 2022 •

edited

Loading

codecov bot commented Oct 25, 2022 •

edited

Loading

taikitanaka3 Oct 26, 2022

taikitanaka3 Oct 26, 2022 •

edited

Loading

taikitanaka3 Oct 26, 2022

taikitanaka3 Oct 26, 2022

taikitanaka3 Oct 26, 2022

TakaHoribe left a comment

feat(raw_vehicle_cmd_converter): validate map at first before run time #2152

feat(raw_vehicle_cmd_converter): validate map at first before run time #2152

Conversation

taikitanaka3 commented Oct 25, 2022 • edited Loading

Description

Related links

Tests performed

Notes for reviewers

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

codecov bot commented Oct 25, 2022 • edited Loading

Codecov Report

taikitanaka3 Oct 26, 2022

Choose a reason for hiding this comment

taikitanaka3 Oct 26, 2022 • edited Loading

Choose a reason for hiding this comment

taikitanaka3 Oct 26, 2022

Choose a reason for hiding this comment

taikitanaka3 Oct 26, 2022

Choose a reason for hiding this comment

taikitanaka3 Oct 26, 2022

Choose a reason for hiding this comment

TakaHoribe left a comment

Choose a reason for hiding this comment

taikitanaka3 commented Oct 25, 2022 •

edited

Loading

codecov bot commented Oct 25, 2022 •

edited

Loading

taikitanaka3 Oct 26, 2022 •

edited

Loading