WIP: feat(motors): implement all standard FRC motors #1
Conversation
Baconing
added
Kind/Enhancement
WalkthroughThe recent changes introduce two new motor classes, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SparkMotor
participant CANSparkMax
User->>SparkMotor: setSpeed(speed)
SparkMotor->>CANSparkMax: set(speed)
CANSparkMax-->>SparkMotor: Confirm speed set
SparkMotor-->>User: Speed set confirmation
sequenceDiagram
participant User
participant TalonFXMotor
participant TalonFX
User->>TalonFXMotor: setSpeed(speed)
TalonFXMotor->>TalonFX: set(speed)
TalonFX-->>TalonFXMotor: Confirm speed set
TalonFXMotor-->>User: Speed set confirmation
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add 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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/main/java/net/frc5183/librobot/hardware/motor/SparkMaxMotor.java (2 hunks)
- src/main/java/net/frc5183/librobot/hardware/motor/SparkMotor.java (1 hunks)
- src/main/java/net/frc5183/librobot/hardware/motor/TalonFXMotor.java (1 hunks)
Additional context used
GitHub Check: pmd
src/main/java/net/frc5183/librobot/hardware/motor/TalonFXMotor.java
[warning] 4-4: Unused import 'edu.wpi.first.wpilibj.motorcontrol.MotorController'
Reports import statements that can be removed. They are either unused,
duplicated, or the members they import are already implicitly in scope,
because they're in java.lang, or the current package.If some imports cannot be resolved, for instance because you run PMD with
an incomplete auxiliary classpath, some imports may be conservatively marked
as used even if they're not to avoid false positives.UnnecessaryImport (Priority: 4, Ruleset: Code Style)
https://docs.pmd-code.org/pmd-doc-7.4.0/pmd_rules_java_codestyle.html#unnecessaryimport
Additional comments not posted (4)
src/main/java/net/frc5183/librobot/hardware/motor/SparkMotor.java (1)
1-62: LGTM!The
SparkMotorclass is well-implemented, providing essential motor control functionalities. The constructors and methods are clear and follow best practices. Ensure that unit tests cover all functionalities.src/main/java/net/frc5183/librobot/hardware/motor/TalonFXMotor.java (1)
1-63: LGTM!The
TalonFXMotorclass is well-implemented, providing essential motor control functionalities. The constructors and methods are clear and follow best practices. Ensure that unit tests cover all functionalities.Tools
GitHub Check: pmd
[warning] 4-4: Unused import 'edu.wpi.first.wpilibj.motorcontrol.MotorController'
Reports import statements that can be removed. They are either unused,
duplicated, or the members they import are already implicitly in scope,
because they're in java.lang, or the current package.If some imports cannot be resolved, for instance because you run PMD with
an incomplete auxiliary classpath, some imports may be conservatively marked
as used even if they're not to avoid false positives.UnnecessaryImport (Priority: 4, Ruleset: Code Style)
https://docs.pmd-code.org/pmd-doc-7.4.0/pmd_rules_java_codestyle.html#unnecessaryimportsrc/main/java/net/frc5183/librobot/hardware/motor/SparkMaxMotor.java (2)
12-12: Improved documentation.The updated class documentation accurately reflects the purpose of the
SparkMaxMotorclass. This enhances the clarity of the code.
71-71: Simplified method signature.The
getRawMotor()method now directly returns aCANSparkMaxinstance, simplifying the method and aligning it with the class's purpose.
There was a problem hiding this comment.
So, I literally just saw this PR while browsing the org's new repos.
Overall the docs and everything is pretty good but I would consider adding "follow()" functionality into the motor system. The Phoenix API has it built in, I believe that Rev has a follow system as well, implemented differently. It's not strictly necessary but it can be nice to use the hardware-based following (especially for Phoenix-based motors, as I believe it can use daisy chaining and echoing to declog the CAN network instead of 2 separate streams)
This pull requests implements the following motors:
in addition to potential modifications to other motors.
Summary by CodeRabbit
New Features
SparkMotorclass for enhanced control over Spark motor controllers, including methods for speed adjustment, safety, and inversion management.TalonFXMotorclass to simplify integration and control of TalonFX motor controllers, featuring flexible instantiation and various motor control methods.Improvements
SparkMaxMotorclass by updating its documentation and simplifying method visibility.