eboasson
commented
9 months ago Hi @jeffpg144, you raise an interesting issue. I would say that what Cyclone does is in line with the specs:
XTypes 1.3, 7.2.4.4.7 "Enumerated Types", table 18 gives for ENUMERATION_TYPE assignability:
ENUMERATION_TYPEif and only if:
- T1.extensibility == T2.extensibility
- Any literals that have the same name in T1 and T2 also have the same value, and any literals that have the same value in T1 and T2 also have the same name. This behavior may be modified with the
@ignore_literal_namesannotation, see 7.3.1.2.1.11.- If extensibility is final T1 and T2 have the same literals.
and for "Object construction":
Choose the corresponding T1 literal if it exists.
If the name or value of the T2 object does not exist in T1, the object cannot construct any object of type T1.
I read that "the object cannot construct any object of type T1" that the deserialised must reject the sample. I suppose this is debatable because makes the concept of @appendable on an enumerated type somewhat doubtful.
But then, this:
Other OMG-DDS vendors code generate an additional value for all enumerations that indicate an “UNKNOWN VALUE” or “BAD VALUE”. Looking at the generated c code, I do not see an additional enumerated value.
Is a tad problematic at least formally on the grounds of IDL 4.2, 7.4.1.4.4.4.3 "Enumerations", which says
The list of the possible values (enumerators) that makes the enumeration, enclosed within braces ({}). Each enumerator is identified by a specific name (
). In case there are several enumerators, their names are separated by commas (,). An enumeration must contain at least one enumerator and no more than 2^32
So technically it is possible to do 2^32 symbols that get mapped to a 32-bit integer in C (most traditional C99 implementations) and hence that "bad value" may not be possible. It is a bit formalistic, I admit.
More problematic to me is that the IDL-to-C++ language mapping spec states:
6.9 Mapping for Enums
An OMG IDL enum maps directly to the corresponding C++11 type definition. When an enum is used in a structured type, its default value is the first enum value specified.
The IDL-to-C++ language mapping spec is oldish, the one for Java is much newer (2019, IDL4, so post-XTypes):
7.2.4.3.3 "Enumerations"
An IDL enum shall be mapped to a Java public enum with the same name as the IDL enum type in Pascal Case, following the transformation rules defined in 7.1.1.1.1.
The Java enum type shall include a list of the enumerators, a private member to hold the value, and a private constructor to initialize the enumerators with the constant value and name. Additionally, the Java enum type shall have the helper method valueOf(int) to get an enumerator instance from an int.
For example, the IDL:
enum AnEnum { @value(1) ONE, @value(2) TWO };Maps to:
public enum AnEnum { ONE(1), TWO(2); private int value; private AnEnum(int value) { this.value = value; } public int getValue() { return value; } public static AnEnum valueOf(int v) { // return ONE, TWO, or raise java.lang.RuntimeException } };
So also no additional magic values.
I can't find anything that says these rules do not apply to "appendable" enumerate types. So it is at best murky territory ...
There is an obvious solution to it that I like much better than defining magic additional symbols, which is to leave it to the application. An annotation that specifies which value to use for out-of-range inputs when deserialising, perhaps with a fallback to the "default literal" would be much better, in my view.
But right now, no, Cyclone doesn't do that either ...
jeffpg144
Summary
CycloneDDS subscriber fails to deserialize a XTypes appendable enumeration from a publisher sending a new enumerated value. [localhost build]$ idlc -v idlc (Eclipse Cyclone DDS) 0.11.0
Hello World, #1
1) Start with examples/helloworld. 2) Make existing types Appendable. 3) Add enumeration. 4) Update publisher and subscriber to set and print new enumeration.
Hello World, #2
1) Make a copy of examples/helloworld as helloworld2. 2) Update IDL by appending a “long newID” to the topic data type. 3) Update IDL by appending a new color to the enumeration. (CA_RED ordinal of 3) 4) Update publisher to send values for the newID and new enumeration.
Common Changes to Hello World Example
1) Subscriber calls dds_take instead of dss_read so samples are only printed once. 2) Subscriber continues to read additional samples and does not exit after the first sample.
Test Results
Pass o HW #1 pub to HW #1 sub o HW #1 pub to HW #2 sub o HW #2 pub to HW #1 sub when sending existing enumerated color. o HW #2 pub to HW #2 sub Failed o HW #2 pub to HW #1 subs when sending new enumerated value/color.
The follpwing is printed to the console. “1706620617.096516 [0] recvMC: data(application, vendor 1.16): 110fd27:cd3514d3:a0cdd270:202 #2: deserialization HelloWorldData_Msg/HelloWorldData::Msg failed (for reasons unknown)”
Expected Behavior • Expect HW #1 subscriber to deserialize published data and assigned the enumeration with a default value of 0. This is what happens for the newID field, HW #2 subscriber has a value of 0 for published samples from HW #1. • Other OMG-DDS vendors code generate an additional value for all enumerations that indicate an “UNKNOWN VALUE” or “BAD VALUE”. Looking at the generated c code, I do not see an additional enumerated value.
• After reviewing X-Types Release Notes https://github.com/eclipse-cyclonedds/cyclonedds/blob/master/docs/dev/xtypes_relnotes.md this is no mention of appendable enumerations not being supported.
Things I’ve Already Tried
• Verify the IDL syntax for extensibility is correct. Using “@appendable” before the enumeration and structure • Make sure the structure containing the enumeration is also appendable. • Updated the idlc command line to include -Werror -x appendable -f cdrstream-desc
Source code for both Hello World #1 and #2.
examples_helloworld.zip
I have ~5 years experience working with other OMG-DDS vendor software. I have recently started to test interoperability with CycloneDDS focusing on XTypes.