Return self reference from ChassisSpeeds modifiers

[mjansen4857]

This PR is basically identical to #7418, which allows chaining of the toFieldRelativeSpeeds, toRobotRelativeSpeeds, and discretize methods. I'm not exactly sure why that PR was closed, but I think the current implementation makes things a bit more verbose, especially when converting usage of the deprecated static methods to the new instance methods.

For example, a common usage of the static methods could look something like this:

swerve.driveRobotRelative(ChassisSpeeds.fromFieldRelativeSpeeds(
    someXValue,
    someYValue,
    someOmegaValue,
    robotRotation
));

The current implementation that returns void would require the above example to be converted like this:

ChassisSpeeds speeds = new ChassisSpeeds(someXValue, someYValue, someOmegaValue);
speeds.toRobotRelativeSpeeds(robotRotation);
swerve.driveRobotRelative(speeds);

Returning a self reference from these methods allows this conversion to be simplified back down to:

swerve.driveRobotRelative(new ChassisSpeeds(someXValue, someYValue, someOmegaValue)
    .toRobotRelativeSpeeds(robotRotation));

This also has the added benefit of allowing method chaining, but I'm mainly concerned about being able to directly pass the result of these methods to another method without having to create a variable and call a method as an intermediate step.

[mjansen4857]

Honestly I think a better solution here is probably to just make these methods return a new ChassisSpeeds object instead of modifying the original, or un-deprecate the static methods so the user can choose if they want a new object or not. There are a lot of situations where the user may not want the original object modified because it gets used again further down in some method. This happens a bunch in PPLib, for example. The current implementation makes working with those situations pretty rough, since you need to create a copy of the original speeds which is very verbose in java:

ChassisSpeeds prevRobotSpeeds = new ChassisSpeeds(
    prevState.fieldSpeeds.vxMetersPerSecond, 
    prevState.fieldSpeeds.vyMetersPerSecond, 
    prevState.fieldSpeeds.omegaRadiansPerSecond);
prevRobotSpeeds.toRobotRelativeSpeeds(prevState.pose.getRotation());

instead of just:

ChassisSpeeds prevRobotSpeeds = prevState.fieldSpeeds.toRobotRelativeSpeeds(prevState.pose.getRotation());

[KangarooKoala]

I'm not exactly sure why that PR was closed

Here's my understanding of the history:

• [wpimath] Add cosineScale and instance optimize to SwerveModuleState #7113 wanted to aggressively reduce GC allocations by adding a mutable version of Rotation2d and making ChassisSpeeds a mutable API.
• From discussion in that PR, it was decided that only making ChassisSpeeds a mutable API would be done. This was done in [wpimath] ChassisSpeeds fromRelative and discretize methods made instance methods #7115.
• The move from static factories to instance mutators was done purely to reduce Java GC overhead. (See [wpimath] ChassisSpeeds Discretize() and FromRelative() instance methods missing in C++ #7379)
• Allow for inlining of ChassisSpeeds transformation methods #7418 was closed after discussion in the FRC Discord #programming-discussion channel. The main point of the discussion is that mutable APIs in Java are confusing (because of what you've alluded to about the potential of method calls to unexpectedly modifying their arguments), and making the methods return a reference to the modified object makes things even worse because the general expectation is for chaining (common in functional programming) to be without side effects (which is how functional programming is).

I think a better solution here is probably to just make these methods return a new ChassisSpeeds object instead of modifying the original

That would be better than returning a self reference from the ChassisSpeeds modifiers. It comes at the expense of memory allocations, though, so it'd be up to a library maintainer (i.e., @calcmogul) whether the nicer API is worth the memory allocations.

un-deprecate the static methods so the user can choose if they want a new object or not

That's my personal preferred approach (especially since from shortly after the #7418 discussion the change will be reverted for 2027). However, it comes with the drawback of increased maintenance burden.

Thinking more about this, though, the static methods could be implemented in terms of the instance mutators (Just call the methods on a copy/new object), which would mean we would keep only one implementation of the logic. There would still be a lot more overloads, though.

[mjansen4857]

That's my personal preferred approach (especially since from shortly after the #7418 discussion the change will be reverted for 2027). However, it comes with the drawback of increased maintenance burden.

Thinking more about this, though, the static methods could be implemented in terms of the instance mutators (Just call the methods on a copy/new object), which would mean we would keep only one implementation of the logic. There would still be a lot more overloads, though.

Yeah I think removing the deprecation from the factory methods and implementing them to use the instance methods is the best solution. I can rework this PR to do that if a maintainer is on board.