Deprecate PIDCommand and PIDSubsystem

[Oblarg]

PIDCommand is an overwrought abstraction that tries very hard to maintain a roughly pre-2020 API shape while being implemented in the new Command-based framework. It's prone to a number of surprising and hard-to-diagnose initialization/state errors due to the reliance on subclassing as an API interaction, which is a pattern we've mostly moved away from elsewhere. The functional side of the PIDCommand API is not very good either, consisting of a rigid and difficult-to-use many-argument constructor. It is easier and better pedagogy to focus on teaching users to compose controllers within commands themselves by writing their own classes or using capture semantics and inline command definition.

PIDSubsystem is an underwrought abstraction that does very little work for the user while circumventing library features to do so. Notably, the architecture of PIDSubsystem runs motor outputs directly through the subystem periodic() method, which bypasses the subsystem requirement mutex system. This is bad practice, especially given how easy it is to write code that doesn't do this with the subsystem command factory pattern.

Both classes should be deprecated and the examples should be changed to show users how to do the relevant composition themselves.

[Starlight220]

I agree about PIDSubsystem, though I suspect that many teams use it.

As for PIDCommand, I think it's fairly good for use cases that don't need a dynamic feedforward. It doesn't rely on inheritance, and the constructor doesn't have that many arguments (it has 5, including requirements). If errors due to subclassing are common, we should mark the lifecycle methods as final, as we did for command groups.

What about the other control-loop subsystems/commands? RamseteCommand has its problems (#3350), but afaik we want to move it in the direction of TrapezoidProfileCommand (which is very helpful).

Maybe a first tiny-possibly-meaningless step would be flipping the order of their docs pages, presenting the control-loop commands first and then the counterpart subsystems?

[Oblarg]

Marking the lifecycle methods of PIDCommand final would be an improvement; i'm still not sure it's worth keeping around but i'll defer to people closer to the library on that.

I agree with the first-step of switching the order in the docs.

[legoguy1000]

We used PID subsystems last year however this year we're just using regular subsystems and building the pod controllers manually. The PID subsystem tries to be helpful by abstracting stuff but honestly looking back, I think it just made it harder to understand exactly how to implement it which is why we're just doing it manually this year.

[AngleSideAngle]

Bringing this back, PIDSubsystem's counterparts: ProfiledPIDSubsystem and TrapezoidProfileSubsystem also go against good practices with subsystem periodic, since they encourage users to set actuators without commands.

From TrapezoidProfileSubsystem:

  @Override
  public void periodic() {
    var profile = new TrapezoidProfile(m_constraints, m_goal, m_state);
    m_state = profile.calculate(m_period);
    if (m_enabled) {
      useState(m_state);
    }
  }

As for Command classes, I suggested this to improve support for use at runtime: #5302

[calcmogul]

Copy-paste from #5452:

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:

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.