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

fix size calculation for WStrings #23

Merged
merged 1 commit into from
May 4, 2019

Conversation

dirk-thomas
Copy link
Member

@dirk-thomas dirk-thomas commented May 3, 2019

Aiming to fix the rosbag2 test_cdr_converter test...

While ROS uses 2 bytes for a U16String FastRTPS operates on wstrings and therefore we need to count each character with sizeof(wchar_t)` bytes (instead of 2).

@dirk-thomas dirk-thomas added bug Something isn't working in progress Actively being worked on (Kanban column) labels May 3, 2019
@dirk-thomas dirk-thomas self-assigned this May 3, 2019
@dirk-thomas dirk-thomas force-pushed the dirk-thomas/fix-size-calc-wstring branch 3 times, most recently from da93715 to b14b895 Compare May 3, 2019 23:21
sloretz referenced this pull request May 4, 2019
* add WString support

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* fix type of string literal

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* check wstring_to_u16_string function

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* fix style

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* fix dll linkage warnings

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas changed the title [WIP] fix size calculation for WStrings fix size calculation for WStrings May 4, 2019
@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels May 4, 2019
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas force-pushed the dirk-thomas/fix-size-calc-wstring branch from 032fde8 to 12195c0 Compare May 4, 2019 00:26
@dirk-thomas dirk-thomas requested a review from Karsten1987 May 4, 2019 00:26
@dirk-thomas
Copy link
Member Author

And finally in review:

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

Copy link
Contributor

@Karsten1987 Karsten1987 left a comment

Choose a reason for hiding this comment

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

lgtm, with green CI.

I have also triggered another rosbag2 build which makes uses of this function to verify this change.
ros2/rosbag2#111

@dirk-thomas dirk-thomas merged commit 0fda3e5 into master May 4, 2019
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/fix-size-calc-wstring branch May 4, 2019 18:13
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label May 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants