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 XCDRv1 and XCDRv2 [19120] #156

Merged
merged 125 commits into from
Sep 7, 2023
Merged

Support XCDRv1 and XCDRv2 [19120] #156

merged 125 commits into from
Sep 7, 2023

Conversation

richiware
Copy link
Member

@richiware richiware commented Jul 4, 2023 •
edited
Loading

This PR adds support for XCDRv1 and XCDRv2 encoding, adding new functions like:

  • Cdr& begin_serialize_type(Cdr::state& current_state, EncodingAlgorithmFlag type_encoding)
  • Cdr& end_serialize_type(Cdr::state& current_state)
  • Cdr& deserialize_type(EncodingAlgorithmFlag type_encoding, std::function<bool (Cdr&, const MemberId&)> functor)

Also it adds support for optional members through the new class eprosima::fastcdr::optional.

This PR breaks API with previoius commits.

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware richiware changed the title Support XCDRv1 and XCDRv2 Support XCDRv1 and XCDRv2 [19120] Jul 4, 2023
@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

JLBuenoLopez
JLBuenoLopez reviewed Jul 11, 2023
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Partial review. Mostly Cdr.h header file serialization declarations.

test/CMakeLists.txt Outdated Show resolved Hide resolved
cmake/testing/GoogleTest.cmake Outdated Show resolved Hide resolved
fastcdr.repos Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
JLBuenoLopez
JLBuenoLopez reviewed Jul 12, 2023
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Second partial review with the modified header files.

include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
src/cpp/CMakeLists.txt Show resolved Hide resolved
src/cpp/exceptions/BadOptionalAccessException.cpp Outdated Show resolved Hide resolved
include/fastcdr/xcdr/optional.hpp Outdated Show resolved Hide resolved
include/fastcdr/xcdr/optional.hpp Show resolved Hide resolved
include/fastcdr/xcdr/optional.hpp Outdated Show resolved Hide resolved
JLBuenoLopez

This comment was marked as outdated.

Sign in to view

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

JLBuenoLopez
JLBuenoLopez reviewed Jul 13, 2023
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Another partial review of the feature implementation. Only the tests to go! 💪

src/cpp/Cdr.cpp Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
src/cpp/Cdr.cpp Outdated Show resolved Hide resolved
JLBuenoLopez
JLBuenoLopez reviewed Jul 17, 2023
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Partial test suite review

test/SimpleTest.cpp Outdated Show resolved Hide resolved
test/xcdr/appendable.cpp Outdated Show resolved Hide resolved
test/xcdr/appendable.cpp Outdated Show resolved Hide resolved
test/xcdr/appendable.cpp Show resolved Hide resolved
test/xcdr/appendable.cpp Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

JLBuenoLopez
JLBuenoLopez suggested changes Jul 18, 2023
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Final part for the first review. Great work!

test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
include/fastcdr/Cdr.h Outdated Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
test/xcdr/basic_types.cpp Outdated Show resolved Hide resolved
test/CMakeLists.txt Outdated Show resolved Hide resolved
test/xcdr/optional.cpp Outdated Show resolved Hide resolved
test/xcdr/optional.cpp Outdated Show resolved Hide resolved
test/xcdr/optional.cpp Outdated Show resolved Hide resolved
include/fastcdr/detail/container_recursive_inspector.hpp Show resolved Hide resolved
@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiware
Refs #18687. Add tests for calculator
306c650 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Add doxygen for fixed_string
abfe9c4 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Add doxygen for CdrSizeCalculator
5a01e48 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Fix doxygen
73c44f0 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Fix in repos file
358ecf1 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
7df3d47 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
962842f 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Don't POP after PUSH(origin=0)
7599056 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
78371c6 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Add versions.md file
e13957d 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
5a5a280 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
ef678e5 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
f5c99de 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
31b98d9 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
55df9fd 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Fix compilation on Windows.
519aa0a 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Fix test on Mac.
d26c7d3 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suugestions
071e283 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suugestions
6ecd264 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
d67ab8a 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18687. Apply suggestions
10e9235 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@JLBuenoLopez JLBuenoLopez added this to the v2.0.0 milestone Sep 7, 2023
@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

JLBuenoLopez
JLBuenoLopez approved these changes Sep 7, 2023
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM

@JLBuenoLopez JLBuenoLopez merged commit 37d84c8 into master Sep 7, 2023
10 checks passed
@JLBuenoLopez JLBuenoLopez deleted the feature/xcdr branch September 7, 2023 12:49
This was referenced Sep 7, 2023
Merged
Merged
@EduPonz EduPonz mentioned this pull request Sep 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snosenzo snosenzo snosenzo left review comments

@JLBuenoLopez JLBuenoLopez JLBuenoLopez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@richiware @richiprosima @JLBuenoLopez @snosenzo