Heartland Regional Code #7
Conversation
| public void execute() { | ||
| // System.out.println(Timer.getFPGATimestamp()); | ||
| // testing | ||
| if (SHOOTER.isRearBroken()) { |
There was a problem hiding this comment.
Let's try to avoid this significant amount of nested if statements.
ex.
if (!SHOOTER.isRearBroken()) {
CANDLES.setColorLeft(255, 0, 0);
return;
}
if (!SHOOTER.isCenterBroken()) {
CANDLES.setColorLeft(128, 64, 0);
return;
}
...
The more flat we can keep this logic, the easier it'll be to follow what's going on for all the different conditions.
There was a problem hiding this comment.
I'm not sure if this change really satisfies the comment. I'll try to make a proper suggestion.
There was a problem hiding this comment.
I believe something like this should achieve the desired result (or at least similar) while still being easy to digest at a glance:
if (!SHOOTER.isRearBroken()) {
CANDLES.setColorLeft(255, 0, 0);
return;
}
if (!SHOOTER.isCenterBroken()) {
CANDLES.setColorLeft(128, 64, 0);
return;
}
CANDLES.setColorLeft(0, 128, 128);
if (!PHOTON_SPEAKER.hasTargets()) {
CANDLES.setColorRight(255, 0, 0);
return;
}
double dist = Util.getInterpolatedDistance(PHOTON_SPEAKER);
if (dist < 2.25 && dist > 4.25) {
CANDLES.setColorRight(255, 0, 0);
return;
}
if (Util.isWithinTolerance(PHOTON_SPEAKER.getYawVal(), 0.0, 1)
&& ((Timer.getFPGATimestamp()) / BLINK_CONSTANT) == 0) {
CANDLES.setColorRight(0, 0, 0);
return;
}
CANDLES.setColorRight(0, 255, 0);
However, this needs to be tested for correctness on the robot before merging.
There was a problem hiding this comment.
I implemented the suggestion but changed the .setColor methods with their new counterparts for readability.
| // Returns true when the command should end. | ||
| @Override | ||
| public boolean isFinished() { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Command super class defaults to returning false, so it should be safe to remove this override.
There was a problem hiding this comment.
Not just the @Override, but rather the entire method should be good to remove. Since we're not really changing the default behavior of the method.
Code that was running at the Heartland Regional.