String tags for commands for easier command identification about allwpilib HOT 8 OPEN

I feel like the use cases can be implemented using the name field, without needing to tack on another metadata field

from allwpilib.

Also this likely wouldn't fix the issue of compositions without the same issue being fixed for normal names

from allwpilib.

I feel like the use cases can be implemented using the name field, without needing to tack on another metadata field

Yes, this is true, however the issue comes when commands are composed. When commands are composed they lose their name (and thus the data stored in the name). Because tags would be inherited by the composing command, the data isn't lost.

Also this likely wouldn't fix the issue of compositions without the same issue being fixed for normal names

I not sure how you would fix the issue of compositions with normal names. Lets take the example of sequencing 3 commands:

Commands.sequence(command1, command2, command3);

What would the name of the composed command be? Currently it's just "SequencedCommandGroup", which doesn't provide any unique data. If the suggested name is "[command1.getName()], [command2.getName()], [command3.getName()]", this is far from readable (the main purpose of command names).
With tags, the name of the composed command could be anything (e.g. "Quick Auto", "Auto Drive To Amp", etc.), while still maintaining the unique data contained in the tags. The tags would also be able to be common across multiple commands (e.g. "Intake Deployed" could be common for the intake commands "Intake", and "Eject").

from allwpilib.

Why can’t you just decorate using .withName()? Eg Commands.sequence(a,b,c).withName(“quick auto”)?

from allwpilib.

Why can’t you just decorate using .withName()? Eg Commands.sequence(a,b,c).withName(“quick auto”)?

You can, but it still has lost the data from the commands inside.

from allwpilib.

Here's a real case where these tags would be useful. In our (Team 686) code, we have a shooter subsystem (only controls the flywheels) and a kicker subsystem (rollers that push the note into our flywheels). Lets say that we want to implement an auto-shoot trigger that will schedule Kicker.kick() when the flywheels are at speed. However, our shooter is also responsible for placing the note in the amp and has a separate command for that case. When we want to amp we don't want this auto-shoot trigger to fire (because we might still be trying to align to the amp). If we have tags then the trigger is as simple as this:

new Trigger(shooter::atDesiredSpeed).and(() -> shooter.getCurrentCommand().hasTags(Shooter.AUTO_SHOOT_TAG)).onTrue(kicker.kick());

In our shooter commands, we would have only the commands that are attempting to fire at the speaker have the auto-shoot tag.
The benefit of over other ways is that if we compose the shooter command with other commands (e.g. shooter.autoAim().alongWith(pivot.autoAim(), drive.autoAim())) the composed command still retains the Shooter.AUTO_SHOOT_TAG data and the trigger still works (without having to remember to make sure the parallel command name contains some arbitrary string or to wrap shooter.autoAim() in a proxy).

from allwpilib.

I imagine other will say that basing triggers not off the state of the system but off the state of the command schedule is an anti-pattern.

Why not have

new Trigger(shooter::preppedForShot).onTrue(kicker.kick());

and preppedForShot handles checking for the right state

from allwpilib.

I imagine other will say that basing triggers not off the state of the system but off the state of the command schedule is an anti-pattern.

Is the command not the state of the system? From my point of view the command scheduler is a beefed-up state machine without the convenience of easy state checking.

Why not have ... and preppedForShot handles checking for the right state

How would preppedForShot do this without checking the name of the command nor checking toggle flags (that the commands have to remember to set and reset).

I have realized a potential issue with tags where a sequenced command will have the tags of commands that might not be currently running, which might cause undesired behavior.

from allwpilib.