wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.04k stars 605 forks source link

CommandRobot discussion #6624

Open spacey-sooty opened 2 months ago

spacey-sooty commented 2 months ago

There has been some discussion about how a CommandRobot might be implemented and why. It seems to be a generally positive change to move away from RobotContainer and TimedRobot to a single unified class with clarity about its purpose. From the discussions that I've seen there are still some questions to be answered:

  1. Should it extend IterativeRobotBase or TimedRobot?
  2. How much of IterativeRobotBase should it exposed? (eg. making RobotPeriodic or TeleopPeriodic final)
  3. How much of the RobotContainer structure should be maintained? (eg. still have a configure bindings function?)
UserC2 commented 2 months ago

From how my team (2609) uses RobotContainer and TimedRobot, I think:

  1. TimedRobot, because TimedRobot adds a few methods necessary for a Command-Based robot (e.g. startCompetition()), although these could be moved into IterativeRobotBase.
  2. robotPeriodic() and teleopPeriodic() could be marked final to force teams to use commands (and use addPeriodic() for any functions that should run repeatedly)
  3. RobotContainer has getAutonomousCommand() and configureBindings(), which could both be eliminated as they are called once and never used again: getAutonomousCommand() is called once in autonomousInit(), and configureBindings() is called in the constructor. If teams had a long list of bindings to configure or a long method of choosing their autonomous mode, they could create their own function (no need to require teams to use one).
UserC2 commented 2 days ago

Since RobotContainer's functions aren't required, RobotContainer's other main function (holding subsystems) could be moved to CommandRobot.

spacey-sooty commented 2 days ago

CommandRobot should definitely hold subsystems.