Feature/shift UI display

[y0shi]

No description provided.

[github-actions]

AI Code Review

Hello team!

This is a fantastic pull request introducing a comprehensive UI feedback system! It's clear a lot of thought has gone into providing real-time information to the drive team through visual cues and haptic feedback. This will be a huge asset during matches.

Positive Highlights

1. Robust State Management: The UISubsystem uses an explicit FeedbackState enum, making the logic for visual and haptic feedback very clear and easy to understand. The ShiftState also uses enums for shifts, which is excellent.
2. Excellent Use of Command Factory Methods: The UISubsystem provides manualRumbleCommand and timedRumbleCommand, which are perfect examples of how subsystems should expose their functionality as commands. This aligns perfectly with WPILib's command-based best practices.
3. Configurability and Tunability: The creation of UIConstants for fixed values and UIPreferences for tunable thresholds is a great practice. This allows for quick adjustments on the fly without code changes, which is invaluable during competition.

Suggestions

Here are a few suggestions to consider, mostly minor clarifications or refinements, to make this even better:

Code Quality & Clarity

• ShiftState.java - TELEOP_START_TIME Constant:

  • Context: The ShiftState class introduces TELEOP_START_TIME = 140.0; with a comment "20s auto per 2026 REBUILT". Standard FRC matches have a 15-second autonomous period, after which DriverStation.getMatchTime() typically resets to 135 seconds (for a 2:15 total match). The current logic for shifts starts checking matchTime >= TRANSITION_END_TIME (130s).
  • Suggestion: Consider if TELEOP_START_TIME should be 135.0 (for 15s auto) or adjusted based on the actual match rules for 2026. If a 20s auto is indeed planned, then 130.0 for TRANSITION_END_TIME makes sense as the start of the first teleop shift. Clarifying this constant's relation to DriverStation.getMatchTime() and the autonomous duration would be helpful.
  • Why it matters: Accurate timing constants are crucial for the ShiftState to correctly reflect match progression, which directly impacts the UISubsystem's feedback.

• UIConstants.java - Toggle Cycle Comments:

  • Context: Comments like YELLOW_PULSE_TOGGLE_CYCLES = 12; // ~2Hz (on 12 cycles, off 13 cycles) describe the pulsing frequency. The implementation (cycleCounter / TOGGLE_CYCLES) % 2 == 0 means the visual/rumble effect is "on" for TOGGLE_CYCLES and "off" for TOGGLE_CYCLES.
  • Suggestion: Adjust the comments to be consistent with the implementation (e.g., "on 12 cycles, off 12 cycles"). For 12 cycles on and 12 cycles off, the total period is 24 cycles. At 50Hz, this is 50 / 24 = ~2.08 Hz.
  • Why it matters: Consistent comments help future developers understand the exact timing and behavior, preventing confusion or misinterpretation.

FRC/WPILib Best Practices

• UISubsystem.java - manualOverride for manualRumbleCommand:

  • Context: The UISubsystem has a manualOverride flag that, when true, disables the automatic periodic rumble patterns. The timedRumbleCommand correctly sets this flag on initialize and clears it on end. However, manualRumbleCommand (used for operatorJoystick.y().whileTrue(...)) does not interact with manualOverride.
  • Suggestion: If the intent is for any manual rumble to override periodic rumble, then manualRumbleCommand should also set manualOverride = true on initialize and false on end. If manualOverride is only intended for timedRumbleCommand (e.g., for specific event-driven feedback), then consider renaming manualOverride to timedRumbleActive for clarity.
  • Why it matters: This ensures consistent behavior where manual feedback doesn't conflict with or is not overridden by automatic feedback, and clarifies the intended interaction between different rumble types.

• UISubsystem.java - timedRumbleCommand Intensity Constant:

  • Context: The timedRumbleCommand hardcodes a rumble intensity of 0.5.
  • Suggestion: Consider defining a constant in UIConstants for this specific rumble intensity (e.g., RUMBLE_TIMED_COMMAND_INTENSITY). This would improve consistency and make it easier to adjust this value if needed.
  • Why it matters: Using named constants instead of "magic numbers" improves readability and maintainability, making it easier to understand and modify the code.

Questions

• Are there any plans to add visual feedback on the robot itself (e.g., LEDs) that would also be controlled by the UISubsystem?
• Is the current manualOverride behavior (global disable of periodic rumble) the intended behavior, even if only one controller is manually rumbling?

Encouraging Note

This is a fantastic addition to the robot's control system! The detailed feedback mechanism will undoubtedly enhance driver awareness and performance during matches. Keep up the great work!

---

This review was automatically generated by AI. Please use your judgment and feel free to discuss any suggestions!

[github-actions]

✓ Build successful and code formatting check passed!