-
Notifications
You must be signed in to change notification settings - Fork 0
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
arm #20
base: master
Are you sure you want to change the base?
Conversation
The method getAngle in Arm system will change.
I added 2 methods for 2 different directions - backwards and forward.
src/main/java/subSystems/Arm.java
Outdated
private CANSparkMax master; | ||
private CANSparkMax follower; | ||
private PidController pid; | ||
private final double KP = 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.
Add the word static
to each constant field.
src/main/java/actions/SetAngle.java
Outdated
|
||
@Override | ||
public void initialize(ActionControl control) { | ||
arm.pidReset(); |
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.
Great
src/main/java/subSystems/Arm.java
Outdated
this.master = master; | ||
this.follower = follower; | ||
|
||
follower.follow(master); |
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.
Great
src/main/java/subSystems/Arm.java
Outdated
} | ||
|
||
public double getAngle(){ | ||
return 5; //Will change |
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.
In here, read the absolute encoder in order to find out current angle.
How to read and use the encoder
Moreover, you can always use Ctrl + B in order to see how the DutyCycelEncoder works
src/main/java/subSystems/Arm.java
Outdated
} | ||
|
||
public void stop(){ master.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.
Once you've fixed everything, commit & push, and pull request again
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.
I haven't seen you creating in SystemFactory the arm system-so create it(and use RobotMap as well)
src/main/java/subSystems/Arm.java
Outdated
|
||
public class Arm extends Subsystem { | ||
|
||
private double 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.
Specify the variable to angle2Target
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.
When you write 500 lines of code and exit without saving it;
⚰️
super(robotControl); | ||
arm = SystemFactory.createArm(); |
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.
Great
public static final int ARM_MASTER = 3; | ||
public static final int ARM_FOLLOW = 3; | ||
public static final HidChannel ARM_ENCODER = RoboRio.newHidChannel(0); | ||
public static final double ARM_ANGLE = 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.
What is that?
CANSparkMax master = new CANSparkMax(RobotMap.ARM_MASTER, CANSparkMaxLowLevel.MotorType.kBrushed); | ||
CANSparkMax follow = new CANSparkMax(RobotMap.ARM_FOLLOW, CANSparkMaxLowLevel.MotorType.kBrushless); | ||
DutyCycleEncoder encoder = new DutyCycleEncoder((DutyCycle) RobotMap.ARM_ENCODER); | ||
double angle2Target = RobotMap.ARM_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.
The angle2Target is set by the encoder value! :) use the encoder.position
CANSparkMax follow = new CANSparkMax(RobotMap.ARM_FOLLOW, CANSparkMaxLowLevel.MotorType.kBrushless); | ||
DutyCycleEncoder encoder = new DutyCycleEncoder((DutyCycle) RobotMap.ARM_ENCODER); | ||
double angle2Target = RobotMap.ARM_ANGLE; | ||
return new Arm(angle2Target, master, follow, encoder); |
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.
Without angle2Target
src/main/java/subSystems/Arm.java
Outdated
private static final double KI = 0; | ||
private static final double KD = 0; | ||
private static final double KF = 0; | ||
private static final double ERROR = 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.
Set error and limit values- it's an angle, so don't use set too much but not so little as well. Choose from 0.5-1
src/main/java/subSystems/Arm.java
Outdated
} | ||
|
||
public double getAngle2Target(){ | ||
encoder.reset(); |
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.
It shouldn't be here. It should be in the initialize- We don't want everysingle second to reset the encoder
src/main/java/subSystems/Arm.java
Outdated
|
||
public double getAngle2Target(){ | ||
encoder.reset(); | ||
return encoder.getPositionOffset(); |
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.
Great
src/main/java/subSystems/Arm.java
Outdated
} | ||
|
||
public double speed(){ | ||
return pid.applyAsDouble(getAngle2Target(), angle2Target); |
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.
Awesome
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.
A few problems I noticed
@@ -0,0 +1,23 @@ | |||
<component name="ProjectRunConfigurationManager"> |
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.
@NoamZW there are conflicts. |
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.
some more things. Sorry for not commenting all at once. Kinda preoccupied with other stuff.
src/main/java/subSystems/Arm.java
Outdated
return encoder.getPositionOffset(); | ||
} | ||
|
||
public double speed(){ |
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.
We don't want to use just a specific set point, as you do with angle2Target
value. But rather we are looking for the target angle to be provided depending on use, so it should actually be a parameter.
@NoamZW @Maayan-Luzon you may want to discuss this with @stavit456.
No description provided.