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.

Sign up for GitHub

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

Jump to bottom

Support Connext 7+ #121

Merged
merged 1 commit into from
Sep 23, 2023
Merged

Support Connext 7+ #121

merged 1 commit into from
Sep 23, 2023

Conversation

asorbini
Copy link
Collaborator

The current version of rmw_connextdds does not support Connext 7+ because of an internal API change.

This PR wraps the "troubling" call with an #if checking macro RTI_DDS_VERSION_MAJOR, which is defined by the Connext Pro headers.

I have created a branch to backport the fix to humble but I noticed there seems to be a "backporting bot" now, so I'm not sure if I should create a separate PR, or if we can use the bot to automatically backport this one. I think the commit should apply cleanly on top of humble too.

@asorbini asorbini requested a review from clalancette May 22, 2023 20:35
@clalancette
Copy link
Contributor

I have created a branch to backport the fix to humble but I noticed there seems to be a "backporting bot" now, so I'm not sure if I should create a separate PR, or if we can use the bot to automatically backport this one. I think the commit should apply cleanly on top of humble too.

Yeah, you can just use the bot. Once this has been approved, CI has been run, and merged, you can ask the bot to backport it for you with a @Mergifyio backport iron humble.

clalancette
clalancette approved these changes May 22, 2023
View reviewed changes
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 reasonable to me with green CI.

@asorbini
Copy link
Collaborator Author

Thank you for the info, I'll try that after merging.

CI results:

  • Linux: Build Status
  • Windows: Build Status

Should I run macOS too?

@clalancette
Copy link
Contributor

Should I run macOS too?

No, but you should use https://ci.ros2.org/job/ci_launcher/ to make sure to run CI on our default platforms.

@asorbini
Conditional internal API access to support Connext 7+
8bd04d5 
Signed-off-by: Andrea Sorbini <asorbini@rti.com>
@asorbini asorbini force-pushed the asorbini/connextdds-7 branch from f8954d9 to 8bd04d5 Compare September 22, 2023 20:41
@asorbini
Copy link
Collaborator Author

asorbini commented Sep 22, 2023 •
edited
Loading

Coming back to this PR, and trying to get it merged. I rebased it on top of rolling, running CI:

  • Linux: Build Status
  • Windows: Build Status

ETA: I stopped the ci_linux-aarch64 plan since rmw_connextdds is not enabled on that platform.

@asorbini asorbini merged commit afa5055 into rolling Sep 23, 2023
@delete-merged-branch delete-merged-branch bot deleted the asorbini/connextdds-7 branch September 23, 2023 02:22
@asorbini asorbini mentioned this pull request Sep 25, 2023
Closed
asaba96 pushed a commit to PITT-MIT-IAC/rmw_connextdds that referenced this pull request Nov 6, 2023
@asorbini
Conditional internal API access to support Connext 7+ (ros2#121)
180ee7b 
Signed-off-by: Andrea Sorbini <asorbini@rti.com>
trubio-rti pushed a commit that referenced this pull request Aug 5, 2024
@asorbini @trubio-rti
backport: Conditional internal API access to support Connext 7+ (#121) (
5a56c90 
afa5055)

Signed-off-by: Andrea Sorbini <asorbini@rti.com>
Signed-off-by: Taxo Rubio <trubio@rti.com>
trubio-rti added a commit that referenced this pull request Aug 20, 2024
@trubio-rti @asorbini
Backport rmw callbacks implementation to Humble [ros2-73] (#157)
831e8b9 
* backport: Add support for listener callbacks (#76) (d4330cc)

* Add support for listener callbacks.

* Fix wrong debug assertion when converting DDS_Duration values

* Clarify interactions between count_unread_samples() and take_next()

* Notify on changed matched status

Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
Signed-off-by: Andrea Sorbini <asorbini@rti.com>
Signed-off-by: Taxo Rubio <trubio@rti.com>

* backport: Conditional internal API access to support Connext 7+ (#121) (afa5055)

Signed-off-by: Andrea Sorbini <asorbini@rti.com>
Signed-off-by: Taxo Rubio <trubio@rti.com>

* style: Fixed header inclusion order

Signed-off-by: Taxo Rubio <trubio@rti.com>

---------

Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
Signed-off-by: Andrea Sorbini <asorbini@rti.com>
Signed-off-by: Taxo Rubio <trubio@rti.com>
Co-authored-by: Andrea Sorbini <asorbini@rti.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@asorbini @clalancette