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

Subsystem Disable Option #5549

Closed
wants to merge 3 commits into from
Closed

Conversation

mebrahimaleem
Copy link

Implements being able to disable a subsystem, meaning that both the default command and periodic function will not execute while the subsystem is disabled.

@mebrahimaleem mebrahimaleem requested a review from a team as a code owner August 18, 2023 00:12
@mebrahimaleem
Copy link
Author

Resolves #4061

@Starlight220
Copy link
Member

Feels like this adds unneeded complexity for a rather niche use case. What significant benefit does this add over simply calling unregisterSubsystem or completely not initializing the subsystem?

@mebrahimaleem
Copy link
Author

Unlike not initializing the subsystem, this implementation allows disabling both the periodic and the default command while still being able to call other methods implemented by teams, often used to get information about the robot's current state. Unlike calling the unregisterSubsystem method, this implementation also allows restoring the default command upon re-enable.

I do understand that this implementation is rather complex. Do you have any suggestions on what parts of the code should be simplified/removed?

@Starlight220
Copy link
Member

while still being able to call other methods implemented by teams, often used to get information about the robot's current state

This is opposite to the feature request that all calls to the subsystem are blocked without erroring.

Regardless of the feature request, what benefit do these changes bring? What use cases does it help?

@rzblue rzblue added the component: command-based WPILib Command Based Library label Sep 18, 2023
@calcmogul
Copy link
Member

Abandoned by author.

@calcmogul calcmogul closed this Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: command-based WPILib Command Based Library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants