-
Notifications
You must be signed in to change notification settings - Fork 315
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
rclpy action examples #222
Conversation
This is weird, possible setuptools bug? When I run the action
setuptools warns the name must be unique within a 'distribution' but the entrypoint uses pkg_resources.load_entry_point which clearly shows the distribution is
|
…r examples This package structure is consistent with examples for services and topics.
Now the 'simplest' example is 'server.py'.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
1119b27
to
32ea3b6
Compare
Rebased on master and updated action server examples. This ready for review in conjunction with ros2/rclpy#270. |
This avoid top-level module names from clashing when installed. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server.py
Outdated
Show resolved
Hide resolved
"""Executes the goal.""" | ||
self.get_logger().info('Executing goal...') | ||
with self._goal_lock: | ||
self._goal_handle = goal_handle |
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.
Is it possible for two goals to get through goal_callback
before the first reaches execute_callback
? In that case one of them will overwrite the other in self._goal_handle
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.
Fixed with the use of handle_accepted_callback
b0c739a
rclpy/actions/minimal_action_client/examples_rclpy_minimal_action_client/client.py
Show resolved
Hide resolved
|
||
def cancel_callback(self, goal_handle): | ||
"""Accepts or rejects a client request to cancel an action.""" | ||
self.get_logger().info('Received cancel request') |
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.
When running the cancel example this is the last log message I see. Both the client and the server continue to run. I was expecting to see the server say Goal canceld
from line 67.
$ ros2 run examples_rclpy_minimal_action_client client_cancel
[INFO] [minimal_action_client]: Waiting for action server...
[INFO] [minimal_action_client]: Sending goal request...
[INFO] [minimal_action_client]: Goal accepted :)
[INFO] [minimal_action_client]: Received feedback: [0, 1, 1]
[INFO] [minimal_action_client]: Received feedback: [0, 1, 1, 2]
[INFO] [minimal_action_client]: Received feedback: [0, 1, 1, 2, 3]
[INFO] [minimal_action_client]: Canceling goal
$ ros2 run examples_rclpy_minimal_action_server server
[INFO] [minimal_action_server]: Received goal request
[INFO] [minimal_action_server]: Executing goal...
[INFO] [minimal_action_server]: Publishing feedback: [0, 1, 1]
[INFO] [minimal_action_server]: Publishing feedback: [0, 1, 1, 2]
[INFO] [minimal_action_server]: Publishing feedback: [0, 1, 1, 2, 3]
[INFO] [minimal_action_server]: Received cancel request
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.
Fixed in ce7bb67
I was making a synchronous call in a callback (which is no good, as documented 😞)
* Update author * Update copyright year * Shutdown client example after result received Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
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>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
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, just a couple nitpick comments
rclpy/actions/minimal_action_client/examples_rclpy_minimal_action_client/client_cancel.py
Outdated
Show resolved
Hide resolved
goal_handle.publish_feedback(feedback_msg) | ||
|
||
# Sleep for demonstration purposes | ||
time.sleep(1) |
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.
nit, should be able to use asyncio sleep to let the executor do other stuff (although, there's no other stuff being done in this example)
await asyncio.sleep(1)
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.
Good point, but there must be a bug in rclpy
because changing this to await asyncio.sleep(1)
causes it to hang forever at this line. If I also change the executor to a SingleThreadedExecutor
, then there's an assertion error with the following trace:
Traceback (most recent call last):
File "/home/jacob/ws/actions_ws/install/examples_rclpy_minimal_action_server/lib/examples_rclpy_minimal_action_server/server", line 11, in <module>
load_entry_point('examples-rclpy-minimal-action-server==0.6.1', 'console_scripts', 'server')()
File "/home/jacob/ws/actions_ws/install/examples_rclpy_minimal_action_server/lib/python3.5/site-packages/examples_rclpy_minimal_action_server/server.py", line 103, in main
rclpy.spin(minimal_action_server, executor=executor)
File "/home/jacob/ws/actions_ws/install/rclpy/lib/python3.5/site-packages/rclpy/__init__.py", line 119, in spin
executor.spin_once()
File "/home/jacob/ws/actions_ws/install/rclpy/lib/python3.5/site-packages/rclpy/executors.py", line 573, in spin_once
raise handler.exception()
File "/home/jacob/ws/actions_ws/install/rclpy/lib/python3.5/site-packages/rclpy/task.py", line 207, in __call__
self._handler.send(None)
File "/home/jacob/ws/actions_ws/install/rclpy/lib/python3.5/site-packages/rclpy/action/server.py", line 323, in _execute_goal
execute_result = await await_or_execute(execute_callback, goal_handle)
File "/home/jacob/ws/actions_ws/install/rclpy/lib/python3.5/site-packages/rclpy/executors.py", line 89, in await_or_execute
return await callback(*args)
File "/home/jacob/ws/actions_ws/install/examples_rclpy_minimal_action_server/lib/python3.5/site-packages/examples_rclpy_minimal_action_server/server.py", line 80, in execute_callback
await asyncio.sleep(1)
File "/usr/lib/python3.5/asyncio/tasks.py", line 516, in sleep
return (yield from future)
File "/usr/lib/python3.5/asyncio/futures.py", line 362, in __iter__
assert self.done(), "yield from wasn't used with future"
AssertionError: yield from wasn't used with future
I'll see if I can figure out what's gone wrong.
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.
This looks like me being wrong about what asyncio.sleep()
expects. https://github.com/python/cpython/blob/6b0d50d92e80faaed8a043e6554ccb5d046114a5/Lib/asyncio/tasks.py#L517-L532
Looks like it needs an asyncio event loop. You can ignore my comments about using it
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.
I see.
I can reproduce the same issue with a Service server as well. It seems like something rclpy should be able to handle though, so I've opened an issue ros2/rclpy#279.
minimal_action_server = MinimalActionServer() | ||
|
||
# Use a MultiThreadedExecutor to enable processing goals concurrently | ||
executor = MultiThreadedExecutor() |
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.
I would expect a single threaded executor to be ok if using asyncio.sleep()
above.
- Fix comment - Add author tag to package.xml Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Replaces #216.