feat(autoware_external_cmd_converter): add ext cmd converter tests #9118
Conversation
danielsanchezaran
added
the
run:build-and-test-differential
github-actions
bot
added
the
component:vehicle
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9118 +/- ##
==========================================
+ Coverage 28.48% 28.59% +0.10%
==========================================
Files 1306 1307 +1
Lines 101074 101196 +122
Branches 39234 39333 +99
==========================================
+ Hits 28792 28934 +142
+ Misses 69339 69277 -62
- Partials 2943 2985 +42
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| std::shared_ptr<ExternalCmdConverterNode> external_cmd_converter_; | ||
| }; | ||
|
|
||
| TEST_F(TestExternalCmdConverter, testCheckEmergencyStopTimeout) |
There was a problem hiding this comment.
Would you show a brief description of each test case?
| EXPECT_DOUBLE_EQ(0.0, test_get_shift_velocity_sign(cmd)); | ||
| } | ||
|
|
||
| TEST_F(TestExternalCmdConverter, testCalculateAcc) |
There was a problem hiding this comment.
Could you make test cases for boundary conditions? For example, a case with throttle = Inf (large number)?
There was a problem hiding this comment.
@TakaHoribe thanks for your suggestions. I have added descriptions to the tests and functions used to evaluate. I also added combinations of std::numeric_limits::infinity() to the throttle and brake. Could you please check again?
There was a problem hiding this comment.
You only check if the output is positive/negative for infinite cases. Are you sure that those outputs are not nan nor other invalid numbers?
This PR looks ok if the above concern is solved.
Signed-off-by: Daniel Sanchez <danielsanchezaran@gmail.com>
Signed-off-by: Daniel Sanchez <danielsanchezaran@gmail.com>
Signed-off-by: Daniel Sanchez <danielsanchezaran@gmail.com>
danielsanchezaran
force-pushed
the
feat/add-ext-cmd-converter-tests
branch
from
771811c to
f39fd7a
Compare
| if ( | ||
| std::isnan(desired_throttle) || std::isnan(desired_brake) || std::isinf(desired_throttle) || | ||
| std::isinf(desired_brake)) { | ||
| std::cerr << "Input brake or throttle is out of range. returning 0.0 acceleration." | ||
| << std::endl; | ||
| return 0.0; | ||
| } | ||
|
|
There was a problem hiding this comment.
| if ( | |
| std::isnan(desired_throttle) || std::isnan(desired_brake) || std::isinf(desired_throttle) || | |
| std::isinf(desired_brake)) { | |
| std::cerr << "Input brake or throttle is out of range. returning 0.0 acceleration." | |
| << std::endl; | |
| return 0.0; | |
| } | |
| if ( | |
| std::isnan(desired_throttle) || std::isnan(desired_brake) || std::isinf(desired_throttle) || | |
| std::isinf(desired_brake)) { | |
| std::cerr << "Input brake or throttle is out of range. returning 0.0 acceleration." | |
| << std::endl; | |
| return 0.0; | |
| } | |
@TakaHoribe I noticed there is no proper handling of Nan or infinite values in the external_cmd_converter so I decided to add this check, but I am wondering if returning 0 acceleration is the right answer in this case?
I have also made checks to combinations of infinity and nan inputs and checking the output to not be NaN or inf.
danielsanchezaran
merged commit 26f617d
into
autowarefoundation:main
Description
Add tests to increase test coverage of external cmd converter
Related links
Parent Issue:
How was this PR tested?
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.