Implement QoS: liveliness, deadline, lifespan by ross-desmond · Pull Request #171 · ros2/rmw

ross-desmond · 2019-03-29T17:30:28Z

Summary
Provide init/fini and take functions for events. Modify wait_set to include event handles for notification of status changes.

Details:

ADD initial qos interface changes
ADD rmw take_event interface
ADD rmw event type enum
MODIFY events type from void** to rmw_event_t**
ADD RMW QoS Event Definitions
ADD section about DCO to CONTRIBUTING.md
EDIT liveliness policy types to reflect actual settings
ADD an invalid enum in rmw_event_type_t for variables to default to
MOVE rmw_*_event_init implementation to rmw
** Event initialization should require no modifications from the underlying rmw layer and therefore can be initialized in the rmw layer.

Signed-off-by: Ross Desmond 44277324+ross-desmond@users.noreply.github.com

Depends on #173

CONTRIBUTING.md

rmw/include/rmw/event.h

rmw/include/rmw/types.h

rmw/src/event.c

dabonnie · 2019-04-02T18:25:03Z

@wjwwood I believe we have addressed all of your comments. Please let us know if there is anything else outstanding given the latest changes.

wjwwood · 2019-04-03T18:58:32Z

This branch has conflicts and cannot be merged.

wjwwood

Unfortunately I cannot review it in the current state, as it has lots of commits from master mixed into it.

CONTRIBUTING.md

rmw/cmake/get_rmw_typesupport.cmake

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

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>

…eration of the feature 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>

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Removed unsupported QoS types Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

**Summary** Event initialization should require no modifications from the underlying rmw layer and therefore can be initialized in the rmw layer. Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

thomas-moulard · 2019-05-02T17:53:25Z

Linux
Linux-aarch64
macOS
Windows

mm318 · 2019-05-02T18:36:21Z

@thomas-moulard - please run the following CI job:

Gist file: https://gist.githubusercontent.com/mm318/582ceeae9b2d2efc4c48bfa7c2b98f12/raw/cc86a231fd3f6c20c03e576290a1968fa97ef6c8/ros2.repos
Additional BUILD flags: --packages-up-to demo_nodes_cpp
Additional TEST flags: --packages-up-to demo_nodes_cpp
CI Job: ci_launcher

thomas-moulard · 2019-05-02T18:40:45Z

Linux
Linux-aarch64
macOS
Windows

wjwwood · 2019-05-02T22:03:44Z

Let me know if I can help. Sorry about the last minute collisions, I know they're a pain.

Signed-off-by: Miaofei <miaofei@amazon.com>

emersonknapp · 2019-05-02T22:19:00Z

Thanks @wjwwood - we're running through it and will let you know - most of the rebases have been fairly straightforward but a few things are throwing us for a loop - hopefully have a good build ready to go soon

thomas-moulard · 2019-05-02T22:22:03Z

Linux
Linux-aarch64
macOS
Windows

mm318 · 2019-05-02T22:30:01Z

Windows

thomas-moulard · 2019-05-02T23:46:54Z

Linux
Linux-aarch64
macOS
Windows

mm318 · 2019-05-03T00:49:11Z

HI @wjwwood and @sloretz, can this be merged now based on the #171 (comment) CI run? We will have quick follow up to fix the warnings.

wjwwood · 2019-05-03T02:54:24Z

CI with (https://gist.githubusercontent.com/mm318/582ceeae9b2d2efc4c48bfa7c2b98f12/raw/cc86a231fd3f6c20c03e576290a1968fa97ef6c8/ros2.repos and --packages-up-to rcl and --packages-select rcl for testing):

Linux
Linux-aarch64
macOS
Windows

wjwwood · 2019-05-03T03:44:29Z

We're investigating the test failure on macOS, but they look like flaky (new) tests.

rutvih · 2019-05-03T03:51:28Z

@wjwwood what are your thoughts on disabling failing tests for now and our team submitting a separate PR for the fix so we can get this merged?

wjwwood · 2019-05-03T04:10:36Z

@rutvih I think we're going to do that, I just started a macOS only job to see if that works:

wjwwood · 2019-05-03T04:33:05Z

Ok, here's full CI:

Linux
Linux-aarch64
macOS
Windows

EDIT: The macOS and Windows jobs got disconnected and re-queued due to the router reboot...

I'll merge if it is green 🎉.

@mm318 can you open an issue about fixing that test we disabled, just so we don't loose track of it.

mm318 · 2019-05-03T04:34:21Z

@mm318 can you open an issue about fixing that test we disabled, just so we don't loose track of it.

Sure, will do.

EDIT: Tracked at ros2/rcl#431

dirk-thomas · 2019-05-03T05:06:39Z

We're investigating the test failure on macOS, but they look like flaky (new) tests.

How exactly is the failing test flaky? The tests were run with --retest-until-pass 10 and the error output contains:

> failed to resolve symbol 'rmw_get_serialized_message_size' in shared library

which doesn't sound flaky to me. Does it work for people in local testing?

I am fine disabling the new test for now - I would just like to be clear about why / how.

mm318 · 2019-05-03T05:17:04Z

That error output is unrelated to the test failures. rmw_fastrtps_cpp, rmw_fastrtps_dynamic_cpp, and rmw_opensplice_cpp all have rmw_get_serialized_message_size implemented, but rmw_connext_cpp/rmw_connext_shared_cpp does not (and implementing it is not supposed to be part of this set of PRs).

The test is flaky because it passes 90% of the time on @wjwwood's macOS laptop and actually only the RTI Connext version of the test kept failing on the buildfarm while the OpenSplice version did not.

wjwwood · 2019-05-03T05:20:08Z

Does it work for people in local testing?

Yes, for what ever reason that message only prints when it fails. I think it might be cached when they are pre-fetched, and only prints when the test fails.

That symbol was added in https://github.com/ros2/rmw_implementation/pull/51/files#diff-0e10897bf5aa86e527bbfbb0e10759e9R299 and not in rmw_connext's pr (https://github.com/ros2/rmw_connext/pull/353/files).

wjwwood · 2019-05-03T05:23:29Z

@mjcarroll fyi

wjwwood · 2019-05-03T06:34:44Z

Unfortunately the linters are now failing, at some point, around 4 hours ago, code was pushed to the rmw connext branch and it has a line that's too long, see: ros2/rmw_connext#352 (comment)

I cannot get this retested before nightlies start, so we'll have to re-evaluate in the morning.

mm318 · 2019-05-03T06:54:15Z

@wjwwood Would you be able to kick off CI to run overnight?

tfoote · 2019-05-03T07:36:12Z

Here's a retrigger of the builds

Linux
Linux-aarch64
macOS
Windows

emersonknapp · 2019-05-03T16:28:29Z

@tfoote I'm a bit confused about what we're seeing there - the linux aarch64 one is one that I've seen periodically that seems like it's a buildfarm quirk

projectroot.test_params_yaml
test_cli.test_params_yaml.xunit.missing_result

and the windows has a bunch of test failures for the CLI, which also seems unrelated to what we've done here - considering the previous builds did pass those tests?

mm318 · 2019-05-03T16:39:57Z

@wjwwood @sloretz Can we merge this based on these results? #171 (comment)

clalancette · 2019-05-03T16:45:10Z

projectroot.test_params_yaml
test_cli.test_params_yaml.xunit.missing_result

This one is actually a bug, but a long running, known bug: ros2/build_farmer#166 . So you can safely ignore that one.

wjwwood · 2019-05-03T17:15:58Z

We've decided to merge after looking at the CI manually. 👍

mm318 · 2019-05-03T17:18:45Z

Awesome! Thank you!

tfoote added the in review Waiting for review (Kanban column) label Mar 29, 2019

wjwwood reviewed Mar 29, 2019

View reviewed changes

dabonnie force-pushed the qos_reviewed branch from 9e70f87 to a13bbdd Compare April 1, 2019 21:35

nburek mentioned this pull request Apr 1, 2019

[ROS2 QoS meta-ticket] Dealine, Liveliness, and Lifespan ros2/ros2#680

Closed

29 tasks

mm318 mentioned this pull request Apr 2, 2019

Implementation for Deadline, Liveliness, and Lifespan QoS #167

Closed

dabonnie force-pushed the qos_reviewed branch 2 times, most recently from ff6246d to 074c30a Compare April 2, 2019 21:48

wjwwood reviewed Apr 3, 2019

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

rmw/cmake/get_rmw_typesupport.cmake Show resolved Hide resolved

nburek force-pushed the qos_reviewed branch from 1a23e12 to 13b7a41 Compare April 4, 2019 20:35

mm318 and others added 19 commits April 4, 2019 13:36

initial qos interface changes

7670c01

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

add creation of event waitable type

e80a7b8

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Add rmw take_event interface

d2b076d

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

add rmw event type enum

8402cfe

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Modify events type from void** to rmw_event_t**

67880eb

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

fix lint errors

6dbe51b

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Added RMW QoS Event Definitions

3c84836

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Added RMW QoS Event Definitions

92c3025

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Code formatting fixes

3fe8f64

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Change liveliness policy types to reflect actual settings

5c7a011

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>

Comment out types of liveliness that will not be supported in this it…

4a732ef

…eration of the feature 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, name to by_node

41dac48

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Fixed style issues

25db919

Removed unsupported QoS types Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Removed extra QoS types

d764efa

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

add an invalid enum in rmw_event_type_t for variables to default to

5957a92

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

change rmw_event_t APIs from create()/destroy() to init()/fini() pattern

a0966fe

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Fix uncrustify divergence in event.c

f43f029

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Remove erroneous rmw_delete_event

8987226

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com> Signed-off-by: Burek <burekn@f45c89c6612b.ant.amazon.com>

Merge in 'ros2/master'

93b740c

Signed-off-by: Miaofei <miaofei@amazon.com>

wjwwood merged commit e7ae806 into ros2:master May 3, 2019

wjwwood removed the in review Waiting for review (Kanban column) label May 3, 2019

Conversation

ross-desmond commented Mar 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dabonnie commented Apr 2, 2019

Uh oh!

wjwwood commented Apr 3, 2019

Uh oh!

wjwwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thomas-moulard commented May 2, 2019

Uh oh!

mm318 commented May 2, 2019

Uh oh!

thomas-moulard commented May 2, 2019

Uh oh!

wjwwood commented May 2, 2019

Uh oh!

emersonknapp commented May 2, 2019

Uh oh!

thomas-moulard commented May 2, 2019

Uh oh!

mm318 commented May 2, 2019

Uh oh!

thomas-moulard commented May 2, 2019

Uh oh!

mm318 commented May 3, 2019

Uh oh!

wjwwood commented May 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wjwwood commented May 3, 2019

Uh oh!

rutvih commented May 3, 2019

Uh oh!

wjwwood commented May 3, 2019

Uh oh!

wjwwood commented May 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mm318 commented May 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dirk-thomas commented May 3, 2019

Uh oh!

mm318 commented May 3, 2019

Uh oh!

wjwwood commented May 3, 2019

Uh oh!

wjwwood commented May 3, 2019

Uh oh!

wjwwood commented May 3, 2019

Uh oh!

mm318 commented May 3, 2019

Uh oh!

tfoote commented May 3, 2019

Uh oh!

emersonknapp commented May 3, 2019

Uh oh!

mm318 commented May 3, 2019

Uh oh!

clalancette commented May 3, 2019

Uh oh!

wjwwood commented May 3, 2019

Uh oh!

mm318 commented May 3, 2019

Uh oh!

ross-desmond commented Mar 29, 2019 •

edited

Loading

wjwwood commented May 3, 2019 •

edited

Loading

wjwwood commented May 3, 2019 •

edited

Loading

mm318 commented May 3, 2019 •

edited

Loading