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

Accept any RMW implementation, not just the default #172

Merged
merged 1 commit into from
Jan 29, 2021

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Jan 28, 2021

The get_default_rmw_implementation macro contains the same functionality as is removed by this change, but it contains logic to fall back to any available RMW implementation if neither RMW_IMPLEMENTATION is specified nor the default is available.

The 'get_default_rmw_implementation' macro contains the same
functionality as is removed by this change, but it contains logic to
fall back to any available RMW implementation if neither
RMW_IMPLEMENTATION is specified nor the default is available.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay added enhancement New feature or request in review Waiting for review (Kanban column) labels Jan 28, 2021
@cottsay cottsay self-assigned this Jan 28, 2021
@cottsay
Copy link
Member Author

cottsay commented Jan 28, 2021

Here's some pretty broad CI jobs to check for regressions, but it should be noted that the crux of the problem that this change addresses can't be tested on ci.ros2.org because of the way the jobs are structured.

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

@clalancette
Copy link
Contributor

The idea here looks good to me, and solved the problem in my local testing. So I'm somewhat confident it will fix the underlying problem. It looks like the test_rmw_implementation test is failing for some reason, though.

@cottsay
Copy link
Member Author

cottsay commented Jan 28, 2021

It looks like the test_rmw_implementation test is failing for some reason, though.

I just suck at package selection arguments. Trying again:

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

@clalancette
Copy link
Contributor

@cottsay Once you merge this, can you please do a new release of rmw_implementation into Rolling? That should unblock the build of demo_nodes_cpp_native. Thanks.

@cottsay cottsay merged commit 5824f47 into master Jan 29, 2021
@delete-merged-branch delete-merged-branch bot deleted the cottsay/rmw_fallback branch January 29, 2021 19:12
@cottsay cottsay restored the cottsay/rmw_fallback branch January 29, 2021 19:15
@cottsay cottsay deleted the cottsay/rmw_fallback branch January 29, 2021 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants