ros2 / rmw_fastrtps

Implementation of the ROS Middleware (rmw) Interface using eProsima's Fast RTPS.
Apache License 2.0
155 stars 117 forks source link

Add support for data representation #756

Closed MiguelCompany closed 4 months ago

MiguelCompany commented 4 months ago

This fixes the failures mentioned here by implementing the new is_plain overload of Fast DDS type support interface.

It additionally sets the data representation policy of created DataReader and DataWriter entities to XCDR1, since it is what the type support is using.

MiguelCompany commented 4 months ago

@clalancette @ahcorde @MichaelOrlov FYI

This makes ROS 2 CI pass (I checked locally on both Windows and Ubuntu) with the current head of Fast DDS 2.14.x, making it possible to revert https://github.com/ros2/ros2/pull/1560

MiguelCompany commented 4 months ago

can we add a test ?

I extended the tests for get_datawriter_qos and get_datareader_qos in 6a4243e.

The new is_plain overload will be automatically tested by reverting https://github.com/ros2/ros2/pull/1560 (i.e. changing this to 2.14.x)

clalancette commented 4 months ago

Because of where we are in the release process for Jazzy, I don't think we should put this in at this time.

What I think we should do right now is to get this PR ready, but not merge it yet. That is, we keep both Rolling and Jazzy pinned to the known working commit for Fast-DDS for now. Once Fast-DDS has made a tag available with the breaking change in it, we merge this into Rolling and then backport to Jazzy (that will be for Jazzy patch release 1).

@marcoag FYI.

EduPonz commented 4 months ago

What I think we should do right now is to get this PR ready, but not merge it yet. That is, we keep both Rolling and Jazzy pinned to the known working commit for Fast-DDS for now. Once Fast-DDS has made a tag available with the breaking change in it, we merge this into Rolling and then backport to Jazzy (that will be for Jazzy patch release 1).

I think that is a sensible thing to do. Just by chance I'm working on releasing Fast DDS v2.14.1 between today and tomorrow. I'll keep you posted.

EduPonz commented 4 months ago

@clalancette @ahcorde In case you want to move this forward, we just released Fast DDS v2.14.1:

clalancette commented 4 months ago
  • Rolling PR:

Oh, I am going to close both of those. They can't possibly compile right now, because we don't have this PR in.

MiguelCompany commented 4 months ago
  • Rolling PR:

Oh, I am going to close both of those. They can't possibly compile right now, because we don't have this PR in.

They will compile, but tests for loaned messages will fail until this one is merged.

MiguelCompany commented 4 months ago

@clalancette Can we move forward with this?

fujitatomoya commented 4 months ago

CI:

ahcorde commented 4 months ago

https://github.com/Mergifyio backport jazzy

mergify[bot] commented 4 months ago

backport jazzy

✅ Backports have been created

* [#759 Add support for data representation (backport #756)](https://github.com/ros2/rmw_fastrtps/pull/759) has been created for branch `jazzy`