Open berkakinci opened 4 months ago
It was unclear to me if this got tested. I assumed not. So I started code review to see check for unintentional differences.
It looks to me like our use of driveFieldOriented()
and the earlier drive()
would have done the same thing... IF we were giving both functions equivalent inputs.
Unfortunately, in this case, you also changed the input to the functions, so they don't do the same thing.
The old:
swerve.drive(translation, desiredSpeeds.omegaRadiansPerSecond, true);
The new:
swerve.driveFieldOriented(desiredSpeeds);
The impact: Translation of the robot uses desiredSpeeds
instead of translation
. desiredSpeeds
is unmodified input from driver. This change drops the use of SwerveMath.limitVelocity
I noticed this among yesterday's changes:
+ swerve.driveFieldOriented(desiredSpeeds);
I think this is our first time using that. Please test this behaves as expected for both Blue and Red alliance. Preferably with AprilTags of the correct alliance. Unrelated: This is also an opportunity to test autonomous on both Alliance.