Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the MESSAGE_LOST event #581

Closed
wants to merge 13 commits into from

Conversation

MiguelCompany
Copy link
Collaborator

This replaces #580 and adds proper support for MESSAGE_LOST event.

The interfaces are already present in Fast-DDS, but the listener callback is currently never called.
This means the RMW will behave as if messages are never lost, until we implement on_sample_lost on Fast DDS v2.6.0.

clalancette and others added 5 commits February 9, 2022 21:38
Fast-DDS (and rmw_fastrtps_cpp) don't really support this right
now, but having it in the list here allows RViz2 to start up.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Looks good to me with green CI. Thanks @MiguelCompany !

@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@MiguelCompany
Copy link
Collaborator Author

@clalancette this test will fail unless we have a correct implementation for on_sample_lost on Fast DDS.

The only way to push this foward I can think of is to disable MESSAGE_LOST support on the CI, by putting this inside a #if !defined(BUILD_TESTING) with a // TODO: remove when on_sample_lost is correctly implemented by Fast DDS

Let me know what you think.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Copy link
Collaborator Author

Closed in favor of #583

@MiguelCompany MiguelCompany deleted the feature/message-lost branch February 14, 2022 10:03
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.

None yet

3 participants