eclipse-cyclonedds / cyclonedds

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

DataRepresentationId common api #1924

Open trittsv opened 8 months ago

trittsv commented 8 months ago

Hello,

i dont know if the DDS-Standard provides a definition for the modern c++ api on how to get the DataRepresentationId. But different DDS-Vendors seems to have different ways like the following.

#if defined(DDS_VENDOR_CYCLONE)

#define ID_FOR_XCDR2 dds::core::policy::DataRepresentationId::XCDR2  // cyclone

#elif defined(DDS_VENDOR_CONNEXT)

#define ID_FOR_XCDR2 dds::core::policy::DataRepresentation::xcdr2() // rti connext

#else
#error "DDS_VENDOR not set"
#endif

qosReadWrite() << Ownership(OwnershipKind::EXCLUSIVE)
               << Durability(DurabilityKind::VOLATILE)
               << DataRepresentation({ID_FOR_XCDR2});

Is this defined in the standard? Could we add the static function dds::core::policy::DataRepresentation::xcdr2() to cyclonedds?

PS: I have to set this manually because otherwise cyclonedds and rti will not talk to each other.

eboasson commented 8 months ago

There is this:

7.6.3.1.1 DataRepresentationQosPolicy: Conceptual Model [...] Within the OMG-reserved range, this specification defines three representation identifiers:

  • XCDR, which corresponds to the Extended CDR Representation encoding version 1 and takes the value 0.
  • XML, which corresponds to the XM L Data Representation and takes the value 1.
  • XCDR2, which corresponds to Extended CDR Representation encoding version 2 and takes the value 2.

If one takes the "new" C++ API mapping, that means an enum class containing XCDR, XML and XCDR2. That, I think, is the basis for our choice.

However, the "new" C++ API doesn't following the IDL mapping for QoS settings in other cases, but the XTypes spec doesn't define the mapping for that API. (What else is new, I seem to be typing comments on the sorry state of the OMG's efforts a lot this week!) I suppose RTI's choice is not unreasonable within that context ...

I don't see any real problem with adding this, I figure it is actually trivial. I'd like @e-hndrks' and/or @reicheratwork's opinion on this, though because their opinion is will be based a better understanding of the consequences in this specific API.

jeffpg144 commented 8 months ago

OMG-DDS standards have never about providing common language bindings(c, C++, Java, python, C#) with the exception of XML data files for QoS or DDS secure configuration. Because I need to test interoperability between vendors, the lack of standard language bindings is a real problem. I'm suspect the "pay for a license vendors" are not big fans of the idea as it would make porting between OMG-DDS implementations easy. It's not clear to me trying to have Cyclone DDS match one other OMG-DDS vendors API(RTI) for one language(C++) and one abstraction (DataRepresentationId) will be much help. It's a very long and slippery slope.

trittsv commented 8 months ago

@jeffpg144 With the exception of a handful ifdefs we are able to compile our software with 3 different DDS-Vendors successfully (more to come). I think the DDS-Vendors hurt themself when they do Vendor-Lock-In tactics. Because it would make the standard obsolete and people would start to look for alternative protocols.

eboasson commented 8 months ago

OMG-DDS standards have never about providing common language bindings(c, C++, Java, python, C#) with the exception of XML data files for QoS or DDS secure configuration.

I beg to differ. Once upon a time, all it offered was common language bindings. No interoperability, just portability! The original DCPS spec (indeed, today, still) defines the interface in IDL, with the idea that the various language bindings simply follow the IDL-to-language mapping.

Because of this, OpenSplice and RTI (and perhaps others) offer a "somewhat compatible" C binding, most offer a "somewhat compatible" C++ binding, and the same for Java. It is always only "somewhat compatible" because some vendors opted to not follow the specified mapping where it is was inconvenient or inefficient, and some things were left unspecified in the IDL (the various "NATIVE" types).

With the "new" C++ and Java bindings, an effort was made to fix this, but, as always, incompletely ... Or perhaps it was fine when those APIs were really new — but if so, they started diverged when the ink was still wet. The current issue is an example of how XTypes didn't cover all it should have covered, and there are also differences because various vendors fixed bugs in the specification in different ways. I dare say Cyclone's C++ binding is generally among the more faithful ones.

Where Cyclone really deviates is in its C binding. I've used the "old" C binding myself and have witnessed the suffering it caused for people forced to use it, and so tried to make a C binding that works a bit better. It was originally aimed at a subset of the common DDS use cases and it, too, is pretty old by now (older than Cyclone itself). That heritage is still visible and a bit annoying. But in general, it is a much more workable C API than the one that you get if you (roughly or carefully) follow the IDL-to-C mapping.

I do think Cyclone should also get the "old" C binding, if only in a limited fashion, aimed at supporting common cases in well-behaved applications. It would make porting ancient DDS code so much easier. It should really not take much effort, but it is kinda uninteresting work that doesn't solve any immediate problem.

jeffpg144 commented 8 months ago

@jeffpg144 With the exception of a handful ifdefs we are able to compile our software with 3 different DDS-Vendors successfully (more to come). I think the DDS-Vendors hurt themself when they do Vendor-Lock-In tactics. Because it would make the standard obsolete and people would start to look for alternative protocols.

Maybe I should have removed c and C++ from the list of programming languages. Happy to learn these languages have better support and ifdef can solve many problems. I have been using Java and recently python with very different results.