Update taking API documentation.

[hidmic]

Precisely what the title says. Closely related to #270.

[ivanpauno]

nit:

Suggested change

	* - Access to ROS messages is not synchronized.
	* - Access to the same ROS message is not synchronized.

I find that less confusing, but maybe it's just that I'm reading it incorrectly

[hidmic]

Yeah, I meant it in that sense. To say things and to say the same thing sounds equivalent to me, but I may be wrong.

[wjwwood]

Well things implies a group, whereas the same thing implies a specific instance.

I'd say the suggested change is worth doing. Or you could say:

Suggested change

	* - Access to ROS messages is not synchronized.
	* - Access to the ROS message is not synchronized.

Because in this case there's only one message interacting with this function.

[hidmic]

Fair enough.

[ivanpauno]

I think this comment is still valid

[hidmic]

Whoops! See 005ba40.

[wjwwood]

Well things implies a group, whereas the same thing implies a specific instance.

I'd say the suggested change is worth doing. Or you could say:

Suggested change

	* - Access to ROS messages is not synchronized.
	* - Access to the ROS message is not synchronized.

Because in this case there's only one message interacting with this function.

[hidmic]

Oof, this is getting big. On top of what we discussed, I added a few more clarifications as to what to expect when a logical/runtime error occurs.

[hidmic]

Another thing we should also document is if take (and publish) calls are: blocking or not, synchronous or not. OR leave that door open.

[ivanpauno]

blocking or not, synchronous or not. OR leave that door open.

publish depends on the rmw implementation now.

I think take is always non-blocking and synchronous.
Considering how we use it, I think that that should be an API requirement.

[hidmic]

I think take is always non-blocking and synchronous.

I agree with it being synchronous, but non-blocking is a tricky one. Well, it depends on how you define it. take will not wait for the queue to fill up, that for sure, and the documentation already says so. But it may have to contend with other threads for the reader queue (or the message loaning pool if any). Presumably for a limited amount of time, but the thread will get re-scheduled anyways.

I'd say we allow implementations to wait on locks, but we don't allow them to wait for external events (like a message arriving, or an empty message loaning pool getting refilled, effectively banning semaphores). Some implementation may choose to go full non-blocking, but that's a bit too much for all, I think.

[ivanpauno]

I agree with it being synchronous, but non-blocking is a tricky one. Well, it depends on how you define it. take will not wait for the queue to fill up, that for sure, and the documentation already says so. But it may have to contend with other threads for the reader queue (or the message loaning pool if any). Presumably for a limited amount of time, but the thread will get re-scheduled anyways.

I think that non-blocking isn't the same as lock-free, but maybe I'm wrong.
But yeah, I agree with your requirements above.

(edit) Mutex locking is considered a blocking operation, so I think you're right. It would be great to have a clear requirement in the doc block.

[hidmic]

See be1db39. Couldn't come up with a better subtitle than " Runtime behavior". I'm open to better naming.

[ivanpauno]

lgtm, great work @hidmic!!!

Maybe in the future, we can only have a single Thread-safety, Memory-Allocation, Runtime-Behavior paragraph for the whole "take data" API, and link to that single paragraph from the documentation of each function.
I'm not quite sure on how to do that in doxygen.

[ivanpauno]

I think this comment is still valid

[hidmic]

Alright, going in!