Wojtaszek - Autonomous and other stuff by wojtase1 · Pull Request #6 · DevilBotz2876/XrpReference2024

wojtase1 · 2024-12-08T15:54:02Z

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.

…utton is pressed on the XBox controller

… Arcade Left and right, and tank mode.

My First Pull Request

Adding PID logic

BrownGenius

Logic looks good! I would make some minor changes to enhance reusability, but functionally ok as-is.

At the least, add a comment for the "A" button and confirm you means to remove the field from the SmartDashboard.

BrownGenius · 2024-12-08T16:11:57Z

src/main/java/frc/robot/RobotContainer.java

            () -> mainController.getLeftTriggerAxis(),
            () -> mainController.getRightTriggerAxis()));
+
+    mainController.a().toggleOnTrue(new AutonomousDistance(drive));


I'd add a comment saying that this binds the "A" button to execute the AutonomousDistance routine.

I updated this to be part of the autonomous command logic.

BrownGenius · 2024-12-08T16:14:46Z

src/main/java/frc/robot/commands/AutonomousDistance.java

+   *
+   * @param drivetrain The drivetrain subsystem on which this command will run
+   */
+  public AutonomousDistance(DriveSubsystemXrp drivetrain) {


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.

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.

BrownGenius · 2024-12-08T16:15:16Z

src/main/java/frc/robot/commands/DriveDistance.java

+import frc.robot.subsystems.drive.DriveSubsystemXrp;
+
+public class DriveDistance extends Command {
+  private final DriveSubsystemXrp drive;


Same: Change DriveSubsystemXrp --> Drive

BrownGenius · 2024-12-08T16:17:12Z

src/main/java/frc/robot/commands/DriveDistance.java

+    speed = driveSpeed;
+    drive = driveSubsystem;
+
+    pidController = new PIDController(1.0, 0.0, 0.0); // Adjust PID constants as needed


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.

BrownGenius · 2024-12-08T16:18:48Z

src/main/java/frc/robot/commands/DriveDistance.java

+  public void execute() {
+    double leftDistance = leftWheelStartPosition - drive.getLeftPositionMeters();
+    double rightDistance = rightWheelStartPosition - drive.getRightPositionMeters();
+    System.out.println("Left: " + leftDistance + " Right: " + rightDistance);


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/

I removed it but will use advantage kit in the future

BrownGenius · 2024-12-08T16:19:10Z

src/main/java/frc/robot/commands/DriveTime.java

+public class DriveTime extends Command {
+  private final double duration;
+  private final double speed;
+  private final DriveSubsystemXrp drive;


DriveSubsystemXrp --> Drive

BrownGenius · 2024-12-08T16:21:12Z

src/main/java/frc/robot/commands/DriveDistance.java

+    // Create a DifferentialDriveKinematics object with the track width
+    kinematics = new DifferentialDriveKinematics(DriveConstants.trackWidthMeters);
+
+    addRequirements(drive);


If you change DriveSubsystemXrp --> Drive, then this needs to be cast to a Subsystem: addRequirements((Subsystem) drive);

BrownGenius · 2024-12-08T16:22:50Z

src/main/java/frc/robot/commands/DriveTime.java

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.

No I plan on removing this. Right now I don't see a use for this class.

BrownGenius · 2024-12-08T16:25:08Z

src/main/java/frc/robot/subsystems/drive/DriveSubsystemXrp.java

-
-    // START: Setup Odometry
-    SmartDashboard.putData("Field", field);
-    // END: Setup Odometry


Did you mean to remove the "Field" from the smartdashboard?

No. I put it back.

wojtase1 added 14 commits November 15, 2024 15:58

Added capability to switch how the robot is controlled when the 'A' b…

95109b7

…utton is pressed on the XBox controller

Adding documentation to RobotContainer.java and DriveSybsystemXrp.java

f569bac

Adding logic to toggle between three driving modes, Arcade Left only,…

accb41a

… Arcade Left and right, and tank mode.

Putting gradle back to default settings

47a2aa4

cleaning up code in robotContainer.java

650140b

documenation cleanup

73e9280

update comments

d411605

added autonomous drive logic

744df25

Merge pull request #4 from wojtase1/main

d911a8a

My First Pull Request

Added PID logic to DriveDistance.java

96c828f

Adding logic to drive using the wheel encoders.

f86d925

Merge pull request #5 from wojtase1/main

1de758c

Adding PID logic

updated logic for tank drive to use chassisppeed

19329e6

Cleaned up logic.

9fc33d0

wojtase1 requested a review from BrownGenius December 8, 2024 15:54

BrownGenius requested changes Dec 8, 2024

View reviewed changes

wojtase1 added 4 commits December 8, 2024 15:01

updating per pull request comments.

4397cc0

Putting field back into DriveSubSystemXRP.java

2d1e03b

adding more autonous stuff

9df19e5

adding logic to get both servos to move at the same time:

3159d50

Conversation

wojtase1 commented Dec 8, 2024

Uh oh!

BrownGenius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants