Add TorqueCurrent Example #51
Conversation
📝 WalkthroughWalkthroughThe pull request adds torque characterization capabilities to the drive subsystem by introducing a new SysId routine in the Drive class and two new request classes. The Drive class now holds member variables for torque translation characterization that are configured with specific parameters and integrated with a logging mechanism. In addition, new classes implementing the SwerveRequest interface are added to apply torque current to both steer and translation modules through overloaded methods and an apply function. Changes
Sequence Diagram(s)sequenceDiagram
participant Operator
participant Drive as Drive Subsystem
participant TorqueTranslation as SysIdSwerveTranslation_Torque
participant Logger as SignalLogger
participant Module as SwerveModule
Operator->>Drive: Trigger torque translation characterization
Drive->>TorqueTranslation: Initialize with configuration (ramp rate, step voltage, timeout)
TorqueTranslation->>Logger: Log start of torque characterization
TorqueTranslation->>Module: Apply torque current (via withTorqueCurrent)
Module-->>TorqueTranslation: Acknowledge application
TorqueTranslation-->>Drive: Return StatusCode
sequenceDiagram
participant Operator
participant SteerGains as SysIdSwerveSteerGains_Torque
participant Module as SwerveModule
Operator->>SteerGains: Set torque current (withTorqueCurrent)
SteerGains->>Module: Apply drive (CoastOut) and steer (TorqueCurrentFOC) requests
Module-->>SteerGains: Acknowledge application
SteerGains-->>Operator: Return StatusCode
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/frc/robot/subsystems/drive/requests/SysIdSwerveSteerGains_Torque.java (1)
19-19: Follow Java naming conventions for public variables.The variable
TorqueCurrentToApplyshould be renamed totorqueCurrentToApplyto follow Java naming conventions.- public double TorqueCurrentToApply = 0; + public double torqueCurrentToApply = 0;src/main/java/frc/robot/subsystems/drive/requests/SysIdSwerveTranslation_Torque.java (2)
19-19: Follow Java naming conventions for public variables.The variable
TorqueCurrentToApplyshould be renamed totorqueCurrentToApplyto follow Java naming conventions.- public double TorqueCurrentToApply = 0; + public double torqueCurrentToApply = 0;
24-26: Document the selection criteria for steer closed-loop type.While the comment indicates there are two options, it would be helpful to document the criteria for selecting between PositionVoltage and PositionTorqueCurrentFOC.
- // Select one of the two below based on which type of steer closed-loop you are using + // Select the appropriate steer closed-loop type: + // - Use PositionVoltage for voltage-based position control + // - Use PositionTorqueCurrentFOC for torque-based position control with field-oriented control
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/frc/robot/subsystems/drive/Drive.java(3 hunks)src/main/java/frc/robot/subsystems/drive/requests/SysIdSwerveSteerGains_Torque.java(1 hunks)src/main/java/frc/robot/subsystems/drive/requests/SysIdSwerveTranslation_Torque.java(1 hunks)
🔇 Additional comments (6)
src/main/java/frc/robot/subsystems/drive/requests/SysIdSwerveSteerGains_Torque.java (2)
26-32: LGTM! Clean implementation of the apply method.The implementation correctly applies coast to drive motors while applying torque current to steer motors.
40-53: LGTM! Well-structured method overloading.Good use of method overloading to support both primitive double and Current unit types.
src/main/java/frc/robot/subsystems/drive/requests/SysIdSwerveTranslation_Torque.java (1)
28-35: LGTM! Clean implementation of the apply method.The implementation correctly applies torque current to drive motors while maintaining steer position at 0.
src/main/java/frc/robot/subsystems/drive/Drive.java (3)
93-96: LGTM! Clear initialization of torque characterization.Good addition of the torque characterization request with clear comment indicating it's an example.
97-113: LGTM! Well-configured SysId routine for torque characterization.The configuration is well-structured with:
- Appropriate ramp rate of 5 A/s
- Reasonable dynamic step of 10 A
- Safe timeout of 5 seconds
- Proper logging integration
164-164: Consider updating default SysId routine.The default routine is set to translation characterization. Consider if torque characterization should be the default for better initial characterization.
Would you like me to help determine the most appropriate default characterization routine based on the codebase usage?
|
Closed #51 |
Add TorqueCurrent example
Summary by CodeRabbit