Add action server with logic for accepting and canceling goals #53
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jacobperron
changed the base branch from
galactic-devel
to
jacob/more_action_interfaces
ivanpauno
reviewed
Nov 16, 2020
Collaborator
ivanpauno
left a comment
ivanpauno
left a comment
There was a problem hiding this comment.
Partial review, I have only some minor comments up to now
rcljava/src/main/cpp/org_ros2_rcljava_action_ActionServerImpl.cpp
Outdated
Show resolved
Hide resolved
rcljava/src/main/cpp/org_ros2_rcljava_action_ActionServerImpl_GoalHandleImpl.cpp
Show resolved
Hide resolved
ivanpauno
reviewed
Jan 5, 2021
Collaborator
ivanpauno
left a comment
ivanpauno
left a comment
There was a problem hiding this comment.
Ok, I have finally reviewed most of this ...
I still have to check the native code more closely, and I haven't still looked at the executor code and tests.
rcljava/src/main/java/org/ros2/rcljava/action/ActionServerGoalHandle.java
Show resolved
Hide resolved
rcljava/src/main/java/org/ros2/rcljava/action/ActionServerGoalHandle.java
Show resolved
Hide resolved
rcljava/src/main/java/org/ros2/rcljava/action/ActionServerImpl.java
Outdated
Show resolved
Hide resolved
rcljava/src/main/java/org/ros2/rcljava/action/ActionServerImpl.java
Outdated
Show resolved
Hide resolved
jacobperron
force-pushed
the
jacob/action_server
branch
from
0230bb2 to
21b194a
Compare
ivanpauno
approved these changes
Jan 18, 2021
Collaborator
ivanpauno
left a comment
ivanpauno
left a comment
There was a problem hiding this comment.
The code looks good to me!
I don't know if there is something else pending before merging
Implementing these interfaces in the code generation template makes it easier to pass around these types in a generic way. Note, the 'final' modifier had to be removed from generated message types in order to extend goal, result, and feedback types in action definitions. Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Also make inner classes static. Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Partially revert commit dd04614. I don't think we need to aliases for the message types, but I'll add them back if they turn out to be useful.
Signed-off-by: Jacob Perron <[email protected]>
jacobperron
force-pushed
the
jacob/more_action_interfaces
branch
from
4924965 to
f7d7f55
Compare
It should return a List, since it is hashable. Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
* Add new action module with classes/interfaces related to the ActionServer * Add methods for creating and removing ActionServers from a Node * Implement dispose method for ActionServer. Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
This changeset includes the following: * an implementation of the goal handle for action servers * action server integration with the base executor * action server integration with ROS nodes * unit tests for the action server, receiving goal and cancel requests Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
The goal request already contains a getter for the goal ID. Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
…erOfEntites()' function Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Replace with 'namespace rcljava' Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
jacobperron
force-pushed
the
jacob/action_server
branch
from
6fe23e6 to
f94150b
Compare
Author
|
@ivanpauno In addition to addressing your feedback, I've made some patches for the change from Lists to arrays (f94150b). |
Base automatically changed from
jacob/more_action_interfaces
to
galactic-devel
January 20, 2021 21:02
ivanpauno
approved these changes
Jan 21, 2021
Collaborator
ivanpauno
left a comment
ivanpauno
left a comment
There was a problem hiding this comment.
LGTM!
If there are any details missing we can figure it out when this is completed and a tutorial is written.
ivanpauno
pushed a commit
that referenced
this pull request
May 17, 2021
* Add interfaces for action goal, result, and feedback Implementing these interfaces in the code generation template makes it easier to pass around these types in a generic way. Note, the 'final' modifier had to be removed from generated message types in order to extend goal, result, and feedback types in action definitions. Signed-off-by: Jacob Perron <[email protected]> * Add new definitions for action goal response and request Signed-off-by: Jacob Perron <[email protected]> * Add getter for UUID to SendGoalRequest Also make inner classes static. Signed-off-by: Jacob Perron <[email protected]> * Add getStamp method to GoalResponseDefinition Signed-off-by: Jacob Perron <[email protected]> * Partially revert "Add interfaces for action goal, result, and feedback" Partially revert commit dd04614. I don't think we need to aliases for the message types, but I'll add them back if they turn out to be useful. * Parameterize goal request and response interfaces on action type Signed-off-by: Jacob Perron <[email protected]> * Fix getGoalUuid implementation It should return a List, since it is hashable. Signed-off-by: Jacob Perron <[email protected]> * List<Byte> -> byte[] Signed-off-by: Jacob Perron <[email protected]> * Add ActionServer skeleton * Add new action module with classes/interfaces related to the ActionServer * Add methods for creating and removing ActionServers from a Node * Implement dispose method for ActionServer. Signed-off-by: Jacob Perron <[email protected]> * Add ActionServer creation logic and unit tests Signed-off-by: Jacob Perron <[email protected]> * Add action server logic for accepting and canceling goals This changeset includes the following: * an implementation of the goal handle for action servers * action server integration with the base executor * action server integration with ROS nodes * unit tests for the action server, receiving goal and cancel requests Signed-off-by: Jacob Perron <[email protected]> * cleanup Signed-off-by: Jacob Perron <[email protected]> * more cleanup Signed-off-by: Jacob Perron <[email protected]> * Minor refactor: move response enums into callback interfaces Signed-off-by: Jacob Perron <[email protected]> * Remove TODO The goal request already contains a getter for the goal ID. Signed-off-by: Jacob Perron <[email protected]> * Fix accident Signed-off-by: Jacob Perron <[email protected]> * Alphabetize Signed-off-by: Jacob Perron <[email protected]> * Refactor: replace 'getNumberOf*()' JNI functions with single 'getNumberOfEntites()' function Signed-off-by: Jacob Perron <[email protected]> * Avoid string copy Signed-off-by: Jacob Perron <[email protected]> * Remove extern C Replace with 'namespace rcljava' Signed-off-by: Jacob Perron <[email protected]> * Remove unnecessary synchronization Signed-off-by: Jacob Perron <[email protected]> * Refactor access to goalCallback Signed-off-by: Jacob Perron <[email protected]> * typesafe request and response definitions Signed-off-by: Jacob Perron <[email protected]> * Be more specific about template type for GoalCallback Signed-off-by: Jacob Perron <[email protected]> * Minor refactor Signed-off-by: Jacob Perron <[email protected]> * Make GoalHandleImpl inner class instead of static Signed-off-by: Jacob Perron <[email protected]> * Remove synchronized from getHandle Signed-off-by: Jacob Perron <[email protected]> * Add TODO for Waitable interface Signed-off-by: Jacob Perron <[email protected]> * Fix style Signed-off-by: Jacob Perron <[email protected]> * Minor refactor Signed-off-by: Jacob Perron <[email protected]> * Add docs for createActionServer Signed-off-by: Jacob Perron <[email protected]> * Switch from List to array Signed-off-by: Jacob Perron <[email protected]>
jacobperron
added a commit
to ros2-java/ros2_java
that referenced
this pull request
May 17, 2022
* Add interfaces for action goal, result, and feedback Implementing these interfaces in the code generation template makes it easier to pass around these types in a generic way. Note, the 'final' modifier had to be removed from generated message types in order to extend goal, result, and feedback types in action definitions. Signed-off-by: Jacob Perron <[email protected]> * Add new definitions for action goal response and request Signed-off-by: Jacob Perron <[email protected]> * Add getter for UUID to SendGoalRequest Also make inner classes static. Signed-off-by: Jacob Perron <[email protected]> * Add getStamp method to GoalResponseDefinition Signed-off-by: Jacob Perron <[email protected]> * Partially revert "Add interfaces for action goal, result, and feedback" Partially revert commit dd04614. I don't think we need to aliases for the message types, but I'll add them back if they turn out to be useful. * Parameterize goal request and response interfaces on action type Signed-off-by: Jacob Perron <[email protected]> * Fix getGoalUuid implementation It should return a List, since it is hashable. Signed-off-by: Jacob Perron <[email protected]> * List<Byte> -> byte[] Signed-off-by: Jacob Perron <[email protected]> * Add ActionServer skeleton * Add new action module with classes/interfaces related to the ActionServer * Add methods for creating and removing ActionServers from a Node * Implement dispose method for ActionServer. Signed-off-by: Jacob Perron <[email protected]> * Add ActionServer creation logic and unit tests Signed-off-by: Jacob Perron <[email protected]> * Add action server logic for accepting and canceling goals This changeset includes the following: * an implementation of the goal handle for action servers * action server integration with the base executor * action server integration with ROS nodes * unit tests for the action server, receiving goal and cancel requests Signed-off-by: Jacob Perron <[email protected]> * cleanup Signed-off-by: Jacob Perron <[email protected]> * more cleanup Signed-off-by: Jacob Perron <[email protected]> * Minor refactor: move response enums into callback interfaces Signed-off-by: Jacob Perron <[email protected]> * Remove TODO The goal request already contains a getter for the goal ID. Signed-off-by: Jacob Perron <[email protected]> * Fix accident Signed-off-by: Jacob Perron <[email protected]> * Alphabetize Signed-off-by: Jacob Perron <[email protected]> * Refactor: replace 'getNumberOf*()' JNI functions with single 'getNumberOfEntites()' function Signed-off-by: Jacob Perron <[email protected]> * Avoid string copy Signed-off-by: Jacob Perron <[email protected]> * Remove extern C Replace with 'namespace rcljava' Signed-off-by: Jacob Perron <[email protected]> * Remove unnecessary synchronization Signed-off-by: Jacob Perron <[email protected]> * Refactor access to goalCallback Signed-off-by: Jacob Perron <[email protected]> * typesafe request and response definitions Signed-off-by: Jacob Perron <[email protected]> * Be more specific about template type for GoalCallback Signed-off-by: Jacob Perron <[email protected]> * Minor refactor Signed-off-by: Jacob Perron <[email protected]> * Make GoalHandleImpl inner class instead of static Signed-off-by: Jacob Perron <[email protected]> * Remove synchronized from getHandle Signed-off-by: Jacob Perron <[email protected]> * Add TODO for Waitable interface Signed-off-by: Jacob Perron <[email protected]> * Fix style Signed-off-by: Jacob Perron <[email protected]> * Minor refactor Signed-off-by: Jacob Perron <[email protected]> * Add docs for createActionServer Signed-off-by: Jacob Perron <[email protected]> * Switch from List to array Signed-off-by: Jacob Perron <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #52
This is a partial implementation; trying to keep the diff smaller for review.
I'll follow-up with more PRs competing the implementation.