[cmd] Control command/subsystem deprecation

[AngleSideAngle]

See #5067 for motivation. This deprecates PIDCommand, ProfiledPIDCommand, PIDSubsystem, ProfiledPIDSubsystem, and TrapezoidProfileSubsystem. TrapezoidProfileCommand is kept because it manages a lot of variables (timer, profile, io), although it might be useful to incorporate trapezoid profiles into a future api for general trajectory following.

As for documentation, I have so far removed all the ReplaceMeX examples, since they reference deprecated classes and promote bad design practices. There still are many examples that should be edited to avoid using newly deprecated classes, most of which would probably benefit from inlining.

• [x] Deprecate control subsystems
• [x] Deprecate control commands
• [x] Remove ReplaceMe examples
• [ ] Update use-case examples

resolves #5452

[AngleSideAngle]

Another thing to note is that TimedCommand doesn't offer a full replacement to TrapezoidProfileCommand (At least, a hypothetical version that took in state suppliers. Currently you have to wrap it in a DeferredCommand, which doesn't exist in WPILib yet.)

eg.

public CommandBase profiledToState(double goal) {
  TrapezoidProfile profile;
  return runOnce(
          () ->
              profile =
                  new TrapezoidProfile(
                      CONSTRAINTS, new State(goal, 0), new State(getPosition(), getVelocity())))
      .andThen(Commands.runTimed(t -> doSomething(profile.calculate(t))));
}

I'm not sure how this would be fixed, or if it's an issue. Maybe it would be better to have a more generalized trajectory following command (related to an idea I had about rewriting the trajectory library).

[Starlight220]

That example wouldn't work because of reassignment and capture semantics.

[Starlight220]

I do think there's value in pre-built base classes or templates for integrating control loops into commands, ie a command class that has prebuilt factories for commands like going to a setpoint etc. The problem is naming: we can't just change the semantics of these classes without a deprecation cycle, so maybe templates and docs are the way to go?

[AngleSideAngle]

I think the largest benefit to having predefined factories for controller commands would be "encouraging" users to use features like that in a pragmatic way. However, most of these commands wouldn't actually reduce any boilerplate.

Also I just checked the profile command example. That's definitely an issue, since removing TrapezoidProfileCommand would definitely be a regression in that deferring would be the only way to follow a trapezoid profile evaluated at runtime. This extends more generally to initializing anything at runtime in a command, which currently can only be done via subclassing.

[Gold856]

Have we decided on whether or not we want to keep the control commands? If we are going to deprecate them, we should do it this year so we won't have to wait another year. We can always un-deprecate stuff as well. Personally, I'm okay with leaving the control commands in, since they aren't terrible, and because the alternative is letting teams implement it themselves, which, for such a common use case, is probably not worth it.

[AngleSideAngle]

I personally think, at the very least, the control subsystems should be removed in the future. There is more of a case for the existence of control commands, especially TrapezoidProfileCommand, although I think the latter would benefit from a generalized command for all time parameterized interpolating trajectories.

[calcmogul]

We tried replacing the controller commands with TrajectoryCommand in https://github.com/wpilibsuite/allwpilib/pull/3366, but we couldn't get CI to pass.

[Gold856]

Since we're getting close to kickoff, can this PR be rebased and changed to deprecate the control subsystems and remove the subclassing examples? Best to deprecate sooner rather than later.

[AngleSideAngle]

Yep, I've been kind of busy this week, but I should be able to get this and proxyAll finalized by tonight.

[AngleSideAngle]

@Starlight220 How do you feel about TimedCommand? I'm getting everything up to date and not sure if it's worth keeping or not. I could remove only the class and make it a FunctionalCommand in Commands, but its purpose is pretty difficult to justify since trapezoid profile commands are staying.

[AngleSideAngle]

/format

[Starlight220]

If the control commands are staying, I'm not sure it's needed. If this PR is focused on deprecating control subsystems, TimedCommand is out of scope.

[AngleSideAngle]

/format

[Starlight220]

The control subsystems need to go.

However, the control commands are relatively useful, and I'm not sure we should remove them. Thoughts?

[Gold856]

If we remove control commands, I'm concerned that less experienced teams will struggle to create commands that use PIDControllers or motion profiles. It may not reduce boilerplate by that much, but teams are forced to adhere to the constructor args, which means it's harder for them to mess up. The ease of use to teams is worth more than the potential diffculty they would have creating their own.

[AngleSideAngle]

I don't necessarily disagree with the existence of control commands, given the documentation uses them inline rather than subclassing. However,

new PIDCommand(new PIDController(...), sensor::get, setpoint::get, out -> motor.set(out), this); // the current PIDCommand behavior would have to be changed to implement `isFinished` with `controller.atSetpoint`

Doesn't seem much better than

var controller = new PIDController(...);
run(() -> motor.set(controller.calculate(sensor.get(), setpoint.get()))).until(controller::atSetpoint)

Especially in debugging, where teams can have a much better understanding of what their code does in the second example. I think the largest contributor to ease of use is just documenting it well, with could be accomplished if examples (and probably docs.wpilib.org) encourage teams to use the latter.

Regarding the constructor forcing teams to structure their commands in certain way: the added level of opacity makes understanding what the code is actually doing more difficult.

[Oblarg]

Re: control commands, please see:

https://www.chiefdelphi.com/t/command-based-help/446143/19?u=oblarg

When you write it out directly, it's clear that the PIDCommand abstraction does not save a single line of code (the controller is injected anyway). The only thing it abstracts out is a single calculate call - at the cost of making the representation completely opaque.

[tom131313]

proxyAll

I'm sorry I'm not fluent in github or allwpilib but maybe this belongs to you - I can't find it again so I don't know the URLs.

There are two very different implementations of proxyAll (same results though). One of them has a typo in the javadoc. * <p>This is useful to ensure that default commands of subsystems withing a command group are should be * <p>This is useful to ensure that default commands of subsystems within a command group are