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

feature: optional argument to manually set dt #10

Merged
merged 1 commit into from
Oct 8, 2019
Merged

feature: optional argument to manually set dt #10

merged 1 commit into from
Oct 8, 2019

Conversation

Yasu31
Copy link
Contributor

@Yasu31 Yasu31 commented Oct 3, 2019

When using PID controllers in simulation, the timestep used for PID calculation should be set to simulation time instead of how long it actually takes between calls to PID().
This PR addresses it by adding a new optional argument that lets the user designate dt themselves.

Copy link
Owner

@m-lundberg m-lundberg left a comment

Choose a reason for hiding this comment

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

Hello,

Thank you for taking the time to create this PR, I really appreciate it. This seems like a good feature to support.

When the small typo I found is fixed, could you rebase your commits into one single commit? Let me know if you don't know what I mean by this and I will explain. When you are done I will merge your changes.

@Yasu31
Copy link
Contributor Author

Yasu31 commented Oct 6, 2019

Hi, I committed the fixes and squashed them into a single commit.
Thanks for maintaining this library, it's proving to be very useful for my thesis project!

Copy link
Owner

@m-lundberg m-lundberg left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@m-lundberg m-lundberg merged commit 5693260 into m-lundberg:master Oct 8, 2019
@m-lundberg
Copy link
Owner

This feature is now included in version 0.2.4 of the library

@Yasu31
Copy link
Contributor Author

Yasu31 commented Oct 9, 2019

Thank you!

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.

2 participants