Change IDL generation to remove "//@top-level" and use "@nested

[emammoser]

Issue and tracking information

Developer's time Estimated effort to fix (hours):

Developer's Actual time spent on fix (hours)

Issue reporter to provide a detailed description of the issue in the space below

For our generated IDL structure and messages, we have "//@top-level true/false" appended to every definition. While this is valid RTI DDS syntax, it is not valid for all DDS vendors or IDL in general. Instead, we would like this changed to be the equivalent @nested prepended to the structure definition in a similar fashion to the @final/@appendable values (i.e. no prefixed comments). The values also become the opposite. So:

"//@top-level true" becomes "@nested(FALSE)" and "//@top-level false" becomes "@nested(TRUE)"

We want this change made to the following branches:

1. streams/v2
2. streams/v3

[jwillemsen]

This issue is related with https://github.com/RemedyIT/ciaox11/pull/428, AXCIOMA is moving from @Toplevel/@top-level to @nested which is part of IDL 4.2.

[eposse]

A couple of questions.

1. If I understand correctly, the following, current pattern:

   @final
   struct TestData_msg {
       // ...
   }; //@top-level true

   should become

   @final
   @nested(TRUE)
   struct TestData_msg {
       // ...
   }; 

   Is this correct?

2. Assuming that it is correct, does the order of the annotations (@final/@appendable/@nested) matter?

3. Do we have to remove the "//@top-level true" comment?

[jwillemsen]

1. nested is the inverse of top-level, so it should become

    @final
    @nested(FALSE)
    struct TestData_msg {
        // ...
    }; 

2. The order of the annotations doesn't matter, IDL4 defines a lot more possible annotations
3. Yes, //@top-level true should be removed, it is a RTI specific annotation for which we had a hack in AXCIOMA which was causing problems when working on more IDL4 annotation support

[emammoser]

@eposse I agree with Johnny above. Let me know if you have any other questions

[eposse]

Two more questions:

1. @nested(TRUE) is the default, correct?
2. if the answer is yes, is it still required to add @nested(TRUE) or is @nested(FALSE) the only one required (if the struct or message is top-level)?

[emammoser]

@nested(TRUE) is the default, correct? if the answer is yes, is it still required to add @nested(TRUE) or is @nested(FALSE) the only one required (if the struct or message is top-level)?

I am not sure what you mean by default. We don't have any selectable options for "top-level" at the moment, so I wouldn't expect there to be any for "nested" now. As far as I know, if we have a struct (CXStruct), it should be @nested(TRUE) and if we have a message (DDSMessage) it should be @nested(FALSE)

[eposse]

By default I meant that any declared CXStruct or DDSMessage was "nested", but your answer clarifies that CXStructs are nested and DDSMessages are not.

But the second question is still relevant. While it is clear that a DDSMessage must have the @nested(FALSE) annotation, should every CXStruct have an explicit @nested(TRUE) annotation?

[emammoser]

Yes, I believe that is how it works currently with top-level, right? At least looking through generated IDL that we have, that is the case.

[eposse]

I've tested the solution and prepared the builds for you to test:

• build for streams/v2
• build for streams/v3

Let me know if you have any issues.

[eposse]

Here are the updated builds:

• build for streams/v2
• build for streams/v3

Let me know if you have any further comments.

[eposse]

And the latest updated build:

• build for streams/v2
• build for streams/v3

[eposse]

By the way, should I add these changes to the streams/v2.5.x-maintenance branch too?

[emammoser]

No, that streams maintenance branch is meant to work with a specific version of AXCIOMA (which maybe we should document in CX? Not sure) and that version of AXCIOMA does not use @nested, it only uses the top-level annotation

[eposse]

Ok. Let me know when you have tested to merge it. Thanks.

[emammoser]

I have tested the v2 stream and it worked as expected. So I think you can merge them.

[eposse]

Closing