-
Notifications
You must be signed in to change notification settings - Fork 1
67 feature implement pure pursuit controller #132
Conversation
…t-controller' into 67-feature-implement-pure-pursuit-controller
Also incorporated feedback from #126.
…ould be. Positions are only updated when there was a significant change. Started filtering GPS signals.
…d to a different issue)
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.
Nice new functionality for the action portion of the agent.
Unfortunately, due to an issue with the docker container. I couldn't test the controller so far. There are still quite some todos so the PR is blocked for now.
If you have any questions please let me know!
One last note:
The localization you implemented will probably need at different corners of the project, so a refactor to a more centralized spot might be a good idea in the future.
Review time: 1,5 hours
Simulation results
|
I'm not sure how changing the md document managed to break the build? |
For some reason, the leaderboard evaluation failed. Maybe just rerunning the tests should suffice otherwise @nylser might be of help. |
|
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.
Good job, all of the suggested changes have been incorporated. However, for now the ego vehicle drives to the right side of the road and hit a parking clock. I guess that is related to the GPS problem?
…ctory. Added one more publisher for debugging.
Simulation results
|
As suggested I have set the GPS noise to 0 for the time being. |
Fixed a few errors, tested some more parameters, and added a PID controller for the steering angle. ~ 6h by @schwalga |
Simulation results
|
Simulation results
|
* feat(#131): Added dedicated node for the publishing of the current position * feat(#131): Moved the publishing from DummySub to PositionPublisher * feat(#131): Added dedicated node for the publishing of the current position * feat(#131): Move PositionPublisher to perception * feat(#131): Added publishing of heading * feat(#131): Install and launch robot-pose-ekf * feat(#131): Fix install robot-pose-ekf * feat(#132): Started adding translation node for the EKF * fix(#131): Got EKF to work. Values for covariance need tweaking * feat(#131): Move installation in dockerfile * feat(#131): Adapt PositionPublisher to use filtered values * fix(#131): Added an average filter to the gps signal to improve the ekf * fix(#131): minor fix to rerun CI * fix(#131): fixed imu orientation, fixed gps quaternion * feat(#131): Delete unnecessary comment Co-authored-by: schwalga <[email protected]> Co-authored-by: Julian-Graf <[email protected]> Co-authored-by: Julian Graf <[email protected]>
During testing with a more complex trajectory, a few errors appeared, which have now been fixed. I also added a custom message that useful for #151 ~ 4h by @schwalga |
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.
Nice PR, the pure pursuit controller seems to work nicely. One last question before I approve the PR. Is is intended to cut the corner at the left turn and drive in the middle of the two lanes afterwards?
Review time: 30 min
Also could you please resolve the merge conflicts? |
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.
Looks good. Happily approve! Still some conflicts though
# Conflicts: # code/acting/launch/acting.launch
Simulation results
|
Description
This PR implements the pure pursuit controller. The first tests suggest, that it is working as intended, however due to problems with the GPS signal the car is not able to hold course at the moment.
While i tried fixing the problem at first under this issue, after a few hours of trial and error I realise, that this warrants a seperate issue. The documentation added in this PR gives a first overview over the problem it will be amended with the final filter in #131. For now I have left the simple filter added to
DummyTrajectorySub.py
so that the controller can be tested by a reviewer.Fixes #67
Time invested
Type of change
Does this PR introduce a breaking change?
Most important changes
The most important file is
pure_pursuit_controller.py
Checklist: