-
Notifications
You must be signed in to change notification settings - Fork 1
Merge pivot to develop (by dror) #2
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
base: develop
Are you sure you want to change the base?
Conversation
orira22
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the code is good
Need to define some constants and fix documentation
|
|
||
| /* You should consider using the more terse Command factories API instead https://docs.wpilib.org/en/stable/docs/software/commandbased/organizing-command-based.html#defining-commands */ | ||
| public class PivotMoveToAngle extends Command { | ||
| public Pivot myPivot; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instance should be private
|
|
||
| /* You should consider using the more terse Command factories API instead https://docs.wpilib.org/en/stable/docs/software/commandbased/organizing-command-based.html#defining-commands */ | ||
| public class PivotMoveWithJoystick extends Command { | ||
| public Pivot myPivot; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instance should be private
| public class PivotMoveWithJoystick extends Command { | ||
| public Pivot myPivot; | ||
|
|
||
| private DoubleSupplier _double_Supplier; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change instance name to camel case
|
|
||
| private DoubleSupplier _double_Supplier; | ||
|
|
||
| public PivotMoveWithJoystick(DoubleSupplier Double_Supplier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change parameter name to camel case
|
|
||
| private static Pivot instance = null; | ||
|
|
||
| public double tolerance = 2.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define in constants file
|
|
||
| private Pivot() | ||
| { | ||
| motor = new SparkFlex(1, MotorType.kBrushless); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define motor ID in constants file
| } | ||
|
|
||
| /*** | ||
| * The function moves the motor to the wanted angle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moves the pivot to the wanted angle :)
| closedLoopController.setReference(angle, ControlType.kMAXMotionPositionControl,ClosedLoopSlot.kSlot0); | ||
| } | ||
|
|
||
| public void SetPower(double power) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name should start with little letter
| motor.set(power); | ||
| } | ||
|
|
||
| public void StopMotor() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name should start with little letter
| } | ||
|
|
||
| /*** | ||
| * The function checks if the motor has reached the wanted angle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the pivot*
No description provided.