Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added GearPlacerServo subsystem and added some Javadocs. #4

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

mannguyen4624
Copy link
Contributor

No description provided.

@@ -24,8 +24,9 @@
public static final int GYRO = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We know the RobotMap class is full of port numbers, eliminate that from the comments throughout the class

@@ -5,7 +5,7 @@
import edu.wpi.first.wpilibj.command.Command;

/**
*
* Command to lower the gear placer to safely drive.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We know this is a command. Write the purpose only

@@ -5,7 +5,7 @@
import edu.wpi.first.wpilibj.command.Command;

/**
*
* Command that brings gear placer up to give the lift a gear.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't label as a command in the docs

import edu.wpi.first.wpilibj.command.Subsystem;

/**
* Similar to GearPlacer class but uses a servo instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't reference another class without an explicit JavaDoc link.

@@ -2,41 +2,22 @@

public class RobotMap {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bit that said they were wheels was helpful. Add a comment:
// Wheel motors

public static final int FRONT_LEFT = 0;
public static final int FRONT_RIGHT = 1;
public static final int BACK_LEFT = 2;
public static final int BACK_RIGHT = 3;

// Ports of intake
public static final int ROLLER_A = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which ones are A and B? Top/Bottom? Left/Right? Specify. If you don't know, put that in a TODO comment

public static final int MICRO_SWITCH = 0;

// Ports of encoders
public static final int DT_ENCODER_1 = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A number tells us nothing. Where are these on the robot?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll be using CANTalons so we won't need these anymore.

@@ -51,8 +32,6 @@
public static final int TWIN_ENCODER_4 = 12;
public static final int SHOOT_ENCODER_1 = 0;
public static final int SHOOT_ENCODER_2 = 1;

// Port of joysticks
public static final int JOYSTICK1 = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which joysticks? The number does not tell me anything

@@ -5,7 +5,7 @@
import edu.wpi.first.wpilibj.command.Command;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These descriptions are for commands. Write them like commands i.e. "Raise the gear placer" instead of "Raises the gear placer"

@@ -6,7 +6,7 @@
import edu.wpi.first.wpilibj.command.Subsystem;

/**
* Similar to GearPlacer class but uses a servo instead.
* Similar to {@link GearPlacer} class but uses a servo instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should extend the GearPlacer class or alternately you should have an interface that abstracts over both.

@MiloDavis
Copy link
Contributor

@sytect Fix merge conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants