-
Notifications
You must be signed in to change notification settings - Fork 9
Refactor future #80
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 future #80
Conversation
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
|
I think we could actually use https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html instead of our own implementation of a Future. |
jacobperron
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one suggestion.
I guess we should update the examples repo too: https://github.com/osrf/ros2_java_examples
rcljava/src/main/java/org/ros2/rcljava/concurrent/RCLFuture.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Don't use
spin()inRCLFuture.get().The problem with that is that you cannot currently call
spin()more than once, so that model is really limited.This uses a model more similar to the one in
rclcpp.You can block on a future while spinning in another thread, or use
Executor.spinUntilComplete().That model is still somewhat limited, but a bit better.
Note: this may break prexisting code that were calling
Future.get()without spinning.