[cmd3] Add rising and falling edge trigger factories #8366
Conversation
SamCarlberg
added
2027
| // Ensure bindings. Triggers aren't polled when no bindings have been added. | ||
| baseTrigger.addBinding(BindingScope.global(), BindingType.RUN_WHILE_HIGH, command); | ||
| risingEdgeTrigger.addBinding(BindingScope.global(), BindingType.RUN_WHILE_HIGH, command); |
There was a problem hiding this comment.
It feels like a major footgun that base triggers need a binding in order for any derived triggers to work. At least from what I've seen in commandsv2, it's not unheard of for triggers to be composed from temporary objects, and silently always using false is those cases would probably cause a lot of confusion and frustration.
One possibility would be for Triggers to track their base trigger(s), so that when a binding is added to a Trigger, it can first make sure all of its base trigger(s) (and their base(s) and so on) are added to the event loop.
Another, simpler but less user-friendly approach is for getAsBoolean() to give a descriptive error or warning if the trigger has not been added to an event loop.
Only binding when a command binding is added would have resulted in intermediate edge triggers never firing
|
When we made |
This allows intermediate unbound triggers to still function
KangarooKoala
left a comment
KangarooKoala
left a comment
There was a problem hiding this comment.
Generally seems okay to me (someone who has not really interacted with commandsv3, just commandsv2 and BooleanEvent), for what it's worth. Hopefully someone else could also review?
| * from {@code getAsBoolean()} may not agree with the result of checking the signal directly. | ||
| * | ||
| * <p>Triggers are only bound to an event loop when at least one command has been bound to the | ||
| * trigger. This method will always return {@code false} before commands have been bound. |
There was a problem hiding this comment.
Maybe adjust this to reflect that commands being bound to dependencies will also bind the Trigger to an event loop?
2 participants
Trigger.getAsBoolean()behavior has been changed from passing through the underlying boolean supplier to returning the latest cached signal as determined by the most recent call topoll(). This allows rising and falling edge triggers to have a consistent return value over an entire polling cycle, rather than only being high for the first check in a cycle.Closes #8309