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

feat(raw_vehicle_cmd_converter): add steer command conversion with VGR #8504

Merged
merged 9 commits into from
Aug 22, 2024

Conversation

kosuke55
Copy link
Contributor

@kosuke55 kosuke55 commented Aug 16, 2024

Description

Added conversion from steer angle to handle angle by variable gear ratio.
Both cmd and status are supported. (However, status conversion is not supported for steer_map.)
This is to unify the conversion of control commands, which has been done in vehicle_interface.
Also, status correction will be implemented in the future.

image

Related links

Parent Issue:

  • Link

How was this PR tested?

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

@github-actions github-actions bot added the component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) label Aug 16, 2024
Copy link

github-actions bot commented Aug 16, 2024

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@kosuke55 kosuke55 added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Aug 16, 2024
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 12.32877% with 64 lines in your changes missing coverage. Please review.

Project coverage is 23.89%. Comparing base (9a461b4) to head (5b667cc).
Report is 45 commits behind head on main.

Files Patch % Lines
...le/autoware_raw_vehicle_cmd_converter/src/node.cpp 0.00% 63 Missing ⚠️
...include/autoware_raw_vehicle_cmd_converter/vgr.hpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8504      +/-   ##
==========================================
- Coverage   23.90%   23.89%   -0.01%     
==========================================
  Files        1381     1384       +3     
  Lines      101923   101995      +72     
  Branches    38808    38819      +11     
==========================================
+ Hits        24363    24372       +9     
- Misses      75116    75179      +63     
  Partials     2444     2444              
Flag Coverage Δ *Carryforward flag
differential 4.79% <12.32%> (?)
total 23.90% <ø> (+<0.01%) ⬆️ Carriedforward from 5bc37c2

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@TakaHoribe TakaHoribe left a comment

Choose a reason for hiding this comment

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

Could you update the README, particularly by adding an image of the steering conversion scheme and clarifying/plotting what kind of value the VGR would be?

@kosuke55
Copy link
Contributor Author

@TakaHoribe

Could you update the README, particularly by adding an image of the steering conversion scheme and clarifying/plotting what kind of value the VGR would be?

thanks! I updated readme and scripts for plot.

@@ -86,11 +109,11 @@ RawVehicleCommandConverterNode::RawVehicleCommandConverterNode(

void RawVehicleCommandConverterNode::publishActuationCmd()
{
if (!current_twist_ptr_ || !control_cmd_ptr_ || !current_steer_ptr_) {
if (!current_twist_ptr_ || !control_cmd_ptr_ || !current_steer_ptr_ || !actuation_status_ptr_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Some vehicles do not publish actiation_status. This is not necessary for vehicles that do not require the steering conversion. Please make a specific condition for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! fixed in 0a660ef

@@ -215,6 +241,41 @@ void RawVehicleCommandConverterNode::onControlCmd(const Control::ConstSharedPtr
control_cmd_ptr_ = msg;
publishActuationCmd();
}

void RawVehicleCommandConverterNode::onSteering(const Steering::ConstSharedPtr msg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please mention that this subscriber can not be replaced by take() method in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally it was take(), in which case the topic name must be stated in the declaration. In that case, there would be two identical topics. Therefore, subscriber, which does not require a topic name to be specified at the time of declaration, is used.
In any case, add a comment to the code.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, there would be two identical topics.

I mean, this node also need to publish steering topic if converting option is true.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comment why not use polling sub

5b667cc

@TakaHoribe TakaHoribe self-requested a review August 22, 2024 04:12
@kosuke55 kosuke55 merged commit d95cfae into autowarefoundation:main Aug 22, 2024
27 of 30 checks passed
@kosuke55 kosuke55 deleted the feat/vgr_converter branch August 22, 2024 04:59
kosuke55 added a commit to tier4/autoware.universe that referenced this pull request Aug 22, 2024
autowarefoundation#8504)

* feat(raw_vehicle_cmd_converter): add steer command conversion with VGR

Signed-off-by: kosuke55 <[email protected]>

* make class and add test

Signed-off-by: kosuke55 <[email protected]>

* remove member vgr_coef from node

Signed-off-by: kosuke55 <[email protected]>

* update readme

Signed-off-by: kosuke55 <[email protected]>

* add svg

Signed-off-by: kosuke55 <[email protected]>

* add plot scripts

Signed-off-by: kosuke55 <[email protected]>

* Update vehicle/autoware_raw_vehicle_cmd_converter/README.md

Co-authored-by: Takamasa Horibe <[email protected]>

* not always subscribe actuation_status

Signed-off-by: kosuke55 <[email protected]>

* add comment for using normal sub for steering status

Signed-off-by: kosuke55 <[email protected]>

---------

Signed-off-by: kosuke55 <[email protected]>
Co-authored-by: Takamasa Horibe <[email protected]>
kyoichi-sugahara pushed a commit to kyoichi-sugahara/autoware.universe that referenced this pull request Aug 27, 2024
autowarefoundation#8504)

* feat(raw_vehicle_cmd_converter): add steer command conversion with VGR

Signed-off-by: kosuke55 <[email protected]>

* make class and add test

Signed-off-by: kosuke55 <[email protected]>

* remove member vgr_coef from node

Signed-off-by: kosuke55 <[email protected]>

* update readme

Signed-off-by: kosuke55 <[email protected]>

* add svg

Signed-off-by: kosuke55 <[email protected]>

* add plot scripts

Signed-off-by: kosuke55 <[email protected]>

* Update vehicle/autoware_raw_vehicle_cmd_converter/README.md

Co-authored-by: Takamasa Horibe <[email protected]>

* not always subscribe actuation_status

Signed-off-by: kosuke55 <[email protected]>

* add comment for using normal sub for steering status

Signed-off-by: kosuke55 <[email protected]>

---------

Signed-off-by: kosuke55 <[email protected]>
Co-authored-by: Takamasa Horibe <[email protected]>
kosuke55 added a commit to tier4/autoware.universe that referenced this pull request Aug 29, 2024
autowarefoundation#8504)

* feat(raw_vehicle_cmd_converter): add steer command conversion with VGR

Signed-off-by: kosuke55 <[email protected]>

* make class and add test

Signed-off-by: kosuke55 <[email protected]>

* remove member vgr_coef from node

Signed-off-by: kosuke55 <[email protected]>

* update readme

Signed-off-by: kosuke55 <[email protected]>

* add svg

Signed-off-by: kosuke55 <[email protected]>

* add plot scripts

Signed-off-by: kosuke55 <[email protected]>

* Update vehicle/autoware_raw_vehicle_cmd_converter/README.md

Co-authored-by: Takamasa Horibe <[email protected]>

* not always subscribe actuation_status

Signed-off-by: kosuke55 <[email protected]>

* add comment for using normal sub for steering status

Signed-off-by: kosuke55 <[email protected]>

---------

Signed-off-by: kosuke55 <[email protected]>
Co-authored-by: Takamasa Horibe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants