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

Adds a timeout feature to rostopic echo #792

Merged
merged 15 commits into from
Feb 2, 2023

Conversation

arjo129
Copy link
Contributor

@arjo129 arjo129 commented Jan 17, 2023

Builds on top of:

This PR adds a --time-out feature which allows you to set the amount of time that rostopic echo should wait. This feature was
requested by #529, however, we only added --once and no time out. I have a use case where this timeout could be useful hence I added it

Signed-off-by: Arjo Chakravarty arjo@openrobotics.org

A lot of types across verbs in ros2 topic are shared. This
PR refactors the types to be used by different verbs preventing
redefinition of the same types in different verbs. There is no
behaviour changed introduced.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
This commit addsa `--time-out` feature which allows you to set the
amount of time that rostopic echo should wait. This feature was
requested by ros2#529, however we only added `--once` and no time out.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

test would be ideal, to check certain published messages are printed in specific time window until it times out.

ros2topic/ros2topic/verb/echo.py Outdated Show resolved Hide resolved
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@fujitatomoya
Copy link
Collaborator

test would be ideal, to check certain published messages are printed in specific time window until it times out.

i think you can refer to,

@launch_testing.markers.retry_on_failure(times=5)
def test_echo_once(self, launch_service, proc_info, proc_output):
topic = '/clitest/topic/echo_once'
publisher = self.node.create_publisher(String, topic, 10)
assert publisher
def publish_message():
publisher.publish(String(data='hello'))
publish_timer = self.node.create_timer(1.0, publish_message)
try:
command_action = ExecuteProcess(
cmd=['ros2', 'topic', 'echo', '--once', topic, 'std_msgs/msg/String'],
additional_env={
'PYTHONUNBUFFERED': '1'
},
output='screen'
)
with launch_testing.tools.launch_process(
launch_service, command_action, proc_info, proc_output,
output_filter=launch_testing_ros.tools.basic_output_filter(
filtered_rmw_implementation=get_rmw_implementation_identifier()
)
) as command:
# The future won't complete - we will hit the timeout
self.executor.spin_until_future_complete(
rclpy.task.Future(), timeout_sec=3
)
assert command.wait_for_shutdown(timeout=5)
assert launch_testing.tools.expect_output(
expected_lines=[
'data: hello',
'---',
],
text=command.output,
strict=True
)
finally:
# Cleanup
self.node.destroy_timer(publish_timer)
self.node.destroy_publisher(publisher)

@arjo129
Copy link
Contributor Author

arjo129 commented Jan 18, 2023

Thanks! Working on it.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 requested review from fujitatomoya and removed request for gbiggs and audrow January 19, 2023 06:39
ros2topic/ros2topic/api/__init__.py Outdated Show resolved Hide resolved
ros2topic/ros2topic/verb/echo.py Outdated Show resolved Hide resolved
ros2topic/test/test_echo_pub.py Show resolved Hide resolved
ros2topic/test/test_echo_pub.py Outdated Show resolved Hide resolved
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
ros2topic/ros2topic/api/__init__.py Outdated Show resolved Hide resolved
ros2topic/test/test_echo_pub.py Show resolved Hide resolved
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'll run CI on it next.

But I'd like to get @fujitatomoya's opinion before I merge.

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@arjo129 LTGM!

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.

None yet

3 participants