Setting Unions with no value

[FirebarSim]

This is more of a cry for help than an issue. I am attempting to work with the OARIS IDL from the OMG. This spec uses a number of Union types to indicate if data is present, but for the life of me I cannot figure out how to use the union types when data is not present!

For example cartesian_position_z_coordinate_type(discriminator=cartesian_coordinate_type, value=7) Seems to parse ok, but cartesian_position_z_coordinate_type() cartesian_position_z_coordinate_type(discriminator=False) cartesian_position_z_coordinate_type(discriminator=None) Etc do not.

I suspect I have the wrong end of the stick on unions and would appreciate a pointer!

[eboasson]

Hi @FirebarSim, I had to give it some thought, but I found it. The trick is discriminator=False, value=None:

from time import sleep
from cyclonedds.domain import DomainParticipant
from cyclonedds.pub import DataWriter
from cyclonedds.topic import Topic

# u.idl:
'''
@appendable
union U switch (boolean) {
  case TRUE: string v;
};

@appendable
struct S {
  U v;
};
'''

from u import U, S

dp = DomainParticipant(0)
tp = Topic(dp, "S", S)
dw = DataWriter(dp, tp)

b = 0
while True:
    if b == 0:
        dw.write(S(v=U(discriminator=False, value=None)))
        b = 1
    elif b == 1:
        dw.write(S(v=U(discriminator=True, value="strand")))
        b = 2
    else:
        dw.write(S(v=U(v="bolderkar")))
        b = 0
    sleep(1)

running dynsub S in parallel then prints:

dds_find_topic: Timeout ... continuing on the assumptions that topic discovery is disabled
  1 STRUCTURE typename=S typeflags={IS_APPENDABLE}
  2   name=v memberid=0x0 memberflags={TRY_CONSTRUCT1}
  3     COMPLETE id=b32c1699e3de24481eead5a00dab
  4       UNION typename=U typeflags={IS_APPENDABLE}
  5         discriminator=memberflags={TRY_CONSTRUCT1,IS_MUST_UNDERSTAND} BOOLEAN
  6         case 1:
  7           name=v memberid=0x0 memberflags={TRY_CONSTRUCT1}
  8             STRING8_SMALL bound=0
{"v":{"_d":false}}
{"v":{"_d":true,"v":"strand"}}
{"v":{"_d":true,"v":"bolderkar"}}
{"v":{"_d":false}}

[FirebarSim]

Thankyou! I think you have pointed me at something. Could you try the exact same code except with this IDL as a sanity check?

`# u.idl: ''' @appendable union U switch (boolean) { case TRUE: string value; };

@appendable struct S { U v; }; `

I think the issue I am hitting is that OARIS has a lot of Unions where “value” is used as the field name. Then cyclonedds-python is breaking and not serializing correctly because value refers to something specific?

[eboasson]

Yes, a union case where the field is named value is indeed the problem. So, now the question is: what do we do about it? The best I came up with so far would be to rewrite the name, by prepending an underscore, just like the IDL spec says clashes with keywords should be handled. That is, the value in the IDL would turn into _value in Python. (I think IDL disallows having both value and _value in the same scope because of the obvious problems that would create.)

I have fiddled a bit with the code trying to understand why this happens, and I am pretty confident that it is the @property value definition without an accompanying setter that causes it to report AttributeError: property 'value' of 'U' object has no setter.

It gets there because the IdlUnion class overrides setters using __setattr__, in which it first sets its internal fields (like __value, __active and __discriminator) and then invokes the superclass's method for setting the "normal" fields that the user ordinarily accesses. The superclass (Object, or whatever it is called in Python) apparently checks if a property with that name has been declared, and because it has been, checks for a corresponding setter definition. It then raises an exception because the property doesn't have a setter. I tried adding a @value.setter and then it "works", where "works" means my little test script publishes the expected data.

Looking at the code, the discriminator and value properties always return the current value of the discriminator and the current associated value. I don't think one should mess with that because it would break existing code. This makes rewriting the names in the IDL compiler more attractive. I think somewhere around https://github.com/eclipse-cyclonedds/cyclonedds-python/blob/84220f42890c672b78c2c2b8bff5d824373dffc7/cyclonedds/idl/_main.py#L471 one should then forbid an instance of IdlUnion with a value or discriminator among the cases (as they should then be named _value and _discriminator). All that is a bit of a guess for me.

Just on a whim, I tried what happens if I just skip (trying) setting the attribute if it is named value (see patch below) and it also kinda seems to work. (It also breaks when I use discriminator as the name of the field in the union in exactly the same manner.) Not sure what I broke by doing that!

In any case, the pieces of the puzzle do seem to fit. And I really should try to learn more about these things in Python ...

Next project on this subject is to have a look at the IDL compiler, see how hard it would be to map value and discriminator to _value and _discriminator.

diff --git a/cyclonedds/idl/__init__.py b/cyclonedds/idl/__init__.py
index fb667b9..9501152 100644
--- a/cyclonedds/idl/__init__.py
+++ b/cyclonedds/idl/__init__.py
@@ -17,7 +17,6 @@ from .types import ValidUnionHolder
 from ._main import IdlMeta, IdlUnionMeta, IdlBitmaskMeta, IdlEnumMeta
 from ._support import Buffer, Endianness

-
 _TIS = TypeVar('_TIS', bound='IdlStruct')
 _TIU = TypeVar('_TIU', bound='IdlUnion')
 _TIB = TypeVar('_TIB', bound='IdlBitmask')
@@ -79,7 +78,8 @@ class IdlUnion(metaclass=IdlUnionMeta):
             self.__active = name
             self.__discriminator = None
             self.__value = value
-            return super().__setattr__(name, value)
+            if name != 'value':
+                return super().__setattr__(name, value)

         for label, case in self.__idl_cases__.items():
             if case[0] == name:
@@ -89,7 +89,10 @@ class IdlUnion(metaclass=IdlUnionMeta):
                 self.__active = name
                 self.__discriminator = label
                 self.__value = value
-                return super().__setattr__(name, value)
+                if name != 'value':
+                    return super().__setattr__(name, value)
+                else:
+                    return # avoid exception below ...

         raise Exception("Programmer error, should not get here.")

@@ -108,7 +111,8 @@ class IdlUnion(metaclass=IdlUnionMeta):

             if self.__idl_default__:
                 self.__active = self.__idl_default__[0]
-                return super().__setattr__(self.__idl_default__[0], value)
+                if self.__idl_default__[0] != 'value':
+                    return super().__setattr__(self.__idl_default__[0], value)
         else:
             case = self.__idl_cases__[discriminator]
             if self.__active:
@@ -117,7 +121,8 @@ class IdlUnion(metaclass=IdlUnionMeta):
             self.__active = case[0]
             self.__discriminator = discriminator
             self.__value = value
-            return super().__setattr__(case[0], value)
+            if case[0] != 'value':
+                return super().__setattr__(case[0], value)

     def get(self):
         return self.__discriminator, self.__value

[FirebarSim]

Glad to know I’ve not gone insane, for my purposes I just changed the generated module files manually, value: became val:, which is now happily talking away to an opensplice instance on another machine. I think your suggestion of a change in the IDL processor sounds appropriate.