Demo nodes for raw publish and subscribe #185

Karsten1987 · 2017-10-23T18:14:40Z

I am making this the top level PR to gather all related PRs. This is WIP but as it affects basically all packages down the stack, I open this PR for early visibility.

This PR (and all others) aim to expose the raw CDR stream through the ROS2 API. This enables to send a raw data stream directly to the wire and in return take the data without going through the serialization process.
This ultimately serves as the base for a future upcoming ROSbag implementation.

dirk-thomas · 2017-10-23T18:27:42Z

demo_nodes_cpp/src/topics/talker_raw.cpp

+  while (rclcpp::ok()) {
+    sprintf(raw_msg.buffer, "%c%c%c%c%c%c%c%c%s %d",
+      0x00, 0x01, 0x00, 0x00, 0x0f, 0x00, 0x00,0x00, "hello world", (++i));
+    std::cout << "Publishing: '" << raw_msg.buffer << "'" << std::endl;


Use printf.

copied from the existing talker example. It's outside of this PR, but I'll update the talker code accordingly.

The other demo code will be updated to use logging pretty soon so I think it's fine to just use printf in the new code without modifying existing one

@mikaelarguedas Just saw your comment. I'll rebase accordingly if necessary.

mikaelarguedas · 2017-10-23T19:11:54Z

demo_nodes_cpp/src/topics/talker.cpp

@@ -57,7 +57,7 @@ int main(int argc, char * argv[])

  while (rclcpp::ok()) {
    msg->data = "Hello World: " + std::to_string(i++);
-    std::cout << "Publishing: '" << msg->data << "'" << std::endl;
+    printf("Publishing: %s\n", msg->data.c_str());


If we modify this in this PR, it needs to print the exacte same message as before (with single quotes around the data). Otherwise the tests will fail because they expect them:

demos/demo_nodes_cpp/test/talker.txt

Line 1 in 0c2fa6b

Publishing: 'Hello World: 1'

Karsten1987 · 2018-03-06T22:47:36Z

I think this is up for review.

As I believe this is a quite huge PR, I'll explain the details on each referenced PR if necessary.

CI:
linux:
osx:
win: (unrelated)

wjwwood · 2018-03-08T00:38:59Z

demo_nodes_cpp/src/topics/listener_raw.cpp

@@ -0,0 +1,95 @@
+// Copyright 2014 Open Source Robotics Foundation, Inc.


wjwwood · 2018-03-08T00:40:39Z

demo_nodes_cpp/src/topics/talker_raw.cpp

+      [this]() -> void
+      {
+        rcutils_snprintf(raw_msg_.buffer, raw_msg_.buffer_length, "%c%c%c%c%c%c%c%c%s %zu",
+          0x00, 0x01, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00, "Hello World:", (count_++));


I think this deverse a comment explaining that this is basically manual serialization of a string to CDR, and/or a TODO to replace this with a call to the serialization API which is forthcoming (I assume).

Also, the () around count++ are redundant.

wjwwood · 2018-03-08T02:31:05Z

demo_nodes_cpp/src/topics/talker_raw.cpp

+  : Node("raw_talker")
+  {
+    raw_msg_.buffer_length = 24;
+    raw_msg_.buffer = new char[raw_msg_.buffer_length];


This needs to be deleted in the destructor of the class.

While the delete in the destructor is sufficient for the demo code to be "correct" I would suggest to use a smart pointer. Otherwise this might encourage people to copy-n-paste this code which is "incomplete" without a copy constructor and assignment operator.

agreed, a smart pointer would be nice, but I don't think it's more user friendly. I would rather say it's even more error prone to make copy-paste mistakes as you can't simply instantiate a shared ptr with that c-struct. You have to pass a custom deleter function / callable object with the smart pointer to clean up correctly. Does this make sense?

A std::unique_ptr is specialized for arrays and should be fine to use (as of C++11) without a custom deleter afaik (see http://en.cppreference.com/w/cpp/memory/unique_ptr).

wjwwood · 2018-03-21T21:59:04Z

@Karsten1987 Should/can this be put into "in progress"? I have outstanding comments in ros2/rmw_fastrtps#186 and ros2/rmw_connext#259 (at least I think so) and this has stayed in review for several days. It would help me to put it in progress until issues are addressed so that I don't have to reevaluate its state each time I go through waffle to check for needed reviews.

wjwwood · 2018-06-15T05:57:46Z

I've re-reviewed all the pull requests and so here's some CI:

Linux
Linux-aarch64
macOS
Windows

Karsten1987 · 2018-06-15T15:27:59Z

linux:
osx:
aarch:
win:

I further opened a PR against rmw_opensplice_cpp which implements the missing functions and sets the error message accordingly when calling them.

wjwwood · 2018-06-15T23:47:54Z

Rerun of Windows CI after ros2/rmw_connext@8eaeb9e:

wjwwood · 2018-06-16T01:49:25Z

Rerun of Windows CI after ros2/rmw_connext@f778624:

wjwwood · 2018-06-16T04:43:04Z

Rerun of Windows CI after ros2/rmw_connext@27cd763:

Karsten1987 · 2018-06-16T08:37:38Z

So I merged this!
Thanks @wjwwood for finishing up the last hiccup on Windows! Appreciated!

Karsten1987 added the in progress Actively being worked on (Kanban column) label Oct 23, 2017

Karsten1987 self-assigned this Oct 23, 2017

This was referenced Oct 23, 2017

Expose raw CDR stream for publish and subscribe ros2/rmw_connext#259

Merged

*_raw function ros2/rmw_implementation#31

Merged

*_raw function ros2/rmw#125

Merged

*_raw function ros2/rcl#170

Merged

*_raw function ros2/rclcpp#388

Merged

dirk-thomas reviewed Oct 23, 2017

View reviewed changes

mikaelarguedas reviewed Oct 23, 2017

View reviewed changes

Karsten1987 force-pushed the expose_cdr branch from 5f84510 to a778906 Compare November 14, 2017 23:38

Karsten1987 mentioned this pull request Jan 20, 2018

Expose raw CDR stream for publish and subscribe ros2/rmw_fastrtps#186

Merged

Karsten1987 force-pushed the expose_cdr branch 5 times, most recently from 5f86772 to 4421d11 Compare January 20, 2018 00:29

dirk-thomas mentioned this pull request Jan 29, 2018

Ros 2 typesupport for rosbag:: MessageInstance class ros2/rosidl_typesupport#18

Closed

Karsten1987 force-pushed the expose_cdr branch from 3f5a448 to 23b7be3 Compare March 6, 2018 18:28

Karsten1987 added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Mar 6, 2018

wjwwood approved these changes Mar 8, 2018

View reviewed changes

wjwwood reviewed Mar 8, 2018

View reviewed changes

mikaelarguedas added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Mar 22, 2018

This was referenced Apr 11, 2018

Send and Receive Serialized Data ADLINK-IST/opensplice#50

Closed

Ability to serialize messages without publishing ros2/rclcpp#462

Closed

This was referenced May 1, 2018

FastBuffer::resize will not resize below 200 eProsima/Fast-CDR#15

Closed

Expose cdr ros2/system_tests#267

Merged

wjwwood added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 14, 2018

wjwwood approved these changes Jun 14, 2018

View reviewed changes

suppress unused lambda capture warning on macOS

3edd5e5

This was referenced Jun 14, 2018

proposed changes to expose_cdr pr's to use size_t ros2/rmw#139

Merged

use size_t in expose_cdr prs #243

Merged

use size_t ros2/rclcpp#497

Merged

use size_t ros2/rmw_connext#289

Merged

clalancette assigned wjwwood Jun 14, 2018

wjwwood mentioned this pull request Jun 14, 2018

use size_t ros2/rmw_fastrtps#206

Merged

dhood added the in progress Actively being worked on (Kanban column) label Jun 14, 2018

clalancette removed the in review Waiting for review (Kanban column) label Jun 14, 2018

wjwwood removed their assignment Jun 14, 2018

use size_t (#243)

3303236

Karsten1987 mentioned this pull request Jun 14, 2018

use size_t ros2/system_tests#283

Merged

wjwwood added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 14, 2018

wjwwood approved these changes Jun 15, 2018

View reviewed changes

Karsten1987 mentioned this pull request Jun 15, 2018

gracefully fail when using serialized functions ros2/rmw_opensplice#228

Merged

Karsten1987 merged commit b026e52 into master Jun 16, 2018

dirk-thomas removed the in review Waiting for review (Kanban column) label Jun 16, 2018

Karsten1987 deleted the expose_cdr branch June 16, 2018 08:37

Karsten1987 mentioned this pull request Jun 16, 2018

comply with new rclcpp subscription templates ros2/realtime_support#64

Merged

josephduchesne mentioned this pull request Oct 8, 2018

[ros2topic] enable ros2topic hz delay bw command ros2/ros2cli#132

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Demo nodes for raw publish and subscribe #185

Demo nodes for raw publish and subscribe #185

Karsten1987 commented Oct 23, 2017 •

edited

Loading

dirk-thomas Oct 23, 2017

Karsten1987 Oct 23, 2017

mikaelarguedas Oct 23, 2017

Karsten1987 Oct 23, 2017

mikaelarguedas Oct 23, 2017

Karsten1987 commented Mar 6, 2018

wjwwood Mar 8, 2018

wjwwood Mar 8, 2018

wjwwood Mar 8, 2018

wjwwood Mar 8, 2018

dirk-thomas Mar 8, 2018

Karsten1987 Mar 8, 2018

dirk-thomas Mar 8, 2018

wjwwood commented Mar 21, 2018

wjwwood commented Jun 15, 2018 •

edited

Loading

Karsten1987 commented Jun 15, 2018

wjwwood commented Jun 15, 2018

wjwwood commented Jun 16, 2018

wjwwood commented Jun 16, 2018

Karsten1987 commented Jun 16, 2018

		@@ -0,0 +1,95 @@
		// Copyright 2014 Open Source Robotics Foundation, Inc.

Demo nodes for raw publish and subscribe #185

Demo nodes for raw publish and subscribe #185

Conversation

Karsten1987 commented Oct 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karsten1987 commented Mar 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjwwood commented Mar 21, 2018

wjwwood commented Jun 15, 2018 • edited Loading

Karsten1987 commented Jun 15, 2018

wjwwood commented Jun 15, 2018

wjwwood commented Jun 16, 2018

wjwwood commented Jun 16, 2018

Karsten1987 commented Jun 16, 2018

Karsten1987 commented Oct 23, 2017 •

edited

Loading

wjwwood commented Jun 15, 2018 •

edited

Loading