-
Notifications
You must be signed in to change notification settings - Fork 677
[wpilib] Add robotPeriodic and simulationPeriodic to OpModeRobot #8512
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: 2027
Are you sure you want to change the base?
Conversation
…t and integrate with PeriodicOpMode
| * | ||
| * @param watchdog watchdog instance, typically passed in from the calling {@link PeriodicOpMode}. | ||
| */ | ||
| public final void internalRobotPeriodic(Watchdog watchdog) { |
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.
Maybe I missed something while reading through chat history, but why make a separate internal method here instead of inlining it into PeriodicOpMode.loopFunc()?
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'm sure Peter has a better answer but we want to make it easy for other OpMode types (ie not just PeriodicOpMode) to be created by users if they want; for example, I will be using internalRobotPeriodic for CommandOpMode.
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.
Makes sense, but if we intend for users to call this method, why call it internal?
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.
It needs to be distinguished from the robotPeriodic method that users can override in their OpModeRobot subclasses and I could not think of a better name.
…odic hooks to OpModeRobot.yml
…ure proper base initialization
…and InternalRobotPeriodic in OpModeRobot.hpp
…sure proper base initialization
|
|
||
| m_watchdog.reset(); | ||
| periodic(); | ||
| m_watchdog.addEpoch("periodic()"); |
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.
Was this intentional? C++ doesn't have this change.
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.
it was not removed it was replaced with addEpoch("OpMode periodic") to clarify which periodic was running; I will change it for c++
|
@PeterJohnson do we also want to bring back a robot-level |
…isabledPeriodic and update NonePeriodic to call DisabledPeriodic by default
Note that these are only used by
PeriodicOpModeand notLinearOpMode, thoughinternalRobotPeriodicis exposed so that users can manually call it inLinearOpModeor customOpModeimplementations if they need to.