-
Notifications
You must be signed in to change notification settings - Fork 55
Edited the mecanum drive example code to be more frc-docs like. #50
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
Edited the mecanum drive example code to be more frc-docs like. #50
Conversation
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.
Most of the renames, including the constant prefix k
, look reasonable to me. I'd like a couple of changes undone so that this actually is more frc-docs like though.
You'll need to run the black
autoformatter after making those changes.
mecanum-drive/robot.py
Outdated
|
||
def teleopInit(self): | ||
self.drive.setSafetyEnabled(True) | ||
self.m_stick = Joystick(self.kJoystickChannel) |
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.
Our examples drop the m_
prefix as it's already clear that these are member variables by the mandatory use of self
. We settled on this in the frc-docs PR too.
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.
on it.
mecanum-drive/robot.py
Outdated
@@ -6,52 +6,38 @@ | |||
|
|||
import wpilib | |||
from wpilib.drive import MecanumDrive | |||
from wpilib import Joystick, PWMSparkMax, TimedRobot |
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 believe we standardised on avoiding from wpilib import
over in frc-docs (cf. the Zen of Python). For anything that's in the root wpilib
namespace, we should just reference wpilib.
similarly to how the C++ examples use frc::TimedRobot
and don't do using namespace frc;
or similar.
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.
on it.
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.
Thank you for helping keep our examples up-to-date ✨
As stated in the #49 issue in the base repository, we need example code that matches WPIlib examples closely.
I may have broken some PEP guidelines, but I believe we need formatting to be in line with C++/Java examples.
The code has been tested in simulation only, because my team does not have any SparkMax motor controllers.