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

[20134] Change serialize function default behaviour to omit the data representation #4131

Merged
merged 4 commits into from
Dec 18, 2023
Merged

[20134] Change serialize function default behaviour to omit the data representation #4131

merged 4 commits into from
Dec 18, 2023

Conversation

rsanchez15
Copy link
Contributor

@rsanchez15 rsanchez15 commented Dec 14, 2023 •
edited by EduPonz
Loading

The write operation in latest versions of Fast DDS calls the new serialize method in which the topic data representation is needed (serialize(void* data, SerializedPayload_t* payload, DataRepresentationId_t data_representation)). If the data type was generated with old versions of Fast DDS Gen, this function is not implemented in the type support so the write operation will fail. This PR updates the default behaviour of this function so it will call the old serialize method in case this one is not overridden.

Description

@Mergifyio backport 2.12.x

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • N/A: Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • N/A: New feature has been added to the versions.md file (if applicable).
  • N/A: New feature has been documented/Current behavior is correctly described in the documentation.
  • Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@rsanchez15
Refs #20134: Change serialize function default behaviour to omit the …
7c2d1fd 
…data representation

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
@rsanchez15 rsanchez15 added this to the v2.13.0 milestone Dec 14, 2023
@rsanchez15 rsanchez15 changed the title Change serialize function default behaviour to omit the data representation [20134] Change serialize function default behaviour to omit the data representation Dec 14, 2023
@rsanchez15
Refs #20134: Separate header and sources files for TopicDataType
a10299c 
Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
@rsanchez15
Refs #20134: Add deprecation note
685e4fc 
Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
EduPonz
EduPonz previously approved these changes Dec 14, 2023
View reviewed changes
@EduPonz
Copy link

EduPonz commented Dec 14, 2023

@richiprosima please test this

Tempate
Tempate previously approved these changes Dec 14, 2023
View reviewed changes
Copy link
Contributor

@Tempate Tempate left a comment

Choose a reason for hiding this comment

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

The tests in the https://github.com/eProsima/DDS-Router that were failing (https://github.com/eProsima/DDS-Router/actions/runs/7202566190/job/19620934800) pass with Fast-DDS on this branch and Fast-CDR on master.

@EduPonz EduPonz added the ci-pending PR which CI is running label Dec 14, 2023
@rsanchez15
Refs #20134: Uncrustify
d9316c1 
Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
@EduPonz
Copy link

EduPonz commented Dec 14, 2023

@richiprosima please test mac

1 similar comment
@EduPonz
Copy link

EduPonz commented Dec 15, 2023

@richiprosima please test mac

@EduPonz EduPonz added the no-test Skip CI tests if PR marked with this label label Dec 15, 2023
@rsanchez15 rsanchez15 dismissed stale reviews from Tempate and EduPonz via d9316c1 December 15, 2023 12:16
@EduPonz
Copy link

EduPonz commented Dec 15, 2023

@richiprosima please test this

@EduPonz EduPonz removed the no-test Skip CI tests if PR marked with this label label Dec 16, 2023
@EduPonz
Copy link

EduPonz commented Dec 16, 2023

@richiprosima please test mac

@EduPonz EduPonz merged commit 3625421 into master Dec 18, 2023
13 of 14 checks passed
@EduPonz EduPonz deleted the feature/defult-serialize branch December 18, 2023 06:11
@EduPonz
Copy link

EduPonz commented Dec 18, 2023

@Mergifyio backport 2.12.x

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 18, 2023 •
edited
Loading

backport 2.12.x

✅ Backports have been created

@mergify mergify bot mentioned this pull request Dec 18, 2023
Merged
EduPonz pushed a commit that referenced this pull request Dec 18, 2023
@rsanchez15 @EduPonz
Change serialize function default behaviour to omit the data represen…
6bc6419 
…tation (#4131)

* Refs #20134: Change serialize function default behaviour to omit the data representation

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Separate header and sources files for TopicDataType

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Add deprecation note

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Uncrustify

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

---------

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
(cherry picked from commit 3625421)
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz EduPonz mentioned this pull request Dec 18, 2023
Merged
7 tasks
EduPonz pushed a commit that referenced this pull request Dec 20, 2023
@rsanchez15 @EduPonz
Change serialize function default behaviour to omit the data represen…
d1a35f3 
…tation (#4131)

* Refs #20134: Change serialize function default behaviour to omit the data representation

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Separate header and sources files for TopicDataType

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Add deprecation note

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Uncrustify

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

---------

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
EduPonz added a commit that referenced this pull request Jan 11, 2024
@mergify @rsanchez15 @EduPonz
Change serialize function default behaviour to omit the data represen…
6fde8ac 
…tation (#4131) (#4154)

* Change serialize function default behaviour to omit the data representation (#4131)

* Refs #20134: Change serialize function default behaviour to omit the data representation

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Separate header and sources files for TopicDataType

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Add deprecation note

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

* Refs #20134: Uncrustify

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>

---------

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
(cherry picked from commit 3625421)
Signed-off-by: EduPonz <eduardoponz@eprosima.com>

* Refs #20055: Fix compilation of Fast DDS Python tests

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

---------

Signed-off-by: Raul Sanchez-Mateos <raul@eprosima.com>
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
Co-authored-by: Raul Sanchez-Mateos Lizano <raul@eprosima.com>
Co-authored-by: EduPonz <eduardoponz@eprosima.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EduPonz EduPonz EduPonz approved these changes

@Tempate Tempate Tempate left review comments

Assignees
No one assigned
Labels
ci-pending PR which CI is running
Projects
None yet
Milestone
v2.13.0
Development

Successfully merging this pull request may close these issues.
3 participants
@rsanchez15 @EduPonz @Tempate