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

Add callback handlers for LiveWindow #2053

Merged
merged 8 commits into from
Jan 13, 2020

Conversation

ThadHouse
Copy link
Member

No description provided.

@sciencewhiz
Copy link
Contributor

Fixes #2223

@sciencewhiz
Copy link
Contributor

I tested with an java old command project and made a command that sets a speed controller to a random number in execute. I added a call to the scheduler run in testPeriodic.

With 2020.1.2, when I went to test mode, the command continued to execute. With this PR, the command stopped executing when going to test mode as expected.

More testing with new commands and C++ to come

@sciencewhiz
Copy link
Contributor

The New Command template calls CommandScheduler.getInstance().cancelAll(); in testInit. Should that be removed in this PR so it doesn't happen twice?

@sciencewhiz
Copy link
Contributor

Tested New Command with Java and didn't have any problems.

@sciencewhiz
Copy link
Contributor

Tested old command with C++ without issues

@sciencewhiz
Copy link
Contributor

@Oblarg any heartburn about removing the cancelAll in the template if it's being done behind the scenes with this PR?

@Oblarg
Copy link
Contributor

Oblarg commented Jan 13, 2020

I think it's slightly better to do it explicitly but I don't feel super strongly about it.

@sciencewhiz
Copy link
Contributor

Tested new command with C++ without issues

@PeterJohnson PeterJohnson merged commit cb66bcc into wpilibsuite:master Jan 13, 2020
pjbuterbaugh pushed a commit to pjbuterbaugh/allwpilib that referenced this pull request Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants