Skip to content
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

Determine command pattern #8

Open
CS-5 opened this issue Mar 9, 2023 · 5 comments
Open

Determine command pattern #8

CS-5 opened this issue Mar 9, 2023 · 5 comments
Labels
command documentation Improvements or additions to documentation p0 Base functionality / Critical Bug question Further information is requested

Comments

@CS-5
Copy link
Member

CS-5 commented Mar 9, 2023

Need to figure out what makes sense for a general pattern to follow when creating commands:

  • What justifies the use of a standalone command (as opposed to inline commands in RobotContainer)?
  • How should commands be separated?
    • e.g.: "Scoring" command (for arm and claw) vs distinct "Claw" and "Arm" commands
  • Should controller input be handled inside or outside of commands
    • DanceCommand is an example of input being outside of the command
    • ClawToggleCmd is an example of input being inside the command
  • Other ideas?
@CS-5 CS-5 added documentation Improvements or additions to documentation question Further information is requested command labels Mar 9, 2023
@Kieth-the-25th
Copy link
Member

Another thing which relates to this is abstracting the controls a bit so we could change them easily, and figuring out when to use the Trigger class or have conditional stuff.

@CS-5
Copy link
Member Author

CS-5 commented Mar 9, 2023

Agreed, great point! I would lean towards having all controller button/axis mappings in RobotContainer, but there are other ways to accomplish the same thing too.

@CS-5 CS-5 added the p0 Base functionality / Critical Bug label Mar 14, 2023
@Kieth-the-25th
Copy link
Member

Kieth-the-25th commented Mar 15, 2023

@CS-5 So I've spent some time thinking, and I think this is reasonable:

  • Commands with no methods other than execute() and initialize() and no stored types should be inline (because Runnables physically can't store types.)
  • Each command should access only one subsystem and commands shouldn't need to communicate with each other, so we can have all our control structure in one place.
  • Personally, I think it makes more sense for button-mapped commands to handle input outside of the command (like ClawToggleCmd being bound to a Trigger object), and for commands needing joystick input to handle that inside the command (like our drivetrain inline command), since button mappings will probably change more than joystick mappings.
  • Additionally, commands should have different "modes" rather than a separate command for each "mode" (e.g. a separate command for each arm height.) Personally, I think it's just easier to write it that way.

@CS-5
Copy link
Member Author

CS-5 commented Mar 15, 2023

Great ideas, I agree on all points 👍

On your last point, I think there may be cases in very complex robots where splitting code into multiple separate commands might make sense, but I don't believe we're there yet.


We will want to see how autonomous changes things (if at all), but I say lets move forward with the above. Nice work!

@Kieth-the-25th
Copy link
Member

Autonomous will definitely change things, since in autonomous we'll likely have to use multiple subsystems in sync, and a single command is the easiest way of doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command documentation Improvements or additions to documentation p0 Base functionality / Critical Bug question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants