Example Best-Practices of Command-Based usage

[tom131313]

Add an Example Best-Practices of Command-Based usage.
Issue #6616
Reference: CD thread
Utilize an LED-based system to show various best-practices in coding Command-Based programs.

[illinar]

Did you mean to put strip.setData(buffer) in the periodic callback and remove periodic() altogether? strip.start() should be invoked once. Any specific reason to offset by 1ms and run at 100ms?

[illinar]

Not sure why such a verbose name, also I would not include Subsystem suffix since this is no longer a subsystem. How about LEDResource?

[tom131313]

I apologize for leaving some trash from the start and then got distracted trying to get output from the example and getting it into a WPILib Pull Request.

My refactored version is available here. Believe it or not it does resemble the original except I have added working output with commentary and didn't pursue the anticipated new AddressableLED functions which we can add when they are readily available. (Xbox controller buttons A, B, and X do something in different robot modes. Now I see that I should document the expected behavior.)

I can't remember - are we including the word subsystem in the Java class/file name or not? I left in subsystem but scrubbed Command and Trigger from names.)

I would appreciate your and @Oblarg comments on what can be done better as a best practice. Thanks.

[tom131313]

@illinar I have completed a working example LED resource/subsystem/view based about 99% on your example code.

I did find @SamCarlberg nice addition to the LED process that you recommended. I copied his entire work into my example since I don't see any other way to reference it in my code. All those files would have to be deleted from my example and appropriate imports made to the WPILib when it has those files available.

My changes to your code are trivial to make it a little easier to test. I simulated a couple of events with Xbox controller button presses and changed an onTrue to whileTrue. I added a third view of the LEDs only to display your dynamic Enable/Disable Patterns.

Since the basis of all LED addressable views is the same I have only one class for the LEDview and instantiate it three times for the Top, Main, and EnableDisable views.

I welcome comments from all - you, @Oblarg and anyone to further refine structure.

Thanks for your contribution of a fine example.

[illinar]

I feel like this should be part of WPILIB API surface if this pattern is an officially blessed mechanism of injecting periodic processing support.

[calcmogul]

if this pattern is an officially blessed mechanism of injecting periodic processing support

It's not. The official mechanism is TimedRobot.addPeriodic().

[tom131313]

I've taken this program with a few examples of using command-based as far as I can - it accomplishes all I intended. I can't go any further without WPILib reviewer input. If these examples are of value to WPILib, I'd be happy to recode whatever needs to make it a real WPILib example. The name of the folder should be changed at least to remove the "word" led from it as that is misleading and off-putting. I used LEDs in place of the PWM motor controllers or other devices merely to show actions of the examples.

If these examples aren't what is wanted, I'll close the PR and delete the repository from my account.

[Gold856]

The overall structure of this example confuses me a lot. A big overarching theme is that subsystems are used by other subsystems. This feels contradictory to the entire point of the Command-based framework. Why are LEDViews, which are also subsystems, created in RobotSignals, then passed to each subsystem? Why does GroupDisjointTest contain multiple instances of GroupDisjoint? What is GroupDisjointTest trying to show me? There are so many commands, and I'm not sure what they're doing or what it's supposed to demonstrate.

The MooreLikeFSM subsystem also confuses me. It's contradictory to HistoryFSM where it says that commands should not schedule other commands, yet that's what MooreLikeFSM does. MooreLikeFSM also seems like it would be better represented as a giant sequential command since it seems the entire structure is just running commands one after another.

[spacey-sooty]

I agree with all of @Gold856 comments but I also don't see the benefit in adding a new example to show command based best practices. The RapidReactBot example has been used as the up to date and best practice example up until this point in time and I don't see why we should change that. If theres anything that that does incorrectly that should be updated instead of creating a new example. We already have issues keeping current examples up to date, why make that problem worse?

[tom131313]

These examples of Command-Based programming are intended to illustrate acceptable practices that tend to push usage to all the edges of somewhat unusual usage. Thus, the comments are all valid in that there are multiple examples to sort through and they don't seem to illustrate the normal best practice. Since confusion is anticipated and that is counter to the intention, this project will be dropped and closed.