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 command.repeatdly(int count) to both java and c++ #6589
base: main
Are you sure you want to change the base?
Conversation
Also has unit tests in it!
In a single commit so it can be easily reverted when wanted
a diffrent pr This reverts commit 1bb8f4b.
* @return the decorated command | ||
*/ | ||
public ParallelRaceGroup repeatedly(int times) { | ||
int[] counter = {0}; |
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'm familiar with this idiom, but we use AtomicInteger for by-ref integers everywhere else - so this should too.
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 looks good just a few comments. Dunno why C++ would print stuff out especially since Java doesn't, along with this for printing in C++ its generally better to use fmt::print()
wpilibNewCommands/src/main/native/cpp/frc2/command/CommandScheduler.cpp
Outdated
Show resolved
Hide resolved
wpilibNewCommands/src/main/native/cpp/frc2/command/CommandPtr.cpp
Outdated
Show resolved
Hide resolved
printf("Command %s is finished\n", command->GetName().c_str()); | ||
m_impl->endingCommands.insert(command); | ||
printf("Ending command %s\n", command->GetName().c_str()); | ||
command->End(false); | ||
printf("Command %s has ended\n", command->GetName().c_str()); |
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 print stuff out?
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 is in the PR's message, that if you were to look at the unit tests, the C++ code doesn't function like the test expects it to do so. This is why I have added these print statements as a separate commit. They will be reverted before the merge
/format |
This was stated in Discord, but tests are probably failing because of an inconsistent order of when the commands in the internal deadline group are checked, which should be fixed by #6602. |
Currently, the unit tests fail and there are a lot of print statements for whoever might be checking into this.
The problem is. The exact same function calls create different results between C++ and Java. I may just be the idiot but it seems to require further look into
The debug's are in different commits making it a lot easier to revert them if needed.