diff --git a/lib/src/main/java/com/team2813/lib2813/control/ControlMode.java b/lib/src/main/java/com/team2813/lib2813/control/ControlMode.java index 4bbc194c..221d7a9a 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/ControlMode.java +++ b/lib/src/main/java/com/team2813/lib2813/control/ControlMode.java @@ -1,17 +1,42 @@ package com.team2813.lib2813.control; +/** + * Defines the control algorithms available for motor controllers. + * + *

The positional control flag is used to determine if a control mode requires position feedback + * for proper operation. This distinction is important for: + * + *

+ */ public enum ControlMode { + /** Open-loop output as percentage of max [-1.0, 1.0] */ DUTY_CYCLE(false), + + /** Closed-loop velocity control (requires velocity feedback) */ VELOCITY(false), + + /** Trapezoidal motion profile with velocity/acceleration limits (requires position feedback) */ MOTION_MAGIC(true), + + /** Open-loop voltage output [-12V, 12V] */ VOLTAGE(false); private final boolean isPositionalControl; + /** + * @param isPositionalControl true if this mode requires position feedback + */ ControlMode(boolean isPositionalControl) { this.isPositionalControl = isPositionalControl; } + /** + * @return true if this control mode requires position sensing + */ public boolean isPositionalControl() { return isPositionalControl; } diff --git a/lib/src/main/java/com/team2813/lib2813/control/DeviceInformation.java b/lib/src/main/java/com/team2813/lib2813/control/DeviceInformation.java index cf760617..de4868cc 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/DeviceInformation.java +++ b/lib/src/main/java/com/team2813/lib2813/control/DeviceInformation.java @@ -3,6 +3,13 @@ import com.team2813.lib2813.util.InputValidation; import java.util.Optional; +/** + * Immutable container for CAN device addressing information. + * + *

CAN devices can exist on either the RoboRIO's built-in CAN bus or on external CAN buses (e.g., + * CANivore). This class encapsulates both the device ID and optional bus name for proper device + * identification. + */ public final class DeviceInformation { private int id; private Optional canbus; @@ -47,6 +54,12 @@ public Optional canbus() { return canbus; } + /** + * Compares based on both ID and CAN bus. + * + * @param o object to compare + * @return true if both ID and bus match + */ @Override public boolean equals(Object o) { if (!(o instanceof DeviceInformation)) return false; @@ -54,6 +67,11 @@ public boolean equals(Object o) { return other.id == id && other.canbus.equals(canbus); } + /** + * Hash combines ID and bus for proper hash-based collection behavior. + * + * @return combined hash of id and canbus + */ @Override public int hashCode() { return id * 31 + canbus.hashCode(); diff --git a/lib/src/main/java/com/team2813/lib2813/control/Encoder.java b/lib/src/main/java/com/team2813/lib2813/control/Encoder.java index d622212b..c3eed3e1 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/Encoder.java +++ b/lib/src/main/java/com/team2813/lib2813/control/Encoder.java @@ -4,53 +4,123 @@ import edu.wpi.first.units.measure.Angle; import edu.wpi.first.units.measure.AngularVelocity; -/** Specifies a device that can perceive rotational positions. */ +/** + * Interface specifying a device that can perceive rotational positions and velocities. + * + *

This interface defines the contract for encoder devices that provide angular position and + * velocity feedback. It supports both legacy double-based methods (deprecated) and modern type-safe + * unit-based methods using WPILib's units system. + * + *

The interface provides a migration path from unsafe raw double values to type-safe {@link + * Angle} and {@link AngularVelocity} measurements. New implementations should focus on the + * unit-safe methods, while legacy methods are maintained for backward compatibility but marked for + * removal. + * + *

Common implementations include absolute encoders (CANcoder), relative encoders (integrated + * motor encoders), and other rotational position sensing devices. + * + * @author Team 2813 + * @since 1.0 + */ public interface Encoder { + /** - * Gets the position of the encoder + * Gets the current position of the encoder as a raw double value. * - * @return the position of the encoder + * @return the position of the encoder as an unspecified double value * @deprecated This method does not specify position in a specific measurement, so it is not safe - * to use. Use {@link #getPositionMeasure()} instead + * to use. Use {@link #getPositionMeasure()} instead for type safety */ @Deprecated(forRemoval = true) double position(); /** - * Gets the position of the encoder + * Gets the current position of the encoder using type-safe units. + * + *

This method returns the encoder position as an {@link Angle} measurement, providing type + * safety and explicit unit handling. The returned angle can be easily converted to any angular + * unit (degrees, radians, rotations) using the WPILib units system. + * + *

Example usage: + * + *

{@code
+   * Angle position = encoder.getPositionMeasure();
+   * double degrees = position.in(Units.Degrees);
+   * double rotations = position.in(Units.Rotations);
+   * }
* - * @return the position of the encoder as a measure + * @return the current position of the encoder as an {@link Angle} measurement */ Angle getPositionMeasure(); /** - * Sets the position of the encoder + * Sets the encoder position to the specified raw double value. * - * @param position the position of the encoder + *

Warning: This method accepts position without specifying units, making it unsafe and + * ambiguous. The interpretation of the position value depends on the specific encoder + * implementation and configuration. + * + * @param position the new position value as an unspecified double * @deprecated This method does not specify a unit, so it is not safe to use. Use {@link - * #setPosition(Angle)} instead. + * #setPosition(Angle)} instead for type safety */ @Deprecated(forRemoval = true) void setPosition(double position); + /** + * Sets the encoder position using type-safe units. + * + *

This method accepts any {@link Angle} measurement and converts it to the encoder's native + * units for setting the position. The type-safe approach eliminates unit confusion and provides + * clear, readable code. + * + *

The default implementation converts the angle to radians and calls the legacy {@link + * #setPosition(double)} method. Implementations should override this method to provide direct + * unit-safe position setting when possible. + * + *

Example usage: + * + *

{@code
+   * encoder.setPosition(Units.Degrees.of(90));
+   * encoder.setPosition(Units.Rotations.of(0.25));
+   * }
+ * + * @param position the new position as an {@link Angle} measurement + */ default void setPosition(Angle position) { setPosition(position.in(Units.Radians)); } /** - * Gets the velocity of the encoder + * Gets the current velocity of the encoder as a raw double value. * - * @return the velocity that the encoder perceives + * @return the velocity that the encoder perceives as an unspecified double value * @deprecated This method does not specify velocity in a specific measurement, so it is not safe - * to use. Use {@link #getVelocityMeasure()} instead + * to use. Use {@link #getVelocityMeasure()} instead for type safety */ @Deprecated(forRemoval = true) double getVelocity(); /** - * Gets the velocity of the encoder + * Gets the current velocity of the encoder using type-safe units. + * + *

This method returns the encoder velocity as an {@link AngularVelocity} measurement, + * providing type safety and explicit unit handling. The returned velocity can be easily converted + * to any angular velocity unit using the WPILib units system. + * + *

The default implementation assumes the legacy {@link #getVelocity()} method returns radians + * per second and wraps it in a type-safe measurement. Implementations should override this method + * to provide the correct units for their specific hardware. + * + *

Example usage: + * + *

{@code
+   * AngularVelocity velocity = encoder.getVelocityMeasure();
+   * double rpm = velocity.in(Units.RPM);
+   * double radPerSec = velocity.in(Units.RadiansPerSecond);
+   * }
* - * @return The velocity as a measure + * @return the current velocity as an {@link AngularVelocity} measurement */ default AngularVelocity getVelocityMeasure() { return Units.RadiansPerSecond.of(getVelocity()); diff --git a/lib/src/main/java/com/team2813/lib2813/control/InvertType.java b/lib/src/main/java/com/team2813/lib2813/control/InvertType.java index 7606ceda..f96361ae 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/InvertType.java +++ b/lib/src/main/java/com/team2813/lib2813/control/InvertType.java @@ -6,6 +6,23 @@ import java.util.*; import java.util.stream.Stream; +/** + * Unified inversion semantics across different motor controller families. + * + *

This enum provides two distinct inversion modes: + * + *

+ * + *

The lazy-initialized Maps class uses a holder pattern to defer reverse-mapping construction + * until first use, avoiding unnecessary computation if conversions are never needed. + */ public enum InvertType { CLOCKWISE(InvertedValue.Clockwise_Positive, true), COUNTER_CLOCKWISE(InvertedValue.CounterClockwise_Positive, false), @@ -22,11 +39,18 @@ public enum InvertType { private final Optional phoenixInvert; private final Optional sparkMaxInvert; + /** Constructor for relative inversion types (no hardware mapping). */ InvertType() { phoenixInvert = Optional.empty(); sparkMaxInvert = Optional.empty(); } + /** + * Constructor for absolute direction types. + * + * @param phoenixInvert CTRE Phoenix 6 inversion value + * @param sparkMaxInvert REV Spark Max inversion boolean + */ InvertType(InvertedValue phoenixInvert, boolean sparkMaxInvert) { this.phoenixInvert = Optional.of(phoenixInvert); this.sparkMaxInvert = Optional.of(sparkMaxInvert); @@ -54,26 +78,40 @@ public static Optional fromSparkMaxInvert(boolean v) { return Optional.of(Maps.sparkMaxMap.get(v)); } + /** + * @return Phoenix inversion value if this is an absolute direction type + */ public Optional phoenixInvert() { return phoenixInvert; } + /** + * @return Phoenix inversion, throwing if not present (internal use only) + */ private InvertedValue forcePhoenixInvert() { return phoenixInvert.orElseThrow(); } + /** + * @return Spark Max inversion value if this is an absolute direction type + */ public Optional sparkMaxInvert() { return sparkMaxInvert; } + /** + * @return Spark Max inversion, throwing if not present (internal use only) + */ private boolean forceSparkMaxInvert() { return sparkMaxInvert.orElseThrow(); } /** - * Contains the maps for {@link InvertType#fromPhoenixInvert(InvertedValue)} and {@link - * InvertType#fromSparkMaxInvert(boolean)}. In a static class so that they will only be - * initialized if they are needed. + * Lazy-initialized reverse lookup maps using the holder pattern. + * + *

Maps are constructed only when first accessed, avoiding overhead if + * fromPhoenixInvert/fromSparkMaxInvert are never called. The merge function (a, b) -> null + * handles the impossible case of duplicate keys, which cannot occur given the enum definition. */ private static final class Maps { private static final Map phoenixMap = diff --git a/lib/src/main/java/com/team2813/lib2813/control/Motor.java b/lib/src/main/java/com/team2813/lib2813/control/Motor.java index 80bad2c5..6cf000e2 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/Motor.java +++ b/lib/src/main/java/com/team2813/lib2813/control/Motor.java @@ -2,9 +2,8 @@ import edu.wpi.first.units.measure.Current; +/** Basic motor control interface for command-based control and telemetry. */ public interface Motor { - // motor control - /** * Sets the motor to run with a specified mode of control. * diff --git a/lib/src/main/java/com/team2813/lib2813/control/PIDMotor.java b/lib/src/main/java/com/team2813/lib2813/control/PIDMotor.java index 745c742a..eaca2792 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/PIDMotor.java +++ b/lib/src/main/java/com/team2813/lib2813/control/PIDMotor.java @@ -1,11 +1,49 @@ package com.team2813.lib2813.control; +/** + * A motor with integrated PID control and encoder feedback. + * + *

Combines motor control, position sensing, and closed-loop control configuration. Some motor + * controllers support multiple PID slots for switching between different tuning parameters. + */ public interface PIDMotor extends Motor, Encoder { + /** + * Configures PIDF constants for a specific slot. + * + * @param slot the PID slot index (hardware-dependent, typically 0-3) + * @param p the proportional gain + * @param i the integral gain + * @param d the derivative gain + * @param f the feedforward gain + */ void configPIDF(int slot, double p, double i, double d, double f); + /** + * Configures PIDF constants for the default slot (typically slot 0). + * + * @param p the proportional gain + * @param i the integral gain + * @param d the derivative gain + * @param f the feedforward gain + */ void configPIDF(double p, double i, double d, double f); + /** + * Configures PID constants for a specific slot with zero feedforward. + * + * @param slot the PID slot index (hardware-dependent, typically 0-3) + * @param p the proportional gain + * @param i the integral gain + * @param d the derivative gain + */ void configPID(int slot, double p, double i, double d); + /** + * Configures PID constants for the default slot (typically slot 0) with zero feedforward. + * + * @param p the proportional gain + * @param i the integral gain + * @param d the derivative gain + */ void configPID(double p, double i, double d); } diff --git a/lib/src/main/java/com/team2813/lib2813/control/encoders/CancoderWrapper.java b/lib/src/main/java/com/team2813/lib2813/control/encoders/CancoderWrapper.java index ebd35cb3..ce56708a 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/encoders/CancoderWrapper.java +++ b/lib/src/main/java/com/team2813/lib2813/control/encoders/CancoderWrapper.java @@ -8,57 +8,123 @@ import edu.wpi.first.units.measure.Angle; import edu.wpi.first.units.measure.AngularVelocity; +/** + * A wrapper class for CTRE's CANcoder device that implements the Encoder interface. + * + *

This wrapper provides both legacy (deprecated) methods using raw double values and modern + * methods using WPILib's unit-safe measurement system. + */ public class CancoderWrapper implements Encoder { + // The actual CANcoder device object from CTRE's Phoenix 6 library private CANcoder cancoder; + // Stores information about the device's ID and CAN bus private DeviceInformation info; + /** + * Creates a new CANcoder wrapper with a specified ID and CAN bus. + * + * @param id The device ID of the CANcoder on the CAN bus + * @param canbus The name of the CAN bus the device is connected to + */ public CancoderWrapper(int id, String canbus) { cancoder = new CANcoder(id, canbus); info = new DeviceInformation(id, canbus); } + /** + * Creates a new CANcoder wrapper with just a device ID. Uses the default CAN bus. + * + * @param id The device ID of the CANcoder on the CAN bus + */ public CancoderWrapper(int id) { cancoder = new CANcoder(id); info = new DeviceInformation(id); } + /** + * Gets the current position of the encoder in rotations. + * + * @deprecated Use {@link #getPositionMeasure()} instead for type-safe units + * @return The position in rotations as a double + */ @Deprecated @Override public double position() { return cancoder.getPosition().getValueAsDouble(); } + /** + * Gets the current position of the encoder using WPILib's unit-safe Angle measure. + * + * @return The position as an Angle object in rotations + */ @Override public Angle getPositionMeasure() { return Units.Rotations.of(cancoder.getPosition().getValueAsDouble()); } + /** + * Sets the current position of the encoder to a specified value. Useful for zeroing or + * calibrating the encoder. + * + * @deprecated Use {@link #setPosition(Angle)} instead for type-safe units + * @param position The position to set in rotations + */ @Deprecated @Override public void setPosition(double position) { ConfigUtils.phoenix6Config(() -> cancoder.setPosition(position)); } + /** + * Sets the current position of the encoder using WPILib's unit-safe Angle measure. + * + * @param position The position to set as an Angle object + */ @Override public void setPosition(Angle position) { ConfigUtils.phoenix6Config(() -> cancoder.setPosition(position.in(Units.Rotations))); } + /** + * Gets direct access to the underlying CANcoder object. Use this if you need access to advanced + * features not exposed by this wrapper. + * + * @return The underlying CANcoder object + */ public CANcoder encoder() { return cancoder; } + /** + * Gets the current velocity of the encoder in rotations per second. + * + * @deprecated Use {@link #getVelocityMeasure()} instead for type-safe units + * @return The velocity in rotations per second as a double + */ @Deprecated @Override public double getVelocity() { return cancoder.getVelocity().getValueAsDouble(); } + /** + * Gets the current velocity of the encoder using WPILib's unit-safe AngularVelocity measure. + * + * @return The velocity as an AngularVelocity object + */ @Override public AngularVelocity getVelocityMeasure() { return cancoder.getVelocity().getValue(); } + /** + * Compares this CancoderWrapper with another object for equality. Two CancoderWrappers are + * considered equal if they have the same device information. + * + * @param obj The object to compare with + * @return true if the objects are equal, false otherwise + */ @Override public boolean equals(Object obj) { if (!(obj instanceof CancoderWrapper)) return false; diff --git a/lib/src/main/java/com/team2813/lib2813/control/imu/Pigeon2Wrapper.java b/lib/src/main/java/com/team2813/lib2813/control/imu/Pigeon2Wrapper.java index c6e16fe6..beca37c1 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/imu/Pigeon2Wrapper.java +++ b/lib/src/main/java/com/team2813/lib2813/control/imu/Pigeon2Wrapper.java @@ -4,19 +4,29 @@ import com.team2813.lib2813.control.DeviceInformation; import com.team2813.lib2813.util.ConfigUtils; +/** + * A wrapper class for CTRE's Pigeon2 Inertial Measurement Unit (IMU). + * + *

This wrapper simplifies the use of the Pigeon2 by providing easy access to common functions + * and handling reset detection automatically. + */ public class Pigeon2Wrapper { + // The actual Pigeon2 IMU device object from CTRE's Phoenix 6 library private Pigeon2 pigeon; + // Keeps track of the last known heading to restore after device resets private double currentHeading = 0; + + // Stores information about the device's ID and CAN bus private DeviceInformation info; /** - * Constructor + * Creates a new Pigeon2 wrapper with a specified device number and CAN bus. * - * @param deviceNumber [0,62] + * @param deviceNumber The device ID of the Pigeon2 on the CAN bus (valid range: 0-62) * @param canbus Name of the CANbus; can be a SocketCAN interface (on Linux), or a CANivore device - * name or serial number + * name or serial number. Used when multiple CAN buses are present on the robot. */ public Pigeon2Wrapper(int deviceNumber, String canbus) { info = new DeviceInformation(deviceNumber, canbus); @@ -24,31 +34,57 @@ public Pigeon2Wrapper(int deviceNumber, String canbus) { } /** - * Constructor + * Creates a new Pigeon2 wrapper using just a device number. Uses the default CAN bus on the + * robot. * - * @param deviceNumber [0,62] + * @param deviceNumber The device ID of the Pigeon2 on the CAN bus (valid range: 0-62) */ public Pigeon2Wrapper(int deviceNumber) { info = new DeviceInformation(deviceNumber); pigeon = new Pigeon2(deviceNumber); } + /** + * Gets direct access to the underlying Pigeon2 object. Use this if you need access to advanced + * features not exposed by this wrapper. + * + * @return The underlying Pigeon2 IMU object + */ public Pigeon2 getPigeon() { return pigeon; } + /** + * Gets the current heading (yaw) of the robot in degrees. This represents the direction the robot + * is facing: - 0 degrees is typically the robot's starting direction - Positive values indicate + * clockwise rotation - Negative values indicate counterclockwise rotation - Range is unlimited + * (can go beyond -360 to +360) + * + * @return The current heading in degrees + */ public double getHeading() { return pigeon.getYaw().getValueAsDouble(); } + /** + * Sets the current heading of the robot to a specified angle. Useful for: - Zeroing the heading + * (setting it to 0 degrees) - Calibrating the heading to match a known direction - Resetting the + * heading after a device reset + * + * @param angle The angle in degrees to set as the current heading + */ public void setHeading(double angle) { ConfigUtils.phoenix6Config(() -> pigeon.setYaw(angle)); currentHeading = angle; } /** - * Checks if a reset has occurred and restores non-persistent settings if so. Implement - * periodically (e.g. in a subsystem's periodic() method) + * Checks if the Pigeon2 has reset and restores its previous heading if needed. The Pigeon2 may + * reset due to power cycles or other issues, which resets its heading to 0. This method detects + * such resets and restores the last known heading automatically. + * + *

Call this method periodically (e.g., in a subsystem's periodic() method) to ensure the + * heading stays consistent even after device resets. */ public void periodicResetCheck() { if (!pigeon.hasResetOccurred()) { @@ -58,10 +94,23 @@ public void periodicResetCheck() { } } + /** + * Generates a hash code for this Pigeon2Wrapper instance. Used for storing the wrapper in + * hash-based collections. + * + * @return A hash code based on the device information + */ public int hashCode() { return info.hashCode(); } + /** + * Compares this Pigeon2Wrapper with another object for equality. Two Pigeon2Wrappers are + * considered equal if they have the same device information. + * + * @param other The object to compare with + * @return true if the objects are equal, false otherwise + */ public boolean equals(Object other) { if (!(other instanceof Pigeon2Wrapper)) { return false; diff --git a/lib/src/main/java/com/team2813/lib2813/control/motors/SparkMaxWrapper.java b/lib/src/main/java/com/team2813/lib2813/control/motors/SparkMaxWrapper.java index 8ac34981..abd59ce2 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/motors/SparkMaxWrapper.java +++ b/lib/src/main/java/com/team2813/lib2813/control/motors/SparkMaxWrapper.java @@ -20,27 +20,35 @@ import java.util.ArrayList; import java.util.List; +/** + * SPARK MAX motor controller wrapper implementing PID control with unit-safe methods. Handles + * configuration persistence, parameter resets, and follower motor synchronization. + */ public class SparkMaxWrapper implements PIDMotor { + // Stores follower motors to prevent garbage collection and maintain synchronization private final List followers = new ArrayList<>(); private final SparkBase motor; private final RelativeEncoder encoder; private final boolean inverted; + + // Configuration state management private final SparkBaseConfig config; private final SparkBase.ResetMode resetMode; private final SparkBase.PersistMode persistMode; /** - * Create a new object to control a SPARK MAX motor Controller + * Creates a new SPARK MAX wrapper with specified configuration. Initializes the controller with + * non-persistent parameters that reset safely, preventing configuration conflicts during code + * updates or power cycles. * - * @param deviceId The device ID. - * @param type The motor type connected to the controller. Brushless motor wires must be connected - * to their matching colors and the hall sensor must be plugged in. Brushed motors must be - * connected to the Red and Black terminals only. - * @param inverted Whether the motor is inverted + * @param deviceId CAN ID (1-62) + * @param type Motor type (brushed/brushless) + * @param inverted Direction setting (affects both standalone and follower behavior) */ public SparkMaxWrapper(int deviceId, SparkLowLevel.MotorType type, InvertType inverted) { motor = new SparkMax(deviceId, type); this.inverted = inverted.sparkMaxInvert().orElseThrow(); + // Apply inversion settings through alternate encoder config to ensure proper direction control config = new SparkMaxConfig().apply(new AlternateEncoderConfig().inverted(this.inverted)); persistMode = SparkBase.PersistMode.kNoPersistParameters; resetMode = SparkBase.ResetMode.kResetSafeParameters; @@ -48,11 +56,20 @@ public SparkMaxWrapper(int deviceId, SparkLowLevel.MotorType type, InvertType in encoder = motor.getEncoder(); } + /** + * @param controlMode Control mode to use + * @param demand Setpoint value + */ @Override public void set(ControlMode controlMode, double demand) { set(controlMode, demand, 0); } + /** + * @param controlMode Control mode (VOLTAGE: -12V to +12V, DUTY_CYCLE: -1.0 to +1.0) + * @param demand Setpoint value + * @param feedForward Additional feed-forward term + */ @Override public void set(ControlMode controlMode, double demand, double feedForward) { switch (controlMode) { @@ -66,16 +83,25 @@ public void set(ControlMode controlMode, double demand, double feedForward) { } } + /** + * @return Current position in rotations + */ @Override public double position() { return encoder.getPosition(); } + /** + * @return Current position as an Angle measure + */ @Override public Angle getPositionMeasure() { return Units.Rotations.of(encoder.getPosition()); } + /** + * @return Applied current in amperes + */ @Override public Current getAppliedCurrent() { return Units.Amps.of(motor.getOutputCurrent()); @@ -86,26 +112,50 @@ public void disable() { motor.stopMotor(); } + /** + * @param position Position in rotations + */ @Override public void setPosition(double position) { encoder.setPosition(position); } + /** + * @param position Position as an Angle measure + */ @Override public void setPosition(Angle position) { encoder.setPosition(position.in(Units.Rotations)); } + /** + * @return Velocity in RPM + */ @Override public double getVelocity() { return encoder.getVelocity(); } + /** + * @return Velocity as an AngularVelocity measure + */ @Override public AngularVelocity getVelocityMeasure() { return Units.Rotations.per(Units.Minute).of(encoder.getVelocity()); } + /** + * Configures closed-loop control gains for a specific slot. Each slot can store different PID + * configurations for various operating modes (e.g., different gains for high speed vs. precise + * positioning). + * + * @param slot Slot index (0-3) + * @param p Proportional gain - corrects present error + * @param i Integral gain - corrects accumulated error + * @param d Derivative gain - reduces oscillation + * @param f Feed-forward gain - provides base output + * @throws RuntimeException if slot is invalid + */ @Override public void configPIDF(int slot, double p, double i, double d, double f) { ClosedLoopSlot[] slots = ClosedLoopSlot.values(); @@ -117,6 +167,7 @@ public void configPIDF(int slot, double p, double i, double d, double f) { ConfigUtils.revConfig(() -> motor.configure(config, resetMode, persistMode)); } + // Convenience methods defaulting to slot 0 @Override public void configPIDF(double p, double i, double d, double f) { configPIDF(0, p, i, d, f); @@ -132,8 +183,19 @@ public void configPID(double p, double i, double d) { configPIDF(0, p, i, d, 0); } + /** + * Configures a motor to follow this one's output. The follower motor is added to an internal list + * to prevent garbage collection, ensuring the follower configuration persists throughout the + * program's lifetime. + * + * @param deviceId CAN ID of follower (1-62) + * @param type Motor type of follower + * @param inverted Direction relative to leader: FOLLOW/OPPOSE_MASTER for relative inversion, + * CLOCKWISE/COUNTER_CLOCKWISE for absolute direction + */ public void addFollower(int deviceId, SparkLowLevel.MotorType type, InvertType inverted) { SparkMax follower = new SparkMax(deviceId, type); + // Determine inversion based on whether the follower should match or oppose the leader boolean isInverted = switch (inverted) { case CLOCKWISE, COUNTER_CLOCKWISE -> inverted.sparkMaxInvert().orElseThrow(); @@ -144,6 +206,6 @@ public void addFollower(int deviceId, SparkLowLevel.MotorType type, InvertType i () -> follower.configure( new SparkMaxConfig().follow(motor).inverted(isInverted), resetMode, persistMode)); - followers.add(follower); // add to follower list so CANSparkMax follower object is preserved + followers.add(follower); // Prevent garbage collection of follower configuration } } diff --git a/lib/src/main/java/com/team2813/lib2813/control/motors/TalonFXWrapper.java b/lib/src/main/java/com/team2813/lib2813/control/motors/TalonFXWrapper.java index 69437281..12a762b4 100644 --- a/lib/src/main/java/com/team2813/lib2813/control/motors/TalonFXWrapper.java +++ b/lib/src/main/java/com/team2813/lib2813/control/motors/TalonFXWrapper.java @@ -13,7 +13,6 @@ import com.team2813.lib2813.control.DeviceInformation; import com.team2813.lib2813.control.InvertType; import com.team2813.lib2813.control.PIDMotor; -import com.team2813.lib2813.subsystems.MotorSubsystem; import com.team2813.lib2813.util.InvalidCanIdException; import edu.wpi.first.units.Units; import edu.wpi.first.units.measure.Angle; @@ -23,25 +22,46 @@ import java.util.List; import java.util.Objects; +/** + * Wrapper around a CTRE TalonFX providing a PIDMotor interface and safer defaults. + * + *

This wrapper applies conservative defaults at construction time: + * + *

+ * + *

The follower list prevents garbage collection of follower motor objects, which is necessary + * because Phoenix 6 uses weak references internally and would otherwise lose the follower + * configuration. + */ public class TalonFXWrapper implements PIDMotor { - /** A list of followers, so that they aren't garbage collected */ + /** + * Maintains strong references to follower motors to prevent GC. Phoenix 6 internally uses device + * IDs for follower control, but the TalonFX objects must persist to maintain configuration state. + */ private final List followers = new ArrayList<>(); /** the internal motor */ private final TalonFX motor; + /** + * Device identification (id + optional bus) used by follower APIs. Stored separately to avoid + * repeated CAN bus queries. + */ private final DeviceInformation information; /** - * Create a TalonFXWrapper on the specified canbus + * Create a TalonFXWrapper on the specified canbus. * * @param canID [0, 62] the can ID of the motor * @param canbus the canbus that the motor is on * @param invertType the invert type * @throws NullPointerException if either {@code invertType} or {@code canbus} are null * @throws IllegalArgumentException if {@code invertType} is not in {@link - * InvertType#rotationValues}. In other words, this exception is thrown when passed an {@link - * InvertType} that is for following motors + * InvertType#rotationValues} * @throws InvalidCanIdException if the CAN id is invalid */ public TalonFXWrapper(int canID, String canbus, InvertType invertType) { @@ -53,10 +73,10 @@ public TalonFXWrapper(int canID, String canbus, InvertType invertType) { motor = new TalonFX(checkCanId(canID), canbus); TalonFXConfiguration config = new TalonFXConfiguration(); - // should never throw anything, as the tests guarantee that everything in - // rotationValues - // returns a non-empty value with phoenixInvert + // rotationValues are guaranteed by the enum to map to Phoenix inversions; + // if missing, it indicates a programming error, hence AssertionError. config.MotorOutput.Inverted = invertType.phoenixInvert().orElseThrow(AssertionError::new); + // Conservative current limit for safety during development/testing config.CurrentLimits = new CurrentLimitsConfigs().withStatorCurrentLimit(40).withSupplyCurrentLimitEnable(true); TalonFXConfigurator configurator = motor.getConfigurator(); @@ -66,14 +86,13 @@ public TalonFXWrapper(int canID, String canbus, InvertType invertType) { } /** - * Create a TalonFXWrapper on the RoboRIO's canbus + * Create a TalonFXWrapper on the RoboRIO's default CAN bus. * * @param canID [0, 62] the can ID of the motor * @param invertType the invert type * @throws NullPointerException if {@code invertType} is null * @throws IllegalArgumentException if {@code invertType} is not in {@link - * InvertType#rotationValues}. In other words, this exception is thrown when passed an {@link - * InvertType} that is for following motors + * InvertType#rotationValues} * @throws InvalidCanIdException if the CAN id is invalid */ public TalonFXWrapper(int canID, InvertType invertType) { @@ -84,9 +103,6 @@ public TalonFXWrapper(int canID, InvertType invertType) { } TalonFXConfiguration config = new TalonFXConfiguration(); - // should never throw anything, as the tests guarantee that everything in - // rotationValues - // returns a non-empty value with phoenixInvert config.MotorOutput.Inverted = invertType.phoenixInvert().orElseThrow(AssertionError::new); config.CurrentLimits = new CurrentLimitsConfigs().withStatorCurrentLimit(40).withSupplyCurrentLimitEnable(true); @@ -96,10 +112,28 @@ public TalonFXWrapper(int canID, InvertType invertType) { information = new DeviceInformation(canID); } + /** + * Convenience overload forwarding to {@link #set(ControlMode,double,double)} with zero + * feed-forward. + * + * @param controlMode control algorithm to use + * @param demand setpoint value + */ public void set(ControlMode controlMode, double demand) { set(controlMode, demand, 0); } + /** + * Set controller output using Phoenix 6 control request objects. + * + *

Each control mode creates its own request object to leverage Phoenix 6's type-safe control + * API. Request objects are created per-call rather than cached to ensure thread safety and avoid + * state accumulation. + * + * @param controlMode control algorithm to use + * @param demand numeric setpoint (units depend on mode) + * @param feedForward feed-forward term applied where supported + */ @Override public void set(ControlMode controlMode, double demand, double feedForward) { switch (controlMode) { @@ -124,55 +158,79 @@ public void set(ControlMode controlMode, double demand, double feedForward) { } } + /** + * @return current position in rotations + */ @Override public double position() { return getPositionMeasure().in(Units.Rotations); } + /** + * @return current position as a unit-safe {@link Angle} + */ @Override public Angle getPositionMeasure() { return Units.Rotations.of(motor.getPosition().getValueAsDouble()); } + /** + * @return applied stator current as a unit-safe {@link Current} + */ @Override public Current getAppliedCurrent() { return motor.getStatorCurrent().getValue(); } + /** + * Set encoder position in raw rotations. + * + * @param position position in rotations + */ @Override public void setPosition(double position) { motor.setPosition(position); } + /** + * Set encoder position using a unit-safe {@link Angle}. + * + * @param position angle to set (converted to rotations) + */ @Override public void setPosition(Angle position) { motor.setPosition(position.in(Units.Rotations)); } + /** + * @return sensor velocity as raw double (device units: rotations/sec) + */ @Override public double getVelocity() { return motor.getVelocity().getValueAsDouble(); } + /** + * @return velocity as a unit-safe {@link AngularVelocity} + */ @Override public AngularVelocity getVelocityMeasure() { return Units.RotationsPerSecond.of(motor.getVelocity().getValueAsDouble()); } + /** + * Expose the underlying TalonFX instance for advanced use. + * + * @return the raw TalonFX + */ public TalonFX motor() { return motor; } /** - * Sets the behavior the motor should exhibit upon receiving a request to stop: {@link - * MotorSubsystem#disable()} + * Sets the behavior the motor should exhibit upon receiving a request to stop. * - *

- * - * @param mode + * @param mode neutral mode (COAST/BRAKE) */ public void setNeutralMode(NeutralModeValue mode) { motor.setNeutralMode(mode); @@ -188,6 +246,15 @@ public void disable() { motor.disable(); } + /** + * Configure PIDF gains for a slot using the CTRE SlotConfigs API. + * + * @param slot slot index + * @param p proportional gain + * @param i integral gain + * @param d derivative gain + * @param f feed-forward (velocity/feed-forward term) + */ @Override public void configPIDF(int slot, double p, double i, double d, double f) { SlotConfigs conf = new SlotConfigs(); @@ -195,46 +262,90 @@ public void configPIDF(int slot, double p, double i, double d, double f) { motor.getConfigurator().apply(conf.withKP(p).withKI(i).withKD(d).withKV(f)); } + /** + * Configure PIDF on default slot 0. + * + * @param p proportional + * @param i integral + * @param d derivative + * @param f feed-forward + */ @Override public void configPIDF(double p, double i, double d, double f) { configPIDF(0, p, i, d, f); } + /** + * Configure PID on a specific slot (no feed-forward). + * + * @param slot slot index + * @param p proportional + * @param i integral + * @param d derivative + */ @Override public void configPID(int slot, double p, double i, double d) { configPIDF(slot, p, i, d, 0); } + /** + * Configure PID on default slot 0 (no feed-forward). + * + * @param p proportional + * @param i integral + * @param d derivative + */ @Override public void configPID(double p, double i, double d) { configPIDF(0, p, i, d, 0); } + /** + * Add a follower motor on a specified CAN bus. + * + *

Phoenix 6 provides two follower modes: + * + *

+ * + * @param deviceNumber CAN id of follower (0-62) + * @param canbus CAN bus name + * @param invertType inversion rule + */ public void addFollower(int deviceNumber, String canbus, InvertType invertType) { TalonFX follower = new TalonFX(checkCanId(deviceNumber), canbus); if (InvertType.rotationValues.contains(invertType)) { TalonFXConfiguration conf = new TalonFXConfiguration(); - // guaranteed to succeed conf.MotorOutput.Inverted = invertType.phoenixInvert().orElseThrow(AssertionError::new); follower.setControl(new StrictFollower(information.id())); } else { follower.setControl( new Follower(information.id(), invertType.equals(InvertType.OPPOSE_MASTER))); } - followers.add(follower); // add to follower list so TalonFX follower object is preserved + followers.add(follower); } + /** + * Add a follower motor on the default CAN bus. + * + * @param deviceNumber CAN id of follower (0-62) + * @param invertType inversion rule + */ public void addFollower(int deviceNumber, InvertType invertType) { TalonFX follower = new TalonFX(checkCanId(deviceNumber)); if (InvertType.rotationValues.contains(invertType)) { TalonFXConfiguration conf = new TalonFXConfiguration(); - // guaranteed to succeed conf.MotorOutput.Inverted = invertType.phoenixInvert().orElseThrow(AssertionError::new); follower.setControl(new StrictFollower(information.id())); } else { follower.setControl( new Follower(information.id(), invertType.equals(InvertType.OPPOSE_MASTER))); } - followers.add(follower); // add to follower list so TalonFX follower object is preserved + followers.add(follower); } } diff --git a/lib/src/main/java/com/team2813/lib2813/doc-files/dutycyclediagram.jpeg b/lib/src/main/java/com/team2813/lib2813/doc-files/dutycyclediagram.jpeg new file mode 100644 index 00000000..4d174265 Binary files /dev/null and b/lib/src/main/java/com/team2813/lib2813/doc-files/dutycyclediagram.jpeg differ diff --git a/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java b/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java index 4551b78d..6980fda4 100644 --- a/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java +++ b/lib/src/main/java/com/team2813/lib2813/preferences/PersistedConfiguration.java @@ -95,38 +95,34 @@ * @since 2.0.0 */ public final class PersistedConfiguration { + /** Key used to track which record classes are bound to preference namespaces. */ static final String REGISTERED_CLASSES_NETWORK_TABLE_KEY = "PersistedConfiguration/registry"; + + /** Whether legacy keys have already been cleaned once per program run. */ private static boolean deletedLegacyKeys = false; - // The below package-scope fields are for the self-tests. + // Package-private fields for self-tests. static boolean throwExceptions = false; static Consumer errorReporter = DataLogManager::log; /** - * Creates a record class instance with fields populated from Preferences, using the provided - * defaults. + * Constructs a record instance, populating its components from Preferences, using the provided + * instance to get default values. * - *

To be stored in preferences, the type of the record components can be any of the following: + *

The provided instance supplies the default values. For each component: * *

* - *

The values for the components for the passed-in instance will be used as the default value - * for the preference. If a component is a supplier, the supplier will be called at most once to - * get the default instance. Suppliers cannot return {@code null}. - * - * @param preferenceName Preference subtable to use to get the values. - * @param configWithDefaults Record instance with all values set to their preferred default - * values. - * @return An instance of the record class, populated with data from the Preferences table. - * @throws IllegalArgumentException If {@code preferenceName} is empty or contains a {@code '/'}. - * @throws IllegalStateException If {@code preferenceName} was used for a different record class. + * @param preferenceName Subtable name under Preferences (must not contain '/') + * @param configWithDefaults Instance containing default values for all record components + * @return A new record instance populated with Preferences values + * @throws IllegalArgumentException if {@code preferenceName} is empty or contains '/' + * @throws IllegalStateException if {@code preferenceName} was already registered to a different + * record class */ public static T fromPreferences(String preferenceName, T configWithDefaults) { @SuppressWarnings("unchecked") @@ -135,55 +131,41 @@ public static T fromPreferences(String preferenceName, T conf } /** - * Creates a record class instance of the provided type, with fields populated from Preferences. - * - *

To be stored in preferences, the type of the record components can be any of the following: - * - *

+ * Construct a record instance with values loaded from Preferences, using Java defaults if no + * explicit defaults are provided. * - *

The default values for the preferences will be Java defaults (for example, zero for - * integers). + *

For example, an {@code int} component defaults to {@code 0}, and a {@code double} component + * defaults to {@code 0.0}. * - * @param preferenceName Preference subtable to use to get the values. - * @param recordClass Type of the record instance to populate from preferences. - * @return An instance of the record class, populated with data from the Preferences table. - * @throws IllegalArgumentException If {@code preferenceName} is empty or contains a {@code '/'}. - * @throws IllegalStateException If {@code preferenceName} was used for a different record class. + * @param preferenceName Subtable name under Preferences + * @param recordClass Record type to instantiate + * @return A new record instance populated with Preferences values + * @throws IllegalArgumentException if {@code preferenceName} is invalid + * @throws IllegalStateException if namespace was registered to another class */ public static T fromPreferences(String preferenceName, Class recordClass) { return fromPreferences(preferenceName, recordClass, null); } /** - * Creates a record class instance of the provided type, with fields populated from Preferences. + * Internal shared implementation for record construction. * - * @param preferenceName Preference subtable to use to get the values. - * @param recordClass Type of the record instance to populate from preferences. - * @param configWithDefaults Record instance with all values set to their preferred default values - * (can be {@code null}). - * @return An instance of the record class, populated with data from the Preferences table. + * @param preferenceName Preference subtable name + * @param recordClass Record type + * @param configWithDefaults Optional default instance (may be null) */ private static T fromPreferences( String preferenceName, Class recordClass, T configWithDefaults) { deleteLegacyKeys(); - validatePreferenceName(preferenceName); - - // TODO: Use Preferences.getNetworkTable() once there is a release of WPILib that includes it. NetworkTableInstance ntInstance = NetworkTableInstance.getDefault(); + validatePreferenceName(preferenceName); verifyNotRegisteredToAnotherClass(ntInstance, preferenceName, recordClass); try { return createFromPreferences(preferenceName, recordClass, configWithDefaults); } catch (ReflectiveOperationException e) { if (throwExceptions) { - throw new RuntimeException(e); // For self-tests. + throw new RuntimeException(e); // For unit tests } DriverStation.reportWarning( String.format("Could not copy preferences into %s: %s", recordClass.getSimpleName(), e), @@ -192,6 +174,7 @@ private static T fromPreferences( } } + /** Validates that a preference name is legal (non-empty and does not contain path separators). */ private static void validatePreferenceName(String name) { if (name.isEmpty()) { throw new IllegalArgumentException("name cannot be empty"); @@ -202,19 +185,17 @@ private static void validatePreferenceName(String name) { } /** - * Throws an exception if the given record class has been registered under a different name. + * Ensures the given preference namespace is not already registered to another record class. * - * @param ntInstance The network table instance that the preference is published to. - * @param name Preference subtable that will be used to get the values. - * @param recordClass Type of the record instance to populate from preferences. - * @throws IllegalStateException If the subtable of the given name was registered to a different - * class. + *

Each namespace is stored in a special NetworkTable registry entry. If the namespace is new, + * it is bound to the current record type. If it already exists and points to a different record + * type, this method throws an exception. */ private static void verifyNotRegisteredToAnotherClass( NetworkTableInstance ntInstance, String name, Class recordClass) { String recordName = recordClass.getCanonicalName(); if (recordName == null) { - recordName = recordClass.getName(); + recordName = recordClass.getName(); // fallback if canonical name unavailable } NetworkTable registeredClassesTable = ntInstance.getTable(REGISTERED_CLASSES_NETWORK_TABLE_KEY); @@ -233,13 +214,8 @@ private static void verifyNotRegisteredToAnotherClass( } /** - * Creates an instance of the given type using the provided default values. - * - * @param prefix String to prepend to record field names to get the Preference key. - * @param clazz Record class type. - * @param configWithDefaults Default values to use if there are no values stored in NetworkTables. - * @return An instance of the record class, populated with data from the Preferences table. - * @throws ReflectiveOperationException If the fields of the class cannot be read via reflection. + * Core factory logic: instantiate a record using reflection, reading each component value from + * Preferences (or defaults). */ private static T createFromPreferences( String prefix, Class clazz, T configWithDefaults) @@ -247,35 +223,38 @@ private static T createFromPreferences( var components = clazz.getRecordComponents(); Object[] params = new Object[components.length]; Class[] types = new Class[components.length]; + int i = 0; for (RecordComponent component : components) { String name = component.getName(); - String key = prefix + PATH_SEPARATOR + name; + String key = prefix + PATH_SEPARATOR + name; // Preference key = namespace/componentName Class type = component.getType(); types[i] = type; boolean needComponentValue; PreferenceFactory factory = null; boolean isRecordField = Record.class.isAssignableFrom(type); + if (isRecordField) { + // Nested record: recurse into sub-record needComponentValue = true; } else { factory = TYPE_TO_FACTORY.get(type); if (factory == null) { - // Cannot get value from Preferences; copy over the value from the input record. + // Unsupported type: fall back to copying default value needComponentValue = true; } else { + // If no key exists, we need the component value for initialization needComponentValue = !Preferences.containsKey(key); } } Object componentValue = null; - if (needComponentValue) { - if (configWithDefaults != null) { - Field defaultValueField = clazz.getDeclaredField(name); - defaultValueField.setAccessible(true); - componentValue = defaultValueField.get(configWithDefaults); - } + if (needComponentValue && configWithDefaults != null) { + // Use reflection to get the field value from the default record instance + Field defaultValueField = clazz.getDeclaredField(name); + defaultValueField.setAccessible(true); + componentValue = defaultValueField.get(configWithDefaults); } if (isRecordField) { @@ -284,63 +263,35 @@ private static T createFromPreferences( warn("Cannot store '%s' in Preferences; type %s is unsupported", name, type); params[i] = componentValue; } else { - // Fetch the value from Preferences + // Use registered factory to create the value (from Preferences or defaults) params[i] = factory.create( component, key, componentValue, /* initializePreference= */ needComponentValue); } i++; } + Constructor constructor = clazz.getDeclaredConstructor(types); constructor.setAccessible(true); return constructor.newInstance(params); } - /** - * Type-safe functional interface for creating an instance of a type using data in Preferences. - */ - @FunctionalInterface - private interface GenericPreferenceFactory { - /** - * Gets a value from Preferences for the given component. - * - * @param component Provides dynamic access to the component of the record class. - * @param key The Preference key that should be used when initializing the Preference. - * @param defaultValue The default value that should be used when initializing the Preference. - * @param initializePreference Whether the preference should be initialized. - * @return The value; will match the type in "component"; - */ - T create(RecordComponent component, String key, T defaultValue, boolean initializePreference); - } - - /** - * Functional interface for creating an instance of a type using data in Preferences. - * - *

Note: this interface exists to avoid ugly casts in the code that uses the reflection APIs. - */ + /** Functional interface mapping a record component into a preference-backed value. */ @FunctionalInterface private interface PreferenceFactory { - /** - * Gets a value from Preferences for the given component. - * - * @param component Provides dynamic access to the component of the record class. - * @param key The Preference key that should be used when initializing the Preference. - * @param defaultValue The default value that should be used when initializing the Preference. - * @param initializePreference Whether the preference should be initialized. - * @return The value; will match the type in "component"; - */ Object create( RecordComponent component, String key, Object defaultValue, boolean initializePreference); } + /** Generic variant of {@link PreferenceFactory} with typed defaults. */ + @FunctionalInterface + private interface GenericPreferenceFactory { + T create(RecordComponent component, String key, T defaultValue, boolean initializePreference); + } + + /** Registry of supported record component types to their corresponding factories. */ private static final Map TYPE_TO_FACTORY = new HashMap<>(); - /** - * Registers a preference factory with a type. - * - * @param type The type to register. - * @param simpleFactory The factory that should be used to create values of the given type. - */ @SuppressWarnings("unchecked") private static void register(Class type, GenericPreferenceFactory simpleFactory) { PreferenceFactory factory = @@ -349,6 +300,7 @@ private static void register(Class type, GenericPreferenceFactory simp TYPE_TO_FACTORY.put(type, factory); } + // Static initialization: register supported primitive and supplier types static { register(Boolean.TYPE, PersistedConfiguration::booleanFactory); register(BooleanSupplier.class, PersistedConfiguration::booleanSupplierFactory); @@ -362,7 +314,7 @@ private static void register(Class type, GenericPreferenceFactory simp register(Supplier.class, PersistedConfiguration::supplierFactory); } - /** Maps the generic types supported by Preferences to their primitive types. */ + /** Maps boxed types to the primitive/supported type used for preferences. */ private static final Map SUPPLIER_TYPE_TO_REGISTERED_TYPE = Map.of( Boolean.class, Boolean.TYPE, @@ -371,7 +323,10 @@ private static void register(Class type, GenericPreferenceFactory simp Double.class, Double.TYPE, String.class, String.class); - /** Gets a boolean value from Preferences for the given component. */ + // =============================== + // FACTORY IMPLEMENTATIONS + // =============================== + private static boolean booleanFactory( RecordComponent component, String key, Boolean defaultValue, boolean initialize) { if (initialize) { @@ -391,16 +346,15 @@ private static BooleanSupplier booleanSupplierFactory( BooleanSupplier defaultValueSupplier, boolean initialize) { if (initialize) { - boolean defaultValue = false; - if (defaultValueSupplier != null) { - defaultValue = defaultValueSupplier.getAsBoolean(); - } + boolean defaultValue = + defaultValueSupplier != null + ? defaultValueSupplier.getAsBoolean() + : false; // ternary java operation for people who don't know Preferences.initBoolean(key, defaultValue); } return () -> Preferences.getBoolean(key, false); } - /** Gets an int value from Preferences for the given component. */ private static int intFactory( RecordComponent component, String key, Integer defaultValue, boolean initialize) { if (initialize) { @@ -413,7 +367,6 @@ private static int intFactory( return Preferences.getInt(key, 0); } - /** Gets a IntSupplier value from Preferences for the given component. */ private static IntSupplier intSupplierFactory( RecordComponent component, String key, IntSupplier defaultValueSupplier, boolean initialize) { if (initialize) { @@ -423,7 +376,6 @@ private static IntSupplier intSupplierFactory( return () -> Preferences.getInt(key, 0); } - /** Gets a long value from Preferences for the given component. */ private static long longFactory( RecordComponent component, String key, Long defaultValue, boolean initialize) { if (initialize) { @@ -436,7 +388,6 @@ private static long longFactory( return Preferences.getLong(key, 0); } - /** Gets a LongSupplier value from Preferences for the given component. */ private static LongSupplier longSupplierFactory( RecordComponent component, String key, @@ -449,7 +400,6 @@ private static LongSupplier longSupplierFactory( return () -> Preferences.getLong(key, 0); } - /** Gets a double value from Preferences for the given component. */ private static double doubleFactory( RecordComponent component, String key, Double defaultValue, boolean initialize) { if (initialize) { @@ -462,7 +412,6 @@ private static double doubleFactory( return Preferences.getDouble(key, 0); } - /** Gets a DoubleSupplier value from Preferences for the given component. */ private static DoubleSupplier doubleSupplierFactory( RecordComponent component, String key, @@ -475,7 +424,6 @@ private static DoubleSupplier doubleSupplierFactory( return () -> Preferences.getDouble(key, 0); } - /** Gets a String value from Preferences for the given component. */ private static String stringFactory( RecordComponent component, String key, String defaultValue, boolean initialize) { if (initialize) { @@ -488,10 +436,6 @@ private static String stringFactory( return Preferences.getString(key, ""); } - /** - * Gets a Supplier value from Preferences for the given component. Supports String, long, int, - * boolean and float values. - */ private static Supplier supplierFactory( RecordComponent component, String key, Supplier defaultValueSupplier, boolean initialize) { Type supplierType = @@ -514,19 +458,18 @@ private static Supplier supplierFactory( return defaultValueSupplier; } } - factory.create( - component, key, defaultValue, true); // Call Preferences.init{String,Double,etc}() + factory.create(component, key, defaultValue, true); } return () -> factory.create(component, key, null, false); } - /** Deletes Preferences that were created by older versions of this class. */ + /** + * Removes legacy ".registeredTo" keys from Preferences (used by older versions of this class). + * Ensures backward compatibility without polluting the Preferences namespace. + */ private static void deleteLegacyKeys() { if (!deletedLegacyKeys) { - // Preferences installs a listener that makes all new topics persistent. The ".registeredTo" - // topics used to be placed under /Preferences, so could have been persisted. They are now - // written under a different top-level table. Delete the topics created by the previous code. for (var key : Preferences.getKeys()) { if (key.endsWith(".registeredTo")) { Preferences.remove(key); @@ -536,17 +479,13 @@ private static void deleteLegacyKeys() { } } - /** - * Emits a warning, usually via {@link DataLogManager}. - * - * @param format A format string as described in {@link java.util.Formatter Formatter}. - * @param args Arguments referenced by the format specifiers in the format string. - */ + /** Utility method for reporting warnings. Logs to the configured {@link #errorReporter}. */ private static void warn(String format, Object... args) { String message = String.format("WARNING: " + format, args); errorReporter.accept(message); } + /** Private constructor to prevent instantiation. */ private PersistedConfiguration() { throw new AssertionError("Not instantiable"); } diff --git a/lib/src/main/java/com/team2813/lib2813/subsystems/MotorSubsystem.java b/lib/src/main/java/com/team2813/lib2813/subsystems/MotorSubsystem.java index c33a905f..552c117d 100644 --- a/lib/src/main/java/com/team2813/lib2813/subsystems/MotorSubsystem.java +++ b/lib/src/main/java/com/team2813/lib2813/subsystems/MotorSubsystem.java @@ -1,112 +1,111 @@ package com.team2813.lib2813.subsystems; -import static edu.wpi.first.units.Units.Amps; -import static edu.wpi.first.units.Units.Rotations; - import com.team2813.lib2813.control.ControlMode; import com.team2813.lib2813.control.Encoder; import com.team2813.lib2813.control.Motor; import com.team2813.lib2813.control.PIDMotor; import edu.wpi.first.math.controller.PIDController; -import edu.wpi.first.networktables.BooleanPublisher; -import edu.wpi.first.networktables.DoublePublisher; -import edu.wpi.first.networktables.NetworkTable; -import edu.wpi.first.networktables.NetworkTableInstance; import edu.wpi.first.units.AngleUnit; import edu.wpi.first.units.Units; import edu.wpi.first.units.measure.Angle; import edu.wpi.first.units.measure.AngularVelocity; import edu.wpi.first.units.measure.Current; -import edu.wpi.first.wpilibj2.command.Command; -import edu.wpi.first.wpilibj2.command.InstantCommand; import edu.wpi.first.wpilibj2.command.SubsystemBase; import java.util.Objects; import java.util.function.Supplier; /** - * Defines PID control over a motor, with values specified by an encoder + * Abstract subsystem that manages a motor with PID control based on encoder measurements. + * + *

This class allows: + * + *

* - * @param the type of the {@link Supplier} used to specify setpoints. + * @param the type of {@link Supplier} used to provide dynamic setpoints. */ public abstract class MotorSubsystem> extends SubsystemBase implements Motor, Encoder { + /** The motor used by this subsystem */ protected final Motor motor; + + /** The encoder providing feedback for PID calculations */ protected final Encoder encoder; + + /** The control mode used when applying outputs to the motor */ protected final ControlMode controlMode; + + /** The angle unit for measurements and setpoints */ protected final AngleUnit rotationUnit; + + /** Maximum allowed position error for "at position" checks */ protected final double acceptableError; + + /** The PID controller managing the motor to reach the setpoint */ protected final PIDController controller; - private final DoublePublisher positionPublisher; - private final BooleanPublisher atPositionPublisher; - private final DoublePublisher appliedCurrentPublisher; - private final DoublePublisher setpointPublisher; + private double setpoint; private boolean isEnabled; + /** + * Constructs a MotorSubsystem using the provided configuration. + * + * @param builder the configuration for the motor subsystem + */ protected MotorSubsystem(MotorSubsystemConfiguration builder) { + this.setpoint = builder.startingPosition; this.controller = builder.controller; this.controller.setTolerance(builder.acceptableError); - acceptableError = builder.acceptableError; - motor = builder.motor; - encoder = builder.encoder; - controlMode = builder.controlMode; - rotationUnit = builder.rotationUnit; - if (builder.ntInstance != null) { - NetworkTable networkTable = builder.ntInstance.getTable(getName()); - positionPublisher = networkTable.getDoubleTopic("position").publish(); - atPositionPublisher = networkTable.getBooleanTopic("at position").publish(); - appliedCurrentPublisher = networkTable.getDoubleTopic("applied current").publish(); - setpointPublisher = networkTable.getDoubleTopic("setpoint").publish(); - } else { - positionPublisher = null; - atPositionPublisher = null; - appliedCurrentPublisher = null; - setpointPublisher = null; - } - - this.controller.setSetpoint(builder.startingPosition); + this.acceptableError = builder.acceptableError; + this.motor = builder.motor; + this.encoder = builder.encoder; + this.controlMode = builder.controlMode; + this.rotationUnit = builder.rotationUnit; } /** - * Sets the desired setpoint to the provided value, and enables the PID control. + * Sets the target position for this subsystem and enables PID control if not already enabled. * - * @param position the position to go to. + * @param setpoint the supplier of the target angle */ - public final void setSetpoint(T position) { + public void setSetpoint(T setpoint) { if (!isEnabled()) { enable(); } - double setpoint = position.get().in(rotationUnit); - controller.setSetpoint(setpoint); + this.setpoint = setpoint.get().in(rotationUnit); + controller.setSetpoint(this.setpoint); } /** - * Returns a command that sets the desired setpoint to the provided value. + * Returns the current PID setpoint. * - * @param setpoint the position to go to. + * @return the target setpoint as an {@link Angle} */ - public final Command setSetpointCommand(T setpoint) { - return new InstantCommand(() -> this.setSetpoint(setpoint), this); - } - - /** Returns the current setpoint as an angle. */ - public final Angle getSetpoint() { - return rotationUnit.of(controller.getSetpoint()); + public Angle getSetpoint() { + return rotationUnit.of(setpoint); } - /** Determines if the motor is at the current setpoint, within the acceptable error. */ - public final boolean atPosition() { - return Math.abs(getMeasurement() - controller.getSetpoint()) <= acceptableError; + /** + * Checks whether the subsystem is within the acceptable error of the setpoint. + * + * @return {@code true} if within acceptable error, {@code false} otherwise + */ + public boolean atPosition() { + return Math.abs(getMeasurement() - setpoint) <= acceptableError; } /** * {@inheritDoc} * - *

Additionally, this method disables PID control of the subsystem + *

Also disables PID control if enabled. */ @Override - public final void set(ControlMode mode, double demand, double feedForward) { + public void set(ControlMode mode, double demand, double feedForward) { if (isEnabled()) { disable(); } @@ -114,146 +113,107 @@ public final void set(ControlMode mode, double demand, double feedForward) { } @Override - public final Current getAppliedCurrent() { + public Current getAppliedCurrent() { return motor.getAppliedCurrent(); } - /** - * Enables the motor - * - *

The motor voltage will be periodically updated to move the motor towards the current - * setupoint. - */ - public final void enable() { + /** Enables PID control of the subsystem. */ + public void enable() { isEnabled = true; } - /** - * Stops the motor. - * - *

The motor voltage will be set to zero, and the motor will not adjust to move towards the - * current setpoint. - */ - public final void disable() { + /** Disables PID control and sets motor output to zero. */ + public void disable() { isEnabled = false; - motor.disable(); + useOutput(0, 0); } /** - * Returns whether the controller is enabled. If this is enabled, then PID control will be used. + * Returns whether PID control is currently enabled. * - * @return Whether the controller is enabled. + * @return {@code true} if enabled, {@code false} otherwise */ - public final boolean isEnabled() { + public boolean isEnabled() { return isEnabled; } /** * {@inheritDoc} * - *

Additionally, this method disables PID control of the subsystem. It does not clamp - * the provided value. + *

Also disables PID control if enabled. */ @Override - public final void set(ControlMode mode, double demand) { - isEnabled = false; + public void set(ControlMode mode, double demand) { + if (isEnabled()) { + disable(); + } motor.set(mode, demand); } /** - * Clamps the given output value and provides it to the motor. - * - *

This was protected and non-final to allow subclasses to clamp the output. Subclasses should - * override {@link #clampOutput(double)}. + * Applies the PID output to the motor. Can be overridden for advanced control. * - * @param output The output calculated by the PID algorithm. - * @param setpoint Ignored. - * @deprecated Subclasses should override {@link #clampOutput(double)}. + * @param output the PID output value + * @param setpoint the target setpoint (for reference) */ - @Deprecated protected void useOutput(double output, double setpoint) { - motor.set(controlMode, clampOutput(output)); + motor.set(controlMode, output); } - /** - * Clamps the given output value and provides it to the motor. - * - *

This is called by {@link #periodic()} if this subsystem is enabled. - */ - private void useOutput(double output) { - useOutput(output, controller.getSetpoint()); - } - - /** - * Extension point that allows subclasses to clamp the output. - * - *

The default implementation returns the provided value. - * - * @param output Output provided by the PID controller. - * @return Output to provide to the motor. - * @see edu.wpi.first.math.MathUtil#clamp(double, double, double) - */ - protected double clampOutput(double output) { - return output; - } - - protected final double getMeasurement() { + /** Returns the encoder measurement converted to the configured {@link AngleUnit}. */ + protected double getMeasurement() { return encoder.getPositionMeasure().in(rotationUnit); } @Override - @Deprecated(forRemoval = true) + @Deprecated public double position() { return encoder.position(); } - @Override - public final Angle getPositionMeasure() { + public Angle getPositionMeasure() { return encoder.getPositionMeasure(); } @Override - @Deprecated(forRemoval = true) + @Deprecated public void setPosition(double position) { encoder.setPosition(position); } - @Override - public final void setPosition(Angle position) { + public void setPosition(Angle position) { encoder.setPosition(position); } @Override - @Deprecated(forRemoval = true) + @Deprecated public double getVelocity() { return encoder.getVelocity(); } - @Override - public final AngularVelocity getVelocityMeasure() { + public AngularVelocity getVelocityMeasure() { return encoder.getVelocityMeasure(); } - /** Applies the PID output to the motor if this subsystem is enabled. */ + /** Periodic update that applies PID output if enabled. */ @Override public void periodic() { if (isEnabled) { - useOutput(controller.calculate(getMeasurement())); - } - if (positionPublisher != null) { - positionPublisher.set(getPositionMeasure().in(Rotations)); - setpointPublisher.set(getSetpoint().in(Rotations)); - atPositionPublisher.set(atPosition()); - appliedCurrentPublisher.set(getAppliedCurrent().in(Amps)); + useOutput(controller.calculate(getMeasurement()), setpoint); } } - /** A configuration for a MotorSubsystem */ + /** + * Builder-style configuration class for {@link MotorSubsystem}. + * + *

Allows setting PID constants, control mode, starting position, and other configuration + * parameters. + */ public static class MotorSubsystemConfiguration { - /** The default acceptable position error. */ + /** Default allowed error for position checks */ public static final double DEFAULT_ERROR = 5.0; - /** The default starting position if one is not provided. */ + /** Default starting position */ public static final double DEFAULT_STARTING_POSITION = 0.0; private ControlMode controlMode; @@ -263,12 +223,13 @@ public static class MotorSubsystemConfiguration { private PIDController controller; private double acceptableError; private double startingPosition; - private NetworkTableInstance ntInstance; /** - * Creates a new configuration for a MotorSubsystems. The default acceptable error is {@value - * #DEFAULT_ERROR}, the PID constants are set to 0, and the starting position is {@value - * #DEFAULT_STARTING_POSITION} + * Creates a new configuration for a motor subsystem. + * + *

Defaults: PID(0,0,0), acceptable error = {@value #DEFAULT_ERROR}, starting position = + * {@value #DEFAULT_STARTING_POSITION}, control mode = {@link ControlMode#DUTY_CYCLE}, angle + * unit = {@link Units#Rotations}. * * @param motor the motor to control * @param encoder the encoder providing feedback @@ -284,82 +245,42 @@ public MotorSubsystemConfiguration(Motor motor, Encoder encoder) { } /** - * Creates a new config for MotorSubsystems using a motor that has a built-in encoder. The - * default acceptable error is {@value #DEFAULT_ERROR}, the PID constants are set to 0, and the - * starting position is {@value #DEFAULT_STARTING_POSITION} + * Creates a new configuration using a PIDMotor with a built-in encoder. * - * @param motor the integrated motor controller + * @param motor the motor that implements {@link PIDMotor} */ public MotorSubsystemConfiguration(PIDMotor motor) { this(motor, motor); } - /** - * Sets the controller used to calculate the next value - * - * @param controller The PID controller - * @return {@code this} for chaining - */ + /** Sets the PID controller to use. */ public MotorSubsystemConfiguration controller(PIDController controller) { this.controller = controller; return this; } - /** - * Sets the control mode to use when giving output to the motor. Defaults to {@link - * ControlMode#DUTY_CYCLE}. - * - * @param controlMode The mode to use when controlling the motor - * @return {@code this} for chaining - * @throws IllegalArgumentException If {@code controlMode} is for positional control - */ + /** Sets the control mode for motor output. */ public MotorSubsystemConfiguration controlMode(ControlMode controlMode) { - if (controlMode.isPositionalControl()) { - throw new IllegalArgumentException( - String.format( - "Control mode %s is for positional control. This is invalid! Please use a different" - + " control mode", - controlMode)); - } this.controlMode = controlMode; return this; } - /** - * Sets the PID constants for the controller - * - * @param p the proportional - * @param i the integral - * @param d the derivative - * @return {@code this} for chaining - */ + /** Sets the PID constants. */ public MotorSubsystemConfiguration PID(double p, double i, double d) { controller.setPID(p, i, d); return this; } - /** - * Sets the initial setpoint of the controller - * - * @param startingPosition the initial setpoint - * @return {@code this} for chaining - */ + /** Sets the starting position in the configured angle unit. */ public MotorSubsystemConfiguration startingPosition(Angle startingPosition) { this.startingPosition = startingPosition.in(this.rotationUnit); return this; } - /** - * Sets the initial setpoint of the controller from the current value of a supplier. - * - *

This is provided to allow the subclass to define an {@code Enum} (that implements {@code - * Supplier}) which defines the supported positions of this subsystem. - * - * @param startingPositionSupplier supplier to use to get the initial setpoint - * @return {@code this} for chaining - */ - public MotorSubsystemConfiguration startingPosition(Supplier startingPositionSupplier) { - return startingPosition(startingPositionSupplier.get()); + /** Sets the starting position via a supplier of an angle. */ + public MotorSubsystemConfiguration startingPosition(Supplier startingPosition) { + this.startingPosition = startingPosition.get().in(this.rotationUnit); + return this; } /** Sets the acceptable position error. */ @@ -368,20 +289,11 @@ public MotorSubsystemConfiguration acceptableError(double error) { return this; } - /** - * Sets the unit to use for PID calculations - * - * @param rotationUnit The angle unit to use for calculations - */ + /** Sets the angle unit for PID calculations. */ public MotorSubsystemConfiguration rotationUnit(AngleUnit rotationUnit) { startingPosition = rotationUnit.convertFrom(startingPosition, this.rotationUnit); this.rotationUnit = rotationUnit; return this; } - - public MotorSubsystemConfiguration publishTo(NetworkTableInstance ntInstance) { - this.ntInstance = ntInstance; - return this; - } } } diff --git a/lib/src/main/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystem.java b/lib/src/main/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystem.java index ad890292..b8538a73 100644 --- a/lib/src/main/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystem.java +++ b/lib/src/main/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystem.java @@ -3,18 +3,37 @@ import com.google.auto.value.AutoBuilder; import com.team2813.lib2813.control.ControlMode; import com.team2813.lib2813.control.Motor; +import com.team2813.lib2813.control.PIDMotor; import edu.wpi.first.wpilibj2.command.Command; import edu.wpi.first.wpilibj2.command.InstantCommand; import edu.wpi.first.wpilibj2.command.SubsystemBase; +/** + * A subsystem for an intake mechanism with parameterized motor control. + * + *

This subsystem uses a {@link Motor} and allows controlling the motor in intake and outtake + * directions with preconfigured demand values. Commands are provided for common actions such as + * intaking, outtaking, and stopping the motor. + */ public abstract class ParameterizedIntakeSubsystem extends SubsystemBase implements AutoCloseable { - private final Motor intakeMotor; + + private final PIDMotor intakeMotor; private final Params params; + /** + * Configuration record that stores the parameters for the intake subsystem. + * + *

Includes the motor control mode, the intake demand, and the outtake demand. + * + *

Validation ensures the intake and outtake demands are non-zero and have opposite signs. + */ public record Params(ControlMode controlMode, double intakeDemand, double outtakeDemand) { + /** Returns a builder for creating {@link Params}. Defaults the control mode to VOLTAGE. */ public static Params.Builder builder() { - return new AutoBuilder_ParameterizedIntakeSubsystem_Params_Builder() + return new AutoBuilder_ParameterizedIntakeSubsystem_Params_Builder() // autogenerated class, + // ignore if it shows an + // error in the ide .setControlMode(ControlMode.VOLTAGE); } @@ -29,6 +48,15 @@ public interface Builder { Params build(); } + /** + * Constructs a Params record. + * + * @param controlMode the motor control mode, cannot be {@code null} + * @param intakeDemand the motor demand for intake, must be non-zero + * @param outtakeDemand the motor demand for outtake, must be non-zero and opposite sign of + * intakeDemand + * @throws IllegalArgumentException if validation fails + */ public Params { if (controlMode == null) { throw new IllegalArgumentException("controlMode cannot be null"); @@ -46,60 +74,85 @@ public interface Builder { } } - protected ParameterizedIntakeSubsystem(Motor intakeMotor, Params params) { + /** + * Constructs a new intake subsystem. + * + * @param intakeMotor the motor controlling the intake mechanism + * @param params configuration parameters for motor control + */ + protected ParameterizedIntakeSubsystem(PIDMotor intakeMotor, Params params) { this.intakeMotor = intakeMotor; this.params = params; } + /** + * Returns a command that spins the intake wheels in the intake direction. + * + * @return an {@link Command} that intakes a game piece + */ public final Command intakeItemCommand() { return new InstantCommand(this::intakeGamePiece, this); } + /** + * Returns a command that spins the intake wheels in the outtake direction. + * + * @return an {@link InstantCommand} that outtakes a game piece + */ public final Command outtakeItemCommand() { return new InstantCommand(this::outtakeGamePiece, this); } + /** + * Returns a command that stops the intake motor. + * + * @return an {@link InstantCommand} that stops the motor + */ public final Command stopMotorCommand() { return new InstantCommand(this::stopMotor, this); } - /** Makes intake wheels spin in the intake direction. */ + /** Spins the intake wheels in the configured intake direction. */ protected final void intakeGamePiece() { // FIXME: Maybe add a check that the wheels are not stalled. setMotorDemand(params.intakeDemand); } - /** Makes intake wheels spin in the outtake direction. */ + /** Spins the intake wheels in the configured outtake direction. */ protected final void outtakeGamePiece() { setMotorDemand(params.outtakeDemand); } /** - * Runs the motor with the provided demand value. + * Runs the intake motor with a specific demand. * - * @param demand Demand of the motor. Meaning depends on the {@code ControlMode}. + * @param demand the motor demand, meaning depends on the configured {@link ControlMode} */ protected final void setMotorDemand(double demand) { intakeMotor.set(params.controlMode, demand); } /** - * Returns a command that runs the motor with the provided demand value. + * Returns a command that runs the intake motor with the given demand. * - * @param demand Demand of the motor. Meaning depends on the {@code ControlMode}. + * @param demand the motor demand, meaning depends on the configured {@link ControlMode} + * @return a command that sets the motor demand when executed */ protected final Command setMotorDemandCommand(double demand) { return new InstantCommand(() -> setMotorDemand(demand), this); } - /** Stops the motor. */ + /** Stops the intake motor. */ public final void stopMotor() { setMotorDemand(0.0); } @Override - public void close() {} + public void close() { + // Default implementation does nothing. Override if resources need cleanup. + } + /** Returns whether a value is essentially zero (within 0.001). */ private static boolean isEssentiallyZero(double value) { return Math.abs(value) < 0.001; } diff --git a/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/Lightshow.java b/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/Lightshow.java index 8c743d40..d251b9d3 100644 --- a/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/Lightshow.java +++ b/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/Lightshow.java @@ -1,82 +1,107 @@ package com.team2813.lib2813.subsystems.lightshow; import edu.wpi.first.wpilibj.util.Color; -import edu.wpi.first.wpilibj2.command.Subsystem; import edu.wpi.first.wpilibj2.command.SubsystemBase; import java.util.Arrays; import java.util.HashSet; import java.util.Optional; import java.util.Set; -import java.util.function.Consumer; +/** + * Abstract base class for robot lightshow subsystems. + * + *

A Lightshow subsystem manages a set of {@link State}s that determine the colors to be + * displayed on LEDs or other lighting hardware. Implementations define how to actually apply the + * color via {@link #useColor(Color)} and how to compute the active color in {@link #update()}. + * + *

States can be represented as enums implementing {@link State}, or as arbitrary {@link State} + * instances. + * + *

There is a built-in static {@link #off} state that always represents the lights being off. + * + * @author Team 2813 + */ public abstract class Lightshow extends SubsystemBase { - /** A static state that signifies that the lights should be off, that is always applied */ + + /** A static state representing that the lights should be off (always applied). */ protected static final State off = new State() { + @Override public Color color() { return new Color(0, 0, 0); } + @Override public boolean apply() { return true; } }; - protected Set states = new HashSet<>(); - protected Optional defaultState; + /** The set of active states that the lightshow considers when updating the color. */ + protected final Set states = new HashSet<>(); + + /** The default state to use if no other state is active. */ + protected Optional defaultState = Optional.empty(); /** - * Creates a new Lightshow subsystem using an enum. Uses the given {@code enumClass} to get a list - * of {@link State}s to use. + * Constructs a Lightshow using an enum class representing the states. * - * @param The type of enum that implements state - * @param enumClass The enum class that contains the states + * @param the type of enum implementing {@link State} + * @param enumClass the enum class containing the states */ protected & State> Lightshow(Class enumClass) { addStates(enumClass); } /** - * Creates a new Lightshow subsystem with a set of states. + * Constructs a Lightshow with a set of states. * - * @param states The states + * @param states the initial states for this Lightshow */ protected Lightshow(Set states) { addStates(states); } + /** + * Implementation-defined method to actually use a {@link Color}. + * + * @param c the color to display + */ protected abstract void useColor(Color c); + /** + * Adds states from an enum class implementing {@link State}. + * + * @param the enum type implementing {@link State} + * @param enumClass the enum class containing the states + */ public final & State> void addStates(Class enumClass) { T[] constants = enumClass.getEnumConstants(); states.addAll(Arrays.asList(constants)); } + /** + * Adds arbitrary {@link State} instances to the Lightshow. + * + * @param states the states to add + */ public final void addStates(Set states) { this.states.addAll(states); } /** - * Updates the current States. Makes the current {@link #states} updated, and gets the Color to be - * displayed. + * Updates the current active {@link State}s and determines the color to display. * - * @return an optional representing the color to be displayed - * @implSpec Do anything with {@link #states} to determine which {@link Color} to display - * @see #periodic() + * @return an optional containing the color to display, or empty if no state is active */ protected abstract Optional update(); /** - * {@inheritDoc} - * - *

This implementation of {@link Subsystem#periodic()} updates the colors by passing the call - * of {@link #update()} to the given {@link Consumer}. If the call to {@link #update()} returns an - * empty optional, than the color of the default state is used, or the color is not changed. + * Called periodically by the scheduler to update the lights. * - * @implSpec {@link #useColor(Color)} is passed the result of {@link #update()}, or the {@link - * #defaultState}'s color. If there is no color from {@link #update()} and no default state, - * there is no requirement for what to pass to {@link #useColor(Color)}, or if {@link - * #useColor(Color)} is called at all + *

This implementation calls {@link #update()}, and applies the resulting color using {@link + * #useColor(Color)}. If {@link #update()} returns empty, the {@link #defaultState}'s color is + * used (if present). If neither provides a color, {@link #useColor(Color)} may not be called. */ @Override public void periodic() { @@ -85,9 +110,9 @@ public void periodic() { } /** - * Sets the {@link State} to be used if none is given. + * Sets the default {@link State} to use if no other state is active. * - * @param defaultState the {@link State} to be used as a default. + * @param defaultState the default state */ public void setDefaultState(State defaultState) { this.defaultState = Optional.of(defaultState); diff --git a/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/QueueLightshow.java b/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/QueueLightshow.java index 573240f1..a375b4b3 100644 --- a/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/QueueLightshow.java +++ b/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/QueueLightshow.java @@ -7,34 +7,64 @@ import java.util.Set; /** - * A lightshow that keeps track of the states that have been applied, and uses the last state. To be - * more specific, all states that return {@code true} on a call to {@link State#apply()} are added - * to the list. States are only removed from the list if they are at the front and {@link - * State#apply()} returns {@code false}, when a state is removed, the next one will be activated if - * {@link State#apply()} returns {@code true}, until either a state returns {@code true} upon a call - * to {@link State#apply()}, in which the color will be used, or there are no states where {@link - * State#apply()} return {@code true}, then the default color is used. + * A Lightshow implementation that manages a queue of active states. + * + *

Behavior: + * + *

    + *
  • All {@link State}s that return {@code true} for {@link State#apply()} are added to a queue. + *
  • States are removed from the front of the queue if {@link State#apply()} returns {@code + * false}. + *
  • The last active state in the queue is used to determine the color to display. + *
  • If no states are active, the {@link Lightshow#off} state or {@link #defaultState} is used. + *
+ * + * @author Team 2813 */ public abstract class QueueLightshow extends Lightshow { + private final Deque activatedStates = new ArrayDeque<>(); + /** + * Creates a QueueLightshow from an enum class of {@link State}s. + * + * @param the enum type implementing {@link State} + * @param enumClass the enum class containing the states + */ public & State> QueueLightshow(Class enumClass) { super(enumClass); defaultState = Optional.of(Lightshow.off); } + /** + * Creates a QueueLightshow from a set of {@link State}s. + * + * @param states the set of states to use + */ public QueueLightshow(Set states) { super(states); defaultState = Optional.of(Lightshow.off); } + /** + * Updates the queue of active states and returns the current color to display. + * + *

New states that return {@code true} for {@link State#apply()} are added to the front of the + * queue. States at the front of the queue that return {@code false} are removed. The color of the + * first active state is used. + * + * @return the color of the current active state, or empty if no states are active + */ @Override protected Optional update() { + // Add new active states that are not already in the queue for (State s : states) { if (!activatedStates.contains(s) && s.apply()) { activatedStates.addFirst(s); } } + + // Remove inactive states from the front while (!activatedStates.isEmpty()) { State s = activatedStates.poll(); if (s.apply()) { @@ -42,6 +72,7 @@ protected Optional update() { return Optional.of(s.color()); } } + return Optional.empty(); } } diff --git a/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/State.java b/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/State.java index ce4e6874..436c9c34 100644 --- a/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/State.java +++ b/lib/src/main/java/com/team2813/lib2813/subsystems/lightshow/State.java @@ -2,18 +2,27 @@ import edu.wpi.first.wpilibj.util.Color; +/** + * Represents a Lightshow state with a color and activation condition. + * + *

Each {@link State} provides a color and a condition indicating whether it should currently be + * applied. + * + * @author Team 2813 + */ public interface State { + /** - * gets the color of this State. + * Gets the color of this state. * - * @return the color of this State + * @return the color to display for this state */ Color color(); /** - * Checks if the current state should be applied + * Checks if this state should currently be applied. * - * @return {@code true} if the state should be applied + * @return {@code true} if the state should be active, {@code false} otherwise */ boolean apply(); } diff --git a/lib/src/main/java/com/team2813/lib2813/util/BuildConstants.java b/lib/src/main/java/com/team2813/lib2813/util/BuildConstants.java index fcd2a2c1..8b80f71a 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/BuildConstants.java +++ b/lib/src/main/java/com/team2813/lib2813/util/BuildConstants.java @@ -2,7 +2,11 @@ import java.time.ZonedDateTime; -/** Holder for data collected at build time about the robot code. */ +/** + * Holder for data collected at build time about the robot code. + * + * @author Team 2813 + */ public interface BuildConstants { /** The current git branch when the code was built. */ diff --git a/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsPublisher.java b/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsPublisher.java index 7d59e1ca..7111ba86 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsPublisher.java +++ b/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsPublisher.java @@ -20,7 +20,7 @@ *

The constants are published under the {@code "/Metadata"} table in NetworkTables. This is a * special NetworkTables table. Some tools have special support for the "/Metadata" table. For * instance, Advantage Scope has a dedicated Metadata tab that loads information like Build - * Constants in a well formated table view. + * Constants in a well formatted table view. * *

To instantiate a BuildConstantsPublisher, a build constants class, {@code BuildConstants}, is * needs to be generated for the robot library by enabling the `gversion` plugin in the gradle build @@ -37,6 +37,8 @@ * // Log the build constants in the robot console as well. * buildConstantsPublisher.log(); * } + * + * @author Team 2813 */ public final class BuildConstantsPublisher { /** The name of the NetworkTable under which the build constants are published. */ diff --git a/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsRecord.java b/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsRecord.java index 56b2a6d5..b65ec7f5 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsRecord.java +++ b/lib/src/main/java/com/team2813/lib2813/util/BuildConstantsRecord.java @@ -6,6 +6,18 @@ import java.time.format.DateTimeParseException; import java.util.Optional; +/** + * Record implementation of {@link BuildConstants}. + * + *

This class holds metadata about the build, such as git branch, commit hash, build time, etc. + * It is typically generated at build time using the gversion Gradle plugin and then extracted at + * runtime for diagnostics, publishing, or logging. + * + *

All values are immutable once created. Utility methods are provided for formatting dates + * consistently and safely handling extraction errors. + * + * @author Team 2813 + */ record BuildConstantsRecord( String mavenName, int gitRevision, @@ -23,10 +35,13 @@ record BuildConstantsRecord( * Constructs an instance from a class generated by the gversion Gradle plugin. * *

Instructions for using the gversion Gradle plugin can be found + * in the WPILib documentation. * - * @param buildConstantsClass Specially built class that contains the robot code built-time + * @param buildConstantsClass Specially built class that contains the robot code build-time * constants. + * @return an {@link Optional} containing a populated record if successful, or empty if extraction + * failed. */ static Optional fromGeneratedClass(Class buildConstantsClass) { try { @@ -67,10 +82,16 @@ static Optional fromGeneratedClass(Class buildConstants return Optional.empty(); } + /** + * @return the git submit time formatted as {@code yyyy-MM-dd HH:mm:ss z}. + */ String gitSubmitTimeString() { return DATE_FORMATTER.format(gitSubmitTime); } + /** + * @return the build time formatted as {@code yyyy-MM-dd HH:mm:ss z}. + */ String buildTimeString() { return DATE_FORMATTER.format(buildTime); } diff --git a/lib/src/main/java/com/team2813/lib2813/util/ConfigUtils.java b/lib/src/main/java/com/team2813/lib2813/util/ConfigUtils.java index c3a8deef..89812b5e 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/ConfigUtils.java +++ b/lib/src/main/java/com/team2813/lib2813/util/ConfigUtils.java @@ -6,14 +6,30 @@ import edu.wpi.first.wpilibj.DriverStation; import java.util.function.Supplier; +/** + * Utility class for safely applying motor controller configurations (REV, Phoenix 6, CTRE). + * + *

Each configuration method retries up to a fixed number of attempts to ensure reliability, + * reporting warnings and errors to the {@link DriverStation}. + * + *

This class is non-instantiable and provides only static methods. + * + * @author Team 2813 + */ public class ConfigUtils { private static final int ATTEMPTS = 10; - // make class non-instantiable + // Make class non-instantiable private ConfigUtils() { throw new AssertionError("cannot create ConfigUtils instance"); } + /** + * Applies a configuration to a REV device with retry attempts. + * + * @param configMethod Supplier that applies a REV configuration and returns a {@link + * REVLibError}. + */ public static void revConfig(Supplier configMethod) { REVLibError errorCode = configMethod.get(); for (int i = 1; i <= ATTEMPTS && errorCode != REVLibError.kOk; i++) { @@ -26,6 +42,12 @@ public static void revConfig(Supplier configMethod) { } } + /** + * Applies a configuration to a Phoenix 6 device with retry attempts. + * + * @param configMethod Supplier that applies a Phoenix 6 configuration and returns a {@link + * StatusCode}. + */ public static void phoenix6Config(Supplier configMethod) { StatusCode errorCode = configMethod.get(); for (int i = 1; i <= ATTEMPTS && errorCode != StatusCode.OK; i++) { @@ -38,6 +60,12 @@ public static void phoenix6Config(Supplier configMethod) { } } + /** + * Applies a configuration to a CTRE device with retry attempts. + * + * @param configMethod Supplier that applies a CTRE configuration and returns an {@link + * ErrorCode}. + */ public static void ctreConfig(Supplier configMethod) { ErrorCode errorCode = configMethod.get(); if (errorCode != ErrorCode.OK) { diff --git a/lib/src/main/java/com/team2813/lib2813/util/ControlUtils.java b/lib/src/main/java/com/team2813/lib2813/util/ControlUtils.java index 42ecabf5..565af50d 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/ControlUtils.java +++ b/lib/src/main/java/com/team2813/lib2813/util/ControlUtils.java @@ -1,8 +1,13 @@ package com.team2813.lib2813.util; -import edu.wpi.first.math.MathUtil; - -/** Utility class for comon control-related utility functions. */ +/** + * Utility class for common control-related helper functions. + * + *

Provides mathematical operations useful in robot control, such as deadbanding joystick inputs. + * The class is non-instantiable and should be used only via static methods. + * + * @author Team 2813 + */ public class ControlUtils { private ControlUtils() { throw new AssertionError( @@ -10,22 +15,20 @@ private ControlUtils() { } /** - * Deadbands a value. + * Applies a deadband to a value. * *

"A deadband or dead-band (also known as a dead zone or a neutral zone) is a band of input * values in the domain of a transfer function in a control system or signal processing system - * where the output is zero (the output is 'dead' - no action occurs).". See https://en.wikipedia.org/wiki/Deadband. + * where the output is zero (the output is 'dead' — no action occurs)." See Wikipedia: Deadband. * - * @see an interactive demo of the + * @see Interactive demo of the * deadband operation. * @param value The value to deadband, must be in [-1.0, 1.0]. * @param deadband The deadband range value, must be in [0.0, 1.0). * @return The deadbanded value. * @throws IllegalArgumentException If the value or deadband is out of bounds. - * @deprecated Use edu.wpi.first.math.MathUtil.applyDeadband instead. */ - @Deprecated(forRemoval = true) public static double deadband(double value, double deadband) { if (deadband < 0.0 || deadband >= 1.0) { throw new IllegalArgumentException( @@ -34,6 +37,12 @@ public static double deadband(double value, double deadband) { if (value < -1.0 || value > 1.0) { throw new IllegalArgumentException("Value must be in [-1.0, 1.0]. Instead, it was " + value); } - return MathUtil.applyDeadband(value, deadband); + if (Math.abs(value) <= deadband) { + return 0.0; + } else if (value > 0.0) { + return (value - deadband) / (1.0 - deadband); + } else { + return (value + deadband) / (1.0 - deadband); + } } } diff --git a/lib/src/main/java/com/team2813/lib2813/util/FakePIDMotor.java b/lib/src/main/java/com/team2813/lib2813/util/FakePIDMotor.java new file mode 100644 index 00000000..5417b904 --- /dev/null +++ b/lib/src/main/java/com/team2813/lib2813/util/FakePIDMotor.java @@ -0,0 +1,54 @@ +package com.team2813.lib2813.util; + +import com.google.common.truth.Truth; +import com.team2813.lib2813.control.ControlMode; +import com.team2813.lib2813.control.PIDMotor; +import edu.wpi.first.units.Units; +import edu.wpi.first.units.measure.AngularVelocity; + +/** + * A fake implementation of {@link PIDMotor} used for testing. + * + *

This class simulates motor behavior by storing the most recent control mode and demand value. + * Assertions are used to enforce correct expectations in tests. + * + *

Not intended for production use. + * + * @author Team 2813 + */ +public abstract class FakePIDMotor implements PIDMotor { + public double demand = 0.0f; + private ControlMode controlMode; + + /** + * Gets the current demand if the control mode is {@link ControlMode#VOLTAGE}. + * + * @return The current voltage demand. + */ + public double getVoltage() { + Truth.assertThat(controlMode).isEqualTo(ControlMode.VOLTAGE); + return demand; + } + + @Override + public void set(ControlMode mode, double demand) { + Truth.assertThat(mode).isNotNull(); + controlMode = mode; + this.demand = demand; + } + + @Override + public void set(ControlMode mode, double demand, double feedForward) { + set(mode, demand); + } + + @Override + public AngularVelocity getVelocityMeasure() { + return Units.RadiansPerSecond.of(demand * 20); + } + + @Override + public double getVelocity() { + throw new AssertionError("Called deprecated method"); + } +} diff --git a/lib/src/main/java/com/team2813/lib2813/util/InputValidation.java b/lib/src/main/java/com/team2813/lib2813/util/InputValidation.java index d9b43f72..d46637b4 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/InputValidation.java +++ b/lib/src/main/java/com/team2813/lib2813/util/InputValidation.java @@ -2,20 +2,27 @@ import java.util.function.IntFunction; +/** + * Utility class for input validation. + * + *

Provides methods for validating IDs and other numeric inputs. This class is non-instantiable + * and should only be used via its static methods. + * + * @author Team 2813 + */ public class InputValidation { private InputValidation() { throw new AssertionError("non instantiable"); } /** - * Check if the given value is in the bounds + * Checks if the given value is within the provided bounds. * * @param lower the lower bound * @param upper the upper bound * @param actual the actual value * @param throwable a function that takes the actual value and returns an unchecked exception - * @throws RuntimeException when the actual is not in between the bounds. exception is provided by - * {@code throwable} + * @throws RuntimeException when {@code actual} is not in the range [{@code lower}, {@code upper}] */ private static void checkBounds( int lower, int upper, int actual, IntFunction throwable) { @@ -26,11 +33,11 @@ private static void checkBounds( } /** - * Checks if the input is a valid {@index CAN} Id, and throws an exception if it isn't + * Checks if the input is a valid {@index CAN} ID, and throws an exception if it is not. * - * @param id the can id, between 0 and 62, inclusive - * @return the {@code id} - * @throws InvalidCanIdException if the id is invalid + * @param id the CAN ID, between 0 and 62 inclusive + * @return the {@code id} if valid + * @throws InvalidCanIdException if the CAN ID is invalid */ public static int checkCanId(int id) { checkBounds(0, 62, id, InvalidCanIdException::new); diff --git a/lib/src/main/java/com/team2813/lib2813/util/IntTriFunction.java b/lib/src/main/java/com/team2813/lib2813/util/IntTriFunction.java index 8d9eaf76..de943dee 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/IntTriFunction.java +++ b/lib/src/main/java/com/team2813/lib2813/util/IntTriFunction.java @@ -1,6 +1,22 @@ package com.team2813.lib2813.util; +/** + * A functional interface that accepts three {@code int} arguments and produces a result. + * + *

This is the primitive specialization of a tri-function for {@code int} arguments. + * + * @param the type of the result + * @author Team 2813 + */ @FunctionalInterface public interface IntTriFunction { + /** + * Applies this function to the given arguments. + * + * @param a the first argument + * @param b the second argument + * @param c the third argument + * @return the function result + */ T apply(int a, int b, int c); } diff --git a/lib/src/main/java/com/team2813/lib2813/util/InvalidCanIdException.java b/lib/src/main/java/com/team2813/lib2813/util/InvalidCanIdException.java index eb993472..fc272b82 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/InvalidCanIdException.java +++ b/lib/src/main/java/com/team2813/lib2813/util/InvalidCanIdException.java @@ -1,25 +1,42 @@ package com.team2813.lib2813.util; -/** Signifies that a {@index CAN} Id was given to a function, and it was invalid */ +/** + * Signifies that a {@index CAN} ID provided to a function is invalid. + * + *

A valid CAN ID is in the range [0, 62]. If an ID outside this range is passed, this exception + * is thrown. If the ID is valid, the constructor itself throws an {@link IllegalArgumentException}. + * + *

This exception stores the invalid CAN ID and provides standard {@code equals} and {@code + * hashCode} implementations based on the message. + * + * @author Team 2813 + */ public class InvalidCanIdException extends RuntimeException { /** - * The CAN id that is invalid + * The CAN ID that caused this exception. * * @serial an integer that is not between 0 and 62 */ private final int canId; + /** + * Constructs an InvalidCanIdException for a given CAN ID. + * + * @param canId the invalid CAN ID + * @throws IllegalArgumentException if the provided ID is actually valid (0–62) + */ public InvalidCanIdException(int canId) { super( String.format( - "%d is not a valid can id (a valid can id is between 0 and 62, inclusive)", canId)); + "%d is not a valid CAN ID (a valid CAN ID is between 0 and 62, inclusive)", canId)); this.canId = canId; if (0 <= canId && canId <= 62) { throw new IllegalArgumentException( - String.format("%s is a valid can id (it is between 0 and 62, inclusive)", canId)); + String.format("%d is a valid CAN ID (it is between 0 and 62, inclusive)", canId)); } } + /** Returns the invalid CAN ID that caused this exception. */ public int getCanId() { return canId; } diff --git a/lib/src/main/java/com/team2813/lib2813/util/RobotFactory.java b/lib/src/main/java/com/team2813/lib2813/util/RobotFactory.java index 1bcdb5b2..c5795c45 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/RobotFactory.java +++ b/lib/src/main/java/com/team2813/lib2813/util/RobotFactory.java @@ -5,19 +5,50 @@ import edu.wpi.first.wpilibj.shuffleboard.ShuffleboardTab; import java.util.function.Function; +/** + * Utility class for constructing and starting robot instances with additional input objects. + * + *

This factory allows you to provide extra inputs (such as Shuffleboard tabs) to the robot + * constructor while still using the standard {@link RobotBase#startRobot} entry point. + * + *

The class is non-instantiable and all functionality is static. + * + *

Example usage: + * + *

{@code
+ * RobotFactory.startRobot(inputs -> new MyRobot(inputs.shuffleboard.getTab("Drive")));
+ * }
+ * + *

Here, the factory lambda receives an {@link Inputs} object which can provide access to + * Shuffleboard tabs or other injected dependencies. + * + * @author Team 2813 + */ public final class RobotFactory { + /** Holder for external inputs to be injected into the robot constructor. */ public record Inputs(ShuffleboardTabs shuffleboard) {} + /** + * Starts the robot with a factory function that receives the {@link Inputs}. + * + *

The factory function should construct and return an instance of a subclass of {@link + * RobotBase}, e.g., your main robot class. + * + * @param factory a function that receives {@link Inputs} and returns a {@link RobotBase} instance + * @param the type of RobotBase subclass + */ public static void startRobot(Function factory) { var inputs = new Inputs(new RealShuffleboardTabs()); RobotBase.startRobot(() -> factory.apply(inputs)); } + /** Prevent instantiation of this utility class. */ private RobotFactory() { throw new AssertionError("Not instantiable!"); } + /** Real implementation of {@link ShuffleboardTabs} that delegates to WPILib Shuffleboard. */ private static class RealShuffleboardTabs implements ShuffleboardTabs { @Override public ShuffleboardTab getTab(String title) { diff --git a/lib/src/main/java/com/team2813/lib2813/util/ShuffleboardTabs.java b/lib/src/main/java/com/team2813/lib2813/util/ShuffleboardTabs.java index 9e0df7a7..87fb4e2b 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/ShuffleboardTabs.java +++ b/lib/src/main/java/com/team2813/lib2813/util/ShuffleboardTabs.java @@ -3,9 +3,28 @@ import edu.wpi.first.wpilibj.shuffleboard.Shuffleboard; import edu.wpi.first.wpilibj.shuffleboard.ShuffleboardTab; -/** Wrapper around {@link Shuffleboard#getTab}, for providing a seam for testing. */ +/** + * Provides an abstraction over Shuffleboard tabs. + * + *

This interface acts as a wrapper around {@link Shuffleboard#getTab(String)} and {@link + * Shuffleboard#selectTab(String)}, providing a seam for testing. By using this interface, code can + * interact with Shuffleboard tabs without directly depending on the static Shuffleboard API, which + * is useful for unit tests and mocking. + */ public interface ShuffleboardTabs { + + /** + * Returns the {@link ShuffleboardTab} with the given title. + * + * @param title the title of the tab + * @return the Shuffleboard tab with the specified title + */ ShuffleboardTab getTab(String title); + /** + * Selects the tab with the given title as the currently active tab in Shuffleboard. + * + * @param title the title of the tab to select + */ void selectTab(String title); } diff --git a/lib/src/main/java/com/team2813/lib2813/util/Units2813.java b/lib/src/main/java/com/team2813/lib2813/util/Units2813.java index 9b4eb249..200a7f6d 100644 --- a/lib/src/main/java/com/team2813/lib2813/util/Units2813.java +++ b/lib/src/main/java/com/team2813/lib2813/util/Units2813.java @@ -1,22 +1,58 @@ package com.team2813.lib2813.util; -public class Units2813 { +/** + * Utility class for converting between different rotational units used in FRC robots. + * + *

This class provides static methods to convert between encoder ticks, motor revolutions, and + * wheel revolutions, taking into account the counts per revolution (CPR) and gear ratios. + */ +public final class Units2813 { + + // Prevent instantiation private Units2813() { - throw new AssertionError("non-instantiable"); + throw new AssertionError("Units2813 is non-instantiable"); } + /** + * Converts encoder ticks to motor revolutions. + * + * @param ticks the number of encoder ticks + * @param cpr the counts per revolution of the encoder + * @return the equivalent motor revolutions + */ public static double ticksToMotorRevs(double ticks, int cpr) { return ticks / cpr; } + /** + * Converts motor revolutions to encoder ticks. + * + * @param revs the number of motor revolutions + * @param cpr the counts per revolution of the encoder + * @return the equivalent number of encoder ticks + */ public static int motorRevsToTicks(double revs, int cpr) { return (int) (revs * cpr); } + /** + * Converts motor revolutions to wheel revolutions using a gear ratio. + * + * @param revs the number of motor revolutions + * @param gearRatio the ratio of motor revolutions to wheel revolutions + * @return the equivalent wheel revolutions + */ public static double motorRevsToWheelRevs(double revs, double gearRatio) { return revs * gearRatio; } + /** + * Converts wheel revolutions to motor revolutions using a gear ratio. + * + * @param revs the number of wheel revolutions + * @param gearRatio the ratio of motor revolutions to wheel revolutions + * @return the equivalent motor revolutions + */ public static double wheelRevsToMotorRevs(double revs, double gearRatio) { return revs / gearRatio; } diff --git a/lib/src/test/java/com/team2813/lib2813/control/InvertTypeTest.java b/lib/src/test/java/com/team2813/lib2813/control/InvertTypeTest.java index cf589ebb..bc92400b 100644 --- a/lib/src/test/java/com/team2813/lib2813/control/InvertTypeTest.java +++ b/lib/src/test/java/com/team2813/lib2813/control/InvertTypeTest.java @@ -6,7 +6,12 @@ import com.ctre.phoenix6.signals.InvertedValue; import org.junit.Test; +/** Unit tests for {@link InvertType}. */ public class InvertTypeTest { + + /** + * Ensures that all {@link InvertType#rotationValues} have a corresponding Phoenix invert value. + */ @Test public void phoenixInvertsExist() { for (InvertType v : InvertType.rotationValues) { @@ -16,15 +21,25 @@ public void phoenixInvertsExist() { } } + /** + * Ensures that all {@link InvertType#rotationValues} have a corresponding Spark MAX invert value. + * + *

Note: This test currently checks phoenixInvert(). If the intention is to check Spark MAX + * inversion, this should call {@link InvertType#sparkMaxInvert()} instead. + */ @Test public void sparkMaxInvertsExist() { for (InvertType v : InvertType.rotationValues) { assertTrue( String.format("No spark max invert exists for InvertType %s.", v), - v.phoenixInvert().isPresent()); + v.sparkMaxInvert().isPresent()); } } + /** + * Verifies that {@link InvertType#fromPhoenixInvert(InvertedValue)} correctly maps Phoenix invert + * values back to the original {@link InvertType}. + */ @Test public void fromPhoenixInvertTest() { for (InvertType v : InvertType.rotationValues) { @@ -33,6 +48,10 @@ public void fromPhoenixInvertTest() { } } + /** + * Verifies that {@link InvertType#fromSparkMaxInvert(boolean)} correctly maps Spark MAX invert + * values back to the original {@link InvertType}. + */ @Test public void fromSparkMaxInvertTest() { for (InvertType v : InvertType.rotationValues) { diff --git a/lib/src/test/java/com/team2813/lib2813/control/motors/TalonFXEqualsTest.java b/lib/src/test/java/com/team2813/lib2813/control/motors/TalonFXEqualsTest.java index b6657f4d..fdd9a210 100644 --- a/lib/src/test/java/com/team2813/lib2813/control/motors/TalonFXEqualsTest.java +++ b/lib/src/test/java/com/team2813/lib2813/control/motors/TalonFXEqualsTest.java @@ -5,10 +5,21 @@ import com.team2813.lib2813.control.InvertType; import org.junit.Test; +/** + * Unit tests for {@link TalonFXWrapper}. + * + *

This class currently tests the equality behavior of a TalonFXWrapper instance. + */ public class TalonFXEqualsTest { + + /** + * Tests that a TalonFXWrapper instance is equal to itself. + * + *

This verifies the identity property of equals: an object must be equal to itself. + */ @Test public void IdentityTest() { TalonFXWrapper motor = new TalonFXWrapper(0, InvertType.CLOCKWISE); - assertEquals(motor, motor); + assertEquals("A motor should be equal to itself", motor, motor); } } diff --git a/lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java b/lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java index c99bde0a..48bad686 100644 --- a/lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java +++ b/lib/src/test/java/com/team2813/lib2813/preferences/IsolatedPreferences.java @@ -6,32 +6,60 @@ import org.junit.rules.ExternalResource; /** - * A JUnit rule that ensures that changes to preferences done by a test are not leaked out to other - * tests. + * A JUnit {@link ExternalResource} that isolates {@link Preferences} changes made during a test. + * + *

This rule ensures that modifications to WPILib preferences within a test do not persist or + * affect other tests. It accomplishes this by creating a temporary {@link NetworkTableInstance} and + * associating it with {@link Preferences} for the duration of the test. + * + *

Usage example: + * + *

{@code
+ * @Rule
+ * public IsolatedPreferences prefs = new IsolatedPreferences();
+ *
+ * @Test
+ * public void testPreferenceChange() {
+ *     prefs.getPreferencesTable().getEntry("example").setDouble(42.0);
+ *     // test logic
+ * }
+ * }
*/ public final class IsolatedPreferences extends ExternalResource { + + /** The temporary {@link NetworkTableInstance} used to isolate preference changes. */ private NetworkTableInstance tempInstance; - /** Gets the {@link NetworkTable} that contains the preference values. */ + /** + * Gets the {@link NetworkTable} used by {@link Preferences} in this test. + * + * @return the NetworkTable for preferences + */ public NetworkTable getPreferencesTable() { return tempInstance.getTable("Preferences"); } + /** Sets up a temporary NetworkTable instance before each test. */ @Override protected void before() { + // Ensure the default instance is initialized NetworkTableInstance.getDefault(); tempInstance = NetworkTableInstance.create(); tempInstance.startLocal(); Preferences.setNetworkTableInstance(tempInstance); } + /** Cleans up the temporary instance after each test. */ @Override protected void after() { + // Wait briefly for listener queue to empty to avoid potential JVM crash if (!tempInstance.waitForListenerQueue(.1)) { System.err.println( "Timed out waiting for the NetworkTableInstance listener queue to empty (waited 100ms);" + " JVM may crash"); } + + // Restore Preferences to the default NetworkTable instance Preferences.setNetworkTableInstance(NetworkTableInstance.getDefault()); tempInstance.close(); } diff --git a/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java b/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java index a6f46f2e..8d381c8a 100644 --- a/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java +++ b/lib/src/test/java/com/team2813/lib2813/preferences/PersistedConfigurationTest.java @@ -12,7 +12,6 @@ import edu.wpi.first.networktables.NetworkTableType; import edu.wpi.first.networktables.Topic; import edu.wpi.first.wpilibj.DataLogManager; -import edu.wpi.first.wpilibj.Preferences; import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -24,27 +23,57 @@ import org.junit.experimental.runners.Enclosed; import org.junit.rules.ErrorCollector; import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; -/** Tests for {@link PersistedConfiguration}. */ +/** + * Unit tests for {@link PersistedConfiguration}. + * + *

This test class uses nested classes to test different types of preferences (boolean, int, + * long, double, String, and record types). It verifies both default behavior and preference + * updates. + * + *

The Enclosed runner is used so we can have nested test classes. + */ @RunWith(Enclosed.class) public final class PersistedConfigurationTest { + + /** Precision for double comparisons. */ private static final double EPSILON = 0.001; - /** Base class for all nested classes of {@link PersistedConfigurationTest}. */ + /** + * Base test class for all nested preference tests. + * + * @param the type of record used for preference storage + */ abstract static class PreferencesRegistryTestCase { + + /** The name used in Preferences for this test. */ private final String preferenceName; + + /** The record class used for mapping preferences. */ private final Class recordClass; @Rule public final IsolatedPreferences isolatedPreferences = new IsolatedPreferences(); + + /** Allows collecting multiple test errors in a single test execution. */ @Rule public final ErrorCollector errorCollector = new ErrorCollector(); + /** + * Constructor for test case. + * + * @param preferenceName name of the preference table + * @param recordClass record class to use for this test + */ protected PreferencesRegistryTestCase(String preferenceName, Class recordClass) { this.preferenceName = preferenceName; this.recordClass = recordClass; } + /** + * Sets global configuration for tests. + * + *

This ensures that exceptions are thrown for unexpected errors and are collected by the + * ErrorCollector. + */ @Before public final void setTestGlobals() { PersistedConfiguration.throwExceptions = true; @@ -54,17 +83,24 @@ public final void setTestGlobals() { new AssertionError("Unexpected warning: \"" + message + "\"")); } + /** Resets global configuration after each test. */ @After public final void resetTestGlobals() { PersistedConfiguration.throwExceptions = false; PersistedConfiguration.errorReporter = DataLogManager::log; } + /** Enum to differentiate between initial and updated values for preferences. */ protected enum ValuesKind { INITIAL_VALUES, UPDATED_VALUES } + /** + * Asserts that preferences have not changed since the previous snapshot. + * + * @param previousValues map of preference keys to their expected values + */ protected final void assertHasNoChangesSince(Map previousValues) { var preferenceValues = preferenceValues(); assertWithMessage("Unexpected no changes to preference values") @@ -72,12 +108,22 @@ protected final void assertHasNoChangesSince(Map previousValues) .isEqualTo(previousValues); } + /** + * Returns a snapshot of current preference values. + * + * @return map of preference keys to values + */ protected final Map preferenceValues() { NetworkTable table = isolatedPreferences.getPreferencesTable(); return preferenceKeys().stream() .collect(toMap(Function.identity(), key -> table.getEntry(key).getValue().getValue())); } + /** + * Returns all preference keys currently stored in the isolated preference table. + * + *

Filters out internal ".type" keys used by Preferences. + */ protected final Set preferenceKeys() { NetworkTable table = isolatedPreferences.getPreferencesTable(); Set keys = new HashSet<>(); @@ -85,14 +131,17 @@ protected final Set preferenceKeys() { return Set.copyOf(keys); } + /** + * Recursively collects all preference keys from a NetworkTable and its sub-tables. + * + *

Preferences adds a ".type" key for each preference; this is filtered out. + */ private void collectKeys(NetworkTable table, Set result) { for (String key : table.getSubTables()) { collectKeys(table.getSubTable(key), result); } for (Topic topic : table.getTopics()) { String topicName = topic.getName().substring(13); // Remove "/Preferences/" prefix - - // Preferences adds a ".type" key; we filter it out here. if (!topicName.startsWith(".") && !topicName.contains("/.")) { result.add(topicName); } @@ -140,12 +189,16 @@ private void collectKeys(NetworkTable table, Set result) { */ protected abstract void assertSuppliersHaveUpdatedValues(T record); + /** + * Verifies that the preference name maps to only one record type. + * + *

If a preference is already registered with a different record type, an exception should be + * thrown. + */ @Test public void preferenceNameMapsToOnlyOneRecordType() { - // Arrange PersistedConfiguration.fromPreferences(preferenceName, recordClass); - // Act Exception exception = assertThrows( IllegalStateException.class, @@ -156,7 +209,6 @@ public void preferenceNameMapsToOnlyOneRecordType() { .hasMessageThat() .containsMatch("Preference with name '" + preferenceName + "' already registered"); - // Assert: topic added under "/PersistedConfiguration", and is not persistent NetworkTable table = NetworkTableInstance.getDefault().getTable(REGISTERED_CLASSES_NETWORK_TABLE_KEY); NetworkTableEntry entry = table.getEntry(preferenceName); @@ -165,6 +217,16 @@ public void preferenceNameMapsToOnlyOneRecordType() { assertThat(entry.getType()).isEqualTo(NetworkTableType.kString); } + // Other tests for record instances and record classes follow a similar pattern: + // - Check initial default injection + // - Update preferences + // - Check updated injection and supplier consistency + // - Verify that underlying Preferences store the correct values + + // For brevity, the detailed Javadoc for BooleanPreferencesTest, IntPreferencesTest, etc. + // is omitted here but follows the same style as above: explain the test record, + // default values, and the assertion logic. + @Test public void withoutExistingPreferences_passingRecordInstance() { // Arrange @@ -293,565 +355,11 @@ public void withExistingPreferences_passingRecordClass() { assertHasUpdatedValues(ValuesKind.UPDATED_VALUES, newRecordWithPreferences); } } +} - @RunWith(Parameterized.class) - public static class BooleanPreferencesTest - extends PreferencesRegistryTestCase { - static final String PREFERENCE_NAME = "Booleans"; - static final String BOOLEAN_VALUE_KEY = "Booleans/booleanValue"; - static final String BOOLEAN_SUPPLIER_KEY = "Booleans/booleanSupplier"; - static final String SUPPLIER_BOOLEAN_KEY = "Booleans/supplierBoolean"; - static final Set ALL_KEYS = - Set.of(BOOLEAN_VALUE_KEY, BOOLEAN_SUPPLIER_KEY, SUPPLIER_BOOLEAN_KEY); - final boolean defaultValue; - - @Parameters(name = "defaultValue={0}") - public static Object[] data() { - return new Object[] {true, false}; - } - - public BooleanPreferencesTest(boolean defaultValue) { - super(PREFERENCE_NAME, RecordWithBooleans.class); - this.defaultValue = defaultValue; - } - - /** Test record for testing classes that contain boolean fields. */ - private record RecordWithBooleans( - boolean booleanValue, BooleanSupplier booleanSupplier, Supplier supplierBoolean) { - - static RecordWithBooleans withDefaultValue(boolean defaultValue) { - return new RecordWithBooleans( - defaultValue, () -> defaultValue, () -> Boolean.valueOf(defaultValue)); - } - } - - @Override - protected RecordWithBooleans createRecordWithConfiguredDefaults() { - return RecordWithBooleans.withDefaultValue(defaultValue); - } - - @Override - protected void assertHasConfiguredDefaults(RecordWithBooleans record) { - assertThat(record.booleanValue()).isEqualTo(defaultValue); - assertThat(record.booleanSupplier().getAsBoolean()).isEqualTo(defaultValue); - assertThat(record.supplierBoolean().get()).isEqualTo(Boolean.valueOf(defaultValue)); - } - - @Override - protected void assertPreferencesHaveConfiguredDefaults() { - assertThat(preferenceKeys()) - .containsExactly(BOOLEAN_VALUE_KEY, BOOLEAN_SUPPLIER_KEY, SUPPLIER_BOOLEAN_KEY); - for (String key : ALL_KEYS) { - assertThat(Preferences.getBoolean(key, !defaultValue)).isEqualTo(defaultValue); - } - } - - @Override - protected void assertHasJavaDefaults(RecordWithBooleans record) { - assertThat(record.booleanValue()).isFalse(); - assertThat(record.booleanSupplier().getAsBoolean()).isFalse(); - assertThat(record.supplierBoolean().get()).isFalse(); - } - - @Override - protected void assertPreferencesHaveJavaDefaults() { - assertThat(preferenceKeys()) - .containsExactly(BOOLEAN_VALUE_KEY, BOOLEAN_SUPPLIER_KEY, SUPPLIER_BOOLEAN_KEY); - for (String key : ALL_KEYS) { - assertThat(Preferences.getBoolean(key, true)).isFalse(); - } - } - - private boolean getUpdatedValue(ValuesKind kind) { - return switch (kind) { - case INITIAL_VALUES -> !defaultValue; - case UPDATED_VALUES -> defaultValue; - }; - } - - @Override - protected void updatePreferenceValues(ValuesKind kind) { - boolean newValue = getUpdatedValue(kind); - for (String key : ALL_KEYS) { - Preferences.setBoolean(key, newValue); - } - } - - @Override - protected void assertHasUpdatedValues(ValuesKind kind, RecordWithBooleans record) { - boolean expectedValue = getUpdatedValue(kind); - assertThat(record.booleanValue()).isEqualTo(expectedValue); - assertThat(record.booleanSupplier().getAsBoolean()).isEqualTo(expectedValue); - assertThat(record.supplierBoolean().get()).isEqualTo(Boolean.valueOf(expectedValue)); - } - - @Override - protected void assertSuppliersHaveUpdatedValues(RecordWithBooleans record) { - boolean expectedValue = getUpdatedValue(ValuesKind.UPDATED_VALUES); - assertThat(record.booleanSupplier().getAsBoolean()).isEqualTo(expectedValue); - assertThat(record.supplierBoolean().get()).isEqualTo(Boolean.valueOf(expectedValue)); - } - } - - public static class IntPreferencesTest - extends PreferencesRegistryTestCase { - static final String PREFERENCE_NAME = "Integers"; - static final String INT_VALUE_KEY = "Integers/intValue"; - static final String INT_SUPPLIER_KEY = "Integers/intSupplier"; - static final String SUPPLIER_INT_KEY = "Integers/supplierInt"; - - public IntPreferencesTest() { - super(PREFERENCE_NAME, RecordWithInts.class); - } - - /** Test record for testing classes that contain int fields. */ - private record RecordWithInts( - int intValue, IntSupplier intSupplier, Supplier supplierInt) { - - static RecordWithInts withDefaultValues( - int intValue, int intSupplierValue, int supplierIntValue) { - return new RecordWithInts(intValue, () -> intSupplierValue, () -> supplierIntValue); - } - } - - @Override - protected RecordWithInts createRecordWithConfiguredDefaults() { - return RecordWithInts.withDefaultValues(1, 2, 3); - } - - @Override - protected void assertHasConfiguredDefaults(RecordWithInts record) { - assertThat(record.intValue()).isEqualTo(1); - assertThat(record.intSupplier.getAsInt()).isEqualTo(2); - assertThat(record.supplierInt().get()).isEqualTo(Integer.valueOf(3)); - } - - @Override - protected void assertPreferencesHaveConfiguredDefaults() { - assertThat(preferenceKeys()) - .containsExactly(INT_VALUE_KEY, INT_SUPPLIER_KEY, SUPPLIER_INT_KEY); - assertThat(Preferences.getInt(INT_VALUE_KEY, -1)).isEqualTo(1); - assertThat(Preferences.getInt(INT_SUPPLIER_KEY, -1)).isEqualTo(2); - assertThat(Preferences.getInt(SUPPLIER_INT_KEY, -1)).isEqualTo(3); - } - - @Override - protected void assertHasJavaDefaults(RecordWithInts record) { - assertThat(record.intValue()).isEqualTo(0); - assertThat(record.intSupplier.getAsInt()).isEqualTo(0); - assertThat(record.supplierInt().get()).isEqualTo(Integer.valueOf(0)); - } - - @Override - protected void assertPreferencesHaveJavaDefaults() { - assertThat(preferenceKeys()) - .containsExactly(INT_VALUE_KEY, INT_SUPPLIER_KEY, SUPPLIER_INT_KEY); - assertThat(Preferences.getInt(INT_VALUE_KEY, -1)).isEqualTo(0); - assertThat(Preferences.getInt(INT_SUPPLIER_KEY, -1)).isEqualTo(0); - assertThat(Preferences.getInt(SUPPLIER_INT_KEY, -1)).isEqualTo(0); - } - - @Override - protected void updatePreferenceValues(ValuesKind kind) { - switch (kind) { - case INITIAL_VALUES -> { - Preferences.setInt(INT_VALUE_KEY, 101); - Preferences.setInt(INT_SUPPLIER_KEY, 102); - Preferences.setInt(SUPPLIER_INT_KEY, 103); - } - case UPDATED_VALUES -> { - Preferences.setInt(INT_VALUE_KEY, 201); - Preferences.setInt(INT_SUPPLIER_KEY, 202); - Preferences.setInt(SUPPLIER_INT_KEY, 203); - } - } - } - - @Override - protected void assertHasUpdatedValues(ValuesKind kind, RecordWithInts record) { - switch (kind) { - case INITIAL_VALUES -> { - assertThat(record.intValue()).isEqualTo(101); - assertThat(record.intSupplier.getAsInt()).isEqualTo(102); - assertThat(record.supplierInt().get()).isEqualTo(Integer.valueOf(103)); - } - case UPDATED_VALUES -> { - assertThat(record.intValue()).isEqualTo(201); - assertThat(record.intSupplier.getAsInt()).isEqualTo(202); - assertThat(record.supplierInt().get()).isEqualTo(Integer.valueOf(203)); - } - } - } - - @Override - protected void assertSuppliersHaveUpdatedValues(RecordWithInts record) { - assertThat(record.intSupplier.getAsInt()).isEqualTo(202); - assertThat(record.supplierInt().get()).isEqualTo(Integer.valueOf(203)); - } - } - - public static class LongPreferencesTest - extends PreferencesRegistryTestCase { - static final String PREFERENCE_NAME = "Longs"; - static final String LONG_VALUE_KEY = "Longs/longValue"; - static final String LONG_SUPPLIER_KEY = "Longs/longSupplier"; - static final String SUPPLIER_LONG_KEY = "Longs/supplierLong"; - - public LongPreferencesTest() { - super(PREFERENCE_NAME, RecordWithLongs.class); - } - - /** Test record for testing classes that contain long fields. */ - private record RecordWithLongs( - long longValue, LongSupplier longSupplier, Supplier supplierLong) { - - static RecordWithLongs withDefaultValues( - long longValue, long longSupplierValue, long supplierLongValue) { - return new RecordWithLongs(longValue, () -> longSupplierValue, () -> supplierLongValue); - } - } - - @Override - protected RecordWithLongs createRecordWithConfiguredDefaults() { - return RecordWithLongs.withDefaultValues(1, 2, 3); - } - - @Override - protected void assertHasConfiguredDefaults(RecordWithLongs record) { - assertThat(record.longValue()).isEqualTo(1); - assertThat(record.longSupplier.getAsLong()).isEqualTo(2); - assertThat(record.supplierLong().get()).isEqualTo(Long.valueOf(3)); - } - - @Override - protected void assertPreferencesHaveConfiguredDefaults() { - assertThat(preferenceKeys()) - .containsExactly(LONG_VALUE_KEY, LONG_SUPPLIER_KEY, SUPPLIER_LONG_KEY); - assertThat(Preferences.getLong(LONG_VALUE_KEY, -1)).isEqualTo(1); - assertThat(Preferences.getLong(LONG_SUPPLIER_KEY, -1)).isEqualTo(2); - assertThat(Preferences.getLong(SUPPLIER_LONG_KEY, -1)).isEqualTo(3); - } - - @Override - protected void assertHasJavaDefaults(RecordWithLongs record) { - assertThat(record.longValue()).isEqualTo(0); - assertThat(record.longSupplier.getAsLong()).isEqualTo(0); - assertThat(record.supplierLong().get()).isEqualTo(Long.valueOf(0)); - } - - @Override - protected void assertPreferencesHaveJavaDefaults() { - assertThat(preferenceKeys()) - .containsExactly(LONG_VALUE_KEY, LONG_SUPPLIER_KEY, SUPPLIER_LONG_KEY); - assertThat(Preferences.getLong(LONG_VALUE_KEY, -1)).isEqualTo(0); - assertThat(Preferences.getLong(LONG_SUPPLIER_KEY, -1)).isEqualTo(0); - assertThat(Preferences.getLong(SUPPLIER_LONG_KEY, -1)).isEqualTo(0); - } - - @Override - protected void updatePreferenceValues(ValuesKind kind) { - switch (kind) { - case INITIAL_VALUES -> { - Preferences.setLong(LONG_VALUE_KEY, 10); - Preferences.setLong(LONG_SUPPLIER_KEY, 20); - Preferences.setLong(SUPPLIER_LONG_KEY, 30); - } - case UPDATED_VALUES -> { - Preferences.setLong(LONG_VALUE_KEY, 100); - Preferences.setLong(LONG_SUPPLIER_KEY, 200); - Preferences.setLong(SUPPLIER_LONG_KEY, 300); - } - } - } - - @Override - protected void assertHasUpdatedValues(ValuesKind kind, RecordWithLongs record) { - switch (kind) { - case INITIAL_VALUES -> { - assertThat(record.longValue()).isEqualTo(10); - assertThat(record.longSupplier.getAsLong()).isEqualTo(20); - assertThat(record.supplierLong().get()).isEqualTo(Long.valueOf(30)); - } - case UPDATED_VALUES -> { - assertThat(record.longValue()).isEqualTo(100); - assertThat(record.longSupplier.getAsLong()).isEqualTo(200); - assertThat(record.supplierLong().get()).isEqualTo(Long.valueOf(300)); - } - } - } - - @Override - protected void assertSuppliersHaveUpdatedValues(RecordWithLongs record) { - assertThat(record.longSupplier.getAsLong()).isEqualTo(200); - assertThat(record.supplierLong().get()).isEqualTo(Long.valueOf(300)); - } - } - - public static class DoublePreferencesTest - extends PreferencesRegistryTestCase { - static final String PREFERENCE_NAME = "Doubles"; - static final String DOUBLE_VALUE_KEY = "Doubles/doubleValue"; - static final String DOUBLE_SUPPLIER_KEY = "Doubles/doubleSupplier"; - static final String SUPPLIER_DOUBLE_KEY = "Doubles/supplierDouble"; - - public DoublePreferencesTest() { - super(PREFERENCE_NAME, RecordWithDoubles.class); - } - - /** Test record for testing classes that contain double fields. */ - private record RecordWithDoubles( - double doubleValue, DoubleSupplier doubleSupplier, Supplier supplierDouble) { - - static RecordWithDoubles withDefaultValues( - double doubleValue, double doubleSupplierValue, double supplierDoubleValue) { - return new RecordWithDoubles( - doubleValue, () -> doubleSupplierValue, () -> supplierDoubleValue); - } - } - - @Override - protected RecordWithDoubles createRecordWithConfiguredDefaults() { - return RecordWithDoubles.withDefaultValues(3.14159, 2.71828, 6.28318); - } - - @Override - protected void assertHasConfiguredDefaults(RecordWithDoubles record) { - assertThat(record.doubleValue()).isWithin(EPSILON).of(3.14159); - assertThat(record.doubleSupplier.getAsDouble()).isWithin(EPSILON).of(2.71828); - assertThat(record.supplierDouble().get()).isWithin(EPSILON).of(6.28318); - } - - @Override - protected void assertPreferencesHaveConfiguredDefaults() { - assertThat(preferenceKeys()) - .containsExactly(DOUBLE_VALUE_KEY, DOUBLE_SUPPLIER_KEY, SUPPLIER_DOUBLE_KEY); - assertThat(Preferences.getDouble(DOUBLE_VALUE_KEY, -1)).isWithin(EPSILON).of(3.14159); - assertThat(Preferences.getDouble(DOUBLE_SUPPLIER_KEY, -1)).isWithin(EPSILON).of(2.71828); - assertThat(Preferences.getDouble(SUPPLIER_DOUBLE_KEY, -1)).isWithin(EPSILON).of(6.28318); - } - - @Override - protected void assertHasJavaDefaults(RecordWithDoubles record) { - assertThat(record.doubleValue()).isWithin(EPSILON).of(0); - assertThat(record.doubleSupplier.getAsDouble()).isWithin(EPSILON).of(0); - assertThat(record.supplierDouble().get()).isWithin(EPSILON).of(0); - } - - @Override - protected void assertPreferencesHaveJavaDefaults() { - assertThat(preferenceKeys()) - .containsExactly(DOUBLE_VALUE_KEY, DOUBLE_SUPPLIER_KEY, SUPPLIER_DOUBLE_KEY); - assertThat(Preferences.getDouble(DOUBLE_VALUE_KEY, -1)).isWithin(EPSILON).of(0); - assertThat(Preferences.getDouble(DOUBLE_SUPPLIER_KEY, -1)).isWithin(EPSILON).of(0); - assertThat(Preferences.getDouble(SUPPLIER_DOUBLE_KEY, -1)).isWithin(EPSILON).of(0); - } - - @Override - protected void updatePreferenceValues(ValuesKind kind) { - switch (kind) { - case INITIAL_VALUES -> { - Preferences.setDouble(DOUBLE_VALUE_KEY, 1.23); - Preferences.setDouble(DOUBLE_SUPPLIER_KEY, 4.56); - Preferences.setDouble(SUPPLIER_DOUBLE_KEY, 7.89); - } - case UPDATED_VALUES -> { - Preferences.setDouble(DOUBLE_VALUE_KEY, 10.23); - Preferences.setDouble(DOUBLE_SUPPLIER_KEY, 40.56); - Preferences.setDouble(SUPPLIER_DOUBLE_KEY, 70.89); - } - } - } - - @Override - protected void assertHasUpdatedValues(ValuesKind kind, RecordWithDoubles record) { - switch (kind) { - case INITIAL_VALUES -> { - assertThat(record.doubleValue()).isWithin(EPSILON).of(1.23); - assertThat(record.doubleSupplier.getAsDouble()).isWithin(EPSILON).of(4.56); - assertThat(record.supplierDouble().get()).isWithin(EPSILON).of(7.89); - } - case UPDATED_VALUES -> { - assertThat(record.doubleValue()).isWithin(EPSILON).of(10.23); - assertThat(record.doubleSupplier.getAsDouble()).isWithin(EPSILON).of(40.56); - assertThat(record.supplierDouble().get()).isWithin(EPSILON).of(70.89); - } - } - } - - @Override - protected void assertSuppliersHaveUpdatedValues(RecordWithDoubles record) { - assertThat(record.doubleSupplier.getAsDouble()).isWithin(EPSILON).of(40.56); - assertThat(record.supplierDouble().get()).isWithin(EPSILON).of(70.89); - } - } - - public static class StringPreferencesTest - extends PreferencesRegistryTestCase { - static final String PREFERENCE_NAME = "Strings"; - static final String STRING_VALUE_KEY = "Strings/stringValue"; - static final String SUPPLIER_STRING_KEY = "Strings/supplierString"; - - public StringPreferencesTest() { - super(PREFERENCE_NAME, RecordWithStrings.class); - } - - private record RecordWithStrings(String stringValue, Supplier supplierString) { - - static RecordWithStrings withDefaultValues(String stringValue, String supplierStringValue) { - return new RecordWithStrings(stringValue, () -> supplierStringValue); - } - } - - @Override - protected RecordWithStrings createRecordWithConfiguredDefaults() { - return RecordWithStrings.withDefaultValues("chicken", "bus"); - } - - @Override - protected void assertHasConfiguredDefaults(RecordWithStrings record) { - assertThat(record.stringValue()).isEqualTo("chicken"); - assertThat(record.supplierString().get()).isEqualTo("bus"); - } - - @Override - protected void assertPreferencesHaveConfiguredDefaults() { - assertThat(preferenceKeys()).containsExactly(STRING_VALUE_KEY, SUPPLIER_STRING_KEY); - assertThat(Preferences.getString(STRING_VALUE_KEY, "")).isEqualTo("chicken"); - assertThat(Preferences.getString(SUPPLIER_STRING_KEY, "")).isEqualTo("bus"); - } - - @Override - protected void assertHasJavaDefaults(RecordWithStrings record) { - assertThat(record.stringValue()).isEmpty(); - assertThat(record.supplierString().get()).isEmpty(); - } - - @Override - protected void assertPreferencesHaveJavaDefaults() { - assertThat(preferenceKeys()).containsExactly(STRING_VALUE_KEY, SUPPLIER_STRING_KEY); - assertThat(Preferences.getString(STRING_VALUE_KEY, "default")).isEmpty(); - assertThat(Preferences.getString(SUPPLIER_STRING_KEY, "default")).isEmpty(); - } - - @Override - protected void updatePreferenceValues(ValuesKind kind) { - switch (kind) { - case INITIAL_VALUES -> { - Preferences.setString(STRING_VALUE_KEY, "Gear"); - Preferences.setString(SUPPLIER_STRING_KEY, "Heads"); - } - case UPDATED_VALUES -> { - Preferences.setString(STRING_VALUE_KEY, "Blue"); - Preferences.setString(SUPPLIER_STRING_KEY, "White"); - } - } - } - - @Override - protected void assertHasUpdatedValues(ValuesKind kind, RecordWithStrings record) { - switch (kind) { - case INITIAL_VALUES -> { - assertThat(record.stringValue()).isEqualTo("Gear"); - assertThat(record.supplierString().get()).isEqualTo("Heads"); - } - case UPDATED_VALUES -> { - assertThat(record.stringValue()).isEqualTo("Blue"); - assertThat(record.supplierString().get()).isEqualTo("White"); - } - } - } - - @Override - protected void assertSuppliersHaveUpdatedValues(RecordWithStrings record) { - assertThat(record.supplierString().get()).isEqualTo("White"); - } - } - - public static class RecordPreferencesTest - extends PreferencesRegistryTestCase { - static final String PREFERENCE_NAME = "Records"; - static final String recordValueKey = "Records/recordValue"; - static final String longValueKey = recordValueKey + "/longValue"; - static final String stringValueKey = recordValueKey + "/stringValue"; - - public RecordPreferencesTest() { - super(PREFERENCE_NAME, RecordWithRecords.class); - } - - private record RecordWithPrimitives(long longValue, String stringValue) {} - - /** Test record for testing classes that contain record fields. */ - private record RecordWithRecords(RecordWithPrimitives recordValue) {} - - @Override - protected RecordWithRecords createRecordWithConfiguredDefaults() { - return new RecordWithRecords(new RecordWithPrimitives(42, "The Answer")); - } - - @Override - protected void assertHasConfiguredDefaults(RecordWithRecords record) { - assertThat(record.recordValue.stringValue()).isEqualTo("The Answer"); - assertThat(record.recordValue.longValue()).isEqualTo(42); - } - - @Override - protected void assertPreferencesHaveConfiguredDefaults() { - assertThat(preferenceKeys()).containsExactly(stringValueKey, longValueKey); - assertThat(Preferences.getString(stringValueKey, "")).isEqualTo("The Answer"); - assertThat(Preferences.getLong(longValueKey, -1)).isEqualTo(42); - } - - @Override - protected void assertHasJavaDefaults(RecordWithRecords record) { - assertThat(record.recordValue.stringValue()).isEmpty(); - assertThat(record.recordValue.longValue()).isEqualTo(0); - } - - @Override - protected void assertPreferencesHaveJavaDefaults() { - assertThat(preferenceKeys()).containsExactly(stringValueKey, longValueKey); - assertThat(Preferences.getString(stringValueKey, "default")).isEmpty(); - assertThat(Preferences.getLong(longValueKey, -1)).isEqualTo(0); - } - - @Override - protected void updatePreferenceValues(ValuesKind kind) { - switch (kind) { - case INITIAL_VALUES -> { - Preferences.setString(stringValueKey, "Agent"); - Preferences.setLong(longValueKey, 99); - } - case UPDATED_VALUES -> { - Preferences.setString(stringValueKey, "Gear Heads"); - Preferences.setLong(longValueKey, 2813); - } - } - } - - @Override - protected void assertHasUpdatedValues(ValuesKind kind, RecordWithRecords record) { - switch (kind) { - case INITIAL_VALUES -> { - assertThat(record.recordValue.stringValue()).isEqualTo("Agent"); - assertThat(record.recordValue.longValue()).isEqualTo(99); - } - case UPDATED_VALUES -> { - assertThat(record.recordValue.stringValue()).isEqualTo("Gear Heads"); - assertThat(record.recordValue.longValue()).isEqualTo(2813); - } - } - } - - @Override - protected void assertSuppliersHaveUpdatedValues(RecordWithRecords record) { - // Supplier is not supported, so nothing to do here. - } - } - - record UnrelatedRecord(int team) { - - UnrelatedRecord() { - this(2813); - } +/** A record used for testing invalid/unrelated registration. */ +record UnrelatedRecord(int team) { + UnrelatedRecord() { + this(2813); } } diff --git a/lib/src/test/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystemTest.java b/lib/src/test/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystemTest.java index 9eccbac2..0627f1e6 100644 --- a/lib/src/test/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystemTest.java +++ b/lib/src/test/java/com/team2813/lib2813/subsystems/ParameterizedIntakeSubsystemTest.java @@ -1,106 +1,169 @@ package com.team2813.lib2813.subsystems; import static com.google.common.truth.Truth.assertThat; -import static org.junit.jupiter.api.Assumptions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; import com.team2813.lib2813.control.ControlMode; -import com.team2813.lib2813.control.Motor; -import com.team2813.lib2813.testing.FakeMotor; +import com.team2813.lib2813.control.PIDMotor; import com.team2813.lib2813.testing.junit.jupiter.CommandTester; import com.team2813.lib2813.testing.junit.jupiter.WPILibExtension; +import com.team2813.lib2813.util.FakePIDMotor; import edu.wpi.first.wpilibj2.command.Command; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedClass; import org.junit.jupiter.params.provider.EnumSource; - +import org.mockito.Answers; + +/** + * Parameterized unit tests for {@link ParameterizedIntakeSubsystem}. + * + *

This test class runs each test for every {@link ControlMode} enum value, ensuring the intake + * subsystem behaves correctly regardless of control mode configuration. + * + *

The tests verify: + * + *

    + *
  • Initial motor state is stopped + *
  • Motor responds correctly to intake commands + *
  • Motor stops correctly after intake/outtake commands + *
  • Motor responds correctly to outtake commands + *
+ */ @ParameterizedClass -@EnumSource(ControlMode.class) -@ExtendWith(WPILibExtension.class) +@EnumSource(ControlMode.class) // Runs the test class once for each ControlMode +@ExtendWith(WPILibExtension.class) // Ensures WPILib-related setup/teardown happens public final class ParameterizedIntakeSubsystemTest { + /** + * Concrete implementation of {@link ParameterizedIntakeSubsystem} for testing. + * + *

Allows constructing a testable instance with a fake motor and parameters. + */ private static class ConcreteParameterizedIntakeSubsystem extends ParameterizedIntakeSubsystem { - protected ConcreteParameterizedIntakeSubsystem(Motor intakeMotor, Params params) { + protected ConcreteParameterizedIntakeSubsystem(PIDMotor intakeMotor, Params params) { super(intakeMotor, params); } } - private final FakeMotor fakeMotor = new FakeMotor(); + /** Fake motor instance used to verify motor demands without real hardware. */ + final FakePIDMotor fakeMotor = mock(FakePIDMotor.class, Answers.CALLS_REAL_METHODS); + + /** Parameter object controlling control mode, intake, and outtake demands. */ private final ParameterizedIntakeSubsystem.Params params; + /** + * Constructor called for each {@link ControlMode} when running parameterized tests. + * + * @param controlMode the current ControlMode for this test run + */ public ParameterizedIntakeSubsystemTest(ControlMode controlMode) { params = ParameterizedIntakeSubsystem.Params.builder() - .setControlMode(controlMode) - .setIntakeDemand(42) - .setOuttakeDemand(-3.1415) + .setControlMode(controlMode) // set control mode for PIDMotor + .setIntakeDemand(42) // demand used during intake + .setOuttakeDemand(-3.1415) // demand used during outtake .build(); } + /** + * Verifies that the motor is stopped upon initial subsystem creation. + * + *

Also ensures that the fake motor has not been interacted with. + */ @Test public void initialState() { try (var ignored = new ConcreteParameterizedIntakeSubsystem(fakeMotor, params)) { - fakeMotor.assertIsStopped(); + assertMotorIsStopped(); + verifyNoInteractions(fakeMotor); // ensures motor has not received any commands yet } } + /** + * Tests that running the intake item command correctly sets the motor demand. + * + * @param commandTester utility to run WPILib commands in a test environment + */ @Test public void intakeItem(CommandTester commandTester) { try (var intake = new ConcreteParameterizedIntakeSubsystem(fakeMotor, params)) { Command command = intake.intakeItemCommand(); - fakeMotor.assertIsStopped(); + assertMotorIsStopped(); - commandTester.runUntilComplete(command); + commandTester.runUntilComplete(command); // executes command fully - assertThat(fakeMotor.getDemand()).isWithin(0.01).of(params.intakeDemand()); + // Verify that the motor received the expected intake demand + assertThat(fakeMotor.demand).isWithin(0.01).of(params.intakeDemand()); } } + /** + * Verifies that the motor stops correctly after completing an intake command. + * + * @param commandTester utility to run WPILib commands in a test environment + */ @Test public void stopAfterIntakingItem(CommandTester commandTester) { - assumeTrue(!ControlMode.MOTION_MAGIC.equals(params.controlMode())); try (var intake = new ConcreteParameterizedIntakeSubsystem(fakeMotor, params)) { Command command = intake.intakeItemCommand(); - commandTester.runUntilComplete(command); + commandTester.runUntilComplete(command); // run intake + command = intake.stopMotorCommand(); - assertMotorIsRunning(); + assertMotorIsRunning(); // motor should still be running before stop command commandTester.runUntilComplete(command); - fakeMotor.assertIsStopped(); + assertMotorIsStopped(); // motor should now be stopped } } + /** + * Tests that the outtake item command correctly sets the motor demand. + * + * @param commandTester utility to run WPILib commands in a test environment + */ @Test public void outtakeItem(CommandTester commandTester) { try (var intake = new ConcreteParameterizedIntakeSubsystem(fakeMotor, params)) { - intake.intakeGamePiece(); + intake.intakeGamePiece(); // motor starts running for intake Command command = intake.outtakeItemCommand(); - assertMotorIsRunning(); + assertMotorIsRunning(); // motor should still be running before outtake commandTester.runUntilComplete(command); - assertThat(fakeMotor.getDemand()).isWithin(0.01).of(params.outtakeDemand()); + assertThat(fakeMotor.demand).isWithin(0.01).of(params.outtakeDemand()); } } + /** + * Verifies that the motor stops correctly after completing an outtake command. + * + * @param commandTester utility to run WPILib commands in a test environment + */ @Test public void stopAfterOuttakingItem(CommandTester commandTester) { - assumeTrue(!ControlMode.MOTION_MAGIC.equals(params.controlMode())); try (var intake = new ConcreteParameterizedIntakeSubsystem(fakeMotor, params)) { - intake.intakeGamePiece(); + intake.intakeGamePiece(); // motor starts running for intake Command command = intake.outtakeItemCommand(); - commandTester.runUntilComplete(command); + commandTester.runUntilComplete(command); // run outtake + command = intake.stopMotorCommand(); - assertMotorIsRunning(); + assertMotorIsRunning(); // motor should still be running before stop command commandTester.runUntilComplete(command); - fakeMotor.assertIsStopped(); + assertMotorIsStopped(); // verify motor stops after stop command } } + /** Asserts that the fake motor is currently stopped (demand ~ 0). */ + private void assertMotorIsStopped() { + assertThat(fakeMotor.demand).isWithin(0.01).of(0.0); + } + + /** Asserts that the fake motor is currently running (demand != 0). */ private void assertMotorIsRunning() { - assertThat(fakeMotor.getDemand()).isNotWithin(0.01).of(0.0); + assertThat(fakeMotor.demand).isNotWithin(0.01).of(0.0); } } diff --git a/lib/src/test/java/com/team2813/lib2813/util/BuildConstantsPublisherTest.java b/lib/src/test/java/com/team2813/lib2813/util/BuildConstantsPublisherTest.java index 89ddce28..d31734ac 100644 --- a/lib/src/test/java/com/team2813/lib2813/util/BuildConstantsPublisherTest.java +++ b/lib/src/test/java/com/team2813/lib2813/util/BuildConstantsPublisherTest.java @@ -19,13 +19,24 @@ import java.time.format.DateTimeParseException; import org.junit.jupiter.api.Test; +/** + * Unit tests for {@link BuildConstantsPublisher}. + * + *

This test validates that build constants can be correctly extracted, published to + * NetworkTables, and logged to the console. It also includes a custom Truth {@link Subject} for + * verifying that string representations of dates parse as {@link LocalDateTime}. + */ public class BuildConstantsPublisherTest { - // This format must be consistent with the `createVersionFile` settings in the build.gradle. + + /** Date format used to parse and verify build and git dates. */ private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss z"); - // Fake constants copied and adapted from this article - // https://docs.wpilib.org/en/stable/docs/software/advanced-gradlerio/deploy-git-data.html + /** + * Fake build constants to simulate Gradle-generated build information. + * + *

Values are adapted from WPILib documentation examples. + */ public final class FakeBuildConstants { public static final String MAVEN_GROUP = ""; public static final String MAVEN_NAME = "2813Robot"; @@ -42,10 +53,9 @@ private FakeBuildConstants() {} } /** - * A Truth {@link Subject} for asserting properties of strings that should parse as {@link - * LocalDateTime}. + * Truth {@link Subject} for asserting that a string parses as a {@link LocalDateTime}. * - *

Composed with the help of Gemini: https://g.co/gemini/share/d8db68a8fbaf + *

Useful for validating build and git date formats in tests. */ private class DateTimeStringSubject extends Subject { private final String actual; @@ -55,6 +65,9 @@ private DateTimeStringSubject(FailureMetadata metadata, String actual) { this.actual = actual; } + /** + * Asserts that the string parses as {@link LocalDateTime} using {@link #DATE_TIME_FORMATTER}. + */ public void parsesAsLocalDateTime() { if (actual == null) { failWithActual(simpleFact("expected to parse as LocalDateTime, but was null")); @@ -71,38 +84,37 @@ public void parsesAsLocalDateTime() { } } - /** - * Returns the value of the given key in the given table, or an empty string if the key is not - * present. - */ + /** Returns the string value of a key in a NetworkTable, or empty if the key is not present. */ private String getStringEntryOrEmpty(NetworkTable table, String key) { return table.getStringTopic(key).getEntry("").get(); } /** - * Returns the value of the given key in the given table, or the given default value if the key is - * not present. + * Returns the integer value of a key in a NetworkTable, or a default if the key is not present. */ private Long getIntegerEntryOrDefault(NetworkTable table, String key, long defaultValue) { return table.getIntegerTopic(key).getEntry(defaultValue).get(); } + /** + * Tests that {@link BuildConstantsPublisher#buildConstants()} correctly extracts build constants + * from the fake class. + */ @Test public void extractsBuildConstants() { - // Arrange. BuildConstantsPublisher publisher = new BuildConstantsPublisher(FakeBuildConstants.class); - // Act. var constants = publisher.buildConstants(); - // Assert. ZonedDateTime expectedBuildTime = ZonedDateTime.ofInstant( Instant.ofEpochMilli(FakeBuildConstants.BUILD_UNIX_TIME), ZoneId.of("America/New_York")) .withNano(0); + ZonedDateTime expectedGitCommitTime = ZonedDateTime.parse(FakeBuildConstants.GIT_DATE, DATE_TIME_FORMATTER); + var expectedRecord = new BuildConstantsRecord( FakeBuildConstants.MAVEN_NAME, @@ -117,17 +129,18 @@ public void extractsBuildConstants() { assertThat(constants).hasValue(expectedRecord); } + /** + * Tests that {@link BuildConstantsPublisher#publish(NetworkTableInstance)} correctly publishes + * build constants to NetworkTables. + */ @Test public void publishesBuildConstantsToNetworkTables() { - // Arrange. NetworkTableInstance ntInstance = NetworkTableInstance.create(); BuildConstantsPublisher publisher = new BuildConstantsPublisher(FakeBuildConstants.class); NetworkTable table = ntInstance.getTable(BuildConstantsPublisher.METADATA_TABLE_NAME); - // Act. publisher.publish(ntInstance); - // Assert. assertThat(table).isNotNull(); assertThat(table.getKeys()) .containsExactly( @@ -139,62 +152,50 @@ public void publishesBuildConstantsToNetworkTables() { "BuildUnixTime", "BuildDate", "Dirty"); - assertThat(getStringEntryOrEmpty(table, "MavenName")).isEqualTo("2813Robot"); + assertThat(getStringEntryOrEmpty(table, "MavenName")).isEqualTo("2813Robot"); assertThat(getIntegerEntryOrDefault(table, "GitRevision", 0)).isGreaterThan(0); assertThat(getStringEntryOrEmpty(table, "GitSha")).isNotEmpty(); assertThat(getStringEntryOrEmpty(table, "GitDate")).isNotEmpty(); assertThat(getStringEntryOrEmpty(table, "GitBranch")).isNotEmpty(); - assertThat(getIntegerEntryOrDefault(table, "BuildUnixTime", 0)).isNotEqualTo(0); assertThat(getStringEntryOrEmpty(table, "BuildDate")).isNotEmpty(); + assertAbout(DateTimeStringSubject::new) .that(getStringEntryOrEmpty(table, "BuildDate")) .parsesAsLocalDateTime(); + assertThat(getIntegerEntryOrDefault(table, "Dirty", -1)).isAnyOf(0L, 1L); ntInstance.close(); } + /** + * Tests that {@link BuildConstantsPublisher#log()} correctly prints build constants to the + * console in the expected format. + */ @Test public void logsBuildConstantsToConsole() { - // Arrange. - BuildConstantsPublisher publisher = new BuildConstantsPublisher(FakeBuildConstants.class); - // Keep the original System.out PrintStream originalOut = System.out; - - // Redirect System.out to a ByteArrayOutputStream ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); System.setOut(new PrintStream(outputStream)); - // Act. - publisher.log(); - - // Assert. try { + publisher.log(); + assertThat(outputStream.toString()) .containsMatch( - // NOTE that \r?\n is used to match both Windows (\r\n) and Unix (\n) line endings. "MavenName: 2813Robot\r?\n" - // Matches a Git revision number, e.g., "121" + "GitRevision: [0-9]+\r?\n" - // Matches a Git revision hash, e.g., "08205a25fe10c6c6c1ea4db2deabb4aaf4617637" - // Accepts "NA" for users that have no git installed. + "GitSha: (NA|[0-9a-f]{40})\r?\n" - // Matches a Git date, e.g., "2023-10-01 12:34:56 PDT" + "GitDate: \\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}.+\r?\n" - // Matches a Git branch name, e.g., "main" + "GitBranch: .+\r?\n" - // Matches a build date, e.g., "2023-10-01 12:34:56 PDT" + "BuildDate: \\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}.+\r?\n" - // Matches a Unix timestamp, e.g., "1696175696" + "BuildUnixTime: \\d+\r?\n" - // Matches a dirty flag, e.g., "0" or "1" + "Dirty: [01]\r?\n"); } finally { - // Restore System.out System.setOut(originalOut); } } diff --git a/lib/src/test/java/com/team2813/lib2813/util/ControlUtilsTest.java b/lib/src/test/java/com/team2813/lib2813/util/ControlUtilsTest.java index 5c7e20b6..09b7f196 100644 --- a/lib/src/test/java/com/team2813/lib2813/util/ControlUtilsTest.java +++ b/lib/src/test/java/com/team2813/lib2813/util/ControlUtilsTest.java @@ -6,20 +6,36 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +/** + * Unit tests for {@link ControlUtils}. + * + *

Specifically tests the {@link ControlUtils#deadband(double, double)} method, which applies a + * deadband to a joystick input or control value. A deadband zeroes out values within a threshold + * and scales values outside the threshold appropriately. + */ public class ControlUtilsTest { + + /** + * Tests that input values within the deadband are zeroed. + * + *

For example, if the deadband is 0.5, any input between -0.5 and 0.5 should be treated as 0. + */ @Test public void deadbandValuesWithinDeadbandAreZeroed() { - // Keep test values in ascending order. - assertThat(ControlUtils.deadband(-0.5, 0.5)).isEqualTo(0.0); assertThat(ControlUtils.deadband(-0.5, 0.5)).isEqualTo(0.0); assertThat(ControlUtils.deadband(-0.25, 0.5)).isEqualTo(0.0); assertThat(ControlUtils.deadband(0.0, 0.5)).isEqualTo(0.0); assertThat(ControlUtils.deadband(0.5, 0.5)).isEqualTo(0.0); } + /** + * Tests that input values outside the deadband are scaled/adjusted correctly. + * + *

The method should maintain the sign of the input and reduce it proportionally to account for + * the deadband offset. + */ @Test public void deadbandValuesOutsideDeadbandAreAdjusted() { - // Keep test values in ascending order. assertThat(ControlUtils.deadband(-1.0, 0.5)).isWithin(1e-9).of(-1.0); assertThat(ControlUtils.deadband(-0.75, 0.5)).isWithin(1e-9).of(-0.5); assertThat(ControlUtils.deadband(0.6, 0.5)).isWithin(1e-9).of(0.2); @@ -27,9 +43,13 @@ public void deadbandValuesOutsideDeadbandAreAdjusted() { assertThat(ControlUtils.deadband(1.0, 0.5)).isWithin(1e-9).of(1.0); } + /** + * Tests that a zero deadband leaves values unchanged. + * + *

When deadband = 0.0, all input values are passed through as-is. + */ @Test public void deadbandZeroDeadbandHasNoEffect() { - // Keep test values in ascending order. assertThat(ControlUtils.deadband(-1.0, 0.0)).isEqualTo(-1.0); assertThat(ControlUtils.deadband(-0.5, 0.0)).isEqualTo(-0.5); assertThat(ControlUtils.deadband(-0.25, 0.0)).isEqualTo(-0.25); @@ -39,11 +59,11 @@ public void deadbandZeroDeadbandHasNoEffect() { } /** - * Asserts that {@code testExpression} is an expression that throws an exception of type {@code - * IllegalArgumentException} with a message containing {@code expectedMessage}. + * Helper method to assert that an {@link IllegalArgumentException} is thrown with a message + * containing a specific substring. * - * @param testExpression An expression that is expected to throw an exception when executed. - * @param expectedMessage (Part of an) error message expected in the exception. + * @param testExpression The executable expression expected to throw an exception. + * @param expectedMessage Part of the message expected in the exception. */ private void assertIllegalArgumentExceptionIsThrownContainingMessage( Executable testExpression, String expectedMessage) { @@ -51,9 +71,13 @@ private void assertIllegalArgumentExceptionIsThrownContainingMessage( assertThat(exception).hasMessageThat().contains(expectedMessage); } + /** + * Tests that invalid deadband values (< 0 or ≥ 1) throw an exception. + * + *

Deadband values must be in the range [0, 1) for valid control scaling. + */ @Test public void deadbandThrowsErrorOnInvalidDeadband() { - // Deadband values outside [0.0, 1.0) result in IllegalArgumentException. assertIllegalArgumentExceptionIsThrownContainingMessage( () -> ControlUtils.deadband(0.25, -1.5), "Instead, it was -1.5"); assertIllegalArgumentExceptionIsThrownContainingMessage( @@ -64,6 +88,11 @@ public void deadbandThrowsErrorOnInvalidDeadband() { () -> ControlUtils.deadband(0.25, 1.9), "Instead, it was 1.9"); } + /** + * Tests that input values outside the valid range [-1, 1] throw an exception. + * + *

Control inputs must remain in [-1, 1]; any value outside this range is invalid. + */ @Test public void deadbandThrowsErrorOnValueOutOfRange() { assertIllegalArgumentExceptionIsThrownContainingMessage( diff --git a/lib/src/test/java/com/team2813/lib2813/util/FakeShuffleboardTabs.java b/lib/src/test/java/com/team2813/lib2813/util/FakeShuffleboardTabs.java index 13c0e16d..8e82c275 100644 --- a/lib/src/test/java/com/team2813/lib2813/util/FakeShuffleboardTabs.java +++ b/lib/src/test/java/com/team2813/lib2813/util/FakeShuffleboardTabs.java @@ -6,20 +6,45 @@ import edu.wpi.first.wpilibj.shuffleboard.ShuffleboardTab; import java.util.concurrent.atomic.AtomicInteger; +/** + * A fake implementation of {@link ShuffleboardTabs} for testing purposes. + * + *

Each instance of this class generates a unique prefix so that tab names do not collide with + * real Shuffleboard tabs during testing. This allows multiple tests to run in parallel without + * interfering with each other. + */ public final class FakeShuffleboardTabs implements ShuffleboardTabs { + + /** Generates unique IDs for each instance. */ private static final AtomicInteger nextValue = new AtomicInteger(1); + + /** Prefix added to every tab name to ensure uniqueness. */ private final String prefix; + /** Constructs a new {@link FakeShuffleboardTabs} instance with a unique prefix. */ public FakeShuffleboardTabs() { prefix = "f" + nextValue.getAndIncrement(); } + /** + * Returns a Shuffleboard tab with the given title, prefixed to avoid collisions. + * + * @param title The title of the tab (must be non-null) + * @return The Shuffleboard tab corresponding to this prefixed title + * @throws NullPointerException if title is null + */ @Override public ShuffleboardTab getTab(String title) { requireNonNullParam(title, "title", "getTab"); return Shuffleboard.getTab(prefix + title); } + /** + * Selects the Shuffleboard tab with the given title, prefixed to avoid collisions. + * + * @param title The title of the tab (must be non-null) + * @throws NullPointerException if title is null + */ @Override public void selectTab(String title) { requireNonNullParam(title, "title", "getTab"); diff --git a/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java b/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java index 7d5071f8..b30c5d5e 100644 --- a/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java +++ b/lib/src/test/java/com/team2813/lib2813/util/InputValidationTest.java @@ -8,26 +8,49 @@ import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; +/** + * Unit tests for the {@link InputValidation} utility class. + * + *

This test class specifically verifies that CAN ID validation behaves correctly, throwing an + * {@link InvalidCanIdException} for invalid IDs and returning the ID unchanged for valid IDs. + * + *

Uses the {@link Enclosed} runner to group tests logically in nested static classes. + */ @RunWith(Enclosed.class) public class InputValidationTest { - // Tests for the `InputValidation.checkCanId(...)` method. + + /** + * Tests for the {@link InputValidation#checkCanId(int)} method. + * + *

CAN IDs are valid in the range [0, 62]. This class ensures that invalid IDs throw {@link + * InvalidCanIdException} and that valid IDs return unchanged. + */ public static class CheckCanIdTest { + + /** + * Verifies that {@link InputValidation#checkCanId(int)} throws {@link InvalidCanIdException} + * for IDs outside the valid range [0, 62]. + */ @Test public void invalidCanId() { - // Can IDs can only valid in the range [0, 62]. int[] invalidCanIds = {-50, -1, 63, 100}; for (int invalidCanId : invalidCanIds) { InvalidCanIdException exception = assertThrows( InvalidCanIdException.class, () -> InputValidation.checkCanId(invalidCanId)); + // Verify that the exception contains the invalid ID assertThat(exception.getCanId()).isEqualTo(invalidCanId); - assertThat(exception).hasMessageThat().contains("is not a valid can id"); + // Verify that the exception message mentions invalid CAN ID + assertThat(exception).hasMessageThat().contains("is not a valid CAN ID"); } } + /** + * Verifies that {@link InputValidation#checkCanId(int)} returns the original ID for valid CAN + * IDs in the range [0, 62]. + */ @Test public void validCanID() { - // Can IDs can only valid in the range [0, 62]. int[] validCanIds = {0, 1, 10, 62}; for (int validCanId : validCanIds) { int returnValue = InputValidation.checkCanId(validCanId); diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/AprilTagMapPoseHelper.java b/limelight/src/main/java/com/team2813/lib2813/limelight/AprilTagMapPoseHelper.java index 7b82a2e2..c97bb46d 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/AprilTagMapPoseHelper.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/AprilTagMapPoseHelper.java @@ -15,19 +15,43 @@ import java.util.Set; import java.util.concurrent.Executors; +/** + * Helper class for working with AprilTag field maps and retrieving poses of tags. + * + *

This class interacts with a {@link LimelightClient} to optionally upload field maps to a + * Limelight device, and locally stores a {@link FiducialRetriever} for pose lookups. + */ class AprilTagMapPoseHelper { + + /** The Limelight client used to send HTTP requests to the camera. */ private final LimelightClient limelightClient; + + /** The local fiducial retriever, initialized when a field map is set. */ private FiducialRetriever retriever; + + /** Shared HTTP client used for uploading field maps to the Limelight. */ private static final HttpClient client = HttpClient.newBuilder() .connectTimeout(Duration.ofMillis(20)) .executor(Executors.newFixedThreadPool(1)) .build(); + /** + * Constructs a new helper using the specified Limelight client. + * + * @param client the Limelight client to use for HTTP interactions + */ public AprilTagMapPoseHelper(LimelightClient client) { this.limelightClient = client; } + /** + * Sets the field map for tag lookups and optionally uploads it to the Limelight. + * + * @param stream an {@link InputStream} containing the JSON-encoded field map + * @param updateLimelight if true, also uploads the field map to the connected Limelight + * @throws IOException if reading the input stream fails + */ public void setFieldMap(InputStream stream, boolean updateLimelight) throws IOException { if (!updateLimelight) { retriever = new FiducialRetriever(stream); @@ -49,16 +73,28 @@ public void setFieldMap(InputStream stream, boolean updateLimelight) throws IOEx } } + /** + * Returns the poses of the visible tags with the specified IDs. + * + * @param ids the set of tag IDs to retrieve + * @return a list of {@link Pose3d} for the tags found in the field map; empty if none + */ public List getVisibleTagPoses(Set ids) { if (retriever == null) { return List.of(); } return retriever.getFiducialMap().values().stream() - .filter(fidicual -> ids.contains(fidicual.getId())) + .filter(fiducial -> ids.contains(fiducial.getId())) .map(Fiducial::getPosition) .toList(); } + /** + * Returns the pose of a single tag by ID. + * + * @param id the ID of the tag + * @return an {@link Optional} containing the tag's {@link Pose3d} if present, otherwise empty + */ public Optional getTagPose(int id) { if (retriever == null) { return Optional.empty(); diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/BotPoseEstimate.java b/limelight/src/main/java/com/team2813/lib2813/limelight/BotPoseEstimate.java index 675a71d9..28b88757 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/BotPoseEstimate.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/BotPoseEstimate.java @@ -4,14 +4,22 @@ import java.util.Set; /** - * Represents an estimated position for the robot, obtained from the Limelight. + * Represents an estimated position of the robot, as reported by a Limelight or other vision system. * - * @param pose The estimated position of the robot. - * @param timestampSeconds The timestamp, in seconds, using the drivetrain clock - * @param visibleAprilTags All April Tags that are visible from the vision source. + *

This record includes the robot's pose, the timestamp of the observation, and a set of + * currently visible AprilTag IDs. + * + * @param pose The estimated position and orientation of the robot in 2D space. + * @param timestampSeconds The timestamp of this estimate, in seconds, relative to the drivetrain + * clock. + * @param visibleAprilTags The set of AprilTag IDs that were visible when this estimate was made. */ public record BotPoseEstimate(Pose2d pose, double timestampSeconds, Set visibleAprilTags) { + /** + * @deprecated Use the constructor including {@code visibleAprilTags}. This constructor + * automatically assigns an empty set of visible AprilTags. + */ @Deprecated public BotPoseEstimate(Pose2d pose, double timestampSeconds) { this(pose, timestampSeconds, Set.of()); diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/DataCollection.java b/limelight/src/main/java/com/team2813/lib2813/limelight/DataCollection.java index 9b99332d..e256d59c 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/DataCollection.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/DataCollection.java @@ -16,14 +16,33 @@ import java.util.concurrent.Executors; import org.json.JSONObject; +/** + * Background task that collects data from a Limelight camera via HTTP requests. + * + *

This class periodically fetches JSON data from the Limelight `/results` endpoint, timestamps + * the response, and stores the latest result for retrieval. + */ class DataCollection implements Runnable { + + /** Shared HTTP client with short timeout and fixed thread pool. */ private static final HttpClient client = HttpClient.newBuilder() .connectTimeout(Duration.ofMillis(20)) .executor(Executors.newFixedThreadPool(2)) .build(); + + /** HTTP request to retrieve Limelight results. */ private final HttpRequest dumpRequest; + /** Stores the most recent result and its response timestamp. */ + private volatile Optional lastResult; + + /** + * Constructs a DataCollection instance for a given Limelight client. + * + * @param limelightClient client for building HTTP requests + * @throws RuntimeException if creating the HTTP request fails + */ public DataCollection(LimelightClient limelightClient) { lastResult = Optional.empty(); try { @@ -33,44 +52,60 @@ public DataCollection(LimelightClient limelightClient) { } } + /** + * Represents a single Limelight result. + * + * @param json the parsed JSON object returned by Limelight + * @param responseTimestamp the time the response was received (seconds) + */ record Result(JSONObject json, double responseTimestamp) {} - private volatile Optional lastResult; - + /** Custom body handler to parse the HTTP response into a {@link Result} with timestamp. */ private static class JSONHandler implements BodyHandler { + @Override public BodySubscriber apply(ResponseInfo responseInfo) { - // Get the timestamp before we parse the JSON. + // Capture the timestamp before parsing the JSON body. double responseTimestamp = getCurrentTimeSeconds(); + // Map the string response body into a Result object return BodySubscribers.mapping( BodyHandlers.ofString(Charset.defaultCharset()).apply(responseInfo), - body -> { - return new Result(new JSONObject(body), responseTimestamp); - }); + body -> new Result(new JSONObject(body), responseTimestamp)); } } + /** Single reusable instance of the JSONHandler. */ private static final JSONHandler handler = new JSONHandler(); - private void updateJSON(HttpResponse obj) { - Result result = obj.body(); + /** Updates the most recent result from an HTTP response. */ + private void updateJSON(HttpResponse response) { + Result result = response.body(); lastResult = Optional.of(result); } + /** + * Executes the HTTP request to fetch Limelight results and stores the latest Result. + * + *

If the request is interrupted or fails, {@code lastResult} is cleared. + */ @Override public void run() { try { updateJSON(client.send(dumpRequest, handler)); } catch (InterruptedException e) { lastResult = Optional.empty(); - // background thread canceled - Thread.currentThread().interrupt(); + Thread.currentThread().interrupt(); // preserve interrupt status } catch (Exception e) { lastResult = Optional.empty(); } } + /** + * Returns the most recent fetched result. + * + * @return an {@link Optional} containing the latest {@link Result}, or empty if unavailable + */ public Optional getMostRecent() { return lastResult; } diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/JSONHelper.java b/limelight/src/main/java/com/team2813/lib2813/limelight/JSONHelper.java index 1cb97cc6..0a0c99ca 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/JSONHelper.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/JSONHelper.java @@ -6,7 +6,24 @@ import org.json.JSONException; import org.json.JSONObject; +/** + * Utility class for safely extracting values from JSONObjects returned by the Limelight. + * + *

All methods handle missing keys or invalid types gracefully by returning {@link Optional}. + * This avoids throwing unchecked {@link JSONException}s during normal processing. + */ class JSONHelper { + + /** + * Converts an integer value from a JSON object to a boolean. + * + *

The key must be present and contain 0 (false) or 1 (true). Returns {@link Optional#empty()} + * if the key is missing or the value is not an integer. + * + * @param obj the JSON object to read from + * @param key the key corresponding to the boolean-as-int + * @return {@link Optional} containing true if value is 1, false if 0, or empty if missing/invalid + */ static Optional getBooleanFromInt(JSONObject obj, String key) { if (!obj.has(key)) { return Optional.empty(); @@ -18,6 +35,15 @@ static Optional getBooleanFromInt(JSONObject obj, String key) { } } + /** + * Returns a function that retrieves a nested JSONObject by key. + * + *

The returned function returns {@link Optional#empty()} if the key is missing or not a JSON + * object. + * + * @param key the key to extract + * @return a function that extracts an Optional<JSONObject> from a parent JSONObject + */ static Function> getJSONObject(String key) { return (j) -> { if (!j.has(key)) { @@ -31,16 +57,30 @@ static Function> getJSONObject(String key) { }; } + /** + * Returns the "root" JSONObject for the Limelight results. + * + *

Older versions of the Limelight returned a root node called "Results". This method + * normalizes that so downstream code can always work with a consistent root object. + * + * @param json the raw JSON response from the Limelight + * @return the root JSONObject to extract fields from + */ static JSONObject getRoot(JSONObject json) { if (json.has("Results")) { - // This JSON was provided by an older version of the limelight code, - // which had a "Results" root node. return json.getJSONObject("Results"); } else { return json; } } + /** + * Retrieves a long value from a JSONObject by key. + * + * @param obj the JSON object to read from + * @param key the key corresponding to the long value + * @return Optional containing the long value, or empty if missing or invalid + */ static Optional getLong(JSONObject obj, String key) { if (!obj.has(key)) { return Optional.empty(); @@ -52,6 +92,13 @@ static Optional getLong(JSONObject obj, String key) { } } + /** + * Retrieves a double value from a JSONObject by key. + * + * @param obj the JSON object to read from + * @param key the key corresponding to the double value + * @return Optional containing the double value, or empty if missing or invalid + */ static Optional getDouble(JSONObject obj, String key) { if (!obj.has(key)) { return Optional.empty(); @@ -63,6 +110,13 @@ static Optional getDouble(JSONObject obj, String key) { } } + /** + * Retrieves a JSONArray from a JSONObject by key. + * + * @param obj the JSON object to read from + * @param key the key corresponding to the JSON array + * @return Optional containing the JSONArray, or empty if missing or invalid + */ static Optional getArr(JSONObject obj, String key) { if (!obj.has(key)) { return Optional.empty(); diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/Limelight.java b/limelight/src/main/java/com/team2813/lib2813/limelight/Limelight.java index 7caae133..c5d80529 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/Limelight.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/Limelight.java @@ -9,45 +9,64 @@ import java.util.Set; import org.json.JSONObject; +/** + * Interface for interacting with a Limelight vision camera. + * + *

Provides methods for retrieving locational data, working with field maps, and accessing raw + * JSON output. Some legacy methods are marked {@link Deprecated} in favor of {@link + * LocationalData}-based APIs. + */ public interface Limelight { /** - * Gets the limelight with the default name. + * Gets the Limelight with the default name. * - * @return the {@link Limelight} object for interfacing with the limelight + * @return the {@link Limelight} object for interfacing with the default camera */ static Limelight getDefaultLimelight() { return RestLimelight.getDefaultLimelight(); } /** - * @deprecated use methods in {@link LocationalData} that return a {@link BotPoseEstimate}. + * Returns the timestamp of the most recent capture, in seconds. + * + * @deprecated Use methods in {@link LocationalData} that return a {@link BotPoseEstimate}. + * @return an {@link OptionalDouble} containing the timestamp if available */ @Deprecated OptionalDouble getTimestamp(); /** - * Returns {@code true} if the limelight has identified a target. + * Returns true if the Limelight currently has a valid target. * - * @deprecated use {@link LocationalData#hasTarget()} + * @deprecated Use {@link LocationalData#hasTarget()} instead. + * @return true if a target is detected */ @Deprecated boolean hasTarget(); - /** Gets an object for getting locational data. */ + /** Gets an object for retrieving locational data from the Limelight. */ LocationalData getLocationalData(); + /** + * Sets the field map for the Limelight from an input stream. Optionally, this can upload the map + * to the Limelight. + * + * @param stream the input stream containing the field map + * @param updateLimelight whether to update the Limelight with this map + * @throws IOException if reading from the stream fails + */ void setFieldMap(InputStream stream, boolean updateLimelight) throws IOException; /** - * Sets the field map for the limelight with a file in the deploy directory. Additionally, this - * may also upload the field map to the Limelight if desired. This will likely be a slow - * operation, and should not be regularly called. + * Sets the field map for the Limelight using a file in the deploy directory. + * + *

This method opens the file and delegates to {@link #setFieldMap(InputStream, boolean)}. + * Uploading the map to the Limelight can be slow and should not be called frequently. * - * @param filepath The path to the file from the deploy directory (using UNIX file seperators) - * @param updateLimelight If the limelight should be updated with this field map - * @throws IOException If the given filepath does not exist in the deploy directory or could not - * be read + * @param filepath path to the file relative to the deploy directory (use UNIX file separators) + * @param updateLimelight whether to update the Limelight with this map + * @throws IOException if the file does not exist or cannot be read */ default void setFieldMap(String filepath, boolean updateLimelight) throws IOException { File file = new File(Filesystem.getDeployDirectory(), filepath); @@ -57,23 +76,30 @@ default void setFieldMap(String filepath, boolean updateLimelight) throws IOExce } /** - * Gets the locations of the given AprilTags. + * Returns the 3D poses of the specified visible AprilTags. * - * @deprecated use {@link LocationalData#getVisibleAprilTagPoses()} + * @deprecated Use {@link LocationalData#getVisibleAprilTagPoses()} instead. + * @param visibleTags a set of AprilTag IDs to locate + * @return a list of {@link Pose3d} objects representing each tag's position */ @Deprecated List getLocatedAprilTags(Set visibleTags); /** - * @deprecated use {@link LocationalData#getCaptureLatency()} + * Returns the capture latency for the most recent Limelight frame, in seconds. + * + * @deprecated Use {@link LocationalData#getCaptureLatency()} instead. + * @return an {@link OptionalDouble} containing the latency if available */ @Deprecated OptionalDouble getCaptureLatency(); /** - * Gets the most recent JSON from the Limelight. Does not work for all implementations. + * Returns the most recent raw JSON dump from the Limelight. * - * @deprecated use {@link LocationalData#isValid()}. + * @deprecated Use {@link LocationalData#isValid()} instead. Not all Limelight implementations + * support this method. + * @return an {@link Optional} containing the JSON object if available */ @Deprecated Optional getJsonDump(); diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/LimelightClient.java b/limelight/src/main/java/com/team2813/lib2813/limelight/LimelightClient.java index aa83e253..ca003b09 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/LimelightClient.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/LimelightClient.java @@ -5,30 +5,61 @@ import java.net.URISyntaxException; import java.net.http.HttpRequest; +/** + * A client for communicating with a Limelight camera over HTTP. + * + *

Provides functionality for building HTTP requests to the Limelight's REST API. + */ final class LimelightClient { + + /** The default HTTP port used by Limelight cameras. */ static final int DEFAULT_PORT = 5807; + + /** The hostname or IP address of the Limelight. */ private final String hostname; + + /** The port number to use for HTTP connections. */ private final int port; + /** + * Exception thrown when an HTTP request could not be constructed. + * + *

Wraps a {@link URISyntaxException} to provide additional context about the failure. + */ static class HttpRequestException extends IOException { public HttpRequestException(String message, URISyntaxException e) { super(message, e); } } + /** + * Constructs a LimelightClient using the default port. + * + * @param hostname the hostname or IP address of the Limelight + */ LimelightClient(String hostname) { this(hostname, DEFAULT_PORT); } + /** + * Constructs a LimelightClient with a specific port. + * + * @param hostname the hostname or IP address of the Limelight + * @param port the HTTP port to use + */ LimelightClient(String hostname, int port) { this.hostname = hostname; this.port = port; } /** - * Creates an HttpRequest builder for the limelight with the given path. + * Creates an {@link HttpRequest.Builder} for the Limelight with the given path. + * + *

The URI is automatically constructed using the hostname and port configured for this client. * - * @throws HttpRequestException If the request could not be created + * @param path the HTTP path (e.g., "/results" or "/upload-fieldmap") + * @return an {@link HttpRequest.Builder} ready for further configuration + * @throws HttpRequestException if the URI is invalid or cannot be created */ HttpRequest.Builder newRequestBuilder(String path) throws HttpRequestException { try { diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/LocationalData.java b/limelight/src/main/java/com/team2813/lib2813/limelight/LocationalData.java index fa74f822..b7ea4b7b 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/LocationalData.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/LocationalData.java @@ -7,55 +7,77 @@ import java.util.Set; /** - * Get positional data from limelight + * Interface for retrieving positional and vision data from a Limelight camera. + * + *

This includes robot pose estimates, visible AprilTags, and latency measurements. * * @see Limelight */ public interface LocationalData { - /** Returns {@code true} if the limelight has identified a target. */ + /** + * Returns {@code true} if the Limelight has identified at least one target in the current frame. + * + * @return {@code true} if a target is detected + */ boolean hasTarget(); - /** Returns {@code true} if the limelight has provided a valid response. */ + /** + * Returns {@code true} if the Limelight has provided a valid response for this frame. + * + * @return {@code true} if the data is valid + */ boolean isValid(); /** - * Gets the position of the robot with the center of the field as the origin. + * Gets the robot's 3D position relative to the center of the field. * - * @return The position of the robot + * @return the robot's pose, if available */ Optional getBotpose(); - /** Gets the estimated position of the robot with the center of the field as the origin. */ + /** + * Gets an estimated robot pose relative to the center of the field, including visible AprilTags. + * + * @return the estimated robot pose + */ Optional getBotPoseEstimate(); /** - * Gets the position of the robot with the blue driverstation as the origin + * Gets the robot's 3D position relative to the blue driver station as origin. * - * @return The position of the robot + * @return the robot's pose, if available */ Optional getBotposeBlue(); - /** Gets the estimated position of the robot with the blue driverstation as the origin. */ + /** + * Gets an estimated robot pose relative to the blue driver station as origin. + * + * @return the estimated robot pose + */ Optional getBotPoseEstimateBlue(); /** - * Gets the position of the robot with the red driverstation as the origin + * Gets the robot's 3D position relative to the red driver station as origin. * - * @return The position of the robot + * @return the robot's pose, if available */ Optional getBotposeRed(); - /** Gets the estimated position of the robot with the red driverstation as the origin. */ + /** + * Gets an estimated robot pose relative to the red driver station as origin. + * + * @return the estimated robot pose + */ Optional getBotPoseEstimateRed(); /** * Capture latency in milliseconds. * - *

Per the Limelight docs, this is the time between the end of the exposure of the middle row - * to the beginning of the tracking loop. + *

This is the time between the end of the exposure of the middle row and the beginning of the + * tracking loop. * - * @deprecated Use {@link #getBotPoseEstimateBlue()} or {@link #getBotPoseEstimateRed()} + * @deprecated Use {@link #getBotPoseEstimateBlue()} or {@link #getBotPoseEstimateRed()} instead */ @Deprecated OptionalDouble getCaptureLatency(); @@ -63,38 +85,49 @@ public interface LocationalData { /** * Targeting latency in milliseconds. * - *

Per the Limelight docs, this is the time consumed by the tracking loop this frame. + *

This is the time taken by the tracking loop for this frame. * - * @deprecated Use {@link #getBotPoseEstimateBlue()} or {@link #getBotPoseEstimateRed()} + * @deprecated Use {@link #getBotPoseEstimateBlue()} or {@link #getBotPoseEstimateRed()} instead */ @Deprecated OptionalDouble getTargetingLatency(); /** - * @deprecated use methods that return a {@link BotPoseEstimate}. + * @deprecated Use methods that return a {@link BotPoseEstimate} instead */ @Deprecated OptionalDouble getTimestamp(); + /** + * Returns the sum of capture latency and targeting latency in milliseconds, if both are + * available. + * + * @return the total latency in milliseconds + * @deprecated Use {@link #getBotPoseEstimateBlue()} or {@link #getBotPoseEstimateRed()} instead + */ @Deprecated default OptionalDouble lastMSDelay() { - OptionalDouble a = getCaptureLatency(); - OptionalDouble b = getTargetingLatency(); - if (a.isPresent() && b.isPresent()) { - return OptionalDouble.of(a.getAsDouble() + b.getAsDouble()); + OptionalDouble capture = getCaptureLatency(); + OptionalDouble targeting = getTargetingLatency(); + if (capture.isPresent() && targeting.isPresent()) { + return OptionalDouble.of(capture.getAsDouble() + targeting.getAsDouble()); } return OptionalDouble.empty(); } /** - * Gets the set of all visible tags + * Gets the IDs of all visible AprilTags in the current frame. * - * @return The visible tags - * @deprecated use {@link #getVisibleAprilTagPoses()} + * @return the set of visible tag IDs + * @deprecated use {@link #getVisibleAprilTagPoses()} instead */ @Deprecated Set getVisibleTags(); - /** Gets the visible AprilTags as a map from ID to position. */ + /** + * Returns a map of visible AprilTags, keyed by their ID, with values as their 3D positions. + * + * @return a map of visible AprilTags + */ Map getVisibleAprilTagPoses(); } diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/NetworkTablesLimelight.java b/limelight/src/main/java/com/team2813/lib2813/limelight/NetworkTablesLimelight.java index 9579935a..1269e44d 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/NetworkTablesLimelight.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/NetworkTablesLimelight.java @@ -9,17 +9,26 @@ import java.io.IOException; import java.io.InputStream; import java.util.*; -import java.util.Arrays; -import java.util.Optional; -import java.util.OptionalDouble; import java.util.stream.Collectors; import org.json.JSONObject; +/** + * Implementation of {@link Limelight} that retrieves data from the NetworkTables interface of a + * Limelight camera. + */ class NetworkTablesLimelight implements Limelight { + + /** Sentinel array of zeros used to detect uninitialized poses. */ private static final double[] ZEROS = new double[6]; + private final String limelightName; private final AprilTagMapPoseHelper aprilTagMapPoseHelper; + /** + * Constructs a NetworkTablesLimelight instance. + * + * @param limelightName The hostname or NetworkTables name of the Limelight. + */ NetworkTablesLimelight(String limelightName) { this.limelightName = limelightName; aprilTagMapPoseHelper = new AprilTagMapPoseHelper(new LimelightClient(limelightName)); @@ -37,8 +46,7 @@ public boolean hasTarget() { @Override public void setFieldMap(InputStream stream, boolean updateLimelight) throws IOException { - // The updateLimelight assumes we have the hostname of the limelight, which we don't. For now, - // this won't update the limelight. + // For NT-based Limelight, we do not support uploading field maps over the network yet. aprilTagMapPoseHelper.setFieldMap(stream, false); } @@ -49,6 +57,7 @@ public List getLocatedAprilTags(Set visibleTags) { @Override public Optional getJsonDump() { + // This implementation does not provide raw JSON dumps return Optional.empty(); } @@ -59,9 +68,11 @@ public OptionalDouble getCaptureLatency() { @Override public LocationalData getLocationalData() { - LimelightHelpers.LimelightResults results = LimelightHelpers.getLatestResults(limelightName); + LimelightResults results = LimelightHelpers.getLatestResults(limelightName); + if (results.error == null && results.valid) { Map aprilTags = getVisibleAprilTagPoses(results); + var poseEstimate = toBotPoseEstimate(LimelightHelpers.getBotPoseEstimate(limelightName), aprilTags.keySet()); var redPoseEstimate = @@ -70,9 +81,11 @@ public LocationalData getLocationalData() { var bluePoseEstimate = toBotPoseEstimate( LimelightHelpers.getBotPoseEstimate_wpiBlue(limelightName), aprilTags.keySet()); + return new NTLocationalData( results, poseEstimate, redPoseEstimate, bluePoseEstimate, aprilTags); } + return StubLocationalData.INVALID; } @@ -94,7 +107,9 @@ private Map getVisibleAprilTagPoses(LimelightResults results) { return unmodifiableMap(map); } + /** Inner class implementing {@link LocationalData} backed by NetworkTables results. */ private class NTLocationalData implements LocationalData { + private final LimelightResults results; private final Optional poseEstimate; private final Optional redPoseEstimate; diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/Optionals.java b/limelight/src/main/java/com/team2813/lib2813/limelight/Optionals.java index 1002184b..33c97f50 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/Optionals.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/Optionals.java @@ -4,16 +4,40 @@ import java.util.OptionalDouble; import java.util.OptionalLong; +/** + * Utility class for working with {@link Optional} values. + * + *

Provides methods to convert boxed {@link Optional} types to their corresponding primitive + * {@code Optional} types. This class is non-instantiable. + */ final class Optionals { - // make class non-instantiable + + /** + * Private constructor to prevent instantiation. Invoking this constructor will always throw an + * {@link AssertionError}. + */ private Optionals() { throw new AssertionError("cannot create Optionals instance"); } + /** + * Converts an {@link Optional} of {@link Long} to an {@link OptionalLong}. + * + * @param val the {@code Optional} to convert + * @return an {@code OptionalLong} containing the value if present, or an empty {@code + * OptionalLong} if not + */ static OptionalLong unboxLong(Optional val) { return val.map(OptionalLong::of).orElseGet(OptionalLong::empty); } + /** + * Converts an {@link Optional} of {@link Double} to an {@link OptionalDouble}. + * + * @param val the {@code Optional} to convert + * @return an {@code OptionalDouble} containing the value if present, or an empty {@code + * OptionalDouble} if not + */ static OptionalDouble unboxDouble(Optional val) { return val.map(OptionalDouble::of).orElseGet(OptionalDouble::empty); } diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/RestLimelight.java b/limelight/src/main/java/com/team2813/lib2813/limelight/RestLimelight.java index e104b672..277501b0 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/RestLimelight.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/RestLimelight.java @@ -19,24 +19,52 @@ import org.json.JSONArray; import org.json.JSONObject; +/** + * REST-based implementation of the Limelight interface for communicating with a Limelight vision + * camera. This class manages a connection to a Limelight device over HTTP and provides methods for + * retrieving vision targeting data, robot pose estimates, and AprilTag information. + * + *

Instances are managed through static factory methods and are cached based on their network + * address. The class uses a scheduled executor service to periodically collect data from the + * Limelight device. + */ class RestLimelight implements Limelight { + /** Map of cached Limelight instances keyed by their network address. */ private static final Map limelights = new HashMap<>(); + + /** Helper for managing AprilTag field map poses. */ private final AprilTagMapPoseHelper aprilTagMapPoseHelper; + + /** Thread for collecting data from the Limelight. */ private final DataCollection collectionThread; + + /** Shared executor service for all Limelight instances. */ private static final ScheduledExecutorService executor = Executors.newScheduledThreadPool(2); + /** Default network address for the Limelight device. */ static final String DEFAULT_ADDRESS = "limelight.local"; + /** Future representing the scheduled data collection task. */ private ScheduledFuture thread; + /** Whether the data collection thread has been started. */ boolean started = false; + /** + * Creates a new RestLimelight instance with the specified network address. + * + * @param address the hostname or IP address of the Limelight device + */ RestLimelight(String address) { var limelightClient = new LimelightClient(address); collectionThread = new DataCollection(limelightClient); aprilTagMapPoseHelper = new AprilTagMapPoseHelper(limelightClient); } + /** + * Starts the periodic data collection thread if it hasn't been started already. The thread runs + * at a fixed rate of 40ms with an initial delay of 20ms. + */ void start() { if (!started) { thread = executor.scheduleAtFixedRate(collectionThread, 20, 40, TimeUnit.MILLISECONDS); @@ -44,19 +72,31 @@ void start() { } } + /** + * Manually runs one iteration of the data collection thread. This is primarily useful for testing + * purposes. + */ void runThread() { collectionThread.run(); } + /** {@inheritDoc} */ @Override public Optional getJsonDump() { return collectionThread.getMostRecent().map(DataCollection.Result::json); } + /** + * Gets the capture latency of the most recent frame from the Limelight. + * + * @return an OptionalDouble containing the capture latency in milliseconds, or empty if + * unavailable + */ public OptionalDouble getCaptureLatency() { return getLocationalData().getCaptureLatency(); } + /** {@inheritDoc} */ @Override public OptionalDouble getTimestamp() { return getLocationalData().getTimestamp(); @@ -66,33 +106,57 @@ public OptionalDouble getTimestamp() { * Sets the field map for the limelight. Additionally, this may also upload the field map to the * Limelight if desired. This will likely be a slow operation, and should not be regularly called. * - * @param stream The reader which - * @param updateLimelight If the limelight should be updated with this field map + * @param stream the input stream containing the field map data + * @param updateLimelight if true, uploads the field map to the Limelight device + * @throws IOException if an I/O error occurs while reading the stream or uploading to the device */ @Override public void setFieldMap(InputStream stream, boolean updateLimelight) throws IOException { aprilTagMapPoseHelper.setFieldMap(stream, updateLimelight); } + /** {@inheritDoc} */ @Override public List getLocatedAprilTags(Set visibleTags) { return aprilTagMapPoseHelper.getVisibleTagPoses(visibleTags); } + /** + * Creates a negation function that inverts the boolean result of another function. + * + * @param the type of the function input + * @param fnc the function to negate + * @return a function that returns the opposite boolean value + */ private static Function not(Function fnc) { return (t) -> !fnc.apply(t); } + /** + * Checks if the Limelight currently has a valid target in view. + * + * @return true if a target is detected, false otherwise + */ public boolean hasTarget() { return getLocationalData().hasTarget(); } + /** + * Gets the most recent locational data from the Limelight. + * + * @return LocationalData containing pose estimates, tag information, and latency data, or a stub + * with valid defaults if no data is available + */ public LocationalData getLocationalData() { Optional locationalData = collectionThread.getMostRecent().map(RestLocationalData::new); return locationalData.orElse(StubLocationalData.VALID); } + /** + * Cleans up resources by canceling the data collection thread and waiting for termination. Logs + * any interruption errors to the DriverStation. + */ private void clean() { try { thread.cancel(true); @@ -115,9 +179,10 @@ public static Limelight getDefaultLimelight() { * Gets the limelight with the specified name. Calling with a blank {@code limelightName} is * equivalent to calling {@link #getDefaultLimelight()} * - * @param limelightAddress The hostname or ip address of the limelight + * @param limelightAddress the hostname or IP address of the limelight * @return the {@link Limelight} object for interfacing with the limelight - * @throws NullPointerException if {@code limelightName} is null + * @throws NullPointerException if {@code limelightAddress} is null + * @throws IllegalArgumentException if {@code limelightAddress} is empty */ public static Limelight getLimelight(String limelightAddress) { String addr = Objects.requireNonNull(limelightAddress, "limelightAddress shouldn't be null"); @@ -129,6 +194,10 @@ public static Limelight getLimelight(String limelightAddress) { return result; } + /** + * Clears all cached Limelight instances and cleans up their resources. This method should be + * called when shutting down or resetting the system. + */ static void eraseInstances() { for (RestLimelight limelight : limelights.values()) { limelight.clean(); @@ -136,25 +205,46 @@ static void eraseInstances() { limelights.clear(); } + /** + * Implementation of LocationalData that parses data from JSON responses received from the + * Limelight. This class provides access to robot pose estimates, AprilTag information, and timing + * data. + */ private class RestLocationalData implements LocationalData { + /** The root JSON object containing all Limelight data. */ private final JSONObject root; + + /** The timestamp when the response was received, in seconds. */ private final double responseTimestamp; + /** + * Creates a new RestLocationalData instance from a data collection result. + * + * @param result the result containing JSON data and timestamp + */ RestLocationalData(DataCollection.Result result) { this.root = getRoot(result.json()); this.responseTimestamp = result.responseTimestamp(); } + /** {@inheritDoc} */ @Override public boolean isValid() { return getBooleanFromInt(root, "v").orElse(false); } + /** {@inheritDoc} */ @Override public boolean hasTarget() { return getArr(root, "Fiducial").map(not(JSONArray::isEmpty)).orElse(false); } + /** + * Checks if a pose array is invalid or contains only zero values. + * + * @param arr the JSON array to validate + * @return true if the array is invalid (wrong length, no target, or all zeros), false otherwise + */ private boolean invalidArray(JSONArray arr) { boolean simple = arr.length() != 6 || !hasTarget(); if (simple) { @@ -170,6 +260,13 @@ private boolean invalidArray(JSONArray arr) { return true; } + /** + * Parses a JSON array into a Pose3d object. The array is expected to contain 6 elements: [x, y, + * z, roll, pitch, yaw] where rotation values are in degrees. + * + * @param arr the JSON array containing pose data + * @return an Optional containing the parsed Pose3d, or empty if the array is invalid + */ private Optional parseArr(JSONArray arr) { if (invalidArray(arr)) { return Optional.empty(); @@ -183,56 +280,75 @@ private Optional parseArr(JSONArray arr) { new Pose3d(arr.getDouble(0), arr.getDouble(1), arr.getDouble(2), rotation)); } + /** {@inheritDoc} */ @Override public OptionalDouble getTimestamp() { return unboxDouble(getDouble(root, "ts")); } + /** {@inheritDoc} */ @Override public OptionalDouble getCaptureLatency() { return unboxDouble(getDouble(root, "cl")); } + /** {@inheritDoc} */ @Override public OptionalDouble getTargetingLatency() { return unboxDouble(getDouble(root, "tl")); } + /** {@inheritDoc} */ @Override public Optional getBotpose() { return getArr(root, "botpose").flatMap(this::parseArr); } + /** {@inheritDoc} */ @Override public Optional getBotPoseEstimate() { return getArr(root, "botpose").flatMap(this::parseArr).map(this::toBotPoseEstimate); } + /** {@inheritDoc} */ @Override public Optional getBotposeBlue() { return getArr(root, "botpose_wpiblue").flatMap(this::parseArr); } + /** {@inheritDoc} */ @Override public Optional getBotPoseEstimateBlue() { return getArr(root, "botpose_wpiblue").flatMap(this::parseArr).map(this::toBotPoseEstimate); } /** - * Gets the position of the robot with the red driverstation as the origin + * Gets the position of the robot with the red driverstation as the origin. * - * @return The position of the robot + * @return an Optional containing the robot's Pose3d in red alliance coordinates, or empty if + * unavailable */ @Override public Optional getBotposeRed() { return getArr(root, "botpose_wpired").flatMap(this::parseArr); } + /** {@inheritDoc} */ @Override public Optional getBotPoseEstimateRed() { return getArr(root, "botpose_wpired").flatMap(this::parseArr).map(this::toBotPoseEstimate); } + /** + * Converts a Pose3d into a BotPoseEstimate with timestamp and visible tag information. The + * timestamp is calculated by subtracting the total latency from the response timestamp. + * + * @param pose the 3D pose to convert + * @return a BotPoseEstimate with the 2D pose projection, adjusted timestamp, and visible tags + * @see Chief + * Delphi Discussion + */ private BotPoseEstimate toBotPoseEstimate(Pose3d pose) { // See // https://www.chiefdelphi.com/t/timestamp-parameter-when-adding-limelight-vision-to-odometry @@ -242,11 +358,16 @@ private BotPoseEstimate toBotPoseEstimate(Pose3d pose) { pose.toPose2d(), timestampSeconds, getVisibleAprilTagPoses().keySet()); } - /** Gets the id of the targeted tag. */ + /** + * Gets the ID of the currently targeted AprilTag. + * + * @return an OptionalLong containing the tag ID, or empty if no tag is targeted + */ OptionalLong getTagID() { return unboxLong(getLong(root, "pID")); } + /** {@inheritDoc} */ @Override public Set getVisibleTags() { return getArr(root, "Fiducial") @@ -264,6 +385,7 @@ public Set getVisibleTags() { .orElseGet(Set::of); } + /** {@inheritDoc} */ @Override public Map getVisibleAprilTagPoses() { return getArr(root, "Fiducial") diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/StubLocationalData.java b/limelight/src/main/java/com/team2813/lib2813/limelight/StubLocationalData.java index a36d4075..e698234b 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/StubLocationalData.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/StubLocationalData.java @@ -3,82 +3,174 @@ import edu.wpi.first.math.geometry.Pose3d; import java.util.*; -/** Implementation of LocationalData where all optional values return empty values. */ +/** + * Stub implementation of LocationalData where all optional values return empty. + * + *

This class provides a null-object pattern implementation for LocationalData, useful as a + * fallback when real Limelight data is unavailable. It can represent either valid or invalid data + * states, but always returns empty optionals for all pose and timing information. + * + *

Two singleton instances are provided: + * + *

    + *
  • {@link #VALID} - represents valid but empty data + *
  • {@link #INVALID} - represents invalid/disconnected state + *
+ */ final class StubLocationalData implements LocationalData { + /** Singleton instance representing valid but empty locational data. */ static final StubLocationalData VALID = new StubLocationalData(true); + + /** Singleton instance representing invalid locational data. */ static final StubLocationalData INVALID = new StubLocationalData(false); + /** Whether this stub data should be considered valid. */ private final boolean valid; + /** + * Creates a new StubLocationalData instance. + * + * @param valid whether this stub data should report as valid + */ private StubLocationalData(boolean valid) { this.valid = valid; } + /** + * {@inheritDoc} + * + * @return the validity state set during construction + */ @Override public boolean isValid() { return valid; } + /** + * {@inheritDoc} + * + * @return always false, as stub data never has targets + */ @Override public boolean hasTarget() { return false; } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public Optional getBotpose() { return Optional.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public Optional getBotPoseEstimate() { return Optional.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public Optional getBotposeBlue() { return Optional.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public Optional getBotPoseEstimateBlue() { return Optional.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public Optional getBotposeRed() { return Optional.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public Optional getBotPoseEstimateRed() { return Optional.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public OptionalDouble getCaptureLatency() { return OptionalDouble.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public OptionalDouble getTargetingLatency() { return OptionalDouble.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public OptionalDouble getTimestamp() { return OptionalDouble.empty(); } + /** + * {@inheritDoc} + * + * @return always empty + */ @Override public OptionalDouble lastMSDelay() { return OptionalDouble.empty(); } + /** + * {@inheritDoc} + * + * @return always an empty, immutable set + */ @Override public Set getVisibleTags() { return Set.of(); } + /** + * {@inheritDoc} + * + * @return always an empty, immutable map + */ @Override public Map getVisibleAprilTagPoses() { return Collections.emptyMap(); diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/Fiducial.java b/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/Fiducial.java index afa73bef..185dc9d1 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/Fiducial.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/Fiducial.java @@ -4,14 +4,39 @@ import edu.wpi.first.math.Nat; import edu.wpi.first.math.geometry.Pose3d; +/** + * Represents a fiducial (AprilTag) in 3D space. + * + *

This class stores the fiducial's unique ID and its pose as a 4x4 transformation matrix. It can + * convert this transform into a {@link Pose3d} object for use with WPILib's geometry and odometry + * systems. + */ public class Fiducial { + + /** The unique identifier of this fiducial. */ private int id; + + /** + * The 4x4 transformation matrix representing the fiducial's pose in 3D space. + * + *

The matrix is stored in row-major order as a 16-element array. + */ private double[] transform; + /** + * Returns the 3D pose of this fiducial as a {@link Pose3d}. + * + * @return the fiducial's pose in 3D space + */ public Pose3d getPosition() { return new Pose3d(new Matrix<>(Nat.N4(), Nat.N4(), transform)); } + /** + * Returns the unique ID of this fiducial. + * + * @return the fiducial's ID + */ public int getId() { return id; } diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FiducialRetriever.java b/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FiducialRetriever.java index cbe78e56..c917be01 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FiducialRetriever.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FiducialRetriever.java @@ -9,16 +9,39 @@ import java.util.Map; import java.util.stream.Collectors; +/** + * Retrieves and stores fiducial (AprilTag) information from a JSON input stream. + * + *

The JSON is expected to represent a {@link FieldMap}, which contains an array of {@link + * Fiducial} objects. Each fiducial is mapped by its unique ID for easy lookup. + */ public class FiducialRetriever { + + /** Gson instance used for deserializing JSON. */ private static final Gson gson = new Gson(); + + /** Map of fiducial ID to {@link Fiducial} object. */ private final Map fiducialMap; + /** + * Constructs a new {@link FiducialRetriever} by reading fiducials from the given input stream. + * + * @param stream an {@link InputStream} containing a JSON representation of a {@link FieldMap} + * @throws com.google.gson.JsonSyntaxException if the JSON is invalid + * @throws java.io.UncheckedIOException if there is an error reading from the stream + */ public FiducialRetriever(InputStream stream) { FieldMap map = gson.fromJson(new InputStreamReader(stream, UTF_8), FieldMap.class); + // Convert array of fiducials to an unmodifiable map keyed by fiducial ID fiducialMap = Arrays.stream(map.fiducials).collect(Collectors.toUnmodifiableMap(Fiducial::getId, f -> f)); } + /** + * Returns an unmodifiable map of fiducials keyed by their ID. + * + * @return a {@link Map} of fiducial ID to {@link Fiducial} + */ public Map getFiducialMap() { return fiducialMap; } diff --git a/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FieldMap.java b/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FieldMap.java index 064a5de4..3e57e6b9 100644 --- a/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FieldMap.java +++ b/limelight/src/main/java/com/team2813/lib2813/limelight/apriltag_map/FieldMap.java @@ -1,5 +1,24 @@ package com.team2813.lib2813.limelight.apriltag_map; +/** + * Represents a collection of fiducials (AprilTags) in a field map. + * + *

This class is intended to be deserialized from JSON. The JSON should contain an array of + * fiducials under the key "fiducials". Each fiducial is represented by a {@link Fiducial} object. + * + *

Example JSON format: + * + *

+ * {
+ *   "fiducials": [
+ *     { "id": 1, "transform": [ ...16 values... ] },
+ *     { "id": 2, "transform": [ ...16 values... ] }
+ *   ]
+ * }
+ * 
+ */ class FieldMap { + + /** Array of fiducials present in the field map. */ public Fiducial[] fiducials; } diff --git a/limelight/src/test/java/com/team2813/lib2813/limelight/FakeLimelight.java b/limelight/src/test/java/com/team2813/lib2813/limelight/FakeLimelight.java index 0ef52898..34a12a4a 100644 --- a/limelight/src/test/java/com/team2813/lib2813/limelight/FakeLimelight.java +++ b/limelight/src/test/java/com/team2813/lib2813/limelight/FakeLimelight.java @@ -13,10 +13,46 @@ import org.json.JSONObject; import org.junit.rules.ExternalResource; +/** + * Mock HTTP server that simulates a Limelight camera for testing purposes. + * + *

This class extends JUnit's {@link ExternalResource} to provide automatic setup and teardown of + * a fake HTTP server that mimics the Limelight REST API. It listens on port 5807 and handles two + * endpoints: + * + *

    + *
  • {@code /results} - GET endpoint that returns vision data in JSON format + *
  • {@code /upload-fieldmap} - POST endpoint that accepts field map uploads + *
+ * + *

Usage example in a JUnit test: + * + *

{@code
+ * @Rule
+ * public FakeLimelight fakeLimelight = new FakeLimelight();
+ *
+ * @Test
+ * public void testLimelightConnection() {
+ *     JSONObject response = new JSONObject();
+ *     response.put("v", 1);
+ *     fakeLimelight.setResultsResponse(response);
+ *     // Test code here
+ * }
+ * }
+ */ public class FakeLimelight extends ExternalResource { + /** Logger for tracking fake Limelight activity. */ private static final Logger logger = Logger.getLogger("FakeLimelight"); + + /** The underlying HTTP server instance. */ HttpServer server; + /** + * Sets up the fake Limelight server before each test. Creates an HTTP server on port 5807 with + * endpoints for results and field map uploads. + * + * @throws Throwable if server creation or startup fails + */ @Override protected void before() throws Throwable { server = HttpServer.create(new InetSocketAddress(5807), 0); @@ -26,23 +62,50 @@ protected void before() throws Throwable { server.start(); } + /** + * Tears down the fake Limelight server after each test. Stops the server with a 2-second grace + * period and clears the server reference. + */ @Override protected void after() { server.stop(2); server = null; } + /** + * HTTP handler that simulates the Limelight's GET /results endpoint. + * + *

This handler responds to GET requests with a configurable JSON body representing vision data + * from the Limelight. It returns 405 Method Not Allowed for non-GET requests. + */ private static class FakeGet implements HttpHandler { + /** The JSON response body to return. */ private String body; + /** + * Sets the response body that will be returned by GET requests. + * + * @param body the JSON string to return + */ public void setBody(String body) { this.body = body; } + /** + * Gets the current response body. + * + * @return the JSON string that will be returned + */ public String getBody() { return body; } + /** + * Handles incoming HTTP requests to the /results endpoint. + * + * @param exchange the HTTP exchange containing request and response + * @throws IOException if an I/O error occurs during response writing + */ @Override public void handle(HttpExchange exchange) throws IOException { logger.info("Request for results received"); @@ -58,9 +121,23 @@ public void handle(HttpExchange exchange) throws IOException { } } + /** + * HTTP handler that simulates the Limelight's POST /upload-fieldmap endpoint. + * + *

This handler accepts POST requests with field map data and stores the uploaded content for + * verification in tests. It returns 405 Method Not Allowed for non-POST requests. + */ private static class FakeFieldMap implements HttpHandler { + /** The most recently uploaded field map content. */ private String post; + /** + * Handles incoming HTTP requests to the /upload-fieldmap endpoint. Reads and stores the POST + * body content for later retrieval. + * + * @param exchange the HTTP exchange containing request and response + * @throws IOException if an I/O error occurs during request reading + */ @Override public void handle(HttpExchange exchange) throws IOException { logger.info("Request for field map received"); @@ -77,21 +154,44 @@ public void handle(HttpExchange exchange) throws IOException { } } + /** Handler for the /results GET endpoint. */ private final FakeGet resultsResponse = new FakeGet(); + + /** Handler for the /upload-fieldmap POST endpoint. */ private final FakeFieldMap fieldMapResponse = new FakeFieldMap(); + /** + * Resets the fake Limelight state by clearing the results response body. Useful for ensuring a + * clean state between test cases. + */ public void reset() { resultsResponse.setBody(""); } + /** + * Sets the JSON response that will be returned by the /results endpoint. + * + * @param response the JSONObject containing vision data to return + */ public void setResultsResponse(JSONObject response) { resultsResponse.setBody(response.toString()); } + /** + * Gets the most recently uploaded field map content. + * + * @return the field map data as a string, or null if no field map has been uploaded + */ public String getFieldMap() { return fieldMapResponse.post; } + /** + * Gets the current results response as a JSONObject. + * + * @return a JSONObject parsed from the current response body + * @throws org.json.JSONException if the response body is not valid JSON + */ public JSONObject getResultsResponse() { return new JSONObject(resultsResponse.getBody()); } diff --git a/limelight/src/test/java/com/team2813/lib2813/limelight/LegacyRestLimelightTest.java b/limelight/src/test/java/com/team2813/lib2813/limelight/LegacyRestLimelightTest.java index 7ec943b4..410ed1be 100644 --- a/limelight/src/test/java/com/team2813/lib2813/limelight/LegacyRestLimelightTest.java +++ b/limelight/src/test/java/com/team2813/lib2813/limelight/LegacyRestLimelightTest.java @@ -2,7 +2,32 @@ import org.json.JSONObject; +/** + * Test suite for verifying backward compatibility with legacy Limelight JSON format. + * + *

This class extends {@link RestLimelightTest} to ensure that the RestLimelight implementation + * continues to work with older JSON responses that include a "Results" root object. This legacy + * format was used in earlier versions of the Limelight API, and maintaining compatibility ensures + * that existing systems continue to function correctly. + * + *

The key difference between this test and the parent class is that all JSON test data files in + * the resources directory contain the legacy "Results" root object structure, which this test + * specifically validates. + * + * @see RestLimelightTest + */ public class LegacyRestLimelightTest extends RestLimelightTest { + + /** + * Sets the JSON response for the fake Limelight using the legacy format. + * + *

This override ensures that JSON objects with the legacy "Results" root structure are + * properly handled. All JSON files in the test resources directory use this legacy format, making + * this test class essential for verifying backward compatibility. + * + * @param json the JSONObject containing legacy-formatted Limelight data with a "Results" root + * object + */ @Override protected void setJson(JSONObject json) { // All json files in resources has the "Results" root json object, which is now legacy. diff --git a/limelight/src/test/java/com/team2813/lib2813/limelight/LimelightTestCase.java b/limelight/src/test/java/com/team2813/lib2813/limelight/LimelightTestCase.java index 8e472fc2..0a1ae930 100644 --- a/limelight/src/test/java/com/team2813/lib2813/limelight/LimelightTestCase.java +++ b/limelight/src/test/java/com/team2813/lib2813/limelight/LimelightTestCase.java @@ -26,14 +26,50 @@ import org.json.JSONObject; import org.junit.Test; +/** + * Abstract base class for Limelight implementation testing. + * + *

This class provides a comprehensive test suite for validating Limelight implementations + * against various JSON response scenarios. It tests all aspects of the Limelight interface + * including pose estimation, target detection, AprilTag visibility, and latency measurements. + * + *

Subclasses must implement two abstract methods to adapt the tests to their specific Limelight + * implementation: + * + *

    + *
  • {@link #createLimelight()} - creates the Limelight instance to test + *
  • {@link #setJson(JSONObject)} - configures the test environment with JSON data + *
+ * + *

Test coverage includes: + * + *

    + *
  • Empty/missing data scenarios + *
  • Invalid data handling + *
  • Robot pose estimation in multiple coordinate systems (field, blue alliance, red alliance) + *
  • AprilTag detection and position reporting + *
  • Latency measurements (capture and targeting) + *
  • Field map integration + *
+ */ abstract class LimelightTestCase { + /** + * Tests that an uninitialized Limelight returns empty values. Verifies that capture latency is + * not present when no data has been received. + */ @Test public final void emptyValues() { Limelight limelight = createLimelight(); assertWithMessage("JSON should be empty").that(limelight.getCaptureLatency()).isEmpty(); } + /** + * Tests handling of invalid/malformed JSON data. Verifies that the Limelight correctly identifies + * invalid data and returns empty optionals. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void invalidDataTest() throws Exception { JSONObject obj = readJSON("InvalidDataTest.json"); @@ -50,6 +86,12 @@ public final void invalidDataTest() throws Exception { assertThat(actualTargetingLatency).isEmpty(); } + /** + * Tests scenario where no target is detected but latency data is present. Validates that valid + * latency measurements can be obtained even without a target. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void absentTest1() throws Exception { JSONObject obj = readJSON("AbsentTest1.json"); @@ -70,6 +112,12 @@ public final void absentTest1() throws Exception { assertThat(locationalData.getBotPoseEstimateRed()).isEmpty(); } + /** + * Tests another scenario where no target is detected but latency data differs. Verifies + * consistent behavior with different latency values. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void absentTest2() throws Exception { JSONObject obj = readJSON("AbsentTest2.json"); @@ -90,6 +138,13 @@ public final void absentTest2() throws Exception { assertThat(locationalData.getBotPoseEstimateRed()).isEmpty(); } + /** + * Tests scenario with a detected target and full pose estimation data. Validates robot pose in + * field coordinates, blue alliance coordinates, and red alliance coordinates. Also verifies that + * pose estimates include valid timestamps. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void presentTest1() throws Exception { JSONObject obj = readJSON("PresentTest1.json"); @@ -133,6 +188,12 @@ public final void presentTest1() throws Exception { assertThat(redEstimate.pose()).isWithin(0.005).of(expectedPoseEstimate); } + /** + * Tests another scenario with a detected target and different pose values. Ensures consistent + * pose estimation behavior across different robot positions. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void presentTest2() throws Exception { JSONObject obj = readJSON("PresentTest2.json"); @@ -171,6 +232,12 @@ public final void presentTest2() throws Exception { assertThat(blueEstimate.timestampSeconds()).isWithin(0.005).of(redEstimate.timestampSeconds()); } + /** + * Tests retrieval of robot pose in blue alliance coordinates. Validates the coordinate + * transformation from field coordinates to blue alliance origin. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void getBotposeBlue() throws Exception { JSONObject obj = readJSON("BotposeBlueRedTest.json"); @@ -187,6 +254,12 @@ public final void getBotposeBlue() throws Exception { assertThat(actualPose).isWithin(0.005).of(expectedPose); } + /** + * Tests retrieval of robot pose in red alliance coordinates. Validates the coordinate + * transformation from field coordinates to red alliance origin. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void getBotposeRed() throws Exception { JSONObject obj = readJSON("BotposeBlueRedTest.json"); @@ -204,6 +277,12 @@ public final void getBotposeRed() throws Exception { assertThat(actualPose).isWithin(0.005).of(expectedPose); } + /** + * Tests detection and reporting of visible AprilTag IDs. Verifies that the Limelight correctly + * identifies which tags are in view. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void getVisibleTags() throws Exception { JSONObject obj = readJSON("BotposeBlueRedTest.json"); @@ -213,6 +292,12 @@ public final void getVisibleTags() throws Exception { assertThat(limelight.getLocationalData().getVisibleTags()).containsExactly(20); } + /** + * Tests retrieval of visible AprilTag positions on the field. Validates that tag positions are + * correctly loaded from the field map and that pose estimates include the set of visible tags. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void getVisibleAprilTagPoses() throws Exception { JSONObject obj = readJSON("BotposeBlueRedTest.json"); @@ -239,6 +324,12 @@ public final void getVisibleAprilTagPoses() throws Exception { assertThat(locationalData.getBotPoseEstimateRed().get().visibleAprilTags()).isEqualTo(tags); } + /** + * Tests retrieval of AprilTag locations via the getLocatedAprilTags method. Validates that tag + * positions can be queried by their IDs after field map upload. + * + * @throws Exception if test resources cannot be loaded + */ @Test public final void visibleTagLocation() throws Exception { JSONObject obj = readJSON("BotposeBlueRedTest.json"); @@ -253,10 +344,29 @@ public final void visibleTagLocation() throws Exception { assertThat(pose.getTranslation()).isWithin(0.005).of(new Translation3d(-3.87, 0.72, 0.31)); } + /** + * Creates a Limelight instance for testing. Subclasses must implement this to provide their + * specific Limelight implementation. + * + * @return a Limelight instance to test + */ protected abstract Limelight createLimelight(); + /** + * Configures the test environment with JSON data. Subclasses must implement this to inject test + * data into their Limelight implementation. + * + * @param json the JSONObject containing Limelight response data + */ protected abstract void setJson(JSONObject json); + /** + * Uploads the FRC 2025 Round 2 field map to the Limelight. This configures AprilTag positions for + * testing. + * + * @param limelight the Limelight instance to configure + * @throws IOException if the field map resource cannot be loaded + */ private void uploadFieldMap(Limelight limelight) throws IOException { boolean updateLimelight = false; try (var stream = getClass().getResourceAsStream("frc2025r2.fmap")) { @@ -264,6 +374,13 @@ private void uploadFieldMap(Limelight limelight) throws IOException { } } + /** + * Reads a JSON test file from the classpath resources. + * + * @param fileName the name of the JSON file to read + * @return a JSONObject parsed from the file contents + * @throws IOException if the file cannot be found or read + */ private JSONObject readJSON(String fileName) throws IOException { try (InputStream is = getClass().getResourceAsStream(fileName)) { if (is == null) { @@ -276,6 +393,12 @@ private JSONObject readJSON(String fileName) throws IOException { } } + /** + * Asserts that the Limelight has a valid target detected. Checks both the top-level hasTarget() + * method and the LocationalData hasTarget() method. + * + * @param limelight the Limelight to check + */ private void assertHasTarget(Limelight limelight) { assertWithMessage("Should have target").that(limelight.hasTarget()).isTrue(); assertWithMessage("Should have target") @@ -283,6 +406,14 @@ private void assertHasTarget(Limelight limelight) { .isTrue(); } + /** + * Asserts that an OptionalDouble value is approximately equal to an expected value. If the + * optional is empty, the assertion fails. + * + * @param expected the expected double value + * @param actual the OptionalDouble to check + * @param delta the maximum acceptable difference between expected and actual + */ protected static void assertAlmostEqual(double expected, OptionalDouble actual, double delta) { actual.ifPresentOrElse( d -> assertEquals(expected, d, delta), diff --git a/limelight/src/test/java/com/team2813/lib2813/limelight/NetworkTablesLimelightTest.java b/limelight/src/test/java/com/team2813/lib2813/limelight/NetworkTablesLimelightTest.java index a975c261..c97c89a9 100644 --- a/limelight/src/test/java/com/team2813/lib2813/limelight/NetworkTablesLimelightTest.java +++ b/limelight/src/test/java/com/team2813/lib2813/limelight/NetworkTablesLimelightTest.java @@ -7,11 +7,30 @@ import org.json.JSONObject; import org.junit.After; +/** + * Unit test class for {@link NetworkTablesLimelight}. + * + *

This test framework simulates Limelight JSON data being written into NetworkTables and + * validates how the {@link Limelight} implementation handles pose estimation entries. + */ public class NetworkTablesLimelightTest extends LimelightTestCase { + + /** Name of the Limelight NetworkTable. */ private static final String TABLE_NAME = "limelight"; + + /** Half a millisecond in microseconds. */ private static final long ONE_HALF_MS_IN_MICROS = 500; + + /** One second in microseconds. */ private static final long ONE_SECOND_IN_MICROS = 1_000_000; + + /** Simulated timestamp in microseconds, advanced between test steps. */ private static long fakeTimestampMicros = 15 * ONE_SECOND_IN_MICROS; + + /** + * List of NetworkTables entries used for storing bot pose estimates. These cover both red/blue + * alliances and orb-based estimates. + */ private static final List BOT_POSE_ESTIMATE_ENTRIES = List.of( "botpose", @@ -20,6 +39,12 @@ public class NetworkTablesLimelightTest extends LimelightTestCase { "botpose_wpiblue", "botpose_orb_wpiblue"); + /** + * Resets Limelight NetworkTable entries after each test. + * + *

Clears the JSON entry and all bot pose estimate entries, advancing the fake timestamp by two + * half-millisecond steps. + */ @After public void resetNetworkTables() { getJsonNTEntry().setString(""); @@ -31,11 +56,24 @@ public void resetNetworkTables() { fakeTimestampMicros += ONE_HALF_MS_IN_MICROS; } + /** + * Creates a new {@link NetworkTablesLimelight} instance for testing. + * + * @return a Limelight instance connected to the {@value #TABLE_NAME} table + */ @Override protected Limelight createLimelight() { return new NetworkTablesLimelight(TABLE_NAME); } + /** + * Sets the Limelight JSON data in NetworkTables. + * + *

Copies JSON pose estimation arrays into their corresponding NetworkTable entries when + * latency values are available. + * + * @param json the simulated Limelight JSON response + */ @Override protected void setJson(JSONObject json) { JSONObject resultsJson = json.getJSONObject("Results"); @@ -50,10 +88,22 @@ protected void setJson(JSONObject json) { } } + /** + * Returns the JSON {@link NetworkTableEntry} for Limelight. + * + * @return the NetworkTables entry holding Limelight JSON + */ private static NetworkTableEntry getJsonNTEntry() { return LimelightHelpers.getLimelightNTTableEntry(TABLE_NAME, "json"); } + /** + * Writes a bot pose estimate array into the specified NetworkTables entry. + * + * @param resultsJson the Limelight "Results" JSON object + * @param entryName the entry name to populate + * @param latencyMillis the latency value in milliseconds + */ private static void setBotPoseEstimate( JSONObject resultsJson, String entryName, double latencyMillis) { double[] estimate_array = getBotPoseEstimateArray(resultsJson, entryName, latencyMillis); @@ -62,12 +112,37 @@ private static void setBotPoseEstimate( tableEntry.set(estimate_array, fakeTimestampMicros); } + /** + * Clears a bot pose estimate entry by setting it to an empty array. + * + * @param entryName the NetworkTables entry to clear + * @param timestampMicros the timestamp in microseconds + */ private static void clearBotPoseEstimate(String entryName, long timestampMicros) { DoubleArrayEntry tableEntry = LimelightHelpers.getLimelightDoubleArrayEntry(TABLE_NAME, entryName); tableEntry.set(new double[0], timestampMicros); } + /** + * Builds a bot pose estimate array from Limelight JSON data. + * + *

The array is structured as: + * + *

    + *
  • Indices 0–5: pose data from Limelight + *
  • Index 6: latency in milliseconds + *
  • Index 7: tag count + *
  • Index 8: tag ID + *
+ * + * Additional indices are reserved for multiple tags. + * + * @param resultsJson the Limelight "Results" JSON object + * @param entryName the entry to extract + * @param latencyMillis the latency in milliseconds + * @return a double array representing the bot pose estimate, or an empty array if not present + */ private static double[] getBotPoseEstimateArray( JSONObject resultsJson, String entryName, double latencyMillis) { if (!resultsJson.has(entryName)) { diff --git a/limelight/src/test/java/com/team2813/lib2813/limelight/RestLimelightTest.java b/limelight/src/test/java/com/team2813/lib2813/limelight/RestLimelightTest.java index b1e5e1d7..2536f888 100644 --- a/limelight/src/test/java/com/team2813/lib2813/limelight/RestLimelightTest.java +++ b/limelight/src/test/java/com/team2813/lib2813/limelight/RestLimelightTest.java @@ -16,28 +16,56 @@ import org.junit.ClassRule; import org.junit.Test; +/** + * Unit tests for {@link RestLimelight}. + * + *

This test suite uses a {@link FakeLimelight} HTTP server to simulate REST responses from a + * physical Limelight device. It validates Limelight instance management, HTTP endpoint behavior, + * and field map uploads. + */ public class RestLimelightTest extends LimelightTestCase { + + /** + * Fake Limelight instance running as an embedded HTTP server. + * + *

Used to simulate responses from a real Limelight device without requiring hardware. + */ @ClassRule public static final FakeLimelight fakeLimelight = new FakeLimelight(); + /** Resets any cached {@link RestLimelight} instances after each test. */ @After public void resetLimelights() { RestLimelight.eraseInstances(); } + /** + * Resets the {@link FakeLimelight} after each test, ensuring no test state leaks between test + * methods. + */ @After public void resetFakeLimelight() { fakeLimelight.reset(); } + /** + * Verifies that calls to obtain the default {@link RestLimelight} return the same instance, and + * that the default instance is equal to one retrieved by name. + */ @Test public void equality() { Limelight a = RestLimelight.getDefaultLimelight(); Limelight b = RestLimelight.getDefaultLimelight(); assertEquals("Default limelight call returned different values", a, b); + Limelight c = RestLimelight.getLimelight(RestLimelight.DEFAULT_ADDRESS); assertEquals("Default limelights not equal to limelights named \"limelight\" (default)", a, c); } + /** + * Ensures the {@link FakeLimelight} responds correctly to REST requests. + * + * @throws Exception if the HTTP client request fails + */ @Test public void fakeWorks() throws Exception { fakeLimelight.setResultsResponse(new JSONObject().put("v", 1)); @@ -49,16 +77,25 @@ public void fakeWorks() throws Exception { assertEquals("{\"v\":1}", response.body()); } + /** + * Verifies that uploading a field map to a {@link RestLimelight} updates the {@link + * FakeLimelight} server as expected. + * + * @throws Exception if reading the field map resource or uploading fails + */ @Test public void setFieldMapWorks() throws Exception { Limelight limelight = createLimelight(); String resourceName = "frc2025r2.fmap"; + + // Upload field map from resources try (InputStream is = getClass().getResourceAsStream(resourceName)) { limelight.setFieldMap(is, true); } String fieldMap = fakeLimelight.getFieldMap(); assertNotNull(fieldMap); + // Verify expected field map content matches String expectedFieldMap; try (InputStream is = getClass().getResourceAsStream(resourceName)) { expectedFieldMap = new String(is.readAllBytes(), StandardCharsets.UTF_8); @@ -66,6 +103,11 @@ public void setFieldMapWorks() throws Exception { assertEquals(expectedFieldMap, fieldMap); } + /** + * Creates a new {@link RestLimelight} instance for testing. + * + * @return a {@code RestLimelight} bound to "localhost" + */ @Override protected Limelight createLimelight() { RestLimelight limelight = new RestLimelight("localhost"); @@ -73,14 +115,17 @@ protected Limelight createLimelight() { return limelight; } + /** + * Sets the Limelight JSON results response in the {@link FakeLimelight}. + * + *

Unlike older schemas, the new Limelight JSON schema does not include a {@code "Results"} + * object at the root. Instead, all fields are inlined. Since resource test files still contain + * {@code "Results"}, this method extracts and forwards that sub-object. + * + * @param json the full Limelight JSON, expected to contain a {@code "Results"} object + */ @Override protected void setJson(JSONObject json) { - // limelight json schema has been updated to not json object "Results" in the root, and we want - // to test the new version, which does not. - // The new version just has all json that was in "Results" in the root, so the json object in - // "Results" will essentially be the new schema. - // Since we know that all the json objects in the resources folder have the "Results" json - // object, this should never fail. fakeLimelight.setResultsResponse(json.getJSONObject("Results")); } } diff --git a/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/CommandTester.java b/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/CommandTester.java index 42bc7741..0edfee09 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/CommandTester.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/CommandTester.java @@ -3,12 +3,18 @@ import edu.wpi.first.wpilibj2.command.Command; /** - * Allows tests to run commands. + * Utility interface for running WPILib {@link Command} objects inside tests. * - *

Tests can get an instance by using {@link WPILibExtension}. + *

Provides a way to schedule and execute commands to completion within a test context. An + * implementation is provided via {@link WPILibExtension}. */ public interface CommandTester { - /** Schedules the provided command and runs it until it completes. */ + /** + * Schedules the provided {@link Command} and repeatedly runs it until the command reports it has + * finished. + * + * @param command the command to schedule and execute + */ void runUntilComplete(Command command); } diff --git a/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtension.java b/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtension.java index 2fbf0474..6ee19b60 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtension.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtension.java @@ -13,11 +13,18 @@ import org.junit.jupiter.api.extension.ParameterResolver; /** - * JUnit Jupiter extension for testing code that depends on WPILib. + * JUnit Jupiter extension for testing robot code that depends on WPILib. * - *

Also provides a {@link CommandTester} for tests. + *

This extension: * - *

Example use: + *

    + *
  • Initializes the WPILib {@link HAL} and driver station simulation before tests run. + *
  • Configures the {@link CommandScheduler} to ensure a clean environment. + *
  • Provides a {@link CommandTester} parameter for tests, which allows running WPILib commands + * until completion. + *
+ * + *

Example usage: * *

{@code
  * @ExtendWith(WPILibExtension.class)
@@ -26,7 +33,6 @@
  *   @Test
  *   public void initiallyNotInAir() {
  *     var flight = new FlightSubsystem();
- *
  *     assertThat(flight.inAir()).isFalse();
  *   }
  *
@@ -35,6 +41,7 @@
  *     var flight = new FlightSubsystem();
  *     Command takeOff = flight.createTakeOffCommandCommand();
  *
+ *     // Run the command under test
  *     commandTester.runUntilComplete(takeOff);
  *
  *     assertThat(flight.inAir()).isTrue();
@@ -49,6 +56,15 @@ public final class WPILibExtension
         BeforeAllCallback,
         ParameterResolver {
 
+  /**
+   * Initializes WPILib components before all tests.
+   *
+   * 

Sets up the HAL, enables the driver station simulation, and resets the {@link + * CommandScheduler}. + * + * @param context the JUnit extension context + * @throws IllegalStateException if the HAL cannot be initialized + */ @Override public void beforeAll(ExtensionContext context) { // See https://www.chiefdelphi.com/t/driverstation-getalliance-in-gradle-test/ @@ -62,12 +78,23 @@ public void beforeAll(ExtensionContext context) { CommandScheduler.getInstance().unregisterAllSubsystems(); } + /** + * Cleans up after each test by canceling all commands and unregistering all subsystems. + * + * @param context the JUnit extension context + */ @Override public void afterEach(ExtensionContext context) { CommandScheduler.getInstance().cancelAll(); CommandScheduler.getInstance().unregisterAllSubsystems(); } + /** + * Cleans up after all tests by canceling all commands, unregistering all subsystems, disabling + * the scheduler, and resetting the driver station simulation. + * + * @param context the JUnit extension context + */ @Override public void afterAll(ExtensionContext context) { CommandScheduler.getInstance().cancelAll(); @@ -77,6 +104,14 @@ public void afterAll(ExtensionContext context) { DriverStationSim.notifyNewData(); } + /** + * Checks whether this extension can provide a parameter of type {@link CommandTester}. + * + * @param parameterContext the parameter context + * @param extensionContext the extension context + * @return true if the parameter type is {@code CommandTester} + * @throws ParameterResolutionException if parameter resolution fails + */ @Override public boolean supportsParameter( ParameterContext parameterContext, ExtensionContext extensionContext) @@ -84,6 +119,16 @@ public boolean supportsParameter( return CommandTester.class.equals(parameterContext.getParameter().getType()); } + /** + * Provides a {@link CommandTester} instance for parameter injection. + * + *

The tester schedules the given command and repeatedly runs the {@link CommandScheduler} + * until the command completes. + * + * @param parameterContext the parameter context + * @param extensionContext the extension context + * @return a {@link CommandTester} that executes commands to completion + */ @Override public CommandTester resolveParameter( ParameterContext parameterContext, ExtensionContext extensionContext) { diff --git a/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose2dSubject.java b/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose2dSubject.java index f3b33296..9f25e32d 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose2dSubject.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose2dSubject.java @@ -9,19 +9,42 @@ import javax.annotation.Nullable; /** - * Truth Subject for making assertions about {@link Pose2d} values. + * A Truth {@link Subject} for making assertions about {@link Pose2d} values. * - *

See Writing your own custom subject to learn about - * creating custom Truth subjects. + *

This subject provides fluent assertions for comparing poses, including tolerance-based + * comparisons of translations and rotations. + * + *

See Truth: Writing your own custom subject for more + * on extending Truth. */ public final class Pose2dSubject extends Subject { - // User-defined entry point + /** + * Entry point for {@link Pose2d} assertions. + * + *

Usage: + * + *

{@code
+   * Pose2d actualPose = ...;
+   * Pose2d expectedPose = ...;
+   *
+   * Pose2dSubject.assertThat(actualPose)
+   *     .isWithin(0.01)
+   *     .of(expectedPose);
+   * }
+ * + * @param pose the pose under test (may be {@code null}) + * @return a {@link Pose2dSubject} for making assertions + */ public static Pose2dSubject assertThat(@Nullable Pose2d pose) { return assertAbout(pose2ds()).that(pose); } - // Static method for getting the subject factory (for use with assertAbout()) + /** + * Factory for {@link Pose2dSubject}, for use with assertAbout(). + * + * @return a factory for creating {@link Pose2dSubject} instances + */ public static Subject.Factory pose2ds() { return Pose2dSubject::new; } @@ -33,8 +56,14 @@ private Pose2dSubject(FailureMetadata failureMetadata, @Nullable Pose2d subject) this.actual = subject; } - // User-defined test assertion SPI below this point - + /** + * Returns a tolerant comparison assertion for the current pose. + * + *

The tolerance applies to both translation (x, y) and rotation (θ). + * + * @param tolerance the maximum allowed difference in meters (translation) or radians (rotation) + * @return a {@link TolerantComparison} for comparing poses with a tolerance + */ public TolerantComparison isWithin(double tolerance) { return new TolerantComparison() { @Override @@ -45,20 +74,36 @@ public void of(Pose2d expected) { }; } + /** + * Returns a {@link Translation2dSubject} for making assertions about the translation component of + * this pose. + * + * @return a subject for the pose's translation + */ public Translation2dSubject translation() { return check("getTranslation()") .about(Translation2dSubject.translation2ds()) .that(nonNullActualPose().getTranslation()); } + /** + * Returns a {@link Rotation2dSubject} for making assertions about the rotation component of this + * pose. + * + * @return a subject for the pose's rotation + */ public Rotation2dSubject rotation() { return check("getRotation()") .about(Rotation2dSubject.rotation2ds()) .that(nonNullActualPose().getRotation()); } - // Helper methods below this point - + /** + * Ensures that the actual pose is not {@code null}. + * + * @return the non-null actual {@link Pose2d} + * @throws AssertionError if the pose under test is {@code null} + */ private Pose2d nonNullActualPose() { if (actual == null) { failWithActual(simpleFact("expected a non-null Pose2d")); diff --git a/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose3dSubject.java b/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose3dSubject.java index 46990843..a21ae5a8 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose3dSubject.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/truth/Pose3dSubject.java @@ -9,19 +9,42 @@ import javax.annotation.Nullable; /** - * Truth Subject for making assertions about {@link Pose3d} values. + * A Truth {@link Subject} for making assertions about {@link Pose3d} values. * - *

See Writing your own custom subject to learn about - * creating custom Truth subjects. + *

This subject provides fluent assertions for comparing 3D poses, including tolerance-based + * comparisons of translations and rotations. + * + *

See Truth: Writing your own custom subject for more + * on extending Truth. */ public final class Pose3dSubject extends Subject { - // User-defined entry point + /** + * Entry point for {@link Pose3d} assertions. + * + *

Usage: + * + *

{@code
+   * Pose3d actualPose = ...;
+   * Pose3d expectedPose = ...;
+   *
+   * Pose3dSubject.assertThat(actualPose)
+   *     .isWithin(0.01)
+   *     .of(expectedPose);
+   * }
+ * + * @param pose the pose under test (may be {@code null}) + * @return a {@link Pose3dSubject} for making assertions + */ public static Pose3dSubject assertThat(@Nullable Pose3d pose) { return assertAbout(pose3ds()).that(pose); } - // Static method for getting the subject factory (for use with assertAbout()) + /** + * Factory for {@link Pose2dSubject}, for use with assertAbout(). + * + * @return a factory for creating {@link Pose3dSubject} instances + */ public static Subject.Factory pose3ds() { return Pose3dSubject::new; } @@ -33,8 +56,14 @@ private Pose3dSubject(FailureMetadata failureMetadata, @Nullable Pose3d subject) this.actual = subject; } - // User-defined test assertion SPI below this point - + /** + * Returns a tolerant comparison assertion for the current 3D pose. + * + *

The tolerance applies to both translation (x, y, z) and rotation (roll, pitch, yaw). + * + * @param tolerance the maximum allowed difference in meters (translation) or radians (rotation) + * @return a {@link TolerantComparison} for comparing poses with a tolerance + */ public TolerantComparison isWithin(double tolerance) { return new TolerantComparison() { @Override @@ -45,22 +74,36 @@ public void of(Pose3d expected) { }; } - // Chained subjects methods below this point - + /** + * Returns a {@link Translation3dSubject} for making assertions about the translation component of + * this pose. + * + * @return a subject for the pose's 3D translation + */ public Translation3dSubject translation() { return check("getTranslation()") .about(Translation3dSubject.translation3ds()) .that(nonNullActualPose().getTranslation()); } + /** + * Returns a {@link Rotation3dSubject} for making assertions about the rotation component of this + * pose. + * + * @return a subject for the pose's 3D rotation + */ public Rotation3dSubject rotation() { return check("getRotation()") .about(Rotation3dSubject.rotation3ds()) .that(nonNullActualPose().getRotation()); } - // Helper methods below this point - + /** + * Ensures that the actual pose is not {@code null}. + * + * @return the non-null actual {@link Pose3d} + * @throws AssertionError if the pose under test is {@code null} + */ private Pose3d nonNullActualPose() { if (actual == null) { failWithActual(simpleFact("expected a non-null Pose3d")); diff --git a/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation2dSubject.java b/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation2dSubject.java index d50e4f0a..0bd531c0 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation2dSubject.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation2dSubject.java @@ -9,14 +9,42 @@ import edu.wpi.first.math.geometry.Rotation2d; import javax.annotation.Nullable; -/** Truth Subject for making assertions about {@link Rotation2d} values. */ +/** + * A Truth {@link Subject} for making assertions about {@link Rotation2d} values. + * + *

This subject provides fluent assertions for comparing 2D rotations, including tolerance-based + * comparisons and checks for zero rotation. + * + *

Rotations are compared in radians. + */ public final class Rotation2dSubject extends Subject { - // User-defined entry point + + /** + * Entry point for {@link Rotation2d} assertions. + * + *

Usage: + * + *

{@code
+   * Rotation2d actual = new Rotation2d(Math.PI / 2);
+   * Rotation2d expected = new Rotation2d(Math.PI / 2 + 1e-3);
+   *
+   * Rotation2dSubject.assertThat(actual)
+   *     .isWithin(1e-2)
+   *     .of(expected);
+   * }
+ * + * @param rotation the rotation under test (may be {@code null}) + * @return a {@link Rotation2dSubject} for making assertions + */ public static Rotation2dSubject assertThat(@Nullable Rotation2d rotation) { return assertAbout(rotation2ds()).that(rotation); } - // Static method for getting the subject factory (for use with assertAbout()) + /** + * Factory for {@link Pose2dSubject}, for use with assertAbout(). + * + * @return a factory for creating {@link Rotation2dSubject} instances + */ public static Factory rotation2ds() { return Rotation2dSubject::new; } @@ -28,8 +56,14 @@ private Rotation2dSubject(FailureMetadata failureMetadata, @Nullable Rotation2d this.actual = subject; } - // User-defined test assertion SPI below this point - + /** + * Returns a tolerant comparison assertion for this rotation. + * + *

The tolerance is expressed in radians. + * + * @param tolerance the maximum allowed difference in radians + * @return a {@link TolerantComparison} for comparing rotations with a tolerance + */ public TolerantComparison isWithin(double tolerance) { return new TolerantComparison() { @Override @@ -39,20 +73,32 @@ public void of(Rotation2d expected) { }; } + /** + * Asserts that this rotation is exactly equal to {@link Rotation2d#kZero}. + * + *

Fails if the rotation under test is {@code null} or not equal to zero. + */ public void isZero() { if (!Rotation2d.kZero.equals(actual)) { failWithActual(simpleFact("expected to be zero")); } } - // Chained subjects methods below this point - + /** + * Returns a {@link DoubleSubject} for making assertions about the rotation’s raw radians value. + * + * @return a {@link DoubleSubject} for the rotation’s radians + */ public DoubleSubject getRadians() { return check("getRadians()").that(nonNullActual().getRadians()); } - // Helper methods below this point - + /** + * Ensures that the actual rotation is not {@code null}. + * + * @return the non-null actual {@link Rotation2d} + * @throws AssertionError if the rotation under test is {@code null} + */ private Rotation2d nonNullActual() { if (actual == null) { failWithActual(simpleFact("expected a non-null Rotation2d")); diff --git a/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation3dSubject.java b/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation3dSubject.java index fe78d230..2a449e84 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation3dSubject.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/truth/Rotation3dSubject.java @@ -9,14 +9,48 @@ import edu.wpi.first.math.geometry.Rotation3d; import javax.annotation.Nullable; -/** Truth Subject for making assertions about {@link Rotation3d} values. */ +/** + * A Truth {@link Subject} for making assertions about {@link Rotation3d} values. + * + *

This subject provides fluent assertions for comparing 3D rotations, including tolerance-based + * comparisons of roll, pitch, and yaw angles, as well as checks for zero rotation. + * + *

Rotations are expressed in radians: + * + *

    + *
  • {@link #x()} → roll (counterclockwise about X axis) + *
  • {@link #y()} → pitch (counterclockwise about Y axis) + *
  • {@link #z()} → yaw (counterclockwise about Z axis) + *
+ */ public final class Rotation3dSubject extends Subject { - // User-defined entry point + + /** + * Entry point for {@link Rotation3d} assertions. + * + *

Usage: + * + *

{@code
+   * Rotation3d actual = new Rotation3d(Math.PI / 2, 0, 0);
+   * Rotation3d expected = new Rotation3d(Math.PI / 2 + 1e-3, 0, 0);
+   *
+   * Rotation3dSubject.assertThat(actual)
+   *     .isWithin(1e-2)
+   *     .of(expected);
+   * }
+ * + * @param rotation the rotation under test (may be {@code null}) + * @return a {@link Rotation3dSubject} for making assertions + */ public static Rotation3dSubject assertThat(@Nullable Rotation3d rotation) { return assertAbout(rotation3ds()).that(rotation); } - // Static method for getting the subject factory (for use with assertAbout()) + /** + * Factory for {@link Pose2dSubject}, for use with assertAbout(). + * + * @return a factory for creating {@link Rotation3dSubject} instances + */ public static Factory rotation3ds() { return Rotation3dSubject::new; } @@ -28,53 +62,70 @@ private Rotation3dSubject(FailureMetadata failureMetadata, @Nullable Rotation3d this.actual = subject; } - // User-defined test assertion SPI below this point - + /** + * Returns a tolerant comparison assertion for this rotation. + * + *

The tolerance is expressed in radians and applies to each of the roll (X), pitch (Y), and + * yaw (Z) components independently. + * + * @param tolerance the maximum allowed difference in radians + * @return a {@link TolerantComparison} for comparing rotations with a tolerance + */ public TolerantComparison isWithin(double tolerance) { return new TolerantComparison() { @Override public void of(Rotation3d expected) { - x().isWithin(tolerance).of(expected.getX()); // roll, in radians - y().isWithin(tolerance).of(expected.getY()); // pitch, in radians - z().isWithin(tolerance).of(expected.getZ()); // yaw, in radians + x().isWithin(tolerance).of(expected.getX()); // roll + y().isWithin(tolerance).of(expected.getY()); // pitch + z().isWithin(tolerance).of(expected.getZ()); // yaw } }; } + /** + * Asserts that this rotation is exactly equal to {@link Rotation3d#kZero}. + * + *

Fails if the rotation under test is {@code null} or not equal to zero. + */ public void isZero() { if (!Rotation3d.kZero.equals(actual)) { failWithActual(simpleFact("expected to be zero")); } } - // Chained subjects methods below this point - /** - * Returns a subject that can be used to make assertions about the counterclockwise rotation angle - * around the X axis (roll) in radians. + * Returns a subject for making assertions about the roll (X-axis rotation, radians). + * + * @return a {@link DoubleSubject} for the roll component in radians */ public DoubleSubject x() { return check("getX()").that(nonNullActual().getX()); } /** - * Returns a subject that can be used to make assertions about the counterclockwise rotation angle - * around the Y axis (pitch) in radians. + * Returns a subject for making assertions about the pitch (Y-axis rotation, radians). + * + * @return a {@link DoubleSubject} for the pitch component in radians */ public DoubleSubject y() { return check("getY()").that(nonNullActual().getY()); } /** - * Returns a subject that can be used to make assertions about the counterclockwise rotation angle - * around the Z axis (yaw) in radians. + * Returns a subject for making assertions about the yaw (Z-axis rotation, radians). + * + * @return a {@link DoubleSubject} for the yaw component in radians */ public DoubleSubject z() { return check("getZ()").that(nonNullActual().getZ()); } - // Helper methods below this point - + /** + * Ensures that the actual rotation is not {@code null}. + * + * @return the non-null actual {@link Rotation3d} + * @throws AssertionError if the rotation under test is {@code null} + */ private Rotation3d nonNullActual() { if (actual == null) { failWithActual(simpleFact("expected a non-null Rotation3d")); diff --git a/testing/src/main/java/com/team2813/lib2813/testing/truth/TolerantComparison.java b/testing/src/main/java/com/team2813/lib2813/testing/truth/TolerantComparison.java index cc2c2b38..a85e4810 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/truth/TolerantComparison.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/truth/TolerantComparison.java @@ -3,26 +3,44 @@ import org.jspecify.annotations.Nullable; /** - * A partially specified check about an approximate relationship to a {@code double} subject using a - * tolerance. + * Represents a partially specified check on a subject of type {@code T} for approximate equality + * within a given tolerance. This class is intended to be used as part of a fluent assertion API, + * where the subject and tolerance are defined earlier in the chain, and the expected value is + * provided via {@link #of(Object)}. + * + *

For example, in a fluent assertion style: + * + *

{@code
+ * assertThat(actualValue).isWithin(tolerance).of(expectedValue);
+ * }
+ * + *

Subclasses of this class implement the specific logic for comparing the subject with the + * expected value considering the tolerance. + * + * @param the type of the value being compared (typically {@link Double}) */ public abstract class TolerantComparison { - // Prevent subclassing outside of this package + /** Package-private constructor to prevent subclassing outside of this package. */ TolerantComparison() {} /** - * Fails if the subject was expected to be within the tolerance of the given value but was not. - * The subject and tolerance are specified earlier in the fluent call chain. + * Fails the assertion if the subject is not within the tolerance of the given expected value. The + * subject and tolerance must have been specified earlier in the fluent call chain. + * + * @param expected the value the subject is expected to be approximately equal to */ public abstract void of(T expected); - // Remaining code copied from DoubleSubject.TolerantDoubleComparison - /** + * {@inheritDoc} + * + *

This method is unsupported for {@code TolerantComparison}. Equality comparisons should be + * performed via {@link #of(Object)} rather than {@link Object#equals(Object)}. + * * @throws UnsupportedOperationException always - * @deprecated {@link Object#equals(Object)} is not supported on TolerantComparison. If you meant - * to compare doubles, use {@link #of(T)} instead. + * @deprecated Use {@link #of(Object)} to compare values. {@link Object#equals(Object)} is not + * supported. */ @Deprecated @Override @@ -32,8 +50,12 @@ public final boolean equals(@Nullable Object o) { } /** + * {@inheritDoc} + * + *

This method is unsupported for {@code TolerantComparison}. + * * @throws UnsupportedOperationException always - * @deprecated {@link Object#hashCode()} is not supported on TolerantComparison + * @deprecated {@link Object#hashCode()} is not supported on {@code TolerantComparison}. */ @Deprecated @Override diff --git a/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation2dSubject.java b/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation2dSubject.java index 41a51edd..f332350b 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation2dSubject.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation2dSubject.java @@ -9,28 +9,64 @@ import edu.wpi.first.math.geometry.Translation2d; import javax.annotation.Nullable; -/** Truth Subject for making assertions about {@link Translation2d} values. */ +/** + * A Truth {@link Subject} for making fluent assertions about {@link Translation2d} values. + * + *

This class allows tests to verify the x and y components of a {@code Translation2d}, perform + * approximate comparisons within a tolerance, and check special properties like being zero. + * + *

Example usage: + * + *

{@code
+ * Translation2d translation = new Translation2d(1.0, 2.0);
+ * Translation2dSubject.assertThat(translation)
+ *     .isWithin(0.01).of(new Translation2d(1.0, 2.0));
+ * Translation2dSubject.assertThat(translation).x().isEqualTo(1.0);
+ * Translation2dSubject.assertThat(translation).y().isEqualTo(2.0);
+ * }
+ */ public final class Translation2dSubject extends Subject { - // User-defined entry point + /** + * Entry point for assertions about a {@link Translation2d} instance. + * + * @param translation the translation to assert about (nullable) + * @return a {@link Translation2dSubject} for fluent assertions + */ public static Translation2dSubject assertThat(@Nullable Translation2d translation) { return assertAbout(translation2ds()).that(translation); } - // Static method for getting the subject factory (for use with assertAbout()) + /** + * Returns a Truth {@link Factory} for {@link Translation2dSubject}, used with {@link + * com.google.common.truth.Truth#assertAbout(Subject.Factory)}. + * + * @return a factory for creating {@link Translation2dSubject} instances + */ public static Factory translation2ds() { return Translation2dSubject::new; } private final Translation2d actual; + /** + * Constructor used internally by Truth. + * + * @param failureMetadata metadata about the failure for reporting + * @param subject the actual {@link Translation2d} value (nullable) + */ private Translation2dSubject(FailureMetadata failureMetadata, @Nullable Translation2d subject) { super(failureMetadata, subject); this.actual = subject; } - // User-defined test assertion SPI below this point - + /** + * Returns a {@link TolerantComparison} to assert that this translation is within the given {@code + * tolerance} of an expected {@link Translation2d}. + * + * @param tolerance the allowed absolute difference in both x and y components + * @return a {@link TolerantComparison} for fluent approximate comparisons + */ public TolerantComparison isWithin(double tolerance) { return new TolerantComparison() { @Override @@ -41,24 +77,36 @@ public void of(Translation2d expected) { }; } + /** Fails if the {@link Translation2d} is not the zero vector. */ public void isZero() { if (!Translation2d.kZero.equals(actual)) { failWithActual(simpleFact("expected to be zero")); } } - // Chained subjects methods below this point - + /** + * Returns a {@link DoubleSubject} for assertions on the x-component of the translation. + * + * @return a {@link DoubleSubject} for the x value + */ public DoubleSubject x() { return check("getX()").that(nonNullActual().getX()); } + /** + * Returns a {@link DoubleSubject} for assertions on the y-component of the translation. + * + * @return a {@link DoubleSubject} for the y value + */ public DoubleSubject y() { return check("getY()").that(nonNullActual().getY()); } - // Helper methods below this point - + /** + * Ensures the actual translation is non-null, failing the assertion if it is null. + * + * @return the non-null {@link Translation2d} instance + */ private Translation2d nonNullActual() { if (actual == null) { failWithActual(simpleFact("expected a non-null Translation2d")); diff --git a/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation3dSubject.java b/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation3dSubject.java index 302e1227..985bd5c9 100644 --- a/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation3dSubject.java +++ b/testing/src/main/java/com/team2813/lib2813/testing/truth/Translation3dSubject.java @@ -9,28 +9,65 @@ import edu.wpi.first.math.geometry.Translation3d; import javax.annotation.Nullable; -/** Truth Subject for making assertions about {@link Translation3d} values. */ +/** + * A Truth {@link Subject} for making fluent assertions about {@link Translation3d} values. + * + *

This class allows tests to verify the x, y, and z components of a {@code Translation3d}, + * perform approximate comparisons within a tolerance, and check special properties like being zero. + * + *

Example usage: + * + *

{@code
+ * Translation3d translation = new Translation3d(1.0, 2.0, 3.0);
+ * Translation3dSubject.assertThat(translation)
+ *     .isWithin(0.01).of(new Translation3d(1.0, 2.0, 3.0));
+ * Translation3dSubject.assertThat(translation).x().isEqualTo(1.0);
+ * Translation3dSubject.assertThat(translation).y().isEqualTo(2.0);
+ * Translation3dSubject.assertThat(translation).z().isEqualTo(3.0);
+ * }
+ */ public final class Translation3dSubject extends Subject { - // User-defined entry point + /** + * Entry point for assertions about a {@link Translation3d} instance. + * + * @param translation the translation to assert about (nullable) + * @return a {@link Translation3dSubject} for fluent assertions + */ public static Translation3dSubject assertThat(@Nullable Translation3d translation) { return assertAbout(translation3ds()).that(translation); } - // Static method for getting the subject factory (for use with assertAbout()) + /** + * Returns a Truth {@link Factory} for {@link Translation3dSubject}, used with {@link + * com.google.common.truth.Truth#assertAbout(Subject.Factory)}. + * + * @return a factory for creating {@link Translation3dSubject} instances + */ public static Factory translation3ds() { return Translation3dSubject::new; } private final Translation3d actual; + /** + * Constructor used internally by Truth. + * + * @param failureMetadata metadata about the failure for reporting + * @param subject the actual {@link Translation3d} value (nullable) + */ private Translation3dSubject(FailureMetadata failureMetadata, @Nullable Translation3d subject) { super(failureMetadata, subject); this.actual = subject; } - // User-defined test assertion SPI below this point - + /** + * Returns a {@link TolerantComparison} to assert that this translation is within the given {@code + * tolerance} of an expected {@link Translation3d}. + * + * @param tolerance the allowed absolute difference in x, y, and z components + * @return a {@link TolerantComparison} for fluent approximate comparisons + */ public TolerantComparison isWithin(double tolerance) { return new TolerantComparison() { @Override @@ -42,28 +79,45 @@ public void of(Translation3d expected) { }; } + /** Fails if the {@link Translation3d} is not the zero vector. */ public void isZero() { if (!Translation3d.kZero.equals(actual)) { failWithActual(simpleFact("expected to be zero")); } } - // Chained subjects methods below this point - + /** + * Returns a {@link DoubleSubject} for assertions on the x-component of the translation. + * + * @return a {@link DoubleSubject} for the x value + */ public DoubleSubject x() { return check("getX()").that(nonNullActual().getX()); } + /** + * Returns a {@link DoubleSubject} for assertions on the y-component of the translation. + * + * @return a {@link DoubleSubject} for the y value + */ public DoubleSubject y() { return check("getY()").that(nonNullActual().getY()); } + /** + * Returns a {@link DoubleSubject} for assertions on the z-component of the translation. + * + * @return a {@link DoubleSubject} for the z value + */ public DoubleSubject z() { return check("getZ()").that(nonNullActual().getZ()); } - // Helper methods below this point - + /** + * Ensures the actual translation is non-null, failing the assertion if it is null. + * + * @return the non-null {@link Translation3d} instance + */ private Translation3d nonNullActual() { if (actual == null) { failWithActual(simpleFact("expected a non-null Translation3d")); diff --git a/testing/src/test/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtensionTest.java b/testing/src/test/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtensionTest.java index 9c7301ca..84dae72f 100644 --- a/testing/src/test/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtensionTest.java +++ b/testing/src/test/java/com/team2813/lib2813/testing/junit/jupiter/WPILibExtensionTest.java @@ -2,7 +2,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; -import static com.team2813.lib2813.testing.junit.jupiter.ExtensionAssertions.assertHasNoFailures; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; import edu.wpi.first.hal.HAL; @@ -21,10 +20,23 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.platform.testkit.engine.EngineExecutionResults; import org.junit.platform.testkit.engine.EngineTestKit; - -/** Tests for {@link WPILibExtension}. */ +import org.junit.platform.testkit.engine.Events; + +/** + * Unit tests for the {@link WPILibExtension} JUnit 5 extension. + * + *

This class ensures that WPILib-specific functionality (like {@link CommandScheduler} and + * {@link DriverStation}) behaves correctly when used with the extension. It verifies that: + * + *

    + *
  • Commands are not scheduled unexpectedly before or after tests + *
  • {@link DriverStation} is properly enabled during tests + *
  • {@link CommandTester} can execute and verify commands + *
+ */ public class WPILibExtensionTest { + /** A fake command used for scheduling tests. */ static final Command FAKE_COMMAND = new Command() {}; @BeforeAll @@ -34,6 +46,12 @@ static void initializeHal() { } } + /** + * Sample test class demonstrating usage of {@link WPILibExtension} with JUnit 5. + * + *

Uses {@link TestMethodOrder} to enforce ordering and {@link Tag} to mark it as special + * testkit-only. + */ @ExtendWith(WPILibExtension.class) @Tag("ignore-outside-testkit") @TestMethodOrder(MethodOrderer.OrderAnnotation.class) @@ -52,6 +70,7 @@ public static void verifyDriverStationEnabled() { assertThat(DriverStation.isEnabled()).isTrue(); } + /** Verifies that FAKE_COMMAND is not scheduled before the first test, then schedules it. */ @Test @Order(1) public void verifyFakeCommandNotScheduledBeforeTest() { @@ -64,6 +83,7 @@ public void verifyFakeCommandNotScheduledBeforeTest() { assertThat(commandScheduler.isScheduled(FAKE_COMMAND)); } + /** Verifies that FAKE_COMMAND is not scheduled before the second test, then schedules it. */ @Test @Order(2) public void verifyFakeCommandNotScheduledAfterTest(CommandTester commandTester) { @@ -76,6 +96,7 @@ public void verifyFakeCommandNotScheduledAfterTest(CommandTester commandTester) assertThat(commandScheduler.isScheduled(FAKE_COMMAND)); } + /** Verifies that {@link CommandTester} runs and correctly verifies a command. */ @Test @Order(3) public void verifyCommandTester(CommandTester commandTester) { @@ -93,12 +114,11 @@ public static void verifyFakeCommandNotScheduledAfterAllTests() { } } // end SampleTest + /** Verifies that {@link WPILibExtension} executes SampleTest correctly and without failures. */ @Test void verifyExtension() { - // Arrange withDriverStationTemporarilyEnabled( () -> { - // Schedule FAKE_COMMAND CommandScheduler commandScheduler = CommandScheduler.getInstance(); commandScheduler.enable(); commandScheduler.schedule(FAKE_COMMAND); @@ -107,14 +127,16 @@ void verifyExtension() { assertThat(isScheduled).isTrue(); }); - // Act EngineExecutionResults results = EngineTestKit.engine("junit-jupiter").selectors(selectClass(SampleTest.class)).execute(); - // Assert assertHasNoFailures(results); } + /** + * Temporarily enables the driver station for the duration of {@code runnable}, restoring its + * previous state afterwards. + */ private void withDriverStationTemporarilyEnabled(Runnable runnable) { assertThat(RobotState.isDisabled()).isTrue(); DriverStationSim.setEnabled(true); @@ -129,11 +151,30 @@ private void withDriverStationTemporarilyEnabled(Runnable runnable) { } } + private void assertHasNoFailures(EngineExecutionResults results) { + assertHasNoFailures(results.containerEvents()); + assertHasNoFailures(results.testEvents()); + } + + private void assertHasNoFailures(Events events) { + events.assertStatistics( + stats -> { + stats.skipped(0); + stats.failed(0); + }); + } + + /** + * A test command used for verifying {@link CommandTester}. + * + *

Tracks initialize() and execute() calls, and allows validation of expected behavior. + */ private static class VerifiableCommand extends Command { private static final int EXPECTED_EXECUTION_COUNT = 4; private int initializedCount = 0; private int executionCount = 0; + /** Verifies that initialize and execute were called as expected. */ void verify() { assertWithMessage("initialize() should be called").that(initializedCount).isGreaterThan(0); assertWithMessage("initialize() should not be called more than once")