-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
add XRCE DDS client #19326
add XRCE DDS client #19326
Conversation
Trying to figure out how to get this built, but am running into an issue around what seems to be an improperly generated expected size for a static assertion check for certain message types:
The uORB input generated code for uint64_t timestamp;
uint8_t type;
uint8_t _padding0[7]; // required for logger
struct trajectory_waypoint_s waypoints[5]; Whereas the generated ucdr code has a size of 0 for the waypoints field: bool ucdr_serialize_vehicle_trajectory_waypoint(const vehicle_trajectory_waypoint_s& topic, ucdrBuffer& buf)
{
if (ucdr_buffer_remaining(&buf) < 9) {
return false;
}
static_assert(sizeof(topic.timestamp) == 8, "size mismatch");
memcpy(buf.iterator, &topic.timestamp, sizeof(topic.timestamp));
buf.iterator += sizeof(topic.timestamp);
buf.offset += sizeof(topic.timestamp);
static_assert(sizeof(topic.type) == 1, "size mismatch");
memcpy(buf.iterator, &topic.type, sizeof(topic.type));
buf.iterator += sizeof(topic.type);
buf.offset += sizeof(topic.type);
static_assert(sizeof(topic.waypoints) == 0, "size mismatch");
memcpy(buf.iterator, &topic.waypoints, sizeof(topic.waypoints));
buf.iterator += sizeof(topic.waypoints);
buf.offset += sizeof(topic.waypoints);
return true;
} Looking at PX4-Autopilot/msg/templates/ucdr/msg.h.em Line 85 in 317b56c
|
On the DDS/ROS2 side of things, users ultimately need to convert the .msg files to .idl files via ros2 run rosidl_adapter msg2idl.py px4/msg However, there are two issues with the current formatting of the message files:
I wrote a quick python script that does the first step and manually corrected the 4-5 cases of nested type usage, which satisfied From there, I can now use the IDL type definitions in both pure DDS or ROS2. |
I think that what may need to happen is the replacement of bool ucdr_serialize_sensor_combined(const sensor_combined_s& topic, ucdrBuffer& buf)
{
if (ucdr_buffer_remaining(&buf) < 45) {
return false;
}
static_assert(sizeof(topic.timestamp) == 8, "size mismatch");
memcpy(buf.iterator, &topic.timestamp, sizeof(topic.timestamp));
buf.iterator += sizeof(topic.timestamp);
buf.offset += sizeof(topic.timestamp);
static_assert(sizeof(topic.gyro_rad) == 12, "size mismatch");
memcpy(buf.iterator, &topic.gyro_rad, sizeof(topic.gyro_rad));
buf.iterator += sizeof(topic.gyro_rad);
buf.offset += sizeof(topic.gyro_rad);
static_assert(sizeof(topic.gyro_integral_dt) == 4, "size mismatch");
memcpy(buf.iterator, &topic.gyro_integral_dt, sizeof(topic.gyro_integral_dt));
buf.iterator += sizeof(topic.gyro_integral_dt);
buf.offset += sizeof(topic.gyro_integral_dt);
static_assert(sizeof(topic.accelerometer_timestamp_relative) == 4, "size mismatch");
memcpy(buf.iterator, &topic.accelerometer_timestamp_relative, sizeof(topic.accelerometer_timestamp_relative));
buf.iterator += sizeof(topic.accelerometer_timestamp_relative);
buf.offset += sizeof(topic.accelerometer_timestamp_relative);
static_assert(sizeof(topic.accelerometer_m_s2) == 12, "size mismatch");
memcpy(buf.iterator, &topic.accelerometer_m_s2, sizeof(topic.accelerometer_m_s2));
buf.iterator += sizeof(topic.accelerometer_m_s2);
buf.offset += sizeof(topic.accelerometer_m_s2);
static_assert(sizeof(topic.accelerometer_integral_dt) == 4, "size mismatch");
memcpy(buf.iterator, &topic.accelerometer_integral_dt, sizeof(topic.accelerometer_integral_dt));
buf.iterator += sizeof(topic.accelerometer_integral_dt);
buf.offset += sizeof(topic.accelerometer_integral_dt);
static_assert(sizeof(topic.accelerometer_clipping) == 1, "size mismatch");
memcpy(buf.iterator, &topic.accelerometer_clipping, sizeof(topic.accelerometer_clipping));
buf.iterator += sizeof(topic.accelerometer_clipping);
buf.offset += sizeof(topic.accelerometer_clipping);
return true;
} with use of the ucdr_serialize_x() methods: bool ucdr_serialize_sensor_combined(const sensor_combined_s& topic, ucdrBuffer& buf)
{
bool ret = true;
ret &= ucdr_serialize_uint64_t( &buf, topic.timestamp );
ret &= ucdr_serialize_array_float( &buf, topic.gyro_rad, 3 );
ret &= ucdr_serialize_uint32_t( &buf, topic.gyro_integral_dt );
ret &= ucdr_serialize_int32_t( &buf, topic.accelerometer_timestamp_relative );
ret &= ucdr_serialize_array_float( &buf, topic.accelerometer_m_s2, 3 );
ret &= ucdr_serialize_uint32_t( &buf, topic.accelerometer_integral_dt );
ret &= ucdr_serialize_uint8_t( &buf, topic.accelerometer_clipping );
return ret;
} |
Unfortunately, even after the above changes, the types are still mismatching. I've taken steps to ensure that the type names, module names, and CDR fields are matching on both sides, but there still seems to be some delta. Wondering if a flag needs to be used to relax type consistency checks relating to member names on the DDS side. I know RTI has the following options:
Given that the XRCE client code is not currently declaring type information for the topics, including their field names, this may be what is failing. @bkueng as part of your testing, did you validate ROS2/DDS-side communications to the Agent? If so, can you share your steps? |
I've now confirmed communication on the DDS side with both FastDDS and CycloneDDS. ConnextDDS is reporting that it is successfully matching, but samples don't seem to get delivered from XRCE Agent when requested by a Connext Datareader. Not sure what is going on here yet. Maybe XCDR vs XCDR2 settings. Opened an issue here: eProsima/Micro-XRCE-DDS-Agent#307 On the PX4 side, it looks like the remaining steps are generally:
|
Hello @bkueng, I have been checking this PR and I have some comments.
|
Thanks for checking @pablogs9.
Alignment is considered, and the message size is below the MTU, or rather I used the best-effort streams, and we're only using little-endian systems, thus I don't see a problem here. Can you be specific what you think is problematic?
My concern here is reliability: if some (corrupt) data can cause an uncaught exception (crash), it is a problem.
|
Corrupt data should not raise any uncaught exception that ends in a crash, but if you detect any case where it happens we will fix it ASAP.
There is not an active API for doing ping in a non-blocking manner. A workaround can be: |
Possible fix for Fast DDS & RTI Connext issue here: eProsima/Fast-DDS#2580 Once it is merged we will update the Micro XRCE-DDS Agent |
Noted.
Ok, I'm directly checking for |
According to what we have been talked here there should not be any alignment error in your implementation. Probably the issue cames from eProsima/Fast-DDS#2580
Let us know which kind of API would fit your requirements and we will make a proposal. |
This is getting ready to merge (I can change from xml to binary later, mostly a flash optimization).
I just realized I can also check for |
This adds eProsima's XRCE DDS bridge in order to bridge uORB to DDS.
Notes
Serialization
CDR serialization code is directly generated from .msg files and templates instead of using XRCE-DDS Gen. This has some advantages:
The only caveat is that the
ucdrBuffer
is accessed directly (somewhat internal API).Tested
TODO
Overall I'm not yet convinced to go with XRCE, but with this it's in a state where we can test it further.
@spiderkeys @pablogs9