-
Notifications
You must be signed in to change notification settings - Fork 38
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 lowpass & refactor filter structure #119
Conversation
b4a108b
to
b0c5134
Compare
The amount of comments I made may suggest otherwise, but I really like this change. You added a nice abstraction, cleaned up Nice work! |
d979f0c
to
0749297
Compare
b27ab71
to
dfb3f49
Compare
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.
My comments are addressed. And PR is backwards compatible with P and PI controllers.
I leave the other approvals to their respective commentors 🙂
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.
My comments were already addressed
- Fixed second order lowpass - Made the crossover frequency and damping tuneble - Use the filtered signal for PID calculations - Add a derivative filter
How the lowpass filter was derived: https://github.com/nobleo/path_tracking_pid/blob/fix/lowpass/doc/second_order_lowpass_tustin.ipynb
PidDebug output with lowpass crossover frequency at 20, 4 and 1 Hz.