Ratio Coupling

[ShmayaR]

No description provided.

[coderabbitai]

Warning

Rate limit exceeded

@ShmayaR has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 58 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Walkthrough

The SwerveModule subsystem was modified to account for drive-steer wheel coupling. A new constant COUPLED_RATIO = 3 was added to SwerveModuleConstants. The getDriveWheelPositionRadians method now subtracts the steer angle coupling offset from the raw drive position before converting to radians. However, in setTargetClosedLoopDriveVelocity, a new local variable velocityToSet is calculated using the coupled ratio but remains unused—the drive motor still receives the original targetDriveVelocityRotationsPerSecond. This appears to be an incomplete implementation that requires verification to ensure the velocity compensation is intentional or if velocityToSet should be passed to the motor control.

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No description was provided by the author, making it impossible to assess whether the pull request intent was documented.	Add a description explaining the coupling ratio implementation, why it's needed, and how it affects the swerve module behavior.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Ratio Coupling' accurately reflects the main change: introducing a COUPLED_RATIO constant and applying it to compensate for drive wheel position coupling in the swerve module.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/java/frc/trigon/robot/subsystems/swerve/swervemodule/SwerveModule.java (1)

131-136: ⚠️ Potential issue | 🟠 Major

Odometry positions lack coupling compensation.

getDriveWheelPositionRadians now compensates for steer-drive coupling, but getOdometryPosition feeds raw latestOdometryDrivePositions directly into driveWheelRotationsToMeters without any coupling adjustment. This means your high-frequency odometry updates will still include steer-induced drift, undermining the fix you applied in getDriveWheelPositionRadians.

You should apply the same coupling subtraction here, using the corresponding steer position from latestOdometrySteerPositions:

Proposed fix

 public SwerveModulePosition getOdometryPosition(int odometryUpdateIndex) {
+    final double steerPositionRotations = latestOdometrySteerPositions[odometryUpdateIndex];
+    final double coupledDrivePosition = latestOdometryDrivePositions[odometryUpdateIndex] - (steerPositionRotations * SwerveModuleConstants.COUPLED_RATIO);
     return new SwerveModulePosition(
-            driveWheelRotationsToMeters(latestOdometryDrivePositions[odometryUpdateIndex]),
+            driveWheelRotationsToMeters(coupledDrivePosition),
             Rotation2d.fromRotations(latestOdometrySteerPositions[odometryUpdateIndex])
     );
 }

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Add a brief comment explaining what this ratio represents.

COUPLED_RATIO is not self-evident to someone unfamiliar with the mechanical coupling between the steer and drive gears. A one-liner explaining it (e.g., the gear ratio between the steer and drive shafts that causes drive encoder drift when steering) would go a long way. Based on learnings, comments should explain "why" or non-obvious decisions.