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

Implement QoS: liveliness, deadline, lifespan#352

Merged
wjwwood merged 39 commits intoros2:masterfrom
aws-ros-dev:qos_reviewed
May 3, 2019
Merged

Implement QoS: liveliness, deadline, lifespan#352
wjwwood merged 39 commits intoros2:masterfrom
aws-ros-dev:qos_reviewed

Conversation

@dabonnie
Copy link
Copy Markdown
Contributor

@dabonnie dabonnie commented Apr 3, 2019 •
edited
Loading

Summary
Provide implementation of QoS changes for rmw events. If applied this PR will

  • ADD support for QoS Liveliness Lost, Offered Deadline Missed, Liveliness Changed, and Requested Deadline Missed
  • ADD manual Liveliness assertion
  • ADD conversion methods to map from RTI Connext QoS DDS events to RMW QoS events

Relates to ros2/design#212
Connects to ros2/rmw#171

@tfoote tfoote added the in progress Actively being worked on (Kanban column) label Apr 3, 2019
@nburek nburek mentioned this pull request Apr 3, 2019
Closed
29 tasks
@nburek
Copy link
Copy Markdown

nburek commented Apr 4, 2019

We are in the process of making sure all the commits are signed off and a little other git hygiene to make sure these commits can be rebased easily when ready.

ross-desmond and others added 26 commits April 4, 2019 10:39
@ross-desmond
Add events to rmw_wait
f1a4615 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@ross-desmond
Add rmw_take_event stub
8f3a91e 
Edit: Fix handle active event conditions to only set active specific
events.

@todo: found multiple loops through active conditions which is not necessary in
wait.hpp, time cost on average O(n), where n is the number of
conditions. Actual time cost: 4*n. Can be removed and made O(1).

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@ross-desmond
Start filling in rmw_take_event
f2aa49b 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
fix broken build
9432a94 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
fix lint errors
0762003 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
apply uncrustify
c996a60 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Implemented the take event method
0b64474 
Added handling of the take event in both static publisher and static subscriber

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Implemented the take event method
2405a01 
Added handling of the take event in both static publisher and static subscriber

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
Fix build
ac0c251 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Code and formatting fixes
9724e74 
Handle DDS return codes
Added event_converter implementation

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Addressed review comments
f915289 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Removed unused includes
2f8de98 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
Very linty
1c57b0c 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
fix runtime linking error
85739d1 
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
Set liveliness/deadline/lifespan QoS settings on Connext
bc05aa5 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
Disable manual liveliness and add comment for infinite
8788677 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
Re-enable manual liveliness, rename to by_node
76689ec 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Removed unsupported events
fb6f514 
Sync with changes made in rmw

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
change rmw_event_t APIs from create()/destroy() to init()/fini() pattern
a1e564c 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Added documentation
b9af9d7 
Remove StatusCondition trigger check

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Fix broken logical comparison
1075c0b 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Rename implementation method to match header file
493606a 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
Move rmw_*_event_init implementation to rmw
f140c18 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
fix issue with events not being triggered
c326ccb 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@mm318
implement assert_liveliness()
87857ac 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@dabonnie
Added implementation files for inline methods
973cecb 
Edited code to fix linting
Addressed review comments

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>
@jwillemsen
Copy link
Copy Markdown
Contributor

jwillemsen commented Apr 8, 2019

@dabonnie great, I started some time ago with an effort to add OpenDDS but lack sponsoring to complete this, hopefully we can continue that at some point

@dabonnie
Copy link
Copy Markdown
Contributor Author

dabonnie commented Apr 12, 2019

@wjwwood @jwillemsen any other comments?

jwillemsen
jwillemsen reviewed Apr 12, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jwillemsen jwillemsen left a comment

Choose a reason for hiding this comment

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

Some more DDS without DDS:: in the code

Comment thread rmw_connext_cpp/include/rmw_connext_cpp/connext_static_subscriber_info.hpp Outdated
Comment thread rmw_connext_shared_cpp/include/rmw_connext_shared_cpp/connext_static_event_info.hpp Outdated
Comment thread rmw_connext_shared_cpp/include/rmw_connext_shared_cpp/wait.hpp Outdated
dabonnie and others added 2 commits April 12, 2019 16:19
@dabonnie
Addressed more naming from review
3d34de7 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@mm318
Merge remote-tracking branch 'origin/master'
0232cfa 
Signed-off-by: Miaofei <miaofei@amazon.com>
jwillemsen
jwillemsen reviewed Apr 16, 2019
View reviewed changes
Comment thread rmw_connext_shared_cpp/include/rmw_connext_shared_cpp/event_converter.hpp Outdated
mm318 and others added 2 commits April 16, 2019 15:57
@mm318
fix windows build
3bfa6c0 
Signed-off-by: Miaofei <miaofei@amazon.com>
@ross-desmond
Disable inactive status events
0f12385 
Signed-off-by: Ross Desmond <44277324+ross-desmond@users.noreply.github.com>
@nburek
Copy link
Copy Markdown

nburek commented Apr 19, 2019

@wjwwood This is ready for review again. Can you please update the label? Thanks.

@wjwwood wjwwood added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 19, 2019
wjwwood
wjwwood suggested changes Apr 24, 2019
View reviewed changes
Comment thread rmw_connext_cpp/include/rmw_connext_cpp/connext_static_publisher_info.hpp Outdated
Comment thread rmw_connext_cpp/include/rmw_connext_cpp/connext_static_publisher_info.hpp Outdated
Comment thread rmw_connext_cpp/include/rmw_connext_cpp/connext_static_publisher_info.hpp Outdated
Comment thread rmw_connext_cpp/include/rmw_connext_cpp/connext_static_publisher_info.hpp Outdated
Comment thread rmw_connext_cpp/include/rmw_connext_cpp/connext_static_subscriber_info.hpp
Comment thread rmw_connext_shared_cpp/src/event.cpp Outdated
Comment thread rmw_connext_shared_cpp/src/event.cpp
Comment thread rmw_connext_shared_cpp/src/event.cpp Outdated
Comment thread rmw_connext_shared_cpp/src/event.cpp
Comment thread rmw_connext_shared_cpp/src/event.cpp
@wjwwood wjwwood mentioned this pull request Apr 24, 2019
Merged
@ross-desmond
Address PR comments and formatting
6eff78d 
Signed-off-by: Ross Desmond <44277324+ross-desmond@users.noreply.github.com>
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Apr 25, 2019

The latest changes look a lot better, thanks for iterating! There are a few lingering comments that haven't been addressed (changed or commented on), for example there's two or three in wait.hpp. I'll check again after a while to see if those have been resolved.

@dabonnie
Address review comments and formatting
5e2bb63 
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@dabonnie
Copy link
Copy Markdown
Contributor Author

dabonnie commented Apr 29, 2019

@wjwwood I believe this should take care of your requested changes :-)

@mm318
fix cpplint and uncrustify issues
de529fd 
Signed-off-by: Miaofei <miaofei@amazon.com>
@mm318
Merge in 'ros2/master'
fa97d23 
Signed-off-by: Miaofei <miaofei@amazon.com>
Fix OSX and MSbuild warnings
883ba2e 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented May 3, 2019

Unfortunately, 883ba2e doesn't pass linting, due to a line now being too long. We had already begun testing when the last force push happened (4 hours before this comment).

@wjwwood wjwwood mentioned this pull request May 3, 2019
Merged
@mm318
Copy link
Copy Markdown
Member

mm318 commented May 3, 2019

I can fix it now.

@mm318
fix line length issue
8e18d41 
Signed-off-by: Miaofei <miaofei@amazon.com>
@wjwwood wjwwood merged commit cadd787 into ros2:master May 3, 2019
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label May 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers

@jwillemsen jwillemsen jwillemsen left review comments

@mm318 mm318 mm318 left review comments

@ross-desmond ross-desmond ross-desmond left review comments

@wjwwood wjwwood wjwwood approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@dabonnie @nburek @jwillemsen @wjwwood @mm318 @ross-desmond @tfoote