-
Notifications
You must be signed in to change notification settings - Fork 668
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(raw_vehicle_cmd_converter): fix parameter files to parse path to csv files #6136
fix(raw_vehicle_cmd_converter): fix parameter files to parse path to csv files #6136
Conversation
…csv files Signed-off-by: Fumiya Watanabe <[email protected]>
@PhoebeWu21 I changed the parameter file in |
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.
LGTM
Looks good to me. |
@PhoebeWu21 All parameters defined in the C++ code should be listed in the schema and parameter files according to the ROS Node Configuration page. If we want both that and being able to pass the parameters from external launch file, I think this leads to the same discussion about parameter override as in #5773. I don't think there was a clear conclusion on what should be done. |
@PhoebeWu21 @ambroise-arm Thank you for your comments! |
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.
This PR looks good to me to fix the current problem. However, we need to discuss how to support both the json schema and passing arguments from launch as in the comment from @ambroise-arm .
@rej55 Would you make an issue for this?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6136 +/- ##
=======================================
Coverage 14.60% 14.60%
=======================================
Files 1860 1860
Lines 127418 127418
Branches 37286 37286
=======================================
Hits 18606 18606
Misses 87903 87903
Partials 20909 20909
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
It seems the same issue is discussed in autowarefoundation/autoware-github-actions#232 |
…csv files (autowarefoundation#6136) Signed-off-by: Fumiya Watanabe <[email protected]>
…csv files (autowarefoundation#6136) Signed-off-by: Fumiya Watanabe <[email protected]>
…csv files (autowarefoundation#6136) Signed-off-by: Fumiya Watanabe <[email protected]>
…csv files (autowarefoundation#6136) (#1139) Signed-off-by: Fumiya Watanabe <[email protected]> Co-authored-by: Fumiya Watanabe <[email protected]>
…csv files (autowarefoundation#6136) (#1139) Signed-off-by: Fumiya Watanabe <[email protected]> Co-authored-by: Fumiya Watanabe <[email protected]> (cherry picked from commit 604848f)
…csv files (autowarefoundation#6136) Signed-off-by: Fumiya Watanabe <[email protected]>
Description
According to #5134, some parameters are moved into yaml files.
However, if we launch
raw_vehicle_cmd_converter
fromautoware_launch
, some parameters defined in launch files does not appear.In this change, I fixed above problems.
accel/brake/steer map path should change at launch, so I removed
csv_path_**_map
from yaml file and add them into xml file.Tests performed
Not applicable.
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.