Skip to content

launch testing : Wait for topics to publish#274

Merged
adityapande-1995 merged 11 commits intomasterfrom
aditya/wait_for_topic
Oct 6, 2021
Merged

launch testing : Wait for topics to publish#274
adityapande-1995 merged 11 commits intomasterfrom
aditya/wait_for_topic

Conversation

@adityapande-1995
Copy link
Copy Markdown
Contributor

@adityapande-1995 adityapande-1995 commented Oct 4, 2021

This aims to close #272. It aims to provide an API in to be used with launch_testing where we provide a set of topics and we wait for at least a single message to appear on all of them.

Sample usage :

import unittest
import launch
import launch.actions
import launch_ros.actions
import launch_testing.actions
import launch_testing.markers
import pytest
import rclpy
from rclpy.node import Node
from std_msgs.msg import String

from launch_testing_ros import WaitForTopics

def generate_node(i):
    return launch_ros.actions.Node(
        executable='talker',
        package='demo_nodes_cpp',
        name='demo_node_' + str(i),
        remappings = [('chatter', 'chatter_' + str(i))]
    )

@pytest.mark.launch_test
@launch_testing.markers.keep_alive
def generate_test_description():
    # 'n' changes the number of nodes and topics
    n = 5
    description = [generate_node(i) for i in range(n)] + [launch_testing.actions.ReadyToTest()]
    return launch.LaunchDescription(description), {'count':n}

class TestFixture(unittest.TestCase):

    def test_check_if_msgs_published(self, proc_output, count):

        topic_list = [('chatter_' + str(i), String) for i in range(count)]
        with WaitForTopics(topic_list, timeout=5.0):
            print("The topics are alive !")


Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
@adityapande-1995 adityapande-1995 self-assigned this Oct 4, 2021
Copy link
Copy Markdown
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

I think this is a good start. I've left some feedback below.

Also, I'm not sure that creating a separate node class adds much value; I would have just put all the logic inside WaitForTopics. I'm okay with it either way though.

self.create_subscription(
topic_type,
topic_name,
self.callback_template(topic_name),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alternatively, we can use functools.partial.

functools.partial(self.topic_callback, topic_name)

and then the callback would look like:

def topic_callback(self, topic_name, data):
    ...

I don't think there's anything wrong with this, so feel free to leave it as-is.

@jacobperron jacobperron added the enhancement New feature or request label Oct 5, 2021
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Copy link
Copy Markdown
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM, two more comments below

Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
@adityapande-1995
Copy link
Copy Markdown
Contributor Author

CI :
build : --packages-above-and-dependencies launch_testing_ros
test : --packages-above launch_testing_ros

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

Copy link
Copy Markdown
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

A couple suggestions for the test code. LGTM

Signed-off-by: Aditya Pande <aditya050995@gmail.com>
@adityapande-1995 adityapande-1995 merged commit bbcc0cc into master Oct 6, 2021
@delete-merged-branch delete-merged-branch bot deleted the aditya/wait_for_topic branch October 6, 2021 17:36
@Blast545
Copy link
Copy Markdown
Contributor

Blast545 commented Oct 7, 2021

👨‍🌾 It seems that this PR caused several regressions in the WIndows buildfarms jobs:

nightly_win_rel#2079
nightly_win_deb#2134

Do you think you can take a look to a possible fix @adityapande-1995? I think it's likely related to these lines:

https://github.com/ros2/launch_ros/blob/master/launch_testing_ros/launch_testing_ros/wait_for_topics.py#L20-L23

It seems to me that rclpy can't be imported there for some reason. I'll open a revert PR in the meantime to be sure this PR caused the regressions.

@clalancette
Copy link
Copy Markdown
Contributor

It's weird that this caused a regression, particularly because CI seems to have been run properly and came back green. But let's see if the revert PR fixes it.

Blast545 added a commit that referenced this pull request Oct 7, 2021
Blast545 added a commit that referenced this pull request Oct 7, 2021
This reverts commit bbcc0cc.

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545
Copy link
Copy Markdown
Contributor

Blast545 commented Oct 7, 2021

I can't replicate the error in CI. I'll investigate a bit deeper before reaching any conclusion.

@aprotyas
Copy link
Copy Markdown
Member

aprotyas commented Oct 8, 2021

It seems to me that rclpy can't be imported there for some reason. I'll open a revert PR in the meantime to be sure this PR caused the regressions.

I believe that's for this. Not sure why it can't be imported before running tests though.
https://github.com/ros2/rclpy/blob/6dd9540661631c971499d5672fb50a8ec1e3ac22/rclpy/test/__init__.py#L20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request (launch_testing): Check if a topic is publishing messages, with a timeout

5 participants