Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Added read_condition for services and clients#66

Merged
esteve merged 1 commit intomasterfrom
fix_service_samples
Oct 15, 2015
Merged

Added read_condition for services and clients#66
esteve merged 1 commit intomasterfrom
fix_service_samples

Conversation

@esteve
Copy link
Copy Markdown
Member

@esteve esteve commented Aug 3, 2015

This should fix ros2/rclcpp#52 for services

Connects to ros2/rclcpp#52

@esteve esteve added the in progress Actively being worked on (Kanban column) label Aug 3, 2015
@esteve
Copy link
Copy Markdown
Member Author

esteve commented Aug 3, 2015

@dirk-thomas I applied the same fix as for subscriptions, but can't seem to make the client fire when the service sends back the response. I ran rtiddsspy and messages are sent between the service and the client, though. Would appreciate any feedback from you. Thanks.

@esteve
Copy link
Copy Markdown
Member Author

esteve commented Aug 3, 2015

@dirk-thomas I think I found where the problem is, the client's read condition was not being attached to the waitset.

@esteve
Copy link
Copy Markdown
Member Author

esteve commented Aug 3, 2015

Linux buildfarm:

http://ci.ros2.org/job/ros2_batch_ci_linux/137/

dirk-thomas
dirk-thomas reviewed Aug 3, 2015
View reviewed changes
rmw_connext_cpp/src/functions.cpp
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas Aug 3, 2015

Choose a reason for hiding this comment

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

This needs to be cleaned up in the fail section.

Same below.

Copy link
Copy Markdown
Member Author

@esteve esteve Aug 3, 2015

Choose a reason for hiding this comment

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

Fixed.

@dirk-thomas dirk-thomas mentioned this pull request Aug 3, 2015
Closed
@tfoote tfoote mentioned this pull request Aug 3, 2015
Merged
@esteve esteve force-pushed the fix_service_samples branch from 6186f82 to 08c0b0c Compare August 3, 2015 23:31
@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Aug 4, 2015

+1

Has it been tested that this fixes ros2/system_tests#14

@esteve esteve force-pushed the fix_service_samples branch from c371f39 to b9878ab Compare August 4, 2015 17:42
@esteve
Copy link
Copy Markdown
Member Author

esteve commented Aug 4, 2015

Buildfarm job:

http://ci.ros2.org/job/ros2_batch_ci_linux/143/

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Aug 4, 2015

This CI job does not cover the new parameter test yet.

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Aug 4, 2015

Code changes look good, waiting on the test.

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 18, 2015

+1, unfortunately this needs to be rebased now

@esteve esteve force-pushed the fix_service_samples branch from b9878ab to 6943f23 Compare August 18, 2015 19:18
@esteve
Copy link
Copy Markdown
Member Author

esteve commented Aug 18, 2015

Rebased.

@esteve esteve force-pushed the fix_service_samples branch from 6943f23 to 51f7514 Compare August 20, 2015 18:49
@esteve esteve force-pushed the fix_service_samples branch 2 times, most recently from 8eb34b2 to 650068f Compare September 15, 2015 23:52
@dirk-thomas dirk-thomas mentioned this pull request Oct 15, 2015
Merged
@jacquelinekay
Copy link
Copy Markdown
Contributor

jacquelinekay commented Oct 15, 2015

I had to rebase this on master to get it to build on my machine (currently test ros2/system_tests#57)

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Oct 15, 2015
@jacquelinekay
Copy link
Copy Markdown
Contributor

jacquelinekay commented Oct 15, 2015

+1

esteve added a commit that referenced this pull request Oct 15, 2015
@esteve
Merge pull request #66 from ros2/fix_service_samples
7528ffe 
Added read_condition for services and clients
@esteve esteve merged commit 7528ffe into master Oct 15, 2015
@esteve esteve removed the in review Waiting for review (Kanban column) label Oct 15, 2015
@esteve esteve deleted the fix_service_samples branch October 15, 2015 18:39
@dirk-thomas dirk-thomas mentioned this pull request Feb 5, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Subscriptions are not triggered sometimes

5 participants

@esteve @dirk-thomas @wjwwood @tfoote @jacquelinekay