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

Refactor TimedRobot, add support for one-shot actions and cancelling … #7315

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

emintz
Copy link

@emintz emintz commented Oct 30, 2024

…actions, along with support for actions that cancel themselves. Changes include:

Implement TimedRobot.Cancellable, a functional interface invoked to cancel a scheduled callback.

Implement an enum represention of the schedule status where each enumeration contains a callback handler.

Implement TimedRobot.Callback.invoke() to handle Runnable invocation on behalf of the TimedRobot, where the method returns true if and only if the callback should be invoked again. Have invoke() delegate callback invocation to the callback's status. Replace Callback.func.run() invocations with Callback.invoke(), and reschedule the callback if and only if invoke() returns true.

Enhance TimedRobot.Callback to maintain its schedule status and enhance the addPeriodic() methods to set the scheduling status to PERIODIC (i.e. indefinite periodic invocation) and to return a Cancellable (the Callback that holds the user-provided Runnable) so that applications can cancel invocation at will. Base the enhanced class on Cancellable and implement cancel(). The returned Cancellable is actually the Callable that holds the user's Runnable.

Add TimedRobot.addOneShot(), which schedules a Runnable to be invoked exactly once after a set delay. Like its addPeriodic() counterparts, addOneShot() returns a Cancellable. The enhanced TimedRobot currently provides one addOneShot version that takes a timeout in seconds. Please let me know if users will need to specify delays as a Time.

Simplify the loop in TimedRobot.startCompetition() and enhance it to reschedule a callback when and only when its callback() invocation returns true.

Note that the status enumerations, PERIODIC, ONE_SHOT, and CANCELLED each provide an invoke() method that accepts a Runnable and "does the right thing." The enumerations have the following characteristics

| Name | Action | Runs Callback | Returns |
+-----------+---------------------- +---------------+---------+
| PERIODIC | Invoke indefinitately | Yes | true |
+-----------+---------------------- +---------------+---------+
| ONE_SHOT | Invoke exactly once | Yes | false |
+-----------+---------------------- +---------------+---------+
| CANCELLED | Cancel invocation | No | false |
+-----------+---------------------- +---------------+---------+

For example, ONE_SHOT.invoke(Runnable callback) invokes callback.run(), but returns false so it will not be rescheduled. The other implications work similarly.

…actions, along with support for actions that cancel themselves. Changes include:

Implement TimedRobot.Cancellable, a functional interface invoked to cancel a scheduled callback.

Implement an enum represention of the schedule status where each enumeration contains a callback handler.

Implement TimedRobot.Callback.invoke() to handle Runnable invocation on behalf of the TimedRobot, where the method returns true if and only if the callback should be invoked again. Have invoke() delegate callback invocation to the callback's status. Replace Callback.func.run() invocations with Callback.invoke(), and reschedule the callback if and only if invoke() returns true.

Enhance TimedRobot.Callback to maintain its schedule status and enhance the addPeriodic() methods to set the scheduling status to PERIODIC (i.e. indefinite periodic invocation) and to return a Cancellable (the Callback that holds the user-provided Runnable) so that applications can cancel invocation at will. Base the enhanced class on Cancellable and implement cancel(). The returned Cancellable is actually the Callable that holds the user's Runnable.

Add TimedRobot.addOneShot(), which schedules a Runnable to be invoked exactly once after a set delay. Like its addPeriodic() counterparts, addOneShot() returns a Cancellable. The enhanced TimedRobot currently provides one addOneShot version that takes a timeout in seconds. Please let me know if users will need to specify delays as a Time.

Simplify the loop in TimedRobot.startCompetition() and enhance it to reschedule a callback when and only when its callback() invocation returns true.

Note that the status enumerations, PERIODIC, ONE_SHOT, and CANCELLED each provide an invoke() method that accepts a Runnable and "does the right thing." The enumerations have the following characteristics

| Name      | Action                | Runs Callback | Returns |
+-----------+---------------------- +---------------+---------+
| PERIODIC  | Invoke indefinitately | Yes           | true    |
+-----------+---------------------- +---------------+---------+
| ONE_SHOT  | Invoke exactly once   | Yes           | false   |
+-----------+---------------------- +---------------+---------+
| CANCELLED | Cancel invocation     | No            | false   |
+-----------+---------------------- +---------------+---------+

For example, ONE_SHOT.invoke(Runnable callback) invokes callback.run(), but returns false so it will not be rescheduled. The other implications work similarly.
@emintz emintz requested a review from a team as a code owner October 30, 2024 21:16
@KangarooKoala
Copy link
Contributor

What's the intended use case? (What's a specific situation in robot code where this would be useful?)

@emintz
Copy link
Author

emintz commented Oct 31, 2024 via email

…actions, along with support for actions that cancel themselves. Changes include:

Implement TimedRobot.Cancellable, a functional interface invoked to cancel a scheduled callback.

Implement an enum represention of the schedule status where each enumeration contains a callback handler.

Implement TimedRobot.Callback.invoke() to handle Runnable invocation on behalf of the TimedRobot, where the method returns true if and only if the callback should be invoked again. Have invoke() delegate callback invocation to the callback's status. Replace Callback.func.run() invocations with Callback.invoke(), and reschedule the callback if and only if invoke() returns true.

Enhance TimedRobot.Callback to maintain its schedule status and enhance the addPeriodic() methods to set the scheduling status to PERIODIC (i.e. indefinite periodic invocation) and to return a Cancellable (the Callback that holds the user-provided Runnable) so that applications can cancel invocation at will. Base the enhanced class on Cancellable and implement cancel(). The returned Cancellable is actually the Callable that holds the user's Runnable.

Add TimedRobot.addOneShot(), which schedules a Runnable to be invoked exactly once after a set delay. Like its addPeriodic() counterparts, addOneShot() returns a Cancellable. The enhanced TimedRobot currently provides one addOneShot version that takes a timeout in seconds. Please let me know if users will need to specify delays as a Time.

Simplify the loop in TimedRobot.startCompetition() and enhance it to reschedule a callback when and only when its callback() invocation returns true.

Note that the status enumerations, PERIODIC, ONE_SHOT, and CANCELLED each provide an invoke() method that accepts a Runnable and "does the right thing." The enumerations have the following characteristics

| Name      | Action                | Runs Callback | Returns |
+-----------+---------------------- +---------------+---------+
| PERIODIC  | Invoke indefinitately | Yes           | true    |
+-----------+---------------------- +---------------+---------+
| ONE_SHOT  | Invoke exactly once   | Yes           | false   |
+-----------+---------------------- +---------------+---------+
| CANCELLED | Cancel invocation     | No            | false   |
+-----------+---------------------- +---------------+---------+

For example, ONE_SHOT.invoke(Runnable callback) invokes callback.run(), but returns false so it will not be rescheduled. The other implications work similarly.

Also fix lint errors.
@emintz
Copy link
Author

emintz commented Oct 31, 2024 via email

@KangarooKoala
Copy link
Contributor

Thanks for the explanation! Those are great examples. However, those can all be implemented via the command based framework. (For example, to stop a motor after 1.5 seconds, use runMotor().withTimeout(1.5).andThen(stopMotor()) where runMotor() and stopMotor() return commands) Any reason to add another way to do these sorts of tasks?

@emintz
Copy link
Author

emintz commented Oct 31, 2024 via email

@KangarooKoala
Copy link
Contributor

However, there's still equipment protection. Suppose the robot has an elevator controlled by limit switches that is supposed to raise in, say, 1.5 seconds. I can schedule a protective action that turns off the motor to run in two seconds and cancel it when the upper limit switch closes. When the operation succeeds, the protective action does not run. When the operation fails and the limit switch doesn't close soon enough, the action runs and stops the elevator motor. Is there already a way to accomplish this?

Usually, you can always run the protective action after the command is done, whether or not it succeeded or was canceled. In that case, you would just run the protective action inside .end() of the command running the operation. (For example, instead of using run(() -> setMotorSpeed()), you would use runEnd(() -> setMotorSpeed(), () -> stopMotor())) Then, if you add a timeout (use command.withTimeout(2.0) instead of just command), it'll also run the protective action.

There are a few exceptions that don't fit that category, but they're a bit harder to explain so I won't do so now. If you do have a case like that, though, feel free to send a link to the code or describe it and we can discuss other solutions!

How do you see the self cancelling periodic actions? Do the examples make sense?

I think the examples show some great use cases, but the commands library was made for those very cases, so I don't think it's worth the complexity (which includes more stuff to update when we make changes).

Incidentally, please let me know if I can be working on a higher priority issue. I'd be happy to set this aside.

Thanks for contributing! I don't know if there are higher priority issues that would be good for you to start on (Full disclosure- I'm not one of the main developers, I just make some contributions when I have free time), but issues labeled "good first issue" are a good place to get started and then you can see what sort of stuff is going on and see what you'd like to work on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants