Conversation
* Small fix * spotless
…vantageScope and other tools parse them.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates robot path parameters, increases drivebase acceleration, and refines launcher and turret control logic. Key changes include switching the turret to position-based control, updating PID gains and current limits across several subsystems, and adding sticky fault clearing to motor initializations. The vision subsystem was simplified by removing the intake pivot dependency, and vendor dependencies were updated. Feedback focuses on resolving command conflicts in trigger bindings and parallel groups within Controls.java, ensuring consistent indexer speed calculations, and optimizing Limelight configuration updates in the robot's disabled state.
| double fRPS = flywheelRPS.getAsDouble() * 0.83 + 15; | ||
| double sRPS = Math.min(fRPS * 1.5, 70); |
There was a problem hiding this comment.
These magic numbers (0.83, 15, 1.5, 70) used for calculating indexer speeds should be defined as named constants in the class or a configuration file to improve maintainability and clarity.
There was a problem hiding this comment.
they are magic numbers Tristan came up with
There was a problem hiding this comment.
Not really magic - they're the ratios to get fuel to transition more smoothly between spindexer, feeder & flywheel by having it not suddenly change velocity at the transition points. But I'm not sure what you'd call them, as they're essentially the a and b in the output = a + bx equation. Just having a comment before each equation explaining what it does would likely be better.
There was a problem hiding this comment.
Agree - a comment explaining here would be sufficient.
| double fRPS = flywheelRPS.getAsDouble() * 0.83 + 15; | ||
| double sRPS = Math.min(fRPS * 1.5, 70); |
| public Command setVelocityCommand(double rps) { | ||
| return runEnd( | ||
| () -> { | ||
| request.Velocity = rps; |
There was a problem hiding this comment.
Since this code is the same as setVelocityRPS(), just replace this with a call to that method.
| config.MotorOutput.Inverted = InvertedValue.Clockwise_Positive; | ||
| applyConfig(flywheelTwo, config); | ||
|
|
||
| // flywheelTwo.setControl(new Follower(Hardware.FLYWHEEL_ONE_ID, MotorAlignmentValue.Opposed)); |
jamesdooley4
left a comment
jamesdooley4
left a comment
There was a problem hiding this comment.
Some minor comments to cleanup
7 participants
missing some ds commits will be added monday