Code Review
Introduction to Code Review
Code review is a systematic process of examining code to find bugs, improve quality, and ensure consistency. For FTC teams, code reviews help catch issues early and improve team collaboration. This lesson covers how to conduct effective code reviews for robot programming.
Why Code Review Matters for FTC
- Catches bugs before they reach the robot during competition
- Improves code quality and maintainability
- Helps team members learn from each other
- Ensures safety and performance standards are met
- Maintains consistent coding standards across the team
Code Review Process for FTC Teams
Establishing a systematic code review process helps ensure all code changes are properly examined. A good review process includes checklists, feedback guidelines, and clear communication.
Code Review Checklist - Review Categories
Main review categories for FTC code
Good Practices:
- Functionality - Ensure code works correctly and handles edge cases
- Safety - Prevent robot damage and ensure safe operation
- Performance - Maintain smooth robot operation and efficiency
- Style - Improve code readability and maintainability
- Documentation - Ensure code is understandable and well-documented
Avoid:
- Reviewing only one category at a time
- Skipping safety checks in favor of functionality
- Ignoring performance issues for cleaner code
- Focusing only on style without checking functionality
- Documenting without verifying code correctness
Understanding Review Categories
The review categories help organize the code review process. Functionality checks ensure the code works correctly, safety checks prevent robot damage, performance checks maintain smooth operation, style checks improve readability, and documentation checks ensure maintainability.
Code Review Checklist - Functionality and Safety Checks
Critical categories for FTC code reviews
Good Practices:
- Does the code do what it's supposed to do?
- Are all edge cases handled?
- Does the code handle hardware failures gracefully?
- Are there any infinite loops or deadlocks?
- Does the autonomous sequence complete successfully?
- Are gamepad controls intuitive and responsive?
- Does the robot stop when it should?
- Are there safety timeouts for all operations?
- Does the robot stop if sensors detect obstacles?
- Are motor powers limited to safe values?
- Is there emergency stop functionality?
- Does the code prevent the robot from damaging itself?
- Are there checks for hardware initialization failures?
- Does the robot have a safe default state?
Avoid:
- Assuming hardware will always work correctly
- Ignoring edge cases for simpler code
- Using unlimited motor powers
- Missing emergency stop functionality
- Not testing autonomous sequences thoroughly
- Overlooking sensor failure scenarios
- Forgetting to handle initialization errors
Understanding Functionality and Safety Checks
Functionality checks ensure your robot code works as intended and handles real-world scenarios. Safety checks are critical for preventing robot damage and ensuring safe operation during competition. These categories should be reviewed first as they directly impact robot performance and safety.
Code Review Checklist - Performance and Style Checks
Focus on code efficiency and maintainability
Good Practices:
- Does the main loop run at acceptable speed (50Hz)?
- Are there any memory leaks or excessive object creation?
- Are sensor readings optimized (not read too frequently)?
- Are complex calculations broken into smaller pieces?
- Is telemetry updated at reasonable intervals?
- Are there any blocking operations in the main loop?
- Is the code efficient for the target hardware?
- Are variable and method names descriptive?
- Is the code properly indented and formatted?
- Are there consistent naming conventions?
- Is the code organized into logical methods?
- Are magic numbers replaced with named constants?
- Is the code readable and well-structured?
- Are there appropriate comments for complex logic?
Avoid:
- Creating objects in tight loops
- Reading sensors every loop iteration
- Updating telemetry too frequently
- Using blocking operations in main loop
- Using unclear variable names
- Leaving magic numbers in code
- Writing overly complex methods
- Forgetting to comment complex logic
Performance and Style Considerations
Performance checks ensure your robot operates smoothly and efficiently. Style checks improve code readability and maintainability, making it easier for team members to understand and modify the code. Documentation checks ensure that complex logic is properly explained for future reference.
Code Review Checklist - Documentation Checks
Ensure code is maintainable and understandable
Good Practices:
- Is there JavaDoc for public methods?
- Are complex algorithms explained with comments?
- Is there a README file for the project?
- Are hardware configurations documented?
- Are there comments explaining robot behavior?
- Is the code self-documenting where possible?
- Are there inline comments for non-obvious code?
Avoid:
- Writing code without any comments
- Using unclear method names
- Not documenting hardware requirements
- Leaving complex logic unexplained
- Forgetting to update documentation
- Using outdated comments
- Not explaining robot behavior decisions
Understanding Documentation Checks
Documentation checks ensure that your code is maintainable and understandable by other team members. Good documentation includes JavaDoc comments, inline explanations for complex logic, and project-level documentation that explains the overall robot design and configuration.
Example of Well-Reviewed OpMode
Let's look at an example of a well-reviewed OpMode that follows the checklist. This example demonstrates proper code organization, safety measures, and documentation.
Reviewed OpMode - Constants and Hardware Components
/**
* Example of a well-reviewed OpMode that follows the checklist
*/
public static class ReviewedOpMode extends OpMode {
// Constants - no magic numbers
private static final double MAX_DRIVE_SPEED = 0.8;
private static final double MAX_TURN_SPEED = 0.6;
private static final double DEADBAND_THRESHOLD = 0.1;
private static final double SAFETY_DISTANCE_CM = 10.0;
private static final long TELEMETRY_UPDATE_INTERVAL_MS = 100;
// Hardware components - clearly named
private DcMotor leftDriveMotor, rightDriveMotor;
private Servo armServo, clawServo;
private ColorSensor gameElementDetector;
private DistanceSensor proximitySensor;
// State variables - descriptive names
private boolean isEmergencyStopActive = false;
private long lastTelemetryUpdate = 0;
private double currentArmPosition = 0.5;
private boolean isClawOpen = false;Understanding Well-Reviewed OpMode Structure
The well-reviewed OpMode uses named constants instead of magic numbers, has clearly named hardware components, and includes descriptive state variables. This makes the code more readable and maintainable.
Reviewed OpMode - Initialization Methods
/**
* Initializes the OpMode and all hardware components.
* Includes safety checks and error handling.
*/
@Override
public void init() {
try {
initializeHardware();
configureHardware();
resetRobotState();
displayInitializationStatus();
} catch (Exception e) {
handleInitializationError(e);
}
}
/**
* Main control loop with safety and performance considerations.
* Runs at approximately 50Hz for smooth robot control.
*/
@Override
public void loop() {
// Check for emergency stop first
if (isEmergencyStopActive) {
stopAllMotors();
return;
}
// Process input with safety checks
processDriverInput();
// Update robot state
updateRobotState();
// Apply safety checks
applySafetyChecks();
// Update telemetry periodically for performance
updateTelemetryIfNeeded();
}Understanding Initialization and Main Loop
The initialization method includes proper error handling with try-catch blocks. The main loop is organized into clear sections with safety checks first, followed by input processing, state updates, and periodic telemetry updates for performance.
Reviewed OpMode - Hardware Initialization and Configuration
/**
* Initializes all hardware components with error handling.
* Throws exception if critical hardware is missing.
*/
private void initializeHardware() throws Exception {
// Initialize drive motors
leftDriveMotor = hardwareMap.get(DcMotor.class, "left_drive");
rightDriveMotor = hardwareMap.get(DcMotor.class, "right_drive");
// Initialize servos
armServo = hardwareMap.get(Servo.class, "arm_servo");
clawServo = hardwareMap.get(Servo.class, "claw_servo");
// Initialize sensors
gameElementDetector = hardwareMap.get(ColorSensor.class, "color_sensor");
proximitySensor = hardwareMap.get(DistanceSensor.class, "distance_sensor");
// Verify critical hardware is available
if (leftDriveMotor == null || rightDriveMotor == null) {
throw new Exception("Critical drive motors not found");
}
}
/**
* Configures hardware settings for safe operation.
*/
private void configureHardware() {
// Configure drive motors for safety
leftDriveMotor.setDirection(DcMotor.Direction.FORWARD);
rightDriveMotor.setDirection(DcMotor.Direction.REVERSE);
leftDriveMotor.setZeroPowerBehavior(DcMotor.ZeroPowerBehavior.BRAKE);
rightDriveMotor.setZeroPowerBehavior(DcMotor.ZeroPowerBehavior.BRAKE);
// Set initial servo positions
if (armServo != null) armServo.setPosition(currentArmPosition);
if (clawServo != null) clawServo.setPosition(isClawOpen ? 1.0 : 0.0);
}Understanding Hardware Initialization and Configuration
The hardware initialization method includes null checks for critical components and throws exceptions if essential hardware is missing. The configuration method sets up safe default behaviors like brake mode for motors and initial servo positions.
Common FTC-Specific Code Issues
FTC code has specific patterns and common issues that reviewers should look for. Understanding these patterns helps identify potential problems before they cause issues during competition.
Common Issues - Deadband and Timeout Examples
// Common FTC Code Issues and Solutions
public class CommonIssuesExamples {
/**
* ISSUE 1: No deadband on gamepad input
* PROBLEM: Controller drift causes unwanted movement
* SOLUTION: Apply deadband to prevent drift
*/
public static class DeadbandExample {
// BAD - No deadband
public void badGamepadProcessing() {
double forward = gamepad1.left_stick_y;
double turn = gamepad1.right_stick_x;
// This can cause unwanted movement from controller drift
}
// GOOD - With deadband
public void goodGamepadProcessing() {
double forward = applyDeadband(gamepad1.left_stick_y);
double turn = applyDeadband(gamepad1.right_stick_x);
// Prevents unwanted movement from controller drift
}
private double applyDeadband(double input) {
return Math.abs(input) < 0.1 ? 0.0 : input;
}
}Understanding Deadband Issues
Controller drift is a common problem in FTC. Without a deadband, small controller movements can cause unwanted robot movement. The applyDeadband method filters out small inputs below the threshold, preventing drift-related issues.
Common Issues - Timeout Examples
/**
* ISSUE 2: No safety timeouts
* PROBLEM: Robot can get stuck in operations
* SOLUTION: Add timeouts for all operations
*/
public static class TimeoutExample {
private double operationStartTime;
private static final double OPERATION_TIMEOUT = 5.0; // 5 seconds
// BAD - No timeout
public void badOperation() {
while (!isOperationComplete()) {
// Robot can get stuck here forever
performOperation();
}
}
// GOOD - With timeout
public void goodOperation() {
operationStartTime = getRuntime();
while (!isOperationComplete() && !isTimeoutReached()) {
performOperation();
}
if (isTimeoutReached()) {
handleTimeout();
}
}
private boolean isTimeoutReached() {
return getRuntime() - operationStartTime > OPERATION_TIMEOUT;
}
private void handleTimeout() {
// Stop operation and notify driver
stopOperation();
telemetry.addData("Warning", "Operation timed out");
}
}Understanding Timeout Issues
Timeouts prevent robots from getting stuck in operations indefinitely. The good example tracks operation start time and checks for timeout conditions, ensuring the robot can recover from stuck operations and notify the driver of issues.
Common Issues - Hardware Failure and Telemetry Examples
/**
* ISSUE 3: No hardware failure handling
* PROBLEM: Code crashes if hardware is missing or fails
* SOLUTION: Check for null and handle exceptions
*/
public static class HardwareFailureExample {
private DcMotor driveMotor;
// BAD - No null check
public void badHardwareAccess() {
driveMotor = hardwareMap.get(DcMotor.class, "drive_motor");
driveMotor.setPower(0.5); // Crashes if motor is null
}
// GOOD - With null check
public void goodHardwareAccess() {
driveMotor = hardwareMap.get(DcMotor.class, "drive_motor");
if (driveMotor != null) {
driveMotor.setPower(0.5);
} else {
telemetry.addData("ERROR", "Drive motor not found");
}
}
}Understanding Hardware Failure Handling
Hardware failures are common in FTC due to loose connections or missing components. The good example includes null checks to prevent crashes and provides feedback to the driver when hardware is missing.
Common Issues - Telemetry Optimization
/**
* ISSUE 4: Inefficient telemetry updates
* PROBLEM: Too much telemetry slows down the main loop
* SOLUTION: Update telemetry periodically
*/
public static class TelemetryExample {
private long lastTelemetryUpdate = 0;
private static final long TELEMETRY_INTERVAL = 100; // 100ms
// BAD - Updates every loop
public void badTelemetryUpdate() {
telemetry.addData("Runtime", getRuntime());
telemetry.addData("Motor Power", motor.getPower());
telemetry.addData("Sensor Value", sensor.getValue());
// This runs every 20ms, which is excessive
}
// GOOD - Updates periodically
public void goodTelemetryUpdate() {
long currentTime = System.currentTimeMillis();
if (currentTime - lastTelemetryUpdate > TELEMETRY_INTERVAL) {
telemetry.addData("Runtime", getRuntime());
telemetry.addData("Motor Power", motor.getPower());
telemetry.addData("Sensor Value", sensor.getValue());
lastTelemetryUpdate = currentTime;
}
}
}
}Understanding Telemetry Optimization
Excessive telemetry updates can slow down the main loop and reduce robot responsiveness. The good example updates telemetry only periodically (every 100ms) instead of every loop iteration, maintaining smooth robot control while still providing useful feedback.
Performance and Safety Review Criteria
Performance and safety are critical for FTC robots. Reviewers must ensure code meets performance requirements and includes proper safety measures.
Performance Review Criteria
- Main loop runs at 50Hz (20ms per loop) or faster
- No blocking operations in the main loop
- Telemetry updates are limited to reasonable frequency
- Memory usage is optimized (no excessive object creation)
- Sensor readings are not performed too frequently
- Complex calculations are broken into smaller pieces
Safety Review Criteria
- All operations have timeout limits
- Emergency stop functionality is implemented
- Motor powers are limited to safe values
- Proximity sensors are used to prevent collisions
- Hardware failures are handled gracefully
- Robot has a safe default state when stopped
Collaborative Development Workflows
Effective collaboration requires clear workflows, communication, and conflict resolution strategies. Teams need to work together efficiently while maintaining code quality.
Git Workflow for Team Collaboration
// Git Workflow for FTC Team Collaboration
/*
* BRANCHING STRATEGY FOR FTC TEAMS
*
* Main Branch Structure:
* - main: Production-ready code for competition
* - development: Integration branch for testing
* - feature/*: Individual feature branches
* - hotfix/*: Emergency fixes for competition
*
* Workflow:
* 1. Create feature branch from development
* 2. Develop and test feature
* 3. Create pull request to development
* 4. Code review and testing
* 5. Merge to development
* 6. Integration testing
* 7. Merge to main for competition
*
* Example Commands:
*
* # Start new feature
* git checkout development
* git pull origin development
* git checkout -b feature/autonomous-collector
*
* # Develop feature
* # ... make changes ...
* git add .
* git commit -m "Add autonomous collector functionality"
* git push origin feature/autonomous-collector
*
* # Create pull request (on GitHub/GitLab)
* # Review and merge to development
*
* # Prepare for competition
* git checkout main
* git merge development
* git tag -a v1.0 -m "Competition version 1.0"
* git push origin main --tags
*/Understanding Git Workflow
The Git workflow provides a structured approach to team collaboration. Feature branches allow individual development, pull requests enable code review, and the main branch contains only competition-ready code. This ensures code quality and team coordination.
Practice: Code Review Exercise
Practice code review skills with these exercises:
- Review a provided OpMode using the checklist
- Identify and fix common FTC code issues
- Create a well-structured OpMode following best practices
- Conduct a peer review of your code
- Document your review findings
// Sample OpMode to review:
import com.qualcomm.robotcore.eventloop.opmode.OpMode;
import com.qualcomm.robotcore.eventloop.opmode.TeleOp;
import com.qualcomm.robotcore.hardware.DcMotor;
import com.qualcomm.robotcore.hardware.Servo;
@TeleOp(name = "SampleOpMode")
public class SampleOpMode extends OpMode {
// Hardware components
private DcMotor leftMotor, rightMotor;
private Servo armServo;
// State variables
private boolean isArmUp = false;
private double driveSpeed = 0.8;
@Override
public void init() {
leftMotor = hardwareMap.get(DcMotor.class, "left_motor");
rightMotor = hardwareMap.get(DcMotor.class, "right_motor");
armServo = hardwareMap.get(Servo.class, "arm_servo");
}
@Override
public void loop() {
// Drive control
double forward = gamepad1.left_stick_y;
double turn = gamepad1.right_stick_x;
double leftPower = forward + turn;
double rightPower = forward - turn;
leftMotor.setPower(leftPower);
rightMotor.setPower(rightPower);
// Arm control
if (gamepad1.a) {
armServo.setPosition(1.0);
isArmUp = true;
} else if (gamepad1.b) {
armServo.setPosition(0.0);
isArmUp = false;
}
// Telemetry
telemetry.addData("Left Power", leftPower);
telemetry.addData("Right Power", rightPower);
telemetry.addData("Arm Up", isArmUp);
telemetry.update();
}
}