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

Expose cdr #267

Merged
merged 9 commits into from
Jun 16, 2018
Merged

Expose cdr #267

merged 9 commits into from
Jun 16, 2018

Conversation

Karsten1987
Copy link
Contributor

Connects to ros2/demos#185

@Karsten1987 Karsten1987 added the in progress Actively being worked on (Kanban column) label May 1, 2018
macro(serialize)
set(serialize_target_name "test_serialize${target}${target_suffix}")
ament_add_gtest(
${serialize_target_name} test/test_message_serialization.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

recommend indenting the arguments to ament_add_gtest()

TEST_F(CLASSNAME(TestMessageSerialization, RMW_IMPLEMENTATION), raw_callback) {
size_t counter = 0;

auto raw_callback = [&counter](const std::shared_ptr<rmw_message_raw_t> raw_msg) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this test will pass if raw_callback is never called. Maybe add EXPECT_GT(counter, 0u) after the loop at the bottom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in f6400e0


auto ret = RMW_RET_OK;
ret =
rmw_serialize(&bounded_array_nested_msg_c, message_c_typesupport, &raw_message_c);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this call would fit on the line above

EXPECT_EQ(RMW_RET_OK, ret);

print_raw_buffer(raw_message_c, "raw message c");
print_raw_buffer(raw_message_cpp, "raw message cpp");
Copy link
Contributor

Choose a reason for hiding this comment

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

Are raw_message_c.buffer and raw_message_cpp.buffer expected to be identical?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this might be RMW implementation dependent.

Karsten1987 and others added 2 commits June 14, 2018 02:09
* raw->serialized

* use size_t
@Karsten1987 Karsten1987 merged commit 6838ed3 into master Jun 16, 2018
@dirk-thomas dirk-thomas removed the in progress Actively being worked on (Kanban column) label Jun 16, 2018
@Karsten1987 Karsten1987 deleted the expose_cdr branch June 16, 2018 08:38
@dirk-thomas
Copy link
Member

This change might be the reason for the recently failing build: https://ci.ros2.org/job/ci_osx/3897/

@Karsten1987
Copy link
Contributor Author

but the static typesuppport branch has conflicts. Most likely due to my merge.
I believe the branch has to be rebased:
ros2/rmw_fastrtps#203

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants