Update publishing API documentation. #270

hidmic · 2020-09-01T16:16:59Z

Precisely what the title says.

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

rmw/include/rmw/rmw.h

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

hidmic · 2020-09-02T15:27:21Z

Alright, I think this is now complete. PTAL !

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

hidmic · 2020-09-02T15:52:48Z

Ok, now it is ready. A couple things were either not constrained enough or not generic enough.

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

hidmic · 2020-09-02T16:16:01Z

rmw/include/rmw/rmw.h

- * \param[in] publisher The publisher object registered to send the message.
- * \param[in] serialized_message The serialized message holding the byte stream.
- * \param[in] allocation Specify preallocated memory to use (may be NULL).
+ * Send a ROS message serialized in a byte stream directly over the wire to


What about implementations that do not use serialization (e.g. shared memory-based ones)? Do we allow them to give back the ROS message as a blob, with no changes?

I think in those cases they can implement "serialize/deserialize" as a noop, and publish/taking the serialized message would be the same that the non-serialized one.

(edit) more than a noop, the serialized message would be an exact copy of the original one. I think that's what you meant with "give back the ROS message as a blob" now that I read again.

more than a noop, the serialized message would be an exact copy of the original one.

Exactly. I didn't think we should make any promises either, but I wanted to make sure others agree.

I would just say "publish a serialized ROS message", because several things could occur at this point:

it could go to the wire

it could need to be deserialized and converted to a new type (DDS type maybe if serialized publishing is not supported properly)

it could be sent over shared memory or within the process as a serialized blob

other things possibly...

So I'd just say "published" where that means "delivered to all matched subscriptions", without saying more than that.

ivanpauno

lgtm

rmw/include/rmw/rmw.h

ivanpauno · 2020-09-02T17:40:48Z

rmw/include/rmw/rmw.h

- * \param[in] publisher The publisher object registered to send the message.
- * \param[in] serialized_message The serialized message holding the byte stream.
- * \param[in] allocation Specify preallocated memory to use (may be NULL).
+ * Send a ROS message serialized in a byte stream directly over the wire to


I think in those cases they can implement "serialize/deserialize" as a noop, and publish/taking the serialized message would be the same that the non-serialized one.

(edit) more than a noop, the serialized message would be an exact copy of the original one. I think that's what you meant with "give back the ROS message as a blob" now that I read again.

wjwwood

lgtm other than the one comment thread

wjwwood · 2020-09-02T19:10:43Z

rmw/include/rmw/rmw.h

- * \param[in] publisher The publisher object registered to send the message.
- * \param[in] serialized_message The serialized message holding the byte stream.
- * \param[in] allocation Specify preallocated memory to use (may be NULL).
+ * Send a ROS message serialized in a byte stream directly over the wire to


I would just say "publish a serialized ROS message", because several things could occur at this point:

it could go to the wire

it could need to be deserialized and converted to a new type (DDS type maybe if serialized publishing is not supported properly)

it could be sent over shared memory or within the process as a serialized blob

other things possibly...

So I'd just say "published" where that means "delivered to all matched subscriptions", without saying more than that.

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

wjwwood

lgtm, thanks this is really a great improvement!

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

hidmic · 2020-09-03T16:32:21Z

Alright, that should be it.

ivanpauno

lgtm

ivanpauno · 2020-09-04T17:11:09Z

rmw/include/rmw/rmw.h

+ *
+ * \pre Given `publisher` must be a valid publisher, as returned by rmw_create_publisher().
+ * \pre Given `type_support` must be a valid `rosidl` message type support, matching the
+ *   one registered with the `publisher` on creation.


unrelated, but it's weird that we're passing the typesupport again here when the publisher already stores one internally.

Yeap, it is weird.

hidmic · 2020-09-04T20:31:19Z

Going in!

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

Update publishing API documentation.

df17cde

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

hidmic requested review from wjwwood, clalancette and ivanpauno September 1, 2020 16:16

wjwwood reviewed Sep 1, 2020

View reviewed changes

rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved

rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved

rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved

rmw/include/rmw/rmw.h Outdated Show resolved Hide resolved

hidmic added 2 commits September 1, 2020 17:27

Address peer review comments.

f01962a

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

Iterate again on publish API docs.

8386b7b

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

hidmic requested a review from wjwwood September 2, 2020 15:27

hidmic added 2 commits September 2, 2020 12:39

Constrain publisher allocation usage.

526e7e7

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

Do not mention serialization on publish.

7ced171

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

Ensure consistent writing style.

b2062fe

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

hidmic commented Sep 2, 2020

View reviewed changes

hidmic mentioned this pull request Sep 2, 2020

Update taking API documentation. #271

Merged

ivanpauno approved these changes Sep 2, 2020

View reviewed changes

wjwwood approved these changes Sep 2, 2020

View reviewed changes

Address peer review comments.

dbf1312

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

wjwwood approved these changes Sep 2, 2020

View reviewed changes

hidmic added 3 commits September 2, 2020 17:03

Refer to argument types in singular.

0c0dfa2

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

Assume publisher allocations are not thread-safe.

74bab53

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

Document runtime behavior.

811fe70

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

hidmic requested review from wjwwood and ivanpauno September 3, 2020 16:32

ivanpauno mentioned this pull request Sep 3, 2020

Standardize thread-safety documentation ros2/ros2#1004

Open

ivanpauno approved these changes Sep 4, 2020

View reviewed changes

hidmic merged commit 2d6ea69 into master Sep 4, 2020

delete-merged-branch bot deleted the hidmic/update-publish-docs branch September 4, 2020 20:31

Lobotuerk mentioned this pull request Sep 8, 2020

Add tests non-implemented functions + loan bad arguments ros2/rmw_implementation#122

Merged

ahcorde pushed a commit that referenced this pull request Oct 13, 2020

Update publishing API documentation. (#270)

bbb569a

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

ahcorde pushed a commit that referenced this pull request Oct 13, 2020

Update publishing API documentation. (#270)

ba66702

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update publishing API documentation. #270

Update publishing API documentation. #270

hidmic commented Sep 1, 2020

hidmic commented Sep 2, 2020

hidmic commented Sep 2, 2020

hidmic Sep 2, 2020 •

edited

Loading

ivanpauno Sep 2, 2020 •

edited

Loading

hidmic Sep 2, 2020

wjwwood Sep 2, 2020

ivanpauno left a comment

ivanpauno Sep 2, 2020 •

edited

Loading

wjwwood left a comment

wjwwood Sep 2, 2020

wjwwood left a comment

hidmic commented Sep 3, 2020

ivanpauno left a comment

ivanpauno Sep 4, 2020 •

edited

Loading

hidmic Sep 4, 2020

hidmic commented Sep 4, 2020

Update publishing API documentation. #270

Update publishing API documentation. #270

Conversation

hidmic commented Sep 1, 2020

hidmic commented Sep 2, 2020

hidmic commented Sep 2, 2020

hidmic Sep 2, 2020 • edited Loading

Choose a reason for hiding this comment

ivanpauno Sep 2, 2020 • edited Loading

Choose a reason for hiding this comment

hidmic Sep 2, 2020

Choose a reason for hiding this comment

wjwwood Sep 2, 2020

Choose a reason for hiding this comment

ivanpauno left a comment

Choose a reason for hiding this comment

ivanpauno Sep 2, 2020 • edited Loading

Choose a reason for hiding this comment

wjwwood left a comment

Choose a reason for hiding this comment

wjwwood Sep 2, 2020

Choose a reason for hiding this comment

wjwwood left a comment

Choose a reason for hiding this comment

hidmic commented Sep 3, 2020

ivanpauno left a comment

Choose a reason for hiding this comment

ivanpauno Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

hidmic Sep 4, 2020

Choose a reason for hiding this comment

hidmic commented Sep 4, 2020

hidmic Sep 2, 2020 •

edited

Loading

ivanpauno Sep 2, 2020 •

edited

Loading

ivanpauno Sep 2, 2020 •

edited

Loading

ivanpauno Sep 4, 2020 •

edited

Loading