[cmd] whileTrueLowPriority on trigger

[oh-yes-0-fps]

I want some input on this before I port it to C++.

Specifically how do people feel about the Command Trigger factories and whats a better name for whileTrueLowPriority (maybe whileTrueDefault?).

[github-actions]

This PR modifies commands. Please open a corresponding PR in Python Commands and include a link to this PR.

[SamCarlberg]

These seem footgun-y to me. Composed commands are invisible to the scheduler, so their isScheduled method calls will always return false:

var nextCommand = subsystem.run(...);

controller.a().onTrue(subsystem.someCommand().andThen(nextCommand));

// The scheduler does not see this command due to its inclusion in a composition
// This trigger can never fire
nextCommand.done().onTrue(...);

They are also specific to the command instances on which they're invoked, so something like this would never work:

controller.a().onTrue(subsystem.someCommand())

// New instance from someCommand() that never gets scheduled
// This trigger can never fire
subsystem.someCommand().done().onTrue(subsystem.someOtherCommand());

[Starlight220]

I think that whileTrueLowPriority is a bit too complex/niche and could be solved by a Subsystem.idle trigger that would be more useful to more use cases.
A few years ago I tried reimplementing default commands that way and ran into the problem of triggers not triggering on the initial state.

As for the triggers on Command, I'm not sure how useful they'd be? Triggering on end can be done via finallyDo/andThen

[SamCarlberg]

A different approach would be to implement command priority levels and suspend-on-interrupt behavior (both of which could be backported from coroutine commands)

https://github.com/SamCarlberg/allwpilib/blob/better-async-commands/commandsv3/src/main/java/edu/wpi/first/wpilibj/commandsv3/Command.java#L66-L78

https://github.com/SamCarlberg/allwpilib/blob/better-async-commands/commandsv3/src/main/java/edu/wpi/first/wpilibj/commandsv3/Command.java#L162-L167

[oh-yes-0-fps]

I think that whileTrueLowPriority is a bit too complex/niche

This is something that's being "upstreamed" from the new choreolib rewrite.
You can find examples here, the loop.enabled().whileTrueDefault(spinnup()); is the usage example. I'm not opposed to Subsystem.idle() trigger aswell but something like this would be very helpful for these trigger based autos.

[KangarooKoala]

If a Subsystem.idle() trigger factory is added, then the logic could be implemented as condition.and(mySubsystem.idle()).whileTrue(command.repeatedly()), which reads well and is explicit about how it's implemented, so user's can better understand any potential weirdness. Up to other people whether that sort of thing would be worth adding syntax sugar for.

[oh-yes-0-fps]

Unfortunately condition.and(mySubsystem.idle()).whileTrue(command.repeatedly()) does not have the same functionality.
command.repeatedly() acts like a sequential command with an infinite length of the same element.
Once it is canceled it won't reschedule itself and whileTrue does not reschedule an ended command either.
Another thing is whileTrueLowPriority will not interrupt other commands besides the default command when scheduled. nvm the idle can fix allat

[oh-yes-0-fps]

So the subsystem.idle() has one slight problem, if it's used to schedule a command it thus makes itself false.

  /**
   * Returns a {@link Trigger} that is true when this subsystem is
   * currently being required by no commands or being required by the default command.
   * 
   * This can be used to create a control flow similar to a temporary default command
   * <pre><code>
   * {
   *  DriveSubsystem drive = ...;
   *  Command myCommand = ...;
   *  Trigger otherCondition = ...;
   *  
   *  otherCondition.and(drive.idle(loop)).whileTrue(myCommand.repeatedly());
   *}</code></pre><p>
   * 
   * @param loop the event loop to poll this trigger on
   * @return a Trigger that is true when this subsystem is not being required by any commands other than the default command
   */
  default Trigger idle(EventLoop loop) {
    return new Trigger(loop, () -> {
      Command currentCommand = getCurrentCommand();
      return currentCommand == null || currentCommand == getDefaultCommand();
    });
  }

This was my implementation

A solution to this is to do otherCondition.and(drive.idle(loop)).onTrue(myCommand.repeatedly().onlyWhile(otherCondition)); but that is incredibly verbose so we end up back into the syntactic sugar realm

[oh-yes-0-fps]

Maybe adding a

//inside Subsystem.java
default void defaultCommandWhile(BooleanSupplier condition, Command cmd) {
    Trigger condTrigger = new Trigger(condTrigger );
    condTrigger.and(this.idle()).onTrue(cmd.repeatedly().onlyWhile(condTrigger).withName(cmd.getName()));
}

would be a sufficient abstraction