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

How to use cyclonedds cdrstream #1963

Closed
uupks opened this issue Apr 2, 2024 · 7 comments
Closed

How to use cyclonedds cdrstream #1963

uupks opened this issue Apr 2, 2024 · 7 comments

Comments

@uupks
Copy link

uupks commented Apr 2, 2024

I want to use zenoh-pico application to communicate with ROS 2 nodes based on zenoh-bridge-ros2dds, but I encountered some problems. There is the zenoh-pico-example.

  • add_two_ints_client
Sending Query 'add_two_ints'... with a = 123456, b = 654321
0 0X1 0 0 0X10 0 0 0 0X40 0XE2 0X1 0 0 0 0 0 0XF1 0XFB 0X9 0 0 0 0 0 
>> Received ('add_two_ints': 3340526778581008)


client send reqeust with a = 123456(0x01_E240), b = 654321(0x09_FBF1)

while server received request with a = 530239482494992(0x01_E240_0000_0010), b = 2810287296086016(0x09_FBF1_0000_0000)

client received response with sum = 3340526778581008(0x0B_DE31_0000_0010), but actual sum = 777777(0x0B_DE31)
  • listener
dds_is_get1: Assertion `is->m_index < is->m_size' failed.

It looks like there is something wrong with cdrstream serialization.

@uupks
Copy link
Author

uupks commented Apr 2, 2024

we can also have a discussion at eclipse-zenoh/zenoh-plugin-ros2dds#70

@uupks
Copy link
Author

uupks commented Apr 3, 2024

typedef struct example_interfaces_srv_AddTwoInts_Request
{
  int64_t a;
  int64_t b;
} example_interfaces_srv_AddTwoInts_Request;

static const uint32_t example_interfaces_srv_AddTwoInts_Request_ops [] =
{
  /* AddTwoInts_Request */
  DDS_OP_DLC,
  DDS_OP_ADR | DDS_OP_TYPE_8BY | DDS_OP_FLAG_SGN, offsetof (example_interfaces_srv_AddTwoInts_Request, a),
  DDS_OP_ADR | DDS_OP_TYPE_8BY | DDS_OP_FLAG_SGN, offsetof (example_interfaces_srv_AddTwoInts_Request, b),
  DDS_OP_RTS
};

const dds_topic_descriptor_t example_interfaces_srv_AddTwoInts_Request_desc =
{
  .m_size = sizeof (example_interfaces_srv_AddTwoInts_Request),
  .m_align = dds_alignof (example_interfaces_srv_AddTwoInts_Request),
  .m_flagset = DDS_TOPIC_FIXED_SIZE | DDS_TOPIC_XTYPES_METADATA,
  .m_nkeys = 0u,
  .m_typename = "example_interfaces::srv::AddTwoInts_Request",
  .m_keys = NULL,
  .m_nops = 4,
  .m_ops = example_interfaces_srv_AddTwoInts_Request_ops,
  .m_meta = "",
  .type_information = { .data = TYPE_INFO_CDR_example_interfaces_srv_AddTwoInts_Request, .sz = TYPE_INFO_CDR_SZ_example_interfaces_srv_AddTwoInts_Request },
  .type_mapping = { .data = TYPE_MAP_CDR_example_interfaces_srv_AddTwoInts_Request, .sz = TYPE_MAP_CDR_SZ_example_interfaces_srv_AddTwoInts_Request }
};

struct dds_cdrstream_desc desc_wr;
dds_cdrstream_desc_init(&desc_wr, &allocator, 
            example_interfaces_srv_AddTwoInts_Request_desc.m_size, 
            example_interfaces_srv_AddTwoInts_Request_desc.m_align, 
            example_interfaces_srv_AddTwoInts_Request_desc.m_flagset,
            example_interfaces_srv_AddTwoInts_Request_desc.m_ops,
            example_interfaces_srv_AddTwoInts_Request_desc.m_keys,
            example_interfaces_srv_AddTwoInts_Request_desc.m_nkeys);

example_interfaces_srv_AddTwoInts_Request req;
req.a = 123456;
req.b = 654321;

// Do serialization
bool ret = dds_stream_write_sampleLE(&os, &allocator, (void *)&req, &desc_wr);

The serialized payload generated by the above code is 0X10 0 0 0 0X40 0XE2 0X1 0 0 0 0 0 0XF1 0XFB 0X9 0 0 0 0 0.

My first thought is that cdrstream inserts payload size(16bytes) at the first 4 bytes.

If I replace the first 4 bytes with 0x00 0x01 0x00 0x00(CDR_LE) and send the request with zenoh-pico, I can get the correct response.

@JEnoch
Copy link
Contributor

JEnoch commented Apr 3, 2024

@eboasson please correct me if I'm wrong:

Looking at your generated example_interfaces_srv_AddTwoInts_Request_ops the DDS_OP_DLC op code seems to indicate that you used the -x appendable option with idlc command. Thus a DHEADER is inserted before the type. That's the 4 extra bytes you see.

rmw_cyclonedds_cpp doesn't expect XCDR encoding, but CDR encoding. Thus, you shall use -x final with idlc command.
I tested your example regenerating the AddTwoInts.c file with idlc -x final and it works.

@eboasson
Copy link
Contributor

eboasson commented Apr 4, 2024

@JEnoch you're right, but I would additionally expect that a request header needs to be prepended as well.

The RMW layer does this in a weird way, see https://github.com/ros2/rmw_cyclonedds/blob/26773ea459d115d85b026fec5b528f86b85471e4/rmw_cyclonedds_cpp/src/serdata.cpp#L380C35-L380C56 and https://github.com/ros2/rmw_cyclonedds/blob/26773ea459d115d85b026fec5b528f86b85471e4/rmw_cyclonedds_cpp/src/TypeSupport_impl.hpp#L449

The request/response headers don't show up in the message format (nor in the IDL generated by the ROS toolchain), but they need to be sent somehow and the only possibility in DDS is have it in the data. So, once upon a time, when I needed a solution without understanding the details of the ROS 2 ecosystem, I put this "prefix" thing, it worked and it stuck.

It means you have a 16 bytes prefix to the request contents and the type isn't really:

struct example_interfaces_srv_AddTwoInts_Request {
  int64_t a;
  int64_t b;
};

but rather

struct request_header {
  uint64_t guid;
  int64_t seq;
};

struct example_interfaces_srv_AddTwoInts_Request {
  struct request_header hdr;
  int64_t a;
  int64_t b;
};

the GUID is here just a random number, I'm afraid.

And, @uupks :

dds_is_get1: Assertion is->m_index < is->m_size' failed.`

am I correct in assuming you call dds_stream_read without first having called dds_stream_normalize and gotten a return value of true from it?

read is deliberately assuming well-formed input in native endianness, relying on normalize to verify well-formedness and performing byte swapping.

@JEnoch
Copy link
Contributor

JEnoch commented Apr 4, 2024

The request/response headers don't show up in the message format (nor in the IDL generated by the ROS toolchain), but they need to be sent somehow and the only possibility in DDS is have it in the data

That's the job of zenoh-bridge-ros2dds! 😃
It inserts a request_header on the fly, thus a Zenoh Service client don't have to care about it. Internally Zenoh deals with its own identifiers for queries.
Symmetrically, the header is stripped from the Reply before routing to Zenoh Service client.

@eboasson
Copy link
Contributor

eboasson commented Apr 4, 2024

The request/response headers don't show up in the message format (nor in the IDL generated by the ROS toolchain), but they need to be sent somehow and the only possibility in DDS is have it in the data

That's the job of zenoh-bridge-ros2dds! 😃

Nice! 😀

@uupks
Copy link
Author

uupks commented Apr 7, 2024

Thanks @JEnoch and @eboasson, sorry for the late reply.

The zenoh pico examples are working fine now, i'll close this issue.

@uupks uupks closed this as completed Apr 7, 2024
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