-
Notifications
You must be signed in to change notification settings - Fork 631
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
[design-docs] Add modes design doc #7863
base: 2027
Are you sure you want to change the base?
Conversation
Here are some thoughts from my initial read of things.
I think there has been some discussion of this already, but a partial solution I could see to (1) and (2) is having a separate robot base class for command-based that requires only command-based modes and calls the command scheduler itself across all modes (and maybe provides a global
Clearly I'm approaching this (in part) from the perspective of supporting AdvantageKit in some form going forward. While I don't think we're planning any official AdvantageKit integration in 2027 (maybe 2028 or beyond), we have also been successful on the roboRIO with tweaking WPILIb to minimize the number of "intrusive" modifications required on the part of AdvantageKit. I'm operating on the assumption that we would like to continue a similar approach in 2027 to minimize the support burden for FRC teams that continue to use AdvantageKit. |
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.
These are mostly just comments on clarity and grammar. (Up to and not including the design section, since that's as far as I got before needing to go) There's a few more nitpicks (inconsistent usage of dictionary versus map, inconsistent inclusion of parentheses for method calls), but those are pretty minor and don't detract from the readability.
Is the plan that the RobotBase class managing these modes is single-threaded or multi-threaded? Definitely still single threaded, I don't expect that to change. Linear modes would still run in the main robot thread. It would need to be an external thread monitoring, that user code won't have access to (In fact, its possible its a completely separate process and not just a thread). And if the loop doesn't exit, the whole process would be torn down, not just the thread. |
Default commands can still exist as they do today. They will just only run when the command scheduler runs, which only happens in command-based modes. We could change the example code to use a default command and it will still work as it does today.
Yeah it’s mentioned in the trades section that potentially command-based should be a robot-wide thing, and you wouldn’t be able to mix command and non-command modes in a command-based robot. That restriction might make command-based feel more cohesive, but would make it basically a completely separate project level thing you have to use, rather that being able to use it as a transitional thing in a project (which maybe isn’t realistic anyway due to subsystems etc, to your point). |
In this case, does there need to be a way to create mode-specific default commands in addition to the current implementation? It seems like the example code was trying to demonstrate how a command like joystick drive could be bound to a specific mode. Global default commands are definitely still useful and should stick around, but it looked like the intention based on the example was to have a similar feature available in a mode-specific context.
This path makes sense to me overall. How can/should global periodic callbacks fit into this structure, given the utility of The other question is whether something like this is in scope for the non-command variant of robot base. I think there's still an application for a periodic telemetry function regardless of mode, but that may just be impractical alongside e.g. linear modes. |
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.
At some point it'd also be nice to clean up group vs category, but it doesn't matter that much.
|
||
- Should it be possible to have multiple top-level Robot classes (e.g. for different robot configurations), and have that be selectable at the DS as well? This is a bit ugly to support even if when the Robot object is passed into the mode constructor, because different Robot class types will only work with certain modes. | ||
|
||
- Python--will decorators be able to work similarly to Java annotations for mode registration? |
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.
As someone already commented in Discord, I doubt we could have a robust setup in Python for finding all classes that use a certain annotation. However, I'm not sure why we would want to use a metaclass-based registration instead of a decorator-based registration.
I unfortunately don't know the idiomatic way to handle the Python overloading, but something like this should work:
def Autonomous(cls_or_name: str | type, category: str = "default", description: str = ""):
if isinstance(cls_or_name, str):
def autonomous_mode_factory(cls: type):
RobotBase.autonomous_mode_factory(lambda: cls(), cls_or_name, category, description)
return cls
return autonomous_mode_factory
elif isinstance(cls_or_name, type):
RobotBase.add_autonomous_mode(lambda: cls_or_name(), cls_or_name.__name__, category, description)
else:
# Handle invalid type here
pass
(See Python reference 8.8: Class definitions for details about the decorator process. Notably, no-arg at the decoration site means the decorator is called directly, while args at the decoration site means the decorator is called with the arguments and returns a callable that receives the class.)
Right now it uses RobotBase.add_autonomous_mode()
which is currently non-static in the example. I'm not sure why it needs to be non-static, but if it does actually need to be non-static, then we could have a global dictionary or list that the modes get added to which the robot instance reads and registers when it is initialized.
Co-authored-by: Starlight220 <[email protected]>
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.
Overall, I am concerned about how complicated this is. It does not feel approachable for a beginning programmer.
This document describes a standardized approach for operators to select different code to run for different robot modes of operation and for programmers to easily write code that creates these selection options. | ||
|
||
A note on terminology: the word "mode" is used in this document to describe this functionality. While this word overlaps with the more general "modes" of robot operation (e.g. disabled, periodic, and auto), it's natural to describe the robot as having "several autonomous modes," and "in autonomous mode" (without referring to a specific one) refers to the general robot mode. | ||
|
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 think this confusion is probably a good enough reason for us to come up with another term than modes.
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.
"Routine" was about the only other option I came up with, unless we just want to stick with "OpMode" (which works, but might be confusing during the transition period, which is why I didn't use it).
|
||
A clear enable/disable in the Driver Station that disables all robot actuators when disabled is a requirement because it is a safety-critical feature for FRC due to the size and power of FRC robots. | ||
|
||
The enabled "Init" step in the 2025 FTC SDK/DS is not a requirement due to anticipated rule changes. However, performing initialization of modes while the robot is still disabled is a requirement for both FTC and FRC, as it's important for user code to be able to do expensive mode-specific operations (e.g. computing autonomous paths) prior to actually starting the match. |
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.
Does this just mean that for FTC, you don't have to be able to move actuators or does it mean more than this?
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.
Yes, the main difference compared to current FTC "init" is that you won't be able to move actuators in this state--the robot is still disabled. You can still do other things (read sensors, do computations, etc), just not drive motors.
|
||
- Modes may also be registered via annotation of functions (in the Robot class only) or via explicit function calls. As C++ does not support annotations, function call registration is the only available method in that language. | ||
|
||
- For maximum flexibility, all code in the robot project has access to the enable/disable state, the overall robot teleop/auto/test mode, and the selected modes for teleop, auto, and test (even when the robot is disabled). |
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.
Why is this a good thing? It feels like this could complicate things quite a bit.
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.
This is important for subsystems or other facilities (e.g. logging) that might want to change behavior based on the state of the robot. We don't want to (and can't) hide this from teams, so we should provide a good API for 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.
It might also be worth clarifying that this is read-only access, so the complexity should be minimal. The library will already need to parse this data, so unless I'm missing something, this would only involve writing convenience methods that let teams read that data.
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.
And in general, the library itself will use these same methods (e.g. to implement the mode transition machinery). So it's not any additional work over what we already have to implement.
### RobotBase | ||
|
||
The `RobotBase` class is the base class for the user's `Robot` class. It also implements the private library machinery for robot startup and robot execution (including creating and transitioning between modes in accordance with the mode lifecycle, as described in the following section). | ||
|
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.
This is not what I was thinking at all. I was thinking of each "OpMode" (or whatever it gets named) having a class member that got a singleton instance of the "Robot" class.
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.
That's probably the biggest difference from current FTC opmodes. The Robot instance is constructed first (by main()) and modes are constructed later. Robot is effectively a singleton; it's a single object that persists through all modes that is provided to each mode's constructor (and the mode constructor stores it into a member variable).
Change command-based robots to use different Robot base class. Remove CommandModes and integrate into this base class. Add mode-sugared functions for Subsystem.setDefaultCommand().
I split the command-based design into a separate design document. Based on the initial feedback, I've changed it to be a separate robot base class (so it's not possible to intermix command-based and non-command-based modes). I merged the CommandModes class into this new base class (so it's now less verbose to set up). I also added support/sugar to set per-mode subsystem default commands. I haven't added sugaring for joystick buttons yet, that's an open trade. |
|
||
// returns the current active mode; | ||
// returns "" when the robot is disabled | ||
public static String getActiveMode() {...} |
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.
Since disabledPeriodic from a selected mode runs while disabled should this return that mode?
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.
Interesting idea. We'd probably want to change the DS behavior to do that, but it might be more consistent overall since we still have the enabled bit.
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.
If this is done I'd also change the other getActiveMode methods for consistency.
The `Subsystem` interface adds an overload of `setDefaultCommand` to support creating per-mode default commands, and similarly a new `removeDefaultCommand` overload. The non-`CommandMode` overloads are applied in all modes where a per-mode default has not been set. | ||
|
||
```java | ||
class Subsystem { |
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.
would we want to consider Resource for 2027?
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.
We're still thinking about it. Resource isn't a great name either.
I don't see a way to set the default mode for a specific driverstation mode? Would that be something that could be added ie |
At this point, I'm not sure we need a default. The way this is planned to be implemented at the DS level, its part of the direct control packets, so theres never a case one wouldn't be set. And the DS would save its last value. |
It would be nice to be able to enforce the default irrespective of what the driverstation had last selected. |
|
||
# Unresolved Questions | ||
|
||
- FRC SendableChooser has a "default" option set by robot code. Do we want something similar here or should it be 100% DS driven? It's kind of nice to be able to set a default (e.g. via a `setDefaultAutonomousMode(String)` function in `Robot`), but also might be a little fragile since it's name based. If it's done via annotation, what happens if multiple annotations are marked as default? |
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.
The setting a default question is captured here as an unresolved one. One issue is it's not obvious how it will work with annotations. We'd also definitely have to work through all the same scenarios as dashboard sendable choosers re: when the default actually gets used... is it only when the DS is first started? What about if the DS is an app or a physical device?
Maybe it might be better to have an annotation option to make some options bold/highlighted? Or even allow setting a color code to make it more visually distinct?
We also might want different behavior for match vs non-match mode?
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.
The main difference between sendable choosers and integrated into the DS is that the one integrated into the DS will always be there. So does the default force change the DS? When does it do this? When the DS first starts? Every time code reboots? Every time the robot disconnects? Theres a lot more edge cases here. The only time the DS won't have a selection is the first time a project connects to the DS. Otherwise, the DS will always have something, and I feel like that being changed arbitrarily seems like it'd be more of a footgun then it should be.
Agreed that bolding/highlighting is a good idea.
|
||
# Trades | ||
|
||
- Binding teleop joysticks is very verbose if different behavior is desired in different teleop modes. Maybe add to CommandMode a separate event loop that's only active in that mode? At the minimum, it may make sense to add CommandMode overloads to joysticks so users could write `driverController.x(teleop)` instead of `teleop.running.and(driverController.x())`? |
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.
From a code organization standpoint, declaring bindings for all modes in one place like the Robot constructor seems like it'd get very messy very quickly. Even the simple example above is showing some messiness. I think it's worth looking into better ways of declaring and separating mode-specific bindings; something that I like in Ruby (and which I used for epilogue configuration) is passing a lambda function as the last parameter that will accept the object to configure:
CommandMode teleop = teleoperated("Teleop", mode -> {
control.x().whileTrue(intake.intakeCommand());
// etc...
});
This gives the framework control over when the configuration occurs, instead of it all happening at startup. For example, it means we could wipe all bindings when a mode exits and load all new bindings when a different mode starts by invoking the callback, without needing to worry about old bindings interfering with the new mode. Though that would effectively act as if everything in the lambda had a mode.running.and(...)
wrapping everything, so more thought is definitely needed.
This document describes a standardized approach for operators to select different code to run for different robot modes of operation (auto/teleop/test) and for programmers to easily write code that creates these selection options.
A note on terminology: the word "mode" is used in this document to describe this functionality. While this word overlaps with the more general "modes" of robot operation (e.g. disabled, periodic, and auto), it's natural to describe the robot as having "several autonomous modes" and "in autonomous mode" (without referring to a specific one) refers to the general robot mode.
TODO: