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

fix(pid_longitudinal_controller): revive hysteresis of state transition #1219

Conversation

takayuki5168
Copy link
Contributor

@takayuki5168 takayuki5168 commented Nov 8, 2024

Description

Previously, there was a hysteresis between drive_state_stop_dist and stopping_state_stop_dist so that the state transition of DRIVE <--> STOPPED will not chatter.

However, the following commit removed the hysteresis by mistake, so this PR revived it.
c6a3df8#diff-a311c12a65b491b5b4c0e9e5ee83bc1488a360826fe46e4204355c9fe009127eL14

Tests performed

unit test

Effects on system behavior

No chattering of the DRIVE/STOPPED state transition in pid_longitudinal_controller

Interface changes

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.

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

@kosuke55
Copy link
Contributor

better to comment why 0.49

@takayuki5168
Copy link
Contributor Author

@kosuke55
Let me know if the description is unclear. I added a hysteresis 0.1 to stopping_state_stop_dist.

@takayuki5168 takayuki5168 merged commit 40f04ee into autowarefoundation:main Nov 11, 2024
17 checks passed
@takayuki5168 takayuki5168 deleted the fix/inconsistent-pid-state-transition branch November 11, 2024 01:40
@kosuke55
Copy link
Contributor

I meant comment in param file.
but it is ok because I can find this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants