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(longitudinal_controller): skip integral in manual mode #2619

Merged

Conversation

TakaHoribe
Copy link
Contributor

@TakaHoribe TakaHoribe commented Jan 8, 2023

Description

In the current implementation, if the vehicle speed is almost zero, the control error integral is skipped. However, when the vehicle is moving under manual control, the integration is performed which is unexpected behavior. This causes an overshoot when the vehicle is engaged while driving.

This PR fixes the issue by disabling the integration when the control_mode is not in autonomous.

Related links

must be merged with tier4/autoware_launch#683

Tests performed

  • psim, run/stop

Notes for reviewers

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.

  • 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.

@TakaHoribe TakaHoribe requested review from a team and takayuki5168 as code owners January 8, 2023 09:57
@github-actions github-actions bot added component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) labels Jan 8, 2023
@TakaHoribe TakaHoribe force-pushed the feature/no-integration-in-manual-mode branch from c3a4a14 to 2f09489 Compare January 23, 2023 02:58
Copy link
Contributor

@takayuki5168 takayuki5168 left a comment

Choose a reason for hiding this comment

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

LGTM

@takayuki5168
Copy link
Contributor

@TakaHoribe Test on CI failed. Regarding the smoke_test, operation_mode must be published so that the input_data will be initialized?

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>
Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>
Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>
@TakaHoribe TakaHoribe force-pushed the feature/no-integration-in-manual-mode branch from 8412b5c to fca9ba0 Compare January 23, 2023 08:36
@codecov
Copy link

codecov bot commented Jan 23, 2023

Codecov Report

Base: 11.64% // Head: 12.57% // Increases project coverage by +0.92% 🎉

Coverage data is based on head (2373332) compared to base (d5a627e).
Patch coverage: 50.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2619      +/-   ##
==========================================
+ Coverage   11.64%   12.57%   +0.92%     
==========================================
  Files        1312     1203     -109     
  Lines       91430    85458    -5972     
  Branches    24327    24262      -65     
==========================================
+ Hits        10648    10745      +97     
+ Misses      69667    63607    -6060     
+ Partials    11115    11106       -9     
Flag Coverage Δ *Carryforward flag
differential 40.46% <40.00%> (?)
total 12.35% <41.93%> (+0.71%) ⬆️ Carriedforward from fca9ba0

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

Impacted Files Coverage Δ
...clude/pid_longitudinal_controller/debug_values.hpp 50.00% <ø> (ø)
...tudinal_controller/pid_longitudinal_controller.hpp 30.00% <ø> (+1.42%) ⬆️
...clude/trajectory_follower_node/controller_node.hpp 100.00% <ø> (ø)
...ectory_follower_node/test/test_controller_node.cpp 32.87% <28.57%> (+8.93%) ⬆️
...l/trajectory_follower_node/src/controller_node.cpp 44.09% <42.85%> (+6.00%) ⬆️
...nal_controller/src/pid_longitudinal_controller.cpp 45.55% <81.25%> (+6.50%) ⬆️
...alization/gyro_odometer/src/gyro_odometer_core.cpp 0.00% <0.00%> (-46.97%) ⬇️
localization/ekf_localizer/src/mahalanobis.cpp 60.00% <0.00%> (-20.00%) ⬇️
...calization/ekf_localizer/test/test_mahalanobis.cpp 44.44% <0.00%> (-5.56%) ⬇️
...vehicle_model/sim_model_ideal_steer_acc_geared.cpp 80.95% <0.00%> (-4.77%) ⬇️
... and 180 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.

@TakaHoribe TakaHoribe enabled auto-merge (squash) January 27, 2023 03:08
@TakaHoribe TakaHoribe disabled auto-merge January 27, 2023 03:09
@TakaHoribe TakaHoribe enabled auto-merge (squash) January 27, 2023 11:04
@TakaHoribe TakaHoribe disabled auto-merge January 27, 2023 11:18
@TakaHoribe TakaHoribe enabled auto-merge (squash) January 27, 2023 11:18
@TakaHoribe TakaHoribe merged commit 75ae084 into autowarefoundation:main Jan 27, 2023
@TakaHoribe TakaHoribe deleted the feature/no-integration-in-manual-mode branch January 27, 2023 11:48
maxime-clem pushed a commit to maxime-clem/autoware.universe that referenced this pull request Jan 30, 2023
…foundation#2619)

* feat(longitudinal_controller): skip integral in manual mode

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

* change control_mode to operation_mode

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

* fix test

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>
lexavtanke pushed a commit to lexavtanke/autoware.universe that referenced this pull request Jan 31, 2023
…foundation#2619)

* feat(longitudinal_controller): skip integral in manual mode

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

* change control_mode to operation_mode

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

* fix test

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>
Signed-off-by: Alexey Panferov <lexavtanke@gmail.com>
asana17 pushed a commit to asana17/autoware.universe that referenced this pull request Feb 8, 2023
…foundation#2619)

* feat(longitudinal_controller): skip integral in manual mode

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

* change control_mode to operation_mode

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

* fix test

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>

Signed-off-by: Takamasa Horibe <horibe.takamasa@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:control Vehicle control algorithms and mechanisms. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants