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

[wpimath] PIDController field and method names for error and errorDerivative #7088

Merged
merged 19 commits into from
Sep 23, 2024

Conversation

narmstro2020
Copy link
Contributor

I replaced positionError with error and velocityError with errorDerivative across the fields and methods.

I also added a few items to the initSendable for tuning purposes.

@narmstro2020 narmstro2020 requested a review from a team as a code owner September 17, 2024 21:23
@narmstro2020 narmstro2020 changed the title [wpimath] PIDController names [wpimath] PIDController field and method names for error and errorDerivate Sep 17, 2024
@narmstro2020 narmstro2020 changed the title [wpimath] PIDController field and method names for error and errorDerivate [wpimath] PIDController field and method names for error and errorDerivative Sep 17, 2024
Copy link
Member

@calcmogul calcmogul left a comment

Choose a reason for hiding this comment

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

Looks good, except the old PIDController error methods should be deprecated instead of immediately removed.

@narmstro2020
Copy link
Contributor Author

narmstro2020 commented Sep 17, 2024

Looks good, except the old PIDController error methods should be deprecated instead of immediately removed.

Will do. I need to finish up the C++ as well. I’ll challenge myself next time and do the C++ first.

Quick general question. Is a PR for RobotPy required only for commands or for every package. I have no problem making them. I just wanted to make sure not to break anything

@calcmogul
Copy link
Member

RobotPy PRs are only needed for pure Python ports, like commands. The rest is autowrapped.

@sciencewhiz
Copy link
Contributor

Is a PR for RobotPy required only for commands or for every package. I have no problem making them. I just wanted to make sure not to break anything

Robotpy is primarily done by wrapping C++. Only commands is done by writing pure Python, which is why there needs to be Python changes for changes in commands.

@narmstro2020 narmstro2020 requested a review from a team as a code owner September 17, 2024 23:02
calcmogul
calcmogul previously approved these changes Sep 18, 2024
@narmstro2020
Copy link
Contributor Author

/format

@PeterJohnson PeterJohnson merged commit 6281ec0 into wpilibsuite:main Sep 23, 2024
33 checks passed
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.

5 participants