fix(autoware_pid_longitudinal_controller): correct the PID controller

[HansOersted]

Description

The previous PID controller in Autoware is actually equivalent to a PD/PI controller.
This PR corrects the utilization of and upgrades to the PID controller.

N.B. Without specific reasons, the parameters adopted in the previous PD/PI controller are adopted in another PR. And the parameters in that PR are equivalent to the PD/PI controller.

Before change:
Screencast from 07-19-2024 02_05_57 PM.webm

After change:
Screencast from 07-19-2024 02_37_49 PM.webm

---

Note that one of the benefits of using implementing PID controllers is being robust to the external noise (e.g., driving on an ice road). Therefore, a noise was introduced artificially to test the PD/PI controller (before) and PID controller (after).

Noise definition: a constant acceleration (+1.0 m/s2), that is return feedback_acc + 1.0;.
Also, note the PID output constraint has been relaxted to max_out: 2.0, min_out: -2.0 in the analysis.

Before change:
Screencast from 07-19-2024 02_15_43 PM.webm

After change (with new parameters below)

kp: 1.5 # kp > 0 (stability proof) for third-order
ki: 0.8 # ki > 0 && Ki < kp * kd (stability proof) for third-order
kd: 2.5 # kd > 0 (stability proof) for third-order

Screencast.from.07-19-2024.04_26_14.PM.webm

N.B. In the parameter PR, we picked the parameters equivalent to the PI/PD controller.
It means that it will also show limit robustness to the external noise.

---

Conclusions:

1. The previous PI/PD controller shows the logical error , which is fixed in this PR.
2. The updated PID controller (this PR) works as expected in the normal case.
3. The previous PI/PD controller can fail to track the reference once the external disturbance in introduced.
4. The previous PI/PD controller successfully tracks the reference for some introduced external disturbances.

---

Further steps:

1. The user may change the coefficients of the PID controller based on the concentrations and missions.
2. The max P, I, and D constraints can be adjusted.
3. The coefficients of the PID controller for a robust tracking performances can be analyzed.

---

Discussions:

1. Honestly, erasing the integration when and only when the car is controlled manually is personally preferred. If latecomers have interests on this topic, do not hesitate to contact me.
2. If you are trying to implement Discussion 1, consider using reset().

Tests performed

Effects on system behavior

Not applicable.

Interface changes

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.

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.

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

[github-actions]

Thank you for contributing to the Autoware project!

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

Please ensure:

• You've checked our contribution guidelines.
• Your PR follows our pull request guidelines.
• All required CI checks pass before marking the PR ready for review.

[kyoichi-sugahara]

autoware.universe/control/autoware_pid_longitudinal_controller/include/autoware/pid_longitudinal_controller/pid.hpp

Line 33 in c0f07c2

* @param [in] is_integrated if true, will use the integral component for calculation

doxygen comment also should be changed

[codecov]

Codecov Report

Attention: Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.

Project coverage is 29.25%. Comparing base (7fe1e36) to head (0555ea5).
Report is 1229 commits behind head on main.

Files with missing lines	Patch %	Lines
...ware_pid_longitudinal_controller/test/test_pid.cpp	12.50%	0 Missing and 7 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8118   +/-   ##
=======================================
  Coverage   29.25%   29.25%           
=======================================
  Files        1600     1600           
  Lines      117736   117751   +15     
  Branches    50713    50707    -6     
=======================================
+ Hits        34438    34447    +9     
+ Misses      74098    74064   -34     
- Partials     9200     9240   +40     

Flag	Coverage Δ		*Carryforward flag
differential	17.79% <63.15%> (?)
total	29.25% <ø> (+<0.01%)	⬆️	Carriedforward from ceab242

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

[HansOersted]

autoware.universe/control/autoware_pid_longitudinal_controller/include/autoware/pid_longitudinal_controller/pid.hpp

Line 33 in c0f07c2

* @param [in] is_integrated if true, will use the integral component for calculation

doxygen comment also should be changed

fixed in 55f0787

[takayuki5168]

Any reason to use the negative enable_integration here?
Using enable_integration consistently is easier to understand the code.

[HansOersted]

fixed in 0555ea5

[stale]

This pull request has been automatically marked as stale because it has not had recent activity.

[mitsudome-r]

@HansOersted @takayuki5168 Do you still need this modification?

[takayuki5168]

It will take time to confirm this PR including the real vehicle experiment and we don't have a plan for that, so let me close it for now.