Update graph API documentation.

[hidmic]

Precisely what the title says. First pass.

[hidmic]

We need to come up with a way to document object thread-safety and not just that of functions. This is way more duplication I'm comfortable with.

[clalancette]

We need to come up with a way to document object thread-safety and not just that of functions.

This is something of a philosophical debate, honestly. In some sense, there is no such thing as a thread-safe object; thread safety only comes into play when accessing data, and that can only be done through functions or methods.

I think your main concern here is the duplication of this paragraph down below. It seems to me that that can mostly be resolved by pointing the reader to the documentation about the node API, which presumably talks about that.

[hidmic]

This is something of a philosophical debate, honestly. In some sense, there is no such thing as a thread-safe object; thread safety only comes into play when accessing data, and that can only be done through functions or methods.

Sure, but what is an object if not state (data) coupled with behavior (functions)? At any rate, I think ros2/ros2#1004 would benefit from your POV.

It seems to me that that can mostly be resolved by pointing the reader to the documentation about the node API, which presumably talks about that.

Do we have node API documentation? Or are you talking about something that we should have at some point?

[wjwwood]

We can have higher level sections in doxygen (using the file for example), or we can document the struct, like I do with context:

https://github.com/ros2/rcl/blob/8bcada7b884693644d7ba2e3bd2c22fd9541eba7/rcl/include/rcl/context.h#L43-L143

[hidmic]

Any objections to proceed without relocating documentation? For the sake of speed, and to keep it consistent with previous API spec updates that do things this way.

[wjwwood]

Fine with me.

[hidmic]

We need to come up with a way to document whole features and abstractions/concepts like the ROS graph.

[clalancette]

Agreed. Two places I can think to put it:

1. In a doc subdirectory at the top-level of this repository. That's nice in that it keeps it close to where it is "defined", but it also buries the concept down in an essentially "detail" package.
2. At index.ros.org or docs.ros2.org .

[hidmic]

I'd be inclined towards having it here, in this repository, close to the API specifications that rely on it. External web sites can then consume that content.

[wjwwood]

I think here is fine, but it depends on the purpose. If it is to make this API clear, then here, but it's goal is to explain the concept to users, then higher up the stack is more appropriate, but some details will not, like the thread-safety of this API, as an example. Because higher level APIs may give different thread-safety guarantees and the users are more likely to use those interfaces.

[hidmic]

If it is to make this API clear, then here, but it's goal is to explain the concept to users, then higher up the stack is more appropriate

Hmm, higher up the stack where? IMHO we cannot use terms and concepts in API documentation without an explanation or a link to an explanation. Besides, isn't the ROS middleware interface the best place to introduce ROS middleware concepts (barring tutorials and conceptual overviews that we can have elsewhere for educational purposes)?

[wjwwood]

Yes, I was just saying that tutorials and conceptual guides (things actual users will need) shouldn't depend on this except obliquely.

[hidmic]

These APIs expect the same allocator to be passed around to not hit UB, presumably to spare storage. I wonder if these shouldn't be explicitly tagged for internal implementation use only.

[hidmic]

Also, documentation duplication makes me cry.

[wjwwood]

They really should store the allocator given during init and reuse it during fini rather than having the user pass it...

[wjwwood]

That is to say, I agree with you.

[clalancette]

I've got a bunch of comments in here.

I think there are 2 major things that this has pointed out.

The first is that there is a lot of duplication in the comments. Partly this is because the API could be improved. One example is rmw_get_node_names() and rmw_get_node_names_with_enclaves(). In my opinion, there is no reason to have both of those APIs. Instead we could have rmw_get_node_names take an enclave argument, and don't attempt to fill in enclave information if it is NULL. Partly the duplication is because we are missing some place to point to that has higher-level documentation about concepts.

The second major point is that we need a place to put higher-level documentation about concepts. My first inclination would be to start putting some conceptual documents at https://index.ros.org/doc/ros2/Tutorials/ or http://docs.ros2.org/foxy/ , but I'm not sure.

I honestly think neither is blocking; I would go with duplication for now, and we can review both of them later. But if we do that, then we should open up issues and/or add roadmap items to do so.

[clalancette]

Agreed. Two places I can think to put it:

1. In a doc subdirectory at the top-level of this repository. That's nice in that it keeps it close to where it is "defined", but it also buries the concept down in an essentially "detail" package.
2. At index.ros.org or docs.ros2.org .

[clalancette]

We need to come up with a way to document object thread-safety and not just that of functions.

This is something of a philosophical debate, honestly. In some sense, there is no such thing as a thread-safe object; thread safety only comes into play when accessing data, and that can only be done through functions or methods.

I think your main concern here is the duplication of this paragraph down below. It seems to me that that can mostly be resolved by pointing the reader to the documentation about the node API, which presumably talks about that.

[hidmic]

Comments addressed, PTAL!

[gbiggs]

@clalancette Regarding writing a higher level concepts document, "write a tutorial on how to write an RMW implementation" is one of the outputs we planned for the Zenoh work. I'm happy to help out with writing the higher-level concepts document, provided someone with more experience with the API is willing to review it in detail.

[hidmic]

FYI I'll start writing tests and adapting implementation to comply with this. We can sort out where to put documentation later on.

[wjwwood]

lgtm, it's a great improvement!

I commented on most of the conversations, but you guys picked out everything I noticed already, so no extra stuff from me I think.

[hidmic]

Alright, going in!