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

API for action servers and clients #216

Closed
wants to merge 46 commits into from
Closed

API for action servers and clients #216

wants to merge 46 commits into from

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Oct 12, 2018

This PR proposes an API for action clients and action services. It is incomplete, but what is here is ready for review.

connects to ros2/design#193

@sloretz sloretz added the in review Waiting for review (Kanban column) label Oct 12, 2018
handle_goal,
handle_cancel,
execute,
rclcpp_action::StaticThreadPoolExecutePolicy(4));

Choose a reason for hiding this comment

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

I'm concerned about the use of a synchronous ``execute()` function here. It has been my experience in ROS1 that many of the actions that control the high level behavior of our robots themselves dispatch to one or more asynchronous tasks (be they library calls, another set of actions, etc). I find it more elegant to handle these via the callbacks of these sub-functions rather than spinning in a given thread. Furthermore, using an execute callback means that you require a 1 to 1 mapping of goals to threads. This likely won't scale well when you hit many goals active at once.

So, in my humble opinion, I would prefer to see a base API that mimics the original (not simple) action server.

Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that the signature of the execute callback can optionally return void, which essentially means you can dispatch any asynchronous tasks and return from it immediately, electing to asynchronously "finish" the execution of the action in the future by using set_canceled, set_aborted, or set_success asynchronously.

In that case the execute function is just your notification that the goal is ready to be executed.

Also, if you don't want the asynchronous execute notice at all, one possible value for the "execution policy" is "do nothing". Then it's up to the user to poll the action server object to "drain" ready goals and execute them to one of the terminal states.

So in short, I think what you're asking for should be possible in the current design.

Choose a reason for hiding this comment

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

👍 Sounds good.

Please forgive a dumb question: Is the contents of the rclcpp_action/rclcpp_action.hpp file available somewhere for review so that I might intuit that answer myself?

I also have one more sort-of-pedantic question: The given execution callback does not appear to return a value if the loop aborts early because of rclcpp::ok() returning false. In this case, what would the appropriate value be aborted?

Copy link
Member

Choose a reason for hiding this comment

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

Please forgive a dumb question: Is the contents of the rclcpp_action/rclcpp_action.hpp file available somewhere for review so that I might intuit that answer myself?

No unfortunately, we're sort in the process of developing these API's. This pr/examples is meant to be a motivating example for how the API should work. So basically it's a made up API at the moment 😄. I also expect it will evolve more. So for now, asking us questions about the examples is the most reasonable thing.

I also have one more sort-of-pedantic question: The given execution callback does not appear to return a value if the loop aborts early because of rclcpp::ok() returning false. In this case, what would the appropriate value be aborted?

The user defines the "execute" callback so if in the process they check for and notice the rclcpp::ok is returning false, then they can choose to call set_abort themselves.

If it occurs inbetween calls to the user (e.g. after handle_goal has completed but before execute is called), I'm not 100% sure right now. I'd assume the action server would cancel all outstanding goals and then wait for any on going user callbacks to finish executing. It's something to maybe keep an eye and ask about again when we have a more complete prototype.

@sloretz sloretz mentioned this pull request Dec 6, 2018
jacobperron added a commit that referenced this pull request Jan 8, 2019
@jacobperron jacobperron mentioned this pull request Jan 9, 2019
jacobperron added a commit that referenced this pull request Feb 7, 2019
@sloretz
Copy link
Contributor Author

sloretz commented Feb 27, 2019

Replaced by #216

@sloretz sloretz closed this Feb 27, 2019
@sloretz sloretz removed the in review Waiting for review (Kanban column) label Feb 27, 2019
@sloretz sloretz deleted the actions_proposal branch February 27, 2019 22:56
jacobperron added a commit that referenced this pull request Mar 5, 2019
* Copy rclpy action examples from #216

* Bump version of examples_rclpy_action to match other packages

* Consolidate composable action client example

* Add action client example that is not composable

* Wait for action server

* Restructure into separate packages for action client and action server examples

This package structure is consistent with examples for services and topics.

* Update API in action server examples

* Rename action server examples

Now the 'simplest' example is 'server.py'.

* Add action server example that is not composable

* Update setup.py

* Fix syntax

* Update action client examples to use goal handle API for the result

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Improve action client output and result handling

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update action server examples

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Move action examples into Python packages

This avoid top-level module names from clashing when installed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add action client cancel example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Address review comments

* Update author
* Update copyright year
* Shutdown client example after result received

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* GoalResponse.ACCEPT_AND_EXECUTE -> GoalResponse.ACCEPT

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix client_cancel example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove race from server_single_goal example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add action server example that defers the execution of an accepted goal

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Reduce the timer period for the client cancel example

This makes it easy to experiment with the scenario where a deferred goal is canceled prior to execution.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Support canceling goals with non-composable action server example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add action server example that queues goals

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Address review

- Fix comment
- Add author tag to package.xml

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
nuclearsandwich pushed a commit that referenced this pull request Mar 10, 2019
* Copy rclpy action examples from #216

* Bump version of examples_rclpy_action to match other packages

* Consolidate composable action client example

* Add action client example that is not composable

* Wait for action server

* Restructure into separate packages for action client and action server examples

This package structure is consistent with examples for services and topics.

* Update API in action server examples

* Rename action server examples

Now the 'simplest' example is 'server.py'.

* Add action server example that is not composable

* Update setup.py

* Fix syntax

* Update action client examples to use goal handle API for the result

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Improve action client output and result handling

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update action server examples

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Move action examples into Python packages

This avoid top-level module names from clashing when installed.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add action client cancel example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Address review comments

* Update author
* Update copyright year
* Shutdown client example after result received

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* GoalResponse.ACCEPT_AND_EXECUTE -> GoalResponse.ACCEPT

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix client_cancel example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove race from server_single_goal example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add action server example that defers the execution of an accepted goal

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Reduce the timer period for the client cancel example

This makes it easy to experiment with the scenario where a deferred goal is canceled prior to execution.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Support canceling goals with non-composable action server example

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add action server example that queues goals

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Address review

- Fix comment
- Add author tag to package.xml

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
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.

4 participants