Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation clarification on ROS2 zenoh-pico demo #7

Open
Entropy512 opened this issue Feb 3, 2023 · 7 comments
Open

Documentation clarification on ROS2 zenoh-pico demo #7

Entropy512 opened this issue Feb 3, 2023 · 7 comments

Comments

@Entropy512
Copy link

I see that the demo is serializing Twist messages as little-endian - the serialization approach looks consistent with CDR, except that if I'm reading the spec correctly, a CDR encapsulation is supposed to start with an octet that indicates endianness. This octet appears to be missing from the serialized message.

Little-endian is also not network byte order, which is what I would expect absent any other convention - where does the little-endian convention originate from?

@cguimaraes
Copy link
Member

If I remember correctly, the endianness is set on the 2nd octet.
0 to big endian and 1 to little endian.

void serialize_twist(Twist *t, char *buf)
{
    // Serialize Twist header for little endian
    *(buf++) = 0x00;
    *(buf++) = 0x01;
    *(buf++) = 0x00;
    *(buf++) = 0x00;
    buf = serialize_vector3(&t->linear, buf);
    buf = serialize_vector3(&t->angular, buf);
}

@Entropy512
Copy link
Author

Hmm, I may have been mislead by reading the wrong section of the CDR specification. The document is a little bit opaque...

In fact, it looks almost like if you go straight to the base CDR spec (section 15.3 of https://www.omg.org/cgi-bin/doc?formal/02-06-51 ), you wind up being led in the wrong direction...

It looks like if you go instead down the rabbit hole starting from Section 10.5 of https://www.omg.org/spec/DDSI-RTPS/2.3/PDF - that byte stream does indicate little endian using "Classic CDR", referencing clause 7.4.1 of https://www.omg.org/spec/DDS-XTypes/1.3/PDF

I would have assumed that particular header format was specific to RTPS and thus too low-level for Zenoh, but it appears that assumption was incorrect.

Thanks for the info. I may submit a pull request this weekend adding a comment above that code that has references to the relevant specs and detail as to the exact meaning of the header.

@JEnoch
Copy link
Member

JEnoch commented Feb 3, 2023

The CDR specification (§15.3 of CORBA spec) supports both endianness, but the way it's encoded as a flag in a message is not specified here.

  • For CORBA it's specified as a bool flag (byte_order) in GIOP Message Header (§15.4.1 of CORBA spec)
  • While for DDSI that reuses the CDR spec, it's specified in the SerializedPayloadHeader.representation_identifier (§10 of DDSI spec)

The zenoh-bridge-dds you make zenoh-pico communicate with acts at DDSI level, receiving Serialized Payloads for User-defined DDS Topics (§10.5) from DDS Writers. On the other way, it expects Serialized Payloads from Zenoh publishers to be routed to DDS Readers.

Wrt. choice of endianness: the network byte order doesn't need to come in consideration since no component of the network stack will try to deserialize this payload. Only the DDS applications (and the zenoh-pico app) will. Thus, better to use the platform endianness. And most are little-endian (or bi-endian) nowadays.

@Entropy512
Copy link
Author

Thanks for the additional clarification. I'll try to add that as a comment to the code this weekend in a PR, unless you feel there's a better location for such documentation.

@JEnoch
Copy link
Member

JEnoch commented Feb 3, 2023

Adding such comment in the code will be very fine! Moreover we love to get PRs from contributors/users 😄

Note that you would need to to create an account on eclipse.org (with the same email that you use for commits) and to sign the ECA. Thanks!

@cguimaraes
Copy link
Member

Related to #8

@Entropy512
Copy link
Author

So I didn't get around to PRing some additional commenting last weekend - this month is a bit hectic. I'll try to get around to it two weekends from now (next weekend is really bad for me)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants