That’s pretty awesome then. I just now noticed you’ve been messing with the experimental package and not the current command package.
OK, I have a few suggestions that are mostly just my personal opinion.
I’d really like Commands to be unit testable. The current scheduler can’t be initialized unless you’re doing a simulation or are testing on a roboRIO.
I like immutability. I know that the current Sendable interface has a setName method, which I wish could somehow be phased out so no one is tempted to change the name of something which should never really be changed. I think that the requirements for Subsystems on a SendableCommandBase could be passed through the constructor so they’re never changed as well.
While I like the use of most things as an interface, they act a little too much like abstract classes because of all the default methods. I understand that you could just override everything that you want to change, but I really don’t feel like that’s what default methods were made for.
Now for SendableCommandBase. A lot of the changes you’ve made make the library better, but by implementing Command and Sendable, you make the class do two different things (Just like current Command class). When I create classes, I try to make them do one thing and one thing only. This year, instead of directly implementing Sendable or extending SendableBase, I created my own Sendable classes which took my subsystems as arguments through the constructor. My point being yes, implementing Sendable makes it easier on people who want their command to be a Sendable, but for people that might want to separate that code a little more, it encourages that code to be piled into one class, potentially making it more messy than if people had an option on whether or not they want it to extend Sendable.
Solution: Make SendableCommandBase extend CommandBase or come up with another way so people aren’t forced to inherit a class that also inherits Sendable.
ConditionalCommand is a cool idea, but why not just have something like DynamicCommand which has a
Supplier<Comamnd> that will use whatever that returns. It gives the caller more options too because they aren’t just limited to two commands.
Now, for even more personal preference. In my action-lib library, my Actions class just had a bunch of factory methods. This had an advantage when I wanted to change how my classes worked as the factory methods could easily adapt to whatever default arguments constructors now needed. Take my DynamicCommand idea. Yes, you could do
new DynamicCommand(myCommandSupplier), but a factory method would be better as there’s really no point in the caller knowing that the Command is a DynamicCommand. Plus, factory method names can sometimes be more descriptive.
Anyway, just my personal opinion on some things. I’m guessing not everything I said above makes everything better, but you can take it or leave it.