Cleanup drive subsystem #90
Conversation
| import edu.wpi.first.wpilibj2.command.InstantCommand; | ||
| import frc.robot.util.AllianceUtil; | ||
|
|
||
| public class DriveCommands { |
There was a problem hiding this comment.
I see what you are going for here. Any thought to just adding these to the DriveSubsystem as non-static, and then they wouldnt need you to pass it in?
It theoretically couples the DriveSubsystem implementation to be specific to this year, but i'm not entirely sure that's a bad thing?
Also maybe it should just be a turn command where you pass in the angle (and then instantiating the command is the part that is coupled to this years implementation, in RobotContainer)
There was a problem hiding this comment.
I wanted to keep that out of the subsystem because I wanted the subsystem to have zero year-specific implementation. I think the best solution other than this would be a turn command producer in the subsystem and pulling all of the angles out to constants if we don't want to go for the DriveCommands class.
The other consideration is should there be a turn to angle command that gets triggered by a button if we move to the heading drive idea?
There was a problem hiding this comment.
I would argue that no commands to an evergreen subsystem like the drive should have year-specific implementation. Instead it should be paramaterized to inputs.
| e.printStackTrace(); | ||
| throw new Error("Swerve drive configuration could not be loaded from " + Filesystem.getDeployDirectory() + "/swerve", e); |
There was a problem hiding this comment.
any particular reason you decided to go with Error here instead of rethrowing the exception to not be caught (or just not catching the exception)? Just curious, I had to look up the java Error class myself
There was a problem hiding this comment.
Unless I am mistaken you must catch an exception somewhere or the code won't compile.
At least the IDE complains if you never catch the exception. However you can't catch an error, it just ends the program.
There was a problem hiding this comment.
Error might be appropriate but check out this for more info on what I mean https://stackoverflow.com/questions/4519557/is-there-a-way-to-throw-an-exception-without-adding-the-throws-declaration
See other PR for new vision integration strategy
| dashboard.addString("Current Command", this::getCommandName); | ||
|
|
||
| dashboard.add("Rotation PID", rotationController); | ||
| dashboard.addDouble("Yaw", () ->swerve.getYaw().getDegrees()); | ||
| dashboard.addDouble("Target angle", () -> targetAngle == null ? -1 : targetAngle); | ||
| dashboard.addDouble("Target angle", () -> rotationTarget == null ? -1 : rotationTarget.getDegrees()); |
There was a problem hiding this comment.
Different problem now. We should not rely on -1 being no target. I think if there is no target we should just log this as the current angle, we can compare with the Boolean as well to know when something is actually a target.
|
I doubt this is in YAGSL by default but we should write a command that can put the subsystem into "X-lock" mode (move pods to an X and ignore all other input) and one that resets it back to normal |
No description provided.