Add Copilot PR review instructions for FRC robot code (#56)

All PRs lacked FRC-aware review context, leaving Copilot unable to
enforce team standards like the state machine architecture,
FreeSpin/Positional motor subsystem split, constants discipline, and
naming conventions from the [team
wiki](https://github.com/FRCTeam3255/Wiki/blob/main/Software/Conventions.md).

## Changes

- **`.github/copilot-instructions.md`** — Global baseline applied to
every PR:
- State machine rules: one `RobotState` enum, gerund state names
(`INTAKING` not `INTAKE`), `setRobotState()` must be first in
`initialize()`
- Motor architecture: `FreeSpin.java` = velocity-controlled,
`Positional.java` = position-controlled; no cross-contamination
- Hardcoded number flagging: any numeric literal outside `constants/` or
`DeviceIDs.java` is a violation
- Follower motor naming:
`...FollowerAlignedRequest`/`...FollowerOpposedRequest`, no compass
directions in request object names
- WPILib typed units everywhere (`AngularVelocity`, `Angle`,
`Distance`); never raw `double` for physical quantities
  - `@Logged` required on every subsystem; wildcard imports preferred

- **`.github/instructions/*.instructions.md`** — Path-scoped
instructions activated via `applyTo` front-matter:

| File | Scope | Key additions |
|---|---|---|
| `constants.instructions.md` | `constants/**` | Every `TalonFX` needs a
`_CONFIGURATION` constant; all setup in `static {}` block |
| `states.instructions.md` | `commands/states/**` | `setRobotState()`
first rule; `BasePreps` `command`/`input` variable prefix pattern |
| `subsystems.instructions.md` | `subsystems/**` | FreeSpin/Positional
split enforcement; follower request naming; `periodic()` must be
lightweight |
| `commands.instructions.md` | `commands/**` | No hardware instantiation
in commands; `ChoreoTraj` constants not raw strings; `end()` cleanup |
| `deviceids.instructions.md` | `DeviceIDs.java` | CAN ID range comments
(`0–9`, `10–29`, `30–49`); one constant per motor; no subsystem name
repetition |
| `robotcontainer.instructions.md` | `RobotContainer.java` | `public
static final *Instance` + `private final logged*Instance` pair; `TRY_`
prefix for state commands; bindings in separate `config*Bindings()`
methods |

<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #55

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Tayler Uva <8679670+TaylerUva@users.noreply.github.com>

Author: Copilot

.github/copilot-instructions.md

@@ -0,0 +1,145 @@
+# GitHub Copilot Instructions — FRCTeam3255 Robot Code
+
+All code in this repository is FRC (FIRST Robotics Competition) robot code written in Java using the WPILib command-based framework. Reviews and suggestions must be evaluated through the lens of FRC best practices and team conventions documented below.
+
+Reference: [FRCTeam3255 Software Conventions](https://github.com/FRCTeam3255/Wiki/blob/main/Software/Conventions.md)
+
+---
+
+## Project Overview
+
+This robot uses a **state machine architecture**. All robot behavior is coordinated through:
+- `StateMachine.java` — manages the `RobotState` enum and validates state transitions
+- `DriverStateMachine.java` — manages driver input and drivetrain states
+- `commands/states/` — one `Command` class per `RobotState`
+
+Motor hardware is split across exactly two generic subsystems:
+- `FreeSpin.java` — all velocity-controlled (free-spinning) motors
+- `Positional.java` — all position-controlled (MotionMagic) motors
+
+All subsystem instances are declared as `public static` fields in `RobotContainer` so state commands can reference them directly.
+
+---
+
+## Naming Conventions
+
+| Element | Convention | Example |
+|---|---|---|
+| Classes | `UpperCamelCase` | `StateMachine`, `FreeSpin` |
+| Methods | `lowerCamelCase` | `setFlywheelVelocity()` |
+| Variables | `lowerCamelCase` | `commandTurretAngle` |
+| Constants | `SCREAMING_SNAKE_CASE` | `FLYWHEEL_CORNER_SPEED` |
+| Device IDs | `SCREAMING_SNAKE_CASE` | `INTAKE_ROLLERS_WEST_CAN` |
+| State commands | `TRY_` prefix + `SCREAMING_SNAKE_CASE` | `TRY_INTAKING` |
+| Controller fields | `con` prefix | `conDriver`, `conOperator` |
+| Subsystem instances | `*Instance` suffix | `freeSpinInstance`, `drivetrainInstance` |
+| Logged subsystem refs | `logged*` | `loggedFreeSpin` |
+
+---
+
+## Hardcoded Numbers — Flag These
+
+**Always flag** numeric literals that appear outside of `constants/` or `DeviceIDs.java`:
+- Magic numbers in subsystem methods, commands, or `RobotContainer`
+- Motor percent output values (e.g. `motor.set(0.5)`) — must reference a `SCREAMING_SNAKE_CASE` constant like `ConstFreeSpin.INTAKE_PERCENT_OUTPUT`
+- PID gains, setpoints, timeouts, tolerances — all belong in the matching `Const*.java` file
+- Physical quantities must use WPILib `Units` typed measures (e.g. `Angle`, `AngularVelocity`, `Distance`) — never raw `double` values
+
+The only acceptable raw `double` for motor power is a reference to a constant with the suffix `_PERCENT_OUTPUT`.
+
+---
+
+## State Machine Rules
+
+- There is **one** `RobotState` enum shared across the entire robot — never create per-mechanism enums.
+- `RobotState` values must be in gerund form (`INTAKING`, `SHOOTING`, `CLIMBING`) — not imperative (`INTAKE`, `SHOOT`, `CLIMB`).
+- Every state must have a corresponding class in `commands/states/` that extends `Command`.
+- In any state command's `initialize()`, **`setRobotState()` must be called first** — before any hardware commands.
+- State commands only require `stateMachineInstance`; hardware is driven through `RobotContainer.freeSpinInstance` and `RobotContainer.positionalInstance`.
+- The `StateMachine.tryState()` method enforces legal transitions — only valid `RobotState` transitions reach hardware.
+
+---
+
+## Subsystem Architecture
+
+- `FreeSpin.java` holds **all** velocity-controlled TalonFX motors. Do not put positional motors here.
+- `Positional.java` holds **all** position-controlled TalonFX motors. Do not put free-spin motors here.
+- Motor clusters: exactly one `...Leader` motor; all others are `...Follower`.
+- `Follower` control request objects must be named with `Follower` in the name and end in `AlignedRequest` or `OpposedRequest` — **never** include compass directions (`East`, `West`, `North`, `South`) in the request object name.
+- Separate motor clusters with a blank line for readability.
+- Every subsystem class must have `@Logged` annotation.
+- All motor configurations (TalonFX, CANcoder, Pigeon2) must come from `constants/` — no inline configuration.
+
+---
+
+## Constants Rules
+
+- One `Const*.java` file per subsystem in `constants/` (e.g. `ConstFreeSpin.java`, `ConstDrivetrain.java`).
+- All fields are `public static final`.
+- Physical quantities use WPILib `Units` typed measures — never raw `double`.
+- **Every** TalonFX motor must have a corresponding `TalonFXConfiguration` constant named `<MOTOR_NAME>_CONFIGURATION`, declared at the top of the class and fully configured in a `static {}` block.
+- Nested inner classes for logical sub-groups use `SCREAMING_SNAKE_CASE` (e.g. `PRACTICE_BOT`, `AUTO_ALIGN`) — except `constControllers` in `ConstSystem` which uses `lowerCamelCase` by convention.
+- Constant names follow `PURPOSE_DESCRIPTION` (e.g. `FLYWHEEL_CORNER_SPEED`, `INTAKE_PERCENT_OUTPUT`). Do not repeat the subsystem name inside the constant name — it's redundant through the class reference.
+
+---
+
+## DeviceIDs Rules
+
+- All hardware port/ID mappings live in a single `DeviceIDs.java` file at the root of `frc/robot/`.
+- Each subsystem has a nested inner class with `lowerCamelCase` + `IDs` suffix (e.g. `freeSpinIDs`, `drivetrainIDs`, `positionalIDs`).
+- All ID constants are `SCREAMING_SNAKE_CASE`.
+- CAN ID naming: `MECHANISM_POSITION_CAN` (e.g. `INTAKE_ROLLERS_WEST_CAN`).
+- Non-CAN IDs: `DEVICETYPE_LOCATION_CONNECTIONTYPE` (e.g. `ENCODER_FRONT_LEFT_DIO`).
+- Do not specify which subsystem an ID belongs to inside the ID name — the nested inner class already provides that context.
+- CAN ID ranges must be documented in comments: drivetrain `0–9`, FreeSpin `10–29`, Positional `30–49`.
+
+---
+
+## Units
+
+- Always use WPILib's `Units` class — never hardcode unit conversions as raw numbers.
+- Method parameters and return types for physical quantities must use typed units (e.g. `Angle`, `AngularVelocity`, `Distance`, `LinearVelocity`) not `double`.
+
+```java
+// ❌ Bad
+public void setFlywheelSpeed(double rpm) { ... }
+
+// ✔ Good
+public void setFlywheelVelocity(AngularVelocity velocity) { ... }
+```
+
+---
+
+## Logging
+
+- Every subsystem class must be annotated with `@Logged`.
+- Fields that should be excluded from logging must be annotated with `@NotLogged`.
+- Use `edu.wpi.first.epilogue.Logged` (Epilogue framework), not SmartDashboard/NetworkTables for subsystem state logging.
+
+---
+
+## Imports
+
+- Prefer wildcard imports (`.*`) over listing individual classes from the same package.
+- Import `frc.robot.commands.states.*` rather than listing each state command individually.
+
+---
+
+## RobotContainer Conventions
+
+- Controllers: `private final SN_XboxController con<Role> = new SN_XboxController(controllerIDs.<ROLE>_USB);`
+- Subsystem declarations: `public static final <Type> <type>Instance = new <Type>();` followed immediately by `private final <Type> logged<Type>Instance = <type>Instance;`
+- State-transition commands: `Command TRY_<STATE> = Commands.deferredProxy(() -> stateMachineInstance.tryState(RobotState.<STATE>));`
+- Bindings separated into `configDriverBindings()` and `configOperatorBindings()` — each a private method with no parameters.
+- Autonomous setup in `configAutonomous()`.
+
+---
+
+## FRC-Specific Considerations
+
+- `periodic()` methods should be lightweight — avoid blocking calls, heavy computation, or I/O in the scheduler loop.
+- Never use `Thread.sleep()` in robot code; use WPILib scheduling (`Commands.waitSeconds()`, etc.).
+- Brownout voltage is configured in `RobotContainer` constructor via `RobotController.setBrownoutVoltage()`.
+- Practice bot vs. competition bot differentiation is handled via RIO serial number in `ConstSystem.PRACTICE_BOT_RIO_SERIAL_NUMBER`.
+- Alliance-aware poses must use `ConstField.Pose2dAllianceSet` — never manually mirror coordinates inline.
+- Trajectory names must reference `ChoreoTraj` enum constants — never use raw strings for trajectory lookup.

.github/instructions/constants.instructions.md

@@ -0,0 +1,71 @@
+---
+applyTo: "src/main/java/frc/robot/constants/**"
+---
+
+# Constants Folder Instructions
+
+Files in `constants/` define all static configuration values for the robot. Every number that controls hardware behavior — setpoints, gains, speeds, positions, timeouts — must live here.
+
+## File Naming & Structure
+
+- One file per subsystem: `Const<SubsystemName>.java` (e.g. `ConstFreeSpin.java`, `ConstPositional.java`).
+- File is a `public class` (not `final`, unless it is `ConstSystem`) with no constructor.
+- `ConstSystem.java` is `public final class` and holds cross-subsystem constants (controller deadbands, RIO serial numbers, etc.).
+
+## Field Declarations
+
+- All fields must be `public static final`.
+- All field names must be `SCREAMING_SNAKE_CASE`.
+- Use WPILib `Units` typed measures for **all** physical quantities — no raw `double` for speeds, angles, distances, or times.
+  - `AngularVelocity`, `Angle`, `Distance`, `LinearVelocity`, `Time`, `Voltage`, `Current`, etc.
+- Raw `double` is only acceptable for dimensionless ratios or motor percent output values (suffix `_PERCENT_OUTPUT`).
+
+## TalonFX Configuration Requirement
+
+- **Every** TalonFX motor in the project **must** have a corresponding `TalonFXConfiguration` constant in its subsystem's constants file.
+- Name: `<MOTOR_DESCRIPTIVE_NAME>_CONFIGURATION` (e.g. `FLYWHEEL_WEST_CONFIGURATION`).
+- Declare the configuration object at the top of the class.
+- All `.apply()` calls must happen in a `static {}` initializer block — never inline in the subsystem constructor.
+
+```java
+// ✔ Good
+public class ConstFreeSpin {
+    public static final TalonFXConfiguration FLYWHEEL_WEST_CONFIGURATION = new TalonFXConfiguration();
+
+    static {
+        FLYWHEEL_WEST_CONFIGURATION.MotorOutput.NeutralMode = NeutralModeValue.Coast;
+        FLYWHEEL_WEST_CONFIGURATION.MotorOutput.Inverted = InvertedValue.CounterClockwise_Positive;
+    }
+}
+```
+
+## Grouping with Inner Classes
+
+- Use nested `public static class` blocks to group related constants (e.g. hardware variants, tuning sets).
+
+
+## Naming Rules
+
+- Constant names follow `PURPOSE_DESCRIPTION` — what it is used for + minimum detail to remove ambiguity.
+- Do **not** repeat the subsystem name inside a constant's name — it is already implied by the class context.
+- Valid examples: `FLYWHEEL_CORNER_SPEED`, `INTAKE_ROLLER_PERCENT_OUTPUT`, `OUTTAKE_TOLERANCE`, `CURRENT_LIMIT_FLOOR`.
+- Avoid: `FREESPIN_FLYWHEEL_CORNER_SPEED` (subsystem name is redundant).
+
+## Unit Conversions
+
+- Never hardcode a pre-computed conversion result. Derive it from other constants using `Units`:
+
+```java
+// ❌ Bad
+public static final double WHEEL_CIRCUMFERENCE = 0.31742888;
+
+// ✔ Good
+public static final double WHEEL_CIRCUMFERENCE = WHEEL_DIAMETER.in(Units.Meters) * Math.PI;
+```
+
+## Common Review Flags
+
+- Any raw numeric literal in a subsystem or command that should be in constants.
+- Missing `TalonFXConfiguration` for any motor that exists in `DeviceIDs`.
+- Physical quantity stored as `double` instead of a WPILib typed measure.
+- Configuration logic placed in the subsystem constructor rather than a `static {}` block here.

.github/instructions/deviceids.instructions.md

@@ -0,0 +1,76 @@
+---
+applyTo: "src/main/java/frc/robot/DeviceIDs.java"
+---
+
+# DeviceIDs.java Instructions
+
+`DeviceIDs.java` is the **single source of truth** for all hardware port and ID mappings on the robot. Every motor, encoder, sensor, and controller ID lives here and nowhere else.
+
+## File Structure
+
+- One top-level `public class DeviceIDs` with no constructor.
+- One nested `public static class` per subsystem, named in `lowerCamelCase` with an `IDs` suffix.
+- A comment above each inner class indicating its CAN ID range.
+
+```java
+public class DeviceIDs {
+    public static class controllerIDs { ... }   // USB ports
+
+    // Drivetrain IDs: 0–9
+    public static class drivetrainIDs { ... }
+
+    // FreeSpin IDs: 10–29
+    public static class freeSpinIDs { ... }
+
+    // Positional IDs: 30–49
+    public static class positionalIDs { ... }
+}
+```
+
+## ID Constant Naming
+
+- All ID constants are `public static final int` (or `CANBus` for the CAN bus name) and `SCREAMING_SNAKE_CASE`.
+- CAN IDs: `MECHANISM_POSITION_CAN` (e.g. `INTAKE_ROLLERS_WEST_CAN`, `FLYWHEEL_WEST_CAN`).
+- Non-CAN: `DEVICETYPE_LOCATION_CONNECTIONTYPE` (e.g. `ENCODER_FRONT_LEFT_DIO`, `DRIVER_USB`).
+- Do **not** include the subsystem name in the constant name — the inner class already provides that context.
+
+```java
+// ❌ Bad — subsystem name is redundant inside freeSpinIDs
+public static class freeSpinIDs {
+    public static final int FREESPIN_INTAKE_ROLLERS_WEST_CAN = 10;
+}
+
+// ✔ Good
+public static class freeSpinIDs {
+    public static final int INTAKE_ROLLERS_WEST_CAN = 10;
+}
+```
+
+## CAN ID Ranges
+
+Maintain the established ID range blocks and keep IDs within their assigned ranges:
+- `drivetrainIDs`: `0–9`
+- `freeSpinIDs`: `10–29`
+- `positionalIDs`: `30–49`
+
+Flag any ID that falls outside its designated range.
+
+## Every Motor Needs an ID
+
+- Every `TalonFX` declared in `FreeSpin.java` or `Positional.java` must have a corresponding constant in `freeSpinIDs` or `positionalIDs` respectively.
+- Every `TalonFX` in `Drivetrain.java` must have a corresponding constant in `drivetrainIDs`.
+- Flag any motor instantiated with a raw integer literal instead of a reference to `DeviceIDs`.
+
+## CANBus
+
+- The swerve drivetrain CAN bus is declared as a `CANBus` object (not an `int`) in `drivetrainIDs`.
+- Name: `CAN_BUS_NAME`.
+
+## Common Review Flags
+
+- New motor added to a subsystem but no corresponding ID added here.
+- ID constant name repeats the subsystem name (redundant with the inner class).
+- Raw integer literal used in a subsystem motor constructor instead of referencing `DeviceIDs`.
+- ID value falls outside the designated range for its inner class.
+- New subsystem added but no corresponding inner class created in `DeviceIDs`.
+- Inner class name doesn't follow `lowerCamelCase` + `IDs` pattern.

.github/instructions/robotcontainer.instructions.md

@@ -0,0 +1,96 @@
+---
+applyTo: "src/main/java/frc/robot/RobotContainer.java"
+---
+
+# RobotContainer.java Instructions
+
+`RobotContainer.java` is the top-level composition class — it instantiates all subsystems, declares all command objects, and configures all button-to-command bindings.
+
+## Subsystem Declarations
+
+Each subsystem must be declared as a `public static final` instance with an `*Instance` suffix, immediately followed by a `private final` logged reference pointing to the same object:
+
+```java
+public static final FreeSpin freeSpinInstance = new FreeSpin();
+private final FreeSpin loggedFreeSpin = freeSpinInstance;
+
+public static final Positional positionalInstance = new Positional();
+private final Positional loggedPositional = positionalInstance;
+```
+
+- The `loggedXxx` field enables Epilogue's `@Logged` annotation on `RobotContainer` to automatically log subsystem state.
+- Do not add `@NotLogged` to the logged reference fields — that would defeat their purpose.
+- Do not instantiate subsystems anywhere else in the codebase.
+
+## Controller Declarations
+
+```java
+private final SN_XboxController conDriver = new SN_XboxController(controllerIDs.DRIVER_USB);
+private final SN_XboxController conOperator = new SN_XboxController(controllerIDs.OPERATOR_USB);
+```
+
+- Controller fields are `private final`.
+- Name: `con` + role (e.g. `conDriver`, `conOperator`).
+- Always reference `controllerIDs` from `DeviceIDs` — never use a raw integer.
+
+## State Transition Command Declarations
+
+State transition commands must:
+- Be `SCREAMING_SNAKE_CASE`.
+- Be prefixed with `TRY_`.
+- Use `Commands.deferredProxy(() -> stateMachineInstance.tryState(RobotState.<STATE>))`.
+
+```java
+Command TRY_INTAKING = Commands.deferredProxy(
+    () -> stateMachineInstance.tryState(RobotState.INTAKING));
+Command TRY_NONE = Commands.deferredProxy(
+    () -> stateMachineInstance.tryState(RobotState.NONE));
+```
+
+Non-state commands (drive modes) are also `SCREAMING_SNAKE_CASE` with a descriptive name (no `TRY_` prefix):
+
+```java
+Command MANUAL = new DeferredCommand(
+    driverStateMachineInstance.tryState(DriverState.MANUAL, ...), Set.of(...));
+```
+
+## Constructor
+
+The constructor must call these methods in order:
+1. Set controller deadbands (e.g. `conDriver.setLeftDeadband(...)`)
+2. Set default commands (e.g. `driverStateMachineInstance.setDefaultCommand(MANUAL)`)
+3. `configDriverBindings()`
+4. `configOperatorBindings()`
+5. `configAutonomous()`
+6. Any global hardware setup (e.g. `RobotController.setBrownoutVoltage(ConstSystem.BROWNOUT_VOLTAGE)`)
+
+## Binding Methods
+
+- `configDriverBindings()` — `private`, no parameters, configures all driver controller bindings.
+- `configOperatorBindings()` — `private`, no parameters, configures all operator controller bindings.
+- `configAutonomous()` — `private` or package-private, configures `AutoFactory`, registers autonomous routines, and sets up `SendableChooser`.
+
+Never define bindings inline in the constructor.
+
+## Autonomous Configuration
+
+- The `AutoFactory` is the single factory for all Choreo trajectory commands.
+- Autonomous routines are registered with `autoChooser.addOption()`.
+- Starting poses must be set via `autoFactory.resetOdometry(path.name()).ignoringDisable(true)` in an `onChange` listener.
+- All trajectory references use `ChoreoTraj` enum constants — never raw strings.
+
+## `isPracticeBot()`
+
+- Practice bot detection uses `RobotController.getSerialNumber().equals(ConstSystem.PRACTICE_BOT_RIO_SERIAL_NUMBER)`.
+- The serial number lives in `ConstSystem` — never hardcode it here.
+
+## Common Review Flags
+
+- Subsystem declared without the paired `loggedXxx` field.
+- Subsystem declared without `public static final`.
+- Controller declared with a raw integer port instead of `controllerIDs.*`.
+- State transition command not using `Commands.deferredProxy()`.
+- State transition command missing the `TRY_` prefix.
+- Bindings defined inline in the constructor instead of in `configDriverBindings()` / `configOperatorBindings()`.
+- Trajectory referenced as a raw string instead of a `ChoreoTraj` constant.
+- Numeric literal used for brownout voltage or deadband instead of a `ConstSystem` constant.