Implement QoS: liveliness, deadline, lifespan

[ross-desmond]

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

[dabonnie]

@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]

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.

[mm318]

@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]

• Linux
• Linux-aarch64
• macOS
• Windows

[wjwwood]

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

[emersonknapp]

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]

• Linux
• Linux-aarch64
• macOS
• Windows

[mm318]

• Windows

[thomas-moulard]

• Linux
• Linux-aarch64
• macOS
• Windows

[mm318]

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]

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]

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

[rutvih]

@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]

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

[wjwwood]

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]

@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]

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]

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]

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]

@mjcarroll fyi

[wjwwood]

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]

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

[tfoote]

Here's a retrigger of the builds

• Linux
• Linux-aarch64
• macOS
• Windows

[emersonknapp]

@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]

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

[clalancette]

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]

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

[mm318]

Awesome! Thank you!