strongback / strongback-java

A library for FIRST Robotics Competition robots that makes it easier to write and test your robot code.
MIT License
41 stars 38 forks source link

Issue 81: Fixed CommandGroup behavior inconsistency #82

Closed rothso closed 7 years ago

rothso commented 7 years ago

Proposal to fix #81.

Subclassed CommandGroups are now indistinguishable from those created with the static helpers. This lets the Scheduler skip the getRoot step and forward the submitted command to the builder.

After playing around with this, I see why root is needed now. I decided the least invasive solution would be to remove getRoot (assuming it's not part of the public API) and keep the notion of root internal. Then the getCommands and getType methods would delegate to the root object if there is one.

Let me know what you think about this approach!

rhauch commented 7 years ago

@rothso on the surface this looks good, and I do appreciate trying to be least invasive -- this code is finicky. I see that the unit tests have passed, but have you tried running these changes with a robot?

rhauch commented 7 years ago

@rothso Oh, it'd also be great if you added your name in the CONTRIBUTORS.txt file.

rothso commented 7 years ago

Unfortunately I haven't had an opportunity to deploy to our bot yet—my next chance is this Sunday, so I'll be able to tell you how it goes then.

And alright, will do.