eclipse-cyclonedds / cyclonedds

Eclipse Cyclone DDS project
https://projects.eclipse.org/projects/iot.cyclonedds
Other
799 stars 349 forks source link

How to use cyclonedds cdrstream #1963

Closed uupks closed 2 months ago

uupks commented 3 months ago

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.

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)
dds_is_get1: Assertion `is->m_index < is->m_size' failed.

It looks like there is something wrong with cdrstream serialization.

uupks commented 3 months ago

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

uupks commented 3 months ago
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 commented 3 months ago

@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 commented 3 months ago

@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: Assertionis->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 commented 3 months ago

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 commented 3 months ago

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 commented 2 months ago

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

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