Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cmd] Add andThenWaitUntil, andThenWaitSeconds, andThenWaitTime command decorators #7184

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,58 @@ public ParallelRaceGroup until(BooleanSupplier condition) {
return raceWith(new WaitUntilCommand(condition));
}

/**
* Decorates this command with a condition to wait for after this command is finished. The command will finish after
* this condition is met.
*
* <p>Note: This decorator works by adding this command to a composition. The command the
* decorator was called on cannot be scheduled independently or be added to a different
* composition (namely, decorators), unless it is manually cleared from the list of composed
* commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition
* returned from this method can be further decorated without issue.
*
* @param condition the condition to wait for after this command is finished
* @return the command with the condition to await
*/
public SequentialCommandGroup thenAwait(BooleanSupplier condition) {
return andThen(Commands.waitUntil(condition));
}

/**
* Decorates this command with a timeout to pass for after this command is finished. The command will finish after
* this timeout has passed.
*
* <p>Note: This decorator works by adding this command to a composition. The command the
* decorator was called on cannot be scheduled independently or be added to a different
* composition (namely, decorators), unless it is manually cleared from the list of composed
* commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition
* returned from this method can be further decorated without issue.
*
* @param seconds the timeout duration
* @return the command with the timeout to await
*/
public SequentialCommandGroup thenAwaitTimeout(double seconds) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public SequentialCommandGroup thenAwaitTimeout(double seconds) {
public SequentialCommandGroup thenTimeout(double seconds) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be a little less clear? Because you don't know what the timeout is there for.
But I think my choice of "timeout" is not very good, other suggestions are welcome

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thenWait?

Copy link
Contributor Author

@katzuv katzuv Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think that's better! I'll change it tomorrow. Thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I wonder if it's not too similar to thenAwait.

return andThen(Commands.waitSeconds(seconds));
}

/**
* Decorates this command with a timeout to pass for after this command is finished. The command will finish after
* this timeout has passed.
*
* <p>Note: This decorator works by adding this command to a composition. The command the
* decorator was called on cannot be scheduled independently or be added to a different
* composition (namely, decorators), unless it is manually cleared from the list of composed
* commands with {@link CommandScheduler#removeComposedCommand(Command)}. The command composition
* returned from this method can be further decorated without issue.
*
* @param time the timeout duration
* @return the command with the timeout to await
*/
public SequentialCommandGroup thenAwaitTimeout(Time time) {
return thenAwaitTimeout(time.in(Seconds));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public SequentialCommandGroup thenAwaitTimeout(Time time) {
return thenAwaitTimeout(time.in(Seconds));
public SequentialCommandGroup thenTimeout(Time time) {
return thenTimeout(time.in(Seconds));

}


/**
* Decorates this command with a run condition. If the specified condition becomes false before
* the command finishes normally, the command will be interrupted and un-scheduled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,48 @@ void untilOrderTest() {
}
}

@Test
void thenAwaitTest() {
try (CommandScheduler scheduler = new CommandScheduler()) {
AtomicBoolean finish = new AtomicBoolean();

Command command = new RunCommand(() -> {}).thenAwait(finish::get);

scheduler.schedule(command);
scheduler.run();

assertTrue(scheduler.isScheduled(command));

finish.set(true);
scheduler.run();

assertFalse(scheduler.isScheduled(command));
}
}

@Test
@ResourceLock("timing")
void awaitTimeoutTest() {
HAL.initialize(500, 0);
SimHooks.pauseTiming();
try (CommandScheduler scheduler = new CommandScheduler()) {
Command awaitTimeout = new RunCommand(() -> {}).thenAwaitTimeout(0.1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Command awaitTimeout = new RunCommand(() -> {}).thenAwaitTimeout(0.1);
Command awaitTimeout = new RunCommand(() -> {}).thenTimeout(0.1);


scheduler.schedule(awaitTimeout);
scheduler.run();

assertTrue(scheduler.isScheduled(awaitTimeout));

SimHooks.stepTiming(0.15);
scheduler.run();

assertFalse(scheduler.isScheduled(awaitTimeout));
} finally {
SimHooks.resumeTiming();
}
}


@Test
void onlyWhileTest() {
try (CommandScheduler scheduler = new CommandScheduler()) {
Expand Down
Loading