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 lowpass & refactor filter structure #119

Merged
merged 5 commits into from
Mar 14, 2022
Merged

Fix lowpass & refactor filter structure #119

merged 5 commits into from
Mar 14, 2022

Conversation

Rayman
Copy link
Contributor

@Rayman Rayman commented Mar 2, 2022

  • 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.
lowpass_20hz
lowpass_4hz
lowpass_1hz

cfg/Pid.cfg Show resolved Hide resolved
@Rayman Rayman force-pushed the fix/lowpass branch 2 times, most recently from b4a108b to b0c5134 Compare March 3, 2022 13:00
@lewie-donckers
Copy link
Contributor

The amount of comments I made may suggest otherwise, but I really like this change. You added a nice abstraction, cleaned up Controller and added unit tests.

Nice work!

src/controller.cpp Outdated Show resolved Hide resolved
@Rayman Rayman force-pushed the fix/lowpass branch 3 times, most recently from d979f0c to 0749297 Compare March 4, 2022 15:27
@Rayman Rayman force-pushed the fix/lowpass branch 2 times, most recently from b27ab71 to dfb3f49 Compare March 7, 2022 10:20
Copy link
Member

@Timple Timple left a 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 🙂

Copy link

@cesar-lopez-mar cesar-lopez-mar left a 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

Rayman added 5 commits March 14, 2022 15:03
- Fixed second order lowpass
- Made the crossover frequency and damping tuneble
- Use the filtered signal for PID calculations
- Add a derivative filter
@Rayman Rayman merged commit 3ada333 into main Mar 14, 2022
@Rayman Rayman deleted the fix/lowpass branch March 14, 2022 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants