allwpilib
allwpilib copied to clipboard
Add ability to get list of registered subsystem
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.
Sounds good. One point for question: should the returned collection be read-only, a capture, or fully backed and writeable?
Sounds good. One point for question: should the returned collection be read-only, a capture, or fully backed and writeable?
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 for (var subsystem : CommandScheduler.getAllSubsystems())
in a periodic. Is that a legit concern?
If ImmutableSet(m_subsystems.keySet())
still reads from the backing set (rather than a one-time copy), that can be a simple solution.
EDIT: Collections.unmodifiableSet
looks like exactly what we need.
C++ would also be interesting, I'm not versed enough with C++ STL/LLVM collections to know how this can be done properly.
EDIT: C++20 has https://en.cppreference.com/w/cpp/ranges/keys_view, that might do what we need if it's compatible with llvm::DenseMap
C++ would also be interesting, I'm not versed enough with C++ STL/LLVM collections to know how this can be done properly. EDIT: C++20 has https://en.cppreference.com/w/cpp/ranges/keys_view, that might do what we need if it's compatible with
llvm::DenseMap
Is WPILib using C++ 20 yet? If not would we want to upgrade?
WPILib enables C++20, but ranges support isn't complete on all compilers. You can use whatever passes CI.