Skip to content

add test with multiple service calls#57

Merged
dirk-thomas merged 1 commit intomasterfrom
test_multiple_service_calls
Oct 15, 2015
Merged

add test with multiple service calls#57
dirk-thomas merged 1 commit intomasterfrom
test_multiple_service_calls

Conversation

@dirk-thomas
Copy link
Copy Markdown
Member

This new test implement exactly the same test for services as #20 does for pub/sub. It covers what has been tracked in ros2/rclcpp#52 for a while.

Without any patches the test fails since most rmw implementations currently don't wake up a second time: http://ci.ros2.org/job/ros2_batch_ci_osx/349/

With the two patches (ros2/rmw_connext#66 ros2/rmw_opensplice#60) the test passes since the rmw implementations check the read correctly (as suggested in ros2/rclcpp#52 (comment)): http://ci.ros2.org/job/ros2_batch_ci_osx/350/

Connect to ros2/rclcpp#52

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Oct 15, 2015
@dirk-thomas dirk-thomas self-assigned this Oct 15, 2015
@jacquelinekay
Copy link
Copy Markdown
Contributor

Nice. Just for my own sanity, I'm going to build and the fix_service_samples branches locally and run the multiple services test to make sure it's not flaky.

@esteve
Copy link
Copy Markdown
Member

esteve commented Oct 15, 2015

+1

@dirk-thomas dirk-thomas mentioned this pull request Oct 15, 2015
@jacquelinekay
Copy link
Copy Markdown
Contributor

The test passed 50/50 times on my machine. I feel good about that. +1

@esteve
Copy link
Copy Markdown
Member

esteve commented Oct 15, 2015

I've merged the two PRs that fix the issue, this PR should be ready for merging.

dirk-thomas added a commit that referenced this pull request Oct 15, 2015
add test with multiple service calls
@dirk-thomas dirk-thomas merged commit b791232 into master Oct 15, 2015
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Oct 15, 2015
@dirk-thomas dirk-thomas deleted the test_multiple_service_calls branch October 15, 2015 18:53
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.

3 participants