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

Extend rmw_serialized_message_t tests. #261

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Aug 14, 2020

Precisely what the title says. Follow-up after #258

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Copy link

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

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

LGTM, pending CI

Copy link
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Looks good except for two minor comments.

rmw_serialized_message_t serialized_message =
rmw_get_zero_initialized_serialized_message();
rcutils_allocator_t default_allocator = rcutils_get_default_allocator();
size_t serialized_message_size = 5lu;
Copy link
Contributor

Choose a reason for hiding this comment

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

constexpr

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 a5c92af.

EXPECT_EQ(1u << i, serialized_message.buffer[i]);
}

size_t serialized_message_new_size = 2 * serialized_message_size;
Copy link
Contributor

Choose a reason for hiding this comment

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

constexpr

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 a5c92af.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Contributor Author

hidmic commented Aug 14, 2020

CI up to rmw:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@hidmic
Copy link
Contributor Author

hidmic commented Aug 18, 2020

Going in!

@hidmic hidmic merged commit 5a38167 into master Aug 18, 2020
@hidmic hidmic deleted the hidmic/extend-serialized-message-tests branch August 18, 2020 14:10
ahcorde pushed a commit that referenced this pull request Oct 13, 2020
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
ahcorde pushed a commit that referenced this pull request Oct 13, 2020
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
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.

None yet

3 participants