Added autonomous driving code to hoppers using ping sensors #18

ShreyaRavi · 2017-02-01T15:35:46Z

Just realized I never made a pull request for this. Added ping sensors to drive to the hopper.

@rothso

…ut need to add ability to add constant to the SynchronousPID @rothso.

…ce dummy values with real values and test. Fingers crossed.

… - can tune later.

…le running command or while in teleop.

…ngleHoldController. Works when not right up against ping sensor, not sure why very small readings make it go in the opposite direction. Testing needed.

rothso

Preliminary review!

I've kept in mind that this is still very much a rough draft and so your names, organization, and structure are likely temporary. I've geared my comments to be not so much about polishing a product but rather reorganizing what you currently have to reduce code duplication, standardize vocabulary, and remove dead code to repay the techinical debt of moving fast and breaking things before the very act comes to ironically slow us down.

With respect to the autonomous routine, is it safe to transition from Y to X without braking in between? I also believe we're currently running at a slow speed. Are things ready to be tested and tuned on the mock field?

rothso · 2017-02-11T18:18:48Z

src/main/java/org/teamresistance/frc/Feedback.java

+  public final double yDist;

-  Feedback(double currentAngle) {
+  Feedback(double currentAngle, double xDist, double yDist) {


If we want more than two ping sensors on the bot, then these won't be xDist and yDist for long. Maybe distinguish them with left/right/front/rear instead?

rothso · 2017-02-11T18:20:06Z

src/main/java/org/teamresistance/frc/IO.java

      gyro
  );
+  //  private static final DistanceSensor xDistPing = Hardware.DistanceSensors.digitalUltrasonic(0,1);
+  //  private static final DistanceSensor yDistPing = Hardware.DistanceSensors.digitalUltrasonic(2,3);


Dead code. Can be removed to avoid confusion.

rothso · 2017-02-11T18:22:17Z

src/main/java/org/teamresistance/frc/Robot.java

  public void robotInit() {
    Strongback.configure().recordNoEvents().recordNoData();
+    IO.xDistPing.setAutomaticMode(true);
+    IO.yDistPing.setAutomaticMode(true);


These two lines are super important. I think they're better placed in a static initializer at the bottom of IO so they're closer to the ping sensor declarations. Someone in the future who wants to just copy/paste the IO class into another project may not remember to configure the ping sensors.

static { xDistPing.setAutomaticMode(true); yDistPing.setAutomaticMode(true); }

rothso · 2017-02-11T18:45:13Z

src/main/java/org/teamresistance/frc/Robot.java

    reactor.onTriggeredSubmit(rightJoystick.getButton(3), () -> new HoldAngleCommand(drive, 135));
    reactor.onTriggeredSubmit(rightJoystick.getButton(4), () -> new HoldAngleCommand(drive, 0));

+    reactor.onTriggeredSubmit(leftJoystick.getButton(6), () -> new DriveToYX(drive,60,66,60,0));


Button 6 was being used for one of the Driving buttons. Could you find that test up there and change it to 9? Maybe move the line for 10 to be above 11 while you're at it too.

After this PR gets merged we should consider archiving some of these tests. I've got a long-term solution in mind for this button fiasco that doesn't involve attaching a piano to the DriverStation.

rothso · 2017-02-11T18:47:05Z

src/main/java/org/teamresistance/frc/Robot.java

  @Override
  public void autonomousInit() {
    Strongback.start();
+    // make sure these measurements are right


Can you remove this line? Sorry about that, looks like it was left over from the rebase.

rothso · 2017-02-11T19:16:55Z

src/main/java/org/teamresistance/frc/subsystem/drive/DriveToXController.java

+
+  @Override
+  public Drive.Signal computeSignal(Drive.Signal feedForward, Feedback feedback) {
+    // multiplied xSpeed by -1 because otherwise, would go in wrong direction


Is this comment out of date?

rothso · 2017-02-11T19:18:18Z

src/main/java/org/teamresistance/frc/subsystem/drive/DriveToXController.java

+  @Override
+  public Drive.Signal computeSignal(Drive.Signal feedForward, Feedback feedback) {
+    // multiplied xSpeed by -1 because otherwise, would go in wrong direction
+    double xSpeed = this.xTranslationPID.calculate(feedback.xDist) + this.ADDED_POWER;


this.ADDED_POWER might be better written as just ADDED_POWER because it's static.

On that note, how does the bot stop when the ADDED_POWER is always added? Are you relying on friction?

rothso · 2017-02-11T19:18:45Z

src/main/java/org/teamresistance/frc/subsystem/drive/DriveToXController.java

+  @Override
+  public boolean isOnTarget() {
+   return (xDistPingSensor <= hackTargetX);
+//    return xTranslationPID.isWithinTolerance();


Looks like we're good to revert the code back.

rothso · 2017-02-11T19:28:38Z

src/main/java/org/teamresistance/frc/command/DriveToYX.java

+/**
+ * Created by shrey on 1/29/2017.
+ */
+public class DriveToYX extends CommandGroup {


Do you think it's too early to put this into a class? This may make it harder to add more Commands to the ping sensor routine because you would have to add additional parameters to the class. I would consider removing this class and inlining the CommandGroup in Robot.java, similar to how the strafing CommandGroup is written.

rothso · 2017-02-11T19:34:21Z

src/main/java/org/teamresistance/frc/subsystem/drive/DriveToYController.java

+  private static final double ROTATION_KP = 0.008;
+//  private static final double ROTATION_KD = 0;
+//  private static final double ROTATION_KI = 0;
+//  private static final double ROTATION_FF = 0;


Dead code. Looks like it can be removed, since you've inlined all the 0s.

ShreyaRavi requested review from 098tarik and rothso February 1, 2017 15:35

ShreyaRavi changed the title ~~Rothso/develop/auto~~ Added autonomous driving code to the hoppers using the ping sensors Feb 1, 2017

ShreyaRavi changed the title ~~Added autonomous driving code to the hoppers using the ping sensors~~ Added autonomous driving code to hoppers using ping sensors Feb 1, 2017

rothso mentioned this pull request Feb 3, 2017

Register the ping sensor #14

Closed

rothso added the Drive Subsystem label Feb 4, 2017

rothso force-pushed the rothso/develop/auto branch 3 times, most recently from 9bcb438 to e7042ea Compare February 11, 2017 17:42

ShreyaRavi and others added 8 commits February 11, 2017 12:52

Drives To Y Using Ping Sensor. TODO: Drive to X, which is the same, b…

f0503eb

…ut need to add ability to add constant to the SynchronousPID @rothso.

Primitive autonomous code using ping sensors (not timed). TODO: repla…

d5ab269

…ce dummy values with real values and test. Fingers crossed.

Cleanup

f263f54

More Cleanup

b5689a7

Stop command at target. Increased tolerance so there's room for error…

f574112

… - can tune later.

Print out Ping sensor readings to SmartDashboard. Can read values whi…

92a9c3a

…le running command or while in teleop.

Seems like withinTolerance() is not working, will test theory using A…

f621807

…ngleHoldController. Works when not right up against ping sensor, not sure why very small readings make it go in the opposite direction. Testing needed.

Update old controllers to use Feedback instead of Pose

f084149

rothso force-pushed the rothso/develop/auto branch from e7042ea to f084149 Compare February 11, 2017 17:52

Update old StrafeCommand references to DriveTimedCommand, remove dupes

3bd2f1a

rothso force-pushed the rothso/develop/auto branch from b52beab to 3bd2f1a Compare February 11, 2017 18:00

Add back in Brake test

46c4a8a

rothso suggested changes Feb 11, 2017

View reviewed changes

Added autonomous driving code to hoppers using ping sensors #18

Are you sure you want to change the base?

Added autonomous driving code to hoppers using ping sensors #18

Uh oh!

Conversation

ShreyaRavi commented Feb 1, 2017

Uh oh!

rothso left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

rothso left a comment •

edited

Loading