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

Deprecate PIDSubsystem and ProfiledPIDSubsystem for removal #5452

Closed
calcmogul opened this issue Jul 17, 2023 · 1 comment · May be fixed by #5352
Closed

Deprecate PIDSubsystem and ProfiledPIDSubsystem for removal #5452

calcmogul opened this issue Jul 17, 2023 · 1 comment · May be fixed by #5352
Labels
component: command-based WPILib Command Based Library type: chore Formatting, reorganizing, and other necessary (but less impactful) tasks.
Milestone

Comments

@calcmogul
Copy link
Member

Paraphrased from @Oblarg on the FRC Discord:

PIDSubsystem and ProfiledPIDSubsystem don't provide abstraction benefits, and they break the command-based paradigm. Here's an example:

https://github.com/wpilibsuite/allwpilib/tree/main/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/armbot

The motor output happens outside of commands, so it's not possible for the command scheduler alone to make sure that there's no resource contention.

These classes could also be considered safety hazards. If you write a command for the arm that tells the motor to go at a specific rate (e.g. for debugging) you have to manually disable the PID before using it.

The PIDSubsystem classes were basically just ports of the ones from the old command-based framework, whose asynchronous PID implementation forced there to be these sort of concurrency problems. Now that we do everything synchronously, we can and should do better.

Fluent command factories, on the other hand, automatically avoid any concurrency problems because the implicit mutexing just does the right thing. When you're in the happy path, a lot of failure states are rigorously impossible.

@calcmogul calcmogul added the component: command-based WPILib Command Based Library label Jul 17, 2023
@calcmogul calcmogul added this to the 2024 milestone Jul 17, 2023
@Starlight220
Copy link
Member

Duplicate of #5067.

@calcmogul calcmogul added the type: chore Formatting, reorganizing, and other necessary (but less impactful) tasks. label Aug 2, 2023
@calcmogul calcmogul closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: command-based WPILib Command Based Library type: chore Formatting, reorganizing, and other necessary (but less impactful) tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants