Wojtaszek - Autonomous and other stuff #6
Conversation
…utton is pressed on the XBox controller
… Arcade Left and right, and tank mode.
My First Pull Request
Adding PID logic
| shooter.setDefaultCommand( | ||
| new ShooterCommand( | ||
| shooter, | ||
| () -> mainController.getLeftTriggerAxis(), | ||
| () -> mainController.getRightTriggerAxis())); | ||
|
|
||
| mainController.a().toggleOnTrue(new AutonomousDistance(drive)); |
There was a problem hiding this comment.
I'd add a comment saying that this binds the "A" button to execute the AutonomousDistance routine.
There was a problem hiding this comment.
I updated this to be part of the autonomous command logic.
| * | ||
| * @param drivetrain The drivetrain subsystem on which this command will run | ||
| */ | ||
| public AutonomousDistance(DriveSubsystemXrp drivetrain) { |
There was a problem hiding this comment.
I would change the "DriveSystemXrp" (the implementation) to "Drive" (the interface), so that this command could be re-used in any subsystem that implements the Drive interface. E.g. we could have the same command work on Romi, XRP, and even the actual comp bot.
There was a problem hiding this comment.
How would reccomend I do this? I would have to add a function to the Drive interface for getting the left and right positions. In the current code I do this by accessing that data through the DriveSubsystemXRP through the drivedifferentialIOXrp/DriveDifferentialIo class/interface but that is really specific to robots with a left and right wheels. Adding that logic to the Drive interface would limit it and all derived implementations to 2wd bots.
| import frc.robot.subsystems.drive.DriveSubsystemXrp; | ||
|
|
||
| public class DriveDistance extends Command { | ||
| private final DriveSubsystemXrp drive; |
There was a problem hiding this comment.
Same: Change DriveSubsystemXrp --> Drive
| speed = driveSpeed; | ||
| drive = driveSubsystem; | ||
|
|
||
| pidController = new PIDController(1.0, 0.0, 0.0); // Adjust PID constants as needed |
There was a problem hiding this comment.
To make the command re-usable on any robot, the PID Controller should be passed into the command, the constant could be moved to Constants.java, the constants could be passed in, etc.
| public void execute() { | ||
| double leftDistance = leftWheelStartPosition - drive.getLeftPositionMeters(); | ||
| double rightDistance = rightWheelStartPosition - drive.getRightPositionMeters(); | ||
| System.out.println("Left: " + leftDistance + " Right: " + rightDistance); |
There was a problem hiding this comment.
The "production" code shouldn't print to the console. Try to add AdvantageKit logging for these values. See https://docs.advantagekit.org/data-flow/recording-outputs/
There was a problem hiding this comment.
I removed it but will use advantage kit in the future
| public class DriveTime extends Command { | ||
| private final double duration; | ||
| private final double speed; | ||
| private final DriveSubsystemXrp drive; |
There was a problem hiding this comment.
DriveSubsystemXrp --> Drive
| // Create a DifferentialDriveKinematics object with the track width | ||
| kinematics = new DifferentialDriveKinematics(DriveConstants.trackWidthMeters); | ||
|
|
||
| addRequirements(drive); |
There was a problem hiding this comment.
If you change DriveSubsystemXrp --> Drive, then this needs to be cast to a Subsystem: addRequirements((Subsystem) drive);
There was a problem hiding this comment.
Does the DriveTime command work? I tried something similar using drive(1 m/s) + WaitCommand(5 sec) + drive(0 m/s), and the XRP only drove for a split second.
There was a problem hiding this comment.
No I plan on removing this. Right now I don't see a use for this class.
|
|
||
| // START: Setup Odometry | ||
| SmartDashboard.putData("Field", field); | ||
| // END: Setup Odometry |
There was a problem hiding this comment.
Did you mean to remove the "Field" from the smartdashboard?
I think that I have logic that should be in the subsystem in the command logic for the "DriveDistance". I will probably clean that up.