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: update acceleration limit #1181

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brkay54
Copy link
Member

@brkay54 brkay54 commented Oct 1, 2024

Description

Following the recent discussions in the ODD Working Group, we've agreed to set the deceleration limit to 0.255G (~2.499 m/s²) for Dense Urban ODD. To implement this change, we need to update the acceleration limit parameters on both the planning and control sides.

For more details, refer to the ODD WG Discussion page: ODD WG Discussion #5290

image

Tests performed

I tested Dense Urban ODD scenarios in the local.

Not applicable.

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.

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.

Signed-off-by: Berkay Karaman <[email protected]>
Copy link
Contributor

@yuki-takagi-66 yuki-takagi-66 left a comment

Choose a reason for hiding this comment

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

I feel that this PR is too specific to Dense Urban ODD. Can I ask you to reconsider?

@@ -11,7 +11,7 @@

# constraints to be observed
limit:
min_acc: -2.5 # min deceleration limit [m/ss]
min_acc: -2.49 # min deceleration limit [m/ss]
Copy link
Contributor

Choose a reason for hiding this comment

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

This change may be only for Dense Urban ODD.

@@ -53,7 +53,7 @@

# stopped state
stopped_vel: 0.0
stopped_acc: -3.4 # denotes pedal position
stopped_acc: -2.49 # denotes pedal position
Copy link
Contributor

Choose a reason for hiding this comment

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

stopped_acc does not denote deceleration value, but denotes pedal position after the stop has achieved.
Hence this change is not required.

max_acc: 3.0
min_acc: -5.0
max_acc: 1.5
min_acc: -2.49
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to follow the trajectory planned with limit.min_acc and limit.max_acc in the planning params, this values must be greater than planning params. Because we need a margin for disturbance and error.
However, I think we have an option to align these values to the limit.min_acc and limit.max_acc in common.pram.yaml to restrict the feedback control.

@xmfcx
Copy link
Contributor

xmfcx commented Nov 26, 2024

@Igata-ctrl @mitsudome-r what should be done about this PR?

@brkay54 won't be able to work on this task anymore. But how should we handle scenarios and parameters like these?

Are there other scenarios that the public cannot access? I think the autoware_launch default parameters should be tuned for scenarios that are accessible by the public.

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

Successfully merging this pull request may close these issues.

3 participants