Skip to content

add to_yaml() function for C++ messages#523

Merged
dirk-thomas merged 10 commits intomasterfrom
dirk-thomas/cpp_to_yaml
Sep 10, 2020
Merged

add to_yaml() function for C++ messages#523
dirk-thomas merged 10 commits intomasterfrom
dirk-thomas/cpp_to_yaml

Conversation

@dirk-thomas
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas commented Sep 4, 2020

Closes #259. Instead of defining a << operator on the message this patch adds a to_yaml() function taking a message as well as an ostream. That allows to implement other formats like to_xml() side by side.

Atm all logic is in header files since rosidl_generator_cpp doesn't create a library.

CI builds testing rosidl_generator_cpp:

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

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas added the enhancement New feature or request label Sep 4, 2020
@dirk-thomas dirk-thomas self-assigned this Sep 4, 2020
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

I have left some minor comments, otherwise lgtm!


If in the future we want to support different serialization formats, we could use policy based design, where a serialize_to_x template function takes a policy providing overloads for the corresponding character_value_to_x, value_to_x functions.

In that way, different formats can be supported without the need of generating more code.

(edit) maybe some other things need to be added to the policy in order to be more general, but I think it's possible.

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

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@dirk-thomas
Copy link
Copy Markdown
Member Author

CI builds testing rosidl_generator_cpp:

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

@dirk-thomas dirk-thomas merged commit aac71e5 into master Sep 10, 2020
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/cpp_to_yaml branch September 10, 2020 17:49
@dabonnie dabonnie mentioned this pull request Sep 10, 2020
dabonnie added a commit to aws-ros-dev/rosidl that referenced this pull request Sep 10, 2020
inline void character_value_to_yaml(unsigned char value, std::ostream & out)
{
auto flags = out.flags();
out << "0x" << std::hex << std::setw(2) << std::setfill('0') << \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This causes a downstream build failure in rmw_dds_common

https://ci.ros2.org/job/ci_linux/12266/console

--- stderr: rmw_dds_common
15:49:21 In file included from /home/jenkins-agent/workspace/ci_linux/ws/build/rmw_dds_common/rosidl_generator_cpp/rmw_dds_common/msg/detail/gid__traits.hpp:9,
15:49:21                  from /home/jenkins-agent/workspace/ci_linux/ws/build/rmw_dds_common/rosidl_generator_cpp/rmw_dds_common/msg/gid.hpp:9,
15:49:21                  from /home/jenkins-agent/workspace/ci_linux/ws/src/ros2/rmw_dds_common/rmw_dds_common/include/rmw_dds_common/gid_utils.hpp:21,
15:49:21                  from /home/jenkins-agent/workspace/ci_linux/ws/src/ros2/rmw_dds_common/rmw_dds_common/src/gid_utils.cpp:21:
15:49:21 /home/jenkins-agent/workspace/ci_linux/ws/install/rosidl_runtime_cpp/include/rosidl_runtime_cpp/traits.hpp: In function ‘void rosidl_generator_traits::character_value_to_yaml(unsigned char, std::ostream&)’:
15:49:21 /home/jenkins-agent/workspace/ci_linux/ws/install/rosidl_runtime_cpp/include/rosidl_runtime_cpp/traits.hpp:33:35: error: ‘setw’ is not a member of ‘std’
15:49:21    33 |   out << "0x" << std::hex << std::setw(2) << std::setfill('0') << \
15:49:21       |                                   ^~~~
15:49:21 /home/jenkins-agent/workspace/ci_linux/ws/install/rosidl_runtime_cpp/include/rosidl_runtime_cpp/traits.hpp:33:51: error: ‘setfill’ is not a member of ‘std’; did you mean ‘fill’?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Possibly just needs the <iomanip> include https://en.cppreference.com/w/cpp/io/manip/setw

dabonnie added a commit to aws-ros-dev/rosidl that referenced this pull request Sep 10, 2020
This reverts commit aac71e5.

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
sloretz pushed a commit that referenced this pull request Sep 10, 2020
This reverts commit aac71e5.

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rosidl_generator_cpp] add <<operator for message classes

3 participants