-
Notifications
You must be signed in to change notification settings - Fork 611
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 ability to get list of registered subsystem #6475
Comments
Sounds good. |
Good question, I would have just put a getter for the existing collection in there, which would have made for some interesting bugs when teams started tweaking the returned collection. For the purposes I envision, a read-only would be great. A writable capture would be confusing, and fully backed would be problematic (how to detect and properly respond to changes to the collection?) For performance reasons, I wonder if a read-only copy should be kept and maintained by the scheduler as subsystems are added. I would worry about someone putting a |
If C++ would also be interesting, I'm not versed enough with C++ STL/LLVM collections to know how this can be done properly. |
Is WPILib using C++ 20 yet? If not would we want to upgrade? |
Still needs C++, most likely to be done with https://en.cppreference.com/w/cpp/ranges/keys_view depending on compatibility with `llvm::DenseMap` and C++ 20 support. Resolves wpilibsuite#6475
WPILib enables C++20, but ranges support isn't complete on all compilers. You can use whatever passes CI. |
Still needs C++, most likely to be done with https://en.cppreference.com/w/cpp/ranges/keys_view depending on compatibility with `llvm::DenseMap` and C++ 20 support. Resolves wpilibsuite#6475
Is your feature request related to a problem? Please describe.
We would like the ability to run over the list of registered subsystems to check for annotations and any implementations.
Describe the solution you'd like
Add a CommandScheduler.getAllSubsystems() method.
Describe alternatives you've considered
Hacking in with reflection.
Additional context
I'm willing to take this one on after CMP.
The text was updated successfully, but these errors were encountered: