TCPROS documentation doesn't match implementations.

asked 2019-10-15 18:32:45 -0600

Edwin Hayes gravatar image

updated 2019-10-16 15:25:22 -0600

A cursory look at the implementation of the C++ and python client libraries seems to suggest the handling of the TCPROS connection header is not exactly the same as what the documentation on the wiki suggests:

The wiki page: http://wiki.ros.org/ROS/TCPROS implies that provision of the message_definition field by a subscriber is mandatory (at least, in version 13 of the wiki page).

However, based on the implementation of validateHeader()in https://github.com/ros/ros_comm/blob/..., it appears that this field is not required in practice, because it is never checked. Further, the implementation also implies that the use of * as a wildcard value for the MD5 sum is supported, even though there seems to be no mention of this in the documentation for the protocol.

The implementation of rospy matches that of roscpp, so that's basically both the reference implementations different to the documentation. This makes client library development pretty difficult:

  • What behaviour should a new client library implement?
  • Why is message_definition required to be sent by a subscriber, if the value isn't used for anything?
  • Why was the wiki page changed from version 12->13, changing message_definition to mandatory?
  • If the protocol specify that the subscriber should send message_definition, why in actual implementations it's the publisher which replies with this instead?
  • Is it safe to assume that all publishers will send the message_definition field although they are not required to?
  • Thus, which fields in the headers are actually mandatory in order to maintain compatibility with all existing ROS implementations? Do some implementations follow the wiki more faithfully?
  • Is it actually safe for production code to use the wildcard behaviour?
edit retag flag offensive close merge delete

Comments

The wiki page: http://wiki.ros.org/ROS/TCPROS implies that provision of the message_definition field by a subscriber is mandatory [..] However, [..] it appears that this field is not required in practice, because it is never checked

I didn't write those specs, nor the implementations, but I'm not sure wiki/TCPROS explicitly states that message_definition should be checked. Only that it should be included in the header sent to a remote recipient.

Topics are statically typed. Combined with the MD5, you wouldn't necessarily need the full message definition to determine whether publishers and subscribers are compatible -- that is, assuming the MD5 is correctly calculated on the sending side. If an implementation would so desire, it could recalculate the MD5 on the receiving side and check whether it matches and be extra sure.

Message definitions should be included as it allows things like recording messages (such as rosbag ...(more)

gvdhoorn gravatar image gvdhoorn  ( 2019-10-16 01:34:13 -0600 )edit

.. definitions around.

Logistically it would become really difficult to process .bags (or look at serialised message traffic in general) without having all messages present on the system that is being used. A good example of this problem is rosjava/rosjava_bootstrap#16 (rosjava doesn't include the definition (or at least, not the full one) and so .bags recorded with messages originating from that client library are essentially useless without post-processing).

Further, the implementation also implies that the use of * as a wildcard value for the MD5 sum is supported, even though there seems to be no mention of this in the documentation for the protocol.

I believe * is used by topic_tools/ShapeShifter (here).

I don't know why that is not documented, but it ..

gvdhoorn gravatar image gvdhoorn  ( 2019-10-16 01:37:22 -0600 )edit

.. would seem like in general you wouldn't want people to use that value, as it would reduce the usefulness of the MD5 field (just put * everywhere and suddenly everything is compatible).

So that may be a deliberate omission, or a genuine oversight.

On the Publisher side in roscpp it's actually disallowed and enforced by code (here).

Is it actually safe for production code to use the wildcard behaviour?

I would say: yes, but only if you implement it for the reasons topic_tools/ShapeShifer does. Not to circumvent the purpose of the MD5.

gvdhoorn gravatar image gvdhoorn  ( 2019-10-16 01:42:30 -0600 )edit

And finally: seeing the context in which you (most likely) ask this question: there are quite a few alternative client libraries already available. If you're trying to integrate ROS systems into your own (non-ROS) system, it may be worthwhile to look for those to evaluate whether writing a new one would be necessary.

Additionally, rospy can be used without any other ROS dependencies on your system these days. See Experimental Python package index for ROS for that.

gvdhoorn gravatar image gvdhoorn  ( 2019-10-16 01:46:21 -0600 )edit

And I posted this as a comment as I can't give an authoritative answer here.

It's all based on my understanding of the code and runtime usage of the infrastructure.

gvdhoorn gravatar image gvdhoorn  ( 2019-10-16 07:03:26 -0600 )edit

@gvdhoorn - Thanks for that info. I might update my question with some more specific points. The need I'm trying to satisfy is for something with similar behaviour to the shapeshifter, accepting unknown messages at runtime, but require to use a language without very comprehensive client library support. So you can see how all this becomes relevant!

Edwin Hayes gravatar image Edwin Hayes  ( 2019-10-16 15:16:05 -0600 )edit

@Dirk Thomas wrote rev 12. It's unclear who unknownentity_1 is. @tfoote: any idea?

gvdhoorn gravatar image gvdhoorn  ( 2019-10-17 02:31:24 -0600 )edit

unknownentity_1 is an old user from before we were whitelisting users. Thus their username is a pretty good eponym. I suspect that that change was made without any discussion. One of the reasons we want to get away from a wiki.

tfoote gravatar image tfoote  ( 2019-10-17 15:46:38 -0600 )edit
add a comment