-
Notifications
You must be signed in to change notification settings - Fork 284
Improve motor controller code examples for clarity #3144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Improve motor controller code examples for clarity #3144
Conversation
- Changed confusing variable names (Spark spark -> intakeMotor) - Used different PWM ports (0 and 1) to avoid confusion - Added descriptive comments for each line - Added 'Basic Usage' section header - Added important note explaining set() range (-1.0 to 1.0) - Added 'Where to Put This Code' section with examples for: - Command-Based programs (subsystem class) - Timed Robot programs (Robot class with robotInit/teleopPeriodic) - Added 'Common Use Cases' section listing real mechanisms: - Intakes, shooters, conveyors, arms/elevators, climbers - Links to drivetrain classes and WPILib examples Fixes wpilibsuite#1385
source/docs/software/hardware-apis/motors/using-motor-controllers.rst
Outdated
Show resolved
Hide resolved
…ers.rst Co-authored-by: Dan Katzuv <[email protected]>
} | ||
private: | ||
std::unique_ptr<frc::Spark> m_intakeMotor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is how we want to declare in C++
|
||
## CAN Motor Controllers | ||
|
||
A handful of CAN motor controllers are available through vendors such as CTR Electronics, REV Robotics, and Playing with Fusion. See :doc:`/docs/software/can-devices/third-party-devices`, :doc:`/docs/software/vscode-overview/3rd-party-libraries`, and :doc:`/docs/software/examples-tutorials/third-party-examples` for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's more vendors with CAN motors now, right?
|
||
## Using PWM Motor Controllers | ||
|
||
PWM motor controllers can be controlled in the same way as a CAN motor controller. For a more detailed background on *how* they work, see :doc:`pwm-controllers`. To use a PWM motor controller, simply use the appropriate motor controller class provided by WPILib and supply it the port the motor controller(s) are plugged into on the roboRIO. All approved motor controllers have WPILib classes provided for them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd delete the line about PWM motor controllers being controlled the same as CAN controllers. I think it's potentially confusing since there's a lot of differences, and as CAN controllers haven't been discussed, assuming that people know what that means doesn't seem like a good assumption.
|
||
.. note:: The ``Spark`` and ``VictorSP`` classes are used here as an example; other PWM motor controller classes have exactly the same API. | ||
|
||
### Basic Usage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the Basic Usage section doesn't add value, compared to the command and timed sections below and is potentially confusing. I'd delete it. For example, it shows both declaration and usage, and only addresses where declaration goes, but the command and timed sections show both clearly. Really the only thing this adds is the ranges of the set method.
Summary
Significantly improves the motor controller code examples to address issue #1385 by fixing confusing variable names and adding context about where code should be placed in a robot program.
Changes
Fixed Confusing Variable Names
Before:
Spark spark = new Spark(0);
(the "Buffalo buffalo" problem)After:
Spark intakeMotor = new Spark(0);
(clear, descriptive name)intakeMotor
,shooterMotor
)Added Context Sections
Basic Usage:
set()
range (-1.0 to 1.0 with meanings)Where to Put This Code (NEW):
runIntake()
andstopIntake()
methodsrobotInit()
andteleopPeriodic()
usageCommon Use Cases (NEW):
Impact
This transformation makes the article significantly more useful for beginners by:
Fixes #1385