[wpilib] Replace MecanumDriveMotorVoltages with a functional interface

[WispySparks]

Resolves #5929

[github-actions[bot]]

This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR.

[WispySparks]

This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR.

There doesn't appear to be a corresponding MecanumControllerCommand in python. Also the failing test is unrelated, something flaky.

[KangarooKoala]

Looks pretty good! I'm just wondering, though, about whether we want to remove it without deprecating first since it is a user-facing API element. It could be safe to remove the protobuf definition (since there isn't a WPILib implementation yet so there's likely not many teams using it), but I'm concerned about completely removing the class. (Especially since there's likely multiple examples floating around somewhere that would need time to be updated)

[WispySparks]

So should I keep everything using the new functional interface but just not delete MecanumDriveMotorVoltages and instead deprecate it?

[KangarooKoala]

So should I keep everything using the new functional interface but just not delete MecanumDriveMotorVoltages and instead deprecate it?

The point of deprecating instead of completely removing is to give users time to migrate, so user-facing overloads using MecanumDriveMotorVoltages should not be deleted and instead deprecated, with overloads using the new functional interface being added.

Practically speaking, what this would look like relative to what you already have is adding new overloads with identical signatures to the old ones that use the other overloads. (For example, add the following constructor)

@Deprecated
public MecanumControllerCommand(
      Trajectory trajectory,
      Supplier<Pose2d> pose,
      SimpleMotorFeedforward feedforward,
      MecanumDriveKinematics kinematics,
      PIDController xController,
      PIDController yController,
      ProfiledPIDController thetaController,
      double maxWheelVelocityMetersPerSecond,
      PIDController frontLeftController,
      PIDController rearLeftController,
      PIDController frontRightController,
      PIDController rearRightController,
      Supplier<MecanumDriveWheelSpeeds> currentWheelSpeeds,
      MecanumVoltagesConsumer outputDriveVoltages,
      Subsystem... requirements) {
  this(trajectory,
       pose,
       feedforward,
       kinematics,
       xController,
       yController,
       thetaController,
       maxWheelVelocityMetersPerSecond,
       frontLeftController,
       rearLeftController,
       frontRightController,
       rearRightController,
       currentWheelSpeeds,
       (frontLeftVoltage, frontRightVoltage, rearLeftVoltage, rearRightVoltage) ->
        outputDriveVoltages.accept(
          new MecanumDriveWheelVoltages(frontLeftVoltage, frontRightVoltage, rearLeftVoltage, rearRightVoltage));
       requirements);
}

[WispySparks]

I've added back in the old constructors as overloads and MecanumDriveMotorVoltages and deprecated them instead.

[sciencewhiz]

Hopefully someone else can answer if the since argument should be 2024 or 2025

It should be 2025, since 2025 will be the first season it's deprecated. It's then eligible to be removed before the 2026 season.

[WispySparks]

Both a struct and proto impl were made for this class on the Java side of things in this pr https://github.com/wpilibsuite/allwpilib/pull/5953, should they be removed in my pr?

[PeterJohnson]

Yes, since they’ve already been added, they should be removed from this PR.

[KangarooKoala]

@PeterJohnson Anything blocking this from being merged? I'd like for the deprecation (and undoing protobuf and struct support) to happen before this season.

Locally, I was able to merge f377a9c573ea90f9d50cf85880a150f7036ae644 into this branch and successfully build and test via cmake.

[KangarooKoala]

To fix the pregenerated files, download the artifacts from the pregenerated files CI run at https://github.com/wpilibsuite/allwpilib/actions/runs/12168953220/artifacts/2276116716, extract the downloaded zip file, and then run git apply <path to pregenerated-files-patches.patch> from your clone of allwpilib.

[WispySparks]

I can't just run pregen?

[KangarooKoala]

Right, forgot about that. You should be able to add a comment to the PR that starts with /pregen, and locally you can also run ./.github/workflows/pregen_all.py.