-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat: Add reverse mode in pure pursuit #1194
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
base: master
Are you sure you want to change the base?
Conversation
@AtsushiSakai some error not caused by this PR ocurred |
@yangtzech Thank you for your PR. Could you please rebase on the master branch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds reverse mode support to the pure pursuit algorithm and enhances vehicle visualization.
- Adds a new unit test for reverse mode (test_backward)
- Updates the State class and pure pursuit steering control to account for reverse movement
- Introduces an improved vehicle plotting function with a reverse mode indicator
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/test_pure_pursuit.py | Adds a new test case to validate reverse driving mode |
PathTracking/pure_pursuit/pure_pursuit.py | Updates algorithm logic for reverse mode and enhances vehicle visualization |
Comments suppressed due to low confidence (2)
PathTracking/pure_pursuit/pure_pursuit.py:34
- [nitpick] Global simulation control variables are declared at the module level; consider centralizing these related settings in a dedicated configuration block to improve maintainability.
show_animation = True // pause_simulation = False // is_reverse_mode = False
PathTracking/pure_pursuit/pure_pursuit.py:183
- The numpy module is referenced via 'np' in plot_vehicle but there is no corresponding import (e.g., 'import numpy as np') in this module.
wheel = np.array([
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yangtzech Thank you for the PR, and sorry for the delayed review.
I’ve added a few comments, so please take a look. Also, it would be great if you could add some diagrams or equations explaining the simulation to this document (though it’s not mandatory).
https://atsushisakai.github.io/PythonRobotics/modules/6_path_tracking/pure_pursuit_tracking/pure_pursuit_tracking.html
""" | ||
# Adjust heading angle in reverse mode | ||
if is_reverse: | ||
yaw = angle_mod(yaw + math.pi) # Rotate heading by 180 degrees |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it necessary to rotate the heading by 180 degrees? I thought the heading would stay the same even when moving backward, and that it wouldn’t affect the rendering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's because the velocity is always defined as a scalor, therefore there has to be a variant to reprent the real movement direction of the vehicle.
self.rear_x = self.x - ((WB / 2) * math.cos(self.yaw)) | ||
self.rear_y = self.y - ((WB / 2) * math.sin(self.yaw)) | ||
self.direction = -1 if is_reverse else 1 # Direction based on reverse flag | ||
self.rear_x = self.x - self.direction * ((WB / 2) * math.cos(self.yaw)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the rear_x, rear_y for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It stands for the rear axle position (rear_x or rear_y) of a vehicle, which most vehicles use to steer. It was already added in the original code, and is modified to match the yaw angle change in reverse mode.
rebased, and sorry for the late reply, just got back from a business trip |
@yangtzech Rebase is failed??? It looks like the change list of this PR has unrelated changes... |
…re pursuit control
done cleaning commit history |
Reference issue
What does this implement/fix?
Additional information
CheckList